Skip to content
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: adds a CertReq builder #1034

Merged
merged 3 commits into from
May 3, 2023
Merged

Conversation

baloo
Copy link
Member

@baloo baloo commented May 1, 2023

Fixes #1031

Generates CSR like:

---- certificate_request stdout ----
Certificate Request:
    Data:
        Version: 1 (0x0)
        Subject: CN = service.domination.world
        Subject Public Key Info:
            Public Key Algorithm: id-ecPublicKey
                Public-Key: (256 bit)
                pub:
                    04:1c:ac:ff:b5:5f:2f:2c:ef:d8:9d:89:eb:37:4b:
                    26:81:15:24:52:80:2d:ee:a0:99:16:06:81:37:d8:
                    39:cf:7f:c4:81:a4:44:92:30:4d:7e:f6:6a:c1:17:
                    be:fe:83:a8:d0:8f:15:5f:2b:52:f9:f6:18:dd:44:
                    70:29:04:8e:0f
                ASN1 OID: prime256v1
                NIST CURVE: P-256
        Attributes:
            Requested Extensions:
                X509v3 Subject Alternative Name:
                    IP Address:192.0.2.0
    Signature Algorithm: ecdsa-with-SHA256
    Signature Value:
        73:29:74:17:23:a1:12:f4:94:82:3e:b8:8b:eb:f4:21:f4:80:
        7e:ba:1a:0e:63:b4:ea:03:5e:6c:87:43:f1:01:6d:19:34:3c:
        1c:f8:e7:0f:d0:1b:c4:5b:20:b5:80:46:dd:73:0a:84:b4:94:
        00:5e:a3:31:81:21:70:3b:57:a5
-----BEGIN CERTIFICATE REQUEST-----
MIH5MIGnAgEAMCMxITAfBgNVBAMMGHNlcnZpY2UuZG9taW5hdGlvbi53b3JsZDBZ
MBMGByqGSM49AgEGCCqGSM49AwEHA0IABBys/7VfLyzv2J2J6zdLJoEVJFKALe6g
mRYGgTfYOc9/xIGkRJIwTX72asEXvv6DqNCPFV8rUvn2GN1EcCkEjg+gIjAgBgkq
hkiG9w0BCQ4xEzARMA8GA1UdEQQIMAaHBMAAAgAwCgYIKoZIzj0EAwIDQQBzKXQX
I6ES9JSCPriL6/Qh9IB+uhoOY7TqA15sh0PxAW0ZNDwc+OcP0BvEWyC1gEbdcwqE
tJQAXqMxgSFwO1el
-----END CERTIFICATE REQUEST-----

@baloo baloo marked this pull request as draft May 1, 2023 19:51
@baloo
Copy link
Member Author

baloo commented May 1, 2023

cc @lumag

x509-cert/tests/builder.rs Outdated Show resolved Hide resolved
@lumag
Copy link
Contributor

lumag commented May 1, 2023

@baloo maybe I miss something, why do you have to pass extensions to the to_extension()?

@baloo
Copy link
Member Author

baloo commented May 1, 2023

@baloo maybe I miss something, why do you have to pass extensions to the to_extension()?

Because some extensions may require that, like Basic Constraints (https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.9):

Conforming CAs MUST include this extension in all CA certificates
   that contain public keys used to validate digital signatures on
   certificates and MUST mark the extension as critical in such
   certificates.  This extension MAY appear as a critical or non-
   critical extension in CA certificates that contain public keys used
   exclusively for purposes other than validating digital signatures on
   certificates.  Such CA certificates include ones that contain public
   keys used exclusively for validating digital signatures on CRLs and
   ones that contain key management public keys used with certificate
   enrollment protocols.  This extension MAY appear as a critical or
   non-critical extension in end entity certificates.

for now, the behavior of the crate is to mark BasicConstraints as critical, but I prefer to leave the door open if we need to revisit that.

@lumag
Copy link
Contributor

lumag commented May 1, 2023

I think we should check/fix that at the build time. For now I'd suggest skipping that (and it definitely isn't a part of CSR builder patchset).

@baloo
Copy link
Member Author

baloo commented May 1, 2023

Not a fan of breaking the API in the future if we ever need to have the other extensions to make a decision :/

@lumag
Copy link
Contributor

lumag commented May 1, 2023

Anyway, it should be a separate change (separate PR or patch inside the PR). Having everything spliced into a single patch makes review very hard. I'd suggest landing the RequestBuilder first and then taking a glance on the to_extension in a separate PR.

Regarding the quoted part. I think we should check that at the build time, depending on the profile/whatsoever, rather than enforcing it at the add_extensions time.

@baloo
Copy link
Member Author

baloo commented May 1, 2023

@lumag split that in its own commit.

@baloo baloo force-pushed the baloo/x509-cert/request-builder branch from 3a1fc2a to ac60e56 Compare May 1, 2023 21:04
@baloo
Copy link
Member Author

baloo commented May 1, 2023

🤦 and I did push to origin not my fork

@baloo baloo force-pushed the baloo/x509-cert/request-builder branch from ac60e56 to be6d26f Compare May 1, 2023 21:43
@baloo
Copy link
Member Author

baloo commented May 1, 2023

@lumag last commit is what I meant with the trait Builder

@baloo baloo force-pushed the baloo/x509-cert/request-builder branch 2 times, most recently from f057d61 to d7eaa09 Compare May 1, 2023 21:57
@lumag
Copy link
Contributor

lumag commented May 1, 2023

@baloo the request builder an the trait looks good. As for the fist commit, I'd leave that up to you and @tarcieri .

@baloo baloo force-pushed the baloo/x509-cert/request-builder branch from d7eaa09 to 472b209 Compare May 1, 2023 22:53
This reverts commit ee0e150edc3695e757ba19d7dae7a70496006ecd.
@baloo baloo force-pushed the baloo/x509-cert/request-builder branch from 472b209 to 182ee5b Compare May 1, 2023 22:56
@baloo baloo marked this pull request as ready for review May 1, 2023 22:57
@baloo baloo force-pushed the baloo/x509-cert/request-builder branch from 182ee5b to 10d428f Compare May 1, 2023 23:22
@baloo baloo force-pushed the baloo/x509-cert/request-builder branch from 10d428f to c33f1f0 Compare May 2, 2023 16:07
@baloo baloo merged commit 2030132 into master May 3, 2023
43 checks passed
@baloo baloo deleted the baloo/x509-cert/request-builder branch May 3, 2023 18:53
baloo added a commit to baloo/formats that referenced this pull request May 3, 2023
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])
@baloo baloo mentioned this pull request May 3, 2023
baloo added a commit to baloo/formats that referenced this pull request May 3, 2023
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])
baloo added a commit to baloo/formats that referenced this pull request May 10, 2023
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)
baloo added a commit to baloo/formats that referenced this pull request May 11, 2023
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)
baloo added a commit that referenced this pull request May 19, 2023
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x509-cert: Generate certificate signing request?
3 participants