From: job <job@openbsd.org>
Date: Tue, 12 Sep 2023 09:33:30 +0000 (+0000)
Subject: Ensure the X.509 Subject only contains commonName and optionally serialNumber
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=7cc1142d2a57a12cb3ba3ea0326081e58f9c374f;p=openbsd

Ensure the X.509 Subject only contains commonName and optionally serialNumber

OK tb@
---

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index c1b12b10c26..c123640f6ad 100644
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -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)
diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h
index e41baff2f1d..dbef64d767f 100644
--- a/usr.sbin/rpki-client/extern.h
+++ b/usr.sbin/rpki-client/extern.h
@@ -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 */
diff --git a/usr.sbin/rpki-client/x509.c b/usr.sbin/rpki-client/x509.c
index 5d63a8fb48d..9a3b637a035 100644
--- a/usr.sbin/rpki-client/x509.c
+++ b/usr.sbin/rpki-client/x509.c
@@ -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.