refactor(dpi/ssh): collapse dead conditional around parse_kexinit_algorithms#296
Merged
domcyrus merged 1 commit intoMay 20, 2026
Merged
Conversation
…gorithms` Both branches of the `if payload.len() > 20 && payload[5] == 20` gate called `parse_kexinit_algorithms(payload)` with the same argument and assigned the result to the same field, so the conditional was a no-op — the function ran unconditionally either way. `parse_kexinit_algorithms` is a substring scan over the raw bytes (it looks for known algorithm name literals like "aes128-ctr" / "ssh-ed25519"), which works equally on a structured KEXINIT message and on free-form banner / text content. The gate offered no protection and no faster path. Drop the conditional, keep the unconditional call, and replace the inline comment with a note explaining why no branching is needed so a future reader doesn't reintroduce the same dead split.
Owner
|
@obchain Thanks for this PR. This LGTM! |
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.
Summary
Collapses the dead
if/elseblock inanalyze_ssh(src/network/dpi/ssh.rs:107-117) where both branches calledparse_kexinit_algorithms(payload)with the same argument and assigned to the same field.Closes #295.
Why
The gate
payload.len() > 20 && payload[5] == 20(the KEXINIT signature at the standard SSH packet offset) selected between two arms that did exactly the same thing.parse_kexinit_algorithmsis a substring scan over the raw bytes — it works on KEXINIT-structured packets and on free-form banner / text content equally — so the branching offered no fast-path and no correctness gate, just two duplicated 4-line blocks with misleading comments.Drop the conditional, call
parse_kexinit_algorithms(payload)unconditionally, replace the inline comment with a one-liner explaining why no branching is needed. Same spirit as #279 / #289 / #290.Test plan
cargo clippy --all-targets --all-features -- -D warnings— cleancargo fmt --check— cleancargo test --lib ssh— 17 passed, 0 failedcargo test --lib— 361 passed (full suite still green)test_openssh_banner,test_putty_banner,test_ssh1_banner), KEXINIT detection (test_kexinit_detection), and userauth state (test_userauth_success) exercise both the "structured KEXINIT" and "banner only" code paths