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);
 }