-
Notifications
You must be signed in to change notification settings - Fork 0
Merge 2.2.0 to main #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
52eee75
70fe240
f9e1564
516d230
844a7e1
b0819c4
db730d5
aceca2d
a4dfbe2
d0e5a80
cd8fd90
19fae71
fe7e05d
8850680
d195faf
1b14604
a64934c
03a5fa5
208fece
509ad46
c3a719f
d534241
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| using Keyfactor.Extensions.CAPlugin.DigiCert.Models; | ||
| using Newtonsoft.Json; | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Text; | ||
| using System.Threading.Tasks; | ||
|
|
||
| namespace Keyfactor.Extensions.CAPlugin.DigiCert.API | ||
| { | ||
| [Serializable] | ||
| public class DuplicateRequest : CertCentralBaseRequest | ||
| { | ||
| public DuplicateRequest(uint orderId) | ||
| { | ||
| Method = "POST"; | ||
| OrderId = orderId; | ||
| Resource = $"services/v2/order/certificate/{OrderId}/duplicate"; | ||
| Certificate = new CertificateDuplicateRequest(); | ||
| } | ||
|
|
||
| [JsonProperty("certificate")] | ||
| public CertificateDuplicateRequest Certificate { get; set; } | ||
|
|
||
| [JsonProperty("order_id")] | ||
| public uint OrderId { get; set; } | ||
|
|
||
| [JsonProperty("skip_approval")] | ||
| public bool SkipApproval { get; set; } | ||
| } | ||
|
|
||
| public class CertificateDuplicateRequest | ||
| { | ||
| [JsonProperty("common_name")] | ||
| public string CommonName { get; set; } | ||
|
|
||
| [JsonProperty("dns_names")] | ||
| public List<string> DnsNames { get; set; } | ||
|
|
||
| [JsonProperty("csr")] | ||
| public string CSR { get; set; } | ||
|
|
||
| [JsonProperty("server_platform")] | ||
| public Server_platform ServerPlatform { get; set; } | ||
|
|
||
| [JsonProperty("signature_hash")] | ||
| public string SignatureHash { get; set; } | ||
|
|
||
| [JsonProperty("ca_cert_id")] | ||
| public string CACertID { get; set; } | ||
| } | ||
|
|
||
| public class DuplicateResponse : CertCentralBaseResponse | ||
| { | ||
| public DuplicateResponse() | ||
| { | ||
| Requests = new List<Requests>(); | ||
| } | ||
|
|
||
| [JsonProperty("id")] | ||
| public int OrderId { get; set; } | ||
|
|
||
| [JsonProperty("requests")] | ||
| public List<Requests> Requests { get; set; } | ||
|
|
||
| [JsonProperty("certificate_chain")] | ||
| public List<CertificateChainElement> CertificateChain { get; set; } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |||||||||||||||||||||||||||||||||||||
| using Newtonsoft.Json; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| using Org.BouncyCastle.Asn1.X509; | ||||||||||||||||||||||||||||||||||||||
| using Org.BouncyCastle.Pqc.Crypto.Falcon; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| using System.Collections.Concurrent; | ||||||||||||||||||||||||||||||||||||||
| using System.Runtime.InteropServices; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -294,33 +295,62 @@ public async Task<EnrollmentResult> Enroll(string csr, string subject, Dictionar | |||||||||||||||||||||||||||||||||||||
| string priorCertSnString = null; | ||||||||||||||||||||||||||||||||||||||
| string priorCertReqID = null; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Current gateway core leaves it up to the integration to determine if it is a renewal or a reissue | ||||||||||||||||||||||||||||||||||||||
| if (typeOfCert.Equals("ssl") && Convert.ToBoolean(productInfo.ProductParameters[CertCentralConstants.Config.INCLUDE_CLIENT_AUTH])) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| orderRequest.Certificate.ProfileOption = "server_client_auth_eku"; | ||||||||||||||||||||||||||||||||||||||
| _logger.LogWarning($"{CertCentralConstants.Config.INCLUDE_CLIENT_AUTH}: Ability to include client auth EKU in SSL certs is currently planned to cease in May 2026. Make sure any workflows that depend on this feature are updated before then to avoid interruptions."); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+298
to
+302
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| bool dupe = false; | ||||||||||||||||||||||||||||||||||||||
| // Current gateway core leaves it up to the integration to determine if it is a renewal, a reissue, or a duplicate | ||||||||||||||||||||||||||||||||||||||
| if (enrollmentType == EnrollmentType.RenewOrReissue) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| //// Determine if we're going to do a renew or a reissue. | ||||||||||||||||||||||||||||||||||||||
| //// Determine if we're going to do a renew, reissue, or duplicate. | ||||||||||||||||||||||||||||||||||||||
| priorCertSnString = productInfo.ProductParameters["PriorCertSN"]; | ||||||||||||||||||||||||||||||||||||||
| _logger.LogTrace($"Attempting to retrieve the certificate with serial number {priorCertSnString}."); | ||||||||||||||||||||||||||||||||||||||
| var reqId = _certificateDataReader.GetRequestIDBySerialNumber(priorCertSnString).Result; | ||||||||||||||||||||||||||||||||||||||
| if (string.IsNullOrEmpty(reqId)) | ||||||||||||||||||||||||||||||||||||||
| priorCertReqID = await _certificateDataReader.GetRequestIDBySerialNumber(priorCertSnString); | ||||||||||||||||||||||||||||||||||||||
| if (string.IsNullOrEmpty(priorCertReqID)) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| throw new Exception($"No certificate with serial number '{priorCertSnString}' could be found."); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| var expDate = _certificateDataReader.GetExpirationDateByRequestId(reqId); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| var renewCutoff = DateTime.Now.AddDays(renewWindow * -1); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (expDate > renewCutoff) | ||||||||||||||||||||||||||||||||||||||
| if (productInfo.ProductParameters.ContainsKey(CertCentralConstants.Config.DUPLICATE)) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| string dupStr = productInfo.ProductParameters[CertCentralConstants.Config.DUPLICATE].ToString(); | ||||||||||||||||||||||||||||||||||||||
| if (!bool.TryParse(dupStr, out dupe)) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| _logger.LogError($"Could not parse 'Duplicate' field as true or false. Check configuration. Value: {dupStr}"); | ||||||||||||||||||||||||||||||||||||||
| throw new Exception($"Could not parse 'Duplicate' field as true or false. Check configuration"); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| if (!dupe) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| _logger.LogTrace($"Certificate with serial number {priorCertSnString} is within renewal window"); | ||||||||||||||||||||||||||||||||||||||
| enrollmentType = EnrollmentType.Renew; | ||||||||||||||||||||||||||||||||||||||
| var expDate = _certificateDataReader.GetExpirationDateByRequestId(priorCertReqID); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| var renewCutoff = DateTime.Now.AddDays(renewWindow * -1); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (expDate > renewCutoff) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| _logger.LogTrace($"Certificate with serial number {priorCertSnString} is within renewal window"); | ||||||||||||||||||||||||||||||||||||||
| enrollmentType = EnrollmentType.Renew; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| _logger.LogTrace($"Certificate with serial number {priorCertSnString} is not within renewal window. Reissuing..."); | ||||||||||||||||||||||||||||||||||||||
| enrollmentType = EnrollmentType.Reissue; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| _logger.LogTrace($"Certificate with serial number {priorCertSnString} is not within renewal window. Reissuing..."); | ||||||||||||||||||||||||||||||||||||||
| enrollmentType = EnrollmentType.Reissue; | ||||||||||||||||||||||||||||||||||||||
| _logger.LogTrace($"'Duplicate' flag set, performing duplication"); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (dupe) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| return await Duplicate(client, productInfo, priorCertReqID, commonName, csr, dnsNames, signatureHash, caCertId); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Check if the order has more validity in it (multi-year cert). If so, do a reissue instead of a renew | ||||||||||||||||||||||||||||||||||||||
| if (enrollmentType == EnrollmentType.Renew) | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -584,6 +614,13 @@ public Dictionary<string, PropertyConfigInfo> GetTemplateParameterAnnotations() | |||||||||||||||||||||||||||||||||||||
| DefaultValue = "ssl", | ||||||||||||||||||||||||||||||||||||||
| Type = "String" | ||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| [CertCentralConstants.Config.INCLUDE_CLIENT_AUTH] = new PropertyConfigInfo() | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| Comments = "OPTIONAL for SSL certs, ignored otherwise. If set to 'true', SSL certs enrolled under this template will have the Client Authentication EKU added to the request. NOTE: This feature is currently planned to be removed by DigiCert in May 2026.", | ||||||||||||||||||||||||||||||||||||||
| Hidden = false, | ||||||||||||||||||||||||||||||||||||||
| DefaultValue = false, | ||||||||||||||||||||||||||||||||||||||
| Type = "Boolean" | ||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| [CertCentralConstants.Config.ENROLL_DIVISION_ID] = new PropertyConfigInfo() | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| Comments = "OPTIONAL: The division (container) ID to use for enrollments against this template.", | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -600,9 +637,9 @@ public Dictionary<string, PropertyConfigInfo> GetTemplateParameterAnnotations() | |||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| [CertCentralConstants.Config.PROFILE_TYPE] = new PropertyConfigInfo() | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| Comments = "Optional for secure_email_* types, ignored otherwise. Valid values are: strict, multipurpose. Default value is strict.", | ||||||||||||||||||||||||||||||||||||||
| Comments = "Optional for secure_email_* types, ignored otherwise. Valid values are: strict, multipurpose. Use 'multipurpose' if your cert includes any additional EKUs such as client auth. Default if not provided is dependent on product configuration within Digicert portal.", | ||||||||||||||||||||||||||||||||||||||
| Hidden = false, | ||||||||||||||||||||||||||||||||||||||
| DefaultValue = "strict", | ||||||||||||||||||||||||||||||||||||||
| DefaultValue = "", | ||||||||||||||||||||||||||||||||||||||
| Type = "String" | ||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| [CertCentralConstants.Config.FIRST_NAME] = new PropertyConfigInfo() | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -747,8 +784,14 @@ public async Task Synchronize(BlockingCollection<AnyCAPluginCertificate> blockin | |||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| _logger.MethodEntry(LogLevel.Trace); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| lastSync = lastSync.HasValue ? lastSync.Value.AddHours(-7) : DateTime.MinValue; // DigiCert issue with treating the timezone as mountain time. -7 to accomodate DST | ||||||||||||||||||||||||||||||||||||||
| // DigiCert issue with treating the timezone as mountain time. -7 hours to accomodate DST | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
| // DigiCert issue with treating the timezone as mountain time. -7 hours to accomodate DST | |
| // DigiCert issue with treating the timezone as mountain time. -7 hours to accommodate DST |
Copilot
AI
Feb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The date calculation uses '.Days' property which returns only the whole number of days in the TimeSpan, potentially losing precision. For example, if the difference is 6.5 days, this will evaluate to 6 and the condition won't trigger. Consider using '.TotalDays' instead and comparing with a double value, or ensure this behavior is intentional for the sync window logic.
| if ((utcDate.Value - lastSync.Value).Days > 6) | |
| if ((utcDate.Value - lastSync.Value).TotalDays > 6.0) |
Copilot
AI
Feb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter documentation mentions 'request' with type 'OrderRequest', but this parameter does not exist in the method signature. The method actually has individual parameters like 'enrollmentProductInfo', 'caRequestId', 'commonName', etc. Update the documentation to accurately reflect the actual parameters or remove the incorrect parameter reference.
| /// <param name="request">The <see cref="OrderRequest"/>.</param> | |
| /// <param name="enrollmentProductInfo">Information about the DigiCert product this certificate uses.</param> | |
| /// <returns></returns> | |
| /// <param name="enrollmentProductInfo">Information about the DigiCert product this certificate uses.</param> | |
| /// <param name="caRequestId">The CA request identifier from which to derive the original order ID.</param> | |
| /// <param name="commonName">The common name (CN) for the certificate.</param> | |
| /// <param name="csr">The certificate signing request (CSR) for the new duplicate certificate.</param> | |
| /// <param name="dnsNames">Additional DNS names (SANs) to include in the duplicate certificate.</param> | |
| /// <param name="signatureHash">The signature hash algorithm to use.</param> | |
| /// <param name="caCertId">The DigiCert CA certificate identifier to use for issuance.</param> | |
| /// <returns>The result of the duplicate enrollment operation.</returns> |
Copilot
AI
Feb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The DuplicateRequest is created without setting 'SkipApproval', which will default to false. The similar Reissue method explicitly sets 'SkipApproval = true' with a comment explaining it allows the certificate ID to return a value (see line 1479). Consider whether the Duplicate method should also set 'SkipApproval = true' for consistency and to ensure the certificate ID is returned, unless there's a specific reason for the different behavior.
Copilot
AI
Feb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment contains a spelling error: 'arround' should be spelled 'around' (with one 'r').
| //Another check for duplicate PEMs to get arround issue with DigiCert API returning incorrect data sometimes on reissued/duplicate certs | |
| //Another check for duplicate PEMs to get around issue with DigiCert API returning incorrect data sometimes on reissued/duplicate certs |
Copilot
AI
Feb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'certificate' variable could be null if the certificate status is not GENERATED or REVOKED (line 1639-1650), but it's added to 'pemList' unconditionally at line 1658. This means null values could be added to the list, and the duplicate check at line 1653 would match multiple null values incorrectly. Consider only adding to pemList when certificate is not null, or restructure the logic to handle null certificates appropriately.
| //Another check for duplicate PEMs to get arround issue with DigiCert API returning incorrect data sometimes on reissued/duplicate certs | |
| if (pemList.Contains(certificate)) | |
| { | |
| _logger.LogWarning($"Found duplicate PEM for ID {caReqId}. Skipping..."); | |
| continue; | |
| } | |
| pemList.Add(certificate); | |
| //Another check for duplicate PEMs to get around issue with DigiCert API returning incorrect data sometimes on reissued/duplicate certs | |
| if (certificate != null) | |
| { | |
| if (pemList.Contains(certificate)) | |
| { | |
| _logger.LogWarning($"Found duplicate PEM for ID {caReqId}. Skipping..."); | |
| continue; | |
| } | |
| pemList.Add(certificate); | |
| } |
Copilot
AI
Feb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The else block at lines 1788-1791 that sets 'profile = null' is redundant. The variable 'profile' is already initialized to null at line 1774, and the only assignment inside the if block (line 1777) converts the parameter to a string. If that string is empty, it won't pass the validation at line 1780-1786 and profile would remain as set. This else block appears unnecessary and could be removed for clarity.
| else | |
| { | |
| profile = null; | |
| } |
Copilot
AI
Feb 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line has trailing whitespace after the closing brace. Consider removing the trailing whitespace to maintain code cleanliness and consistency.
| } | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import 'using Org.BouncyCastle.Pqc.Crypto.Falcon;' appears to be unused. This namespace is typically used for post-quantum cryptography (Falcon signature scheme), but there are no references to Falcon types anywhere in this file. This unused import should be removed to keep the code clean and avoid confusion.