danielzgtg commented 4 years ago

[x] I carefully read the contribution guidelines and agree to them.
[x] I checked if the issue/feature exists in the latest version.
[ ] I did use the incredible bugreport to markdown converter to paste bug reports.

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 commented 4 years ago

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

mqus commented 4 years ago

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 commented 4 years ago

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

mqus commented 4 years ago

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 :wink: )

TobiGr commented 4 years ago

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

mqus commented 4 years ago

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 commented 4 years ago

Will this force np to abandon kitkat users?

My kodi hardware is unable to upgrade past kitkat

mqus commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

Anything changed here?

wb9688 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

mqus commented 4 years ago

@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 commented 4 years ago

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

danielzgtg commented 4 years ago

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 commented 3 years ago

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

mqus commented 3 years ago

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 commented 3 years ago

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 commented 1 year ago

Close ?

danielzgtg commented 1 year ago

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

opusforlife2 commented 1 year ago

From #8624 linked above:

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

So we're good.

TeamNewPipe / NewPipe

Deprecate TLS 1.1 #2898

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.