cashapp / certifikit

Kotlin Certificate processing library.
https://cashapp.github.io/certifikit/
Apache License 2.0
40 stars 11 forks source link

Failure converting to X509Certificate #95

Closed yschimke closed 2 years ago

yschimke commented 2 years ago

From https://github.com/cashapp/certifikit/pull/94

Certificate(
tbsCertificate=TbsCertificate(
version=2, serialNumber=233941827916992643644392934961461462906, signature=AlgorithmIdentifier(
algorithm=1.2.840.113549.1.1.11, parameters=null), issuer=[[AttributeTypeAndValue(
type=2.5.4.6, value=US)], [AttributeTypeAndValue(
type=2.5.4.10, value=Google Trust Services LLC)], [AttributeTypeAndValue(
type=2.5.4.3, value=GTS CA 1C3)]], validity=Validity(
notBefore=1641785732000, notAfter=1649043331000), subject=[[AttributeTypeAndValue(
type=2.5.4.3, value=www.google.com)]], subjectPublicKeyInfo=SubjectPublicKeyInfo(
algorithm=AlgorithmIdentifier(
algorithm=1.2.840.10045.2.1, parameters=1.2.840.10045.3.1.7), subjectPublicKey=BitString(
byteString=[size=65 hex=04d5c42e1bba569ae183f00484dff38b5e893dca2ffc52e5ee57634b385739cb2fd35ca57731ef5d248327be41ecce13f01c3b83dd3bbae589e1141a7dcb8ae2…], unusedBitsCount=0)), issuerUniqueID=null, subjectUniqueID=null, extensions=[Extension(
id=2.5.29.15, critical=true, value=BitString(
byteString=[hex=80], unusedBitsCount=7)), Extension(
id=2.5.29.37, critical=false, value=[1.3.6.1.5.5.7.3.1]), Extension(
id=2.5.29.19, critical=true, value=BasicConstraints(
ca=false, maxIntermediateCas=null)), Extension(
id=2.5.29.14, critical=false, value=[hex=0414ccfadcc51b82d2de8bbd7178015b6e0cb0154a62]), Extension(
id=2.5.29.35, critical=false, value=[hex=301680148a747faf85cdee95cd3d9cd0e24614f371351d27]), Extension(
id=1.3.6.1.5.5.7.1.1, critical=false, value=[AccessDescription(
accessMethod=1.3.6.1.5.5.7.48.1, accessLocation=(
IA5 STRING [128/6], http://ocsp.pki.goog/gts1c3)), AccessDescription(
accessMethod=1.3.6.1.5.5.7.48.2, accessLocation=(
IA5 STRING [128/6], http://pki.goog/repo/certs/gts1c3.der))]), Extension(
id=2.5.29.17, critical=false, value=[(
IA5 STRING [128/2], www.google.com)]), Extension(
id=2.5.29.32, critical=false, value=[hex=30183008060667810c010201300c060a2b06010401d679020503]), Extension(
id=2.5.29.31, critical=false, value=[hex=30333031a02fa02d862b687474703a2f2f63726c732e706b692e676f6f672f6774733163332f66564a7862562d4b746d6b2e63726c]), Extension(
id=1.3.6.1.4.1.11129.2.4.2, critical=false, value=[size=247 hex=0481f400f200770046a555eb75fa912030b5a28969f4f37d112c4174befd49b885abf2fc70fe6d470000017e42451dbd0000040300483046022100d532913f26…])]), signatureAlgorithm=AlgorithmIdentifier(
algorithm=1.2.840.113549.1.1.11, parameters=null), signatureValue=BitString(
byteString=[size=256 hex=3b32449208f4a944aa01efa9b26f600b0f95117694d8e36cf46de62be252112460fbafeaadfa4a1b44347835eb8050a0f3ad97ca031d723f8860267c936ba9b5…], unusedBitsCount=0))

Fails on

image image
yschimke commented 2 years ago

Basic Constraints from the Certificate shortly after X509 -> Certifikit.

image
yschimke commented 2 years ago

Suggestion from here that it should be [30, 0]

https://stackoverflow.com/questions/59432135/how-to-get-basicconstraints-extension-from-java-x509-certificate

yschimke commented 2 years ago

Investigating here https://github.com/cashapp/certifikit/pull/98

Diffs

:13:13:13:13:3:
:0c:0c:0c:0c:1:
yschimke commented 2 years ago

The Basic Constraint difference is 3->1

300c0603551d130101ff04023000
300c0603551d130101ff04021000
yschimke commented 2 years ago

Diffs of 13s

image
yschimke commented 2 years ago

Diffs from parsed content

From https://report-uri.com/home/pem_decoder

image
yschimke commented 2 years ago

https://letsencrypt.org/docs/a-warm-welcome-to-asn1-and-der/

The first thing to know about SEQUENCE is that it always uses Constructed encoding because it contains other objects. In other words, the value bytes of a SEQUENCE contain the concatenation of the encoded fields of that SEQUENCE (in the order those fields were defined). This also means that bit 6 of a SEQUENCE’s tag (the Constructed vs Primitive bit) is always set to 1. So even though the tag number for SEQUENCE is technically 0x10, its tag byte, once encoded, is always 0x30.

~I don't understand the tag classes~

From JDK BasicConstraintsExtension & DerValue

    /**
     * Tag value indicating an ASN.1
     * "SEQUENCE" (zero to N elements, order is significant).
     */
    public static final byte    tag_Sequence = 0x30;

    /**
     * Tag value indicating an ASN.1
     * "SEQUENCE OF" (one to N elements, order is significant).
     */
    public static final byte    tag_SequenceOf = 0x30;

From Adapters.sequence

    return BasicDerAdapter(
        name = name,
        tagClass = DerHeader.TAG_CLASS_UNIVERSAL,
        tag = 16L,
        codec = codec
    )
yschimke commented 2 years ago

I suspect Der.writer should have constructed = true when writing BasicConstraints

Temporary fix for the crash with

    try {
      block(content)
      if (name == "BasicConstraints") {
        constructed = true
      }
      constructedBit = if (constructed) 0b0010_0000 else 0
      constructed = true // The enclosing object is constructed.
    } finally {

Seemingly because both values are optional

    if (isOptional && value == defaultValue) {
      // Nothing to write!
      return
    }
yschimke commented 2 years ago

The other relates to the charset mapping. It's choosing UTF-8 over PrintableString.

https://www.alvestrand.no/objectid/2.5.4.6.html

oid: 2.5.4.6 - id-at-countryName 060355040613025553 vs 06035504060c025553

yschimke commented 2 years ago

Can flip these to get it working for this certificate.

  private val attributeTypeAndValue: BasicDerAdapter<AttributeTypeAndValue> = Adapters.sequence(
      "AttributeTypeAndValue",
      Adapters.OBJECT_IDENTIFIER,
      Adapters.any(
          String::class to Adapters.UTF8_STRING,
          Nothing::class to Adapters.PRINTABLE_STRING,

But UTF-8 was intentional as a safe default. However 2.5.4.6 - id-at-countryName is Printable String so effectively we don't have a distinct encoding.

yschimke commented 2 years ago

Addressed on other PR and I'll open another PR for our variable encoding.