Feature #2669
implement pin/tofu/ca/prompt for Fdroid client
Status: | Closed | Start date: | 11/27/2013 | |
---|---|---|---|---|
Priority: | Normal | Due date: | ||
Assignee: | pd0x | % Done: | 0% | |
Category: | - | |||
Target version: | 0.2 - ChatSecure/Bluetooth | |||
Component: |
Description
Related issues
Associated revisions
Implement a custom KeyManager to force an SSLContext to always use the HTTPS key alias.
Kerplapp has one keypair and two certificates in its keystore. The first
certificate is generated on first run and is used for signing the index.jar. The
second certificate is regenerated as required and contains the current IP used
for the HTTPS server. This certificate must be regenerated whenever the IP
changes. Using the default KeyManager is unreliable: it sometimes uses the
index.jar certificate and not the https certificate. In order to force the use
of the HTTPS certificate we provide a wrapped KeyManager that forces the right
alias to be used. Convoluted, but the only apparent solution outside of
managing two entirely separate keystores with two copies of the private key
and only one certificate per keystore.
This should be the last requirement for issue #2669
History
#1 Updated by pd0x about 4 years ago
- Status changed from New to In Progress
Switching status to in progress.
#2 Updated by hans about 4 years ago
status update from IRC:
pd0x: nothing I can't summarize here quickly :-) pd0x: I have pinning & tofu added to my fdroid client fork. Pushed the commits to master today pd0x: It's working in the sense that it prompts for the Kerplapp self-signed cert instead of hard failing pd0x: but it has another issue with the URLConnection throwing an error about the hostname (using the IP as the CN) pd0x: Looking around online it seems like there is an extension you can add to the cert that specifies the hostname as an ip instead of via DNS. I have to explore whether that will fix this or if there will need to be something messier done (overloading hostname validation?) pd0x: Does anyone know best practice for self-signed certs w/o a hostname? pd0x: It seems like putting a CN attribute with the IP address is both a pain & doesn't validate _hc: pd0x: that's exciting! _hc: pd0x: have you ever seen a cert with an IP address in it? _hc: I don't remember ever seeing one that way pd0x: not recently, but I have vague memories of embedded devices doing something like this _hc: hmm, yeah pd0x: I might try removing the CN field entirely and seeing how the android ssl stack handles it devrandom: pd0x: IP doesn't validate? pd0x: devrandom: one sec, let me get logcat on this machine and I can paste the exact error devrandom: it seems it should devrandom: (except for the part that it's self-signed) pd0x: Right, so I get the MemorizingTrustManager prompt for the cert (since it's self signed) and I choose "Always" to accept it devrandom: OK, so that should work from there, since MTM only gets invoked after a connection was made pd0x: http://pastebin.com/WPK3AstT pd0x: agreed, it fails afterwars in libcore.net.http.HttpConnection.verifySecureSocketHostname pd0x: afterwards* pd0x: "Hostname '192.168.11.51' was not verified" devrandom: oh, that's fdroid making the connection pd0x: where the cert has CN=192.168.11.51 and the repo is being hosted at https://192.168.11.51:8888/repo devrandom: not using MTM pd0x: Yeah, it happens after MTM adds it to the store devrandom: but how did MTM get invoked earlier? do you first make your own connection with an SSLContext that refers to MTM? pd0x: I have it right now so that the pinning & MTM wrap the system default and are set up app-wide devrandom: oh, I didn't know you could do that devrandom: I would guess that it's not triggering for the way frdoid connects pd0x: https://gitorious.org/f-droid/kerplapp-fdroidclient/source/ed6f8f3a1d2d1e13f6144205c38f6dc5fbedd3b4:src/org/fdroid/fdroid/FDroidApp.java#L138 pd0x: I modelled it after ChatSecure pd0x: Maybe that isn't as "global" as I thought? devrandom: looking n8fr8: pd0x: ChatSecure doesn't use URLConnection though devrandom: ChatSecure does its own connection management and explicitly sets SSLContext, etc. n8fr8: yes pd0x: hmmmmmmmmmmmmmm devrandom: so what you did is plausible devrandom: I didn't know that HttpsURLConnection had that pd0x: That's how Ge0rG's integration wiki page for Memorizing Trust Manager recommends integrating w/ your app devrandom: you could try to step through the system libs and see why it's not triggering MTM devrandom: or if MTM is returning incorrect results pd0x: Yeah, I'll try to pursue that line of reasoning ***devrandom is looking at android sources pd0x: Maybe the issue isn't the CN=ip in the cert but that one (or more) of the connections isn't using MTM after the cert gets whitelisted devrandom: huh, I don't see libcore package in sources devrandom: right devrandom: pd0x: https://android.googlesource.com/platform/libcore/+/android-4.3.1_r1/luni/src/main/java/libcore/net/http/ pd0x: devrandom: thanks! koenmetsu1 [~Koen@ip-83-101-80-112.customer.schedom-europe.net] entered the room. koenmetsu left the room (quit: Ping timeout: 250 seconds). pd0x: hmm, it's not clear from the src code what HostnameVerifier will be used at runtime devrandom: pd0x: is HostnameVerifier separate from the trust manager? pd0x: devrandom: It seems to be devrandom: the docs say " devrandom: The interface to be used to provide hostname verification functionality. pd0x: or at least there is a separate devrandom: This is an extended verification option that implementers can provide. It is to be used during a handshake if the URL's hostname does not match the peer's identification hostname. " pd0x: HttpsURLConnection.setDefaultHostnameVerifier() pd0x: Where peer's identification hostname comes from the cert? I assume? devrandom: The interface to be used to provide hostname verification functionality. devrandom: oops devrandom: https://developer.android.com/reference/org/apache/http/conn/ssl/StrictHostnameVerifier.html Ge0rG: in java, hostname verification is a major f*ckup. pd0x: ugh. That's not pleasant news :-( Ge0rG: it is separate from the cert verifier and has no direct access to the cert pd0x: Ok, so MemorizingTrustManager doesn't have anything to do with the HostnameVerifier? Ge0rG: so you need quirks like to enforce the handshake, if not already happened, in the verifier Ge0rG: pd0x: exactly. I tried to integrate it and failed miserably devrandom: if MTM will already checks enough, should we just set an AllowAllHostnameVerifier? Ge0rG: devrandom: no pd0x: devrandom: that seems really really scary devrandom: MTM doesn't check the actual hostname? Ge0rG: devrandom: AllowAllHostnameVerifier will lead to no questions if you MITM guardianproject.org with a valid cert for evilhax0r.net Ge0rG: devrandom: MTM does not affect hostname verification in any way. devrandom: got it devrandom: so the default hostname verifier doesn't like IP addresses? Ge0rG: probably so Ge0rG: I wanted to add hostname verification into MTM once, feel free to do it... if you dare. Ge0rG: I would gladly accept patches pd0x: I'm wondering how safely I could wrap the default hostname verifier just enough to handle the IP case Ge0rG: patches to allow something like "host xyz claims to be asdf. accept once / always / reject" devrandom: in the case of an IP address, hostname and CN should be String.equals devrandom: so you could wrap and return true if wrapped verifier succeeds, or String.equals ***devrandom is looking at source for apache httpclient devrandom: (which is where StrictHostnameVerifier comes from) pd0x: I would have thought StrictHostnameVerifier is doing something fairly similar pd0x: Let me try to find that src too Ge0rG: do you have a valid cert for an ip address? pd0x: I -think- so pd0x: one sec and I'll put an eg cert somewhere if you want Ge0rG: nah, I've gotta go again. Just wondering because certs for IP addresses are a bad practice pd0x: Agreed, this is for a service started on a user's device pd0x: self-signed certificate with the IP as the CN seems the only thing to do there to quickly startup an HTTPS server on device devrandom: pd0x: looking at the source, I don't see why it would fail on IP addresses devrandom: there is even specific handling for normalizing IPv6 pd0x: hmmm Ge0rG: what happens if the ip changes? pd0x: Ge0rG: certificate is regenerated pd0x: Ge0rG: associated pubkey is the same across cert regens so SPKI pins won't break. Ge0rG: hmm.. interesting approach. I wonder if using some other element than the IP would be more useful. repository name or somesuch... and match it against the CN pd0x: Open to better suggestions :-/ this is what I arrived at from a set of limited choices pd0x: possibly. It'd be less of a pain than regenerating every IP change, that's for sure. pd0x: Firefox/Chrome complain when the CN isn't the hostname (which is the IP), but it will already complain about the fact that it's a self signed cert pd0x: so I'm not really buying myself anything by using the IP for that case. Just one less warning on the exception adding screen Ge0rG: yes. and for an app you could anchor your hostname verifier to some other identity element of that device 17:25 pd0x: Yeah, app is the primary use-case. Browsers are 2ndary nice-to-have for the bootstrap page Ge0rG: maybe you could obtain the hostname and rely on m-dns Ge0rG: or a local dns server adding the device to the resolver, like dnsmasq pd0x: m-dns is on the roadmap. Personally I need to read up, I'm not at all familiar pd0x: I think _hc and n8fr8 have some experience Ge0rG: basically, in many LANs if you obtain an IP from DHCP and send your hostname, your IP will be linked to hostname.$localdomain Ge0rG: so you might get through with a cert for hostname Ge0rG: but I'm merely speculating here _hc: android does not support mDNS itself, so I think it would be tricky to use it here to provide hostnames pd0x: I'm going to try dropping the CN field entirely and see what the implications of that are. _hc: pd0x devrandom, Ge0rG I like the String.equals() wrapper idea where if there is a test for IP == hostname == CN, otherwise call the normal hostname verifier. Seems not too risky since its a simple implementation
#3 Updated by pd0x about 4 years ago
I tried a few work-arounds in my quest to solve this in the safest way. First I tried removing the CN field all together, this works fine in the browsers I tested with on my OSX machine (Chrome, Firefox, Safari). There was an additional "warning" under the gory details of the self-signed cert warning, but because it's self-signed anyway that doesn't really matter. Unfortunately, it didn't fix the problem client side as the StrictHostnameVerifier that Android uses by default still barfed at hostname verification (use of MemorizingTrustManager et. al. withstanding).
Rather than trying to implement my own HostnameVerifier wrapper that allowed IP based hostname verification I went another route. I added a Subject Alternative Name (https://en.wikipedia.org/wiki/Subject_Alternative_Name) to the certificate of type "IP" with the local IP address of the repo. This is enough to convince the stock StrictHostnameVerifier that the certificate can be trusted for the hostname that the client connects to. Yay!
Using the patched F-droid client available here: https://gitorious.org/f-droid/kerplapp-fdroidclient and the latest Kerplapp master from https://github.com/binaryparadox/kerplapp allows full end-to-end HTTPS between a F-droid client and a Kerplapp repo. Upon first connection to the Kerplapp repo the F-droid client prompts the user to memorize the certificate once or always (using the Memorizing Trust Manager) and encrypted communication happens uninhibited from there.
One issue remains before this ticket can be closed: There is currently one public/private keypair used by the Kerplapp app to generate: 1) a self signed certificate used for signing the index.jar 2) self-signed https certificates for every IP address the repo is hosted on. We can't use 1 for https because it doesn't have the correct CN and IP subject alt name (because we didn't know the IP when we generated it). For some reason there is bug right now where the HTTPS NanoHTTPD instance sometimes uses cert 1 for https and sometimes the correct cert 2. This still needs addressing before HTTPS support is reliable. Whenever NanoHTTPD is using cert 1 it will not function with the F-droid clients.
#4 Updated by pd0x about 4 years ago
I've fix the improper certificate use by patching NanoHTTPD to add another method that accepts an array of KeyManagers instead of a KeyManagerFactory. The code that was there just asks the KeyManagerFactory for an array of KeyManagers anyway.
This allowed me to modify the KerplappKeyStore to provide a wrapped KeyManager implementation that used the system KeyManager's default behavior except for when it asked what the keystore alias should be for the HTTPS server, where the Kerplapp implementation is hardcoded to return the HTTPS certificate alias.
Should be good to go!
#5 Updated by hans about 4 years ago
- Target version changed from 0.1 - "Kerplapp" to 0.2 - ChatSecure/Bluetooth
#6 Updated by pd0x about 4 years ago
- Status changed from In Progress to Resolved
Pull req incorporating TOFU & Pinning submitted to upstream project's development branch:
https://gitorious.org/f-droid/fdroidclient/merge_requests/56
#7 Updated by pd0x almost 4 years ago
- Status changed from Resolved to Closed
Feature merged into FDroid client.