Skip to content

Commit

Permalink
Add support for automatically reloading CA certificates
Browse files Browse the repository at this point in the history
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 the rclone's cache when deleting a remote.

Signed-off-by: Andrew Gunnerson <accounts+github@chiller3.com>
  • Loading branch information
chenxiaolong committed Jan 16, 2025
1 parent 46c8cd4 commit b90b12f
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 55 deletions.
4 changes: 4 additions & 0 deletions UPDATES.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ Hopefully, most issues can be found just by trying to compile rcbridge with `./g

If librclone gained new functionality that can replace current uses of internal APIs, then that new functionality should be used. librclone RPC-related tasks should be done purely on the Android side in [`RcloneRpc`](./app/src/main/java/com/chiller3/rsaf/rclone/RcloneRpc.kt), not in go.

### Certificate reloading

Check if there's any way hook into `fshttp.(*Transport).RoundTrip()`. The hook to update `tls.Config.RootCAs` is the only reason we need to fork rclone.

### `RbDocMkdir`

Check the `vfs.Dir.Mkdir()` implementation to see if it fails with EEXIST when the path already exists. If so, `RcloneProvider` can be updated to avoid an unnecessary stat when creating directories with Android semantics.
Expand Down
6 changes: 3 additions & 3 deletions app/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
<string name="pref_edit_remote_configure_name">Configure remote</string>
<string name="pref_edit_remote_configure_desc">Rerun the rclone configuration wizard.</string>
<string name="pref_edit_remote_rename_name">Rename remote</string>
<string name="pref_edit_remote_rename_desc">Change the name of the remote. If other remotes depends on this one, they will need to be manually updated with the new name.</string>
<string name="pref_edit_remote_rename_desc">Change the name of this remote. If other remotes depends on this one, they will need to be manually updated with the new name.</string>
<string name="pref_edit_remote_duplicate_name">Duplicate remote</string>
<string name="pref_edit_remote_duplicate_desc">Create a copy of this remote with identical configuration.</string>
<string name="pref_edit_remote_delete_name">Delete remote</string>
Expand Down Expand Up @@ -93,7 +93,7 @@

<!-- Edit remote alerts -->
<string name="alert_edit_remote_success">Successfully edited remote %1$s</string>
<string name="alert_delete_remote_failure">Failed to delete %1$s: %2$s</string>
<string name="alert_delete_remote_failure">Failed to delete remote %1$s: %2$s</string>
<string name="alert_rename_remote_failure">Failed to rename remote %1$s to %2$s: %3$s</string>
<string name="alert_duplicate_remote_failure">Failed to duplicate remote %1$s to %2$s: %3$s</string>
<string name="alert_set_config_failure">Failed to set %1$s config option for remote %2$s: %3$s</string>
Expand Down Expand Up @@ -127,7 +127,7 @@
<string name="dialog_export_password_hint">Encryption password</string>
<string name="dialog_inactivity_timeout_title">Inactivity timeout</string>
<string name="dialog_inactivity_timeout_message">Enter a duration in seconds.</string>
<string name="dialog_vfs_cache_deletion_message">There are file operations in progress. These operations will be interrupted and pending uploads in the VFS cache will be permanently deleted.</string>
<string name="dialog_vfs_cache_deletion_message">There are pending uploads in progress that may be interrupted. These will be permanently deleted from the VFS cache.</string>

<!-- Interactive configuration -->
<string name="ic_title_add_remote">Add remote: %1$s</string>
Expand Down
2 changes: 2 additions & 0 deletions rcbridge/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ require (
golang.org/x/mobile v0.0.0-20250106192035-c31d5b91ecc3
)

replace github.com/rclone/rclone v1.69.0 => github.com/chenxiaolong/rclone v1.69.0-rsaf

require (
cloud.google.com/go/auth v0.12.1 // indirect
cloud.google.com/go/auth/oauth2adapt v0.2.6 // indirect
Expand Down
4 changes: 2 additions & 2 deletions rcbridge/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ github.com/calebcase/tmpfile v1.0.3/go.mod h1:UAUc01aHeC+pudPagY/lWvt2qS9ZO5Zzof
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/chenxiaolong/rclone v1.69.0-rsaf h1:URDbtwBv0+aKN2k7MkLCl1wxyxxKVSNy+it1t8JqL7A=
github.com/chenxiaolong/rclone v1.69.0-rsaf/go.mod h1:RfT8WA1rU1/wHyujQ1r0MZFdO89zLSDgCuu62uImArg=
github.com/chilts/sid v0.0.0-20190607042430-660e94789ec9 h1:z0uK8UQqjMVYzvk4tiiu3obv2B44+XBsvgEJREQfnO8=
github.com/chilts/sid v0.0.0-20190607042430-660e94789ec9/go.mod h1:Jl2neWsQaDanWORdqZ4emBl50J4/aRBBS4FyyG9/PFo=
github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
Expand Down Expand Up @@ -467,8 +469,6 @@ github.com/quic-go/qtls-go1-20 v0.4.1 h1:D33340mCNDAIKBqXuAvexTNMUByrYmFYVfKfDN5
github.com/quic-go/qtls-go1-20 v0.4.1/go.mod h1:X9Nh97ZL80Z+bX/gUXMbipO6OxdiDi58b/fMC9mAL+k=
github.com/quic-go/quic-go v0.40.1 h1:X3AGzUNFs0jVuO3esAGnTfvdgvL4fq655WaOi1snv1Q=
github.com/quic-go/quic-go v0.40.1/go.mod h1:PeN7kuVJ4xZbxSv/4OX6S1USOX8MJvydwpTx31vx60c=
github.com/rclone/rclone v1.69.0 h1:fRfYu6Ha2Zn+nDLUa4ZuPXogddfeZG+jsCyhbOdAamw=
github.com/rclone/rclone v1.69.0/go.mod h1:RfT8WA1rU1/wHyujQ1r0MZFdO89zLSDgCuu62uImArg=
github.com/redis/go-redis/v9 v9.6.1 h1:HHDteefn6ZkTtY5fGUE8tj8uy85AHk6zP7CpzIAM0y4=
github.com/redis/go-redis/v9 v9.6.1/go.mod h1:0C0c6ycQsdpVNQpxb1njEQIqkx5UcsM8FJCQLgE9+RA=
github.com/relvacode/iso8601 v1.3.0 h1:HguUjsGpIMh/zsTczGN3DVJFxTU/GX+MMmzcKoMO7ko=
Expand Down
77 changes: 27 additions & 50 deletions rcbridge/rcbridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
// This package's init() MUST run first
_ "rcbridge/envhack"

"crypto/tls"
"crypto/x509"
"encoding/pem"
"fmt"
Expand All @@ -35,7 +36,6 @@ import (
"time"

_ "golang.org/x/mobile/event/key"
"golang.org/x/sys/unix"

_ "github.com/rclone/rclone/backend/all"
"github.com/rclone/rclone/fs"
Expand Down Expand Up @@ -89,7 +89,7 @@ var (
config.ErrorConfigFileNotFound: syscall.ENOENT,
}
caCertsLock goSync.Mutex
caCertsFile *os.File
caCertsPool *x509.CertPool
)

// Load as many certificates from the PEM data as possible and return the last
Expand Down Expand Up @@ -117,17 +117,17 @@ func parsePemCerts(data []byte) (certs []*x509.Certificate, err error) {
return certs, err
}

// Generate a trust store in a single file that we can pass to rclone via its
// CaCert config option. This is necessary because golang currently does not
// support reading from the proper Android directories. We can't just set
// SSL_CERT_DIR either, even with envhack, because the user CA directory
// Generate a trust store pool that we can pass to rclone via the per-request
// hook we add in our rclone fork. This is necessary because golang currently
// does not support reading from the proper Android directories. We can't just
// set SSL_CERT_DIR either, even with envhack, because the user CA directory
// contains DER-encoded certificates and golang only supports loading PEM.
//
// Additionally, our implementation will not trust any system CA certificates
// that the user explicitly disabled from Android's settings.
//
// https://github.com/golang/go/issues/71258
func generateTrustStoreTempFile(tempDir string) *os.File {
func generateTrustStorePool() *x509.CertPool {
systemDir := os.Getenv("ANDROID_ROOT")
dataDir := os.Getenv("ANDROID_DATA")

Expand Down Expand Up @@ -187,14 +187,7 @@ func generateTrustStoreTempFile(tempDir string) *os.File {
}
}

fd, err := unix.Open(tempDir, unix.O_RDWR|unix.O_TMPFILE|unix.O_CLOEXEC, 0600)
if err != nil {
fs.Errorf(nil, "Failed to create temp file in: %v: %v", tempDir, err)
return nil
}

path := fmt.Sprintf("/proc/self/fd/%d", fd)
file := os.NewFile(uintptr(fd), path)
pool := x509.NewCertPool()

for _, path := range caFiles {
data, err := os.ReadFile(path)
Expand All @@ -213,22 +206,21 @@ func generateTrustStoreTempFile(tempDir string) *os.File {
}

for _, cert := range certs {
block := &pem.Block{
Type: "CERTIFICATE",
Bytes: cert.Raw,
}

err = pem.Encode(file, block)
if err != nil {
fs.Logf(nil, "Failed to encode certificate: %v", cert)
continue
}
pool.AddCert(cert)
}
}

fs.Logf(nil, "Loaded %d certificates", len(caFiles))

return file
return pool
}

// Set the trusted CA certificates on every HTTP request.
func perRequestHook(config *tls.Config) {
caCertsLock.Lock()
defer caCertsLock.Unlock()

config.RootCAs = caCertsPool
}

// Initialize global aspects of the library.
Expand All @@ -238,34 +230,16 @@ func RbInit() {
// Don't allow interactive password prompts
ci := fs.GetConfig(context.Background())
ci.AskPassword = false

fshttp.SetRoundTripHook(perRequestHook)
}

// Reload certificates from the system and user trust stores. This is thread
// safe within RSAF, but may race with rclone's NewTransportCustom(). Note that
// reloading is not that useful because the changes do not affect fshttp clients
// that were previously created.
func RbReloadCerts() bool {
// Reload certificates from the system and user trust stores.
func RbReloadCerts() {
caCertsLock.Lock()
defer caCertsLock.Unlock()

ci := fs.GetConfig(context.Background())
ci.CaCert = nil

if caCertsFile != nil {
caCertsFile.Close()
caCertsFile = nil
}

caCertsFile = generateTrustStoreTempFile(config.GetCacheDir())
if caCertsFile == nil {
return false
}

ci.CaCert = append(ci.CaCert, caCertsFile.Name())

fshttp.ResetTransport()

return true
caCertsPool = generateTrustStorePool()
}

// Clean up library resources.
Expand Down Expand Up @@ -355,7 +329,10 @@ func RbCacheClearRemote(remote string) {
}
}

cache.ClearConfig(remote)
parsed, err := fspath.Parse(remote)
if err == nil {
cache.ClearConfig(parsed.Name)
}
}

// Clear cached fs and vfs instances. All vfs instances will be shut down
Expand Down

0 comments on commit b90b12f

Please sign in to comment.