Skip to content

Allow separate public key in CSR - #15400

Open
ronaldvanrij wants to merge 3 commits into
pyca:mainfrom
ronaldvanrij:mlkem-csr
Open

Allow separate public key in CSR#15400
ronaldvanrij wants to merge 3 commits into
pyca:mainfrom
ronaldvanrij:mlkem-csr

Conversation

@ronaldvanrij

Copy link
Copy Markdown
Contributor

This PR:

  • Allows any public key type to be included in a CSR, in particular ML-KEM public keys.

@reaperhulk reaperhulk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two comments, but also could you share the use case here? Are there specific systems that require CSR submission for public keys they'll encode in certs that are non-signature? We obviously support this in certs, but I'm curious what platforms require submission via CSR for this.

def tbs_certrequest_bytes(self) -> bytes: ...
@property
def is_signature_valid(self) -> bool: ...
def validate(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This needs to be documented

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking at this more, why not refactor is_signature_valid to have it take an optional public key to handle this case?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It can'tm because it's a property. But I do think it'd be more appropriate to name this verify_directly_signed_by or something so what it's checking is clear.

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.

Indeed, I considered allowing an optional public key (as with CertificateRevocationList) , but that would break existing code because the function would always evaluate to a truthy value.

I've renamed the validate function to verify_directly_signed_by, and aligned its behaviour with Certificate.verify_directly_issued_by: made the public key mandatory and raise exceptions instead of returning a boolean. I've also added appropriate test cases.

@@ -0,0 +1,44 @@
-----BEGIN PUBLIC KEY-----

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All the vectors need to be documented in test-vectors.rst.

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.

I wasn't aware of that, I now have added descriptions.

@ronaldvanrij

Copy link
Copy Markdown
Contributor Author

The primary use case is when both digital signature and encryption are used in digital communication, for example in AS2. With the caveat here that both ends need full control over their private keys so only they can read the content and assert integrity and origin.

Within the Dutch Government we have a similar standard called Digikoppeling, which requires the use of both digital signature and payload encryption and enforces certificates issued by specific entities, which are part of the PKIoverheid ecosystem. To request certificates, organizations need to generate their own key pairs and issue a CSR towards said entities so they can issue appropriate certificates.

Both digital signature and encryption can currently be done using RSA, but with the PQC algorithms having only a single purpose, logically they will need to generate two key pairs (signing and non-signing) and request the two associated certificates.

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.

3 participants