Age | Commit message (Collapse) | Author | Files | Lines |
|
The method of creating a random ID from an openssl random generator of a
particular length is something that is generally useful, and something
we can write unit tests for. Add it.
Tested:
Redfish service validator login flows work correctly in redfish service
validator.
Change-Id: Ic3b58d33f1421f3eb39e2d57585958f87f6fb8ea
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
When a user's password is changed, existing Redfish sessions for that
user, created with the old password, continue to work.
As per OWASP session management, "The session ID must be renewed or
regenerated by the web application after any privilege level change
within the associated user session... Common scenarios to consider
include; password changes, permission changes, or switching from a
regular user role to an administrator role within the web application."
[1] https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html
This commit removes existing user sessions when the user's password is
changed. This commit leaves the current session in place though a new
removeSessionsByUsernameExceptSession().
This commit doesn't completely get us fully to what owasp says but is a
start.
Tested:
Create some users:
```
curl -k -v -X POST -H "Content-Type: application/json" \
https://$bmc/redfish/v1/AccountService/Accounts/ -d \
'{"UserName":"testadminuser","Password":"<password>","RoleId":"Administrator","Enabled":true}'
```
Using basic auth was able to update own password and another user's
password.
Using token auth, verified the current session did not get deleted but
other sessions from that user did.
```
curl -k -H "Content-Type: application/json" -X POST -D headers.txt \
https://${bmc}/redfish/v1/SessionService/Sessions -d \
'{"UserName":"testadminuser", "Password":"<password>"}'
```
```
curl -k -v -X PATCH -H "X-Auth-Token: $token" \
-H "Content-Type:application/json" \
https://$bmc/redfish/v1/AccountService/Accounts/testadminuser \
-d '{"Password":"<password>"}'
```
Verified when changing another user's password all sessions were
dropped.
Change-Id: I4de60b84964a6b29c021dc3a2bece9ed4bc09eac
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
|
|
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>
|
|
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>
|
|
clang-tidy-18 must've fixed their checking for these in headers.
Resolve as the robot commands.
Tested: Noop changes made by tidy. Code compiles.
Change-Id: I1de7686c597deffb0df91c30dae1a29f9ba7900e
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This reverts commit 8ed41c35a314580bb794fa0fff2e01b0bf7efcf7.
In discord, it was posted 2 systems are hitting 403 Forbidden for all
endpoints.
Reverting fixed the problem, until time is given to dive into this,
just revert.
One of the things wrong is this is missing an After/Want
xyz.openbmc_project.User.Manager.service.
Change-Id: I1766a6ec2dbc9fb52da3940b07ac002a1a6d269a
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
There is an async call within the router that leads to a small, but
pervasive performance issue for all queries. Removing that call from the
router has the potential to increase the performance of every
authenticated query, and significantly reduce our dbus traffic for
"simple" operations.
This commit re-implements the role cache in session object that existed
previously many years ago. Each users role is fetched during
authentication and persisted in session object. Each successive request
can then be matched against the privilege which is there in the
in-memory session object.
This was discussed on below commit
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/39756
Tested by:
```
POST /redfish/v1/SessionService/Sessions {"UserName":"root", "Password": “0penBmc”}
```
Followed by redfish queries
Get /redfish/v1/AccountService
Tested user role persistency
Redfish service validator passes.
Signed-off-by: Ravi Teja <raviteja28031990@gmail.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I575599c29358e32849446ce6ee7f62c8eb3885f6
|
|
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
|
|
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>
|
|
This reverts commit e628df8658c57f6943b6d3612e1077618e5a168a.
This appears to cause problems with non-cookie login of the console
websocket. This appears to be a gap in both our testing, and things
that we have scripting to do, but clearly it's a change in behavior, so
if we want to change the behavior, we should do it intentionally, and
clearly, ideally with a path to make clients work, or an explicit
documentation that the webui is the only supported client.
Change-Id: I334257e1355a5b8431cb7ecfe58ef8a942f4981c
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
The new pre-defined usergroup named "hostconsole" is added to
differentiate access between host console and manager console.
The only users allowed to interact with host console are part of the
"hostconsole" group and they are in an administrator role.
Note: The changes are spread across multiple repositories listed under
"Related commits:"
The bmcweb changes to incorporate new group are as follows:
- The new user is added in the hostconsole group only if it has an
administrative role.
- The ssh usergroup is only translated to ManagerConsole redfish group
and hostconsole usergroup is translated to HostConsole redfish group.
- The following changes are made to check the privileges for host console
access
- The new OEM privilege "OpenBMCHostConsole" added for host console
access. This privilege is not shared externally hence it is not
documented.
- Updated obmc_console BMCWEB_ROUTE to use the new privilege.
- Router functions now save user role and user groups in the session
- getUserPrivileges() function now takes session reference instead
of user role. This function now also checks for the user group
"hostconsole" and add the new privilege if user is member of this
group.
- Updated all callers of the getUserPrivileges to pass session
reference.
- Added test to validate that new privilege is set correctly.
Tested:
Loaded code on the system and validated that;
- New user gets added in hostconsole group. NOTE: Prior to this commit
all groups are assigned to new user. This drop does not change that
behavior.
- Access from the web gui is only available for users in hostconsole
group. Used IBM internal simulator called simics to test this. This
simulator allows accessing openbmc from GUI.
- Checked the role collection and there is no change.
$ curl -k -H "X-Auth-Token: $TOKEN" -X GET \
https://${bmc}/redfish/v1/AccountService/Roles
$ curl -k -H "X-Auth-Token: $TOKEN" -X GET \
https://${bmc}/redfish/v1/AccountService/Roles/Administrator
$ curl -k -H "X-Auth-Token: $TOKEN" -X GET \
https://${bmc}/redfish/v1/AccountService/Roles/ReadOnly
$ curl -k -H "X-Auth-Token: $TOKEN" -X GET \
https://${bmc}/redfish/v1/AccountService/Roles/Operator
- HostConsole is in AccountType when hostconsole group is present in
UserGroups D-Bus property
$ id user99
uid=1006(user99) gid=100(users) groups=1000(priv-admin),1005(web),\
1006(redfish),1013(hostconsole),100(users)
$ curl -k https://${bmc}/redfish/v1/AccountService/Accounts/user99
{
"@odata.id": "/redfish/v1/AccountService/Accounts/user99",
"@odata.type": "#ManagerAccount.v1_4_0.ManagerAccount",
"AccountTypes": [
"HostConsole",
"Redfish",
"WebUI",
"ManagerConsole"
],
"Description": "User Account",
"Enabled": true,
"Id": "user99",
"Links": {
"Role": {
"@odata.id": "/redfish/v1/AccountService/Roles/Administrator"
}
},
"Locked": false,
"Locked@Redfish.AllowableValues": [
"false"
],
"Name": "User Account",
"Password": null,
"PasswordChangeRequired": false,
"RoleId": "Administrator",
"UserName": "user99"
- The hostconsole group is not present for readonly or operator users
and also made sure that console access is not provided. This testing
is done one the system and console access was tried by modifying the
https://github.com/openbmc/bmcweb/blob/master/scripts/websocket_test.py
+ curl -k https://${bmc}/redfish/v1/AccountService/Accounts/user99
{
"@odata.id": "/redfish/v1/AccountService/Accounts/user99",
"@odata.type": "#ManagerAccount.v1_4_0.ManagerAccount",
"AccountTypes": [
"IPMI",
"Redfish",
"WebUI",
"ManagerConsole"
],
"Description": "User Account",
"Enabled": true,
"Id": "user99",
"Links": {
"Role": {
"@odata.id": "/redfish/v1/AccountService/Roles/ReadOnly"
}
},
"Locked": false,
"Locked@Redfish.AllowableValues": [
"false"
],
"Name": "User Account",
"Password": null,
"PasswordChangeRequired": false,
"RoleId": "ReadOnly",
"UserName": "user99"
[INFO "http_connection.hpp":209] Request: 0x150ac38 HTTP/1.1 GET /console0 ::ffff:x.x.xx.xxx
[DEBUG "routing.hpp":1265] Matched rule (upgrade) '/console0' 1 / 2
[DEBUG "routing.hpp":1084] userName = user99 userRole = priv-user
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=ipmi
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=redfish
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=ssh
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=web
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: Login
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: ConfigureSelf
[DEBUG "routing.hpp":113] checkPrivileges: OEM REQUIRED: OpenBMCHostConsole
[ERROR "routing.hpp":1192] Insufficient Privilege
+ curl -k https://${bmc}/redfish/v1/AccountService/Accounts/user99
{
"@odata.id": "/redfish/v1/AccountService/Accounts/user99",
"@odata.type": "#ManagerAccount.v1_4_0.ManagerAccount",
"AccountTypes": [
"IPMI",
"Redfish",
"WebUI",
"ManagerConsole"
],
"Description": "User Account",
"Enabled": true,
"Id": "user99",
"Links": {
"Role": {
"@odata.id": "/redfish/v1/AccountService/Roles/Operator"
}
},
"Locked": false,
"Locked@Redfish.AllowableValues": [
"false"
],
"Name": "User Account",
"Password": null,
"PasswordChangeRequired": false,
"RoleId": "Operator",
"UserName": "user99"
[INFO "http_connection.hpp":209] Request: 0x21c7c38 HTTP/1.1 GET /console0 ::ffff:x.x.xx.xxx
[DEBUG "routing.hpp":1265] Matched rule (upgrade) '/console0' 1 / 2
[DEBUG "routing.hpp":1084] userName = user99 userRole = priv-operator
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=ipmi
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=redfish
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=ssh
[DEBUG "routing.hpp":1123] IsUserPrivileged: group=web
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: Login
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: ConfigureComponents
[DEBUG "routing.hpp":93] checkPrivileges: BASE USER: ConfigureSelf
[DEBUG "routing.hpp":113] checkPrivileges: OEM REQUIRED: OpenBMCHostConsole
[ERROR "routing.hpp":1192] Insufficient Privilege
Related commits:
NOTE: docs, openbmc, obmc-console changes are already merged. bmcweb
and phosphor-user-manager will be merged together.
docs: https://gerrit.openbmc.org/c/openbmc/docs/+/60968
phosphor-user-manager: https://gerrit.openbmc.org/c/openbmc/phosphor-user-manager/+/61583
openbmc: https://gerrit.openbmc.org/c/openbmc/openbmc/+/61582
obmc-console: https://gerrit.openbmc.org/c/openbmc/obmc-console/+/61581
bmcweb: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/61580
Change-Id: Ia5a33dafc9a76444e6a8e74e752f0f90cb0a31c8
Signed-off-by: Ninad Palsule <ninadpalsule@us.ibm.com>
|
|
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>
|
|
https://github.com/openbmc/bmcweb/commit/f8aa3d2704d3897eb724dab9ac596af8b1f0e33e
(4/15/20) added CSRF check into websockets but later setting cookieAuth
to true was removed so this session->cookieAuth is always false.
https://github.com/openbmc/bmcweb/commit/3909dc82a003893812f598434d6c4558107afa28
(7/15/20).
2 choices here add back this cookieAuth=true when cookie auth is used or
remove this "if cookieAuth" and do this check anytime
BMCWEB_INSECURE_DISABLE_CSRF_PREVENTION isn't enabled.
Really we shouldn't support any other auth on websockets so maybe
if (!session->cookieAuth){
unauthorized;
}
if go with the first choice. Went with the 2nd choice because cleaner.
This checking is a bit weird because it uses protocol for csrf checking.
https://github.com/openbmc/webui-vue/blob/b63e9d9a70dabc4c9a7038f7727fca6bd17d940a/src/views/Operations/SerialOverLan/SerialOverLanConsole.vue#L98
Tested: Before could log in to webui-vue, delete the XSRF-TOKEN but
still connect to the host console. After if deleted the XSRF-TOKEN
(browser dev tools), the websocket does not connect. Don't have a system
with KVM, VM enabled so wasn't able to check those but the webui-vue
code for them looks to pass the token. The webui-vue host console works
the same as before if you aren't messing with the XSRF-TOKEN.
Change-Id: Ibd5910587648f68809c7fd518bcf5a0bcf8cf329
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
An Internal server Error will happen if you delete the login user.
Match the "InterfacesRemoved" signal for monitoring the user status and
delete the session to fix this bug.
Tested:
1. Add a new user such as test
2. Login with the new user in web
3. Delete or rename the user by web and ipmi command
4. Refresh the web and a new user was needed to login in the web
Signed-off-by: Xie Ning <xiening.xll@bytedance.com>
Change-Id: I2b53edb71d9a4e904c7da54393539f87eeb2d7a3
|
|
string_view should always be passed by value; This commit is a sed
replace of the code to make all string_views pass by value, per general
coding guidelines[1].
[1] https://quuxplusone.github.io/blog/2021/11/09/pass-string-view-by-value/
Tested: Code compiles.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I55b342a29a0fbfce0a4ed9ea63db6014d03b134c
|
|
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
|
|
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
|
|
clang-tidy-15 is raising the following warning, which it automatically
fixed:
```
../include/sessions.hpp:262:38: error: unnecessary temporary object created while calling emplace [modernize-use-emplace,-warnings-as-errors]
auto it = authTokens.emplace(std::make_pair(sessionToken, session));
^~~~~~~~~~~~~~~ ~
```
Apply automatically generated fix.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I425efdcba4bf08b03d64f8fb913eab956f5a198d
|
|
While revisiting change 49039 I saw session.hpp included a few
files that are not used. I removed them and the code still
compiles.
Signed-off-by: Sui Chen <suichen6@gmail.com>
Change-Id: I97aa2359053ce6102b84af1ef555d881cd35eaba
|
|
The header pam_authenticate isn't referenced at all in these two files.
So remove it.
Tested: codes compiles
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4d9ea06dca2ce4d05add710ec3d6cc0df5c1a39d
|
|
This saves about 4k on the binary size
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I9546227a19c691b1aecb80e80307889548c0293f
|
|
This check involves explicitly declaring variables const when they're
declared auto, which helps in readability, and makes it more clear that
the variables are const.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I71198ea03850384a389a56ad26f2c4a48c75b148
|
|
There were a few places we weren't initting our variables per cpp core
guidelines. Fix all of them, and enable checks for this.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iba09924beb9fb26f597ff94d1cecbd6d6b1af912
|
|
Part of enforcing cpp core guidelines involves explicitly including all
constructors required on a non-trivial class. We were missing quite a
few. In all cases, the copy/move/and operator= methods are simply
deleted.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie8d6e8bf2bc311fa21a9ae48b0d61ee5c1940999
|
|
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>
|
|
bmcweb::OpenSSLGenerator is defined in include/random.hpp.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I1d8751b49a0121526a40b885ed7f5b520640f9f0
|
|
This reduces our web session timeout to 30 minutes per
NIST guidelines:
"Reauthentication of the subscriber SHALL be repeated following
any period of inactivity lasting 30 minutes or longer."
https://pages.nist.gov/800-63-3/sp800-63b.html
Change-Id: Icb600b5bc026107f582bc93be355bc5e2b46677d
Signed-off-by: Jason M. Bills <jason.m.bills@linux.intel.com>
|
|
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
|
|
Add meson options to enabled/disabled authentication methods:
- basic-auth : For enable basic authentication, default is enabled
- session-auth : For enable session token authentication, default is
enabled
- xtoken-auth : For enable x-token authentication, default is enabled
- cookie-auth : For enabled cookie authentication, default is enabled
Signed-off-by: Alan Kuo <Alan_Kuo@quantatw.com>
Change-Id: I52e636f2534a14897cb57d35e563ea8841cc68b9
|
|
Before writing bmcweb_persistent_data.json on bmcweb shutdown call
applySessionTimeouts() to ensure no stale sessions are wrote.
To accomplish this had to move applySessionTimeouts to public.
Tested: Stop bmcweb, modify bmcweb_persistent_data.json timeout to
be 30 seconds. Start bmcweb. Verify timeout 30 seconds and
1 session is restored. Wait 1 min. stop bmcweb. Verify
no sessions in bmcweb_persistent_data.json.
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
Change-Id: Idfaf7c144b3bdeb2741b48f603d7213ac1a51f10
|
|
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
|
|
fix regression on 5fb91ba400e0482813cf5e1a86fdca17468d0a6a.
Timeout is a global setting, not a per-session setting. This caused
problems with regenerating it, as session restoration doesn't follow the
"best effort" policy we've done before.
This commit:
1. Makes Session::fromJson more robust against extra keys.
2. Disallowed reading in client_id if IBM_Management_console isn't
enabled.
3. Moves timeout to the proper place in the persistent config file.
Resolves https://github.com/openbmc/bmcweb/issues/158
Tested:
Downloaded to bmc, cleared bmcweb_persistent_data.json, then logged in
using webui-vue.
Rebooted BMC.
Reloaded /redfish/v1/SessionService/Sessions/<sessionid> and observed
that all data restored properly. Unclear why, but ClientOriginIPAddress
seems broken, but that seems true prior to this patch.
Data that got returned is included for completeness.
{
"@odata.id": "/redfish/v1/SessionService/Sessions/BKqK5dNfNS",
"@odata.type": "#Session.v1_3_0.Session",
"ClientOriginIPAddress": "",
"Description": "Manager User Session",
"Id": "BKqK5dNfNS",
"Name": "User Session",
"UserName": "root"
}
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I716431fd4775af63715d07973f723caa8cb34259
|
|
(In the voice of the kid from sixth sense) I see string copies...
Apparently there are a lot of places we make unnecessary copies. This
fixes all of them.
Not sure how to split this up into smaller patches, or if it even needs
split up. It seems pretty easy to review to me, because basically every
diff is identical.
Change-Id: I22b4ae4f96f7e4082d2bc701098a04f7bed95369
Signed-off-by: Ed Tanous <ed@tanous.net>
Signed-off-by: Wludzik, Jozef <jozef.wludzik@intel.com>
|
|
This commit moves the openssl random number generator into its own file,
so it can be used in EventService, and moves it to its own file. Seeding a
random number generator with time is bad practice in general, so much so
that there's a CERT rule about it as well as a clang-tidy check.
https://clang.llvm.org/extra/clang-tidy/checks/cert-msc51-cpp.html
This doesn't matter much in this case, as we're generating a randomized
int for an ID, but it will matter in other cases, and we'd like to have
the check on to verify that.
Change-Id: I8e6aebb7962d259045ffd558eea22f07f9c23821
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Several pieces of code seems to be using the adl_serializer from
nlohmann. This unfortunately has very undesirable behavior in some
cases, and makes a lot of things really difficult to track back to the
function that did the serialization, which has caused several bugs in
the past with incorrect types.
This patchset removes them, and opts for the inline version of the
nlohmann json serialization.
Tested:
Booted bmcweb, and logged in.
cat bmcweb_persistent_data.json showed persistent data written properly.
Logged into bmc through webui-vue
systemctl restart bmcweb
Then refreshed webui-vue, and didn't get logged out.
Change-Id: I92868629c54d08b37dd1d956f7c2e2a954f9b670
|
|
Lots of bad includes got put in recently, including big things, like
boost/http and beast/core. These are lots of code to parse, and leads
to files including things they didn't mean to.
Tested:
Code compiles
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I38de889fdfd9b23f66a2259bb30bf6584345e77f
|
|
- This commit would add the patch support for the session
timeout propery under the sessionservice.
- This commit also brings in support for persistent session
timeout property.
Tested By:
1. Redfish validator passed.
2. PATCH the session time out property using the below command
PATCH -d '{"SessionTimeout": 100}' https://<bmcip>/redfish/v1/SessionService
3. GET on sessionservice should return the value of time out which is
patched by using the above command & also GET on the session service fails
with Unauthorized error post the patched timeout value.
4. And also, the existing sessions that are open for the new timeout value are
also closed.
5. As per the schema , the range of values that are allowed for
session timeout are between 30 sec to 86400 sec, so any value which
is patched out of the range is failed with an appropriate error message.
6. PATCH the session timeout to new value using 2, and them restart the bmcweb
and the GET using 3 should return the new value.
Signed-off-by: Manojkiran Eda <manojkiran.eda@gmail.com>
Change-Id: Id50eacc5018b7a82371fd37a2ae1e7fb7596ed2b
|
|
There were a couple places in code where we still use index based for
loops. Move these to the more modern range based for loops.
Tested: Needs testing. Changes made by clang-tidy.
Signed-off-by: Ed Tanous <ed.tanous@intel.com>
Change-Id: I30bf6fae6b2540434d5c98900a8f6bd0c8f2be93
|
|
This commit enables clang warnings, and fixes all warnings that were
found. Most of these fall into a couple categories:
Variable shadow issues were fixed by renaming variables
unused parameter warnings were resolved by either checking error codes
that had been ignored, or removing the name of the variable from the
scope.
Other various warnings were fixed in the best way I was able to come up
with.
Note, the redfish Node class is especially insidious, as it causes all
imlementers to have variables for parameters, regardless of whether or
not they are used. Deprecating the Node class is on my list of things
to do, as it adds extra overhead, and in general isn't a useful
abstraction. For now, I have simply fixed all the handlers.
Tested:
Added the current meta-clang meta layer into bblayers.conf, and added
TOOLCHAIN_pn-bmcweb = "clang" to my local.conf
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: Ia75b94010359170159c703e535d1c1af182fe700
|
|
Middlewares, while kinda cool from an academic standpoint, make our
build times even worse than they already are. Given that we only really
use 1 real middleware today (token auth) and it needs to move into the
parser mode anyway (for security limiting buffer sizes), we might as well
use this as an opportunity to delete some code.
Some other things that happen:
1. Persistent data now moves out of the crow namespace
2. App is no longer a template
3. All request_routes implementations no longer become templates. This
should be a decent (unmeasured) win on compile times.
This commit was part of a commit previously called "various cleanups".
This separates ONLY the middleware deletion part of that.
Note, this also deletes about 400 lines of hard to understand code.
Change-Id: I4c19e25491a153a2aa2e4ef46fc797bcb5b3581a
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
This commit saves the IP Address of the client from where the session
was created.
- This is not a user supplied value. The BMC will internally pull the
IP address from the incoming create-session request.
- It should also be noted that ClientIP will not change if the same session
token is used from some other IP address for further management of the BMC.
Tested by:
1. Create session
2. Display the Session details with GET command to check the IP from where
the session is created/updated.
GET https://${bmc}/redfish/v1/SessionService/Sessions/<id>
{
"@odata.id": "/redfish/v1/SessionService/Sessions/<id>",
"@odata.type": "#Session.v1_0_2.Session",
"Description": "Manager User Session",
"Id": "<id>",
"Name": "User Session",
"Oem": {
"OpenBMC": {
"@odata.type": "#OemSession.v1_0_0.Session",
"ClientOriginIP": "<ip address>"
}
},
"UserName": "root"
}
3. Redfish validator is run successfully.
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: I0076f260f50a991600ec060c72f3e46fb9a9cbb8
|
|
This commit implements handling the OemSession ClientID parameter
for the IBM management console.
Each session gets a random generated unique Id (Resource Id); but this Id
is not a parameter that the client can set to a well known identifier.
This Oem parameter ClientID is the string which the client can supply to
uniquely identify itself among other sessions in the BMC. This is a read-only
property which shall be passed in only during the session creation.
1. Create session by supplying the ClientID Oem parameter
2. Display the ClientID associated with the session
3. Persist the ClientID across BMC reboot
Tested by:
============
1. POST https://${bmc}/redfish/v1/SessionService/Sessions -d
'{"UserName":"root", "Password":<>, "Oem":{"OpenBMC" : {"ClientID":"<client unique id>"}}}'
2. GET https://${bmc}/redfish/v1/SessionService/Sessions/<id>
{
"@odata.id": "/redfish/v1/SessionService/Sessions/<id>",
"@odata.type": "#Session.v1_0_2.Session",
"Description": "Manager User Session",
"Id": "<id>",
"Name": "User Session",
"Oem": {
"OpenBMC": {
"@odata.type": "#OemSession.v1_0_0.Session",
"ClientID": "<client unique id>"
}
},
"UserName": "root"
}
3. Verified the session creation works fine without the Oem parameters.
4. Redfish validator
Signed-off-by: Sunitha Harish <sunithaharish04@gmail.com>
Change-Id: Ia740a610e3974dc3781bcee702c74ded9903944a
|
|
This is from openbmc/docs/style/cpp/.clang-format
Other OpenBMC repos are doing the same.
Tested: Built and validator passed.
Change-Id: Ief26c755c9ce012823e16a506342b0547a53517a
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This implements the Redfish PasswordChangeRequired handling. See
section 13.3.7.1 "Password change required handling" in the 1.9.1 spec:
https://www.dmtf.org/sites/default/files/standards/documents/DSP0266_1.9.1.pdf
These portions of the spec are implemented:
- Authenticatation with a correct but expired password creates a
session:
- The session is restricted to the ConfigureSelf privilege which
allows a user to change their own password (via GET and PATCH
Password for their own account). Support for the ConfigureSelf
privilege is already in BMCWeb.
- The session object has the PasswordChangeRequired message.
- All other operations respond with http status code 403 Forbidden
and include the PasswordChangeRequired message.
- The ManagerAccount (URI /redfish/v1/AccountService/Accounts/USER)
PasswordChangeRequired property is implemented for local accounts
but not present for remote accounts.
This has the following additional behavior:
The PasswordChangeRequired property is updated at the start of each new
REST operation, even within an existing session. This behavior
implements a "dynamic" PasswordChangeRequired handling that responds to
changes to the underlying "password expired" status. Specifically:
- Sessions restricted by the PasswordChangeRequired handling lose that
restriction when the underlying account password is changed.
- Sessions become subject to the PasswordChangeRequired handling
restrictions whenever the underlying account password expires.
- The mechanism is to check if the password is expired at the start of
every new REST API operation, effectively updating the ManagerAccount
PasswordChangeRequired property each time. This makes BMCWeb
responsive to changes in the underlying account due to other activity
on the BMC.
Notes:
1. Note that when an account password status is changed (for example,
the password becomes expired or is changed) and that account has
active sessions, those sessions remain. They are not deleted. Any
current operations are allowed to complete. Subsequent operations
with that session pick up the new password status.
2. This does not implement OWASP recommendations which call for sessions
to be dropped when there is a significant change to the underlying
account. For example, when the password is changed, the password
becomes expired, or when the account's Role changes. OWASP's
recommendation is due to the session fixation vulnerability. See the
OWASP Session Management Cheat Sheet section "Renew the Session ID
After Any Privilege Level Change":
https://cheatsheetseries.owasp.org/cheatsheets/Session_Management_Cheat_Sheet.html#renew-the-session-id-after-any-privilege-level-change
BMCWeb protects against session fixation vulnerabilities because it
always regenerates new session IDs when successful authentication
creates a new session.
3. Users authenticating via mTLS are not subject to the
PasswordChangeRequired behavior because mTLS takes precedence over
password-based authentication.
Tested:
0. Setup:
- The `passwd --expire USERNAME` command was used to expire
passwords. The `chage USER` command was also used.
- The following were used to change the password: Redfish API,
passwd command, and the SSH password change dialog.
- Tested the following via Basic Auth, /login, and Redfish login
(except where Basic Auth does not create a persistent session).
- Only local user account were tested.
- Did not test authentication via mTLS or with LDAP users.
1. When the password is not expired, authentication behaves as usual
for both correct and incorrect passwords.
2. When the password is incorrect and expired, authentication fails as
usual.
3. When the password is correct but expired:
A. A session is created and has the PasswordChangeRequired message.
B. That session cannot access resources that require Login privilege
and the 403 message contains the PasswordChangeRequired message.
C. That session can be used to GET the user's account, PATCH the
Password, and DELETE the session object.
D. The account PasswordChangeRequired reports true.
4. While a session is established, try expiring and changing
(unexpiring) the password using various mechanisms. Ensure both the
session object and the ManagerAccount PasswordChangeRequired property
report the correct condition, and ensure PasswordChangeRequired
handling (restricting operations to ConfigureSelf when
PasswordChangeRequired is true) is applied correctly.
Signed-off-by: Joseph Reynolds <joseph-reynolds@charter.net>
Change-Id: Iedc61dea8f949e4b182e14dc189de02d1f74d3e8
|
|
This commit does the following
=> makes the lock class singleton.
=> during session timeout erase the locks associated
with the session.
=> Erase the locks when the session is explicitly deleted
on a user request.
We need to find a different way of calculating session timeout
currently session timeout gets calculated when the
request comes to BMC.
TODO: We need some module which keeps looking at the sessions
in certain time interval and earse the session if it is
timeout, It is useful in the case where there is resources
which gets free after session timeout.
It may happen that client gets the session, obtain cerain resources
on that session and never sends any request, in that case session timeout
will never occur for that session.
Signed-off-by: Ratan Gupta <ratagupt@linux.vnet.ibm.com>
Change-Id: Ic9962f761fc84a03747a90bd951ea36eb8962455
|
|
std::random_device is not a cryptographically secure
algorithm. Switch to RAND_bytes instead.
Tested: Login and logout works as expected
Change-Id: If24fa6c3a0652c011bc50ae611b180f342d68433
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
|
This adds CSRF check into websockets to avoid
attacks on websockets.
Tested: Could no longer use crosssite scripting to
open websocket. KVM and SOL still work once web-ui
changes are updated
Change-Id: I325079ae3d4db2701671564dff733e034d2670d6
Signed-off-by: James Feist <james.feist@linux.intel.com>
|
|
This commit sets the mutual TLS option to ON by default.
Core mTLS implementation was accepted under this commit:
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/23588
where by default the mTLS was disabled.
Tested:
Manual tests were made to verify if this option turns the
mTLS ON.
Signed-off-by: Zbigniew Kurzynski <zbigniew.kurzynski@intel.com>
Change-Id: I3bc5f5453d4c4df1cd7ecb0b8868423285b6ab83
|
|
Modified the code to make an asynchronous call to GetUserInfo to get
the user role for authorization.
For local users, DBus matches are used to store user role map hot in
memory. Hence, bmcweb has to know whether a user is a local user or
LDAP user to get the role. To avoid this, removed the existing DBus
matches and modified the code to call GetUserInfo to get the role of
local users as well as LDAP users.
Tested:
- Created a local user having admin privilege and verified that he is
able to restart the system
/redfish/v1/Systems/system/Actions/ComputerSystem.Reset
-d '{"ResetType": "GracefulRestart"}'
- Created a local user having user privilege and verified that he is
unauthorized to restart the system
/redfish/v1/Systems/system/Actions/ComputerSystem.Reset
-d '{"ResetType": "GracefulRestart"}'
- Created a remote user having admin privilege and verified that he is
able to restart the system
/redfish/v1/Systems/system/Actions/ComputerSystem.Reset
-d '{"ResetType": "GracefulRestart"}'
- Created a remote user having user privilege and verified that he is
unauthorized to restart the system
/redfish/v1/Systems/system/Actions/ComputerSystem.Reset
-d '{"ResetType": "GracefulRestart"}'
- Tested Redfish ConfigureSelf privilege
Signed-off-by: RAJESWARAN THILLAIGOVINDAN <rajeswgo@in.ibm.com>
Change-Id: Ic3e46a0c0aff2cf456c98048350e58e302011c57
|
|
This commit is a fix for revert:
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/27205
The mTLS will work only when:
* Compilation flag BMCWEB_ENABLE_MUTUAL_TLS_AUTHENTICATION is
enabled and,
* AccountService->Oem->OpenBMC->AuthMethods->TLS is set and,
* at least one certificate is uploaded to TrustStore.
To enforce a new mTLS settings after switching AuthMethods->TLS
all awaiting for acceptance connections are dropped.
In case of authorized mTLS requests a cookie with XSRF-TOKEN entry
and SESSION is added to response. It will force the X-XSRF-TOKEN
challenge in case of applications supporting cookies like web
browsers.
Also some minor log messages changes were made in this commit.
Tested:
validator_service run with status: 'Validation has succeeded'.
Tests were made with Chrome browser Version 78.0.3904.108.
The certificate popup appears only when mTLS is enabled in CMake,
the TLS option is set and at least one CA certificate is present.
In other cases mTLS is not active and there is no popup for certs.
Signed-off-by: Zbigniew Kurzynski <zbigniew.kurzynski@intel.com>
Change-Id: I778b848b767356b60386694f802a89b3c1f6cb14
|