776420 - [Security Review][Action Item] Review trusted dialog js code

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

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

Review the modal dialog used to display navigator.pay messages.

Comment 1

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

Just checking, the code is question is trusted_dialog.js ?  (https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/trusted_dialog.js) The file is called something slightly different in bug 768943, but it looks very similar so assume that it just had a name change.

Just recapping my understanding of the navigator.pay review:  the plan to prevent spoofing is that Navigator.pay will only launch for payment endpoints which are whitelisted in preferences, so we can trusted those pages not to phish credentials.

But there is no URL bar, so SSL errors must be treated like network connection errors (no way to override them). (Currently SSL errors on b2g already behave like this, with a "This address isn't valid" displayed in case of an SSL error.)

I still see a number of possible threats:
- If insecure (non-http) content the is ever shown in this window, then there is a risk of attack. It would be good if we could enfore SSL in the browser somehow (ie ensure hsts and no mixed content)
- navigator.pay relies on there only ever being trusted payment providers in that bar. What happens if the there is a link from the payment window provided by the payment provider, and the user ends up browsing somewhere else (maybe to somewhere insecure?) Also what other apps/system features will use this trusted_dialog?

Comment 2

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Paul Theriault [:pauljt] from comment #1)
> Just checking, the code is question is trusted_dialog.js ? 
> (https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> trusted_dialog.js) The file is called something slightly different in bug
> 768943, but it looks very similar so assume that it just had a name change.
> 
Yes, that's the code.

> Just recapping my understanding of the navigator.pay review:  the plan to
> prevent spoofing is that Navigator.pay will only launch for payment
> endpoints which are whitelisted in preferences, so we can trusted those
> pages not to phish credentials.
> 
> But there is no URL bar, so SSL errors must be treated like network
> connection errors (no way to override them). (Currently SSL errors on b2g
> already behave like this, with a "This address isn't valid" displayed in
> case of an SSL error.)
> 
> I still see a number of possible threats:
> - If insecure (non-http) content the is ever shown in this window, then
> there is a risk of attack. It would be good if we could enfore SSL in the
> browser somehow (ie ensure hsts and no mixed content)

I am not sure how to do that from content, but I'll try to find out.

> - navigator.pay relies on there only ever being trusted payment providers in
> that bar. What happens if the there is a link from the payment window
> provided by the payment provider, and the user ends up browsing somewhere
> else (maybe to somewhere insecure?) Also what other apps/system features
> will use this trusted_dialog?

Yes it makes sense to control the trusted dialog in a way that it can't at least browse to a different origin. I am not familiar with the iframe 'sandbox' attribute, but it may be useful for this if it is what I think it is. I'll check it out.

Thanks.

Comment 3

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

Something that occured to me - an app can currently just embed the homescreen UI to simulate the trusted UI. This works at the moment, but not sure if this will change when the permissions model comes in. I'll raise this in the Trusted UI bug.

Comment 4

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

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #2)
> (In reply to Paul Theriault [:pauljt] from comment #1)
> > Just checking, the code is question is trusted_dialog.js ? 
> > (https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> > trusted_dialog.js) The file is called something slightly different in bug
> > 768943, but it looks very similar so assume that it just had a name change.
> > 
> Yes, that's the code.
> 
> > Just recapping my understanding of the navigator.pay review:  the plan to
> > prevent spoofing is that Navigator.pay will only launch for payment
> > endpoints which are whitelisted in preferences, so we can trusted those
> > pages not to phish credentials.
> > 
> > But there is no URL bar, so SSL errors must be treated like network
> > connection errors (no way to override them). (Currently SSL errors on b2g
> > already behave like this, with a "This address isn't valid" displayed in
> > case of an SSL error.)
> > 
> > I still see a number of possible threats:
> > - If insecure (non-http) content the is ever shown in this window, then
> > there is a risk of attack. It would be good if we could enfore SSL in the
> > browser somehow (ie ensure hsts and no mixed content)
> 
> I am not sure how to do that from content, but I'll try to find out.

This seems to be something that would be in chrome not content? ( I was thinking like somewhere in Payment.jsm... but I have no idea really)

> 
> > - navigator.pay relies on there only ever being trusted payment providers in
> > that bar. What happens if the there is a link from the payment window
> > provided by the payment provider, and the user ends up browsing somewhere
> > else (maybe to somewhere insecure?) Also what other apps/system features
> > will use this trusted_dialog?
> 
> Yes it makes sense to control the trusted dialog in a way that it can't at
> least browse to a different origin. I am not familiar with the iframe
> 'sandbox' attribute, but it may be useful for this if it is what I think it
> is. I'll check it out.

 I dont think Sandbox will help. Afaik the main goal of sandbox is to protect the parent page from child content (main this is that all cross-origin checks fail). Ian Melven can tell you more, since he is implementing sandbox in Firefox.

For trusted Apps I know we plan to force a CSP on them, maybe we can use the same mechanism (which i think is new). Ill try to find more info.

> 
> Thanks.

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Paul Theriault [:pauljt] from comment #4)
> (In reply to Fernando Jiménez Moreno [:ferjm] from comment #2)
> > (In reply to Paul Theriault [:pauljt] from comment #1)
> > > Just checking, the code is question is trusted_dialog.js ? 
> > > (https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> > > trusted_dialog.js) The file is called something slightly different in bug
> > > 768943, but it looks very similar so assume that it just had a name change.
> > > 
> > Yes, that's the code.
> > 
> > > Just recapping my understanding of the navigator.pay review:  the plan to
> > > prevent spoofing is that Navigator.pay will only launch for payment
> > > endpoints which are whitelisted in preferences, so we can trusted those
> > > pages not to phish credentials.
> > > 
> > > But there is no URL bar, so SSL errors must be treated like network
> > > connection errors (no way to override them). (Currently SSL errors on b2g
> > > already behave like this, with a "This address isn't valid" displayed in
> > > case of an SSL error.)
> > > 
> > > I still see a number of possible threats:
> > > - If insecure (non-http) content the is ever shown in this window, then
> > > there is a risk of attack. It would be good if we could enfore SSL in the
> > > browser somehow (ie ensure hsts and no mixed content)
> > 
> > I am not sure how to do that from content, but I'll try to find out.
> 
> This seems to be something that would be in chrome not content? ( I was
> thinking like somewhere in Payment.jsm... but I have no idea really)
> 

The trusted dialog is part of Gaia, all that the payment API does is sending a mozChromeEvent so the Gaia system app can create and show the trusted dialog. The trusted dialog can be created from the system app without chrome intervention.

> > 
> > > - navigator.pay relies on there only ever being trusted payment providers in
> > > that bar. What happens if the there is a link from the payment window
> > > provided by the payment provider, and the user ends up browsing somewhere
> > > else (maybe to somewhere insecure?) Also what other apps/system features
> > > will use this trusted_dialog?
> > 
> > Yes it makes sense to control the trusted dialog in a way that it can't at
> > least browse to a different origin. I am not familiar with the iframe
> > 'sandbox' attribute, but it may be useful for this if it is what I think it
> > is. I'll check it out.
> 
>  I dont think Sandbox will help. Afaik the main goal of sandbox is to
> protect the parent page from child content (main this is that all
> cross-origin checks fail). Ian Melven can tell you more, since he is
> implementing sandbox in Firefox.

For what I see in https://wiki.mozilla.org/Features/Platform/Iframe_Sandbox there are plans for a CSP sandbox directive. Maybe that can be usefull for the trusted dialog. I am CCing Ian Melven, in case that he can provide further info.

Thanks Paul!

Comment 6

[Ian Melven :imelven]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #5)
> (In reply to Paul Theriault [:pauljt] from comment #4)
> > (In reply to Fernando Jiménez Moreno [:ferjm] from comment #2)
> > > (In reply to Paul Theriault [:pauljt] from comment #1)
> > > > Just checking, the code is question is trusted_dialog.js ? 
> > > > (https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/
> > > > trusted_dialog.js) The file is called something slightly different in bug
> > > > 768943, but it looks very similar so assume that it just had a name change.
> > > > 
> > > Yes, that's the code.
> > > 
> > > > Just recapping my understanding of the navigator.pay review:  the plan to
> > > > prevent spoofing is that Navigator.pay will only launch for payment
> > > > endpoints which are whitelisted in preferences, so we can trusted those
> > > > pages not to phish credentials.
> > > > 
> > > > But there is no URL bar, so SSL errors must be treated like network
> > > > connection errors (no way to override them). (Currently SSL errors on b2g
> > > > already behave like this, with a "This address isn't valid" displayed in
> > > > case of an SSL error.)
> > > > 
> > > > I still see a number of possible threats:
> > > > - If insecure (non-http) content the is ever shown in this window, then
> > > > there is a risk of attack. It would be good if we could enfore SSL in the
> > > > browser somehow (ie ensure hsts and no mixed content)
> > > 
> > > I am not sure how to do that from content, but I'll try to find out.
> > 
> > This seems to be something that would be in chrome not content? ( I was
> > thinking like somewhere in Payment.jsm... but I have no idea really)
> > 
> 
> The trusted dialog is part of Gaia, all that the payment API does is sending
> a mozChromeEvent so the Gaia system app can create and show the trusted
> dialog. The trusted dialog can be created from the system app without chrome
> intervention.
> 
> > > 
> > > > - navigator.pay relies on there only ever being trusted payment providers in
> > > > that bar. What happens if the there is a link from the payment window
> > > > provided by the payment provider, and the user ends up browsing somewhere
> > > > else (maybe to somewhere insecure?) Also what other apps/system features
> > > > will use this trusted_dialog?
> > > 
> > > Yes it makes sense to control the trusted dialog in a way that it can't at
> > > least browse to a different origin. I am not familiar with the iframe
> > > 'sandbox' attribute, but it may be useful for this if it is what I think it
> > > is. I'll check it out.
> > 
> >  I dont think Sandbox will help. Afaik the main goal of sandbox is to
> > protect the parent page from child content (main this is that all
> > cross-origin checks fail). Ian Melven can tell you more, since he is
> > implementing sandbox in Firefox.
> 
> For what I see in https://wiki.mozilla.org/Features/Platform/Iframe_Sandbox
> there are plans for a CSP sandbox directive. Maybe that can be usefull for
> the trusted dialog. I am CCing Ian Melven, in case that he can provide
> further info.
> 
> Thanks Paul!

Hi, please see bug 671389 for CSP sandbox - this is dependent on landing 341604 which is iframe sandbox itself, which seems like it's getting fairly close. 

Paul is right that one of the goals of sandbox is to remove the document's 'natural origin' - while the sandbox attribute does restrict navigation to some extent, particulaly navigating window.top, it doesn't restrict the sandboxed content navigating itself at all (this is explicitly allowed), but the attribute can also disable scripting, which might make it harder for the sandboxed content to automatically navigate itself (although consider meta refresh and other non-script-y means - and that IE10 for example blocks meta refresh in a sandboxed document that does not specify 'allow-scripts'.

Comment 7

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Ian Melven :imelven from comment #6)
> (In reply to Fernando Jiménez Moreno [:ferjm] from comment #5)
> > (In reply to Paul Theriault [:pauljt] from comment #4)
> > > (In reply to Fernando Jiménez Moreno [:ferjm] from comment #2)
> > > > (In reply to Paul Theriault [:pauljt] from comment #1)

> > Thanks Paul!
> 
> Hi, please see bug 671389 for CSP sandbox - this is dependent on landing
> 341604 which is iframe sandbox itself, which seems like it's getting fairly
> close. 
> 
> Paul is right that one of the goals of sandbox is to remove the document's
> 'natural origin' - while the sandbox attribute does restrict navigation to
> some extent, particulaly navigating window.top, it doesn't restrict the
> sandboxed content navigating itself at all (this is explicitly allowed), but
> the attribute can also disable scripting, which might make it harder for the
> sandboxed content to automatically navigate itself (although consider meta
> refresh and other non-script-y means - and that IE10 for example blocks meta
> refresh in a sandboxed document that does not specify 'allow-scripts'.


Considering the objective of this UI is to contain a page served by a payment provider, I don't think restricting scripts will fly very well. Also, I don't think restricting navigation is a viable proposal. The payment provider should be able to navigate away from its domain--- in fact it might even need to do just that. For example, some of my credit cards, when used on some payment providers require authorization of payments by entering a coordinate from a card. The coordinate requesting page is served by my bank, not by the payment provider itself. So the payment flow would be:

www.mypaymentprovider.com -> I enter my payment details here -> www.somevisaservicethatidentifiesbanks.com -> www.mybank.com -> I enter my coordinates here -> www.mypaymentprovider.com (hey thanks for your money).

And all of the steps use Javascript. 

Not allowing HTTP on the landing pages (on the initial pages we load) is something I requested already. But from then on, I think it's the payment provider responsibility. In other words, what we assure is that you're landing at the payment provider you selected, and you can't even select the actual URL, that's hardcoded. How good or how bad the actual provider is it's not something we can control, nor I think we should even (that's taking responsibility from a risk that shouldn't be ours).

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Is there anything to do here? Does this really need to remain a blocker?

Comment 9

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

The intent of this bug was to review the trusted UI created to support payment.js which I think is now 794999. Since 794999 isn't basecamp blocking I am making this non-blocking, but I still think its important to have this UI reviewed once it is finished.

Comment 10

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

Actually 794999 is mainly a cosmetic change so I don't think there is anything more to do here.