diff --git a/Changes b/Changes index 1a52a4631..ee1cafb08 100644 --- a/Changes +++ b/Changes @@ -4,6 +4,22 @@ Changes v2 has many incompatibilities with v1. To see the full list of differences between v1 and v2, please read the Changes-v2.md file (https://github.com/lestrrat-go/jwx/blob/develop/v2/Changes-v2.md) +v2.0.6 - 25 Aug 2022 +[Bug fixes][Security] + * [jwe] Agreement Party UInfo and VInfo (apv/apu) were not properly being + passed to the functions to compute the aad when encrypting using ECDH-ES + family of algorithms. Therefore, when using apu/apv, messages encrypted + via this module would have failed to be properly decrypted. + + Please note that bogus encrypted messages would not have succeed being + decrypted (i.e. this problem does not allow spoofed messages to be decrypted). + Therefore this would not have caused unwanted data to to creep in -- + however it did pose problems for data to be sent and decrypted from this module + when using ECDH-ES with apu/apv. + + While not extensively tested, we believe this regression was introduced + with the v2 release. + v2.0.5 - 11 Aug 2022 [Bug fixes] * [jwt] Remove stray debug log diff --git a/bench/performance/go.sum b/bench/performance/go.sum index 704635e81..818cfefed 100644 --- a/bench/performance/go.sum +++ b/bench/performance/go.sum @@ -2,10 +2,10 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/decred/dcrd/crypto/blake256 v1.0.0/go.mod h1:sQl2p6Y26YV+ZOcSTP6thNdn47hh8kt6rqSlvmrXFAc= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1 h1:YLtO71vCjJRCBcrPMtQ9nqBsqpA1m5sE92cU+pd5Mcc= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1/go.mod h1:hyedUtir6IdtD/7lIxGeCxkaw7y45JueMRL4DIyJDKs= -github.com/goccy/go-json v0.9.10 h1:hCeNmprSNLB8B8vQKWl6DpuH0t60oEs+TAk9a7CScKc= -github.com/goccy/go-json v0.9.10/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 h1:HbphB4TFFXpv7MNrT52FGrrgVXF1owhMVTHFZIlnvd4= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0/go.mod h1:DZGJHZMqrU4JJqFAWUS2UO1+lbSKsdiOoYi9Zzey7Fc= +github.com/goccy/go-json v0.9.11 h1:/pAaQDLHEoCq/5FFmSKBswWmK6H0e8g4159Kc/X/nqk= +github.com/goccy/go-json v0.9.11/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/lestrrat-go/blackmagic v1.0.1 h1:lS5Zts+5HIC/8og6cGHb0uCcNCa3OUt1ygh3Qz2Fe80= github.com/lestrrat-go/blackmagic v1.0.1/go.mod h1:UrEqBzIR2U6CnzVyUtfM6oZNMt/7O7Vohk2J0OGSAtU= github.com/lestrrat-go/httpcc v1.0.1 h1:ydWCStUeJLkpYyjLDHihupbn2tYmZ7m22BGkcvZZrIE= diff --git a/cmd/jwx/go.mod b/cmd/jwx/go.mod index d807c5937..caaab3780 100644 --- a/cmd/jwx/go.mod +++ b/cmd/jwx/go.mod @@ -10,8 +10,8 @@ require ( require ( github.com/cpuguy83/go-md2man/v2 v2.0.1 // indirect - github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1 // indirect - github.com/goccy/go-json v0.9.10 // indirect + github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 // indirect + github.com/goccy/go-json v0.9.11 // indirect github.com/lestrrat-go/blackmagic v1.0.1 // indirect github.com/lestrrat-go/httpcc v1.0.1 // indirect github.com/lestrrat-go/httprc v1.0.4 // indirect diff --git a/cmd/jwx/go.sum b/cmd/jwx/go.sum index 89818a5c1..18397315e 100644 --- a/cmd/jwx/go.sum +++ b/cmd/jwx/go.sum @@ -6,10 +6,10 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/decred/dcrd/crypto/blake256 v1.0.0/go.mod h1:sQl2p6Y26YV+ZOcSTP6thNdn47hh8kt6rqSlvmrXFAc= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1 h1:YLtO71vCjJRCBcrPMtQ9nqBsqpA1m5sE92cU+pd5Mcc= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1/go.mod h1:hyedUtir6IdtD/7lIxGeCxkaw7y45JueMRL4DIyJDKs= -github.com/goccy/go-json v0.9.10 h1:hCeNmprSNLB8B8vQKWl6DpuH0t60oEs+TAk9a7CScKc= -github.com/goccy/go-json v0.9.10/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 h1:HbphB4TFFXpv7MNrT52FGrrgVXF1owhMVTHFZIlnvd4= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0/go.mod h1:DZGJHZMqrU4JJqFAWUS2UO1+lbSKsdiOoYi9Zzey7Fc= +github.com/goccy/go-json v0.9.11 h1:/pAaQDLHEoCq/5FFmSKBswWmK6H0e8g4159Kc/X/nqk= +github.com/goccy/go-json v0.9.11/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/lestrrat-go/blackmagic v1.0.1 h1:lS5Zts+5HIC/8og6cGHb0uCcNCa3OUt1ygh3Qz2Fe80= github.com/lestrrat-go/blackmagic v1.0.1/go.mod h1:UrEqBzIR2U6CnzVyUtfM6oZNMt/7O7Vohk2J0OGSAtU= github.com/lestrrat-go/httpcc v1.0.1 h1:ydWCStUeJLkpYyjLDHihupbn2tYmZ7m22BGkcvZZrIE= diff --git a/docs/03-jwe.md b/docs/03-jwe.md index 0fc9bfa7f..83026396b 100644 --- a/docs/03-jwe.md +++ b/docs/03-jwe.md @@ -270,9 +270,9 @@ source: [examples/jwe_encrypt_json_example_test.go](https://github.com/lestrrat- By default, only some header fields are included in the result from `jwe.Encrypt()`. -For protected headers, you can use the jws. +For global protected headers, you can use the `jwe.WithProtectedHeaders()` option. -In order to provide extra headers to the encrypted message, you will need to use +In order to provide extra headers to the encrypted message such as `apu` and `apv`, you will need to use `jwe.WithKey()` option with the `jwe.WithPerRecipientHeaders()` suboption. diff --git a/examples/go.sum b/examples/go.sum index a5f866ad3..e2f410751 100644 --- a/examples/go.sum +++ b/examples/go.sum @@ -5,10 +5,10 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/decred/dcrd/crypto/blake256 v1.0.0/go.mod h1:sQl2p6Y26YV+ZOcSTP6thNdn47hh8kt6rqSlvmrXFAc= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1 h1:YLtO71vCjJRCBcrPMtQ9nqBsqpA1m5sE92cU+pd5Mcc= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1/go.mod h1:hyedUtir6IdtD/7lIxGeCxkaw7y45JueMRL4DIyJDKs= -github.com/goccy/go-json v0.9.10 h1:hCeNmprSNLB8B8vQKWl6DpuH0t60oEs+TAk9a7CScKc= -github.com/goccy/go-json v0.9.10/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 h1:HbphB4TFFXpv7MNrT52FGrrgVXF1owhMVTHFZIlnvd4= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0/go.mod h1:DZGJHZMqrU4JJqFAWUS2UO1+lbSKsdiOoYi9Zzey7Fc= +github.com/goccy/go-json v0.9.11 h1:/pAaQDLHEoCq/5FFmSKBswWmK6H0e8g4159Kc/X/nqk= +github.com/goccy/go-json v0.9.11/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/lestrrat-go/blackmagic v1.0.1 h1:lS5Zts+5HIC/8og6cGHb0uCcNCa3OUt1ygh3Qz2Fe80= github.com/lestrrat-go/blackmagic v1.0.1/go.mod h1:UrEqBzIR2U6CnzVyUtfM6oZNMt/7O7Vohk2J0OGSAtU= github.com/lestrrat-go/httpcc v1.0.1 h1:ydWCStUeJLkpYyjLDHihupbn2tYmZ7m22BGkcvZZrIE= diff --git a/go.mod b/go.mod index f59a61e89..ca5edc19d 100644 --- a/go.mod +++ b/go.mod @@ -3,8 +3,8 @@ module github.com/lestrrat-go/jwx/v2 go 1.16 require ( - github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1 - github.com/goccy/go-json v0.9.10 + github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 + github.com/goccy/go-json v0.9.11 github.com/lestrrat-go/blackmagic v1.0.1 github.com/lestrrat-go/httprc v1.0.4 github.com/lestrrat-go/iter v1.0.2 diff --git a/go.sum b/go.sum index 704635e81..818cfefed 100644 --- a/go.sum +++ b/go.sum @@ -2,10 +2,10 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/decred/dcrd/crypto/blake256 v1.0.0/go.mod h1:sQl2p6Y26YV+ZOcSTP6thNdn47hh8kt6rqSlvmrXFAc= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1 h1:YLtO71vCjJRCBcrPMtQ9nqBsqpA1m5sE92cU+pd5Mcc= -github.com/decred/dcrd/dcrec/secp256k1/v4 v4.0.1/go.mod h1:hyedUtir6IdtD/7lIxGeCxkaw7y45JueMRL4DIyJDKs= -github.com/goccy/go-json v0.9.10 h1:hCeNmprSNLB8B8vQKWl6DpuH0t60oEs+TAk9a7CScKc= -github.com/goccy/go-json v0.9.10/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0 h1:HbphB4TFFXpv7MNrT52FGrrgVXF1owhMVTHFZIlnvd4= +github.com/decred/dcrd/dcrec/secp256k1/v4 v4.1.0/go.mod h1:DZGJHZMqrU4JJqFAWUS2UO1+lbSKsdiOoYi9Zzey7Fc= +github.com/goccy/go-json v0.9.11 h1:/pAaQDLHEoCq/5FFmSKBswWmK6H0e8g4159Kc/X/nqk= +github.com/goccy/go-json v0.9.11/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/lestrrat-go/blackmagic v1.0.1 h1:lS5Zts+5HIC/8og6cGHb0uCcNCa3OUt1ygh3Qz2Fe80= github.com/lestrrat-go/blackmagic v1.0.1/go.mod h1:UrEqBzIR2U6CnzVyUtfM6oZNMt/7O7Vohk2J0OGSAtU= github.com/lestrrat-go/httpcc v1.0.1 h1:ydWCStUeJLkpYyjLDHihupbn2tYmZ7m22BGkcvZZrIE= diff --git a/jwe/internal/keyenc/keyenc.go b/jwe/internal/keyenc/keyenc.go index 00b25e01d..3e19e62b0 100644 --- a/jwe/internal/keyenc/keyenc.go +++ b/jwe/internal/keyenc/keyenc.go @@ -210,12 +210,12 @@ func (kw PBES2Encrypt) Encrypt(cek []byte) (keygen.ByteSource, error) { } // NewECDHESEncrypt creates a new key encrypter based on ECDH-ES -func NewECDHESEncrypt(alg jwa.KeyEncryptionAlgorithm, enc jwa.ContentEncryptionAlgorithm, keysize int, keyif interface{}) (*ECDHESEncrypt, error) { +func NewECDHESEncrypt(alg jwa.KeyEncryptionAlgorithm, enc jwa.ContentEncryptionAlgorithm, keysize int, keyif interface{}, apu, apv []byte) (*ECDHESEncrypt, error) { var generator keygen.Generator var err error switch key := keyif.(type) { case *ecdsa.PublicKey: - generator, err = keygen.NewEcdhes(alg, enc, keysize, key) + generator, err = keygen.NewEcdhes(alg, enc, keysize, key, apu, apv) case x25519.PublicKey: generator, err = keygen.NewX25519(alg, enc, keysize, key) default: diff --git a/jwe/internal/keygen/interface.go b/jwe/internal/keygen/interface.go index 331a00eba..10543c056 100644 --- a/jwe/internal/keygen/interface.go +++ b/jwe/internal/keygen/interface.go @@ -26,6 +26,8 @@ type Ecdhes struct { keysize int algorithm jwa.KeyEncryptionAlgorithm enc jwa.ContentEncryptionAlgorithm + apu []byte + apv []byte } // X25519KeyGenerate generates keys using ECDH-ES algorithm / X25519 curve diff --git a/jwe/internal/keygen/keygen.go b/jwe/internal/keygen/keygen.go index 5b2d8a6ad..bab6041a9 100644 --- a/jwe/internal/keygen/keygen.go +++ b/jwe/internal/keygen/keygen.go @@ -55,12 +55,14 @@ func (g Random) Generate() (ByteSource, error) { } // NewEcdhes creates a new key generator using ECDH-ES -func NewEcdhes(alg jwa.KeyEncryptionAlgorithm, enc jwa.ContentEncryptionAlgorithm, keysize int, pubkey *ecdsa.PublicKey) (*Ecdhes, error) { +func NewEcdhes(alg jwa.KeyEncryptionAlgorithm, enc jwa.ContentEncryptionAlgorithm, keysize int, pubkey *ecdsa.PublicKey, apu, apv []byte) (*Ecdhes, error) { return &Ecdhes{ algorithm: alg, enc: enc, keysize: keysize, pubkey: pubkey, + apu: apu, + apv: apv, }, nil } @@ -89,7 +91,7 @@ func (g Ecdhes) Generate() (ByteSource, error) { z, _ := priv.PublicKey.Curve.ScalarMult(g.pubkey.X, g.pubkey.Y, priv.D.Bytes()) zBytes := ecutil.AllocECPointBuffer(z, priv.PublicKey.Curve) defer ecutil.ReleaseECPointBuffer(zBytes) - kdf := concatkdf.New(crypto.SHA256, []byte(algorithm), zBytes, []byte{}, []byte{}, pubinfo, []byte{}) + kdf := concatkdf.New(crypto.SHA256, []byte(algorithm), zBytes, g.apu, g.apv, pubinfo, []byte{}) kek := make([]byte, g.keysize) if _, err := kdf.Read(kek); err != nil { return nil, fmt.Errorf(`failed to read kdf: %w`, err) diff --git a/jwe/jwe.go b/jwe/jwe.go index 95a020d97..be11b3dfd 100644 --- a/jwe/jwe.go +++ b/jwe/jwe.go @@ -127,7 +127,13 @@ func (b *recipientBuilder) Build(cek []byte, calg jwa.ContentEncryptionAlgorithm switch key := rawKey.(type) { case x25519.PublicKey: - v, err := keyenc.NewECDHESEncrypt(b.alg, calg, keysize, rawKey) + var apu, apv []byte + if hdrs := b.headers; hdrs != nil { + apu = hdrs.AgreementPartyUInfo() + apv = hdrs.AgreementPartyVInfo() + } + + v, err := keyenc.NewECDHESEncrypt(b.alg, calg, keysize, rawKey, apu, apv) if err != nil { return nil, nil, fmt.Errorf(`failed to create ECDHS key wrap encrypter: %w`, err) } @@ -137,7 +143,14 @@ func (b *recipientBuilder) Build(cek []byte, calg jwa.ContentEncryptionAlgorithm if err := keyconv.ECDSAPublicKey(&pubkey, rawKey); err != nil { return nil, nil, fmt.Errorf(`failed to generate public key from key (%T): %w`, key, err) } - v, err := keyenc.NewECDHESEncrypt(b.alg, calg, keysize, &pubkey) + + var apu, apv []byte + if hdrs := b.headers; hdrs != nil { + apu = hdrs.AgreementPartyUInfo() + apv = hdrs.AgreementPartyVInfo() + } + + v, err := keyenc.NewECDHESEncrypt(b.alg, calg, keysize, &pubkey, apu, apv) if err != nil { return nil, nil, fmt.Errorf(`failed to create ECDHS key wrap encrypter: %w`, err) } @@ -592,7 +605,6 @@ func (dctx *decryptCtx) decryptKey(ctx context.Context, alg jwa.KeyEncryptionAlg if apu := h2.AgreementPartyUInfo(); len(apu) > 0 { dec.AgreementPartyUInfo(apu) } - if apv := h2.AgreementPartyVInfo(); len(apv) > 0 { dec.AgreementPartyVInfo(apv) } diff --git a/jwe/jwe_test.go b/jwe/jwe_test.go index d51a329d2..a729b0414 100644 --- a/jwe/jwe_test.go +++ b/jwe/jwe_test.go @@ -22,6 +22,7 @@ import ( "github.com/lestrrat-go/jwx/v2/jwk" "github.com/lestrrat-go/jwx/v2/x25519" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -780,11 +781,6 @@ func TestGH554(t *testing.T) { return } - { - buf, _ := json.MarshalIndent(msg, "", " ") - t.Logf("%s", buf) - } - recipients := msg.Recipients() // The epk must have the same key ID as the original @@ -793,3 +789,33 @@ func TestGH554(t *testing.T) { return } } + +func TestGH803(t *testing.T) { + privateKey, err := ecdsa.GenerateKey(elliptic.P384(), rand.Reader) + require.NoError(t, err, `ecdsa.GenerateKey should succeed`) + + payload := []byte("Lorem Ipsum") + apu := []byte(`Alice`) + apv := []byte(`Bob`) + hdrs := jwe.NewHeaders() + hdrs.Set(jwe.AgreementPartyUInfoKey, apu) + hdrs.Set(jwe.AgreementPartyVInfoKey, apv) + encrypted, err := jwe.Encrypt( + payload, + jwe.WithJSON(), + jwe.WithKey(jwa.ECDH_ES, privateKey.PublicKey, jwe.WithPerRecipientHeaders(hdrs)), + jwe.WithContentEncryption(jwa.A128GCM), + ) + require.NoError(t, err, `jwe.Encrypt should succeed`) + + var msg jwe.Message + decrypted, err := jwe.Decrypt( + encrypted, + jwe.WithKey(jwa.ECDH_ES, privateKey), + jwe.WithMessage(&msg), + ) + require.NoError(t, err, `jwe.Decrypt should succeed`) + require.Equal(t, payload, decrypted, `decrypt messages match`) + require.Equal(t, apu, msg.ProtectedHeaders().AgreementPartyUInfo()) + require.Equal(t, apv, msg.ProtectedHeaders().AgreementPartyVInfo()) +}