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

rfc5280: Add more name constraint tests #44

Merged
merged 9 commits into from
Oct 20, 2023
Merged

Conversation

tetsuo-cpp
Copy link
Contributor

@tetsuo-cpp tetsuo-cpp commented Oct 18, 2023

Closes #38

Long term we can do more, but I think this covers most of the main stuff.

limbo/testcases/_core.py Show resolved Hide resolved
limbo/testcases/rfc5280.py Show resolved Hide resolved
critical=False,
)
)
intermediate = builder.intermediate_ca(root)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe subject also needs to be set here, otherwise this chain will be invalid for unrelated reasons (leaf is signed by intermediate, but leaf.issuer != intermediate.subject) 🙂

Copy link
Collaborator

Choose a reason for hiding this comment

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

I did this in 0f2deaa, but confusingly it was already passing for OpenSSL 🤔 -- OpenSSL ought to have rejected the previous chain, unless I'm missing something.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good point. Thanks for fixing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure why the original chain would be passing via OpenSSL as I think you're correct. I'm not sure how our pyca code responds to it as it's not convenient to test right now (branches have diverged).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, hmm. I'm going to link this out to our testcase tracking issue; there's possibly some invalid OpenSSL behavior here that we'll want to test for.

Signed-off-by: William Woodruff <william@trailofbits.com>
Merge branch 'main' into alex/name-constraint-tests

Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
@tetsuo-cpp tetsuo-cpp merged commit 39bec78 into main Oct 20, 2023
6 checks passed
@tetsuo-cpp tetsuo-cpp deleted the alex/name-constraint-tests branch October 20, 2023 03:53
@woodruffw woodruffw mentioned this pull request Oct 20, 2023
24 tasks
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.

Improve testing around name constraints
2 participants