-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Network Block Device Info Script #609
Conversation
… fixed bugs it found.
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.
Looking good! Thanks for the test server, that is really helpful. Probably you'll want to strip any references to it before publishing, though. Just a few things to fix.
nselib/nbd.lua
Outdated
self.protocol = {ssl_tls = (proto == "ssl")} | ||
|
||
if #rep ~= 8 then | ||
stdnse.debug1("Failed to receive first 64 bits of magic from server: %s", rep) |
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.
I get this message a lot for ports 10807 and 10808, and it looks like it's sending more than just the magic first:
NSOCK INFO [0.9450s] nsock_trace_handler_callback(): Callback: READ SUCCESS for EID 42 [138.197.130.253:10807] (24 bytes): NBDMAGIC..B....S........
NSE: TCP 72.14.177.105:49524 < 138.197.130.253:10807 | 00000000: 4e 42 44 4d 41 47 49 43 00 00 42 02 81 86 12 53 NBDMAGIC B S
00000010: 00 00 00 00 00 10 00 00
NSE: [nbd-info 138.197.130.253:10807] Failed to receive first 64 bits of magic from server: NBDMAGIC
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.
Your analysis was correct, the issue was getting sent extra data. I instituted a receive buffer in the protocol object to handle varying sizes of initial receives.
nselib/nbd.lua
Outdated
if not status then | ||
stdnse.debug1("Failed to receive zero pad from server while attaching to export: %s", err) | ||
self:close() | ||
return false |
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.
Super-minor nitpick, but these 3 lines are tabs instead of spaces for indentation. A couple other places have this, too.
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.
Sorry about that, it's what I get for using a default config and 'smart' indentation. Fixed all tabs in new files.
scripts/nbd-info.nse
Outdated
categories = {"discovery", "intrusive"} | ||
|
||
-- XXX-MAK: The expanded port range is for testing against nmap.kolybabi.com. | ||
portrule = shortport.version_port_or_service({10807, 10808, 10809}, "nbd", "tcp") |
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.
What should the port numbers be for this? Do we have good service probe matches that can identify NBD?
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.
There is an existing, working service probe. I have reverted the portrule to its non-debug form.
scripts/nbd-info.nse
Outdated
table.insert(tbl, "The server software appears to support fixed newstyle negotiation, but not on this port.") | ||
end | ||
end | ||
table.insert(tbl, ("Negotiation: %s"):format(comm.protocol.negotiation)) |
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.
Instead of inserting strings containing "key: value", make tbl
a stdnse.output_table
and set tbl[key] = value
. I know it doesn't work with the "not on this port" thing, but maybe if you had a key like "newstyle negotiation supported:" and the value is "other port"? The other thing to try would be to use both integer (with table.insert
) and string keys. I know NSE can handle that, but I don't know if it works via output_table
.
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.
Changed reporting method to attempt to address this. Properly, I hope.
scripts/nbd-info.nse
Outdated
for name, info in pairs(comm.exports) do | ||
local exp = {} | ||
if type(info.size) == "number" then | ||
table.insert(exp, ("Size: %d bytes"):format(info.size)) |
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.
Here again, if exp
is an output_table
, then you can insert keys and values instead of just strings.
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.
Changed reporting method to attempt to address this. Properly, I hope.
scripts/nbd-info.nse
Outdated
output["Exported Block Devices"] = tbl | ||
end | ||
|
||
return output, stdnse.format_output(true, output) |
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.
stdnse.output_table
should not be filtered through stdnse.format_output
. Just return it directly.
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.
Fixed.
All issues addressed. Please let me know if there is anything else I should do. |
@dmiller-nmap Any more reviews on this patch? |
Knocking out one from the Script Ideas article on the wiki: https://secwiki.org/w/Nmap/Script_Ideas#nbd-info.
The server software isn't all that stable, but I've got a decently reliable test server at nmap.kolybabi.com that you're welcome to test against.