Skip to content

Normalize EC2 key size at input time to ensure RFC 9053 compliance #223

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kentakayama wants to merge 9 commits into
base: main
Choose a base branch
from
+138 −34
Open

Normalize EC2 key size at input time to ensure RFC 9053 compliance #223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cb33f56
update: always handle x, y and d with full size for the curve
kentakayama Dec 4, 2025
1337f29
add: test ensuring the NewKeyEC2 always returns correct byte size of …
kentakayama Dec 4, 2025
f3b1be1
add: failure test ensuring invalid size of x and d for OKP
kentakayama Dec 4, 2025
b9d3c0b
update: Normalize and validate EC2 key lengths to ensure compliance w…
kentakayama Dec 4, 2025
52af599
fix: typo
kentakayama Dec 4, 2025
88057f2
refactor: clarify error handling for missing vs malformed params for …
kentakayama Dec 6, 2025
13e02ff
refactor: comments
kentakayama Dec 6, 2025
b745bfa
refactor: split curveSize into keySizeEC2 and keySizeOKP
kentakayama Dec 8, 2025
be9afe2
fix: bug in validate() with length 0 slice
kentakayama Dec 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 82 additions & 26 deletions key.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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) {
Comment on lines +445 to +453
Copy link
Contributor

@thomas-fossati thomas-fossati Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why these changes were necessary?

Copy link
Author

@kentakayama kentakayama Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I'm sorry -- while checking this error case, I also fixed a bug introduced by me in be9afe2 while checking this error case.)

As I mentioned here, I think it is important to distinguish between:

  • the zero value of []byte (i.e. nil), and
  • a non-nil []byte whose length is 0.

The following test demonstrates the differences clearly:

func TestZeroValueVSZeroLength(t *testing.T) {
	key := Key{
		Type: KeyTypeEC2,
		Params: map[any]any{
			KeyLabelEC2Curve: CurveP256,
		},
	}
	_, err := key.PublicKey()
	if err != nil {
		fmt.Printf("not set: %s\n", err.Error())
	}

	var ec256x []byte
	var ec256y []byte
	key = Key{
		Type: KeyTypeEC2,
		Params: map[any]any{
			KeyLabelEC2Curve: CurveP256,
			KeyLabelEC2X:     ec256x,
			KeyLabelEC2Y:     ec256y,
		},
	}
	_, err = key.PublicKey()
	if err != nil {
		fmt.Printf("zero value of []byte: %s\n", err.Error())
	}

	key = Key{
		Type: KeyTypeEC2,
		Params: map[any]any{
			KeyLabelEC2Curve: CurveP256,
			KeyLabelEC2X:     [0]byte{}, // will not be returned by EC2()
			KeyLabelEC2Y:     [0]byte{}, // will not be returned by EC2()
		},
	}
	_, err = key.PublicKey()
	if err != nil {
		fmt.Printf("[0]byte{} (type mismatch): %s\n", err.Error())
	}

	key = Key{
		Type: KeyTypeEC2,
		Params: map[any]any{
			KeyLabelEC2Curve: CurveP256,
			KeyLabelEC2X:     []byte{},
			KeyLabelEC2Y:     []byte{},
		},
	}
	_, err = key.PublicKey()
	if err != nil {
		fmt.Printf("[]byte{}: %s\n", err.Error())
	}

	key = Key{
		Type: KeyTypeEC2,
		Params: map[any]any{
			KeyLabelEC2Curve: CurveP256,
			KeyLabelEC2X:     make([]byte, 0),
			KeyLabelEC2Y:     make([]byte, 0),
		},
	}
	_, err = key.PublicKey()
	if err != nil {
		fmt.Printf("make([]byte, 0): %s\n", err.Error())
	}

	key = Key{
		Type: KeyTypeEC2,
		Params: map[any]any{
			KeyLabelEC2Curve: CurveP256,
			KeyLabelEC2X:     make([]byte, 0, 32),
			KeyLabelEC2Y:     make([]byte, 0, 32),
		},
	}
	_, err = key.PublicKey()
	if err != nil {
		fmt.Printf("make([]byte, 0, 32): %s\n", err.Error())
	}
}

With previous implementation, all of these cases produced the same error:

not set: cannot create PrivateKey from EC2 key: missing x or y
zero value of []byte: cannot create PrivateKey from EC2 key: missing x or y
[0]byte{} (type mismatch): cannot create PrivateKey from EC2 key: missing x or y
[]byte{}: cannot create PrivateKey from EC2 key: missing x or y
make([]byte, 0): cannot create PrivateKey from EC2 key: missing x or y
make([]byte, 0, 32): cannot create PrivateKey from EC2 key: missing x or y

This Pull Request updates the behavior so that cases where a non-nil but empty (does not match to the expected size) slice is produced are reported more accurately:

not set: cannot create PrivateKey from EC2 key: missing x or y
zero value of []byte: cannot create PrivateKey from EC2 key: missing x or y
[0]byte{} (type mismatch): cannot create PrivateKey from EC2 key: missing x or y
[]byte{}: invalid key: coordinate size mismatch
make([]byte, 0): invalid key: coordinate size mismatch
make([]byte, 0, 32): invalid key: coordinate size mismatch

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 x != nil && len(x) != size {
return errCoordSizeMismatch
}
if y != nil && len(y) != size {
return errCoordSizeMismatch
}
if d != nil && len(d) != size {
return errCoordSizeMismatch
}
}
switch crv {
Expand All @@ -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 x != nil && len(x) != size {
return errCoordSizeMismatch
}
if d != nil && len(d) != size {
return errCoordSizeMismatch
}
}
switch crv {
case CurveP256, CurveP384, CurveP521:
Expand Down Expand Up @@ -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...)
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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:
Expand All @@ -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 {
Expand Down
64 changes: 56 additions & 8 deletions key_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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())
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1564,7 +1604,7 @@ func TestKey_PrivateKey(t *testing.T) {
},
},
nil,
"invalid key: overflowing coordinate",
errCoordSizeMismatch.Error(),
}, {
"OKP incorrect d size", &Key{
Type: KeyTypeOKP,
Expand All @@ -1575,7 +1615,7 @@ func TestKey_PrivateKey(t *testing.T) {
},
},
nil,
"invalid key: overflowing coordinate",
errCoordSizeMismatch.Error(),
}, {
"EC2 missing D", &Key{
Type: KeyTypeEC2,
Expand Down Expand Up @@ -1610,7 +1650,7 @@ func TestKey_PrivateKey(t *testing.T) {
},
},
nil,
"invalid key: overflowing coordinate",
errCoordSizeMismatch.Error(),
}, {
"EC2 incorrect y size", &Key{
Type: KeyTypeEC2,
Expand All @@ -1622,7 +1662,7 @@ func TestKey_PrivateKey(t *testing.T) {
},
},
nil,
"invalid key: overflowing coordinate",
errCoordSizeMismatch.Error(),
}, {
"EC2 incorrect d size", &Key{
Type: KeyTypeEC2,
Expand All @@ -1634,7 +1674,7 @@ func TestKey_PrivateKey(t *testing.T) {
},
},
nil,
"invalid key: overflowing coordinate",
errCoordSizeMismatch.Error(),
},
}
for _, tt := range tests {
Expand Down Expand Up @@ -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
}