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: Hugo Landau <hlandau@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org>
openssl · Feb 2, 2023 · c927a34 · c927a34
1 parent 36d85b0
commit c927a34
Showing 1 changed file with 31 additions and 14 deletions.
diff --git a/crypto/x509/v3_ncons.c b/crypto/x509/v3_ncons.c
@@ -31,7 +31,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);
@@ -472,14 +473,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
@@ -488,7 +492,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;
@@ -497,7 +504,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)
@@ -511,12 +518,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)
@@ -528,15 +538,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);