summaryrefslogtreecommitdiff
path: root/http
AgeCommit message (Collapse)AuthorFilesLines
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 Tanous2-2/+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-04-01Fix SSE socketsEd Tanous2-8/+9
Redfish protocol validatator has SSE tests that expose some bad coding practies in SSE handlers, namely, that there are several cases where we don't check for nullptr. Fix them. This appears to have been introduced in: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/41319 Tested: Redfish service validator passes more tests. Change-Id: Id980725f007d044b7d120dbe0f4b625865cab6ba Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-01Fix regression in http_file_bodyEd Tanous2-0/+2
The commit: b5f288d Make use of filebody for dump offload Caused a minor failure in clearing responses, where open file handles wouldn't be closed in between queries, resulting in the next read to return empty content. This caused redfish protocol validator to fail. boost::beast::http::response::clear() documentation shows that it only clears the headers, not the file body. Now normally, this doesn't matter, because bmcweb completely replaces the response body when a new response is driven, but not in the case of files. Add response.body().clear() during the clear to ensure the response is cleared. In addition, add encodingType to the clear() call, to ensure that it is reset as well. This is a bug, but I don't know the reproduction steps. Tested: Redfish protocol validator now completes (with SSE failures) Change-Id: Ice6d5085003034a1bed48397ddc6316e9cd0536f Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-28Rename http2 unpackerEd Tanous1-8/+15
These classes accidentally overlapped in naming with the nghttp2 classes. This is because this class, unlike most nghttp2 classes doesn't end in _ptr for a type. This changes the class name to add a _ex to differentiate the two classes, and avoid a warning in clang. Tested: Unit tests pass. Code only used in unit test. Change-Id: I91a6982264df69bc65166ab38feddc21f72cd223 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-27Check optionals in tidyEd Tanous1-9/+15
clang-tidy-18 makes this feature stable enough for us to use in general. Enable the check, and fix the couple of regressions that have snuck in since we last ran the check. Tidy seems to not be able to understand that ASSERT will not continue, so if we ASSERT a std::optional, it's not a bug. Add explicit checks to keep tidy happy. Tested: clang-tidy passes. Change-Id: I0986453851da5471056a7b47b8ad57a9801df259 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-25Fix redundant init issuesEd Tanous2-6/+6
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-25Add nolint for naming violationsEd Tanous1-0/+3
Change-Id: I0133bbd0a7573bd3d1e3c3c99382442b286696f6 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-25Don't use switch for single conditional http2Ed Tanous1-40/+38
This code was copied from one of the nghttp2 examples, and makes things more complicated than they should be. We only handle one case here, so a pattern of returning early is easier. Also, this resolves a possible clang-tidy bugprone warning (that we don't yet enable). Tested: Http2 unit tests pass (good coverage for this case). Change-Id: Ie8606872f3a96f1bb0329bf22a4f7429f431bbef Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-21Allow reading http2 bodiesEd Tanous2-10/+98
This allows http2 connections to now host authenticated endpoints. Note, this work exposed that the http2 path was not calling preparePayload() and responses were therefore missing the Content-Length header. preparePayload is now called, and Content-Length is added to the unit tests. This commit also allows a full Redfish Service Validator test to pass entirely using HTTP2. Tested: Unit tests pass. Curl /redfish/v1/Managers/bmc/LogServices/Journal/Entries (which returns a payload larger than 16kB) succeeds and returns the data. Manually logging in with both basic and session authentication succeeds over http2. A modified Redfish-Service-Validator, changed to use httpx as its backend, (thus using http2) succeeds. Change-Id: I956f3ff8f442e9826312c6147d7599ab136a8e7c Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-21Allow routes with 5 wildcardsEd Tanous1-1/+1
We don't have any routes that use 5 wildcards, but clearly someone uses it because of the bug #270. There's no reason not to fix this. Ideally we would support an arbitrary number of wildcards, but that's a template problem for another day. Tested: No way to test, inspection only. Change-Id: I5de75f5288124e84c153518966d658e1c899f6d5 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-21Allow no spaces in content-typeEd Tanous1-1/+3
For the content type header application/json;charset=utf-8 The Redfish specification DSP0266 shows no space between the ; and charset. Sites like mozilla show the space included [1] Considering the discrepancy, we should just accept both. Resolves #271 [1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type Tested: Submitter reports issue fixed. Change-Id: I77b7db91d65acc84f2221ec50985d4b942fbe77f Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-19Rename FileBody to HttpBodyEd Tanous8-21/+25
Now that our custom body type does things more than files, it makes sense to rename it. This commit renames the header itself, then all instances of the class. Tested: Basic GET requests succeed. Change-Id: If4361ac8992fc7c268f48a336707f96e68d3576c Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-18Add unit test for SSEEd Tanous2-40/+36
Writing this test exposed some bugs in SSE that got merged. sendSSEHeader was never called, leading to a connection that starts and immediately closes with no error code. This issue has been corrected in code, such that the sockets start. To allow for unit tests, the io_service needs to be passed into the class, previously, the SSE connection was pulling the io_context from the DBus connection, which is odd, given that the SSE connection has no other dependencies on DBus. Unit tests should help keep it working. Tested: Unit tests pass. Change-Id: I48080d2a94b6349989f556cd1c7b103bad498526 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-02-28Fix duplicated etag generationMyung Bae1-5/+0
bmcweb generates the duplicated etags on GET and it may cause the PATCH failure if etag is used - e.g. redfishtool.. This commit is to put only one Etag on http_response on GET. Tested: - Check the duplicated Etag on GET, and check it after fix Before: ``` $ curl -v -k -X GET https://admin:${password}@${bmc}:18080/redfish/v1/AccountService/Accounts/readonly ... < ETag: "D4B30BB4" < ETag: "D4B30BB4" ``` After: ``` $ curl -v -k -X GET https://admin:${password}@${bmc}:18080/redfish/v1/AccountService/Accounts/readonly ... < ETag: "D4B30BB4" ``` Change-Id: I5361919c5671b546181a26de792bc57de2c4f670 Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2024-02-16Write unit tests for http2 connectionEd Tanous1-0/+39
This unit test currently only tests a simple connect and settings frame transfer, but should form the basis for more complex testing in the future. Tested: Unit tests pass Change-Id: Ieb803dbe490129ec5fe99fb3d4505a06202e282e Signed-off-by: Ed Tanous <ed@tanous.net>
2024-02-16Simplify HTTP/2 bufferingEd Tanous2-72/+46
Using the mem_send methods of nghttp2 can reduce the amount of buffering we need to do. This is recommended by the nghttp2 docs. Tested: Enabled experimental-http. Curl succeeds on /redfish/v1, and shows: * using HTTP/2 * [HTTP/2] [1] OPENED stream for https://localhost:18080/redfish/v1 Change-Id: I287d8c956f064d244116fac853055a17fca915a2 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-02-16Simplify bodyEd Tanous8-315/+327
Now that we have a custom boost http body class, we can use it in more cases. There's some significant overhead and code when switching to a file body, namely removing all the headers. Making the body class support strings would allow us to completely avoid that inefficiency. At the same time, it would mean that we can now use that class for all cases, including HttpClient, and http::Request. This leads to some code reduction overall, and means we're reliant on fewer beast structures. As an added benefit, we no longer have to take a dependency on boost::variant2. Tested: Redfish service validator passes, with the exception of badNamespaceInclude, which is showing warnings prior to this commit. Change-Id: I061883a73230d6085d951c15891465c2c8445969 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-02-06Make tests not require body interactionEd Tanous1-0/+10
The muitipart test interacts with some significant details of the response class. This was largely only done because Request lacked an addHeader method that Request already had. Add addHeader() method to the Request class, and adapt multipart unit tests to use it. Tested: Unit tests pass. Unit test only changes. Change-Id: Icb3b92dce6d17011ae0063a962678173b1b01a87 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-01-22Make use of filebody for dump offloadAbhilash Raju2-7/+163
Logservice has been rewritten to use file_body to offload dump files from BMC. There are two kind of dump files, BMC dump and System dump.While BMC dump just requires default support from beast::file_body, System dump requires base64 encoding support from beast. But beast::file_body do not have ready-made support for base64 encoding. So a custom file_body has been written for the base64 encoding. The openFile apis in crow::Response do not have support for unix file descriptor. Since dump files are accesses via descriptors, added new openFile api that accepts descriptors. Tested: Functionality test have been executed to verify the bmc dump offload. Did sanity test by invoking bmcweb pages via browser. Change-Id: I24192657c03d8b2f0394d31e7424c6796ba3227a Signed-off-by: Abhilash Raju <abhilash.kollam@gmail.com>
2024-01-22Make base64 encoder incrementalEd Tanous1-33/+105
As part of https://gerrit.openbmc.org/c/openbmc/bmcweb/+/67667, it would be desirable if we could incrementally encode base64 in chunks. Given that base64 encoding requires encoding 3 characters to 4, there's a possibility that a chunk might not be mod 3 length. This commit moves the base64 encoder into a class that can run incrementally. Tested: Unit tests pass. More tests in next commit. Change-Id: Ic7da3fd4db865c99fcbd96ae06fdecb87628f94c Signed-off-by: Ed Tanous <ed@tanous.net>
2024-01-19Remove some boost includesEd Tanous4-12/+15
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-09Fix spelling mistakesEd Tanous5-13/+13
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>
2024-01-09http_client: fix for broken connectionAbhilash Raju1-11/+37
http_client is not handling connection termination by server due to keep alive timeout. At present client is not aware of connection termination from server. So whenever next redfish is event ready to be sent, the client will try send/receives data over broken connection. After failed operation the client will try to restart the connection by closing the current connection. Problems: 1) Restart is not attempted on all failure paths. Eg: stream_truncated error was ignored, which usually happens when try to read from broken connection, due to which retry is never performed. 2) Ssl shutdown over broken connection often fails to call the shutdown callback 3) ssl session was reused for new connection attempt. Which is wrong Solution: This patch will try to reattempt the connection in all failure cases. It uses new socket object and new ssl session for the retries Tested by: Test normal event flow between redfish-event clients and the BMC Test failure event flow between redfish-event clients and the BMC Tested the bad path by keeping the setup idle for 3 hours on the above two setups. Verified the events flow after this idle time Change-Id: I3d725b9d77bea22e2e8860e01ee0dfc971789008 Signed-off-by: Abhilash Raju <abhilash.kollam@gmail.com> Signed-off-by: Ed Tanous <ed@tanous.net>
2023-12-25bmcweb: Add nullptr check for weakptrzhaogang.01081-0/+5
When we call a null weakptr, it will cause a crash. Add nullptr check for weakptr can avoid this situation. Tested: bmcweb.service did not experience core-dump. Change-Id: I4490d68c70ea5d43681f4fb18b3859afb01ed70a Signed-off-by: Zhao Gang <zhaogang.0108@bytedance.com>
2023-12-09mutual-tls: Add support for Meta certificatesMarco Kawajiri2-0/+87
Meta Inc's client certificates use an internal Subject CN format which AFAIK is specific to Meta and don't adhere to a known standard: Subject: CN = <type>:<entity>/<hostname> Commit adds the `mutual-tls-common-name-parsing=meta` option to, on Meta builds, parse the Subject CN field and map either the <entity> to a local user. The <type> field determines what kind of client identity the cert represents. Only type="user" is supported for now with <entity> being the unixname of a Meta employee. For example, the Subject CN string below maps to a local BMC user named "kawmarco": Subject CN = "user:kawmarco/dev123.facebook.com" Tested: Unit tests, built and tested on romulus using the script below: https://gist.github.com/kawmarco/87170a8250020023d913ed5f7ed5c01f Flags used in meta-ibm/meta-romulus/conf/layer.conf : ``` -Dbmcweb-logging='enabled' -Dmutual-tls-common-name-parsing='meta' ``` Change-Id: I35ee9b92d163ce56815a5bd9cce5296ba1a44eef Signed-off-by: Marco Kawajiri <kawajiri@meta.com>
2023-12-09Simplify mutual TLS checksEd Tanous1-64/+4
bmcweb should be using the openssl primitives for these checks. There are examples where we've known to have gotten the behavior incorrect, so given that OpenSSL clearly should know these things better than we do, use it. Tested: unit tests pass. Change-Id: I0bcd381a9e3c9a1e8e6dc39534e81fa698570689 Signed-off-by: Ed Tanous <ed@tanous.net>
2023-12-08Reponse: Fix incomplete implementation in writeAbhilash Raju1-2/+5
Write function in http_response.hpp missing implementation for first write after changing from filebody. Usecase: Current resonse type is filebody. Developer tries to change the body type to stringbody by calling write function. Observed: The write fails to update the body type. Expected: Write should succeed and body should change to string body. Tested: Unit test has been added for crow::Response. Manual sanity test done for file offloads using curl. Change-Id: Icbf8585b5b04c3ac5120d7b334c13d89ed3eb4aa Signed-off-by: Abhilash Raju <abhilash.kollam@gmail.com>
2023-12-05Fix missing dateEd Tanous1-1/+1
At some point, the date got removed from http1 requests. HTTP2 does not show this issue, but this showed up in unit tests (which is why the prior commit is adding unit tests). The Date Header is useful for synchronizing things like Cache-Control-Policy, with the actual server time, instead of the local system time. Tested: Unit tests pass. Change-Id: I8f105f0cbb6c816c5ec6b14cbeae587d728a20d2 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-12-05Unit test ConnectionEd Tanous3-104/+94
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 Tanous5-133/+242
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-24clang-format: copy latest and re-formatPatrick Williams5-6/+6
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-16Remove extra variables in websocketsEd Tanous2-30/+38
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-09-25Fix http2 stream pointerEd Tanous2-40/+30
Response and Request are now movable, so lets use that to our advantage and make this no longer require a pointer. This removes a couple NOLINT exceptions in our code, and cleans up a lot of places where we could potentially get a nullptr. Tested: enabled http2-experimental option. Loaded service root from redfish in curl with logging enabled, logging verified http/2 was being used. Redfish service validator passes. Curl compiled with http returns service root correctly. Change-Id: I65e11a2311be982df594086413d52838235e1a0c Signed-off-by: Ed Tanous <ed@tanous.net>
2023-09-21Fix unessesary URL readEd Tanous1-7/+2
This call was neccesary back when we were doing moves of a url_view, but because this constructor doesn't use a url_view anymore, this isn't neccesary. Functionally, this clears up a strange unit test failure that occured in some cases where this consturctor is used. This constructor is not used for anything but unit tests. Tested: Unit tests pass. Change-Id: I034a69d3a6b6aeada2460bb39f3518846b39f817 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-08-24Avoid setting SNI hostname for IP addressesRavi Teja1-0/+7
ssl_handshake fails while establishing connection to IPv6 destination address, as IPv6 addresses considered as invalid value for SNI hostname due to special characters. SNI allows valid HostName which allows characters are only {alphabetic characters (A-Z), numeric characters (0-9), the minus sign This commit adds check to avoid setting SNI hostname if its an IP address Tested By: Verified redfish events 1. Subscribing Destination with IPv6 address. 2. Subscribing Destination with IPv4 address. Change-Id: I32d30292bbc29c753f1c1815c66fcc93e8074eaa Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
2023-08-23Move http client to URLEd Tanous3-152/+100
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-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-19Flush each log messageJonathan Doman1-1/+1
cout is usually buffered, so make sure that every log message gets individually flushed. This is especially important when relying on the systemd journal for timestamping of messages. Change-Id: I28f6f46978c2fad7855f819b04df964ab3c51351 Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
2023-08-14Reduce some Error log severitiesCarson Labrado1-1/+1
There are instances of ERROR logs that would work better as WARNING or DEBUG since they do not actually result in bailing early and returning an error response. Signed-off-by: Carson Labrado <clabrado@google.com> Change-Id: I1e7bca0bb38487b26a4642ab72ce475170bb53c6
2023-08-07Fix bugprone-unchecked-optional-access findingsEd Tanous4-35/+59
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-02Fix logging level handlingMyung Bae1-1/+1
The log entries with the meson configured log level are currently missing. Tested: - Add the logging level to debug to local.conf ``` conf/local.conf: EXTRA_OEMESON:pn-bmcweb:append = "-Dbmcweb-logging='debug'" ``` - Run the current bmcweb and check bmcweb DEBUG logs which won't be shown. - With the fix, do the same test and check the DEBUG logs. ``` Aug 02 00:07:52 p10bmc bmcweb[229]: [INFO http_connection.hpp:229] Request: 0x1759d10 HTTP/1.1 GET /redfish ::ffff:127.0.0.1 Aug 02 00:07:52 p10bmc bmcweb[229]: [DEBUG http_connection.hpp:260] Setting completion handler Aug 02 00:07:52 p10bmc bmcweb[229]: [DEBUG http_response.hpp:238] 0x16d2540 setting completion handler Aug 02 00:07:52 p10bmc bmcweb[229]: [DEBUG routing.hpp:669] Matched rule '/redfish/' 1 / 2 Aug 02 00:07:52 p10bmc bmcweb[229]: [DEBUG query.hpp:121] setup redfish route ``` Similar tests can be done with the other logging level. Change-Id: Ifd6dac5b734363fbad70bc62f3dd03a5053ed2fd Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2023-08-01Revert "Cache user role in session object"Gunnar Mills1-0/+1
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-28Cache user role in session objectEd Tanous1-1/+0
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-27Fix loggingEd Tanous1-27/+15
The recent change to logging has caused a couple of bugs. First, when building within yocto, the complete path is now returned on log messages. This is wasteful of speed, and not super helpful to developers to have a full path. Per the discussion on the original patchset, drop this down to just the filename. 2, because of it's use as a pseudo log level, "enabled" is in the list of strings. This causes an index mismatch, which causes logs to be logged at the wrong level beyond debug. Move the entry to the end to fix this. Third, move the logging of level to upper case, to follow the old convention. Tested: Enabled meson option for logging, observed logs like: ``` Jul 25 18:39:20 qemux86-64 bmcweb[209]: [DEBUG query.hpp:121] setup redfish route Jul 25 18:39:20 qemux86-64 bmcweb[209]: [DEBUG http_response.hpp:248] 0x561bc11a7a40 releasing ce Jul 25 18:39:20 qemux86-64 bmcweb[209]: [DEBUG http_response.hpp:238] 0x561bc11a7a40 setting comr Jul 25 18:39:20 qemux86-64 bmcweb[209]: [DEBUG http_response.hpp:223] 0x561bc11a7a40 calling comr Jul 25 18:39:20 qemux86-64 bmcweb[209]: [DEBUG http_response.hpp:226] 0x561bc11a7a40 completion d Jul 25 18:39:20 qemux86-64 bmcweb[209]: [DEBUG query_param.hpp:1019] Processing query params ``` Change-Id: I4ac506c623a17f81ae83545e59291d2729dc82cb Signed-off-by: Ed Tanous <edtanous@google.com>
2023-07-25Fix events destination url parsing for IPv6Ravi Teja1-1/+1
Currently while parsing destination URL, host address enclosed in [] braces for IPv6 addresses, so Resolve hostname fails for IPv6 addresses because of this invalid hostname which is enclosed in [] braces. This commit uses encoded_host_address() method to fix this parsing hostname for IPv6 address. Tested By: Configured redfish event subscription with IPv6 destination URI verified parsing logic of destination URI with IPv6 addresses. Change-Id: I0e43468086ae0b961eb724de30e211d61ccda2d8 Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
2023-07-20Replace logging with std::formatEd Tanous15-391/+470
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-1/+1
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-11HTTP Client: Improve error messageSunitha Harish1-6/+12
This commit adds the host and port data to the existing error messages that are hit during redfish event Tested by: Checked the traces on BMC when there is a bad subscriber Change-Id: I3f18bc3b999c136c42c4c0021c5fcadddb9e4bff Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
2023-07-11Implement data pointer clang-tidy checkEd Tanous1-1/+1
readability-container-data-pointer flags one error in our codebase, but can definitely find issues in patchsets. Fix the one error (that came from crow), and enable the check. Change-Id: I3045ec9a58d80300c90921dda1a2fe3859ffed7b Signed-off-by: Ed Tanous <edtanous@google.com>