Fix Redis Service Reporting #19283

adeherdt-r7 · 2024-06-25T13:20:40Z

Preliminary pull request to resolve an issue with a service not being properly detected for Redis.

Ensure service name is properly passed down when detecting vulnerabilities
Ensure Redis properly detects no-auth requirements

Verification

List the steps needed to make sure this thing works

Run redis in docker without auth:

docker run --rm -p 6379:6379 redis

If needing to configure redis for auth

$ nc 127.0.0.1 6379
config set requirepass p4$$W0rd
+OK

Start msfconsole
use auxiliary/scanner/redis/redis_login
run RHOSTS=127.0.0.1 if no auth
run RHOSTS=127.0.0.1 username=default password=p4$$w0rd if auth enabled
Verify output and scanner works as expected; if msfdb is enabled - services should be populated correctly:

msf6 auxiliary(scanner/redis/redis_login) > run rhost=127.0.0.1

[+] 127.0.0.1:6379        - 127.0.0.1:6379        - No password is required.
[*] 127.0.0.1:6379        - Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
msf6 auxiliary(scanner/redis/redis_login) > services
Services
========

host       port  proto  name   state  info
----       ----  -----  ----   -----  ----
127.0.0.1  6379  tcp    redis

msf6 auxiliary(scanner/redis/redis_login) >

msf6 auxiliary(scanner/redis/redis_login) > run RHOSTS=192.168.73.1 rport=6380 anonymous_login=true

[!] 192.168.73.1:6380     - No active DB -- Credential data will not be saved!
[+] 192.168.73.1:6380     - 192.168.73.1:6380     - Login Successful: :foobared (Successful: +OK)
[*] 192.168.73.1:6380     - Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed

msf6 auxiliary(scanner/redis/redis_login) > run RHOSTS=192.168.73.1 rport=6380 username=redis password=foobared

[!] 192.168.73.1:6380     - No active DB -- Credential data will not be saved!
[+] 192.168.73.1:6380     - 192.168.73.1:6380     - Login Successful: :foobared (Successful: +OK)
[*] 192.168.73.1:6380     - Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed

lib/metasploit/framework/version.rb

lib/metasploit/framework/login_scanner/redis.rb

lib/msf/core/db_manager/vuln.rb

adfoster-r7 · 2024-06-26T09:33:51Z

@adeherdt-r7 Looks like this is missing parts of the PR template, specifically the replication steps - which is useful for future travellers to understand more about how to setup their environment for testing and verifying the issue: https://github.com/rapid7/metasploit-framework/blob/master/.github/PULL_REQUEST_TEMPLATE.md

Does this PR need to make changes to the existing automated tests to verify this change in behavior too? 👀

https://github.com/rapid7/metasploit-framework/blob/master/spec/lib/metasploit/framework/login_scanner/redis_spec.rb

adfoster-r7 · 2024-06-26T09:38:43Z

It'd be great to squash down the commits too as part of this PR to help clean up things a bit; the rationale being, some of the later commits are undoing work from the previous commits - so they could just be merged to help tidy things up 👍

adeherdt-r7 · 2024-06-26T09:48:41Z

@adeherdt-r7 Looks like this is missing parts of the PR template, specifically the replication steps - which is useful for future travellers to understand more about how to setup their environment for testing and verifying the issue: https://github.com/rapid7/metasploit-framework/blob/master/.github/PULL_REQUEST_TEMPLATE.md

Does this PR need to make changes to the existing automated tests to verify this change in behavior too? 👀

https://github.com/rapid7/metasploit-framework/blob/master/spec/lib/metasploit/framework/login_scanner/redis_spec.rb

Updated the template usage for the pull request description.
Scanner still works, as this should not affect anything directly for the console.

adfoster-r7 · 2024-06-26T10:19:42Z

I updated the verification steps a bit in the description

No auth 🟢

msf6 auxiliary(scanner/redis/redis_login) > run rhost=127.0.0.1

[+] 127.0.0.1:6379        - 127.0.0.1:6379        - No password is required.
[*] 127.0.0.1:6379        - Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
msf6 auxiliary(scanner/redis/redis_login) > services
Services
========

host       port  proto  name   state  info
----       ----  -----  ----   -----  ----
127.0.0.1  6379  tcp    redis

msf6 auxiliary(scanner/redis/redis_login) >

Auth 🔴

msf6 auxiliary(scanner/redis/redis_login) > run rhost=127.0.0.1

[-] 127.0.0.1:6379        - Auxiliary failed: NoMethodError undefined method `each_unfiltered_username_first' for #<Metasploit::Framework::PrivateCredentialCollection:0x0000000115022f40 @blank_passwords=false, @pass_file="/Users/adfoster/Documents/code/metasploit-framework/data/wordlists/unix_passwords.txt", @password="foobared", @prepended_creds=[], @additional_privates=[], @filter=nil>
[-] 127.0.0.1:6379        - Call stack:
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/credential_collection.rb:98:in `each_filtered'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:141:in `each_credential'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:205:in `scan!'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/modules/auxiliary/scanner/redis/redis_login.rb:83:in `run_host'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/msf/core/auxiliary/scanner.rb:128:in `block (2 levels) in run'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/msf/core/thread_manager.rb:105:in `block in spawn'
[*] Auxiliary module execution completed
msf6 auxiliary(scanner/redis/redis_login) >

The auth scenario being broken is not a blocker for me as it's unrelated; and we can circle back to that. We do have better testing infrastructure available to us now, so we can fix this up and add integration tests separately.

Edit: Looks like this introduces a regression even after the above is fixed, so we'll want to fix this PR

adfoster-r7 · 2024-06-26T10:44:23Z

Fixing the creds collection exception; gives me exceptions against a newer redis version due to status being nil:

    60:             status = validate_login(sock.get_once)

From: /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/redis.rb:61 Metasploit::Framework::LoginScanner::Redis#attempt_login:

    56:             command = redis_proto(['AUTH', credential.private.to_s])
    57: 
    58:             sock.put(command)
    59: 
    60:             status = validate_login(sock.get_once)
 => 61:             result_options[:status] = status unless status.nil?
    62:           rescue Rex::ConnectionError, EOFError, Timeout::Error, Errno::EPIPE => e
    63:             result_options.merge!(
    64:               proof: e,
    65:               status: Metasploit::Model::Login::Status::UNABLE_TO_CONNECT
    66:             )

[1] pry(#<Metasploit::Framework::LoginScanner::Redis>)> status
=> nil

Putting a breakpoint into validate_login shows data is equal to "-WRONGPASS invalid username-password pair or user is disabled.\r\n" which doesn't equal any of these matches:

[3] pry(#<Socket>)> next

From: /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/redis.rb:83 Metasploit::Framework::LoginScanner::Redis#validate_login:

    82: def validate_login(data)
 => 83:   return if data.nil?
    84: 
    85:   return Metasploit::Model::Login::Status::NO_AUTH_REQUIRED if data =~ OLD_PASSWORD_NOT_SET
    86:   return Metasploit::Model::Login::Status::NO_AUTH_REQUIRED if data =~ PASSWORD_NOT_SET
    87:   return Metasploit::Model::Login::Status::INCORRECT if (data =~ /^-ERR invalid password/) == 0
    88:   return Metasploit::Model::Login::Status::SUCCESSFUL if (data =~ /^\+OK/) == 0
    89: 
    90:   nil
    91: end

[3] pry(#<Metasploit::Framework::LoginScanner::Redis>)> data
=> "-WRONGPASS invalid username-password pair or user is disabled.\r\n"
[4] pry(#<Metasploit::Framework::LoginScanner::Redis>)> OLD_PASSWORD_NOT_SET
=> /but no password is set/i
[5] pry(#<Metasploit::Framework::LoginScanner::Redis>)> PASSWORD_NOT_SET
=> /without any password configured/i
[6] pry(#<Metasploit::Framework::LoginScanner::Redis>)>

Which causes a npe in the module:

[8] pry(#<Socket>)> continue

[-] 127.0.0.1:6379        - Auxiliary failed: NoMethodError undefined method `strip' for nil:NilClass
[-] 127.0.0.1:6379        - Call stack:
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/modules/auxiliary/scanner/redis/redis_login.rb:107:in `block in run_host'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:234:in `block in scan!'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:179:in `block in each_credential'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/credential_collection.rb:94:in `block in each_filtered'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/credential_collection.rb:111:in `each_unfiltered'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/credential_collection.rb:91:in `each_filtered'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:141:in `each_credential'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/metasploit/framework/login_scanner/base.rb:205:in `scan!'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/modules/auxiliary/scanner/redis/redis_login.rb:83:in `run_host'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/msf/core/auxiliary/scanner.rb:128:in `block (2 levels) in run'
[-] 127.0.0.1:6379        -   /Users/user/Documents/code/metasploit-framework/lib/msf/core/thread_manager.rb:105:in `block in spawn'
[*] Auxiliary module execution completed

On the strip here:

metasploit-framework/modules/auxiliary/scanner/redis/redis_login.rb

Line 107 in 4c81b39

    
           vprint_error "#{peer} - LOGIN FAILED: #{result.credential} (#{result.status}: #{result.proof.strip})"

[2] pry(#<Msf::Modules::Auxiliary__Scanner__Redis__Redis_login::MetasploitModule>)> result.proof
=> nil

lib/metasploit/framework/login_scanner/redis.rb

adeherdt-r7 · 2024-06-26T12:26:33Z

Waiting on #19284 to be merged down to resolve the password problems.

adeherdt-r7 · 2024-06-26T13:08:10Z

Updated the pullrequest description with examples, rebased to pull in the fixes and address the code concerns.

Preliminary pull request to resolve an issue with a service not being properly detected for Redis. * Ensure service name is properly passed down when detecting vulnerabilities * Ensure Redis properly detects no-auth requirements

adfoster-r7 · 2024-06-26T15:09:20Z

No auth 🟢

msf6 auxiliary(scanner/redis/redis_login) > run rhost=127.0.0.1 password=p4$$w0rd_wrong

[+] 127.0.0.1:6379        - 127.0.0.1:6379        - No password is required.
[*] 127.0.0.1:6379        - Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed

Valid cred 🟢

msf6 auxiliary(scanner/redis/redis_login) > run rhost=127.0.0.1 password=p4$$w0rd

[+] 127.0.0.1:6379        - 127.0.0.1:6379        - Login Successful: :p4$$w0rd (Successful: +OK)
[*] 127.0.0.1:6379        - Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
msf6 auxiliary(scanner/redis/redis_login) >

Invalid cred 🟢

msf6 auxiliary(scanner/redis/redis_login) > run rhost=127.0.0.1 password=p4$$w0rd_wrong

[-] 127.0.0.1:6379        - 127.0.0.1:6379        - LOGIN FAILED: :p4$$w0rd_wrong (Incorrect: -WRONGPASS invalid username-password pair or user is disabled.)
[-] 127.0.0.1:6379        - 127.0.0.1:6379        - LOGIN FAILED: :admin (Incorrect: -WRONGPASS invalid username-password pair or user is disabled.)
[-] 127.0.0.1:6379        - 127.0.0.1:6379        - LOGIN FAILED: :123456 (Incorrect: -WRONGPASS invalid username-password pair or user is disabled.)
[-] 127.0.0.1:6379        - 127.0.0.1:6379        - LOGIN FAILED: :12345 (Incorrect: -WRONGPASS invalid username-password pair or user is disabled.)
[-] 127
.. etc ...

Services 🍏

msf6 auxiliary(scanner/redis/redis_login) > services
Services
========

host       port  proto  name   state  info
----       ----  -----  ----   -----  ----
127.0.0.1  6379  tcp    redis

We don't store redis as a service in the invalid password scenario, even though we 'know' it's Redis. That's existing behavior though.

adfoster-r7 · 2024-06-26T15:12:56Z

Release notes

Fixes the auxiliary/scanner/redis/redis_login module to correctly track the registered service name as redis - previously it was blank

adfoster-r7 · 2024-06-26T15:13:45Z

lib/msf/core/db_manager/vuln.rb

@@ -158,7 +160,9 @@ def report_vuln(opts)
          sname = opts[:proto]
        end

-        service = host.services.where(port: opts[:port].to_i, proto: proto).first_or_create
+        services = host.services.where(port: opts[:port].to_i, proto: proto)
+        services = services.where(name: sname) if sname.present?


We might need to change this again in the future; if multiple calls to this API have different snames present - odd behavior may occur

Yeah, I'm actually wondering if this is closer to what we want:

Mdm::Host.first.services.where(port: 6379, proto: 'tcp').first_or_create(name: 'redis')

So we still want to support a generic lookup of not needing to provide sname, but still being able to assign a service name to a created service it's not already created.

Although that still doesn't handle the scenario of an existing service with a nil name being present, and a caller having a valid sname that it could be set to. So maybe there isn't a nice oner liner that would work here

dwelch-r7 reviewed Jun 25, 2024

View reviewed changes

lib/metasploit/framework/version.rb Outdated Show resolved Hide resolved

dwelch-r7 reviewed Jun 25, 2024

View reviewed changes

lib/metasploit/framework/login_scanner/redis.rb Outdated Show resolved Hide resolved

bcoles reviewed Jun 25, 2024

View reviewed changes

lib/metasploit/framework/login_scanner/redis.rb Show resolved Hide resolved

adfoster-r7 reviewed Jun 25, 2024

View reviewed changes

lib/msf/core/db_manager/vuln.rb Show resolved Hide resolved

adeherdt-r7 marked this pull request as ready for review June 26, 2024 08:16

adeherdt-r7 force-pushed the adeherdt-r7/MS-9445 branch from 2b0a95e to a640b59 Compare June 26, 2024 09:50

adfoster-r7 reviewed Jun 26, 2024

View reviewed changes

lib/metasploit/framework/login_scanner/redis.rb Show resolved Hide resolved

adeherdt-r7 force-pushed the adeherdt-r7/MS-9445 branch from a1eb197 to 8eeaf1b Compare June 26, 2024 13:02

MS-9445 Fix Service Reporting

51176e7

Preliminary pull request to resolve an issue with a service not being properly detected for Redis. * Ensure service name is properly passed down when detecting vulnerabilities * Ensure Redis properly detects no-auth requirements

adeherdt-r7 force-pushed the adeherdt-r7/MS-9445 branch from 8eeaf1b to 51176e7 Compare June 26, 2024 13:11

adfoster-r7 changed the title ~~MS-9445 Fix Service Reporting~~ Fix Redis Service Reporting Jun 26, 2024

adfoster-r7 merged commit 9343a35 into master Jun 26, 2024
140 checks passed

adfoster-r7 added rn-enhancement release notes enhancement rn-fix release notes fix and removed rn-enhancement release notes enhancement labels Jun 26, 2024

adfoster-r7 reviewed Jun 26, 2024

View reviewed changes

adeherdt-r7 deleted the adeherdt-r7/MS-9445 branch August 1, 2024 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Redis Service Reporting #19283

Fix Redis Service Reporting #19283

adeherdt-r7 commented Jun 25, 2024 •

edited

Loading

adfoster-r7 commented Jun 26, 2024

adfoster-r7 commented Jun 26, 2024

adeherdt-r7 commented Jun 26, 2024

adfoster-r7 commented Jun 26, 2024 •

edited

Loading

adfoster-r7 commented Jun 26, 2024

adeherdt-r7 commented Jun 26, 2024

adeherdt-r7 commented Jun 26, 2024

adfoster-r7 commented Jun 26, 2024

adfoster-r7 commented Jun 26, 2024 •

edited

Loading

adfoster-r7 Jun 26, 2024

adfoster-r7 Jun 26, 2024

Fix Redis Service Reporting #19283

Fix Redis Service Reporting #19283

Conversation

adeherdt-r7 commented Jun 25, 2024 • edited Loading

Verification

adfoster-r7 commented Jun 26, 2024

adfoster-r7 commented Jun 26, 2024

adeherdt-r7 commented Jun 26, 2024

adfoster-r7 commented Jun 26, 2024 • edited Loading

No auth 🟢

Auth 🔴

adfoster-r7 commented Jun 26, 2024

adeherdt-r7 commented Jun 26, 2024

adeherdt-r7 commented Jun 26, 2024

adfoster-r7 commented Jun 26, 2024

No auth 🟢

Valid cred 🟢

Invalid cred 🟢

Services 🍏

adfoster-r7 commented Jun 26, 2024 • edited Loading

Release notes

adfoster-r7 Jun 26, 2024

Choose a reason for hiding this comment

adfoster-r7 Jun 26, 2024

Choose a reason for hiding this comment

adeherdt-r7 commented Jun 25, 2024 •

edited

Loading

adfoster-r7 commented Jun 26, 2024 •

edited

Loading

adfoster-r7 commented Jun 26, 2024 •

edited

Loading