summaryrefslogtreecommitdiff
path: root/include/sessions.hpp
AgeCommit message (Collapse)AuthorFilesLines
4 daysBreak out random ID methodsEd Tanous1-41/+9
The method of creating a random ID from an openssl random generator of a particular length is something that is generally useful, and something we can write unit tests for. Add it. Tested: Redfish service validator login flows work correctly in redfish service validator. Change-Id: Ic3b58d33f1421f3eb39e2d57585958f87f6fb8ea Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-20Remove sessions on user password updateRavi Teja1-0/+14
When a user's password is changed, existing Redfish sessions for that user, created with the old password, continue to work. As per OWASP session management, "The session ID must be renewed or regenerated by the web application after any privilege level change within the associated user session... Common scenarios to consider include; password changes, permission changes, or switching from a regular user role to an administrator role within the web application." [1] https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html This commit removes existing user sessions when the user's password is changed. This commit leaves the current session in place though a new removeSessionsByUsernameExceptSession(). This commit doesn't completely get us fully to what owasp says but is a start. Tested: Create some users: ``` curl -k -v -X POST -H "Content-Type: application/json" \ https://$bmc/redfish/v1/AccountService/Accounts/ -d \ '{"UserName":"testadminuser","Password":"<password>","RoleId":"Administrator","Enabled":true}' ``` Using basic auth was able to update own password and another user's password. Using token auth, verified the current session did not get deleted but other sessions from that user did. ``` curl -k -H "Content-Type: application/json" -X POST -D headers.txt \ https://${bmc}/redfish/v1/SessionService/Sessions -d \ '{"UserName":"testadminuser", "Password":"<password>"}' ``` ``` curl -k -v -X PATCH -H "X-Auth-Token: $token" \ -H "Content-Type:application/json" \ https://$bmc/redfish/v1/AccountService/Accounts/testadminuser \ -d '{"Password":"<password>"}' ``` Verified when changing another user's password all sessions were dropped. Change-Id: I4de60b84964a6b29c021dc3a2bece9ed4bc09eac Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
2024-05-01Bring consistency to config optionsEd Tanous1-29/+5
The configuration options that exist in bmcweb are an amalgimation of CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms and meson options using a config file. This history has led to a lot of different ways to configure code in the codebase itself, which has led to problems, and issues in consistency. ifdef options do no compile time checking of code not within the branch. This is good when you have optional dependencies, but not great when you're trying to ensure both options compile. This commit moves all internal configuration options to: 1. A namespace called bmcweb 2. A naming scheme matching the meson option. hyphens are replaced with underscores, and the option is uppercased. This consistent transform allows matching up option keys with their code counterparts, without naming changes. 3. All options are bool true = enabled, and any options with _ENABLED or _DISABLED postfixes have those postfixes removed. (note, there are still some options with disable in the name, those are left as-is) 4. All options are now constexpr booleans, without an explicit compare. To accomplish this, unfortunately an option list in config/meson.build is required, given that meson doesn't provide a way to dump all options, as is a manual entry in bmcweb_config.h.in, in addition to the meson_options. This obsoletes the map in the main meson.build, which helps some of the complexity. Now that we've done this, we have some rules that will be documented. 1. Runtime behavior changes should be added as a constexpr bool to bmcweb_config.h 2. Options that require optionally pulling in a dependency shall use an ifdef, defined in the primary meson.build. (note, there are no options that currently meet this class, but it's included for completeness.) Note, that this consolidation means that at configure time, all options are printed. This is a good thing and allows direct comparison of configs in log files. Tested: Code compiles Server boots, and shows options configured in the default build. (HTTPS, log level, etc) Change-Id: I94e79a56bcdc01755036e4e7278c7e69e25809ce Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-18Remove ibm locks featureSunitha Harish1-10/+0
This feature was introduced to manage the operation sync at BMC while multiple clients manage the BMC. This feature scope has gone away and it is not a simple code to maintain as per the growing standards of bmcweb. This commit removes the feature from this repo. Tested by: Locks routes are not available anymore Change-Id: I257225cfb1f43d7d5dadb21a28a2ee5345c5112a Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com> Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-25Fix redundant init issuesEd Tanous1-7/+15
clang-tidy-18 must've fixed their checking for these in headers. Resolve as the robot commands. Tested: Noop changes made by tidy. Code compiles. Change-Id: I1de7686c597deffb0df91c30dae1a29f9ba7900e Signed-off-by: Ed Tanous <ed@tanous.net>
2023-08-01Revert "Cache user role in session object"Gunnar Mills1-7/+1
This reverts commit 8ed41c35a314580bb794fa0fff2e01b0bf7efcf7. In discord, it was posted 2 systems are hitting 403 Forbidden for all endpoints. Reverting fixed the problem, until time is given to dive into this, just revert. One of the things wrong is this is missing an After/Want xyz.openbmc_project.User.Manager.service. Change-Id: I1766a6ec2dbc9fb52da3940b07ac002a1a6d269a Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2023-07-28Cache user role in session objectEd Tanous1-1/+7
There is an async call within the router that leads to a small, but pervasive performance issue for all queries. Removing that call from the router has the potential to increase the performance of every authenticated query, and significantly reduce our dbus traffic for "simple" operations. This commit re-implements the role cache in session object that existed previously many years ago. Each users role is fetched during authentication and persisted in session object. Each successive request can then be matched against the privilege which is there in the in-memory session object. This was discussed on below commit https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/39756 Tested by: ``` POST /redfish/v1/SessionService/Sessions {"UserName":"root", "Password": “0penBmc”} ``` Followed by redfish queries Get /redfish/v1/AccountService Tested user role persistency Redfish service validator passes. Signed-off-by: Ravi Teja <raviteja28031990@gmail.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I575599c29358e32849446ce6ee7f62c8eb3885f6
2023-07-20Replace logging with std::formatEd Tanous1-7/+8
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging. Unfortunately, given its level of compile time checks, it needs to be a method, instead of the stream style logging we had before. This requires a pretty substantial change. Fortunately, this change can be largely automated, via the script included in this commit under scripts/replace_logs.py. This is to aid people in moving their patchsets over to the new form in the short period where old patches will be based on the old logging. The intention is that this script eventually goes away. The old style logging (stream based) looked like. BMCWEB_LOG_DEBUG << "Foo " << foo; The new equivalent of the above would be: BMCWEB_LOG_DEBUG("Foo {}", foo); In the course of doing this, this also cleans up several ignored linter errors, including macro usage, and array to pointer deconstruction. Note, This patchset does remove the timestamp from the log message. In practice, this was duplicated between journald and bmcweb, and there's no need for both to exist. One design decision of note is the addition of logPtr. Because the compiler can't disambiguate between const char* and const MyThing*, it's necessary to add an explicit cast to void*. This is identical to how fmt handled it. Tested: compiled with logging meson_option enabled, and launched bmcweb Saw the usual logging, similar to what was present before: ``` [Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled [Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800 [Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist [Info src/webserver_main.cpp:59] Starting webserver on port 18080 [Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file. [Info src/webserver_main.cpp:137] Start Hostname Monitor Service... ``` Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8
2023-07-12Use openssl random number generatorEd Tanous1-1/+1
We already have a generator class. We should use it. Wrap this into a function that can be unit tested, and add unit tests. Note, some files also needed to change name, because random.hpp conflicts with the built in random, and causes circular build problems. This commit changes it to ossl_random. Tested: Unit tests pass. Now has coverage. Redfish service validator passes. Change-Id: I5f8eee1af5f4843a352c6fd0e26d67fd3320ef53 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-17Revert "Fix websocket csrf checking"Ed Tanous1-1/+2
This reverts commit e628df8658c57f6943b6d3612e1077618e5a168a. This appears to cause problems with non-cookie login of the console websocket. This appears to be a gap in both our testing, and things that we have scripting to do, but clearly it's a change in behavior, so if we want to change the behavior, we should do it intentionally, and clearly, ideally with a path to make clients work, or an explicit documentation that the webui is the only supported client. Change-Id: I334257e1355a5b8431cb7ecfe58ef8a942f4981c Signed-off-by: Ed Tanous <edtanous@google.com>
2023-05-20Added new pre-defined usergroup called hostconsoleNinad Palsule1-0/+2
The new pre-defined usergroup named "hostconsole" is added to differentiate access between host console and manager console. The only users allowed to interact with host console are part of the "hostconsole" group and they are in an administrator role. Note: The changes are spread across multiple repositories listed under "Related commits:" The bmcweb changes to incorporate new group are as follows: - The new user is added in the hostconsole group only if it has an administrative role. - The ssh usergroup is only translated to ManagerConsole redfish group and hostconsole usergroup is translated to HostConsole redfish group. - The following changes are made to check the privileges for host console access - The new OEM privilege "OpenBMCHostConsole" added for host console access. This privilege is not shared externally hence it is not documented. - Updated obmc_console BMCWEB_ROUTE to use the new privilege. - Router functions now save user role and user groups in the session - getUserPrivileges() function now takes session reference instead of user role. This function now also checks for the user group "hostconsole" and add the new privilege if user is member of this group. - Updated all callers of the getUserPrivileges to pass session reference. - Added test to validate that new privilege is set correctly. Tested: Loaded code on the system and validated that; - New user gets added in hostconsole group. NOTE: Prior to this commit all groups are assigned to new user. This drop does not change that behavior. - Access from the web gui is only available for users in hostconsole group. Used IBM internal simulator called simics to test this. This simulator allows accessing openbmc from GUI. - Checked the role collection and there is no change. $ curl -k -H "X-Auth-Token: $TOKEN" -X GET \ https://${bmc}/redfish/v1/AccountService/Roles $ curl -k -H "X-Auth-Token: $TOKEN" -X GET \ https://${bmc}/redfish/v1/AccountService/Roles/Administrator $ curl -k -H "X-Auth-Token: $TOKEN" -X GET \ https://${bmc}/redfish/v1/AccountService/Roles/ReadOnly $ curl -k -H "X-Auth-Token: $TOKEN" -X GET \ https://${bmc}/redfish/v1/AccountService/Roles/Operator - HostConsole is in AccountType when hostconsole group is present in UserGroups D-Bus property $ id user99 uid=1006(user99) gid=100(users) groups=1000(priv-admin),1005(web),\ 1006(redfish),1013(hostconsole),100(users) $ curl -k https://${bmc}/redfish/v1/AccountService/Accounts/user99 { "@odata.id": "/redfish/v1/AccountService/Accounts/user99", "@odata.type": "#ManagerAccount.v1_4_0.ManagerAccount", "AccountTypes": [ "HostConsole", "Redfish", "WebUI", "ManagerConsole" ], "Description": "User Account", "Enabled": true, "Id": "user99", "Links": { "Role": { "@odata.id": "/redfish/v1/AccountService/Roles/Administrator" } }, "Locked": false, "Locked@Redfish.AllowableValues": [ "false" ], "Name": "User Account", "Password": null, "PasswordChangeRequired": false, "RoleId": "Administrator", "UserName": "user99" - The hostconsole group is not present for readonly or operator users and also made sure that console access is not provided. This testing is done one the system and console access was tried by modifying the https://github.com/openbmc/bmcweb/blob/master/scripts/websocket_test.py + curl -k https://${bmc}/redfish/v1/AccountService/Accounts/user99 { "@odata.id": "/redfish/v1/AccountService/Accounts/user99", "@odata.type": "#ManagerAccount.v1_4_0.ManagerAccount", "AccountTypes": [ "IPMI", "Redfish", "WebUI", "ManagerConsole" ], "Description": "User Account", "Enabled": true, "Id": "user99", "Links": { "Role": { "@odata.id": "/redfish/v1/AccountService/Roles/ReadOnly" } }, "Locked": false, "Locked@Redfish.AllowableValues": [ "false" ], "Name": "User Account", "Password": null, "PasswordChangeRequired": false, "RoleId": "ReadOnly", "UserName": "user99" [INFO "http_connection.hpp":209] Request: 0x150ac38 HTTP/1.1 GET /console0 ::ffff:x.x.xx.xxx [DEBUG "routing.hpp":1265] Matched rule (upgrade) '/console0' 1 / 2 [DEBUG "routing.hpp":1084] userName = user99 userRole = priv-user [DEBUG "routing.hpp":1123] IsUserPrivileged: group=ipmi [DEBUG "routing.hpp":1123] IsUserPrivileged: group=redfish [DEBUG "routing.hpp":1123] IsUserPrivileged: group=ssh [DEBUG "routing.hpp":1123] IsUserPrivileged: group=web [DEBUG "routing.hpp":93] checkPrivileges: BASE USER: Login [DEBUG "routing.hpp":93] checkPrivileges: BASE USER: ConfigureSelf [DEBUG "routing.hpp":113] checkPrivileges: OEM REQUIRED: OpenBMCHostConsole [ERROR "routing.hpp":1192] Insufficient Privilege + curl -k https://${bmc}/redfish/v1/AccountService/Accounts/user99 { "@odata.id": "/redfish/v1/AccountService/Accounts/user99", "@odata.type": "#ManagerAccount.v1_4_0.ManagerAccount", "AccountTypes": [ "IPMI", "Redfish", "WebUI", "ManagerConsole" ], "Description": "User Account", "Enabled": true, "Id": "user99", "Links": { "Role": { "@odata.id": "/redfish/v1/AccountService/Roles/Operator" } }, "Locked": false, "Locked@Redfish.AllowableValues": [ "false" ], "Name": "User Account", "Password": null, "PasswordChangeRequired": false, "RoleId": "Operator", "UserName": "user99" [INFO "http_connection.hpp":209] Request: 0x21c7c38 HTTP/1.1 GET /console0 ::ffff:x.x.xx.xxx [DEBUG "routing.hpp":1265] Matched rule (upgrade) '/console0' 1 / 2 [DEBUG "routing.hpp":1084] userName = user99 userRole = priv-operator [DEBUG "routing.hpp":1123] IsUserPrivileged: group=ipmi [DEBUG "routing.hpp":1123] IsUserPrivileged: group=redfish [DEBUG "routing.hpp":1123] IsUserPrivileged: group=ssh [DEBUG "routing.hpp":1123] IsUserPrivileged: group=web [DEBUG "routing.hpp":93] checkPrivileges: BASE USER: Login [DEBUG "routing.hpp":93] checkPrivileges: BASE USER: ConfigureComponents [DEBUG "routing.hpp":93] checkPrivileges: BASE USER: ConfigureSelf [DEBUG "routing.hpp":113] checkPrivileges: OEM REQUIRED: OpenBMCHostConsole [ERROR "routing.hpp":1192] Insufficient Privilege Related commits: NOTE: docs, openbmc, obmc-console changes are already merged. bmcweb and phosphor-user-manager will be merged together. docs: https://gerrit.openbmc.org/c/openbmc/docs/+/60968 phosphor-user-manager: https://gerrit.openbmc.org/c/openbmc/phosphor-user-manager/+/61583 openbmc: https://gerrit.openbmc.org/c/openbmc/openbmc/+/61582 obmc-console: https://gerrit.openbmc.org/c/openbmc/obmc-console/+/61581 bmcweb: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/61580 Change-Id: Ia5a33dafc9a76444e6a8e74e752f0f90cb0a31c8 Signed-off-by: Ninad Palsule <ninadpalsule@us.ibm.com>
2023-05-11clang-format: copy latest and re-formatPatrick Williams1-2/+1
clang-format-16 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest .clang-format from the docs repository and reformat the repository. Change-Id: I75f89d2959b0f1338c20d72ad669fbdc1d720835 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
2023-04-06Fix websocket csrf checkingGunnar Mills1-2/+1
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>
2023-02-28Remove sessions when user is deletedXie Ning1-0/+12
An Internal server Error will happen if you delete the login user. Match the "InterfacesRemoved" signal for monitoring the user status and delete the session to fix this bug. Tested: 1. Add a new user such as test 2. Login with the new user in web 3. Delete or rename the user by web and ipmi command 4. Refresh the web and a new user was needed to login in the web Signed-off-by: Xie Ning <xiening.xll@bytedance.com> Change-Id: I2b53edb71d9a4e904c7da54393539f87eeb2d7a3
2023-02-24Pass string views by valueEd Tanous1-5/+3
string_view should always be passed by value; This commit is a sed replace of the code to make all string_views pass by value, per general coding guidelines[1]. [1] https://quuxplusone.github.io/blog/2021/11/09/pass-string-view-by-value/ Tested: Code compiles. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I55b342a29a0fbfce0a4ed9ea63db6014d03b134c
2023-01-18Fix a boatload of #includesEd Tanous1-2/+2
Most of these missing includes were found by running clang-tidy on all files, including headers. The existing scripts just run clang-tidy on source files, which doesn't catch most of these. Tested: Code compiles Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ic741fbb2cc9e5e92955fd5a1b778a482830e80e8
2022-10-07Move ClientID parameter out of OEMEd Tanous1-6/+5
In 2022.2, Redfish added support for the Context parameter on the Session Resource. This parameter has the same function that the OemSession.ClientId field served. This commit moves all the existing ClientId code to produce Context as well. Functionally, this has one important difference, in that Context in Redfish is optionally provided by the user, which means we need to omit it if not given by the user. The old implementation left it set to empty string (""). Because of this, a few minor interfaces need to change to use std::optional. Existing uses of clientId are moved to using value_or("") to keep the same behavior as before. Tested: curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\": \"0penBmc\"}" https://192.168.7.2/redfish/v1/SessionService/Sessions Returns a Session object with no Context key present curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\": \"0penBmc\", \"Context\": \"Foobar\"}" https://192.168.7.2/redfish/v1/SessionService/Sessions Returns a Session object with: "Context": "Foobar" Subsequent Gets of /redfish/v1/SessionService/Sessions/<sid> return the same session objects, both with and without Context. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I4df358623f93f3e6cb659e99970ad909cefebc62
2022-09-28clang-15: fixes for emplacePatrick Williams1-1/+1
clang-tidy-15 is raising the following warning, which it automatically fixed: ``` ../include/sessions.hpp:262:38: error: unnecessary temporary object created while calling emplace [modernize-use-emplace,-warnings-as-errors] auto it = authTokens.emplace(std::make_pair(sessionToken, session)); ^~~~~~~~~~~~~~~ ~ ``` Apply automatically generated fix. Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I425efdcba4bf08b03d64f8fb913eab956f5a198d
2022-06-25sessions: iwyunitroglycerine1-8/+0
While revisiting change 49039 I saw session.hpp included a few files that are not used. I removed them and the code still compiles. Signed-off-by: Sui Chen <suichen6@gmail.com> Change-Id: I97aa2359053ce6102b84af1ef555d881cd35eaba
2022-06-23pam_authenticate: remove unused referenceNan Zhou1-1/+0
The header pam_authenticate isn't referenced at all in these two files. So remove it. Tested: codes compiles Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I4d9ea06dca2ce4d05add710ec3d6cc0df5c1a39d
2022-03-22Consitently use dbus::utility typesEd Tanous1-1/+0
This saves about 4k on the binary size Tested: Redfish service validator passes. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I9546227a19c691b1aecb80e80307889548c0293f
2022-02-09Enable readability-avoid-const-params-in-declsEd Tanous1-1/+1
This check involves explicitly declaring variables const when they're declared auto, which helps in readability, and makes it more clear that the variables are const. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I71198ea03850384a389a56ad26f2c4a48c75b148
2022-01-12Enforce variable initEd Tanous1-1/+1
There were a few places we weren't initting our variables per cpp core guidelines. Fix all of them, and enable checks for this. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iba09924beb9fb26f597ff94d1cecbd6d6b1af912
2022-01-12Enable cppcoreguidelines-special-member-functions checksEd Tanous1-0/+3
Part of enforcing cpp core guidelines involves explicitly including all constructors required on a non-trivial class. We were missing quite a few. In all cases, the copy/move/and operator= methods are simply deleted. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie8d6e8bf2bc311fa21a9ae48b0d61ee5c1940999
2021-12-10Convert IPv4-mapped IPv6 ClientIP back to IPv4Jiaqing Zhao1-6/+9
Current HTTP server creates an IPv6 acceptor to accept both IPv4 and IPv6 connections. In this way, IPv4 address will be presented as IPv6 address in IPv4-mapped format. This patch converts it back to IPv4. Tested: Verified the ClientOriginIP in Session is shown in native IPv4 format instead of IPv4-mapped IPv6 format. Change-Id: Icd51260b2d4572d52f5c670128b7f07f6b5e6912 Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
2021-08-17bmcweb: fix the random headersNan Zhou1-0/+1
bmcweb::OpenSSLGenerator is defined in include/random.hpp. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I1d8751b49a0121526a40b885ed7f5b520640f9f0
2021-08-12Reduce session timeout to 30 minutesJason M. Bills1-1/+1
This reduces our web session timeout to 30 minutes per NIST guidelines: "Reauthentication of the subscriber SHALL be repeated following any period of inactivity lasting 30 minutes or longer." https://pages.nist.gov/800-63-3/sp800-63b.html Change-Id: Icb600b5bc026107f582bc93be355bc5e2b46677d Signed-off-by: Jason M. Bills <jason.m.bills@linux.intel.com>
2021-03-11Redfish Session : Fix clientIp getting mapped to clientIdSunitha Harish1-3/+3
When the session is created using /login, the ClientOriginIPAddress is mapped to the clientId parameter which displayed the clientIP instead of the of clientId. The similar problem is observed with auth methods other than sessions created using the SessionService resource This commit swaps the clientId and clientIp parameters passed to generateUserSession API, so that the optional clientId is passed as the last parameter Tested by : 1. Create session using Redfish command POST https://${bmc}/login -d '{"username": <>,"password": <>}' POST https://${bmc}/redfish/v1/SessionService/Sessions -d '{"username": <>,"password": <>}' 2. Open the GUI session to check the clientId is not displaying the ClientOriginIPAddress Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com> Change-Id: I6cee3de963c489e690d2ad0bb09ba78dca39e4f9
2020-12-15Add meson options for all authentication methods.Alan Kuo1-2/+26
Add meson options to enabled/disabled authentication methods: - basic-auth : For enable basic authentication, default is enabled - session-auth : For enable session token authentication, default is enabled - xtoken-auth : For enable x-token authentication, default is enabled - cookie-auth : For enabled cookie authentication, default is enabled Signed-off-by: Alan Kuo <Alan_Kuo@quantatw.com> Change-Id: I52e636f2534a14897cb57d35e563ea8841cc68b9
2020-11-12Call applySessionTimeouts before writing dataGunnar Mills1-17/+17
Before writing bmcweb_persistent_data.json on bmcweb shutdown call applySessionTimeouts() to ensure no stale sessions are wrote. To accomplish this had to move applySessionTimeouts to public. Tested: Stop bmcweb, modify bmcweb_persistent_data.json timeout to be 30 seconds. Start bmcweb. Verify timeout 30 seconds and 1 session is restored. Wait 1 min. stop bmcweb. Verify no sessions in bmcweb_persistent_data.json. Signed-off-by: Gunnar Mills <gmills@us.ibm.com> Change-Id: Idfaf7c144b3bdeb2741b48f603d7213ac1a51f10
2020-10-23fix include namesEd Tanous1-3/+2
cppcheck isn't smart enough to recognize these are c++ headers, not c headers. Considering we're already inconsistent about our naming, it's easier to just be consistent, and move the last few files to use .hpp instead of .h. Tested: Code builds, no changes. Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Ic348d695f8527fa4a0ded53f433e1558c319db40
2020-10-23Timeout is not per-sessionEd Tanous1-2/+15
fix regression on 5fb91ba400e0482813cf5e1a86fdca17468d0a6a. Timeout is a global setting, not a per-session setting. This caused problems with regenerating it, as session restoration doesn't follow the "best effort" policy we've done before. This commit: 1. Makes Session::fromJson more robust against extra keys. 2. Disallowed reading in client_id if IBM_Management_console isn't enabled. 3. Moves timeout to the proper place in the persistent config file. Resolves https://github.com/openbmc/bmcweb/issues/158 Tested: Downloaded to bmc, cleared bmcweb_persistent_data.json, then logged in using webui-vue. Rebooted BMC. Reloaded /redfish/v1/SessionService/Sessions/<sessionid> and observed that all data restored properly. Unclear why, but ClientOriginIPAddress seems broken, but that seems true prior to this patch. Data that got returned is included for completeness. { "@odata.id": "/redfish/v1/SessionService/Sessions/BKqK5dNfNS", "@odata.type": "#Session.v1_3_0.Session", "ClientOriginIPAddress": "", "Description": "Manager User Session", "Id": "BKqK5dNfNS", "Name": "User Session", "UserName": "root" } Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I716431fd4775af63715d07973f723caa8cb34259
2020-10-15Lots of performance improvementsEd Tanous1-1/+1
(In the voice of the kid from sixth sense) I see string copies... Apparently there are a lot of places we make unnecessary copies. This fixes all of them. Not sure how to split this up into smaller patches, or if it even needs split up. It seems pretty easy to review to me, because basically every diff is identical. Change-Id: I22b4ae4f96f7e4082d2bc701098a04f7bed95369 Signed-off-by: Ed Tanous <ed@tanous.net> Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
2020-10-09Move Openssl GeneratorEd Tanous1-39/+3
This commit moves the openssl random number generator into its own file, so it can be used in EventService, and moves it to its own file. Seeding a random number generator with time is bad practice in general, so much so that there's a CERT rule about it as well as a clang-tidy check. https://clang.llvm.org/extra/clang-tidy/checks/cert-msc51-cpp.html This doesn't matter much in this case, as we're generating a randomized int for an ID, but it will matter in other cases, and we'd like to have the check on to verify that. Change-Id: I8e6aebb7962d259045ffd558eea22f07f9c23821 Signed-off-by: Ed Tanous <ed@tanous.net>
2020-10-07Remove adl_serializer uses for jsonEd Tanous1-42/+0
Several pieces of code seems to be using the adl_serializer from nlohmann. This unfortunately has very undesirable behavior in some cases, and makes a lot of things really difficult to track back to the function that did the serialization, which has caused several bugs in the past with incorrect types. This patchset removes them, and opts for the inline version of the nlohmann json serialization. Tested: Booted bmcweb, and logged in. cat bmcweb_persistent_data.json showed persistent data written properly. Logged into bmc through webui-vue systemctl restart bmcweb Then refreshed webui-vue, and didn't get logged out. Change-Id: I92868629c54d08b37dd1d956f7c2e2a954f9b670
2020-10-06Fix includesEd Tanous1-3/+0
Lots of bad includes got put in recently, including big things, like boost/http and beast/core. These are lots of code to parse, and leads to files including things they didn't mean to. Tested: Code compiles Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I38de889fdfd9b23f66a2259bb30bf6584345e77f
2020-10-05Add PATCH support for SessionTimeout PropertyManojkiran Eda1-5/+11
- This commit would add the patch support for the session timeout propery under the sessionservice. - This commit also brings in support for persistent session timeout property. Tested By: 1. Redfish validator passed. 2. PATCH the session time out property using the below command PATCH -d '{"SessionTimeout": 100}' https://<bmcip>/redfish/v1/SessionService 3. GET on sessionservice should return the value of time out which is patched by using the above command & also GET on the session service fails with Unauthorized error post the patched timeout value. 4. And also, the existing sessions that are open for the new timeout value are also closed. 5. As per the schema , the range of values that are allowed for session timeout are between 30 sec to 86400 sec, so any value which is patched out of the range is failed with an appropriate error message. 6. PATCH the session timeout to new value using 2, and them restart the bmcweb and the GET using 3 should return the new value. Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com> Change-Id: Id50eacc5018b7a82371fd37a2ae1e7fb7596ed2b
2020-08-24Modernize; Move some apis to range based for loopEd Tanous1-6/+6
There were a couple places in code where we still use index based for loops. Move these to the more modern range based for loops. Tested: Needs testing. Changes made by clang-tidy. Signed-off-by: Ed Tanous <ed.tanous@intel.com> Change-Id: I30bf6fae6b2540434d5c98900a8f6bd0c8f2be93
2020-08-17Enable clang warningsEd Tanous1-2/+2
This commit enables clang warnings, and fixes all warnings that were found. Most of these fall into a couple categories: Variable shadow issues were fixed by renaming variables unused parameter warnings were resolved by either checking error codes that had been ignored, or removing the name of the variable from the scope. Other various warnings were fixed in the best way I was able to come up with. Note, the redfish Node class is especially insidious, as it causes all imlementers to have variables for parameters, regardless of whether or not they are used. Deprecating the Node class is on my list of things to do, as it adds extra overhead, and in general isn't a useful abstraction. For now, I have simply fixed all the handlers. Tested: Added the current meta-clang meta layer into bblayers.conf, and added TOOLCHAIN_pn-bmcweb = "clang" to my local.conf Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Ia75b94010359170159c703e535d1c1af182fe700
2020-08-17Remove middlewaresEd Tanous1-25/+16
Middlewares, while kinda cool from an academic standpoint, make our build times even worse than they already are. Given that we only really use 1 real middleware today (token auth) and it needs to move into the parser mode anyway (for security limiting buffer sizes), we might as well use this as an opportunity to delete some code. Some other things that happen: 1. Persistent data now moves out of the crow namespace 2. App is no longer a template 3. All request_routes implementations no longer become templates. This should be a decent (unmeasured) win on compile times. This commit was part of a commit previously called "various cleanups". This separates ONLY the middleware deletion part of that. Note, this also deletes about 400 lines of hard to understand code. Change-Id: I4c19e25491a153a2aa2e4ef46fc797bcb5b3581a Signed-off-by: Ed Tanous <ed@tanous.net>
2020-06-17Fetch the ClientIP during session creationSunitha Harish1-12/+19
This commit saves the IP Address of the client from where the session was created. - This is not a user supplied value. The BMC will internally pull the IP address from the incoming create-session request. - It should also be noted that ClientIP will not change if the same session token is used from some other IP address for further management of the BMC. Tested by: 1. Create session 2. Display the Session details with GET command to check the IP from where the session is created/updated. GET https://${bmc}/redfish/v1/SessionService/Sessions/<id> { "@odata.id": "/redfish/v1/SessionService/Sessions/<id>", "@odata.type": "#Session.v1_0_2.Session", "Description": "Manager User Session", "Id": "<id>", "Name": "User Session", "Oem": { "OpenBMC": { "@odata.type": "#OemSession.v1_0_0.Session", "ClientOriginIP": "<ip address>" } }, "UserName": "root" } 3. Redfish validator is run successfully. Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com> Change-Id: I0076f260f50a991600ec060c72f3e46fb9a9cbb8
2020-06-17Session creation : Get and Set Oem ClientIDSunitha Harish1-6/+19
This commit implements handling the OemSession ClientID parameter for the IBM management console. Each session gets a random generated unique Id (Resource Id); but this Id is not a parameter that the client can set to a well known identifier. This Oem parameter ClientID is the string which the client can supply to uniquely identify itself among other sessions in the BMC. This is a read-only property which shall be passed in only during the session creation. 1. Create session by supplying the ClientID Oem parameter 2. Display the ClientID associated with the session 3. Persist the ClientID across BMC reboot Tested by: ============ 1. POST https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName":"root", "Password":<>, "Oem":{"OpenBMC" : {"ClientID":"<client unique id>"}}}' 2. GET https://${bmc}/redfish/v1/SessionService/Sessions/<id> { "@odata.id": "/redfish/v1/SessionService/Sessions/<id>", "@odata.type": "#Session.v1_0_2.Session", "Description": "Manager User Session", "Id": "<id>", "Name": "User Session", "Oem": { "OpenBMC": { "@odata.type": "#OemSession.v1_0_0.Session", "ClientID": "<client unique id>" } }, "UserName": "root" } 3. Verified the session creation works fine without the Oem parameters. 4. Redfish validator Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com> Change-Id: Ia740a610e3974dc3781bcee702c74ded9903944a
2020-06-11clang-format: update to latest from docs repoGunnar Mills1-7/+8
This is from openbmc/docs/style/cpp/.clang-format Other OpenBMC repos are doing the same. Tested: Built and validator passed. Change-Id: Ief26c755c9ce012823e16a506342b0547a53517a Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2020-05-20Implement Redfish PasswordChangeRequiredJoseph Reynolds1-4/+15
This implements the Redfish PasswordChangeRequired handling. See section 13.3.7.1 "Password change required handling" in the 1.9.1 spec: https://www.dmtf.org/sites/default/files/standards/documents/DSP0266_1.9.1.pdf These portions of the spec are implemented: - Authenticatation with a correct but expired password creates a session: - The session is restricted to the ConfigureSelf privilege which allows a user to change their own password (via GET and PATCH Password for their own account). Support for the ConfigureSelf privilege is already in BMCWeb. - The session object has the PasswordChangeRequired message. - All other operations respond with http status code 403 Forbidden and include the PasswordChangeRequired message. - The ManagerAccount (URI /redfish/v1/AccountService/Accounts/USER) PasswordChangeRequired property is implemented for local accounts but not present for remote accounts. This has the following additional behavior: The PasswordChangeRequired property is updated at the start of each new REST operation, even within an existing session. This behavior implements a "dynamic" PasswordChangeRequired handling that responds to changes to the underlying "password expired" status. Specifically: - Sessions restricted by the PasswordChangeRequired handling lose that restriction when the underlying account password is changed. - Sessions become subject to the PasswordChangeRequired handling restrictions whenever the underlying account password expires. - The mechanism is to check if the password is expired at the start of every new REST API operation, effectively updating the ManagerAccount PasswordChangeRequired property each time. This makes BMCWeb responsive to changes in the underlying account due to other activity on the BMC. Notes: 1. Note that when an account password status is changed (for example, the password becomes expired or is changed) and that account has active sessions, those sessions remain. They are not deleted. Any current operations are allowed to complete. Subsequent operations with that session pick up the new password status. 2. This does not implement OWASP recommendations which call for sessions to be dropped when there is a significant change to the underlying account. For example, when the password is changed, the password becomes expired, or when the account's Role changes. OWASP's recommendation is due to the session fixation vulnerability. See the OWASP Session Management Cheat Sheet section "Renew the Session ID After Any Privilege Level Change": https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#renew-the-session-id-after-any-privilege-level-change BMCWeb protects against session fixation vulnerabilities because it always regenerates new session IDs when successful authentication creates a new session. 3. Users authenticating via mTLS are not subject to the PasswordChangeRequired behavior because mTLS takes precedence over password-based authentication. Tested: 0. Setup: - The `passwd --expire USERNAME` command was used to expire passwords. The `chage USER` command was also used. - The following were used to change the password: Redfish API, passwd command, and the SSH password change dialog. - Tested the following via Basic Auth, /login, and Redfish login (except where Basic Auth does not create a persistent session). - Only local user account were tested. - Did not test authentication via mTLS or with LDAP users. 1. When the password is not expired, authentication behaves as usual for both correct and incorrect passwords. 2. When the password is incorrect and expired, authentication fails as usual. 3. When the password is correct but expired: A. A session is created and has the PasswordChangeRequired message. B. That session cannot access resources that require Login privilege and the 403 message contains the PasswordChangeRequired message. C. That session can be used to GET the user's account, PATCH the Password, and DELETE the session object. D. The account PasswordChangeRequired reports true. 4. While a session is established, try expiring and changing (unexpiring) the password using various mechanisms. Ensure both the session object and the ManagerAccount PasswordChangeRequired property report the correct condition, and ensure PasswordChangeRequired handling (restricting operations to ConfigureSelf when PasswordChangeRequired is true) is applied correctly. Signed-off-by: Joseph Reynolds <joseph-reynolds@charter.net> Change-Id: Iedc61dea8f949e4b182e14dc189de02d1f74d3e8
2020-05-14Remove the locks associated with the sessionRatan Gupta1-0/+11
This commit does the following => makes the lock class singleton. => during session timeout erase the locks associated with the session. => Erase the locks when the session is explicitly deleted on a user request. We need to find a different way of calculating session timeout currently session timeout gets calculated when the request comes to BMC. TODO: We need some module which keeps looking at the sessions in certain time interval and earse the session if it is timeout, It is useful in the case where there is resources which gets free after session timeout. It may happen that client gets the session, obtain cerain resources on that session and never sends any request, in that case session timeout will never occur for that session. Signed-off-by: Ratan Gupta <ratagupt@linux.vnet.ibm.com> Change-Id: Ic9962f761fc84a03747a90bd951ea36eb8962455
2020-04-17Use CPRNG for session secrets generationJames Feist1-4/+59
std::random_device is not a cryptographically secure algorithm. Switch to RAND_bytes instead. Tested: Login and logout works as expected Change-Id: If24fa6c3a0652c011bc50ae611b180f342d68433 Signed-off-by: James Feist <james.feist@linux.intel.com>
2020-04-15Add CSRF check into websocketsJames Feist1-0/+1
This adds CSRF check into websockets to avoid attacks on websockets. Tested: Could no longer use crosssite scripting to open websocket. KVM and SOL still work once web-ui changes are updated Change-Id: I325079ae3d4db2701671564dff733e034d2670d6 Signed-off-by: James Feist <james.feist@linux.intel.com>
2019-12-19Turning ON the mutual TLSZbigniew Kurzynski1-1/+1
This commit sets the mutual TLS option to ON by default. Core mTLS implementation was accepted under this commit: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/23588 where by default the mTLS was disabled. Tested: Manual tests were made to verify if this option turns the mTLS ON. Signed-off-by: Zbigniew Kurzynski <zbigniew.kurzynski@intel.com> Change-Id: I3bc5f5453d4c4df1cd7ecb0b8868423285b6ab83
2019-12-17Fix authorization for LDAP usersRAJESWARAN THILLAIGOVINDAN1-244/+0
Modified the code to make an asynchronous call to GetUserInfo to get the user role for authorization. For local users, DBus matches are used to store user role map hot in memory. Hence, bmcweb has to know whether a user is a local user or LDAP user to get the role. To avoid this, removed the existing DBus matches and modified the code to call GetUserInfo to get the role of local users as well as LDAP users. Tested: - Created a local user having admin privilege and verified that he is able to restart the system /redfish/v1/Systems/system/Actions/ComputerSystem.Reset -d '{"ResetType": "GracefulRestart"}' - Created a local user having user privilege and verified that he is unauthorized to restart the system /redfish/v1/Systems/system/Actions/ComputerSystem.Reset -d '{"ResetType": "GracefulRestart"}' - Created a remote user having admin privilege and verified that he is able to restart the system /redfish/v1/Systems/system/Actions/ComputerSystem.Reset -d '{"ResetType": "GracefulRestart"}' - Created a remote user having user privilege and verified that he is unauthorized to restart the system /redfish/v1/Systems/system/Actions/ComputerSystem.Reset -d '{"ResetType": "GracefulRestart"}' - Tested Redfish ConfigureSelf privilege Signed-off-by: RAJESWARAN THILLAIGOVINDAN <rajeswgo@in.ibm.com> Change-Id: Ic3e46a0c0aff2cf456c98048350e58e302011c57
2019-12-16Fixing mTLS switchesZbigniew Kurzynski1-0/+7
This commit is a fix for revert: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/27205 The mTLS will work only when: * Compilation flag BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION is enabled and, * AccountService->Oem->OpenBMC->AuthMethods->TLS is set and, * at least one certificate is uploaded to TrustStore. To enforce a new mTLS settings after switching AuthMethods->TLS all awaiting for acceptance connections are dropped. In case of authorized mTLS requests a cookie with XSRF-TOKEN entry and SESSION is added to response. It will force the X-XSRF-TOKEN challenge in case of applications supporting cookies like web browsers. Also some minor log messages changes were made in this commit. Tested: validator_service run with status: 'Validation has succeeded'. Tests were made with Chrome browser Version 78.0.3904.108. The certificate popup appears only when mTLS is enabled in CMake, the TLS option is set and at least one CA certificate is present. In other cases mTLS is not active and there is no popup for certs. Signed-off-by: Zbigniew Kurzynski <zbigniew.kurzynski@intel.com> Change-Id: I778b848b767356b60386694f802a89b3c1f6cb14