summaryrefslogtreecommitdiff
path: root/include
AgeCommit message (Collapse)AuthorFilesLines
2023-06-01Simplify obmc console buffersEd Tanous1-11/+15
Backpressure on incoming bytes helps both to simplify the layering of the console, as well as prevent some cases of OOM crashes. Similar to what we did with nbd_proxy, move obmc console over to the new sendEx interface, allowing for backpressure, and fixed size std::array buffers. Tested: Made sure single console can see the data. Made sure two consoles can see the data. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I63d142fc5e8f8a734f3a7b8d0aa3f0d8c263d5ba
2023-05-31Introduce ConsoleHandler class under obmc_consoleNinad Palsule1-143/+159
Added new ConsoleHandler class to prepare for the multiple consoles support. All global fields are moved to the ConsoleHandler class and a new global map added to remember the ConsoleHandler for each console path. There is single ConsoleHandler per connection so we don't need session map per route. There is a limit added for max number of connection allowed to avoid any service attacks. Testing: - Make sure that single console works fine and data is seen on the console. - Make sure that multiple consoles of type host console are created and data is seen on all consoles. Also using traces made sure that new handlers are destroyed. Traces: Traces shows that multiple consoles active and later destroyed. [INFO "http_connection.hpp":209] Request: 0x24bb790 HTTP/1.1 GET /console0 ::ffff:x.xx.xxx.xx [DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console0' 1 / 2 [DEBUG "obmc_console.hpp":238] Connection 0x24eb424 opened [DEBUG "obmc_console.hpp":150] Obmc handler 0x24c18fc added 1 for path 0x24eb424 [DEBUG "obmc_console.hpp":257] Console Object path = /xyz/openbmc_project/console/default service = xyz.openbmc_project.Console.default Request target = /console0 [DEBUG "obmc_console.hpp":224] Console web socket path: /console0 Console unix FD: 13 duped FD: 14 [DEBUG "obmc_console.hpp":44] Outbuffer empty. Bailing out [INFO "http_connection.hpp":209] Request: 0x265d740 HTTP/1.1 GET /console0 ::ffff:x.xx.xxx.xx [DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console0' 1 / 2 [DEBUG "obmc_console.hpp":238] Connection 0x2661de4 opened [DEBUG "obmc_console.hpp":150] Obmc handler 0x25e69ac added 1 for path 0x2661de4 [DEBUG "obmc_console.hpp":257] Console Object path = /xyz/openbmc_project/console/default service = xyz.openbmc_project.Console.default Request target = /console0 [DEBUG "obmc_console.hpp":224] Console web socket path: /console0 Console unix FD: 19 duped FD: 20 [DEBUG "obmc_console.hpp":44] Outbuffer empty. Bailing out [INFO "http_connection.hpp":209] Request: 0x265d740 HTTP/1.1 GET /console0 ::ffff:x.xx.xxx.xx [DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console0' 1 / 2 [DEBUG "obmc_console.hpp":238] Connection 0x25f1fdc opened [DEBUG "obmc_console.hpp":150] Obmc handler 0x26ff22c added 1 for path 0x25f1fdc [DEBUG "obmc_console.hpp":257] Console Object path = /xyz/openbmc_project/console/default service = xyz.openbmc_project.Console.default Request target = /console0 [DEBUG "obmc_console.hpp":224] Console web socket path: /console0 Console unix FD: 19 duped FD: 21 [DEBUG "obmc_console.hpp":44] Outbuffer empty. Bailing out [INFO "obmc_console.hpp":177] Closing websocket. Reason: [DEBUG "obmc_console.hpp":184] Remove connection 0x25f1fdc from obmc handler 0x26ff22c for path /console0 [INFO "obmc_console.hpp":177] Closing websocket. Reason: [DEBUG "obmc_console.hpp":184] Remove connection 0x2661de4 from obmc handler 0x25e69ac for path /console0 [INFO "obmc_console.hpp":177] Closing websocket. Reason: [DEBUG "obmc_console.hpp":184] Remove connection 0x24eb424 from obmc handler 0x24c18fc for path /console0 Change-Id: I77a58a3a186e87611219aed90b221f9b8be7fa2f Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
2023-05-30Allow async resolver to be optionalEd Tanous1-8/+32
This commit adds a meson option to allow selecting which dns resolver bmcweb uses. There are use cases, like Open Compute Project Inband Management Agent, that would require not using dbus, which would require us to fall back to the asio resolver. This commit makes the existing asio resolver constructor, and async_resolve methods match the equivalents in asio (which we intended to do anyway), then adds a macro and configure option for being able to select which resolver backend to rely on. Tested: Code can now compile without sdbusplus. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I3220214367179f131a60082bdfaf7e725d35c125
2023-05-26Add maybe_unused to possibly unused argumentEd Tanous1-1/+1
There are cases in this method where if CSRF protection is disabled, this argument will not be used, and will trigger a compile error. This commit fixes the compile error. Tested: Code compiles with CSRF disabled option set. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I6daa5193fa162c73c57991600058c198dc38a418
2023-05-26Make all std::regex instances staticEd Tanous3-5/+5
Per [1] we really shouldn't be using regex. In the cases we do, it's a HUUUUUGE performance benefit to be compiling the regex ONCE. The only downside is a slight increase in memory usage. [1]: https://github.com/openbmc/bmcweb/issues/176 Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I8644b8a07810349fb60bfa0258a13e815912a38e
2023-05-25Fix some includesEd Tanous5-5/+5
System includes should be included with <>, in-tree includes should be included with "". This was found manually, with the help of the following grep statement[1]. git grep -o -h "#include .*" | sort | uniq Tested: Code compiles Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I1a6b2a5ba35ccbbb61c67b7c4b036a2d7b3a36a3
2023-05-25Use console information from DBUSNinad Palsule1-31/+122
This drop is a preparation of supporting multiple consoles in bmcweb. In this drop we will hook up the new DBUS interface to get the unix socket file descriptor for existing host console. At this time bmcweb only allows host console. This drop includes following bmcweb changes: - The default console leaf node is set to "default" by the obmc-console - Currently the URL is still maintained to /console0 for GUI compatibility. In future, it will be changed to /console/<str> where <str> could be any string which represents the console id. - In the obmc routing function, query the console DBUS interface for all available consoles. If the object leaf matches with the target string, then create socket and assign the file descriptor returned by the DBUS console. [INFO "http_connection.hpp":209] Request: 0x1b8c608 HTTP/1.1 GET /console0 ::ffff:x.xxx.xx.xxx | [DEBUG "routing.hpp":1440] Matched rule (upgrade) '/console0' 1 / 2 | [DEBUG "obmc_console.hpp":247] Connection 0x13e3c8c opened [DEBUG "obmc_console.hpp":268] Console Object path = \ /xyz/openbmc_project/console/default Request target = /console0 [DEBUG "obmc_console.hpp":230] Looking up unixFD for Service \ xyz.openbmc_project.Console.default Path /xyz/openbmc_project/console/default [DEBUG "obmc_console.hpp":157] Console web socket path: /console0 Console\ unix FD: 12 duped FD: 13 Testing: Make sure that console open is working for /console0 on rainier machine Related commits: 1) phosphor-dbus-interface: https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/61486 2) obmc-console: https://gerrit.openbmc.org/c/openbmc/obmc-console/+/62496 3) bmcweb: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/62525 Change-Id: I476f1bb3e3be384ab09802340a59ffa036ca0278 Signed-off-by: Ninad Palsule <ninadpalsule@us.ibm.com>
2023-05-24Add Links/Triggers to MetricReportDefinitionSzymon Dompke1-0/+1
This change is adding Triggers property to Links when GET is called on MetricReportDefinition. It contains array of @odata.id pointing to Trigger resource if it is also linking to given MRD. Testing done: - Links/Trigger property is returned by GET request on /redfish/v1/TelemetryService/MetricReportDefinitions/<str>/ Signed-off-by: Szymon Dompke <szymon.dompke@intel.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5accf4b50324437b0b185003200078ad2c7020b0
2023-05-23Switched bmcweb to use new telemetry service APIKrzysztof Grobelny1-3/+4
Added support for multiple MetricProperties. Added support for new parameters: CollectionTimeScope, CollectionDuration. ReadingParameters was not yet changed in telemetry backend, instead temporary property ReadingParametersFutureVersion was introduced. Once bmcweb is adapted to use ReadingParametersFutureVersion this property will be renamed in backend to ReadingParameters. Then bmcweb will change to use ReadingParameters. Then ReadingParametersFutureVersion will be removed from backend and everything will be exactly like described in phosphor-dbus-interfaces without introducing breaking changes. Related change in phosphor-dbus-interfaces [1], [2]. This change needs to be bumped together with [3]. Tested: - It is possible to create MetricReportDefinitions with multiple MetricProperties. - Stub values for new parameters are correctly passed to telemetry service. - All existing telemetry service functionalities remain unchanged. [1]: https://github.com/openbmc/phosphor-dbus-interfaces/commit/4f9c09144b60edc015291d2c120fc5b33aa0bec2 [2]: https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/60750 [3]: https://gerrit.openbmc.org/c/openbmc/telemetry/+/58229 Change-Id: I2cd17069e3ea015c8f5571c29278f1d50536272a Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com> Signed-off-by: Lukasz Kazmierczak <lukasz.kazmierczak@intel.com>
2023-05-20Added new pre-defined usergroup called hostconsoleNinad Palsule2-1/+3
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-19Capture all boost::system::error_codes by refEd Tanous2-2/+2
Capturing these possibly overloaded values by reference can avoid a copy in some cases, and it's good to be consistent. This change was made automatically by grep/sed. Tested: Code compiles. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iafeaca2a5dc52f39753b5a3880419d6bc943f81b
2023-05-19Fix integer display in HTMLJason M. Bills1-1/+1
When displaying an integer in HTML, the number of characters is off by 1 causing the numbers to display without the last digit. This is because the pointer into numberbuffer gets initially advanced by the number of characters which ends up being one too many. For example, the buffer pointer is pointing at numberbuffer[0]. If we display a 4 digit number, we advance by 4, so it ends up pointing at numberbuffer[4] for the last digit. In the end, we return only the number of characters which is numberbuffer[0-3] cutting off the last digit. This changes the pointer to advance by one less than the number of digits, so the buffer fills from numberbuffer[3] and returns all 4 digits. Tested: Read redfish/v1/SessionService and confirmed that the "SessionTimeout" value displays the full value of 1800. Change-Id: Iaa974d7a41352fd9a15024ccf04c5926a4efe7a2 Signed-off-by: Jason M. Bills <jason.m.bills@intel.com>
2023-05-17Remove bad include from dbus singletonEd Tanous1-1/+0
Neither of these files make use of io_context, so they shouldn't be here Tested: Code compiles Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I86c195a5881a50a275e977aa73b9d7144b53844b
2023-05-16Boost::urls::formatEd Tanous1-2/+3
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-12fix more push vs emplace callsPatrick Williams3-14/+14
It seems like clang-tidy doesn't catch every place that an emplace could be used instead of a push. Use a few grep/sed pairs to find and fix up some common patterns. Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I93eaec26b8e3be240599e92b66cf54947073dc4c
2023-05-12ibm-locks: shut up clang-tidy by disabling codePatrick Williams1-2/+9
There is code in ibm/locks that has had clang-tidy warnings disabled for a while due to multiple safety and endianness issues. The code has not been fixed in a while and with clang-16 it is unable to be exempted further. Disable it until someone who cares can fix this in the proper way. ``` ../include/ibm/locks.hpp:522:14: error: 'p' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage] uint8_t* p = reinterpret_cast<uint8_t*>(&resourceId1); ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../include/ibm/locks.hpp:527:25: note: used in buffer access here uint8_t pPosition = p[position]; ^ ../include/ibm/locks.hpp:524:14: error: 'q' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage] uint8_t* q = reinterpret_cast<uint8_t*>(&resourceId2); ~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../include/ibm/locks.hpp:529:25: note: used in buffer access here uint8_t qPosition = q[position]; ``` Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I8a7fcbed1099419ad1715c86ffcbfef20820251e
2023-05-12use emplace where appropriate per clang-tidyPatrick Williams2-4/+3
The clang-tidy warning 'modernize-use-emplace' correctly flags a few places where emplace should be used over push. Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I6ca79285a87d6927e718345dc8dce0387e6b1eda
2023-05-12log-services: fix clang-tidy warningsPatrick Williams1-5/+3
A number of similar warnings about unsafe pointer arithmetic. ``` ../redfish-core/lib/log_services.hpp:269:39: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] indexStr.data(), indexStr.data() + indexStr.size(), index); ``` Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: Icc4a0d2f418c76d6987ef2318b0098d30d116389
2023-05-11json-html-serializer: fix clang-tidy warningPatrick Williams1-25/+26
``` ../include/json_html_serializer.hpp:426:42: error: 'end' declared with a const-qualified typedef; results in the type being 'char *const' instead of 'const char *' [misc-misplaced-const,-warnings-as-errors] const std::array<char, 64>::iterator end = ^ /usr/lib/gcc/x86_64-pc-linux-gnu/12/include/g++-v12/array:106:44: note: typedef declared here typedef value_type* iterator; ../include/json_html_serializer.hpp:165:60: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] std::snprintf(stringBuffer.data() + bytes, 7, ../include/json_html_serializer.hpp:327:11: error: 'bufferPtr' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage] auto* bufferPtr = begin(numberbuffer); ../include/json_html_serializer.hpp:393:11: error: 'begin' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage] char* begin = numberbuffer.data(); ../include/json_html_serializer.hpp:425:56: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage] std::remove(numberbuffer.begin(), numberbuffer.begin() + len, ','); ``` Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: If6aaf038c939ad76da73e68e746a56b0905b2804
2023-05-11pam-authenticate: fix clang-tidy warningPatrick Williams1-6/+9
``` ../include/pam_authenticate.hpp:11:75: error: 'msg' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage] inline int pamFunctionConversation(int numMsg, const struct pam_message** msg, ``` Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: Ic0e6d63b01eea78cac54407246363177cb208f8b
2023-05-11multipart-parser: fix clang-tidy issuesPatrick Williams1-18/+11
``` ../include/multipart_parser.hpp:77:21: error: 'buffer' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage] const char* buffer = req.body().data(); ../include/multipart_parser.hpp:246:38: error: 'buffer' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage] void skipNonBoundary(const char* buffer, size_t len, size_t boundaryEnd, ``` Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: Iad3b4b241ec75a152e240755a307a970798079fb
2023-05-11human-sort: fix clang-tidy warnings about pointer arithmeticPatrick Williams1-6/+6
Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I8122bfffaac469ee551c72632b671e8644a4da44
2023-05-11clang-format: copy latest and re-formatPatrick Williams12-42/+29
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-25dbus_utility: Support new ObjectMapper methodsWilly Tu1-0/+36
The new ObjectMapper methods are added as part of https://gerrit.openbmc.org/c/openbmc/phosphor-objmgr/+/57822 - GetAssociatedSubTree - GetAssociatedSubTreePaths The two methods are meant to be used to replace places where two dbus calls are used to get subtree and then get associated objects. Change-Id: I80a7ea935700a1ac5aebe6271f242aa103cc3d59 Signed-off-by: Willy Tu <wltu@google.com>
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-03-23nbd proxy and websocket cleanupsEd Tanous2-112/+114
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-23Break out another lambdaEd Tanous1-60/+66
Tested: No way to test. Non-upstream backend, inspection only. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib2593b66407e0f102f543777ecf907b434acac52
2023-03-23Remove authorization checks in nbd_proxyEd Tanous1-107/+57
nbd proxy should not have its own authorization checks, as these are now handled in the core as of 7e9093e625961f533250a6c193c1a474e98007c4 Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I8874d8a09278ba21d2acfdf076cb76dee718ecf4
2023-03-17dbus_rest: Fix dangling reference of crow::ResponseLei YU1-19/+20
The openbmc_dbus_reset was holding reference of `crow::Response`, set the response in `~InProgressActionData()`, and call res.end() to complete the result of the response. The bmcweb code now uses `std::shared_ptr<AsyncResp>` for the response and the `res.end()` is handled in `~AsyncResp()`. By using the reference of `crow::Response`, the `InProgressActionData` is actually using a dangling reference because the `std::shared_ptr<AsyncResp>` is already destructed, and bmcweb will crash on `action` calls, or not crash but get invalid response, as it's undefined behavior. Fix the above issue by using `std::shared_ptr<AsyncResp>` to make sure the response is correctly handled. Tested: 1. Without the fix, bmcweb crashes, or get no json output response on the below method call, be noted that it's an invalid call: ``` $ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/deleteAll ``` 2. With the fix, bmcweb gives expected response: ``` $ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/deleteAll { "data": { "description": "The specified method cannot be found" }, "message": "404 Not Found", "status": "error" } $ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/DeleteAll { "data": null, "message": "200 OK", "status": "ok" } ``` Signed-off-by: Lei YU <yulei.sh@bytedance.com> Change-Id: I38ef34fe8ff18e4e127664c853c6792461f6edf8
2023-03-17Add the GetManagedObjects method to dbus_utilityGeorge Liu1-0/+15
There are currently many files that use the GetManagedObjects method. Since they are a general method, they are defined in the dbus_utility.hpp file and refactors them. Tested: 1. Built bmcweb successfully and Validator passes. 2. We got the same result as previously in the ethernet schema. Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: I0c25b7b6b9421bea46ff0afadbaa4783b704e664
2023-03-11Remove body member from RequestEd Tanous4-5/+6
Per cpp core guidelines, these should be methods. Tested: on last patchset of the series. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib16479db9d2b68da68e7ad6e825c7e205c64f1de
2023-03-09Move logging level to WARNING for 404Gunnar Mills1-3/+3
These are 4xx errors, 404 not found. Move the logging to WARNING so they don't log unless WARNING level is enabled. This follows the guidance in the commit below. Tested: None. Change-Id: I38b2bec64507d75286f79d61acf7a96226598e0b Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2023-03-06Move nbdproxy to non lambda methodsEd Tanous1-140/+144
In a similar way we've transformed code in bmcweb, move these callbacks to use non-lambdas, to simplify their use. Tested: No good test harness here. Inspection only, mechanical transform. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ic5c2c86fef0abfaadb07022123ad93914d5ddf69
2023-03-03Add the getAssociationEndPoints methodGeorge Liu1-0/+13
There are currently many files that use the get endpoints methods[1]. Since they are general methods, they are defined in the dbus_utility.hpp file and will be further refactored in subsequent patches. Since the current endpoints of phosphor-objmgr do not support object_path and fails in romulus CI[2], so we should revert to std::string. Also, Updated the populateSoftwareInformation method of sw_utils.hpp [1] https://github.com/openbmc/docs/blob/master/architecture/object-mapper.md#associations [2] https://gerrit.openbmc.org/c/openbmc/bmcweb/+/58924/22/include/dbus_utility.hpp#98 When an object with, for example, an object path of pathA uses the following values: ["foo", "bar", "pathB"] The mapper will create 2 new objects: pathA/foo pathB/bar Tested: Built bmcweb successuflly and Validator passes curl -k -H "X-Auth-Token: $token" -X GET https://${bmc}/redfish/v1/Managers/bmc { "@odata.id": "/redfish/v1/Managers/bmc", "@odata.type": "#Manager.v1_14_0.Manager", ... "FirmwareVersion": "2.14.0-dev-95-gea3949e76-dirty", ... } Tested: Validator passes Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: I32a2c663bf2b8c84517bd0ecb4ccba61ce87c7e2
2023-02-28sdbusplus: use shorter type aliasesPatrick Williams1-1/+1
The sdbusplus headers provide shortened aliases for many types. Switch to using them to provide better code clarity and shorter lines. Possible replacements are for: * bus_t * exception_t * manager_t * match_t * message_t * object_t * slot_t Change-Id: Id13cf695f25312a9561c0954f5ed133985ab1222 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
2023-02-28Remove sessions when user is deletedXie Ning2-0/+42
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-28Fixing blocking system call to async modeTroy Lee1-0/+2
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> Change-Id: Id81db4d512b5ea1222a145dc2b9e9907e8b0f084
2023-02-24Take boost error_code by referenceEd Tanous6-33/+34
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 Tanous8-15/+11
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-02-23Update most resources to use urlFromPiecesWilly Tu1-2/+2
Only id in event_service and account_service have not been updated due to the risk of it breaking the username/id. It will require further testing to verify. Use urlFromPieces wherever that is needed to insert a variable in the URI. Don't use urlFromPieces when it is hardcoded values. This allow us to control all resource URIs that is dynamically added and to sync with the current recommanded method for `@odata.id`. The goal is to have a common place to manage the url created from dbus-paths in order to manage/update it easily when needed. Tested: RedfishValidtor Passed for all resource including the sensors with the fragments. Change-Id: I95cdfaaee58fc7f21c95f5944e1e5c813b3215f2 Signed-off-by: Willy Tu <wltu@google.com> Signed-off-by: Ed Tanous <edtanous@google.com>
2023-02-21Disable token compress in strEd Tanous1-1/+2
There are certain cases where we use this split function, and we expect tokens to be read out. For example: /xyz/openbmc_project/sensors/unit/name Should split into a "" in the first position. This use case is not common, and a quick grep shows only two places in the code expect this behavior. Boost::split has this behavior already, which is what this function is emulating. While we could fix these, in the end they should be following the rules outlined in COMMON_ERRORS.md, which disallow this kind of parsing completely. Tested: New unit tests passing. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iec3dcbf2b495b2b3b4ed419172c4133b16f7c65d
2023-02-17Add option for validating content-type headerEd Tanous1-11/+38
For systems implementing to the OWASP security guidelines[1] (of which all should ideally) we should be checking the content-type header all times that we parse a request as JSON. This commit adds an option for parsing content-type, and sets a default of "must get content-type". Ideally this would not be a breaking change, but given the number of guides and scripts that omit the content type, it seems worthwhile to add a trapdoor, such that people can opt into their own model on how they would like to see this checking work. Tested: ``` curl --insecure -H "Content-Type: application/json" -X POST -D headers.txt https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName":"root", "Password":"0penBmc"}' ``` Succeeds. Removing Content-Type argument causes bmc to return Base.1.13.0.UnrecognizedRequestBody. [1] cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html Change-Id: Iaa47dd563b40036ff2fc2cacb70d941fd8853038 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-02-17Implement alternative to on boost::splitEd Tanous2-2/+26
boost::split has a documented false-positive in clang-tidy. While normally we'd handle this with NOLINTNEXTLINE, this doesn't appear to work in all cases. Unclear why, but seems to be due to some of our lambda callback complexity. Each of these uses is a case where we should be using a more specific check, rather than split, but for the moment, this is the best we have. Tested: clang-tidy passes. [1] https://github.com/llvm/llvm-project/issues/40486 Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I144c6610cb740287b7225e2be03b4142a64f9563
2023-02-13Fix a couple #includesEd Tanous2-2/+0
In the continual quest to get tidy passing when run in isolation, fix some more includes. This includes removing a circular #include to app.hpp. We don't use app.hpp in these files, which is why our code compiles but having this include it here causes a few circular dependencies app.hpp -> http_server.hpp -> persistent_data.hpp -> app.hpp. app.hpp -> http_server.hpp -> authentication.hpp -> app.hpp. This confuses clang when run on header files directly. Fix a couple more includes at the same time. Tested: Code compiles Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib62d78b49c7e38ef7061c9fbbf6b3d463f11917d
2023-02-13Change static to inlineEd Tanous1-1/+1
This function is declared in a header, it should be inline, not static. Tested: Code compiles and passes clang-tidy Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I6b05e3e302d11a64f97f9f444eb0dbc76db3cd70
2023-02-10Revert "Add the getAssociationEndPoints method"Gunnar Mills1-12/+0
This reverts commit 369ea3ffb0c76c33c7ccd0bbb0e8dcb0039cd285. bmcweb bumps are failing romulus qemu CI tests. This started with https://gerrit.openbmc.org/c/openbmc/openbmc/+/60786. https://gerrit.openbmc.org/c/openbmc/openbmc/+/60756 passed. Only 1 commit diff here. The manager call is failing here: ``` curl -k -H "X-Auth-Token: $token" -X GET https://${bmc}/redfish/v1/Managers/bmc { "@odata.id": "/redfish/v1/Managers/bmc", "@odata.type": "#Manager.v1_14_0.Manager", ... "UUID": "0623b376-dc4f-4a29-93e0-cc982bfb9aae", "error": { "@Message.ExtendedInfo": [ { "@odata.type": "#Message.v1_1_1.Message", "Message": "The request failed due to an internal service error. The service is still operational.", "MessageArgs": [], "MessageId": "Base.1.13.0.InternalError", "MessageSeverity": "Critical", "Resolution": "Resubmit the request. If the problem persists, consider resetting the service." } ], "code": "Base.1.13.0.InternalError", "message": "The request failed due to an internal service error. The service is still operational." } } ``` Let's get the bumps back to passing. Change-Id: Ia27b1a5024b480786cc776c4ab9586bd75bf1242 Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2023-02-10Remove unused gzip helperEd Tanous1-65/+0
This was used way back in the day when bmcweb static files were ungzipping their contents. Today we push that to the client. Remove. Tested: Code compiles. Unused file. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie798b5eeb8854a677725d90adead8c1bf00bb5f4
2023-02-09Add the getAssociationEndPoints methodGeorge Liu1-0/+12
There are currently many files that use the get endpoints methods[1]. Since they are general methods, they are defined in the dbus_utility.hpp file and will be further refactored in subsequent patches. Also, Updated the populateSoftwareInformation method of sw_utils.hpp [1] https://github.com/openbmc/docs/blob/master/architecture/object-mapper.md#associations When an object with, for example, an object path of pathA uses the following values: ["foo", "bar", "pathB"] The mapper will create 2 new objects: pathA/foo pathB/bar Tested: Built bmcweb successuflly and Validator passes curl -k -H "X-Auth-Token: $token" -X GET https://${bmc}/redfish/v1/Managers/bmc { "@odata.id": "/redfish/v1/Managers/bmc", "@odata.type": "#Manager.v1_14_0.Manager", ... "FirmwareVersion": "2.14.0-dev-95-gea3949e76-dirty", ... } Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: I6567f63ab63709504b46ed49b00055a8ffc34124
2023-01-24Refactor GetSubTree methodGeorge Liu3-18/+14
Since the GetSubTree method has been implemented in dbus_utility and this commit is to integrate all the places where the GetSubTree method is called, and use the method in dbus_utility uniformly. Tested: Redfish Validator Passed Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: If3852b487d74e7cd8f123e0efffbd4affe92743c
2023-01-19Removed checking cookie in mTLS authenticationKarol Niczyj1-16/+10
mTLS authentication should have the highest priority (according to code in [1]) so it shouldn't be affected by cookies. If you provide a valid certificate and a dummy cookie value, request will fail which means cookies had higher priority than mTLS. Tested: Follow the guide in [2] to create a valid certificate for a user that can access some resource (for example /redfish/v1/Chassis) and make two requests: curl --cert client-cert.pem --key client-key.pem -vvv --cacert CA-cert.pem https://BMC_IP/redfish/v1/Chassis curl --cert client-cert.pem --key client-key.pem -vvv --cacert CA-cert.pem https://BMC_IP/redfish/v1/Chassis -H "Cookie: SESSION=123" Before this change second request would fail with "401 Unauthorized" [1]: https://github.com/openbmc/bmcweb/blob/bb759e3aeaadfec9f3aac4485f253bcc8a523e4c/include/authentication.hpp#L275 [2]: https://github.com/openbmc/docs/blob/f4febd002df578bad816239b70950f84ea4567e8/security/TLS-configuration.md Signed-off-by: Karol Niczyj <karol.niczyj@intel.com> Signed-off-by: Boleslaw Ogonczyk Makowski <boleslawx.ogonczyk-makowski@intel.com> Change-Id: I5d6267332b7b97c11f638850108e671d0baa26fd