Age | Commit message (Collapse) | Author | Files | Lines |
|
Backpressure on incoming bytes helps both to simplify the layering of
the console, as well as prevent some cases of OOM crashes.
Similar to what we did with nbd_proxy, move obmc console over to the new
sendEx interface, allowing for backpressure, and fixed size std::array
buffers.
Tested:
Made sure single console can see the data.
Made sure two consoles can see the data.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I63d142fc5e8f8a734f3a7b8d0aa3f0d8c263d5ba
|
|
Added new ConsoleHandler class to prepare for the multiple consoles
support. All global fields are moved to the ConsoleHandler class and a
new global map added to remember the ConsoleHandler for each console
path. There is single ConsoleHandler per connection so we don't need
session map per route. There is a limit added for max number of
connection allowed to avoid any service attacks.
Testing:
- Make sure that single console works fine and data is seen on the
console.
- Make sure that multiple consoles of type host console are created
and data is seen on all consoles. Also using traces made sure
that new handlers are destroyed.
Traces: Traces shows that multiple consoles active and later destroyed.
[INFO "http_connection.hpp":209] Request: 0x24bb790 HTTP/1.1 GET
/console0 ::ffff:x.xx.xxx.xx
[DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console0' 1 / 2
[DEBUG "obmc_console.hpp":238] Connection 0x24eb424 opened
[DEBUG "obmc_console.hpp":150] Obmc handler 0x24c18fc added 1 for path
0x24eb424
[DEBUG "obmc_console.hpp":257] Console Object path =
/xyz/openbmc_project/console/default service =
xyz.openbmc_project.Console.default Request target = /console0
[DEBUG "obmc_console.hpp":224] Console web socket path: /console0
Console unix FD: 13 duped FD: 14
[DEBUG "obmc_console.hpp":44] Outbuffer empty. Bailing out
[INFO "http_connection.hpp":209] Request: 0x265d740 HTTP/1.1 GET
/console0 ::ffff:x.xx.xxx.xx
[DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console0' 1 / 2
[DEBUG "obmc_console.hpp":238] Connection 0x2661de4 opened
[DEBUG "obmc_console.hpp":150] Obmc handler 0x25e69ac added 1 for path
0x2661de4
[DEBUG "obmc_console.hpp":257] Console Object path =
/xyz/openbmc_project/console/default service =
xyz.openbmc_project.Console.default Request target = /console0
[DEBUG "obmc_console.hpp":224] Console web socket path: /console0
Console unix FD: 19 duped FD: 20
[DEBUG "obmc_console.hpp":44] Outbuffer empty. Bailing out
[INFO "http_connection.hpp":209] Request: 0x265d740 HTTP/1.1 GET
/console0 ::ffff:x.xx.xxx.xx
[DEBUG "routing.hpp":1240] Matched rule (upgrade) '/console0' 1 / 2
[DEBUG "obmc_console.hpp":238] Connection 0x25f1fdc opened
[DEBUG "obmc_console.hpp":150] Obmc handler 0x26ff22c added 1 for path
0x25f1fdc
[DEBUG "obmc_console.hpp":257] Console Object path =
/xyz/openbmc_project/console/default service =
xyz.openbmc_project.Console.default Request target = /console0
[DEBUG "obmc_console.hpp":224] Console web socket path: /console0
Console unix FD: 19 duped FD: 21
[DEBUG "obmc_console.hpp":44] Outbuffer empty. Bailing out
[INFO "obmc_console.hpp":177] Closing websocket. Reason:
[DEBUG "obmc_console.hpp":184] Remove connection 0x25f1fdc from obmc
handler 0x26ff22c for path /console0
[INFO "obmc_console.hpp":177] Closing websocket. Reason:
[DEBUG "obmc_console.hpp":184] Remove connection 0x2661de4 from obmc
handler 0x25e69ac for path /console0
[INFO "obmc_console.hpp":177] Closing websocket. Reason:
[DEBUG "obmc_console.hpp":184] Remove connection 0x24eb424 from obmc
handler 0x24c18fc for path /console0
Change-Id: I77a58a3a186e87611219aed90b221f9b8be7fa2f
Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
|
|
This commit adds a meson option to allow selecting which dns resolver
bmcweb uses. There are use cases, like Open Compute Project Inband
Management Agent, that would require not using dbus, which would require
us to fall back to the asio resolver. This commit makes the existing
asio resolver constructor, and async_resolve methods match the
equivalents in asio (which we intended to do anyway), then adds a macro
and configure option for being able to select which resolver backend to
rely on.
Tested: Code can now compile without sdbusplus.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I3220214367179f131a60082bdfaf7e725d35c125
|
|
There are cases in this method where if CSRF protection is disabled,
this argument will not be used, and will trigger a compile error. This
commit fixes the compile error.
Tested: Code compiles with CSRF disabled option set.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I6daa5193fa162c73c57991600058c198dc38a418
|
|
Per [1] we really shouldn't be using regex. In the cases we do, it's a
HUUUUUGE performance benefit to be compiling the regex ONCE.
The only downside is a slight increase in memory usage.
[1]: https://github.com/openbmc/bmcweb/issues/176
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8644b8a07810349fb60bfa0258a13e815912a38e
|
|
System includes should be included with <>, in-tree includes should be
included with "". This was found manually, with the help of the
following grep statement[1].
git grep -o -h "#include .*" | sort | uniq
Tested:
Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I1a6b2a5ba35ccbbb61c67b7c4b036a2d7b3a36a3
|
|
This drop is a preparation of supporting multiple consoles in bmcweb.
In this drop we will hook up the new DBUS interface to get the unix
socket file descriptor for existing host console. At this time bmcweb
only allows host console.
This drop includes following bmcweb changes:
- The default console leaf node is set to "default" by the obmc-console
- Currently the URL is still maintained to /console0 for GUI
compatibility. In future, it will be changed to /console/<str>
where <str> could be any string which represents the console id.
- In the obmc routing function, query the console DBUS interface for
all available consoles. If the object leaf matches with the target
string, then create socket and assign the file descriptor returned
by the DBUS console.
[INFO "http_connection.hpp":209] Request: 0x1b8c608 HTTP/1.1 GET /console0
::ffff:x.xxx.xx.xxx
|
[DEBUG "routing.hpp":1440] Matched rule (upgrade) '/console0' 1 / 2
|
[DEBUG "obmc_console.hpp":247] Connection 0x13e3c8c opened
[DEBUG "obmc_console.hpp":268] Console Object path = \
/xyz/openbmc_project/console/default Request target = /console0
[DEBUG "obmc_console.hpp":230] Looking up unixFD for Service \
xyz.openbmc_project.Console.default Path /xyz/openbmc_project/console/default
[DEBUG "obmc_console.hpp":157] Console web socket path: /console0 Console\
unix FD: 12 duped FD: 13
Testing:
Make sure that console open is working for /console0 on rainier machine
Related commits:
1) phosphor-dbus-interface: https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/61486
2) obmc-console: https://gerrit.openbmc.org/c/openbmc/obmc-console/+/62496
3) bmcweb: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/62525
Change-Id: I476f1bb3e3be384ab09802340a59ffa036ca0278
Signed-off-by: Ninad Palsule <ninadpalsule@us.ibm.com>
|
|
This change is adding Triggers property to Links when GET is called on
MetricReportDefinition. It contains array of @odata.id pointing to
Trigger resource if it is also linking to given MRD.
Testing done:
- Links/Trigger property is returned by GET request on
/redfish/v1/TelemetryService/MetricReportDefinitions/<str>/
Signed-off-by: Szymon Dompke <szymon.dompke@intel.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I5accf4b50324437b0b185003200078ad2c7020b0
|
|
Added support for multiple MetricProperties. Added support for new
parameters: CollectionTimeScope, CollectionDuration. ReadingParameters
was not yet changed in telemetry backend, instead temporary property
ReadingParametersFutureVersion was introduced. Once bmcweb is adapted to
use ReadingParametersFutureVersion this property will be renamed in
backend to ReadingParameters. Then bmcweb will change to use
ReadingParameters. Then ReadingParametersFutureVersion will be removed
from backend and everything will be exactly like described in
phosphor-dbus-interfaces without introducing breaking changes.
Related change in phosphor-dbus-interfaces [1], [2]. This change needs
to be bumped together with [3].
Tested:
- It is possible to create MetricReportDefinitions with multiple
MetricProperties.
- Stub values for new parameters are correctly passed to telemetry
service.
- All existing telemetry service functionalities remain unchanged.
[1]: https://github.com/openbmc/phosphor-dbus-interfaces/commit/4f9c09144b60edc015291d2c120fc5b33aa0bec2
[2]: https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/60750
[3]: https://gerrit.openbmc.org/c/openbmc/telemetry/+/58229
Change-Id: I2cd17069e3ea015c8f5571c29278f1d50536272a
Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com>
Signed-off-by: Lukasz Kazmierczak <lukasz.kazmierczak@intel.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>
|
|
Capturing these possibly overloaded values by reference can avoid a copy
in some cases, and it's good to be consistent.
This change was made automatically by grep/sed.
Tested: Code compiles.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iafeaca2a5dc52f39753b5a3880419d6bc943f81b
|
|
When displaying an integer in HTML, the number of characters is off by 1
causing the numbers to display without the last digit.
This is because the pointer into numberbuffer gets initially advanced by
the number of characters which ends up being one too many.
For example, the buffer pointer is pointing at numberbuffer[0]. If we
display a 4 digit number, we advance by 4, so it ends up pointing at
numberbuffer[4] for the last digit. In the end, we return only the
number of characters which is numberbuffer[0-3] cutting off the last
digit.
This changes the pointer to advance by one less than the number of
digits, so the buffer fills from numberbuffer[3] and returns all 4
digits.
Tested:
Read redfish/v1/SessionService and confirmed that the "SessionTimeout"
value displays the full value of 1800.
Change-Id: Iaa974d7a41352fd9a15024ccf04c5926a4efe7a2
Signed-off-by: Jason M. Bills <jason.m.bills@intel.com>
|
|
Neither of these files make use of io_context, so they shouldn't be here
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I86c195a5881a50a275e977aa73b9d7144b53844b
|
|
Boost 1.82 dropped a lovely new toy, boost::urls::format, which is a lot
like our urlFromPieces method, but better in that it makes the resulting
uris more readable, and allows doing things like fragments in a single
line instead of multiple. We should prefer it in some cases.
Tested:
Redfish service validator passes.
Spot checks of URLs work as expected.
Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia7b38f0a95771c862507e7d5b4aa68aa1c98403c
|
|
It seems like clang-tidy doesn't catch every place that an emplace could
be used instead of a push. Use a few grep/sed pairs to find and fix up
some common patterns.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I93eaec26b8e3be240599e92b66cf54947073dc4c
|
|
There is code in ibm/locks that has had clang-tidy warnings disabled
for a while due to multiple safety and endianness issues. The code
has not been fixed in a while and with clang-16 it is unable to be
exempted further. Disable it until someone who cares can fix this
in the proper way.
```
../include/ibm/locks.hpp:522:14: error: 'p' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
uint8_t* p = reinterpret_cast<uint8_t*>(&resourceId1);
~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/ibm/locks.hpp:527:25: note: used in buffer access here
uint8_t pPosition = p[position];
^
../include/ibm/locks.hpp:524:14: error: 'q' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
uint8_t* q = reinterpret_cast<uint8_t*>(&resourceId2);
~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/ibm/locks.hpp:529:25: note: used in buffer access here
uint8_t qPosition = q[position];
```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I8a7fcbed1099419ad1715c86ffcbfef20820251e
|
|
The clang-tidy warning 'modernize-use-emplace' correctly flags a
few places where emplace should be used over push.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I6ca79285a87d6927e718345dc8dce0387e6b1eda
|
|
A number of similar warnings about unsafe pointer arithmetic.
```
../redfish-core/lib/log_services.hpp:269:39: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage]
indexStr.data(), indexStr.data() + indexStr.size(), index);
```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: Icc4a0d2f418c76d6987ef2318b0098d30d116389
|
|
```
../include/json_html_serializer.hpp:426:42: error: 'end' declared with a const-qualified typedef; results in the type being 'char *const' instead of 'const char *' [misc-misplaced-const,-warnings-as-errors]
const std::array<char, 64>::iterator end =
^
/usr/lib/gcc/x86_64-pc-linux-gnu/12/include/g++-v12/array:106:44: note: typedef declared here
typedef value_type* iterator;
../include/json_html_serializer.hpp:165:60: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage]
std::snprintf(stringBuffer.data() + bytes, 7,
../include/json_html_serializer.hpp:327:11: error: 'bufferPtr' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
auto* bufferPtr = begin(numberbuffer);
../include/json_html_serializer.hpp:393:11: error: 'begin' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
char* begin = numberbuffer.data();
../include/json_html_serializer.hpp:425:56: error: unsafe pointer arithmetic [-Werror,-Wunsafe-buffer-usage]
std::remove(numberbuffer.begin(), numberbuffer.begin() + len, ',');
```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: If6aaf038c939ad76da73e68e746a56b0905b2804
|
|
```
../include/pam_authenticate.hpp:11:75: error: 'msg' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
inline int pamFunctionConversation(int numMsg, const struct pam_message** msg,
```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: Ic0e6d63b01eea78cac54407246363177cb208f8b
|
|
```
../include/multipart_parser.hpp:77:21: error: 'buffer' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
const char* buffer = req.body().data();
../include/multipart_parser.hpp:246:38: error: 'buffer' is an unsafe pointer used for buffer access [-Werror,-Wunsafe-buffer-usage]
void skipNonBoundary(const char* buffer, size_t len, size_t boundaryEnd,
```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: Iad3b4b241ec75a152e240755a307a970798079fb
|
|
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I8122bfffaac469ee551c72632b671e8644a4da44
|
|
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>
|
|
The new ObjectMapper methods are added as part of
https://gerrit.openbmc.org/c/openbmc/phosphor-objmgr/+/57822
- GetAssociatedSubTree
- GetAssociatedSubTreePaths
The two methods are meant to be used to replace places where two dbus
calls are used to get subtree and then get associated objects.
Change-Id: I80a7ea935700a1ac5aebe6271f242aa103cc3d59
Signed-off-by: Willy Tu <wltu@google.com>
|
|
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>
|
|
As-written, the nbd (and all websocket daemons) suffer from a problem
where there is no way to apply socket backpressure, so in certain
conditions, it's trivial to run the BMC out of memory on a given
message. This is a problem.
This commit implements the idea of an incremental callback handler, that
accepts a callback function to be run when the processing of the message
is complete. This allows applying backpressure on the socket, which in
turn, should provide pressure back to the client, and prevent buffering
crashes on slow connections, or connections with high latency.
Tested: NBD proxy not upstream, no way to test. No changes made to
normal websocket flow.
Signed-off-by: Michal Orzel <michalx.orzel@intel.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I3f116cc91eeadc949579deacbeb2d9f5e0f4fa53
|
|
Tested: No way to test. Non-upstream backend, inspection only.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib2593b66407e0f102f543777ecf907b434acac52
|
|
nbd proxy should not have its own authorization checks, as these are
now handled in the core as of 7e9093e625961f533250a6c193c1a474e98007c4
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8874d8a09278ba21d2acfdf076cb76dee718ecf4
|
|
The openbmc_dbus_reset was holding reference of `crow::Response`, set
the response in `~InProgressActionData()`, and call res.end() to
complete the result of the response.
The bmcweb code now uses `std::shared_ptr<AsyncResp>` for the response
and the `res.end()` is handled in `~AsyncResp()`.
By using the reference of `crow::Response`, the `InProgressActionData`
is actually using a dangling reference because the
`std::shared_ptr<AsyncResp>` is already destructed, and bmcweb will
crash on `action` calls, or not crash but get invalid response, as it's
undefined behavior.
Fix the above issue by using `std::shared_ptr<AsyncResp>` to make sure
the response is correctly handled.
Tested:
1. Without the fix, bmcweb crashes, or get no json output response on
the below method call, be noted that it's an invalid call:
```
$ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/deleteAll
```
2. With the fix, bmcweb gives expected response:
```
$ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/deleteAll
{
"data": {
"description": "The specified method cannot be found"
},
"message": "404 Not Found",
"status": "error"
}
$ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/DeleteAll
{
"data": null,
"message": "200 OK",
"status": "ok"
}
```
Signed-off-by: Lei YU <yulei.sh@bytedance.com>
Change-Id: I38ef34fe8ff18e4e127664c853c6792461f6edf8
|
|
There are currently many files that use the GetManagedObjects method.
Since they are a general method, they are defined in the
dbus_utility.hpp file and refactors them.
Tested:
1. Built bmcweb successfully and Validator passes.
2. We got the same result as previously in the ethernet schema.
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: I0c25b7b6b9421bea46ff0afadbaa4783b704e664
|
|
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
|
|
These are 4xx errors, 404 not found. Move the logging to WARNING so they
don't log unless WARNING level is enabled. This follows the guidance in
the commit below.
Tested: None.
Change-Id: I38b2bec64507d75286f79d61acf7a96226598e0b
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
In a similar way we've transformed code in bmcweb, move these callbacks
to use non-lambdas, to simplify their use.
Tested: No good test harness here. Inspection only, mechanical
transform.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic5c2c86fef0abfaadb07022123ad93914d5ddf69
|
|
There are currently many files that use the get endpoints methods[1].
Since they are general methods, they are defined in the
dbus_utility.hpp file and will be further refactored in subsequent
patches.
Since the current endpoints of phosphor-objmgr do not support
object_path and fails in romulus CI[2], so we should revert to
std::string.
Also, Updated the populateSoftwareInformation method of sw_utils.hpp
[1] https://github.com/openbmc/docs/blob/master/architecture/object-mapper.md#associations
[2] https://gerrit.openbmc.org/c/openbmc/bmcweb/+/58924/22/include/dbus_utility.hpp#98
When an object with, for example, an object path of pathA uses
the following values:
["foo", "bar", "pathB"]
The mapper will create 2 new objects:
pathA/foo
pathB/bar
Tested: Built bmcweb successuflly and Validator passes
curl -k -H "X-Auth-Token: $token" -X GET
https://${bmc}/redfish/v1/Managers/bmc
{
"@odata.id": "/redfish/v1/Managers/bmc",
"@odata.type": "#Manager.v1_14_0.Manager",
...
"FirmwareVersion": "2.14.0-dev-95-gea3949e76-dirty",
...
}
Tested: Validator passes
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: I32a2c663bf2b8c84517bd0ecb4ccba61ce87c7e2
|
|
The sdbusplus headers provide shortened aliases for many types.
Switch to using them to provide better code clarity and shorter
lines. Possible replacements are for:
* bus_t
* exception_t
* manager_t
* match_t
* message_t
* object_t
* slot_t
Change-Id: Id13cf695f25312a9561c0954f5ed133985ab1222
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
|
|
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
|
|
Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
Change-Id: Id81db4d512b5ea1222a145dc2b9e9907e8b0f084
|
|
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
|
|
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
|
|
Only id in event_service and account_service have not been updated due
to the risk of it breaking the username/id. It will require further
testing to verify.
Use urlFromPieces wherever that is needed to insert a variable in the
URI. Don't use urlFromPieces when it is hardcoded values. This allow us
to control all resource URIs that is dynamically added and to sync with
the current recommanded method for `@odata.id`. The goal is to have a
common place to manage the url created from dbus-paths in order to
manage/update it easily when needed.
Tested:
RedfishValidtor Passed for all resource including the sensors with the
fragments.
Change-Id: I95cdfaaee58fc7f21c95f5944e1e5c813b3215f2
Signed-off-by: Willy Tu <wltu@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
There are certain cases where we use this split function, and we expect
tokens to be read out. For example:
/xyz/openbmc_project/sensors/unit/name
Should split into a "" in the first position. This use case is not
common, and a quick grep shows only two places in the code expect this
behavior. Boost::split has this behavior already, which is what this
function is emulating. While we could fix these, in the end they should
be following the rules outlined in COMMON_ERRORS.md, which disallow
this kind of parsing completely.
Tested: New unit tests passing.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iec3dcbf2b495b2b3b4ed419172c4133b16f7c65d
|
|
For systems implementing to the OWASP security guidelines[1] (of which all
should ideally) we should be checking the content-type header all times
that we parse a request as JSON.
This commit adds an option for parsing content-type, and sets a default
of "must get content-type". Ideally this would not be a breaking
change, but given the number of guides and scripts that omit the content
type, it seems worthwhile to add a trapdoor, such that people can opt
into their own model on how they would like to see this checking work.
Tested:
```
curl --insecure -H "Content-Type: application/json" -X POST -D headers.txt https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName":"root", "Password":"0penBmc"}'
```
Succeeds.
Removing Content-Type argument causes bmc to return
Base.1.13.0.UnrecognizedRequestBody.
[1] cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html
Change-Id: Iaa47dd563b40036ff2fc2cacb70d941fd8853038
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
boost::split has a documented false-positive in clang-tidy. While
normally we'd handle this with NOLINTNEXTLINE, this doesn't appear to
work in all cases. Unclear why, but seems to be due to some of our
lambda callback complexity.
Each of these uses is a case where we should be using a more specific
check, rather than split, but for the moment, this is the best we have.
Tested: clang-tidy passes.
[1] https://github.com/llvm/llvm-project/issues/40486
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I144c6610cb740287b7225e2be03b4142a64f9563
|
|
In the continual quest to get tidy passing when run in isolation, fix
some more includes.
This includes removing a circular #include to app.hpp. We don't use
app.hpp in these files, which is why our code compiles but having this
include it here causes a few circular dependencies
app.hpp -> http_server.hpp -> persistent_data.hpp -> app.hpp.
app.hpp -> http_server.hpp -> authentication.hpp -> app.hpp.
This confuses clang when run on header files directly.
Fix a couple more includes at the same time.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib62d78b49c7e38ef7061c9fbbf6b3d463f11917d
|
|
This function is declared in a header, it should be inline, not static.
Tested: Code compiles and passes clang-tidy
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I6b05e3e302d11a64f97f9f444eb0dbc76db3cd70
|
|
This reverts commit 369ea3ffb0c76c33c7ccd0bbb0e8dcb0039cd285.
bmcweb bumps are failing romulus qemu CI tests.
This started with https://gerrit.openbmc.org/c/openbmc/openbmc/+/60786.
https://gerrit.openbmc.org/c/openbmc/openbmc/+/60756 passed.
Only 1 commit diff here.
The manager call is failing here:
```
curl -k -H "X-Auth-Token: $token" -X GET
https://${bmc}/redfish/v1/Managers/bmc
{
"@odata.id": "/redfish/v1/Managers/bmc",
"@odata.type": "#Manager.v1_14_0.Manager",
...
"UUID": "0623b376-dc4f-4a29-93e0-cc982bfb9aae",
"error": {
"@Message.ExtendedInfo": [
{
"@odata.type": "#Message.v1_1_1.Message",
"Message": "The request failed due to an internal service error.
The service is still operational.",
"MessageArgs": [],
"MessageId": "Base.1.13.0.InternalError",
"MessageSeverity": "Critical",
"Resolution": "Resubmit the request. If the problem persists,
consider resetting the service."
}
],
"code": "Base.1.13.0.InternalError",
"message": "The request failed due to an internal service error.
The service is still operational."
}
}
```
Let's get the bumps back to passing.
Change-Id: Ia27b1a5024b480786cc776c4ab9586bd75bf1242
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
This was used way back in the day when bmcweb static files were
ungzipping their contents. Today we push that to the client.
Remove.
Tested: Code compiles. Unused file.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie798b5eeb8854a677725d90adead8c1bf00bb5f4
|
|
There are currently many files that use the get endpoints methods[1].
Since they are general methods, they are defined in the
dbus_utility.hpp file and will be further refactored in subsequent
patches.
Also, Updated the populateSoftwareInformation method of sw_utils.hpp
[1] https://github.com/openbmc/docs/blob/master/architecture/object-mapper.md#associations
When an object with, for example, an object path of pathA uses
the following values:
["foo", "bar", "pathB"]
The mapper will create 2 new objects:
pathA/foo
pathB/bar
Tested: Built bmcweb successuflly and Validator passes
curl -k -H "X-Auth-Token: $token" -X GET
https://${bmc}/redfish/v1/Managers/bmc
{
"@odata.id": "/redfish/v1/Managers/bmc",
"@odata.type": "#Manager.v1_14_0.Manager",
...
"FirmwareVersion": "2.14.0-dev-95-gea3949e76-dirty",
...
}
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: I6567f63ab63709504b46ed49b00055a8ffc34124
|
|
Since the GetSubTree method has been implemented in dbus_utility and
this commit is to integrate all the places where the GetSubTree
method is called, and use the method in dbus_utility uniformly.
Tested: Redfish Validator Passed
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: If3852b487d74e7cd8f123e0efffbd4affe92743c
|
|
mTLS authentication should have the highest priority (according to code
in [1]) so it shouldn't be affected by cookies. If you provide a valid
certificate and a dummy cookie value, request will fail which means
cookies had higher priority than mTLS.
Tested:
Follow the guide in [2] to create a valid certificate for a user that
can access some resource (for example /redfish/v1/Chassis) and make two
requests:
curl --cert client-cert.pem --key client-key.pem -vvv --cacert
CA-cert.pem https://BMC_IP/redfish/v1/Chassis
curl --cert client-cert.pem --key client-key.pem -vvv --cacert
CA-cert.pem https://BMC_IP/redfish/v1/Chassis -H "Cookie: SESSION=123"
Before this change second request would fail with "401 Unauthorized"
[1]: https://github.com/openbmc/bmcweb/blob/bb759e3aeaadfec9f3aac4485f253bcc8a523e4c/include/authentication.hpp#L275
[2]: https://github.com/openbmc/docs/blob/f4febd002df578bad816239b70950f84ea4567e8/security/TLS-configuration.md
Signed-off-by: Karol Niczyj <karol.niczyj@intel.com>
Signed-off-by: Boleslaw Ogonczyk Makowski <boleslawx.ogonczyk-makowski@intel.com>
Change-Id: I5d6267332b7b97c11f638850108e671d0baa26fd
|