The sniffer's session, server and secret tables are qualified THREAD_LS_T, so
each thread that calls ssl_InitSniffer*() owns its own. Its trace file, its
mutexes and its crypto device are not: they are shared by the whole process.
ssl_FreeSniffer() released all of it on every call, so in a build such as
snifftest with THREADED_SNIFFTEST, where every worker thread initializes and
frees the sniffer for itself, a thread that finished early closed the trace
file and freed the mutexes under the threads still running. A worker that was
still registering its key then crashed inside ssl_SetNamedEphemeralKey(),
writing to a file another thread had closed:
flockfile -> vfprintf -> fprintf -> ssl_SetNamedEphemeralKey -> load_key
Count the callers of ssl_InitSniffer_ex(), which ssl_InitSniffer() and
ssl_InitSniffer_ex2() both funnel through, and only release the shared state
once the count reaches zero. The count is atomic where the platform offers
atomics and a plain int otherwise, which covers the single threaded build. The
per-thread tables are still freed on every call, and wolfSSL_Cleanup() is
still called every time, since it keeps a count of its own.
Verified with two ssl_InitSniffer() callers and one ssl_FreeSniffer(): before
this the trace file was closed by the early free and everything traced after
it was lost, now both entries are present. The full sniffer test suite passes
with and without THREADED_SNIFFTEST.
snifferWorker() looped while the shutdown flag was clear and only then
drained its queue. Reading a capture file, main() enqueues every packet
and then sets that flag and joins, normally all before the worker thread
is scheduled for the first time, so the worker woke up, saw the flag and
returned without decoding a single packet. Instrumenting the loop shows
it exiting with zero iterations and a non-empty queue, at -O0 as well as
-O1, so this is an ordering race rather than a hoisted load.
Keep going after shutdown for as long as packets are queued. The check
has to be its own helper that takes the worker's semaphore, because a
placeholder head means an empty queue: looping on worker->head alone
would spin forever on a placeholder nothing will ever fill. shutdown and
unused are polled across threads, so mark them volatile.
That alone left -keylogfile broken in this build. The sniffer's server
and secret tables are thread local, so every worker has to repeat the
setup main() did for itself, but the workers only ever called load_key()
and so reported every packet as coming from an unregistered server. Carry
the keylog path in SnifferWorker and load the secrets and create the
keylog sniffer server per worker.
Verified with --enable-sniffer CFLAGS=-DTHREADED_SNIFFTEST: the whole
sniffer test suite passes, and the multi-session captures decode the same
application data at 1, 2, 4 and 8 threads as they do without threading.
Hand the corrected plaintext length to the WOLFSSL_SNIFFER_STORE_DATA_CB
callback. The previous commit fixed the length only for the branch that
copies into the caller's buffer; the callback branch still passed the raw
record size, which spans the explicit IV, the tag or MAC and any padding.
The callback pointer already points at the start of the plaintext, so the
extra bytes run off the end of the decrypt output buffer, which leaves only
a record header of slack. AddressSanitizer reports a heap-buffer-overflow
read of 68 bytes on a TLS 1.2 CBC capture before this change and is clean
after it.
Reject a non-empty encrypt_then_mac extension in the ServerHello. RFC 7366
section 3.1 specifies empty extension_data and TLSX_EncryptThenMac_Parse
enforces it, as do the neighbouring length-constrained cases in the same
switch.
Clear the Encrypt-Then-MAC decision at the start of every ServerHello.
Nothing reset it, so a renegotiation that dropped the extension kept
stripping a MAC that was no longer there, and one that added it started
stripping while the previous cipher was still active.
Reset the decrypted flag at doMessage and gate the padSz subtraction on it.
The flag is set inside the decrypt block and was never cleared, so it
described whichever earlier record in the packet had last been decrypted.
The handshake case already guards the same value with it.
Set the decoded-data flag on the WOLFSSL_ASYNC_CRYPT drain path too. A
record that goes pending is completed inside SnifferAsyncPollQueue rather
than DecodePacket, so an async build could decrypt a capture correctly and
still report that nothing was decrypted. Send that diagnostic to stderr
rather than into the decoded output, and note the exit status in the usage
text so a wrapper is not surprised by it.
Add a TLS 1.2 CBC Encrypt-Then-MAC capture and the sniffer-gen.sh recipe
that produces it, so the fix has a regression test that can be regenerated.
The existing static RSA and IPv6 captures exercise the same path but no
script in the tree can rebuild them. The new capture covers both failure
modes: with an HMAC-SHA1 suite the old code fails the decrypt outright,
and with HMAC-SHA256, whose MAC is a multiple of the block size, it
silently decrypts the wrong byte range and emits 48 bytes where 14 are
correct.
Warn when the Encrypt-Then-MAC gate drops the TLS 1.2 CBC captures, so a
build configuration that loses coverage says so, and regenerate the keylog
reference output with a binary that carries the new feature token.
The sniffer ignored the encrypt_then_mac extension (RFC 7366) in the
ServerHello. For a CBC suite that meant the trailing MAC was handed to the
block decrypt along with the ciphertext, so the length was not a multiple of
the block size and every record failed with BAD_FUNC_ARG. wolfSSL peers
negotiate Encrypt-Then-MAC by default, so no TLS 1.2 CBC capture taken from
one could be decrypted. Record the extension and remove the MAC before
decrypting.
The plaintext length returned to the caller, printed by snifftest as
SSL App Data(packet:length), came from the record size without subtracting
what DecryptMessage() had already worked out in ssl->keys.padSz. A 14 byte
payload was reported as 30 under TLS 1.2 AES-GCM and as 31 under TLS 1.3.
Subtract padSz so the sniffer removes the same bytes the ordinary read path
removes from ssl->curSize.
Both failures were invisible because the tests could not fail. snifftest
overwrote hadBadPacket on every packet instead of accumulating it, so errors
early in a capture were erased by later good packets and the process still
exited 0. The keylog leg read $? after a pipe into tee, which reports tee's
status rather than snifftest's. Accumulate the flag, use PIPESTATUS, and
treat a saved capture that yields no plaintext at all as a failure.
The two static RSA captures decode correctly again and the keylog reference
output is regenerated from the checked-in pcaps, which had drifted by a
packet since the files were last produced on 5.6.3.
The X25519 legs are skipped: both captures are in the tree as a bare 24 byte
pcap header with no packets. sniffer-gen.sh could never produce them because
its configure line for those two builds without ed25519, so the example
client and server fail to load their certificates and tcpdump writes an empty
file. Add the missing option, and pin the group with --disable-mlkem so the
capture negotiates X25519 rather than the X25519MLKEM768 hybrid the static
key does not match. Regenerating them is left out of this change: the sniffer
cannot yet derive the shared secret for an X25519 static ephemeral key, which
is a separate defect.
Detect reading of packet errors
--enable-all and --enable-sniffer exposed this issue
Don't need variable
Rework argument parsing
Need a way to allow arguments to be supplied more granularly. Partucilarly, I needed a "-tracefile" argument without requiring the use of a PCAP file
Fix error prints to STDERR
Fix setting of port filtering
Fix 80 char limit
Not actually a bad packet when there are no more packets
Fix strcat size
Allow the sniffer to print the trace to STDOUT
Fix indexing
Take out superfluous error which is handled later
Set default port to 11111
Single return point
Combine chain to one contiguous memory block
Fix return
Add in error handling for XMALLOC
Add in debugging output when --enable-debug
It makes no sense to allocate a ton of small buffers to process chains
Ultimately, the code is slower because of the several small memcpy instead of a single large contiguous memcpy
Pass in a device name
Fix unused variable
Fix cast
Addressing PR comments
Add new flags to --help
* Sniffer: Remove old restrictions for max strength, encrypt-then-mac and forcing openssl-extra.
* Fix bound warning with strncpy in sniffer.c.
* Fix for async DH issue.
* Fix for SP math all not initializing raw big int.
* Fix for array bounds warning with "-O3" on SetEccPublicKey.
* Fix a sniffer async edge case with TLS v1.2 static RSA and extended master.
* Improved the sniffer test script detection of features.
* Disable ECC custom curve test with Intel QuickAssist.
* Adds stateful handling of DH shared secret computation in `SetupKeys`.
* Improved the decrypt handling to use internal functions and avoid generating alerts on failures.
* Fix for sniffer resume due to missing `sessionIDSz` broken in #4807.
* Fix sniffer test cases to split resume (session_ticket) tests.
* Add `snifftest` list of build features so test script can gate running resume test.
Currently, wolfssl.vcxproj and IDE/WIN10/wolfssl-fips.vcxproj do not use the
same scheme for their output and intermediate directories. Further, across
configuration/platform combinations, wolfssl.vcxproj isn't consistent, either.
For example:
```
Release|x64
OutDir: $(SolutionDir)$(Platform)\$(Configuration)\
IntDir: $(Platform)\$(Configuration)\obj\
Release|Win32
OutDir: $(SolutionDir)$(Configuration)\
IntDir: $(Configuration)\obj\
```
This commit makes every configuration/platform combo for all Visual Studio
projects follow the same pattern:
```
OutDir: $(SolutionDir)$(Platform)\$(Configuration)\
IntDir: $(Configuration)\$(Platform)\$(ProjectName)_obj\
```
The `$(ProjectName)_obj` piece gets rid of a Visual Studio warning about not
mingling the intermediate objects of disparate builds.
