Thunderbird needs to move its search engine configuration into Remote Settings with a different bucket
Categories
(Thunderbird :: General, task)
Tracking
(thunderbird_esr78 unaffected, thunderbird81 unaffected)
Tracking | Status | |
---|---|---|
thunderbird_esr78 | --- | unaffected |
thunderbird81 | --- | unaffected |
People
(Reporter: standard8, Assigned: mkmelin)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
text/x-phabricator-request
|
Details | |
(deleted),
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
In bug 1542269 we are transitioning the new search engine configuration for Firefox from a built-in file to remote settings (this has always been planned as part of the new configuration).
I am hoping to land that patch in Firefox within the next few days, as we are then going to enable the new configuration for nightly users only (it is likely to be at least next cycle before we roll it out to more, and then a couple of cycles after that, we'll remove the legacy code).
Thunderbird already has an engines.json file.
Ideally this should be transitioned to a remote settings server (I do have a script I can share which will allow easy upload), and Thunderbird set up to load from that. Depending on the exact setup, we may need an alternate SETTINGS_KEY for Thunderbird (we do not want the Thunderbird config mixed with the Firefox one).
If we don't have any of that set up for Thunderbird already, I could possibly accept a short-term hack of loading engines.json for Thunderbird only, though it would probably break most of the search tests. In that case, I'd also appreciate knowing that remote settings set-up was being worked upon.
Reporter | ||
Comment 1•5 years ago
|
||
Not sure who should be looking at this.
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
|
||
Is it still possible to just keep the engine configuration local in Thunderbird, like having the config file being local? There doesn't seem to be a lot of upside for us to have it remote, especially since search is not that much used...
Assignee | ||
Comment 3•5 years ago
|
||
I'm sure the conversion script would come in handy.
But is it possible to change the url pointing at it to some chrome:// url?
Reporter | ||
Comment 4•5 years ago
|
||
I've taken another look at this, and discussed it with a couple of people. There's basically two choices.
The first is that we could add a couple of if statements and let Thunderbird use the existing resource://search-extensions/engines.json
url. However as there's no update mechanism, it would make keeping the some of the tests running for Thunderbird really hard, as various tests do an initial test, update the config, and then do a second test - that would be broken, and there's no easy way to work around that if you're not using remote settings.
We might want in this first instance that we disable all the search xpcshell tests for Thunderbird, and Thunderbird can do its own integration tests if you want to - this would limit the additional noise as we land tests and potentially break Thunderbird's CI tree.
The second option is basically the remote settings but in a different bucket (this is the only case you might want the upload script).
Thoughts?
Assignee | ||
Comment 5•5 years ago
|
||
In the interest of minimizing problems down the line, I think it sounds like option #2 could be preferable.
I guess what I'm asking, is it possible to have the remote settings point to a resource:// or do we actually need to host it somewhere?
Reporter | ||
Comment 6•5 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #5)
In the interest of minimizing problems down the line, I think it sounds like option #2 could be preferable.
I guess what I'm asking, is it possible to have the remote settings point to a resource:// or do we actually need to host it somewhere?
You might be able to point to a resource url, if you could get that to simulate the entire server, though I suspect that might be tricky. I'll pass a needinfo to Mathieu who has been very helpful getting remote settings set up for the search code.
Note that currently it looks like Thunderbird is using the main remote settings server for at least some things: https://searchfox.org/comm-central/search?q=services.settings.server&case=false®exp=false&path=
Blocklists and dictionaries at least - so you may want at least some of those things.
Comment 7•5 years ago
|
||
I would not recommend mocking an entire server. There's however one way for Remote Settings to load data from a resource file.
If you package a JSON file in services/settings/dumps/main/
then it will be loaded when .get()
is called on an empty database.
Ideally, this JSON dump should come from the server (because of generated record IDs, timestamps etc.). But if you craft it correctly (by looking at others), then it should be fine. I can help you with review on that.
If you're not interested with Remote Settings updates at all, then you should disable two things:
- the 24h timer https://searchfox.org/mozilla-central/rev/3fd53c47864fedb916f0ed8f002f15456324f729/services/settings/servicesSettings.manifest#7
- the Push broadcast init https://searchfox.org/mozilla-central/rev/3fd53c47864fedb916f0ed8f002f15456324f729/browser/components/BrowserGlue.jsm#2357
Let me know if you need more clarifications (or if I answered off track)
Reporter | ||
Comment 8•5 years ago
|
||
An update here: Firefox is planning on turning on the modern configuration in 78. I would expect us to be removing the legacy code in 80.
Reporter | ||
Comment 9•4 years ago
|
||
I'm most likely going to be doing bug 1619926 to remove the legacy configuration code the week of 6th July - assuming we don't find any issues when we release 78. I don't want to push it back further as it will be blocking other work from happening.
Please can we move forward to a solution here?
Assignee | ||
Comment 10•4 years ago
|
||
Sancus is looking into our own Kinto bucket for blocklist. I'm not sure how/if that's connected - is it? If it is, I guess the next step is to run the script you mentioned to generate the jsons, then get that bucket set up, add the jsons there and point Thunderbird to it?
https://searchfox.org/comm-central/rev/93db6b3c1403b18025e1ad81466979c5b4cd2772/mozilla/modules/libpref/init/all.js#2241-2242
Reporter | ||
Comment 11•4 years ago
|
||
I believe Kinto's the same as remote settings. So yes, if you're having your own bucket there, you'd just need to set up the json - although in theory, you can just upload the file you've already got: https://searchfox.org/comm-central/source/mail/components/search/extensions/engines.json
We have a tool that can do that upload, it would probably need a couple of changes for the bucket configuration.
Reporter | ||
Comment 12•4 years ago
|
||
Sancus, have you made any progress on the separate Kinto bucket?
I have a work around that seems to work and we could land. Though I'd really like to avoid having it in the code for too long as we're more likely to break the automated tests when they're run against Thunderbird.
If you're literally just about to switch to the new bucket, then we might wait, otherwise we need to start getting things removed so I might land the work around anyway.
Comment 13•4 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #12)
Sancus, have you made any progress on the separate Kinto bucket?
I have managed to get Kinto working: https://thunderbird-settings.thunderbird.net/v1/
I do have some concerns about this. If all we are going to store are the remote settings stuff and the amo blocklist, that's two blobs of JSON as far as I understand. Kinto requires a database, cache server, web servers, etc and presumably regular updates to the software. To store two blobs of JSON, that seems unreasonably costly to me in terms of maintenance burden.
Are these things we could just dump in an S3 bucket? Or is there something in the code that depends on particular Kinto features?
In any case, we can use this to unblock things for now. Do I just need to upload the engines.json file above using the tool from Comment #11, or are there more things I need to configure?
Updated•4 years ago
|
Reporter | ||
Comment 14•4 years ago
|
||
(In reply to Andrei Hajdukewycz [:sancus] from comment #13)
(In reply to Mark Banner (:standard8) from comment #12)
I do have some concerns about this. If all we are going to store are the remote settings stuff and the amo blocklist, that's two blobs of JSON as far as I understand. Kinto requires a database, cache server, web servers, etc and presumably regular updates to the software. To store two blobs of JSON, that seems unreasonably costly to me in terms of maintenance burden.
There's other collections you should probably store as well. Also, you probably need a different primary bucket name (services.settings.default_bucket
) to Firefox, or a way of overriding the built-in dumps that Firefox has (https://searchfox.org/mozilla-central/source/services/settings/dumps).
Are these things we could just dump in an S3 bucket? Or is there something in the code that depends on particular Kinto features?
AIUI there's differentials, signing of data and other features that I suspect would make it not suitable for S3. I don't know if you could support a way of having local-dumps only shipped with Thunderbird and not having a server.
At the end of the day, that's a call for the Thunderbird team to make.
In any case, we can use this to unblock things for now. Do I just need to upload the engines.json file above using the tool from Comment #11, or are there more things I need to configure?
I think you need:
- Either a different bucket name, or a way in the build to override the normal dumps.
- Create a new search-config collection, with the search-config schema from https://searchfox.org/mozilla-central/source/toolkit/components/search/schema
- Upload via the tools in comment 11
From the code side, I think we'd need:
- Ensure the local dumps are saved/separate from FF.
- Set the equivalent preferences for Thunderbird: https://searchfox.org/mozilla-central/rev/50cb0892948fb4291b9a6b1b30122100ec7d4ef2/modules/libpref/init/all.js#2245-2249
- Probably set the
security.content.signature.root_hash
to the right value (not totally sure this is needed, might just be normandy stuff). - Add local dumps which are exact copies of the server responses for the full collection.
Let me know if you have questions.
Magnus, will you be able to get someone to handle the dumps, depending on what's decided with the buckets?
Comment 15•4 years ago
|
||
Set the equivalent preferences for Thunderbird: https://searchfox.org/mozilla-central/rev/50cb0892948fb4291b9a6b1b30122100ec7d4ef2/modules/libpref/init/all.js#2245-2249
Quick note on that, we may move away from preferences for certain config values. Relying on app constants seemed like the way to go. If you have comments or ideas, please let me know!
Probably set the security.content.signature.root_hash to the right value (not totally sure this is needed, might just be normandy stuff).
This is the hash of the root certificate, used to verify content signatures, and the certificate chain. I am not fully aware of the necessary steps to host and monitor your own Autograph signer server, rotate keys, etc.
Comment 16•4 years ago
|
||
Well, I couldn't get Kinto authentication to actually work through Apache, and the documentation is basically nonexistent. Fortunately, I found a workaround, and managed to set up the bucket and records I THINK: https://thunderbird-settings.thunderbird.net/v1/buckets/thunderbird/collections/search-config/records
No idea if that's correct. I uploaded https://searchfox.org/comm-central/rev/91a3a1514045b75e4d6cf3c9d8236ba75d8716b9/mail/components/search/extensions/engines.json
Let me know if this is or isn't the correct thing. The bucket name we'd need to use is 'thunderbird'.
Assignee | ||
Comment 17•4 years ago
|
||
I can make a patch for the prefs if we find out the values.
Looks like security.content.signature.root_hash should be the sha-256 fingerprint... of something. But of what? It doesn't appear to be for the certificate of https://firefox.settings.services.mozilla.com/v1/
Assignee | ||
Comment 18•4 years ago
|
||
Assignee | ||
Comment 19•4 years ago
|
||
(In reply to Mathieu Leplatre [:leplatrem] from comment #15)
Probably set the security.content.signature.root_hash to the right value
This is the hash of the root certificate, used to verify content signatures, and the certificate chain. I am not fully aware of the necessary steps to host and monitor your own Autograph signer server, rotate keys, etc.
Oh I see (but not).
What content signatures are that? If it's about verifying e.g. blocklist content, and we get that from Firefox anyway, then I assume there would be trouble if we need to change it to something else.
Comment 20•4 years ago
|
||
What content signatures are that? If it's about verifying e.g. blocklist content, and we get that from Firefox anyway, then I assume there would be trouble if we need to change it to something else.
Basically, content signatures allow the clients to verify that the content of their local DB matches the content of the remote server (integrity, authenticity, etc.). When signatures are enabled, the server uses a private key to compute a signature for the data each time something new is published. The clients fetch the public key, the signature, and the diff of changes. They verify that the public key was issued by the right authority (root hash), and that signature matches the final data after having applied the diff locally.
From what I can see, you don't run the signature part on your server. There's no signing metadata here:
https://thunderbird-settings.thunderbird.net/v1/buckets/thunderbird/collections/search-config
(some insights about enabling signatures)
This probably means that you will have to disable signature verification entirely in your clients. And for now I don't think there is a clean way to do it. We could add an app constant maybe? What's the safest / most convenient?
Assignee | ||
Comment 21•4 years ago
|
||
An app constant should work. It all depends on where the code is.
Reporter | ||
Comment 22•4 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #21)
An app constant should work. It all depends on where the code is.
I've just taken a look, I think you could set this to false (via AppConstants):
or maybe make sure that this returns true:
Mathieu's out this week, but if that works and you can put up a patch, then I'd be willing to review (assuming it only affects TB).
It would probably be good if Thunderbird could sort out content signatures at some stage, but for now that's probably ok.
Reporter | ||
Updated•4 years ago
|
Assignee | ||
Comment 23•4 years ago
|
||
I wonder if this could just not be a pref instead of and AppConstant.
I assume anyone except Mozilla distributing Firefox would need this pref as well. Remote settings aren't possible to build time remove it looks like.
In case an AppConstant is needed what would it be keyed on? MOZ_APP_NAME != thunderbird?
Reporter | ||
Comment 24•4 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #23)
I wonder if this could just not be a pref instead of and AppConstant.
I assume anyone except Mozilla distributing Firefox would need this pref as well. Remote settings aren't possible to build time remove it looks like.
I would expect generally that they'd set up remote settings and a content signing system as well (I'm assuming Thunderbird might as well eventually once you figure out the details...).
In case an AppConstant is needed what would it be keyed on? MOZ_APP_NAME != thunderbird?
Yeah, I used that in my other patch I was experimenting with.
We might need to review this again once Mathieu comes back, but for now this should be ok to get us going I think.
Assignee | ||
Comment 25•4 years ago
|
||
Any reason not to just use a pref?
Reporter | ||
Comment 26•4 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #25)
Any reason not to just use a pref?
AIUI The content signing is part of the security package around remote settings. I don't think this is something that should be able to normally be turned off and I think that Thunderbird should be working towards re-enabling it - turning it off for now seems reasonable as it is only going to be on nightly/beta for a while.
Assignee | ||
Comment 27•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 28•4 years ago
|
||
Assignee | ||
Comment 29•4 years ago
|
||
Since all the app constant would do is check thunderbird, I didn't add a new one.
One test needed adjustment, since the bucket name is different and clearing the pref will keep resetting it to "thunderbird" for us, while the test assumes "main".
Reporter | ||
Updated•4 years ago
|
Assignee | ||
Comment 30•4 years ago
|
||
I've got an updated patch now to set services.settings.default_bucket as needed in the test code. But then I realized that may be the wrong approach.
Sancus, was there a good reason not to keepuse the same name of the bucket as Firefox ("main")? It seems naming it differently is likely to cause some issues down the line, so if there's no reason we should keep the same name.
Comment 31•4 years ago
|
||
(In reply to Magnus Melin [:mkmelin] from comment #30)
I've got an updated patch now to set services.settings.default_bucket as needed in the test code. But then I realized that may be the wrong approach.
Sancus, was there a good reason not to keepuse the same name of the bucket as Firefox ("main")? It seems naming it differently is likely to cause some issues down the line, so if there's no reason we should keep the same name.
I named it differently because Comment #14 said we probably need a different primary bucket name.
Assignee | ||
Comment 32•4 years ago
|
||
Ah yes I forgot they dumps are packaged and shipped.
Updated•4 years ago
|
Comment 33•4 years ago
|
||
Comment 34•4 years ago
|
||
Backed out for Xpc failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/db7cb1b70eefb605b3b7fb2bae27bec2b4acb246
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=426d017d7e46048761db9e0d8eb7e140e0999f78
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314587216&repo=autoland&lineNumber=3225
Comment 35•4 years ago
|
||
Fyi the whole push (all 6 bugs) got backout.
This also caused this to perma-fail: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314579178&repo=autoland&lineNumber=2629
And this intermittent failure: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314581704&repo=autoland&lineNumber=6977
Reporter | ||
Comment 36•4 years ago
|
||
(In reply to Andreea Pavel [:apavel] from comment #35)
Fyi the whole push (all 6 bugs) got backout.
This also caused this to perma-fail: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314579178&repo=autoland&lineNumber=2629
This is fixed in https://phabricator.services.mozilla.com/D89062
And this intermittent failure: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314581704&repo=autoland&lineNumber=6977
This is still WIP, though it is the second half of the patches, so I might land the first half next time.
(In reply to Atila Butkovits from comment #34)
Backed out for Xpc failures.
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314587216&repo=autoland&lineNumber=3225
I'm looking into these at the moment.
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Comment 37•4 years ago
|
||
Magnus: After having done some investigation and not really got anywhere, we've decided to reduce the patch to do the verify signature change, but not make the test-only changes for the default bucket. We are disabling the tests for Thunderbird for now. I've filed bug 1662758 as a follow-up to re-enable that in slower time. At the moment, I really need to get the search patches that are waiting for this one landed.
Comment 38•4 years ago
|
||
Comment 39•4 years ago
|
||
bugherder |
Comment 40•4 years ago
|
||
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 41•4 years ago
|
||
Updated•4 years ago
|
Description
•