From f02809acf3afa31f5f60a7321777acdc779197f2 Mon Sep 17 00:00:00 2001 From: qmuntal Date: Thu, 13 Jul 2023 08:50:48 +0200 Subject: [PATCH] consolidate Key.validate errors Signed-off-by: qmuntal --- key.go | 44 +++++++++++++++++++------------------------- key_test.go | 30 +++++++++++++++--------------- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/key.go b/key.go index 890f64c..e6b307b 100644 --- a/key.go +++ b/key.go @@ -414,6 +414,15 @@ func NewKeyFromPrivate(priv crypto.PrivateKey) (*Key, error) { } } +var ( + // The following errors are used multiple times + // in Key.validate. We declare them here to avoid + // duplication. They are not considered public errors. + errCoordOverflow = fmt.Errorf("%w: overflowing coordinate", ErrInvalidKey) + errReqParamsMissing = fmt.Errorf("%w: required parameters missing", ErrInvalidKey) + errInvalidCurve = fmt.Errorf("%w: curve not supported for the given key type", ErrInvalidKey) +) + // Validate ensures that the parameters set inside the Key are internally // consistent (e.g., that the key type is appropriate to the curve). // It also checks that the key is valid for the requested operation. @@ -432,29 +441,20 @@ func (k Key) validate(op KeyOp) error { } } if crv == CurveInvalid || (len(x) == 0 && len(y) == 0 && len(d) == 0) { - return ErrInvalidKey + return errReqParamsMissing } if size := curveSize(crv); size > 0 { // RFC 8152 Section 13.1.1 says that x and y leading zero octets MUST be preserved, // but the Go crypto/elliptic package trims them. So we relax the check // here to allow for omitted leading zero octets, we will add them back // when marshaling. - if len(x) > size { - return fmt.Errorf("invalid x size: expected lower or equal to %d, got %d", size, len(x)) - } - if len(y) > size { - return fmt.Errorf("invalid y size: expected lower or equal to %d, got %d", size, len(y)) - } - if len(d) > size { - return fmt.Errorf("invalid d size: expected lower or equal to %d, got %d", size, len(d)) + if len(x) > size || len(y) > size || len(d) > size { + return errCoordOverflow } } switch crv { case CurveX25519, CurveX448, CurveEd25519, CurveEd448: - return fmt.Errorf( - "Key type mismatch for curve %q (must be OKP, found EC2)", - crv.String(), - ) + return errInvalidCurve default: // ok -- a key may contain a currently unsupported curve // see https://www.rfc-editor.org/rfc/rfc8152#section-13.1.1 @@ -472,20 +472,14 @@ func (k Key) validate(op KeyOp) error { } } if crv == CurveInvalid || (len(x) == 0 && len(d) == 0) { - return ErrInvalidKey - } - if len(x) > 0 && len(x) != ed25519.PublicKeySize { - return fmt.Errorf("invalid x size: expected %d, got %d", ed25519.PublicKeySize, len(x)) + return errReqParamsMissing } - if len(d) > 0 && len(d) != ed25519.SeedSize { - return fmt.Errorf("invalid d size: expected %d, got %d", ed25519.SeedSize, len(d)) + if (len(x) > 0 && len(x) != ed25519.PublicKeySize) || (len(d) > 0 && len(d) != ed25519.SeedSize) { + return errCoordOverflow } switch crv { case CurveP256, CurveP384, CurveP521: - return fmt.Errorf( - "Key type mismatch for curve %q (must be EC2, found OKP)", - crv.String(), - ) + return errInvalidCurve default: // ok -- a key may contain a currently unsupported curve // see https://www.rfc-editor.org/rfc/rfc8152#section-13.2 @@ -493,10 +487,10 @@ func (k Key) validate(op KeyOp) error { case KeyTypeSymmetric: k := k.Symmetric() if len(k) == 0 { - return ErrInvalidKey + return errReqParamsMissing } case KeyTypeInvalid: - return errors.New("invalid kty value 0") + return fmt.Errorf("%w: kty value 0", ErrInvalidKey) default: // Unknown key type, we can't validate custom parameters. } diff --git a/key_test.go b/key_test.go index 3363be5..f234a3a 100644 --- a/key_test.go +++ b/key_test.go @@ -357,7 +357,7 @@ func TestKey_UnmarshalCBOR(t *testing.T) { 0x01, 0x01, // kty: OKP }, want: nil, - wantErr: ErrInvalidKey.Error(), + wantErr: "invalid key: required parameters missing", }, { name: "EC2 missing curve", data: []byte{ @@ -365,7 +365,7 @@ func TestKey_UnmarshalCBOR(t *testing.T) { 0x01, 0x02, // kty: EC2 }, want: nil, - wantErr: ErrInvalidKey.Error(), + wantErr: "invalid key: required parameters missing", }, { name: "OKP invalid curve", data: []byte{ @@ -379,7 +379,7 @@ func TestKey_UnmarshalCBOR(t *testing.T) { 0x28, 0x14, 0x87, 0xef, 0x4a, 0xe6, 0x7b, 0x46, }, want: nil, - wantErr: `Key type mismatch for curve "P-256" (must be EC2, found OKP)`, + wantErr: "invalid key: curve not supported for the given key type", }, { name: "EC2 invalid curve", data: []byte{ @@ -398,7 +398,7 @@ func TestKey_UnmarshalCBOR(t *testing.T) { 0x28, 0x14, 0x87, 0xef, 0x4a, 0xe6, 0x7b, 0x46, }, want: nil, - wantErr: `Key type mismatch for curve "Ed25519" (must be OKP, found EC2)`, + wantErr: "invalid key: curve not supported for the given key type", }, { name: "Symmetric missing K", data: []byte{ @@ -406,7 +406,7 @@ func TestKey_UnmarshalCBOR(t *testing.T) { 0x01, 0x04, // kty: Symmetric }, want: nil, - wantErr: ErrInvalidKey.Error(), + wantErr: "invalid key: required parameters missing", }, { name: "EC2 invalid algorithm", data: []byte{ @@ -864,7 +864,7 @@ func TestNewOKPKey(t *testing.T) { }, { name: "x and d missing", args: args{AlgorithmEd25519, nil, nil}, want: nil, - wantErr: ErrInvalidKey.Error(), + wantErr: "invalid key: required parameters missing", }, } for _, tt := range tests { @@ -943,7 +943,7 @@ func TestNewEC2Key(t *testing.T) { }, { name: "x, y and d missing", args: args{AlgorithmES512, nil, nil, nil}, want: nil, - wantErr: ErrInvalidKey.Error(), + wantErr: "invalid key: required parameters missing", }, } for _, tt := range tests { @@ -1301,7 +1301,7 @@ func TestKey_Signer(t *testing.T) { }, }, AlgorithmInvalid, - `Key type mismatch for curve "P-256" (must be EC2, found OKP)`, + "invalid key: curve not supported for the given key type", }, { "can't sign", &Key{ @@ -1385,7 +1385,7 @@ func TestKey_Verifier(t *testing.T) { }, }, AlgorithmInvalid, - `Key type mismatch for curve "P-256" (must be EC2, found OKP)`, + "invalid key: curve not supported for the given key type", }, { "can't verify", &Key{ @@ -1570,7 +1570,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid x size: expected 32, got 10", + "invalid key: overflowing coordinate", }, { "OKP incorrect d size", &Key{ KeyType: KeyTypeOKP, @@ -1581,7 +1581,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid d size: expected 32, got 5", + "invalid key: overflowing coordinate", }, { "EC2 missing D", &Key{ KeyType: KeyTypeEC2, @@ -1616,7 +1616,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid x size: expected lower or equal to 32, got 48", + "invalid key: overflowing coordinate", }, { "EC2 incorrect y size", &Key{ KeyType: KeyTypeEC2, @@ -1628,7 +1628,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid y size: expected lower or equal to 32, got 48", + "invalid key: overflowing coordinate", }, { "EC2 incorrect d size", &Key{ KeyType: KeyTypeEC2, @@ -1640,7 +1640,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid d size: expected lower or equal to 32, got 48", + "invalid key: overflowing coordinate", }, } for _, tt := range tests { @@ -1734,7 +1734,7 @@ func TestKey_PublicKey(t *testing.T) { KeyType: KeyTypeInvalid, }, nil, - `invalid kty value 0`, + `invalid key: kty value 0`, }, { "OKP missing X", &Key{ KeyType: KeyTypeOKP,