openbmc / bmcweb

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

mTLS currently broken outside of `TLSStrict` #296

Open williamspatrick opened 3 hours ago

williamspatrick commented 3 hours ago

Is this the right place to submit this?

Bug Description

Running Redfish operations with mTLS enabled currently 401 Unauthorized, unless explicitly setting TLSStrict: true in the bmcweb_persistent_data.json.

curl --include --key $KEY_PATH --cert $CERT_PATH https://bmcdevice/redfish/v1/Systems/system

This was introduced with 463a0e3e880340a119bf986bab039d1afbe1a432 and appears to be fixed by reverting.

The current bmcweb compile modifications and PACKAGECONFIG can be seen at https://github.com/openbmc/openbmc/blob/56a1db99ffbce908d8e78e0f5540f5db55cb546f/meta-facebook/recipes-phosphor/interfaces/bmcweb_%25.bbappend#L4 .

Version

2.16.0-dev-2197-g3debd6217b

Additional Information

No response

edtanous commented 3 hours ago

Thanks for filing this.

Technically 463a0e3e880340a119bf986bab039d1afbe1a432 was a partial revert to fix a previous regression. To my understanding there has never been a mode where:

  1. The browser doesn't prompt for a certificate.
  2. Automation (curl etc) can optionally provide a certificate if it's available

Given that the number of users of mtls is very low and the number of people that use the webui is quite high, I picked the lesser of two evils.

Openssl man pages shows that there's an option, SSL_VERIFY_POST_HANDSHAKE, that might be able to handle this use case, but it appears to be more complex than just enabling the option, and requires some level of connection code to handle it.

Alternatively, we could try to detect the browser at the TLS connection, but a wireshark dump of the data didn't show anything obvious that we could use to identify "this is a browser".

Would love some help to fix this for all use cases.

williamspatrick commented 2 hours ago

Given that the number of users of mtls is very low and the number of people that use the webui is quite high, I picked the lesser of two evils.

Understand. Was raising this partially so it was known and tracked.

Could we add BMCWEB_META_TLS_COMMON_NAME_PARSING to that "if tlsStrict" branch? We don't ever use the webui.

edtanous commented 58 minutes ago

Could we add BMCWEB_META_TLS_COMMON_NAME_PARSING to that "if tlsStrict" branch?

I'm not sure I understand. If tls strict works, just use that? It's already runtime settable.

williamspatrick commented 2 minutes ago

Using TLS strict disables password based authentication. We would still like that as an alternative method.