immich icon indicating copy to clipboard operation
immich copied to clipboard

[META] Experimental network features

Open mmomjian opened this issue 1 year ago • 58 comments

Experimental network features

Features

There are some network features that have been added to the Immich mobile app through the years that interact with various libraries and background tasks in unexpected ways. These features include:

  • HTTP Basic Auth for server URL
  • Self-signed SSL certs
  • Mutual TLS
  • Custom HTTP proxy headers

Limitations

Typically, these network features work fine in the normal library view, but cause issues with:

  • video playback
  • foreground asset upload
  • background asset upload
  • interrupted uploads
  • asset download
  • App crashing

Solutions

Thanks to the recent implementation of auto-switching server URLs in #14437, many of these features are no longer needed for a smooth Immich experience. There are other options, including the use of a VPN or a real SSL cert.

Handling known issues

We have updated the FAQ (link will work once #15228 is merged). This issue can be used to track any known issues with these experimental network features. These issues will not be a priority for the dev team, but can be improved by the community if possible.

Please do not use this discussion for comments like "same" or +1". This is designed only to keep track of current known limitations to these experimental features, or discuss potential solutions and future patches.

mmomjian avatar Jan 10 '25 18:01 mmomjian

The final sentences sound quite intimidating, so I'm wondering: Is it allowed to discuss analysis of root causes and potential solutions here? Or where would that go?

rovo89 avatar Jan 10 '25 19:01 rovo89

The final sentences sound quite intimidating, so I'm wondering: Is it allowed to discuss analysis of root causes and potential solutions here? Or where would that go?

If the discussion is relevant to potential future PRs or improvements, or the actual underlying issue, that's fine. Often these topics turn into people repetitively saying "I want this feature too", that is what we are trying to avoid

mmomjian avatar Jan 10 '25 19:01 mmomjian

I propose that we use the discussions area to discuss a specific feature. If you start a discussion, mention it in the discussion and ask @mmomjian to update the top comment so that it links to the discussion.

The moment, two or more features are discussed in this single thread, it's going to get chaotic.

ckuyehar avatar Jan 10 '25 19:01 ckuyehar

It is already know that these features do not work well, so we should avoid rehashing "it's broken" over and over again. I would propose instead to keep this thread focussed on very specific discussion of why things are not working (eg referencing specific code paths and such) that might result in a PR. That way things should not get chaotic, and there should be no need for more separate issues or discussions (which is chaotic in its own way).

bo0tzz avatar Jan 10 '25 20:01 bo0tzz

As per https://github.com/immich-app/immich/issues/14845#issuecomment-2584007919, "app crashes" should be added to the list of limitations. Happens on Android whenever I navigate to a video, e.g. by clicking on it in the timeline or when swiping through it. I don't even need to click the "play" button to provoke the crash.

rovo89 avatar Jan 10 '25 20:01 rovo89

As far as analysis is concerned, I'm copying my findings from https://github.com/immich-app/immich/issues/5553#issuecomment-2582523158 which is closed meanwhile. My setup uses mTLS (client certificates) and a Let's Encrypt server certificate, I'm using Android.


A major problem seems to be the many levels of abstraction and different packages used. Flutter (using Dart), Jetpack, Android framework... that make it hard to follow the call chain and pass the required information down to the places where connections are actually used.

For most parts of the app, Immich uses HttpSSLCertOverride to set the client certificate and allow self-signed certificates (for the Immich server host only), which is set globally here. But I think that only applies to places where Dart's standard HttpClient class is used. Under the hood, I think it makes adjustments to OpenSSL's SSLCertContext which is wrapped by Dart's SecurityContext. My understanding is that the native HTTP client isn't involved at all here, but OpenSSL is used directly.

The video player seems to use the Android native HTTP client which is created here. DefaultHttpDataSource comes from here and uses HttpURLConnection. IIUC, Android uses a customized OpenJDK with their own handlers, in this case com.android.okhttp.HttpsHandler. This seems to be the place where they set SSL options, which uses HttpsURLConnection.getDefaultSSLSocketFactory(). I haven't tracked it down completely yet, but I think SSLContext.init() is where a client certificate would be specified.

I have just written this down to understand the current flow and maybe get corrections from someone. Next step would be finding a way to pass the client certificate to the appropriate place without breaking abstraction... Maybe it would be sufficient to set call HttpsURLConnection.setDefaultSSLSocketFactory() in Immich code?

For completeness, the download library has some code to accept untrusted certificates which calls exactly that method with an accept-all trust manager, indicating that it could profit from this approach as well.

rovo89 avatar Jan 10 '25 20:01 rovo89

Custom HTTP proxy headers

  • I think this issue refers to using the "custom proxy headers" in the Immich mobile app.
  • I use this feature and I have no issue with this.

Can someone refer an issue related to this one?

ckuyehar avatar Jan 10 '25 21:01 ckuyehar

Custom HTTP proxy headers

  • I think this issue refers to using the "custom proxy headers" in the Immich mobile app.

  • I use this feature and I have no issue with this.

Can someone refer an issue related to this one?

It's possible there's no known issues. However we still consider it an experimental feature. Glad it's working well!

mmomjian avatar Jan 10 '25 21:01 mmomjian

As for the crash in the video player, here is the trace with a debug build:

E/ExoPlayerImplInternal(10356): Playback error
E/ExoPlayerImplInternal(10356):   androidx.media3.exoplayer.ExoPlaybackException: Source error
E/ExoPlayerImplInternal(10356):       at androidx.media3.exoplayer.ExoPlayerImplInternal.handleIoException(ExoPlayerImplInternal.java:737)
E/ExoPlayerImplInternal(10356):       at androidx.media3.exoplayer.ExoPlayerImplInternal.handleMessage(ExoPlayerImplInternal.java:709)
E/ExoPlayerImplInternal(10356):       at android.os.Handler.dispatchMessage(Handler.java:105)
E/ExoPlayerImplInternal(10356):       at android.os.Looper.loopOnce(Looper.java:232)
E/ExoPlayerImplInternal(10356):       at android.os.Looper.loop(Looper.java:317)
E/ExoPlayerImplInternal(10356):       at android.os.HandlerThread.run(HandlerThread.java:85)
E/ExoPlayerImplInternal(10356):   Caused by: androidx.media3.datasource.HttpDataSource$InvalidResponseCodeException: Response code: 400
E/ExoPlayerImplInternal(10356):       at androidx.media3.datasource.DefaultHttpDataSource.open(DefaultHttpDataSource.java:401)
E/ExoPlayerImplInternal(10356):       at androidx.media3.datasource.StatsDataSource.open(StatsDataSource.java:87)
E/ExoPlayerImplInternal(10356):       at androidx.media3.exoplayer.source.ProgressiveMediaPeriod$ExtractingLoadable.load(ProgressiveMediaPeriod.java:1085)
E/ExoPlayerImplInternal(10356):       at androidx.media3.exoplayer.upstream.Loader$LoadTask.run(Loader.java:450)
E/ExoPlayerImplInternal(10356):       at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
E/ExoPlayerImplInternal(10356):       at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:644)
E/ExoPlayerImplInternal(10356):       at java.lang.Thread.run(Thread.java:1012)
D/AndroidRuntime(10356): Shutting down VM
E/AndroidRuntime(10356): FATAL EXCEPTION: main
E/AndroidRuntime(10356): Process: app.alextran.immich.debug, PID: 10356
E/AndroidRuntime(10356): java.lang.ClassCastException: androidx.media3.datasource.HttpDataSource$InvalidResponseCodeException cannot be cast to java.lang.Error
E/AndroidRuntime(10356): 	at me.albemala.native_video_player.NativeVideoPlayerViewController.onPlayerError(NativeVideoPlayerViewController.kt:137)
E/AndroidRuntime(10356): 	at androidx.media3.exoplayer.ExoPlayerImpl.lambda$updatePlaybackInfo$16(ExoPlayerImpl.java:2111)
E/AndroidRuntime(10356): 	at androidx.media3.exoplayer.ExoPlayerImpl$$ExternalSyntheticLambda27.invoke(D8$$SyntheticClass:0)
E/AndroidRuntime(10356): 	at androidx.media3.common.util.ListenerSet$ListenerHolder.invoke(ListenerSet.java:342)
E/AndroidRuntime(10356): 	at androidx.media3.common.util.ListenerSet.lambda$queueEvent$0(ListenerSet.java:226)
E/AndroidRuntime(10356): 	at androidx.media3.common.util.ListenerSet$$ExternalSyntheticLambda1.run(D8$$SyntheticClass:0)
E/AndroidRuntime(10356): 	at androidx.media3.common.util.ListenerSet.flushEvents(ListenerSet.java:248)
E/AndroidRuntime(10356): 	at androidx.media3.exoplayer.ExoPlayerImpl.updatePlaybackInfo(ExoPlayerImpl.java:2174)
E/AndroidRuntime(10356): 	at androidx.media3.exoplayer.ExoPlayerImpl.handlePlaybackInfo(ExoPlayerImpl.java:2008)
E/AndroidRuntime(10356): 	at androidx.media3.exoplayer.ExoPlayerImpl.lambda$new$1$androidx-media3-exoplayer-ExoPlayerImpl(ExoPlayerImpl.java:348)
E/AndroidRuntime(10356): 	at androidx.media3.exoplayer.ExoPlayerImpl$$ExternalSyntheticLambda10.run(D8$$SyntheticClass:0)
E/AndroidRuntime(10356): 	at android.os.Handler.handleCallback(Handler.java:991)
E/AndroidRuntime(10356): 	at android.os.Handler.dispatchMessage(Handler.java:102)
E/AndroidRuntime(10356): 	at android.os.Looper.loopOnce(Looper.java:232)
E/AndroidRuntime(10356): 	at android.os.Looper.loop(Looper.java:317)
E/AndroidRuntime(10356): 	at android.app.ActivityThread.main(ActivityThread.java:8787)
E/AndroidRuntime(10356): 	at java.lang.reflect.Method.invoke(Native Method)
E/AndroidRuntime(10356): 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:591)
E/AndroidRuntime(10356): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:871)
I/Process (10356): Sending signal. PID: 10356 SIG: 9

So it crashes here because HttpDataSource.InvalidResponseCodeException is a java.lang.Exception, not a java.lang.Error. The common super-class for both is java.lang.Throwable, and indeed changing the cast and the onError() parameter to Throwable solves the crash. The throbber (animated circle spinning while loading) is simply shown indefinitely and I can navigate to other images.

I'll send a PR for that to ~~both upstream and~~ the Immich fork. EDIT: It's only in the code that was added to move to Exoplayer.

rovo89 avatar Jan 10 '25 21:01 rovo89

For completeness, the download library has some code to accept untrusted certificates which calls exactly that method with an accept-all trust manager, indicating that it could profit from this approach as well.

The background_downloader does NOT allow self-signed certificates in release mode. I previously mentioned this in #15188.

However... even if you were to trust your CA and import your client certificate in your mobile device, the background_downloader will still fail. Why? The package background_downloader v8.9.0 doesn't support mutual TLS.

ckuyehar avatar Jan 10 '25 21:01 ckuyehar

The background_downloader does NOT allow self-signed certificates in release mode. I previously mentioned this in #15188.

I wasn't planning to use their debug feature. I mentioned it because it shows that their implementation seems to consider whatever is dictated by HttpsURLConnection.setDefaultSSLSocketFactory(). Calling the latter from Immich code should therefore have the same effect, and I have hopes that this approach would also allow to pass client certificates. See KeyManager in SSLContext.init().

rovo89 avatar Jan 10 '25 21:01 rovo89

Yeah, my proof of concept for Android is working. 🥳 I added the following in ImmichApp.kt:

val keyStore = KeyStore.getInstance("PKCS12")
val clientCertificateContent = getAssets().open("mykey.pfx")
keyStore.load(clientCertificateContent, "mypassword".toCharArray())

val keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm())
keyManagerFactory.init(keyStore, null)

val sslContext = SSLContext.getInstance("TLS")
sslContext.init(keyManagerFactory.keyManagers, null, SecureRandom())
HttpsURLConnection.setDefaultSSLSocketFactory(sslContext.socketFactory)

I successfully tested videos and image downloads. That's for mTLS only. I see no problem accepting any server certificate, the question is whether that would need to be restricted to only the Immich server host like here. But maybe that could be done by checking the hostnames in the certificate... not sure.

Any comments on whether such an approach would be acceptable? Of course, the imported key would need to be used instead of assets (was just easier for testing) and it would have to be reloaded when the key is changed. Also no idea how I can create a dedicate native "component" for this stuff, I might need some assistance with that once the general idea has been confirmed. And sorry, no idea about iOS.

rovo89 avatar Jan 11 '25 00:01 rovo89

I successfully tested videos and image downloads.

🥳 congrats! awesome job.

I see no problem accepting any server certificate, the question is whether that would need to be restricted to only the Immich server host like here.

Immich should get out of the business of making that decision and defer that to Android/iOS. Let the OS perform the necessary checks.

Any comments on whether such an approach would be acceptable?

I do have some thoughts... Instead of maintaining/creating another certificate store within the Immich mobile app (abbrev IMA)... IMA should select the certificate from the OS. This will ensure we're using the OS certificate stores (trusted CAs and user) instead of a custom store within IMA. This will also allow IMA (when the time is right) to depreciate "allow self-signed certificates" because the CA would be trusted on the mobile device and a "self-signed" error would never appear.

In other words you should be able to trust the CA, uncheck "allow self-signed certificates" in IMA, select your client certificate in the Android User Store and login to IMA, download images and play videos without issue.

ckuyehar avatar Jan 11 '25 00:01 ckuyehar

cc @etnoy if you have any thoughts on this

mertalev avatar Jan 11 '25 00:01 mertalev

Immich should get out of the business of making that decision and defer that to Android/iOS. Let the OS perform the necessary checks.

Flutter has its own certificate handling and doesn't use the system's. That's the whole reason we're here in the first place.

bo0tzz avatar Jan 11 '25 02:01 bo0tzz

Exactly. It's neither my idea nor my preferred approach to upload the certificate in the Immich app, I'm just following the path that was already taken.

rovo89 avatar Jan 11 '25 07:01 rovo89

https://github.com/dart-lang/sdk/issues/50435 sounds like no real solution is planned for the HTTP client in Dart, they suggest using package:cronet_http. No idea about that, I'm not familiar with Dart. Indeed it feels a bit clunky having to upload the certificates again that I already configured in the OS, but it's a one-time thing, so it's acceptable for me as a user with a special setup.

I also thought about adding the self-signed certificate to the trust store, which I would anyway do in the OS, but using it from there has the same limitations as above and I'm not sure if yet another upload button in Immich would be worth it. So the simple switch sounds like an acceptable compromise to me.

rovo89 avatar Jan 11 '25 07:01 rovo89

Right now, the system looks like so overloaded with all of it security stuff, it's no wonder something breaks. I'll join the commentators above, all apps on device MUST trust system CA, it just built for it. Some strange decisions like SSL Pinning can be acceptable in banking or payment apps, but not in userspace.

LordArrin avatar Jan 29 '25 02:01 LordArrin

And yep, I use Immich behind nginx with my personal CA's, and literally all works amasing, but not a video playback and downloading images from Immich on Android. From desktop browsers, android browsers all of it works like charm.

LordArrin avatar Jan 29 '25 02:01 LordArrin

While I generally agree that apps should use the system SSL stack, the reason why this is not the case is in the platform abstraction of Dart, not in the immich app itself. As document above, Dart doesn't use the Android HTTPS client, but rolls its own based on OpenSSL directly.

I don't think switching away from Dart is an option. 😉 There are some ideas to add support for private keys from the Android keystore (which can't be accessed as bytes, the system will take care of encryption): https://github.com/dart-lang/http/issues/1237. Or it has been mentioned in https://github.com/dart-lang/sdk/issues/50435 to use package:cronet_http instead.

But all of these options are either just ideas/wishes without any commitment and timeline, or they would have a bigger impact on the app itself. As it has been made clear that use-cases such as mTLS have low priority, I can't imagine that we'll see such changes anytime soon.

On the other hand, the Immich app does already have options to upload the client certificate. Yes, it's redundant, but for me as a user/admin, that's much better than other workarounds I'd have to implement in the current state (e.g. add another domain without mTLS), or not being able to watch remote videos at all.

The approach I mentioned in https://github.com/immich-app/immich/issues/15230#issuecomment-2584938841 is fairly low impact, not much code (= less to maintain) and doesn't add any further user-facing workarounds. It also matches the semantics of the existing codes to globally enable the client certificate - just for a different HTTP client.

That's why I'd still like to work on implementing this properly, and I hope to get some agreement from the core devs that such a PR would be merged once it's ready. Can I get that?

rovo89 avatar Jan 29 '25 09:01 rovo89

Find a temporary solution. I enabled LSPosed module SSLUnpinning for Immich, relaunch app and all videos plays like it may do.

LordArrin avatar Jan 29 '25 18:01 LordArrin

Interesting to see that variants of Xposed are still around, I'm the original author 😉 But nowadays I don't even root my phone anymore, which is the prerequisite.

Besides that, this method might help to disable SSL validation globally (which is a very big hammer), but I don't see how it would make mTLS possible. The private key of the client certificate should be stored in separate hardware, unaccessible even for the Android core, and encryption takes place via IPC as far as I understood.

rovo89 avatar Jan 29 '25 19:01 rovo89

Interesting to see that variants of Xposed are still around, I'm the original author 😉 But nowadays I don't even root my phone anymore, which is the prerequisite.

Besides that, this method might help to disable SSL validation globally (which is a very big hammer), but I don't see how it would make mTLS possible. The private key of the client certificate should be stored in separate hardware, unaccessible even for the Android core, and encryption takes place via IPC as far as I understood.

I have nothing to contribute to this issue, but I just want to thank you for Xposed. It's been a few years but I remember those wild days fondly :)

etnoy avatar Jan 29 '25 19:01 etnoy

@rovo89

I'd have to implement in the current state (e.g. add another domain without mTLS)

Having multiple domains for the app might not work as in that case you'd need to work around the authentication (at least for the web version, it uses cookies, which aren't easily shared between domains).

After some trial and error, I solved it on my setup using HAProxy (it is most likely possible with other reverse proxies, but I needed something with low CPU/RAM consumption).

Here I first define verify optional which will validate the client certificate only if it is available. Later in the code it checks the URL path, and lets the request through without the client cert only for the video playback endpoints. Basically, mTLS is required on everything else, except the problematic video playback endpoint.

haproxy.cfg:

    global
      log stdout format raw local0

    defaults
      mode http

    frontend https-in
      # tls-certs should contain two files tls.crt, tls.crt.key (issued by trusted authority)
      # ca.crt is a self-signed CA for mTLS
      bind *:8443 ssl crt /etc/haproxy/tls-certs/ ca-file /etc/haproxy/mtls-ca/ca.crt verify optional

      acl public_video_playback path_reg ^/api/assets/[a-f0-9\-]+/video/playback
      acl client_has_cert ssl_c_used

      # Require mTLS for everything EXCEPT video playback
      http-request deny if !client_has_cert !public_video_playback

      use_backend backend_servers

    backend backend_servers
      server immich-server 127.0.0.1:2283 check # this might need to be adjusted based on setup

This is slightly not ideal, but if there is no proper fix, allows us to keep using mTLS setup and all is behind one domain / ip. IMHO mutual TLS is a robust, secure, and reliable way for people to expose their services to the internet and should be a recommended approach. Much better then compared to relying on VPNs like Tailscale (as then it can't even be considered as pure self-hosting anymore :) ).

polomani avatar Feb 05 '25 20:02 polomani

I drafted PR #16403 to support remote video playback and asset downloads with mTLS. Android-only, not sure if this problem even exists on iOS. It shouldn't be too hard to add support for self-signed certificates, but I want to get some feedback first.

rovo89 avatar Feb 27 '25 22:02 rovo89

I drafted PR #16403 to support remote video playback and asset downloads with mTLS. Android-only, not sure if this problem even exists on iOS. It shouldn't be too hard to add support for self-signed certificates, but I want to get some feedback first.

@rovo89 I can confirm that the problem also affects iOS.

pedropombeiro avatar Feb 27 '25 22:02 pedropombeiro

I drafted PR #16403 to support remote video playback and asset downloads with mTLS. Android-only, not sure if this problem even exists on iOS. It shouldn't be too hard to add support for self-signed certificates, but I want to get some feedback first.

The changes required for cronet_http should be rather simple. We should test it a lot to make sure it is stable and that nothing breaks. However, AFAIK, using it will solve the issue only with the network calls made from dart but not the one made from the native side like the video player or the background downloads.

shenlong-tanwen avatar Mar 03 '25 12:03 shenlong-tanwen

However, AFAIK, using it will solve the issue only with the network calls made from dart but not the one made from the native side like the video player or the background downloads.

Now that you say it, that makes sense... In other words, using cronet_http might make the current workarounds more elegant, but it won't solve the open issues.

"More elegant" means that self-signed certificates could be imported into the Android trust store and would be automatically accepted. Client certificates could be imported into the Android keystore, however an app must bring up the dialog to choose the key and grant permission to it. So some special code would still be required, and I'm not sure whether all required interfaces are exposed to Dart. I assume that additional code would be required for the video player and downloads.

I have no idea which timeline would be realistic for a switch - if it happens at all. So from my point of view, it make sense to move forward with the PR. I'm currently trying to enhance it for self-signed certificates. The main difficulty is to accept them only for the Immich host...

rovo89 avatar Mar 03 '25 21:03 rovo89

On the other hand, the Immich app does already have options to upload the client certificate.

This isn't really an acceptable solution, it needs to allow CAs, rotating your client certs regularly is a basic and common best practice and Caddy (and others) do it for free. I've had to "Allow self-signed SSL certificates" which is not ideal to even connect to the server, it's strange that even with that though you still can't download remote assets, it should just be ignoring it.

Trezamere avatar Mar 09 '25 20:03 Trezamere

rotating your client certs regularly is a basic and common best practice and Caddy (and others) do it for free

Are you sure you're talking about client certificates here? Everything in your comment sounds like you mean server certificates instead. The former is for authentication, proofing that you are a certain user (comparable to a password), whereas the latter is how the server proofs that it's the one you wanted to talk to and not some man-in-the-middle which intercepted your traffic.

The import button is only meant for client certificates. If you upload a server certificate instead, you might not get an error because the format is the same. But it won't help anything because the certificate will only be used when the server asks for it, and it won't be used to check the server's authenticity. The "Allow self-signed SSL certificates" on the other hand is only for server certificates, because obviously there can't be a toggle in the app to tell the server to trust your client certificate.

As you mention best practices - the much better way would be getting a domain and using Let's Encrypt to get a valid, trusted server certificate. That can even work for installations in your home network because you can proof ownership via DNS, in which case you can also get a wildcard certificate for your whole domain (*.example.org).

it's strange that even with that though you still can't download remote assets, it should just be ignoring it.

Exactly that issue would be solved on Android with #16403, for both client certificates and self-signed server certificates.

rovo89 avatar Mar 09 '25 21:03 rovo89