From 5d35bdf1dd698b2f681c16cc089c982a430e34cd Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Fri, 22 Apr 2022 09:16:55 -0700 Subject: [PATCH] Generate RFC 5280 conformant serial numbers Generating RFC 5280 conformant serial numbers is slightly treacherous. Section 4.1.2.2 dictates that conforming implementations "MUST NOT use serialNumber values longer than 20 octets". A seemingly obvious way to pick serials that conform to this requirement is choosing a random int between [0, 1 << 20*8), which is what the mitm package previously did. The pitfall here is that the DER encoding of integers uses the MSB to indicate the sign, so if encoding/asn1 is passed a positive big.Int for which the encoding has the MSB set it will prefix the encoding with a 0x00 byte. The result of this is that if a serial number is picked that is exactly 20 bytes, and has its MSB set, it will be encoded as 21 bytes. The simple solution is to just re-generate the serial if you happen to pick one which is exactly 20 bytes with the MSB set. Since there are a number of users of mitm.MaxSerialNumber (both within this project, and externally), a helper function is added that can be used as a drop-in replacement for the existing crypto/rand.Int calls. --- h2/testing/certs.go | 2 +- mitm/mitm.go | 25 +++++++++++++++++++++++-- mitm/mitm_test.go | 16 +++++++++++++++- 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/h2/testing/certs.go b/h2/testing/certs.go index fe73004e0..2300f2650 100644 --- a/h2/testing/certs.go +++ b/h2/testing/certs.go @@ -93,7 +93,7 @@ func initLocalhostCert(ca *x509.Certificate, caPriv crypto.PrivateKey) (*tls.Cer hasher.Write(pkixpub) keyID := hasher.Sum(nil) - serial, err := rand.Int(rand.Reader, mitm.MaxSerialNumber) + serial, err := mitm.GenerateSerial() if err != nil { return nil, fmt.Errorf("generating serial number: %w", err) } diff --git a/mitm/mitm.go b/mitm/mitm.go index 83ee60f5f..c90b636aa 100644 --- a/mitm/mitm.go +++ b/mitm/mitm.go @@ -36,9 +36,30 @@ import ( "github.com/google/martian/v3/log" ) +// GenerateSerial generates a RFC 5280 conformant serial number for use in +// X.509 certificates. +func GenerateSerial() (serial *big.Int, err error) { + for { + serial, err = rand.Int(rand.Reader, MaxSerialNumber) + if err != nil { + return nil, err + } + // If the generated serial is 20 bytes, check that the MSB is not set. If + // it is set, generate a new serial, as encoding/asn1 will prefix the serial + // with 0x00 so that it isn't interpreted as a negative integer, resulting + // in a 21 byte serial number. + if serialBytes := serial.Bytes(); len(serialBytes) < 20 || len(serialBytes) > 0 && serialBytes[0]&0x80 == 0 { + return serial, nil + } + } +} + // MaxSerialNumber is the upper boundary that is used to create unique serial // numbers for the certificate. This can be any unsigned integer up to 20 // bytes (2^(8*20)-1). +// +// NOTE: Rather than using this as an input to crypto/rand.Int, GenerateSerial +// should be used. var MaxSerialNumber = big.NewInt(0).SetBytes(bytes.Repeat([]byte{255}, 20)) // Config is a set of configuration values that are used to build TLS configs @@ -81,7 +102,7 @@ func NewAuthority(name, organization string, validity time.Duration) (*x509.Cert // TODO: keep a map of used serial numbers to avoid potentially reusing a // serial multiple times. - serial, err := rand.Int(rand.Reader, MaxSerialNumber) + serial, err := GenerateSerial() if err != nil { return nil, nil, err } @@ -261,7 +282,7 @@ func (c *Config) cert(hostname string) (*tls.Certificate, error) { log.Debugf("mitm: cache miss for %s", hostname) - serial, err := rand.Int(rand.Reader, MaxSerialNumber) + serial, err := GenerateSerial() if err != nil { return nil, err } diff --git a/mitm/mitm_test.go b/mitm/mitm_test.go index d9f584ea7..e264efb74 100644 --- a/mitm/mitm_test.go +++ b/mitm/mitm_test.go @@ -132,7 +132,7 @@ func TestCert(t *testing.T) { t.Fatal("x509c: got nil, want *x509.Certificate") } - if got := x509c.SerialNumber; got.Cmp(MaxSerialNumber) >= 0 { + if got := x509c.SerialNumber; got.Cmp(MaxSerialNumber) > 0 { t.Errorf("x509c.SerialNumber: got %v, want <= MaxSerialNumber", got) } if got, want := x509c.Subject.CommonName, "example.com"; got != want { @@ -203,3 +203,17 @@ func TestCert(t *testing.T) { t.Fatalf("x509c.IPAddresses: got %v, want %v", got, want) } } + +func TestGenerateSerial(t *testing.T) { + for i := 0; i < 100; i++ { + serial, err := GenerateSerial() + if err != nil { + t.Fatalf("GenerateSerial(): got %v, want no error", err) + } + if serial.Cmp(MaxSerialNumber) > 0 { + t.Errorf("serial: got %v, want <= MaxSerialNumber", serial) + } else if serialBytes := serial.Bytes(); len(serialBytes) == 20 && serialBytes[0]&0x80 != 0 { + t.Errorf("serial: got len=20 with MSB set, want MSB unset") + } + } +}