From: Claudio Jeker Subject: Re: rpki-client: rfc 3779 naming mess To: Theo Buehler Cc: tech@openbsd.org Date: Tue, 7 Apr 2026 10:25:23 +0200 On Tue, Apr 07, 2026 at 07:46:47AM +0200, Theo Buehler wrote: > A bit of spring cleaning and the result isn't perfect... > > NID_sbgp_ipAddrBlock and NID_sbgp_autonomousSysNum are the weird OpenSSL > names for id-pe-ipAddrBlocks and id-pe-autonomousSysIds in RFC 3779. > Those led to our invention of sbgp-ipAddrBlock and sbgp-autonomousSysNum > and variants thereof, all of which don't seem to exist elsewhere. > > Just use ipAddrBlocks and autonomousSysIds where we need to spell them > out, in particular in comments and warnings. > > ipaddrblk is ugly and can be expanded in the API. I used addrs for the > variable to avoid overlong lines. There's precedent in the constraints > code. > > The doubled s in assysnum makes no sense and since autonomoussysids is > long and unreadable, I used asids in the API. > > PS: I kept the annoying sbgp_ prefix for now. I now think most of the > sbgp_* functions in cert.c don't belong there. They should go into ip.c > and as.c, respectively, because they are about parsing things into our > own data structures and have nothing to do with certs per se. Moving > them should solve the naming mystery cleanly. I like this paint of the bike shed better then the ugly rusty pink from before. OK claudio@ I think the usage of autonomousSysIds in the warnings is ugly but the RFC called them like this. shrug > Index: cert.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v > diff -u -p -r1.231 cert.c > --- cert.c 3 Apr 2026 02:41:03 -0000 1.231 > +++ cert.c 4 Apr 2026 06:11:53 -0000 > @@ -1118,7 +1118,7 @@ sbgp_addr_inherit(const char *fn, struct > } > > int > -sbgp_parse_ipaddrblk(const char *fn, const IPAddrBlocks *addrblk, > +sbgp_parse_ipaddrblocks(const char *fn, const IPAddrBlocks *addrs, > struct cert_ip **out_ips, size_t *out_num_ips) > { > const IPAddressFamily *af; > @@ -1128,20 +1128,19 @@ sbgp_parse_ipaddrblk(const char *fn, con > struct cert_ip *ips = NULL; > size_t num_ips = 0, num; > int ipv4_seen = 0, ipv6_seen = 0; > - int i, j, ipaddrblocksz; > + int i, j, addrsz; > > assert(*out_ips == NULL && *out_num_ips == 0); > > - ipaddrblocksz = sk_IPAddressFamily_num(addrblk); > - if (ipaddrblocksz != 1 && ipaddrblocksz != 2) { > + addrsz = sk_IPAddressFamily_num(addrs); > + if (addrsz != 1 && addrsz != 2) { > warnx("%s: RFC 6487 section 4.8.10: unexpected number of " > - "ipAddrBlocks (got %d, expected 1 or 2)", > - fn, ipaddrblocksz); > + "ipAddrBlocks (got %d, expected 1 or 2)", fn, addrsz); > goto out; > } > > - for (i = 0; i < ipaddrblocksz; i++) { > - af = sk_IPAddressFamily_value(addrblk, i); > + for (i = 0; i < addrsz; i++) { > + af = sk_IPAddressFamily_value(addrs, i); > > switch (af->ipAddressChoice->type) { > case IPAddressChoice_inherit: > @@ -1230,40 +1229,40 @@ sbgp_parse_ipaddrblk(const char *fn, con > } > > /* > - * Parse an sbgp-ipAddrBlock X509 extension, RFC 6487 4.8.10, with > + * Parse an IP Resources X.509v3 extension, RFC 6487 4.8.10, with > * syntax documented in RFC 3779 starting in section 2.2. > * Returns zero on failure, non-zero on success. > */ > static int > -sbgp_ipaddrblk(const char *fn, struct cert *cert, const X509_EXTENSION *ext) > +sbgp_ipaddrblocks(const char *fn, struct cert *cert, const X509_EXTENSION *ext) > { > - IPAddrBlocks *addrblk = NULL; > + IPAddrBlocks *addrs = NULL; > int rc = 0; > > if (!X509_EXTENSION_get_critical(ext)) { > - warnx("%s: RFC 6487 section 4.8.10: sbgp-ipAddrBlock: " > + warnx("%s: RFC 6487 section 4.8.10: ipAddrBlocks: " > "extension not critical", fn); > goto out; > } > > /* XXX - cast away const for OpenSSL 3 and LibreSSL */ > - if ((addrblk = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) { > - warnx("%s: RFC 6487 section 4.8.10: sbgp-ipAddrBlock: " > + if ((addrs = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) { > + warnx("%s: RFC 6487 section 4.8.10: ipAddrBlocks: " > "failed extension parse", fn); > goto out; > } > > - if (!sbgp_parse_ipaddrblk(fn, addrblk, &cert->ips, &cert->num_ips)) > + if (!sbgp_parse_ipaddrblocks(fn, addrs, &cert->ips, &cert->num_ips)) > goto out; > > if (cert->num_ips == 0) { > - warnx("%s: RFC 6487 section 4.8.10: empty ipAddrBlock", fn); > + warnx("%s: RFC 6487 section 4.8.10: empty ipAddrBlocks", fn); > goto out; > } > > rc = 1; > out: > - IPAddrBlocks_free(addrblk); > + IPAddrBlocks_free(addrs); > return rc; > } > > @@ -1376,7 +1375,7 @@ cert_has_one_as(const struct cert *cert) > } > > int > -sbgp_parse_assysnum(const char *fn, const ASIdentifiers *asidentifiers, > +sbgp_parse_asids(const char *fn, const ASIdentifiers *asidentifiers, > struct cert_as **out_as, size_t *out_num_ases) > { > const ASIdOrRanges *aors = NULL; > @@ -1387,13 +1386,13 @@ sbgp_parse_assysnum(const char *fn, cons > assert(*out_as == NULL && *out_num_ases == 0); > > if (asidentifiers->rdi != NULL) { > - warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: " > + warnx("%s: RFC 6487 section 4.8.11: autonomousSysIds: " > "should not have RDI values", fn); > goto out; > } > > if (asidentifiers->asnum == NULL) { > - warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: " > + warnx("%s: RFC 6487 section 4.8.11: autonomousSysIds: " > "no AS number resource set", fn); > goto out; > } > @@ -1462,31 +1461,30 @@ sbgp_parse_assysnum(const char *fn, cons > } > > /* > - * Parse RFC 6487 4.8.11 X509v3 extension, with syntax documented in RFC > - * 3779 starting in section 3.2. > + * Parse an AS Resources X.509v3 extension, RFC 6487 4.8.11, with > + * syntax documented in RFC 3779 starting in section 3.2. > * Returns zero on failure, non-zero on success. > */ > static int > -sbgp_assysnum(const char *fn, struct cert *cert, const X509_EXTENSION *ext) > +sbgp_asids(const char *fn, struct cert *cert, const X509_EXTENSION *ext) > { > ASIdentifiers *asidentifiers = NULL; > int rc = 0; > > if (!X509_EXTENSION_get_critical(ext)) { > - warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: " > + warnx("%s: RFC 6487 section 4.8.11: autonomousSysIds: " > "extension not critical", fn); > goto out; > } > > /* XXX - cast away const for OpenSSL 3 and LibreSSL */ > if ((asidentifiers = X509V3_EXT_d2i((X509_EXTENSION *)ext)) == NULL) { > - warnx("%s: RFC 6487 section 4.8.11: autonomousSysNum: " > + warnx("%s: RFC 6487 section 4.8.11: autonomousSysIds: " > "failed extension parse", fn); > goto out; > } > > - if (!sbgp_parse_assysnum(fn, asidentifiers, &cert->ases, > - &cert->num_ases)) > + if (!sbgp_parse_asids(fn, asidentifiers, &cert->ases, &cert->num_ases)) > goto out; > > rc = 1; > @@ -1589,13 +1587,13 @@ cert_parse_extensions(const char *fn, st > case NID_sbgp_ipAddrBlock: > if (ip++ > 0) > goto dup; > - if (!sbgp_ipaddrblk(fn, cert, ext)) > + if (!sbgp_ipaddrblocks(fn, cert, ext)) > goto out; > break; > case NID_sbgp_autonomousSysNum: > if (as++ > 0) > goto dup; > - if (!sbgp_assysnum(fn, cert, ext)) > + if (!sbgp_asids(fn, cert, ext)) > goto out; > break; > default: > Index: constraints.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/constraints.c,v > diff -u -p -r1.5 constraints.c > --- constraints.c 12 Nov 2024 09:23:07 -0000 1.5 > +++ constraints.c 4 Apr 2026 05:57:20 -0000 > @@ -452,13 +452,13 @@ constraints_parse_talid(int talid) > errx(1, "%s: failed to canonize AS numbers denylist", fn); > > if (have_allow_as) { > - if (!sbgp_parse_assysnum(fn, allow_asids, &allow_ases, > + if (!sbgp_parse_asids(fn, allow_asids, &allow_ases, > &num_allow_ases)) > errx(1, "%s: failed to parse AS identifiers allowlist", > fn); > } > if (have_deny_as) { > - if (!sbgp_parse_assysnum(fn, deny_asids, &deny_ases, > + if (!sbgp_parse_asids(fn, deny_asids, &deny_ases, > &num_deny_as)) > errx(1, "%s: failed to parse AS identifiers denylist", > fn); > @@ -466,7 +466,7 @@ constraints_parse_talid(int talid) > if (have_allow_ips) { > constraints_normalize_ip_addrblocks(fn, &allow_addrs); > > - if (!sbgp_parse_ipaddrblk(fn, allow_addrs, &allow_ips, > + if (!sbgp_parse_ipaddrblocks(fn, allow_addrs, &allow_ips, > &num_allow_ips)) > errx(1, "%s: failed to parse IP addresses allowlist", > fn); > @@ -474,7 +474,7 @@ constraints_parse_talid(int talid) > if (have_deny_ips) { > constraints_normalize_ip_addrblocks(fn, &deny_addrs); > > - if (!sbgp_parse_ipaddrblk(fn, deny_addrs, &deny_ips, > + if (!sbgp_parse_ipaddrblocks(fn, deny_addrs, &deny_ips, > &num_deny_ips)) > errx(1, "%s: failed to parse IP addresses denylist", > fn); > Index: extern.h > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v > diff -u -p -r1.277 extern.h > --- extern.h 3 Feb 2026 16:21:37 -0000 1.277 > +++ extern.h 4 Apr 2026 06:10:01 -0000 > @@ -824,7 +824,7 @@ int sbgp_addr(const char *, struct cer > int sbgp_addr_range(const char *, struct cert_ip *, size_t *, > enum afi, const IPAddressRange *); > > -int sbgp_parse_ipaddrblk(const char *, const IPAddrBlocks *, > +int sbgp_parse_ipaddrblocks(const char *, const IPAddrBlocks *, > struct cert_ip **, size_t *); > > /* Work with RFC 3779 AS numbers, ranges. */ > @@ -841,7 +841,7 @@ int sbgp_as_id(const char *, struct ce > int sbgp_as_range(const char *, struct cert_as *, size_t *, > const ASRange *); > > -int sbgp_parse_assysnum(const char *, const ASIdentifiers *, > +int sbgp_parse_asids(const char *, const ASIdentifiers *, > struct cert_as **, size_t *); > > /* Constraints-specific */ > @@ -1037,8 +1037,8 @@ int mkpathat(int, const char *); > #define CERTID_MAX 1000000 > > /* > - * Maximum number of elements in the sbgp-ipAddrBlock (IP) and > - * sbgp-autonomousSysNum (AS) X.509v3 extension of CA/EE certificates. > + * Maximum number of elements in the ipAddrBlocks (IP) and > + * autonomousSysIds (AS) X.509v3 extension of certificates. > */ > #define MAX_IP_SIZE 200000 > #define MAX_AS_SIZE 200000 > Index: x509.c > =================================================================== > RCS file: /cvs/src/usr.sbin/rpki-client/x509.c,v > diff -u -p -r1.128 x509.c > --- x509.c 11 Feb 2026 14:41:34 -0000 1.128 > +++ x509.c 4 Apr 2026 07:26:12 -0000 > @@ -217,7 +217,7 @@ x509_inherits(X509 *x) > addrblk = X509_get_ext_d2i(x, NID_sbgp_ipAddrBlock, &crit, NULL); > if (addrblk == NULL) { > if (crit != -1) > - warnx("error parsing ipAddrBlock"); > + warnx("error parsing ipAddrBlocks"); > goto out; > } > > @@ -266,7 +266,7 @@ x509_any_inherits(X509 *x) > > addrblk = X509_get_ext_d2i(x, NID_sbgp_ipAddrBlock, &crit, NULL); > if (addrblk == NULL && crit != -1) > - warnx("error parsing ipAddrBlock"); > + warnx("error parsing ipAddrBlocks"); > if (X509v3_addr_inherits(addrblk)) > rc = 1; > > -- :wq Claudio