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

Added hooks to com.android.org.conscrypt.TrustManagerImpl #101

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jamoski3112
Copy link

Add a new hook to the native conscrypt TrustManagerImpl class

@CLAassistant
Copy link

CLAassistant commented Aug 1, 2024

CLA assistant check
All committers have signed the CLA.

@pimterry
Copy link
Member

pimterry commented Aug 2, 2024

Hi @jamoski3112! Do you have a specific example of when this is required?

In general this should already be handled by the android/android-system-certificate-injection.js script, which prepopulates the TrustedCertificateIndex with your configured CA certificate. That index is used within TrustManagerImpl (and various other places) and as far as I'm aware this means that all those implementations should already happily trust your certificate.

The key difference is that that existing index hook just makes the application additionally trust the specific CA you add to config.js, whereas the changes in this PR disable TLS validation completely (which in general this repo tries to avoid, since it allows anybody to intercept your traffic, not just you personally).

All of that said, it's quite possible there are other more specific cases that aren't handled for some reason, and I'd love to hear more about that if so.

@jamoski3112
Copy link
Author

I made this change based on an app that I was testing while trying to intercpet the traffic the TrustManagerImpl was used and it threw an error by making this chage I was able to intercept the traffic

@pimterry
Copy link
Member

pimterry commented Aug 3, 2024

Can you share the details of the specific app and the full Frida command that you ran, so that this can be reproduced? In most cases, this shouldn't happen as long as you have the correct CA certificate in config.js and you're using the android/android-system-certificate-injection.js, but if there's a case where that's not true I definitely want to take a look and debug that further.

@jamoski3112
Copy link
Author

jamoski3112 commented Aug 5, 2024

Hi @pimterry ,

Sorry but I cant share the details of the app since it is an internal build but i can give you the command and the output before i made the change.
image.png

The app that i am using it built on flutter which has a web view component which loads an external site when accessing this functionality the frida script throws this error.

@pimterry
Copy link
Member

pimterry commented Aug 5, 2024

The app that i am using it built on flutter which has a web view component which loads an external site when accessing this functionality the frida script throws this error.

That sounds like a likely cause, that makes sense. I can imagine that could cause this. In general Flutter apps aren't very well supported by these scripts anyway, but I'm certainly open to improvements there.

Sorry but I cant share the details of the app since it is an internal build

Fair enough. Is it possible to build & share a minimal demo Flutter app that does the same thing? If you can reproduce it in a tiny app that just opens a web view and fails in the same way, that would be enough to investigate further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants