<div dir="ltr">Hi Ed,<div><br></div><div>Thanks for the email with great details!</div><div><br></div><div>I came to realize that it was my misunderstanding/assumption which caused the confusion. </div><div>I've reviewed the bmcweb TLS documentation and learned mTLS was only one authentication option other than a requirement.</div><div><br></div><div>Zhenfei</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Jun 7, 2020 at 7:49 PM Ed Tanous <<a href="mailto:ed@tanous.net">ed@tanous.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">>> I did more testing and found the reason why it accepts any client<br>
certification.<br>
<br>
It looks like you never got a great answer to this.<br>
<br>
There's a slight conflict between needs here.  On the one hand, bmcweb<br>
needs to support multiple authentication mechanisms, some of which are<br>
compatible with standards that are more or less set in stone (Redfish,<br>
Dbus-rest api, ect).  On the other hand, a lot of people looking to<br>
turn on mutual TLS auth are doing so to reduce the scope of code they<br>
have to "trust" for authentication down to only the SSL library, which<br>
(hopefully) is rigorously tested.  The problem arises here that there<br>
are modes, like Redfish and the webui, that require certain assets to<br>
be available without authentication.  In the case of Redfish, it<br>
requires the introspectable schema files, in the case of the webui,<br>
the static pages that make it up need to be loaded so the UI launches<br>
and the user sees a login page.  (Unrelated note, we make more than is<br>
needed available here, but that's a different problem.)<br>
<br>
When I first built the patch to do mutual TLS, my intention was to at<br>
least try to support as many authentication mechanisms as I could,<br>
hence the code you're looking at now that only uses the mutual TLS<br>
auth as a _possible_ authentication mechanism, leaving the final<br>
decision be made by the auth code in bmcweb.  One thing that seems to<br>
have gotten lost in translation somewhere between that code and when<br>
it hit master is that if mutual TLS is the only enabled authentication<br>
mechanism at that point in time, we know that we're not operating in<br>
any standards that would require static assets, and bmcweb can simply<br>
deny the connection on the front end, like you would expect, in the<br>
code that you've already found.<br>
<br>
TL;DR;<br>
<br>
Add something like this:<br>
<br>
// Get the current auth config<br>
AuthConfigMethods& methods =<br>
crow::persistent_data::SessionStore::getInstance().getAuthMethodsConfig();<br>
// if only mTLS is enabled, we can close the connection immediately,<br>
as no other auth methods will be tried.<br>
if (methods. xtoken == false &&<br>
methods.cookie == false &&<br>
methods.sessionToken == false &&<br>
methods.basic = false &&<br>
methods.tls == true){<br>
    return false;<br>
}<br>
<br>
Here:<br>
<a href="https://github.com/openbmc/bmcweb/blame/master/http/http_connection.h#L302" rel="noreferrer" target="_blank">https://github.com/openbmc/bmcweb/blame/master/http/http_connection.h#L302</a><br>
<br>
...and I suspect it'll work like you want.<br>
</blockquote></div>