From: Theo Buehler Subject: rpki-client: check issuer for certs and CRLs To: tech@openbsd.org Date: Thu, 30 May 2024 16:43:42 +0200 This slightly generalizes x509_valid_subject() into a Name validating function, applies it to both subject and issuer of certs and uses it for CRLs as well. Now the verifier does check that the issuer's subject matches the subject's issuer when building chains, but what exactly it checks on the CRL side of things is not quite so obvious. I think we're better off checking both, as the check is simple and cheap enough. I haven't looked into adding some smarts for avoiding the afrinic special #if 0, but I'm not sure it's worth it. Index: cert.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v diff -u -p -r1.131 cert.c --- cert.c 20 May 2024 15:51:43 -0000 1.131 +++ cert.c 30 May 2024 13:27:24 -0000 @@ -651,6 +651,28 @@ certificate_policies(const char *fn, str 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 t 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 uns goto out; } - if (!x509_valid_subject(fn, x)) + if (!cert_check_subject_and_issuer(fn, x)) goto out; /* Look for X509v3 extensions. */ Index: crl.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/crl.c,v diff -u -p -r1.35 crl.c --- crl.c 29 May 2024 13:26:24 -0000 1.35 +++ crl.c 30 May 2024 13:27:37 -0000 @@ -166,6 +166,7 @@ crl_parse(const char *fn, const unsigned 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; @@ -191,6 +192,13 @@ crl_parse(const char *fn, const unsigned warnx("%s: RFC 6487 section 5: version 2 expected", fn); 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) { Index: extern.h =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v diff -u -p -r1.219 extern.h --- extern.h 29 May 2024 13:26:24 -0000 1.219 +++ extern.h 30 May 2024 12:07:07 -0000 @@ -917,7 +917,7 @@ int x509_location(const char *, const 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 */ Index: x509.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v diff -u -p -r1.88 x509.c --- x509.c 29 May 2024 13:26:24 -0000 1.88 +++ x509.c 30 May 2024 13:28:58 -0000 @@ -842,24 +842,18 @@ x509_location(const char *fn, const char } /* - * 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 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 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 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; }