rustls / rustls-platform-verifier

A certificate verification library for rustls that uses the operating system's verifier
Apache License 2.0
57 stars 18 forks source link

Can I optimize this on Android? #66

Open laiyi55 opened 5 months ago

laiyi55 commented 5 months ago

I found https cost more time on rust than java, then I found function verifyCertificateChain cost most time

CertificateVerifier.kt

            Log.d(TAG, "verifyCertificateChain PKIXBuilderParameters start")
            val parameters = PKIXBuilderParameters(keystore, null)
            Log.d(TAG, "verifyCertificateChain PKIXBuilderParameters end")

it cost 200ms every request

If change it, we can save time

                var trustAnchors = HashSet<TrustAnchor>()
                validChain.forEach {
                    trustAnchors.add(TrustAnchor(it, null));
                }

                var pkixParameters = PKIXParameters(trustAnchors)
                pkixParameters.certPathCheckers = listOf(revocationChecker)
                pkixParameters.isRevocationEnabled = false

                validator.validate(certFactory.generateCertPath(validChain), pkixParameters)

I have already try and it did work, so can we do this?

cpu commented 5 months ago

Hi @laiyi55,

Thanks for the question. That's interesting.

In general I think if you're proposing changes based on performance it would be helpful to know more about how you're profiling this code. Do you have any intuition as to why the builder step is expensive?

  var trustAnchors = HashSet<TrustAnchor>()
                validChain.forEach {
                    trustAnchors.add(TrustAnchor(it, null));
                }

This doesn't look right to me. I don't think we want the leaf and intermediates from the Android platform verifier's built chain (validChain) to be the set of trust anchors used to validate revocation information.

If the profiling justifies it perhaps there's a way to cache PKIXParameters built with the keystore between verifications instead?

laiyi55 commented 5 months ago

Thanks for your answer. So we can't use trust anchors to validate revocation information Sorry,I just find note : 1 Android does not provide any way only to attempt to validate revocation from cached data like the other platforms do so I guess https cost more time on rust than java is correct, and we can't do anything to optimize this

My test: Rust(hyper_rustls): 1.3s(first) 1s(average) Java(native api, HttpsURLConnection):800ms(first) 600ms(average)

complexspaces commented 5 months ago

Yes, in general the revocation cost is going to be higher on Android because of the lack of caching and multiple layers of JVM overhead compared to a platform like macOS (for example).

However, it seems like there is room for optimization.

Do you have any intuition as to why the builder step is expensive?

IIUC correctly, one of the biggest reasons its so expensive right now is that we are collecting all trust roots out of the system store, which involves reading from disk, parsing tons of X509, etc. Any networking that Android is doing shouldn't be contributing to the slowdown noticeably. We won't need 99% of those roots.

I would need to do more research on this since I am not that familiar with how OCSP's signing/verification works. We definitely don't want to add the whole chain in there though. Instead, just passing in the intermediates and roots would provide a public key the client can use the verify the OCSP signature. Again, this is just a hypothesis and I would want to perform more research on what HttpsURLConnection is doing, how/when Conscrypt does OCSP checks, etc.