Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove SSH scanner using known_hosts #10456

Merged
merged 7 commits into from
Aug 16, 2018
Merged

Remove SSH scanner using known_hosts #10456

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4450d34
Remove SSH scanner using known_hosts
kkirsche Aug 14, 2018
3783347
Fix failing argument spec test
kkirsche Aug 14, 2018
2e75f46
Fix failing argument for password based spec test
kkirsche Aug 14, 2018
905f263
Remove host key checks on ssh scanner modules
Aug 15, 2018
09434bd
Fix tabbing caused by incorrect VM nvim configuration
Aug 15, 2018
cd01f11
Remove verifying host keys for all exploits
Aug 15, 2018
45e0b53
Fix spacing issue with rocket
Aug 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/metasploit/framework/login_scanner/ssh.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ def attempt_login(credential)
:config => false,
:verbose => verbosity,
:proxy => factory,
:non_interactive => true
:non_interactive => true,
:verify_host_key => :never
}
case credential.private_type
when :password, nil
Expand Down
15 changes: 8 additions & 7 deletions modules/auxiliary/scanner/ssh/apache_karaf_command_execution.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,14 @@ def cmd
def do_login(user, pass, ip)
factory = ssh_socket_factory
opts = {
auth_methods: ['password'],
port: rport,
config: false,
use_agent: false,
password: pass,
proxy: factory,
non_interactive: true
:auth_methods => ['password'],
:port => rport,
:config => false,
:use_agent => false,
:password => pass,
:proxy => factory,
:non_interactive => true,
:verify_host_key => :never
}

opts.merge!(verbose: :debug) if datastore['SSH_DEBUG']
Expand Down
26 changes: 14 additions & 12 deletions modules/auxiliary/scanner/ssh/cerberus_sftp_enumusers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,14 @@ def retry_num

def check_vulnerable(ip)
opt_hash = {
port: rport,
auth_methods: ['password', 'keyboard-interactive'],
use_agent: false,
config: false,
password_prompt: Net::SSH::Prompt.new,
non_interactive: true,
proxies: datastore['Proxies']
:port => rport,
:auth_methods => ['password', 'keyboard-interactive'],
:use_agent => false,
:config => false,
:password_prompt => Net::SSH::Prompt.new,
:non_interactive => true,
:proxies => datastore['Proxies'],
:verify_host_key => :never
}

begin
Expand Down Expand Up @@ -105,11 +106,12 @@ def check_user(ip, user, port)
pass = Rex::Text.rand_text_alphanumeric(8)

opt_hash = {
auth_methods: ['password', 'keyboard-interactive'],
port: port,
use_agent: false,
config: false,
proxies: datastore['Proxies']
:auth_methods => ['password', 'keyboard-interactive'],
:port => port,
:use_agent => false,
:config => false,
:proxies => datastore['Proxies'],
:verify_host_key => :never
}

opt_hash.merge!(verbose: :debug) if datastore['SSH_DEBUG']
Expand Down
13 changes: 7 additions & 6 deletions modules/auxiliary/scanner/ssh/fortinet_backdoor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,15 @@ def run_host(ip)
factory = ssh_socket_factory

ssh_opts = {
port: rport,
:port => rport,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason for changing the syntax here? I prefer the newer hash syntax if possible. All these keys are symbols, too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, hashrockets are more consistent with varying key types. I won't complain too much - just asking why it's necessary to change syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistency is the only reason. While I also prefer the new syntax, rocket formatted ones are what I’ve been asked for before when building exploit information hashes and was the style of the LoginScanner. As such, while I prefer the newer one, I think consistency is more important than minimal changes. I’m open though to other opinions if we want to use them, but stylistically I think the framework would benefit from a more defined code style guideline around hash structures

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I ask only because you're modifying my module. :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think hashrockets will be the way to go if we publish our own style guide (and not just cargo-cult the Ruby style guide). We have such wildly varying hashes that it'd make sense to stick to one style.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python is a little more predictable. :-)

# The auth method is converted into a class name for instantiation,
# so fortinet-backdoor here becomes FortinetBackdoor from the mixin
auth_methods: ['fortinet-backdoor'],
non_interactive: true,
config: false,
use_agent: false,
proxy: factory
:auth_methods => ['fortinet-backdoor'],
:non_interactive => true,
:config => false,
:use_agent => false,
:proxy => factory,
:verify_host_key => :never
}

ssh_opts.merge!(verbose: :debug) if datastore['SSH_DEBUG']
Expand Down
11 changes: 6 additions & 5 deletions modules/auxiliary/scanner/ssh/juniper_backdoor.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ def initialize(info = {})
def run_host(ip)
factory = ssh_socket_factory
ssh_opts = {
port: rport,
auth_methods: ['password', 'keyboard-interactive'],
password: %q{<<< %s(un='%s') = %u},
proxy: factory,
:non_interactive => true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was one hash that had a mix of the styles

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, yuck. And they're still all symbol keys, so they COULD have used the newer syntax. Oh well. Hashrockets it is!

:port => rport,
:auth_methods => ['password', 'keyboard-interactive'],
:password => %q{<<< %s(un='%s') = %u},
:proxy => factory,
:non_interactive => true,
:verify_host_key => :never
}

ssh_opts.merge!(verbose: :debug) if datastore['SSH_DEBUG']
Expand Down
3 changes: 2 additions & 1 deletion modules/auxiliary/scanner/ssh/ssh_enumusers.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ def check_user(ip, user, port)
:password => pass,
:config => false,
:proxy => factory,
:non_interactive => true
:non_interactive => true,
:verify_host_key => :never
}

opt_hash.merge!(:verbose => :debug) if datastore['SSH_DEBUG']
Expand Down
15 changes: 8 additions & 7 deletions modules/auxiliary/scanner/ssh/ssh_identify_pubkeys.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,14 @@ def do_login(ip, port, user)

factory = ssh_socket_factory
opt_hash = {
:auth_methods => ['publickey'],
:port => port,
:key_data => key_data[:public],
:use_agent => false,
:config =>false,
:proxy => factory,
:non_interactive => true
:auth_methods => ['publickey'],
:port => port,
:key_data => key_data[:public],
:use_agent => false,
:config =>false,
:proxy => factory,
:non_interactive => true,
:verify_host_key => :never
}

opt_hash.merge!(:verbose => :debug) if datastore['SSH_DEBUG']
Expand Down
18 changes: 10 additions & 8 deletions spec/lib/metasploit/framework/login_scanner/ssh_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@
:proxy => factory,
:auth_methods => ['password','keyboard-interactive'],
:password => private,
:non_interactive => true
:non_interactive => true,
:verify_host_key => :never
}
allow(Rex::Socket::SSHFactory).to receive(:new).and_return factory
expect(Net::SSH).to receive(:start).with(
Expand All @@ -161,13 +162,14 @@
it 'calls Net::SSH with the correct arguments' do
factory = Rex::Socket::SSHFactory.new(nil,nil,nil)
opt_hash = {
:auth_methods => ['publickey'],
:port => ssh_scanner.port,
:use_agent => false,
:key_data => key,
:config => false,
:verbose => ssh_scanner.verbosity,
:proxy => factory
:auth_methods => ['publickey'],
:port => ssh_scanner.port,
:use_agent => false,
:key_data => key,
:config => false,
:verbose => ssh_scanner.verbosity,
:proxy => factory,
:verify_host_key => :never
}
allow(Rex::Socket::SSHFactory).to receive(:new).and_return factory
expect(Net::SSH).to receive(:start).with(
Expand Down