diff --git a/Rakefile b/Rakefile index 7872aad..590db27 100644 --- a/Rakefile +++ b/Rakefile @@ -82,13 +82,22 @@ file check("client") => build/parse.o build/client.o build/serve.o + build/acl.o build/ioutil.o build/util.o} do |t| gcc_link t.name, t.prerequisites + [LIBCHECK] end +file check("acl") => +%w{tests/check_acl.c + build/parse.o + build/acl.o + build/util.o} do |t| + gcc_link t.name, t.prerequisites + [LIBCHECK] +end -(TEST_MODULES-["client"]).each do |m| + +(TEST_MODULES- %w{acl client}).each do |m| deps = ["tests/check_#{m}.c", "build/ioutil.o", "build/util.o"] maybe_obj_name = "build/#{m}.o" diff --git a/src/acl.c b/src/acl.c new file mode 100644 index 0000000..054ed0c --- /dev/null +++ b/src/acl.c @@ -0,0 +1,99 @@ +#include + +#include "util.h" +#include "parse.h" + +#include "acl.h" + + +struct acl * acl_create( int len, char ** lines, int default_deny ) +{ + struct acl * acl; + + acl = (struct acl *)xmalloc( sizeof( struct acl ) ); + acl->len = parse_acl( &acl->entries, len, lines ); + acl->default_deny = default_deny; + return acl; +} + + +static int testmasks[9] = { 0,128,192,224,240,248,252,254,255 }; + +/** Test whether AF_INET or AF_INET6 sockaddr is included in the given access + * control list, returning 1 if it is, and 0 if not. + */ +static int is_included_in_acl(int list_length, struct ip_and_mask (*list)[], union mysockaddr* test) +{ + NULLCHECK( test ); + + int i; + + for (i=0; i < list_length; i++) { + struct ip_and_mask *entry = &(*list)[i]; + int testbits; + unsigned char *raw_address1, *raw_address2; + + debug("checking acl entry %d (%d/%d)", i, test->generic.sa_family, entry->ip.family); + + if (test->generic.sa_family != entry->ip.family) { + continue; + } + + if (test->generic.sa_family == AF_INET) { + debug("it's an AF_INET"); + raw_address1 = (unsigned char*) &test->v4.sin_addr; + raw_address2 = (unsigned char*) &entry->ip.v4.sin_addr; + } + else if (test->generic.sa_family == AF_INET6) { + debug("it's an AF_INET6"); + raw_address1 = (unsigned char*) &test->v6.sin6_addr; + raw_address2 = (unsigned char*) &entry->ip.v6.sin6_addr; + } + + debug("testbits=%d", entry->mask); + + for (testbits = entry->mask; testbits > 0; testbits -= 8) { + debug("testbits=%d, c1=%02x, c2=%02x", testbits, raw_address1[0], raw_address2[0]); + if (testbits >= 8) { + if (raw_address1[0] != raw_address2[0]) + goto no_match; + } + else { + if ((raw_address1[0] & testmasks[testbits%8]) != + (raw_address2[0] & testmasks[testbits%8]) ) + goto no_match; + } + + raw_address1++; + raw_address2++; + } + + return 1; + + no_match: ; + debug("no match"); + } + + return 0; +} + +int acl_includes( struct acl * acl, union mysockaddr * addr ) +{ + NULLCHECK( acl ); + + if ( 0 == acl->len ) { + return !( acl->default_deny ); + } + else { + return is_included_in_acl( acl->len, acl->entries, addr ); + } +} + + +void acl_destroy( struct acl * acl ) +{ + free( acl->entries ); + acl->len = 0; + acl->entries = NULL; + free( acl ); +} diff --git a/src/acl.h b/src/acl.h new file mode 100644 index 0000000..1fd2b77 --- /dev/null +++ b/src/acl.h @@ -0,0 +1,34 @@ +#ifndef ACL_H +#define ACL_H + +#include "parse.h" + +struct acl { + int len; + int default_deny; + struct ip_and_mask (*entries)[]; +}; + +/** Allocate a new acl structure, parsing the given lines to sockaddr + * structures in the process. After allocation, acl->len might not + * equal len. In that case, there was an error in parsing and acl->len + * will be the index of the failed entry in lines. + * + * default_deny controls the behaviour of an empty list: if true, all + * requests will be denied. If true, all requests will be accepted. + */ +struct acl * acl_create( int len, char **lines, int default_deny ); + + +/** Check to see whether an address is allowed by an acl. + * See acl_create for how the default_deny setting affects this. + */ +int acl_includes( struct acl *, union mysockaddr *); + + +/** Free the acl structure and the internal acl entries table. + */ +void acl_destroy( struct acl * ); + + +#endif diff --git a/src/control.c b/src/control.c index 00fed45..67e7c30 100644 --- a/src/control.c +++ b/src/control.c @@ -32,6 +32,7 @@ #include "readwrite.h" #include "bitset.h" #include "self_pipe.h" +#include "acl.h" #include #include @@ -259,22 +260,21 @@ int control_mirror(struct control_params* client, int linesc, char** lines) /** Command parser to alter access control list from socket input */ int control_acl(struct control_params* client, int linesc, char** lines) { - int parsedc; - struct ip_and_mask (*acl)[], (*old_acl)[]; + NULLCHECK( client ); + + struct acl * old_acl = client->serve->acl; + struct acl * new_acl = acl_create( linesc, lines, old_acl ? old_acl->default_deny : 0 ); - parsedc = parse_acl(&acl, linesc, lines); - if (parsedc != linesc) { + if (new_acl->len != linesc) { write(client->socket, "1: bad spec: ", 13); - write(client->socket, lines[parsedc], - strlen(lines[parsedc])); + write(client->socket, lines[new_acl->len], + strlen(lines[new_acl->len])); write(client->socket, "\n", 1); - free(acl); + acl_destroy( new_acl ); } else { - old_acl = client->serve->acl; - client->serve->acl = acl; - client->serve->acl_entries = linesc; - free(old_acl); + client->serve->acl = new_acl; + acl_destroy( old_acl ); write_socket("0: updated"); } diff --git a/src/flexnbd.c b/src/flexnbd.c index e616438..b334594 100644 --- a/src/flexnbd.c +++ b/src/flexnbd.c @@ -19,6 +19,7 @@ * elsewhere in the program. */ + #include "serve.h" #include "util.h" @@ -33,7 +34,9 @@ #include #include + #include "options.h" +#include "acl.h" void exit_err( char *msg ) @@ -50,11 +53,8 @@ void params_serve( char *s_ctrl_sock, int default_deny, int acl_entries, - char** s_acl_entries /* first may actually be path to control socket */ -) + char** s_acl_entries ) { - int parsed; - out->tcp_backlog = 10; /* does this need to be settable? */ if (s_ip_address == NULL) @@ -72,14 +72,9 @@ void params_serve( * we pass NULL. */ out->control_socket_name = s_ctrl_sock; - /* If this is true then an empty ACL means "nobody is allowed to connect", - * rather than "anybody is allowed to connect" */ - out->default_deny = default_deny; - - out->acl_entries = acl_entries; - parsed = parse_acl(&out->acl, acl_entries, s_acl_entries); - if (parsed != acl_entries) - SERVER_ERROR("Bad ACL entry '%s'", s_acl_entries[parsed]); + out->acl = acl_create( acl_entries, s_acl_entries, default_deny ); + if (out->acl && out->acl->len != acl_entries) + SERVER_ERROR("Bad ACL entry '%s'", s_acl_entries[out->acl->len]); out->bind_to.v4.sin_port = atoi(s_port); if (out->bind_to.v4.sin_port < 0 || out->bind_to.v4.sin_port > 65535) diff --git a/src/serve.c b/src/serve.c index 7979c4d..b6e5000 100644 --- a/src/serve.c +++ b/src/serve.c @@ -65,65 +65,6 @@ void server_unlock_io( struct server* serve ) ); } -static int testmasks[9] = { 0,128,192,224,240,248,252,254,255 }; - -/** Test whether AF_INET or AF_INET6 sockaddr is included in the given access - * control list, returning 1 if it is, and 0 if not. - */ -int is_included_in_acl(int list_length, struct ip_and_mask (*list)[], union mysockaddr* test) -{ - NULLCHECK( test ); - - int i; - - for (i=0; i < list_length; i++) { - struct ip_and_mask *entry = &(*list)[i]; - int testbits; - unsigned char *raw_address1, *raw_address2; - - debug("checking acl entry %d (%d/%d)", i, test->generic.sa_family, entry->ip.family); - - if (test->generic.sa_family != entry->ip.family) - continue; - - if (test->generic.sa_family == AF_INET) { - debug("it's an AF_INET"); - raw_address1 = (unsigned char*) &test->v4.sin_addr; - raw_address2 = (unsigned char*) &entry->ip.v4.sin_addr; - } - else if (test->generic.sa_family == AF_INET6) { - debug("it's an AF_INET6"); - raw_address1 = (unsigned char*) &test->v6.sin6_addr; - raw_address2 = (unsigned char*) &entry->ip.v6.sin6_addr; - } - - debug("testbits=%d", entry->mask); - - for (testbits = entry->mask; testbits > 0; testbits -= 8) { - debug("testbits=%d, c1=%02x, c2=%02x", testbits, raw_address1[0], raw_address2[0]); - if (testbits >= 8) { - if (raw_address1[0] != raw_address2[0]) - goto no_match; - } - else { - if ((raw_address1[0] & testmasks[testbits%8]) != - (raw_address2[0] & testmasks[testbits%8]) ) - goto no_match; - } - - raw_address1++; - raw_address2++; - } - - return 1; - - no_match: ; - debug("no match"); - } - - return 0; -} - /** Prepares a listening socket for the NBD server, binding etc. */ void serve_open_server_socket(struct server* params) { @@ -259,13 +200,10 @@ int server_acl_accepts( struct server *params, union mysockaddr * client_address NULLCHECK( client_address ); if (params->acl) { - if (is_included_in_acl(params->acl_entries, params->acl, client_address)) - return 1; - } else { - if (!params->default_deny) - return 1; + return acl_includes( params->acl, client_address ); } - return 0; + + return 1; } @@ -405,11 +343,11 @@ void serve_accept_loop(struct server* params) client_fd = accept(activity_fd, &client_address.generic, &socklen); if (activity_fd == params->server_fd) { - debug("Accepted nbd client"); + debug("Accepted nbd client socket"); accept_nbd_client(params, client_fd, &client_address); } if (activity_fd == params->control_fd) { - debug("Accepted control client"); + debug("Accepted control client socket"); accept_control_connection(params, client_fd, &client_address); } diff --git a/src/serve.h b/src/serve.h index 2f78632..f96222b 100644 --- a/src/serve.h +++ b/src/serve.h @@ -8,6 +8,7 @@ #endif #include "parse.h" +#include "acl.h" #include @@ -46,12 +47,8 @@ struct client_tbl_entry { struct server { /** address/port to bind to */ union mysockaddr bind_to; - /** does an empty ACL mean "deny all"? */ - int default_deny; - /** number of entries in current access control list*/ - int acl_entries; - /** pointer to access control list entries*/ - struct ip_and_mask (*acl)[0]; + /** access control list */ + struct acl * acl; /** (static) file name to serve */ char* filename; /** file name of INCOMPLETE flag */ diff --git a/tests/check_acl.c b/tests/check_acl.c new file mode 100644 index 0000000..06e4983 --- /dev/null +++ b/tests/check_acl.c @@ -0,0 +1,122 @@ +#include +#include + +#include "acl.h" + +START_TEST( test_null_acl ) +{ + struct acl *acl = acl_create( 0,NULL, 0 ); + + fail_if( NULL == acl, "No acl alloced." ); + fail_unless( 0 == acl->len, "Incorrect length" ); +} +END_TEST + + +START_TEST( test_parses_single_line ) +{ + char *lines[] = {"127.0.0.1"}; + struct acl * acl = acl_create( 1, lines, 0 ); + + fail_unless( 1 == acl->len, "Incorrect length." ); + fail_if( NULL == acl->entries, "No entries present." ); +} +END_TEST + + +START_TEST( test_destroy_doesnt_crash ) +{ + char *lines[] = {"127.0.0.1"}; + struct acl * acl = acl_create( 1, lines, 0 ); + + acl_destroy( acl ); +} +END_TEST + + +START_TEST( test_includes_single_address ) +{ + char *lines[] = {"127.0.0.1"}; + struct acl * acl = acl_create( 1, lines, 0 ); + union mysockaddr x; + + parse_ip_to_sockaddr( &x.generic, "127.0.0.1" ); + + fail_unless( acl_includes( acl, &x ), "Included address wasn't covered" ); +} +END_TEST + + +START_TEST( test_doesnt_include_other_address ) +{ + char *lines[] = {"127.0.0.1"}; + 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_default_deny_rejects ) +{ + struct acl * acl = acl_create( 0, NULL, 1 ); + union mysockaddr x; + + parse_ip_to_sockaddr( &x.generic, "127.0.0.1" ); + + fail_if( acl_includes( acl, &x ), "Default deny accepted." ); +} +END_TEST + + +START_TEST( test_default_accept_rejects ) +{ + struct acl * acl = acl_create( 0, NULL, 0 ); + union mysockaddr x; + + parse_ip_to_sockaddr( &x.generic, "127.0.0.1" ); + + fail_unless( acl_includes( acl, &x ), "Default accept rejected." ); +} +END_TEST + + +Suite* acl_suite() +{ + Suite *s = suite_create("acl"); + TCase *tc_create = tcase_create("create"); + TCase *tc_includes = tcase_create("includes"); + TCase *tc_destroy = tcase_create("destroy"); + + tcase_add_test(tc_create, test_null_acl); + tcase_add_test(tc_create, test_parses_single_line); + + tcase_add_test(tc_includes, test_includes_single_address); + tcase_add_test(tc_includes, test_doesnt_include_other_address); + tcase_add_test(tc_includes, test_default_deny_rejects); + tcase_add_test(tc_includes, test_default_accept_rejects); + + tcase_add_test(tc_destroy, test_destroy_doesnt_crash); + + suite_add_tcase(s, tc_create); + suite_add_tcase(s, tc_includes); + suite_add_tcase(s, tc_destroy); + + + return s; +} + +int main(void) +{ + set_debug(1); + int number_failed; + Suite *s = acl_suite(); + SRunner *sr = srunner_create(s); + srunner_run_all(sr, CK_NORMAL); + number_failed = srunner_ntests_failed(sr); + srunner_free(sr); + return (number_failed == 0) ? 0 : 1; +} +