Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate first party cookies, cookie prefix, and insecure modification of secure cookies #8700

Open
jdm opened this issue Nov 27, 2015 · 5 comments

Comments

@DemiMarie
Copy link

@DemiMarie DemiMarie commented Dec 1, 2015

Does this subsume #7962 ?

@jdm
Copy link
Member Author

@jdm jdm commented Dec 1, 2015

Unclear! It's good to link them together as you have, however.

@jdm
Copy link
Member Author

@jdm jdm commented Feb 3, 2017

#14445 and #14491 should have implemented the "leave secure cookies alone" specification.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Nov 11, 2017

I managed to find the cookie prefixes draft to be incorrect in its implementation details. The draft says the following:

After step 10 of the current algorithm, the cookies flags are set.
Insert the following steps to perform the prefix checks this document
specifies:
[...]
2. If the "cookie-name" begins with the string "$Host-", abort these
steps and ignore the cookie entirely unless the following
conditions are true:
* The cookie's "host-only-flag" is "true"
* The cookie's "path" is "/"

The cookie's path at this point is never empty, since the original RFC6265 section 5.3 step 7 always sets the cookie path to a default value.

@KiChjang
Copy link
Member

@KiChjang KiChjang commented Nov 15, 2017

Let me clarify why the observation above is important - the draft says that these cookies should always be rejected:

   Set-Cookie: $Host-SID=12345
   Set-Cookie: $Host-SID=12345; Secure
   Set-Cookie: $Host-SID=12345; Domain=example.com
   Set-Cookie: $Host-SID=12345; Domain=example.com; Path=/
   Set-Cookie: $Host-SID=12345; Secure; Domain=example.com; Path=/

Note that the first 3 do not specify a Path attribute. This following cookie is accepted, however:

   Set-Cookie: $Host-SID=12345; Secure; Path=/

Now, the 2nd rejected cookie above and this accepted cookie differs in only one aspect - the existence of the Path attribute. Since RFC6265 defines a step that always sets the Path attribute to a default value, this would cause the rejected cookie to be accepted, if we were to follow the draft proposal in verbatim.

bors-servo added a commit that referenced this issue Nov 23, 2017
Implement secure and host cookie prefixes

Part of #8700.

I modified the algorithm so that it accurately checks for the presence of the `Path` attribute of the cookie, before checking whether it has a value of `/`.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19185)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Nov 23, 2017
Implement secure and host cookie prefixes

Part of #8700.

I modified the algorithm so that it accurately checks for the presence of the `Path` attribute of the cookie, before checking whether it has a value of `/`.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19185)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.