rpki-client: check issuer for certs and CRLs

Per RFC 6487, the subject and issuer fields of a certificate and the issuer
field of a CRL are subject to the same restrictions: only a commonName and
an optional serialNumber may be present and the commonName must be an ASN.1
printable string.

So far we've only checked the subject of certificates, which covers almost
everything by relying on the verifier to check that the issuer's subject is
identical to the subject's issuer, also for CRLs per X509_V_FLAG_CRL_CHECK.
The only thing missing this way is the TA's issuer.

Since the check is cheap and simple, we're better off doing it ourselves:
Refactor the x509_vaild_subject() helper to take an X509_NAME (which is of
course the appropriate name for a type representing an X.501 distinguished
name). This checks the details of RFC 6487, section 4.4, except that we
still can't check for a printable string since afrinic has ~3000 EE certs
that don't follow the spec, which would knock out ~45% of their ROAs. We're
told that this is going to be fixed this year.

looks good to claudio
ok job

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index 3eb769f..d180c68 100644 (file)
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: cert.c,v 1.131 2024/05/20 15:51:43 claudio Exp $ */
+/*     $OpenBSD: cert.c,v 1.132 2024/05/31 02:45:15 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Job Snijders <job@openbsd.org>
@@ -651,6 +651,28 @@ certificate_policies(const char *fn, struct cert *cert, X509_EXTENSION *ext)
        return rc;
 }
 
+static int
+cert_check_subject_and_issuer(const char *fn, const X509 *x)
+{
+       const X509_NAME *name;
+
+       if ((name = X509_get_subject_name(x)) == NULL) {
+               warnx("%s: X509_get_subject_name", fn);
+               return 0;
+       }
+       if (!x509_valid_name(fn, "subject", name))
+               return 0;
+
+       if ((name = X509_get_issuer_name(x)) == NULL) {
+               warnx("%s: X509_get_issuer_name", fn);
+               return 0;
+       }
+       if (!x509_valid_name(fn, "issuer", name))
+               return 0;
+
+       return 1;
+}
+
 /*
  * Lightweight version of cert_parse_pre() for EE certs.
  * Parses the two RFC 3779 extensions, and performs some sanity checks.
@@ -671,7 +693,7 @@ cert_parse_ee_cert(const char *fn, int talid, X509 *x)
                goto out;
        }
 
-       if (!x509_valid_subject(fn, x))
+       if (!cert_check_subject_and_issuer(fn, x))
                goto out;
 
        if (X509_get_key_usage(x) != KU_DIGITAL_SIGNATURE) {
@@ -791,7 +813,7 @@ cert_parse_pre(const char *fn, const unsigned char *der, size_t len)
                goto out;
        }
 
-       if (!x509_valid_subject(fn, x))
+       if (!cert_check_subject_and_issuer(fn, x))
                goto out;
 
        /* Look for X509v3 extensions. */

diff --git a/usr.sbin/rpki-client/crl.c b/usr.sbin/rpki-client/crl.c
index 45eaa62..6896b6f 100644 (file)
--- a/usr.sbin/rpki-client/crl.c
+++ b/usr.sbin/rpki-client/crl.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: crl.c,v 1.35 2024/05/29 13:26:24 tb Exp $ */
+/*     $OpenBSD: crl.c,v 1.36 2024/05/31 02:45:15 tb Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -166,6 +166,7 @@ crl_parse(const char *fn, const unsigned char *der, size_t len)
        const unsigned char     *oder;
        struct crl              *crl;
        const X509_ALGOR        *palg;
+       const X509_NAME         *name;
        const ASN1_OBJECT       *cobj;
        const ASN1_TIME         *at;
        int                      count, nid, rc = 0;
@@ -192,6 +193,13 @@ crl_parse(const char *fn, const unsigned char *der, size_t len)
                goto out;
        }
 
+       if ((name = X509_CRL_get_issuer(crl->x509_crl)) == NULL) {
+               warnx("%s: X509_CRL_get_issuer", fn);
+               goto out;
+       }
+       if (!x509_valid_name(fn, "issuer", name))
+               goto out;
+
        X509_CRL_get0_signature(crl->x509_crl, NULL, &palg);
        if (palg == NULL) {
                warnx("%s: X509_CRL_get0_signature", fn);

diff --git a/usr.sbin/rpki-client/extern.h b/usr.sbin/rpki-client/extern.h
index 6a871ba..46c64af 100644 (file)
--- a/usr.sbin/rpki-client/extern.h
+++ b/usr.sbin/rpki-client/extern.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: extern.h,v 1.219 2024/05/29 13:26:24 tb Exp $ */
+/*     $OpenBSD: extern.h,v 1.220 2024/05/31 02:45:15 tb Exp $ */
 /*
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
  *
@@ -917,7 +917,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 *);
+int             x509_valid_name(const char *, const char *, const X509_NAME *);
 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 d3d3a6c..9eabdac 100644 (file)
--- a/usr.sbin/rpki-client/x509.c
+++ b/usr.sbin/rpki-client/x509.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: x509.c,v 1.88 2024/05/29 13:26:24 tb Exp $ */
+/*     $OpenBSD: x509.c,v 1.89 2024/05/31 02:45:15 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Claudio Jeker <claudio@openbsd.org>
@@ -842,24 +842,18 @@ x509_location(const char *fn, const char *descr, const char *proto,
 }
 
 /*
- * Check that the subject only contains commonName and serialNumber.
+ * Check that subject or issuer only contain commonName and serialNumber.
  * Return 0 on failure.
  */
 int
-x509_valid_subject(const char *fn, const X509 *x)
+x509_valid_name(const char *fn, const char *descr, const X509_NAME *xn)
 {
-       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);
@@ -874,8 +868,8 @@ x509_valid_subject(const char *fn, const X509 *x)
                switch (nid) {
                case NID_commonName:
                        if (cn++ > 0) {
-                               warnx("%s: duplicate commonName in subject",
-                                   fn);
+                               warnx("%s: duplicate commonName in %s",
+                                   fn, descr);
                                return 0;
                        }
                        if ((as = X509_NAME_ENTRY_get_data(ne)) == NULL) {
@@ -897,8 +891,8 @@ x509_valid_subject(const char *fn, const X509 *x)
                        break;
                case NID_serialNumber:
                        if (sn++ > 0) {
-                               warnx("%s: duplicate serialNumber in subject",
-                                   fn);
+                               warnx("%s: duplicate serialNumber in %s",
+                                   fn, descr);
                                return 0;
                        }
                        break;
@@ -907,14 +901,14 @@ x509_valid_subject(const char *fn, const X509 *x)
                        return 0;
                default:
                        warnx("%s: RFC 6487 section 4.5: unexpected attribute"
-                           " %s", fn, nid2str(nid));
+                           " %s in %s", fn, nid2str(nid), descr);
                        return 0;
                }
        }
 
        if (cn == 0) {
-               warnx("%s: RFC 6487 section 4.5: subject missing commonName",
-                   fn);
+               warnx("%s: RFC 6487 section 4.5: %s missing commonName",
+                   fn, descr);
                return 0;
        }