Fix in CheckForAltNames to handle IPSAN

embhorn · embhorn · commit 48b580cac9db · 2026-04-30T08:59:18.000-05:00
diff --git a/src/internal.c b/src/internal.c
@@ -13463,8 +13463,25 @@ int CheckForAltNames(DecodedCert* dCert, const char* domain, word32 domainLen,
     if (dCert != NULL)
         altName = dCert->altNames;
 
-    if (checkCN != NULL)
-        *checkCN = (altName == NULL) ? 1 : 0;
+    if (checkCN != NULL) {
+        /* CN fallback is suppressed when the cert presents any altName
+         * usable for hostname matching. Without WOLFSSL_IP_ALT_NAME the
+         * iPAddress branch below is compiled out, so iPAddress entries
+         * cannot match anything here; treat them as absent so a cert
+         * presenting only iPAddress SANs still falls back to CN as it
+         * did before iPAddress entries were unconditionally added to
+         * altNames for name-constraint enforcement. */
+        DNS_entry* a = altName;
+        *checkCN = 1;
+        for (; a != NULL; a = a->next) {
+#ifndef WOLFSSL_IP_ALT_NAME
+            if (a->type == ASN_IP_TYPE)
+                continue;
+#endif
+            *checkCN = 0;
+            break;
+        }
+    }
 
     for (; altName != NULL; altName = altName->next) {
         WOLFSSL_MSG("\tindividual AltName check");
diff --git a/src/x509.c b/src/x509.c
@@ -4262,6 +4262,25 @@ char* wolfSSL_X509_get_next_altname(WOLFSSL_X509* cert)
         return NULL;
     }
 
+#ifndef WOLFSSL_IP_ALT_NAME
+    /* In default builds iPAddress entries hold raw 4/16 octet payloads
+     * (no human-readable ipString), so returning them as a C string would
+     * truncate at any embedded NUL byte. Such entries are still parsed
+     * into altNames for name-constraint enforcement; skip them here so
+     * string-iteration callers see the same set of entries as before.
+     *
+     * With WOLFSSL_MULTICIRCULATE_ALTNAMELIST, a list consisting only of
+     * iPAddress entries collapses to "no entries" on the first pass and
+     * resets to head on the next call; the cycle shape matches the
+     * pre-fix behavior where such entries were never parsed. */
+    while (cert->altNamesNext != NULL &&
+           cert->altNamesNext->type == ASN_IP_TYPE) {
+        cert->altNamesNext = cert->altNamesNext->next;
+    }
+    if (cert->altNamesNext == NULL)
+        return NULL;
+#endif
+
     /* unsafe cast required for ABI compatibility. */
     ret = (char *)(wc_ptr_t)cert->altNamesNext->name;
 #ifdef WOLFSSL_IP_ALT_NAME
diff --git a/tests/api/test_certman.c b/tests/api/test_certman.c
@@ -1705,6 +1705,200 @@ int test_wolfSSL_CertManagerNameConstraint_DNS_CN(void)
     return EXPECT_RESULT();
 }
 
+int test_wolfSSL_CertManagerNameConstraint_IP_SAN(void)
+{
+    EXPECT_DECLS;
+#if !defined(NO_FILESYSTEM) && !defined(NO_CERTS) && \
+    !defined(NO_WOLFSSL_CM_VERIFY) && !defined(NO_RSA) && \
+    defined(OPENSSL_EXTRA) && defined(WOLFSSL_CERT_GEN) && \
+    defined(WOLFSSL_CERT_EXT) && defined(WOLFSSL_ALT_NAMES) && \
+    !defined(NO_SHA256) && !defined(IGNORE_NAME_CONSTRAINTS)
+    /* Regression test for TALOS-2026-2409 (CVE-2026-28739).
+     *
+     * The CA at cert-ext-ncip.der declares a permittedSubtrees iPAddress
+     * constraint of 192.168.1.0/255.255.255.0. A leaf with an iPAddress
+     * SAN outside that subnet must be rejected. Prior to the fix, default
+     * builds (without WOLFSSL_IP_ALT_NAME) silently skipped iPAddress SANs
+     * during parsing, so the constraint loop saw no IP entries and the
+     * leaf was accepted.
+     *
+     * The bypass only existed when WOLFSSL_IP_ALT_NAME was undefined (the
+     * default). To exercise the regression target, this test must run in a
+     * configuration without --enable-ip-alt-name and without
+     * --enable-opensslall (which implies WOLFSSL_IP_ALT_NAME via
+     * settings.h). With WOLFSSL_IP_ALT_NAME defined the same assertions
+     * still hold, but the negative case there is enforcement of an
+     * already-working path rather than the regression itself.
+     *
+     * Scope: this test exercises the permittedSubtrees code path. The
+     * excludedSubtrees path uses the same parsing plumbing
+     * (DecodeGeneralName -> SetDNSEntry into cert->altNames) and the same
+     * ConfirmNameConstraints walk; the TALOS bug was strictly about
+     * iPAddress entries being absent from cert->altNames, so once that is
+     * fixed both directions are restored. The pre-existing
+     * test_wolfSSL_NAME_CONSTRAINTS_excluded test exercises the excluded
+     * direction more broadly. */
+    WOLFSSL_CERT_MANAGER* cm = NULL;
+    WOLFSSL_EVP_PKEY *priv = NULL;
+    WOLFSSL_X509_NAME* name = NULL;
+    const char* ca_cert = "./certs/test/cert-ext-ncip.der";
+    const char* server_cert = "./certs/test/server-goodcn.pem";
+    /* Raw IPv4 bytes for SAN values (not dotted-quad strings). */
+    static const byte ip_inside[]  = { 192, 168, 1, 10 };  /* permitted */
+    static const byte ip_outside[] = {  10,   0, 0,  1 };  /* violates */
+
+    byte    *der = NULL;
+    int     derSz;
+    byte    *pt;
+    WOLFSSL_X509 *x509 = NULL;
+    WOLFSSL_X509 *ca = NULL;
+
+    pt = (byte*)server_key_der_2048;
+    ExpectNotNull(priv = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL,
+                (const unsigned char**)&pt, sizeof_server_key_der_2048));
+
+    ExpectNotNull(cm = wolfSSL_CertManagerNew());
+    ExpectNotNull(ca = wolfSSL_X509_load_certificate_file(ca_cert,
+                WOLFSSL_FILETYPE_ASN1));
+    ExpectNotNull((der = (byte*)wolfSSL_X509_get_der(ca, &derSz)));
+    ExpectIntEQ(wolfSSL_CertManagerLoadCABuffer(cm, der, derSz,
+                WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
+
+    /* Negative case: leaf with IP SAN outside permitted subnet. Must be
+     * rejected with ASN_NAME_INVALID_E. */
+    ExpectNotNull(x509 = wolfSSL_X509_load_certificate_file(server_cert,
+                WOLFSSL_FILETYPE_PEM));
+    ExpectNotNull(name = wolfSSL_X509_get_subject_name(ca));
+    ExpectIntEQ(wolfSSL_X509_set_issuer_name(x509, name), WOLFSSL_SUCCESS);
+    name = NULL;
+
+    /* Use add_altname_ex with raw IP bytes so the test runs in default
+     * builds where add_altname (string form) requires WOLFSSL_IP_ALT_NAME. */
+    ExpectIntEQ(wolfSSL_X509_add_altname_ex(x509, (const char*)ip_outside,
+                sizeof(ip_outside), ASN_IP_TYPE), WOLFSSL_SUCCESS);
+    ExpectIntGT(wolfSSL_X509_sign(x509, priv, EVP_sha256()), 0);
+    ExpectNotNull((der = (byte*)wolfSSL_X509_get_der(x509, &derSz)));
+    ExpectIntEQ(wolfSSL_CertManagerVerifyBuffer(cm, der, derSz,
+                WOLFSSL_FILETYPE_ASN1), WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
+    wolfSSL_X509_free(x509);
+    x509 = NULL;
+
+    /* Positive case: leaf with IP SAN inside the permitted subnet must be
+     * accepted. Confirms the fix does not over-reject. */
+    ExpectNotNull(x509 = wolfSSL_X509_load_certificate_file(server_cert,
+                WOLFSSL_FILETYPE_PEM));
+    ExpectNotNull(name = wolfSSL_X509_get_subject_name(ca));
+    ExpectIntEQ(wolfSSL_X509_set_issuer_name(x509, name), WOLFSSL_SUCCESS);
+    name = NULL;
+
+    ExpectIntEQ(wolfSSL_X509_add_altname_ex(x509, (const char*)ip_inside,
+                sizeof(ip_inside), ASN_IP_TYPE), WOLFSSL_SUCCESS);
+    ExpectIntGT(wolfSSL_X509_sign(x509, priv, EVP_sha256()), 0);
+    ExpectNotNull((der = (byte*)wolfSSL_X509_get_der(x509, &derSz)));
+    ExpectIntEQ(wolfSSL_CertManagerVerifyBuffer(cm, der, derSz,
+                WOLFSSL_FILETYPE_ASN1), WOLFSSL_SUCCESS);
+
+    wolfSSL_CertManagerFree(cm);
+    wolfSSL_X509_free(x509);
+    wolfSSL_X509_free(ca);
+    wolfSSL_EVP_PKEY_free(priv);
+#endif
+    return EXPECT_RESULT();
+}
+
+int test_wolfSSL_X509_check_host_IP_only_SAN_CN_fallback(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)
+    /* Companion regression test for the CheckForAltNames CN-fallback
+     * preservation introduced alongside TALOS-2026-2409.
+     *
+     * Once iPAddress SAN entries are unconditionally added to altNames
+     * (so name constraints can be enforced), a leaf that presents only
+     * iPAddress SANs would suppress CN fallback in CheckForAltNames in
+     * default builds, where the iPAddress matching path is compiled out.
+     * That would silently break TLS hostname verification for callers
+     * that previously relied on the CN fallback. The fix in
+     * src/internal.c treats iPAddress entries as absent for the
+     * *checkCN decision when WOLFSSL_IP_ALT_NAME is undefined.
+     *
+     * This test pins both directions:
+     *   - default build (no WOLFSSL_IP_ALT_NAME): IP-only-SAN cert with a
+     *     matching CN must succeed via CN fallback.
+     *   - WOLFSSL_IP_ALT_NAME defined: the same cert must fail because
+     *     the SAN presence suppresses CN fallback (RFC 6125 compliant).
+     * Independently, a cert with a non-matching DNS SAN must always fail
+     * regardless of build flags, since DNS SAN presence unambiguously
+     * suppresses CN fallback. */
+    WOLFSSL_EVP_PKEY *priv = NULL;
+    WOLFSSL_X509_NAME* name = NULL;
+    const char* server_cert = "./certs/test/server-goodcn.pem";
+    const char hostName[] = "cnhost.local";
+    static const byte ip_san[] = { 10, 0, 0, 1 };
+    byte    *pt;
+    WOLFSSL_X509 *leafIp = NULL;
+    WOLFSSL_X509 *leafDns = NULL;
+
+    pt = (byte*)server_key_der_2048;
+    ExpectNotNull(priv = wolfSSL_d2i_PrivateKey(EVP_PKEY_RSA, NULL,
+                (const unsigned char**)&pt, sizeof_server_key_der_2048));
+
+    /* Leaf with CN matching hostName and only an iPAddress SAN. */
+    ExpectNotNull(leafIp = wolfSSL_X509_load_certificate_file(server_cert,
+                WOLFSSL_FILETYPE_PEM));
+    ExpectNotNull(name = X509_NAME_new());
+    ExpectIntEQ(X509_NAME_add_entry_by_txt(name, "commonName", MBSTRING_UTF8,
+                (byte*)hostName, (int)XSTRLEN(hostName), -1, 0), SSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_X509_set_subject_name(leafIp, name), WOLFSSL_SUCCESS);
+    X509_NAME_free(name);
+    name = NULL;
+    ExpectIntEQ(wolfSSL_X509_add_altname_ex(leafIp, (const char*)ip_san,
+                sizeof(ip_san), ASN_IP_TYPE), WOLFSSL_SUCCESS);
+    ExpectIntGT(wolfSSL_X509_sign(leafIp, priv, EVP_sha256()), 0);
+
+#ifndef WOLFSSL_IP_ALT_NAME
+    /* Default build: iPAddress entries are present in altNames for
+     * constraint enforcement but treated as absent for *checkCN, so the
+     * lookup falls back to the Subject CN, which matches. */
+    ExpectIntEQ(wolfSSL_X509_check_host(leafIp, hostName, XSTRLEN(hostName),
+                0, NULL), WOLFSSL_SUCCESS);
+#else
+    /* IP_ALT_NAME build: SAN presence suppresses CN fallback per RFC 6125.
+     * The hostName ("cnhost.local") cannot match the iPAddress entry, so
+     * the check must fail. */
+    ExpectIntEQ(wolfSSL_X509_check_host(leafIp, hostName, XSTRLEN(hostName),
+                0, NULL), WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
+#endif
+
+    /* Leaf with CN matching hostName but a non-matching DNS SAN. CN
+     * fallback must be suppressed in every build (DNS SAN unambiguously
+     * counts toward *checkCN), so the check must fail. This pins the
+     * other side of the boundary so a future change that broadly skips
+     * altNames in *checkCN does not silently regress. */
+    ExpectNotNull(leafDns = wolfSSL_X509_load_certificate_file(server_cert,
+                WOLFSSL_FILETYPE_PEM));
+    ExpectNotNull(name = X509_NAME_new());
+    ExpectIntEQ(X509_NAME_add_entry_by_txt(name, "commonName", MBSTRING_UTF8,
+                (byte*)hostName, (int)XSTRLEN(hostName), -1, 0), SSL_SUCCESS);
+    ExpectIntEQ(wolfSSL_X509_set_subject_name(leafDns, name), WOLFSSL_SUCCESS);
+    X509_NAME_free(name);
+    name = NULL;
+    ExpectIntEQ(wolfSSL_X509_add_altname(leafDns, "other.example",
+                ASN_DNS_TYPE), WOLFSSL_SUCCESS);
+    ExpectIntGT(wolfSSL_X509_sign(leafDns, priv, EVP_sha256()), 0);
+    ExpectIntEQ(wolfSSL_X509_check_host(leafDns, hostName, XSTRLEN(hostName),
+                0, NULL), WC_NO_ERR_TRACE(WOLFSSL_FAILURE));
+
+    wolfSSL_X509_free(leafIp);
+    wolfSSL_X509_free(leafDns);
+    wolfSSL_EVP_PKEY_free(priv);
+#endif
+    return EXPECT_RESULT();
+}
+
 int test_wolfSSL_CertManagerCRL(void)
 {
     EXPECT_DECLS;
diff --git a/tests/api/test_certman.h b/tests/api/test_certman.h
@@ -36,6 +36,8 @@ int test_wolfSSL_CertManagerNameConstraint3(void);
 int test_wolfSSL_CertManagerNameConstraint4(void);
 int test_wolfSSL_CertManagerNameConstraint5(void);
 int test_wolfSSL_CertManagerNameConstraint_DNS_CN(void);
+int test_wolfSSL_CertManagerNameConstraint_IP_SAN(void);
+int test_wolfSSL_X509_check_host_IP_only_SAN_CN_fallback(void);
 int test_wolfSSL_CertManagerCRL(void);
 int test_wolfSSL_CRL_reason_extensions_cleanup(void);
 int test_wolfSSL_CRL_static_revoked_list(void);
@@ -59,6 +61,9 @@ int test_wolfSSL_CertManagerRejectMD5Cert(void);
     TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerNameConstraint4),    \
     TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerNameConstraint5),    \
     TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerNameConstraint_DNS_CN), \
+    TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerNameConstraint_IP_SAN), \
+    TEST_DECL_GROUP("certman",                                              \
+        test_wolfSSL_X509_check_host_IP_only_SAN_CN_fallback),              \
     TEST_DECL_GROUP("certman", test_wolfSSL_CertManagerCRL),                \
     TEST_DECL_GROUP("certman", test_wolfSSL_CRL_reason_extensions_cleanup), \
     TEST_DECL_GROUP("certman", test_wolfSSL_CRL_static_revoked_list),      \
diff --git a/wolfcrypt/src/asn.c b/wolfcrypt/src/asn.c
@@ -18115,16 +18115,39 @@ static int DecodeGeneralName(const byte* input, word32* inOutIdx, byte tag,
             idx += (word32)len;
         }
     }
-    #ifdef WOLFSSL_IP_ALT_NAME
-    /* GeneralName choice: iPAddress */
+    /* GeneralName choice: iPAddress
+     *
+     * Always parse iPAddress into cert->altNames so ConfirmNameConstraints
+     * can enforce permitted/excluded iPAddress subtrees (RFC 5280
+     * Sec. 4.2.1.10). The entry holds raw 4/16 octet payloads;
+     * WOLFSSL_IP_ALT_NAME still gates the human-readable ipString
+     * generation in SetDNSEntry.
+     *
+     * Consequences for downstream consumers when WOLFSSL_IP_ALT_NAME is
+     * undefined:
+     *   - wolfSSL_X509_get_next_altname (string iterator): explicitly
+     *     skips iPAddress entries, since returning raw bytes as a C
+     *     string would truncate at any embedded NUL. This preserves the
+     *     pre-fix behavior for that getter.
+     *   - CheckForAltNames (TLS hostname matching): the iPAddress branch
+     *     is compiled out, so iPAddress entries cannot match anything;
+     *     they are also excluded from the *checkCN decision so an
+     *     IP-only-SAN cert still falls back to CN matching as before.
+     *   - All other altNames walkers (e.g. ALT_NAMES_OID handling in
+     *     wolfSSL_X509_get_ext_d2i, wolfssl_x509_alt_names_to_gn,
+     *     FlattenAltNames in cert generation) now see iPAddress entries
+     *     unconditionally. This is intentional and brings wolfSSL closer
+     *     to OpenSSL's SAN-exposure semantics; the OPENSSL_EXTRA APIs
+     *     surface the raw octets as OCTET_STRING already (see the
+     *     ASN_IP_TYPE case under WOLFSSL_GEN_IPADD in src/x509.c).
+     */
     else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_IP_TYPE)) {
         ret = SetDNSEntry(cert->heap, (const char*)(input + idx), len,
                 ASN_IP_TYPE, &cert->altNames);
         if (ret == 0) {
             idx += (word32)len;
         }
     }
-    #endif /* WOLFSSL_IP_ALT_NAME */
     #ifdef WOLFSSL_RID_ALT_NAME
     /* GeneralName choice: registeredID */
     else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_RID_TYPE)) {
@@ -37018,15 +37041,23 @@ static int DecodeAcertGeneralName(const byte* input, word32* inOutIdx,
     }
     #if defined(WOLFSSL_QT) || defined(OPENSSL_ALL) || \
                                             defined(WOLFSSL_IP_ALT_NAME)
-    /* GeneralName choice: iPAddress */
+    /* GeneralName choice: iPAddress
+     *
+     * Asymmetric with the X.509 DecodeGeneralName path on purpose:
+     * attribute-certificate names (RFC 5755) are not consumed by
+     * ConfirmNameConstraints, which only walks DecodedCert lists. These
+     * entries flow into AC holder/issuer name fields where the iPAddress
+     * is only consumed by callers that opt in (Qt, OpenSSL_ALL, or the
+     * IP-SAN compat layer). If iPAddress name-constraint enforcement is
+     * ever extended to attribute certificates, this gate must drop. */
     else if (tag == (ASN_CONTEXT_SPECIFIC | ASN_IP_TYPE)) {
         ret = SetDNSEntry(acert->heap, (const char*)(input + idx), len,
                           ASN_IP_TYPE, entries);
         if (ret == 0) {
             idx += (word32)len;
         }
     }
-    #endif /* WOLFSSL_QT || OPENSSL_ALL */
+    #endif /* WOLFSSL_QT || OPENSSL_ALL || WOLFSSL_IP_ALT_NAME */
 
     #ifdef OPENSSL_ALL
     /* GeneralName choice: registeredID */
