-
Notifications
You must be signed in to change notification settings - Fork 173
feat(x509): implement RFC 4514 string serialization for x509 names #464
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
Conversation
bfb2ef9 to
882bba9
Compare
x509/src/attr.rs
Outdated
| /// This function follows the rules in [RFC 4514]. | ||
| /// | ||
| /// [RFC 4514]: https://datatracker.ietf.org/doc/html/rfc4514 | ||
| pub fn parse(s: &str) -> Result<Vec<u8>, der::Error> { |
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.
I'm a little confused by the fn parse name along with parse_str and parse_der. These seem to be combination parser/serializer methods?
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.
Yes, I agree it is a bit awkward. Basically, RFC 4514 defines a human readable encoding for certificate name types. That is, we want to convert from the name type into a string (this part is easy) and from the string back into the relevant name type. The latter operation is difficult because the result needs to be owned, but the name types borrow references.
The compromise is that we parse the string into an owned buffer. Then we create the named type referencing that owned buffer. Then, because of the lifetime issues, we serialize the named type to der.
In practice it looks like this to avoid the lifetime issues:
let der = RdnSequence::parse("CN=foo").unwrap();
let rdns = RdnSequence::from_der(&der).unwrap();If you have alternative suggestions, I'm all ears.
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.
Since it's producing encoded DER, I'd suggest something like RdnSequence::encode_*, like RdnSequence::encode_from_string
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.
I renamed all the public functions to encode_from_string(). The internal functions were renamed to encode_str() and encode_hex().
tarcieri
left a comment
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.
A few small nits, otherwise looks good
ee4d3ec to
2add659
Compare
Signed-off-by: Nathaniel McCallum <nathaniel@profian.com>
The main commit is the last one. The previous commits are just enablement.
This PR depends on:
Resolves: #417