From: Michael Rash <mbr@cipherdyne.org>
Date: Wed, 12 Sep 2012 01:54:26 +0000 (-0400)
Subject: [server] (Vlad Glagolev) Submitted a patch to fix command exec mode
X-Git-Tag: fwknop-2.0.4~118
X-Git-Url: http://www.cipherdyne.com/cgi-bin/gitweb.cgi?p=fwknop.git;a=commitdiff_plain;h=f8374c8aefe7a3cf4fcc8763267b139a3504cd66

[server] (Vlad Glagolev) Submitted a patch to fix command exec mode

(Vlad Glagolev) Submitted a patch to fix command exec mode
under SPA message type validity test.  Support for command exec mode was
also added to the test suite.
---

diff --git a/CREDITS b/CREDITS
index 7817119..ffdcec6 100644
--- a/CREDITS
+++ b/CREDITS
@@ -72,3 +72,6 @@ Vlad Glagolev
       used for the autoconf configure script.  This functionality was broken in
       be4193d734850fe60f14a26b547525ea0b9ce1e9 through improper handling of
       #define macros from --disable-file-cache.
+    - Submitted a patch to fix command exec mode under SPA message type
+      validity test.  Support for command exec mode was also added to the test
+      suite.
diff --git a/ChangeLog b/ChangeLog
index fc6e21e..0ba633e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,9 @@ fwknop-2.0.4 (09/20/2012):
       --disable-file-cache is used for the autoconf configure script.  This
       functionality was broken in be4193d734850fe60f14a26b547525ea0b9ce1e9
       through improper handling of #define macros from --disable-file-cache.
+    - [server] (Vlad Glagolev) Submitted a patch to fix command exec mode
+      under SPA message type validity test.  Support for command exec mode was
+      also added to the test suite.
 
 fwknop-2.0.3 (09/03/2012):
     - [server] Fernando Arnaboldi from IOActive found several DoS/code
diff --git a/lib/fko_decode.c b/lib/fko_decode.c
index 6dfc14d..b1e36a6 100644
--- a/lib/fko_decode.c
+++ b/lib/fko_decode.c
@@ -259,6 +259,12 @@ fko_decode_spa_data(fko_ctx_t ctx)
 
     ctx->message_type = (unsigned int)atoi(tbuf);
 
+    if(ctx->message_type < 0 || ctx->message_type >= FKO_LAST_MSG_TYPE)
+    {
+        free(tbuf);
+        return(FKO_ERROR_INVALID_DATA);
+    }
+
     /* Extract the SPA message string.
     */
     ndx += t_size + 1;
@@ -285,12 +291,25 @@ fko_decode_spa_data(fko_ctx_t ctx)
 
     b64_decode(tbuf, (unsigned char*)ctx->message);
 
-    /* Require a message similar to: 1.2.3.4,tcp/22
-    */
-    if(validate_access_msg(ctx->message) != FKO_SUCCESS)
+    if(ctx->message_type == FKO_COMMAND_MSG)
     {
-        free(tbuf);
-        return(FKO_ERROR_INVALID_DATA);
+        /* Require a message similar to: 1.2.3.4,<command>
+        */
+        if(validate_cmd_msg(ctx->message) != FKO_SUCCESS)
+        {
+            free(tbuf);
+            return(FKO_ERROR_INVALID_DATA);
+        }
+    }
+    else
+    {
+        /* Require a message similar to: 1.2.3.4,tcp/22
+        */
+        if(validate_access_msg(ctx->message) != FKO_SUCCESS)
+        {
+            free(tbuf);
+            return(FKO_ERROR_INVALID_DATA);
+        }
     }
 
     /* Extract nat_access string if the message_type indicates so.
diff --git a/test/conf/cmd_access.conf b/test/conf/cmd_access.conf
new file mode 100644
index 0000000..3161d77
--- /dev/null
+++ b/test/conf/cmd_access.conf
@@ -0,0 +1,4 @@
+SOURCE: ANY;
+KEY: fwknoptest;
+FW_ACCESS_TIMEOUT:  3;
+ENABLE_CMD_EXEC: Y;
diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl
index b678d31..0ea3f61 100755
--- a/test/test-fwknop.pl
+++ b/test/test-fwknop.pl
@@ -29,6 +29,7 @@ my %cf = (
     'exp_epoch_access'        => "$conf_dir/expired_epoch_stanza_access.conf",
     'invalid_exp_access'      => "$conf_dir/invalid_expire_access.conf",
     'force_nat_access'        => "$conf_dir/force_nat_access.conf",
+    'cmd_access'              => "$conf_dir/cmd_access.conf",
     'local_nat'               => "$conf_dir/local_nat_fwknopd.conf",
     'ipfw_active_expire'      => "$conf_dir/ipfw_active_expire_equal_fwknopd.conf",
     'dual_key_access'         => "$conf_dir/dual_key_usage_access.conf",
@@ -73,6 +74,7 @@ my $default_spa_port = 62201;
 my $non_std_spa_port = 12345;
 
 my $spoof_user = 'testuser';
+my $cmd_exec_test_file = '/tmp/fwknoptest';
 #================== end config ===================
 
 my $passed = 0;
@@ -1491,6 +1493,24 @@ my @tests = (
         'fatal'    => $NO
     },
 
+    ### command execution tests
+    {
+        'category' => 'Rijndael SPA',
+        'subcategory' => 'client+server',
+        'detail'   => 'command execution',
+        'err_msg'  => 'could not complete SPA cycle',
+        'function' => \&spa_cmd_exec_cycle,
+        'cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            qq|$fwknopCmd --server-cmd "echo fwknoptest > $cmd_exec_test_file" | .
+            "-a $fake_ip -D $loopback_ip --get-key $local_key_file " .
+            "--verbose --verbose",
+        'fwknopd_cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopdCmd -c $cf{'def'} -a $cf{'cmd_access'} " .
+            "-d $default_digest_file -p $default_pid_file $intf_str",
+        'fw_rule_created' => $REQUIRE_NO_NEW_RULE,
+        'fatal'    => $NO
+    },
+
     {
         'category' => 'Rijndael SPA',
         'subcategory' => 'server',
@@ -2330,6 +2350,20 @@ sub spoof_username() {
     return $rv;
 }
 
+sub spa_cmd_exec_cycle() {
+    my $test_hr = shift;
+
+    my $rv = &spa_cycle($test_hr);
+
+    if (-e $cmd_exec_test_file) {
+        unlink $cmd_exec_test_file;
+    } else {
+        $rv = 0;
+    }
+
+    return $rv;
+}
+
 sub replay_detection() {
     my $test_hr = shift;
 
@@ -3248,6 +3282,7 @@ sub init() {
     die "[*] $conf_dir directory does not exist." unless -d $conf_dir;
     die "[*] $lib_dir directory does not exist." unless -d $lib_dir;
 
+    unlink $cmd_exec_test_file if -e $cmd_exec_test_file;
     for my $name (keys %cf) {
         die "[*] $cf{$name} does not exist" unless -e $cf{$name};
         chmod 0600, $cf{$name} or die "[*] Could not chmod 0600 $cf{$name}";