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.

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

Merged
merged 15 commits into from
Feb 17, 2017
Merged

Detect HTTPS interception #1430

merged 15 commits into from
Feb 17, 2017

Conversation

mholt
Copy link
Member

@mholt mholt commented Feb 13, 2017

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:

  • Write tests
  • Flush out the Safari heuristics
  • Seek clarification on the IE/Edge heuristics as they seem to overlap with Chrome
  • Reading the TLS Client Hello in Accept() is currently blocking, I think. Verify if this is the case and, if it is, find a way to read the Hello without blocking other connections...
  • A way to close the connection right away if a MITM is suspected (will be exposed via Caddyfile directive later if this feature is useful)

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!

@mholt mholt added help wanted 🆘 Extra attention is needed in progress 🏃‍♂️ Being actively worked on needs docs ✍️ Requires documentation changes needs tests 💯 Requires automated tests labels Feb 13, 2017
@mholt mholt self-assigned this Feb 13, 2017
Copy link
Collaborator

@elcore elcore left a 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

@mholt mholt removed the needs tests 💯 Requires automated tests label Feb 14, 2017
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.
// 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
Copy link
Collaborator

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, thanks.

Copy link
Collaborator

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

@mholt mholt removed in progress 🏃‍♂️ Being actively worked on needs docs ✍️ Requires documentation changes labels Feb 15, 2017
@tobya
Copy link
Collaborator

tobya commented Feb 15, 2017

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

178.167.254.199 - [15/Feb/2017:15:48:19 +0000] 'GET  /list.php /list.php  - HTTP/2.0 200 51060' https unlikely
178.167.254.199 - [15/Feb/2017:15:48:23 +0000] 'GET  /favicon.ico /favicon.ico  - HTTP/2.0 404 58442' https likely

Would there be a reason that the file I requested list.php is unlikely but the favicon.ico (which doesnt exist) would be likely?

All other requests on this website over mobile network are unlikely. I even tried requesting a non-existant file (404) and got unlikely. Requesting favicon.ico a second time give a unlikely

@mholt
Copy link
Member Author

mholt commented Feb 15, 2017

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.

@mholt
Copy link
Member Author

mholt commented Feb 15, 2017

@tobya Interesting... can you add {remote}:{port} to your log format and see if they are the same? Also, since you're building this from source, if you dump the request headers (fmt.Printf("Headers: %+v", r.Header) might be helpful too. Also, which browser are you using on mobile?

@tobya
Copy link
Collaborator

tobya commented Feb 15, 2017

@mholt {remote} and {port} are there (just apart)

178.167.254.199:51060 /list.php https unlikely
178.167.254.199:58442 /favicon.ico' https likely

The ports are differnt.

I can reproduce consistently with 3G on my mobile on Chrome for iOS. This is output of headers.

178.167.254.34:58772 - [15/Feb/2017:21:59:47 +0000] 'GET asite4.bcs.cookingisfun.ie  /list.php /list.php  - HTTP/2.0 200 ' https unlikely

Headers: map[Accept-Encoding:[gzip, deflate] Cookie:[_ga=GA1.2.1285204035.1421831904; __utma=71723596.1285204035.1421831904.1478764756.1478864814.78;     __utmz=71723596.1478705289.76.15.utmcsr=jobsforcooks.com|utmccn=(referral)|utmcmd=referral|utmcct=/employers/] Accept:[text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8] User-Agent:[Mozilla/5.0 (    iPhone; CPU iPhone OS 10_0_2 like Mac OS X) AppleWebKit/602.1.50 (KHTML, like Gecko) CriOS/56.0.2924.79 Mobile/14A456 Safari/602.1] Accept-Language:[en-ie]]

178.167.254.34:50453 - [15/Feb/2017:21:59:48 +0000] 'GET asite4.bcs.cookingisfun.ie  /favicon.ico /favicon.ico  - HTTP/2.0 404 ' https likely

Headers: map[User-Agent:[Mozilla/5.0 (iPhone; CPU iPhone OS 10_0_2 like Mac OS X) AppleWebKit/602.1.50 (KHTML, like Gecko) CriOS/56.0.2924.79 Mobile/14A456 Safari/602.1] Accept-Encoding:[gzip, deflate, sdch]     Accept-Language:[en-US,en;q=0.8]]

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

@mholt
Copy link
Member Author

mholt commented Feb 15, 2017

@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?

@mholt mholt removed the help wanted 🆘 Extra attention is needed label Feb 16, 2017
@tobya
Copy link
Collaborator

tobya commented Feb 16, 2017

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 ext at this line and output

uint16=49188

which seems to be TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384

Would a fix to be to add CriOS as another specific browser and remove that cipher from the list?

@mholt
Copy link
Member Author

mholt commented Feb 16, 2017

@tobya This issue is fine, we're almost done I think.

I need from you:

  • The hex encoding of the TLS ClientHello (the current commit on this branch prints this out for you)
  • The exact browser and iOS versions

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.

@wendigo
Copy link

wendigo commented Feb 17, 2017

LGTM :)

@mholt
Copy link
Member Author

mholt commented Feb 17, 2017

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement MITM detection described by Durumeric, et. al.
4 participants