datatheorem / TrustKit-Android

Easy SSL pinning validation and reporting for Android.
MIT License
588 stars 87 forks source link

Question about using this for all api levels #28

Closed calvarez-ov closed 6 years ago

calvarez-ov commented 6 years ago

Should TrustKit be used only if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N)?

It's unclear to me what will happen if we do this on N+ devices: connection.setSSLSocketFactory(TrustKit.getInstance().getSSLSocketFactory(serverHostname));

This example is for HttpsURLConnection, but the question applies to OkHttp too. Will there be a double pinning check in this case? One by core Android and one by TrustKit?

I did see this in the readme:

On Android N devices, TrustKit uses the OS's implementation of pinning

But it's not clear what happens if our own code is setting SSLSocketFactorys on connections in N. Maybe an explicit statement in the readme could remove doubt that this can indeed be done on N, and that we don't need to include any api level checks.

Thanks!

jobot0 commented 6 years ago

Hi @calvarez-ov, TrustKit can be used for every version of Android, at least for now. I'm not sure to understand what is the issue ? TrustKit is doing the SSL pinning part of every connections where you're using it before Android N. Still you can use the reporting part even with N or more recent version of Android. Thanks for the feedback and does that answer to your question ?

calvarez-ov commented 6 years ago

Thanks. I was wondering what's the difference between using TrustKit vs not using TrustKit on Nougat. Indeed, I see that the reporting is one of the things that TrustKit adds.

I took a look at PinningTrustManager.java for more insight. I see that the manual pinning is only done before N, and for N+ it relies on the baseline (system) trust manager to check the pinning. So I guess the actual validation of pins shouldn't be different whether TrustKit is used or not.

On N+, looks like there are a couple of other things that happen in TrustKit in addition to reporting: the hostname verification is always done first, and errors can be ignored if enforcePinning is set to false.

This is enough for me to understand what TrustKit adds on N+. Thanks.