summaryrefslogtreecommitdiff
path: root/include
AgeCommit message (Collapse)AuthorFilesLines
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
2023-06-29Update to owasp headersEd Tanous2-49/+43
Change the Cache-Control header to what owasp recommends. Remove the X-XSS-Protection. This has been removed from Chrome, and is unimplemented in other browsers[1]. Add: X-Permitted-Cross-Domain-Policies Clear-Site-Data Cross-Origin-Embedder-Policy Cross-Origin-Opener-Policy Cross-Origin-Resource-Policy And set them to the OWASP recommended values. Tested: The OWASP Venom test suite now passes more tests. [1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-XSS-Protection Change-Id: I2860041c1037f47bb85a6444cec66960d0aa55f9 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-28HTTP/2 supportEd Tanous1-0/+42
HTTP/2 gives a number of optimizations, while keeping support for the protocol. HTTP/2 support was recently added to the Redfish specification. The largest performance increase in bmc usage is likely header compression. Almost all requests reuse the same header values, so the hpack based compression scheme in HTTP/2 allows OpenBMC to be more efficient as a transport, and has the potential to significantly reduce the number of bytes we're sending on the wire. This commit adds HTTP2 support to bmcweb through nghttp2 library. When static linked into bmcweb, this support adds 53.4KB to the bmcweb binary size. nghttp2 is available in meta-oe already. Given the experimental nature of this option, it is added under the meson option "experimental-http2" and disabled by default. The hope is to enable it at some point in the future. To accomplish the above, there a new class, HTTP2Connection is created. This is intended to isolate HTTP/2 connections code from HttpConnection such that it is far less likely to cause bugs, although it does duplicate about 20 lines of code (async_read_some, async_write_some, buffers, etc). This seems worth it for the moment. In a similar way to Websockets, when an HTTP/2 connection is detected through ALPN, the HTTP2Connection class will be instantiated, and the socket object passed to it, thus allowing the Connection class to be destroyed, and the HTTP2Connection to take over for the user. Tested: Redfish service validator passes with option enabled With option disabled GET /redfish/v1 in curl shows ALPN non negotiation, and fallback to http1.1 With the option enable GET /redfish/v1 in curl shows ALPN negotiates to HTTP2 Change-Id: I7839e457e0ba918b0695e04babddd0925ed3383c Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-28Rename all error_code instances to ecEd Tanous1-2/+2
We're not consistent here, which leads to people copying and pasting code all over, which has lead to a bunch of different names for error codes. This commit changes to coerce them all to "ec", because that's what boost uses for a naming convention. Tested: Rename only, code compiles. Change-Id: I7053cc738faa9f7a82f55fc46fc78618bdf702a5 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-23error_messages: remove source_location indirectPatrick Williams1-27/+0
clang-16 now supports std::source_location so remove the indirection that uses experimental::source_location in some cases. Tested: Compiled with `CXX=clang++`. Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: Id55bdf01855206c6892684e1e56cc5ff56e2b5a2
2023-06-20Add headers Referrer-Policy and Permissions-PolicyJoseph Reynolds1-0/+44
This adds HTTP response headers Referrer-Policy and Permissions-Policy per OWASP guidelines, with some appropriate values for BMCWeb. https://owasp.org/www-project-secure-headers/ Policies are given for all standardized feature. Most features are disabled except for the following which the web application uses: usb=(self). Tested: Yes Via curl, confirmed headers are present. On selected browsers, opened browser tools and confirmed browsers didn't complain about the new headers. Browsers checked were: - Firefox 111.0.1 (64-bit) - Safari Version 16.4 (18615.1.26.11.23) Did not test access to features secured by the Permissions-Policy. Did not test if the web application features still work. Change-Id: I65f89d2959b0b1338c20d7222229fbdc1d720834 Signed-off-by: Joseph Reynolds <joseph-reynolds@charter.net>
2023-06-20Change cipher suite variableEd Tanous1-10/+12
This variable was poorly named. At one point it represented mozilla modern cipher suites, but it has been long since changed to mozilla intermediate. Name the variable appropriately. While we're here, also change the type to const char*, such that we're not allocating the string for every connection. Change-Id: I0faae73448d953c173c3d3b9e4916b41b2a2497a Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-20Upgrade to mozilla intermediate 5.7Ed Tanous1-7/+6
Mozilla intermediate 5.7 was released last month[1] The last release to these was 3 years ago, so we haven't really had to update much. Update cipher suites to match new list for mozilla intermediate. Note, the variable is called "mozilla modern" but it hasn't tracked the modern recommendations for some time. Tested: testssl.sh, from the master branch (864877df) Returns a passing result, showing no change in supported products, and the cipher suites correctly applied. Redfish service validator shows no change in result. [1] https://ssl-config.mozilla.org/guidelines/5.7.json [2] https://github.com/mozilla/ssl-config-generator/tree/master/src/static/guidelines Tested: WIP Change-Id: Ie9ccb7757ae527fa3ac129f781ae32657c7dfdd9 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-20Refactor getManagedObjects methodGeorge Liu2-10/+8
Since the getManagedObjects method has been implemented in dbus_utility and this commit is to integrate all the places where the GetManagedObjects method is obtained, and use the method in dbus_utility uniformly. Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: Ic13f2bef7b30f805cd3444a75d7df17b031f2eb0
2023-06-17Set cookieAuth variable for cookieEd Tanous1-0/+1
Change-Id: Ib5fb6dcfaf63520cbc07ca909e0806480440296a Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-17Revert "Fix websocket csrf checking"Ed Tanous1-1/+2
This reverts commit e628df8658c57f6943b6d3612e1077618e5a168a. This appears to cause problems with non-cookie login of the console websocket. This appears to be a gap in both our testing, and things that we have scripting to do, but clearly it's a change in behavior, so if we want to change the behavior, we should do it intentionally, and clearly, ideally with a path to make clients work, or an explicit documentation that the webui is the only supported client. Change-Id: I334257e1355a5b8431cb7ecfe58ef8a942f4981c Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-14Add missing pragma onceEd Tanous1-0/+2
This header didn't include a pragma once. Fix it. Tested: Code compiles Change-Id: I8bd4f9d870ec9b7dc1687e8de1c8a61d93140c7e Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-09Fix hack on Set-CookieEd Tanous2-19/+12
This is one that I couldn't figure out for a while. Turns out that fields has both a set() and an insert() method. Whereas set() replaces, insert() appends, which is what we want in this case. This allows us to call the actual methods several times, instead of essentially string injecting our own code, which should make it clearer. At the same time, there was one unit test that was structured such that it was using addHeader to clear a header, so this commit adds an explicit "clearHeader()" method, so we can be explicit. Tested: Logging into the webui in chrome (which uses POST /login) shows: 401 with no cookie header if the incorrect password is used 200 with 2 Set-Cookie headers set: Set-Cookie: SESSION=<session tag>; SameSite=Strict; Secure; HttpOnly Set-Cookie: XSRF-TOKEN=<token tag>; SameSite=Strict; Secure Change-Id: I9b87a48ea6ba892fc08e66940563dea86edb9a65 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-09Break up router into separate filesEd Tanous1-0/+180
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-08Remove urlEncodeEd Tanous2-25/+6
All new uses should be using boost::urls::url now. This was the last usage. Tested: Logged into webui, and observed the correct URL behavior. In browser window /foobar Forwarded to /?next=/foobar#/login Which is correct. Note, this is different behavior slightly than before. It was found that the URI precedence goes query string THEN fragment, rather than the other way around that we had it. This was flagged when moving over to boost url structures. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ifb354537d71a43c531d7d380dd889cf646731e39
2023-06-08Remove unused const-castEd Tanous1-2/+1
std::string::data now has a non-const variation in c++20. This allows us to remove a NOLINT and follow the standard. Tested: Login succeeds. Change-Id: Ie49932fae8efa90afe1a238f7059924747300521 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-06Remove extra characterEd Tanous1-1/+1
This extra quote snuch into the comment somehow. Fix it. Change-Id: I5aa14e1f43b1de9cabda006f7f9727d611c5aea3 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-06Add support for multiple consolesNinad Palsule1-19/+81
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>
generated by cgit v1.2.3 (git 2.39.0) at 2024-06-14 15:42:15 +0300