add flags->is_option

and clean up decoding of option vs tag.  Sometimes it might not
have done the right thing

diff --git a/src/protocols/der/base.c b/src/protocols/der/base.c
index 931a06e720844a13d08fa54022579821d003451f..5efdf9414f4cc40c675b3cdaffe3568e3dd2caef 100644 (file)
--- a/src/protocols/der/base.c
+++ b/src/protocols/der/base.c
@@ -526,6 +526,7 @@ check:
 
        flags->class = FR_DER_CLASS_CONTEXT;
        flags->option = num;
+       flags->is_option = true;
 
        return 0;
 }
@@ -695,6 +696,7 @@ static bool type_parse(fr_type_t *type_p,fr_dict_attr_t **da_p, char const *name
 
                flags->class = FR_DER_CLASS_CONTEXT;
                flags->option = 3;
+               flags->is_option = true;
 
                flags->is_sequence_of = true;
                flags->sequence_of = FR_DER_TAG_SEQUENCE;
@@ -861,13 +863,8 @@ static bool attr_valid(fr_dict_attr_t *da)
 #endif
 
                        /*
-                        *      When sequence_of=choice, it MUST also have a ref.  But we can't check that
-                        *      right now, as the ref isn't added until later.
-                        *
                         *      A group of choices is really a sequence of sequences.  i.e. x509extensions
                         *      contain only a sequence, as does sequence_of=oid_and_value.
-                        *
-                        *      @todo - fix that.
                         */
                        flags->restrictions = (1 << FR_DER_TAG_SEQUENCE);
 
@@ -886,11 +883,12 @@ static bool attr_valid(fr_dict_attr_t *da)
        if (parent->is_choice) {
                if (!flags->option) {
                        fr_assert(da->attr < FR_DER_TAG_VALUE_MAX);
+
+                       flags->class = FR_DER_CLASS_CONTEXT;
                        flags->option = da->attr;
+                       flags->is_option = true;
                }
 
-               flags->class = FR_DER_CLASS_CONTEXT;
-
                if ((parent->restrictions & (1 << flags->option)) != 0) {
                        fr_strerror_printf("Parent %s already has a child with option %u - duplicates are not allowed",
                                           da->parent->name, flags->option);
@@ -903,6 +901,7 @@ static bool attr_valid(fr_dict_attr_t *da)
                fr_assert(flags->der_type < FR_DER_TAG_VALUE_MAX);
 
                flags->class = FR_DER_CLASS_CONTEXT;
+//             flags->option = flags->der_type;
 
                if ((parent->restrictions & (1 << flags->der_type)) != 0) {
                        fr_strerror_printf("Parent %s already has a child with tag %s - duplicates are not allowed",
@@ -920,6 +919,11 @@ static bool attr_valid(fr_dict_attr_t *da)
                        return false;
                }
 
+               /*
+                *      A sequence can sometimes have mixed tags && options.
+                */
+               fr_assert(!flags->is_option);
+
        } else if (parent->is_set_of) {
                if (flags->der_type != parent->set_of) {
                        fr_strerror_printf("Parent %s is a set_of=%s - a child cannot be %s",

diff --git a/src/protocols/der/decode.c b/src/protocols/der/decode.c
index fa4a904fca30371114616f61c5f159dc713ea035..60ca69dec1e913f466746484105351a0165b4871 100644 (file)
--- a/src/protocols/der/decode.c
+++ b/src/protocols/der/decode.c
@@ -1699,7 +1699,7 @@ static ssize_t fr_der_decode_combo_ip_addr(TALLOC_CTX *ctx, fr_pair_list_t *out,
 }
 
 
-static const fr_der_tag_decode_t tag_funcs[FR_DER_TAG_MAX] = {
+static const fr_der_tag_decode_t tag_funcs[FR_DER_TAG_VALUE_MAX] = {
        [FR_DER_TAG_BOOLEAN]          = { .constructed = FR_DER_TAG_PRIMITIVE, .decode = fr_der_decode_boolean },
        [FR_DER_TAG_INTEGER]          = { .constructed = FR_DER_TAG_PRIMITIVE, .decode = fr_der_decode_integer },
        [FR_DER_TAG_BITSTRING]        = { .constructed = FR_DER_TAG_PRIMITIVE, .decode = fr_der_decode_bitstring },
@@ -1780,11 +1780,18 @@ static ssize_t fr_der_decode_hdr(fr_dict_attr_t const *parent, fr_dbuff_t *in, u
        /*
         *      Check if the tag is not universal
         */
-       if (tag_class != FR_DER_CLASS_UNIVERSAL) {
+       switch (tag_class) {
+       case FR_DER_CLASS_UNIVERSAL:
+               if ((*tag == FR_DER_TAG_INVALID) || (*tag >= FR_DER_TAG_VALUE_MAX)) {
+                       fr_strerror_printf("Invalid tag %u", *tag);
+                       return -1;
+               }
+               break;
+
+       case FR_DER_CLASS_CONTEXT:
                /*
                 *      The data type will need to be resolved using the dictionary and the tag value
                 */
-
                if (!parent) {
                        fr_strerror_const("No parent attribute to resolve tag");
                        return -1;
@@ -1800,19 +1807,32 @@ static ssize_t fr_der_decode_hdr(fr_dict_attr_t const *parent, fr_dbuff_t *in, u
                /*
                 *      Doesn't match, check if it's optional.
                 */
-               if (*tag != flags->option) {
-                       if (flags->optional) return 0;
+               if (flags->is_option) {
+                       if (*tag != flags->option) {
+                               if (flags->optional) return 0;
 
-                       fr_strerror_printf("Invalid tag %u for attribute %s. Expected %u", *tag, parent->name,
-                                          fr_der_flag_option(parent));
-                       return -1;
-               }
+                               fr_strerror_printf("Invalid option %u for attribute %s. Expected option %u",
+                                                  *tag, parent->name, flags->option);
+                               return -1;
+                       }
 
-               *tag = flags->der_type;
-       }
+                       *tag = flags->der_type;
 
-       if ((*tag >= NUM_ELEMENTS(tag_funcs)) || (*tag == FR_DER_TAG_INVALID)) {
-               fr_strerror_printf("Unknown tag %u", *tag);
+               } else {
+                       if (*tag != flags->der_type) {
+                               if (flags->optional) return 0;
+
+                               fr_strerror_printf("Invalid tag %s for attribute %s. Expected tag %s",
+                                                  fr_der_tag_to_str(*tag), parent->name, fr_der_tag_to_str(flags->der_type));
+                               return -1;
+                       }
+               }
+               fr_assert(flags->der_type != FR_DER_TAG_INVALID);
+               fr_assert(flags->der_type < NUM_ELEMENTS(tag_funcs));
+               break;
+
+       default:
+               fr_strerror_printf("Unsupported tag class %02x", tag_class);
                return -1;
        }
 
@@ -1857,20 +1877,19 @@ static ssize_t fr_der_decode_hdr(fr_dict_attr_t const *parent, fr_dbuff_t *in, u
                                FR_DBUFF_OUT_RETURN(&len_byte, &our_in);
                                *len = (*len << 8) | len_byte;
                        }
-               }
-
-               else if (!constructed) {
+               } else if (!constructed) {
                        fr_strerror_const("Primitive data with indefinite form length field is invalid");
                        return -1;
                }
+
        } else {
                *len = len_byte;
        }
 
        /*
-        *      Check if the length is valid for our buffer
+        *      Ensure that there is the correct amount of data available to read.
         */
-       if (unlikely(fr_dbuff_extend_lowat(NULL, &our_in, *len) < *len)) {
+       if (*len && unlikely((fr_dbuff_extend_lowat(NULL, &our_in, *len) < *len))) {
                fr_strerror_printf("Insufficient data for length field (%zu)", *len);
                return -1;
        }

diff --git a/src/protocols/der/der.h b/src/protocols/der/der.h
index 4d06925145788e45c45b1cb0ac88e3f2306ded91..b74adc1ad5c65ac3734521970a142c5b283493be 100644 (file)
--- a/src/protocols/der/der.h
+++ b/src/protocols/der/der.h
@@ -103,6 +103,7 @@ typedef struct {
        uint32_t                restrictions;           //!< for choice of options and tags - no dups allowed
        uint8_t                 min;                    //!< mininum count
        uint8_t                 option;                 //!< an "attribute number" encoded in the tag field.
+       bool                    is_option : 1;          //!< has an option defined
        bool                    optional : 1;           //!< optional, we MUST already have set 'option'
        bool                    is_sequence_of : 1;     //!< sequence_of has been defined
        bool                    is_set_of : 1;          //!< set_of has been defined

diff --git a/src/protocols/der/encode.c b/src/protocols/der/encode.c
index 2e1da652e891baa1fbecefd3a21fc96cb2be561b..725badc7d747af6523c322cea9c2344729bf1f67 100644 (file)
--- a/src/protocols/der/encode.c
+++ b/src/protocols/der/encode.c
@@ -1918,7 +1918,7 @@ static ssize_t encode_value(fr_dbuff_t *dbuff, UNUSED fr_da_stack_t *da_stack, U
         *      tag, but we might need to encode an option value
         *      instead of a tag.
         */
-       if (flags->option | tag_class) tag = flags->option;
+       if (flags->is_option) tag = flags->option;
 
        fr_dbuff_marker(&encoding_start, &our_dbuff);