ietf
[Top] [All Lists]

Re: Gen-ART LC review of draft-ietf-httpstate-cookie-18

2010-11-29 17:18:47
Hey Adam,

Thanks for following up.  Responses inline.

[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/

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.


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?

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."


[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.

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.


[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.

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.


[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.

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.


[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.

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"?


[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.

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.


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

See my response to S4.1.1 P1.

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"?


[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.

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 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).
"


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.

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>


[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.
]]

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."


[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".
]]

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.


[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.

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."  


[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?

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.

Cheers.
--Richard




_______________________________________________
Ietf mailing list
Ietf(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/ietf