From f636fb6da6209ce1f147295396f14329cd83b682 Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Wed, 3 Apr 2024 23:20:57 +0200 Subject: [PATCH 01/12] Reworked SSL certificate logging --- src/Agent.Sdk/Util/SslUtil.cs | 168 +++++++++++++++------------------- 1 file changed, 74 insertions(+), 94 deletions(-) diff --git a/src/Agent.Sdk/Util/SslUtil.cs b/src/Agent.Sdk/Util/SslUtil.cs index 5d6f2b1011..7cdc278bd6 100644 --- a/src/Agent.Sdk/Util/SslUtil.cs +++ b/src/Agent.Sdk/Util/SslUtil.cs @@ -1,22 +1,24 @@ -using System.Net.Security; -using System.Security.Cryptography.X509Certificates; -using System.Net.Http; using Agent.Sdk; +using System; using System.Collections.Generic; using System.Linq; -using System; +using System.Net.Http; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; +using System.Text; namespace Microsoft.VisualStudio.Services.Agent.Util { public sealed class SslUtil { - private ITraceWriter _trace { get; set; } - private bool _IgnoreCertificateErrors { get; set; } + private readonly ITraceWriter _trace; - public SslUtil(ITraceWriter trace, bool IgnoreCertificateErrors = false) + private readonly bool _ignoreCertificateErrors; + + public SslUtil(ITraceWriter trace, bool ignoreCertificateErrors = false) { this._trace = trace; - this._IgnoreCertificateErrors = IgnoreCertificateErrors; + this._ignoreCertificateErrors = ignoreCertificateErrors; } /// <summary>Implementation of custom callback function that logs SSL-related data from the web request to the agent's logs.</summary> @@ -24,38 +26,42 @@ public SslUtil(ITraceWriter trace, bool IgnoreCertificateErrors = false) public bool RequestStatusCustomValidation(HttpRequestMessage requestMessage, X509Certificate2 certificate, X509Chain chain, SslPolicyErrors sslErrors) { bool isRequestSuccessful = (sslErrors == SslPolicyErrors.None); - + if (!isRequestSuccessful) { LoggingRequestDiagnosticData(requestMessage, certificate, chain, sslErrors); } - if (this._IgnoreCertificateErrors) { - this._trace?.Info("Ignoring certificate errors."); + if (this._ignoreCertificateErrors) + { + this._trace?.Info("Ignoring certificate errors."); } - return this._IgnoreCertificateErrors || isRequestSuccessful; + return this._ignoreCertificateErrors || isRequestSuccessful; } /// <summary>Logs SSL related data to agent's logs</summary> private void LoggingRequestDiagnosticData(HttpRequestMessage requestMessage, X509Certificate2 certificate, X509Chain chain, SslPolicyErrors sslErrors) { - string diagInfo = "Diagnostic data for request:\n"; - if (this._trace != null) { - diagInfo += SslDiagnosticDataProvider.ResolveSslPolicyErrorsMessage(sslErrors); - diagInfo += SslDiagnosticDataProvider.GetRequestMessageData(requestMessage); - diagInfo += SslDiagnosticDataProvider.GetCertificateData(certificate); - diagInfo += SslDiagnosticDataProvider.GetChainData(chain); + var logBuilder = new SslDiagnosticsLogBuilder(); + logBuilder.AddSslPolicyErrorsMessages(sslErrors); + logBuilder.AddRequestMessageLog(requestMessage); + logBuilder.AddCertificateLog(certificate); + logBuilder.AddChainLog(chain); + + var formattedLog = logBuilder.BuildLog(); - this._trace?.Info(diagInfo); + this._trace?.Info($"Diagnostic data for request:{Environment.NewLine}{formattedLog}"); } } } - public static class SslDiagnosticDataProvider + internal sealed class SslDiagnosticsLogBuilder { + private readonly StringBuilder _logBuilder = new StringBuilder(); + /// <summary>A predefined list of headers to get from the request</summary> private static readonly string[] _requiredRequestHeaders = new[] { @@ -74,122 +80,99 @@ public static class SslDiagnosticDataProvider }; /// <summary> - /// Get diagnostic data about the HTTP request. + /// Add diagnostics data about the HTTP request. /// This method extracts common information about the request itself and the request's headers. /// To expand list of headers please update <see cref="_requiredRequestHeaders"/></summary>. - /// <returns>Diagnostic data as a formatted string</returns> - public static string GetRequestMessageData(HttpRequestMessage requestMessage) + public void AddRequestMessageLog(HttpRequestMessage requestMessage) { // Getting general information about request - string requestDiagInfoHeader = "HttpRequest"; - string diagInfo = string.Empty; - if (requestMessage is null) { - return $"{requestDiagInfoHeader} data is empty"; + _logBuilder.AppendLine($"HttpRequest data is empty"); + return; } - var requestDiagInfo = new List<KeyValuePair<string, string>>(); var requestedUri = requestMessage?.RequestUri.ToString(); var methodType = requestMessage?.Method.ToString(); - requestDiagInfo.Add(new KeyValuePair<string, string>("Requested URI", requestedUri)); - requestDiagInfo.Add(new KeyValuePair<string, string>("Request method", methodType)); - - diagInfo = GetFormattedData(requestDiagInfoHeader, requestDiagInfo); + _logBuilder.AppendLine($"[HttpRequest]"); + _logBuilder.AppendLine($"Requested URI: {requestedUri}"); + _logBuilder.AppendLine($"Requested method: {methodType}"); // Getting informantion from headers var requestHeaders = requestMessage?.Headers; if (requestHeaders is null) { - return diagInfo; + return; } - string headersDiagInfoHeader = "HttpRequestHeaders"; - - var headersDiagInfo = new List<KeyValuePair<string, string>>(); + _logBuilder.AppendLine($"[HttpRequestHeaders]"); foreach (var headerKey in _requiredRequestHeaders) { IEnumerable<string> headerValues; if (requestHeaders.TryGetValues(headerKey, out headerValues)) { - var headerValue = string.Join(", ", headerValues.ToArray()); - if (headerValue != null) - { - headersDiagInfo.Add(new KeyValuePair<string, string>(headerKey, headerValue.ToString())); - } + _logBuilder.AppendLine($"{headerKey}: {string.Join(", ", headerValues.ToArray())}"); } } - - diagInfo += GetFormattedData(headersDiagInfoHeader, headersDiagInfo); - - return diagInfo; } /// <summary> - /// Get diagnostic data about the certificate. + /// Add diagnostics data about the certificate. /// This method extracts common information about the certificate. /// </summary> - /// <returns>Diagnostic data as a formatted string</returns> - public static string GetCertificateData(X509Certificate2 certificate) + public void AddCertificateLog(X509Certificate2 certificate) { - string diagInfoHeader = "Certificate"; var diagInfo = new List<KeyValuePair<string, string>>(); if (certificate is null) { - return $"{diagInfoHeader} data is empty"; + _logBuilder.AppendLine($"Certificate data is empty"); + return; } - diagInfo.Add(new KeyValuePair<string, string>("Effective date", certificate?.GetEffectiveDateString())); - diagInfo.Add(new KeyValuePair<string, string>("Expiration date", certificate?.GetExpirationDateString())); - diagInfo.Add(new KeyValuePair<string, string>("Issuer", certificate?.Issuer)); - diagInfo.Add(new KeyValuePair<string, string>("Subject", certificate?.Subject)); - - return GetFormattedData(diagInfoHeader, diagInfo); + _logBuilder.AppendLine($"[Certificate]"); + AddCertificateData(certificate); } /// <summary> - /// Get diagnostic data about the chain. + /// Add diagnostics data about the chain. /// This method extracts common information about the chain. /// </summary> - /// <returns>Diagnostic data as a formatted string</returns> - public static string GetChainData(X509Chain chain) + public void AddChainLog(X509Chain chain) { - string diagInfoHeader = "ChainStatus"; - var diagInfo = new List<KeyValuePair<string, string>>(); - - if (chain is null) + if (chain is null || chain.ChainElements is null) { - return $"{diagInfoHeader} data is empty"; + _logBuilder.AppendLine($"ChainElements data is empty"); + return; } - foreach (var status in chain.ChainStatus) + _logBuilder.AppendLine("[ChainElements]"); + foreach (var chainElement in chain.ChainElements) { - diagInfo.Add(new KeyValuePair<string, string>("Status", status.Status.ToString())); - diagInfo.Add(new KeyValuePair<string, string>("Status Information", status.StatusInformation)); + AddCertificateData(chainElement.Certificate); + foreach (var status in chainElement.ChainElementStatus) + { + _logBuilder.AppendLine($"Status: {status.Status}"); + _logBuilder.AppendLine($"Status Information: {status.StatusInformation}"); + } } - - return GetFormattedData(diagInfoHeader, diagInfo); } /// <summary> - /// Get list of SSL policy errors with descriptions. + /// Add list of SSL policy errors with descriptions. /// This method checks SSL policy errors and mapping them to user-friendly descriptions. /// To update SSL policy errors description please update <see cref="_sslPolicyErrorsMapping"/>. /// </summary> - /// <returns>Diagnostic data as a formatted string</returns> - public static string ResolveSslPolicyErrorsMessage(SslPolicyErrors sslErrors) + public void AddSslPolicyErrorsMessages(SslPolicyErrors sslErrors) { - string diagInfoHeader = $"SSL Policy Errors"; - var diagInfo = new List<KeyValuePair<string, string>>(); + _logBuilder.AppendLine($"[SSL Policy Errors]"); if (sslErrors == SslPolicyErrors.None) { - diagInfo.Add(new KeyValuePair<string, string>(sslErrors.ToString(), _sslPolicyErrorsMapping[sslErrors])); - return GetFormattedData(diagInfoHeader, diagInfo); + _logBuilder.AppendLine($"No SSL policy errors"); } // Since we can get several SSL policy errors we should check all of them @@ -197,33 +180,30 @@ public static string ResolveSslPolicyErrorsMessage(SslPolicyErrors sslErrors) { if ((sslErrors & errorCode) != 0) { - string errorValue = errorCode.ToString(); - string errorMessage = string.Empty; - - if (!_sslPolicyErrorsMapping.TryGetValue(errorCode, out errorMessage)) + if (!_sslPolicyErrorsMapping.ContainsKey(errorCode)) { - errorMessage = "Could not resolve related error message"; + _logBuilder.AppendLine($"{errorCode.ToString()}: Could not resolve related error message"); + } + else + { + _logBuilder.AppendLine($"{errorCode.ToString()}: {_sslPolicyErrorsMapping[errorCode]}"); } - - diagInfo.Add(new KeyValuePair<string, string>(errorValue, errorMessage)); } } - - return GetFormattedData(diagInfoHeader, diagInfo); } - /// <summary>Get diagnostic data as formatted text</summary> - /// <returns>Formatted string</returns> - private static string GetFormattedData(string diagInfoHeader, List<KeyValuePair<string, string>> diagInfo) + public string BuildLog() { - string formattedData = $"[{diagInfoHeader}]\n"; + return _logBuilder.ToString(); + } - foreach (var record in diagInfo) - { - formattedData += $"{record.Key}: {record.Value}\n"; - } - return formattedData; + private void AddCertificateData(X509Certificate2 certificate) + { + _logBuilder.AppendLine($"Effective date: {certificate?.GetEffectiveDateString()}"); + _logBuilder.AppendLine($"Expiration date: {certificate?.GetExpirationDateString()}"); + _logBuilder.AppendLine($"Issuer: {certificate?.Issuer}"); + _logBuilder.AppendLine($"Subject: {certificate?.Subject}"); } } } From e9ffe5639207ead93ba584a26c9176fd00916ac5 Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Mon, 8 Apr 2024 21:18:27 +0200 Subject: [PATCH 02/12] Added initial tests --- src/Test/L0/Util/SslUtilL0.cs | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 src/Test/L0/Util/SslUtilL0.cs diff --git a/src/Test/L0/Util/SslUtilL0.cs b/src/Test/L0/Util/SslUtilL0.cs new file mode 100644 index 0000000000..7f48c46b8c --- /dev/null +++ b/src/Test/L0/Util/SslUtilL0.cs @@ -0,0 +1,45 @@ +using Microsoft.VisualStudio.Services.Agent.Util; +using System.Net.Http; +using Xunit; + +namespace Test.L0.Util +{ + public sealed class SslUtilL0 + { + [Fact] + public void AddRequestMessageLog_RequestMessageIsNull_ShouldReturnCorrectLog() + { + // Arrange + HttpRequestMessage requestMessage = null; + var logBuilder = new SslDiagnosticsLogBuilder(); + + // Act + logBuilder.AddRequestMessageLog(requestMessage); + var log = logBuilder.BuildLog(); + + // Assert + Assert.Equal("Request message is null.", log); + } + + [Fact] + public void AddRequestMessageLog_RequestMessageIsNotNull_ShouldReturnCorrectLog() + { + // Arrange + var requestMessage = new HttpRequestMessage(HttpMethod.Get, "http://localhost"); + var logBuilder = new SslDiagnosticsLogBuilder(); + var log = string.Empty; + + // Act + using (requestMessage) + { + logBuilder.AddRequestMessageLog(requestMessage); + log = logBuilder.BuildLog(); + } + + // Assert + Assert.Contains("Request message:", log); + Assert.Contains("Method: GET", log); + Assert.Contains("Request URI: http://localhost/", log); + } + } +} From 89a58f96a14cab5ac52785ca4f15c2951269a866 Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Wed, 3 Apr 2024 23:20:57 +0200 Subject: [PATCH 03/12] Reworked SSL certificate logging --- src/Agent.Sdk/Util/SslUtil.cs | 168 +++++++++++++++------------------- 1 file changed, 74 insertions(+), 94 deletions(-) diff --git a/src/Agent.Sdk/Util/SslUtil.cs b/src/Agent.Sdk/Util/SslUtil.cs index 5d6f2b1011..7cdc278bd6 100644 --- a/src/Agent.Sdk/Util/SslUtil.cs +++ b/src/Agent.Sdk/Util/SslUtil.cs @@ -1,22 +1,24 @@ -using System.Net.Security; -using System.Security.Cryptography.X509Certificates; -using System.Net.Http; using Agent.Sdk; +using System; using System.Collections.Generic; using System.Linq; -using System; +using System.Net.Http; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; +using System.Text; namespace Microsoft.VisualStudio.Services.Agent.Util { public sealed class SslUtil { - private ITraceWriter _trace { get; set; } - private bool _IgnoreCertificateErrors { get; set; } + private readonly ITraceWriter _trace; - public SslUtil(ITraceWriter trace, bool IgnoreCertificateErrors = false) + private readonly bool _ignoreCertificateErrors; + + public SslUtil(ITraceWriter trace, bool ignoreCertificateErrors = false) { this._trace = trace; - this._IgnoreCertificateErrors = IgnoreCertificateErrors; + this._ignoreCertificateErrors = ignoreCertificateErrors; } /// <summary>Implementation of custom callback function that logs SSL-related data from the web request to the agent's logs.</summary> @@ -24,38 +26,42 @@ public SslUtil(ITraceWriter trace, bool IgnoreCertificateErrors = false) public bool RequestStatusCustomValidation(HttpRequestMessage requestMessage, X509Certificate2 certificate, X509Chain chain, SslPolicyErrors sslErrors) { bool isRequestSuccessful = (sslErrors == SslPolicyErrors.None); - + if (!isRequestSuccessful) { LoggingRequestDiagnosticData(requestMessage, certificate, chain, sslErrors); } - if (this._IgnoreCertificateErrors) { - this._trace?.Info("Ignoring certificate errors."); + if (this._ignoreCertificateErrors) + { + this._trace?.Info("Ignoring certificate errors."); } - return this._IgnoreCertificateErrors || isRequestSuccessful; + return this._ignoreCertificateErrors || isRequestSuccessful; } /// <summary>Logs SSL related data to agent's logs</summary> private void LoggingRequestDiagnosticData(HttpRequestMessage requestMessage, X509Certificate2 certificate, X509Chain chain, SslPolicyErrors sslErrors) { - string diagInfo = "Diagnostic data for request:\n"; - if (this._trace != null) { - diagInfo += SslDiagnosticDataProvider.ResolveSslPolicyErrorsMessage(sslErrors); - diagInfo += SslDiagnosticDataProvider.GetRequestMessageData(requestMessage); - diagInfo += SslDiagnosticDataProvider.GetCertificateData(certificate); - diagInfo += SslDiagnosticDataProvider.GetChainData(chain); + var logBuilder = new SslDiagnosticsLogBuilder(); + logBuilder.AddSslPolicyErrorsMessages(sslErrors); + logBuilder.AddRequestMessageLog(requestMessage); + logBuilder.AddCertificateLog(certificate); + logBuilder.AddChainLog(chain); + + var formattedLog = logBuilder.BuildLog(); - this._trace?.Info(diagInfo); + this._trace?.Info($"Diagnostic data for request:{Environment.NewLine}{formattedLog}"); } } } - public static class SslDiagnosticDataProvider + internal sealed class SslDiagnosticsLogBuilder { + private readonly StringBuilder _logBuilder = new StringBuilder(); + /// <summary>A predefined list of headers to get from the request</summary> private static readonly string[] _requiredRequestHeaders = new[] { @@ -74,122 +80,99 @@ public static class SslDiagnosticDataProvider }; /// <summary> - /// Get diagnostic data about the HTTP request. + /// Add diagnostics data about the HTTP request. /// This method extracts common information about the request itself and the request's headers. /// To expand list of headers please update <see cref="_requiredRequestHeaders"/></summary>. - /// <returns>Diagnostic data as a formatted string</returns> - public static string GetRequestMessageData(HttpRequestMessage requestMessage) + public void AddRequestMessageLog(HttpRequestMessage requestMessage) { // Getting general information about request - string requestDiagInfoHeader = "HttpRequest"; - string diagInfo = string.Empty; - if (requestMessage is null) { - return $"{requestDiagInfoHeader} data is empty"; + _logBuilder.AppendLine($"HttpRequest data is empty"); + return; } - var requestDiagInfo = new List<KeyValuePair<string, string>>(); var requestedUri = requestMessage?.RequestUri.ToString(); var methodType = requestMessage?.Method.ToString(); - requestDiagInfo.Add(new KeyValuePair<string, string>("Requested URI", requestedUri)); - requestDiagInfo.Add(new KeyValuePair<string, string>("Request method", methodType)); - - diagInfo = GetFormattedData(requestDiagInfoHeader, requestDiagInfo); + _logBuilder.AppendLine($"[HttpRequest]"); + _logBuilder.AppendLine($"Requested URI: {requestedUri}"); + _logBuilder.AppendLine($"Requested method: {methodType}"); // Getting informantion from headers var requestHeaders = requestMessage?.Headers; if (requestHeaders is null) { - return diagInfo; + return; } - string headersDiagInfoHeader = "HttpRequestHeaders"; - - var headersDiagInfo = new List<KeyValuePair<string, string>>(); + _logBuilder.AppendLine($"[HttpRequestHeaders]"); foreach (var headerKey in _requiredRequestHeaders) { IEnumerable<string> headerValues; if (requestHeaders.TryGetValues(headerKey, out headerValues)) { - var headerValue = string.Join(", ", headerValues.ToArray()); - if (headerValue != null) - { - headersDiagInfo.Add(new KeyValuePair<string, string>(headerKey, headerValue.ToString())); - } + _logBuilder.AppendLine($"{headerKey}: {string.Join(", ", headerValues.ToArray())}"); } } - - diagInfo += GetFormattedData(headersDiagInfoHeader, headersDiagInfo); - - return diagInfo; } /// <summary> - /// Get diagnostic data about the certificate. + /// Add diagnostics data about the certificate. /// This method extracts common information about the certificate. /// </summary> - /// <returns>Diagnostic data as a formatted string</returns> - public static string GetCertificateData(X509Certificate2 certificate) + public void AddCertificateLog(X509Certificate2 certificate) { - string diagInfoHeader = "Certificate"; var diagInfo = new List<KeyValuePair<string, string>>(); if (certificate is null) { - return $"{diagInfoHeader} data is empty"; + _logBuilder.AppendLine($"Certificate data is empty"); + return; } - diagInfo.Add(new KeyValuePair<string, string>("Effective date", certificate?.GetEffectiveDateString())); - diagInfo.Add(new KeyValuePair<string, string>("Expiration date", certificate?.GetExpirationDateString())); - diagInfo.Add(new KeyValuePair<string, string>("Issuer", certificate?.Issuer)); - diagInfo.Add(new KeyValuePair<string, string>("Subject", certificate?.Subject)); - - return GetFormattedData(diagInfoHeader, diagInfo); + _logBuilder.AppendLine($"[Certificate]"); + AddCertificateData(certificate); } /// <summary> - /// Get diagnostic data about the chain. + /// Add diagnostics data about the chain. /// This method extracts common information about the chain. /// </summary> - /// <returns>Diagnostic data as a formatted string</returns> - public static string GetChainData(X509Chain chain) + public void AddChainLog(X509Chain chain) { - string diagInfoHeader = "ChainStatus"; - var diagInfo = new List<KeyValuePair<string, string>>(); - - if (chain is null) + if (chain is null || chain.ChainElements is null) { - return $"{diagInfoHeader} data is empty"; + _logBuilder.AppendLine($"ChainElements data is empty"); + return; } - foreach (var status in chain.ChainStatus) + _logBuilder.AppendLine("[ChainElements]"); + foreach (var chainElement in chain.ChainElements) { - diagInfo.Add(new KeyValuePair<string, string>("Status", status.Status.ToString())); - diagInfo.Add(new KeyValuePair<string, string>("Status Information", status.StatusInformation)); + AddCertificateData(chainElement.Certificate); + foreach (var status in chainElement.ChainElementStatus) + { + _logBuilder.AppendLine($"Status: {status.Status}"); + _logBuilder.AppendLine($"Status Information: {status.StatusInformation}"); + } } - - return GetFormattedData(diagInfoHeader, diagInfo); } /// <summary> - /// Get list of SSL policy errors with descriptions. + /// Add list of SSL policy errors with descriptions. /// This method checks SSL policy errors and mapping them to user-friendly descriptions. /// To update SSL policy errors description please update <see cref="_sslPolicyErrorsMapping"/>. /// </summary> - /// <returns>Diagnostic data as a formatted string</returns> - public static string ResolveSslPolicyErrorsMessage(SslPolicyErrors sslErrors) + public void AddSslPolicyErrorsMessages(SslPolicyErrors sslErrors) { - string diagInfoHeader = $"SSL Policy Errors"; - var diagInfo = new List<KeyValuePair<string, string>>(); + _logBuilder.AppendLine($"[SSL Policy Errors]"); if (sslErrors == SslPolicyErrors.None) { - diagInfo.Add(new KeyValuePair<string, string>(sslErrors.ToString(), _sslPolicyErrorsMapping[sslErrors])); - return GetFormattedData(diagInfoHeader, diagInfo); + _logBuilder.AppendLine($"No SSL policy errors"); } // Since we can get several SSL policy errors we should check all of them @@ -197,33 +180,30 @@ public static string ResolveSslPolicyErrorsMessage(SslPolicyErrors sslErrors) { if ((sslErrors & errorCode) != 0) { - string errorValue = errorCode.ToString(); - string errorMessage = string.Empty; - - if (!_sslPolicyErrorsMapping.TryGetValue(errorCode, out errorMessage)) + if (!_sslPolicyErrorsMapping.ContainsKey(errorCode)) { - errorMessage = "Could not resolve related error message"; + _logBuilder.AppendLine($"{errorCode.ToString()}: Could not resolve related error message"); + } + else + { + _logBuilder.AppendLine($"{errorCode.ToString()}: {_sslPolicyErrorsMapping[errorCode]}"); } - - diagInfo.Add(new KeyValuePair<string, string>(errorValue, errorMessage)); } } - - return GetFormattedData(diagInfoHeader, diagInfo); } - /// <summary>Get diagnostic data as formatted text</summary> - /// <returns>Formatted string</returns> - private static string GetFormattedData(string diagInfoHeader, List<KeyValuePair<string, string>> diagInfo) + public string BuildLog() { - string formattedData = $"[{diagInfoHeader}]\n"; + return _logBuilder.ToString(); + } - foreach (var record in diagInfo) - { - formattedData += $"{record.Key}: {record.Value}\n"; - } - return formattedData; + private void AddCertificateData(X509Certificate2 certificate) + { + _logBuilder.AppendLine($"Effective date: {certificate?.GetEffectiveDateString()}"); + _logBuilder.AppendLine($"Expiration date: {certificate?.GetExpirationDateString()}"); + _logBuilder.AppendLine($"Issuer: {certificate?.Issuer}"); + _logBuilder.AppendLine($"Subject: {certificate?.Subject}"); } } } From 555a7cae7209c8d1ee677ca4e87be9c3cd016381 Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Mon, 8 Apr 2024 21:18:27 +0200 Subject: [PATCH 04/12] Added initial tests --- src/Test/L0/Util/SslUtilL0.cs | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 src/Test/L0/Util/SslUtilL0.cs diff --git a/src/Test/L0/Util/SslUtilL0.cs b/src/Test/L0/Util/SslUtilL0.cs new file mode 100644 index 0000000000..7f48c46b8c --- /dev/null +++ b/src/Test/L0/Util/SslUtilL0.cs @@ -0,0 +1,45 @@ +using Microsoft.VisualStudio.Services.Agent.Util; +using System.Net.Http; +using Xunit; + +namespace Test.L0.Util +{ + public sealed class SslUtilL0 + { + [Fact] + public void AddRequestMessageLog_RequestMessageIsNull_ShouldReturnCorrectLog() + { + // Arrange + HttpRequestMessage requestMessage = null; + var logBuilder = new SslDiagnosticsLogBuilder(); + + // Act + logBuilder.AddRequestMessageLog(requestMessage); + var log = logBuilder.BuildLog(); + + // Assert + Assert.Equal("Request message is null.", log); + } + + [Fact] + public void AddRequestMessageLog_RequestMessageIsNotNull_ShouldReturnCorrectLog() + { + // Arrange + var requestMessage = new HttpRequestMessage(HttpMethod.Get, "http://localhost"); + var logBuilder = new SslDiagnosticsLogBuilder(); + var log = string.Empty; + + // Act + using (requestMessage) + { + logBuilder.AddRequestMessageLog(requestMessage); + log = logBuilder.BuildLog(); + } + + // Assert + Assert.Contains("Request message:", log); + Assert.Contains("Method: GET", log); + Assert.Contains("Request URI: http://localhost/", log); + } + } +} From a6814e89d1fdc412c5bc1db0ff543dcd98dee682 Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Tue, 23 Apr 2024 22:12:50 +0200 Subject: [PATCH 05/12] Added new tests --- src/Agent.Sdk/Util/SslUtil.cs | 2 +- src/Test/L0/Util/SslUtilL0.cs | 77 ++++++++++++++++++++++++++++++++--- 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/Agent.Sdk/Util/SslUtil.cs b/src/Agent.Sdk/Util/SslUtil.cs index 7cdc278bd6..7b2dddd460 100644 --- a/src/Agent.Sdk/Util/SslUtil.cs +++ b/src/Agent.Sdk/Util/SslUtil.cs @@ -102,7 +102,7 @@ public void AddRequestMessageLog(HttpRequestMessage requestMessage) // Getting informantion from headers var requestHeaders = requestMessage?.Headers; - if (requestHeaders is null) + if (requestHeaders is null || !requestHeaders.Any()) { return; } diff --git a/src/Test/L0/Util/SslUtilL0.cs b/src/Test/L0/Util/SslUtilL0.cs index 7f48c46b8c..7657c3c02f 100644 --- a/src/Test/L0/Util/SslUtilL0.cs +++ b/src/Test/L0/Util/SslUtilL0.cs @@ -1,8 +1,10 @@ using Microsoft.VisualStudio.Services.Agent.Util; +using System; using System.Net.Http; +using System.Security.Cryptography.X509Certificates; using Xunit; -namespace Test.L0.Util +namespace Microsoft.VisualStudio.Services.Agent.Tests.Util { public sealed class SslUtilL0 { @@ -18,7 +20,7 @@ public void AddRequestMessageLog_RequestMessageIsNull_ShouldReturnCorrectLog() var log = logBuilder.BuildLog(); // Assert - Assert.Equal("Request message is null.", log); + Assert.Equal($"HttpRequest data is empty{Environment.NewLine}", log); } [Fact] @@ -37,9 +39,74 @@ public void AddRequestMessageLog_RequestMessageIsNotNull_ShouldReturnCorrectLog( } // Assert - Assert.Contains("Request message:", log); - Assert.Contains("Method: GET", log); - Assert.Contains("Request URI: http://localhost/", log); + Assert.Contains("Requested URI: http://localhost/", log); + Assert.Contains("Requested method: GET", log); + } + + [Fact] + public void AddRequestMessageLog_RequestMessageHasHeaders_ShouldReturnCorrectLog() + { + // Arrange + var requestMessage = new HttpRequestMessage(HttpMethod.Get, "http://localhost"); + requestMessage.Headers.Add("X-TFS-Session", "value1"); + requestMessage.Headers.Add("X-VSS-E2EID", "value2_1"); + requestMessage.Headers.Add("X-VSS-E2EID", "value2_2"); + requestMessage.Headers.Add("User-Agent", "value3"); + requestMessage.Headers.Add("CustomHeader", "CustomValue"); + var logBuilder = new SslDiagnosticsLogBuilder(); + var log = string.Empty; + + // Act + using (requestMessage) + { + logBuilder.AddRequestMessageLog(requestMessage); + log = logBuilder.BuildLog(); + } + + // Assert + Assert.Contains("Requested URI: http://localhost/", log); + Assert.Contains("Requested method: GET", log); + Assert.Contains("X-TFS-Session: value1", log); + Assert.Contains("X-VSS-E2EID: value2_1, value2_2", log); + Assert.Contains("User-Agent: value3", log); + Assert.DoesNotContain("CustomHeader", log); + } + + [Fact] + public void AddCertificateLog_CertificateIsNull_ShouldReturnCorrectLog() + { + // Arrange + var logBuilder = new SslDiagnosticsLogBuilder(); + + // Act + logBuilder.AddCertificateLog(null); + var log = logBuilder.BuildLog(); + + // Assert + Assert.Equal($"Certificate data is empty{Environment.NewLine}", log); + } + + [Fact] + public void AddCertificateLog_CertificateIsNotNull_ShouldReturnCorrectLog() + { + // Arrange + var certificate = new X509Certificate2(); + var logBuilder = new SslDiagnosticsLogBuilder(); + var log = string.Empty; + + // Act + using (certificate) + { + logBuilder.AddCertificateLog(certificate); + log = logBuilder.BuildLog(); + } + + // Assert + Assert.Contains("Subject: ", log); + Assert.Contains("Issuer: ", log); + Assert.Contains("Thumbprint: ", log); + Assert.Contains("Valid from: ", log); + Assert.Contains("Valid until: ", log); } } } From 4c8520fe17be7a92c34eaf29faae915fedc42c16 Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Mon, 20 May 2024 18:48:36 +0200 Subject: [PATCH 06/12] Testing --- src/Agent.Sdk/Util/SslUtil.cs | 1 + src/Test/L0/Util/SslUtilL0.cs | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Agent.Sdk/Util/SslUtil.cs b/src/Agent.Sdk/Util/SslUtil.cs index 7b2dddd460..a74bdb8332 100644 --- a/src/Agent.Sdk/Util/SslUtil.cs +++ b/src/Agent.Sdk/Util/SslUtil.cs @@ -204,6 +204,7 @@ private void AddCertificateData(X509Certificate2 certificate) _logBuilder.AppendLine($"Expiration date: {certificate?.GetExpirationDateString()}"); _logBuilder.AppendLine($"Issuer: {certificate?.Issuer}"); _logBuilder.AppendLine($"Subject: {certificate?.Subject}"); + _logBuilder.AppendLine($"Thumbprint: {certificate?.Thumbprint}"); } } } diff --git a/src/Test/L0/Util/SslUtilL0.cs b/src/Test/L0/Util/SslUtilL0.cs index 7657c3c02f..3dbf1df0b3 100644 --- a/src/Test/L0/Util/SslUtilL0.cs +++ b/src/Test/L0/Util/SslUtilL0.cs @@ -90,7 +90,15 @@ public void AddCertificateLog_CertificateIsNull_ShouldReturnCorrectLog() public void AddCertificateLog_CertificateIsNotNull_ShouldReturnCorrectLog() { // Arrange - var certificate = new X509Certificate2(); +#pragma warning disable SYSLIB0026 // Type or member is obsolete + var certificate = new X509Certificate2() + { + Subject = "CN=TestSubject", + Issuer = "CN=TestIssuer", + Thumbprint = "TestThumbprint", + NotBefore = DateTime.Now + }; +#pragma warning restore SYSLIB0026 // Type or member is obsolete var logBuilder = new SslDiagnosticsLogBuilder(); var log = string.Empty; From 2e118aa67112f4fe0bf4ae090b98cd16d19ed6fc Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Wed, 3 Apr 2024 23:20:57 +0200 Subject: [PATCH 07/12] Reworked SSL certificate logging --- src/Agent.Sdk/Util/SslUtil.cs | 168 +++++++++++++++------------------- 1 file changed, 74 insertions(+), 94 deletions(-) diff --git a/src/Agent.Sdk/Util/SslUtil.cs b/src/Agent.Sdk/Util/SslUtil.cs index 5d6f2b1011..7cdc278bd6 100644 --- a/src/Agent.Sdk/Util/SslUtil.cs +++ b/src/Agent.Sdk/Util/SslUtil.cs @@ -1,22 +1,24 @@ -using System.Net.Security; -using System.Security.Cryptography.X509Certificates; -using System.Net.Http; using Agent.Sdk; +using System; using System.Collections.Generic; using System.Linq; -using System; +using System.Net.Http; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; +using System.Text; namespace Microsoft.VisualStudio.Services.Agent.Util { public sealed class SslUtil { - private ITraceWriter _trace { get; set; } - private bool _IgnoreCertificateErrors { get; set; } + private readonly ITraceWriter _trace; - public SslUtil(ITraceWriter trace, bool IgnoreCertificateErrors = false) + private readonly bool _ignoreCertificateErrors; + + public SslUtil(ITraceWriter trace, bool ignoreCertificateErrors = false) { this._trace = trace; - this._IgnoreCertificateErrors = IgnoreCertificateErrors; + this._ignoreCertificateErrors = ignoreCertificateErrors; } /// <summary>Implementation of custom callback function that logs SSL-related data from the web request to the agent's logs.</summary> @@ -24,38 +26,42 @@ public SslUtil(ITraceWriter trace, bool IgnoreCertificateErrors = false) public bool RequestStatusCustomValidation(HttpRequestMessage requestMessage, X509Certificate2 certificate, X509Chain chain, SslPolicyErrors sslErrors) { bool isRequestSuccessful = (sslErrors == SslPolicyErrors.None); - + if (!isRequestSuccessful) { LoggingRequestDiagnosticData(requestMessage, certificate, chain, sslErrors); } - if (this._IgnoreCertificateErrors) { - this._trace?.Info("Ignoring certificate errors."); + if (this._ignoreCertificateErrors) + { + this._trace?.Info("Ignoring certificate errors."); } - return this._IgnoreCertificateErrors || isRequestSuccessful; + return this._ignoreCertificateErrors || isRequestSuccessful; } /// <summary>Logs SSL related data to agent's logs</summary> private void LoggingRequestDiagnosticData(HttpRequestMessage requestMessage, X509Certificate2 certificate, X509Chain chain, SslPolicyErrors sslErrors) { - string diagInfo = "Diagnostic data for request:\n"; - if (this._trace != null) { - diagInfo += SslDiagnosticDataProvider.ResolveSslPolicyErrorsMessage(sslErrors); - diagInfo += SslDiagnosticDataProvider.GetRequestMessageData(requestMessage); - diagInfo += SslDiagnosticDataProvider.GetCertificateData(certificate); - diagInfo += SslDiagnosticDataProvider.GetChainData(chain); + var logBuilder = new SslDiagnosticsLogBuilder(); + logBuilder.AddSslPolicyErrorsMessages(sslErrors); + logBuilder.AddRequestMessageLog(requestMessage); + logBuilder.AddCertificateLog(certificate); + logBuilder.AddChainLog(chain); + + var formattedLog = logBuilder.BuildLog(); - this._trace?.Info(diagInfo); + this._trace?.Info($"Diagnostic data for request:{Environment.NewLine}{formattedLog}"); } } } - public static class SslDiagnosticDataProvider + internal sealed class SslDiagnosticsLogBuilder { + private readonly StringBuilder _logBuilder = new StringBuilder(); + /// <summary>A predefined list of headers to get from the request</summary> private static readonly string[] _requiredRequestHeaders = new[] { @@ -74,122 +80,99 @@ public static class SslDiagnosticDataProvider }; /// <summary> - /// Get diagnostic data about the HTTP request. + /// Add diagnostics data about the HTTP request. /// This method extracts common information about the request itself and the request's headers. /// To expand list of headers please update <see cref="_requiredRequestHeaders"/></summary>. - /// <returns>Diagnostic data as a formatted string</returns> - public static string GetRequestMessageData(HttpRequestMessage requestMessage) + public void AddRequestMessageLog(HttpRequestMessage requestMessage) { // Getting general information about request - string requestDiagInfoHeader = "HttpRequest"; - string diagInfo = string.Empty; - if (requestMessage is null) { - return $"{requestDiagInfoHeader} data is empty"; + _logBuilder.AppendLine($"HttpRequest data is empty"); + return; } - var requestDiagInfo = new List<KeyValuePair<string, string>>(); var requestedUri = requestMessage?.RequestUri.ToString(); var methodType = requestMessage?.Method.ToString(); - requestDiagInfo.Add(new KeyValuePair<string, string>("Requested URI", requestedUri)); - requestDiagInfo.Add(new KeyValuePair<string, string>("Request method", methodType)); - - diagInfo = GetFormattedData(requestDiagInfoHeader, requestDiagInfo); + _logBuilder.AppendLine($"[HttpRequest]"); + _logBuilder.AppendLine($"Requested URI: {requestedUri}"); + _logBuilder.AppendLine($"Requested method: {methodType}"); // Getting informantion from headers var requestHeaders = requestMessage?.Headers; if (requestHeaders is null) { - return diagInfo; + return; } - string headersDiagInfoHeader = "HttpRequestHeaders"; - - var headersDiagInfo = new List<KeyValuePair<string, string>>(); + _logBuilder.AppendLine($"[HttpRequestHeaders]"); foreach (var headerKey in _requiredRequestHeaders) { IEnumerable<string> headerValues; if (requestHeaders.TryGetValues(headerKey, out headerValues)) { - var headerValue = string.Join(", ", headerValues.ToArray()); - if (headerValue != null) - { - headersDiagInfo.Add(new KeyValuePair<string, string>(headerKey, headerValue.ToString())); - } + _logBuilder.AppendLine($"{headerKey}: {string.Join(", ", headerValues.ToArray())}"); } } - - diagInfo += GetFormattedData(headersDiagInfoHeader, headersDiagInfo); - - return diagInfo; } /// <summary> - /// Get diagnostic data about the certificate. + /// Add diagnostics data about the certificate. /// This method extracts common information about the certificate. /// </summary> - /// <returns>Diagnostic data as a formatted string</returns> - public static string GetCertificateData(X509Certificate2 certificate) + public void AddCertificateLog(X509Certificate2 certificate) { - string diagInfoHeader = "Certificate"; var diagInfo = new List<KeyValuePair<string, string>>(); if (certificate is null) { - return $"{diagInfoHeader} data is empty"; + _logBuilder.AppendLine($"Certificate data is empty"); + return; } - diagInfo.Add(new KeyValuePair<string, string>("Effective date", certificate?.GetEffectiveDateString())); - diagInfo.Add(new KeyValuePair<string, string>("Expiration date", certificate?.GetExpirationDateString())); - diagInfo.Add(new KeyValuePair<string, string>("Issuer", certificate?.Issuer)); - diagInfo.Add(new KeyValuePair<string, string>("Subject", certificate?.Subject)); - - return GetFormattedData(diagInfoHeader, diagInfo); + _logBuilder.AppendLine($"[Certificate]"); + AddCertificateData(certificate); } /// <summary> - /// Get diagnostic data about the chain. + /// Add diagnostics data about the chain. /// This method extracts common information about the chain. /// </summary> - /// <returns>Diagnostic data as a formatted string</returns> - public static string GetChainData(X509Chain chain) + public void AddChainLog(X509Chain chain) { - string diagInfoHeader = "ChainStatus"; - var diagInfo = new List<KeyValuePair<string, string>>(); - - if (chain is null) + if (chain is null || chain.ChainElements is null) { - return $"{diagInfoHeader} data is empty"; + _logBuilder.AppendLine($"ChainElements data is empty"); + return; } - foreach (var status in chain.ChainStatus) + _logBuilder.AppendLine("[ChainElements]"); + foreach (var chainElement in chain.ChainElements) { - diagInfo.Add(new KeyValuePair<string, string>("Status", status.Status.ToString())); - diagInfo.Add(new KeyValuePair<string, string>("Status Information", status.StatusInformation)); + AddCertificateData(chainElement.Certificate); + foreach (var status in chainElement.ChainElementStatus) + { + _logBuilder.AppendLine($"Status: {status.Status}"); + _logBuilder.AppendLine($"Status Information: {status.StatusInformation}"); + } } - - return GetFormattedData(diagInfoHeader, diagInfo); } /// <summary> - /// Get list of SSL policy errors with descriptions. + /// Add list of SSL policy errors with descriptions. /// This method checks SSL policy errors and mapping them to user-friendly descriptions. /// To update SSL policy errors description please update <see cref="_sslPolicyErrorsMapping"/>. /// </summary> - /// <returns>Diagnostic data as a formatted string</returns> - public static string ResolveSslPolicyErrorsMessage(SslPolicyErrors sslErrors) + public void AddSslPolicyErrorsMessages(SslPolicyErrors sslErrors) { - string diagInfoHeader = $"SSL Policy Errors"; - var diagInfo = new List<KeyValuePair<string, string>>(); + _logBuilder.AppendLine($"[SSL Policy Errors]"); if (sslErrors == SslPolicyErrors.None) { - diagInfo.Add(new KeyValuePair<string, string>(sslErrors.ToString(), _sslPolicyErrorsMapping[sslErrors])); - return GetFormattedData(diagInfoHeader, diagInfo); + _logBuilder.AppendLine($"No SSL policy errors"); } // Since we can get several SSL policy errors we should check all of them @@ -197,33 +180,30 @@ public static string ResolveSslPolicyErrorsMessage(SslPolicyErrors sslErrors) { if ((sslErrors & errorCode) != 0) { - string errorValue = errorCode.ToString(); - string errorMessage = string.Empty; - - if (!_sslPolicyErrorsMapping.TryGetValue(errorCode, out errorMessage)) + if (!_sslPolicyErrorsMapping.ContainsKey(errorCode)) { - errorMessage = "Could not resolve related error message"; + _logBuilder.AppendLine($"{errorCode.ToString()}: Could not resolve related error message"); + } + else + { + _logBuilder.AppendLine($"{errorCode.ToString()}: {_sslPolicyErrorsMapping[errorCode]}"); } - - diagInfo.Add(new KeyValuePair<string, string>(errorValue, errorMessage)); } } - - return GetFormattedData(diagInfoHeader, diagInfo); } - /// <summary>Get diagnostic data as formatted text</summary> - /// <returns>Formatted string</returns> - private static string GetFormattedData(string diagInfoHeader, List<KeyValuePair<string, string>> diagInfo) + public string BuildLog() { - string formattedData = $"[{diagInfoHeader}]\n"; + return _logBuilder.ToString(); + } - foreach (var record in diagInfo) - { - formattedData += $"{record.Key}: {record.Value}\n"; - } - return formattedData; + private void AddCertificateData(X509Certificate2 certificate) + { + _logBuilder.AppendLine($"Effective date: {certificate?.GetEffectiveDateString()}"); + _logBuilder.AppendLine($"Expiration date: {certificate?.GetExpirationDateString()}"); + _logBuilder.AppendLine($"Issuer: {certificate?.Issuer}"); + _logBuilder.AppendLine($"Subject: {certificate?.Subject}"); } } } From d71e4fedcd84c4569643c8c029229e10d165c9f9 Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Mon, 8 Apr 2024 21:18:27 +0200 Subject: [PATCH 08/12] Added initial tests --- src/Test/L0/Util/SslUtilL0.cs | 45 +++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 src/Test/L0/Util/SslUtilL0.cs diff --git a/src/Test/L0/Util/SslUtilL0.cs b/src/Test/L0/Util/SslUtilL0.cs new file mode 100644 index 0000000000..7f48c46b8c --- /dev/null +++ b/src/Test/L0/Util/SslUtilL0.cs @@ -0,0 +1,45 @@ +using Microsoft.VisualStudio.Services.Agent.Util; +using System.Net.Http; +using Xunit; + +namespace Test.L0.Util +{ + public sealed class SslUtilL0 + { + [Fact] + public void AddRequestMessageLog_RequestMessageIsNull_ShouldReturnCorrectLog() + { + // Arrange + HttpRequestMessage requestMessage = null; + var logBuilder = new SslDiagnosticsLogBuilder(); + + // Act + logBuilder.AddRequestMessageLog(requestMessage); + var log = logBuilder.BuildLog(); + + // Assert + Assert.Equal("Request message is null.", log); + } + + [Fact] + public void AddRequestMessageLog_RequestMessageIsNotNull_ShouldReturnCorrectLog() + { + // Arrange + var requestMessage = new HttpRequestMessage(HttpMethod.Get, "http://localhost"); + var logBuilder = new SslDiagnosticsLogBuilder(); + var log = string.Empty; + + // Act + using (requestMessage) + { + logBuilder.AddRequestMessageLog(requestMessage); + log = logBuilder.BuildLog(); + } + + // Assert + Assert.Contains("Request message:", log); + Assert.Contains("Method: GET", log); + Assert.Contains("Request URI: http://localhost/", log); + } + } +} From 1e82f2b39e909d9949f7405a166075cc741a99e5 Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Tue, 23 Apr 2024 22:12:50 +0200 Subject: [PATCH 09/12] Added new tests --- src/Agent.Sdk/Util/SslUtil.cs | 2 +- src/Test/L0/Util/SslUtilL0.cs | 77 ++++++++++++++++++++++++++++++++--- 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/src/Agent.Sdk/Util/SslUtil.cs b/src/Agent.Sdk/Util/SslUtil.cs index 7cdc278bd6..7b2dddd460 100644 --- a/src/Agent.Sdk/Util/SslUtil.cs +++ b/src/Agent.Sdk/Util/SslUtil.cs @@ -102,7 +102,7 @@ public void AddRequestMessageLog(HttpRequestMessage requestMessage) // Getting informantion from headers var requestHeaders = requestMessage?.Headers; - if (requestHeaders is null) + if (requestHeaders is null || !requestHeaders.Any()) { return; } diff --git a/src/Test/L0/Util/SslUtilL0.cs b/src/Test/L0/Util/SslUtilL0.cs index 7f48c46b8c..7657c3c02f 100644 --- a/src/Test/L0/Util/SslUtilL0.cs +++ b/src/Test/L0/Util/SslUtilL0.cs @@ -1,8 +1,10 @@ using Microsoft.VisualStudio.Services.Agent.Util; +using System; using System.Net.Http; +using System.Security.Cryptography.X509Certificates; using Xunit; -namespace Test.L0.Util +namespace Microsoft.VisualStudio.Services.Agent.Tests.Util { public sealed class SslUtilL0 { @@ -18,7 +20,7 @@ public void AddRequestMessageLog_RequestMessageIsNull_ShouldReturnCorrectLog() var log = logBuilder.BuildLog(); // Assert - Assert.Equal("Request message is null.", log); + Assert.Equal($"HttpRequest data is empty{Environment.NewLine}", log); } [Fact] @@ -37,9 +39,74 @@ public void AddRequestMessageLog_RequestMessageIsNotNull_ShouldReturnCorrectLog( } // Assert - Assert.Contains("Request message:", log); - Assert.Contains("Method: GET", log); - Assert.Contains("Request URI: http://localhost/", log); + Assert.Contains("Requested URI: http://localhost/", log); + Assert.Contains("Requested method: GET", log); + } + + [Fact] + public void AddRequestMessageLog_RequestMessageHasHeaders_ShouldReturnCorrectLog() + { + // Arrange + var requestMessage = new HttpRequestMessage(HttpMethod.Get, "http://localhost"); + requestMessage.Headers.Add("X-TFS-Session", "value1"); + requestMessage.Headers.Add("X-VSS-E2EID", "value2_1"); + requestMessage.Headers.Add("X-VSS-E2EID", "value2_2"); + requestMessage.Headers.Add("User-Agent", "value3"); + requestMessage.Headers.Add("CustomHeader", "CustomValue"); + var logBuilder = new SslDiagnosticsLogBuilder(); + var log = string.Empty; + + // Act + using (requestMessage) + { + logBuilder.AddRequestMessageLog(requestMessage); + log = logBuilder.BuildLog(); + } + + // Assert + Assert.Contains("Requested URI: http://localhost/", log); + Assert.Contains("Requested method: GET", log); + Assert.Contains("X-TFS-Session: value1", log); + Assert.Contains("X-VSS-E2EID: value2_1, value2_2", log); + Assert.Contains("User-Agent: value3", log); + Assert.DoesNotContain("CustomHeader", log); + } + + [Fact] + public void AddCertificateLog_CertificateIsNull_ShouldReturnCorrectLog() + { + // Arrange + var logBuilder = new SslDiagnosticsLogBuilder(); + + // Act + logBuilder.AddCertificateLog(null); + var log = logBuilder.BuildLog(); + + // Assert + Assert.Equal($"Certificate data is empty{Environment.NewLine}", log); + } + + [Fact] + public void AddCertificateLog_CertificateIsNotNull_ShouldReturnCorrectLog() + { + // Arrange + var certificate = new X509Certificate2(); + var logBuilder = new SslDiagnosticsLogBuilder(); + var log = string.Empty; + + // Act + using (certificate) + { + logBuilder.AddCertificateLog(certificate); + log = logBuilder.BuildLog(); + } + + // Assert + Assert.Contains("Subject: ", log); + Assert.Contains("Issuer: ", log); + Assert.Contains("Thumbprint: ", log); + Assert.Contains("Valid from: ", log); + Assert.Contains("Valid until: ", log); } } } From 5f751d2c8ff68e4121af44105d6d6fd49725edef Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Mon, 20 May 2024 18:48:36 +0200 Subject: [PATCH 10/12] Testing --- src/Agent.Sdk/Util/SslUtil.cs | 1 + src/Test/L0/Util/SslUtilL0.cs | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Agent.Sdk/Util/SslUtil.cs b/src/Agent.Sdk/Util/SslUtil.cs index 7b2dddd460..a74bdb8332 100644 --- a/src/Agent.Sdk/Util/SslUtil.cs +++ b/src/Agent.Sdk/Util/SslUtil.cs @@ -204,6 +204,7 @@ private void AddCertificateData(X509Certificate2 certificate) _logBuilder.AppendLine($"Expiration date: {certificate?.GetExpirationDateString()}"); _logBuilder.AppendLine($"Issuer: {certificate?.Issuer}"); _logBuilder.AppendLine($"Subject: {certificate?.Subject}"); + _logBuilder.AppendLine($"Thumbprint: {certificate?.Thumbprint}"); } } } diff --git a/src/Test/L0/Util/SslUtilL0.cs b/src/Test/L0/Util/SslUtilL0.cs index 7657c3c02f..3dbf1df0b3 100644 --- a/src/Test/L0/Util/SslUtilL0.cs +++ b/src/Test/L0/Util/SslUtilL0.cs @@ -90,7 +90,15 @@ public void AddCertificateLog_CertificateIsNull_ShouldReturnCorrectLog() public void AddCertificateLog_CertificateIsNotNull_ShouldReturnCorrectLog() { // Arrange - var certificate = new X509Certificate2(); +#pragma warning disable SYSLIB0026 // Type or member is obsolete + var certificate = new X509Certificate2() + { + Subject = "CN=TestSubject", + Issuer = "CN=TestIssuer", + Thumbprint = "TestThumbprint", + NotBefore = DateTime.Now + }; +#pragma warning restore SYSLIB0026 // Type or member is obsolete var logBuilder = new SslDiagnosticsLogBuilder(); var log = string.Empty; From 8ee12b25ddbea3ad4dd11706bc04055d93a99ffd Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Wed, 19 Jun 2024 22:29:07 +0200 Subject: [PATCH 11/12] Added remaining unit tests --- src/Test/L0/Util/SslUtilL0.cs | 95 +++++++++++++++++++++++++++++++---- 1 file changed, 84 insertions(+), 11 deletions(-) diff --git a/src/Test/L0/Util/SslUtilL0.cs b/src/Test/L0/Util/SslUtilL0.cs index 3dbf1df0b3..2f57ea7e7c 100644 --- a/src/Test/L0/Util/SslUtilL0.cs +++ b/src/Test/L0/Util/SslUtilL0.cs @@ -1,6 +1,8 @@ using Microsoft.VisualStudio.Services.Agent.Util; using System; using System.Net.Http; +using System.Net.Security; +using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Xunit; @@ -8,6 +10,36 @@ namespace Microsoft.VisualStudio.Services.Agent.Tests.Util { public sealed class SslUtilL0 { + [Fact] + public void AddSslPolicyErrorsMesssages_NoErrors_ShouldReturnCorrectLog() + { + // Arrange + var logBuilder = new SslDiagnosticsLogBuilder(); + + // Act + logBuilder.AddSslPolicyErrorsMessages(SslPolicyErrors.None); + var log = logBuilder.BuildLog(); + + // Assert + Assert.Contains("No SSL policy errors", log); + } + + [Fact] + public void AddSslPolicyErrorsMesssages_HasErrors_ShouldReturnCorrectLog() + { + // Arrange + var logBuilder = new SslDiagnosticsLogBuilder(); + + // Act + logBuilder.AddSslPolicyErrorsMessages(SslPolicyErrors.RemoteCertificateNameMismatch); + logBuilder.AddSslPolicyErrorsMessages(SslPolicyErrors.RemoteCertificateChainErrors); + var log = logBuilder.BuildLog(); + + // Assert + Assert.Contains("ChainStatus has returned a non empty array", log); + Assert.Contains("Certificate name mismatch", log); + } + [Fact] public void AddRequestMessageLog_RequestMessageIsNull_ShouldReturnCorrectLog() { @@ -90,15 +122,7 @@ public void AddCertificateLog_CertificateIsNull_ShouldReturnCorrectLog() public void AddCertificateLog_CertificateIsNotNull_ShouldReturnCorrectLog() { // Arrange -#pragma warning disable SYSLIB0026 // Type or member is obsolete - var certificate = new X509Certificate2() - { - Subject = "CN=TestSubject", - Issuer = "CN=TestIssuer", - Thumbprint = "TestThumbprint", - NotBefore = DateTime.Now - }; -#pragma warning restore SYSLIB0026 // Type or member is obsolete + var certificate = GenerateTestCertificate(); var logBuilder = new SslDiagnosticsLogBuilder(); var log = string.Empty; @@ -110,11 +134,60 @@ public void AddCertificateLog_CertificateIsNotNull_ShouldReturnCorrectLog() } // Assert + Assert.Contains("Effective date: ", log); + Assert.Contains("Expiration date: ", log); + Assert.Contains("Subject: ", log); + Assert.Contains("Issuer: ", log); + Assert.Contains("Thumbprint: ", log); + } + + [Fact] + public void AddChainLog_ChainIsNull_ShouldReturnCorrectLog() + { + // Arrange + var logBuilder = new SslDiagnosticsLogBuilder(); + + // Act + logBuilder.AddChainLog(null); + var log = logBuilder.BuildLog(); + + // Assert + Assert.Equal($"ChainElements data is empty{Environment.NewLine}", log); + } + + [Fact] + public void AddChainLog_ChainIsNotNull_ShouldReturnCorrectLog() + { + // Arrange + var certificate = GenerateTestCertificate(); + using var chain = new X509Chain(); + chain.Build(certificate); + var logBuilder = new SslDiagnosticsLogBuilder(); + var log = string.Empty; + + // Act + using (certificate) + { + logBuilder.AddChainLog(chain); + log = logBuilder.BuildLog(); + } + + // Assert + Assert.Contains("[ChainElements]", log); + Assert.Contains("Effective date: ", log); + Assert.Contains("Expiration date: ", log); Assert.Contains("Subject: ", log); Assert.Contains("Issuer: ", log); Assert.Contains("Thumbprint: ", log); - Assert.Contains("Valid from: ", log); - Assert.Contains("Valid until: ", log); + } + + private X509Certificate2 GenerateTestCertificate() + { + using var ecdsa = ECDsa.Create(); // generate asymmetric key pair + var req = new CertificateRequest("CN=TestSubject", ecdsa, HashAlgorithmName.SHA256); + var cert = req.CreateSelfSigned(DateTimeOffset.Now, DateTimeOffset.Now.AddYears(5)); + + return cert; } } } From a1837b06e409c7fce7ff28694f369ff5fa243b7a Mon Sep 17 00:00:00 2001 From: Evgenii Dergachev <84813671+DergachevE@users.noreply.github.com> Date: Wed, 19 Jun 2024 22:32:39 +0200 Subject: [PATCH 12/12] Fixed formatting --- src/Test/L0/Util/SslUtilL0.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Test/L0/Util/SslUtilL0.cs b/src/Test/L0/Util/SslUtilL0.cs index 1ff4180d8a..2f57ea7e7c 100644 --- a/src/Test/L0/Util/SslUtilL0.cs +++ b/src/Test/L0/Util/SslUtilL0.cs @@ -143,7 +143,7 @@ public void AddCertificateLog_CertificateIsNotNull_ShouldReturnCorrectLog() [Fact] public void AddChainLog_ChainIsNull_ShouldReturnCorrectLog() - { + { // Arrange var logBuilder = new SslDiagnosticsLogBuilder();