diff --git a/CHANGELOG.md b/CHANGELOG.md index 95b5c04..82b3f19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,23 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) loos ## Unreleased +## 2026.07.30.2350 + +- Build produced from commit 67cf0abac2dd. + +## Unreleased (carried forward) + +## 2026.07.30.2344 + +- Build produced from commit dadba2f4c890. + +## Unreleased (carried forward) + ## 2026.07.30.2305 - Build produced from commit f65124fd9911. -## Unreleased (carried forward) +## Unreleased (carried forward) ## 2026.07.30.2259 @@ -18,6 +30,13 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) loos ## Unreleased (carried forward) +### Fixed (certificate reuse) + +- **Switching certificate profiles reused the old certificate.** The reuse check matched on common name alone, so requesting from a client-authentication profile on a host already holding a server-authentication certificate for the same name returned the existing certificate — with the wrong extended key usages. The reuse search is now scoped by `-CertificateProfileId` or `-CertificateAuthorityId`. +- A reuse search that completed and found nothing no longer falls through to a name-only local match. Previously an empty result disabled the serial filter entirely, which is what allowed a certificate from another issuer to be returned. +- **Adding a SAN reused the old certificate.** Reuse compared only the common name, so extending `-DnsName` or `-IpAddress` returned the existing certificate without the new name. A candidate must now carry every requested name, and the name that disqualified it is reported. Comparison is coverage rather than equality — extra names on the certificate still qualify — with case-insensitive DNS matching and normalized IP addresses so `::1` matches `0:0:0:0:0:0:0:1`. +- When Infisical cannot be reached the check still falls back to matching on the common name, but now says so with a warning instead of silently. + ### Fixed (certificate installation) - **`Request-InfisicalCertificate -InstallChain` could hang indefinitely.** Adding a root certificate to `CurrentUser\Root` makes Windows raise a modal trust confirmation dialog, and `X509Store.Add` blocks until it is answered; when that dialog was hidden or the session non-interactive, the cmdlet appeared to stop right after installing the intermediate. A warning is now emitted before the blocking call, and the new elevation-aware default avoids the prompt entirely for elevated sessions. diff --git a/Module/PSInfisicalAPI/PSInfisicalAPI.psd1 b/Module/PSInfisicalAPI/PSInfisicalAPI.psd1 index 5db849e..727a891 100644 --- a/Module/PSInfisicalAPI/PSInfisicalAPI.psd1 +++ b/Module/PSInfisicalAPI/PSInfisicalAPI.psd1 @@ -1,6 +1,6 @@ @{ RootModule = 'PSInfisicalAPI.psm1' - ModuleVersion = '2026.07.30.2305' + ModuleVersion = '2026.07.30.2350' GUID = 'b8a2f3d4-7c51-4d2f-9e6a-1f0c8b3d4e51' Author = 'Grace Solutions' CompanyName = 'Grace Solutions' @@ -74,7 +74,7 @@ LicenseUri = 'https://www.gnu.org/licenses/agpl-3.0.html' ProjectUri = 'https://prod.git.gracesolution.info/gsadmin/PSInfisicalAPI' ReleaseNotes = 'See CHANGELOG.md in the project repository for release history.' - CommitHash = 'f65124fd9911' + CommitHash = '67cf0abac2dd' } } } \ No newline at end of file diff --git a/Module/PSInfisicalAPI/bin/PSInfisicalAPI.dll b/Module/PSInfisicalAPI/bin/PSInfisicalAPI.dll index a71e29a..a1a4d8a 100644 Binary files a/Module/PSInfisicalAPI/bin/PSInfisicalAPI.dll and b/Module/PSInfisicalAPI/bin/PSInfisicalAPI.dll differ diff --git a/Module/PSInfisicalAPI/bin/en-US/PSInfisicalAPI.dll-Help.xml b/Module/PSInfisicalAPI/bin/en-US/PSInfisicalAPI.dll-Help.xml index 5b888fb..132b5ff 100644 --- a/Module/PSInfisicalAPI/bin/en-US/PSInfisicalAPI.dll-Help.xml +++ b/Module/PSInfisicalAPI/bin/en-US/PSInfisicalAPI.dll-Help.xml @@ -1289,6 +1289,7 @@ $GetInfisicalCertificatePolicyResult = Get-InfisicalCertificatePolicy @GetInfisi Installing a root into CurrentUser\Root makes Windows display a modal trust confirmation dialog, and the call blocks until it is answered; in a non-interactive session this looks like a hang. The cmdlet emits a warning before blocking. Run elevated or pass -StoreLocation LocalMachine to install machine-wide without a prompt. Only the leaf honours -StoreName (default My). Chain members are routed by what they are: a self-signed certificate is a root and goes to the trusted-root store, anything with an issuer above it is a subordinate CA and goes to the intermediate store, for a chain of any depth. Issuers are installed before the leaf, and the chain is then validated against the machine's stores; an incomplete chain is reported as a warning naming the missing issuer, which is the condition Windows shows as "The issuer of this certificate could not be found". The installed certificate's Windows friendly name defaults to the common name in upper case. -FriendlyName overrides it and is accepted on every parameter set; on the -CertificateAuthorityId path the same value is additionally sent to Infisical as the issued certificate's friendlyName. + The reuse check is scoped to the issuer being requested: the search is filtered by -CertificateProfileId or -CertificateAuthorityId, so a certificate issued by a different profile is not reused. This matters when two profiles over one CA differ in key usage, such as server authentication versus client authentication, where a common-name match alone would return a certificate with the wrong extended key usages. Reuse additionally requires the existing certificate to carry every requested subject alternative name, so adding an entry to -DnsName or -IpAddress issues a new certificate instead of returning one that would fail validation for the new name. The rule is coverage rather than equality: a certificate carrying more names than requested still qualifies, DNS names compare case-insensitively, and IP addresses are normalized so ::1 matches 0:0:0:0:0:0:0:1. Use -Force when the SAN set needs trimming rather than extending. When Infisical cannot be reached the check falls back to matching on the common name alone and says so with a warning. diff --git a/Module/PSInfisicalAPI/en-US/PSInfisicalAPI.dll-Help.xml b/Module/PSInfisicalAPI/en-US/PSInfisicalAPI.dll-Help.xml index 5b888fb..132b5ff 100644 --- a/Module/PSInfisicalAPI/en-US/PSInfisicalAPI.dll-Help.xml +++ b/Module/PSInfisicalAPI/en-US/PSInfisicalAPI.dll-Help.xml @@ -1289,6 +1289,7 @@ $GetInfisicalCertificatePolicyResult = Get-InfisicalCertificatePolicy @GetInfisi Installing a root into CurrentUser\Root makes Windows display a modal trust confirmation dialog, and the call blocks until it is answered; in a non-interactive session this looks like a hang. The cmdlet emits a warning before blocking. Run elevated or pass -StoreLocation LocalMachine to install machine-wide without a prompt. Only the leaf honours -StoreName (default My). Chain members are routed by what they are: a self-signed certificate is a root and goes to the trusted-root store, anything with an issuer above it is a subordinate CA and goes to the intermediate store, for a chain of any depth. Issuers are installed before the leaf, and the chain is then validated against the machine's stores; an incomplete chain is reported as a warning naming the missing issuer, which is the condition Windows shows as "The issuer of this certificate could not be found". The installed certificate's Windows friendly name defaults to the common name in upper case. -FriendlyName overrides it and is accepted on every parameter set; on the -CertificateAuthorityId path the same value is additionally sent to Infisical as the issued certificate's friendlyName. + The reuse check is scoped to the issuer being requested: the search is filtered by -CertificateProfileId or -CertificateAuthorityId, so a certificate issued by a different profile is not reused. This matters when two profiles over one CA differ in key usage, such as server authentication versus client authentication, where a common-name match alone would return a certificate with the wrong extended key usages. Reuse additionally requires the existing certificate to carry every requested subject alternative name, so adding an entry to -DnsName or -IpAddress issues a new certificate instead of returning one that would fail validation for the new name. The rule is coverage rather than equality: a certificate carrying more names than requested still qualifies, DNS names compare case-insensitively, and IP addresses are normalized so ::1 matches 0:0:0:0:0:0:0:1. Use -Force when the SAN set needs trimming rather than extending. When Infisical cannot be reached the check falls back to matching on the common name alone and says so with a warning. diff --git a/README.md b/README.md index 59f8011..2448a76 100644 --- a/README.md +++ b/README.md @@ -309,6 +309,29 @@ Windows will report "The issuer of this certificate could not be found" until th The installed certificate's Windows friendly name defaults to the common name in upper case (`WEB01`), which is what shows in `certmgr`. Pass `-FriendlyName` on any parameter set to override it; on the `-CertificateAuthorityId` path the same value is also forwarded to Infisical as the issued certificate's `friendlyName`. +### Reuse and renewal + +A second run does not issue a new certificate if a still-valid one is already installed. That check is **scoped to the issuer you asked for**, not just the common name: the reuse search is filtered by `-CertificateProfileId` or `-CertificateAuthorityId`, so switching profiles issues a new certificate rather than handing back the old one. + +This matters when two profiles over the same CA differ in key usage. Requesting from a client-authentication profile on a host that already holds a server-authentication certificate for the same name issues a new certificate, because a name match alone would return one with the wrong EKUs: + +```text +VERBOSE: Reuse search for CN=WEB01 scoped to certificate profile 'a42f8446-...' returned 0 active certificate(s). +``` + +Reuse also requires the existing certificate to carry **every** name being requested. Adding an entry to `-DnsName` and re-running issues a new certificate rather than returning one that would fail validation for the name you just added: + +```text +VERBOSE: An existing certificate for CN=WEB01 does not carry the requested name DNS:api.contoso.com; + requesting a new certificate rather than reusing one that would fail validation for it. +``` + +The rule is coverage, not equality — a certificate carrying more names than requested still satisfies the request. DNS names compare case-insensitively and IP addresses are normalized, so `::1` matches `0:0:0:0:0:0:0:1`. Removing a name from the request therefore reuses the existing certificate; use `-Force` when you need the SAN set trimmed rather than extended. + +`-Force` issues unconditionally, and `-AllowRenewal` with `-RenewalThresholdDays` rotates a certificate that is inside its renewal window. + +If Infisical cannot be reached, the reuse check cannot confirm which certificates belong to which issuer and falls back to matching on the common name alone. That is announced as a warning, since it can return a certificate from a different profile. + When the resolved location is `LocalMachine` and `-KeyStorageFlags` was not supplied, the private key is written to the machine key store. Without that the key lands in the calling user's profile while the certificate sits in `LocalMachine\My`, which is the usual cause of an installed certificate that reports no usable private key to a service. > **Non-elevated root installs prompt.** Adding a root to `CurrentUser\Root` makes Windows raise a modal trust dialog, and the call blocks until it is answered — if the dialog is hidden or the session is non-interactive (a scheduled task, an MECM task sequence), the cmdlet appears to hang indefinitely. It warns before blocking. Run elevated, or pass `-StoreLocation LocalMachine`, to install machine-wide with no prompt. diff --git a/src/PSInfisicalAPI.Tests/CertificateReuseScopingTests.cs b/src/PSInfisicalAPI.Tests/CertificateReuseScopingTests.cs new file mode 100644 index 0000000..062e39f --- /dev/null +++ b/src/PSInfisicalAPI.Tests/CertificateReuseScopingTests.cs @@ -0,0 +1,116 @@ +using System; +using System.Reflection; +using PSInfisicalAPI.Pki; +using Xunit; + +namespace PSInfisicalAPI.Tests +{ + /// + /// Reuse must be scoped to the issuer being requested. Two profiles over the same CA issue certificates with + /// the same common name but different key usages (server authentication vs client authentication), so a + /// name-only match hands back a certificate that does not satisfy the request that was made. + /// + public class CertificateReuseScopingTests + { + private static readonly Assembly ModuleAssembly = typeof(PSInfisicalAPI.Connections.InfisicalConnection).Assembly; + + private static string InvokeApplyIssuerScope(PSInfisicalAPI.Cmdlets.RequestInfisicalCertificateCmdlet cmdlet, InfisicalCertificateSearchQuery query) + { + MethodInfo method = cmdlet.GetType().GetMethod("ApplyIssuerScope", BindingFlags.NonPublic | BindingFlags.Instance); + Assert.NotNull(method); + return (string)method.Invoke(cmdlet, new object[] { query }); + } + + [Fact] + public void Profile_Issuance_Scopes_The_Reuse_Search_To_That_Profile() + { + PSInfisicalAPI.Cmdlets.RequestInfisicalCertificateCmdlet cmdlet = new PSInfisicalAPI.Cmdlets.RequestInfisicalCertificateCmdlet + { + ProjectId = "proj-1", + CertificateProfileId = "profile-clientauth" + }; + + InfisicalCertificateSearchQuery query = new InfisicalCertificateSearchQuery(); + string scope = InvokeApplyIssuerScope(cmdlet, query); + + Assert.Equal(new[] { "profile-clientauth" }, query.ProfileIds); + Assert.Null(query.CaIds); + Assert.Contains("profile-clientauth", scope); + } + + [Fact] + public void Ca_Issuance_Scopes_The_Reuse_Search_To_That_Ca() + { + PSInfisicalAPI.Cmdlets.RequestInfisicalCertificateCmdlet cmdlet = new PSInfisicalAPI.Cmdlets.RequestInfisicalCertificateCmdlet + { + ProjectId = "proj-1", + CertificateAuthorityId = "ca-1" + }; + + InfisicalCertificateSearchQuery query = new InfisicalCertificateSearchQuery(); + string scope = InvokeApplyIssuerScope(cmdlet, query); + + Assert.Equal(new[] { "ca-1" }, query.CaIds); + Assert.Null(query.ProfileIds); + Assert.Contains("ca-1", scope); + } + + [Fact] + public void Subscriber_Issuance_Needs_No_Server_Side_Scope() + { + // A subscriber pins its own common name, so a name match is already a subscriber match. + PSInfisicalAPI.Cmdlets.RequestInfisicalCertificateCmdlet cmdlet = new PSInfisicalAPI.Cmdlets.RequestInfisicalCertificateCmdlet + { + ProjectId = "proj-1", + PkiSubscriberSlug = "web-tier" + }; + + InfisicalCertificateSearchQuery query = new InfisicalCertificateSearchQuery(); + string scope = InvokeApplyIssuerScope(cmdlet, query); + + Assert.Null(query.ProfileIds); + Assert.Null(query.CaIds); + Assert.Contains("web-tier", scope); + } + + [Fact] + public void Two_Profiles_Produce_Distinct_Reuse_Scopes() + { + InfisicalCertificateSearchQuery serverQuery = new InfisicalCertificateSearchQuery(); + InvokeApplyIssuerScope(new PSInfisicalAPI.Cmdlets.RequestInfisicalCertificateCmdlet { CertificateProfileId = "profile-serverauth" }, serverQuery); + + InfisicalCertificateSearchQuery clientQuery = new InfisicalCertificateSearchQuery(); + InvokeApplyIssuerScope(new PSInfisicalAPI.Cmdlets.RequestInfisicalCertificateCmdlet { CertificateProfileId = "profile-clientauth" }, clientQuery); + + Assert.NotEqual(serverQuery.ProfileIds[0], clientQuery.ProfileIds[0]); + } + + [Fact] + public void Issuer_Scope_Survives_Serialization_Into_The_Search_Request() + { + // The scope is only effective if it actually reaches the wire. + Type clientType = ModuleAssembly.GetType("PSInfisicalAPI.Pki.InfisicalPkiClient", true); + MethodInfo build = clientType.GetMethod("BuildSearchRequest", BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Public); + Assert.NotNull(build); + + InfisicalCertificateSearchQuery query = new InfisicalCertificateSearchQuery + { + ProjectId = "proj-1", + CommonName = "WEB01", + Status = "active", + ProfileIds = new[] { "profile-clientauth" } + }; + + object dto = build.Invoke(null, new object[] { query }); + Assert.NotNull(dto); + + PropertyInfo profileIds = dto.GetType().GetProperty("ProfileIds"); + Assert.NotNull(profileIds); + Assert.Equal(new[] { "profile-clientauth" }, (string[])profileIds.GetValue(dto)); + + string json = Newtonsoft.Json.JsonConvert.SerializeObject(dto); + Assert.Contains("profileIds", json); + Assert.Contains("profile-clientauth", json); + } + } +} diff --git a/src/PSInfisicalAPI.Tests/CertificateSanCoverageTests.cs b/src/PSInfisicalAPI.Tests/CertificateSanCoverageTests.cs new file mode 100644 index 0000000..35bae84 --- /dev/null +++ b/src/PSInfisicalAPI.Tests/CertificateSanCoverageTests.cs @@ -0,0 +1,220 @@ +using System; +using System.Collections.Generic; +using System.Reflection; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using PSInfisicalAPI.Pki; +using Xunit; + +namespace PSInfisicalAPI.Tests +{ + /// + /// Reuse must not return a certificate that predates a newly requested SAN. These build real certificates + /// through the module's own CSR path so the SAN reader is exercised against genuine DER, not a hand-rolled + /// approximation of it. + /// + public class CertificateSanCoverageTests + { + private static readonly Assembly ModuleAssembly = typeof(PSInfisicalAPI.Connections.InfisicalConnection).Assembly; + + /// + /// Produces a self-signed certificate carrying exactly the requested SANs, by round-tripping the module's + /// CSR builder output into a signed certificate. + /// + private static X509Certificate2 CreateCertificateWithSans(string commonName, string[] dnsNames, string[] ipAddresses) + { + InfisicalCsrSubject subject = new InfisicalCsrSubject { CommonName = commonName }; + InfisicalCsrResult csr = InfisicalCsrBuilder.Build(subject, dnsNames, ipAddresses, new InfisicalCsrOptions()); + + Org.BouncyCastle.Pkcs.Pkcs10CertificationRequest request; + using (System.IO.StringReader reader = new System.IO.StringReader(csr.CsrPem)) + { + Org.BouncyCastle.OpenSsl.PemReader pemReader = new Org.BouncyCastle.OpenSsl.PemReader(reader); + request = (Org.BouncyCastle.Pkcs.Pkcs10CertificationRequest)pemReader.ReadObject(); + } + + Org.BouncyCastle.Asn1.Pkcs.CertificationRequestInfo info = request.GetCertificationRequestInfo(); + Org.BouncyCastle.Asn1.X509.X509Extensions extensions = null; + foreach (Org.BouncyCastle.Asn1.Asn1Encodable attributeEncodable in info.Attributes) + { + Org.BouncyCastle.Asn1.Cms.Attribute attribute = Org.BouncyCastle.Asn1.Cms.Attribute.GetInstance(attributeEncodable); + if (attribute.AttrType.Equals(Org.BouncyCastle.Asn1.Pkcs.PkcsObjectIdentifiers.Pkcs9AtExtensionRequest)) + { + extensions = Org.BouncyCastle.Asn1.X509.X509Extensions.GetInstance(attribute.AttrValues[0]); + } + } + + Assert.NotNull(extensions); + Org.BouncyCastle.Asn1.X509.X509Extension sanExtension = + extensions.GetExtension(Org.BouncyCastle.Asn1.X509.X509Extensions.SubjectAlternativeName); + Assert.NotNull(sanExtension); + + using (RSA rsa = RSA.Create(2048)) + { + CertificateRequest netRequest = new CertificateRequest( + string.Concat("CN=", commonName), rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + + netRequest.CertificateExtensions.Add(new X509Extension( + new Oid("2.5.29.17"), + sanExtension.Value.GetOctets(), + false)); + + return netRequest.CreateSelfSigned(DateTimeOffset.UtcNow.AddMinutes(-5), DateTimeOffset.UtcNow.AddDays(30)); + } + } + + private static bool CoversRequestedNames(X509Certificate2 cert, string[] dns, string[] ips, out string missing) + { + Type reader = ModuleAssembly.GetType("PSInfisicalAPI.Pki.InfisicalCertificateSanReader", true); + MethodInfo method = reader.GetMethod("CoversRequestedNames", BindingFlags.Public | BindingFlags.Static); + Assert.NotNull(method); + + object[] args = new object[] { cert, dns, ips, null }; + bool result = (bool)method.Invoke(null, args); + missing = (string)args[3]; + return result; + } + + private static (HashSet Dns, HashSet Ips) ReadSans(X509Certificate2 cert) + { + Type reader = ModuleAssembly.GetType("PSInfisicalAPI.Pki.InfisicalCertificateSanReader", true); + MethodInfo read = reader.GetMethod("Read", BindingFlags.Public | BindingFlags.Static); + object sans = read.Invoke(null, new object[] { cert }); + + HashSet dns = (HashSet)sans.GetType().GetProperty("DnsNames").GetValue(sans); + HashSet ips = (HashSet)sans.GetType().GetProperty("IpAddresses").GetValue(sans); + return (dns, ips); + } + + [Fact] + public void Reader_Recovers_Both_Dns_And_Ip_Sans() + { + using (X509Certificate2 cert = CreateCertificateWithSans( + "WEB01", + new[] { "WEB01", "WEB01.contoso.com" }, + new[] { "10.20.30.40", "127.0.0.1", "::1" })) + { + (HashSet dns, HashSet ips) = ReadSans(cert); + + Assert.Equal(2, dns.Count); + Assert.Contains("WEB01", dns); + Assert.Contains("WEB01.contoso.com", dns); + + Assert.Equal(3, ips.Count); + Assert.Contains("10.20.30.40", ips); + Assert.Contains("127.0.0.1", ips); + Assert.Contains("::1", ips); + } + } + + [Fact] + public void A_Certificate_Covering_Every_Requested_Name_Is_Reusable() + { + using (X509Certificate2 cert = CreateCertificateWithSans( + "WEB01", new[] { "WEB01", "WEB01.contoso.com" }, new[] { "10.20.30.40" })) + { + string missing; + Assert.True(CoversRequestedNames(cert, new[] { "WEB01", "WEB01.contoso.com" }, new[] { "10.20.30.40" }, out missing)); + Assert.Null(missing); + } + } + + [Fact] + public void A_Newly_Requested_Dns_Name_Disqualifies_The_Existing_Certificate() + { + // The reported gap: adding a name to -DnsName previously returned the old certificate. + using (X509Certificate2 cert = CreateCertificateWithSans( + "WEB01", new[] { "WEB01", "WEB01.contoso.com" }, null)) + { + string missing; + bool covers = CoversRequestedNames( + cert, + new[] { "WEB01", "WEB01.contoso.com", "api.contoso.com" }, + null, + out missing); + + Assert.False(covers); + Assert.Equal("DNS:api.contoso.com", missing); + } + } + + [Fact] + public void A_Newly_Requested_Ip_Disqualifies_The_Existing_Certificate() + { + using (X509Certificate2 cert = CreateCertificateWithSans("WEB01", new[] { "WEB01" }, new[] { "10.20.30.40" })) + { + string missing; + Assert.False(CoversRequestedNames(cert, new[] { "WEB01" }, new[] { "10.20.30.41" }, out missing)); + Assert.Equal("IP:10.20.30.41", missing); + } + } + + [Fact] + public void Extra_Names_On_The_Certificate_Do_Not_Disqualify_It() + { + // A superset still satisfies the request; only a missing name forces reissuance. + using (X509Certificate2 cert = CreateCertificateWithSans( + "WEB01", new[] { "WEB01", "WEB01.contoso.com", "legacy.contoso.com" }, new[] { "10.20.30.40", "127.0.0.1" })) + { + string missing; + Assert.True(CoversRequestedNames(cert, new[] { "WEB01" }, new[] { "127.0.0.1" }, out missing)); + Assert.Null(missing); + } + } + + [Fact] + public void Dns_Comparison_Is_Case_Insensitive() + { + using (X509Certificate2 cert = CreateCertificateWithSans("WEB01", new[] { "WEB01.Contoso.COM" }, null)) + { + string missing; + Assert.True(CoversRequestedNames(cert, new[] { "web01.contoso.com" }, null, out missing)); + } + } + + [Theory] + [InlineData("::1", "0:0:0:0:0:0:0:1")] + [InlineData("0:0:0:0:0:0:0:1", "::1")] + [InlineData("10.20.30.40", "10.20.30.40")] + public void Ip_Comparison_Normalizes_Textual_Variations(string inCertificate, string requested) + { + using (X509Certificate2 cert = CreateCertificateWithSans("WEB01", new[] { "WEB01" }, new[] { inCertificate })) + { + string missing; + Assert.True(CoversRequestedNames(cert, null, new[] { requested }, out missing), string.Concat("missing: ", missing)); + } + } + + [Fact] + public void A_Certificate_Without_Any_San_Extension_Fails_A_San_Request() + { + using (RSA rsa = RSA.Create(2048)) + { + CertificateRequest request = new CertificateRequest( + "CN=NoSans", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + using (X509Certificate2 cert = request.CreateSelfSigned(DateTimeOffset.UtcNow.AddMinutes(-5), DateTimeOffset.UtcNow.AddDays(1))) + { + string missing; + Assert.False(CoversRequestedNames(cert, new[] { "NoSans" }, null, out missing)); + Assert.Equal("DNS:NoSans", missing); + + // With nothing requested there is nothing to fail on. + Assert.True(CoversRequestedNames(cert, null, null, out missing)); + } + } + } + + [Fact] + public void FindMatch_Keeps_Its_Original_Signature_For_Callers_Without_San_Requirements() + { + Type lookup = ModuleAssembly.GetType("PSInfisicalAPI.Pki.InfisicalLocalCertificateLookup", true); + MethodInfo original = lookup.GetMethod( + "FindMatch", + BindingFlags.Public | BindingFlags.Static, + null, + new[] { typeof(StoreName), typeof(StoreLocation), typeof(string), typeof(IEnumerable) }, + null); + Assert.NotNull(original); + } + } +} diff --git a/src/PSInfisicalAPI/Cmdlets/RequestInfisicalCertificateCmdlet.cs b/src/PSInfisicalAPI/Cmdlets/RequestInfisicalCertificateCmdlet.cs index 48320c1..5e37785 100644 --- a/src/PSInfisicalAPI/Cmdlets/RequestInfisicalCertificateCmdlet.cs +++ b/src/PSInfisicalAPI/Cmdlets/RequestInfisicalCertificateCmdlet.cs @@ -90,7 +90,7 @@ namespace PSInfisicalAPI.Cmdlets if (string.IsNullOrEmpty(csrSubject.CommonName) && dnsNames.Count > 0) { csrSubject.CommonName = dnsNames[0]; } if (string.IsNullOrEmpty(csrSubject.CommonName)) { throw new InvalidOperationException("Subject CommonName could not be determined and no DnsName was provided."); } - X509Certificate2 existing = TryFindExisting(client, connection, ProjectId, csrSubject.CommonName, resolvedStoreLocation); + X509Certificate2 existing = TryFindExisting(client, connection, ProjectId, csrSubject.CommonName, resolvedStoreLocation, dnsNames, ipAddresses); if (existing != null && !Force.IsPresent && !(AllowRenewal.IsPresent && InfisicalLocalCertificateLookup.IsRenewable(existing, RenewalThresholdDays))) { Logger.Information(Component, string.Concat("Reusing existing certificate (Thumbprint=", existing.Thumbprint, ", NotAfter=", existing.NotAfter.ToString("u"), ").")); @@ -307,17 +307,39 @@ namespace PSInfisicalAPI.Cmdlets return System.Net.IPAddress.TryParse(value, out parsed); } - private X509Certificate2 TryFindExisting(InfisicalPkiClient client, InfisicalConnection connection, string projectId, string commonName, StoreLocation storeLocation) + /// + /// Finds a still-valid local certificate that this same request would have produced. The match is scoped + /// to the issuer being asked for: a certificate issued by a different profile or CA carries different key + /// usages and policy, so reusing one across issuers hands back a certificate that does not satisfy the + /// request that was actually made. + /// + private X509Certificate2 TryFindExisting(InfisicalPkiClient client, InfisicalConnection connection, string projectId, string commonName, StoreLocation storeLocation, List requestedDnsNames, List requestedIpAddresses) { List candidateSerials = new List(); + bool searchCompleted = false; + try { - InfisicalCertificateSearchQuery query = new InfisicalCertificateSearchQuery { ProjectId = projectId, CommonName = commonName, Status = "active", Limit = 50 }; + InfisicalCertificateSearchQuery query = new InfisicalCertificateSearchQuery + { + ProjectId = projectId, + CommonName = commonName, + Status = "active", + Limit = 50 + }; + + string scope = ApplyIssuerScope(query); + InfisicalCertificateSearchResult page = client.SearchCertificates(connection, query); + searchCompleted = true; if (page != null && page.Certificates != null) { foreach (InfisicalCertificate hit in page.Certificates) { if (!string.IsNullOrEmpty(hit.SerialNumber)) { candidateSerials.Add(hit.SerialNumber); } } } + + Logger.Verbose(Component, string.Concat( + "Reuse search for CN=", commonName, " scoped to ", scope, " returned ", + candidateSerials.Count.ToString(System.Globalization.CultureInfo.InvariantCulture), " active certificate(s).")); } catch (Exception searchException) { @@ -325,7 +347,61 @@ namespace PSInfisicalAPI.Cmdlets Logger.Verbose(Component, string.Concat("Infisical search for idempotency check failed: ", searchException.Message)); } - return InfisicalLocalCertificateLookup.FindMatch(StoreName, storeLocation, commonName, candidateSerials); + // A completed search that found nothing is a definite answer: this issuer has never issued for this + // common name, so there is nothing to reuse. Falling through to a name-only local match here is what + // let a certificate from another profile be handed back. + if (searchCompleted && candidateSerials.Count == 0) + { + return null; + } + + if (!searchCompleted) + { + Logger.Warning(Component, string.Concat( + "Could not confirm with Infisical which certificates belong to this issuer, so reuse falls back to ", + "matching on the common name alone. That can return a certificate issued by a different profile or CA; ", + "pass -Force to issue unconditionally.")); + } + + string missingName; + X509Certificate2 match = InfisicalLocalCertificateLookup.FindMatch( + StoreName, storeLocation, commonName, candidateSerials, requestedDnsNames, requestedIpAddresses, out missingName); + + if (match == null && missingName != null) + { + Logger.Information(Component, string.Concat( + "An existing certificate for CN=", commonName, " does not carry the requested name ", missingName, + "; requesting a new certificate rather than reusing one that would fail validation for it.")); + } + + return match; + } + + /// + /// Narrows a certificate search to the issuer this invocation targets, and names that scope for logging. + /// The subscriber path has no server-side filter, but a subscriber pins its own common name, so matching + /// on the name is already equivalent to matching on the subscriber. + /// + private string ApplyIssuerScope(InfisicalCertificateSearchQuery query) + { + if (!string.IsNullOrEmpty(CertificateProfileId)) + { + query.ProfileIds = new[] { CertificateProfileId }; + return string.Concat("certificate profile '", CertificateProfileId, "'"); + } + + if (!string.IsNullOrEmpty(CertificateAuthorityId)) + { + query.CaIds = new[] { CertificateAuthorityId }; + return string.Concat("certificate authority '", CertificateAuthorityId, "'"); + } + + if (!string.IsNullOrEmpty(PkiSubscriberSlug)) + { + return string.Concat("PKI subscriber '", PkiSubscriberSlug, "'"); + } + + return "this project"; } private X509KeyStorageFlags ResolveEffectiveKeyStorageFlags(StoreLocation storeLocation) diff --git a/src/PSInfisicalAPI/Pki/InfisicalCertificateSanReader.cs b/src/PSInfisicalAPI/Pki/InfisicalCertificateSanReader.cs new file mode 100644 index 0000000..29be1d2 --- /dev/null +++ b/src/PSInfisicalAPI/Pki/InfisicalCertificateSanReader.cs @@ -0,0 +1,164 @@ +using System; +using System.Collections.Generic; +using System.Net; +using System.Security.Cryptography.X509Certificates; +using Org.BouncyCastle.Asn1; +using Org.BouncyCastle.Asn1.X509; +using NetX509Extension = System.Security.Cryptography.X509Certificates.X509Extension; + +namespace PSInfisicalAPI.Pki +{ + /// + /// The subject alternative names carried by a certificate, split the way a request specifies them. + /// + internal sealed class InfisicalCertificateSans + { + public HashSet DnsNames { get; } = new HashSet(StringComparer.OrdinalIgnoreCase); + public HashSet IpAddresses { get; } = new HashSet(StringComparer.OrdinalIgnoreCase); + } + + /// + /// Reads the subject alternative name extension from an installed certificate. + /// + /// netstandard2.0 has no X509SubjectAlternativeNameExtension, and the string form produced by + /// X509Extension.Format is localized and therefore unusable for comparison, so the extension is decoded from + /// its DER bytes with BouncyCastle, which the module already carries for CSR generation. + /// + /// + internal static class InfisicalCertificateSanReader + { + private const string SubjectAlternativeNameOid = "2.5.29.17"; + + public static InfisicalCertificateSans Read(X509Certificate2 cert) + { + InfisicalCertificateSans result = new InfisicalCertificateSans(); + if (cert == null) { return result; } + + foreach (NetX509Extension extension in cert.Extensions) + { + if (extension == null || extension.Oid == null) { continue; } + if (!string.Equals(extension.Oid.Value, SubjectAlternativeNameOid, StringComparison.Ordinal)) { continue; } + + try + { + // X509Extension.RawData is the content of the extnValue OCTET STRING, so it decodes straight + // into the GeneralNames SEQUENCE. + Asn1Object decoded = Asn1Object.FromByteArray(extension.RawData); + GeneralNames names = GeneralNames.GetInstance(decoded); + if (names == null) { continue; } + + foreach (GeneralName name in names.GetNames()) + { + if (name == null) { continue; } + AddName(result, name); + } + } + catch (Exception) + { + // A certificate this malformed cannot be matched against a request; treat it as carrying no + // usable SANs rather than failing the caller's issuance. + } + } + + return result; + } + + private static void AddName(InfisicalCertificateSans target, GeneralName name) + { + switch (name.TagNo) + { + case GeneralName.DnsName: + { + string value = name.Name != null ? name.Name.ToString() : null; + if (!string.IsNullOrEmpty(value)) { target.DnsNames.Add(value.Trim()); } + break; + } + case GeneralName.IPAddress: + { + string value = FormatIpAddress(name); + if (!string.IsNullOrEmpty(value)) { target.IpAddresses.Add(value); } + break; + } + } + } + + /// + /// An iPAddress general name holds raw address octets, four for IPv4 and sixteen for IPv6. + /// + private static string FormatIpAddress(GeneralName name) + { + try + { + Asn1OctetString octets = Asn1OctetString.GetInstance(name.Name); + if (octets == null) { return null; } + + byte[] bytes = octets.GetOctets(); + if (bytes == null) { return null; } + if (bytes.Length != 4 && bytes.Length != 16) { return null; } + + return NormalizeIpAddress(new IPAddress(bytes).ToString()); + } + catch (Exception) + { + return null; + } + } + + /// + /// Collapses the textual variations of one address so "::1" and "0:0:0:0:0:0:0:1" compare equal. + /// + public static string NormalizeIpAddress(string value) + { + if (string.IsNullOrEmpty(value)) { return value; } + + IPAddress parsed; + if (IPAddress.TryParse(value.Trim(), out parsed)) + { + return parsed.ToString(); + } + + return value.Trim(); + } + + /// + /// Reports whether a candidate certificate carries every name the caller asked for. A certificate with + /// extra names still satisfies the request; one missing a requested name does not, and reusing it would + /// hand back a certificate that fails validation for the name that was added. + /// + public static bool CoversRequestedNames(X509Certificate2 candidate, IEnumerable dnsNames, IEnumerable ipAddresses, out string missingName) + { + missingName = null; + if (candidate == null) { return false; } + + InfisicalCertificateSans present = Read(candidate); + + if (dnsNames != null) + { + foreach (string dns in dnsNames) + { + if (string.IsNullOrEmpty(dns)) { continue; } + if (!present.DnsNames.Contains(dns.Trim())) + { + missingName = string.Concat("DNS:", dns.Trim()); + return false; + } + } + } + + if (ipAddresses != null) + { + foreach (string ip in ipAddresses) + { + if (string.IsNullOrEmpty(ip)) { continue; } + if (!present.IpAddresses.Contains(NormalizeIpAddress(ip))) + { + missingName = string.Concat("IP:", NormalizeIpAddress(ip)); + return false; + } + } + } + + return true; + } + } +} diff --git a/src/PSInfisicalAPI/Pki/InfisicalLocalCertificateLookup.cs b/src/PSInfisicalAPI/Pki/InfisicalLocalCertificateLookup.cs index 366ed98..8d4b791 100644 --- a/src/PSInfisicalAPI/Pki/InfisicalLocalCertificateLookup.cs +++ b/src/PSInfisicalAPI/Pki/InfisicalLocalCertificateLookup.cs @@ -8,9 +8,36 @@ namespace PSInfisicalAPI.Pki { public static X509Certificate2 FindMatch(StoreName storeName, StoreLocation storeLocation, string commonName, IEnumerable candidateSerialNumbers) { + string ignored; + return FindMatch(storeName, storeLocation, commonName, candidateSerialNumbers, null, null, out ignored); + } + + /// + /// Finds the longest-lived installed certificate for a subject that also carries every requested subject + /// alternative name. A certificate that predates a newly added SAN would fail validation for that name, + /// so it is not a reusable answer to the current request. + /// + /// + /// The first name that disqualified an otherwise-matching certificate, so the caller can explain why it + /// is reissuing rather than reusing. + /// + public static X509Certificate2 FindMatch( + StoreName storeName, + StoreLocation storeLocation, + string commonName, + IEnumerable candidateSerialNumbers, + IEnumerable requiredDnsNames, + IEnumerable requiredIpAddresses, + out string rejectedForMissingName) + { + rejectedForMissingName = null; HashSet serialSet = NormalizeSerials(candidateSerialNumbers); string subjectFilter = !string.IsNullOrEmpty(commonName) ? string.Concat("CN=", commonName) : null; + List dnsList = ToList(requiredDnsNames); + List ipList = ToList(requiredIpAddresses); + bool requireSans = dnsList.Count > 0 || ipList.Count > 0; + X509Store store = new X509Store(storeName, storeLocation); try { @@ -33,12 +60,24 @@ namespace PSInfisicalAPI.Pki } } + if (requireSans) + { + string missingName; + if (!InfisicalCertificateSanReader.CoversRequestedNames(candidate, dnsList, ipList, out missingName)) + { + if (rejectedForMissingName == null) { rejectedForMissingName = missingName; } + continue; + } + } + if (bestMatch == null || candidate.NotAfter > bestMatch.NotAfter) { bestMatch = candidate; } } + // Only report a rejection when nothing else qualified; a covering certificate makes it irrelevant. + if (bestMatch != null) { rejectedForMissingName = null; } return bestMatch; } finally @@ -47,6 +86,18 @@ namespace PSInfisicalAPI.Pki } } + private static List ToList(IEnumerable values) + { + List result = new List(); + if (values == null) { return result; } + foreach (string value in values) + { + if (!string.IsNullOrEmpty(value)) { result.Add(value); } + } + + return result; + } + public static bool IsRenewable(X509Certificate2 cert, int renewalThresholdDays) { if (cert == null) { return true; }