-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sha256 digests do not match test vectors #4
Comments
What system are you building on? I'm using openSUSE 15.4 and I don't get any errors (using version
|
I'm on openSUSE Tumbleweed. Using the current HEAD,
The test suite error log is:
I've run the same tests and test-suite inside an Ubuntu 23.04 VM and container on my laptop, as well as on my openSUSE Leap 15.4 server (both on the host and in an Ubuntu container). It seems that on openSUSE Leap 15.4 the test suite passes but if you run in inside a Ubuntu 23.04 or openSUSE Tumbleweed container or VM it fails. This error also shows up with the properly packaged I looked into this a bit deeper last night -- this code comes from Aaron Gifford and it seems that there was a sha256 hashing bug in version 1.0.0beta1 (which appears to be the version used by
I was staring at diffs between the two codebases last night but nothing obvious stood out -- there were a few changes to some of the hashing code but nothing that looks like the "off by one" referenced in the changelog. It's possible the code in NetBSD was already updated with the fix and they didn't update the copyright header mentioning the version... |
It seems like this is related to the different GCC versions being used. Leap 15.4 has GCC 7, but Tumbleweed and Ubuntu 23.04 have newer versions. If you install all of the intermediate versions on your Leap machine (I did this in a container), you'll note that the tests start failing with GCC 11:
Interestingly, the following warnings are new from GCC 11 (but I don't think they're related, the two ways of expressing an "array" parameter in C are equivalent):
I also tried all the versions of clang available on Leap (clang 7, 9, 11, 13, and 15), and it seems that this is a GCC-specific issue. I guess this implies it's either some kind of compiler bug, or there is some undefined behaviour in the sha256 code that changes the behaviour on newer GCC versions. |
Building with |
Wow, if you actually found a compiler bug then you get a gold star! Hmm. I was able to reproduce the problem at first using gcc-11, but somehow now I'm having this problem trying to even run the tests:
The problem seems to go away when I revert this change: diff --git a/Makefile.am b/Makefile.am
index aeab142..f29608f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -84,7 +84,7 @@ noinst_HEADERS= private/cclass.h \
private/test-helpers.h \
private/utils.h
-noinst_SOURCES = private/cavs2c.awk
+noinst_SCRIPTS = private/cavs2c.awk
if WITH_DB
|
Ah my bad, it's because it typo'd the actual filename in the version I checked in while locally I had two copies... I've forwarded the issue to the SUSE compiler people to see if they have an idea of what might be going on... |
Ah, it seems the issue is with how |
Using diff --git a/sha2.c b/sha2.c
index bdcbd31..5aad3bd 100644
--- a/sha2.c
+++ b/sha2.c
@@ -567,7 +567,7 @@ void SHA256_Final(sha2_byte digest[SHA256_DIGEST_LENGTH], SHA256_CTX* context) {
*context->buffer = 0x80;
}
/* Set the bit count: */
- *(sha2_word64*)(void *)&context->buffer[SHA256_SHORT_BLOCK_LENGTH] = context->bitcount;
+ memcpy(&context->buffer[SHA256_SHORT_BLOCK_LENGTH], &context->bitcount, sizeof(context->bitcount));
/* Final transform: */
SHA256_Transform(context, (sha2_word32*)(void *)context->buffer);
@@ -870,8 +870,8 @@ static void SHA512_Last(SHA512_CTX* context) {
*context->buffer = 0x80;
}
/* Store the length of input data (in bits): */
- *(sha2_word64*)(void *)&context->buffer[SHA512_SHORT_BLOCK_LENGTH] = context->bitcount[1];
- *(sha2_word64*)(void *)&context->buffer[SHA512_SHORT_BLOCK_LENGTH+8] = context->bitcount[0];
+ memcpy(&context->buffer[SHA512_SHORT_BLOCK_LENGTH], &context->bitcount[1], sizeof(context->bitcount[1]));
+ memcpy(&context->buffer[SHA512_SHORT_BLOCK_LENGTH+8], &context->bitcount[0], sizeof(context->bitcount[0]));
/* Final transform: */
SHA512_Transform(context, (sha2_word64*)(void *)context->buffer); |
Should be fixed by that patch in 864c1cf. Reopen if you find otherwise. Thanks for detecting and tracking this one down! |
&context->buffer is uint8_t*, but we try to access it as sha2_word64*, which is an aliasing violation (undefined behaviour). Use memcpy instead to avoid being miscompiled by e.g. >= GCC 12. Bug: https://gcc.gnu.org/PR114698 Bug: NetBSD/pkgsrc#122 Bug: archiecobbs/libnbcompat#4
`&context->buffer` is `uint8_t*`, but we try to access it as `sha2_word64*`, which is an aliasing violation (undefined behaviour). Use memcpy instead to avoid being miscompiled by e.g. >= GCC 12. This is just as fast with any modern compiler. Bug: https://gcc.gnu.org/PR114698 Bug: NetBSD/pkgsrc#122 Bug: archiecobbs/libnbcompat#4 Signed-off-by: Sam James <sam@gentoo.org>
`&context->buffer` is `uint8_t*`, but we try to access it as `sha2_word64*`, which is an aliasing violation (undefined behaviour). Use memcpy instead to avoid being miscompiled by e.g. >= GCC 12. This is just as fast with any modern compiler. Bug: https://gcc.gnu.org/PR114698 Bug: NetBSD/pkgsrc#122 Bug: archiecobbs/libnbcompat#4 Bug: https://bugs.launchpad.net/ubuntu-power-systems/+bug/2033405 Signed-off-by: Sam James <sam@gentoo.org>
`&context->buffer` is `uint8_t*`, but we try to access it as `sha2_word64*`, which is an aliasing violation (undefined behaviour). Use memcpy instead to avoid being miscompiled by e.g. >= GCC 12. This is just as fast with any modern compiler. Bug: https://gcc.gnu.org/PR114698 Bug: NetBSD/pkgsrc#122 Bug: archiecobbs/libnbcompat#4 Bug: https://bugs.launchpad.net/ubuntu-power-systems/+bug/2033405 Signed-off-by: Sam James <sam@gentoo.org>
`&context->buffer` is `uint8_t*`, but we try to access it as `sha2_word64*`, which is an aliasing violation (undefined behaviour). Use memcpy instead to avoid being miscompiled by e.g. >= GCC 12. This is just as fast with any modern compiler. Bug: https://gcc.gnu.org/PR114698 Bug: NetBSD/pkgsrc#122 Bug: archiecobbs/libnbcompat#4 Bug: https://bugs.launchpad.net/ubuntu-power-systems/+bug/2033405 Signed-off-by: Sam James <sam@gentoo.org>
As part of debugging archiecobbs/nmtree#4, I have confirmed that the issue is that some part of the SHA256 hashing code in this library produces output that doesn't match any of the test vectors.
However, if you enable
SHA2_UNROLL_TRANSFORM
, the output is fixed. So there's some kind of bug in the non-unrolled transform. In archiecobbs/nmtree#4, I speculated that it was a porting bug, but there are no substantial changes from NetBSD to this port, so it seems possible this is a NetBSD bug (maybe they always useSHA2_UNROLL_TRANSFORM
and thus have never hit this issue?).I've opened #3 to add tests that check the output of the library implementations against known-good test vectors. If you run
make check
you'll find that all of the SHA256 test vectors fail, but if you runmake CFLAGS="-DSHA2_UNROLL_TRANSFORM" check
all of the tests pass.The text was updated successfully, but these errors were encountered: