summaryrefslogtreecommitdiff
path: root/http/routing.hpp
AgeCommit message (Collapse)AuthorFilesLines
2024-05-03Manage Request with shared_ptrJonathan Doman1-21/+22
This is an attempt to solve a class of use-after-move bugs on the Request objects which have popped up several times. This more clearly identifies code which owns the Request objects and has a need to keep it alive. Currently it's just the `Connection` (or `HTTP2Connection`) (which needs to access Request headers while sending the response), and the `validatePrivilege()` function (which needs to temporarily own the Request while doing an asynchronous D-Bus call). Route handlers are provided a non-owning `Request&` for immediate use and required to not hold the `Request&` for future use. Tested: Redfish validator passes (with a few unrelated fails). Redfish URLs are sent to a browser as HTML instead of raw JSON. Change-Id: Id581fda90b6bceddd08a5dc7ff0a04b91e7394bf Signed-off-by: Jonathan Doman <jonathan.doman@intel.com> Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Simplify routerEd Tanous1-201/+172
Now that we only support string types in the router we no longer need to build a "Tag" to be used for constructing argument types. Now, we can just track the number of arguments, which simplifies the code significantly, and removes the need to convert to and from the tag to parameter counts. This in turn deletes a lot of code in the router, removing the need for tracking tag types. Tested: Redfish service validator passes. Unit tests pass. Change-Id: Ide1d665dc1984552681e8c05952b38073d5e32dd Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-09Rework logger to create compile time errorsEd Tanous1-9/+7
The logger changes to move to std::format incidentally caused format errors to no longer be flagged at compile time. The example here[1] shows that the latest gcc/c++ gave us a way to solve this, using std::format_string. This incidentally shows two places where we got the format arguments wrong, so fix them. [1] https://stackoverflow.com/questions/72795189/how-can-i-wrap-stdformat-with-my-own-template-function Change-Id: Id884200e2c98eeaf5ef8db6c1d6362ede2ffb858 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-07Fix moves/forwardEd Tanous1-1/+1
Clang has new checks for std::move/std::forward correctness, which catches quite a few "wrong" things where we were making copies of callback handlers. Unfortunately, the lambda syntax of callback{std::forward<Callback>(callback)} in a capture confuses it, so change usages to callback = std::forward<Callback>(callback) to be consistent. Tested: Redfish service validator passes. Change-Id: I7a111ec00cf78ecb7d5f5b102c786c1c14d74384 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-01Use no-switch-default on clangEd Tanous1-1/+1
clang-18 improves this check so that we can actually use it. Enable it and fix all violations. Change-Id: Ibe4ce19c423d447a4cbe593d1abba948362426af Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-21Allow routes with 5 wildcardsEd Tanous1-1/+1
We don't have any routes that use 5 wildcards, but clearly someone uses it because of the bug #270. There's no reason not to fix this. Ideally we would support an arbitrary number of wildcards, but that's a template problem for another day. Tested: No way to test, inspection only. Change-Id: I5de75f5288124e84c153518966d658e1c899f6d5 Signed-off-by: Ed Tanous <ed@tanous.net>
2023-10-24clang-format: copy latest and re-formatPatrick Williams1-1/+1
clang-format-17 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: I2f9540cf0d545a2da4d6289fc87b754f684bc9a7 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
2023-07-20Replace logging with std::formatEd Tanous1-22/+23
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-06-28Simplify the routerEd Tanous1-9/+53
There's a lot of complexity left in the router. The recent decision to only support string arguments means that this can be significantly cleaned up. In some cases, this is done to simply expand the variadic template and handle all parameter cases up to 5 (which should be the max we ever see). While this might seem like it's not very DRY friendly (Don't repeat yourself) this is significantly better than what we had, which was very tough to deciper. Tested: Redfish service validator passes Change-Id: Ic72e54cffd7b9f4a85e6c9d143c45fa20530a2cd Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-09Break up router into separate filesEd Tanous1-761/+6
The router is a giant behemoth. Start breaking it down into pieces. Tested: Redfish service validator passes. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I9d04f53a58ffce3ecbd88dded1aa6e9648d2a762
2023-06-06Add support for multiple consolesNinad Palsule1-2/+2
This drop adds support for multiple consoles. The following changes are made to achieve this. - Kept the "/console0" route for backward compatibility - Added a new route "/console/<str>" to support multiple consoles. All new consoles must use this route string. Testing: - Make sure that old console path /console0 is working. [INFO "http_connection.hpp":209] Request: 0x1bc2e60 HTTP/1.1 GET /console0 ::ffff:x.x.xx.xxx [DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console0' 1 / 2 [DEBUG "obmc_console.hpp":212] Connection 0x1bdb67c opened [DEBUG "obmc_console.hpp":241] Console Object path = /xyz/openbmc_project/console/default service = xyz.openbmc_project.Console.default Request target = /console0 [DEBUG "obmc_console.hpp":198] Console web socket path: /console0 Console unix FD: 12 duped FD: 13 [DEBUG "obmc_console.hpp":82] Reading from socket [DEBUG "obmc_console.hpp":162] Remove connection 0x1bdb67c from obmc console - Make sure that new path for default console working [INFO "http_connection.hpp":209] Request: 0x1bd76a8 HTTP/1.1 GET /console/default ::ffff:x.x.xx.xxx [DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console/<str>' 1 / 2 [DEBUG "obmc_console.hpp":212] Connection 0x1baf82c opened [DEBUG "obmc_console.hpp":241] Console Object path = /xyz/openbmc_project/console/default service = xyz.openbmc_project.Console.default Request target = /console/default [DEBUG "obmc_console.hpp":198] Console web socket path: /console/default Console unix FD: 12 duped FD: 13 [DEBUG "obmc_console.hpp":82] Reading from socket [INFO "obmc_console.hpp":154] Closing websocket. Reason: [DEBUG "obmc_console.hpp":162] Remove connection 0x1baf82c from obmc console - Make sure that path for hypervisor console is working. [INFO "http_connection.hpp":209] Request: 0x1bc2e60 HTTP/1.1 GET /console/hypervisor ::ffff:x.x.xx.xxx [DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console/<str>' 1 / 2 [DEBUG "obmc_console.hpp":212] Connection 0x1bc5234 opened [DEBUG "obmc_console.hpp":241] Console Object path = /xyz/openbmc_project/console/hypervisor service = xyz.openbmc_project.Console.hypervisor Request target = /console/hypervisor [DEBUG "obmc_console.hpp":198] Console web socket path: /console/hypervisor Console unix FD: 12 duped FD: 13 [DEBUG "obmc_console.hpp":82] Reading from socket [INFO "obmc_console.hpp":154] Closing websocket. Reason: [DEBUG "obmc_console.hpp":162] Remove connection 0x1bc5234 from obmc console - Make sure that bad console path is failing properly due to DBUS error. [INFO "http_connection.hpp":209] Request: 0x1bd76a8 HTTP/1.1 GET /console/badconsoleid ::ffff:x.x.xx.xxx [DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console/<str>' 1 / 2 [DEBUG "obmc_console.hpp":212] Connection 0x1bdb67c opened [DEBUG "obmc_console.hpp":241] Console Object path = /xyz/openbmc_project/console/badconsoleid service = xyz.openbmc_project.Console.badconsoleid Request target = /console/badconsoleid [ERROR "obmc_console.hpp":174] Failed to call console Connect() method DBUS error: No route to host Change-Id: I9b617bc51e3ddc605dd7f4d213c805d05d2cfead Signed-off-by: Ninad Palsule <ninad@linux.ibm.com> Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-01Server-sent-event fixesEd Tanous1-26/+6
This makes several changes to server-sent events to allow it to merge to master. The routing system has been removed in leiu of using content-type eventstream detection. Timers have been added to the sse connections, and sse connections now rely on async_wait, rather than a full read. Tested: WIP Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id0ff0ebc2b3a795b3dba008e440556a9fdd882c2
2023-06-01Add Server-Sent-Event supportV-Sanjana1-0/+91
Server-Sent-Event is a standard describing how servers can initiate data transmission towards clients once an initial client connection has been established. Unlike websockets (which are bidirectional), Server-Sent-Events(SSE) are unidirectional and commonly used to send message updates or continuous data streams to a browser client. This is base patch for adding Server-Sent-Events routing support to bmcweb. Redfish EventService SSE style subscription uses SSE route for sending the Events/MetricReports to client which establishes the connection. Tested this patch with along with EventService SSE support patches and verified the functionalty on browser. Tested: - Tested using follow-up patches on top which adds support for Redfish EventService SSE style subscription and observed events are getting sent periodically. - Created SSE subscription from the browser by visiting https://<BMC IP>/redfish/v1/EventService/SSE Change-Id: I36956565cbba30c2007852c9471f477f6d1736e9 Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com> Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com> Signed-off-by: V-Sanjana <sanjana.v@intel.com>
2023-05-20Added new pre-defined usergroup called hostconsoleNinad Palsule1-13/+40
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-16Boost::urls::formatEd Tanous1-3/+5
Boost 1.82 dropped a lovely new toy, boost::urls::format, which is a lot like our urlFromPieces method, but better in that it makes the resulting uris more readable, and allows doing things like fragments in a single line instead of multiple. We should prefer it in some cases. Tested: Redfish service validator passes. Spot checks of URLs work as expected. Unit tests pass. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ia7b38f0a95771c862507e7d5b4aa68aa1c98403c
2023-05-11clang-format: copy latest and re-formatPatrick Williams1-21/+12
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-27Remove nameStr from routerEd Tanous1-26/+2
It isn't used anywhere in the code, so it can be removed, and the router simplified. These common data structures have caused problems, in that they're not copied to child handlers, and cause bugs like #249. Tested: Redfish service validator passes. Basic sanity tests of both static file routes such as $metadata (which use DynamicRule) as well as method routes, such as /redfish/v1, return valid data. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I93ad74581912e18ee5db9aaa9ecdaf08ed765418
2023-04-27Remove number support from the routerEd Tanous1-247/+49
The router historically came from crow. Crow supported wildcards of <int>, <float>, and <double>. bmcweb doesn't use them, nor should it in basically any case, as we now have explicit 404 handling. This commit removes them. This amounts to about -450 lines of code, but it's some of the scarier code we have, some of it existing in the namespace "black_magic". Reducing the brain debt for people working in this subsystem seems worthwhile. There is no case in the future where we would use integer based url parameters. Tested: Redfish service validator passes. Should be good enough coverage for a code removal. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I34add8df7d3486952474ca7ec3dc6be990c50ed0
2023-04-18Copy privilegeset into websocket ruleEd Tanous1-0/+1
Resolves #249 Tested: Running websockets now shows rules being applied ``` Apr 10 20:26:35 p10bmc bmcweb[745]: (2023-04-10 20:26:35) [INFO "http_connection.hpp":209] Request: 0x11e3248 HTTP/1.1 GET /console0 ::ffff:X.XX.XX.XXX | Apr 10 20:26:35 p10bmc bmcweb[745]: (2023-04-10 20:26:35) [DEBUG "routing.hpp":1460] Matched rule (upgrade) '/console0' 1 / 2 Apr 10 20:26:36 p10bmc bmcweb[745]: (2023-04-10 20:26:36) [DEBUG "routing.hpp":1299] userName = admin userRole = priv-admin Apr 10 20:26:36 p10bmc bmcweb[745]: (2023-04-10 20:26:36) [DEBUG "routing.hpp":101] checkPrivileges: Active BASE priv: ConfigureManager <<<<<< Apr 10 20:26:36 p10bmc bmcweb[745]: (2023-04-10 20:26:36) [DEBUG "routing.hpp":101] checkPrivileges: Active BASE priv: ConfigureComponents <<<<<< Apr 10 20:26:36 p10bmc bmcweb[745]: (2023-04-10 20:26:36) [ERROR "routing.hpp":1361] isUserPrivileged: URL: /console0 IsPrivelegeSetEmpty: 0 Username: admin ``` Signed-off-by: Ed Tanous <edtanous@google.com> Tested-by: Ninad Palsule <ninadpalsule@us.ibm.com> Change-Id: Ia2eae1847822b50a425afd2e5e13b528393aa7ad
2023-03-23nbd proxy and websocket cleanupsEd Tanous1-2/+13
As-written, the nbd (and all websocket daemons) suffer from a problem where there is no way to apply socket backpressure, so in certain conditions, it's trivial to run the BMC out of memory on a given message. This is a problem. This commit implements the idea of an incremental callback handler, that accepts a callback function to be run when the processing of the message is complete. This allows applying backpressure on the socket, which in turn, should provide pressure back to the client, and prevent buffering crashes on slow connections, or connections with high latency. Tested: NBD proxy not upstream, no way to test. No changes made to normal websocket flow. Signed-off-by: Michal Orzel <michalx.orzel@intel.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I3f116cc91eeadc949579deacbeb2d9f5e0f4fa53
2023-03-22Fix Request use-after-moveJonathan Doman1-11/+10
Partial revert of 915d2d4e59be56958b04a79ba96e0242ef735f44 Request object was being moved out of the owning Connection object, which would then try to use the Request again in completeRequest(). Just pass around a reference instead of taking ownership. The obvious symptom was that Redfish pages were served as json in the browser instead of HTML, because the headers in the Request were no longer valid after being moved. Tested: /redfish/v1 is served as HTML in the browser again. Change-Id: Iae68a68817146c28377bbcade04716725e4a6096 Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
2023-03-17Fix some movesEd Tanous1-17/+22
This code is doing some copy operations instead of moves. This commit moves to passing a Request&& through the validate function, so that we don't have to split the usage of req between the two paths. Ideally someday we'd run Request as a shared_ptr like we do with Response and remove the possibility of this, but that's a longer term thing. This fixes a regression introduced in 7e9093e625961f533250a6c193c1a474e98007c4 Tested: Redfish service validator passes. /redfish/v1/Systems/system passes. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib6d99726a64326b7c8bad15bc9d4ca774ab6256d
2023-03-15Add Support for privilege check in handleUpgradeP Dheeraj Srujan Kumar1-45/+78
This commit enables privilege check for user(s) in case of upgraded connections. Currently users with no privileges will also be able to access Websockets connections (Ex: KVM). The privilege check was already in place for normal connections (i.e. router->handle()). This commit lifts off the privilege check code and moves it into a common function (validatePrivilege()), which can be used both by handle() and handleUpgrade() and register required callback to be called. Also, the const qualifier for Request in the handleUpgrade() function's signature is removed to enable setting "isConfigureSelf" field of request. The signature of handleUpgrade() is made identical to handle() Tested: - websocket_test.py Passed - Admin and Operator users are able to access KVM on WebUI - Readonly User was unable to access KVM on WebUI Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com> Change-Id: I6f743c27e7e6077f1c6c56e6958922027e4404e8
2023-03-15Remove try-catch blocks on handleUpgradeEd Tanous1-22/+2
handleUpgrade is pretty simple, and has no methods that can throw. This was there previously because of handling exceptions in handle() and was copied to handleUpgrade(), even though it doesn't make a ton of sense to do so, given the throw conditions don't really exist, and start() doesn't call path handlers directly anymore. Tested: Code compiles. Only affects error conditions. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iedd7e42b7e908282ab2c2d698e9f6c815b88e857
2023-03-15Move validation code to unpackPropertiesNoThrowEd Tanous1-32/+28
Tested: Tested in 46991 Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ia714c7de02d714e636d5624ea884dbb6633baee5
2023-03-15Break out large lambda into callbackEd Tanous1-96/+108
This lambda was very large, and needs broken into a method. Tested: Tested in 46991 Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I86c1ac749580eb5b42c347808b4660c894a9bb9b
2023-03-15Add asyncResp support to handleUpgradeP Dheeraj Srujan Kumar1-21/+23
This commit enables passing down the asyncResp (of the connection) to the handler of upgraded connections. This is already in place for normal requests (i.e. Class Router -> handle()) This change would enable any async calls that would be required before upgrade of the connection. For example, as on today, we have only Authentication of user in place for upgraded connection, but not Authorization. So, this asyncResp could further be used for such dbus calls to return informative response. This commit updates the signature of all the handleUpgrade() functions present in router.hpp to take in asyncResp object instead of normal response. Tested : - websocket_test.py Passed - KVM was functional in WebUI. Change-Id: I1c6c91f126b734e1b5573d5ef204fe2bf6ed6c26 Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com>
2023-03-11Make url by value in RequestEd Tanous1-8/+13
There's some tough-to-track-down safety problems in http Request. This commit is an attempt to make things more safe, even if it isn't clear how the old code was wrong. Previously, the old code took a url_view from the target() string for a given URI. This was effectively a pointer, and needed to be updated in custom move/copy constructors that were error prone to write. This commit moves to taking the URI by non-view, which involves a copy, but allows us to use the default move and copy constructors, as well as have no internal references within Request, which should improve the safety and reviewability. There's already so many string copies in bmcweb, that this is unlikely to show up as any sort of performance regression, and simple code is much better in this case. Note, because of a bug in boost::url, we have to explicitly construct a url_view in any case where we want to use segments() or query() on a const Request. This has been reported to the boost maintainers, and is being worked for a long term solution. https://github.com/boostorg/url/pull/704 Tested: Redfish service validator passed on last commit in series. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I49a7710e642dff624d578ec1dde088428f284627
2023-02-24Take boost error_code by referenceEd Tanous1-1/+1
By convention, we should be following boost here, and passing error_code by reference, not by value. This makes our code consistent, and removes the need for a copy in some cases. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id42ea4a90b6685a84818b87d1506c11256b3b9ae
2023-02-24Pass string views by valueEd Tanous1-3/+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-1/+1
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-12-28Add missing usage of new verb class in routerSnehalatha Venkatesh1-4/+4
Despite introduction of a new enum class containing method verbs, some functions were still using the one from Boost. This had caused erratic behaviour when trying to create a websocket (e.g. /nbd/<str>), because enum value of old type was compared to the one of new type. This change fixes that. Tested: Verified that websockets are now created without errors. Change-Id: I52c874de9b02463618143d3b031f5c795dd42ad8 Signed-off-by: Michal Orzel <michalx.orzel@intel.com> Signed-off-by: Snehalatha Venkatesh <snehalathax.v@intel.com>
2022-12-07Implement If-Match header in Http layerEd Tanous1-5/+5
If-Match is a header in the HTTP specification[1] designed for handling atomic operations within a given HTTP tree. It allows a mechanism for an implementation to explicitly declare "only take this action if the resource has not been changed". While most things within the Redfish tree don't require this level of interlocking, it continues to round out our redfish support for the specific use cases that might require it. Redfish specification 6.5 states: If a service supports the return of the ETag header on a resource, the service may respond with HTTP 428 status code if the If-Match or If-None-Match header is missing from the PUT or PATCH request for the same resource, as specified in RFC6585 This commit implements that behavior for all handlers to follow the following flow. If If-Match is present Repeat the same request as a GET Compare the ETag produced by the GET, to the one provided by If-Match If they don't match, return 428 if they do match, re-run the query. [1] https://www.rfc-editor.org/rfc/rfc2616#section-14.24 As a consequence, this requires declaring copy and move constructors onto the Request object, so the request object can have its lifetime extended through a request, which is very uncommon. Tested: Tests run on /redfish/v1/AccountService/Accounts/root PATCH with correct If-Match returns 200 success PATCH with an incorrect If-Match returns 419 precondition required GET returns the resource as expected Redfish service validator passes Redfish protocol validator passes! ! ! ! ! Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I530ab255259c32fe4402eb8e5104bd091925c77b
2022-12-06Code move to prevent circular dependencyEdward Lee1-8/+0
While implementing https://gerrit.openbmc.org/c/openbmc/bmcweb/+/57932, there has been an issue where there is a circular dependency between routing.hpp and privileges.hpp. This code move predates this change to resolve it before implementing the heart of redfish authz. Circular dependency will occur when we try to use the http verb index variables in privilege.hpp. If this occurs routing.hpp and privilege.hpp will co-depend on each other and this code move prevents this from occuring. Tested: bitbake bmcweb Code compiles (code move only) Redfish Validator passed on next commit Signed-off-by: Edward Lee <edwarddl@google.com> Change-Id: I46551d9fe222e702d239ed3ea6d3d7e505d488c8
2022-12-06Make router take up less space for verbsEd Tanous1-18/+27
As is, the router designates routes for every possible boost verb, of which there are 31. In bmcweb, we only make use of 6 of those verbs, so that ends up being quite a bit of wasted space and cache non-locality. This commit invents a new enum class for declaring a subset of boost verbs that we support, and a mapping between bmcweb verbs and boost verbs. Then it walks through and updates the router to support converting one to another. Tested: Unit Tested Redfish Service Validator performed on future commit Signed-off-by: Ed Tanous <edtanous@google.com> Signed-off-by: Edward Lee <edwarddl@google.com> Change-Id: I3c89e896c632a5d4134dbd08a30b313c12a60de6
2022-09-01Add Method Not Allowed (405) handlerEd Tanous1-31/+53
Similar to the 404 handler, add a 405 handler for registering custom 405 handlers for a given tree. The primary use case is for protocols like redfish that support specific messages for 405 handlers that don't have an empty body. Tested: Unit tests pass. PATCH /redfish/v1 returns 405 Method Not Allowed POST /redfish/v1/Chassis returns 405 Method Not Allowed POST /redfish/v1/Chassis/foo returns 405 Method Not Allowed PATCH /redfish/v1/foo/bar returns 404 Not Found GET /redfish/v1 returns ServiceRoot GET /redfish/v1/Chassis returns Chassis collection Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib0afd23d46bb5b88f89cf1e3f4e0606a48ae47ca Signed-off-by: Carson Labrado <clabrado@google.com>
2022-09-01Allow custom 404 handlersEd Tanous1-68/+118
Different HTTP protocols have different http responses for 404. This commit adds support for registering a route designed to host a handler meant for when a response would otherwise return. This allows registering a custom 404 handler for Redfish, for which all routes will now return a Redfish response. This was in response to the 404 handler not working in all cases (in the case of POST/PATCH/DELETE). Allowing an explicit registration helps to give the intended behavior in all cases. Tested: GET /redfish/v1/foo returns 404 Not found PATCH /redfish/v1/foo returns 404 Not found GET /redfish/v1 returns 200 OK, and content PATCH /redfish/v1 returns 405 Method Not Allowed With Redfish Aggregation: GET /redfish/v1/foo gets forwarded to satellite BMC PATCH /redfish/v1/foo does not get forwarded and returns 404 PATCH /redfish/v1/foo/5B247A_bar gets forwarded Unit tests pass Redfish-service-validator passes Redfish-Protocol-Validator fails 7 tests (same as before) Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I731a5b4e736a2480700d8f3e81f9c9c6cbe6efca Signed-off-by: Carson Labrado <clabrado@google.com>
2022-08-10routing: fix some commentsNan Zhou1-3/+3
Found these when I went through the code path of authx. Tested: comment only changes. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Id80725f4bf5f3972e975347dcac8598e2ffab332
2022-08-01ServiceRoot Support Link headerEd Tanous1-6/+0
The Redfish standard in section 8.2 states: A Link header containing rel=describedby shall be returned on GET and HEAD requests for Redfish resources. If the referenced JSON Schema is a versioned schema, it shall match the version contained in the value of the @odata.type property returned in this resource. This commit attempts to add this capability to ServiceRoot. Future similar patches will start adding this across the tree. To do this, a few things happen. First, this removes the implicit HEAD handling in the router. Because we now need explicit HEAD handling per-route with specific headers, there's no good way to make this generic. Next, it rearranges the code such that handleServiceRootGet can first call handleServiceRootHead, to avoid duplicating the addHeader call. Tested: Redfish protocol validator passes the RESP_HEADERS_REL_LINK_DESCRIBED_BY check for ServiceRoot. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I92248089a3545432c14f551309ea62332e554647
2022-06-30Require explicit decorator on one arg constructorsEd Tanous1-5/+5
We essentially follow this rule already, not relying on implicit operators, although there are a number of cases where in theory we could've implicitly constructed an object. This commit enables the clang-tidy check. Tested: Code compiles, passes clang-tidy. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ia428463313b075c69614fdb326e8c5c094e7adde
2022-06-01Try to fix the lambda formatting issueEd Tanous1-84/+80
clang-tidy has a setting, LambdaBodyIndentation, which it says: "For callback-heavy code, it may improve readability to have the signature indented two levels and to use OuterScope." bmcweb is very callback heavy code. Try to enable it and see if that improves things. There are many cases where the length of a lambda call will change, and reindent the entire lambda function. This is really bad for code reviews, as it's difficult to see the lines changed. This commit should resolve it. This does have the downside of reindenting a lot of functions, which is unfortunate, but probably worth it in the long run. All changes except for the .clang-format file were made by the robot. Tested: Code compiles, whitespace changes only. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43
2022-05-17Handle HEAD and Allow headers per the specEd Tanous1-11/+42
The Redfish specification calls out that the Allow header should be returned for all resources to give a client an indication of what actions are allowed on that resource. The router internally has all this data, so this patchset allows the router to construct an allow header value, as well as return early on a HEAD request. This was reverted once here: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/53637 Due to a redfish validator failure. With the previous patches workaround, this error has now been resolved. Tested: Called curl with various parameters and observed the Allow header curl -vvvv --insecure -X <VERB> --user root:0penBmc https://<bmc>/url HEAD /redfish/v1/SessionService/Sessions returned Allow: GET, POST HEAD /redfish/v1 returned Allow: GET HEAD /redfish/v1/SessionService returned Allow: GET, PATCH POST /redfish/v1 returned Allow: GET (method not allowed) GET /redfish/v1 returned Allow: GET GET /redfish/v1/SessionService returned Allow: GET, PATCH Redfish-Protocol-Validator now reports more tests passing. Prior to this patch: Pass: 255, Warning: 0, Fail: 27, Not tested: 45 After this patch: Pass: 262, Warning: 0, Fail: 21, Not tested: 43 Diff: 7 more tests passing All tests under RESP_HEADERS_ALLOW_METHOD_NOT_ALLOWED and RESP_HEADERS_ALLOW_GET_OR_HEAD are now passing Included unit tests passing. Redfish service validator is now passing. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ibd52a7c2babe19020a0e27fa1ac79a9d33463f25
2022-05-11Revert "Handle HEAD and Allow headers per the spec"Ed Tanous1-42/+11
This reverts commit 867b2056d44300db9769e0d0b8883435a179834c. Apparently we have broken the Redfish spec in a way that adding this feature now allows the service validator to find. @odata.id /redfish/v1/UpdateService ERROR - Allow header should NOT contain POST for UpdateService.v1_5_0.UpdateService Need to figure out what to do, but for now, revert to get the build passing again. Change-Id: Ieef20573b9caa03aba6fd2bbc999e517e4b7de3d Signed-off-by: Ed Tanous <edtanous@google.com>
2022-05-10Handle HEAD and Allow headers per the specEd Tanous1-11/+42
The Redfish specification calls out that the Allow header should be returned for all resources to give a client an indication of what actions are allowed on that resource. The router internally has all this data, so this patchset allows the router to construct an allow header value, as well as return early on a HEAD request. Tested: Called curl with various parameters and observed the Allow header curl -vvvv --insecure -X <VERB> --user root:0penBmc https://<bmc>/url HEAD /redfish/v1/SessionService/Sessions returned Allow: GET, POST HEAD /redfish/v1 returned Allow: GET HEAD /redfish/v1/SessionService returned Allow: GET, PATCH POST /redfish/v1 returned Allow: GET (method not allowed) GET /redfish/v1 returned Allow: GET GET /redfish/v1/SessionService returned Allow: GET, PATCH Redfish-Protocol-Validator now reports more tests passing. Prior to this patch: Pass: 255, Warning: 0, Fail: 27, Not tested: 45 After this patch: Pass: 262, Warning: 0, Fail: 21, Not tested: 43 Diff: 7 more tests passing All tests under RESP_HEADERS_ALLOW_METHOD_NOT_ALLOWED and RESP_HEADERS_ALLOW_GET_OR_HEAD are now passing Included unit tests passing. Change-Id: Ib99835050b15eb4f419bfd21375b26e4db74fa2c Signed-off-by: Ed Tanous <edtanous@google.com>
2022-04-29Delete forked function_traitsEd Tanous1-5/+5
The function_traits class was very clearly "borrowed" from boost::function traits, then added to to support lambdas. boost::function_traits has been superceeded by boost::callable_traits, which fixes the same shortcomings that we have fixed here. This commit replaces almost the entirety of the uses of function_traits with callable traits, with one exception: arg<i>. In the callable traits model, arg_t is a std::tuple, which, while better, doesn't unpack easily into a variadic pack that our router code expects. Ideally, at some point, we would rewrite the router core to not rely on std::make_integer_sequence, but that's a much more invasive change. Tested: Called curl --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/Managers/bmc/LogServices/Journal/Entries/1646953359619803 and verified callback return the correct result (not 404). That API has several flexible router parameters, which is the only thing this commit could break. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Icf3299b2d5c1a5ff111f68858bb46139735aaabe
2022-03-29Remove AsyncResp from openHandlerzhanghch051-3/+1
This change, moving the openHandler back to only supporting websocket disconnects and not 404s.Because AsyncResp is removed from openHandler. Tested: (from previous commit) Opened KVM in webui-vue and it works. Signed-off-by: zhanghaicheng <zhanghch05@inspur.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I793f05836aeccdc275b7aaaeede41b3a2c276595
2022-03-22Consitently use dbus::utility typesEd Tanous1-39/+37
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-03-17Remove special router logic for trailing slashEd Tanous1-51/+2
The trailing slash logic in the router has been long since deprecated in leiu of adding two routes internally, so this "special case" is no longer needed or used, as can be seen from the variable being read, but never set anywhere. Tested: curl --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/SessionService/Sessions/ Succeeds Ran redfish service validator. No new failures. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I9a6744d311aacaed1cc3eb3a98d55006c5b4246d
2022-03-01Make the router const correctEd Tanous1-9/+12
Subtly, the individual members of a const std::pair are not implicitly const. In most cases, this is solved by a compiler error, but it seems that flat_map allows implicitly pulling out by a non const reference, even when the underlying container is const. This is not how the maps should work. This commit changes the router to declare a "ChildMap" type, which can then use the value_type to make this const correctness stuff more reasonable to manage. Tested: Code compiles. No-op const change. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id99079a86e392a03416a69506934dbfff7bc3b29
2022-02-28websocket: fix authlessNan Zhou1-0/+2
We should check if session is nullptr before referencing its data member. Tested: 1. build authless BMCWeb ``` meson -Drest=enabled -Dbmcweb-logging=enabled -Dinsecure-disable-auth=enabled build && ninja -C build && ./build/bmcweb ``` 2. start websocket client without problems ``` python scripts/websocket_test.py --host localhost:18080 ``` 3. bmcweb log ``` [DEBUG "websocket.hpp":221] Websocket accepted connection [DEBUG "dbus_monitor.hpp":114] Connection opened [DEBUG "dbus_monitor.hpp":115] Connection 0x55b22d618670 opened [DEBUG "http_response.hpp":134] 0x55b22d611040 calling completion handler [DEBUG "dbus_monitor.hpp":129] Connection 0x55b22d618670 received {"paths": ["/xyz/openbmc_project/sensors"], "interfaces": ["xyz.openbmc_project.Sensor.Value"]} [DEBUG "dbus_monitor.hpp":231] Creating match type='signal',interface='org.freedesktop.DBus.Properties', path_namespace='/xyz/openbmc_project/sensors',member='PropertiesChanged', arg0='xyz.openbmc_project.Sensor.Value' [DEBUG "dbus_monitor.hpp":246] Creating match type='signal',interface='org.freedesktop.DBus.ObjectManager', path_namespace='/xyz/openbmc_project/sensors',member='InterfacesAdded' ``` Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I56613a26c129736f0e6980bb24e83f22ef60eea0
generated by cgit v1.2.3 (git 2.39.0) at 2024-06-01 14:22:55 +0300