From e1c6f04ef9658557fbfe99ff0953d206d8f0f0f5 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Tue, 7 May 2013 21:43:38 -0400 Subject: [PATCH 01/26] [client] fix missing 'break' in switch statement (discovered by Coverity) --- client/config_init.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/config_init.c b/client/config_init.c index 6350689c..0c9856f0 100644 --- a/client/config_init.c +++ b/client/config_init.c @@ -1541,12 +1541,14 @@ config_init(fko_cli_options_t *options, int argc, char **argv) options->use_hmac = 1; cli_arg_bitmask |= FWKNOP_CLI_ARG_BM(FWKNOP_CLI_ARG_KEY_HMAC_BASE64); cli_arg_bitmask |= FWKNOP_CLI_ARG_BM(FWKNOP_CLI_ARG_USE_HMAC); + break; case KEY_HMAC: strlcpy(options->hmac_key, optarg, sizeof(options->hmac_key)); options->have_hmac_key = 1; options->use_hmac = 1; cli_arg_bitmask |= FWKNOP_CLI_ARG_BM(FWKNOP_CLI_ARG_KEY_HMAC); cli_arg_bitmask |= FWKNOP_CLI_ARG_BM(FWKNOP_CLI_ARG_USE_HMAC); + break; case KEY_LEN: options->key_len = strtol_wrapper(optarg, 1, MAX_KEY_LEN, NO_EXIT_UPON_ERR, &is_err); From 50f0ee2f7db5d0d2290efa3fee10339318fa023f Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Tue, 7 May 2013 22:52:35 -0400 Subject: [PATCH 02/26] [server] bug fix for GPG 'nesting level does not match indentation' issue (discovered by Coverity) --- server/incoming_spa.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/incoming_spa.c b/server/incoming_spa.c index 88f0487b..c6660698 100644 --- a/server/incoming_spa.c +++ b/server/incoming_spa.c @@ -446,6 +446,7 @@ incoming_spa(fko_srv_options_t *opts) /* 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) { @@ -458,6 +459,7 @@ incoming_spa(fko_srv_options_t *opts) acc = acc->next; continue; } + } if(acc->gpg_decrypt_id != NULL) fko_set_gpg_recipient(ctx, acc->gpg_decrypt_id); From 8d980ae68646af35b531713b2d01bbf24e3a9468 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Tue, 7 May 2013 23:02:49 -0400 Subject: [PATCH 03/26] remove dead code caught by Coverity --- client/fwknop.c | 2 +- lib/cipher_funcs.c | 6 ++---- server/extcmd.c | 3 --- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/client/fwknop.c b/client/fwknop.c index bb5b95e6..b1e96ca5 100644 --- a/client/fwknop.c +++ b/client/fwknop.c @@ -447,7 +447,7 @@ main(int argc, char **argv) */ if(options.digest_type) { - fko_set_spa_digest_type(ctx, options.digest_type); + res = fko_set_spa_digest_type(ctx, options.digest_type); if(res != FKO_SUCCESS) { errmsg("fko_set_spa_digest_type", res); diff --git a/lib/cipher_funcs.c b/lib/cipher_funcs.c index abfb5737..2d3c1f51 100644 --- a/lib/cipher_funcs.c +++ b/lib/cipher_funcs.c @@ -213,14 +213,12 @@ rijndael_init(RIJNDAEL_context *ctx, const char *key, if(encryption_mode == FKO_ENC_MODE_CBC || encryption_mode == FKO_ENC_MODE_CBC_LEGACY_IV) ctx->mode = MODE_CBC; - else if(encryption_mode == FKO_ENC_MODE_CFB) - ctx->mode = MODE_CFB; + else if(encryption_mode == FKO_ENC_MODE_CTR) + ctx->mode = MODE_CTR; else if(encryption_mode == FKO_ENC_MODE_PCBC) ctx->mode = MODE_PCBC; else if(encryption_mode == FKO_ENC_MODE_OFB) ctx->mode = MODE_OFB; - else if(encryption_mode == FKO_ENC_MODE_CTR) - ctx->mode = MODE_CTR; else if(encryption_mode == FKO_ENC_MODE_CFB) ctx->mode = MODE_CFB; else if(encryption_mode == FKO_ENC_MODE_ECB) diff --git a/server/extcmd.c b/server/extcmd.c index fd9e4f66..0cb49b4f 100644 --- a/server/extcmd.c +++ b/server/extcmd.c @@ -149,9 +149,6 @@ _run_extcmd(uid_t user_uid, const char *cmd, char *so_buf, const size_t so_buf_s while((fgets(so_read_buf, IO_READ_BUF_LEN, ipt)) != NULL) { - if(so_buf == NULL) - continue; - strlcat(so_buf, so_read_buf, so_buf_sz); if(strlen(so_buf) >= so_buf_sz-1) From 3a1efd9321b428fc3dcebab18ee1d3453de4cab0 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Tue, 7 May 2013 23:35:34 -0400 Subject: [PATCH 04/26] [server] fixed several (non-exploitable) overflow conditions found by Coverity --- lib/fko_encryption.c | 2 +- lib/hmac.c | 60 +++++++++++++++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index 4ec929d4..f9dc5990 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -84,7 +84,7 @@ _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+1, "%s:%s", ctx->encoded_msg, ctx->digest); + pt_len = snprintf(plaintext, pt_len, "%s:%s", ctx->encoded_msg, ctx->digest); if(! is_valid_pt_msg_len(pt_len)) return(FKO_ERROR_INVALID_DATA); diff --git a/lib/hmac.c b/lib/hmac.c index 2efa7c3f..73ae2c73 100644 --- a/lib/hmac.c +++ b/lib/hmac.c @@ -101,15 +101,21 @@ hmac_md5_init(hmac_md5_ctx *ctx, const char *key, const int key_len) { unsigned char final_key[MAX_DIGEST_BLOCK_LEN] = {0}; unsigned char init_key[MAX_DIGEST_BLOCK_LEN] = {0}; + int final_len = key_len; memset(final_key, 0x00, MAX_DIGEST_BLOCK_LEN); - memcpy(init_key, key, key_len); + memset(init_key, 0x00, MAX_DIGEST_BLOCK_LEN); - if(MD5_BLOCK_LEN < key_len) + if(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; + + memcpy(init_key, key, final_len); + + if(MD5_BLOCK_LEN < final_len) { /* Calculate the digest of the key */ - md5(final_key, init_key, key_len); + md5(final_key, init_key, final_len); } else { @@ -169,15 +175,21 @@ hmac_sha1_init(hmac_sha1_ctx *ctx, const char *key, const int key_len) { unsigned char final_key[MAX_DIGEST_BLOCK_LEN] = {0}; unsigned char init_key[MAX_DIGEST_BLOCK_LEN] = {0}; + int final_len = key_len; memset(final_key, 0x00, MAX_DIGEST_BLOCK_LEN); - memcpy(init_key, key, key_len); + memset(init_key, 0x00, MAX_DIGEST_BLOCK_LEN); - if(SHA1_BLOCK_LEN < key_len) + if(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; + + memcpy(init_key, key, final_len); + + if(SHA1_BLOCK_LEN < final_len) { /* Calculate the digest of the key */ - sha1(final_key, init_key, key_len); + sha1(final_key, init_key, final_len); } else { @@ -237,15 +249,21 @@ hmac_sha256_init(hmac_sha256_ctx *ctx, const char *key, const int key_len) { unsigned char final_key[MAX_DIGEST_BLOCK_LEN] = {0}; unsigned char init_key[MAX_DIGEST_BLOCK_LEN] = {0}; + int final_len = key_len; memset(final_key, 0x00, MAX_DIGEST_BLOCK_LEN); - memcpy(init_key, key, key_len); + memset(init_key, 0x00, MAX_DIGEST_BLOCK_LEN); - if(SHA256_BLOCK_LEN < key_len) + if(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; + + memcpy(init_key, key, final_len); + + if(SHA256_BLOCK_LEN < final_len) { /* Calculate the digest of the key */ - sha256(final_key, init_key, key_len); + sha256(final_key, init_key, final_len); } else { @@ -305,15 +323,21 @@ hmac_sha384_init(hmac_sha384_ctx *ctx, const char *key, const int key_len) { unsigned char final_key[MAX_DIGEST_BLOCK_LEN] = {0}; unsigned char init_key[MAX_DIGEST_BLOCK_LEN] = {0}; + int final_len = key_len; memset(final_key, 0x00, MAX_DIGEST_BLOCK_LEN); - memcpy(init_key, key, key_len); + memset(init_key, 0x00, MAX_DIGEST_BLOCK_LEN); - if(SHA384_BLOCK_LEN < key_len) + if(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; + + memcpy(init_key, key, final_len); + + if(SHA384_BLOCK_LEN < final_len) { /* Calculate the digest of the key */ - sha384(final_key, init_key, key_len); + sha384(final_key, init_key, final_len); } else { @@ -373,15 +397,21 @@ hmac_sha512_init(hmac_sha512_ctx *ctx, const char *key, const int key_len) { unsigned char final_key[MAX_DIGEST_BLOCK_LEN] = {0}; unsigned char init_key[MAX_DIGEST_BLOCK_LEN] = {0}; + int final_len = key_len; memset(final_key, 0x00, MAX_DIGEST_BLOCK_LEN); - memcpy(init_key, key, key_len); + memset(init_key, 0x00, MAX_DIGEST_BLOCK_LEN); - if(SHA512_BLOCK_LEN < key_len) + if(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; + + memcpy(init_key, key, final_len); + + if(SHA512_BLOCK_LEN < final_len) { /* Calculate the digest of the key */ - sha512(final_key, init_key, key_len); + sha512(final_key, init_key, final_len); } else { From aaa28d4ab3437f3641aedf98074d8325ecec1196 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Wed, 8 May 2013 23:44:13 -0400 Subject: [PATCH 05/26] [server] double free bug fix in access.conf parsing routine caught by Coverity --- server/access.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/access.c b/server/access.c index 0a0b207c..d690e2fd 100644 --- a/server/access.c +++ b/server/access.c @@ -1185,8 +1185,6 @@ parse_access_file(fko_srv_options_t *opts) if(curr_acc->gpg_allow_no_pw == 1) { add_acc_bool(&(curr_acc->use_gpg), "Y"); - if(curr_acc->gpg_decrypt_pw != NULL && curr_acc->gpg_decrypt_pw[0] != '\0') - free(curr_acc->gpg_decrypt_pw); add_acc_string(&(curr_acc->gpg_decrypt_pw), ""); } } From 9f9bbcbcdd8a47ee29bf60bb2f2728685bbc7aec Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Wed, 8 May 2013 23:55:35 -0400 Subject: [PATCH 06/26] fixed several resource leak conditions found by Coverity --- client/config_init.c | 9 +++++---- client/spa_comm.c | 1 + lib/fko_encryption.c | 23 ++++++++++++++++++++++- lib/fko_user.c | 10 +++++++++- server/fwknopd.c | 33 ++++++++++++++++++--------------- 5 files changed, 55 insertions(+), 21 deletions(-) diff --git a/client/config_init.c b/client/config_init.c index 0c9856f0..e13b2cc3 100644 --- a/client/config_init.c +++ b/client/config_init.c @@ -1148,10 +1148,10 @@ update_rc(fko_cli_options_t *options, uint32_t args_bitmask) rcfile_fd = open(rcfile_update, FWKNOPRC_OFLAGS, FWKNOPRC_MODE); if (rcfile_fd == -1) { - log_msg(LOG_VERBOSITY_WARNING, - "update_rc() : Unable to create temporary rc file: %s: %s", - rcfile_update, strerror(errno)); - return; + log_msg(LOG_VERBOSITY_WARNING, + "update_rc() : Unable to create temporary rc file: %s: %s", + rcfile_update, strerror(errno)); + return; } close(rcfile_fd); @@ -1170,6 +1170,7 @@ update_rc(fko_cli_options_t *options, uint32_t args_bitmask) log_msg(LOG_VERBOSITY_WARNING, "update_rc() : Unable to open rc file: %s: %s", rcfile_update, strerror(errno)); + fclose(rc); return; } diff --git a/client/spa_comm.c b/client/spa_comm.c index fea36794..2532669b 100644 --- a/client/spa_comm.c +++ b/client/spa_comm.c @@ -552,6 +552,7 @@ send_spa_packet_http(const char *spa_data, const int sd_len, log_msg(LOG_VERBOSITY_ERROR, "[-] proxy port value is invalid, must be in [%d-%d]", 1, MAX_PORT); + free(spa_data_copy); return 0; } } diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index f9dc5990..3acdd002 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -93,7 +93,10 @@ _rijndael_encrypt(fko_ctx_t ctx, const char *enc_key, const int enc_key_len) */ ciphertext = calloc(1, pt_len + 32); /* Plus padding for salt and Block */ if(ciphertext == NULL) + { + free(plaintext); return(FKO_ERROR_MEMORY_ALLOCATION); + } cipher_len = rij_encrypt( (unsigned char*)plaintext, pt_len, @@ -105,7 +108,11 @@ _rijndael_encrypt(fko_ctx_t ctx, const char *enc_key, const int enc_key_len) */ b64ciphertext = malloc(((cipher_len / 3) * 4) + 8); if(b64ciphertext == NULL) + { + free(ciphertext); + free(plaintext); return(FKO_ERROR_MEMORY_ALLOCATION); + } b64_encode(ciphertext, b64ciphertext, cipher_len); strip_b64_eq(b64ciphertext); @@ -158,7 +165,10 @@ _rijndael_decrypt(fko_ctx_t ctx, return(FKO_ERROR_MEMORY_ALLOCATION); if((cipher_len = b64_decode(ctx->encrypted_msg, cipher)) < 0) + { + free(cipher); return(FKO_ERROR_INVALID_DATA); + } /* Since we're using AES, make sure the incoming data is a multiple of * the blocksize @@ -177,7 +187,10 @@ _rijndael_decrypt(fko_ctx_t ctx, */ ctx->encoded_msg = malloc(cipher_len); if(ctx->encoded_msg == NULL) + { + free(cipher); return(FKO_ERROR_MEMORY_ALLOCATION); + } pt_len = rij_decrypt(cipher, cipher_len, dec_key, key_len, (unsigned char*)ctx->encoded_msg, encryption_mode); @@ -270,7 +283,10 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key) pt_len = snprintf(plain, pt_len+1, "%s:%s", ctx->encoded_msg, ctx->digest); if(! is_valid_pt_msg_len(pt_len)) + { + free(plain); return(FKO_ERROR_INVALID_DATA); + } if (enc_key != NULL) { @@ -291,7 +307,7 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key) { free(plain); - if(cipher) + if(cipher != NULL) free(cipher); return(res); @@ -301,7 +317,12 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key) */ b64cipher = malloc(((cipher_len / 3) * 4) + 8); if(b64cipher == NULL) + { + free(plain); + if(cipher != NULL) + free(cipher); return(FKO_ERROR_MEMORY_ALLOCATION); + } b64_encode(cipher, b64cipher, cipher_len); strip_b64_eq(b64cipher); diff --git a/lib/fko_user.c b/lib/fko_user.c index 1f504d6e..c829902a 100644 --- a/lib/fko_user.c +++ b/lib/fko_user.c @@ -41,7 +41,7 @@ int fko_set_username(fko_ctx_t ctx, const char * const spoof_user) { char *username = NULL; - int res = FKO_SUCCESS; + int res = FKO_SUCCESS, is_user_heap_allocated=0; /* Must be initialized */ @@ -81,6 +81,7 @@ fko_set_username(fko_ctx_t ctx, const char * const spoof_user) username = strdup("NO_USER"); if(username == NULL) return(FKO_ERROR_MEMORY_ALLOCATION); + is_user_heap_allocated = 1; } } } @@ -92,7 +93,11 @@ fko_set_username(fko_ctx_t ctx, const char * const spoof_user) *(username + MAX_SPA_USERNAME_SIZE - 1) = '\0'; if((res = validate_username(username)) != FKO_SUCCESS) + { + if(is_user_heap_allocated == 1) + free(username); return res; + } /* Just in case this is a subsquent call to this function. We * do not want to be leaking memory. @@ -104,6 +109,9 @@ fko_set_username(fko_ctx_t ctx, const char * const spoof_user) ctx->state |= FKO_DATA_MODIFIED; + if(is_user_heap_allocated == 1) + free(username); + if(ctx->username == NULL) return(FKO_ERROR_MEMORY_ALLOCATION); diff --git a/server/fwknopd.c b/server/fwknopd.c index f3a1bf9b..53f85bcf 100644 --- a/server/fwknopd.c +++ b/server/fwknopd.c @@ -627,14 +627,14 @@ write_pid_file(fko_srv_options_t *opts) lck_res = lockf(op_fd, F_TLOCK, 0); if(lck_res == -1) { + close(op_fd); + if(errno != EAGAIN) { perror("Unexpected error from lockf: "); return -1; } - close(op_fd); - /* Look for an existing lock holder. If we get a pid return it. */ old_pid = get_running_pid(opts); @@ -685,22 +685,25 @@ get_running_pid(const fko_srv_options_t *opts) op_fd = open(opts->config[CONF_FWKNOP_PID_FILE], O_RDONLY); - if(op_fd > 0) + if(op_fd == -1) { - if (read(op_fd, buf, PID_BUFLEN) > 0) - { - buf[PID_BUFLEN-1] = '\0'; - /* max pid value is configurable on Linux - */ - rpid = (pid_t) strtol_wrapper(buf, 0, (2 << 30), - NO_EXIT_UPON_ERR, &is_err); - if(is_err != FKO_SUCCESS) - rpid = 0; - } - - close(op_fd); + perror("Error trying to open PID file: "); + return(rpid); } + if (read(op_fd, buf, PID_BUFLEN) > 0) + { + buf[PID_BUFLEN-1] = '\0'; + /* max pid value is configurable on Linux + */ + rpid = (pid_t) strtol_wrapper(buf, 0, (2 << 30), + NO_EXIT_UPON_ERR, &is_err); + if(is_err != FKO_SUCCESS) + rpid = 0; + } + + close(op_fd); + return(rpid); } From b92f892ae089679a80cb3ecc0217c5c0b8b700d8 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 9 May 2013 21:11:45 -0400 Subject: [PATCH 07/26] [test suite] minor bug fix for printing the number of test buckets to be executed --- test/test-fwknop.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index 364aa183..634803dd 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -581,7 +581,7 @@ for my $test_hr (@tests) { next unless &process_include_exclude(&get_msg($test_hr)); $test_buckets++; if ($test_limit > 0) { - last if $executed >= $test_limit; + last if $test_buckets >= $test_limit; } } &logr("[+] Total test buckets to execute: $test_buckets\n\n"); From 8c09d38941485623a452b4f2c8fd3946482414d0 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 9 May 2013 21:17:27 -0400 Subject: [PATCH 08/26] various sizeof() usage and type bug fixes found by Coverity --- client/config_init.c | 2 +- client/fwknop.c | 14 +++++++------- lib/fko_encryption.c | 6 ++++-- lib/hmac.c | 10 +++++----- lib/md5.c | 3 ++- lib/sha2.c | 10 +++++----- server/fwknopd.c | 7 +++++-- 7 files changed, 29 insertions(+), 23 deletions(-) diff --git a/client/config_init.c b/client/config_init.c index e13b2cc3..2ce1853a 100644 --- a/client/config_init.c +++ b/client/config_init.c @@ -329,7 +329,7 @@ is_rc_param(const char *line, rc_file_param_t *param) char val[MAX_LINE_LEN] = {0}; char *ndx; - memset(param, 0, sizeof(param)); + memset(param, 0, sizeof(*param)); /* Fetch the variable and its value */ if(sscanf(line, "%s %[^ ;\t\n\r#]", var, val) != 2) diff --git a/client/fwknop.c b/client/fwknop.c index b1e96ca5..c4eb4466 100644 --- a/client/fwknop.c +++ b/client/fwknop.c @@ -209,7 +209,7 @@ main(int argc, char **argv) options.hmac_key_base64, options.hmac_key_len, options.hmac_type); - if(options.key_gen_file != NULL && options.key_gen_file[0] != '\0') + if(options.key_gen_file[0] != '\0') { if ((key_gen_file_ptr = fopen(options.key_gen_file, "w")) == NULL) { @@ -388,7 +388,7 @@ main(int argc, char **argv) * this has to occur before calling any of the other GPG-related * functions. */ - if(options.gpg_home_dir != NULL && strlen(options.gpg_home_dir) > 0) + if(strlen(options.gpg_home_dir) > 0) { res = fko_set_gpg_home_dir(ctx, options.gpg_home_dir); if(res != FKO_SUCCESS) @@ -410,7 +410,7 @@ main(int argc, char **argv) return(EXIT_FAILURE); } - if(options.gpg_signer_key != NULL && strlen(options.gpg_signer_key)) + if(strlen(options.gpg_signer_key) > 0) { res = fko_set_gpg_signer(ctx, options.gpg_signer_key); if(res != FKO_SUCCESS) @@ -561,7 +561,7 @@ main(int argc, char **argv) */ if(options.use_gpg) { - if(options.gpg_home_dir != NULL && strlen(options.gpg_home_dir) > 0) + if(strlen(options.gpg_home_dir) > 0) { res = fko_set_gpg_home_dir(ctx2, options.gpg_home_dir); if(res != FKO_SUCCESS) @@ -875,7 +875,7 @@ prev_exec(fko_cli_options_t *options, int argc, char **argv) { char args_save_file[MAX_PATH_LEN] = {0}; - if(options->args_save_file != NULL && options->args_save_file[0] != 0x0) + if(options->args_save_file[0] != 0x0) { strlcpy(args_save_file, options->args_save_file, sizeof(args_save_file)); } @@ -1107,7 +1107,7 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options, */ if(options->use_gpg && (options->use_gpg_agent - || (crypt_op == CRYPT_OP_ENCRYPT && options->gpg_signer_key == NULL))) + || (crypt_op == CRYPT_OP_ENCRYPT && options->gpg_signer_key[0] == '\0'))) return; if (options->have_key) @@ -1151,7 +1151,7 @@ get_keys(fko_ctx_t ctx, fko_cli_options_t *options, strlcpy(key, key_tmp, MAX_KEY_LEN+1); *key_len = strlen(key); } - else if(options->gpg_signer_key && strlen(options->gpg_signer_key)) + else if(strlen(options->gpg_signer_key)) { key_tmp = getpasswd("Enter passphrase for signing: "); if(key_tmp == NULL) diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index 3acdd002..51b16adb 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -355,7 +355,7 @@ gpg_decrypt(fko_ctx_t ctx, const char *dec_key) { unsigned char *cipher; size_t cipher_len; - int res, pt_len; + int res, pt_len, b64_decode_len; /* Now see if we need to add the "hQ" string to the front of the * base64-encoded-GPG-encrypted data. @@ -370,9 +370,11 @@ gpg_decrypt(fko_ctx_t ctx, const char *dec_key) if(cipher == NULL) return(FKO_ERROR_MEMORY_ALLOCATION); - if((cipher_len = b64_decode(ctx->encrypted_msg, cipher)) < 0) + if((b64_decode_len = b64_decode(ctx->encrypted_msg, cipher)) < 0) return(FKO_ERROR_INVALID_DATA); + cipher_len = b64_decode_len; + /* Create a bucket for the plaintext data and decrypt the message * data into it. */ diff --git a/lib/hmac.c b/lib/hmac.c index 73ae2c73..eb565e60 100644 --- a/lib/hmac.c +++ b/lib/hmac.c @@ -159,7 +159,7 @@ hmac_md5(const char *msg, const unsigned int msg_len, { hmac_md5_ctx ctx; - memset(&ctx, 0, sizeof(&ctx)); + memset(&ctx, 0, sizeof(ctx)); hmac_md5_init(&ctx, hmac_key, hmac_key_len); hmac_md5_update(&ctx, msg, msg_len); @@ -233,7 +233,7 @@ hmac_sha1(const char *msg, const unsigned int msg_len, { hmac_sha1_ctx ctx; - memset(&ctx, 0, sizeof(&ctx)); + memset(&ctx, 0, sizeof(ctx)); hmac_sha1_init(&ctx, hmac_key, hmac_key_len); hmac_sha1_update(&ctx, msg, msg_len); @@ -307,7 +307,7 @@ hmac_sha256(const char *msg, const unsigned int msg_len, { hmac_sha256_ctx ctx; - memset(&ctx, 0, sizeof(&ctx)); + memset(&ctx, 0, sizeof(ctx)); hmac_sha256_init(&ctx, hmac_key, hmac_key_len); hmac_sha256_update(&ctx, msg, msg_len); @@ -381,7 +381,7 @@ hmac_sha384(const char *msg, const unsigned int msg_len, { hmac_sha384_ctx ctx; - memset(&ctx, 0, sizeof(&ctx)); + memset(&ctx, 0, sizeof(ctx)); hmac_sha384_init(&ctx, hmac_key, hmac_key_len); hmac_sha384_update(&ctx, msg, msg_len); @@ -455,7 +455,7 @@ hmac_sha512(const char *msg, const unsigned int msg_len, { hmac_sha512_ctx ctx; - memset(&ctx, 0, sizeof(&ctx)); + memset(&ctx, 0, sizeof(ctx)); hmac_sha512_init(&ctx, hmac_key, hmac_key_len); hmac_sha512_update(&ctx, msg, msg_len); diff --git a/lib/md5.c b/lib/md5.c index ab1d130f..33080aaa 100644 --- a/lib/md5.c +++ b/lib/md5.c @@ -170,7 +170,8 @@ MD5Final(unsigned char digest[16], MD5Context *ctx) MD5Transform(ctx->buf, (uint32_t *) ctx->in); byteReverse((unsigned char *) ctx->buf, 4); memcpy(digest, ctx->buf, 16); - memset(ctx, 0, sizeof(ctx)); /* In case it's sensitive */ + + memset(ctx, 0, sizeof(*ctx)); /* In case it's sensitive */ } diff --git a/lib/sha2.c b/lib/sha2.c index ee8447dc..dd81ce5b 100644 --- a/lib/sha2.c +++ b/lib/sha2.c @@ -644,7 +644,7 @@ void SHA256_Final(sha2_byte digest[], SHA256_CTX* context) { } /* Clean up state data: */ - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); usedspace = 0; } @@ -974,7 +974,7 @@ void SHA512_Final(sha2_byte digest[], SHA512_CTX* context) { } /* Zero out state data */ - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } char *SHA512_End(SHA512_CTX* context, char buffer[]) { @@ -994,7 +994,7 @@ char *SHA512_End(SHA512_CTX* context, char buffer[]) { } *buffer = (char)0; } else { - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } MEMSET_BZERO(digest, SHA512_DIGEST_LEN); return buffer; @@ -1049,7 +1049,7 @@ void SHA384_Final(sha2_byte digest[], SHA384_CTX* context) { } /* Zero out state data */ - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } char *SHA384_End(SHA384_CTX* context, char buffer[]) { @@ -1069,7 +1069,7 @@ char *SHA384_End(SHA384_CTX* context, char buffer[]) { } *buffer = (char)0; } else { - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } MEMSET_BZERO(digest, SHA384_DIGEST_LEN); return buffer; diff --git a/server/fwknopd.c b/server/fwknopd.c index 53f85bcf..fb91d55c 100644 --- a/server/fwknopd.c +++ b/server/fwknopd.c @@ -259,8 +259,11 @@ main(int argc, char **argv) log_msg(LOG_WARNING, "Error opening digest cache file. Incoming digests will not be remembered." ); - strlcpy(opts.config[CONF_ENABLE_DIGEST_PERSISTENCE], "N", - sizeof(opts.config[CONF_ENABLE_DIGEST_PERSISTENCE])); + /* Destination points to heap memory, and is guaranteed to be + * at least two bytes large via validate_options(), + * DEF_ENABLE_DIGEST_PERSISTENCE, and set_config_entry() + */ + strlcpy(opts.config[CONF_ENABLE_DIGEST_PERSISTENCE], "N", 2); } if(opts.verbose) From 9046acaf22650b2c3f71185d8a1201647c431a7b Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 9 May 2013 21:56:13 -0400 Subject: [PATCH 09/26] [libfko] memory leak fixes found by Coverity --- lib/fko_encryption.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index 51b16adb..0c19e7fb 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -84,10 +84,14 @@ _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)) + { + free(plaintext); return(FKO_ERROR_INVALID_DATA); + } /* Make a bucket for the encrypted version and populate it. */ @@ -371,7 +375,10 @@ gpg_decrypt(fko_ctx_t ctx, const char *dec_key) return(FKO_ERROR_MEMORY_ALLOCATION); if((b64_decode_len = b64_decode(ctx->encrypted_msg, cipher)) < 0) + { + free(cipher); return(FKO_ERROR_INVALID_DATA); + } cipher_len = b64_decode_len; From add518016c533c06fbdce5eb8a9adb5a903e178f Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 9 May 2013 22:10:38 -0400 Subject: [PATCH 10/26] [client] removed unnecessary array NULL check found by Coverity --- client/config_init.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/config_init.c b/client/config_init.c index 2ce1853a..ee632694 100644 --- a/client/config_init.c +++ b/client/config_init.c @@ -1347,8 +1347,7 @@ validate_options(fko_cli_options_t *options) */ if(options->use_gpg) { - if(options->gpg_recipient_key == NULL - || strlen(options->gpg_recipient_key) == 0) + if(strlen(options->gpg_recipient_key) == 0) { log_msg(LOG_VERBOSITY_ERROR, "Must specify --gpg-recipient-key when GPG is used."); From 62edf0910147435290c8fb8bc3d9d78c37ef1758 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 9 May 2013 22:13:25 -0400 Subject: [PATCH 11/26] [libfko] fixed remaining buffer constraints in lib/hmac.c code found by Coverity --- lib/hmac.c | 52 ++++++++++++++++------------------------------------ 1 file changed, 16 insertions(+), 36 deletions(-) diff --git a/lib/hmac.c b/lib/hmac.c index eb565e60..259fe05a 100644 --- a/lib/hmac.c +++ b/lib/hmac.c @@ -111,7 +111,7 @@ hmac_md5_init(hmac_md5_ctx *ctx, const char *key, const int key_len) memcpy(init_key, key, final_len); - if(MD5_BLOCK_LEN < final_len) + if(MD5_BLOCK_LEN < key_len) { /* Calculate the digest of the key */ @@ -122,7 +122,7 @@ hmac_md5_init(hmac_md5_ctx *ctx, const char *key, const int key_len) memcpy(final_key, init_key, key_len); } - pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, key_len); + pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, final_len); MD5Init(&ctx->ctx_inside); MD5Update(&ctx->ctx_inside, ctx->block_inner_pad, MD5_BLOCK_LEN); @@ -185,7 +185,7 @@ hmac_sha1_init(hmac_sha1_ctx *ctx, const char *key, const int key_len) memcpy(init_key, key, final_len); - if(SHA1_BLOCK_LEN < final_len) + if(SHA1_BLOCK_LEN < key_len) { /* Calculate the digest of the key */ @@ -196,7 +196,7 @@ hmac_sha1_init(hmac_sha1_ctx *ctx, const char *key, const int key_len) memcpy(final_key, init_key, key_len); } - pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, key_len); + pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, final_len); sha1_init(&ctx->ctx_inside); sha1_update(&ctx->ctx_inside, ctx->block_inner_pad, SHA1_BLOCK_LEN); @@ -259,7 +259,7 @@ hmac_sha256_init(hmac_sha256_ctx *ctx, const char *key, const int key_len) memcpy(init_key, key, final_len); - if(SHA256_BLOCK_LEN < final_len) + if(SHA256_BLOCK_LEN < key_len) { /* Calculate the digest of the key */ @@ -270,7 +270,7 @@ hmac_sha256_init(hmac_sha256_ctx *ctx, const char *key, const int key_len) memcpy(final_key, init_key, key_len); } - pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, key_len); + pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, final_len); SHA256_Init(&ctx->ctx_inside); SHA256_Update(&ctx->ctx_inside, ctx->block_inner_pad, SHA256_BLOCK_LEN); @@ -322,29 +322,19 @@ static void hmac_sha384_init(hmac_sha384_ctx *ctx, const char *key, const int key_len) { unsigned char final_key[MAX_DIGEST_BLOCK_LEN] = {0}; - unsigned char init_key[MAX_DIGEST_BLOCK_LEN] = {0}; int final_len = key_len; memset(final_key, 0x00, MAX_DIGEST_BLOCK_LEN); - memset(init_key, 0x00, MAX_DIGEST_BLOCK_LEN); if(key_len > MAX_DIGEST_BLOCK_LEN) final_len = MAX_DIGEST_BLOCK_LEN; - memcpy(init_key, key, final_len); + /* When we eventually support arbitrary key sizes, take the digest + * of the key with: sha384(final_key, init_key, final_len); + */ + memcpy(final_key, key, final_len); - if(SHA384_BLOCK_LEN < final_len) - { - /* Calculate the digest of the key - */ - sha384(final_key, init_key, final_len); - } - else - { - memcpy(final_key, init_key, key_len); - } - - pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, key_len); + pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, final_len); SHA384_Init(&ctx->ctx_inside); SHA384_Update(&ctx->ctx_inside, ctx->block_inner_pad, SHA384_BLOCK_LEN); @@ -396,29 +386,19 @@ static void hmac_sha512_init(hmac_sha512_ctx *ctx, const char *key, const int key_len) { unsigned char final_key[MAX_DIGEST_BLOCK_LEN] = {0}; - unsigned char init_key[MAX_DIGEST_BLOCK_LEN] = {0}; int final_len = key_len; memset(final_key, 0x00, MAX_DIGEST_BLOCK_LEN); - memset(init_key, 0x00, MAX_DIGEST_BLOCK_LEN); if(key_len > MAX_DIGEST_BLOCK_LEN) final_len = MAX_DIGEST_BLOCK_LEN; - memcpy(init_key, key, final_len); + /* When we eventually support arbitrary key sizes, take the digest + * of the key with: sha512(final_key, init_key, final_len); + */ + memcpy(final_key, key, final_len); - if(SHA512_BLOCK_LEN < final_len) - { - /* Calculate the digest of the key - */ - sha512(final_key, init_key, final_len); - } - else - { - memcpy(final_key, init_key, key_len); - } - - pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, key_len); + pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, final_len); SHA512_Init(&ctx->ctx_inside); SHA512_Update(&ctx->ctx_inside, ctx->block_inner_pad, SHA512_BLOCK_LEN); From 72e4edbf6a3b0c4bc361183b94e5495908e1e618 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 9 May 2013 22:14:06 -0400 Subject: [PATCH 12/26] [libfko] fixed remaining sizeof() usage bug in SHA256 code found by Coverity --- lib/sha2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/sha2.c b/lib/sha2.c index dd81ce5b..af9d8212 100644 --- a/lib/sha2.c +++ b/lib/sha2.c @@ -665,7 +665,7 @@ char *SHA256_End(SHA256_CTX* context, char buffer[]) { } *buffer = (char)0; } else { - MEMSET_BZERO(context, sizeof(context)); + MEMSET_BZERO(context, sizeof(*context)); } MEMSET_BZERO(digest, SHA256_DIGEST_LEN); return buffer; From aafc3ac264e9e8b347ba6b3b3b487e94b03fe7ef Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 9 May 2013 22:35:08 -0400 Subject: [PATCH 13/26] [server] setsockopt() nad fcntl() return value checking (found by Coverity) --- server/fwknopd.c | 7 ++++++- server/tcp_server.c | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/server/fwknopd.c b/server/fwknopd.c index fb91d55c..0225f24f 100644 --- a/server/fwknopd.c +++ b/server/fwknopd.c @@ -621,7 +621,12 @@ write_pid_file(fko_srv_options_t *opts) return -1; } - fcntl(op_fd, F_SETFD, FD_CLOEXEC); + if(fcntl(op_fd, F_SETFD, FD_CLOEXEC) == -1) + { + close(op_fd); + perror("Unexpected error from fcntl: "); + return -1; + } /* Attempt to lock the PID file. If we get an EWOULDBLOCK * error, another instance already has the lock. So we grab diff --git a/server/tcp_server.c b/server/tcp_server.c index 48f0f88c..d60bff5e 100644 --- a/server/tcp_server.c +++ b/server/tcp_server.c @@ -108,7 +108,12 @@ run_tcp_server(fko_srv_options_t *opts) /* So that we can re-bind to it without TIME_WAIT problems */ - setsockopt(s_sock, SOL_SOCKET, SO_REUSEADDR, &reuse_addr, sizeof(reuse_addr)); + if(setsockopt(s_sock, SOL_SOCKET, SO_REUSEADDR, &reuse_addr, sizeof(reuse_addr)) == -1) + { + log_msg(LOG_ERR, "run_tcp_server: setsockopt error: %s", + strerror(errno)); + exit(EXIT_FAILURE); + } /* Make our main socket non-blocking so we don't have to be stuck on * listening for incoming connections. From 282b0198ecabc69b1aa9adc9bc839b6a9dea2967 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Thu, 9 May 2013 22:43:05 -0400 Subject: [PATCH 14/26] [libfko] changed 'state' context element to 'int' type to fix a 'extra high-order bits' bug found by Coverity --- lib/fko_context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fko_context.h b/lib/fko_context.h index 8bc91380..9b9dce2a 100644 --- a/lib/fko_context.h +++ b/lib/fko_context.h @@ -94,7 +94,7 @@ struct fko_context { int added_gpg_prefix; /* State info */ - unsigned short state; + unsigned int state; unsigned char initval; #if HAVE_LIBGPGME From a8410d8f2a6a77ae2be76a67f05af80f47927f9d Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sat, 11 May 2013 13:28:55 -0400 Subject: [PATCH 15/26] [test suite] allow valgrind coverage test to run after --test-limit --- test/test-fwknop.pl | 58 ++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index 634803dd..0aad5b9c 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -545,36 +545,6 @@ if ($enable_valgrind) { } } -if ($enable_profile_coverage_check) { - push @tests, - { - 'category' => 'profile coverage', - 'detail' => 'gcov profile coverage', - 'function' => \&profile_coverage, - 'fatal' => $NO - }; -} - -if ($enable_valgrind) { - push @tests, - { - 'category' => 'valgrind', - 'subcategory' => 'fko-wrapper', - 'detail' => 'multiple libfko calls', - 'function' => \&compile_execute_fko_wrapper, - 'fatal' => $NO - }; - - push @tests, - { - 'category' => 'valgrind output', - 'subcategory' => 'flagged functions', - 'detail' => '', - 'function' => \&parse_valgrind_flagged_functions, - 'fatal' => $NO - }; -} - ### print a summary of how many test buckets will be run my $test_buckets = 0; for my $test_hr (@tests) { @@ -594,6 +564,34 @@ for my $test_hr (@tests) { } } +if ($enable_profile_coverage_check) { + &run_test({ + 'category' => 'profile coverage', + 'detail' => 'gcov profile coverage', + 'function' => \&profile_coverage, + 'fatal' => $NO} + ); +} + +if ($enable_valgrind) { + &run_test({ + 'category' => 'valgrind', + 'subcategory' => 'fko-wrapper', + 'detail' => 'multiple libfko calls', + 'function' => \&compile_execute_fko_wrapper, + 'fatal' => $NO} + ); + + &run_test({ + 'category' => 'valgrind output', + 'subcategory' => 'flagged functions', + 'detail' => '', + 'function' => \&parse_valgrind_flagged_functions, + 'fatal' => $NO} + ); +} + + &logr("\n"); &remove_permissions_warnings() unless $include_permissions_warnings; From 3302dd42207d1aa40a3a90386aec8e6a34169c36 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 14:42:35 -0400 Subject: [PATCH 16/26] [test suite] added -g to fko_wrapper Makefile for debugging symbols --- test/fko-wrapper/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fko-wrapper/Makefile b/test/fko-wrapper/Makefile index 96711e95..7ddaf8e1 100644 --- a/test/fko-wrapper/Makefile +++ b/test/fko-wrapper/Makefile @@ -1,6 +1,6 @@ all : fko_wrapper.c - gcc -Wall -I../../common -I../../client -I../../lib fko_wrapper.c -o fko_wrapper -L../../lib/.libs -lfko + gcc -Wall -g -I../../common -I../../client -I../../lib fko_wrapper.c -o fko_wrapper -L../../lib/.libs -lfko clean: rm -f fko_wrapper From 38395b04c69268004519a54efd3331e6e1c6583d Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 14:43:19 -0400 Subject: [PATCH 17/26] [test suite] add -x to run_valgrind.sh fko-wrapper script --- test/fko-wrapper/run_valgrind.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fko-wrapper/run_valgrind.sh b/test/fko-wrapper/run_valgrind.sh index 22c79c91..7c0b6b86 100755 --- a/test/fko-wrapper/run_valgrind.sh +++ b/test/fko-wrapper/run_valgrind.sh @@ -1,3 +1,3 @@ -#!/bin/sh +#!/bin/sh -x LD_LIBRARY_PATH=../../lib/.libs valgrind --leak-check=full --show-reachable=yes --track-origins=yes ./fko_wrapper From 6d0f970b3441b5980cff69eeb636963558b1e617 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 15:02:31 -0400 Subject: [PATCH 18/26] [libfko] bug fix to apply ctx initialization check before attempting to use ctx->message_type in fko_set_spa_client_timeout() --- lib/fko_client_timeout.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/fko_client_timeout.c b/lib/fko_client_timeout.c index abfe49b3..6a62223c 100644 --- a/lib/fko_client_timeout.c +++ b/lib/fko_client_timeout.c @@ -36,7 +36,7 @@ int fko_set_spa_client_timeout(fko_ctx_t ctx, const int timeout) { - int old_msg_type = ctx->message_type; + int old_msg_type; /* Context must be initialized. */ @@ -48,6 +48,8 @@ fko_set_spa_client_timeout(fko_ctx_t ctx, const int timeout) if(timeout < 0) return(FKO_ERROR_INVALID_DATA); + old_msg_type = ctx->message_type; + ctx->client_timeout = timeout; ctx->state |= FKO_DATA_MODIFIED; From 7b3c854a024c9778b4c16fea075e5a80a53c7ea2 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 20:49:00 -0400 Subject: [PATCH 19/26] [libfko] added context initialized check to fko_decrypt_spa_data() --- lib/fko_encryption.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index 0c19e7fb..d7b45956 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -542,6 +542,9 @@ fko_decrypt_spa_data(fko_ctx_t ctx, const char * const dec_key, const int key_le { int enc_type, res; + if(!CTX_INITIALIZED(ctx)) + return(FKO_ERROR_CTX_NOT_INITIALIZED); + /* Get the (assumed) type of encryption used. This will also provide * some data validation. */ From d85c2e74ce06ac461bb84dd508f8a5562a0483c8 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 20:53:22 -0400 Subject: [PATCH 20/26] [libfko] set ctx=NULL after fko_destroy(), add NULL check for encrypted msg pointer in fko_new_with_data() --- lib/fko_funcs.c | 120 ++++++++++++++++++++++++++---------------------- 1 file changed, 65 insertions(+), 55 deletions(-) diff --git a/lib/fko_funcs.c b/lib/fko_funcs.c index 351ad0fc..a44761c3 100644 --- a/lib/fko_funcs.c +++ b/lib/fko_funcs.c @@ -39,7 +39,7 @@ int fko_new(fko_ctx_t *r_ctx) { - fko_ctx_t ctx; + fko_ctx_t ctx = NULL; int res; char *ver; @@ -77,6 +77,7 @@ fko_new(fko_ctx_t *r_ctx) if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } @@ -88,6 +89,7 @@ fko_new(fko_ctx_t *r_ctx) if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } @@ -99,6 +101,7 @@ fko_new(fko_ctx_t *r_ctx) if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } @@ -110,6 +113,7 @@ fko_new(fko_ctx_t *r_ctx) if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } @@ -121,6 +125,7 @@ fko_new(fko_ctx_t *r_ctx) if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } @@ -132,6 +137,7 @@ fko_new(fko_ctx_t *r_ctx) if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } @@ -143,6 +149,7 @@ fko_new(fko_ctx_t *r_ctx) if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } @@ -174,10 +181,13 @@ fko_new_with_data(fko_ctx_t *r_ctx, const char * const enc_msg, int encryption_mode, const char * const hmac_key, const int hmac_key_len, const int hmac_type) { - fko_ctx_t ctx; + fko_ctx_t ctx = NULL; int res = FKO_SUCCESS; /* Are we optimistic or what? */ int enc_msg_len; + if(enc_msg == NULL) + return(FKO_ERROR_INVALID_DATA); + ctx = calloc(1, sizeof *ctx); if(ctx == NULL) return(FKO_ERROR_MEMORY_ALLOCATION); @@ -211,6 +221,7 @@ fko_new_with_data(fko_ctx_t *r_ctx, const char * const enc_msg, if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } ctx->initval = 0; @@ -222,6 +233,7 @@ fko_new_with_data(fko_ctx_t *r_ctx, const char * const enc_msg, if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } ctx->initval = 0; @@ -234,6 +246,7 @@ fko_new_with_data(fko_ctx_t *r_ctx, const char * const enc_msg, if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; return res; } ctx->initval = 0; @@ -252,6 +265,7 @@ fko_new_with_data(fko_ctx_t *r_ctx, const char * const enc_msg, if(res != FKO_SUCCESS) { fko_destroy(ctx); + ctx = NULL; *r_ctx = NULL; /* Make sure the caller ctx is null just in case */ return(res); } @@ -278,83 +292,79 @@ fko_destroy(fko_ctx_t ctx) fko_gpg_sig_t gsig, tgsig; #endif - if(CTX_INITIALIZED(ctx)) - { - if(ctx->rand_val != NULL) - free(ctx->rand_val); + if(!CTX_INITIALIZED(ctx)) + return; - if(ctx->username != NULL) - free(ctx->username); + if(ctx->rand_val != NULL) + free(ctx->rand_val); - if(ctx->version != NULL) - free(ctx->version); + if(ctx->username != NULL) + free(ctx->username); - if(ctx->message != NULL) - free(ctx->message); + if(ctx->version != NULL) + free(ctx->version); - if(ctx->nat_access != NULL) - free(ctx->nat_access); + if(ctx->message != NULL) + free(ctx->message); - if(ctx->server_auth != NULL) - free(ctx->server_auth); + if(ctx->nat_access != NULL) + free(ctx->nat_access); - if(ctx->digest != NULL) - free(ctx->digest); + if(ctx->server_auth != NULL) + free(ctx->server_auth); - if(ctx->raw_digest != NULL) - free(ctx->raw_digest); + if(ctx->digest != NULL) + free(ctx->digest); - if(ctx->encoded_msg != NULL) - free(ctx->encoded_msg); + if(ctx->raw_digest != NULL) + free(ctx->raw_digest); - if(ctx->encrypted_msg != NULL) - free(ctx->encrypted_msg); + if(ctx->encoded_msg != NULL) + free(ctx->encoded_msg); - if(ctx->msg_hmac != NULL) - free(ctx->msg_hmac); + if(ctx->encrypted_msg != NULL) + free(ctx->encrypted_msg); + + if(ctx->msg_hmac != NULL) + free(ctx->msg_hmac); #if HAVE_LIBGPGME - if(ctx->gpg_exe != NULL) - free(ctx->gpg_exe); + if(ctx->gpg_exe != NULL) + free(ctx->gpg_exe); - if(ctx->gpg_home_dir != NULL) - free(ctx->gpg_home_dir); + if(ctx->gpg_home_dir != NULL) + free(ctx->gpg_home_dir); - if(ctx->gpg_recipient != NULL) - free(ctx->gpg_recipient); + if(ctx->gpg_recipient != NULL) + free(ctx->gpg_recipient); - if(ctx->gpg_signer != NULL) - free(ctx->gpg_signer); + if(ctx->gpg_signer != NULL) + free(ctx->gpg_signer); - if(ctx->recipient_key != NULL) - { - gpgme_key_unref(ctx->recipient_key); - } + if(ctx->recipient_key != NULL) + gpgme_key_unref(ctx->recipient_key); - if(ctx->signer_key != NULL) - { - gpgme_key_unref(ctx->signer_key); - } + if(ctx->signer_key != NULL) + gpgme_key_unref(ctx->signer_key); - if(ctx->gpg_ctx != NULL) - gpgme_release(ctx->gpg_ctx); + if(ctx->gpg_ctx != NULL) + gpgme_release(ctx->gpg_ctx); - gsig = ctx->gpg_sigs; - while(gsig != NULL) - { - if(gsig->fpr != NULL) - free(gsig->fpr); + gsig = ctx->gpg_sigs; + while(gsig != NULL) + { + if(gsig->fpr != NULL) + free(gsig->fpr); - tgsig = gsig; - gsig = gsig->next; + tgsig = gsig; + gsig = gsig->next; - free(tgsig); - } + free(tgsig); + } #endif /* HAVE_LIBGPGME */ - bzero(ctx, sizeof(*ctx)); - } + bzero(ctx, sizeof(*ctx)); free(ctx); } From c555a35489b830b20f2270b91bace1e42d455e3e Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 20:54:04 -0400 Subject: [PATCH 21/26] [client] set ctx=NULL after fko_destroy() calls --- client/fwknop.c | 27 +++++++++++++++++++++++++++ client/getpasswd.c | 2 ++ 2 files changed, 29 insertions(+) diff --git a/client/fwknop.c b/client/fwknop.c index c4eb4466..7e22c2c7 100644 --- a/client/fwknop.c +++ b/client/fwknop.c @@ -252,6 +252,7 @@ main(int argc, char **argv) MY_VERSION, version); fko_destroy(ctx); + ctx = NULL; return(EXIT_SUCCESS); } @@ -264,6 +265,7 @@ main(int argc, char **argv) { errmsg("fko_set_spa_client_timeout", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -275,6 +277,7 @@ main(int argc, char **argv) { errmsg("fko_set_spa_message_type", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } @@ -287,6 +290,7 @@ main(int argc, char **argv) { errmsg("fko_set_timestamp", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -297,6 +301,7 @@ main(int argc, char **argv) { errmsg("fko_set_timestamp", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -319,6 +324,7 @@ main(int argc, char **argv) if(resolve_ip_http(&options) < 0) { fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -335,6 +341,7 @@ main(int argc, char **argv) { errmsg("fko_set_spa_message", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } @@ -347,6 +354,7 @@ main(int argc, char **argv) { errmsg("fko_set_nat_access_str", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -360,6 +368,7 @@ main(int argc, char **argv) { errmsg("fko_set_username", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -381,6 +390,7 @@ main(int argc, char **argv) { errmsg("fko_set_spa_encryption_type", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } @@ -395,6 +405,7 @@ main(int argc, char **argv) { errmsg("fko_set_gpg_home_dir", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -407,6 +418,7 @@ 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); } @@ -421,6 +433,7 @@ main(int argc, char **argv) log_msg(LOG_VERBOSITY_ERROR, "GPG ERR: %s", fko_gpg_errstr(ctx)); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -452,6 +465,7 @@ main(int argc, char **argv) { errmsg("fko_set_spa_digest_type", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -492,6 +506,7 @@ main(int argc, char **argv) { log_msg(LOG_VERBOSITY_ERROR, "send_spa_packet: packet not sent."); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } else @@ -513,6 +528,7 @@ main(int argc, char **argv) { errmsg("fko_get_spa_data", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } @@ -524,6 +540,7 @@ main(int argc, char **argv) errmsg("fko_get_spa_encryption_mode", res); fko_destroy(ctx); fko_destroy(ctx2); + ctx = ctx2 = NULL; return(EXIT_FAILURE); } @@ -545,6 +562,7 @@ main(int argc, char **argv) errmsg("fko_new_with_data", res); fko_destroy(ctx); fko_destroy(ctx2); + ctx = ctx2 = NULL; return(EXIT_FAILURE); } @@ -554,6 +572,7 @@ main(int argc, char **argv) errmsg("fko_set_spa_encryption_mode", res); fko_destroy(ctx); fko_destroy(ctx2); + ctx = ctx2 = NULL; return(EXIT_FAILURE); } @@ -569,6 +588,7 @@ main(int argc, char **argv) errmsg("fko_set_gpg_home_dir", res); fko_destroy(ctx); fko_destroy(ctx2); + ctx = ctx2 = NULL; return(EXIT_FAILURE); } } @@ -597,11 +617,13 @@ main(int argc, char **argv) "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); } @@ -609,6 +631,7 @@ main(int argc, char **argv) display_ctx(ctx2); fko_destroy(ctx2); + ctx2 = NULL; } clean_exit(ctx, &options, EXIT_SUCCESS); @@ -637,6 +660,7 @@ get_rand_port(fko_ctx_t ctx) { errmsg("get_rand_port(), fko_get_rand_value", res); fko_destroy(ctx); + ctx = NULL; exit(EXIT_FAILURE); } @@ -649,6 +673,7 @@ get_rand_port(fko_ctx_t ctx) "[*] get_rand_port(), could not convert rand_val str '%s', to integer", rand_val); fko_destroy(ctx); + ctx = NULL; exit(EXIT_FAILURE); } @@ -665,6 +690,7 @@ get_rand_port(fko_ctx_t ctx) { errmsg("get_rand_port(), fko_get_rand_value", res); fko_destroy(ctx); + ctx = NULL; exit(EXIT_FAILURE); } @@ -1263,6 +1289,7 @@ clean_exit(fko_ctx_t ctx, fko_cli_options_t *opts, unsigned int exit_status) { free_configs(opts); fko_destroy(ctx); + ctx = NULL; exit(exit_status); } diff --git a/client/getpasswd.c b/client/getpasswd.c index 2a8f1b6d..a84a2447 100644 --- a/client/getpasswd.c +++ b/client/getpasswd.c @@ -182,6 +182,7 @@ get_key_file(char *key, int *key_len, const char *key_file, { log_msg(LOG_VERBOSITY_ERROR, "Could not open config file: %s", key_file); fko_destroy(ctx); + ctx = NULL; exit(EXIT_FAILURE); } @@ -238,6 +239,7 @@ get_key_file(char *key, int *key_len, const char *key_file, 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); } From 1caf6035d9e475f3c98ee97e9c28996c7f5e54d6 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 20:54:44 -0400 Subject: [PATCH 22/26] [server] fixed potential double-free condition found by Coverity Within the access loop always call fko_destroy() right up front whenever ctx != NULL to ensure a clean slate each time through the loop regardless of what state may have been reached the previous time through the loop. --- server/incoming_spa.c | 58 +++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/server/incoming_spa.c b/server/incoming_spa.c index c6660698..81e3722a 100644 --- a/server/incoming_spa.c +++ b/server/incoming_spa.c @@ -152,6 +152,7 @@ get_raw_digest(char **digest, char *pkt_data) log_msg(LOG_WARNING, "Error initializing FKO context from SPA data: %s", fko_errstr(res)); fko_destroy(ctx); + ctx = NULL; return(SPA_MSG_FKO_CTX_ERROR); } @@ -161,6 +162,7 @@ get_raw_digest(char **digest, char *pkt_data) log_msg(LOG_WARNING, "Error setting digest type for SPA data: %s", fko_errstr(res)); fko_destroy(ctx); + ctx = NULL; return(SPA_MSG_DIGEST_ERROR); } @@ -170,6 +172,7 @@ get_raw_digest(char **digest, char *pkt_data) log_msg(LOG_WARNING, "Error setting digest for SPA data: %s", fko_errstr(res)); fko_destroy(ctx); + ctx = NULL; return(SPA_MSG_DIGEST_ERROR); } @@ -179,6 +182,7 @@ get_raw_digest(char **digest, char *pkt_data) log_msg(LOG_WARNING, "Error getting digest from SPA data: %s", fko_errstr(res)); fko_destroy(ctx); + ctx = NULL; return(SPA_MSG_DIGEST_ERROR); } @@ -188,6 +192,7 @@ get_raw_digest(char **digest, char *pkt_data) return SPA_MSG_ERROR; fko_destroy(ctx); + ctx = NULL; return res; } @@ -354,6 +359,14 @@ incoming_spa(fko_srv_options_t *opts) attempted_decrypt = 0; stanza_num++; + /* Start access loop with a clean FKO context + */ + if(ctx != NULL) + { + fko_destroy(ctx); + ctx = NULL; + } + /* 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))) @@ -437,8 +450,6 @@ incoming_spa(fko_srv_options_t *opts) "[%s] (stanza #%d) Error creating fko context (before decryption): %s", spadat.pkt_source_ip, stanza_num, fko_errstr(res) ); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -454,8 +465,6 @@ incoming_spa(fko_srv_options_t *opts) "[%s] (stanza #%d) Error setting GPG keyring path to %s: %s", spadat.pkt_source_ip, stanza_num, acc->gpg_home_dir, fko_errstr(res) ); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -510,8 +519,6 @@ incoming_spa(fko_srv_options_t *opts) log_msg(LOG_WARNING, "[%s] (stanza #%d) - GPG ERROR: %s", spadat.pkt_source_ip, stanza_num, fko_gpg_errstr(ctx)); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -525,8 +532,6 @@ incoming_spa(fko_srv_options_t *opts) { log_msg(LOG_WARNING, "[%s] (stanza #%d) Could not add digest to replay cache", spadat.pkt_source_ip, stanza_num); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -551,8 +556,6 @@ incoming_spa(fko_srv_options_t *opts) { log_msg(LOG_WARNING, "[%s] (stanza #%d) Error pulling the GPG signature ID from the context: %s", spadat.pkt_source_ip, stanza_num, fko_gpg_errstr(ctx)); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -566,8 +569,6 @@ incoming_spa(fko_srv_options_t *opts) log_msg(LOG_WARNING, "[%s] (stanza #%d) Incoming SPA packet signed by ID: %s, but that ID is not the GPG_REMOTE_ID list.", spadat.pkt_source_ip, stanza_num, gpg_id); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -593,8 +594,6 @@ incoming_spa(fko_srv_options_t *opts) log_msg(LOG_ERR, "[%s] (stanza #%d) Unexpected error pulling SPA data from the context: %s", spadat.pkt_source_ip, stanza_num, fko_errstr(res)); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -612,8 +611,6 @@ incoming_spa(fko_srv_options_t *opts) log_msg(LOG_WARNING, "[%s] (stanza #%d) SPA data time difference is too great (%i seconds).", spadat.pkt_source_ip, stanza_num, ts_diff); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -629,8 +626,6 @@ incoming_spa(fko_srv_options_t *opts) log_msg(LOG_WARNING, "[%s] (stanza #%d) Error parsing SPA message string: %s", spadat.pkt_source_ip, stanza_num, fko_errstr(res)); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -645,7 +640,10 @@ incoming_spa(fko_srv_options_t *opts) spadat.pkt_source_ip, stanza_num, fko_errstr(res)); if(ctx != NULL) + { fko_destroy(ctx); + ctx = NULL; + } break; } @@ -663,8 +661,6 @@ incoming_spa(fko_srv_options_t *opts) spadat.pkt_source_ip, stanza_num ); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -686,8 +682,6 @@ incoming_spa(fko_srv_options_t *opts) spadat.pkt_source_ip, stanza_num, spadat.username, acc->require_username ); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -708,8 +702,6 @@ incoming_spa(fko_srv_options_t *opts) stanza_num, spadat.pkt_source_ip ); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -719,8 +711,6 @@ incoming_spa(fko_srv_options_t *opts) stanza_num, spadat.pkt_source_ip ); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; #endif @@ -737,8 +727,6 @@ incoming_spa(fko_srv_options_t *opts) spadat.pkt_source_ip, stanza_num ); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -781,7 +769,10 @@ incoming_spa(fko_srv_options_t *opts) res = SPA_MSG_COMMAND_ERROR; if(ctx != NULL) + { fko_destroy(ctx); + ctx = NULL; + } /* we processed the command on a matching access stanza, so we * don't look for anything else to do with this SPA packet @@ -803,8 +794,6 @@ incoming_spa(fko_srv_options_t *opts) spadat.pkt_source_ip, stanza_num ); - if(ctx != NULL) - fko_destroy(ctx); acc = acc->next; continue; } @@ -815,13 +804,22 @@ incoming_spa(fko_srv_options_t *opts) */ process_spa_request(opts, acc, &spadat); if(ctx != NULL) + { fko_destroy(ctx); + ctx = NULL; + } break; } if (raw_digest != NULL) free(raw_digest); + if(ctx != NULL) + { + fko_destroy(ctx); + ctx = NULL; + } + return; } From 838782f19810d38ef2ffe556426faaf6e49d42f5 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 20:57:19 -0400 Subject: [PATCH 23/26] [test suite] added fko_destroy() calls to fko-wrapper --- test/fko-wrapper/fko_wrapper.c | 127 ++++++++++++++++++++++++++++++--- 1 file changed, 118 insertions(+), 9 deletions(-) diff --git a/test/fko-wrapper/fko_wrapper.c b/test/fko-wrapper/fko_wrapper.c index 2b710152..2b0ad1ab 100644 --- a/test/fko-wrapper/fko_wrapper.c +++ b/test/fko-wrapper/fko_wrapper.c @@ -13,119 +13,229 @@ #include "fko.h" #define ENABLE_GPG_TESTS 0 -#define FCN_CALLS 5 +#define FCN_CALLS 5 +#define CTX_DESTROY 0 +#define NO_CTX_DESTROY 1 static void display_ctx(fko_ctx_t ctx); +static void test_loop(int destroy_ctx_flag); int main(void) { + test_loop(NO_CTX_DESTROY); + test_loop(CTX_DESTROY); + + return 0; +} + +static void +test_loop(int destroy_ctx_flag) +{ fko_ctx_t ctx = NULL, decrypt_ctx = NULL; int i; - char *spa_data; + char *spa_data = NULL; printf("fko_new(): %d\n", fko_new(&ctx)); for (i=0; i" : spa_data); + if (destroy_ctx_flag == CTX_DESTROY) + { + fko_destroy(ctx); + ctx = NULL; + } } /* now decrypt */ printf("fko_new_with_data(): %d\n", fko_new_with_data(&decrypt_ctx, spa_data, NULL, - 0, ctx->encryption_mode, NULL, 0, 0)); + 0, FKO_ENC_MODE_CBC, NULL, 0, 0)); for (i=0; i" : spa_message); printf(" Nat Access: %s\n", nat_access == NULL ? "" : nat_access); printf(" Server Auth: %s\n", server_auth == NULL ? "" : server_auth); - printf(" Client Timeout: %u\n", client_timeout); + printf(" Client Timeout: %d\n", client_timeout); printf(" Digest Type: %d\n", digest_type); printf(" HMAC Type: %d\n", hmac_type); printf("Encryption Mode: %d\n", encryption_mode); @@ -192,4 +302,3 @@ display_ctx(fko_ctx_t ctx) printf("\nFinal Packed/Encrypted/Encoded Data:\n\n%s\n\n", spa_data); } - From 09f073d393ea29c9ad22b72491e0cf97da058c1c Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 21:04:25 -0400 Subject: [PATCH 24/26] Added blurb on Coverity to the ChangeLog --- ChangeLog | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/ChangeLog b/ChangeLog index 7b15b447..2671927f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -28,6 +28,14 @@ fwknop-2.5 (//2013): engine. If a vulnerability is discovered in OpenSSL and a change is made, then the --enable-openssl-checks mode will allow the test suite to discover this in a automated fashion for fwknop. + - The fwknop project is using Coverity for source code validation (in + addition to other tools such as the CLANG static analyzer). Many bugs + have been fixed in this release that were discovered by Coverity. These + bugs spanned multiple classes of problems from memory leaks, improper use + of sizeof(), potential double-free conditions, and more. Full details on + these fixes are available in the git history. Any open source project + that is written in a language supported by Coverity would benefit highly + from participating. - [test suite] Changed how the test suite interacts with the fwknop client and server by looking for indications that SPA packets are actually received. This is done by first waiting for 'main event loop' in fwknopd From 3246c3c6b0a40c380660f4885334c06e48213977 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 22:30:28 -0400 Subject: [PATCH 25/26] [test suite] added hmac_get_key_access.conf file --- test/conf/hmac_get_key_access.conf | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 test/conf/hmac_get_key_access.conf diff --git a/test/conf/hmac_get_key_access.conf b/test/conf/hmac_get_key_access.conf new file mode 100644 index 00000000..6ef17975 --- /dev/null +++ b/test/conf/hmac_get_key_access.conf @@ -0,0 +1,4 @@ +SOURCE: ANY; +KEY: rijndaelkey; +HMAC_KEY: hmackey; +FW_ACCESS_TIMEOUT: 3; From c83bc15c5eb9d6597df17cd9b421ab818548b210 Mon Sep 17 00:00:00 2001 From: Michael Rash Date: Sun, 12 May 2013 22:42:13 -0400 Subject: [PATCH 26/26] bumped VERSION file to fwknop-2.5-pre1 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index c2c8dec5..859c330e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -fwknop-2.0.4 +fwknop-2.5-pre1