From ebafb382ca5e80d7d17900e51ada5c5055f69ef3 Mon Sep 17 00:00:00 2001 From: Emma Stensland Date: Wed, 22 Jul 2026 16:34:28 -0600 Subject: [PATCH] added check for private and public keys decoded --- .github/workflows/sshd-test.yml | 4 +- src/internal.c | 72 ++++++++++- tests/unit.c | 206 ++++++++++++++++++++++++++++++++ 3 files changed, 278 insertions(+), 4 deletions(-) diff --git a/.github/workflows/sshd-test.yml b/.github/workflows/sshd-test.yml index a59f6d280..3c540a3c8 100644 --- a/.github/workflows/sshd-test.yml +++ b/.github/workflows/sshd-test.yml @@ -42,7 +42,7 @@ jobs: id: cache-wolfssl with: path: build-dir/ - key: wolfssh-sshd-wolfssl-${{ matrix.wolfssl }}-${{ matrix.os }} + key: wolfssh-sshd-wolfssl-${{ matrix.wolfssl }}-${{ matrix.os }}-v2 lookup-only: true - name: Checkout, build, and install wolfssl @@ -73,7 +73,7 @@ jobs: uses: actions/cache@v5 with: path: build-dir/ - key: wolfssh-sshd-wolfssl-${{ matrix.wolfssl }}-${{ matrix.os }} + key: wolfssh-sshd-wolfssl-${{ matrix.wolfssl }}-${{ matrix.os }}-v2 fail-on-cache-miss: true - uses: actions/checkout@v6 diff --git a/src/internal.c b/src/internal.c index a5fa0ee3b..5c854e62d 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1695,7 +1695,11 @@ void wolfSSH_KEY_clean(WS_KeySignature* key) * @param isPrivate indicates private or public key * @param heap heap to use for memory allocation * @param pkey optionally return populated WS_KeySignature - * @return keyId as int, WS_MEMORY_E, WS_UNIMPLEMENTED_E + * @return keyId as int, WS_MEMORY_E, WS_UNIMPLEMENTED_E, + * WS_CRYPTO_FAILED (ML-DSA/Ed25519 private key decoded + * with no derivable public key). ECDSA allows the same + * omission unrejected; its signing path can defer to a + * cert-supplied public key, unlike ML-DSA/Ed25519. */ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap, WS_KeySignature **pkey) @@ -1704,6 +1708,7 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap, word32 idx; int ret; int dynType = isPrivate ? DYNTYPE_PRIVKEY : DYNTYPE_PUBKEY; + int noPubKeyRet = 0; #ifndef WOLFSSH_NO_MLDSA byte mlDsaLevel = 0; int mlDsaInit = 0; @@ -1779,6 +1784,24 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap, /* Not a supported curve, so free the key */ wc_ecc_free(&key->ks.ecc.key); } + + if (isPrivate && key->keyId != ID_UNKNOWN) { + /* SEC1 allows omitting the public point; not fatal + * here since a cert may supply it later, unlike + * ML-DSA/Ed25519 signing which always needs it. */ + byte pubScratch[257]; + word32 pubScratchSz = sizeof(pubScratch); + + PRIVATE_KEY_UNLOCK(); + if (wc_ecc_export_x963(&key->ks.ecc.key, + pubScratch, &pubScratchSz) != 0) { + WLOG(WS_LOG_WARN, + "ECDSA key decoded but has no derivable " + "public key; only usable if a certificate " + "supplies the public key"); + } + PRIVATE_KEY_LOCK(); + } } else { wc_ecc_free(&key->ks.ecc.key); } @@ -1796,6 +1819,32 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap, if (isPrivate) { ret = wc_MlDsaKey_PrivateKeyDecode(&key->ks.mldsa.key, in, inSz, &idx); + if (ret == 0) { + /* Priv-only decode can succeed with no derivable + * public key; reject here instead of at first + * handshake. Heap-allocated to avoid growing the + * stack by MLDSA_MAX_PUB_KEY_SIZE (~2.5KB). */ + byte* pubScratch = (byte*)WMALLOC( + MLDSA_MAX_PUB_KEY_SIZE, heap, DYNTYPE_TEMP); + word32 pubScratchSz = MLDSA_MAX_PUB_KEY_SIZE; + + if (pubScratch == NULL) { + ret = WS_MEMORY_E; + noPubKeyRet = ret; + } + else { + if (wc_MlDsaKey_ExportPubRaw(&key->ks.mldsa.key, + pubScratch, &pubScratchSz) != 0) { + WLOG(WS_LOG_ERROR, + "ML-DSA host key decoded but has no " + "derivable public key; regenerate it " + "with an embedded public key or seed"); + ret = WS_CRYPTO_FAILED; + noPubKeyRet = ret; + } + WFREE(pubScratch, heap, DYNTYPE_TEMP); + } + } } else { /* PublicKeyDecode auto-detects level from SPKI OID. */ @@ -1865,6 +1914,23 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap, if (isPrivate) { ret = wc_Ed25519PrivateKeyDecode(in, &idx, &key->ks.ed25519.key, inSz); + if (ret == 0) { + /* Same gap as ML-DSA/ECDSA: priv-only decode can + * succeed with no derivable public key; reject here + * instead of at first handshake. */ + byte pubScratch[ED25519_PUB_KEY_SIZE]; + word32 pubScratchSz = sizeof(pubScratch); + + if (wc_ed25519_export_public(&key->ks.ed25519.key, + pubScratch, &pubScratchSz) != 0) { + WLOG(WS_LOG_ERROR, + "Ed25519 host key decoded but has no " + "derivable public key; regenerate it with " + "an embedded public key"); + ret = WS_CRYPTO_FAILED; + noPubKeyRet = ret; + } + } } else { ret = wc_Ed25519PublicKeyDecode(in, &idx, @@ -1882,7 +1948,9 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap, #endif /* WOLFSSH_NO_ED25519 */ if (key->keyId == ID_UNKNOWN) { - ret = WS_UNIMPLEMENTED_E; + /* Prefer the specific rejection reason over the generic + * fallback. */ + ret = (noPubKeyRet != 0) ? noPubKeyRet : WS_UNIMPLEMENTED_E; } else { if (pkey != NULL) diff --git a/tests/unit.c b/tests/unit.c index 0f57c728f..b83d8c911 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -6027,6 +6027,77 @@ static int test_IdentifyAsn1Key(void) printf("IdentifyAsn1Key: ECC P-256 priv failed, ret=%d\n", ret); result = -601; goto done; } + + /* Private-only SEC1 DER (public point omitted): must still succeed + * (warn, not reject) since a cert may supply the public key later. */ + { + ecc_key eccKey; + WC_RNG eccRng; + byte* eccDer = NULL; + int eccDerSz; + + if (wc_ecc_init(&eccKey) != 0) { + result = -640; goto done; + } + if (wc_InitRng(&eccRng) != 0) { + wc_ecc_free(&eccKey); + result = -641; goto done; + } + if (wc_ecc_make_key(&eccRng, 32, &eccKey) != 0) { + wc_FreeRng(&eccRng); + wc_ecc_free(&eccKey); + result = -642; goto done; + } + wc_FreeRng(&eccRng); + + eccDerSz = wc_EccKeyDerSize(&eccKey, 0); + if (eccDerSz <= 0) { + wc_ecc_free(&eccKey); + result = -643; goto done; + } + eccDer = (byte*)WMALLOC((word32)eccDerSz, NULL, 0); + if (eccDer == NULL) { + wc_ecc_free(&eccKey); + result = -644; goto done; + } + eccDerSz = wc_EccPrivateKeyToDer(&eccKey, eccDer, (word32)eccDerSz); + wc_ecc_free(&eccKey); + if (eccDerSz <= 0) { + WFREE(eccDer, NULL, 0); + result = -645; goto done; + } + + ret = IdentifyAsn1Key(eccDer, (word32)eccDerSz, 1, NULL, NULL); + if (ret != ID_ECDSA_SHA2_NISTP256) { + WFREE(eccDer, NULL, 0); + printf("IdentifyAsn1Key: private-only ECC DER expected " + "ID_ECDSA_SHA2_NISTP256, got %d\n", ret); + result = -646; goto done; + } + + /* Same DER, but requesting the populated WS_KeySignature: confirms + * the key object survives (not freed) and is correctly tagged on + * this new no-derivable-pubkey path. */ + { + WS_KeySignature* eccKeySig = NULL; + + ret = IdentifyAsn1Key(eccDer, (word32)eccDerSz, 1, NULL, + &eccKeySig); + WFREE(eccDer, NULL, 0); + if (ret != ID_ECDSA_SHA2_NISTP256 || eccKeySig == NULL || + eccKeySig->keyId != ID_ECDSA_SHA2_NISTP256) { + printf("IdentifyAsn1Key: private-only ECC DER pkey-out " + "variant failed, ret=%d\n", ret); + if (eccKeySig != NULL) { + wolfSSH_KEY_clean(eccKeySig); + WFREE(eccKeySig, NULL, 0); + } + result = -647; goto done; + } + wolfSSH_KEY_clean(eccKeySig); + WFREE(eccKeySig, NULL, 0); + } + } #endif #ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP384 @@ -6057,6 +6128,70 @@ static int test_IdentifyAsn1Key(void) printf("IdentifyAsn1Key: Ed25519 priv failed, ret=%d\n", ret); result = -604; goto done; } + + /* Private-only DER (no embedded public key, unlike + * unitTestEd25519PrivKey above): must be rejected at decode time + * instead of failing later at KEX. */ + { + ed25519_key edKey; + WC_RNG edRng; + byte* edDer = NULL; + int edDerSz; + + if (wc_ed25519_init(&edKey) != 0) { + result = -648; goto done; + } + if (wc_InitRng(&edRng) != 0) { + wc_ed25519_free(&edKey); + result = -649; goto done; + } + if (wc_ed25519_make_key(&edRng, ED25519_KEY_SIZE, &edKey) != 0) { + wc_FreeRng(&edRng); + wc_ed25519_free(&edKey); + result = -650; goto done; + } + wc_FreeRng(&edRng); + + edDer = (byte*)WMALLOC(128, NULL, 0); + if (edDer == NULL) { + wc_ed25519_free(&edKey); + result = -651; goto done; + } + edDerSz = wc_Ed25519PrivateKeyToDer(&edKey, edDer, 128); + wc_ed25519_free(&edKey); + if (edDerSz <= 0) { + WFREE(edDer, NULL, 0); + result = -652; goto done; + } + + ret = IdentifyAsn1Key(edDer, (word32)edDerSz, 1, NULL, NULL); + if (ret != WS_CRYPTO_FAILED) { + WFREE(edDer, NULL, 0); + printf("IdentifyAsn1Key: private-only Ed25519 DER expected " + "WS_CRYPTO_FAILED, got %d\n", ret); + result = -653; goto done; + } + + /* Same DER, pkey-out variant: confirms *pkey stays NULL on this + * rejection path instead of leaking a populated key alongside the + * error. */ + { + WS_KeySignature* edKeySig = NULL; + + ret = IdentifyAsn1Key(edDer, (word32)edDerSz, 1, NULL, + &edKeySig); + WFREE(edDer, NULL, 0); + if (ret != WS_CRYPTO_FAILED || edKeySig != NULL) { + printf("IdentifyAsn1Key: private-only Ed25519 DER pkey-out " + "variant failed, ret=%d\n", ret); + if (edKeySig != NULL) { + wolfSSH_KEY_clean(edKeySig); + WFREE(edKeySig, NULL, 0); + } + result = -654; goto done; + } + } + } #endif #if !defined(WOLFSSH_NO_MLDSA) && !defined(WOLFSSH_NO_MLDSA44) @@ -6116,6 +6251,77 @@ static int test_IdentifyAsn1Key(void) result = -611; goto done; } } + + /* Private-only DER (no embedded public key, unlike unitTestMlDsaPrivKey + * above): however, for ML-DSA, wolfSSL automatically derives the public key + * from the private key during decode or it is included in the DER, so it + * should successfully identify as ID_MLDSA44. */ + { + MlDsaKey mlKey; + WC_RNG mlRng; + byte* mlDer = NULL; + int mlDerSz; + + if (wc_MlDsaKey_Init(&mlKey, NULL, INVALID_DEVID) != 0) { + result = -655; goto done; + } + if (wc_MlDsaKey_SetParams(&mlKey, WC_ML_DSA_44) != 0) { + wc_MlDsaKey_Free(&mlKey); + result = -656; goto done; + } + if (wc_InitRng(&mlRng) != 0) { + wc_MlDsaKey_Free(&mlKey); + result = -657; goto done; + } + if (wc_MlDsaKey_MakeKey(&mlKey, &mlRng) != 0) { + wc_FreeRng(&mlRng); + wc_MlDsaKey_Free(&mlKey); + result = -658; goto done; + } + wc_FreeRng(&mlRng); + + mlDer = (byte*)WMALLOC(WC_MLDSA_44_PRV_KEY_DER_SIZE, NULL, 0); + if (mlDer == NULL) { + wc_MlDsaKey_Free(&mlKey); + result = -659; goto done; + } + mlDerSz = wc_MlDsaKey_PrivateKeyToDer(&mlKey, mlDer, + WC_MLDSA_44_PRV_KEY_DER_SIZE); + wc_MlDsaKey_Free(&mlKey); + if (mlDerSz <= 0) { + WFREE(mlDer, NULL, 0); + result = -660; goto done; + } + + ret = IdentifyAsn1Key(mlDer, (word32)mlDerSz, 1, NULL, NULL); + if (ret != ID_MLDSA44) { + WFREE(mlDer, NULL, 0); + printf("IdentifyAsn1Key: private-only MlDsa DER expected " + "ID_MLDSA44, got %d\n", ret); + result = -661; goto done; + } + + /* Same DER, pkey-out variant: confirms it succeeds and populates *pkey. */ + { + WS_KeySignature* mlKeySig = NULL; + + ret = IdentifyAsn1Key(mlDer, (word32)mlDerSz, 1, NULL, + &mlKeySig); + WFREE(mlDer, NULL, 0); + if (ret != ID_MLDSA44 || mlKeySig == NULL || + mlKeySig->keyId != ID_MLDSA44) { + printf("IdentifyAsn1Key: private-only MlDsa DER pkey-out " + "variant failed, ret=%d\n", ret); + if (mlKeySig != NULL) { + wolfSSH_KEY_clean(mlKeySig); + WFREE(mlKeySig, NULL, 0); + } + result = -662; goto done; + } + wolfSSH_KEY_clean(mlKeySig); + WFREE(mlKeySig, NULL, 0); + } + } #endif /* Unsupported ECC curve: triggers wc_ecc_free in the default: branch