Accept DER certificates with non lexicographical order in attributes

[THS-on]

Versions

• Python: 3.10.4
• cryptography: 37.01
• cffi: 1.15.0
• setuptools: 60.2.0

All packages are installed using pip.

Issue

The ASN.1 parser assumes that the attributes in e.g. the issuer field need to be in lexicographical order. If not, parsing fails with the following error:

Traceback (most recent call last):
  File "/tmp/pythonProject/test.py", line 6, in <module>
    load_der_x509_certificate(cert)
  File "/tmp/pythonProject/venv/lib/python3.10/site-packages/cryptography/x509/base.py", line 521, in load_der_x509_certificate
    return rust_x509.load_der_x509_certificate(data)
ValueError: error parsing asn1 value: ParseError { kind: InvalidSetOrdering, location: ["RawCertificate::tbs_cert", "TbsCertificate::issuer", "0", "2"] }

We have seen certificates in the wild that do not conform to this which cannot be easily changed (TPM EK certificates). I could not find something in the RFCs (maybe I missed it or looked at the wrong one) that enforces that order. OpenSSL parses those certificates without an error.

This introduced in cryptography v35 and it is noted in the changelog (https://cryptography.io/en/latest/changelog/#v35-0-0) that parsing now fails if some of the fields are invalid.

It would be nice to parse those certificates directly with cryptography without using OpenSSL.

Steps to reproduce

Download Nuvoton Root CA (one certificate with that issue)

wget --no-check-certificate https://www.nuvoton.com/security/NTC-TPM-EK-Cert/Nuvoton%20TPM%20Root%20CA%202110.cer -O nuvoton.cer

Check that OpenSSL parses it:

openssl x509 -inform der  -in nuvoton.cer -text
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 4581057599893149371 (0x3f932f9d993c16bb)
        Signature Algorithm: ecdsa-with-SHA256
        Issuer: CN = Nuvoton TPM Root CA 2110 + O = Nuvoton Technology Corporation + C = TW
        Validity
            Not Before: Oct 19 04:32:00 2015 GMT
            Not After : Oct 15 04:32:00 2035 GMT
        Subject: CN = Nuvoton TPM Root CA 2110 + O = Nuvoton Technology Corporation + C = TW
...

Create a test.py with the following content:

from cryptography.x509 import load_der_x509_certificate

with open("nuvoton.cer", 'rb') as f:
    cert = f.read()

load_der_x509_certificate(cert)

Run test.py:

python test.py
Traceback (most recent call last):
  File "/tmp/pythonProject/test.py", line 6, in <module>
    load_der_x509_certificate(cert)
  File "/tmp/pythonProject/venv/lib/python3.10/site-packages/cryptography/x509/base.py", line 521, in load_der_x509_certificate
    return rust_x509.load_der_x509_certificate(data)
ValueError: error parsing asn1 value: ParseError { kind: InvalidSetOrdering, location: ["RawCertificate::tbs_cert", "TbsCertificate::issuer", "0", "2"] 

Related Keylime issue: https://github.com/keylime/keylime/issues/944

[alex]

The requirement for lexicographical order of SETs isn't part of the X.509 RFCs, it's part of the definition of ASN.1 DER itself.

Before we can consider what potential workarounds are appropriate for cryptography, we need to start with: has the vendor been informed that they are generating invalid certs, and that they need to stop?

[kgold2]

Can you point me to a spec that says C ST O OU etc. must be in a certain order? I thought the elements of a SEQUENCE could be in any order. I see X.509 certificates with all different orders.

If you point me to a definitive source, then I can communicate with the vendor.

[alex]

Here are the relevant X.509 structures from RFC 5280:

   Name ::= CHOICE { -- only one possibility for now --
     rdnSequence  RDNSequence }

   RDNSequence ::= SEQUENCE OF RelativeDistinguishedName

   RelativeDistinguishedName ::=
     SET SIZE (1..MAX) OF AttributeTypeAndValue

Then ITU X.690 specifies the encoding of SET values, which I will cite from a Go comment for simplicity:

https://github.com/golang/go/blob/64b6e44ad7e4db4525b7f05be128bc7d8713afb8/src/encoding/asn1/marshal.go#L94-L97

[mpeters]

Just to clarify, if the vendor fixes their certs to be spec compliant, how does that work with existing hardware that's already out int he field? Is it a matter of updating the firmware on those devices? Can they be updated? If not, do we need a way to support real hardware that has buggy certs?

[alex]

The vendor is definitely capable of doing this with a firmware upgrade.

But I think it's quite likely they wouldn't, because it'd be a more compilcated update.

[THS-on]

@kgold2 have you talked to the vendor?

@alex as you stated, is unlikely that this will be fixed for all devices that are deployed in the wild with a firmware update. I think introducing a flag for parsing "wrong" but from openssl accepted certificates is probably the best idea.

[alex]

It's unlikely that our approach would be to introduce a flag for allowing malformed certs, as this complicates our API surface and is difficult for users to reason about -- it's also a commitment to keeping that flag indefinitely.

What we would consider is potentially removing the enforcement of this requirement and turning it into a warning, for a temporary period. This is how we handled https://github.com/pyca/cryptography/issues/6368 for example.

But the first step is to ensure that the vendor is no longer issuing these malformed certs.

[THS-on]

It's unlikely that our approach would be to introduce a flag for allowing malformed certs, as this complicates our API surface and is difficult for users to reason about -- it's also a commitment to keeping that flag indefinitely.

Thanks for pointing that out, I had not fully considered that.

What we would consider is potentially removing the enforcement of this requirement and turning it into a warning, for a temporary period. This is how we handled https://github.com/pyca/cryptography/issues/6368 for example.

This should work. How long should the warning be kept? The issued certificates will probably never be replaced during the devices life cycle.

But the first step is to ensure that the vendor is no longer issuing these malformed certs.

I fully agree.

[F30]

I encountered an instance of this issue when the RelativeDistinguishedName was a SEQUENCE containing a single SET containing the attributes, instead of one SET per attribute.

In that case, the OpenSSL CLI also separates the attributes with + instead of , (as known from above and keylime/keylime#944).

To illustrate the difference, here is an ASN.1 dump of the form produced by most tools and accepted by Cryptography:

SEQUENCE {
  SET {
    SEQUENCE {
      OBJECT IDENTIFIER countryName (2 5 4 6)
      PrintableString 'DE'
      }
    }
  SET {
    SEQUENCE {
      OBJECT IDENTIFIER stateOrProvinceName (2 5 4 8)
      UTF8String 'Bavaria'
      }
    }
  ...
  }

The more exotic form, accepted by most tools but not by Cryptography:

SEQUENCE {
    SET {
      SEQUENCE {
        OBJECT IDENTIFIER countryName (2 5 4 6)
        PrintableString 'DE'
        }
      SEQUENCE {
        OBJECT IDENTIFIER stateOrProvinceName (2 5 4 8)
        UTF8String 'Bavaria'
        }
      ...
      }
    }

In my case, the second form was created using PKI.js. Multiple PKI.js issues about this behavior have been dismissed, e.g. PeculiarVentures/PKI.js#46, PeculiarVentures/PKI.js#169, and PeculiarVentures/PKI.js#337.

I know too little about X.509, ASN.1, and LDAP to assess whether the second form is valid. The PKI.js maintainer has been very determined about it being correct.

[ronf]

The second form is valid, but only if the elements in a SET are sorted into a consistent order. This does not apply to the other form, as order matters to SEQUENCEs, meaning that "C=DE,ST=Bavaria" is a different name than "ST=Bavaria,C=DE". So, no sorting is needed (and in fact doing so would be a bug). However, "C=DE+ST=Bavaria" is supposed to be exactly the same name as "ST=Bavaria+C=DE".

To state it another way, comma-separated elements must be encoded in the order they are listed, but plus-separated elements must always be sorted into a consistent order before encoding them in DER/PEM. This is done by sorting the byte strings created when DER-encoding the SEQUENCEs inside the SETs.

In this example, I believe the countryName field should always come before the stateOrProvinceName, because the DER-encoded version of the OID for countryName comes before the DER-encoded version of stateOrProvinceName. More generally, you'd look at the DER-encoded version of the entire inner SEQUENCEs inside each SET to decide the ordering.

Cryptography already does the right thing here when you pass in X.509 name objects of either form to the certificate builder. In that case, regardless of the order in the X.509 name object you pass in, the resulting certs will always have the SET elements sorted properly. However, if some external tool is used to create the DER/PEM, I can see where cryptography might reject the cert if the order is wrong.

[F30]

@ronf: I see, thanks for the explanation!

[kgoldman]

The vendor is definitely capable of doing this with a firmware upgrade.

But I think it's quite likely they wouldn't, because it'd be a more compilcated update.

Can this be fixed with a firmware update? The certificate is created at the manufacturer, not the TPM

[alex]

The manufacturer could sign new certs for every device they've shipped and then publish them -- this is fine because the cert is only usable if you have the private key, which is inside the TPM. Then people could load the correct certs onto their devices.

[kgoldman]

Yes but:

1. It could be a big TPM vendor expense, because these are done on a secure production line and it's a new secure process.
2. The TPM vendor would have to determine all the EKs, and have a way to distribute them.
3. The OEM would need a new process to carefully provision the new EK certs without letting an attacker in. All OEMs would have to create new platform firmware to support this.
4. The end user would have flash new firmware, get the EK certs from somewhere, and reprovision the TPM.

Try it, but it may be hard to convince the TPM vendors and OEMs to fund that, and the end users to changes their processes.

An alternative would be for keylime to use openssl rather than python libraries. That may be faster.

Would it possible to fix the python library?

[alex]

As I've said, we're willing to consider potential workarounds, but we won't do that until the issuer of these malformed certs has taken action to fix that, at the very least prospectively (for new issuances). I have yet to see any indication that the manufacturer has done so, so they are the ones to direct feedback to at this point.

[kgoldman]

I don't even know if the manufacturer has been notified. Join a TCG call and bring it up.

However, even if they changed something to be compatible with python in the future, it would not help keylime for years, and would not address the millions already shipped.

[reaperhulk]

Notifying the manufacturer is not the responsibility of an OSS project run by volunteers, although it doesn't seem like the TCG is likely to be a productive path when it is a specific TPM vendor with the encoding issue (Nuvoton, who has a forum!). As previously stated, temporary workarounds are possible (we've done this in the past for Red Hat shipping invalid DER in CSRs their software generated), but valid DER is important to a healthy certificate ecosystem so entities that are doing this improperly should be notified so they can fix it. Without a path to correct this issue any workaround becomes a permanent and expected feature for the entire ecosystem.

As a side note, invalid DER like this is disallowed and considered misissuance in the web PKI.

[kgoldman]

Here are the relevant X.509 structures from RFC 5280:

   Name ::= CHOICE { -- only one possibility for now --
     rdnSequence  RDNSequence }

   RDNSequence ::= SEQUENCE OF RelativeDistinguishedName

   RelativeDistinguishedName ::=
     SET SIZE (1..MAX) OF AttributeTypeAndValue

Then ITU X.690 specifies the encoding of SET values, which I will cite from a Go comment for simplicity:

https://github.com/golang/go/blob/64b6e44ad7e4db4525b7f05be128bc7d8713afb8/src/encoding/asn1/marshal.go#L94-L97

I've searched through the RFCs and still cannot find any place where the order is mandated. Can you point me to a spec (not a golang comment., that starts with X.509.

[alex]

https://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf

I don't mean to be rude, but literally I took the Go comment, searched for the referenced spec, and then went to the referenced section.

[kgoldman]

I'm certainly not an X.509 expert, but I don't see anything about certificate issuer there, nor can I see a path from X509 to X690.

Regardless, there are millions of these in the field that cannot easily be changed and that are parsed by other crypto libraries.

[alex]

ITU 690 defines what DER is. https://www.rfc-editor.org/rfc/rfc5280.html#section-4.1 explicitly states that:

For signature calculation, the data that is to be signed is encoded using the ASN.1 distinguished encoding rules (DER) [X.690]

If you're just curious that's fine, but FYI your questions are of a nature that is very reminiscent of misbehaving WebPKI CAs who tried to explain why their mis-issuances didn't technically violate the rules. These defenses were uniformly rejected in mozilla.dev.security.policy.

We've stated several times now that we're happy to look at doing a workaround here, but the first step is communicating to the vendor. Have you reached out to the vendor? Just because OSS maintainers are more accessible to file issues with, doesn't mean we're the correct ones to resolve the issue.

[tiran]

Then trust the experts. :)

The X.690 standard specifies ASN.1 encodings, among others Distinguished Encoding Rules (DER). X.509 certificates use ASN.1 DER. DER requires that SET elements are ordered in a specific way. A non-compliant ordering is a violation of DER and therefore an invalid certificate.

[reaperhulk]

I'd like to try to turn this thread in a bit more productive direction.

If it is your belief that the reality of the current ecosystem is that it's untenable to error on this type of invalid encoding, then that's a reasonable position to take! Convincing this project of that involves gathering some data. Are there any other widely deployed DER libs that also enforce this constraint? Can we find additional significant deployments of certificates (in the TPM ecosystem or otherwise) that have similar or other encoding errors that would prevent parsing by this library?

What we have now is a single (large) vendor with a problem, which is a very different situation from a pervasive issue spanning across the entire ecosystem. For us to take action we need data, and right now we have weak signals from our own issue tracker that this isn't really a huge problem. That may be wrong, but we won't know that until someone digs in.

[teddyhartanto]

Hi all,

I highly appreciate all the discussions & efforts in this thread. I'm a complete newbie when it comes to X509 certificate / ASN.1 encoding standards. I see there is an openness to implementing workaround in the library, but first vendor has to be nudged to stop issuing malformed certs. That's great.

In the meantime, I wonder what would be the best alternative short-term solution, if one would have to work with these malformed certs? It's necessary for me to be able to parse these malformed certs by Nuvoton (haven't seen instances from other vendors yet, will update if I see more).

I have thought of 3 possible ways:

1. Use openssl to parse the malformed certs
2. Use cryptography==3.4.8, the latest version that is not so strict with ASN1 parsing
3. Use pyasn1 to parse the malformed certs, keylime's approach

I'm leaning towards option (2). Do you agree that that's probably the best option, or am I missing something here? Thanks!

[ronf]

There's a lightweight DER encoder and decoder in AsyncSSH which could be used for this. Here's an example:

from asyncssh.asn1 import der_decode, der_encode
from cryptography import x509

def fix_cert(filename: str) -> bytes:
    with open(filename, 'rb') as f:
        return der_encode(der_decode(f.read()))

cert = x509.load_der_x509_certificate(fix_cert('nuvoton.cer'))

This works with the latest version of the cryptography package and successfully loads the nuvoton cert, as the der_decode() in AsyncSSH is forgiving about the set ordering requirement, but the der_encode() enforces this requirement when re-encoding the set objects within the cert.

I'm not sure if you'd want all of AsyncSSH as a dependency, but if not its asn1 package is pretty much standalone if you wanted to pull that out for this purpose. The whole module is around 700-800 lines.

I'd be hesitant to go with your option 2 here, as sticking with that old version would mean you'd be missing out on potentially important security patches.

[alex]

That will work, as long as you don't need to verify the certificate's signature

[ronf]

Good point. This isn't an issue with load_der_x509_certificate() because it doesn't do any signature validation itself, and generally speaking you wouldn't do a signature check when loading a root CA such as the one given in this example. However, if you were given a certificate chain with a cert that had this ordering issue, that would be more of a problem.

Unfortunately, I think any solution here short of allowing you to relax the ordering check is going to suffer from this. You'd need the private key of the issuer of the cert to be able to regenerate its signature after fixing the DER encoding of the names, and when validating a certificate chain you generally won't have that for the certs involved.

[github-actions[bot]]

This issue has been waiting for a reporter response for 5 days. It will be auto-closed if no activity occurs in the next week.

[THS-on]

The current state is that we have implemented a workaround because in our case it is unlikely that new certificates are issued for those devices.

I'll ask around what the status with the vendor is, because I don' t have direct communication with them.

[THS-on]

The vendor was contacted through their official contact form, but we haven't heard anything back yet.

Newer CA certificates for different chips do not have this issue because they are using sets around each sequence (the first form shown in https://github.com/pyca/cryptography/issues/7189#issuecomment-1164444418). Unfortunately I don't have access to hardware that contains certificates issued by those CAs to verify that those also have changed.