From 560861664efee967572b03bbff42ae4bce751616 Mon Sep 17 00:00:00 2001 From: Swetha Ravichandran Date: Fri, 13 May 2022 04:07:08 +0530 Subject: [PATCH] Fix #197 - AWSEKSResourceDetector fails to detect resources due to exception "The SSL connection could not be established" (#208) * #197 - AWSEKSResourceDetector fails to detect resources due to exception "The SSL connection could not be established" Issue Description: With the current code, it requires an additional step of installing kubernetes self-signed certificate to the trusted store as .NET core expects them to the trsuted store to make a secure connection. Fix Provided: ServerCertificationValidationProvider - Safely loads the certificate to the trusted collection along with ServerSideValidation. This would avoid the unnecessary step of installing the certificate manually. Handler - Creates HttpClientHandler with client certificate. (cherry picked from commit 41e68d3c55ec3c61604a9c5ba07242747f5a17e9) * PR Review Comments (cherry picked from commit 0b5d9a9360c35e5db93f92e22fbea5f43b1adfd0) * Address PR comments (cherry picked from commit 4aa1c6213aabc78e60b841632ebc4db03cd4986c) * Remove unused import * Added Unit Tests * Removed unused directive * Added Trait for codecov build * Use temporary files to store test certificate * Fixed File Access Issue in Unit Test * Use TempFileName for Unit Tests * Added Debug logging for build test Fixed For loop tries * Revert "Added Debug logging for build test" This reverts commit b9823252f545324efc0031b0b654462085cb1f68. * Fixed tries logic in for loop Co-authored-by: Prashant Srivastava <50466688+srprash@users.noreply.github.com> Co-authored-by: Igor Kiselev --- .../AWSXRayEventSource.cs | 6 + ...elemetry.Contrib.Extensions.AWSXRay.csproj | 4 +- .../Resources/AWSEKSResourceDetector.cs | 25 +-- .../Resources/Http/Handler.cs | 51 ++++++ .../ServerCertificateValidationProvider.cs | 161 ++++++++++++++++++ ...ry.Contrib.Extensions.AWSXRay.Tests.csproj | 4 + .../Resources/Http/CertificateUploader.cs | 84 +++++++++ .../Resources/Http/TestHandler.cs | 46 +++++ ...TestServerCertificateValidationProvider.cs | 62 +++++++ 9 files changed, 426 insertions(+), 17 deletions(-) create mode 100644 src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/Http/Handler.cs create mode 100644 src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/Http/ServerCertificateValidationProvider.cs create mode 100644 test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/CertificateUploader.cs create mode 100644 test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/TestHandler.cs create mode 100644 test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/TestServerCertificateValidationProvider.cs diff --git a/src/OpenTelemetry.Contrib.Extensions.AWSXRay/AWSXRayEventSource.cs b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/AWSXRayEventSource.cs index 83e48a8931..945c09dc05 100644 --- a/src/OpenTelemetry.Contrib.Extensions.AWSXRay/AWSXRayEventSource.cs +++ b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/AWSXRayEventSource.cs @@ -62,6 +62,12 @@ public void FailedToExtractResourceAttributes(string format, string exception) this.WriteEvent(3, format, exception); } + [Event(4, Message = "Failed to validate certificate in format: '{0}', error: '{1}'.", Level = EventLevel.Warning)] + public void FailedToValidateCertificate(string format, string error) + { + this.WriteEvent(4, format, error); + } + /// /// Returns a culture-independent string representation of the given object, /// appropriate for diagnostics tracing. diff --git a/src/OpenTelemetry.Contrib.Extensions.AWSXRay/OpenTelemetry.Contrib.Extensions.AWSXRay.csproj b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/OpenTelemetry.Contrib.Extensions.AWSXRay.csproj index 9e6e756ea4..108b059cf1 100644 --- a/src/OpenTelemetry.Contrib.Extensions.AWSXRay/OpenTelemetry.Contrib.Extensions.AWSXRay.csproj +++ b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/OpenTelemetry.Contrib.Extensions.AWSXRay.csproj @@ -1,4 +1,4 @@ - + net452;netstandard2.0 OpenTelemetry extensions for AWS X-Ray. @@ -15,6 +15,8 @@ + + diff --git a/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/AWSEKSResourceDetector.cs b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/AWSEKSResourceDetector.cs index 2bc5850515..7be2e1ebd9 100644 --- a/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/AWSEKSResourceDetector.cs +++ b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/AWSEKSResourceDetector.cs @@ -17,8 +17,8 @@ using System; using System.Collections.Generic; using System.Net.Http; -using System.Security.Cryptography.X509Certificates; using System.Text; +using OpenTelemetry.Contrib.Extensions.AWSXRay.Resources.Http; using OpenTelemetry.Contrib.Extensions.AWSXRay.Resources.Models; namespace OpenTelemetry.Contrib.Extensions.AWSXRay.Resources @@ -41,13 +41,15 @@ public class AWSEKSResourceDetector : IResourceDetector public IEnumerable> Detect() { var credentials = this.GetEKSCredentials(AWSEKSCredentialPath); - if (credentials == null || !this.IsEKSProcess(credentials)) + var httpClientHandler = Handler.Create(AWSEKSCertificatePath); + + if (credentials == null || !this.IsEKSProcess(credentials, httpClientHandler)) { return null; } return this.ExtractResourceAttributes( - this.GetEKSClusterName(credentials), + this.GetEKSClusterName(credentials, httpClientHandler), this.GetEKSContainerId(AWSEKSMetadataFilePath)); } @@ -125,11 +127,11 @@ internal AWSEKSClusterInformationModel DeserializeResponse(string response) return ResourceDetectorUtils.DeserializeFromString(response); } - private string GetEKSClusterName(string credentials) + private string GetEKSClusterName(string credentials, HttpClientHandler httpClientHandler) { try { - var clusterInfo = this.GetEKSClusterInfo(credentials); + var clusterInfo = this.GetEKSClusterInfo(credentials, httpClientHandler); return this.DeserializeResponse(clusterInfo)?.Data?.ClusterName; } catch (Exception ex) @@ -140,12 +142,11 @@ private string GetEKSClusterName(string credentials) return null; } - private bool IsEKSProcess(string credentials) + private bool IsEKSProcess(string credentials, HttpClientHandler httpClientHandler) { string awsAuth = null; try { - var httpClientHandler = this.CreateHttpClientHandler(); awsAuth = ResourceDetectorUtils.SendOutRequest(AWSAuthUrl, "GET", new KeyValuePair("Authorization", credentials), httpClientHandler).Result; } catch (Exception ex) @@ -156,17 +157,9 @@ private bool IsEKSProcess(string credentials) return !string.IsNullOrEmpty(awsAuth); } - private string GetEKSClusterInfo(string credentials) + private string GetEKSClusterInfo(string credentials, HttpClientHandler httpClientHandler) { - var httpClientHandler = this.CreateHttpClientHandler(); return ResourceDetectorUtils.SendOutRequest(AWSClusterInfoUrl, "GET", new KeyValuePair("Authorization", credentials), httpClientHandler).Result; } - - private HttpClientHandler CreateHttpClientHandler() - { - var httpClientHandler = new HttpClientHandler(); - httpClientHandler.ClientCertificates.Add(new X509Certificate2(AWSEKSCertificatePath)); - return httpClientHandler; - } } } diff --git a/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/Http/Handler.cs b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/Http/Handler.cs new file mode 100644 index 0000000000..2b3326deb2 --- /dev/null +++ b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/Http/Handler.cs @@ -0,0 +1,51 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Net.Http; + +namespace OpenTelemetry.Contrib.Extensions.AWSXRay.Resources.Http +{ + internal class Handler + { + public static HttpClientHandler Create(string certificateFile) + { + try + { + ServerCertificateValidationProvider serverCertificateValidationProvider = + ServerCertificateValidationProvider.FromCertificateFile(certificateFile); + + if (!serverCertificateValidationProvider.IsCertificateLoaded) + { + AWSXRayEventSource.Log.FailedToValidateCertificate(nameof(Handler), "Failed to Load the certificate file into trusted collection"); + return null; + } + + var clientHandler = new HttpClientHandler(); + clientHandler.ServerCertificateCustomValidationCallback = + (sender, x509Certificate2, x509Chain, sslPolicyErrors) => + serverCertificateValidationProvider.ValidationCallback(null, x509Certificate2, x509Chain, sslPolicyErrors); + return clientHandler; + } + catch (Exception ex) + { + AWSXRayEventSource.Log.ResourceAttributesExtractException($"{nameof(Handler)} : Failed to create HttpClientHandler", ex); + } + + return null; + } + } +} diff --git a/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/Http/ServerCertificateValidationProvider.cs b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/Http/ServerCertificateValidationProvider.cs new file mode 100644 index 0000000000..ce24067908 --- /dev/null +++ b/src/OpenTelemetry.Contrib.Extensions.AWSXRay/Resources/Http/ServerCertificateValidationProvider.cs @@ -0,0 +1,161 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.IO; +using System.Linq; +using System.Net.Security; +using System.Security.Cryptography.X509Certificates; + +namespace OpenTelemetry.Contrib.Extensions.AWSXRay.Resources.Http +{ + internal class ServerCertificateValidationProvider + { + private static readonly ServerCertificateValidationProvider InvalidProvider = + new ServerCertificateValidationProvider(null); + + private readonly X509Certificate2Collection trustedCertificates; + + private ServerCertificateValidationProvider(X509Certificate2Collection trustedCertificates) + { + if (trustedCertificates == null) + { + this.trustedCertificates = null; + this.ValidationCallback = null; + this.IsCertificateLoaded = false; + return; + } + + this.trustedCertificates = trustedCertificates; + this.ValidationCallback = (sender, cert, chain, errors) => + this.ValidateCertificate(new X509Certificate2(cert), chain, errors); + this.IsCertificateLoaded = true; + } + + public bool IsCertificateLoaded { get; } + + public RemoteCertificateValidationCallback ValidationCallback { get; } + + public static ServerCertificateValidationProvider FromCertificateFile(string certificateFile) + { + if (!File.Exists(certificateFile)) + { + AWSXRayEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Certificate File does not exist"); + return InvalidProvider; + } + + var trustedCertificates = new X509Certificate2Collection(); + if (!LoadCertificateToTrustedCollection(trustedCertificates, certificateFile)) + { + AWSXRayEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Failed to load certificate in trusted collection"); + return InvalidProvider; + } + + return new ServerCertificateValidationProvider(trustedCertificates); + } + + private static bool LoadCertificateToTrustedCollection(X509Certificate2Collection collection, string certFileName) + { + try + { + collection.Import(certFileName); + return true; + } + catch (Exception) + { + return false; + } + } + + private bool ValidateCertificate(X509Certificate2 cert, X509Chain chain, SslPolicyErrors errors) + { + var isSslPolicyPassed = errors == SslPolicyErrors.None || + errors == SslPolicyErrors.RemoteCertificateChainErrors; + if (!isSslPolicyPassed) + { + if ((errors | SslPolicyErrors.RemoteCertificateNotAvailable) == errors) + { + AWSXRayEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Failed to validate certificate due to RemoteCertificateNotAvailable"); + } + + if ((errors | SslPolicyErrors.RemoteCertificateNameMismatch) == errors) + { + AWSXRayEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Failed to validate certificate due to RemoteCertificateNameMismatch"); + } + } + + chain.ChainPolicy.ExtraStore.AddRange(this.trustedCertificates); + chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority; + + // building the chain to process basic validations e.g. signature, use, expiration, revocation + var isValidChain = chain.Build(cert); + + if (!isValidChain) + { + var chainErrors = string.Empty; + foreach (var element in chain.ChainElements) + { + foreach (var status in element.ChainElementStatus) + { + chainErrors += + $"\nCertificate [{element.Certificate.Subject}] Status [{status.Status}]: {status.StatusInformation}"; + } + } + + AWSXRayEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), $"Failed to validate certificate due to {chainErrors}"); + } + + // check if at least one certificate in the chain is in our trust list + var isTrusted = this.HasCommonCertificate(chain, this.trustedCertificates); + if (!isTrusted) + { + var serverCertificates = string.Empty; + foreach (var element in chain.ChainElements) + { + serverCertificates += " " + element.Certificate.Subject; + } + + var trustCertificates = string.Empty; + foreach (var trustCertificate in this.trustedCertificates) + { + trustCertificates += " " + trustCertificate.Subject; + } + + AWSXRayEventSource.Log.FailedToValidateCertificate( + nameof(ServerCertificateValidationProvider), + $"Server Certificates Chain cannot be trusted. The chain doesn't match with the Trusted Certificates provided. Server Certificates:{serverCertificates}. Trusted Certificates:{trustCertificates}"); + } + + return isSslPolicyPassed && isValidChain && isTrusted; + } + + private bool HasCommonCertificate(X509Chain chain, X509Certificate2Collection collection) + { + foreach (var chainElement in chain.ChainElements) + { + foreach (var certificate in collection) + { + if (Enumerable.SequenceEqual(chainElement.Certificate.GetPublicKey(), certificate.GetPublicKey())) + { + return true; + } + } + } + + return false; + } + } +} diff --git a/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests.csproj b/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests.csproj index f596591539..274e57f917 100644 --- a/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests.csproj +++ b/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests.csproj @@ -30,6 +30,10 @@ + + + + diff --git a/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/CertificateUploader.cs b/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/CertificateUploader.cs new file mode 100644 index 0000000000..152f6abc04 --- /dev/null +++ b/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/CertificateUploader.cs @@ -0,0 +1,84 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.IO; +using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; +using System.Threading; + +namespace OpenTelemetry.Contrib.Extensions.AWSXRay.Tests.Resources.Http +{ + internal class CertificateUploader : IDisposable + { + private const string CRTHEADER = "-----BEGIN CERTIFICATE-----\n"; + private const string CRTFOOTER = "\n-----END CERTIFICATE-----"; + private string filePath; + + public CertificateUploader() + { + this.filePath = Path.GetTempFileName(); + } + + public string FilePath + { + get { return this.filePath; } + set { this.filePath = value; } + } + + public void Create() + { + using var rsa = RSA.Create(); + var certRequest = new CertificateRequest("cn=test", rsa, HashAlgorithmName.SHA256, RSASignaturePadding.Pkcs1); + + var subjectAlternativeNames = new SubjectAlternativeNameBuilder(); + subjectAlternativeNames.AddDnsName("test"); + certRequest.CertificateExtensions.Add(subjectAlternativeNames.Build()); + + // Create a temporary certificate and add validity for 1 day + var certificate = certRequest.CreateSelfSigned(DateTimeOffset.Now, DateTimeOffset.Now.AddDays(1)); + + var exportData = certificate.Export(X509ContentType.Cert); + var crt = Convert.ToBase64String(exportData, Base64FormattingOptions.InsertLineBreaks); + + using (FileStream stream = new FileStream(this.filePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.ReadWrite | FileShare.Delete)) + { + using (StreamWriter sw = new StreamWriter(stream)) + { + sw.Write(CRTHEADER + crt + CRTFOOTER); + } + } + } + + public void Dispose() + { + for (int tries = 0; ; tries++) + { + try + { + File.Delete(this.filePath); + return; + } + catch (IOException) when (tries < 3) + { + // the file is unavailable because it is: still being written to or being processed by another thread + // sleep for sometime before deleting + Thread.Sleep(1000); + } + } + } + } +} diff --git a/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/TestHandler.cs b/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/TestHandler.cs new file mode 100644 index 0000000000..4d6b4703e3 --- /dev/null +++ b/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/TestHandler.cs @@ -0,0 +1,46 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using OpenTelemetry.Contrib.Extensions.AWSXRay.Resources.Http; +using Xunit; + +namespace OpenTelemetry.Contrib.Extensions.AWSXRay.Tests.Resources.Http +{ + [Trait("Platform", "Any")] + public class TestHandler + { + private const string INVALIDCRTNAME = "invalidcert"; + + [Fact] + public void TestValidHandler() + { + using (CertificateUploader certificateUploader = new CertificateUploader()) + { + certificateUploader.Create(); + + // Validates if the handler created. + Assert.NotNull(Handler.Create(certificateUploader.FilePath)); + } + } + + [Fact] + public void TestInValidHandler() + { + // Validates if the handler created if no certificate is loaded into the trusted collection + Assert.Null(Handler.Create(INVALIDCRTNAME)); + } + } +} diff --git a/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/TestServerCertificateValidationProvider.cs b/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/TestServerCertificateValidationProvider.cs new file mode 100644 index 0000000000..89355260b4 --- /dev/null +++ b/test/OpenTelemetry.Contrib.Extensions.AWSXRay.Tests/Resources/Http/TestServerCertificateValidationProvider.cs @@ -0,0 +1,62 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System.Security.Cryptography.X509Certificates; +using OpenTelemetry.Contrib.Extensions.AWSXRay.Resources.Http; +using Xunit; + +namespace OpenTelemetry.Contrib.Extensions.AWSXRay.Tests.Resources.Http +{ + [Trait("Platform", "Any")] + public class TestServerCertificateValidationProvider + { + private const string INVALIDCRTNAME = "invalidcert"; + + [Fact] + public void TestValidCertificate() + { + using (CertificateUploader certificateUploader = new CertificateUploader()) + { + certificateUploader.Create(); + + // Loads the certificate to the trusted collection from the file + ServerCertificateValidationProvider serverCertificateValidationProvider = + ServerCertificateValidationProvider.FromCertificateFile(certificateUploader.FilePath); + + // Validates if the certificate loaded into the trusted collection. + Assert.True(serverCertificateValidationProvider.IsCertificateLoaded); + + var certificate = new X509Certificate2(certificateUploader.FilePath); + X509Chain chain = new X509Chain(); + chain.Build(certificate); + + // validates if certificate is valid + Assert.True(serverCertificateValidationProvider.ValidationCallback(null, certificate, chain, System.Net.Security.SslPolicyErrors.None)); + } + } + + [Fact] + public void TestInValidCertificate() + { + // Loads the certificate to the trusted collection from the file + ServerCertificateValidationProvider serverCertificateValidationProvider = + ServerCertificateValidationProvider.FromCertificateFile(INVALIDCRTNAME); + + // Validates if the certificate file loaded. + Assert.False(serverCertificateValidationProvider.IsCertificateLoaded); + } + } +}