Make TorSettings interact with the old Android Settings

added Next label

Settings are managed through our TorSettings class.

In general, it exposes them through a JS object whose schema can be seen in defaultSettings(), and it has a setSettings() method that allows to pass such an object to store it, but not to save them to disk! They're saved only when we call saveToPrefs.

The rationale is that we can set whatever we receive from Moat, try to bootstrap, and save only if they work.

The easiest approach here for plumbing is passing up and down that object, and marshal it on the Java side (either on GeckoView, or in Android Components). We could rename the saveToPrefs to just save, and when it's Android, we emit an event to be caught on the Java side. Mozilla usually implements event catching on GeckoView, and then plumbs them down through delegates, I think we can do something similar.

Also, we call setSettings only in the connection assist, so we should be fine, but we also have setters... I'd like to have an API that works as one'd expect, so we'd have to somehow signal Android that $something changed the settings through setters, even though it shouldn't happen normally.

There's a big problem here: what should we do with settings that haven't been saved yet (e.g., a user goes to settings during a long bootstrap with settings set by the connection assist)? Should we show them in Android UI anyway? At the moment, going to the settings on Android cancels the bootstrap (!). Should we keep this UX and restore the previous settings and show them?

Second problem (smaller): the quick bootstrap sets a pref when the connection failed to prevent it from happening again at the next app start. Is a pref good here (I think so), or should we plumb it?

mentioned in merge request !852 (merged)

carrying over from !852 (merged):

firefox-android uses in code TorController which uses tor-android-service Prefs class with all custom prefs.
The prefs are not tied to the library in any way I can see other than it's currently the canonical place to get their names so we'll have to copy all those into our code, prolly just copy the whole Prefs.java file into our tor module, convert it to kotlin and go from there. It has no other deps.

how are you envisioning using them with Geckoview? My first idea was a one time migrator that slurps them all up and sends them to GV, and then we can ignore them?

This was a question I had for you (and possibly also @clairehurst) .

Should we keep prefs in Android storage, until possible? From what I understand, Moz is doing the same, but maybe the reason is that they try to stay generic wrt the engine they use.

it seems a pain to me to duplicate all that. and maintain wiring for it? I'd love to do away with it. But I suspect as I start trying to wiring sending them to GV today I'll get a better view of how much of a PITA it is or not. I'll be writing some code to harvest and use them either way

So, I'd definitely avoid using the generalization they used. I think the assumption we're using GeckoView is going to stay valid for a very long time. We don't care

As I wrote somewhere, we use the EventDispatcher for plumbing... But what I didn't write is that there might be more than one EventDispatcher (more details).

If it's the case, for the settings we should use the one of GeckoRuntime, which is a sort of singleton. (I'm not sure the various GeckoSessions have other EventDispatchers, which are good for other stuff, such as the circuit display, even though eventually the API is the same).

What I'd do, is create a Java version of the settings object we have in TorSettings.sys.mjs. You can get see the general schema in the defaultSettings() function.

I don't think we need to create nested objects, just a representation, with various getters and setters for the various properties.

Then, it could have constructor from a GeckoBundle and a function to export to a GeckoBundle. We could use that for wiring.

For notifying changes, in TorSettings we already notify the TorSettingsTopics.SettingChanged topic, but only for quickstart.

I think we should do it for all the various settings.

Also, we could specify an empty string as a third parameter when we do setSettings as a special case, and the JS object we broadcast could be just a full settings object.

Service.obs is another way of broadcasting events. So, one thing we could do is not to call it directly, but to write a helper function in TorSettings, and when we call it on Android, we also broadcast an event on the EventDispatcher, in addition to Service.obs. EventDispatcher isn't available on desktop, so we have to check whether it's available (but since it can be a lazy import, this shouldn't end up in warnings).

Otherwise, we should write a bridge of Service.obs and EventDispatcher, where we could do the wiring of everything in a single central place. Also other pieces of the Tor integration use Service.obs. For example, TorConnect uses them, and we'll need to wire them to plumb the events for the native UX of the connection.

/cc @richard

I think we'd need to plumb up and down at least these things:

TorSettings.loadFromPrefs/TorSettings.saveToPrefs if we intend using GV's prefs as storage (or rename them in load and save, and implement them differently when we're on desktop/Android; the current method could become private methods in case).
TorSettings.getSettings/TorSettings.setSettings/TorSettings.applySettings for the settings activity (saveToPrefs doesn't send settings down to tor, applySettings does it)

They're all Java -> JS -> Java events, so you should take what I did for the circuit display as an example (with the only exception that the circuit display uses a GeckoSession EventDispatcher, instead of using the generic one).

If we take for granted that while bootstrapping you can't change settings and we use preferences, we don't need to implement JS -> Java -> JS events (which are simpler ).

The one-time migration could use the normal saveSettings method.

In that case we don't need this:

I think we should do it for all the various settings.

Also, we could specify an empty string as a third parameter when we do setSettings as a special case, and the JS object we broadcast could be just a full settings object.

By "keeping prefs in android storage", do you mean SharedPreferences? If so, I'd say it's fine

So further fun, in tor-android-service in CustomTorInstaller it seems their bridge storing is super custom

https://gitlab.torproject.org/tpo/applications/tor-android-service/-/blob/main/service/src/main/java/org/torproject/android/service/CustomTorInstaller.java?ref_type=heads#L41

they store either the bridge line OR the bridge type name in the pref, and if it is a bridge line, they load it with context.getResources().getAssets().open("common/bridges.txt");

which means only Java and only java with access to context can load that. which means my first hope of adding in an android pref loader in TorSettings in GV is... in trouble, again, unless we want a 1 time migrate, which, yeah, that's some gnarly code, I'm all for converting those settings to tor-browser style

By "keeping prefs in android storage", do you mean SharedPreferences? If so, I'd say it's fine

Yes, it seems to be that thing .

Do you have some suggestions on why we should use them instead of GV prefs? GV prefs are stored in the profile, which is stored in app data.

I don't know if shared prefs have some "advantages" such as being sync'ed to your Google account for backup.

In that case, it might be even something we don't want, because bridges are privileged nodes, in a certain sense (just like guards).

only java with access to context can load that.

That actually isn't a problem for GV . There's a nice function that gives you the (last? only?) context that someone passed to GV:

GeckoAppShell.getApplicationContext();

It even seems context will never change (can you confirm?), so it should be fine to just call that function when we need a context on GV.

To be on the safe side, I tried use the context passed as parameter in !852 (merged) wherever possible, but on a method I also get the context from GeckoAppShell, and it worked.

unless we want a 1 time migrate, which, yeah, that's some gnarly code, I'm all for converting those settings to tor-browser style

FWIW, I think we can do a hybrid.

If we think the current format of shared prefs sucks, but we want to keep using them for $reasons, nothing prevents us from doing a migration once, but to other shared prefs, rather than GV prefs.

Or, in other words, I think that "migrating old prefs" and "choosing a backend for new prefs" are independent chats to a certain extent.

.open("common/bridges.txt");

So, about this...

Some time ago, we introduced a pt_config.json file with some bridge information.

That file can't be directly consumed, so we transform it into that bridges.txt for Android and in preferences we append to 000-tor-browser.js for desktop.

I think we don't have to keep doing that. Gecko provides all the JS we need to parse JSON files.

So, instead of doing all of that and reading bridges from prefs (which is something we'll also need to do on Android, or the connect assist will not work), we could change TorSettings to consume the JSON directly.

We could put the JSON in omni.ja. We do something similar in connectionPane.js already for emoji descriptions, but it's all in content. TorSettings isn't in-content, and it might need another strategy (but maybe it won't). But I'm not too worried, I'm quite sure we can do it.

I can probably draft something today, and let you find it ready to use.

Otherwise, we should write a bridge of Service.obs and EventDispatcher, where we could do the wiring of everything in a single central place.

I'm thinking this could be a good approach, because we could also use it for all the various Java -> JS -> Java "RPC", not only for the JS -> Java -> JS (or JS -> Java, the last part is returning a result, and is easy to do).

The alternative is to add EventDispatcher here and there where we need it. It might not be a clean approach, after all.

I've done it for MeekTransportAndroid and TorProcessAndroid, but they're very Android specific. For stuff not Android-centric it might make sense to do that, instead.

We could start it from mobile/android/components/geckoview/GeckoViewStartup.jsm, where we initialize the other Tor stuff for Android.

Yeah it seems that SharedPreferences are backed up to google https://developer.android.com/guide/topics/data/autobackup

"Android preserves app data by uploading it to the user's Google Drive, where it's protected by the user's Google account credentials"

But we can also disable backup

https://developer.android.com/guide/topics/data/autobackup#EnablingAutoBackup

<manifest ... >
...
<application android:allowBackup="false" ... >
...
</application>
</manifest>

We should probably verify that its actually disabled though with a network analyzer to be sure though (wireshark or similiar)

I see, thanks!

TBH, now I wonder if the profile is backed up without us knowing.

In firefox-android/fenix we do already have android:allowBackup="false". It seems that data_extraction_rules.xml is the preferred way now. It allows for more granular levels of including or excluding data. We probably want to just exclude everything right?. https://developer.android.com/about/versions/12/behavior-changes-12#backup-restore

I'd expect so. As usual, something we should check in a threat model .

Re: Can Context change? The application context that you're calling should exist (and it's reference should stay the same) throughout the lifecycle of the application. The global information that goes with it may change though.

Note that there are different types (technically different sub classes) of Context (Application, Service, and Activity are the main ones) with different life cycles tracking different things.

We could try to trust google infra to respect android:allowBackup="false" but yeah, my gut would be not to use that part of android at all and keep it in moz prefs, where google can't later change its mind. I get that tor-android-service, being built in a more app agnostic way doesn't have that option, but we do, and I vote we move off it

Note that there are different types (technically different sub classes) of Context (Application, Service, and Activity are the main ones) with different life cycles tracking different things.

We mainly need to access app directories (data dir, cache dir, library directory) and assets.

The application context is what you want then

mentioned in issue #42323 (closed)

assigned to @dan

mentioned in merge request !875 (merged)

removed Next label

closed

marked this issue as related to tor-browser-build#41021 (closed)

mentioned in merge request !879 (merged)

mentioned in merge request !881 (merged)

Make TorSettings interact with the old Android Settings

Designs

Child items ...

Activity