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

Deprecate TLS 1.1 #2898

Closed
2 of 3 tasks
danielzgtg opened this issue Dec 28, 2019 · 24 comments
Closed
2 of 3 tasks

Deprecate TLS 1.1 #2898

danielzgtg opened this issue Dec 28, 2019 · 24 comments
Labels
discussion This needs to be discussed before anything is done feature request Issue is related to a feature in the app

Comments

@danielzgtg
Copy link

#2792 enabled TLS 1.1 and TLS 1.2. However, there are concerns that TLS 1.1 is vulnerable. The whole web has already started phasing out TLS 1.1. If left enabled, it makes it easy to execute a downgrade attack.

Are we sure that we need both TLS 1.1 and TLS 1.2? If we only need the latter then it's better to have only TLS 1.2 enabled.

@TobiGr
Copy link
Member

TobiGr commented Dec 28, 2019

I agree. @mqus could you please check whether it is necessary to enable TLS 1.1?

@Stypox Stypox added discussion This needs to be discussed before anything is done feature request Issue is related to a feature in the app labels Dec 28, 2019
@mqus
Copy link
Contributor

mqus commented Dec 28, 2019

No, we don't need TLS 1.1. Youtube, mediaCCC, Soundcloud and propably all peertube instances support tls 1.2 by now so 1.1 is not neccessary.

Just to be sure though, TLS 1.1 was only "enabled" for Kitkat (Android 4.4.2) devices and fixing that is easy. I don't know what tls versions are enabled/disabled by default on the other androids, the android docs suggest that 1.1 and even tls 1.0 are enabled on all newer platforms by default.

But I also think that the security risk is limited, even browsers still have tls 1.0/1.1 enabled and Newpipe doesn't even handle information which is sensitive.

So to summarize, I don't think we need tls 1.1 right now, but removing it does require a bit of work** which I find not worth the effort. We would also preserve a bit of backwards-compatibility in the rare case that a peertube instance doesn't support tls 1.2. But feel free to correct me if I'm wrong here.

**(for platforms other than kitkat)

@DI555
Copy link

DI555 commented Dec 28, 2019

the bad thing of dropping tls 1.1 will be dropping support andoid 4.4, am i right? (if so , would be sadly)

@mqus
Copy link
Contributor

mqus commented Dec 28, 2019

no, not at all, #2792 added support for tls 1.1 and 1.2 , so just disabling 1.1 would totally be possible and easy for android kitkat. (I have only an android 4.4 smartphone myself and would like to keep NewPipe for a bit longer 😉 )

@TobiGr
Copy link
Member

TobiGr commented Dec 28, 2019

Thanks @mqus. Can you create a PR to remove TLS 1.1, please?

@mqus
Copy link
Contributor

mqus commented Dec 28, 2019

Sure, but that will only apply for Android 4.4, all other android versions will have enabled TLSv1.0,v1.1,v1.2 and also v1.3 if new enough.

Disabling TLSv1.1 (or TLSv1.0) for those other androids is something I would rather leave to a time when the browsers actually stop supporting TLS. I'm also not sure yet how to implement this. Adding TLS 1.2 for only a small subset of users (like I did) with a workaround is something i can get behind but using the same workaround to disable TLSv1.1/1.0 for ALL users is not something I would like to do and then confidently stand behind.

Removing TLS 1.1 support for Kitkat (meaning: for a minority of users) is only a cosmetic fix in my opinion.

@bleedingcrow
Copy link

bleedingcrow commented Jan 24, 2020

Will this force np to abandon kitkat users?

My kodi hardware is unable to upgrade past kitkat

@mqus
Copy link
Contributor

mqus commented Jan 24, 2020

no, not at all, #2792 added support for tls 1.1 and 1.2 , so just disabling 1.1 would totally be possible and easy for android kitkat. (I have only an android 4.4 smartphone myself and would like to keep NewPipe for a bit longer 😉 )

(Please read above)

To summarize: we'll keep tls 1.2 and disable 1.1.

@mqus
Copy link
Contributor

mqus commented Jan 29, 2020

While searching for a fix for #3021, I found https://guardianproject.info/code/netcipher/ .
This is mostly a generalized version of my workaround (#2792) and will enable the appropriate TLS versions and ciphersuites for the matching android versions. They don't include any ciphers or crypto on their own and just configure the built-in crypto of android (at least for now, they are looking at including a wrapped version of the Google Conscrypt library and using it in older android versions). For example. they already disabled support for TLS 1.1 and 1.0 if 1.2 is enabled.

We could use NetCipher to replace util.TlsSocketFactoryCompat with their version and then apply it where it's needed. As I assume that they know more than me about Android & Connection security, we could just ride on their changes and save the effort for something else.

@mqus
Copy link
Contributor

mqus commented Mar 3, 2020

Tbh, I tried to implement it but got stuck on https://gitlab.com/guardianproject/NetCipher/issues/13. As long as that issue isnt solved, I can't really make progress.

@Redirion
Copy link
Member

Redirion commented Mar 6, 2020

@mqus
this project looks like it is not well maintained. If you check here: https://dl.bintray.com/guardianproject/CipherKit/info/guardianproject/netcipher/netcipher/

the last version that included the matching okhttp3 dependency was 2.0.0-alpha1

@opusforlife2
Copy link
Collaborator

Anything changed here?

@wb9688
Copy link
Contributor

wb9688 commented Aug 12, 2020

@opusforlife2: No, imho we should only do it as soon as we move to minimum Android version of 5.0, which has to be before the end of 2021.

@mqus
Copy link
Contributor

mqus commented Aug 12, 2020

tbh TLS 1.1 can be deprecated right now but the "how", especially on Android >=5 is not clear. For Android 4.4 this is simple and has no downsides.

@wb9688
Copy link
Contributor

wb9688 commented Aug 12, 2020

@mqus: Meh, there are quite some 4.4 devices not supporting TLS 1.2 unfortunately

@mqus
Copy link
Contributor

mqus commented Aug 13, 2020

@wb9688 There are? I thought that Android did support this on all devices but each app has to enable it, because it was "experimental" at the time. But I'm not an expert and device vendors do all sorts of terrible things.

@wb9688
Copy link
Contributor

wb9688 commented Aug 13, 2020

@mqus: That was the idea, however device vendors are indeed terrible ;)

@danielzgtg
Copy link
Author

But I also think that the security risk is limited, even browsers still have tls 1.0/1.1 enabled and Newpipe doesn't even handle information which is sensitive.

This is no longer true. Chrome 84 (August 2020) has removed TLS 1.1.

Removing TLS 1.1 support for Kitkat

I now believe it's not worth the effort bothering to remove TLS1.1 for KitKat.

Old Android versions are end-of-life and don't get security updates. They have so many security vulnerabilities left unpatched whether due to Google discontinuing support or manufacturers' planned obsolescence. Of particular relevance to NewPipe are the infamous MediaServer vulnerabilities that plague Android and mean that just playing a video might allow someone to take over the device.

Trying to secure Android 4's TLS is like "locks on the chimney with the front door wide open." Only securing Android 8 and other supported versions will make a difference.

@FireMasterK
Copy link
Member

Can't we add TLS 1.3 support with https://github.com/google/conscrypt?

@mqus
Copy link
Contributor

mqus commented Dec 20, 2020

Can't we add TLS 1.3 support with https://github.com/google/conscrypt?

I haven't looked at it but it should be possible. netcipher also planned to build on(/wrap) conscrypt... but as written above, it(netcipher) is sadly no longer maintained.

@Redirion
Copy link
Member

Tbh, I tried to implement it but got stuck on https://gitlab.com/guardianproject/NetCipher/issues/13. As long as that issue isnt solved, I can't really make progress.

@mqus there has been progress at Netcipher. 2.1.0 and 2.2.0alpha have been released with okhttp dependencies in April und May respectively: https://mvnrepository.com/artifact/info.guardianproject.netcipher

Do you want to give it another shot?

@ShareASmile
Copy link
Collaborator

Close ?

@danielzgtg
Copy link
Author

Closing as I assume you're saying this is already implemented

@opusforlife2
Copy link
Collaborator

From #8624 linked above:

TLSv1 and TLSv1.1 are no longer enabled by default.

So we're good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion This needs to be discussed before anything is done feature request Issue is related to a feature in the app
Projects
None yet