diff --git a/docs/identity-provider.md b/docs/identity-provider.md index e0b52c4..9eb04fc 100644 --- a/docs/identity-provider.md +++ b/docs/identity-provider.md @@ -49,7 +49,7 @@ The client assertion is signed with ES256 by the client's own ECDSA P-256 key an | `aud` | `BoteCredentials.Authority`, the issuer's identifier (not the token endpoint URL) | | `jti` | a new GUID for every assertion | | `nbf`, `exp` | now, and five minutes later | -| header `kid` | the key id of the client's signing key | +| header `kid` | the key id of the client's signing key: `Authentication:KeyId` when configured, otherwise the key's JWK thumbprint (RFC 7638, SHA-256, base64url) | | header `typ` | `client-authentication+jwt` (explicit typing, RFC 8725; the identity provider requires it) | The identity provider must: @@ -65,7 +65,8 @@ The identity provider must: tenant (`/{tenant}/token`) is fine as long as the client must be registered under that tenant. Rotating a client's key: register the new public key next to the old one and tell them apart by -`kid`, then remove the old one when the client has switched. +`kid`, then remove the old one when the client has switched. Whoever registers a key can name it +without being told: its thumbprint is what the client signs with unless configured otherwise. ## Access token diff --git a/src/client/src/Dbosoft.Bote.Client.Authentication/BoteAuthenticationOptionsExtensions.cs b/src/client/src/Dbosoft.Bote.Client.Authentication/BoteAuthenticationOptionsExtensions.cs index 1242a90..0a02995 100644 --- a/src/client/src/Dbosoft.Bote.Client.Authentication/BoteAuthenticationOptionsExtensions.cs +++ b/src/client/src/Dbosoft.Bote.Client.Authentication/BoteAuthenticationOptionsExtensions.cs @@ -22,10 +22,20 @@ public static SecurityKey GetSigningKey( // TODO Do we need to create new instance of ECDsa every time? (thread safety and dispose) var ecdsa = ECDsa.Create(); ecdsa.ImportPkcs8PrivateKey(Convert.FromBase64String(options.Authentication.SigningKey), out _); - return new ECDsaSecurityKey(ecdsa) - { - // TODO improve KID? - KeyId = $"{options.TenantId}-{options.ClientId}", - }; + var key = new ECDsaSecurityKey(ecdsa); + // The identity provider finds the key by this id among the client's keys. A fixed id per + // client (tenant and client) let a client hold one key only, so a new key could not be + // registered beside the old one while the client still signs with the old. + key.KeyId = string.IsNullOrWhiteSpace(options.Authentication.KeyId) + ? Thumbprint(key) + : options.Authentication.KeyId; + return key; } + + /// + /// The key's JWK thumbprint (RFC 7638, SHA-256, base64url): the same for whoever has the + /// public key, so whoever registers it can name it without being told. + /// + public static string Thumbprint(ECDsaSecurityKey key) => + Base64UrlEncoder.Encode(JsonWebKeyConverter.ConvertFromECDsaSecurityKey(key).ComputeJwkThumbprint()); } diff --git a/src/client/src/Dbosoft.Bote.Client.KeyGenerator/KeyGenerator.cs b/src/client/src/Dbosoft.Bote.Client.KeyGenerator/KeyGenerator.cs index cb85ced..d548f19 100644 --- a/src/client/src/Dbosoft.Bote.Client.KeyGenerator/KeyGenerator.cs +++ b/src/client/src/Dbosoft.Bote.Client.KeyGenerator/KeyGenerator.cs @@ -9,12 +9,11 @@ public class KeyGenerator { public KeyInfo GenerateKey() { - var keyId = Guid.NewGuid().ToString(); using var key = ECDsa.Create(ECCurve.CreateFromFriendlyName("secp256r1")); - var securityKey = new ECDsaSecurityKey(key) - { - KeyId = keyId, - }; + var securityKey = new ECDsaSecurityKey(key); + // The id the client signs with when none is configured: the key's thumbprint. + var keyId = Base64UrlEncoder.Encode(JsonWebKeyConverter.ConvertFromECDsaSecurityKey(securityKey).ComputeJwkThumbprint()); + securityKey.KeyId = keyId; var jwk = JsonWebKeyConverter.ConvertFromECDsaSecurityKey(securityKey); var publicJwkJson = (string)typeof(JsonWebKey) diff --git a/test/Dbosoft.Bote.Rebus.Tests/SigningKeyIdTests.cs b/test/Dbosoft.Bote.Rebus.Tests/SigningKeyIdTests.cs new file mode 100644 index 0000000..5837bf9 --- /dev/null +++ b/test/Dbosoft.Bote.Rebus.Tests/SigningKeyIdTests.cs @@ -0,0 +1,54 @@ +using System.Security.Cryptography; +using System.Text; +using AwesomeAssertions; +using Dbosoft.Bote.Client.Authentication; +using Dbosoft.Bote.Client.Options; +using Microsoft.IdentityModel.Tokens; +using Xunit; + +namespace Dbosoft.Bote.Rebus.Tests; + +// A fixed key id per client (tenant and client) let a client hold one key only: the identity +// provider takes a key id once, so a new key could not stand beside the old one. +public class SigningKeyIdTests +{ + private static BoteOptions Options(ECDsa key, string? keyId = null) => new() + { + TenantId = "tenant", + ClientId = "client", + Authentication = new BoteAuthenticationOptions + { + AuthenticationType = BoteAuthenticationType.Value, + SigningKey = Convert.ToBase64String(key.ExportPkcs8PrivateKey()), + KeyId = keyId!, + }, + }; + + [Fact] + public void Signs_under_the_configured_key_id() + { + using var key = ECDsa.Create(ECCurve.NamedCurves.nistP256); + + Options(key, "key-2").GetSigningKey().KeyId.Should().Be("key-2"); + } + + [Fact] + public void Signs_under_the_keys_rfc_7638_thumbprint_when_none_is_configured() + { + using var key = ECDsa.Create(ECCurve.NamedCurves.nistP256); + var p = key.ExportParameters(false); + var canonical = $$"""{"crv":"P-256","kty":"EC","x":"{{Base64UrlEncoder.Encode(p.Q.X)}}","y":"{{Base64UrlEncoder.Encode(p.Q.Y)}}"}"""; + var expected = Base64UrlEncoder.Encode(SHA256.HashData(Encoding.UTF8.GetBytes(canonical))); + + Options(key).GetSigningKey().KeyId.Should().Be(expected); + } + + [Fact] + public void Names_two_keys_of_one_client_apart() + { + using var old = ECDsa.Create(ECCurve.NamedCurves.nistP256); + using var next = ECDsa.Create(ECCurve.NamedCurves.nistP256); + + Options(old).GetSigningKey().KeyId.Should().NotBe(Options(next).GetSigningKey().KeyId); + } +}