Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

smb2.lua, smb-protocols.nse, smb2-capabilities and smb2-security-mode #943

Closed
wants to merge 32 commits into
from

Conversation

Projects
None yet
2 participants

cldrn commented Jul 12, 2017

  • smb-protocols: Lists supported SMB1/SMB2/SMB3 protocols and dialects
  • smb2-capabilities: Lists the capabilities of SMB2/SMB3 servers
  • smb2-security-mode: Reads the message signing configuration in SMB2/SMB3 servers.

More info:
http://seclists.org/nmap-dev/2017/q3/20

@dmiller-nmap

I'd really like to get this committed soon! Great stuff.

scripts/smb2-capabilities.nse
+ -- we need a clean connection for each negotiate request
+ status, smbstate = smb.start(host)
+ if(status == false) then
+ return false, smbstate
@dmiller-nmap

dmiller-nmap Jul 27, 2017

Don't return false from the action function. Either return nil (no output) or return an error message. I guess this was probably part of a function at one time.

scripts/smb2-capabilities.nse
+ end
+ -- We set our overrides Dialects table with the dialect we are testing
+ overrides['Dialects'] = {dialect}
+ status, _ = smb2.negotiate_v2(smbstate, overrides)
@dmiller-nmap

dmiller-nmap Jul 27, 2017

_ is global here. Just leave it off; unlike Python, Lua ignores any extra/unassigned return values.

scripts/smb2-capabilities.nse
+
+ -- We check the capabilities flags. Not all of them are supported by
+ -- every dialect but we dumb check anyway.
+ if ( bit.band(smbstate['capabilities'], 0x00000001) == 0x00000001) then
@dmiller-nmap

dmiller-nmap Jul 27, 2017

if smbstate.capabilities & 0x01 == 0x01 then
nselib/smb.lua
-function negotiate_protocol(smb, overrides)
+-- @param smb The SMB object associated with the connection.
+-- @param overrides Overrides table.
+-- @return (status, dialect) If status is true, the negotiated dialect in human readable form is returned as the second value.
@dmiller-nmap

dmiller-nmap Jul 27, 2017

NSEdoc supports multiple @return statements. The convention for this sort of thing is:

-- @return Boolean status
-- @return The negotiated dialect in human-readable form, or an error message if status is false
nselib/smb.lua
@@ -2498,7 +2561,6 @@ end
-- data is given as a string, not a file.
--
--@param host The host object
---@param data The string containing the data to be written
@dmiller-nmap

dmiller-nmap Jul 27, 2017

Was this an accidental deletion?

@cldrn

cldrn Jul 28, 2017

Yes. Fixed it. Thanks!

nselib/smb2.lua
+-- get updated. I tried to be consistent with the current implementation of
+-- smb.lua but some fields may have changed name or don't exist anymore.
+--
+-- TODO:
@dmiller-nmap

dmiller-nmap Jul 27, 2017

Is this TODO list current? It probably doesn't belong in the NSEdoc. Please make a separate Github Issue for each item that you think really ought to be done.

nselib/smb2.lua
+local table = require "table"
+local match = require "match"
+local bit = require "bit"
+local nsedebug = require "nsedebug"
@dmiller-nmap

dmiller-nmap Jul 27, 2017

I guess this was left in from testing; please remove it before committing.

+ local attempts = 5
+ local status, err
+
+ local out = string.pack(">I<c" .. #body, #body, body)
@dmiller-nmap

dmiller-nmap Jul 27, 2017

Is this really big-endian? I thought all SMB stuff was little-endian. Also, you need to specify a length in bytes, otherwise it uses "native" length which may vary on some platforms. Lastly, use the "S" family of format strings for length-prefixed strings. I'd guess this is supposed to be string.pack("<S4", body), but you should verify.

@cldrn

cldrn Jul 28, 2017

This is one of the oddities of SMB. If we are sending SMB over NetBIOS we actually have to mix endianness. Ron posted about this here: https://blog.skullsecurity.org/2008/andx-and-what

nselib/smb2.lua
+ return false, "SMB2: ERROR:Server returned less data than it was supposed to"
+ end
+ -- Make the length 24 bits
+ netbios_length = bit.band(netbios_length, 0x00FFFFFF)
@dmiller-nmap

dmiller-nmap Jul 27, 2017

Please use native Lua 5.3 bit operations. The bit library is deprecated.

@cldrn

cldrn Jul 28, 2017

Removed all calls to the bit library.

nselib/smb2.lua
+ 0x0002, -- Ciphers (2 bytes each): AES-128-GCM
+ 0x0001 -- Ciphers (2 bytes each): AES-128-CCM
+ )
+ data = data .. string.pack("<I2 I2 I4 c" .. #context_data,
@dmiller-nmap

dmiller-nmap Jul 27, 2017

Instead of string.pack("XYZ c" .. #something, X, Y, Z, something), just do string.pack("XYZ", X, Y, Z) .. something; it's simpler.

nselib/smb2.lua
+ total_data = #header+#data
+ padding_data = ""
+ while((total_data)%8 ~= 0) do
+ padding_data = padding_data .. string.pack("<c1", 0x0)
@dmiller-nmap

dmiller-nmap Jul 27, 2017

Padding is simpler like this:

padding = string.rep("\0", (8 - total_data % 8) % 8)
@cldrn

cldrn Jul 28, 2017

Nice one. Much simpler.

cldrn commented Jul 28, 2017

Added smb2-time.nse to this branch too. I'll create a separate PR for the vulnerability detection script based on the system uptime.

cldrn commented Jul 28, 2017

Actually, as we want to push everything we have for SMB2 now, I've added smb2-vuln-uptime.nse in the same PR.

@nmap-bot nmap-bot closed this in ed0b960 Jul 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment