Closed Bug 1308951 Opened 5 years ago Closed 5 years ago

Add a pref to whitelist specific domains as SecureContexts

Categories

(Core :: DOM: Security, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: rbarnes, Assigned: rbarnes)

Details

Attachments

(1 file)

One of the objections to restricting APIs to secure contexts (cf. Bug 1177957) is that this makes it difficult to run small experiments, since you need to get a certificate for the website hosting the experiment.   While the industry has made progress in making it easier to get certificates, I'm willing to concede that this can still be a barrier for some folks.

To remove this barrier, we should consider adding a pref that allows users to whitelist certain domains as secure contexts.  Specifically, for domains in the whitelist, we would consider an origin as potentially trustworthy [1] if the hostname in the origin is in the whitelist, regardless of the scheme or port.

For example, if the preference contains "example.com,example.net", then the origins "http://example.com:8090" would be considered potentially trustworthy.

[1] https://w3c.github.io/webappsec-secure-contexts/#is-origin-trustworthy
Assignee: nobody → rlb
Attachment #8799441 - Flags: review?(jjones)
Comment on attachment 8799441 [details]
Bug 1308951 - Add a pref to whitelist specific domains as SecureContexts

https://reviewboard.mozilla.org/r/84618/#review83210

I'd recommend getting a second opinion than mine that this isn't dangerous in some non-obvious way before landing, but from every angle I look at it, it looks fine.

::: dom/security/test/unit/test_isOriginPotentiallyTrustworthy.js:22
(Diff revision 1)
>  
>  XPCOMUtils.defineLazyServiceGetter(this, "gContentSecurityManager",
>                                     "@mozilla.org/contentsecuritymanager;1",
>                                     "nsIContentSecurityManager");
>  
> +var prefs = Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch);

We should also test the case of the whitelist containing non-network resources, to catch that branch.
Attachment #8799441 - Flags: review?(jjones) → review+
Attachment #8799441 - Flags: review?(ckerschb)
Comment on attachment 8799441 [details]
Bug 1308951 - Add a pref to whitelist specific domains as SecureContexts

https://reviewboard.mozilla.org/r/84618/#review83210

> We should also test the case of the whitelist containing non-network resources, to catch that branch.

- Added a test based on a "chrome://" URL
- Changed one of the whitelisted URLs to a "ws://" URL
- Added tests for the remaining whitelisted schemes
Comment on attachment 8799441 [details]
Bug 1308951 - Add a pref to whitelist specific domains as SecureContexts

https://reviewboard.mozilla.org/r/84618/#review83210

Added :ckerschb
Comment on attachment 8799441 [details]
Bug 1308951 - Add a pref to whitelist specific domains as SecureContexts

https://reviewboard.mozilla.org/r/84618/#review83246

looks good, r=me

::: dom/security/nsContentSecurityManager.cpp:681
(Diff revision 2)
> +  // check to see if it has been whitelisted by the user.  We only apply this
> +  // whitelist for network resources, i.e., those with scheme "http" or "ws".
> +  // The pref should contain a comma-separated list of hostnames.
> +  bool networkResource = scheme.EqualsLiteral("http") || scheme.EqualsLiteral("ws");
> +  nsAdoptingCString whitelist = Preferences::GetCString("dom.securecontext.whitelist");
> +  if (networkResource && whitelist) {

performance nit: would it make sense to only query the whitelist in case we are evaluting a scheme of http or ws? In other words. Would it make sense to do:

if ("http" || "ws") {
  if (whitelist) {
   ...
  }
}

I would imagine that quering the whitelist is rather slow and we could avoid doing it if not necessarily needed.
Attachment #8799441 - Flags: review?(ckerschb) → review+
Comment on attachment 8799441 [details]
Bug 1308951 - Add a pref to whitelist specific domains as SecureContexts

https://reviewboard.mozilla.org/r/84618/#review83258
Comment on attachment 8799441 [details]
Bug 1308951 - Add a pref to whitelist specific domains as SecureContexts

https://reviewboard.mozilla.org/r/84618/#review83260
Pushed by rlb@ipv.sx:
https://hg.mozilla.org/integration/autoland/rev/cfb9de0c9f2a
Add a pref to whitelist specific domains as SecureContexts r=ckerschb,jcj
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/cfb9de0c9f2a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.