Added access stanza expiration feature, multiple access stanza bug fix
authorMichael Rash <mbr@cipherdyne.org>
Tue, 29 Nov 2011 03:03:21 +0000 (22:03 -0500)
committerMichael Rash <mbr@cipherdyne.org>
Tue, 29 Nov 2011 03:03:21 +0000 (22:03 -0500)
This commit does two major things:

1) Two new access.conf variables are added "ACCESS_EXPIRE" and
"ACCESS_EXPIRE_EPOCH" to allow access stanzas to be expired without having
to modify the access.conf file and restart fwknopd.

2) Allow an access stanza that matches the SPA source address to not
automatically short circuit other stanzas if there is an error (such as when
there are multiple encryption keys involved and an incoming SPA packet is
meant for, say, the second stanza and the first therefore doesn't allow
proper decryption).

doc/fwknopd.man.asciidoc
server/access.c
server/access.h
server/fw_util_iptables.c
server/fwknopd_common.h
server/incoming_spa.c
server/incoming_spa.h
test/conf/expired_epoch_stanza_access.conf [new file with mode: 0644]
test/conf/expired_stanza_access.conf [new file with mode: 0644]
test/conf/multi_stanzas_with_broken_keys.conf [new file with mode: 0644]
test/test-fwknop.pl

index f25a9d6..f0e4a20 100644 (file)
@@ -175,6 +175,17 @@ See the 'fwknopd.conf' file for the full list and corresponding details.
     synchronization with the *fwknopd* server system (NTP is good).  The
     default age is 120 seconds (two minutes).
 
+*ACCESS_EXPIRE* '<MM/DD/YYYY>'::
+    Defines an expiration date for the access stanza in MM/DD/YYYY format.
+    All SPA packets that match an expired stanza will be ignored.  This
+    parameter is optional.
+
+*ACCESS_EXPIRE_EPOCH* '<seconds>'::
+    Defines an expiration date for the access stanza as the epoch time, and is
+    useful if a more accurate expiration time needs to be given than the day
+    resolution offered by the ACCESS_EXPIRE variable agove.  All SPA packets
+    that match an expired stanza will be ignored.  This parameter is optional.
+
 *ENABLE_DIGEST_PERSISTENCE* '<Y/N>'::
     Track digest sums associated with previous SPA packets processed by
     *fwknopd*.  This allows digest sums to remain persistent across
index 41311c6..e7f8cc9 100644 (file)
@@ -71,6 +71,65 @@ add_acc_bool(unsigned char *var, const char *val)
     return(*var = (strncasecmp(val, "Y", 1) == 0) ? 1 : 0);
 }
 
+/* Add expiration time - convert date to epoch seconds
+*/
+static void
+add_acc_expire_time(fko_srv_options_t *opts, time_t *access_expire_time, const char *val)
+{
+    struct tm tm;
+
+    memset(&tm, 0, sizeof(struct tm));
+
+    if (sscanf(val, "%d/%d/%d", &tm.tm_mon, &tm.tm_mday, &tm.tm_year) != 3)
+    {
+
+        log_msg(LOG_ERR,
+            "Fatal invalid epoch seconds value for access stanza expiration time"
+        );
+        clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
+    }
+
+    if(tm.tm_mon > 0)
+        tm.tm_mon -= 1;  /* 0-11 */
+
+    /* number of years since 1900
+    */
+    if(tm.tm_year > 1900)
+        tm.tm_year -= 1900;
+    else
+        if(tm.tm_year < 100)
+            tm.tm_year += 100;
+
+    *access_expire_time = mktime(&tm);
+
+    return;
+}
+
+/* Add expiration time via epoch seconds defined in access.conf
+*/
+static void
+add_acc_expire_time_epoch(fko_srv_options_t *opts, time_t *access_expire_time, const char *val)
+{
+    char *endptr;
+    unsigned long expire_time = 0;
+
+    errno = 0;
+
+    expire_time = (time_t) strtoul(val, &endptr, 10);
+
+    if (errno == ERANGE || (errno != 0 && expire_time == 0))
+    {
+        log_msg(LOG_ERR,
+            "Fatal invalid epoch seconds value for access stanza expiration time"
+        );
+        clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
+    }
+
+    *access_expire_time = (time_t) expire_time;
+
+    return;
+}
+
 /* Take an IP or Subnet/Mask and convert it to mask for later
  * comparisons of incoming source IPs against this mask.
 */
@@ -848,6 +907,14 @@ parse_access_file(fko_srv_options_t *opts)
         {
             add_acc_string(&(curr_acc->gpg_remote_id), val);
         }
+        else if(CONF_VAR_IS(var, "ACCESS_EXPIRE"))
+        {
+            add_acc_expire_time(opts, &(curr_acc->access_expire_time), val);
+        }
+        else if(CONF_VAR_IS(var, "ACCESS_EXPIRE_EPOCH"))
+        {
+            add_acc_expire_time_epoch(opts, &(curr_acc->access_expire_time), val);
+        }
         else
         {
             fprintf(stderr,
@@ -894,7 +961,7 @@ parse_access_file(fko_srv_options_t *opts)
     return;
 }
 
-static int
+int
 compare_addr_list(acc_int_list_t *source_list, const uint32_t ip)
 {
     int match = 0;
@@ -913,34 +980,6 @@ compare_addr_list(acc_int_list_t *source_list, const uint32_t ip)
     return(match);
 }
 
-/* Check an IP address against the list of allowed SOURCE stanzas.
- * return the a pointer to the access stanza that matches first or
- * NULL if no match is found.
-*/
-acc_stanza_t*
-acc_check_source(fko_srv_options_t *opts, const uint32_t ip)
-{
-    acc_stanza_t    *acc = opts->acc_stanzas;
-
-    if(acc == NULL)
-    {
-        log_msg(LOG_WARNING,
-            "Check access source called with no access stanzas defined."
-        );
-        return(NULL);
-    }
-
-    while(acc)
-    {
-        if(compare_addr_list(acc->source_list, ip))
-            break;
-
-        acc = acc->next;
-    }
-
-    return(acc);
-}
-
 /* Compare the contents of 2 port lists.  Return true on a match.
  * Match depends on the match_any flag.  if match_any is 1 then any
  * entry in the incoming data need only match one item to return true.
@@ -1084,6 +1123,7 @@ dump_access_list(const fko_srv_options_t *opts)
             "              CMD_EXEC_USER:  %s\n"
             "           REQUIRE_USERNAME:  %s\n"
             "     REQUIRE_SOURCE_ADDRESS:  %s\n"
+            "              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"
@@ -1100,6 +1140,7 @@ dump_access_list(const fko_srv_options_t *opts)
             (acc->cmd_exec_user == NULL) ? "<not set>" : acc->cmd_exec_user,
             (acc->require_username == NULL) ? "<not set>" : acc->require_username,
             acc->require_source_address ? "Yes" : "No",
+            (acc->access_expire_time > 0) ? asctime(localtime(&acc->access_expire_time)) : "<not set>",
             (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,
index adc94cb..b650101 100644 (file)
@@ -37,7 +37,7 @@
 /* Function Prototypes
 */
 void parse_access_file(fko_srv_options_t *opts);
-acc_stanza_t* acc_check_source(fko_srv_options_t *opts, const uint32_t ip);
+int compare_addr_list(acc_int_list_t *source_list, const uint32_t ip);
 int acc_check_port_access(acc_stanza_t *acc, char *port_str);
 int acc_check_gpg_remote_id(acc_stanza_t *acc, const char *gpg_id);
 void dump_access_list(const fko_srv_options_t *opts);
index fe632dd..e6ccd84 100644 (file)
@@ -801,7 +801,7 @@ process_spa_request(const fko_srv_options_t *opts, spa_data_t *spadat)
                     snat_chain->next_expire = exp_ts;
             }
             else
-                log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err_buf); 
+                log_msg(LOG_ERR, "Error %i from cmd:'%s': %s", res, cmd_buf, err_buf);
         }
     }
 
index fbc5a83..817891e 100644 (file)
@@ -280,6 +280,8 @@ typedef struct acc_stanza
     unsigned char       gpg_ignore_sig_error;
     char                *gpg_remote_id;
     acc_string_list_t   *gpg_remote_id_list;
+    time_t              access_expire_time;
+    int                 expired;
     struct acc_stanza   *next;
 } acc_stanza_t;
 
index 2c50294..ebd8caa 100644 (file)
@@ -154,17 +154,17 @@ get_spa_data_fields(fko_ctx_t ctx, spa_data_t *spdat)
 
 /* Process the SPA packet data
 */
-int
+void
 incoming_spa(fko_srv_options_t *opts)
 {
     /* Always a good idea to initialize ctx to null if it will be used
-     * repeatedly (especially when using fko_new_with_data().
+     * repeatedly (especially when using fko_new_with_data()).
     */
     fko_ctx_t       ctx = NULL;
 
     char            *spa_ip_demark, *gpg_id;
     time_t          now_ts;
-    int             res, status, ts_diff, enc_type;
+    int             res, status, ts_diff, enc_type, found_acc_sip=0, stanza_num=0;
 
     spa_pkt_info_t *spa_pkt = &(opts->spa_pkt);
 
@@ -172,23 +172,13 @@ incoming_spa(fko_srv_options_t *opts)
     */
     spa_data_t spadat;
 
-    /* Get the access.conf data for the stanza that matches this incoming
-     * source IP address.
+    /* Loop through all access stanzas looking for a match
     */
-    acc_stanza_t   *acc = acc_check_source(opts, ntohl(spa_pkt->packet_src_ip));
+    acc_stanza_t    *acc = opts->acc_stanzas;
 
     inet_ntop(AF_INET, &(spa_pkt->packet_src_ip),
         spadat.pkt_source_ip, sizeof(spadat.pkt_source_ip));
 
-    if(acc == NULL)
-    {
-        log_msg(LOG_WARNING,
-            "No access data found for source IP: %s", spadat.pkt_source_ip
-        );
-
-        return(SPA_MSG_ACCESS_DENIED);
-    }
-
     /* At this point, we want to validate and (if needed) preprocess the
      * SPA data and/or to be reasonably sure we have a SPA packet (i.e
      * try to eliminate obvious non-spa packets).
@@ -199,344 +189,446 @@ incoming_spa(fko_srv_options_t *opts)
         if(opts->verbose > 1)
             log_msg(LOG_INFO, "preprocess_spa_data() returned error %i: '%s' for incoming packet.",
                 res, get_errstr(res));
-        return(res);
+        return;
     }
 
-    log_msg(LOG_INFO, "SPA Packet from IP: %s received.", spadat.pkt_source_ip);
+    while(acc)
+    {
+        stanza_num++;
 
-    if(opts->verbose > 1)
-        log_msg(LOG_INFO, "SPA Packet: '%s'\n", spa_pkt->packet_data);
+        /* Check for a match for the SPA source IP and the access stanza
+        */
+        if(! compare_addr_list(acc->source_list, ntohl(spa_pkt->packet_src_ip)))
+        {
+            acc = acc->next;
+            continue;
+        }
 
-    /* Get encryption type and try its decoding routine first (if the key
-     * for that type is set)
-    */
-    enc_type = fko_encryption_type((char *)spa_pkt->packet_data);
+        found_acc_sip = 1;
 
-    if(enc_type == FKO_ENCRYPTION_RIJNDAEL)
-    {
-        if(acc->key != NULL)
-            res = fko_new_with_data(&ctx, (char *)spa_pkt->packet_data, acc->key);
-        else
+        log_msg(LOG_INFO, "(stanza #%d) SPA Packet from IP: %s received with access source match",
+            stanza_num, spadat.pkt_source_ip);
+
+        if(opts->verbose > 1)
+            log_msg(LOG_INFO, "SPA Packet: '%s'\n", spa_pkt->packet_data);
+
+        /* Make sure this access stanza has not expired
+        */
+        if(acc->access_expire_time > 0)
         {
-            log_msg(LOG_ERR,
-                "No KEY for RIJNDAEL encrypted messages");
-            return(SPA_MSG_FKO_CTX_ERROR);
+            if(acc->expired)
+            {
+                acc = acc->next;
+                continue;
+            }
+            else
+            {
+                if(time(NULL) > acc->access_expire_time)
+                {
+                    log_msg(LOG_INFO, "(stanza #%d) Access stanza has expired",
+                        stanza_num);
+                    acc->expired = 1;
+                    acc = acc->next;
+                    continue;
+                }
+            }
         }
-    }
-    else if(enc_type == FKO_ENCRYPTION_GPG)
-    {
-        /* For GPG we create the new context without decrypting on the fly
-         * so we can set some  GPG parameters first.
+
+        /* Get encryption type and try its decoding routine first (if the key
+         * for that type is set)
         */
-        if(acc->gpg_decrypt_pw != NULL)
+        enc_type = fko_encryption_type((char *)spa_pkt->packet_data);
+
+        if(enc_type == FKO_ENCRYPTION_RIJNDAEL)
         {
-            res = fko_new_with_data(&ctx, (char *)spa_pkt->packet_data, NULL);
-            if(res != FKO_SUCCESS)
+            if(acc->key != NULL)
+                res = fko_new_with_data(&ctx, (char *)spa_pkt->packet_data, acc->key);
+            else
             {
-                log_msg(LOG_WARNING,
-                    "Error creating fko context (before decryption): %s",
-                    fko_errstr(res)
+                log_msg(LOG_ERR,
+                    "(stanza #%d) No KEY for RIJNDAEL encrypted messages",
+                    stanza_num
                 );
-                return(SPA_MSG_FKO_CTX_ERROR);
+                acc = acc->next;
+                continue;
             }
-
-            /* Set whatever GPG parameters we have.
+        }
+        else if(enc_type == FKO_ENCRYPTION_GPG)
+        {
+            /* For GPG we create the new context without decrypting on the fly
+             * so we can set some  GPG parameters first.
             */
-            if(acc->gpg_home_dir != NULL)
-                res = fko_set_gpg_home_dir(ctx, acc->gpg_home_dir);
+            if(acc->gpg_decrypt_pw != NULL)
+            {
+                res = fko_new_with_data(&ctx, (char *)spa_pkt->packet_data, NULL);
                 if(res != FKO_SUCCESS)
                 {
                     log_msg(LOG_WARNING,
-                        "Error setting GPG keyring path to %s: %s",
-                        acc->gpg_home_dir,
-                        fko_errstr(res)
+                        "(stanza #%d) Error creating fko context (before decryption): %s",
+                        stanza_num, fko_errstr(res)
                     );
-                    return(SPA_MSG_FKO_CTX_ERROR);
+                    acc = acc->next;
+                    continue;
                 }
 
-            if(acc->gpg_decrypt_id != NULL)
-                fko_set_gpg_recipient(ctx, acc->gpg_decrypt_id);
+                /* Set whatever GPG parameters we have.
+                */
+                if(acc->gpg_home_dir != NULL)
+                    res = fko_set_gpg_home_dir(ctx, acc->gpg_home_dir);
+                    if(res != FKO_SUCCESS)
+                    {
+                        log_msg(LOG_WARNING,
+                            "(stanza #%d) Error setting GPG keyring path to %s: %s",
+                            stanza_num, acc->gpg_home_dir, fko_errstr(res)
+                        );
+                        acc = acc->next;
+                        continue;
+                    }
+
+                if(acc->gpg_decrypt_id != NULL)
+                    fko_set_gpg_recipient(ctx, acc->gpg_decrypt_id);
+
+                /* If GPG_REQUIRE_SIG is set for this acc stanza, then set
+                 * the FKO context accordingly and check the other GPG Sig-
+                 * related parameters. This also applies when REMOTE_ID is
+                 * set.
+                */
+                if(acc->gpg_require_sig)
+                {
+                    fko_set_gpg_signature_verify(ctx, 1);
 
-            /* If GPG_REQUIRE_SIG is set for this acc stanza, then set
-             * the FKO context accordingly and check the other GPG Sig-
-             * related parameters. This also applies when REMOTE_ID is
-             * set.
-            */
-            if(acc->gpg_require_sig)
-            {
-                fko_set_gpg_signature_verify(ctx, 1);
+                    /* Set whether or not to ignore signature verification errors.
+                    */
+                    fko_set_gpg_ignore_verify_error(ctx, acc->gpg_ignore_sig_error);
+                }
+                else
+                {
+                    fko_set_gpg_signature_verify(ctx, 0);
+                    fko_set_gpg_ignore_verify_error(ctx, 1);
+                }
 
-                /* Set whether or not to ignore signature verification errors.
+                /* Now decrypt the data.
                 */
-                fko_set_gpg_ignore_verify_error(ctx, acc->gpg_ignore_sig_error);
+                res = fko_decrypt_spa_data(ctx, acc->gpg_decrypt_pw);
+
             }
             else
             {
-                fko_set_gpg_signature_verify(ctx, 0);
-                fko_set_gpg_ignore_verify_error(ctx, 1);
+                log_msg(LOG_ERR,
+                    "(stanza #%d) No GPG_DECRYPT_PW for GPG encrypted messages",
+                    stanza_num
+                );
+                acc = acc->next;
+                continue;
             }
-
-            /* Now decrypt the data.
-            */
-            res = fko_decrypt_spa_data(ctx, acc->gpg_decrypt_pw);
-
         }
         else
         {
-            log_msg(LOG_ERR,
-                "No GPG_DECRYPT_PW for GPG encrypted messages");
-            return(SPA_MSG_FKO_CTX_ERROR);
+            log_msg(LOG_ERR, "(stanza #%d) Unable to determing encryption type. Got type=%i.",
+                stanza_num, enc_type);
+            acc = acc->next;
+            continue;
         }
-    }
-    else
-    {
-        log_msg(LOG_ERR, "Unable to determing encryption type. Got type=%i.",
-            enc_type);
-        return(SPA_MSG_FKO_CTX_ERROR);
-    }
 
-    /* Do we have a valid FKO context?
-    */
-    if(res != FKO_SUCCESS)
-    {
-        log_msg(LOG_WARNING, "Error creating fko context: %s",
-            fko_errstr(res));
+        /* Do we have a valid FKO context?
+        */
+        if(res != FKO_SUCCESS)
+        {
+            log_msg(LOG_WARNING, "(stanza #%d) Error creating fko context: %s",
+                stanza_num, fko_errstr(res));
 
-        if(IS_GPG_ERROR(res))
-            log_msg(LOG_WARNING, " - GPG ERROR: %s",
-                fko_gpg_errstr(ctx));
+            if(IS_GPG_ERROR(res))
+                log_msg(LOG_WARNING, "(stanza #%d) - GPG ERROR: %s",
+                    stanza_num, fko_gpg_errstr(ctx));
 
-        goto clean_and_bail;
-    }
+            if(ctx != NULL)
+                fko_destroy(ctx);
+            acc = acc->next;
+            continue;
+        }
 
-    /* At this point, we assume the SPA data is valid.  Now we need to see
-     * if it meets our access criteria.
-    */
-    if(opts->verbose > 1)
-        log_msg(LOG_INFO, "SPA Decode (res=%i):\n%s", res, dump_ctx(ctx));
+        /* At this point, we assume the SPA data is valid.  Now we need to see
+         * if it meets our access criteria.
+        */
+        if(opts->verbose > 1)
+            log_msg(LOG_INFO, "(stanza #%d) SPA Decode (res=%i):\n%s",
+                stanza_num, res, dump_ctx(ctx));
 
-    /* First, if this is a GPG message, and GPG_REMOTE_ID list is not empty,
-     * then we need to make sure this incoming message is signer ID matches
-     * an entry in the list.
-    */
-    if(enc_type == FKO_ENCRYPTION_GPG && acc->gpg_require_sig)
-    {
-        res = fko_get_gpg_signature_id(ctx, &gpg_id);
-        if(res != FKO_SUCCESS)
+        /* First, if this is a GPG message, and GPG_REMOTE_ID list is not empty,
+         * then we need to make sure this incoming message is signer ID matches
+         * an entry in the list.
+        */
+        if(enc_type == FKO_ENCRYPTION_GPG && acc->gpg_require_sig)
         {
-            log_msg(LOG_WARNING, "Error pulling the GPG signature ID from the context: %s",
-                fko_gpg_errstr(ctx));
-            goto clean_and_bail;
-        }
+            res = fko_get_gpg_signature_id(ctx, &gpg_id);
+            if(res != FKO_SUCCESS)
+            {
+                log_msg(LOG_WARNING, "(stanza #%d) Error pulling the GPG signature ID from the context: %s",
+                    stanza_num, fko_gpg_errstr(ctx));
+                if(ctx != NULL)
+                    fko_destroy(ctx);
+                acc = acc->next;
+                continue;
+            }
 
-        if(opts->verbose)
-            log_msg(LOG_INFO, "Incoming SPA data signed by '%s'.", gpg_id);
+            if(opts->verbose)
+                log_msg(LOG_INFO, "(stanza #%d) Incoming SPA data signed by '%s'.",
+                    stanza_num, gpg_id);
 
-        if(acc->gpg_remote_id != NULL && !acc_check_gpg_remote_id(acc, gpg_id))
-        {
-            log_msg(LOG_WARNING,
-                "Incoming SPA packet signed by ID: %s, but that ID is not the GPG_REMOTE_ID list.",
-                gpg_id);
-            goto clean_and_bail;
+            if(acc->gpg_remote_id != NULL && !acc_check_gpg_remote_id(acc, gpg_id))
+            {
+                log_msg(LOG_WARNING,
+                    "(stanza #%d) Incoming SPA packet signed by ID: %s, but that ID is not the GPG_REMOTE_ID list.",
+                    stanza_num, gpg_id);
+                if(ctx != NULL)
+                    fko_destroy(ctx);
+                acc = acc->next;
+                continue;
+            }
         }
-    }
-
-    /* Check for replays if so configured.
-    */
-    if(strncasecmp(opts->config[CONF_ENABLE_DIGEST_PERSISTENCE], "Y", 1) == 0)
-    {
-        res = replay_check(opts, ctx);
-        if(res != 0) /* non-zero means we have seen this packet before. */
-            goto clean_and_bail;
-    }
 
-    /* Populate our spa data struct for future reference.
-    */
-    res = get_spa_data_fields(ctx, &spadat);
+        /* Check for replays if so configured.
+        */
+        if(strncasecmp(opts->config[CONF_ENABLE_DIGEST_PERSISTENCE], "Y", 1) == 0)
+        {
+            res = replay_check(opts, ctx);
+            if(res != 0) /* non-zero means we have seen this packet before. */
+            {
+                if(ctx != NULL)
+                    fko_destroy(ctx);
+                acc = acc->next;
+                continue;
+            }
+        }
 
-    /* Figure out what our timeout will be. If it is specified in the SPA
-     * data, then use that.  If not, try the FW_ACCESS_TIMEOUT from the
-     * access.conf file (if there is one).  Otherwise use the default.
-    */
-    if(spadat.client_timeout > 0)
-        spadat.fw_access_timeout = spadat.client_timeout;
-    else if(acc->fw_access_timeout > 0)
-        spadat.fw_access_timeout = acc->fw_access_timeout;
-    else
-        spadat.fw_access_timeout = DEF_FW_ACCESS_TIMEOUT;
+        /* Populate our spa data struct for future reference.
+        */
+        res = get_spa_data_fields(ctx, &spadat);
 
-    if(res != FKO_SUCCESS)
-    {
-        log_msg(LOG_ERR, "Unexpected error pulling SPA data from the context: %s",
-            fko_errstr(res));
-        res = SPA_MSG_ERROR;
-        goto clean_and_bail;
-    }
+        /* Figure out what our timeout will be. If it is specified in the SPA
+         * data, then use that.  If not, try the FW_ACCESS_TIMEOUT from the
+         * access.conf file (if there is one).  Otherwise use the default.
+        */
+        if(spadat.client_timeout > 0)
+            spadat.fw_access_timeout = spadat.client_timeout;
+        else if(acc->fw_access_timeout > 0)
+            spadat.fw_access_timeout = acc->fw_access_timeout;
+        else
+            spadat.fw_access_timeout = DEF_FW_ACCESS_TIMEOUT;
 
-    /* Check packet age if so configured.
-    */
-    if(strncasecmp(opts->config[CONF_ENABLE_SPA_PACKET_AGING], "Y", 1) == 0)
-    {
-        time(&now_ts);
+        if(res != FKO_SUCCESS)
+        {
+            log_msg(LOG_ERR, "(stanza #%d) Unexpected error pulling SPA data from the context: %s",
+                stanza_num, fko_errstr(res));
 
-        ts_diff = abs(now_ts - spadat.timestamp);
+            if(ctx != NULL)
+                fko_destroy(ctx);
+            acc = acc->next;
+            continue;
+        }
 
-        if(ts_diff > atoi(opts->config[CONF_MAX_SPA_PACKET_AGE]))
+        /* Check packet age if so configured.
+        */
+        if(strncasecmp(opts->config[CONF_ENABLE_SPA_PACKET_AGING], "Y", 1) == 0)
         {
-            log_msg(LOG_WARNING, "SPA data time difference is too great (%i seconds).",
-                ts_diff);
-            res = SPA_MSG_TOO_OLD;
-            goto clean_and_bail;
-        }
-    }
+            time(&now_ts);
 
-    /* At this point, we have enough to check the embedded (or packet source)
-     * IP address against the defined access rights.  We start by splitting
-     * the spa msg source IP from the remainder of the message.
-    */
-    spa_ip_demark = strchr(spadat.spa_message, ',');
-    if(spa_ip_demark == NULL)
-    {
-        log_msg(LOG_WARNING, "Error parsing SPA message string: %s",
-            fko_errstr(res));
-        res = SPA_MSG_ERROR;
-        goto clean_and_bail;
-    }
+            ts_diff = abs(now_ts - spadat.timestamp);
 
-    strlcpy(spadat.spa_message_src_ip, spadat.spa_message, (spa_ip_demark-spadat.spa_message)+1);
-    strlcpy(spadat.spa_message_remain, spa_ip_demark+1, 1024);
+            if(ts_diff > atoi(opts->config[CONF_MAX_SPA_PACKET_AGE]))
+            {
+                log_msg(LOG_WARNING, "(stanza #%d) SPA data time difference is too great (%i seconds).",
+                    stanza_num, ts_diff);
 
-    /* If use source IP was requested (embedded IP of 0.0.0.0), make sure it
-     * is allowed.
-    */
-    if(strcmp(spadat.spa_message_src_ip, "0.0.0.0") == 0)
-    {
-        if(acc->require_source_address)
+                if(ctx != NULL)
+                    fko_destroy(ctx);
+                acc = acc->next;
+                continue;
+            }
+        }
+
+        /* At this point, we have enough to check the embedded (or packet source)
+         * IP address against the defined access rights.  We start by splitting
+         * the spa msg source IP from the remainder of the message.
+        */
+        spa_ip_demark = strchr(spadat.spa_message, ',');
+        if(spa_ip_demark == NULL)
         {
-            log_msg(LOG_WARNING,
-                "Got 0.0.0.0 when valid source IP was required."
-            );
-            res = SPA_MSG_ACCESS_DENIED;
-            goto clean_and_bail;
+            log_msg(LOG_WARNING, "(stanza #%d) Error parsing SPA message string: %s",
+                stanza_num, fko_errstr(res));
+
+            if(ctx != NULL)
+                fko_destroy(ctx);
+            acc = acc->next;
+            continue;
         }
 
-        spadat.use_src_ip = spadat.pkt_source_ip;
-    }
-    else
-        spadat.use_src_ip = spadat.spa_message_src_ip;
+        strlcpy(spadat.spa_message_src_ip, spadat.spa_message, (spa_ip_demark-spadat.spa_message)+1);
+        strlcpy(spadat.spa_message_remain, spa_ip_demark+1, 1024);
 
-    /* If REQUIRE_USERNAME is set, make sure the username in this SPA data
-     * matches.
-    */
-    if(acc->require_username != NULL)
-    {
-        if(strcmp(spadat.username, acc->require_username) != 0)
+        /* If use source IP was requested (embedded IP of 0.0.0.0), make sure it
+         * is allowed.
+        */
+        if(strcmp(spadat.spa_message_src_ip, "0.0.0.0") == 0)
         {
-            log_msg(LOG_WARNING,
-                "Username in SPA data (%s) does not match required username: %s",
-                spadat.username, acc->require_username
-            );
-            res = SPA_MSG_ACCESS_DENIED;
-            goto clean_and_bail;
+            if(acc->require_source_address)
+            {
+                log_msg(LOG_WARNING,
+                    "(stanza #%d) Got 0.0.0.0 when valid source IP was required.",
+                    stanza_num
+                );
+
+                if(ctx != NULL)
+                    fko_destroy(ctx);
+                acc = acc->next;
+                continue;
+            }
+
+            spadat.use_src_ip = spadat.pkt_source_ip;
         }
-    }
+        else
+            spadat.use_src_ip = spadat.spa_message_src_ip;
 
-    /* Take action based on SPA message type.
-    */
-    if(spadat.message_type == FKO_LOCAL_NAT_ACCESS_MSG
-          || spadat.message_type == FKO_CLIENT_TIMEOUT_LOCAL_NAT_ACCESS_MSG
-          || spadat.message_type == FKO_NAT_ACCESS_MSG
-          || spadat.message_type == FKO_CLIENT_TIMEOUT_NAT_ACCESS_MSG)
-    {
-        if(strncasecmp(opts->config[CONF_ENABLE_IPT_FORWARDING], "Y", 1)!=0)
+        /* If REQUIRE_USERNAME is set, make sure the username in this SPA data
+         * matches.
+        */
+        if(acc->require_username != NULL)
         {
-            log_msg(LOG_WARNING,
-                "SPA packet from %s requested NAT access, but is not enabled",
-                spadat.pkt_source_ip
-            );
-            res = SPA_MSG_NAT_NOT_ENABLED;
-            goto clean_and_bail;
+            if(strcmp(spadat.username, acc->require_username) != 0)
+            {
+                log_msg(LOG_WARNING,
+                    "(stanza #%d) Username in SPA data (%s) does not match required username: %s",
+                    stanza_num, spadat.username, acc->require_username
+                );
+
+                if(ctx != NULL)
+                    fko_destroy(ctx);
+                acc = acc->next;
+                continue;
+            }
         }
-    }
 
-    /* Command messages.
-    */
-    if(spadat.message_type == FKO_COMMAND_MSG)
-    {
-        if(!acc->enable_cmd_exec)
+        /* Take action based on SPA message type.
+        */
+        if(spadat.message_type == FKO_LOCAL_NAT_ACCESS_MSG
+              || spadat.message_type == FKO_CLIENT_TIMEOUT_LOCAL_NAT_ACCESS_MSG
+              || spadat.message_type == FKO_NAT_ACCESS_MSG
+              || spadat.message_type == FKO_CLIENT_TIMEOUT_NAT_ACCESS_MSG)
         {
-            log_msg(LOG_WARNING,
-                "SPA Command message are not allowed in the current configuration."
-            );
-            res = SPA_MSG_ACCESS_DENIED;
+            if(strncasecmp(opts->config[CONF_ENABLE_IPT_FORWARDING], "Y", 1)!=0)
+            {
+                log_msg(LOG_WARNING,
+                    "(stanza #%d) SPA packet from %s requested NAT access, but is not enabled",
+                    stanza_num, spadat.pkt_source_ip
+                );
+
+                if(ctx != NULL)
+                    fko_destroy(ctx);
+                acc = acc->next;
+                continue;
+            }
         }
-        else
+
+        /* Command messages.
+        */
+        if(spadat.message_type == FKO_COMMAND_MSG)
         {
-            log_msg(LOG_INFO,
-                "Processing SPA Command message: command='%s'.",
-                spadat.spa_message_remain
-            );
+            if(!acc->enable_cmd_exec)
+            {
+                log_msg(LOG_WARNING,
+                    "(stanza #%d) SPA Command message are not allowed in the current configuration.",
+                    stanza_num
+                );
 
-            /* Do we need to become another user? If so, we call
-             * run_extcmd_as and pass the cmd_exec_uid.
-            */
-            if(acc->cmd_exec_user != NULL && strncasecmp(acc->cmd_exec_user, "root", 4) != 0)
+                if(ctx != NULL)
+                    fko_destroy(ctx);
+                acc = acc->next;
+                continue;
+            }
+            else
             {
-                if(opts->verbose)
-                    log_msg(LOG_INFO, "Setting effective user to %s (UID=%i) before running command.",
-                        acc->cmd_exec_user, acc->cmd_exec_uid);
+                log_msg(LOG_INFO,
+                    "(stanza #%d) Processing SPA Command message: command='%s'.",
+                    stanza_num, spadat.spa_message_remain
+                );
 
+                /* Do we need to become another user? If so, we call
+                 * run_extcmd_as and pass the cmd_exec_uid.
+                */
+                if(acc->cmd_exec_user != NULL && strncasecmp(acc->cmd_exec_user, "root", 4) != 0)
+                {
+                    if(opts->verbose)
+                        log_msg(LOG_INFO, "(stanza #%d) Setting effective user to %s (UID=%i) before running command.",
+                            stanza_num, acc->cmd_exec_user, acc->cmd_exec_uid);
 
-                res = run_extcmd_as(acc->cmd_exec_uid,
-                                    spadat.spa_message_remain, NULL, 0, 0);
-            }
-            else /* Just run it as we are (root that is). */
-                res = run_extcmd(spadat.spa_message_remain, NULL, 0, 5);
 
-            /* --DSS XXX: I have found that the status (and res for that
-             *            matter) have been unreliable indicators of the
-             *            actual exit status of some commands.  Not sure
-             *            why yet.  For now, we will take what we get.
-            */
-            status = WEXITSTATUS(res);
+                    res = run_extcmd_as(acc->cmd_exec_uid,
+                                        spadat.spa_message_remain, NULL, 0, 0);
+                }
+                else /* Just run it as we are (root that is). */
+                    res = run_extcmd(spadat.spa_message_remain, NULL, 0, 5);
 
-            if(opts->verbose > 1)
-                log_msg(LOG_WARNING,
-                    "CMD_EXEC: command returned %i", status);
+                /* --DSS XXX: I have found that the status (and res for that
+                 *            matter) have been unreliable indicators of the
+                 *            actual exit status of some commands.  Not sure
+                 *            why yet.  For now, we will take what we get.
+                */
+                status = WEXITSTATUS(res);
 
-            if(status != 0)
-                res = SPA_MSG_COMMAND_ERROR;
+                if(opts->verbose > 1)
+                    log_msg(LOG_WARNING,
+                        "(stanza #%d) CMD_EXEC: command returned %i",
+                        stanza_num, status);
+
+                if(status != 0)
+                    res = SPA_MSG_COMMAND_ERROR;
+
+                if(ctx != NULL)
+                    fko_destroy(ctx);
+
+                /* we processed the command on a matching access stanza, so we
+                 * don't look for anything else to do with this SPA packet
+                */
+                break;
+            }
+        }
+
+        /* From this point forward, we have some kind of access message. So
+         * we first see if access is allowed by checking access against
+         * restrict_ports and open_ports.
+         *
+         *  --DSS TODO: We should add BLACKLIST support here as well.
+        */
+        if(! acc_check_port_access(acc, spadat.spa_message_remain))
+        {
+            log_msg(LOG_WARNING,
+                "(stanza #%d) One or more requested protocol/ports was denied per access.conf.",
+                stanza_num
+            );
+
+            if(ctx != NULL)
+                fko_destroy(ctx);
+            acc = acc->next;
+            continue;
         }
 
-        goto clean_and_bail;
+        /* At this point, we process the SPA request and break out of the
+         * access stanza loop (first valid access stanza stops us looking
+         * for others).
+        */
+        process_spa_request(opts, &spadat);
+        break;
+
     }
 
-    /* From this point forward, we have some kind of access message. So
-     * we first see if access is allowed by checking access against
-     * restrict_ports and open_ports.
-     *
-     *  --DSS TODO: We should add BLACKLIST support here as well.
-    */
-    if(! acc_check_port_access(acc, spadat.spa_message_remain))
+    if(! found_acc_sip)
     {
         log_msg(LOG_WARNING,
-            "One or more requested protocol/ports was denied per access.conf."
+            "No access data found for source IP: %s", spadat.pkt_source_ip
         );
-
-        res = SPA_MSG_ACCESS_DENIED;
-
-        goto clean_and_bail;
     }
 
-    /* At this point, we can process the SPA request.
-    */
-    res = process_spa_request(opts, &spadat);
-
-clean_and_bail:
-    if(ctx != NULL)
-        fko_destroy(ctx);
-
-    return(res);
+    return;
 }
 
 /***EOF***/
index cae32bf..2ed8ef9 100644 (file)
@@ -33,6 +33,6 @@
 
 /* Prototypes
 */
-int incoming_spa(fko_srv_options_t *opts);
+void incoming_spa(fko_srv_options_t *opts);
 
 #endif  /* INCOMING_SPA_H */
diff --git a/test/conf/expired_epoch_stanza_access.conf b/test/conf/expired_epoch_stanza_access.conf
new file mode 100644 (file)
index 0000000..6f273b8
--- /dev/null
@@ -0,0 +1,4 @@
+SOURCE: ANY;
+KEY: fwknoptest;
+FW_ACCESS_TIMEOUT:  3;
+ACCESS_EXPIRE_EPOCH: 1111111111;   ### very old
diff --git a/test/conf/expired_stanza_access.conf b/test/conf/expired_stanza_access.conf
new file mode 100644 (file)
index 0000000..a928005
--- /dev/null
@@ -0,0 +1,4 @@
+SOURCE: ANY;
+KEY: fwknoptest;
+FW_ACCESS_TIMEOUT:  3;
+ACCESS_EXPIRE: 3/10/1999;   ### very old
diff --git a/test/conf/multi_stanzas_with_broken_keys.conf b/test/conf/multi_stanzas_with_broken_keys.conf
new file mode 100644 (file)
index 0000000..53b391c
--- /dev/null
@@ -0,0 +1,19 @@
+SOURCE: 4.3.2.0/24, 23.43.0.0/16, 10.10.10.10;
+KEY: fwknoptest;
+FW_ACCESS_TIMEOUT:  3;
+
+SOURCE: 23.43.0.0/16, 10.10.10.10;
+KEY: fwknoptest;
+FW_ACCESS_TIMEOUT:  3;
+
+SOURCE: 4.3.2.0/24, 127.0.0.0/24, 23.43.0.0/16, 10.10.10.10;
+KEY: badkey123;
+FW_ACCESS_TIMEOUT:  3;
+
+SOURCE: 4.3.2.0/24, 127.0.0.0/24, 23.43.0.0/16, 10.10.10.10;
+KEY: fwknoptest;
+FW_ACCESS_TIMEOUT:  3;
+
+SOURCE: 4.3.2.0/24, 10.10.10.10;
+KEY: fwknoptest;
+FW_ACCESS_TIMEOUT:  3;
index 333e257..516b5e4 100755 (executable)
@@ -22,12 +22,15 @@ my $gpg_client_home_dir = "$conf_dir/client-gpg";
 my $nat_conf            = "$conf_dir/nat_fwknopd.conf";
 my $default_conf        = "$conf_dir/default_fwknopd.conf";
 my $default_access_conf = "$conf_dir/default_access.conf";
+my $expired_access_conf = "$conf_dir/expired_stanza_access.conf";
+my $expired_epoch_access_conf = "$conf_dir/expired_epoch_stanza_access.conf";
 my $gpg_access_conf     = "$conf_dir/gpg_access.conf";
 my $default_digest_file = "$run_dir/digest.cache";
 my $default_pid_file    = "$run_dir/fwknopd.pid";
 my $open_ports_access_conf = "$conf_dir/open_ports_access.conf";
 my $multi_gpg_access_conf  = "$conf_dir/multi_gpg_access.conf";
 my $multi_stanzas_access_conf = "$conf_dir/multi_stanzas_access.conf";
+my $multi_stanzas_with_broken_keys_conf = "$conf_dir/multi_stanzas_with_broken_keys.conf";
 my $mismatch_open_ports_access_conf = "$conf_dir/mismatch_open_ports_access.conf";
 my $require_user_access_conf = "$conf_dir/require_user_access.conf";
 my $mismatch_user_access_conf = "$conf_dir/mismatch_user_access.conf";
@@ -119,7 +122,8 @@ exit &anonymize_results() if $anonymize_results;
 
 &identify_loopback_intf();
 
-$valgrind_str = "$valgrindCmd --leak-check=full --show-reachable=yes --track-origins=yes" if $use_valgrind;
+$valgrind_str = "$valgrindCmd --leak-check=full " .
+    "--show-reachable=yes --track-origins=yes" if $use_valgrind;
 
 my $intf_str = "-i $loopback_intf --foreground --verbose --verbose";
 
@@ -608,6 +612,34 @@ my @tests = (
         'fw_rule_created' => $REQUIRE_NO_NEW_RULE,
         'fatal'    => $NO
     },
+    {
+        'category' => 'Rijndael SPA',
+        'subcategory' => 'client+server',
+        'detail'   => 'expired stanza (tcp/22 ssh)',
+        'err_msg'  => 'SPA packet accepted',
+        'function' => \&spa_cycle,
+        'cmdline'  => $default_client_args,
+        'fwknopd_cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopdCmd -c $default_conf -a $expired_access_conf " .
+            "-d $default_digest_file -p $default_pid_file $intf_str",
+        'server_positive_output_matches' => [qr/Access\sstanza\shas\sexpired/],
+        'fw_rule_created' => $REQUIRE_NO_NEW_RULE,
+        'fatal'    => $NO
+    },
+    {
+        'category' => 'Rijndael SPA',
+        'subcategory' => 'client+server',
+        'detail'   => 'expired epoch stanza (tcp/22 ssh)',
+        'err_msg'  => 'SPA packet accepted',
+        'function' => \&spa_cycle,
+        'cmdline'  => $default_client_args,
+        'fwknopd_cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopdCmd -c $default_conf -a $expired_epoch_access_conf " .
+            "-d $default_digest_file -p $default_pid_file $intf_str",
+        'server_positive_output_matches' => [qr/Access\sstanza\shas\sexpired/],
+        'fw_rule_created' => $REQUIRE_NO_NEW_RULE,
+        'fatal'    => $NO
+    },
 
     {
         'category' => 'Rijndael SPA',
@@ -798,6 +830,21 @@ my @tests = (
     {
         'category' => 'Rijndael SPA',
         'subcategory' => 'client+server',
+        'detail'   => 'bad/good key stanzas (tcp/22 ssh)',
+        'err_msg'  => "could not complete SPA cycle",
+        'function' => \&spa_cycle,
+        'cmdline'  => $default_client_args,
+        'fwknopd_cmdline'  => "LD_LIBRARY_PATH=$lib_dir $valgrind_str " .
+            "$fwknopdCmd -c $default_conf -a $multi_stanzas_with_broken_keys_conf " .
+            "-d $default_digest_file -p $default_pid_file $intf_str",
+        'fw_rule_created' => $NEW_RULE_REQUIRED,
+        'fw_rule_removed' => $NEW_RULE_REMOVED,
+        'fatal'    => $NO
+    },
+
+    {
+        'category' => 'Rijndael SPA',
+        'subcategory' => 'client+server',
         'detail'   => "non-enabled NAT (tcp/22 ssh)",
         'err_msg'  => "SPA packet not filtered",
         'function' => \&spa_cycle,
@@ -2322,6 +2369,8 @@ sub init() {
             $require_src_access_conf,
             $multi_gpg_access_conf,
             $multi_stanzas_access_conf,
+            $expired_access_conf,
+            $expired_epoch_access_conf,
     ) {
         die "[*] $file does not exist" unless -e $file;
     }