Skip to content
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

Can I optimize this on Android? #66

Open
laiyi55 opened this issue Feb 2, 2024 · 3 comments
Open

Can I optimize this on Android? #66

laiyi55 opened this issue Feb 2, 2024 · 3 comments
Labels
enhancement New feature or request O-Android Work related to the Android verifier implementation

Comments

@laiyi55
Copy link

laiyi55 commented Feb 2, 2024

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
Copy link
Member

cpu commented Feb 2, 2024

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
Copy link
Author

laiyi55 commented Feb 4, 2024

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
Copy link
Collaborator

complexspaces commented Feb 5, 2024

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.

@complexspaces complexspaces added enhancement New feature or request O-Android Work related to the Android verifier implementation labels Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request O-Android Work related to the Android verifier implementation
Projects
None yet
Development

No branches or pull requests

3 participants