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

tests: add name constraints integration test. #40

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

cpu
Copy link
Member

@cpu cpu commented Aug 9, 2023

This is a follow up to #39 that adds a basic integration test for trust anchors in webpki-roots with name constraints.

The general idea is that for each name constraints extension we:

  • parse the name constraints with x509-parser, verifying that the encoding is well formed and contains something approximating what we expect (e.g. at least one permitted subtree, no excluded subtrees).
  • convert the name constraints into the form rcgen expects for certificate generation parameters.
  • issue our own trust anchor CA certificate with the name constraints from the webpki trust anchor.
  • for each permitted subtree base dns name in the name constraints we use our generated CA to issue end entity certificates that will be permitted, and rejected by the name constraints.
  • we then translate our issued CA back to a webpki trust anchor, and use webpki to verify each of the permitted and rejected end entity certificates, asserting the result matches what we expect for the name constraint.

Checking out v0.25.1 and backporting this test identifies a problem with the KamuSM root name constraint, finding no subtrees after parsing:

running 1 test
test name_constraints ... FAILED

failures:

---- name_constraints stdout ----
thread 'name_constraints' panicked at 'empty permitted subtrees in constraints', tests/verify.rs:142:5

With the fix from #39 the test passes.

@cpu cpu self-assigned this Aug 9, 2023
Comment on lines +129 to +114
// Note: We take the cheap way out here and assume single byte length - if the following
// assert fails we'll need to more intelligently encode the sequence DER length.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a little bit shoddy but I think probably sufficient for tests 🤷

Comment on lines +147 to +132
// We don't expect any excluded subtrees as this time.
assert!(constraints.excluded_subtrees.is_none());
Copy link
Member Author

@cpu cpu Aug 9, 2023

Choose a reason for hiding this comment

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

We could revisit this if it turns out that there will be trust anchors with name constraints including excluded subtrees. There aren't any today so I didn't bother complicating the testing harness to handle them.

@cpu cpu force-pushed the cpu-name-constraint-integration-tests branch 3 times, most recently from d817342 to 99c5e77 Compare August 9, 2023 14:38
@cpu
Copy link
Member Author

cpu commented Aug 9, 2023

cpu force-pushed the cpu-name-constraint-integration-tests branch from d817342 to 99c5e77

Sorry for the force pushes. Just tidying up small things (stale comments, bad var names, etc) I noticed reviewing this after opening the PR. All done now.

@cpu cpu changed the title tests: add name constraint integration test. tests: add name constraints integration test. Aug 9, 2023
Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Nice! Some hopefully simplifying suggestions...

tests/verify.rs Outdated Show resolved Hide resolved
tests/verify.rs Outdated Show resolved Hide resolved
tests/verify.rs Outdated Show resolved Hide resolved
tests/verify.rs Outdated Show resolved Hide resolved
tests/verify.rs Show resolved Hide resolved
This commit adds integration testing for trust anchors in webpki-roots
with name constraints.

The general idea is that for each name constraints extension we:

* parse the name constraints with x509-parser, verifying that the
  encoding is well formed and contains something approximating what we
  expect (e.g. at least one permitted subtree, no excluded subtrees).
* convert the name constraints into the form rcgen expects for
  certificate generation parameters.
* issue our own trust anchor CA certificate with the name constraints
  from the webpki trust anchor.
* for each permitted subtree base dns name in the name constraints we
  use our generated CA to issue end entity certificates that will be
  permitted, and rejected by the name constraints.
* we then translate our issued CA back to a webpki trust anchor, and use
  webpki to verify each of the permitted and rejected end entity
  certificates, asserting the result matches what we expect for the name
  constraint.
@cpu cpu force-pushed the cpu-name-constraint-integration-tests branch from 99c5e77 to 4c59e8a Compare August 9, 2023 14:57
@cpu
Copy link
Member Author

cpu commented Aug 9, 2023

Going to merge this w/ one review since it's only test code. Thanks!

@cpu cpu merged commit 82a1e42 into rustls:main Aug 9, 2023
1 check passed
@cpu cpu deleted the cpu-name-constraint-integration-tests branch August 9, 2023 15:47
@cpu cpu mentioned this pull request Nov 22, 2023
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.

2 participants