Write unit tests for http2 connection

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: Uni

Write unit tests for http2 connection

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>

show more ...

Add unit tests for file body

File body needs some unit tests for managing the move constructors.

Tested: Unit tests pass.

Change-Id: Ia640aec75a6f3f85640a50f5dd492638871f9eca
Signed-off-by: Ed Tan

Add unit tests for file body

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>

show more ...

Simplify body

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

Simplify body

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>

show more ...

Make tests not require body interaction

The muitipart test interacts with some significant details of the
response class. This was largely only done because Request lacked an
addHeader method that

Make tests not require body interaction

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>

show more ...

Make use of filebody for dump offload

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 r

Make use of filebody for dump offload

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>

show more ...

Make base64 encoder incremental

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 r

Make base64 encoder incremental

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>

show more ...

Remove some boost includes

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::alg

Remove some boost includes

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>

show more ...

Clean up tftp update to use URL

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 properl

Clean up tftp update to use URL

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>

show more ...

Fix spelling mistakes

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: Iccb57b2adfd06a2

Fix spelling mistakes

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>

show more ...

Use MonotonicTimeStamp for bmc log id

/redfish/v1/Managers/bmc/LogServices/Journal/Entries gives the system
journal entries whose ID is based on the realtime timestmap. However,
the system realtime

Use MonotonicTimeStamp for bmc log id

/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>

show more ...

mutual-tls: Add support for Meta certificates

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 =

mutual-tls: Add support for Meta certificates

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>

show more ...

Simplify mutual TLS checks

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

Simplify mutual TLS checks

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>

show more ...

Add mutual tls unit test

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 nee

Add mutual tls unit test

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>

show more ...

Reponse: Fix incomplete implementation in write

Write function in http_response.hpp missing implementation for first
write after changing from filebody.
Usecase:
Current resonse type is filebody. De

Reponse: Fix incomplete implementation in write

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>

show more ...

Fix missing date

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

Fix missing date

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>

show more ...

Unit test Connection

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 som

Unit test Connection

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>

show more ...

Move to file_body in boost

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

Move to file_body in boost

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>

show more ...

Update to boost 1.83.0

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 remo

Update to boost 1.83.0

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>

show more ...

Clean up vm CredentialPipe

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 accep

Clean up vm CredentialPipe

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>

show more ...

Simplify datetime parsing

This code as it stands pulls in the full datetime library from boost,
including io, and a bunch of timezone code. The bmc doesn't make use of
any of this, so we can rely o

Simplify datetime parsing

This code as it stands pulls in the full datetime library from boost,
including io, and a bunch of timezone code. The bmc doesn't make use of
any of this, so we can rely on a much simplified version.

Unfortunately for us, gcc still doesn't implement the c++20
std::chrono::parse[1]. There is a reference library available from [2]
that backports the parse function to compilers that don't yet support
it, and is the basis for the libc++ version. This commit opts to copy
in the header as-written, under the assumption that we will never need
to pull in new versions of this library, and will move to the std
ersion as soon as it's available in the next gcc version.

This commit simplifies things down to improve compile times and binary
size. It saves ~22KB of compressed binary size, or about 3%.

Tested: Unit tests pass. Pretty good coverage.

[1] https://en.cppreference.com/w/cpp/chrono/parse
[2] https://github.com/HowardHinnant/date/blob/master/include/date/date.h

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I706b91cc3d9df3f32068125bc47ff0c374eb8d87

show more ...

Move http client to URL

Type safety is a good thing. In:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/65606

It was found that splitting out the URI into encoded pieces in the early
phase removed

Move http client to URL

Type safety is a good thing. In:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/65606

It was found that splitting out the URI into encoded pieces in the early
phase removed some information we needed, namely whether or not a URI
was ipv6. This commit changes http client such that it passes all the
information through, with the correct type, rather than passing in
hostname, port, path, and ssl separately.

Opportunistically, because a number of log lines are changing, this uses
the opportunity to remove a number of calls to std::to_string, and rely
on std::format instead.

Now that we no longer use custom URI splitting code, the
ValidateAndSplitUrl() method can be removed, given that our validation
now happens in the URI class.

Tested: Aggregation works properly when satellite URIs are queried.

Change-Id: I9f605863179af54c5af2719bc5ce9d29cbfffab7
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...

Fix bugprone-unchecked-optional-access findings

Clang-tidy has the aforementioned check, which shows a few places in the
core where we ignored the required optional checks. Fix all uses.
Note, we c

Fix bugprone-unchecked-optional-access findings

Clang-tidy has the aforementioned check, which shows a few places in the
core where we ignored the required optional checks. Fix all uses.
Note, we cannot enable the check that this time because of some weird
code in health.hpp that crashes tidy[1]. That will need to be a future
improvement.

There are tests that call something like
ASSERT(optional)
EXPECT(optional->foo())

While this isn't an actual violation, clang-tidy doesn't seem to be
smart enough to deal with it, so add some explicit checks.

[1] https://github.com/llvm/llvm-project/issues/55530

Tested: Redfish service validator passes.

Change-Id: Ied579cd0b957efc81aff5d5d1091a740a7a2d7e3
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...

AsyncResolve cleanups and error handling

The Async DBus resolver really has nothing to do with crow, which is our
core http library namespace and has some opportunistic cleanups that can
be done.

T

AsyncResolve cleanups and error handling

The Async DBus resolver really has nothing to do with crow, which is our
core http library namespace and has some opportunistic cleanups that can
be done.

This commit moves it into the bmcweb namespace (unimportantly) and
breaks out one of the larger functions such that it can be unit tested,
and unit tests it.

Tested: Unit tests pass.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie3cfbb0ef81a027a1ad42358c04967a517471117

show more ...

Use openssl random number generator

We already have a generator class. We should use it. Wrap this into a
function that can be unit tested, and add unit tests.

Note, some files also needed to cha

Use openssl random number generator

We already have a generator class. We should use it. Wrap this into a
function that can be unit tested, and add unit tests.

Note, some files also needed to change name, because random.hpp
conflicts with the built in random, and causes circular build problems.
This commit changes it to ossl_random.

Tested: Unit tests pass. Now has coverage.

Redfish service validator passes.

Change-Id: I5f8eee1af5f4843a352c6fd0e26d67fd3320ef53
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...

Remove the black_magic namespace

The black_magic namespace has been eradicated of what most would call
"black magic" and while there's some non-trivial stuff in there, it's
far from the most complic

Remove the black_magic namespace

The black_magic namespace has been eradicated of what most would call
"black magic" and while there's some non-trivial stuff in there, it's
far from the most complicated part of this stack.

This commit takes the two remaining things in the black_magic namespace,
namely the parameter tagging functionality, and moves them into the
utility namespace.

Tested: Redfish service validator passes

Change-Id: I9e2686fff5ef498cafc4cb83d4d808ea849f7737
Signed-off-by: Ed Tanous <edtanous@google.com>

show more ...