Possible incorrect sha256 values for 64 bytes buffers

wlou commented

2025-01-30 13:29:22 -06:00

(Migrated from github.com)

Hi,

There seem to be a bug that leads to incorrect sha256 values for 64 bytes buffers.

The broken sequence: submit(64 bytes, ISAL_HASH_FIRST) -> submit(0 bytes, ISAL_HASH_LAST) -> flush() -> Wrong Hash
The working one: submit(0 bytes, ISAL_HASH_FIRST) -> submit(64 bytes, ISAL_HASH_LAST) -> flush() -> Correct Hash
I'd expect the value to be the same and correct regardless of the permutation.

Environment:

➜  isa-l_crypto git:(master) ✗ uname -a
Linux DESKTOP-6315RI1 5.15.167.4-microsoft-standard-WSL2 #1 SMP Tue Nov 5 00:21:55 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
➜  isa-l_crypto git:(master) ✗ gcc -v
Target: x86_64-linux-gnu
gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04)

Minimal reproducible example:

main.c

void
correct(void *buffer, uint32_t size)
{
        ISAL_SHA256_HASH_CTX_MGR manager;
        isal_sha256_ctx_mgr_init(&manager);

        ISAL_SHA256_HASH_CTX in, *out = NULL;
        isal_hash_ctx_init(&in);

        isal_sha256_ctx_mgr_submit(&manager, &in, &out, NULL, 0, ISAL_HASH_FIRST);
        isal_sha256_ctx_mgr_submit(&manager, &in, &out, buffer, size, ISAL_HASH_LAST);
        isal_sha256_ctx_mgr_flush(&manager, &out);

        printf("Correct: ");
        for (int i = 0; i < SHA256_DIGEST_NWORDS; i++) {
                printf("%08x", in.job.result_digest[i]);
        }
        printf("\n");
}

void
wrong(void *buffer, uint32_t size)
{
        ISAL_SHA256_HASH_CTX_MGR manager;
        isal_sha256_ctx_mgr_init(&manager);

        ISAL_SHA256_HASH_CTX in, *out = NULL;
        isal_hash_ctx_init(&in);

        isal_sha256_ctx_mgr_submit(&manager, &in, &out, buffer, size, ISAL_HASH_FIRST);
        isal_sha256_ctx_mgr_submit(&manager, &in, &out, NULL, 0, ISAL_HASH_LAST); // <----- seems here the padded block is not considered
        isal_sha256_ctx_mgr_flush(&manager, &out);

        printf("Wrong:   ");
        for (int i = 0; i < SHA256_DIGEST_NWORDS; i++) {
                printf("%08x", in.job.result_digest[i]);
        }
        printf("\n");
}

int
main()
{
        uint8_t buffer[64];
        memset(buffer, 'a', sizeof(buffer));
        printf("Buffer:  ");
        for (int i = 0; i < sizeof(buffer); i++) {
                printf("%c", buffer[i]);
        }
        printf("\n");

        correct(buffer, sizeof(buffer));
        wrong(buffer, sizeof(buffer));
}

output

➜  isa-l_crypto git:(master) ✗ gcc -I$(pwd)/include -L$(pwd)/lib -o demo main.c -lisal_crypto
➜  isa-l_crypto git:(master) ✗ LD_LIBRARY_PATH=$(pwd)/lib ./demo                  
Buffer:  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
Correct: ffe054fe7ae0cb6dc65c3af9b61d5209f439851db43d0ba5997337df154668eb
Wrong:   df5bb81ce81e0626fb45a8944fd40f31b25e6816d6d499c1ab90492900635e66

Hi, There seem to be a bug that leads to incorrect sha256 values for 64 bytes buffers. The broken sequence: `submit(64 bytes, ISAL_HASH_FIRST) ` -> `submit(0 bytes, ISAL_HASH_LAST)` -> `flush()` -> `Wrong Hash` The working one: `submit(0 bytes, ISAL_HASH_FIRST)` -> `submit(64 bytes, ISAL_HASH_LAST)` -> `flush()` -> `Correct Hash` I'd expect the value to be the same and correct regardless of the permutation. Environment: ```sh ➜ isa-l_crypto git:(master) ✗ uname -a Linux DESKTOP-6315RI1 5.15.167.4-microsoft-standard-WSL2 #1 SMP Tue Nov 5 00:21:55 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux ➜ isa-l_crypto git:(master) ✗ gcc -v Target: x86_64-linux-gnu gcc version 13.3.0 (Ubuntu 13.3.0-6ubuntu2~24.04) ``` Minimal reproducible example: - _main.c_ ```c void correct(void *buffer, uint32_t size) { ISAL_SHA256_HASH_CTX_MGR manager; isal_sha256_ctx_mgr_init(&manager); ISAL_SHA256_HASH_CTX in, *out = NULL; isal_hash_ctx_init(&in); isal_sha256_ctx_mgr_submit(&manager, &in, &out, NULL, 0, ISAL_HASH_FIRST); isal_sha256_ctx_mgr_submit(&manager, &in, &out, buffer, size, ISAL_HASH_LAST); isal_sha256_ctx_mgr_flush(&manager, &out); printf("Correct: "); for (int i = 0; i < SHA256_DIGEST_NWORDS; i++) { printf("%08x", in.job.result_digest[i]); } printf("\n"); } void wrong(void *buffer, uint32_t size) { ISAL_SHA256_HASH_CTX_MGR manager; isal_sha256_ctx_mgr_init(&manager); ISAL_SHA256_HASH_CTX in, *out = NULL; isal_hash_ctx_init(&in); isal_sha256_ctx_mgr_submit(&manager, &in, &out, buffer, size, ISAL_HASH_FIRST); isal_sha256_ctx_mgr_submit(&manager, &in, &out, NULL, 0, ISAL_HASH_LAST); // <----- seems here the padded block is not considered isal_sha256_ctx_mgr_flush(&manager, &out); printf("Wrong: "); for (int i = 0; i < SHA256_DIGEST_NWORDS; i++) { printf("%08x", in.job.result_digest[i]); } printf("\n"); } int main() { uint8_t buffer[64]; memset(buffer, 'a', sizeof(buffer)); printf("Buffer: "); for (int i = 0; i < sizeof(buffer); i++) { printf("%c", buffer[i]); } printf("\n"); correct(buffer, sizeof(buffer)); wrong(buffer, sizeof(buffer)); } ``` - _output_ ```sh ➜ isa-l_crypto git:(master) ✗ gcc -I$(pwd)/include -L$(pwd)/lib -o demo main.c -lisal_crypto ➜ isa-l_crypto git:(master) ✗ LD_LIBRARY_PATH=$(pwd)/lib ./demo Buffer: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa Correct: ffe054fe7ae0cb6dc65c3af9b61d5209f439851db43d0ba5997337df154668eb Wrong: df5bb81ce81e0626fb45a8944fd40f31b25e6816d6d499c1ab90492900635e66 ```

pablodelara commented

2025-01-31 07:14:42 -06:00

(Migrated from github.com)

Hi @wlou. Thanks for the detailed explanation of this sighting. In order to make it work, regardless the order, you need to flush after each submit call, unless the submit function returns the ctx ('out' in your case).

Check out the explanation in sha256_mb.h (at the top).



 * Each ISAL_SHA256_HASH_CTX must be initialized before first use by the isal_hash_ctx_init macro
 * defined in multi_buffer.h. After initialization, the application may begin computing
 * a hash by giving the ISAL_SHA256_HASH_CTX to a ISAL_SHA256_HASH_CTX_MGR using the submit
 * functions sha256_ctx_mgr_submit*() with the ISAL_HASH_FIRST flag set. When the
 * ISAL_SHA256_HASH_CTX is returned to the application (via this or a later call to
 * sha256_ctx_mgr_submit*() or sha256_ctx_mgr_flush*()), the application can then re-submit it with
 * another call to sha256_ctx_mgr_submit*(), but without the ISAL_HASH_FIRST flag set.

Hi @wlou. Thanks for the detailed explanation of this sighting. In order to make it work, regardless the order, you need to flush after each submit call, unless the submit function returns the ctx ('out' in your case). Check out the explanation in sha256_mb.h (at the top). ``` * Each ISAL_SHA256_HASH_CTX must be initialized before first use by the isal_hash_ctx_init macro * defined in multi_buffer.h. After initialization, the application may begin computing * a hash by giving the ISAL_SHA256_HASH_CTX to a ISAL_SHA256_HASH_CTX_MGR using the submit * functions sha256_ctx_mgr_submit*() with the ISAL_HASH_FIRST flag set. When the * ISAL_SHA256_HASH_CTX is returned to the application (via this or a later call to * sha256_ctx_mgr_submit*() or sha256_ctx_mgr_flush*()), the application can then re-submit it with * another call to sha256_ctx_mgr_submit*(), but without the ISAL_HASH_FIRST flag set. ```

wlou commented

2025-01-31 10:04:24 -06:00

(Migrated from github.com)

Thanks for quick reply @pablodelara ! Got it, please feel free to close the issue.

Tested the following and it works as expected:

submit(0 bytes, ISAL_HASH_FIRST) -> flush() -> submit(64 bytes, ISAL_HASH_LAST) -> flush() -> Correct
submit(64 bytes, ISAL_HASH_FIRST) -> flush() -> submit(0 bytes, ISAL_HASH_LAST) -> flush() -> Correct

Just to clarify, is it correct to assume that redundant flush() calls are safe?

Thanks for quick reply @pablodelara ! Got it, please feel free to close the issue. Tested the following and it works as expected: 1. `submit(0 bytes, ISAL_HASH_FIRST)` -> `flush()` -> `submit(64 bytes, ISAL_HASH_LAST)` -> `flush()` -> Correct 2. `submit(64 bytes, ISAL_HASH_FIRST) ` -> `flush()` -> `submit(0 bytes, ISAL_HASH_LAST)` -> `flush()` -> Correct Just to clarify, is it correct to assume that redundant `flush()` calls are safe?

pablodelara commented

2025-01-31 10:16:39 -06:00

(Migrated from github.com)

Yes, redundant flush()s are safe. "Worse" case, nothing gets out and ctx is always NULL.

Yes, redundant `flush()`s are safe. "Worse" case, nothing gets out and ctx is always NULL.

Rows
Columns

Possible incorrect sha256 values for 64 bytes buffers #156