jedisct1 / libhydrogen

A lightweight, secure, easy-to-use crypto library suitable for constrained environments.
https://libhydrogen.org
Other
608 stars 92 forks source link

BLAKE2X: BLAKE2s produces wrong hash value for zero msg #4

Closed aead closed 7 years ago

aead commented 7 years ago

Following test:

static void
test_hash(void)
{
    hydro_hash_state st;
    uint8_t          ctx[8];
    uint8_t          h[32];
    uint8_t          key[hydro_hash_KEYBYTES_MAX];
    uint8_t          msg[1];
    char             hex[32 * 2 + 1];
    size_t           i;

    memset(msg, 0, sizeof msg);
    memset(ctx, 0, sizeof ctx);

    for (i = 0; i < sizeof key; i++) {
        key[i] = (uint8_t) i;
    }
    hydro_bin2hex(hex, sizeof hex, key, sizeof key);    
    assert(
        hydro_equal("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
        hex, sizeof hex));

    hydro_hash_init(&st, ctx, key, sizeof key, sizeof h);   
    hydro_hash_update(&st, msg, 0); // without this update call, the test fails too!
    hydro_hash_final(&st, h, sizeof h);
    hydro_bin2hex(hex, sizeof hex, h, sizeof h);

    // Test vectors 0 from https://blake2.net/blake2s-test.txt 
     assert(
        hydro_equal("48a8997da407876b3d79c0d92325ad3b89cbb754d86ab71aee047ad345fd2c49",
        hex, sizeof hex));

    // hex now contains: "59d9495d33c17c9980c68d5e4b7d1f3ae61c8487ceca0f43531d73d2098f740d"
}

int
main(void)
{
    int ret;

    ret = hydro_init();
    assert(ret == 0);

    test_hash();

    return 0;
}

fails, while this test passes successfully:

static void
test_hash(void)
{
    hydro_hash_state st;
    uint8_t          ctx[8];
    uint8_t          h[32];
    uint8_t          key[hydro_hash_KEYBYTES_MAX];
    uint8_t          msg[1];
    char             hex[32 * 2 + 1];
    size_t           i;

    memset(msg, 0, sizeof msg);
    memset(ctx, 0, sizeof ctx);

    for (i = 0; i < sizeof key; i++) {
        key[i] = (uint8_t) i;
    }
    hydro_bin2hex(hex, sizeof hex, key, sizeof key);    
    assert(
        hydro_equal("000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
        hex, sizeof hex));

    hydro_hash_init(&st, ctx, key, sizeof key, sizeof h);   
    hydro_hash_update(&st, msg, 1); // without this update call, the test fails too!
    hydro_hash_final(&st, h, sizeof h);
    hydro_bin2hex(hex, sizeof hex, h, sizeof h);

    // Test vectors 1 from https://blake2.net/blake2s-test.txt 
     assert(
        hydro_equal("40d15fee7c328830166ac3f918650f807e7e01e177258cdc0a39b11f598066f1",
        hex, sizeof hex));
}
aead commented 7 years ago

The problem is within hydro_hash_init_with_tweak. You're not allowed to update the hash with the key, because if the msg length is 0 the hydro_hash_blake2s_final will compute the wrong hash.

Replacing the key-processing

uint8_t block[hydro_hash_BLOCKBYTES];
memset(block, 0, sizeof block);
mem_cpy(block, key, key_len);
hydro_hash_update(state, block, sizeof block);
hydro_memzero(block, sizeof block);

with:

mem_cpy(state->buf, key, key_len);
state->buf_off = hydro_hash_BLOCKBYTES;

Should fix the problem - of course this requires new test vectors...

jedisct1 commented 7 years ago

Good catch 👍

Your change wouldn't work if the message size was 1 or more blocks.

But https://github.com/jedisct1/libhydrogen/commit/3ed17947e8ebfc6ab25500e28ff956db1db5d03c should fix it.

Thanks!