Repository navigation
feat(v2): consolidated V2 plugin: 1.0.x ports, V2 spec alignment, inline CSR default - #39
Open
spbsoluble wants to merge 241 commits into
Open
spbsoluble wants to merge 241 commits into
spbsoluble wants to merge 241 commits into
Conversation
After submitting an order, Enroll() polls GetCertificate up to PickupRetries times (default 5), PickupDelay seconds apart (default 10), after a 5s initial delay, so a fast-issuing order returns the certificate in the same enrollment call instead of waiting for the next sync. Mirrors the legacy Sectigo connector's PickUpEnrolledCertificate (~55s max worker-thread occupancy by default). Applied to the new, reissue, and renew paths; both build flavors. PickupRetries=0 disables. Orders not issued within the window are returned pending and imported by a later sync (unchanged). OV/EV are issued asynchronously by the CA and typically exhaust the window; DV / already-approved orders return in-call.
RequestFormat is received by Enroll() but was never logged, so logs could not show what Command passes for CSR vs PFX enrollments. Add it to the enrollment-start Information line for diagnostics. Behavior unchanged — the value is still not used for any decision (the gateway treats every enrollment as a CSR-based request).
…k timeout A network-level timeout on GenerateOrderSSL / SubmitCSR can occur after CERTInext has already received and created the order. The inner HTTP retry re-sent the same request body (same requestTxn), which CERTInext rejected as EMS-947 "Duplicate requestTxn" — failing the enrollment while orphaning the created order. ExecuteWithRetryAsync gains an `idempotent` flag; PlaceOrderAsync and SubmitCsrAsync now submit once (idempotent:false). A transient submit failure and an EMS-947 duplicate are each logged as an explicit no-retry decision and surfaced with a clear, conditional message (if an order was created it is imported by the next sync). Idempotent read calls are unchanged and still retry.
…dance Compliance follow-ups (both Low): - Add DomainName as a non-sensitive correlation key to the PlaceOrder transient and EMS-947 warnings so an orphaned order can be tied to its enrollment under concurrency (requestTxn is deliberately NOT logged — it is part of the authKey preimage). - SubmitCSR now carries the "may already have been received; do not resubmit" guidance in the thrown exception on a transient failure, for parity with PlaceOrderAsync (previously only in the log line).
…dit logging Review-driven refinements to the v1.0.1 synchronous-pickup feature: - Skip the pickup poll when the DCV path already owns the in-call issuance wait, so the two never stack and a cancelled/rejected order is not re-polled for the full window (fixes a regression that broke the terminal-order guard). - Cap total in-call pickup wait at 180s regardless of how PickupRetries and PickupDelay are configured, so an aggressive combination can't exceed Command's enrollment timeout. - Log a terminal FAILED at Error and REVOKED at Warning; trace each poll at Debug; distinguish "all polls errored" from "still pending" in the timeout summary; include OrderNumber in the CSR transient-failure warning; surface a pending result that has no order number to poll instead of skipping silently. - Default pickup off in the unit-test fixtures and add targeted pickup tests (disabled / issued / terminal / budget-exhausted). Both flavors build clean (0 warnings); DCV 199/199, no-DCV 176/176.
…t empty (#21) * fix(enroll): UCC SANs never reached CERTInext — additionalDomains sent empty Certificates enrolled through a UCC product came back holding only the CN, even though the requested SANs were present on the CSR and in the SAN data Command supplied. The names were being dropped inside the plugin, not by the CA. Root cause: the AnyCA REST Gateway keys its SAN dictionary "dnsname", but MapSanType only recognized "dns". Every DNS SAN was therefore typed "dnsname", which failed the DNS-only test in BuildAdditionalDomains, so certificateInformation.additionalDomains was null and JsonIgnore-WhenWritingNull removed the field from the order body entirely. Confirmed against a customer gateway log: Enrollment attempt started. ... SANs=dnsname:CLAUDIOTEST20.ucsd.edu; dnsname:CLAUDIOTEST20.ad.ucsd.edu The CSR did not compensate, because CERTInext ignores the CSR's subjectAltName extension outright — measured, see below. Changes: * MapSanType now recognizes the spellings the gateway actually sends: dnsname, rfc822name, ipaddress, uniformresourceidentifier (the short forms still work). * BuildSanList unions the gateway-supplied SANs with the SANs parsed out of the CSR (BouncyCastle, per the project crypto policy), de-duplicating on type+value case-insensitively. Parsing the CSR is not redundant with sending it: CERTInext will not read those names itself, so re-submitting them through additionalDomains is the only way a CSR-only SAN reaches the certificate. CSR parsing is non-throwing — an unparseable CSR falls back to the gateway set. * BuildAdditionalDomains no longer filters to DNS-only. Every requested SAN is submitted; discarding the non-DNS ones issued certificates quietly missing names the subscriber asked for, which is the worse failure. It also excludes the value already going out as domainName so the CN is not submitted twice. * Renewals carried no SANs at all and took their primary domain from the prior order's requestorName. RenewCertificateRequest now carries Subject + Sans, and the renewal order derives domainName from the subject CN with the old value as a logged fallback. * PlaceOrderAsync now logs domainName and additionalDomains. The absence of any outbound domain logging is what made this look like CA-side stripping: the gateway log recorded the SANs Command supplied and nothing about what was put on the wire. Measured CERTInext behaviour (SanSubmissionProbeTests, sandbox-us, product 844 OV SSL UCC) — these replace assumptions the old code encoded but never tested: * additionalDomains is what puts extra names on the order (CN + extra1 → both registered). * CERTInext IGNORES CSR SANs. A CSR carrying two DNS names with additionalDomains omitted produced an order with only the CN registered. This is the customer-facing root cause. * Non-DNS values are NOT rejected, contrary to what the DNS-only filter assumed. An email address, an IPv4 literal and an https URI were each accepted and registered verbatim as order domains, so such an order is created and then cannot pass validation rather than failing up front. The plugin warns accordingly. * Repeating the CN inside additionalDomains is accepted and collapsed by the CA, so our de-duplication is defence in depth rather than a requirement. Tests: 9 new unit tests drive plugin.Enroll through a real client against WireMock and assert on the JSON actually posted — a test of the mapping function alone would not have caught this, since the mapping "worked" and the loss happened in its interaction with the downstream filter. The live probe is opt-in behind CERTINEXT_SAN_PROBE=1. * docs(enroll): scope the CERTInext SAN measurements to the sandbox The probe ran against sandbox-us, but the comments and the non-DNS warning read as though the behaviour were established generally. The customer this fix is for is on production, so the distinction matters. * The non-DNS finding (CERTInext accepts an email/IP/URI verbatim as an order domain rather than rejecting it) is explicitly sandbox-only and flagged unverified on production. The operator-facing warning no longer promises a parked order — it names the offending SANs and says the order will either be rejected or fail validation, noting what the sandbox did. * The CN-collapse finding is likewise marked sandbox-only, which strengthens rather than weakens the case for de-duplicating on our side: we should not depend on undocumented CA behaviour we have not seen in production. * The CSR-SAN finding — CERTInext ignores the CSR's subjectAltName entirely — is noted as corroborated by production independently of the probe: the report that prompted this work was a production UCC order whose CSR carried the SANs and whose certificate came back holding only the CN. * Probe header now says how to re-run against production, and warns that product numbering is per-account (Constants.Products holds defaults, not guarantees). No functional change. * fix(enroll): address full-review round 1 — DCV strand, ASN.1 debris, log injection Six confirmed findings from the five gating lenses, collapsing into three defects plus an upgrade-safety gap. 1. Undrainable pending domains stranded the valid ones (correctness, medium). Submitting non-DNS SANs means CERTInext registers them verbatim as order domains, so an email/URI SAN turns up as a domainVerification key that fails PerformDcvIfNeededAsync's FQDN check. That check threw for the whole order, before staging anything — and the exception escapes Enroll, which has no catch, after the order was already placed. Result: failed enrollment, orphaned order at the CA, no TXT record staged for the valid domains beside it, and every later Synchronize/GetSingleRecord retry re-threw into TryRunDcvDuringSyncAsync's catch-and-return-false, so the order could never progress. Invalid domains are now excluded (still LogError, so the audit trail keeps the signal) and the rest of the order proceeds. Same treatment where no DNS provider resolves for a domain, which is where an IP-literal SAN dead-ends: it clears the FQDN regex but no zone can match it. The genuine "no DNS provider deployed" misconfiguration still throws — distinguished by nothing on the order resolving at all — so Dcv_Throws_WhenNoProviderForDomain keeps its meaning. This is the file's own stated principle, already written at the EMS-956 branch: do not throw out of DCV for a condition that leaves the order legitimately pending. 2. GeneralNameToValue emitted ASN.1 debris (correctness + security + api-compat). Its default branch returned BouncyCastle's stringification, so a UPN otherName from a Windows-generated CSR was submitted as "[1.3.6.1.4.1.311.20.2.3, [CONTEXT 0]svc@corp.example.com]" and a directoryName as "CN=host.example.com,O=Acme" — in a domain-name field, contradicting the method's own doc comment and breaking orders that previously succeeded. These now return null. They are genuinely unrepresentable as a domain, unlike a well-formed IP/email/URI SAN, which we still submit on purpose. Skipping is not silent: ExtractSanEntriesFromCsr reports the skipped GeneralName tags and BuildSanList warns with them. 3. Log injection in the new audit sinks (security, low, CWE-117). SAN values come from the CSR and Command's dictionary — i.e. the requester — and were interpolated into three new log lines unescaped. Structured templates stop format-string abuse but not embedded CRLF, and NLog's text layout does not escape it, so a requester could forge audit records in the very lines added to make the submitted SAN set auditable. Added SanitizeForLog and applied it at all sinks. Deliberately a logging-only scrub: the value submitted to the CA is unchanged. 4. Upgrade safety (api-compat, medium). Submitting non-DNS SANs flips affected enrollments from "issues, silently incomplete" to "parks pending", with no way back short of downgrading the plugin. Added the SubmitNonDnsSans connector setting (default true — current behaviour) to restore DNS-only submission, and a CHANGELOG upgrade note, since the review's residual concern was process rather than logic. Also applied the endorsed advisory: RenewCertificateAsync parsed the subject twice; hoisted to one call, matching what the sibling method already does. Tests: 12 added — two DCV tests proving a non-FQDN domain and an unresolvable domain each leave their co-tenant staged and issuing, an otherName/directoryName test asserting no ASN.1 debris reaches the posted JSON, a SanitizeForLog theory, a CRLF-does-not-break-enrollment test, and SubmitNonDnsSans on/default coverage. Release, no-DCV: 195/195. Release, DCV: 220/220. Zero code warnings in both. * fix(enroll): address full-review round 2 — false pending-domain invariant, TXT leak Four confirmed findings. 1+2. The round-1 misconfiguration throw assumed "the CN is always a pending domain too" — false whenever CERTInext has cached a prior DCV validation for the CN/parent domain (a case this same method already special-cases earlier, at the aggregate/per-domain "already validated" check). When that happens the CN drops out of pendingDomains, and an order carrying only a non-DNS SAN alongside it hit the "nothing on the order resolves a provider" branch and threw — reopening the exact orphaned/stranded-order failure round 1 fixed, just narrowed to this input shape. The check now asks the right question: does ANY domain on the order — pending or already validated — resolve a DNS provider? If yes, a provider is clearly deployed and working, so this is the bad-SAN case (defer, don't throw). Only if nothing on the whole order resolves is it the genuine "no provider deployed" misconfiguration. 4. The TXT-staging loop's throw/defer sites (GetDcv failure, empty token, stage failure, EMS-956 not-ready) were all outside the try/finally that owns cleanup. Round 1 made multi-domain staging the normal case by finally submitting every SAN — before, a UCC order's SANs never reached CERTInext at all, so an order rarely had more than one pending domain. A later domain's failure now orphans every TXT record already published for the earlier domains in the same order, permanently — nothing else in the codebase ever calls CleanupValidation for them. The staging loop is now wrapped so any exit — exception or the not-yet-ready deferral — cleans up whatever was already staged first. 3. BuildAdditionalDomains' new duplicate-collapse debug log was the one sink in the diff that skipped the round-1 SanitizeForLog scrub. Applied. Also applied all 4 endorsed advisory simplifications: collapsed SanTypeFromGeneralNameTag + GeneralNameToValue into one GeneralNameToSanEntry (the split had four dead branches — a type mapping for tags whose value always came back null); moved the twice-duplicated SanitizeForLog into a shared internal LogSanitizer.Strip (Models/LogSanitizer.cs); nested BuildSanList's two non-DNS branches under one `nonDns.Count > 0` test instead of two; hoisted the repeated SAN-list log rendering into a local. Tests: 2 added (cached-CN-plus-unresolvable-SAN must defer without throwing; a second domain's stage failure must clean up the first domain's TXT record). SanitizeForLog's reflection-based test now calls LogSanitizer.Strip directly (it's internal, not private, and InternalsVisibleTo already covers the test project). Release, no-DCV: 195/195. Release, DCV: 222/222. Zero code warnings in both. * fix(enroll): address full-review round 3 — never throw out of DCV staging, CSR fallback not union All three dispositions carried forward from round 2 were broken by this round's adjudicator (real, not accepted), plus 8 more findings collapsing into the same three root causes. 1. PerformDcvIfNeededAsync's per-domain isolation (rounds 1-2) covered only the validator-resolution-null case. A GetDcv failure, an empty DCV token, and a StageValidation failure all still threw and aborted the WHOLE order — exactly the orphaned/stranded-order failure the isolation exists to prevent, just narrower. Confirmed reachable via the non-DNS SANs this PR submits by design (an IP-literal SAN clears the FQDN filter and reaches GetDcv; its live behavior there is unmeasured — my round-2 "measured" claim was based on a Moq stub asserting my own assumption, not the live API). Separately, the post-loop misconfiguration throw ("no DNS provider configured") could fire on an ordinary non-DNS Subject CN with no config escape hatch: SubmitNonDnsSans only filters the returned SAN list, never `subject`/`domainName`, so an IP-format CN reaches this path unfiltered. Fix: every per-domain failure in the staging loop (GetDcv error, empty token, no resolvable validator, StageValidation throwing or returning failure) is now LogError + skip-this-domain-and-continue. Nothing in the loop throws for an input- or API-driven reason any more. The only remaining "abort the whole pass" case is EMS-956 (DCV not yet exposed at the CA) — an order-readiness condition, not a per-domain one, so it still defers immediately rather than isolating per domain. The post-loop misconfiguration throw is gone; "nothing could be staged" now always defers to the next sync cycle with a LogError naming every skipped domain and why, rather than sometimes throwing depending on which domain failed or what else was on the order. 2. BuildSanList's CSR union (rounds 1-2) let a signed CSR's own SAN extension reintroduce names regardless of what Command's SAN dictionary supplied. External research against Keyfactor Command's documented enrollment-pattern behavior found no evidence Command enforces SAN policy by narrowing a signed CSR's embedded SANs before calling Enroll — reconciliation between an externally-generated CSR and Command's SAN data is documented as plugin/CA-configuration-dependent, not Command-enforced. A subscriber's own CSR routinely carries more names than an enrollment pattern computed, and the union let all of them through. Fix: the CSR is now a fallback, consulted only when Command supplies no SAN data at all (the case the original UCC-SAN-drop customer defect actually needed). When Command supplies any SAN entries, the CSR's own SAN extension is ignored entirely — the gateway dictionary is authoritative, not merely first. 3. Three findings on BuildSanList's logging: (a) the "N SAN(s) ... have been added to the order" line fired for CSR-fallback entries before the SubmitNonDnsSans=false filter removed exactly those entries two lines later — a false claim in the same call; (b) the "Resolved N SAN(s)" provenance line had the same before/after-filter mismatch; (c) two throw sites (empty token, stage failure) had no preceding structured log before the bare cleanup-and-rethrow wrapper caught them — moot now that neither throws, since both are LogError'd before being skipped. Fixed by reordering: apply the SubmitNonDnsSans filter first, log the resolved set and CSR-fallback provenance from the final, already-filtered result. Also fixed on the same pass: RenewCertificateAsync's "no usable CN" warning logged the prior order's RequestorName fallback unsanitized — the one sink in this diff that had skipped LogSanitizer.Strip. Tests: rewrote 6 existing DCV tests whose names and assertions pinned the old throw behavior (Dcv_Throws_* → Dcv_SkipsAndDefers_*, including reversing Dcv_Rethrows_When_GetDcv_FailsWithUnrelatedError's stated intent, and rewriting the round-2 TXT-leak test since a skipped domain no longer needs mid-call cleanup — the good domain now just completes its normal lifecycle). Rewrote the CSR-union test into CsrOnlySans_AreIgnored_WhenGatewaySuppliesAnyEntries. Added one test for the log-ordering fix's underlying data flow (CSR-fallback non-DNS SAN genuinely absent from the wire when SubmitNonDnsSans=false, not just mis-described in the log). Release, no-DCV: 196/196. Release, DCV: 223/223. Zero code warnings in both. * fix(enroll): address full-review round 4 — regex $ quirk, cancellation, CSR-fallback edge case Six confirmed findings, a clean round: 0 dismissed, 0 inconclusive. 1. The FQDN validation regex used ^...$, and in .NET's default (non-Multiline) mode $ matches immediately before a single trailing '\n', not only at the true end of the string — so "evil.com\n" passed as "valid" and reached several unsanitized-relative-to-siblings log sinks further down the same method (Staging/Triggering/cleanup/verified/rejected lines). Round 1 fixed log injection at other sinks in this file, but this one slipped through because the domain LOOKED validated. Fixed at the source: the regex now anchors with \A/\z (absolute string bounds regardless of trailing newlines), so a value with any trailing control character is rejected by the FQDN gate itself. Also applied LogSanitizer.Strip at every remaining domain/hostname sink in PerformDcvIfNeededAsync and WaitForDcvVerificationAsync for defense in depth and consistency with the sibling error-path logs that already had it. Worth noting for the record: this path is reachable for ANY order the account has at EXTERNALVALIDATION via Synchronize/GetSingleRecord, not only ones this plugin's own Enroll call placed — TrackOrder's domainVerification keys for an externally-created order are never passed through this plugin's own outbound Trim() calls, so those calls (correct for the outbound path) do not protect this inbound one. 2. SubmitNonDnsSans — the toggle that decides whether a certificate can issue silently missing requested SAN names — was never included in the Initialize startup config-dump log, unlike every sibling setting (DcvEnabled, DcvTxtRecordTemplate, IgnoreExpired, PageSize) that log line exists to make auditable. Added. 3+4+5. The generic per-domain `catch (Exception ex)` blocks around GetDcvAsync and StageValidation also caught OperationCanceledException/ TaskCanceledException raised by the shared DcvTimeoutMinutes-bound cancellation token, mislabeling a genuine timeout as an ordinary per-domain CA/DNS-provider failure in the skippedDomains audit summary — directly contradicting the outer catch's own comment, which claimed to be the handler for exactly this case but could never actually see it, since the inner catches intercepted it first. Both per-domain catch sites now re-throw OperationCanceledException explicitly before their generic Exception clause, so cancellation reaches the outer catch. That outer catch previously logged nothing before cleaning up and rethrowing — with neither EnrollNewAsync nor Enroll adding a catch of their own, an unanticipated failure during the synchronous Enroll-time DCV path left no plugin-emitted audit record at all. Added a LogError there. 6. BuildSanList's CSR-fallback trigger was "the gateway SAN dictionary computed to zero added entries" (fromGateway == 0), which cannot distinguish a null/absent dictionary from a non-null dictionary whose only keys map to empty arrays. An enrollment pattern that runs and deliberately computes zero SANs for a request is a policy decision this plugin must respect — round 3's fallback-over-union redesign existed specifically to stop CSR names overriding Command's SAN policy, and this edge case reopened exactly that. The trigger now checks `san == null` directly: only the literal absence of a dictionary engages the CSR fallback. Tests: 4 added — a trailing-newline domain routed to invalidDomains rather than reaching GetDcv; a cancellation during GetDcv propagating rather than being reported as a skipped-domain failure; and a non-null, all-empty-array gateway SAN dictionary correctly suppressing the CSR fallback (CN-only result, not backfilled from the CSR). Release, no-DCV: 197/197. Release, DCV: 226/226. Zero code warnings in both. * fix(enroll): address full-review round 5 — cancellation swallowed by RestSharp, unsanitized DomainName log Two confirmed findings; the first invalidated round 4's own cancellation fix in a way only a real-HTTP-level test could catch. 1. Round 4 added `catch (OperationCanceledException) { throw; }` guards ahead of the generic per-domain catches in PerformDcvIfNeededAsync, to stop a DCV timeout from being mislabeled as an ordinary per-domain failure. That guard is correct but dead for the real trigger: CERTInextClient is built with ThrowOnAnyError=false, so when the shared cancellation token fires mid-call, RestSharp catches HttpClient.SendAsync's cancellation internally and returns a non-throwing, unsuccessful RestResponse instead of propagating OperationCanceledException. DeserializeOrThrow then wraps that into a plain Exception — which lands in the generic catch, not the new guard, and gets logged and reported as "GetDcv failed" for whatever domain happened to be in flight. Round 4's regression test only proved the plugin-side logic works when a Moq mock is told to throw OperationCanceledException directly — which the real client never does, so it gave false confidence. Fixed at the actual source: ExecuteWithRetryAsync (the one place in the client that holds `ct`) now calls ct.ThrowIfCancellationRequested() immediately after the HTTP call, before any retry or error-wrapping logic sees the response. This fixes every caller of ExecuteWithRetryAsync, not just GetDcvAsync — the same swallow-and-wrap otherwise applies to VerifyDcvAsync, TrackOrderAsync, and everything else that goes through it. 2. PlaceOrderAsync's transient-failure and duplicate-transaction warning logs interpolated the requester-derived DomainName without LogSanitizer.Strip, inconsistent with a sibling log statement three lines above in the same method that already sanitizes the identical field. Fixed both. Also applied the one endorsed advisory: CleanupPartialStagingAsync (added in round 2 for early-exit paths) duplicated the pre-existing try/finally cleanup loop almost line-for-line; both now share CleanupOneStagedValidationAsync, parameterized by a log-context string for the one place their wording differs. Left as-is: a safely-dismissed finding about Enroll()'s original "Enrollment attempt started" log (pre-existing, outside this diff) not being sanitized — the adjudicator tried to break that as out-of-scope and could not. Tests: added GetDcvAsync_ThrowsOperationCanceled_WhenCancellationTokenIsCancelled in CERTInextClientTests.cs — against the REAL client and a real (local) WireMock HTTP call with a pre-cancelled token, not a mock told to throw whatever type is asked for. This is the test shape round 4 was missing. Release, no-DCV: 198/198. Release, DCV: 227/227. Zero code warnings in both. * fix(enroll): address full-review round 6 — cleanup reuses cancelled token, no correlation ID Two confirmed findings, both in code from earlier rounds. 1. The DCV-timeout cleanup path (added round 2, hardened round 3) calls CleanupValidation with the same `ct` the operation was just cancelled by. Any IDomainValidator that forwards its token into its own HTTP calls — the reference CloudflareDomainValidator in this repo does exactly that — throws immediately on an already-cancelled token and never attempts the delete. So the one cleanup path specifically built to handle a DCV timeout is the one most likely to silently no-op in exactly that scenario, leaving a published TXT record behind with only a Warning logged ("may require manual removal"). CleanupOneStagedValidationAsync (deduplicated in round 5) now calls CleanupValidation with CancellationToken.None. This is a best-effort compensating action — removing a record we already published — and it must run regardless of why we're cleaning up, including when `ct` itself is the reason. 2. BuildSanList's provenance/resolution log lines (the exact logging this diff added specifically to close "the blind spot that hid the original defect") carried no Subject, unlike nearly every other enrollment-path log line in this file, which repeats Subject={Subject} per line rather than relying on any log-scope mechanism (there is none in this codebase). Under concurrent enrollments, an auditor could not attribute either line back to a specific request. Threaded `subject` through BuildSanList's signature and both call sites, added to all five of its log statements. Advisory noted, not acted on: the file's original "Enrollment attempt started" log doesn't sanitize Subject/SANs either — round 5's adjudicator already tried to break accepting that as pre-existing/out-of-scope and could not, so it stands. Tests: added Dcv_CleanupAfterCancellation_UsesCancellationTokenNone_NotTheAmbientToken, which asserts CleanupValidation's token argument is exactly CancellationToken.None (not merely "not visibly cancelled during a fast test run", which the real DcvTimeoutMinutes-bound token can't be driven to within a unit test) — proving the code passes the literal value regardless of the ambient token's state. Release, no-DCV: 198/198. Release, DCV: 228/228. Zero code warnings in both. * fix(enroll): address full-review round 7 — cleanup call unbounded, subject unsanitized in BuildSanList Three confirmed findings; two are the same root cause (severity high + medium, same location) and the third is a direct consequence of round 6's own fix. 1+3. Round 6's CancellationToken.None fix for the cleanup call over-corrected: it stopped the compensating CleanupValidation call from reusing an already-cancelled token, but in doing so removed its timeout bound entirely — at every one of its three call sites, including the routine, always-runs finally-block cleanup on the ordinary successful-DCV path, which was never cancellation-related to begin with. This directly contradicts the method's own documented SOX CC7.3 guarantee that the DCV flow is hard-timeout-bounded so a stuck DNS provider cannot hold a gateway worker request open indefinitely. A hanging network call inside any third-party IDomainValidator's CleanupValidation would now block forever. Fixed with a fresh, independently-bounded token instead of either extreme: CleanupOneStagedValidationAsync now creates its own CancellationTokenSource with a new Constants.Dcv.CleanupValidationTimeoutSeconds (60s) ceiling for each cleanup call. Not cancelled going in (so a cooperative validator still gets a real chance to run, closing round 6's original gap), but still bounded (closing this round's regression on top of it). 2. BuildSanList's six Subject={Subject} log lines (added last round for audit-trail correlation) logged the requester-controlled Subject DN raw, while every OTHER requester-controlled value in the same function (domain, SAN value, hostname) already goes through LogSanitizer.Strip — an inconsistency introduced within this diff's own new code, unlike the file's pre-existing "Enrollment attempt started" log (which round 5's adjudicator confirmed is legitimately out of scope, being unrelated pre-existing code). Wrapped all six. Also applied the one endorsed advisory: two new DCV test helpers built three byte-for-byte-identical DomainVerificationDetail JSON blocks; extracted a one-line DcvDetail(dcvStatus) helper. Updated the round-6 regression test to match: it asserted the cleanup token equals CancellationToken.None exactly, which is no longer true. Now asserts the two properties that actually matter — IsCancellationRequested is false (not reusing the cancelled ambient token) and CanBeCanceled is true (still bounded, not CancellationToken.None). Release, no-DCV: 198/198. Release, DCV: 228/228. Zero code warnings in both. * fix(enroll): address full-review round 8 — stale gateway count in log, cancellation swallows audit line Two confirmed findings, low/medium severity — cosmetic/observability rather than functional. 1. BuildSanList's own "Resolved N SAN(s)" log line reported a stale, pre-filter FromGatewayRequest count alongside the post-filter Total — reproducing the exact self-contradicting-audit-trail defect class round 3 already fixed once, but only for the CSR-fallback count (fromCsrKept), not the gateway count. With SubmitNonDnsSans=false and a gateway SAN dictionary mixing DNS and non-DNS entries, the line could read "Resolved 1 SAN(s) ... FromGatewayRequest=3" — 3 does not reconcile to 1. Fixed by computing the gateway count post-filter too: gateway- and CSR-sourced entries are mutually exclusive by construction (the CSR fallback only ever runs when the gateway supplied nothing at all), so `result.Count - fromCsrKept` is exactly the right post-filter gateway count. Removed the now-fully-superseded pre-filter `fromGateway` variable. 2. ExecuteWithRetryAsync's cancellation check (added round 5) throws before any caller reaches its own per-call audit line (Method/Path/HttpStatus/ LatencyMs). A DCV-timeout cancellation landing mid-flight on a CERTInext call therefore left no per-call record anywhere — only a coarser, order-level "unexpected failure" log with no domain/endpoint/status/ latency, since the per-domain cancellation catches in PerformDcvIfNeededAsync deliberately re-throw without logging (to avoid mislabeling a timeout as a per-domain failure). Fixed by logging Method/Path/HttpStatus/ResponseStatus/LatencyMs right at the cancellation-detection point inside ExecuteWithRetryAsync itself — the one place that reliably sees every cancellation regardless of which of its ~10 callers is in flight — before throwing. Also applied the one endorsed advisory: SanSubmissionTests.cs's two CSR builders (GenerateCsrPem, GenerateCsrPemWithGeneralNames) duplicated ~20 lines of BouncyCastle CSR-construction boilerplate; GenerateCsrPem is now a one-line delegator to GenerateCsrPemWithGeneralNames. Tests: added one regression test for the stale-count fix, pinning the payload-level data the log line is computed from (only the DNS entry survives SubmitNonDnsSans=false filtering out of a 3-entry mixed gateway dict) — there is no log-capture seam in this codebase (ILogger comes from a fixed LogHandler.GetClassLogger() field, not an injectable dependency), so the log line's exact text cannot be asserted directly, and the cancellation- logging fix has no independently observable test surface for the same reason (round 5's existing cancellation test already covers the only externally-visible behavior — the exception type — unchanged by this fix). Release, no-DCV: 199/199. Release, DCV: 229/229. Zero code warnings in both. * fix(enroll): address full-review round 9 — TXT cleanup unbounded in aggregate across domains One confirmed root cause (found independently by two lenses), continuing the round 6→7 pattern: round 7 fixed each cleanup call's own timeout bound, but running those calls one after another meant the bound was per-call, not in aggregate. Both the early-exit cleanup (CleanupPartialStagingAsync) and the routine, always-runs finally-block cleanup iterated staged domains sequentially. A UCC/multi-SAN order (the exact feature this diff exists to support) with N staged domains could hold the calling request open for up to N x CleanupValidationTimeoutSeconds if the DNS provider was merely slow — not even hung — on every delete: a realistic degraded-provider condition, not a contrived one. For a large SAN count this can exceed DcvTimeoutMinutes itself, contradicting the method's own SOX CC7.3 "the entire DCV flow is hard-timeout-bounded" comment for exactly the multi-domain case this whole fix chain has been hardening. Fixed by running the per-domain cleanup calls concurrently (Task.WhenAll) instead of sequentially, at both call sites. Each call keeps its own independent 60s bound from round 7; running them concurrently means the wall-clock time for the whole batch is bounded by the slowest single call, not the sum — these are independent per-domain operations on different hostnames/records with no shared mutable state, so there is nothing for concurrent execution to race on. Also applied both endorsed advisories: three new test-only IDomainValidator/ IDomainValidatorFactory implementations (PartiallyFailingDomainValidator, TokenCapturingDomainValidator, SelectiveDomainValidatorFactory) each re-implemented boilerplate the pre-existing FakeDomainValidator/ FakeDomainValidatorFactory already provided. Extended those two shared fakes instead (ShouldFail predicate + CleanupTokens capture on the validator; an optional resolvableDomain filter on the factory) and deleted the three duplicates, updating call sites. Tests: added a timing-based regression test proving cleanup for 3 domains completes in close to one cleanup delay's worth of wall time, not three — verified it actually catches the regression by temporarily reverting the fix locally (confirmed FAIL at ~4.5s) before restoring it (confirmed PASS at ~3s), so the threshold is proven discriminating, not just a number that happens to pass. Extended FakeDomainValidator with a configurable CleanupDelay to make this possible; its two List<T> fields needed a lock now that cleanup calls can genuinely run concurrently (StagedRecords did not, since nothing awaits with a real yield point before writing to it). Release, no-DCV: 199/199. Release, DCV: 230/230. Zero code warnings in both. * fix(enroll): address full-review round 11 — check-after-await cancellation race, Subject sanitization (issue 0008) Round 11 confirmed both round-10 out-of-scope dispositions for real (both landed in safelyDismissed — the Sync N+1 pattern and the broader Subject-unsanitized claim are genuinely pre-existing, untouched by this diff) and surfaced one new, real defect plus one endorsed simplification. 1. ExecuteWithRetryAsync (round 5) checked ct.IsCancellationRequested / called ct.ThrowIfCancellationRequested() BEFORE checking whether the just-completed HTTP call actually succeeded. A CancellationTokenSource's timer callback and the awaited HTTP task's completion are not mutually synchronized, so it's possible for the call to genuinely succeed (the response already fully arrived) while `ct` independently flips to cancelled in the same instant — a real check-after-await race, not a fabricated one. Hitting it discarded a genuine CERTInext success and reported OperationCanceledException instead: for VerifyDcv specifically, that would abort PerformDcvIfNeededAsync's loop before WaitForDcvVerificationAsync ever ran, and its finally block would delete the just-staged TXT record even though CERTInext had genuinely received the verify trigger — a self-inflicted DCV failure out of an actual success. Fixed by checking resp.IsSuccessful (or the 4xx client-error case) BEFORE the cancellation check, so a call that completed successfully is returned regardless of the token's state at that instant. Only a call that did NOT succeed goes on to ask "was that because of cancellation?" No dedicated regression test: reproducing this exact race deterministically requires the HTTP response to fully complete before the cancellation timer fires by mere ticks — round 5's own test already shows a pre-cancelled token makes RestSharp report the call as Aborted, not Successful, so a straightforward pre-cancel test cannot exercise this specific ordering. A test that could would need either a flaky real-timing race or refactoring the HTTP-call/cancellation-check split into an independently testable unit, which is more machinery than this ordering fix warrants. 2. Issue 0008 (filed after round 10, per user request): Subject={Subject} was logged raw at ~13 sites across Enroll's own audit log, Revoke, Synchronize, and RenewOrReissueAsync — all pre-existing, confirmed untouched by this diff, and confirmed pre-existing again by round 11's adjudicator — but folded into this PR anyway per explicit instruction rather than left for a separate PR. Wrapped every site in LogSanitizer.Strip, plus the SANs={SANs} (sanSummary) argument on the "Enrollment attempt started" line, which had the identical unsanitized- raw-dictionary gap for the same reason. Also applied the one endorsed advisory: BuildSanList repeated the exact `LogSanitizer.Strip(string.Join("; ", X.Select(...)))` SAN-formatting expression three times; extracted a local FormatSans(...) helper alongside the method's existing Add(...) local-function pattern. Release, no-DCV: 199/199. Release, DCV: 230/230. Zero code warnings in both. * fix(enroll): don't report GENERATED with no certificate body CERTInext can mark an order auto-approved (certificateStatusId 15) before the certificate bytes are actually generated. The immediate GetCertificate after order placement then fails, but the legacy client still reported Status=issued with Certificate=null, and BuildEnrollmentResult trusted that status over the missing body — handing the gateway framework a GENERATED result with no PEM, which crashes CertificateConverterFactory.FromPEM downstream (confirmed against a live support escalation, UCSD order 5435716354). Demote GENERATED to EXTERNALVALIDATION whenever the certificate body is missing, matching the invariant PickUpEnrolledCertificateAsync already enforces on its own GENERATED branch. * docs(changelog): trim 1.0.1 entries to plain, concise bullets The 1.0.1 section had ballooned into multi-sentence paragraphs per bullet. Cut each down to the essential fact for a customer skimming release notes; no information dropped, just the padding.
docsource hadn't been touched since v1.0.0; a full freshness pass against current source found several stale/wrong claims and fixed them: - Order Lifecycle status-code table didn't match StatusMapper.cs (several codes were in the wrong bucket, several real codes weren't listed). - GroupNumber description omitted its per-order use (delegationInformation), contradicting a note three lines below it. - AutoApprove and DefaultProductCode were documented as doing things the code doesn't do; corrected to describe actual behavior (both filed as separate GitHub issues, not fixed here). - ~15 real config properties (OrganizationNumber, TechnicalContact*, SubmitNonDnsSans, PickupRetries/PickupDelay, DcvWaitFor*Seconds, DcvSyncMax*, etc.) existed in code with no mention anywhere in docs. - Enrollment/sync sequence diagrams in architecture.md were silent on DCV and synchronous certificate pickup entirely. - development.md's product test-coverage table cited a removed test file and hardcoded requestNumbers documented elsewhere as non-portable; replaced with a pointer to `make probe-products` and TESTING.md.
…sibility (#28) fix(enroll): renewals ignore template product code, correct AutoApprove UI text, log config presence Three independent fixes found during UCSD triage (issues #25, #26, #27): - RenewCertificateAsync built every renewal order from the connector's DefaultProductCode alone, ignoring the template's own ProductCode/ProfileId entirely. Threaded the template's code through RenewCertificateRequest.ProfileId, falling back to DefaultProductCode only when the template doesn't have one (using a blank-check, not ??, since EnrollmentParams.ProductCode never returns null — the same dead-fallback bug that made DefaultProductCode a no-op for new enrollments). - AutoApprove's UI text claimed the plugin attempts automatic approval of pending certificates; no such call exists anywhere in the code. Corrected to say so plainly. - OrganizationNumber, DefaultProductCode, and GroupNumber had zero log visibility, which is what made a stuck-pending-orders question undiagnosable from a support log. Added presence flags to the plugin-initialized log line.
…logging fixes to 1.0.1
…sponse PlaceOrderAsync logs the serialized GenerateOrderSslRequest (including additionalDomains) and TrackOrderAsync logs the raw response body (including domainVerification). Enables UCC SAN debugging when the gateway log level is set to Trace.
Lets the template specify subscription validity directly in years (1/2/3) via the ValidityYears enrollment parameter, bypassing the days-to-years ceiling conversion. Precedence: ValidityYears > ValidityDays > config default.
Implements an opt-in V2 API code path (disabled by default) alongside the existing V1 path. V1 behaviour is fully preserved when UseV2Api=false. Key changes: - Constants.ApiV2 / Constants.ConfigV2 inner classes for all V2 paths and config keys; EnrollmentParam gains ProductFamily and ProductVariant. - CERTInextConfig gains UseV2Api (bool, default false), ApiUrlV2, ClientId, ClientSecret; all V2 annotations wired into GetCAConnectorAnnotations / GetTemplateParameterAnnotations. - V2 DTO layer: CertificateRequestV2.cs and CertificateResponseV2.cs in CERTInext/API/V2/ covering create-order, track, download, revoke, auth/me, and OAuth2 token (flat access_token shape). - StatusMapper: V2StatusToRequestDisposition maps all six V2 status strings; ToV2RevocationReason maps CRL reason codes to V2 string values. - ICERTInextClient: nine new V2 method signatures (PingV2Async, PlaceOrderV2Async, SubmitCsrV2Async, TrackOrderV2Async, DownloadCertificateV2Async, RevokeOrderV2Async, GetAuthMeV2Async, ResolveAndTrackOrderV2Async, ResolveAndDownloadCertificateV2Async). - CERTInextClient: OAuth2 client_credentials token fetch (form-encoded POST, cached with 60s early-refresh buffer, SemaphoreSlim-protected); all nine V2 REST methods; product-family resolver (probes SSL → PrivatePKI → Signature); RFC 7807 problem+json error extraction; Idempotency-Key and X-Product-Code header injection. - CERTInextCAPlugin: if/else dispatch on _config.UseV2Api in Ping, Enroll, GetSingleRecord, Revoke; Synchronize logs a warning and stays on V1 (V2 /reports/orders returns 501). - Tests: 51 new V2 unit tests (WireMock client tests, Moq plugin dispatch tests, StatusMapper theory tests, integration stubs gated behind CERTINEXT_USE_V2_API=1). Full suite: 256 tests, 0 failures. - Build: 0 warnings, 0 errors (net8.0 and net10.0).
- Read CERTINEXT_CLIENT_ID/CLIENT_SECRET/API_URL/PRODUCT_CODE/DCV_DOMAIN from ~/.env_certinext_v2 (was using non-existent CERTINEXT_V2_* prefix) - Lifecycle test: remove immediate-revoke step (order is pending-csr, not issued; revoke requires issued state — tested separately with DCV) - Sync test: gate on _v2Enabled; accept V1 API errors as proof V1 path ran; assert exception does NOT contain "V2 product family" to catch routing bugs - Do not load ~/.env_certinext alongside _v2 file — both define CERTINEXT_API_URL and mixing them sends V1 sync to the wrong host
…tional signerIp Gap 1: add GetDcvV2Async / VerifyDcvV2Async to ICERTInextClient + CERTInextClient. V2DcvChallengeResponse, V2DcvVerifyRequest, V2DcvVerifyResponse DTOs added to CertificateResponseV2.cs. PerformDcvV2IfNeededAsync added to plugin under #if SUPPORTS_DCV; called inline from EnrollV2Async (pending-dcv orders) and GetSingleRecordV2Async (single-record refresh). TXT prefix: _emudhra-challenge. Gap 2: add ChainPem (List<string>) to V2CertificateDownloadResponse. New AssembleV2CertChain helper concatenates leaf + intermediates leaf-first; used in EnrollV2Async and GetSingleRecordV2Async. Gap 3: add IssuedAt / ExpiresAt to V2OrderStatusResponse (ISO 8601, nullable); logged at Debug in GetSingleRecordV2Async when present. Gap 4: add JsonIgnore(WhenWritingNull) to V2AgreementParams.SignerIp and SignerPlace; EnrollV2Async passes null instead of empty string when fields are not configured so the properties are omitted from the serialised request body. 10 new unit tests (WireMock + Moq): GetDcvV2Async, VerifyDcvV2Async (200, 204, 422), dns-txt body assertion, chainPem deserialisation (with/without), chain assembly (with/without intermediate).
…d, Revoke, DCV, and chain assembly Adds GetProductDetailsV2Async to ICERTInextClient/CERTInextClient (GET /api/certinext/v2/catalog/products) with a tolerant parser that handles both bare-array and wrapped-object API responses. Adds Constants.ApiV2.CatalogProductsPath. New tests in V2ApiTests (all [SkippableFact], gated on _v2Enabled): - GetProductDetails_V2_ReturnsProducts: calls GetProductDetailsV2Async, asserts non-empty - GetSingleRecord_V2_ReturnsOrderDetails: places order, calls ResolveAndTrackOrderV2Async - Revoke_V2_IssuedOrder: gated on CERTINEXT_V2_ISSUED_ORDER_ID; revokes and re-tracks - DcvFlow_V2_PublishesAndVerifies: gated on SUPPORTS_DCV + CF creds; real DNS round-trip - ChainPem_V2_IsAssembled: gated on CERTINEXT_V2_ISSUED_ORDER_ID; asserts PEM shape Refactors: BuildStandardOrderRequest helper eliminates duplicated order-payload construction; Lifecycle_V2_EnrollTrackRevoke uses it instead of inline literal.
…, cancellation) - Dispose tempClient in ValidateCAConnectionInfo and ValidateProductInfo finally blocks - Throw InvalidOperationException in AssembleV2CertChain when CertificatePem is null but ChainPem is non-empty - Fix PerformDcvV2IfNeededAsync to always run CleanupValidation when staging succeeded (staged flag + single try/finally) - Propagate gateway cancellation from PerformDcvIfNeededAsync challenge-wait loop instead of swallowing it - Add familySlug parameter to GetDcvV2Async/VerifyDcvV2Async; expose ResolveAndTrackOrderV2WithFamilyAsync so GetSingleRecordV2Async passes the real resolved family to DCV rather than hardcoded FamilySsl - Add CancellationToken parameter to PickUpEnrolledCertificateAsync and wire it through to GetCertificateAsync and Task.Delay calls - Add IDisposable to CloudflareDomainValidator/Factory test helpers; guard opt-in destructive test flags from env-file auto-promotion
- Move profileId guard before CERTInextClient allocation in ValidateProductInfo so the finally/Dispose path is never skipped - Add LogV2ApiFailure (structured logging) to ThrowOnV2Failure, matching V1's LogApiFailure pattern for SOX/SOC2 audit parity - Wrap PlaceOrderAsync trace payload through RedactCredentials
…nfo ClientSecret scrub RevokeOrderV2Async had no explicit NotFound→KeyNotFoundException conversion before ThrowOnV2Failure, so revocations of orders in FamilyPrivatePki or FamilySignature (UseV2Api=true) propagated a generic Exception instead of being caught by the family-probe loop in RevokeV2Async. ValidateProductInfo's finally block omitted tempConfig.ClientSecret = string.Empty, leaving the V2 OAuth credential live in the transient config until GC, while ValidateCAConnectionInfo's identical block already zeroed it.
…erName/Email
EnrollV2Async received the CSR parameter but never called SubmitCsrV2Async, so every V2 order
stayed permanently at pending-csr and no certificate was ever issued. Added the PUT /{family}-
certificates/{orderId}/csr call immediately after PlaceOrderV2Async, followed by a TrackOrderV2Async
re-read so the post-CSR status drives the rest of the enrollment flow.
RequesterName and RequesterEmail were passed raw to LogInformation on the enrollment audit entry
while all other caller-supplied fields (subject, SANs) already used LogSanitizer.Strip, enabling
CRLF log injection via the Command enrollment API.
Unit tests updated to mock the two new client calls now required by the V2 enrollment path.
…ewCertificateAsync BuildOrderRequestFromLegacyEnrollRequest populates both fields on new enrollments (Sectigo-parity work on this branch). RenewCertificateAsync was building GenerateOrderSslRequest without them, causing renewal orders to route to the wrong account group and miss the configured technical contact. Replicate the same fallback logic from the enroll path (config fields → requestor defaults). Fixes F5.
…tificateAsync catch The inner catch (Exception ex) in the poll loop swallowed OperationCanceledException/TaskCanceledException, violating the .NET cancellation contract. All current callers pass CancellationToken.None so this was latent, but a future refactor passing a real token would silently consume cancellation. Add `if (ex is OperationCanceledException) throw;` before the log-and-continue path. Fixes F6.
…econds WaitForDcvVerificationAsync (V1) and the post-verify TrackOrderV2 loop in PerformDcvV2IfNeededAsync (V2) both used DcvPropagationDelaySeconds as their polling interval. That config is a one-shot DNS propagation wait (often set to tens or hundreds of seconds), not a loop cadence — reusing it leaves only ~2 polls before the 5-minute timeout expires. Switch both loops to Constants.Dcv.SyncPropagationDelaySeconds (3 s), matching the fixed-cadence pattern already used in WaitForIssuanceAfterDcvAsync. Fixes F4 (V1 + V2).
The V1 path acquires a ConcurrentDictionary key via _dcvInFlight before staging a TXT record and releases it in finally. V2 had no equivalent guard: concurrent enrollment + sync pickup for the same order could both enter PerformDcvV2IfNeededAsync and double-stage the TXT record. Add TryAdd at entry (mirroring TryRunDcvDuringSyncAsync) and TryRemove at each early exit (challenge-fetch failure, no token, no validator) and in the staged finally block so the guard is released on every code path. Fixes F2.
…sient failure After PlaceOrderV2Async + SubmitCsrV2Async both succeed, TrackOrderV2Async was called with no try/catch. A transient 5xx or timeout threw before any EnrollmentResult was returned, so Command never recorded a CARequestID and the order was permanently untracked. Wrap the call in try/catch; on failure log a warning and return a pending EnrollmentResult with CARequestID = orderId. The order is already placed in the CA; the next sync will resolve the status. Fixes F1.
The V2 API (V2CreateSslOrderRequest) only supports a single domain field plus autoSecureWww for the www. variant. A CSR with additional DNS SANs would either be silently dropped or produce an opaque CA-side rejection with no actionable error surfaced to the operator. Add upfront validation in EnrollV2Async (before PlaceOrderV2Async) using the existing BouncyCastle ExtractSanEntriesFromCsr helper. If DNS SANs beyond the primary domain and www.<domain> are present, return an immediate FAILED result with a descriptive StatusMessage. Documents the CA API limitation in issues/f3-v2-multi-san-limitation.md. Fixes F3.
…e/chain tests LoadEnvFile was seeding process env last, so the V1 CERTINEXT_API_URL (promoted by IntegrationTestFixture with /emSignHub-API suffix) overwrote the V2 base URL from ~/.env_certinext_v2, causing 404 on /oauth/token. Invert merge priority so V2-file-defined keys always win, then force-promote them back into process env. Add s_lastCreatedOrderId static so Lifecycle_V2_EnrollTrackRevoke can pass its order ID to Revoke_V2_IssuedOrder and ChainPem_V2_IsAssembled. Both tests fall back to the lifecycle ID when CERTINEXT_V2_ISSUED_ORDER_ID is absent, and skip gracefully on sandbox-timing 422s instead of hard-failing.
…ecycleTests from no-DCV build Prepares the test project for new V2 plugin-level lifecycle tests: the bulk test's opt-in flag must not be auto-promoted from ~/.env_certinext, and the upcoming V2DcvLifecycleTests.cs file (uses IDomainValidatorFactory) must be excluded from the IAnyCAPlugin 3.2.0 no-DCV build like its V1 counterpart.
Adds a product-code presence check to GetProductDetails_V2_ReturnsProducts and a links.self.href check to Lifecycle_V2_EnrollTrackRevoke. Both are logged diagnostically rather than hard-asserted: a live run against the sandbox showed neither field is currently populated by the V2 API for this account (see issues/0016), so a hard assertion would make the suite flaky on an environment limitation rather than a plugin regression.
…ValidityDays/365)) Port of 5a3930a. Both template fields default to one year, so ValidityYears winning outright silently shortened templates saved with only ValidityDays (for example 730 days became 1 year). Each field now counts only when set and the larger term wins; neither set falls back to SubscriptionValidityYears. ValidityDays is no longer described as deprecated (V1 only; V2 uses ValidityYears alone). Also adds the CERTInextClient.OverrideLoggerForTests seam and the shared LoggingStateCollection that the ported 1.0.x tests use to capture client logs.
…lection Port of 91ad175. The named collections "LogHandlerFactory-NoParallel" had no CollectionDefinition, so xUnit only serialized the classes within each name and still ran them in parallel with every other class, racing on the process-wide LogHandler.Factory (an ObjectDisposedException surfaced once more log-capturing tests were added). Move them into LoggingStateCollection, which disables parallelization.
…k when values are blank Port of 797f784, 50637b4, d16c67b and 8c6499c from the 1.0.x line. - V1 agreementDetails now resolves each signer value as template parameter, then connector value, then built-in default, with blank (not just null) checks, for both new enrollment and renewal. The SignerName/SignerPlace/ SignerIp template parameters previously took effect on V2 only and the V1 builder ignored them; the "V1 wire behaviour intentionally unchanged" comment is removed with the old builder. - A blank template ProductCode now falls back to DefaultProductCode on new V1 enrollments (the template resolves to "", so the null-coalesce never fired). - A Warning is logged when SignerName/SignerPlace fall back to the "Keyfactor Gateway"/"Gateway" placeholders, and when SignerIp is not an IP address (the value is still sent unchanged). - Config comments, manifest enrollment_config, docs and the migration guide no longer describe the signer template parameters as V2 only. Tests: 729e834, 414a115, f1360b1, 18af44a and the unit parts of 8847142.
…ls after the order is placed Port of 6baa5d3 (tests d951a1d). PlaceOrder has already created (and billed) the CERTInext order when the follow-up TrackOrder throws, so Enroll failed without Command learning the order number and an operator retry could place a duplicate paid order. TrackOrder failures after placement (V1 client enroll and renewal) are now logged as a Warning naming the order and return a pending result carrying the order number. A real caller cancellation still propagates; a spurious timeout cancellation does not. The in-call DCV / issuance wait in EnrollNewAsync had the same exposure and is absorbed the same way; the sync-DCV retry path completes the order. Three existing DCV tests that asserted the exception propagated out of Enroll now assert the pending result instead.
…hold (V1) Port of 2f222c6. The V1 client never downloads a revoked certificate's body (CERTInext refuses it and IsCertificateDownloadable excludes status 22), so V1 Synchronize and GetSingleRecord emitted every revoked order with Certificate = null. For an order the gateway never stored with a body, the gateway persists a row with RevocationDate set and no certificate, never clears that date, and its revoked-certificate search then fails on every Command scan of the CA. Apply the existing DecideBodylessRevokedRecord guard (until now V2 only) to V1: a date from GetExpirationDateByRequestId (gateway holds a body) emits REVOKED unchanged; null (row, no body) emits FAILED with no revocation data; no row, an error or no reader makes Sync skip the record and GetSingleRecord return FAILED. The V1 sync summary now reports FailedFromBodylessRevoked and BodylessRevokedSkipped, and the guard's docs and log text no longer say V2. Rows already written this way need the documented gateway DB repair.
… s on V1 (issue 0090) Port of 0df1a53. The V2 branch already held back recent pending/issued orders in the V2 sync and documented the setting as applying to both modes; the V1 sync did not. V1 Synchronize (full and incremental) now skips a pending or issued order younger than SyncRecentOrderGraceSeconds (default 120, 0 disables, negative = default) plus the 5 s clock margin, before any DCV, refetch or download, and counts the holds in the summary. The V2 helpers are renamed to neutral names and shared: GetSyncRecentOrderGraceSeconds, ShouldDeferRecentOrder, and the grace/margin constants move from ApiV2 to Api. GetSyncIssuedAfter moves the V1 incremental listing start back by the hold window plus 300 s of slack, because the V1 listing filters orderDateFrom by UTC calendar day and a held order created just before midnight would otherwise never be re-listed. V2 is unaffected (V2SyncLookbackHours already covers it). Architecture docs now describe the V1 hold-back and the body-less revoked guard (2f222c6). Tests: CERTInextCAPluginSyncRecentOrderTests and the read-only OrderDateFormatProbeTests (opt-in).
Port of b5da885. The V1 order report filters only by calendar day (orderDateFrom = yyyy-MM-dd), so every incremental pass re-listed every order placed that day and refetched each issued or revoked one (TrackOrder + DownloadCertificate) even when the gateway already held it. A customer gateway with ~225 orders in the day made 451 serial calls per 10-minute pass. On incremental passes, skip an order listed as GENERATED or REVOKED when ICertificateDataReader.GetStatusByRequestID returns the same status and GetExpirationDateByRequestId shows a stored body. New orders, status changes, rows without a body, no reader and reader errors are processed as before. Pending orders and full sync are unchanged; the summary reports UnchangedInGateway. The V2 sync is deliberately not changed here.
…veRequestData is on Port of 9662882 (tests 6364e4b), extended to V2. meta.errorMessage / legacy message were logged and thrown verbatim while the adjacent body was redacted, so an email echoed by CERTInext leaked into logs and into the exception message Command stores. Add LogSanitizer.SanitizeCaText (email tokens masked via MaskEmail, CR/LF/tab stripped, flag on = unchanged) and apply it in LogApiFailure, ExtractErrorMessage (new fail-closed logSensitiveRequestData parameter) and every thrown message built from meta.ErrorMessage. IsRateLimitSurface and the duplicate-txn check still see the raw text. V2 addition: ExtractV2ErrorMessage, which folds RFC 7807 title/detail/errors[] into the exception message, gets the same fail-closed parameter and masking, and its four call sites (revoke 422, DCV verify 422, 403, generic) pass the flag. New V2ErrorTextMaskingTests cover it.
…o not leak value tails Port of e0caa5c (tests 50c8e19). RedactCredentials and RedactJsonField matched JSON string values with [^"]*, so a value such as "ab\"cd" ended at the escaped quote and leaked the rest (the key=value form pattern had the same problem). The patterns now consume backslash escapes as part of the value.
…s unless LogSensitiveRequestData is set Port of 08132d4 (tests 89a48fe). URI SANs were logged verbatim, so a mailto: address or user:pw@ userinfo reached the gateway log. LogSanitizer.MaskUri masks the opaque-URI address and replaces authority userinfo with ***; plain URIs and the flag-on case are unchanged. Applies wherever FormatSans is used, including the V2 SAN log lines.
…client Port of 51cfbb0 (tests 1d69bef, 46cd930), applied to the V2 trace sites too. The PlaceOrder request and TrackOrder response payload dumps (and LogApiFailure) computed ApplyLoggingRedaction eagerly even with the level disabled, and a full sync calls TrackOrder thousands of times. They now check Logger.IsEnabled first; output when enabled is unchanged. V2: the PlaceOrderV2 request/response dumps and LogV2ApiFailure get the same guard. Also adds the 1.0.x ClientPayloadLogRedactionTests and V1NonSuccessLogRedactionTests, which pin the redaction at the real client log call sites (including the poc* keys) and were missing from this branch.
Port of 32caf31 (tests a47fa3b). PerformDcvIfNeededAsync now returns a DcvOutcome. When some pending domains are skipped (no DNS provider, stage failure, non-FQDN entry), the staged domains are still verified and cleaned up, but Enroll no longer holds the worker in WaitForIssuanceAfterDcvAsync for an order that cannot issue; sync DCV retries later. Sync/refresh callers keep their existing semantics.
…ion when DCV is enabled Port of 89d6db6 (tests 8dc1ebe, 392e5ae). The V1 renew path never runs an in-call DCV, so on a DCV-enabled deployment a renewal still waiting on DNS-01 validation cannot issue inside the pickup window and polling it only holds a gateway worker for a guaranteed miss. One TrackOrder call detects a non-terminal renewal order with a pending DNS-01 domain and skips pickup, returning the pending result for the sync-driven DCV retry to complete. Every other renewal (validation reused, no DCV pending, DCV disabled) keeps pickup, and the check never throws. The pending-DNS-domain filter is shared with PerformDcvIfNeededAsync as IsPendingDnsDcvEntry.
…ported 1.0.x fixes - CHANGELOG: insert the 1.0.2 and 1.0.1 sections verbatim from 1.0.2-rc.2 between 2.0.0 and 1.0.0. Reword the 2.0.0 bullets that restated 1.0.1 fixes so 2.0.0 lists only V2-era changes, and add entries for what this branch gained beyond the 1.0.x sections: the 75 s pickup ceiling, V2 error-text masking, V2 Trace guard, escape-aware redaction, URI SAN masking. Note the V2 unchanged- order skip as a known limitation. - integration-manifest.json: required/example/default on every ca_plugin_config entry (7b22876, f05605e, 684e469, 011da0a, 01f1733), with Conditional where the requirement depends on UseV2Api (AccountNumber, ApiKey, OAuthClientId, OAuthClientSecret, SignerPlace) and values for the V2-only entries. Existing \uXXXX escapes are untouched. - overview.md: port the "invalid custom extension or certificate policy OIDs" troubleshooting entry, noting that the 120 s hold-back makes the race less likely, not impossible; update the Status=90 causes (75 s pickup cap, DCV partial skip, renewal pending DNS-01). - architecture.md: post-placement failures return pending, DCV partial skip, and the renewal pickup skip.
…tCode The V2 spec makes X-Product-Code an optional override: CERTInext's wrapper resolves the catalog product from productVariant/variant, whether certificate.domain starts with "*.", and whether additionalDomains is non-empty. Send the header only when the template explicitly sets ProductCode (or the ProfileId alias), for SSL/TLS and Private PKI. - Connector DefaultProductCode no longer pins V2 orders (V1 unchanged). - Remove the client guard that rejected a blank code and the plugin "no product code resolved" backstop. - Stop rejecting an ambiguous catalog match on V2 (enroll and template validation); template validation makes no catalog call without an explicit code. Private PKI no longer requires ProductCode. - Without an explicit code, wildcard = domain starts "*.", UCC = the template product's type or extra DNS SANs beyond the primary domain (www. and a wildcard's apex excluded); no catalog call. With an explicit code the catalog still decides UCC/wildcard. - Capture resolvedProductCode from the create response and log it. - Rewrite docs, config text and manifest that claimed an unpinned create resolves to a PFX product that never issues.
The V2 spec's Agreement schema (used by both the SSL and Document Signer create bodies, and the accept-agreement body) is signerName, signerPlace and accepted; it has no signerIp. Remove the member from V2AgreementParams so the V2 body can never carry it, and mark the connector SignerIp setting and the template SignerIp parameter as V1-only in their annotations, the manifest and the docs. V1 behavior (127.0.0.1 placeholder, non-IP warning) is unchanged.
The V2 spec's emailNotifications enum is consent-only | all; the legacy "0"
the plugin sent for EmailNotifications=0 is not a member. Map the silent
setting to "consent-only" on SSL/TLS and Private PKI creates ("1" still maps
to "all", blank is still omitted). V1 is unchanged. Update the connector
annotation, manifest, docs and the golden/enrollment tests.
…ed-yet CERTInext can report an order as issued while Download Certificate still answers 422 EMS-1165 (the certificate is generated about 20 s later, and much later on the sandbox). That is an expected state, not a failure. - The client throws V2CertificateNotGeneratedException (same message as before) and logs that response at Debug instead of Warning. - Enroll: the immediate download and the pickup poll keep waiting, log per attempt at Debug and write one Info summary, then return pending; the attempts are not counted as poll errors. - GetSingleRecord logs it at Info; Synchronize logs it at Debug and counts IssuedNotGeneratedYet in the pass summary. Other download errors still warn. - Capture orderState and certificateState from Track Order and include them in those log lines (diagnostic only; no logic branches on them).
…g-agreement
When Track Order shows an SSL/TLS order at pending-agreement (the CA will not
release the certificate until the agreement is accepted), call
POST /ssl-certificates/{orderId}/agreement with
{"agreement":{signerName,signerPlace,accepted:true}} (AgreementAcceptRequest),
using the same resolved signer values as create.
- Client: AcceptAgreementV2Async, SSL family only (any other family throws
before a request is made).
- Enroll: at most once per order per call, checked after create, after DCV
and on every pickup-poll status read.
- Synchronize: at most once per order per pass, with the connector's
RequestorName and SignerPlace; a pending SSL order not already tracked
costs one extra Track call, and nothing is called when no signer is set.
- The intent is audit-logged before the call (signer name and place only
when LogSensitiveRequestData is on). A failure logs a warning and never
fails the enrollment or the sync.
Before placing a V2 OV or EV SSL/TLS order for the connector's
OrganizationNumber, read the organization's SSL.com validation status
(GET .../organizations/{org}/ca-validations/sslcom). If the order's level
is not validated, refresh it once (POST .../sslcom/sync) and re-check. If it
is still not validated and the new SslComSubmitOrganization setting (default
false) is on, the level isn't already requested, and SSL.com's readiness block
says the organization is ready, submit it (POST .../sslcom with levels,
subscriberAgreementAccepted:true and the optional SslComAdminEmail). That
accepts SSL.com's subscriber agreement on the organization's behalf, so the
intent is audit-logged before the call and the setting says so.
The step is advisory: a not-validated organization, a not-ready one or any
failed SSL.com call logs one warning (with what SSL.com reports missing;
emails masked) and the order is placed anyway. DV and Private PKI skip it.
The response model (V2SslComValidation) tolerates unknown fields, and
"validated for level" is decided defensively from per-level entries (status
word, validated flag, validTill); an unrecognized shape counts as not
validated. ValidateCAConnectionInfo rejects a non-boolean
SslComSubmitOrganization and a malformed SslComAdminEmail.
… the pinned templates
Stage 01 also upserts a "<ProductID> V2" gateway profile per product and stage 02
emits {ProductID:P, CertificateProfile:"P V2"} templates with no ProductCode, both
controlled by V2_TEMPLATES (default 1; 0 restores pinned-only). Command imports
templates as AnyCA_<CertificateProfile>, so the two sets coexist; stage 06 already
iterates every template in the tenant. Stage 01 CHECK now also verifies every
profile the run registered.
…ned V2 template Each V2 test that enrolls a product is now a SkippableTheory over V2PinMode: Pinned (ProductID = product name + ProductCode) and V2Template (same ProductID, no ProductCode, so CERTInext resolves the product). Tests use real product names instead of numeric ProductIDs to mirror the gateway templates. Env-order tests run only the pinned row; the bulk, Private PKI and experiment tests are not doubled.
…mitations Changed C# files now use the repo's 6-line Keyfactor Apache-2.0 header (as in CERTInext/Constants.cs), keeping each file's copyright year. CHANGELOG gains known limitations found live: catalog-dependent product resolution without a template ProductCode (sandbox DV resolves to 917), no DCV yet for SSL.com-validated domains, and Command's 60 s enrollment workflow limit.
…ll_V2_ReturnsCARequestID Moves the V2DcvLifecycleTests log routing into TestOutputLogging so other live tests can show the plugin's call sequence; CERTINEXT_TEST_LOG_LEVEL=Trace adds the redacted payloads. Used to diagnose the two-call 'Something went Wrong' CSR 422.
…sr=true) The two-call flow (create, then PUT .../csr) can't complete for a domain the account has already validated: CERTInext reports the order issued at create with csrSubmitted=false, then rejects the CSR with 422 'Something went Wrong.' (Finding 32). V2InlineCsr now defaults to true; false still selects two-call, and Private PKI stays two-call. Two-call failure-path and body-shape tests pin V2InlineCsr=false; integration tests use CERTINEXT_V2_INLINE_CSR=0 for two-call. Docs, manifest and CHANGELOG (with an upgrade note for connectors that have false saved) updated.
… V1-only pending-DV runbook Maps the CERTInext errors seen on the sandbox (EMS-1165 after issued, 917 resolution for unpinned DV, EMS-1164 at DCV, two-call 'Something went Wrong' on validated domains, token HTTP 0) to their causes and how to confirm each, and notes that CompleteAllPendingDvOrders is V1-only.
…mark them emSign-era The Product Codes section opens with the V2 ProductVariant values (ssl: dv/ov/ev with wildcard/UCC by request shape; private-pki: intranet-ssl/igtf-host), notes that a variant can resolve to an unwanted product (sandbox dv -> 1-month 917), and scopes the numeric tables to V1 and the optional V2 override. The tables are labelled eMudhra's published emSign-era reference values, unverified per environment and likely different for SSL.com-issuing accounts. The obsolete 'short-validity products aren't in the API' note is replaced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidates the V2 line into one branch: everything in #30 (
feat/certinextv2), the 1.0.x fixes it was missing, and alignment with CERTInext's current V2 OpenAPI spec (2.0.0). Supersedes #30.What's in it
feat/certinextv2docs rework merged with origin (the V2InlineCsr setting and the 0090 sync hold-back), plus the opt-in latency, inline-CSR and 45-day validity probes.poc*keys redacted.X-Product-Codeunless the template setsProductCode;resolvedProductCodeis logged.agreement.signerIpdropped (SignerIpis now V1-only).emailNotificationsis sent asconsent-only.EMS-1165afterissuedis treated as "not generated yet" (pending).pending-agreementgetsPOST /agreement.SslComSubmitOrganization, default off.V2InlineCsrdefaults totrue: the two-call flow can't complete for an already-validated domain. CERTInext reports the orderissuedat create, then rejectsPUT /csrwith 422 "Something went Wrong." The upgrade note covers connectors that havefalsesaved.<product> V2template set withoutProductCodenext to the pinned set.Testing
DcvSupport=trueandfalse. Unit tests: 1236/1236.DcvSupport=false, V2): Command enrollments on pinned and no-code templates were placed with the CSR inline, and a gateway sync kept them pending.Known gaps (tracked for a follow-up PR stacked on this one)
POST /ssl-certificates/{id}/reissue.privilege-withdrawn.prevettingTokenspelling.