Http connection calls into the filesystem for every connection request

[edtanous]

https://github.com/openbmc/bmcweb/blob/5a806649ec187926be5829f42f8f18606a3d4e75/http/http_connection.h#L286

It's not clear from inspection why this check is even here, and significantly goes against one of the bmcweb founding principals that we reduce the number of times we execute a context switch. On inspection, it isn't clear why we would need it at all, as the SSLContext will have no public keys, and return the appropriate error. Happy to put up a CR to remove it (although I'd like have issues with testing), but I'd like to verify that it's truly not needed, and I'm not missing something.

[zkurzyns]

When adaptor.set_verify_mode(boost::asio::ssl::verify_peer); is called, which is done few code lines later then every connection is done using the TLS handshake. When TLS handshake is done against a web browser then it popups up a dialog box asking you to choose a certificate that should be used for this connection. There is no point of showing this annoying popup when you already know that there are no CA installed. That is why we check if any CA certificate is in the folder.

[edtanous]

On Sun, Aug 2, 2020 at 11:53 PM Zbigniew notifications@github.com wrote:

When adaptor.set_verify_mode(boost::asio::ssl::verify_peer); is called, which is done few code lines later then every connection is done using the TLS handshake. When TLS handshake is done with a web browser then it popups up a dialog box asking you to choose a certificate that should be used for this connection.

Whomever wrote that code seems to have ignored the comment here: https://github.com/openbmc/bmcweb/blob/c94d37490b0090a2a4a0b6bad51cd8ab1339051e/include/ssl_key_handler.hpp#L315

So, when a CA is installed, all resources (including those that explicitly don't require authentication) are protected behind mtls? That seems like we're breaking the Redfish spec if mtls is enabled, no?

In this specific case, I wonder if it's better to be basing the mtls enablement entirely on the mtls enabled parameter, and having THAT be driven by the filesystem, as the SSL context is what actually controls whether or not a CA is present. That would prevent hitting the file system for every single request, and remove a small race condition right around where a CA shows up in the filesystem, but isn't installed yet.

In regards to the dialog box, I wonder if this is something we can just add to the login page in javascript? Checking a box saying "use my key" seems like a much better experience. Using the "Is a CA installed" as the preference to pop up the dialog box feels unwarranted. I could understand if it were "Is a CA installed and all other authentication mechanisms are disabled", but as it, it seems like it gets in the way. Also, I'm surprised the verify_peer parameter drives the dialog box. According to the HTTP RFC, I thought the www-authenticate header determined the dialog box, in a very similar way to basic auth. Has anyone looked at that? It seems like a better approach, and has the standard backing it up (I think). Has anyone tested the dialog box works in all the common browsers, or is it just specific to chrome?

-Ed

[zkurzyns]

So, when a CA is installed, all resources (including those that explicitly don't require authentication) are protected behind mtls?

All whitelisted Redfish endpoints should be available despite mTLS is enabled and one will not use any or valid certificate. If you have any doubts please verify it first, then create a new Issue.

It seems like a better approach, and has the standard backing it up (I think) I do not like current solution either and I am glad to hear someone has a better idea of how to implement this. Please provide link to your code change, I will verify if it works.

Has anyone tested the dialog box works in all the common browsers, or is it just specific to chrome Chrom, IE, Firefox where checked.

[edtanous]

On Mon, Aug 3, 2020 at 1:17 AM Zbigniew notifications@github.com wrote:

So, when a CA is installed, all resources (including those that explicitly don't require authentication) are protected behind mtls?

All whitelisted Redfish endpoints should be available despite mTLS is enabled and one will not use any or valid certificate. If you have any doubts please verify it first, then create a new Issue.

I wanted to make sure this was the case. Based on how it's coded, it looks like it'll pop up the dialog box for all resources, not just the ones protected by mtls, but it's very possible I've read it wrong. I was mostly just making sure this wasn't the intent, which it seems like it isn't. I'll verify and get a bug filed.

It seems like a better approach, and has the standard backing it up (I think) I do not like current solution either and I am glad to hear someone has a better idea of how to implement this. Please provide link to your code change, I will verify if it works.

I don't have a code change yet, I wanted to make sure there wasn't something critical I was missing.

Has anyone tested the dialog box works in all the common browsers, or is it just specific to chrome Chrom, IE, Firefox where checked.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[edtanous]

I believe this is still an issue on master.

https://github.com/openbmc/bmcweb/blob/6ce82fabeba1f2549b98c3ad2f8542c6879682c8/http/http_connection.hpp#L120

[edtanous]

Fixed in https://gerrit.openbmc.org/c/openbmc/bmcweb/+/72358