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

Refactor proxy-enabled payload handling #4295

Merged
merged 14 commits into from
Mar 18, 2015
Merged

Refactor proxy-enabled payload handling #4295

merged 14 commits into from
Mar 18, 2015

Conversation

hdm
Copy link
Contributor

@hdm hdm commented Dec 2, 2014

This fixes a couple bugs and generally cleans up the proxy option parsing code.

zeroSteiner
zeroSteiner reviewed Dec 2, 2014
View reviewed changes
modules/payloads/stagers/python/reverse_http.rb
@@ -24,8 +24,8 @@ def initialize(info = {})

register_options(
[
OptString.new('PROXYHOST', [ false, "The address of an http proxy to use", "" ]),
OptInt.new('PROXYPORT', [ false, "The Proxy port to connect to", 8080 ])
OptString.new('PROXY_HOST', [true, "The proxy server's IP address", "127.0.0.1"]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing this to a required option and defaulting it to 127.0.0.1 instead of an empty string is causing L50 to evaluate to true and the stager to explicitly use http://127.0.0.1:8080/ as a proxy which wont be valid in most cases.

@zeroSteiner
Copy link
Contributor

The python reverse_http stager is working now with and without PROXY_HOST specified. There is a lot of library refactoring going on here so I'm going to test this more thoroughly in a few days to give other people a chance to review and comment.

jlee-r7
jlee-r7 reviewed Dec 3, 2014
View reviewed changes
lib/msf/core/handler/reverse_http.rb
@@ -56,18 +56,12 @@ def initialize(info = {})
], Msf::Handler::ReverseHttp)
end

# Toggle for IPv4 vs IPv6 mode
#
def ipv6?
Copy link
Contributor

Choose a reason for hiding this comment

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

Why inline this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The previous code only considered LHOST and used that to globally define ipv6 support, where the actual use case needs to handle PROXY_HOST and LHOST separately.

@todb-r7 todb-r7 added the bug label Dec 3, 2014
@hdm hdm mentioned this pull request Dec 14, 2014
Merged
@hdm
Copy link
Contributor Author

hdm commented Dec 15, 2014

Internet Explorer does accept IPv6 addresses and only if they are in brackets.

@hdm
Copy link
Contributor Author

hdm commented Dec 17, 2014

This will need a rebase since #3632 was landed

@hdm
Copy link
Contributor Author

hdm commented Dec 17, 2014

@jlee-r7 It made more sense to handle the IPv6 stuff in this PR instead since #3632 was focused on the migrate use case.

@bcook-r7
Copy link
Contributor

Can you provide us some test cases so we can verify the fixes? Is the rebase still in progress?

@sempervictus
Copy link
Contributor

Been running this for a while now by the way, no issues so far, despite my consistent attempts at mangling the framework.

@bcook-r7
Copy link
Contributor

bcook-r7 commented Mar 5, 2015

Rebasing this wasn't too much trouble, I pushed a new branch here: https://github.com/bcook-r7/metasploit-framework/commits/land-4295-refactor-proxy

I have a squid proxy setup from an earlier PR, anything else that should be tested to verify proxy support works properly?

@bcook-r7
Copy link
Contributor

bcook-r7 commented Mar 5, 2015

Also, the changes from PROXY* to PROXY_* - don't these cause backward compatibility issues that we should be aware of with upstream tools/scripts?

@bcook-r7
Copy link
Contributor

@hmoore-r7 I believe there are some future PRs that may be waiting on this one to land. Do you mind if I rebase with a new PR, or do you want to update this one?

@hdm
Copy link
Contributor Author

hdm commented Mar 13, 2015

@bcook-r7 Please do! Feel free to close this and push up your rebased PR. Not concerned with back-compat given the newness of proxy support in general.

@hdm
Copy link
Contributor Author

hdm commented Mar 16, 2015

@bcook-r7 Feel free to close this out once the new PR is up. I took a different approach with the reverse_http(s) factor in terms of the option names StagerProxy(Host|Port|User|Pass) but am happy to change so long as were consistent.

@bcook-r7 bcook-r7 mentioned this pull request Mar 16, 2015
Closed
@bcook-r7
Copy link
Contributor

Updated PR at #4935, still double-checking it myself since I just re-rebased it on master.

@bcook-r7 bcook-r7 closed this Mar 16, 2015
@bcook-r7
Copy link
Contributor

I incorrectly rebased this PR since it undid the refactoring work in 1930eb1, resurrecting this PR.

@bcook-r7 bcook-r7 reopened this Mar 17, 2015
HD Moore added 2 commits March 18, 2015 00:59
HD Moore added 6 commits March 18, 2015 00:59
Fix up the offset detection again, cleanup redundant code
85fb534
Cleanup proxy handling code (consistency & bugs)
a4df6d5 
One subtle bug was that each time a request was received, a null byte was being appended to the datastore options for PROXY_USERNAME and PROXY_PASSWORD. Eventually this would break new sessions. This change centralizes the proxy configuration and cleans up the logic.
Don't mandate and default PROXY_HOST (miscopy from the proxy stager)
0601946
Adds IPv6 and option-related issues with the previous patch
2ab14e7
Remove user/pass and invalid class from the options
259db26
Place an IPv6 proxy IP between brackets
87a4899
@hdm hdm force-pushed the refactor/http-proxy-settings branch from 9ee78a6 to 87a4899 Compare March 18, 2015 06:04
@hdm hdm mentioned this pull request Mar 18, 2015
Merged
@OJ OJ assigned OJ and unassigned bcook-r7 Mar 18, 2015
@OJ OJ merged commit 97def50 into rapid7:master Mar 18, 2015
OJ added a commit that referenced this pull request Mar 18, 2015
@OJ
Land #4295 : Refactory proxy-enabled payload handling
b46e5f8
techpeace pushed a commit to techpeace/metasploit-framework that referenced this pull request Mar 19, 2015
@mbuck-r7
Merge master
2056ff6 
Squashed commit of the following:

commit 1dcad7c
Merge: 1a2f35d 35d29f5
Author: OJ <oj@buffered.io>
Date:   Thu Mar 19 14:43:27 2015 +1000

    Land rapid7#4953 : Updated POSIX meterpreter binaries

commit 35d29f5
Author: Brent Cook <bcook@rapid7.com>
Date:   Wed Mar 18 22:57:03 2015 -0500

    update linux meterpreter bins

commit 1a2f35d
Merge: 076f15f 346b1d5
Author: OJ <oj@buffered.io>
Date:   Thu Mar 19 12:41:20 2015 +1000

    Land rapid7#4951: Dynamic URI generation for Java/Python reverse_http(s)

commit 076f15f
Merge: b33e7f4 3f8ed56
Author: Spencer McIntyre <zeroSteiner@gmail.com>
Date:   Wed Mar 18 20:59:54 2015 -0400

    Land rapid7#4792 @jakxx Publish It PUI file exploit

commit 3f8ed56
Author: Spencer McIntyre <zeroSteiner@gmail.com>
Date:   Wed Mar 18 20:57:58 2015 -0400

    Add available space to the payload info

commit b33e7f4
Merge: 0d1f205 5dd718e
Author: joev <joev@metasploit.com>
Date:   Wed Mar 18 17:17:34 2015 -0500

    Land rapid7#4947, h0ng10's TWiki exploit.

commit 346b1d5
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 16:24:01 2015 -0500

    Revert Java back to static size for cache purposes (less cpu usage on startup)

commit 33bbf7c
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 16:08:11 2015 -0500

    Dynamic URI generation for python/java http(s) stagers

commit 0d1f205
Merge: e943cb5 dab4333
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 15:31:22 2015 -0500

    Lands rapid7#4949 which fixes rapid7#4845

commit dab4333
Author: rwhitcroft <rw81junk@gmail.com>
Date:   Wed Mar 18 16:07:46 2015 -0400

    updated asm in block

commit 7ae9739
Author: rwhitcroft <rw81junk@gmail.com>
Date:   Wed Mar 18 15:34:31 2015 -0400

    fix x64/reverse_https stager shellcode

commit e943cb5
Merge: d152c41 d1a2f58
Author: OJ <oj@buffered.io>
Date:   Wed Mar 18 22:34:52 2015 +1000

    Land rapid7#4585 : CVE-2015-0975 XXE in OpenNMS

commit d1a2f58
Author: OJ <oj@buffered.io>
Date:   Wed Mar 18 22:17:44 2015 +1000

    Fix of regex for file capture and format tweaks

commit 5dd718e
Author: Hans-Martin Münch (h0ng10) <muench@mogwaisecurity.de>
Date:   Wed Mar 18 09:51:51 2015 +0100

    Better description

commit 00de437
Author: Hans-Martin Münch (h0ng10) <muench@mogwaisecurity.de>
Date:   Wed Mar 18 09:45:08 2015 +0100

    Initial commit

commit fa72423
Author: OJ <oj@buffered.io>
Date:   Wed Mar 18 18:18:54 2015 +1000

    Move the module to the correct location

commit d152c41
Merge: b46e5f8 b62da42
Author: OJ <oj@buffered.io>
Date:   Wed Mar 18 17:42:19 2015 +1000

    Land rapid7#4934 : Proxy and auth support in reverse_http(s)

commit b62da42
Merge: c607cf7 b46e5f8
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 01:51:15 2015 -0500

    Merge branch 'master' into feature/add-proxies-to-wininet

commit b46e5f8
Merge: bd4738b 97def50
Author: OJ <oj@buffered.io>
Date:   Wed Mar 18 16:49:13 2015 +1000

    Land rapid7#4295 : Refactory proxy-enabled payload handling

commit c607cf7
Merge: 0513852 bd4738b
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 01:45:44 2015 -0500

    Merging master

commit 97def50
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 01:26:59 2015 -0500

    Whitespace cleanup

commit 8d3cb8b
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 01:25:42 2015 -0500

    Fix up meterpreter patching arguments and names

commit ef443c8
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 01:21:53 2015 -0500

    Fix overgreed search/replace

commit 390a704
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 01:19:05 2015 -0500

    Cleanup proxyhost/proxyport arguments to match new names

commit f7a06d8
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 01:15:32 2015 -0500

    Rework PROXY_{HOST|PORT|TYPE|USERNAME|PASSWORD) to the new syntax

commit 3aa8cb6
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 01:08:09 2015 -0500

    Fix two use cases of PROXYHOST/PROXYPORT

commit 87a4899
Author: HD Moore <hd_moore@rapid7.com>
Date:   Mon Dec 15 14:48:09 2014 -0600

    Place an IPv6 proxy IP between brackets

commit 259db26
Author: HD Moore <hd_moore@rapid7.com>
Date:   Tue Dec 2 15:36:14 2014 -0600

    Remove user/pass and invalid class from the options

commit 2ab14e7
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 01:01:10 2015 -0500

    Adds IPv6 and option-related issues with the previous patch

commit 0601946
Author: HD Moore <hd_moore@rapid7.com>
Date:   Tue Dec 2 13:29:39 2014 -0600

    Don't mandate and default PROXY_HOST (miscopy from the proxy stager)

commit a4df6d5
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 00:59:59 2015 -0500

    Cleanup proxy handling code (consistency & bugs)

    One subtle bug was that each time a request was received, a null byte was being appended to the datastore options for PROXY_USERNAME and PROXY_PASSWORD. Eventually this would break new sessions. This change centralizes the proxy configuration and cleans up the logic.

commit 85fb534
Author: HD Moore <hd_moore@rapid7.com>
Date:   Tue Dec 2 12:57:30 2014 -0600

    Fix up the offset detection again, cleanup redundant code

commit 2f13988
Author: HD Moore <hd_moore@rapid7.com>
Date:   Tue Dec 2 12:33:53 2014 -0600

    Use OptPort vs OptInt and cleanup the description

commit a01be36
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 18 00:59:13 2015 -0500

    Rework PROXYHOST/PROXYPORT to PROXY_HOST/PROXY_PORT

    This also cleans up the windows reverse_https_proxy stager.

commit b197b7a
Author: jakxx <jakx.ppr@gmail.com>
Date:   Tue Mar 17 19:24:13 2015 -0400

    Additional Updates

    -Removed unused mixin
    -Cleaned up Module name
    -Cleaned up author name

commit bd4738b
Merge: 47a7f99 d7fa0ec
Author: James Lee <egypt@metasploit.com>
Date:   Tue Mar 17 17:37:55 2015 -0500

    Land rapid7#4827, capture and nbns fixups

commit d7fa0ec
Author: James Lee <egypt@metasploit.com>
Date:   Tue Mar 17 17:36:45 2015 -0500

    Let IPAddr#hton do the calculating

commit 47a7f99
Merge: d1d6378 5fd3637
Author: Brent Cook <bcook@rapid7.com>
Date:   Tue Mar 17 16:22:46 2015 -0500

    Land rapid7#4930, @hmoore-r7 winhttp stager certificate check

commit 085e6cc
Author: jakxx <jakx.ppr@gmail.com>
Date:   Tue Mar 17 16:39:56 2015 -0400

    Implemented Recommended Changes

    -corrected spelling error
    -set only option to required
    -dumped header data to included file
    -Used Rex for jmp values

commit 0490af8
Author: jstnkndy <jstnkndy@gmail.com>
Date:   Tue Mar 17 10:20:22 2015 -0400

    Added error checks, randomness, and uuid delimeter

commit f3fc400
Author: jstnkndy <jstnkndy@gmail.com>
Date:   Tue Mar 17 10:19:40 2015 -0400

    typo

commit b92d243
Merge: e0a7f53 766a07a
Author: jstnkndy <jstnkndy@gmail.com>
Date:   Tue Mar 17 10:18:32 2015 -0400

    Merge branch 'module-cve-2015-0975' of https://github.com/jstnkndy/metasploit-framework into module-cve-2015-0975

commit e0a7f53
Author: jstnkndy <jstnkndy@gmail.com>
Date:   Tue Mar 17 10:10:51 2015 -0400

    Added error checking, randomness, uuid delimiters

commit 2ea9844
Author: HD Moore <hd_moore@rapid7.com>
Date:   Mon Mar 16 14:08:01 2015 -0500

    while(true)->loop, use thread.join

commit 5fd3637
Author: HD Moore <hd_moore@rapid7.com>
Date:   Mon Mar 16 14:00:51 2015 -0500

    Remove the i32 size specifier (not needed)

commit 69d9280
Author: HD Moore <hd_moore@rapid7.com>
Date:   Mon Mar 16 13:52:13 2015 -0500

    Fix yard docs, retries, push.i8 instructions. See commit 0513852

    Note that StagerRetryCount is not defined here, but will be in the parent class once rapid7#4934 lands

commit 0513852
Author: HD Moore <hd_moore@rapid7.com>
Date:   Mon Mar 16 13:35:36 2015 -0500

    Fix yard docs, fix retries, trim bytes, retested and working

commit 69a808b
Author: HD Moore <hd_moore@rapid7.com>
Date:   Mon Mar 16 12:14:42 2015 -0500

    StagerProxy -> PayloadProxy

commit f361e4e
Author: HD Moore <hd_moore@rapid7.com>
Date:   Mon Mar 16 00:22:10 2015 -0500

    Prefer the new-style proxy datastore options when available

commit 7e89281
Author: HD Moore <hd_moore@rapid7.com>
Date:   Mon Mar 16 00:03:31 2015 -0500

    Adds proxy (with authentication) support to reverse_http(s)

commit 8e37342
Author: HD Moore <hd_moore@rapid7.com>
Date:   Sat Mar 14 16:52:04 2015 -0500

    Comment typo

commit 0d12ca4
Author: HD Moore <hd_moore@rapid7.com>
Date:   Sat Mar 14 16:19:13 2015 -0500

    Work around lack of option normalization during size calculation

commit 03019cf
Author: HD Moore <hd_moore@rapid7.com>
Date:   Sat Mar 14 15:53:21 2015 -0500

    Adds StagerVerifySSLCert support (SHA1 of HandlerSSLCert)

commit 1159380
Author: HD Moore <hd_moore@rapid7.com>
Date:   Sat Mar 14 15:52:23 2015 -0500

    Move X509 PEM parsing into Rex::Parser::X509Certificate

commit 1001061
Author: HD Moore <hd_moore@rapid7.com>
Date:   Wed Mar 4 18:52:18 2015 -0600

    Initialize @capture_count

commit 1b1716b
Author: HD Moore <hd_moore@rapid7.com>
Date:   Sun Feb 22 22:01:01 2015 -0600

    Fix a handful of bugs that broke this modules. Fixes rapid7#4799

commit 9730a16
Author: HD Moore <hd_moore@rapid7.com>
Date:   Sun Feb 22 22:00:42 2015 -0600

    Small cleanups to the LLMR responder module

commit bdd5276
Author: HD Moore <hd_moore@rapid7.com>
Date:   Sun Feb 22 21:53:47 2015 -0600

    This fixes a number of issues with the Capture mixin

     * The use of www.metasploit.com in a datastore option results in a DNS lookup (infoleak). Switch to 8.8.8.8 (TTL=1)
     * The hackey code around #each_packet is no longer necessary in newer Ruby versions
     * The arp()/probe_gateway() calls to inject_reply() had broken logic leading to early exit and missed replies
     * The arp() function now tries up to three times to get a reply (helpful with lossy L2)
     * GC.start is extraneous and should be removed
     * Increased timeouts

commit 615d71d
Author: HD Moore <hd_moore@rapid7.com>
Date:   Sun Feb 22 21:51:33 2015 -0600

    Remove extraneous calls to GC.start()

commit 44a7e7e
Author: jakxx <jakx.ppr@gmail.com>
Date:   Wed Feb 18 13:22:54 2015 -0500

    publish-it fileformat exploit

commit 766a07a
Author: jstnkndy <jstnkndy@gmail.com>
Date:   Tue Jan 13 22:08:08 2015 -0500

    Add CVE-2015-0975 XXE for OpenNMS <= 14.0.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@OJ OJ

Labels
bug library module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@hdm @zeroSteiner @bcook-r7 @sempervictus @jlee-r7 @OJ @todb-r7