Adding an option to specify a target URL #13766

dwelch-r7 · 2020-06-24T22:37:42Z

This PR is for an additional option that allows users to specify a URL (i.e. set RHOST_URL https://example.com/path) rather than setting each individual option that is the current workflow (i.e. set RHOSTS example.com, set RPORT 443, set SSL true ... etc)

For full details on the various approaches to adding URL Support to Metasploit:
https://github.com/rapid7/metasploit-framework/wiki/RFC---Metasploit-URL-support

I currently only have the new option added to Exploit::Remote::HttpClient but I'm sure there are a few other places it also belongs, more than happy to add it in anywhere that makes sense

Here is an example of the setting in action:

The reverse is also true, i.e. you can set multiple individual values and the RHOST_URL will be displayed for you:

Http username and password is also supported:

Some things to note with the current implementation:

Only single URLs may be specified (In other words you are not able specify an address range in the new option), attempting this will blank out the RHOST_URL option, but any previous options that were set will remain.
URLs of the format example.com notably missing the scheme are not supported, you must do //example.com:<port> or even better https://example.com

To sum up, if this feature is to be added users would be able to simply input set RHOST_URL and paste in their target rather than having to set anywhere up to 6 different options depending on what that particular module cared about.

adfoster-r7 · 2020-06-24T23:23:38Z

lib/msf/core/opt_rhost_url.rb

+          # The datastore uses both `TARGETURI` and `URI` to denote the path of a URL, we try both here and fall back to `/`
+          uri.path=(datastore['TARGETURI'] || datastore['URI'] || '/')
+          uri.user=datastore['HttpUsername']
+          uri.password=datastore['HttpPassword'] if uri.user


Looks like the protocol's getting dropped at the minute, can we slice in the scheme too? 👀

> set RHOST_URL http://google.com RHOST_URL => //google.com:80

Not in a reliable way that I was able to come up with no
see we aren't actually storing the scheme on our end, we'd have to sort of reverse engineer the scheme from the provided options but there's no guarantee we'd get it right, we could end up having the user set a url then immediately be showing them one with a different scheme

I think what I could do though is slice off the leading // that would solve the problem I think

It's an interesting discussion point. The original RFC made notes about different schemes too - such as ftp, postgres, smb, etc.

Unless you think it'll cause more confusion not having those formats work from the get go I still think this current iteration would benefit a majority of users and we can always add mroe use cases down the line when it becomes apparent they are warranted

I'm just keeping an open mind for now, discussing how the different implementations could impact various workflows. We want to reduce the possibility of confusing users in the initial release, and avoiding potential confusion for when we do support more use cases in the future. Finding the right balance between usable, shippable, and iterable will be good 👍

I wonder if things would become easier if this was renamed from the current generic name to something more specific like OptHttpUrl, that way the correct protocol scheme is easy to infer 🤔

Alan fights for the users.

adfoster-r7 · 2020-06-26T11:18:42Z

@dwelch-r7 Are there any additional edge cases to consider? For instance not having a module loaded, setting RHOST_URL, then loading the module and running it? Would that cause any bugs or unexpected weirdness?

setg RHOST_URL http://example.com/foo
use scanner/http/title
run

Or running a module without setting any url information:

use scanner/http/title
run

Which I think will cause a crash with the current implementation

dwelch-r7 · 2020-06-26T11:36:20Z

@dwelch-r7 Are there any additional edge cases to consider? For instance not having a module loaded, setting RHOST_URL, then loading the module and running it? Would that cause any bugs or unexpected weirdness?
setg RHOST_URL http://example.com/foo
use scanner/http/title
run
Or running a module without setting any url information:
use scanner/http/title
run
Which I think will cause a crash with the current implementation

uhhh yea looks like you're right, I did not realise it was trying to validate it on setting and when running the module, I'll figure something out

adfoster-r7 · 2020-06-26T16:36:12Z

Spotted another edgecase:

msf5 exploit(windows/http/rejetto_hfs_exec) > set RHOST_URL http://10.10.87.250:8080
RHOST_URL => 10.10.87.250:8080
msf5 exploit(windows/http/rejetto_hfs_exec) > options

Module options (exploit/windows/http/rejetto_hfs_exec):

   Name       Current Setting    Required  Description
   ----       ---------------    --------  -----------
   HTTPDELAY  10                 no        Seconds to wait before terminating web server
   Proxies                       no        A proxy chain of format type:host:port[,type:host:port][...]
   RHOSTS     10.10.87.250       yes       The target host(s), range CIDR identifier, or hosts file with syntax 'file:<path>'
   RHOST_URL  10.10.87.250:8080  no        The target URL, only applicable if there is a single URL
   RPORT      8080               yes       The target port (TCP)
   SRVHOST    0.0.0.0            yes       The local host or network interface ...
   SRVPORT    8080               yes       The local port to listen on.
   SSL        false              no        Negotiate SSL/TLS for outgoing connections
   SSLCert                       no        Path to a custom SSL certificate (default is randomly generated)
   TARGETURI                     yes       The path of the web application
   URIPATH                       no        The URI to use for this exploit (default is random)
   VHOST                         no        HTTP server virtual host

msf5 exploit(windows/http/rejetto_hfs_exec) > run

[-] Exploit failed: bad URI(is not URI?): "10.10.87.250:8080"
[*] Exploit completed, but no session was created.

Looks like it doesn't work well for modules that require a TARGETURI, the default value was /

dwelch-r7 · 2020-06-26T16:41:37Z

Spotted another edgecase:

msf5 exploit(windows/http/rejetto_hfs_exec) > set RHOST_URL http://10.10.87.250:8080
RHOST_URL => 10.10.87.250:8080
msf5 exploit(windows/http/rejetto_hfs_exec) > options

Module options (exploit/windows/http/rejetto_hfs_exec):

   Name       Current Setting    Required  Description
   ----       ---------------    --------  -----------
   HTTPDELAY  10                 no        Seconds to wait before terminating web server
   Proxies                       no        A proxy chain of format type:host:port[,type:host:port][...]
   RHOSTS     10.10.87.250       yes       The target host(s), range CIDR identifier, or hosts file with syntax 'file:<path>'
   RHOST_URL  10.10.87.250:8080  no        The target URL, only applicable if there is a single URL
   RPORT      8080               yes       The target port (TCP)
   SRVHOST    0.0.0.0            yes       The local host or network interface ...
   SRVPORT    8080               yes       The local port to listen on.
   SSL        false              no        Negotiate SSL/TLS for outgoing connections
   SSLCert                       no        Path to a custom SSL certificate (default is randomly generated)
   TARGETURI                     yes       The path of the web application
   URIPATH                       no        The URI to use for this exploit (default is random)
   VHOST                         no        HTTP server virtual host

msf5 exploit(windows/http/rejetto_hfs_exec) > run

[-] Exploit failed: bad URI(is not URI?): "10.10.87.250:8080"
[*] Exploit completed, but no session was created.

Looks like it doesn't work well for modules that require a TARGETURI, the default value was /

Pushed up a fix for that, I had that in there at one point and removed it for some reason

adfoster-r7 · 2020-06-26T16:54:51Z

@dwelch-r7 I think I was expecting semantics similar to curl which handles that scenario gracefully:

$ curl -v http://example.com
...
> GET / HTTP/1.1
...

adfoster-r7 · 2020-07-01T15:46:33Z

We should double check the ability to check this works with run OPTIONS on the command line

wvu · 2020-07-08T23:03:49Z

lib/msf/ui/console/command_dispatcher/core.rb

+  # Sets a name to a value in a context aware environment.
+  #
+  def set_option(name, value, global: false, append: false)
+
+    # Determine which data store we're operating on
+    if active_module and !global
+      datastore = active_module.datastore
+    else
+      global = true
+      datastore = self.framework.datastore
+    end


Suggested change

# Sets a name to a value in a context aware environment.

#

def set_option(name, value, global: false, append: false)

# Determine which data store we're operating on

if active_module and !global

datastore = active_module.datastore

else

global = true

datastore = self.framework.datastore

end

# Sets a name to a value in a context-aware environment.

#

def set_option(name, value, global: false, append: false)

# Determine which datastore we're operating on

if active_module && !global

datastore = active_module.datastore

else

global = true

datastore = framework.datastore

end

wvu · 2020-07-08T23:05:02Z

lib/msf/ui/console/command_dispatcher/core.rb

-    elsif (args.length == 1)
-      if (not datastore[args[0]].nil?)
+    elsif args.length == 1
+      if not datastore[args[0]].nil?


Suggested change

if not datastore[args[0]].nil?

if datastore[args[0]]

wvu · 2020-07-08T23:06:19Z

lib/msf/ui/console/command_dispatcher/core.rb

+                Msf::Serializer::ReadableText.dump_datastore(
+                    (global) ? "Global" : "Module: #{active_module.refname}",
+                    datastore) + "\n")


I can't tell what this formatting change is for.

wvu · 2020-07-08T23:07:08Z

lib/msf/ui/console/command_dispatcher/core.rb

+                  Msf::Serializer::ReadableText.dump_datastore(
+                      "Global", framework.datastore))


I can't tell what this formatting change is for.

wvu · 2020-07-08T23:10:49Z

lib/msf/core/option_container.rb

@@ -18,6 +18,7 @@ module Msf
  autoload :OptRaw, 'msf/core/opt_raw'
  autoload :OptRegexp, 'msf/core/opt_regexp'
  autoload :OptString, 'msf/core/opt_string'
+  autoload :OptRhostUrl, 'msf/core/opt_rhost_url'


Suggested change

autoload :OptRhostUrl, 'msf/core/opt_rhost_url'

autoload :OptRhostURL, 'msf/core/opt_rhost_url'

https://github.com/rubocop-hq/ruby-style-guide#camelcase-for-classes

wvu · 2020-07-08T23:13:41Z

lib/msf/core/data_store.rb

+    if v.is_a? Hash
+      v.each { |key, value| self[key] = value }
+    else
+      super(k,v)


Suggested change

super(k,v)

super(k, v)

wvu · 2020-07-08T23:14:07Z

lib/msf/core/data_store.rb

  end

  #
  # Case-insensitive wrapper around hash lookup
  #
  def [](k)
-    super(find_key_case(k))
+    k = find_key_case(k)


Suggested change

k = find_key_case(k)

k = find_key_case(k)

wvu · 2020-07-08T23:27:24Z

lib/msf/core/opt.rb

@@ -62,6 +62,10 @@ def self.SSLVersion
      )
    end

+    def self.RHOST_URL(default=nil, required=false, desc="The target URL, only applicable if there is a single URL")


Suggested change

def self.RHOST_URL(default=nil, required=false, desc="The target URL, only applicable if there is a single URL")

def self.RHOST_URL(default = nil, required = false, desc = 'The target URL, only applicable if there is a single URL')

https://github.com/rubocop-hq/ruby-style-guide#spaces-around-equals

wvu · 2020-07-08T23:29:24Z

lib/msf/core/opt.rb

@@ -62,6 +62,10 @@ def self.SSLVersion
      )
    end

+    def self.RHOST_URL(default=nil, required=false, desc="The target URL, only applicable if there is a single URL")
+      Msf::OptRhostUrl.new(__method__.to_s, [ required, desc, default ])


https://github.com/rapid7/metasploit-framework/pull/13766/files#r451874140

wvu · 2020-07-08T23:29:50Z

lib/msf/core/opt_rhost_url.rb

+  # RHOST URL option.
+  #
+  ###
+  class OptRhostUrl < OptBase


https://github.com/rapid7/metasploit-framework/pull/13766/files#r451874140

wvu · 2020-07-08T23:30:28Z

lib/msf/core/opt_rhost_url.rb

+      'rhost url'
+    end
+
+


Suggested change

wvu · 2020-07-08T23:32:29Z

lib/msf/core/opt_rhost_url.rb

+
+      option_hash['RHOSTS'] = uri.hostname
+      option_hash['RPORT'] = uri.port
+      option_hash['SSL'] = %w{ssl https}.include?(uri.scheme)


Suggested change

option_hash['SSL'] = %w{ssl https}.include?(uri.scheme)

option_hash['SSL'] = %w[ssl https].include?(uri.scheme)

https://github.com/rubocop-hq/ruby-style-guide#percent-literal-braces

During module hacking we discussed just running Rubocop over this file in its entirety, there's a things to lint such as a few unneeded uses of and etc.

#13766 (review)

Sounds good.

wvu · 2020-07-08T23:57:59Z

lib/msf/core/opt_rhost_url.rb

+      option_hash['TARGETURI'] = uri.path || '/'
+      option_hash['URI'] = uri.path || '/'
+
+      if uri.scheme and %{http https}.include?(uri.scheme)


Suggested change

if uri.scheme and %{http https}.include?(uri.scheme)

if uri.scheme && %[http https].include?(uri.scheme)

https://github.com/rapid7/metasploit-framework/pull/13766/files#r451880965

wvu · 2020-07-09T00:00:33Z

lib/msf/core/opt_rhost_url.rb

+    def valid?(value, check_empty: false)
+      return true unless value
+      uri = get_uri(value)
+      false unless uri.host != nil and uri.port != nil


Suggested change

false unless uri.host != nil and uri.port != nil

return false if uri.host.nil? && uri.port.nil?

Did you mean to return? What happens with the super?

wvu · 2020-07-09T00:04:43Z

lib/msf/core/opt_rhost_url.rb

+    def get_uri(value)
+      begin
+        uri = URI(value)
+      rescue URI::InvalidURIError
+        uri = URI('//' + value)
+      end
+      uri
+    end


Suggested change

def get_uri(value)

begin

uri = URI(value)

rescue URI::InvalidURIError

uri = URI('//' + value)

end

uri

end

def get_uri(value)

URI(value)

rescue URI::InvalidURIError

URI("//#{value}")

end

https://github.com/rubocop-hq/ruby-style-guide#implicit-begin

wvu · 2020-07-09T00:05:57Z

lib/msf/core/opt_rhost_url.rb

+    end
+
+    def calculate_value(datastore)
+      if datastore['RHOSTS']


Guard clause, please?

https://github.com/rubocop-hq/ruby-style-guide#nested-conditionals

wvu · 2020-07-09T00:06:21Z

lib/msf/core/opt_rhost_url.rb

+
+    def calculate_value(datastore)
+      if datastore['RHOSTS']
+        begin


https://github.com/rapid7/metasploit-framework/pull/13766/files#r451890300

wvu

It might be a good idea to use RuboCop.

adfoster-r7 · 2020-07-16T09:26:36Z

lib/msf/ui/console/command_dispatcher/core.rb

+  #
+  # Sets a name to a value in a context aware environment.
+  #
+  def set_option(name, value, global: false, append: false)


Can we revert this file now? 🤔

@dwelch-r7 Just confirming if this file can be reverted or not 👍

It's not needed for these changes no, but I think they're good changes, I can revert and put it up as a separate PR, that probably makes more sense

adfoster-r7 · 2020-07-16T09:32:33Z

lib/msf/core/exploit/http/client.rb

@@ -22,6 +22,7 @@ def initialize(info = {})

    register_options(


Let's add a feature flag for this by modifying the feature manager's DEFAULTS and conditionally checking the include:

if framework.features.enabled?("feature_name") register_options([OPT::RHOST_URL]) end

Would you mind pulling the latest of the #13833 PR to get the latest features API

adfoster-r7 reviewed Jun 24, 2020

View reviewed changes

dwelch-r7 added the feature label Jun 25, 2020

wvu reviewed Jul 8, 2020

View reviewed changes

lib/msf/core/opt_rhost_url.rb Outdated

'rhost url'

end

Copy link

Contributor

wvu Jul 8, 2020

Choose a reason for hiding this comment

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

Suggested change

wvu reviewed Jul 8, 2020

View reviewed changes

wvu reviewed Jul 9, 2020

View reviewed changes

wvu suggested changes Jul 9, 2020

View reviewed changes

dwelch-r7 force-pushed the rhost-url-option-3 branch from 6578c63 to 1a61308 Compare July 15, 2020 12:37

adfoster-r7 added 2 commits July 15, 2020 21:00

Add WrappedTable support with feature flag integration

25305ee

Add feature configuration persistence

fdb4499

adfoster-r7 reviewed Jul 16, 2020

View reviewed changes

dwelch-r7 added 9 commits July 29, 2020 13:16

Add rhost url macro

a644a58

Add new rhost url option

729e83a

Check for an RHOSTS entry before attempting to create a URI

cc52f3a

remove leading // from rhost url option

79c0c43

If no path is specified default to /

53e89f7

Add some extra validation and checks

d72b846

Ran rubocop

13fcabe

Code review changes

4125f75

Refactor rhost_url to be http/https only

fbd0d0e

dwelch-r7 force-pushed the rhost-url-option-3 branch from cdea10f to fbd0d0e Compare July 31, 2020 11:05

dwelch-r7 added 7 commits July 31, 2020 12:22

use HTTP/HTTPS uris when re-calculating the URI

dd0a883

Add initial test suite for rhost url

6c0d3bf

More tests

b14a74e

Add guard clause

f4def25

Add feature flag and start fixing tests

1a634fc

More test fixes

0fdac85

Fixed all tests

ffbc5cb

jmartin-tech closed this Aug 6, 2020

dwelch-r7 mentioned this pull request Aug 20, 2020

Adds rhost url support behind a feature flag #13961

Merged

		Msf::Serializer::ReadableText.dump_datastore(
		"Global", framework.datastore))

	autoload :OptRhostUrl, 'msf/core/opt_rhost_url'
	autoload :OptRhostURL, 'msf/core/opt_rhost_url'

	def self.RHOST_URL(default=nil, required=false, desc="The target URL, only applicable if there is a single URL")
	def self.RHOST_URL(default = nil, required = false, desc = 'The target URL, only applicable if there is a single URL')

	option_hash['SSL'] = %w{ssl https}.include?(uri.scheme)
	option_hash['SSL'] = %w[ssl https].include?(uri.scheme)

	if uri.scheme and %{http https}.include?(uri.scheme)
	if uri.scheme && %[http https].include?(uri.scheme)

	false unless uri.host != nil and uri.port != nil
	return false if uri.host.nil? && uri.port.nil?

		@@ -22,6 +22,7 @@ def initialize(info = {})

		register_options(

Adding an option to specify a target URL #13766

Adding an option to specify a target URL #13766

Conversation

dwelch-r7 commented Jun 24, 2020 • edited by adfoster-r7 Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adfoster-r7 Jun 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adfoster-r7 commented Jun 26, 2020 • edited Loading

dwelch-r7 commented Jun 26, 2020

adfoster-r7 commented Jun 26, 2020

dwelch-r7 commented Jun 26, 2020

adfoster-r7 commented Jun 26, 2020

adfoster-r7 commented Jul 1, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wvu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adfoster-r7 Jul 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dwelch-r7 commented Jun 24, 2020 •

edited by adfoster-r7

Loading

adfoster-r7 Jun 26, 2020 •

edited

Loading

adfoster-r7 commented Jun 26, 2020 •

edited

Loading

adfoster-r7 Jul 16, 2020 •

edited

Loading