Skip to content

Commit

Permalink
consolidate Key.validate errors
Browse files Browse the repository at this point in the history 
Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>
  • Loading branch information
@qmuntal
qmuntal committed Jul 13, 2023
1 parent 377ee80 commit f02809a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 40 deletions.
44 changes: 19 additions & 25 deletions key.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -472,31 +472,25 @@ 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
}
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.
}
Expand Down
30 changes: 15 additions & 15 deletions key_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,15 +357,15 @@ 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{
0xa1, // map (2)
0x01, 0x02, // kty: EC2
},
want: nil,
wantErr: ErrInvalidKey.Error(),
wantErr: "invalid key: required parameters missing",
}, {
name: "OKP invalid curve",
data: []byte{
Expand All @@ -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{
Expand All @@ -398,15 +398,15 @@ 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{
0xa1, // map (1)
0x01, 0x04, // kty: Symmetric
},
want: nil,
wantErr: ErrInvalidKey.Error(),
wantErr: "invalid key: required parameters missing",
}, {
name: "EC2 invalid algorithm",
data: []byte{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit f02809a

Please sign in to comment.