-
Notifications
You must be signed in to change notification settings - Fork 136
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
x509-cert: rework certificate builder profiles #1306
x509-cert: rework certificate builder profiles #1306
Conversation
cc @lumag |
I took a glance a the GOST certificate which is used in the signature on my Ham license. It has:
|
I don't think I've seen nonRepudiation and keyEncipherment together before as the latter often is accompanied by escrow of the private key. I've not caught up yet to current release to try it out but the PR looks OK. |
We still allow the use to set I think that's in line with the security note from ETSI:
|
That note talks about the nonRepudiation bit. However I'm more concerned about mixing of other usage bits. This extends to GOST certs which can have digitalSignature + keyEncipherment + dataEncipherment. |
Forr the reference, RFC 4055 allows keyEncipherment and dataEncipherment, but explicitly tellls that those two bits SHOULD NOT be used together. |
yeaaah, there is more to this story. I'll revisit this. There are also code signing certificates, with different requirements. |
b5dc812
to
b2b6b6d
Compare
@tarcieri can I ask for your opinion on this trait based approach? |
9fa3709
to
7213a65
Compare
@baloo as a general direction it looks great! |
7213a65
to
748dce8
Compare
ad8b0d5
to
4daa75e
Compare
4daa75e
to
aad2c72
Compare
@@ -38,6 +37,23 @@ pub enum Error { | |||
|
|||
/// Signing error propagated for the [`signature::Error`] type. | |||
Signature(signature::Error), | |||
|
|||
/// Each RelativeDistinguishedName MUST contain exactly one AttributeTypeAndValue. | |||
NonUniqueRdn, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those new error entries are mostly tied to the checks run for compliance with cabf, I could probably make a subtype?
aad2c72
to
fd4a90e
Compare
This is now providing a trait to be implemented by the consumer. A number of implementation are available, including ones trying to abide by CABF Baseline Requirements. Fixes RustCrypto#1281
fd4a90e
to
8e7d9c7
Compare
I intend to merge this tomorrow, unless someone has something to say about it. |
I tried to abide by the ETSI policy.
https://www.etsi.org/deliver/etsi_en/319400_319499/31941202/02.03.01_60/en_31941202v020301p.pdf
The policy recommends to make KeyUsage bits 0, 1, (2 and/or 4) exclusive. This is meant to avoid making signing of commitments during authentication.
This commit goes a bit further by making bit 2 and 4 exclusive. I don't find a use-case for requiring both at the same time (and I would think only key agreement is required.
Fixes #1281
Fixes #1392