fuzz-tests: Add differential test for HMAC-SHA256 #8185
Conversation
Makefile
Outdated
| @@ -283,7 +283,7 @@ ifeq ($(STATIC),1) | |||
| # But that doesn't static link. | |||
| LDLIBS = -L$(CPATH) -Wl,-dn $(SQLITE3_LDLIBS) -Wl,-dy -lm -lpthread -ldl $(COVFLAGS) | |||
| else | |||
| LDLIBS = -L$(CPATH) -lm $(SQLITE3_LDLIBS) $(COVFLAGS) | |||
There was a problem hiding this comment.
Question for the linker experts: does this end up linking CLN against openssl in release builds? If yes, we should reduce the linker arg changes to just impact the fuzz testing.
There was a problem hiding this comment.
Question for the linker experts: does this end up linking CLN against openssl in release builds? If yes, we should reduce the linker arg changes to just impact the fuzz testing.
You're right, that's an error on my part. I should've instead added this line in tests/fuzz/Makefile:
tests/fuzz/fuzz-hmac-sha256: LDLIBS += -lcrypto
which is similar to the linking routine for SHA256 and RIPEMD160 fuzz tests. I'll add this change in the next push.
|
CC: @morehouse |
|
Hm, these fuzz corpora are getting rather large. Is there any way of making them use fewer files, or we can make them optional? They take rather long to materialize on disk, slowing down git considerably, and we run fuzzing mostly on dedicated machines anyway. |
|
@cdecker There's multiple benefits to having the corpora stored publicly.
Of course, the corpora don't need to live inside the CLN repo itself. For example, Bitcoin Core and LND both store their corpora in separate repos. The main benefits of having them co-located with CLN is ease-of-use and better visibility. |
|
Oh so t get me wrong, the corpora should be public, and running should be as simple as possible. My main issue is that were creating and tracking thousands of files, that will not change. I was mostly thinking if there is a way to bundle the corpus into a tarball and track that instead. |
|
The corpora do change though, as new inputs are found. How do you propose we compare and review tarballs when that happens? If you don't want the corpora stored in the CLN repo, I'd recommend creating a separate repo to store them uncompressed. |
tests/fuzz/fuzz-hmac-sha256.c
Outdated
| static const unsigned char hmac_key[] = "fuzz"; | ||
| static const size_t hmac_key_len = sizeof(hmac_key) - 1; |
There was a problem hiding this comment.
It might be worth allowing the fuzzer to choose the key.
tests/fuzz/fuzz-hmac-sha256.c
Outdated
| size_t hash_size; | ||
| EVP_MAC_CTX *ctx; | ||
| OSSL_PARAM params[] = { | ||
| OSSL_PARAM_construct_utf8_string("digest", "SHA256", 0), |
There was a problem hiding this comment.
Does this actually work? I thought the MD algorithm was named SHA-256.
There was a problem hiding this comment.
Apparently SHA-256, SHA2-256 and SHA256 are all aliases to fetch the SHA-256 digest.
Changelog-None: Add a differential fuzz test for HMAC-SHA256, similar to those for SHA256 and RIPEMD160, to verify CCAN’s implementation against OpenSSL’s.
Add a minimal input set as a seed corpus for the newly introduced test. This leads to discovery of interesting code paths faster.
| hmac_key = (unsigned char*) data; | ||
| hmac_key_len = size; |
There was a problem hiding this comment.
This isn't the right way to let the fuzzer choose the key, as the key is no longer independent of the actual data being HMAC'd.
Instead we should peel off some prefix of data as the key before doing the rest of the test, just like we do for num_splits. In this case, I'd suggest using the first byte of data as the size of the key, then peel off that many subsequent bytes as the key itself. Finally peel off num_splits and leave the rest of the data to be HMAC'd.
Add a differential fuzz test for
HMAC-SHA256, similar to thosefor
SHA256andRIPEMD160, to verify CCAN’s implementationagainst OpenSSL’s.