]> git.ipfire.org Git - thirdparty/freeradius-server.git/commitdiff
remove is_pairs and other cleanup
authorAlan T. DeKok <aland@freeradius.org>
Wed, 19 Feb 2025 18:31:34 +0000 (13:31 -0500)
committerAlan T. DeKok <aland@freeradius.org>
Wed, 19 Feb 2025 18:31:34 +0000 (13:31 -0500)
Also, cut some variables which weren't being used when sorting
set items (reducing code).

Fixed a test case which should not have failed, and changed it
to test an actual failing case.

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

index d39751509a9d8e17b27a1ae812413e6d53980c3b..e1c9c3ef6da9f62d0a805ce27951a223aa010aa6 100644 (file)
@@ -338,15 +338,6 @@ static int dict_flag_is_oid_leaf(fr_dict_attr_t **da_p, UNUSED char const *value
        return 0;
 }
 
-static int dict_flag_is_pairs(fr_dict_attr_t **da_p, UNUSED char const *value, UNUSED fr_dict_flag_parser_rule_t const *rules)
-{
-       fr_der_attr_flags_t *flags = fr_dict_attr_ext(*da_p, FR_DICT_ATTR_EXT_PROTOCOL_SPECIFIC);
-
-       flags->is_pairs = true;
-
-       return 0;
-}
-
 static int dict_flag_is_choice(fr_dict_attr_t **da_p, UNUSED char const *value, UNUSED fr_dict_flag_parser_rule_t const *rules)
 {
        fr_der_attr_flags_t *flags = fr_dict_attr_ext(*da_p, FR_DICT_ATTR_EXT_PROTOCOL_SPECIFIC);
@@ -420,7 +411,6 @@ static const fr_dict_flag_parser_t  der_flags[] = {
        { L("is_extensions"),   { .func = dict_flag_is_extensions } },
        { L("is_oid_leaf"),     { .func = dict_flag_is_oid_leaf } },
        { L("is_pair"),         { .func = dict_flag_is_pair } },
-       { L("is_pairs"),        { .func = dict_flag_is_pairs } },
        { L("max"),             { .func = dict_flag_max, .needs_value = true } },
        { L("option"),          { .func = dict_flag_option} },
        { L("sequence_of"),     { .func = dict_flag_sequence_of, .needs_value = true } },
index 12b53d237418b010d8a3ed1ded266bf176582678..24833f290b89643862ae7326b5bcb626b60c26dd 100644 (file)
@@ -870,31 +870,6 @@ static ssize_t fr_der_decode_sequence(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_d
                return fr_dbuff_set(in, &our_in);
        }
 
-       if (unlikely(fr_der_flag_is_pairs(parent) || decode_ctx->oid_value_pairs)) {
-               /*
-                *      This sequence contains sequences/sets of pairs
-                */
-               bool old                    = decode_ctx->oid_value_pairs;
-               decode_ctx->oid_value_pairs = true;
-               while (fr_dbuff_remaining(&our_in) > 0) {
-                       child = NULL;
-                       child = fr_dict_attr_iterate_children(parent, &child);
-
-                       FR_PROTO_TRACE("decode context %s -> %s", parent->name, child->name);
-
-                       if (unlikely(fr_der_decode_pair_dbuff(vp, &vp->vp_group, child, &our_in, decode_ctx) < 0)) {
-                               talloc_free(vp);
-                               return -1;
-                       }
-               }
-
-               decode_ctx->oid_value_pairs = old;
-
-               fr_pair_append(out, vp);
-
-               return fr_dbuff_set(in, &our_in);
-       }
-
        if (unlikely(fr_der_flag_is_sequence_of(parent))) {
                /*
                 *      This is a sequence-of, meaning there are restrictions on the types which can be present
@@ -1044,32 +1019,6 @@ static ssize_t fr_der_decode_set(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_a
                return fr_dbuff_set(in, &our_in);
        }
 
-       if (fr_der_flag_is_pairs(parent) || decode_ctx->oid_value_pairs) {
-               /*
-                *      This set contains sequences/sets of pairs
-                */
-               bool old                    = decode_ctx->oid_value_pairs;
-
-               decode_ctx->oid_value_pairs = true;
-               while (fr_dbuff_remaining(&our_in) > 0) {
-                       child = NULL;
-                       child = fr_dict_attr_iterate_children(parent, &child);
-
-                       FR_PROTO_TRACE("decode context %s -> %s", parent->name, child->name);
-
-                       if (unlikely(fr_der_decode_pair_dbuff(vp, &vp->vp_group, child, &our_in, decode_ctx) < 0)) {
-                               talloc_free(vp);
-                               return -1;
-                       }
-               }
-
-               decode_ctx->oid_value_pairs = old;
-
-               fr_pair_append(out, vp);
-
-               return fr_dbuff_set(in, &our_in);
-       }
-
        if (fr_der_flag_is_set_of(parent)) {
                /*
                 *      This is a set-of, meaning there are restrictions on the types which can be present
@@ -1122,9 +1071,13 @@ static ssize_t fr_der_decode_set(TALLOC_CTX *ctx, fr_pair_list_t *out, fr_dict_a
                                                goto error;
                                        }
 
+                                       if (prev_char < curr_char) {
+                                               break;
+                                       }
+
                                } while (fr_dbuff_remaining(&our_in) > 0 && fr_dbuff_remaining(&previous_item) > 0);
 
-                               if (fr_dbuff_remaining(&previous_item) > 0) {
+                               if (prev_char > curr_char && fr_dbuff_remaining(&previous_item) > 0) {
                                        fr_strerror_const(
                                                "Set tags are not in ascending order. Previous item has more data");
                                        ret = -1;
index 69ad80db53c788d5f2ce558aaef2bd27e69d6210..e9883a0dc373e71c5a2c679c907681bb20b84079 100644 (file)
@@ -75,7 +75,6 @@ typedef struct {
        bool                    is_sequence_of : 1;     //!< sequence_of has been defined
        bool                    is_set_of : 1;          //!< set_of has been defined
        bool                    is_pair : 1;
-       bool                    is_pairs : 1;
        bool                    is_extensions : 1;      //!< a list of X.509 extensions
        bool                    has_default : 1;        //!< a default value exists
        bool                    is_oid_leaf : 1;
@@ -96,7 +95,6 @@ static inline fr_der_attr_flags_t const *fr_der_attr_flags(fr_dict_attr_t const
 #define fr_der_flag_is_set_of(_da)     (fr_der_attr_flags(_da)->is_set_of)
 #define fr_der_flag_max(_da)           (fr_der_attr_flags(_da)->max)
 #define fr_der_flag_is_pair(_da)       (fr_der_attr_flags(_da)->is_pair)
-#define fr_der_flag_is_pairs(_da)      (fr_der_attr_flags(_da)->is_pairs)
 #define fr_der_flag_is_extensions(_da)         (fr_der_attr_flags(_da)->is_extensions)
 #define fr_der_flag_has_default(_da)   (fr_der_attr_flags(_da)->has_default)
 #define fr_der_flag_is_oid_leaf(_da)   (fr_der_attr_flags(_da)->is_oid_leaf)
index 4c2e5640cc5ea8dc2a0ac44c00c9de2a28dd6248..e46cac0dee07297c96053478702c031245d58b8e 100644 (file)
@@ -38,13 +38,11 @@ RCSID("$Id$")
 
 typedef struct {
        fr_dbuff_marker_t encoding_start;       //!< This is the start of the encoding. It is NOT the same as the start of the
+                                               //!< encoded value. It includes the tag, length, and value.
        uint8_t *tmp_ctx;                       //!< Temporary context for encoding.
                                                //!< encoded value. It is the position of the tag.
        size_t encoding_length;                 //!< This is the length of the entire encoding. It is NOT the same as the length
                                                //!< of the encoded value. It includes the tag, length, and value.
-       ssize_t value_length;                   //!< This is the number of bytes used by the encoded value. It is NOT the
-                                               //!< same as the encoded length field.
-       uint8_t *encoded_value;                 //!< This is a pointer to the start of the encoded value.
 } fr_der_encode_ctx_t;
 
 /** Function signature for DER encode functions
@@ -667,8 +665,6 @@ static ssize_t fr_der_encode_sequence(fr_dbuff_t *dbuff, fr_dcursor_t *cursor, f
 typedef struct {
        fr_dbuff_marker_t item_ptr;     //!< Pointer to the start of the encoded item (beginning of the tag)
        size_t   item_len;              //!< Length of the encoded item (tag + length + value)
-       uint8_t *octet_ptr;             //!< Pointer to the current octet
-       size_t   remaining;             //!< Remaining octets
 } fr_der_encode_set_of_ptr_pairs_t;
 
 /*
@@ -827,8 +823,6 @@ static ssize_t fr_der_encode_set(fr_dbuff_t *dbuff, fr_dcursor_t *cursor, fr_der
 
                        ptr_pairs[i].item_ptr  = encode_ctx->encoding_start;
                        ptr_pairs[i].item_len  = encode_ctx->encoding_length;
-                       ptr_pairs[i].octet_ptr = encode_ctx->encoded_value;
-                       ptr_pairs[i].remaining = encode_ctx->value_length;
 
                        slen += len_count;
                }
@@ -1681,12 +1675,13 @@ static ssize_t encode_value(fr_dbuff_t *dbuff, UNUSED fr_da_stack_t *da_stack, U
 {
        fr_pair_t const     *vp;
        fr_dbuff_t           our_dbuff = FR_DBUFF(dbuff);
-       fr_dbuff_marker_t    marker;
+       fr_dbuff_marker_t    marker, encoding_start;
        fr_der_tag_encode_t *func;
        fr_der_tag_t         tag;
        fr_der_tag_class_t   tag_class;
        fr_der_encode_ctx_t *uctx = encode_ctx;
        ssize_t              slen = 0;
+       size_t               encoding_length;
 
        if (unlikely(cursor == NULL)) {
                fr_strerror_const("No cursor to encode");
@@ -1796,16 +1791,16 @@ static ssize_t encode_value(fr_dbuff_t *dbuff, UNUSED fr_da_stack_t *da_stack, U
         */
        if (fr_der_flag_option(vp->da) | tag_class) tag = fr_der_flag_option(vp->da);
 
-       fr_dbuff_marker(&uctx->encoding_start, &our_dbuff);
+       fr_dbuff_marker(&encoding_start, &our_dbuff);
 
        slen = fr_der_encode_tag(&our_dbuff, tag, tag_class, func->constructed);
        if (slen < 0) {
        error:
-               fr_dbuff_marker_release(&uctx->encoding_start);
+               fr_dbuff_marker_release(&encoding_start);
                return slen;
        }
 
-       uctx->encoding_length = slen;
+       encoding_length = slen;
 
        /*
         *      Mark and reserve space in the buffer for the length field
@@ -1823,21 +1818,21 @@ static ssize_t encode_value(fr_dbuff_t *dbuff, UNUSED fr_da_stack_t *da_stack, U
                goto error;
        }
 
-       uctx->encoding_length += slen;
-       uctx->value_length = slen;
+       encoding_length += slen;
 
        /*
-        *      Encode the length of the value
-        */
+             Encode the length of the value
+       */
        slen = fr_der_encode_len(&our_dbuff, &marker);
        if (slen < 0) {
                fr_dbuff_marker_release(&marker);
                goto error;
        }
 
-       uctx->encoded_value = fr_dbuff_start(&marker) + slen + 1;
        fr_dbuff_marker_release(&marker);
-       uctx->encoding_length += slen;
+
+       uctx->encoding_start = encoding_start;
+       uctx->encoding_length = encoding_length + slen;
 
        fr_dcursor_next(cursor);
        return fr_dbuff_set(dbuff, &our_dbuff);
@@ -1887,8 +1882,6 @@ static int encode_test_ctx(void **out, TALLOC_CTX *ctx, UNUSED fr_dict_t const *
 
        test_ctx->tmp_ctx            = talloc(test_ctx, uint8_t);
        test_ctx->encoding_length    = 0;
-       test_ctx->value_length = 0;
-       test_ctx->encoded_value      = NULL;
 
        *out = test_ctx;
 
index 88bed1f6c67aeed434efed315ab40470e0986dd7..2d9ad021b698eaf6fe02a9ff40d427d0ffde47bf 100644 (file)
@@ -26,8 +26,8 @@ proto-dictionary-root Test-Set-Of
 decode-pair 31 06 02 01 01 02 01 02
 match Test-Set-Of = { Test-First-Integer = 1, Test-First-Integer = 2 }
 
-decode-pair 31 07 02 02 00 80 02 01 02
-match Set tags are not in ascending order. Previous item has more data
+decode-pair 31 07 02 01 02 02 02 00 80
+match Set tags are not in ascending order
 
 proto-dictionary-root Test-Set-Of
 
index 17e1ce159d38e62e4fb125342de7ab50e5ea003b..6e3831fe8524bf40b1d872fc9ed89ce0360539dd 100644 (file)
@@ -4,7 +4,7 @@
 # Version $Id$
 DEFINE Certificate-Extensions                          x509_extensions ref=OID-Tree
 
-DEFINE Issuer                                          sequence is_pairs
+DEFINE Issuer                                          sequence sequence_of=set
 BEGIN Issuer
 DEFINE RelativeDistinguishedName                       set
 BEGIN RelativeDistinguishedName
@@ -12,9 +12,9 @@ DEFINE        AttributeTypeAndValue                           group ref=OID-Tree,sequence_of=set,is_pair
 END RelativeDistinguishedName
 END Issuer
 
-DEFINE Issuer-Set                                      sequence is_pairs
+DEFINE Issuer-Set                                      set set_of=set
 BEGIN Issuer-Set
-DEFINE RelativeDistinguishedName                       sequence
+DEFINE RelativeDistinguishedName                       set
 BEGIN RelativeDistinguishedName
 DEFINE AttributeTypeAndValue                           group ref=OID-Tree,sequence_of=set,is_pair
 END RelativeDistinguishedName
index bbfa3dc4867c09694d635839cce088d1ad5336f9..be2d9b80d208277c8c3844fd55a121ca05cbf89c 100644 (file)
@@ -48,9 +48,17 @@ match Issuer = { RelativeDistinguishedName = { AttributeTypeAndValue = { joint-i
 decode-pair 30 4A 31 0B 30 09 06 03 55 04 06 13 02 55 53 31 16 30 14 06 03 55 04 0A 13 0D 4C 65 74 27 73 20 45 6E 63 72 79 70 74 31 23 30 21 06 03 55 04 03 13 1A 4C 65 74 27 73 20 45 6E 63 72 79 70 74 20 41 75 74 68 6F 72 69 74 79 20 58 33
 match Issuer = { RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { countryName = "US" } } } } }, RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { organizationName = "Let's Encrypt" } } } } }, RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { commonName = "Let's Encrypt Authority X3" } } } } } }
 
+proto-dictionary der
+encode-pair Issuer = { RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { countryName = "US" } } } } }, RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { organizationName = "Let's Encrypt" } } } } }, RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { commonName = "Let's Encrypt Authority X3" } } } } } }
+match 30 4a 31 0b 30 09 06 03 55 04 06 13 02 55 53 31 16 30 14 06 03 55 04 0a 13 0d 4c 65 74 27 73 20 45 6e 63 72 79 70 74 31 23 30 21 06 03 55 04 03 13 1a 4c 65 74 27 73 20 45 6e 63 72 79 70 74 20 41 75 74 68 6f 72 69 74 79 20 58 33
+
+proto-dictionary der
+encode-pair Issuer-Set = { RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { commonName = "DST Root CA X3" } } } } }, RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { organizationName = "Digital Signature Trust Co." } } } } } }
+match 31 3f 31 17 30 15 06 03 55 04 03 13 0e 44 53 54 20 52 6f 6f 74 20 43 41 20 58 33 31 24 30 22 06 03 55 04 0a 13 1b 44 69 67 69 74 61 6c 20 53 69 67 6e 61 74 75 72 65 20 54 72 75 73 74 20 43 6f 2e
+
 proto-dictionary-root Issuer-Set
-decode-pair 31 3F 31 24 30 22 06 03 55 04 0A 13 1B 44 69 67 69 74 61 6C 20 53 69 67 6E 61 74 75 72 65 20 54 72 75 73 74 20 43 6F 2E 31 17 30 15 06 03 55 04 03 13 0E 44 53 54 20 52 6F 6F 74 20 43 41 20 58 33
-match Issuer-Set = { RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { organizationName = "Digital Signature Trust Co." } } } } }, RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { commonName = "DST Root CA X3" } } } } } }
+decode-pair 31 3F 31 17 30 15 06 03 55 04 03 13 0E 44 53 54 20 52 6F 6F 74 20 43 41 20 58 33 31 24 30 22 06 03 55 04 0A 13 1B 44 69 67 69 74 61 6C 20 53 69 67 6E 61 74 75 72 65 20 54 72 75 73 74 20 43 6F 2E
+match Issuer-Set = { RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { commonName = "DST Root CA X3" } } } } }, RelativeDistinguishedName = { AttributeTypeAndValue = { joint-iso-itu-t = { ds = { attributeType = { organizationName = "Digital Signature Trust Co." } } } } } }
 
 count
-match 35
+match 41