Winrm login workaround #13974

dwelch-r7 · 2020-08-10T14:53:11Z

This PR fixes an issue where the winrm login scanner was failing to authenticate with serves that did not accept 'Basic' authentication.

The issues arose from a recent PR which made an attempt to fix logging into winrm servers with 'Basic' authentication here #13442
The data this PR added to the request caused logging into winrm with anything other than 'Basic' authentication to fail

This PR addresses this issue in a couple ways

Only when the preferred auth method is set to 'Basic' will the extra data be sent in the request
The auth method is defaulted to 'Negotiate' whereas previously it was 'Basic'

acammack-r7 · 2020-08-13T14:29:00Z

lib/metasploit/framework/login_scanner/http.rb

@@ -246,7 +246,8 @@ def send_request(opts)
          begin
            cli.connect
            req = cli.request_cgi(opts)
-            res = cli.send_recv(req)
+            # Authenticate by default
+            res = opts['authenticate'].nil? || opts['authenticate'] ? cli.send_recv(req) : cli._send_recv(req)


Suggested change

res = opts['authenticate'].nil? || opts['authenticate'] ? cli.send_recv(req) : cli._send_recv(req)

res = if opts['authenticate'].nil? || opts['authenticate']

cli.send_recv(req)

else

cli._send_recv(req)

end

adfoster-r7 · 2020-08-13T14:38:13Z

lib/metasploit/framework/login_scanner/winrm.rb

+          opts['authenticate'] = false
+          allowed_auth_methods = parse_auth_methods(super(opts))
+          opts['authenticate'] = true


This side effect was unexpected for me, maybe something like this is cleaner:

Suggested change

opts['authenticate'] = false

allowed_auth_methods = parse_auth_methods(super(opts))

opts['authenticate'] = true

allowed_auth_methods = parse_auth_methods(super(opts.merge({ 'authenticate' => true }))

But I don't have the full picture here to know if this is the best approach overall just yet

acammack-r7 · 2020-08-13T14:38:56Z

lib/metasploit/framework/login_scanner/winrm.rb

        # send an HTTP request that WinRM would consider as valid  (SOAP XML in the message matching the XML schema definition)
        def send_request(opts)
+          opts['authenticate'] = false
+          allowed_auth_methods = parse_auth_methods(super(opts))


This would be better off in a check_request override to prevent doubling the volume of requests sent to the server.

adfoster-r7 · 2020-08-18T11:18:19Z

lib/metasploit/framework/login_scanner/winrm.rb

@@ -9,6 +9,7 @@ module LoginScanner

      # Windows Remote Management login scanner
      class WinRM < HTTP
+        # include Msf::Exploit::Remote::WinRM


Is this needed still? 👀

adfoster-r7 · 2020-08-18T11:18:34Z

lib/rex/proto/http/client.rb

@@ -272,7 +272,6 @@ def send_auth(res, opts, t, persist)

    # if several providers are available, the client may want one in particular
    preferred_auth = opts['preferred_auth']
-


Can this file be reverted? 👀

lib/metasploit/framework/login_scanner/http.rb

adfoster-r7 · 2020-08-19T09:58:08Z

lib/metasploit/framework/login_scanner/winrm.rb

+        def parse_auth_methods(resp)
+          return [] unless resp and resp.code == 401
+          methods = []
+          methods << "Negotiate" if resp.headers['WWW-Authenticate'].include? "Negotiate"


@dwelch-r7 Under what circumstances will winrm return this header? I haven't found a target that sends this back yet. What target did you use? 👀

Edit: Found it https://docs.microsoft.com/en-us/windows/win32/winrm/authentication-for-remote-connections

dwelch-r7 · 2020-08-19T10:10:02Z

lib/metasploit/framework/login_scanner/winrm.rb

        # send an HTTP request that WinRM would consider as valid  (SOAP XML in the message matching the XML schema definition)
        def send_request(opts)
+          allowed_auth_methods = parse_auth_methods(super(opts.merge({ 'authenticate' => true })))


Suggested change

allowed_auth_methods = parse_auth_methods(super(opts.merge({ 'authenticate' => true })))

allowed_auth_methods = parse_auth_methods(super(opts.merge({ 'authenticate' => false })))

Typo, will fix

adfoster-r7 · 2020-08-19T10:38:56Z

Tested successfully against metasploitable3, a windows 7 target, and the original box from #13839

Valid credentials:

use auxiliary/scanner/winrm/winrm_login
set RHOSTS 10.10.10.149
set USERNAME chase
set PASSWORD Q4)sJu\\Y8qz*A3?d
run

Output:

[+] 10.10.10.149:5985 - Login Successful: WORKSTATION\chase:Q4)sJu\Y8qz*A3?d
[*] Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed

Invalid credentials working as expected:

[-] 10.10.10.149:5985 - LOGIN FAILED: WORKSTATION\chase:wrong (Incorrect: )
[*] Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed

As a sanity test, I checked auxiliary/scanner/winrm/winrm_cmd - which appears to be broken:

msf6 auxiliary(scanner/winrm/winrm_cmd) > run

[-] Got unexpected response: 
 HTTP/1.1 500
Server: Microsoft-HTTPAPI/2.0
Date: Wed, 19 Aug 2020 10:43:14 GMT
Connection: close
Content-Length: 0


[*] Scanned 1 of 1 hosts (100% complete)
[*] Auxiliary module execution completed
msf6 auxiliary(scanner/winrm/winrm_cmd) >

adfoster-r7 · 2020-08-19T11:25:39Z

Release Notes

Improved the winrm_login module to correctly negotiate authentication, where previously it would always assume that basic auth is required.

acammack-r7 · 2020-08-19T16:55:07Z

This still sends two requests to the WinRM server per credential pair. Was there a reason that the check_setup method couldn't be used to check the authentication method once at the start of the run?

dwelch-r7 · 2020-08-19T18:24:18Z

@acammack-r7 I took a look at the check_setup method, it doesn't seem like the right place to make this kind of check

          # @note Override this to detect that the service is up, is the right
          #   version, etc.
          # @return [false] Indicates there were no errors
          # @return [String] a human-readable error message describing why
          #   this scanner can't run
          def check_setup
            false
          end

seems to be intended more for checking that the scanner can actually run rather than for setting options per server
I checked around to see if there was some other function that'd be more suited but I didn't see one and didn't want to block on making one just for this, but it'd definitely be good to have

acammack-r7 · 2020-08-19T18:53:12Z

It would be an error if the server wanted an authentication method we didn't support or we weren't configured to connect with, correct? We already check that there are authentication methods in

metasploit-framework/lib/metasploit/framework/login_scanner/http.rb

Lines 197 to 198 in d300ddb

    
           if !(response && response.code == 401 && response.headers['WWW-Authenticate']) 
        
             error_message = "No authentication required"

That would also be a reasonable place to set an instance variable and make sure that we use an expected authentication. For an example, see the Glassfish scanner which auto-detects SSL:

metasploit-framework/lib/metasploit/framework/login_scanner/glassfish.rb

Lines 41 to 42 in d300ddb

    
           if !self.ssl && res.headers['Location'] =~ /^https:/ 
        
             self.ssl = true

This code as-is creates a large performance regression for this scanner since it doubles the time it takes to run.

adfoster-r7 · 2020-08-21T13:59:14Z

@acammack-r7 We can circle back to this, for now the module was pretty broke - so a performance regression wasn't a priority in comparison to making it work 📈

sempervictus · 2021-01-05T18:37:46Z

lib/metasploit/framework/login_scanner/winrm.rb

+          if allowed_auth_methods.include? 'Negotiate'
+            opts['preferred_auth'] = 'Negotiate'
+          elsif allowed_auth_methods.include? 'Basic'
+            # Straight up hack since if Basic auth is used winrm complains about the content size being 0


Can we set it to 1 and give them a null byte instead?

dwelch-r7 added 3 commits August 10, 2020 02:03

Add workaround for winrm login modules

ef07ac9

Add clarifying comments

efb3fe8

undo hackiness, apply different less hacky stuff

4d40c62

dwelch-r7 added the bug label Aug 10, 2020

use default of send_recv which does authentication

f761743

gwillcox-r7 added the library label Aug 12, 2020

acammack-r7 reviewed Aug 13, 2020

View reviewed changes

adfoster-r7 reviewed Aug 13, 2020

View reviewed changes

acammack-r7 reviewed Aug 13, 2020

View reviewed changes

Code review comments

a47e4d4

adfoster-r7 reviewed Aug 18, 2020

View reviewed changes

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

adfoster-r7 reviewed Aug 19, 2020

View reviewed changes

dwelch-r7 commented Aug 19, 2020

View reviewed changes

Stop trying to authenticate when poking for headers

897c51d

adfoster-r7 merged commit d488dab into rapid7:master Aug 19, 2020

adfoster-r7 added the rn-enhancement release notes enhancement label Aug 19, 2020

dwelch-r7 mentioned this pull request Jan 5, 2021

auxiliary/scanner/winrm/winrm_cmd: WinRM Authentication Issue (HTTP Error 500) #8900

Closed

1 task

sempervictus reviewed Jan 5, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Winrm login workaround #13974

Winrm login workaround #13974

dwelch-r7 commented Aug 10, 2020

acammack-r7 Aug 13, 2020

adfoster-r7 Aug 13, 2020

acammack-r7 Aug 13, 2020

adfoster-r7 Aug 18, 2020

adfoster-r7 Aug 18, 2020

adfoster-r7 Aug 19, 2020 •

edited

Loading

dwelch-r7 Aug 19, 2020

adfoster-r7 commented Aug 19, 2020

adfoster-r7 commented Aug 19, 2020 •

edited by pbarry-r7

Loading

acammack-r7 commented Aug 19, 2020

dwelch-r7 commented Aug 19, 2020

acammack-r7 commented Aug 19, 2020

adfoster-r7 commented Aug 21, 2020

sempervictus Jan 5, 2021

		@@ -272,7 +272,6 @@ def send_auth(res, opts, t, persist)

		# if several providers are available, the client may want one in particular
		preferred_auth = opts['preferred_auth']

	allowed_auth_methods = parse_auth_methods(super(opts.merge({ 'authenticate' => true })))
	allowed_auth_methods = parse_auth_methods(super(opts.merge({ 'authenticate' => false })))

Winrm login workaround #13974

Winrm login workaround #13974

Conversation

dwelch-r7 commented Aug 10, 2020

acammack-r7 Aug 13, 2020

Choose a reason for hiding this comment

adfoster-r7 Aug 13, 2020

Choose a reason for hiding this comment

acammack-r7 Aug 13, 2020

Choose a reason for hiding this comment

adfoster-r7 Aug 18, 2020

Choose a reason for hiding this comment

adfoster-r7 Aug 18, 2020

Choose a reason for hiding this comment

adfoster-r7 Aug 19, 2020 • edited Loading

Choose a reason for hiding this comment

dwelch-r7 Aug 19, 2020

Choose a reason for hiding this comment

adfoster-r7 commented Aug 19, 2020

adfoster-r7 commented Aug 19, 2020 • edited by pbarry-r7 Loading

Release Notes

acammack-r7 commented Aug 19, 2020

dwelch-r7 commented Aug 19, 2020

acammack-r7 commented Aug 19, 2020

adfoster-r7 commented Aug 21, 2020

sempervictus Jan 5, 2021

Choose a reason for hiding this comment

adfoster-r7 Aug 19, 2020 •

edited

Loading

adfoster-r7 commented Aug 19, 2020 •

edited by pbarry-r7

Loading