From e1a2ba3b02e06fff68d1e13256f7d1bfb6ce99eb Mon Sep 17 00:00:00 2001 From: Eric Blankenhorn Date: Fri, 26 Jun 2026 11:11:22 -0500 Subject: [PATCH] Restore error code from DecodeGeneralName --- doc/dox_comments/header_files/ssl.h | 9 ++++ tests/api/test_asn.c | 25 +++++++++-- tests/api/test_ossl_x509.c | 67 +++++++++++++++++++++++++++++ tests/api/test_ossl_x509.h | 2 + wolfcrypt/src/asn.c | 33 +++----------- wolfcrypt/src/asn_orig.c | 29 +++---------- 6 files changed, 112 insertions(+), 53 deletions(-) diff --git a/doc/dox_comments/header_files/ssl.h b/doc/dox_comments/header_files/ssl.h index 1a56a4ae7c7..71c72b2debb 100644 --- a/doc/dox_comments/header_files/ssl.h +++ b/doc/dox_comments/header_files/ssl.h @@ -13859,6 +13859,14 @@ int wolfSSL_set_msg_callback_arg(WOLFSSL *ssl, void* arg); \param cert a pointer to the wolfSSL_X509 structure. + \warning The returned value is a bare C string with no length. A dNSName, + rfc822Name, or uniformResourceIdentifier SAN may legally contain an embedded + NUL byte (RFC 6125 Sec. 6.3 treats such a name as an invalid presented + identifier, not a malformed certificate, so the certificate still parses), + which silently truncates the returned string. Do not use strlen or strcmp on + the result for security comparisons; use wolfSSL_X509_get_ext_d2i and the + GENERAL_NAME ASN1_STRING length instead. + _Example_ \code WOLFSSL_X509 x509 = (WOLFSSL_X509*)XMALLOC(sizeof(WOLFSSL_X509), NULL, @@ -13872,6 +13880,7 @@ int wolfSSL_set_msg_callback_arg(WOLFSSL *ssl, void* arg); \sa wolfSSL_X509_get_issuer_name \sa wolfSSL_X509_get_subject_name + \sa wolfSSL_X509_get_ext_d2i */ char* wolfSSL_X509_get_next_altname(WOLFSSL_X509*); diff --git a/tests/api/test_asn.c b/tests/api/test_asn.c index bfa95626368..906a3f44691 100644 --- a/tests/api/test_asn.c +++ b/tests/api/test_asn.c @@ -1378,7 +1378,9 @@ int test_wc_DecodeRsaPssParams(void) } /* Test that DecodeAltNames rejects a SAN entry whose length exceeds the - * remaining SEQUENCE length (integer underflow on the length tracker). */ + * remaining SEQUENCE length (integer underflow on the length tracker), and + * that a dNSName SAN carrying an embedded NUL is stored rather than rejected + * so verification reports a name mismatch instead of a parse error. */ int test_DecodeAltNames_length_underflow(void) { EXPECT_DECLS; @@ -1481,14 +1483,29 @@ int test_DecodeAltNames_length_underflow(void) WC_NO_ERR_TRACE(ASN_PARSE_E)); wc_FreeDecodedCert(&cert); - /* NUL in dNSName SAN must be rejected per RFC 5280 4.2.1.6. */ + /* An embedded NUL in a dNSName SAN is an invalid presented identifier + * (RFC 6125 Sec. 6.3 / RFC 9525 Sec. 6.3), not a malformed certificate. + * Set the third byte of the 4-byte dNSName ("a*b*") to NUL, giving + * "a*\0*". The certificate must still parse: the entry is stored with + * its embedded NUL intact (length 4, not truncated) so that hostname + * verification reports DOMAIN_NAME_MISMATCH rather than the parser + * aborting with ASN_PARSE_E (regression from curl test 311). */ XMEMCPY(bad_san_cert, good_san_cert, sizeof(good_san_cert)); bad_san_cert[SAN_SEQ_LEN_OFFSET + 5] = 0x00; wc_InitDecodedCert(&cert, bad_san_cert, (word32)sizeof(bad_san_cert), NULL); - ExpectIntEQ(wc_ParseCert(&cert, CERT_TYPE, NO_VERIFY, NULL), - WC_NO_ERR_TRACE(ASN_PARSE_E)); + ExpectIntEQ(wc_ParseCert(&cert, CERT_TYPE, NO_VERIFY, NULL), 0); + ExpectNotNull(cert.altNames); + if (cert.altNames != NULL) { + ExpectIntEQ(cert.altNames->type, ASN_DNS_TYPE); + ExpectIntEQ(cert.altNames->len, 4); + /* Embedded NUL preserved at offset 2: stored, not truncated. */ + ExpectNotNull(cert.altNames->name); + if (cert.altNames->name != NULL) { + ExpectIntEQ(cert.altNames->name[2], 0x00); + } + } wc_FreeDecodedCert(&cert); #endif /* !NO_CERTS && !NO_RSA && !NO_ASN */ diff --git a/tests/api/test_ossl_x509.c b/tests/api/test_ossl_x509.c index 30e9f82b201..a41f93558ff 100644 --- a/tests/api/test_ossl_x509.c +++ b/tests/api/test_ossl_x509.c @@ -1795,6 +1795,73 @@ int test_wolfSSL_MatchDomainName_idn(void) return EXPECT_RESULT(); } +/* Regression test mirroring curl test 311 ("HTTPS wrong subjectAltName but + * right CN"). The leaf Subject CN is the wanted host ("localhost") while its + * only dNSName SAN carries an embedded NUL ("localhost\0h"). Such a SAN is an + * invalid presented identifier (RFC 6125 Sec. 6.3 / RFC 9525 Sec. 6.3), not a + * malformed certificate, so the certificate must parse. Verification must then + * report DOMAIN_NAME_MISMATCH: the SAN presence suppresses Subject CN fallback + * and the NUL name can never match the NUL-free reference host, so the + * otherwise-correct CN must not rescue it. Before the parser stored rather than + * rejected the entry, verification aborted earlier with ASN_PARSE_E. */ +int test_wolfSSL_X509_check_host_embedded_nul_san(void) +{ + EXPECT_DECLS; +#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && !defined(NO_RSA) && \ + defined(OPENSSL_EXTRA) && defined(WOLFSSL_CERT_GEN) && \ + defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_ALT_NAMES) && \ + !defined(NO_SHA256) && !defined(NO_ASN) + WOLFSSL_EVP_PKEY* priv = NULL; + WOLFSSL_X509* leaf = NULL; + const char* server_cert = "./certs/test/server-goodcn.pem"; + const char host[] = "localhost"; + /* dNSName "localhost" + embedded NUL + 'h', length 11. */ + static const byte nulSan[] = { + 'l', 'o', 'c', 'a', 'l', 'h', 'o', 's', 't', '\0', 'h' + }; + byte* keyPt = NULL; + const byte* der = NULL; + int derSz = 0; + DecodedCert dCert; + int parseRet = -1; + + keyPt = (byte*)server_key_der_2048; + ExpectNotNull(priv = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL, + (const unsigned char**)&keyPt, sizeof_server_key_der_2048)); + + /* server-goodcn.pem has CN=localhost and no SAN; add one dNSName SAN that + * carries an embedded NUL, then re-sign so the SAN is serialized. */ + ExpectNotNull(leaf = wolfSSL_X509_load_certificate_file(server_cert, + WOLFSSL_FILETYPE_PEM)); + ExpectIntEQ(wolfSSL_X509_add_altname_ex(leaf, (const char*)nulSan, + sizeof(nulSan), ASN_DNS_TYPE), WOLFSSL_SUCCESS); + ExpectIntGT(wolfSSL_X509_sign(leaf, priv, EVP_sha256()), 0); + + /* Regression pin: the signed certificate must parse despite the embedded + * NUL (it aborted with ASN_PARSE_E before the parser stored the entry). */ + ExpectNotNull(der = wolfSSL_X509_get_der(leaf, &derSz)); + ExpectIntGT(derSz, 0); + if ((der != NULL) && (derSz > 0)) { + wc_InitDecodedCert(&dCert, der, (word32)derSz, NULL); + parseRet = wc_ParseCert(&dCert, CERT_TYPE, NO_VERIFY, NULL); + ExpectIntEQ(parseRet, 0); + wc_FreeDecodedCert(&dCert); + } + + /* Security pin: the host must still be rejected. With parsing now + * succeeding, the only remaining failure path in check_host for a + * non-IP host is the hostname comparison, so a failure here means + * DOMAIN_NAME_MISMATCH: the SAN presence suppressed CN fallback and the + * NUL name did not match. The correct CN must not rescue the wrong SAN. */ + ExpectIntEQ(wolfSSL_X509_check_host(leaf, host, XSTRLEN(host), 0, NULL), + WC_NO_ERR_TRACE(WOLFSSL_FAILURE)); + + wolfSSL_X509_free(leaf); + wolfSSL_EVP_PKEY_free(priv); +#endif + return EXPECT_RESULT(); +} + int test_wolfSSL_X509_max_altnames(void) { EXPECT_DECLS; diff --git a/tests/api/test_ossl_x509.h b/tests/api/test_ossl_x509.h index f2844092af8..a4b06cfa96e 100644 --- a/tests/api/test_ossl_x509.h +++ b/tests/api/test_ossl_x509.h @@ -50,6 +50,7 @@ int test_wolfSSL_X509_name_match2(void); int test_wolfSSL_X509_name_match3(void); int test_wolfssl_local_IsValidFQDN(void); int test_wolfSSL_MatchDomainName_idn(void); +int test_wolfSSL_X509_check_host_embedded_nul_san(void); int test_wolfSSL_X509_max_altnames(void); int test_wolfSSL_X509_max_name_constraints(void); int test_wolfSSL_X509_check_ca(void); @@ -83,6 +84,7 @@ int test_wolfSSL_X509_cmp(void); TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_name_match3), \ TEST_DECL_GROUP("ossl_x509", test_wolfssl_local_IsValidFQDN), \ TEST_DECL_GROUP("ossl_x509", test_wolfSSL_MatchDomainName_idn), \ + TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_check_host_embedded_nul_san),\ TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_max_altnames), \ TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_max_name_constraints), \ TEST_DECL_GROUP("ossl_x509", test_wolfSSL_X509_check_ca), \ diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c index 7e855986f67..bbcb3288cb9 100644 --- a/wolfcrypt/src/asn.c +++ b/wolfcrypt/src/asn.c @@ -18900,31 +18900,20 @@ static int DecodeOtherName(DecodedCert* cert, const byte* input, * @return ASN_UNKNOWN_OID_E when the OID cannot be verified. * @return MEMORY_E when dynamic memory allocation fails. */ -/* Reject IA5String SAN content that cannot legally appear in - * dNSName / rfc822Name / URI per RFC 5280 4.2.1.6. Currently just NUL. */ -static int DecodeGeneralNameCheckChars(const byte* input, int len) -{ - int i; - for (i = 0; i < len; i++) { - if (input[i] == 0) { - return ASN_PARSE_E; - } - } - return 0; -} - static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag, int len, DecodedCert* cert) { int ret = 0; word32 idx = *inOutIdx; - /* GeneralName choice: dnsName */ + /* GeneralName choice: dnsName. + * An embedded NUL makes a dNSName an invalid presented identifier + * (RFC 6125 Sec. 6.3 / RFC 9525 Sec. 6.3), not a malformed certificate. + * Store it so its presence still suppresses Subject CN fallback, but + * length-based matching in MatchDomainName never matches a NUL-free + * reference hostname. The result is DOMAIN_NAME_MISMATCH at verification + * time rather than ASN_PARSE_E at parse time. */ if (tag == (ASN_CONTEXT_SPECIFIC | ASN_DNS_TYPE)) { - ret = DecodeGeneralNameCheckChars(input + idx, len); - if (ret != 0) { - return ret; - } ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len, ASN_DNS_TYPE, &cert->altNames); if (ret == 0) { @@ -18952,10 +18941,6 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag, } /* GeneralName choice: rfc822Name */ else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_RFC822_TYPE)) { - ret = DecodeGeneralNameCheckChars(input + idx, len); - if (ret != 0) { - return ret; - } ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len, ASN_RFC822_TYPE, &cert->altEmailNames); if (ret == 0) { @@ -18964,10 +18949,6 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag, } /* GeneralName choice: uniformResourceIdentifier */ else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_URI_TYPE)) { - ret = DecodeGeneralNameCheckChars(input + idx, len); - if (ret != 0) { - return ret; - } WOLFSSL_MSG("\tPutting URI into list but not using"); #ifndef WOLFSSL_NO_ASN_STRICT diff --git a/wolfcrypt/src/asn_orig.c b/wolfcrypt/src/asn_orig.c index 375da8eaf0b..747c699ac26 100644 --- a/wolfcrypt/src/asn_orig.c +++ b/wolfcrypt/src/asn_orig.c @@ -3125,19 +3125,6 @@ static int DecodeConstructedOtherName(DecodedCert* cert, const byte* input, return ret; } -/* Reject IA5String SAN content that cannot legally appear in - * dNSName / rfc822Name / URI per RFC 5280 4.2.1.6. Currently just NUL. */ -static int DecodeGeneralNameCheckChars(const byte* input, int len) -{ - int i; - for (i = 0; i < len; i++) { - if (input[i] == 0) { - return ASN_PARSE_E; - } - } - return 0; -} - static int DecodeAltNames(const byte* input, word32 sz, DecodedCert* cert) { word32 idx = 0; @@ -3200,9 +3187,12 @@ static int DecodeAltNames(const byte* input, word32 sz, DecodedCert* cert) if ((word32)strLen + idx > sz) { return BUFFER_E; } - if (DecodeGeneralNameCheckChars(&input[idx], strLen) != 0) { - return ASN_PARSE_E; - } + /* An embedded NUL makes a dNSName an invalid presented identifier + * (RFC 6125 Sec. 6.3), not a malformed certificate. Store it so + * its presence still suppresses Subject CN fallback; length-based + * matching in MatchDomainName never matches a NUL-free hostname, + * giving DOMAIN_NAME_MISMATCH at verification rather than + * ASN_PARSE_E at parse time. */ dnsEntry = AltNameNew(cert->heap); if (dnsEntry == NULL) { @@ -3292,9 +3282,6 @@ static int DecodeAltNames(const byte* input, word32 sz, DecodedCert* cert) if ((word32)strLen + idx > sz) { return BUFFER_E; } - if (DecodeGeneralNameCheckChars(&input[idx], strLen) != 0) { - return ASN_PARSE_E; - } emailEntry = AltNameNew(cert->heap); if (emailEntry == NULL) { @@ -3341,10 +3328,6 @@ static int DecodeAltNames(const byte* input, word32 sz, DecodedCert* cert) return BUFFER_E; } - if (DecodeGeneralNameCheckChars(&input[idx], strLen) != 0) { - return ASN_PARSE_E; - } - #ifndef WOLFSSL_NO_ASN_STRICT /* Verify RFC 5280 Sec 4.2.1.6 rule: "The name MUST NOT be a relative URI"