rapid7 / metasploit-framework

Metasploit Framework
https://www.metasploit.com/
Other
33.88k stars 13.92k forks source link

Disabling Weak SSL Ciphers on reverse_https listeners #6537

Closed ghostop14 closed 5 years ago

ghostop14 commented 8 years ago

Is there a way to create a reverse_https handler and disable weak SSL ciphers for the HTTPS listener? Among a days worth of attempts throughout the framework code, I've tried adding an SSLContext to reverse_http.rb to specify ciphers and disable SSLv2 and SSLv3 but the result is always the same. nmap scans of the created handler always have the default ciphers.

mubix commented 8 years ago

Not that I know of but definitely interested in having this option :+1:

ghostop14 commented 8 years ago

After a long day of trial and error, I did finally figure it out. It's in /opt/metasploit/apps/pro/msf3/lib/rex/socket/ssl_tcp_server.rb

Find these lines in makessl(): ctx = OpenSSL::SSL::SSLContext.new() ctx.key = key ctx.cert = cert ctx.extra_chain_cert = chain ctx.options = 0

And change them to this (adding the ciphers list, and disabling SSLv2/3) ctx = OpenSSL::SSL::SSLContext.new() ctx.key = key ctx.cert = cert ctx.extra_chain_cert = chain ctx.ciphers='HIGH!ADH:!RC4:!aNULL:!eNULL:!MD5:!EXPORT:!SSLv2:!DES:!3DES' ctx.options = 0 ctx.options |= OpenSSL::SSL::OP_NO_SSLv2 ctx.options |= OpenSSL::SSL::OP_NO_SSLv3 ctx.options |= OpenSSL::SSL::OP_NO_COMPRESSION

OJ commented 8 years ago

The cipher list might need revising a little, but the rest looks good. Thanks for investigating this.

So, Windows XP supports TLS 1.0, according to this:

Windows TLS support

If we can be certain that this configuration works with XP, then we can set it as the default. If not, we'll have to find a way of exposing this as an option.

firefart commented 8 years ago

@OJ it depends on the patch version of XP if TLS1.0 is supported. For example XP with IE6 does not support TLS: https://www.ssllabs.com/ssltest/viewClient.html?name=IE&version=6&platform=XP

errors related to wrong SSL settings may be hard to find during a penetration test because often there are only some cryptic error messages.

OJ commented 8 years ago

Good catch.

Fucking XP.

firefart commented 8 years ago

also i think the cipher order ctx.ciphers='HIGH!ADH:!RC4:!aNULL:!eNULL:!MD5:!EXPORT:!SSLv2:!DES:!3DES' may kill IE6 support. Maybe it's better to only change the order from high to low?

ghostop14 commented 8 years ago

Great points. (btw there's a colon missing after HIGH in the string. Picked it up after I pasted it).

Now that I know where it is in the code I can make adjustments. Might be nice if the SSLv2/v3 support and cipher list could be configurable through a global setting.

sempervictus commented 8 years ago

Changing the cipher list to allow low ciphers is self-defeating. MITM on the initial socket negotiation would result in the attacker filtering out the high ciphers from the list, allowing only the low ones to pass into the the DH negotiation, then attacking the resulting context.

This needs to be configurable in the Rex Socket Params module as suggested above in order to ensure the user has enough rope to hang themselves when they screw with this.

Good catch. If nobody else has cycles, i might execute this in the coming days as i've added a number of options in there anyway, and should be able to get this working pretty quick.

@devs: i had submitted some work a while back allowing for configuration of the client side of a TLS connection (cert, etc). If i implement this, i'll be re-introducing those parameters and implementing for client and server sides. Is there some specific reason that we didn't end up using the client options upstream, or should i just resubmit as part of this work?

mubix commented 8 years ago

Currently testing out #6961

msf exploit(regsvr32_applocker_bypass_server) >
[*] Started HTTPS reverse handler on https://172.16.102.1:8443
[-] Exploit failed: OpenSSL::SSL::SSLError SSL_CTX_set_cipher_list: no cipher match

So I had to set an SSLCipher manually:

msf exploit(regsvr32_applocker_bypass_server) > set SSLCipher ADH
SSLCipher => ADH

two are suggested:

   SSLCipher                                      no        String for SSL cipher spec - "DHE-RSA-AES256-SHA" or "ADH"

But what happens is this:

screen shot 2016-06-10 at 2 53 27 pm

And the SCT payload doesn't work because regsvr32 operates via IExplore's info

Should I file this under a separate bug? Seemed to fit in to this issue..

bcook-r7 commented 8 years ago

Hmm, it should not have affected behavior, as the default should still be Ruby's default of:

"ALL:!ADH:!EXPORT:!SSLv2:RC4+RSA:+HIGH:+MEDIUM:+LOW"

What happens if you set it manually to the above string, or even "ALL" ?

bcook-r7 commented 8 years ago

Note, by default we have RC4 turned on (which was also the default earlier), because that's how Ruby works still IIRC. It seems that your browser error is complaining that we do have RC4 enabled. We added this feature more so people could turn it off. A giant, but carefully-constructed, string like this should be compatible down to IE8:

"EECDH+AESGCM:EDH+AESGCM:ECDHE-RSA-AES128-GCM-SHA256:AES256+EECDH:DHE-RSA-AES128-GCM-SHA256:AES256+EDH:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA:ECDHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES256-GCM-SHA384:AES128-GCM-SHA256:AES256-SHA256:AES128-SHA256:AES256-SHA:AES128-SHA:DES-CBC3-SHA:HIGH:!aNULL:!eNULL:!EXPORT:!DES:!MD5:!PSK:!RC4"

sempervictus commented 8 years ago

Given the length of that string, would it make sense to create a lookup table for cipher sets used by specific targets? On Jun 10, 2016 16:15, "Brent Cook" notifications@github.com wrote:

Note, by default we have RC4 turned on (which was also the default earlier), because that's how Ruby works still IIRC. It seems that your browser error is complaining that we do have RC4 enabled. We added this feature more so people could turn it off. A giant, but carefully-constructed, string like this should be compatible down to IE8:

"EECDH+AESGCM:EDH+AESGCM:ECDHE-RSA-AES128-GCM-SHA256:AES256+EECDH:DHE-RSA-AES128-GCM-SHA256:AES256+EDH:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA:ECDHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA256:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA:ECDHE-RSA-DES-CBC3-SHA:EDH-RSA-DES-CBC3-SHA:AES256-GCM-SHA384:AES128-GCM-SHA256:AES256-SHA256:AES128-SHA256:AES256-SHA:AES128-SHA:DES-CBC3-SHA:HIGH:!aNULL:!eNULL:!EXPORT:!DES:!MD5:!PSK:!RC4"

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rapid7/metasploit-framework/issues/6537#issuecomment-225284176, or mute the thread https://github.com/notifications/unsubscribe/ABRPjJQJJXnO1FWlHJeGQcVnYJ2pQdO2ks5qKcXJgaJpZM4HUkaH .

busterb commented 5 years ago

This mostly updates when you upgrade your OpenSSL / Ruby versions. In the last 3 years things have certainly improved. If we want to further restrict what Metasploit provides, we need to update our defaults for the SSL socket configuration.

busterb commented 5 years ago

Verified that things are stronger with newer versions of Ruby and OpenSSL. Not much we should need to do here by default.