letsencrypt / website

Let's Encrypt Website and Documentation
https://letsencrypt.org
Mozilla Public License 2.0
841 stars 573 forks source link

Guide to integrating ARI: Go code is less efficient than necessary? #1670

Closed mholt closed 2 months ago

mholt commented 4 months ago

In testing my ARI implementation, I noticed that my initial code to craft the ARI cert ID, simply by using leaf.SerialNumber.Bytes(), results in the same bytes as the code in the post at https://letsencrypt.org/2024/04/25/guide-to-integrating-ari-into-existing-acme-clients which calls asn1.Marshal() and then strips the 2 leading bytes.

SerialNumber.Bytes():      2be9dda95b3b220bb693cfe2f48c8e6161a2
ASN.1 Marshaled DER Bytes: 2be9dda95b3b220bb693cfe2f48c8e6161a2

In Go, is marshaling ASN.1 really necessary? Or is this just a fluke. I don't know enough about DER to be sure.

beautifulentropy commented 2 months ago

In DER encoding, integers are represented in two's complement. If an integer's highest bit is set, it's interpreted as negative. To prevent this for large positive numbers, DER prepends a zero byte when necessary. In the case above, using leaf.SerialNumber.Bytes() directly yields the same result as asn1.Marshal() followed by stripping the first two bytes because the most significant byte of your serial number is less than 0x80. This means there's no need for a leading zero to ensure it's seen as positive. While this approach seems adequate for this specific serial number, DER marshaling is generally recommended to handle potential cases where the serial number might start with a byte ≥ 0x80, ensuring it's correctly interpreted as positive.

This change to the ARI draft was the result of the following issue: https://github.com/aarongable/draft-acme-ari/issues/52

package main

import (
    "encoding/asn1"
    "fmt"
    "math/big"
)

func main() {
    // Two serial numbers: one that does not start with a high bit set and one that does.
    serials := []string{"123456", "80ABCDEF"}

    for _, hexSerial := range serials {
        serialBigInt, _ := new(big.Int).SetString(hexSerial, 16)

        rawBytes := serialBigInt.Bytes()
        fmt.Printf("Raw Bytes for %s: %x\n", hexSerial, rawBytes)

        derBytes, err := asn1.Marshal(serialBigInt)
        if err != nil {
            fmt.Println("Error marshalling to ASN.1:", err)
            continue
        }

        fmt.Printf("ASN.1 DER Bytes for %s: %x\n", hexSerial, derBytes)

        derIntegerBytes := derBytes[2:]
        fmt.Printf("DER Integer Bytes for %s (excluding tag and length): %x\n\n", hexSerial, derIntegerBytes)
    }
}

https://go.dev/play/p/2aaQK3bJaAE

beautifulentropy commented 2 months ago

I'm going to close this out for now but feel free to reach out if you need any further clarification.

mholt commented 2 months ago

Ah, that makes sense. Thanks for the excellent explainer. I didn't know that!

DER/ASN.1 is a minefield :dizzy_face: