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
cookies: leave secure cookies alone (draft-ietf-httpbis-cookie-alone-01) #2956
cookies: leave secure cookies alone (draft-ietf-httpbis-cookie-alone-01) #2956
Conversation
But yes, a test case that also verifies that it works as intended would be really good! |
Do you think this should be added to an existing test, or should a new test be added? |
I'm fine with either way. Whatever you find easiest. |
dac8ee2
to
a377072
Compare
Rebased, test added and bugs fixed |
56f757b
to
a322771
Compare
The Appveyor redness comes from 1506 failing which I believe is unrelated |
Regarding the replacement policy for when a non-secure cookie is allowed to replace a secure cookie, the document states this for path comparisons:
This leaves some room for implementation details IMHO. I have interpreted it in the most conservative way. The code implementing this could definitely need a set of eyes to ensure it's sane. |
#HttpOnly_127.0.0.1 FALSE /p4/ TRUE 0 httpandsec4 myvalue5 | ||
#HttpOnly_127.0.0.1 FALSE /p4/ TRUE 0 httpandsec3 myvalue4 | ||
#HttpOnly_127.0.0.1 FALSE /p4/ TRUE 0 httpandsec2 myvalue3 | ||
#HttpOnly_127.0.0.1 FALSE /p4/ TRUE 0 httpandsec myvalue2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason for these large sections removed is that they are setting secure cookies over HTTP, which with this patch is no longer allowed.
As a reminder to myself for when the window opens: this patch also needs to remove item 5.8 from the TODO list. Will add that before this is pushed. |
a322771
to
a7a483a
Compare
Only allow secure origins to be able to write cookies with the 'secure' flag set. This reduces the risk of non-secure origins to influence the state of secure origins. This implements IETF Internet-Draft draft-ietf-httpbis-cookie-alone-01 which updates RFC6265. Closes curl#2956
Only allow secure origins to be able to write cookies with the 'secure' flag set. This reduces the risk of non-secure origins to influence the state of secure origins. This implements IETF Internet-Draft draft-ietf-httpbis-cookie-alone-01 which updates RFC6265. Closes curl#2956
a7a483a
to
739deca
Compare
Only allow secure origins to be able to write cookies with the 'secure' flag set. This reduces the risk of non-secure origins to influence the state of secure origins. This implements IETF Internet-Draft draft-ietf-httpbis-cookie-alone-01 which updates RFC6265.
This is an old patch I dusted off which I wouldn't mind eyes on.
There needs to be tests added of course which is on my TODO.Update: this used to be marked WIP as it lacked the required tests. These have now been added and the PR title is updated to reflect this.