Add (jwk.Key).Validate

Changes

@@ -25,6 +25,11 @@ v2.0.16 UNRELEASED
     fail. Therefore this in itself should not pose any security risk, albeit
     allowing some illegally formated messages to be verified.
 
+  * [jwk] `jwk.Key` objects now have a `Validate()` method to validate the data
+    stored in the keys. However, this still does not necessarily mean that the key's
+    are valid for use in cryptographic operations. If `Validate()` is successful,
+    it only means that the keys are in the right _format_, including the presence
+    of required fields and that certain fields are have proper length, etc.
 
 v2.0.15 19 20 Oct 2023
 [Bug fixes]

jwk/ecdsa.go

@@ -226,3 +226,48 @@ func (k ecdsaPrivateKey) Thumbprint(hash crypto.Hash) ([]byte, error) {
 		base64.EncodeToString(ybuf),
 	), nil
 }
+
+func ecdsaValidateKey(h interface {
+	Crv() jwa.EllipticCurveAlgorithm
+	X() []byte
+	Y() []byte
+}, checkPrivate bool) error {
+	crv, ok := ecutil.CurveForAlgorithm(h.Crv())
+	if !ok {
+		return fmt.Errorf(`invalid curve algorithm %q`, h.Crv())
+	}
+
+	keySize := ecutil.CalculateKeySize(crv)
+	if x := h.X(); len(x) != keySize {
+		return fmt.Errorf(`invalid "x" length (%d) for curve %q`, len(x), crv.Params().Name)
+	}
+
+	if y := h.Y(); len(y) != keySize {
+		return fmt.Errorf(`invalid "y" length (%d) for curve %q`, len(y), crv.Params().Name)
+	}
+
+	if checkPrivate {
+		if priv, ok := h.(interface{ D() []byte }); ok {
+			if len(priv.D()) != keySize {
+				return fmt.Errorf(`invalid "d" length (%d) for curve %q`, len(priv.D()), crv.Params().Name)
+			}
+		} else {
+			return fmt.Errorf(`missing "d" value`)
+		}
+	}
+	return nil
+}
+
+func (h *ecdsaPrivateKey) Validate() error {
+	if err := ecdsaValidateKey(h, true); err != nil {
+		return fmt.Errorf(`jwk.ECDSAPrivateKey: failed to validate key: %w`, err)
+	}
+	return nil
+}
+
+func (h *ecdsaPublicKey) Validate() error {
+	if err := ecdsaValidateKey(h, false); err != nil {
+		return fmt.Errorf(`jwk.ECDSAPublicKey: failed to validate key: %w`, err)
+	}
+	return nil
+}

jwk/jwk_internal_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/lestrrat-go/jwx/v2/cert"
+	"github.com/lestrrat-go/jwx/v2/internal/base64"
 	"github.com/lestrrat-go/jwx/v2/internal/json"
 	"github.com/lestrrat-go/jwx/v2/jwa"
 	"github.com/stretchr/testify/assert"
@@ -129,9 +130,18 @@ func TestIterator(t *testing.T) {
 		{
 			Extras: map[string]interface{}{
 				ECDSACrvKey: jwa.P256,
-				ECDSAXKey:   []byte("MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4"),
-				ECDSAYKey:   []byte("4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM"),
-				ECDSADKey:   []byte("870MB6gfuTJ4HtUnUvYMyJpr5eUZNP4Bk43bVdj3eAE"),
+				ECDSAXKey: (func() []byte {
+					s, _ := base64.DecodeString("MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4")
+					return s
+				})(),
+				ECDSAYKey: (func() []byte {
+					s, _ := base64.DecodeString("4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM")
+					return s
+				})(),
+				ECDSADKey: (func() []byte {
+					s, _ := base64.DecodeString("870MB6gfuTJ4HtUnUvYMyJpr5eUZNP4Bk43bVdj3eAE")
+					return s
+				})(),
 			},
 			Func: func() Key {
 				return newECDSAPrivateKey()
@@ -140,8 +150,14 @@ func TestIterator(t *testing.T) {
 		{
 			Extras: map[string]interface{}{
 				ECDSACrvKey: jwa.P256,
-				ECDSAXKey:   []byte("MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4"),
-				ECDSAYKey:   []byte("4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM"),
+				ECDSAXKey: (func() []byte {
+					s, _ := base64.DecodeString("MKBCTNIcKUSDii11ySs3526iDZ8AiTo7Tu6KPAqv7D4")
+					return s
+				})(),
+				ECDSAYKey: (func() []byte {
+					s, _ := base64.DecodeString("4Etl6SRW2YiLUrN5vfvVHuhp7x8PxltmWWlbbM4IFyM")
+					return s
+				})(),
 			},
 			Func: func() Key {
 				return newECDSAPublicKey()
@@ -184,6 +200,7 @@ func TestIterator(t *testing.T) {
 			}
 
 			if !assert.NoError(t, json.Unmarshal(buf, key2), `json.Unmarshal should succeed`) {
+				t.Logf("%s", buf)
 				return
 			}
 

jwk/jwk_test.go

@@ -2217,3 +2217,55 @@ func TestGH947(t *testing.T) {
 	var exported []byte
 	require.Error(t, k.Raw(&exported), `(okpkey).Raw with 0-length OKP key should fail`)
 }
+
+func TestValidation(t *testing.T) {
+	{
+		key, err := jwxtest.GenerateRsaJwk()
+		require.NoError(t, err, `jwx.GenerateRsaJwk should succeed`)
+		require.NoError(t, key.Validate(), `key.Validate should succeed (vanilla key)`)
+
+		require.NoError(t, key.Set(jwk.RSADKey, []byte(nil)), `key.Set should succeed`)
+		require.Error(t, key.Validate(), `key.Validate should fail`)
+	}
+
+	{
+		key, err := jwxtest.GenerateEcdsaJwk()
+		require.NoError(t, err, `jwx.GenerateEcdsaJwk should succeed`)
+		require.NoError(t, key.Validate(), `key.Validate should succeed`)
+
+		//nolint:forcetypeassert
+		x := key.(jwk.ECDSAPrivateKey).X()
+		require.NoError(t, key.Set(jwk.ECDSAXKey, x[:len(x)/2]), `key.Set should succeed`)
+		require.Error(t, key.Validate(), `key.Validate should fail`)
+
+		require.NoError(t, key.Set(jwk.ECDSAXKey, x), `key.Set should succeed`)
+		require.NoError(t, key.Validate(), `key.Validate should succeed`)
+
+		require.NoError(t, key.Set(jwk.ECDSADKey, []byte(nil)), `key.Set should succeed`)
+		require.Error(t, key.Validate(), `key.Validate should fail`)
+	}
+
+	{
+		key, err := jwxtest.GenerateEd25519Jwk()
+		require.NoError(t, err, `jwx.GenerateEd25519Jwk should succeed`)
+		require.NoError(t, key.Validate(), `key.Validate should succeed`)
+		x := key.(jwk.OKPPrivateKey).X()
+		require.NoError(t, key.Set(jwk.OKPXKey, []byte(nil)), `key.Set should succeed`)
+		require.Error(t, key.Validate(), `key.Validate should fail`)
+
+		require.NoError(t, key.Set(jwk.OKPXKey, x), `key.Set should succeed`)
+		require.NoError(t, key.Validate(), `key.Validate should succeed`)
+
+		require.NoError(t, key.Set(jwk.OKPDKey, []byte(nil)), `key.Set should succeed`)
+		require.Error(t, key.Validate(), `key.Validate should fail`)
+	}
+
+	{
+		key, err := jwxtest.GenerateSymmetricJwk()
+		require.NoError(t, err, `jwx.GenerateSymmetricJwk should succeed`)
+		require.NoError(t, key.Validate(), `key.Validate should succeed`)
+
+		require.NoError(t, key.Set(jwk.SymmetricOctetsKey, []byte(nil)), `key.Set should succeed`)
+		require.Error(t, key.Validate(), `key.Validate should fail`)
+	}
+}

jwk/okp.go

@@ -187,3 +187,35 @@ func (k okpPrivateKey) Thumbprint(hash crypto.Hash) ([]byte, error) {
 		base64.EncodeToString(k.x),
 	), nil
 }
+
+func validateOKPKey(key interface {
+	Crv() jwa.EllipticCurveAlgorithm
+	X() []byte
+}) error {
+	if key.Crv() == jwa.InvalidEllipticCurve {
+		return fmt.Errorf(`invalid curve algorithm`)
+	}
+
+	if len(key.X()) == 0 {
+		return fmt.Errorf(`missing "x" field`)
+	}
+
+	if priv, ok := key.(interface{ D() []byte }); ok {
+		if len(priv.D()) == 0 {
+			return fmt.Errorf(`missing "d" field`)
+		}
+	}
+	return nil
+}
+
+func (k *okpPublicKey) Validate() error {
+	k.mu.RLock()
+	defer k.mu.RUnlock()
+	return validateOKPKey(k)
+}
+
+func (k *okpPrivateKey) Validate() error {
+	k.mu.RLock()
+	defer k.mu.RUnlock()
+	return validateOKPKey(k)
+}

jwk/rsa.go

@@ -241,3 +241,43 @@ func rsaThumbprint(hash crypto.Hash, key *rsa.PublicKey) ([]byte, error) {
 	}
 	return h.Sum(nil), nil
 }
+
+func validateRSAKey(key interface {
+	N() []byte
+	E() []byte
+}, checkPrivate bool) error {
+	if len(key.N()) == 0 {
+		// Ideally we would like to check for the actual length, but unlike
+		// EC keys, we have nothing in the key itself that will tell us
+		// how many bits this key should have.
+		return fmt.Errorf(`missing "n" value`)
+	}
+	if len(key.E()) == 0 {
+		return fmt.Errorf(`missing "e" value`)
+	}
+	if checkPrivate {
+		if priv, ok := key.(interface{ D() []byte }); ok {
+			if len(priv.D()) == 0 {
+				return fmt.Errorf(`missing "d" value`)
+			}
+		} else {
+			return fmt.Errorf(`missing "d" value`)
+		}
+	}
+
+	return nil
+}
+
+func (k *rsaPrivateKey) Validate() error {
+	if err := validateRSAKey(k, true); err != nil {
+		return fmt.Errorf(`jwk.RSAPrivateKey: failed to validate key: %w`, err)
+	}
+	return nil
+}
+
+func (k *rsaPublicKey) Validate() error {
+	if err := validateRSAKey(k, false); err != nil {
+		return fmt.Errorf(`jwk.RSAPublicKey: failed to validate key: %w`, err)
+	}
+	return nil
+}

jwk/symmetric.go

@@ -58,3 +58,10 @@ func (k *symmetricKey) PublicKey() (Key, error) {
 	}
 	return newKey, nil
 }
+
+func (k *symmetricKey) Validate() error {
+	if len(k.Octets()) == 0 {
+		return fmt.Errorf(`missing "k" field`)
+	}
+	return nil
+}

jws/jws.go

@@ -290,6 +290,7 @@ func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
 	var detachedPayload []byte
 	var keyProviders []KeyProvider
 	var keyUsed interface{}
+	var validateKey bool
 
 	ctx := context.Background()
 
@@ -316,6 +317,8 @@ func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
 			keyUsed = option.Value()
 		case identContext{}:
 			ctx = option.Value().(context.Context)
+		case identValidateKey{}:
+			validateKey = option.Value().(bool)
 		default:
 			return nil, fmt.Errorf(`invalid jws.VerifyOption %q passed`, `With`+strings.TrimPrefix(fmt.Sprintf(`%T`, option.Ident()), `jws.ident`))
 		}
@@ -386,6 +389,20 @@ func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
 				//nolint:forcetypeassert
 				alg := pair.alg.(jwa.SignatureAlgorithm)
 				key := pair.key
+
+				if validateKey {
+					jwkKey, ok := key.(jwk.Key)
+					if !ok {
+						converted, err := jwk.FromRaw(key)
+						if err != nil {
+							return nil, fmt.Errorf(`jws.Verify: could not convert key of type %T to jwk.Key for validation: %w`, key, err)
+						}
+						jwkKey = converted
+					}
+					if err := jwkKey.Validate(); err != nil {
+						return nil, fmt.Errorf(`jws.Verify: failed to validate key: %w`, err)
+					}
+				}
 				verifier, err := NewVerifier(alg)
 				if err != nil {
 					return nil, fmt.Errorf(`failed to create verifier for algorithm %q: %w`, alg, err)

jws/jws_test.go

@@ -1129,7 +1129,8 @@ func TestVerifyNonUniqueKid(t *testing.T) {
 			Name: `match 2 keys via same "kid" and different key type / alg`,
 			Key: func() jwk.Key {
 				privateKey, _ := jwxtest.GenerateEcdsaKey(jwa.P256)
-				wrongKey, _ := jwk.PublicKeyOf(privateKey)
+				wrongKey, err := jwk.PublicKeyOf(privateKey)
+				require.NoError(t, err, `jwk.PublicKeyOf should succeed`)
 				_ = wrongKey.Set(jwk.KeyIDKey, kid)
 				_ = wrongKey.Set(jwk.AlgorithmKey, jwa.ES256K)
 				return wrongKey

jws/options.yaml

@@ -88,6 +88,27 @@ options:
       `jwk.Key` here unless you are 100% sure that all keys that you
       have provided are instances of `jwk.Key` (remember that the
       jwx API allows users to specify a raw key such as *rsa.PublicKey)
+  - ident: ValidateKey
+    interface: VerifyOption
+    argument_type: bool
+    comment: |
+      WithValidateKey specifies whether the key used for verification
+      should be validated before using. Note that this means calling
+      `key.Validate()` on the key, which in turn means that your key
+      must be a `jwk.Key` instance, or a key that can be converted to
+      a `jwk.Key` by calling `jwk.FromRaw()`. This means that your
+      custom hardware-backed keys will probably not work.
+
+      You can directly call `key.Validate()` yourself if you need to
+      mix keys that cannot be converted to `jwk.Key`.
+
+      Please also note that use of this option will also result in
+      one extra conversion of raw keys to a `jwk.Key` instance. If you
+      care about shaving off as much as possible, consider using a
+      pre-validated key instead of using this option to validate
+      the key on-demand each time.
+
+      By default, the key is not validated.
   - ident: InferAlgorithmFromKey
     interface: WithKeySetSuboption
     argument_type: bool