From: Theo Buehler Subject: rpki-client: split a few things out of ta_parse() To: tech@openbsd.org Date: Mon, 19 Jan 2026 08:47:54 +0100 This pulls the non-inheritance check for TAs into cert_parse_extensions() and adds a check that the INRs are a non-empty set. The latter is redundant with existing checks for presence of at least one of ASIdentifiers and IPAddrBlocks combined with non-inheritance but it does not hurt to be explicit. The second change is splitting everything to do with the SPKI into a helper since the current logic is messy and has completely unrelated things interleaved. In a follow-up I'll also split out the validity check. Later on ta_parse() will be renamed into something more appropriate. Index: cert.c =================================================================== RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v diff -u -p -r1.210 cert.c --- cert.c 13 Jan 2026 21:36:17 -0000 1.210 +++ cert.c 14 Jan 2026 20:11:20 -0000 @@ -1738,6 +1738,19 @@ cert_parse_extensions(const char *fn, st goto out; } + if (cert->purpose == CERT_PURPOSE_TA) { + if (x509_any_inherits(cert->x509)) { + warnx("%s: RFC 8630, 2.3: Trust Anchor INRs " + "must not inherit", fn); + goto out; + } + if (cert->num_ips == 0 && cert->num_ases == 0) { + warnx("%s: RFC 8630, 2.3: Trust Anchor INR set " + "must not be empty", fn); + goto out; + } + } + if (cert->purpose == CERT_PURPOSE_BGPSEC_ROUTER) { if (ip != 0) { warnx("%s: RFC 8209, 3.1.3.4: BGPsec Router cert " @@ -1915,15 +1928,17 @@ cert_parse(const char *fn, const unsigne return NULL; } -struct cert * -ta_parse(const char *fn, struct cert *p, const unsigned char *pkey, +/* + * Check that the subjectPublicKeyInfo from the TAL matches the one in the cert. + * Verify that this key signed the cert. + * Returns 1 on success and 0 on failure. + */ +static int +ta_check_pubkey(const char *fn, struct cert *cert, const unsigned char *pkey, size_t pkeysz) { EVP_PKEY *pk, *opk; - time_t now = get_current_time(); - - if (p == NULL) - return NULL; + int rv = 0; /* first check pubkey against the one from the TAL */ pk = d2i_PUBKEY(NULL, &pkey, pkeysz); @@ -1931,7 +1946,7 @@ ta_parse(const char *fn, struct cert *p, warnx("%s: RFC 6487 (trust anchor): bad TAL pubkey", fn); goto badcert; } - if ((opk = X509_get0_pubkey(p->x509)) == NULL) { + if ((opk = X509_get0_pubkey(cert->x509)) == NULL) { warnx("%s: RFC 6487 (trust anchor): missing pubkey", fn); goto badcert; } @@ -1941,37 +1956,52 @@ ta_parse(const char *fn, struct cert *p, goto badcert; } - if (p->notbefore > now) { - warnx("%s: certificate not yet valid", fn); - goto badcert; - } - if (p->notafter < now) { - warnx("%s: certificate has expired", fn); + /* + * Do not replace with a <= 0 check since OpenSSL 3 broke that: + * https://github.com/openssl/openssl/issues/24575 + */ + if (X509_verify(cert->x509, pk) != 1) { + warnx("%s: failed to verify signature", fn); goto badcert; } + + rv = 1; + badcert: + EVP_PKEY_free(pk); + return rv; +} + +/* XXX - this really doesn't parse anything except the SPKI in the TAL. */ +struct cert * +ta_parse(const char *fn, struct cert *p, const unsigned char *pkey, + size_t pkeysz) +{ + time_t now = get_current_time(); + + if (p == NULL) + return NULL; + if (p->purpose != CERT_PURPOSE_TA) { warnx("%s: expected trust anchor purpose, got %s", fn, purpose2str(p->purpose)); goto badcert; } - /* - * Do not replace with a <= 0 check since OpenSSL 3 broke that: - * https://github.com/openssl/openssl/issues/24575 - */ - if (X509_verify(p->x509, pk) != 1) { - warnx("%s: failed to verify signature", fn); + + if (!ta_check_pubkey(fn, p, pkey, pkeysz)) + goto badcert; + + if (p->notbefore > now) { + warnx("%s: certificate not yet valid", fn); goto badcert; } - if (x509_any_inherits(p->x509)) { - warnx("%s: Trust anchor IP/AS resources may not inherit", fn); + if (p->notafter < now) { + warnx("%s: certificate has expired", fn); goto badcert; } - EVP_PKEY_free(pk); return p; badcert: - EVP_PKEY_free(pk); cert_free(p); return NULL; }