diff --git a/src/bfcli/parser.y b/src/bfcli/parser.y index dde21918..da601a81 100644 --- a/src/bfcli/parser.y +++ b/src/bfcli/parser.y @@ -531,7 +531,6 @@ matcher : matcher_type negate matcher_op RAW_PAYLOAD | SET_TYPE negate matcher_op SET_RAW_PAYLOAD { _free_bf_matcher_ struct bf_matcher *matcher = NULL; - _free_bf_set_ struct bf_set *set = NULL; _cleanup_free_ const char *raw_key = $1; _cleanup_free_ const char *payload = $4; enum bf_matcher_op op = $3; @@ -540,18 +539,10 @@ matcher : matcher_type negate matcher_op RAW_PAYLOAD if (op != BF_MATCHER_IN) bf_parse_err("only the 'in' operator is supported for sets"); - r = bf_set_new_from_raw(&set, NULL, raw_key, payload); - if (r) - bf_parse_err("failed to create new set"); - - r = bf_matcher_new(&matcher, BF_MATCHER_SET, BF_MATCHER_IN, set, sizeof(*set), $2); + r = bf_matcher_new_for_anon_set_from_raw(&matcher, raw_key, payload, $2); if (r) bf_parse_err("failed to create a new matcher"); - r = bf_hashset_add(&ruleset->sets, (void **)&set); - if (r) - bf_parse_err("failed to add new set to the ruleset"); - $$ = TAKE_PTR(matcher); } | SET_TYPE negate matcher_op STRING @@ -578,7 +569,7 @@ matcher : matcher_type negate matcher_op RAW_PAYLOAD if (found_set->n_comps != test_key->n_comps || memcmp(found_set->key, test_key->key, found_set->n_comps * sizeof(enum bf_matcher_type))) bf_parse_err("using named set '%s', but key doesn't match", name); - r = bf_matcher_new(&matcher, BF_MATCHER_SET, BF_MATCHER_IN, found_set, sizeof(*found_set), $2); + r = bf_matcher_new_for_named_set(&matcher, found_set, $2); if (r) bf_parse_err("failed to create a new matcher"); diff --git a/src/libbpfilter/cgen/cgroup_sock_addr.c b/src/libbpfilter/cgen/cgroup_sock_addr.c index babf10da..9ac30e37 100644 --- a/src/libbpfilter/cgen/cgroup_sock_addr.c +++ b/src/libbpfilter/cgen/cgroup_sock_addr.c @@ -294,8 +294,7 @@ static int _bf_cgroup_sock_addr_generate_set(struct bf_program *program, set = bf_chain_get_set_for_matcher(program->runtime.chain, matcher); if (!set) { - return bf_err_r(-ENOENT, "set #%u not found in %s", - *(uint32_t *)bf_matcher_payload(matcher), + return bf_err_r(-ENOENT, "matcher set not found in %s", program->runtime.chain->name); } diff --git a/src/libbpfilter/cgen/matcher/set.c b/src/libbpfilter/cgen/matcher/set.c index c67ebec0..cbf0d9af 100644 --- a/src/libbpfilter/cgen/matcher/set.c +++ b/src/libbpfilter/cgen/matcher/set.c @@ -28,8 +28,7 @@ int bf_set_generate_map_lookup(struct bf_program *program, set = bf_chain_get_set_for_matcher(program->runtime.chain, matcher); if (!set) { - return bf_err_r(-ENOENT, "set '%s' not found in %s", - (const char *)bf_matcher_payload(matcher), + return bf_err_r(-ENOENT, "matcher set not found in %s", program->runtime.chain->name); } diff --git a/src/libbpfilter/cgen/packet.c b/src/libbpfilter/cgen/packet.c index 969c4e85..a904959e 100644 --- a/src/libbpfilter/cgen/packet.c +++ b/src/libbpfilter/cgen/packet.c @@ -338,8 +338,7 @@ static int _bf_matcher_pkt_generate_set(struct bf_program *program, set = bf_chain_get_set_for_matcher(program->runtime.chain, matcher); if (!set) { - return bf_err_r(-ENOENT, "set #%u not found in %s", - *(uint32_t *)bf_matcher_payload(matcher), + return bf_err_r(-ENOENT, "matcher set not found in %s", program->runtime.chain->name); } diff --git a/src/libbpfilter/cgen/program.c b/src/libbpfilter/cgen/program.c index eb53e6c6..9ce7c0e5 100644 --- a/src/libbpfilter/cgen/program.c +++ b/src/libbpfilter/cgen/program.c @@ -164,6 +164,52 @@ _bf_program_find_set_group(const struct bf_program *program, return NULL; } +static int _bf_program_add_set_to_group(struct bf_program *program, + const struct bf_set *set) +{ + _free_bf_set_group_ struct bf_set_group *group = NULL; + int r; + + assert(program); + assert(set); + + if (bf_hashset_is_empty(&set->elems)) + return 0; + + /* If a group with a matching key already exists, reuse it. For LPM trie, + * sets can't be grouped (as we can't define which rule matches on which + * prefix), so they always are the sole member of their group. */ + if (!set->use_trie) { + bf_list_foreach (&program->set_groups, group_node) { + struct bf_set_group *group = bf_list_node_get_data(group_node); + const struct bf_set *head = + bf_list_node_get_data(bf_list_get_head(&group->sets)); + + if (bf_set_same_key(set, head)) { + r = bf_list_add_tail(&group->sets, (void *)set); + if (r) + return bf_err_r(r, "failed to add set to existing group"); + + return 0; + } + } + } + + r = _bf_set_group_new(&group); + if (r) + return bf_err_r(r, "failed to allocate set group"); + + r = bf_list_add_tail(&group->sets, (void *)set); + if (r) + return bf_err_r(r, "failed to seed set group"); + + r = bf_list_push(&program->set_groups, (void **)&group); + if (r) + return bf_err_r(r, "failed to register set group"); + + return 0; +} + static int _bf_program_build_set_groups(struct bf_program *program) { assert(program); @@ -172,48 +218,30 @@ static int _bf_program_build_set_groups(struct bf_program *program) program->set_groups = bf_list_default(_bf_set_group_free, NULL); bf_hashset_foreach (&program->runtime.chain->sets, set_node) { - struct bf_set *set = set_node->data; - struct bf_set_group *match = NULL; - int r; - - if (bf_hashset_is_empty(&set->elems)) - continue; + int r = _bf_program_add_set_to_group(program, set_node->data); + if (r) + return r; + } - /* LPM trie sets are not grouped: BPF LPM trie lookup always - * returns the longest-prefix match, so the read-modify-write - * step in _bf_program_load_sets_maps() can't preserve the - * per-set bitmask when prefixes overlap. */ - if (!set->use_trie) { - bf_list_foreach (&program->set_groups, group_node) { - struct bf_set_group *group = bf_list_node_get_data(group_node); - const struct bf_set *head = - bf_list_node_get_data(bf_list_get_head(&group->sets)); - - if (bf_set_same_key(set, head)) { - match = group; - break; - } - } - } + bf_list_foreach (&program->runtime.chain->rules, rule_node) { + const struct bf_rule *rule = bf_list_node_get_data(rule_node); - if (match) { - r = bf_list_add_tail(&match->sets, set); - if (r) - return bf_err_r(r, "failed to add set to existing group"); - } else { - _free_bf_set_group_ struct bf_set_group *new_group = NULL; + bf_list_foreach (&rule->matchers, matcher_node) { + const struct bf_matcher *matcher = + bf_list_node_get_data(matcher_node); + const struct bf_set *set; + int r; - r = _bf_set_group_new(&new_group); - if (r) - return bf_err_r(r, "failed to allocate set group"); + if (bf_matcher_get_type(matcher) != BF_MATCHER_SET) + continue; - r = bf_list_add_tail(&new_group->sets, set); - if (r) - return bf_err_r(r, "failed to seed set group"); + memcpy(&set, bf_matcher_payload(matcher), sizeof(struct bf_set *)); + if (!bf_set_is_anonymous(set)) + continue; - r = bf_list_push(&program->set_groups, (void **)&new_group); + r = _bf_program_add_set_to_group(program, set); if (r) - return bf_err_r(r, "failed to register set group"); + return r; } } diff --git a/src/libbpfilter/chain.c b/src/libbpfilter/chain.c index 65b4cb05..05c3bcf2 100644 --- a/src/libbpfilter/chain.c +++ b/src/libbpfilter/chain.c @@ -292,6 +292,15 @@ int bf_chain_new(struct bf_chain **chain, const char *name, enum bf_hook hook, return bf_err_r(-EINVAL, "invalid policy '%s'", bf_verdict_to_str(policy)); + if (sets) { + bf_hashset_foreach (sets, set_node) { + const struct bf_set *set = set_node->data; + + if (!bf_set_is_valid_name(set->name)) + return bf_err_r(-EINVAL, "chain sets must have valid names"); + } + } + _chain = calloc(1, sizeof(*_chain)); if (!_chain) return -ENOMEM; @@ -317,8 +326,13 @@ int bf_chain_new(struct bf_chain **chain, const char *name, enum bf_hook hook, } r = _bf_chain_validate_rules(_chain); - if (r) + if (r) { + if (rules) + *rules = bf_list_move(&_chain->rules); + if (sets) + *sets = bf_hashset_move(&_chain->sets); return r; + } *chain = TAKE_PTR(_chain); @@ -332,8 +346,8 @@ int bf_chain_new_from_pack(struct bf_chain **chain, bf_rpack_node_t node) enum bf_hook hook; enum bf_verdict policy; bf_rpack_node_t array, array_node; - _clean_bf_list_ bf_list rules = bf_list_default(bf_rule_free, bf_rule_pack); _clean_bf_hashset_ bf_hashset sets = bf_set_hashset_default(); + _clean_bf_list_ bf_list rules = bf_list_default(bf_rule_free, bf_rule_pack); int r; r = bf_rpack_kv_str(node, "name", &name); @@ -358,6 +372,9 @@ int bf_chain_new_from_pack(struct bf_chain **chain, bf_rpack_node_t node) if (r) return bf_err_r(r, "failed to unpack bf_set into bf_chain.sets"); + if (bf_set_is_anonymous(set)) + return bf_err_r(-EINVAL, "anonymous set outside a matcher"); + r = bf_hashset_add(&sets, (void **)&set); if (r) return bf_err_r(r, "failed to add unpacked set to chain"); @@ -369,7 +386,8 @@ int bf_chain_new_from_pack(struct bf_chain **chain, bf_rpack_node_t node) bf_rpack_array_foreach (array, array_node) { _free_bf_rule_ struct bf_rule *rule = NULL; - r = bf_list_emplace(&rules, bf_rule_new_from_pack, rule, array_node); + r = bf_list_emplace(&rules, bf_rule_new_from_pack, rule, array_node, + &sets); if (r) { return bf_err_r(r, "failed to unpack bf_rule into bf_chain.rules"); } @@ -391,8 +409,8 @@ void bf_chain_free(struct bf_chain **chain) if (!*chain) return; - bf_hashset_clean(&(*chain)->sets); bf_list_clean(&(*chain)->rules); + bf_hashset_clean(&(*chain)->sets); BF_FREEP(&(*chain)->name); BF_FREEP(chain); } @@ -491,6 +509,9 @@ int bf_chain_add_set(struct bf_chain *chain, struct bf_set *set) { assert(chain && set); + if (!bf_set_is_valid_name(set->name)) + return bf_err_r(-EINVAL, "only named sets can be added to a chain"); + return bf_hashset_add(&chain->sets, (void **)&set); } @@ -498,13 +519,19 @@ const struct bf_set * bf_chain_get_set_for_matcher(const struct bf_chain *chain, const struct bf_matcher *matcher) { + const struct bf_set *set; + assert(chain); assert(matcher); if (bf_matcher_get_type(matcher) != BF_MATCHER_SET) return NULL; - return bf_chain_get_set_by_name(chain, bf_matcher_payload(matcher)); + memcpy(&set, bf_matcher_payload(matcher), sizeof(struct bf_set *)); + if (bf_set_is_anonymous(set)) + return set; + + return bf_hashset_get(&chain->sets, set) == set ? set : NULL; } static struct bf_set *_bf_chain_get_set_by_name(struct bf_chain *chain, @@ -612,6 +639,29 @@ int bf_chain_new_from_copy(struct bf_chain **dest, const struct bf_chain *src) if (r) return r; + bf_list_foreach (&rule->matchers, matcher_node) { + struct bf_matcher *matcher = bf_list_node_get_data(matcher_node); + const struct bf_set *set; + + if (bf_matcher_get_type(matcher) != BF_MATCHER_SET) + continue; + + set = bf_chain_get_set_for_matcher(src, matcher); + if (!set) + return bf_err_r(-ENOENT, + "matcher set not found in source chain"); + if (!set->name) + continue; + + set = bf_chain_get_set_by_name(copy, set->name); + if (!set) + return bf_err_r(-ENOENT, + "matcher set not found in copied chain"); + + memcpy((void *)bf_matcher_payload(matcher), &set, + sizeof(struct bf_set *)); + } + r = bf_chain_add_rule(copy, rule); if (r) return r; diff --git a/src/libbpfilter/include/bpfilter/chain.h b/src/libbpfilter/include/bpfilter/chain.h index f0a60a37..bbcd74ae 100644 --- a/src/libbpfilter/include/bpfilter/chain.h +++ b/src/libbpfilter/include/bpfilter/chain.h @@ -55,6 +55,8 @@ struct bf_chain uint8_t flags; enum bf_hook hook; enum bf_verdict policy; + + /// Owning collection of named sets. Anonymous sets belong to matchers. bf_hashset sets; bf_list rules; @@ -77,7 +79,7 @@ struct bf_chain * @param name Name of the chain. Can't be NULL. * @param hook Expected hook to attach the chain to. * @param policy Default action of the chain if no rule matched. - * @param sets Owning collection of sets used by `rules`, initialized with + * @param sets Owning collection of named sets, initialized with * bf_set_hashset_default(). Names must be unique within the chain. * @param rules List of rules. * @return 0 on success, or negative errno value on failure. @@ -136,11 +138,10 @@ void bf_chain_dump(const struct bf_chain *chain, prefix_t *prefix); int bf_chain_add_rule(struct bf_chain *chain, struct bf_rule *rule); /** - * @brief Insert a set into a chain. + * @brief Insert a named set into a chain. * * The chain will own the set and is responsible for freeing it. To reference - * the set in a matcher, pass the set pointer and sizeof(*set) to - * bf_matcher_new(). The matcher stores a copy of the set's name. + * the set in a matcher, use `bf_matcher_new_for_named_set`. * * @param chain Chain to insert the rule into. Can't be NULL. * @param set Set to insert into the chain. Can't be NULL. @@ -155,6 +156,7 @@ int bf_chain_add_set(struct bf_chain *chain, struct bf_set *set); * @param matcher Matching filtering on a set. Can't be NULL. * @return The set `matcher` filters on, or NULL if the set can't be found or * if `matcher->type` is not `BF_MATCHER_SET`. + * The pointer is borrowed. Anonymous sets belong to their matcher. */ const struct bf_set * bf_chain_get_set_for_matcher(const struct bf_chain *chain, diff --git a/src/libbpfilter/include/bpfilter/matcher.h b/src/libbpfilter/include/bpfilter/matcher.h index 86510ea6..6aec3efa 100644 --- a/src/libbpfilter/include/bpfilter/matcher.h +++ b/src/libbpfilter/include/bpfilter/matcher.h @@ -9,6 +9,7 @@ #include #include +#include #include #include @@ -39,6 +40,7 @@ #define _free_bf_matcher_ __attribute__((__cleanup__(bf_matcher_free))) struct bf_matcher; +struct bf_set; /** * Matcher type. @@ -268,14 +270,14 @@ struct bf_matcher_meta * * @param matcher Matcher object to allocate and initialise. Can't be NULL. On * success, contain a pointer to the matcher object, unchanged on error. - * @param type Matcher type. + * @param type Matcher type. `BF_MATCHER_SET` will be rejected: use + * `bf_matcher_new_for_named_set`, `bf_matcher_new_for_anon_set`, or + * `bf_matcher_new_for_anon_set_from_raw` instead. * @param op Comparison operator. * @param payload Payload of the matcher, its content and size depends on * @p type . Can be NULL but only if @p payload_len is 0, in which case - * there is no payload. For BF_MATCHER_SET, pass a pointer to a bf_set. - * The constructor copies its name and retains no pointer to the set. - * @param payload_len Length of the input payload. For BF_MATCHER_SET, pass - * sizeof(struct bf_set). The stored payload is the NUL-terminated name. + * there is no payload. + * @param payload_len Length of the input payload. * @param negate If true, negate the comparison result. * @return 0 on success, or negative errno value on failure. */ @@ -283,6 +285,57 @@ int bf_matcher_new(struct bf_matcher **matcher, enum bf_matcher_type type, enum bf_matcher_op op, const void *payload, size_t payload_len, bool negate); +/** + * @brief Create a matcher that borrows an existing named set. + * + * The matcher uses `BF_MATCHER_SET` and `BF_MATCHER_IN`. The set's owner must + * outlive the matcher. `set` must be part of the chain `matcher` will be + * inserted into. + * + * @param matcher Destination matcher. Unchanged on failure. Can't be NULL. + * @param set Set to borrow. Must have a valid non-NULL name. + * @param negate Whether to negate membership. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_matcher_new_for_named_set(struct bf_matcher **matcher, + const struct bf_set *set, bool negate); + +/** + * @brief Create a matcher that owns a new anonymous set. + * + * The matcher uses `BF_MATCHER_SET` and `BF_MATCHER_IN`. The caller's elements + * are copied. The anonymous set is freed with the matcher. + * + * @param matcher Destination matcher. Unchanged on failure. Can't be NULL. + * @param key Set key components. Can't be NULL. + * @param n_comps Number of key components. + * @param elems Array of element pointers. Each element contains the packed + * key components. Can be NULL when `n_elems` is zero. + * @param n_elems Number of elements in `elems`. + * @param negate Whether to negate membership. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_matcher_new_for_anon_set(struct bf_matcher **matcher, + const enum bf_matcher_type *key, size_t n_comps, + const void * const *elems, size_t n_elems, + bool negate); + +/** + * @brief Create a matcher that owns a new anonymous set from raw values. + * + * The matcher uses `BF_MATCHER_SET` and `BF_MATCHER_IN`. Partial allocations + * are freed on failure. The anonymous set is freed with the matcher. + * + * @param matcher Destination matcher. Unchanged on failure. Can't be NULL. + * @param raw_key Set key, as accepted by `bf_set_new_from_raw()`. Can't be NULL. + * @param raw_payload Set elements to parse. Can't be NULL. + * @param negate Whether to negate membership. + * @return 0 on success, or a negative errno value on failure. + */ +int bf_matcher_new_for_anon_set_from_raw(struct bf_matcher **matcher, + const char *raw_key, + const char *raw_payload, bool negate); + /** * @brief Allocate and initialise a new matcher from a raw payload (string). * @@ -305,14 +358,24 @@ int bf_matcher_new_from_raw(struct bf_matcher **matcher, * @param matcher On success, points to the newly allocated and initialised * matcher. Can't be NULL. * @param node Serialized matcher. Can't be NULL. + * @param sets Sets used to resolve serialized names. Can be NULL if the + * matcher does not reference a named set. Anonymous sets are loaded + * inline and owned by the matcher. `sets` retains ownership of the + * named sets. * @return 0 on success, or negative error value on failure. */ -int bf_matcher_new_from_pack(struct bf_matcher **matcher, bf_rpack_node_t node); +int bf_matcher_new_from_pack(struct bf_matcher **matcher, bf_rpack_node_t node, + const bf_hashset *sets); /** - * @brief Allocate an independent copy of a matcher and its payload. + * @brief Allocate a copy of a matcher and its payload. + * + * Anonymous sets are deep-copied. Named sets remain borrowed from the same + * owner as the source matcher. * - * Set matchers copy the set name. The referenced set, if any, is not copied. + * @warning If you manually deep-copy a chain (i.e. without + * `bf_chain_new_from_copy`), you also need to update the reference to named + * sets contained in the matchers. * * @param dest Destination matcher. Unchanged on failure. Can't be NULL. * @param src Matcher to copy. Can't be NULL. diff --git a/src/libbpfilter/include/bpfilter/rule.h b/src/libbpfilter/include/bpfilter/rule.h index 7356ca48..191b0341 100644 --- a/src/libbpfilter/include/bpfilter/rule.h +++ b/src/libbpfilter/include/bpfilter/rule.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -112,16 +113,28 @@ int bf_rule_new(struct bf_rule **rule); * data. The caller will own the object. On failure, `*rule` is * unchanged. Can't be NULL. * @param node Node containing the serialized rule. Can't be NULL. + * @param sets Sets used to resolve named matcher references. Can be NULL if + * no matcher references a named set. * @return 0 on success, or a negative errno value on failure. */ -int bf_rule_new_from_pack(struct bf_rule **rule, bf_rpack_node_t node); +int bf_rule_new_from_pack(struct bf_rule **rule, bf_rpack_node_t node, + const bf_hashset *sets); /** - * @brief Allocate an independent copy of a rule and its matchers. + * @brief Allocate a copy of a rule and its matchers. + * + * Anonymous sets are deep-copied. Named sets remain borrowed from the same + * owner as the source rule. That owner must outlive the copied rule, including + * its destruction. * * Runtime counters and disabled state are reset. Adding the copy to a chain * assigns its index, validates set references, and recalculates disabled state. * + * @warning Before inserting the copy into another chain, replace its named-set + * matchers with matchers that borrow that chain's sets. Matching names alone + * do not rebind references. To copy a complete chain, use + * `bf_chain_new_from_copy()`, which rebinds references to the copied sets. + * * @param dest Destination rule. Unchanged on failure. Can't be NULL. * @param src Rule to copy. Can't be NULL. * @return 0 on success, or a negative errno value on failure. @@ -158,14 +171,13 @@ void bf_rule_dump(const struct bf_rule *rule, prefix_t *prefix); * Create a new matcher and add it to the rule. * * @param rule Rule to add the matcher to. Can't be NULL. - * @param type Matcher type. + * @param type Matcher type. `BF_MATCHER_SET` is not supported. Create set + * matchers with their constructors and add them to `rule->matchers`. * @param op Comparison operator. * @param payload Payload of the matcher, its content and size depends on @p - * type . Can be NULL but only if @p payload_len is 0, in which case - * there is no payload. For BF_MATCHER_SET, pass a pointer to a bf_set. - * The matcher stores an owned copy of its name. - * @param payload_len Length of the input payload. For BF_MATCHER_SET, pass - * sizeof(struct bf_set). + * type . Can be NULL but only if `payload_len` is 0, in which case + * there is no payload. + * @param payload_len Length of the input payload. * @param negate If true, negate the matcher's comparison. * @return 0 on success, or negative errno value on failure. */ diff --git a/src/libbpfilter/include/bpfilter/set.h b/src/libbpfilter/include/bpfilter/set.h index e55d6b31..3ffcf873 100644 --- a/src/libbpfilter/include/bpfilter/set.h +++ b/src/libbpfilter/include/bpfilter/set.h @@ -44,8 +44,7 @@ */ struct bf_set { - /** Immutable name of the set. Anonymous sets receive a generated name. - * Never NULL. */ + /** Immutable name of the set. NULL for anonymous sets. */ const char *name; /** Key of the set. Can't contain more than `BF_SET_MAX_N_COMPS` types. */ @@ -85,7 +84,7 @@ struct bf_set bool bf_set_is_valid_name(const char *name); /** - * @brief Check whether a set has a generated name. + * @brief Check whether a set is anonymous (no name). * * @param set Set to check. Can't be NULL. * @return true if the set is anonymous, false if it has a user-defined name. @@ -93,7 +92,7 @@ bool bf_set_is_valid_name(const char *name); bool bf_set_is_anonymous(const struct bf_set *set); /** - * @brief Initialize an empty owning collection of sets, keyed by name. + * @brief Initialize an empty owning collection of named sets. * * @return An empty hashset that preserves insertion order and frees its sets. */ @@ -104,7 +103,7 @@ bf_hashset bf_set_hashset_default(void); * * @param set Set to allocate and initialise. Can't be NULL. * @param name Name of the set. Non-NULL names must satisfy - * `bf_set_is_valid_name()`. + * `bf_set_is_valid_name()`. NULL creates an anonymous set. * @param key Key of the set, as an array of `bf_matcher_type`. Not all the * matcher types can be used as set key components. Can't be NULL. * @param n_comps Number of components in `key`. @@ -118,7 +117,7 @@ int bf_set_new(struct bf_set **set, const char *name, * * @param set Set to allocate and initialise. Can't be NULL. * @param name Name of the set. Non-NULL names must satisfy - * `bf_set_is_valid_name()`. + * `bf_set_is_valid_name()`. NULL creates an anonymous set. * @param raw_key Set key, as a string of comma-separated matcher types enclosed * in parentheses. Can't be NULL. * @param raw_payload Set payload, to parse according to `raw_key`. Can't be @@ -131,9 +130,6 @@ int bf_set_new_from_raw(struct bf_set **set, const char *name, /** * @brief Allocate and initialize a new set from serialized data. * - * Validates the stored name, accepting user-defined and internal generated - * names. - * * @param set Set object to allocate and initialize from the serialized data. * The caller will own the object. On failure, `*set` is unchanged. * Can't be NULL. diff --git a/src/libbpfilter/matcher.c b/src/libbpfilter/matcher.c index 3fee0f11..0a59eb3a 100644 --- a/src/libbpfilter/matcher.c +++ b/src/libbpfilter/matcher.c @@ -1369,10 +1369,8 @@ static int _bf_matcher_new(struct bf_matcher **matcher, assert(matcher); assert((payload && payload_len) || (!payload && !payload_len)); - if (type == BF_MATCHER_SET && - (op != BF_MATCHER_IN || !payload || payload_len < 2 || - memchr(payload, '\0', payload_len) != - (const char *)payload + payload_len - 1)) + if (type == BF_MATCHER_SET && (op != BF_MATCHER_IN || !payload || + payload_len != sizeof(struct bf_set *))) return bf_err_r(-EINVAL, "invalid set matcher payload or operator"); if (payload_len > SIZE_MAX - sizeof(*_matcher)) @@ -1399,17 +1397,92 @@ int bf_matcher_new(struct bf_matcher **matcher, enum bf_matcher_type type, { assert(matcher); - if (type == BF_MATCHER_SET) { - const struct bf_set *set = payload; + if (type == BF_MATCHER_SET) + return bf_err_r(-EINVAL, "use a set matcher constructor"); + + return _bf_matcher_new(matcher, type, op, payload, payload_len, negate); +} + +int bf_matcher_new_for_named_set(struct bf_matcher **matcher, + const struct bf_set *set, bool negate) +{ + assert(matcher); + + if (!set || !bf_set_is_valid_name(set->name)) + return bf_err_r(-EINVAL, "set matcher requires a named set"); + + return _bf_matcher_new(matcher, BF_MATCHER_SET, BF_MATCHER_IN, &set, + sizeof(struct bf_set *), negate); +} - if (!set || payload_len != sizeof(*set) || !set->name) - return bf_err_r(-EINVAL, "set matcher requires a set object"); +static int _bf_matcher_new_for_anon_set(struct bf_matcher **matcher, + struct bf_set **set, bool negate) +{ + int r; + + assert(matcher); + assert(set && *set); + + r = _bf_matcher_new(matcher, BF_MATCHER_SET, BF_MATCHER_IN, set, + sizeof(struct bf_set *), negate); + if (r) + return r; + + TAKE_PTR(*set); - payload = set->name; - payload_len = strlen(set->name) + 1; + return 0; +} + +int bf_matcher_new_for_anon_set(struct bf_matcher **matcher, + const enum bf_matcher_type *key, size_t n_comps, + const void * const *elems, size_t n_elems, + bool negate) +{ + _free_bf_set_ struct bf_set *set = NULL; + int r; + + assert(matcher); + assert(key); + + if (n_elems && !elems) + return bf_err_r(-EINVAL, "missing anonymous set elements"); + + r = bf_set_new(&set, NULL, key, n_comps); + if (r) + return r; + + r = bf_set_reserve(set, n_elems); + if (r) + return r; + + for (size_t i = 0; i < n_elems; ++i) { + if (!elems[i]) + return bf_err_r(-EINVAL, "missing anonymous set element"); + + r = bf_set_add_elem(set, elems[i]); + if (r) + return r; } - return _bf_matcher_new(matcher, type, op, payload, payload_len, negate); + return _bf_matcher_new_for_anon_set(matcher, &set, negate); +} + +int bf_matcher_new_for_anon_set_from_raw(struct bf_matcher **matcher, + const char *raw_key, + const char *raw_payload, bool negate) +{ + _free_bf_set_ struct bf_set *set = NULL; + int r; + + assert(matcher); + assert(raw_key); + assert(raw_payload); + + r = bf_set_new_from_raw(&set, NULL, raw_key, raw_payload); + if (r) + return r; + + return _bf_matcher_new_for_anon_set(matcher, &set, negate); } int bf_matcher_new_from_raw(struct bf_matcher **matcher, @@ -1447,7 +1520,8 @@ int bf_matcher_new_from_raw(struct bf_matcher **matcher, return 0; } -int bf_matcher_new_from_pack(struct bf_matcher **matcher, bf_rpack_node_t node) +int bf_matcher_new_from_pack(struct bf_matcher **matcher, bf_rpack_node_t node, + const bf_hashset *sets) { _free_bf_matcher_ struct bf_matcher *_matcher = NULL; _cleanup_free_ char *set_name = NULL; @@ -1475,18 +1549,53 @@ int bf_matcher_new_from_pack(struct bf_matcher **matcher, bf_rpack_node_t node) } if (type == BF_MATCHER_SET) { - r = bf_rpack_kv_str(node, "payload", &set_name); - if (!r) { - payload = set_name; - payload_len = strlen(set_name) + 1; + const struct bf_set *set; + + if (op != BF_MATCHER_IN) + return bf_err_r(-EINVAL, "invalid set matcher operator"); + + if (bf_rpack_kv_contains(node, "set")) { + _free_bf_set_ struct bf_set *anonymous = NULL; + bf_rpack_node_t set_node; + + if (bf_rpack_kv_contains(node, "payload")) + return bf_err_r(-EINVAL, "ambiguous set matcher payload"); + + r = bf_rpack_kv_obj(node, "set", &set_node); + if (r) + return bf_rpack_key_err(r, "bf_matcher.set"); + + r = bf_set_new_from_pack(&anonymous, set_node); + if (r) + return r; + if (!bf_set_is_anonymous(anonymous)) { + return bf_err_r(-EINVAL, + "inline matcher set must be anonymous"); + } + + return _bf_matcher_new_for_anon_set(matcher, &anonymous, negate); } + + r = bf_rpack_kv_str(node, "payload", &set_name); + if (r) + return bf_rpack_key_err(r, "bf_matcher.payload"); + + if (!sets || + !(set = bf_hashset_get(sets, &(struct bf_set) {.name = set_name}))) + return bf_err_r(-ENOENT, "set '%s' not found", set_name); + + payload = set; + payload_len = sizeof(*set); } else { r = bf_rpack_kv_bin(node, "payload", &payload, &payload_len); + if (r) + return bf_rpack_key_err(r, "bf_matcher.payload"); } - if (r) - return bf_rpack_key_err(r, "bf_matcher.payload"); - r = _bf_matcher_new(&_matcher, type, op, payload, payload_len, negate); + if (type == BF_MATCHER_SET) + r = bf_matcher_new_for_named_set(&_matcher, payload, negate); + else + r = bf_matcher_new(&_matcher, type, op, payload, payload_len, negate); if (r) return bf_err_r(r, "failed to create bf_matcher from pack"); @@ -1511,6 +1620,31 @@ int bf_matcher_new_from_copy(struct bf_matcher **dest, payload_len = src->len - sizeof(*src); + if (src->type == BF_MATCHER_SET) { + const struct bf_set *set; + + if (src->op != BF_MATCHER_IN || payload_len != sizeof(struct bf_set *)) + return bf_err_r(-EINVAL, + "invalid source set matcher payload or operator"); + + memcpy(&set, src->payload, sizeof(struct bf_set *)); + if (!set) + return bf_err_r(-EINVAL, "missing source set matcher payload"); + + if (bf_set_is_anonymous(set)) { + _free_bf_set_ struct bf_set *copy = NULL; + int r; + + r = bf_set_new_from_copy(©, set); + if (r) + return r; + + return _bf_matcher_new_for_anon_set(dest, ©, src->negate); + } + + return bf_matcher_new_for_named_set(dest, set, src->negate); + } + return _bf_matcher_new(dest, src->type, src->op, payload_len ? src->payload : NULL, payload_len, src->negate); @@ -1523,6 +1657,14 @@ void bf_matcher_free(struct bf_matcher **matcher) if (!*matcher) return; + if ((*matcher)->type == BF_MATCHER_SET) { + struct bf_set *set; + + memcpy(&set, (*matcher)->payload, sizeof(struct bf_set *)); + if (bf_set_is_anonymous(set)) + bf_set_free(&set); + } + free(*matcher); *matcher = NULL; } @@ -1535,11 +1677,25 @@ int bf_matcher_pack(const struct bf_matcher *matcher, bf_wpack_t *pack) bf_wpack_kv_int(pack, "type", matcher->type); bf_wpack_kv_int(pack, "op", matcher->op); bf_wpack_kv_bool(pack, "negate", matcher->negate); - if (matcher->type == BF_MATCHER_SET) - bf_wpack_kv_str(pack, "payload", (const char *)matcher->payload); - else + if (matcher->type == BF_MATCHER_SET) { + const struct bf_set *set; + + memcpy(&set, matcher->payload, sizeof(struct bf_set *)); + if (bf_set_is_anonymous(set)) { + int r; + + bf_wpack_open_object(pack, "set"); + r = bf_set_pack(set, pack); + if (r) + return r; + bf_wpack_close_object(pack); + } else { + bf_wpack_kv_str(pack, "payload", set->name); + } + } else { bf_wpack_kv_bin(pack, "payload", matcher->payload, matcher->len - sizeof(*matcher)); + } return bf_wpack_is_valid(pack) ? 0 : -EINVAL; } diff --git a/src/libbpfilter/rule.c b/src/libbpfilter/rule.c index 3c181c96..482e738d 100644 --- a/src/libbpfilter/rule.c +++ b/src/libbpfilter/rule.c @@ -66,7 +66,8 @@ int bf_rule_new(struct bf_rule **rule) return 0; } -int bf_rule_new_from_pack(struct bf_rule **rule, bf_rpack_node_t node) +int bf_rule_new_from_pack(struct bf_rule **rule, bf_rpack_node_t node, + const bf_hashset *sets) { _free_bf_rule_ struct bf_rule *_rule = NULL; bf_rpack_node_t m_nodes, m_node; @@ -134,7 +135,7 @@ int bf_rule_new_from_pack(struct bf_rule **rule, bf_rpack_node_t node) _free_bf_matcher_ struct bf_matcher *matcher = NULL; r = bf_list_emplace(&_rule->matchers, bf_matcher_new_from_pack, matcher, - m_node); + m_node, sets); if (r) { return bf_err_r( r, "failed to unpack bf_matcher into bf_rule.matchers"); diff --git a/src/libbpfilter/set.c b/src/libbpfilter/set.c index e2992a8e..6b7cf319 100644 --- a/src/libbpfilter/set.c +++ b/src/libbpfilter/set.c @@ -10,8 +10,6 @@ #include #include #include -#include -#include #include "bpfilter/core/hashset.h" #include "bpfilter/dump.h" @@ -21,9 +19,6 @@ #define _BF_SET_NAME_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-" -#define _BF_SET_ANON_NAME_PREFIX "@anon:" -#define _BF_SET_ANON_NAME_ID_LEN 16 -#define _BF_SET_ANON_NAME_ID_CHARS "0123456789abcdef" /// Mask value of matcher types supporting LPM trie maps. #define _BF_SET_USE_TRIE_MASK \ @@ -97,43 +92,20 @@ bool bf_set_is_valid_name(const char *name) bool bf_set_is_anonymous(const struct bf_set *set) { assert(set); - assert(set->name); - return strncmp(set->name, _BF_SET_ANON_NAME_PREFIX, - sizeof(_BF_SET_ANON_NAME_PREFIX) - 1) == 0; + return !set->name; } -static bool _bf_set_is_valid_internal_name(const char *name) -{ - const char *identifier; - - assert(name); - - if (bf_set_is_valid_name(name)) - return true; - - if (strncmp(name, _BF_SET_ANON_NAME_PREFIX, - sizeof(_BF_SET_ANON_NAME_PREFIX) - 1) != 0) - return false; - - identifier = name + sizeof(_BF_SET_ANON_NAME_PREFIX) - 1; - - return strlen(identifier) == _BF_SET_ANON_NAME_ID_LEN && - strspn(identifier, _BF_SET_ANON_NAME_ID_CHARS) == - _BF_SET_ANON_NAME_ID_LEN; -} - -static int _bf_set_new(struct bf_set **set, const char *name, - const enum bf_matcher_type *key, size_t n_comps) +int bf_set_new(struct bf_set **set, const char *name, + const enum bf_matcher_type *key, size_t n_comps) { _free_bf_set_ struct bf_set *_set = NULL; uint64_t mask = 0; assert(set); - assert(name); assert(key); - if (!_bf_set_is_valid_internal_name(name)) + if (name && !bf_set_is_valid_name(name)) return bf_err_r(-EINVAL, "invalid set name '%s'", name); if (n_comps == 0) @@ -149,9 +121,11 @@ static int _bf_set_new(struct bf_set **set, const char *name, if (!_set) return -ENOMEM; - _set->name = strdup(name); - if (!_set->name) - return bf_err_r(-ENOMEM, "failed to allocate memory for set name"); + if (name) { + _set->name = strdup(name); + if (!_set->name) + return bf_err_r(-ENOMEM, "failed to allocate memory for set name"); + } memcpy(&_set->key, key, n_comps * sizeof(enum bf_matcher_type)); _set->n_comps = n_comps; @@ -186,36 +160,6 @@ static int _bf_set_new(struct bf_set **set, const char *name, return 0; } -int bf_set_new(struct bf_set **set, const char *name, - const enum bf_matcher_type *key, size_t n_comps) -{ - char generated_name[sizeof(_BF_SET_ANON_NAME_PREFIX) + - _BF_SET_ANON_NAME_ID_LEN] = _BF_SET_ANON_NAME_PREFIX; - - assert(set); - assert(key); - - if (name && !bf_set_is_valid_name(name)) - return bf_err_r(-EINVAL, "invalid user-defined set name '%s'", name); - - // Generate a random identifier for anonymous sets. - if (!name) { - uint8_t random_bytes[_BF_SET_ANON_NAME_ID_LEN]; - char *identifier = - generated_name + sizeof(_BF_SET_ANON_NAME_PREFIX) - 1; - - if (getentropy(random_bytes, sizeof(random_bytes)) < 0) - return bf_err_r(-errno, "failed to generate anonymous set name"); - - for (size_t i = 0; i < ARRAY_SIZE(random_bytes); ++i) - identifier[i] = _BF_SET_ANON_NAME_ID_CHARS[random_bytes[i] % 16]; - - name = generated_name; - } - - return _bf_set_new(set, name, key, n_comps); -} - /** * @brief Parse a set's raw key into an array of `bf_matcher_type`. * @@ -449,10 +393,16 @@ int bf_set_new_from_pack(struct bf_set **set, bf_rpack_node_t node) assert(set); - r = bf_rpack_kv_str(node, "name", &name); + r = bf_rpack_kv_node(node, "name", &child); if (r) return bf_rpack_key_err(r, "bf_set.name"); + if (!bf_rpack_is_nil(child)) { + r = bf_rpack_str(child, &name); + if (r) + return bf_rpack_key_err(r, "bf_set.name"); + } + r = bf_rpack_kv_array(node, "key", &child); if (r) return bf_rpack_key_err(r, "bf_set.key"); @@ -470,7 +420,7 @@ int bf_set_new_from_pack(struct bf_set **set, bf_rpack_node_t node) return bf_rpack_key_err(r, "bf_set.key"); } - r = _bf_set_new(&_set, name, key, n_comps); + r = bf_set_new(&_set, name, key, n_comps); if (r) return bf_err_r(r, "failed to create bf_set from pack"); @@ -513,7 +463,7 @@ int bf_set_new_from_copy(struct bf_set **dest, const struct bf_set *src) assert(dest); assert(src); - r = _bf_set_new(©, src->name, src->key, src->n_comps); + r = bf_set_new(©, src->name, src->key, src->n_comps); if (r) return r; @@ -554,7 +504,10 @@ int bf_set_pack(const struct bf_set *set, bf_wpack_t *pack) assert(set); assert(pack); - bf_wpack_kv_str(pack, "name", set->name); + if (set->name) + bf_wpack_kv_str(pack, "name", set->name); + else + bf_wpack_kv_nil(pack, "name"); bf_wpack_open_array(pack, "key"); for (size_t i = 0; i < set->n_comps; ++i) @@ -581,7 +534,7 @@ void bf_set_dump(const struct bf_set *set, prefix_t *prefix) DUMP(prefix, "struct bf_set at %p", set); bf_dump_prefix_push(prefix); - DUMP(prefix, "name: %s", set->name); + DUMP(prefix, "name: %s", set->name ?: ""); DUMP(prefix, "key: bf_matcher_type[%zu]", set->n_comps); bf_dump_prefix_push(prefix); for (size_t i = 0; i < set->n_comps; ++i) { diff --git a/tests/e2e/rules/set_grouping.sh b/tests/e2e/rules/set_grouping.sh index ea519660..337e2c4d 100755 --- a/tests/e2e/rules/set_grouping.sh +++ b/tests/e2e/rules/set_grouping.sh @@ -35,6 +35,20 @@ ${FROM_NS} ${BFCLI} chain set --from-str "chain isolation BF_HOOK_XDP{ifindex=${ ping -c 1 -W 1 ${NS_IP_ADDR} || { echo "ERROR: ping should have succeeded"; exit 1; } ${FROM_NS} ${BFCLI} chain flush --name isolation +# Named declarations and anonymous matchers must keep independent contents. +${FROM_NS} ${BFCLI} chain set --from-str "chain mixed BF_HOOK_XDP{ifindex=${NS_IFINDEX}} ACCEPT + set unused (ip4.saddr) in { ${HOST_IP_ADDR} } + rule (ip4.saddr) in { 192.0.2.40 } counter DROP + rule (ip4.saddr) in { ${HOST_IP_ADDR} } counter DROP" +(! ping -c 1 -W 1 ${NS_IP_ADDR}) || { echo "ERROR: anonymous set should drop ping"; exit 1; } +test "$(get_counter mixed 0)" = "0" +test "$(get_counter mixed 1)" -ge 1 + +dump=$(${FROM_NS} ${BFCLI} chain get --name mixed) +[[ "${dump}" == *"set unused ("* ]] || { echo "ERROR: unused named declaration was lost"; exit 1; } +[[ "${dump}" != *"@anon:"* ]] || { echo "ERROR: anonymous sets must not have names"; exit 1; } +${FROM_NS} ${BFCLI} chain flush --name mixed + # Multibyte bitmask: more than 8 same-key sets force a 2-byte value # (1 byte per 8 sets). The host's address sits in s8 only (bit_index 8 -> # byte 1, bit 0). Rule 0 references s0 (byte 0, bit 0) and must miss; diff --git a/tests/harness/Chain.hpp b/tests/harness/Chain.hpp index 48692335..48c4f621 100644 --- a/tests/harness/Chain.hpp +++ b/tests/harness/Chain.hpp @@ -6,6 +6,7 @@ #pragma once #include +#include #include extern "C" { @@ -89,29 +90,32 @@ class Chain if (r != 0) throw std::runtime_error("failed to create bf_chain"); + std::unique_ptr owned(chain); + for (const Set &set: _sets) { + if (bf_set_is_anonymous(set.ref().get())) + continue; + auto bfset = set.get(); r = bf_chain_add_set(chain, bfset.get()); if (r != 0) { - bf_chain_free(&chain); throw std::runtime_error("failed to add bf_set to bf_chain"); } - bfset.release(); + std::ignore = bfset.release(); } for (const Rule &rule: _rules) { - auto bfrule = rule.get(); + auto bfrule = rule.get(chain); r = bf_chain_add_rule(chain, bfrule.get()); if (r != 0) { - bf_chain_free(&chain); throw std::runtime_error("failed to add bf_rule to bf_chain"); } - bfrule.release(); + std::ignore = bfrule.release(); } - return std::unique_ptr(chain); + return owned; } }; diff --git a/tests/harness/Matcher.hpp b/tests/harness/Matcher.hpp index 817ccfb4..55a51986 100644 --- a/tests/harness/Matcher.hpp +++ b/tests/harness/Matcher.hpp @@ -13,6 +13,7 @@ #include "Set.hpp" extern "C" { +#include #include } @@ -82,17 +83,37 @@ class Matcher return _negate; } - [[nodiscard]] std::unique_ptr get() const + [[nodiscard]] std::unique_ptr + get(const bf_chain *chain = nullptr) const { struct bf_matcher *matcher = nullptr; - const void *payload = _type == BF_MATCHER_SET ? - static_cast(_set.get()) : - _payload.data(); - size_t payload_len = - _type == BF_MATCHER_SET ? sizeof(bf_set) : _payload.size(); - int r = - bf_matcher_new(&matcher, _type, _op, payload, payload_len, _negate); + const bf_set *set = _set.get(); + if (_type == BF_MATCHER_SET && bf_set_is_anonymous(set)) { + std::vector elements; + + for (auto *node = set->elems.head; node; node = node->next) + elements.push_back(node->data); + + int r = bf_matcher_new_for_anon_set(&matcher, set->key, + set->n_comps, elements.data(), + elements.size(), _negate); + if (r != 0) + throw std::runtime_error( + "failed to create anonymous set matcher"); + + return std::unique_ptr(matcher); + } + + if (_type == BF_MATCHER_SET && chain) + set = bf_chain_get_set_by_name(chain, set->name); + + int r; + if (_type == BF_MATCHER_SET) + r = bf_matcher_new_for_named_set(&matcher, set, _negate); + else + r = bf_matcher_new(&matcher, _type, _op, _payload.data(), + _payload.size(), _negate); if (r != 0) throw std::runtime_error("failed to create bf_matcher"); diff --git a/tests/harness/Rule.hpp b/tests/harness/Rule.hpp index 11cc8429..e997b58d 100644 --- a/tests/harness/Rule.hpp +++ b/tests/harness/Rule.hpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "Matcher.hpp" @@ -58,7 +59,8 @@ class Rule return *this; } - [[nodiscard]] std::unique_ptr get() const + [[nodiscard]] std::unique_ptr + get(const bf_chain *chain = nullptr) const { struct bf_rule *rule; @@ -74,13 +76,13 @@ class Rule rule->verdict = _verdict; for (const Matcher &matcher: _matchers) { - auto bfmatcher = matcher.get(); + auto bfmatcher = matcher.get(chain); r = bf_list_add_tail(&rule->matchers, bfmatcher.get()); if (r != 0) throw std::runtime_error("failed to add bf_matcher to bf_rule"); - bfmatcher.release(); + std::ignore = bfmatcher.release(); } return owned; diff --git a/tests/harness/Set.hpp b/tests/harness/Set.hpp index 3ba9a7d5..67d86130 100644 --- a/tests/harness/Set.hpp +++ b/tests/harness/Set.hpp @@ -6,8 +6,8 @@ #pragma once #include +#include #include -#include #include extern "C" { @@ -30,14 +30,14 @@ class Set }; std::shared_ptr _prototype; - std::vector> _elements; public: - Set(std::vector key) + Set(std::vector key, std::optional name = {}) { struct bf_set *set = nullptr; - if (bf_set_new(&set, nullptr, key.data(), key.size()) != 0) + if (bf_set_new(&set, name ? name->c_str() : nullptr, key.data(), + key.size()) != 0) throw std::runtime_error("failed to create a new bf_set"); _prototype = std::shared_ptr(set, deleter()); @@ -50,7 +50,8 @@ class Set "number of elements in the vector should match the number of components in the set key"); } - _elements.push_back(element); + if (bf_set_add_elem(_prototype.get(), element.data()) != 0) + throw std::runtime_error("failed to add element to bf_set"); return *this; } @@ -63,16 +64,6 @@ class Set if (r != 0) throw std::runtime_error("failed to create a new bf_set"); - for (const auto &element: _elements) { - r = bf_set_add_elem( - set, const_cast( - reinterpret_cast(element.data()))); - if (r != 0) { - bf_set_free(&set); - throw std::runtime_error("failed to add element to bf_set"); - } - } - return std::unique_ptr(set); } diff --git a/tests/harness/fake.c b/tests/harness/fake.c index 980e39d7..253ada07 100644 --- a/tests/harness/fake.c +++ b/tests/harness/fake.c @@ -83,6 +83,21 @@ const void *bft_get_randomly_filled_buffer(size_t len) return TAKE_PTR(buffer); } +int bft_rule_add_named_set_matcher(struct bf_rule *rule, + const struct bf_set *set, bool negate) +{ + _free_bf_matcher_ struct bf_matcher *matcher = NULL; + int r; + + assert(rule); + + r = bf_matcher_new_for_named_set(&matcher, set, negate); + if (r) + return r; + + return bf_list_push(&rule->matchers, (void **)&matcher); +} + struct bf_chain *bft_chain_dummy(bool with_rules) { _free_bf_chain_ struct bf_chain *chain = NULL; @@ -96,17 +111,16 @@ struct bf_chain *bft_chain_dummy(bool with_rules) if (with_rules) { { // Create a rule with SET, any matcher, SET + _free_bf_set_ struct bf_set *set0 = bft_set_dummy("first", 4); + _free_bf_set_ struct bf_set *set1 = bft_set_dummy("second", 4); _free_bf_rule_ struct bf_rule *rule = NULL; - _free_bf_set_ struct bf_set *set0 = bft_set_dummy(NULL, 4); - _free_bf_set_ struct bf_set *set1 = bft_set_dummy(NULL, 4); uint32_t ip = 0xff; r = bf_rule_new(&rule); if (r) return NULL; - r = bf_rule_add_matcher(rule, BF_MATCHER_SET, BF_MATCHER_IN, set0, - sizeof(*set0), false); + r = bft_rule_add_named_set_matcher(rule, set0, false); if (r) return NULL; @@ -115,8 +129,7 @@ struct bf_chain *bft_chain_dummy(bool with_rules) if (r) return NULL; - r = bf_rule_add_matcher(rule, BF_MATCHER_SET, BF_MATCHER_IN, set1, - sizeof(*set1), false); + r = bft_rule_add_named_set_matcher(rule, set1, false); if (r) return NULL; @@ -148,15 +161,14 @@ struct bf_chain *bft_chain_dummy(bool with_rules) { // Create a rule with a single set, no other matcher + _free_bf_set_ struct bf_set *set0 = bft_set_dummy("third", 4); _free_bf_rule_ struct bf_rule *rule = NULL; - _free_bf_set_ struct bf_set *set0 = bft_set_dummy(NULL, 4); r = bf_rule_new(&rule); if (r) return NULL; - r = bf_rule_add_matcher(rule, BF_MATCHER_SET, BF_MATCHER_IN, set0, - sizeof(*set0), false); + r = bft_rule_add_named_set_matcher(rule, set0, false); if (r) return NULL; @@ -198,8 +210,7 @@ struct bf_chain *bft_chain_with_set(const char *key, const char *elements, rule->verdict = BF_VERDICT_DROP; rule->has_counters = 1; - r = bf_rule_add_matcher(rule, BF_MATCHER_SET, BF_MATCHER_IN, set, - sizeof(*set), negate); + r = bft_rule_add_named_set_matcher(rule, set, negate); if (r) return NULL; diff --git a/tests/harness/fake.h b/tests/harness/fake.h index 709d5f8c..defc8a6b 100644 --- a/tests/harness/fake.h +++ b/tests/harness/fake.h @@ -10,6 +10,9 @@ #include +struct bf_rule; +struct bf_set; + typedef bool (*bft_list_eq_cb)(const void *, const void *); typedef int (*bft_list_dummy_inserter)(bf_list *, void *); @@ -66,5 +69,7 @@ struct bf_chain *bft_chain_with_set(const char *key, const char *elements, bool negate); struct bf_rule *bft_rule_dummy(size_t n_matchers); +int bft_rule_add_named_set_matcher(struct bf_rule *rule, + const struct bf_set *set, bool negate); struct bf_matcher *bft_matcher_dummy(const void *data, size_t data_len); struct bf_set *bft_set_dummy(const char *name, size_t n_elems); diff --git a/tests/harness/test.c b/tests/harness/test.c index 2850d7ce..0951d676 100644 --- a/tests/harness/test.c +++ b/tests/harness/test.c @@ -265,7 +265,8 @@ bool bft_set_eq_ordered(const struct bf_set *lhs, const struct bf_set *rhs) return false; } - return bf_streq(lhs->name, rhs->name) && lhs->n_comps == rhs->n_comps && + return (lhs->name == rhs->name || bf_streq(lhs->name, rhs->name)) && + lhs->n_comps == rhs->n_comps && 0 == memcmp(lhs->key, rhs->key, sizeof(enum bf_matcher_type) * lhs->n_comps) && lhs->use_trie == rhs->use_trie && lhs->min_size == rhs->min_size; @@ -312,6 +313,19 @@ bool bft_rule_equal(const struct bf_rule *rule0, const struct bf_rule *rule1) bool bft_matcher_equal(const struct bf_matcher *matcher0, const struct bf_matcher *matcher1) { + if (bf_matcher_get_type(matcher0) == BF_MATCHER_SET && + bf_matcher_get_type(matcher1) == BF_MATCHER_SET) { + const struct bf_set *set0; + const struct bf_set *set1; + + memcpy(&set0, bf_matcher_payload(matcher0), sizeof(set0)); + memcpy(&set1, bf_matcher_payload(matcher1), sizeof(set1)); + return bf_matcher_get_op(matcher0) == bf_matcher_get_op(matcher1) && + bf_matcher_get_negate(matcher0) == + bf_matcher_get_negate(matcher1) && + bft_set_eq_ordered(set0, set1); + } + return bf_matcher_get_type(matcher0) == bf_matcher_get_type(matcher1) && bf_matcher_get_op(matcher0) == bf_matcher_get_op(matcher1) && bf_matcher_get_negate(matcher0) == bf_matcher_get_negate(matcher1) && diff --git a/tests/unit/libbpfilter/chain.c b/tests/unit/libbpfilter/chain.c index bf82acfd..a85383f8 100644 --- a/tests/unit/libbpfilter/chain.c +++ b/tests/unit/libbpfilter/chain.c @@ -41,6 +41,34 @@ static void new_and_free(void **state) } } +static void new_failure_preserves_inputs(void **state) +{ + _clean_bf_hashset_ bf_hashset sets = bf_set_hashset_default(); + _clean_bf_list_ bf_list rules = bf_list_default(bf_rule_free, NULL); + _free_bf_chain_ struct bf_chain *chain = NULL; + _free_bf_set_ struct bf_set *set = NULL; + _free_bf_rule_ struct bf_rule *rule = NULL; + + (void)state; + + assert_ok( + bf_set_new_from_raw(&set, "addresses", "(ip4.saddr)", "{192.0.2.1}")); + assert_ok(bf_rule_new(&rule)); + assert_ok(bft_rule_add_named_set_matcher(rule, set, false)); + bf_rule_mark_set(rule, 1); + assert_ok(bf_hashset_add(&sets, (void **)&set)); + assert_ok(bf_list_push(&rules, (void **)&rule)); + + // XDP chains cannot set packet marks. + assert_err(bf_chain_new(&chain, "test", BF_HOOK_XDP, BF_VERDICT_ACCEPT, + &sets, &rules)); + assert_null(chain); + assert_int_equal(bf_hashset_size(&sets), 1); + assert_int_equal(bf_list_size(&rules), 1); + assert_non_null( + bf_hashset_get(&sets, &(struct bf_set) {.name = "addresses"})); +} + static void pack_and_unpack(void **state) { _free_bf_chain_ struct bf_chain *chain0 = NULL; @@ -77,6 +105,39 @@ static void copy_preserves_configuration(void **state) assert_true(bft_chain_equal(source, copy)); } +static void copy_rebinds_shared_set_references(void **state) +{ + _free_bf_chain_ struct bf_chain *source = + bft_chain_with_set("(ip4.saddr)", "{192.0.2.1}", false); + _free_bf_chain_ struct bf_chain *copy = NULL; + const struct bf_set *source_set; + const struct bf_set *copied_set; + struct bf_rule *rule; + uint32_t address = htonl(0xc0000201); + + (void)state; + + assert_non_null(source); + source_set = bf_chain_get_set_by_name(source, "addresses"); + rule = bf_list_get_at(&source->rules, 0); + assert_ok(bft_rule_add_named_set_matcher(rule, source_set, true)); + + assert_ok(bf_chain_new_from_copy(©, source)); + copied_set = bf_chain_get_set_by_name(copy, "addresses"); + assert_non_null(copied_set); + assert_ptr_not_equal(source_set, copied_set); + assert_int_equal(bf_hashset_size(©->sets), 1); + bf_chain_free(&source); + + rule = bf_list_get_at(©->rules, 0); + assert_int_equal(bf_list_size(&rule->matchers), 2); + bf_list_foreach (&rule->matchers, node) { + assert_ptr_equal(copied_set, bf_chain_get_set_for_matcher( + copy, bf_list_node_get_data(node))); + } + assert_true(bf_hashset_contains(&copied_set->elems, &address)); +} + static void copy_resets_chain_counters(void **state) { _free_bf_chain_ struct bf_chain *source = bft_chain_dummy(false); @@ -184,20 +245,82 @@ static void copy_missing_set_leaves_destination_unchanged(void **state) bft_chain_with_set("(ip4.saddr)", "{192.0.2.1}", false); _free_bf_chain_ struct bf_chain *destination = bft_chain_dummy(false); struct bf_chain *original = destination; + void *taken = NULL; (void)state; assert_non_null(source); assert_non_null(destination); - assert_ok(bf_hashset_delete(&source->sets, - bf_chain_get_set_by_name(source, "addresses"))); + assert_ok(bf_hashset_take( + &source->sets, bf_chain_get_set_by_name(source, "addresses"), &taken)); assert_err(bf_chain_new_from_copy(&destination, source)); + assert_ok(bf_hashset_add(&source->sets, &taken)); assert_ptr_equal(destination, original); assert_true(bf_list_is_empty(&destination->rules)); assert_true(bf_hashset_is_empty(&destination->sets)); } +static void anonymous_matchers_survive_chain_copy(void **state) +{ + _free_bf_chain_ struct bf_chain *source = + bft_chain_with_set("(ip4.saddr)", "{192.0.2.1}", false); + _free_bf_chain_ struct bf_chain *copy = NULL; + _free_bf_chain_ struct bf_chain *restored = NULL; + _free_bf_set_ struct bf_set *unused = NULL; + _free_bf_matcher_ struct bf_matcher *matcher = NULL; + _free_bf_wpack_ bf_wpack_t *wpack = NULL; + _free_bf_rpack_ bf_rpack_t *rpack = NULL; + struct bf_rule *rule; + const void *data; + size_t data_len; + + (void)state; + + assert_non_null(source); + assert_ok(bf_set_new_from_raw(&unused, "unused", "(tcp.dport)", "{80}")); + assert_ok(bf_chain_add_set(source, unused)); + TAKE_PTR(unused); + rule = bf_list_get_at(&source->rules, 0); + assert_ok(bf_matcher_new_for_anon_set_from_raw(&matcher, "(ip4.saddr)", + "{192.0.2.2}", true)); + assert_ok(bf_list_push(&rule->matchers, (void **)&matcher)); + + assert_ok(bf_chain_new_from_copy(©, source)); + assert_true(bft_chain_equal(source, copy)); + bf_chain_free(&source); + assert_int_equal(bf_hashset_size(©->sets), 2); + assert_non_null(bf_chain_get_set_by_name(copy, "unused")); + + assert_ok(bf_wpack_new(&wpack)); + assert_ok(bf_chain_pack(copy, wpack)); + assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); + assert_ok(bf_rpack_new(&rpack, data, data_len)); + assert_ok(bf_chain_new_from_pack(&restored, bf_rpack_root(rpack))); + assert_true(bft_chain_equal(copy, restored)); + bf_chain_free(©); + + rule = bf_list_get_at(&restored->rules, 0); + bf_list_delete(&rule->matchers, bf_list_get_tail(&rule->matchers)); + assert_int_equal(bf_hashset_size(&restored->sets), 2); + assert_non_null(bf_chain_get_set_by_name(restored, "addresses")); + assert_non_null(bf_chain_get_set_by_name(restored, "unused")); +} + +static void reject_anonymous_chain_set(void **state) +{ + _free_bf_chain_ struct bf_chain *chain = bft_chain_dummy(false); + _free_bf_set_ struct bf_set *set = NULL; + + (void)state; + + assert_non_null(chain); + assert_ok(bf_set_new_from_raw(&set, NULL, "(ip4.saddr)", "{192.0.2.1}")); + assert_int_equal(bf_chain_add_set(chain, set), -EINVAL); + assert_true(bf_hashset_is_empty(&chain->sets)); + assert_ok(bf_set_add_elem_raw(set, "192.0.2.2")); +} + static void dump(void **state) { _free_bf_chain_ struct bf_chain *chain0 = bft_chain_dummy(false); @@ -249,8 +372,8 @@ static void mixed_enabled_disabled_log_flag(void **state) _free_bf_chain_ struct bf_chain *chain = NULL; _clean_bf_hashset_ bf_hashset sets = bf_set_hashset_default(); _clean_bf_list_ bf_list rules = bf_list_default(bf_rule_free, bf_rule_pack); - _free_bf_set_ struct bf_set *set0 = bft_set_dummy(NULL, 0); - _free_bf_set_ struct bf_set *set1 = bft_set_dummy(NULL, 4); + _free_bf_set_ struct bf_set *set0 = bft_set_dummy("empty", 0); + _free_bf_set_ struct bf_set *set1 = bft_set_dummy("populated", 4); struct bf_rule *r0 = NULL; struct bf_rule *r1 = NULL; @@ -260,13 +383,11 @@ static void mixed_enabled_disabled_log_flag(void **state) assert_non_null(set1); assert_ok(bf_rule_new(&r0)); r0->log = 1; - assert_ok(bf_rule_add_matcher(r0, BF_MATCHER_SET, BF_MATCHER_IN, set0, - sizeof(*set0), false)); + assert_ok(bft_rule_add_named_set_matcher(r0, set0, false)); assert_ok(bf_list_add_tail(&rules, r0)); assert_ok(bf_rule_new(&r1)); - assert_ok(bf_rule_add_matcher(r1, BF_MATCHER_SET, BF_MATCHER_IN, set1, - sizeof(*set1), false)); + assert_ok(bft_rule_add_named_set_matcher(r1, set1, false)); assert_ok(bf_list_add_tail(&rules, r1)); assert_ok(bf_hashset_add(&sets, (void **)&set0)); @@ -311,8 +432,7 @@ static void apply_set_delta_updates_derived_state(void **state) rule->log = BF_LOG_OPT_DEFAULT; rule->log_rate_ns = 1; - assert_ok(bf_rule_add_matcher(rule, BF_MATCHER_SET, BF_MATCHER_IN, - matcher_set, sizeof(*matcher_set), false)); + assert_ok(bft_rule_add_named_set_matcher(rule, matcher_set, false)); assert_ok(bf_rule_add_matcher(rule, BF_MATCHER_IP6_NEXTHDR, BF_MATCHER_EQ, &nexthdr, sizeof(nexthdr), false)); assert_ok(bf_list_add_tail(&rules, rule)); @@ -364,13 +484,11 @@ static void empty_set_only_disables_in_rules(void **state) assert_ok(bf_hashset_add(&sets, (void **)&set)); assert_ok(bf_rule_new(&in_rule)); - assert_ok(bf_rule_add_matcher(in_rule, BF_MATCHER_SET, BF_MATCHER_IN, - matcher_set, sizeof(*matcher_set), false)); + assert_ok(bft_rule_add_named_set_matcher(in_rule, matcher_set, false)); assert_ok(bf_list_add_tail(&rules, in_rule)); assert_ok(bf_rule_new(¬_in_rule)); - assert_ok(bf_rule_add_matcher(not_in_rule, BF_MATCHER_SET, BF_MATCHER_IN, - matcher_set, sizeof(*matcher_set), true)); + assert_ok(bft_rule_add_named_set_matcher(not_in_rule, matcher_set, true)); assert_ok(bf_list_add_tail(&rules, not_in_rule)); assert_ok(bf_chain_new(&chain, "test", BF_HOOK_XDP, BF_VERDICT_ACCEPT, @@ -447,9 +565,7 @@ static void incompatible_matchers_disable_rule(void **state) assert_ok(bf_hashset_add(&sets, (void **)&set)); assert_ok(bf_rule_new(&rule)); - assert_ok(bf_rule_add_matcher(rule, BF_MATCHER_SET, BF_MATCHER_IN, - matcher_set, sizeof(*matcher_set), - false)); + assert_ok(bft_rule_add_named_set_matcher(rule, matcher_set, false)); assert_ok(bf_rule_add_matcher(rule, BF_MATCHER_IP6_DADDR, BF_MATCHER_EQ, ip6_addr, sizeof(ip6_addr), false)); assert_ok(bf_list_add_tail(&rules, rule)); @@ -538,16 +654,15 @@ static void set_component_unsupported_hook(void **state) assert_ok(bf_hashset_add(&sets, (void **)&set)); assert_ok(bf_rule_new(&rule)); - assert_ok(bf_rule_add_matcher(rule, BF_MATCHER_SET, BF_MATCHER_IN, - matcher_set, sizeof(*matcher_set), false)); + assert_ok(bft_rule_add_named_set_matcher(rule, matcher_set, false)); assert_ok(bf_list_add_tail(&rules, rule)); assert_err(bf_chain_new(&chain, "test", BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4, BF_VERDICT_ACCEPT, &sets, &rules)); // Supported component: ip4.daddr on CONNECT4 should succeed. - bf_hashset_clean(&sets); bf_list_clean(&rules); + bf_hashset_clean(&sets); enum bf_matcher_type daddr_key[] = {BF_MATCHER_IP4_DADDR}; @@ -557,8 +672,7 @@ static void set_component_unsupported_hook(void **state) assert_ok(bf_hashset_add(&sets, (void **)&set)); assert_ok(bf_rule_new(&rule)); - assert_ok(bf_rule_add_matcher(rule, BF_MATCHER_SET, BF_MATCHER_IN, - matcher_set, sizeof(*matcher_set), false)); + assert_ok(bft_rule_add_named_set_matcher(rule, matcher_set, false)); assert_ok(bf_list_add_tail(&rules, rule)); assert_ok(bf_chain_new(&chain, "test", BF_HOOK_CGROUP_SOCK_ADDR_CONNECT4, @@ -642,6 +756,7 @@ static void named_reference_stability(void **state) _free_bf_set_ struct bf_set *target = NULL; _free_bf_set_ struct bf_set *unrelated = NULL; _free_bf_rule_ struct bf_rule *rule = NULL; + _free_bf_rule_ struct bf_rule *foreign_rule = NULL; struct bf_set unrelated_key = {.name = "unrelated"}; const struct bf_set *stored; struct bf_matcher *matcher; @@ -656,8 +771,7 @@ static void named_reference_stability(void **state) assert_ok( bf_set_new_from_raw(&unrelated, "unrelated", "(ip4.saddr)", "{}")); assert_ok(bf_rule_new(&rule)); - assert_ok(bf_rule_add_matcher(rule, BF_MATCHER_SET, BF_MATCHER_IN, target, - sizeof(*target), false)); + assert_ok(bft_rule_add_named_set_matcher(rule, target, false)); matcher = bf_list_node_get_data(bf_list_get_head(&rule->matchers)); assert_null(bf_chain_get_set_for_matcher(chain, matcher)); assert_err(bf_chain_add_rule(chain, rule)); @@ -691,6 +805,11 @@ static void named_reference_stability(void **state) assert_err(bf_chain_add_set(chain, target)); assert_non_null(target); assert_ptr_equal(stored, bf_chain_get_set_by_name(chain, "target")); + + // A different object with the same name cannot reference the stored set. + assert_ok(bf_rule_new(&foreign_rule)); + assert_ok(bft_rule_add_named_set_matcher(foreign_rule, target, false)); + assert_err(bf_chain_add_rule(chain, foreign_rule)); } static void unpack_duplicate_sets(void **state) @@ -739,14 +858,18 @@ int main(void) { const struct CMUnitTest tests[] = { cmocka_unit_test(new_and_free), + cmocka_unit_test(new_failure_preserves_inputs), cmocka_unit_test(pack_and_unpack), cmocka_unit_test(copy_preserves_configuration), + cmocka_unit_test(copy_rebinds_shared_set_references), cmocka_unit_test(copy_resets_chain_counters), cmocka_unit_test(copy_set_updates_do_not_change_source), cmocka_unit_test(copy_set_matcher_survives_source_free), cmocka_unit_test(copy_preserves_disabled_empty_set_rule), cmocka_unit_test(copy_preserves_negated_empty_set_rule), cmocka_unit_test(copy_missing_set_leaves_destination_unchanged), + cmocka_unit_test(anonymous_matchers_survive_chain_copy), + cmocka_unit_test(reject_anonymous_chain_set), cmocka_unit_test(dump), cmocka_unit_test(get_set_from_matcher), cmocka_unit_test(mixed_enabled_disabled_log_flag), diff --git a/tests/unit/libbpfilter/matcher.c b/tests/unit/libbpfilter/matcher.c index a60c9c3a..be44233d 100644 --- a/tests/unit/libbpfilter/matcher.c +++ b/tests/unit/libbpfilter/matcher.c @@ -40,15 +40,16 @@ static void new_and_free(void **state) assert_non_null(matcher); } -static void set_name_payload(void **state) +static void set_pointer_round_trip(void **state) { enum bf_matcher_type key[] = {BF_MATCHER_IP4_SADDR}; - const char *names[] = {"blocked", NULL}; + const char *names[] = {"blocked", "allowed"}; (void)state; for (size_t i = 0; i < ARRAY_SIZE(names); ++i) { + _clean_bf_hashset_ bf_hashset sets = bf_set_hashset_default(); _free_bf_set_ struct bf_set *set = NULL; _free_bf_matcher_ struct bf_matcher *matcher = NULL; _free_bf_matcher_ struct bf_matcher *restored = NULL; @@ -61,12 +62,8 @@ static void set_name_payload(void **state) assert_ok(bf_set_new(&set, names[i], key, ARRAY_SIZE(key))); assert_non_null(name = strdup(set->name)); - assert_ok(bf_matcher_new(&matcher, BF_MATCHER_SET, BF_MATCHER_IN, set, - sizeof(*set), true)); - assert_string_equal(bf_matcher_payload(matcher), name); - assert_int_equal(bf_matcher_payload_len(matcher), strlen(name) + 1); - bf_set_free(&set); - assert_string_equal(bf_matcher_payload(matcher), name); + assert_ok(bf_matcher_new_for_named_set(&matcher, set, true)); + assert_ok(bf_hashset_add(&sets, (void **)&set)); assert_ok(bf_wpack_new(&wpack)); assert_ok(bf_matcher_pack(matcher, wpack)); @@ -75,11 +72,116 @@ static void set_name_payload(void **state) assert_ok( bf_rpack_kv_str(bf_rpack_root(rpack), "payload", &serialized_name)); assert_string_equal(serialized_name, name); - assert_ok(bf_matcher_new_from_pack(&restored, bf_rpack_root(rpack))); + assert_err( + bf_matcher_new_from_pack(&restored, bf_rpack_root(rpack), NULL)); + assert_null(restored); + assert_ok( + bf_matcher_new_from_pack(&restored, bf_rpack_root(rpack), &sets)); assert_true(bft_matcher_equal(matcher, restored)); + bf_matcher_free(&matcher); + bf_matcher_free(&restored); + assert_ok(bf_set_add_elem_raw( + bf_hashset_get(&sets, &(struct bf_set) {.name = name}), + "192.0.2.1")); } } +static void anonymous_set_copy_and_round_trip(void **state) +{ + _free_bf_matcher_ struct bf_matcher *source = NULL; + _free_bf_matcher_ struct bf_matcher *expected = NULL; + _free_bf_matcher_ struct bf_matcher *copy = NULL; + _free_bf_matcher_ struct bf_matcher *restored = NULL; + _free_bf_wpack_ bf_wpack_t *wpack = NULL; + _free_bf_rpack_ bf_rpack_t *rpack = NULL; + const enum bf_matcher_type key[] = {BF_MATCHER_IP4_SADDR}; + uint32_t addresses[] = {htonl(0xc0000201), htonl(0xc0000202)}; + const void *elements[] = {&addresses[0], &addresses[1], &addresses[0]}; + const void *data; + size_t data_len; + + (void)state; + + assert_ok(bf_matcher_new_for_anon_set( + &source, key, ARRAY_SIZE(key), elements, ARRAY_SIZE(elements), true)); + addresses[0] = 0; + addresses[1] = 0; + assert_ok(bf_matcher_new_for_anon_set_from_raw( + &expected, "(ip4.saddr)", "{192.0.2.1; 192.0.2.2}", true)); + assert_true(bft_matcher_equal(source, expected)); + + assert_ok(bf_matcher_new_from_copy(©, source)); + bf_matcher_free(&source); + assert_true(bft_matcher_equal(copy, expected)); + + assert_ok(bf_wpack_new(&wpack)); + assert_ok(bf_matcher_pack(copy, wpack)); + assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); + assert_ok(bf_rpack_new(&rpack, data, data_len)); + assert_ok(bf_matcher_new_from_pack(&restored, bf_rpack_root(rpack), NULL)); + bf_matcher_free(©); + assert_true(bft_matcher_equal(restored, expected)); +} + +static void anonymous_set_failures_preserve_destination(void **state) +{ + _free_bf_matcher_ struct bf_matcher *matcher = NULL; + const enum bf_matcher_type key[] = {BF_MATCHER_IP4_SADDR}; + uint32_t address = htonl(0xc0000201); + const void *elements[] = {&address, NULL}; + struct bf_matcher *original; + + (void)state; + + assert_ok(bf_matcher_new_for_anon_set_from_raw(&matcher, "(ip4.saddr)", + "{192.0.2.1}", false)); + original = matcher; + assert_err(bf_matcher_new_for_anon_set(&matcher, key, 0, NULL, 0, false)); + assert_ptr_equal(matcher, original); + assert_err(bf_matcher_new_for_anon_set(&matcher, key, ARRAY_SIZE(key), NULL, + 1, false)); + assert_ptr_equal(matcher, original); + assert_err(bf_matcher_new_for_anon_set( + &matcher, key, ARRAY_SIZE(key), elements, ARRAY_SIZE(elements), false)); + assert_ptr_equal(matcher, original); + assert_err(bf_matcher_new_for_anon_set_from_raw( + &matcher, "(ip4.saddr)", "{192.0.2.1; invalid}", false)); + assert_ptr_equal(matcher, original); + assert_err(bf_matcher_new_for_anon_set_from_raw(&matcher, "(invalid)", "{}", + false)); + assert_ptr_equal(matcher, original); +} + +static void anonymous_set_empty(void **state) +{ + _free_bf_matcher_ struct bf_matcher *typed = NULL; + _free_bf_matcher_ struct bf_matcher *raw = NULL; + const enum bf_matcher_type key[] = {BF_MATCHER_TCP_DPORT}; + + (void)state; + + assert_ok(bf_matcher_new_for_anon_set(&typed, key, ARRAY_SIZE(key), NULL, 0, + false)); + assert_ok( + bf_matcher_new_for_anon_set_from_raw(&raw, "(tcp.dport)", "{}", false)); + assert_true(bft_matcher_equal(typed, raw)); +} + +static void anonymous_set_pointer_rejected(void **state) +{ + _free_bf_set_ struct bf_set *set = NULL; + _free_bf_matcher_ struct bf_matcher *matcher = NULL; + + (void)state; + + assert_ok(bf_set_new_from_raw(&set, NULL, "(ip4.saddr)", "{192.0.2.1}")); + assert_int_equal(bf_matcher_new(&matcher, BF_MATCHER_SET, BF_MATCHER_IN, + set, sizeof(*set), false), + -EINVAL); + assert_null(matcher); + assert_ok(bf_set_add_elem_raw(set, "192.0.2.2")); +} + static void set_invalid_input(void **state) { _free_bf_matcher_ struct bf_matcher *matcher = NULL; @@ -98,12 +200,39 @@ static void set_invalid_input(void **state) assert_err(bf_matcher_new(&matcher, BF_MATCHER_SET, BF_MATCHER_IN, &set, sizeof(set), false)); set.name = "blocked"; + assert_err(bf_matcher_new(&matcher, BF_MATCHER_SET, BF_MATCHER_IN, &set, + sizeof(set), false)); assert_err(bf_matcher_new(&matcher, BF_MATCHER_SET, BF_MATCHER_EQ, &set, sizeof(set), false)); assert_null(matcher); } -static void set_legacy_payload_rejected(void **state) +static void named_set_failures_preserve_destination(void **state) +{ + _free_bf_set_ struct bf_set *set = NULL; + _free_bf_matcher_ struct bf_matcher *matcher = NULL; + struct bf_set invalid = {.name = "invalid.name"}; + struct bf_matcher *original; + + (void)state; + + assert_ok(bf_set_new_from_raw(&set, NULL, "(ip4.saddr)", "{192.0.2.1}")); + assert_ok(bf_matcher_new_for_anon_set_from_raw(&matcher, "(tcp.dport)", + "{80}", false)); + original = matcher; + assert_int_equal(bf_matcher_new_for_named_set(&matcher, NULL, false), + -EINVAL); + assert_ptr_equal(matcher, original); + assert_int_equal(bf_matcher_new_for_named_set(&matcher, set, false), + -EINVAL); + assert_ptr_equal(matcher, original); + assert_int_equal(bf_matcher_new_for_named_set(&matcher, &invalid, false), + -EINVAL); + assert_ptr_equal(matcher, original); + assert_ok(bf_set_add_elem_raw(set, "192.0.2.2")); +} + +static void set_binary_payload_rejected(void **state) { _free_bf_matcher_ struct bf_matcher *matcher = NULL; _free_bf_wpack_ bf_wpack_t *wpack = NULL; @@ -120,7 +249,7 @@ static void set_legacy_payload_rejected(void **state) bf_wpack_kv_bin(wpack, "payload", &index, sizeof(index)); assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_err(bf_matcher_new_from_pack(&matcher, bf_rpack_root(rpack))); + assert_err(bf_matcher_new_from_pack(&matcher, bf_rpack_root(rpack), NULL)); assert_null(matcher); } @@ -188,13 +317,14 @@ static void copy_payload_survives_source_free(void **state) bf_matcher_free(©); assert_ok(bf_set_new_from_raw(&set, "addresses", "(ip4.saddr)", "{}")); - assert_ok(bf_matcher_new(&source, BF_MATCHER_SET, BF_MATCHER_IN, set, - sizeof(*set), true)); - bf_set_free(&set); + assert_ok(bf_matcher_new_for_named_set(&source, set, true)); assert_ok(bf_matcher_new_from_copy(©, source)); + assert_true(bft_matcher_equal(source, copy)); bf_matcher_free(&source); - assert_string_equal(bf_matcher_payload(copy), "addresses"); + assert_ok(bf_set_add_elem_raw(set, "192.0.2.1")); + assert_ok(bf_matcher_new_for_named_set(&source, set, true)); + assert_true(bft_matcher_equal(source, copy)); assert_true(bf_matcher_get_negate(copy)); } @@ -234,7 +364,8 @@ static void pack_and_unpack(void **state) // Unpack into destination assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_ok(bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack))); + assert_ok( + bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack), NULL)); // Verify they're equal assert_true(bft_matcher_equal(source, destination)); @@ -261,7 +392,8 @@ static void pack_and_unpack_small_payload(void **state) // Unpack assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_ok(bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack))); + assert_ok( + bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack), NULL)); assert_true(bft_matcher_equal(source, destination)); } @@ -289,7 +421,8 @@ static void pack_and_unpack_with_negate(void **state) assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_ok(bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack))); + assert_ok( + bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack), NULL)); // Verify negate round-trips correctly assert_true(bf_matcher_get_negate(destination)); @@ -314,7 +447,7 @@ static void unpack_without_negate_defaults_false(void **state) assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_ok(bf_matcher_new_from_pack(&matcher, bf_rpack_root(rpack))); + assert_ok(bf_matcher_new_from_pack(&matcher, bf_rpack_root(rpack), NULL)); assert_int_equal(bf_matcher_get_type(matcher), BF_MATCHER_TCP_SPORT); assert_int_equal(bf_matcher_get_op(matcher), BF_MATCHER_EQ); @@ -881,7 +1014,8 @@ static void meta_probability_pack_unpack(void **state) assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_ok(bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack))); + assert_ok( + bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack), NULL)); assert_true(bft_matcher_equal(source, destination)); bf_matcher_free(&source); @@ -897,7 +1031,8 @@ static void meta_probability_pack_unpack(void **state) assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_ok(bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack))); + assert_ok( + bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack), NULL)); assert_true(bft_matcher_equal(source, destination)); } @@ -2071,7 +2206,8 @@ static void meta_flow_probability_pack_unpack(void **state) assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_ok(bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack))); + assert_ok( + bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack), NULL)); assert_true(bft_matcher_equal(source, destination)); bf_matcher_free(&source); @@ -2087,7 +2223,8 @@ static void meta_flow_probability_pack_unpack(void **state) assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_ok(bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack))); + assert_ok( + bf_matcher_new_from_pack(&destination, bf_rpack_root(rpack), NULL)); assert_true(bft_matcher_equal(source, destination)); } @@ -2098,9 +2235,14 @@ int main(void) cmocka_unit_test(new_and_free), cmocka_unit_test(copy_payload_survives_source_free), cmocka_unit_test(copy_preserves_empty_payload), - cmocka_unit_test(set_name_payload), + cmocka_unit_test(set_pointer_round_trip), + cmocka_unit_test(anonymous_set_copy_and_round_trip), + cmocka_unit_test(anonymous_set_failures_preserve_destination), + cmocka_unit_test(anonymous_set_empty), + cmocka_unit_test(anonymous_set_pointer_rejected), cmocka_unit_test(set_invalid_input), - cmocka_unit_test(set_legacy_payload_rejected), + cmocka_unit_test(named_set_failures_preserve_destination), + cmocka_unit_test(set_binary_payload_rejected), cmocka_unit_test(new_with_payload), cmocka_unit_test(getters), cmocka_unit_test(pack_and_unpack), diff --git a/tests/unit/libbpfilter/rule.c b/tests/unit/libbpfilter/rule.c index 10e7c7c5..21def26f 100644 --- a/tests/unit/libbpfilter/rule.c +++ b/tests/unit/libbpfilter/rule.c @@ -38,9 +38,9 @@ static void new_and_free(void **state) static void copy_configuration_and_ownership(void **state) { + _free_bf_set_ struct bf_set *set = NULL; _free_bf_rule_ struct bf_rule *source = bft_rule_dummy(2); _free_bf_rule_ struct bf_rule *copy = NULL; - _free_bf_set_ struct bf_set *set = NULL; const struct bf_matcher *set_matcher; (void)state; @@ -53,8 +53,7 @@ static void copy_configuration_and_ownership(void **state) bf_rule_mark_set(source, 0x1234); bf_rule_set_redirect(source, 8, BF_REDIRECT_EGRESS); assert_ok(bf_set_new_from_raw(&set, "rule_copy_set", "(ip4.saddr)", "{}")); - assert_ok(bf_rule_add_matcher(source, BF_MATCHER_SET, BF_MATCHER_IN, set, - sizeof(*set), true)); + assert_ok(bft_rule_add_named_set_matcher(source, set, true)); assert_ok(bf_rule_new_from_copy(©, source)); assert_true(bft_rule_equal(source, copy)); @@ -67,7 +66,9 @@ static void copy_configuration_and_ownership(void **state) bf_rule_free(&source); set_matcher = bf_list_node_get_data(bf_list_get_tail(©->matchers)); - assert_string_equal(bf_matcher_payload(set_matcher), "rule_copy_set"); + assert_int_equal(bf_matcher_get_type(set_matcher), BF_MATCHER_SET); + assert_true(bf_matcher_get_negate(set_matcher)); + assert_ok(bf_set_add_elem_raw(set, "192.0.2.1")); } static void copy_failure_preserves_destination(void **state) @@ -109,7 +110,7 @@ static void pack_and_unpack(void **state) // Unpack in the destination counter assert_ok(bf_rpack_new(&rpack, data, data_len)); assert_ok(bf_rpack_kv_obj(bf_rpack_root(rpack), "rule", &node)); - assert_ok(bf_rule_new_from_pack(&destination, node)); + assert_ok(bf_rule_new_from_pack(&destination, node, NULL)); assert_true(bft_rule_equal(source, destination)); } @@ -129,7 +130,7 @@ static void unpack_error(void **state) assert_ok(bf_wpack_get_data(wpack, &data, &data_len)); assert_ok(bf_rpack_new(&rpack, data, data_len)); - assert_err(bf_rule_new_from_pack(&destination, bf_rpack_root(rpack))); + assert_err(bf_rule_new_from_pack(&destination, bf_rpack_root(rpack), NULL)); } static void dump(void **state) diff --git a/tests/unit/libbpfilter/set.c b/tests/unit/libbpfilter/set.c index 8d2be1f0..397ee076 100644 --- a/tests/unit/libbpfilter/set.c +++ b/tests/unit/libbpfilter/set.c @@ -35,7 +35,6 @@ static void name_is_valid(void **state) static void new_with_invalid_names(void **state) { _free_bf_set_ struct bf_set *set = NULL; - _clean_bft_mock_ bft_mock getentropy_mock = bft_mock_default(); enum bf_matcher_type key[] = {BF_MATCHER_IP4_SADDR}; const char *names[] = { "", "two words", "name.with.dot", "name/slash", "caf\xc3\xa9", @@ -50,13 +49,6 @@ static void new_with_invalid_names(void **state) assert_null(set); } - getentropy_mock = bft_mock_get(getentropy); - assert_err(bf_set_new(&set, NULL, key, ARRAY_SIZE(key))); - assert_null(set); - assert_err(bf_set_new_from_raw(&set, NULL, "(ip4.saddr)", "{}")); - assert_null(set); - - // User-defined names do not require entropy. assert_ok(bf_set_new(&set, "named", key, ARRAY_SIZE(key))); assert_false(bf_set_is_anonymous(set)); } @@ -66,7 +58,6 @@ static void new_and_free(void **state) _free_bf_set_ struct bf_set *set = NULL; _free_bf_set_ struct bf_set *other = NULL; _free_bf_set_ struct bf_set *copy = NULL; - _clean_bft_mock_ bft_mock getentropy_mock = bft_mock_default(); enum bf_matcher_type key[] = {BF_MATCHER_IP4_SADDR}; (void)state; @@ -83,29 +74,20 @@ static void new_and_free(void **state) // Free set using the cleanup attribute assert_ok(bf_set_new(&set, NULL, key, ARRAY_SIZE(key))); - assert_non_null(set->name); + assert_null(set->name); assert_true(bf_set_is_anonymous(set)); - assert_false(bf_set_is_valid_name(set->name)); - assert_string_not_equal(set->name, ""); - - assert_err(bf_set_new(©, set->name, key, ARRAY_SIZE(key))); - assert_null(copy); - assert_err(bf_set_new_from_raw(©, set->name, "(ip4.saddr)", "{}")); - assert_null(copy); assert_ok(bf_set_new_from_raw(&other, NULL, "(ip4.saddr)", "{192.0.2.1}")); assert_true(bf_set_is_anonymous(other)); - assert_string_not_equal(set->name, other->name); + assert_null(other->name); - // Copying must preserve the name without generating a new one. - getentropy_mock = bft_mock_get(getentropy); + // Copies of anonymous sets remain anonymous. assert_ok(bf_set_new_from_copy(©, other)); assert_true(bf_set_is_anonymous(copy)); - assert_string_equal(copy->name, other->name); + assert_null(copy->name); assert_true(bft_set_eq_ordered(copy, other)); - // After the source is freed, the copy must retain its anonymous name - // and original element while accepting another element. + // The copy remains independent after the source is freed. bf_set_free(&other); assert_true(bf_set_is_anonymous(copy)); @@ -281,7 +263,6 @@ static void pack_and_unpack_anon(void **state) _free_bf_set_ struct bf_set *copy = NULL; _free_bf_wpack_ bf_wpack_t *wpack = NULL; _free_bf_rpack_ bf_rpack_t *rpack = NULL; - _clean_bft_mock_ bft_mock getentropy_mock = bft_mock_default(); bf_rpack_node_t node; const void *data; size_t data_len; @@ -293,7 +274,6 @@ static void pack_and_unpack_anon(void **state) assert_true(bf_set_is_anonymous(source)); source->min_size = 16; - getentropy_mock = bft_mock_get(getentropy); assert_ok(bf_wpack_new(&wpack)); bf_wpack_open_object(wpack, "set"); assert_ok(bf_set_pack(source, wpack)); @@ -399,7 +379,8 @@ static void copy_empty_set_accepts_elements_after_source_free(void **state) static void pack_and_unpack_err(void **state) { const char *names[] = { - "", "two words", "name.with.dot", "name/slash", "caf\xc3\xa9", + "", "two words", "name.with.dot", + "name/slash", "caf\xc3\xa9", "@anon:0123456789abcdef", }; (void)state;