From: Alan T. DeKok Date: Wed, 19 Feb 2025 18:31:34 +0000 (-0500) Subject: remove is_pairs and other cleanup X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b0729f2d4b1209cd85736949145a91125f293d80;p=thirdparty%2Ffreeradius-server.git remove is_pairs and other cleanup 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. --- diff --git a/src/protocols/der/base.c b/src/protocols/der/base.c index d39751509a..e1c9c3ef6d 100644 --- a/src/protocols/der/base.c +++ b/src/protocols/der/base.c @@ -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 } }, diff --git a/src/protocols/der/decode.c b/src/protocols/der/decode.c index 12b53d2374..24833f290b 100644 --- a/src/protocols/der/decode.c +++ b/src/protocols/der/decode.c @@ -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; diff --git a/src/protocols/der/der.h b/src/protocols/der/der.h index 69ad80db53..e9883a0dc3 100644 --- a/src/protocols/der/der.h +++ b/src/protocols/der/der.h @@ -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) diff --git a/src/protocols/der/encode.c b/src/protocols/der/encode.c index 4c2e5640cc..e46cac0dee 100644 --- a/src/protocols/der/encode.c +++ b/src/protocols/der/encode.c @@ -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; diff --git a/src/tests/unit/protocols/der/base.txt b/src/tests/unit/protocols/der/base.txt index 88bed1f6c6..2d9ad021b6 100644 --- a/src/tests/unit/protocols/der/base.txt +++ b/src/tests/unit/protocols/der/base.txt @@ -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 diff --git a/src/tests/unit/protocols/der/dictionary.test b/src/tests/unit/protocols/der/dictionary.test index 17e1ce159d..6e3831fe85 100644 --- a/src/tests/unit/protocols/der/dictionary.test +++ b/src/tests/unit/protocols/der/dictionary.test @@ -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 diff --git a/src/tests/unit/protocols/der/pkcs10.txt b/src/tests/unit/protocols/der/pkcs10.txt index bbfa3dc486..be2d9b80d2 100644 --- a/src/tests/unit/protocols/der/pkcs10.txt +++ b/src/tests/unit/protocols/der/pkcs10.txt @@ -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