-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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.
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
Add TLS config mirroring #5157
Add TLS config mirroring #5157
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the initial PR! 🍰 Regarding your questions from Slack
Chrome sends GREASE ciphersuites. Can pyOpenSSL be somehow tricked into sending them without patching & recompiling?
I think the question is if OpenSSL can, pyOpenSSL we can make work somehow. Unfortunately I don't think OpenSSL will cooperate. On a more positive note, at least JA3 hashes seem to ignore GREASE entirely.
OpenSSL sends TLS_FALLBACK_SCSV ciphersuite to signal some protection against downgrading. Can I turn it off? (https://wiki.openssl.org/index.php/SSL_MODE_SEND_FALLBACK_SCSV states that unsafe modes must be turned off, but that's against the whole mirroring idea)
That question is slightly confusing to me - when do you see SSL_MODE_SEND_FALLBACK_SCSV? I thought this would only show up on retries if the application sets it explicitly.
from mitmproxy import certs, ctx, exceptions, connection, tls | ||
from mitmproxy.net import tls as net_tls | ||
from mitmproxy.options import CONF_BASENAME | ||
from mitmproxy.proxy import context | ||
from mitmproxy.proxy.layers import modes | ||
from mitmproxy.proxy.layers import tls as proxy_tls | ||
from mitmproxy.utils import ciphersuites as cs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't alias modules unless it's absolutely necessary.
@@ -25,6 +28,11 @@ | |||
) | |||
|
|||
|
|||
class TlsStrategy(Enum): | |||
DEFAULT_CIPHERSET = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a slight preference for naming the default "secure", because one benefit of not mirroring is that we can avoid insecure/old ciphers. Naming it "secure" makes the tradeoff more clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid (I'll rename it), but consider reviewing default cipher list - I don't think DES-CBC3-SHA (at least) counts secure anymore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current list is what Mozilla recommends when old clients might be present (https://ssl-config.mozilla.org/#config=old). You have a valid point though. How about we introduce secure/old/mirror, with secure being turned on by default? We can use the intermediate and old settings on https://ssl-config.mozilla.org/ for secure and old respectively.
@@ -112,6 +129,7 @@ def tls_clienthello(self, tls_clienthello: tls.ClientHelloData): | |||
ctx.options.add_upstream_certs_to_client_chain or | |||
ctx.options.upstream_cert | |||
) | |||
self.client_tls_hello = tls_clienthello.client_hello |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't work with concurrent connections, tls_clienthello
may be called for two different connections before the corresponding tls_start_server
calls are made. The proxyauth addon has an example of how you can keep state for a connection around:
mitmproxy/mitmproxy/addons/proxyauth.py
Line 26 in 65773cc
self.authenticated: MutableMapping[connection.Client, Tuple[str, str]] = weakref.WeakKeyDictionary() |
@@ -25,6 +28,11 @@ | |||
) | |||
|
|||
|
|||
class TlsStrategy(Enum): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of limiting options. However, I'd prefer if we could use typing.Literal
for this:
TlsStrategy = Literal["secure","mirror"]
# getting all options:
choices=typing.get_args(TlsStrategy)
This is a bit simpler to handle than enums, in particular you don't run into issues like omitting .name
(see below).
@@ -199,6 +217,17 @@ def tls_start_server(self, tls_start: tls.TlsData) -> None: | |||
|
|||
if not server.cipher_list and ctx.options.ciphers_server: | |||
server.cipher_list = ctx.options.ciphers_server.split(":") | |||
|
|||
if not server.cipher_list and ctx.options.tls_strategy is not TlsStrategy.DEFAULT_CIPHERSET: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First, I think this line is missing a .name
. Second, please use ==
, we shouldn't rely on string interning here. While it's correct to compare Enums with is
the same does not apply for their name (which is a plain string). Small strings happen to be inlined in Python, but that's nothing that should be relied upon.
get_get_cipher_name converts ciphersuite number to OpenSSL cipher name string. | ||
""" | ||
if num in ciphersuite_names: | ||
return ciphersuite_names[num] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An easier way to accomplish this is to say ciphersuite_names.get(num, None)
. :-)
@@ -225,6 +254,18 @@ def tls_start_server(self, tls_start: tls.TlsData) -> None: | |||
alpn_protos=tuple(server.alpn_offers), | |||
) | |||
|
|||
if ctx.options.tls_strategy is TlsStrategy.MIRROR.name: | |||
# Set TLS1.3 ciphers | |||
# Maybe there is a better way, but I suck at python :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just call SSL._lib.SSL_CTX_set_ciphersuites(ssl_ctx._context, ciphersuites)
directly here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? (Sorry, but I'm not a python coder, so I'd like to know)
What if pyopenssl merges the pull request where this one is from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dislike
ssl_ctx.set_ciphersuites = cs.set_ciphersuites
because it monkeypatches a new method onto an existing object. That can be very confusing to debug down the line, especially when pyOpenSSL merges a slightly different implementation for set_ciphersuites
. You would expect ssl_ctx.set_ciphersuites
to be what pyOpenSSL's provides, but we'd be secretly overwriting it here then.
It's more straightforward if we skip the pyOpenSSL wrapper and directly interact with the OpenSSL API:
SSL._openssl_assert(
SSL._lib.SSL_CTX_set_ciphersuites(ssl_ctx._context, ciphersuites) == 1
)
Not OP, but on my end SCSV suites are sent by pyOpenSSL on the client hello. Specifically, |
That's exactly like @LicketySpliket says - it always appears at the end of the ciphersuites' list, even if I comment it out in map (i.e. it's not copied from client request). Tested it by visiting https://howsmyssl.com with Chrome and Firefox with and without mitmproxy. Also, all requests using curl always send this CS (even without mitmproxy). I suppose this is from OpenSSL itself. |
Thanks. I'm as smart as anyone here, but https://www.openssl.org/docs/manmaster/man3/SSL_get_secure_renegotiation_support.html#SECURE-RENEGOTIATION has two options that could at least be tried out. |
f0383ea
to
7352811
Compare
I am also interested in this feature. I hope this will defeat TLS fingerprinting like cloudflare does which is annoyance for me. One question though: Is it enough to just clone the cipher list or are the extensions also part of the fingerprint? |
That depends on the fingerprint being used. https://github.com/salesforce/ja3 is common, although I'd suspect that Cloudflare uses their own thingy.
(I know as much as you do here)
I'm afraid we kind of need to get our TLS library (OpenSSL curently) to emit the right thing, otherwise the crypto operations will fail later (you can't just modify the ClientHello and expect to get away with it). I'm afraid the options are 1) coerce OpenSSL to do what we want (not sure if possible), or 2) switch to rustls as an alternative TLS backend and modify it to our needs. I refuse to ship a custom patched version of OpenSSL (too much pain), but PyO3 is actually very awesome to use. |
Thanks for the link. That implementation checks more than the cipher list so that is a no then and this PR alone will not solve anything.
With the answer from above this question became irrelevant.
Are you sure? As long as the modification of the client_helo does not advertise a feature that openssl does not support, this should work? The frame is AFAIK not cryptographically secured so any functional modification such as these should pass:
|
The entire ClientHello directly feeds into the transcript hash (https://datatracker.ietf.org/doc/html/rfc8446#section-4.4.1), so modifications will be detected. |
You are right, my memory was a little rusty from the times where cipher downgrade attacks were still possible. :) However, I did look at this a little more. Maybe it is possible to alter the client hello message by calling If that does not work, I think it will get tough, openssl just does not give enough control over the client hello construction process. And the use of |
Talking about fingerprinting, there's also Cloudflare's MALCOLM (https://github.com/cloudflare/mitmengine & https://malcolm.cloudflare.com/) which, as I get it, uses technique other than JA3 |
Thanks for the pointer @fedosgad! I think it's not completely unintentional that OpenSSL does not support spoofing browser ClientHellos. Making rustls work would be a very fun undertaking, but also a considerable effort. |
Upstream merge
From my testings, the one actually matters the most is Extensions, I tried to match browser's ciphersuites, signature algorithms, ec point formats, supported groups and websites that use Cloudflare always return 403 status error because OpenSSL can't really spoof Extensions. You can easily test with curl-impersonate, try to modify User-Agent, even ciphersuite of Chrome and it still passes Cloudflare's checks, because Extensions are still matched with Chrome. In fact I changed heavily chrome_104.config to:
And chrome_104.headers to:
Test cmd (this website uses Cloudflare's anti bot settings):
And it stills works, even with heavily utterly modified Ciphersuite and User-agent. Now, change chrome_104.config again to:
Run above cmd again, and Cloudflare will return 403 because I removed --no-npn to add NPN extension to Chrome, and that's a death sentence because I changed Extensions. So my conclusions, for Cloudflare:
|
Closing this PR because:
Thanks everyone who participated in discussion and special thanks to @mhils for code review and valuable hints about it! |
Description
Modify tlsconfig.py to mirror client's ciphersuites to server
Checklist