diff --git a/CHANGELOG.md b/CHANGELOG.md index a571c73..31bb4a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,3 +39,12 @@ ### 2.4.1 * Fix for missing parameter errors + +### 2.5.0 +* Fix for checking if product is a DV cert to ignore organization checks +* Performance enhancements to reduce API calls +* Added null checks/error handling to catch bad certs on sync +* Improved rate limiting handling based on DigiCert guidance +* Improved handling of API error responses +* Fixes for sync filtering +* Incremental sync fix diff --git a/digicert-certcentral-caplugin/CertCentralCAPlugin.cs b/digicert-certcentral-caplugin/CertCentralCAPlugin.cs index 176c2bd..a5e2869 100644 --- a/digicert-certcentral-caplugin/CertCentralCAPlugin.cs +++ b/digicert-certcentral-caplugin/CertCentralCAPlugin.cs @@ -174,7 +174,21 @@ public async Task Enroll(string csr, string subject, Dictionar CertCentralClient client = CertCentralClientUtilities.BuildCertCentralClient(_config); int? organizationId = null; // DV certs have no organization, so only do the org check if its a non-DV cert - if (!string.Equals(productInfo.ProductID, CertCentralConstants.ProductTypes.DV_SSL_CERT, StringComparison.OrdinalIgnoreCase)) + + // Get product ID details. + CertificateTypeDetailsRequest detailsRequest = new CertificateTypeDetailsRequest(productInfo.ProductID); + + // For pulling product ID details, we use the Connection-level Division ID rather than the template-level one. + detailsRequest.ContainerId = null; + if (_config.DivisionId.HasValue) + { + detailsRequest.ContainerId = _config.DivisionId.Value; + } + + CertificateTypeDetailsResponse details = client.GetCertificateTypeDetails(detailsRequest); + + // Only do org check if the product type is NOT the group dv_ssl_certificate (https://dev.digicert.com/certcentral-apis/services-api/glossary.html#product-identifiers) + if (!string.Equals(details.GroupName, CertCentralConstants.ProductTypes.DV_SSL_CERT, StringComparison.OrdinalIgnoreCase)) { if (organization == null) { @@ -585,7 +599,7 @@ public async Task GetSingleRecord(string caRequestID) CertCentralClient client = CertCentralClientUtilities.BuildCertCentralClient(_config); ViewCertificateOrderResponse orderResponse = client.ViewCertificateOrder(new ViewCertificateOrderRequest((uint)orderId)); - var orderCerts = GetAllCertsForOrder(orderId); + var orderCerts = GetAllCertsForOrder(orderId, orderResponse); StatusOrder certToCheck = orderCerts.Where(c => c.certificate_id == certIdInt).First(); @@ -873,7 +887,7 @@ public async Task Synchronize(BlockingCollection blockin _logger.LogTrace($"Sync CAs: {syncCAstring}"); List caList = _config.SyncCAs; - caList.ForEach(c => c.ToUpper()); + caList = caList.Select(c => c?.Trim().ToUpper()).Where(c => !string.IsNullOrEmpty(c)).ToList(); List divFilters = new List(); if (!string.IsNullOrEmpty(_config.SyncDivisionFilter)) @@ -969,7 +983,7 @@ public async Task Synchronize(BlockingCollection blockin cancelToken.ThrowIfCancellationRequested(); string caReqId = order.order_id + "-" + order.certificate_id; orderCerts = GetAllConnectorCertsForOrder(caReqId, caList, divFilters, productFilters); - if (orderCerts == null || orderCerts.Count > 0) + if (orderCerts == null || orderCerts.Count == 0) { continue; } @@ -1699,9 +1713,10 @@ private List GetAllConnectorCertsForOrder(string caReque if (productIds != null && productIds.Count > 0 && !productIds.Contains(orderResponse.product.name_id.ToString())) { _logger.LogTrace($"Found order ID {orderId} that does not match Product filter. Product ID: {orderResponse.product.name_id.ToString()} Skipping..."); + return null; } - var orderCerts = GetAllCertsForOrder(orderId); + var orderCerts = GetAllCertsForOrder(orderId, orderResponse); List certList = new List(); List pemList = new List(); @@ -1719,6 +1734,10 @@ private List GetAllConnectorCertsForOrder(string caReque CertificateChainResponse certificateChainResponse = client.GetCertificateChain(new CertificateChainRequest($"{cert.certificate_id}")); if (certificateChainResponse.Status == CertCentralBaseResponse.StatusType.SUCCESS) { + if (certificateChainResponse.Intermediates == null || certificateChainResponse.Intermediates.Count == 0) + { + throw new Exception($"DigiCert returned an empty certificate chain for certificate {cert.certificate_id} on order {orderId}."); + } certificate = certificateChainResponse.Intermediates[0].PEM; } else @@ -1727,12 +1746,15 @@ private List GetAllConnectorCertsForOrder(string caReque } } //Another check for duplicate PEMs to get arround issue with DigiCert API returning incorrect data sometimes on reissued/duplicate certs - if (pemList.Contains(certificate)) + if (certificate != null && pemList.Contains(certificate)) { _logger.LogWarning($"Found duplicate PEM for ID {caReqId}. Skipping..."); continue; } - pemList.Add(certificate); + if (certificate != null) + { + pemList.Add(certificate); + } var connCert = new AnyCAPluginCertificate { CARequestID = caReqId, @@ -1757,10 +1779,12 @@ private List GetAllConnectorCertsForOrder(string caReque /// /// /// - private List GetAllCertsForOrder(int orderId) + private List GetAllCertsForOrder(int orderId, ViewCertificateOrderResponse existingOrderResponse = null) { CertCentralClient client = CertCentralClientUtilities.BuildCertCentralClient(_config); - ViewCertificateOrderResponse orderResponse = client.ViewCertificateOrder(new ViewCertificateOrderRequest((uint)orderId)); + + // If the caller provides an existing order response, reuse that to save the API call. + ViewCertificateOrderResponse orderResponse = existingOrderResponse ?? client.ViewCertificateOrder(new ViewCertificateOrderRequest((uint)orderId)); if (orderResponse.Status == CertCentralBaseResponse.StatusType.ERROR) { string errorMessage = String.Format("Request {0} was not found in CertCentral database or is not valid", orderId); diff --git a/digicert-certcentral-caplugin/Client/CertCentralClient.cs b/digicert-certcentral-caplugin/Client/CertCentralClient.cs index 753f54f..1b4f6c6 100644 --- a/digicert-certcentral-caplugin/Client/CertCentralClient.cs +++ b/digicert-certcentral-caplugin/Client/CertCentralClient.cs @@ -6,6 +6,7 @@ using System; using System.Collections.Generic; +using System.Data.SqlTypes; using System.Linq; using System.Net; using System.Security.Cryptography.X509Certificates; @@ -64,6 +65,43 @@ public CertCentralResponse() public bool Success { get; set; } public string Response { get; set; } + public int StatusCode { get; set; } + } + + /// + /// Turn a DigiCert response body into a non-empty list of Errors, without throwing + /// + /// + /// + /// + internal static List ParseErrors(string body, int statusCode) + { + string codeSuffix = statusCode > 0 ? $" (HTTP {statusCode})" : ""; + + if (string.IsNullOrWhiteSpace(body)) + { + return new List { new Error { code = "empty_response_body", message = $"DigiCert returned no response body{codeSuffix}." } }; + } + + try + { + Errors parsed = JsonConvert.DeserializeObject(body); + if (parsed?.errors != null && parsed.errors.Count > 0) + { + return parsed.errors; + } + return new List { new Error { code = "unrecognized_error_response", message = $"DigiCert returned a response with no 'errors' array{codeSuffix} : {Truncate(body)}" } }; + } + catch (JsonException) + { + return new List { new Error { code = "non_json_response", message = $"DigiCert returned a non-JSON response{codeSuffix}: {Truncate(body)}" } }; + } + } + + private static string Truncate(string s) + { + s = s.Replace("\r", " ").Replace("\n", " ").Trim(); + return s.Length <= 500 ? s : s.Substring(0, 500) + "\u2026"; } private CertCentralResponse Request(CertCentralBaseRequest request) @@ -72,8 +110,14 @@ private CertCentralResponse Request(CertCentralBaseRequest request) } private static int RequestIDCounter = 1; + private const int MaxRateLimitRetries = 3; private CertCentralResponse Request(CertCentralBaseRequest request, string parameters) + { + return Request(request, parameters, 1); + } + + private CertCentralResponse Request(CertCentralBaseRequest request, string parameters, int attempt) { //set in config files //ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls12; @@ -118,6 +162,7 @@ private CertCentralResponse Request(CertCentralBaseRequest request, string param { string respString = new StreamReader(objResponse.GetResponseStream()).ReadToEnd(); oCertCertResponse.Response = respString; + oCertCertResponse.StatusCode = (int)objResponse.StatusCode; Logger.LogTrace($"CertCentral CA (Request ID: {reqID}) has returned Response '{objResponse.StatusCode}: {respString}"); } } @@ -129,10 +174,25 @@ private CertCentralResponse Request(CertCentralBaseRequest request, string param { if (errorResponse.StatusCode == (HttpStatusCode)429/*Too Many Requests*/) { - Logger.LogInformation($"Request ID: {reqID} was rate-limited. Trying again in 5 seconds"); - // TODO - Figure out how long to wait, then wait that long - System.Threading.Thread.Sleep(5000); - return Request(request, parameters); + // DigiCert's documented limits are 1000 requests / 3 minutes AND 100 requests / 5 seconds, + // rolling, per API key. + // Guidance is exponential backoff with a default maximum of 3 retries. + if (attempt >= MaxRateLimitRetries) + { + Logger.LogWarning($"Request ID: {reqID} was rate-limited by DigiCert and has exhausted {MaxRateLimitRetries} attempts. Giving up."); + using (var limitReader = new StreamReader(errorResponse.GetResponseStream())) + { + oCertCertResponse.Success = false; + oCertCertResponse.Response = limitReader.ReadToEnd(); + } + } + else + { + int waitSeconds = 5 * (int)Math.Pow(2, attempt - 1); + Logger.LogInformation($"Request ID: {reqID} was rate-limited. Retry {attempt} of {MaxRateLimitRetries - 1} in {waitSeconds} seconds"); + System.Threading.Thread.Sleep(waitSeconds * 1000); + return Request(request, parameters, attempt + 1); + } } else { @@ -141,6 +201,7 @@ private CertCentralResponse Request(CertCentralBaseRequest request, string param string errorString = reader.ReadToEnd(); oCertCertResponse.Success = false; oCertCertResponse.Response = errorString; + oCertCertResponse.StatusCode = (int)errorResponse.StatusCode; Logger.LogTrace($"CertCentral CA (Request ID: {reqID}) has returned Response '{errorResponse.StatusCode}: {errorString}"); } } @@ -169,9 +230,9 @@ public ListOrganizationsResponse ListOrganizations(ListOrganizationsRequest requ if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listOrganizationsResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listOrganizationsResponse.Errors = errors.errors; + listOrganizationsResponse.Errors = errors; } else listOrganizationsResponse = JsonConvert.DeserializeObject(response.Response); @@ -187,9 +248,9 @@ public ListDomainsResponse ListDomains(ListDomainsRequest request) if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors= ParseErrors(response.Response, response.StatusCode); listDomainsResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listDomainsResponse.Errors = errors.errors; + listDomainsResponse.Errors = errors; } else listDomainsResponse = JsonConvert.DeserializeObject(response.Response); @@ -205,9 +266,9 @@ public ListContainersResponse ListContainers(ListContainersRequest request) if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listContainersResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listContainersResponse.Errors = errors.errors; + listContainersResponse.Errors = errors; } else { @@ -225,9 +286,9 @@ public ListDuplicatesResponse ListDuplicates(ListDuplicatesRequest duplicatesReq if (!ccResponse.Success) { - Errors errors = JsonConvert.DeserializeObject(ccResponse.Response); + List errors = ParseErrors(ccResponse.Response, ccResponse.StatusCode); duplicatesResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - duplicatesResponse.Errors = errors.errors; + duplicatesResponse.Errors = errors; } else { @@ -245,9 +306,9 @@ public ListReissueResponse ListReissues(ListReissueRequest reissueRequest) if (!ccResponse.Success) { - Errors errors = JsonConvert.DeserializeObject(ccResponse.Response); + List errors = ParseErrors(ccResponse.Response, ccResponse.StatusCode); reissueResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - reissueResponse.Errors = errors.errors; + reissueResponse.Errors = errors; } else { @@ -265,9 +326,9 @@ public ListRequestsResponse ListRequests(ListRequestsRequest request) if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listRequestsResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listRequestsResponse.Errors = errors.errors; + listRequestsResponse.Errors = errors; } else listRequestsResponse = JsonConvert.DeserializeObject(response.Response); @@ -283,9 +344,9 @@ public ListMetadataResponse ListMetadata(ListMetadataRequest request) if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listMetadataResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listMetadataResponse.Errors = errors.errors; + listMetadataResponse.Errors = errors; } else listMetadataResponse = JsonConvert.DeserializeObject(response.Response); @@ -304,9 +365,9 @@ public OrderResponse OrderCertificate(OrderRequest request) OrderResponse orderResponse = new OrderResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); orderResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - orderResponse.Errors = errors.errors; + orderResponse.Errors = errors; } else orderResponse = JsonConvert.DeserializeObject(response.Response); @@ -325,9 +386,9 @@ public OrderResponse OrderSmimeCertificate(OrderSmimeRequest request) OrderResponse orderResponse = new OrderResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); orderResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - orderResponse.Errors = errors.errors; + orderResponse.Errors = errors; } else orderResponse = JsonConvert.DeserializeObject(response.Response); @@ -345,9 +406,9 @@ public OrderResponse ReissueCertificate(ReissueRequest request) OrderResponse reissueResponse = new OrderResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); reissueResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - reissueResponse.Errors = errors.errors; + reissueResponse.Errors = errors; } else { @@ -367,9 +428,9 @@ public OrderResponse DuplicateCertificate(DuplicateRequest request) OrderResponse duplicateResponse = new OrderResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); duplicateResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - duplicateResponse.Errors = errors.errors; + duplicateResponse.Errors = errors; } else { @@ -386,9 +447,9 @@ public RevokeCertificateResponse RevokeCertificate(RevokeCertificateRequest requ RevokeCertificateResponse revokeOrderResponse = new RevokeCertificateResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); revokeOrderResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - revokeOrderResponse.Errors = errors.errors; + revokeOrderResponse.Errors = errors; } else revokeOrderResponse = JsonConvert.DeserializeObject(response.Response); @@ -403,9 +464,9 @@ public RevokeCertificateResponse RevokeCertificate(RevokeCertificateByOrderReque RevokeCertificateResponse revokeOrderResponse = new RevokeCertificateResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); revokeOrderResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - revokeOrderResponse.Errors = errors.errors; + revokeOrderResponse.Errors = errors; } else revokeOrderResponse = JsonConvert.DeserializeObject(response.Response); @@ -420,9 +481,9 @@ public UpdateRequestStatusResponse UpdateRequestStatus(UpdateRequestStatusReques UpdateRequestStatusResponse updateRequestResponse = new UpdateRequestStatusResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); updateRequestResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - updateRequestResponse.Errors = errors.errors; + updateRequestResponse.Errors = errors; } else { @@ -443,9 +504,9 @@ public DVCheckDCVResponse DVCheckDCV(DVCheckDCVRequest request) DVCheckDCVResponse checkDCVResponse = new DVCheckDCVResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); checkDCVResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - checkDCVResponse.Errors = errors.errors; + checkDCVResponse.Errors = errors; } else { @@ -461,9 +522,9 @@ public CertificateChainResponse GetCertificateChain(CertificateChainRequest requ CertificateChainResponse chainResponse = new CertificateChainResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); chainResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - chainResponse.Errors = errors.errors; + chainResponse.Errors = errors; } else { @@ -479,9 +540,9 @@ public StatusChangesResponse StatusChanges(StatusChangesRequest request) StatusChangesResponse statusChangeResponse = new StatusChangesResponse(); if (!certResponse.Success) { - Errors errors = JsonConvert.DeserializeObject(certResponse.Response); + List errors = ParseErrors(certResponse.Response, certResponse.StatusCode); statusChangeResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - statusChangeResponse.Errors = errors.errors; + statusChangeResponse.Errors = errors; } else { @@ -496,9 +557,9 @@ public DownloadCertificateByFormatResponse DownloadCertificateByFormat(DownloadC DownloadCertificateByFormatResponse dlCertificateRequestResponse = new DownloadCertificateByFormatResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); dlCertificateRequestResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - dlCertificateRequestResponse.Errors = errors.errors; + dlCertificateRequestResponse.Errors = errors; } else { @@ -545,9 +606,9 @@ public ListCertificateOrdersResponse ListAllCertificateOrders(bool ignoreExpired ListCertificateOrdersResponse listCertificateResponse = new ListCertificateOrdersResponse(); if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); listCertificateResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - listCertificateResponse.Errors = errors.errors; + listCertificateResponse.Errors = errors; return listCertificateResponse; } @@ -571,9 +632,9 @@ public ViewCertificateOrderResponse ViewCertificateOrder(ViewCertificateOrderReq if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); viewCertResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - viewCertResponse.Errors = errors.errors; + viewCertResponse.Errors = errors; } else { @@ -596,9 +657,9 @@ public CertificateTypeDetailsResponse GetCertificateTypeDetails(CertificateTypeD if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); detailsResponse.Status = CertCentralBaseResponse.StatusType.ERROR; - detailsResponse.Errors = errors.errors; + detailsResponse.Errors = errors; } else { @@ -620,9 +681,9 @@ public CertificateTypesResponse GetAllCertificateTypes() if (!response.Success) { - Errors errors = JsonConvert.DeserializeObject(response.Response); + List errors = ParseErrors(response.Response, response.StatusCode); allTypes.Status = CertCentralBaseResponse.StatusType.ERROR; - allTypes.Errors = errors.errors; + allTypes.Errors = errors; } else { diff --git a/digicert-certcentral-caplugin/Models/Error.cs b/digicert-certcentral-caplugin/Models/Error.cs index e963096..1a7c1c6 100644 --- a/digicert-certcentral-caplugin/Models/Error.cs +++ b/digicert-certcentral-caplugin/Models/Error.cs @@ -15,6 +15,13 @@ public class Error [JsonProperty("message")] public string message { get; set; } + + public override string ToString() + { + if (string.IsNullOrEmpty(code)) return message ?? string.Empty; + if (string.IsNullOrEmpty(message)) return code; + return $"{code}: {message}"; + } } public class Errors diff --git a/digicert-certcentral-caplugin/digicert-certcentral-caplugin.csproj b/digicert-certcentral-caplugin/digicert-certcentral-caplugin.csproj index 7510b07..8b1c555 100644 --- a/digicert-certcentral-caplugin/digicert-certcentral-caplugin.csproj +++ b/digicert-certcentral-caplugin/digicert-certcentral-caplugin.csproj @@ -6,8 +6,6 @@ enable disable DigicertCAPlugin - 2.1.2 - 2.1.2