Merge pull request #10354 from embhorn/zd21725

Fix IPSAN and registeredID handling

Author: dgarske

certs/test/cert-ext-ncrid.der

@@ -0,0 +1,25 @@
+-----BEGIN CERTIFICATE-----
+MIIEOjCCAyKgAwIBAgIUBD6sBdFHNsQ5qoW3MIkJ/BKsB/4wDQYJKoZIhvcNAQEL
+BQAwezELMAkGA1UEBhMCQVUxEzARBgNVBAgMClF1ZWVuc2xhbmQxETAPBgNVBAcM
+CEJyaXNiYW5lMRQwEgYDVQQKDAt3b2xmU1NMIEluYzEUMBIGA1UECwwLRW5naW5l
+ZXJpbmcxGDAWBgNVBAMMD3d3dy53b2xmc3NsLmNvbTAeFw0yNjA0MjkyMTAzMTZa
+Fw0yOTAxMjMyMTAzMTZaMHsxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApRdWVlbnNs
+YW5kMREwDwYDVQQHDAhCcmlzYmFuZTEUMBIGA1UECgwLd29sZlNTTCBJbmMxFDAS
+BgNVBAsMC0VuZ2luZWVyaW5nMRgwFgYDVQQDDA93d3cud29sZnNzbC5jb20wggEi
+MA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDAlQjhV0HycW230kVBJwFlxkWu
+8rwkMLiVzi9O1vYciLx8n/uoZ3/+XJxRdfeKygfnNS+P4b17wC98q2SoF/zKXXu6
+4CHlci5vLobYlXParBtTuV8/1xkNJU/hY2NRiwtkP61DuKUcXDSzrgCgY8X2fwtZ
+aHhzpowYqQJtr8MZAS64EOPGzEC0aaNGM2mHbsS7F6bz6N2tc7x7LyG1/WZRDL1U
+s+FtXxy8I3PRCQOJFNIQuWTDKtChlkq84dQaW8egwMFjeA9ENzAyloAyI5Whd7oT
+0pdz4l0lyWoNwzlgpLSwaUJCCenYCLwzILNYIqeq68Th5mGDxdKW39nQT63XAgMB
+AAGjgbUwgbIwHQYDVR0OBBYEFLMRMsmSmITiyfjQO24DQsofDo48MB8GA1UdIwQY
+MBaAFLMRMsmSmITiyfjQO24DQsofDo48MBIGA1UdEwEB/wQIMAYBAf8CAQAwDgYD
+VR0PAQH/BAQDAgGGMBYGA1UdHgEB/wQMMAqgCDAGiAQqAwQFMDQGCWCGSAGG+EIB
+DQQnFiVUZXN0aW5nIHJlZ2lzdGVyZWRJRCBuYW1lIGNvbnN0cmFpbnRzMA0GCSqG
+SIb3DQEBCwUAA4IBAQARs/sdZs71KWRoyWuTevfnS5h5PH9S07xl+hFcfSXpI6sx
+gQoBMzEv8BBN3vLL4HOoNCwnqmFbwG5ogt/HuohAT6bXtdIkK1jNEP50t1pdDOmi
+/3fYKDvjvtX2DDQe2K8o/qVsu63MtKkOuc1ZURtsVrlrFFW7D7FsuNxa1dS8fbGl
+qrEnIXKemPmco5EIFDygXFIFkL/CP/LxHjTOuib68hBUDOqhMpMruSp7OfAt9Kfe
+rsEVY5mv82DfaCjo9bw17dSHUFo4vC+wzB5E8Wg33Pxbn0v8aRTvAsPz22Mlkd9b
+mxdLFh0mqPBj/UvWOK1xH0kXGsblNNAz5KeDceAZ
+-----END CERTIFICATE-----

certs/test/cert-ext-ncrid.pem

@@ -196,6 +196,35 @@ EOF
 gen_cert
 rm -f ./certs/test/cert-ext-ncip.cfg
 
+OUT=certs/test/cert-ext-ncrid
+KEYFILE=certs/test/cert-ext-ncrid-key.der
+CONFIG=certs/test/cert-ext-ncrid.cfg
+tee >$CONFIG <<EOF
+[ req ]
+distinguished_name = req_distinguished_name
+prompt             = no
+x509_extensions    = v3_ca
+
+[ req_distinguished_name ]
+C             = AU
+ST            = Queensland
+L             = Brisbane
+O             = wolfSSL Inc
+OU            = Engineering
+CN            = www.wolfssl.com
+
+[ v3_ca ]
+subjectKeyIdentifier = hash
+authorityKeyIdentifier = keyid:always,issuer
+basicConstraints = critical, CA:true, pathlen:0
+keyUsage = critical, digitalSignature, cRLSign, keyCertSign
+nameConstraints = critical,permitted;RID:1.2.3.4.5
+nsComment       = "Testing registeredID name constraints"
+
+EOF
+gen_cert
+rm -f ./certs/test/cert-ext-ncrid.cfg
+
 OUT=certs/test/cert-ext-ia
 KEYFILE=certs/test/cert-ext-ia-key.der
 CONFIG=certs/test/cert-ext-ia.cfg

certs/test/gen-ext-certs.sh

@@ -13,6 +13,8 @@ EXTRA_DIST += \
          certs/test/cert-ext-nc-combined.pem \
          certs/test/cert-ext-ncip.der \
          certs/test/cert-ext-ncip.pem \
+         certs/test/cert-ext-ncrid.der \
+         certs/test/cert-ext-ncrid.pem \
          certs/test/cert-ext-ncdns.der \
          certs/test/cert-ext-ncdns.pem \
          certs/test/cert-ext-ncmulti.der \

certs/test/include.am

@@ -13575,8 +13575,29 @@ 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. The same reasoning
+         * applies to registeredID entries: hostname matching against an
+         * OID is not meaningful, so they never count toward *checkCN. */
+        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
+            if (a->type == ASN_RID_TYPE)
+                continue;
+            *checkCN = 0;
+            break;
+        }
+    }
 
     for (; altName != NULL; altName = altName->next) {
         WOLFSSL_MSG("\tindividual AltName check");
@@ -13599,6 +13620,13 @@ int CheckForAltNames(DecodedCert* dCert, const char* domain, word32 domainLen,
             continue;
         }
 
+        /* registeredID entries hold raw OID bytes; they are not eligible
+         * for hostname matching regardless of build flags. */
+        if (altName->type == ASN_RID_TYPE) {
+            WOLFSSL_MSG("\tAltName is registeredID, skipping for hostname");
+            continue;
+        }
+
         if (MatchDomainName(buf, (int)len, domain, domainLen, flags)) {
             match = 1;
             if (checkCN != NULL) {

src/internal.c

@@ -650,8 +650,14 @@ static int DNS_to_GENERAL_NAME(WOLFSSL_GENERAL_NAME* gn, DNS_entry* dns)
             /* @TODO extract dir name info from DNS_entry */
             break;
 
-#ifdef WOLFSSL_RID_ALT_NAME
         case WOLFSSL_GEN_RID:
+            /* registeredID is parsed into altNames unconditionally so
+             * ConfirmNameConstraints can enforce RID name constraints
+             * (RFC 5280 Sec. 4.2.1.10). The body uses only the raw OID
+             * bytes carried in dns->name/dns->len and constructs a
+             * proper ASN1_OBJECT, so this case is independent of
+             * WOLFSSL_RID_ALT_NAME (which only gates the human-readable
+             * ridString form). */
             gn->type = dns->type;
             /* wolfSSL_GENERAL_NAME_new() mallocs this by default */
             wolfSSL_ASN1_STRING_free(gn->d.ia5);
@@ -681,7 +687,6 @@ static int DNS_to_GENERAL_NAME(WOLFSSL_GENERAL_NAME* gn, DNS_entry* dns)
             gn->d.registeredID->dynamic |= WOLFSSL_ASN1_DYNAMIC_DATA;
             gn->d.registeredID->grp = oidCertExtType;
             break;
-#endif
 
         case WOLFSSL_GEN_X400:
             /* Unsupported: fall through */
@@ -2533,8 +2538,12 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c,
                             gn->d.iPAddress->type = WOLFSSL_V_ASN1_OCTET_STRING;
                             break;
 
-                    #ifdef WOLFSSL_RID_ALT_NAME
                         case ASN_RID_TYPE:
+                            /* Always handle registeredID: the union
+                             * member d.registeredID is populated from
+                             * raw OID body bytes. WOLFSSL_RID_ALT_NAME
+                             * only gates the human-readable ridString,
+                             * which this path does not need. */
                             gn->type = dns->type;
                             /* Free ia5 before using union for registeredID */
                             wolfSSL_ASN1_STRING_free(gn->d.ia5);
@@ -2567,7 +2576,6 @@ void* wolfSSL_X509_get_ext_d2i(const WOLFSSL_X509* x509, int nid, int* c,
                                 WOLFSSL_ASN1_DYNAMIC_DATA;
                             gn->d.registeredID->grp = oidCertExtType;
                             break;
-                    #endif /* WOLFSSL_RID_ALT_NAME */
 
                         default:
                             gn->type = dns->type;
@@ -4272,13 +4280,49 @@ char* wolfSSL_X509_get_next_altname(WOLFSSL_X509* cert)
         return NULL;
     }
 
+    /* In default builds iPAddress entries hold raw 4/16 octet payloads
+     * and registeredID entries hold raw OID body bytes (no human-readable
+     * ipString/ridString), 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
+     * skipped 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. */
+#if !defined(WOLFSSL_IP_ALT_NAME) || !defined(WOLFSSL_RID_ALT_NAME)
+    while (cert->altNamesNext != NULL) {
+        int skip = 0;
+#ifndef WOLFSSL_IP_ALT_NAME
+        if (cert->altNamesNext->type == ASN_IP_TYPE)
+            skip = 1;
+#endif
+#ifndef WOLFSSL_RID_ALT_NAME
+        if (cert->altNamesNext->type == ASN_RID_TYPE)
+            skip = 1;
+#endif
+        if (!skip)
+            break;
+        cert->altNamesNext = cert->altNamesNext->next;
+    }
+    if (cert->altNamesNext == NULL)
+        return NULL;
+#endif /* !WOLFSSL_IP_ALT_NAME || !WOLFSSL_RID_ALT_NAME */
+
     /* unsafe cast required for ABI compatibility. */
     ret = (char *)(wc_ptr_t)cert->altNamesNext->name;
 #ifdef WOLFSSL_IP_ALT_NAME
     /* return the IP address as a string */
     if (cert->altNamesNext->type == ASN_IP_TYPE) {
         ret = cert->altNamesNext->ipString;
     }
+#endif
+#ifdef WOLFSSL_RID_ALT_NAME
+    /* return the registeredID as a string */
+    if (cert->altNamesNext->type == ASN_RID_TYPE) {
+        ret = cert->altNamesNext->ridString;
+    }
 #endif
     cert->altNamesNext = cert->altNamesNext->next;
 
@@ -6909,6 +6953,14 @@ static int X509_print_name_entry(WOLFSSL_BIO* bio,
             len = XSNPRINTF(scratch, MAX_WIDTH, "IP Address:%s",
                     entry->ipString);
         }
+    #else
+        else if (entry->type == ASN_IP_TYPE) {
+            /* iPAddress entries are now always parsed into altNames so
+             * name constraints can be enforced. Without the
+             * human-readable ipString field, emit a fixed label so this
+             * print path does not fail. */
+            len = XSNPRINTF(scratch, MAX_WIDTH, "IP Address:<unavailable>");
+        }
     #endif /* OPENSSL_ALL || WOLFSSL_IP_ALT_NAME */
         else if (entry->type == ASN_RFC822_TYPE) {
             len = XSNPRINTF(scratch, MAX_WIDTH, "email:%s",
@@ -6921,12 +6973,20 @@ static int X509_print_name_entry(WOLFSSL_BIO* bio,
             len = XSNPRINTF(scratch, MAX_WIDTH, "URI:%s",
                 entry->name);
         }
-    #if defined(OPENSSL_ALL)
+    #ifdef WOLFSSL_RID_ALT_NAME
         else if (entry->type == ASN_RID_TYPE) {
             len = XSNPRINTF(scratch, MAX_WIDTH, "Registered ID:%s",
                 entry->ridString);
         }
-    #endif
+    #else
+        else if (entry->type == ASN_RID_TYPE) {
+            /* registeredID entries are now always parsed into altNames
+             * so name constraints can be enforced. Without the
+             * human-readable ridString field, emit a fixed label so
+             * this print path does not fail. */
+            len = XSNPRINTF(scratch, MAX_WIDTH, "Registered ID:<unavailable>");
+        }
+    #endif /* WOLFSSL_RID_ALT_NAME */
         else if (entry->type == ASN_OTHER_TYPE) {
             len = XSNPRINTF(scratch, MAX_WIDTH,
                 "othername <unsupported>");

src/x509.c

@@ -24006,11 +24006,25 @@ static word32 build_otherName_san(byte* out, word32 outSz, const char* val7)
     return (word32)(sizeof(prefix) + 7);
 }
 
+/* Build a SubjectAltName extension value with a single registeredID
+ * GeneralName for OID 1.2.3.4, to use as the leaf SAN in RID tests. */
+static word32 build_registeredID_san(byte* out, word32 outSz)
+{
+    static const byte ridSan[] = {
+        0x30, 0x05,                                     /* SEQUENCE, 5 */
+        0x88, 0x03,                                     /* [8] regId, 3 */
+            0x2A, 0x03, 0x04                            /* OID 1.2.3.4 */
+    };
+    if (outSz < sizeof(ridSan))
+        return 0;
+    XMEMCPY(out, ridSan, sizeof(ridSan));
+    return (word32)sizeof(ridSan);
+}
+
 /* Build a NameConstraints extension value with a single excludedSubtree
- * carrying a registeredID GeneralName for OID 1.2.3.4. registeredID is a
- * GeneralName form wolfSSL does not enforce, so DecodeSubtree() must
- * record it as 'unsupported' and ConfirmNameConstraints() must fail
- * closed when the extension is critical (RFC 5280 4.2.1.10). */
+ * carrying a registeredID GeneralName for OID 1.2.3.4. wolfSSL enforces
+ * registeredID name constraints by byte-comparing OID bodies, so a leaf
+ * whose registeredID SAN matches this exclusion must be rejected. */
 static word32 build_registeredID_nameConstraints(byte* out, word32 outSz)
 {
     static const byte ridNc[] = {
@@ -24187,10 +24201,12 @@ static int verify_with_otherName_chain(const byte* nameConstraintsDer,
  *      (excluded is enforced regardless of criticality)
  *   4. Critical permitted subtree, leaf SAN matches           -> accept
  *   5. Critical permitted subtree, leaf SAN does NOT match    -> reject
- *   6. Critical nameConstraints carrying an unsupported form
- *      (registeredID), leaf has no relevant SAN              -> reject
- *      (RFC 5280 4.2.1.10 fail-closed for unprocessed forms)
- *   7. Same as (6) but non-critical                           -> accept
+ *   6. Critical excluded registeredID subtree, leaf SAN matches -> reject
+ *      (registeredID is enforced by byte-comparison of OID bodies)
+ *   7. Non-critical excluded registeredID subtree, leaf SAN matches -> reject
+ *      (excluded subtrees are enforced regardless of criticality)
+ *   8. Critical excluded registeredID subtree, leaf has no RID SAN -> accept
+ *      (no match in excluded list, constraint is satisfied)
  */
 static int test_NameConstraints_OtherName(void)
 {
@@ -24203,16 +24219,19 @@ static int test_NameConstraints_OtherName(void)
     defined(HAVE_OID_ENCODING) && !defined(IGNORE_NAME_CONSTRAINTS)
     byte sanBlocked[64];
     byte sanAllowed[64];
+    byte sanRegisteredID[16];
     byte ncExcludedBlocked[64];
     byte ncPermittedAllowed[64];
     byte ncRegisteredID[16];
-    word32 sanBlockedSz, sanAllowedSz;
+    word32 sanBlockedSz, sanAllowedSz, sanRegisteredIDSz;
     word32 ncExcludedBlockedSz, ncPermittedAllowedSz, ncRegisteredIDSz;
 
     sanBlockedSz =
         build_otherName_san(sanBlocked, sizeof(sanBlocked), "blocked");
     sanAllowedSz =
         build_otherName_san(sanAllowed, sizeof(sanAllowed), "allowed");
+    sanRegisteredIDSz =
+        build_registeredID_san(sanRegisteredID, sizeof(sanRegisteredID));
     ncExcludedBlockedSz = build_otherName_nameConstraints(
         ncExcludedBlocked, sizeof(ncExcludedBlocked), 1, "blocked");
     ncPermittedAllowedSz = build_otherName_nameConstraints(
@@ -24221,6 +24240,7 @@ static int test_NameConstraints_OtherName(void)
         ncRegisteredID, sizeof(ncRegisteredID));
     ExpectIntGT((int)sanBlockedSz, 0);
     ExpectIntGT((int)sanAllowedSz, 0);
+    ExpectIntGT((int)sanRegisteredIDSz, 0);
     ExpectIntGT((int)ncExcludedBlockedSz, 0);
     ExpectIntGT((int)ncPermittedAllowedSz, 0);
     ExpectIntGT((int)ncRegisteredIDSz, 0);
@@ -24263,20 +24283,25 @@ static int test_NameConstraints_OtherName(void)
             sanBlocked, sanBlockedSz),
         WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
 
-    /* (6) Critical nameConstraints carrying a GeneralName form wolfSSL
-     *     does not enforce (registeredID). RFC 5280 4.2.1.10 requires the
-     *     verifier to either process the constraint or reject; we reject
-     *     fail-closed. The leaf needs no SAN to exercise this path. */
+    /* (6) Critical excluded registeredID subtree, leaf SAN matches:
+     *     registeredID is now enforced by byte-comparing OID bodies, so a
+     *     matching excluded entry must be rejected. */
     ExpectIntEQ(verify_with_otherName_chain(
-            ncRegisteredID, ncRegisteredIDSz, 1, NULL, 0),
+            ncRegisteredID, ncRegisteredIDSz, 1,
+            sanRegisteredID, sanRegisteredIDSz),
+        WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
+
+    /* (7) Non-critical excluded registeredID subtree, leaf SAN matches:
+     *     excluded subtrees are enforced regardless of criticality. */
+    ExpectIntEQ(verify_with_otherName_chain(
+            ncRegisteredID, ncRegisteredIDSz, 0,
+            sanRegisteredID, sanRegisteredIDSz),
         WC_NO_ERR_TRACE(ASN_NAME_INVALID_E));
 
-    /* (7) Same as (6) but non-critical: RFC 5280 only mandates the
-     *     fail-closed reject when the extension is critical, so a
-     *     non-critical unsupported constraint form is silently ignored
-     *     and verification succeeds. */
+    /* (8) Critical excluded registeredID subtree, leaf has no RID SAN:
+     *     no excluded match, so verification succeeds. */
     ExpectIntEQ(verify_with_otherName_chain(
-            ncRegisteredID, ncRegisteredIDSz, 0, NULL, 0),
+            ncRegisteredID, ncRegisteredIDSz, 1, NULL, 0),
         0);
 #endif
     return EXPECT_RESULT();
@@ -26733,11 +26758,25 @@ static int test_wolfSSL_X509_print(void)
       /* Will print IP address subject alt name. */
      ExpectIntEQ(BIO_get_mem_data(bio, NULL), 3350);
   #endif
-#elif defined(NO_ASN_TIME)
-    /* With NO_ASN_TIME defined, X509_print skips printing Validity. */
+#elif defined(IGNORE_NAME_CONSTRAINTS)
+    /* DecodeGeneralName skips iPAddress entries when name constraints
+     * are disabled, so the IP SAN never reaches the print path. */
+  #if defined(NO_ASN_TIME)
     ExpectIntEQ(BIO_get_mem_data(bio, NULL), 3213);
-#else
+  #else
     ExpectIntEQ(BIO_get_mem_data(bio, NULL), 3328);
+  #endif
+#elif defined(NO_ASN_TIME)
+    /* With NO_ASN_TIME defined, X509_print skips printing Validity.
+     * iPAddress SAN now always parsed; prints as
+     * "IP Address:<unavailable>" (+26 bytes) without
+     * WOLFSSL_IP_ALT_NAME. */
+    ExpectIntEQ(BIO_get_mem_data(bio, NULL), 3239);
+#else
+    /* iPAddress SAN now always parsed; prints as
+     * "IP Address:<unavailable>" (+26 bytes) without
+     * WOLFSSL_IP_ALT_NAME. */
+    ExpectIntEQ(BIO_get_mem_data(bio, NULL), 3354);
 #endif
     BIO_free(bio);
     bio = NULL;