Add support for EV, OV Wildcard, and DV Wildcard multi-name certificate products - #18
Open
bhillkeyfactor wants to merge 36 commits into
Open
bhillkeyfactor wants to merge 36 commits into
bhillkeyfactor wants to merge 36 commits into
Conversation
…te products CSC TrustedSecure EV, Multiple Names (7), CSC TrustedSecure OV Wildcard, Multiple Names (8), and CSC TrustedSecure DV Wildcard, Multiple Names (9) per CSC's certificate type table. Type 7 requires both EvCertificateDetails and SubjectAlternativeNames since it is EV plus multi-name; 8 and 9 only need SubjectAlternativeNames.
Introduce a step-oriented FlowLogger for tracing Enroll, Revoke, and Synchronize pipelines end to end, and add logging throughout RequestManager (previously had none) and CscGlobalClient (missing error/warning branches, wrong logger category on CscGlobalClient).
…s unset Only look up the optional additional-SAN-emails field when the domain control validation method is EMAIL, and use TryGetValue instead of the indexer so a missing/blank value no longer throws.
…no 'dnsname' key UC certificate enrollments with zero SANs supplied threw KeyNotFoundException from the sans["dnsname"] indexer. Use TryGetValue and treat a missing key as no SANs instead of throwing.
Registration/Renewal/Reissue/Revoke calls only handled CSC Global's 400 error-body case; any other failure status (e.g. 404) fell through to deserializing an empty/unexpected body, leaving Result and RegistrationError both null and causing a downstream NullReferenceException in RequestManager instead of a clear error.
Add null and bounds guards throughout the enrollment, revoke, and sync paths: CSC Global responses (Result/RegistrationError/Status may be null even on a 200), optional ProductParameters lookups that were using the strict indexer, CARequestID substring truncation, and config/dictionary lookups in Initialize and the CscGlobalClient constructor. Replaces silent NREs with clear, logged failures.
…/Keyfactor/cscglobal-caplugin into feature/ev-ov-dv-multiname-certs
Previously the CSC Global list request filtered to status=in=(ACTIVE,REVOKED) and SyncCertificates only forwarded GENERATED/REVOKED records, so any pending or failed request was never reported back to Command on any sync cycle. Command eventually treats a request it hasn't seen in recent syncs as outdated and tries to prune it, which can trigger an internal Command cleanup bug for requests with no staged private key. Now every request CSC Global returns is forwarded to Command each sync, with certificate content attached only when actually issued or revoked.
…ed/revoked" This reverts commit fa1b23e.
…ng sync CSC Global renamed its product line at some point; its sync/list API now returns current names (e.g. "CSC TrustedSecure DV") while this plugin's ProductIDs/GetCertificateType still use the legacy names it shipped with (e.g. "CSC TrustedSecure Domain Validated SSL"). Syncing a cert back set ProductID directly from CSC's current name, which never matched Command's Certificate Profile/Template mappings (keyed on the legacy names) for 7 of the 10 product types. Add a reverse lookup so synced certificates resolve to the same ProductID used during enrollment.
Drop the legacy product-name compatibility layer (GetCertificateType's duplicate/old-name cases, the CscCertificateTypeToProductId sync mapping) and use CSC's certificate type names directly everywhere: CSC TrustedSecure OV/OV Wildcard/OV Multiple Names/EV/DV/DV Wildcard/DV Multiple Names plus the three new EV/OV Wildcard/DV Wildcard Multiple Names products. There is no template mapping persisted in the gateway database, so no migration is needed, but existing Command Certificate Templates must have their Template Short Name updated to the new names.
TemplateSync defaulted to off and was never turned on, so template sync never ran. There is no reason to keep it opt-in, so always resolve synced certificates back to their CSC certificate type/template and drop the gateway registration setting.
…/Keyfactor/cscglobal-caplugin into feature/ev-ov-dv-multiname-certs
Matches the pattern used in hydrantid-caplugin: pass the resolved value through as-is rather than substituting a fallback string that looks like a real product name.
Add a CSCGlobalCAPlugin.Tests project modeled on hydrantid-caplugin's test setup (xUnit, Moq, coverlet). Covers RequestManager, FlowLogger, CscGlobalClient (via a fake HttpMessageHandler), and CSCGlobalCAPlugin's orchestration logic (Synchronize, Enroll, Revoke, GetSingleRecord, GetEndEntityCertificate, annotations). Rework needed for testability: - CSCGlobalCAPlugin now resolves its ICscGlobalClient through an internal ClientFactory delegate instead of constructing one eagerly in Initialize, so tests can substitute a mock. - CscGlobalClient gained an internal constructor overload that accepts an HttpMessageHandler, so HTTP calls can be intercepted without a real network client. - Added AssemblyInfo.cs with InternalsVisibleTo for the test project. Bugs found and fixed while writing tests: - MultiNameCertificateTypes was missing certificate type "6" (CSC TrustedSecure DV, Multiple Names), so that product never got SubjectAlternativeNames populated despite its name. - The prior-cert-serial-number extraction in Enroll() gated on the lowercase key "priorcertsn" but read the properly-cased "PriorCertSN", so priorSn was effectively never populated and Renew/Reissue enrollments always failed with "no prior certificate serial number was supplied". Also fixed a blocking .Result call in CscGlobalClient.SubmitCertificateListRequestAsync to use await.
The gateway host now runs on .NET 10 as well as .NET 6/8, so build the plugin against all three. The net8.0 package set (Keyfactor.PKI 8.1.1, IAnyCAPlugin 3.1.0) is reused for net10.0 since those libraries are forward-compatible with the newer runtime.
Existing Certificate Templates in Command may still reference the pre-1.2.0 CSC product names (e.g. "CSC TrustedSecure UC Certificate"). Add a legacy name -> canonical name alias table and resolve it in ValidateProductInfo and RequestManager.GetCertificateType so both old and new names continue to work during enrollment.
CSC Global rejects a multi-name registration if any subjectAlternativeNames entry is missing domainControlValidation. GetSubjectAlternativeNames could return a SAN with a null DomainControlValidation when the domain didn't match any address in "Addtl Sans Comma Separated DVC Emails" (or when that field wasn't set at all), causing enrollment to fail. Both the EMAIL no-match case and the CNAME case now mirror the primary CN's DCV method/ email instead of returning null or a hardcoded empty value.
Previously, Enroll failures gave the requester only a terse message (e.g. CSC Global's "Open order in progress"), with no indication of what the plugin actually attempted before hitting that error. Every failure path in Enroll now prepends flow.GetSummary() to StatusMessage, matching the pattern used in acme-provider-caplugin. Also: - An exception thrown mid-enrollment is now caught and returned as a failed EnrollmentResult (with flow summary + error detail) instead of bubbling up as an unhandled exception that Command has to wrap itself. - Fixed several early-return failure branches reporting Status = 30 (INPROCESS) instead of the correct FAILED status. - Fixed the reissue "one click not available" message incorrectly saying "Renew" instead of "Reissue".
LogHandler.FlattenException includes the full stack trace, which was getting embedded both in the flow summary's failed step line and in the trailing error text, making the message shown to the requester in Command needlessly long. Use e.Message for the flow step and the final error line; the full flattened exception still goes to the log.
CSC Global returns "price.total": null when a certificate cannot be reissued (e.g. it's not in an active status). Price.Total being a non-nullable decimal caused Newtonsoft to throw a JsonSerializationException while parsing the response, which surfaced to the requester as a raw deserialization error instead of the actual RegistrationError/order status CSC was trying to report.
CSC Global's renewal/reissue responses never include an issued certificate - the request is only submitted and still needs domain control validation (CNAME record or email confirmation) before CSC actually issues the certificate. GetRenewResponse and GetReIssueResult were unconditionally returning Status = GENERATED with no Certificate set, which crashed the gateway host (NullReferenceException in PemUtilities.PEMCertificateToDER) when it tried to parse a certificate that didn't exist. Both now return EXTERNALVALIDATION on success, matching new enrollment, and populate EnrollmentContext with the CNAME/email DCV instructions via a shared BuildDcvEnrollmentContext helper (previously only wired up for new enrollment). Also fixed GetRenewResponse never setting CARequestID on success.
The legacy-name backward compat tests only covered new enrollment and ValidateProductInfo. GetRenewalRequest, GetReissueRequest, and the full Enroll RenewOrReissue path had no test proving a Certificate Template still configured with a pre-1.2.0 product name resolves correctly.
The int-literal initializer was dead (every switch branch, including default, already assigns a Keyfactor.PKI EndEntityStatus value), and was the last place in the plugin where a status value wasn't backed by the library enum. Every Status assignment elsewhere already uses (int)EndEntityStatus.X.
Adds a data-driven test covering all 10 CSC certificate products with realistic domain scenarios, grounded in Sectigo's own Multi-Domain/UCC and Multi-Domain Wildcard product documentation (CSC resells Sectigo behind the scenes): SANs on the Multiple Names products are exercised with unrelated domains (not just subdomains of one base domain), and the wildcard multi-name types are exercised with wildcard SANs for unrelated domains and with the base-domain-plus-wildcard-SAN pairing Sectigo's docs call out as required for full coverage.
RequestManager.GetSubjectAlternativeNames looked up "Addtl Sans Comma Separated DVC Emails" (V/C swapped), but the field is registered and sent by Command as "Addtl Sans Comma Separated DCV Emails" per EnrollmentConfigConstants.AdditionalSansCommaSeparatedDcvEmails. The lookup never matched, so any SAN on a domain unrelated to the CN silently fell back to the CN's own DCV email - which has no authority to validate a different domain. Now reads the same constant used to register the field, so the two can't drift apart again. Also updated the one existing test that had (accidentally) encoded the same typo, matching the buggy behavior instead of the real field name.
Command's enrollment UI does not surface StatusMessage on a successful result - only EnrollmentContext (the DCV instructions dictionary) is shown. Renamed EnrichFailureWithFlowSummary to AttachFlowSummary and extended it to add a "Flow Summary" entry to EnrollmentContext on success, alongside whatever DCV entries came back, so the requester can see what the plugin actually did even when there's nothing to troubleshoot.
Command's enrollment UI renders EnrollmentContext as a bulleted list but doesn't respect embedded newlines within a single entry's value, so the multi-line flow summary was showing up as one unreadable run-on line. Added FlowLogger.GetSummaryEntries(), which returns one dictionary entry per step (plus an overview entry) instead of a single multi-line string, so each step naturally becomes its own bullet - using the UI's existing list rendering rather than trying to force line breaks into a string value.
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
1 participant
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.
Summary
Test plan