Add tests that check if getsockopt(TCP_AO_GET_KEYS) returns the right keys when using different filters.
Sample output:
# ok 114 filter keys: by sndid, rcvid, address # ok 115 filter keys: by sndid, rcvid # ok 116 filter keys: by is_current # ok 117 filter keys: by is_rnext
Signed-off-by: Leo Stone leocstone@gmail.com --- This patch is meant to address the TODO in setsockopt-closed.c:
/*
- TODO: check getsockopt(TCP_AO_GET_KEYS) with different filters
- returning proper nr & keys;
*/
Is this a reasonable way to do these tests? If so, what cases should I add? --- .../selftests/net/tcp_ao/setsockopt-closed.c | 158 +++++++++++++++++- 1 file changed, 157 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c index 084db4ecdff6..4c8aa06eef5a 100644 --- a/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c +++ b/tools/testing/selftests/net/tcp_ao/setsockopt-closed.c @@ -6,6 +6,8 @@
static union tcp_addr tcp_md5_client;
+#define FILTER_TEST_NKEYS 16 + static int test_port = 7788; static void make_listen(int sk) { @@ -813,12 +815,166 @@ static void duplicate_tests(void) setsockopt_checked(sk, TCP_AO_ADD_KEY, &ao, EEXIST, "duplicate: SendID differs"); }
+ +static void fetch_all_keys(int sk, struct tcp_ao_getsockopt *keys) +{ + socklen_t optlen = sizeof(struct tcp_ao_getsockopt); + + memset(keys, 0, sizeof(struct tcp_ao_getsockopt) * FILTER_TEST_NKEYS); + keys[0].get_all = 1; + keys[0].nkeys = FILTER_TEST_NKEYS; + if (getsockopt(sk, IPPROTO_TCP, TCP_AO_GET_KEYS, &keys[0], &optlen)) + test_error("getsockopt"); +} + +static int prepare_test_keys(struct tcp_ao_getsockopt *keys) +{ + struct tcp_ao_add test_ao[FILTER_TEST_NKEYS]; + u8 rcvid = 100, sndid = 100; + const char *test_password = "Test password number "; + char test_password_scratch[64] = {}; + int sk = socket(test_family, SOCK_STREAM, IPPROTO_TCP); + + if (sk < 0) + test_error("socket()"); + + for (int i = 0; i < FILTER_TEST_NKEYS; i++) { + snprintf(test_password_scratch, 64, "%s %d", test_password, i); + test_prepare_key(&test_ao[i], DEFAULT_TEST_ALGO, this_ip_dest, false, false, + DEFAULT_TEST_PREFIX, 0, sndid++, rcvid++, 0, 0, + strlen(test_password_scratch), test_password_scratch); + } + test_ao[0].set_current = 1; + test_ao[1].set_rnext = 1; + /* One key with a different addr and overlapping sndid, rcvid */ + tcp_addr_to_sockaddr_in(&test_ao[2].addr, &this_ip_addr, 0); + test_ao[2].sndid = 100; + test_ao[2].rcvid = 100; + + /* Add keys in a random order */ + for (int i = 0; i < FILTER_TEST_NKEYS; i++) { + int randidx = rand() % (FILTER_TEST_NKEYS - i); + + if (setsockopt(sk, IPPROTO_TCP, TCP_AO_ADD_KEY, &test_ao[randidx], + sizeof(struct tcp_ao_add))) + test_error("setsockopt()"); + memcpy(&test_ao[randidx], &test_ao[FILTER_TEST_NKEYS - 1 - i], + sizeof(struct tcp_ao_add)); + } + + fetch_all_keys(sk, keys); + + return sk; +} + +/* Assumes passwords are unique */ +static int compare_mkts(struct tcp_ao_getsockopt *expected, int nexpected, + struct tcp_ao_getsockopt *actual, int nactual) +{ + int matches = 0; + + for (int i = 0; i < nexpected; i++) { + for (int j = 0; j < nactual; j++) { + if (memcmp(expected[i].key, actual[j].key, TCP_AO_MAXKEYLEN) == 0) + matches++; + } + } + return nexpected - matches; +} + +static void filter_keys_checked(int sk, struct tcp_ao_getsockopt *filter, + struct tcp_ao_getsockopt *expected, + unsigned int nexpected, const char *tst) +{ + struct tcp_ao_getsockopt all_keys[FILTER_TEST_NKEYS] = {}; + struct tcp_ao_getsockopt filtered_keys[FILTER_TEST_NKEYS] = {}; + socklen_t len = sizeof(struct tcp_ao_getsockopt); + + fetch_all_keys(sk, all_keys); + memcpy(&filtered_keys[0], filter, sizeof(struct tcp_ao_getsockopt)); + filtered_keys[0].nkeys = FILTER_TEST_NKEYS; + if (getsockopt(sk, IPPROTO_TCP, TCP_AO_GET_KEYS, filtered_keys, &len)) + test_error("getsockopt"); + if (filtered_keys[0].nkeys != nexpected) + test_error("wrong nr of keys, expected %u got %u", nexpected, + filtered_keys[0].nkeys); + if (compare_mkts(expected, nexpected, filtered_keys, filtered_keys[0].nkeys)) + test_error("got wrong keys back"); + test_ok("filter keys: %s", tst); + + close(sk); + memset(filter, 0, sizeof(struct tcp_ao_getsockopt)); +} + +static void filter_tests(void) +{ + struct tcp_ao_getsockopt original_keys[FILTER_TEST_NKEYS]; + struct tcp_ao_getsockopt expected_keys[FILTER_TEST_NKEYS]; + struct tcp_ao_getsockopt filter = {}; + int sk, f, nmatches; + + f = 2; + sk = prepare_test_keys(original_keys); + filter.rcvid = original_keys[f].rcvid; + filter.sndid = original_keys[f].sndid; + memcpy(&filter.addr, &original_keys[f].addr, sizeof(original_keys[f].addr)); + filter.prefix = original_keys[f].prefix; + filter_keys_checked(sk, &filter, &original_keys[f], 1, "by sndid, rcvid, address"); + + f = -1; + sk = prepare_test_keys(original_keys); + for (int i = 0; i < original_keys[0].nkeys; i++) { + if (original_keys[i].is_current) { + f = i; + break; + } + } + if (f < 0) + test_error("No current key after adding one"); + filter.is_current = 1; + filter_keys_checked(sk, &filter, &original_keys[f], 1, "by is_current"); + + f = -1; + sk = prepare_test_keys(original_keys); + for (int i = 0; i < original_keys[0].nkeys; i++) { + if (original_keys[i].is_rnext) { + f = i; + break; + } + } + if (f < 0) + test_error("No rnext key after adding one"); + filter.is_rnext = 1; + filter_keys_checked(sk, &filter, &original_keys[f], 1, "by is_rnext"); + + f = -1; + nmatches = 0; + sk = prepare_test_keys(original_keys); + for (int i = 0; i < original_keys[0].nkeys; i++) { + if (original_keys[i].sndid == 100) { + f = i; + memcpy(&expected_keys[nmatches], &original_keys[i], + sizeof(struct tcp_ao_getsockopt)); + nmatches++; + } + } + if (f < 0) + test_error("No key for sndid 100"); + if (nmatches != 2) + test_error("Should have 2 keys with sndid 100"); + filter.rcvid = original_keys[f].rcvid; + filter.sndid = original_keys[f].sndid; + filter.addr.ss_family = test_family; + filter_keys_checked(sk, &filter, expected_keys, nmatches, "by sndid, rcvid"); +} + static void *client_fn(void *arg) { if (inet_pton(TEST_FAMILY, __TEST_CLIENT_IP(2), &tcp_md5_client) != 1) test_error("Can't convert ip address"); extend_tests(); einval_tests(); + filter_tests(); duplicate_tests(); /* * TODO: check getsockopt(TCP_AO_GET_KEYS) with different filters @@ -830,6 +986,6 @@ static void *client_fn(void *arg)
int main(int argc, char *argv[]) { - test_init(121, client_fn, NULL); + test_init(125, client_fn, NULL); return 0; }
Hi Leo,
On Mon, 14 Oct 2024 at 22:37, Leo Stone leocstone@gmail.com wrote:
Add tests that check if getsockopt(TCP_AO_GET_KEYS) returns the right keys when using different filters.
Sample output:
# ok 114 filter keys: by sndid, rcvid, address # ok 115 filter keys: by sndid, rcvid # ok 116 filter keys: by is_current # ok 117 filter keys: by is_rnext
Signed-off-by: Leo Stone leocstone@gmail.com
This patch is meant to address the TODO in setsockopt-closed.c:
/*
- TODO: check getsockopt(TCP_AO_GET_KEYS) with different filters
- returning proper nr & keys;
*/
Is this a reasonable way to do these tests? If so, what cases should I add?
Your change does look reasonable to me. I think you could add one more test here for passing (FILTER_TEST_NKEYS/2) to getsockopt() as tcp_ao_getsockopt::nkeys with get_all = 1, and check that the value in tcp_ao_getsockopt::nkeys after getsockopt() reflects the number of matched keys (FILTER_TEST_NKEYS).
See also minor nits inline.
[..]
+static void filter_keys_checked(int sk, struct tcp_ao_getsockopt *filter,
struct tcp_ao_getsockopt *expected,
unsigned int nexpected, const char *tst)
+{
struct tcp_ao_getsockopt all_keys[FILTER_TEST_NKEYS] = {};
struct tcp_ao_getsockopt filtered_keys[FILTER_TEST_NKEYS] = {};
socklen_t len = sizeof(struct tcp_ao_getsockopt);
fetch_all_keys(sk, all_keys);
memcpy(&filtered_keys[0], filter, sizeof(struct tcp_ao_getsockopt));
filtered_keys[0].nkeys = FILTER_TEST_NKEYS;
if (getsockopt(sk, IPPROTO_TCP, TCP_AO_GET_KEYS, filtered_keys, &len))
test_error("getsockopt");
I think the following two checks would be better s/test_error/test_fail/. The difference between _error() and _fail() is that for the later exit() is not called, which allows the person running the test to gather all "not okay" cases. So, in tcp_ao selftests I used _error() only for failures where nothing meaningful could be done afterwards, i.e. memory allocation or socket() creation.
if (filtered_keys[0].nkeys != nexpected)
test_error("wrong nr of keys, expected %u got %u", nexpected,
filtered_keys[0].nkeys);
if (compare_mkts(expected, nexpected, filtered_keys, filtered_keys[0].nkeys))
test_error("got wrong keys back");
^ in those two it seems to be better to do : test_fail("...") : goto out_close;
which would allow to go through other "filter" and "duplicate" selftests even if one of the "filter" tests has failed.
[..]
static void *client_fn(void *arg) { if (inet_pton(TEST_FAMILY, __TEST_CLIENT_IP(2), &tcp_md5_client) != 1) test_error("Can't convert ip address"); extend_tests(); einval_tests();
filter_tests(); duplicate_tests(); /* * TODO: check getsockopt(TCP_AO_GET_KEYS) with different filters
^ please, remove the related TODO comment, I think you just fixed it :-)
Thank you for the patch, Dmitry
linux-kselftest-mirror@lists.linaro.org