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

check wildcard mTLS alt names #12068

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

lgfa29
Copy link
Contributor

@lgfa29 lgfa29 commented Feb 15, 2022

#11089 introduced strict server role mTLS role check for Raft requests.

As pointed out in this comment, this change caused an issue for mTLS setups that use wildcard alt name domains, as they would fail this new check.

This PRs changes the mTLS verification to allow wildcard domains in addition to an equal match.

where wildcard values in the certificate alternative name list are not
accepted.

This breaking change will be fixed in a future version of Nomad.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A follow-up PR will update this line with the proper version where this was fixed.

Copy link
Member

Choose a reason for hiding this comment

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

Let's also add:

It is recommended to avoid wildcards and use different certificates for clients and servers.

Comment on lines +1073 to 1075
name: "globs without role",
cn: "*.global.nomad",
canRaft: false,
Copy link
Member

Choose a reason for hiding this comment

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

Why doesn't this work? A CN of *.global.noamd should satisfy the server.global.nomad requirement unless CN's never support wildcards?

Comment on lines +101 to +102
Raft requests were not being verified for their certificate common alternative
name. A check was introduced to make sure only servers in the same region are
Copy link
Member

Choose a reason for hiding this comment

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

their certificate common alternative name.

Did you mean Common Name or Subject Alternative Name? Or maybe I'm forgetting a term.

Raft requests were not being verified for their certificate common alternative
name. A check was introduced to make sure only servers in the same region are
able to make Raft requests. This additional check results in a breaking change
where wildcard values in the certificate alternative name list are not
Copy link
Member

Choose a reason for hiding this comment

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

Let's add an example:

where wildcard values (eg *.global.nomad) in the certificate...

where wildcard values in the certificate alternative name list are not
accepted.

This breaking change will be fixed in a future version of Nomad.
Copy link
Member

Choose a reason for hiding this comment

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

Let's also add:

It is recommended to avoid wildcards and use different certificates for clients and servers.

@lgfa29 lgfa29 removed this from the 1.3.0 milestone Apr 20, 2022
@lgfa29 lgfa29 marked this pull request as draft April 20, 2022 19:47
@tgross tgross added the stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/needs-rebase This PR needs to be rebased on main before it can be backported to pick up new BPA workflows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants