From 252ea8d5de154da3a71f224f7a5c655f64a6ccac Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Fri, 7 Aug 2026 17:05:03 +0900 Subject: [PATCH] Report a failed PEM decode as WS_PARSE_E, not WS_BAD_FILE_E --- src/internal.c | 4 +- src/ssh.c | 4 +- tests/api.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++--- wolfssh/ssh.h | 3 +- 4 files changed, 115 insertions(+), 11 deletions(-) diff --git a/src/internal.c b/src/internal.c index 2de2e45cc..cc116157b 100644 --- a/src/internal.c +++ b/src/internal.c @@ -2607,14 +2607,14 @@ int wolfSSH_ProcessBuffer(WOLFSSH_CTX* ctx, return WS_UNIMPLEMENTED_E; } #endif /* WOLFSSH_CERTS */ - if (ret < 0) { + if (ret <= 0) { if (type == BUFTYPE_PRIVKEY) { /* wc_KeyPemToDer may have written partial key material; * zeroize before free on the private-key path. */ WS_FORCEZERO(der, inSz); } WFREE(der, heap, dynamicType); - return WS_BAD_FILE_E; + return WS_PARSE_E; } derSz = (word32)ret; } diff --git a/src/ssh.c b/src/ssh.c index 8bd609fc7..0bcb0e3c5 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -2459,9 +2459,9 @@ static int DoPemCert(const byte* in, word32 inSz, byte** out, word32* outSz, ret = wc_CertPemToDer(in, (int)inSz, der, (int)inSz, CERT_TYPE); if (ret <= 0) { - WLOG(WS_LOG_DEBUG, "PEM to DER of certificate failed."); + WLOG(WS_LOG_DEBUG, "PEM certificate body would not decode."); WFREE(der, heap, DYNTYPE_CERT); - return WS_BAD_FILE_E; + return WS_PARSE_E; } derSz = (word32)ret; diff --git a/tests/api.c b/tests/api.c index d2c25e407..d5fb276ba 100644 --- a/tests/api.c +++ b/tests/api.c @@ -670,6 +670,43 @@ static int load_file(const char* filename, byte** buf, word32* bufSz) #endif +#ifdef WOLFSSH_CERTS + +/* PEM shapes that carry a header the sniff accepts but a body no decoder + * will take, so the failure lands in the decoder rather than the sniff. */ +static const char badPemCert[] = + "-----BEGIN CERTIFICATE-----\n" + "!!!! this is not base64 !!!!\n" + "-----END CERTIFICATE-----\n"; +static const char noBodyPemCert[] = "-----BEGIN CERTIFICATE-----\n"; +/* Under one full base64 group, so the body decodes to nothing rather than + * failing, and wolfSSL answers 0 for it instead of a negative code. */ +static const char zeroLenPemCert[] = + "-----BEGIN CERTIFICATE-----\n" + "MI\n" + "-----END CERTIFICATE-----\n"; + +#endif /* WOLFSSH_CERTS */ + + +#ifndef WOLFSSH_NO_SERVER + +/* The same shapes for a private key, which decodes without certificate + * support and so is pinned outside WOLFSSH_CERTS. */ +static const char badPemKey[] = + "-----BEGIN PRIVATE KEY-----\n" + "!!!! this is not base64 !!!!\n" + "-----END PRIVATE KEY-----\n"; +/* This one goes through wc_KeyPemToDer, which answers 0 rather than a + * negative code, so it needs its own fixture. */ +static const char zeroLenPemKey[] = + "-----BEGIN PRIVATE KEY-----\n" + "MI\n" + "-----END PRIVATE KEY-----\n"; + +#endif /* WOLFSSH_NO_SERVER */ + + static void test_wolfSSH_CTX_UseCert_buffer(void) { #ifdef WOLFSSH_CERTS @@ -704,6 +741,18 @@ static void test_wolfSSH_CTX_UseCert_buffer(void) AssertIntEQ(WS_BAD_FILETYPE_E, wolfSSH_CTX_UseCert_buffer(ctx, cert, certSz, 99)); + /* Content the caller declared PEM but that will not decode is malformed + * input, not a file that would not read. */ + AssertIntEQ(WS_PARSE_E, + wolfSSH_CTX_UseCert_buffer(ctx, (const byte*)badPemCert, + (word32)WSTRLEN(badPemCert), WOLFSSH_FORMAT_PEM)); + AssertIntEQ(WS_PARSE_E, + wolfSSH_CTX_UseCert_buffer(ctx, (const byte*)noBodyPemCert, + (word32)WSTRLEN(noBodyPemCert), WOLFSSH_FORMAT_PEM)); + AssertIntEQ(WS_PARSE_E, + wolfSSH_CTX_UseCert_buffer(ctx, (const byte*)zeroLenPemCert, + (word32)WSTRLEN(zeroLenPemCert), WOLFSSH_FORMAT_PEM)); + free(cert); cert = NULL; @@ -776,6 +825,7 @@ static void test_wolfSSH_ReadCert_buffer(void) #ifdef WOLFSSH_CERTS byte* cert = NULL; word32 certSz = 0; + byte stale[1]; #ifndef WOLFSSH_NO_ED25519 int ret; #endif @@ -834,6 +884,37 @@ static void test_wolfSSH_ReadCert_buffer(void) free(cert); cert = NULL; + /* Keeping the header sends these past the sniff and into the decoder, + * where a body that will not decode is a parse failure, not a file error. + * Sentinels go in, as the preceding rejection already cleared them all. */ + out = stale; + outSz = 0xDEADBEEF; + outType = stale; + outTypeSz = 0xDEADBEEF; + flavor = WOLFSSH_CERT_FLAVOR_X509; + AssertIntEQ(WS_PARSE_E, wolfSSH_ReadCert_buffer((const byte*)badPemCert, + (word32)WSTRLEN(badPemCert), + &out, &outSz, &outType, &outTypeSz, &flavor, NULL)); + AssertNull(out); + AssertIntEQ(outSz, 0); + AssertNull(outType); + AssertIntEQ(outTypeSz, 0); + AssertIntEQ(flavor, WOLFSSH_CERT_FLAVOR_UNKNOWN); + + out = stale; + outSz = 0xDEADBEEF; + outType = stale; + outTypeSz = 0xDEADBEEF; + flavor = WOLFSSH_CERT_FLAVOR_X509; + AssertIntEQ(WS_PARSE_E, wolfSSH_ReadCert_buffer((const byte*)noBodyPemCert, + (word32)WSTRLEN(noBodyPemCert), + &out, &outSz, &outType, &outTypeSz, &flavor, NULL)); + AssertNull(out); + AssertIntEQ(outSz, 0); + AssertNull(outType); + AssertIntEQ(outTypeSz, 0); + AssertIntEQ(flavor, WOLFSSH_CERT_FLAVOR_UNKNOWN); + AssertIntEQ(0, load_file("./keys/server-cert.der", &cert, &certSz)); #ifndef WOLFSSH_NO_ECDSA_SHA2_NISTP256 AssertIntEQ(WS_SUCCESS, wolfSSH_ReadCert_buffer(cert, certSz, @@ -1058,6 +1139,15 @@ static void test_wolfSSH_CTX_AddRootCert_file(void) /* The cert manager rejects a non-CA in wolfSSL's codes; this path maps. */ AssertIntEQ(WS_PARSE_E, wolfSSH_CTX_AddRootCert_file(ctx, "./keys/server-key-ecc.der")); + + /* The buffer entry point, tested here because it shares this one's + * decoder: a PEM body that will not decode is a parse failure. */ + AssertIntEQ(WS_PARSE_E, + wolfSSH_CTX_AddRootCert_buffer(ctx, (const byte*)badPemCert, + (word32)WSTRLEN(badPemCert), WOLFSSH_FORMAT_PEM)); + AssertIntEQ(WS_PARSE_E, + wolfSSH_CTX_AddRootCert_buffer(ctx, (const byte*)zeroLenPemCert, + (word32)WSTRLEN(zeroLenPemCert), WOLFSSH_FORMAT_PEM)); #ifdef WOLFSSH_TEST_OSSH_CERT_FILE AssertIntEQ(0, writeTmpFile(osshCertPath, osshCertLine, WSTRLEN(osshCertLine))); @@ -1082,15 +1172,19 @@ static void test_wolfSSH_CTX_AddRootCert_file(void) { ; } static void test_wolfSSH_CTX_UsePrivateKey_buffer_pem(void) { -#if defined(WOLFSSH_CERTS) && !defined(WOLFSSH_NO_SERVER) +#if !defined(WOLFSSH_NO_SERVER) WOLFSSH_CTX* ctx = NULL; +#ifdef WOLFSSH_CERTS byte* key = NULL; word32 keySz = 0; +#endif ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL); AssertNotNull(ctx); -#ifndef WOLFSSH_NO_RSA +/* The key files come in through load_file(), which certificate support + * carries, so the cases reading one are gated with it. */ +#if defined(WOLFSSH_CERTS) && !defined(WOLFSSH_NO_RSA) AssertIntEQ(0, load_file("./keys/server-key-rsa.pem", &key, &keySz)); AssertNotNull(key); AssertIntNE(0, keySz); @@ -1102,9 +1196,9 @@ static void test_wolfSSH_CTX_UsePrivateKey_buffer_pem(void) free(key); key = NULL; -#endif /* WOLFSSH_NO_RSA */ +#endif /* WOLFSSH_CERTS && !WOLFSSH_NO_RSA */ -#ifndef WOLFSSH_NO_ECDSA +#if defined(WOLFSSH_CERTS) && !defined(WOLFSSH_NO_ECDSA) AssertIntEQ(0, load_file("./keys/server-key-ecc.pem", &key, &keySz)); AssertNotNull(key); AssertIntNE(0, keySz); @@ -1116,10 +1210,19 @@ static void test_wolfSSH_CTX_UsePrivateKey_buffer_pem(void) free(key); key = NULL; -#endif /* WOLFSSH_NO_ECDSA */ +#endif /* WOLFSSH_CERTS && !WOLFSSH_NO_ECDSA */ + + /* A body that will not decode and one that decodes to nothing are both + * parse failures, on a path that needs no certificate support. */ + AssertIntEQ(WS_PARSE_E, + wolfSSH_CTX_UsePrivateKey_buffer(ctx, (const byte*)badPemKey, + (word32)WSTRLEN(badPemKey), WOLFSSH_FORMAT_PEM)); + AssertIntEQ(WS_PARSE_E, + wolfSSH_CTX_UsePrivateKey_buffer(ctx, (const byte*)zeroLenPemKey, + (word32)WSTRLEN(zeroLenPemKey), WOLFSSH_FORMAT_PEM)); wolfSSH_CTX_free(ctx); -#endif /* WOLFSSH_CERTS && !WOLFSSH_NO_SERVER */ +#endif /* WOLFSSH_NO_SERVER */ } diff --git a/wolfssh/ssh.h b/wolfssh/ssh.h index 96fef949e..88f75e119 100644 --- a/wolfssh/ssh.h +++ b/wolfssh/ssh.h @@ -106,7 +106,8 @@ WOLFSSH_API int wolfSSH_ReadKey_file(const char* name, #if defined(WOLFSSH_CERTS) || defined(WOLFSSH_OSSH_CERTS) /* Decodes a PEM/DER X.509 cert or OpenSSH cert line, detected from content. - * Caller frees out via heap; on failure every out param is cleared. */ + * Caller frees out via heap; on failure every out param is cleared. A body + * that will not decode is WS_PARSE_E; WS_BAD_FILE_E is from _file alone. */ WOLFSSH_API int wolfSSH_ReadCert_buffer(const byte* in, word32 inSz, byte** out, word32* outSz, const byte** outType, word32* outTypeSz, byte* flavor, void* heap);