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

Allow to match on other SAN fields for mTLS policies #4622

Closed

Conversation

calderonth
Copy link

Summary

This introduces more capabilities for the mTLS matching as opposed to only supporting fingerprint and spki_hash.
Indeed, this allows to match on valuable fields of the mTLS certificate such as SubjectAltName fields such as email address, dns names, ip addresses and URIs.

Related issues

Fixes #4615

User Explanation

Self-explanatory in the above section.

Checklist

  • reference any related issues
  • updated docs
  • updated unit tests
  • updated UPGRADING.md
  • add appropriate tag (improvement / bug / etc)
  • ready for review

@calderonth calderonth requested a review from a team as a code owner October 5, 2023 16:09
@calderonth calderonth requested a review from wasaga October 5, 2023 16:09
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@wasaga wasaga requested a review from kenjenkins October 5, 2023 16:50
@kenjenkins
Copy link
Contributor

Thanks for opening this PR! I'd like to see support for SAN matching conditions as well.

I think we may want to discuss the exact policy syntax internally, so please bear with us if that takes some time.

One specific question I have is whether it would make sense to use the existing StringMatcher in conjunction with the different SAN types. Maybe something like this syntax?

    - client_certificate:
        san_email:
          ends_with: "@example.com"

Although StringMatcher might not make sense for IP address SANs. It might be more natural to accept CIDR notation there; maybe something like this?

    - client_certificate:
        san_ip_address: "10.1.2.0/24"

@calderonth
Copy link
Author

calderonth commented Oct 9, 2023

Sure thing, feel free to discuss, it was essentially a first stab at it.
The IP SAN field would likely contain a /32 address, so if you'd want to to CIDR match specific matcher logic would need to be added.

@kenjenkins
Copy link
Contributor

Hi @calderonth, sorry it's taken me so long to get back to this. Two questions:

  1. Are you able to sign the CLA? If so I'll plan to make some changes based on this PR (to support the StringMatcher syntax).
  2. Do you have a specific need for IP address SAN matching? I'm wondering if we should hold off on adding support for IP addresses unless there's a concrete use case for this.

@calderonth
Copy link
Author

calderonth commented Dec 15, 2023 via email

@kenjenkins
Copy link
Contributor

Closing in favor of #4913.

@kenjenkins kenjenkins closed this Jan 23, 2024
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.

Add more capabilities to certificate-matcher for mTLS routes
3 participants