-
Notifications
You must be signed in to change notification settings - Fork 278
Custom headers and additional header support #56
Comments
Yeah, we currently don't have good support for that. I'm not sure what would be a good interface for doing this, e.g. how do you control whether you want to set headers for the entire message or only for individual recipients? I think it would make sense to change the signature of e.g. |
I think adding a new |
I was just looking. Can we maybe as a first pass consider using the same mechanism as |
Lemme also poke at the recipient interface as well. Obviously the whole |
FWIW currently in our case we don't need this to per-recipient |
I'm not sure I would consider merging something into master if we don't intend to keep it there long-term, but I'd be happy to merge it into a different branch (e.g. one that you could pull in via gopkg.in). |
Not supporting |
okay here's a short git diff of my current fix that appears to accomplish the task: diff --git a/crypter.go b/crypter.go
index 9aa1e15..6eba7c3 100644
--- a/crypter.go
+++ b/crypter.go
@@ -28,6 +28,7 @@ type Encrypter interface {
Encrypt(plaintext []byte) (*JsonWebEncryption, error)
EncryptWithAuthData(plaintext []byte, aad []byte) (*JsonWebEncryption, error)
SetCompression(alg CompressionAlgorithm)
+ SetKid(kid string)
}
// MultiEncrypter represents an encrypter which supports multiple recipients.
@@ -68,6 +69,7 @@ type genericEncrypter struct {
cipher contentCipher
recipients []recipientKeyInfo
keyGenerator keyGenerator
+ kid string
}
type recipientKeyInfo struct {
@@ -80,6 +82,11 @@ func (ctx *genericEncrypter) SetCompression(compressionAlg CompressionAlgorithm)
ctx.compressionAlg = compressionAlg
}
+// SetUnprotectedHeader sets a JWE unprotected header
+func (ctx *genericEncrypter) SetKid(kid string) {
+ ctx.kid = kid
+}
+
// NewEncrypter creates an appropriate encrypter based on the key type
func NewEncrypter(alg KeyAlgorithm, enc ContentEncryption, encryptionKey interface{}) (Encrypter, error) {
encrypter := &genericEncrypter{
@@ -247,6 +254,10 @@ func (ctx *genericEncrypter) EncryptWithAuthData(plaintext, aad []byte) (*JsonWe
obj.protected.Zip = ctx.compressionAlg
}
+ if ctx.kid != "" {
+ obj.protected.Kid = ctx.kid
+ }
+
authData := obj.computeAuthData()
parts, err := ctx.cipher.encrypt(cek, authData, plaintext)
if err != nil { I think this would work but as you said it may not be a long term approach. I also still need to write a test case for it. I'm outside of my comfort zone in crypto related stuff and I really wanna make sure that it doesn't break anything/cause a security issue. |
Side note related to testing, evidently it's totally valid to have an empty placeholder when using
This may need to be reflected when I update the tests. |
No need to worry about breaking anything, we have a dedicated team of security engineers that can review code and help out :). I can try to come up with a sketch for adding a recipient interface over the weekend. |
I believe #58 is a partial solution to this (in particular, it addresses the key ID problem). |
Another important case would be |
@yawn: I have a |
FYI the ACME specification, which relies on JOSE, now specifies an Having to change go-jose every time some specification wants to put something in the protected header seems untenable. How about providing a means of injecting arbitrary keys into the protected header via a |
That's a fair point, I think that's something we could add if you have a pull request for it. |
The JWE spec supports more than just the two fields allowed currently in go-jose (
alg
andenc
).You can also specify a kid which is really important when using symmetric encryption like AES with JWE.
I'm attempting something like so:
but when I attempt to base64 decode the header (first part), I still only see
alg
andenc
.I'm proposing either changing the Encrypter interface to support new funcs for adding
kid
specifically or a more generic one to allow setting headers that are unrelated to the encryption and key methods being used.I can maybe make a PR on this but I need to know which way you'd rather go first.
Thanks!
The text was updated successfully, but these errors were encountered: