search

ibm-openbmc / dev

Product Development Project Mgmt and Tracking
16 stars 2 forks source link

D-Bus service: disable BMC-attached USB ports #1793

Closed joseph-reynolds closed 2 years ago

joseph-reynolds commented 4 years ago

This story is to provide a D-Bus service to expose the BMC's physically-attached USB ports, identifying each of them, showing if the port is enabled or disabled, and allowing each port to become enabled or disabled. There are several pieces of work:

  1. Bring a USB driver into the BMC image. TODO: For a flash driver? A UPS power supply?
  2. Configure the USB ports in the BMC's device tree.
  3. Provide a way to enable and disable each USB port. When disabled, the port must not respond to any device plugged into it.
  4. Create a D-Bus interface to model the BMC's USB ports.
    • Identify each USB port and provide a list of them.
    • Show if each port is enabled or disabled.
    • Allow each port to be changed to the enabled or disabled state.

This is blocking https://github.com/ibm-openbmc/dev/issues/1781, the equivalent BMCWeb Redfish function.

joseph-reynolds commented 4 years ago

For example, if a security bug was found in the BMC's USB driver, the admin may want to disable BMC-attached USB ports as a mitigation until a fix was applied. I assume the function to disable USB would involve unloading the USB driver.

This may interact with designs that use the USB, for example USB code update: https://github.com/ibm-openbmc/dev/issues/1874

geissonator commented 3 years ago

Per request from Ruby, please default the USB ports to on (gpiochip0 115 to 0)

ghost commented 3 years ago

Are Rainier and Everest identical on the requirement and the actual USB ports and mappings?

eddiejames commented 3 years ago

Everest has both USB controllers wired with one port each. Rainier has a single USB controller wired up to one port

joseph-reynolds commented 3 years ago

IBM systems have two sets of USB ports:

ChicagoDuan commented 2 years ago

Hi @BMC-Bruce What is the progress of this task? I need to rely on the D-BUS interface provided by this task. I didn't find this commit on gerrit. Can you post the link here? thank you

ghost commented 2 years ago

Hello @lxwinspur and @ChicagoDuan, I will share tomorrow what I have, once it is stabilized and tested a bit more.

-- Bruce

ghost commented 2 years ago

I Slacked @joseph-reynolds and we came to agreement we will Enable or Disable all the BMC's USB ports as one operation; so there is no different then between Rainier and Everest. Also the default is Enabled, the intent of Disabled is really just for the customers that really want the BMC's USB ports Disabled.

ghost commented 2 years ago

Hello @lxwinspur and @ChicagoDuan,

Here is the first PR you will be needing Adds D-Bus service: disable BMC-attached USB ports #25 https://github.com/ibm-openbmc/phosphor-dbus-interfaces/pull/25

-- Bruce

ghost commented 2 years ago

@rfrandse or possibly @geissonator

Not sure how to create a PR for https://github.com/BMC-Bruce/phosphor-state-manager BRANCH bmcusb since I had to fork from https://github.com/openbmc/phosphor-state-manager as https://github.com/ibm-openbmc/phosphor-state-manager hasn't been updated since Jul 30, 2019.

geissonator commented 2 years ago

We're using upstream phosphor-state-manager and I'm hoping we can keep doing this for a while. I don't see any reason we can't upstream this function from the start?

ghost commented 2 years ago

Ok. I'll do that.

ghost commented 2 years ago

Hello @lxwinspur and @ChicagoDuan,

The interface will be:

obmcutil usbstate
obmcutil usbenable
obmcutil usbdisable

Look at the other existing obmcutil to see how they work.

joseph-reynolds commented 2 years ago

The D-Bus interfaces for this are in review here: https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/47180

ghost commented 2 years ago

The D-Bus interfaces for this are in review here: https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/47180

I think Patrick Williams is making some good points in this review and is really asking what is the threat(s) we are wanting to protect against (and which threat(s) we are NOT wanting to protect against) with the controlling of the USB ports?

ghost commented 2 years ago

Also was discussed in the OpenBMC Security Working Group meeting minutes that was held 2021-09-29 can be found here: https://docs.google.com/document/d/1b7x9BaxsfcukQDqbvZsU2ehMq4xoJRQvLxxsDUWmAOI/edit#heading=h.e2d35kl5v04s

And I have asked Brad to add this topic to the OpenBMC Design Reviews on Tuesday October 5, 2021. On Slack: https://ibm-systems-power.slack.com/archives/C0Q6TQP5Z/p1632851597462400 and https://ibm-systems-power.slack.com/archives/C1HT3FHNK/p1633356973429100

ghost commented 2 years ago

From @mzipse

For your awareness. I don’t believe I agree with Brad’s ( @bradbishop )comment on disabling the USB ports is just preventing a USB Code Update. It’s more than that.

Maurice Zipse  [10:02 AM]
Hi Adam.  Question for you….   Many moons ago, I seem to recall discussing with our Design team the requirements about disabling the USB ports.  And I was thinking that you had made some comments about PHYP needing to know if those ports are turned off or not.  Does that ring a bell to you?  Or is there a better person to talk to?  We're trying to make sure we have our requirements straight when it comes to disabling USB ports.

Adam Stallman  [10:03 AM]
Yes, this is communicated to PHYP in HDAT

Maurice Zipse  [10:04 AM]
ok.  So there is a real requirement that is implemented today on FSP based systems.
[10:04 AM] That gives me enough to work with.  I'll let you know if more input is needed.

Adam Stallman  [10:04 AM]
yes, a user can disable USB on the system.  This was a Line Item back on P9 (I think)

Maurice Zipse  [10:04 AM]
thx

Adam Stallman  [10:04 AM]
PHYP is expecting the value to be correctly set in HDAT
ghost commented 2 years ago

@eddiejames does our USB driver handle this is communicated to PHYP in HDAT and does PHYP is expecting the value to be correctly set in HDAT actually happen?

eddiejames commented 2 years ago

No it doesn't, but I suspect Adam was talking about host USB ports? don't see why PHYP cares about BMC usb ports...?

ghost commented 2 years ago

Thanks @eddiejames. @mzipse is that your take on this was well?

ghost commented 2 years ago

Hello @lxwinspur and @ChicagoDuan,

There was a Decision: Create OEM Redfish API to “disable USB code update”. For now, just disable the USB code update service (which is actually not even done yet). Extra points to disable additional interfaces and services which use USB.

This is just to keep you updated on the refinement of this story and any impacts may need to be taken into account for story Redfish: USB Port Enable / Disable #1781

Please let me know if you have any questions or concerns.

ChicagoDuan commented 2 years ago

This is just to keep you updated on the refinement of this story and any impacts may need to be taken into account for story Redfish: USB Port Enable / Disable #1781

Please let me know if you have any questions or concerns.

Okay, so we don't need to do https://github.com/ibm-openbmc/dev/issues/1781 anymore？

ghost commented 2 years ago

Okay, so we don't need to do #1781 anymore？

I believe #1781 still needs to be done, but since is is redefined I see it as implying #1781 needs to be redefined as well. USB Port Enable / Disable from Redfish still has to happen.

See https://github.com/ibm-openbmc/dev/issues/2819 for indication of the direction the Web UI is going and think how it has to use Redfish to do that task.

ChicagoDuan commented 2 years ago

Okay, so we don't need to do #1781 anymore？

I believe #1781 still needs to be done, but since is is redefined I see it as implying #1781 needs to be redefined as well. USB Port Enable / Disable from Redfish still has to happen.

See #2819 for indication of the direction the Web UI is going and think how it has to use Redfish to do that task.

Okay. Thank you

ghost commented 2 years ago

I am unable to update Rainer or Everest systems BMC firmware, thus this slipping.

ChicagoDuan commented 2 years ago

Hello @lxwinspur and @ChicagoDuan,

There was a Decision: Create OEM Redfish API to “disable USB code update”. For now, just disable the USB code update service (which is actually not even done yet). Extra points to disable additional interfaces and services which use USB.

  • Utilize https://github.com/openbmc/service-config-manager to disable/enable the service
  • Make it like how we enable/disable ssh via Redfish via bmcweb
  • Just move this work item under the USB code update work since that's required to have something to disable

Hi @BMC-Bruce， It seems that UsbCodeUpdate service is not a daemon, but an app that will be called after inserting a USB flash disk. So we cannot use service-config-manager to enable or disable it. Please see the code: https://gerrit.openbmc-project.xyz/q/topic:%22usb-code-update%22+(status:open%20OR%20status:merged)

ghost commented 2 years ago

Hello @lxwinspur and @ChicagoDuan,

How is the "inserting a USB flash disk" detected?

  1. I believe that is where we would need disable operations so the signal of "inserting a USB flash disk" doesn't happen so that the app does not get called.
  2. Or enhance the app to listen and read the D-Bus message for Disabling the USB Updates.

Which do you think is easy to do? Safer to do?

Thanks!

-- Bruce

ChicagoDuan commented 2 years ago

Hello @lxwinspur and @ChicagoDuan,

How is the "inserting a USB flash disk" detected?

Using udev rules. https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-bmc-code-mgmt/+/48236/2/usb/70-bmc-usb.rules

  1. I believe that is where we would need disable operations so the signal of "inserting a USB flash disk" doesn't happen so that the app does not get called.

I think we can disable/enable UsbCodeUpdate by deleting/creating the rules file under /etc/udev/rules.d. Do you think this is OK?

  1. Or enhance the app to listen and read the D-Bus message for Disabling the USB Updates.

There is no dbus object about the UsbCodeUpdate. And when the service is not started, the service-config-manager will not create a path to manager the UsbCodeUpdate. So I don't think this direction is available.

ghost commented 2 years ago

Hello @lxwinspur and @ChicagoDuan,

I am OK with it, let's check with @joseph-reynolds and @anoo1

-- Bruce

ghost commented 2 years ago

D-Bus service: disable BMC-attached USB ports #1793 is being CLOSED, as we are not really disabling USB Ports but just induvial services or features.

See Redfish: USB Port Enable / Disable #1781 for other control of the USB Port features.