mirror of
https://github.com/golang/go
synced 2024-11-27 03:11:19 -07:00
crypto/x509: handle ECC private keys with the wrong length.
SEC-1 says: “The component privateKey is the private key defined to be the octet string of length ⌊log₂(n)/8⌋ (where n is the order of the curve)”. Previously the code for parsing ECC private keys would panic (on non-amd64) when the private was too long. It would also pass a too-short private key to crypto/elliptic, possibly resulting in undesirable behaviour. This change makes the parsing function handle both too much and too little padding because GnuTLS does the former and OpenSSL did the latter until 30cd4ff294252c4b6a4b69cbef6a5b4117705d22. It also causes serialisation to pad private keys correctly. Fixes #13699 Change-Id: If9c2faeaeb45af8a4d7770d784f3d2633e7f8290 Reviewed-on: https://go-review.googlesource.com/18094 Run-TryBot: Adam Langley <agl@golang.org> Reviewed-by: Adam Langley <agl@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
This commit is contained in:
parent
aaabe3d849
commit
9338f39459
@ -39,9 +39,14 @@ func MarshalECPrivateKey(key *ecdsa.PrivateKey) ([]byte, error) {
|
||||
if !ok {
|
||||
return nil, errors.New("x509: unknown elliptic curve")
|
||||
}
|
||||
|
||||
privateKeyBytes := key.D.Bytes()
|
||||
paddedPrivateKey := make([]byte, (key.Curve.Params().N.BitLen() + 7) / 8)
|
||||
copy(paddedPrivateKey[len(paddedPrivateKey) - len(privateKeyBytes):], privateKeyBytes)
|
||||
|
||||
return asn1.Marshal(ecPrivateKey{
|
||||
Version: 1,
|
||||
PrivateKey: key.D.Bytes(),
|
||||
PrivateKey: paddedPrivateKey,
|
||||
NamedCurveOID: oid,
|
||||
PublicKey: asn1.BitString{Bytes: elliptic.Marshal(key.Curve, key.X, key.Y)},
|
||||
})
|
||||
@ -71,13 +76,30 @@ func parseECPrivateKey(namedCurveOID *asn1.ObjectIdentifier, der []byte) (key *e
|
||||
}
|
||||
|
||||
k := new(big.Int).SetBytes(privKey.PrivateKey)
|
||||
if k.Cmp(curve.Params().N) >= 0 {
|
||||
curveOrder := curve.Params().N
|
||||
if k.Cmp(curveOrder) >= 0 {
|
||||
return nil, errors.New("x509: invalid elliptic curve private key value")
|
||||
}
|
||||
priv := new(ecdsa.PrivateKey)
|
||||
priv.Curve = curve
|
||||
priv.D = k
|
||||
priv.X, priv.Y = curve.ScalarBaseMult(privKey.PrivateKey)
|
||||
|
||||
privateKey := make([]byte, (curveOrder.BitLen() + 7) / 8)
|
||||
|
||||
// Some private keys have leading zero padding. This is invalid
|
||||
// according to [SEC1], but this code will ignore it.
|
||||
for len(privKey.PrivateKey) > len(privateKey) {
|
||||
if privKey.PrivateKey[0] != 0 {
|
||||
return nil, errors.New("x509: invalid private key length")
|
||||
}
|
||||
privKey.PrivateKey = privKey.PrivateKey[1:]
|
||||
}
|
||||
|
||||
// Some private keys remove all leading zeros, this is also invalid
|
||||
// according to [SEC1] but since OpenSSL used to do this, we ignore
|
||||
// this too.
|
||||
copy(privateKey[len(privateKey) - len(privKey.PrivateKey):], privKey.PrivateKey)
|
||||
priv.X, priv.Y = curve.ScalarBaseMult(privateKey)
|
||||
|
||||
return priv, nil
|
||||
}
|
||||
|
@ -10,21 +10,35 @@ import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
// Generated using:
|
||||
// openssl ecparam -genkey -name secp384r1 -outform PEM
|
||||
var ecPrivateKeyHex = `3081a40201010430bdb9839c08ee793d1157886a7a758a3c8b2a17a4df48f17ace57c72c56b4723cf21dcda21d4e1ad57ff034f19fcfd98ea00706052b81040022a16403620004feea808b5ee2429cfcce13c32160e1c960990bd050bb0fdf7222f3decd0a55008e32a6aa3c9062051c4cba92a7a3b178b24567412d43cdd2f882fa5addddd726fe3e208d2c26d733a773a597abb749714df7256ead5105fa6e7b3650de236b50`
|
||||
var ecKeyTests = []struct{
|
||||
derHex string
|
||||
shouldReserialize bool
|
||||
}{
|
||||
// Generated using:
|
||||
// openssl ecparam -genkey -name secp384r1 -outform PEM
|
||||
{"3081a40201010430bdb9839c08ee793d1157886a7a758a3c8b2a17a4df48f17ace57c72c56b4723cf21dcda21d4e1ad57ff034f19fcfd98ea00706052b81040022a16403620004feea808b5ee2429cfcce13c32160e1c960990bd050bb0fdf7222f3decd0a55008e32a6aa3c9062051c4cba92a7a3b178b24567412d43cdd2f882fa5addddd726fe3e208d2c26d733a773a597abb749714df7256ead5105fa6e7b3650de236b50", true},
|
||||
// This key was generated by GnuTLS and has illegal zero-padding of the
|
||||
// private key. See https://github.com/golang/go/issues/13699.
|
||||
{"3078020101042100f9f43a04b9bdc3ab01f53be6df80e7a7bc3eaf7b87fc24e630a4a0aa97633645a00a06082a8648ce3d030107a1440342000441a51bc318461b4c39a45048a16d4fc2a935b1ea7fe86e8c1fa219d6f2438f7c7fd62957d3442efb94b6a23eb0ea66dda663dc42f379cda6630b21b7888a5d3d", false},
|
||||
// This was generated using an old version of OpenSSL and is missing a
|
||||
// leading zero byte in the private key that should be present.
|
||||
{"3081db0201010441607b4f985774ac21e633999794542e09312073480baa69550914d6d43d8414441e61b36650567901da714f94dffb3ce0e2575c31928a0997d51df5c440e983ca17a00706052b81040023a181890381860004001661557afedd7ac8d6b70e038e576558c626eb62edda36d29c3a1310277c11f67a8c6f949e5430a37dcfb95d902c1b5b5379c389873b9dd17be3bdb088a4774a7401072f830fb9a08d93bfa50a03dd3292ea07928724ddb915d831917a338f6b0aecfbc3cf5352c4a1295d356890c41c34116d29eeb93779aab9d9d78e2613437740f6", false},
|
||||
}
|
||||
|
||||
func TestParseECPrivateKey(t *testing.T) {
|
||||
derBytes, _ := hex.DecodeString(ecPrivateKeyHex)
|
||||
key, err := ParseECPrivateKey(derBytes)
|
||||
if err != nil {
|
||||
t.Errorf("failed to decode EC private key: %s", err)
|
||||
}
|
||||
serialized, err := MarshalECPrivateKey(key)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to encode EC private key: %s", err)
|
||||
}
|
||||
if !bytes.Equal(serialized, derBytes) {
|
||||
t.Fatalf("serialized key differs: got %x, want %x", serialized, derBytes)
|
||||
for i, test := range ecKeyTests {
|
||||
derBytes, _ := hex.DecodeString(test.derHex)
|
||||
key, err := ParseECPrivateKey(derBytes)
|
||||
if err != nil {
|
||||
t.Fatalf("#%d: failed to decode EC private key: %s", i, err)
|
||||
}
|
||||
serialized, err := MarshalECPrivateKey(key)
|
||||
if err != nil {
|
||||
t.Fatalf("#%d: failed to encode EC private key: %s", i, err)
|
||||
}
|
||||
matches := bytes.Equal(serialized, derBytes)
|
||||
if matches != test.shouldReserialize {
|
||||
t.Fatalf("#%d: when serializing key: matches=%t, should match=%t: original %x, reserialized %x", i, matches, test.shouldReserialize, serialized, derBytes)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user