Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/x509: CreateCertificate creates invalid serial number field #33310

Closed
zelbrium opened this issue Jul 26, 2019 · 4 comments
Closed

crypto/x509: CreateCertificate creates invalid serial number field #33310

zelbrium opened this issue Jul 26, 2019 · 4 comments
Assignees
Milestone

Comments

@zelbrium
Copy link

@zelbrium zelbrium commented Jul 26, 2019

What version of Go are you using (go version)?

go1.12.7 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.12.7/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.12.7/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

I created an x509 certificate with x509.CreateCertificate from crypto/x509 with the following code (modified from Martian's mitm NewAuthority function):

package main

import (
	"bytes"
	"crypto/rand"
	"crypto/rsa"
	"crypto/sha1"
	"crypto/x509"
	"crypto/x509/pkix"
	"fmt"
	"log"
	"math/big"
	"time"
)

var MaxSerialNumber = big.NewInt(0).SetBytes(bytes.Repeat([]byte{255}, 20))

func main() {
	name := "Test name"
	organization := "Test Org"
	validity := time.Duration(24 * time.Hour)

	priv, err := rsa.GenerateKey(rand.Reader, 2048)
	if err != nil {
		log.Fatal(err)
	}
	pub := priv.Public()

	// Subject Key Identifier support for end entity certificate.
	// https://www.ietf.org/rfc/rfc3280.txt (section 4.2.1.2)
	pkixpub, err := x509.MarshalPKIXPublicKey(pub)
	if err != nil {
		log.Fatal(err)
	}
	h := sha1.New()
	h.Write(pkixpub)
	keyID := h.Sum(nil)

	serial := MaxSerialNumber

	tmpl := &x509.Certificate{
		SerialNumber: serial,
		Subject: pkix.Name{
			CommonName:   name,
			Organization: []string{organization},
		},
		SubjectKeyId:          keyID,
		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature | x509.KeyUsageCertSign,
		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
		BasicConstraintsValid: true,
		NotBefore:             time.Now().Add(-validity),
		NotAfter:              time.Now().Add(validity),
		DNSNames:              []string{name},
		IsCA:                  true,
	}

	fmt.Println(MaxSerialNumber)
	fmt.Println(serial)

	raw, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, pub, priv)
	fmt.Println(err, len(raw), raw)
}

(Unfortunately, this won't run in play.golang.org; it errors with process took too long.)

What did you expect to see?

A certificate with 20 bytes of serial number (which is the maximum allowed) where all 20 bytes are 255.

What did you see instead?

A certificate with 21 bytes of serial number, specifically, a 0 byte followed by 20 255 bytes. The field length correctly indicates 21 bytes, but some software will refuse to read a certificate with that many bytes.

The leading 0 byte is added whenever the big-end byte is >= 128. This can be seen by changing serial := MaxSerialNumber to serial := big.NewInt(127) and then serial := big.NewInt(128). The 127 serial number will have a field length of 1 byte, followed by the single byte 127. The 128 serial number will have a field length of 2 bytes, followed by the the two bytes 0 128.

@julieqiu julieqiu changed the title crypto/x509 CreateCertificate creates invalid serial number field crypto/x509: CreateCertificate creates invalid serial number field Jul 29, 2019
@julieqiu
Copy link
Contributor

@julieqiu julieqiu commented Jul 29, 2019

@FiloSottile FiloSottile added this to the Go1.14 milestone Jul 30, 2019
@FiloSottile FiloSottile self-assigned this Jul 30, 2019
@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Oct 1, 2019

What you are observing is a quirk of ASN.1 encoding. Numbers that start with a byte higher than 127 are negative numbers, so a sequence of 20 255 bytes would be negative, which is also not something that conformant CAs are supposed to do. To express a positive number that would otherwise start with a byte higher than 127, we have to add a 0 byte in front of it.

This means that the highest serial number you can fit in 20 bytes is []byte{127, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255}.

@FiloSottile FiloSottile closed this Oct 1, 2019
@zelbrium
Copy link
Author

@zelbrium zelbrium commented Oct 1, 2019

The reason for adding a 0 makes sense, but given that some software (in my case, Mozilla's NSS library) fails to read certificates with more than 20 serial number bytes, one option is to not add the 0 if it results in 21 total bytes. The RFC says

 Note: Non-conforming CAs may issue certificates with serial numbers
   that are negative, or zero.  Certificate users SHOULD be prepared to
   gracefully handle such certificates.

so leaving a negative serial number is not ideal, but software should be prepared to handle that situation.

@FiloSottile
Copy link
Member

@FiloSottile FiloSottile commented Oct 2, 2019

We can't turn a number negative against the wishes of the application. If you want to set a negative number, you can set SerialNumber to a negative big.Int.

@golang golang locked and limited conversation to collaborators Oct 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.