Add support for automatically reloading CA certificates #125
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chenxiaolong
force-pushed
the
reload
branch
2 times, most recently
from
b90b12f to
f723889
Compare
The initial implementation of loading Android's CA trust stores wrote the certificates to a temporary file and then passed them to rclone via the CaCert config option. This would only get loaded when a new backend Fs instance was created. If the backend was already created, the user would need to delete and recreate the remote or force quit RSAF, which is a terrible user experience. Unfortunately, reloading by recreating the Fs and VFS instances would not provide a good experience either. The VFS would need to be shut down, interrupting VFS writeback, and we need to wait for the garbage collector to clean up the VFS instance so that the corresponding Fs instance would be unpinned from the cache. This precludes the CA reloading process from being automatic. Instead, this commit makes use of a per-request hook to set the trusted CA certificates on every connection. This requires a soft-fork of rclone to add a hook point to fshttp.(*Transport).RoundTrip(). The patch is extremely simple and is easy to port to future versions of rclone. No other patches will be added to the fork and I plan to switch back to the upstream version if another good solution is found, even if hacky. Some other approaches I tried or considered: * Replace fshttp.isCertificateExpired() using go:linkname and in the replacement, update the tls.Config.RootCAs in addition to executing a copy of the original code. This function would be called during each RoundTrip() invocation. This approach does not work with how gomobile builds rcbridge as a library and causes duplicate symbol errors during linking. * Add the code as is done in this commit, except via a -toolexec program that preprocesses the AST of the go sources. This approach does not work because -toolexec is not invoked at the necessary points when compiling with gomobile. * Patch fshttp.isCertificateExpired() at runtime by inserting a jump call to our own implementation. This is error prone, requires specific assembly code for each CPU architecture, and does not work if the function we want to patch was inlined. This commit also fixes a bug where a remote's Fs instance is not properly removed from rclone's cache when deleting the remote. Issue: #119 Signed-off-by: Andrew Gunnerson <accounts+github@chiller3.com>
chenxiaolong
added a commit
that referenced
this pull request
Jan 16, 2025
Signed-off-by: Andrew Gunnerson <accounts+github@chiller3.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The initial implementation of loading Android's CA trust stores wrote the certificates to a temporary file and then passed them to rclone via the
CaCertconfig option. This would only get loaded when a new backendFsinstance was created. If the backend was already created, the user would need to delete and recreate the remote or force quit RSAF, which is a terrible user experience.Unfortunately, reloading by recreating the
FsandVFSinstances would not provide a good experience either. The VFS would need to be shut down, interrupting VFS writeback, and we need to wait for the garbage collector to clean up theVFSinstance so that the correspondingFsinstance would be unpinned from the cache. This precludes the CA reloading process from being automatic.Instead, this commit makes use of a per-request hook to set the trusted CA certificates on every connection. This requires a soft-fork of rclone to add a hook point to
fshttp.(*Transport).RoundTrip(). The patch is extremely simple and is easy to port to future versions of rclone. No other patches will be added to the fork and I plan to switch back to the upstream version if another good solution is found, even if hacky.Some other approaches I tried or considered:
fshttp.isCertificateExpired()usinggo:linknameand in the replacement, update thetls.Config.RootCAsin addition to executing a copy of the original code. This function would be called during eachRoundTrip()invocation. This approach does not work with how gomobile builds rcbridge as a library and causes duplicate symbol errors during linking.-toolexecprogram that preprocesses the AST of the go sources. This approach does not work because-toolexecis not invoked at the necessary points when compiling with gomobile.fshttp.isCertificateExpired()at runtime by inserting a jump call to our own implementation. This is error prone, requires specific assembly code for each CPU architecture, and does not work if the function we want to patch was inlined.This commit also fixes a bug where a remote's
Fsinstance is not properly removed from rclone's cache when deleting the remote.Issue: #119