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

diff --git a/lib/libcrypto/x509/x509_addr.c b/lib/libcrypto/x509/x509_addr.c
index e66d408..0383190 100644 (file)
--- 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)))