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

basic Ia5String support for DistinguishedName values #182

Merged
merged 6 commits into from
Oct 30, 2023

Conversation

cpu
Copy link
Member

@cpu cpu commented Oct 27, 2023

This branch adds basic support emitting and parsing distinguished name values that are Ia5Strings. For example, email address attributes in a certificate subject distinguished name.

Note that because of #181 this code will panic when emitting invalid Ia5String values. This problem is general to rcgen's handling of ASN.1 string types and so isn't addressed with additional care in this branch. A broader rework is required.

Along the way I also fixed a warning from #176 related to where we were defining the custom profile.dev.package.num-bigint-dig profile metadata.

Resolves #180

@cpu cpu mentioned this pull request Oct 27, 2023
@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Merging #182 (8891a8c) into main (20ee1f6) will increase coverage by 0.72%.
The diff coverage is 66.66%.

❗ Current head 8891a8c differs from pull request most recent head eb9bae7. Consider uploading reports for the commit eb9bae7 to get more accurate results

@@            Coverage Diff             @@
##             main     #182      +/-   ##
==========================================
+ Coverage   72.13%   72.86%   +0.72%     
==========================================
  Files           7        7              
  Lines        1888     1861      -27     
==========================================
- Hits         1362     1356       -6     
+ Misses        526      505      -21     
Files Coverage Δ
rcgen/src/lib.rs 76.61% <66.66%> (+1.20%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@est31 est31 left a comment

Choose a reason for hiding this comment

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

see comments.

Cargo.toml Outdated Show resolved Hide resolved
rcgen/src/lib.rs Show resolved Hide resolved
rcgen/src/lib.rs Show resolved Hide resolved
cpu added 6 commits October 30, 2023 09:16
Fixes the warning:

```
warning: profiles for the non root package will be ignored, specify profiles at the workspace root:
package:   /home/daniel/Code/Rust/rcgen/rcgen/Cargo.toml
workspace: /home/daniel/Code/Rust/rcgen/Cargo.toml
```
Previously the `CertificateParams::write_request` fn duplicated code
already present in `write_distinguished_name` for serializing
a `DistinguishedName`. This commit replaces the duplicate code with
usage of that helper.
This commit updates the `DnValue` enum to add a variant for `Ia5String`
values in distinguished names, and adds support for serializing it.
These are often used for email address attributes since the printable
string type can't contain a `@` character.

Note that like existing `DnValue` variants, we accept a `String` but the
ASN.1 specification of the encoding constrains allowed values further.
Writing a `DnValue::Ia5String` that contains non-ascii characters will
panic, similar to how using a `DnValue::PrintableString` with the
current code using a value with characters outside of the allowed set
will panic.

In the future we should consider rewriting the `DnValue` enum to better
enforce the character set constraints of each variant at construction
time.
This commit updates the logic for converting from the x509-parser
distinguished name types into the rcgen equivalent in order to support
`Ia5String` values.

A small unit test is added that shows round-tripping a certificate with
a subject containing an `Ia5String`, serializing it, parsing with
x509-parser, and then recreating `CertificateParams` from the DER using
`from_ca_cert_der`.
@cpu cpu force-pushed the cpu-180-ia5string branch from 8891a8c to eb9bae7 Compare October 30, 2023 13:16
@cpu
Copy link
Member Author

cpu commented Oct 30, 2023

@djc Did you want to give this branch a pass before merge?

@cpu cpu added this pull request to the merge queue Oct 30, 2023
Merged via the queue into rustls:main with commit 58febf6 Oct 30, 2023
13 checks passed
@cpu cpu deleted the cpu-180-ia5string branch October 30, 2023 14:45
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.

Ia5String support
3 participants