| commit | e628df8658c57f6943b6d3612e1077618e5a168a | [log] [tgz] |
|---|---|---|
| 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 [diff] |
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>
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";