allow "length=uint8" for structs

and update encode / decode of struct

and ensure that the encoders don't add too many lengths

diff --git a/src/lib/util/dict_validate.c b/src/lib/util/dict_validate.c
index 03bd829b146e122ba310554a88e9dd799ba4ce7a..eef34c956c3e82103703cfbbac3bfc8f3109350f 100644 (file)
--- a/src/lib/util/dict_validate.c
+++ b/src/lib/util/dict_validate.c
@@ -196,7 +196,7 @@ bool dict_attr_flags_valid(fr_dict_t *dict, fr_dict_attr_t const *parent,
                case FR_TYPE_UINT32:
                case FR_TYPE_UINT64:
                        if ((flags->subtype != FLAG_KEY_FIELD) && (flags->subtype != FLAG_BIT_FIELD)) {
-                               fr_strerror_const("Invalid type for extra flag.");
+                               fr_strerror_const("Invalid type (not 'key' field or 'bit' field) for extra flag.");
                                return false;
                        }
 
@@ -230,7 +230,7 @@ bool dict_attr_flags_valid(fr_dict_t *dict, fr_dict_attr_t const *parent,
                                if ((flags->subtype != FLAG_LENGTH_UINT8) && (flags->subtype != FLAG_LENGTH_UINT16)) goto invalid_extra;
                        } else if (flags->subtype) {
                        invalid_extra:
-                               fr_strerror_const("Invalid type for extra flag.");
+                               fr_strerror_const("Invalid type (not 'length=...') for extra flag.");
                                return false;
                        }
 
@@ -239,8 +239,8 @@ bool dict_attr_flags_valid(fr_dict_t *dict, fr_dict_attr_t const *parent,
                        break;
 
                case FR_TYPE_STRUCT:
-                       if (flags->subtype != FLAG_LENGTH_UINT16) {
-                               fr_strerror_const("Invalid type for extra flag.");
+                       if ((flags->subtype != FLAG_LENGTH_UINT8) && (flags->subtype != FLAG_LENGTH_UINT16)) {
+                               fr_strerror_const("Invalid type (not 'length=...') for extra flag.");
                                return false;
                        }
 

diff --git a/src/lib/util/struct.c b/src/lib/util/struct.c
index 372351a61511608fd47d0fbb062049177dc6c984..afe5eb2c896203c84eff5f5ee64c3f886f665bd5 100644 (file)
--- a/src/lib/util/struct.c
+++ b/src/lib/util/struct.c
@@ -110,26 +110,37 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
         *      Decode structs with length prefixes.
         */
        if (da_is_length_field(parent)) {
-               size_t struct_len;
+               size_t struct_len, need;
 
-               if ((end - p) < 2) {
+               if (parent->flags.subtype == FLAG_LENGTH_UINT8) {
+                       need = 1;
+               } else {
+                       need = 2;
+               }
+
+               if ((size_t) (end - p) < need) {
                        FR_PROTO_TRACE("Insufficient room for length header");
                        goto unknown;
                }
 
-               struct_len = (p[0] << 8) | p[1];
-               if ((p + struct_len + 2) > end) {
+               struct_len = p[0];
+               if (need > 1) {
+                       struct_len <<= 8;
+                       struct_len |= p[1];
+               }
+
+               if ((p + struct_len + need) > end) {
                        FR_PROTO_TRACE("Length header is larger than remaining data");
                        goto unknown;
                }
 
                /*
-                *      Skip the "length" field, and tell the decoder
-                *      to stop at the end of the length field.
+                *      Skip the length field, and tell the decoder to
+                *      stop at the end of the data we're supposed to decode.
                 */
-               p += 2;
+               p += need;
                end = p + struct_len;
-               data_len = struct_len + 2;
+               data_len = struct_len + need;
        }
 
        while (p < end) {
@@ -532,7 +543,11 @@ ssize_t fr_struct_to_network(fr_dbuff_t *dbuff,
         *      Some structs are prefixed by a 16-bit length.
         */
        if (da_is_length_field(parent)) {
-               FR_DBUFF_ADVANCE_RETURN(&work_dbuff, 2);
+               if (parent->flags.subtype == FLAG_LENGTH_UINT8) {
+                       FR_DBUFF_ADVANCE_RETURN(&work_dbuff, 1);
+               } else {
+                       FR_DBUFF_ADVANCE_RETURN(&work_dbuff, 2);
+               }
                do_length = true;
        }
 
@@ -778,12 +793,30 @@ done:
        }
 
        if (do_length) {
-               uint32_t len = fr_dbuff_used(&work_dbuff) - 2;
-               if (len > 65535) {
+               size_t need, max, len;
+
+               /*
+                *      @todo - maybe just limit the size of the work_dbuff.
+                */
+               if (parent->flags.subtype == FLAG_LENGTH_UINT8) {
+                       need = 1;
+                       max = 256;
+               } else {
+                       need = 2;
+                       max = 65536;
+               }
+
+               len = fr_dbuff_used(&work_dbuff) - need;
+               if (len > max) {
                        fr_strerror_const("Structure size is too large for 16-bit length field.");
                        return -1;
                }
-               fr_dbuff_in(&hdr_dbuff, (uint16_t)len);
+
+               if (need == 1) {
+                       fr_dbuff_in(&hdr_dbuff, (uint8_t)len);
+               } else {
+                       fr_dbuff_in(&hdr_dbuff, (uint16_t)len);
+               }
        }
 
        /*

diff --git a/src/protocols/dhcpv4/encode.c b/src/protocols/dhcpv4/encode.c
index 2712dea6a6b744eb323115d2c851880f60bbf1c1..3bb6f2bc2aa7861403c5b94076ca3730c7f72461 100644 (file)
--- a/src/protocols/dhcpv4/encode.c
+++ b/src/protocols/dhcpv4/encode.c
@@ -238,8 +238,10 @@ static ssize_t encode_array(fr_dbuff_t *dbuff,
                /*
                 *      If the data is variable length i.e. strings or octets
                 *      we need to include an 8-bit length field before each element.
+                *
+                *      The struct encoder adds it's own length field.
                 */
-               if (da_is_length_field(da)) {
+               if (da_is_length_field(da) && (da->type != FR_TYPE_STRUCT)) {
                        len_field = true;
                        FR_DBUFF_ADVANCE_RETURN(&element_dbuff, sizeof(uint8_t));       /* Make room for the length field */
                }

diff --git a/src/protocols/dhcpv6/encode.c b/src/protocols/dhcpv6/encode.c
index 76fa0f27d595f3a8e2a61adb21622723bbcf5158..f7acd815c352d354b9a23b549156cf1424fe2076 100644 (file)
--- a/src/protocols/dhcpv6/encode.c
+++ b/src/protocols/dhcpv6/encode.c
@@ -435,8 +435,10 @@ static inline ssize_t encode_array(fr_dbuff_t *dbuff,
                 *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
                 *   |       text-len                |        String                 |
                 *   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-...-+-+-+-+-+-+-+
+                *
+                *      The struct encoder already adds the length field.
                 */
-               if (da_is_length_field(da)) {
+               if (da_is_length_field(da) && (da->type != FR_TYPE_STRUCT)) {
                        len_field = true;
                        FR_DBUFF_ADVANCE_RETURN(&element_dbuff, sizeof(uint16_t));      /* Make room for the length field */
                }