Skip to content

Commit

Permalink
always add kid to header (#3499)
Browse files Browse the repository at this point in the history
  • Loading branch information
gerardsn authored Oct 18, 2024
1 parent de4f587 commit 4e53223
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 9 deletions.
13 changes: 7 additions & 6 deletions crypto/jwx.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,7 @@ func (client *Crypto) SignJWS(ctx context.Context, payload []byte, headers map[s
return "", err
}

if _, ok := headers["jwk"]; !ok {
headers["kid"] = kid
}
headers["kid"] = kid
return SignJWS(ctx, payload, headers, privateKey, detached)
}

Expand Down Expand Up @@ -247,10 +245,13 @@ func SignJWS(ctx context.Context, payload []byte, protectedHeaders map[string]in
return "", fmt.Errorf("unable to set header %s: %w", key, err)
}
}
// The JWX library is fine with creating a JWK for a private key (including the private exponents), so
// we want to make sure the `jwk` header (if present) does not (accidentally) contain a private key.
// That would lead to the node leaking its private key material in the resulting JWS which would be very, very bad.
if headers.JWK() != nil {
// 'kid' has been logged, use 'jwk' to sign
_ = headers.Remove(jwk.KeyIDKey)

// The JWX library is fine with creating a JWK for a private key (including the private exponents), so
// we want to make sure the `jwk` header (if present) does not (accidentally) contain a private key.
// That would lead to the node leaking its private key material in the resulting JWS which would be very, very bad.
var jwkAsPrivateKey crypto.Signer
if err := headers.JWK().Raw(&jwkAsPrivateKey); err == nil {
// `err != nil` is good in this case, because that means the key is not assignable to crypto.Signer,
Expand Down
18 changes: 18 additions & 0 deletions crypto/jwx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,24 @@ func TestCrypto_SignJWS(t *testing.T) {
require.NoError(t, err)
auditLogs.AssertContains(t, ModuleName, "SignJWS", audit.TestActor, "Signing a JWS with key: kid")
})
t.Run("writes audit log for jwk", func(t *testing.T) {
auditLogs := audit.CaptureAuditLogs(t)
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
require.NoError(t, err)
publicKeyAsJWK, _ := jwk.FromRaw(key.Public())
hdrs := map[string]interface{}{
"kid": kid,
"jwk": publicKeyAsJWK,
}

signature, err := SignJWS(audit.TestContext(), []byte{1, 2, 3}, hdrs, key, false)

require.NoError(t, err)
auditLogs.AssertContains(t, ModuleName, "SignJWS", audit.TestActor, "Signing a JWS with key: kid")
// kid is not in headers
msg, err := jws.Parse([]byte(signature))
assert.Empty(t, msg.Signatures()[0].ProtectedHeaders().KeyID())
})

t.Run("returns error for not found", func(t *testing.T) {
payload, _ := json.Marshal(map[string]interface{}{"iss": "nuts"})
Expand Down
4 changes: 1 addition & 3 deletions crypto/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,7 @@ func (m MemoryJWTSigner) SignJWS(ctx context.Context, payload []byte, headers ma
if kid != m.Key.KeyID() {
return "", ErrPrivateKeyNotFound
}
if _, ok := headers["jwk"]; !ok {
headers["kid"] = kid
}
headers["kid"] = kid
return SignJWS(ctx, payload, headers, signer, detached)
}

Expand Down

0 comments on commit 4e53223

Please sign in to comment.