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

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

MTLS Client: Enabling mtls support in http_client

http_client currently does not uses mtls client certificates. This is
a good feature for both authentication and authorization purpose.
It will help

MTLS Client: Enabling mtls support in http_client

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>

show more ...

Make cert generate for readonly directories

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 genera

Make cert generate for readonly directories

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>

show more ...

Bring consistency to config options

The configuration options that exist in bmcweb are an amalgimation of
CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms
and meson options usi

Bring consistency to config options

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>

show more ...

Change ssl stream implementations

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

Change ssl stream implementations

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>

show more ...

Clean up BMCWEB_ENABLE_SSL

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 dep

Clean up BMCWEB_ENABLE_SSL

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>

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

clang-format: copy latest and re-format

clang-format-17 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest

clang-format: copy latest and re-format

clang-format-17 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest .clang-format from the docs repository and reformat the
repository.

Change-Id: I2f9540cf0d545a2da4d6289fc87b754f684bc9a7
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

show more ...

Replace logging with std::format

std::format is a much more modern logging solution, and gives us a lot
more flexibility, and better compile times when doing logging.

Unfortunately, given its level

Replace logging with std::format

std::format is a much more modern logging solution, and gives us a lot
more flexibility, and better compile times when doing logging.

Unfortunately, given its level of compile time checks, it needs to be a
method, instead of the stream style logging we had before. This
requires a pretty substantial change. Fortunately, this change can be
largely automated, via the script included in this commit under
scripts/replace_logs.py. This is to aid people in moving their
patchsets over to the new form in the short period where old patches
will be based on the old logging. The intention is that this script
eventually goes away.

The old style logging (stream based) looked like.

BMCWEB_LOG_DEBUG << "Foo " << foo;

The new equivalent of the above would be:
BMCWEB_LOG_DEBUG("Foo {}", foo);

In the course of doing this, this also cleans up several ignored linter
errors, including macro usage, and array to pointer deconstruction.

Note, This patchset does remove the timestamp from the log message. In
practice, this was duplicated between journald and bmcweb, and there's
no need for both to exist.

One design decision of note is the addition of logPtr. Because the
compiler can't disambiguate between const char* and const MyThing*, it's
necessary to add an explicit cast to void*. This is identical to how
fmt handled it.

Tested: compiled with logging meson_option enabled, and launched bmcweb

Saw the usual logging, similar to what was present before:
```
[Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled
[Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800
[Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist
[Info src/webserver_main.cpp:59] Starting webserver on port 18080
[Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file.
[Info src/webserver_main.cpp:137] Start Hostname Monitor Service...
```
Signed-off-by: Ed Tanous <ed@tanous.net>

Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8

show more ...

Implement data pointer clang-tidy check

readability-container-data-pointer flags one error in our codebase, but
can definitely find issues in patchsets. Fix the one error (that came
from crow), and

Implement data pointer clang-tidy check

readability-container-data-pointer flags one error in our codebase, but
can definitely find issues in patchsets. Fix the one error (that came
from crow), and enable the check.

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

show more ...

Remove this->

this-> is redundant when operating within a class member scope. We
should aim to be consistent.

This change was done automatically with sed replace

Tested: Code compiles

Signed-off

Remove this->

this-> is redundant when operating within a class member scope. We
should aim to be consistent.

This change was done automatically with sed replace

Tested: Code compiles

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

show more ...

clang-format: copy latest and re-format

clang-format-16 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest

clang-format: copy latest and re-format

clang-format-16 has some backwards incompatible changes that require
additional settings for best compatibility and re-running the formatter.
Copy the latest .clang-format from the docs repository and reformat the
repository.

Change-Id: I75f89d2959b0f1338c20d72ad669fbdc1d720835
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>

show more ...

Take boost error_code by reference

By convention, we should be following boost here, and passing error_code
by reference, not by value. This makes our code consistent, and removes
the need for a co

Take boost error_code by reference

By convention, we should be following boost here, and passing error_code
by reference, not by value. This makes our code consistent, and removes
the need for a copy in some cases.

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

show more ...

Fix a boatload of #includes

Most of these missing includes were found by running clang-tidy on all
files, including headers. The existing scripts just run clang-tidy on
source files, which doesn't

Fix a boatload of #includes

Most of these missing includes were found by running clang-tidy on all
files, including headers. The existing scripts just run clang-tidy on
source files, which doesn't catch most of these.

Tested: Code compiles

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

show more ...

Drop boost::posix_time

Per the coding standard, if we can support what we need to do with
std variants of something, we should prefer that. This commit adds an
iso8160 to string method that support

Drop boost::posix_time

Per the coding standard, if we can support what we need to do with
std variants of something, we should prefer that. This commit adds an
iso8160 to string method that supports any arbitrary
std::chrono::duration object, which allows doing the full range of all
of our integer types, and reduces the complexity (and presumably compile
times) not pulling in a complex library.

Despite the heavy templating, this only appears to add 108 bytes of
compressed binary size to bmcweb. This is likely due to the decreased
complexity compared to the boost variant (that likely pulls in
boost::locale). (Ie 3 template instantiations of the simple one take
about the same binary space as 1 complex instantiation).

Tested:
Unit tests pass (pretty good coverage here)

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

show more ...

Fix shadowed variable issues

This patchset is the conclusion of a multi-year effort to try to fix
shadowed variable names. Variables seem to be shadowed all over, and in
most places they exist, the

Fix shadowed variable issues

This patchset is the conclusion of a multi-year effort to try to fix
shadowed variable names. Variables seem to be shadowed all over, and in
most places they exist, there's a "code smell" of things that aren't
doing what the author intended.

This commit attempts to clean up these in several ways by:
1. Renaming variables where appropriate.
2. Preferring to refer to member variables directly when operating
within a class
3. Rearranging code so that pass through variables are handled in the
calling scope, rather than passing them through.

These patterns are applied throughout the codebase, to the point where
-Wshadow can be enabled in meson.build.

Tested: Code compiles, unit tests pass. Still need to run redfish
service validator.

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

show more ...

http_server: remove unused variable

|useSsl| was not referenced anywhere else, so delete it.

Tested: code compiles.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ie6ba9c122ab0897254e

http_server: remove unused variable

|useSsl| was not referenced anywhere else, so delete it.

Tested: code compiles.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ie6ba9c122ab0897254ed3cb0e278d43e8b6a283c

show more ...

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels

Try to fix the lambda formatting issue

clang-tidy has a setting, LambdaBodyIndentation, which it says:
"For callback-heavy code, it may improve readability to have the
signature indented two levels and to use OuterScope."

bmcweb is very callback heavy code. Try to enable it and see if that
improves things. There are many cases where the length of a lambda call
will change, and reindent the entire lambda function. This is really
bad for code reviews, as it's difficult to see the lines changed. This
commit should resolve it. This does have the downside of reindenting a
lot of functions, which is unfortunate, but probably worth it in the
long run.

All changes except for the .clang-format file were made by the robot.

Tested: Code compiles, whitespace changes only.

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

show more ...

Don't rely on operator << for object logging

In the upcoming fmt patch, we remove the use of streams, and a number of
our logging statements are relying on them. This commit changes them to
no long

Don't rely on operator << for object logging

In the upcoming fmt patch, we remove the use of streams, and a number of
our logging statements are relying on them. This commit changes them to
no longer rely on operator>> or operator+ to build their strings. This
alone isn't very useful, but in the context of the next patch makes the
automation able to do a complete conversion of all log statements
automatically.

Tested: enabled logging on local and saw log statements print to console

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

show more ...

Deduplicate doAccept code

doAccept does essentially the same code in two ways.
boost::beast::lowest_layer is used elsewhere to deduplicate this code.
Use it here as well.

Tested:
curl -vvvv --insec

Deduplicate doAccept code

doAccept does essentially the same code in two ways.
boost::beast::lowest_layer is used elsewhere to deduplicate this code.
Use it here as well.

Tested:
curl -vvvv --insecure -u root:0penBmc "https://192.168.7.2:443/redfish/v1"
succeeds.

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

show more ...

Make timer system use boost

The original crow timeout system had a timer queue setup for handling
many thousands of connections at a time efficiently. The most common
use cases for the bmc involve

Make timer system use boost

The original crow timeout system had a timer queue setup for handling
many thousands of connections at a time efficiently. The most common
use cases for the bmc involve a handful of connections, so this code
doesn't help us much.

These days, boost asio also implements a very similar timer queue
https://www.boost.org/doc/libs/1_72_0/boost/asio/detail/timer_queue.hpp
internally, so the only thing we're loosing here is the "fuzzy"
coalescing of timeout actions, for which it's tough to say if anyone
will even notice.

This commit implements a timer system that's self contained within each
connection, using steady_timer. This is much more "normal" and how most
of the beast examples implement timers.

Tested:
Minimal touch testing to ensure that things work, but more testing is
required, probably using sloworis to ensure that our timeouts are no
longer issues.

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

show more ...

Fix .clang-tidy

camelLower is not a type, camelBack is.

Changes were made automatically with clang-tidy --fix-errors

To be able to apply changes automatically, the only way I've found that
works w

Fix .clang-tidy

camelLower is not a type, camelBack is.

Changes were made automatically with clang-tidy --fix-errors

To be able to apply changes automatically, the only way I've found that
works was to build the version of clang/clang-tidy that yocto has, and
run the fix script within bitbake -c devshell bmcweb. Unfortunately,
yocto has clang-tidy 11, which can apparently find a couple extra errors
in tests we already had enabled. As such, a couple of those are also
included.

Tested:
Ran clang-tidy-11 and got a clean result.

Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I9d1080b67f0342229c2f267160849445c065ca51

show more ...

fix include names

cppcheck isn't smart enough to recognize these are c++ headers, not c
headers. Considering we're already inconsistent about our naming, it's
easier to just be consistent, and move

fix include names

cppcheck isn't smart enough to recognize these are c++ headers, not c
headers. Considering we're already inconsistent about our naming, it's
easier to just be consistent, and move the last few files to use .hpp
instead of .h.

Tested:
Code builds, no changes.

Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Ic348d695f8527fa4a0ded53f433e1558c319db40

show more ...