Validate key sizes and allow signing with empty public keys (#159)

Signed-off-by: qmuntal <qmuntaldiaz@microsoft.com>

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,14 +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 || 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
@@ -457,25 +472,25 @@ func (k Key) validate(op KeyOp) error {
 			}
 		}
 		if crv == CurveInvalid || (len(x) == 0 && len(d) == 0) {
-			return ErrInvalidKey
+			return errReqParamsMissing
+		}
+		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.
 	}
@@ -540,6 +555,18 @@ func (k *Key) MarshalCBOR() ([]byte, error) {
 		existing[lbl] = struct{}{}
 		tmp[lbl] = v
 	}
+	if k.KeyType == 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 0 < len(x) && len(x) < size {
+				tmp[KeyLabelEC2X] = append(make([]byte, size-len(x), size), x...)
+			}
+			if 0 < len(y) && len(y) < size {
+				tmp[KeyLabelEC2Y] = append(make([]byte, size-len(y), size), y...)
+			}
+		}
+	}
 	return encMode.Marshal(tmp)
 }
 
@@ -672,14 +699,6 @@ func (k *Key) PrivateKey() (crypto.PrivateKey, error) {
 
 	switch alg {
 	case AlgorithmES256, AlgorithmES384, AlgorithmES512:
-		_, x, y, d := k.EC2()
-		// RFC8152 allows omitting X and Y from private keys;
-		// crypto.PrivateKey assumes they are available.
-		// see https://www.rfc-editor.org/rfc/rfc8152#section-13.1.1
-		if len(x) == 0 || len(y) == 0 {
-			return nil, ErrEC2NoPub
-		}
-
 		var curve elliptic.Curve
 
 		switch alg {
@@ -691,22 +710,24 @@ func (k *Key) PrivateKey() (crypto.PrivateKey, error) {
 			curve = elliptic.P521()
 		}
 
-		priv := &ecdsa.PrivateKey{
-			PublicKey: ecdsa.PublicKey{Curve: curve, X: new(big.Int), Y: new(big.Int)},
-			D:         new(big.Int),
+		_, 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)
 		}
-		priv.X.SetBytes(x)
-		priv.Y.SetBytes(y)
-		priv.D.SetBytes(d)
+		bd := new(big.Int).SetBytes(d)
 
-		return priv, nil
+		return &ecdsa.PrivateKey{
+			PublicKey: ecdsa.PublicKey{Curve: curve, X: bx, Y: by},
+			D:         bd,
+		}, nil
 	case AlgorithmEd25519:
 		_, x, d := k.OKP()
-		// RFC8152 allows omitting X from private keys;
-		// crypto.PrivateKey assumes it is available.
-		// see https://www.rfc-editor.org/rfc/rfc8152#section-13.2
 		if len(x) == 0 {
-			return nil, ErrOKPNoPub
+			return ed25519.NewKeyFromSeed(d), nil
 		}
 
 		buf := make([]byte, ed25519.PrivateKeySize)
@@ -820,6 +841,19 @@ func algorithmFromEllipticCurve(c elliptic.Curve) Algorithm {
 	}
 }
 
+func curveSize(crv Curve) int {
+	var bitSize int
+	switch crv {
+	case CurveP256:
+		bitSize = elliptic.P256().Params().BitSize
+	case CurveP384:
+		bitSize = elliptic.P384().Params().BitSize
+	case CurveP521:
+		bitSize = elliptic.P521().Params().BitSize
+	}
+	return (bitSize + 7) / 8
+}
+
 func decodeBytes(dic map[interface{}]interface{}, lbl interface{}) (b []byte, ok bool, err error) {
 	val, ok := dic[lbl]
 	if !ok {