Fix includes

Clang-tidy misc-include-cleaner appears to now be enforcing
significantly more headers than previously. That is overall a good
thing, but forces us to fix some issues. This commit is

Fix includes

Clang-tidy misc-include-cleaner appears to now be enforcing
significantly more headers than previously. That is overall a good
thing, but forces us to fix some issues. This commit is largely just
taking the clang-recommended fixes and checking them in. Subsequent
patches will fix the more unique issues.

Note, that a number of new ignores are added into the .clang-tidy file.
These can be cleaned up over time as they're understood. The majority
are places where boost includes a impl/x.hpp and x.hpp, but expects you
to use the later. include-cleaner opts for the impl, but it isn't clear
why.

Change-Id: Id3fdd7ee6df6c33b2fd35626898523048dd51bfb
Signed-off-by: Ed Tanous <etanous@nvidia.com>
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Use SPDX identifiers

SPDX identifiers are simpler, and reduce the amount of cruft we have in
code files. They are recommended by linux foundation, and therefore we
should do as they allow.

This pa

Use SPDX identifiers

SPDX identifiers are simpler, and reduce the amount of cruft we have in
code files. They are recommended by linux foundation, and therefore we
should do as they allow.

This patchset does not intend to modify any intent on any existing
copyrights or licenses, only to standardize their inclusion.

[1] https://www.linuxfoundation.org/blog/blog/copyright-notices-in-open-source-software-projects

Change-Id: I935c7c0156caa78fc368c929cebd0f068031e830
Signed-off-by: Ed Tanous <etanous@nvidia.com>

show more ...

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

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

Change unauthorized message if no UI is installed

This "Unauthorized" message has been a constant source of confusion for
users that forget to install a UI. This commit updates the message to
be mo

Change unauthorized message if no UI is installed

This "Unauthorized" message has been a constant source of confusion for
users that forget to install a UI. This commit updates the message to
be more clear, and present users with some hints that they have
forgotten to install a webui if they expected the auth to succeed.

Tested: String change only. Code compiles.

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

show more ...

Remove urlEncode

All new uses should be using boost::urls::url now. This was the last
usage.

Tested: Logged into webui, and observed the correct URL behavior.
In browser window /foobar
Forwarded t

Remove urlEncode

All new uses should be using boost::urls::url now. This was the last
usage.

Tested: Logged into webui, and observed the correct URL behavior.
In browser window /foobar
Forwarded to /?next=/foobar#/login

Which is correct.

Note, this is different behavior slightly than before. It was found
that the URI precedence goes query string THEN fragment, rather than the
other way around that we had it. This was flagged when moving over to
boost url structures.

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

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

Add check for globals

We don't follow this cpp core guidelines rule well. This is something
that we should aspire to cleaning up in the future, but for the moment,
lets turn the rule on in clang-ti

Add check for globals

We don't follow this cpp core guidelines rule well. This is something
that we should aspire to cleaning up in the future, but for the moment,
lets turn the rule on in clang-tidy to stop the bleeding, add ignores
for the things that we know need some better abstractions, and work on
these over time.

Most of this commit is just adding NOLINTNEXTLINE exceptions for all of
our globals. There was one case in the sensor code where clang
correctly noted that those globals weren't actually const, which got
missed because of the use of auto.

Tested: CI should be good enough for this. Passes clang-tidy.

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

show more ...

Fix content-type return behavior for */*

An HTTP header of Accepts: */* throws a big wrench into our
implementation for a couple reasons. First, because it's the default in
a lot of commonly-used l

Fix content-type return behavior for */*

An HTTP header of Accepts: */* throws a big wrench into our
implementation for a couple reasons. First, because it's the default in
a lot of commonly-used libraries, and second, because clients use it
when they certainly don't mean what the specification says it should
mean "ie, I accept ANY type".

This commit tries to address some of that, by making an explicit option
for content-type="ANY" and pushes it to the individual callers to handle
explicitly as if it were yet another type. In most protocols, there's a
"most common" representation, so protocols are free to use that, or to
explicitly handle it, and require that the user be explicit.

Tested:
Redfish Protocol Validator no longer locks up. (TBD, getting bugs filed
with protocol validator for this missing Accepts header).

For ServiceRoot
GET /redfish/v1 Accepts: application/json - returns json
GET /redfish/v1 Accepts: */* - returns json
GET /redfish/v1 Accepts: text/html - returns html
GET /redfish/v1 no-accepts header - returns json

Redfish-service-validator passes.

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

show more ...

Improve content type

We have a number of specialized content-type functions for varying
levels of degree, and most of them rely on quite a few strings. This
commit changes them to consolidate on tw

Improve content type

We have a number of specialized content-type functions for varying
levels of degree, and most of them rely on quite a few strings. This
commit changes them to consolidate on two APIs.

isContentTypeSupported, which as the name implies, takes a single
content type, and returns a bool about whether or not that content type
is allowed.

getPreferedContentType, which takes an array of multiple options, and
fine the first one in the list that matches the clients expected string.

These two functions makes these functions more able to be reused in the
future, and don't require specialized entries for each possible type or
combination of types that we need to check for.

Tested: Unit tests passing. Pretty good coverage.

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

show more ...

Use enum overload for field setting

There are two overloads of addHeader, one that takes a string, and one
that takes a boost enum. For most common headers, boost contains a
string table with all o

Use enum overload for field setting

There are two overloads of addHeader, one that takes a string, and one
that takes a boost enum. For most common headers, boost contains a
string table with all of those entries anyway, so there's no point in
duplicating the strings, and ensures that we don't make trivial
mistakes, like capitalization or - versus underscore that aren't caught
at compile time.

Tested:
This saves a trivial amount (572 bytes) of compressed binary size.

curl --insecure -vvv --user root:0penBmc https://192.168.7.2/redfish/v1

returns < Content-Type: application/json

curl --insecure -vvv -H "Accept: text/html" --user root:0penBmc https://192.168.7.2/redfish/v1

Returns
< Content-Type: text/html;charset=UTF-8

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

show more ...

Rearrange forward_unauthorized

This file is kind of hard to read. Try to improve it.

This readability problem caused me to miss one of the cases and invert
it, and because there's 6 possible clien

Rearrange forward_unauthorized

This file is kind of hard to read. Try to improve it.

This readability problem caused me to miss one of the cases and invert
it, and because there's 6 possible clients/flows that need tested through
these, my testing didn't catch it originally.

Tested:
Redfish protocol validator now passes one more test for
www-authenticate. 18 failing test cases down to 12.

'''
curl -vvvv --insecure -H "Accepts: application/json" https://192.168.7.2/redfish/v1/SessionService/Sessions
'''

Now returns WWW-Authenticate when basic auth is enabled.

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

show more ...

Fix www-authenticate behavior

bmcweb is in a weird position where, on the one hand, we would like to
support Redfish to the specification, while also supporting a secure
webui. For better or worse,

Fix www-authenticate behavior

bmcweb is in a weird position where, on the one hand, we would like to
support Redfish to the specification, while also supporting a secure
webui. For better or worse, the webui can't currently use non-cookie
auth because of the impacts to things outside of Redfish like
websockets.

This has lead to some odd code in bmcweb that tries to "detect" whether
the browser is present, so we don't accidentally pop up the basic auth
window if a user happens to get logged out on an xhr request. Basic
auth in a browser actually causes CSRF vulnerabilities, as the browser
caches the credentials, so we don't want to make that auth method
available at all.

Previously, this detection was based on the presence of the user-agent
header, but in the years since this code was originally written, a
majority of implementations have moved to sending a user-agent by
default, which makes this check pretty much useless for its purpose. To
work around that, this patchset relies on the X-Requested-With header,
to determine if a json payload request was done by xhr. In theory, all
browsers will set this header when doing xhr requests, so this should
provide a "more correct" solution to this issue.

Background:
https://en.wikipedia.org/wiki/List_of_HTTP_header_fields
"X-Requested-With Mainly used to identify Ajax requests (most JavaScript
frameworks send this field with value of XMLHttpRequest)"

Tested:
curl -vvvv --insecure https://192.168.7.2/redfish/v1/SessionService/Sessions
Now returns a WWW-Authenticate header

Redfish-protocol-validator now passes 7 more tests from the
RESP_HEADERS_WWW_AUTHENTICATE category.

Launched webui-vue and logged in. Responses in network tab appear to
work, and data populates the page as expected.
Used curl to delete redfish session from store with
DELETE /redfish/v1/SessionService/Sessions/<SessionId>
Then clicked an element on the webui, page forwarded to login page as
expected.

Opened https://localhost:8000/redfish/v1/CertificateService in a
browser, and observed that page forwarded to the login page as it
should.

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

show more ...

Change ownership of boost::req to crow::req

req is being created later, in the connection life cycle. req was
holding many important values when it was passed to authenticate, so the
authenticate ca

Change ownership of boost::req to crow::req

req is being created later, in the connection life cycle. req was
holding many important values when it was passed to authenticate, so the
authenticate call had to be refactored to includes all the data req was
holding.

Also uses of req before handle have been changed to direct calls to
boot::parse

Tested:
Made a request that did not require authentication
$ curl -vvvv --insecure "https://192.168.7.2:18080/redfish/v1"
Got correct service root

Made a unauthenticated request (Chassis)
$ curl -c cjar -b cjar -k -H "Content-Type: application/json" -X GET https://192.168.7.2:18080/redfish/v1/Chassis
Unauthenticated

Made a log-in request
$ curl -c cjar -b cjar -k -H "Content-Type: application/json" -X POST https://192.168.7.2:18080/login -d "{\"data\": [ \"root\", \"0penBmc\" ] }"

Made (same) Chassis request
$ curl -c cjar -b cjar -k -H "Content-Type: application/json" -X GET https://192.168.7.2:18080/redfish/v1/Chassis

Tested the websockets using scripts/websocket_test.py
Websockets continued to work after this change.

Followed the mTLS instructions here https://github.com/openbmc/docs/blob/master/security/TLS-configuration.md
mTLS continues to work after this change.

Change-Id: I78f78063be0331be00b66349d5d184847add1708
Signed-off-by: John Edward Broadbent <jebr@google.com>

show more ...

Make code compile with clang-13

Clang-13 rightfully warns that the hasWebuiRoute variable isn't declared
as static. This commit resolves that, and adds the static keyword so it
can be used in multi

Make code compile with clang-13

Clang-13 rightfully warns that the hasWebuiRoute variable isn't declared
as static. This commit resolves that, and adds the static keyword so it
can be used in multiple compile units. It also adds the static keyword
to the privilege registry, and the inline keyword to many methods that
now need it.

clang-format is also updated to version 12 in parse_registies.py, as
that's what CI uses, and what most people have installed.

Tested:
Followed clang-tidy instructions in README.md
"bitbake bmcweb" step now succeeds.

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

show more ...

Fix infinite redirect when webui isn't installed

In the begining, bmcweb had its own webui checked in as source. Largely
conceived of clay, and built by someone that doesn't understand UI
developme

Fix infinite redirect when webui isn't installed

In the begining, bmcweb had its own webui checked in as source. Largely
conceived of clay, and built by someone that doesn't understand UI
development (me), it was eventually superceeded by phosphor-webui. When
we did that, we created a bug where bmcweb was expecting a UI to always
be installed, and when it wasn't resolved into an infinite recursive
redirect as it tried to find the login page.

This patchset fixes that, by adding a connection between the
authorization class, and the webassets class, for bmcweb to detect at
runtime whether or not the UI is installed, and change behavior in that
case.

Along the way, we got a circular #include, so some includes needed to be
rearranged slightly.

This patchset will change no behavior when the UI is installed. Login
failures will continue to redirect to /, to hit the login page.

If the UI is not installed, and there is no / route, BMCWEB will return
the plaintext UNAUTHORIZED if you attempt to open the webui from the
browser without having a webui installed and without having credentials.

Tested:
Launched in a build without webui-vue, and observed "UNAUTHORIZED" when
I connected through chrome.
Also launched in a build with webui-vue installed with:
IMAGE_INSTALL_append = "webui-vue"
And loaded the webui in chrome, and logged in successfully.

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

show more ...