No end-to-end tests? about sslcontext-kickstart HOT 12 CLOSED

Yes, I have indeed integration test using badssl. But this library is sort of a wrapper containing helper/builder methods to easily contruct a SSLContext and other properties for developers to simplify things. It mostly uses default jdk keymanager, trustmanager and other ssl related objects. The custom trustmanagers and keymanagers in this library is mosly a wrapper to add additional capabilities, such as:

• Reloading ssl - HotSwappableX509ExtendedKeyManager and HotSwappableX509ExtendedTrustManager
• Combining multiple keymanagers and trustmanagers- CompositeX509ExtendedKeyManager and CompositeX509ExtendedTrustManager
• Having partially custom validations - EnhanceableX509ExtendedTrustManager
• Capturing certificates - CertificateCapturingX509ExtendedTrustManager

Or utilities to easily do some action related to those classes:

• CertificateUtils
• KeyStoreUtils
• KeyManagerUtils
• TrustManagerUtils
• PemUtils
• SSLContextUtils
• SSLFactoryUtils
• SSLSessionUtils
• SSLSocketUtils

I didn't spent enough time on the hostname verifier yet and Jesse discovered that the basic one is not secure enough, but that will be fixed in the next release. If there is anything which you guys can discover within this library I would really appreciate your feedback. You guys are probably already quite busy as you guys are mainly the maintainers of the OkHttp libraries. If you find anything which raises your doubts or which you think is wrong or something which can be improved, please share it here. I will appreciate it very much!

from sslcontext-kickstart.

Yeah, I've written a few of these, like https://github.com/cashapp/certifikit/blob/e87baea684ae9a3051efe3d19283fccd9f420304/certifikit-cli/src/jvmMain/kotlin/app/cash/certifikit/cli/okhttp/CapturingTrustManager.kt and Composite TrustManager. So I like the idea. For OkHttp it would be easy for us to fall into a trap of being too helpful, so we don't provide insecure TrustManager for example, for fear it would get used.

But I share @swankjesse concern if I was to use these. I'd like to be able to understand through clear documentation that the library is secure against all these edge cases. If someone is using HostnameVerifierUtils.createBasic() are they aware it is insecure?

from sslcontext-kickstart.

Really cool to see our implementation of the capturing trustmanager is similar and i understand your design decision regarding other parts such as unsafe trustmanager.

I will take your remarks and apply it, thank you again!

from sslcontext-kickstart.

Hi @swankjesse

Yes I have a-lot end-end tests, see here:

• https://github.com/Hakky54/mutual-tls-ssl
• https://github.com/Hakky54/java-tutorials#security-

The first one contains a tutorial of setting up a secured connection with TLS between a server and a client. However the client module contains over 40+ client configurations which will be tested during the integration test for one way tls and two way tls while using this library. You can find the client configuration here:

• https://github.com/Hakky54/mutual-tls-ssl/blob/master/client/src/main/java/nl/altindag/client/ClientConfig.java
• https://github.com/Hakky54/mutual-tls-ssl/tree/master/client/src/main/java/nl/altindag/client/service

Also see here for the full list of client which I have tested the library with: https://github.com/Hakky54/mutual-tls-ssl#tested-clients

On my java-tutorials project I demonstrate the usage of this lib with grpc netty, vertx, spring, jetty

It took a-lot of time to test all of these clients and servers as I wanted to provide examples to the community so they could easily adapt. With this projects I wanted to showcase that it is secure and easily to use and to configure with a client of server. There are still parts which can be improved over time, for example the hostname verifier.

from sslcontext-kickstart.

What what about tests to confirm the client doesn’t connect when the server is untrusted? The insecure implementation of HostnameVerifier is scary both because it’s a vulnerability that impacts every one of its users, and because it shows your testing strategy lacks negative tests.

from sslcontext-kickstart.

You are right on the part of the HostnameVerifier which you also mentioned here: #210 However negative tests are also covered in this projects tests, but mostly focussed on the TrustManager having/not having the trusted certificates of the counter party, but maybe I overlooked something and you discovered it. If that's the case can you please share a list which you think should be also tested?

Coming back to the HostnameVerifier, it looks like the version of OkHttp is advanced and well tested and trusted. The HostnameVerifier has been written by you and @yschimke @Goooler @jemaystermind @JakeWharton Would you and they be ok if I could copy the source code of it into this library? In that way it will provide the same level of security as the one which is available within OkHttp, but the advantage will also come to non OkHttp users, such as Apache, Ktor, Http4k, Fuel, Http4s, ScalaJ, Sttp,
kohttp, HttpUrlConnection, as they will be able to benefit of it via this library. What do you think?

from sslcontext-kickstart.

Without wading in on the other points. It's Apache2 licensed, if you keep the same copyright notice you are welcome to copy it.

One caveat however is that we had a CVE recently, now fixed, that was only avoided in practice because of some sanitisation before it was called. So it hasn't necessarily been designed upfront to be used independently. Though I'm not aware of any similar issues.

from sslcontext-kickstart.

If you use OkHttp's hostname verifier, then I'd make sure you have cleaned the certificate chain before hand. But I'm out of my depth here.

from sslcontext-kickstart.

Without wading in on the other points. It's Apache2 licensed, if you keep the same copyright notice you are welcome to copy it.

Thank you! This library is also Apache2 licensed so it won't be an issue.

If you use OkHttp's hostname verifier, then I'd make sure you have cleaned the certificate chain before hand.

What do you exactly mean by cleaning the certificate chain?

from sslcontext-kickstart.

@Hakky54 Again I'm out of my depth here. But I always get a bit confused with the ordering of Certificate Chain cleaning, Certificate Pinning and Hostname Verification. I can't remember the exact details that forces the host certificate to be first, maybe it has to be, so it's not a problem. but if you are implementing these yourself, you probably need to understand the security implications of all of these.

from sslcontext-kickstart.

I was going to suggest https://badssl.com/ but looks like you already use it somewhat.

c6860d6

from sslcontext-kickstart.

I am closing this issue as the project contains end-to-end test with https://badssl.com as integration test and next to that it is also being tested in the following project: https://github.com/Hakky54/mutual-tls-ssl The library is mostly using default jdk security classes which is already tested by them. Other classes in the library are mostly wrappers with additional capabilities which is already heavily tested. Next to that there is a separate issue report by you on the hostname verifier, so it should be ok to close this issue.

from sslcontext-kickstart.