Closed Bug 1612380 Opened 4 years ago Closed 4 years ago

Thunderbird needs to move its search engine configuration into Remote Settings with a different bucket

Categories

(Thunderbird :: General, task)

Product:
Thunderbird
Component:
General
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(thunderbird_esr78 unaffected, thunderbird81 unaffected)

Status:
RESOLVED FIXED
Milestone:
82 Branch
Tracking Flags:
Tracking Status
thunderbird_esr78 --- unaffected
thunderbird81 --- unaffected

People

(Reporter: standard8, Assigned: mkmelin)

References

Details

Attachments

(2 files, 2 obsolete files)

Bug 1612380 - don't verify content signatures for Thunderbird. r=standard8, leplatrem
47 bytes, text/x-phabricator-request
Details | Review
bug1612380_kinto.patch
1.71 KB, 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.

Not sure who should be looking at this.

Flags: needinfo?(mkmelin+mozilla)
See Also: → 1593188

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...

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?

Flags: needinfo?(mkmelin+mozilla) → needinfo?(standard8)

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?

Flags: needinfo?(standard8) → needinfo?(mkmelin+mozilla)

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?

Flags: needinfo?(mkmelin+mozilla)

(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&regexp=false&path=

Blocklists and dictionaries at least - so you may want at least some of those things.

Flags: needinfo?(mathieu)

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:

Let me know if you need more clarifications (or if I answered off track)

Flags: needinfo?(mathieu)

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.

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?

Flags: needinfo?(mkmelin+mozilla)

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

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.

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.

Flags: needinfo?(sancus)

(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?

Flags: needinfo?(sancus)
Flags: needinfo?(mkmelin+mozilla)

(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:

From the code side, I think we'd need:

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?

Flags: needinfo?(sancus)
Flags: needinfo?(mkmelin+mozilla)

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.

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'.

Flags: needinfo?(sancus)

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/

Flags: needinfo?(mkmelin+mozilla)
Attached file (WIP) bug1612380_kinto.patch (obsolete) —

(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.

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?

An app constant should work. It all depends on where the code is.

(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):

https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/services/settings/RemoteSettingsClient.jsm#262

or maybe make sure that this returns true:

https://searchfox.org/mozilla-central/rev/73a14f1b367948faa571ed2fe5d7eb29460787c1/services/settings/RemoteSettingsClient.jsm#812

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.

Flags: needinfo?(mkmelin+mozilla)

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?

(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.

Any reason not to just use a pref?

Flags: needinfo?(mkmelin+mozilla)

(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: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #9171165 - Attachment is obsolete: true
Attachment #9172603 - Flags: review?(standard8)

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".

Attachment #9172603 - Flags: review?(standard8) → review+

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.

Flags: needinfo?(sancus)

(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.

Flags: needinfo?(sancus)

Ah yes I forgot they dumps are packaged and shipped.

Attachment #9172602 - Attachment description: Bug 1612380 - don't verify content signatures for Thunderbird. r=standard8 → Bug 1612380 - don't verify content signatures for Thunderbird. r=standard8, leplatrem
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b9cd6b7047b9
don't verify content signatures for Thunderbird. r=leplatrem

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

Flags: needinfo?(standard8)

(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.

Flags: needinfo?(standard8)
Flags: needinfo?(mkmelin+mozilla)
Blocks: 1662758

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.

Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c92f286ce680
don't verify content signatures for Thunderbird. r=leplatrem

https://hg.mozilla.org/mozilla-central/rev/c92f286ce680

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 82 Branch
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/0acfa219b783
use Thunderbird remote settings server for search engine config. r=standard8
Attachment #9173859 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: