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

Self signed certificates are not pinned, opening the door to MITM #40

Open
SafwatHalaby opened this issue Apr 10, 2015 · 7 comments
Open

Comments

@SafwatHalaby
Copy link

See https://github.com/siacs/Conversations/issues/1084 .

@SafwatHalaby
Copy link
Author

What this means is that if you click "Accept" on a self signed certificate, and some evil CA or three-letter-agency decides to MITM you, they can completely override your self signed cert by offering your app a properly signed certificate.

@SafwatHalaby
Copy link
Author

Also: yaxim-org/yaxim#187

@SafwatHalaby SafwatHalaby changed the title Self signed certificates are not pinned Self signed certificates are not pinned, opening the door to MITM Apr 10, 2015
@ge0rg
Copy link
Owner

ge0rg commented Apr 10, 2015

This is a hard problem. There are several sides of it to consider:

  1. technical: the Java API for TrustManager does not allow to see the server name/address/service name of the service we are connecting to. Therefore, pinning a cert to this server is impossible with the default API, I can only add all "always-accept" certs into a trust store that affects all MTM-checked connections. I was hit hard by this problem when adding hostname verification, and my fix (wrapping a hostnameverifier) turned out as a bad design. In a future API, I need to require the server name as a constructor parameter to MTM, and then I can use that server name as a key for cert pinning as well as perform hostname validation in one step. I'd love to hear explicit app developer feedback on this one.
  2. historical: when I started MTM, Root CAs were considered reasonably-behaving enough to not have to work around them. The last API change introduced a mode where you can define your own trust root instead of Android's, which Conversations has used for the paranoid mode. This still doesn't fix the "real pinning" vs. "adding to a cert pool" issue from above, so if you use MTM for two different servers, they can MiTM each other until point 1 above is fixed.
  3. user experience: popping up security dialogs is really bad UX. The more often I do so, the more people become ignorant of it and tap "always" without reading. As I can not force the server admins to never change their keys/certs, and there is no mechanism for the server admin to provide that info via XMPP (let's consider TACK dead, and HPKP clearly inapplicable), there is no way for MTM do differentiate between an attack and a server admin following best practices (replace keys/certs in periodic intervals). I can only do it wrong here, whatever I do ;-)
  4. future-oriented: we need a secure out-of-band mechanism for the server admin to tell us which certificate the server has. Fortunately, DANE is providing exactly that, and my stated goal is to write/find a DANETrustManager that will either become part of MTM or be chained into it. Rumor has it that some DNSSEC work for Java will happen this summer, stay tuned!

@SafwatHalaby
Copy link
Author

Regarding 3, I think it's best to be verbose.

  • Changing a self signed cert on regular intervals will generate warnings regardless of what you do.
  • It's safe to assume that the people who are using servers with self-signed certs are almost always tech-savvy users (People running their own servers and such), those people will not be annoyed by popups and in fact would be thankful for verbose cert warnings, because not being verbose is a door to MITM.
  • It is better to annoy the non tech-savvy minority with popups and scare them off to a better server, they shouldn't be on a server with self a signed cert anyways, and they are most likely not secure anyways, since they most likely did not check the cert hashes. Thanks to point 1, that minority will be annoyed anyways.

@iNPUTmice
Copy link

I actually would not call this a bug.
MTM allows you to trust a self signed certificate. It never promised to pin your certificates.
If you want to pin your certificates there are other solutions out there (AndroidPinning for example)
If you don't trust the CAs provided by your system you can disable all or certain CAs directly in your Android Operating System (Settings / Security / Trusted credentials)

@ge0rg I'm not sure about what kind of input you need here exactly. But I'd be ok with creating a new MTM instance for every connection using the host name in the constructor for example.

But I'd prefer a 'proper' DANE solution as well.

@SafwatHalaby
Copy link
Author

MTM allows you to trust a self signed certificate. It never promised to pin your certificates.

Ignoring the fact that different servers with self-signed certificates can mitm each other, you're right.

@Flowdalic
Copy link
Collaborator

...the fact that different servers with self-signed certificates can mitm each other...

Not if Hostname verification is done correctly.

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

No branches or pull requests

4 participants