Restore error code from DecodeGeneralName

Author: embhorn

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*);
 

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 */

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;

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),                  \

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

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"