go-jose icon indicating copy to clipboard operation
go-jose copied to clipboard

No way to specify ephemeral ECDH-ES key

Open NeilMadden opened this issue 8 years ago • 4 comments

As mentioned in issue #162 it is good practice to include the hashes of all public keys in the key derivation process for ECDH. This can be done (once that issue is resolved) by putting the hashes in the apu/apv claims. However, to do this the caller needs to know the ephemeral public key before encryption takes place. Currently the API does not allow this as a fresh ephemeral key pair is always generated just before key derivation occurs in genericEncrypter.EncryptWithAuthData (line 300 of crypter.go).

If the caller does attempt to provide their own ephemeral key, by setting the "epk" header value, this is currently ignored. However, the header they provide will still be used as the header merging on line 305 will ignore the fresh epk value if one is already present, resulting in incorrect information being sent to the recipient.

One solution would be to check if there is an existing "epk" header and if so to use that as the ephemeral key (after checking it is on the correct curve). Note that in this case the JWK provided would have to include the private key, and so the code would have to be careful to remove the private claims from the header before sending.

A different (probably better) alternative would be to provide an optional callback argument (as part of EncrypterOptions) that will be called with the ephemeral key and current headers just before key derivation and given the chance to update the apu/apv header values.

NeilMadden avatar Sep 20 '17 09:09 NeilMadden

NB - I might be able to contribute PRs for these issues, but I'd like feedback on the preferred approach first. I also need to run the contributor license agreement passed my employer (ForgeRock) first.

NeilMadden avatar Sep 20 '17 09:09 NeilMadden

Your approach to add a callback sounds reasonable to me. Happy to accept a pull request to add that.

csstaub avatar Sep 20 '17 18:09 csstaub

FYI: Square also has a corporate CLA if your employer prefers that over the individual one, just email [email protected] to get that set up.

csstaub avatar Sep 20 '17 18:09 csstaub

Thanks, I'm still getting sign off, but it looks like it shouldn't be a problem. In the meantime, we have a working (hacked together ;-) ) fork that uses callbacks for this. We have a deadline approaching, but I will try and find time to polish up a PR in the next week or so.

NeilMadden avatar Sep 29 '17 09:09 NeilMadden