there's no need to check the restriction types at run time

either there's one value (is_sequence_of=foo).

or it's a CHOICE, and all of the children are numbered options.
In which case we don't care what the values are.  If they exist,
we will find them, or they won't exist, and we will decode them
as raw octets

diff --git a/src/protocols/der/decode.c b/src/protocols/der/decode.c
index 8b7092ab944a8c120f63d17d554d7a05e8c3f423..b84213785a1e3f3202d70ec0554d855ede6d2a22 100644 (file)
--- a/src/protocols/der/decode.c
+++ b/src/protocols/der/decode.c
@@ -773,41 +773,18 @@ static ssize_t fr_der_decode_sequence(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_d
        }
 
        /*
-        *      This is a sequence-of, meaning there are restrictions on the types which can be present
+        *      This is a sequence-of, which means it either has only one child, or it's a sequence_of=choice,
+        *      and all of the children are numbered options.
         */
        if (unlikely(flags->is_sequence_of)) {
-               bool restriction_types[FR_DER_TAG_MAX] = { };
-
                if (flags->sequence_of != FR_DER_TAG_CHOICE) {
-                       fr_assert(flags->sequence_of < FR_DER_TAG_MAX);
-
-                       restriction_types[flags->sequence_of] = true;
                        child = fr_dict_attr_iterate_children(parent, &child);
                        if (!child) {
                                fr_strerror_printf_push("Sequence %s has no children", parent->name);
+                       error:
+                               talloc_free(vp);
                                return -1;
                        }
-
-               } else {
-                       /*
-                        *      If it is a sequence of choices, then we must construct the list of restriction_types.
-                        *      This will be a list of the number of choices, starting at 0.
-                        */
-                       fr_dict_attr_t const *choices = NULL;
-                       fr_dict_attr_t const *ref;
-
-                       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;
-                       }
-
-                       while ((choices = fr_dict_attr_iterate_children(ref, &choices))) {
-                               fr_assert(choices->attr < FR_DER_TAG_MAX);
-                               restriction_types[choices->attr] = true;
-                       }
                }
 
                /*
@@ -823,36 +800,40 @@ static ssize_t fr_der_decode_sequence(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_d
 
                        current_tag = (tag_byte & DER_TAG_CONTINUATION); /* always <= FR_DER_TAG_MAX */
 
-                       if (unlikely(!restriction_types[current_tag])) {
-                               fr_strerror_printf_push("Attribute %s is a sequence-of which does not allow DER type '%s'", parent->name,
-                                                  fr_der_tag_to_str(current_tag));
-                       error:
-                               talloc_free(vp);
-                               return -1;
-
-                       }
-
                        /*
                         *      If we have a choice, the children are numbered.
                         */
                        if (unlikely(flags->sequence_of == FR_DER_TAG_CHOICE)) {
-                               child = fr_dict_attr_child_by_num(parent, current_tag);
-                               if (!child) {
-                                       child = fr_dict_attr_unknown_raw_afrom_num(decode_ctx->tmp_ctx, parent, current_tag);
-                                       if (!child) return -1;
-                               }
-
                                /*
                                 *      Options always have to have the context field set.
                                 */
                                if ((tag_byte & DER_TAG_CLASS_MASK) != FR_DER_CLASS_CONTEXT) {
                                        fr_strerror_printf_push("Tag has unexpected class %20x", tag_byte & DER_TAG_CLASS_MASK);
-                                       return -1;
+                                       goto error;
+                               }
+
+                               child = fr_dict_attr_child_by_num(parent, current_tag);
+                               if (!child) {
+                                       child = fr_dict_attr_unknown_raw_afrom_num(decode_ctx->tmp_ctx, parent, current_tag);
+                                       if (!child) goto error;
                                }
                        } else {
+                               fr_assert(flags->is_sequence_of);
+
+                               /*
+                                *      It must be a DER type, and must be only one type.
+                                */
                                if ((tag_byte & DER_TAG_CLASS_MASK) != FR_DER_CLASS_UNIVERSAL) {
                                        fr_strerror_printf_push("Tag has unexpected class %20x", tag_byte & DER_TAG_CLASS_MASK);
-                                       return -1;
+                                       goto error;
+                               }
+
+                               if (unlikely(current_tag != flags->sequence_of)) {
+                                       fr_strerror_printf_push("Attribute %s is a sequence_of=%s which does not allow DER type '%s'",
+                                                               parent->name,
+                                                               fr_der_tag_to_str(flags->sequence_of),
+                                                               fr_der_tag_to_str(current_tag));
+                                       goto error;
                                }
                        }
 
@@ -876,8 +857,8 @@ static ssize_t fr_der_decode_sequence(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_d
        }
 
        /*
-        *      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.
+        *      Decode the children.  Since it's not a sequence_of=..., we must have a random bunch of
+        *      children.  The children are packed in order.  Some may be optional.
         *
         *      We loop over all of the children, because some might have default values.
         */

diff --git a/src/tests/unit/protocols/der/base.txt b/src/tests/unit/protocols/der/base.txt
index 2757d06cc6fea3e64d6ba2a85f7be2f4189880dd..647e3dd851a8e8b77160ad7738b40c142c64e963 100644 (file)
--- a/src/tests/unit/protocols/der/base.txt
+++ b/src/tests/unit/protocols/der/base.txt
@@ -22,7 +22,7 @@ match 02 01 01 02 01 02
 proto-dictionary-root Test-Seq-Of
 
 decode-pair 30 06 02 01 01 01 01 FF
-match Attribute Test-Seq-Of is a sequence-of which does not allow DER type 'boolean'
+match Attribute Test-Seq-Of is a sequence_of=integer which does not allow DER type 'boolean'
 
 proto-dictionary-root Test-Set-Of