Fixes memory flags issues

Memory flags fixed by:
 - Clarifying defaults
 - Making the option to wipe internal memory a global flag
 - Introducing the clear_internal_memory function
 - Cleaning up memory allocation/freeing, so the custom allocator is
   used both when allocating the memory instance and when allocating the
   pseudo_random data.

include/argon2.h

@@ -84,10 +84,14 @@ extern "C" {
 #define ARGON2_MIN_SECRET UINT32_C(0)
 #define ARGON2_MAX_SECRET UINT32_C(0xFFFFFFFF)
 
+/* Flags to determine which fields are securely wiped (default = no wipe). */
+#define ARGON2_DEFAULT_FLAGS UINT32_C(0)
 #define ARGON2_FLAG_CLEAR_PASSWORD (UINT32_C(1) << 0)
 #define ARGON2_FLAG_CLEAR_SECRET (UINT32_C(1) << 1)
-#define ARGON2_FLAG_CLEAR_MEMORY (UINT32_C(1) << 2)
-#define ARGON2_DEFAULT_FLAGS (ARGON2_FLAG_CLEAR_MEMORY)
+
+/* Global flag to determine if we are wiping internal memory buffers. This flag
+ * is defined in core.c and deafults to 1 (wipe internal memory). */
+extern int FLAG_clear_internal_memory;
 
 /* Error codes */
 typedef enum Argon2_ErrorCodes {

src/argon2.c

@@ -139,7 +139,7 @@ int argon2_hash(const uint32_t t_cost, const uint32_t m_cost,
     result = argon2_ctx(&context, type);
 
     if (result != ARGON2_OK) {
-        secure_wipe_memory(out, hashlen);
+        clear_internal_memory(out, hashlen);
         free(out);
         return result;
     }
@@ -152,13 +152,13 @@ int argon2_hash(const uint32_t t_cost, const uint32_t m_cost,
     /* if encoding requested, write it */
     if (encoded && encodedlen) {
         if (encode_string(encoded, encodedlen, &context, type) != ARGON2_OK) {
-            secure_wipe_memory(out, hashlen); /* wipe buffers if error */
-            secure_wipe_memory(encoded, encodedlen);
+            clear_internal_memory(out, hashlen); /* wipe buffers if error */
+            clear_internal_memory(encoded, encodedlen);
             free(out);
             return ARGON2_ENCODING_FAIL;
         }
     }
-    secure_wipe_memory(out, hashlen);
+    clear_internal_memory(out, hashlen);
     free(out);
 
     return ARGON2_OK;

src/blake2/blake2-impl.h

@@ -151,6 +151,6 @@ static BLAKE2_INLINE uint64_t rotr64(const uint64_t w, const unsigned c) {
     return (w >> c) | (w << (64 - c));
 }
 
-void secure_wipe_memory(void *v, size_t n);
+void clear_internal_memory(void *v, size_t n);
 
 #endif

src/blake2/blake2b.c

@@ -61,7 +61,7 @@ static BLAKE2_INLINE void blake2b_increment_counter(blake2b_state *S,
 }
 
 static BLAKE2_INLINE void blake2b_invalidate_state(blake2b_state *S) {
-    secure_wipe_memory(S, sizeof(*S));      /* wipe */
+    clear_internal_memory(S, sizeof(*S));      /* wipe */
     blake2b_set_lastblock(S); /* invalidate for further use */
 }
 
@@ -157,7 +157,8 @@ int blake2b_init_key(blake2b_state *S, size_t outlen, const void *key,
         memset(block, 0, BLAKE2B_BLOCKBYTES);
         memcpy(block, key, keylen);
         blake2b_update(S, block, BLAKE2B_BLOCKBYTES);
-        secure_wipe_memory(block, BLAKE2B_BLOCKBYTES); /* Burn the key from stack */
+        /* Burn the key from stack */
+        clear_internal_memory(block, BLAKE2B_BLOCKBYTES);
     }
     return 0;
 }
@@ -284,9 +285,9 @@ int blake2b_final(blake2b_state *S, void *out, size_t outlen) {
     }
 
     memcpy(out, buffer, S->outlen);
-    secure_wipe_memory(buffer, sizeof(buffer));
-    secure_wipe_memory(S->buf, sizeof(S->buf));
-    secure_wipe_memory(S->h, sizeof(S->h));
+    clear_internal_memory(buffer, sizeof(buffer));
+    clear_internal_memory(S->buf, sizeof(S->buf));
+    clear_internal_memory(S->h, sizeof(S->h));
     return 0;
 }
 
@@ -324,7 +325,7 @@ int blake2b(void *out, size_t outlen, const void *in, size_t inlen,
     ret = blake2b_final(&S, out, outlen);
 
 fail:
-    secure_wipe_memory(&S, sizeof(S));
+    clear_internal_memory(&S, sizeof(S));
     return ret;
 }
 
@@ -382,7 +383,7 @@ int blake2b_long(void *pout, size_t outlen, const void *in, size_t inlen) {
         memcpy(out, out_buffer, toproduce);
     }
 fail:
-    secure_wipe_memory(&blake_state, sizeof(blake_state));
+    clear_internal_memory(&blake_state, sizeof(blake_state));
     return ret;
 #undef TRY
 }

src/core.c

@@ -82,25 +82,43 @@ static void store_block(void *output, const block *src) {
     }
 }
 
-/***************Memory allocators*****************/
-int allocate_memory(block **memory, uint32_t m_cost) {
-    if (memory != NULL) {
-        size_t memory_size = sizeof(block) * m_cost;
-        if (m_cost != 0 &&
-            memory_size / m_cost !=
-                sizeof(block)) { /*1. Check for multiplication overflow*/
-            return ARGON2_MEMORY_ALLOCATION_ERROR;
-        }
+/***************Memory functions*****************/
 
-        *memory = (block *)malloc(memory_size); /*2. Try to allocate*/
+int allocate_memory(const argon2_context *context, uint8_t **memory,
+                    size_t num, size_t size) {
+    size_t memory_size = num*size;
+    if (memory == NULL) {
+        return ARGON2_MEMORY_ALLOCATION_ERROR;
+    }
 
-        if (!*memory) {
-            return ARGON2_MEMORY_ALLOCATION_ERROR;
-        }
-        return ARGON2_OK;
+    /* 1. Check for multiplication overflow */
+    if (size != 0 && memory_size / size != num) {
+        return ARGON2_MEMORY_ALLOCATION_ERROR;
+    }
+
+    /* 2. Try to allocate with appropriate allocator */
+    if (context->allocate_cbk) {
+        (context->allocate_cbk)(memory, memory_size);
     } else {
+        *memory = malloc(memory_size);
+    }
+
+    if (*memory == NULL) {
         return ARGON2_MEMORY_ALLOCATION_ERROR;
     }
+
+    return ARGON2_OK;
+}
+
+void free_memory(const argon2_context *context, uint8_t *memory,
+                 size_t num, size_t size) {
+    size_t memory_size = num*size;
+    clear_internal_memory(memory, memory_size);
+    if (context->free_cbk) {
+        (context->free_cbk)(memory, memory_size);
+    } else {
+        free(memory);
+    }
 }
 
 void NOT_OPTIMIZED secure_wipe_memory(void *v, size_t n) {
@@ -116,17 +134,14 @@ void NOT_OPTIMIZED secure_wipe_memory(void *v, size_t n) {
 #endif
 }
 
-/*********Memory functions*/
-
-void clear_memory(argon2_instance_t *instance, int clear) {
-    if (instance->memory != NULL && clear) {
-        secure_wipe_memory(instance->memory,
-                           sizeof(block) * instance->memory_blocks);
-    }
+/* Memory clear flag defaults to true. */
+int FLAG_clear_internal_memory = 1;
+void clear_internal_memory(void *v, size_t n) {
+  if (FLAG_clear_internal_memory && v) {
+    secure_wipe_memory(v, n);
+  }
 }
 
-void free_memory(block *memory) { free(memory); }
-
 void finalize(const argon2_context *context, argon2_instance_t *instance) {
     if (context != NULL && instance != NULL) {
         block blockhash;
@@ -147,26 +162,17 @@ void finalize(const argon2_context *context, argon2_instance_t *instance) {
             store_block(blockhash_bytes, &blockhash);
             blake2b_long(context->out, context->outlen, blockhash_bytes,
                          ARGON2_BLOCK_SIZE);
-            secure_wipe_memory(blockhash.v,
-                               ARGON2_BLOCK_SIZE); /* clear blockhash */
-            secure_wipe_memory(blockhash_bytes,
-                               ARGON2_BLOCK_SIZE); /* clear blockhash_bytes */
+            /* clear blockhash and blockhash_bytes */
+            clear_internal_memory(blockhash.v, ARGON2_BLOCK_SIZE);
+            clear_internal_memory(blockhash_bytes, ARGON2_BLOCK_SIZE);
         }
 
 #ifdef GENKAT
         print_tag(context->out, context->outlen);
 #endif
 
-        /* Clear memory */
-        clear_memory(instance, context->flags & ARGON2_FLAG_CLEAR_PASSWORD);
-
-        /* Deallocate the memory */
-        if (NULL != context->free_cbk) {
-            context->free_cbk((uint8_t *)instance->memory,
-                              instance->memory_blocks * sizeof(block));
-        } else {
-            free_memory(instance->memory);
-        }
+        free_memory(context, (uint8_t *)instance->memory,
+                    instance->memory_blocks, sizeof(block));
     }
 }
 
@@ -246,7 +252,7 @@ static unsigned __stdcall fill_segment_thr(void *thread_data)
 static void *fill_segment_thr(void *thread_data)
 #endif
 {
-    argon2_thread_data *my_data = (argon2_thread_data *)thread_data;
+    argon2_thread_data *my_data = thread_data;
     fill_segment(my_data->instance_ptr, my_data->pos);
     argon2_thread_exit();
     return 0;
@@ -485,7 +491,7 @@ void fill_first_blocks(uint8_t *blockhash, const argon2_instance_t *instance) {
         load_block(&instance->memory[l * instance->lane_length + 1],
                    blockhash_bytes);
     }
-    secure_wipe_memory(blockhash_bytes, ARGON2_BLOCK_SIZE);
+    clear_internal_memory(blockhash_bytes, ARGON2_BLOCK_SIZE);
 }
 
 void initial_hash(uint8_t *blockhash, argon2_context *context,
@@ -568,22 +574,13 @@ int initialize(argon2_instance_t *instance, argon2_context *context) {
 
     if (instance == NULL || context == NULL)
         return ARGON2_INCORRECT_PARAMETER;
+    instance->context_ptr = context;
 
     /* 1. Memory allocation */
-
-    if (NULL != context->allocate_cbk) {
-        uint8_t *p;
-        result = context->allocate_cbk(&p, instance->memory_blocks *
-                                               ARGON2_BLOCK_SIZE);
-        if (ARGON2_OK != result) {
-            return result;
-        }
-        instance->memory = (block *)p;
-    } else {
-        result = allocate_memory(&(instance->memory), instance->memory_blocks);
-        if (ARGON2_OK != result) {
-            return result;
-        }
+    result = allocate_memory(context, (uint8_t **)&(instance->memory),
+                             instance->memory_blocks, sizeof(block));
+    if (result != ARGON2_OK) {
+        return result;
     }
 
     /* 2. Initial hashing */
@@ -592,9 +589,9 @@ int initialize(argon2_instance_t *instance, argon2_context *context) {
     /* Hashing all inputs */
     initial_hash(blockhash, context, instance->type);
     /* Zeroing 8 extra bytes */
-    secure_wipe_memory(blockhash + ARGON2_PREHASH_DIGEST_LENGTH,
-                       ARGON2_PREHASH_SEED_LENGTH -
-                           ARGON2_PREHASH_DIGEST_LENGTH);
+    clear_internal_memory(blockhash + ARGON2_PREHASH_DIGEST_LENGTH,
+                          ARGON2_PREHASH_SEED_LENGTH -
+                              ARGON2_PREHASH_DIGEST_LENGTH);
 
 #ifdef GENKAT
     initial_kat(blockhash, context, instance->type);
@@ -604,7 +601,7 @@ int initialize(argon2_instance_t *instance, argon2_context *context) {
      */
     fill_first_blocks(blockhash, instance);
     /* Clearing the hash */
-    secure_wipe_memory(blockhash, ARGON2_PREHASH_SEED_LENGTH);
+    clear_internal_memory(blockhash, ARGON2_PREHASH_SEED_LENGTH);
 
     return ARGON2_OK;
 }

src/core.h

@@ -30,8 +30,7 @@
 
 #define CONST_CAST(x) (x)(uintptr_t)
 
-/*************************Argon2 internal
- * constants**************************************************/
+/**********************Argon2 internal constants*******************************/
 
 enum argon2_core_constants {
     /* Memory block size in bytes */
@@ -49,8 +48,7 @@ enum argon2_core_constants {
     ARGON2_PREHASH_SEED_LENGTH = 72
 };
 
-/*************************Argon2 internal data
- * types**************************************************/
+/*************************Argon2 internal data types***********************/
 
 /*
  * Structure for the (1KB) memory block implemented as 128 64-bit words.
@@ -87,6 +85,7 @@ typedef struct Argon2_instance_t {
     uint32_t threads;
     argon2_type type;
     int print_internals; /* whether to print the memory blocks */
+    argon2_context *context_ptr; /* points back to original context */
 } argon2_instance_t;
 
 /*
@@ -106,32 +105,43 @@ typedef struct Argon2_thread_data {
     argon2_position_t pos;
 } argon2_thread_data;
 
-/*************************Argon2 core
- * functions**************************************************/
+/*************************Argon2 core functions********************************/
 
-/* Allocates memory to the given pointer
+/* Allocates memory to the given pointer, uses the appropriate allocator as
+ * specified in the context. Total allocated memory is num*size.
+ * @param context argon2_context which specifies the allocator
  * @param memory pointer to the pointer to the memory
- * @param m_cost number of blocks to allocate in the memory
+ * @param size the size in bytes for each element to be allocated
+ * @param num the number of elements to be allocated
  * @return ARGON2_OK if @memory is a valid pointer and memory is allocated
  */
-int allocate_memory(block **memory, uint32_t m_cost);
+int allocate_memory(const argon2_context *context, uint8_t **memory,
+                    size_t num, size_t size);
 
-/* Function that securely cleans the memory
+/*
+ * Frees memory at the given pointer, uses the appropriate deallocator as
+ * specified in the context. Also cleans the memory using clear_internal_memory.
+ * @param context argon2_context which specifies the deallocator
+ * @param memory pointer to buffer to be freed
+ * @param size the size in bytes for each element to be deallocated
+ * @param num the number of elements to be deallocated
+ */
+void free_memory(const argon2_context *context, uint8_t *memory,
+                 size_t num, size_t size);
+
+/* Function that securely cleans the memory. This ignores any flags set
+ * regarding clearing memory. Usually one just calls clear_internal_memory.
  * @param mem Pointer to the memory
  * @param s Memory size in bytes
  */
 void secure_wipe_memory(void *v, size_t n);
 
-/* Clears memory
- * @param instance pointer to the current instance
- * @param clear_memory indicates if we clear the memory with zeros.
- */
-void clear_memory(argon2_instance_t *instance, int clear);
-
-/* Deallocates memory
- * @param memory pointer to the blocks
+/* Function that securely clears the memory if FLAG_clear_internal_memory is
+ * set. If the flag isn't set, this function does nothing.
+ * @param mem Pointer to the memory
+ * @param s Memory size in bytes
  */
-void free_memory(block *memory);
+void clear_internal_memory(void *v, size_t n);
 
 /*
  * Computes absolute position of reference block in the lane following a skewed
@@ -205,6 +215,7 @@ void finalize(const argon2_context *context, argon2_instance_t *instance);
 /*
  * Function that fills the segment using previous segments also from other
  * threads
+ * @param context current context
  * @param instance Pointer to the current instance
  * @param position Current position
  * @pre all block pointers must be valid

src/genkat.c

@@ -169,7 +169,7 @@ static void generate_testvectors(argon2_type type, const uint32_t version) {
     context.threads = lanes;
     context.allocate_cbk = myown_allocator;
     context.free_cbk = myown_deallocator;
-    context.flags = 0;
+    context.flags = ARGON2_DEFAULT_FLAGS;
 
 #undef TEST_OUTLEN
 #undef TEST_PWDLEN