mirror of
https://github.com/golang/go
synced 2024-09-29 13:24:28 -06:00
crypto/x509: don't create certs with negative serials
Refuse to create certificates with negative serial numbers, as they are explicitly disallowed by RFC 5280. We still allow parsing certificates with negative serial numbers, because in the past there were buggy CA implementations which would produce them (although there are currently *no* trusted certificates that have this issue). We may want to revisit this decision if we can find metrics about the prevalence of this issue in enterprise settings. Change-Id: I131262008db99b6354f542f335abc68775a2d6d0 Reviewed-on: https://go-review.googlesource.com/c/go/+/400494 Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Roland Shoemaker <roland@golang.org> Run-TryBot: Roland Shoemaker <roland@golang.org> Auto-Submit: Roland Shoemaker <roland@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
parent
5c707f5f3a
commit
082cfabf12
@ -1478,13 +1478,17 @@ func CreateCertificate(rand io.Reader, template, parent *Certificate, pub, priv
|
|||||||
return nil, errors.New("x509: no SerialNumber given")
|
return nil, errors.New("x509: no SerialNumber given")
|
||||||
}
|
}
|
||||||
|
|
||||||
// RFC 5280 Section 4.1.2.2: serial number must not be longer than 20 octets
|
// RFC 5280 Section 4.1.2.2: serial number must positive and should not be longer
|
||||||
|
// than 20 octets.
|
||||||
//
|
//
|
||||||
// We cannot simply check for len(serialBytes) > 20, because encoding/asn1 may
|
// We cannot simply check for len(serialBytes) > 20, because encoding/asn1 may
|
||||||
// pad the slice in order to prevent the integer being mistaken for a negative
|
// pad the slice in order to prevent the integer being mistaken for a negative
|
||||||
// number (DER uses the high bit of the left-most byte to indicate the sign.),
|
// number (DER uses the high bit of the left-most byte to indicate the sign.),
|
||||||
// so we need to double check the composition of the serial if it is exactly
|
// so we need to double check the composition of the serial if it is exactly
|
||||||
// 20 bytes.
|
// 20 bytes.
|
||||||
|
if template.SerialNumber.Sign() == -1 {
|
||||||
|
return nil, errors.New("x509: serial number must be positive")
|
||||||
|
}
|
||||||
serialBytes := template.SerialNumber.Bytes()
|
serialBytes := template.SerialNumber.Bytes()
|
||||||
if len(serialBytes) > 20 || (len(serialBytes) == 20 && serialBytes[0]&0x80 != 0) {
|
if len(serialBytes) > 20 || (len(serialBytes) == 20 && serialBytes[0]&0x80 != 0) {
|
||||||
return nil, errors.New("x509: serial number exceeds 20 octets")
|
return nil, errors.New("x509: serial number exceeds 20 octets")
|
||||||
|
@ -602,11 +602,7 @@ func TestCreateSelfSignedCertificate(t *testing.T) {
|
|||||||
for _, test := range tests {
|
for _, test := range tests {
|
||||||
commonName := "test.example.com"
|
commonName := "test.example.com"
|
||||||
template := Certificate{
|
template := Certificate{
|
||||||
// SerialNumber is negative to ensure that negative
|
SerialNumber: big.NewInt(1),
|
||||||
// values are parsed. This is due to the prevalence of
|
|
||||||
// buggy code that produces certificates with negative
|
|
||||||
// serial numbers.
|
|
||||||
SerialNumber: big.NewInt(-1),
|
|
||||||
Subject: pkix.Name{
|
Subject: pkix.Name{
|
||||||
CommonName: commonName,
|
CommonName: commonName,
|
||||||
Organization: []string{"Σ Acme Co"},
|
Organization: []string{"Σ Acme Co"},
|
||||||
@ -3628,3 +3624,40 @@ func TestCreateCertificateLongSerial(t *testing.T) {
|
|||||||
t.Errorf("CreateCertificate returned unexpected error: want %q, got %q", expectedErr, err)
|
t.Errorf("CreateCertificate returned unexpected error: want %q, got %q", expectedErr, err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var negativeSerialCert = `-----BEGIN CERTIFICATE-----
|
||||||
|
MIIBBTCBraADAgECAgH/MAoGCCqGSM49BAMCMA0xCzAJBgNVBAMTAjopMB4XDTIy
|
||||||
|
MDQxNDIzNTYwNFoXDTIyMDQxNTAxNTYwNFowDTELMAkGA1UEAxMCOikwWTATBgcq
|
||||||
|
hkjOPQIBBggqhkjOPQMBBwNCAAQ9ezsIsj+q17K87z/PXE/rfGRN72P/Wyn5d6oo
|
||||||
|
5M0ZbSatuntMvfKdX79CQxXAxN4oXk3Aov4jVSG12AcDI8ShMAoGCCqGSM49BAMC
|
||||||
|
A0cAMEQCIBzfBU5eMPT6m5lsR6cXaJILpAaiD9YxOl4v6dT3rzEjAiBHmjnHmAss
|
||||||
|
RqUAyJKFzqZxOlK2q4j2IYnuj5+LrLGbQA==
|
||||||
|
-----END CERTIFICATE-----`
|
||||||
|
|
||||||
|
func TestParseNegativeSerial(t *testing.T) {
|
||||||
|
pemBlock, _ := pem.Decode([]byte(negativeSerialCert))
|
||||||
|
_, err := ParseCertificate(pemBlock.Bytes)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("failed to parse certificate: %s", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCreateNegativeSerial(t *testing.T) {
|
||||||
|
k, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
tmpl := &Certificate{
|
||||||
|
SerialNumber: big.NewInt(-1),
|
||||||
|
Subject: pkix.Name{
|
||||||
|
CommonName: ":)",
|
||||||
|
},
|
||||||
|
NotAfter: time.Now().Add(time.Hour),
|
||||||
|
NotBefore: time.Now().Add(-time.Hour),
|
||||||
|
}
|
||||||
|
expectedErr := "x509: serial number must be positive"
|
||||||
|
_, err = CreateCertificate(rand.Reader, tmpl, tmpl, k.Public(), k)
|
||||||
|
if err == nil || err.Error() != expectedErr {
|
||||||
|
t.Errorf("CreateCertificate returned unexpected error: want %q, got %q", expectedErr, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user