continued zeroing out of sensitive data buffers in support of issue #93
authorMichael Rash <mbr@cipherdyne.org>
Tue, 9 Jul 2013 03:00:18 +0000 (23:00 -0400)
committerMichael Rash <mbr@cipherdyne.org>
Tue, 9 Jul 2013 03:00:18 +0000 (23:00 -0400)
client/fwknop.c
lib/fko.h
lib/fko_context.h
lib/fko_encryption.c
lib/fko_error.c
lib/fko_funcs.c
lib/fko_hmac.c
lib/fko_util.c
lib/fko_util.h
server/access.c
server/incoming_spa.c

index c79f0f2..9038c6b 100644 (file)
@@ -57,7 +57,7 @@ int resolve_ip_http(fko_cli_options_t *options);
 static void clean_exit(fko_ctx_t ctx, fko_cli_options_t *opts,
     char *key, int *key_len, char *hmac_key, int *hmac_key_len,
     unsigned int exit_status);
-static void zero_buf(char *buf, int len);
+static void zero_buf_wrapper(char *buf, int len);
 static int is_hostname_str_with_port(const char *str, char *hostname, size_t hostname_bufsize, int *port);
 
 #define MAX_CMDLINE_ARGS            50                  /*!< should be way more than enough */
@@ -185,6 +185,8 @@ main(int argc, char **argv)
 
     fko_cli_options_t   options;
 
+    memset(&options, 0x0, sizeof(fko_cli_options_t));
+
     /* Initialize the log module */
     log_new();
 
@@ -502,7 +504,9 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_get_spa_encryption_mode", res);
-            fko_destroy(ctx2);
+            if(fko_destroy(ctx2) == FKO_ERROR_ZERO_OUT_DATA)
+                log_msg(LOG_VERBOSITY_ERROR,
+                        "[*] Could not zero out sensitive data buffer.");
             ctx2 = NULL;
             clean_exit(ctx, &options, key, &key_len,
                 hmac_key, &hmac_key_len, EXIT_FAILURE);
@@ -524,7 +528,9 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_new_with_data", res);
-            fko_destroy(ctx2);
+            if(fko_destroy(ctx2) == FKO_ERROR_ZERO_OUT_DATA)
+                log_msg(LOG_VERBOSITY_ERROR,
+                        "[*] Could not zero out sensitive data buffer.");
             ctx2 = NULL;
             clean_exit(ctx, &options, key, &key_len,
                 hmac_key, &hmac_key_len, EXIT_FAILURE);
@@ -534,7 +540,9 @@ main(int argc, char **argv)
         if(res != FKO_SUCCESS)
         {
             errmsg("fko_set_spa_encryption_mode", res);
-            fko_destroy(ctx2);
+            if(fko_destroy(ctx2) == FKO_ERROR_ZERO_OUT_DATA)
+                log_msg(LOG_VERBOSITY_ERROR,
+                        "[*] Could not zero out sensitive data buffer.");
             ctx2 = NULL;
             clean_exit(ctx, &options, key, &key_len,
                 hmac_key, &hmac_key_len, EXIT_FAILURE);
@@ -550,7 +558,9 @@ main(int argc, char **argv)
                 if(res != FKO_SUCCESS)
                 {
                     errmsg("fko_set_gpg_home_dir", res);
-                    fko_destroy(ctx2);
+                    if(fko_destroy(ctx2) == FKO_ERROR_ZERO_OUT_DATA)
+                        log_msg(LOG_VERBOSITY_ERROR,
+                                "[*] Could not zero out sensitive data buffer.");
                     ctx2 = NULL;
                     clean_exit(ctx, &options, key, &key_len,
                         hmac_key, &hmac_key_len, EXIT_FAILURE);
@@ -577,8 +587,9 @@ main(int argc, char **argv)
                 log_msg(LOG_VERBOSITY_ERROR, "GPG ERR: %s\n%s", fko_gpg_errstr(ctx2),
                     "No access to recipient private key?");
             }
-
-            fko_destroy(ctx2);
+            if(fko_destroy(ctx2) == FKO_ERROR_ZERO_OUT_DATA)
+                log_msg(LOG_VERBOSITY_ERROR,
+                        "[*] Could not zero out sensitive data buffer.");
             ctx2 = NULL;
             clean_exit(ctx, &options, key, &key_len,
                 hmac_key, &hmac_key_len, EXIT_FAILURE);
@@ -587,7 +598,9 @@ main(int argc, char **argv)
         log_msg(LOG_VERBOSITY_NORMAL,"\nDump of the Decoded Data");
         display_ctx(ctx2);
 
-        fko_destroy(ctx2);
+        if(fko_destroy(ctx2) == FKO_ERROR_ZERO_OUT_DATA)
+            log_msg(LOG_VERBOSITY_ERROR,
+                    "[*] Could not zero out sensitive data buffer.");
         ctx2 = NULL;
     }
 
@@ -602,14 +615,14 @@ free_configs(fko_cli_options_t *opts)
 {
     if (opts->resolve_url != NULL)
         free(opts->resolve_url);
-    zero_buf(opts->key, MAX_KEY_LEN+1);
-    zero_buf(opts->key_base64, MAX_B64_KEY_LEN+1);
-    zero_buf(opts->hmac_key, MAX_KEY_LEN+1);
-    zero_buf(opts->hmac_key_base64, MAX_B64_KEY_LEN+1);
-    zero_buf(opts->gpg_recipient_key, MAX_GPG_KEY_ID);
-    zero_buf(opts->gpg_signer_key, MAX_GPG_KEY_ID);
-    zero_buf(opts->gpg_home_dir, MAX_PATH_LEN);
-    zero_buf(opts->server_command, MAX_LINE_LEN);
+    zero_buf_wrapper(opts->key, MAX_KEY_LEN+1);
+    zero_buf_wrapper(opts->key_base64, MAX_B64_KEY_LEN+1);
+    zero_buf_wrapper(opts->hmac_key, MAX_KEY_LEN+1);
+    zero_buf_wrapper(opts->hmac_key_base64, MAX_B64_KEY_LEN+1);
+    zero_buf_wrapper(opts->gpg_recipient_key, MAX_GPG_KEY_ID);
+    zero_buf_wrapper(opts->gpg_signer_key, MAX_GPG_KEY_ID);
+    zero_buf_wrapper(opts->gpg_home_dir, MAX_PATH_LEN);
+    zero_buf_wrapper(opts->server_command, MAX_LINE_LEN);
 }
 
 static int
@@ -1245,23 +1258,13 @@ errmsg(const char *msg, const int err) {
         MY_NAME, msg, err, fko_errstr(err));
 }
 
-/* zero out sensitive information in a way that isn't optimized out by the compiler
- * since we force a comparision
-*/
 static void
-zero_buf(char *buf, int len)
+zero_buf_wrapper(char *buf, int len)
 {
-    int i;
 
-    if(len <= 0)
-        return;
-
-    memset(buf, 0x0, len);
-
-    for(i=0; i < len; i++)
-        if(buf[i] != 0x0)
-            log_msg(LOG_VERBOSITY_ERROR,
-                    "[*] Could not zero out sensitive data buffer.");
+    if(zero_buf(buf, len) != FKO_SUCCESS)
+        log_msg(LOG_VERBOSITY_ERROR,
+                "[*] Could not zero out sensitive data buffer.");
 
     return;
 }
@@ -1273,11 +1276,13 @@ clean_exit(fko_ctx_t ctx, fko_cli_options_t *opts,
         char *key, int *key_len, char *hmac_key, int *hmac_key_len,
         unsigned int exit_status)
 {
-    free_configs(opts);
-    fko_destroy(ctx);
+    if(fko_destroy(ctx) == FKO_ERROR_ZERO_OUT_DATA)
+        log_msg(LOG_VERBOSITY_ERROR,
+                "[*] Could not zero out sensitive data buffer.");
     ctx = NULL;
-    zero_buf(key, *key_len);
-    zero_buf(hmac_key, *hmac_key_len);
+    free_configs(opts);
+    zero_buf_wrapper(key, *key_len);
+    zero_buf_wrapper(hmac_key, *hmac_key_len);
     *key_len = 0;
     *hmac_key_len = 0;
     exit(exit_status);
index ce7be12..c80866c 100644 (file)
--- a/lib/fko.h
+++ b/lib/fko.h
@@ -149,6 +149,7 @@ typedef enum {
     FKO_ERROR_INVALID_HMAC_KEY_LEN,
     FKO_ERROR_UNSUPPORTED_HMAC_MODE,
     FKO_ERROR_UNSUPPORTED_FEATURE,
+    FKO_ERROR_ZERO_OUT_DATA,
     FKO_ERROR_UNKNOWN,
 
     /* Start GPGME-related errors */
@@ -234,13 +235,13 @@ enum {
 
 /* Function prototypes */
 
-/* General api calls
+/* General API calls
 */
 DLL_API int fko_new(fko_ctx_t *ctx);
 DLL_API int fko_new_with_data(fko_ctx_t *ctx, const char * const enc_msg,
     const char * const dec_key, const int dec_key_len, int encryption_mode,
     const char * const hmac_key, const int hmac_key_len, const int hmac_type);
-DLL_API void fko_destroy(fko_ctx_t ctx);
+DLL_API int fko_destroy(fko_ctx_t ctx);
 DLL_API int fko_spa_data_final(fko_ctx_t ctx, const char * const enc_key,
     const int enc_key_len, const char * const hmac_key, const int hmac_key_len);
 
index 94a1ff6..61e428c 100644 (file)
@@ -55,6 +55,7 @@ typedef struct fko_gpg_sig *fko_gpg_sig_t;
 /* The pieces we need to make an FKO  SPA data packet.
 */
 struct fko_context {
+
     /* FKO SPA user-definable message data */
     char           *rand_val;
     char           *username;
index 5308079..28e30a4 100644 (file)
@@ -51,6 +51,7 @@ _rijndael_encrypt(fko_ctx_t ctx, const char *enc_key, const int enc_key_len)
     unsigned char  *ciphertext;
     int             cipher_len;
     int             pt_len;
+    int             zero_free_rv = FKO_SUCCESS;
 
     if(enc_key_len > RIJNDAEL_MAX_KEYSIZE)
         return(FKO_ERROR_INVALID_KEY_LEN);
@@ -87,19 +88,13 @@ _rijndael_encrypt(fko_ctx_t ctx, const char *enc_key, const int enc_key_len)
     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);
-    }
+        return(zero_free(plaintext, pt_len, 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);
-    }
+        return(zero_free(plaintext, pt_len, FKO_ERROR_MEMORY_ALLOCATION));
 
     cipher_len = rij_encrypt(
         (unsigned char*)plaintext, pt_len,
@@ -112,25 +107,38 @@ _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);
+        zero_free_rv = zero_free((char *) ciphertext,
+            pt_len+32, FKO_ERROR_MEMORY_ALLOCATION);
+
+        if(zero_free(plaintext, pt_len, FKO_ERROR_MEMORY_ALLOCATION)
+                    == FKO_ERROR_MEMORY_ALLOCATION)
+            return(zero_free_rv);
+        else
+            return(FKO_ERROR_ZERO_OUT_DATA);
     }
 
     b64_encode(ciphertext, b64ciphertext, cipher_len);
     strip_b64_eq(b64ciphertext);
 
     if(ctx->encrypted_msg != NULL)
-        free(ctx->encrypted_msg);
+        zero_free_rv = zero_free(ctx->encrypted_msg,
+                strnlen(ctx->encrypted_msg, MAX_SPA_ENCODED_MSG_SIZE),
+                FKO_SUCCESS);
 
     ctx->encrypted_msg     = strdup(b64ciphertext);
     ctx->encrypted_msg_len = strnlen(ctx->encrypted_msg, MAX_SPA_ENCODED_MSG_SIZE);
 
     /* Clean-up
     */
-    free(plaintext);
-    free(ciphertext);
-    free(b64ciphertext);
+    if(zero_free(plaintext, pt_len, FKO_SUCCESS) != FKO_SUCCESS)
+        zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
+
+    if(zero_free((char *) ciphertext, pt_len+32, FKO_SUCCESS) != FKO_SUCCESS)
+        zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
+
+    if(zero_free(b64ciphertext, strnlen(b64ciphertext,
+                    MAX_SPA_ENCODED_MSG_SIZE), FKO_SUCCESS) != FKO_SUCCESS)
+        zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
     if(ctx->encrypted_msg == NULL)
         return(FKO_ERROR_MEMORY_ALLOCATION);
@@ -138,7 +146,7 @@ _rijndael_encrypt(fko_ctx_t ctx, const char *enc_key, const int enc_key_len)
     if(! is_valid_encoded_msg_len(ctx->encrypted_msg_len))
         return(FKO_ERROR_INVALID_DATA);
 
-    return(FKO_SUCCESS);
+    return(zero_free_rv);
 }
 
 /* Decode, decrypt, and parse SPA data into the context.
@@ -150,6 +158,7 @@ _rijndael_decrypt(fko_ctx_t ctx,
     unsigned char  *ndx;
     unsigned char  *cipher;
     int             cipher_len, pt_len, i, err = 0, res = FKO_SUCCESS;
+    int             zero_free_rv = FKO_SUCCESS;
 
     if(key_len > RIJNDAEL_MAX_KEYSIZE)
         return(FKO_ERROR_INVALID_KEY_LEN);
@@ -172,39 +181,36 @@ _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);
-    }
+        return(zero_free((char *)cipher,
+                    ctx->encrypted_msg_len, FKO_ERROR_INVALID_DATA));
 
     /* Since we're using AES, make sure the incoming data is a multiple of
      * the blocksize
     */
     if((cipher_len % RIJNDAEL_BLOCKSIZE) != 0)
-    {
-        free(cipher);
-        return(FKO_ERROR_INVALID_DATA);
-    }
+        return(zero_free((char *)cipher,
+                    ctx->encrypted_msg_len, FKO_ERROR_INVALID_DATA));
 
     if(ctx->encoded_msg != NULL)
-        free(ctx->encoded_msg);
+        zero_free_rv = zero_free(ctx->encoded_msg,
+                strnlen(ctx->encoded_msg, MAX_SPA_ENCODED_MSG_SIZE),
+                FKO_SUCCESS);
 
     /* Create a bucket for the plaintext data and decrypt the message
      * data into it.
     */
     ctx->encoded_msg = malloc(cipher_len);
     if(ctx->encoded_msg == NULL)
-    {
-        free(cipher);
-        return(FKO_ERROR_MEMORY_ALLOCATION);
-    }
+        return(zero_free((char *)cipher,
+                    ctx->encrypted_msg_len, FKO_ERROR_MEMORY_ALLOCATION));
 
     pt_len = rij_decrypt(cipher, cipher_len, dec_key, key_len,
                 (unsigned char*)ctx->encoded_msg, encryption_mode);
 
     /* Done with cipher...
     */
-    free(cipher);
+    if(zero_free((char *)cipher, ctx->encrypted_msg_len, FKO_SUCCESS) != FKO_SUCCESS)
+        zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
     /* The length of the decrypted data should be within 32 bytes of the
      * length of the encrypted version.
@@ -218,6 +224,9 @@ _rijndael_decrypt(fko_ctx_t ctx,
     if(! is_valid_encoded_msg_len(pt_len))
         return(FKO_ERROR_INVALID_DATA);
 
+    if(zero_free_rv != FKO_SUCCESS)
+        return(zero_free_rv);
+
     ctx->encoded_msg_len = pt_len;
 
     /* At this point we can check the data to see if we have a good
@@ -248,7 +257,7 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key)
 {
     int             res;
     char           *plain;
-    int             pt_len;
+    int             pt_len, zero_free_rv = FKO_SUCCESS;
     char           *b64cipher;
     unsigned char  *cipher = NULL;
     size_t          cipher_len;
@@ -290,10 +299,7 @@ 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);
-    }
+        return(zero_free(plain, pt_len, FKO_ERROR_INVALID_DATA));
 
     if (enc_key != NULL)
     {
@@ -312,10 +318,11 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key)
     */
     if(res != FKO_SUCCESS)
     {
-        free(plain);
+        zero_free_rv = zero_free(plain, pt_len, FKO_SUCCESS);
 
         if(cipher != NULL)
-            free(cipher);
+            if(zero_free((char *) cipher, cipher_len, FKO_SUCCESS) != FKO_SUCCESS)
+                zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
         return(res);
     }
@@ -325,9 +332,13 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key)
     b64cipher = malloc(((cipher_len / 3) * 4) + 8);
     if(b64cipher == NULL)
     {
-        free(plain);
+        if(zero_free(plain, pt_len, FKO_SUCCESS) != FKO_SUCCESS)
+            zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
+
         if(cipher != NULL)
-            free(cipher);
+            if(zero_free((char *) cipher, cipher_len, FKO_SUCCESS) != FKO_SUCCESS)
+                zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
+
         return(FKO_ERROR_MEMORY_ALLOCATION);
     }
 
@@ -335,16 +346,23 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key)
     strip_b64_eq(b64cipher);
 
     if(ctx->encrypted_msg != NULL)
-        free(ctx->encrypted_msg);
+        zero_free_rv = zero_free(ctx->encrypted_msg,
+                strnlen(ctx->encrypted_msg, MAX_SPA_ENCODED_MSG_SIZE), FKO_SUCCESS);
 
     ctx->encrypted_msg     = strdup(b64cipher);
     ctx->encrypted_msg_len = strnlen(ctx->encrypted_msg, MAX_SPA_ENCODED_MSG_SIZE);
 
     /* Clean-up
     */
-    free(plain);
-    free(cipher);
-    free(b64cipher);
+    if(zero_free(plain, pt_len, FKO_SUCCESS) != FKO_SUCCESS)
+        zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
+
+    if(zero_free((char *) cipher, cipher_len, FKO_SUCCESS) != FKO_SUCCESS)
+        zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
+
+    if(zero_free(b64cipher, strnlen(b64cipher,
+                    MAX_SPA_ENCODED_MSG_SIZE), FKO_SUCCESS) != FKO_SUCCESS)
+        zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
     if(ctx->encrypted_msg == NULL)
         return(FKO_ERROR_MEMORY_ALLOCATION);
@@ -352,7 +370,7 @@ gpg_encrypt(fko_ctx_t ctx, const char *enc_key)
     if(! is_valid_encoded_msg_len(ctx->encrypted_msg_len))
         return(FKO_ERROR_INVALID_DATA);
 
-    return(FKO_SUCCESS);
+    return(zero_free_rv);
 }
 
 /* Prep and decrypt using gpgme
@@ -378,10 +396,8 @@ 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);
-    }
+        return(zero_free((char *) cipher, ctx->encrypted_msg_len,
+                    FKO_ERROR_INVALID_DATA));
 
     cipher_len = b64_decode_len;
 
@@ -401,7 +417,7 @@ gpg_decrypt(fko_ctx_t ctx, const char *dec_key)
 
     /* Done with cipher...
     */
-    free(cipher);
+    res = zero_free((char *) cipher, ctx->encrypted_msg_len, FKO_SUCCESS);
 
     if(res != FKO_SUCCESS)
         return(res);
index c43059c..bd50d00 100644 (file)
@@ -111,6 +111,9 @@ fko_errstr(const int err_code)
         case FKO_ERROR_UNSUPPORTED_FEATURE:
             return("Unsupported or unimplemented feature or function");
 
+        case FKO_ERROR_ZERO_OUT_DATA:
+            return("Could not zero out sensitive data");
+
         case FKO_ERROR_UNKNOWN:
             return("Unknown/Unclassified error");
 
index 0dc1ea0..db5b99b 100644 (file)
@@ -285,15 +285,17 @@ fko_new_with_data(fko_ctx_t *r_ctx, const char * const enc_msg,
 
 /* Destroy a context and free its resources
 */
-void
+int
 fko_destroy(fko_ctx_t ctx)
 {
+    int zero_free_rv = FKO_SUCCESS;
+
 #if HAVE_LIBGPGME
     fko_gpg_sig_t   gsig, tgsig;
 #endif
 
     if(!CTX_INITIALIZED(ctx))
-        return;
+        return(zero_free_rv);
 
     if(ctx->rand_val != NULL)
         free(ctx->rand_val);
@@ -314,19 +316,24 @@ fko_destroy(fko_ctx_t ctx)
         free(ctx->server_auth);
 
     if(ctx->digest != NULL)
-        free(ctx->digest);
+        if(zero_free(ctx->digest, ctx->digest_len, FKO_SUCCESS) != FKO_SUCCESS)
+            zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
     if(ctx->raw_digest != NULL)
-        free(ctx->raw_digest);
+        if(zero_free(ctx->raw_digest, ctx->raw_digest_len, FKO_SUCCESS) != FKO_SUCCESS)
+            zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
     if(ctx->encoded_msg != NULL)
-        free(ctx->encoded_msg);
+        if(zero_free(ctx->encoded_msg, ctx->encoded_msg_len, FKO_SUCCESS) != FKO_SUCCESS)
+            zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
     if(ctx->encrypted_msg != NULL)
-        free(ctx->encrypted_msg);
+        if(zero_free(ctx->encrypted_msg, ctx->encrypted_msg_len, FKO_SUCCESS) != FKO_SUCCESS)
+            zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
     if(ctx->msg_hmac != NULL)
-        free(ctx->msg_hmac);
+        if(zero_free(ctx->msg_hmac, ctx->msg_hmac_len, FKO_SUCCESS) != FKO_SUCCESS)
+            zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
 #if HAVE_LIBGPGME
     if(ctx->gpg_exe != NULL)
@@ -364,9 +371,11 @@ fko_destroy(fko_ctx_t ctx)
 
 #endif /* HAVE_LIBGPGME */
 
-    bzero(ctx, sizeof(*ctx));
+    memset(ctx, 0x0, sizeof(*ctx));
 
     free(ctx);
+
+    return(zero_free_rv);
 }
 
 /* Generate Rijndael and HMAC keys from /dev/random and base64
@@ -491,7 +500,8 @@ fko_spa_data_final(fko_ctx_t ctx,
 
             strlcat(tbuf, ctx->msg_hmac, data_with_hmac_len);
 
-            ctx->encrypted_msg = tbuf;
+            ctx->encrypted_msg     = tbuf;
+            ctx->encrypted_msg_len = data_with_hmac_len;
         }
     }
 
index 03a19d5..0e8aedb 100644 (file)
@@ -41,7 +41,7 @@ fko_verify_hmac(fko_ctx_t ctx,
     char    *hmac_digest_from_data = NULL;
     char    *tbuf = NULL;
     int      res = FKO_SUCCESS;
-    int      hmac_b64_digest_len = 0;
+    int      hmac_b64_digest_len = 0, zero_free_rv = FKO_SUCCESS;
 
     /* Must be initialized
     */
@@ -85,12 +85,13 @@ fko_verify_hmac(fko_ctx_t ctx,
     tbuf = strndup(ctx->encrypted_msg,
             ctx->encrypted_msg_len - hmac_b64_digest_len);
     if(tbuf == NULL)
-    {
-        free(hmac_digest_from_data);
-        return(FKO_ERROR_MEMORY_ALLOCATION);
-    }
+        return(zero_free(hmac_digest_from_data,
+                    strnlen(hmac_digest_from_data, MAX_SPA_ENCODED_MSG_SIZE),
+                FKO_ERROR_MEMORY_ALLOCATION));
 
-    free(ctx->encrypted_msg);
+    if(zero_free(ctx->encrypted_msg, ctx->encrypted_msg_len,
+                FKO_SUCCESS) != FKO_SUCCESS)
+        zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
 
     ctx->encrypted_msg      = tbuf;
     ctx->encrypted_msg_len -= hmac_b64_digest_len;
@@ -118,8 +119,15 @@ fko_verify_hmac(fko_ctx_t ctx,
 
     if (res != FKO_SUCCESS)
     {
-        free(hmac_digest_from_data);
-        return(res);
+        if(zero_free(hmac_digest_from_data,
+                    strnlen(hmac_digest_from_data, MAX_SPA_ENCODED_MSG_SIZE),
+                FKO_SUCCESS) != FKO_SUCCESS)
+            zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
+
+        if(zero_free_rv == FKO_SUCCESS)
+            return(res);
+        else
+            return(zero_free_rv);
     }
 
     /* Calculate the HMAC from the encrypted data and then
@@ -140,8 +148,15 @@ fko_verify_hmac(fko_ctx_t ctx,
         }
     }
 
-    free(hmac_digest_from_data);
-    return(res);
+    if(zero_free(hmac_digest_from_data,
+                strnlen(hmac_digest_from_data, MAX_SPA_ENCODED_MSG_SIZE),
+            FKO_SUCCESS) != FKO_SUCCESS)
+        zero_free_rv = FKO_ERROR_ZERO_OUT_DATA;
+
+    if(res == FKO_SUCCESS)
+        return(zero_free_rv);
+    else
+        return(res);
 }
 
 /* Return the fko HMAC data
index 3479a81..922ce72 100644 (file)
@@ -398,6 +398,42 @@ strtol_wrapper(const char * const str, const int min,
     return val;
 }
 
+/* zero out a buffer before free()
+*/
+int zero_free(char *buf, int len, int default_err)
+{
+    int res;
+
+    res = zero_buf(buf, len);
+
+    if(res == FKO_SUCCESS)
+        res = default_err;
+
+    free(buf);
+
+    return res;
+}
+
+/* zero out sensitive information in a way that isn't optimized out by the compiler
+ * since we force a comparision and return an error if there is a problem (though
+ * the caller should do something with this information too).
+*/
+int
+zero_buf(char *buf, int len)
+{
+    int i, res = FKO_SUCCESS;
+
+    if(len <= 0 || len > MAX_SPA_ENCODED_MSG_SIZE)
+        return FKO_ERROR_ZERO_OUT_DATA;
+
+    memset(buf, 0x0, len);
+
+    for(i=0; i < len; i++)
+        if(buf[i] != 0x0)
+            res = FKO_ERROR_ZERO_OUT_DATA;
+
+    return res;
+}
 
 #if defined(WIN32) || !defined(HAVE_STRNDUP)
 /* Windows does not have strndup, so we well implement it here.
index 790b467..29aba53 100644 (file)
@@ -45,6 +45,8 @@ short   digest_inttostr(int digest, char* digest_str, size_t digest_size);
 short   hmac_digest_strtoint(const char *dt_str);
 short   hmac_digest_inttostr(int digest, char* digest_str, size_t digest_size);
 int     constant_runtime_cmp(const char *a, const char *b, int len);
+int     zero_free(char *buf, int len, int default_err);
+int     zero_buf(char *buf, int len);
 
 const char * enc_type_inttostr(const int type);
 const char * msg_type_inttostr(const int type);
index 94bacb1..1b9bb38 100644 (file)
@@ -631,18 +631,13 @@ free_acc_string_list(acc_string_list_t *stl)
     }
 }
 
-/* zero out key information in a way that isn't optimized out by the compiler
-*/
 static void
-zero_key(char *key, int len)
+zero_buf_wrapper(char *buf, int len)
 {
-    int i;
-
-    memset(key, 0x0, len);
 
-    for(i=0; i < len; i++)
-        if(key[i] != 0x0)
-            log_msg(LOG_ERR, "[*] Could not zero out key data.");
+    if(zero_buf(buf, len) != FKO_SUCCESS)
+        log_msg(LOG_ERR,
+                "[*] Could not zero out sensitive data buffer.");
 
     return;
 }
@@ -680,25 +675,25 @@ free_acc_stanza_data(acc_stanza_t *acc)
 
     if(acc->key != NULL)
     {
-        zero_key(acc->key, acc->key_len);
+        zero_buf_wrapper(acc->key, acc->key_len);
         free(acc->key);
     }
 
     if(acc->key_base64 != NULL)
     {
-        zero_key(acc->key_base64, strlen(acc->key_base64));
+        zero_buf_wrapper(acc->key_base64, strlen(acc->key_base64));
         free(acc->key_base64);
     }
 
     if(acc->hmac_key != NULL)
     {
-        zero_key(acc->hmac_key, acc->hmac_key_len);
+        zero_buf_wrapper(acc->hmac_key, acc->hmac_key_len);
         free(acc->hmac_key);
     }
 
     if(acc->hmac_key_base64 != NULL)
     {
-        zero_key(acc->hmac_key_base64, strlen(acc->hmac_key_base64));
+        zero_buf_wrapper(acc->hmac_key_base64, strlen(acc->hmac_key_base64));
         free(acc->hmac_key_base64);
     }
 
index a69f2b6..e45d830 100644 (file)
@@ -147,6 +147,7 @@ get_raw_digest(char **digest, char *pkt_data)
     */
     res = fko_new_with_data(&ctx, (char *)pkt_data, NULL, 0,
             FKO_DEFAULT_ENC_MODE, NULL, 0, 0);
+
     if(res != FKO_SUCCESS)
     {
         log_msg(LOG_WARNING, "Error initializing FKO context from SPA data: %s",
@@ -362,7 +363,11 @@ incoming_spa(fko_srv_options_t *opts)
         */
         if(ctx != NULL)
         {
-            fko_destroy(ctx);
+            if(fko_destroy(ctx) == FKO_ERROR_ZERO_OUT_DATA)
+                log_msg(LOG_WARNING,
+                    "[%s] (stanza #%d) fko_destroy() could not zero out sensitive data buffer.",
+                    spadat.pkt_source_ip, stanza_num, fko_errstr(res)
+                );
             ctx = NULL;
         }
 
@@ -638,7 +643,11 @@ incoming_spa(fko_srv_options_t *opts)
 
             if(ctx != NULL)
             {
-                fko_destroy(ctx);
+                if(fko_destroy(ctx) == FKO_ERROR_ZERO_OUT_DATA)
+                    log_msg(LOG_WARNING,
+                        "[%s] (stanza #%d) fko_destroy() could not zero out sensitive data buffer.",
+                        spadat.pkt_source_ip, stanza_num, fko_errstr(res)
+                    );
                 ctx = NULL;
             }
             break;
@@ -765,7 +774,11 @@ incoming_spa(fko_srv_options_t *opts)
 
                 if(ctx != NULL)
                 {
-                    fko_destroy(ctx);
+                    if(fko_destroy(ctx) == FKO_ERROR_ZERO_OUT_DATA)
+                        log_msg(LOG_WARNING,
+                            "[%s] (stanza #%d) fko_destroy() could not zero out sensitive data buffer.",
+                            spadat.pkt_source_ip, stanza_num, fko_errstr(res)
+                        );
                     ctx = NULL;
                 }
 
@@ -800,7 +813,11 @@ incoming_spa(fko_srv_options_t *opts)
         process_spa_request(opts, acc, &spadat);
         if(ctx != NULL)
         {
-            fko_destroy(ctx);
+            if(fko_destroy(ctx) == FKO_ERROR_ZERO_OUT_DATA)
+                log_msg(LOG_WARNING,
+                    "[%s] (stanza #%d) fko_destroy() could not zero out sensitive data buffer.",
+                    spadat.pkt_source_ip, stanza_num
+                );
             ctx = NULL;
         }
         break;
@@ -811,7 +828,11 @@ incoming_spa(fko_srv_options_t *opts)
 
     if(ctx != NULL)
     {
-        fko_destroy(ctx);
+        if(fko_destroy(ctx) == FKO_ERROR_ZERO_OUT_DATA)
+            log_msg(LOG_WARNING,
+                "[%s] fko_destroy() could not zero out sensitive data buffer.",
+                spadat.pkt_source_ip
+            );
         ctx = NULL;
     }