From: Viktor Dukhovni <openssl-users@dukhovni.org>
Date: Tue, 13 Dec 2022 07:49:13 +0000 (+0100)
Subject: Fix type confusion in nc_match_single()
X-Git-Tag: openssl-3.2.0-alpha1~1341
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=748f478f814bc8e418542c68599ec7dbcbac97b2;p=thirdparty%2Fopenssl.git

Fix type confusion in nc_match_single()

This function assumes that if the "gen" is an OtherName, then the "base"
is a rfc822Name constraint. This assumption is not true in all cases.
If the end-entity certificate contains an OtherName SAN of any type besides
SmtpUtf8Mailbox and the CA certificate contains a name constraint of
OtherName (of any type), then "nc_email_eai" will be invoked, with the
OTHERNAME "base" being incorrectly interpreted as a ASN1_IA5STRING.

Reported by Corey Bonnell from Digicert.

CVE-2022-4203

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
---

diff --git a/crypto/x509/v3_ncons.c b/crypto/x509/v3_ncons.c
index 8c7aee32a2a..ba8141b8c1b 100644
--- a/crypto/x509/v3_ncons.c
+++ b/crypto/x509/v3_ncons.c
@@ -34,7 +34,8 @@ static int do_i2r_name_constraints(const X509V3_EXT_METHOD *method,
 static int print_nc_ipadd(BIO *bp, ASN1_OCTET_STRING *ip);
 
 static int nc_match(GENERAL_NAME *gen, NAME_CONSTRAINTS *nc);
-static int nc_match_single(GENERAL_NAME *sub, GENERAL_NAME *gen);
+static int nc_match_single(int effective_type, GENERAL_NAME *sub,
+                           GENERAL_NAME *gen);
 static int nc_dn(const X509_NAME *sub, const X509_NAME *nm);
 static int nc_dns(ASN1_IA5STRING *sub, ASN1_IA5STRING *dns);
 static int nc_email(ASN1_IA5STRING *sub, ASN1_IA5STRING *eml);
@@ -481,14 +482,17 @@ static int nc_match(GENERAL_NAME *gen, NAME_CONSTRAINTS *nc)
 {
     GENERAL_SUBTREE *sub;
     int i, r, match = 0;
+    int effective_type = gen->type;
+
     /*
      * We need to compare not gen->type field but an "effective" type because
      * the otherName field may contain EAI email address treated specially
      * according to RFC 8398, section 6
      */
-    int effective_type = ((gen->type == GEN_OTHERNAME) &&
-                          (OBJ_obj2nid(gen->d.otherName->type_id) ==
-                           NID_id_on_SmtpUTF8Mailbox)) ? GEN_EMAIL : gen->type;
+    if (effective_type == GEN_OTHERNAME &&
+        (OBJ_obj2nid(gen->d.otherName->type_id) == NID_id_on_SmtpUTF8Mailbox)) {
+        effective_type = GEN_EMAIL;
+    }
 
     /*
      * Permitted subtrees: if any subtrees exist of matching the type at
@@ -497,7 +501,10 @@ static int nc_match(GENERAL_NAME *gen, NAME_CONSTRAINTS *nc)
 
     for (i = 0; i < sk_GENERAL_SUBTREE_num(nc->permittedSubtrees); i++) {
         sub = sk_GENERAL_SUBTREE_value(nc->permittedSubtrees, i);
-        if (effective_type != sub->base->type)
+        if (effective_type != sub->base->type
+            || (effective_type == GEN_OTHERNAME &&
+                OBJ_cmp(gen->d.otherName->type_id,
+                        sub->base->d.otherName->type_id) != 0))
             continue;
         if (!nc_minmax_valid(sub))
             return X509_V_ERR_SUBTREE_MINMAX;
@@ -506,7 +513,7 @@ static int nc_match(GENERAL_NAME *gen, NAME_CONSTRAINTS *nc)
             continue;
         if (match == 0)
             match = 1;
-        r = nc_match_single(gen, sub->base);
+        r = nc_match_single(effective_type, gen, sub->base);
         if (r == X509_V_OK)
             match = 2;
         else if (r != X509_V_ERR_PERMITTED_VIOLATION)
@@ -520,12 +527,15 @@ static int nc_match(GENERAL_NAME *gen, NAME_CONSTRAINTS *nc)
 
     for (i = 0; i < sk_GENERAL_SUBTREE_num(nc->excludedSubtrees); i++) {
         sub = sk_GENERAL_SUBTREE_value(nc->excludedSubtrees, i);
-        if (effective_type != sub->base->type)
+        if (effective_type != sub->base->type
+            || (effective_type == GEN_OTHERNAME &&
+                OBJ_cmp(gen->d.otherName->type_id,
+                        sub->base->d.otherName->type_id) != 0))
             continue;
         if (!nc_minmax_valid(sub))
             return X509_V_ERR_SUBTREE_MINMAX;
 
-        r = nc_match_single(gen, sub->base);
+        r = nc_match_single(effective_type, gen, sub->base);
         if (r == X509_V_OK)
             return X509_V_ERR_EXCLUDED_VIOLATION;
         else if (r != X509_V_ERR_PERMITTED_VIOLATION)
@@ -537,15 +547,22 @@ static int nc_match(GENERAL_NAME *gen, NAME_CONSTRAINTS *nc)
 
 }
 
-static int nc_match_single(GENERAL_NAME *gen, GENERAL_NAME *base)
+static int nc_match_single(int effective_type, GENERAL_NAME *gen,
+                           GENERAL_NAME *base)
 {
     switch (gen->type) {
     case GEN_OTHERNAME:
-        /*
-         * We are here only when we have SmtpUTF8 name,
-         * so we match the value of othername with base->d.rfc822Name
-         */
-        return nc_email_eai(gen->d.otherName->value, base->d.rfc822Name);
+        switch (effective_type) {
+        case GEN_EMAIL:
+            /*
+             * We are here only when we have SmtpUTF8 name,
+             * so we match the value of othername with base->d.rfc822Name
+             */
+            return nc_email_eai(gen->d.otherName->value, base->d.rfc822Name);
+
+        default:
+            return X509_V_ERR_UNSUPPORTED_CONSTRAINT_TYPE;
+        }
 
     case GEN_DIRNAME:
         return nc_dn(gen->d.directoryName, base->d.directoryName);