-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Enabling TLS1.1/1.2 on Android 4.4 devices (API 19/KitKat) #2792
Conversation
Player may be complicated as it is a Wohle framework. |
I found google/ExoPlayer#1819 which says I should set Where would be the best place to call that static method and where should I call it if I also want to use it for the other two connections? As I wrote, I'm clueless, but motivated. I can do some research myself but I wanted to try the easy way (asking), first ;-). |
I tried the static-function approach and put In any case, please check the given commits. Should I roll them up into one? |
I'm sorry for my impatience, but Is there a way to speed this process up? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sorry for the silence. There are just too many open PRs and KitKat has only a small number of active users so other PRs had a higher priority. There was a second PR which changed some important aspects of the downloader. Therefore, this one needed to wait, too.
However, we have time to review your changes now. can you please rebase your commits? The new downloader was moved to https://github.com/TeamNewPipe/NewPipe/blob/dev/app/src/main/java/org/schabi/newpipe/DownloaderImpl.java
@Stypox, @yausername, @theScrabi or @mauriciocolli Can someone review this PR, too? This is security relevant and I am not an expert on this topic. I'll try to give a hint regarding the player within the next week, but I'd be glad if someone else could take a look at it.
app/src/main/java/org/schabi/newpipe/util/TLSSocketFactoryCompat.java
Outdated
Show resolved
Hide resolved
app/src/main/java/org/schabi/newpipe/util/TLSSocketFactoryCompat.java
Outdated
Show resolved
Hide resolved
Thanks for reviewing so far! I rebased and added most of your suggestions. It should look much cleaner now. I tested it on my device and everything still works. |
I now rebased on the current |
So I rebased and tried out PeerTube. It didn't work. Here is why: The following are enabled by default on an Android 4.4.2 Emulator (All Lists are sorted alphabetically to make comparison easier.)
The following are supported on an Android 4.4.2 Emulator, but not enabled:
(these are either Those will be enabled by the last commit, if possible: (OkHttp moderntls +
On an Android 4.4.2 device without vendor changes to the suites, this means that the following cipher suites are enabled with tls1.2:
Which doesn't overlap with the Framatube.org cipher suites, so no connection can be made. If you notice, it does say that kitkat devices can connect below but I simply can't find the source for that statement; I also couldn't enable the given Ciphersuite in any way. If we want to fix that on the server (we probably have to), we could start by changing the default nginx config recommended by Peertube introduced here: (Chocobozzz/PeerTube@e883399) |
There was another thing I noticed, while trying out mediaCCC some more: Sometimes,when I download a video, it will work at first but then stall, while the console throws the following error(with context):
I can pause and resume the download and it will continue where it stopped, so It's not a huge issue but it 'll be annoying. I'll investigate that some more. |
I tried watching with wireshark and only found one mirror (selfhost.de) having an unspecified TLS error while connecting... I don't know how to fix this so i'll let it be for now. |
After a reread I understand that peertube won't work on kitkat unless the instances update their |
I tried your |
Interestingly, the peertube docker config contains a traefik instance which would work with kitkat: https://github.com/Chocobozzz/PeerTube/blob/develop/support/docker/production/config/traefik.toml#L27 |
For the meantime, I tried another instance (tube.tchncs.de) and tested downloading and playing. While browsing works, downloading obviously only works for videos from instances with similar TLS settings. For instances with "wrong" tls settings, that doesn't happen and it will instead repeat the message |
With help from @Chocobozzz, framatube.org is working flawlessly on Android 4.4 and the upstream nginx template was altered (Chocobozzz/PeerTube#2330)! I checked video playback again and found that this is mostly an emulator issue. On the x86 Kitkat emulator without play services, video playback was not possible. On my personal phone it works just fine though. As this thread has gotten a bit long and unwieldy, I'll summarize the past and present issues with newpipe on Kitkat: MediaCCC:
Peertube instances with matching ciphersuites (set server-side):
Peertube instances without matching ciphersuites (set server-side):
This is now in a state I would consider finished. The last big group has to be solved by writing kind emails to the admins of affected instances and maybe tuning the Newpipe UI for Peertube a bit by displaying instences for remote videos. If there are no other opinions for the commented-out code in the reviewed section, I'll clean it up and this is ready to get merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, this is on my list for 0.18.0.
@mqus Can you please rebase on the current dev. @yausername I'll rebase the release branch after merging.
app/src/main/java/org/schabi/newpipe/util/TLSSocketFactoryCompat.java
Outdated
Show resolved
Hide resolved
This enables modern TLS versions in the collection browser, the Downloader and the Player. This is neccessary because media.ccc.de rejects all older TLS connection attempts, see issue #2777.
…standard Android 4.4.2 devices
Rebase&fixing is done. Github still wants me to adress your concerns but wont let me resolve them... EDIT: just to be clear though, I've applied every change you requested. I don't think that an error screen on that position is good (when starting the app and when initializing the DownloaderImpl). The Connection errors shown currently don't say much but I think that is good enough for now and would require more extensive changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mqus Thanks.
I checked video playback again and found that this is mostly an emulator issue. On the x86 Kitkat emulator without play services, video playback was not possible. On my personal phone it works just fine though.
I've tested this with two Android KitKat emulators and could not watch PeerTube videos for some reason, too (tested with framatube and video.blender.org). But accessing the video info and performing searches works fine. I have the strong feeling, that the emulators are far from being perfect. They are way too buggy. Unfortunately, I do not own a phone running KitKat. I'll trust you and we'll see, whether playback works fine on other devices, too.
I don't think that an error screen on that position is good
I agree.
Github still wants me to address your concerns but wont let me resolve them...
Yep, that's my job :)
Thank you @ all! If there are any connection errors with kitkat devices in the future or any other stuff I should reproduce with my phone, just ping me and I'll see what I can do. I'll also write some more E-Mails to instance admins but I probably wont cover them all. |
@mqus thank you for the effort! I'll gladly come back to your offer ❤️ |
This is an attempt to fix #2777 . I already got the Downloader and the Extractor parts working. What I may need some help on is the player. I simply don't know how to approach this and where to start.
Please give me some feedback on the "finished" parts, too.