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

SameSite Setting for Cookies #5583

Closed
2 of 7 tasks
jakeshaffer opened this issue Dec 23, 2018 · 8 comments · Fixed by #14900
Closed
2 of 7 tasks

SameSite Setting for Cookies #5583

jakeshaffer opened this issue Dec 23, 2018 · 8 comments · Fixed by #14900
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@jakeshaffer
Copy link

Description

The SameSite setting should be enabled on the session and CSRF cookies as an added prevention against CSRF. Mozilla does a good job of explaining its purpose, but the gist is that it prevents cookies being sent in a request initiated from a foreign origin.

Screenshots

N/A

@silverwind
Copy link
Member

Yes, this is a good idea, also see gogs/gogs#3525 (comment) for other cookie options.

@techknowlogick techknowlogick added type/enhancement An improvement of existing functionality type/proposal The new feature has not been accepted yet but needs to be discussed first. issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented labels Jan 31, 2019
@tbleiker
Copy link

What is the state on this?

@silverwind
Copy link
Member

Not implemented to my knowledge. Browsers are moving towards enabling it by default, so the issue might solve itself eventually:

https://blog.chromium.org/2019/10/developers-get-ready-for-new.html

@alexanderadam
Copy link

alexanderadam commented Mar 18, 2020

Browsers are moving towards enabling it by default, so the issue might solve itself eventually:

But this is not true for other settings that can increase security, right?

For example the others you mentioned, like HttpOnly and (see comment below) Secure

@dpertin
Copy link

dpertin commented Mar 30, 2020

@alexanderadam HttpOnly option is implemented since #4706.
However, according to Mozilla Observatory, it seems to be the only one:
cookie_gitea

@alexanderadam
Copy link

alexanderadam commented Mar 31, 2020

Ah okay, I didn't know that. Thank you for pointing out, Dimitri. 👍

@dpertin
Copy link

dpertin commented Apr 5, 2020

While waiting for these flags to be set at the application level, I found a workaround which consists in setting cookie flags at the web server level. For instance with nginx, one could adapt the configuration sample provided by the documentation by using the proxy_cookie_path directive as such:

server {
    (...)

    location / {
        # Workaround to set cookie flags:
        proxy_cookie_path / "/; Secure; HttpOnly; SameSite=lax";

        proxy_pass http://localhost:3000;
    }
}

Which results in:
cookie_gitea2

If your instance of gitea is supposed to be accessed with an unsecure channel (HTTP), remove the Secure flag. I also set SameSite to Lax which provides a reasonable level of protection. Strict mode implies higher security rules but it might have potential drawbacks since it prevents the browser from attaching any cookies to cross-site requests. I do not know gitea enough to determine if it could be a problem.

@m-ueberall
Copy link

FYI, the corresponding apache2 directive is ProxyPassReverseCookiePath – just put this below the ProxyPassReverse directive from the configuration sample:

ProxyPassReverseCookiePath / "/; HttpOnly; Secure; SameSite=lax"

Here, the HttpOnly; specifically addresses issue #9690 (lang cookie) en passant

zeripath added a commit to zeripath/gitea that referenced this issue Mar 5, 2021
Fix go-gitea#5583

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit that referenced this issue Mar 7, 2021
Add SameSite setting for cookies and rationalise the cookie setting code. Switches SameSite to Lax by default. 

There is a possible future extension of differentiating which cookies could be set at Strict by default but that is for a future PR.

Fix #5583

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants