From: Theo Buehler Subject: Re: rpki-client: check certificate CRLDP alignment To: Job Snijders Cc: tech@openbsd.org Date: Thu, 24 Sep 2026 11:08:29 +0200 On Thu, Sep 24, 2026 at 08:34:58AM +0000, Job Snijders wrote: > On Thu, Sep 24, 2026 at 08:47:17AM +0200, Theo Buehler wrote: > > On Sat, Sep 12, 2026 at 12:55:04AM +0000, Job Snijders wrote: > > > RFC 6480 section 4.2 figure 2 shows that all valid products in the same > > > CA repository (i.e., listed on the same manifest) point to the same CRL. > > > > > > Already today rpki-client checks whether the CRLDP in a given Manifest > > > EE certificate aligns with the CRL location derived from the fileList > > > (see mft.c lines 193-197). This diff adds a similar check to verify > > > alignment of the CRLDP in CA certificates, BGPsec certs, and the EE > > > certs in other signed objects. > > > > We have only done this for CA and BGPsec certs for now. In order not to > > introduce further complications with deduplication efforts, I think the > > correct placement for this check is at the end of valid_x509() at which > > point we know we have a CRL (due to passing X509_V_FLAG_CRL_CHECK). > > > > Here's a first, trivial refactoring step towards this goal. The second > > step will add the strcmp(). Subsequent steps will deal with valid_cert() > > and the somewhat unfortunate naming. The second step has two parts to it: Match the cert's CRL distribution point against the CA repository from which the CRL was obtained. Do this in valid_x509() after successful validation. This way we know from X509_V_CHECK_CRL that a CRL was present and we also cover all EE certs with the same check. Although a TA cert should never get there, I feel better checking the cert->purpose for !TA. Use an exit path that frees things rather than adding third copy of the same freeing stanza. Teach filemode about crl->mftcrldp: the CRL lookup path in the cache. To make this work nicely add a file variable that points into the uri. The assert() is only there to avoid a possible future leak. This way filemode keeps working with the new strcmp() check in valid_cert() without special casing in the valid_x509() code. Index: parser.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v diff -u -p -r1.186 parser.c --- parser.c 24 Sep 2026 08:37:38 -0000 1.186 +++ parser.c 24 Sep 2026 08:51:37 -0000 @@ -625,11 +625,6 @@ proc_parser_cert(char *file, const unsig goto out; crl = crl_get(&crls, a); - if (strcmp(cert->crl, crl->mftcrldp) != 0) { - warnx("%s: invalid CRLDP pointer", file); - goto out; - } - if (!valid_x509(file, ctx, cert, a, crl, &errstr) || !valid_cert(file, a, cert)) { if (errstr != NULL) Index: validate.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v diff -u -p -r1.87 validate.c --- validate.c 24 Sep 2026 08:37:38 -0000 1.87 +++ validate.c 24 Sep 2026 08:51:37 -0000 @@ -417,7 +417,7 @@ valid_x509(char *file, X509_STORE_CTX *s STACK_OF(X509) *intermediates, *root; STACK_OF(X509_CRL) *crls = NULL; unsigned long flags; - int error; + int error, ret = 0; *errstr = NULL; build_chain(a, &intermediates, &root); @@ -456,18 +456,23 @@ valid_x509(char *file, X509_STORE_CTX *s *errstr = X509_verify_cert_error_string(error); if (filemode && error == X509_V_ERR_CERT_REVOKED) pretty_revocation_time(x509, crl->x509_crl, errstr); - X509_STORE_CTX_cleanup(store_ctx); - sk_X509_free(intermediates); - sk_X509_free(root); - sk_X509_CRL_free(crls); - return 0; + goto out; } + if (cert->purpose != CERT_PURPOSE_TA) { + if (strcmp(cert->crl, crl->mftcrldp) != 0) { + *errstr = "invalid CRLDP pointer"; + goto out; + } + } + + ret = 1; + out: X509_STORE_CTX_cleanup(store_ctx); sk_X509_free(intermediates); sk_X509_free(root); sk_X509_CRL_free(crls); - return 1; + return ret; } /* Index: filemode.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v diff -u -p -r1.88 filemode.c --- filemode.c 24 Sep 2026 08:37:38 -0000 1.88 +++ filemode.c 24 Sep 2026 08:51:37 -0000 @@ -100,7 +100,7 @@ static void parse_load_crl(char *uri) { struct crl *crl; - char *f; + char *f, *file; size_t flen; if (uri == NULL) @@ -109,17 +109,22 @@ parse_load_crl(char *uri) warnx("bad CRL distribution point URI %s", uri); return; } - uri += RSYNC_PROTO_LEN; + file = uri + RSYNC_PROTO_LEN; - f = load_file(uri, &flen); + f = load_file(file, &flen); if (f == NULL) { - warn("parse file %s", uri); + warn("parse file %s", file); return; } - crl = crl_parse(uri, f, flen); - if (crl != NULL && !crl_insert(&crls, crl)) - crl_free(crl); + crl = crl_parse(file, f, flen); + if (crl != NULL) { + assert(crl->mftcrldp == NULL); + if ((crl->mftcrldp = strdup(uri)) == NULL) + err(1, NULL); + if (!crl_insert(&crls, crl)) + crl_free(crl); + } free(f); }