Skip to content

fix: enforce strict URI SAN validation for X.509-SVID parsing#421

Merged
maxlambrecht merged 1 commit intospiffe:mainfrom
maxlambrecht:fix/x509-leaf-validation-uri-sans
Mar 26, 2026
Merged

fix: enforce strict URI SAN validation for X.509-SVID parsing#421
maxlambrecht merged 1 commit intospiffe:mainfrom
maxlambrecht:fix/x509-leaf-validation-uri-sans

Conversation

@maxlambrecht
Copy link
Member

@maxlambrecht maxlambrecht commented Mar 25, 2026

What

  • Require X.509-SVID leaf certificates to contain exactly one URI SAN during X509Svid parsing
  • Add focused tests covering rejection of leaf certificates with zero or multiple URI SANs
  • Harden key usage helpers to return false when the KeyUsage extension is absent
  • Extend test fixtures to generate certificates with custom URI SANs and without KeyUsage

Why

Align X.509-SVID leaf parsing with the SPIFFE X.509-SVID requirement that a leaf certificate contain exactly one URI SAN.

Also make shared key usage helpers null-safe.

How tested

  • Ran ./gradlew test

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR tightens X.509 SVID handling by enforcing leaf-certificate constraints during validation and by rejecting certificates that contain multiple URI SANs when extracting the SPIFFE ID, aligning parsing and validation logic.

Changes:

  • Enforce leaf X.509 SVID constraints during verifyChain / verifySpiffeId validation.
  • Reject certificates containing multiple URI SANs during SPIFFE ID extraction.
  • Refactor X.509 SVID parsing/validation code paths to share common checks (via X509SvidChecks).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
java-spiffe-core/src/main/java/io/spiffe/svid/x509svid/X509SvidValidator.java Adds leaf-constraint validation and derives trust domain from leaf SPIFFE ID.
java-spiffe-core/src/main/java/io/spiffe/svid/x509svid/X509Svid.java Refactors parsing-time validation to delegate to shared checks.
java-spiffe-core/src/main/java/io/spiffe/internal/CertificateUtils.java Makes URI SAN parsing explicit and rejects multiple URI SANs.
java-spiffe-core/src/test/java/io/spiffe/svid/x509svid/X509SvidValidatorTest.java Adds tests for invalid leaf constraints and multiple URI SAN rejection.
java-spiffe-core/src/test/java/io/spiffe/internal/CertificateUtilsTest.java Adds test ensuring multiple URI SANs are rejected when extracting SPIFFE ID.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@maxlambrecht maxlambrecht changed the title fix(x509): enforce leaf SVID validation and reject multiple URI SANs fix(x509): enforce leaf SVID validations Mar 25, 2026
@maxlambrecht maxlambrecht requested a review from Copilot March 25, 2026 19:34
@maxlambrecht maxlambrecht changed the title fix(x509): enforce leaf SVID validations fix(x509): enforce leaf SVID validations and eject multiple URI SANs Mar 25, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@maxlambrecht maxlambrecht changed the title fix(x509): enforce leaf SVID validations and eject multiple URI SANs fix(x509): enforce leaf SVID validations and reject multiple URI SANs Mar 25, 2026
}

SpiffeId spiffeId = CertificateUtils.getSpiffeId(x509Certificate);
X509SvidChecks.validateLeafConstraints(x509Certificate, spiffeId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a breaking change. The docs of X509SvidValidator.verifySpiffeId() do not constrain this method to be used only with leaf X.509-SVIDs. Today, if you pass a CA with a trust domain SPIFFE ID, e.g. spiffe://example.org, this method will not throw an exception.

What is the use case for this leaf-specific X.509-SVID validation?

Copy link
Member Author

Choose a reason for hiding this comment

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

You’re right. This should not be enforced in verifySpiffeId() as a general SPIFFE ID check, since that method is not documented as leaf-only and changing it would broaden behavior in a breaking way.

I’ll keep the leaf-specific X.509-SVID checks scoped to the X.509-SVID parsing/validation path only, and leave verifySpiffeId() unchanged.

@maxlambrecht maxlambrecht force-pushed the fix/x509-leaf-validation-uri-sans branch from c2bd558 to eb3a7da Compare March 26, 2026 19:27
@maxlambrecht maxlambrecht changed the title fix(x509): enforce leaf SVID validations and reject multiple URI SANs fix: enforce strict URI SAN validation for X.509-SVID parsing Mar 26, 2026
Signed-off-by: Max Lambrecht <maxlambrecht@gmail.com>
@maxlambrecht maxlambrecht force-pushed the fix/x509-leaf-validation-uri-sans branch from eb3a7da to 88f1ac8 Compare March 26, 2026 20:58
@maxlambrecht maxlambrecht merged commit b1f8a54 into spiffe:main Mar 26, 2026
8 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.

3 participants