summaryrefslogtreecommitdiff
path: root/http
AgeCommit message (Collapse)AuthorFilesLines
2024-10-24Remove subscription for TerminateAfterRetries policyMyung Bae1-0/+38
Redfish Data Model [1] section 6.42.5.2 and EventDestination schema [2] specify that the subscription is terminated under the following policy and the conditions. DeliveryRetryPolicy: - `TerminateAfterRetries`: ``` This value shall indicate the subscription is terminated after the maximum number of retries is reached, specified by the DeliveryRetryAttempts property in the event service. ``` This implements this policy to delete subscription of the stale client connections after trying `DeliveryRetryAttempts` when `DeliveryRetryPolicy == TerminateAfterRetries`. Tested: 1) Subscription with blocked communication between bmcweb and listener - Run Redfish Event Listener [3] to subscribe events with `DeliveryRetryPolicy == TerminateAfterRetries` - Check the subscription creation ``` curl -k -X GET https://${bmc}/redfish/v1/EventService/Subscriptions/ ``` - Generate an event and check the delivery to the listener. For example, ``` curl -k -X POST https://${bmc}/redfish/v1/EventService/Actions/EventService.SubmitTestEvent ``` - Block the communication between bmcweb and listener (or modify the listener not to delete the subscription at its exit, and kill the listener so that its subscription is still alive) - If the above task is already finished, generate more events and wait for the sufficient time till `DeliveryRetryAttempts`. - bmcweb journal log may contain the entries like ``` Sep 20 10:47:55 p10bmc bmcwebd[286]: [ERROR http_client.hpp:444] Maximum number of retries reached. https://9.3.62.209:8080/Redfish-Event-Listener Sep 20 10:47:55 p10bmc bmcwebd[286]: [ERROR event_service_manager.hpp:1459] Subscription 590587653 is deleted after MaxRetryAttempts ``` - Check the subscription again if the subscription is deleted. 2) Redfish Validator passes [1] https://www.dmtf.org/sites/default/files/standards/documents/DSP0268_2024.3.html [2] https://github.com/openbmc/bmcweb/blob/878edd599b1706ec8ffe6c3d81ba7cb3534f6393/redfish-core/schema/dmtf/csdl/EventDestination_v1.xml#L857 [3] https://github.com/DMTF/Redfish-Event-Listener Change-Id: I6e41288995cbb6e37e17a7ef1be093abb7ce54b9 Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2024-10-18Break out lambdas in http serverEd Tanous1-144/+167
These lambdas originally came from crow[1] and are a lot harder to maintain than normal methods. Move to normal methods. Tested: Unit tests pass. Good coverage on connection class. [1] https://github.com/CrowCpp/Crow/blob/master/include/crow/http_connection.h#L485 Change-Id: I9b177a0c456e44a261ea335f68354ad857739662 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-10-14Move UserSubscription to compositionEd Tanous1-3/+5
This allows for two very important simplifying changes. First, we can use the default copy operators on the UserSubscription class, which is far less error prone than writing it manually, which we have two copies of in code already. Second, it allows the Subscription class to move to using values rather than shared_ptr everywhere, which cleans up a significant amount of code. Tested: Ran Redfish-Event-Listener, subscription created and destroyed correctly. Calling POST SubmitTestEvent showed events propagating to server. Change-Id: I6d258cfe3594edddf3960ae2d4559d70acca1bf8 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-10-14Reformat with Never-AlignTrailingComments styleMyung Bae1-8/+14
clang-format currently formats the codes to align the trailing comments of the consecutive lines via `AlignTrailingComments/Kind` as `Always` in `.clang-format` file. This could shift the comment lines by the neighboring code changes and also potentially mislead the `diff` of code changes. This commit is to keep the existing trailing comments as they were. Tested: - Check whitespace only - Code compiles & CI passes. Change-Id: I1c64d53572a81d5012aa748fe44478f80c271c5f Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2024-09-16Fix status for non-existent JsonSchema FileGetMyung Bae1-5/+17
This will fix the incorrect status 500 to status 404 for the non-eixstent JsonSchema FileGet. ``` % redfishtool raw GET -r ${bmc} -u root -p 0penBmc -S Always /redfish/v1/JsonSchemas/ComputerSystem/ComputerSystem.v1_99_1.json redfishtool: Transport: Response Error: status_code: 500 -- Internal Server Error redfishtool: raw: Error getting response ``` This commit also refactor `Response::openFile()` to return `ec` so that the caller can check the reason of the failure. Tested: - Verify redfishtool result for the non-existent JsonSchema file like ``` % redfishtool raw GET -r ${bmc} -u root -p 0penBmc -S Always /redfish/v1/JsonSchemas/<schema>/<non-existent-schema>.json redfishtool: Transport: Response Error: status_code: 404 -- Not Found redfishtool: raw: Error getting response ``` - Redfish Service validator passes Change-Id: I98927c076bb6e7dfb3742183b4b3545e328d2657 Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2024-09-13Rename sendEventEd Tanous1-2/+2
There are currently 3 function prototypes that hold the name "sendEvent". This makes them hard to search for, and even though they take different arguments, and are attached to different classes, they're still difficult to trace. Rename two of the classes. Tested: Code compiles. Rename only. Change-Id: I5df9c690ba0ca8ebe19c73fc0848e9c3ef4d52f7 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-09-13websocket: Handle eof and truncated streamLei YU1-1/+4
When doRead() fails, the code was checking the `closed` error code and print the error log if it's other error codes. In field we noticed that the error code could be `eof` or `stream_truncated` if the websocket gets closed. Add the above error codes as well so that it does not print error log on closed websocket. Signed-off-by: Lei YU <yulei.sh@bytedance.com> Change-Id: Id25f9750521d67643a125d7641eb73c75c328a85
2024-09-11Remove duplicated block commentsEd Tanous1-13/+13
Static analysis flags that these two comments are redundant[1], which seem to be duplicated a lot in copyright headers. Although there is a larger discussion that can likely be had. [1] https://sonarcloud.io/project/issues?issueStatuses=OPEN%2CCONFIRMED&id=edtanous_bmcweb&open=AY9_HYjgKXKyw1ZFwgVP Tested: Comment change only. Code compiles. Change-Id: Ia960317761f558a87842347ca0b5f3da63f8e730 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-09-10static -> inlineEd Tanous1-2/+1
Declaring a function static in a header makes no sense, because a header isn't a compile unit. Find all the issues and replace them with inline. Change-Id: Icfc2b72d94b41a3a880da1ae6975beaa30a6792b Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-09-04Fix includesEd Tanous2-1/+12
This commit is automatically generated by enabling clang-include-fixer. Tested: Code compiles. Change-Id: I475d7b9d43e95bbdeeaadf11905d3b2a60aa8ef3 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-08-28Fix redfish response for unknown methodEd Tanous3-20/+27
Redfish protocol validator recently added a new test to check for invalid methods[1] and expect a valid redfish response. bmcweb will return the correct response if the HTTP method is known, but returns 404 for unknown methods. This commit implements support for returning the proper code for completely unknown methods. Tested: Redfish protocol validator passes. ``` curl -k -X METHODNOTEXIST "https://192.168.7.2/redfish/v1" curl --http1.1 -k -X METHODNOTEXIST "https://192.168.7.2/redfish/v1" ``` works for both HTTP1 and HTTP2. [1] https://github.com/DMTF/Redfish-Protocol-Validator/commit/2476e126b818f040c7231de3e9dc4df93ee636a1 Change-Id: Id7436345042bd387d7a7b706acd06afda744ddc4 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-08-25Clang-format-18Ed Tanous1-2/+1
These files were checked in during the clang-18 merge. Update them. Change-Id: I857a87dac29469a4c24e83c6ee8b7c8461002f04 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-08-23Enable bmcweb dynamic loggingAushim Nagarkatti1-3/+19
Create a CLI app called "bmcweb" that can set logging levels on "bmcwebd", the new bmcweb daemon. Create a dbus connection to set log level using the CLI app Define the "setLogLevel" method on dbus to control logging level in bmcwebd Add logic to move logging level from build option to dynamic overloading Reason: bmcweb picks up logging level as a compile flag. We want it to be more flexible to debug errors in the field. Using the bmcweb CLI app, we can set log levels on the bmcweb daemon during runtime. Splitting bmcweb. For example, to set logging level to INFO on the target: bmcweb -l INFO Change-Id: I7192e4d0ac7aa3a91babecc473521be27ea8acd1 Signed-off-by: Aushim Nagarkatti <anagarkatti@nvidia.com>
2024-08-21Partial revert of http_clientEd Tanous1-5/+4
4d69861f shows that after being applied, connections are no longer reused. It's unclear why, but very likely due to a use after move of the request object we've seen before. This doesn't cause functional issues, only performance ones. Considering that this was only a minor size reduction in the first place, Revert the http_client part of the patch for now. https://gerrit.openbmc.org/c/openbmc/bmcweb/+/69234 Tested: Aggregation through /redfish/v1/Chassis works as expected. Change-Id: I9b56e2c90b108e41df3ba006105106adf8ced154 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-08-17clang-format: re-format for clang-18Patrick Williams11-270/+262
clang-format-18 isn't compatible with the clang-format-17 output, so we need to reformat the code with the latest version. The way clang-18 handles lambda formatting also changed, so we have made changes to the organization default style format to better handle lambda formatting. See I5e08687e696dd240402a2780158664b7113def0e for updated style. See Iea0776aaa7edd483fa395e23de25ebf5a6288f71 for clang-18 enablement. Change-Id: Iceec1dc95b6c908ec6c21fb40093de9dd18bf11a Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
2024-08-13http_body: Implement static bmcweb::HttpBody::size() functionPotin Lai1-0/+10
In boost::beast::http::message::prepare_payload(), if HttpBody does not implement a size() function, it defaults to calling chunked(true) for HTTP/1.1 and chunked(false) for other versions. We encountered an issue with request data (an extra final chunk) when `chunked(true)` is called with empty data. To ensure prepare_payload behaves as expected, we want it to follow the code path that handles the payload size correctly. Specifically, we want it to: - Use `content_length(n)` for requests with data. - Use `chunked(false)` for requests without data. By adding a static implementation of the bmcweb::HttpBody::size() function, we ensure the payload size is returned correctly, guiding prepare_payload to the expected code section[1] Tested on Catalina with continuous Redfish aggregation queries; no errors or incorrect responses were observed. [1] https://github.com/boostorg/beast/blob/fee9be0be10c9c9a22ac1505a710d1d8ed5a3dfb/include/boost/beast/http/impl/message.hpp#L364-L374 Signed-off-by: Potin Lai <potin.lai@quantatw.com> Signed-off-by: Ed Tanous <etanous@nvidia.com> Change-Id: I4d84c8b6b9b3d65ce97e010b875ea49b3e1fc9d0
2024-08-09Add case defaultEd Tanous1-1/+4
Clang-18 notes that this doesn't have a case default. Rearrange. Tested: unit test pass. Change-Id: I0e1c9e5aa576ef48466a1ff98d12a3e0cbab3978 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-08-08Fix overflowEd Tanous1-1/+5
Static analysis correctly notes that in the case of empty string, this can overflow. Given that this is coming from a constexpr expression, that would be impossible in practice, but having static analysis pass is helpful. Refactor the code to account for index overflows. Tested: Code compiles Change-Id: I00a1e0661182a6fb15acd6822faabcc0ff8191b7 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-07-30Break out SSL key handler into a compile unitEd Tanous4-147/+137
This commit allows for no code to have to pull in openssl headers directly. All openssl code is now included in compile units, or transitively from boost. Because http2 is optional, no-unneeded-internal-declaration is needed to prevent clang from marking the functions as unused. Chromium has disabled this as well[1] Tested: Redfish service validator passes. [1] https://issues.chromium.org/issues/40340369 Change-Id: I327e8ffa45941c2282db804d0be56cf64155e67d Signed-off-by: Ed Tanous <ed@tanous.net>
2024-07-30Add SSE filter param supportEd Tanous2-11/+13
The Redfish spec require filtering of SSE entries to be supported. This commit rearranges the code, and implements SSE sorting as well as support for Last-Event-Id. To do this it adds a dependency on boost circular_buffer. Tested: SSE connections succeed. Show filtered results. Change-Id: I7aeb266fc40471519674c7b65cd5cc4625019e68 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-07-23EventDestination: Implement VerifyCertificateEd Tanous1-11/+24
VerifyCertificate is a property on the Redfish EventDestination schema. It specifies that this property is: ``` An indication of whether the service will verify the certificate of the server referenced by the `Destination` property prior to sending the event ``` To keep prior behavior, and to ensure behavior that's secure by default, if the user omits the property, it is assumed to be true. This property is also persisted and restored. Tested: Redfish-Event-Listener succeeds with the following procedure Start Redfish-Event-Listener PATCH /redfish/v1/Subscriptions/<subid> VerifyCertificate: false POST /redfish/v1/EventService/Actions/EventService.SubmitTestEvent Redfish-Event-Listener then hits an internal error, due to an encoding compatibility unrelated to this patch, but is documented in the receiver [1] POST of a subscription with VerifyCertificate: false set, succeeds. [1] https://github.com/DMTF/Redfish-Event-Listener/blob/6f3f98beafc89fa9bbf86aa4f8cac6c1987390fb/RedfishEventListener_v1.py#L61 Change-Id: I27e0a3fe87b4dbd0432bfaa22ebf593c3955db11 Signed-off-by: Ravi Teja <raviteja28031990@gmail.com> Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-07-14Break out DuplicatableFileHandleEd Tanous1-51/+14
Static analysis notes that writing non-trivial move constructors and move operator= handlers is non trivial [1]. Funnily enough, we actually already had bugs on this that were fixed in 06fc9be. Simplify the code. Tested: Redfish service validator passes. [1] https://sonarcloud.io/project/issues?issues=AY9_HYiwKXKyw1ZFwgUG%2CAY9_HYiwKXKyw1ZFwgUF&id=edtanous_bmcweb&open=AY9_HYiwKXKyw1ZFwgUG Change-Id: If96383d8c669ae9e5a8cc13304071b2dfe1ff2d2 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-07-12Support RespondToUnauthenticatedClients PATCHEd Tanous3-71/+103
RespondToUnauthenticatedClients allows users to explicitly select mTLS as their only authentication mechanism, thus significantly reducing their code exposure to unauthenticated clients. From the Redfish specification ``` The RespondToUnauthenticatedClients property within the ClientCertificate property within the MFA property of the AccountService resource controls the response behavior when an invalid certificate is provided by the client. • If the property contains true or is not supported by the service, the service shall not fail the TLS handshake. This is to allow the service to send error messages or unauthenticated resources to the client. • If the property contains false , the service shall fail the TLS handshake. ``` This commit implements that behavior. This also has some added benefits in that we no longer have to check the filesystem for every connection, as TLS is controlled explicitly, and not whether or not a root cert is in place. Note, this also implements a TODO to disable cookie auth when using mTLS. Clients can still use IsAuthenticated to determine if they are authenticated on request. Tested: Run scripts/generate_auth_certs.py to set up a root certificate and client certificate. This verifies that mTLS as optional has not been broken. Script succeeds. ``` PATCH /redfish/v1/AccountService {"MultiFactorAuth": {"ClientCertificate": {"RespondToUnauthenticatedClients": false}}} ``` GET /redfish/v1 without a client certificate now fails with an ssl verification error GET /redfish/v1 with a client certificate returns the result ``` PATCH /redfish/v1/AccountService {"MultiFactorAuth": {"ClientCertificate": {"RespondToUnauthenticatedClients": false}}} With certificate returns non mTLS functionality. ``` Change-Id: I5a9d6d6b1698bff83ab62b1f760afed6555849c9 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-07-12Store Request Fields that are needed laterEd Tanous4-13/+15
Because of recent changes to how dbus authentication is done, Requests might be moved out before they can be used. This commit is an attempt to mitigate the problem without needing to revert that patch. This commit does two relatively distinct things. First, it moves basic auth types to a model where they're timed out instead of removed on destruction. This removes the need for a Request object to track that state, and arguably gives better behavior, as basic auth sessions will survive through the timeout. To prevent lots of basic auth sessions getting created, a basic auth session is reused if it was: 1. Created by basic auth previously. 2. Created by the same user. 3. Created from the same source IP address. Second, both connection classes now store the accept, and origin headers from the request in the connection class itself, removing the need for them. Tested: HTML page now loads when pointing at a redfish URL with a browser. Change-Id: I623b43cbcbb43d9e65b408853660be09a5edb2b3 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-07-11Mutual TLS parsing change at runtimeEd Tanous1-24/+44
Redfish AccountService[1] defines methods for selecting how to map a certificate CommonName attribute to a user. These are intended to be a patch parameter. This commit implements the Redfish defined schemas; The parsing mode is stored in the bmcweb persistent configuration file as an integer enum, with Mapping to the Redfish schema. To handle OEM specific parsing modes, an enum value of 100+ is defined to allow the additional OEM parameters. Unfortunately, Redfish doesn't have a way to represent these today, so those modes are currently not selectable at runtime. Now that things are runtime selectable, this obsoletes the option mutual-tls-common-name-parsing, as it is not longer required at compile time. Tested: GET /redfish/v1/AccountService returns MultiFactorAuth/ClientCertificate/CertificateMappingAttribute PATCH /redfish/v1/AccountService ``` {"MultiFactorAuth": {"ClientCertificate": {"CertificateMappingAttribute":"CommonName"}}} ``` Returns 200 [1] https://github.com/DMTF/Redfish-Publications/blob/5b217908b5378b24e4f390c063427d7a707cd308/csdl/AccountService_v1.xml#L1631 Change-Id: I67db0dfa5245a9da973320aab666d12dbd9229e4 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-06-25Remove isSecure flagEd Tanous2-6/+1
There's nowhere this is used anymore; Nothing dealing with things at a Request/Response level should have to care about whether the transport was TLS or not. Previously it was used for some HTTP/HTTPS redirection logic, but that logic has been gone for many years. Tested: Code compiles. Change-Id: Id95bab1f9dce0168505b1377b833008b3a18a948 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-06-18Fix spelling mistakes using codespellManojkiran Eda2-2/+2
This commit corrects various spelling mistakes throughout the repository. The corrections were made automatically using `codespell`[1] tool. [1]: https://github.com/codespell-project/codespell Change-Id: I8751c35ad7246f73ec2e7cd4a04488c25bde3f39 Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com> Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2024-06-10MTLS Client: Enabling mtls support in http_clientAbhilash Raju1-32/+4
http_client currently does not uses mtls client certificates. This is a good feature for both authentication and authorization purpose. It will help external servers to trust the identity of bmc for better security.This patch will add MTLS client certificate support for bmcweb This is a needed feature to support secure redfish aggregation between BMCs. To support secure aggregation BMCs should be provisioned with CA signed certificate with an authorized username as the subject name field of the certificate. With the support of strong MTLS authentication from Bmcweb server we can use the MTLS path to enable secure redfish aggregation among BMCs. This can avoid complexities and extra API calls needed for token based approach. Tested by: Aggregation Test1: 1) Setup two instance of romulus qemu session at different ports.This will act as two BMCs 2) Installed CA root certificates at /etc/ssl/certs/authority in both BMCs 3) Installed server.pem and client.pem entity certificates signed by the root CA at /etc/ssl/certs/https folder in both BMCs 4) Enable aggregation for Bmcweb. 5) Fired several redfish queries to BMC1 Result Observed that the aggregation worked fine. User session created using username mentined in the CN field of certificate. Aggregation Test2: Followed same steps from Aggregation Test1 with modification in step 3 In step3 installed only the server.pem. Result Bmcweb ran as usual. But aggregation failed to collect resources from BMC2. No crash observed. Redfish Event Test: Subscribed for redfish events using test server. Fired redfish test events from BMC. Result: Events reached server successfully. Change-Id: Id8cccf9beec77da0f16adb72d52f3adf46347d06 Signed-off-by: Abhilash Raju <abhilash.kollam@gmail.com> Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-06-10Make cert generate for readonly directoriesEd Tanous1-2/+13
When run from a development PC, we shouldn't REQUIRE that the cert directory exists or is writable. This commit reworks the SSL cert generation to generate a string with the certification info, instead of writing it to disk and reading it back. This allows bmcweb to start up in read-only environments, or environments where there isn't access to the key information. Tested: Launching the application on a dev desktop without an ssl directory present no longer crashes. Change-Id: I0d44eb1ce8d298986c5560803ca2d72958d3707c Signed-off-by: Ed Tanous <ed@tanous.net>
2024-06-07http_client: Fixing bug in retry after a close callAbhilash Raju1-2/+2
After a close call httpclient is not starting with fresh socket to restart the connection. Send failure is observed in cases where new connection is started from doResolve.Calling restartConnection instead of doResolve did fix the issue. Tested By: Running developer test on use cases such as redfish aggregation where number of retries are smaller. Change-Id: I12f6a73fbafd14f482807f34ffa1e02fad944fc1 Signed-off-by: Abhilash Raju <abhilash.kollam@gmail.com>
2024-06-05Make SSE passEd Tanous6-74/+95
Redfish protocol validator is failing SSE. This is due to a clause in the Redfish specification that requires a "json" error to be returned when the SSE URI is hit with a standard request. In what exists today, we return 4XX (method not allowed) but because this is handled by the HTTP layer, it's not possible to return the correct Redfish payloads for when that 4XX happens within the Redfish tree, because there is in fact a route that matches, that route just doesn't support the type that we need. This commit rearranges the router such that there are now 4 classes of rules. 1. "verb" rules. These are GET/POST/PATCH type, and they are stored using the existing PerMethod array index. 2. "upgrade" rules. These are for websocket or SSE routes that we expect to upgrade to another route 3. 404 routes. These are called in the case where no route exists with that given URI pattern, and no routes exist in the table for any verb. 4. 405 method not allowed. These are called in the case where routes exist in the tree for some method, but not for the method the user requested. To accomplish this, some minor refactors are implemented to separate out the 4xx handlers to be their own variables, rather than just existing at an index at the end of the verb table. This in turn means that getRouteByIndex now changes to allow getting the route by PerMethod instance, rather than index. Tested: unit tests pass (okish coverage) Redfish protocol validator passes (with the exception of #277, which fails identically before and after). SSE tests now pass. Redfish service validator passes. Change-Id: I555c50f392cb12ecbc39fbadbae6a3d50f4d1b23 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-05-28Implement Chunking for unix socketsEd Tanous3-12/+38
Response::openFd was added recently to allow handlers to pass in a file descriptor to be used to read. This worked great for files, but had some trouble with unix sockets. First, unix sockets have no known length that we can get. They are fed by another client until that client decides to stop sending data and sends an EOF. HTTP in general needs to set the Content-Length header before starting a reply, so the previous code just passes an error back. HTTP has a concept of HTTP chunking, where a payload might not have a known size, but can still be downloaded in chunks. Beast has handling for this that we can enable that just deals with this at the protocol layer silently. This patch enables that. In addition, a unix socket very likely might not have data and will block on the read call. Blocking in an async reactor is bad, and especially bad when you don't know how large a payload is to be expected, so it's possible those bytes will never come. This commit sets all FDs into O_NONBLOCK[1] mode when they're sent to a response, then handles the subsequent EWOULDBLOCK and EAGAIN messages when beast propagates them to the http connection class. When these messages are received, the doWrite loop is simply re-executed directly, attempting to read from the socket again. For "slow" unix sockets, this very likely results in some wasted cycles where we read 0 bytes from the socket, so shouldn't be used for eventing purposes, given that bmcweb is essentially in a spin loop while waiting for data, but given that this is generally used for handling chunking of large payloads being generated, and while spinning, other reactor operations can still progress, this seems like a reasonable compromise. [1] https://www.gnu.org/software/libc/manual/html_node/Open_002dtime-Flags.html Tested: The next patch in this series includes an example of explicitly adding a unix socket as a response target, using the CredentialsPipe that bmcweb already has. When this handler is present, curl shows the response data, including the newlines (when dumped to a file) ``` curl -vvvv -k --user "root:0penBmc" https://192.168.7.2/testpipe -o output.txt ``` Loading the webui works as expected, logging in produces the overview page as expected, and network console shows no failed requests. Redfish service validator passes. Change-Id: I8bd8586ae138f5b55033b78df95c798aa1d014db Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-13Move under exception handlerEd Tanous1-1/+2
Static analysis still sometimes flags that this throws, even through clang-tidy doesn't. Move it under the exception handler. Tested: Logging still works. Change-Id: I67425749b97b0a259746840c7b9a9b4834dfe52e Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-10Fix lesser used optionsEd Tanous4-60/+63
25b54dba775b31021a3a4677eb79e9771bcb97f7 missed several cases where we had ifndef instead of ifdef. because these weren't the defaults, these don't show up as failures when testing. Tested: Redfish service validator passes. Inspection primarily. Mechanical change. Change-Id: I3f6915a97eb44d071795aed76476c6bee7e8ed27 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-07Move logging argsEd Tanous3-11/+32
Args captured by logging functions should be captured by rvalue, and use std::forward to get perfect forwarding. In addition, separate out the various std::out lines. While we're here, also try to optimize a little. We should ideally be writing each log line to the output once, and ideally not use iostreams, which induce a lot of overhead. Similar to spdlog[1] (which at one point this codebase used), construct the string, then call fwrite and fflush once, rather than calling std::cout repeatedly. Now that we don't have a dependency on iostreams anymore, we can remove it from the places where it has snuck in. Tested: Logging still functions as before. Logs present. [1] https://github.com/gabime/spdlog/blob/27cb4c76708608465c413f6d0e6b8d99a4d84302/include/spdlog/sinks/stdout_sinks-inl.h#L70C7-L70C13 Change-Id: I1dd4739e06eb506d68989a066d122109b71b92cd Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-03Manage Request with shared_ptrJonathan Doman5-60/+60
This is an attempt to solve a class of use-after-move bugs on the Request objects which have popped up several times. This more clearly identifies code which owns the Request objects and has a need to keep it alive. Currently it's just the `Connection` (or `HTTP2Connection`) (which needs to access Request headers while sending the response), and the `validatePrivilege()` function (which needs to temporarily own the Request while doing an asynchronous D-Bus call). Route handlers are provided a non-owning `Request&` for immediate use and required to not hold the `Request&` for future use. Tested: Redfish validator passes (with a few unrelated fails). Redfish URLs are sent to a browser as HTML instead of raw JSON. Change-Id: Id581fda90b6bceddd08a5dc7ff0a04b91e7394bf Signed-off-by: Jonathan Doman <jonathan.doman@intel.com> Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-03Increase the file bufferEd Tanous1-1/+5
When we added file buffer, this number was picked arbitrarily. Prior to the file body patch series, files were buffered entirely in ram, regardless of what size they were. While not doing that was an improvement, I suspect that we were overly conservative in the buffer size. Nginx picks a default buffer size somewhere in the 8k - 64k range dependent on what paths the code takes. Using the higher end of that range seems like a better starting point, but generally we have more ram on the bmc than we have users. Increase the buffer to 64K. Tested: Unit tests pass. [1] https://docs.nginx.com/nginx-management-suite/acm/how-to/policies/http-backend-configuration/#buffers Change-Id: Idb472ccae02a8519c0976aab07b45562e327ce9b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-05-01Bring consistency to config optionsEd Tanous7-18/+16
The configuration options that exist in bmcweb are an amalgimation of CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms and meson options using a config file. This history has led to a lot of different ways to configure code in the codebase itself, which has led to problems, and issues in consistency. ifdef options do no compile time checking of code not within the branch. This is good when you have optional dependencies, but not great when you're trying to ensure both options compile. This commit moves all internal configuration options to: 1. A namespace called bmcweb 2. A naming scheme matching the meson option. hyphens are replaced with underscores, and the option is uppercased. This consistent transform allows matching up option keys with their code counterparts, without naming changes. 3. All options are bool true = enabled, and any options with _ENABLED or _DISABLED postfixes have those postfixes removed. (note, there are still some options with disable in the name, those are left as-is) 4. All options are now constexpr booleans, without an explicit compare. To accomplish this, unfortunately an option list in config/meson.build is required, given that meson doesn't provide a way to dump all options, as is a manual entry in bmcweb_config.h.in, in addition to the meson_options. This obsoletes the map in the main meson.build, which helps some of the complexity. Now that we've done this, we have some rules that will be documented. 1. Runtime behavior changes should be added as a constexpr bool to bmcweb_config.h 2. Options that require optionally pulling in a dependency shall use an ifdef, defined in the primary meson.build. (note, there are no options that currently meet this class, but it's included for completeness.) Note, that this consolidation means that at configure time, all options are printed. This is a good thing and allows direct comparison of configs in log files. Tested: Code compiles Server boots, and shows options configured in the default build. (HTTPS, log level, etc) Change-Id: I94e79a56bcdc01755036e4e7278c7e69e25809ce Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-30Use fadvise to trigger sequential readingEd Tanous1-0/+8
Nginx and other webservers use fadvise to inform the kernel of in-order reading. We should to the same. Tested: Webui loads correctly, no direct performance benefits immediately obvious, but likely would operate better under load. Change-Id: I4acce316c719df7df012cea8cb89237b28932c15 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-29Break out formattersEd Tanous2-91/+1
In the change made to move to std::format, we defined some custom type formatters in logging.hpp. This had the unintended effect of making all compile units pull in the majority of boost::url, and nlohmann::json as includes. This commit breaks out boost and json formatters into their own separate includes. Tested: Code compiles. Logging changes only. Change-Id: I6a788533169f10e19130a1910cd3be0cc729b020 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-27Add static webpack etag supportEd Tanous2-11/+23
Webpack (which is what vue uses to compress its HTML) is capable of generating hashes of files when it produces the dist files[1]. This gets generated in the form of <filename>.<hash>.<extension> This commit attempts to detect these patterns, and enable etag caching to speed up webui load times. It detects these patterns, grabs the hash for the file, and returns it in the Etag header[2]. The behavior is implemented such that: If the file has an etag, the etag header is returned. If the request has an If-None-Match header, and that header matches, only 304 is returned. Tested: Tests were run on qemu S7106 bmcweb with default error logging level, and HTTP/2 enabled, along with svg optimization patches. Run scripts/generate_auth_certificate.py to set up TLS certificates. (valid TLS certs are required for HTTP caching to work properly in some browsers). Load the webui. Note that DOM load takes 1.10 seconds, Load takes 1.10 seconds, and all requests return 200 OK. Refresh the GUI. Note that most resources now return 304, and DOM time is reduced to 279 milliseconds and load is reduced to 280 milliseconds. DOM load (which is what the BMC has control over) is decreased by a factor of 3-4X. Setting chrome to "Fast 5g" throttling in the network tab shows a more pronounced difference, 1.28S load time vs 3.96S. BMC also shows 477KB transferred on the wire, versus 2.3KB transferred on the wire. This has the potential to significantly reduce the load on the BMC when the webui refreshes. [1] https://webpack.js.org/guides/caching/ [2] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag Change-Id: I68aa7ef75533506d98e8fce10bb04a494dc49669 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-24Fix large content error codesEd Tanous1-88/+203
When pushing multi-part payloads, it's quite helpful if the server supports the header field of "Expect: 100-Continue". What this does, is on a large file push, allows the server to possibly reject a request before the payload is actually sent, thereby saving bandwidth, and giving the user more information. Bmcweb, since commit 3909dc82a003893812f598434d6c4558107afa28 by James (merged July 2020) has simply closed the connection if a user attempts to send too much data, thereby making the bmcweb implementation simpler. Unfortunately, to a security tester, this has the appearance on the network as a crash, which will likely then get filed as a "verify this isn't failing" bug. In addition, the default args on curl multipart upload enable the Expect: 100-Continue behavior, so folks testing must've just been disabling that behavior. Bmcweb should just support the right thing here. Unfortunately, closing a connection uncleanly is easy. Closing a connection cleanly is difficult. This requires a pretty large refactor of the http connection class to accomplish. Tested: Create files of various size and try to send them (Note, default body limit is 30 MB) and upload them with an without a username. ``` dd if=/dev/zero of=zeros-file bs=1048576 count=16 of=16mb.txt curl -k --location POST https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":["/redfish/v1/Managers/bmc"]} ;type=application/json' -F UpdateFile=@32mb.txt -v ``` No Username: 32MB returns < HTTP/1.1 413 Payload Too Large 16MB returns < HTTP/1.1 401 Unauthorized With Username 32MB returns < HTTP/1.1 413 Payload Too Large 16MB returns < HTTP/1.1 400 Bad Request Note, in all cases except the last one, the payload is never sent from curl. Redfish protocol validator fails no new tests (SSE failure still present). Redfish service validator passes. Change-Id: I72bc8bbc49a05555c31dc7209292f846ec411d43 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-23Fix http2 use after free bugEd Tanous1-2/+2
In the below code, we move out of Response, then use it to set unauthorized, which never gets returned to the user. This results in the browser showing an empty 200 ok request, because while the request was propagated rejected, the 401 error code didn't get propagated to the user. Tested: If not logged in on a chrome browser: /redfish/v1 -> Returns the UI /refish/v1/AccountService -> returns a forward to the webui login page. If logged into the webui. /redfish/v1/AccountService now returns the expected HTML redfish representation of the json response. Change-Id: I2c906f818367ebb253b3e6097e6787ba4c215e0a Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-20Change ssl stream implementationsEd Tanous8-18/+17
Boost beast ssl_stream is just a wrapper around asio ssl_stream, and aims to optimize the case where we're writing small payloads (one or two bytes.) which needs to be optimized in SSL. bmcweb never writes one or two bytes, we almost always write the full payload of what we received, so there's no reason to take the binary size overhead, and additional boost headers that this implementation requires. Tested: This drops the on-target binary size by 2.6% Redfish service validator passes. Change-Id: Ie1ae6f197f8e5ed70cf4abc6be9b1b382c42d64d Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-19Add missing headersEd Tanous1-1/+0
Most of these were found by breaking every redfish class handler into its own compile unit: When that's done, these missing headers become compile errors. We should just fix them. In addition, this allows us to enable automatic header checking in clang-tidy using misc-header-cleaner. Because the compiler can now "see" all the defines, it no longer tries to remove headers that it thinks are unused. [1] https://github.com/openbmc/bmcweb/commit/4fdee9e39e9f03122ee16a6fb251a380681f56ac Tested: Code compiles. Change-Id: Ifa27ac4a512362b7ded7cc3068648dc4aea6ad7b Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-18Clean up BMCWEB_ENABLE_SSLEd Tanous6-90/+66
This macro came originally from CROW_ENABLE_SSL, and was used as a macro to optionally compile without openssl being required. OpenSSL has been pulled into many other dependencies, and has been functionally required to be included for a long time, so there's no reason to hold onto this macro. Remove most uses of the macro, and for the couple functional places the macro is used, transition to a constexpr if to enable the TLS paths. This allows a large simplification of code in some places. Tested: Redfish service validator passes. Change-Id: Iebd46a68e5e417b6031479e24be3c21bef782f4c Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-16Remove OpenSSL warnings ignoreEd Tanous2-0/+6
If we include OpenSSL in extern "C" blocks consistently, c++ warnings no longer appear. This means we can remove the special case from meson. Tested: Code compiles when built locally on an ubuntu 22.04 system. Change-Id: I5add4113b32cd88b7fdd874174c845425a7c287a Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-14Use beast message_generatorEd Tanous2-11/+11
Beast 331 added the message_generator class, which allows deduplicating some templated code for the HTTP parser. When we use it, we can drop our binary size, and ensure that we have code reuse. This saves 2.2% on the compressed binary size. Tested: Redfish service validator passes. Change-Id: I5540d52dc256adfb62507c67ea642a9ea86d27ee Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-13Fix nullptr failures for image uploadEd Tanous1-0/+2
Several places that call *req.ioService were missing nullptr checks. Add them, and fix the one case where it might not be filled in. Tested: With HTTP2 enabled, the following command succeeds. ``` curl -k https://192.168.7.2/redfish/v1/UpdateService/update -F 'UpdateParameters={"Targets":["/redfish/v1/Managers/bmc"]} ;type=application/json' --user "root:0penBmc" -F UpdateFile=@/home/ed/bmcweb/16mb.txt -v -H "Expect:" ``` Change-Id: I81e7944c22f5922d461bf5d231086c7468a16e62 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Fix buffer_copyEd Tanous1-3/+3
boost::asio::buffer_copy returns an integer of the number of values copied. Some static analysis tools mark that value as nodiscard, although it should never fail. Audit all uses of buffer_copy, and make sure that they're using the return value. In theory this should have no change on the behavior. Change-Id: I6af39b5347954c2932cf3d4e48e96ff9ae01583a Signed-off-by: Ed Tanous <ed@tanous.net>
generated by cgit v1.2.3 (git 2.39.0) at 2026-10-01 04:38:48 +0300