Added HMAC support to GPG encryption modes, closes #58

ChangeLog

@@ -2,7 +2,10 @@ fwknop-2.5 (//2013):
     - Major release of new functionality - HMAC authenticated encryption
       support in the encrypt-then-authenticate model for SPA communications.
       Supported HMAC digests include MD5, SHA1, SHA256, SHA384, and SHA512.
-      The default is HMAC-SHA256 when HMAC is used.
+      The default is HMAC-SHA256 when HMAC is used.  HMAC is supported for both
+      Rijndael and GPG encrypted SPA packet data, and provides a significant
+      security benefit since the HMAC verification is more simplisitic than
+      decryption operations (particularly for GPG).
     - [libfko] Significant bug fix to honor the full encryption key length for
       user-supplied Rijndael keys > 16 bytes long.  Previous to this fix,
       only the first 16 bytes of a key were actually used in the encryption/

Makefile.am

@@ -248,6 +248,7 @@ EXTRA_DIST = \
     test/tests/rijndael_hmac.pl \
     test/tests/rijndael_backwards_compatibility.pl \
     test/tests/gpg_no_pw.pl \
+    test/tests/gpg_no_pw_hmac.pl \
     test/tests/gpg.pl \
     test/tests/rijndael_fuzzing.pl \
     test/tests/perl_FKO_module.pl \

lib/cipher_funcs.c

@@ -355,5 +355,40 @@ add_salted_str(fko_ctx_t ctx)
     return(FKO_SUCCESS);
 }
 
+/* See if we need to add the "hQ" string to the front of the
+ * encrypted data.
+*/
+int
+add_gpg_prefix(fko_ctx_t ctx)
+{
+    char           *tbuf;
+
+    if(strncmp(ctx->encrypted_msg,
+            B64_GPG_PREFIX, B64_GPG_PREFIX_STR_LEN))
+    {
+        /* We need to realloc space for the prefix.
+        */
+        tbuf = realloc(ctx->encrypted_msg, ctx->encrypted_msg_len
+                    + B64_GPG_PREFIX_STR_LEN+1);
+        if(tbuf == NULL)
+            return(FKO_ERROR_MEMORY_ALLOCATION);
+
+        memmove(tbuf+B64_GPG_PREFIX_STR_LEN, tbuf, ctx->encrypted_msg_len);
+
+        ctx->encrypted_msg = memcpy(tbuf,
+                B64_GPG_PREFIX, B64_GPG_PREFIX_STR_LEN);
+
+        /* Adjust the encoded msg len for added SALT value and Make sure we
+         * are still a properly NULL-terminated string (Ubuntu was one system
+         * for which this was an issue).
+        */
+        ctx->encrypted_msg_len += B64_GPG_PREFIX_STR_LEN;
+        tbuf[ctx->encrypted_msg_len] = '\0';
+
+        ctx->added_gpg_prefix = 1;
+    }
+
+    return(FKO_SUCCESS);
+}
 
 /***EOF***/

lib/cipher_funcs.h

@@ -48,6 +48,7 @@ size_t rij_decrypt(unsigned char *in, size_t len,
     const char *key, const int key_len,
     unsigned char *out, int encryption_mode);
 int add_salted_str(fko_ctx_t ctx);
+int add_gpg_prefix(fko_ctx_t ctx);
 
 #endif /* CIPHER_FUNCS_H */
 

lib/fko_context.h

@@ -91,6 +91,7 @@ struct fko_context {
     char           *msg_hmac;
     int             msg_hmac_len;
     int             added_salted_str;
+    int             added_gpg_prefix;
 
     /* State info */
     unsigned short  state;

lib/fko_encryption.c

@@ -332,7 +332,6 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key)
 static int
 gpg_decrypt(fko_ctx_t ctx, const char *dec_key)
 {
-    char           *tbuf;
     unsigned char  *cipher;
     size_t          cipher_len;
     int             res, pt_len;
@@ -340,25 +339,8 @@ gpg_decrypt(fko_ctx_t ctx, const char *dec_key)
     /* Now see if we need to add the "hQ" string to the front of the
      * base64-encoded-GPG-encrypted data.
     */
-    if(strncmp(ctx->encrypted_msg, B64_GPG_PREFIX, B64_GPG_PREFIX_STR_LEN))
+    if(! ctx->added_gpg_prefix)
-    {
+        add_gpg_prefix(ctx);
-        /* We need to realloc space for the GPG prefix of hQ.
-        */
-        tbuf = realloc(ctx->encrypted_msg, ctx->encrypted_msg_len + 12);
-        if(tbuf == NULL)
-            return(FKO_ERROR_MEMORY_ALLOCATION);
-
-        memmove(tbuf+B64_GPG_PREFIX_STR_LEN, tbuf, ctx->encrypted_msg_len);
-
-        ctx->encrypted_msg = memcpy(tbuf, B64_GPG_PREFIX, B64_GPG_PREFIX_STR_LEN);
-
-        /* Adjust encrypted msg length for GnuPG prefix and make sure we are still
-         * a properly NULL-terminated string (Ubuntu was one system for
-         * which this was an issue).
-        */
-        ctx->encrypted_msg_len += B64_GPG_PREFIX_STR_LEN;
-        tbuf[ctx->encrypted_msg_len] = '\0';
-    }
 
     /* Create a bucket for the (base64) decoded encrypted data and get the
      * raw cipher data.

lib/fko_hmac.c

@@ -94,11 +94,26 @@ int fko_verify_hmac(fko_ctx_t ctx,
     ctx->encrypted_msg      = tbuf;
     ctx->encrypted_msg_len -= hmac_b64_digest_len;
 
-    /* See if we need to add the "Salted__" string to the front of the
+    if(ctx->encryption_mode == FKO_ENC_MODE_ASYMMETRIC)
-     * encrypted data.
+    {
-    */
+        /* See if we need to add the "hQ" string to the front of the
-    if(! ctx->added_salted_str)
+         * encrypted data.
-        res = add_salted_str(ctx);
+         */
+        if(! ctx->added_gpg_prefix)
+        {
+            res = add_gpg_prefix(ctx);
+        }
+    }
+    else
+    {
+        /* See if we need to add the "Salted__" string to the front of the
+         * encrypted data.
+         */
+        if(! ctx->added_salted_str)
+        {
+            res = add_salted_str(ctx);
+        }
+    }
 
     if (res != FKO_SUCCESS)
     {

server/access.c

@@ -1076,7 +1076,7 @@ parse_access_file(fko_srv_options_t *opts)
             }
             add_acc_string(&(curr_acc->hmac_key_base64), val);
             add_acc_b64_string(&(curr_acc->hmac_key),
-                &curr_acc->hmac_key_len, curr_acc->hmac_key_base64);
+                &(curr_acc->hmac_key_len), curr_acc->hmac_key_base64);
         }
         else if(CONF_VAR_IS(var, "HMAC_KEY"))
         {
@@ -1457,7 +1457,12 @@ dump_access_list(const fko_srv_options_t *opts)
             "==============================================================\n"
             "                 OPEN_PORTS:  %s\n"
             "             RESTRICT_PORTS:  %s\n"
-            "                        KEY:  <see the access.conf file>\n"
+            "                        KEY:  %s\n"
+            "                 KEY_BASE64:  %s\n"
+            "                    KEY_LEN:  %d\n"
+            "                   HMAC_KEY:  %s\n"
+            "            HMAC_KEY_BASE64:  %s\n"
+            "               HMAC_KEY_LEN:  %d\n"
             "          FW_ACCESS_TIMEOUT:  %i\n"
             "            ENABLE_CMD_EXEC:  %s\n"
             "              CMD_EXEC_USER:  %s\n"
@@ -1469,7 +1474,7 @@ dump_access_list(const fko_srv_options_t *opts)
             "              ACCESS_EXPIRE:  %s"  /* asctime() adds a newline */
             "               GPG_HOME_DIR:  %s\n"
             "             GPG_DECRYPT_ID:  %s\n"
-            "             GPG_DECRYPT_PW:  <see the access.conf file>\n"
+            "             GPG_DECRYPT_PW:  %s\n"
             "            GPG_REQUIRE_SIG:  %s\n"
             "GPG_IGNORE_SIG_VERIFY_ERROR:  %s\n"
             "              GPG_REMOTE_ID:  %s\n",
@@ -1477,7 +1482,12 @@ dump_access_list(const fko_srv_options_t *opts)
             acc->source,
             (acc->open_ports == NULL) ? "<not set>" : acc->open_ports,
             (acc->restrict_ports == NULL) ? "<not set>" : acc->restrict_ports,
-            //(acc->key == NULL) ? "<not set>" : acc->key,
+            (acc->key == NULL) ? "<not set>" : "<see the access.conf file>",
+            (acc->key_base64 == NULL) ? "<not set>" : "<see the access.conf file>",
+            acc->key_len ? acc->key_len : 0,
+            (acc->hmac_key == NULL) ? "<not set>" : "<see the access.conf file>",
+            (acc->hmac_key_base64 == NULL) ? "<not set>" : "<see the access.conf file>",
+            acc->hmac_key_len ? acc->hmac_key_len : 0,
             acc->fw_access_timeout,
             acc->enable_cmd_exec ? "Yes" : "No",
             (acc->cmd_exec_user == NULL) ? "<not set>" : acc->cmd_exec_user,
@@ -1489,7 +1499,7 @@ dump_access_list(const fko_srv_options_t *opts)
             (acc->access_expire_time > 0) ? asctime(localtime(&acc->access_expire_time)) : "<not set>\n",
             (acc->gpg_home_dir == NULL) ? "<not set>" : acc->gpg_home_dir,
             (acc->gpg_decrypt_id == NULL) ? "<not set>" : acc->gpg_decrypt_id,
-            //(acc->gpg_decrypt_pw == NULL) ? "<not set>" : acc->gpg_decrypt_pw,
+            (acc->gpg_decrypt_pw == NULL) ? "<not set>" : "<see the access.conf file>",
             acc->gpg_require_sig ? "Yes" : "No",
             acc->gpg_ignore_sig_error  ? "Yes" : "No",
             (acc->gpg_remote_id == NULL) ? "<not set>" : acc->gpg_remote_id

server/incoming_spa.c

@@ -428,7 +428,8 @@ incoming_spa(fko_srv_options_t *opts)
             if(acc->gpg_decrypt_pw != NULL || acc->gpg_allow_no_pw)
             {
                 res = fko_new_with_data(&ctx, (char *)spa_pkt->packet_data, NULL,
-                        0, acc->encryption_mode, NULL, 0, 0);
+                        0, FKO_ENC_MODE_ASYMMETRIC, acc->hmac_key,
+                        acc->hmac_key_len, acc->hmac_type);
                 attempted_decrypt = 1;
                 if(res != FKO_SUCCESS)
                 {

test/test-fwknop.pl

@@ -72,6 +72,7 @@ our %cf = (
     'gpg_access'                   => "$conf_dir/gpg_access.conf",
     'legacy_iv_access'             => "$conf_dir/legacy_iv_access.conf",
     'gpg_no_pw_access'             => "$conf_dir/gpg_no_pw_access.conf",
+    'gpg_no_pw_hmac_access'        => "$conf_dir/gpg_no_pw_hmac_access.conf",
     'tcp_server'                   => "$conf_dir/tcp_server_fwknopd.conf",
     'tcp_pcap_filter'              => "$conf_dir/tcp_pcap_filter_fwknopd.conf",
     'icmp_pcap_filter'             => "$conf_dir/icmp_pcap_filter_fwknopd.conf",
@@ -175,6 +176,7 @@ my @test_files = (
     "$tests_dir/perl_FKO_module.pl",
     "$tests_dir/python_fko.pl",
     "$tests_dir/gpg_no_pw.pl",
+    "$tests_dir/gpg_no_pw_hmac.pl",
     "$tests_dir/gpg.pl",
 );
 #================== end config ===================
@@ -190,6 +192,7 @@ our @rijndael_replay_attacks = ();  ### from tests/rijndael_replay_attacks.pl
 our @rijndael_hmac           = ();  ### from tests/rijndael_hmac.pl
 our @rijndael_fuzzing        = ();  ### from tests/rijndael_fuzzing.pl
 our @gpg_no_pw               = ();  ### from tests/gpg_now_pw.pl
+our @gpg_no_pw_hmac          = ();  ### from tests/gpg_now_pw_hmac.pl
 our @gpg                     = ();  ### from tests/gpg.pl
 our @perl_FKO_module         = ();  ### from tests/perl_FKO_module.pl
 our @python_fko              = ();  ### from tests/python_fko.pl
@@ -281,6 +284,8 @@ our $MATCH_ANY = 1;
 our $MATCH_ALL = 2;
 our $REQUIRE_SUCCESS = 0;
 our $REQUIRE_FAILURE = 1;
+my $ENC_RIJNDAEL = 1;
+my $ENC_GPG      = 2;
 our $LINUX   = 1;
 our $FREEBSD = 2;
 our $MACOSX  = 3;
@@ -410,6 +415,11 @@ our $default_server_gpg_args_no_pw = "LD_LIBRARY_PATH=$lib_dir " .
     "-a $cf{'gpg_no_pw_access'} $intf_str " .
     "-d $default_digest_file -p $default_pid_file";
 
+our $default_server_gpg_args_no_pw_hmac = "LD_LIBRARY_PATH=$lib_dir " .
+    "$valgrind_str $fwknopdCmd -c $cf{'def'} " .
+    "-a $cf{'gpg_no_pw_hmac_access'} $intf_str " .
+    "-d $default_digest_file -p $default_pid_file";
+
 ### point the compiled binaries at the local libary path
 ### instead of any installed libfko instance
 $ENV{'LD_LIBRARY_PATH'} = $lib_dir;
@@ -454,6 +464,7 @@ my @tests = (
     @perl_FKO_module,
     @python_fko,
     @gpg_no_pw,
+    @gpg_no_pw_hmac,
     @gpg,
 );
 
@@ -1101,6 +1112,8 @@ sub client_send_spa_packet() {
         }
 
         if ($is_hmac_type and $hmac_key) {
+            my $enc_mode = $ENC_RIJNDAEL;
+            $enc_mode = $ENC_GPG if $test_hr->{'msg'} =~ /GPG\s/;
             unless (&openssl_hmac_verification($encrypted_msg,
                     $encoded_msg, '', $hmac_key, $b64_decode_key,
                     $hmac_digest, $hmac_mode)) {
@@ -2473,7 +2486,7 @@ sub perl_fko_module_complete_cycle_hmac() {
                             if ($enable_openssl_compatibility_tests) {
                                 unless (&openssl_hmac_verification($encrypted_msg,
                                         '', $msg, $hmac_key, 0, $hmac_digest,
-                                        &hmac_type_to_str($hmac_type))) {
+                                        &hmac_type_to_str($hmac_type), $ENC_RIJNDAEL)) {
                                     $rv = 0;
                                 }
 
@@ -4170,7 +4183,7 @@ sub immediate_binding() {
 
 sub openssl_hmac_verification() {
     my ($encrypted_msg, $encoded_msg, $access_msg, $tmp_key,
-        $b64_decode_key, $hmac_digest, $hmac_mode) = @_;
+        $b64_decode_key, $hmac_digest, $hmac_mode, $enc_mode) = @_;
 
     $openssl_hmac_ctr++;
 
@@ -4214,8 +4227,13 @@ sub openssl_hmac_verification() {
     }
 
     ### transform encrypted message into the format that openssl expects
-    $enc_msg_without_hmac = 'U2FsdGVkX1' . $enc_msg_without_hmac
+    if ($enc_mode) {
-        unless $enc_msg_without_hmac =~ /^U2FsdGVkX1/;
+        $enc_msg_without_hmac = 'U2FsdGVkX1' . $enc_msg_without_hmac
+            unless $enc_msg_without_hmac =~ /^U2FsdGVkX1/;
+    } else {
+        $enc_msg_without_hmac = 'hQ' . $enc_msg_without_hmac
+            unless $enc_msg_without_hmac =~ /^hQ/;
+    }
 
     &write_test_file("    Calculating HMAC over: '$enc_msg_without_hmac'\n",
         $curr_test_file);
@@ -5449,7 +5467,7 @@ sub file_find_regex() {
 }
 
 sub remove_permissions_warnings() {
-    system qq|perl -p -i -e 's/.*not owned by current effective.*\n//' $output_dir/*|;
+    system qq|perl -p -i -e 's/.*not owned by current effective.*\n//' $output_dir/*.test|;
     return;
 }
 

test/tests/gpg_no_pw_hmac.pl

@@ -0,0 +1,115 @@
+@gpg_no_pw_hmac = (
+    ### no password GPG testing
+    {
+        'category' => 'GPG (no pw) HMAC',
+        'subcategory' => 'client+server',
+        'detail'   => 'complete cycle (tcp/22 ssh)',
+        'function' => \&spa_cycle,
+        'cmdline'  => "$default_client_gpg_args_no_homedir "
+            . "--gpg-home-dir $gpg_client_home_dir_no_pw "
+            . "--rc-file $cf{'rc_hmac_b64_key'}",
+        'fwknopd_cmdline'  => $default_server_gpg_args_no_pw_hmac,
+        'fw_rule_created' => $NEW_RULE_REQUIRED,
+        'fw_rule_removed' => $NEW_RULE_REMOVED,
+        'key_file' => $cf{'rc_hmac_b64_key'},
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'GPG (no pw) HMAC',
+        'subcategory' => 'client+server',
+        'detail'   => 'complete cycle (tcp/23 telnet)',
+        'function' => \&spa_cycle,
+        'cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopCmd -A tcp/23 -a $fake_ip -D $loopback_ip --get-key " .
+            "$local_key_file --verbose --verbose " .
+            "--gpg-recipient-key $gpg_server_key " .
+            "--gpg-signer-key $gpg_client_key " .
+            "--gpg-home-dir $gpg_client_home_dir_no_pw " .
+            "--rc-file $cf{'rc_hmac_b64_key'}",
+        'fwknopd_cmdline'  => $default_server_gpg_args_no_pw_hmac,
+        'fw_rule_created' => $NEW_RULE_REQUIRED,
+        'fw_rule_removed' => $NEW_RULE_REMOVED,
+        'key_file' => $cf{'rc_hmac_b64_key'},
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'GPG (no pw) HMAC',
+        'subcategory' => 'client+server',
+        'detail'   => 'complete cycle (tcp/9418 git)',
+        'function' => \&spa_cycle,
+        'cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopCmd -A tcp/9418 -a $fake_ip -D $loopback_ip --get-key " .
+            "$local_key_file --verbose --verbose " .
+            "--gpg-recipient-key $gpg_server_key " .
+            "--gpg-signer-key $gpg_client_key " .
+            "--gpg-home-dir $gpg_client_home_dir_no_pw " .
+            "--rc-file $cf{'rc_hmac_b64_key'}",
+        'fwknopd_cmdline'  => $default_server_gpg_args_no_pw_hmac,
+        'fw_rule_created' => $NEW_RULE_REQUIRED,
+        'fw_rule_removed' => $NEW_RULE_REMOVED,
+        'key_file' => $cf{'rc_hmac_b64_key'},
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'GPG (no pw) HMAC',
+        'subcategory' => 'client+server',
+        'detail'   => 'complete cycle (tcp/60001 git)',
+        'function' => \&spa_cycle,
+        'cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopCmd -A tcp/60001 -a $fake_ip -D $loopback_ip --get-key " .
+            "$local_key_file --verbose --verbose " .
+            "--gpg-recipient-key $gpg_server_key " .
+            "--gpg-signer-key $gpg_client_key " .
+            "--gpg-home-dir $gpg_client_home_dir_no_pw " .
+            "--rc-file $cf{'rc_hmac_b64_key'}",
+        'fwknopd_cmdline'  => $default_server_gpg_args_no_pw_hmac,
+        'fw_rule_created' => $NEW_RULE_REQUIRED,
+        'fw_rule_removed' => $NEW_RULE_REMOVED,
+        'key_file' => $cf{'rc_hmac_b64_key'},
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'GPG (no pw) HMAC',
+        'subcategory' => 'client+server',
+        'detail'   => 'complete cycle (udp/53 dns)',
+        'function' => \&spa_cycle,
+        'cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopCmd -A udp/53 -a $fake_ip -D $loopback_ip --get-key " .
+            "$local_key_file --verbose --verbose " .
+            "--gpg-recipient-key $gpg_server_key " .
+            "--gpg-signer-key $gpg_client_key " .
+            "--gpg-home-dir $gpg_client_home_dir_no_pw " .
+            "--rc-file $cf{'rc_hmac_b64_key'}",
+        'fwknopd_cmdline'  => $default_server_gpg_args_no_pw_hmac,
+        'fw_rule_created' => $NEW_RULE_REQUIRED,
+        'fw_rule_removed' => $NEW_RULE_REMOVED,
+        'key_file' => $cf{'rc_hmac_b64_key'},
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'GPG (no pw) HMAC',
+        'subcategory' => 'client+server',
+        'detail'   => 'replay attack detection',
+        'function' => \&replay_detection,
+        'cmdline'  => "$default_client_gpg_args_no_homedir "
+            . "--gpg-home-dir $gpg_client_home_dir_no_pw "
+            . "--rc-file $cf{'rc_hmac_b64_key'}",
+        'fwknopd_cmdline'  => $default_server_gpg_args_no_pw_hmac,
+        'key_file' => $cf{'rc_hmac_b64_key'},
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'GPG (no pw) HMAC',
+        'subcategory' => 'client+server',
+        'detail'   => 'detect replay #1 (GnuPG prefix)',
+        'function' => \&replay_detection,
+        'pkt_prefix' => 'hQ',
+        'cmdline'  => "$default_client_gpg_args_no_homedir "
+            . "--gpg-home-dir $gpg_client_home_dir_no_pw "
+            . "--rc-file $cf{'rc_hmac_b64_key'}",
+        'fwknopd_cmdline'  => $default_server_gpg_args_no_pw_hmac,
+        'replay_positive_output_matches' => [qr/Data\sis\snot\sa\svalid\sSPA\smessage\sformat/],
+        'fatal'    => $NO
+    },
+
+);

test/tests/rijndael_replay_attacks.pl

@@ -34,6 +34,4 @@
             "$fwknopdCmd $default_server_conf_args $intf_str",
         'fatal'    => $NO
     },
-
-
 );