-
Notifications
You must be signed in to change notification settings - Fork 14.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Note when SMBv1 is supported to the user #19131
Conversation
desc = "SMB Detected (versions:#{info[:versions].join(', ')}) (preferred dialect:#{info[:preferred_dialect]})" | ||
if info[:versions].include?(1) | ||
desc << ' (SMBv1: true)' | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't this be covered on the previous line where (version:
includes 1? This seems redundant. I would expect a server that supports SMBv1 to show (versions:1)
or (versions:1,2)
, etc.
This is based on an issue raised here
#17402
If this is it needed (I agree btw that it’s shown twice) then ignore this
PR and close the issue
…On Wed, 24 Apr 2024 at 16:12, Spencer McIntyre ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In modules/auxiliary/scanner/smb/smb_version.rb
<#19131 (comment)>
:
> desc = "SMB Detected (versions:#{info[:versions].join(', ')}) (preferred dialect:#{info[:preferred_dialect]})"
+ if info[:versions].include?(1)
+ desc << ' (SMBv1: true)'
+ end
Wouldn't this be covered on the previous line where (version: includes 1?
This seems redundant. I would expect a server that supports SMBv1 to show
(versions:1) or (versions:1,2), etc.
—
Reply to this email directly, view it on GitHub
<#19131 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPQE3SLCODQL455S3EDYWTY66VTJAVCNFSM6AAAAABGWVHNA6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMJZHAYTONRXHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
I think that issue was to say that the host operating system information isn't extracted from the NTLM challenge/response when SMBv1 isn't in use. I think we could address it, though it may require an update to RubySMB if the NTLM challenge/response, isn't readily accessible. I could probably help if you were interested in working on that. |
Yip; it looks like from the original ticket I was hoping that the host information was extracted from the NTLMSSP_CHALLENGE response:
Sorry for the confusion in the original ticket |
Can you clarify what are the requirements for completing the task? I will
try to work on it
…On Wed, 24 Apr 2024 at 16:49, adfoster-r7 ***@***.***> wrote:
I think that issue was to say that the host operating system information
isn't extracted from the NTLM challenge/response when SMBv1 isn't in use.
Yip; it looks like from the original ticket I was hoping that the host
information was extracted from the NTLMSSP_CHALLENGE response:
I would have expected the Windows 10.0 Build 14393 metadata to be present
I think the information should be extractable from the
STATUS_MORE_PROCESSING_REQUIRED, NTLMSSP_CHALLENGE response
Sorry for the confusion in the original ticket
—
Reply to this email directly, view it on GitHub
<#19131 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPQE3URTLJWWV7ERT6Z4V3Y66Z6HAVCNFSM6AAAAABGWVHNA6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZUHE4TONZQG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Requirements for completing the task would be extracting extra details from the NTLM negotiation and showing it to the user, i.e the mockup: - [*] 192.168.123.13:445 - SMB Detected (versions:2, 3) (preferred dialect:SMB 3.1.1) (compression capabilities:) (encryption capabilities:AES-128-GCM) (signatures:required) (uptime:3m 46s) (guid:{3863d7d4-26ca-4913-8ff3-d4e27787e43d}) (authentication domain:ADF3)
+ [*] 192.168.123.13:445 - SMB Detected (versions:2, 3) (Windows 10.0 Build 14393 x64) (preferred dialect:SMB 3.1.1) (compression capabilities:) (encryption capabilities:AES-128-GCM) (signatures:required) (uptime:3m 46s) (guid:{3863d7d4-26ca-4913-8ff3-d4e27787e43d}) (authentication domain:ADF3)
^^^^^^^^^^^ Added ^^^^^^^^^^^^ Running the module against a windows box should provide the details that we can log out for the user, I'm not sure what samba gives you |
I think what you are asking to add already exists:
NOTE the part about Windows 6.1 (Samba 4.12.2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only pom.xml has changed, approved!
Running smb_version against a windows target with SMB v1,v2,v3 enabled outputs the target build information ✅
Disabling SMB v1 on the windows target:
Running the same module against the target with SMB v1 disabled no longer shows the target build information ❌
We can see that that build information is missing on the re-scan; but from wireshark it looks like it should be available just fine in the NTLMSSP_CHALLENGE packet: |
This information is processed in these two lines of code:
|
Specifically, the: |
There seems to be no parsing of the |
There seems to be some thought placed into the code to support GSS?
But no one is calling this function |
Parsing this data while coding it in Ruby is at the moment beyond my technical skills |
You should be able to parse it using RubyNTLM / |
iirc there's also all of the required parsers over in the ruby_smb library for that octet string/buffer, i.e. these methods: And the binary model: But if RubyNTLM is easier to reach for in this context, that sounds good to me 💯 |
Is RubyNTLM being shipped/used by metasploit atm? or do I need to copy-paste the code there into the smb.rb that comes with Metasploit? |
It's already included with Metasploit https://github.com/rapid7/metasploit-framework/blob/master/metasploit-framework.gemspec#L84 It's worth noting though, that for the actual authentication workflow we use our own client from RubySMB because it includes some changes we haven't been able to get merged into the upstream library. Most notably, it supports anonymous authentication which is required in certain contexts by Metasploit such as exploiting zerologon. For what's going on here though, you just need to use the parsing code. I only point this out to provide context that for the actual processing we have to use our version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only pom.xml has changed, approved!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like there's some changes to make here to support the requirements 👍
Thanks for your contribution to Metasploit Framework! We've looked at this pull request, and we agree that it seems like a good addition to Metasploit, but it looks like it is not quite ready to land. We've labeled it What does this generally mean? It could be one or more of several things:
We would love to land this pull request when it's ready. If you have a chance to address all comments, we would be happy to reopen and discuss how to merge this! |
This improves the feedback listed here: #17402
Verification
List the steps needed to make sure this thing works
docker-compose up
, SMBv1 supported server, and SMBv1 not supported and see difference in outcomemsfconsole
use smb_version
run 172.24.0.2
docker-compose with SMBv1:
docker-compose without SMBv1: