openbmc / bmcweb

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

Errant async_method_call in http router #157

Open edtanous opened 3 years ago

edtanous commented 3 years ago

In the HTTP router, there appears to be this async_method_call;

https://github.com/openbmc/bmcweb/blob/3fad0d59b94d951a752530d01db5451773f7d374/http/routing.h#L1296

This is causing a dbus method call for every request. To my understanding, this was added because of LDAP support needing to fill in an unknown users role. That's fine, but causing a performance impact to all requests, not just those systems relying on LDAP, or when LDAP is enabled is a problem, and needs resolved.

Other notable issues: The routing layer/file should have nothing to do with looking up a users role. At a minimum this is the wrong place to put the code. It forces a dependence on phosphor-user-manager and Dbus. bmcweb is used in contexts without dbus or phosphor-user-manager (ie, openrmc). While that project is a fork, I'd like to see them joined at some point, and the more openbmc-specific restrictions we impose at the framework level, the harder that becomes. req, res, and rules are all captured by reference, which might lead to lifetime issues if the connection is closed while the UserRole lookup is occurring. std::map is less efficient than could be used.

I have very little context on what this is needed for. Does anyone have any suggestions on how these could be resolved?

It should be noted, this was checked in as a "fix" https://github.com/openbmc/bmcweb/commit/61dbeef97168db1a1f7a351c5f95e09afd361e48

The code was arguably better before, as it didn't cause a performance impact to users not using ldap, but the commit message is not clear about why exactly we had to move to what is effectively a block call.

kingzmm commented 3 years ago

@edtanous How to set up ldap service so that users in ldap can log in successfully

edtanous commented 3 years ago

Ideally we would use Pam in async mode. I’ve never had time to do that, but it on my list.

The phosphor user manager client needs to make sure that the new user is added to dbus before the Pam call is allowed to complete. After that, bmcweb should have a record of it, and it should go through just fine.

worst case, if we get a weird ordering problem (which I don’t think we would), we need to split the auth up and use an io_context::post to put the post-login activities after the messages come through and are processed.

kingzmm commented 3 years ago

@edtanous The problem I have now is that my attempt to log in to the user in the ldap service fails, and I don’t know if my connection to the ldap service is successful

kingzmm commented 3 years ago

@edtanous How to detect whether the connection to the ldap service is successful

edtanous commented 3 years ago

If the LDAP service fails to connect, the PAM conversation (in authenticate_pam) should fail, indicating the user isn't authenticated, and bmcweb should return 401.

If bmcweb authenticates via PAM, and the ldap server disconnected, that's a bug in the pam implementation that needs fixed.

kingzmm commented 3 years ago

@edtanous I did not see the verification of pam. I connected to the ldap service on the web. It seems to be through /redfish/v1/Managers/bmc/Truststore/Certificates/,I want to build ldap service now, can you give me some suggestions, thank you very much

kingzmm commented 3 years ago

@edtanous Still have trouble asking, the web interface can automatically generate the ldap service, you don’t need to build the ldap server yourself, is this okay?

edtanous commented 3 years ago

if you want to use LDAP, first you need to build a bmc with ldap support. That's outside the scope of this bug.

kingzmm commented 3 years ago

@edtanous I’m very sorry, I don’t know if my bmc supports ldap function, it should be reset to support bmc, so I just tried it, please ask, how to detect whether bmc has ldap function,thank you very much

edtanous commented 3 years ago

This is a discussion much better had on the openbmc mailing list. This bug is unrelated to your question.

kingzmm commented 3 years ago

@edtanous I'm really sorry, I really don't have a clue about the ldap service, can I trouble you to share your general steps,Many thanks

edtanous commented 3 years ago

Steps:

  1. Google "openbmc mailing list"
  2. Sign up for mailing list.
  3. Read rules for mailing list.
  4. Send email to mailing list, asking your questions.

Alternatively, you could also sign up for discord, where we have a server set up (look in the docs repo for details).

Either of those are better forums to ask questions than this unrelated bug.

kingzmm commented 3 years ago

@edtanous Thank you very much for your answer

edtanous commented 1 year ago

Note, the safety issues in this call did cause some problems recently. We still need to come up with a better solution to this. One possible fix that was at one point reverted is here:

https://gerrit.openbmc.org/c/openbmc/bmcweb/+/56081