Improvements in SMB2 dialect negotiation#2208
Closed
nnposter wants to merge 8 commits intonmap:masterfrom
nnposter:smb2-dialectnego
Closed
Improvements in SMB2 dialect negotiation#2208nnposter wants to merge 8 commits intonmap:masterfrom nnposter:smb2-dialectnego
nnposter wants to merge 8 commits intonmap:masterfrom
nnposter:smb2-dialectnego
Conversation
Do not try individual dialects if SMB2 is not supported at all Do not test for dialects with revision codes higher than what was agreed to by the target
This is expensive if the target does not support lower dialect revisions
Otherwise the script fails with targets that do not support the particular dialect
Otherwise the script fails with targets that do not support the particular dialect
Author
|
This PR will be committed on/after January 15 unless concerns are raised. |
Author
|
The patch has been committed as r38184. |
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
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.
There are a few weaknesses in the current handling of SMB 2 dialect negotiation:
Scripts
smb-protocolsandsmb2-capabilitieslack a check whether SMB 2 is supported by the target. Instead, they immediately start with probing one dialect revision at a time. The effect is that for older SMB 2 implementations, such as Windows 7, they are sending probes for dialects that are guaranteed to fail. The issue is even more pronounced with targets that do not support SMB 2 at all, such as XP. In this case, a native SMB2 probe does not result in any response from the target so the script pauses until the probe times out. The time-out is set to 10s, which means that in total the script is guaranteed to take at least 50s under these conditions (with 5 supported dialects).Script
smb2-security-modeiterates through SMB 2 dialects until it succeeds with negotiating a session with the target. If the target supports only the latest dialects, such as 3.1.1, then the script performance is notably degraded.Script
smb2-vuln-uptimehard-codes one specific SMB 2 dialect to use. If the target does not support it then the script fails. Other scripts, such assmb2-time, do not explicitly force a dialect but they leave the dialect negotiation completely up to functionsmb2.negotiate_v2, which again defaults to a single specific dialect so these scripts might suffer the same fate.This proposed code implements the following changes:
Scripts that are looping through all dialects by design are now guarded with early SMB 2 negotiation, which determines the highest supported dialect revision and, implicitly, whether SMB 2 is supported at all. This means that the dialect loop can be terminated early, as soon as the highest revision is reached.
Scripts that do not have to force a specific dialect with the target are now relying on function
smb2.negotiate_v2, which has been modified to propose all supported dialects to the target by default, maximizing the chance that the negotiation succeeds.Various duplicated instances of the list of supported SMB 2 dialect revision codes have been centralized and replaced with calls to function
smb2.dialects. Similarly, duplicated code for converting integer revision codes to human-friendly strings has been replaced with calls to functionsmb2.dialect_name.Cosmetically, all SMB 2 dialect names have been modified to match official revisions: 2.0.2, 2.1, 3.0, 3.0.2, and 3.1.1. The same revisions were previously called 2.02, 2.10, 3.00, 3.02, and 3.11.
Performance Comparison
@cldrn In some cases it was not clear why the current code was designed this way so there is a decent chance that I have missed some important aspect. Could you please review?