From: Alan T. DeKok Date: Thu, 21 Nov 2024 16:16:23 +0000 (-0500) Subject: correct encoding / decode of prefixes X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0cb02fa2de3b6b6cf1d81287add859ddad433fc3;p=thirdparty%2Ffreeradius-server.git correct encoding / decode of prefixes and add tests --- diff --git a/src/lib/util/cbor.c b/src/lib/util/cbor.c index 8551ce5f3d8..04fe81fbd37 100644 --- 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 2bf26eef07c..1f6a81da975 100644 --- 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 1a58cf46455..933b3b43283 100644 --- 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