summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* tests: shell: skip flowtable-uaf if we lack table owner supportFlorian Westphal2023-09-222-0/+7
| | | | Signed-off-by: Florian Westphal <fw@strlen.de>
* parser_json: Default meter size to zeroPhil Sutter2023-09-221-1/+1
| | | | | | | | JSON parser was missed when performing the same change in standard syntax parser. Fixes: c2cad53ffc22a ("meters: do not set a defaut meter size from userspace") Signed-off-by: Phil Sutter <phil@nwl.cc>
* parser_json: Catch nonsense ops in match statementPhil Sutter2023-09-221-4/+9
| | | | | | | | Since expr_op_symbols array includes binary operators and more, simply checking the given string matches any of the elements is not sufficient. Fixes: 586ad210368b7 ("libnftables: Implement JSON parser") Signed-off-by: Phil Sutter <phil@nwl.cc>
* parser_json: Wrong check in json_parse_ct_timeout_policy()Phil Sutter2023-09-221-1/+1
| | | | | | | | | The conditional around json_unpack() was meant to accept a missing policy attribute. But the accidentally inverted check made the function either ignore a given policy or access uninitialized memory. Fixes: c82a26ebf7e9f ("json: Add ct timeout support") Signed-off-by: Phil Sutter <phil@nwl.cc>
* parser_json: Fix synproxy object mss/wscale parsingPhil Sutter2023-09-221-3/+4
| | | | | | | | The fields are 16 and 8 bits in size, introduce temporary variables to parse into. Fixes: f44ab88b1088e ("src: add synproxy stateful object support") Signed-off-by: Phil Sutter <phil@nwl.cc>
* parser_json: Fix limit object burst value parsingPhil Sutter2023-09-221-1/+1
| | | | | | | The field is of type uint32_t, use lower case 'i' format specifier. Fixes: c36288dbe2ba3 ("JSON: Fix parsing and printing of limit objects") Signed-off-by: Phil Sutter <phil@nwl.cc>
* parser_json: Fix flowtable prio value parsingPhil Sutter2023-09-221-1/+1
| | | | | | | | Using format specifier 'I' requires a 64bit variable to write into. The temporary variable 'prio' is of type int, though. Fixes: 586ad210368b7 ("libnftables: Implement JSON parser") Signed-off-by: Phil Sutter <phil@nwl.cc>
* parser_json: Proper ct expectation attribute parsingPhil Sutter2023-09-221-6/+7
| | | | | | | | | Parts of the code were unsafe (parsing 'I' format into uint32_t), the rest just plain wrong (parsing 'o' format into char *tmp). Introduce a temporary int variable to parse into. Fixes: 1dd08fcfa07a4 ("src: add ct expectations support") Signed-off-by: Phil Sutter <phil@nwl.cc>
* parser_json: Fix typo in json_parse_cmd_add_object()Phil Sutter2023-09-221-1/+1
| | | | | | | | A case of bad c'n'p in the fixed commit broke ct timeout objects parsing. Fixes: c7a5401943df8 ("parser_json: Fix for ineffective family value checks") Signed-off-by: Phil Sutter <phil@nwl.cc>
* parser_json: Catch wrong "reset" payloadPhil Sutter2023-09-221-1/+8
| | | | | | | | | | | | The statement happily accepted any valid expression as payload and assumed it to be a tcpopt expression (actually, a special case of exthdr). Add a check to make sure this is the case. Standard syntax does not provide this flexibility, so no need to have the check there as well. Fixes: 5d837d270d5a8 ("src: add tcp option reset support") Signed-off-by: Phil Sutter <phil@nwl.cc>
* tests: shell: add feature probe for sctp chunk matchingFlorian Westphal2023-09-212-10/+23
| | | | | | Skip the relavant parts of the test if nft_exthdr lacks sctp support. Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: shell: add feature probe for sets with more than one elementFlorian Westphal2023-09-215-0/+23
| | | | | | | | | | Kernels < 5.11 can handle only one expression per element, e.g. its possible to attach a counter per key, or a rate limiter, or a quota, but not two at the same time. Add a probe file and skip the relevant tests if the feature is absent. Signed-off-by: Florian Westphal <fw@strlen.de>
* tests: shell: skip adding catchall elements if unuspportedFlorian Westphal2023-09-211-2/+6
| | | | | | | | | | The test fails on kernels without catchall support, so elide this small part. No need to skip the test in this case, the dump file validates that the added elements are no longer there after the timeout. Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: honor NFT_TEST_FAIL_ON_SKIP variable to fail on any skipped testsThomas Haller2023-09-211-1/+7
| | | | | | | | | | | | | | | | | | The test suite should pass with various kernels and build configurations. Of course, that means, that some tests will be gracefully skipped, and we don't treat that as an overall failure. However, it should be possible to run a specific kernel (net-next?) and build configuration, where we expect that all tests pass. Add an option to fail the run, if any tests were skipped. This is to ensure that we don't have broken tests that never pass. This will make more sense with automated CI is running, to enable on a test system and ensure that at least on that system, all tests pass. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* datatype: return const pointer from datatype_get()Thomas Haller2023-09-212-2/+2
| | | | | | | | | | | | "struct datatype" is for the most part immutable, and most callers deal with const pointers. That's why datatype_get() accepts a const pointer to increase the reference count (mutating the refcnt field). It should also return a const pointer. In fact, all callers are fine with that already. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* payload: use enum icmp_hdr_field_type in payload_may_dependency_kill_icmp()Thomas Haller2023-09-201-6/+4
| | | | | | | Don't mix icmp_dep (enum icmp_hdr_field_type) and the uint8_t icmp_type. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* datatype: use "enum byteorder" instead of int in set_datatype_alloc()Thomas Haller2023-09-203-3/+3
| | | | | | | Use the enum types as we have them. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* netlink: handle invalid etype in set_make_key()Thomas Haller2023-09-201-0/+2
| | | | | | | | | It's not clear to me, what ensures that the etype is always valid. Handle a NULL. Fixes: 6e48df5329ea ('src: add "typeof" build/parse/print support') Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* include: fix missing definitions in <cache.h>/<headers.h>Thomas Haller2023-09-202-0/+11
| | | | | | | | | | | | | The headers should be self-contained so they can be included in any order. With exception of <nft.h>, which any internal header can rely on. Some fixes for <cache.h>/<headers.h>. In case of <cache.h>, forward declare some of the structs instead of including the headers. <headers.h> uses struct in6_addr. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* proto: add missing proto_definitions for PROTO_DESC_GENEVEThomas Haller2023-09-201-1/+2
| | | | | | | | | | While at it, make proto_definitions const. For global variables, this allows the linker to mark the memory as read only. It's just good to do by default. Fixes: 156d22654003 ("src: add geneve matching support") Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* src: fix indentation/whitespaceThomas Haller2023-09-201-2/+2
| | | | | Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* datatype: initialize TYPE_CT_EVENTBIT slot in datatype arrayPablo Neira Ayuso2023-09-203-1/+3
| | | | | | | | | | | | Matching on ct event makes no sense since this is mostly used as statement to globally filter out ctnetlink events, but do not crash if it is used from concatenations. Add the missing slot in the datatype array so this does not crash. Fixes: 2595b9ad6840 ("ct: add conntrack event mask support") Reported-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* datatype: initialize TYPE_CT_LABEL slot in datatype arrayPablo Neira Ayuso2023-09-203-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | Otherwise, ct label with concatenations such as: table ip x { chain y { ct label . ct mark { 0x1 . 0x1 } } } crashes: ../include/datatype.h:196:11: runtime error: member access within null pointer of type 'const struct datatype' AddressSanitizer:DEADLYSIGNAL ================================================================= ==640948==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fc970d3199b bp 0x7fffd1f20560 sp 0x7fffd1f20540 T0) ==640948==The signal is caused by a READ memory access. ==640948==Hint: address points to the zero page. sudo #0 0x7fc970d3199b in datatype_equal ../include/datatype.h:196 Fixes: 2fcce8b0677b ("ct: connlabel matching support") Reported-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* limit: display default burst when listing rulesetPablo Neira Ayuso2023-09-208-21/+19
| | | | | | | | | | | | Default burst for limit is 5 for historical reasons but it is not displayed when listing the ruleset. Update listing to display the default burst to disambiguate. man nft(8) has been recently updated to document this, no action in this front is therefore required. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests/shell: run `nft --check` on persisted dump filesThomas Haller2023-09-192-1/+34
| | | | | | | | | | | | | | | | "nft --check" will trigger a rollback in kernel. The existing dump files might hit new code paths. Take the opportunity to call the command on the existing files. And alternative would be to write a separate tests, that iterates over all files. However, then we can only run all the commands sequentially (unless we do something smart). That might be slower than the opportunity to run the checks in parallel. More importantly, it would be nice if the check for the dump file is clearly tied to the file's test. So run it right after the test, from the test wrapper. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* libnftables: move init-once guard inside xt_init()Thomas Haller2023-09-192-7/+14
| | | | | | | | | | | | | | | | A library should not restrict being used by multiple threads or make assumptions about how it's being used. Hence a "init_once" pattern without no locking is racy, a code smell and should be avoided. Note that libxtables is full of global variables and when linking against it, libnftables cannot be used from multiple threads either. That is not easy to fix. Move the ugliness of "init_once" away from nft_ctx_new(), so that the problem is concentrated closer to libxtables. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* libnftables: drop gmp_init() and mp_set_memory_functions()Thomas Haller2023-09-193-12/+0
| | | | | | | | | | | | | | | | | | | | | | | Setting global handles for libgmp via mp_set_memory_functions() is very ugly. When we don't use mini-gmp, then potentially there are other users of the library in the same process, and every process fighting about the allocation functions is not gonna work. It also means, we must not reset the allocation functions after somebody already allocated GMP data with them. Which we cannot ensure, as we don't know what other parts of the process are doing. It's also unnecessary. The default allocation functions for gmp and mini-gmp already abort the process on allocation failure ([1], [2]), just like our xmalloc(). Just don't do this. [1] https://gmplib.org/repo/gmp/file/8225bdfc499f/memory.c#l37 [2] https://git.netfilter.org/nftables/tree/src/mini-gmp.c?id=6d19a902c1d77cb51b940b1ce65f31b1cad38b74#n286 Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: perform mark datatype compatibility check from mapsPablo Neira Ayuso2023-09-193-7/+74
| | | | | | | | | | | | | Wrap datatype compatibility check into a helper function and use it for map evaluation, otherwise the following bogus error message is displayed: Error: datatype mismatch, map expects packet mark, mapping expression has type integer Add unit tests to improve coverage for this usecase. Fixes: 5d8e33ddb112 ("evaluate: relax type-checking for integer arguments in mark statements") Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: expand sets and maps before evaluationPablo Neira Ayuso2023-09-198-40/+71
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 3975430b12d9 ("src: expand table command before evaluation") moved ruleset expansion before evaluation, except for sets and maps. For sets and maps there is still a post_expand() phase. This patch moves sets and map expansion to allocate an independent CMD_OBJ_SETELEMS command to add elements to named set and maps which is evaluated, this consolidates the ruleset expansion to happen always before the evaluation step for all objects, except for anonymous sets and maps. This approach avoids an interference with the set interval code which detects overlaps and merges of adjacents ranges. This set interval routine uses set->init to maintain a cache of existing elements. Then, the post_expand() phase incorrectly expands set->init cache and it triggers a bogus ENOENT errors due to incorrect bytecode (placing element addition before set creation) in combination with user declared sets using the flat syntax notation. Since the evaluation step (coming after the expansion) creates implicit/anonymous sets and maps, those are not expanded anymore. These anonymous sets still need to be evaluated from set_evaluate() path and the netlink bytecode generation path, ie. do_add_set(), needs to deal with anonymous sets. Note that, for named sets, do_add_set() does not use set->init. Such content is part of the existing cache, and the CMD_OBJ_SETELEMS command is responsible for adding elements to named sets. Fixes: 3975430b12d9 ("src: expand table command before evaluation") Reported-by: Jann Haber <jannh@selfnet.de> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* evaluate: fix memleak in prefix evaluation with wildcard interface namePablo Neira Ayuso2023-09-191-1/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The following ruleset: table ip x { chain y { meta iifname { abcde*, xyz } } } triggers the following memleak: ==6871== 16 bytes in 1 blocks are definitely lost in loss record 1 of 1 ==6871== at 0x483877F: malloc (vg_replace_malloc.c:307) ==6871== by 0x48AD898: xmalloc (utils.c:37) ==6871== by 0x4BC8B22: __gmpz_init2 (in /usr/lib/x86_64-linux-gnu/libgmp.so.10.4.1) ==6871== by 0x4887E67: constant_expr_alloc (expression.c:424) ==6871== by 0x488EF1F: expr_evaluate_prefix (evaluate.c:1138) ==6871== by 0x488EF1F: expr_evaluate (evaluate.c:2725) ==6871== by 0x488E76D: expr_evaluate_set_elem (evaluate.c:1662) ==6871== by 0x488E76D: expr_evaluate (evaluate.c:2739) ==6871== by 0x4891033: list_member_evaluate (evaluate.c:1454) ==6871== by 0x488E2B6: expr_evaluate_set (evaluate.c:1757) ==6871== by 0x488E2B6: expr_evaluate (evaluate.c:2737) ==6871== by 0x48910D0: elems_evaluate (evaluate.c:4605) ==6871== by 0x4891432: set_evaluate (evaluate.c:4711) ==6871== by 0x48915BC: implicit_set_declaration (evaluate.c:122) ==6871== by 0x488F18A: expr_evaluate_relational (evaluate.c:2503) ==6871== by 0x488F18A: expr_evaluate (evaluate.c:2745) expr_evaluate_prefix() calls constant_expr_alloc() which have already called mpz_init2(), the second call to mpz_init2() overlaps the existing mpz_t data memory area. Remove extra mpz_init2() call to fix this memleak. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* netlink: fix leaking typeof_expr_data/typeof_expr_key in ↵Thomas Haller2023-09-191-6/+6
| | | | | | | | | | | | netlink_delinearize_set() There are various code paths that return without freeing typeof_expr_data and typeof_expr_key. It's not at all obvious, that there isn't a leak that way. Quite possibly there is a leak. Fix it, or at least make the code more obviously correct. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
* tests/shell: simplify collecting error result in "test-wrapper.sh"Thomas Haller2023-09-181-8/+8
| | | | | | | | | | | | | | | | | | | The previous pattern was unnecessarily confusing. The "$rc_{dump,valgrind,tainted}" variable should only remember whether that particular check failed, not the overall exit code of the test wrapper. Otherwise, if you want to know in which case the wrapper exits with code 122, you have to oddly follow the rc_valgrind variable. This change will make more sense, when we add another such variable, but which will be assigned the non-zero value at multiple places. Assigning there the exit code of the wrapper, duplicates the places where the condition maps to the exit code. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: colorize NFT_TEST_HAS_SOCKET_LIMITSThomas Haller2023-09-181-11/+18
| | | | | | | | | | | | | | NFT_TEST_HAS_SOCKET_LIMITS= is similar to NFT_TEST_HAVE_* variables and indicates a feature (or lack thereof), except that it's inverted. Maybe this should be consolidated, however, NFT_TEST_HAS_SOCKET_LIMITS= is detected in the root namespace, unlike the shell scripts from features. So it's unclear how to consolidate them best. Anyway. Still highlight a lack of the capability, as it can cause tests to be skipped and we should see that easily. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: don't show the exit status for failed testsThomas Haller2023-09-181-6/+3
| | | | | | | | | | | | | | | | | | | | Previously, for failed tests we would print the exit code W: [FAILED] 2/2 tests/shell/testcases/listing/0013objects_0: got 1 This doesn't seem very useful. For one, we have special exit codes like 0 (OK), 77 (SKIPPED), 124 (DUMP FAIL), 123 (TAINTED), 122 (VALGRIND). Any other exit code is just an arbitrary failure. We don't define any special codes, and printing them is not useful. Note that further exit codes (118 - 121) are reserved, and could be special purposed, when there is a use. You can find the real exit code from the test in the result data in the "rc-failed" file. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: set C locale in "run-tests.sh"Thomas Haller2023-09-181-2/+6
| | | | | | | | | | The tests should run always the same, regardless of the user's language settings. Set LANG=C and LC_ALL=C and unset LANGUAGE. If some part wants to test a different language, it would set it explicitly. They anyway wouldn't want to depend on something from the user's environment. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: fix preserving ruleset diff after testThomas Haller2023-09-181-0/+1
| | | | | | | | | We want to delete the file in the case when there was no diff (and we expect the file to be empty). The condition was wrong. Fixes: 55fe071cd193 ('tests/shell: cleanup result handling in "test-wrapper.sh"') Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: check diff in "maps/typeof_maps_0" and "sets/typeof_sets_0" testThomas Haller2023-09-182-31/+177
| | | | | | | | | | | These tests run different variants based on NFT_TEST_HAVE_osf support. Consequently, we cannot check the pre-generated diff. Instead, construct what we expect dynamically in the script, and compare the ruleset against that. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: implement NFT_TEST_HAVE_json feature detection as scriptThomas Haller2023-09-182-14/+10
| | | | | | | | No more need to special case the "run a script" approach for detecting the json feature. Use the new mechanism instead. Signed-off-by: Thomas Haller <thaller@redhat.com> Signed-off-by: Florian Westphal <fw@strlen.de>
* tests/shell: skip reset tests if kernel lacks supportFlorian Westphal2023-09-185-4/+43
| | | | | | | | | | | | reset is implemented via flush + extra attribute, so older kernels perform a flush. This means .nft doesn't work, we need to check if the individual set contents/sets are still in place post-reset. Make this generic and permit use of feat.sh in addition to the simpler foo.nft feature files. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip test cases if ct expectation and/or timeout lacks supportFlorian Westphal2023-09-185-39/+33
| | | | | Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip test cases involving osf match if kernel lacks supportFlorian Westphal2023-09-183-10/+39
| | | | | Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip catchall tests if kernel lacks supportFlorian Westphal2023-09-186-2/+35
| | | | | Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip destroy tests if kernel lacks supportFlorian Westphal2023-09-186-0/+13
| | | | | | | | Destroy support was added for table/flowtable/chain etc. in a single commit, so no need to add capability tests for each destroy subtype. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip inet ingress tests if kernel lacks supportFlorian Westphal2023-09-184-7/+18
| | | | | | | Split the bridge autoremove test to a new file. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip some tests if kernel lacks netdev egress supportFlorian Westphal2023-09-183-1/+18
| | | | | Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip bitshift tests if kernel lacks supportFlorian Westphal2023-09-1811-0/+27
| | | | | Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip inner matching tests if unsupportedFlorian Westphal2023-09-182-0/+9
| | | | | Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip map query if kernel lacks supportFlorian Westphal2023-09-182-8/+38
| | | | | | | | | | | | | | On recent kernels one can perform a lookup in a map without a destination register (i.e., treat the map like a set -- pure existence check). Add a feature probe and work around the missing feature in typeof_maps_add_delete: do the test with a simplified ruleset, Indicate skipped even though a reduced test was run (earlier errors cause a failure) to not trigger dump validation error. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: skip netdev_chain_0 if kernel requires netdev deviceFlorian Westphal2023-09-182-0/+9
| | | | | | | | | | | | | | This test case only works on kernel 6.4+. Add feature probe for this and tag the test accordingly using the scheme added by Thomas Haller in "tests/shell: skip tests if nft does not support JSON mode" so that run-test.sh skips it if kernel requires a device. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>
* tests/shell: add and use chain binding feature probeFlorian Westphal2023-09-184-7/+62
| | | | | | | | | | Alter 30s-stress to suppress anon chains when its unuspported. Note that 30s-stress is optionally be run standalone, so also update the test script. Signed-off-by: Florian Westphal <fw@strlen.de> Signed-off-by: Thomas Haller <thaller@redhat.com>