diff --git a/key.go b/key.go index b657bff..3483406 100644 --- a/key.go +++ b/key.go @@ -336,14 +336,19 @@ func NewKeyEC2(alg Algorithm, x, y, d []byte) (*Key, error) { KeyLabelEC2Curve: curve, }, } + + // RFC 9053 Section 7.1.1 says that x and y leading zero octets + // MUST be preserved, but the Go crypto/elliptic package trims them. + // Since x, y might be used before marshaling, we add 0x00 padding here. + size := keySizeEC2(curve) if x != nil { - key.Params[KeyLabelEC2X] = x + key.Params[KeyLabelEC2X] = append(make([]byte, size-len(x), size), x...) } if y != nil { - key.Params[KeyLabelEC2Y] = y + key.Params[KeyLabelEC2Y] = append(make([]byte, size-len(y), size), y...) } if d != nil { - key.Params[KeyLabelEC2D] = d + key.Params[KeyLabelEC2D] = append(make([]byte, size-len(d), size), d...) } if err := key.validate(KeyOpReserved); err != nil { return nil, err @@ -422,9 +427,9 @@ 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) + errCoordSizeMismatch = fmt.Errorf("%w: coordinate size mismatch", 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 @@ -434,26 +439,51 @@ func (k Key) validate(op KeyOp) error { switch k.Type { case KeyTypeEC2: crv, x, y, d := k.EC2() + // Check that required parameters are present based on the key operation. switch op { case KeyOpVerify: - if len(x) == 0 || len(y) == 0 { + if x == nil || y == nil { return ErrEC2NoPub } case KeyOpSign: - if len(d) == 0 { + if d == nil { return ErrNotPrivKey } } - if crv == CurveReserved || (len(x) == 0 && len(y) == 0 && len(d) == 0) { + if crv == CurveReserved || (x == nil && y == nil && d == nil) { 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 || len(y) > size || len(d) > size { - return errCoordOverflow + + // If the curve size is known, validate the length of each parameter if present. + if size := keySizeEC2(crv); size > 0 { + if len(y) == 0 && len(x) == size+1 { + // NOTE: RFC 9053 Section 7.1.1 describes compressed points in COSE_Key + // using a boolean y-coordinate value (false/true). However, this code + // currently assumes SEC1-style compression, where 0x02 or 0x03 is prepended + // to the x-coordinate. + // + // This behavior may change in the future, for example, we might compute the + // y-coordinate during UnmarshalCBOR, and MarshalCBOR would avoid emitting + // compressed points entirely. + // + // In that case, this conditional may become unnecessary, since the general + // length check below (`len(x) > 0 && len(x) != size`) would already catch + // invalid compressed input. + // + // See discussion in https://github.com/veraison/go-cose/pull/223 . + // Consider revisiting this logic in a future update. + return fmt.Errorf("%w: compressed point not supported", ErrInvalidPubKey) + } + + // If present, x, y, and d must match the expected size. + if len(x) > 0 && len(x) != size { + return errCoordSizeMismatch + } + if len(y) > 0 && len(y) != size { + return errCoordSizeMismatch + } + if len(d) > 0 && len(d) != size { + return errCoordSizeMismatch } } switch crv { @@ -465,21 +495,30 @@ func (k Key) validate(op KeyOp) error { } case KeyTypeOKP: crv, x, d := k.OKP() + // Check that required parameters are present based on the key operation. switch op { case KeyOpVerify: - if len(x) == 0 { + if x == nil { return ErrOKPNoPub } case KeyOpSign: - if len(d) == 0 { + if d == nil { return ErrNotPrivKey } } - if crv == CurveReserved || (len(x) == 0 && len(d) == 0) { + if crv == CurveReserved || (x == nil && d == nil) { return errReqParamsMissing } - if (len(x) > 0 && len(x) != ed25519.PublicKeySize) || (len(d) > 0 && len(d) != ed25519.SeedSize) { - return errCoordOverflow + + // If the curve size is known, validate the length of each parameter if present. + if size := keySizeOKP(crv); size > 0 { + // If present, x and d must match the expected size. + if len(x) > 0 && len(x) != size { + return errCoordSizeMismatch + } + if len(d) > 0 && len(d) != size { + return errCoordSizeMismatch + } } switch crv { case CurveP256, CurveP384, CurveP521: @@ -562,7 +601,7 @@ func (k *Key) MarshalCBOR() ([]byte, error) { if k.Type == KeyTypeEC2 { // If EC2 key, ensure that x and y are padded to the correct size. crv, x, y, _ := k.EC2() - if size := curveSize(crv); size > 0 { + if size := keySizeEC2(crv); size > 0 { if 0 < len(x) && len(x) < size { tmp[KeyLabelEC2X] = append(make([]byte, size-len(x), size), x...) } @@ -705,9 +744,6 @@ func (k *Key) PrivateKey() (crypto.PrivateKey, error) { switch alg { case AlgorithmES256, AlgorithmES384, AlgorithmES512: _, 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 { @@ -844,9 +880,10 @@ func algorithmFromEllipticCurve(c elliptic.Curve) Algorithm { } } -func curveSize(crv Curve) int { +func keySizeEC2(crv Curve) int { var bitSize int switch crv { + // SEC 1: Standards for Efficient Cryptography case CurveP256: bitSize = elliptic.P256().Params().BitSize case CurveP384: @@ -857,6 +894,25 @@ func curveSize(crv Curve) int { return (bitSize + 7) / 8 } +func keySizeOKP(crv Curve) int { + switch crv { + // RFC 8032: Edwards-Curve Digital Signature Algorithm (EdDSA) + case CurveEd25519: + return ed25519.PublicKeySize // 32 + case CurveEd448: + return 57 + + // RFC 7748: Elliptic Curves for Security + case CurveX25519: + return 32 + case CurveX448: + return 56 + + default: + return 0 + } +} + func decodeBytes(dic map[any]any, lbl any) (b []byte, ok bool, err error) { val, ok := dic[lbl] if !ok { diff --git a/key_test.go b/key_test.go index 6e5ee0c..b96c217 100644 --- a/key_test.go +++ b/key_test.go @@ -866,6 +866,14 @@ func TestNewKeyOKP(t *testing.T) { name: "x and d missing", args: args{AlgorithmEdDSA, nil, nil}, want: nil, wantErr: "invalid key: required parameters missing", + }, { + name: "invalid x", args: args{AlgorithmEdDSA, x[:31], d}, + want: nil, + wantErr: errCoordSizeMismatch.Error(), + }, { + name: "invalid d", args: args{AlgorithmEdDSA, x, d[:31]}, + want: nil, + wantErr: errCoordSizeMismatch.Error(), }, } for _, tt := range tests { @@ -883,6 +891,7 @@ func TestNewKeyOKP(t *testing.T) { } func TestNewNewKeyEC2(t *testing.T) { + // newEC2 always return the full size []byte ec256x, ec256y, ec256d := newEC2(t, elliptic.P256()) ec384x, ec384y, ec384d := newEC2(t, elliptic.P384()) ec521x, ec521y, ec521d := newEC2(t, elliptic.P521()) @@ -911,6 +920,19 @@ func TestNewNewKeyEC2(t *testing.T) { }, }, wantErr: "", + }, { + name: "short x, y and d but valid", args: args{AlgorithmES256, ec256x[:31], ec256y[:31], ec256d[:31]}, + want: &Key{ + Type: KeyTypeEC2, + Algorithm: AlgorithmES256, + Params: map[any]any{ + KeyLabelEC2Curve: CurveP256, + KeyLabelEC2X: append([]byte{0x00}, ec256x[:31]...), + KeyLabelEC2Y: append([]byte{0x00}, ec256y[:31]...), + KeyLabelEC2D: append([]byte{0x00}, ec256d[:31]...), + }, + }, + wantErr: "", }, { name: "valid ES384", args: args{AlgorithmES384, ec384x, ec384y, ec384d}, want: &Key{ @@ -1480,15 +1502,33 @@ func TestKey_PrivateKey(t *testing.T) { }, "", }, { - "CurveP256 missing x and y", &Key{ + "CurveP256 compressed x", &Key{ Type: KeyTypeEC2, Params: map[any]any{ KeyLabelEC2Curve: CurveP256, + KeyLabelEC2X: append([]byte{0x02}, ec256x...), KeyLabelEC2D: ec256d, }, }, nil, - "invalid private key: compressed point not supported", + "invalid public key: compressed point not supported", + }, { + "CurveP256 missing x and y", &Key{ + Type: KeyTypeEC2, + Params: map[any]any{ + KeyLabelEC2Curve: CurveP256, + KeyLabelEC2D: ec256d, + }, + }, + &ecdsa.PrivateKey{ + PublicKey: ecdsa.PublicKey{ + Curve: elliptic.P256(), + X: new(big.Int).SetBytes([]byte{}), + Y: new(big.Int).SetBytes([]byte{}), + }, + D: new(big.Int).SetBytes(ec256d), + }, + "", }, { "CurveP384", &Key{ Type: KeyTypeEC2, @@ -1564,7 +1604,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid key: overflowing coordinate", + errCoordSizeMismatch.Error(), }, { "OKP incorrect d size", &Key{ Type: KeyTypeOKP, @@ -1575,7 +1615,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid key: overflowing coordinate", + errCoordSizeMismatch.Error(), }, { "EC2 missing D", &Key{ Type: KeyTypeEC2, @@ -1610,7 +1650,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid key: overflowing coordinate", + errCoordSizeMismatch.Error(), }, { "EC2 incorrect y size", &Key{ Type: KeyTypeEC2, @@ -1622,7 +1662,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid key: overflowing coordinate", + errCoordSizeMismatch.Error(), }, { "EC2 incorrect d size", &Key{ Type: KeyTypeEC2, @@ -1634,7 +1674,7 @@ func TestKey_PrivateKey(t *testing.T) { }, }, nil, - "invalid key: overflowing coordinate", + errCoordSizeMismatch.Error(), }, } for _, tt := range tests { @@ -1890,5 +1930,13 @@ func newEC2(t *testing.T, crv elliptic.Curve) (x, y, d []byte) { if err != nil { t.Fatal(err) } - return priv.X.Bytes(), priv.Y.Bytes(), priv.D.Bytes() + + size := (crv.Params().BitSize + 7) / 8 + x = make([]byte, size) + copy(x[size-len(priv.X.Bytes()):], priv.X.Bytes()) + y = make([]byte, size) + copy(y[size-len(priv.Y.Bytes()):], priv.Y.Bytes()) + d = make([]byte, size) + copy(d[size-len(priv.D.Bytes()):], priv.D.Bytes()) + return x, y, d }