From ed2d6ec8eaa3624e79697acc653ab59ef3845dd5 Mon Sep 17 00:00:00 2001 From: Franck Joncourt Date: Sun, 7 Apr 2013 19:00:38 +0200 Subject: [PATCH 1/5] Added tests to the test suite in order to check the update. --- test/local_spa.key | 1 + test/test-fwknop.pl | 3 +++ test/tests/client_nat.pl | 24 ++++++++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 test/tests/client_nat.pl diff --git a/test/local_spa.key b/test/local_spa.key index d2eddbdf..f4dc5162 100644 --- a/test/local_spa.key +++ b/test/local_spa.key @@ -1,3 +1,4 @@ 127.0.0.1: fwknoptest localhost: fwknoptest some.host.through.proxy.com: fwknoptest +74.220.215.85: fwknoptest diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index 65ef49c7..5e92ac9d 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -171,6 +171,7 @@ my @test_files = ( "$tests_dir/python_fko.pl", "$tests_dir/gpg_no_pw.pl", "$tests_dir/gpg.pl", + "$tests_dir/client_nat.pl", ); #================== end config =================== @@ -189,6 +190,7 @@ our @gpg = (); ### from tests/gpg.pl our @perl_FKO_module = (); ### from tests/perl_FKO_module.pl our @python_fko = (); ### from tests/python_fko.pl our @rijndael_backwards_compatibility = (); ### from tests/rijndael_backwards_compatibility.pl +our @client_nat_dns_resolution_fko = (); ### from tests/client_nat.pl my $passed = 0; my $failed = 0; @@ -445,6 +447,7 @@ my @tests = ( @python_fko, @gpg_no_pw, @gpg, + @client_nat_dns_resolution_fko, ); my %test_keys = ( diff --git a/test/tests/client_nat.pl b/test/tests/client_nat.pl new file mode 100644 index 00000000..50533991 --- /dev/null +++ b/test/tests/client_nat.pl @@ -0,0 +1,24 @@ +@client_nat_dns_resolution_fko = ( + { + 'category' => 'Franck', + 'subcategory' => 'client nat-local', + 'detail' => 'Bad dns resolution in nat-local mode', + 'function' => \&generic_exec, + 'positive_output_matches' => [qr/Unable\sto\sresolve\swww.cipherdyne.co\sas\san\sip\saddress/i], + 'exec_err' => $YES, + 'cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . + "$fwknopCmd -a 1.2.3.4 -A tcp/22 --nat-local --nat-port 80 -D www.cipherdyne.co", + 'fatal' => $NO + }, + { + 'category' => 'Franck', + 'subcategory' => 'client nat-local', + 'detail' => 'Good dns resolution in nat-local mode', + 'function' => \&generic_exec, + 'positive_output_matches' => [qr/Nat\sAccess:\s74.220.215.85,22/i], + 'cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . + "$fwknopCmd -a 1.2.3.4 -A tcp/22 --nat-local --nat-port 80 -D www.cipherdyne.com " . + "--get-key $local_key_file --no-save-args --verbose --verbose", + 'fatal' => $NO + }, +); From fd767a1f47937c64c60a2a79066d23a0b34a827f Mon Sep 17 00:00:00 2001 From: Franck Joncourt Date: Wed, 10 Apr 2013 16:06:06 +0200 Subject: [PATCH 2/5] Resolve ip address in all of tha nat modes (mrash/fwknop#43). --- client/fwknop.c | 155 ++++++++++++++++++++++++++++++++++++++------- test/local_spa.key | 1 - 2 files changed, 133 insertions(+), 23 deletions(-) diff --git a/client/fwknop.c b/client/fwknop.c index c845701a..722e42db 100644 --- a/client/fwknop.c +++ b/client/fwknop.c @@ -59,14 +59,117 @@ static void clean_exit(fko_ctx_t ctx, fko_cli_options_t *opts, unsigned int exit_status); static void *get_in_addr(struct sockaddr *sa); static int resolve_dest_adr(const char *dns_str, struct addrinfo *hints, char *ip_str, size_t ip_bufsize); +static int is_hostname_str_with_port(const char *str, char *hostname, size_t hostname_bufsize, int *port); -#define MAX_CMDLINE_ARGS 50 /* should be way more than enough */ +#define MAX_CMDLINE_ARGS 50 /*!< should be way more than enough */ +#define IPV4_STR_TEMPLATE "%u.%u.%u.%u" /*!< Template for a string as an ipv4 address with sscanf */ +#define NAT_ACCESS_STR_TEMPLATE "%s,%d" /*!< Template for a nat access string ip,port with sscanf*/ +#define HOSTNAME_BUFSIZE 64 /*!< Maximum size of a hostname string */ +/** + * @brief Check whether a string is an ipv4 address or not + * + * @param str String to check for an ipv4 address. + * + * @return 1 if the string is an ipv4 address, 0 otherwise. + */ +static int +is_ipv4_str(char *str) +{ + int o1, o2, o3, o4; + int valid_ipv4; + + /* Check format and values. + */ + if((sscanf(str, IPV4_STR_TEMPLATE, &o1, &o2, &o3, &o4)) == 4 + && o1 >= 0 && o1 <= 255 + && o2 >= 0 && o2 <= 255 + && o3 >= 0 && o3 <= 255 + && o4 >= 0 && o4 <= 255) + { + valid_ipv4 = 1; + } + else + valid_ipv4 = 0; + + return valid_ipv4; +} + +/** + * @brief Check whether a string is an ipv6 address or not + * + * @param str String to check for an ipv6 address. + * + * @return 1 if the string is an ipv6 address, 0 otherwise. + */ +static int +is_ipv6_str(char *str) +{ + return 0; +} + +/** + * @brief Check a string to find out if it is built as 'hostname,port' + * + * This function check if we can extract an hostname and a port from the string. + * If yes, we return 1, and both the hostname buffer and the port number are set + * accordingly. + * + * We could have used sscanf() here with a template "%[^,],%u", but this way we + * do not limit the size of the value copy in the hostname destination buffer. + * Limiting the string in the sscanf() can be done but would prevent any easy change + * for the hostname buffer size. + * + * @param str String to parse. + * @param hostname Buffer where to store the hostname value read from @str. + * @param hostname_bufsize Hostname buffer size. + * @param port Value of the port read from @str. + * + * @return 1 if the string is built as 'hostname,port', 0 otherwise. + */ +static int +is_hostname_str_with_port(const char *str, char *hostname, size_t hostname_bufsize, int *port) +{ + int valid = 0; /* Result of the function */ + char buf[MAX_LINE_LEN]; /* Copy of the buffer eg. "hostname,port" */ + char *h; /* Pointer on the hostname string */ + char *p; /* Ponter on the port string */ + + memset(buf, 0, sizeof(buf)); + memset(hostname, 0, hostname_bufsize); + *port = 0; + + /* Replace the comma in the string with a NULL char to split the + * buffer in two strings (hostname and port) */ + strlcpy(buf, str, sizeof(buf)); + p = strchr(buf, ','); + + if(p != NULL) + { + *p++ = 0; + h = buf; + + *port = atoi(p); + + /* If the string does not match an ipv4 or ipv6 address we assume this + * is an hostname. We make sure the port is in the good range too */ + if ( (is_ipv4_str(buf) == 0) + && (is_ipv6_str(buf) == 0) + && ((*port > 0) && (*port < 65536)) ) + { + strlcpy(hostname, h, hostname_bufsize); + valid = 1; + } + } + + return valid; +} + /** * @brief Grab the sin address from the sockaddr structure. * - * This functions returns the sin address as a sockaddr_in or sockaddr_in6 + * This function returns the sin address as a sockaddr_in or sockaddr_in6 * structure according to the family set (ipv4 or ipv6) in the sockaddr * structure. * @@ -74,7 +177,8 @@ static int resolve_dest_adr(const char *dns_str, struct addrinfo *hints, char *i * * @return the sin addr if the sa family is AF_INET or the sin6_addr otherwise. */ -static void *get_in_addr(struct sockaddr *sa) +static void * +get_in_addr(struct sockaddr *sa) { if (sa->sa_family == AF_INET) { @@ -97,7 +201,8 @@ static void *get_in_addr(struct sockaddr *sa) * * @return 0 if successful, 1 if an error occured. */ -static int resolve_dest_adr(const char *dns_str, struct addrinfo *hints, char *ip_str, size_t ip_bufsize) +static int +resolve_dest_adr(const char *dns_str, struct addrinfo *hints, char *ip_str, size_t ip_bufsize) { int error; /* Function error return code */ struct addrinfo *result; /* Result of getaddrinfo() */ @@ -747,7 +852,9 @@ set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options, const char * const acc char nat_access_buf[MAX_LINE_LEN] = {0}; char tmp_access_port[MAX_PORT_STR_LEN+1], *ndx = NULL; int access_port = 0, i = 0, is_err = 0; - char dst_ip_str[INET_ADDRSTRLEN]; + char dst_ip_str[INET_ADDRSTRLEN] = {0}; + char hostname[HOSTNAME_BUFSIZE] = {0}; + int port = 0; struct addrinfo hints; memset(nat_access_buf, 0x0, MAX_LINE_LEN); @@ -781,22 +888,7 @@ set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options, const char * const acc if (options->nat_local && options->nat_access_str[0] == 0x0) { - /* Speed up the name resolution by forcing ipv4 (AF_INET). - * A NULL pointer could be used instead if there is no constraint. - * Maybe when ipv6 support will be enable the structure could initialize the - * family to either AF_INET or AF_INET6 */ - hints.ai_family = AF_INET; - - if (resolve_dest_adr(options->spa_server_str, &hints, dst_ip_str, sizeof(dst_ip_str)) != 0) - { - fprintf(stderr, "[*] Unable to resolve %s as an ip address\n", - options->spa_server_str); - clean_exit(ctx, options, EXIT_FAILURE); - } - - strlcpy(options->spa_server_str, dst_ip_str, sizeof(options->spa_server_str)); - - snprintf(nat_access_buf, MAX_LINE_LEN, "%s,%d", + snprintf(nat_access_buf, MAX_LINE_LEN, NAT_ACCESS_STR_TEMPLATE, options->spa_server_str, access_port); } @@ -809,11 +901,30 @@ set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options, const char * const acc } else { - snprintf(nat_access_buf, MAX_LINE_LEN, "%s,%d", + snprintf(nat_access_buf, MAX_LINE_LEN, NAT_ACCESS_STR_TEMPLATE, options->nat_access_str, access_port); } } + if (is_hostname_str_with_port(nat_access_buf, hostname, sizeof(hostname), &port)) + { + /* Speed up the name resolution by forcing ipv4 (AF_INET). + * A NULL pointer could be used instead if there is no constraint. + * Maybe when ipv6 support will be enable the structure could initialize the + * family to either AF_INET or AF_INET6 */ + hints.ai_family = AF_INET; + + if (resolve_dest_adr(hostname, &hints, dst_ip_str, sizeof(dst_ip_str)) != 0) + { + fprintf(stderr, "[*] Unable to resolve %s as an ip address\n", + hostname); + clean_exit(ctx, options, EXIT_FAILURE); + } + + snprintf(nat_access_buf, MAX_LINE_LEN, NAT_ACCESS_STR_TEMPLATE, + dst_ip_str, port); + } + if(options->nat_rand_port) { /* Must print to stdout what the random port is since diff --git a/test/local_spa.key b/test/local_spa.key index f4dc5162..d2eddbdf 100644 --- a/test/local_spa.key +++ b/test/local_spa.key @@ -1,4 +1,3 @@ 127.0.0.1: fwknoptest localhost: fwknoptest some.host.through.proxy.com: fwknoptest -74.220.215.85: fwknoptest From 9faa625d956ac0a9da881d008055840d7ba2713f Mon Sep 17 00:00:00 2001 From: Franck Joncourt Date: Thu, 11 Apr 2013 13:08:36 +0200 Subject: [PATCH 3/5] Removed tests. --- test/tests/client_nat.pl | 24 ------------------------ 1 file changed, 24 deletions(-) delete mode 100644 test/tests/client_nat.pl diff --git a/test/tests/client_nat.pl b/test/tests/client_nat.pl deleted file mode 100644 index 50533991..00000000 --- a/test/tests/client_nat.pl +++ /dev/null @@ -1,24 +0,0 @@ -@client_nat_dns_resolution_fko = ( - { - 'category' => 'Franck', - 'subcategory' => 'client nat-local', - 'detail' => 'Bad dns resolution in nat-local mode', - 'function' => \&generic_exec, - 'positive_output_matches' => [qr/Unable\sto\sresolve\swww.cipherdyne.co\sas\san\sip\saddress/i], - 'exec_err' => $YES, - 'cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopCmd -a 1.2.3.4 -A tcp/22 --nat-local --nat-port 80 -D www.cipherdyne.co", - 'fatal' => $NO - }, - { - 'category' => 'Franck', - 'subcategory' => 'client nat-local', - 'detail' => 'Good dns resolution in nat-local mode', - 'function' => \&generic_exec, - 'positive_output_matches' => [qr/Nat\sAccess:\s74.220.215.85,22/i], - 'cmdline' => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " . - "$fwknopCmd -a 1.2.3.4 -A tcp/22 --nat-local --nat-port 80 -D www.cipherdyne.com " . - "--get-key $local_key_file --no-save-args --verbose --verbose", - 'fatal' => $NO - }, -); From d988f95a46994de722424c63faebb4537315becd Mon Sep 17 00:00:00 2001 From: Franck Joncourt Date: Thu, 11 Apr 2013 13:36:58 +0200 Subject: [PATCH 4/5] Fixed test-fwknop.pl to remove any references to my test files. --- test/test-fwknop.pl | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index d7f989bd..23353a50 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -174,7 +174,6 @@ my @test_files = ( "$tests_dir/python_fko.pl", "$tests_dir/gpg_no_pw.pl", "$tests_dir/gpg.pl", - "$tests_dir/client_nat.pl", ); #================== end config =================== @@ -193,7 +192,6 @@ our @gpg = (); ### from tests/gpg.pl our @perl_FKO_module = (); ### from tests/perl_FKO_module.pl our @python_fko = (); ### from tests/python_fko.pl our @rijndael_backwards_compatibility = (); ### from tests/rijndael_backwards_compatibility.pl -our @client_nat_dns_resolution_fko = (); ### from tests/client_nat.pl my $passed = 0; my $failed = 0; @@ -449,7 +447,6 @@ my @tests = ( @python_fko, @gpg_no_pw, @gpg, - @client_nat_dns_resolution_fko, ); my %test_keys = ( From fbd38d805b2fca970369c16fe3cd936272288165 Mon Sep 17 00:00:00 2001 From: Franck Joncourt Date: Fri, 12 Apr 2013 14:48:26 +0200 Subject: [PATCH 5/5] Added some else statements and their comments. --- client/fwknop.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/client/fwknop.c b/client/fwknop.c index 722e42db..f80a04dd 100644 --- a/client/fwknop.c +++ b/client/fwknop.c @@ -161,8 +161,14 @@ is_hostname_str_with_port(const char *str, char *hostname, size_t hostname_bufsi strlcpy(hostname, h, hostname_bufsize); valid = 1; } + + /* The port is out of range or the ip is an ipv6 or ipv4 address */ + else; } + /* No port found in the string, let's skip */ + else; + return valid; } @@ -906,6 +912,7 @@ set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options, const char * const acc } } + /* Check if there is a hostname to resolve as an ip address in the NAT access buffer */ if (is_hostname_str_with_port(nat_access_buf, hostname, sizeof(hostname), &port)) { /* Speed up the name resolution by forcing ipv4 (AF_INET). @@ -925,6 +932,9 @@ set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options, const char * const acc dst_ip_str, port); } + /* Nothing to resolve */ + else; + if(options->nat_rand_port) { /* Must print to stdout what the random port is since