From 0466b83f31718e719206ea60685d0def51e591f0 Mon Sep 17 00:00:00 2001 From: tb Date: Fri, 31 May 2024 02:45:15 +0000 Subject: [PATCH] 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 --- usr.sbin/rpki-client/cert.c | 28 +++++++++++++++++++++++++--- usr.sbin/rpki-client/crl.c | 10 +++++++++- usr.sbin/rpki-client/extern.h | 4 ++-- usr.sbin/rpki-client/x509.c | 26 ++++++++++---------------- 4 files changed, 46 insertions(+), 22 deletions(-) diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c index 3eb769f95ac..d180c68176e 100644 --- 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 * Copyright (c) 2021 Job Snijders @@ -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 45eaa623cdd..6896b6f6784 100644 --- 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 * @@ -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 6a871baad8a..46c64afc903 100644 --- 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 * @@ -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 d3d3a6c6ae5..9eabdac9d33 100644 --- 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 * Copyright (c) 2021 Claudio Jeker @@ -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; } -- 2.20.1