diff --git a/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainConverter.cs b/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainConverter.cs index b342a51d..0a40de46 100644 --- a/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainConverter.cs +++ b/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainConverter.cs @@ -1,7 +1,6 @@ // Contrast Security, Inc licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information. -using System; using System.IO; using System.Security.Cryptography.X509Certificates; using System.Text; @@ -18,8 +17,6 @@ public interface ITlsCertificateChainConverter public class TlsCertificateChainConverter : ITlsCertificateChainConverter { - private const byte GenerationVersion = 2; - public TlsCertificateChainExport Export(TlsCertificateChain chain) { var caCertificatePem = chain.CaCertificate.Export(X509ContentType.Pkcs12); @@ -29,22 +26,17 @@ public TlsCertificateChainExport Export(TlsCertificateChain chain) var serverCertificatePem = chain.ServerCertificate.Export(X509ContentType.Pkcs12); - return new TlsCertificateChainExport(caCertificatePem, caPublicPem, serverCertificatePem, new[]{ GenerationVersion }); + return new TlsCertificateChainExport(caCertificatePem, caPublicPem, serverCertificatePem, chain.Version); } public TlsCertificateChain Import(TlsCertificateChainExport export) { var (caCertificatePem, _, serverCertificatePem, version) = export; - if (version.Length != 1 || version[0] != GenerationVersion) - { - throw new Exception($"Incompatible generated version '{string.Join(",", version)}' detected, expected '{GenerationVersion}'."); - } - var caCertificate = new X509Certificate2(caCertificatePem, (string?)null, X509KeyStorageFlags.Exportable); var serverCertificate = new X509Certificate2(serverCertificatePem, (string?)null, X509KeyStorageFlags.Exportable); - return new TlsCertificateChain(caCertificate, serverCertificate); + return new TlsCertificateChain(caCertificate, serverCertificate, version); } private static byte[] CreatePem(object o) diff --git a/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainGenerator.cs b/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainGenerator.cs index 31412a5f..83c185da 100644 --- a/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainGenerator.cs +++ b/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainGenerator.cs @@ -19,6 +19,8 @@ public interface ITlsCertificateChainGenerator public class TlsCertificateChainGenerator : ITlsCertificateChainGenerator { + public static readonly byte[] GenerationVersion = { 3 }; + private readonly CreateCertificates _createCertificates; private readonly TlsCertificateOptions _options; @@ -33,7 +35,7 @@ public TlsCertificateChain CreateTlsCertificateChain() var ca = CreateRootCa(_options); var serverCertificate = CreateServerCertificate(ca); - return new TlsCertificateChain(ca, serverCertificate); + return new TlsCertificateChain(ca, serverCertificate, GenerationVersion); } private X509Certificate2 CreateRootCa(TlsCertificateOptions options) @@ -135,7 +137,7 @@ private X509Certificate2 CreateServerCertificate(X509Certificate2 signingCertifi }; } - public record TlsCertificateChain(X509Certificate2 CaCertificate, X509Certificate2 ServerCertificate) : IDisposable + public record TlsCertificateChain(X509Certificate2 CaCertificate, X509Certificate2 ServerCertificate, byte[] Version) : IDisposable { public void Dispose() { diff --git a/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainValidator.cs b/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainValidator.cs new file mode 100644 index 00000000..88c528b3 --- /dev/null +++ b/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateChainValidator.cs @@ -0,0 +1,50 @@ +// Contrast Security, Inc licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information. + +using System; +using System.Linq; + +namespace Contrast.K8s.AgentOperator.Core.Tls +{ + public interface ITlsCertificateChainValidator + { + bool IsValid(TlsCertificateChain chain, out ValidationResultReason reason); + } + + public class TlsCertificateChainValidator : ITlsCertificateChainValidator + { + public bool IsValid(TlsCertificateChain chain, out ValidationResultReason reason) + { + var renewThreshold = DateTime.Now + TimeSpan.FromDays(90); + + if (!chain.CaCertificate.HasPrivateKey + || !chain.ServerCertificate.HasPrivateKey) + { + reason = ValidationResultReason.MissingPrivateKey; + } + else if (chain.CaCertificate.NotAfter < renewThreshold + || chain.ServerCertificate.NotAfter < renewThreshold) + { + reason = ValidationResultReason.Expired; + } + else if (!TlsCertificateChainGenerator.GenerationVersion.SequenceEqual(chain.Version)) + { + reason = ValidationResultReason.OldVersion; + } + else + { + reason = ValidationResultReason.NoError; + } + + return reason == ValidationResultReason.NoError; + } + } + + public enum ValidationResultReason + { + NoError = 0, + MissingPrivateKey, + Expired, + OldVersion + } +} diff --git a/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateMaintenanceHandler.cs b/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateMaintenanceHandler.cs index 273c2cfb..76acea8a 100644 --- a/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateMaintenanceHandler.cs +++ b/src/Contrast.K8s.AgentOperator/Core/Tls/TlsCertificateMaintenanceHandler.cs @@ -22,24 +22,29 @@ public class TlsCertificateMaintenanceHandler : INotificationHandler notification, CancellationToken cancellationToken) { if (_webHookSecretParser.TryGetWebHookCertificateSecret(notification.Entity, out var chain)) { + // The certificate may not be valid, but that's not for us to figure out right now. + // At this point, we only care if the certificate was parseable. if (_certificateSelector.TakeOwnershipOfCertificate(chain)) { Logger.Info($"Secret '{notification.Entity.Namespace()}/{notification.Entity.Name()}' was changed, updated internal certificates."); @@ -58,27 +63,31 @@ public async Task Handle(LeaderStateChanged notification, CancellationToken canc if (notification.IsLeader) { var existingSecret = await _webHookConfigurationWriter.FetchCurrentCertificate(); - if (existingSecret == null) + if (existingSecret == null + || !_webHookSecretParser.TryGetWebHookCertificateSecret(existingSecret, out var chain)) { // Missing. await GenerateAndPublishCertificate(); } - else if (_webHookSecretParser.TryGetWebHookCertificateSecret(existingSecret, out var chain)) + else { using (chain) { - // Existing and valid, ensure web hook ca bundle is okay. - Logger.Info("Web hook certificate secret is valid."); - var chainExport = _certificateChainConverter.Export(chain); - await _webHookConfigurationWriter.UpdateClusterWebHookConfiguration(chainExport); + if (_validator.IsValid(chain, out var reason)) + { + // Existing and valid, ensure web hook ca bundle is okay. + Logger.Info("Web hook certificate secret is valid."); + var chainExport = _certificateChainConverter.Export(chain); + await _webHookConfigurationWriter.UpdateClusterWebHookConfiguration(chainExport); + } + else + { + // Invalid. + Logger.Info($"Web hook certificate secret is invalid (Reason: '{reason}')."); + await GenerateAndPublishCertificate(); + } } } - else - { - // Invalid. - Logger.Info("Web hook certificate secret is invalid."); - await GenerateAndPublishCertificate(); - } } } diff --git a/src/Contrast.K8s.AgentOperator/Core/Tls/WebHookSecretParser.cs b/src/Contrast.K8s.AgentOperator/Core/Tls/WebHookSecretParser.cs index 2d7c4c21..0b0fd53b 100644 --- a/src/Contrast.K8s.AgentOperator/Core/Tls/WebHookSecretParser.cs +++ b/src/Contrast.K8s.AgentOperator/Core/Tls/WebHookSecretParser.cs @@ -35,12 +35,18 @@ public bool TryGetWebHookCertificateSecret(V1Secret secret, [NotNullWhen(true)] && secret.Data != null && secret.Data.TryGetValue(_tlsStorageOptions.ServerCertificateName, out var serverCertificateBytes) && secret.Data.TryGetValue(_tlsStorageOptions.CaPublicName, out var caPublicBytes) - && secret.Data.TryGetValue(_tlsStorageOptions.CaCertificateName, out var caCertificateBytes) - && secret.Data.TryGetValue(_tlsStorageOptions.VersionName, out var versionBytes)) + && secret.Data.TryGetValue(_tlsStorageOptions.CaCertificateName, out var caCertificateBytes)) { + // Version is newer, so we might be upgrading from a version without versions. + var version = Array.Empty(); + if (secret.Data.TryGetValue(_tlsStorageOptions.VersionName, out var versionBytes)) + { + version = versionBytes; + } + try { - chain = _certificateChainConverter.Import(new TlsCertificateChainExport(caCertificateBytes, caPublicBytes, serverCertificateBytes, versionBytes)); + chain = _certificateChainConverter.Import(new TlsCertificateChainExport(caCertificateBytes, caPublicBytes, serverCertificateBytes, version)); var renewThreshold = DateTime.Now + TimeSpan.FromDays(90); return chain.CaCertificate.HasPrivateKey diff --git a/tests/Contrast.K8s.AgentOperator.Tests/Core/Tls/TlsCertificateChainValidatorTests.cs b/tests/Contrast.K8s.AgentOperator.Tests/Core/Tls/TlsCertificateChainValidatorTests.cs new file mode 100644 index 00000000..250ae2eb --- /dev/null +++ b/tests/Contrast.K8s.AgentOperator.Tests/Core/Tls/TlsCertificateChainValidatorTests.cs @@ -0,0 +1,81 @@ +// Contrast Security, Inc licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information. + +using System; +using AutoFixture; +using CertificateManager; +using Contrast.K8s.AgentOperator.Core.Tls; +using Contrast.K8s.AgentOperator.Options; +using FluentAssertions; +using FluentAssertions.Execution; +using Xunit; + +namespace Contrast.K8s.AgentOperator.Tests.Core.Tls +{ + public class TlsCertificateChainValidatorTests + { + private static readonly Fixture AutoFixture = new(); + + [Fact] + public void When_chain_expiration_is_valid_then_IsValid_should_return_true() + { + using var chainFake = FakeCertificates(TimeSpan.FromDays(180)); + + var validator = new TlsCertificateChainValidator(); + + // Act + var result = validator.IsValid(chainFake, out _); + + // Assert + result.Should().BeTrue(); + } + + [Fact] + public void When_chain_expiration_is_expired_then_IsValid_should_return_expired() + { + using var chainFake = FakeCertificates(TimeSpan.FromDays(45)); + + var validator = new TlsCertificateChainValidator(); + + // Act + var result = validator.IsValid(chainFake, out var reason); + + // Assert + using (new AssertionScope()) + { + result.Should().BeFalse(); + reason.Should().Be(ValidationResultReason.Expired); + } + } + + [Fact] + public void When_chain_version_differs_then_IsValid_should_return_old_version() + { + using var chainFake = FakeCertificates(TimeSpan.FromDays(180)) with + { + Version = AutoFixture.Create() + }; + + var validator = new TlsCertificateChainValidator(); + + // Act + var result = validator.IsValid(chainFake, out var reason); + + // Assert + using (new AssertionScope()) + { + result.Should().BeFalse(); + reason.Should().Be(ValidationResultReason.OldVersion); + } + } + + private static TlsCertificateChain FakeCertificates(TimeSpan expiresAfter) + { + var generator = new TlsCertificateChainGenerator( + new CreateCertificates(new CertificateUtility()), + AutoFixture.Create() with { ExpiresAfter = expiresAfter } + ); + return generator.CreateTlsCertificateChain(); + } + } +} diff --git a/tests/Contrast.K8s.AgentOperator.Tests/Core/Tls/TlsCertificateMaintenanceHandlerTests.cs b/tests/Contrast.K8s.AgentOperator.Tests/Core/Tls/TlsCertificateMaintenanceHandlerTests.cs index d5af0405..06bd9d44 100644 --- a/tests/Contrast.K8s.AgentOperator.Tests/Core/Tls/TlsCertificateMaintenanceHandlerTests.cs +++ b/tests/Contrast.K8s.AgentOperator.Tests/Core/Tls/TlsCertificateMaintenanceHandlerTests.cs @@ -61,10 +61,18 @@ public async Task When_leader_elected_and_secret_is_valid_then_cluster_web_hook_ var converterMock = Substitute.For(); converterMock.Export(chainFake).Returns(exportFake); + var validatorMock = Substitute.For(); + validatorMock.IsValid(Arg.Is(chainFake), out _).Returns(info => + { + info[1] = ValidationResultReason.NoError; + return true; + }); + var handler = CreateGraph( webHookSecretParser: parserMock, webHookConfigurationWriter: writerMock, - certificateChainConverter: converterMock + certificateChainConverter: converterMock, + certificateChainValidator:validatorMock ); // Act @@ -143,14 +151,16 @@ private static TlsCertificateMaintenanceHandler CreateGraph(IKestrelCertificateS ITlsCertificateChainGenerator? certificateChainGenerator = null, ITlsCertificateChainConverter? certificateChainConverter = null, IKubeWebHookConfigurationWriter? webHookConfigurationWriter = null, - IWebHookSecretParser? webHookSecretParser = null) + IWebHookSecretParser? webHookSecretParser = null, + ITlsCertificateChainValidator? certificateChainValidator = null) { return new TlsCertificateMaintenanceHandler( certificateSelector ?? Substitute.For(), certificateChainGenerator ?? Substitute.For(), certificateChainConverter ?? Substitute.For(), webHookConfigurationWriter ?? Substitute.For(), - webHookSecretParser ?? Substitute.For() + webHookSecretParser ?? Substitute.For(), + certificateChainValidator ?? Substitute.For() ); }