Simplify router

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, wh

Simplify router

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>

show more ...

Revert "Refactor after login"

This reverts commit cd40b060ee2df5469077a70d15590f86158f2c60.

Cookie based login is no longer functional with this patch. It looks
like we got a merge conflict that I

Revert "Refactor after login"

This reverts commit cd40b060ee2df5469077a70d15590f86158f2c60.

Cookie based login is no longer functional with this patch. It looks
like we got a merge conflict that I resolved incorrectly.

Tested: Webui can now log in.
Change-Id: I60b8aeae173b1838d8745a2c499fbcb410813ef3

show more ...

Refactor after login

Break out this method into a smaller section.

Tested: Redfish service validator passes

Change-Id: I0ca4e9ea14c505a1ed00dae4cba1285e4ac1f36d
Signed-off-by: Ed Tanous <edtanous@

Refactor after login

Break out this method into a smaller section.

Tested: Redfish service validator passes

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

show more ...

Remove phosphor-rest workarounds

Phosphor-rest is no longer supported by the project, and phosphor-webui,
which required some of these workarounds has been archived a year ago.
There's no reason to

Remove phosphor-rest workarounds

Phosphor-rest is no longer supported by the project, and phosphor-webui,
which required some of these workarounds has been archived a year ago.
There's no reason to keep this login type, given that it was
undocumented.

NOTE: Upon inspection, it looks like webui-vue used the same hack.
[1] https://github.com/openbmc/webui-vue/blob/43e3bd26133b06ed117a3a3f10b2bc09e2c2aafc/src/store/modules/Authentication/AuthenticanStore.js#L41

Tested:
Combined with https://gerrit.openbmc.org/c/openbmc/webui-vue/+/65811
Webui Login succceeds.

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

show more ...

Refactor login/logout

Similar to what we've done elsewhere, move login and logout into their
own methods. This reduces the amount of scopes that need to be read at
any given time.

Tested: At last

Refactor login/logout

Similar to what we've done elsewhere, move login and logout into their
own methods. This reduces the amount of scopes that need to be read at
any given time.

Tested: At last commit in series.

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

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

Update to owasp headers

Change the Cache-Control header to what owasp recommends.
Remove the X-XSS-Protection. This has been removed from Chrome, and is
unimplemented in other browsers[1].

Add:
X-

Update to owasp headers

Change the Cache-Control header to what owasp recommends.
Remove the X-XSS-Protection. This has been removed from Chrome, and is
unimplemented in other browsers[1].

Add:
X-Permitted-Cross-Domain-Policies
Clear-Site-Data
Cross-Origin-Embedder-Policy
Cross-Origin-Opener-Policy
Cross-Origin-Resource-Policy

And set them to the OWASP recommended values.

Tested: The OWASP Venom test suite now passes more tests.

[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-XSS-Protection

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

show more ...

Fix hack on Set-Cookie

This is one that I couldn't figure out for a while. Turns out that
fields has both a set() and an insert() method. Whereas set() replaces,
insert() appends, which is what we

Fix hack on Set-Cookie

This is one that I couldn't figure out for a while. Turns out that
fields has both a set() and an insert() method. Whereas set() replaces,
insert() appends, which is what we want in this case.

This allows us to call the actual methods several times, instead of
essentially string injecting our own code, which should make it clearer.

At the same time, there was one unit test that was structured such that
it was using addHeader to clear a header, so this commit adds an
explicit "clearHeader()" method, so we can be explicit.

Tested:
Logging into the webui in chrome (which uses POST /login) shows:
401 with no cookie header if the incorrect password is used
200 with 2 Set-Cookie headers set:
Set-Cookie:
SESSION=<session tag>; SameSite=Strict; Secure; HttpOnly
Set-Cookie:
XSRF-TOKEN=<token tag>; SameSite=Strict; Secure

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

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

Remove body member from Request

Per cpp core guidelines, these should be methods.

Tested: on last patchset of the series.

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

Remove body member from Request

Per cpp core guidelines, these should be methods.

Tested: on last patchset of the series.

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

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

Move ClientID parameter out of OEM

In 2022.2, Redfish added support for the Context parameter on the
Session Resource. This parameter has the same function that the
OemSession.ClientId field served

Move ClientID parameter out of OEM

In 2022.2, Redfish added support for the Context parameter on the
Session Resource. This parameter has the same function that the
OemSession.ClientId field served. This commit moves all the existing
ClientId code to produce Context as well.

Functionally, this has one important difference, in that Context in
Redfish is optionally provided by the user, which means we need to omit
it if not given by the user. The old implementation left it set to
empty string ("").

Because of this, a few minor interfaces need to change to use
std::optional. Existing uses of clientId are moved to using
value_or("") to keep the same behavior as before.

Tested:
curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\":
\"0penBmc\"}" https://192.168.7.2/redfish/v1/SessionService/Sessions

Returns a Session object with no Context key present

curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\":
\"0penBmc\", \"Context\": \"Foobar\"}"
https://192.168.7.2/redfish/v1/SessionService/Sessions

Returns a Session object with:
"Context": "Foobar"

Subsequent Gets of /redfish/v1/SessionService/Sessions/<sid>
return the same session objects, both with and without Context.

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

show more ...

Fixes invalid memory access

std::string_view causes invalid memory access in multipart branch when
assigned local variable goes out of scope and string_view is passed to
ramAuthenticateUser. Moved M

Fixes invalid memory access

std::string_view causes invalid memory access in multipart branch when
assigned local variable goes out of scope and string_view is passed to
ramAuthenticateUser. Moved MultipartParser to higher scope, to ensure
it is not deleted before std::string_view.

Tested:
- Executed post on /login, got response:
{
"data": "User 'root' logged in",
"message": "200 OK",
"status": "ok"
}

Change-Id: I0b02dddcb1a887d442525ffedb7a08a00087f2f2
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>

show more ...

Remove usages of boost::starts/ends_with

Per the coding standard, now that C++ supports std::string::starts_with
and std::string::ends_with, we should be using them over the boost
alternatives. Thi

Remove usages of boost::starts/ends_with

Per the coding standard, now that C++ supports std::string::starts_with
and std::string::ends_with, we should be using them over the boost
alternatives. This commit goes through and updates all usages.

Arguably some of these are incorrect, and instances of common error 13,
but because this is mostly a mechanical it intentionally doesn't try to
handle it.

Tested: Unit tests pass.

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

show more ...

On logout set Session cookie with expired date

The Session cookie is an HttpOnly cookie.
HttpOnly means the cookie cannot be accessed through client side script
because of this the GUI can not delet

On logout set Session cookie with expired date

The Session cookie is an HttpOnly cookie.
HttpOnly means the cookie cannot be accessed through client side script
because of this the GUI can not delete this cookie on log out.
Recommendation online was setting this cookie to an expired date.

From https://tools.ietf.org/search/rfc6265
"Finally, to remove a cookie, the server returns a Set-Cookie header
with an expiration date in the past. The server will be successful in
removing the cookie only if the Path and the Domain attribute in the
Set-Cookie header match the values used when the cookie was created."

For more information see
https://stackoverflow.com/questions/5285940/correct-way-to-delete-cookies-server-side

Modern browsers delete expired cookies although based on reading it
might not be right away but on the next request from that domain or
when the browser is cleaning up cookies.

When I tested the cookie is deleted right away.

Also set the SESSION to an empty string.

Discussed in discord here:
https://discord.com/channels/775381525260664832/855566794994221117/982351098998321163

Webui-vue and phosphor-webui both use this /logout route:
https://github.com/openbmc/webui-vue/blob/a5fefd0ad25753e5f7da03d77dfe7fe10255ebb6/src/store/modules/Authentication/AuthenticanStore.js#L50
https://github.com/openbmc/phosphor-webui/blob/339db9a4c8610c5ecb92993c0bbc2219933bc858/app/common/services/userModel.js#L46
It seemed unnecessary to add it to the SessionCollection Post.

Tested: No longer have the cookie after log out on webui-vue.
Tested on Firefox and Chrome.

Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
Change-Id: Ic12b6f628293a80c93ffbbe1bf06c9b2d6a53af7

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

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies.

Remove brace initialization of json objects

Brace initialization of json objects, while quite interesting from an
academic sense, are very difficult for people to grok, and lead to
inconsistencies. This patchset aims to remove a majority of them in
lieu of operator[]. Interestingly, this saves about 1% of the binary
size of bmcweb.

This also has an added benefit that as a design pattern, we're never
constructing a new object, then moving it into place, we're always
adding to the existing object, which in the future _could_ make things
like OEM schemas or properties easier, as there's no case where we're
completely replacing the response object.

Tested:
Ran redfish service validator. No new failures.

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

show more ...

Enable readability checks

clang-tidy readability checks are overall a good thing, and help us to
write consistent and readable code, even if it doesn't change the
result.

All changes done by the ro

Enable readability checks

clang-tidy readability checks are overall a good thing, and help us to
write consistent and readable code, even if it doesn't change the
result.

All changes done by the robot.

Tested: Code compiles, inspection only (changes made by robot)

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

show more ...

Add readability-redundant-* checks

There's a number of redundancies in our code that clang can sanitize
out. Fix the existing problems, and enable the checks.

Signed-off-by: Ed Tanous <edtanous@go

Add readability-redundant-* checks

There's a number of redundancies in our code that clang can sanitize
out. Fix the existing problems, and enable the checks.

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

show more ...

Enable readability-redundant-control-flow checks

These checks are a nice addition to our static analysis, as they
simplify code quite a bit, as can be seen by this diff being negative
lines.

Signed

Enable readability-redundant-control-flow checks

These checks are a nice addition to our static analysis, as they
simplify code quite a bit, as can be seen by this diff being negative
lines.

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

show more ...

Implement MIME parsing

This commit adds two core features to bmcweb:

1. A multipart mime parser that can read multipart form requests into
bmcweb. This is implemented as a gene

Implement MIME parsing

This commit adds two core features to bmcweb:

1. A multipart mime parser that can read multipart form requests into
bmcweb. This is implemented as a generic parser that identifies the
content-type strings and parses them into structures.

2. A /login route that can be logged into with a multipart form. This
is to allow changing the login screen to a purely forms based
implementation, thus removing the very large whitelist we currently have
to maintain, and removing javascript from our threat envelope.

More testing is still needed, as this is a parser that exists outside of
the secured areas, but in this simple example, it seems to work well.

Tested: curl -vvvvv --insecure -X POST -F 'username=root' -F
'password=0penBmc' https://<bmc ip address>:18080/login

Returned; { "data": "User 'root' logged in", "message": "200 OK",
"status": "ok" }

Change-Id: Icc3f4c082d584170b65b9e82f7876926cd38035d
Signed-off-by: Ed Tanous<ed@tanous.net>
Signed-off-by: George Liu <liuxiwei@inspur.com>

show more ...

Convert IPv4-mapped IPv6 ClientIP back to IPv4

Current HTTP server creates an IPv6 acceptor to accept both IPv4 and
IPv6 connections. In this way, IPv4 address will be presented as IPv6

Convert IPv4-mapped IPv6 ClientIP back to IPv4

Current HTTP server creates an IPv6 acceptor to accept both IPv4 and
IPv6 connections. In this way, IPv4 address will be presented as IPv6
address in IPv4-mapped format. This patch converts it back to IPv4.

Tested:
Verified the ClientOriginIP in Session is shown in native IPv4 format
instead of IPv4-mapped IPv6 format.

Change-Id: Icd51260b2d4572d52f5c670128b7f07f6b5e6912
Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>

show more ...

Using AsyncResp everywhere

Get the core using AsyncResp everywhere, and not have each individual handler
creating its own object.We can call app.handle() without fear of the response

Using AsyncResp everywhere

Get the core using AsyncResp everywhere, and not have each individual handler
creating its own object.We can call app.handle() without fear of the response
getting ended after the first tree is done populating.
Don't use res.end() anymore.

Tested:
1. Validator passed.

Signed-off-by: zhanghaicheng <zhanghch05@inspur.com>
Change-Id: I867367ce4a0caf8c4b3f4e07e06c11feed0782e8

show more ...

Add SameSite=Strict on Set-Cookie

Set SameSite to Strict since OpenBMC does not have functionality
that requires Lax or None.

SameSite Strict provides a little protection agains

Add SameSite=Strict on Set-Cookie

Set SameSite to Strict since OpenBMC does not have functionality
that requires Lax or None.

SameSite Strict provides a little protection against CSRF attacks
by ensuring the cookie is only sent to requests originating from
the same site that set the cookie.

This came from some discussion on discord.

From https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie

SameSite=<samesite-value> Optional
Controls whether a cookie is sent with cross-origin requests,
providing some protection against cross-site request forgery attacks

Inline options are:

Strict: The browser sends the cookie only for same-site requests
(that is, requests originating from the same site that set the
cookie). If the request originated from a different URL than the
current one, no cookies with the SameSite=Strict attribute are sent.

Lax: The cookie is not sent on cross-site requests, such as calls
to load images or frames, but is sent when a user is navigating to
the origin site from an external site (e.g. if following a link).
This is the default behavior if the SameSite attribute is not
specified.

None: The browser sends the cookie with both cross-site and
same-site requests. The Secure attribute must also be set when
SameSite=None!

Note: On Firefox 85, FireFox still doesn't have the Default set
to SameSite=Lax. This can be changed via "about:config" and
"network.cookie.sameSite.laxByDefault".

Tested: Webui-vue works. Redfish GUI browser works.
Websockets work on the GUI.
Tested GUI functions that call POST and PATCH.
Can see the XSRF-TOKEN and SESSION cookies are SameSite
Strict with this build. Before were SameSite None.
Browser DevTools -> Storage on Firefox to view cookies.
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

Change-Id: I4402f2930847c1d47b22696631be26d33c78b6f9

show more ...

Redfish Session : Fix clientIp getting mapped to clientId

When the session is created using /login, the ClientOriginIPAddress
is mapped to the clientId parameter which displayed the clie

Redfish Session : Fix clientIp getting mapped to clientId

When the session is created using /login, the ClientOriginIPAddress
is mapped to the clientId parameter which displayed the clientIP
instead of the of clientId.
The similar problem is observed with auth methods other than sessions
created using the SessionService resource

This commit swaps the clientId and clientIp parameters passed to
generateUserSession API, so that the optional clientId is
passed as the last parameter

Tested by :
1. Create session using Redfish command
POST https://${bmc}/login -d '{"username": <>,"password": <>}'
POST https://${bmc}/redfish/v1/SessionService/Sessions
-d '{"username": <>,"password": <>}'
2. Open the GUI session to check the clientId is not displaying the
ClientOriginIPAddress

Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: I6cee3de963c489e690d2ad0bb09ba78dca39e4f9

show more ...