commit e628df8658c57f6943b6d3612e1077618e5a168a
author Gunnar Mills <gmills@us.ibm.com> Tue Apr 04 10:15:42 2023 -0500
committer Ed Tanous <ed@tanous.net> Thu Apr 06 17:36:37 2023 +0000
tree d5f8e35d0c04fcf0f748c9738a9aa9ba7d8496cd
parent 2ec5b14d046e287c740ef59360f3d7637efb8085

Fix websocket csrf checking

https://github.com/openbmc/bmcweb/commit/f8aa3d2704d3897eb724dab9ac596af8b1f0e33e
(4/15/20) added CSRF check into websockets but later setting cookieAuth
to true was removed so this session->cookieAuth is always false.
https://github.com/openbmc/bmcweb/commit/3909dc82a003893812f598434d6c4558107afa28
(7/15/20).

2 choices here add back this cookieAuth=true when cookie auth is used or
remove this "if cookieAuth" and do this check anytime
BMCWEB_INSECURE_DISABLE_CSRF_PREVENTION isn't enabled.

Really we shouldn't support any other auth on websockets so maybe
if (!session->cookieAuth){
  unauthorized;
}
if go with the first choice. Went with the 2nd choice because cleaner.

This checking is a bit weird because it uses protocol for csrf checking.
https://github.com/openbmc/webui-vue/blob/b63e9d9a70dabc4c9a7038f7727fca6bd17d940a/src/views/Operations/SerialOverLan/SerialOverLanConsole.vue#L98

Tested: Before could log in to webui-vue, delete the XSRF-TOKEN but
still connect to the host console. After if deleted the XSRF-TOKEN
(browser dev tools), the websocket does not connect. Don't have a system
with KVM, VM enabled so wasn't able to check those but the webui-vue
code for them looks to pass the token. The webui-vue host console works
the same as before if you aren't messing with the XSRF-TOKEN.

Change-Id: Ibd5910587648f68809c7fd518bcf5a0bcf8cf329
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

http/websocket.hpp

diff --git a/http/websocket.hpp b/http/websocket.hpp
index 9a5aa29..8774200 100644
--- a/http/websocket.hpp
+++ b/http/websocket.hpp
@@ -101,8 +101,7 @@
             if (session != nullptr)
             {
                 // use protocol for csrf checking
-                if (session->cookieAuth &&
-                    !crow::utility::constantTimeStringCompare(
+                if (!crow::utility::constantTimeStringCompare(
                         protocol, session->csrfToken))
                 {
                     BMCWEB_LOG_ERROR << "Websocket CSRF error";

include/sessions.hpp

diff --git a/include/sessions.hpp b/include/sessions.hpp
index 62700be..99a1a0e 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -39,7 +39,6 @@
     std::string clientIp;
     std::chrono::time_point<std::chrono::steady_clock> lastUpdated;
     PersistenceType persistence{PersistenceType::TIMEOUT};
-    bool cookieAuth = false;
     bool isConfigureSelfOnly = false;
 
     // There are two sources of truth for isConfigureSelfOnly:
@@ -256,7 +255,7 @@
         auto session = std::make_shared<UserSession>(UserSession{
             uniqueId, sessionToken, std::string(username), csrfToken, clientId,
             redfish::ip_util::toString(clientIp),
-            std::chrono::steady_clock::now(), persistence, false,
+            std::chrono::steady_clock::now(), persistence,
             isConfigureSelfOnly});
         auto it = authTokens.emplace(sessionToken, session);
         // Only need to write to disk if session isn't about to be destroyed.