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

Add Last-Event-ID as CORS-safelisted request-header #597

Closed
wants to merge 1 commit into from

Conversation

annevk
Copy link
Member

@annevk annevk commented Sep 4, 2017

It turns out that you can set the Last-Event-ID request header to arbitrary values and get it across origins. That seems like sufficient reason to safelist it and hopefully make it clear to server administrators to pay extra attention to this header.

Tests: ...

Fixes #568.


Preview | Diff

It turns out that you can set the Last-Event-ID request header to arbitrary values and get it across origins. That seems like sufficient reason to safelist it and hopefully make it clear to server administrators to pay extra attention to this header.

Tests: ...

Fixes #568.
@sideshowbarker
Copy link
Contributor

Doesn’t this also need browser bugs?

Copy link
Contributor

@sideshowbarker sideshowbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I preemptively updated the MDN CORS article to include this change https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS

@annevk
Copy link
Member Author

annevk commented Sep 5, 2017

Yeah, it'll need browser bugs. And I suppose it does theoretically increase the attack surface a little bit by doing this, as you'll be able to control more than just Last-Event-ID once this is safelisted, but I don't think that's sufficient reason not to do it.

@annevk annevk requested review from mikewest and yoavweiss March 18, 2018 10:13
@annevk annevk added needs tests Moving the issue forward requires someone to write tests needs implementer interest Moving the issue forward requires implementers to express interest labels Mar 18, 2018
@annevk annevk added the security/privacy There are security or privacy implications label Apr 12, 2018
@annevk
Copy link
Member Author

annevk commented Jun 1, 2018

This will need a new plan on top of #736. Given that I'm not sure this is still worth pursuing.

@annevk
Copy link
Member Author

annevk commented Jun 1, 2018

Closing this, will leave the issue open for now.

@annevk annevk closed this Jun 1, 2018
@annevk annevk deleted the annevk/last-event-id branch June 1, 2018 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs implementer interest Moving the issue forward requires implementers to express interest needs tests Moving the issue forward requires someone to write tests security/privacy There are security or privacy implications
Development

Successfully merging this pull request may close these issues.

2 participants