-
Notifications
You must be signed in to change notification settings - Fork 574
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
[TLS 1.3] Fuzz Target for Handshake Message Parsing #2977
Conversation
The fuzzer configuration is rather opaque to us, we're guessing it's somewhere in https://oss-fuzz.com/ to which we don't have access. Ideally the seed would change between runs, it's currently always 1337. In any case - we'd like to speed up the fuzzers coverage, so we added a corpus over at randombit/crypto-corpus#1 which oss-fuzz should then be able to pick up here: https://github.com/google/oss-fuzz/blob/master/projects/botan/Dockerfile#L20 Regarding our local test setup, we had to do a little workaround in order to run libFuzzer: ./configure.py --compiler-cache=ccache --cc=clang --cc-bin=/usr/local/opt/llvm@13/bin/clang++ --build-fuzzer=libfuzzer2 --unsafe-fuzzer-mode --enable-sanitizers=coverage,address,undefined,fuzzer
make -j$(nproc) fuzzers
DYLD_LIBRARY_PATH=. build/fuzzer/tls_13_handshake_layer msg_corpus Notes:
We also visualized the coverage of the corpus (without additional fuzzing) via mkdir coverage
./configure.py --compiler-cache=ccache --cc=clang --cc-bin=/usr/local/opt/llvm@13/bin/clang++ --build-fuzzer=libfuzzer2 --unsafe-fuzzer-mode --enable-sanitizers=coverage,address,undefined,fuzzer --with-coverage-info
make -j$(nproc) fuzzers
DYLD_LIBRARY_PATH=. build/fuzzer/tls_13_handshake_layer -runs=0 msg_corpus
gcovr --html --html-details --object-directory=build/obj/lib -r . -o coverage/coverage.html
open coverage/coverage.html See google/fuzzing#41 (comment) for details. |
Codecov Report
@@ Coverage Diff @@
## master #2977 +/- ##
==========================================
- Coverage 92.57% 92.53% -0.04%
==========================================
Files 596 595 -1
Lines 69729 69713 -16
Branches 6613 6598 -15
==========================================
- Hits 64552 64512 -40
- Misses 5144 5168 +24
Partials 33 33
Continue to review full report at Codecov.
|
Rebased to master. |
Co-Authored-By: Lukas Zeller <[email protected]>
Botan::TLS::Handshake_Layer prepare(const std::vector<uint8_t>& data) | ||
{ | ||
Botan::TLS::Handshake_Layer hl(Botan::TLS::Connection_Side::CLIENT); | ||
hl.copy_data(data); | ||
return hl; | ||
} | ||
|
||
} // namespace; | ||
|
||
|
||
void fuzz(const uint8_t in[], size_t len) | ||
{ | ||
static Botan::TLS::Default_Policy policy; | ||
|
||
try | ||
{ | ||
std::vector<uint8_t> v(in, in + len); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@reneme I'm unable to push the following change (403) to the vector's type:
Botan::TLS::Handshake_Layer prepare(const std::vector<uint8_t>& data) | |
{ | |
Botan::TLS::Handshake_Layer hl(Botan::TLS::Connection_Side::CLIENT); | |
hl.copy_data(data); | |
return hl; | |
} | |
} // namespace; | |
void fuzz(const uint8_t in[], size_t len) | |
{ | |
static Botan::TLS::Default_Policy policy; | |
try | |
{ | |
std::vector<uint8_t> v(in, in + len); | |
Botan::TLS::Handshake_Layer prepare(const Botan::secure_vector<uint8_t>& data) | |
{ | |
Botan::TLS::Handshake_Layer hl(Botan::TLS::Connection_Side::CLIENT); | |
hl.copy_data(data); | |
return hl; | |
} | |
} // namespace; | |
void fuzz(const uint8_t in[], size_t len) | |
{ | |
static Botan::TLS::Default_Policy policy; | |
try | |
{ | |
Botan::secure_vector<uint8_t> v(in, in + len); |
Can you approve the suggestion so the build can hopefully pass? Alternately I can open a separate PR, but that feels overkill.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to open a new PR anyway, the fuzzer now works as intended: #3013
I suggest closing this PR (I apparently can't do this myself) in favor of the other one, because (a) the other one is ready to merge (b) I can rebase the other PR if something else changes in the meantime.
This is a basic fuzz target that shoves the fuzzer input into
Handshake_Layer_13::copy_data()
and tries to parse the messages. We've seen fairly good coverage of the handshake message parsing code with a seed corpus based on the RFC 8448 handshake messages.We weren't able to figure out how to integrate this target into the OSS-Fuzz run, though.OSS-Fuzz seems to pick up the new target but (obviously) fails to find a seed corpus for it.--terminate-on-asserts
in the OSS-Fuzz build step once this is mergedFor CI convenience we left the base commits of the TLS 1.3 PR in here. The relevant fuzz target can be found in this commit: 3151d0d