allow raw attributes in structs

but enforce that the length is correct, so that the parent struct
is not malformed.

Only encode a struct member once, even if the admin specifies it
multiple times.

update tests to match

diff --git a/src/lib/util/dict_unknown.c b/src/lib/util/dict_unknown.c
index 8834ef15f3c5de913cdb3436bb2d865986f08938..98351278da1b0e109a207f44c98197c0845c19a8 100644 (file)
--- a/src/lib/util/dict_unknown.c
+++ b/src/lib/util/dict_unknown.c
@@ -71,10 +71,8 @@ static int dict_attr_unknown_init(fr_dict_attr_t const *parent, UNUSED fr_dict_a
                }
        }
 
-#if 0
        /*
-        *      @todo - This is commented out because it changes some of the debug output, and we want to
-        *      separate commits into logically associated ones.
+        *      Ensure that raw members of a structure have the correct length.
         */
        if (parent->type == FR_TYPE_STRUCT) {
                if (!da) {
@@ -93,12 +91,14 @@ static int dict_attr_unknown_init(fr_dict_attr_t const *parent, UNUSED fr_dict_a
 
                } else if (da->type != type) {
                cannot_change_type:
+                       /*
+                        *      @todo - why not?  So long as the size is the same...
+                        */
                        fr_strerror_printf("Cannot create 'raw' attribute in 'struct' which changes data type from '%s' to '%s'",
                                           fr_type_to_str(da->type), fr_type_to_str(type));
                        return -1;
                }
        }
-#endif
 
        return 0;
 }

diff --git a/src/lib/util/struct.c b/src/lib/util/struct.c
index 5a091ed231945b55fd8d460675d10ff3602d96d4..771ee0b75dc2e4484a6eed9ea2dbf1beacf933f2 100644 (file)
--- a/src/lib/util/struct.c
+++ b/src/lib/util/struct.c
@@ -229,8 +229,7 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                         *      of the input is suspect.
                         */
                        if (child_length > (size_t) (end - p)) {
-                               FR_PROTO_TRACE("fr_struct_from_network - child length %zu overflows buffer", child_length);
-                               goto remainder_raw;
+                               child_length = (size_t) (end - p);
                        }
                }
 
@@ -299,7 +298,7 @@ ssize_t fr_struct_from_network(TALLOC_CTX *ctx, fr_pair_list_t *out,
                        fr_assert(!fr_dict_attr_child_by_num(parent, child_num + 1)); /* has to be the last one */
                        if (!key_vp) {
                        remainder_raw:
-                               child_length = end - p;
+                               child_length = (size_t) (end - p);
                                goto raw;
                        }
 
@@ -758,6 +757,7 @@ ssize_t fr_struct_to_network(fr_dbuff_t *dbuff,
        bool                    do_length = false;
        uint8_t                 bit_buffer = 0;
        fr_pair_t const         *vp = fr_dcursor_current(parent_cursor);
+       fr_pair_t const         *last = NULL;
        fr_pair_t const         *key_vp = NULL;
        fr_dict_attr_t const    *child, *parent, *key_da = NULL;
        fr_dcursor_t            child_cursor, *cursor;
@@ -859,6 +859,15 @@ ssize_t fr_struct_to_network(fr_dbuff_t *dbuff,
                 */
                FR_PROTO_TRACE("fr_struct_to_network child %s", child->name);
 
+               /*
+                *      If the caller specifies a member twice, then we only encode the first member.
+                */
+               while (last && vp && (last->da->parent == vp->da->parent) && (last->da->attr == vp->da->attr)) {
+                       fr_assert(last != vp);
+                       vp = fr_dcursor_next(cursor);
+               }
+               last = vp;
+
                /*
                 *      The MEMBER may be raw, in which case it is encoded as octets.
                 *
@@ -903,6 +912,7 @@ ssize_t fr_struct_to_network(fr_dbuff_t *dbuff,
                                        fr_strerror_printf("Failed encoding bit field %s", child->name);
                                        return offset;
                                }
+                               last = NULL;
                                continue;
                        }
 
@@ -917,6 +927,11 @@ ssize_t fr_struct_to_network(fr_dbuff_t *dbuff,
                         *      Zero out the unused field.
                         */
                        FR_DBUFF_MEMSET_RETURN(&work_dbuff, 0, child->flags.length);
+
+                       /*
+                        *      We didn't encode the current VP, so it's not the last one.
+                        */
+                       last = NULL;
                        continue;
                }
 
@@ -965,6 +980,10 @@ ssize_t fr_struct_to_network(fr_dbuff_t *dbuff,
                                return offset;
                        }
 
+                       /*
+                        *      We have to go to the next pair manually, as the protocol-specific
+                        *      encode_value() function will normally go to the next cursor entry.
+                        */
                        vp = fr_dcursor_next(cursor);
                        /* We need to continue, there may be more fields to encode */
 

diff --git a/src/tests/unit/protocols/dhcpv6/rfc6355.txt b/src/tests/unit/protocols/dhcpv6/rfc6355.txt
index 42d6c6361af868368e2c8c317c3af16d2a4d6171..f32cc104b7439118bb7dd9b581f90b50462f2649 100644 (file)
--- a/src/tests/unit/protocols/dhcpv6/rfc6355.txt
+++ b/src/tests/unit/protocols/dhcpv6/rfc6355.txt
@@ -74,7 +74,10 @@ match 00 01 00 12 00 04 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d 00 00
 
 #  And if we see something that's too short, we get a bad attribute.
 decode-pair 00 01 00 10 00 04 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d
-match Client-ID = { DUID = ::UUID, UUID = { raw.Value = 0x000102030405060708090a0b0c0d } }
+match Client-ID = { DUID = ::UUID, Value = { raw.4 = 0x000102030405060708090a0b0c0d } }
+
+encode-pair -
+match 00 01 00 10 00 04 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d
 
 #
 #  Server Identifier
@@ -111,7 +114,7 @@ match Server-ID = { DUID = ::UUID, UUID = { Value = 0x000102030405060708090a0b0c
 
 #  And if we see something that's too short, we get a bad attribute.
 decode-pair 00 02 00 10 00 04 00 01 02 03 04 05 06 07 08 09 0a 0b 0c 0d
-match Server-ID = { DUID = ::UUID, UUID = { raw.Value = 0x000102030405060708090a0b0c0d } }
+match Server-ID = { DUID = ::UUID, Value = { raw.4 = 0x000102030405060708090a0b0c0d } }
 
 count
-match 37
+match 39

diff --git a/src/tests/unit/protocols/radius/foreign.txt b/src/tests/unit/protocols/radius/foreign.txt
index 82cf72cab68909e922934af6bbb764ceff074fc5..ab56efb48e718d21fda0b58e8f7b613660f35598 100644 (file)
--- a/src/tests/unit/protocols/radius/foreign.txt
+++ b/src/tests/unit/protocols/radius/foreign.txt
@@ -55,7 +55,7 @@ match Extended-Attribute-5 = { DHCPv6-Options = { Rapid-Commit = yes, Informatio
 #  Various cross-protocol tests taken from fuzzer output.
 #
 decode-proto 04ac00edf504000060800402f50303f5040403f5040402f50302d604040202046000f30303f5040402f502f50302d604040202046000f30303f5040402f50302040202046000f303040402280303f54404029e0200037d2c0404040404400404040402024404029e0200007d2c04040404044004210404020202046000f30303f50202046000f303029e2c7d0402040400000303f5040402f50302040202046000f303040402280303f54404029e0200037d2c040404040440040404040404020202046000f3000000e902046000f303029eaaaaaaaaaaaaaaaaaaaaaaaa2c7d04020404009004ffff60d000032ae00400000004040303f50404
-match Packet-Type = ::Accounting-Request, Packet-Authentication-Vector = 0xf504000060800402f50303f5040403f5, raw.NAS-IP-Address = 0x02f5, raw.214 = 0x0402, User-Password = "\366\356", raw.Extended-Attribute-3 = 0x03, raw.Extended-Attribute-5 = 0x0402, raw.Extended-Attribute-5 = 0x02, raw.214 = 0x0402, User-Password = "\366\356", raw.Extended-Attribute-3 = 0x03, raw.Extended-Attribute-5 = 0x0402, raw.Extended-Attribute-5 = 0x02, User-Password = "\366\356", raw.Extended-Attribute-3 = 0x04, raw.Acct-Status-Type = 0x03, Extended-Attribute-5 = { DHCPv4-Options = { PCP-IPv4-Server-Addresses = { server = { }, raw.server = 0x03 }, raw.V-I-Vendor-Specific = 0x0404040404400404040402024404029e0200007d2c04040404044004210404020202046000f30303f5020204, raw.96 = 0x, Site-specific-19 = 0x029e2c, raw.V-I-Vendor-Specific = 0x02040400 }, raw.DHCPv4-Options = 0x9e0200037d2c040404040440040404040404020202046000f3000000e902046000f303029eaaaaaaaaaaaaaaaaaaaaaaaa2c7d04020404009004ffff60d00003 }, CHAP-Password = 0xf5, raw.NAS-IP-Address = 0x02f5, User-Password = "\366\356", raw.Extended-Attribute-3 = 0x04, raw.Acct-Status-Type = 0x03
+match Packet-Type = ::Accounting-Request, Packet-Authentication-Vector = 0xf504000060800402f50303f5040403f5, raw.NAS-IP-Address = 0x02f5, raw.214 = 0x0402, User-Password = "\366\356", raw.Extended-Attribute-3 = 0x03, raw.Extended-Attribute-5 = 0x0402, raw.Extended-Attribute-5 = 0x02, raw.214 = 0x0402, User-Password = "\366\356", raw.Extended-Attribute-3 = 0x03, raw.Extended-Attribute-5 = 0x0402, raw.Extended-Attribute-5 = 0x02, User-Password = "\366\356", raw.Extended-Attribute-3 = 0x04, raw.Acct-Status-Type = 0x03, Extended-Attribute-5 = { DHCPv4-Options = { raw.PCP-IPv4-Server-Addresses = 0x0003, raw.V-I-Vendor-Specific = 0x0404040404400404040402024404029e0200007d2c04040404044004210404020202046000f30303f5020204, raw.96 = 0x, Site-specific-19 = 0x029e2c, raw.V-I-Vendor-Specific = 0x02040400 }, raw.DHCPv4-Options = 0x9e0200037d2c040404040440040404040404020202046000f3000000e902046000f303029eaaaaaaaaaaaaaaaaaaaaaaaa2c7d04020404009004ffff60d00003 }, CHAP-Password = 0xf5, raw.NAS-IP-Address = 0x02f5, User-Password = "\366\356", raw.Extended-Attribute-3 = 0x04, raw.Acct-Status-Type = 0x03
 
 
 decode-proto 03 12 02 00 00 00 00 00 00 03 03 03 03 02 fc 03 02 d7 00 04 04 02 02 02 03 02 3d 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 f5 04 04 02 02 02 04 04 04 03 f5 04 04 02 f5 03 02 d6 04 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 f5 04 04 02 02 02 04 04 04 04 04 04 04 04 f3 03 03 f5 04 04 02 f5 03 03 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 f5 04 02 02 04 04 04 04 04 04 04 04 f3 03 03 f5 04 04 02 f5 03 03 04 02 02 04 60 00 f3 03 03 f5 9a 04 02 f5 03 03 f5 f1 04 04 04 03 f5 03 02 03 03 04 02 02 00 f3 04 60 03 03 f5 04 04 02 f5 03 03 f5 04 04 02 02 02 03 02 3d 04 60 00 f3 03 03 f5 04 03 03 f5 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 03 e8 03 03 02 5c 03 03 03 03 03 03 02 5c 04 04 02 f5 03 03 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 f5 04 02 02 04 04 04 04 04 04 04 04 f3 03 03 f5 04 04 02 f5 03 03 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 f5 04 03 03 f5 04 04 02 f5 03 03 04 02 02 02 3d 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 04 02 02 04 60 00 f5 04 02 f3 03 03 04 f5 03 03 f5 04 04 02 02 02 04 04 04 03 f5 04 04 02 f5 03 02 d6 04 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 f5 04 04 02 02 02 04 04 04 04 04 04 04 04 f3 03 03 f5 04 04 02 f5 03 03 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 f5 04 03 03 f5 04 04 02 f5 03 03 04 02 02 04 60 00 f3 03 03 f5 04 04 02 f5 03 03 f5 04 04 02 02 02 03 02 46 04 2a 04 04 04 44 03 03 03 03 03 03 03 03 03 72

diff --git a/src/tests/unit/protocols/radius/struct.txt b/src/tests/unit/protocols/radius/struct.txt
index 768bf55cd782a8c25cea7434c1c86b9679131364..b6cf59a8035000700e2a6cd6807c5fb873c97cb8 100644 (file)
--- a/src/tests/unit/protocols/radius/struct.txt
+++ b/src/tests/unit/protocols/radius/struct.txt
@@ -94,14 +94,21 @@ encode-pair -
 match ff 0d 02 00 00 1a 99 ff fe fd fc fb fa
 
 #
-#  @todo - these are perhaps wrong?  The raw attribute should perhaps
-#  be ignored, or else it should be an error to create it.
+#  raw.2 has the same number as "Filler".  We don't want to encode fields twice.
 #
 encode-pair Test-Struct = { Key-Field = 2, Filler = 6809, raw.2 = 0xfffefdfcfbfa }
-match ff 0d 02 00 00 1a 99 ff fe fd fc fb fa
+match ff 07 02 00 00 1a 99
+
+decode-pair -
+match Test-Struct = { Key-Field = 2, Filler = 6809 }
 
-encode-pair Test-Struct = { Key-Field = 3, Filler = 6809, raw.2 = 0xfffefdfcfbfa }
-match ff 0d 03 00 00 1a 99 ff fe fd fc fb fa
+#
+#  The final field is too long, but we still decode it.
+#
+#  @todo - arguably we should ignore the extra data, as the contents of "data" should only be 2 octets?
+#
+decode-pair ff 0d 02 00 00 1a 99 ff fe fd fc fb fa
+match Test-Struct = { Key-Field = 2, Filler = 6809, Data = { raw.2 = 0xfffefdfcfbfa } }
 
 count
-match 50
+match 52