| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
This is yet another step in parsing HTTP requests.
Tested:
'''
curl -vvvv -k --user "root:0penBmc" -H "Content-Type: application/json" \
-X POST https://192.168.7.2/redfish/v1/UpdateService/Actions/UpdateService.SimpleUpdate \
-d '{ \
"TransferProtocol":"TFTP", \
"ImageURI":"https://192.168.7.1/myfile.bin" \
}'
'''
Returns ActionParameterNotSupported
TransferProtocol: Omitted
ImageURI: https://192.168.7.1/myfile.bin
Returns ActionParameterNotSupported
TransferProtocol: Omitted
ImageURI: 192.168.7.1/myfile.bin
Returns ActionParameterValueTypeError
TransferProtocol: Bad
ImageURI: https:/192.168.7.1/myfile.bin
Returns: ActionParameterNotSupported
No changes to GET requests, so Redfish Service Validator not necessary.
Change-Id: Ibf4b69877031f3b8617412c06d40f2d0d0827ac3
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
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>
|
|
Boost process v2 brings some significant benefits to our launching of
processes[1]. In bmcweb terms:
1. The code is radically simpler, which decreaeses compile times, and
reduces the scope for code scanning tools.
2. The code now uses standard asio pipes instead of inventing its own.
3. Separate compilation.
Tested:
We don't have a lot of unit tests for the virtual media stuff that I can
run, but we do have unit tests for credentials pipe, which in this
change have been ported over, so the feature works. Unit tests are
passing.
[1] https://www.boost.org/doc/libs/1_80_0/doc/html/boost_process/v2.html#boost_process.v2.introduction
Change-Id: Ia20226819d75ff6e492f8852185f0b73e8f5cf83
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This TODO has been in bmcweb for a very long time. Implement it.
W3 sets rules for what security policies apply to which content
types[1]. Reading through this, essentially CSP should only apply to
HTML files.
Tested: Unit tests pass. Webui loads properly. Chrome network window
Shows headers show up as expected.
[1] https://www.w3.org/TR/CSP2/#which-policy-applies
Change-Id: I5467d0373832668763c72a66da2a8872e07bfb58
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
This feature was introduced to manage the operation sync at BMC while
multiple clients manage the BMC.
This feature scope has gone away and it is not a simple code to maintain
as per the growing standards of bmcweb.
This commit removes the feature from this repo.
Tested by: Locks routes are not available anymore
Change-Id: I257225cfb1f43d7d5dadb21a28a2ee5345c5112a
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This function in the next patch will be used for more than just TFTP, so
rename it to match intent, and refactor to use non-TFTP specific types.
Tested: Rename only. Need help on TFTP setups if we need it.
Change-Id: Ifc7485aa60ec53407c38b3d1bec530bdacf50075
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
If we use string_view with std::from_chars, we can use begin() and end()
directly (because they return pointers) and not have to do silly things
like dereference end(), which, while works in practice, is technically
undefined behavior, and some static analyzers complain about it.
Tested: Unit tests pass against both old parsePostCode and new.
Change-Id: Icfdec3b81f4a9c9bed3599571a8bc8779f9bfb98
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Tested: Code compiles.
Change-Id: Ifb254b703b6193a1faf68a54ad823dc807f92514
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Now that we only support string types in the router we no longer need to
build a "Tag" to be used for constructing argument types. Now, we can
just track the number of arguments, which simplifies the code
significantly, and removes the need to convert to and from the tag to
parameter counts.
This in turn deletes a lot of code in the router, removing the need for
tracking tag types.
Tested: Redfish service validator passes. Unit tests pass.
Change-Id: Ide1d665dc1984552681e8c05952b38073d5e32dd
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
In Redfish schema, just about all values can be a type (string,
EDM.Numeric, etc) or null. Most APIs don't allow explicitly setting
null, but there are a few cases where it is useful, namely in lists,
where an an empty object {} keeps the value the same, and null deletes
the value from the list.
Previously we handled this by unpacking as nlohmann::json, but this
allowed things like
[1.0, {}] to pass the check for an array of string values. We'd
ideally like to reject the 1.0 at the first stage, as well as reduce
the number of tiered readJson calls that we make.
This commit introducess support for unpacking std::variant types, that
allows unpacking a known type, or explicitly allowing null, by unpacking
std::nullptr_t.
Tested: Unit tests pass.
Change-Id: Ic7451877c824ac743faf1951cc2b5d9f8df8019c
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
TemporaryFileHandle class is used to create temp files in the
filesystem, and hold a handle to them until the class goes out of scope,
at which time they can be removed. It replaces makeFile(), which was
not RAII safe if an exception gets thrown, and could potentially leave
files in the filesystem if the tests fail.
Tested: Unit tests pass
Change-Id: I03eb0d342a6cd7b78115a8c42be9175f30c4ccd0
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Several places access the members of `req` indirectly like
`req.req.method()`. This can be simplified as `req.method()` .
This would also make the code clearer.
Tested:
- Compiles
- Redfish service validator passes
Change-Id: Ie129564ff907cdea7ac224b1e3d80cc0dedfbd7b
Signed-off-by: Myung Bae <myungbae@us.ibm.com>
|
|
Change-Id: I147664c3d181ba8ec535c7cddcb5c714e05616ea
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
These tests are commented out, and have been for a very long time.
Clearly they don't matter.
Change-Id: I084378ee9bc43bb64bd6e134398bbf2173d263ff
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
There are currently 78 sdbusplus::asio::setProperty calls in
redfish-core. The error handler for nearly all of them looks something
like:
```
if (ec)
{
const sd_bus_error* dbusError = msg.get_error();
if ((dbusError != nullptr) &&
(dbusError->name ==
std::string_view(
"xyz.openbmc_project.Common.Error.InvalidArgument")))
{
BMCWEB_LOG_WARNING("DBUS response error: {}", ec);
messages::propertyValueIncorrect(asyncResp->res, "<PropertyName>", <PropertyValue>);
return;
}
messages::internalError(asyncResp->res);
return;
}
messages::success(asyncResp->res);
```
In some cases there are more errors handled that translate to more error
messages, but the vast majority only handle InvalidArgument. Many of
these, like the ones in account_service.hpp, do the error handling in a
lambda, which causes readability problems. This commit starts to make
things more consistent, and easier for trivial property sets.
This commit invents a setDbusProperty method in the redfish namespace
that tries to handle all DBus errors in a consistent manner. Looking
for input on whether this will work before changing over the other 73
calls. Overall this is less code, fewer inline lambdas, and defaults
that should work for MOST use cases of calling an OpenBMC daemon, and
fall back to more generic errors when calling a "normal" dbus daemon.
As part of this, I've ported over several examples. Some things that
might be up in the air:
1. Do we always return 204 no_content on property sets? Today there's a
mix of 200, with a Base::Success message, and 204, with an empty body.
2. Do all DBus response codes map to the same error? A majority are
covered by xyz.openbmc_project.Common.Error.InvalidArgument, but there
are likely differences. If we allow any daemon to return any return
code, does that cause compatibility problems later?
Tested:
```
curl -k --user "root:0penBmc" -H "Content-Type: application/json" -X PATCH -d '{"HostName":"openbmc@#"}' https://192.168.7.2/redfish/v1/Managers/bmc/EthernetInterfaces/eth0
```
Returns the appropriate error in the response
Base.1.16.0.PropertyValueIncorrect
Change-Id: If033a1112ba516792c9386c997d090c8f9094f3a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
And fix the includes that are wrong.
Note, there is a very large ignore list included in the .clang-tidy
configcfile. These are things that clang-tidy doesn't yet handle
well, like knowing about a details include.
Change-Id: Ie3744f2c8cba68a8700b406449d6c2018a736952
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
These variables aren't used, and clang-tidy-18 flags it. Remove
Tested: Code compiles.
Change-Id: I414c4614a5f789aecab7700a4ec805e98c09cade
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
Redfish supports several type systems for json. This makes parsing into
proper types a challenge. Nlohmann supports 3 core data types,
nlohmann::json, which supports all json types (float, int, array,
object). Nlohmann::json::object_t, which is a specific typedef of
std::map, and nlohmann::json::array_t, which is a specific typedef of
std::map.
Redfish allows reading our arrays of complex objects, similar to
NtpServers: [null, {}, "string"]
Which makes it a challenge to support. This commit allows parsing out
objects as a nlohmann::object_t, which gives the ability to later use it
in a type safe manner, without having to call
get_ptr<nlohmann::json::object_t later>.
Tested:
Unit tests pass.
Change-Id: I4134338951ce27c2f56841a45b56bc64ad1753db
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
Commit [1] introduced a new optional dbus property that OpenBMC
developers can populate to define which
redfish/v1/Systems/system/ResetActionInfo AllowableValues are.
Look for that new property on dbus. If not found, hard code the
previous values otherwise utilize the property to fill in the return
value.
Tested:
- Put new property on dbus and confirmed Redfish API returned expected
values:
```
curl -k -H "X-Auth-Token: $token" -X GET https://${bmc}/redfish/v1/Systems/system/ResetActionInfo
{
"@odata.id": "/redfish/v1/Systems/system/ResetActionInfo",
"@odata.type": "#ActionInfo.v1_1_2.ActionInfo",
"Id": "ResetActionInfo",
"Name": "Reset Action Info",
"Parameters": [
{
"AllowableValues": [
"ForceOff",
"PowerCycle",
"Nmi",
"On",
"ForceOn",
"ForceRestart",
"GracefulRestart",
"GracefulShutdown"
],
"DataType": "String",
"Name": "ResetType",
"Required": true
}
]
}
```
- Did not run redfish validator as response was same as previous
[1]: https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/68933
Change-Id: Iecece14e7ff55db98d96df71b106ecc9e3f0ac33
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
Add support to fetch MemoryStatistics, FreeStorageSpaceKiB and
ProcessorStatistics for Manager Diagnostic Data.
https://redfish.dmtf.org/schemas/v1/ManagerDiagnosticData.v1_2_1.json
This change is in relation to following design and D-Bus interface -
https://gerrit.openbmc.org/c/openbmc/docs/+/64917
https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/64914
Test:
Redfish query output -
{
"@odata.id": "/redfish/v1/Managers/bmc/ManagerDiagnosticData",
"@odata.type": "#ManagerDiagnosticData.v1_2_0.ManagerDiagnosticData",
"FreeStorageSpaceKiB": 3772,
"Id": "ManagerDiagnosticData",
"MemoryStatistics": {
"AvailableBytes": 354224066,
"BuffersAndCacheBytes": 78984633,
"SharedBytes": 11876066,
"TotalBytes": 425516000
},
"Name": "Manager Diagnostic Data",
"ProcessorStatistics": {
"KernelPercent": 13.0234,
"UserPercent": 5.7374
},
"ServiceRootUptimeSeconds": 2255.117
}
Redfish service validator passing -
Elapsed time: 0:03:12
metadataNamespaces: 3726
pass: 5133
passAction: 9
passGet: 205
passRedfishUri: 197
skipNoSchema: 3
skipOptional: 3492
warnDeprecated: 4
warningPresent: 7
Validation has succeeded.
Change-Id: I43758a993eb7f342cb9ac5f5574498b37261c2cc
Signed-off-by: Jagpal Singh Gill <paligill@gmail.com>
|
|
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>
|
|
File body needs some unit tests for managing the move constructors.
Tested: Unit tests pass.
Change-Id: Ia640aec75a6f3f85640a50f5dd492638871f9eca
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
Similar to transforms we've done elsewhere, we shouldn't be parsing
urls using std::string::find, regex, or anything else, as they don't
handle URL % encoding properly.
Change-Id: I48bb30c0c737c4df2ae73f40fc49c63bac5b658f
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
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>
|
|
/redfish/v1/Managers/bmc/LogServices/Journal/Entries gives the system
journal entries whose ID is based on the realtime timestmap. However,
the system realtime may go backward if the system time is changed either
manually or via NTP.
If that happens, those entries may not found via redfish GET as
`sd_journal_seek_realtime_usec()`[1] may not always work on the entries
which are not sorted in time-order.
This may cause the inconsistency between the content of
`/redfish/v1/Managers/bmc/LogServices/Journal/Entries/`
and /redfish/v1/Managers/bmc/LogServices/Journal/Entries/<bmc_journal_id>`.
For example,
```
sudo journalctl --vacuum-time=1s
<wait for a while to clear up journal>
date -s "<backward-time>"
date -s "<forward-time>"
```
Run redfish journal entries and get each entry id from the output
```
curl -k -X GET https://${bmc}/redfish/v1/Managers/bmc/LogServices/Journal/Entries > rj.out
```
Take some logEntry Id that its time going backward like
```
grep "@odata.id" rj.out
```
Run redfish query for each id, and some of them can't be successful.
```
% curl -k -X GET https://${bmc}/redfish/v1/Managers/bmc/LogServices/Journal/Entries/1701604800002075
{
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The requested resource of type LogEntry named '1701604800002075' was not found.",
"MessageArgs": [
"LogEntry",
"1701604800002075"
],
"MessageId": "Base.1.13.0.ResourceNotFound",
"MessageSeverity": "Critical",
"Resolution": "Provide a valid resource identifier and resubmit the request."
}
],
"code": "Base.1.13.0.ResourceNotFound",
"message": "The requested resource of type LogEntry named '1701604800002075' was not found."
}
}%
```
This can also be verified by checking the failure of Redfish Validator run
```
python3 RedfishServiceValidator.py --auth Session -i https://${bmc} -u admin -p 0penBmc0 --payload Tree /redfish/v1/Managers/bmc/LogServices/Journal/Entries
```
For example,
```
ERROR - Members: GET of resource at URI /redfish/v1/Managers/bmc/LogServices/Journal/Entries/1701604800002075 returned HTTP error. Check URI.
ERROR - Members: GET of resource at URI /redfish/v1/Managers/bmc/LogServices/Journal/Entries/1701604800065949 returned HTTP error. Check URI.
ERROR - Members: GET of resource at URI /redfish/v1/Managers/bmc/LogServices/Journal/Entries/1701607680003048 returned HTTP error. Check URI.
```
```
--Time goes backwrd
{
"@odata.id": "/redfish/v1/Managers/bmc/LogServices/Journal/Entries/1701604800002075",
"@odata.type": "#LogEntry.v1_9_0.LogEntry",
"Created": "2023-12-03T12:00:00+00:00",
"EntryType": "Oem",
"Id": "1701604800002075",
"Message": "systemd-resolved: Clock change detected. Flushing caches.",
"Name": "BMC Journal Entry",
"OemRecordFormat": "BMC Journal Entry",
"Severity": "OK"
},
...
{
"@odata.id": "/redfish/v1/Managers/bmc/LogServices/Journal/Entries/1701607680003048",
"@odata.type": "#LogEntry.v1_9_0.LogEntry",
"Created": "2023-12-03T12:48:00+00:00",
"EntryType": "Oem",
"Id": "1701607680003048",
"Message": "systemd-resolved: Clock change detected. Flushing caches.",
"Name": "BMC Journal Entry",
"OemRecordFormat": "BMC Journal Entry",
"Severity": "OK"
},
-- Time comes back to the previous moment
```
The solution is proposed to use <bootid> + <monototic timestamp> as the
redfish journal entry id instead of realtime timestamp.
Unlike realtime timestamp which may go backward, <monotonic timestamp>
is monotonically increasing.
Tested:
- Redfish Validator passes
- GET Journal Entry ID will be found even if its time goes backward.
[1] https://github.com/openbmc/bmcweb/blob/7164bc62dd26ec92b01985aaae97ecc48276dea5/redfish-core/lib/log_services.hpp#L2690
Change-Id: I83bfb1ed88c9cf036f594757aa4a00d2709dd196
Signed-off-by: Myung Bae <myungbae@us.ibm.com>
|
|
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>
|
|
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>
|
|
Mutual TLS paths were not tested. Add some unit tests.
Because CI doesn't actually compile dependent libraries with ASAN
enabled, and these tests call into openssl, we need to add a check for
if we're compiling with asan enabled.
Tested: unit tests pass.
Change-Id: I02dcb69708619cc00fffd840738c608db3ae8bdf
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
In boost 1.83.0, the boost::url maintainers deprecated the header only
usage of the library without warning. A discussion with the
maintainers[1] made it clear that they removed the abiliy on purpose,
and they're not going to add it back or add a deprecation strategy (they
did say they would update the documentation to actually match the
intent), and that from here on in we should be using the cmake boost
project to pull in the non-header-only boost libraries we use (which at
this point is ONLY boost url).
This commit updates to remove the usage of boost::urls::result typedef,
which was deprecated in this release (which causes a compile error) and
moves it to boost::system::result.
In addition, it updates our meson files to pull in the boost project as
a cmake dependency.
[1] https://cpplang.slack.com/archives/C01JR6C9C4U/p1696441238739129
Tested: Not yet.
Change-Id: Ia7adfc0348588915440687c3ab83a1de3e6b845a
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
This code is needlessly complicated for what it does. Even with the
intent, which is secure buffer cleanup, it's trivial to encase all this
into a single class that accepts the strings by rvalue reference, then
cleans them up afterward.
Doing this also cleans up a potential lifetime problem, where if the
unix socket returned immediately, it would've invalidated the buffers
that were being sent. It also moves to async_write, instead of
async_write_some. The former could in theory fail if the socket blocks
(unlikely in this scenario) but it's good to handle anyway.
Tested: Need some help here. There's no backend for this, so we might
just have to rely on inspection.
Change-Id: I9032d458f8eb7a0689bee575aae611641bacee26
Signed-off-by: Ed Tanous <edtanous@google.com>
|
