Allows users to disable hostname verification (SSL)
-
Users of DavDroid may use their own servers, possibly with several virtual HTTPS hostnames.
To allow them to use DavDroid, this commit adds an option to disable hostname verification
for SSL handshake as there is no support for SNI in Apache DefaultHttpClient.
-
Thanks for the commit. However, how do you argue against the opinion that SSL without hostname verification is useless und gives a false sense of security because everyone can (often easily, think of WiFi networks, there are ready-to-use tools which require 2 buttons to click) do MITM attacks?
-
@rfc2822 I think there are two good answers.
First, SSL prevents passive traffic sniffing. The easiest and most common shady activity is done passively, not actively. So rather than say useless, I'd say incredibly useful.
The second is that right now, some of us simply cannot use DAVDroid. Right now, DAVDroid is totally useless to me. I'd rather have an insecure but functional DAVDroid than nothing. A somewhat insecure fix is a step in the right direction.
... which is not to say I think you should or shouldn't merge this. But there are reasons you might want to do so, and they are worth considering.
-
Regarding traffic sniffing: Nevertheless you think your data is transferred securely and encrypted, but as soon as you log in to a malicious WiFi or as soon as someone (government, for instance) intercepts your Internet connection, your data is sniffed and processed while you don't even think about this. I consider this as worse than knowingly transferring data without encryption (because then you know that your data can be read).
However, I understand that there's a practical need to use encryption for DAVdroid (instead of using HTTP and knowing that it's not secure). SNI would be nice, but it's not possible at the moment (see FAQ for the reason). So: Why don't you use port-based SSL virtual hosts or mount DAV into your main SSL host? Just wondering.
-
my two cents: this may be put in some "expert settings" with a big, fat, bold warning pointing out the consideration about how insecure this might be…
If users want to be (somewhat) less secured, that's their problem, I think ;). A multi-domain certificate isn't expensive, by the way (startssl for example) ;).Cheers,
C.
-
I see your point. I was going to use Http(s)URLConnection as it has support for SNI, but this implies a major rewriting of davdroid, which I could not do on my own, and without talking to you first
I then opted for an option, as I host several websistes (multi domains BTW, so having a wildcard cert will not help here) on my server.
By reading your comments, I agree that it would give a false sense of security. So I have an idea. Instead of allowing all hostnames, the user could allow one particular hostname to be considered ok. In that case, the first one that the server will propose.
In my case, I have my calDAV/cardDAV on baikal.bluepyth.fr, but the hostname that davdroid got is analytics.bluepyth.fr. So I could tell DavDroid that if it finds analytics.bluepyth.fr it is OK, and it can go on.
Wouldn't this keep the security at its maximum without losing the flexibility for DavDroid users?
Thanks!
EDIT: I think that it should be clients that should try to support SNI, instead of having every server configured for clients that do not support SNI, I think it makes more sense, but here is just an opinion wthout much argumentation
-
wouldn't the proposed patch provide the same functionality as 99% of all apps do? Most apps handle this problem by displaying a warning about the mismatch while providing a way to "continue anyway"? I'd appreciate this feature.
-
@BluePyth I tried to write DAVdroid with HttpURLConnection first, but this crappy API doesn't support HTTP verbs like PROPFIND and REPORT (the API structure would even be compatible, but such requests just won't be sent) and so you can't use it for DAV. See https://developer.android.com/reference/java/net/HttpURLConnection.html "HTTP Methods": »Other HTTP methods (OPTIONS, HEAD, PUT, DELETE and TRACE) can be used with setRequestMethod(String).« I tried it out, PROPFIND and REPORT don't work although they're just another strings. So, I was forced to use the HttpClient lib.
Concerning your work around: couldn't you just make baikal.bluepyth.fr the primary SSL host (by moving it to top of your configuration, for most servers)?
And I full agree with you that clients should support SNI (by using a HTTPS library that supports SNI). So, everyone complaining is invited to submit a PR
@rigid No, it would not. It would make HTTPS useless because without host verification, SSL provides no security. To ignore the host name would be a severe security issue and would cause users to believe their data is transferred securely while every script kiddie (or NSA) could read the data.
Most apps have certificate pinning with a GUI which is another problem: In the first "add account" activity DAVdroid could show such an "Accept certificate?" window and use certificate pinning, OK. But what when the certificate expires? The sync adapter has no possibility to directly pop-up a window when syncing (as far as I know, correct me if I'm wrong). So you would have to add your account again as soon as you set a new certificate... I don't know that sounds bad too. I think I'll wait for better solutions.
-
By the way, I'm really happy about the PR and that you participate in developing and about discussing with you. But I will wait some time until a good solution is found for a) the SNI problem (I think the solution will be to use a recent HttpClient library which means some work because Android doesn't allow to just import a recent version) and b) the self-signed certificates: here I don't know what would be appropriate. Either certificate pinning in a way that handles certificate changes or otherwise, I'd stay at the "import CA" possibility.
-
@rfc2822 I've made some research about SNI support in java, and it seems that it is only supported in Java 7. Therefore, it should not work in Android.
Although, if Google has added support for SNI in HttpUrlConnection, it means that they have included it in Android, and we should be able to find a way to use it with Apache HTTP client.
I'm willing to find a good solution for this problem, because I plan to have other WebDAV services on my server, and I cannot change the order to make one work, because another would not then
What I have in mind, for now, is to improve my PR with an option that would allow one to set the expected hostname as I explained earlier. I understand that you don't want to merge it, but hopefully, this patch will help users that need a workaround while we try to add real support for it. (it's just importing the project into eclipse and launching it on one's smartphone).
After that, I can try to find a good solution with Apache HTTP client if you are okay with it. I've already seen that someone created a lib that repackages Apache HTTP client so that we can use the latest version, on Android.
This could be a start, but the committers responsible for the SNi support issue are not willing to add support for SNI in Apache (in their point of view, which I share, it should be the responsibility of the TLS layer, thus Java).
So I think about digging in HttpUrlConnection to see how Google added support for SNI.
What do you thing about it?
-
@BluePyth SNI will be available soon, see https://github.com/rfc2822/davdroid/issues/9#issuecomment-28437577 (some technical details there, you might be interested)
Because I'm not going to disable hostname verification, I will close this issue now.
For discussions about certificate pinning and better support for self-signed certificates, please see issue #3.
-
@rfc2822 Perfect! I should have read the issues before
BTW thanks for DavDroid, was looking for one good sync adapter for webdav, even thought of building one, but you did a really good job here
-
@rfc2822 Thanks for the SNI support in 0.4 !
-
Hi,
I have the same problem, self signed wildcard certificate, DAVCARD complain about "cannot verify hostname". I imported the .crt into Android (CM 11) but still same error
I'm stuck here,
Thanks.
-
@sulliwane this is not exactly the same problem. You should refer to issue #3 as @rfc2822 said in his last comment.