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

TrustAllX509TrustManager - lint error #1306

Closed
daniel-tailored opened this issue Dec 19, 2022 · 10 comments
Closed

TrustAllX509TrustManager - lint error #1306

daniel-tailored opened this issue Dec 19, 2022 · 10 comments

Comments

@daniel-tailored
Copy link

Hi, I write concerning below lint errors we got for our android project.

Ofc adding to baseline would resolve the issue but that's not really the best approach I guess (implementation could change in the future etc.). Any suggestions or awareness for this already?

Thx in advance.

<issue
    id="TrustAllX509TrustManager"
    message="`checkClientTrusted` is empty, which could cause insecure network traffic due to trusting arbitrary TLS/SSL certificates presented by peers">
    <location
        file="org/bouncycastle/jsse/BCX509ExtendedTrustManager.class"/>
</issue>

<issue
    id="TrustAllX509TrustManager"
    message="`checkClientTrusted` is empty, which could cause insecure network traffic due to trusting arbitrary TLS/SSL certificates presented by peers">
    <location
        file="org/bouncycastle/jsse/BCX509ExtendedTrustManager.class"/>
</issue>

<issue
    id="TrustAllX509TrustManager"
    message="`checkServerTrusted` is empty, which could cause insecure network traffic due to trusting arbitrary TLS/SSL certificates presented by peers">
    <location
        file="org/bouncycastle/jsse/BCX509ExtendedTrustManager.class"/>
</issue>

<issue
    id="TrustAllX509TrustManager"
    message="`checkServerTrusted` is empty, which could cause insecure network traffic due to trusting arbitrary TLS/SSL certificates presented by peers">
    <location
        file="org/bouncycastle/jsse/BCX509ExtendedTrustManager.class"/>
</issue>

<issue
    id="TrustAllX509TrustManager"
    message="`checkClientTrusted` is empty, which could cause insecure network traffic due to trusting arbitrary TLS/SSL certificates presented by peers">
    <location
        file="org/bouncycastle/est/jcajce/JcaJceUtils$1.class"/>
</issue>

<issue
    id="TrustAllX509TrustManager"
    message="`checkServerTrusted` is empty, which could cause insecure network traffic due to trusting arbitrary TLS/SSL certificates presented by peers">
    <location
        file="org/bouncycastle/est/jcajce/JcaJceUtils$1.class"/>
</issue>

<issue
    id="TrustAllX509TrustManager"
    message="`checkClientTrusted` is empty, which could cause insecure network traffic due to trusting arbitrary TLS/SSL certificates presented by peers">
    <location
        file="org/bouncycastle/est/jcajce/JcaJceUtils$2.class"/>
</issue>
@francescocervone
Copy link

This lint issue is pretty serious. Any Android app containing unsafe implementations of X509TrustManager may be removed from the Play Store.

https://support.google.com/faqs/answer/6346016?hl=en

@dghgit
Copy link
Contributor

dghgit commented Jan 17, 2023

Yes, I'd say it's serious, BCX509ExtendedTrustManager is an abstract class, there's obviously a bug in the lint checker. Is there a directive that turns it off for a specific method?

So with JcaJceUtils - section 4.1.1 of RFC 7030 actually requires support for connecting to an unknown server as part of it's boot strapping process. These things shouldn't ever have to evaluate client auth though - we've added illegal state exceptions for the client auth side.

@daniel-tailored
Copy link
Author

@dghgit

Yes, I'd say it's serious, BCX509ExtendedTrustManager is an abstract class, there's obviously a bug in the lint checker. Is there a directive that turns it off for a specific method?

I'm not aware of any, no.

So with JcaJceUtils - section 4.1.1 of RFC 7030 actually requires support for connecting to an unknown server as part of it's boot strapping process. These things shouldn't ever have to evaluate client auth though - we've added illegal state exceptions for the client auth side.

So with that added do you not get the lint error any more? Can you reproduce it when running lint?
If so, fixed, it would then probably be in the next release?

Thx for the reply and looking into it!

@daniel-tailored
Copy link
Author

Also: with that change, as you mentioned, checkServerTrusted at least for getTrustAllTrustManager would still be empty.
This needs to be according to RFC 7030 4.1.1, right?
Anything that can be done to meet the security requirements (from google), to not have that?

change your code in the checkServerTrusted method of your custom X509TrustManager interface to raise .. exceptions .. whenever the certificate presented by the server does not meet your expectations.
https://support.google.com/faqs/answer/6346016?hl=en

ATM there is no way to use this dependency without the custom TrustManager implementations, even though they are not used.. for example: appmattus/certificatetransparency#18

Maybe that could also offer a way to resolve this for a library like the above from appmattus.

@skylerreimer
Copy link

Hey there. My team's app is also dealing with this. Is there any update here?

@dghgit
Copy link
Contributor

dghgit commented Apr 10, 2023

I'm not aware of anyone fixing the Android lint checker as yet.

@fvermeulen
Copy link

Hey any news on this?

@daniel-tailored
Copy link
Author

As for the certificate transparency library from app mattus, they removed the dependency.. therefore resolving the issue.
appmattus/certificatetransparency#50

@artyomdeynega
Copy link

Any update?

@dghgit
Copy link
Contributor

dghgit commented Feb 3, 2024

Apparently it's now possible to add:

@SuppressLint("TrustAllX509TrustManager")

to the source to disable this check.

I'd also note that:

plaid/plaid-link-android#231

provides other directions for how to suppress the error.

@dghgit dghgit closed this as completed Feb 3, 2024
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

No branches or pull requests

6 participants