search

openbmc / bmcweb

A do everything Redfish, KVM, GUI, and DBus webserver for OpenBMC
Apache License 2.0
148 stars 130 forks source link

Operator user can run BMC firmware update #242

Closed thangqn-ampere closed 1 year ago

thangqn-ampere commented 1 year ago

Describe the bug Operator user can login to WebUI and start firmware update. It can also flash new firmware via Redfish.

Environment Tested on Ampere Mt.Jade and Mt.Mitchell platform with OpenBMC firmware compiled from commit d4f5137e4e0ef00870b8f1e9d19125272cfe0784.

To Reproduce Steps to reproduce the behavior:

  1. Create a new user usertest from WebUI with Operator privilege.
  2. Login to WebUI via usertest.
  3. Click on Operators -> Firmware. In Update firmware -> Image file, click Add file to select new firmware. Then select Start update to start update process.
  4. New firmware is uploaded and then update process start. After complete, the BMC boots with new firmware
  5. I expect that only Administrator user can flash the firmware.
  6. Check with Redfish over /redfish/v1/UpdateService and the process still successes.
edtanous commented 1 year ago

bmcweb implements the Redfish base privilege registry from DMTF. Can you please quote where in the privilge registry you believe we've implemented it incorrectly?

thangqn-ampere commented 1 year ago

I am not aware of privilege derived from DMTF registry. However, I check the user privilege roles at https://github.com/openbmc/docs/blob/master/architecture/user-management.md#supported-privilege-roles:

Operator: Users are allowed to view and control basic operations. This includes reboot of the host, etc.
But users are not allowed to change other configuration like user, network, etc.

Having firmware update privilege can destroy current setting from administrators and I think is outside above operator privilege definition

edtanous commented 1 year ago

Like I said, the permissions are governed by the base privilege registry published by DMTF. If you believe that the permissions should be different, feel free to take it up with Redfish, but unless we’re not implementing the privileges they publish correctly, this is not a bug.

thangqn-ampere commented 1 year ago

Can you give me some information to check on bmcweb codes that derived from DMTF registries? I will check more on it. Do you think we need to update our documents about operator user privilege to match bmcweb and DMTF registries?

edtanous commented 1 year ago

Can you give me some information to check on bmcweb codes that derived from DMTF registries?

Check out the scripts directory that builds the internal data structures we use for privilege registry.

Do you think we need to update our documents about operator user privilege to match bmcweb and DMTF registries?

Possibly, or if the DMTF registries don't meet the intent, have them changed, but it's not clear yet if this is something that openbmc implemented incorrectly, or DMTF specified incorrectly.

edtanous commented 1 year ago

@thangqn-ampere have you determined if this is a bug in bmcweb? Or in the privilege registry.

thangqn-ampere commented 1 year ago

I am sorry, we have just been back from Lunar New Year holiday. I have asked Hieu (hieuhuynh-ampere) to help me check and will update you soon.

hieuhuynh-ampere commented 1 year ago

Hi @edtanous,

OpenBMC uses the POST method for Firmware updates [1]. Based on the DMTF registries, the UpdateService's POST method has 'ConfigureComponents' privilege [2]. BMCWeb defined Operator with 'ConfigureComponents' privilege [3]. And implement Firmware update POST method with 'ConfigureComponents' privilege [4] [5].

According to my understanding, there is no issue with DMTF registries. The question is: why BMCWeb defined the Operator has 'ConfigureComponents' privilege [3]?

[1] https://github.com/openbmc/docs/blob/master/architecture/code-update/firmware-update-over-redfish.md#update-an-image [2] https://redfish.dmtf.org/registries/Redfish_1.3.1_PrivilegeRegistry.json#UpdateService [3] https://github.com/openbmc/bmcweb/blob/master/redfish-core/include/privileges.hpp#L229 [4] https://github.com/openbmc/bmcweb/blob/master/redfish-core/include/registries/privilege_registry.hpp#L1508 [5] https://github.com/openbmc/bmcweb/blob/master/redfish-core/lib/update_service.hpp#L733

edtanous commented 1 year ago

why BMCWeb defined the Operator has 'ConfigureComponents' privilege [3]?

Please reference Table 41 — Required standard roles in the redfish specification, which specifies Operator should have ConfigureComponents privilege. To my reading of the above, this is a DMTF issue, and bmcweb implements to the specification; Please file it with them and work there.