correct encoding / decode of prefixes

and add tests

diff --git a/src/lib/util/cbor.c b/src/lib/util/cbor.c
index 8551ce5f3d8990730d72c1afcd3eaa9155c107a4..04fe81fbd372dd2e233cfa5ecdb213b41b1aa99b 100644 (file)
--- a/src/lib/util/cbor.c
+++ b/src/lib/util/cbor.c
@@ -67,6 +67,7 @@ static ssize_t cbor_encode_integer(fr_dbuff_t *dbuff, uint8_t type, uint64_t dat
        fr_dbuff_t      work_dbuff = FR_DBUFF(dbuff);
        uint8_t         value[8];
 
+       fr_assert(type < 8);
        type <<= 5;
 
        if (data < 24) {
@@ -164,6 +165,7 @@ ssize_t fr_cbor_encode_value_box(fr_dbuff_t *dbuff, fr_value_box_t *vb)
        uint64_t        data;
        int64_t         neg;
        ssize_t         slen;
+       uint8_t         const *p, *end;
 
        switch (vb->type) {
        case FR_TYPE_BOOL:
@@ -344,7 +346,24 @@ ssize_t fr_cbor_encode_value_box(fr_dbuff_t *dbuff, fr_value_box_t *vb)
                slen = cbor_encode_integer(&work_dbuff, CBOR_INTEGER, vb->vb_ip.prefix);
                if (slen <= 0) return slen;
 
-               slen = cbor_encode_octets(&work_dbuff, (uint8_t const *) &vb->vb_ip.addr.v4.s_addr, 4);
+               p = (uint8_t const *) &vb->vb_ip.addr.v4.s_addr;
+               end = p + 3;
+
+               /*
+                *      RFC 9164 Section 4.2 - Drop lower octets which are all zero.
+                *
+                *      If this results in a zero-length string, so be it.
+                *
+                *      Note also that "There is no relationship between the number of bytes omitted and the
+                *      prefix length."
+                */
+               do {
+                       if (*end != 0) break;
+
+                       end--;
+               } while (end != p);
+
+               slen = cbor_encode_octets(&work_dbuff, p, (end - p) + (*end != 0));
                if (slen <= 0) return slen;
                break;
 
@@ -363,7 +382,24 @@ ssize_t fr_cbor_encode_value_box(fr_dbuff_t *dbuff, fr_value_box_t *vb)
                slen = cbor_encode_integer(&work_dbuff, CBOR_INTEGER, vb->vb_ip.prefix);
                if (slen <= 0) return slen;
 
-               slen = cbor_encode_octets(&work_dbuff, (uint8_t const *) &vb->vb_ip.addr.v6.s6_addr, 16);
+               p = (uint8_t const *) &vb->vb_ip.addr.v6.s6_addr;
+               end = p + 15;
+
+               /*
+                *      RFC 9164 Section 4.2 - Drop lower octets which are all zero.
+                *
+                *      If this results in a zero-length string, so be it.
+                *
+                *      Note also that "There is no relationship between the number of bytes omitted and the
+                *      prefix length."
+                */
+               do {
+                       if (*end != 0) break;
+
+                       end--;
+               } while (end != p);
+
+               slen = cbor_encode_octets(&work_dbuff, p, (end - p) + (*end != 0));
                if (slen <= 0) return slen;
 
                break;
@@ -488,7 +524,7 @@ static ssize_t cbor_decode_count(uint64_t *out, int expected, fr_dbuff_t *dbuff)
 
 typedef ssize_t (*cbor_decode_type_t)(TALLOC_CTX *ctx, fr_value_box_t *vb, fr_dbuff_t *dbuff);
 
-static ssize_t cbor_decode_octets_memcpy(uint8_t *dst, size_t dst_len, fr_dbuff_t *dbuff)
+static ssize_t cbor_decode_octets_memcpy(uint8_t *dst, size_t dst_min, size_t dst_max, fr_dbuff_t *dbuff)
 {
        fr_dbuff_t work_dbuff = FR_DBUFF(dbuff);
        ssize_t slen;
@@ -497,12 +533,17 @@ static ssize_t cbor_decode_octets_memcpy(uint8_t *dst, size_t dst_len, fr_dbuff_
        slen = cbor_decode_count(&value, CBOR_OCTETS, &work_dbuff);
        if (slen < 0) return slen;
 
-       if (value != dst_len) {
-               fr_strerror_printf("Invalid length for data - expected %zu got %" PRIu64, dst_len, value);
+       if (value < dst_min) {
+               fr_strerror_printf("Invalid length for data - expected at least %zu got %" PRIu64, dst_min, value);
+               return -1;
+       }
+
+       if (value > dst_max) {
+               fr_strerror_printf("Invalid length for data - expected no more than %zu got %" PRIu64, dst_max, value);
                return -1;
        }
 
-       FR_DBUFF_OUT_MEMCPY_RETURN(dst, &work_dbuff, dst_len);
+       FR_DBUFF_OUT_MEMCPY_RETURN(dst, &work_dbuff, value);
 
        return fr_dbuff_set(dbuff, &work_dbuff);
 }
@@ -515,7 +556,7 @@ static ssize_t *cbor_decode_octets_memdup(TALLOC_CTX *ctx, uint8_t **out, fr_dbu
        uint64_t value;
        uint8_t *ptr;
 
-       slen = cbor_decode_count(&value, CBOR_OCTETS&work_dbuff);
+       slen = cbor_decode_count(&value, CBOR_OCTETS, &work_dbuff);
        if (slen < 0) return slen;
 
        if (value > (1 << 20)) {
@@ -538,18 +579,20 @@ static ssize_t *cbor_decode_octets_memdup(TALLOC_CTX *ctx, uint8_t **out, fr_dbu
 
 static ssize_t cbor_decode_ethernet(UNUSED TALLOC_CTX *ctx, fr_value_box_t *vb, fr_dbuff_t *dbuff)
 {
-       return cbor_decode_octets_memcpy(vb->vb_ether, sizeof(vb->vb_ether), dbuff);
+       return cbor_decode_octets_memcpy(vb->vb_ether, sizeof(vb->vb_ether), sizeof(vb->vb_ether), dbuff);
 }
 
 static ssize_t cbor_decode_ipv4_addr(UNUSED TALLOC_CTX *ctx, fr_value_box_t *vb, fr_dbuff_t *dbuff)
 {
        return cbor_decode_octets_memcpy((uint8_t *) &vb->vb_ip.addr.v4.s_addr,
+                                        sizeof(vb->vb_ip.addr.v4.s_addr),
                                         sizeof(vb->vb_ip.addr.v4.s_addr), dbuff);
 }
 
 static ssize_t cbor_decode_ipv6_addr(UNUSED TALLOC_CTX *ctx, fr_value_box_t *vb, fr_dbuff_t *dbuff)
 {
        return cbor_decode_octets_memcpy((uint8_t *) &vb->vb_ip.addr.v6.s6_addr,
+                                        sizeof(vb->vb_ip.addr.v6.s6_addr),
                                         sizeof(vb->vb_ip.addr.v6.s6_addr), dbuff);
 }
 
@@ -559,6 +602,7 @@ static ssize_t cbor_decode_ipv4_prefix(UNUSED TALLOC_CTX *ctx, fr_value_box_t *v
        ssize_t slen;
        uint8_t header;
        uint64_t value = 0;
+       uint8_t buffer[sizeof(vb->vb_ip.addr.v4.s_addr)];
 
        FR_DBUFF_OUT_RETURN(&header, &work_dbuff);
 
@@ -568,8 +612,8 @@ static ssize_t cbor_decode_ipv4_prefix(UNUSED TALLOC_CTX *ctx, fr_value_box_t *v
                return -1;
        }
 
-       slen = cbor_decode_count(&value, CBOR_OCTETS, &work_dbuff);
-       if (slen < 0) return slen - fr_dbuff_used(&work_dbuff);
+       slen = cbor_decode_count(&value, CBOR_INTEGER, &work_dbuff);
+       if (slen <= 0) return slen - fr_dbuff_used(&work_dbuff);
 
        if (value > 32) {
                fr_strerror_printf("Invalid IPv4 prefix - expected prefix < 32, got %" PRIu64, value);
@@ -577,12 +621,19 @@ static ssize_t cbor_decode_ipv4_prefix(UNUSED TALLOC_CTX *ctx, fr_value_box_t *v
        }
 
        /*
-        *      We encode the entire IP.  But maybe others don't?
+        *      RFC 9164 Section 4.3 - Trailing bytes of zero are omitted, so we
+        *      first copy the data to a fixed-sized buffer which was
+        *      zeroed out, and then (@todo) also check that unused bits in
+        *      the last byte are all zero.
         */
-       slen = cbor_decode_octets_memcpy((uint8_t *) &vb->vb_ip.addr.v4.s_addr,
-                                        sizeof(vb->vb_ip.addr.v4.s_addr), &work_dbuff);
+       memset(buffer, 0, sizeof(buffer));
+
+       slen = cbor_decode_octets_memcpy(buffer, 0, sizeof(buffer), &work_dbuff);
        if (slen <= 0) return slen - fr_dbuff_used(&work_dbuff);
 
+       memcpy((uint8_t *) &vb->vb_ip.addr.v4.s_addr, buffer, sizeof(vb->vb_ip.addr.v4.s_addr));
+       vb->vb_ip.prefix = value;
+
        return fr_dbuff_set(dbuff, &work_dbuff);
 }
 
@@ -592,6 +643,7 @@ static ssize_t cbor_decode_ipv6_prefix(UNUSED TALLOC_CTX *ctx, fr_value_box_t *v
        ssize_t slen;
        uint8_t header;
        uint64_t value = 0;
+       uint8_t buffer[sizeof(vb->vb_ip.addr.v6.s6_addr)];
 
        FR_DBUFF_OUT_RETURN(&header, &work_dbuff);
 
@@ -601,8 +653,8 @@ static ssize_t cbor_decode_ipv6_prefix(UNUSED TALLOC_CTX *ctx, fr_value_box_t *v
                return -1;
        }
 
-       slen = cbor_decode_count(&value, CBOR_OCTETS, &work_dbuff);
-       if (slen < 0) return slen - fr_dbuff_used(&work_dbuff);
+       slen = cbor_decode_count(&value, CBOR_INTEGER, &work_dbuff);
+       if (slen <= 0) return slen - fr_dbuff_used(&work_dbuff);
 
        if (value > 128) {
                fr_strerror_printf("Invalid IPv6 prefix - expected prefix < 128, got %" PRIu64, value);
@@ -610,12 +662,19 @@ static ssize_t cbor_decode_ipv6_prefix(UNUSED TALLOC_CTX *ctx, fr_value_box_t *v
        }
 
        /*
-        *      We encode the entire IP.  But maybe others don't?
+        *      RFC 9164 Section 4.3 - Trailing bytes of zero are omitted, so we
+        *      first copy the data to a fixed-sized buffer which was
+        *      zeroed out, and then (@todo) also check that unused bits in
+        *      the last byte are all zero.
         */
-       slen = cbor_decode_octets_memcpy((uint8_t *) &vb->vb_ip.addr.v6.s6_addr,
-                                        sizeof(vb->vb_ip.addr.v6.s6_addr), &work_dbuff);
+       memset(buffer, 0, sizeof(buffer));
+
+       slen = cbor_decode_octets_memcpy(buffer, 0, sizeof(buffer), &work_dbuff);
        if (slen <= 0) return slen - fr_dbuff_used(&work_dbuff);
 
+       memcpy((uint8_t *) &vb->vb_ip.addr.v6.s6_addr, buffer, sizeof(vb->vb_ip.addr.v6.s6_addr));
+       vb->vb_ip.prefix = value;
+
        return fr_dbuff_set(dbuff, &work_dbuff);
 }
 

diff --git a/src/tests/keywords/cbor b/src/tests/keywords/cbor
index 2bf26eef07c684767e1aa20b019a2e981af12bba..1f6a81da9750e0c3207de792fb49fe2cf5ed453f 100644 (file)
--- a/src/tests/keywords/cbor
+++ b/src/tests/keywords/cbor
@@ -7,11 +7,11 @@ cbor = %cbor.encode(User-Name)
 #  9f          array of indefinite length
 #    a1                map of lenght 1
 #      01      key is integer, value 1 (User-Name)
-#      43      value is string, of length 3
+#      63      value is string, of length 3
 #        626f62 string 'bob'
 #  ff          end of indefinite array
 #
-if (cbor != 0x9fa10143626f62ff) {
+if (cbor != 0x9fa10163626f62ff) {
        test_fail
 }
 

diff --git a/src/tests/unit/protocols/cbor/base.txt b/src/tests/unit/protocols/cbor/base.txt
index 1a58cf464557e4479fe6ce8e5030037b6c81a921..933b3b4328353de5e90f844cf6733cab5fb58e4f 100644 (file)
--- a/src/tests/unit/protocols/cbor/base.txt
+++ b/src/tests/unit/protocols/cbor/base.txt
@@ -98,5 +98,36 @@ match 9f a1 18 1a 9f a1 19 19 7f f6 ff ff
 decode-pair -
 match Vendor-Specific = { Nokia-SR = { } }
 
+# 97 is the attribute number
+encode-pair PMIP6-Home-HN-Prefix = ::/8
+match 9f a1 18 97 d8 36 82 08 40 ff
+
+# 9b is the attribute number
+encode-pair PMIP6-Home-IPv4-HoA = 0/8
+match 9f a1 18 9b d8 34 82 08 40 ff
+
+#
+#  Prefixes get trailing zeros dropped, no matter what the prefix says.
+#
+#  e.g. ::/128 gets encoded as prefix=128, octets=""
+#
+encode-pair PMIP6-Home-IPv4-HoA = 192/8
+match 9f a1 18 9b d8 34 82 08 41 c0 ff
+
+decode-pair -
+match PMIP6-Home-IPv4-HoA = 192.0.0.0/8
+
+encode-pair PMIP6-Home-IPv4-HoA = 192.0.2/24
+match 9f a1 18 9b d8 34 82 18 18 43 c0 00 02 ff
+
+decode-pair -
+match PMIP6-Home-IPv4-HoA = 192.0.2.0/24
+
+encode-pair PMIP6-Home-HN-Prefix = ::/128
+match 9f a1 18 97 d8 36 82 18 80 40 ff
+
+decode-pair -
+match PMIP6-Home-HN-Prefix = ::/128
+
 count
-match 43
+match 59