diff --git a/docs/agent-identity/INTENT.agent-identity.md b/docs/agent-identity/INTENT.agent-identity.md index 8e2763815..43577caf3 100644 --- a/docs/agent-identity/INTENT.agent-identity.md +++ b/docs/agent-identity/INTENT.agent-identity.md @@ -164,7 +164,7 @@ Not intended: Authority-issued nonces would tolerate a skewed agent clock, but they cost a round trip after an idle period, stall a retry after a lost response, and need issuance state shared across authority nodes. - The authority records a nonce only after the request signature and any body digest are valid. Requests with an invalid signature or body digest therefore cannot consume nonce storage. -- The authority keeps each accepted nonce until 60 s after its signature expires, the last moment the freshness window could accept the same signature again. +- The authority keeps each accepted nonce at least until 60 s after its signature expires, the last moment the freshness window could accept the same signature again. - Published trust anchors help third parties validate certificate chains, but they carry no revocation information. ### Agent channel diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/AuthenticationTypeTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/AuthenticationTypeTests.cs index 3986c5d25..1d16d9616 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/AuthenticationTypeTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/AuthenticationTypeTests.cs @@ -3,7 +3,7 @@ namespace Devolutions.Agent.Identity.Authority.Tests; /// -/// The types that only request authentication, the channel handshake and the enrollment rules create. +/// The types that only request authentication, the channel handshake, and the enrollment and renewal rules create. /// /// /// INTENT.md: code that acts for an identity takes it as a type only the signature check can create, @@ -23,6 +23,10 @@ public class AuthenticationTypeTests // so a decision built by a caller would certify a key for an identity that no enrollment created. // oracle: intent dotnet/Devolutions.Agent.Identity.Authority/INTENT.md §Authenticated identity typeof(EnrollmentDecision), + // A renewal's certificate is issued from the decision to issue it, which carries the renewing identity, + // so a decision built by a caller would certify a key for an identity that no renew request proved. + // oracle: intent dotnet/Devolutions.Agent.Identity.Authority/INTENT.md §Authenticated identity + typeof(RenewalDecision), // oracle: mutant a public AuthenticationResult constructor typeof(AuthenticationResult), // oracle: mutant a public ChannelOpeningResult constructor @@ -55,7 +59,7 @@ public void TypeHasNoPublicOrProtectedConstructor(Type type) // The intent forbids creating an identity outside that check but not forwarding one that it created, // so the list of members allowed to produce one is the library's own. // oracle: mutant a public member that builds an AuthenticatedIdentity outside the signature check, - // or an EnrollmentDecision outside the enrollment rules + // or an EnrollmentDecision or a RenewalDecision outside the enrollment or renewal rules [Fact] public void NoOtherPublicMemberProducesTheseTypes() { @@ -82,6 +86,7 @@ public void NoOtherPublicMemberProducesTheseTypes() "ChannelOpening.VerifyHelloAsync", "ChannelOpeningResult.get_Opening", "EnrollmentRules.Decide", + "RenewalRules.Decide", "RequestAuthenticator.AuthenticateAsync", "RequestAuthenticator.AuthenticateChannelOpeningAsync", ], diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/RequestAuthenticatorTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/RequestAuthenticatorTests.cs index e080d003a..20fd7eb8e 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/RequestAuthenticatorTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/RequestAuthenticatorTests.cs @@ -262,17 +262,53 @@ public async Task RenewGraceIsTheCertificateLifetimeAfterItsNotAfter( // CONTRACT.agent-identity.md §6 states the window of step 3 and the certificate rule of step 4 // against the same `now`. // A second reading of the clock between the window and the certificate check - // would judge the certificate a day later, after it expired. + // would judge the certificate a second later, after it expired, though the window still holds then. // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §signatures [Fact] public async Task WindowAndCertificateAreJudgedAtOneReadingOfTheClock() { DateTimeOffset first = DeviceACertificate.NotAfter; - TestAuthority authority = new(first, clock: new SteppingTimeProvider(first, first.AddDays(1))); + TestAuthority authority = new(first, clock: new SteppingTimeProvider(first, first.AddSeconds(1))); Assert.Equal("ok", await authority.OutcomeAsync(authority.Valid(AgentOperation.Confirm))); } + // The shared intent keeps each accepted nonce until 60 s after its signature expires, the last instant at which + // step 3 accepts the signature, so a store may have discarded the pair after that instant. + // A request whose window closes while the registry answers would reach the store after it, + // where a copy of the request could find no pair and be accepted twice, so step 3 fails at that later now. + // The request expires 60 s after Now, so its window closes 120 s after Now; the lookup ends one tick later. + // oracle: intent docs/agent-identity/INTENT.agent-identity.md §Request authentication + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §signatures + [Theory] + [MemberData(nameof(Operations))] + public async Task RequestWhoseWindowClosesBeforeItsNonceIsRecordedIsClockSkew(AgentOperation operation) + { + FixedTimeProvider clock = new(Now); + TestAuthority authority = new(Now, clock: clock); + authority.Registry.OnLookup = () => clock.Now = Now.AddSeconds(120).AddTicks(1); + + string outcome = await authority.OutcomeAsync(authority.Valid(operation)); + + Assert.Equal("clock_skew", outcome); + Assert.Empty(authority.Nonces.Calls); + } + + // Step 3 accepts a signature until 60 s after it expires, that instant included, + // so a request still inside its window when its nonce is recorded is accepted, however long the registry took. + // The request expires 60 s after Now, so its window closes 120 s after Now, when the lookup ends. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §signatures + [Theory] + [MemberData(nameof(Operations))] + public async Task RequestInsideItsWindowWhenItsNonceIsRecordedIsAccepted(AgentOperation operation) + { + FixedTimeProvider clock = new(Now); + TestAuthority authority = new(Now, clock: clock); + authority.Registry.OnLookup = () => clock.Now = Now.AddSeconds(120); + + Assert.Equal("ok", await authority.OutcomeAsync(authority.Valid(operation))); + } + // Step 3: 0 < expires − created ≤ 300. // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §signatures [Fact] diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/RequestAuthenticatorVectorTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/RequestAuthenticatorVectorTests.cs index bf57b3ff4..84819a570 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/RequestAuthenticatorVectorTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Authentication/RequestAuthenticatorVectorTests.cs @@ -135,13 +135,15 @@ public async Task FailingCaseDetailHoldsNoRequestData(string name) Assert.All(headers.Concat(values), value => Assert.DoesNotContain(value, outcome.FailureDetail)); } - // CONTRACT.agent-identity.md §6 step 6 and the shared intent: the authority keeps each accepted nonce - // until 60 s after its signature expires, the last moment the window could accept the signature again. + // CONTRACT.agent-identity.md §6 step 6 accepts each (keyid, nonce) pair once, and the shared intent keeps each + // accepted nonce until 60 s after its signature expires, the last instant at which step 3 accepts the signature. + // The store call follows the last check of the window and may discard the pair once retainUntil has passed, + // so a copy of the signature accepted at that instant reaches the store after it, and the pair must outlive it. // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §signatures // oracle: intent docs/agent-identity/INTENT.agent-identity.md §Request authentication [Theory] [MemberData(nameof(OkCaseNames))] - public async Task OkCaseRecordsItsNonceOnceUntilExpiresPlus60Seconds(string name) + public async Task OkCaseRecordsItsNonceOnceUntilAfterItsWindowCloses(string name) { HttpSignatureCaseVector vector = Vectors.Case(name); VectorAuthority authority = new(); @@ -150,11 +152,12 @@ public async Task OkCaseRecordsItsNonceOnceUntilExpiresPlus60Seconds(string name string signatureInput = vector.Headers.SignatureInput!; long expires = long.Parse(Parameter(signatureInput, "expires"), CultureInfo.InvariantCulture); - (string, string, DateTimeOffset) recorded = ( - Parameter(signatureInput, "keyid"), - Parameter(signatureInput, "nonce"), - DateTimeOffset.FromUnixTimeSeconds(expires + 60)); - Assert.Equal([recorded], authority.Nonces.Calls); + DateTimeOffset windowEnd = DateTimeOffset.FromUnixTimeSeconds(expires + 60); + (string KeyId, string Nonce, DateTimeOffset RetainUntil) call = Assert.Single(authority.Nonces.Calls); + Assert.Equal(Parameter(signatureInput, "keyid"), call.KeyId); + Assert.Equal(Parameter(signatureInput, "nonce"), call.Nonce); + // oracle: mutant a retainUntil whose margin past the window's end is not 60 s, such as one tick + Assert.Equal(windowEnd.AddSeconds(60), call.RetainUntil); } // Code that acts for an identity relies on these facts, diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Certificates/IdentityCertificateIssuerTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Certificates/IdentityCertificateIssuerTests.cs index 2b7cea213..6eefa338d 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Certificates/IdentityCertificateIssuerTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Certificates/IdentityCertificateIssuerTests.cs @@ -42,10 +42,11 @@ public sealed class IdentityCertificateIssuerTests : IDisposable public static TheoryData SerialNumberDrawNames => new(SerialNumberDraws.Keys); // A token that can create an identity. - // The `u` of the token that enrolls the identity, and the CSR of the enrollment, which names it. + // The `u` of the token that enrolls the identity, which the CSR of each enrollment names. private const string TokenUrl = "https://host/dvls"; - private static readonly CertificateSigningRequest EnrollingCsr = TestEnrollmentUrl.Csr(TokenUrl); + // The CSR of another device enrolling with the same token: another key, the same URL. + private static readonly CertificateSigningRequest OtherKeyCsr = TestEnrollmentUrl.Csr(TokenUrl); private static EnrollmentTokenState UsableToken => new(false, TokenUrl, Now.AddDays(1), maxUses: 10, identitiesCreated: 0); @@ -56,24 +57,37 @@ public sealed class IdentityCertificateIssuerTests : IDisposable [EnrollmentOutcome.Reject] = EnrollmentRules.Decide( new(false, TokenUrl, Now, maxUses: 10, identitiesCreated: 0), - EnrollingCsr, + OtherKeyCsr, null, Now), [EnrollmentOutcome.ReturnExistingIdentity] = EnrollmentRules.Decide( UsableToken, - EnrollingCsr, + OtherKeyCsr, IssuedKeyMatch.ForIdentity(Guid.Parse(IdentityIdText), true, false, CertificateStatus.Current), Now), }; + // For each renewal outcome but a new pending certificate, the issued key whose CSR gets it: + // the renewing identity's pending key gets that certificate again, and its current key gets invalid_request. + private static Dictionary> KeysButANewPendingCertificate => new() + { + [RenewalOutcome.ReturnPendingCertificate] = identityId => + IssuedKeyMatch.ForIdentity(identityId, false, false, CertificateStatus.Pending), + [RenewalOutcome.Reject] = identityId => + IssuedKeyMatch.ForIdentity(identityId, false, false, CertificateStatus.Current), + }; + private readonly ECDsa _rootKey = ECDsa.Create(ECCurve.NamedCurves.nistP256); private readonly ECDsa _agentKey = ECDsa.Create(ECCurve.NamedCurves.nistP256); private readonly RandomNumberGenerator _random = RandomNumberGenerator.Create(); private readonly X509Certificate2 _root; + + // The CSR of the agent key, which names the token's URL, and which each test issues for unless it passes another. private readonly CertificateSigningRequest _csr; - // The decision that creates the identity whose certificate each test issues, unless the test passes another. - private readonly EnrollmentDecision _creating = EnrollmentRules.Decide(UsableToken, EnrollingCsr, null, Now); + // The decision that creates the identity whose certificate each test issues, unless the test passes another, + // made for the CSR of the agent key. + private readonly EnrollmentDecision _creating; // The ID that the decision minted, as CONTRACT.agent-identity.md §1 writes IDs: // Guid.ToString("D") writes a UUID lowercase and hyphenated. @@ -82,9 +96,8 @@ public sealed class IdentityCertificateIssuerTests : IDisposable public IdentityCertificateIssuerTests() { _root = CreateRoot(Now.AddDays(-1), Now.AddYears(10)); - - byte[] csr = new CertificateRequest("CN=agent", _agentKey, HashAlgorithmName.SHA256).CreateSigningRequest(); - _csr = CertificateSigningRequest.Validate(csr).Csr!; + _csr = TestEnrollmentUrl.Csr(_agentKey, TokenUrl); + _creating = EnrollmentRules.Decide(UsableToken, _csr, null, Now); } public void Dispose() @@ -604,59 +617,88 @@ public async Task RenewalCertifiesTheKeyOfTheCsrForTheIdentityThatSignedTheRenew Assert.Equal(_agentKey.ExportSubjectPublicKeyInfo(), leaf.PublicKey.ExportSubjectPublicKeyInfo()); } - // Code that acts for an identity takes the identity that the signature check produced, - // and only a renew request proves the identity that a renewal acts for. - // An identity that proved its key for another operation never had a renew request accepted for it. - // oracle: intent dotnet/Devolutions.Agent.Identity.Authority/INTENT.md §Authenticated identity + // Every issued public key is unique by SPKI SHA-256, so a certificate is issued only for the key whose lookup + // decided to issue it: another key may already be issued, or be decided on by a concurrent request. + // A host that issues from one enrollment's decision with another request's CSR would certify a key twice. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §token-rules + // oracle: spec Framework Design Guidelines §Validating Arguments + [Fact] + public void ThrowsForTheCsrOfAnotherKeyThanTheEnrollmentDecision() + { + ArgumentException exception = Assert.Throws(() => Issue(csr: OtherKeyCsr)); + + // oracle: spec Framework Design Guidelines §Using Standard Exception Types + Assert.Equal("csr", exception.ParamName); + } + + // Every issued public key is unique by SPKI SHA-256, and a renew request whose CSR key was never issued gets a new + // pending certificate for that key, so a renewal decision issues for its own CSR key only. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §token-rules // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §renew // oracle: spec Framework Design Guidelines §Validating Arguments - [Theory] - [InlineData(nameof(IdentityCertificateIssuer), AgentOperation.Confirm)] - [InlineData(nameof(IdentityCertificateIssuer), AgentOperation.CheckIn)] - [InlineData(nameof(IdentityCertificateIssuer), AgentOperation.Connect)] - [InlineData(nameof(RenewalRules), AgentOperation.Confirm)] - [InlineData(nameof(RenewalRules), AgentOperation.CheckIn)] - [InlineData(nameof(RenewalRules), AgentOperation.Connect)] - public async Task RenewalThrowsForAnIdentityAuthenticatedForAnotherOperation( - string renewal, - AgentOperation operation) + [Fact] + public async Task RenewalThrowsForTheCsrOfAnotherKeyThanTheDecision() { - AuthenticatedIdentity identity = await TestIdentity.AuthenticateAsync(operation); + AuthenticatedIdentity renewing = await TestIdentity.AuthenticateAsync(AgentOperation.Renew); - ArgumentException exception = Assert.Throws(() => Renewals[renewal](identity)); + ArgumentException exception = + Assert.Throws(() => IssueForRenewal(renewing, csr: OtherKeyCsr)); // oracle: spec Framework Design Guidelines §Using Standard Exception Types - Assert.Equal("renewing", exception.ParamName); + Assert.Equal("csr", exception.ParamName); } - // INTENT.md: a renewal acts for the identity that a renew request proved, and null is no identity; - // the .NET Framework Design Guidelines throw ArgumentNullException for a null argument that is not allowed. - // oracle: intent dotnet/Devolutions.Agent.Identity.Authority/INTENT.md §Authenticated identity + // The CSR key of the identity's pending certificate gets that pending chain again, and the key of any other issued + // certificate gets invalid_request: neither issues a certificate, which would certify an issued key twice. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §renew + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §token-rules // oracle: spec Framework Design Guidelines §Validating Arguments [Theory] - [InlineData(nameof(IdentityCertificateIssuer))] - [InlineData(nameof(RenewalRules))] - public void RenewalThrowsForANullIdentity(string renewal) + [InlineData(RenewalOutcome.ReturnPendingCertificate)] + [InlineData(RenewalOutcome.Reject)] + public async Task RenewalThrowsForADecisionThatIssuesNoCertificate(RenewalOutcome outcome) { - ArgumentNullException exception = Assert.Throws(() => Renewals[renewal](null!)); + AuthenticatedIdentity renewing = await TestIdentity.AuthenticateAsync(AgentOperation.Renew); + RenewalDecision decision = + RenewalRules.Decide(renewing, _csr, KeysButANewPendingCertificate[outcome](renewing.IdentityId)); + + ArgumentException exception = Assert.Throws(() => IssueForRenewal(decision)); // oracle: spec Framework Design Guidelines §Using Standard Exception Types - Assert.Equal("renewing", exception.ParamName); + Assert.Equal("issuing", exception.ParamName); } - // The two calls that act for a renewing identity, named by the class of each. - private Dictionary> Renewals => new() + // A null argument throws ArgumentNullException, whose ParamName names it. + // oracle: spec Framework Design Guidelines §Validating Arguments + [Fact] + public void RenewalThrowsForANullDecision() { - [nameof(IdentityCertificateIssuer)] = renewing => IssueForRenewal(renewing).Dispose(), - [nameof(RenewalRules)] = renewing => RenewalRules.Decide(renewing, null), - }; + ArgumentNullException exception = + Assert.Throws(() => IssueForRenewal((RenewalDecision)null!)); - private X509Certificate2 IssueForRenewal(AuthenticatedIdentity renewing, string enrollmentUrl = TokenUrl) + // oracle: spec Framework Design Guidelines §Using Standard Exception Types + Assert.Equal("issuing", exception.ParamName); + } + + // Decides a renew request of the identity for the CSR of the agent key, + // then issues its pending certificate with the given CSR, by default that one. + private X509Certificate2 IssueForRenewal( + AuthenticatedIdentity renewing, + string enrollmentUrl = TokenUrl, + CertificateSigningRequest? csr = null) + { + return IssueForRenewal(RenewalRules.Decide(renewing, _csr, null), enrollmentUrl, csr); + } + + private X509Certificate2 IssueForRenewal( + RenewalDecision issuing, + string enrollmentUrl = TokenUrl, + CertificateSigningRequest? csr = null) { return IdentityCertificateIssuer.Issue( - renewing, + issuing, enrollmentUrl, - _csr, + csr ?? _csr, new TestRootSigner(_root, _rootKey), Lifetime, new FixedTimeProvider(Now), diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Certificates/RenewalRulesTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Certificates/RenewalRulesTests.cs index 8cd4a0b37..aff5054d0 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Certificates/RenewalRulesTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Certificates/RenewalRulesTests.cs @@ -1,3 +1,5 @@ +using System.Security.Cryptography; + namespace Devolutions.Agent.Identity.Authority.Tests; /// @@ -10,6 +12,9 @@ public class RenewalRulesTests { private static readonly Guid OtherIdentityId = TestVectors.Instance.Key("device-b").IdentityId; + // The CSR of the renew request; a renewal ignores the enrollment URL it names (CONTRACT.agent-identity.md §4). + private static readonly CertificateSigningRequest Csr = TestEnrollmentUrl.Csr("https://host/dvls"); + // CONTRACT.agent-identity.md §5.3: the key of any other certificate that the server has issued, including this // identity's current or retired certificates and every certificate of another identity, gets invalid_request. // A deleted identity's certificates are certificates that the server has issued. @@ -48,10 +53,12 @@ public async Task ReturnsThePendingCertificateForThePendingKeyOfTheIdentity(bool identityRevoked: false, CertificateStatus.Pending); - RenewalDecision decision = RenewalRules.Decide(renewing, keyMatch); + RenewalDecision decision = RenewalRules.Decide(renewing, Csr, keyMatch); Assert.Equal(RenewalOutcome.ReturnPendingCertificate, decision.Outcome); Assert.Null(decision.Error); + // oracle: mutant a decision to return the pending certificate that carries the key digest of a decision to issue + Assert.True(decision.SubjectPublicKeyInfoSha256.IsEmpty); } // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §renew @@ -61,7 +68,7 @@ public async Task RejectsAnyOtherIssuedKeyWithInvalidRequest(string key) { AuthenticatedIdentity renewing = await TestIdentity.AuthenticateAsync(AgentOperation.Renew); - RenewalDecision decision = RenewalRules.Decide(renewing, OtherIssuedKeys[key](renewing.IdentityId)); + RenewalDecision decision = RenewalRules.Decide(renewing, Csr, OtherIssuedKeys[key](renewing.IdentityId)); Assert.Equal(RenewalOutcome.Reject, decision.Outcome); Assert.Equal(AgentIdentityErrorCode.InvalidRequest, decision.Error?.Code); @@ -70,6 +77,8 @@ public async Task RejectsAnyOtherIssuedKeyWithInvalidRequest(string key) // oracle: mutant an empty failure detail for the CSR key of a deleted identity, of another identity, // or of a current or retired certificate of the renewing identity Assert.False(string.IsNullOrEmpty(decision.FailureDetail)); + // oracle: mutant a refusal that carries the key digest of a decision to issue + Assert.True(decision.SubjectPublicKeyInfoSha256.IsEmpty); } // If the CSR key was never issued, the server issues a new pending certificate @@ -80,9 +89,74 @@ public async Task IssuesAPendingCertificateForAKeyNeverIssued() { AuthenticatedIdentity renewing = await TestIdentity.AuthenticateAsync(AgentOperation.Renew); - RenewalDecision decision = RenewalRules.Decide(renewing, null); + RenewalDecision decision = RenewalRules.Decide(renewing, Csr, null); Assert.Equal(RenewalOutcome.IssuePendingCertificate, decision.Outcome); Assert.Null(decision.Error); } + + // Every issued public key is unique by SPKI SHA-256, the SHA-256 of the DER SubjectPublicKeyInfo, + // so a decision to issue names the key it was made for by that digest, computed here from the key itself. + // A decision naming another value would have the host index a key other than the one it certifies. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §token-rules + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §definitions + [Fact] + public async Task IssuePendingCertificateCarriesTheSpkiSha256OfTheCsrKey() + { + using ECDsa key = ECDsa.Create(ECCurve.NamedCurves.nistP256); + CertificateSigningRequest csr = TestEnrollmentUrl.Csr(key, "https://host/dvls"); + AuthenticatedIdentity renewing = await TestIdentity.AuthenticateAsync(AgentOperation.Renew); + + RenewalDecision decision = RenewalRules.Decide(renewing, csr, null); + + Assert.Equal(SHA256.HashData(key.ExportSubjectPublicKeyInfo()), decision.SubjectPublicKeyInfoSha256.ToArray()); + } + + // Code that acts for an identity takes the identity that the signature check produced, + // and only a renew request proves the identity that a renewal acts for. + // An identity that proved its key for another operation never had a renew request accepted for it. + // oracle: intent dotnet/Devolutions.Agent.Identity.Authority/INTENT.md §Authenticated identity + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §renew + // oracle: spec Framework Design Guidelines §Validating Arguments + [Theory] + [InlineData(AgentOperation.Confirm)] + [InlineData(AgentOperation.CheckIn)] + [InlineData(AgentOperation.Connect)] + public async Task ThrowsForAnIdentityAuthenticatedForAnotherOperation(AgentOperation operation) + { + AuthenticatedIdentity identity = await TestIdentity.AuthenticateAsync(operation); + + ArgumentException exception = Assert.Throws(() => RenewalRules.Decide(identity, Csr, null)); + + // oracle: spec Framework Design Guidelines §Using Standard Exception Types + Assert.Equal("renewing", exception.ParamName); + } + + // INTENT.md: a renewal acts for the identity that a renew request proved, and null is no identity; + // the .NET Framework Design Guidelines throw ArgumentNullException for a null argument that is not allowed. + // oracle: intent dotnet/Devolutions.Agent.Identity.Authority/INTENT.md §Authenticated identity + // oracle: spec Framework Design Guidelines §Validating Arguments + [Fact] + public void ThrowsForANullIdentity() + { + ArgumentNullException exception = + Assert.Throws(() => RenewalRules.Decide(null!, Csr, null)); + + // oracle: spec Framework Design Guidelines §Using Standard Exception Types + Assert.Equal("renewing", exception.ParamName); + } + + // A null argument throws ArgumentNullException, whose ParamName names it. + // oracle: spec Framework Design Guidelines §Validating Arguments + [Fact] + public async Task ThrowsForANullCsr() + { + AuthenticatedIdentity renewing = await TestIdentity.AuthenticateAsync(AgentOperation.Renew); + + ArgumentNullException exception = + Assert.Throws(() => RenewalRules.Decide(renewing, null!, null)); + + // oracle: spec Framework Design Guidelines §Using Standard Exception Types + Assert.Equal("csr", exception.ParamName); + } } diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Channel/ChannelHandshakeTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Channel/ChannelHandshakeTests.cs index 47c2d88e4..07eccae74 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Channel/ChannelHandshakeTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Channel/ChannelHandshakeTests.cs @@ -741,7 +741,7 @@ public async Task HelloExactlyAtTheTimeoutSucceeds() DateTimeOffset opened = VectorChannel.OpeningTime; DateTimeOffset answered = opened.AddSeconds(10); ChannelOpening opening = await VectorChannel.OpenAsync( - new SteppingTimeProvider(opened, opened, answered, answered.AddTicks(1))); + new SteppingTimeProvider(opened, opened, opened, answered, answered.AddTicks(1))); ChannelHandshakeResult result = await opening.VerifyHelloAsync(VectorChannel.ValidHelloMessage(opening)); @@ -751,8 +751,8 @@ public async Task HelloExactlyAtTheTimeoutSucceeds() // Steps 1 and 2: the agent sends nothing until it receives the Challenge, // which the server sends once it has verified the opening request. // The agent can answer only once the opening is verified and its challenge sent, - // so the 10 s run from the end of verification, not from the earlier reading that judged the request. - // Here the registry and the nonce store take 5 s, and the Hello comes 10 s after them. + // so the 10 s run from the end of verification, not from the earlier readings that judged the request. + // Here the nonce store takes 5 s after the request is last judged, and the Hello comes 10 s after it. // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §channel-sequence [Fact] public async Task HelloTimeoutRunsFromTheEndOfTheOpening() @@ -760,7 +760,7 @@ public async Task HelloTimeoutRunsFromTheEndOfTheOpening() DateTimeOffset judged = VectorChannel.OpeningTime; DateTimeOffset verified = judged.AddSeconds(5); ChannelOpening opening = await VectorChannel.OpenAsync( - new SteppingTimeProvider(judged, verified, verified.AddSeconds(10))); + new SteppingTimeProvider(judged, judged, verified, verified.AddSeconds(10))); ChannelHandshakeResult result = await opening.VerifyHelloAsync(VectorChannel.ValidHelloMessage(opening)); @@ -895,7 +895,7 @@ private static async Task HandshakeAsync( DateTimeOffset opened = VectorChannel.OpeningTime; ChannelOpening opening = await VectorChannel.OpenAsync( authority, - new SteppingTimeProvider(opened, opened, scenario.HandshakeTime ?? opened)); + new SteppingTimeProvider(opened, opened, opened, scenario.HandshakeTime ?? opened)); if (scenario.CertificateRemoved) { @@ -918,12 +918,13 @@ private static async Task HandshakeAsync( // Opens a stream five seconds before the certificate of device-a expires, with a valid Hello signed for it, // so that the handshake can reach notAfter within the Hello timeout. - // The clock reads the opening's time to judge the request and to start the timeout, then handshakeTime. + // The clock reads the opening's time to judge the request, to judge its window again before recording its nonce, + // and to start the timeout, then handshakeTime. private static async Task<(ChannelOpening Opening, AgentMessage Hello)> OpenBeforeNotAfterAsync( DateTimeOffset handshakeTime) { DateTimeOffset opened = DeviceANotAfter.AddSeconds(-5); - TestAuthority authority = new(opened, clock: new SteppingTimeProvider(opened, opened, handshakeTime)); + TestAuthority authority = new(opened, clock: new SteppingTimeProvider(opened, opened, opened, handshakeTime)); ChannelOpening opening = await authority.OpenChannelAsync(); diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/CsrVectorEndpointTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/CsrVectorEndpointTests.cs index 4b45e0e9d..72163fa18 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/CsrVectorEndpointTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/CsrVectorEndpointTests.cs @@ -72,7 +72,7 @@ private static async Task Renew(CsrVector vector) } AuthenticatedIdentity renewing = await TestIdentity.AuthenticateAsync(AgentOperation.Renew); - RenewalDecision decision = RenewalRules.Decide(renewing, null); + RenewalDecision decision = RenewalRules.Decide(renewing, result.Csr, null); return decision.Outcome == RenewalOutcome.IssuePendingCertificate ? "ok" : decision.Error!.Code.ToWireValue(); } diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/EnrollmentRulesTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/EnrollmentRulesTests.cs index b067d372f..83ec5b0c2 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/EnrollmentRulesTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/EnrollmentRulesTests.cs @@ -1,3 +1,5 @@ +using System.Security.Cryptography; + namespace Devolutions.Agent.Identity.Authority.Tests; public class EnrollmentRulesTests @@ -82,17 +84,53 @@ public void EachCreateIdentityCarriesADifferentIdentityId() Assert.NotEqual(first.NewIdentityId, second.NewIdentityId); } - // A refusal creates no identity, and a replay returns the identity that the key already belongs to. + // Concurrent identity creations never create more than `max_uses` identities with one token, + // so a decision to create one carries the count of identities it was made from, the token's own, + // for a host that counts the use only while the stored count still equals it. + // A decision carrying another count would have that update refuse every enrollment. + // oracle: mutant a decision to create an identity that carries another count than the token's IdentitiesCreated + [Theory] + [InlineData(0)] + [InlineData(9)] + public void CreateIdentityCarriesTheCountOfIdentitiesItWasDecidedFrom(int identitiesCreated) + { + EnrollmentTokenState token = new(false, TokenUrl, Now.AddDays(1), maxUses: 10, identitiesCreated); + + EnrollmentDecision decision = EnrollmentRules.Decide(token, Csr, null, Now); + + Assert.Equal(identitiesCreated, decision.ExpectedIdentitiesCreated); + } + + // Every issued public key is unique by SPKI SHA-256, the SHA-256 of the DER SubjectPublicKeyInfo, + // so a decision to create an identity names the key it was made for by that digest, computed here from the key. + // A decision naming another value would have the host index a key other than the one it certifies. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §token-rules + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §definitions + [Fact] + public void CreateIdentityCarriesTheSpkiSha256OfTheCsrKey() + { + using ECDsa key = ECDsa.Create(ECCurve.NamedCurves.nistP256); + CertificateSigningRequest csr = TestEnrollmentUrl.Csr(key, TokenUrl); + + EnrollmentDecision decision = EnrollmentRules.Decide(Token(), csr, null, Now); + + Assert.Equal(SHA256.HashData(key.ExportSubjectPublicKeyInfo()), decision.SubjectPublicKeyInfoSha256.ToArray()); + } + + // A refusal creates no identity, and a replay returns the identity that the key already belongs to, + // so neither carries what creating one takes. [Theory] - // oracle: mutant a refusal that carries a new identity ID + // oracle: mutant a refusal that carries a new identity ID, a count of identities created, or a key digest [InlineData(EnrollmentOutcome.Reject)] - // oracle: mutant a replay that carries a new identity ID + // oracle: mutant a replay that carries a new identity ID, a count of identities created, or a key digest [InlineData(EnrollmentOutcome.ReturnExistingIdentity)] - public void CarriesNoNewIdentityIdWhenCreatingNone(EnrollmentOutcome outcome) + public void CarriesNoNewIdentityWhenCreatingNone(EnrollmentOutcome outcome) { EnrollmentDecision decision = DecisionsButCreateIdentity[outcome]; Assert.Null(decision.NewIdentityId); + Assert.Null(decision.ExpectedIdentitiesCreated); + Assert.True(decision.SubjectPublicKeyInfoSha256.IsEmpty); } // When several outcomes apply, the earliest wins: diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/EnrollmentTokenTests.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/EnrollmentTokenTests.cs index 8b5f66304..9a7b31d95 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/EnrollmentTokenTests.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Enrollment/EnrollmentTokenTests.cs @@ -133,9 +133,6 @@ public void ProductBaseUrlIsTheUOfTheBag(string productBaseUrl) // White space around a URI is not part of it. // oracle: spec RFC 3986 §Appendix C [InlineData(" https://host/dvls\t", "https://host/dvls")] - // A non-ASCII character becomes its UTF-8 octets, percent-encoded with uppercase hexadecimal digits. - // oracle: spec RFC 3987 §3.1 - [InlineData("https://host/dvls\u00E9", "https://host/dvls%C3%A9")] public void CreateRejectsOrNormalizesAUrlNotInTheFormOfU(string productBaseUrl, string normalized) { Creation creation = CreateFrom(productBaseUrl); @@ -143,16 +140,12 @@ public void CreateRejectsOrNormalizesAUrlNotInTheFormOfU(string productBaseUrl, Assert.Contains(creation, (Creation[])[Creation.Holding(normalized), Creation.Rejection]); } - // The unacceptable URLs, a URL one byte longer than the longest that fits in a token, - // and URLs that are not ASCII, since `u` holds only ASCII characters (CONTRACT.agent-identity.md §2): - // an internationalized host not in its `xn--` form, and a fullwidth commercial at (U+FF20) in the host. + // The unacceptable URLs, and a URL one byte longer than the longest that fits in a token. // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enrollment-token public static TheoryData RejectedUrlRows => new( [ .. UnacceptableUrls.All, UrlOfLength(3026), - "https://bücher.example/dvls", - "https://admin＠host/dvls", ]); // oracle: spec Framework Design Guidelines §Validating Arguments diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/TestDoubles.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/TestDoubles.cs index 951d805cf..75d96bcd9 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/TestDoubles.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/TestDoubles.cs @@ -44,6 +44,11 @@ internal sealed class FakeCertificateRegistry : ICertificateRegistry /// public Task? Gate { get; set; } + /// + /// An action that every lookup runs before it answers, when set, such as moving a test's clock on. + /// + public Action? OnLookup { get; set; } + public void Register(string thumbprint, RegisteredCertificate certificate) { _certificates[thumbprint] = certificate; @@ -67,6 +72,7 @@ public void Unregister(string thumbprint) CancellationToken cancellationToken) { Lookups.Add(thumbprint); + OnLookup?.Invoke(); if (Gate is { } gate) { diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/TestEnrollmentUrl.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/TestEnrollmentUrl.cs index de1388574..015311133 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/TestEnrollmentUrl.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/TestEnrollmentUrl.cs @@ -144,6 +144,14 @@ public static byte[] Request(ECDsa key, string url) public static CertificateSigningRequest Csr(string url) { using ECDsa key = ECDsa.Create(ECCurve.NamedCurves.nistP256); + return Csr(key, url); + } + + /// + /// The validated CSR an agent sends for a key, with the enrollment URL extension of a URL. + /// + public static CertificateSigningRequest Csr(ECDsa key, string url) + { CsrValidationResult result = CertificateSigningRequest.Validate(Request(key, url)); Assert.True(result.Succeeded, result.FailureDetail); return result.Csr; diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/UnacceptableUrls.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/UnacceptableUrls.cs index f8a2d337b..278e44908 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/UnacceptableUrls.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.Tests/Support/UnacceptableUrls.cs @@ -1,14 +1,15 @@ namespace Devolutions.Agent.Identity.Authority.Tests; /// -/// URLs that are not an absolute https URL without an @, a query or a fragment, +/// URLs that are not an absolute https URL in ASCII without an @, a query or a fragment, /// the form that a token's u and a config's agent_channel_url both take. /// public static class UnacceptableUrls { // CONTRACT.agent-identity.md §2 makes `u`, and §5.2 makes `agent_channel_url`, an absolute `https` URL // without a user name or password, a query or a fragment, read as text: - // it begins with `https://` and contains no '@', since any '@' counts as a user name. + // it begins with `https://` and contains no '@', since any '@' counts as a user name, + // and holds only ASCII characters, so an internationalized host is written in its `xn--` form. public static string[] All { get; } = [ // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enrollment-token @@ -63,6 +64,18 @@ public static class UnacceptableUrls // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enrollment-token // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enroll "https:@host/dvls", + // An internationalized host not in its `xn--` form. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enrollment-token + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enroll + "https://b\u00FCcher.example/dvls", + // A non-ASCII character in the path, which the IRI-to-URI mapping of RFC 3987 §3.1 would percent-encode. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enrollment-token + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enroll + "https://host/dvls\u00E9", + // A fullwidth commercial at (U+FF20), which is not the ASCII '@' but which NFKC normalization turns into one. + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enrollment-token + // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §enroll + "https://admin\uFF20host/dvls", ]; public static TheoryData Rows => new(All); diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Authentication/INonceStore.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Authentication/INonceStore.cs index c5dc8f625..595814b8b 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Authentication/INonceStore.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Authentication/INonceStore.cs @@ -16,8 +16,10 @@ public interface INonceStore /// The keyid of the signature, which is the thumbprint of its certificate. /// The nonce of the signature, as its canonical base64url text. /// - /// The last instant at which the signature could be accepted again, - /// until which the store keeps the pair; it may discard the pair afterwards. + /// The instant until which the store keeps the pair. + /// It falls 60 s after the last instant at which the signature could be accepted again, + /// a margin for the time that this call takes, + /// so a store whose calls complete in under 60 s may discard the pair once it has passed. /// /// A token that cancels the operation. /// True when this call recorded the pair; false when the pair was already recorded. diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Authentication/RequestAuthenticator.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Authentication/RequestAuthenticator.cs index 33d761c92..c5f8d1bf0 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Authentication/RequestAuthenticator.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Authentication/RequestAuthenticator.cs @@ -25,9 +25,14 @@ public sealed class RequestAuthenticator private const string EcPublicKeyOid = "1.2.840.10045.2.1"; // CONTRACT.agent-identity.md §6 step 6, and the shared intent: - // a nonce is kept until 60 s after its signature expires. + // a nonce is kept until 60 s after its signature expires, the last instant at which step 3 accepts it. private static readonly TimeSpan NonceRetentionAfterExpiry = TimeSpan.FromSeconds(60); + // The window is judged a last time just before the nonce store is called, and the call takes time after that + // reading, so a copy of the signature accepted at the window's last instant can reach the store after that instant. + // The pair is kept this much longer, so that such a copy still finds it. + private static readonly TimeSpan NonceRetentionMargin = TimeSpan.FromSeconds(60); + // RFC 5480 §2.1.1.1: the DER encoding of the namedCurve secp256r1, which is P-256. private static readonly byte[] P256CurveParameters = [0x06, 0x08, 0x2A, 0x86, 0x48, 0xCE, 0x3D, 0x03, 0x01, 0x07]; @@ -68,7 +73,7 @@ public RequestAuthenticator( /// A body larger than fails first, /// with . /// The request then goes through the steps of CONTRACT.agent-identity.md §6, - /// and its nonce is recorded only once every other step has passed. + /// and its nonce is recorded only once every other step has passed and the window of step 3 still holds. /// /// The operation of the endpoint that received the request. /// The request, as received. @@ -126,7 +131,7 @@ public async ValueTask AuthenticateAsync( /// /// /// The opening request goes through the steps of CONTRACT.agent-identity.md §6 with tag connect, - /// and its nonce is recorded only once every other step has passed. + /// and its nonce is recorded only once every other step has passed and the window of step 3 still holds. /// A verified opening does not authenticate the stream: /// the agent must still sign , /// which checks. @@ -169,7 +174,7 @@ public async ValueTask AuthenticateChannelOpeningAsync( byte[] challenge = new byte[ChallengeLength]; _randomNumberGenerator.GetBytes(challenge); - // The Hello timeout runs from the end of verification rather than from the reading that judged the request, + // The Hello timeout runs from the end of verification rather than from the readings that judged the request, // so that time spent in the registry and the nonce store does not shorten the agent's time to answer. DateTimeOffset openedAt = _timeProvider.GetUtcNow(); @@ -215,6 +220,7 @@ private async ValueTask VerifyAsync( // Step 3. // Steps 3 and 4 compare against one reading of the clock, exactly, so that they agree on what now is. + // The window alone is judged again before step 6. DateTimeOffset now = _timeProvider.GetUtcNow(); if (!IsWithinWindow(fields.Created, fields.Expires, now)) { @@ -293,8 +299,25 @@ private async ValueTask VerifyAsync( "Step 5: the signature does not verify."); } + // Step 3 again, just before the nonce is recorded. + // The store keeps a pair only until the margin after the window closes, so a request whose window closed during + // the steps above could reach the store after a copy's pair was discarded, and both would be accepted. + // This is step 3 evaluated at a later now, so clock_skew is the earliest failing step. + // UNTESTED: this reading is the last clock reading and the last await before TryRecordAsync, + // because NonceRetentionMargin covers only the time that the store call takes after it. + // Work awaited between them would let a copy of the signature reach the store after its pair was discarded. + // Keep this check immediately before the store call, or widen the margin for whatever is added. + if (!IsWithinWindow(fields.Created, fields.Expires, _timeProvider.GetUtcNow())) + { + return Verification.Reject( + AgentIdentityErrorCode.ClockSkew, + "Step 3: the accepted window closed before the nonce was recorded."); + } + // Step 6. - DateTimeOffset retainUntil = DateTimeOffset.FromUnixTimeSeconds(fields.Expires) + NonceRetentionAfterExpiry; + DateTimeOffset retainUntil = DateTimeOffset.FromUnixTimeSeconds(fields.Expires) + + NonceRetentionAfterExpiry + + NonceRetentionMargin; bool recorded = await _nonceStore .TryRecordAsync(fields.KeyId, fields.Nonce, retainUntil, cancellationToken) .ConfigureAwait(false); diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Bodies/IdentityConfig.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Bodies/IdentityConfig.cs index f2ac55d3f..6b09b55c8 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Bodies/IdentityConfig.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Bodies/IdentityConfig.cs @@ -75,13 +75,13 @@ private IdentityConfig(ulong revision, string? agentChannelUrl, byte[] utf8Json) /// /// The revision of the config. /// - /// The absolute https base URL of the agent channel, such as https://host/dvls, + /// The absolute https base URL of the agent channel, in ASCII, such as https://host/dvls, /// without user information or any other @, query or fragment; /// null when the authority does not offer the channel to the identity, so that the agent opens none. /// /// The product-specific fields, if any. /// - /// is not an absolute https URL, + /// is not ASCII, is not an absolute https URL, /// or has user information or any other @, a query or a fragment; /// or a product-specific field has no name, a name that is not snake_case or that the contract gives a member, /// no value, the value null, or a value that UTF-8 JSON cannot carry, such as a lone surrogate; diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/IdentityCertificateIssuer.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/IdentityCertificateIssuer.cs index 142b82c40..009e96ba1 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/IdentityCertificateIssuer.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/IdentityCertificateIssuer.cs @@ -60,15 +60,19 @@ public static class IdentityCertificateIssuer /// /// Taking the decision rather than an ID keeps the authority from certifying a key for an identity /// that no enrollment created. + /// Requiring the key that the decision was made for keeps it from certifying a key whose lookup no decision saw, + /// which could be a key already issued. /// A renewal certifies a key for an existing identity, - /// so it uses the overload that takes the identity that the renew request proved. + /// so it uses the overload that takes the renewal decision. /// /// /// /// The decision of to create the identity, /// whose is . /// - /// The validated CSR whose key the certificate is for. + /// + /// The validated CSR whose key the certificate is for, the key that was made for. + /// /// The root that signs the certificate. /// The certificate lifetime, usually . /// The clock, read once. @@ -76,6 +80,8 @@ public static class IdentityCertificateIssuer /// An argument is null. /// /// does not create an identity; + /// or the SHA-256 digest of the SubjectPublicKeyInfo of is not the + /// of ; /// or the issuer certificate has no subject key identifier, or its notAfter is not after the current second; /// or the issuer is outside the chain profile of CONTRACT.agent-identity.md §4: /// its certificate has neither a P-256 nor a P-384 key, no basicConstraints with cA set or no key usage with @@ -100,6 +106,8 @@ public static X509Certificate2 Issue( throw new ArgumentException("The enrollment decision does not create an identity.", nameof(creating)); } + ThrowIfNotTheDecidedKey(csr, creating.SubjectPublicKeyInfoSha256); + // A decision to create an identity always carries the ID minted for it and the token's URL. return Issue( creating.NewIdentityId!.Value, @@ -255,31 +263,44 @@ private static (HashAlgorithmName Hash, byte[] SignatureAlgorithm)? SignaturePro } /// - /// Issues the pending certificate of a renewal, for the identity that signed the renew request. + /// Issues the pending certificate that a renewal decision calls for, + /// for the identity that signed the renew request. /// /// - /// The certificate is the one that the other overload issues, with - /// as the identity ID and in the enrollment URL extension. - /// Taking the identity that the signature check produced, rather than an ID, - /// keeps a renewal from certifying a key for an identity that a request only named. + /// + /// The certificate is the one that the other overload issues, with the identity ID of the + /// that was decided for + /// and in the enrollment URL extension. + /// + /// + /// Only produces a decision, from the identity that the signature check produced, + /// so taking the decision rather than an ID keeps a renewal from certifying a key for an identity that a request + /// only named. + /// Requiring the key that the decision was made for keeps it from certifying a key whose lookup no decision saw, + /// which could be a key already issued. + /// /// - /// - /// The identity that a renew request proved, as - /// returns it for . + /// + /// The decision of to issue a pending certificate, + /// whose is . /// /// /// The identity's enrollment URL, the of the decision that created /// it, which every certificate of the identity names. /// - /// The validated CSR whose key the certificate is for. + /// + /// The validated CSR whose key the certificate is for, the key that was made for. + /// /// The root that signs the certificate. /// The certificate lifetime, usually . /// The clock, read once. /// The source of the serial number. /// An argument is null. /// - /// was authenticated for another operation than renew; + /// does not issue a pending certificate; /// or is empty, ends in a slash or is not ASCII; + /// or the SHA-256 digest of the SubjectPublicKeyInfo of is not the + /// of ; /// or the issuer certificate has no subject key identifier, or its notAfter is not after the current second; /// or the issuer is outside the chain profile of CONTRACT.agent-identity.md §4: /// its certificate has neither a P-256 nor a P-384 key, no basicConstraints with cA set or no key usage with @@ -290,7 +311,7 @@ private static (HashAlgorithmName Hash, byte[] SignatureAlgorithm)? SignaturePro /// is shorter than one second. /// public static X509Certificate2 Issue( - AuthenticatedIdentity renewing, + RenewalDecision issuing, string enrollmentUrl, CertificateSigningRequest csr, IRootSigner issuer, @@ -298,11 +319,11 @@ public static X509Certificate2 Issue( TimeProvider timeProvider, RandomNumberGenerator random) { - ArgumentNullException.ThrowIfNull(renewing); + ArgumentNullException.ThrowIfNull(issuing); - if (renewing.Operation != AgentOperation.Renew) + if (issuing.Outcome != RenewalOutcome.IssuePendingCertificate) { - throw new ArgumentException("The identity was not authenticated by a renew request.", nameof(renewing)); + throw new ArgumentException("The renewal decision does not issue a pending certificate.", nameof(issuing)); } ArgumentNullException.ThrowIfNull(enrollmentUrl); @@ -315,7 +336,22 @@ public static X509Certificate2 Issue( nameof(enrollmentUrl)); } - return Issue(renewing.IdentityId, enrollmentUrl, csr, issuer, lifetime, timeProvider, random); + ThrowIfNotTheDecidedKey(csr, issuing.SubjectPublicKeyInfoSha256); + + // A decision to issue a pending certificate always carries the identity that it was made for. + return Issue(issuing.Renewing!.IdentityId, enrollmentUrl, csr, issuer, lifetime, timeProvider, random); + } + + // CONTRACT.agent-identity.md §2.1: every issued key is unique by SPKI SHA-256, + // which the decision's key lookup checked for its own CSR key only. + private static void ThrowIfNotTheDecidedKey(CertificateSigningRequest csr, ReadOnlyMemory decidedKey) + { + ArgumentNullException.ThrowIfNull(csr); + + if (!csr.SubjectPublicKeyInfoSha256.Span.SequenceEqual(decidedKey.Span)) + { + throw new ArgumentException("The CSR key is not the key that the decision was made for.", nameof(csr)); + } } private static DateTimeOffset TruncateToSecond(DateTimeOffset value) diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalDecision.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalDecision.cs index 92786910f..ccb5415df 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalDecision.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalDecision.cs @@ -5,9 +5,16 @@ namespace Devolutions.Agent.Identity.Authority; /// public sealed class RenewalDecision { - private RenewalDecision(RenewalOutcome outcome, AgentIdentityError? error, string? failureDetail) + private RenewalDecision( + RenewalOutcome outcome, + AuthenticatedIdentity? renewing, + ReadOnlyMemory subjectPublicKeyInfoSha256, + AgentIdentityError? error, + string? failureDetail) { Outcome = outcome; + Renewing = renewing; + SubjectPublicKeyInfoSha256 = subjectPublicKeyInfoSha256; Error = error; FailureDetail = failureDetail; } @@ -17,6 +24,17 @@ private RenewalDecision(RenewalOutcome outcome, AgentIdentityError? error, strin /// public RenewalOutcome Outcome { get; } + /// + /// The SHA-256 digest of the SubjectPublicKeyInfo of the CSR that the decision was made for, + /// its , + /// when is ; otherwise empty. + /// + /// + /// The decision holds only for the key whose lookup it was made from, + /// so the overload that takes the decision certifies no other key. + /// + public ReadOnlyMemory SubjectPublicKeyInfoSha256 { get; } + /// /// The error that refuses the request, 400 invalid_request, /// when is ; otherwise null. @@ -29,16 +47,31 @@ private RenewalDecision(RenewalOutcome outcome, AgentIdentityError? error, strin /// public string? FailureDetail { get; } - internal static RenewalDecision IssuePendingCertificate { get; } = - new(RenewalOutcome.IssuePendingCertificate, null, null); + // The identity that the renew request proved, when the decision issues a pending certificate; otherwise null. + // Internal so that the certificate is issued for the identity that the decision was made for, and no other. + internal AuthenticatedIdentity? Renewing { get; } + + internal static RenewalDecision IssuePendingCertificate( + AuthenticatedIdentity renewing, + ReadOnlyMemory subjectPublicKeyInfoSha256) + { + return new RenewalDecision( + RenewalOutcome.IssuePendingCertificate, + renewing, + subjectPublicKeyInfoSha256, + null, + null); + } internal static RenewalDecision ReturnPendingCertificate { get; } = - new(RenewalOutcome.ReturnPendingCertificate, null, null); + new(RenewalOutcome.ReturnPendingCertificate, null, ReadOnlyMemory.Empty, null, null); internal static RenewalDecision Reject(string failureDetail) { return new RenewalDecision( RenewalOutcome.Reject, + null, + ReadOnlyMemory.Empty, AgentIdentityError.From(AgentIdentityErrorCode.InvalidRequest), failureDetail); } diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalOutcome.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalOutcome.cs index 63d00dd0f..8a9dace82 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalOutcome.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalOutcome.cs @@ -12,7 +12,7 @@ public enum RenewalOutcome /// /// Issue a pending certificate for the CSR key, with the overload that - /// takes the renewing identity, and retire the identity's previous pending certificate, if any. + /// takes the decision, and retire the identity's previous pending certificate, if any. /// IssuePendingCertificate = 1, diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalRules.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalRules.cs index 04c036e97..12f0d4c0a 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalRules.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Certificates/RenewalRules.cs @@ -18,26 +18,37 @@ public static class RenewalRules /// the identity's current or retired key, any key of another identity, revoked or not, /// and any key of a deleted identity. /// A key never issued gets a new pending certificate, which retires the identity's previous pending certificate, - /// if any. + /// if any, and which the overload that takes the decision issues + /// for and the key of alone. /// Renewal never promotes a certificate; only confirm does. /// /// /// Every accepted request, including one that gets the pending certificate again, replaces the identity's metadata. /// The decision holds only for the state it was made from. - /// The host issues the certificate atomically, so that of two concurrent renewals with one CSR key, - /// exactly one issues the certificate and the other gets that pending certificate. + /// The host stores the pending certificate, inserts its key under a unique index of every issued key, + /// and retires the pending certificate that the identity holds then, in one transaction that first takes a write + /// lock on the identity's record, and decides again from the stored state when the index refuses the key, + /// acting on the new decision, so that of two concurrent renewals with one CSR key, + /// exactly one issues the certificate and the other gets that pending certificate, + /// and two concurrent renewals with different keys leave one pending certificate. /// /// /// /// The identity that the renew request proved, as returns it /// for . /// + /// The request's validated CSR. /// The issued certificate whose public key is the CSR key, or null when none has it. - /// is null. + /// + /// or is null. + /// /// /// was authenticated for another operation than renew. /// - public static RenewalDecision Decide(AuthenticatedIdentity renewing, IssuedKeyMatch? keyMatch) + public static RenewalDecision Decide( + AuthenticatedIdentity renewing, + CertificateSigningRequest csr, + IssuedKeyMatch? keyMatch) { ArgumentNullException.ThrowIfNull(renewing); @@ -46,9 +57,11 @@ public static RenewalDecision Decide(AuthenticatedIdentity renewing, IssuedKeyMa throw new ArgumentException("The identity was not authenticated by a renew request.", nameof(renewing)); } + ArgumentNullException.ThrowIfNull(csr); + if (keyMatch is null) { - return RenewalDecision.IssuePendingCertificate; + return RenewalDecision.IssuePendingCertificate(renewing, csr.SubjectPublicKeyInfoSha256); } // A public key belongs to one certificate lineage, so only the pending key of the identity itself is a retry. diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.csproj b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.csproj index ab99845b6..0e5268263 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.csproj +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority.csproj @@ -14,7 +14,7 @@ Agent-facing protocol rules shared by every Devolutions Agent Identity authority © Devolutions Inc. All rights reserved. Devolutions.Agent.Identity.Authority - 2026.10.26 + 2026.10.27 MIT OR Apache-2.0 README.md https://github.com/Devolutions/devolutions-gateway.git diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentDecision.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentDecision.cs index 17f346142..46eb58163 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentDecision.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentDecision.cs @@ -9,6 +9,8 @@ private EnrollmentDecision( EnrollmentOutcome outcome, Guid? newIdentityId, string? enrollmentUrl, + int? expectedIdentitiesCreated, + ReadOnlyMemory subjectPublicKeyInfoSha256, Guid? existingIdentityId, AgentIdentityError? error, string? failureDetail) @@ -16,6 +18,8 @@ private EnrollmentDecision( Outcome = outcome; NewIdentityId = newIdentityId; EnrollmentUrl = enrollmentUrl; + ExpectedIdentitiesCreated = expectedIdentitiesCreated; + SubjectPublicKeyInfoSha256 = subjectPublicKeyInfoSha256; ExistingIdentityId = existingIdentityId; Error = error; FailureDetail = failureDetail; @@ -47,6 +51,31 @@ private EnrollmentDecision( /// public string? EnrollmentUrl { get; } + /// + /// The token's that the decision was made from, + /// when is ; otherwise null. + /// + /// + /// The authority counts the token use in the transaction that creates the identity, with an update that succeeds + /// only while the stored count is still below the token's , + /// or, stricter, only while it still equals this value, + /// so that concurrent enrollments never create more identities than that limit. + /// When the update fails, the authority decides again from the stored state and acts on the new decision, + /// discarding any certificate issued from this one. + /// + public int? ExpectedIdentitiesCreated { get; } + + /// + /// The SHA-256 digest of the SubjectPublicKeyInfo of the CSR that the decision was made for, + /// its , + /// when is ; otherwise empty. + /// + /// + /// The decision holds only for the key whose lookup it was made from, + /// so the overload that takes the decision certifies no other key. + /// + public ReadOnlyMemory SubjectPublicKeyInfoSha256 { get; } + /// /// The identity whose enrollment the request replays, /// when is ; otherwise null. @@ -65,14 +94,34 @@ private EnrollmentDecision( /// public string? FailureDetail { get; } - internal static EnrollmentDecision CreateIdentity(Guid newIdentityId, string enrollmentUrl) + internal static EnrollmentDecision CreateIdentity( + Guid newIdentityId, + string enrollmentUrl, + int expectedIdentitiesCreated, + ReadOnlyMemory subjectPublicKeyInfoSha256) { - return new EnrollmentDecision(EnrollmentOutcome.CreateIdentity, newIdentityId, enrollmentUrl, null, null, null); + return new EnrollmentDecision( + EnrollmentOutcome.CreateIdentity, + newIdentityId, + enrollmentUrl, + expectedIdentitiesCreated, + subjectPublicKeyInfoSha256, + null, + null, + null); } internal static EnrollmentDecision ReturnExistingIdentity(Guid identityId) { - return new EnrollmentDecision(EnrollmentOutcome.ReturnExistingIdentity, null, null, identityId, null, null); + return new EnrollmentDecision( + EnrollmentOutcome.ReturnExistingIdentity, + null, + null, + null, + ReadOnlyMemory.Empty, + identityId, + null, + null); } internal static EnrollmentDecision Reject(AgentIdentityErrorCode code, string failureDetail) @@ -82,6 +131,8 @@ internal static EnrollmentDecision Reject(AgentIdentityErrorCode code, string fa null, null, null, + ReadOnlyMemory.Empty, + null, AgentIdentityError.From(code), failureDetail); } diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentRules.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentRules.cs index 9db283a32..adbea2f72 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentRules.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentRules.cs @@ -22,7 +22,8 @@ public static class EnrollmentRules /// then token_expired once reaches its expiry; /// token_exhausted once it has created identities; /// a new identity, whose ID, , is minted at random for this decision, - /// and whose certificates name . + /// whose certificates name , + /// and whose certificate holds the CSR key, . /// /// /// The CSR key lookup takes the first matching rule: @@ -34,7 +35,10 @@ public static class EnrollmentRules /// /// /// The decision holds only for the state it was made from. - /// The host creates the identity and counts the use atomically, + /// The host creates the identity, inserts its key under a unique index of every issued key, + /// and counts the use only while the stored count is still below , + /// or, stricter, still equals , in one transaction, + /// and decides again from the stored state when the count or the index refuses it, acting on the new decision, /// so that concurrent enrollments never create more than identities /// with one token, and of two concurrent enrollments with one CSR key, exactly one creates the identity /// and the other gets the replay. @@ -97,7 +101,11 @@ public static EnrollmentDecision Decide( "The token has created its maximum number of identities."); } - return EnrollmentDecision.CreateIdentity(Guid.NewGuid(), enrollmentUrl); + return EnrollmentDecision.CreateIdentity( + Guid.NewGuid(), + enrollmentUrl, + token.IdentitiesCreated, + csr.SubjectPublicKeyInfoSha256); } // The key rules after the replay, in their order; a deleted identity matches only its own rule. diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentToken.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentToken.cs index b69baf784..c4f41dafb 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentToken.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Enrollment/EnrollmentToken.cs @@ -1,6 +1,5 @@ using System.Buffers; using System.Security.Cryptography; -using System.Text; using System.Text.Json; namespace Devolutions.Agent.Identity.Authority; @@ -91,14 +90,6 @@ public static EnrollmentToken Create(string productBaseUrl, RandomNumberGenerato ArgumentNullException.ThrowIfNull(productBaseUrl); ArgumentNullException.ThrowIfNull(randomNumberGenerator); - // CONTRACT.agent-identity.md §2: `u` holds only ASCII characters, since certificates carry it as an IA5String. - if (!Ascii.IsValid(productBaseUrl)) - { - throw new ArgumentException( - "The product base URL is not ASCII; write an internationalized host in its xn-- form.", - nameof(productBaseUrl)); - } - ArrayBufferWriter json = new(); string u = BaseUrl.Normalize(productBaseUrl, "The product base URL", nameof(productBaseUrl)); diff --git a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Protocol/BaseUrl.cs b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Protocol/BaseUrl.cs index 5fd8d7395..6701465ef 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Protocol/BaseUrl.cs +++ b/dotnet/Devolutions.Agent.Identity.Authority/Devolutions.Agent.Identity.Authority/Protocol/BaseUrl.cs @@ -1,3 +1,5 @@ +using System.Text; + namespace Devolutions.Agent.Identity.Authority; /// @@ -9,15 +11,24 @@ internal static class BaseUrl /// Returns the URL in normalized absolute form without trailing slashes, /// so that the path the agent appends follows exactly one slash. /// - /// The URL text. + /// The URL text, in ASCII. /// What the URL is, as error messages start, such as The product base URL. /// The parameter that holds the URL. /// - /// The URL is not an absolute https URL, + /// The URL is not ASCII, is not an absolute https URL, /// or has user information or any other @, a query or a fragment. /// public static string Normalize(string url, string name, string paramName) { + // CONTRACT.agent-identity.md §2: `u` holds only ASCII characters, since certificates carry it as an IA5String, + // and §5.2 reads `agent_channel_url` as text like `u`. + if (!Ascii.IsValid(url)) + { + throw new ArgumentException( + $"{name} is not ASCII; write an internationalized host in its xn-- form.", + paramName); + } + if (!Uri.TryCreate(url, UriKind.Absolute, out Uri? parsed)) { throw new ArgumentException($"{name} is not an absolute URL.", paramName); diff --git a/dotnet/Devolutions.Agent.Identity.Authority/README.md b/dotnet/Devolutions.Agent.Identity.Authority/README.md index a46301f10..23989f3a7 100644 --- a/dotnet/Devolutions.Agent.Identity.Authority/README.md +++ b/dotnet/Devolutions.Agent.Identity.Authority/README.md @@ -9,8 +9,8 @@ The contract is `docs/agent-identity/CONTRACT.agent-identity.md` in the [devolut The host stores the token's `SecretHash` and its `ProductBaseUrl`, never its `Value`, which it shows once and never logs. - Request authentication: `RequestAuthenticator` verifies the signed `renew`, `confirm` and `check-in` requests and the signed opening of an agent channel stream (CONTRACT.agent-identity.md [§6]). - Request bodies: `EnrollRequestBody`, `RenewRequestBody` and `CheckInRequestBody` parse a body and validate its CSR (`CertificateSigningRequest`) and metadata (`AgentMetadata`), and `ConfirmRequestBody` checks that a `confirm` body is empty. -- Decisions: `EnrollmentRules.Decide` decides an enrollment and `RenewalRules.Decide` a renewal, given the issued certificate that holds the CSR key. -- Certificates: `IdentityCertificateIssuer.Issue` issues the certificate of the new identity that an enrollment decision creates, or of the identity that a renew request proved. +- Decisions: `EnrollmentRules.Decide` decides an enrollment and `RenewalRules.Decide` a renewal, given the request's CSR and the issued certificate that holds its key. +- Certificates: `IdentityCertificateIssuer.Issue` issues the certificate of the new identity that an enrollment decision creates, or the pending certificate that a renewal decision calls for, each for the CSR key that the decision was made for. - Responses: `IdentityConfig` holds an identity's config, and `ResponseBodies` writes the bodies of the `trust-anchor`, `enroll`, `renew` and `check-in` responses. - Agent channel: `ChannelOpening` verifies the handshake that authenticates a stream, and `ChannelMessages` builds the `ConfigUpdate`, `RenewRequested` and `Reconnect` messages that the authority pushes. - Errors: `AgentIdentityError` gives each error its code, HTTP status and message, and writes its JSON body and gRPC exception. @@ -31,6 +31,7 @@ The host provides these dependencies, with the guarantees the library relies on: Index certificates by `AgentIdentityProtocol.ComputeThumbprint`, not by the SHA-1 `X509Certificate2.Thumbprint`. Each `RegisteredCertificate` holds the DER exactly as issued, and the current `Status` and `IdentityRevoked`, so that a revocation takes effect from the next request. - `INonceStore.TryRecordAsync` records a `keyid` and `nonce` pair, or returns false when the pair is already recorded, in one atomic operation, and keeps the pair until `retainUntil`. + `retainUntil` falls 60 s after the last instant at which the signature could be accepted again, a margin for the time the call takes, so a store whose calls complete in under 60 s may discard the pair once `retainUntil` has passed. - `IRootSigner` holds the root that issues new certificates and a `SignatureGenerator` over its private key. Its `Certificate` must have a Subject Key Identifier extension, which becomes the Authority Key Identifier of each issued certificate; without one, `IdentityCertificateIssuer.Issue` throws `ArgumentException`. The root must also fit the chain profile (CONTRACT.agent-identity.md [§4]): a P-256 or P-384 key, basicConstraints with `cA` set, key usage with `keyCertSign`, and a `SignatureGenerator` that signs with `ecdsa-with-SHA256` for a P-256 key or `ecdsa-with-SHA384` for a P-384 key; otherwise `Issue` throws `ArgumentException`. @@ -39,9 +40,20 @@ The host provides these dependencies, with the guarantees the library relies on: Take the `now` of `EnrollmentRules.Decide` and the `serverTime` of every error from the same clock, since the agent corrects its own clock from `server_time`. - `RandomNumberGenerator` must be cryptographically secure, such as `RandomNumberGenerator.Create()`, since it supplies channel challenges, token secrets and certificate serial numbers. +A decision of `EnrollmentRules.Decide` or `RenewalRules.Decide` holds only for the stored state it was made from, which a concurrent request can change before the host acts on it. +To keep the concurrency properties of CONTRACT.agent-identity.md [§2.1], the host follows these rules: + +- Keep a unique index on the SPKI SHA-256 of every issued key, those of current, pending and retired certificates and the retained records of deleted identities' keys, and insert each key in the transaction that stores its certificate. + Two requests can both find a key unissued, and the index lets only one of them issue it. +- Count a token use with an update that succeeds only while the stored count is still below the token's `MaxUses`, in the transaction that creates the identity. + A host may instead require the count to still equal the decision's `ExpectedIdentitiesCreated`, which also refuses when another enrollment took a use below the limit, at the cost of deciding again more often when many devices enroll with one token at once. + Two enrollments can both decide from the count before the token's last use, and the update lets only one of them create an identity. +- When that update fails or the key insert hits the unique index, roll the transaction back, look the key up again, call `Decide` again from the stored state, and act on the new decision as for a new request: discard any certificate issued from the old decision, and issue again from the new one. + The new decision gives the replay, the pending certificate or the error that the other request's commit calls for, or, under the stricter update, when the stored count changed but is still below the limit, a new identity to create under a new `NewIdentityId`, which the discarded certificate does not name. + The library puts this load on the registry and the nonce store: -- Each signed request that passes parsing, the tag and the time window costs one `FindByThumbprintAsync`, and only one whose signature verifies also costs one `TryRecordAsync` (CONTRACT.agent-identity.md [§6]). +- Each signed request that passes parsing, the tag and the time window costs one `FindByThumbprintAsync`, and only one whose signature verifies, and whose window has not closed since, also costs one `TryRecordAsync` (CONTRACT.agent-identity.md [§6]). The thumbprint comes from the request, so an unauthenticated client can drive lookups with random `keyid`s. - Each channel stream whose opening verifies costs two registry reads, one for the opening and one when the handshake ends, as CONTRACT.agent-identity.md [§7.4] requires, and one nonce write. @@ -77,8 +89,8 @@ Answer with `ResponseBodies.TrustAnchor`, given the published roots: one normall 5. Pass the token's `EnrollmentTokenState`, with the `ProductBaseUrl` stored for it, the body's `Csr`, the match and the current time to `EnrollmentRules.Decide`, and act on its `Outcome`; a CSR that names no enrollment URL, or not the token's, gets `token_url_mismatch` before any key rule applies (CONTRACT.agent-identity.md [§2.1]): - `Reject`: answer with `Error`. - `ReturnExistingIdentity`: take identity `ExistingIdentityId`, and change nothing, not even its `last_seen_at`, since a replay proves no current possession of the device key. - - `CreateIdentity`: take `NewIdentityId`, which the library minted, as the identity's ID, issue its certificate with the `IdentityCertificateIssuer.Issue` overload that takes the decision, which names the decision's `EnrollmentUrl` in it, name it with `FriendlyNameFormat.Evaluate`, create its config with `IdentityConfig.Create`, and store it with its metadata, its `EnrollmentUrl`, the key's `SubjectPublicKeyInfoSha256`, and the current time as its `last_seen_at`. - Create the identity and count the token use atomically, as CONTRACT.agent-identity.md [§2.1] requires of concurrent enrollments. + - `CreateIdentity`: take `NewIdentityId`, which the library minted, as the identity's ID, issue its certificate, which names the decision's `EnrollmentUrl`, with the `IdentityCertificateIssuer.Issue` overload that takes the decision and the body's `Csr`, name it with `FriendlyNameFormat.Evaluate`, create its config with `IdentityConfig.Create`, and store it with its metadata, its `EnrollmentUrl`, the key's `SubjectPublicKeyInfoSha256`, and the current time as its `last_seen_at`. + Create the identity, insert its key and count the token use in one transaction, and decide again when the transaction fails, as [Host dependencies][host-dependencies] describes. 6. Answer with `ResponseBodies.Enroll`, given the authority ID and the ID taken in step 5, with that identity's current certificate chain, leaf first, and config (CONTRACT.agent-identity.md [§5.2]). ### Signed requests @@ -95,11 +107,11 @@ Each then acts for the result's `Identity` only. ### `renew` 1. Look up the CSR key as for `enroll`, passing false for `enrolledWithPresentedToken`. -2. Pass `Identity` and the match to `RenewalRules.Decide`, and act on its `Outcome`: +2. Pass `Identity`, the body's `Csr` and the match to `RenewalRules.Decide`, and act on its `Outcome`: - `Reject`: answer with `Error`. - `ReturnPendingCertificate`: keep the identity's pending certificate, which holds the key. - - `IssuePendingCertificate`: issue a certificate with the `IdentityCertificateIssuer.Issue` overload that takes the `AuthenticatedIdentity`, passing the `EnrollmentUrl` stored with the identity so that every certificate of the identity names the same URL, and store it as the identity's pending certificate, retiring the previous one; the renew CSR's own enrollment URL is ignored (CONTRACT.agent-identity.md [§4]). - Store it atomically, as CONTRACT.agent-identity.md [§2.1] requires of concurrent renewals. + - `IssuePendingCertificate`: issue a certificate with the `IdentityCertificateIssuer.Issue` overload that takes the decision and the body's `Csr`, passing the `EnrollmentUrl` stored with the identity so that every certificate of the identity names the same URL, and store it as the identity's pending certificate; the renew CSR's own enrollment URL is ignored (CONTRACT.agent-identity.md [§4]). + Store the certificate, insert its key and retire whichever pending certificate the identity holds then, in one transaction that first takes a write lock on the identity's record, so that two concurrent renewals with different keys leave one pending certificate, and decide again when the key insert fails, as [Host dependencies][host-dependencies] describes. 3. Replace the identity's metadata, set its `last_seen_at` to the current time, and answer with `ResponseBodies.Renew`, given the pending certificate's chain (CONTRACT.agent-identity.md [§5.3]). ### `confirm` @@ -166,8 +178,10 @@ Log `FailureDetail` beside the error: it names the rule that the request broke a Only the signature check, `RequestAuthenticator.AuthenticateAsync`, and the channel handshake, `ChannelOpening.VerifyHelloAsync`, create an `AuthenticatedIdentity`, which has no public constructor. Code that acts for an identity should take one, so that it acts for the identity a signature proved, never for an identity ID that the agent or a caller sent. -The library does so itself: `RenewalRules.Decide` and the renewal overload of `IdentityCertificateIssuer.Issue` take the renewing `AuthenticatedIdentity`, and throw `ArgumentException` for one authenticated for another operation. -The certificate of a new identity is issued from the `EnrollmentDecision` that creates it, which only `EnrollmentRules.Decide` produces, under the `NewIdentityId` that it minted; the overload that takes the decision throws `ArgumentException` for one of another outcome. +The library does so itself: `RenewalRules.Decide` takes the renewing `AuthenticatedIdentity`, and throws `ArgumentException` for one authenticated for another operation. +A renewal's certificate is issued from the `RenewalDecision` that only `RenewalRules.Decide` produces, for the identity that it was given. +The certificate of a new identity is issued from the `EnrollmentDecision` that creates it, which only `EnrollmentRules.Decide` produces, under the `NewIdentityId` that it minted. +Each `IdentityCertificateIssuer.Issue` overload throws `ArgumentException` for a decision of another outcome, and for a CSR whose key is not the one that the decision was made for, since the decision's key lookup covered that key alone. No `IdentityCertificateIssuer.Issue` overload takes an identity ID. A verified channel opening yields only a `ChannelOpening`, whose identity stays hidden until the agent signs the stream's challenge, since signed opening headers are not tied to one stream and can be captured by a TLS-terminating proxy, TLS inspection or a header log (security invariant 7 of [INTENT.agent-identity.md][invariants]). For the same reason, `AuthenticateAsync` throws `ArgumentOutOfRangeException` for `AgentOperation.Connect`. @@ -206,3 +220,4 @@ Implement the `AgentChannel` service with that package; a feed that serves this [invariants]: https://github.com/Devolutions/devolutions-gateway/blob/master/docs/agent-identity/INTENT.agent-identity.md#security-invariants [last-seen-at]: https://github.com/Devolutions/devolutions-gateway/blob/master/docs/agent-identity/INTENT.agent-identity.md#configuration-and-metadata [errors]: #errors +[host-dependencies]: #host-dependencies