openbmc / bmcweb

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

EthernetInterface under Manager should require ConfigureManager privilege #209

Closed joseph-reynolds closed 2 years ago

joseph-reynolds commented 3 years ago

Describe the bug BMCWeb implements a Redfish EthernetInterface at URI /redfish/v1/Managers/bmc/EthernetInterfaces/eth0. Permission to access this URI is controlled by Redfish PrivilegeRegistry entry for EthernetInterface which (for example) specifies the ConfigureComponents privilege is needed to perform the PATCH operation. So BMCWeb hard-coded this into the implementation of /redfish/v1/Managers/bmc/EthernetInterfaces/

However, this permission value does not take into account the PrivilegeRegistry SubordinateOverride (Redfish spec DSP0266 > Security details > Authorization > Redfish service operation-to-privilege mapping > Subordinate override) which shows that when the EthernetInterface is subordinate to a Manager->EthernetInterfaceCollection then the PATCH operation shall require the ConfigureManager privilege. In this case (referring to the URI above), the "bmc" is a Manager, and the EthernetInterfaces is an EthernetInterfaceCollection.

So, the various hard-coded permissions should be changed to match the PrivilegeRegistry (change several from ConfigureComponents to ConfigureManager). When this is done, only a BMC Administrator will be able to modify the network interfaces.

Environment What OpenBMC platform was this found on? What specific OpenBMC versions did you use?

To Reproduce Steps to reproduce the behavior: Not applicable. Create an Administrator user and an Operator user, and try to work with an EthernetInterface.

Is this a regression I believe this was the behavior from the beginning of time.

gtmills commented 3 years ago

Check out https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/43939

gtmills commented 3 years ago

https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/45469/4/redfish-core/lib/ethernet.hpp should resolve this

edtanous commented 2 years ago

Believe the aforementioned patchset should've resolved this. Closing as fixed.