Conversation
0675b5b to
7ff102f
Compare
e7ecb19 to
38d93ae
Compare
…r parsing - Add `Fuzzing` package trait wiring fuzzer/ASan flags and two fuzz targets (`FuzzQUICPackets`, `FuzzTransportParameters`), plus CI and nightly workflow integration with corpus caching - Add no-op `SecFramerNull` protector (FuzzBuild only) so fuzz inputs are treated as already-plaintext on the receive path - Stop processing coalesced frames/packets once the connection is terminal, add `QUICFrame.discard`, guard missing TLS options on INITIAL retransmission, and clear crypto reassembly queues on stop
| ) throws(QUICError) | ||
| } | ||
|
|
||
| #if Fuzzing |
There was a problem hiding this comment.
What do you think of this broken out into its own file Protector+SecFramerNull.swift or Protector+Fuzzing.swift
| enum KeyType: Equatable { | ||
| case aesGCM | ||
| case chaChaPoly | ||
| #if Fuzzing |
There was a problem hiding this comment.
This causes lots of #if statements throughout the code which I think in general makes a codebase hard to follow and reason about. I'm wondering if we could get away with unconditionally defining the .null case and having strategic fatalErrors or similar which trigger if you use it in a non-fuzzing context. Maybe using an inlinable assertFuzzing() call with an #if !Fuzzing in it?
There was a problem hiding this comment.
I really didn't want to compile it in !Fuzzing because it's somewhat dangerous to have this option.
There was a problem hiding this comment.
Can you expand on that? I'm not sure I follow.
There was a problem hiding this comment.
I want the null fuzzer to never be compiled into production or tests. Only when we are fuzzing. I'm probably splitting hairs but not having the enum available outside of Fuzzing is a bit safer than fatalError().
| } | ||
|
|
||
| if closeError != nil { | ||
| if closeError != nil || state.isTerminal { |
There was a problem hiding this comment.
Is this a bug which fuzzing surfaced? Are some of those included in this PR?
There was a problem hiding this comment.
Yes, I'm moving these fixes to #157 (review)
Fuzzingpackage trait wiring fuzzer/ASan flags and two fuzz targets (FuzzQUICPackets,FuzzTransportParameters), plus CI and nightly workflow integration with corpus cachingSecFramerNullprotector (FuzzBuild only) so fuzz inputs are treated as already-plaintext on the receive pathQUICFrame.discard, guard missing TLS options on INITIAL retransmission, and clear crypto reassembly queues on stop