search

twilio / twilio-chat-demo-android

Chat API Demo Application for Android
MIT License
63 stars 51 forks source link

Insecure TLS/SSL trust manager #150

Closed emartynov closed 3 years ago

emartynov commented 3 years ago

Description

We updating to AGP 4.1 and it reports SUBJ issue for the OkHttpFactory inner class in the library:

com/twilio/messaging/transport/OkHttpFactory$1.class: checkClientTrusted is empty, which could cause insecure network traffic due to trusting arbitrary TLS/SSL certificates presented by peers

Steps to Reproduce

  1. Add Twilio chat SDK to project with AGP 4.1
  2. Run lint

Expected Behavior

No lint reports around Twilio library

Actual Behavior

Reported above error

Reproduces how Often

Always

Chat Android SDK

Which SDK version did you use? 6.1.0

berkus commented 3 years ago

This is red-herring warning, code is valid. The only way to instantiate this empty validator is running local tests.

emartynov commented 3 years ago

@berkus Do you mean instrumental tests? Do you have documentation how to use it in test?

berkus commented 3 years ago

@emartynov you don't, this is for sdk tests only.

emartynov commented 3 years ago

@berkus Then why to ship production library with code that is used in the internal tests?

emartynov commented 3 years ago

@berkus Should i pass it to your support?

berkus commented 3 years ago

@emartynov pass what?

berkus commented 3 years ago

We are aware of your concern with ssl testing, we will handle it. No need to worry.

emartynov commented 3 years ago

@berkus Does it mean next update lint will not report me about the class that is used in the internal tests?

berkus commented 3 years ago

No, it does not.

emartynov commented 3 years ago

@berkus I'm sorry for the harsh comment. I was just triggered by "you don't need to worry" phrase. I should say instead:

Can I hope that in the nearest releases you will fix it and I can remove lint rule ignorance? If yes, do you have ETA for it?

berkus commented 3 years ago

I don't have an ETA, but we have this in our short-term plans. As soon as one of our developers is available to review your dependencies update PR he will most probably also look at the warnings this update produces and hopefully fix them, too.