-
Notifications
You must be signed in to change notification settings - Fork 131
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: builder updates #1001
Conversation
3346519
to
e96fd0f
Compare
@baloo another question: would it be useful to accept |
7a94229
to
a858334
Compare
{ | ||
/// Creates a new certificate builder | ||
pub fn new<Signature>( | ||
profile: Profile, | ||
version: Version, |
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.
good call. Thank you!
.to_extension(tbs)?, | ||
); | ||
extensions | ||
.push(KeyUsage(KeyUsages::KeyCertSign | KeyUsages::CRLSign).to_extension(tbs)?); |
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.
KeyUsages::DigitalSignature
is getting dropped, I don't remember why I added it in the first place.
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.
@baloo it is reasoned in the commit message. Please review by the commit.
A couple potential options here:
|
@tarcieri I think the option 1 sounds better. In fact I think there is option 3: switch to |
I think we could also potentially do both. Having a |
That would save a round trip to der serialization, and it's more consistent with the signer. Up to you. |
(the rest of the changes are okay with me, I've pulled the changes over to |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Without this, x509-cert::builder::Error can not be used in the pattern of Box<dyn Error>, which is pretty common. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
…keys Per RFC5280, DigitalSignature 'is asserted when the subject public key is used for verifying digital signatures, other than signatures on certificates (bit 5) and CRLs (bit 6)'. Using CA keys to sign random data would definitely be a bad practice and should be avoided. Thus remove the DigitalSignature keyUsage from these certificates. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
RSA PSS implements DynSignatureAlgorithmIdentifier only for the SigningKey, not for the verifying key. To allow using CertificateBuilder with RSA PSS keys require DynSignatureAlgorithmIdentifier implementation on S rather than on S::VerifyingKey. This also follows the following logic: verifying key can possibly verify several kinds of signatures, while for the signing key we must know exact signature kind and parameters. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signer (unlike SignerMut) is not expected to be mutable. Don't require mutability of the Signer argument. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
ECDSA keys can not be used for keyEncipherment. Make this keyUsage bit optional. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Add Eq to the list of derived traits to make clippy happy. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
There are no dependencies on the "derive" featue of the signature crate. Drop it now. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
…rsion Follow the rules from RFC 5280 Section 4.1.2.1 to set the certificate's version depending on the presence of the extensions and identifiers. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Remove unused conversion when building RDN fields. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
After rereading RFC5280 I found that I was incorrect regarding the |
@lumag can you rebase? |
done |
FWIW, this should allow RSASSA-PSS with the |
Yes, I saw that, thank you. I'm going to push a chane adding |
Added - Certificate builder ([RustCrypto#764]) - Support for `RandomizedSigner` in builder ([RustCrypto#1007]) - Provide parsing profiles ([RustCrypto#987]) - Support for `Time::INFINITY` ([RustCrypto#1024]) - Conversion from `std::net::IpAddr` ([RustCrypto#1035]) - `CertReq` builder ([RustCrypto#1034]) Changed - use `ErrorKind::Value` for overlength serial ([RustCrypto#988]) - Bump `hex-literal` to v0.4.1 ([RustCrypto#999]) - Builder updates ([RustCrypto#1001]) - better debug info when `zlint` isn't installed ([RustCrypto#1018]) - make SKI optional in leaf certificate ([RustCrypto#1028]) - bump rsa from 0.9.0-pre.2 to 0.9.0 ([RustCrypto#1033]) Fixed - fix `KeyUsage` bit tests ([RustCrypto#993]) - extraneous PhantomData in `TbsCertificate` ([RustCrypto#1019])
Added - Certificate builder ([RustCrypto#764]) - Support for `RandomizedSigner` in builder ([RustCrypto#1007]) - Provide parsing profiles ([RustCrypto#987]) - Support for `Time::INFINITY` ([RustCrypto#1024]) - Conversion from `std::net::IpAddr` ([RustCrypto#1035]) - `CertReq` builder ([RustCrypto#1034]) Changed - use `ErrorKind::Value` for overlength serial ([RustCrypto#988]) - Bump `hex-literal` to v0.4.1 ([RustCrypto#999]) - Builder updates ([RustCrypto#1001]) - better debug info when `zlint` isn't installed ([RustCrypto#1018]) - make SKI optional in leaf certificate ([RustCrypto#1028]) - bump rsa from 0.9.0-pre.2 to 0.9.0 ([RustCrypto#1033]) Fixed - fix `KeyUsage` bit tests ([RustCrypto#993]) - extraneous PhantomData in `TbsCertificate` ([RustCrypto#1017])
Added - Certificate builder (RustCrypto#764) - Support for `RandomizedSigner` in builder (RustCrypto#1007) - Provide parsing profiles (RustCrypto#987) - Support for `Time::INFINITY` (RustCrypto#1024) - Conversion from `std::net::IpAddr` (RustCrypto#1035) - `CertReq` builder (RustCrypto#1034) - missing extension implementations (RustCrypto#1050) - notes about `UTCTime` range being 1970-2049 (RustCrypto#1052) Changed - use `ErrorKind::Value` for overlength serial (RustCrypto#988) - Bump `hex-literal` to v0.4.1 (RustCrypto#999) - Builder updates (RustCrypto#1001) - better debug info when `zlint` isn't installed (RustCrypto#1018) - make SKI optional in leaf certificate (RustCrypto#1028) - bump rsa from 0.9.0-pre.2 to 0.9.0 (RustCrypto#1033) - bump rsa from 0.9.1 to 0.9.2 (RustCrypto#1056) Fixed - fix `KeyUsage` bit tests (RustCrypto#993) - extraneous PhantomData in `TbsCertificate` (RustCrypto#1017) - CI flakiness (RustCrypto#1042) - usage of ecdsa signer (RustCrypto#1043)
Added - Certificate builder (RustCrypto#764) - Support for `RandomizedSigner` in builder (RustCrypto#1007) - Provide parsing profiles (RustCrypto#987) - Support for `Time::INFINITY` (RustCrypto#1024) - Conversion from `std::net::IpAddr` (RustCrypto#1035) - `CertReq` builder (RustCrypto#1034) - missing extension implementations (RustCrypto#1050) - notes about `UTCTime` range being 1970-2049 (RustCrypto#1052) - consume the `SignatureBitStringEncoding` trait (RustCrypto#1048) Changed - use `ErrorKind::Value` for overlength serial (RustCrypto#988) - Bump `hex-literal` to v0.4.1 (RustCrypto#999) - Builder updates (RustCrypto#1001) - better debug info when `zlint` isn't installed (RustCrypto#1018) - make SKI optional in leaf certificate (RustCrypto#1028) - bump rsa from 0.9.0-pre.2 to 0.9.0 (RustCrypto#1033) - bump rsa from 0.9.1 to 0.9.2 (RustCrypto#1056) Fixed - fix `KeyUsage` bit tests (RustCrypto#993) - extraneous PhantomData in `TbsCertificate` (RustCrypto#1017) - CI flakiness (RustCrypto#1042) - usage of ecdsa signer (RustCrypto#1043)
Added - Certificate builder (#764) - Support for `RandomizedSigner` in builder (#1007) - Provide parsing profiles (#987) - Support for `Time::INFINITY` (#1024) - Conversion from `std::net::IpAddr` (#1035) - `CertReq` builder (#1034) - missing extension implementations (#1050) - notes about `UTCTime` range being 1970-2049 (#1052) - consume the `SignatureBitStringEncoding` trait (#1048) Changed - use `ErrorKind::Value` for overlength serial (#988) - Bump `hex-literal` to v0.4.1 (#999) - Builder updates (#1001) - better debug info when `zlint` isn't installed (#1018) - make SKI optional in leaf certificate (#1028) - bump rsa from 0.9.0-pre.2 to 0.9.0 (#1033) - bump rsa from 0.9.1 to 0.9.2 (#1056) Fixed - fix `KeyUsage` bit tests (#993) - extraneous PhantomData in `TbsCertificate` (#1017) - CI flakiness (#1042) - usage of ecdsa signer (#1043)
I started rebasing my code to use
CertificateBuilder
and I had to make several useful modifications.Note: It is still not possible to create RSA PSS certificates, since there is no
Signer
implementation for RSA PSS, onlyRandomizedSigner
(there is no standard detereministic RSA PSS signature process). Locally I have changed theCertificateBuilder
to acceptRandomizedSigner
instead of bareSigner
, however this doesn't look like an universal solution too.