From 3e445b5ba04cda33a57fd190d236a6716f4e5c70 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 2 Feb 2023 08:31:38 +1000 Subject: [PATCH 1/4] SP int: sp_invmod_mont_ct check err before setting Two places in sp_invmod_mont_ct were not checking err is set before performing a new operation and setting err. Change to check error before performing operation. --- wolfcrypt/src/sp_int.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/wolfcrypt/src/sp_int.c b/wolfcrypt/src/sp_int.c index d98bab6fd..ce37b7539 100644 --- a/wolfcrypt/src/sp_int.c +++ b/wolfcrypt/src/sp_int.c @@ -12051,7 +12051,9 @@ int sp_invmod_mont_ct(const sp_int* a, const sp_int* m, sp_int* r, s = bit; /* 6.4.4. t = (t * pre[j-1]) mod m */ - err = sp_mul(t, pre[j-1], t); + if (err == MP_OKAY) { + err = sp_mul(t, pre[j-1], t); + } if (err == MP_OKAY) { err = _sp_mont_red(t, m, mp); } @@ -12072,6 +12074,8 @@ int sp_invmod_mont_ct(const sp_int* a, const sp_int* m, sp_int* r, err = _sp_mont_red(t, m, mp); } } + } + if (err == MP_OKAY) { /* 8. If j > 0 then r = (t * pre[j-1]) mod m */ if (j > 0) { err = sp_mul(t, pre[j-1], r); From 1912aaf91bc45a52a405eb2e1b73788f42f367e3 Mon Sep 17 00:00:00 2001 From: Sean Parkinson Date: Thu, 2 Feb 2023 12:10:52 +1000 Subject: [PATCH 2/4] EC OpenSSL compat: validate point after setting wolfSSL_EC_POINT_set_affine_coordinates_GFp wasn't checking the point is valid for the curve. Added call to check point when setting. Made check available for opensslextra. Fixed test to have valid ordinates to set. --- src/pk.c | 25 ++++++++++++++++++------- tests/api.c | 28 +++++++++++++++++++--------- wolfssl/wolfcrypt/settings.h | 12 ++++++++++-- 3 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/pk.c b/src/pk.c index d7b740c19..1bd29d1ef 100644 --- a/src/pk.c +++ b/src/pk.c @@ -10149,7 +10149,8 @@ WOLFSSL_BIGNUM *wolfSSL_EC_POINT_point2bn(const WOLFSSL_EC_GROUP* group, return ret; } -#if defined(USE_ECC_B_PARAM) && (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) +#if defined(USE_ECC_B_PARAM) && !defined(HAVE_SELFTEST) && \ + (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) /* Check if EC point is on the the curve defined by the EC group. * * @param [in] group EC group defining curve. @@ -10190,7 +10191,7 @@ int wolfSSL_EC_POINT_is_on_curve(const WOLFSSL_EC_GROUP *group, /* Return boolean of on curve. No error means on curve. */ return !err; } -#endif /* USE_ECC_B_PARAM && !(FIPS_VERSION <= 2) */ +#endif /* USE_ECC_B_PARAM && !HAVE_SELFTEST && !(FIPS_VERSION <= 2) */ #if !defined(WOLFSSL_SP_MATH) && !defined(WOLF_CRYPTO_CB_ONLY_ECC) /* Convert Jacobian ordinates to affine. @@ -10335,9 +10336,9 @@ int wolfSSL_EC_POINT_get_affine_coordinates_GFp(const WOLFSSL_EC_GROUP* group, * @return 1 on success. * @return 0 on error. */ -int wolfSSL_EC_POINT_set_affine_coordinates_GFp(const WOLFSSL_EC_GROUP *group, - WOLFSSL_EC_POINT *point, const WOLFSSL_BIGNUM *x, const WOLFSSL_BIGNUM *y, - WOLFSSL_BN_CTX *ctx) +int wolfSSL_EC_POINT_set_affine_coordinates_GFp(const WOLFSSL_EC_GROUP* group, + WOLFSSL_EC_POINT* point, const WOLFSSL_BIGNUM* x, const WOLFSSL_BIGNUM* y, + WOLFSSL_BN_CTX* ctx) { int ret = 1; @@ -10394,6 +10395,16 @@ int wolfSSL_EC_POINT_set_affine_coordinates_GFp(const WOLFSSL_EC_GROUP *group, ret = 0; } +#if defined(USE_ECC_B_PARAM) && !defined(HAVE_SELFTEST) && \ + (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) + /* Check that the point is valid. */ + if ((ret == 1) && (wolfSSL_EC_POINT_is_on_curve(group, + (WOLFSSL_EC_POINT *)point, ctx) != 1)) { + WOLFSSL_MSG("EC_POINT_is_on_curve failed"); + ret = 0; + } +#endif + return ret; } @@ -11018,8 +11029,8 @@ int wolfSSL_EC_POINT_copy(WOLFSSL_EC_POINT *dest, const WOLFSSL_EC_POINT *src) } /* Copy internal EC points. */ - if ((ret == 1) && (wc_ecc_copy_point((ecc_point*) dest->internal, - (ecc_point*) src->internal) != MP_OKAY)) { + if ((ret == 1) && (wc_ecc_copy_point((ecc_point*)src->internal, + (ecc_point*)dest->internal) != MP_OKAY)) { ret = 0; } diff --git a/tests/api.c b/tests/api.c index dc1fac0be..eaa4e0f63 100644 --- a/tests/api.c +++ b/tests/api.c @@ -26331,7 +26331,8 @@ static int test_wc_ecc_pointFns(void) } } -#if !defined(HAVE_FIPS) || (defined(HAVE_FIPS_VERSION) && (HAVE_FIPS_VERSION>2)) +#if !defined(HAVE_SELFTEST) && (!defined(HAVE_FIPS) || \ + (defined(HAVE_FIPS_VERSION) && (HAVE_FIPS_VERSION>2))) #ifdef USE_ECC_B_PARAM /* On curve if ret == 0 */ if (ret == 0) { @@ -26351,7 +26352,7 @@ static int test_wc_ecc_pointFns(void) } } #endif /* USE_ECC_B_PARAM */ -#endif /* !HAVE_FIPS || HAVE_FIPS_VERSION > 2 */ +#endif /* !HAVE_SELFTEST && (!HAVE_FIPS || HAVE_FIPS_VERSION > 2) */ /* Free */ wc_ecc_del_point(point); @@ -56225,14 +56226,10 @@ static int test_wolfSSL_EC_POINT(void) /* check if point X coordinate is zero */ AssertIntEQ(BN_is_zero(new_point->X), 0); -#ifdef USE_ECC_B_PARAM +#if defined(USE_ECC_B_PARAM) && !defined(HAVE_SELFTEST) && \ + (!defined(HAVE_FIPS) || FIPS_VERSION_GT(2,0)) AssertIntEQ(EC_POINT_is_on_curve(group, new_point, ctx), 1); -#endif /* USE_ECC_B_PARAM */ - - /* Force non-affine coordinates */ - AssertIntEQ(BN_add(new_point->Z, (WOLFSSL_BIGNUM*)BN_value_one(), - (WOLFSSL_BIGNUM*)BN_value_one()), 1); - new_point->inSet = 0; +#endif /* extract the coordinates from point */ AssertIntEQ(EC_POINT_get_affine_coordinates_GFp(group, new_point, X, Y, @@ -56267,6 +56264,19 @@ static int test_wolfSSL_EC_POINT(void) AssertIntEQ(EC_POINT_invert(group, NULL, ctx), 0); AssertIntEQ(EC_POINT_invert(group, new_point, ctx), 1); + /* Test getting affine converts from projective. */ + AssertIntEQ(EC_POINT_copy(set_point, new_point), 1); + /* Force non-affine coordinates */ + AssertIntEQ(BN_add(new_point->Z, (WOLFSSL_BIGNUM*)BN_value_one(), + (WOLFSSL_BIGNUM*)BN_value_one()), 1); + new_point->inSet = 0; + /* extract the coordinates from point */ + AssertIntEQ(EC_POINT_get_affine_coordinates_GFp(group, new_point, X, Y, + ctx), WOLFSSL_SUCCESS); + /* check if point ordinates have changed. */ + AssertIntNE(BN_cmp(X, set_point->X), 0); + AssertIntNE(BN_cmp(Y, set_point->Y), 0); + /* Test check for infinity */ #ifndef WOLF_CRYPTO_CB_ONLY_ECC AssertIntEQ(EC_POINT_is_at_infinity(NULL, NULL), 0); diff --git a/wolfssl/wolfcrypt/settings.h b/wolfssl/wolfcrypt/settings.h index 8bb3d87f7..2a2089e75 100644 --- a/wolfssl/wolfcrypt/settings.h +++ b/wolfssl/wolfcrypt/settings.h @@ -1975,7 +1975,6 @@ extern void uITRON4_free(void *p) ; - /* user can specify what curves they want with ECC_USER_CURVES otherwise * all curves are on by default for now */ #ifndef ECC_USER_CURVES @@ -2010,7 +2009,8 @@ extern void uITRON4_free(void *p) ; /* ECC Configs */ #ifdef HAVE_ECC - /* By default enable Sign, Verify, DHE, Key Import and Key Export unless explicitly disabled */ + /* By default enable Sign, Verify, DHE, Key Import and Key Export unless + * explicitly disabled */ #if !defined(NO_ECC_SIGN) && \ (!defined(ECC_TIMING_RESISTANT) || \ (defined(ECC_TIMING_RESISTANT) && !defined(WC_NO_RNG))) @@ -2039,6 +2039,14 @@ extern void uITRON4_free(void *p) ; #endif #endif /* HAVE_ECC */ +#if defined(OPENSSL_EXTRA) && defined(HAVE_ECC) && \ + !defined(WOLFSSL_ATECC508A) && !defined(WOLFSSL_ATECC608A) && \ + !defined(WOLFSSL_CRYPTOCELL) && !defined(WOLFSSL_SE050) && \ + !defined(WOLF_CRYPTO_CB_ONLY_ECC) && !defined(WOLFSSL_STM32_PKA) + #undef USE_ECC_B_PARAM + #define USE_ECC_B_PARAM +#endif + /* Curve25519 Configs */ #ifdef HAVE_CURVE25519 /* By default enable shared secret, key export and import */ From 62cfd8725a3b4d9fb406c5c2689f448a51566ef2 Mon Sep 17 00:00:00 2001 From: Andras Fekete Date: Mon, 6 Feb 2023 11:03:41 -0500 Subject: [PATCH 3/4] Disable latest OpenWrt test --- .github/workflows/docker-OpenWrt.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docker-OpenWrt.yml b/.github/workflows/docker-OpenWrt.yml index 70c05ffe1..08e3f04cf 100644 --- a/.github/workflows/docker-OpenWrt.yml +++ b/.github/workflows/docker-OpenWrt.yml @@ -29,7 +29,7 @@ jobs: needs: build_library strategy: matrix: - release: [ "snapshot", "22.03-SNAPSHOT", "21.02-SNAPSHOT" ] # some other versions: 21.02.0 21.02.5 22.03.0 22.03.3 + release: [ "22.03-SNAPSHOT", "21.02-SNAPSHOT" ] # some other versions: 21.02.0 21.02.5 22.03.0 22.03.3 snapshot steps: - uses: actions/checkout@v3 - uses: docker/setup-buildx-action@v2 From 7a6ed68f2dbbbf4c2f4aee347a80df0182f5223a Mon Sep 17 00:00:00 2001 From: Anthony Hu Date: Fri, 3 Feb 2023 10:36:26 -0500 Subject: [PATCH 4/4] Ensure that i2d APIs for public keys gives appropriate data. --- src/pk.c | 10 ++-- src/ssl.c | 116 ++++++++++++++++++++++++++++++++++++++++++++- tests/api.c | 10 +++- wolfssl/internal.h | 2 +- 4 files changed, 129 insertions(+), 9 deletions(-) diff --git a/src/pk.c b/src/pk.c index 1bd29d1ef..3e31f5202 100644 --- a/src/pk.c +++ b/src/pk.c @@ -1200,7 +1200,7 @@ WOLFSSL_RSA* wolfSSL_RSAPublicKey_dup(WOLFSSL_RSA *rsa) #endif /* WOLFSSL_KEY_GEN && !HAVE_USER_RSA */ -#if defined(WOLFSSL_KEY_GEN) && !defined(HAVE_USER_RSA) +#ifndef HAVE_USER_RSA static int wolfSSL_RSA_To_Der_ex(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey, void* heap); #endif @@ -1307,7 +1307,7 @@ WOLFSSL_RSA *wolfSSL_d2i_RSAPrivateKey(WOLFSSL_RSA **out, return rsa; } -#if defined(WOLFSSL_KEY_GEN) && !defined(HAVE_USER_RSA) && \ +#if defined(OPENSSL_EXTRA) && !defined(HAVE_USER_RSA) && \ !defined(HAVE_FAST_RSA) /* Converts an internal RSA structure to DER format for the private key. * @@ -1382,7 +1382,7 @@ int wolfSSL_i2d_RSAPublicKey(WOLFSSL_RSA *rsa, unsigned char **pp) return ret; } -#endif /* defined(WOLFSSL_KEY_GEN) && !defined(HAVE_USER_RSA) && +#endif /* defined(OPENSSL_EXTRA) && !defined(HAVE_USER_RSA) && * !defined(HAVE_FAST_RSA) */ #endif /* OPENSSL_EXTRA */ @@ -1512,7 +1512,7 @@ WOLFSSL_RSA* wolfSSL_d2i_RSAPrivateKey_bio(WOLFSSL_BIO *bio, WOLFSSL_RSA **out) #ifdef OPENSSL_EXTRA -#if defined(WOLFSSL_KEY_GEN) && !defined(HAVE_USER_RSA) +#ifndef HAVE_USER_RSA /* Create a DER encoding of key. * * Not OpenSSL API. @@ -1645,7 +1645,7 @@ static int wolfSSL_RSA_To_Der_ex(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey, WOLFSSL_LEAVE("wolfSSL_RSA_To_Der", ret); return ret; } -#endif /* WOLFSSL_KEY_GEN && !HAVE_USER_RSA */ +#endif /* !HAVE_USER_RSA */ #endif /* OPENSSL_EXTRA */ diff --git a/src/ssl.c b/src/ssl.c index 5e43f2fef..e28ca0bf5 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -10402,6 +10402,9 @@ WOLFSSL_EVP_PKEY* wolfSSL_d2i_PUBKEY(WOLFSSL_EVP_PKEY** out, return d2iGenericKey(out, in, inSz, 0); } +#if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && !defined(NO_ASN) && \ + !defined(NO_PWDBASED) + /* helper function to get raw pointer to DER buffer from WOLFSSL_EVP_PKEY */ static int wolfSSL_EVP_PKEY_get_der(const WOLFSSL_EVP_PKEY* key, unsigned char** der) { @@ -10439,9 +10442,11 @@ static int wolfSSL_EVP_PKEY_get_der(const WOLFSSL_EVP_PKEY* key, unsigned char** int wolfSSL_i2d_PUBKEY(const WOLFSSL_EVP_PKEY *key, unsigned char **der) { - return wolfSSL_EVP_PKEY_get_der(key, der); + return wolfSSL_i2d_PublicKey(key, der); } +#endif /* OPENSSL_EXTRA && !NO_CERTS && !NO_ASN && !NO_PWDBASED */ + static WOLFSSL_EVP_PKEY* _d2i_PublicKey(int type, WOLFSSL_EVP_PKEY** out, const unsigned char **in, long inSz, int priv) { @@ -22949,7 +22954,114 @@ int wolfSSL_i2d_PrivateKey(const WOLFSSL_EVP_PKEY* key, unsigned char** der) int wolfSSL_i2d_PublicKey(const WOLFSSL_EVP_PKEY *key, unsigned char **der) { - return wolfSSL_EVP_PKEY_get_der(key, der); +#if !defined(NO_RSA) || defined(HAVE_ECC) +#ifdef HAVE_ECC + unsigned char *local_der = NULL; + word32 local_derSz = 0; + unsigned char *pub_der = NULL; + ecc_key eccKey; + word32 inOutIdx = 0; +#endif + word32 pub_derSz = 0; + int ret = 0; + int key_type = 0; + + if (key == NULL) { + return WOLFSSL_FATAL_ERROR; + } + + key_type = key->type; + if ((key_type != EVP_PKEY_EC) && (key_type != EVP_PKEY_RSA)) { + ret = WOLFSSL_FATAL_ERROR; + } + +#ifndef NO_RSA + if (key_type == EVP_PKEY_RSA) { + return wolfSSL_i2d_RSAPublicKey(key->rsa, der); + } +#endif + + /* Now that RSA is taken care of, we only need to consider the ECC case. */ + +#ifdef HAVE_ECC + + /* We need to get the DER, then convert it to a public key. But what we get + * might be a buffereed private key so we need to decode it and then encode + * the public part. */ + if (ret == 0) { + local_derSz = wolfSSL_EVP_PKEY_get_der(key, &local_der); + if (local_derSz <= 0) { + ret = WOLFSSL_FATAL_ERROR; + } + } + + if (ret == 0) { + ret = wc_ecc_init(&eccKey); + } + + if (ret == 0) { + ret = wc_EccPublicKeyDecode(local_der, &inOutIdx, &eccKey, local_derSz); + } + + if (ret == 0) { + pub_derSz = wc_EccPublicKeyDerSize(&eccKey, 0); + if (pub_derSz <= 0) { + ret = WOLFSSL_FAILURE; + } + } + + if (ret == 0) { + pub_der = (unsigned char*)XMALLOC(pub_derSz, NULL, + DYNAMIC_TYPE_PUBLIC_KEY); + if (pub_der == NULL) { + WOLFSSL_MSG("Failed to allocate output buffer."); + ret = WOLFSSL_FATAL_ERROR; + } + } + + if (ret == 0) { + pub_derSz = wc_EccPublicKeyToDer(&eccKey, pub_der, pub_derSz, 0); + if (pub_derSz <= 0) { + ret = WOLFSSL_FATAL_ERROR; + } + } + + /* This block is for actually returning the DER of the public key */ + if ((ret == 0) && (der != NULL)) { + if (*der == NULL) { + *der = (unsigned char*)XMALLOC(pub_derSz, NULL, + DYNAMIC_TYPE_PUBLIC_KEY); + if (*der == NULL) { + WOLFSSL_MSG("Failed to allocate output buffer."); + ret = WOLFSSL_FATAL_ERROR; + } + + if (ret == 0) { + XMEMCPY(*der, pub_der, pub_derSz); + } + } + else { + XMEMCPY(*der, pub_der, pub_derSz); + *der += pub_derSz; + } + } + + XFREE(pub_der, NULL, DYNAMIC_TYPE_PUBLIC_KEY); + XFREE(local_der, NULL, DYNAMIC_TYPE_PUBLIC_KEY); + + wc_ecc_free(&eccKey); +#else + ret = WOLFSSL_FATAL_ERROR; +#endif /* HAVE_ECC */ + + if (ret == 0) { + return pub_derSz; + } + + return ret; +#else + return WOLFSSL_FATAL_ERROR; +#endif /* !NO_RSA || HAVE_ECC */ } #endif /* !NO_ASN && !NO_PWDBASED */ diff --git a/tests/api.c b/tests/api.c index eaa4e0f63..d20e53c3e 100644 --- a/tests/api.c +++ b/tests/api.c @@ -47216,7 +47216,7 @@ static int test_wolfSSL_d2i_and_i2d_PublicKey(void) #if defined(OPENSSL_EXTRA) && !defined(NO_RSA) EVP_PKEY* pkey; const unsigned char* p; - unsigned char* der = NULL; + unsigned char *der = NULL, *tmp = NULL; int derLen; p = client_keypub_der_2048; @@ -47229,6 +47229,14 @@ static int test_wolfSSL_d2i_and_i2d_PublicKey(void) AssertIntEQ(derLen, sizeof_client_keypub_der_2048); AssertIntEQ(XMEMCMP(der, client_keypub_der_2048, derLen), 0); + /* Do same test except with pre-allocated buffer to ensure the der pointer + * is advanced. */ + tmp = der; + AssertIntGE((derLen = wolfSSL_i2d_PublicKey(pkey, &tmp)), 0); + AssertIntEQ(derLen, sizeof_client_keypub_der_2048); + AssertIntEQ(XMEMCMP(der, client_keypub_der_2048, derLen), 0); + AssertTrue(der + derLen == tmp); + XFREE(der, HEAP_HINT, DYNAMIC_TYPE_OPENSSL); EVP_PKEY_free(pkey); diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 435082a42..91739be75 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -6094,7 +6094,7 @@ WOLFSSL_LOCAL int EncryptDerKey(byte *der, int *derSz, const EVP_CIPHER* cipher, #endif #endif -#if defined(WOLFSSL_KEY_GEN) && !defined(NO_RSA) && !defined(HAVE_USER_RSA) +#if !defined(NO_RSA) && !defined(HAVE_USER_RSA) WOLFSSL_LOCAL int wolfSSL_RSA_To_Der(WOLFSSL_RSA* rsa, byte** outBuf, int publicKey, void* heap); #endif