Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mTLS currently broken outside of TLSStrict #296

Open
4 tasks done
williamspatrick opened this issue Nov 18, 2024 · 6 comments
Open
4 tasks done

mTLS currently broken outside of TLSStrict #296

williamspatrick opened this issue Nov 18, 2024 · 6 comments

Comments

@williamspatrick
Copy link
Member

williamspatrick commented Nov 18, 2024

Is this the right place to submit this?

  • This is not a security vulnerability or a crashing bug
  • This is not a question about how to use OpenBMC
  • This is not a bug in an OpenBMC fork or a bug in code still under code review.
  • 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 463a0e3 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
Copy link
Contributor

Thanks for filing this.

Technically 463a0e3 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
Copy link
Member Author

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
Copy link
Contributor

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 edtanous reopened this Nov 18, 2024
@williamspatrick
Copy link
Member Author

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

@edtanous
Copy link
Contributor

edtanous commented Nov 18, 2024

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} ?

@williamspatrick
Copy link
Member Author

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]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants