[test suite/client] memory leak bug fix and test coverage

This commit fixes a minor memory leak in the fwknop client before
calling exit() when an abnormally large number of command line arguments
are given.  The leak was found with valgrind together with the test
suite (specifically the 'show last args (4)' test):

==23748== 175 bytes in 50 blocks are definitely lost in loss record 1 of 1
==23748==    at 0x4C2C494: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==23748==    by 0x1112F1: run_last_args (fwknop.c:991)
==23748==    by 0x110D36: prev_exec (fwknop.c:916)
==23748==    by 0x10D953: main (fwknop.c:170)

Additional test coverage was added for the client via the
basic_operations.pl tests.
This commit is contained in:
Michael Rash 2014-04-08 21:12:46 -04:00
parent 2e4eea8d49
commit 0ff2100993
4 changed files with 163 additions and 15 deletions

View File

@ -326,6 +326,7 @@ EXTRA_DIST = \
test/invalid2.key \
test/invalid3.key \
test/long_spa.key \
test/invalid.args \
test/test-fwknop.pl \
test/fko-python.py \
test/run-test-suite.sh \

View File

@ -940,9 +940,13 @@ show_last_command(const char * const args_save_file)
}
if ((fgets(args_str, MAX_LINE_LEN, args_file_ptr)) != NULL) {
log_msg(LOG_VERBOSITY_NORMAL, "Last fwknop client command line: %s", args_str);
log_msg(LOG_VERBOSITY_NORMAL,
"Last fwknop client command line: %s", args_str);
} else {
log_msg(LOG_VERBOSITY_NORMAL, "Could not read line from file: %s", args_save_file);
log_msg(LOG_VERBOSITY_NORMAL,
"Could not read line from file: %s", args_save_file);
fclose(args_file_ptr);
return 0;
}
fclose(args_file_ptr);
@ -957,7 +961,7 @@ run_last_args(fko_cli_options_t *options, const char * const args_save_file)
FILE *args_file_ptr = NULL;
int current_arg_ctr = 0;
int argc_new = 0;
int argc_new = 0, args_broken = 0;
int i = 0;
char args_str[MAX_LINE_LEN] = {0};
@ -1002,18 +1006,22 @@ run_last_args(fko_cli_options_t *options, const char * const args_save_file)
{
log_msg(LOG_VERBOSITY_ERROR, "[*] max command line args exceeded.");
fclose(args_file_ptr);
return 0;
args_broken = 1;
break;
}
}
}
}
fclose(args_file_ptr);
/* Reset the options index so we can run through them again.
*/
optind = 0;
if(! args_broken)
{
/* Reset the options index so we can run through them again.
*/
optind = 0;
config_init(options, argc_new, argv_new);
config_init(options, argc_new, argv_new);
}
/* Since we passed in our own copies, free up malloc'd memory
*/
@ -1025,6 +1033,9 @@ run_last_args(fko_cli_options_t *options, const char * const args_save_file)
free(argv_new[i]);
}
if(args_broken)
return 0;
return 1;
}

1
test/invalid.args Normal file
View File

@ -0,0 +1 @@
-A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp -A tcp

View File

@ -40,11 +40,46 @@
'subcategory' => 'client',
'detail' => 'show last args (3)',
'function' => \&rm_last_args,
'positive_output_matches' => [qr/Unable\sto.*HOME/i],
'positive_output_matches' => [qr/Unable\sto\sdetermine/i],
'exec_err' => $YES,
'cmdline' => "env -u HOME $fwknopCmd --show-last",
'cmdline' => "env -u HOME $fwknopCmd --show-last --rc-file $cf{'rc_def_key'}",
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'show last args (4)',
'function' => \&generic_exec,
'exec_err' => $YES,
'cmdline' => "$fwknopCmd --save-args-file empty.args " .
"--show-last --rc-file $cf{'rc_def_key'}",
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'save args too long',
'function' => \&generic_exec,
'exec_err' => $YES,
'cmdline' => "$fwknopCmd -A tcp/22 -a $fake_ip -D $loopback_ip " .
"--get-key $local_key_file --save-args-file too_long.args " . "-A tcp/22 "x300
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'previous args (1)',
'function' => \&generic_exec,
'positive_output_matches' => [qr/max\scommand\sline\sargs/i],
'exec_err' => $YES,
'cmdline' => "$fwknopCmd -l --save-args-file invalid.args",
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'previous args (2)',
'function' => \&generic_exec,
'exec_err' => $YES,
'cmdline' => "$fwknopCmd -l --save-args-file /dev/null",
},
{
'category' => 'basic operations',
'subcategory' => 'client',
@ -171,7 +206,7 @@
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'SPA --key-base64-rijndael invalid',
'detail' => 'SPA --key-base64-rijndael invalid (1)',
'function' => \&generic_exec,
'exec_err' => $YES,
'cmdline' => "$default_client_args_no_get_key --key-base64-rijndael a%aaaaaaaaaaa"
@ -179,12 +214,31 @@
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'SPA --key-base64-hmac invalid',
'detail' => 'SPA --key-base64-rijndael invalid (2)',
'function' => \&generic_exec,
'exec_err' => $YES,
'cmdline' => "$default_client_args_no_get_key --key-base64-rijndael " . 'QUFB'x100 ### 'A' base64 encoded
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'SPA --key-base64-hmac invalid (1)',
'function' => \&generic_exec,
'exec_err' => $YES,
'cmdline' => "$default_client_args_no_get_key " .
"--key-base64-rijndael aaaaaaaaaaaaa --key-base64-hmac a%aaaaaaa"
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'SPA --key-base64-hmac invalid (2)',
'function' => \&generic_exec,
'exec_err' => $YES,
'cmdline' => "$default_client_args_no_get_key " .
"--key-base64-rijndael aaaaaaaaaaaaa --key-base64-hmac " . 'QUFB'x300 ### 'A' base64 encoded
},
{
'category' => 'basic operations',
'subcategory' => 'client',
@ -720,7 +774,7 @@
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'invalid SPA destination',
'detail' => 'invalid SPA destination (1)',
'function' => \&client_rc_file,
'cmdline' => "$lib_view_str $valgrind_str $fwknopCmd -A tcp/22 -a $fake_ip " .
"--no-save-args $verbose_str -D .168.10.1 -n default " .
@ -730,6 +784,19 @@
'exec_err' => $YES,
'positive_output_matches' => [qr/packet\snot\ssent/],
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'invalid SPA destination (2)',
'function' => \&client_rc_file,
'cmdline' => "$lib_view_str $valgrind_str $fwknopCmd -A tcp/22 -a $fake_ip " .
"--no-save-args $verbose_str -D badhost -n default " .
"--rc-file $save_rc_file --save-rc-stanza --force-stanza",
'save_rc_stanza' => [{'name' => 'default',
'vars' => {'KEY' => 'testtest', 'DIGEST_TYPE' => 'MD5'}}],
'exec_err' => $YES,
'positive_output_matches' => [qr/packet\snot\ssent/],
},
{
'category' => 'basic operations',
@ -883,6 +950,19 @@
'positive_output_matches' => [qr/Username\:\ssomeuser/],
'rc_positive_output_matches' => [qr/SPOOF_USER.*someuser/],
},
{
'category' => 'basic operations',
'subcategory' => 'client save rc file',
'detail' => '--spoof-user invalid',
'function' => \&client_rc_file,
'cmdline' => "$client_save_rc_args -n default --spoof-user some=user",
'save_rc_stanza' => [{'name' => 'default',
'vars' => {'KEY' => 'testtest', 'HMAC_KEY' => 'hmactest',
'HMAC_DIGEST_TYPE' => 'SHA1'}}],
'exec_err' => $YES,
'positive_output_matches' => [qr/Args\scontain\sinvalid/],
},
{
'category' => 'basic operations',
'subcategory' => 'client save rc file',
@ -2005,11 +2085,66 @@
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'bad file descriptor',
'detail' => 'pw bad file descriptor (1)',
'function' => \&generic_exec,
'cmdline' => $default_client_args . " --test --fd -1",
'cmdline' => $default_client_args_no_get_key . " --test --fd -1",
'positive_output_matches' => [qr/Value\s.*out\sof\srange/],
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'pw bad file descriptor (2)',
'function' => \&generic_exec,
'cmdline' => $default_client_args_no_get_key . " --test --fd 100",
'positive_output_matches' => [qr/Bad\sfile\sdescriptor/],
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'pw fd 0 PW_BS_CHAR',
'function' => \&generic_exec,
'cmdline' => qq/perl -e 'print "test\x08test"' |/
. $default_client_args_no_get_key . " --test --fd 0",
'positive_output_matches' => [qr/FKO\sVersion/],
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'pw fd 0 PW_BREAK_CHAR',
'function' => \&generic_exec,
'cmdline' => qq/perl -e 'print "test\x03test"' |/
. $default_client_args_no_get_key . " --test --fd 0",
'positive_output_matches' => [qr/FKO\sVersion/],
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'pw fd 0 PW_LF_CHAR',
'function' => \&generic_exec,
'cmdline' => qq/perl -e 'print "test\x0atest"' |/
. $default_client_args_no_get_key . " --test --fd 0",
'positive_output_matches' => [qr/FKO\sVersion/],
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'pw fd 0 PW_CR_CHAR',
'function' => \&generic_exec,
'cmdline' => qq/perl -e 'print "test\x0dtest"' |/
. $default_client_args_no_get_key . " --test --fd 0",
'positive_output_matches' => [qr/FKO\sVersion/],
},
{
'category' => 'basic operations',
'subcategory' => 'client',
'detail' => 'pw fd 0 PW_CLEAR_CHAR',
'function' => \&generic_exec,
'cmdline' => qq/perl -e 'print "test\x15test"' |/
. $default_client_args_no_get_key . " --test --fd 0",
'positive_output_matches' => [qr/FKO\sVersion/],
},
{
'category' => 'basic operations',
'subcategory' => 'client',