Improve x509_get_purpose()
authortb <tb@openbsd.org>
Sat, 8 Jun 2024 13:31:37 +0000 (13:31 +0000)
committertb <tb@openbsd.org>
Sat, 8 Jun 2024 13:31:37 +0000 (13:31 +0000)
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

usr.sbin/rpki-client/cert.c
usr.sbin/rpki-client/filemode.c
usr.sbin/rpki-client/main.c
usr.sbin/rpki-client/x509.c

index ae16265..5cf6011 100644 (file)
@@ -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;
index 630b633..71dcc5c 100644 (file)
@@ -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 */
index 8546861..342548c 100644 (file)
@@ -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);
index 3c0517c..908778c 100644 (file)
@@ -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) {