Index | Thread | Search

From:
Theo Buehler <tb@theobuehler.org>
Subject:
rpki-client: non-canonical ROAIPAddress order
To:
Job Snijders <job@bsd.nl>
Cc:
tech@openbsd.org
Date:
Wed, 21 Oct 2026 11:21:25 +0200

Download raw body.

Thread
On Sat, Oct 17, 2026 at 07:32:56AM +0000, Job Snijders wrote:
> Perhaps in the future in a similar way non-canonically encoded ROA
> eContent can be tracked?

Here's a diff that does this. There are currently 12 ROAs under ARIN
that fail the check. None fails the IPv6 before IPv4 check. Still, I
decided to hide the warning under -v.

Not sure how/where to print that in file mode, so I only added something
to JSON for now.

Index: extern.h
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/extern.h,v
diff -u -p -r1.313 extern.h
--- extern.h	19 Oct 2026 03:28:19 -0000	1.313
+++ extern.h	21 Oct 2026 08:58:50 -0000
@@ -323,6 +323,7 @@ struct roa {
 	int		 talid; /* ROAs are covered by which TAL */
 	int		 issuerid; /* certid of the issuer */
 	int		 valid; /* validated resources */
+	int		 noncanonical; /* ips not ordered per RFC 9582, 4.3.3 */
 	time_t		 signtime; /* CMS signing-time attribute */
 	time_t		 expires; /* when the signature path expires */
 };
Index: print.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/print.c,v
diff -u -p -r1.80 print.c
--- print.c	19 Oct 2026 03:16:39 -0000	1.80
+++ print.c	21 Oct 2026 09:06:42 -0000
@@ -524,6 +524,7 @@ roa_print(const struct cert *c, const st
 		json_do_int("signing_time", p->signtime);
 		json_do_int("valid_since", c->notbefore);
 		json_do_int("valid_until", c->notafter);
+		json_do_bool("noncanonical", p->noncanonical);
 		if (p->expires)
 			json_do_int("expires", p->expires);
 	} else {
Index: roa.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/roa.c,v
diff -u -p -r1.95 roa.c
--- roa.c	19 Oct 2026 03:16:39 -0000	1.95
+++ roa.c	21 Oct 2026 09:16:18 -0000
@@ -59,6 +59,42 @@ ASN1_SEQUENCE(ROAIPAddress) = {
 	ASN1_OPT(ROAIPAddress, maxLength, ASN1_INTEGER),
 } ASN1_SEQUENCE_END(ROAIPAddress);
 
+/* XXX - This is very similar to vrpcmp() below. Can we dedup? */
+static int
+roa_ip_cmp(const struct roa_ip *a, const struct roa_ip *b)
+{
+	int cmp, length;
+
+	if (a->afi < b->afi)
+		return -1;
+	if (a->afi > b->afi)
+		return -1;
+
+	length = a->afi == AFI_IPV4 ? 4 : 16;
+
+	if ((cmp = memcmp(a->min, b->min, length)) != 0) {
+		if (cmp < 0)
+			return -1;
+		else
+			return 1;
+	}
+
+	/* Inverted comparison: larger prefixes come first. */
+	if ((cmp = memcmp(a->max, b->max, length)) != 0) {
+		if (cmp > 0)
+			return -1;
+		else
+			return 1;
+	}
+
+	if (a->maxlength < b->maxlength)
+		return -1;
+	if (a->maxlength > b->maxlength)
+		return 1;
+
+	return 0;
+}
+
 /*
  * Parses the eContent section of an ROA file, RFC 9582, section 4.
  * Returns zero on failure, non-zero on success.
@@ -77,7 +113,7 @@ roa_parse_econtent(const char *fn, void 
 	const ROAIPAddress		*addr;
 	uint64_t			 maxlen;
 	struct ip_addr			 ipaddr;
-	struct roa_ip			*res;
+	struct roa_ip			*res, *last = NULL;
 	int				 ipaddrblocksz, superfluous, warned = 0;
 	int				 i, j, rc = 0;
 
@@ -133,6 +169,11 @@ roa_parse_econtent(const char *fn, void 
 				    "IPv4 appears twice", fn);
 				goto out;
 			}
+			if (ipv6_seen) {
+				warnx("%s: RFC 9582 section 4.3.3.1: "
+				    "IPv4 should precede IPv6", fn);
+				roa->noncanonical = 1;
+			}
 			break;
 		case AFI_IPV6:
 			if (ipv6_seen++ > 0) {
@@ -159,6 +200,7 @@ roa_parse_econtent(const char *fn, void 
 		if (roa->ips == NULL)
 			err(1, NULL);
 
+		last = NULL;
 		for (j = 0; j < addrsz; j++) {
 			addr = sk_ROAIPAddress_value(addrs, j);
 
@@ -206,8 +248,17 @@ roa_parse_econtent(const char *fn, void 
 			res->maxlength = maxlen;
 			res->superfluous = superfluous;
 			ip_roa_compose_ranges(res);
+
+			if (last != NULL) {
+				if (roa_ip_cmp(last, res) >= 0)
+					roa->noncanonical = 1;
+			}
+			last = res;
 		}
 	}
+
+	if (verbose > 0 && roa->noncanonical)
+		warnx("%s: RFC 9582, 4.3.3: non-canonical address order", fn);
 
 	rc = 1;
  out: