[client+server] ensure HMAC key and encryption passphrase are not the same
authorMichael Rash <mbr@cipherdyne.org>
Sat, 18 May 2013 16:10:18 +0000 (12:10 -0400)
committerMichael Rash <mbr@cipherdyne.org>
Sat, 18 May 2013 16:10:18 +0000 (12:10 -0400)
client/fwknop.c
server/access.c
test/test-fwknop.pl
test/tests/rijndael_hmac.pl

index 3711526..b934ab8 100644 (file)
@@ -1272,6 +1272,18 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
             clean_exit(ctx, options, EXIT_FAILURE);
         }
 
+        /* Make sure the same key is not used for both encryption and the HMAC
+        */
+        if(*hmac_key_len == *key_len)
+        {
+            if(memcmp(hmac_key, key, *key_len) == 0)
+            {
+                log_msg(LOG_VERBOSITY_ERROR,
+                        "[*] The encryption passphrase and the HMAC key should not be identical.");
+                clean_exit(ctx, options, EXIT_FAILURE);
+            }
+        }
+
         res = fko_set_spa_hmac_type(ctx, options->hmac_type);
         if(res != FKO_SUCCESS)
         {
index 88039c9..85be75d 100644 (file)
@@ -860,7 +860,7 @@ set_acc_defaults(fko_srv_options_t *opts)
 static int
 acc_data_is_valid(const acc_stanza_t *acc)
 {
-    if(((acc->key == NULL || !strlen(acc->key))
+    if(((acc->key == NULL || acc->key_len == 0)
       && ((acc->gpg_decrypt_pw == NULL || !strlen(acc->gpg_decrypt_pw))
           && acc->gpg_allow_no_pw == 0))
       || (acc->use_rijndael == 0 && acc->use_gpg == 0 && acc->gpg_allow_no_pw == 0))
@@ -871,6 +871,34 @@ acc_data_is_valid(const acc_stanza_t *acc)
         return(0);
     }
 
+    if(acc->hmac_key_len != 0)
+    {
+        if(((acc->key_len != 0) && (acc->key_len == acc->hmac_key_len)))
+        {
+            if(memcmp(acc->key, acc->hmac_key, acc->hmac_key_len) == 0)
+            {
+                fprintf(stderr,
+                    "[*] The encryption passphrase and HMAC key should not be identical for access stanza source: '%s'\n",
+                    acc->source
+                );
+                return(0);
+            }
+        }
+        else if((acc->gpg_allow_no_pw == 0)
+                && acc->gpg_decrypt_pw != NULL
+                && (strlen(acc->gpg_decrypt_pw) == acc->hmac_key_len))
+        {
+            if(memcmp(acc->gpg_decrypt_pw, acc->hmac_key, acc->hmac_key_len) == 0)
+            {
+                fprintf(stderr,
+                    "[*] The encryption passphrase and HMAC key should not be identical for access stanza source: '%s'\n",
+                    acc->source
+                );
+                return(0);
+            }
+        }
+    }
+
     return(1);
 }
 
index 5c128b3..7b085cb 100755 (executable)
@@ -40,6 +40,7 @@ our %cf = (
     'def_access'                   => "$conf_dir/default_access.conf",
     'hmac_access'                  => "$conf_dir/hmac_access.conf",
     'hmac_get_key_access'          => "$conf_dir/hmac_get_key_access.conf",
+    'hmac_equal_keys_access'       => "$conf_dir/hmac_equal_keys_access.conf",
     'hmac_no_b64_access'           => "$conf_dir/hmac_no_b64_access.conf",
     'hmac_md5_access'              => "$conf_dir/hmac_md5_access.conf",
     'hmac_md5_short_key_access'    => "$conf_dir/hmac_md5_short_key_access.conf",
@@ -108,6 +109,7 @@ our %cf = (
     'rc_def_key'                   => "$conf_dir/fwknoprc_with_default_key",
     'rc_def_b64_key'               => "$conf_dir/fwknoprc_with_default_base64_key",
     'rc_named_key'                 => "$conf_dir/fwknoprc_named_key",
+    'rc_hmac_equal_keys'           => "$conf_dir/fwknoprc_hmac_equal_keys",
     'rc_invalid_b64_key'           => "$conf_dir/fwknoprc_invalid_base64_key",
     'rc_hmac_b64_key'              => "$conf_dir/fwknoprc_default_hmac_base64_key",
     'rc_hmac_b64_key2'             => "$conf_dir/fwknoprc_hmac_key2",
@@ -5742,6 +5744,7 @@ sub file_find_regex() {
 }
 
 sub remove_permissions_warnings() {
+    return unless -e "$output_dir/1.test";
     system qq|perl -p -i -e 's/.*not owned by current effective.*\n//' $output_dir/*.test|;
     system qq|perl -p -i -e 's/.*permissions should only be user.*\n//' $output_dir/*.test|;
     return;
index 71e70f6..9c95ece 100644 (file)
     },
     {
         'category' => 'Rijndael+HMAC',
+        'subcategory' => 'client',
+        'detail'   => 'rc file HMAC+encryption keys not equal',
+        'function' => \&generic_exec,
+        'cmdline'  => "$default_client_args_no_get_key " .
+            "--rc-file $cf{'rc_hmac_equal_keys'}",
+        'positive_output_matches' => [qr/should\snot\sbe\sidentical/i],
+        'exec_err' => $YES,
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'Rijndael+HMAC',
+        'subcategory' => 'server',
+        'detail'   => 'rc file HMAC+encryption keys not equal',
+        'function' => \&generic_exec,
+        'cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+             "$fwknopdCmd -c $cf{'def'} -a $cf{'hmac_equal_keys_access'} " .
+            "-d $default_digest_file -p $default_pid_file $intf_str",
+        'positive_output_matches' => [qr/should\snot\sbe\sidentical/i],
+        'exec_err' => $YES,
+        'fatal'    => $NO
+    },
+
+    {
+        'category' => 'Rijndael+HMAC',
         'subcategory' => 'server',
         'detail'   => 'access file invalid HMAC type arg',
         'function' => \&generic_exec,