From 224bdcbf8785d2f53d350fb34f9ec3c32b9aa9af Mon Sep 17 00:00:00 2001 From: nick Date: Mon, 11 Jun 2012 12:56:45 +0100 Subject: [PATCH] Fix handling ACLs where > 1 entry exists --- src/parse.c | 28 ++++++------ tests/check_acl.c | 102 ++++++++++++++++++++++++++++++++++++++++++++ tests/check_serve.c | 11 +++-- 3 files changed, 122 insertions(+), 19 deletions(-) diff --git a/src/parse.c b/src/parse.c index f67bf46..f856cf4 100644 --- a/src/parse.c +++ b/src/parse.c @@ -46,7 +46,7 @@ int parse_ip_to_sockaddr(struct sockaddr* out, char* src) int parse_acl(struct ip_and_mask (**out)[], int max, char **entries) { - struct ip_and_mask (*list)[0]; + struct ip_and_mask* list; int i; if (max == 0) { @@ -54,35 +54,37 @@ int parse_acl(struct ip_and_mask (**out)[], int max, char **entries) return 0; } else { - *out = xmalloc(max * sizeof(struct ip_and_mask)); + list = xmalloc(max * sizeof(struct ip_and_mask)); + *out = (struct ip_and_mask (*)[])list; debug("acl alloc: %p", *out); } - - list = *out; - + + for (i = 0; i < max; i++) { -# define MAX_MASK_BITS (outentry->ip.family == AF_INET ? 32 : 128) int j; - struct ip_and_mask* outentry = list[i]; - - if (parse_ip_to_sockaddr(&outentry->ip.generic, entries[i]) == 0) + struct ip_and_mask* outentry = &list[i]; +# define MAX_MASK_BITS (outentry->ip.family == AF_INET ? 32 : 128) + if (parse_ip_to_sockaddr(&outentry->ip.generic, entries[i]) == 0) { return i; - + } + for (j=0; entries[i][j] && entries[i][j] != '/'; j++) - ; + ; // increment j! + if (entries[i][j] == '/') { outentry->mask = atoi(entries[i]+j+1); if (outentry->mask < 1 || outentry->mask > MAX_MASK_BITS) return i; } - else + else { outentry->mask = MAX_MASK_BITS; + } # undef MAX_MASK_BITS debug("acl ptr[%d]: %p %d",i, outentry, outentry->mask); } for (i=0; i < max; i++) { - debug("acl entry %d @ %p has mask %d", i, list[i], list[i]->mask); + debug("acl entry %d @ %p has mask %d", i, list[i], list[i].mask); } return max; diff --git a/tests/check_acl.c b/tests/check_acl.c index 34263ef..996934a 100644 --- a/tests/check_acl.c +++ b/tests/check_acl.c @@ -24,6 +24,24 @@ START_TEST( test_parses_single_line ) } END_TEST +START_TEST( test_parses_multiple_lines ) +{ + char *lines[] = {"127.0.0.1", "::1"}; + struct acl * acl = acl_create( 2, lines, 0 ); + union mysockaddr e0, e1; + + parse_ip_to_sockaddr( &e0.generic, lines[0] ); + parse_ip_to_sockaddr( &e1.generic, lines[1] ); + + fail_unless( acl->len == 2, "Multiple lines not parsed" ); + + struct ip_and_mask *entry; + entry = &(*acl->entries)[0]; + fail_unless(entry->ip.family == e0.family, "entry 0 has wrong family!"); + entry = &(*acl->entries)[1]; + fail_unless(entry->ip.family == e1.family, "entry 1 has wrong family!"); +} +END_TEST START_TEST( test_destroy_doesnt_crash ) { @@ -47,6 +65,55 @@ START_TEST( test_includes_single_address ) } END_TEST +START_TEST( test_includes_single_address_when_netmask_specified_ipv4 ) +{ + char *lines[] = {"127.0.0.1/24"}; + struct acl * acl = acl_create( 1, lines, 0 ); + union mysockaddr x; + + parse_ip_to_sockaddr( &x.generic, "127.0.0.0" ); + fail_unless( acl_includes( acl, &x ), "Included address wasn't covered" ); + + parse_ip_to_sockaddr( &x.generic, "127.0.0.1" ); + fail_unless( acl_includes( acl, &x ), "Included address wasn't covered" ); + + parse_ip_to_sockaddr( &x.generic, "127.0.0.255" ); + fail_unless( acl_includes( acl, &x ), "Included address wasn't covered" ); +} +END_TEST + +START_TEST( test_includes_single_address_when_netmask_specified_ipv6 ) +{ + char *lines[] = {"fe80::/10"}; + struct acl * acl = acl_create( 1, lines, 0 ); + union mysockaddr x; + + parse_ip_to_sockaddr( &x.generic, "fe80::1" ); + fail_unless( acl_includes( acl, &x ), "Included address wasn't covered" ); + + parse_ip_to_sockaddr( &x.generic, "fe80::2" ); + fail_unless( acl_includes( acl, &x ), "Included address wasn't covered" ); + + parse_ip_to_sockaddr( &x.generic, "fe80:ffff:ffff::ffff" ); + fail_unless( acl_includes( acl, &x ), "Included address wasn't covered" ); +} +END_TEST + +START_TEST( test_includes_single_address_when_multiple_entries_exist ) +{ + char *lines[] = {"127.0.0.1", "::1"}; + struct acl * acl = acl_create( 2, lines, 0 ); + union mysockaddr e0; + union mysockaddr e1; + + parse_ip_to_sockaddr( &e0.generic, "127.0.0.1" ); + parse_ip_to_sockaddr( &e1.generic, "::1" ); + + fail_unless( acl_includes( acl, &e0 ), "Included address 0 wasn't covered" ); + fail_unless( acl_includes( acl, &e1 ), "Included address 1 wasn't covered" ); +} +END_TEST + START_TEST( test_doesnt_include_other_address ) { @@ -59,6 +126,31 @@ START_TEST( test_doesnt_include_other_address ) } END_TEST +START_TEST( test_doesnt_include_other_address_when_netmask_specified ) +{ + char *lines[] = {"127.0.0.1/32"}; + struct acl * acl = acl_create( 1, lines, 0 ); + union mysockaddr x; + + parse_ip_to_sockaddr( &x.generic, "127.0.0.2" ); + fail_if( acl_includes( acl, &x ), "Excluded address was covered." ); +} +END_TEST + +START_TEST( test_doesnt_include_other_address_when_multiple_entries_exist ) +{ + char *lines[] = {"127.0.0.1", "::1"}; + struct acl * acl = acl_create( 2, lines, 0 ); + union mysockaddr e0; + union mysockaddr e1; + + parse_ip_to_sockaddr( &e0.generic, "127.0.0.2" ); + parse_ip_to_sockaddr( &e1.generic, "::2" ); + + fail_if( acl_includes( acl, &e0 ), "Excluded address 0 was covered." ); + fail_if( acl_includes( acl, &e1 ), "Excluded address 1 was covered." ); +} +END_TEST START_TEST( test_default_deny_rejects ) { @@ -93,9 +185,18 @@ Suite* acl_suite() tcase_add_test(tc_create, test_null_acl); tcase_add_test(tc_create, test_parses_single_line); + tcase_add_test(tc_includes, test_parses_multiple_lines); tcase_add_test(tc_includes, test_includes_single_address); + tcase_add_test(tc_includes, test_includes_single_address_when_netmask_specified_ipv4); + tcase_add_test(tc_includes, test_includes_single_address_when_netmask_specified_ipv6); + + tcase_add_test(tc_includes, test_includes_single_address_when_multiple_entries_exist); + tcase_add_test(tc_includes, test_doesnt_include_other_address); + tcase_add_test(tc_includes, test_doesnt_include_other_address_when_netmask_specified); + tcase_add_test(tc_includes, test_doesnt_include_other_address_when_multiple_entries_exist); + tcase_add_test(tc_includes, test_default_deny_rejects); tcase_add_test(tc_includes, test_default_accept_rejects); @@ -111,6 +212,7 @@ Suite* acl_suite() int main(void) { + log_level = 0; int number_failed; Suite *s = acl_suite(); SRunner *sr = srunner_create(s); diff --git a/tests/check_serve.c b/tests/check_serve.c index 5aa196c..2a4fbf2 100644 --- a/tests/check_serve.c +++ b/tests/check_serve.c @@ -156,12 +156,10 @@ END_TEST START_TEST( test_acl_update_leaves_good_client ) { struct server * s = server_create( "127.0.0.7", "0", dummy_file, NULL, 0, 0, NULL ); - /* There's an assumption here that the localhost *is* 127.0.0.1. - * If it's not, this test will fail and we'll have to explicitly - * pick a source address. - */ - char *lines[] = {"127.0.0.1"}; - struct acl * new_acl = acl_create( 1, lines, 0); + + // Client source address may be IPv4 or IPv6 localhost. Should be explicit + char *lines[] = {"127.0.0.1", "::1"}; + struct acl * new_acl = acl_create( 2, lines, 1 ); struct client * c; struct client_tbl_entry * entry; @@ -169,6 +167,7 @@ START_TEST( test_acl_update_leaves_good_client ) int client_fd; int server_fd; + myfail_if(new_acl->len != 2, "sanity: new_acl length is not 2"); serve_open_server_socket( s ); actual_port = server_port( s );