Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Existing ServerCertificateValidation Classes to Shared #1550

Closed
wants to merge 8 commits into from
Closed

Move Existing ServerCertificateValidation Classes to Shared #1550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
bc33fc3
Saving changes to switch to another branch
anoopbabu29 Jan 22, 2024
5a5e782
Fully adjusted move to shared
anoopbabu29 Jan 26, 2024
81ace52
Removing shared event source entirely, and using invoked actions for …
anoopbabu29 Jan 29, 2024
dab2b61
Merge branch 'main' into share-certificate-validation
anoopbabu29 Jan 29, 2024
5158730
Remove unncessary file
anoopbabu29 Jan 30, 2024
1fe55dc
Resolving conflict in merge
anoopbabu29 Feb 7, 2024
ef63c27
Updated change to fix the AWS Event Source and changed delegate logge…
anoopbabu29 Feb 7, 2024
fa08aa6
Merge branch 'main' into share-certificate-validation
vishweshbankwar Mar 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions opentelemetry-dotnet-contrib.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{1FCC8E
src\Shared\ExceptionExtensions.cs = src\Shared\ExceptionExtensions.cs
src\Shared\Guard.cs = src\Shared\Guard.cs
src\Shared\InstrumentationEventSource.cs = src\Shared\InstrumentationEventSource.cs
src\Shared\IServerCertificateValidationEventSource.cs = src\Shared\IServerCertificateValidationEventSource.cs
src\Shared\IsExternalInit.cs = src\Shared\IsExternalInit.cs
src\Shared\ListenerHandler.cs = src\Shared\ListenerHandler.cs
src\Shared\MultiTypePropertyFetcher.cs = src\Shared\MultiTypePropertyFetcher.cs
Expand Down
3 changes: 1 addition & 2 deletions src/OpenTelemetry.ResourceDetectors.AWS/AWSEKSResourceDetector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Collections.Generic;
using System.Net.Http;
using System.Text;
using OpenTelemetry.ResourceDetectors.AWS.Http;
using OpenTelemetry.ResourceDetectors.AWS.Models;
using OpenTelemetry.Resources;

Expand All @@ -31,7 +30,7 @@ public sealed class AWSEKSResourceDetector : IResourceDetector
public Resource Detect()
{
var credentials = GetEKSCredentials(AWSEKSCredentialPath);
using var httpClientHandler = Handler.Create(AWSEKSCertificatePath);
using var httpClientHandler = ServerCertificateValidationHandler.Create(AWSEKSCertificatePath, AWSResourcesEventSource.Log);

if (credentials == null || !IsEKSProcess(credentials, httpClientHandler))
{
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry.ResourceDetectors.AWS/AWSResourcesEventSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
namespace OpenTelemetry.ResourceDetectors.AWS;

[EventSource(Name = "OpenTelemetry-ResourceDetectors-AWS")]
internal sealed class AWSResourcesEventSource : EventSource
internal sealed class AWSResourcesEventSource : EventSource, IServerCertificateValidationEventSource
{
public static AWSResourcesEventSource Log = new();

Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry.ResourceDetectors.AWS/OpenTelemetry.ResourceDetectors.AWS.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
<ItemGroup>
<Compile Include="$(RepoRoot)\src\Shared\ExceptionExtensions.cs" Link="Includes\ExceptionExtensions.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
<Compile Include="$(RepoRoot)\src\Shared\IServerCertificateValidationEventSource.cs" Link="Includes\IServerCertificateValidationEventSource.cs" />
<Compile Include="$(RepoRoot)\src\Shared\ServerCertificateValidationHandler.cs" Link="Includes\ServerCertificateValidationHandler.cs" />
<Compile Include="$(RepoRoot)\src\Shared\ServerCertificateValidationProvider.cs" Link="Includes\ServerCertificateValidationProvider.cs" />
</ItemGroup>

</Project>
11 changes: 11 additions & 0 deletions src/Shared/IServerCertificateValidationEventSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

namespace OpenTelemetry.ResourceDetectors;

internal interface IServerCertificateValidationEventSource
{
public void FailedToExtractResourceAttributes(string format, string exception);

public void FailedToValidateCertificate(string format, string error);
}
22 changes: 12 additions & 10 deletions ...try.ResourceDetectors.AWS/Http/Handler.cs → ...red/ServerCertificateValidationHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,35 @@

using System;
using System.Net.Http;
using OpenTelemetry.Internal;

namespace OpenTelemetry.ResourceDetectors.AWS.Http;
namespace OpenTelemetry.ResourceDetectors;

internal class Handler
internal class ServerCertificateValidationHandler
{
public static HttpClientHandler? Create(string certificateFile)
public static HttpClientHandler? Create(string certificateFile, IServerCertificateValidationEventSource? log = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging wasn't really optional before; I'm thinking log should be a required parameter.

I don't want future maintainers to think that log is optional, unless there is a clear (and documented) usecase as to when it doesn't need to be provided.

{
try
{
ServerCertificateValidationProvider? serverCertificateValidationProvider =
ServerCertificateValidationProvider.FromCertificateFile(certificateFile);
ServerCertificateValidationProvider? serverCertificateValidationProvider = ServerCertificateValidationProvider.FromCertificateFile(certificateFile, log);

if (serverCertificateValidationProvider == null)
{
AWSResourcesEventSource.Log.FailedToValidateCertificate(nameof(Handler), "Failed to Load the certificate file into trusted collection");
log?.FailedToValidateCertificate(nameof(ServerCertificateValidationHandler), "Failed to Load the certificate file into trusted collection");
return null;
}

var clientHandler = new HttpClientHandler();
clientHandler.ServerCertificateCustomValidationCallback =
var clientHandler = new HttpClientHandler
{
ServerCertificateCustomValidationCallback =
(sender, x509Certificate2, x509Chain, sslPolicyErrors) =>
serverCertificateValidationProvider.ValidationCallback(sender, x509Certificate2, x509Chain, sslPolicyErrors);
serverCertificateValidationProvider.ValidationCallback(sender, x509Certificate2, x509Chain, sslPolicyErrors),
};
return clientHandler;
}
catch (Exception ex)
{
AWSResourcesEventSource.Log.ResourceAttributesExtractException($"{nameof(Handler)} : Failed to create HttpClientHandler", ex);
log?.FailedToExtractResourceAttributes($"{nameof(ServerCertificateValidationHandler)} : Failed to create HttpClientHandler", ex.ToInvariantString());
}

return null;
Expand Down
27 changes: 15 additions & 12 deletions ...tp/ServerCertificateValidationProvider.cs → ...ed/ServerCertificateValidationProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,37 +9,40 @@
using System.Net.Security;
using System.Security.Cryptography.X509Certificates;

namespace OpenTelemetry.ResourceDetectors.AWS.Http;
namespace OpenTelemetry.ResourceDetectors;

internal class ServerCertificateValidationProvider
{
public static IServerCertificateValidationEventSource? Log;

private readonly X509Certificate2Collection trustedCertificates;

private ServerCertificateValidationProvider(X509Certificate2Collection trustedCertificates)
private ServerCertificateValidationProvider(X509Certificate2Collection trustedCertificates, IServerCertificateValidationEventSource? log = null)
{
this.trustedCertificates = trustedCertificates;
this.ValidationCallback = (_, cert, chain, errors) =>
this.ValidateCertificate(cert != null ? new X509Certificate2(cert) : null, chain, errors);
Log = log;
}

public RemoteCertificateValidationCallback ValidationCallback { get; }

public static ServerCertificateValidationProvider? FromCertificateFile(string certificateFile)
public static ServerCertificateValidationProvider? FromCertificateFile(string certificateFile, IServerCertificateValidationEventSource? log = null)
{
if (!File.Exists(certificateFile))
{
AWSResourcesEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Certificate File does not exist");
log?.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Certificate File does not exist");
return null;
}

var trustedCertificates = new X509Certificate2Collection();
if (!LoadCertificateToTrustedCollection(trustedCertificates, certificateFile))
{
AWSResourcesEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Failed to load certificate in trusted collection");
log?.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Failed to load certificate in trusted collection");
return null;
}

return new ServerCertificateValidationProvider(trustedCertificates);
return new ServerCertificateValidationProvider(trustedCertificates, log);
}

private static bool LoadCertificateToTrustedCollection(X509Certificate2Collection collection, string certFileName)
Expand Down Expand Up @@ -84,24 +87,24 @@ private bool ValidateCertificate(X509Certificate2? cert, X509Chain? chain, SslPo
{
if ((errors | SslPolicyErrors.RemoteCertificateNotAvailable) == errors)
{
AWSResourcesEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "SslPolicyError RemoteCertificateNotAvailable occurred");
Log?.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "SslPolicyError RemoteCertificateNotAvailable occurred");
}

if ((errors | SslPolicyErrors.RemoteCertificateNameMismatch) == errors)
{
AWSResourcesEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "SslPolicyError RemoteCertificateNameMismatch occurred");
Log?.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "SslPolicyError RemoteCertificateNameMismatch occurred");
}
}

if (chain == null)
{
AWSResourcesEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Certificate chain is null.");
Log?.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Certificate chain is null.");
return false;
}

if (cert == null)
{
AWSResourcesEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Certificate is null.");
Log?.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), "Certificate is null.");
return false;
}

Expand All @@ -123,7 +126,7 @@ private bool ValidateCertificate(X509Certificate2? cert, X509Chain? chain, SslPo
}
}

AWSResourcesEventSource.Log.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), chainErrors);
Log?.FailedToValidateCertificate(nameof(ServerCertificateValidationProvider), chainErrors);
}

// check if at least one certificate in the chain is in our trust list
Expand All @@ -142,7 +145,7 @@ private bool ValidateCertificate(X509Certificate2? cert, X509Chain? chain, SslPo
trustCertificates += " " + trustCertificate.Subject;
}

AWSResourcesEventSource.Log.FailedToValidateCertificate(
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}");
}
Expand Down
5 changes: 2 additions & 3 deletions test/OpenTelemetry.ResourceDetectors.AWS.Tests/Http/HandlerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

#if !NETFRAMEWORK

using OpenTelemetry.ResourceDetectors.AWS.Http;
using Xunit;

namespace OpenTelemetry.ResourceDetectors.AWS.Tests.Http;
Expand All @@ -20,15 +19,15 @@ public void TestValidHandler()
certificateUploader.Create();

// Validates if the handler created.
Assert.NotNull(Handler.Create(certificateUploader.FilePath));
Assert.NotNull(ServerCertificateValidationHandler.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));
Assert.Null(ServerCertificateValidationHandler.Create(INVALIDCRTNAME));
}
}

Expand Down
1 change: 0 additions & 1 deletion ...penTelemetry.ResourceDetectors.AWS.Tests/Http/ServerCertificateValidationProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#if !NETFRAMEWORK

using System.Security.Cryptography.X509Certificates;
using OpenTelemetry.ResourceDetectors.AWS.Http;
using Xunit;

namespace OpenTelemetry.ResourceDetectors.AWS.Tests.Http;
Expand Down
Loading