continued changes to zero out sensitive information before exit (#93)
authorMichael Rash <mbr@cipherdyne.org>
Mon, 8 Jul 2013 02:32:30 +0000 (22:32 -0400)
committerMichael Rash <mbr@cipherdyne.org>
Mon, 8 Jul 2013 02:32:30 +0000 (22:32 -0400)
12 files changed:
client/config_init.c
client/fwknop.c
client/getpasswd.c
client/getpasswd.h
client/spa_comm.c
client/utils.c
lib/fko_encryption.c
server/access.c
server/config_init.c
server/fwknopd.c
server/replay_cache.c
server/utils.c

index f12c23b..c6cf8ac 100644 (file)
@@ -612,7 +612,8 @@ set_rc_file(char *rcfile, fko_cli_options_t *options)
      * client consumes a proper rc file with strict permissions set (thanks
      * to Fernando Arnaboldi from IOActive for pointing this out).
     */
-    verify_file_perms_ownership(rcfile);
+    if(verify_file_perms_ownership(rcfile) != 1)
+        exit(EXIT_FAILURE);
 
     return;
 }
index 8073d6a..c79f0f2 100644 (file)
 
 /* prototypes
 */
-static void get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
+static int get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
     char *key, int *key_len, char *hmac_key, int *hmac_key_len);
 static void display_ctx(fko_ctx_t ctx);
 static void errmsg(const char *msg, const int err);
-static void prev_exec(fko_cli_options_t *options, int argc, char **argv);
+static int prev_exec(fko_cli_options_t *options, int argc, char **argv);
 static int get_save_file(char *args_save_file);
-static void show_last_command(const char * const args_save_file);
-static void save_args(int argc, char **argv, const char * const args_save_file);
-static void run_last_args(fko_cli_options_t *options,
+static int show_last_command(const char * const args_save_file);
+static int save_args(int argc, char **argv, const char * const args_save_file);
+static int run_last_args(fko_cli_options_t *options,
         const char * const args_save_file);
 static int set_message_type(fko_ctx_t ctx, fko_cli_options_t *options);
 static int set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options,
         const char * const access_buf);
-static void set_access_buf(fko_ctx_t ctx, fko_cli_options_t *options,
+static int set_access_buf(fko_ctx_t ctx, fko_cli_options_t *options,
         char *access_buf);
 static int get_rand_port(fko_ctx_t ctx);
 int resolve_ip_http(fko_cli_options_t *options);
 static void clean_exit(fko_ctx_t ctx, fko_cli_options_t *opts,
+    char *key, int *key_len, char *hmac_key, int *hmac_key_len,
     unsigned int exit_status);
+static void zero_buf(char *buf, int len);
 static int is_hostname_str_with_port(const char *str, char *hostname, size_t hostname_bufsize, int *port);
 
 #define MAX_CMDLINE_ARGS            50                  /*!< should be way more than enough */
@@ -179,6 +181,7 @@ main(int argc, char **argv)
     char                key[MAX_KEY_LEN+1]       = {0};
     char                hmac_key[MAX_KEY_LEN+1]  = {0};
     int                 key_len = 0, hmac_key_len = 0, enc_mode;
+    int                 tmp_port = 0;
 
     fko_cli_options_t   options;
 
@@ -191,7 +194,13 @@ main(int argc, char **argv)
 
     /* Handle previous execution arguments if required
     */
-    prev_exec(&options, argc, argv);
+    if(prev_exec(&options, argc, argv) != 1)
+        clean_exit(ctx, &options, key, &key_len, hmac_key,
+                &hmac_key_len, EXIT_FAILURE);
+
+    if(options.show_last_command)
+        clean_exit(ctx, &options, key, &key_len, hmac_key,
+                &hmac_key_len, EXIT_SUCCESS);
 
     /* Intialize the context
     */
@@ -199,7 +208,8 @@ main(int argc, char **argv)
     if(res != FKO_SUCCESS)
     {
         errmsg("fko_new", res);
-        return(EXIT_FAILURE);
+        clean_exit(ctx, &options, key, &key_len, hmac_key,
+                &hmac_key_len, EXIT_FAILURE);
     }
 
     /* Display version info and exit.
@@ -211,9 +221,8 @@ main(int argc, char **argv)
         fprintf(stdout, "fwknop client %s, FKO protocol version %s\n",
             MY_VERSION, version);
 
-        fko_destroy(ctx);
-        ctx = NULL;
-        return(EXIT_SUCCESS);
+        clean_exit(ctx, &options, key, &key_len,
+            hmac_key, &hmac_key_len, EXIT_SUCCESS);
     }
 
     /* Set client timeout
@@ -224,9 +233,8 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_spa_client_timeout", res);
-            fko_destroy(ctx);
-            ctx = NULL;
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
     }
 
@@ -236,9 +244,8 @@ main(int argc, char **argv)
     if(res != FKO_SUCCESS)
     {
         errmsg("fko_set_spa_message_type", res);
-        fko_destroy(ctx);
-        ctx = NULL;
-        return(EXIT_FAILURE);
+        clean_exit(ctx, &options, key, &key_len,
+            hmac_key, &hmac_key_len, EXIT_FAILURE);
     }
 
     /* Adjust the SPA timestamp if necessary
@@ -249,9 +256,8 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_timestamp", res);
-            fko_destroy(ctx);
-            ctx = NULL;
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
     }
     if(options.time_offset_minus > 0)
@@ -260,9 +266,8 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_timestamp", res);
-            fko_destroy(ctx);
-            ctx = NULL;
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
     }
 
@@ -283,9 +288,8 @@ main(int argc, char **argv)
         {
             if(resolve_ip_http(&options) < 0)
             {
-                fko_destroy(ctx);
-                ctx = NULL;
-                return(EXIT_FAILURE);
+                clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
             }
         }
 
@@ -294,15 +298,16 @@ main(int argc, char **argv)
          * to be specified as well, so in this case append the string
          * "none/0" to the allow IP.
         */
-        set_access_buf(ctx, &options, access_buf);
+        if(set_access_buf(ctx, &options, access_buf) != 1)
+            clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
     }
     res = fko_set_spa_message(ctx, access_buf);
     if(res != FKO_SUCCESS)
     {
         errmsg("fko_set_spa_message", res);
-        fko_destroy(ctx);
-        ctx = NULL;
-        return(EXIT_FAILURE);
+        clean_exit(ctx, &options, key, &key_len,
+            hmac_key, &hmac_key_len, EXIT_FAILURE);
     }
 
     /* Set NAT access string
@@ -313,9 +318,8 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_nat_access_str", res);
-            fko_destroy(ctx);
-            ctx = NULL;
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
     }
 
@@ -327,9 +331,8 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_username", res);
-            fko_destroy(ctx);
-            ctx = NULL;
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
     }
 
@@ -349,9 +352,8 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_spa_encryption_type", res);
-            fko_destroy(ctx);
-            ctx = NULL;
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
 
         /* If a GPG home dir was specified, set it here.  Note: Setting
@@ -364,9 +366,8 @@ main(int argc, char **argv)
             if(res != FKO_SUCCESS)
             {
                 errmsg("fko_set_gpg_home_dir", res);
-                fko_destroy(ctx);
-                ctx = NULL;
-                return(EXIT_FAILURE);
+                clean_exit(ctx, &options, key, &key_len,
+                        hmac_key, &hmac_key_len, EXIT_FAILURE);
             }
         }
 
@@ -377,9 +378,8 @@ main(int argc, char **argv)
 
             if(IS_GPG_ERROR(res))
                 log_msg(LOG_VERBOSITY_ERROR, "GPG ERR: %s", fko_gpg_errstr(ctx));
-            fko_destroy(ctx);
-            ctx = NULL;
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
 
         if(strlen(options.gpg_signer_key) > 0)
@@ -391,10 +391,8 @@ main(int argc, char **argv)
 
                 if(IS_GPG_ERROR(res))
                     log_msg(LOG_VERBOSITY_ERROR, "GPG ERR: %s", fko_gpg_errstr(ctx));
-
-                fko_destroy(ctx);
-                ctx = NULL;
-                return(EXIT_FAILURE);
+                clean_exit(ctx, &options, key, &key_len,
+                        hmac_key, &hmac_key_len, EXIT_FAILURE);
             }
         }
 
@@ -402,7 +400,8 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_spa_encryption_mode", res);
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
     }
 
@@ -412,7 +411,8 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_spa_encryption_mode", res);
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
     }
 
@@ -424,15 +424,16 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_spa_digest_type", res);
-            fko_destroy(ctx);
-            ctx = NULL;
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
     }
 
     /* Acquire the necessary encryption/hmac keys
     */
-    get_keys(ctx, &options, key, &key_len, hmac_key, &hmac_key_len);
+    if(get_keys(ctx, &options, key, &key_len, hmac_key, &hmac_key_len) != 1)
+        clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
 
     /* Finalize the context data (encrypt and encode the SPA data)
     */
@@ -443,8 +444,8 @@ main(int argc, char **argv)
 
         if(IS_GPG_ERROR(res))
             log_msg(LOG_VERBOSITY_ERROR, "GPG ERR: %s", fko_gpg_errstr(ctx));
-
-        clean_exit(ctx, &options, EXIT_FAILURE);
+        clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
     }
 
     /* Display the context data.
@@ -458,15 +459,20 @@ main(int argc, char **argv)
         write_spa_packet_data(ctx, &options);
 
     if (options.rand_port)
-        options.spa_dst_port = get_rand_port(ctx);
+    {
+        tmp_port = get_rand_port(ctx);
+        if(tmp_port < 0)
+            clean_exit(ctx, &options, key, &key_len,
+                    hmac_key, &hmac_key_len, EXIT_FAILURE);
+        options.spa_dst_port = tmp_port;
+    }
 
     res = send_spa_packet(ctx, &options);
     if(res < 0)
     {
         log_msg(LOG_VERBOSITY_ERROR, "send_spa_packet: packet not sent.");
-        fko_destroy(ctx);
-        ctx = NULL;
-        return(EXIT_FAILURE);
+        clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
     }
     else
     {
@@ -486,9 +492,8 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_get_spa_data", res);
-            fko_destroy(ctx);
-            ctx = NULL;
-            return(EXIT_FAILURE);
+            clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
 
         /* Pull the encryption mode.
@@ -497,10 +502,10 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_get_spa_encryption_mode", res);
-            fko_destroy(ctx);
             fko_destroy(ctx2);
-            ctx = ctx2 = NULL;
-            return(EXIT_FAILURE);
+            ctx2 = NULL;
+            clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
 
         /* If gpg-home-dir is specified, we have to defer decrypting if we
@@ -519,20 +524,20 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_new_with_data", res);
-            fko_destroy(ctx);
             fko_destroy(ctx2);
-            ctx = ctx2 = NULL;
-            return(EXIT_FAILURE);
+            ctx2 = NULL;
+            clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
 
         res = fko_set_spa_encryption_mode(ctx2, enc_mode);
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_spa_encryption_mode", res);
-            fko_destroy(ctx);
             fko_destroy(ctx2);
-            ctx = ctx2 = NULL;
-            return(EXIT_FAILURE);
+            ctx2 = NULL;
+            clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
 
         /* See if we are using gpg and if we need to set the GPG home dir.
@@ -545,10 +550,10 @@ main(int argc, char **argv)
                 if(res != FKO_SUCCESS)
                 {
                     errmsg("fko_set_gpg_home_dir", res);
-                    fko_destroy(ctx);
                     fko_destroy(ctx2);
-                    ctx = ctx2 = NULL;
-                    return(EXIT_FAILURE);
+                    ctx2 = NULL;
+                    clean_exit(ctx, &options, key, &key_len,
+                        hmac_key, &hmac_key_len, EXIT_FAILURE);
                 }
             }
         }
@@ -571,16 +576,12 @@ main(int argc, char **argv)
                  debugging purposes. */
                 log_msg(LOG_VERBOSITY_ERROR, "GPG ERR: %s\n%s", fko_gpg_errstr(ctx2),
                     "No access to recipient private key?");
-                fko_destroy(ctx);
-                fko_destroy(ctx2);
-                ctx = ctx2 = NULL;
-                return(EXIT_SUCCESS);
             }
 
-            fko_destroy(ctx);
             fko_destroy(ctx2);
-            ctx = ctx2 = NULL;
-            return(EXIT_FAILURE);
+            ctx2 = NULL;
+            clean_exit(ctx, &options, key, &key_len,
+                hmac_key, &hmac_key_len, EXIT_FAILURE);
         }
 
         log_msg(LOG_VERBOSITY_NORMAL,"\nDump of the Decoded Data");
@@ -590,9 +591,10 @@ main(int argc, char **argv)
         ctx2 = NULL;
     }
 
-    clean_exit(ctx, &options, EXIT_SUCCESS);
+    clean_exit(ctx, &options, key, &key_len,
+            hmac_key, &hmac_key_len, EXIT_SUCCESS);
 
-    return(EXIT_SUCCESS);
+    return EXIT_SUCCESS;  /* quiet down a gcc warning */
 }
 
 void
@@ -600,6 +602,14 @@ free_configs(fko_cli_options_t *opts)
 {
     if (opts->resolve_url != NULL)
         free(opts->resolve_url);
+    zero_buf(opts->key, MAX_KEY_LEN+1);
+    zero_buf(opts->key_base64, MAX_B64_KEY_LEN+1);
+    zero_buf(opts->hmac_key, MAX_KEY_LEN+1);
+    zero_buf(opts->hmac_key_base64, MAX_B64_KEY_LEN+1);
+    zero_buf(opts->gpg_recipient_key, MAX_GPG_KEY_ID);
+    zero_buf(opts->gpg_signer_key, MAX_GPG_KEY_ID);
+    zero_buf(opts->gpg_home_dir, MAX_PATH_LEN);
+    zero_buf(opts->server_command, MAX_LINE_LEN);
 }
 
 static int
@@ -615,9 +625,7 @@ get_rand_port(fko_ctx_t ctx)
     if(res != FKO_SUCCESS)
     {
         errmsg("get_rand_port(), fko_get_rand_value", res);
-        fko_destroy(ctx);
-        ctx = NULL;
-        exit(EXIT_FAILURE);
+        return -1;
     }
 
     strlcpy(port_str, rand_val, sizeof(port_str));
@@ -628,9 +636,7 @@ get_rand_port(fko_ctx_t ctx)
         log_msg(LOG_VERBOSITY_ERROR,
             "[*] get_rand_port(), could not convert rand_val str '%s', to integer",
             rand_val);
-        fko_destroy(ctx);
-        ctx = NULL;
-        exit(EXIT_FAILURE);
+        return -1;
     }
 
     /* Convert to a random value between 1024 and 65535
@@ -645,9 +651,7 @@ get_rand_port(fko_ctx_t ctx)
     if(res != FKO_SUCCESS)
     {
         errmsg("get_rand_port(), fko_get_rand_value", res);
-        fko_destroy(ctx);
-        ctx = NULL;
-        exit(EXIT_FAILURE);
+        return -1;
     }
 
     return port;
@@ -683,7 +687,7 @@ ipv4_str_has_port(char *str)
 
 /* Set access buf
 */
-static void
+static int
 set_access_buf(fko_ctx_t ctx, fko_cli_options_t *options, char *access_buf)
 {
     char   *ndx = NULL, tmp_nat_port[MAX_PORT_STR_LEN+1] = {0};
@@ -714,7 +718,7 @@ set_access_buf(fko_ctx_t ctx, fko_cli_options_t *options, char *access_buf)
             if(ndx == NULL)
             {
                 log_msg(LOG_VERBOSITY_ERROR, "[*] Expecting <proto>/<port> for -A arg.");
-                clean_exit(ctx, options, EXIT_FAILURE);
+                return 0;
             }
             snprintf(access_buf, MAX_LINE_LEN, "%s%s",
                     options->allow_ip_str, ",");
@@ -728,7 +732,7 @@ set_access_buf(fko_ctx_t ctx, fko_cli_options_t *options, char *access_buf)
             {
                 log_msg(LOG_VERBOSITY_ERROR,
                         "[*] NAT for multiple ports/protocols not yet supported.");
-                clean_exit(ctx, options, EXIT_FAILURE);
+                return 0;
             }
 
             /* Now add the NAT port
@@ -748,7 +752,7 @@ set_access_buf(fko_ctx_t ctx, fko_cli_options_t *options, char *access_buf)
         snprintf(access_buf, MAX_LINE_LEN, "%s%s%s",
                 options->allow_ip_str, ",", "none/0");
     }
-    return;
+    return 1;
 }
 
 /* Set NAT access string
@@ -770,7 +774,7 @@ set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options, const char * const acc
     if(ndx == NULL)
     {
         log_msg(LOG_VERBOSITY_ERROR, "[*] Expecting <proto>/<port> for -A arg.");
-        clean_exit(ctx, options, EXIT_FAILURE);
+        return FKO_ERROR_INVALID_DATA;
     }
     ndx++;
 
@@ -788,7 +792,7 @@ set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options, const char * const acc
     {
         log_msg(LOG_VERBOSITY_ERROR, "[*] Invalid port value '%d' for -A arg.",
                 access_port);
-        clean_exit(ctx, options, EXIT_FAILURE);
+        return FKO_ERROR_INVALID_DATA;
     }
 
     if (options->nat_local && options->nat_access_str[0] == 0x0)
@@ -824,7 +828,7 @@ set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options, const char * const acc
         {
             log_msg(LOG_VERBOSITY_ERROR, "[*] Unable to resolve %s as an ip address",
                     hostname);
-            clean_exit(ctx, options, EXIT_FAILURE);
+            return FKO_ERROR_INVALID_DATA;
         }
 
         snprintf(nat_access_buf, MAX_LINE_LEN, NAT_ACCESS_STR_TEMPLATE,
@@ -848,10 +852,11 @@ set_nat_access(fko_ctx_t ctx, fko_cli_options_t *options, const char * const acc
     return fko_set_spa_nat_access(ctx, nat_access_buf);
 }
 
-static void
+static int
 prev_exec(fko_cli_options_t *options, int argc, char **argv)
 {
     char       args_save_file[MAX_PATH_LEN] = {0};
+    int        res = 1;
 
     if(options->args_save_file[0] != 0x0)
     {
@@ -862,33 +867,35 @@ prev_exec(fko_cli_options_t *options, int argc, char **argv)
         if (get_save_file(args_save_file) != 1)
         {
             log_msg(LOG_VERBOSITY_ERROR, "Unable to determine args save file");
-            exit(EXIT_FAILURE);
+            return 0;
         }
     }
 
     if(options->run_last_command)
-        run_last_args(options, args_save_file);
+        res = run_last_args(options, args_save_file);
     else if(options->show_last_command)
-        show_last_command(args_save_file);
+        res = show_last_command(args_save_file);
     else if (!options->no_save_args)
-        save_args(argc, argv, args_save_file);
+        res = save_args(argc, argv, args_save_file);
 
-    return;
+    return res;
 }
 
 /* Show the last command that was executed
 */
-static void
+static int
 show_last_command(const char * const args_save_file)
 {
     char args_str[MAX_LINE_LEN] = {0};
     FILE *args_file_ptr = NULL;
 
-    verify_file_perms_ownership(args_save_file);
+    if(verify_file_perms_ownership(args_save_file) != 1)
+        return 0;
+
     if ((args_file_ptr = fopen(args_save_file, "r")) == NULL) {
         log_msg(LOG_VERBOSITY_ERROR, "Could not open args file: %s",
             args_save_file);
-        exit(EXIT_FAILURE);
+        return 0;
     }
 
     if ((fgets(args_str, MAX_LINE_LEN, args_file_ptr)) != NULL) {
@@ -898,12 +905,12 @@ show_last_command(const char * const args_save_file)
     }
     fclose(args_file_ptr);
 
-    exit(EXIT_SUCCESS);
+    return 1;
 }
 
 /* Get the command line arguments from the previous invocation
 */
-static void
+static int
 run_last_args(fko_cli_options_t *options, const char * const args_save_file)
 {
     FILE           *args_file_ptr = NULL;
@@ -916,12 +923,14 @@ run_last_args(fko_cli_options_t *options, const char * const args_save_file)
     char            arg_tmp[MAX_LINE_LEN]  = {0};
     char           *argv_new[MAX_CMDLINE_ARGS];  /* should be way more than enough */
 
-    verify_file_perms_ownership(args_save_file);
+    if(verify_file_perms_ownership(args_save_file) != 1)
+        return 0;
+
     if ((args_file_ptr = fopen(args_save_file, "r")) == NULL)
     {
         log_msg(LOG_VERBOSITY_ERROR, "Could not open args file: %s",
                 args_save_file);
-        exit(EXIT_FAILURE);
+        return 0;
     }
     if ((fgets(args_str, MAX_LINE_LEN, args_file_ptr)) != NULL)
     {
@@ -942,7 +951,8 @@ run_last_args(fko_cli_options_t *options, const char * const args_save_file)
                 if (argv_new[argc_new] == NULL)
                 {
                     log_msg(LOG_VERBOSITY_ERROR, "[*] malloc failure for cmd line arg.");
-                    exit(EXIT_FAILURE);
+                    fclose(args_file_ptr);
+                    return 0;
                 }
                 strlcpy(argv_new[argc_new], arg_tmp, strlen(arg_tmp)+1);
                 current_arg_ctr = 0;
@@ -950,7 +960,8 @@ run_last_args(fko_cli_options_t *options, const char * const args_save_file)
                 if(argc_new >= MAX_CMDLINE_ARGS)
                 {
                     log_msg(LOG_VERBOSITY_ERROR, "[*] max command line args exceeded.");
-                    exit(EXIT_FAILURE);
+                    fclose(args_file_ptr);
+                    return 0;
                 }
             }
         }
@@ -973,7 +984,7 @@ run_last_args(fko_cli_options_t *options, const char * const args_save_file)
             free(argv_new[i]);
     }
 
-    return;
+    return 1;
 }
 
 static int
@@ -998,7 +1009,7 @@ get_save_file(char *args_save_file)
 
 /* Save our command line arguments
 */
-static void
+static int
 save_args(int argc, char **argv, const char * const args_save_file)
 {
     char args_str[MAX_LINE_LEN] = {0};
@@ -1008,14 +1019,15 @@ save_args(int argc, char **argv, const char * const args_save_file)
     if (args_file_fd == -1) {
         log_msg(LOG_VERBOSITY_ERROR, "Could not open args file: %s",
             args_save_file);
-        exit(EXIT_FAILURE);
+        return 0;
     }
     else {
         for (i=0; i < argc; i++) {
             args_str_len += strlen(argv[i]);
             if (args_str_len >= MAX_PATH_LEN) {
                 log_msg(LOG_VERBOSITY_ERROR, "argument string too long, exiting.");
-                exit(EXIT_FAILURE);
+                close(args_file_fd);
+                return 0;
             }
             strlcat(args_str, argv[i], sizeof(args_str));
             strlcat(args_str, " ", sizeof(args_str));
@@ -1028,7 +1040,7 @@ save_args(int argc, char **argv, const char * const args_save_file)
         }
         close(args_file_fd);
     }
-    return;
+    return 1;
 }
 
 /* Set the SPA packet message type
@@ -1069,7 +1081,7 @@ set_message_type(fko_ctx_t ctx, fko_cli_options_t *options)
 
 /* Prompt for and receive a user password.
 */
-static void
+static int
 get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
     char *key, int *key_len, char *hmac_key, int *hmac_key_len)
 {
@@ -1096,7 +1108,7 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
         {
             log_msg(LOG_VERBOSITY_ERROR, "[*] Invalid key length: '%d', must be in [1,%d]",
                     *key_len, MAX_KEY_LEN);
-            clean_exit(ctx, options, EXIT_FAILURE);
+            return 0;
         }
     }
     else
@@ -1105,7 +1117,10 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
         */
         if(options->get_key_file[0] != 0x0)
         {
-            get_key_file(key, key_len, options->get_key_file, ctx, options);
+            if(get_key_file(key, key_len, options->get_key_file, ctx, options) != 1)
+            {
+                return 0;
+            }
         }
         else if(options->use_gpg)
         {
@@ -1121,7 +1136,7 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
                 if(key_tmp == NULL)
                 {
                     log_msg(LOG_VERBOSITY_ERROR, "[*] getpasswd() key error.");
-                    clean_exit(ctx, options, EXIT_FAILURE);
+                    return 0;
                 }
                 strlcpy(key, key_tmp, MAX_KEY_LEN+1);
                 *key_len = strlen(key);
@@ -1134,7 +1149,7 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
             if(key_tmp == NULL)
             {
                 log_msg(LOG_VERBOSITY_ERROR, "[*] getpasswd() key error.");
-                clean_exit(ctx, options, EXIT_FAILURE);
+                return 0;
             }
             strlcpy(key, key_tmp, MAX_KEY_LEN+1);
             *key_len = strlen(key);
@@ -1156,7 +1171,7 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
             log_msg(LOG_VERBOSITY_ERROR,
                     "[*] Invalid decoded key length: '%d', must be in [0,%d]",
                     *hmac_key_len, MAX_KEY_LEN);
-            clean_exit(ctx, options, EXIT_FAILURE);
+            return 0;
         }
         memcpy(hmac_key, options->hmac_key, *hmac_key_len);
         use_hmac = 1;
@@ -1167,8 +1182,11 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
         */
         if(options->get_hmac_key_file[0] != 0x0)
         {
-            get_key_file(hmac_key, hmac_key_len,
-                options->get_hmac_key_file, ctx, options);
+            if(get_key_file(hmac_key, hmac_key_len,
+                options->get_hmac_key_file, ctx, options) != 1)
+            {
+                return 0;
+            }
             use_hmac = 1;
         }
         else
@@ -1178,7 +1196,7 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
             if(hmac_key_tmp == NULL)
             {
                 log_msg(LOG_VERBOSITY_ERROR, "[*] getpasswd() key error.");
-                clean_exit(ctx, options, EXIT_FAILURE);
+                return 0;
             }
 
             strlcpy(hmac_key, hmac_key_tmp, MAX_KEY_LEN+1);
@@ -1193,7 +1211,7 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
         {
             log_msg(LOG_VERBOSITY_ERROR, "[*] Invalid HMAC key length: '%d', must be in [0,%d]",
                     *hmac_key_len, MAX_KEY_LEN);
-            clean_exit(ctx, options, EXIT_FAILURE);
+            return 0;
         }
 
         /* Make sure the same key is not used for both encryption and the HMAC
@@ -1204,7 +1222,7 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
             {
                 log_msg(LOG_VERBOSITY_ERROR,
                     "[*] The encryption passphrase and HMAC key should not be identical, no SPA packet sent. Exiting.");
-                clean_exit(ctx, options, EXIT_FAILURE);
+                return 0;
             }
         }
 
@@ -1212,11 +1230,11 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options,
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_spa_hmac_type", res);
-            exit(EXIT_FAILURE);
+            return 0;
         }
     }
 
-    return;
+    return 1;
 }
 
 /* Display an FKO error message.
@@ -1227,14 +1245,41 @@ errmsg(const char *msg, const int err) {
         MY_NAME, msg, err, fko_errstr(err));
 }
 
+/* zero out sensitive information in a way that isn't optimized out by the compiler
+ * since we force a comparision
+*/
+static void
+zero_buf(char *buf, int len)
+{
+    int i;
+
+    if(len <= 0)
+        return;
+
+    memset(buf, 0x0, len);
+
+    for(i=0; i < len; i++)
+        if(buf[i] != 0x0)
+            log_msg(LOG_VERBOSITY_ERROR,
+                    "[*] Could not zero out sensitive data buffer.");
+
+    return;
+}
+
 /* free up memory and exit
 */
 static void
-clean_exit(fko_ctx_t ctx, fko_cli_options_t *opts, unsigned int exit_status)
+clean_exit(fko_ctx_t ctx, fko_cli_options_t *opts,
+        char *key, int *key_len, char *hmac_key, int *hmac_key_len,
+        unsigned int exit_status)
 {
     free_configs(opts);
     fko_destroy(ctx);
     ctx = NULL;
+    zero_buf(key, *key_len);
+    zero_buf(hmac_key, *hmac_key_len);
+    *key_len = 0;
+    *hmac_key_len = 0;
     exit(exit_status);
 }
 
index a31ee35..827bb30 100644 (file)
@@ -128,7 +128,7 @@ char*
 getpasswd(const char *prompt, int fd)
 {
     char *ptr;
-    
+
 #ifndef WIN32
     sigset_t        sig, old_sig;
     struct termios  ts, old_ts;
@@ -143,7 +143,7 @@ getpasswd(const char *prompt, int fd)
             log_msg(LOG_VERBOSITY_ERROR, "getpasswd() - "
                 "Unable to create a stream from the file descriptor : %s",
                 strerror(errno));
-            exit(EXIT_FAILURE);
+            return(NULL);
         }
     }
 
@@ -211,7 +211,7 @@ getpasswd(const char *prompt, int fd)
 
 /* Function for accepting password input from a file
 */
-void
+int
 get_key_file(char *key, int *key_len, const char *key_file,
     fko_ctx_t ctx, const fko_cli_options_t *options)
 {
@@ -227,9 +227,7 @@ get_key_file(char *key, int *key_len, const char *key_file,
     if ((pwfile_ptr = fopen(key_file, "r")) == NULL)
     {
         log_msg(LOG_VERBOSITY_ERROR, "Could not open config file: %s", key_file);
-        fko_destroy(ctx);
-        ctx = NULL;
-        exit(EXIT_FAILURE);
+        return 0;
     }
 
     while ((fgets(conf_line_buf, MAX_LINE_LEN, pwfile_ptr)) != NULL)
@@ -284,14 +282,12 @@ get_key_file(char *key, int *key_len, const char *key_file,
     if (key[0] == '\0') {
         log_msg(LOG_VERBOSITY_ERROR, "Could not get key for IP: %s from: %s",
             options->spa_server_str, key_file);
-        fko_destroy(ctx);
-        ctx = NULL;
-        exit(EXIT_FAILURE);
+        return 0;
     }
 
     *key_len = strlen(key);
 
-    return;
+    return 1;
 }
 
 /***EOF***/
index 2f95c0a..ec68d8b 100644 (file)
@@ -37,7 +37,7 @@ char* getpasswd(const char *prompt, int fd);
 
 /* This can be used to acquire an encryption key or HMAC key
 */
-void get_key_file(char *key, int *key_len, const char *key_file,
+int get_key_file(char *key, int *key_len, const char *key_file,
     fko_ctx_t ctx, const fko_cli_options_t *options);
 
 #endif  /* GETPASSWD_H */
index 638ed81..b567d66 100644 (file)
@@ -125,7 +125,7 @@ send_spa_packet_tcp_or_udp(const char *spa_data, const int sd_len,
     if (error != 0)
     {
         log_msg(LOG_VERBOSITY_ERROR, "error in getaddrinfo: %s", gai_strerror(error));
-        exit(EXIT_FAILURE);
+        return -1;
     }
 
     for (rp = result; rp != NULL; rp = rp->ai_next) {
@@ -145,8 +145,10 @@ send_spa_packet_tcp_or_udp(const char *spa_data, const int sd_len,
     }
 
     if (rp == NULL) {
-        log_msg(LOG_VERBOSITY_ERROR, "send_spa_packet_tcp_or_udp: Could not create socket: ", strerror(errno));
-        exit(EXIT_FAILURE);
+        log_msg(LOG_VERBOSITY_ERROR,
+                "send_spa_packet_tcp_or_udp: Could not create socket: ",
+                strerror(errno));
+        return -1;
     }
 
     freeaddrinfo(result);
@@ -503,7 +505,7 @@ send_spa_packet_http(const char *spa_data, const int sd_len,
     if (spa_data_copy == NULL)
     {
         log_msg(LOG_VERBOSITY_ERROR, "[*] Fatal, could not allocate memory.");
-        exit(EXIT_FAILURE);
+        return -1;
     }
     memcpy(spa_data_copy, spa_data, sd_len+1);
 
@@ -539,7 +541,7 @@ send_spa_packet_http(const char *spa_data, const int sd_len,
             memmove(ndx, ndx+7, strlen(ndx)+1);
 
         /* If there is a colon assume the proxy hostame or IP is on the left
-         * and the proxy port is on the right. So we make the : a \0 and 
+         * and the proxy port is on the right. So we make the : a \0 and
          * extract the port value.
         */
         ndx = strchr(options->http_proxy, ':');
@@ -553,7 +555,7 @@ send_spa_packet_http(const char *spa_data, const int sd_len,
                     "[-] proxy port value is invalid, must be in [%d-%d]",
                     1, MAX_PORT);
                 free(spa_data_copy);
-                return 0;
+                return -1;
             }
         }
 
@@ -601,7 +603,6 @@ send_spa_packet(fko_ctx_t ctx, fko_cli_options_t *options)
     WSADATA wsa_data;
 #endif
 
-
     /* Initialize the hint buffer */
     memset(&hints, 0 , sizeof(hints));
 
@@ -667,8 +668,8 @@ send_spa_packet(fko_ctx_t ctx, fko_cli_options_t *options)
 
         if (saddr.sin_addr.s_addr == -1)
         {
-            fprintf(stderr, "Could not set source IP.\n");
-            exit(EXIT_FAILURE);
+            log_msg(LOG_VERBOSITY_ERROR, "Could not set source IP.");
+            return -1;
         }
 
         /* Set destination port
@@ -683,7 +684,7 @@ send_spa_packet(fko_ctx_t ctx, fko_cli_options_t *options)
         {
             log_msg(LOG_VERBOSITY_ERROR, "[*] Unable to resolve %s as an ip address",
                     options->spa_server_str);
-            exit(EXIT_FAILURE);
+            return -1;
         }
         else;
 
index 69cfd1e..1dce38c 100644 (file)
@@ -118,53 +118,62 @@ int
 verify_file_perms_ownership(const char *file)
 {
     int res = 1;
+
 #if HAVE_STAT
     struct stat st;
 
     /* Every file that the fwknop client deals with should be owned
      * by the user and permissions set to 600 (user read/write)
     */
-    if((stat(file, &st)) != 0)
+    if((stat(file, &st)) == 0)
     {
-        /* if the path doesn't exist, just return, but otherwise something
-         * went wrong
+        /* Make sure it is a regular file
         */
-        if(errno == ENOENT)
+        if(S_ISREG(st.st_mode) != 1 && S_ISLNK(st.st_mode) != 1)
         {
-            return 0;
-        } else {
-            log_msg(LOG_VERBOSITY_ERROR, "[-] stat() against file: %s returned: %s",
-                file, strerror(errno));
-            exit(EXIT_FAILURE);
+            log_msg(LOG_VERBOSITY_ERROR,
+                "[-] file: %s is not a regular file or symbolic link.",
+                file
+            );
+            /* when we start in enforcing this instead of just warning
+             * the user
+            res = 0;
+            */
         }
-    }
 
-    /* Make sure it is a regular file or symbolic link
-    */
-    if(S_ISREG(st.st_mode) != 1 && S_ISLNK(st.st_mode) != 1)
-    {
-        log_msg(LOG_VERBOSITY_WARNING,
-            "[-] file: %s is not a regular file or symbolic link.",
-            file
-        );
-        res = 0;
-    }
+        if((st.st_mode & (S_IRWXU|S_IRWXG|S_IRWXO)) != (S_IRUSR|S_IWUSR))
+        {
+            log_msg(LOG_VERBOSITY_ERROR,
+                "[-] file: %s permissions should only be user read/write (0600, -rw-------)",
+                file
+            );
+            /* when we start in enforcing this instead of just warning
+             * the user
+            res = 0;
+            */
+        }
 
-    if((st.st_mode & (S_IRWXU|S_IRWXG|S_IRWXO)) != (S_IRUSR|S_IWUSR))
-    {
-        log_msg(LOG_VERBOSITY_WARNING,
-            "[-] file: %s permissions should only be user read/write (0600, -rw-------)",
-            file
-        );
-        res = 0;
+        if(st.st_uid != getuid())
+        {
+            log_msg(LOG_VERBOSITY_ERROR, "[-] file: %s not owned by current effective user id",
+                file);
+            /* when we start in enforcing this instead of just warning
+             * the user
+            res = 0;
+            */
+        }
     }
-
-    if(st.st_uid != getuid())
+    else
     {
-        log_msg(LOG_VERBOSITY_WARNING,
-            "[-] file: %s not owned by current effective user id.",
-            file);
-        res = 0;
+        /* if the path doesn't exist, just return, but otherwise something
+         * went wrong
+        */
+        if(errno != ENOENT)
+        {
+            log_msg(LOG_VERBOSITY_ERROR, "[-] stat() against file: %s returned: %s",
+                file, strerror(errno));
+            res = 0;
+        }
     }
 #endif
 
index 4235b2b..5308079 100644 (file)
@@ -84,7 +84,6 @@ _rijndael_encrypt(fko_ctx_t ctx, const char *enc_key, const int enc_key_len)
     if(plaintext == NULL)
         return(FKO_ERROR_MEMORY_ALLOCATION);
 
-
     pt_len = snprintf(plaintext, pt_len, "%s:%s", ctx->encoded_msg, ctx->digest);
 
     if(! is_valid_pt_msg_len(pt_len))
index 78be516..94bacb1 100644 (file)
@@ -686,7 +686,7 @@ free_acc_stanza_data(acc_stanza_t *acc)
 
     if(acc->key_base64 != NULL)
     {
-        zero_key(acc->key, strlen(acc->key_base64));
+        zero_key(acc->key_base64, strlen(acc->key_base64));
         free(acc->key_base64);
     }
 
@@ -981,7 +981,8 @@ parse_access_file(fko_srv_options_t *opts)
         clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
     }
 
-    verify_file_perms_ownership(opts->config[CONF_ACCESS_FILE]);
+    if(verify_file_perms_ownership(opts->config[CONF_ACCESS_FILE]) != 1)
+        clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
 
     /* A note on security here: Coverity flags the following fopen() as a
      * Time of check time of use (TOCTOU) bug with a low priority due to the
index 16b162a..6fd358f 100644 (file)
@@ -217,7 +217,8 @@ parse_config_file(fko_srv_options_t *opts, const char *config_file)
         clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
     }
 
-    verify_file_perms_ownership(config_file);
+    if(verify_file_perms_ownership(config_file) != 1)
+        clean_exit(opts, NO_FW_CLEANUP, EXIT_FAILURE);
 
     /* See the comment in the parse_access_file() function regarding security
      * here relative to a TOCTOU bug flagged by Coverity.
index c600b39..caeada0 100644 (file)
@@ -697,7 +697,11 @@ get_running_pid(const fko_srv_options_t *opts)
     pid_t   rpid            = 0;
 
 
-    verify_file_perms_ownership(opts->config[CONF_FWKNOP_PID_FILE]);
+    if(verify_file_perms_ownership(opts->config[CONF_FWKNOP_PID_FILE]) != 1)
+    {
+        fprintf(stderr, "verify_file_perms_ownership() error\n");
+        return(rpid);
+    }
 
     op_fd = open(opts->config[CONF_FWKNOP_PID_FILE], O_RDONLY);
 
index c9c8d16..255adcc 100644 (file)
@@ -285,7 +285,8 @@ replay_file_cache_init(fko_srv_options_t *opts)
         }
     }
 
-    verify_file_perms_ownership(opts->config[CONF_DIGEST_FILE]);
+    if(verify_file_perms_ownership(opts->config[CONF_DIGEST_FILE]) != 1)
+        return(-1);
 
     /* File exists, and we have access - create in-memory digest cache
     */
index 3a805e9..1dcbff2 100644 (file)
@@ -202,7 +202,7 @@ is_valid_dir(const char *path)
     {
         log_msg(LOG_ERR, "[-] unable to stat() directory: %s: %s",
             path, strerror(errno));
-        exit(EXIT_FAILURE);
+        return(0);
     }
 
     if(!S_ISDIR(st.st_mode))
@@ -216,53 +216,64 @@ int
 verify_file_perms_ownership(const char *file)
 {
     int res = 1;
+
 #if HAVE_STAT
     struct stat st;
 
     /* Every file that fwknopd deals with should be owned
      * by the user and permissions set to 600 (user read/write)
     */
-    if((stat(file, &st)) != 0)
+    if((stat(file, &st)) == 0)
+    {
+        /* Make sure it is a regular file
+        */
+        if(S_ISREG(st.st_mode) != 1 && S_ISLNK(st.st_mode) != 1)
+        {
+            log_msg(LOG_WARNING,
+                "[-] file: %s is not a regular file or symbolic link.",
+                file
+            );
+            /* when we start in enforcing this instead of just warning
+             * the user
+            res = 0;
+            */
+        }
+
+        if((st.st_mode & (S_IRWXU|S_IRWXG|S_IRWXO)) != (S_IRUSR|S_IWUSR))
+        {
+            log_msg(LOG_WARNING,
+                "[-] file: %s permissions should only be user read/write (0600, -rw-------)",
+                file
+            );
+            /* when we start in enforcing this instead of just warning
+             * the user
+            res = 0;
+            */
+        }
+
+        if(st.st_uid != getuid())
+        {
+            log_msg(LOG_WARNING, "[-] file: %s not owned by current effective user id",
+                file);
+            /* when we start in enforcing this instead of just warning
+             * the user
+            res = 0;
+            */
+        }
+    }
+    else
     {
         /* if the path doesn't exist, just return, but otherwise something
          * went wrong
         */
-        if(errno == ENOENT)
+        if(errno != ENOENT)
         {
-            return 0;
-        } else {
             log_msg(LOG_ERR, "[-] stat() against file: %s returned: %s",
                 file, strerror(errno));
-            exit(EXIT_FAILURE);
+            res = 0;
         }
     }
 
-    /* Make sure it is a regular file
-    */
-    if(S_ISREG(st.st_mode) != 1 && S_ISLNK(st.st_mode) != 1)
-    {
-        log_msg(LOG_WARNING,
-            "[-] file: %s is not a regular file or symbolic link.",
-            file
-        );
-        res = 0;
-    }
-
-    if((st.st_mode & (S_IRWXU|S_IRWXG|S_IRWXO)) != (S_IRUSR|S_IWUSR))
-    {
-        log_msg(LOG_WARNING,
-            "[-] file: %s permissions should only be user read/write (0600, -rw-------)",
-            file
-        );
-        res = 0;
-    }
-
-    if(st.st_uid != getuid())
-    {
-        log_msg(LOG_WARNING, "[-] file: %s not owned by current effective user id",
-            file);
-        res = 0;
-    }
 #endif
 
     return res;