Refine libar2_hash_buf_size + add test + securely erase parts of the hash output buffer that is written to but is not part of the hash

Signed-off-by: Mattias Andrée <maandree@kth.se>

7 files changed, 90 insertions, 9 deletions

diff --git a/common.h b/common.h
index d918563..e02d2c6 100644
--- a/common.h
+++ b/common.h
@@ -28,6 +28,11 @@
 #endif
 
 
+#if defined(__GNUC__)
+# define LIBAR2_WEAKLY_LINKED__ __attribute__((weak))
+#endif
+
+
 #ifndef CACHE_LINE_SIZE
 # define CACHE_LINE_SIZE 256 /* better with larger than actual than smaller than actual */
 #endif
@@ -48,6 +53,7 @@
 # endif
 #endif
 
+
 #define ELEMSOF(ARR) (sizeof(ARR) / sizeof(*(ARR)))
 
 #define MAX(A, B) ((A) > (B) ? (A) : (B))
diff --git a/libar2.h b/libar2.h
index f518c8d..3b14b3a 100644
--- a/libar2.h
+++ b/libar2.h
@@ -671,7 +671,7 @@ int libar2_hash(void *hash, void *msg, size_t msglen, struct libar2_argon2_param
  *                  result is too large
  */
 LIBAR2_PUBLIC__ LIBAR2_NONNULL__(1)
-size_t libar2_hash_buf_size(struct libar2_argon2_parameters *params);
+size_t libar2_hash_buf_size(const struct libar2_argon2_parameters *params);
 
 #if defined(__clang__)
 # pragma clang diagnostic pop
diff --git a/libar2_erase.c b/libar2_erase.c
index 6760390..f4fa2d3 100644
--- a/libar2_erase.c
+++ b/libar2_erase.c
@@ -23,6 +23,9 @@ __attribute__((optnone))
 __attribute__((optimize("O0")))
 # endif
 #endif
+#ifdef LIBAR2_WEAKLY_LINKED__
+LIBAR2_WEAKLY_LINKED__
+#endif
 void
 libar2_erase(volatile void *mem_, size_t size)
 {
diff --git a/libar2_hash.c b/libar2_hash.c
index 6ce898c..757dbdf 100644
--- a/libar2_hash.c
+++ b/libar2_hash.c
@@ -487,7 +487,7 @@ libar2_hash(void *hash, void *msg, size_t msglen, struct libar2_argon2_parameter
 	unsigned char block[1024 + 128], hash0[256];
 	uint_least32_t blocks, seglen, lanelen;
 	struct block *memory;
-	size_t i, p, s, nthreads, ts[16], ti, tn;
+	size_t i, p, s, nthreads, ts[16], ti, tn, bufsize;
 	struct threaded_fill_segments_params *tparams = NULL;
 	uint_least64_t *sbox = NULL; /* This is 8K large (assuming support for uint64_t), so we allocate it dynamically */
 
@@ -596,6 +596,9 @@ libar2_hash(void *hash, void *msg, size_t msglen, struct libar2_argon2_parameter
 		memxor(&memory[lanelen - 1], &memory[i * lanelen + lanelen - 1], sizeof(*memory));
 	store_block(block, &memory[lanelen - 1]);
 	argon2_blake2b_exthash(hash, params->hashlen, block, 1024);
+	bufsize = libar2_hash_buf_size(params);
+	if (bufsize) /* should never be 0 as that would indicate the user provided a too small buffer */
+		libar2_erase(&((char *)hash)[params->hashlen], bufsize - params->hashlen);
 
 	ERASE_ARRAY(block);
 	if (sbox)
diff --git a/libar2_hash_buf_size.3 b/libar2_hash_buf_size.3
index dcb8285..66cd05d 100644
--- a/libar2_hash_buf_size.3
+++ b/libar2_hash_buf_size.3
@@ -6,7 +6,7 @@ libar2_hash_buf_size - Determine require output buffer size
 .nf
 #include <libar2.h>
 
-size_t libar2_hash_buf_size(struct libar2_argon2_parameters *\fIparams\fP);
+size_t libar2_hash_buf_size(const struct libar2_argon2_parameters *\fIparams\fP);
 .fi
 .PP
 Link with
@@ -26,10 +26,11 @@ may not be
 .PP
 The return value is
 .I params->hashlen
-if this number is 64 or less, a multiple of 128
-that is around twice as large as
+if this number is 64 or less, otherwise the
+it will be larger than
 .I params->hashlen
-(can be a bit more).
+but no larger than
+.IR "(params->hashlen + 63)" .
 
 .SH RETURN VALUES
 The
diff --git a/libar2_hash_buf_size.c b/libar2_hash_buf_size.c
index 109969f..3010a8a 100644
--- a/libar2_hash_buf_size.c
+++ b/libar2_hash_buf_size.c
@@ -3,13 +3,14 @@
 
 
 size_t
-libar2_hash_buf_size(struct libar2_argon2_parameters *params)
+libar2_hash_buf_size(const struct libar2_argon2_parameters *params)
 {
 	if (params->hashlen <= 64)
 		return params->hashlen;
-	if (params->hashlen > SIZE_MAX / 128 * 64 - 31) {
+	if (params->hashlen > SIZE_MAX - 31 ||
+	    ((params->hashlen + 31) | 31) == SIZE_MAX) {
 		errno = EOVERFLOW;
 		return 0;
 	}
-	return (params->hashlen + 31) / 64 * 128;
+	return ((params->hashlen + 31) | 31) + 1;
 }
diff --git a/test.c b/test.c
index fbabc1c..9a9b3f7 100644
--- a/test.c
+++ b/test.c
@@ -780,6 +780,68 @@ check_libar2_hash(void)
 }
 
 
+#ifdef LIBAR2_WEAKLY_LINKED__
+
+void
+libar2_erase(volatile void *mem, size_t size)
+{
+	(void) mem;
+	(void) size;
+}
+
+static void
+check_libar2_hash_buf_size(void)
+{
+	struct libar2_argon2_parameters params;
+	char pwd[512], output[2049], *doutput;
+	unsigned char salt[LIBAR2_MIN_SALTLEN];
+	size_t size, size0, size1, i;
+	volatile char x, *avoid_code_elimination = &x;
+
+	errno = 0;
+
+	memset(&params, 0, sizeof(params));
+	memset(salt, 0, sizeof(salt));
+	params.saltlen = LIBAR2_MIN_SALTLEN;
+	params.salt = salt;
+	params.m_cost = LIBAR2_MIN_M_COST;
+	params.t_cost = LIBAR2_MIN_T_COST;
+	params.lanes = LIBAR2_MIN_LANES;
+	params.type = LIBAR2_ARGON2I;
+
+	for (params.hashlen = LIBAR2_MIN_HASHLEN; params.hashlen < sizeof(output) - 513; params.hashlen++)  {
+		memset(output, 0, sizeof(output));
+		assert(!libar2_hash(output, pwd, 0, &params, &ctx_st));
+		assert(errno == 0);
+		for (size0 = sizeof(output); size0; size0--)
+			if (output[size0 - 1] != 0)
+				break;
+
+		memset(output, 1, sizeof(output));
+		assert(!libar2_hash(output, pwd, 0, &params, &ctx_st));
+		assert(errno == 0);
+		for (size1 = sizeof(output); size1; size1--)
+			if (output[size1 - 1] != 1)
+				break;
+
+		size = MAX(size0, size1);
+		if (libar2_hash_buf_size(&params) != size || size > params.hashlen + 63)
+			fprintf(stderr, "At hashlen = %zu (expect %zu)\n", params.hashlen, size);
+		assert(size <= params.hashlen + 63);
+		assert_zueq(libar2_hash_buf_size(&params), size);
+
+		doutput = malloc(size);
+		assert(!libar2_hash(doutput, pwd, 0, &params, &ctx_st));
+		assert(errno == 0);
+		for(i = 0; i < params.hashlen; i++)
+			*avoid_code_elimination ^= doutput[i];
+		free(doutput);
+	}
+}
+
+#endif
+
+
 int
 main(void)
 {
@@ -793,5 +855,10 @@ main(void)
 	check_libar2_encode_params_libar2_decode_params();
 	check_libar2_validate_params();
 	check_libar2_hash();
+
+#ifdef LIBAR2_WEAKLY_LINKED__
+	check_libar2_hash_buf_size();
+#endif
+
 	return 0;
 }