TLS KeyShare dissection boundary, remove conf.debug_dissector, and import test infrastructure#4957
Open
ali-keys wants to merge 3 commits intosecdev:masterfrom
Open
TLS KeyShare dissection boundary, remove conf.debug_dissector, and import test infrastructure#4957ali-keys wants to merge 3 commits intosecdev:masterfrom
ali-keys wants to merge 3 commits intosecdev:masterfrom
Conversation
ali-keys
force-pushed
the
fix/tls-keyshare-imports
branch
from
aecd423 to
500887a
Compare
Contributor
|
Thanks for the PR. Looks good. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4957 +/- ##
==========================================
- Coverage 80.30% 80.30% -0.01%
==========================================
Files 379 379
Lines 93164 93163 -1
==========================================
- Hits 74820 74815 -5
- Misses 18344 18348 +4
🚀 New features to boost your workflow:
|
ali-keys
force-pushed
the
fix/tls-keyshare-imports
branch
from
500887a to
b3204b5
Compare
…r check The old import_all had three bugs: filenames could be silently dropped when the pool was saturated (no blocking wait for a free slot), the final batch was never drained, and the opening while-loop was dead code. Replace append_processes/check_processes/import_all with ThreadPoolExecutor, which handles queuing and draining correctly and cuts total test time roughly in half. Add a test verifying no module sets conf.debug_dissector as a side-effect of import, using the same ALL_FILES exclusion list and subprocess isolation as the existing import tests.
PacketField has no length bound, so server_share greedily consumed all remaining bytes including subsequent extensions. Switch to PacketLenField with length_from=pkt.len to bound dissection to the extension's own length field.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi everyone,
This PR fixes three things:
TLS_Ext_KeyShare_SH: fix server_share boundary
server_shareusedPacketField(no length bound), causing it toconsume subsequent extensions. Replaced with
PacketLenFieldboundedby the extension's own length field.
contrib/automotive/uds: remove conf.debug_dissector side-effect
In
uds.py,conf.debug_dissector = Truehas been left by accident.I noticed it during a TLS test where UDS was accidentally included from
my previous works. The config affected my TLS work. Removed it.
test/imports: fix pool bugs, add conf side-effect guard
The hand-rolled subprocess pool in
import_allsilently droppedfilenames when saturated, never drained the final batch, and had a
dead opening while-loop. Replaced with
ThreadPoolExecutor(~2xfaster). + Added a test verifying no module mutates
conf.debug_dissectoron import.