Subject: Re: comments on draft-west-first-party-cookies-07

From: =JeffH <[email-masked]>
Date: Mon, 27 Jun 2016 18:48:08 -0700

Hi again,

Quoting Mike West <[email-masked]> [from the very end of Mike's
message, bringing this up to the front here..]
  >
  > I've made a number of changes in..

[1]
https://github.com/httpwg/http-extensions/commit/263db30abb6f0f0b5a0cc3c2c3424108b3c171bc

[2]
https://raw.githubusercontent.com/httpwg/http-extensions/master/draft-ietf-httpbis-cookie-same-site.md

[ and he also uploaded..

[3] https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site

..tho note that [2] contains edits not yet manifested in [3] ]

  > ..which I hope addresses the [below]. I'll make more changes to the
  > various algorithms in patches to HTML, Fetch, URL, et al.

Ok, I reviewed older mailing list discussions (eg [6], below), which i
hadn't earlier (sorry), and have some musings (below, at [4]) wrt the
interfaces between 6265bis, [FETCH], [HTML], RFC7230.

  > On Sat, Jun 18, 2016 at 12:21 AM, <[email-masked]>
  > wrote:
  >
[...]
  >>
  >> * RFC6265 does not wrap its algorithm variables in double quotes
  >> (as this draft is doing), and also hyphenates multi-word variable
  >> names (even ones that aren't ABNF rule names). Are you suggesting
  >> that 6265bis ought to adopt the style used in this draft (where alg
  >> varbs are quoted), or will the below proposed updates to 6265bis
  >> adopt RFC6265's present style?
  >>
  >> I advocate for the latter -- e.g., I suggest: s/"site for
  >> cookies"/site-for-cookies/g
  >>
  >
  > I'm used to W3C specs, where we can actually mark up variables in a
  > way that gives them semantic meaning. I'm happy to align this
  > document with IETF style; if hyphenated names are preferred, I'll
  > start adding hyphens.

yeah, I think we'll want to be minimally-invasive when doing rfc6265bis
and so adopting its style will lower impedance mismatch.

though, I notice that in [2], you substituted backquotes `...` for
double quotes around algorithm variables, cookie attrs, etc, and that
_might_ work if applied throughout 6265bis, we'd have to see what other
reviewers think (rfc-editor), etc.

  >> * RFC6265 uses the terms host, server, domain in its algorithms,
  >> whereas this spec introduces the terms site, same site, top-level
  >> site, "site for cookies". This may cause impedance mismatches when
  >> attempting to merge this draft into 6265bis. Perhaps just something
  >> to be aware of and address at that time.
  >>
  >
  > Agreed. I suspect we'll want to move some of these definitions
  > elsewhere (HTML, Fetch, and/or URL, for instance).

ok, this intersects with musings (below at [4]) wrt the interfaces
between 6265bis, [FETCH], [HTML], RFC7230.

  >> * when comparing host names, or portions thereof, they ought to
  >> first be canonicalized per RFC6265 S 5.1.2, yes?
  >
  > Yes.

Ok, tho I do not see canonicalization added within [1] or [2] as yet
(there's indications where it might be added in my original comments, HTH)

[...]

  >>> 1.1. Goals
  >>
  >> might these be items that should be added into 6265bis' various
  >> "considerations" sections (as appropriate)?
  >
  > I think that's reasonable in the combined document, but in a
  > stand-alone document targeting a specific feature, it seems
  > reasonable to be explicit about that feature's raison d'etre right up
  > front.

agreed.  I am only suggesting that there ought to be a Note or something
on each (sub)section of -cookie-same-site (nee -first-party-cookies)
(and other cookie-patching I-Ds) that is intended to be material for
inclusion in 6265bis.

[...]
  >>> depth against attacks which require embedding an authenticated
  >>> request into an attacker-controlled context:
  >>>
  >>> 1. Timing attacks which yield cross-origin information leakage
  >>> (such as those detailed in [pixel-perfect]) can be substantially
  >>> mitigated by setting the "SameSite" attribute on authentication
  >>> cookies. The attacker will only be able to embed unauthenticated
  >>> resources, as embedding mechanisms such as "<iframe>" will yield
  >>> cross-site requests.
  >>>
  >>> 2. Cross-site script inclusion (XSSI) attacks are likewise
  >>> mitigated by setting the "SameSite" attribute on authentication
  >>> cookies. The attacker will not be able to include authenticated
  >>> resources via "<script>" or "<link>", as these embedding
  >>> mechanisms will likewise yield cross-site requests.
  >>
  >> do you actually mean `<script src="..." />` rather than
  >> `<script>...</script>` here?
  >>
  >
  > Yes, but I'm not sure it's valuable to make that distinction. I meant
  > to refer to "the <script> tag" here, as that's the mechanism by which
  > the request would be triggered.

from my perspective, after having been thrown into the websec deep-end
not terribly long ago, it is very helpful if specs can go a tad extra
distance to be explicit, rather than be implicit and rely upon the
reader intuitively realizing what is being implied.

perhaps writing it like..

    2. Cross-site script inclusion (XSSI) attacks are likewise
    mitigated by setting the "SameSite" attribute on authentication
    cookies. For example, an attacker will not be able to include
    authenticated resources via the "<script>" tag (with a "src"
    attribute) or the "<link>" tag (with a "href" attribute), as
    these embedding mechanisms will likewise yield cross-site
    requests.

..would work?

  >> 1.2. Examples
  >>>
  >>> Same-site cookies are set via the "SameSite" attribute in the
  >>> "Set-
  >>
  >> s/set/declared/ ?
  >>
  >
  > The header is called `Set-Cookie`. Given that, "set" seems like the
  > right verb here.

ok.

  >> > 2.  Terminology and notation
  >>
  >> is the intention that the terminology in this immediate section
  >> would be added to 6265bis' section 2.3 "Terminology" ?
  >>
  >
  > Yup. Or elsewhere, as noted above. I've talked a bit with Anne about
  > moving some things from here into HTML/URL/Fetch, and I think he's
  > amenable.

Links to discussion...?

  >>> The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL
  >>> NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and
  >>> "OPTIONAL" in this document are to be interpreted as described in
  >>> [RFC2119].
  >>>
  >>> This specification uses the Augmented Backus-Naur Form (ABNF)
  >>> notation of [RFC5234].
  >>>
  >>> Two sequences of octets are said to case-insensitively match
  >>> each other if and only if they are equivalent under the
  >>> "i;ascii-casemap" collation defined in [RFC4790].
  >>>
  >>> The terms "active document", "ancestor browsing context",
  >>> "browsing context", "document", "WorkerGlobalScope", "sandboxed
  >>> origin browsing context flag", "parent browsing context", "the
  >>> worker's Documents", "nested browsing context", and "top-level
  >>> browsing context" are defined in [HTML].
  >>
  >> add to the above list:
  >>
  >>               Document, shared worker, dedicated worker
  >>
  >
  > Document was already in the list (though uncapitalized), I've added
  > the other two. Thanks!

I am suggesting adding the capitalized Document term, in addition to the
non-capitalized document term, since the former is distinct from the
latter IIUC, and both are used in -cookie-same-site, yes?

  >>> The term "request", as well as a request's "client", "current
  >>> url", "method", and "target browsing context", are defined in
  >>> [FETCH].
  >>
  >> suggest adding request's "initiator" to the list above since it is
  >> used in opening parag of next section
  >
  > Actually, I need to rename the thing I want to talk about here.
  > Fetch defines an "initiator" which isn't what I want. What I want is
  > the origin of the request's client.

Ok, and I see in [2], that you fixed up the  opening parag of next
section to not use "initiator"

  >> Are the below definitions & algorithms in S 2.1 et al slated to be
  >> inserted into 6265bis, e.g., in 6265bis S 5 "User Agent
  >> Requirements"?

?

  >> Also, before diving into same-/cross-site, document-based,
  >> worker-based requests and all, I suggest adding a section here
  >> defining the overall concept of site-for-cookies...
  >>
  >>
  >> 2.1 The Site-for-Cookies Concept
  >>
  >>    A request is the input to the HTML Fetch algorithm [FETCH].
  >>    Initiators of requests have an associated origin, which may
  >>    contain a host name. If so, then the host name contains a
  >>    registered domain, which is the top-most domain on which
  >>    the initiator server-side may set its cookies. Thus this
  >>    registered domain is the initiator's site-for-cookies,
  >>    which is used in determining whether a request is same-site
  >>    or not.

although the terminology in the above suggested parag is incorrect per
your feedback, if the above were to be edited appropriately and
incorporated, I think it'd help clarify this spec (and ultimately this
portion of the already-complex 6265bis).

  >>> 2.1. "Same-site" and "cross-site" Requests
  >>>
  >>> A request is "same-site" if its target's URI's origin's
  >>> registrable domain is an exact match for the request's
  >>> initiator's "site for cookies", and "cross-site" otherwise. To be
  >>> more precise, for a given request ("request"), the following
  >>> algorithm returns "same- site" or "cross-site":
  >>>
  >>> 1. If "request"'s client is "null", return "same-site".
  >>>
  >>> 2. Let "site" be "request"'s client's "site for cookies" (as
  >>> defined in the following sections).
  >>>
  >>> 3. Let "target" be the registrable domain of "request"'s current
  >>> url.
  >>>
  >>> 4. If "site" is an exact match for "target", return "same-site".
  >>>
  >>> 5. Return "cross-site".
  >>
  >>
  >> ISTM there's various issues with the above, below is a suggested
  >> overall revision.
  >>
  >> also, I wonder whether it is a good idea to have this particular
  >> definition to be characterized as an algorithm if they are not
  >> themselves actually incorporated within the normative 6265bis
  >> cookie-processing algorithms. Thus the below is not characterized
  >> as an algorithm...
  >>
  >
  > Thank you for this thoughtful reformulation. I think I'll take a
  > stab at moving these around out of this document first, and will try
  > to incorporate your feedback into those patches to the external
  > documents on which this relies.

by external docs, I take it you mean [FETCH] and [HTML] (and perhaps
[URL]), more down at [4]...

  >>> 3.1. Grammar
  >>>
  >>> Add "SameSite" to the list of accepted attributes in the
  >>> "Set-Cookie" header field's value by replacing the "cookie-av"
  >>> token definition in Section 4.1.1 of [RFC6265] with the following
  >>> ABNF grammar:
  >> >
  >> >    cookie-av      = expires-av / max-age-av / domain-av /
  >> >                     path-av / secure-av / httponly-av /
  >> >                     samesite-av / extension-av
  >> >    samesite-av    = "SameSite" / "SameSite=" samesite-value
  >>                         ^^^^^^^
  >>       this allows for a SameSite attr without a value?
  >>       is that actually used somewhere in the below algorithms?
  >
  > No, this is https://github.com/mikewest/internetdrafts/issues/11
  > <https://github.com/mikewest/internetdrafts/issues/11> :) which I'll
  > fix now that I finally got around to uploading a -00 draft with the
  > contents of the existing -07.

Ok, this fix in [2]..

      samesite-av    = "SameSite=" samesite-value
      samesite-value = "Strict" / "Lax"

..looks fine.

[...]
  >>> 4.1.  The "SameSite" attribute
  >>>
  >>>    The following attribute definition should be considered part of
  >>>    the the "Set-Cookie" algorithm as described in Section 5.2 of
  >>>    [RFC6265]: If the "attribute-name" case-insensitively matches the
  >>>    string "SameSite", the user agent MUST process the "cookie-av" as
  >>>    follows:
  >>>
  >>>    1. If "cookie-av"'s "attribute-value" is not a case-sensitive
  >>>       match for "Strict" or "Lax", ignore the "cookie-av".
  >>>
  >>>    2. Let "enforcement" be "Lax" if "cookie-av"'s "attribute-value"
  >>>       is a case-insensitive match for "Lax", and "Strict" otherwise.
  >>>
  >>>    3. Append an attribute to the "cookie-attribute-list" with an
  >>>       "attribute-name" of "SameSite" and an "attribute-value" of
  >>>       "enforcement".
  >>
  >> oh, so enforcement here is just a local algorithm variable? and
  >> step 3 is actually saying to use the value of enforcement as the
  >> attr-value for SameSite ? if so, perhaps ought to be clarified..
  >>
  >
  > I understand sections 5.2.* of RFC6265 to be accepting an
  > attribute-name and attribute-value string (see step 6 of that
  > document's 5.2). The attribute that's appended to
  > 'cookie-attribute-list' has its own attribute name and value.

understood, and...

  > I agree that this is a little confusing, and that we could make the
  > expected inputs/outputs clearer when we revise the document.

...nevermind, it's ok as-is now that I've re-reviewed RFC6265 sections
5.2.1 - 5.2.6.

[...]
  >> >    offer a robust defense against CSRF as a general category of
  >>>     attack:
  >> >
  >> >    1.  Attackers can still pop up new windows or trigger top-level
  >> >        navigations in order to create a "same-site" request (as
  >> >        described in section 2.1), which is only a speedbump along
  >> >        the road to exploitation.
  >> >
  >> >    2.  Features like "<link rel='prerender'>" [prerendering] can be
  >> >        exploited to create "same-site" requests without the risk
  >> >        of user detection.
  >> >
  >>>     When possible, developers should use a session management
  >>>     mechanism such as that described in Section 5.2 to mitigate the
  >>>     risk of CSRF more completely.
  >>
  >> "Strict" enforcement is not explicitly defined?
  >>
  >> hm, i guess it is defined in S 3.2 -- perhaps add a x-ref to that?
  >>
  >
  > Clarified this in the first sentence (followed by a link to 5.2).

ah, you clarified this in [2] -- looks fine.

  >>> 4.2.  Monkey-patching the Storage Model
  >>>
  >>> Note: There's got to be a better way to specify this. Until I
  >>> figure out what that is, monkey-patching!
  >>>
  >>> Alter Section 5.3 of [RFC6265] as follows:
  >>>
  >>> 1. Add "samesite-flag" to the list of fields stored for each
  >>> cookie.
  >>
  >> s/Add/Add (in the first paragraph)/ -- the list of fields is in the
  >> first parag of S 5.3
  >>
  >
  > Poked at this.

ah, you clarified this in [2] -- looks fine.

[...]

  >>>  1. If the "cookie-attribute-list" contains an attribute with an
  >>>     "attribute-name" of "SameSite", set the cookie's "samesite-flag"
  >>>     to "attribute-value" ("Strict" or "Lax"). Otherwise, set the
  >>>     cookie's "samesite-flag" to "None".
  >>>
  >>>  2. If the cookie's "samesite-flag" is not "None", and the request
  >>>     which generated the cookie's client's "site for cookies"..
  >>
  >> the term "cookie's client('s)" is used only here (and also isn't
  >> defined in RFC6265) -- what is it's definition?
  >>
  >
  > It's actually referring to "request's client" from FETCH.
  > "request-which-generated-the-cookie's client". This would, I'm sure,
  > be simpler in German. :)

[4] ah. thx. Some thoughts wrt [FETCH], [HTML], RFC7230, RFC7231, and..

[5]  Clarify the hooks into RFC6265 #221
https://github.com/whatwg/fetch/pull/221/commits/5a324a891c42d42de09a01e03c3a063b9a4f882b 

[6] Re: Defining First and Third Party Cookies
https://lists.w3.org/Archives/Public/ietf-http-wg/2016JanMar/0249.html

..and I'm (also) wondering about how to appropriately divide
responsibilities between -cookie-same-site/6265bis and [FETCH]+[HTML].

Not all HTTP clients implement [FETCH]+[HTML], correct?  Yet such HTTP
clients may employ cookies, yes?

So maybe the correct thing to do is to further enhance [FETCH]+[HTML]
similarly to how you did in [5], and to keep -cookie-same-site/6265bis
as free of [FETCH]+[HTML] particulars as possible..?

And perhaps have some language (as approp) in -cookie-same-site/6265bis
saying something like "user agents implementing [FETCH]+[HTML] ought to
do such-and-such, otherwise do this-and-this..." ?

also, 6265bis will also need to be updated to adopt RFC7230 terminology
e.g. request-uri seems to now be "target URI".

[...]

  >> > 5.  Authoring Considerations
  >>
  >> this section is intended to be added to 6265bis?
  >>
  >
  > That's up to the group. I think it makes sense to describe how we
  > intend for developers to actually use this thing.

by "developers" here, you mean "webapp developers, aka 'authors'", yes?

  > It certainly makes
  > sense in the context of this document, and I think it probably also
  > makes sense as a new section in 6265bis that describes authoring
  > considerations with this and other attributes.

agreed.

[...]

  >>> 5.3.  Mashups and Widgets

[...]

  >> >
  >> > 6.1.  Server-controlled
  >> >
  >>
  >>>   of certain kinds of attacks that the server is worried about.
  >>>   The user is not involved in this decision. Moreover, a number of
  >>>   side- channels exist which could allow a server to link distinct
  >>>   requests even in the absence of cookies.
  >>
  >> you mean to say "..could allow a third-party server to link
  >> distinct top-level navigation requests to first-party servers, even
  >> in the absence of cookies." ?
  >>
  >
  > "top-level navigation" is too limiting. Servers receive channel
  > ID/token binding data for any request, for instance, not just
  > navigational requests.

Ok...

  >>>                                Connection and/or socket pooling,
  >>>    Token Binding, and Channel ID all offer explicit methods of
  >>>    identification that servers could take advantage of.
  >>
  >> I'm not sure the above sentence is applicable to the third-party
  >> cookie discussion in RFC6265 S 7.1. My understanding is that Token
  >> Binding and ChannelID present server-specific identifiers and
  >> cannot be used by a third-party server to track the UA's top-level
  >> navigation requests to various first-party servers.
  >>
  >
  > Both these mechanism send origin-specific data to a server as part
  > of a request. They have similar properties to cookies, in that they
  > can be used to correlate requests in a top-level or nested or
  > subresource context. They're not only sent along with top-level
  > requests, and their privacy concerns aren't limited to those
  > top-level contexts (e.g. `google.com` in a frame gets a channel ID as
  > part of the TLS handshake. The same ID, in fact, that it receives in
  > a top-level context).

Ok, thx, I think I now more get (at least some of) the subtleties.  will
have to think about this in the context of the token-binding specs...

[...]
  >> end

HTH,

=JeffH

[FETCH] https://fetch.spec.whatwg.org/

[HTML] https://html.spec.whatwg.org/