summaryrefslogtreecommitdiff
path: root/include
AgeCommit message (Collapse)AuthorFilesLines
2024-04-11Fix file removalEd Tanous2-3/+13
This code used std::remove, which is a mechanism for removing characters from strings. Clearly it meant std::filesystem::remove(), which removes files from the filesystem. Correct it. Change-Id: I030966203c1682a11c723c596accdf34637dd1ba Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Fix buffer_copyEd Tanous3-10/+11
boost::asio::buffer_copy returns an integer of the number of values copied. Some static analysis tools mark that value as nodiscard, although it should never fail. Audit all uses of buffer_copy, and make sure that they're using the return value. In theory this should have no change on the behavior. Change-Id: I6af39b5347954c2932cf3d4e48e96ff9ae01583a Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Fix large copies with url_view and segments_viewEd Tanous1-0/+1
Despite these objects being called "view" they are still relatively large, as clang-tidy correctly flags, and we ignore. Change all function uses to capture by: const boost::urls::url_view_base& Which is the base class of all boost URL types, and any class (url, url_view, etc) is convertible to that base. Change-Id: I8ee2ea3f4cfba38331303a7e4eb520a2b6f8ba92 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Simplify routerEd Tanous2-2/+0
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-10Move run and redfish to compile unitsEd Tanous1-0/+3
Meson supports unity builds[1] natively. There's no reason to continue with the pseudo unity build we've been using by putting implementations in header files. This commit is the first in a long series of starting to break this up into smaller compile units, in the hopes of dropping incremental compile times for developers, and reduce the total per-core memory usage that gcc requires. This commit breaks out the run() function from main() and the constructor of RedfishService from redfish.hpp into their own compile units. According to tracing, even after broken out, these are still by far the two longest to compile units in the build. Tested: Code compiles. Debug build on a 24 core build server results in a decrease in compile time for compiling just bmcweb from 1m38s to 1m22s. [1] https://mesonbuild.com/Unity-builds.html Change-Id: Ibf352e8aba61d64c9a41a7a76e94ab3b5a0dde4b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-07Fix moves/forwardEd Tanous4-6/+5
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-03Call dump() lessEd Tanous1-4/+1
nlohmann::json::dump() is not an easy function to get the call parameters correct on. We should limit the places we use it. Luckily, both logging and redfish::messages support printing json values directly. Use them where appropriate. Tested: Error logging and out of range calls only of heavily used messages and logging calls. Inspection only. Change-Id: I57521d8791dd95250c93e8e3b2d4a959740ac713 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-01Fix redundant inline operatorsEd Tanous1-1/+1
inline is not required on member methods. Clang-tidy has a check for this. Enable the check and fix the two bad usages. Tested: Code compiles. Change-Id: I3115b7c0c4005e1082e0005b818fbe6569511f49 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-01Use no-switch-default on clangEd Tanous1-0/+2
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-29Remove old uses of cout/cerrEd Tanous2-19/+21
Most of this code was written before bmcweb had a logger, and therefore used cout/cerr. This commit greps the codebase and finds all places where we still use cout/cerr, and moves them to logging. Tested: Inspection only. No functional changes. Change-Id: I5ce1883c9941e80203ec29decb3a0206fd118506 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-25Remove unused variableEd Tanous1-1/+0
These variables aren't used, and clang-tidy-18 flags it. Remove Tested: Code compiles. Change-Id: I414c4614a5f789aecab7700a4ec805e98c09cade Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-25Enable bugprone clang checkEd Tanous1-1/+4
bugprone-multi-level-implicit-pointer-conversion is something that we pass currently, with one exception in the deprecated rest API. Ignore the one exception, as it's not clear how to fix it, and enable the check. Tested: Clang tidy passes. Change-Id: Idc10e0bb7b876e1c70afa28f9c27cc7bef1db0d7 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>
2024-03-22Fix content-security-policy disableEd Tanous1-1/+1
If one sets the XSS policy disable, and tries to load the webui, they're met with the following error message: ``` chunk-vendors.6cfb4b74.js:36 Refused to load the image 'data:image/svg+xml;charset=utf-8,%3Csvg xmlns='http://www.w3.org/2000/svg' width='4' height='5'%3E%3Cpath fill='%233f3f3f' d='M2 0L0 2h4zm0 5L0 3h4z'/%3E%3C/svg%3E' because it violates the following Content Security Policy directive: "img-src *". Note that '*' matches only URLs with network schemes ('http', 'https', 'ws', 'wss'), or URLs whose scheme matches `self`'s scheme. The scheme 'data:' must be added explicitly. ``` Do as it asks, and add data: to the content security policy. Tested: Browser console no longer shows error when XSS is enabled. Change-Id: I17f70d7c87a284b33ef6eb5a01a01c23a14898c9 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-22Revert "Refactor after login"Ed Tanous1-23/+28
This reverts commit cd40b060ee2df5469077a70d15590f86158f2c60. Cookie based login is no longer functional with this patch. It looks like we got a merge conflict that I resolved incorrectly. Tested: Webui can now log in. Change-Id: I60b8aeae173b1838d8745a2c499fbcb410813ef3
2024-03-21Clean up management console rest to use readJsonEd Tanous1-9/+9
Change-Id: Idc37e3e98296cf59aa6fab499a27d7ed899b71dd Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-16Refactor after loginEd Tanous1-28/+23
Break out this method into a smaller section. Tested: Redfish service validator passes Change-Id: I0ca4e9ea14c505a1ed00dae4cba1285e4ac1f36d Signed-off-by: Ed Tanous <edtanous@google.com>
2024-02-28Fix coredump on async method during validatePrivilegeMyung Bae1-1/+2
PATCH may cause bmcweb to coredump depending on timing of `validatePrivilege` execution. It is because `req' is captured as reference, and it may be cleared-up before async-call method completes. (This problem can be seen more frequently by enabling debug mode). This commit is to keep `req` during to async-method execution. Tested: - Create a ReadOnly user - here, called as `readonly` - Using `redfishtool`, run PATCH on `readonly` user role. ``` $ redfishtool -vvvvv raw -r ${bmc}:18080 -u ${user} -p ${password} -S Always PATCH /redfish/v1/AccountService/Accounts/readonly --data='{"RoleId":"Administrator"}' ... This sometimes fails because bmcweb coredump ``` After: ``` $ redfishtool raw -r ${bmc}:18080 -u ${user} -p ${password} -S Always PATCH /redfish/v1/AccountService/Accounts/readonly --data='{"RoleId":"Administrator"}' { "@odata.id": "/redfish/v1/AccountService/Accounts/readonly", "@odata.type": "#ManagerAccount.v1_7_0.ManagerAccount", ... } ``` Change-Id: I2a28d1743cfc0fbd9239f69dec5584b34c7ebe43 Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2024-01-19Remove some boost includesEd Tanous6-26/+46
The less we rely on boost, and more on std algorithms, the less people have to look up, and the more likely that our code will deduplicate. Replace all uses of boost::algorithms with std alternatives. Tested: Redfish Service Validator passes. Change-Id: I8a26f39b5709adc444b4178e92f5f3c7b988b05b Signed-off-by: Ed Tanous <edtanous@google.com>
2024-01-16DBus REST: Fix array and dict handlingMikhail Zhvakin1-3/+3
The bmcweb DBus REST API cannot currently handle array or dictionary data types correctly. This commit is meant to fix that. Tested: get/set DBus attributes (consisting of array and/or dictionary) via bmcweb DBus REST API. Change-Id: I9694cb888375c90d7a8fb1a10e53bdb5c0bce3bb Signed-off-by: Mikhail Zhvakin <striker_1993@mail.ru>
2024-01-09Fix spelling mistakesEd Tanous5-11/+11
These were found with: codespell -w $(git ls-files | grep "\.[hc]\(pp\)\?$") At some point in the future, we might want to get this enabled in CI. Change-Id: Iccb57b2adfd06a2e177e99db2923fe4e8e329118 Signed-off-by: Ed Tanous <ed@tanous.net>
2023-12-05Unit test ConnectionEd Tanous1-3/+0
Boost asio provides a test stream object that we can use to begin unit testing the connection object. This patchset uses it to re-enable some simple http1.1 tests. There's some features that have snuck into the connection class that aren't compatible with a stream (like ip address getting), so unfortunately we do need the connection class to be aware if it's in test mode, but that tradeoff seems worthwhile. Tested: Unit test pass. Change-Id: Id8b1f8866582b58502dbafe6139f841bf64b8ef3 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-10-31Move to file_body in boostEd Tanous3-11/+4
As is, it reads the whole file into memory before sending it. While fairly fast for the user, this wastes ram, and makes bmcweb less useful on less capable systems. This patch enables using the boost::beast::http::file_body type, which has more efficient serialization semantics than using a std::string. To do this, it adds a openFile() handler to http::Response, which can be used to properly open a file. Once the file is opened, the existing string body is ignored, and the file payload is sent instead. openFile() also returns success or failure, to allow users to properly handle 404s and other errors. To prove that it works, I moved over every instance of direct use of the body() method over to using this, including the webasset handler. The webasset handler specifically should help with system load when doing an initial page load of the webui. Tested: Redfish service validator passes. Change-Id: Ic7ea9ffefdbc81eb985de7edc0fac114822994ad Signed-off-by: Ed Tanous <ed@tanous.net>
2023-10-27Refactor populateUserInfoJonathan Doman1-47/+21
- No need to set error code in asyncResp since caller already does that. Then we can remove the asyncResp param altogether. - Check if session is valid before unpacking properties to avoid unnecessary work. - Use std::optional instead of pointers for slighter cleaner code. - Enforce required properties for local users based on D-Bus interface documentation (UserGroups must be provided for local users). Change-Id: I770d3556a0d62182b6abd72bfa3f8d62e2a105d1 Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
2023-10-24multipart-parser: eliminate temporary to emplace_backPatrick Williams1-2/+2
Fix the following clang-tidy warning: ``` ../include/multipart_parser.hpp:108:50: error: unnecessary temporary object created while calling emplace_back [modernize-use-emplace,-warnings-as-errors] 108 | mime_fields.emplace_back(FormPart{}); | ^~~~~~~~~~ ``` Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I362b4ad7f90f80a7746b79d643e3a7c5ff1db78c
2023-10-24clang-format: copy latest and re-formatPatrick Williams15-298/+295
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-10-20multipart-parser: use emplace_backPatrick Williams1-2/+2
clang-17 will have a stronger 'modernize-use-emplace' check and fails with the following warning: ``` ../include/multipart_parser.hpp:308:33: error: use emplace_back instead of push_back [modernize-use-emplace,-warnings-as-errors] 308 | mime_fields.push_back({}); | ^~~~~~~~~~~~ | emplace_back( ``` The vector::emplace_back needed an extra hint, as it would not directly coerce an initializer-list into the vector's value_type, so we need to use the value_type constructor. Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I74417e0ff5a6e0991bfbe4936b4814f6ee4c1269
2023-10-16Remove extra variables in websocketsEd Tanous1-2/+2
These variables don't need propagated to handlers. Any usage of them is incorrect. This makes Websocket once again a pure virtual class, which is desired. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id1ecc3911fc502d436a3e6aa29024628fc51aff4
2023-10-05Update to boost 1.83.0Ed Tanous1-1/+1
In boost 1.83.0, the boost::url maintainers deprecated the header only usage of the library without warning. A discussion with the maintainers[1] made it clear that they removed the abiliy on purpose, and they're not going to add it back or add a deprecation strategy (they did say they would update the documentation to actually match the intent), and that from here on in we should be using the cmake boost project to pull in the non-header-only boost libraries we use (which at this point is ONLY boost url). This commit updates to remove the usage of boost::urls::result typedef, which was deprecated in this release (which causes a compile error) and moves it to boost::system::result. In addition, it updates our meson files to pull in the boost project as a cmake dependency. [1] https://cpplang.slack.com/archives/C01JR6C9C4U/p1696441238739129 Tested: Not yet. Change-Id: Ia7adfc0348588915440687c3ab83a1de3e6b845a Signed-off-by: Ed Tanous <edtanous@google.com>
2023-09-25Clean up vm CredentialPipeEd Tanous1-0/+52
This code is needlessly complicated for what it does. Even with the intent, which is secure buffer cleanup, it's trivial to encase all this into a single class that accepts the strings by rvalue reference, then cleans them up afterward. Doing this also cleans up a potential lifetime problem, where if the unix socket returned immediately, it would've invalidated the buffers that were being sent. It also moves to async_write, instead of async_write_some. The former could in theory fail if the socket blocks (unlikely in this scenario) but it's good to handle anyway. Tested: Need some help here. There's no backend for this, so we might just have to rely on inspection. Change-Id: I9032d458f8eb7a0689bee575aae611641bacee26 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-09-08Fixes bmcweb crashed when mounting virtual mediaTroy Lee1-1/+1
After upgrading jsnbd to meson.build, the nbd-proxy path will change from /usr/sbin to /usr/bin. - https://gerrit.openbmc.org/c/openbmc/jsnbd/+/65434 - https://gerrit.openbmc.org/c/openbmc/openbmc/+/65466 Error message: ``` bmcweb[220]: [DEBUG http_connection.hpp:561] 0x235bad0 Clearing response bmcweb[220]: [DEBUG http_response.hpp:181] 0x235dd90 Clearing response containers bmcweb[220]: [DEBUG http_connection.hpp:403] 0x235bad0 doReadHeaders bmcweb[220]: [DEBUG http_connection.hpp:71] 0x22c20c8 Connection open, total 8 bmcweb[220]: [DEBUG http_connection.hpp:625] 0x23dc940 timer started [FAILED] Failed to start Wait for Network to be Configured. See 'systemctl status systemd-networkd-wait-online.service' for details. [ OK ] Reached target Network is Online. Starting System Logging Service... [ OK ] Started System Logging Service. systemd-journald[160]: Received SIGTERM from PID 220 (bmcweb). systemd[1]: avahi-daemon.service: Deactivated successfully. systemd[1]: bmcweb.service: Main process exited, code=exited, status=255/EXCEPTION systemd[1]: bmcweb.service: Failed with result 'exit-code'. systemd[1]: bmcweb.service: Consumed 1.940s CPU time. systemd[1]: obmc-dump-monitor.service: Deactivated successfully. systemd[1]: phosphor-certificate-manager@authority.service: Deactivated successfully. ``` After this fix: ``` bmcweb[219]: [DEBUG http_connection.hpp:268] Setting completion handler bmcweb[219]: [DEBUG http_response.hpp:238] 0xfb23e0 setting completion handler bmcweb[219]: [DEBUG http_response.hpp:238] 0xfb23e0 setting completion handler bmcweb[219]: [DEBUG routing.hpp:601] Matched rule (upgrade) '/vm/0/0' 1 / 2 bmcweb[219]: [DEBUG dbus_privileges.hpp:51] userName = root userRole = priv-admin bmcweb[219]: [DEBUG websocketrule.hpp:50] Websocket handles upgrade bmcweb[219]: [DEBUG websocket.hpp:78] Creating new connection 0xe641ec bmcweb[219]: [DEBUG websocket.hpp:89] starting connection 0xe641ec bmcweb[219]: [DEBUG http_response.hpp:223] 0xfb23e0 calling completion handler bmcweb[219]: [DEBUG http_response.hpp:226] 0xfb23e0 completion handler was valid bmcweb[219]: [DEBUG http_response.hpp:238] 0x108b008 setting completion handler bmcweb[219]: [DEBUG http_connection.hpp:81] 0x1088d48 Connection closed, total 8 bmcweb[219]: [DEBUG websocket.hpp:226] Websocket accepted connection bmcweb[219]: [DEBUG vm_websocket.hpp:172] Connection 0xe641ec opened bmcweb[219]: [DEBUG vm_websocket.hpp:85] inputBuffer empty. Bailing out bmcweb[219]: [DEBUG vm_websocket.hpp:94] Wrote 18bytes bmcweb[219]: [DEBUG vm_websocket.hpp:85] inputBuffer empty. Bailing out bmcweb[219]: [DEBUG vm_websocket.hpp:125] Read done. Read 26 bytes ``` Change-Id: Ic5dc3d0c32517add158d5354b712c166bc6bf204 Signed-off-by: Troy Lee <troy_lee@aspeedtech.com> Signed-off-by: Vince Chang <vince.chang@vertiv.com>
2023-08-25Remove phosphor-rest workaroundsEd Tanous1-29/+9
Phosphor-rest is no longer supported by the project, and phosphor-webui, which required some of these workarounds has been archived a year ago. There's no reason to keep this login type, given that it was undocumented. NOTE: Upon inspection, it looks like webui-vue used the same hack. [1] https://github.com/openbmc/webui-vue/blob/43e3bd26133b06ed117a3a3f10b2bc09e2c2aafc/src/store/modules/Authentication/AuthenticanStore.js#L41 Tested: Combined with https://gerrit.openbmc.org/c/openbmc/webui-vue/+/65811 Webui Login succceeds. Change-Id: Ie42380029e799e44b3a7404d4ec6d285b371402b Signed-off-by: Ed Tanous <edtanous@google.com>
2023-08-24kvm_websocket: Fix crash on dangling referenceXinnan Xie1-7/+19
Kvm_websocket captures the this pointer in the handler lambda of the socket. When the callback is called, if the object has been destructed, there will be a crash problem. This is fixed by using weak_from_this in the callback, if the object was destructed, the callback just returns without doing anything. Tested: 1. Open two kvm sessions in WebUI, and keep refreshing in one of the pages, there is a small chance of coredump happening. Debug infomation shows: ``` bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470 bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470 bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470 bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470 bmcweb[5331]: DEBUG: doRead. conn:0x28d19a0. this: 0x284d470 bmcweb[5331]: DEBUG: onclose. conn:0x28d19a0 bmcweb[5331]: DEBUG: doRead. conn:0x2876648. this: 0x284d470 systemd[1]: bmeweb.service: Main process exited, code=dumped, status=11/SEGV systemd[1]: bmcweb.service: Failed with result 'core-dump systemd[1]: Started Start bmweb server. ``` 2. After this fix no coredump occurred. Change-Id: I7bba9b67c470def90ddb1e471a0ac95edd6165e5 Signed-off-by: Xinnan Xie <xiexinnan@bytedance.com>
2023-08-24Fix typo `DBusInteracesMap` -> `DBusInterfacesMap`Michael Shen2-3/+4
Change-Id: I9a851076eccee9d79ad7bb036e58b717e06ad5d1 Signed-off-by: Michael Shen <gpgpgp@google.com>
2023-08-23Move http client to URLEd Tanous2-4/+12
Type safety is a good thing. In: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/65606 It was found that splitting out the URI into encoded pieces in the early phase removed some information we needed, namely whether or not a URI was ipv6. This commit changes http client such that it passes all the information through, with the correct type, rather than passing in hostname, port, path, and ssl separately. Opportunistically, because a number of log lines are changing, this uses the opportunity to remove a number of calls to std::to_string, and rely on std::format instead. Now that we no longer use custom URI splitting code, the ValidateAndSplitUrl() method can be removed, given that our validation now happens in the URI class. Tested: Aggregation works properly when satellite URIs are queried. Change-Id: I9f605863179af54c5af2719bc5ce9d29cbfffab7 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-08-21Use rangesEd Tanous4-11/+14
C++20 brought us std::ranges for a lot of algorithms. Most of these conversions were done using comby, similar to: ``` comby -verbose 'std::lower_bound(:[a].begin(),:[b].end(),:[c])' 'std::ranges::lower_bound(:[a], :[c])' $(git ls-files | grep "\.[hc]\(pp\)\?$") -in-place ``` Change-Id: I0c99c04e9368312555c08147d474ca93a5959e8d Signed-off-by: Ed Tanous <edtanous@google.com>
2023-08-21Fix duplicate entries in session userGroupsJonathan Doman1-5/+5
The user's groups were getting re-appended in the session object on every request, resulting in a small memory leak (that does clear away when the session is ended). Fix by just overwriting instead of appending. Tested: Added debug prints in ~UserSession to check contents of userGroups, then ran multiple GETs to /redfish/v1 via token auth, then destroyed session via WebUI and confirmed userGroups contained correct set of groups. Change-Id: I7c04a18437f69a97f138ff1f9aeee2731952ae8b Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
2023-08-17Change unauthorized message if no UI is installedEd Tanous1-1/+2
This "Unauthorized" message has been a constant source of confusion for users that forget to install a UI. This commit updates the message to be more clear, and present users with some hints that they have forgotten to install a webui if they expected the auth to succeed. Tested: String change only. Code compiles. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ic68e4622082caf5e32e496ec56f0c8b409b91990
2023-08-16Add missing comma in Permissions-PolicyJoseph Reynolds1-1/+1
This adds a missing comma in the Permissions-Policy response header value. Tested: no; I didn't even try to compile it. Change-Id: I4f08b54a5e5af040e10a95d913ef8b457f5bd457 Signed-off-by: Joseph Reynolds <joseph-reynolds@charter.net>
2023-08-07Fix bugprone-unchecked-optional-access findingsEd Tanous1-2/+3
Clang-tidy has the aforementioned check, which shows a few places in the core where we ignored the required optional checks. Fix all uses. Note, we cannot enable the check that this time because of some weird code in health.hpp that crashes tidy[1]. That will need to be a future improvement. There are tests that call something like ASSERT(optional) EXPECT(optional->foo()) While this isn't an actual violation, clang-tidy doesn't seem to be smart enough to deal with it, so add some explicit checks. [1] https://github.com/llvm/llvm-project/issues/55530 Tested: Redfish service validator passes. Change-Id: Ied579cd0b957efc81aff5d5d1091a740a7a2d7e3 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-08-01Revert "Cache user role in session object"Gunnar Mills3-305/+115
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-31Refactor login/logoutEd Tanous1-177/+171
Similar to what we've done elsewhere, move login and logout into their own methods. This reduces the amount of scopes that need to be read at any given time. Tested: At last commit in series. Change-Id: Ia2aa8b3fcbed18d7a481876fe4ffd55f31120064 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-07-29sdbusplus: use shorter type aliasesPatrick Williams1-3/+3
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: I1621db436cb5e81ca597f5b9dac76452c6e7fd74 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
2023-07-28Cache user role in session objectEd Tanous3-115/+305
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-28Add missing nullptr checksEd Tanous1-0/+9
These were found by inspection, and should be cases that aren't possible, but we should be consistent. Check the pointers for null before dereferencing. Tested: Inspection only. Condition theoretically not possible to hit. Change-Id: I1423bb5bae5445d2b4b0cee2f3315b3ddd1c3836 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-07-20Replace logging with std::formatEd Tanous22-581/+567
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-14AsyncResolve cleanups and error handlingEd Tanous1-30/+33
The Async DBus resolver really has nothing to do with crow, which is our core http library namespace and has some opportunistic cleanups that can be done. This commit moves it into the bmcweb namespace (unimportantly) and breaks out one of the larger functions such that it can be unit tested, and unit tests it. Tested: Unit tests pass. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie3cfbb0ef81a027a1ad42358c04967a517471117
2023-07-12Use openssl random number generatorEd Tanous5-13/+9
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-07-12Fix clang-tidy error for ibm/locksRavi Teja1-23/+13
ibm/locks that has had clang-tidy warnings disabled for a while due to multiple safety and endianness issues. https://github.com/openbmc/bmcweb/commit/26b3630b181d1093483e4d5ebe6aeeb91a6a2976 this commit uncomments locks code which causes warnings and fixes these warnings Tested by: create & list locks with redfish curl and management console Change-Id: I569e0dc5c11f259b7cca4b22722c9b4d87c68c80 Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
2023-07-06Remove IBM console events from RedfishEd Tanous1-7/+0
The /ibm/v1 console is a different tree than Redfish, and as such, should not be sending non-redfish resource events out. This is very likely to break redfish clients on the other end. If the management console wants an event-like entity, it needs to come up with its own EventService-like resource, considering it is a separate tree. Significant related discussion occurred here: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/36368 Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Ic2a9e572099490f8810e03ab08336518f5672690