x509-cert: RdnSequence.to_string() not producing RFC4514 compliant string representations #1121

christopheraranda · 2023-06-26T17:48:29Z

Description

RdnSequence.to_string() is not producing RFC4514 compliant string representations. According to RFC4514 Section 2.1:

If the RDNSequence is an empty sequence, the result is the empty or zero-length string.
Otherwise, the output consists of the string encodings of each RelativeDistinguishedName in the RDNSequence (according to Section 2.2), starting with the last element of the sequence and moving backwards toward the first.
The encodings of adjoining RelativeDistinguishedNames are separated by a comma (',' U+002C) character.

RFC4514 Section 2.3 states the following:

If the AttributeType is defined to have a short name (descriptor) [RFC4512] and that short name is known to be registered [REGISTRY] [RFC4520] as identifying the AttributeType, that short name, a <descr>, is used. Otherwise the AttributeType is encoded as the dotted-decimal encoding, a <numericoid>, of its OBJECT IDENTIFIER. The <descr> and <numericoid> are defined in [RFC4512].

Steps to Reproduce

Given the following Certification Request:

Which is interpreted by OpenSSL:

openssl req -text -noout -verify -in csr.pem  
verify OK
Certificate Request:
    Data:
        Version: 0 (0x0)
        Subject: C=US, ST=California, L=Mountain View, O=Google LLC, CN=OQFAvDNDWs.google.com, OU=management:ds.group.3891111/UID=identity:ds.group.3891111
        Subject Public Key Info:
            Public Key Algorithm: rsaEncryption
                Public-Key: (2048 bit)
                Modulus:
                    00:c2:b4:6b:5c:a3:6e:56:1e:dc:14:ec:38:a3:a2:
                    c5:fb:e0:9e:8d:b7:f0:82:f2:33:22:e6:25:d2:06:
                    dd:a1:71:f9:72:98:8b:41:1f:e1:e4:99:2a:4d:1f:
                    02:0b:f8:48:76:2e:50:71:97:f9:76:73:69:f4:5c:
                    b1:1b:6c:7c:63:bb:9f:57:c5:64:ac:db:a8:e5:e0:
                    31:50:8f:27:94:8b:78:9c:24:d5:3f:71:20:cb:ce:
                    39:40:3e:7f:16:30:64:be:eb:c0:9b:7b:51:b5:af:
                    0b:2b:60:81:8f:7f:00:a6:78:2d:fb:16:64:fc:9e:
                    4f:e2:32:05:b7:ba:3a:60:03:3f:f5:66:ba:04:75:
                    c8:17:3b:7a:6d:06:61:cc:bf:c1:a4:dd:fd:74:6c:
                    41:b0:9a:60:6f:89:51:db:3f:57:5b:d7:0b:df:4e:
                    66:40:24:90:65:4a:1c:c6:3c:89:c2:18:c8:6b:90:
                    17:68:81:e7:24:60:46:fc:dd:b4:15:66:05:ea:a7:
                    ca:32:46:9b:f8:5f:a6:a5:66:ee:77:db:0e:55:6d:
                    e6:b6:90:bf:d1:40:bd:89:cb:24:82:b9:93:ac:78:
                    09:74:21:03:ee:ff:a0:2d:92:50:79:43:e3:c9:cd:
                    e6:c0:cc:50:8d:49:67:53:af:02:f5:5e:5d:b6:71:
                    6f:97
                Exponent: 65537 (0x10001)
        Attributes:
        Requested Extensions:
            X509v3 Subject Alternative Name: 
                DNS:google.com, DNS:www.google.com, DNS:OQFAvDNDWs.google.com
    Signature Algorithm: sha256WithRSAEncryption
         79:cc:89:0b:f3:b1:22:37:c6:51:c8:74:e7:70:56:45:ba:1f:
         92:19:57:20:bc:72:f0:41:49:c9:ab:0b:8f:1a:ea:05:78:8c:
         6c:87:65:28:67:11:c4:87:45:4b:3f:ef:12:a9:d6:37:cb:4a:
         0c:88:3d:16:95:0b:6a:c9:61:1c:81:c2:5a:c7:4c:6e:d2:13:
         cf:78:9f:84:01:0b:0b:bf:e5:a2:c3:e4:a8:28:6d:90:42:e1:
         ef:6c:a5:78:33:a9:9c:97:4e:6e:84:c4:7b:35:9a:fa:bc:d5:
         e3:29:0c:42:d0:b5:e7:f7:3b:4e:55:98:15:da:7b:6f:28:e8:
         11:7b:58:1c:12:92:32:d6:c5:ca:f8:76:d3:05:04:db:0f:93:
         ca:a1:6d:f4:86:36:fc:8d:c7:d0:2f:0a:ac:99:8e:de:2f:17:
         9e:2e:27:dd:41:96:d9:e1:9d:38:dc:95:8e:0d:f8:5e:25:33:
         4e:c7:6a:26:21:bb:3a:82:82:2d:22:e8:0c:1c:3d:86:17:3f:
         f4:6b:dd:0a:5b:43:ed:94:78:79:b8:ff:8d:3e:38:5f:57:d8:
         0b:05:40:cd:e1:76:16:53:de:75:9e:52:b6:e1:71:b0:64:3e:
         ff:13:0f:52:6f:87:9f:86:a0:bf:e1:56:33:20:b8:d2:90:0c:
         41:48:3a:21

Execute the following code:

extern crate x509_cert;

use der::{DecodePem, Encode, EncodePem};
use std::fs;
use x509_cert::{request::CertReq};

pub fn main() {
    parse_certification_request("csr.pem");
}

pub fn parse_certification_request(path: &str) {
    let pem = fs::read_to_string(path).expect("Failed to load file");
    let certification_request = CertReq::from_pem(pem.as_bytes()).expect("Failed to decode PEM");
    let subject = certification_request
        .info
        .subject
        .to_string();

    println!("Subject: {}", subject);
}

Expected Results

The subject string representation should be:

UID=identity:ds.group.3891111,OU=management:ds.group.3891111,CN=OQFAvDNDWs.google.com,O=Google LLC,L=Mountain View,ST=California,C=US

Actual Results

The RDNSequence is not reversed, and the st attribute type is displayed as STATEORPROVINCENAME.

Subject: C=US,STATEORPROVINCENAME=California,L=Mountain View,O=Google LLC,CN=OQFAvDNDWs.google.com,OU=management:ds.group.3891111,UID=identity:ds.group.3891111

Note

This also applies to the issuer and subject attributes in a X.509 certificate.

The text was updated successfully, but these errors were encountered:

tarcieri · 2023-06-27T19:22:56Z

Note: originally implemented in #464 /cc @npmccallum

baloo · 2023-06-27T22:09:54Z

I definitely agree with the ordering issue, but about the naming of the parameter, I think nameing it STATEORPROVINCENAME is compliant with the RFC.
This is listed in http://www.iana.org/assignments/ldap-parameters/ldap-parameters.xhtml

st 	A 	2.5.4.8 	[[RFC4519](https://www.iana.org/go/rfc4519)]
stateOrProvinceName 	A 	2.5.4.8 	[[RFC2256](https://www.iana.org/go/rfc2256)]

As specified in the RFC. Is there a precedence rule that I'd be missing?

In any case, please see #1126

christopheraranda · 2023-06-27T22:25:44Z

I definitely agree with the ordering issue, but about the naming of the parameter, I think nameing it STATEORPROVINCENAME is compliant with the RFC. This is listed in http://www.iana.org/assignments/ldap-parameters/ldap-parameters.xhtml
st 	A 	2.5.4.8 	[[RFC4519](https://www.iana.org/go/rfc4519)]
stateOrProvinceName 	A 	2.5.4.8 	[[RFC2256](https://www.iana.org/go/rfc2256)]
As specified in the RFC. Is there a precedence rule that I'd be missing?

In any case, please see #1126

Thanks for the quick fix, @baloo! This library has been the a joy to use!

It appears as though both RFC2256 and RFC4519 both refer to short name for the stateOrProvinceName attribute as st. But I may not be up to speed on all the RFC's.

baloo · 2023-06-27T22:31:38Z

I don't necessarily disagree with the st vs stateOrProvinceName, and it boils down to the implementation of the oid lookup in const-oid.
If we want to change that to be st, we need to preserve ordering defined in that table:
http://www.iana.org/assignments/ldap-parameters/ldap-parameters.xhtml#ldap-parameters-3

tarcieri · 2023-06-27T22:46:47Z

oiddbgen is order-preserving.

The problem seems to be that stateOrProvinceName is defined in RFC2256, and st is defined RFC4519.

Right now we order the database in terms of the RFCs which the OIDs are defined in, so stateOrProvinceName takes precedence because it's from an earlier RFC.

baloo · 2023-06-27T22:52:41Z

I meant order-preserving in regard to http://www.iana.org/assignments/ldap-parameters/ldap-parameters.xhtml#ldap-parameters-3

I think the only order we currently care about is the order within a single RFC.

baloo · 2023-06-27T22:54:45Z

Everything is ordered by RFC first:

formats/const-oid/src/db/gen.rs

Line 2308 in 96186bb

pub const DB: super::Database<'static> = super::Database(&[

formats/const-oid/oiddbgen/src/root.rs

Line 9 in 96186bb

pub struct Root(BTreeMap<Ident, Spec>);

The ident in the Btreemap is the document id which are ordered per Ord

tarcieri · 2023-06-27T22:59:05Z

Yeah, that's what I said:

Right now we order the database in terms of the RFCs which the OIDs are defined in, so stateOrProvinceName takes precedence because it's from an earlier RFC.

I'm not sure of a good solution though. Short of special casing it for now it seems like it would be a breaking change.

baloo · 2023-06-27T23:00:11Z

Yeah, I'm very much worried about the blast radius if we change that logic.

tarcieri · 2023-06-27T23:15:18Z

Perhaps we should open a separate issue for using the short name.

Alternatively we may need a different API for const-oid which allows retrieving all of the names associated with a particular OID so the short one can be used?

baloo · 2023-06-27T23:18:12Z

we can also just reopen this.

Made that, but I'm not convinced this is the right solution:
https://github.com/RustCrypto/formats/compare/master...baloo:baloo/x509-cert/attr-st-handling?expand=1

baloo · 2023-06-27T23:19:55Z

I'll try to implement a logic to respect the ordering of the IANA table, just to get an idea of what is going to break.

tarcieri · 2023-06-27T23:23:22Z

@baloo the ability to iterate over all of the names associated with an OID, rather than just one, would seem to be a clean solution to this problem that doesn't involve restructuring the database or adding special-case hacks, and potentially useful for other applications

baloo · 2023-06-27T23:32:43Z

From the look of an awk output on the table, that should work.

see RustCrypto#1121

baloo · 2023-06-29T22:08:16Z

the STATEORPROVINCENAME vs ST situation is fixed with #1129 and #1130. Thanks everyone!

christopheraranda · 2023-07-24T18:40:55Z

@baloo @tarcieri Great job and thanks for the quick resolution on this issue! When do you plan to tag and release this?

tarcieri assigned baloo Jun 27, 2023

baloo mentioned this issue Jun 27, 2023

x509-cert: fixup RDN string representation #1126

Merged

baloo closed this as completed in #1126 Jun 27, 2023

baloo added a commit to baloo/formats that referenced this issue Jun 28, 2023

const-oid: provide shortest_by_oid lookups

323842a

see RustCrypto#1121

baloo mentioned this issue Jun 28, 2023

const-oid: provide shortest_by_oid lookups #1127

Closed

baloo added a commit to baloo/formats that referenced this issue Jun 28, 2023

const-oid: provide shortest_name_by_oid lookups

583130f

see RustCrypto#1121

baloo mentioned this issue Jul 25, 2023

x509-cert v0.2.4 #1170

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x509-cert: RdnSequence.to_string() not producing RFC4514 compliant string representations #1121

x509-cert: RdnSequence.to_string() not producing RFC4514 compliant string representations #1121

christopheraranda commented Jun 26, 2023

tarcieri commented Jun 27, 2023

baloo commented Jun 27, 2023

christopheraranda commented Jun 27, 2023

baloo commented Jun 27, 2023 •

edited

Loading

tarcieri commented Jun 27, 2023 •

edited

Loading

baloo commented Jun 27, 2023

baloo commented Jun 27, 2023 •

edited

Loading

tarcieri commented Jun 27, 2023 •

edited

Loading

baloo commented Jun 27, 2023

tarcieri commented Jun 27, 2023

baloo commented Jun 27, 2023

baloo commented Jun 27, 2023

tarcieri commented Jun 27, 2023

baloo commented Jun 27, 2023

baloo commented Jun 29, 2023 •

edited

Loading

christopheraranda commented Jul 24, 2023

x509-cert: RdnSequence.to_string() not producing RFC4514 compliant string representations #1121

x509-cert: RdnSequence.to_string() not producing RFC4514 compliant string representations #1121

Comments

christopheraranda commented Jun 26, 2023

Description

Steps to Reproduce

Expected Results

Actual Results

Note

tarcieri commented Jun 27, 2023

baloo commented Jun 27, 2023

christopheraranda commented Jun 27, 2023

baloo commented Jun 27, 2023 • edited Loading

tarcieri commented Jun 27, 2023 • edited Loading

baloo commented Jun 27, 2023

baloo commented Jun 27, 2023 • edited Loading

tarcieri commented Jun 27, 2023 • edited Loading

baloo commented Jun 27, 2023

tarcieri commented Jun 27, 2023

baloo commented Jun 27, 2023

baloo commented Jun 27, 2023

tarcieri commented Jun 27, 2023

baloo commented Jun 27, 2023

baloo commented Jun 29, 2023 • edited Loading

christopheraranda commented Jul 24, 2023

baloo commented Jun 27, 2023 •

edited

Loading

tarcieri commented Jun 27, 2023 •

edited

Loading

baloo commented Jun 27, 2023 •

edited

Loading

tarcieri commented Jun 27, 2023 •

edited

Loading

baloo commented Jun 29, 2023 •

edited

Loading