]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
ensure that sets are only TLVs
authorAlan T. DeKok <aland@freeradius.org>
Wed, 26 Feb 2025 18:46:30 +0000 (13:46 -0500)
committerAlan T. DeKok <aland@freeradius.org>
Wed, 26 Feb 2025 19:40:41 +0000 (14:40 -0500)
nothing in the standard dictionaries uses SETs as groups.
They're groups only in the test dictionaries.  And there is no
encoding difference between SET of tlv and SET of group.

src/protocols/der/base.c
src/protocols/der/decode.c
src/protocols/der/encode.c
src/tests/unit/protocols/der/base.txt
src/tests/unit/protocols/der/dictionary.test

index 276969de5ca1bc0777aa10f822b3f8c8b62be497..3fdf90aa3612b5a8185522a601c6d3ed59f10e0a 100644 (file)
@@ -156,7 +156,6 @@ static const bool *fr_type_to_der_tags[FR_DER_TAG_MAX] = {
        },
        [FR_TYPE_GROUP] = (bool [FR_DER_TAG_MAX]) {
                [FR_DER_TAG_SEQUENCE] = true,
-               [FR_DER_TAG_SET] = true,
        },
 };
 
index 146edb3f9990c65763536dd8e2e4a46907bab0ea..f0305bc1d4014cc5c87c8c4d1ae742db374200ef 100644 (file)
@@ -814,6 +814,8 @@ static ssize_t fr_der_decode_sequence(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_d
 
                        if (fr_type_is_group(parent->type)) {
                                ref = fr_dict_attr_ref(parent);
+                               fr_assert(fr_der_flag_der_type(ref) == FR_DER_TAG_SEQUENCE);
+
                        } else {
                                ref = parent;
                        }
@@ -880,8 +882,6 @@ static ssize_t fr_der_decode_sequence(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_d
         *      children are packed in order.  Some may be optional.
         *
         *      We loop over all of the children, because some might have default values.
-        *
-        *      @todo - implement OPTIONAL flag.
         */
        while ((child = fr_dict_attr_iterate_children(parent, &child))) {
                ssize_t ret;
@@ -1066,8 +1066,6 @@ static ssize_t fr_der_decode_set(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_a
        /*
         *      Decode the children.  Since it's not a sequence_of=..., we must have a set of children.  The
         *      children are packed in order.  Some may be optional.
-        *
-        *      @todo - implement OPTIONAL flag.
         */
        while ((child = fr_dict_attr_iterate_children(parent, &child))) {
                ssize_t  ret;
@@ -2451,23 +2449,13 @@ static ssize_t fr_der_decode_pair_dbuff(TALLOC_CTX *ctx, fr_pair_list_t *out, fr
                        return 0;
                }
 
+               /*
+                *      We will store the value as raw octets if indicated by the dictionary
+                *
+                *      This is for large 'integer' types, such as serialNumber.
+                */
                if (fr_type_is_octets(parent->type)) {
-                       /*
-                        *      We will store the value as raw octets if indicated by the dictionary
-                        *
-                        *      This is mainly for large 'integer' types, such as serialNumber.
-                        */
                        tag = FR_DER_TAG_OCTETSTRING;
-
-               } else if (!(fr_type_to_der_tag_valid(parent->type, tag) && fr_type_is_structural(parent->type))) {
-                       /*
-                        *      If this is not a sequence/set/structure like thing, then it does not have children that
-                        *      could have defaults.
-                        */
-                       fr_strerror_printf("Attribute %s of DER type '%s' cannot store DER type '%s'", parent->name,
-                                          fr_der_tag_to_str(flags->der_type),
-                                          fr_der_tag_to_str(tag));
-                       return -1;
                }
        }
 
index 409c3a9ed561f621c7f14cd3327d4683a3d126b9..153aff2522f7e30b69b30e1b4b6520d589d9c338 100644 (file)
@@ -789,7 +789,6 @@ static ssize_t fr_der_encode_set(fr_dbuff_t *dbuff, fr_dcursor_t *cursor, fr_der
        fr_pair_t            *vp;
        fr_da_stack_t         da_stack;
        fr_dcursor_t          child_cursor;
-       fr_dict_attr_t const *ref   = NULL;
        ssize_t               slen  = 0;
        unsigned int          depth = 0;
 
@@ -797,7 +796,7 @@ static ssize_t fr_der_encode_set(fr_dbuff_t *dbuff, fr_dcursor_t *cursor, fr_der
        PAIR_VERIFY(vp);
        fr_assert(!vp->da->flags.is_raw);
 
-       fr_assert(fr_type_is_group(vp->vp_type) || fr_type_is_tlv(vp->vp_type));
+       fr_assert(fr_type_is_tlv(vp->vp_type));
 
        /*
         *      ISO/IEC 8825-1:2021
@@ -827,31 +826,6 @@ static ssize_t fr_der_encode_set(fr_dbuff_t *dbuff, fr_dcursor_t *cursor, fr_der
         *                      encodings.
         */
 
-       if (fr_type_is_group(vp->vp_type)) {
-               /*
-                *      Groups could be also be a pair, so we need to check for that.
-                */
-               if (fr_der_flag_is_pair(vp->da)) {
-                       slen = fr_der_encode_oid_value_pair(&our_dbuff, cursor, encode_ctx);
-                       if (slen < 0) {
-                               fr_strerror_printf("Failed to encode OID value pair: %s", fr_strerror());
-                               return -1;
-                       }
-
-                       return fr_dbuff_set(dbuff, &our_dbuff);
-               }
-
-
-               /*
-                *      Check that the group is not referencing a non-DER-thing.
-                */
-               ref = fr_dict_attr_ref(vp->da);
-               if (ref && (ref->dict != dict_der)) {
-                       fr_strerror_printf("Group %s is not a DER group", ref->name);
-                       return -1;
-               }
-       }
-
        if (fr_der_flag_is_set_of(vp->da)) {
                /*
                 *      Set-of items will all have the same tag, so we need to sort them lexicographically
@@ -933,6 +907,9 @@ static ssize_t fr_der_encode_set(fr_dbuff_t *dbuff, fr_dcursor_t *cursor, fr_der
                goto free_and_return;
        }
 
+       /*
+        *      @todo - this gives a partial order.  It does NOT sort by comparing the encoded data.
+        */
        fr_pair_list_sort(&vp->children, fr_der_pair_cmp_by_da_tag);
 
        fr_proto_da_stack_build(&da_stack, vp->da);
index 0572ec9a58a771acf1ef7a4f6e020d60f0bbb5f1..da4b8b88a8337f17107c4eab883d9ad278f5b06b 100644 (file)
@@ -793,12 +793,6 @@ match 31 06 01 01 00 02 01 09
 encode-pair Test-Set-TLV  = { Test-Integer = 9, Test-Boolean = yes }
 match 31 06 01 01 ff 02 01 09
 
-encode-pair Test-Set-GROUP = { Test-Integer = 9 }
-match 31 03 02 01 09
-
-encode-pair Test-Set-GROUP = { Test-Integer = 9, Test-Boolean = yes }
-match 31 06 01 01 ff 02 01 09
-
 encode-pair Test-Oid = "2.999.3"
 match 06 03 88 37 03
 
@@ -865,9 +859,6 @@ match 31 07 02 01 01 02 02 0a 14
 encode-pair Test-Set-Of = { Test-First-Integer = 2, Test-First-Integer = 1 }
 match 31 06 02 01 01 02 01 02
 
-encode-pair Test-Set-Of-Group = { Test-First-Integer = 2, Test-First-Integer = 1 }
-match 31 06 02 01 01 02 01 02
-
 encode-pair Foo-Bar = { Test-Integer = 1, Test-Boolean = yes }
 match 30 03 01 01 ff
 
@@ -910,4 +901,4 @@ decode-pair 03 01 00
 match Test-IPv4-Prefix = 0.0.0.0/0
 
 count
-match 549
+match 543
index 06e8971adcae114843f015add432d3377b40feae..0677a6966fccb640c31fed8db89afa673cbe0d0d 100644 (file)
@@ -24,8 +24,6 @@ BEGIN Test-Set-Of
 DEFINE Test-First-Integer                              integer
 END Test-Set-Of
 
-DEFINE Test-Set-Of-Group                               group ref=@.Test-Set-Of,der_type=set,set_of=integer
-
 DEFINE Test-Boolean                                    bool
 
 DEFINE Test-Integer                                    integer
@@ -177,5 +175,3 @@ BEGIN Test-Set-TLV
 DEFINE Test-Integer                                    integer
 DEFINE Test-Boolean                                    bool
 END Test-Set-TLV
-
-DEFINE Test-Set-GROUP                                  group der_type=set,ref=@.Test-TLV