Subject: Re: [http-state] Gen-ART LC review of draft-ietf-httpstate-cookie-18

From: Adam Barth <[email-masked]>
Date: Mon, 29 Nov 2010 12:25:46 -0800

Thanks for the review.  Comments inline.

On Sun, Nov 28, 2010 at 12:47 PM, Richard L. Barnes <[email-masked]> wrote:
> I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>
> Please resolve these comments along with any other Last Call comments you may receive.
>
> Document: draft-ietf-httpstate-cookie-18
> Reviewer: Richard Barnes
> Review Date: 28 November 2010
> IETF LC End Date: 2 December 2010
> IESG Telechat date: (unknown)
>
> Summary: This document is close to being ready for publication, but could benefit from several clarifications and upgrades to requirements language.  In particular, user agent security requirements need to be slightly stronger.
>
>
> Major issues:
>
> [S8.6] Many of these integrity issues are caused by user agents accepting cookies from outside the context where they would send them, in particular with the Secure and Path attributes.  It seems like this document, in specifying the desired/proper behavior of user agents, should require behavior that would mitigate these attacks.  In direct parallel to the handling of the Domain attribute:
> 1. Set-Cookies with the Secure flag should only be accepted over secure channels
> 2. Set-Cookies with the Path attribute set should only be accepted if the path value matches the request-uri of the request
> (That is, update Steps 7 and 8 of S5.3 to match Steps 6 and 9/10) I realize that as long as there are legacy user agents out there, servers can't rely on these protections, but if any user agents implement these requirements, then fewer transactions will be subject to these attacks.

I agree that these would be desirable changes to the cookie protocol.
However, the http-state working group is not chartered to change the
cookie protocol.  In particular, the charter reads as follows:

[[
The working group must not introduce any new syntax or new semantics
not already in common use.
]] --- https://datatracker.ietf.org/wg/httpstate/charter/

These changes would introduce new semantics not already in common use.

> Minor issues:
>
> [General] It would be very helpful if this document had a summary of changes from RFC 2965, and possibly from RFC 2109 as well.  In particular, the fate of the Cookie2 and Set-Cookie2 header fields is a little unclear.  If the intent of obsoleting 2965 is to deprecate the use of these fields, it would be good to state that explicitly.

The plan is to obsolete RFC 2965 and move it to historic(al).  Is
there a better way of stating that the use of Cookie2 and Set-Cookie2
is deprecated?

> [General] It's unclear where this document fits on the scale of "documenting existing behavior" to "describing proper behavior".  It seems like the focus should be on documenting a proper behavior that is maximally compatible with existing behavior.  The document should try to clearly distinguish when it is talking about the desired behavior vs. the existing behavior.  The former should be subject to strong requirements "MUSTs" while the latter will have no requirements at all.

I'm not sure I understand the distinction between the two.  The
document describes a protocol that matches, to a large extent,
existing implementations of the protocol.  Where existing user agents
differ from the specified protocol in significant ways, the document
contains a note explaining the difference.

> [S2.3 P2] Is the request-host the same as the value of the HTTP/1.1 Host header field?

No.  For example, the HTTP/1.1 Host header field sometimes contains
the port whereas the request-host never contains the port.

> [S3 P4] It seems like there should be an all-caps requirement here, e.g., that "Origin servers MUST NOT fold multiple Set-Cookie header fields into a single header field".

Origin servers certainly can fold multiple Set-Cookie header fields
into a single header field if they desire.  However, doing so changes
the semantics.  Given the bytes on the wire, there's no experiment we
can run to see if the origin server did or did not perform this
folding.  Therefore, a MUST-level requirement is inappropriate.

> [S4.1.1 P1] It seems like it should be MUST NOT instead of SHOULD NOT: "Servers MUST NOT send Set-Cookie headers that fail to conform to the following grammar:".

This requirement was discussed at length in the working group.  I
advocated for a MUST-level requirement here as well.  However, some
members of the working group felt strongly that the server ought to be
able to make use of the full cookie protocol as specified in Section
5.  Therefore, we reduced this requirement from a MUST to a SHOULD.

> [S4.1.1] Is there a reason that path-value is so unconstrained?  Based on S5.1.4, I would have expected it to be abs_path (from RFC 2396).

The document used to refer to abs_path, but someone in the working
group pointed out some cases where this was incorrect.  We tried a
couple different variations, but none of them were satisfactory.  For
this reason, we ended up with the relatively permissive requirement
you see today.  I don't recall all the details, but they're in the
mailing list archive.

> [S4.1.1 P5] Seems like the SHOULD NOT should be "MUST NOT produce more than one attribute with the same name".

See my response to S4.1.1 P1.

> [S4.1.1 P6] Either this should be a "MUST NOT" or it should define which a user agent MUST accept -- or both.  The third paragraph of 4.1.2 seems to imply that the user agent MUST accept the last one (in order of appearance in the HTTP header).

The user agent requirements are contained in Section 5.

> [S4.1.1 P8] Again, "SHOULD" -> "MUST"

See my response to S4.1.1 P1.

> [S8.1] I find this paragraph vague and confusing.  I would suggest separating protocol vulnerabilities from implementation vulnerabilities.  Suggested text:
> "
> Transport-layer encryption, such as that employed in HTTPS, is insufficient to prevent a network attacker from obtaining or altering a victim's cookies.  The cookie protocol itself allows cookies to be accessed or modified from other contexts (subdomains, subordinate paths, or other ports) and some user agents do not even provide the separations required by the protocol.  (See "Weak Confidentiality" and "Weak Integrity", below).
> "

I'm not sure I agree with your diagnosis.  All the issues described in
that paragraph are vulnerabilities in the cookie protocol.  None of
them are implementation vulnerabilities.

> Nits/editorial comments:
>
> [S2.1 P3 "not intended to be performant"] I'm not sure the word "performant" has the meaning that seems to be intended here.  (The definitions I'm finding indicate "one who performs".)  Should probably expand to "not intended to require the performance of specific steps".

This text is boilerplate commonly used in a number of specifications.
Like the goofy grammar in the RFC2119 boilerplate, it's easier if we
all just use exactly the same text.

> [S3 P2 "the default scope"] The notion of the "scope" of a cookie was only briefly introduced in the Introduction.  Could be good to remind the reader in the first paragraph.  "... the user agent will return in future HTTP requests." -> "... the user will return in future HTTP requests that are within the scope of the cookie".

Done.

> [S4.1.1 P9] The paragraph as it stands is kind of useless -- what am I supposed to do with this information?  Suggest changing to:
> "
> Some user agents represent dates using 32-bit UNIX time_t values.  These user agents will process dates after January 19, 2038 incorrectly.  Origin servers SHOULD NOT set expiration times later than this time.
> "

There was a lot of discussion of this paragraph in the working group.
At one point we had text similar to what you propose, but various
folks objected.  The text we have now is the compromise we reached.

> [S4.1.2] The phrase "Non-Normative" seems inapt here.  Suggest changing the heading to something like "Summary of Semantics".

That text is there to emphasize the fact that the text in that section
is non-normative.  Remove that phrase might confuse readers into
thinking that the section actually describes the semantics of the
protocol (which it does not).

> [S4.1.2.3 P1 "... only to the origin server"] It would be helpful to clarify here by adding "... and not any sub-domains".

Done (slightly tweaked).

> [S4.1.2.6] It could be helpful here to note that the "HttpOnly" attribute is not the opposite of the "Secure" attribute (which one might initially imagine as "HttpsOnly"), and thus that setting them both is not a contradiction.

Done.

> [S5.1.1] Just as in Section 5.2, it would help to explain here why you're not just using the rfc1123 grammar (from above and RFC 2616 S3.1.1).  E.g.:
> "
> NOTE: This grammar is more general than the sane-cookie-date grammar used in the definition of the Set-Cookie header. This allows user agents to interoperate with servers that do not implement this specification.
> "

This is already explained two paragraphs earlier:

[[
      For historical reasons, the full semantics of cookies (as presently
      deployed on the Internet) contain a number of exotic quirks. This section
      is intended to specify the Cookie and Set-Cookie headers in sufficient
      detail to allow a user agent implementing these requirements precisely to
      interoperate with existing servers.
]]

> [S5.1.1] It would be helpful to introduce the found-* flags at the beginning of step 2.

They're already introduced:

[[
          Note that the various boolean
          flags defined as a part of the algorithm are initially "not set".
]]

> [S5.1.1 Step5] "the cookie-date if" -> "the cookie-date if any of the following conditions are true:"

Making that change would make that sentence grammatically incorrect.

> [S5.1.1 Step5] I don't understand why the year 1601 is a cutoff here.  Wouldn't any past date be usable to kill a cookie?

This requirement is based on how user agents behave.  I suspect dates
prior to 1601 are more likely to be errors.  For example, a date of
128 is unlikely to actually refer to the year 128.  In any case, it
doesn't matter why we use 1601 as a cut off, just that we do.

> [S8.2] It might be helpful to have a reference or two here, e.g., explaining XSRF attacks and mitigations.

Sure.  Which references do you suggest?

Thanks again,
Adam

---

From: Adam Barth <[email-masked]>
Date: Mon, 29 Nov 2010 16:04:05 -0800

On Mon, Nov 29, 2010 at 3:18 PM, Richard L. Barnes <[email-masked]> wrote:
> I was thinking something obvious at the end of Section 1, like maybe "In particular, in moving RFC 2965 to Historic and obsoleting it, this document deprecates the use of the Cookie2 and Set-Cookie2 header fields."

Done.

> Ok, I think I get it now: The all-caps requirements in this document are basically a maximally safe/interoperable profile of existing practice.

Yes.

> It might be helpful to state this explicitly in the Introduction (toward the end, after "actually used on the Internet"):
> "
> In particular, this document does not create new syntax or semantics beyond those in use today, but neither does it recommend all of the syntactic and semantic variations in use today.  The recommendations in this document represent a preferred subset of current behaviors.  Where some existing software differs from the recommended protocol in significant ways, the document contains a note explaining the difference.
> "
> That's a little wordy, but I think it captures what's meant here.

Done.

> As I understand it, folding would cause multiple Set-Cooking header values to be considered instead as setting one cookie with many attributes.  Is there a situation where this is "acceptable or even useful", to quote RFC 2119?  It seems to me like folding is virtually guaranteed to be a bug.

Ok.  I've changed this to a SHOULD.  I suspect, actually, that this
requirement already exists in the document (in an obtuse way) because
the recommend grammar for servers won't produce a "," in the right
syntactic surrounds to be a folded Set-Cookie header.

> Seems like the principle of being conservative in what you send applies here, but if this was considered and rejected by the working group, I'm not hard over.

It's difficult for me to accept your recommendation here because I
agree with you, but the working group decided to go the other way.  We
could try to raise the issue again, but it doesn't seem worthwhile.

> Same response applies -- given that the server doesn't know how these multiple attributes will be interpreted by a user agent, is there a situation where this is "acceptable or even useful"?

Acceptable, of course, is a normative notion.  We're defining what is
acceptable in this document.  We can define whatever we like to be
acceptable.  Usefulness, however, is extrinsic.  I don't think it's
particularly useful to send duplicate attribute values, but, on the
other hand, nothing terribly bad happens if you do.

I don't feel that strongly about it either way.  My sense is that
having this be the one MUST-level requirement for servers is kind of
silly though.

> Specifically, in S5.3, Step 11.  Might help to note when it does make sense for this to happen, i.e., when the domain or path differ, and conversely, to warn that there's no point to sending more than one Set-Cookie with all three of these the same.

I'm not sure that's needed.  I suspect we could think of use cases for
multiple redundant Set-Cookie headers if we thought about it long
enough.

> Same response applies -- given that the server doesn't know how a non-rfc1123 will be interpreted by a user agent, is there a situation where this is "acceptable or even useful"?

Well, here there are tons of servers that send every kind of nutty
date format imaginable.  Does that mean its acceptable or even useful,
I'm not sure.  I suspect they find it useful because they happen to
have the date sitting around in that format.  Saying servers MUST use
rfc1123 would just be a joke.

> I understood phrases like "Cookies do not always provide isolation by path" and the corresponding examples to mean that browsers are not enforcing the constraints implied by the attributes (e.g., the Path attribute).  Do you mean to say while setting the Path attribute restricts access via the specific channel of the Cookie header, it does not restrict access by other methods, e.g., client-side scripting?

If one thing is isolated form another, it should have both its
confidentiality and integrity protected.  Cookies provide some amount
of confidentiality but not very much in the way of integrity.

Also, notice, that because this specification describes what user
agents do, it's somewhat non-sensical to say that user agents are not
enforcing the constraints implied by the attributes.  If they weren't,
we'd updated the list of constraints.

> If that's the case, suggest the following:
> "
> Transport-layer encryption, such as that employed in HTTPS, is insufficient to prevent a network attacker from obtaining or altering a victim's cookies.  The cookie protocol itself allows cookies to be accessed or modified by other HTTP services (subdomains, subordinate paths, or other ports).  In addition, restrictions set within the cookie protocol are applied only to access via the Set-Cookie and Cookie headers, and are not necessarily enforced for other mechanisms of accessing a user agent's cookie store (e.g., client-side scripting).   (See "Weak Confidentiality" and "Weak Integrity", below).
> "

You keep trying to shift the blame elsewhere.  The problem is not
other mechanisms for accessing cookies.  The problems are in this
document here.  The current paragraph doesn't mince words.  This
protocol, as widely used and important as it is, has some pretty bad
security properties.  Somehow, the world hasn't ended yet, though.

> Ah, I didn't read it that way. Maybe just add a particular note at the end of that: "The grammars used in this section are thus more general than those recommended for servers in Section 4."

I think that's more confusing than useful.  In particular, there is
only one grammar in this section, and its just the universal grammar.

> Just think it might make for easier reading and implementation if I knew up front what booleans I have to be keeping track of, instead of discovering their names as I go through.

Done.

> Elementary Rule of Usage number 7 from Strunk & White (4th edition):
> "Use a colon after an independent clause to introduce a list of particulars, anappositive, an amplification, or an illustrative quotation."

Thanks for quoting Strunk & White, but notice that the current text
does not have a colon.  It's actually just one sentence broken over
multiple lines (with some fancy bullets thrown in for good measure).

> I don't have any especially good ideas.  How about this one?  :)
> <http://www.adambarth.com/papers/2008/barth-jackson-mitchell-b.pdf>
> ... or one of its many references.

Haha, now you're just trying to flatter me.  Done.

Adam

---

From: "Richard L. Barnes" <[email-masked]>
Date: Mon, 29 Nov 2010 18:18:23 -0500

Hey Adam,

Thanks for following up.  Responses inline.

> ]] --- https://datatracker.ietf.org/wg/httpstate/charter/

Thanks for the clarification; I wasn't aware of that charter text.  That also helps address my comment on "observed" vs. "desired" behaviors below.

> These changes would introduce new semantics not already in common use.

I guess that would be the case, if you interpret the recommendations as adding an "set-cookie authorization" semantic to the two parameters, in addition to the "provide-cookie authorization" semantic. Ok, that makes sense.

> contains a note explaining the difference.

Ok, I think I get it now: The all-caps requirements in this document are basically a maximally safe/interoperable profile of existing practice.  It might be helpful to state this explicitly in the Introduction (toward the end, after "actually used on the Internet"):
"
In particular, this document does not create new syntax or semantics beyond those in use today, but neither does it recommend all of the syntactic and semantic variations in use today.  The recommendations in this document represent a preferred subset of current behaviors.  Where some existing software differs from the recommended protocol in significant ways, the document contains a note explaining the difference.
"
That's a little wordy, but I think it captures what's meant here.

> all just use exactly the same text.

I'll take your word for it.  I don't see any other examples in I-Ds or RFCs, or in the obvious Google search:
<http://tools.ietf.org/googleresults?cx=011177064926444307064:rsqif7nmmi0&q=performant&sa=Google+Search&cof=FORID:9>
<http://www.google.com/search?sourceid=chrome&ie=UTF-8&q=%22performant%22>

Cheers.
--Richard

---

From: "Richard L. Barnes" <[email-masked]>
Date: Mon, 29 Nov 2010 20:45:36 -0500

Hey Adam,

I'm OK letting the remainder of these go as you prefer.  Thanks for working this through with me.

--Richard