Ensure the X.509 Subject only contains commonName and optionally serialNumber
authorjob <job@openbsd.org>
Tue, 12 Sep 2023 09:33:30 +0000 (09:33 +0000)
committerjob <job@openbsd.org>
Tue, 12 Sep 2023 09:33:30 +0000 (09:33 +0000)
OK tb@

usr.sbin/rpki-client/cert.c
usr.sbin/rpki-client/extern.h
usr.sbin/rpki-client/x509.c

index c1b12b1..c123640 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: cert.c,v 1.114 2023/06/29 10:28:25 tb Exp $ */
+/*     $OpenBSD: cert.c,v 1.115 2023/09/12 09:33:30 job Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Job Snijders <job@openbsd.org>
@@ -594,9 +594,8 @@ certificate_policies(struct parse *p, X509_EXTENSION *ext)
 }
 
 /*
- * Lightweight version of cert_parse_pre() for ASPA, ROA, and RSC EE certs.
- * This only parses the RFC 3779 extensions since these are necessary for
- * validation.
+ * Lightweight version of cert_parse_pre() for EE certs.
+ * Parses the two RFC 3779 extensions, and performs some sanity checks.
  * Returns cert on success and NULL on failure.
  */
 struct cert *
@@ -616,6 +615,9 @@ cert_parse_ee_cert(const char *fn, X509 *x)
                goto out;
        }
 
+       if (!x509_valid_subject(fn, x))
+               goto out;
+
        if (X509_get_key_usage(x) != KU_DIGITAL_SIGNATURE) {
                warnx("%s: RFC 6487 section 4.8.4: KU must be digitalSignature",
                    fn);
@@ -727,6 +729,9 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
                goto out;
        }
 
+       if (!x509_valid_subject(p.fn, x))
+               goto out;
+
        /* Look for X509v3 extensions. */
 
        if ((extsz = X509_get_ext_count(x)) < 0)
index e41baff..dbef64d 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: extern.h,v 1.188 2023/06/29 14:33:35 tb Exp $ */
+/*     $OpenBSD: extern.h,v 1.189 2023/09/12 09:33:30 job Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -839,6 +839,7 @@ int          x509_location(const char *, const char *, const char *,
                    GENERAL_NAME *, char **);
 int             x509_inherits(X509 *);
 int             x509_any_inherits(X509 *);
+int             x509_valid_subject(const char *, const X509 *);
 time_t          x509_find_expires(time_t, struct auth *, struct crl_tree *);
 
 /* printers */
index 5d63a8f..9a3b637 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: x509.c,v 1.73 2023/06/23 15:32:15 tb Exp $ */
+/*     $OpenBSD: x509.c,v 1.74 2023/09/12 09:33:30 job Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Claudio Jeker <claudio@openbsd.org>
@@ -860,6 +860,86 @@ x509_location(const char *fn, const char *descr, const char *proto,
        return 1;
 }
 
+/*
+ * Check that the subject only contains commonName and serialNumber.
+ * Return 0 on failure.
+ */
+int
+x509_valid_subject(const char *fn, const X509 *x)
+{
+       const X509_NAME *xn;
+       const X509_NAME_ENTRY *ne;
+       const ASN1_OBJECT *ao;
+       const ASN1_STRING *as;
+       int cn = 0, sn = 0;
+       int i, nid;
+
+       if ((xn = X509_get_subject_name(x)) == NULL) {
+               warnx("%s: X509_get_subject_name", fn);
+               return 0;
+       }
+
+       for (i = 0; i < X509_NAME_entry_count(xn); i++) {
+               if ((ne = X509_NAME_get_entry(xn, i)) == NULL) {
+                       warnx("%s: X509_NAME_get_entry", fn);
+                       return 0;
+               }
+               if ((ao = X509_NAME_ENTRY_get_object(ne)) == NULL) {
+                       warnx("%s: X509_NAME_ENTRY_get_object", fn);
+                       return 0;
+               }
+
+               nid = OBJ_obj2nid(ao);
+               switch (nid) {
+               case NID_commonName:
+                       if (cn++ > 0) {
+                               warnx("%s: duplicate commonName in subject",
+                                   fn);
+                               return 0;
+                       }
+                       if ((as = X509_NAME_ENTRY_get_data(ne)) == NULL) {
+                               warnx("%s: X509_NAME_ENTRY_get_data failed",
+                                   fn);
+                               return 0;
+                       }
+/*
+ * The following check can be enabled after AFRINIC re-issues CA certs.
+ * https://lists.afrinic.net/pipermail/dbwg/2023-March/000436.html
+ */
+#if 0
+                       if (ASN1_STRING_type(as) != V_ASN1_PRINTABLESTRING) {
+                               warnx("%s: RFC 6487 section 4.5: commonName is"
+                                   " not PrintableString", fn);
+                               return 0;
+                       }
+#endif
+                       break;
+               case NID_serialNumber:
+                       if (sn++ > 0) {
+                               warnx("%s: duplicate serialNumber in subject",
+                                   fn);
+                               return 0;
+                       }
+                       break;
+               case NID_undef:
+                       warnx("%s: OBJ_obj2nid failed", fn);
+                       return 0;
+               default:
+                       warnx("%s: RFC 6487 section 4.5: unexpected attribute "
+                           "%s", fn, OBJ_nid2sn(nid));
+                       return 0;
+               }
+       }
+
+       if (cn == 0) {
+               warnx("%s: RFC 6487 section 4.5: subject missing commonName",
+                   fn);
+               return 0;
+       }
+
+       return 1;
+}
+
 /*
  * Convert an ASN1_INTEGER into a hexstring.
  * Returned string needs to be freed by the caller.