From 31d13697a608fa1bf9eec11aa6eff1503ade836f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 11 Jun 2026 15:20:29 -0700 Subject: [PATCH] Bind ECDSA host key curve to negotiated algo - Validate blob algorithm name against handshake pubKeyId - Derive curve from negotiated algo, not the key blob - Check curve name instead of skipping it - Add a ParseECCPubKey test checking the key blob algorithm and curve names are validated against the negotiated host key algorithm. Issue: #1012 --- src/internal.c | 51 +++++++++++++++++---- tests/unit.c | 109 +++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 4 ++ 3 files changed, 155 insertions(+), 9 deletions(-) diff --git a/src/internal.c b/src/internal.c index d2c9bb24..86e77311 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5065,6 +5065,8 @@ static int ParseECCPubKey(WOLFSSH *ssh, const byte* q; word32 qSz, pubKeyIdx = 0; int primeId = 0; + const char* algoName; + const char* curveName; ret = wc_ecc_init_ex(&sigKeyBlock_ptr->sk.ecc.key, ssh->ctx->heap, INVALID_DEVID); @@ -5077,20 +5079,34 @@ static int ParseECCPubKey(WOLFSSH *ssh, else ret = GetStringRef(&qSz, &q, pubKey, pubKeySz, &pubKeyIdx); + /* The algorithm name in the key blob must match the negotiated host key + * algorithm. A MitM must not be able to swap in a different curve by + * lying in the blob, so don't trust the blob to choose the curve. */ if (ret == WS_SUCCESS) { - primeId = (int)NameToId((const char*)q, qSz); - if (primeId != ID_UNKNOWN) { - primeId = wcPrimeForId((byte)primeId); - if (primeId == ECC_CURVE_INVALID) - ret = WS_INVALID_PRIME_CURVE; - } - else + algoName = IdToName(ssh->handshake->pubKeyId); + if (qSz != (word32)WSTRLEN(algoName) + || WMEMCMP(q, algoName, qSz) != 0) ret = WS_INVALID_ALGO_ID; } - /* Skip the curve name since we're getting it from the algo. */ + /* Derive the curve from the negotiated algorithm, not from the blob. */ + if (ret == WS_SUCCESS) { + primeId = wcPrimeForId(ssh->handshake->pubKeyId); + if (primeId == ECC_CURVE_INVALID) + ret = WS_INVALID_PRIME_CURVE; + } + + /* The curve name (RFC 5656 section 3.1) in the blob must match the + * curve of the negotiated algorithm. */ if (ret == WS_SUCCESS) - ret = GetSkip(pubKey, pubKeySz, &pubKeyIdx); + ret = GetStringRef(&qSz, &q, pubKey, pubKeySz, &pubKeyIdx); + + if (ret == WS_SUCCESS) { + curveName = PrimeNameForId(ssh->handshake->pubKeyId); + if (qSz != (word32)WSTRLEN(curveName) + || WMEMCMP(q, curveName, qSz) != 0) + ret = WS_INVALID_PRIME_CURVE; + } if (ret == WS_SUCCESS) ret = GetStringRef(&qSz, &q, pubKey, pubKeySz, &pubKeyIdx); @@ -18307,6 +18323,23 @@ int wolfSSH_TestDoUserAuthRequestRsa(WOLFSSH* ssh, #endif /* !WOLFSSH_NO_RSA */ +#ifndef WOLFSSH_NO_ECDSA + +int wolfSSH_TestParseECCPubKey(WOLFSSH* ssh, byte* pubKey, word32 pubKeySz) +{ + struct wolfSSH_sigKeyBlock sigKeyBlock; + int ret; + + WMEMSET(&sigKeyBlock, 0, sizeof(sigKeyBlock)); + sigKeyBlock.useEcc = 1; + ret = ParseECCPubKey(ssh, &sigKeyBlock, pubKey, pubKeySz); + FreePubKey(&sigKeyBlock); + + return ret; +} + +#endif /* !WOLFSSH_NO_ECDSA */ + #ifndef WOLFSSH_NO_ED25519 int wolfSSH_TestDoUserAuthRequestEd25519(WOLFSSH* ssh, diff --git a/tests/unit.c b/tests/unit.c index cd6e040a..14a5a543 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -3935,6 +3935,109 @@ cleanup: #endif /* WOLFSSH_SCP recv callback depth guard test */ +/* ParseECCPubKey() Unit Test */ + +#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256 + +/* The payload of keys/gretel-key-ecc.pub: + * string "ecdsa-sha2-nistp256" | string "nistp256" | string Q + * ParseECCPubKey() must reject blobs whose algorithm name or curve name + * does not match the negotiated host key algorithm. A MitM must not be + * able to choose a different curve by lying in the blob. */ +static const byte eccPubKeyBlob[] = { + 0x00, 0x00, 0x00, 0x13, 0x65, 0x63, 0x64, 0x73, 0x61, 0x2D, 0x73, 0x68, + 0x61, 0x32, 0x2D, 0x6E, 0x69, 0x73, 0x74, 0x70, 0x32, 0x35, 0x36, 0x00, + 0x00, 0x00, 0x08, 0x6E, 0x69, 0x73, 0x74, 0x70, 0x32, 0x35, 0x36, 0x00, + 0x00, 0x00, 0x41, 0x04, 0xA0, 0x2D, 0x1F, 0xC7, 0x2A, 0x68, 0x36, 0xED, + 0x24, 0x58, 0xED, 0xBE, 0x22, 0xE8, 0x6C, 0x70, 0x66, 0x8C, 0x2B, 0x46, + 0xE7, 0xA0, 0xCC, 0x90, 0xFE, 0x80, 0xE0, 0xCD, 0x87, 0xF7, 0x35, 0xF6, + 0xFD, 0x80, 0xA0, 0xD6, 0x1F, 0x5B, 0x61, 0x2E, 0xD6, 0x1D, 0xDF, 0x54, + 0x40, 0x3C, 0x17, 0x3B, 0x51, 0xE1, 0x21, 0x9C, 0xD1, 0x61, 0xE7, 0x17, + 0x87, 0xB4, 0x86, 0xF4, 0xFE, 0x06, 0x85, 0x16, +}; + +/* Offsets of interest in eccPubKeyBlob. */ +#define ECC_BLOB_ALGO_DIGITS 20 /* the "256" in "ecdsa-sha2-nistp256" */ +#define ECC_BLOB_CURVE_DIGITS 32 /* the "256" in "nistp256" */ +#define ECC_BLOB_POINT 39 /* leading byte (0x04) of Q */ +#define ECC_BLOB_TRUNC_SZ 30 /* cuts the blob mid curve name */ + +static const byte eccBadPointFormat[] = { 0x05 }; + +typedef struct { + const char* name; + word32 patchIdx; + const byte* patch; + word32 patchSz; /* 0 = no patch */ + word32 blobSz; + byte pubKeyId; /* negotiated host key algorithm */ + int expected; +} ParseECCPubKeyTestVector; + +static const ParseECCPubKeyTestVector parseECCPubKeyTestVectors[] = { + { "valid nistp256 blob", 0, NULL, 0, sizeof(eccPubKeyBlob), + ID_ECDSA_SHA2_NISTP256, WS_SUCCESS }, + { "algo name mismatch", ECC_BLOB_ALGO_DIGITS, (const byte*)"384", 3, + sizeof(eccPubKeyBlob), ID_ECDSA_SHA2_NISTP256, WS_INVALID_ALGO_ID }, +#ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP384 + { "blob downgrades negotiated nistp384", 0, NULL, 0, + sizeof(eccPubKeyBlob), ID_ECDSA_SHA2_NISTP384, WS_INVALID_ALGO_ID }, +#endif + { "curve name mismatch", ECC_BLOB_CURVE_DIGITS, (const byte*)"384", 3, + sizeof(eccPubKeyBlob), ID_ECDSA_SHA2_NISTP256, + WS_INVALID_PRIME_CURVE }, + { "corrupt point format", ECC_BLOB_POINT, eccBadPointFormat, 1, + sizeof(eccPubKeyBlob), ID_ECDSA_SHA2_NISTP256, WS_ECC_E }, + { "truncated blob", 0, NULL, 0, ECC_BLOB_TRUNC_SZ, + ID_ECDSA_SHA2_NISTP256, WS_BUFFER_E }, +}; + +static int test_ParseECCPubKey(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + const ParseECCPubKeyTestVector* tv; + int tc = (int)(sizeof(parseECCPubKeyTestVectors) + / sizeof(parseECCPubKeyTestVectors[0])); + int i; + int ret; + int failures = 0; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + if (ctx == NULL) + return 1; + ssh = wolfSSH_new(ctx); + if (ssh == NULL || ssh->handshake == NULL) { + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + return 1; + } + + for (i = 0, tv = parseECCPubKeyTestVectors; i < tc; i++, tv++) { + byte blob[sizeof(eccPubKeyBlob)]; + + WMEMCPY(blob, eccPubKeyBlob, sizeof(eccPubKeyBlob)); + if (tv->patchSz > 0) + WMEMCPY(blob + tv->patchIdx, tv->patch, tv->patchSz); + ssh->handshake->pubKeyId = tv->pubKeyId; + + ret = wolfSSH_TestParseECCPubKey(ssh, blob, tv->blobSz); + if (ret != tv->expected) { + fprintf(stderr, "\t[%d] \"%s\" FAIL: got %d, expected %d\n", + i, tv->name, ret, tv->expected); + failures++; + } + } + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + + return failures; +} + +#endif /* !WOLFSSH_NO_ECDSA_SHA2_NISTP256 */ + + /* DoUserAuthRequestRsa() Unit Test */ #if !defined(WOLFSSH_NO_RSA) && !defined(WOLFSSH_NO_SSH_RSA_SHA1) @@ -4241,6 +4344,12 @@ int wolfSSH_UnitTest(int argc, char** argv) (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; #endif +#if !defined(WOLFSSH_NO_ECDSA_SHA2_NISTP256) + unitResult = test_ParseECCPubKey(); + printf("ParseECCPubKey: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; +#endif #if !defined(WOLFSSH_NO_ED25519) && defined(HAVE_ED25519) && \ defined(HAVE_ED25519_SIGN) && defined(HAVE_ED25519_VERIFY) && \ defined(WOLFSSL_ED25519_STREAMING_VERIFY) diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 94628b03..3f80ee80 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1428,6 +1428,10 @@ enum WS_MessageIdLimits { WS_UserAuthData_PublicKey* pk, int hashId, byte* digest, word32 digestSz); #endif /* !WOLFSSH_NO_RSA */ +#ifndef WOLFSSH_NO_ECDSA + WOLFSSH_API int wolfSSH_TestParseECCPubKey(WOLFSSH* ssh, byte* pubKey, + word32 pubKeySz); +#endif /* !WOLFSSH_NO_ECDSA */ #ifndef WOLFSSH_NO_ED25519 WOLFSSH_API int wolfSSH_TestDoUserAuthRequestEd25519(WOLFSSH* ssh, WS_UserAuthData* authData);