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.

Sign up for GitHub

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

Expand the public API slightly #35

Merged
merged 3 commits into from
Jul 7, 2020
Merged

Expand the public API slightly #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
34d74e1
Extend the lifetime of parsed extensions
g2p Jul 6, 2020
9416c1e
Expose the raw DER of an X509Name
g2p Jul 6, 2020
5bcb113
Make parse_name public, for parsing distinguished names
g2p Jul 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/x509.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl<'a> X509Extension<'a> {
}

/// Return the extension type or `None` if the extension is not implemented.
pub fn parsed_extension(&self) -> &ParsedExtension {
pub fn parsed_extension(&self) -> &ParsedExtension<'a> {
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to understand - the compiler was inferring a wrong lifetime without the explicit one?

Copy link
Contributor Author

@g2p g2p Jul 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Indeed! Lifetime elision was picking up the anonymous lifetime on the &self reference, and not the 'a lifetime on the impl block.

My usage goes like this:
I have a long-lived ('a) object that carries the entire DER of the certificate.
A function parses it and returns &str data that is sliced from the same DER.
The &str should be able to live as long as the parent DER, but the X509Extension and ParsedExtension structs are local to the function; having the reference lifetime (function local) distinct from the 'a lifetime used inside the structs allows me to return the right &'a str.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see now, thank you for the details.
I also encountered this kind of limitations within DER, where the compiler did not apply "transitive" lifetimes to inner objects when returned, but agrees when I give it explicitly - maybe I will be able to fix this now!

&self.parsed_extension
}
}
Expand Down Expand Up @@ -79,6 +79,14 @@ pub struct AlgorithmIdentifier<'a> {
#[derive(Debug, PartialEq)]
pub struct X509Name<'a> {
pub rdn_seq: Vec<RelativeDistinguishedName<'a>>,
pub(crate) raw: &'a [u8],
}

impl<'a> X509Name<'a> {
// Not using the AsRef trait, as that would not give back the full 'a lifetime
pub fn as_raw(&self) -> &'a [u8] {
Copy link
Member

Choose a reason for hiding this comment

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

  • I'm also curious about the comment on lifetimes (which seems similar to the above one)
  • Could you share the use case where you needed the raw bytes? I'm asking mostly because, if this is useful for several fields, the as_raw method could be added to all parsed components

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Similar to the above; the AsRef trait takes no lifetime parameter, and the as_ref function can only return data with the lifetime on the X509Name; if the parsing is done in an inner function, the inner function won't be able to return slices from that.

My use case is sending OCSP requests and validating the reponses; the raw data is sometimes hashed (building the CertID for the OCSP protocol), it can also be copied (some parts of the request should exactly match the certificates that are being checked), and compared (to check that the response really matches the request).

self.raw
}
}

impl<'a> fmt::Display for X509Name<'a> {
Expand Down Expand Up @@ -502,6 +510,7 @@ mod tests {
],
},
],
raw: &[], // incorrect, but enough for testing
};
assert_eq!(
name.to_string(),
Expand Down
15 changes: 12 additions & 3 deletions src/x509_parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,23 @@ fn parse_rdn(i: &[u8]) -> BerResult<RelativeDistinguishedName> {
.map(|(rem, x)| (rem, x.1))
}

pub(crate) fn parse_name(i: &[u8]) -> BerResult<X509Name> {
pub fn parse_name(i: &[u8]) -> BerResult<X509Name> {
Copy link
Member

Choose a reason for hiding this comment

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

I agree with making this public, but FYI I think I'll just change to a better name (parse_x509_name) by adding a commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea!

parse_der_struct!(
i,
TAG DerTag::Sequence,
v: many1!(complete!(parse_rdn)) >>
( X509Name{ rdn_seq:v } )
( v )
)
.map(|(rem, x)| (rem, x.1))
.map(|(rem, x)| {
let len = i.len() - rem.len();
(
rem,
X509Name {
rdn_seq: x.1,
raw: &i[..len],
},
)
})
}

fn parse_version(i: &[u8]) -> BerResult<u32> {
Expand Down
2 changes: 2 additions & 0 deletions tests/readcert.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ fn test_x509_parser() {
//
let expected_issuer = "C=FR, ST=France, L=Paris, O=PM/SGDN, OU=DCSSI, CN=IGC/A, [email protected]";
assert_eq!(format!("{}", tbs_cert.issuer), expected_issuer);
let expected_issuer_der = &IGCA_DER[35..171];
assert_eq!(tbs_cert.issuer.as_raw(), expected_issuer_der);
//
let sig_alg = &cert.signature_algorithm;
assert_eq!(sig_alg.algorithm, OID_RSA_SHA1);
Expand Down