Add nc testcases for an empty permittedSubtrees - #658
Conversation
RFC 5280 4.2.1.10 gives GeneralSubtrees as SEQUENCE SIZE (1..MAX), so a present-but-empty permittedSubtrees is malformed. Independently, 6.1.4 (g)(1) intersects the permitted state with the extension's permittedSubtrees, and 6.1.3 (b) requires each name to fall inside the result, so an empty permittedSubtrees admits no names at all. webpki::nc::intermediate-permitted-excluded-subtrees-both-empty-sequences already covers the case where both subtree fields are empty. This adds the case where only permittedSubtrees is empty, alongside a control chain that differs only in that field being non-empty.
cpu
left a comment
cpu
left a comment
There was a problem hiding this comment.
The premise and test case seem OK to me and produce the results i'd expect downstream in rustls-webpki. I think ideally the PR description and comments could use some human editing.
Match the length and structure of the pre-existing testcases in the file: chain block, then what the certificate carries. Drops the cross-reference between the paired cases and the RFC 6.1.x walkthrough. No functional change; limbo.json is updated only in the two description fields. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Edited both docstrings down. I measured the existing ones in the file first — median 9 lines, max 12 without an RFC block quote — and the new ones now sit at 14 and 10, with the same structure: chain block, then what the certificate carries. The cross-reference to the paired testcase is gone; the names carry that. The PR description is rewritten too. Code is unchanged, and the harness gives identical results before and after. |
C2SP/x509-limbo#658 landed, so rfc5280::nc::permitted-empty-sequence-excluded-nonempty and rfc5280::nc::permitted-nonempty-excluded-nonempty cover these two cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI pins x509-limbo to 972626160c26b45426bbd8c935a605219bd93207 (2026-08-27), which predates C2SP/x509-limbo#658, so the new cases are not in the vectors CI runs against yet. Without this test extension.rs:754-756 is uncovered and the coverage gate fails. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![pyca-boringbot[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
Cherry-picked from pyca#15580 at Alex's suggestion on pyca#15560: the new pin 21cc053 is the merge commit for C2SP/x509-limbo#658, so the two new name-constraint testcases are in the vectors CI runs. (cherry picked from commit 319a8a4) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Sorry, only catching up to this today: I suspect like #660 these need a https://x509-limbo.com/testcases/rfc5280/#rfc5280ncpermitted-nonempty-excluded-nonempty and https://x509-limbo.com/testcases/rfc5280/#rfc5280ncpermitted-nonempty-excluded-nonempty |
Mirrors permitted-empty-sequence-excluded-nonempty from C2SP#658: a non-empty permittedSubtrees alongside an excludedSubtrees that is present but empty. Both shapes are rejected by RFC 5280 4.2.1.10, but only the first had a vector behind it. limbo.json carries only the new entry; the existing testcases are untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Noting one downstream consequence of #661: with It's the positive control of the pair: it verifies that a valid chain is accepted. With it skipped, nothing in limbo runs there to catch the FAILURE case passing for the wrong reason. It's also the first of the 19 pedantic-flagged cases to expect SUCCESS; the other 18 are FAILURE. |
|
I guess there's no harm to removing the pedantic flag in the positive case. But it seems very marginal to me. @avalyset Going forwards, please engage with us directly. It's not clear the LLM you're using has a full grasp of the problem or solution space here. |
|
Fair — I'll leave it as is then. And understood. |
4 participants
Adds two
rfc5280::nctestcases for apermittedSubtreesthat is present but empty.webpki::nc::intermediate-permitted-excluded-subtrees-both-empty-sequencesalready covers both subtree fields being empty, which an implementation can reject by treating the whole extension as degenerate. Pairing an emptypermittedSubtreeswith a non-emptyexcludedSubtreescloses that shortcut, since the extension still carries a constraint to enforce.permitted-empty-sequence-excluded-nonemptyis that chain and must be rejected, because an empty permitted set admits no names.permitted-nonempty-excluded-nonemptyis the same chain with a non-emptypermittedSubtreesand must validate, which separates accepting the empty case from failing both for an unrelated reason. Split out from pyca/cryptography#15560, where @alex preferred this be covered as an x509-limbo testcase rather than against the Rust APIs; pyca/cryptography 50.0.1 accepts the invalid case today.