summaryrefslogtreecommitdiff
path: root/http/http_connection.hpp
AgeCommit message (Collapse)AuthorFilesLines
2023-08-20mTLS: Fix handshake failuresMichal Orzel1-3/+5
Change introduced in [1] has exposed significant problem in mTLS verification process, during which an attempt to an uninitialized object was made. This change removes that attempt and replaces it with resource that is available at this specific moment of connection lifetime. Tested: 1. Created and uploaded a set of certificates by following instructions from TLS Configuration guide [2]. 2. Attempted to access /redfish/v1/SessionService/Sessions endpoint using mTLS authentication method. With this fix connection has been successful. [1] https://github.com/openbmc/bmcweb/commit/e01d0c36af115ed46d54b5dbbacfe3ad92226bd3 [2] https://github.com/openbmc/docs/blob/master/security/TLS-configuration.md Change-Id: I434dbf27169d7ea0207dfd139868d5bf398d24b0 Signed-off-by: Michal Orzel <michalx.orzel@intel.com>
2023-08-07Fix bugprone-unchecked-optional-access findingsEd Tanous1-2/+21
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-07-20Replace logging with std::formatEd Tanous1-53/+54
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging. Unfortunately, given its level of compile time checks, it needs to be a method, instead of the stream style logging we had before. This requires a pretty substantial change. Fortunately, this change can be largely automated, via the script included in this commit under scripts/replace_logs.py. This is to aid people in moving their patchsets over to the new form in the short period where old patches will be based on the old logging. The intention is that this script eventually goes away. The old style logging (stream based) looked like. BMCWEB_LOG_DEBUG << "Foo " << foo; The new equivalent of the above would be: BMCWEB_LOG_DEBUG("Foo {}", foo); In the course of doing this, this also cleans up several ignored linter errors, including macro usage, and array to pointer deconstruction. Note, This patchset does remove the timestamp from the log message. In practice, this was duplicated between journald and bmcweb, and there's no need for both to exist. One design decision of note is the addition of logPtr. Because the compiler can't disambiguate between const char* and const MyThing*, it's necessary to add an explicit cast to void*. This is identical to how fmt handled it. Tested: compiled with logging meson_option enabled, and launched bmcweb Saw the usual logging, similar to what was present before: ``` [Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled [Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800 [Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist [Info src/webserver_main.cpp:59] Starting webserver on port 18080 [Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file. [Info src/webserver_main.cpp:137] Start Hostname Monitor Service... ``` Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8
2023-06-28HTTP/2 supportEd Tanous1-1/+30
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-24Break out request completion from connectionEd Tanous1-55/+2
There's a large part of the http::Connection class that has nothing to do with the connection at all, and is all about parsing, and finalizing the response. Break that portion out into its own method that can (in the future) be unit tested. Tested: Redfish service validator passes Change-Id: Ic608d432e69e25c0e0a1555ecc24ed62adba2664 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-06-08Remove urlEncodeEd Tanous1-1/+1
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-05Break out serializer into its own cpp fileEd Tanous1-9/+1
This commit is entirely just moving code, such that not all compile units need to pull in the full html serializer. Tested: Unit tests pass. Pretty good coverage. Redfish service validator passes. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ifaebe9534c0693dc678fd994517563b89aca0cc5
2023-06-01Server-sent-event fixesEd Tanous1-2/+4
This makes several changes to server-sent events to allow it to merge to master. The routing system has been removed in leiu of using content-type eventstream detection. Timers have been added to the sse connections, and sse connections now rely on async_wait, rather than a full read. Tested: WIP Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id0ff0ebc2b3a795b3dba008e440556a9fdd882c2
2023-06-01Add Server-Sent-Event supportV-Sanjana1-4/+5
Server-Sent-Event is a standard describing how servers can initiate data transmission towards clients once an initial client connection has been established. Unlike websockets (which are bidirectional), Server-Sent-Events(SSE) are unidirectional and commonly used to send message updates or continuous data streams to a browser client. This is base patch for adding Server-Sent-Events routing support to bmcweb. Redfish EventService SSE style subscription uses SSE route for sending the Events/MetricReports to client which establishes the connection. Tested this patch with along with EventService SSE support patches and verified the functionalty on browser. Tested: - Tested using follow-up patches on top which adds support for Redfish EventService SSE style subscription and observed events are getting sent periodically. - Created SSE subscription from the browser by visiting https://<BMC IP>/redfish/v1/EventService/SSE Change-Id: I36956565cbba30c2007852c9471f477f6d1736e9 Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com> Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com> Signed-off-by: V-Sanjana <sanjana.v@intel.com>
2023-05-19Capture all boost::system::error_codes by refEd Tanous1-1/+1
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-19http_connection: Allow empty json objectsEd Tanous1-1/+1
Currently http_connection will produce empty body in the response if the res.jsonValue is empty, including empty array, object. This makes the output confusing in case a response does contain an empty object or array. Change the code to print the json object even if it's empty object or array. This patchset was previously reverted because of a regression, but this regression is fixed in 63529. Tested on previous commit: With an OEM URL that returns empty array depending on the system config, the response becomes `[]` instead of empty. Signed-off-by: Lei YU <yulei.sh@bytedance.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I1d1bf01a5277ff1bc953b15d9fc410e10f941e70
2023-05-19Clean up preparePayloadEd Tanous1-10/+0
boost::beast::http::message::prepare_payload [1] can throw, which isn't really the behavior we want (as it throws to the io_context). Luckily, every part of that function is using public methods, and we can simplify it. In past commits, we've worked around this issue: 6295becabb9edba2edb53a3c0dddc13d2ffac8dd This is an attempt to fix it properly. [1] https://github.com/boostorg/beast/blob/ae01f0201dbf940cbc32d96d7a78dc584a02ab26/include/boost/beast/http/impl/message.hpp#L398 Redfish service validator passes Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie88ddeecfd226bba75a7659cfb7ddddd38eb27cb
2023-05-12Revert "http_connection: Allow empty json objects"Ed Tanous1-1/+1
This reverts commit 02e01b5108d46720a0b438c0d79952464320d954. This commit is being reverted because it causes login failures on Firefox browsers. This commit originally was added with the idea that it did not fix anything on upstream, but made some peoples forks better. It appears to have broken some upstream things, so the right thing to do is to revert it until those breakages can be understood. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I04de84fca1a8de657f6941653f2a3e595ee725d5
2023-05-11clang-format: copy latest and re-formatPatrick Williams1-2/+2
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-25http_connection: Allow empty json objectsLei YU1-1/+1
Currently http_connection will produce empty body in the response if the res.jsonValue is empty, including empty array, object. This makes the output confusing in case a response does contain an empty object or array. Change the code to print the json object even if it's empty object or array, so that the output is consistent with the `res.jsonValue`. Tested: With an OEM URL that returns empty array depending on the system config, the response becomes `[]` instead of empty. Signed-off-by: Lei YU <yulei.sh@bytedance.com> Change-Id: Ie97378a2cffce7b1fd6586a56b6cfa7d5c476dc1
2023-03-15Add asyncResp support to handleUpgradeP Dheeraj Srujan Kumar1-4/+15
This commit enables passing down the asyncResp (of the connection) to the handler of upgraded connections. This is already in place for normal requests (i.e. Class Router -> handle()) This change would enable any async calls that would be required before upgrade of the connection. For example, as on today, we have only Authentication of user in place for upgraded connection, but not Authorization. So, this asyncResp could further be used for such dbus calls to return informative response. This commit updates the signature of all the handleUpgrade() functions present in router.hpp to take in asyncResp object instead of normal response. Tested : - websocket_test.py Passed - KVM was functional in WebUI. Change-Id: I1c6c91f126b734e1b5573d5ef204fe2bf6ed6c26 Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com>
2023-03-11Make url by value in RequestEd Tanous1-5/+7
There's some tough-to-track-down safety problems in http Request. This commit is an attempt to make things more safe, even if it isn't clear how the old code was wrong. Previously, the old code took a url_view from the target() string for a given URI. This was effectively a pointer, and needed to be updated in custom move/copy constructors that were error prone to write. This commit moves to taking the URI by non-view, which involves a copy, but allows us to use the default move and copy constructors, as well as have no internal references within Request, which should improve the safety and reviewability. There's already so many string copies in bmcweb, that this is unlikely to show up as any sort of performance regression, and simple code is much better in this case. Note, because of a bug in boost::url, we have to explicitly construct a url_view in any case where we want to use segments() or query() on a const Request. This has been reported to the boost maintainers, and is being worked for a long term solution. https://github.com/boostorg/url/pull/704 Tested: Redfish service validator passed on last commit in series. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I49a7710e642dff624d578ec1dde088428f284627
2023-03-03Allow logged in users to upload incrementallyEd Tanous1-15/+40
There are use cases where logged in users might want to upload a large file over a slow connection, and would exceed the 60 second timeout that bmcweb has. This commit would theoretically allow the user timer to be per-segment, allowing very long timeouts in the case of slow connections, so long as some progress was made within the 15 second window, which seems reasonable. If user authentication is disabled then there is no user session active in this case timer will be refreshed as long as progress was made. This seems like a better alternative compared to setting a very long (5-20 minute) timeout. Testing: - Loaded image on the system $ curl -k -H 'X-Auth-Token: <token>' -H 'Content-Type: application/octet-stream' -X POST -T ./obmc-phosphor-image-p10bmc.ext4.mmc.tar https://${bmc}:443/redfish/v1/UpdateService/update { "@odata.id": "/redfish/v1/TaskService/Tasks/0", "@odata.type": "#Task.v1_4_3.Task", "Id": "0", "TaskState": "Running", "TaskStatus": "OK" } - Tested image load using disable authentication and insecure http connections. - Ran few querries and those are fine. * curl -s -k https://${bmc}:443/redfish/v1/Managers * curl -s -k https://${bmc}:443/redfish/v1/Managers/bmc * curl -s -k https://${bmc}:443/redfish/v1/AccountService/Accounts * curl -s -k https://${bmc}:443/redfish/v1/Systems/system * curl -s -k https://${bmc}:443/redfish/v1/Chassis/chassis * curl -s -k https://${bmc}:443/redfish/v1/AccountService/LDAP/Certificates * curl -k -X POST https://${bmc}:443/redfish/v1/AccountService/Accounts -d '{"UserName": "user99", "Password": "pass123", "RoleId": "Administrator"}' * curl -k https://${bmc}:443/redfish/v1/AccountService/Accounts/user99 * curl -k -X DELETE https://${bmc}:443/redfish/v1/AccountService/Accounts/user99 * curl -k -H 'Content-Type: application/json' -X POST https://${bmc}:443/login -d '{"username" : "admin", "password" : "newpas1"}' * curl -k -H 'X-Auth-Token: ' -X PATCH https://${bmc}:443/redfish/v1/AccountService/Accounts/admin -d '{"Password":"newpas2"}' * curl -k -H 'X-Auth-Token: ' -X POST https://${bmc}:443/logout Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I579c86defdd199c140891a986d70ae2eca63b2aa Signed-off-by: Ninad Palsule <ninadpalsule@us.ibm.com>
2023-02-16Fix some more includesEd Tanous1-0/+1
clang-tidy warns on these when run directly in a header file. Fix them. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib3366699c36e85644107690c23467f2ed22e398d
2023-02-11Fix overwriting mTLS sessionBoleslaw Ogonczyk Makowski1-15/+11
sessionIsFromTransport variable was always being overwritten to false [1], it caused userSession to get cleared [2] while it was also being used for mTLS session which prevented authentication in next requests and made cleanup code inaccessible [3]. Using the same variable for session from request and session created by mTLS broke single responsibility principle. Tested: Follow the guide in [4] to create a valid certificate for a user that can access some resource (for example /redfish/v1/Chassis) and create a file containing the address to it more than once in following format: url=https://BMC_IP/redfish/v1/Chassis url=https://BMC_IP/redfish/v1/Chassis curl --cert client-cert.pem --key client-key.pem -vvv --cacert CA-cert.pem -K addr_file -H "Connection: keep-alive" Before this change requests after first would fail with "401 Unauthorized" [1]: https://github.com/openbmc/bmcweb/blob/770b3ff239f96b419a791bed732f914899b8c202/http/http_connection.hpp#L468 [2]: https://github.com/openbmc/bmcweb/blob/770b3ff239f96b419a791bed732f914899b8c202/http/http_connection.hpp#L555 [3]: https://github.com/openbmc/bmcweb/blob/770b3ff239f96b419a791bed732f914899b8c202/http/http_connection.hpp#L283 [4]: https://github.com/openbmc/docs/blob/f4febd002df578bad816239b70950f84ea4567e8/security/TLS-configuration.md Change-Id: I4cf70ceb23c7a9b2668b2fcb44566f9971ac9ad4 Signed-off-by: Karol Niczyj <karol.niczyj@intel.com> Signed-off-by: Boleslaw Ogonczyk Makowski <boleslawx.ogonczyk-makowski@intel.com>
2023-01-25Timer not started if user session is logged inNinad Palsule1-1/+0
In startDeadline(), If user session is logged in then we simply return without starting the timer. This fix fixes that issue. Tested: Loaded code change on system and verified timeout now works. Change-Id: Ia4359b6dffb3015eb20a2a9d0ff2e5e6dab3500d Signed-off-by: Ninad Palsule <ninadpalsule@us.ibm.com>
2023-01-21Replace "Fix keepalive false" with patchset 3Ed Tanous1-3/+6
This reverts commit 5ae6f9254161f7229216c08b591e31eaf10f69e4. And replaces it with patchset 3 from the same review, for which was tested to work properly. This commit was merged erroneously. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I201924ad27d33923d43bdf82ecb016a0f214b4dd
2023-01-18Fix a boatload of #includesEd Tanous1-4/+4
Most of these missing includes were found by running clang-tidy on all files, including headers. The existing scripts just run clang-tidy on source files, which doesn't catch most of these. Tested: Code compiles Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ic741fbb2cc9e5e92955fd5a1b778a482830e80e8
2023-01-17Add check for globalsEd Tanous1-0/+1
We don't follow this cpp core guidelines rule well. This is something that we should aspire to cleaning up in the future, but for the moment, lets turn the rule on in clang-tidy to stop the bleeding, add ignores for the things that we know need some better abstractions, and work on these over time. Most of this commit is just adding NOLINTNEXTLINE exceptions for all of our globals. There was one case in the sensor code where clang correctly noted that those globals weren't actually const, which got missed because of the use of auto. Tested: CI should be good enough for this. Passes clang-tidy. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ieda08fee69a3b209d4b3e9771809a6c41524f066
2023-01-12Fix keepalive falseEd Tanous1-4/+4
When we changed [1] to using a std::move of the Request object instead of an unsafe reference, we missed one spot where we were using the request object, post handle. This commit moves the keepalive function to be set on the response object before calling handle. Tested: curl request with -H "Connection: close", -H "Connection: keep-alive" and no header all return the correct values. [1] 2d6cb56b6b47c3fbb0d234ade5c1208edb69ef1f Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I91fe32162407f1e1bdfcc06f1751e02d11f8a697
2023-01-12Clean up http end_of_stream tracesMyung Bae1-1/+2
http::end_of_stream log is not properly detected and thus it is being traced as error. It will be fixed to be traced as warning so they do not show up when just error traces are enabled (new default) Here's the log we was getting: Jan 10 13:34:32 ever6bmc bmcweb[2496]: (2023-01-10 13:34:32) \ [ERROR "http_connection.hpp":784] 0x14bd360 Error while \ reading: end of stream Change-Id: I6ecee813c4f7806a676ba0ad3e4ab1a8f78747fd Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2023-01-12Refactor mtls callbacks into their own fileEd Tanous1-161/+20
Mutual TLS is non-trivial enough that it definitely shouldn't be done in an inline lambda method. This commit moves the code. Tested: WIP. This is a pretty negligible code move; Minimal touch testing should be good. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I7a15a6bc66f4d8fb090411509549628f6d1045a5
2022-12-28Add CBOR supportEd Tanous1-2/+8
CBOR is a more efficient way to represent json, and something that, as you can see from this patch, is relatively trivial to implement in our current nlohmann json handlers. This allows users that specify an accepts header of "application/cbor" to request the BMC produce a cbor response. This feature adds 1520 bytes (1.48KB) to the binary size of bmcweb. For ServiceRoot GET /redfish/v1 Accepts: application/json - returns json GET /redfish/v1 Accepts: application/cbor - returns cbor GET /redfish/v1 Accepts: */* - returns json GET /redfish/v1 Accepts: text/html - returns html GET /redfish/v1 no-accepts header - returns json For service root, CBOR encoding drops the payload size from 1520 bytes on my system, to 1021 byes, which is a significant improvement in the number of bytes we need to compress. Redfish-service-validator passes. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I847e678cf79dfd7d55e6d3b26960c419e47063af
2022-12-21Return bad request if can't constructGunnar Mills1-0/+2
If given a bad URI, e.g. "https://$bmc/?a=id;" return http bad request (400) like we do other places, e.g. a few lines below. Certain scanners expect to see bad request when crawling and probing for vulnerabilities and using not valid URIs. Just dropping the connection causes errors with these scanners. They think connection problem or worse the server was taken down. Tested: Tested downstream https://$bmc/?a=id; returns bad request. Change-Id: I99a52d4c5e7f366046c5de1cf22c4db95ab39e13 Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2022-11-02Implement If-None-Match support for caching clientEd Tanous1-0/+9
This commit implements support for the If-None-Match header on http requests. This can be combined with the 89f180089bce9cc431d0b1053410f262f157b987 commit for producing ETag to allow a client to have a highly efficient cache, while still pulling data from the BMC. This behavior is documented several places, in W3C produced docs[1], as well as section 7.1 of the Redfish specification: ''' A service only returns the resource if the current ETag of that resource does not match the ETag sent in this header. If the ETag in this header matches the resource's current ETag, the GET operation returns the HTTP 304 status code. ''' Inside bmcweb, this behavior is accomplished in a relatively naive way, by creating the complete request, then doing a direct ETag comparison between the generated data and the request header. In the event the two match, 304 not-modified is returned, in-line with both the Redfish specification and the HTTP RFC. [1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match Tested (on previous rebase): First, request ServiceRoot curl --insecure -vvvv --user root:0penBmc https://192.168.7.2/redfish/v1 This returns a header similar to: < ETag: "ECE52663" Taking that ETag, and putting it into an If-None-Match header: ``` curl --insecure -vvvv -H "If-None-Match: \"ECE52663\"" \ --user root:0penBmc https://192.168.7.2/redfish/v1 ``` Returns: < HTTP/1.1 304 Not Modified ... < Content-Length: 0 Showing that the payload was not repeated, and the response size was much.... much smaller on the wire. Performance was not measured as part of this testing, but even if it has no performance impact (which is unlikely), this change is still worthwhile to implement more of the Redfish specification. Redfish-service-validator passes. Redfish-protocol-validator passes 1 more atom in comparison to previous. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I1e7d41738884593bf333e4b9b53d318838808008
2022-10-07Move ClientID parameter out of OEMEd Tanous1-2/+1
In 2022.2, Redfish added support for the Context parameter on the Session Resource. This parameter has the same function that the OemSession.ClientId field served. This commit moves all the existing ClientId code to produce Context as well. Functionally, this has one important difference, in that Context in Redfish is optionally provided by the user, which means we need to omit it if not given by the user. The old implementation left it set to empty string (""). Because of this, a few minor interfaces need to change to use std::optional. Existing uses of clientId are moved to using value_or("") to keep the same behavior as before. Tested: curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\": \"0penBmc\"}" https://192.168.7.2/redfish/v1/SessionService/Sessions Returns a Session object with no Context key present curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\": \"0penBmc\", \"Context\": \"Foobar\"}" https://192.168.7.2/redfish/v1/SessionService/Sessions Returns a Session object with: "Context": "Foobar" Subsequent Gets of /redfish/v1/SessionService/Sessions/<sid> return the same session objects, both with and without Context. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I4df358623f93f3e6cb659e99970ad909cefebc62
2022-09-13Make Accepts: */* default to JSONEd Tanous1-2/+2
There are apparently libraries that use an Accepts header of */*, instead of simply omitting the Accepts header, or providing a more correct value for that header. 99351cd856038475cac146029e5db03767a1459c Improve content type The above commit attempted to refine our handling of this header, and changed the behavior for */* to default to HTML. While this is arguably "correct" to the HTTP RFC, and the clients that do this are definitely wrong in their implementation, we can try to shield them a little from their incorrectness, and we can certainly avoid compatibility issues with these clients, without effecting the clients that implement the spec correctly. This commit changes the priority order of Accepts header to JSON then HTML, instead of the other way around. Tested: curl --insecure -H "Accept: */*" --user root:0penBmc \ https://192.168.7.2/redfish/v1 Now returns a json payload. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I7850a3afb0b5d635f8d632fb0a9f790e53fe4466
2022-09-13Improve content typeEd Tanous1-1/+10
We have a number of specialized content-type functions for varying levels of degree, and most of them rely on quite a few strings. This commit changes them to consolidate on two APIs. isContentTypeSupported, which as the name implies, takes a single content type, and returns a bool about whether or not that content type is allowed. getPreferedContentType, which takes an array of multiple options, and fine the first one in the list that matches the clients expected string. These two functions makes these functions more able to be reused in the future, and don't require specialized entries for each possible type or combination of types that we need to check for. Tested: Unit tests passing. Pretty good coverage. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I8b976d0cefec5f24e62fbbfae33d12cc803cb373
2022-08-06Use enum overload for field settingEd Tanous1-1/+2
There are two overloads of addHeader, one that takes a string, and one that takes a boost enum. For most common headers, boost contains a string table with all of those entries anyway, so there's no point in duplicating the strings, and ensures that we don't make trivial mistakes, like capitalization or - versus underscore that aren't caught at compile time. Tested: This saves a trivial amount (572 bytes) of compressed binary size. curl --insecure -vvv --user root:0penBmc https://192.168.7.2/redfish/v1 returns < Content-Type: application/json curl --insecure -vvv -H "Accept: text/html" --user root:0penBmc https://192.168.7.2/redfish/v1 Returns < Content-Type: text/html;charset=UTF-8 Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I34c198b4f9e219247fcfe719f9b3616d35aea3dc
2022-08-03Remove jsonMode methodEd Tanous1-1/+2
The jsonMode method is a single line, that basically only sets the content type, and is only called from one place. In all other cases we set the content-type directly from a header, so we should do the same here. Tested: curl --insecure -vvv --user root:0penBmc https://192.168.7.2/redfish/v1 returns < Content-Type: application/json Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I82f6fcf1b574192620ac7b29f97cdd01334c18c4
2022-07-16Remove usages of boost::starts/ends_withEd Tanous1-1/+0
Per the coding standard, now that C++ supports std::string::starts_with and std::string::ends_with, we should be using them over the boost alternatives. This commit goes through and updates all usages. Arguably some of these are incorrect, and instances of common error 13, but because this is mostly a mechanical it intentionally doesn't try to handle it. Tested: Unit tests pass. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ic4c6e5d0da90f7442693199dc691a47d2240fa4f
2022-06-28Fix shadowed variable issuesEd Tanous1-3/+3
This patchset is the conclusion of a multi-year effort to try to fix shadowed variable names. Variables seem to be shadowed all over, and in most places they exist, there's a "code smell" of things that aren't doing what the author intended. This commit attempts to clean up these in several ways by: 1. Renaming variables where appropriate. 2. Preferring to refer to member variables directly when operating within a class 3. Rearranging code so that pass through variables are handled in the calling scope, rather than passing them through. These patterns are applied throughout the codebase, to the point where -Wshadow can be enabled in meson.build. Tested: Code compiles, unit tests pass. Still need to run redfish service validator. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: If703398c2282f9e096ca2694fd94515de36a098b
2022-06-06Fix www-authenticate behaviorEd Tanous1-1/+1
bmcweb is in a weird position where, on the one hand, we would like to support Redfish to the specification, while also supporting a secure webui. For better or worse, the webui can't currently use non-cookie auth because of the impacts to things outside of Redfish like websockets. This has lead to some odd code in bmcweb that tries to "detect" whether the browser is present, so we don't accidentally pop up the basic auth window if a user happens to get logged out on an xhr request. Basic auth in a browser actually causes CSRF vulnerabilities, as the browser caches the credentials, so we don't want to make that auth method available at all. Previously, this detection was based on the presence of the user-agent header, but in the years since this code was originally written, a majority of implementations have moved to sending a user-agent by default, which makes this check pretty much useless for its purpose. To work around that, this patchset relies on the X-Requested-With header, to determine if a json payload request was done by xhr. In theory, all browsers will set this header when doing xhr requests, so this should provide a "more correct" solution to this issue. Background: https://en.wikipedia.org/wiki/List_of_HTTP_header_fields "X-Requested-With Mainly used to identify Ajax requests (most JavaScript frameworks send this field with value of XMLHttpRequest)" Tested: curl -vvvv --insecure https://192.168.7.2/redfish/v1/SessionService/Sessions Now returns a WWW-Authenticate header Redfish-protocol-validator now passes 7 more tests from the RESP_HEADERS_WWW_AUTHENTICATE category. Launched webui-vue and logged in. Responses in network tab appear to work, and data populates the page as expected. Used curl to delete redfish session from store with DELETE /redfish/v1/SessionService/Sessions/<SessionId> Then clicked an element on the webui, page forwarded to login page as expected. Opened https://localhost:8000/redfish/v1/CertificateService in a browser, and observed that page forwarded to the login page as it should. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I60345caa41e520c23fe57792bf2e8c16ef144a7a
2022-06-01Try to fix the lambda formatting issueEd Tanous1-120/+116
clang-tidy has a setting, LambdaBodyIndentation, which it says: "For callback-heavy code, it may improve readability to have the signature indented two levels and to use OuterScope." bmcweb is very callback heavy code. Try to enable it and see if that improves things. There are many cases where the length of a lambda call will change, and reindent the entire lambda function. This is really bad for code reviews, as it's difficult to see the lines changed. This commit should resolve it. This does have the downside of reindenting a lot of functions, which is unfortunate, but probably worth it in the long run. All changes except for the .clang-format file were made by the robot. Tested: Code compiles, whitespace changes only. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43
2022-05-31meson option: make the insecure-disable-auth macro more accurateNan Zhou1-4/+4
The "auth" term is overloaded in meson option and macros. This commit changes the macro from BMCWEB_INSECURE_DISABLE_AUTHENTICATION to BMCWEB_INSECURE_DISABLE_AUTHX, given that if "insecure-disable-auth" is enabled, both authentication and authorization are disabled. Tested: 1. set 'insecure-disable-auth=enabled', no authz nor authn is performed, no crash on AccountService as well. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Iddca1f866d16346bcc2017338fa6f077cb89cef9
2022-05-27Include-what-you-use in http connectionEd Tanous1-0/+4
Lots of #includes were missing in this file that we tangentially got through boost/beast/websocket.hpp. Tested: Code builds. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iac5198f2f65eabaecf47d0fb6bb05bfa5a261f32
2022-05-26auth: change authorization.hpp to authentication.hppNan Zhou1-4/+4
The existing authorization header is actually doing "authentication" work. The authorization is happening in routing.hpp where we fetch the role of the authenticated user and get their privilege set. This commits changes the name of the file, as well as the namespace, to be more precise on what the file actually does. Tested: 1. Trivial change, it builds Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Ib91ed70507a7308522c7e5363ed2f4dc279a19d9
2022-04-05Redfish: Query parameters: OnlyEd Tanous1-1/+0
Add the query parameter "only" for redfish. The specification is based on DSP0266_1.8.0. This commit is inspired by the commit that carries the same title, but is largely unique, namely, in that it adds the core feature to be able to recall handle with a new Response object, and make sure the result gets to the connection. It does this by swapping the handlers and implementing move semantics on the Response object. It definitely needs broken up into a few smaller patches, but it does pass the below tests without any apparent seg faults or ownership issues. It implements a number of cleanups that deserve their own patches, and will be split up accordingly, but for the moment, I think this is a good start to getting filter and expand support in the future. Tested: Validator passes (on previous patchset) ~$ curl -i -k -H "X-Auth-Token: $token" -X GET "https://${bmc}/redfish/v1/Systems" ~$ curl -i -k -H "X-Auth-Token: $token" -X GET "https://${bmc}/redfish/v1/Systems?only" Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I123d8ab8bcd88a0b63ff131f6b98548951989755
2022-03-07Don't rely on operator << for object loggingEd Tanous1-1/+1
In the upcoming fmt patch, we remove the use of streams, and a number of our logging statements are relying on them. This commit changes them to no longer rely on operator>> or operator+ to build their strings. This alone isn't very useful, but in the context of the next patch makes the automation able to do a complete conversion of all log statements automatically. Tested: enabled logging on local and saw log statements print to console Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I0e5dc2cf015c6924037e38d547535eda8175a6a1
2022-03-01Change the completionhandler to accept ResNan Zhou1-18/+18
These modifications are from WIP:Redfish:Query parameters:Only (https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/47474). It will be used in future CLs for Query Parameters. The code changed the completion handle to accept Res to be able to recall handle with a new Response object. AsyncResp owns a new res, so there is no need to pass in a res. Also fixed a self-move assignment bug. Context: Originally submitted: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/480020 Reveted here: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/48880 Because of failures here: https://gerrit.openbmc-project.xyz/c/openbmc/openbmc/+/48864 Tested: 1. Romulus QEMU + Robot tests; all passed 2. Use scripts/websocket_test.py to test websockets. It is still work correctly. 3. Tested in real hardware; no new validator errors; tested both authless, session, and basic auth. 4. Hacked codes to return 500 errors on certain resource; response is expected; 5. Tested Eventing, the push style one (not SSE which is still under review), worked as expected. 6. Tested 404 errors; response is expected. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Signed-off-by: John Edward Broadbent <jebr@google.com> Change-Id: I52adb174476e0f6656335baa6657456752a031be
2022-02-11Add readability-redundant-* checksEd Tanous1-2/+1
There's a number of redundancies in our code that clang can sanitize out. Fix the existing problems, and enable the checks. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie63d7b7f0777b702fbf1b23a24e1bed7b4f5183b
2022-02-09Enable readability-avoid-const-params-in-declsEd Tanous1-1/+2
This check involves explicitly declaring variables const when they're declared auto, which helps in readability, and makes it more clear that the variables are const. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I71198ea03850384a389a56ad26f2c4a48c75b148
2022-01-12Enable pro-type-cstyle-cast checksEd Tanous1-2/+3
We actually do a pretty good job of this, and only have one C style cast, that's part of an openssl macro, so ignore the one, and enable the checks. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie0462ee947c8310457365ba2aeea78caedb93da1
2022-01-12Enable checks for pointer arithmeticEd Tanous1-2/+4
Quite a few places we've disobeyed this rule, so simply ignore them for now to avoid new issues popping up. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I3e518a8e8742279afb3ad1a9dad54006ed109fb1
2022-01-12Enable reinterpre_cast checksEd Tanous1-2/+4
We seem to use reinterpret cast in a few cases unfortunately. For the moment, simply ignore most of them, and make it so we don't get more. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ic860cf922576b18cdc8d51d6132f5a9cbcc1d9dc
generated by cgit v1.2.3 (git 2.39.0) at 2024-08-06 13:43:20 +0300