Update WithValidateKey, and add tests+docs

Changes

@@ -31,6 +31,17 @@ v2.0.16 UNRELEASED
     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.
 
+[New Features]
+  * [jws] Added `jws.WithValidateKey()` to force calling `key.Validate()` before
+    signing or verification.
+
+  * [jws] `jws.Sign()` now returns a special type of error that can hold the
+    individual errors from the signers. The stringification is still the same
+    as before to preserve backwards compatibility.
+
+  * [jwk] Added `jwk.IsKeyValidationError` that checks if an error is an error
+    from `key.Validate()`.
+
 v2.0.15 19 20 Oct 2023
 [Bug fixes]
   * [jws] jws.Sign() now properly check for valid algorithm / key type pair when

jwk/ecdsa.go

@@ -260,14 +260,14 @@ func ecdsaValidateKey(k interface {
 
 func (k *ecdsaPrivateKey) Validate() error {
 	if err := ecdsaValidateKey(k, true); err != nil {
-		return fmt.Errorf(`jwk.ECDSAPrivateKey: failed to validate key: %w`, err)
+		return NewKeyValidationError(fmt.Errorf(`jwk.ECDSAPrivateKey: %w`, err))
 	}
 	return nil
 }
 
 func (k *ecdsaPublicKey) Validate() error {
 	if err := ecdsaValidateKey(k, false); err != nil {
-		return fmt.Errorf(`jwk.ECDSAPublicKey: failed to validate key: %w`, err)
+		return NewKeyValidationError(fmt.Errorf(`jwk.ECDSAPublicKey: %w`, err))
 	}
 	return nil
 }

jwk/jwk.go

@@ -12,6 +12,7 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"io"
 	"math/big"
@@ -757,3 +758,32 @@ func IsPrivateKey(k Key) (bool, error) {
 	}
 	return false, fmt.Errorf("jwk.IsPrivateKey: %T is not an asymmetric key", k)
 }
+
+type keyValidationErr struct {
+	err error
+}
+
+func (e *keyValidationErr) Error() string {
+	return fmt.Sprintf(`key validation failed: %s`, e.err)
+}
+
+func (e *keyValidationErr) Unwrap() error {
+	return e.err
+}
+
+func (e *keyValidationErr) Is(target error) bool {
+	_, ok := target.(*keyValidationErr)
+	return ok
+}
+
+// NewKeyValidationError wraps the given error with an error that denotes
+// `key.Validate()` has failed. This error type should ONLY be used as
+// return value from the `Validate()` method.
+func NewKeyValidationError(err error) error {
+	return &keyValidationErr{err: err}
+}
+
+func IsKeyValidationError(err error) bool {
+	var kve keyValidationErr
+	return errors.Is(err, &kve)
+}

jwk/okp.go

@@ -211,11 +211,17 @@ func validateOKPKey(key interface {
 func (k *okpPublicKey) Validate() error {
 	k.mu.RLock()
 	defer k.mu.RUnlock()
-	return validateOKPKey(k)
+	if err := validateOKPKey(k); err != nil {
+		return NewKeyValidationError(fmt.Errorf(`jwk.OKPPublicKey: %w`, err))
+	}
+	return nil
 }
 
 func (k *okpPrivateKey) Validate() error {
 	k.mu.RLock()
 	defer k.mu.RUnlock()
-	return validateOKPKey(k)
+	if err := validateOKPKey(k); err != nil {
+		return NewKeyValidationError(fmt.Errorf(`jwk.OKPPrivateKey: %w`, err))
+	}
+	return nil
 }

jwk/rsa.go

@@ -270,14 +270,14 @@ func validateRSAKey(key interface {
 
 func (k *rsaPrivateKey) Validate() error {
 	if err := validateRSAKey(k, true); err != nil {
-		return fmt.Errorf(`jwk.RSAPrivateKey: failed to validate key: %w`, err)
+		return NewKeyValidationError(fmt.Errorf(`jwk.RSAPrivateKey: %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 NewKeyValidationError(fmt.Errorf(`jwk.RSAPublicKey: %w`, err))
 	}
 	return nil
 }

jwk/symmetric.go

@@ -61,7 +61,7 @@ func (k *symmetricKey) PublicKey() (Key, error) {
 
 func (k *symmetricKey) Validate() error {
 	if len(k.Octets()) == 0 {
-		return fmt.Errorf(`missing "k" field`)
+		return NewKeyValidationError(fmt.Errorf(`jwk.SymmetricKey: missing "k" field`))
 	}
 	return nil
 }

jws/jws.go

@@ -28,6 +28,7 @@ import (
 	"crypto/ecdsa"
 	"crypto/ed25519"
 	"crypto/rsa"
+	"errors"
 	"fmt"
 	"io"
 	"reflect"
@@ -107,6 +108,21 @@ const (
 var _ = fmtInvalid
 var _ = fmtMax
 
+func validateKeyBeforeUse(key interface{}) error {
+	jwkKey, ok := key.(jwk.Key)
+	if !ok {
+		converted, err := jwk.FromRaw(key)
+		if err != nil {
+			return fmt.Errorf(`could not convert key of type %T to jwk.Key for validation: %w`, key, err)
+		}
+		jwkKey = converted
+	}
+	if err := jwkKey.Validate(); err != nil {
+		return err
+	}
+	return nil
+}
+
 // Sign generates a JWS message for the given payload and returns
 // it in serialized form, which can be in either compact or
 // JSON format. Default is compact.
@@ -149,6 +165,7 @@ func Sign(payload []byte, options ...SignOption) ([]byte, error) {
 	var signers []*payloadSigner
 	var detached bool
 	var noneSignature *payloadSigner
+	var validateKey bool
 	for _, option := range options {
 		//nolint:forcetypeassert
 		switch option.Ident() {
@@ -185,6 +202,8 @@ func Sign(payload []byte, options ...SignOption) ([]byte, error) {
 				return nil, fmt.Errorf(`jws.Sign: payload must be nil when jws.WithDetachedPayload() is specified`)
 			}
 			payload = option.Value().([]byte)
+		case identValidateKey{}:
+			validateKey = option.Value().(bool)
 		}
 	}
 
@@ -239,6 +258,12 @@ func Sign(payload []byte, options ...SignOption) ([]byte, error) {
 			// cheat. FIXXXXXXMEEEEEE
 			detached: detached,
 		}
+
+		if validateKey {
+			if err := validateKeyBeforeUse(signer.key); err != nil {
+				return nil, fmt.Errorf(`jws.Verify: %w`, err)
+			}
+		}
 		_, _, err := sig.Sign(payload, signer.signer, signer.key)
 		if err != nil {
 			return nil, fmt.Errorf(`failed to generate signature for signer #%d (alg=%s): %w`, i, signer.Algorithm(), err)
@@ -353,6 +378,7 @@ func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
 	verifyBuf := pool.GetBytesBuffer()
 	defer pool.ReleaseBytesBuffer(verifyBuf)
 
+	var errs []error
 	for i, sig := range msg.signatures {
 		verifyBuf.Reset()
 
@@ -391,16 +417,8 @@ func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
 				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)
+					if err := validateKeyBeforeUse(key); err != nil {
+						return nil, fmt.Errorf(`jws.Verify: %w`, err)
 					}
 				}
 				verifier, err := NewVerifier(alg)
@@ -409,6 +427,7 @@ func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
 				}
 
 				if err := verifier.Verify(verifyBuf.Bytes(), sig.signature, key); err != nil {
+					errs = append(errs, err)
 					continue
 				}
 
@@ -426,7 +445,33 @@ func Verify(buf []byte, options ...VerifyOption) ([]byte, error) {
 			}
 		}
 	}
-	return nil, fmt.Errorf(`could not verify message using any of the signatures or keys`)
+	return nil, &verifyError{errs: errs}
+}
+
+type verifyError struct {
+	// Note: when/if we can ditch Go < 1.20, we can change this to a simple
+	// `err error`, where the value is the result of `errors.Join()`
+	//
+	// We also need to implement Unwrap:
+	// func (e *verifyError) Unwrap() error {
+	//	return e.err
+	//}
+	//
+	// And finally, As() can go away
+	errs []error
+}
+
+func (e *verifyError) Error() string {
+	return `could not verify message using any of the signatures or keys`
+}
+
+func (e *verifyError) As(target interface{}) bool {
+	for _, err := range e.errs {
+		if errors.As(err, target) {
+			return true
+		}
+	}
+	return false
 }
 
 // get the value of b64 header field.

jws/jws_test.go

@@ -2121,3 +2121,36 @@ func TestUnpaddedSignatureR(t *testing.T) {
 	_, err = jws.Verify([]byte(unpadded), jws.WithKey(jwa.ES256, pubKey))
 	require.Error(t, err, `jws.Verify should fail`)
 }
+
+func TestValidateKey(t *testing.T) {
+	privKey, err := jwxtest.GenerateRsaJwk()
+	require.NoError(t, err, `jwxtest.GenerateRsaJwk should succeed`)
+
+	signed, err := jws.Sign([]byte("Lorem Ipsum"), jws.WithKey(jwa.RS256, privKey), jws.WithValidateKey(true))
+	require.NoError(t, err, `jws.Sign should succeed`)
+
+	// This should fail because D is empty
+	require.NoError(t, privKey.Set(jwk.RSADKey, []byte(nil)), `jwk.Set should succeed`)
+	_, err = jws.Sign([]byte("Lorem Ipsum"), jws.WithKey(jwa.RS256, privKey), jws.WithValidateKey(true))
+	require.Error(t, err, `jws.Sign should fail`)
+
+	pubKey, err := jwk.PublicKeyOf(privKey)
+	require.NoError(t, err, `jwk.PublicKeyOf should succeed`)
+
+	n := pubKey.(jwk.RSAPublicKey).N()
+
+	// Set N to an empty value
+	require.NoError(t, pubKey.Set(jwk.RSANKey, []byte(nil)), `jwk.Set should succeed`)
+
+	// This is going to fail regardless, because the public key is now
+	// invalid (empty N), but we want to make sure that it fails because
+	// of the validation failing
+	_, err = jws.Verify(signed, jws.WithKey(jwa.RS256, pubKey), jws.WithValidateKey(true))
+	require.Error(t, err, `jws.Verify should fail`)
+	require.True(t, jwk.IsKeyValidationError(err), `jwk.IsKeyValidationError should return true`)
+
+	// The following should now succeed, because N has been reinstated
+	require.NoError(t, pubKey.Set(jwk.RSANKey, n), `jwk.Set should succeed`)
+	_, err = jws.Verify(signed, jws.WithKey(jwa.RS256, pubKey), jws.WithValidateKey(true))
+	require.NoError(t, err, `jws.Verify should succeed`)
+}

jws/options.yaml

@@ -89,10 +89,10 @@ options:
       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
+    interface: SignVerifyOption
     argument_type: bool
     comment: |
-      WithValidateKey specifies whether the key used for verification
+      WithValidateKey specifies whether the key used for signing or 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