Ed25519 support in the OpenSSL compatibility layer#10722
Conversation
|
Can one of the admins verify this patch? |
|
Hi @ordex , this is a very nice contribution. I'd like to hear more about your project and use case. In order to accept this we need some more information and a signed contributor agreement. Please email support@wolfssl.com and reference this PR. Thank you so much, David Garske, wolfSSL |
|
|
Hi @dgarske I reached out to support and I am sorting out the agreement. Regarding my project: I am working on a slim openvpn-compatible client and it is expected to perform the TLS handshake using Ed25519 keys. While this works flawlessly with OpenSSL, I hit the mentioned limitation with wolfSSL. The reason for using also wolfSSL, next to OpenSSL, is because I am planning to deploy the client on OpenWrt, which may ship with wolfSSL only. |
|
Okay to test. Thank you @ordex for those details and working with us on the contributor agreement. I see your ZD ticket 22003. |
wolfCrypt has full Ed25519 (keygen, sign/verify, ASN.1 import/export) and the TLS 1.3 stack already authenticates with Ed25519 certificates, but the OpenSSL-compatibility surface was missing the dispatch for several common operations, so an application driving Ed25519 purely through the OpenSSL API (EVP_PKEY_keygen, i2d_PUBKEY, X509_sign of an in-memory self-signed cert, then loading it into an SSL_CTX) could not use it. Fill those gaps, each mirroring the adjacent RSA/ECC/X25519 case: - EVP_PKEY_keygen (wolfcrypt/src/evp.c): generate an Ed25519 key and cache the PKCS#8 PrivateKeyInfo DER, like the EC/RSA cases. - wolfSSL_i2d_PublicKey / i2d_PUBKEY (wolfcrypt/src/evp_pk.c): encode an Ed25519 SubjectPublicKeyInfo. - pkcs8_encode (src/pk.c): return the already-PKCS#8 cached DER for Ed25519 (as the DH case does), so i2d_PKCS8_PRIV_KEY_INFO works. - d2iTryEd25519Key (wolfcrypt/src/evp_pk.c): derive the public key after decoding a PKCS#8 private key (v1 carries only the seed) so the decoded EVP_PKEY is complete. - d2i_AutoPrivateKey (wolfcrypt/src/evp_pk.c): detect Ed25519 by its algorithm id and decode the full PKCS#8 (its inner key is an OCTET STRING, which the RSA/ECC sequence-counting heuristic cannot classify). - X509_sign / X509_resign_cert / sigTypeFromPKEY / wolfssl_x509_make_der (src/x509.c): sign a certificate with an Ed25519 key (NULL digest), resolving the signature OID as ED25519k and building the SubjectPublicKey from an ed25519_key. - X509_set_pubkey / X509_get_pubkey (src/x509.c): set and retrieve an Ed25519 public key, keeping the X.509 public-key buffer as the raw key bytes to match how DecodeCert/StoreKey store it. tests/api.c gains test_wolfSSL_EVP_PKEY_ED25519_openssl, which exercises the whole sequence (keygen, i2d_PUBKEY, i2d_PKCS8_PRIV_KEY_INFO, d2i_AutoPrivateKey, self-signed X509_sign with a NULL digest, X509_get_pubkey round-trip, and loading the key+cert into an SSL_CTX). Requires --enable-ed25519 --enable-certgen. Signed-off-by: Antonio Quartulli <antonio@mandelbit.com>
There was a problem hiding this comment.
Pull request overview
This PR extends wolfSSL’s OpenSSL-compatibility layer to fully support Ed25519 workflows (EVP keygen, PKCS#8/SPKI serialization, X509 self-signing, and SSL_CTX loading), aligning the OpenSSL API surface with existing wolfCrypt/TLS Ed25519 capabilities.
Changes:
- Add Ed25519 support to
EVP_PKEY_keygen()and public-key DER serialization (i2d_PUBKEY/i2d_PublicKey). - Improve Ed25519 PKCS#8 decode handling and ensure decoded private keys have a derived public component.
- Add an API-level unit test exercising an application-like Ed25519 EVP/X509/TLS flow.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
wolfcrypt/src/evp.c |
Adds Ed25519 handling to EVP_PKEY key generation and caches PKCS#8 DER. |
wolfcrypt/src/evp_pk.c |
Improves Ed25519 PKCS#8 decode behavior and adds SPKI encoding for Ed25519 public keys. |
src/x509.c |
Adds Ed25519 pubkey extraction/import, Ed25519 signing handling with NULL digest, and X509 pubkey setting. |
src/pk.c |
Extends PKCS#8 encoding logic to return cached Ed25519 PKCS#8 bytes. |
tests/api.c |
Adds an end-to-end Ed25519 OpenSSL-compat EVP/X509/SSL_CTX unit test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_EXPORT) | ||
| ctx->pkey->type != WC_EVP_PKEY_ED25519 && | ||
| #endif |
| #if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_EXPORT) | ||
| case WC_EVP_PKEY_ED25519: |
| 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. */ |
| #if defined(HAVE_ED25519) | ||
| if (key->type == WC_EVP_PKEY_ED25519) { | ||
| key->ed25519 = (ed25519_key*)XMALLOC(sizeof(ed25519_key), | ||
| x509->heap, DYNAMIC_TYPE_ED25519); | ||
| if (key->ed25519 == NULL) { |
| #if defined(HAVE_ED25519) | ||
| if (x509->pubKeyOID == ED25519k) { | ||
| ed25519 = (ed25519_key*)XMALLOC(sizeof(ed25519_key), NULL, | ||
| DYNAMIC_TYPE_ED25519); | ||
| if (ed25519 == NULL) { |
| #if defined(OPENSSL_EXTRA) && defined(HAVE_ED25519) && \ | ||
| defined(HAVE_ED25519_KEY_EXPORT) && defined(HAVE_ED25519_KEY_IMPORT) && \ | ||
| defined(WOLFSSL_CERT_GEN) && !defined(NO_CERTS) |
| tmp = p8der; | ||
| ExpectNotNull(decPriv = wolfSSL_d2i_AutoPrivateKey(NULL, &tmp, p8Sz)); | ||
|
|
||
| XFREE(p8der, HEAP_HINT, DYNAMIC_TYPE_OPENSSL); |
| XFREE(spki, HEAP_HINT, DYNAMIC_TYPE_OPENSSL); | ||
| XFREE(spki2, HEAP_HINT, DYNAMIC_TYPE_OPENSSL); |
| 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 | ||
| * 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; | ||
| } |
dgarske
left a comment
There was a problem hiding this comment.
Skoll Code Review
Scan type: reviewOverall recommendation: REQUEST_CHANGES
Findings: 13 total — 13 posted, 0 skipped
12 finding(s) posted as inline comments (see file-level comments below)
Posted findings
- [High] Ed25519 pubkey import in CertFromX509/wolfssl_x509_make_der not gated on HAVE_ED25519_KEY_IMPORT —
src/x509.c:12522-12550 - [High] Ed25519 import paths missing HAVE_ED25519_KEY_IMPORT guard (build break) —
src/x509.c:6582, src/x509.c:12522 - [High] Ed25519 public-key decode in wolfSSL_X509_get_pubkey not gated on HAVE_ED25519_KEY_IMPORT —
src/x509.c:6582-6610 - [Medium] pkcs8_encode Ed25519 special case lacks NULL guard on pkey-pkey.ptr —
src/pk.c:7377-7392 - [Medium] Non-ASCII em-dash in comment violates wolfSSL ASCII-only convention —
src/pk.c:7380 - [Medium] Ed25519 keygen path uses KEY_EXPORT guard but calls MAKE_KEY-only wc_ed25519_make_key —
wolfcrypt/src/evp.c:3939-3958 - [Medium] wc_ed25519_make_public derive call requires HAVE_ED25519_MAKE_KEY guard —
wolfcrypt/src/evp_pk.c:286-295 - [Low] pkcs8_encode Ed25519 branch assumes cached DER is a private PKCS#8 without validating —
src/pk.c:7377-7392 - [Low] Non-ASCII em-dash in comment violates ASCII-only convention —
src/pk.c:7380 - [Low] Use CTC_ED25519 instead of ED25519k for signature-type return value —
src/x509.c:12264-12269 - [Low] Doxygen comment detached from wolfSSL_i2d_PublicKey by inserted helper —
wolfcrypt/src/evp_pk.c:2402-2457 - [Low] i2d_PublicKey for Ed25519 emits SubjectPublicKeyInfo rather than the raw key —
wolfcrypt/src/evp_pk.c:2459-2488 - [Low] Redundant pubKeySet assignment and ignored make_public failure —
wolfcrypt/src/evp_pk.c:290-295
Review generated by Skoll
| key = (void*)dsa; | ||
| } | ||
| #endif | ||
| #if defined(HAVE_ED25519) |
There was a problem hiding this comment.
🔴 [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 #if defined(HAVE_ED25519) but calls wc_ed25519_import_public() (KEY_IMPORT-only, ed25519.c:1235-1469). With HAVE_ED25519 but NO_ED25519_KEY_IMPORT, this block is compiled and references an unbuilt symbol, breaking the link. The matching cleanup block at src/x509.c:12803-12808 uses only wc_ed25519_free/XFREE (always available) so it does not have the same problem, but the import site does.
Fix: Add && defined(HAVE_ED25519_KEY_IMPORT) to the #if guard around the ED25519k public-key decode block.
| #endif /* NO_DSA */ | ||
|
|
||
| /* decode Ed25519 key */ | ||
| #if defined(HAVE_ED25519) |
There was a problem hiding this comment.
🔴 [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 #if defined(HAVE_ED25519), but it calls wc_ed25519_import_public(), which is compiled only under #ifdef HAVE_ED25519_KEY_IMPORT (wolfcrypt/src/ed25519.c:1235-1469). settings.h defines HAVE_ED25519_KEY_IMPORT by default with HAVE_ED25519, but a user can disable it independently via NO_ED25519_KEY_IMPORT while keeping HAVE_ED25519. In that supported configuration this code still compiles and produces an undefined reference / link error. Note the sibling wolfSSL_X509_set_pubkey (added in this same PR) correctly guards with HAVE_ED25519 && HAVE_ED25519_KEY_EXPORT, so the precise-guard precedent already exists in the diff.
Fix: Change the guard to #if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_IMPORT) so the block (and the key->type = WC_EVP_PKEY_ED25519 assignment that feeds it) is only built when the import primitive is available.
| oidSz = 0; | ||
| } | ||
| #endif | ||
| #if defined(HAVE_ED25519) |
There was a problem hiding this comment.
🟠 [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 checks keySz == NULL. Unlike the DH special case it mirrors, it does not validate that pkey->pkey.ptr (or pkey->ed25519) is non-NULL / pkey_sz > 0. The DH branch above is reached only after confirming pkey->dh->priv_key || pub_key are set, which implies the buffer is populated; the Ed25519 branch has no such precondition. An Ed25519 WOLFSSL_EVP_PKEY whose pkey.ptr was never cached (e.g. a key populated only via the in-memory ed25519 object) would dereference NULL here.
Fix: Add a pkey->pkey.ptr == NULL || pkey->pkey_sz <= 0 guard before the copy, returning BAD_FUNC_ARG, to match the defensive style of the surrounding branches.
| #if defined(HAVE_ED25519) | ||
| 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 |
There was a problem hiding this comment.
🟠 [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 - (or --) instead.
Fix: Replace the em-dash with an ASCII hyphen to comply with the repository's ASCII-only source policy.
| } | ||
| break; | ||
| #endif | ||
| #if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_EXPORT) |
There was a problem hiding this comment.
🟠 [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 HAVE_ED25519 && HAVE_ED25519_KEY_EXPORT, but the body calls wc_ed25519_make_key(), which exists only under HAVE_ED25519_MAKE_KEY (ed25519.c:266-409). A build with key export enabled but NO_ED25519_MAKE_KEY (hence NO_ED25519_SIGN) would compile the case yet fail to link. Less common than the KEY_IMPORT cases, but the guard does not match the primitives used.
Fix: Add && defined(HAVE_ED25519_MAKE_KEY) to the Ed25519 keygen guard (both the case label and the allowlist check).
| #if defined(HAVE_ED25519) | ||
| 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 |
There was a problem hiding this comment.
🔵 [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 -- (or -) so the file remains pure ASCII.
| int hashType; | ||
| int sigType = WOLFSSL_FAILURE; | ||
|
|
||
| #if defined(HAVE_ED25519) |
There was a problem hiding this comment.
🔵 [Low] Use CTC_ED25519 instead of ED25519k for signature-type return value
wolfSSL_sigTypeFromPKEY returns a signature-type constant (e.g. CTC_SHA256wECDSA), but the new Ed25519 path returns the key-OID constant ED25519k. These happen to be numerically identical (both are 256 / OID 1.3.101.112, per oid_sum.h), so behavior is correct, but returning the CTC_* signature-type constant from a function whose contract is "sig type" reads more clearly and matches the other branches.
Fix: Return CTC_ED25519 (identical value, clearer intent) for consistency with the RSA/ECC CTC_* returns in this function.
|
|
||
| return derSz; | ||
| } | ||
| #endif /* HAVE_ED25519 && HAVE_ED25519_KEY_EXPORT */ |
There was a problem hiding this comment.
🔵 [Low] Doxygen comment detached from wolfSSL_i2d_PublicKey by inserted helper
The new wolfssl_i_i2d_ed25519_pubkey() helper (and its own block comment) was inserted between the existing doc-block that documents wolfSSL_i2d_PublicKey ("Encode the WOLFSSL_EVP_PKEY object as public key DER...") and the function it describes. The doc-block now sits immediately above the static helper, so it reads as documentation for the helper rather than for wolfSSL_i2d_PublicKey. Purely a readability issue.
Fix: Relocate the inserted helper above the existing function doc-block to keep the documentation attached to wolfSSL_i2d_PublicKey.
| } | ||
| #endif /* HAVE_ED25519 && HAVE_ED25519_KEY_EXPORT */ | ||
|
|
||
| int wolfSSL_i2d_PublicKey(const WOLFSSL_EVP_PKEY *key, unsigned char **der) |
There was a problem hiding this comment.
🔵 [Low] i2d_PublicKey for Ed25519 emits SubjectPublicKeyInfo rather than the raw key
The new Ed25519 case routes wolfSSL_i2d_PublicKey to wolfssl_i_i2d_ed25519_pubkey() with withAlg=1, i.e. it emits a full SubjectPublicKeyInfo. In OpenSSL i2d_PublicKey emits the bare/traditional key while i2d_PUBKEY emits the SPKI. wolfSSL aliases i2d_PUBKEY directly to i2d_PublicKey (evp_pk.c:2499-2502), so the SPKI form is what the PEM_write_bio_PUBKEY path needs and the new test's i2d_PUBKEY round-trip passes. The note is only that a caller invoking wolfSSL_i2d_PublicKey for Ed25519 expecting the 32 raw bytes (OpenSSL semantics) will instead get the SPKI. This matches the alias's behavior, so it is likely intentional; flagging for confirmation.
Fix: Confirm the SPKI output is the intended behavior for both the i2d_PUBKEY and i2d_PublicKey entry points; if a raw form is ever needed for i2d_PublicKey, gate it on the caller. No change required if the alias semantics are accepted.
| * decoded key has no public part. Derive it (it is deterministic from | ||
| * the seed) so the resulting EVP_PKEY is complete and callers can later | ||
| * export/embed the public key. */ | ||
| if (priv && !edKey->pubKeySet) { |
There was a problem hiding this comment.
🔵 [Low] Redundant pubKeySet assignment and ignored make_public failure
wc_ed25519_make_public() already sets key->pubKeySet = 1 internally on success (ed25519.c:353), so the explicit edKey->pubKeySet = 1; is redundant. Additionally, a non-zero return from wc_ed25519_make_public() is silently swallowed: if derivation fails, the code proceeds to build the EVP_PKEY with no public part and no diagnostic. This is acceptable degradation but could at least emit a WOLFSSL_MSG.
Fix: Drop the redundant flag assignment (rely on make_public) and optionally log when derivation fails.
julek-wolfssl
left a comment
There was a problem hiding this comment.
This is great work. I would like to see the following tests included too:
- Raw-key paths, e.g.
EVP_PKEY_new_raw_private_key()/EVP_PKEY_new_raw_public_key(), since those may populatepkey->pkey.ptrdifferently than keygen/decode. - Validation that a PKCS#8-decoded Ed25519 private key can re-export the same public key / SPKI, not just that
d2i_AutoPrivateKey()returns non-NULL. - Negative coverage for public-only Ed25519 keys: PKCS#8 private export should fail cleanly, not emit raw public bytes or cached non-PKCS#8 data.
- Certificate verification after
X509_sign(..., NULL), to prove the generated Ed25519 signature/OID/public-key encoding is actually usable.
| key->ed25519 = (ed25519_key*)XMALLOC(sizeof(ed25519_key), | ||
| x509->heap, DYNAMIC_TYPE_ED25519); | ||
| if (key->ed25519 == NULL) { | ||
| 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; | ||
| } |
There was a problem hiding this comment.
Prefer to use wolfSSL_ED25519_new.
| ed25519 = (ed25519_key*)XMALLOC(sizeof(ed25519_key), NULL, | ||
| DYNAMIC_TYPE_ED25519); | ||
| if (ed25519 == NULL) { | ||
| 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; | ||
| } |
There was a problem hiding this comment.
Also use wolfSSL_ED25519_new
| 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 | ||
| )) { |
There was a problem hiding this comment.
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.
if (md == null) {
if (pkey->type != WC_EVP_PKEY_ED25519)
error;
}
| 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; | ||
| } |
| if (pkey->pkey.ptr != NULL) { | ||
| XFREE(pkey->pkey.ptr, pkey->heap, | ||
| DYNAMIC_TYPE_OPENSSL); | ||
| } |
There was a problem hiding this comment.
| if (pkey->pkey.ptr != NULL) { | |
| XFREE(pkey->pkey.ptr, pkey->heap, | |
| DYNAMIC_TYPE_OPENSSL); | |
| } | |
| XFREE(pkey->pkey.ptr, pkey->heap, | |
| DYNAMIC_TYPE_OPENSSL); |
|
Hi @ordex , your contributor agreement has been approved and is now on file. We posted some review feedback, so take a look at those. Looking forward to your next push. |
Description
wolfCrypt has full Ed25519 (keygen, sign/verify, ASN.1 import/export) and the TLS 1.3 stack already authenticates with Ed25519 certificates, but the OpenSSL-compatibility surface was missing the dispatch for several common operations, so an application driving Ed25519 purely through the OpenSSL API (EVP_PKEY_keygen, i2d_PUBKEY, X509_sign of an in-memory self-signed cert, then loading it into an SSL_CTX) could not use it.
This PR adds the required glue code to have full OpenSSL API support for Ed25591 (by mirroring the existing RSA/ECC/X25519 code).
Note: requires --enable-ed25519 --enable-certgen
Testing
The commit comes with the relevant unit test code.
Moreover, this code is used by an experimental TLS-based VPN client that uses Ed25591 keys/certs.