-
Notifications
You must be signed in to change notification settings - Fork 995
Ed25519 support in the OpenSSL compatibility layer #10722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7373,6 +7373,22 @@ int pkcs8_encode(WOLFSSL_EVP_PKEY* pkey, byte* key, word32* keySz) | |
| curveOid = NULL; | ||
| oidSz = 0; | ||
| } | ||
| #endif | ||
| #if defined(HAVE_ED25519) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 [Low] pkcs8_encode Ed25519 branch assumes cached DER is a private PKCS#8 without validating The new Ed25519 branch unconditionally returns Fix: Add a |
||
| else if (pkey->type == WC_EVP_PKEY_ED25519) { | ||
| /* The cached DER is already a PKCS#8 PrivateKeyInfo (set when the | ||
| * key was generated or decoded), so return it as-is (same as the | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 [Medium] Non-ASCII em-dash in comment violates wolfSSL ASCII-only convention The new comment contains a UTF-8 em-dash (U+2014) instead of an ASCII hyphen: "so return it as-is — same as the". wolfSSL source is expected to be 7-bit ASCII only; this is the sole non-ASCII byte introduced across all five changed files and will trip ASCII/whitespace lint checks. Use Fix: Replace the em-dash with an ASCII hyphen to comply with the repository's ASCII-only source policy.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 [Low] Non-ASCII em-dash in comment violates ASCII-only convention The comment introduced by this PR contains a UTF-8 em-dash (U+2014) rather than ASCII. wolfSSL source must be 7-bit ASCII (no em/en dashes, smart quotes, etc.). This is the only non-ASCII byte introduced across the changed files (verified via a non-ASCII scan of all five files; the rest are clean). Fix: Replace the em-dash with |
||
| * DH special case above). */ | ||
| if (keySz == NULL) | ||
| return BAD_FUNC_ARG; | ||
|
|
||
| *keySz = (word32)pkey->pkey_sz; | ||
| if (key == NULL) | ||
| return LENGTH_ONLY_E; | ||
|
|
||
| XMEMCPY(key, pkey->pkey.ptr, (size_t)pkey->pkey_sz); | ||
| return pkey->pkey_sz; | ||
| } | ||
|
Comment on lines
+7378
to
+7391
|
||
| #endif | ||
| else { | ||
| ret = NOT_COMPILED_IN; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6481,6 +6481,11 @@ WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509) | |
| x509->pubKeyOID == ML_DSA_87k) { | ||
| key->type = WC_EVP_PKEY_DILITHIUM; | ||
| } | ||
| #endif | ||
| #if defined(HAVE_ED25519) | ||
| else if (x509->pubKeyOID == ED25519k) { | ||
| key->type = WC_EVP_PKEY_ED25519; | ||
| } | ||
| #endif | ||
| else { | ||
| key->type = WC_EVP_PKEY_EC; | ||
|
|
@@ -6572,6 +6577,37 @@ WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509) | |
| } | ||
| } | ||
| #endif /* NO_DSA */ | ||
|
|
||
| /* decode Ed25519 key */ | ||
| #if defined(HAVE_ED25519) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 [High] Ed25519 public-key decode in wolfSSL_X509_get_pubkey not gated on HAVE_ED25519_KEY_IMPORT The new Ed25519 decode block is guarded only by Fix: Change the guard to |
||
| if (key->type == WC_EVP_PKEY_ED25519) { | ||
| key->ed25519 = (ed25519_key*)XMALLOC(sizeof(ed25519_key), | ||
| x509->heap, DYNAMIC_TYPE_ED25519); | ||
| if (key->ed25519 == NULL) { | ||
|
Comment on lines
+6582
to
+6586
|
||
| wolfSSL_EVP_PKEY_free(key); | ||
| return NULL; | ||
| } | ||
| if (wc_ed25519_init_ex(key->ed25519, x509->heap, | ||
| INVALID_DEVID) != 0) { | ||
| XFREE(key->ed25519, x509->heap, DYNAMIC_TYPE_ED25519); | ||
| key->ed25519 = NULL; | ||
| wolfSSL_EVP_PKEY_free(key); | ||
| return NULL; | ||
| } | ||
|
Comment on lines
+6584
to
+6596
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prefer to use |
||
| key->ownEd25519 = 1; | ||
|
|
||
| /* The X.509 public key buffer holds the raw Ed25519 key | ||
| * (CopyDecodedToX509 / StoreKey store the BIT STRING | ||
| * contents), so import it directly. */ | ||
| if (wc_ed25519_import_public( | ||
| (const unsigned char*)key->pkey.ptr, | ||
| (word32)key->pkey_sz, key->ed25519) != 0) { | ||
| WOLFSSL_MSG("wc_ed25519_import_public failed"); | ||
| wolfSSL_EVP_PKEY_free(key); | ||
| return NULL; | ||
| } | ||
| } | ||
| #endif /* HAVE_ED25519 */ | ||
| } | ||
| } | ||
| return key; | ||
|
|
@@ -12225,6 +12261,14 @@ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509) | |
| int hashType; | ||
| int sigType = WOLFSSL_FAILURE; | ||
|
|
||
| #if defined(HAVE_ED25519) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 [Low] Use CTC_ED25519 instead of ED25519k for signature-type return value
Fix: Return |
||
| /* Ed25519 carries its own hash, so md is unused (and may be NULL). | ||
| * Resolve it before touching md. */ | ||
| if (pkey->type == WC_EVP_PKEY_ED25519) { | ||
| return ED25519k; | ||
| } | ||
| #endif | ||
|
|
||
| /* Convert key type and hash algorithm to a signature algorithm */ | ||
| if (wolfSSL_EVP_get_hashinfo(md, &hashType, NULL) | ||
| == WC_NO_ERR_TRACE(WOLFSSL_FAILURE)) | ||
|
|
@@ -12337,6 +12381,9 @@ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509) | |
| #ifndef NO_DSA | ||
| DsaKey* dsa = NULL; | ||
| #endif | ||
| #if defined(HAVE_ED25519) | ||
| ed25519_key* ed25519 = NULL; | ||
| #endif | ||
| #if defined(HAVE_FALCON) | ||
| falcon_key* falcon = NULL; | ||
| #endif | ||
|
|
@@ -12472,6 +12519,36 @@ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509) | |
| key = (void*)dsa; | ||
| } | ||
| #endif | ||
| #if defined(HAVE_ED25519) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 [High] Ed25519 pubkey import in CertFromX509/wolfssl_x509_make_der not gated on HAVE_ED25519_KEY_IMPORT The new Ed25519 public-key object-creation block is guarded only by Fix: Add |
||
| if (x509->pubKeyOID == ED25519k) { | ||
| ed25519 = (ed25519_key*)XMALLOC(sizeof(ed25519_key), NULL, | ||
| DYNAMIC_TYPE_ED25519); | ||
| if (ed25519 == NULL) { | ||
|
Comment on lines
+12522
to
+12526
|
||
| WOLFSSL_MSG("Failed to allocate memory for ed25519_key"); | ||
| XFREE(cert, NULL, DYNAMIC_TYPE_CERT); | ||
| return WOLFSSL_FAILURE; | ||
| } | ||
|
|
||
| type = ED25519_TYPE; | ||
| ret = wc_ed25519_init(ed25519); | ||
| if (ret != 0) { | ||
| XFREE(ed25519, NULL, DYNAMIC_TYPE_ED25519); | ||
| XFREE(cert, NULL, DYNAMIC_TYPE_CERT); | ||
| return ret; | ||
| } | ||
|
Comment on lines
+12524
to
+12538
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also use |
||
| /* The X.509 public key buffer holds the raw Ed25519 key. */ | ||
| ret = wc_ed25519_import_public(x509->pubKey.buffer, | ||
| x509->pubKey.length, ed25519); | ||
| if (ret != 0) { | ||
| WOLFSSL_ERROR_VERBOSE(ret); | ||
| wc_ed25519_free(ed25519); | ||
| XFREE(ed25519, NULL, DYNAMIC_TYPE_ED25519); | ||
| XFREE(cert, NULL, DYNAMIC_TYPE_CERT); | ||
| return ret; | ||
| } | ||
| key = (void*)ed25519; | ||
| } | ||
| #endif | ||
| #if defined(HAVE_FALCON) | ||
| if ((x509->pubKeyOID == FALCON_LEVEL1k) || | ||
| (x509->pubKeyOID == FALCON_LEVEL5k)) { | ||
|
|
@@ -12723,6 +12800,12 @@ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509) | |
| XFREE(ecc, NULL, DYNAMIC_TYPE_ECC); | ||
| } | ||
| #endif | ||
| #if defined(HAVE_ED25519) | ||
| if (x509->pubKeyOID == ED25519k) { | ||
| wc_ed25519_free(ed25519); | ||
| XFREE(ed25519, NULL, DYNAMIC_TYPE_ED25519); | ||
| } | ||
| #endif | ||
| #ifndef NO_DSA | ||
| if (x509->pubKeyOID == DSAk) { | ||
| wc_FreeDsaKey(dsa); | ||
|
|
@@ -12807,6 +12890,12 @@ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509) | |
| key = pkey->ecc->internal; | ||
| } | ||
| #endif | ||
| #if defined(HAVE_ED25519) | ||
| if (pkey->type == WC_EVP_PKEY_ED25519) { | ||
| type = ED25519_TYPE; | ||
| key = pkey->ed25519; | ||
| } | ||
| #endif | ||
|
|
||
| /* Sign the certificate (request) body. */ | ||
| ret = wc_InitRng(&rng); | ||
|
|
@@ -12895,7 +12984,14 @@ int wolfSSL_X509_sign(WOLFSSL_X509* x509, WOLFSSL_EVP_PKEY* pkey, | |
|
|
||
| WOLFSSL_ENTER("wolfSSL_X509_sign"); | ||
|
|
||
| if (x509 == NULL || pkey == NULL || md == NULL) { | ||
| /* Ed25519 signs with a NULL digest (the key has a built-in hash); every | ||
| * other key type requires an explicit md. */ | ||
| if (x509 == NULL || pkey == NULL || | ||
| (md == NULL | ||
| #if defined(HAVE_ED25519) | ||
| && pkey->type != WC_EVP_PKEY_ED25519 | ||
| #endif | ||
| )) { | ||
|
Comment on lines
-12898
to
+12994
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's separate this check out into a new block after this one. Something like this. I suspect other algos in the future will have similar edge cases. |
||
| ret = WOLFSSL_FAILURE; | ||
| goto out; | ||
| } | ||
|
|
@@ -16408,6 +16504,30 @@ int wolfSSL_X509_set_pubkey(WOLFSSL_X509 *cert, WOLFSSL_EVP_PKEY *pkey) | |
| cert->pubKeyOID = ECDSAk; | ||
| } | ||
| break; | ||
| #endif | ||
| #if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_EXPORT) | ||
| case WC_EVP_PKEY_ED25519: | ||
| { | ||
| word32 rawLen = ED25519_PUB_KEY_SIZE; | ||
|
|
||
| if (pkey->ed25519 == NULL) | ||
| return WOLFSSL_FAILURE; | ||
|
|
||
| /* Store the RAW public key: wolfSSL keeps an X.509 Ed25519 | ||
| * public key as the bare key bytes (see StoreKey / | ||
| * CopyDecodedToX509), not a SubjectPublicKeyInfo. */ | ||
| p = (byte*)XMALLOC(rawLen, cert->heap, DYNAMIC_TYPE_PUBLIC_KEY); | ||
| if (p == NULL) | ||
| return WOLFSSL_FAILURE; | ||
|
|
||
| if (wc_ed25519_export_public(pkey->ed25519, p, &rawLen) != 0) { | ||
| XFREE(p, cert->heap, DYNAMIC_TYPE_PUBLIC_KEY); | ||
| return WOLFSSL_FAILURE; | ||
| } | ||
| derSz = (int)rawLen; | ||
| cert->pubKeyOID = ED25519k; | ||
| } | ||
| break; | ||
| #endif | ||
| default: | ||
| return WOLFSSL_FAILURE; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3820,6 +3820,9 @@ int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, | |||||||||||||
| #endif | ||||||||||||||
| #ifdef HAVE_CURVE448 | ||||||||||||||
| ctx->pkey->type != WC_EVP_PKEY_X448 && | ||||||||||||||
| #endif | ||||||||||||||
| #if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_EXPORT) | ||||||||||||||
| ctx->pkey->type != WC_EVP_PKEY_ED25519 && | ||||||||||||||
| #endif | ||||||||||||||
|
Comment on lines
+3824
to
3826
|
||||||||||||||
| ctx->pkey->type != WC_EVP_PKEY_DH)) { | ||||||||||||||
| WOLFSSL_MSG("Key not set or key type not supported"); | ||||||||||||||
|
|
@@ -3932,6 +3935,52 @@ int wolfSSL_EVP_PKEY_keygen(WOLFSSL_EVP_PKEY_CTX *ctx, | |||||||||||||
| ret = WOLFSSL_SUCCESS; | ||||||||||||||
| } | ||||||||||||||
| break; | ||||||||||||||
| #endif | ||||||||||||||
| #if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_EXPORT) | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 [Medium] Ed25519 keygen path uses KEY_EXPORT guard but calls MAKE_KEY-only wc_ed25519_make_key The Ed25519 keygen case (and the corresponding type allowlist at evp.c:3824-3826) is guarded by Fix: Add |
||||||||||||||
| case WC_EVP_PKEY_ED25519: | ||||||||||||||
|
Comment on lines
+3939
to
+3940
|
||||||||||||||
| if (pkey->ed25519 == NULL) { | ||||||||||||||
| pkey->ed25519 = (ed25519_key*)XMALLOC(sizeof(ed25519_key), | ||||||||||||||
| pkey->heap, DYNAMIC_TYPE_ED25519); | ||||||||||||||
| if (pkey->ed25519 == NULL) { | ||||||||||||||
| ret = MEMORY_E; | ||||||||||||||
| break; | ||||||||||||||
| } | ||||||||||||||
| if (wc_ed25519_init_ex(pkey->ed25519, pkey->heap, | ||||||||||||||
| INVALID_DEVID) != 0) { | ||||||||||||||
| XFREE(pkey->ed25519, pkey->heap, DYNAMIC_TYPE_ED25519); | ||||||||||||||
| pkey->ed25519 = NULL; | ||||||||||||||
| break; | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+3942
to
+3953
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||||||||||
| pkey->ownEd25519 = 1; | ||||||||||||||
| } | ||||||||||||||
| /* Reuse the RNG already initialized on the EVP_PKEY. */ | ||||||||||||||
| if (wc_ed25519_make_key(&pkey->rng, ED25519_KEY_SIZE, | ||||||||||||||
| pkey->ed25519) == 0) { | ||||||||||||||
| /* Cache the PKCS#8 PrivateKeyInfo DER so the EVP/SSL paths | ||||||||||||||
| * (use_PrivateKey, EVP_PKEY2PKCS8) can load and serialize the | ||||||||||||||
| * key, mirroring the state the decode path produces. */ | ||||||||||||||
|
Comment on lines
+3957
to
+3961
|
||||||||||||||
| int edDerSz = wc_Ed25519PrivateKeyToDer(pkey->ed25519, NULL, 0); | ||||||||||||||
| if (edDerSz > 0) { | ||||||||||||||
| byte* edDer = (byte*)XMALLOC((size_t)edDerSz, pkey->heap, | ||||||||||||||
| DYNAMIC_TYPE_OPENSSL); | ||||||||||||||
| if (edDer != NULL) { | ||||||||||||||
| if (wc_Ed25519PrivateKeyToDer(pkey->ed25519, edDer, | ||||||||||||||
| (word32)edDerSz) == edDerSz) { | ||||||||||||||
| if (pkey->pkey.ptr != NULL) { | ||||||||||||||
| XFREE(pkey->pkey.ptr, pkey->heap, | ||||||||||||||
| DYNAMIC_TYPE_OPENSSL); | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+3969
to
+3972
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| pkey->pkey.ptr = (char*)edDer; | ||||||||||||||
| pkey->pkey_sz = edDerSz; | ||||||||||||||
| ret = WOLFSSL_SUCCESS; | ||||||||||||||
| } | ||||||||||||||
| else { | ||||||||||||||
| XFREE(edDer, pkey->heap, DYNAMIC_TYPE_OPENSSL); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| break; | ||||||||||||||
| #endif | ||||||||||||||
| default: | ||||||||||||||
| break; | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 [Medium] pkcs8_encode Ed25519 special case lacks NULL guard on pkey-pkey.ptr
The new Ed25519 branch copies the cached DER with
XMEMCPY(key, pkey->pkey.ptr, pkey->pkey_sz)but only checkskeySz == NULL. Unlike the DH special case it mirrors, it does not validate thatpkey->pkey.ptr(orpkey->ed25519) is non-NULL /pkey_sz > 0. The DH branch above is reached only after confirmingpkey->dh->priv_key || pub_keyare set, which implies the buffer is populated; the Ed25519 branch has no such precondition. An Ed25519WOLFSSL_EVP_PKEYwhosepkey.ptrwas never cached (e.g. a key populated only via the in-memoryed25519object) would dereference NULL here.Fix: Add a
pkey->pkey.ptr == NULL || pkey->pkey_sz <= 0guard before the copy, returning BAD_FUNC_ARG, to match the defensive style of the surrounding branches.