1359198 - Add-on recipe previews in the control interface

Status: RESOLVED WONTFIX

(Shield :: General, enhancement)

Description

[Osmose [:osmose, :mkelly]]

Currently, the control interface for Normandy[1] allows users to preview execution of a recipe using UITour privileges to trigger Heartbeat prompts. Previews are useful for:

- S&I folks for previewing what a Heartbeat prompt they're working on will look like to end-users.
- QA folks for testing out recipe functionality quickly (instead of having to restart Firefox and meddle with preferences).

As we release the system add-on to power recipe execution, we'd like to carry over this feature to allow the control interface webpages to trigger recipe execution on demand. We'd probably follow the UITour model, with a few tweaks:

- Webpages emit custom events that are caught by a frame script loaded for each tab. These events include JS to execute within the sandbox to preview recipe execution.
- A hard-coded whitelist of origins whose events are accepted in the Normandy add-on
- Require https, about, or chrome protocols for origins
- Check against the nodePrincipal of pages to confirm the origin
- A testingOrigins preference for manually adding domains to the whitelist for testing

The risk here is that the sandbox in which recipes are executed has the ability to do dangerous things, like start a preference experiment, or (in the future) permanently change a preference value. The preview button would bypass signature checking, since we want previewing to work before the recipe is approved and signed. So while we're whitelisting origins, those origins are still able to make these changes.

Mossop suggested only including preview support in a special build of the add-on, and not including it in the released version in mozilla-central and Balrog updates. That would limit the pool of targets.

dveditz, pauljt: ulfr suggested that I get feedback from ya'll on this feature before starting to work on it. I'm happy to provide more details, if this isn't clear enough. What do ya'll think?

[1] See our concepts documentation for more info if you're not familiar with Normandy: http://normandy.readthedocs.io/en/latest/dev/concepts.html

Comment 1

[:Gijs (he/him)]

How hard would it be to get a separate recognized test signature key into the product that we can use to sign things off for testing this way, without approvals? That would help restrict under what circumstances this could be exploited even if we end up with "issues" on the server side for these domains.

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #0)
> Mossop suggested only including preview support in a special build of the
> add-on, and not including it in the released version in mozilla-central and
> Balrog updates. That would limit the pool of targets.

This also sounds like a sensible idea. Even an about:config opt-in pref would help.

Comment 2

[Michael Cooper [:mythmon]]

I'd argue that we don't need to check signatures for previewing recipes. I think the pref based list of allowed domains would be enough. A opt-in boolean pref to enable previewing sounds fine too. As far as I know, the only site that will be previewing recipes is Normandy, and if Normandy is compromised, the attacker doesn't need to bother with the preview mechanism, they can just modify recipes directly.

> So while we're whitelisting origins, those origins are still able to make these changes.

That's kind of the point, right? What's the attack here that we're trying to guard against?

Comment 3

[Osmose [:osmose, :mkelly]]

(In reply to Mike Cooper [:mythmon] from comment #2)
> I'd argue that we don't need to check signatures for previewing recipes. I
> think the pref based list of allowed domains would be enough. A opt-in
> boolean pref to enable previewing sounds fine too. As far as I know, the
> only site that will be previewing recipes is Normandy, and if Normandy is
> compromised, the attacker doesn't need to bother with the preview mechanism,
> they can just modify recipes directly.
> 
> > So while we're whitelisting origins, those origins are still able to make these changes.
> 
> That's kind of the point, right? What's the attack here that we're trying to
> guard against?

I was thinking of frontend compromises being stopped by signing, but thinking more, any significant frontend compromise could fool the backend into signing code to run in a preview by pretending to be the user hitting the API.

I don't think a separate signing key reduces anything, because at some point the frontend has to ask the backend to sign the data, so the frontend code essentially has the ability to sign stuff with the test signature key. But I might be missing something.

Comment 4

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Michael Kelly [:mkelly,:Osmose] from comment #0)
> Currently, the control interface for Normandy[1] allows users to preview

What is the "control interface"? I reviewed the add-on which runs the recipes, but is this part of the add-on, or the Normandy website? (which I know nothing about btw)

> execution of a recipe using UITour privileges to trigger Heartbeat prompts.

I'm not familiar with what UITour permission grants.  bug 1118775 seems to suggest this was a temporary hack... 2 years ago?
Anyways maybe not relevant to the answer here.

Opt-in via build or preference sounds like a sensible idea.

(In reply to Mike Cooper [:mythmon] from comment #2)
> only site that will be previewing recipes is Normandy, and if Normandy is
> compromised, the attacker doesn't need to bother with the preview mechanism,
> they can just modify recipes directly.

What? Are you saying that there is a Normandy web interface that has the privileges to sign recipes? What does signing protect against, if not this attack? My understanding was that a code review process was required in order to sign recipes. That doesnt sound promising? I dont want to sidetrack here, I'll take this up with ulfr to try to get a better understanding here.


(In reply to Michael Kelly [:mkelly,:Osmose] from comment #3)
> 
> I was thinking of frontend compromises being stopped by signing, but
> thinking more, any significant frontend compromise could fool the backend
> into signing code to run in a preview by pretending to be the user hitting
> the API.

Ok so it that sounds pretty dicey, and definitely not my understanding when I looked at this last year.

> I don't think a separate signing key reduces anything, because at some point
> the frontend has to ask the backend to sign the data, so the frontend code
> essentially has the ability to sign stuff with the test signature key. But I
> might be missing something.
 
Typically you would have a review process or similar via a separate infrastructure/interface, but I'm guessing that isn't the case here? Again I'll take this up with ulfr. 

Coming back to original question. It sounds like this boils down to 'we want to give the normandy website the ability to execute privilege scripts'. If you are not going to sign them (or have a meaningful signing system) then I think we need _something_ to make sure this functionality is not enabled by default (ie preference, ideally something which is reset on browser exit though, so we don't end up with people with this enabled when they dont need it). BTW I'm assuming the only people who would ever enable this are recipe writers? Is that assumption correct?

Comment 5

[Osmose [:osmose, :mkelly]]

(In reply to Paul Theriault [:pauljt] from comment #4)
> What is the "control interface"? I reviewed the add-on which runs the
> recipes, but is this part of the add-on, or the Normandy website? (which I
> know nothing about btw)

It's the Normandy website that recipe authors use to define, review, and deploy recipes.

> Opt-in via build or preference sounds like a sensible idea.

Preference opt-in is my preference from a development perspective, so if that's suitable I'm down with it.

> What? Are you saying that there is a Normandy web interface that has the
> privileges to sign recipes? What does signing protect against, if not this
> attack? My understanding was that a code review process was required in
> order to sign recipes. That doesnt sound promising? I dont want to sidetrack
> here, I'll take this up with ulfr to try to get a better understanding here.

There is a review process in the web interface before recipes get signed. I think mythmon meant that if the Normandy backend was compromised, it could just talk directly to autograph, which is true of any service that is doing signing with autograph.

> Ok so it that sounds pretty dicey, and definitely not my understanding when
> I looked at this last year.

Sorry, I meant that in the context of a preview-only key doing signing without a review process. A frontend compromise in Normandy should not allow you to bypass the review requirements that we currently have, as those are implemented on the backend.

My point was that, assuming that we whitelist origins that can execute recipes manually, if the same whitelisted origin can also sign recipes for previewing (without a peer review), then the signature check doesn't really add a meaningful security check because the same origin (with an auth'd session) can do both the signing and the executing.

> Typically you would have a review process or similar via a separate
> infrastructure/interface, but I'm guessing that isn't the case here? Again
> I'll take this up with ulfr. 

We have a review process, but it's not in separate infrastructure, it's part of the service/interface for creating and deploying recipes. Is that an issue?

> Coming back to original question. It sounds like this boils down to 'we want
> to give the normandy website the ability to execute privilege scripts'. If
> you are not going to sign them (or have a meaningful signing system) then I
> think we need _something_ to make sure this functionality is not enabled by
> default (ie preference, ideally something which is reset on browser exit
> though, so we don't end up with people with this enabled when they dont need
> it). 

A preference that resets on browser exit we can totally do, although I'll want to check with the S&I team (who use the control interface) to make sure having to set that preference whenever they preview works for them. If they'll just never preview anything with that in place because it's too inconvenient, then I'd rather do an imperfect preview so that they're still testing things properly.

> BTW I'm assuming the only people who would ever enable this are recipe
> writers? Is that assumption correct?

Yes.

Comment 6

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Sounds fine to me with a preference. Wouldn't be the end of the world if pref is persistent (could at least instruct the recipe writers to use a specific profile or something. needinfo if you need further questions otherwise Ill take this as done.

Comment 7

[Osmose [:osmose, :mkelly]]

SGTM, thanks!

-needinfo dveditz, although more input is welcome if you want. :D

Comment 8

[Michael Cooper [:mythmon]]

We decided to drop the preview feature that we were discussing here. Thanks for all the advice and discussion.