Re: mTLS on bmcweb

* RE: mTLS on bmcweb
@ 2020-06-08  2:48 Ed Tanous
  2020-06-10  3:50 ` Zhenfei Tai
  0 siblings, 1 reply; 14+ messages in thread
From: Ed Tanous @ 2020-06-08  2:48 UTC (permalink / raw)
  To: openbmc

>> I did more testing and found the reason why it accepts any client
certification.

It looks like you never got a great answer to this.

There's a slight conflict between needs here.  On the one hand, bmcweb
needs to support multiple authentication mechanisms, some of which are
compatible with standards that are more or less set in stone (Redfish,
Dbus-rest api, ect).  On the other hand, a lot of people looking to
turn on mutual TLS auth are doing so to reduce the scope of code they
have to "trust" for authentication down to only the SSL library, which
(hopefully) is rigorously tested.  The problem arises here that there
are modes, like Redfish and the webui, that require certain assets to
be available without authentication.  In the case of Redfish, it
requires the introspectable schema files, in the case of the webui,
the static pages that make it up need to be loaded so the UI launches
and the user sees a login page.  (Unrelated note, we make more than is
needed available here, but that's a different problem.)

When I first built the patch to do mutual TLS, my intention was to at
least try to support as many authentication mechanisms as I could,
hence the code you're looking at now that only uses the mutual TLS
auth as a _possible_ authentication mechanism, leaving the final
decision be made by the auth code in bmcweb.  One thing that seems to
have gotten lost in translation somewhere between that code and when
it hit master is that if mutual TLS is the only enabled authentication
mechanism at that point in time, we know that we're not operating in
any standards that would require static assets, and bmcweb can simply
deny the connection on the front end, like you would expect, in the
code that you've already found.

TL;DR;

Add something like this:

// Get the current auth config
AuthConfigMethods& methods =
crow::persistent_data::SessionStore::getInstance().getAuthMethodsConfig();
// if only mTLS is enabled, we can close the connection immediately,
as no other auth methods will be tried.
if (methods. xtoken == false &&
methods.cookie == false &&
methods.sessionToken == false &&
methods.basic = false &&
methods.tls == true){
    return false;
}

Here:
https://github.com/openbmc/bmcweb/blame/master/http/http_connection.h#L302

...and I suspect it'll work like you want.

^ permalink raw reply	[flat|nested] 14+ messages in thread