Skip to content

Commit 17e21ee

Browse files
committed
blake2b: compress and wipe keys immediately
Changes keyed hashing such that: - Keys are not copied into a temporary 128 byte stack buffer, nor the hash state buffer - Keys are compressed immediately during initialization, not during `blake2b_final` or `blake2b_update` - Internal calculation vectors `m` and `v` are wiped - Caller of `blake2b_init_key` must commit to whether non-key data exists or not These changes should make keyed Blake2b hashing more memory secure. Also, for optimization, we use 1 `memcpy` call for regular copying into message vector `m` instead of 16 `load64` calls if we're on a little-endian system.
1 parent 9866a0e commit 17e21ee

File tree

2 files changed

+64
-19
lines changed

2 files changed

+64
-19
lines changed

src/crypto/blake2b.c

Lines changed: 62 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,38 @@ int blake2b_init(blake2b_state *S, size_t outlen) {
286286
return blake2b_init_param(S, &P);
287287
}
288288

289-
int blake2b_init_key(blake2b_state *S, size_t outlen, const void *key, size_t keylen) {
289+
/* Fwd */
290+
static void blake2b_compress_mv(blake2b_state *S, const uint64_t *m, uint64_t * restrict v);
291+
292+
/**
293+
* The difference from `blake2b_compress()` is that this function directly takes
294+
* a variable-sized input, treating it as a block where the remaining bytes are
295+
* zeros, and also wipes stack variables `m` and `v`. The intended use case is
296+
* initializing hash states with keys, hence the stricter memory handling. I
297+
* don't attempt to prevent swapping of the variables `m` and `v` since the OS
298+
* would have to be dumber than a box of rocks to swap stack variables used just
299+
* microseconds ago. `keylen` is assumed to be less than or equal to 128.
300+
*/
301+
static void blake2b_compress_key(blake2b_state *S, const uint8_t *key, size_t keylen) {
302+
uint64_t m[16];
303+
uint64_t v[16];
304+
305+
memset(m, 0, sizeof(m));
306+
#if defined(NATIVE_LITTLE_ENDIAN)
307+
memcpy(m, key, keylen);
308+
#else
309+
for (;keylen--;) {
310+
m[keylen / 8] |= (((uint64_t) key[keylen]) << ((keylen % 8) * 8));
311+
}
312+
#endif
313+
314+
blake2b_compress_mv(S, m, v);
315+
316+
clear_internal_memory(m, sizeof(m));
317+
clear_internal_memory(v, sizeof(v));
318+
}
319+
320+
int blake2b_init_key(blake2b_state *S, size_t outlen, const void *key, size_t keylen, int has_data) {
290321
blake2b_param P;
291322

292323
if (S == NULL) {
@@ -321,25 +352,34 @@ int blake2b_init_key(blake2b_state *S, size_t outlen, const void *key, size_t ke
321352
return -1;
322353
}
323354

324-
{
325-
uint8_t block[BLAKE2B_BLOCKBYTES];
326-
memset(block, 0, BLAKE2B_BLOCKBYTES);
327-
memcpy(block, key, keylen);
328-
blake2b_update(S, block, BLAKE2B_BLOCKBYTES);
329-
/* Burn the key from stack */
330-
clear_internal_memory(block, BLAKE2B_BLOCKBYTES);
355+
blake2b_increment_counter(S, BLAKE2B_BLOCKBYTES);
356+
if (!has_data) {
357+
blake2b_set_lastblock(S);
331358
}
359+
blake2b_compress_key(S, key, keylen);
360+
S->skip_final_compress = 1;
361+
332362
return 0;
333363
}
334364

335365
static void blake2b_compress(blake2b_state *S, const uint8_t *block) {
336366
uint64_t m[16];
337367
uint64_t v[16];
338-
unsigned int i, r;
339368

369+
#if defined(NATIVE_LITTLE_ENDIAN)
370+
memcpy(m, block, BLAKE2B_BLOCKBYTES);
371+
#else
372+
unsigned int i;
340373
for (i = 0; i < 16; ++i) {
341374
m[i] = load64(block + i * sizeof(m[i]));
342375
}
376+
#endif
377+
378+
blake2b_compress_mv(S, m, v);
379+
}
380+
381+
static void blake2b_compress_mv(blake2b_state *S, const uint64_t *m, uint64_t * restrict v) {
382+
unsigned int i, r;
343383

344384
for (i = 0; i < 8; ++i) {
345385
v[i] = S->h[i];
@@ -427,6 +467,7 @@ int blake2b_update(blake2b_state *S, const void *in, size_t inlen) {
427467
}
428468
memcpy(&S->buf[S->buflen], pin, inlen);
429469
S->buflen += (unsigned int)inlen;
470+
S->skip_final_compress = 0;
430471
return 0;
431472
}
432473

@@ -439,15 +480,18 @@ int blake2b_final(blake2b_state *S, void *out, size_t outlen) {
439480
return -1;
440481
}
441482

442-
/* Is this a reused state? */
443-
if (S->f[0] != 0) {
444-
return -1;
445-
}
483+
if (!S->skip_final_compress) {
484+
/* Is this a reused state? */
485+
if (S->f[0] != 0) {
486+
return -1;
487+
}
446488

447-
blake2b_increment_counter(S, S->buflen);
448-
blake2b_set_lastblock(S);
449-
memset(&S->buf[S->buflen], 0, BLAKE2B_BLOCKBYTES - S->buflen); /* Padding */
450-
blake2b_compress(S, S->buf);
489+
blake2b_increment_counter(S, S->buflen);
490+
blake2b_set_lastblock(S);
491+
memset(&S->buf[S->buflen], 0, BLAKE2B_BLOCKBYTES - S->buflen); /* Padding */
492+
blake2b_compress(S, S->buf);
493+
}
494+
S->skip_final_compress = 0;
451495

452496
for (i = 0; i < 8; ++i) { /* Output full hash to temp buffer */
453497
store64(buffer + sizeof(S->h[i]) * i, S->h[i]);
@@ -479,7 +523,7 @@ int blake2b(void *out, size_t outlen, const void *in, size_t inlen,
479523
}
480524

481525
if (keylen > 0) {
482-
if (blake2b_init_key(&S, outlen, key, keylen) < 0) {
526+
if (blake2b_init_key(&S, outlen, key, keylen, inlen != 0) < 0) {
483527
goto fail;
484528
}
485529
}

src/crypto/blake2b.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ extern "C" {
7676
unsigned buflen;
7777
unsigned outlen;
7878
uint8_t last_node;
79+
uint8_t skip_final_compress;
7980
} blake2b_state;
8081

8182
/* Ensure param structs have not been wrongly padded */
@@ -98,7 +99,7 @@ extern "C" {
9899
/* Streaming API */
99100
int blake2b_init(blake2b_state *S, size_t outlen);
100101
int blake2b_init_key(blake2b_state *S, size_t outlen, const void *key,
101-
size_t keylen);
102+
size_t keylen, int has_data);
102103
int blake2b_init_param(blake2b_state *S, const blake2b_param *P);
103104
int blake2b_update(blake2b_state *S, const void *in, size_t inlen);
104105
int blake2b_final(blake2b_state *S, void *out, size_t outlen);

0 commit comments

Comments
 (0)