Skip to content

Commit

Permalink
jwe: re-enable ECDH-ES, fix cek determination logic
Browse files Browse the repository at this point in the history
In Direct Agreement Mode, there is no encrypted cek. It's determined
"emprically" by both sides based on the private / ephemeral keys. So we
have to throw out the generated cek and replace with the "encrypted"
cek.

I don't think there's a way to make this work in multi-recipient mode.
Other libraries have a similar restrictions, I believe.

Fixes lestrrat-go#234
  • Loading branch information
imirkin committed Dec 16, 2020
1 parent dfa17eb commit fedc2c7
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 20 deletions.
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ Supported key encryption algorithm:
| AES key wrap (192) | YES | jwa.A192KW |
| AES key wrap (256) | YES | jwa.A256KW |
| Direct encryption | NO | jwa.DIRECT |
| ECDH-ES | NO (see #234) | jwa.ECDH_ES |
| ECDH-ES | YES (1) | jwa.ECDH_ES |
| ECDH-ES + AES key wrap (128) | YES | jwa.ECDH_ES_A128KW |
| ECDH-ES + AES key wrap (192) | YES | jwa.ECDH_ES_A192KW |
| ECDH-ES + AES key wrap (256) | YES | jwa.ECDH_ES_A256KW |
Expand All @@ -385,6 +385,8 @@ Supported key encryption algorithm:
| PBES2 + HMAC-SHA384 + AES key wrap (192) | YES | jwa.PBES2_HS384_A192KW |
| PBES2 + HMAC-SHA512 + AES key wrap (256) | YES | jwa.PBES2_HS512_A256KW |

Note 1: Single-recipient only

Supported content encryption algorithm:

| Algorithm | Supported? | Constant in go-jwx |
Expand Down
11 changes: 9 additions & 2 deletions jwe/encrypt.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,15 @@ func (e encryptCtx) Encrypt(plaintext []byte) (*Message, error) {
}
return nil, errors.Wrap(err, `failed to encrypt key`)
}
if err := r.SetEncryptedKey(enckey.Bytes()); err != nil {
return nil, errors.Wrap(err, "failed to set encrypted key")
if enc.Algorithm() == jwa.ECDH_ES {
if len(e.keyEncrypters) > 1 {
return nil, errors.Errorf("unable to support multiple recipients for ECDH-ES")
}
cek = enckey.Bytes()
} else {
if err := r.SetEncryptedKey(enckey.Bytes()); err != nil {
return nil, errors.Wrap(err, "failed to set encrypted key")
}
}
if hp, ok := enckey.(populater); ok {
if err := hp.Populate(r.Headers()); err != nil {
Expand Down
8 changes: 1 addition & 7 deletions jwe/jwe_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -375,9 +375,7 @@ func TestEncode_ECDH(t *testing.T) {
}

algorithms := []jwa.KeyEncryptionAlgorithm{
// XXX for ECDH-ES
// uncomment the next line
// jwa.ECDH_ES,
jwa.ECDH_ES,
jwa.ECDH_ES_A256KW,
jwa.ECDH_ES_A192KW,
jwa.ECDH_ES_A128KW,
Expand Down Expand Up @@ -417,10 +415,6 @@ func TestEncode_ECDH(t *testing.T) {
}

func Test_GHIssue207(t *testing.T) {
// XXX for ECDH-ES
// Remove the t.SkipNow()
t.SkipNow()

const plaintext = "hi\n"
var testcases = []struct {
Algorithm jwa.KeyEncryptionAlgorithm
Expand Down
10 changes: 1 addition & 9 deletions jwe/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -476,15 +476,7 @@ func (m *Message) Decrypt(alg jwa.KeyEncryptionAlgorithm, key interface{}) ([]by
}

switch alg {
case jwa.ECDH_ES:
// XXX for ECDH-ES
// disable this entire case, and replace the next case statement with the following:
//
// ```START
// case jwa.ECDH_ES, jwa.ECDH_ES_A128KW, jwa.ECDH_ES_A192KW, jwa.ECDH_ES_A256KW:
// ```END
return nil, errors.New(`ECDH-ES is not yet supported`)
case jwa.ECDH_ES_A128KW, jwa.ECDH_ES_A192KW, jwa.ECDH_ES_A256KW:
case jwa.ECDH_ES, jwa.ECDH_ES_A128KW, jwa.ECDH_ES_A192KW, jwa.ECDH_ES_A256KW:
epkif, ok := h2.Get(EphemeralPublicKeyKey)
if !ok {
return nil, errors.New("failed to get 'epk' field")
Expand Down
5 changes: 4 additions & 1 deletion jwx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,10 @@ func TestJoseCompatibility(t *testing.T) {
{jwa.RSA_OAEP_256, jwa.A128GCM},
{jwa.RSA_OAEP_256, jwa.A128CBC_HS256},
{jwa.RSA_OAEP_256, jwa.A256CBC_HS512},
// {jwa.ECDH_ES, jwa.A256CBC_HS512},
{jwa.ECDH_ES, jwa.A128GCM},
{jwa.ECDH_ES, jwa.A256GCM},
{jwa.ECDH_ES, jwa.A128CBC_HS256},
{jwa.ECDH_ES, jwa.A256CBC_HS512},
{jwa.ECDH_ES_A128KW, jwa.A128GCM},
{jwa.ECDH_ES_A128KW, jwa.A128CBC_HS256},
{jwa.ECDH_ES_A256KW, jwa.A256GCM},
Expand Down

0 comments on commit fedc2c7

Please sign in to comment.