Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redfish: Allow disabling SSH and HTTP (the redirect) #1763

Closed
gtmills opened this issue Feb 25, 2020 · 32 comments
Closed

Redfish: Allow disabling SSH and HTTP (the redirect) #1763

gtmills opened this issue Feb 25, 2020 · 32 comments
Assignees
Labels
prio_high_2 ReadyForDev Stories ready for development work
Milestone

Comments

@gtmills
Copy link

gtmills commented Feb 25, 2020

This story is to enhance BMCWeb to allow the SSH interface to be disabled. The existing Redfish REST API GET /redfish/v1/Managers/bmc/NetworkProtocolshows the SSH.ProtocolEnabled property. This must be enhanced to allow PATCH. The underlying D-Bus interface is https://github.com/openbmc/phosphor-dbus-interfaces/blob/master/xyz/openbmc_project/Control/Service/README.md

This is similar to #513.
The original story also included the need to disable HTTP (the redirect). If we have the redirect we should be able to disable it.

@rfrandse rfrandse added this to the 9.5.2 milestone Feb 26, 2020
@mzipse mzipse added the ReadyForDev Stories ready for development work label Feb 27, 2020
@rfrandse rfrandse modified the milestones: 9.5.2, A.1.202 Feb 28, 2020
@rfrandse rfrandse modified the milestones: A.1.202, A.1.203 Mar 24, 2020
@rfrandse rfrandse modified the milestones: A.1.203, A.1.204 Jun 25, 2020
@XiaochaoMa XiaochaoMa assigned XiaochaoMa and unassigned XiaochaoMa Jul 16, 2020
@joseph-reynolds
Copy link

This is similar to #612

@gtmills
Copy link
Author

gtmills commented Aug 27, 2020

@joseph-reynolds This is assigned to epic #612.. If you want to close this and track this someother way. feel free.

@gtmills gtmills changed the title Redfish: Allow disabling SSH and HTTPS Redfish: Allow disabling SSH and HTTP (the redirect) Sep 8, 2020
@gtmills
Copy link
Author

gtmills commented Sep 8, 2020

We probably won't need disabling HTTPS but @nicoleconser can you do user research on?

@joseph-reynolds
Copy link

Is this for HTTP (redirect) or HTTPS (disable BMCWeb including all REST APIs)?
Is there a use case to disable HTTPS?

@gtmills
Copy link
Author

gtmills commented Jan 5, 2021

Is this for HTTP (redirect) or HTTPS (disable BMCWeb including all REST APIs)?

This is for HTTP (redirect)

Is there a use case to disable HTTPS?

No

@mzipse
Copy link
Contributor

mzipse commented Jan 9, 2021

@zhanghaodi , according to the status I saw recently, it sounds like there is a commit up for review for this Issue. Can you post that commit in this issue? Thanks.

@zhanghaodi
Copy link

@gtmills
Copy link
Author

gtmills commented Jan 13, 2021

https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/39006

Is for disabling HTTPS.
This issue calls for "disabling SSH and HTTP (the redirect)"

@gtmills
Copy link
Author

gtmills commented Jan 13, 2021

https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/39006

Is for disabling HTTPS.
This issue calls for "disabling SSH and HTTP (the redirect)"

We don't have a use case for disabling HTTPS but we are fine either way if it goes in. That doesn't resolve the issue though.

@zhanghaodi
Copy link

@gtmills I am currently experiencing a problem, there is no D-Bus API related to SSH in
https://github.com/openbmc/service-config-manager/blob/1f3813f819f11b27f515891c239113f0b4e60936/src/main.cpp#L37.
After I add it at the end of the list: "obmc-console", "dropbear" };
I see that the Enabled property of SSH obtained through the busctl command is false,but the Redfish get shows it to be Enabled.

@gtmills
Copy link
Author

gtmills commented Jan 28, 2021

@joseph-reynolds Any thoughts here?

@joseph-reynolds
Copy link

joseph-reynolds commented Feb 15, 2021

Here are my ideas how to debug:

@joseph-reynolds
Copy link

@zhanghaodi I added a comment to https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/39006 asking what problem you are trying to solve. Did @gtmills already answer this question? We want to allow the BMC admin to control SSH and HTTP.

@zhanghaodi
Copy link

@mzipse @joseph-reynolds I have tried the above Debug method, this is the result I got :
My D-Bus object name is "dropbear" exactly; Consistent with the name used by BMCWeb's implementation of GET /redfish/v1/Managers/bmc/NetworkProtocol/
The interface gets the correct data,and there is no bug BMCWeb's getData() method.
After adding the D-Bus object dropbear, there is an interface called xyz.openbmc_project.Control.Service.Attributes interface under the corresponding service,the property of "Enabled" is false,but the SSH service is enabled.The added D-Bus object dropbear is not effective or there is a bug.When I add the D-Bus object dropbear,do I need to do other preparations to make it effective?Or there are some bugs in this service,how can I eliminate these bugs.

@joseph-reynolds
Copy link

@zhanghaodi I have some time to work on this issue. What is blocking this? In your most recent post dated Feb 23, you indicate the D-Bus API is showing dropbear and its current Enabled property (as true or false). Then when you "Set" the Enabled property, it has no effect. Is that correct?

I do not fully understand how this is supposed to work.

  1. Do you need a corresponding change to add "dropbear" to the serviceNames in service-config-manager? https://github.com/openbmc/service-config-manager
  2. I remember having seen a gerrit review which enhanced the service-config-manager code with the the new dropbear value, but I cannot locate that code now. Will you provide a link?
  3. I see the abandoned bmcweb review 39006. Do you have a similar review for ssh / dropbear?
  4. (Obviously), to test the bmcweb change, you would have to have in an image with the new service-config-manager. It seems to me that the service-config-manager change can be tested independently so it should be done before the bmcweb change.

@zhanghaodi
Copy link

@mzipse @joseph-reynolds Exactly,currently it is like this.According to
https://github.com/openbmc/phosphor-dbus-interfaces/tree/master/xyz/openbmc_project/Control/Service
When the property Masked of xyz.openbmc_project.Control.Service.Attributes interface is false:Running is true,the service is running,and the "ProtocolEnabled" property of SSH is true (Obtained via redfish),is this correct?
But actually,Running is false,and the "ProtocolEnabled" property of SSH is true (Obtained via redfish).
The questions about you, here are my thoughts,
1.I think that add dropbear to the array serviceNames
in https://github.com/openbmc/service-config-manager/blob/1f3813f819f11b27f515891c239113f0b4e60936/src/main.cpp#L37.
The service is effected,I have tested that the Masked property is effective.And I found a very strange phenomenon.
When I set the Running property true, Running property automatically changes to false after a period of time.

root@fp5280g2:~# busctl call xyz.openbmc_project.Control.Service.Manager /xyz/openbmc_project/control/service/dropbear org.freedesktop.DBus.Properties Set ssv xyz.openbmc_project.Control.Service.Attributes Running b true
root@fp5280g2:~# busctl introspect xyz.openbmc_project.Control.Service.Manager /xyz/openbmc_project/control/service/dropbear
NAME                                                 TYPE      SIGNATURE RESULT/VALUE FLAGS
org.freedesktop.DBus.Introspectable                  interface -         -            -
.Introspect                                          method    -         s            -
org.freedesktop.DBus.Peer                            interface -         -            -
.GetMachineId                                        method    -         s            -
.Ping                                                method    -         -            -
org.freedesktop.DBus.Properties                      interface -         -            -
.Get                                                 method    ss        v            -
.GetAll                                              method    s         a{sv}        -
.Set                                                 method    ssv       -            -
.PropertiesChanged                                   signal    sa{sv}as  -            -
xyz.openbmc_project.Control.Service.Attributes       interface -         -            -
.Enabled                                             property  b         false        emits-change writable
.Masked                                              property  b         false        emits-change writable
.Running                                             property  b         true         emits-change writable
xyz.openbmc_project.Control.Service.SocketAttributes interface -         -            -
.Port                                                property  q         22           emits-change writable
root@fp5280g2:~# busctl introspect xyz.openbmc_project.Control.Service.Manager /xyz/openbmc_project/control/service/dropbear
NAME                                                 TYPE      SIGNATURE RESULT/VALUE FLAGS
org.freedesktop.DBus.Introspectable                  interface -         -            -
.Introspect                                          method    -         s            -
org.freedesktop.DBus.Peer                            interface -         -            -
.GetMachineId                                        method    -         s            -
.Ping                                                method    -         -            -
org.freedesktop.DBus.Properties                      interface -         -            -
.Get                                                 method    ss        v            -
.GetAll                                              method    s         a{sv}        -
.Set                                                 method    ssv       -            -
.PropertiesChanged                                   signal    sa{sv}as  -            -
xyz.openbmc_project.Control.Service.Attributes       interface -         -            -
.Enabled                                             property  b         false        emits-change writable
.Masked                                              property  b         false        emits-change writable
.Running                                             property  b         false        emits-change writable
xyz.openbmc_project.Control.Service.SocketAttributes interface -         -            -
.Port                                                property  q         22           emits-change writable

There are other processes affecting Running,I am looking for the reasons for this phenomenon, can you provide some ideas?
2.Regarding the gerrit comment you mentioned, I have no impression of it.
3.The test didn't pass, so I didn't submit it.What I want to do is similar to
https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/32705
4.Year,What I am doing is an independent test of the service-config-manager change.The changes to the bmcweb part are implemented based on the changes in this part.

@joseph-reynolds
Copy link

I sounds like you made a lot of progress already and got stuck on the service-config-manager implementation. Let's work on that.

Your command output shows the dropbear Running property going from true -> false. I believe the problem is dropbear uses socket activation. That means the dropbear socket is listening (dropbear.socket) but the service itself (dropbear@.service) only runs when needed. For example, you can use these commands to control whether dropbear accepts new connections:

  • systemctl stop dropbear.socket
  • systemctl start dropbear.socket

I think the next step is to change service-config-manager so it understands how work with socket activation. I have no experience in that area, so I cannot offer help. We may want to ask for help from the maintainers. It would help me and other members of the OpenBMC community to see the code for this. Can you push your code to gerrit review? Please mark it work-in-progress (WIP) so reviewers will know the code is not yet finished.

@zhanghaodi
Copy link

Below are my service-config-manager changes,
https://gerrit.openbmc-project.xyz/c/openbmc/service-config-manager/+/42072

@joseph-reynolds
Copy link

I emailed details about the problem to the openbmc email list and mentioned it on Discord, but I have not gotten any response.

@zhanghaodi
Copy link

@mzipse @joseph-reynolds
Thank you, how should we solve this problem now,
Can this issue be temporarily blocked?
Open this issue after the service-config-manager problem is solved.

@mzipse
Copy link
Contributor

mzipse commented Apr 23, 2021

@zhanghaodi , Joseph has reached out to the community and maintainers for help on this issue but unfortunately after a couple of weeks, we have not yet received any response. So, at this point, I would like to ask you or someone on your team to dig a little deeper into the Service Config Manager code to understand why it is failing. It appears that there is a bug in this code. I would suggest opening a github issue against https://github.com/openbmc/service-config-manager/issues and track and progress there. Joseph has provided some guidance 16 days earlier (above) that might shed some light on this problem. If you are unable to figure this out, we'll have to put this on hold for a while until someone from our team frees up to be able to dig into this. Any help you can provide would be much appreciated.

@joseph-reynolds
Copy link

There is discussion in the gerrit review about this: https://gerrit.openbmc-project.xyz/c/openbmc/service-config-manager/+/42072

@zhanghaodi
Copy link

@mzipse @joseph-reynolds
Our team is interested in this and will devote energy to research Service Config Manager code to understand why it is failing.
Thank you very much for the discussion in 42072. We will continue to study in this direction and continue the discussion.

@mzipse mzipse modified the milestones: A.1.211, A.1.212.6 May 10, 2021
@lxwinspur
Copy link

@mzipse @joseph-reynolds
Around disabling SSH, we update the srvcfg-manager code and pushed a new patch, Please review these.
https://gerrit.openbmc-project.xyz/q/topic:%22disabling+SSH%22+(status:open%20OR%20status:merged)

You are about to disable BMC shell access via SSH. No SSH shell connections can be made, but existing connections will still work.
You are about to enable BMC shell access via SSH. Users will be able to connect to the BMC shell via ssh and attempt to authenticate.

@joseph-reynolds Around this issue, I have a question:
Why need to keep the existing connections when we disabling SSH? I think we need to disable all connections(include existing).
So what is the intended design and idea?

Note:
keep existing connections(https://gerrit.openbmc-project.xyz/c/openbmc/service-config-manager/+/43476/2)
disabling existing connections(https://gerrit.openbmc-project.xyz/c/openbmc/service-config-manager/+/43476/3)

Waiting for your reply, Thanks :)

@joseph-reynolds
Copy link

joseph-reynolds commented May 28, 2021

I believe the direction is: all existing connections are dropped when a service is disabled.
That corresponds to patchset 3. (I approved patchset 5.)

@rfrandse rfrandse modified the milestones: A.1.212.6, A.1.214 Jun 18, 2021
@joseph-reynolds
Copy link

For SSH:

@lxwinspur
Copy link

@joseph-reynolds
Thanks a lot.
About the service-config-manager, wait for the maintainer to review it.

@lxwinspur
Copy link

lxwinspur commented Jul 14, 2021

@gtmills
Copy link
Author

gtmills commented Jul 14, 2021

@joseph-reynolds @mzipse Should we close this and track the HTTP redirect separately?

@mzipse
Copy link
Contributor

mzipse commented Jul 15, 2021

Yes. Adriana is pulling in Milton's http redirect work downstream sometime in the next week. This is tracked under #895.

@mzipse mzipse closed this as completed Jul 15, 2021
@gtmills
Copy link
Author

gtmills commented Jul 15, 2021

@mzipse @joseph-reynolds Does #895 include a way to disable/enable the direct? Do we need that?

@anoo1
Copy link

anoo1 commented Jul 16, 2021

There's a bmcweb change almost ready to be merged that would implement http redirect: https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/35265

This can be used instead of pulling in the phosphor-misc redirect package. We can track which one we'll be using under #895

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
prio_high_2 ReadyForDev Stories ready for development work
Projects
None yet
Development

No branches or pull requests

8 participants