Skip to content

Commit

Permalink
refactor!: deprecate crypto elliptic (#187)
Browse files Browse the repository at this point in the history
Signed-off-by: Shiwei Zhang <shizh@microsoft.com>
  • Loading branch information
shizhMSFT authored Jul 11, 2024
1 parent 2300d5c commit 09d6cfa
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 36 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ jobs:
strategy:
fail-fast: false
matrix:
go-version: [1.18, 1.19]
go-version: [1.21, 1.22]
runs-on: ubuntu-latest
steps:
- name: Install Go
Expand Down
12 changes: 0 additions & 12 deletions ecdsa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,18 +172,6 @@ func generateTestECDSAKey(t *testing.T) *ecdsa.PrivateKey {
return key
}

func Test_customCurveKeySigner(t *testing.T) {
// https://github.com/veraison/go-cose/issues/59
pCustom := *elliptic.P256().Params()
pCustom.Name = "P-custom"
pCustom.BitSize /= 2
key, err := ecdsa.GenerateKey(&pCustom, rand.Reader)
if err != nil {
t.Fatalf("ecdsa.GenerateKey() error = %v", err)
}
testSignVerify(t, AlgorithmES256, key, false)
}

func Test_ecdsaKeySigner(t *testing.T) {
key := generateTestECDSAKey(t)
testSignVerify(t, AlgorithmES256, key, false)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/veraison/go-cose

go 1.18
go 1.21

require github.com/fxamacker/cbor/v2 v2.5.0

Expand Down
17 changes: 8 additions & 9 deletions key.go
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,7 @@ func (k *Key) PublicKey() (crypto.PublicKey, error) {
}

// PrivateKey returns a crypto.PrivateKey generated using Key's parameters.
// Compressed point is not supported for EC2 keys.
func (k *Key) PrivateKey() (crypto.PrivateKey, error) {
if err := k.validate(KeyOpSign); err != nil {
return nil, err
Expand All @@ -703,8 +704,12 @@ func (k *Key) PrivateKey() (crypto.PrivateKey, error) {

switch alg {
case AlgorithmES256, AlgorithmES384, AlgorithmES512:
var curve elliptic.Curve
_, x, y, d := k.EC2()
if len(x) == 0 || len(y) == 0 {
return nil, fmt.Errorf("%w: compressed point not supported", ErrInvalidPrivKey)
}

var curve elliptic.Curve
switch alg {
case AlgorithmES256:
curve = elliptic.P256()
Expand All @@ -714,14 +719,8 @@ func (k *Key) PrivateKey() (crypto.PrivateKey, error) {
curve = elliptic.P521()
}

_, x, y, d := k.EC2()
var bx, by *big.Int
if len(x) == 0 || len(y) == 0 {
bx, by = curve.ScalarBaseMult(d)
} else {
bx = new(big.Int).SetBytes(x)
by = new(big.Int).SetBytes(y)
}
bx := new(big.Int).SetBytes(x)
by := new(big.Int).SetBytes(y)
bd := new(big.Int).SetBytes(d)

return &ecdsa.PrivateKey{
Expand Down
11 changes: 2 additions & 9 deletions key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1487,15 +1487,8 @@ func TestKey_PrivateKey(t *testing.T) {
KeyLabelEC2D: ec256d,
},
},
&ecdsa.PrivateKey{
PublicKey: ecdsa.PublicKey{
Curve: elliptic.P256(),
X: new(big.Int).SetBytes(ec256x),
Y: new(big.Int).SetBytes(ec256y),
},
D: new(big.Int).SetBytes(ec256d),
},
"",
nil,
"invalid private key: compressed point not supported",
}, {
"CurveP384", &Key{
Type: KeyTypeEC2,
Expand Down
12 changes: 9 additions & 3 deletions verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ type DigestVerifier interface {
// NewVerifier returns a verifier with a given public key.
// Only golang built-in crypto public keys of type `*rsa.PublicKey`,
// `*ecdsa.PublicKey`, and `ed25519.PublicKey` are accepted.
// When `*ecdsa.PublicKey` is specified, its curve must be supported by
// crypto/ecdh.
//
// The returned signer for rsa and ecdsa keys also implements `cose.DigestSigner`.
// The returned signer for rsa and ecdsa keys also implements
// `cose.DigestSigner`.
func NewVerifier(alg Algorithm, key crypto.PublicKey) (Verifier, error) {
var errReason string
switch alg {
Expand All @@ -60,8 +63,11 @@ func NewVerifier(alg Algorithm, key crypto.PublicKey) (Verifier, error) {
if !ok {
return nil, fmt.Errorf("%v: %w", alg, ErrInvalidPubKey)
}
if !vk.Curve.IsOnCurve(vk.X, vk.Y) {
return nil, errors.New("public key point is not on curve")
if _, err := vk.ECDH(); err != nil {
if err.Error() == "ecdsa: invalid public key" {
return nil, fmt.Errorf("%v: %w", alg, ErrInvalidPubKey)
}
return nil, fmt.Errorf("%v: %w: %v", alg, ErrInvalidPubKey, err)
}
return &ecdsaVerifier{
alg: alg,
Expand Down
15 changes: 14 additions & 1 deletion verifier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ func TestNewVerifier(t *testing.T) {
// craft an EC public key with the x-coord not on curve
ecdsaKeyPointNotOnCurve := generateBogusECKey()

// craft an EC public key with a curve not supported by crypto/ecdh
ecdsaKeyUnsupportedCurve := &ecdsa.PublicKey{
Curve: ecdsaKey.Curve.Params(),
X: ecdsaKey.X,
Y: ecdsaKey.Y,
}

// run tests
tests := []struct {
name string
Expand Down Expand Up @@ -125,7 +132,13 @@ func TestNewVerifier(t *testing.T) {
name: "bogus ecdsa public key (point not on curve)",
alg: AlgorithmES256,
key: ecdsaKeyPointNotOnCurve,
wantErr: "public key point is not on curve",
wantErr: "ES256: invalid public key",
},
{
name: "ecdsa public key with unsupported curve",
alg: AlgorithmES256,
key: ecdsaKeyUnsupportedCurve,
wantErr: "ES256: invalid public key: ecdsa: unsupported curve by crypto/ecdh",
},
}
for _, tt := range tests {
Expand Down

0 comments on commit 09d6cfa

Please sign in to comment.