x509-cert: provide parsing profiles #987

baloo · 2023-04-05T16:42:48Z

This allow the user to relax checks when parsing certificate and cover for non rfc5280 compliant x509 certificates.

Fixes #978
Fixes #984

cc @lumag

x509-cert/src/certificate.rs

x509-cert/tests/certificate.rs

tarcieri · 2023-04-18T00:13:53Z

@baloo this needs a rebase

der/derive/src/attributes.rs

baloo · 2023-04-18T03:29:51Z

I guess I will need to split the der/derive changes in another PR, get those released and need a PR to fix both fuzz and the minimal-version checks
see #1008

the parsing profiles (RustCrypto#987) rely on a PhantomData field to specify the underlying profile used when parsing. This is specified like: ``` rust pub struct TbsCertificate<P: Profile = Rfc5280> { // ... #[asn1(skipped = "Default::default")] pub(crate) _profile: PhantomData<P>, } ```

tarcieri · 2023-04-18T13:00:23Z

Let me take a stab at PhatomData handling with derive...

baloo · 2023-04-18T18:33:16Z

Yeah, I was focusing on providing a EncodeValue implementation, and missed that I needed Encode itself.

That works, I also needed DerOrd and ValueOrd.
~~this is rebased on #1012~~

tarcieri · 2023-04-19T13:40:16Z

Hmm, so #1012 didn't fix the issue? Does it pass locally?

tarcieri · 2023-04-19T16:27:10Z

This one is a straight up test failure: https://github.com/RustCrypto/formats/actions/runs/4736068365/jobs/8427760079?pr=987

baloo · 2023-04-19T16:27:54Z

yes, I deleted my previous comment and trying to figure out what is happening right now.

baloo · 2023-04-19T16:49:47Z

Because I dropped the handling for generics in the derive at the same time as I dropped the handling for the skipped fields.
I still need the der_derive to handle type generics.

tarcieri · 2023-04-19T17:12:28Z

@baloo aah yeah, to properly handle generics it should probably be using Generics::split_for_impl

baloo · 2023-04-19T18:32:08Z

@tarcieri injecting a default lifetime with split for impl in wasn't trivial but this is the best I came up with. I'd love your opinion

baloo · 2023-04-20T03:24:03Z

alright so that solved that.

Folks seems to be relying on `TbsCertificate` directly, we should avoid an API break as much as possible.

lumag · 2023-04-20T13:55:35Z

@baloo this now breaks building certificates:

error: cannot construct `TbsCertificateInner` with struct literal syntax due to private fields
   --> src/cert_gen.rs:105:43
    |
105 |     let tbs_certificate: TbsCertificate = TbsCertificate {
    |                                           ^^^^^^^^^^^^^^
    |
    = note: ... and other private field `_profile` that was not provided

error: could not compile `foo` due to previous error

tarcieri · 2023-04-20T14:02:24Z

Oof, that seems bad. That's another argument for keeping the generic parameters off every type except Certificate

lumag · 2023-04-20T14:07:18Z

Exporting the PhantomData should work

tarcieri · 2023-04-20T14:10:28Z

yes, albeit a bit ugly to have PhantomData in a public API

lumag · 2023-04-20T14:26:23Z

For me this is not a big problem, since I will most likely rewrite my code to use CertificateBuilder. But you should at least be aware of thi problem.

baloo · 2023-04-20T15:24:58Z

We can either revert the thing for now and rework that, or get a constructor (new) for the TbsCertificate ~~and the Certificate~~.

baloo · 2023-04-20T15:45:00Z

or we just get rid of the phantom data there ... #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]) 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)

Fixes RustCrypto#1149 This broke with the parsing profiles (RustCrypto#987)

Fixes #1149 This broke with the parsing profiles (#987)

This commit brings the profiles introduced in RustCrypto#987 to `TrustAnchorChoice` and `Crl`. This is intended for the support of invalid certificates in https://github.com/carl-wallace/rust-pki/tree/main/certval

This commit brings the profiles introduced in #987 to `TrustAnchorChoice` and `Crl`. This is intended for the support of invalid certificates in https://github.com/carl-wallace/rust-pki/tree/main/certval

tarcieri reviewed Apr 5, 2023

View reviewed changes

x509-cert/src/certificate.rs Outdated Show resolved Hide resolved

baloo force-pushed the baloo/x509/parsing-profile branch from 93b81ca to a540a6e Compare April 5, 2023 16:51

tarcieri reviewed Apr 5, 2023

View reviewed changes

x509-cert/src/certificate.rs Outdated Show resolved Hide resolved

baloo force-pushed the baloo/x509/parsing-profile branch from a540a6e to e4c3332 Compare April 5, 2023 16:54

baloo commented Apr 5, 2023

View reviewed changes

x509-cert/tests/certificate.rs Outdated Show resolved Hide resolved

baloo force-pushed the baloo/x509/parsing-profile branch 3 times, most recently from 454ff34 to 44a083e Compare April 5, 2023 21:30

tarcieri mentioned this pull request Apr 5, 2023

x509-cert/der regression #978

Closed

baloo force-pushed the baloo/x509/parsing-profile branch 2 times, most recently from 41d768e to b1f6b80 Compare April 18, 2023 03:07

tarcieri reviewed Apr 18, 2023

View reviewed changes

der/derive/src/attributes.rs Outdated Show resolved Hide resolved

baloo mentioned this pull request Apr 18, 2023

der/derive: support for skipped field #1008

Closed

baloo force-pushed the baloo/x509/parsing-profile branch from b1f6b80 to 2f97707 Compare April 18, 2023 18:32

baloo force-pushed the baloo/x509/parsing-profile branch from 2f97707 to b1b6b73 Compare April 18, 2023 19:09

baloo force-pushed the baloo/x509/parsing-profile branch from 06a53a0 to a122967 Compare April 19, 2023 17:01

baloo force-pushed the baloo/x509/parsing-profile branch from a122967 to f9da73a Compare April 19, 2023 18:31

baloo force-pushed the baloo/x509/parsing-profile branch 2 times, most recently from 65934e7 to 160caca Compare April 19, 2023 18:33

baloo added 2 commits April 19, 2023 16:12

x509-cert: use type aliases

dfa5491

x509-cert: bump der to 0.7.4

cb74b24

baloo force-pushed the baloo/x509/parsing-profile branch from c8455df to cb74b24 Compare April 19, 2023 23:13

x509-cert: provide a type alias for TbsCertificate

592db1d

Folks seems to be relying on `TbsCertificate` directly, we should avoid an API break as much as possible.

tarcieri approved these changes Apr 20, 2023

View reviewed changes

tarcieri merged commit f570f59 into RustCrypto:master Apr 20, 2023

baloo deleted the baloo/x509/parsing-profile branch April 20, 2023 15:25

baloo mentioned this pull request May 3, 2023

x509-cert: v0.2.2 #1040

Merged

baloo added a commit to baloo/formats that referenced this pull request Jul 12, 2023

x509-cert: fixup Arbitrary for certificates

2aa57f7

Fixes RustCrypto#1149 This broke with the parsing profiles (RustCrypto#987)

baloo mentioned this pull request Jul 12, 2023

x509-cert: fixup Arbitrary for certificates #1150

Merged

tarcieri pushed a commit that referenced this pull request Jul 12, 2023

x509-cert: fixup Arbitrary for certificates (#1150)

6964132

Fixes #1149 This broke with the parsing profiles (#987)

baloo mentioned this pull request May 12, 2024

x509-cert: accept rfc5280-invalid certificates as trustanchor #1403

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x509-cert: provide parsing profiles #987

x509-cert: provide parsing profiles #987

baloo commented Apr 5, 2023 •

edited

Loading

tarcieri commented Apr 18, 2023

baloo commented Apr 18, 2023 •

edited

Loading

tarcieri commented Apr 18, 2023

baloo commented Apr 18, 2023 •

edited

Loading

tarcieri commented Apr 19, 2023

tarcieri commented Apr 19, 2023

baloo commented Apr 19, 2023

baloo commented Apr 19, 2023

tarcieri commented Apr 19, 2023

baloo commented Apr 19, 2023

baloo commented Apr 20, 2023

lumag commented Apr 20, 2023 •

edited

Loading

tarcieri commented Apr 20, 2023

lumag commented Apr 20, 2023

tarcieri commented Apr 20, 2023

lumag commented Apr 20, 2023

baloo commented Apr 20, 2023 •

edited

Loading

baloo commented Apr 20, 2023

x509-cert: provide parsing profiles #987

x509-cert: provide parsing profiles #987

Conversation

baloo commented Apr 5, 2023 • edited Loading

tarcieri commented Apr 18, 2023

baloo commented Apr 18, 2023 • edited Loading

tarcieri commented Apr 18, 2023

baloo commented Apr 18, 2023 • edited Loading

tarcieri commented Apr 19, 2023

tarcieri commented Apr 19, 2023

baloo commented Apr 19, 2023

baloo commented Apr 19, 2023

tarcieri commented Apr 19, 2023

baloo commented Apr 19, 2023

baloo commented Apr 20, 2023

lumag commented Apr 20, 2023 • edited Loading

tarcieri commented Apr 20, 2023

lumag commented Apr 20, 2023

tarcieri commented Apr 20, 2023

lumag commented Apr 20, 2023

baloo commented Apr 20, 2023 • edited Loading

baloo commented Apr 20, 2023

baloo commented Apr 5, 2023 •

edited

Loading

baloo commented Apr 18, 2023 •

edited

Loading

baloo commented Apr 18, 2023 •

edited

Loading

lumag commented Apr 20, 2023 •

edited

Loading

baloo commented Apr 20, 2023 •

edited

Loading