diff --git a/client/config_init.c b/client/config_init.c index f12c23b3..c6cf8ac1 100644 --- a/client/config_init.c +++ b/client/config_init.c @@ -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; } diff --git a/client/fwknop.c b/client/fwknop.c index 8073d6ab..c79f0f29 100644 --- a/client/fwknop.c +++ b/client/fwknop.c @@ -37,25 +37,27 @@ /* 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 / 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 / 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); } diff --git a/client/getpasswd.c b/client/getpasswd.c index a31ee350..827bb309 100644 --- a/client/getpasswd.c +++ b/client/getpasswd.c @@ -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***/ diff --git a/client/getpasswd.h b/client/getpasswd.h index 2f95c0a0..ec68d8b5 100644 --- a/client/getpasswd.h +++ b/client/getpasswd.h @@ -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 */ diff --git a/client/spa_comm.c b/client/spa_comm.c index 638ed813..b567d66d 100644 --- a/client/spa_comm.c +++ b/client/spa_comm.c @@ -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; diff --git a/client/utils.c b/client/utils.c index 69cfd1ee..1dce38c2 100644 --- a/client/utils.c +++ b/client/utils.c @@ -118,54 +118,63 @@ 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) + { + /* Make sure it is a regular file + */ + if(S_ISREG(st.st_mode) != 1 && S_ISLNK(st.st_mode) != 1) + { + 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; + */ + } + + 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_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; + */ + } + } + 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_VERBOSITY_ERROR, "[-] stat() against file: %s returned: %s", file, strerror(errno)); - exit(EXIT_FAILURE); + 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_WARNING, - "[-] file: %s permissions should only be user read/write (0600, -rw-------)", - file - ); - res = 0; - } - - if(st.st_uid != getuid()) - { - log_msg(LOG_VERBOSITY_WARNING, - "[-] file: %s not owned by current effective user id.", - file); - res = 0; - } #endif return res; diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index 4235b2b7..53080794 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -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)) diff --git a/server/access.c b/server/access.c index 78be5163..94bacb1d 100644 --- a/server/access.c +++ b/server/access.c @@ -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 diff --git a/server/config_init.c b/server/config_init.c index 16b162a4..6fd358f0 100644 --- a/server/config_init.c +++ b/server/config_init.c @@ -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. diff --git a/server/fwknopd.c b/server/fwknopd.c index c600b39e..caeada08 100644 --- a/server/fwknopd.c +++ b/server/fwknopd.c @@ -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); diff --git a/server/replay_cache.c b/server/replay_cache.c index c9c8d16f..255adcc5 100644 --- a/server/replay_cache.c +++ b/server/replay_cache.c @@ -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 */ diff --git a/server/utils.c b/server/utils.c index 3a805e9b..1dcbff21 100644 --- a/server/utils.c +++ b/server/utils.c @@ -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;