ValueError when x500UniqueIdentifier is of type UTF8String

[vxgmichel]

This issue is about the attribute 2.5.4.45, also called uniqueIdentifier or x500UniqueIdentifier:

• https://www.alvestrand.no/objectid/2.5.4.45.html

• https://ldapwiki.com/wiki/X500UniqueIdentifier

  The OID specification for this attribute requires a value of type BIT STRING (universal tag 3), but in the wild we might see a UTF8String type instead (universal tag 12). For instance, it can easily be generated using the following openssl command:

  openssl req -x509 -newkey rsa:4096 -keyout ca.key -out ca.pem -sha256 -days 365 -subj "/CN=test_ca/x500UniqueIdentifier=test" -nodes

  The problem is that asn1crypto raises a ValueError when building the corresponding subject, which might cause other apparently unrelated operations to fail. I have listed three such cases in the test module below:

  
  import pytest
  import subprocess

from asn1crypto import x509, pem from certvalidator import CertificateValidator, ValidationContext, errors

@pytest.fixture( params=[False, True], ids=["without_unique_identifier", "with_unique_identifier"] ) def self_signed_certificate(request, tmp_path): key_file = tmp_path / "ca.key" cert_file = tmp_path / "ca.pem"

Certificate subject

subject = "/CN=test_ca"
if request.param:
    subject += "/x500UniqueIdentifier=test_ca"
# Create self-signed certificate
subprocess.run(
    f"openssl req -x509 -newkey rsa:4096 -keyout {key_file} -out {cert_file} -sha256"
    f" -days 365 -subj {subject!r} -nodes -addext 'keyUsage = digitalSignature'",
    shell=True,
    check=True,
    capture_output=True,
)
# Load certificate
der_bytes = cert_file.read_bytes()
type_name, headers, der_bytes = pem.unarmor(der_bytes)
return x509.Certificate.load(der_bytes)

def test_subject_common_name(self_signed_certificate): assert self_signed_certificate.subject.native["common_name"] == "test_ca"

def test_validate_certificate_with_trust_root(self_signed_certificate): validation_context = ValidationContext(extra_trust_roots=[self_signed_certificate]) validator = CertificateValidator( self_signed_certificate, validation_context=validation_context ) validator.validate_usage({"digital_signature"})

def test_validate_certificate_without_trust_root(self_signed_certificate): validator = CertificateValidator(self_signed_certificate) with pytest.raises(errors.InvalidCertificateError): validator.validate_usage({"digital_signature"})

It produces the following results:

test_asn1crypto.py::test_subject_common_name[without_unique_identifier] PASSED test_asn1crypto.py::test_subject_common_name[with_unique_identifier] FAILED test_asn1crypto.py::test_validate_certificate_with_trust_root[without_unique_identifier] PASSED test_asn1crypto.py::test_validate_certificate_with_trust_root[with_unique_identifier] FAILED test_asn1crypto.py::test_validate_certificate_without_trust_root[without_unique_identifier] PASSED
test_asn1crypto.py::test_validate_certificate_without_trust_root[with_unique_identifier] FAILED

Note that I have also seen this issue happen here, it happens in case similar to `test_validate_certificate_without_trust_root` when the certificate is not self-signed: 
https://github.com/wbond/certvalidator/blob/fc82b7975f61fc1b2d1cf7ac77b2e4b2964aedb1/certvalidator/registry.py#L312

In order to fix this issue, I used the following patch:
```python
def patch_asn1crypto():
    """
    Patch asn1crypto against x500UniqueIdentifier type-mismatch errors.

    The `unique_identifier` field is defined to be a bit string, but it might happen
    that the corresponding value is encoded as an utf-8 string instead. In that case,
    we allow the bits in the utf-8 bytestring to be used as a bit string, with no
    unused bits.

    An alternative to this issue would be to simply ignore those fields. That would
    work in some cases, typically by replacing `cert.asn1.issuer.native` by a smarter
    implementation that catches and ignores errors while parsing each field. However,
    it doesn't work for cases like `validator.validate_usage({"digital_signature"})`
    that would appear to work fine until an exception is raised: exceptions typically
    include `cert.subject.human_friendly` as part of the message but this would fail
    with a `ValueError` instead of the expected exception (e.g `PathBuildingError`)
    """

    @property
    def _header(self):
        return self.__header

    @_header.setter
    def _header(self, header):
        self.__header = header
        if self.contents is None or not header:
            return
        # This is required to properly set the first byte of `self.contents`
        # to `\0` in order to indicate that there are no unused bits.
        if header[0] == asn1crypto.core.UTF8String.tag:
            self.set(self.contents)

    asn1crypto.core.OctetBitString._bad_tag = asn1crypto.core.UTF8String.tag
    asn1crypto.core.OctetBitString._header = _header

This patch is trickier than I expected because simply registering UTF8String.tag as a bad tag for OctetBitString wasn't enough: the content of OctetBitString also has to be prepended with b"\x00" in order for the utf-8 string to be interpreted as a bit string.

I hope I didn't miss anything important while investigating this issue, and I'll be happy to provide more information if necessary.

[wbond]

If you could provide a PR with a test case that fails using the current implementation, that would make it a bit easier to tweak the implementation to handling such "mis-encoded" certificates.

[vxgmichel]

If you could provide a PR with a test case that fails using the current implementation, that would make it a bit easier to tweak the implementation to handling such "mis-encoded" certificates.

Here you go: https://github.com/wbond/asn1crypto/pull/241 :)