New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 686891 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Add IsSecure() to url::Origin

Project Member Reported by elawrence@chromium.org, Jan 30 2017

Issue description

Chrome Version: 58.0.2997.0 

Per: https://codereview.chromium.org/2660643002/#msg15, would it make sense to add an IsSecure() method to url::Origin directly?

Today, callers have to go up to content::IsOriginSecure() with a URL rather than an origin.
 
URL only knows about the single page, whereas the computation of IsSecure requires walking the whole stack.

Given that //url is used outside of Chrome (in particular, systems like cronet), and that //net isn't bound to Blink, it seems odd to introduce a Web Platform-ism (document ancestry) to URL

If GURL didn't have knowledge about the ancestry, it seems likely that url::Origin::IsSecure() would end up biffed because it doesn't consider that chain?
I believe Nasko's concern is that Chrome today has content::IsOriginSecure, which doesn't take an origin, and which operates based on the GURL in isolation, not accounting for hosting context (e.g. a blob URI created by a HTTPS page returns false).

Comment 3 by mkwst@chromium.org, Jan 30 2017

I think we might have overloaded the word "secure" a bit. I agree with Ryan that //url can't know whether a URL represents a "secure context", because that's not a question you can ask about URLs (or Origins). //url can know whether a URL could be part of a "secure context" in a necessary-but-not-sufficient sort of way. Secure Contexts calls this concept a "potentially trustworthy origin" (https://w3c.github.io/webappsec-secure-contexts/#potentially-trustworthy-origin) which is pretty verbose, but naming is hard.

I'd be fine making that determination in `url::Origin`, with the caveat that we have a command-line option for setting a list of origins that we consider "potentially trustworthy" even though they don't meet the out-of-the-box list. Also `chrome-extension:`. How do you think we could handle those cases outside of //content?

Comment 4 by dcheng@chromium.org, Jan 30 2017

So... I personally find the distinction between is potentially trustworthy and is secure confusing at times, and wish we could remove it from internal code.

In the past, I've wondered if we could bind the "is secure context" bit to a security context, since it can't change from insecure to secure or vice versa. After seeing this bug, I'm wondering if we can just merge the two concepts and bind the "is secure context" bit to SecurityOrigin/url::Origin when we create one?

There's some handwaving here, but if we could do this, it would be a nice simplification in the internal security model.

Comment 5 by dcheng@chromium.org, Jan 30 2017

Cc: dcheng@chromium.org
dcheng: How do we avoid the overhead for non-Web Platform consumers of //net and //url? What does "is secure context" mean for them, since it's very much a Web Platform idea?

Comment 7 by mkwst@chromium.org, Jan 30 2017

Cc: lgar...@chromium.org
dcheng@: I guess? I agree wholeheartedly that the current implementation of the secure context checks in Blink is way too heavy. We know everything we need to know just by looking at the frame tree, so injecting the "you're totally a secure context" bit could certainly be done when creating a renderer for a frame (+lgarron@, I vaguely remember you volunteering for that? :) ).

I'm less convinced that it makes sense from a layering perspective to add that bit to an origin (SecurityOrigin or url::Origin). It's not a property of the origin. It's a property of the execution context that lives in that origin.

Comment 8 by dcheng@chromium.org, Jan 30 2017

sleevi: The object itself is already 40 bytes large, and we can add a bool/enum field without making it larger. However, it's true that it would be a bit weird in the //net and //url layers... however, does //net itself care at all about an origin being secure? What would happen if it always defaulted to false there?

mkwst: My concern is that people will choose arbitrary between the secure bit on the origin and the secure bit on the security context. I'd rather they didn't have to choose, and that there was only one canonical place for the answer.

Overall, it does seem a bit weird from a layering perspective, though that it might still be a net benefit...
dcheng: I was/am primarily concerned with cognitive overhead from having an "unused/unmaintained" (from the POV of other //net consumers) API, which is harder to quantify than object sizes (although object sizes are relevant).

Mentally, it felt like many of these concepts were linked to //content's notion of a user agent, although I realize the continued existence of //components makes that a harder sell (in that //components care about restricting to a secure origin, but //content is responsible for that determination). As mkwst mentioned, it seems very much a concept of the document settings object, and I'm not sure how that's presently encapsulated to make any good suggestions.

Sign in to add a comment