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
Add additional timing options to snmp_login scanner #4722
Conversation
OptInt.new('BATCHSIZE', [true, 'The number of hosts to probe in each set', 256]), | ||
OptEnum.new('VERSION', [true, 'The SNMP version to scan', 'all', ['1','2c','all']]), |
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.
Note used 'all' incase v3 is supported in future etc ;)
1s timeout is too low for slow embedded devices over the internet fwiw |
The fix for this is probably a rewrite of LoginScanner to format the SNMP by hand, as opposed to using the sluggish SNMP::Manager interface. |
It takes almost no time to send 3 probes (v1/v2/v3/etc) to a single host and then listen for any reason at a scan level (via the UDPScanner mixin) |
Another issue with the LoginScanner, its Rex::Socket create call doesn't pass in the framework context, which means pivoting is ignored. |
I thought UDP pivoting wasn't implemented yet anyway? Could add it, but |
It doesn't look like any login scanner would be Rex proxy compatible: lib/metasploit/./framework/tcp/client.rb
|
Unbound sockets wouldn't be pivotable, but bound ones are and work fine through meterpreter. I filed a ticket for the issue above and will PR something over soon. |
0342265
to
0debbbb
Compare
@@ -17,6 +17,35 @@ class SNMP | |||
PRIVATE_TYPES = [ :password ] | |||
REALM_KEY = nil | |||
|
|||
# @!attribute retries |
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.
YARD has advised that @!attribute
is not meant to be used on attr_*
. Use comments directly instead:
# The number of retries
#
# @return [Fixnum]
attr_accessor :retries
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.
You should separate your attributes from your validations, so that all attributes are first, then all validations as the patten shown here
Any movement on this? |
Tagging this as |
This needs a rewrite to get to reasonable performance levels (single socket, multiple sends of community strings in quick succession, detection of valid ones based on replies within a poll period). |
#4718
Set the default retries to 0.
Reduce the timeout to 2s.
Still scan both v1 and v2c, but give the option to scan only one.