[libfko] validation of NAT access strings

Added validation of NAT access strings in the various NAT modes in libfko.
This applies to both the client and server, and test suite support was added
as well.
This commit is contained in:
Michael Rash 2012-10-15 20:52:23 -04:00
parent bf22778ada
commit e0d86f9a33
7 changed files with 252 additions and 108 deletions

View File

@ -17,6 +17,8 @@ fwknop-2.0.4 (09/20/2012):
UDP with a spoofed source IP address. This is in addition to the
original 'tcpraw' and 'icmp' protocols that also support a spoofed
source IP.
- [libfko] Added validation of NAT access strings in the various NAT
modes.
- [server] Bug fix to accept SPA packets over ICMP if the fwknop client
is executed with '-P icmp' and the user has the required privileges.
- Applied patch from Franck Joncourt to have the perl FKO module link

View File

@ -151,6 +151,7 @@ EXTRA_DIST = \
test/conf/subnet_source_match_access.conf \
test/conf/local_nat_fwknopd.conf \
test/conf/disable_aging_fwknopd.conf \
test/conf/disable_aging_nat_fwknopd.conf \
test/conf/fuzzing_source_access.conf \
test/conf/fuzzing_open_ports_access.conf \
test/conf/fuzzing_restrict_ports_access.conf \

View File

@ -342,6 +342,12 @@ fko_decode_spa_data(fko_ctx_t ctx)
}
b64_decode(tbuf, (unsigned char*)ctx->nat_access);
if(validate_nat_access_msg(ctx->nat_access) != FKO_SUCCESS)
{
free(tbuf);
return(FKO_ERROR_INVALID_DATA);
}
}
/* Now look for a server_auth string.

View File

@ -33,6 +33,83 @@
#include "fko_message.h"
#include "fko.h"
static int
have_allow_ip(const char *msg)
{
const char *ndx = msg;
char ip_str[MAX_IPV4_STR_LEN];
int dot_ctr = 0, char_ctr = 0;
int res = FKO_SUCCESS;
#if HAVE_SYS_SOCKET_H
struct in_addr in;
#endif
while(*ndx != ',' && *ndx != '\0')
{
ip_str[char_ctr] = *ndx;
char_ctr++;
if(char_ctr >= MAX_IPV4_STR_LEN)
{
res = FKO_ERROR_INVALID_ALLOW_IP;
break;
}
if(*ndx == '.')
dot_ctr++;
else if(isdigit(*ndx) == 0)
{
res = FKO_ERROR_INVALID_ALLOW_IP;
break;
}
ndx++;
}
if(char_ctr < MAX_IPV4_STR_LEN)
ip_str[char_ctr] = '\0';
else
res = FKO_ERROR_INVALID_ALLOW_IP;
if ((res == FKO_SUCCESS) && (char_ctr < MIN_IPV4_STR_LEN))
res = FKO_ERROR_INVALID_ALLOW_IP;
if((res == FKO_SUCCESS) && dot_ctr != 3)
res = FKO_ERROR_INVALID_ALLOW_IP;
#if HAVE_SYS_SOCKET_H
/* Stronger IP validation now that we have a candidate that looks
* close enough
*/
if((res == FKO_SUCCESS) && (inet_aton(ip_str, &in) == 0))
res = FKO_ERROR_INVALID_ALLOW_IP;
#endif
return(res);
}
static int
have_port(const char *msg)
{
const char *ndx = msg;
int startlen = strnlen(msg, MAX_SPA_MESSAGE_SIZE), port_str_len = 0;
if(startlen == MAX_SPA_MESSAGE_SIZE)
return(FKO_ERROR_INVALID_DATA);
/* Must have at least one digit for the port number
*/
if(isdigit(*ndx) == 0)
return(FKO_ERROR_INVALID_SPA_ACCESS_MSG);
while(*ndx != '\0' && *ndx != ',')
{
port_str_len++;
if((isdigit(*ndx) == 0) || (port_str_len > MAX_PORT_STR_LEN))
return(FKO_ERROR_INVALID_SPA_ACCESS_MSG);
ndx++;
}
return FKO_SUCCESS;
}
/* Set the SPA message type.
*/
int
@ -93,24 +170,10 @@ fko_set_spa_message(fko_ctx_t ctx, const char *msg)
/* Basic message type and format checking...
*/
switch(ctx->message_type)
{
case FKO_COMMAND_MSG:
res = validate_cmd_msg(msg);
break;
case FKO_ACCESS_MSG:
case FKO_CLIENT_TIMEOUT_ACCESS_MSG:
res = validate_access_msg(msg);
break;
case FKO_NAT_ACCESS_MSG:
case FKO_LOCAL_NAT_ACCESS_MSG:
case FKO_CLIENT_TIMEOUT_NAT_ACCESS_MSG:
case FKO_CLIENT_TIMEOUT_LOCAL_NAT_ACCESS_MSG:
res = validate_nat_access_msg(msg);
break;
}
if(ctx->message_type == FKO_COMMAND_MSG)
res = validate_cmd_msg(msg);
else
res = validate_access_msg(msg);
if(res != FKO_SUCCESS)
return(res);
@ -158,9 +221,9 @@ validate_cmd_msg(const char *msg)
if(startlen == MAX_SPA_CMD_LEN)
return(FKO_ERROR_INVALID_DATA);
/* Should have a valid allow IP.
/* Should always have a valid allow IP regardless of message type
*/
if((res = got_allow_ip(msg)) != FKO_SUCCESS)
if((res = have_allow_ip(msg)) != FKO_SUCCESS)
return(res);
/* Commands are fairly free-form so all we can really verify is
@ -184,9 +247,9 @@ validate_access_msg(const char *msg)
if(startlen == MAX_SPA_MESSAGE_SIZE)
return(FKO_ERROR_INVALID_DATA);
/* Should have a valid allow IP.
/* Should always have a valid allow IP regardless of message type
*/
if((res = got_allow_ip(msg)) != FKO_SUCCESS)
if((res = have_allow_ip(msg)) != FKO_SUCCESS)
return(res);
/* Position ourselves beyond the allow IP and make sure we are
@ -208,10 +271,43 @@ validate_access_msg(const char *msg)
return(res);
}
int
validate_nat_access_msg(const char *msg)
{
const char *ndx;
int res = FKO_SUCCESS;
int startlen = strnlen(msg, MAX_SPA_MESSAGE_SIZE);
if(startlen == MAX_SPA_MESSAGE_SIZE)
return(FKO_ERROR_INVALID_DATA);
/* Should always have a valid allow IP regardless of message type
*/
if((res = have_allow_ip(msg)) != FKO_SUCCESS)
return(res);
/* Position ourselves beyond the allow IP and make sure we have
* a single port value
*/
ndx = strchr(msg, ',');
if(ndx == NULL || (1+(ndx - msg)) >= startlen)
return(FKO_ERROR_INVALID_SPA_NAT_ACCESS_MSG);
ndx++;
if((res = have_port(ndx)) != FKO_SUCCESS)
return(res);
if(msg[startlen-1] == ',')
return(FKO_ERROR_INVALID_SPA_NAT_ACCESS_MSG);
return(res);
}
int
validate_proto_port_spec(const char *msg)
{
int startlen = strnlen(msg, MAX_SPA_MESSAGE_SIZE), port_str_len = 0;
int startlen = strnlen(msg, MAX_SPA_MESSAGE_SIZE);
const char *ndx = msg;
if(startlen == MAX_SPA_MESSAGE_SIZE)
@ -233,86 +329,7 @@ validate_proto_port_spec(const char *msg)
*/
ndx++;
/* Must have at least one digit for the port number
*/
if(isdigit(*ndx) == 0)
return(FKO_ERROR_INVALID_SPA_ACCESS_MSG);
while(*ndx != '\0' && *ndx != ',')
{
port_str_len++;
if((isdigit(*ndx) == 0) || (port_str_len > MAX_PORT_STR_LEN))
return(FKO_ERROR_INVALID_SPA_ACCESS_MSG);
ndx++;
}
return(FKO_SUCCESS);
}
int
validate_nat_access_msg(const char *msg)
{
int res = FKO_SUCCESS;
/* Should have a valid access message.
*/
if((res = validate_access_msg(msg)) != FKO_SUCCESS)
return(res);
// --DSS TODO: XXX: Put nat_access validation code here
return(FKO_SUCCESS);
}
int
got_allow_ip(const char *msg)
{
const char *ndx = msg;
char ip_str[MAX_IPV4_STR_LEN];
int dot_ctr = 0, char_ctr = 0;
int res = FKO_SUCCESS;
#if HAVE_SYS_SOCKET_H
struct in_addr in;
#endif
while(*ndx != ',' && *ndx != '\0')
{
ip_str[char_ctr] = *ndx;
char_ctr++;
if(char_ctr >= MAX_IPV4_STR_LEN)
{
res = FKO_ERROR_INVALID_ALLOW_IP;
break;
}
if(*ndx == '.')
dot_ctr++;
else if(isdigit(*ndx) == 0)
{
res = FKO_ERROR_INVALID_ALLOW_IP;
break;
}
ndx++;
}
if(char_ctr < MAX_IPV4_STR_LEN)
ip_str[char_ctr] = '\0';
else
res = FKO_ERROR_INVALID_ALLOW_IP;
if ((res == FKO_SUCCESS) && (char_ctr < MIN_IPV4_STR_LEN))
res = FKO_ERROR_INVALID_ALLOW_IP;
if((res == FKO_SUCCESS) && dot_ctr != 3)
res = FKO_ERROR_INVALID_ALLOW_IP;
#if HAVE_SYS_SOCKET_H
/* Stronger IP validation now that we have a candidate that looks
* close enough
*/
if((res == FKO_SUCCESS) && (inet_aton(ip_str, &in) == 0))
res = FKO_ERROR_INVALID_ALLOW_IP;
#endif
return(res);
return have_port(ndx);
}
/***EOF***/

View File

@ -49,9 +49,8 @@
*/
int validate_cmd_msg(const char *msg);
int validate_access_msg(const char *msg);
int validate_proto_port_spec(const char *msg);
int validate_nat_access_msg(const char *msg);
int got_allow_ip(const char *msg);
int validate_proto_port_spec(const char *msg);
#endif /* FKO_MESSAGE_H */

View File

@ -36,6 +36,8 @@
int
fko_set_spa_nat_access(fko_ctx_t ctx, const char *msg)
{
int res = FKO_SUCCESS;
/* Context must be initialized.
*/
if(!CTX_INITIALIZED(ctx))
@ -52,6 +54,9 @@ fko_set_spa_nat_access(fko_ctx_t ctx, const char *msg)
if(strnlen(msg, MAX_SPA_NAT_ACCESS_SIZE) == MAX_SPA_NAT_ACCESS_SIZE)
return(FKO_ERROR_DATA_TOO_LARGE);
if((res = validate_nat_access_msg(msg)) != FKO_SUCCESS)
return(res);
/* Just in case this is a subsquent call to this function. We
* do not want to be leaking memory.
*/

View File

@ -54,6 +54,7 @@ my %cf = (
'ip_src_match' => "$conf_dir/ip_source_match_access.conf",
'subnet_src_match' => "$conf_dir/ip_source_match_access.conf",
'disable_aging' => "$conf_dir/disable_aging_fwknopd.conf",
'disable_aging_nat' => "$conf_dir/disable_aging_nat_fwknopd.conf",
'fuzz_source' => "$conf_dir/fuzzing_source_access.conf",
'fuzz_open_ports' => "$conf_dir/fuzzing_open_ports_access.conf",
'fuzz_restrict_ports' => "$conf_dir/fuzzing_restrict_ports_access.conf",
@ -1104,6 +1105,17 @@ my @tests = (
'server_conf' => $cf{'nat'},
'fatal' => $NO
},
{
'category' => 'Rijndael SPA',
'subcategory' => 'client',
'detail' => "NAT bogus IP validation",
'err_msg' => "could not complete NAT SPA cycle",
'function' => \&generic_exec,
'exec_err' => $YES,
'cmdline' => "$default_client_args -N 999.1.1.1:22",
'fatal' => $NO
},
{
'category' => 'Rijndael SPA',
'subcategory' => 'client+server',
@ -1114,8 +1126,8 @@ my @tests = (
'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
"$fwknopdCmd -c $cf{'nat'} -a $cf{'force_nat_access'} " .
"-d $default_digest_file -p $default_pid_file $intf_str",
'server_positive_output_matches' => [qr/to\:$force_nat_host\:22/i],
'server_negative_output_matches' => [qr/to\:$internal_nat_host\:22/i],
'server_positive_output_matches' => [qr/\sto\:$force_nat_host\:22/i],
'server_negative_output_matches' => [qr/\sto\:$internal_nat_host\:22/i],
'fw_rule_created' => $NEW_RULE_REQUIRED,
'fw_rule_removed' => $NEW_RULE_REMOVED,
'server_conf' => $cf{'nat'},
@ -1541,6 +1553,30 @@ my @tests = (
"-d $default_digest_file -p $default_pid_file $intf_str",
'fatal' => $NO
},
{
'category' => 'Rijndael SPA',
'subcategory' => 'FUZZING',
'detail' => 'invalid NAT IP',
'err_msg' => 'server crashed or did not detect error condition',
'function' => \&fuzzer,
### this packet was generated with a modified fwknop client via the
### following command line:
#
# LD_LIBRARY_PATH=../lib/.libs ../client/.libs/fwknop -A tcp/22 \
# -a 127.0.0.2 -D 127.0.0.1 --get-key local_spa.key --verbose \
# --verbose -N 999.1.1.1:22
'fuzzing_pkt' =>
'/v/kYVOqw+NCkg8CNEphPPvH3dOAECWjqiF+NNYnK7yKHer/Gy8wCVNa/Rr/Wnm' .
'siApB3jrXEfyEY3yebJV+PHoYIYC3+4Trt2jxw0m+6iR231Ywhw1JetIPwsv7iQ' .
'ATvSTpZ+qiaoN0PPfy0+7yM6KlaQIu7bfG5E2a6VJTqTZ1qYz3H7QaJfbAtOD8j' .
'yEkDgP5+f49xrRA',
'server_positive_output_matches' => [qr/Args\scontain\sinvalid\sdata/],
'fwknopd_cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
"$fwknopdCmd -c $cf{'disable_aging_nat'} -a $cf{'def_access'} " .
"-d $default_digest_file -p $default_pid_file $intf_str",
'fatal' => $NO
},
{
'category' => 'FUZZING',
'subcategory' => 'server',
@ -1741,6 +1777,14 @@ my @tests = (
'function' => \&perl_fko_module_access_msgs,
'fatal' => $NO
},
{
'category' => 'perl FKO module',
'subcategory' => 'basic ops',
'detail' => 'libfko get/set NAT access msgs',
'err_msg' => 'could not get/set libfko NAT access msgs',
'function' => \&perl_fko_module_nat_access_msgs,
'fatal' => $NO
},
{
'category' => 'perl FKO module',
'subcategory' => 'basic ops',
@ -2958,6 +3002,60 @@ sub perl_fko_module_access_msgs() {
return $rv;
}
sub perl_fko_module_nat_access_msgs() {
my $test_hr = shift;
my $rv = 1;
$fko_obj = FKO->new();
unless ($fko_obj) {
&write_test_file("[-] error FKO->new(): " . FKO::error_str() . "\n",
$current_test_file);
return 0;
}
$fko_obj->spa_message_type(FKO->FKO_NAT_ACCESS_MSG);
for my $msg (@{valid_nat_access_messages()}) {
### set message and then see if it matches
my $status = $fko_obj->spa_nat_access($msg);
if ($status == FKO->FKO_SUCCESS and $fko_obj->spa_nat_access() eq $msg) {
&write_test_file("[+] get/set spa_nat_access(): $msg\n",
$current_test_file);
} else {
&write_test_file("[-] could not get/set spa_nat_access(): $msg " .
FKO::error_str() . "\n",
$current_test_file);
$rv = 0;
last;
}
}
for my $bogus_msg (@{&bogus_nat_access_messages()}, @{&valid_access_messages()}) {
### set message type and then see if it matches
my $status = $fko_obj->spa_nat_access($bogus_msg);
if ($status == FKO->FKO_SUCCESS) {
&write_test_file("[-] libfko allowed bogus " .
"spa_nat_access(): $bogus_msg, got: " . $fko_obj->spa_nat_access() . ' ' .
FKO::error_str() . "\n",
$current_test_file);
$rv = 0;
} else {
&write_test_file("[+] libfko rejected bogus spa_nat_access(): $bogus_msg\n",
$current_test_file);
}
}
$fko_obj->destroy();
return $rv;
}
sub perl_fko_module_cmd_msgs() {
my $test_hr = shift;
@ -3025,6 +3123,14 @@ sub valid_access_messages() {
return \@msgs;
}
sub valid_nat_access_messages() {
my @msgs = (
'1.2.3.4,22',
'123.123.123.123,12345',
);
return \@msgs;
}
sub valid_cmd_messages() {
my @msgs = (
'1.2.3.4,cat /etc/hosts',
@ -3038,6 +3144,15 @@ sub valid_cmd_messages() {
}
sub bogus_access_messages() {
my @msgs = ();
push @msgs, @{&bogus_nat_access_messages()};
push @msgs, '1.1.1.2,12345',
push @msgs, @{&valid_nat_access_messages()};
return \@msgs;
}
sub bogus_nat_access_messages() {
my @msgs = (
'1.2.3.4',
'1.2.3.4.',
@ -3065,7 +3180,6 @@ sub bogus_access_messages() {
pack('a', ""),
'1.1.1.p/12345',
'1.1.1.2,,,,12345',
'1.1.1.2,12345',
'1.1.1.2,icmp/123',
',,,',
'----',