diff --git a/README.md b/README.md index 9ff1d79..fac2691 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,8 @@ in the [API reference](https://pkg.go.dev/github.com/yaronf/httpsign). **v0.6.1** breaks foreign-JWS **verify** again: prefer `NewJWSVerifier(allowed, key, …)` (infer alg); use `NewJWSVerifierWithAlg` when needed; pass a `JWSAlgAllowlist` (`nil` skips policy). `NewJWSSigner` defaults to / requires `SignAlg(false)`; use `NewJWSSignerFromJWK` when the private key is a JWK. +**v0.6.2** restores opaque `crypto.Signer` (HSM/KMS) for foreign JWS RSA/ECDSA/Ed25519 and ML-DSA on sign and verify. See [internal-docs/RELEASE-v0.6.2.md](internal-docs/RELEASE-v0.6.2.md). + | Caller | Change in v0.6.1 | |--------|------------------| | Native algorithms only | None. | @@ -47,7 +49,11 @@ Full notes: [internal-docs/RELEASE-v0.6.1.md](internal-docs/RELEASE-v0.6.1.md). Optional algorithms beyond the native set use [`lestrrat-go/jwx/v4`](https://github.com/lestrrat-go/jwx) (≥ v4.5.0) via `NewJWSSigner` / `NewJWSSignerFromJWK` / `NewJWSVerifier`. -**ML-DSA (FIPS 204)** works through the same path with `crypto/mldsa` keys. Prefer inferring the alg from the public key: +For **RSA / ECDSA / Ed25519 / ML-DSA**, `NewJWSSigner` and `NewJWSVerifierWithAlg` accept either raw stdlib keys or an opaque [`crypto.Signer`](https://pkg.go.dev/crypto#Signer) (HSM/KMS) whose `Public()` matches the algorithm. Verify via `crypto.Signer` uses `Public()` locally; you can also pass the exported public key. Opaque Signers must use `NewJWSVerifierWithAlg` — the preferred infer path (`NewJWSVerifier`) does not recognize them. + +Default jwx ML-DSA signing still requires `*mldsa.PrivateKey`. For an opaque ML-DSA Signer (e.g. KMS), register a custom `jws.Signer` with [`jws.RegisterSigner`](https://pkg.go.dev/github.com/lestrrat-go/jwx/v4/jws#RegisterSigner) that calls `crypto.Signer.Sign` (prefer also handling raw `*mldsa.PrivateKey` if other code in the process needs it). ML-DSA remains foreign-JWS only — not a native RFC 9421 algorithm. + +**ML-DSA (FIPS 204)** with raw keys — prefer inferring the alg from the public key: ```go priv, _ := mldsa.GenerateKey(mldsa.MLDSA65()) diff --git a/crypto.go b/crypto.go index 870ee64..aa26ffa 100644 --- a/crypto.go +++ b/crypto.go @@ -130,6 +130,9 @@ func NewEd25519SignerFromSeed(seed []byte, config *SignConfig, fields Fields) (* // NewJWSSigner creates a generic signer for JWS algorithms via github.com/lestrrat-go/jwx/v4. // The particular key type for each algorithm is documented in that package (including // crypto/mldsa keys for ML-DSA on Go 1.27+). HMAC keys must be []byte (not string). +// Classical RSA/ECDSA/Ed25519 and ML-DSA also accept an opaque crypto.Signer (HSM/KMS) whose +// Public() matches the algorithm. Default jwx ML-DSA signing still needs *mldsa.PrivateKey; +// opaque ML-DSA Signers require a custom jws.RegisterSigner (see README). // Do not pass jwk.Key here — use NewJWSSignerFromJWK so alg can be taken from the JWK. // Config may be nil (defaults with SignAlg(false)). A non-nil config that would emit // HTTP Signature-Input "alg" is rejected — foreign JWS has no RFC 9421 algorithm id. @@ -368,7 +371,8 @@ func NewJWSVerifier(allowed *JWSAlgAllowlist, key any, config *VerifyConfig, fie // NewJWSVerifierWithAlg creates a foreign-JWS verifier for an explicit JWS algorithm. // Use when the algorithm cannot be inferred (raw RSA/HMAC) or the store already chose alg. -// Do not pass jwk.Key here — use NewJWSVerifier so alg can be taken from the JWK. +// Classical RSA/ECDSA/Ed25519 and ML-DSA accept raw public keys or an opaque crypto.Signer (local verify +// via Public()). Do not pass jwk.Key here — use NewJWSVerifier so alg can be taken from the JWK. // allowed may be nil to skip alg policy. func NewJWSVerifierWithAlg(allowed *JWSAlgAllowlist, alg jwa.SignatureAlgorithm, key any, config *VerifyConfig, fields Fields) (*Verifier, error) { if key == nil { diff --git a/internal-docs/JWS-ALG-POLICY.md b/internal-docs/JWS-ALG-POLICY.md index 16d3e86..04960b2 100644 --- a/internal-docs/JWS-ALG-POLICY.md +++ b/internal-docs/JWS-ALG-POLICY.md @@ -94,11 +94,14 @@ NewJWSSignerFromJWK(key jwk.Key, config *SignConfig, fields Fields) (*Signer, er 2. Resolve alg via jwx registry (`LookupSignatureAlgorithm`); reject empty / unregistered / `none`. Nil key rejected. 3. Existing **`validateJWSKeyAlg`**; default “unsupported” → fall through to **`jws.VerifierFor(alg)`** (jwx). 4. Reject raw `jwk.Key` here — use preferred `NewJWSVerifier` (or convert). +5. Classical RSA/ECDSA/Ed25519 and ML-DSA: raw keys **or** opaque `crypto.Signer` (local verify via `Public()`). Default jwx ML-DSA Sign still needs `*mldsa.PrivateKey` unless the caller `jws.RegisterSigner`s a custom signer. -**`NewJWSSigner`:** hardening + passthrough; **no** allowlist; force `SignAlg(false)`. Reject raw `jwk.Key` — use `NewJWSSignerFromJWK`. +**`NewJWSSigner`:** hardening + passthrough; **no** allowlist; force `SignAlg(false)`. Reject raw `jwk.Key` — use `NewJWSSignerFromJWK`. Classical RSA/ECDSA/Ed25519 and ML-DSA accept raw keys **or** opaque `crypto.Signer` (HSM/KMS). **`NewJWSSignerFromJWK`:** require a **private** JWK (symmetric `oct` OK); resolve alg with the same table / EdDSA↔Ed25519 rules as verify; export raw **private** material; then `NewJWSSigner`. +**`crypto.Signer` note:** same trade-off as jwx — external Signers make alg/key-type misuse harder to detect than in-process stdlib keys. `Public()` panics (e.g. malformed ed25519) are recovered at construction as validation errors. Opaque ML-DSA Signers additionally need `jws.RegisterSigner` for Sign under stock jwx. + Do **not** put the JWS allowlist on `VerifyConfig`. **Honesty about crypto quality:** hardening ≠ interop. No closed “supported algorithms” docs matrix. diff --git a/internal-docs/RELEASE-v0.6.2.md b/internal-docs/RELEASE-v0.6.2.md new file mode 100644 index 0000000..1235a18 --- /dev/null +++ b/internal-docs/RELEASE-v0.6.2.md @@ -0,0 +1,33 @@ +# Release notes: httpsign v0.6.2 (draft) + +Maintainer draft for the next patch after **v0.6.1**. Tag only when ready. + +--- + +## Summary + +**Additive compatibility restore** for foreign JWS (native signers unchanged). Builds on **v0.6.1**. + +### Highlights + +- **`crypto.Signer` (HSM/KMS)** accepted again for foreign JWS **RSA / ECDSA / Ed25519 / ML-DSA** on both `NewJWSSigner` and `NewJWSVerifierWithAlg` (and verify paths that share `validateJWSKeyAlg`). +- Validation uses `Public()` shape + existing curve / ML-DSA parameter-set checks; panics from `Public()` are recovered as construction errors. +- **ML-DSA note:** default jwx still signs only with `*mldsa.PrivateKey`. Opaque ML-DSA Signers need a custom [`jws.RegisterSigner`](https://pkg.go.dev/github.com/lestrrat-go/jwx/v4/jws#RegisterSigner). ML-DSA stays foreign-JWS only (not a native RFC 9421 algorithm). +- **JWK** still rejected in raw constructors (`NewJWSSignerFromJWK` / preferred `NewJWSVerifier`). + +### Upgrade from v0.6.1 + +| You use | Action | +|---------|--------| +| **Native only / raw stdlib JWS keys** | No change. | +| **Opaque `crypto.Signer` for RS\*/PS\*/ES\*/EdDSA** | Works again (was rejected at construction in v0.6.0–v0.6.1). | +| **Opaque `crypto.Signer` for ML-DSA** | Construction accepted; register a custom jwx ML-DSA signer for Sign, or keep using raw `*mldsa.PrivateKey`. | +| **JWK in `NewJWSSigner`** | Unchanged — use `NewJWSSignerFromJWK`. | + +### Backward compatibility + +This partially rolls back v0.6’s “raw stdlib only” foreign-JWS key policy. Callers already using raw keys are unaffected. Same caveat as jwx: external `crypto.Signer` makes alg/key misuse harder to catch than in-process stdlib types. + +Fixes [#34](https://github.com/yaronf/httpsign/issues/34). + +Thanks to [@ilya-korotya](https://github.com/ilya-korotya) for reporting the regression and for the draft fix in [#35](https://github.com/yaronf/httpsign/pull/35). diff --git a/jws_signer_wrapper_test.go b/jws_signer_wrapper_test.go new file mode 100644 index 0000000..ebc2849 --- /dev/null +++ b/jws_signer_wrapper_test.go @@ -0,0 +1,327 @@ +package httpsign + +import ( + "crypto" + "crypto/ecdsa" + "crypto/ed25519" + "crypto/elliptic" + "crypto/mldsa" + "crypto/rand" + "crypto/rsa" + "fmt" + "io" + "strings" + "testing" + + "github.com/lestrrat-go/jwx/v4/jwa" + "github.com/lestrrat-go/jwx/v4/jws" + "github.com/stretchr/testify/require" +) + +// Opaque wrappers: distinct concrete types that only satisfy crypto.Signer. +// Used to exercise HSM/KMS-style keys without matching *ecdsa.PrivateKey etc. + +type ecdsaCryptoSigner struct{ priv *ecdsa.PrivateKey } + +func (w ecdsaCryptoSigner) Public() crypto.PublicKey { return &w.priv.PublicKey } +func (w ecdsaCryptoSigner) Sign(r io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + return w.priv.Sign(r, digest, opts) +} + +type rsaCryptoSigner struct{ priv *rsa.PrivateKey } + +func (w rsaCryptoSigner) Public() crypto.PublicKey { return &w.priv.PublicKey } +func (w rsaCryptoSigner) Sign(r io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + return w.priv.Sign(r, digest, opts) +} + +type ed25519CryptoSigner struct{ priv ed25519.PrivateKey } + +func (w ed25519CryptoSigner) Public() crypto.PublicKey { return w.priv.Public().(ed25519.PublicKey) } +func (w ed25519CryptoSigner) Sign(r io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + return w.priv.Sign(r, digest, opts) +} + +type mldsaCryptoSigner struct{ priv *mldsa.PrivateKey } + +func (w mldsaCryptoSigner) Public() crypto.PublicKey { return w.priv.Public() } +func (w mldsaCryptoSigner) Sign(r io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + return w.priv.Sign(r, digest, opts) +} + +// wrongFamilySigner returns an RSA public key while claiming to be used as ES256. +type wrongFamilySigner struct{ pub *rsa.PublicKey } + +func (w wrongFamilySigner) Public() crypto.PublicKey { return w.pub } +func (w wrongFamilySigner) Sign(io.Reader, []byte, crypto.SignerOpts) ([]byte, error) { + return nil, nil +} + +func TestCryptoSignerConstructES256(t *testing.T) { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + wrap := ecdsaCryptoSigner{priv: priv} + + signer, err := NewJWSSigner(jwa.ES256(), wrap, nil, *NewFields()) + require.NoError(t, err) + require.NotNil(t, signer) + + verifier, err := NewJWSVerifierWithAlg(nil, jwa.ES256(), wrap, nil, *NewFields()) + require.NoError(t, err) + require.NotNil(t, verifier) +} + +func TestCryptoSignerConstructRS256(t *testing.T) { + priv, err := rsa.GenerateKey(rand.Reader, 1024) + require.NoError(t, err) + wrap := rsaCryptoSigner{priv: priv} + + signer, err := NewJWSSigner(jwa.RS256(), wrap, nil, *NewFields()) + require.NoError(t, err) + require.NotNil(t, signer) + + verifier, err := NewJWSVerifierWithAlg(nil, jwa.RS256(), wrap, nil, *NewFields()) + require.NoError(t, err) + require.NotNil(t, verifier) +} + +func TestCryptoSignerConstructEdDSA(t *testing.T) { + _, priv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + wrap := ed25519CryptoSigner{priv: priv} + + signer, err := NewJWSSigner(jwa.EdDSA(), wrap, nil, *NewFields()) + require.NoError(t, err) + require.NotNil(t, signer) + + verifier, err := NewJWSVerifierWithAlg(nil, jwa.EdDSA(), wrap, nil, *NewFields()) + require.NoError(t, err) + require.NotNil(t, verifier) +} + +func TestCryptoSignerRoundTripES256RawPublic(t *testing.T) { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + wrap := ecdsaCryptoSigner{priv: priv} + + config := NewSignConfig().setFakeCreated(1618884475).SignAlg(false).SetKeyID("kms1") + fields := *NewFields().AddHeader("@method").AddHeader("date").AddHeader("content-type").AddQueryParam("pet") + signer, err := NewJWSSigner(jwa.ES256(), wrap, config, fields) + require.NoError(t, err) + + req := readRequest(httpreq2) + sigInput, sig, err := SignRequest("sig1", *signer, req) + require.NoError(t, err) + req.Header.Add("Signature", sig) + req.Header.Add("Signature-Input", sigInput) + + verifier, err := NewJWSVerifierWithAlg(nil, jwa.ES256(), &priv.PublicKey, + NewVerifyConfig().SetVerifyCreated(false).SetKeyID("kms1"), fields) + require.NoError(t, err) + require.NoError(t, VerifyRequest("sig1", *verifier, req)) +} + +func TestCryptoSignerRoundTripES256SameWrapper(t *testing.T) { + priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + wrap := ecdsaCryptoSigner{priv: priv} + + config := NewSignConfig().setFakeCreated(1618884475).SignAlg(false).SetKeyID("kms1") + fields := *NewFields().AddHeader("@method").AddHeader("date").AddHeader("content-type").AddQueryParam("pet") + signer, err := NewJWSSigner(jwa.ES256(), wrap, config, fields) + require.NoError(t, err) + + req := readRequest(httpreq2) + sigInput, sig, err := SignRequest("sig1", *signer, req) + require.NoError(t, err) + req.Header.Add("Signature", sig) + req.Header.Add("Signature-Input", sigInput) + + verifier, err := NewJWSVerifierWithAlg(nil, jwa.ES256(), wrap, + NewVerifyConfig().SetVerifyCreated(false).SetKeyID("kms1"), fields) + require.NoError(t, err) + require.NoError(t, VerifyRequest("sig1", *verifier, req)) +} + +func TestCryptoSignerRoundTripRS256(t *testing.T) { + priv, err := rsa.GenerateKey(rand.Reader, 1024) + require.NoError(t, err) + wrap := rsaCryptoSigner{priv: priv} + + config := NewSignConfig().setFakeCreated(1618884475).SignAlg(false).SetKeyID("kms-rsa") + fields := *NewFields().AddHeader("@method").AddHeader("date").AddHeader("content-type").AddQueryParam("pet") + signer, err := NewJWSSigner(jwa.RS256(), wrap, config, fields) + require.NoError(t, err) + + req := readRequest(httpreq2) + sigInput, sig, err := SignRequest("sig1", *signer, req) + require.NoError(t, err) + req.Header.Add("Signature", sig) + req.Header.Add("Signature-Input", sigInput) + + verifier, err := NewJWSVerifierWithAlg(nil, jwa.RS256(), &priv.PublicKey, + NewVerifyConfig().SetVerifyCreated(false).SetKeyID("kms-rsa"), fields) + require.NoError(t, err) + require.NoError(t, VerifyRequest("sig1", *verifier, req)) + + verifierWrap, err := NewJWSVerifierWithAlg(nil, jwa.RS256(), wrap, + NewVerifyConfig().SetVerifyCreated(false).SetKeyID("kms-rsa"), fields) + require.NoError(t, err) + require.NoError(t, VerifyRequest("sig1", *verifierWrap, req)) +} + +func TestCryptoSignerRoundTripEdDSA(t *testing.T) { + pub, priv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + wrap := ed25519CryptoSigner{priv: priv} + + config := NewSignConfig().setFakeCreated(1618884475).SignAlg(false).SetKeyID("kms-ed") + fields := *NewFields().AddHeader("@method").AddHeader("date").AddHeader("content-type").AddQueryParam("pet") + signer, err := NewJWSSigner(jwa.EdDSA(), wrap, config, fields) + require.NoError(t, err) + + req := readRequest(httpreq2) + sigInput, sig, err := SignRequest("sig1", *signer, req) + require.NoError(t, err) + req.Header.Add("Signature", sig) + req.Header.Add("Signature-Input", sigInput) + + verifier, err := NewJWSVerifierWithAlg(nil, jwa.EdDSA(), pub, + NewVerifyConfig().SetVerifyCreated(false).SetKeyID("kms-ed"), fields) + require.NoError(t, err) + require.NoError(t, VerifyRequest("sig1", *verifier, req)) + + verifierWrap, err := NewJWSVerifierWithAlg(nil, jwa.EdDSA(), wrap, + NewVerifyConfig().SetVerifyCreated(false).SetKeyID("kms-ed"), fields) + require.NoError(t, err) + require.NoError(t, VerifyRequest("sig1", *verifierWrap, req)) +} + +func TestCryptoSignerRejectWrongPublicFamily(t *testing.T) { + rsaPriv, err := rsa.GenerateKey(rand.Reader, 1024) + require.NoError(t, err) + wrap := wrongFamilySigner{pub: &rsaPriv.PublicKey} + + _, err = NewJWSSigner(jwa.ES256(), wrap, nil, *NewFields()) + require.Error(t, err) + require.Contains(t, err.Error(), "Public") + + _, err = NewJWSVerifierWithAlg(nil, jwa.ES256(), wrap, nil, *NewFields()) + require.Error(t, err) + require.Contains(t, err.Error(), "Public") +} + +func TestCryptoSignerRejectCurveMismatch(t *testing.T) { + p384, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + require.NoError(t, err) + wrap := ecdsaCryptoSigner{priv: p384} + + _, err = NewJWSSigner(jwa.ES256(), wrap, nil, *NewFields()) + require.Error(t, err) + require.Contains(t, err.Error(), "curve") +} + +func TestCryptoSignerConstructMLDSA(t *testing.T) { + priv, err := mldsa.GenerateKey(mldsa.MLDSA44()) + require.NoError(t, err) + wrap := mldsaCryptoSigner{priv: priv} + + signer, err := NewJWSSigner(jwa.MLDSA44(), wrap, nil, *NewFields()) + require.NoError(t, err) + require.NotNil(t, signer) + + verifier, err := NewJWSVerifierWithAlg(nil, jwa.MLDSA44(), wrap, nil, *NewFields()) + require.NoError(t, err) + require.NotNil(t, verifier) +} + +func TestCryptoSignerRejectMLDSAParamMismatch(t *testing.T) { + priv, err := mldsa.GenerateKey(mldsa.MLDSA65()) + require.NoError(t, err) + wrap := mldsaCryptoSigner{priv: priv} + + _, err = NewJWSSigner(jwa.MLDSA44(), wrap, nil, *NewFields()) + require.Error(t, err) + require.Contains(t, err.Error(), "parameter set") +} + +func TestCryptoSignerRoundTripMLDSAWithRegisterSigner(t *testing.T) { + // Stock jwx ML-DSA requires *mldsa.PrivateKey; opaque Signer needs a custom jws.Signer. + prev, err := jws.SignerFor(jwa.MLDSA44()) + require.NoError(t, err) + require.NoError(t, jws.RegisterSigner(jwa.MLDSA44(), jws.SignerFunc(func(key any, payload []byte) ([]byte, error) { + signer, ok := key.(crypto.Signer) + if !ok { + return nil, fmt.Errorf("want crypto.Signer, got %T", key) + } + if _, ok := signer.Public().(*mldsa.PublicKey); !ok { + return nil, fmt.Errorf("Public() want *mldsa.PublicKey, got %T", signer.Public()) + } + return signer.Sign(nil, payload, &mldsa.Options{}) + }))) + t.Cleanup(func() { + _ = jws.RegisterSigner(jwa.MLDSA44(), prev) + }) + + priv, err := mldsa.GenerateKey(mldsa.MLDSA44()) + require.NoError(t, err) + wrap := mldsaCryptoSigner{priv: priv} + pub := priv.Public().(*mldsa.PublicKey) + + config := NewSignConfig().setFakeCreated(1618884475).SignAlg(false).SetKeyID("kms-mldsa") + fields := *NewFields().AddHeader("@method").AddHeader("date").AddHeader("content-type").AddQueryParam("pet") + signer, err := NewJWSSigner(jwa.MLDSA44(), wrap, config, fields) + require.NoError(t, err) + + req := readRequest(httpreq2) + sigInput, sig, err := SignRequest("sig1", *signer, req) + require.NoError(t, err) + req.Header.Add("Signature", sig) + req.Header.Add("Signature-Input", sigInput) + + verifier, err := NewJWSVerifierWithAlg(nil, jwa.MLDSA44(), pub, + NewVerifyConfig().SetVerifyCreated(false).SetKeyID("kms-mldsa"), fields) + require.NoError(t, err) + require.NoError(t, VerifyRequest("sig1", *verifier, req)) +} + +func TestCryptoSignerRecoverMalformedEd25519OnECDSAProbe(t *testing.T) { + // ed25519.PrivateKey implements crypto.Signer; wrong length makes Public() panic. + bad := ed25519.PrivateKey(strings.Repeat("a", 63)) + + err := validateECDSAKey(jwa.ES256(), bad, true) + require.Error(t, err) + + _, err = NewJWSSigner(jwa.ES256(), bad, nil, *NewFields()) + require.Error(t, err) +} + +func TestCryptoSignerRecoverMalformedEd25519OnRSAProbe(t *testing.T) { + bad := ed25519.PrivateKey(strings.Repeat("a", 63)) + + err := validateRSAKey(jwa.RS256(), bad, true) + require.Error(t, err) + + _, err = NewJWSSigner(jwa.RS256(), bad, nil, *NewFields()) + require.Error(t, err) +} + +func TestValidateJWSKeyAlgCryptoSigner(t *testing.T) { + p256, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + rsaPriv, err := rsa.GenerateKey(rand.Reader, 1024) + require.NoError(t, err) + _, edPriv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + mldsaPriv, err := mldsa.GenerateKey(mldsa.MLDSA44()) + require.NoError(t, err) + + require.NoError(t, validateECDSAKey(jwa.ES256(), ecdsaCryptoSigner{priv: p256}, true)) + require.NoError(t, validateECDSAKey(jwa.ES256(), ecdsaCryptoSigner{priv: p256}, false)) + require.NoError(t, validateRSAKey(jwa.RS256(), rsaCryptoSigner{priv: rsaPriv}, true)) + require.NoError(t, validateRSAKey(jwa.RS256(), rsaCryptoSigner{priv: rsaPriv}, false)) + require.NoError(t, validateEd25519Key(jwa.EdDSA(), ed25519CryptoSigner{priv: edPriv}, true)) + require.NoError(t, validateEd25519Key(jwa.EdDSA(), ed25519CryptoSigner{priv: edPriv}, false)) + require.NoError(t, validateMLDSAKey(jwa.MLDSA44(), mldsaCryptoSigner{priv: mldsaPriv}, true)) + require.NoError(t, validateMLDSAKey(jwa.MLDSA44(), mldsaCryptoSigner{priv: mldsaPriv}, false)) +} diff --git a/jwskey.go b/jwskey.go index c1ab7ec..50efae7 100644 --- a/jwskey.go +++ b/jwskey.go @@ -1,6 +1,7 @@ package httpsign import ( + "crypto" "crypto/ecdsa" "crypto/ed25519" "crypto/elliptic" @@ -14,8 +15,15 @@ import ( // validateJWSKeyAlg checks that key is an appropriate Go type for alg, without using // jws.AlgorithmsForKey (deprecated; not a compatibility API; over-broad for ECDSA/ML-DSA). // When signing is true, asymmetric keys must be private; when false, they must be public. -// HMAC keys are symmetric and accepted for either role. Only raw stdlib key types are -// accepted; crypto.Signer / JWK wrappers are rejected here so callers convert first. +// HMAC keys are symmetric and accepted for either role. +// +// Classical algs (RSA, ECDSA, Ed25519) and ML-DSA also accept an opaque crypto.Signer +// (HSM/KMS-backed keys, etc.) whose Public() reports the expected key shape — valid for +// both sign and verify (verify is local via Public()). Default jwx ML-DSA signing still +// requires *mldsa.PrivateKey; callers using an opaque Signer for ML-DSA must register a +// custom jws.Signer (jws.RegisterSigner) that accepts crypto.Signer. ML-DSA is foreign-JWS +// only — not a native RFC 9421 algorithm. +// JWK wrappers are rejected here so callers convert first (NewJWSSignerFromJWK / preferred verify). func validateJWSKeyAlg(alg jwa.SignatureAlgorithm, key any, signing bool) error { switch alg { case jwa.HS256(), jwa.HS384(), jwa.HS512(): @@ -58,6 +66,36 @@ func validateHMACKey(alg jwa.SignatureAlgorithm, key any) error { return nil } +// publicFromSigner returns Public() for a crypto.Signer. Recovers from panics +// (e.g. malformed ed25519.PrivateKey) so cross-family probes return a normal miss. +func publicFromSigner(key any) (pub crypto.PublicKey, ok bool) { + signer, isSigner := key.(crypto.Signer) + if !isSigner || signer == nil { + return nil, false + } + defer func() { + if recover() != nil { + pub = nil + ok = false + } + }() + pub = signer.Public() + if pub == nil { + return nil, false + } + return pub, true +} + +func errNeedKeyType(alg jwa.SignatureAlgorithm, kind, gotType string) error { + return fmt.Errorf("algorithm %s requires an %s key, got %s", alg, kind, gotType) +} + +func errNeedKeyTypeFromSigner(alg jwa.SignatureAlgorithm, kind, gotType, pubType string) error { + return fmt.Errorf("algorithm %s requires an %s key, got %s (Public %s)", alg, kind, gotType, pubType) +} + +func typeString(v any) string { return fmt.Sprintf("%T", v) } + func validateRSAKey(alg jwa.SignatureAlgorithm, key any, signing bool) error { switch k := key.(type) { case *rsa.PrivateKey: @@ -83,7 +121,21 @@ func validateRSAKey(alg jwa.SignatureAlgorithm, key any, signing bool) error { return fmt.Errorf("algorithm %s requires an RSA private key for signing", alg) } default: - return fmt.Errorf("algorithm %s requires an RSA key, got %T", alg, key) + pub, sok := publicFromSigner(key) + if !sok { + return errNeedKeyType(alg, "RSA", typeString(key)) + } + switch rp := pub.(type) { + case *rsa.PublicKey: + if rp == nil { + return fmt.Errorf("algorithm %s: nil RSA public key", alg) + } + case rsa.PublicKey: + // ok + default: + return errNeedKeyTypeFromSigner(alg, "RSA", typeString(key), typeString(pub)) + } + // Opaque crypto.Signer with RSA Public(): allowed for sign and verify. } return nil } @@ -91,15 +143,9 @@ func validateRSAKey(alg jwa.SignatureAlgorithm, key any, signing bool) error { // validateECDSAKey enforces RFC 7518 §3.4: ES256/ES384/ES512 bind to P-256/P-384/P-521. // jwx's SignerFor path does not enforce this. func validateECDSAKey(alg jwa.SignatureAlgorithm, key any, signing bool) error { - curve, isPrivate, ok := ecdsaKeyOf(key) - if !ok { - return fmt.Errorf("algorithm %s requires an ECDSA key, got %T", alg, key) - } - if signing && !isPrivate { - return fmt.Errorf("algorithm %s requires an ECDSA private key for signing", alg) - } - if !signing && isPrivate { - return fmt.Errorf("algorithm %s requires an ECDSA public key for verification", alg) + curve, err := ecdsaCurveFor(alg, key, signing) + if err != nil { + return err } if curve == nil { return fmt.Errorf("algorithm %s: ECDSA key has nil curve", alg) @@ -121,24 +167,53 @@ func validateECDSAKey(alg jwa.SignatureAlgorithm, key any, signing bool) error { return nil } -func ecdsaKeyOf(key any) (curve elliptic.Curve, isPrivate, ok bool) { +// ecdsaCurveFor returns the curve for a concrete ECDSA key or an opaque crypto.Signer +// whose Public() is ECDSA. Role checks (private vs public) apply only to concrete keys; +// Signers are accepted for both sign and verify. +func ecdsaCurveFor(alg jwa.SignatureAlgorithm, key any, signing bool) (elliptic.Curve, error) { switch k := key.(type) { case *ecdsa.PrivateKey: if k == nil { - return nil, false, false + return nil, errNeedKeyType(alg, "ECDSA", typeString(key)) + } + if !signing { + return nil, fmt.Errorf("algorithm %s requires an ECDSA public key for verification", alg) } - return k.Curve, true, true + return k.Curve, nil case ecdsa.PrivateKey: - return k.Curve, true, true + if !signing { + return nil, fmt.Errorf("algorithm %s requires an ECDSA public key for verification", alg) + } + return k.Curve, nil case *ecdsa.PublicKey: if k == nil { - return nil, false, false + return nil, errNeedKeyType(alg, "ECDSA", typeString(key)) + } + if signing { + return nil, fmt.Errorf("algorithm %s requires an ECDSA private key for signing", alg) } - return k.Curve, false, true + return k.Curve, nil case ecdsa.PublicKey: - return k.Curve, false, true + if signing { + return nil, fmt.Errorf("algorithm %s requires an ECDSA private key for signing", alg) + } + return k.Curve, nil default: - return nil, false, false + pub, ok := publicFromSigner(key) + if !ok { + return nil, errNeedKeyType(alg, "ECDSA", typeString(key)) + } + switch ep := pub.(type) { + case *ecdsa.PublicKey: + if ep == nil { + return nil, errNeedKeyTypeFromSigner(alg, "ECDSA", typeString(key), typeString(pub)) + } + return ep.Curve, nil + case ecdsa.PublicKey: + return ep.Curve, nil + default: + return nil, errNeedKeyTypeFromSigner(alg, "ECDSA", typeString(key), typeString(pub)) + } } } @@ -173,23 +248,33 @@ func validateEd25519Key(alg jwa.SignatureAlgorithm, key any, signing bool) error return fmt.Errorf("algorithm %s requires an Ed25519 private key for signing", alg) } default: - return fmt.Errorf("algorithm %s requires an Ed25519 key, got %T", alg, key) + pub, sok := publicFromSigner(key) + if !sok { + return errNeedKeyType(alg, "Ed25519", typeString(key)) + } + switch ep := pub.(type) { + case ed25519.PublicKey: + if len(ep) != ed25519.PublicKeySize { + return fmt.Errorf("algorithm %s: Ed25519 public key must be %d bytes, got %d", alg, ed25519.PublicKeySize, len(ep)) + } + case *ed25519.PublicKey: + if ep == nil || len(*ep) != ed25519.PublicKeySize { + return fmt.Errorf("algorithm %s: invalid Ed25519 public key", alg) + } + default: + return errNeedKeyTypeFromSigner(alg, "Ed25519", typeString(key), typeString(pub)) + } + // Opaque crypto.Signer with Ed25519 Public(): allowed for sign and verify. } return nil } // validateMLDSAKey enforces that an ML-DSA JWS algorithm matches the key's parameter set. -// jwx also rejects mismatches at Sign/Verify; this fails earlier at NewJWS* construction. +// jwx also rejects mismatches at Sign/Verify for raw keys; this fails earlier at NewJWS* construction. func validateMLDSAKey(alg jwa.SignatureAlgorithm, key any, signing bool) error { - got, isPrivate, ok := mldsaKeyOf(key) - if !ok { - return fmt.Errorf("algorithm %s requires a crypto/mldsa key, got %T", alg, key) - } - if signing && !isPrivate { - return fmt.Errorf("algorithm %s requires an ML-DSA private key for signing", alg) - } - if !signing && isPrivate { - return fmt.Errorf("algorithm %s requires an ML-DSA public key for verification", alg) + got, err := mldsaParamsFor(alg, key, signing) + if err != nil { + return err } var want mldsa.Parameters switch alg { @@ -208,19 +293,36 @@ func validateMLDSAKey(alg jwa.SignatureAlgorithm, key any, signing bool) error { return nil } -func mldsaKeyOf(key any) (params mldsa.Parameters, isPrivate, ok bool) { +// mldsaParamsFor returns the parameter set for a concrete crypto/mldsa key or an opaque +// crypto.Signer whose Public() is *mldsa.PublicKey. Role checks apply only to concrete keys; +// Signers are accepted for both sign and verify. +func mldsaParamsFor(alg jwa.SignatureAlgorithm, key any, signing bool) (mldsa.Parameters, error) { switch k := key.(type) { case *mldsa.PrivateKey: if k == nil { - return mldsa.Parameters{}, false, false + return mldsa.Parameters{}, errNeedKeyType(alg, "crypto/mldsa", typeString(key)) + } + if !signing { + return mldsa.Parameters{}, fmt.Errorf("algorithm %s requires an ML-DSA public key for verification", alg) } - return k.PublicKey().Parameters(), true, true + return k.PublicKey().Parameters(), nil case *mldsa.PublicKey: if k == nil { - return mldsa.Parameters{}, false, false + return mldsa.Parameters{}, errNeedKeyType(alg, "crypto/mldsa", typeString(key)) } - return k.Parameters(), false, true + if signing { + return mldsa.Parameters{}, fmt.Errorf("algorithm %s requires an ML-DSA private key for signing", alg) + } + return k.Parameters(), nil default: - return mldsa.Parameters{}, false, false + pub, ok := publicFromSigner(key) + if !ok { + return mldsa.Parameters{}, errNeedKeyType(alg, "crypto/mldsa", typeString(key)) + } + mp, ok := pub.(*mldsa.PublicKey) + if !ok || mp == nil { + return mldsa.Parameters{}, errNeedKeyTypeFromSigner(alg, "crypto/mldsa", typeString(key), typeString(pub)) + } + return mp.Parameters(), nil } } diff --git a/jwskey_test.go b/jwskey_test.go index 184da53..5635ac8 100644 --- a/jwskey_test.go +++ b/jwskey_test.go @@ -80,7 +80,7 @@ func TestValidateECDSAKey(t *testing.T) { require.NoError(t, validateECDSAKey(jwa.ES384(), p384, true)) require.NoError(t, validateECDSAKey(jwa.ES512(), p521, true)) - // value types via ecdsaKeyOf + // value types require.NoError(t, validateECDSAKey(jwa.ES256(), *p256, true)) require.NoError(t, validateECDSAKey(jwa.ES256(), p256.PublicKey, false)) @@ -93,12 +93,9 @@ func TestValidateECDSAKey(t *testing.T) { var nilPriv *ecdsa.PrivateKey var nilPub *ecdsa.PublicKey - _, _, ok := ecdsaKeyOf(nilPriv) - require.False(t, ok) - _, _, ok = ecdsaKeyOf(nilPub) - require.False(t, ok) - _, _, ok = ecdsaKeyOf("nope") - require.False(t, ok) + require.Error(t, validateECDSAKey(jwa.ES256(), nilPriv, true)) + require.Error(t, validateECDSAKey(jwa.ES256(), nilPub, false)) + require.Error(t, validateECDSAKey(jwa.ES256(), "nope", true)) } func TestValidateEd25519Key(t *testing.T) { @@ -145,10 +142,7 @@ func TestValidateMLDSAKey(t *testing.T) { var nilPriv *mldsa.PrivateKey var nilPub *mldsa.PublicKey - _, _, ok := mldsaKeyOf(nilPriv) - require.False(t, ok) - _, _, ok = mldsaKeyOf(nilPub) - require.False(t, ok) - _, _, ok = mldsaKeyOf("nope") - require.False(t, ok) + require.Error(t, validateMLDSAKey(jwa.MLDSA44(), nilPriv, true)) + require.Error(t, validateMLDSAKey(jwa.MLDSA44(), nilPub, false)) + require.Error(t, validateMLDSAKey(jwa.MLDSA44(), "nope", true)) }