diff --git a/CERTInext.IntegrationTests/SanSubmissionProbeTests.cs b/CERTInext.IntegrationTests/SanSubmissionProbeTests.cs new file mode 100644 index 0000000..23681d1 --- /dev/null +++ b/CERTInext.IntegrationTests/SanSubmissionProbeTests.cs @@ -0,0 +1,391 @@ +// Copyright 2026 Keyfactor +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. +// At http://www.apache.org/licenses/LICENSE-2.0 +// +// Probe: establish empirically how CERTInext treats the SAN/domain fields on +// GenerateOrderSSL. Written because the plugin's original behaviour encoded three +// assumptions that were never measured: +// +// A. certificateInformation.additionalDomains is the field that puts extra names on +// the certificate (so a UCC order that omits it yields a CN-only certificate). +// B. additionalDomains accepts DNS names only, so a non-DNS SAN is rejected by the CA. +// C. Repeating the primary domainName inside additionalDomains is harmful (duplicate +// domain / consumes the UCC allowance), so it should be de-duplicated. +// +// None of these had a test. This probe answers them against the live API by placing one +// order per variant and reading back the domain set CERTInext actually registered, via +// TrackOrder's domainVerification block (keys are the domains on the order). That is +// ground truth for "which names did the CA put on this order" without waiting for DCV +// and issuance to complete. +// +// --------------------------------------------------------------------------------------- +// MEASURED RESULTS — SANDBOX ONLY: sandbox-us, account 4951571271, product 844 (OV SSL UCC), +// 2026-08-12. (Product 840 / DV UCC is not enabled on that account: "Invalid Product Code".) +// +// These are sandbox observations. Re-run against production before treating B or C as +// settled there — point ~/.env_certinext at the production account and set +// CERTINEXT_SAN_PROBE_PRODUCTS to a UCC code that account can actually order (product +// numbering is per-account; the codes in Constants.Products are defaults, not guarantees). +// Finding A and the CSR-SAN result below are separately corroborated by production: the +// customer report that prompted this work was a production UCC order whose CSR carried the +// SANs and whose issued certificate held only the CN. +// +// A. CONFIRMED. additionalDomains is what puts extra names on the order. Submitting +// CN + extra1. registered BOTH domains. +// +// B. DISPROVEN. Non-DNS values are NOT rejected. An email address, an IPv4 literal and +// an https:// URI were each accepted at placement AND registered as order domains +// ("san-probe@example.com", "192.0.2.10", "https://san-probe.example.com/x" all came +// back as domainVerification keys). So the CA does not validate the field's contents +// at order time; such an order is created and then cannot pass DCV, rather than +// failing cleanly up front. +// +// C. PARTLY DISPROVEN. Repeating the primary domainName inside additionalDomains is +// accepted and CERTInext collapses it itself — the order came back with the CN +// registered once. De-duplicating on our side is therefore belt-and-braces, not a +// correctness requirement. +// +// Root cause of the customer-reported "UCC SANs not populating": CERTInext IGNORES the +// subjectAltName extension in the CSR. A CSR carrying CN + extra2., submitted with +// additionalDomains omitted, registered ONLY the CN. SANs must be sent in +// additionalDomains or they do not reach the certificate, no matter what the CSR says. +// --------------------------------------------------------------------------------------- +// +// Opt-in: this places real orders against whatever account ~/.env_certinext points at. +// +// set -a; . ~/.env_certinext; set +a +// export CERTINEXT_SAN_PROBE=1 +// dotnet test CERTInext.IntegrationTests/CERTInext.IntegrationTests.csproj -c Release \ +// --filter "FullyQualifiedName~SanSubmissionProbeTests" \ +// --logger "console;verbosity=detailed" > /tmp/sanprobe.log 2>&1 +// +// (xUnit buffers ITestOutputHelper output until the test ends — read the report at the tail.) + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Keyfactor.Extensions.CAPlugin.CERTInext.API; +using Keyfactor.Extensions.CAPlugin.CERTInext.Client; +using Org.BouncyCastle.Asn1; +using Org.BouncyCastle.Asn1.Pkcs; +using Org.BouncyCastle.Asn1.X509; +using Org.BouncyCastle.Crypto; +using Org.BouncyCastle.Crypto.Generators; +using Org.BouncyCastle.Pkcs; +using Org.BouncyCastle.Security; +using Xunit; +using Xunit.Abstractions; + +namespace Keyfactor.Extensions.CAPlugin.CERTInext.IntegrationTests +{ + public class SanSubmissionProbeTests : IClassFixture + { + private const string OptInFlag = "CERTINEXT_SAN_PROBE"; + + /// + /// Comma-separated product codes to probe. Defaults to the Multi-Domain (UCC) codes, + /// because additional domains are only meaningful on a UCC product — a single-domain + /// product (e.g. 842 = OV SSL) registers the CN and nothing else no matter what + /// additionalDomains contains, which makes it useless as a probe target. + /// + private const string ProductCodesFlag = "CERTINEXT_SAN_PROBE_PRODUCTS"; + private const string DefaultProductCodes = "840,844"; + + private readonly IntegrationTestFixture _fixture; + private readonly ITestOutputHelper _out; + + public SanSubmissionProbeTests(IntegrationTestFixture fixture, ITestOutputHelper output) + { + _fixture = fixture; + _out = output; + } + + // ------------------------------------------------------------------------- + // CSR generation (BouncyCastle — project crypto policy) + // ------------------------------------------------------------------------- + + /// + /// Generates a PKCS#10 CSR for , optionally carrying a + /// subjectAltName extension (via the PKCS#9 extensionRequest attribute) holding + /// . The SAN-bearing form is what lets this probe ask + /// whether CERTInext reads SANs out of the CSR at all. + /// + private static string GenerateCsrPem(string cn, params string[] dnsSans) + { + var keyGen = new RsaKeyPairGenerator(); + keyGen.Init(new KeyGenerationParameters(new SecureRandom(), 2048)); + AsymmetricCipherKeyPair kp = keyGen.GenerateKeyPair(); + + Asn1Set attributes = null; + if (dnsSans != null && dnsSans.Length > 0) + { + var names = new GeneralNames( + dnsSans.Select(d => new GeneralName(GeneralName.DnsName, d)).ToArray()); + + var extGen = new X509ExtensionsGenerator(); + extGen.AddExtension(X509Extensions.SubjectAlternativeName, critical: false, extValue: names); + + attributes = new DerSet(new AttributePkcs( + PkcsObjectIdentifiers.Pkcs9AtExtensionRequest, + new DerSet(extGen.Generate()))); + } + + var csr = new Pkcs10CertificationRequest( + "SHA256withRSA", new X509Name($"CN={cn}"), kp.Public, attributes, kp.Private); + + return "-----BEGIN CERTIFICATE REQUEST-----\n" + + Convert.ToBase64String(csr.GetEncoded(), Base64FormattingOptions.InsertLineBreaks) + + "\n-----END CERTIFICATE REQUEST-----"; + } + + // ------------------------------------------------------------------------- + // One probe variant + // ------------------------------------------------------------------------- + + private sealed class ProbeOutcome + { + public string ProductCode; + public string Label; + public bool Accepted; + public string OrderNumber; + public string Detail; + /// Domains CERTInext registered on the order, per TrackOrder. + public List RegisteredDomains = new List(); + /// Names we asked CERTInext to put on the order, for comparison. + public List RequestedDomains = new List(); + + /// + /// True when the rejection was "Invalid Product Code" — the product simply is not + /// enabled on this account, which is not a data point about SAN handling. + /// + public bool ProductUnavailable; + } + + /// + /// Places one order and reads back the domain set CERTInext registered for it. + /// drives certificateInformation.additionalDomains; + /// drives the SAN extension inside the CSR. They are + /// varied independently on purpose — that separation is the whole point of the probe. + /// + private async Task ProbeAsync( + string productCode, + string label, + Func> sansFactory, + string[] csrSans) + { + var outcome = new ProbeOutcome { ProductCode = productCode, Label = label }; + + var client = new CERTInextClient(_fixture.Config); + string cn = $"sanprobe-{DateTime.UtcNow:yyyyMMddHHmmssfff}.{SafeLabel(label)}.example.com"; + + var sans = sansFactory?.Invoke(cn); + outcome.RequestedDomains = sans == null + ? new List() + : sans.Select(s => $"{s.Type}:{s.Value}").ToList(); + + var req = new EnrollCertificateRequest + { + Csr = GenerateCsrPem(cn, csrSans == null ? null : csrSans.Select(s => Format(s, cn)).ToArray()), + Subject = $"CN={cn}", + Sans = sans, + ProfileId = productCode, + RequesterName = _fixture.RequestorName, + RequesterEmail = _fixture.RequestorEmail + }; + + try + { + var resp = await client.EnrollCertificateAsync(req); + outcome.Accepted = true; + outcome.OrderNumber = resp?.Id; + outcome.Detail = $"OrderNumber={resp?.Id} Status={resp?.Status}"; + } + catch (Exception ex) + { + outcome.Accepted = false; + outcome.Detail = ex.Message; + outcome.ProductUnavailable = + ex.Message.IndexOf("Invalid Product Code", StringComparison.OrdinalIgnoreCase) >= 0; + return outcome; + } + + // Read back which domains the CA actually put on the order. + try + { + var track = await client.TrackOrderAsync(outcome.OrderNumber); + var entries = track.OrderDetails?.DomainVerification?.GetDomainEntries(); + if (entries != null) + outcome.RegisteredDomains = entries.Keys.OrderBy(k => k, StringComparer.OrdinalIgnoreCase).ToList(); + } + catch (Exception ex) + { + outcome.Detail += $" | TrackOrder failed: {ex.Message}"; + } + + return outcome; + } + + /// Substitutes the generated CN into a variant's placeholder template. + private static string Format(string template, string cn) => template.Replace("{cn}", cn); + + private static string SafeLabel(string label) => + new string(label.ToLowerInvariant().Select(c => char.IsLetterOrDigit(c) ? c : '-').ToArray()) + .Trim('-'); + + // ------------------------------------------------------------------------- + // The probe + // ------------------------------------------------------------------------- + + [SkippableFact] + public async Task Probe_SanSubmissionBehaviour() + { + IntegrationSkip.IfNotConfigured(_fixture); + Skip.IfNot( + Environment.GetEnvironmentVariable(OptInFlag) == "1", + $"Set {OptInFlag}=1 to run this probe — it places real orders on the configured account."); + + var variants = new List<(string Label, Func> Sans, string[] CsrSans)> + { + // 1. Assumption A, positive control: additionalDomains carries an extra DNS + // name. If the extra name comes back registered, additionalDomains works. + ("dns-extra-via-additionalDomains", + cn => new List + { + new SanEntry { Type = "dns", Value = cn }, + new SanEntry { Type = "dns", Value = $"extra1.{cn}" } + }, + new[] { "{cn}", "extra1.{cn}" }), + + // 2. Assumption A, the actual bug: CSR carries both names, additionalDomains + // is omitted entirely. This is what v1.0.1 sent for every UCC enrollment. + // If only the CN comes back registered, the CA does NOT read CSR SANs and + // the diagnosis is confirmed. + ("csr-sans-only-no-additionalDomains", + _ => null, + new[] { "{cn}", "extra2.{cn}" }), + + // 3. Assumption C: primary domainName repeated inside additionalDomains. + // Does the CA reject it, or silently collapse it? + ("cn-duplicated-in-additionalDomains", + cn => new List + { + new SanEntry { Type = "dns", Value = cn }, + new SanEntry { Type = "dns", Value = cn } + }, + new[] { "{cn}" }), + + // 4-6. Assumption B: non-DNS values in additionalDomains. Rejected, ignored, + // or accepted? Each is submitted alongside a valid DNS name so a rejection + // is attributable to the non-DNS value rather than an empty domain set. + ("nondns-email-in-additionalDomains", + cn => new List + { + new SanEntry { Type = "dns", Value = cn }, + new SanEntry { Type = "email", Value = "san-probe@example.com" } + }, + new[] { "{cn}" }), + + ("nondns-ip-in-additionalDomains", + cn => new List + { + new SanEntry { Type = "dns", Value = cn }, + new SanEntry { Type = "ip", Value = "192.0.2.10" } + }, + new[] { "{cn}" }), + + ("nondns-uri-in-additionalDomains", + cn => new List + { + new SanEntry { Type = "dns", Value = cn }, + new SanEntry { Type = "uri", Value = "https://san-probe.example.com/x" } + }, + new[] { "{cn}" }), + }; + + string[] productCodes = + (Environment.GetEnvironmentVariable(ProductCodesFlag) ?? DefaultProductCodes) + .Split(',', StringSplitOptions.RemoveEmptyEntries) + .Select(p => p.Trim()) + .Where(p => p.Length > 0) + .ToArray(); + + var results = new List(); + foreach (string productCode in productCodes) + { + bool unavailable = false; + foreach (var (label, sans, csrSans) in variants) + { + var outcome = await ProbeAsync(productCode, label, sans, csrSans); + results.Add(outcome); + + // Don't burn five more orders proving the same product code is not + // enabled on this account. + if (outcome.ProductUnavailable) + { + unavailable = true; + break; + } + + // Throttle: the sandbox rate-limits order bursts (~16 orders / 10 s). + await Task.Delay(1500); + } + + if (unavailable) + _out.WriteLine($"(product {productCode} is not enabled on this account — skipped)"); + } + + _out.WriteLine("=== CERTInext SAN submission probe ==="); + _out.WriteLine($"ProductCodes probed : {string.Join(", ", productCodes)}"); + _out.WriteLine($"(fixture default : {_fixture.ProductCode})"); + _out.WriteLine(""); + + foreach (var group in results.GroupBy(r => r.ProductCode)) + { + _out.WriteLine($"--- ProductCode {group.Key} ---"); + foreach (var r in group) + { + _out.WriteLine($"[{(r.Accepted ? "ACCEPTED" : "REJECTED")}] {r.Label}"); + _out.WriteLine($" requested (additionalDomains): {(r.RequestedDomains.Count > 0 ? string.Join(", ", r.RequestedDomains) : "(field omitted)")}"); + _out.WriteLine($" detail : {r.Detail}"); + _out.WriteLine($" registeredDomains (TrackOrder): {(r.RegisteredDomains.Count > 0 ? string.Join(", ", r.RegisteredDomains) : "(none reported)")}"); + _out.WriteLine(""); + } + } + + _out.WriteLine("=== How to read this ==="); + _out.WriteLine("registeredDomains is TrackOrder's domainVerification key set — the domains"); + _out.WriteLine("CERTInext put on the order. Compare it against 'requested':"); + _out.WriteLine("(1) vs (2): if (1) registers the extra name and (2) does not, then"); + _out.WriteLine(" additionalDomains is required and CSR SANs alone are ignored."); + _out.WriteLine("(3) : whether repeating the CN is rejected or collapsed."); + _out.WriteLine("(4)-(6) : whether non-DNS values are rejected, ignored, or accepted"); + _out.WriteLine(" AT PLACEMENT TIME. An order accepted here can still be"); + _out.WriteLine(" rejected later during validation/approval."); + + // The probe reports; it does not assert a specific CA behaviour, because its purpose + // is to discover what that behaviour is. What must hold is that at least one UCC + // product was actually exercised — otherwise the run proved nothing and should not + // read as a pass. + var usable = results + .Where(r => !r.ProductUnavailable) + .GroupBy(r => r.ProductCode) + .ToList(); + + Skip.If( + usable.Count == 0, + "None of the probed product codes are enabled on this account " + + $"({string.Join(", ", productCodes)}). Set {ProductCodesFlag} to a Multi-Domain (UCC) " + + "code this account can order."); + + foreach (var group in usable) + { + var control = group.First(r => r.Label == "dns-extra-via-additionalDomains"); + Assert.True( + control.Accepted, + $"Positive control failed on product {group.Key} — could not place even a " + + $"plain DNS UCC order: {control.Detail}"); + } + } + } +} diff --git a/CERTInext.Tests/SanSubmissionTests.cs b/CERTInext.Tests/SanSubmissionTests.cs new file mode 100644 index 0000000..84222f1 --- /dev/null +++ b/CERTInext.Tests/SanSubmissionTests.cs @@ -0,0 +1,443 @@ +// Copyright 2026 Keyfactor +// Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. +// You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions +// and limitations under the License. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using FluentAssertions; +using Keyfactor.AnyGateway.Extensions; +using Keyfactor.Extensions.CAPlugin.CERTInext.API; +using Keyfactor.Extensions.CAPlugin.CERTInext.Client; +using Moq; +using Org.BouncyCastle.Asn1; +using Org.BouncyCastle.Asn1.Pkcs; +using Org.BouncyCastle.Asn1.X509; +using Org.BouncyCastle.Crypto; +using Org.BouncyCastle.Crypto.Generators; +using Org.BouncyCastle.Pkcs; +using Org.BouncyCastle.Security; +using WireMock.RequestBuilders; +using WireMock.ResponseBuilders; +using WireMock.Server; +using Xunit; + +namespace Keyfactor.Extensions.CAPlugin.CERTInext.Tests +{ + /// + /// Regression tests for UCC SAN submission. + /// + /// The defect these pin down: the AnyCA REST Gateway keys its SAN dictionary + /// dnsname, but MapSanType only recognized dns. Every DNS SAN was + /// therefore typed "dnsname", filtered out by a DNS-only test when building + /// certificateInformation.additionalDomains, and the order reached CERTInext with + /// no additional domains at all — yielding a certificate holding only the CN. Because + /// CERTInext ignores the CSR's subjectAltName extension entirely (measured; see + /// SanSubmissionProbeTests), SANs present on the CSR did not compensate. + /// + /// The end-to-end tests below drive a real against WireMock + /// so they assert on the JSON actually put on the wire, not on an intermediate object. + /// A test that only checked the mapping function would not have caught this bug, since + /// the mapping "worked" — it was the interaction with the downstream filter that lost + /// the names. + /// + public class SanSubmissionTests : IDisposable + { + private readonly WireMockServer _server; + + public SanSubmissionTests() + { + _server = WireMockServer.Start(); + StubHappyEnroll(); + } + + public void Dispose() => _server.Stop(); + + // ----------------------------------------------------------------------- + // Harness + // ----------------------------------------------------------------------- + + private void StubHappyEnroll() + { + _server.Given(Request.Create().WithPath("/GenerateOrderSSL").UsingPost()) + .RespondWith(Response.Create().WithStatusCode(200) + .WithHeader("Content-Type", "application/json") + .WithBody(MockCertificateData.GenerateOrderSuccessJson(MockCertificateData.OrderNumber1))); + + _server.Given(Request.Create().WithPath("/TrackOrder").UsingPost()) + .RespondWith(Response.Create().WithStatusCode(200) + .WithHeader("Content-Type", "application/json") + .WithBody(MockCertificateData.TrackOrderIssuedJson(MockCertificateData.OrderNumber1))); + + _server.Given(Request.Create().WithPath("/GetCertificate").UsingPost()) + .RespondWith(Response.Create().WithStatusCode(200) + .WithHeader("Content-Type", "application/json") + .WithBody(MockCertificateData.GetCertificateSuccessJson())); + } + + private CERTInextClient BuildRealClient() => new CERTInextClient(new CERTInextConfig + { + ApiUrl = _server.Urls[0], + AuthMode = "AccessKey", + ApiKey = "test-key", + AccountNumber = "12345", + RequestorName = "Default Requestor", + RequestorEmail = "default@example.com", + RequestorIsdCode = "1", + RequestorMobileNumber = "5550000000", + SignerPlace = "Austin", + SignerIp = "203.0.113.10", + PageSize = 100 + }); + + /// + /// Plugin wired to a real client pointed at WireMock, so the assertions below run + /// against the JSON actually serialized onto the wire. + /// + private CERTInextCAPlugin BuildPlugin() => new CERTInextCAPlugin(BuildRealClient()); + + private static EnrollmentProductInfo MakeProductInfo(string profileId = "842") => + new EnrollmentProductInfo + { + ProductID = profileId, + ProductParameters = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["ProfileId"] = profileId + } + }; + + /// The orderDetails.certificateInformation block actually POSTed. + private JsonElement CapturedCertificateInformation() + { + var posts = _server.LogEntries + .Where(e => e.RequestMessage.Path == "/GenerateOrderSSL") + .ToList(); + posts.Should().HaveCount(1, "exactly one GenerateOrderSSL POST should have been emitted"); + + string body = posts[0].RequestMessage.Body; + body.Should().NotBeNullOrEmpty(); + + return JsonDocument.Parse(body!).RootElement + .GetProperty("orderDetails") + .GetProperty("certificateInformation"); + } + + private static List AdditionalDomains(JsonElement certificateInformation) => + certificateInformation.TryGetProperty("additionalDomains", out var el) + ? el.EnumerateArray().Select(x => x.GetString()).ToList() + : null; + + // ----------------------------------------------------------------------- + // CSR generation (BouncyCastle — project crypto policy) + // ----------------------------------------------------------------------- + + /// + /// Builds a real PKCS#10 CSR for , optionally carrying a + /// subjectAltName extension holding . + /// + private static string GenerateCsrPem(string cn, params string[] dnsSans) + { + var keyGen = new RsaKeyPairGenerator(); + keyGen.Init(new KeyGenerationParameters(new SecureRandom(), 2048)); + AsymmetricCipherKeyPair kp = keyGen.GenerateKeyPair(); + + Asn1Set attributes = null; + if (dnsSans != null && dnsSans.Length > 0) + { + var names = new GeneralNames( + dnsSans.Select(d => new GeneralName(GeneralName.DnsName, d)).ToArray()); + + var extGen = new X509ExtensionsGenerator(); + extGen.AddExtension(X509Extensions.SubjectAlternativeName, critical: false, extValue: names); + + attributes = new DerSet(new AttributePkcs( + PkcsObjectIdentifiers.Pkcs9AtExtensionRequest, + new DerSet(extGen.Generate()))); + } + + var csr = new Pkcs10CertificationRequest( + "SHA256withRSA", new X509Name($"CN={cn}"), kp.Public, attributes, kp.Private); + + return "-----BEGIN CERTIFICATE REQUEST-----\n" + + Convert.ToBase64String(csr.GetEncoded(), Base64FormattingOptions.InsertLineBreaks) + + "\n-----END CERTIFICATE REQUEST-----"; + } + + // ======================================================================= + // End-to-end: Command's SAN dictionary → the JSON on the wire + // ======================================================================= + + /// + /// THE regression test. "dnsname" is the key the real gateway sends — verified against + /// a customer gateway log: + /// SANs=dnsname:CLAUDIOTEST20.ucsd.edu; dnsname:CLAUDIOTEST20.ad.ucsd.edu + /// Before the fix, additionalDomains was absent from the body entirely. + /// + [Fact] + public async Task GatewayDnsNameKey_ReachesAdditionalDomains() + { + var plugin = BuildPlugin(); + + await plugin.Enroll( + csr: GenerateCsrPem("host.example.com"), + subject: "CN=host.example.com", + san: new Dictionary + { + ["dnsname"] = new[] { "host.example.com", "alt.example.com" } + }, + productInfo: MakeProductInfo(), + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.New); + + var certInfo = CapturedCertificateInformation(); + certInfo.GetProperty("domainName").GetString().Should().Be("host.example.com"); + + AdditionalDomains(certInfo).Should().BeEquivalentTo(new[] { "alt.example.com" }, + "the extra SAN must reach additionalDomains, and the CN must not be repeated there"); + } + + /// + /// The short "dns" spelling must keep working — some callers and older hosts use it. + /// + [Fact] + public async Task ShortDnsKey_StillReachesAdditionalDomains() + { + var plugin = BuildPlugin(); + + await plugin.Enroll( + csr: GenerateCsrPem("host.example.com"), + subject: "CN=host.example.com", + san: new Dictionary + { + ["dns"] = new[] { "alt.example.com" } + }, + productInfo: MakeProductInfo(), + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.New); + + AdditionalDomains(CapturedCertificateInformation()) + .Should().BeEquivalentTo(new[] { "alt.example.com" }); + } + + /// + /// SANs present only on the CSR must still reach additionalDomains. CERTInext does not + /// read the CSR's SAN extension, so if we don't forward these the names never appear + /// on the certificate. + /// + [Fact] + public async Task CsrSans_ReachAdditionalDomains_WhenGatewaySuppliesNone() + { + var plugin = BuildPlugin(); + + await plugin.Enroll( + csr: GenerateCsrPem("host.example.com", "host.example.com", "fromcsr.example.com"), + subject: "CN=host.example.com", + san: null, + productInfo: MakeProductInfo(), + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.New); + + AdditionalDomains(CapturedCertificateInformation()) + .Should().BeEquivalentTo(new[] { "fromcsr.example.com" }); + } + + /// + /// Union, not either/or: names unique to each source survive and the overlap collapses. + /// + [Fact] + public async Task GatewayAndCsrSans_AreUnionedAndDeduplicated() + { + var plugin = BuildPlugin(); + + await plugin.Enroll( + csr: GenerateCsrPem("host.example.com", "shared.example.com", "csronly.example.com"), + subject: "CN=host.example.com", + san: new Dictionary + { + // "shared" appears in both sources, and in different case, to prove the + // de-duplication is case-insensitive. + ["dnsname"] = new[] { "SHARED.example.com", "gatewayonly.example.com" } + }, + productInfo: MakeProductInfo(), + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.New); + + var domains = AdditionalDomains(CapturedCertificateInformation()); + + domains.Should().Contain("gatewayonly.example.com"); + domains.Should().Contain("csronly.example.com"); + domains.Count(d => d.Equals("shared.example.com", StringComparison.OrdinalIgnoreCase)) + .Should().Be(1, "the name present in both sources must appear exactly once"); + } + + /// + /// The CN is already submitted as domainName; repeating it in additionalDomains is + /// suppressed. CERTInext collapses it anyway (measured), so this keeps the body matching + /// what we log rather than relying on undocumented CA-side behaviour. + /// + [Fact] + public async Task Cn_IsNotRepeatedInAdditionalDomains() + { + var plugin = BuildPlugin(); + + await plugin.Enroll( + csr: GenerateCsrPem("host.example.com", "host.example.com"), + subject: "CN=host.example.com", + san: new Dictionary + { + ["dnsname"] = new[] { "host.example.com" } + }, + productInfo: MakeProductInfo(), + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.New); + + var certInfo = CapturedCertificateInformation(); + certInfo.GetProperty("domainName").GetString().Should().Be("host.example.com"); + AdditionalDomains(certInfo).Should().BeNull( + "with the CN as the only SAN there is nothing left to send, so the field is omitted"); + } + + /// + /// Non-DNS SANs are submitted rather than silently discarded. CERTInext accepts them + /// verbatim (measured) and the resulting order cannot pass validation — a visible + /// failure, deliberately preferred over issuing a certificate that quietly lacks names + /// the subscriber requested. + /// + [Fact] + public async Task NonDnsSans_AreSubmitted_NotDropped() + { + var plugin = BuildPlugin(); + + await plugin.Enroll( + csr: GenerateCsrPem("host.example.com"), + subject: "CN=host.example.com", + san: new Dictionary + { + ["dnsname"] = new[] { "alt.example.com" }, + ["ipaddress"] = new[] { "192.0.2.10" }, + ["rfc822name"] = new[] { "admin@example.com" } + }, + productInfo: MakeProductInfo(), + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.New); + + AdditionalDomains(CapturedCertificateInformation()) + .Should().BeEquivalentTo(new[] { "alt.example.com", "192.0.2.10", "admin@example.com" }); + } + + /// + /// A CSR we cannot parse must not break enrollment — the gateway-supplied SANs still go. + /// FakeCsrPem is deliberately truncated, so this also guards the many existing + /// tests that pass it. + /// + [Fact] + public async Task UnparseableCsr_DoesNotBlockGatewaySuppliedSans() + { + var plugin = BuildPlugin(); + + await plugin.Enroll( + csr: MockCertificateData.FakeCsrPem, + subject: "CN=host.example.com", + san: new Dictionary + { + ["dnsname"] = new[] { "alt.example.com" } + }, + productInfo: MakeProductInfo(), + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.New); + + AdditionalDomains(CapturedCertificateInformation()) + .Should().BeEquivalentTo(new[] { "alt.example.com" }); + } + + /// + /// No SANs from either source → the field is omitted rather than emitted as null/empty. + /// + [Fact] + public async Task NoSansAnywhere_OmitsAdditionalDomains() + { + var plugin = BuildPlugin(); + + await plugin.Enroll( + csr: GenerateCsrPem("host.example.com"), + subject: "CN=host.example.com", + san: null, + productInfo: MakeProductInfo(), + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.New); + + AdditionalDomains(CapturedCertificateInformation()).Should().BeNull(); + } + + // ======================================================================= + // Renew path — previously submitted no SANs at all + // ======================================================================= + + /// + /// A renewal that goes through the CERTInext renew API must carry the same domain set + /// as a new enrollment, and must take its primary domain from the subject's CN rather + /// than from the prior order's requestor name. + /// + [Fact] + public async Task RenewalRequest_CarriesSubjectAndSans() + { + var clientMock = new Mock(MockBehavior.Loose); + RenewCertificateRequest captured = null; + + clientMock + .Setup(c => c.RenewCertificateAsync( + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Callback((_, req, __) => captured = req) + .ReturnsAsync(MockCertificateData.IssuedEnrollResponse()); + + var readerMock = new Mock(MockBehavior.Loose); + readerMock + .Setup(r => r.GetRequestIDBySerialNumber(It.IsAny())) + .ReturnsAsync("PRIOR-ORDER-1"); + + // The renewal-window decision reads expiry from the data reader, not from the CA. + // Put the prior cert 10 days out so it lands inside the 30-day window below and the + // renew API path is actually taken. + readerMock + .Setup(r => r.GetExpirationDateByRequestId(It.IsAny())) + .Returns(DateTime.UtcNow.AddDays(10)); + + var plugin = new CERTInextCAPlugin(clientMock.Object, readerMock.Object); + + var productInfo = MakeProductInfo(); + productInfo.ProductParameters["PriorCertSN"] = "AABBCCDDEEFF"; + productInfo.ProductParameters["RenewalWindowDays"] = "30"; + + await plugin.Enroll( + csr: GenerateCsrPem("host.example.com", "host.example.com", "alt.example.com"), + subject: "CN=host.example.com", + san: new Dictionary + { + ["dnsname"] = new[] { "host.example.com", "alt.example.com" } + }, + productInfo: productInfo, + requestFormat: RequestFormat.PKCS10, + enrollmentType: EnrollmentType.Renew); + + captured.Should().NotBeNull("the renew API path should have been taken"); + + // Bind to a local so the compiler's null-flow analysis is satisfied — a + // FluentAssertions NotBeNull() does not narrow the nullable reference. + RenewCertificateRequest renewReq = captured!; + + renewReq.Subject.Should().Be("CN=host.example.com", + "without the subject the renewal order has no usable primary domain"); + renewReq.Sans.Should().NotBeNull("renewals previously dropped every SAN"); + renewReq.Sans.Select(s => s.Value) + .Should().BeEquivalentTo(new[] { "host.example.com", "alt.example.com" }); + } + } +} diff --git a/CERTInext/API/CertificateRequest.cs b/CERTInext/API/CertificateRequest.cs index 336c8a1..c06046b 100644 --- a/CERTInext/API/CertificateRequest.cs +++ b/CERTInext/API/CertificateRequest.cs @@ -631,6 +631,23 @@ public class RenewCertificateRequest [JsonPropertyName("csr")] public string Csr { get; set; } + /// + /// Distinguished name of the certificate being renewed. Supplies the renewal order's + /// primary domain via its CN — without it the renewal falls back to the prior order's + /// requestor name, which is not a domain at all. + /// + [JsonPropertyName("subject")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string Subject { get; set; } + + /// + /// SANs to carry onto the renewal order. Renewals previously submitted none, so a + /// renewed UCC certificate came back holding only its primary domain. + /// + [JsonPropertyName("sans")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public System.Collections.Generic.List Sans { get; set; } + [JsonPropertyName("validityDays")] [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public int? ValidityDays { get; set; } diff --git a/CERTInext/CERTInextCAPlugin.cs b/CERTInext/CERTInextCAPlugin.cs index d6b384c..eabd07b 100644 --- a/CERTInext/CERTInextCAPlugin.cs +++ b/CERTInext/CERTInextCAPlugin.cs @@ -1112,7 +1112,7 @@ private async Task EnrollNewAsync( ValidityYears = ep.ValidityYears > 0 ? ep.ValidityYears : (int?)null, ValidityDays = ep.ValidityDays > 0 ? ep.ValidityDays : (int?)null, Subject = subject, - Sans = BuildSanList(san), + Sans = BuildSanList(san, csr), RequesterName = string.IsNullOrWhiteSpace(ep.RequesterName) ? null : ep.RequesterName, RequesterEmail = string.IsNullOrWhiteSpace(ep.RequesterEmail) ? null : ep.RequesterEmail, KeyType = string.IsNullOrWhiteSpace(ep.KeyType) ? null : ep.KeyType, @@ -1400,6 +1400,11 @@ private async Task RenewOrReissueAsync( var renewReq = new RenewCertificateRequest { Csr = csr, + // Renewals go out as a fresh CERTInext order, so they need the same domain + // set as a new enrollment — otherwise a renewed UCC certificate comes back + // holding only its primary domain. + Subject = subject, + Sans = BuildSanList(san, csr), ValidityDays = ep.ValidityDays > 0 ? ep.ValidityDays : (int?)null, RequesterName = string.IsNullOrWhiteSpace(ep.RequesterName) ? null : ep.RequesterName, RequesterEmail = string.IsNullOrWhiteSpace(ep.RequesterEmail) ? null : ep.RequesterEmail, @@ -2114,46 +2119,250 @@ private static int MapRevocationReasonStringToCode(string reason) } /// - /// Converts the multi-valued SAN dictionary from the AnyCA gateway into the - /// list expected by the CERTInext API. + /// Builds the list submitted to CERTInext, from the union of + /// two sources: the multi-valued SAN dictionary the AnyCA gateway hands us, and the + /// subjectAltName extension carried inside the CSR itself. + /// + /// Both sources are needed. The gateway dictionary is authoritative when Command + /// populates it, but not every enrollment path does — and the CSR is the only place + /// the requested names are guaranteed to appear, because the client built it. Union + /// + de-duplicate rather than preferring one, so a name requested in either place + /// reaches the order. + /// + /// Parsing the CSR here is not redundant with sending the CSR to CERTInext. + /// CERTInext ignores the CSR's subjectAltName extension outright — measured on the + /// US sandbox in SanSubmissionProbeTests: a CSR carrying two DNS names, + /// submitted with additionalDomains omitted, produced an order with only the + /// CN registered. Production behaves the same way: the customer report that prompted + /// this fix was a production UCC order whose CSR carried the SANs and whose issued + /// certificate held only the CN. Re-submitting the CSR's names through + /// additionalDomains is the only way a SAN that exists solely in the CSR + /// reaches the issued certificate. + /// + /// History (UCC SANs silently dropped): the gateway keys this dictionary + /// dnsname, not dns. did not recognize + /// dnsname, so every DNS SAN was typed "dnsname", filtered out by the + /// DNS-only test in BuildAdditionalDomains, and the order went to CERTInext + /// with no additionalDomains at all. The certificate came back holding only + /// the CN, which reads as the CA stripping SANs supplied on the CSR. /// - private static List BuildSanList(Dictionary san) + private List BuildSanList(Dictionary san, string csr) { - if (san == null || san.Count == 0) - return null; - var result = new List(); + // Type+value identity, so the same name requested as two different SAN types is + // preserved while an exact repeat across the two sources collapses. + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); - // AnyCA passes SANs keyed by type name (e.g. "Dns", "Ip", "Email", "Uri") - foreach (var kvp in san) + void Add(string type, string value) { - string sanType = MapSanType(kvp.Key); - if (kvp.Value == null) continue; + if (string.IsNullOrWhiteSpace(value)) return; + string trimmed = value.Trim(); + if (!seen.Add($"{type}|{trimmed}")) return; + result.Add(new SanEntry { Type = type, Value = trimmed }); + } - foreach (string value in kvp.Value) + // AnyCA passes SANs keyed by type name — the real gateway uses "dnsname", + // "rfc822name", "ipaddress"; MapSanType normalizes the spelling variants. + if (san != null) + { + foreach (var kvp in san) { - if (!string.IsNullOrWhiteSpace(value)) - result.Add(new SanEntry { Type = sanType, Value = value.Trim() }); + string sanType = MapSanType(kvp.Key); + if (kvp.Value == null) continue; + + foreach (string value in kvp.Value) + Add(sanType, value); } } - return result.Count > 0 ? result : null; + int fromGateway = result.Count; + + // Union in whatever the CSR asked for. Non-throwing: a malformed or unparseable + // CSR yields an empty list and leaves the gateway-supplied set untouched. + foreach (var csrSan in ExtractSanEntriesFromCsr(csr)) + Add(csrSan.Type, csrSan.Value); + + int fromCsrOnly = result.Count - fromGateway; + + if (result.Count == 0) + { + _logger.LogDebug( + "No SANs supplied by the gateway and none found in the CSR — submitting the order with domainName only."); + return null; + } + + // The blind spot that hid the original defect was that nothing logged what we + // resolved. Log the full resolved set and its provenance at Information. + _logger.LogInformation( + "Resolved {Total} SAN(s) for submission. FromGatewayRequest={FromGateway}, " + + "AddedFromCsr={FromCsr}, Sans={Sans}", + result.Count, fromGateway, fromCsrOnly, + string.Join("; ", result.Select(s => $"{s.Type}:{s.Value}"))); + + if (fromCsrOnly > 0) + { + // Worth a Warning, not Debug: it means Command did not hand us names the + // client actually requested, which is a gateway/template wiring smell even + // though we recover from it here. + _logger.LogWarning( + "{Count} SAN(s) were present in the CSR but absent from the SAN data supplied by Command; " + + "they have been added to the order. Review the enrollment pattern / template SAN configuration.", + fromCsrOnly); + } + + // CERTInext's certificateInformation.additionalDomains is a domain-name field, and + // non-DNS SANs are submitted into it deliberately rather than discarded: dropping + // them would issue a certificate silently missing names the subscriber asked for, + // which is the worse failure. + // + // Measured on the US SANDBOX only (SanSubmissionProbeTests, product 844, + // 2026-08-12): CERTInext did NOT reject these at order placement. It accepted the + // order and registered the value verbatim as an order domain — an email address, an + // IP literal and a URI all came back as domainVerification keys. The order then + // cannot pass domain validation, so it parks pending instead of failing fast. + // + // Production is UNVERIFIED for this case and may reject the order outright instead. + // The warning below therefore describes the sandbox outcome as the expected one + // without promising it: either way the operator is told which SANs are the problem, + // which is the part that matters for diagnosis. + var nonDns = result.Where(s => !string.Equals(s.Type, "dns", StringComparison.OrdinalIgnoreCase)).ToList(); + if (nonDns.Count > 0) + { + _logger.LogWarning( + "{Count} requested SAN(s) are not DNS names: {Sans}. CERTInext's additionalDomains " + + "field takes domain names, so this order will either be rejected outright or be " + + "created and then fail domain validation and sit pending — on the US sandbox it was " + + "accepted verbatim and parked pending. They are submitted rather than dropped on " + + "purpose: a visible failure is preferable to a certificate issued without names the " + + "subscriber requested. Remove them from the CSR or the enrollment pattern if the " + + "order should proceed.", + nonDns.Count, string.Join("; ", nonDns.Select(s => $"{s.Type}:{s.Value}"))); + } + + return result; } private static string MapSanType(string anyCAType) { switch (anyCAType?.ToLowerInvariant()) { - case "dns": return "dns"; + // "dnsname" is what the AnyCA REST Gateway actually sends; "dns"/"dnsnames" + // are kept for callers and older hosts that use the shorter spelling. + case "dns": + case "dnsname": + case "dnsnames": return "dns"; case "ip": - case "ipaddress": return "ip"; + case "ipaddress": + case "ipaddresses": return "ip"; case "email": - case "rfc822": return "email"; - case "uri": return "uri"; + case "rfc822": + case "rfc822name": return "email"; + case "uri": + case "uniformresourceidentifier": return "uri"; default: return anyCAType?.ToLowerInvariant() ?? "dns"; } } + /// + /// Extracts the subjectAltName entries from a PEM-encoded PKCS#10 CSR. + /// + /// Implemented with BouncyCastle (per the project's crypto policy: all certificate + /// and key handling goes through BouncyCastle, never BCL System.Security.Cryptography). + /// Never throws — an absent, truncated, or otherwise unparseable CSR returns an empty + /// list so enrollment continues on the gateway-supplied SAN data alone. + /// + private static List ExtractSanEntriesFromCsr(string csrPem) + { + var result = new List(); + if (string.IsNullOrWhiteSpace(csrPem)) + return result; + + try + { + string b64 = csrPem + .Replace("-----BEGIN CERTIFICATE REQUEST-----", string.Empty) + .Replace("-----END CERTIFICATE REQUEST-----", string.Empty) + .Replace("-----BEGIN NEW CERTIFICATE REQUEST-----", string.Empty) + .Replace("-----END NEW CERTIFICATE REQUEST-----", string.Empty) + .Replace("\r", string.Empty) + .Replace("\n", string.Empty) + .Trim(); + + if (string.IsNullOrWhiteSpace(b64)) + return result; + + var csr = new Org.BouncyCastle.Pkcs.Pkcs10CertificationRequest(Convert.FromBase64String(b64)); + + // SANs live in the PKCS#9 extensionRequest attribute, not the CSR body. + var extensions = csr.GetRequestedExtensions(); + var sanExtension = extensions?.GetExtension( + Org.BouncyCastle.Asn1.X509.X509Extensions.SubjectAlternativeName); + if (sanExtension == null) + return result; + + var names = Org.BouncyCastle.Asn1.X509.GeneralNames.GetInstance(sanExtension.GetParsedValue()); + foreach (var generalName in names.GetNames()) + { + string value = GeneralNameToValue(generalName); + if (!string.IsNullOrWhiteSpace(value)) + result.Add(new SanEntry { Type = SanTypeFromGeneralNameTag(generalName.TagNo), Value = value }); + } + } + catch (Exception ex) + { + // Enrollment must not fail because we could not read the CSR's SANs — the + // gateway-supplied set still applies, and CERTInext validates the CSR itself. + // Debug so an operator diagnosing a missing SAN can see the parse was skipped. + LogHandler.GetClassLogger(typeof(CERTInextCAPlugin)) + .LogDebug(ex, "ExtractSanEntriesFromCsr suppressed CSR parse failure"); + } + + return result; + } + + /// Maps an ASN.1 GeneralName tag to the SAN type string used by this plugin. + private static string SanTypeFromGeneralNameTag(int tagNo) + { + switch (tagNo) + { + case Org.BouncyCastle.Asn1.X509.GeneralName.DnsName: return "dns"; + case Org.BouncyCastle.Asn1.X509.GeneralName.Rfc822Name: return "email"; + case Org.BouncyCastle.Asn1.X509.GeneralName.UniformResourceIdentifier: return "uri"; + case Org.BouncyCastle.Asn1.X509.GeneralName.IPAddress: return "ip"; + case Org.BouncyCastle.Asn1.X509.GeneralName.DirectoryName: return "directoryname"; + case Org.BouncyCastle.Asn1.X509.GeneralName.RegisteredID: return "registeredid"; + case Org.BouncyCastle.Asn1.X509.GeneralName.OtherName: return "othername"; + default: return $"generalname-{tagNo}"; + } + } + + /// + /// Renders a GeneralName's value as the text CERTInext would need to see. Returns null + /// for a name whose value cannot be rendered meaningfully, so it is skipped rather than + /// submitted as ASN.1 debris. + /// + private static string GeneralNameToValue(Org.BouncyCastle.Asn1.X509.GeneralName generalName) + { + switch (generalName.TagNo) + { + case Org.BouncyCastle.Asn1.X509.GeneralName.DnsName: + case Org.BouncyCastle.Asn1.X509.GeneralName.Rfc822Name: + case Org.BouncyCastle.Asn1.X509.GeneralName.UniformResourceIdentifier: + return Org.BouncyCastle.Asn1.DerIA5String.GetInstance(generalName.Name).GetString(); + + case Org.BouncyCastle.Asn1.X509.GeneralName.IPAddress: + // Octet string → dotted-quad / RFC 5952 text, so what we submit and log is + // the address the subscriber asked for rather than its hex encoding. + byte[] octets = Org.BouncyCastle.Asn1.Asn1OctetString.GetInstance(generalName.Name).GetOctets(); + return octets.Length == 4 || octets.Length == 16 + ? new System.Net.IPAddress(octets).ToString() + : null; + + default: + return generalName.Name?.ToString(); + } + } + private static string GetStringValue( Dictionary dict, string key, string defaultValue = "") { diff --git a/CERTInext/Client/CERTInextClient.cs b/CERTInext/Client/CERTInextClient.cs index 2c604aa..4551529 100644 --- a/CERTInext/Client/CERTInextClient.cs +++ b/CERTInext/Client/CERTInextClient.cs @@ -186,9 +186,20 @@ public async Task PlaceOrderAsync( if (request.Meta == null) request.Meta = await BuildMetaAsync(ct); + // The domain set is logged here, at the wire, not just where Command hands it to us. + // A UCC order that silently lost its SANs upstream of this point is otherwise + // indistinguishable in the gateway log from one the CA stripped — reconciling the + // enrollment-start "SANs=" line against this one localizes the loss immediately. + var certInfo = request.OrderDetails?.CertificateInformation; Logger.LogInformation( - "Submitting order to CERTInext. ProductCode={ProductCode}", - request.OrderDetails?.ProductCode); + "Submitting order to CERTInext. ProductCode={ProductCode}, DomainName={DomainName}, " + + "AdditionalDomainCount={AdditionalDomainCount}, AdditionalDomains={AdditionalDomains}", + request.OrderDetails?.ProductCode, + certInfo?.DomainName, + certInfo?.AdditionalDomains?.Count ?? 0, + certInfo?.AdditionalDomains != null && certInfo.AdditionalDomains.Count > 0 + ? string.Join("; ", certInfo.AdditionalDomains) + : "(none)"); GenerateOrderResponse result = null; RestResponse resp = null; @@ -710,6 +721,23 @@ public async Task RenewCertificateAsync( throw new KeyNotFoundException($"Cannot renew: prior order '{certificateId}' was not found in CERTInext."); } + // Primary domain for the renewal order. Prefer the CN of the subject Command gave + // us; the prior order's requestorName is only a last resort and is not a domain — + // it is retained solely so an old caller that sets no Subject behaves as before. + string renewalDomainName = + ExtractCnFromSubject(request.Subject) + ?? priorTrack.OrderDetails?.RequestorInformation?.RequestorName + ?? "unknown"; + + if (ExtractCnFromSubject(request.Subject) == null) + { + Logger.LogWarning( + "Renewal of order {PriorId} has no usable CN in its subject; falling back to " + + "DomainName='{DomainName}' from the prior order. Verify the renewed certificate's " + + "primary domain.", + certificateId, renewalDomainName); + } + // We don't have the product code from TrackOrder — build an order using // the config defaults and the CSR from the renewal request. var orderReq = new GenerateOrderSslRequest @@ -729,7 +757,8 @@ public async Task RenewCertificateAsync( SubscriptionDetails = new SubscriptionDetails { Validity = "1" }, CertificateInformation = new CertificateInformation { - DomainName = priorTrack.OrderDetails?.RequestorInformation?.RequestorName ?? "unknown" + DomainName = renewalDomainName, + AdditionalDomains = BuildAdditionalDomains(request.Sans, renewalDomainName) }, Csr = request.Csr, AgreementDetails = BuildDefaultAgreementDetails() @@ -1337,6 +1366,10 @@ private GenerateOrderSslRequest BuildOrderRequestFromLegacyEnrollRequest(EnrollC string requestorIsd = string.IsNullOrWhiteSpace(_config.RequestorIsdCode) ? "1" : _config.RequestorIsdCode; string requestorMobile = _config.RequestorMobileNumber ?? string.Empty; + // Hoisted: additionalDomains is de-duplicated against the primary domain, so both + // fields have to be built from the same value. + string domainName = ExtractCnFromSubject(request.Subject) ?? "unknown"; + return new GenerateOrderSslRequest { // Meta will be set by PlaceOrderAsync @@ -1382,8 +1415,8 @@ private GenerateOrderSslRequest BuildOrderRequestFromLegacyEnrollRequest(EnrollC }, CertificateInformation = new CertificateInformation { - DomainName = ExtractCnFromSubject(request.Subject) ?? "unknown", - AdditionalDomains = BuildAdditionalDomains(request.Sans), + DomainName = domainName, + AdditionalDomains = BuildAdditionalDomains(request.Sans, domainName), AutoSecureWww = string.IsNullOrWhiteSpace(_config.AutoSecureWww) ? "0" : _config.AutoSecureWww }, @@ -1445,16 +1478,57 @@ private static string ExtractCnFromSubject(string subject) return null; } - private static List BuildAdditionalDomains(System.Collections.Generic.List sans) + /// + /// Projects the resolved SAN list onto certificateInformation.additionalDomains. + /// + /// Every requested SAN is submitted regardless of type. Filtering to DNS-only (the + /// original behaviour) issued certificates quietly missing names the subscriber had + /// requested, which is the worse failure; the caller warns about the non-DNS entries + /// before we get here. + /// + /// is the value already going out as the order's primary + /// domain, and Command normally includes the CN in the SAN set as well. On the US + /// sandbox CERTInext was measured to collapse that repetition itself + /// (SanSubmissionProbeTests: CN submitted twice came back registered once), but that is + /// undocumented and unverified against production — which is exactly why we exclude it + /// here rather than relying on CA-side de-duplication. It also keeps the submitted body + /// matching what we log. + /// + private List BuildAdditionalDomains( + System.Collections.Generic.List sans, + string domainName) { if (sans == null || sans.Count == 0) return null; + var domains = new List(); + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + + bool haveDomainName = !string.IsNullOrWhiteSpace(domainName); + if (haveDomainName) + seen.Add(domainName.Trim()); + + int duplicates = 0; foreach (var san in sans) { - if (string.Equals(san.Type, "dns", StringComparison.OrdinalIgnoreCase) && - !string.IsNullOrWhiteSpace(san.Value)) - domains.Add(san.Value); + if (san == null || string.IsNullOrWhiteSpace(san.Value)) continue; + + string value = san.Value.Trim(); + if (!seen.Add(value)) + { + duplicates++; + continue; + } + domains.Add(value); + } + + if (duplicates > 0) + { + Logger.LogDebug( + "Collapsed {Count} duplicate SAN value(s) out of additionalDomains " + + "(already submitted as domainName '{DomainName}', or repeated in the SAN set).", + duplicates, domainName); } + return domains.Count > 0 ? domains : null; }