-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Detect HTTPS interception #1430
Conversation
Special thanks to @FiloSottile for guidance with the custom listener.
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.
It's looking good at the first glance. Well done Matt!!!
P.S.: I will take a deeper look in the next few days/weeks
As far as I can tell, reading the ClientHello during Accept() prevents new connections from being accepted during the read. Since Read() should be called in its own goroutine, this keeps Accept() non-blocking.
caddyhttp/httpserver/mitm.go
Outdated
// cipher suites missing from the crypto/tls package, | ||
// in no particular order here | ||
TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 = 0xcca9 | ||
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 = 0xcca8 |
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.
ChaCha20 cipher suites are not missing from the crypto/tls package.
https://tip.golang.org/pkg/crypto/tls/#pkg-constants
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305 uint16 = 0xcca8
TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305 uint16 = 0xcca9
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
is not missing in the crypto/tls package.
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.
Oh, thanks.
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.
No problem 😄, in my spare time I will try to review this PR
Not sure if this shows something or nothing, but i tested by editing firefox request as suggested and got expected results. Then I connected using my phone so using the mobile network without editing anything. Got the following results
Would there be a reason that the file I requested list.php is All other requests on this website over mobile network are unlikely. I even tried requesting a non-existant file (404) and got unlikely. Requesting |
The heuristics used in the research will be published probably no sooner than March 1, 2017, but I may merge this PR before then and then iterate on this feature later at that time. |
@tobya Interesting... can you add |
@mholt {remote} and {port} are there (just apart) 178.167.254.199:51060 /list.php https unlikely The ports are differnt. I can reproduce consistently with 3G on my mobile on Chrome for iOS. This is output of headers.
As you guessed the ports are different. This only happens on my mobile phone and only on Chrome, firefox for iOS and Safari for iOS dont cause this. I have uploaded the log with headers for various calls with 3 differnt browsers https://gist.github.com/tobya/8ec9e4b6553264b6e2022b228a58f631 |
@tobya Thanks, this might be a flaw in how I'm guessing the browser from the User-Agent string. It probably thinks it is Safari and not Chrome because Chrome doesn't appear in the UA. 😕 That's annoying. What the heck is CriOS? I don't have iOS so I can't test this. If you want to help fix it, put some print/log statements in the ServeHTTP of mitm.go and see which browser it thinks the client is based on the UA and let's see if it's getting it right, which I bet it's not. It doesn't explain why only some are false positives though... but maybe that will be clear later... The different port means that your phone is opening two separate connections. But why, and why are they somehow different? No clue at this point. If you add a check for "CriOS" (Chrome iOS? I have no idea) here in the UA checking logic of ServeHTTP of mitm.go, does that help with the false positive at all? |
Should I move this to a seperate issue? From my position of ignorance it seems it may be that Chrome on iOS is using a cipher that is in your list of ciphers not supported by Chrome specifically I output
which seems to be Would a fix to be to add CriOS as another specific browser and remove that cipher from the list? |
@tobya This issue is fine, we're almost done I think. I need from you:
And yes, I think that is a reasonable course of action: add CriOS as an alternative to Chrome in our conditions and then remove that cipher from the list of exclusions. If you want to try that in your build too and see if it fixes it, that would be great! And with those two pieces of information I asked you for I'll add this to our test corpus. |
# Conflicts: # caddyhttp/httpserver/server.go
LGTM :) |
@wendigo Thanks. Have you used the "review" feature on Github before? Just so you know, you can leave a review with inline comments and express approval or request changes that way. |
This is a WIP branch which closes #1420. It makes it possible for Caddy to detect when it is likely that the TLS connection is being intercepted. This is possible because TLS proxies are mostly careless. It works by comparing the properties of the TLS Client Hello message to what is expected of the client indicated by the User-Agent HTTP header. If there is a mismatch, a MITM is likely.
The heuristics used here are a partial implementation of The Security Impact of HTTPS Interception by Durumeric, Halderman, and others.
I would like people to try this out -- you will need to compile this branch with Go 1.8.
This PR also adds:
{{.IsMITM}}
template action that returns true or false.{mitm}
placeholder which can be used in the Caddyfile to initiate redirects or add the value to a log entry. It returns likely, unlikely, or unknown.TODO:
An easy way of testing this is to use Firefox to load a HTTPS page, then use the Dev Tools to "Edit and Resend" the HTTP request, changing the User-Agent header to replace "Firefox" with something like "Chrome" or any other browser.
I would like to encourage anyone to test this out and submit feedback, this is a highly experimental feature but one I feel could be important. Any other collaborators that would like to participate in reviewing, please do!