Skip to content

verification/policy: tweak key checks#10311

Merged
alex merged 2 commits intopyca:mainfrom
trail-of-forks:tob-key-checks
Feb 1, 2024
Merged

verification/policy: tweak key checks#10311
alex merged 2 commits intopyca:mainfrom
trail-of-forks:tob-key-checks

Conversation

@woodruffw
Copy link
Copy Markdown
Contributor

This changes the valid_issuer logic slightly, to the following:

  1. The issuer's SPKI is checked instead of the child's, since all issuing certificates must confirm to CABF (previously, this would allow a root with a non-CABF-allowed key when used in root -> EE configuration)
  2. The subject's signature is still checked, and I've added a note explaining why we check the subject's signature rather than the issuer's

With these changes, I've confirmed that the existing tests pass as expected (and additionally, the new tests in the limbo PR below also newly pass).

Needs C2SP/x509-limbo#185.

Needs C2SP/x509-limbo#185.

Signed-off-by: William Woodruff <william@trailofbits.com>
Signed-off-by: William Woodruff <william@trailofbits.com>
"webpki::aki::root-with-aki-ski-mismatch",
# We allow RSA keys that aren't divisible by 8, which is technically
# forbidden under CABF. No other implementation checks this either.
"webpki::forbidden-rsa-key-not-divisable-by-8",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

NB: This test was just renamed; no behavioral change.

@alex alex merged commit e80f3ee into pyca:main Feb 1, 2024
@woodruffw woodruffw deleted the tob-key-checks branch February 1, 2024 01:18
alex pushed a commit to alex/cryptography that referenced this pull request Feb 24, 2024
* verification/policy: tweak key checks

Needs C2SP/x509-limbo#185.

Signed-off-by: William Woodruff <william@trailofbits.com>

* bump limbo

Signed-off-by: William Woodruff <william@trailofbits.com>

---------

Signed-off-by: William Woodruff <william@trailofbits.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants