diff --git a/server/fw_util_firewalld.c b/server/fw_util_firewalld.c index 59e55e59..bbb7da76 100644 --- a/server/fw_util_firewalld.c +++ b/server/fw_util_firewalld.c @@ -73,7 +73,7 @@ rule_exists_no_chk_support(const fko_srv_options_t * const opts, const char * const srcip, const char * const dstip, const unsigned int port, const unsigned int exp_ts) { - int rule_exists=0, rule_num=0, rtmp=0; + int rule_exists=0; char cmd_buf[CMD_BUFSIZE] = {0}; char target_search[CMD_BUFSIZE] = {0}; char proto_search[CMD_BUFSIZE] = {0}; @@ -106,32 +106,17 @@ rule_exists_no_chk_support(const fko_srv_options_t * const opts, } snprintf(exp_ts_search, CMD_BUFSIZE-1, "%u ", exp_ts); - /* search for each of the substrings, and require the returned - * rule number to be the same across all searches to return true + /* search for each of the substrings - yes, matches from different + * rules may get triggered here, but the expiration time is the + * primary search method */ - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, exp_ts_search, &pid_status, opts); - - if(rtmp > 0) - { - rule_num = rtmp; - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, proto_search, &pid_status, opts); - if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, srcip_search, &pid_status, opts); - if(rtmp == rule_num) - rtmp = (dstip == NULL) ? rtmp : search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, dstip_search, &pid_status, opts); - if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, target_search, &pid_status, opts); - if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, port_search, &pid_status, opts); - if(rtmp == rule_num) - rule_exists = 1; - } + if(search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, exp_ts_search, &pid_status, opts) + && search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, proto_search, &pid_status, opts) + && search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, srcip_search, &pid_status, opts) + && (dstip == NULL) ? 1 : search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, dstip_search, &pid_status, opts) + && search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, target_search, &pid_status, opts) + && search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, port_search, &pid_status, opts)) + rule_exists = 1; if(rule_exists) log_msg(LOG_DEBUG, diff --git a/server/fw_util_iptables.c b/server/fw_util_iptables.c index 55fb6e84..d5e219b3 100644 --- a/server/fw_util_iptables.c +++ b/server/fw_util_iptables.c @@ -73,7 +73,7 @@ rule_exists_no_chk_support(const fko_srv_options_t * const opts, const char * const srcip, const char * const dstip, const unsigned int port, const unsigned int exp_ts) { - int rule_exists=0, rule_num=0, rtmp=0; + int rule_exists=0; char cmd_buf[CMD_BUFSIZE] = {0}; char target_search[CMD_BUFSIZE] = {0}; char proto_search[CMD_BUFSIZE] = {0}; @@ -117,36 +117,21 @@ rule_exists_no_chk_support(const fko_srv_options_t * const opts, } snprintf(exp_ts_search, CMD_BUFSIZE-1, "%u ", exp_ts); - /* search for each of the substrings, and require the returned - * rule number to be the same across all searches to return true + /* search for each of the substrings - yes, matches from different + * rules may get triggered here, but the expiration time is the + * primary search method */ - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, exp_ts_search, &pid_status, opts); - - if(rtmp > 0) - { - rule_num = rtmp; - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, proto_search, &pid_status, opts); - if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, srcip_search, &pid_status, opts); - if(rtmp == rule_num) - rtmp = (dstip == NULL) ? rtmp : search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, dstip_search, &pid_status, opts); - if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, target_search, &pid_status, opts); - if(rtmp == rule_num) - rtmp = search_extcmd(cmd_buf, WANT_STDERR, - NO_TIMEOUT, port_search, &pid_status, opts); - if(rtmp == rule_num) - rule_exists = 1; - } + if(search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, exp_ts_search, &pid_status, opts) + && search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, proto_search, &pid_status, opts) + && search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, srcip_search, &pid_status, opts) + && (dstip == NULL) ? 1 : search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, dstip_search, &pid_status, opts) + && search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, target_search, &pid_status, opts) + && search_extcmd(cmd_buf, WANT_STDERR, NO_TIMEOUT, port_search, &pid_status, opts)) + rule_exists = 1; if(rule_exists) log_msg(LOG_DEBUG, - "rule_exists_no_chk_support() %s %u -> %s expires: %u rule (already exists", + "rule_exists_no_chk_support() %s %u -> %s expires: %u rule already exists", proto_search, port, srcip, exp_ts); else log_msg(LOG_DEBUG, diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index d75afcaf..8570ce69 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -21,7 +21,8 @@ our $local_hmac_key_file = 'local_hmac_spa.key'; my $output_dir = 'output'; our $conf_dir = 'conf'; my $run_dir = 'run'; -our $run_tmp_dir = 'runtmp'; +our $run_tmp_dir_top = 'runtmp'; +our $run_tmp_dir = "$run_tmp_dir_top/subdir1/subdir2"; my $cmd_out_tmp = 'cmd.out'; my $server_cmd_tmp = 'server_cmd.out'; my $openssl_cmd_tmp = 'openssl_cmd.out'; @@ -831,12 +832,14 @@ my %test_keys = ( 'server_conf_file' => $OPTIONAL, 'digest_cache_file' => $OPTIONAL, 'cmd_exec_file_owner' => $OPTIONAL, + 'rm_rule_mid_cycle' => $OPTIONAL, 'server_receive_re' => $OPTIONAL, 'positive_output_matches' => $OPTIONAL, 'negative_output_matches' => $OPTIONAL, 'client_and_server_mode' => $OPTIONAL_NUMERIC, 'insert_rule_before_exec' => $OPTIONAL, 'insert_rule_while_running' => $OPTIONAL, + 'insert_duplicate_rule_while_running' => $OPTIONAL, 'weak_server_receive_check' => $OPTIONAL, 'search_for_rule_after_exit' => $OPTIONAL, 'rc_positive_output_matches' => $OPTIONAL, @@ -5293,6 +5296,18 @@ sub client_server_interaction() { $cmd_out_tmp, $curr_test_file); } + if ($test_hr->{'insert_duplicate_rule_while_running'}) { + ### insert duplicate rules - guess that this is for SSH + for (my $i=0; $i < 4; $i++) { + my $time_prefix = '_exp_' . (time() + 2+$i); ### default timeout + &write_test_file("[+] Inserting duplicate rule with expire comment: $time_prefix\n", + $curr_test_file); + &run_cmd("iptables -A FWKNOP_INPUT -p 6 -s $fake_ip -d 0.0.0.0/0 " . + "--dport 22 -m comment --comment $time_prefix -j ACCEPT", + $cmd_out_tmp, $curr_test_file); + } + } + &iptables_rm_chains($test_hr) if $test_hr->{'iptables_rm_chains_after_server_start'}; @@ -5344,11 +5359,20 @@ sub client_server_interaction() { } if ($fw_rule_created) { + if ($test_hr->{'rm_rule_mid_cycle'}) { + &write_test_file("[+] Flushing firewall rules out from under fwknopd...\n", + $curr_test_file); + &run_cmd("$lib_view_str $valgrind_str $fwknopdCmd " . + "$default_server_conf_args --fw-flush $verbose_str", + $cmd_out_tmp, $curr_test_file); + } sleep 3; ### allow time for rule time out. if (&is_fw_rule_active($test_hr)) { - &write_test_file("[-] new fw rule not timed out, setting rv=0.\n", - $curr_test_file); - $rv = 0; + if ($test_hr->{'fw_rule_removed'} ne $REQUIRE_NO_NEW_REMOVED) { + &write_test_file("[-] new fw rule not timed out, setting rv=0.\n", + $curr_test_file); + $rv = 0; + } } else { &write_test_file("[+] new fw rule timed out.\n", $curr_test_file); $fw_rule_removed = 1; @@ -6818,7 +6842,7 @@ sub preserve_previous_test_run_results() { mkdir $output_dir or die "[*] Could not mkdir $output_dir: $!"; } - for my $dir ($run_dir, $run_tmp_dir) { + for my $dir ($run_dir, $run_tmp_dir_top) { if (-d $dir) { rmtree $dir or die $!; } diff --git a/test/tests/rijndael_hmac.pl b/test/tests/rijndael_hmac.pl index 5ec9d0b9..3a1fe721 100644 --- a/test/tests/rijndael_hmac.pl +++ b/test/tests/rijndael_hmac.pl @@ -178,6 +178,47 @@ 'key_file' => $cf{'rc_hmac_b64_key'}, 'client_cycles_per_server_instance' => 3, }, + { + 'category' => 'Rijndael+HMAC', + 'subcategory' => 'client+server', + 'detail' => '--ipt-no-check dupe rule', + 'function' => \&spa_cycle, + 'cmdline' => $default_client_hmac_args, + 'fwknopd_cmdline' => "$fwknopdCmd -c $cf{'def'} -a $cf{'hmac_access'} " . + "-d $default_digest_file -p $default_pid_file " . + "$intf_str --no-ipt-check-support --no-firewd-check-support", + 'fw_rule_created' => $NEW_RULE_REQUIRED, + 'fw_rule_removed' => $REQUIRE_NO_NEW_REMOVED, + 'key_file' => $cf{'rc_hmac_b64_key'}, + 'insert_duplicate_rule_while_running' => $YES, + }, + + { + 'category' => 'Rijndael+HMAC', + 'subcategory' => 'client+server', + 'detail' => 'rm rule mid-cycle', + 'function' => \&spa_cycle, + 'cmdline' => $default_client_hmac_args, + 'fwknopd_cmdline' => "$fwknopdCmd -c $cf{'def'} -a $cf{'hmac_access'} " . + "-d $default_digest_file -p $default_pid_file $intf_str", + 'fw_rule_created' => $NEW_RULE_REQUIRED, + 'fw_rule_removed' => $NEW_RULE_REMOVED, + 'rm_rule_mid_cycle' => $YES, + 'key_file' => $cf{'rc_hmac_b64_key'}, + }, + { + 'category' => 'Rijndael+HMAC', + 'subcategory' => 'client+server', + 'detail' => 'dupe rule mid-cycle', + 'function' => \&spa_cycle, + 'cmdline' => $default_client_hmac_args, + 'fwknopd_cmdline' => "$fwknopdCmd -c $cf{'def'} -a $cf{'hmac_access'} " . + "-d $default_digest_file -p $default_pid_file $intf_str", + 'fw_rule_created' => $NEW_RULE_REQUIRED, + 'fw_rule_removed' => $REQUIRE_NO_NEW_REMOVED, + 'key_file' => $cf{'rc_hmac_b64_key'}, + 'insert_duplicate_rule_while_running' => $YES, + }, { 'category' => 'Rijndael+HMAC', diff --git a/test/tests/rijndael_hmac_cmd_exec.pl b/test/tests/rijndael_hmac_cmd_exec.pl index fde58a8e..4cd9ac56 100644 --- a/test/tests/rijndael_hmac_cmd_exec.pl +++ b/test/tests/rijndael_hmac_cmd_exec.pl @@ -15,6 +15,19 @@ "-d $default_digest_file -p $default_pid_file $intf_str", 'fw_rule_created' => $REQUIRE_NO_NEW_RULE, }, + { + 'category' => 'Rijndael+HMAC', + 'subcategory' => 'client+server', + 'detail' => 'command execution not allowed', + 'function' => \&spa_cycle, + 'cmdline' => qq|$fwknopCmd --server-cmd "touch $cmd_exec_test_file" | . + "-a $fake_ip -D $loopback_ip --rc-file $cf{'rc_hmac_b64_key'} ". + "$verbose_str", + 'fwknopd_cmdline' => "$fwknopdCmd -c $cf{'def'} -a $cf{'hmac_access'} " . + "-d $default_digest_file -p $default_pid_file $intf_str", + 'fw_rule_created' => $REQUIRE_NO_NEW_RULE, + 'server_positive_output_matches' => [qr/Command messages are not allowed/] + }, { 'category' => 'Rijndael+HMAC', 'subcategory' => 'client+server',