rpki-client: make IP address block checks stricter

There are only two valid AFIs in this context, so check that we have one
or two of them. We only accept the IPv4 and IPv6 AFIs in ip_add_afi_parse()
and reject any SAFI, so enforce that neither AFI is repeated. This doesn't
change things for certificates, where all this is implied by other checks
combined. Making this explicit and match the logic needed for ROAs is a win.

looks good to job
ok claudio

diff --git a/usr.sbin/rpki-client/cert.c b/usr.sbin/rpki-client/cert.c
index f9add47..f695f83 100644 (file)
--- a/usr.sbin/rpki-client/cert.c
+++ b/usr.sbin/rpki-client/cert.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: cert.c,v 1.120 2023/12/10 14:18:23 job Exp $ */
+/*     $OpenBSD: cert.c,v 1.121 2023/12/14 07:52:53 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2021 Job Snijders <job@openbsd.org>
@@ -362,11 +362,20 @@ sbgp_parse_ipaddrblk(const char *fn, const IPAddrBlocks *addrblk,
        enum afi                 afi;
        struct cert_ip          *ips = NULL;
        size_t                   ipsz = 0, sz;
-       int                      i, j;
+       int                      ipv4_seen = 0, ipv6_seen = 0;
+       int                      i, j, ipaddrblocksz;
 
        assert(*out_ips == NULL && *out_ipsz == 0);
 
-       for (i = 0; i < sk_IPAddressFamily_num(addrblk); i++) {
+       ipaddrblocksz = sk_IPAddressFamily_num(addrblk);
+       if (ipaddrblocksz != 1 && ipaddrblocksz != 2) {
+               warnx("%s: RFC 6487 section 4.8.10: unexpected number of "
+                   "ipAddrBlocks (got %d, expected 1 or 2)",
+                   fn, ipaddrblocksz);
+               goto out;
+       }
+
+       for (i = 0; i < ipaddrblocksz; i++) {
                af = sk_IPAddressFamily_value(addrblk, i);
 
                switch (af->ipAddressChoice->type) {
@@ -400,6 +409,23 @@ sbgp_parse_ipaddrblk(const char *fn, const IPAddrBlocks *addrblk,
                        goto out;
                }
 
+               switch(afi) {
+               case AFI_IPV4:
+                       if (ipv4_seen++ > 0) {
+                               warnx("%s: RFC 6487 section 4.8.10: "
+                                   "IPv4 appears twice", fn);
+                               goto out;
+                       }
+                       break;
+               case AFI_IPV6:
+                       if (ipv6_seen++ > 0) {
+                               warnx("%s: RFC 6487 section 4.8.10: "
+                                   "IPv6 appears twice", fn);
+                               goto out;
+                       }
+                       break;
+               }
+
                if (aors == NULL) {
                        if (!sbgp_addr_inherit(fn, ips, &ipsz, afi))
                                goto out;

diff --git a/usr.sbin/rpki-client/roa.c b/usr.sbin/rpki-client/roa.c
index 93838d1..6e19ca7 100644 (file)
--- a/usr.sbin/rpki-client/roa.c
+++ b/usr.sbin/rpki-client/roa.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: roa.c,v 1.71 2023/10/13 12:06:49 job Exp $ */
+/*     $OpenBSD: roa.c,v 1.72 2023/12/14 07:52:53 tb Exp $ */
 /*
  * Copyright (c) 2022 Theo Buehler <tb@openbsd.org>
  * Copyright (c) 2019 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -104,7 +104,7 @@ roa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
        RouteOriginAttestation          *roa;
        const ROAIPAddressFamily        *addrfam;
        const STACK_OF(ROAIPAddress)    *addrs;
-       int                              addrsz;
+       int                              addrsz, ipv4_seen = 0, ipv6_seen = 0;
        enum afi                         afi;
        const ROAIPAddress              *addr;
        uint64_t                         maxlen;
@@ -129,8 +129,8 @@ roa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
        }
 
        ipaddrblocksz = sk_ROAIPAddressFamily_num(roa->ipAddrBlocks);
-       if (ipaddrblocksz > 2) {
-               warnx("%s: draft-rfc6482bis: too many ipAddrBlocks "
+       if (ipaddrblocksz != 1 && ipaddrblocksz != 2) {
+               warnx("%s: draft-rfc6482bis: unexpected number of ipAddrBlocks "
                    "(got %d, expected 1 or 2)", p->fn, ipaddrblocksz);
                goto out;
        }
@@ -146,6 +146,29 @@ roa_parse_econtent(const unsigned char *d, size_t dsz, struct parse *p)
                        goto out;
                }
 
+               switch(afi) {
+               case AFI_IPV4:
+                       if (ipv4_seen++ > 0) {
+                               warnx("%s: RFC 6482bis section 4.3.2: "
+                                   "IPv4 appears twice", p->fn);
+                               goto out;
+                       }
+                       break;
+               case AFI_IPV6:
+                       if (ipv6_seen++ > 0) {
+                               warnx("%s: RFC 6482bis section 4.3.2: "
+                                   "IPv6 appears twice", p->fn);
+                               goto out;
+                       }
+                       break;
+               }
+
+               if (addrsz == 0) {
+                       warnx("%s: RFC 6482bis, section 4.3.2: "
+                           "empty ROAIPAddressFamily", p->fn);
+                       goto out;
+               }
+
                if (p->res->ipsz + addrsz >= MAX_IP_SIZE) {
                        warnx("%s: too many ROAIPAddress entries: limit %d",
                            p->fn, MAX_IP_SIZE);