-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Instead of KeyStore pass a KeyStoreFactory around. #388
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
Conversation
Provide several basic implementations of KeyStoreFactory.
Instead of KeyStore pass a KeyStoreFactory around.
Hi. Maybe it's a stupid question, but is custom KeyStore supported? I'm looking at the code and it looks weird - ex. no references to *KeyStoreFactory at all. I try to connect to a server with a self-signed certificate and after hours I cannot get it working. Still getting "java.security.cert.CertPathValidatorException: Trust anchor for certification path not found." I've also posted this problem on SO: http://stackoverflow.com/questions/36016430/unable-to-make-acra-accept-a-self-signed-certificate Do you have any reference implementation? |
Try to use the current master in combination with AssetKeyStoreFactory. |
OK, thanks. It seems to work, but now gives another error: |
Yes, this looks like a server problem. |
Indeed. One of the hints is falling down to SSL3. Would demand looking into library very deep, grep doesn't report any places like this. I'll check my server once again, but could you take a look and judge, if it can be an issue on the client side: http://stackoverflow.com/questions/29916962/javax-net-ssl-sslhandshakeexception-javax-net-ssl-sslprotocolexception-ssl-han ? EDIT: yeah, it seems Android < 5.0 uses SSLv3 by default. |
Hmm interesting. Essentially it means that the client and server weren't able to agree on a protocol to use. That solution just means that the client won't be able to use SSLv3 as one of its protocols, so it is further removing choice. What version of Android were you testing on? |
Exactly! So, 2 devices: one Lenovo TAB2 A7-30 running 4.4.2 and Moto E running 5.0.2. Add Apache with: |
So, just to clarify. |
Partially. I'm just buying a standard SSL certificate to get rid of the "Hostname was not verified" error, but I thing the problem with Android 4.4.2 and its default SSLv3 will persist. I'll think of some solutions for that. |
The SSL issue will only persist as long as you configure your server to not accept SSLv3. Essentially, SSL comms works by the client and server comparing available protocol lists and choosing the strongest method available. If the client can do no better than SSLv3 and your server won't talk down to SSLv3 then you are at an impasse. |
Yes, William, thanks for clarification, I know it. By far it appears that older Androids force communication on level known as insecure nowadays. :) But it's of course some side problem, not connected with the lib. Nota bene great work, keep going! :) |
@maciej-miszczak You should be able to work around this by changing the priority of cipher suites with |
Another idea is to use the provider from Google Play Services, it should let you use a current SSL implementation on older devices: https://developers.google.com/android/reference/com/google/android/gms/security/ProviderInstaller |
Nikolay, thanks for the hints. 1. SSLSocket.setEnabledCipherSuites() - as I wrote, it would (probably, correct me if I'm wrong) demand digging in the lib, what is OK, especially when I have it's master in source. Maybe it's worth to include this tune in some of next releases. 2. Yeah, I partly know the idea of Android, but I really don't want to start using another Google's user-data-harvester just to be able to connect securely. :) |
I just replaces the OpenSSL libs, so I don't think there's any harvesting involved. But you should test it to be sure :) |
Guys, one comment on the latest release. Maybe I did something wrong, but after cleaning all of the mess and leaving only the basic configuration (3 lines after @ReportsCrashes) and just ACRA.init(this) in onCreate() of my App, it seems you don't handle a case when no KeyStoreFactory is given by a lib user. It results with java.lang.NullPointerException at org.acra.util.HttpRequest.send(HttpRequest.java:97) with config.keyStoreFactory() = null. Or maybe I just should always provide a KeyStoreFactory from now? |
Doh! |
One comment: I'm using source, didn't check if the problem occurs if using Gradle. |
The problem is there. |
Happy to help. :) |
Not so fast, Will! :D Heh, another bad news. :> java.lang.IllegalStateException: TrustManagerFactory is not initialized. Sorry. :) Unfortunately I'm new to Java and Android, but it seems there's some default TrustManagerFactory needed and it was something I tried to do, using ConfigurationBuilder before calling ACRA.init(). CORRECTION: maybe not default, just null, due to the docs. Cheers! |
I don't get any fault when not specifying a KeyStoreFactory. What's your config that is showing that fault? |
OK, so now I run on a Lollipop device, using source from the latest release. I get an error "java.lang.IllegalStateException: TrustManagerFactory is not initialized" at org.acra.util.HttpRequest.send(HttpRequest.java:100). It seems to me that if I don't provide a custom KeyStoreFactory, tmf.init() won't be called. By saying "docs" I meant TrustedManagerFactory Javadoc, which says that keystore parameter can be null, if you have nothing interesting to give. :) BUT if I tried the code above (using null), I got an error of ambiguous call. Forgive me, if I'm doing something wrong, just starting with this stuff. :) |
Hmm, runs fine on a Marshmallow device. |
The files I added in this PR do not include copyright notices. Of course they should also be licensed under the Apache license. I'm not quite familiar with the topic. Should they include my name? |
Good pickup. Yes they should include the Apache licence header. |
how to use the crt file? |
@erlangp just reference it in resCertificate or certificatePath |
owh.. 👍 thanks sir @F43nd1r |
Fix #387 by passing a factory instead of the keystore itself.