From c382febf3dac5f6acbe79565c08661885c263761 Mon Sep 17 00:00:00 2001
From: Michael Rash <mbr@cipherdyne.org>
Date: Tue, 26 Nov 2013 23:48:56 -0500
Subject: [PATCH] [client] use libfko is_valid_ipv4_addr() for IP address
 validation

---
 client/config_init.c           | 22 +++++++++++++++++++++-
 client/fwknop.c                | 32 +-------------------------------
 test/tests/basic_operations.pl |  4 ++--
 test/tests/rijndael.pl         |  2 +-
 4 files changed, 25 insertions(+), 35 deletions(-)

diff --git a/client/config_init.c b/client/config_init.c
index 60e39cd7..8bd980d2 100644
--- a/client/config_init.c
+++ b/client/config_init.c
@@ -953,8 +953,12 @@ parse_rc_param(fko_cli_options_t *options, const char *var_name, char * val)
             strlcpy(options->allow_ip_str, "0.0.0.0", sizeof(options->allow_ip_str));
         else if(strcasecmp(val, "resolve") == 0)
             options->resolve_ip_http = 1;
-        else /* Assume IP address */
+        else /* Assume IP address and validate */
+        {
             strlcpy(options->allow_ip_str, val, sizeof(options->allow_ip_str));
+            if(! is_valid_ipv4_addr(options->allow_ip_str))
+                parse_error = -1;
+        }
     }
     /* Time Offset */
     else if (var->pos == FWKNOP_CLI_ARG_TIME_OFFSET)
@@ -1720,6 +1724,22 @@ validate_options(fko_cli_options_t *options)
                 log_msg(LOG_VERBOSITY_WARNING,
                     "[-] WARNING: Should use -a or -R to harden SPA against potential MITM attacks");
             }
+
+            if(! is_valid_ipv4_addr(options->allow_ip_str))
+            {
+                log_msg(LOG_VERBOSITY_ERROR,
+                    "Invalid allow IP specified for SPA access");
+                exit(EXIT_FAILURE);
+            }
+        }
+    }
+
+    if (options->spoof_ip_src_str[0] != 0x00)
+    {
+        if(! is_valid_ipv4_addr(options->spoof_ip_src_str))
+        {
+            log_msg(LOG_VERBOSITY_ERROR, "Invalid spoof IP");
+            exit(EXIT_FAILURE);
         }
     }
 
diff --git a/client/fwknop.c b/client/fwknop.c
index 13e08c6c..cd571f07 100644
--- a/client/fwknop.c
+++ b/client/fwknop.c
@@ -61,40 +61,10 @@ 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 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 */
 #define CTX_DUMP_BUFSIZE            4096                /*!< Maximum size allocated to a FKO context dump */
 
-/**
- * @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
  *
@@ -152,7 +122,7 @@ is_hostname_str_with_port(const char *str, char *hostname, size_t hostname_bufsi
 
         /* 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)
+        if (   (is_valid_ipv4_addr(buf) == 0)
             && (is_ipv6_str(buf) == 0)
             && ((*port > 0) && (*port < 65536)) )
         {
diff --git a/test/tests/basic_operations.pl b/test/tests/basic_operations.pl
index aa0322e3..f5379c80 100644
--- a/test/tests/basic_operations.pl
+++ b/test/tests/basic_operations.pl
@@ -51,7 +51,7 @@
         'subcategory' => 'client',
         'detail'   => '--allow-ip <IP> valid IP',
         'function' => \&generic_exec,
-        'positive_output_matches' => [qr/Invalid\sallow\sIP\saddress/i],
+        'positive_output_matches' => [qr/Invalid\sallow\sIP/i],
         'exec_err' => $YES,
         'cmdline' => "$fwknopCmd -A tcp/22 -a invalidIP -D $loopback_ip",
     },
@@ -551,7 +551,7 @@
         'save_rc_stanza' => [{'name' => 'default',
                 'vars' => {'KEY' => 'testtest', 'FW_TIMEOUT' => '30'}}],
         'positive_output_matches' => [qr/Wrote.*HMAC.*keys/],
-        'rc_positive_output_matches' => [qr/VERBOSE.*Y/,
+        'rc_positive_output_matches' => [qr/VERBOSE.*(Y|\d)/,
             qr/USE_HMAC.*Y/, qr/KEY_BASE64/, qr/HMAC_KEY_BASE64/],
     },
 
diff --git a/test/tests/rijndael.pl b/test/tests/rijndael.pl
index c1325c5d..2266dbbc 100644
--- a/test/tests/rijndael.pl
+++ b/test/tests/rijndael.pl
@@ -962,7 +962,7 @@
         'cmdline'  => '',
         'fwknopd_cmdline' => "$fwknopdCmd -c $cf{'def'} -a $cf{'legacy_iv_access'} " .
             "-d $default_digest_file -p $default_pid_file " .
-            "--pcap-file $replay_pcap_file --foreground $verbose_str",
+            "--pcap-file $replay_pcap_file --foreground $verbose_str --verbose",
         'server_positive_output_matches' => [qr/Replay\sdetected/i,
             qr/candidate\sSPA/, qr/0x0000\:\s+2b/],
         'fw_rule_created' => $NEW_RULE_REQUIRED,