1619242 - Some attributes on nsIPrincipals shouldn't be [noscript]

Status: NEW

(Core :: Security: CAPS, task)

Description

[Johann Hofmann [:johannh]]

i.e. these https://searchfox.org/mozilla-central/rev/a69f60390921e21e1d62d40ce388c44a71db00c6/caps/nsIPrincipal.idl#214-254

This commit https://phabricator.services.mozilla.com/D58537 made them [noscript] "to prevent further abuse" but I don't really understand what that "abuse" is and how it's being prevented by letting one half of our code access attributes that the other can't.

(Bug 1618515 was trying to use it)

Comment 1

[(no longer active)]

(In reply to Johann Hofmann [:johannh] from comment #0)

This commit https://phabricator.services.mozilla.com/D58537 made them [noscript] "to prevent further abuse" but I don't really understand what that "abuse" is and how it's being prevented by letting one half of our code access attributes that the other can't.

I think the rest of the comment which you didn't quote answers your question ("to prevent further abuse as a security check"), but needinfo'ing Christoph so that he can confirm...

Comment 2

[Johann Hofmann [:johannh]]

Oh, right, I interpreted "as a security check" to mean "as an additional security measure". Still, I don't really understand the noscript part. Why would JS developers make this mistake while C++ developers don't?

Comment 3

[Johann Hofmann [:johannh]]

For me the problematic context is that there's a project to avoid using principal.URI and instead have APIs for security checks on the principal directly. I think that's a noble goal but sometimes there are good reasons, for both security and non-security purposes, to use the host, e.g. when you want to apply protections on a coarser level than origin, because the website would be able to switch origins by redirecting to a subdomain. Or as in our last example, caching url classifier results on the host.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

If there are good reasons, then I totally agree, we should allow them to use whatever API needed on the Principal. The goal of the 'Removing GetURI from Principal' project is also to create a better mental model around security checks; and in particular the (ab)use of folks querying the URI and doing their own little dance for performing security checks, which is obviously semi-optimal.

To answer your question, there is no difference between C++ users and JS users, both can make the same mistake. The reason we added [noscript] was because there seemed to be no use case on the JS side and we want to shrink the API on the principal as much as possible. Now there is apparently a good reason to remove [noscript] again - at least now we know that use case.

Happy to r+ a change where we remove [noscript] again.

Comment 5

[(no longer active)]

FWIW it seems to me that the actual use case here is: give me a string that can be used as a key for the principal for non-origin-aware storage backends (that is, cookies). Right now that key is hosts, but we may want to change that at some point (Chromium are switching to schemeful hosts). This is very similar to the origin getter. So I think the real thing that we want to use here is something like a cookieOrigin (with a better name) getter + switching code that currently extracts principal.URI.asciiHost to that getter.

Without that, and with bug 1618515, it is unclear to me what removing the [noscript] again here buys us really; it just opens up the potential for more principal.asciiHost consumers to be added to the tree...