Reject even moduli in sp_exptmod_nct when the generic path is not built

Author: stenslae

.github/configs/os-check-linux.json

@@ -543,9 +543,13 @@
  "configure": ["--enable-cryptonly", "--enable-rsavfy",
   "--enable-ecccustcurves=all", "--disable-examples"]},
 {"name": "cryptonly-rsapub-no-dh-public-mp", "minutes": 1.0,
- "comment": "RSA public only with DH off, so sp_exptmod_ex hands every modulus to sp_exptmod_nct and the constant-time generic exponentiation is not compiled. WOLFSSL_PUBLIC_MP lets wolfcrypt test run mp_test, whose even modulus checks cover the non-Montgomery path of sp_exptmod_nct in this shape.",
+ "comment": "RSA public only with DH off, so sp_exptmod_ex hands every modulus to sp_exptmod_nct and the constant-time generic exponentiation is not compiled. WOLFSSL_PUBLIC_MP lets wolfcrypt test run mp_test, whose even modulus checks confirm sp_exptmod_nct returns MP_VAL in this shape instead of a wrong Montgomery result.",
  "configure": ["--enable-cryptonly", "--enable-rsapub", "--disable-dh",
   "--disable-examples", "CPPFLAGS=-DWOLFSSL_PUBLIC_MP"]},
+{"name": "cryptonly-rsapub-public-mp", "minutes": 1.0,
+ "comment": "RSA public only with DH at its default (on), so the generic exponentiation is compiled and sp_exptmod_nct must use it for an even modulus rather than Montgomery. WOLFSSL_PUBLIC_MP lets wolfcrypt test run mp_test's even modulus checks in this shape.",
+ "configure": ["--enable-cryptonly", "--enable-rsapub",
+  "--disable-examples", "CPPFLAGS=-DWOLFSSL_PUBLIC_MP"]},
 {"name": "pkcs12-pbkdf-mp-api", "minutes": 1.0,
  "comment": "WC_PKCS12_PBKDF_USING_MP_API selects a second wc_PKCS12_PBKDF_ex implementation that no configure option reaches.",
  "configure": ["--enable-pkcs12", "--enable-des3", "--enable-opensslextra",

wolfcrypt/src/sp_int.c

@@ -14199,7 +14199,8 @@ static int _sp_exptmod_base_2(const sp_int* e, int digits, const sp_int* m,
  *
  * @return  MP_OKAY on success.
  * @return  MP_VAL when b, e, m or r is NULL, digits is negative, or m <= 0 or
- *          e is negative.
+ *          e is negative; or m is even in an RSA verify/public only build
+ *          without DH.
  * @return  MP_MEM when dynamic memory allocation fails.
  */
 int sp_exptmod_ex(const sp_int* b, const sp_int* e, int digits, const sp_int* m,
@@ -14830,62 +14831,6 @@ static int _sp_exptmod_nct(const sp_int* b, const sp_int* e, const sp_int* m,
 }
 #endif /* WOLFSSL_SP_FAST_NCT_EXPTMOD || !WOLFSSL_SP_SMALL */
 
-#if defined(WOLFSSL_RSA_VERIFY_ONLY) || defined(WOLFSSL_RSA_PUBLIC_ONLY)
-/* Internal. Exponentiates b to the power of e modulo m into r: r = b ^ e mod m
- * Left-to-right binary square-and-multiply, reducing by m at each step.
- * Works for any modulus including even moduli, which Montgomery form does not
- * support. Is NOT constant time.
- *
- * Assumes e is greater than zero.
- *
- * @param [in]  b  SP integer that is the base.
- * @param [in]  e  SP integer that is the exponent.
- * @param [in]  m  SP integer that is the modulus.
- * @param [out] r  SP integer to hold result.
- *
- * @return  MP_OKAY on success.
- * @return  MP_MEM when dynamic memory allocation fails.
- */
-static int _sp_exptmod_nct_generic(const sp_int* b, const sp_int* e,
-    const sp_int* m, sp_int* r)
-{
-    int i;
-    int err = MP_OKAY;
-    int bits = sp_count_bits(e);
-    DECL_SP_INT_ARRAY(t, m->used * 2 + 1, 2);
-
-    /* Allocate memory for:
-     *  - base reduced modulo m
-     *  - temporary result (in case r is same var as another parameter). */
-    ALLOC_SP_INT_ARRAY(t, m->used * 2 + 1, 2, err, NULL);
-    if (err == MP_OKAY) {
-        _sp_init_size(t[0], m->used * 2 + 1);
-        _sp_init_size(t[1], m->used * 2 + 1);
-        /* t[1] = b mod m */
-        err = sp_mod(b, m, t[1]);
-    }
-    if (err == MP_OKAY) {
-        /* Result starts as base as top bit of exponent is set. */
-        _sp_copy(t[1], t[0]);
-    }
-    /* For each bit in exponent starting at second highest. */
-    for (i = bits - 2; (err == MP_OKAY) && (i >= 0); i--) {
-        err = sp_sqrmod(t[0], m, t[0]);
-        if ((err == MP_OKAY) &&
-                (((e->dp[i >> SP_WORD_SHIFT] >> (i & SP_WORD_MASK)) & 1) !=
-                 0)) {
-            err = sp_mulmod(t[0], t[1], m, t[0]);
-        }
-    }
-    if (err == MP_OKAY) {
-        _sp_copy(t[0], r);
-    }
-
-    FREE_SP_INT_ARRAY(t, NULL);
-    return err;
-}
-#endif /* WOLFSSL_RSA_VERIFY_ONLY || WOLFSSL_RSA_PUBLIC_ONLY */
-
 /* Exponentiates b to the power of e modulo m into r: r = b ^ e mod m
  * Non-constant time implementation.
  *
@@ -14895,7 +14840,8 @@ static int _sp_exptmod_nct_generic(const sp_int* b, const sp_int* e,
  * @param [out] r  SP integer to hold result.
  *
  * @return  MP_OKAY on success.
- * @return  MP_VAL when b, e, m or r is NULL; or m <= 0 or e is negative.
+ * @return  MP_VAL when b, e, m or r is NULL; or m <= 0 or e is negative; or
+ *          m is even in an RSA verify/public only build without DH.
  * @return  MP_MEM when dynamic memory allocation fails.
  */
 int sp_exptmod_nct(const sp_int* b, const sp_int* e, const sp_int* m, sp_int* r)
@@ -14942,14 +14888,17 @@ int sp_exptmod_nct(const sp_int* b, const sp_int* e, const sp_int* m, sp_int* r)
     else if (m->used * 2 >= r->size) {
         err = MP_VAL;
     }
-#if !defined(WOLFSSL_RSA_VERIFY_ONLY) && !defined(WOLFSSL_RSA_PUBLIC_ONLY)
+#if (defined(WOLFSSL_SP_MATH_ALL) && !defined(WOLFSSL_RSA_VERIFY_ONLY) && \
+    !defined(WOLFSSL_RSA_PUBLIC_ONLY)) || !defined(NO_DH) || \
+    defined(OPENSSL_ALL)
     else if (mp_iseven(m)) {
         err = _sp_exptmod_ex(b, e, (int)(e->used * SP_WORD_SIZE), m, r);
     }
 #else
-    /* Montgomery form requires an odd modulus. */
+    /* Montgomery form requires an odd modulus and the generic implementation
+     * is not compiled in (RSA verify/public only without DH). */
     else if (mp_iseven(m)) {
-        err = _sp_exptmod_nct_generic(b, e, m, r);
+        err = MP_VAL;
     }
 #endif
     else {

wolfcrypt/test/test.c

@@ -95254,7 +95254,10 @@ static wc_test_ret_t mp_test_exptmod(mp_int* b, mp_int* e, mp_int* m, mp_int* r,
 
     /* Even, multi-word modulus uses the generic exponentiation path for
      * both mp_exptmod_ex and mp_exptmod_nct.  Verify b^6 mod m against
-     * repeated modular multiplication. */
+     * repeated modular multiplication.  RSA public only builds without DH
+     * do not compile the generic path, and mp_exptmod_ex hands every modulus
+     * to mp_exptmod_nct, so both must reject the even modulus instead of
+     * returning a wrong Montgomery result. */
     if ((ret = mp_set(m, 1)) != MP_OKAY ||
             (ret = mp_mul_2d(m, 521, m)) != MP_OKAY ||   /* m = 2^521 (even) */
             (ret = mp_set(b, 7)) != MP_OKAY ||
@@ -95267,6 +95270,17 @@ static wc_test_ret_t mp_test_exptmod(mp_int* b, mp_int* e, mp_int* m, mp_int* r,
             return WC_TEST_RET_ENC_EC(ret);
         }
     }
+#if defined(WOLFSSL_RSA_PUBLIC_ONLY) && defined(NO_DH) && \
+    !defined(OPENSSL_ALL)
+    ret = mp_exptmod_ex(b, e, (int)e->used, m, r);
+    if (ret != WC_NO_ERR_TRACE(MP_VAL)) {
+        return WC_TEST_RET_ENC_EC(ret);
+    }
+    ret = mp_exptmod_nct(b, e, m, r);
+    if (ret != WC_NO_ERR_TRACE(MP_VAL)) {
+        return WC_TEST_RET_ENC_EC(ret);
+    }
+#else
     if ((ret = mp_exptmod_ex(b, e, (int)e->used, m, r)) != MP_OKAY) {
         return WC_TEST_RET_ENC_EC(ret);
     }
@@ -95279,84 +95293,7 @@ static wc_test_ret_t mp_test_exptmod(mp_int* b, mp_int* e, mp_int* m, mp_int* r,
     if (mp_cmp(r, t) != MP_EQ) {
         return WC_TEST_RET_ENC_NC;
     }
-
-    /* Even modulus, more varied inputs:
-     *   k == 0: base >= m (m + 7) must give the same result as base 7.
-     *   k == 1: exponent 2^130 + 1 spans several words for any word size.
-     *           Reference: square the base 130 times, then multiply by it.
-     *   k == 2: single word even modulus 10 with base 17 >= m, exponent 6. */
-    for (k = 0; k < 3; k++) {
-        int i;
-        int sqrs = 0;
-
-        if ((ret = mp_set(m, 1)) != MP_OKAY) {
-            return WC_TEST_RET_ENC_EC(ret);
-        }
-        if (k == 2) {
-            ret = mp_set(m, 10);
-        }
-        else {
-            ret = mp_mul_2d(m, 521, m);                   /* m = 2^521 */
-        }
-        if (ret == MP_OKAY) {
-            ret = mp_set(b, 7);
-        }
-        if (ret == MP_OKAY) {
-            ret = mp_set(t, 1);
-        }
-        if (ret != MP_OKAY) {
-            return WC_TEST_RET_ENC_EC(ret);
-        }
-
-        if (k == 1) {
-            /* t = b^(2^130 + 1) mod m */
-            sqrs = 130;
-            if ((ret = mp_copy(b, t)) != MP_OKAY) {
-                return WC_TEST_RET_ENC_EC(ret);
-            }
-            for (i = 0; i < sqrs; i++) {
-                if ((ret = mp_sqrmod(t, m, t)) != MP_OKAY) {
-                    return WC_TEST_RET_ENC_EC(ret);
-                }
-            }
-            if ((ret = mp_mulmod(t, b, m, t)) != MP_OKAY) {
-                return WC_TEST_RET_ENC_EC(ret);
-            }
-            if ((ret = mp_set(e, 1)) != MP_OKAY ||
-                    (ret = mp_mul_2d(e, sqrs, e)) != MP_OKAY ||
-                    (ret = mp_add_d(e, 1, e)) != MP_OKAY) {
-                return WC_TEST_RET_ENC_EC(ret);
-            }
-        }
-        else {
-            /* t = b^6 mod m, with the reduced base. */
-            if ((ret = mp_set(e, 6)) != MP_OKAY) {
-                return WC_TEST_RET_ENC_EC(ret);
-            }
-            for (i = 0; i < 6; i++) {
-                if ((ret = mp_mulmod(t, b, m, t)) != MP_OKAY) {
-                    return WC_TEST_RET_ENC_EC(ret);
-                }
-            }
-            /* Bring the base to m + b so it needs reducing. */
-            if ((ret = mp_add(b, m, b)) != MP_OKAY) {
-                return WC_TEST_RET_ENC_EC(ret);
-            }
-        }
-
-        if ((ret = mp_exptmod_ex(b, e, (int)e->used, m, r)) != MP_OKAY) {
-            return WC_TEST_RET_ENC_EC(ret);
-        }
-        if (mp_cmp(r, t) != MP_EQ) {
-            return WC_TEST_RET_ENC_NC;
-        }
-        if ((ret = mp_exptmod_nct(b, e, m, r)) != MP_OKAY) {
-            return WC_TEST_RET_ENC_EC(ret);
-        }
-        if (mp_cmp(r, t) != MP_EQ) {
-            return WC_TEST_RET_ENC_NC;
-        }
-    }
+#endif
 
 #if !defined(HAVE_FIPS) || FIPS_VERSION3_GE(7,0,0)