Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions src/pk.c
Original file line number Diff line number Diff line change
Expand Up @@ -5518,8 +5518,8 @@ int wolfSSL_ED25519_verify(const unsigned char *msg, unsigned int msgSz,

#endif /* OPENSSL_EXTRA && HAVE_ED25519 */

#if (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL)) && \
defined(HAVE_ED25519)
#if (defined(OPENSSL_EXTRA) || defined(WOLFSSL_WPAS_SMALL) || \
defined(OPENSSL_EXTRA_X509_SMALL)) && defined(HAVE_ED25519)
/* Allocate and initialize a new ed25519_key.
*
* @param [in] heap Heap hint for memory allocation.
Expand Down Expand Up @@ -5570,7 +5570,8 @@ void wolfSSL_ED25519_free(ed25519_key* key)
#endif
}
}
#endif /* (OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL) && HAVE_ED25519 */
#endif /* (OPENSSL_EXTRA || WOLFSSL_WPAS_SMALL || OPENSSL_EXTRA_X509_SMALL) &&
* HAVE_ED25519 */

/*******************************************************************************
* END OF ED25519 API
Expand Down Expand Up @@ -7373,6 +7374,27 @@ int pkcs8_encode(WOLFSSL_EVP_PKEY* pkey, byte* key, word32* keySz)
curveOid = NULL;
oidSz = 0;
}
#endif
#if defined(HAVE_ED25519)

Copy link
Copy Markdown
Member

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 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 pkey->pkey.ptr / pkey->pkey_sz as a PKCS#8 PrivateKeyInfo and never NULL-checks pkey->pkey.ptr. For an Ed25519 EVP_PKEY produced by wolfSSL_X509_get_pubkey(), pkey.ptr holds the raw 32-byte PUBLIC key (pkey_sz==32), not a PKCS#8 private key. Calling a PKCS#8 export on such a public-only key would silently copy 32 public bytes and report success rather than failing. The DH special case above shares the same loose assumption, but DH at least dereferences pkey->dh first; here there is no guard at all. This is a misuse path (encoding a public key as a private key), so impact is low, but a NULL/empty check on pkey->pkey.ptr would make the failure explicit.

Fix: Add a pkey->pkey.ptr != NULL / pkey->pkey_sz > 0 validation before the XMEMCPY so a public-only or uninitialized Ed25519 key returns a clean error instead of bogus output.

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 - (or --) instead.

Fix: Replace the em-dash with an ASCII hyphen to comply with the repository's ASCII-only source policy.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 -- (or -) so the file remains pure ASCII.

* DH special case above). */
if (keySz == NULL)
return BAD_FUNC_ARG;
/* The cached DER must be present; e.g. an Ed25519 EVP_PKEY built
* from wolfSSL_X509_get_pubkey() holds only a raw public key, not a
* PKCS#8 private key, so reject rather than emit bogus output. */
if (pkey->pkey.ptr == NULL || pkey->pkey_sz <= 0)
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;
}
#endif
else {
ret = NOT_COMPILED_IN;
Expand Down
3 changes: 3 additions & 0 deletions src/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@
|| defined(OPENSSL_EXTRA_X509_SMALL) \
|| defined(HAVE_WEBSERVER) || defined(WOLFSSL_KEY_GEN))
#include <wolfssl/openssl/evp.h>
/* ed25519 compat helpers (wolfSSL_ED25519_new/free) are used by the X509
* Ed25519 paths, which also build under OPENSSL_EXTRA_X509_SMALL. */
#include <wolfssl/openssl/ed25519.h>
/* openssl headers end, wolfssl internal headers next */
#endif

Expand Down
142 changes: 139 additions & 3 deletions src/x509.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) && defined(HAVE_ED25519_KEY_IMPORT)
else if (x509->pubKeyOID == ED25519k) {
key->type = WC_EVP_PKEY_ED25519;
}
#endif
else {
key->type = WC_EVP_PKEY_EC;
Expand Down Expand Up @@ -6572,6 +6577,29 @@ WOLFSSL_EVP_PKEY* wolfSSL_X509_get_pubkey(WOLFSSL_X509* x509)
}
}
#endif /* NO_DSA */

/* decode Ed25519 key */
#if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_IMPORT)
if (key->type == WC_EVP_PKEY_ED25519) {
key->ed25519 = wolfSSL_ED25519_new(x509->heap, INVALID_DEVID);
if (key->ed25519 == NULL) {
wolfSSL_EVP_PKEY_free(key);
return NULL;
}
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;
Expand Down Expand Up @@ -9010,6 +9038,12 @@ static int verifyX509orX509REQ(WOLFSSL_X509* x509, WOLFSSL_EVP_PKEY* pkey,
const byte* der;
int derSz = 0;
int type;
const byte* pubKey;
int pubKeySz;
#if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_EXPORT)
byte edPubKey[ED25519_PUB_KEY_SIZE];
word32 edPubKeySz = (word32)sizeof(edPubKey);
#endif

(void)req;

Expand All @@ -9023,6 +9057,10 @@ static int verifyX509orX509REQ(WOLFSSL_X509* x509, WOLFSSL_EVP_PKEY* pkey,
return WOLFSSL_FATAL_ERROR;
}

/* Most key types verify against the cached public-key DER. */
pubKey = (const byte*)pkey->pkey.ptr;
pubKeySz = pkey->pkey_sz;

switch (pkey->type) {
case WC_EVP_PKEY_RSA:
type = RSAk;
Expand All @@ -9036,6 +9074,23 @@ static int verifyX509orX509REQ(WOLFSSL_X509* x509, WOLFSSL_EVP_PKEY* pkey,
type = DSAk;
break;

#if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_EXPORT)
case WC_EVP_PKEY_ED25519:
/* The signature check needs the raw public key; for Ed25519
* pkey->pkey.ptr holds a PKCS#8 private blob (or nothing), so
* export the public key from the ed25519_key instead. */
if (pkey->ed25519 == NULL ||
wc_ed25519_export_public(pkey->ed25519, edPubKey,
&edPubKeySz) != 0) {
WOLFSSL_MSG("Unable to export Ed25519 public key");
return WOLFSSL_FATAL_ERROR;
}
pubKey = edPubKey;
pubKeySz = (int)edPubKeySz;
type = ED25519k;
break;
#endif

default:
WOLFSSL_MSG("Unknown pkey key type");
return WOLFSSL_FATAL_ERROR;
Expand All @@ -9044,11 +9099,11 @@ static int verifyX509orX509REQ(WOLFSSL_X509* x509, WOLFSSL_EVP_PKEY* pkey,
#ifdef WOLFSSL_CERT_REQ
if (req)
ret = CheckCSRSignaturePubKey(der, (word32)derSz, x509->heap,
(unsigned char*)pkey->pkey.ptr, pkey->pkey_sz, type);
(unsigned char*)pubKey, pubKeySz, type);
else
#endif
ret = CheckCertSignaturePubKey(der, (word32)derSz, x509->heap,
(unsigned char*)pkey->pkey.ptr, pkey->pkey_sz, type);
(unsigned char*)pubKey, pubKeySz, type);
if (ret == 0) {
return WOLFSSL_SUCCESS;
}
Expand Down Expand Up @@ -12225,6 +12280,14 @@ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509)
int hashType;
int sigType = WOLFSSL_FAILURE;

#if defined(HAVE_ED25519)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 [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.

/* 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 CTC_ED25519;
}
#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))
Expand Down Expand Up @@ -12337,6 +12400,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
Expand Down Expand Up @@ -12472,6 +12538,28 @@ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509)
key = (void*)dsa;
}
#endif
#if defined(HAVE_ED25519) && defined(HAVE_ED25519_KEY_IMPORT)
if (x509->pubKeyOID == ED25519k) {
ed25519 = wolfSSL_ED25519_new(NULL, INVALID_DEVID);
if (ed25519 == NULL) {
WOLFSSL_MSG("Failed to allocate memory for ed25519_key");
XFREE(cert, NULL, DYNAMIC_TYPE_CERT);
return WOLFSSL_FAILURE;
}

type = ED25519_TYPE;
/* 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);
wolfSSL_ED25519_free(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)) {
Expand Down Expand Up @@ -12723,6 +12811,11 @@ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509)
XFREE(ecc, NULL, DYNAMIC_TYPE_ECC);
}
#endif
#if defined(HAVE_ED25519)
if (x509->pubKeyOID == ED25519k) {
wolfSSL_ED25519_free(ed25519);
}
#endif
#ifndef NO_DSA
if (x509->pubKeyOID == DSAk) {
wc_FreeDsaKey(dsa);
Expand Down Expand Up @@ -12807,6 +12900,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);
Expand Down Expand Up @@ -12895,11 +12994,24 @@ int wolfSSL_X509_sign(WOLFSSL_X509* x509, WOLFSSL_EVP_PKEY* pkey,

WOLFSSL_ENTER("wolfSSL_X509_sign");

if (x509 == NULL || pkey == NULL || md == NULL) {
if (x509 == NULL || pkey == NULL) {
ret = WOLFSSL_FAILURE;
goto out;
}

/* Most key types require an explicit digest. Ed25519 is the exception:
* it signs with a NULL digest (the key has a built-in hash). Kept as a
* dedicated block since other algorithms may grow similar edge cases. */
if (md == NULL) {
#if defined(HAVE_ED25519)
if (pkey->type != WC_EVP_PKEY_ED25519)
#endif
{
ret = WOLFSSL_FAILURE;
goto out;
}
}

x509->sigOID = wolfSSL_sigTypeFromPKEY((WOLFSSL_EVP_MD*)md, pkey);
if ((ret = wolfssl_x509_make_der(x509, 0, der, &derSz, 0)) !=
WOLFSSL_SUCCESS) {
Expand Down Expand Up @@ -16408,6 +16520,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;
Expand Down
Loading