Improve x509_get_purpose()

Instead of only differentiating between CA and BGPsec Router certs,
make it recognize TA and EE certs as well. TAs and CAs have the cA
boolean in the basic constraints, while EE and BGPsec router certs
do not.

TAs are self-signed, CAs not self-issued, all other certs with the
cA boolean are invalid. EE certs do not have an extended key usage
and BGPsec certs contain the id-kp-bgpsec-router OID.

Handle the new purposes where needed.
                                                                                                    ok job

4 files changed, 53 insertions, 16 deletions

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index ae162652626..5cf6011d7aa 100644
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: cert.c,v 1.142 2024/06/08 13:28:35 tb Exp $ */
+/*	$OpenBSD: cert.c,v 1.143 2024/06/08 13:31:37 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Job Snijders <job@openbsd.org>
@@ -957,11 +957,12 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 		goto out;
 	if (!x509_get_notafter(x, fn, &cert->notafter))
 		goto out;
-	cert->purpose = x509_get_purpose(x, fn);
 
 	/* Validation on required fields. */
-
+	cert->purpose = x509_get_purpose(x, fn);
 	switch (cert->purpose) {
+	case CERT_PURPOSE_TA:
+		/* XXX - caller should indicate if it expects TA or CA cert */
 	case CERT_PURPOSE_CA:
 		if ((pkey = X509_get0_pubkey(x)) == NULL) {
 			warnx("%s: X509_get0_pubkey failed", fn);
@@ -1015,6 +1016,9 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
 			goto out;
 		}
 		break;
+	case CERT_PURPOSE_EE:
+		warn("%s: unexpected EE cert", fn);
+		goto out;
 	default:
 		warnx("%s: x509_get_purpose failed in %s", fn, __func__);
 		goto out;
diff --git a/usr.sbin/rpki-client/filemode.c b/usr.sbin/rpki-client/filemode.c
index 630b633b2f0..71dcc5c5342 100644
--- a/usr.sbin/rpki-client/filemode.c
+++ b/usr.sbin/rpki-client/filemode.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: filemode.c,v 1.43 2024/06/06 07:19:10 tb Exp $ */
+/*	$OpenBSD: filemode.c,v 1.44 2024/06/08 13:31:38 tb Exp $ */
 /*
  * Copyright (c) 2019 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -157,7 +157,8 @@ parse_load_cert(char *uri)
 	if (cert == NULL)
 		goto done;
 	if (cert->purpose != CERT_PURPOSE_CA) {
-		warnx("AIA reference to bgpsec cert %s", uri);
+		warnx("AIA reference to %s in %s",
+		    purpose2str(cert->purpose), uri);
 		goto done;
 	}
 	/* try to load the CRL of this cert */
diff --git a/usr.sbin/rpki-client/main.c b/usr.sbin/rpki-client/main.c
index 8546861a310..342548ce3ec 100644
--- a/usr.sbin/rpki-client/main.c
+++ b/usr.sbin/rpki-client/main.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: main.c,v 1.259 2024/06/07 08:22:53 claudio Exp $ */
+/*	$OpenBSD: main.c,v 1.260 2024/06/08 13:31:38 tb Exp $ */
 /*
  * Copyright (c) 2021 Claudio Jeker <claudio@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -618,6 +618,7 @@ entity_process(struct ibuf *b, struct stats *st, struct vrp_tree *tree,
 		}
 		cert = cert_read(b);
 		switch (cert->purpose) {
+		case CERT_PURPOSE_TA:
 		case CERT_PURPOSE_CA:
 			queue_add_from_cert(cert);
 			break;
@@ -626,7 +627,7 @@ entity_process(struct ibuf *b, struct stats *st, struct vrp_tree *tree,
 			repo_stat_inc(rp, talid, type, STYPE_BGPSEC);
 			break;
 		default:
-			errx(1, "unexpected cert purpose received");
+			errx(1, "unexpected %s", purpose2str(cert->purpose));
 			break;
 		}
 		cert_free(cert);
diff --git a/usr.sbin/rpki-client/x509.c b/usr.sbin/rpki-client/x509.c
index 3c0517c89ab..908778c1e82 100644
--- a/usr.sbin/rpki-client/x509.c
+++ b/usr.sbin/rpki-client/x509.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: x509.c,v 1.95 2024/06/08 13:28:35 tb Exp $ */
+/*	$OpenBSD: x509.c,v 1.96 2024/06/08 13:31:38 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Claudio Jeker <claudio@openbsd.org>
@@ -266,18 +266,34 @@ x509_get_ski(X509 *x, const char *fn, char **ski)
 }
 
 /*
- * Check the certificate's purpose: CA or BGPsec Router.
- * Return a member of enum cert_purpose.
+ * Check the cert's purpose: the cA bit in basic constraints distinguishes
+ * between TA/CA and EE/BGPsec router. TAs are self-signed, CAs not self-issued,
+ * EEs have no extended key usage, BGPsec router have id-kp-bgpsec-router OID.
  */
 enum cert_purpose
 x509_get_purpose(X509 *x, const char *fn)
 {
 	BASIC_CONSTRAINTS		*bc = NULL;
 	EXTENDED_KEY_USAGE		*eku = NULL;
-	int				 crit;
+	int				 crit, ext_flags, is_ca;
 	enum cert_purpose		 purpose = CERT_PURPOSE_INVALID;
 
-	if (X509_check_ca(x) == 1) {
+	if (!x509_cache_extensions(x, fn))
+		goto out;
+
+	ext_flags = X509_get_extension_flags(x);
+
+	/* This weird API can return 0, 1, 2, 4, 5 but can't error... */
+	if ((is_ca = X509_check_ca(x)) > 1) {
+		if (is_ca == 4)
+			warnx("%s: RFC 6487: sections 4.8.1 and 4.8.4: "
+			    "no basic constraints, but keyCertSign set", fn);
+		else
+			warnx("%s: unexpected legacy certificate", fn);
+		goto out;
+	}
+
+	if (is_ca) {
 		bc = X509_get_ext_d2i(x, NID_basic_constraints, &crit, NULL);
 		if (bc == NULL) {
 			if (crit != -1)
@@ -298,22 +314,37 @@ x509_get_purpose(X509 *x, const char *fn)
 			    "Constraint must be absent", fn);
 			goto out;
 		}
-		purpose = CERT_PURPOSE_CA;
-		/* XXX - we may want to check EXFLAG_SI and add a TA purpose. */
+		/*
+		 * EXFLAG_SI means that issuer and subject are identical.
+		 * EXFLAG_SS is SI plus the AKI is absent or matches the SKI.
+		 * Thus, exactly the trust anchors should have EXFLAG_SS set
+		 * and we should never see EXFLAG_SI without EXFLAG_SS.
+		 */
+		if ((ext_flags & EXFLAG_SS) != 0)
+			purpose = CERT_PURPOSE_TA;
+		else if ((ext_flags & EXFLAG_SI) == 0)
+			purpose = CERT_PURPOSE_CA;
+		else
+			warnx("%s: RFC 6487, section 4.8.3: "
+			    "self-issued cert with AKI-SKI mismatch", fn);
 		goto out;
 	}
 
-	if (X509_get_extension_flags(x) & EXFLAG_BCONS) {
+	if ((ext_flags & EXFLAG_BCONS) != 0) {
 		warnx("%s: Basic Constraints ext in non-CA cert", fn);
 		goto out;
 	}
 
+	/*
+	 * EKU is only defined for BGPsec Router certs and must be absent from
+	 * EE certs.
+	 */
 	eku = X509_get_ext_d2i(x, NID_ext_key_usage, &crit, NULL);
 	if (eku == NULL) {
 		if (crit != -1)
 			warnx("%s: error parsing EKU", fn);
 		else
-			warnx("%s: EKU: extension missing", fn);
+			purpose = CERT_PURPOSE_EE; /* EKU absent */
 		goto out;
 	}
 	if (crit != 0) {