[PR] Enable TLS 1.3 by default

[thanhminhmr]

Fix #1278

[thanhminhmr]

@QuantumBadger I fixed all CI problems that my new code introduces, but there are some problem came from updating target Android to 33 (A bunch of error when run gradle lint). This is a bit outside of my Android knowledge, so I hope that you can either point me to the right direction with those, or just ignore the dependency updates altogether. Thanks!

[QuantumBadger]

Hi @thanhminhmr, thank you for the PR! I'm currently undecided whether it's a good idea to use Conscrypt in RedReader.

It would be a fundamental change to the security model -- previously we've relied on the system APIs for TLS (and other native operations like image/video decoding), with all the app's logic written in Java/Kotlin. As a result there's never been a security issue discovered in RedReader, and we've never had to release an update for security reasons.

Putting Conscrypt (and hence the native BoringSSL library) inside RedReader means that:

• If there are vulnerabilities in Conscrypt, RedReader would be vulnerable
• We have to release a new version of RedReader very quickly after Conscrypt gets updated, to ensure any security fixes get pushed to users

Conscrypt seems to get updated very infrequently -- it's only had one release in the last three years.

I can't find records of any BoringSSL vulnerabilities since 2018, which is either a good sign or a bad sign 😂 It's a large C/C++ project, with the notoriously vulnerable OpenSSL as the base, and frankly I'd be impressed if they had precisely zero vulnerabilities in seven years of development (when OpenSSL has had a vast number in the same time).

So, I'm not yet decided, and open to arguments either way!

[QuantumBadger]

On the topic of the PR itself -- it would be good to remove all the dependency updates, as these are best handled separately (and I think there's another PR open for this already). It's possible to run Lint locally using ./gradlew lint, and an HTML report gets generated in build/reports/.

[thanhminhmr]

I kind of agree with your points. Introducing a security related library that are not that trustworthy is not a good thing to do. For now I think restrict OkHttp to TLSv1.2 and TLSv1.3 is a better way to do it.

There is a few nuance though:

1. Android only supported TLSv1.3 natively in API level 29 (Android 10), so installing RedReader on older Android will still work, but without TLSv1.3. This is why I introduce Conscrypt in the first place: to support TLSv1.3 on older Android.
2. This also prevent OkHttp from accessing any HTTP website (which by the previous configuration it can). I cannot think of any problem introducing by this, as HTTP-only sites are nearly gone for good.

[OmegaAOL]

I support this change. TLS 1.3 is faster and Conscrypt lets you run it on down to Android 2.3 "Gingerbread", so compatibility isn't a problem.