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

Docs: Update documentation of auth-tls-match-cn annotation to add possible check on all DN fields #11842

Open
hichem opened this issue Aug 21, 2024 · 2 comments
Labels
area/docs kind/documentation Categorizes issue or PR as related to documentation. needs-priority triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@hichem
Copy link

hichem commented Aug 21, 2024

Hi All,

I have just a small suggestion to improve/complete the documentation of auth-tls-match-cn annotation.

The doc mentions that the annotation is used to add a sanity check on the CN of the client certificate during an mTLS handshake.

However, the sanity check may also apply to other fields of the DN of the certificate like "OU" and this might be very handy when performing checks on a group of certificates based on "OU" (group of certificate belonging to an Organizational Unit) or other criteria.

So for example, the annotation may have the following value to accept only certificates whose DN contains "'OU=FOO,OU=BAR'"fields
nginx.ingress.kubernetes.io/auth-tls-match-cn: "'OU=FOO,OU=BAR'"

This type of check is already working, having tested it recently, and the code shows indeed that the condition applies the DN, not only CN:

if ( $ssl_client_s_dn !~ {{ $server.CertificateAuth.MatchCN }} ) {
    return 403 "client certificate unauthorized";
}

Refer to https://github.com/kubernetes/ingress-nginx/blob/main/rootfs/etc/nginx/template/nginx.tmpl#L1016

Would it be possible therefore to update the annotation documentation to add these possible checks?

Thanks,

/kind documentation
/remove-kind feature

@hichem hichem added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 21, 2024
@k8s-ci-robot k8s-ci-robot added kind/documentation Categorizes issue or PR as related to documentation. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority and removed kind/feature Categorizes issue or PR as related to a new feature. labels Aug 21, 2024
@longwuyuan
Copy link
Contributor

That will cover more info and hence it will be an improvement.

Please submit a docs PR. Thank you

/triage accepted
/kind documentation
/area docs

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. area/docs and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Aug 21, 2024
@hichem
Copy link
Author

hichem commented Sep 5, 2024

Will do it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs kind/documentation Categorizes issue or PR as related to documentation. needs-priority triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
Development

No branches or pull requests

3 participants