Add a [SecureContext] operator attribute

[mikewest]

As discussed in w3c/webappsec#262, this patch defines a [SecureContext]
attribute for operators that rejects Promises, or throws exceptions, when
an operator is executed from a non-secure context.

[bzbarsky]

The extended attribute definition typically contains only informative text in terms of what it does. The normative requirements should go into the places whose behavior it affects, to prevent comefrom-style specification.

In this case that means the normative text should be in http://heycam.github.io/webidl/#es-operations. When that's done, that will automatically fix the issues with the proposed text underdefining behavior (e.g. which happens first: checks for the right kind of "this" value or the secure context checks) and the Promise behavior will fall out for free, as long as you throw an exception from somewhere inside the steps prefaced by "Try running the following steps".

[bzbarsky]

One other thing worth thinking about: Is it possible for two pages to be same effective script origin but differ in terms of whether they're secure contexts? And if so, which settings object of the four possible ones (entry, incumbent, associated with the callee global, associated with the this value global) is the one whose secure-contextness is checked?

[mikewest]

Thanks, @bzbarsky. This is exactly what I needed. :) I've uploaded a second pass at the patch, and I hope you'll indulge a few more newbie questions.

1. I think it makes sense to perform this check at more or less the same time we're performing "a security check", and to use the incumbent settings object, as I think we ought to care about the context that's using a tool to do some work, rather than the context from which the tool came. Does that make sense?

(As an aside: WebIDL refers to "incumbent script" and "stack of incumbent scripts". Those ought to be renamed to "[stack of] incumbent settings object", right? I can throw you a patch if that's accurate.)
2. Should I leave the slimmed-down descriptive text in the [SecureContext] section? I think it might be helpful for the reader, but someone more familiar with the spec might not need it. Is the normative description in the operations section good enough?

[bzbarsky]

Does that make sense?

It does, but I would prefer we do it after the "a security check" bit. The reason for that is that at least in Gecko's bindings the "security check" bit and "is the this value of the right type" bit is actually a single more or less atomic check on some state of the this value, so inserting steps between them is actually quite difficult without causing performance regressions.

Those ought to be renamed to "[stack of] incumbent settings object", right?

Yeah, looks like the HTML spec mutated out from under IDL here.

Should I leave the slimmed-down descriptive text in the [SecureContext] section?

That's probably fine as long as it's explicitly marked informative; the thing I really want to avoid is accidental conflicts, real or perceived, between normative descriptive text and the normative bits in es-operation. Making the descriptive text non-normative avoids that problem, and I agree that it's helpful to a casual reader.

[bzbarsky]

Oh, and to the main question about whether it's the incumbent script that should be used or not... In Gecko, it's quite easy to do that, but I recall both Blink and IE having serious concerns about use of incumbent settings objects, because those involve some sort of expensive stack walk in those implementations. So it really would be good to figure out whether it actually matters in practice which settings object is used. And that comes back to my question about whether two pages that are same-effectives-script-origin can ever differ in terms of whether they're secure contexts.

[mikewest]

It does, but I would prefer we do it after the "a security check" bit.

As long as the check happens somewhere around here, I think we're fine. The exact ordering isn't something I have strong feelings about, so I'm happy to move it around. Moved under "a security check" in the latest patch.

That's probably fine as long as it's explicitly marked informative.

Wrapped in a <div class="note"> in the latest patch.

whether two pages that are same-effectives-script-origin can ever differ in terms of whether they're secure contexts.

Since the notion walks the ancestor tree, yes, two same-origin contexts can have different security states. I don't think that will end up being a terribly difficult check to do in Blink's bindings, and I think it's the behavior we've already implemented for restrictions on things like WebCrypto and WebRTC.

[bzbarsky]

Since the notion walks the ancestor tree, yes, two same-origin contexts can have different security
states.

Ah, good point. You might still want to check with Microsoft about the situation with incumbent settings objects in their implementation, of course.

[mikewest]

Is there anyone in particular you'd recommend at MS? I can ping random folks, but if there are particular people that would be familiar with their bindings, I'd love to know them. :)

[travisleithead]

I'm looking into it...

[mikewest]

@travisleithead: You are a wonderful man.

[travisleithead]

Took a bit of time to spool-up on the terminology and impact here. :)

We would really prefer being able to setup a static backing implementation for the [SecureContext] operations, so that we can generate the throwing or Promise-resolving backing behavior when we first build the script context for a new browsing context. To enable this, we'd like to have the check be based on the origin of the callee's global object (which can be implemented as a static-check vs. a runtime check). Honestly, I'm not quite sure how callee's origin differs from incumbent's origin? Maybe an example would help :)

In discussing this, however, it occurred to us that any access from a non-secure context to a secure context and vice-versa would go through a security wrapper at the window/document which would deny access so the APIs in the first place. E.g., an https and http URL are always considered different origins, correct? Same-origin policy would not even allow you to get far enough into the destination origin for your proposed security check to ever be executed, right?

[bzbarsky]

Travis, see end of #65 (comment) for an example of when two things can be same-origin when one is considered secure and one is not...

[travisleithead]

Hmm, so it seems that isSecureContext can change dynamically over time? I can understand that for SharedWorker, but is this also true for documents in a Window (when navigation is not involved)?

@mikewest do you have any thoughts on the tail end of #65 (comment) (how the same-origin restrictions might obviate the need for this extended attribute in the first place)?

[bzbarsky]

I don't think isSecureContext can change over time, because it just depends on the ancestor chain and the ancestor chain can't change over time.

[mikewest]

@travisleithead: I don't think there are any scenarios in which isSecureContext can change for a given document, but there are certainly cases in which two same-origin resources can have different values. Consider the case where the nested frame in example 5 (https://w3c.github.io/webappsec-secure-contexts/#examples-framed) pops up a new window (a la example 2 (https://w3c.github.io/webappsec-secure-contexts/#examples-top-level)), for example.

In those cases, the non-secure context could grab an object from the secure context and execute it. I think we want to check the incumbent settings object for that scenario, rather than the settings object associated with the callee's global.

[travisleithead]

I believe I'm on the same page now--yes, incumbent settings object seems appropriate for that scenario. And yes, checking incumbent script context is non-trivial for us and involves a stack walk :( So, no, I'm not a fan. :)

This may not be the right place, but I want to pop-up a level to the principle of the problem (perhaps we can take this offline if you wish). Considering you cite Netflix's workaround in the doc as a general basis for needing slightly stronger protection than simply same-origin isolation, and in section 5.1 you talk about incomplete isolation which is broken by the new window scenario--then incumbent settings object will work against you in that case--and you probably already realize this. (The secure context HTTPS top-level window will call back through the opener property to the HTTPS-but-insecure-context iframe, and be able to execute it's secure context APIs.)

What is it that makes the new window popup that much more of a deterrent than the simple iframe case cited for Netflix? I'll be Netflix for a second under the new rules of the game: now, I popup a new window when the user clicks the play button (or selects a movie) so that I work around the pop-up blocker, I manipulate focus to that the window popup is behind the main window (if it wasn't opened in a new tab), and I carry out my secure operation there. If it's a short term crypto-related use, I can have the window closed before users even know what happened. Otherwise, I can put it in the background--but generally, I'm implying that using a new window vs. an iframe is inherently (from a security standpoint) no different. Why then is it OK to have this loophole?

I apologize if you've been though this discussion before; I'm just concerned about the implementation burden this will have and want to be sure all angles have been thoroughly considered.

[mikewest]

now, I popup a new window when the user clicks the play button (or selects a movie) so that I work around the pop-up blocker, I manipulate focus to that the window popup is behind the main window (if it wasn't opened in a new tab), and I carry out my secure operation there.

If it's a short operation that doesn't require user input, then popping up a window, doing your powerful thing, and then closing window would be effective. Netflix in particular couldn't have done that because they required the WebCrypto API persistently in order to decode their magical DRM.

Otherwise, I can put it in the background

Chrome's done a lot of work to make popunders difficult to impossible to create. I admit that I'm not up on the state of other browsers here, but aren't popunders something we should kill? :)

Why then is it OK to have this loophole?

The thing I'm worried about is navigation. I see popups being used fairly often to move users from one page to another, and it seems strange to me to tie the new page's security state to the page from which it spawned. To make this at all effective, we'd more or less have to taint the whole window somehow, as a cooperative window could otherwise just navigate itself around until we considered it secure.

I guess I'd be willing to do that if Microsoft objects to/would have problems implementing the current definition. Perhaps you could start a thread in WebAppSec? Or extend the thread @rlbmoz started, I suppose: https://lists.w3.org/Archives/Public/public-webappsec/2015Oct/0073.html.

[martinthomson]

I think that this should be an exposure change, and not a variation in implementation. That is, interfaces, attributes and methods annotated with [SecureContext] should not be visible to content. Two reasons: one is that it allows us to remove attributes (e.g., Window.crypto.subtle) and interfaces as well as methods.

Second being that this then relies instead on feature detection rather than error handling. In general, I think that apps treat absent features very differently to failing features. Critically, if getUserMedia() were to be annotated in this fashion (as Chrome is doing), then you only learn that it is broken in this fashion when you call it. However, calling getUserMedia() has side-effects that a site might try to avoid: the permission prompt.

[travisleithead]

@martinthomson That makes a lot of sense, though it doesn't prevent the insecure context from using a new window to broker its access to the features not present--though perhaps that can be solved in a different way as noted in w3c/webappsec-secure-contexts#6 (comment)

[martinthomson]

@travisleithead, I don't think that we can completely prevent the workarounds in secure contexts, but that's a problem for @mikewest to solve over on w3c/webappsec-secure-contexts. WebIDL need only consume the results of that work, which doesn't necessarily require any judgment about whether secure contexts is good or bad (of course I'd prefer good, but recognize how hard it is to get this right).

[mikewest]

@martinthomson: In our F2F yesterday, we noted that doing something like w3c/webappsec-secure-contexts#10 would be lovely. That model really requires throwing rather than hiding. I think this is the model we'd like to run with.

@rlbmoz: You suggested that you'd like to read this patch to figure out whether it did things in an order and location that you thought was sane. Mind taking a look?

[mikewest]

As discussed in w3c/webappsec-secure-contexts#6, the "Secure Context" behavior of popups is now tied to noopener. I think this means we can safely use the callee's settings object, which the current patch does.

@bzbarsky @travisleithead, would you mind taking another look at this?

[bifurcation]

Assuming we're going with the "throw/reject" approach and not the "exposure" approach, this patch looks sensible to me.

In an ideal world, I would really prefer we control this through exposure rather than errors, for reasons I've stated before. Is it possible to have use exposure as a general approach and still be consistent with w3c/webappsec-secure-contexts#10? The case you have to deal with is:

1. if (navigator.secureOnlyFeature) {...}
2. document.domain = document.domain
3. navigator.secureOnlyFeature.doSomething()

What if we were to break the symmetry of w3c/webappsec-secure-contexts#10, in the following way: If you touch something secure first, then document.domain throws. If you touch document.domain first, then you can't see anything that's [SecureOnly]. That's a little bit magical, in that the [SecureOnly] stuff gets yanked from the DOM when you touch document.domain. But it's not clear to me that hiding all of those things is any more work than telling them all to throw.

[bzbarsky]

If you touch something secure first, then document.domain throws.

Keep in mind that per spec all the prototype objects for a page are set up before anything else happens; at that point the "secure" props are either there or not. Making them disappear when document.domain is set is ... complicated at best.

With that in mind, what do you mean when you say "touch something secure"? That is, what constitutes touching?

[mikewest]

@bifurcation: The more interesting case for the "exposure" method is:

var thing = navigator.secureOnlyFeature;
document.domain = document.domain;
thing.whatever();

I don't know what we'd do in that case; it seems difficult to deal with. Throwing seems to deal with it more cleanly (assuming we're doing runtime checks).

@bzbarsky's note that the prototypes are fixed at load time is (I think) true for V8 as well (@jeisinger to confirm). Removing things based on runtime checks might be difficult to implement.

[hax]

Making them disappear when document.domain is set is ... complicated at best.

It seems IE behave like that.

[bzbarsky]

I mean, it's doable, but not without performance penalties and various other problems.

[ghost]

So I can probably live with the "throw approach" if we can't just kill
document.domain. Thanks for talking this through.

Would it be worth throwing in the "potentially trustworthy" distinction
here as well? If a context is not even potentially trustworthy (e.g., an
"http:" origin), there's no issue with document.domain, so you don't get
the raciness. So it seems like you could do something like:

• If the origin is not potentially trustworthy, then don't expose the object
• Otherwise, throw if you're in a non-secure context (either because of
  framing or because of document.domain)

On Fri, Nov 13, 2015 at 11:39 AM, Boris Zbarsky notifications@github.com
wrote:

Note "various other problems". E.g. what if the page has already redefined
the secure APIs as non-configurable? Now you can't delete them. Should
document.domain setting fail in that case? What if the page just grabbed
the relevant functions as in #65 (comment)
#65 (comment)

—
Reply to this email directly or view it on GitHub
#65 (comment).

[travisleithead]

It seems IE behave like that.

Not sure what you mean, we setup all APIs at script engine creation time.

As far as throw vs. change exposure; throw works for me. I was under the impression that document.domain could not change a document's secure context state. Did I understand the following to mean that it can't today, but that you [Mike] want to enable such a feature?

I want to support that feature. To that end, I'd prefer throwing, as outlined in this PR.

Because I very much understand the desire to reduce the "I'm not allowed to use an API because its in a secure context" problem down to a simple API feature-detect. The counter-argument is that you already provide a feature detect via isSecureContext. :) So, I'm not passionate about it either way.

[jeisinger]

what about hiding the attributes for newly created insecure contexts, and spec that if you somehow manage to get hold of the attribute in an insecure context, and you invoke it, it'll throw?

[bifurcation]

@jeisinger That sounds like a fine idea to me.

[mikewest]

Since I'm apparently the only one who feels that throwing is better than changing exposure, I give up. :)

The latest patch adjusts the exposure rules for operators to take account of the [SecureContext] attribute as @jeisinger suggested in #65 (comment). I think this addresses all the feedback thus far. WDYT, @bzbarsky & @heycam?

[mikewest]

(Also, [SecureContext] now applies to everything [Exposed] applies to. This might or might not be a reasonable idea.)

[bzbarsky]

I'm not going to have time to carefully review this until January 4 at best, but one thing that I think needs to be clarified is that [SecureContext] does not dynamically change exposure with the secure context state. That is, things flagged [SecureContext] are exposed if the page was a secure context at the point of creation of the global object, no matter what it is now.

[mikewest]

No worries, thanks for the time you've spent on it already, @bzbarsky. If @heycam has time, brilliant. If not, January seems like a fine time to clean up the patch.

one thing that I think needs to be clarified is that [SecureContext] does not dynamically
change exposure with the secure context state

I'll try to clean up the text in the [SecureContext] section. I talked around that point, but I failed to state it explicitly.

[annevk]

Is not exposing things even compatible at this point? If we cannot make document.domain have an effect because Facebook uses it too much, what makes us think we can change the exposure rules for a bunch of APIs limited to secure contexts? Or do we only do that for future APIs? If so, then it seems we can couple it with document.domain too.

[esprehn]

I don't think we want to make secure contexts have different API surface than non-secure ones. That makes creating heap snapshots difficult since you'd need different ones for the secure vs non-secure cases which means larger binary size. It's also not very friendly to developers, the API surface of Chrome version X should be uniform in the wild.

[jeisinger]

@esprehn, the heap snapshot doesn't contain DOM APIs and if we ever got to the point where we can add some, I think we can pick and chose

[mikewest]

@bzbarsky: Could you help me figure out whether or not the current patch is sane?

@annevk: I suspect we can only tag new APIs with this flag, or APIs that were new at a time when developers were consistently checking whether they existed (e.g. ServiceWorker). You're right that that might give us more of an opportunity to guard against document.domain, but at the same time the usage numbers are fairly hopeless: https://www.chromestatus.com/metrics/feature/timeline/popularity/739 shows ~5% of page views. Some chunk of that is Facebook, some other chunk of it is cargo-culted scripts that aren't actually used, but I'm sure some chunk is used and isn't going away. shrug I'm not sure that fight is the right place to spend our time.

@esprehn: The argument that @bifurcation, @martinthomson, @travisleithead, and others have advanced here and elsewhere is that secure context restrictions ought to fall into developer's feature-detection-handling paths, and not into their error-handling paths. That is, developers are already writing feature-detection code to determine if they can do the thing they'd like to do: if ('serviceWorker' in navigator) { and the like. Removing the API from the surface area we offer developers would be an honest depiction of the feature's availability in a context.

The fact that the APIs might change behavior from context to context is a given; the question is how we help developers understand those changes. shrug The Mozilla and IE folks who have weighed in seem to have preferred this approach, and I don't feel strongly enough about throwing to find it worthwhile pushing back against arguments that just don't match my preference (but seem otherwise sane). :)

[bzbarsky]

@mikewest The "can transition" section should probably explicitly say that things remain exposed, right? Or more precisely, the definition of exposure should use the secure context state at the moment the script environment is created.

It doesn't make sense to talk about an interface being "executed". What can happen is that an interface member or constructor will be "executed". Also, I assume this section is generally informative, not normative, and should be clearly labeled as such. This section claims the normative behavior is defined in "perform a security check", but that's not what actually happens; it's a separate step, right? It might in fact be better to move it into "perform a security check" to avoid all the copy/paste...

For the bits talking about when operations are exposed... I don't think that's the right way to do that. I think the right way is to change the definition of "exposed" at http://heycam.github.io/webidl/#dfn-exposed and that should do what we want. That would presumably also handle attribute exposure, which is not handled in this patch afaict.

Apart from that, seems reasonable.

[jwatt]

Note that the document.domain stuff has just been removed from the Secure Contexts spec and will be a separate attribute to [SecureContext] if we decide to pursue that. See w3c/webappsec-secure-contexts@40985fc and whatwg/html#829

[heycam]

Working on finishing off @mikewest's patch: 8f0ff87

Since the document.domain stuff has been removed, does that mean we can avoid all of the SecurityException throwing and rely only on exposure? Or are there still ways to get access to the Function objects from elsewhere and call them on objects in the non-secure context?

[ghost]

Note additional discussion here:

https://lists.w3.org/Archives/Public/public-script-coord/2016JanMar/0104.html

On Tue, Mar 22, 2016 at 2:06 AM, Cameron McCormack <notifications@github.com

wrote:

Working on finishing off @mikewest https://github.com/mikewest's patch:
8f0ff87
8f0ff87

Since the document.domain stuff has been removed, does that mean we can
avoid all of the SecurityException throwing and rely only on exposure? Or
are there still ways to get access to the Function objects from elsewhere
and call them on objects in the non-secure context?

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#65 (comment)

[mikewest]

Oh thank you thank you @heycam. I haven't touched this PR in a long time because it's huge and I'm buried. I will be your best friend forever if you finish it for me. :)

[mikewest]

@heycam: If we're not doing document.domain, and we continue doing the opener restrictions outlined in the current spec, then I don't think there's any way to get a handle to a same-origin context that is a secure context if you aren't yourself a secure context. That should allow us to simplify things in the way you're suggesting.

[heycam]

Great. So from the public-script-coord thread I think using exposure is the approach to go ahead with. @bzbarsky could you do a quick review of 8f0ff87?

[annevk]

I now use this annotation in https://storage.spec.whatwg.org/, FWIW.

[heycam]

OK, let's just land this and if there are issues we can fix them up later.

[heycam]

Fixed in 710b36c.