From 91fd884c4b6c2e3cde030f739312b71fa40761ef Mon Sep 17 00:00:00 2001
From: tb <tb@openbsd.org>
Date: Tue, 28 Dec 2021 16:26:53 +0000
Subject: [PATCH] Simplify and explain expand_addr() a bit

RFC 3779 section 2.1.2 does a decent job of explaining how IP addresses
are encoded in. What's stored amounts to a prefix with all trailing zero
octets omitted. If there are trailing zero bits in the last non-zero octet,
bs->flags & 7 indicates how many. addr_expand() expands this to an address
of length 4 or 16 depending on whether we deal with IPv4 or IPv6.

Since an address can be the lower or the upper bound of a prefix or
address range, expansion needs to be able to zero-fill or one-fill the
unused bits/octets. No other expansion is ever used, so simplify the
meaning of fill accordingly. There's no need to special case the case
that there are no unused bits, the masking/filling is a noop.

ok jsing
---
 lib/libcrypto/x509/x509_addr.c | 35 ++++++++++++++++++++++------------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/lib/libcrypto/x509/x509_addr.c b/lib/libcrypto/x509/x509_addr.c
index e66d408ffb3..038319087b3 100644
--- a/lib/libcrypto/x509/x509_addr.c
+++ b/lib/libcrypto/x509/x509_addr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: x509_addr.c,v 1.33 2021/12/28 16:21:59 tb Exp $ */
+/*	$OpenBSD: x509_addr.c,v 1.34 2021/12/28 16:26:53 tb Exp $ */
 /*
  * Contributed to the OpenSSL Project by the American Registry for
  * Internet Numbers ("ARIN").
@@ -362,31 +362,42 @@ X509v3_addr_get_afi(const IPAddressFamily *f)
 }
 
 /*
- * Expand the bitstring form of an address into a raw byte array.
- * At the moment this is coded for simplicity, not speed.
+ * Expand the bitstring form (RFC 3779, section 2.1.2) of an address into
+ * a raw byte array.  At the moment this is coded for simplicity, not speed.
+ *
+ * Unused bits in the last octet of |bs| and all bits in subsequent bytes
+ * of |addr| are set to 0 or 1 depending on whether |fill| is 0 or not.
  */
 static int
 addr_expand(unsigned char *addr, const ASN1_BIT_STRING *bs, const int length,
-    const unsigned char fill)
+    uint8_t fill)
 {
 	if (bs->length < 0 || bs->length > length)
 		return 0;
+
+	if (fill != 0)
+		fill = 0xFF;
+
 	if (bs->length > 0) {
+		/* XXX - shouldn't this check ASN1_STRING_FLAG_BITS_LEFT? */
+		uint8_t unused_bits = bs->flags & 7;
+		uint8_t mask = (1 << unused_bits) - 1;
+
 		memcpy(addr, bs->data, bs->length);
-		if ((bs->flags & 7) != 0) {
-			unsigned char mask = 0xFF >> (8 - (bs->flags & 7));
-			if (fill == 0)
-				addr[bs->length - 1] &= ~mask;
-			else
-				addr[bs->length - 1] |= mask;
-		}
+
+		if (fill == 0)
+			addr[bs->length - 1] &= ~mask;
+		else
+			addr[bs->length - 1] |= mask;
 	}
+
 	memset(addr + bs->length, fill, length - bs->length);
+
 	return 1;
 }
 
 /*
- * Extract the prefix length from a bitstring.
+ * Extract the prefix length from a bitstring: 8 * length - unused bits.
  */
 #define addr_prefixlen(bs) ((int) ((bs)->length * 8 - ((bs)->flags & 7)))
 
-- 
2.20.1