463a0e3e | 14-Oct-2024 |
Ed Tanous <etanous@nvidia.com> |
Explicitly set verify_none
As reported, there are cases where a valid certificate isn't present, but a browser still prompts for an MTLS cert. Fix that by explicitly setting verify_none if strict t
Explicitly set verify_none
As reported, there are cases where a valid certificate isn't present, but a browser still prompts for an MTLS cert. Fix that by explicitly setting verify_none if strict tls isn't enabled. Unclear what impacts this will have elsewhere:
Tested (not yet done on this patch): with a self-signed certificate, logging into chrome no longer prompts the certificate screen.
Change-Id: Iaf7d25fec15ad547a6c741c9410995e19ba22016 Signed-off-by: Ed Tanous <etanous@nvidia.com>
show more ...
|
a0969c70 | 19-Sep-2024 |
Myung Bae <myungbae@us.ibm.com> |
Remove subscription for TerminateAfterRetries policy
Redfish Data Model [1] section 6.42.5.2 and EventDestination schema [2] specify that the subscription is terminated under the following policy an
Remove subscription for TerminateAfterRetries policy
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>
show more ...
|
116370d8 | 08-Oct-2024 |
Ed Tanous <etanous@nvidia.com> |
Break out lambdas in http server
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
Break out lambdas in http server
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>
show more ...
|
4b712a29 | 02-Aug-2023 |
Ed Tanous <edtanous@google.com> |
Move UserSubscription to composition
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 pron
Move UserSubscription to composition
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>
show more ...
|
41868c66 | 09-Oct-2024 |
Myung Bae <myungbae@us.ibm.com> |
Reformat with Never-AlignTrailingComments style
clang-format currently formats the codes to align the trailing comments of the consecutive lines via `AlignTrailingComments/Kind` as `Always` in `.cla
Reformat with Never-AlignTrailingComments style
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>
show more ...
|
d51c61b4 | 13-Sep-2024 |
Myung Bae <myungbae@us.ibm.com> |
Fix status for non-existent JsonSchema FileGet
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
Fix status for non-existent JsonSchema FileGet
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>
show more ...
|
6d799e14 | 11-Sep-2024 |
Ed Tanous <etanous@nvidia.com> |
Rename sendEvent
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 dif
Rename sendEvent
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>
show more ...
|
ad6dd39b | 12-Sep-2024 |
Lei YU <yulei.sh@bytedance.com> |
websocket: Handle eof and truncated stream
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 co
websocket: Handle eof and truncated stream
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
show more ...
|
6be832e2 | 10-Sep-2024 |
Ed Tanous <etanous@nvidia.com> |
Remove duplicated block comments
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 c
Remove duplicated block comments
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>
show more ...
|
4ff0f1f4 | 04-Sep-2024 |
Ed Tanous <etanous@nvidia.com> |
static -> inline
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: Icfc2b72d94b41a3a880da1
static -> inline
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>
show more ...
|
41fe81c2 | 02-Sep-2024 |
Ed Tanous <etanous@nvidia.com> |
Fix includes
This commit is automatically generated by enabling clang-include-fixer.
Tested: Code compiles.
Change-Id: I475d7b9d43e95bbdeeaadf11905d3b2a60aa8ef3 Signed-off-by: Ed Tanous <etanous@n
Fix includes
This commit is automatically generated by enabling clang-include-fixer.
Tested: Code compiles.
Change-Id: I475d7b9d43e95bbdeeaadf11905d3b2a60aa8ef3 Signed-off-by: Ed Tanous <etanous@nvidia.com>
show more ...
|
50bfc917 | 29-Jul-2024 |
Ed Tanous <etanous@nvidia.com> |
Fix redfish response for unknown method
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 resp
Fix redfish response for unknown method
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>
show more ...
|
7a16ddc8 | 25-Aug-2024 |
Ed Tanous <etanous@nvidia.com> |
Clang-format-18
These files were checked in during the clang-18 merge. Update them.
Change-Id: I857a87dac29469a4c24e83c6ee8b7c8461002f04 Signed-off-by: Ed Tanous <etanous@nvidia.com> |
bd1299b7 | 12-Aug-2024 |
Aushim Nagarkatti <anagarkatti@nvidia.com> |
Enable bmcweb dynamic logging
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 th
Enable bmcweb dynamic logging
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>
show more ...
|
cd504a94 | 19-Aug-2024 |
Ed Tanous <etanous@nvidia.com> |
Partial revert of http_client
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 b
Partial revert of http_client
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>
show more ...
|
bd79bce8 | 16-Aug-2024 |
Patrick Williams <patrick@stwcx.xyz> |
clang-format: re-format for clang-18
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 forma
clang-format: re-format for clang-18
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>
show more ...
|
608fb7b7 | 09-Aug-2024 |
Potin Lai <potin.lai@quantatw.com> |
http_body: Implement static bmcweb::HttpBody::size() function
In boost::beast::http::message::prepare_payload(), if HttpBody does not implement a size() function, it defaults to calling chunked(true
http_body: Implement static bmcweb::HttpBody::size() function
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
show more ...
|
4f467963 | 06-Aug-2024 |
Ed Tanous <etanous@nvidia.com> |
Add case default
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@nv
Add case default
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>
show more ...
|
ea5e2242 | 06-Aug-2024 |
Ed Tanous <etanous@nvidia.com> |
Fix overflow
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 hav
Fix overflow
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>
show more ...
|
724985ff | 05-Jun-2024 |
Ed Tanous <ed@tanous.net> |
Break out SSL key handler into a compile unit
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 boo
Break out SSL key handler into a compile unit
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>
show more ...
|
f80a87f2 | 16-Jun-2024 |
Ed Tanous <etanous@nvidia.com> |
Add SSE filter param support
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
Add SSE filter param support
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>
show more ...
|
19bb362b | 05-Jul-2024 |
Ed Tanous <etanous@nvidia.com> |
EventDestination: Implement VerifyCertificate
VerifyCertificate is a property on the Redfish EventDestination schema. It specifies that this property is: ``` An indication of whether the service wil
EventDestination: Implement VerifyCertificate
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>
show more ...
|
f51d8635 | 16-May-2024 |
Ed Tanous <ed@tanous.net> |
Break out DuplicatableFileHandle
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 thi
Break out DuplicatableFileHandle
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>
show more ...
|
3281bcf1 | 25-Jun-2024 |
Ed Tanous <ed@tanous.net> |
Support RespondToUnauthenticatedClients PATCH
RespondToUnauthenticatedClients allows users to explicitly select mTLS as their only authentication mechanism, thus significantly reducing their code ex
Support RespondToUnauthenticatedClients PATCH
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>
show more ...
|
89cda63d | 16-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Store Request Fields that are needed later
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 t
Store Request Fields that are needed later
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>
show more ...
|