[server] Add MAX_FW_TIMEOUT to access.conf variables.

[server] Add MAX_FW_TIMEOUT to access.conf stanzas to allow a
maximum number of seconds for client-specified timeouts in SPA packets.
This fixes issue #226 which was spotted by Jeremiah Rothschild.

ChangeLog

@@ -1,3 +1,8 @@
+fwknop-2.6.10 (11//2016):
+    - [server] Add MAX_FW_TIMEOUT to access.conf stanzas to allow a maximum
+      number of seconds for client-specified timeouts in SPA packets. This
+      fixes issue #226 which was spotted by Jeremiah Rothschild.
+
 fwknop-2.6.9 (06/08/2016):
     - (Jonathan Bennett) Added support for the SHA3 "Keccak" algorithm
       (specifically SHA3_256 and SHA3_512) for SPA HMAC and digest checking.

server/access.c

@@ -1180,6 +1180,17 @@ set_acc_defaults(fko_srv_options_t *opts)
         if(acc->fw_access_timeout < 1)
             acc->fw_access_timeout = DEF_FW_ACCESS_TIMEOUT;
 
+        /* set default max_fw_timeout if necessary
+        */
+        if(acc->max_fw_timeout < 1)
+            acc->max_fw_timeout = DEF_MAX_FW_TIMEOUT;
+
+        if(acc->max_fw_timeout < acc->fw_access_timeout)
+            log_msg(LOG_INFO,
+                "Warning: MAX_FW_TIMEOUT < FW_ACCESS_TIMEOUT, honoring MAX_FW_TIMEOUT for stanza source: '%s' (#%d)",
+                acc->source, i
+            );
+
         /* set default gpg keyring path if necessary
         */
         if(acc->gpg_decrypt_pw != NULL)
@@ -1760,6 +1771,18 @@ parse_access_file(fko_srv_options_t *opts, char *access_filename, int *depth)
                 return EXIT_FAILURE;
             }
         }
+        else if(CONF_VAR_IS(var, "MAX_FW_TIMEOUT"))
+        {
+            curr_acc->max_fw_timeout = strtol_wrapper(val, 0,
+                    RCHK_MAX_FW_TIMEOUT, NO_EXIT_UPON_ERR, &is_err);
+            if(is_err != FKO_SUCCESS)
+            {
+                log_msg(LOG_ERR,
+                    "[*] MAX_FW_TIMEOUT value not in range.");
+                fclose(file_ptr);
+                return EXIT_FAILURE;
+            }
+        }
         else if(CONF_VAR_IS(var, "ENCRYPTION_MODE"))
         {
             if((curr_acc->encryption_mode = enc_mode_strtoint(val)) < 0)
@@ -2236,6 +2259,7 @@ dump_access_list(const fko_srv_options_t *opts)
             "               HMAC_KEY_LEN:  %d\n"
             "           HMAC_DIGEST_TYPE:  %d\n"
             "          FW_ACCESS_TIMEOUT:  %i\n"
+            "             MAX_FW_TIMEOUT:  %i\n"
             "            ENABLE_CMD_EXEC:  %s\n"
             "       ENABLE_CMD_SUDO_EXEC:  %s\n"
             "         CMD_SUDO_EXEC_USER:  %s\n"
@@ -2276,6 +2300,7 @@ dump_access_list(const fko_srv_options_t *opts)
             acc->hmac_key_len ? acc->hmac_key_len : 0,
             acc->hmac_type,
             acc->fw_access_timeout,
+            acc->max_fw_timeout,
             acc->enable_cmd_exec ? "Yes" : "No",
             acc->enable_cmd_sudo_exec ? "Yes" : "No",
             (acc->cmd_sudo_exec_user == NULL) ? "<not set>" : acc->cmd_sudo_exec_user,

server/access.conf

@@ -96,8 +96,19 @@
 # firewall after a valid SPA packet is received from the source IP address
 # that matches this stanza's SOURCE.
 #
-# If  FW_ACCESS_TIMEOUT is not set then the fwknopd default timeout of 30
+# If FW_ACCESS_TIMEOUT is not set then a default timeout of 30 seconds will
-# seconds will automatically be set.
+# automatically be set.
+#
+
+# MAX_FW_TIMEOUT     <seconds>
+#
+# Define the maximum length of time access will be granted by fwknop through
+# the firewall after a valid SPA packet is received. This is mostly useful to
+# ensure that clients using the --fw-timeout argument do not grant themselves
+# unduly long access.
+#
+# If MAX_FW_TIMEOUT is not set then a default timeout of 300 seconds (five
+# minutes) will automatically be set.
 #
 
 # ENABLE_CMD_EXEC       <Y/N>

server/fwknopd_common.h

@@ -132,6 +132,7 @@
 #define DEF_ENABLE_DESTINATION_RULE     "N"
 
 #define DEF_FW_ACCESS_TIMEOUT           30
+#define DEF_MAX_FW_TIMEOUT              300
 
 /* For integer variable range checking
 */
@@ -402,6 +403,7 @@ typedef struct acc_stanza
     int                  hmac_type;
     unsigned char        use_rijndael;
     int                  fw_access_timeout;
+    int                  max_fw_timeout;
     unsigned char        enable_cmd_exec;
     unsigned char        enable_cmd_sudo_exec;
     char                *cmd_sudo_exec_user;

server/incoming_spa.c

@@ -874,7 +874,16 @@ set_timeout(acc_stanza_t *acc, spa_data_t *spadat)
     spadat->fw_access_timeout = DEF_FW_ACCESS_TIMEOUT;
 
     if(spadat->client_timeout > 0)
+        if(acc->max_fw_timeout < spadat->client_timeout)
+        {
+            /* don't allow clients to request more time than the max
+            */
+            spadat->fw_access_timeout = acc->max_fw_timeout;
+        }
+        else
+        {
             spadat->fw_access_timeout = spadat->client_timeout;
+        }
     else if(acc->fw_access_timeout > 0)
         spadat->fw_access_timeout = acc->fw_access_timeout;
 

test/tests/basic_operations.pl

@@ -4058,6 +4058,42 @@
         ],
         'positive_output_matches' => [qr/not\sin\srange/],
     },
+    {
+        'category' => 'basic operations',
+        'subcategory' => 'server',
+        'detail'   => 'invalid MAX_FW_TIMEOUT',
+        'function' => \&server_conf_files,
+        'fwknopd_cmdline' => $server_rewrite_conf_files,
+        'exec_err' => $YES,
+        'server_access_file' => [
+            'SOURCE                 any',
+            'KEY                    testtest',
+            'MAX_FW_TIMEOUT         999999999999'
+        ],
+        'server_conf_file' => [
+            '### comment'
+        ],
+        'positive_output_matches' => [qr/not\sin\srange/],
+    },
+    {
+        'category' => 'basic operations',
+        'subcategory' => 'server',
+        'detail'   => 'MAX_FW_TIMEOUT < FW_ACCESS_TIMEOUT',
+        'function' => \&server_conf_files,
+        'fwknopd_cmdline' => "$server_rewrite_conf_files --dump-config",
+        'exec_err' => $NO,
+        'server_access_file' => [
+            'SOURCE                 any',
+            'KEY                    testtest',
+            'FW_ACCESS_TIMEOUT      30',
+            'MAX_FW_TIMEOUT         20'
+        ],
+        'server_conf_file' => [
+            '### comment'
+        ],
+        'positive_output_matches' => [qr/honoring\sMAX_FW_TIMEOUT/],
+    },
+
     {
         'category' => 'basic operations',
         'subcategory' => 'server',