jmshrv / finamp

A Jellyfin music client for mobile
Mozilla Public License 2.0
1.81k stars 123 forks source link

Self-signed certificate support #106

Open endervad opened 3 years ago

endervad commented 3 years ago

I've set up a self-signed certificate on my Jellyfin server and installed it also as a trusted root CA on my devices. It works fine without security warnings on Chrome (both Windows and Android) and on Android apps, like official Jellyfin and Gelli, except Finamp. When I try to connect to my server in Finamp 0.5.1, I get
HandshakeException: Handshake error in client (OS Error: CERTIFICATE_VERIFY_FAILED: unable to get local issuer certificate(handshake.cc:359))

Certificate extensions:

X509v3 extensions:
    X509v3 Key Usage: critical
        Digital Signature, Key Encipherment
    X509v3 Extended Key Usage:
        TLS Web Client Authentication, TLS Web Server Authentication
    X509v3 Subject Alternative Name:
        DNS:<redacted>
    X509v3 Basic Constraints: critical
        CA:TRUE
    X509v3 Subject Key Identifier:
        <redacted>
jmshrv commented 3 years ago

Try this APK, based off the allow-bad-certs branch: app-arm64-v8a-release.zip app-armeabi-v7a-release.zip app-x86_64-release.zip

This just allows all bad certificates, so I won't put this in the main branch. Just wondering, why don't you just use Let's Encrypt to get a proper certificate?

endervad commented 3 years ago

This one works, thanks :) I have a probably rather unique case: I've got a DNS name to my public IP from ISP. It's not part of contract, and I'm not even sure that it was intended, but I don't pay for it so might as well use it as is.

endervad commented 3 years ago

Upon further usage of allow-bad-certs version it seems like I was careless to assume, that if I can see the library, everything works. Neither downloads, nor playing songs work. In case of downloads there's only a single PlatformException((0) Source error, null, null, null) in Main Thread > AudioServiceHelper with null stack trace. All downloads fail almost instantly one by one after selecting where to save them (I gave storage permission to the app and I'm saving to default Internal storage). And in playing there's the exact same handshake exception for every attempt to play any song in Audio Service > MusicPlayerBackgroundTask with null stack trace.

jmshrv commented 3 years ago

Oh yeah, I forgot about native connections (the player and downloader are both native code). I'll look and see if there's any way of allowing bad certs.

About your setup though, are you connecting through an IP or a domain name? If it's a domain name, you should be able to get a certificate pretty easily.

endervad commented 3 years ago

It's a domain name, but not mine, it's ISP's. This same domain name is also used by ISP on their website, e.g. their website is example.com and DNS name to my assigned public IP is hostXXXX.example.com. Hence there's no way they would give me the private key to their certificate.

jmshrv commented 3 years ago

You may be able to get a certificate for just your subdomain: https://stackoverflow.com/questions/39322112/multiple-subdomains-with-lets-encrypt

endervad commented 3 years ago

That would require action from ISP side, since they're the owner of the whole domain with all subdomains. To get a certificate for a domain and/or subdomain you need to pass check(s) and prove that you're this domain owner. I'm not a domain owner in this case, I was just lucky enough to have a domain name for my IP. Ideally ISP could get a certificate specifically for my subdomain, yes, but there's no way they're going to do that, they're not that cooperative.

jmshrv commented 3 years ago

Let's Encrypt's "most common" challenge type just requests a web path on your server with the certificate, no DNS required. I doubt it would work if your ISP has some kind of wildcard certificate for all domains though.

https://letsencrypt.org/docs/challenge-types/

endervad commented 3 years ago

Okay, somehow I was sure that DNS-01 is a "must" initially, and only then either DNS-01, or HTTP-01 is used for automatic renewing. My bad :D I was able to get a proper certificate, it's all good now on release version of Finamp. Sorry for taking up your time and thanks for pointing the challenge type stuff out :) I can still see a scenario though, where allowing self-signed cert could be necessary. For example, if a person has public IP, doesn't have a domain name, and still wants to secure his traffic while not paying for a domain. It's possible to issue a certificate to an FQDN and then specify a desired IP in SAN extension. https://1.1.1.1 did it this way.

Trinnik commented 2 years ago

I have run into this problem with my setup at home and here's my scenario.

None of my services are publicly available, instead, I have an SDN setup using ZeroTier with an nginx proxy server that serves certificates locally. I have my root certificates installed on my user's devices so that they are able to have remote access including https with no open ports. This allows me the ability for my users to access everything as though it were on the public internet without having it actually available to the internet for security purposes.

Would there be a way to have this app use the user certificate stores provided by the OS?

jmshrv commented 2 years ago

Im assuming ZeroTier works by you connecting to an IP? If you control a domain, you could use the DNS-01 challenge. If not, it looks like ZeroTier's network is encrypted, however data may not be encrypted when travelling over the LAN.

I'll have a look at allowing custom certificates since I'm sure there are people out there who can't use a regular certificate, but last time I looked the only stuff I could find was just saying to disable certificate error handling, which isn't exactly secure.

Trinnik commented 2 years ago

ZeroTier uses peer to peer with root servers to negotiate the connection so unfortunately, everything on the network operates with virtual lan addresses' and node IDs, not publicly routeable.

I have no idea how you get access to the certificates trusted by the operating system but both Android and IOS allow you to trust foreign certificates. That's how I get my browsers to still tell me the webpage is secure. As long as my root certificate is installed at the OS level, it shows as a trusted cert.

I would assume that there's a way to access the trusted certificates from the OS. Love the app otherwise!

jmshrv commented 2 years ago

I'll have a look into it then, or I'll at least add an option to disable certificate checking.

sanyarnd commented 2 years ago

Same thing here.

=> can't use HTTP/DNS challenge and have to use self-signed certificated :(

Would be great if application will promt about self-signed certs, and make user accept it, like browser do.

skid9000 commented 2 years ago

I'll have a look into it then, or I'll at least add an option to disable certificate checking.

Hello, i'm bumping this issue cause i use a local CA to sign my certificates but it seems that finamp dosen't use Android local user CA base to check if it's trusted or not.

I do want to still use my own CA so without disabling certificate checking, could that be possible ?

Thx !

jmshrv commented 2 years ago

I believe this is an issue with Dart (or maybe BoringSSL?). I'll have a look into it because it's an annoying issue but no promises since poking around a programming language's source code isn't the easiest thing in the world.

zuoez02 commented 2 years ago

Why not just add a checkbox on login screen? Just let user choose trust the server or not?

ryneeverett commented 1 year ago

Along the lines of the previous suggestion, I'd propose as an alternative to using the system's CAs, apps like this should allow permanent exceptions the way a browser does when you click through all the warnings. It's basically a TOFU model because it only adds an exception for that specific certificate. If you change the certificate you're back to the warning page, or in the case of this app you'd be back to the login page.

avanier commented 1 year ago

From what I can see even system certificates are not trusted. (And I'm aware that this barely makes sense as a statement.) I'm trying to connect to a Jellyfin instance that has a known good self-signed certificate for which I have the root CA for the signing authority in my system bundle and I'm getting the handshake error. Is this the expected behaviour?

From the Flutter documentation it seems like CAs that are part of the system bundle should be respected. :confused:

Edit: After a bit of searching it seems this thread describes the situation quite well ... which is that even if I speak fluent x.509 I barely understand the Dart developers' explanation. :stuck_out_tongue_closed_eyes:

avanier commented 1 year ago

Alright, uh, so mostly bad news. I've been digging down this :rabbit: :hole: and Flutter simply does not support using the system bundles at all since Chopper does not respect network-security-config. It's a known bug. :disappointed:

I tried building the app while injecting my root CA as an additional anchor ... and even that's not being respected. You can find sample code (that cannot work) under this commit https://github.com/avanier/finamp/commit/229f038629d84b7aeef1cdef67d522b689325a1d . You'll see on the same branch as a drive-by I also tried updating Chopper to 5.0.0 and that didn't help.

I think the only realistic solution may be to add a checkbox to ignore the error like @zuoez02 suggests. Otherwise, to properly allow the use of system bundles would require getting rid of Chopper for something that makes native calls with the system API. Not a small task, and I'm not even sure it's possible at all under Flutter. (Lowly sysadmin here, I know absolutely nothing of Android development.)

jmshrv commented 1 year ago

Yeah...

It's a really weird omission from Dart, such a shame that we can't support it

OdinVex commented 1 year ago

I wonder if Chopper can simply be updated to respect system certs or if 'per cert hash' CAs can be allow-listed. Most solutions floating on Flutter's Issues list all revolve around modifying the Trust Anchor and such, which is fine by me (on a per-cert case similar to Mozilla Firefox's old methods before they went fully moronic against non-$$$ certs). ...I notice Flutter has over 5K Issues...wow.

jmshrv commented 1 year ago

I'll have to have a look to see how disabling certs worked, iirc you got a callback when something went wrong, so you might be able to override it for a specific cert.

Also, this isn't Chopper's fault, it's an issue in Dart. Otherwise, this would be a simple "move to a library that does support custom certificates"

OdinVex commented 1 year ago

I'll have to have a look to see how disabling certs worked, iirc you got a callback when something went wrong, so you might be able to override it for a specific cert.

Also, this isn't Chopper's fault, it's an issue in Dart. Otherwise, this would be a simple "move to a library that does support custom certificates"

If I recall, any custom callback circumvention has issues with Google Playstore certification/qualification? I believe I read that somewhere within Flutter's 5K Issues. I think for now it'd be best to just add a checkbox to ignore issues, but perhaps alert the user when the thumbprint of the cert has changed, if so. A decent-ish middle-ground until devs learn to stop using bad SSL/TLS backends that don't respect system-store certs. This problem is prevalent across far too many softwares for me to maintain. Epidemic.

jmshrv commented 1 year ago

Some relevant issues:

https://github.com/dart-lang/sdk/issues/48056 https://github.com/dart-lang/sdk/issues/50435

This function also looks interesting, we could have a manual option for now: https://api.dart.dev/stable/2.19.4/dart-io/SecurityContext/setTrustedCertificates.html

OdinVex commented 1 year ago

This doesn't seem to be getting any fixes, despite the situation being rather small to handle. I'd think this kind of issue would be a much higher priority than theming and coloring, eh. How about releasing two builds, then, one with the worst option of allowing all certs and one without? (Or just provide a bool-toggled checkbox...)

jmshrv commented 1 year ago

I'll get it sorted for the next release, which should hopefully be out this month. I've been busy with uni work since the last release, so I haven't been able to contribute much (a similar thing happened last year)

OdinVex commented 1 year ago

I'll get it sorted for the next release, which should hopefully be out this month. I've been busy with uni work since the last release, so I haven't been able to contribute much (a similar thing happened last year)

University should definitely come first then, totally understand. Awesome. :) I think the “safest” option for this would be to present a dialog to ask the user if they'd like to store the thumbprint of certs in a string array (of thumbprints) when attempting any connection and do thumbprint validation in the HttpOverrides codeblock (describing the allow-bad-certs branch modifications) and allow managing the list in the Settings. It lets everyone enjoy their cake and shouldn't be difficult to manage.

jmshrv commented 1 year ago

If setTrustedCertificates does what it sounds like, we could probably do it "properly" and have the certificate still be checked. It looks like the Flutter/Dart team are talking this seriously based on the activity in the issues linked here, so it'll hopefully be even more proper soon :)

OdinVex commented 1 year ago

If setTrustedCertificates does what it sounds like, we could probably do it "properly" and have the certificate still be checked. It looks like the Flutter/Dart team are talking this seriously based on the activity in the issues linked here, so it'll hopefully be even more proper soon :)

You could do that, but you'd need for users to import the cert manually or grab it from the connection on first-connect and then ask/store it. Proper would be Flutter/Dart trusting the System cert Store like they should be doing.

jmshrv commented 1 year ago

I'll have a look at both solutions, just don't want to accidentally make an insecure system. No point having a self-signed cert if someone can just MITM you by changing it

OdinVex commented 1 year ago

I'll have a look at both solutions, just don't want to accidentally make an insecure system. No point having a self-signed cert if someone can just MITM you by changing it

The whole point behind thumbprints. Presenting a dialogue every time a thumbprint changes (a different cert at endpoint) is what informs users the cert they're interacting with has changed. Dart/Flutter just need mostly to stop using isolated copies of stores and just use the system store, like they should be doing in the first place.

svalavuo commented 1 year ago

I'm getting this same error with valid Letsencrypt R3 certificate. Browser (from computer or mobile) works just fine.

jmshrv commented 1 year ago

What device are you using?

svalavuo commented 1 year ago

What device are you using?

I have Xiaomi Mi 10T Pro Android phone. With browser (I use Brave) from the same phone, Jellyfin behind nginx -proxy (with LE R3 certificate) works. For some reason Finamp gives me the same error. From local network, accessing Jellyfin directly, Finamp works fine.

jmshrv commented 1 year ago

That's really weird. Maybe Xiaomi doesn't have the correct root certificate in the system store? Some browsers like to use their own root certificate stores.

When accessing from the local network, are you just connecting directly over HTTP?

svalavuo commented 1 year ago

Yeah. In local network I tested with http (without ssl) and local ip (and port). I tried with three different browsers (Brave, Chrome and Mi Browser) and they all worked with https and my hostname. Finamp doesn't. :-( edit: I tried Vivaldi too. Same result. It works as well.

svalavuo commented 1 year ago

My issue is solved! I tried ssllabs -site to test my certificate and noticed, that chain is incomplete. After fixing that Finamp works fine. For some reason all browsers in my phone worked with incomplete certificate. For certificate issues ssllabs (https://www.ssllabs.com/) is a great site.

OdinVex commented 1 year ago

My issue is solved! I tried ssllabs -site to test my certificate and noticed, that chain is incomplete. After fixing that Finamp works fine. For some reason all browsers in my phone worked with incomplete certificate. For certificate issues ssllabs (https://www.ssllabs.com/) is a great site.

All browsers on your phone worked fine because they had the root cert for LER3. You didn't need to include a ( common) root in the chain because it was present in your phone browsers. My cert issue appears only with Finamp, even my phone browsers work (once mine is imported to system store on each device and each browser is told to listen to the system store). So mine is an issue with Finamp trusting system store, as it should, or allowing import of certs as a cop-out option.

OdinVex commented 11 months ago

At this point it'd be easier to just include a checkbox to trigger an override since Dart stupidly won't respect system store certs. Curse Mozilla for starting that 'cert bundle' bull@!#% to begin with.

Try this APK, based off the allow-bad-certs branch: app-arm64-v8a-release.zip app-armeabi-v7a-release.zip app-x86_64-release.zip

This just allows all bad certificates, so I won't put this in the main branch. Just wondering, why don't you just use Let's Encrypt to get a proper certificate?

This only allows browsing, playing won't work. AudioService itself reports issues with certs. Sticking to the regular Jellyfin apk for now.

PlexSheep commented 10 months ago

This issue is 2 years old by now. Can we please just get a checkbox that trusts this certificate?

OdinVex commented 10 months ago

This issue is 2 years old by now. Can we please just get a checkbox that trusts this certificate?

As stated in this Issue: Google won't allow developers to include options like that if they're going to use the Play Store. For versions through F-Droid, entirely possible. To allow Finamp to use the system store (certificate store) the backend library (not controlled by Finamp developer(s)) needs to implement and respect proper design. Edit: It would definitely be nice to at least get the option for F-Droid builds.

PlexSheep commented 10 months ago

Yes. I understand it's a restriction with Google play. I'm personally using FDroid and I just can't use finamp when my servers certificate is not accepted.

I don't know how much of a maintenance effort it would be to do two versions, but it would be very handy. Especially regarding the fact that the used library/environment does not seem to address the issue.

Edit: I believe that the impossibility to use self signed certificates should be mentioned in the Known Issues section to avoid confusion.

OdinVex commented 10 months ago

Yes. I understand it's a restriction with Google play. I'm personally using FDroid and I just can't use finamp when my servers certificate is not accepted.

I don't know how much of a maintenance effort it would be to do two versions, but it would be very handy. Especially regarding the fact that the used library/environment does not seem to address the issue.

It'd be rather simple and easy, just a pain to make sure it is F-Droid's compilation. If you're familiar with C/C++, think #define and note that being able to define variables for your project on a repo (GitHub or through F-Droid compile process) would allow you to #ifdef the scenario to include code specifically for F-Droid builds that would add a checkbox and the functionality. No maintenance afterward, just the initial setup and to remember about this until the library backend gets theirs together proper.

Edit: I believe that the impossibility to use self signed certificates should be mentioned in the Known Issues section to avoid confusion.

Agreed, regardless of whether F-Droid has enabled builds or not. It should also note that Jellyfin's app can do music, too, just not as designed for music player-like interaction. I've been using it instead, despite limitations.

Chaphasilor commented 10 months ago

Just to recap:

I agree that adding a checkbox that explicitly disables all checks. This may or may not be supported by Google Play, but a custom build for download from GitHub should be entirely possible, once he issues above are solved.

jmshrv commented 10 months ago

The native code may use the system stores as intended, I haven't got around to this as I'm selfishly not affected by it myself

PlexSheep commented 10 months ago

The native code may use the system stores as intended, I haven't got around to this as I'm selfishly not affected by it myself

Can you clarify what you mean by may?

jmshrv commented 10 months ago

I haven't actually checked but I'd assume ExoPlayer/AVPlayer would respect system certs as other native apps do

OdinVex commented 10 months ago

Just to recap:

* Trusting custom certificates seems to be not possible. There are issues with the Flutter framework itself, and also with the HTTP client we're using

You can add certs to be trusted, it would work, you'd just need to implement, but you'd run into the same Google PlayStore issue. They don't allow that.

* Trusting _all_ certificates might be possible, but I wouldn't bet on it. The native code we're using comes from other libraries, not us, and if they don't support it we're probably out of luck. I'm not sure how far @jmshrv got with his investigations...

Same issue, Google PlayStore. Edit: If the native backend brings support then it'd work and you wouldn't need to do anything at all.

I agree that adding a checkbox that explicitly disables all checks. This may or may not be supported by Google Play, but a custom build for download from GitHub should be entirely possible, once he issues above are solved.

GitHub, especially F-Droid, that'd be awesome.

jmshrv commented 10 months ago

I can't find anything from Google saying that ignoring certificate errors isn't allowed. Browsers let you do it, after all