From 09d6cfa18c96b6f53c870ca56658fabc0a80983c Mon Sep 17 00:00:00 2001 From: Shiwei Zhang Date: Fri, 12 Jul 2024 01:13:07 +0800 Subject: [PATCH] refactor!: deprecate crypto elliptic (#187) Signed-off-by: Shiwei Zhang --- .github/workflows/ci.yml | 2 +- ecdsa_test.go | 12 ------------ go.mod | 2 +- key.go | 17 ++++++++--------- key_test.go | 11 ++--------- verifier.go | 12 +++++++++--- verifier_test.go | 15 ++++++++++++++- 7 files changed, 35 insertions(+), 36 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9693a83..8e8e9e6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 diff --git a/ecdsa_test.go b/ecdsa_test.go index 1cb2c12..e549c02 100644 --- a/ecdsa_test.go +++ b/ecdsa_test.go @@ -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) diff --git a/go.mod b/go.mod index 7e60365..fad879c 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/key.go b/key.go index 71ad331..f3dbbb9 100644 --- a/key.go +++ b/key.go @@ -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 @@ -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() @@ -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{ diff --git a/key_test.go b/key_test.go index 04f499f..6e5ee0c 100644 --- a/key_test.go +++ b/key_test.go @@ -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, diff --git a/verifier.go b/verifier.go index 69e576f..72d2039 100644 --- a/verifier.go +++ b/verifier.go @@ -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 { @@ -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, diff --git a/verifier_test.go b/verifier_test.go index 5295133..82d84ca 100644 --- a/verifier_test.go +++ b/verifier_test.go @@ -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 @@ -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 {