Download raw body.
rpki-client: check issuer for certs and CRLs
On Thu, May 30, 2024 at 04:43:42PM +0200, Theo Buehler wrote:
> 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.
Looks good to me. Is the afrinic special still needed or did they fail
to re-issue CA certs in the last year?
> 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;
> }
>
>
--
:wq Claudio
rpki-client: check issuer for certs and CRLs