Merge pull request #339 from sebastian-carpenter/rsa-validate-tests

rsa key validation test and additions to rsa fromdata test

Author: ColtonWilley

src/wp_rsa_kmgmt.c

@@ -203,6 +203,11 @@ static const char* wp_rsa_param_key[WP_RSA_PARAM_NUMS_CNT] = {
     OSSL_PKEY_PARAM_RSA_EXPONENT1, OSSL_PKEY_PARAM_RSA_EXPONENT2,
     OSSL_PKEY_PARAM_RSA_COEFFICIENT1
 };
+#define WP_RSA_PARAM_KEY_FACTOR_INDEX1 3
+#define WP_RSA_PARAM_KEY_FACTOR_INDEX2 4
+#define WP_RSA_PARAM_KEY_EXPONENT_INDEX1 5
+#define WP_RSA_PARAM_KEY_EXPONENT_INDEX2 6
+#define WP_RSA_PARAM_KEY_COEFFICIENT_INDEX 7
 
 /**
  * RSA PSS parameters.
@@ -1186,6 +1191,29 @@ static int wp_rsa_match(const wp_Rsa* rsa1, const wp_Rsa* rsa2, int selection)
     return ok;
 }
 
+/* These are the primes used by OSSL to reject an RSA N with a small GCD */
+static const mp_digit validate_primes[] = {
+   0x0002, 0x0003, 0x0005, 0x0007, 0x000B, 0x000D, 0x0011, 0x0013,
+   0x0017, 0x001D, 0x001F, 0x0025, 0x0029, 0x002B, 0x002F, 0x0035,
+   0x003B, 0x003D, 0x0043, 0x0047, 0x0049, 0x004F, 0x0053, 0x0059,
+   0x0061, 0x0065, 0x0067, 0x006B, 0x006D, 0x0071, 0x007F, 0x0083,
+   0x0089, 0x008B, 0x0095, 0x0097, 0x009D, 0x00A3, 0x00A7, 0x00AD,
+   0x00B3, 0x00B5, 0x00BF, 0x00C1, 0x00C5, 0x00C7, 0x00D3, 0x00DF,
+   0x00E3, 0x00E5, 0x00E9, 0x00EF, 0x00F1, 0x00FB, 0x0101, 0x0107,
+   0x010D, 0x010F, 0x0115, 0x0119, 0x011B, 0x0125, 0x0133, 0x0137,
+   0x0139, 0x013D, 0x014B, 0x0151, 0x015B, 0x015D, 0x0161, 0x0167,
+   0x016F, 0x0175, 0x017B, 0x017F, 0x0185, 0x018D, 0x0191, 0x0199,
+   0x01A3, 0x01A5, 0x01AF, 0x01B1, 0x01B7, 0x01BB, 0x01C1, 0x01C9,
+   0x01CD, 0x01CF, 0x01D3, 0x01DF, 0x01E7, 0x01EB, 0x01F3, 0x01F7,
+   0x01FD, 0x0209, 0x020B, 0x021D, 0x0223, 0x022D, 0x0233, 0x0239,
+   0x023B, 0x0241, 0x024B, 0x0251, 0x0257, 0x0259, 0x025F, 0x0265,
+   0x0269, 0x026B, 0x0277, 0x0281, 0x0283, 0x0287, 0x028D, 0x0293,
+   0x0295, 0x02A1, 0x02A5, 0x02AB, 0x02B3, 0x02BD, 0x02C5, 0x02CF,
+   0x02D7, 0x02DD, 0x02E3, 0x02E7, 0x02EF,
+};
+#define VALIDATE_PRIMES_SIZE XELEM_CNT(validate_primes)
+wc_static_assert(VALIDATE_PRIMES_SIZE == 133);
+
 /**
  * Validate the RSA key.
  *
@@ -1214,26 +1242,229 @@ static int wp_rsa_validate(const wp_Rsa* rsa, int selection, int checkType)
             WOLFPROV_MSG_DEBUG_RETCODE(WP_LOG_LEVEL_DEBUG, "wc_CheckRsaKey", rc);
             ok = 0;
         }
+
+        /* wc_CheckRsaKey runs the private key operation and only bounds-checks
+         * d (d < n). Check d*e = 1 mod lcm(p-1,q-1) to match OSSL */
+        if (ok && !mp_iszero((mp_int*)&rsa->key.p) &&
+                  !mp_iszero((mp_int*)&rsa->key.q)) {
+            mp_int m;   /* p-1 then q-1 */
+            mp_int r;   /* d*e mod m    */
+
+            if (mp_init(&m) != MP_OKAY) {
+                ok = 0;
+            }
+            else if (mp_init(&r) != MP_OKAY) {
+                mp_clear(&m);
+                ok = 0;
+            }
+            else {
+                if (mp_sub_d((mp_int*)&rsa->key.p, 1, &m) != MP_OKAY ||
+                        mp_mulmod((mp_int*)&rsa->key.d, (mp_int*)&rsa->key.e,
+                                  &m, &r) != MP_OKAY || !mp_isone(&r)) {
+                    ok = 0;
+                }
+                else if (mp_sub_d((mp_int*)&rsa->key.q, 1, &m) != MP_OKAY ||
+                        mp_mulmod((mp_int*)&rsa->key.d, (mp_int*)&rsa->key.e,
+                                  &m, &r) != MP_OKAY || !mp_isone(&r)) {
+                    ok = 0;
+                }
+                mp_forcezero(&r);
+                mp_forcezero(&m);
+                mp_clear(&r);
+                mp_clear(&m);
+            }
+        }
     }
     else
 #endif
     if (checkPriv) {
-        if (mp_isone(&rsa->key.d) || mp_iszero((mp_int*)&rsa->key.d) ||
-            (mp_cmp((mp_int*)&rsa->key.d, (mp_int*)&rsa->key.n) != MP_LT)) {
+        if (mp_iszero((mp_int*)&rsa->key.d) ||
+                (mp_cmp((mp_int*)&rsa->key.d, (mp_int*)&rsa->key.n) != MP_LT)) {
             ok = 0;
         }
     }
     else if (checkPub) {
-        if (mp_iseven(&rsa->key.e) || mp_iszero((mp_int*)&rsa->key.e) ||
-            mp_isone(&rsa->key.e)) {
+        int nbits = mp_count_bits((mp_int*)&rsa->key.n);
+        mp_int res;
+
+        if (mp_iszero((mp_int*)&rsa->key.e) || mp_iszero((mp_int*)&rsa->key.n) ||
+                mp_isone((mp_int*)&rsa->key.e) || mp_isone((mp_int*)&rsa->key.n) ||
+                mp_iseven((mp_int*)&rsa->key.e) || mp_iseven((mp_int*)&rsa->key.n)) {
+            ok = 0;
+        }
+
+        if (ok && nbits > OPENSSL_RSA_MAX_MODULUS_BITS) {
+            ok = 0;
+        }
+
+#ifdef HAVE_FIPS
+        if (ok && (wolfProvider_GetFipsChecks() & WP_FIPS_CHECK_RSA_KEY_SIZE)) {
+            /* n must have at least 112 bits of security, and
+             * 2^16 < e < 2^256 */
+            int ebits = mp_count_bits((mp_int*)&rsa->key.e);
+            if (nbits < 2048 || ebits <= 16 || ebits >= 257) {
+                ok = 0;
+            }
+        }
+#endif
+
+        /* Reject a modulus that shares a small prime factor */
+        if (ok && mp_init(&res) != MP_OKAY) {
+            ok = 0;
+        }
+        else if (ok) {
+            unsigned int prime;
+            for(prime = 0; prime < VALIDATE_PRIMES_SIZE; prime++) {
+                if (mp_set_int(&res, validate_primes[prime]) != MP_OKAY ||
+                        mp_mod((mp_int*)&rsa->key.n, &res, &res) != MP_OKAY ||
+                        mp_iszero(&res)) {
+                    ok = 0;
+                    break;
+                }
+            }
+
+            mp_clear(&res);
+        }
+
+#if (!defined(WOLFSSL_SP_MATH) && !defined(WOLFSSL_SP_MATH_ALL)) || \
+    defined(WOLFSSL_SP_PRIME_GEN)
+        /* Reject prime n since it is not composite.
+         * Note this does not catch a prime power (p^k), which OSSL
+         *   also rejects. */
+        if (ok) {
+            int isPrime = MP_NO;
+            if (mp_prime_is_prime((mp_int*)&rsa->key.n, 5, &isPrime) != MP_OKAY ||
+                    isPrime == MP_YES) {
+                ok = 0;
+            }
+        }
+#endif
+    }
+
+    WOLFPROV_LEAVE(WP_LOG_COMP_RSA, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
+    return ok;
+}
+
+/**
+ * Copy an unsigned value from an OSSL param into a provided RSA key parameter.
+ * Negative values result in an error.
+ *
+ * @param [out] mp         RSA key parameter.
+ * @param [in]  param      Parameter to copy value from.
+ * @return  Size of mp in bits on success.
+ * @return  -1 on failure.
+ */
+static int wp_rsa_import_store_unsigned(mp_int* mp, const OSSL_PARAM* param)
+{
+    int ok;
+    int bits = -1;
+    const unsigned char* data;
+
+    WOLFPROV_ENTER(WP_LOG_COMP_RSA, "wp_rsa_import_store_unsigned");
+
+#if OPENSSL_VERSION_NUMBER <= 0x30100080L
+    ok = param->data != NULL && param->data_type == OSSL_PARAM_UNSIGNED_INTEGER;
+#else
+    ok = param->data != NULL && (param->data_type == OSSL_PARAM_INTEGER ||
+            param->data_type == OSSL_PARAM_UNSIGNED_INTEGER);
+#endif /* OPENSSL_VERSION_NUMBER <= 3.1.8 */
+
+    if (ok && !wp_mp_read_unsigned_bin_le(mp, param->data, param->data_size)) {
+        WOLFPROV_MSG(WP_LOG_COMP_RSA,
+            "Failed to read %s from parameters", param->key);
+        ok = 0;
+    }
+
+    if (ok) {
+        bits = mp_count_bits(mp);
+    }
+
+    /* OSSL accepts negative values for these params, but this doesn't work well
+     * in wolfProvider so reject it. */
+    if (ok && param->data_type == OSSL_PARAM_INTEGER) {
+        data = (const unsigned char*)param->data;
+        /* Assume little-endian when determining sign */
+        if (param->data_size > 0 && (data[param->data_size - 1] & 0x80) != 0) {
+            WOLFPROV_MSG(WP_LOG_COMP_RSA, "Negative value for %s rejected",
+                param->key);
             ok = 0;
         }
     }
 
     WOLFPROV_LEAVE(WP_LOG_COMP_RSA, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
+    return ok ? bits : -1;
+}
+
+/**
+ * Based on the index into wp_rsa_param_key, increment the appropriate counter.
+ *
+ * @param [in]  index      Index associated with wp_rsa_param_key.
+ * @param [out] primes     Count of prime parameters.
+ * @param [out] exps       Count of exponent parameters.
+ * @param [out] coeffs     Count of coefficient parameters.
+ */
+static void wp_rsa_import_increment_crt_counts(int index, int *primes,
+    int *exps, int *coeffs)
+{
+    if (index >= WP_RSA_PARAM_KEY_FACTOR_INDEX1 &&
+            index <= WP_RSA_PARAM_KEY_FACTOR_INDEX2) {
+        *primes += 1;
+    }
+    else if (index >= WP_RSA_PARAM_KEY_EXPONENT_INDEX1 &&
+                index <= WP_RSA_PARAM_KEY_EXPONENT_INDEX2) {
+        *exps += 1;
+    }
+    else if (index == WP_RSA_PARAM_KEY_COEFFICIENT_INDEX) {
+        *coeffs += 1;
+    }
+}
+
+/**
+ * Verify the RSA CRT parameters supplied for a private key import are complete.
+ *
+ * @param [in]  primes     Count of prime factor parameters provided.
+ * @param [in]  exps       Count of CRT exponent parameters provided.
+ * @param [in]  coeffs     Count of CRT coefficient parameters provided.
+ * @return  1 when the CRT parameters are acceptable
+ * @return  0 when the provided factors/CRT parameters are incomplete or
+ *          unsupported.
+ */
+static int wp_rsa_import_verify_crt(int primes, int exps, int coeffs)
+{
+    int ok = 1;
+
+    (void)exps;
+    (void)coeffs;
+
+    if (primes > 0) {
+#if (OPENSSL_VERSION_NUMBER < 0x30100040L && OPENSSL_VERSION_NUMBER > 0x30000120L) \
+    || (OPENSSL_VERSION_NUMBER < 0x300000C0L)
+        if (primes < 2 || primes != exps || primes != coeffs + 1) {
+#else
+        if (primes < 2) {
+#endif /* (Ver < 3.1.4 && Ver > 3.0.18) || (Ver < 3.0.12) */
+            WOLFPROV_MSG(WP_LOG_COMP_RSA,
+                "RSA factors provided but CRT parameters incomplete");
+            ok = 0;
+        }
+        /* TODO: multi-prime checks */
+        else if (primes > 2) {
+            WOLFPROV_MSG(WP_LOG_COMP_RSA, "RSA multi-prime import failed");
+            ok = 0;
+        }
+    }
+
     return ok;
 }
 
+#if OPENSSL_VERSION_NUMBER >= 0x30400000L
+    #define wp_rsa_import_check_bits(bits, nbits)   \
+        ((bits) != -1 && (bits) <= (nbits))
+#else
+    #define wp_rsa_import_check_bits(bits, nbits)   \
+        ((bits) != -1)
+#endif /* OPENSSL_VERSION_NUMBER >= 3.4.0 */
+
 /**
  * Import the key data into RSA key object from parameters.
  *
@@ -1251,20 +1482,39 @@ static int wp_rsa_import_key_data(wp_Rsa* rsa, const OSSL_PARAM params[],
     int cnt = 0;
     mp_int* mp = NULL;
     const OSSL_PARAM* p = NULL;
+    const OSSL_PARAM* n;
+    const OSSL_PARAM* e;
+    const OSSL_PARAM* d = NULL;
+    int bits = -1;
+    int nbits = -1;
+    int primes = 0;
+    int exps = 0;
+    int coeffs = 0;
 
     WOLFPROV_ENTER(WP_LOG_COMP_RSA, "wp_rsa_import_key_data");
 
     /* N and E params are the only ones required by OSSL, so match that.
      * See ossl_rsa_fromdata() and RSA_set0_key() in OpenSSL. */
-    if (OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_RSA_N) == NULL ||
-        OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_RSA_E) == NULL) {
+    if ((n = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_RSA_N)) == NULL ||
+            n->data == NULL ||
+            (e = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_RSA_E)) == NULL ||
+            e->data == NULL) {
         WOLFPROV_MSG(WP_LOG_COMP_RSA, "Param N or E is missing");
         ok = 0;
     }
+    if (ok && priv) {
+        d = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_RSA_D);
+    }
 
     if (ok) {
-        cnt = wp_params_count(params);
         rsa->key.type = priv ? RSA_PRIVATE : RSA_PUBLIC;
+        mp = &rsa->key.n;
+        nbits = wp_rsa_import_store_unsigned(mp, n);
+        ok = nbits != -1;
+    }
+
+    if (ok && d != NULL) {
+        cnt = wp_params_count(params);
 
         for (i = 0; i < cnt; i++) {
             /* Use the table to look up the offset in the rsa struct */
@@ -1273,6 +1523,8 @@ static int wp_rsa_import_key_data(wp_Rsa* rsa, const OSSL_PARAM params[],
             for (j = 0; j < (int)ARRAY_SIZE(wp_rsa_param_key); j++) {
                 if (XSTRCMP(p->key, wp_rsa_param_key[j]) == 0) {
                     index = j;
+                    wp_rsa_import_increment_crt_counts(index, &primes, &exps,
+                                                                       &coeffs);
                     break;
                 }
             }
@@ -1284,21 +1536,39 @@ static int wp_rsa_import_key_data(wp_Rsa* rsa, const OSSL_PARAM params[],
             }
 
             /* Read the value into the rsa struct */
-            if (ok) {
+            if (ok && p != n) {
                 mp = (mp_int*)(((byte*)&rsa->key) + wp_rsa_offset[index]);
-                if (!wp_mp_read_unsigned_bin_le(mp, p->data, p->data_size)) {
-                    WOLFPROV_MSG(WP_LOG_COMP_RSA,
-                        "Failed to read %s from parameters", p->key);
+                bits = wp_rsa_import_store_unsigned(mp, p);
+                ok = wp_rsa_import_check_bits(bits, nbits);
+
+                /* Reject a provided CRT parameter (dP, dQ, u) that is zero.
+                 * The key validation could otherwise pass with a 0 param */
+                if (ok && (((index >= WP_RSA_PARAM_KEY_EXPONENT_INDEX1 &&
+                        index <= WP_RSA_PARAM_KEY_EXPONENT_INDEX2) ||
+                        index == WP_RSA_PARAM_KEY_COEFFICIENT_INDEX) &&
+                        mp_iszero(mp))) {
                     ok = 0;
                 }
             }
         }
+
+        /* Verify the received RSA CRT parameters */
+        if (ok) {
+            ok = wp_rsa_import_verify_crt(primes, exps, coeffs);
+        }
+    }
+    else if (ok && d == NULL) {
+        mp = &rsa->key.e;
+        bits = wp_rsa_import_store_unsigned(mp, e);
+        ok = wp_rsa_import_check_bits(bits, nbits);
     }
 
     WOLFPROV_LEAVE(WP_LOG_COMP_RSA, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
     return ok;
 }
 
+#undef wp_rsa_import_check_bits
+
 /**
  * Import the key into RSA key object from parameters.
  *