Conversation
The HID Global HydrantId AnyCA Gateway REST plugin extends the capabilities of HydrantId Certificate Authority Service to Keyfactor Command via the Keyfactor AnyCA Gateway. This plugin leverages the HydrantId REST API with Hawk authentication to provide comprehensive certificate lifecycle management. The plugin represents a fully featured AnyCA Plugin with the following capabilities:
* **CA Sync**:
* Download all certificates issued by the HydrantId CA
* Support for incremental and full synchronization
* Automatic extraction of end-entity certificates from PEM chains
* **Certificate Enrollment**:
* Support certificate enrollment with new key pairs
* Dynamic policy (profile) discovery from the CA
* Intelligent renewal vs. re-issue logic based on certificate expiration
* Support for PKCS#10 CSR format
* Configurable certificate validity periods
* **Certificate Revocation**:
* Request revocation of previously issued certificates
* Support for standard CRL revocation reasons
---------
Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io>
* feat: release 1.0 (#1) The HID Global HydrantId AnyCA Gateway REST plugin extends the capabilities of HydrantId Certificate Authority Service to Keyfactor Command via the Keyfactor AnyCA Gateway. This plugin leverages the HydrantId REST API with Hawk authentication to provide comprehensive certificate lifecycle management. The plugin represents a fully featured AnyCA Plugin with the following capabilities: * **CA Sync**: * Download all certificates issued by the HydrantId CA * Support for incremental and full synchronization * Automatic extraction of end-entity certificates from PEM chains * **Certificate Enrollment**: * Support certificate enrollment with new key pairs * Dynamic policy (profile) discovery from the CA * Intelligent renewal vs. re-issue logic based on certificate expiration * Support for PKCS#10 CSR format * Configurable certificate validity periods * **Certificate Revocation**: * Request revocation of previously issued certificates * Support for standard CRL revocation reasons --------- Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> * Merge 1.0.1 to main (#4) * feat: release 1.0 (#1) The HID Global HydrantId AnyCA Gateway REST plugin extends the capabilities of HydrantId Certificate Authority Service to Keyfactor Command via the Keyfactor AnyCA Gateway. This plugin leverages the HydrantId REST API with Hawk authentication to provide comprehensive certificate lifecycle management. The plugin represents a fully featured AnyCA Plugin with the following capabilities: * **CA Sync**: * Download all certificates issued by the HydrantId CA * Support for incremental and full synchronization * Automatic extraction of end-entity certificates from PEM chains * **Certificate Enrollment**: * Support certificate enrollment with new key pairs * Dynamic policy (profile) discovery from the CA * Intelligent renewal vs. re-issue logic based on certificate expiration * Support for PKCS#10 CSR format * Configurable certificate validity periods * **Certificate Revocation**: * Request revocation of previously issued certificates * Support for standard CRL revocation reasons --------- Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> * release: 1.0.1 --------- Co-authored-by: Brian Hill <76450501+bhillkeyfactor@users.noreply.github.com> Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> * Hydrant Failed Status Issues and Logging * fixed changelog * Add .NET 10 target framework support Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Change FlowLogger from LogTrace to LogDebug/LogWarning The Keyfactor gateway framework sets the Microsoft.Extensions.Logging minimum level above Trace, causing all LogTrace calls to be silently dropped before reaching NLog. Flow diagram and step logging now uses LogDebug (visible), and failure steps use LogWarning for visibility. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Revert FlowLogger back to LogTrace LogTrace works in the CSC Global plugin with the same gateway framework, so the MEL minimum level is not the issue. Reverting to match the established pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fixed package vulns --------- Co-authored-by: Keyfactor <keyfactor@keyfactor.github.io> Co-authored-by: Morgan Gangwere <470584+indrora@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Automated merge of release-1.0 (v1.0.2) into main, updating the HydrantId AnyCA Gateway REST plugin implementation, packaging metadata, and end-user documentation.
Changes:
- Adds
Enabledconnection flag plus extensive guard clauses and structured logging (FlowLogger) across plugin operations. - Refactors HydrantId client/request logic with more structured logging and expanded error handling.
- Updates docs/manifest/changelog and multi-targets the plugin build (net6.0/net8.0/net10.0).
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| integration-manifest.json | Updates release packaging fields and adds Enabled to CA connection config metadata. |
| docsource/configuration.md | Rewrites configuration/installation guidance and adds revocation reason documentation. |
| README.md | Updates repo naming/links and expands requirements/configuration documentation. |
| HydrantCAProxy/RequestManager.cs | Adds structured logging, guard clauses, and refactors mapping/helpers. |
| HydrantCAProxy/HydrantIdCAPluginConfig.cs | Introduces Enabled config constant/annotation and config property. |
| HydrantCAProxy/HydrantIdCAPlugin.csproj | Multi-targets frameworks and adds new package references. |
| HydrantCAProxy/HydrantIdCAPlugin.cs | Adds FlowLogger usage, more validation/error handling, and sync/enroll/revoke robustness changes. |
| HydrantCAProxy/FlowLogger.cs | New utility for step-based flow tracing/diagrams. |
| HydrantCAProxy/Client/HydrantIdClient.cs | Refactors HTTP calls with structured logging and improved non-success handling. |
| CHANGELOG.md | Replaces prior content with v1.0.2/v1.0.1/v1.0.0 entries reflecting this plugin. |
| .gitignore | Adds local Claude settings ignore and an additional ignored filename. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| certDataReader = certificateDataReader; | ||
| Config = configProvider; | ||
| var rawData = JsonConvert.SerializeObject(configProvider.CAConnectionData); | ||
| _logger.LogTrace("Initialize: raw config JSON: {Json}", rawData); |
There was a problem hiding this comment.
Initialize logs the full serialized CAConnectionData (raw config JSON), which can include secrets like HydrantIdAuthKey. This is a credential-leak risk if trace logs are enabled. Please remove this log line or redact secret fields before logging (e.g., log only non-secret keys / a masked value).
| _logger.LogTrace("Initialize: raw config JSON: {Json}", rawData); |
| if (_config == null) | ||
| { | ||
| flow.Fail("ConfigValidation", "Deserialized config is null"); | ||
| _logger.LogError("Initialize: _config is null after deserialization."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
If _config fails to deserialize, Initialize currently logs and returns without throwing. That can leave the plugin in a partially initialized state and cause later operations to fail in less obvious ways. Consider throwing an exception here so the gateway treats initialization as failed and surfaces the configuration error immediately.
| _logger.LogTrace("ValidateCAConnectionInfo: raw connectionInfo JSON: {Json}", rawData); | ||
|
|
There was a problem hiding this comment.
ValidateCAConnectionInfo logs the full serialized connectionInfo (raw connectionInfo JSON), which can include secrets like HydrantIdAuthKey. Please avoid logging secrets (even at Trace); log only non-secret fields or mask secret values.
| _logger.LogTrace("ValidateCAConnectionInfo: raw connectionInfo JSON: {Json}", rawData); | |
| var sanitizedConnectionInfo = new Dictionary<string, object>(connectionInfo); | |
| if (sanitizedConnectionInfo.ContainsKey(nameof(HydrantIdCAPluginConfig.Config.HydrantIdAuthKey))) | |
| { | |
| sanitizedConnectionInfo[nameof(HydrantIdCAPluginConfig.Config.HydrantIdAuthKey)] = "***"; | |
| } | |
| _logger.LogTrace("ValidateCAConnectionInfo: sanitized connectionInfo JSON: {Json}", | |
| JsonConvert.SerializeObject(sanitizedConnectionInfo)); |
| flow.Skip("Validation", "CA is disabled"); | ||
| _logger.LogWarning("The CA is currently in the Disabled state. It must be Enabled to perform operations. Skipping config validation..."); | ||
| _logger.MethodExit(); | ||
| return Task.CompletedTask; |
There was a problem hiding this comment.
When Enabled is false, connection validation is skipped, but other public operations (e.g., Enroll/Synchronize/Revoke) do not check _config.Enabled and will still run. If Enabled is intended to disable the connector, add a consistent guard in each public method (or centralize it) so the flag actually prevents CA operations.
| flow.Skip("Validation", "CA is disabled"); | |
| _logger.LogWarning("The CA is currently in the Disabled state. It must be Enabled to perform operations. Skipping config validation..."); | |
| _logger.MethodExit(); | |
| return Task.CompletedTask; | |
| flow.Fail("Validation", "CA is disabled"); | |
| _logger.LogWarning("The CA is currently in the Disabled state. It must be Enabled to perform operations."); | |
| throw new InvalidOperationException("The HydrantId CA connector is disabled. Enable it before performing validation or CA operations."); |
| <PackageReference Include="Keyfactor.PKI" Version="5.5.0" /> | ||
| <PackageReference Include="Newtonsoft.Json" Version="13.0.3" /> | ||
| <PackageReference Include="BouncyCastle.Cryptography" Version="2.6.2" /> | ||
| <PackageReference Include="System.Drawing.Common" Version="8.0.25" /> |
There was a problem hiding this comment.
System.Drawing.Common is referenced here but there are no System.Drawing usages in the codebase. This adds unnecessary dependency surface area, and System.Drawing.Common is problematic on non-Windows runtimes. If it’s not required, please remove it; if it is required, consider documenting the platform/runtime expectations and add the corresponding code usage in the PR.
| <PackageReference Include="System.Drawing.Common" Version="8.0.25" /> |
| The plugin supports the following standard CRL revocation reasons: | ||
|
|
||
| | Reason Code | Reason Name | HydrantId API Value | | ||
| |-------------|-------------|---------------------| | ||
| | 0 | Unspecified | `Unspecified` | | ||
| | 1 | Key Compromise | `KeyCompromise` | | ||
| | 2 | CA Compromise | `CaCompromise` | |
There was a problem hiding this comment.
This revocation-reasons table lists reason codes 0 and 2 (and names/values like Unspecified, CaCompromise), but the plugin implementation only supports reasons 1, 3, 4, and 5 (see RevocationReasons enum and GetMapRevokeReasons). Please update the documentation to match the supported reasons, or extend the implementation to support the additional reasons if the HydrantId API accepts them.
| The plugin supports the following standard CRL revocation reasons: | |
| | Reason Code | Reason Name | HydrantId API Value | | |
| |-------------|-------------|---------------------| | |
| | 0 | Unspecified | `Unspecified` | | |
| | 1 | Key Compromise | `KeyCompromise` | | |
| | 2 | CA Compromise | `CaCompromise` | | |
| The plugin currently supports the following CRL revocation reasons: | |
| | Reason Code | Reason Name | HydrantId API Value | | |
| |-------------|-------------|---------------------| | |
| | 1 | Key Compromise | `KeyCompromise` | |
| ### Supported Revocation Reasons | ||
|
|
||
| Naming Recommendations: | ||
| - Each Certificate Profile should be named after its Product ID. | ||
| The plugin supports the following standard CRL revocation reasons: | ||
|
|
||
| Behavior: | ||
| - The plugin maps HID Policy Names directly to Product IDs in the Gateway Portal. | ||
| | Reason Code | Reason Name | HydrantId API Value | | ||
| |-------------|-------------|---------------------| | ||
| | 0 | Unspecified | `Unspecified` | | ||
| | 1 | Key Compromise | `KeyCompromise` | | ||
| | 2 | CA Compromise | `CaCompromise` | | ||
| | 3 | Affiliation Changed | `AffiliationChanged` | | ||
| | 4 | Superseded | `Superseded` | | ||
| | 5 | Cessation of Operation | `CessationOfOperation` | |
There was a problem hiding this comment.
This revocation-reasons table lists reason codes 0 and 2 (and names/values like Unspecified, CaCompromise), but the plugin implementation only supports reasons 1, 3, 4, and 5 (see RevocationReasons enum and GetMapRevokeReasons). Please update the documentation to match the supported reasons, or extend the implementation to support the additional reasons if the HydrantId API accepts them.
| } | ||
|
|
||
| flow.Step("PingCA"); | ||
| _logger.LogDebug("Pinging HydrantId to validate connection"); |
There was a problem hiding this comment.
Ping() never actually performs a connectivity check (it only logs "Pinging HydrantId" and returns). Since ValidateCAConnectionInfo relies on Ping() to validate the connection, this effectively disables real validation. Consider calling HydrantIdClient.Ping() here and throwing/returning failure when it returns false or throws.
| _logger.LogDebug("Pinging HydrantId to validate connection"); | |
| _logger.LogDebug("Pinging HydrantId to validate connection"); | |
| try | |
| { | |
| var hydrantIdClient = new HydrantIdClient(_config); | |
| var pingSucceeded = await hydrantIdClient.Ping().ConfigureAwait(false); | |
| if (!pingSucceeded) | |
| { | |
| flow.Fail("PingCA", "HydrantId ping returned false"); | |
| throw new InvalidOperationException("Unable to validate connection to HydrantId."); | |
| } | |
| flow.Success("PingCA"); | |
| } | |
| catch (Exception ex) | |
| { | |
| flow.Fail("PingCA", ex.Message); | |
| _logger.LogError(ex, "Ping: HydrantId connectivity validation failed."); | |
| throw; | |
| } |
| }; | ||
| } | ||
|
|
||
| var previousX509 = new X509Certificate2(Encoding.ASCII.GetBytes(previousCert.Certificate)); |
There was a problem hiding this comment.
previousCert.Certificate is produced by GetEndEntityCertificate() as a Base64-encoded DER blob, but this code constructs X509Certificate2 from the ASCII bytes of the Base64 text. That will fail to parse (or yield incorrect data). Decode the Base64 string to raw bytes (e.g., Convert.FromBase64String) before creating the X509Certificate2.
| var previousX509 = new X509Certificate2(Encoding.ASCII.GetBytes(previousCert.Certificate)); | |
| var previousCertBytes = Convert.FromBase64String(previousCert.Certificate); | |
| var previousX509 = new X509Certificate2(previousCertBytes); |
| NullValueHandling = NullValueHandling.Ignore, | ||
| TraceWriter = traceWriter | ||
| }; | ||
| var restClient = ConfigureRestClient("post", fullUrl); |
There was a problem hiding this comment.
ConfigureRestClient creates a new HttpClient each call, but the returned client is not disposed here. Over time (especially during sync paging) this can lead to socket/resource exhaustion. Consider disposing the HttpClient (e.g., using var restClient = ...) or refactoring to reuse a shared HttpClient and set Hawk auth per-request.
| var restClient = ConfigureRestClient("post", fullUrl); | |
| using var restClient = ConfigureRestClient("post", fullUrl); |
Merge release-1.0 to main - Automated PR