w3c / webappsec-fetch-metadata

Fetch Metadata
https://w3c.github.io/webappsec-fetch-metadata/
Other
75 stars 28 forks source link

Naming for new items in `mode`. #45

Closed mikewest closed 3 years ago

mikewest commented 5 years ago

At TPAC, @clelland suggested that we could future-proof the mode mechanism by shifting from nested-navigate to navigate-nested, which would allow developers to read navigate- as a prefix. This dovetails with the suggestion in #37 that we might want to distinguish <object> in mode.

How would you feel about this in your implementation, @arturjanc / @lweichselbaum?

@annevk, FYI.

annevk commented 5 years ago

Should object/embed be navigate-from-object/navigate-from-embed or no-cors-from-object/no-cors-from-embed? If I remember correctly the initial request is no-cors and subsequent would be navigate-nested.

arturjanc commented 5 years ago

@jerryzz0 may have thoughts based on his implementation of the logic in our applications.

At a high level, the thing that's important for security is distinguishing navigations from subresource loads, to prevent attacks on resources without breaking linking (with the idea that you can use COOP to prevent leaking information about non-renderable responses / subresources if they are navigated to directly in a top-level window).

To future-proof against changes in mode it might be useful to maintain a guarantee that navigational requests will be easily distinguishable from resource loads and any other requests, e.g. that non-navigational requests never contain "navigate" and navigational requests always contain "navigate". This seems like it could be simple enough, without putting too may requirements on future values of mode, e.g. if (hypothetically) Portals should get a new mode, it could be navigate-portal.

For object/embed I'm still not a great fan of cramming the destination into the mode, but practically it would be okay if we did something like no-cors-from-object. I feel somewhat more strongly about this being no-cors-something rather than navigate-something because:

So overall no-cors-from-X for the initial object/embed request and establishing navigate-X as the mode for navigations seems like it would work.

jerryzz0 commented 5 years ago

I like the idea of a prefix hierarchy as:

  1. The relationship with navigate is a bit clearer - particularly in a sorted list of header values.
  2. It could simplify application-side implementations of "isRequestNavigational".

That being said, it's also not that big of a deal since whether this is future-proof on the application side depends on the specific server-side policy in place (e.g. navigation-blocking vs resource-blocking) i.e.

Since we anticipate resource-blocking to be a primary use case (over navigation-blocking), I would prefer to optimize for that case if we had to choose between navigate-from-object vs no-cors-from-object.

My preference is that if we decide to change nested-navigate to navigate-nested then:

  1. We do so soon while these headers are still relatively new - since dealing with backwards/cross-browser compatibility can be a pain.
  2. We should also change the nested-document destination (potentially exposed as part of the still debated Sec-Fetch-Dest header) to document-nested for consistency.
annevk commented 5 years ago

It might make sense that for hierarchical values we use a different separator as the hyphen is already used for another purpose. E.g., no-cors/from-object or navigate/nested. This would be somewhat novel for enumeration values though (https://w3ctag.github.io/design-principles/#casing-rules doesn't mention it at least). But for writing simple parsers for them it seems really rather good. E.g., "navigate".split("/") yields a single value, "navigate/nested".split("/") yields two.

cc @domenic

mikewest commented 5 years ago

Hrm. If we're going to be .split()ing the value, then it seems like we might really be talking about two things: the mode on the one hand (navigate, no-cors, etc.), and the way the response will be used on the other (nested, top-level, etc.). I think it makes more sense to do that disjunction explicitly, as Sec-Fetch-Dest aimed to do.

Would it be reasonable to drop nested-navigate, and instead introduce something like Chromium's FrameOwnerElementType enum as a distinct header? (e.g. Sec-Fetch-Mode: nested-navigate would instead be represented by Sec-Fetch-Mode: navigate; Sec-Fetch-X-Bikeshed-Browsing-Context-Container-Type: iframe, or something similar). I think I'd prefer to do that, rather than risk falling into the weird naming hole that we ended up with in referrer policy.

annevk commented 5 years ago

That's fair and maybe that's an argument against that syntax. I worry that by using separate headers we make it easier to create faulty policies that forget about embed and object. (Or indeed forget about nesting.)

mikewest commented 5 years ago

I also worry that folks will forget about things they ought to check (I wanted everything in one big header, as you'll recall... ;) ). But given that we just shipped this a minute ago, and the folks who are using it at scale a. haven't really started yet, and b. are pretty much all on this thread, I'm pretty confident that we can both change things up and document them in such a way that developers will understand what they need to do. Checking three headers isn't significantly more complicated than checking two (where the second really splits along two axes). I think that's still my preference.

If y'all disagree, I can certainly be convinced that ever-more-complicated mode entries are reasonable, but it seems like we're jamming things in there just because it already exists.

annevk commented 5 years ago

I guess I'd love to see what it would look like concretely. My worry is that with -Dest it's not really clear how it relates to -Mode or that a mode of no-cors can turn into a navigation.

mikewest commented 5 years ago

I guess I'd love to see what it would look like concretely.

Strawproposal:

Indeed. This is confusing, and it may be better to explicitly label this as navigation-possibly-depending-on-how-you-respond-dearest-server, as it does feel like it might be a distinct kind of request with different behavior than "navigation".

I'm less convinced that nested navigations (or portaled navigations?) are distinct in the same way. It seems like we might want to advertise the latter to give sites choices about what code to send (no reason to check for portalHost, etc. if you know you're not being portaled), but the higher-order choice of whether or not you'd like to be framed seems possible by checking Sec-Fetch-Whatever's value (or lack thereof). That feels like good contextual information, but not a distinct mode...

annevk commented 5 years ago

Okay, so the -Whatever header is only for being framed or the possibility of getting framed and only has four values (the four HTML framing element local names). -Frame seems reasonable in that case.

mikewest commented 5 years ago

Okay, so the -Whatever header is only for being framed or the possibility of getting framed and only has four values (the four HTML framing element local names).

Right. Something like an enum of frame, iframe, object, embed, portal, and whatever we collectively invent tomorrow. I think we'd define it in terms of the request's reserved client's target browsing context's container's tag name? Something like that...

-Frame seems reasonable in that case.

Sec-Fetch-Frame-Owner might be more descriptive? Especially if we add more "frame" attributes as #43 hints at...

mikewest commented 5 years ago

@arturjanc, @jerryzz0, could y'all live with this bisection of Sec-Fetch-Mode? Is it a terrible idea?

jerryzz0 commented 5 years ago

If I understand correctly, the proposal is to:

  1. Keep Sec-Fetch-Mode largely the same but collapse the current navigate and nested-navigate into a single navigate value.

  2. Keep Sec-Fetch-Site unchanged.

  3. Introduce Sec-Fetch-Frame-Owner (or something equivalent) to distinguish between the various framing scenarios.

I think this works. In particular:

In other words, in practice it'd function as a (similar but not identical) substitute to Sec-Fetch-Dest which does not seem like a terrible idea.

it may be better to explicitly label this as navigation-possibly-depending-on-how-you-respond-dearest-server

Hmm that could be nice and might make things easier to understand on the application side; but then Sec-Fetch-Mode wouldn't be a simple reflection of the Request Mode anymore - at which point the question arises as to whether Sec-Fetch-Mode makes more sense than a (as a hypothetical strawman) Sec-Fetch-Is-Navigational with values of yes/no/maybe.

annevk commented 5 years ago

@jerryzz0 Sec-Fetch-Site is unchanged and Sec-Fetch-Mode would largely be the same. I think you interchanged them initially. Note that if we changed the -Mode enumeration values I would expect us to also change request's mode concept. It's not exposed in many places so likely changes can still be made there.

arturjanc commented 5 years ago

Mike's proposal above sounds mostly reasonable to me. My feedback is similar to what @jerryzz0 mentioned above; specifically:

  1. The bisection of mode is positive, it can simplify the restrictions logic. The main use of mode is to allow cross-site navigations while preventing resource loads, so if the value provides a clear signal that the response will be used in a navigation, that's better.

One other possibility could be to reduce the number of values that are reported in Sec-Fetch-Mode to make application code more resilient to new modes added in the future. At the basic level, we probably need navigate, cors and no-cors, but I wonder if might make sense to guarantee that they are the only possible values for Sec-Fetch-Mode, i.e. either never create more modes, or map any others to one of these three core values just for the purposes of Sec-Fetch-Mode.

For example, the only other existing mode (same-origin) could easily be replaced with no-cors in Sec-Fetch-Mode because site already carries the information about the requesting origin. Similarly, if there's ever a successor to CORS that introduces a new mode, we could internally map it to cors. If we did that, then maybe we should use more generic terms such as navigation, subresource-with-opt-in, subresource-without-opt-in.

  1. I have nothing against Sec-Fetch-Frame-Owner, but it seems to perform the function of Sec-Fetch-Dest only without providing information about subresource requests. If we think it's valuable to tell developers about whether a navigation happens in an iframe or object then it seems similarly useful to tell them whether a response will be used as a script or style and it would make the header behave more consistently.

At a high level, we'd end up in a place where Sec-Fetch-Site and Sec-Fetch-Mode are a set of well defined enums (making the basic restrictions logic simple), and Sec-Fetch-Frame-Owner/Sec-Fetch-Dest would provide more detailed information about the requesting element/API, creating an escape hatch for developers if their responses are being legitimately used cross-site in a specific way, e.g. as an object (in which case listing allowed values isn't particularly error-prone because the developer knows exactly what they need to allow).

  1. I'd like to keep object and embed as no-cors instead of a navigation or a new navigation-like mode -- this approach would let servers fail closed and reject cross-site loads via these elements by default. In applications which want to support cross-site loads of renderable responses via object, developers can use the value of Sec-Fetch-Frame-Owner/Sec-Fetch-Dest to allow no-cors requests if they happen via an object; but this seems like a rare case and I don't think we should optimize for it.
mikewest commented 5 years ago
  1. My intuition is that there's clarity in keeping the Sec-Fetch-Mode definition 1:1 with Fetch's mode definition.

  2. I agree with you that Sec-Fetch-Frame-Owner is basically a less-granular Sec-Fetch-Dest. With that in mind, I would prefer to just ship the latter, honestly. Both seem to have the same challenge with regard to service worker interposition. If we've decided that that's not a problem (because all service worker initiated requests will be Sec-Fetch-Dest: fetch; Sec-Fetch-Site: cross-site), great; let's just ship Sec-Fetch-Dest?

  3. Sounds reasonable.

mikewest commented 5 years ago

For clarity, here's my current proposal:

  1. Drop nested-navigation from Sec-Fetch-Mode, aligning it completely with Fetch's current mode enum.

  2. Split Fetch's document destination into document for top-level navigation and nested for <frame> and <iframe> navigation (as I don't think there's much value in matching embed and object's granularity here with iframe and frame?).

    Presumably, Portals and whatever comes after them would define distinct destinations.

  3. Ship Sec-Fetch-Dest with those new values.

Any objections to this approach? (@annevk, @arturjanc?)


Top-level navigations would send:

Sec-Fetch-Mode: navigation
Sec-Fetch-Dest: document

<iframe> and <frame> navigations would send:

Sec-Fetch-Mode: navigation
Sec-Fetch-Dest: nested // I'm totally open to bikeshedding this name, btw...

<object> requests would send:

Sec-Fetch-Mode: no-cors
Sec-Fetch-Dest: object

This basically punts on Service Worker cooperation. They'll be able to take cached responses and reuse them in unexpected ways, and launder fetch() responses into responses. IMO, this risk is managable, as if a Service Worker has cached a response, it's already received it, and the risks of receiving it (Spectre, et al) are already accepted. Likewise, if a server responds with sensitive information to a fetch() request, the Service Worker's interposition doesn't enhance the risk.

arturjanc commented 5 years ago

Sounds good to me, so if @annevk is on board I think we should do this.

A couple of thoughts about this (mostly about adding explanatory text in the spec):

  1. I'm not particularly worried about attackers laundering requests through Service Workers. Based on my testing this isn't currently possible and it wouldn't affect the main use cases we're after. We should probably document this concern in the spec, though, and also mention that destination values are likely to change in the future.
  2. We are essentially punting on future-proofing mode, which is okay, but, as above, maybe we should explicitly say in the spec that new mode values may affect existing isolation logic. Would it make sense to also mention this in Fetch so that folks who introduce new mode or destination values are aware that servers may act on them?
  3. On a parallel issue @annevk asked about reference logic that uses the values of Sec-Fetch-* headers. Should we consider adding this as a non-normative note in the Fetch Metadata spec, or write a separate short doc with details? This could help clarify the impact of future changes to the values of mode and destination.
  4. The tiniest of bikesheds: nested stands out a bit from the current destination values which are all nouns. The original nested-document or frame/iframe may be slightly cleaner.
annevk commented 5 years ago

I like -Frame-Owner better as it puts more emphasis on the risky scenarios, but can live with -Dest I suppose. (I also agree with @arturjanc's tiniest bikeshed.)

mikewest commented 5 years ago

@annevk: I agree with you about the value of -Frame-Owner's potential messaging. But I also see value in the wider array of type information contained in destination. If y'all are on board for that, I'd still prefer it. I'll poke at the spec shortly.

@arturjanc:

  1. :+1:
  2. I'm happy to add some text in this document. @annevk probably has opinions about a note in Fetch (and, really, we still need to figure out how much of this document really ought to live in Fetch).
  3. Send a PR with an "Implementation Considerations" section with helpful advice for developers? Perhaps y'all could wrap up the isolation policies on https://secmetadata.appspot.com with some helpful text about when and why they might be useful?
  4. Ok. frame/iframe then, with embed/object as the model?
arturjanc commented 5 years ago

3. Send a PR with an "Implementation Considerations" section with helpful advice for developers? Perhaps y'all could wrap up the isolation policies on https://secmetadata.appspot.com with some helpful text about when and why they might be useful?

Yep, will do, hopefully sometime this month.

4. Ok. frame/iframe then, with embed/object as the model?

Ship it :)

jugglinmike commented 4 years ago
Sec-Fetch-Mode: navigation
Sec-Fetch-Dest: nested // I'm totally open to bikeshedding this name, btw...

You'll need to set an expiry on invitations like this.

@domenic recently referenced a trend for new headers to use the HTTP Structured Headers syntax. If that's applicable here, then rather than nested or nested-document, we might consider

Sec-Fetch-Mode: navigation
Sec-Fetch-Dest: document; nested
jugglinmike commented 4 years ago

I spoke too soon. We're actually extending the request's associated "destination" with the new values, so parameterizing "document" isn't appropriate, after all.

annevk commented 3 years ago

This can be closed. We settled on changing request's destination concept a bit.

mikewest commented 3 years ago

Indeed. Thanks!