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

net/http: support Cookie "SameSite" attribute #15867

Closed
haoxins opened this Issue May 28, 2016 · 21 comments

Comments

Projects
None yet
@hkeide

This comment has been minimized.

Show comment
Hide comment
@hkeide

hkeide Jun 4, 2016

Since this is already supported in Chrome 51, and it won't be in Go for a while yet, here is a simple workaround (tested to work in Chrome 51):

cs := w.Header().Get("Set-Cookie")
cs += "; SameSite=lax"
w.Header().Set("Set-Cookie", cs)

hkeide commented Jun 4, 2016

Since this is already supported in Chrome 51, and it won't be in Go for a while yet, here is a simple workaround (tested to work in Chrome 51):

cs := w.Header().Get("Set-Cookie")
cs += "; SameSite=lax"
w.Header().Set("Set-Cookie", cs)
@quentinmit

This comment has been minimized.

Show comment
Hide comment
@quentinmit
Contributor

quentinmit commented Jun 17, 2016

@quentinmit quentinmit added this to the Unplanned milestone Jun 17, 2016

@bradfitz bradfitz changed the title from http/cookie: support same-site attribute to net/http: support Cookie "SameSite" attribute Jun 25, 2016

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Jun 25, 2016

Member

This seems trivial, but it also seems like we should wait until there's more web consensus. Chrome can pull or modify support, but our Go 1 compatibility promise is stronger. It would be unfortunate if we added a SameSite bool field to net/http.Cookie and then they renamed it yet again before it became fully standardized.

Member

bradfitz commented Jun 25, 2016

This seems trivial, but it also seems like we should wait until there's more web consensus. Chrome can pull or modify support, but our Go 1 compatibility promise is stronger. It would be unfortunate if we added a SameSite bool field to net/http.Cookie and then they renamed it yet again before it became fully standardized.

@kardianos

This comment has been minimized.

Show comment
Hide comment
@kardianos

kardianos Sep 6, 2016

Contributor

SameSite would probably need to be an const (Strict, Lax).

Would it make sense to serialize Cookie.Unparsed into the cookie string? Then I can just set Unparsed: []string {"SameSite=Strict"},.

Contributor

kardianos commented Sep 6, 2016

SameSite would probably need to be an const (Strict, Lax).

Would it make sense to serialize Cookie.Unparsed into the cookie string? Then I can just set Unparsed: []string {"SameSite=Strict"},.

@Unknwon Unknwon referenced this issue Feb 14, 2017

Open

Cookie security #3525

2 of 6 tasks complete
@JaredWilliams

This comment has been minimized.

Show comment
Hide comment
@JaredWilliams

JaredWilliams Apr 11, 2017

Alternative hack allowing for multiple cookies.

type SameSite string

const (
    SameSiteLax    SameSite = "lax"
    SameSiteStrict          = "strict"
)

func SetCookie(w http.ResponseWriter, cookie *http.Cookie, sameSite SameSite) {
    if v := cookie.String(); v != "" {
        switch sameSite {
        case SameSiteLax:
            v = v + "; SameSite=lax"
        case SameSiteStrict:
            v = v + "; SameSite=strict"
        }
        w.Header().Add("Set-Cookie", v)
    }
}

JaredWilliams commented Apr 11, 2017

Alternative hack allowing for multiple cookies.

type SameSite string

const (
    SameSiteLax    SameSite = "lax"
    SameSiteStrict          = "strict"
)

func SetCookie(w http.ResponseWriter, cookie *http.Cookie, sameSite SameSite) {
    if v := cookie.String(); v != "" {
        switch sameSite {
        case SameSiteLax:
            v = v + "; SameSite=lax"
        case SameSiteStrict:
            v = v + "; SameSite=strict"
        }
        w.Header().Add("Set-Cookie", v)
    }
}
@reedloden

This comment has been minimized.

Show comment
Hide comment
@reedloden

reedloden Apr 19, 2017

Keep in mind there are actually three different possible values here. SameSite, SameSite=Lax, and SameSite=Strict. SameSite without a value means the same as SameSite=Strict.

Updated spec: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00

reedloden commented Apr 19, 2017

Keep in mind there are actually three different possible values here. SameSite, SameSite=Lax, and SameSite=Strict. SameSite without a value means the same as SameSite=Strict.

Updated spec: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00

reedloden added a commit to reedloden/go that referenced this issue Apr 19, 2017

net/http: Add support for Same-site cookies
The same-site cookie attribute prevents a cookie from being sent along with
cross-site requests. The main goal is mitigate the risk of cross-origin
information leakage and provides some protection against cross-site request
forgery attacks.

Spec: https://tools.ietf.org/html/draft-ietf-httpbis-cookie-same-site-00

Fixes #15867

XXX: Write tests.
@blitzprog

This comment has been minimized.

Show comment
Hide comment
@blitzprog

blitzprog Jun 17, 2017

It's been a year now and the name hasn't changed.
I believe it should be added to the standard lib.

blitzprog commented Jun 17, 2017

It's been a year now and the name hasn't changed.
I believe it should be added to the standard lib.

@mbyczkowski

This comment has been minimized.

Show comment
Hide comment
@mbyczkowski

mbyczkowski Aug 12, 2017

Contributor

It's still only supported by Chrome and Opera and the IETF draft (both, orignal and the updated spec) has expired last year. ¯\_(ツ)_/¯

Contributor

mbyczkowski commented Aug 12, 2017

It's still only supported by Chrome and Opera and the IETF draft (both, orignal and the updated spec) has expired last year. ¯\_(ツ)_/¯

srenatus added a commit to srenatus/scs that referenced this issue Nov 27, 2017

add SameSite support
Note that upstream support is still pending [0] (and even when merged, this
will require the latest Go version). So this adds a workaround to allow
for setting SameSite on the cookies used, as per the draft [1].

Also changes the options_test TestCookieOptions calls to `t.Errorf`,
since this will run all checks even if one fails. (`t.Fatal[f]` would
stop test execution.)

[0]: golang/go#15867
[1]: https://tools.ietf.org/html/draft-west-first-party-cookies-07

Signed-off-by: Stephan Renatus <srenatus@chef.io>
@gopherbot

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot Nov 27, 2017

Change https://golang.org/cl/79919 mentions this issue: net/http: Add support for SameSite cookie option

gopherbot commented Nov 27, 2017

Change https://golang.org/cl/79919 mentions this issue: net/http: Add support for SameSite cookie option

@kardianos

This comment has been minimized.

Show comment
Hide comment
@kardianos

kardianos Nov 27, 2017

Contributor

I had to remove the samesite cookie attribute even for chrome due to https://bugs.chromium.org/p/chromium/issues/detail?id=626245 .

Until bugs like this are resolved in chrome, I don't see any viable implementation. Not one, zero.

Contributor

kardianos commented Nov 27, 2017

I had to remove the samesite cookie attribute even for chrome due to https://bugs.chromium.org/p/chromium/issues/detail?id=626245 .

Until bugs like this are resolved in chrome, I don't see any viable implementation. Not one, zero.

@srenatus

This comment has been minimized.

Show comment
Hide comment
@srenatus

srenatus Nov 27, 2017

Contributor

@kardianos oh that's a bummer. Maybe we should close the change then.

Contributor

srenatus commented Nov 27, 2017

@kardianos oh that's a bummer. Maybe we should close the change then.

@kardianos

This comment has been minimized.

Show comment
Hide comment
@kardianos

kardianos Nov 27, 2017

Contributor

@srenatus You can leave it open and put R=go1.11 in a comment line so it won't show up on a dashboard until then. If it does become a standard, the bug will eventually get fixed. It just might not be ready now. Go can't accept the change for now anyway as it is in a freeze.

Contributor

kardianos commented Nov 27, 2017

@srenatus You can leave it open and put R=go1.11 in a comment line so it won't show up on a dashboard until then. If it does become a standard, the bug will eventually get fixed. It just might not be ready now. Go can't accept the change for now anyway as it is in a freeze.

@srenatus

This comment has been minimized.

Show comment
Hide comment
@srenatus

srenatus Nov 27, 2017

Contributor

@kardianos done -- thanks 😃

Contributor

srenatus commented Nov 27, 2017

@kardianos done -- thanks 😃

@agnivade

This comment has been minimized.

Show comment
Hide comment
@agnivade

agnivade Mar 14, 2018

Member

ping on this.

OAWSP site says

As of November 2017 the SameSite attribute is implemented in Chrome, Firefox, and Opera.

caniuse.com is still pretty red but FF and Chrome support is there.

Member

agnivade commented Mar 14, 2018

ping on this.

OAWSP site says

As of November 2017 the SameSite attribute is implemented in Chrome, Firefox, and Opera.

caniuse.com is still pretty red but FF and Chrome support is there.

@andybons

This comment has been minimized.

Show comment
Hide comment
@andybons

andybons Mar 14, 2018

Member

60% usage globally is still pretty low given that Chrome is much more cavalier about removing features.

Member

andybons commented Mar 14, 2018

60% usage globally is still pretty low given that Chrome is much more cavalier about removing features.

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Mar 15, 2018

Member

Still not in Edge or Safari, and still only a draft standard. I still think we should wait. There's no rush and people can't use string concatenation in the meantime.

Member

bradfitz commented Mar 15, 2018

Still not in Edge or Safari, and still only a draft standard. I still think we should wait. There's no rush and people can't use string concatenation in the meantime.

@reedloden

This comment has been minimized.

Show comment
Hide comment

reedloden commented Apr 24, 2018

@bradrydzewski

This comment has been minimized.

Show comment
Hide comment
@bradrydzewski

bradrydzewski commented Jul 9, 2018

FYI this shipped in Edge 18 and in the Edge 17 June security patch
https://developer.microsoft.com/en-us/microsoft-edge/platform/status/samesitecookies/

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Jul 9, 2018

Member

If somebody wants to send a change for this for Go 1.12, that's fine.

Member

bradfitz commented Jul 9, 2018

If somebody wants to send a change for this for Go 1.12, that's fine.

@agnivade

This comment has been minimized.

Show comment
Hide comment
@agnivade

agnivade Jul 9, 2018

Member

I believe CL 79919 is already there. Probably needs to be bumped to 1.12.

Member

agnivade commented Jul 9, 2018

I believe CL 79919 is already there. Probably needs to be bumped to 1.12.

@golang golang deleted a comment from gopherbot Jul 9, 2018

@gopherbot gopherbot closed this in 3d5703b Jul 9, 2018

@ptman

This comment has been minimized.

Show comment
Hide comment
@ptman

ptman Aug 21, 2018

unless I'm interpreting something wrong the samesite code seems to be in 1.11, no need to wait for 1.12

ptman commented Aug 21, 2018

unless I'm interpreting something wrong the samesite code seems to be in 1.11, no need to wait for 1.12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment