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:

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: #241 :)