feat: add support for Android user-installed certificates

[jfly]

Description

This is part of #15230.

Frustratingly, Dart/Flutter ignores user-installed certificates. Working around this requires rooting your Android device to install certificates as "system" certs, which isn't an option for everyone.

This is a known issue with Dart, see
dart-lang/sdk#50435 and
flutter/flutter#56607 for details.

I have read through
#15230 and #13555, and I understand that switching to
cronnet_http would also resolve this. While that may be the correct long-term approach, it looks like there are a lot of network codepaths in Immich, and as I know basically nothing about Dart, nor Flutter, nor Immich's codebase, I thought this would be a better short term approach.

This depends on my fork of
johnstef99/flutter_user_certificates_android, which I've sent a PR for here
johnstef99/flutter_user_certificates_android#2. If y'all don't like the supply chain implications of that, I'm happy to inline the implementation here instead.

How Has This Been Tested?

I have step-ca and DNS configured so that immich.mm resolves to an immich server with a certificate issued by my step-ca certificate authority. I've installed that certificate authority as a user-installed certificate on my Android phone.

• With a fresh install of immich, I can configure https://immich.mm as a server
• After logging in, I can view photos, upload photos and video, and play video.

Screenshots (if appropriate)

The help text on this option is no longer true. I'd suggest that we deprecate it in favor of encouraging people to add certificates to their OS:

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation if applicable
• I have no unrelated changes in the PR.
• I have confirmed that any new dependencies are strictly necessary. Strictly? No. See my offer above about to inline the implementation if that's preferred.
• I have written tests for new code (if applicable)
• I have followed naming conventions/patterns in the surrounding code
• All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
• All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

[bo0tzz]

There's no way to make this opt-in, right?

[jfly]

I don't know, but everything I know about android development I learned in the last week while researching this issue.

[bo0tzz]

If (big if, imo) we're keeping this repo reference then it should at least be pinned to a commit hash so we know what we're getting.

[jfly]

I totally understand. Happy to do any of the following:

1. Pin to a specific commit, as you mention.
2. Depend on the unpatched flutter_user_certificates_android and vendor just the changes from Add support for trusting android user certificates johnstef99/flutter_user_certificates_android#2.
3. Vendor the whole thing. We're not talking about a lot of code here.

Just let me know which approach to take.

[jfly]

@bo0tzz, friendly ping here. Which approach would you like me to take?

[shenlong-tanwen]

@jfly Hey, Thanks a lot for working on this. However, if we are to merge this in, rather than deprecating the existing "Allow Self signed certificates" toggle, we could just remove all the custom verification logic all together. We don't need a lot of different codepath to address the same issue. However, this would mean that we would also need to test this exhaustively to make sure it does not break anything from the current setup. So things like:

• mTLS
• Self-signed certificates
• Custom CAs

For all cases, we'd need to test both the dart side of things and the native side of things. Most network calls are from the dart side. Video player, Asset download are made from native side and we should validate these as well. If we are to remove the self signed toggle, we've to make sure everything works on iOS as well.

You could just inline all the changes. We are migrating to using pigeon for all message passing between flutter and the platform side, so you can add an API to fetch the certificates and load it in the dart's SecurityContext. We might also need some documentation change to inform users about installing certificates to the OS cert store, rather than loading it in the app like they used to.

The core team currently has got it's hands full with working towards the stable release. I know this is a significant undertaking for your free time, but if you can get this properly tested after removing the existing toggle and adding in your changes, we can happily merge this in. If you need help setting up specific test scenarios or access to different certificate setups, let us know - we can coordinate with the community to help test various configurations.

[shenlong-tanwen]

Suggested change

	<base-config>
	<base-config cleartextTrafficPermitted="true">

[jfly]

Why? Does my PR change the default behavior somehow? The docs say:

Any values that are not set use the platform default values.

[shenlong-tanwen]

The android:usesCleartextTraffic="true" set in the manifest file still seems to be respected. But it is best to also set it in the network config since the platform default varies between different versions of Android

[jfly]

But it is best to also set it in the network config since the platform default varies between different versions of Android

That makes sense, but I don't see how this is related to my changes here. In the interest of keeping this PR targeted, should I send it a separate PR that adds this network_security_config.xml file with the explicit cleartextTrafficPermitted="true"?

[jfly]

I know this is a significant undertaking for your free time, but if you can get this properly tested after removing the existing toggle and adding in your changes, we can happily merge this in.

Sure, I'm happy to take this on. I've converted my understanding of your requirements into a checklist below. Please let me know if I'm missing anything:

• Inline the changes (drop dependency on johnstef99/flutter_user_certificates_android)
  • Use pigeon for message passing between dart and kotlin.
• Remove "Allow self-signed SSL certificates" checkbox
• Docs? @shenlong-tanwen, please advise. IMO this is unnecessary, it's just fixing a bug and making things work the way users would expect. I do think a release note would be useful, is there a good place to add that?
• Test on Android. Confirm you can view photos, upload photos and video, play video, and download assets.
  • mTLS (aka "SSL Client Certificate")
  • Self-signed certificates. Works with caveats. See feat: add support for Android user-installed certificates #19830 (comment) for details.
  • Custom CAs
• Test on iOS. Confirm you can view photos, upload photos and video, play video, and download assets. @shenlong-tanwen, I will need help with this as I do not have access to an iOS device, nor development machine.
  • mTLS (aka "SSL Client Certificate")
  • Self-signed certificates
  • Custom CAs
• Test upgrade migration from the older version with the setting on with a certificate to the new one without the toggle
  • Android
  • iOS

[shenlong-tanwen]

Sure, I'm happy to take this on. I've converted my understanding of your requirements into a checklist below.

This is perfect! Thanks a lot for taking this on. As far the documentation, we can discuss this during the release of this PR. But yes, a callout in the release note might just do. It is also probably worth testing the migration from the older version with the setting on with a certificate to the new one without the toggle. Please feel free to ping me here or over discord if you need any help with the implementation or testing

[jfly]

@shenlong-tanwen, I'm ready for you to take another look at this PR. I've updated my checklist with what I have done and not done.

Notably, with these changes, self-signed certificates work, but Android only lets you install them if they have the X.509v3 CA:true flag. You get a really confusing message saying a "private key is required to install a certificate". See https://android.stackexchange.com/questions/237141/how-to-get-android-11-to-trust-a-user-root-ca-without-a-private-key for details.

So, we need to decide what to do here. A couple proposals:

1. Just make the change. Folks who want to self-sign their own certificates can do so, but they should be running a proper certificate authority. The existing feature (where we don't validate certs at all) is dangerous and should go away.
2. Keep the "Allow self-signed SSL certificates" feature around, but reword it so it's clear that you should not be using it if you're running your own CA.
   • I still think it's a dangerous feature and should go away. Ideally we'd tell people it's deprecated and will be removed [at some point].

[richardlt]

Personally I agree with your second point, since this feature here will not strictly replace the existing flag it could be a breaking change for some users to remove it. I'm also not sure about the options for IOS users to install custom CA.
Maybe another proposal could be to keep it for now and to improve it later. For example in some applications if you try to reach a server with a self signed cert you have to approve it directly in the app and then this app will remember your choice so if the cert change you'll have to approve it again.

[jfly]

I'm also not sure about the options for IOS users to install custom CA.

This works fine. My wife is on iOS and has our custom CA installed on her phone, and immich works great.

For example in some applications if you try to reach a server with a self signed cert you have to approve it directly in the app and then this app will remember your choice so if the cert change you'll have to approve it again.

Yeah, that would be a ssh-style TOFU scheme. I don't have the same safety concerns, but that's a whole bunch of code that I don't really see the value in anyone writing, nor having the core immich team maintain.

IMO, it would be better for immich to take an opinionated stance and say that anyone running their immich server without "universally trusted" certs is required to be have that cert chained off of a root of trust with CA:true (aka: running their own certificate authority). Anyone who is doing this is likely going to run into the same issue with other self hosted services, anyways. Baking the trust into your OS is simplest for both the app developers and the users.

Of course I am happy to do whatever the core team needs to get this PR landed. IMO, the best solution would be to rebrand the existing option as dangerous and mark it deprecated, with the intent to remove it before the stable release.