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 diff --git a/VERSION b/VERSION index c2c8dec5..859c330e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -fwknop-2.0.4 +fwknop-2.5-pre1 diff --git a/client/config_init.c b/client/config_init.c index 98403af3..b2b736f8 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) @@ -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; } @@ -1346,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."); @@ -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); diff --git a/client/fwknop.c b/client/fwknop.c index bb5b95e6..7e22c2c7 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) { @@ -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); } @@ -388,13 +398,14 @@ 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) { errmsg("fko_set_gpg_home_dir", res); fko_destroy(ctx); + ctx = NULL; return(EXIT_FAILURE); } } @@ -407,10 +418,11 @@ 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); } - 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) @@ -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); } } @@ -447,11 +460,12 @@ 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); 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); } @@ -561,7 +580,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) @@ -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); } @@ -875,7 +901,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 +1133,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 +1177,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) @@ -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); } 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/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/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; 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 diff --git a/lib/fko_encryption.c b/lib/fko_encryption.c index 4ec929d4..d7b45956 100644 --- a/lib/fko_encryption.c +++ b/lib/fko_encryption.c @@ -84,16 +84,23 @@ _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)) + { + free(plaintext); return(FKO_ERROR_INVALID_DATA); + } /* Make a bucket for the encrypted version and populate it. */ 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 +112,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 +169,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 +191,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 +287,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 +311,7 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key) { free(plain); - if(cipher) + if(cipher != NULL) free(cipher); return(res); @@ -301,7 +321,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); @@ -334,7 +359,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. @@ -349,8 +374,13 @@ 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) + { + free(cipher); return(FKO_ERROR_INVALID_DATA); + } + + cipher_len = b64_decode_len; /* Create a bucket for the plaintext data and decrypt the message * data into it. @@ -512,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. */ 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); } 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/lib/hmac.c b/lib/hmac.c index 2efa7c3f..259fe05a 100644 --- a/lib/hmac.c +++ b/lib/hmac.c @@ -101,22 +101,28 @@ 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(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; + + memcpy(init_key, key, final_len); if(MD5_BLOCK_LEN < key_len) { /* Calculate the digest of the key */ - md5(final_key, init_key, key_len); + md5(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); MD5Init(&ctx->ctx_inside); MD5Update(&ctx->ctx_inside, ctx->block_inner_pad, MD5_BLOCK_LEN); @@ -153,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); @@ -169,22 +175,28 @@ 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(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; + + memcpy(init_key, key, final_len); if(SHA1_BLOCK_LEN < key_len) { /* Calculate the digest of the key */ - sha1(final_key, init_key, key_len); + sha1(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); sha1_init(&ctx->ctx_inside); sha1_update(&ctx->ctx_inside, ctx->block_inner_pad, SHA1_BLOCK_LEN); @@ -221,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); @@ -237,22 +249,28 @@ 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(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; + + memcpy(init_key, key, final_len); if(SHA256_BLOCK_LEN < key_len) { /* Calculate the digest of the key */ - sha256(final_key, init_key, key_len); + sha256(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); SHA256_Init(&ctx->ctx_inside); SHA256_Update(&ctx->ctx_inside, ctx->block_inner_pad, SHA256_BLOCK_LEN); @@ -289,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); @@ -304,23 +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); - memcpy(init_key, key, key_len); - if(SHA384_BLOCK_LEN < key_len) - { - /* Calculate the digest of the key - */ - sha384(final_key, init_key, key_len); - } - else - { - memcpy(final_key, init_key, key_len); - } + if(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; - pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, key_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); + + 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); @@ -357,7 +371,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); @@ -372,23 +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); - memcpy(init_key, key, key_len); - if(SHA512_BLOCK_LEN < key_len) - { - /* Calculate the digest of the key - */ - sha512(final_key, init_key, key_len); - } - else - { - memcpy(final_key, init_key, key_len); - } + if(key_len > MAX_DIGEST_BLOCK_LEN) + final_len = MAX_DIGEST_BLOCK_LEN; - pad_init(ctx->block_inner_pad, ctx->block_outer_pad, final_key, key_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); + + 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); @@ -425,7 +435,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..af9d8212 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; } @@ -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; @@ -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/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), ""); } } 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) diff --git a/server/fwknopd.c b/server/fwknopd.c index f3a1bf9b..0225f24f 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) @@ -618,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 @@ -627,14 +635,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 +693,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); } diff --git a/server/incoming_spa.c b/server/incoming_spa.c index 88f0487b..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; } @@ -446,6 +457,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) { @@ -453,11 +465,10 @@ 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; } + } if(acc->gpg_decrypt_id != NULL) fko_set_gpg_recipient(ctx, acc->gpg_decrypt_id); @@ -508,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; } @@ -523,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; } @@ -549,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; } @@ -564,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; } @@ -591,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; } @@ -610,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; } @@ -627,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; } @@ -643,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; } @@ -661,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; } @@ -684,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; } @@ -706,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; } @@ -717,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 @@ -735,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; } @@ -779,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 @@ -801,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; } @@ -813,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; } 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. 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; 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 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); } - 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 diff --git a/test/test-fwknop.pl b/test/test-fwknop.pl index 364aa183..0aad5b9c 100755 --- a/test/test-fwknop.pl +++ b/test/test-fwknop.pl @@ -545,43 +545,13 @@ 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) { 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"); @@ -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;