Skip to content

Commit

Permalink
Merge develop/v2 for release (#805)
Browse files Browse the repository at this point in the history
* Upgrade deps (#799)

* [jwe] Fix jwe encryption around apu/apv (#804)

* [jwe] Fix jwe encryption around apu/apv

* Add tests
* Add notes in the docs that apu/apv should be passed using
  `jwe.WithPerRecipientHeaders()` option along with the key
* Fix an omission where header values for apu/apv were not
  properly passed when encrypting

* Add entry in Changes

* remove stray fields

* Update Changes
  • Loading branch information
lestrrat authored Aug 25, 2022
1 parent c599c15 commit 9295064
Show file tree
Hide file tree
Showing 13 changed files with 92 additions and 34 deletions.
16 changes: 16 additions & 0 deletions Changes
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions bench/performance/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
4 changes: 2 additions & 2 deletions cmd/jwx/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions cmd/jwx/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
4 changes: 2 additions & 2 deletions docs/03-jwe.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.


Expand Down
8 changes: 4 additions & 4 deletions examples/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
4 changes: 2 additions & 2 deletions jwe/internal/keyenc/keyenc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions jwe/internal/keygen/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions jwe/internal/keygen/keygen.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down
18 changes: 15 additions & 3 deletions jwe/jwe.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
36 changes: 31 additions & 5 deletions jwe/jwe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand All @@ -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())
}

0 comments on commit 9295064

Please sign in to comment.