diff --git a/.github/workflows/sanitizer.yml b/.github/workflows/sanitizer.yml index 158c18326..a1dd03822 100644 --- a/.github/workflows/sanitizer.yml +++ b/.github/workflows/sanitizer.yml @@ -12,9 +12,21 @@ concurrency: jobs: build_wolfssl: - name: Build wolfSSL + name: Build wolfSSL (${{ matrix.name }}) runs-on: ubuntu-latest timeout-minutes: 5 + strategy: + fail-fast: false + matrix: + include: + - name: spmath + config: "--enable-wolfssh --enable-keygen --enable-pkcallbacks" + # Heap math keeps big numbers (RSA/ECC key material) in heap + # allocations. The default SP math keeps them in fixed buffers + # inside the key struct, so a leaked key is invisible to + # LeakSanitizer. This variant makes such leaks detectable. + - name: heapmath + config: "--enable-wolfssh --enable-keygen --enable-pkcallbacks --enable-heapmath" steps: - name: Checkout wolfSSL uses: actions/checkout@v6 @@ -26,7 +38,7 @@ jobs: working-directory: ./wolfssl run: | ./autogen.sh - ./configure --enable-wolfssh --enable-keygen --enable-pkcallbacks + ./configure ${{ matrix.config }} make -j$(nproc) sudo make install sudo ldconfig @@ -37,7 +49,7 @@ jobs: - name: Upload built lib uses: actions/upload-artifact@v7 with: - name: wolfssl-sanitizer + name: wolfssl-sanitizer-${{ matrix.name }} path: wolfssl-install.tgz retention-days: 5 @@ -51,11 +63,19 @@ jobs: matrix: include: - name: "ASan" + wolfssl: spmath cflags: "-fsanitize=address -fno-omit-frame-pointer -g -O1" ldflags: "-fsanitize=address" - name: "UBSan" + wolfssl: spmath cflags: "-fsanitize=undefined -fno-sanitize-recover=all -fno-omit-frame-pointer -g" ldflags: "-fsanitize=undefined" + # ASan against a heap-math wolfSSL so LeakSanitizer can see a + # leaked key (see the build_wolfssl heapmath note above). + - name: "ASan (heap math)" + wolfssl: heapmath + cflags: "-fsanitize=address -fno-omit-frame-pointer -g -O1" + ldflags: "-fsanitize=address" steps: - name: Workaround high-entropy ASLR @@ -67,7 +87,7 @@ jobs: - name: Download wolfSSL uses: actions/download-artifact@v8 with: - name: wolfssl-sanitizer + name: wolfssl-sanitizer-${{ matrix.wolfssl }} - name: Install wolfSSL run: | diff --git a/src/internal.c b/src/internal.c index 996f8f7a6..e2eaa452e 100644 --- a/src/internal.c +++ b/src/internal.c @@ -622,6 +622,22 @@ static void HandshakeInfoFree(HandshakeInfo* hs, void* heap) #ifndef WOLFSSH_NO_DH WFREE(hs->primeGroup, heap, DYNTYPE_MPINT); WFREE(hs->generator, heap, DYNTYPE_MPINT); +#endif +#ifndef WOLFSSH_NO_DH + if (hs->useDh) { + wc_FreeDhKey(&hs->privKey.dh); + } +#endif +#ifndef WOLFSSH_NO_ECDH + if (hs->useEcc || hs->useEccMlKem) { + wc_ecc_free(&hs->privKey.ecc); + } +#endif +#if !defined(WOLFSSH_NO_CURVE25519_SHA256) || \ + !defined(WOLFSSH_NO_CURVE25519_MLKEM768_SHA256) + if (hs->useCurve25519 || hs->useCurve25519MlKem) { + wc_curve25519_free(&hs->privKey.curve25519); + } #endif if (hs->kexHashId != WC_HASH_TYPE_NONE) { wc_HashFree(&hs->kexHash, (enum wc_HashType)hs->kexHashId); @@ -5004,6 +5020,10 @@ struct wolfSSH_sigKeyBlock { }; +/* Defined next to its counterpart FreePubKey() below. */ +static int InitPubKey(struct wolfSSH_sigKeyBlock *p, WOLFSSH *ssh); + + /* Parse out a RAW RSA public key from buffer */ static int ParseRSAPubKey(WOLFSSH *ssh, struct wolfSSH_sigKeyBlock *sigKeyBlock_ptr, byte *pubKey, word32 pubKeySz) @@ -5016,9 +5036,7 @@ static int ParseRSAPubKey(WOLFSSH *ssh, word32 nSz; word32 pubKeyIdx = 0; - ret = wc_InitRsaKey(&sigKeyBlock_ptr->sk.rsa.key, ssh->ctx->heap); - if (ret != 0) - ret = WS_RSA_E; + ret = InitPubKey(sigKeyBlock_ptr, ssh); /* Skip the algo name. */ if (ret == WS_SUCCESS) ret = GetSkip(pubKey, pubKeySz, &pubKeyIdx); @@ -5042,7 +5060,6 @@ static int ParseRSAPubKey(WOLFSSH *ssh, if (ret == 0) { sigKeyBlock_ptr->keySz = (word32)sizeof(sigKeyBlock_ptr->sk.rsa.key); - sigKeyBlock_ptr->keyAllocated = 1; } else ret = WS_RSA_E; @@ -5065,21 +5082,12 @@ static int ParseECCPubKey(WOLFSSH *ssh, word32 pubKeyIdx = 0; int primeId = 0; - ret = wc_ecc_init_ex(&sigKeyBlock_ptr->sk.ecc.key, ssh->ctx->heap, - INVALID_DEVID); - if (ret == 0) { - /* The key is initialized. Mark it so that FreePubKey() cleans - * it up on all paths, including the error paths below. */ - sigKeyBlock_ptr->keyAllocated = 1; - } + ret = InitPubKey(sigKeyBlock_ptr, ssh); #ifdef HAVE_WC_ECC_SET_RNG - if (ret == 0) - ret = wc_ecc_set_rng(&sigKeyBlock_ptr->sk.ecc.key, ssh->rng); -#endif - if (ret != 0) + if (ret == WS_SUCCESS + && wc_ecc_set_rng(&sigKeyBlock_ptr->sk.ecc.key, ssh->rng) != 0) ret = WS_ECC_E; - else - ret = WS_SUCCESS; +#endif /* Get the algorithm name in the key block. It must match the * negotiated host key algorithm. Do not trust the key blob to @@ -5167,10 +5175,7 @@ static int ParseEd25519PubKey(WOLFSSH *ssh, const byte* encA; word32 encASz, pubKeyIdx = 0; - ret = wc_ed25519_init_ex(&sigKeyBlock_ptr->sk.ed25519.key, - ssh->ctx->heap, INVALID_DEVID); - if (ret != 0) - ret = WS_ED25519_E; + ret = InitPubKey(sigKeyBlock_ptr, ssh); /* Skip the algo name */ if (ret == WS_SUCCESS) { @@ -5189,9 +5194,6 @@ static int ParseEd25519PubKey(WOLFSSH *ssh, } } - if (ret == 0) { - sigKeyBlock_ptr->keyAllocated = 1; - } return ret; } #else @@ -5371,8 +5373,7 @@ static int ParseECCPubKeyCert(WOLFSSH *ssh, ret = ParsePubKeyCert(ssh, pubKey, pubKeySz, &der, &derSz); if (ret == WS_SUCCESS) { - error = wc_ecc_init_ex(&sigKeyBlock_ptr->sk.ecc.key, ssh->ctx->heap, - INVALID_DEVID); + error = InitPubKey(sigKeyBlock_ptr, ssh); #ifdef HAVE_WC_ECC_SET_RNG if (error == 0) error = wc_ecc_set_rng(&sigKeyBlock_ptr->sk.ecc.key, ssh->rng); @@ -5382,7 +5383,6 @@ static int ParseECCPubKeyCert(WOLFSSH *ssh, &sigKeyBlock_ptr->sk.ecc.key, derSz); if (error == 0) { sigKeyBlock_ptr->keySz = (word32)sizeof(sigKeyBlock_ptr->sk.ecc.key); - sigKeyBlock_ptr->keyAllocated = 1; } if (error != 0) ret = error; @@ -5413,14 +5413,13 @@ static int ParseRSAPubKeyCert(WOLFSSH *ssh, ret = ParsePubKeyCert(ssh, pubKey, pubKeySz, &der, &derSz); if (ret == WS_SUCCESS) { - error = wc_InitRsaKey(&sigKeyBlock_ptr->sk.rsa.key, ssh->ctx->heap); + error = InitPubKey(sigKeyBlock_ptr, ssh); if (error == 0) error = wc_RsaPublicKeyDecode(der, &idx, &sigKeyBlock_ptr->sk.rsa.key, derSz); if (error == 0) { sigKeyBlock_ptr->keySz = (word32)sizeof(sigKeyBlock_ptr->sk.rsa.key); - sigKeyBlock_ptr->keyAllocated = 1; } if (error != 0) ret = error; @@ -5490,6 +5489,43 @@ static int ParsePubKey(WOLFSSH *ssh, } +/* Initialize the key in the block for its negotiated type. Sets + * keyAllocated so FreePubKey() releases it on every path, including the + * callers' error paths. Counterpart to FreePubKey(); the caller sets the + * use* flag before calling. */ +static int InitPubKey(struct wolfSSH_sigKeyBlock *p, WOLFSSH *ssh) +{ + int ret = WS_INVALID_ALGO_ID; + WOLFSSH_UNUSED(ssh); + + if (p->useRsa) { + #ifndef WOLFSSH_NO_RSA + ret = (wc_InitRsaKey(&p->sk.rsa.key, ssh->ctx->heap) == 0) ? + WS_SUCCESS : WS_RSA_E; + #endif + } + else if (p->useEcc) { + #ifndef WOLFSSH_NO_ECDSA + ret = (wc_ecc_init_ex(&p->sk.ecc.key, ssh->ctx->heap, INVALID_DEVID) + == 0) ? WS_SUCCESS : WS_ECC_E; + #endif + } + else if (p->useEd25519) { + #ifndef WOLFSSH_NO_ED25519 + ret = (wc_ed25519_init_ex(&p->sk.ed25519.key, ssh->ctx->heap, + INVALID_DEVID) == 0) ? WS_SUCCESS : WS_ED25519_E; + #endif + } + + /* Mark the key allocated as soon as init succeeds so FreePubKey() + * frees it even if a later parse step fails. */ + if (ret == WS_SUCCESS) + p->keyAllocated = 1; + + return ret; +} + + static void FreePubKey(struct wolfSSH_sigKeyBlock *p) { if (p && p->keyAllocated) { @@ -18357,6 +18393,19 @@ int wolfSSH_TestDoUserAuthRequestRsaCert(WOLFSSH* ssh, #endif /* WOLFSSH_CERTS */ +int wolfSSH_TestParseRSAPubKey(WOLFSSH* ssh, byte* pubKey, word32 pubKeySz) +{ + struct wolfSSH_sigKeyBlock sigKeyBlock; + int ret; + + WMEMSET(&sigKeyBlock, 0, sizeof(sigKeyBlock)); + sigKeyBlock.useRsa = 1; + ret = ParseRSAPubKey(ssh, &sigKeyBlock, pubKey, pubKeySz); + FreePubKey(&sigKeyBlock); + + return ret; +} + #endif /* !WOLFSSH_NO_RSA */ #ifndef WOLFSSH_NO_ECDSA @@ -18378,6 +18427,19 @@ int wolfSSH_TestParseECCPubKey(WOLFSSH* ssh, byte* pubKey, word32 pubKeySz) #ifndef WOLFSSH_NO_ED25519 +int wolfSSH_TestParseEd25519PubKey(WOLFSSH* ssh, byte* pubKey, word32 pubKeySz) +{ + struct wolfSSH_sigKeyBlock sigKeyBlock; + int ret; + + WMEMSET(&sigKeyBlock, 0, sizeof(sigKeyBlock)); + sigKeyBlock.useEd25519 = 1; + ret = ParseEd25519PubKey(ssh, &sigKeyBlock, pubKey, pubKeySz); + FreePubKey(&sigKeyBlock); + + return ret; +} + int wolfSSH_TestDoUserAuthRequestEd25519(WOLFSSH* ssh, WS_UserAuthData* authData) { diff --git a/tests/unit.c b/tests/unit.c index 882c704ef..690a15a08 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -2487,6 +2487,58 @@ static void Ed25519Test_PutLen(byte* out, word32 v) out[3] = (byte)(v); } +static int test_ParseEd25519PubKey(void) +{ + static const char keyTypeName[] = "ssh-ed25519"; + const word32 keyTypeNameSz = (word32)(sizeof(keyTypeName) - 1); + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + byte blob[64]; + word32 blobSz, off; + 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) { + wolfSSH_CTX_free(ctx); + return 1; + } + + /* string "ssh-ed25519" || string pubkey */ + off = 0; + Ed25519Test_PutLen(blob + off, keyTypeNameSz); off += UINT32_SZ; + WMEMCPY(blob + off, keyTypeName, keyTypeNameSz); off += keyTypeNameSz; + Ed25519Test_PutLen(blob + off, (word32)sizeof(unitTestEd25519Pub)); + off += UINT32_SZ; + WMEMCPY(blob + off, unitTestEd25519Pub, sizeof(unitTestEd25519Pub)); + off += (word32)sizeof(unitTestEd25519Pub); + blobSz = off; + + /* valid blob */ + ret = wolfSSH_TestParseEd25519PubKey(ssh, blob, blobSz); + if (ret != WS_SUCCESS) { + fprintf(stderr, "\t\"valid\" FAIL: got %d, expected %d\n", + ret, WS_SUCCESS); + failures++; + } + + /* truncated blob: fails after the key is initialized */ + ret = wolfSSH_TestParseEd25519PubKey(ssh, blob, blobSz - 4); + if (ret != WS_BUFFER_E) { + fprintf(stderr, "\t\"truncated\" FAIL: got %d, expected %d\n", + ret, WS_BUFFER_E); + failures++; + } + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + + return failures; +} + /* DoUserAuthRequestEd25519 unit test * * Drives DoUserAuthRequestEd25519 directly with a fully-formed Ed25519 @@ -4201,6 +4253,47 @@ static int test_DoUserAuthRequestRsa(void) return failures; } +static int test_ParseRSAPubKey(void) +{ + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + byte blob[sizeof(userAuthRsaPubKeyBlob)]; + 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) { + wolfSSH_CTX_free(ctx); + return 1; + } + + WMEMCPY(blob, userAuthRsaPubKeyBlob, sizeof(blob)); + + /* valid blob */ + ret = wolfSSH_TestParseRSAPubKey(ssh, blob, (word32)sizeof(blob)); + if (ret != WS_SUCCESS) { + fprintf(stderr, "\t\"valid\" FAIL: got %d, expected %d\n", + ret, WS_SUCCESS); + failures++; + } + + /* truncated blob: fails after the key is initialized */ + ret = wolfSSH_TestParseRSAPubKey(ssh, blob, (word32)(sizeof(blob) / 2)); + if (ret != WS_RSA_E) { + fprintf(stderr, "\t\"truncated\" FAIL: got %d, expected %d\n", + ret, WS_RSA_E); + failures++; + } + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + + return failures; +} + #ifdef WOLFSSH_CERTS /* DoUserAuthRequestRsaCert() parses the signature blob the same way as @@ -4502,6 +4595,11 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("DoUserAuthRequestRsa: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + + unitResult = test_ParseRSAPubKey(); + printf("ParseRSAPubKey: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; #ifdef WOLFSSH_CERTS unitResult = test_DoUserAuthRequestRsaCert(); printf("DoUserAuthRequestRsaCert: %s\n", @@ -4522,6 +4620,11 @@ int wolfSSH_UnitTest(int argc, char** argv) printf("DoUserAuthRequestEd25519: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + + unitResult = test_ParseEd25519PubKey(); + printf("ParseEd25519PubKey: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; #endif unitResult = test_ChannelPutData(); printf("ChannelPutData: %s\n", (unitResult == 0 ? "SUCCESS" : "FAILED")); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 6733e092d..b2006bc98 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1432,12 +1432,16 @@ enum WS_MessageIdLimits { WS_UserAuthData_PublicKey* pk, int hashId, byte* digest, word32 digestSz); #endif /* WOLFSSH_CERTS */ + WOLFSSH_API int wolfSSH_TestParseRSAPubKey(WOLFSSH* ssh, byte* pubKey, + word32 pubKeySz); #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_TestParseEd25519PubKey(WOLFSSH* ssh, byte* pubKey, + word32 pubKeySz); WOLFSSH_API int wolfSSH_TestDoUserAuthRequestEd25519(WOLFSSH* ssh, WS_UserAuthData* authData); #endif /* !WOLFSSH_NO_ED25519 */