Added support for parsing challenge password attribute in CSR's

[bkstein]

This branch adds support for parsing a challenge password attribute in a CSR.

Please note: rusticata/oid-registry#10 is a prerequisite, as it adds OID_PKCS9_CHALLENGE_PASSWORD. This PR is merged, but not yet released.

[bkstein]

A remark @chifflier: I think, the attribute parsing could be improved. Currently, X509CertificationRequest::from_der() parses the CSR and knows the (challenge password) attribute's value. This value is held in X509CriAttribute.parsed_attribute, which is not visible outside the crate:

pub struct X509CriAttribute<'a> {
    pub oid: Oid<'a>,
    pub value: &'a [u8],
    pub(crate) parsed_attribute: ParsedCriAttribute<'a>,
}

Why is that? A user of the x509-parser crate needs to re-parse X509CriAttribute.value instead. I think, the already parsed attribute value should be made available for users. What do you think?

[bkstein]

I just compared CriAttribute to X509Extension and found

impl<'a> X509Extension<'a> {
    ...
    /// Return the extension type or `UnsupportedExtension` if the extension is not implemented.
    #[inline]
    pub fn parsed_extension(&self) -> &ParsedExtension<'a> {
        &self.parsed_extension
    }
}

We could do that in a similar manner for attributes

impl<'a> CriAttribute<'a> {
    ...
    /// Return the attribute type or `UnsupportedAttribute` if the attribute is unknown.
    #[inline]
    pub fn parsed_attribute(&self) -> &ParsedCriAttribute<'a> {
        &self.parsed_attribute
    }
}

[bkstein]

I will check my proposal and set this request to draft.

[bkstein]

Seems to work.

[bkstein]

@chifflier The checks will fail until oid-registry with OID for challenge password is released.

[chifflier]

@chifflier The checks will fail until oid-registry with OID for challenge password is released.

oid-registry 0.6.1 has just been released with the required OID

[chifflier]

The nom parser for this is alt so I supposed the code below can be rewritten as something like alt(parse_der_utf8string, parse_der_printablestring).
Example:

x509-parser/src/x509.rs

Lines 153 to 155 in f6d25f6

	fn parse_attribute_value(i: &[u8]) -> ParseResult<Any, Error> {
	alt((Any::from_der, parse_malformed_string))(i)
	}

[chifflier]

Overall the PR looks fine, thanks!
Can you rebase to the latest master (now that oid-registry is released) and address the minor point in comment before merge?

[chifflier]

I think something like

match obj.as_str() {
    Ok(s) => Ok((rem, ChallengePassword { 0: s.to_string() })),
    // ...

is slightly more elegant and does not require to enumerate variants

[bkstein]

Got it, thanks. However, digging into RFC 2985, 5.2.2 I saw, that all string types should be supported (not only the recommended PrintableString and UTF8String). I extended the parser with the other types using nom::branch::alt.

[chifflier]

Isn't that exactly the same as just String::from_der(i) ?

[bkstein]

Yes, I think your right. I'll fix that.

[bkstein]

Changes are implemented. Thanks for reviewing!

[chifflier]

Looks great now, thanks for your patience!
I will merge it as soon as possible