Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
Re: rpki-client: rfc 3779 naming mess
To:
Theo Buehler <tb@theobuehler.org>
Cc:
tech@openbsd.org
Date:
Tue, 7 Apr 2026 10:25:23 +0200

Download raw body.

Thread
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