bmcweb icon indicating copy to clipboard operation
bmcweb copied to clipboard

mTLS currently broken outside of `TLSStrict`

Open williamspatrick opened this issue 1 year ago • 7 comments

Is this the right place to submit this?

  • [X] This is not a security vulnerability or a crashing bug
  • [X] This is not a question about how to use OpenBMC
  • [X] This is not a bug in an OpenBMC fork or a bug in code still under code review.
  • [X] This is not a request for a new feature.

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

williamspatrick avatar Nov 18 '24 17:11 williamspatrick

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.

edtanous avatar Nov 18 '24 18:11 edtanous

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.

williamspatrick avatar Nov 18 '24 19:11 williamspatrick

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.

edtanous avatar Nov 18 '24 20:11 edtanous

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

williamspatrick avatar Nov 18 '24 21:11 williamspatrick

I'd probably rather have a separate option, and at least some exploration (could be only on paper)of whether post handshake requests could solve this in a way that works in all cases. The hope is that this option doesn't have to live very long.

Maybe request_client_certificate{Unconditionally, strict_only} ?

edtanous avatar Nov 18 '24 22:11 edtanous

This is what I meant for a quick change that gets us (Meta) back to previous state:

remote: https://gerrit.openbmc.org/c/openbmc/bmcweb/+/75898 ssl_key_handler: enable verify_peer for Meta TLS mode [NEW]

williamspatrick avatar Nov 18 '24 22:11 williamspatrick

I’m actually surprised that mutual TLS is uncommon. To me, it seems by far the most secure option: if you can’t even make a connection to the BMC, you can’t exploit any vulnerabilities in the server code.

DemiMarie avatar Nov 04 '25 17:11 DemiMarie