Skip to content

Added Public Key Validation for Decoded Private Host Keys #1121

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.

Sign up for GitHub

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

Open
stenslae wants to merge 1 commit into
base: master
Choose a base branch
from
+220 −2
Open

Added Public Key Validation for Decoded Private Host Keys #1121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
81 changes: 79 additions & 2 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1695,7 +1695,9 @@ 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)
*/
int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap,
WS_KeySignature **pkey)
Expand All @@ -1704,6 +1706,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;
Expand Down Expand Up @@ -1757,6 +1760,32 @@ int IdentifyAsn1Key(const byte* in, word32 inSz, int isPrivate, void* heap,
if (isPrivate) {
ret = wc_EccPrivateKeyDecode(in, &idx,
&key->ks.ecc.key, inSz);
if (ret == 0) {
/* SEC1 allows omitting the public point. Not fatal
* here (unlike ML-DSA/Ed25519): SendKexGetSigningKey()
* skips re-deriving it entirely when the key is
* paired with a cert (see its `!isCert` guard around
* wc_ecc_export_x963()), and that pairing isn't known
* yet here, for either a host key or a client
* user-auth key. Unpaired, this still fails, just
* later at KEX/auth. */
byte pubScratch[257];
word32 pubScratchSz = sizeof(pubScratch);

/* ECC-specific: matches wc_ecc_export_x963() usage
* elsewhere in this file (eg SendKexDhReply()); the
* ML-DSA/Ed25519 export calls below are never
* wrapped this way. */
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 {
ret = wc_EccPublicKeyDecode(in, &idx,
Expand Down Expand Up @@ -1796,6 +1825,35 @@ 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: at MLDSA_MAX_PUB_KEY_SIZE
* (ML-DSA-87) this would add ~2.5KB to the stack,
* unlike everywhere else in this file that needs a
* key block this size (eg the sigKeyBlock
* allocations), breaking WOLFSSH_SMALL_STACK. */
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. */
Expand Down Expand Up @@ -1865,6 +1923,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,
Comment thread
stenslae marked this conversation as resolved.
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,
Expand All @@ -1882,7 +1957,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)
Expand Down
141 changes: 141 additions & 0 deletions tests/unit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6027,6 +6027,54 @@ 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 = -627; goto done;
}
if (wc_InitRng(&eccRng) != 0) {
wc_ecc_free(&eccKey);
result = -628; goto done;
}
if (wc_ecc_make_key(&eccRng, 32, &eccKey) != 0) {
wc_FreeRng(&eccRng);
wc_ecc_free(&eccKey);
result = -629; goto done;
}
wc_FreeRng(&eccRng);

eccDerSz = wc_EccKeyDerSize(&eccKey, 0);
if (eccDerSz <= 0) {
wc_ecc_free(&eccKey);
result = -630; goto done;
}
eccDer = (byte*)WMALLOC((word32)eccDerSz, NULL, 0);
if (eccDer == NULL) {
wc_ecc_free(&eccKey);
result = -631; goto done;
}
eccDerSz = wc_EccPrivateKeyToDer(&eccKey, eccDer, (word32)eccDerSz);
wc_ecc_free(&eccKey);
if (eccDerSz <= 0) {
WFREE(eccDer, NULL, 0);
result = -632; goto done;
}

ret = IdentifyAsn1Key(eccDer, (word32)eccDerSz, 1, NULL, NULL);
WFREE(eccDer, NULL, 0);
if (ret != ID_ECDSA_SHA2_NISTP256) {
printf("IdentifyAsn1Key: private-only ECC DER expected "
"ID_ECDSA_SHA2_NISTP256, got %d\n", ret);
result = -633; goto done;
}
}
#endif

#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP384
Expand Down Expand Up @@ -6057,6 +6105,50 @@ 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 = -634; goto done;
}
if (wc_InitRng(&edRng) != 0) {
wc_ed25519_free(&edKey);
result = -635; goto done;
}
if (wc_ed25519_make_key(&edRng, ED25519_KEY_SIZE, &edKey) != 0) {
wc_FreeRng(&edRng);
wc_ed25519_free(&edKey);
result = -636; goto done;
}
wc_FreeRng(&edRng);

edDer = (byte*)WMALLOC(128, NULL, 0);
if (edDer == NULL) {
wc_ed25519_free(&edKey);
result = -637; goto done;
}
edDerSz = wc_Ed25519PrivateKeyToDer(&edKey, edDer, 128);
wc_ed25519_free(&edKey);
if (edDerSz <= 0) {
WFREE(edDer, NULL, 0);
result = -638; goto done;
}

ret = IdentifyAsn1Key(edDer, (word32)edDerSz, 1, NULL, NULL);
WFREE(edDer, NULL, 0);
if (ret != WS_CRYPTO_FAILED) {
printf("IdentifyAsn1Key: private-only Ed25519 DER expected "
"WS_CRYPTO_FAILED, got %d\n", ret);
result = -639; goto done;
}
}
#endif

#if !defined(WOLFSSH_NO_MLDSA) && !defined(WOLFSSH_NO_MLDSA44)
Expand Down Expand Up @@ -6116,6 +6208,55 @@ static int test_IdentifyAsn1Key(void)
result = -611; goto done;
}
}

/* Private-only DER (no embedded public key, unlike unitTestMlDsaPrivKey
* above): must be rejected at decode time instead of failing later at
* KEX. */
{
MlDsaKey mlKey;
WC_RNG mlRng;
byte* mlDer = NULL;
int mlDerSz;

if (wc_MlDsaKey_Init(&mlKey, NULL, INVALID_DEVID) != 0) {
result = -620; goto done;
}
if (wc_MlDsaKey_SetParams(&mlKey, WC_ML_DSA_44) != 0) {
wc_MlDsaKey_Free(&mlKey);
result = -621; goto done;
}
if (wc_InitRng(&mlRng) != 0) {
wc_MlDsaKey_Free(&mlKey);
result = -622; goto done;
}
if (wc_MlDsaKey_MakeKey(&mlKey, &mlRng) != 0) {
wc_FreeRng(&mlRng);
wc_MlDsaKey_Free(&mlKey);
result = -623; 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 = -624; 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 = -625; goto done;
}

ret = IdentifyAsn1Key(mlDer, (word32)mlDerSz, 1, NULL, NULL);
WFREE(mlDer, NULL, 0);
if (ret != WS_CRYPTO_FAILED) {
printf("IdentifyAsn1Key: private-only MlDsa DER expected "
"WS_CRYPTO_FAILED, got %d\n", ret);
result = -626; goto done;
}
}
#endif

/* Unsupported ECC curve: triggers wc_ecc_free in the default: branch
Expand Down
Loading