Use SameSite=strict cookies consistently #1450
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1450 +/- ##
==========================================
- Coverage 35.42% 34.66% -0.76%
==========================================
Files 126 126
Lines 30635 30622 -13
Branches 4733 4733
==========================================
- Hits 10851 10614 -237
- Misses 19220 19361 +141
- Partials 564 647 +83
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This is complicated.
In theory, SendCookie() function allows modules to send cookies to browser.
In practice, the only cookie sent is the session ID.
Session ID needs SameSite for sure. But the custom modules' cookies might suffer from SameSite, from HttpOnly, depending on their use case.
Perhaps ZNC cookies API should be changed, when the need for such cookies arises. Until this happens, +1 from me on adding SameSite the way you did.
src/HTTPSock.cpp
Outdated
| @@ -743,7 +743,7 @@ bool CHTTPSock::PrintHeader(off_t uContentLength, const CString& sContentType, | |||
| for (const auto& it : m_msResponseCookies) { | |||
| Write("Set-Cookie: " + it.first.Escape_n(CString::EURL) + "=" + | |||
| it.second.Escape_n(CString::EURL) + "; HttpOnly; path=/;" + | |||
| (GetSSL() ? "Secure;" : "") + "\r\n"); | |||
| (GetSSL() ? "Secure;" : "") + " SameSite=strict;\r\n"); | |||
There was a problem hiding this comment.
Shouldn't it be Strict with capital S? To follow the example from RFC draft you linked.
Yup, ideally there'd be a cookie API that would let you specify the path, whether or not to set the HttpOnly flag, the SameSite value etc. |
Background
Cross-site request forgery is a common security issue that affects many web applications. ZNC mitigates the risk of CSRF in the same manner as many other applications - by making use of a token mechanism. A
<input type="hidden" name="_CSRF_Check" value="token">field is included in all forms and when the browserPOSTs the data, the token is checked. This ensures that a cross-originPOSTwill be unsuccessful because the malicious site will not know the token's value.SameSite
The
SameSiteattribute can be applied to cookies. Browser such as Chrome and Opera (which implement the spec) will refuse to send the cookie along with potentially dangerous requests which originate from another domain.SameSitecan work in two modes of operation:GET.This PR marks all cookies set by ZNC to be SameSite strict. This choice was made following the principle of least privilege - I cannot think of a compelling reason not to apply
SameSite=strictto ZNC's cookies - it seems unlikely a user would want to deeplink a page within the ZNC administration panel from another origin, for example. If you have a reason to disagree or believe this will cause problems, please do post a reply and elaborate on your use-case.Not a silver bullet
SameSite should be used as part of a defense in depth approach. It is not a replacement for the existing token-based approach at migitaging the risk of CSRF. Firefox does not yet have support for this attribute but a tracking bug exists.
Further reading
SameSiteflag: https://tools.ietf.org/html/draft-west-first-party-cookies-07SameSite: https://www.owasp.org/index.php/SameSite