From 636be396e4f3ecc09e446ca76db380fc0c5b57f9 Mon Sep 17 00:00:00 2001 From: Gunnar Mills Date: Mon, 15 Mar 2021 12:47:07 -0500 Subject: Add SameSite=Strict on Set-Cookie Set SameSite to Strict since OpenBMC does not have functionality that requires Lax or None. SameSite Strict provides a little protection against CSRF attacks by ensuring the cookie is only sent to requests originating from the same site that set the cookie. This came from some discussion on discord. From https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie SameSite= Optional Controls whether a cookie is sent with cross-origin requests, providing some protection against cross-site request forgery attacks Inline options are: Strict: The browser sends the cookie only for same-site requests (that is, requests originating from the same site that set the cookie). If the request originated from a different URL than the current one, no cookies with the SameSite=Strict attribute are sent. Lax: The cookie is not sent on cross-site requests, such as calls to load images or frames, but is sent when a user is navigating to the origin site from an external site (e.g. if following a link). This is the default behavior if the SameSite attribute is not specified. None: The browser sends the cookie with both cross-site and same-site requests. The Secure attribute must also be set when SameSite=None! Note: On Firefox 85, FireFox still doesn't have the Default set to SameSite=Lax. This can be changed via "about:config" and "network.cookie.sameSite.laxByDefault". Tested: Webui-vue works. Redfish GUI browser works. Websockets work on the GUI. Tested GUI functions that call POST and PATCH. Can see the XSRF-TOKEN and SESSION cookies are SameSite Strict with this build. Before were SameSite None. Browser DevTools -> Storage on Firefox to view cookies. Signed-off-by: Gunnar Mills Change-Id: I4402f2930847c1d47b22696631be26d33c78b6f9 --- include/authorization.hpp | 12 +++++++----- include/login_routes.hpp | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/include/authorization.hpp b/include/authorization.hpp index 9e344d8f07..c078ede23a 100644 --- a/include/authorization.hpp +++ b/include/authorization.hpp @@ -193,11 +193,13 @@ static std::shared_ptr cookieValue.find("SESSION=") == std::string::npos) { // TODO: change this to not switch to cookie auth - res.addHeader("Set-Cookie", "XSRF-TOKEN=" + sp->csrfToken + - "; Secure\r\nSet-Cookie: SESSION=" + - sp->sessionToken + - "; Secure; HttpOnly\r\nSet-Cookie: " - "IsAuthenticated=true; Secure"); + res.addHeader( + "Set-Cookie", + "XSRF-TOKEN=" + sp->csrfToken + + "; SameSite=Strict; Secure\r\nSet-Cookie: SESSION=" + + sp->sessionToken + + "; SameSite=Strict; Secure; HttpOnly\r\nSet-Cookie: " + "IsAuthenticated=true; Secure"); BMCWEB_LOG_DEBUG << " TLS session: " << sp->uniqueId << " with cookie will be used for this request."; return sp; diff --git a/include/login_routes.hpp b/include/login_routes.hpp index d4005599bb..4d42b565be 100644 --- a/include/login_routes.hpp +++ b/include/login_routes.hpp @@ -167,11 +167,13 @@ inline void requestRoutes(App& app) // "set-cookie" string into the value header, and get // the result we want, even though we are technicaly // declaring two headers here. - res.addHeader("Set-Cookie", - "XSRF-TOKEN=" + session->csrfToken + - "; Secure\r\nSet-Cookie: SESSION=" + - session->sessionToken + - "; Secure; HttpOnly"); + res.addHeader( + "Set-Cookie", + "XSRF-TOKEN=" + session->csrfToken + + "; SameSite=Strict; Secure\r\nSet-Cookie: " + "SESSION=" + + session->sessionToken + + "; SameSite=Strict; Secure; HttpOnly"); } else { -- cgit v1.2.3