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

CA-403767: verifyPeer can't use root CA for appliance cert check #6187

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

minglumlu
Copy link
Member

It is expected to use root CA certficate to verify an appliance's server
certificate for a xapi outgoing TLS connection.

Prior to this change, the related stunnel configurations are:
"verifyPeer=yes", and "checkHost=".

The 'verifyPeer' option of stunnel doesn't treat the CA bundle as root
CA certificates. The 'checkHost' option of stunnel only checks the
host name against the one in server certificate. In other words, the
issue is that the root CA based checking doesn't work for appliance.

This change adds 'verifyChain' for the appliance to ensure the outgoing
TLS connection from xapi will verify the appliance's server certificates
by real root CA certificate.

It is expected to use root CA certficate to verify an appliance's server
certificate for a xapi outgoing TLS connection.

Prior to this change, the related stunnel configurations are:
"verifyPeer=yes", and "checkHost=<hostname>".

The 'verifyPeer' option of stunnel doesn't treat the CA bundle as root
CA certificates. The 'checkHost' option of stunnel only checks the
host name against the one in server certificate. In other words, the
issue is that the root CA based checking doesn't work for appliance.

This change adds 'verifyChain' for the appliance to ensure the outgoing
TLS connection from xapi will verify the appliance's server certificates
by real root CA certificate.

Signed-off-by: Ming Lu <[email protected]>
Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but would like @psafont to take a look, too. Is the VerifyPeer code already in the right place for appliance connections?

Copy link
Member

@psafont psafont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: pool certificates are verified with VerifyPeer, and appliances with CheckHost. This change only changes the CheckHost case.

Adding VertifyChain makes sense because it ensures stunnel verifies the chain of trust to the trusted root certificates.
Dropping verifyPeer makes sense because it frees operators from having to install the appliances' certificates, and instead install trusted CA certificates.

@lindig
Copy link
Contributor

lindig commented Dec 17, 2024

Do we need documentation updates for this?

@robhoes
Copy link
Member

robhoes commented Dec 17, 2024

Do we need documentation updates for this?

No, this was a regression introduced on master a while ago, when the pool certificates were added. Before that, stunnel had different verify options and we used the equivalent of verifyChain.

@robhoes robhoes added this pull request to the merge queue Dec 17, 2024
Merged via the queue into xapi-project:master with commit 8550769 Dec 17, 2024
15 checks passed
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

Successfully merging this pull request may close these issues.

4 participants