-
Notifications
You must be signed in to change notification settings - Fork 891
[otlp] Add mTLS Support for OTLP Exporter #5918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
00c903c
b10b3fa
0d0aa98
e7c6f5b
3b43fd3
4b6d3cd
5864dbf
0e38ba3
3733dee
f09a650
8630876
2e7e412
afc8df6
c5101b1
84a4d5b
716949c
2781534
31ef9aa
9df6f06
6e940d1
dc39de9
2006fbf
87737eb
4d56a9a
c4ec895
0ad1e13
7a378e6
6bddb6b
9b44067
a0bd2f9
f9fcd24
6675646
9b13d2e
cb5ccdf
d71b483
8694bb9
9d6e67a
7aa2ea3
a09e608
e77d6e1
797816c
0691873
8471933
a9c0e90
5beccbf
8a9639f
97e2ba5
c0b25b6
8d0717c
e32eb71
d01ed8f
9371340
1700559
b0c100f
3f8f077
1dcbcd9
b251110
ef7db36
090785f
6ecc522
0a2c60d
29e4dd5
cfb0c01
45c4525
5033c4b
350e607
d76fccb
7ec6a5d
fac985e
d1d392f
0b98744
3a4f031
91ccacd
66eafa0
7003fe1
ea6c940
31cfe8d
5935f0d
4ffba98
aeacb87
71e19fe
76fa366
0438284
dbd5a05
f6e0577
6821ad1
3eab016
0b0e0c9
d4469f9
f56dc6e
4a50f64
b817179
fc41e6a
a69ed31
a27780f
7898d99
dce4bae
ca70e55
7c611b7
0ee5513
88a9f7f
1ecabf1
5d9751c
fc90786
b7c34cd
72912c2
0571af8
777f530
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| OpenTelemetry.Exporter.OtlpExporterOptions.CertificateFile.get -> string! | ||
| OpenTelemetry.Exporter.OtlpExporterOptions.CertificateFile.set -> void | ||
| OpenTelemetry.Exporter.OtlpExporterOptions.ClientCertificateFile.get -> string! | ||
| OpenTelemetry.Exporter.OtlpExporterOptions.ClientCertificateFile.set -> void | ||
| OpenTelemetry.Exporter.OtlpExporterOptions.ClientKeyFile.get -> string! | ||
| OpenTelemetry.Exporter.OtlpExporterOptions.ClientKeyFile.set -> void | ||
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -19,7 +19,7 @@ internal sealed class OtlpHttpTraceExportClient : BaseOtlpHttpExportClient<OtlpC | |||
| private const string TracesExportPath = "v1/traces"; | ||||
|
|
||||
| public OtlpHttpTraceExportClient(OtlpExporterOptions options, HttpClient httpClient) | ||||
| : base(options, httpClient, TracesExportPath) | ||||
| : base(options, ModifyHttpClient(options, httpClient), TracesExportPath) | ||||
| { | ||||
| } | ||||
|
|
||||
|
|
@@ -28,6 +28,26 @@ protected override HttpContent CreateHttpContent(OtlpCollector.ExportTraceServic | |||
| return new ExportRequestContent(exportRequest); | ||||
| } | ||||
|
|
||||
| private static HttpClient ModifyHttpClient(OtlpExporterOptions options, HttpClient httpClient) | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can't just throw away the opentelemetry-dotnet/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs Line 134 in 5dff99f
This change will break any user doing that or doing anything else to the HttpClient they are intending to use here.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I modified a little bit, does it look fine? @CodeBlanch
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the design looks ok, I will start to write tests :) |
||||
| { | ||||
| // Create a new handler using the existing method that configures mTLS | ||||
| var handler = options.CreateDefaultHttpMessageHandler(); | ||||
|
|
||||
| // Create a new HttpClient with the mTLS-enabled handler | ||||
| var newHttpClient = new HttpClient(handler, disposeHandler: true); | ||||
|
|
||||
| // Copy existing headers from the original HttpClient | ||||
| foreach (var header in httpClient.DefaultRequestHeaders) | ||||
| { | ||||
| newHttpClient.DefaultRequestHeaders.Add(header.Key, header.Value); | ||||
| } | ||||
|
|
||||
| // Copy other properties, such as timeout, if needed | ||||
| newHttpClient.Timeout = httpClient.Timeout; | ||||
|
|
||||
| return newHttpClient; | ||||
| } | ||||
|
|
||||
| internal sealed class ExportRequestContent : HttpContent | ||||
| { | ||||
| private static readonly MediaTypeHeaderValue ProtobufMediaTypeHeader = new(MediaContentType); | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,9 @@ | |
| using OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation; | ||
| using OpenTelemetry.Internal; | ||
| using OpenTelemetry.Trace; | ||
| #if NET6_0_OR_GREATER | ||
|
sandy2008 marked this conversation as resolved.
Outdated
|
||
| using System.Security.Cryptography.X509Certificates; | ||
| #endif | ||
|
|
||
| namespace OpenTelemetry.Exporter; | ||
|
|
||
|
|
@@ -27,6 +30,9 @@ public class OtlpExporterOptions : IOtlpExporterOptions | |
| internal const string DefaultGrpcEndpoint = "http://localhost:4317"; | ||
| internal const string DefaultHttpEndpoint = "http://localhost:4318"; | ||
| internal const OtlpExportProtocol DefaultOtlpExportProtocol = OtlpExportProtocol.Grpc; | ||
| internal const string CertificateFileEnvVarName = "OTEL_EXPORTER_OTLP_CERTIFICATE"; | ||
| internal const string ClientKeyFileEnvVarName = "OTEL_EXPORTER_OTLP_CLIENT_KEY"; | ||
| internal const string ClientCertificateFileEnvVarName = "OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE"; | ||
|
|
||
| internal static readonly KeyValuePair<string, string>[] StandardHeaders = new KeyValuePair<string, string>[] | ||
| { | ||
|
|
@@ -75,6 +81,36 @@ internal OtlpExporterOptions( | |
| }; | ||
|
|
||
| this.BatchExportProcessorOptions = defaultBatchOptions!; | ||
|
|
||
| // Load CertificateFile from environment variable | ||
| if (Environment.GetEnvironmentVariable(CertificateFileEnvVarName) is string certificateFile) | ||
| { | ||
| this.CertificateFile = certificateFile; | ||
| } | ||
| else | ||
| { | ||
| this.CertificateFile = string.Empty; | ||
| } | ||
|
|
||
| // Load ClientKeyFile from environment variable | ||
| if (Environment.GetEnvironmentVariable(ClientKeyFileEnvVarName) is string clientKeyFile) | ||
| { | ||
| this.ClientKeyFile = clientKeyFile; | ||
| } | ||
| else | ||
| { | ||
| this.ClientKeyFile = string.Empty; | ||
| } | ||
|
|
||
| // Load ClientCertificateFile from environment variable | ||
| if (Environment.GetEnvironmentVariable(ClientCertificateFileEnvVarName) is string clientCertificateFile) | ||
| { | ||
| this.ClientCertificateFile = clientCertificateFile; | ||
| } | ||
| else | ||
| { | ||
| this.ClientCertificateFile = string.Empty; | ||
| } | ||
| } | ||
|
|
||
| /// <inheritdoc/> | ||
|
|
@@ -142,6 +178,21 @@ public Func<HttpClient> HttpClientFactory | |
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the trusted certificate to use when verifying a server's TLS credentials. | ||
| /// </summary> | ||
| public string CertificateFile { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the path to the private key to use in mTLS communication in PEM format. | ||
| /// </summary> | ||
| public string ClientKeyFile { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets or sets the path to the certificate/chain trust for client's private key to use in mTLS communication in PEM format. | ||
| /// </summary> | ||
| public string ClientCertificateFile { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Gets a value indicating whether or not the signal-specific path should | ||
| /// be appended to <see cref="Endpoint"/>. | ||
|
|
@@ -220,6 +271,37 @@ internal OtlpExporterOptions ApplyDefaults(OtlpExporterOptions defaultExporterOp | |
| return this; | ||
| } | ||
|
|
||
| internal HttpMessageHandler CreateDefaultHttpMessageHandler() | ||
| { | ||
| var handler = new HttpClientHandler(); | ||
|
|
||
| #if NET6_0_OR_GREATER | ||
| if (!string.IsNullOrEmpty(this.CertificateFile)) | ||
| { | ||
| var trustedCertificate = new X509Certificate2(this.CertificateFile); | ||
|
|
||
| handler.ServerCertificateCustomValidationCallback = (message, cert, chain, errors) => | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using a custom server certificate validation callback can introduce security risks if not handled properly. Can you list what measures have been taken to mitigate potential risks, such as unintended trust, proper error handling, revocation checks, and restricting trust scope? |
||
| { | ||
| chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust; | ||
| chain.ChainPolicy.CustomTrustStore.Add(trustedCertificate); | ||
| return chain.Build(cert); | ||
| }; | ||
| } | ||
|
|
||
| if (!string.IsNullOrEmpty(this.ClientCertificateFile) && !string.IsNullOrEmpty(this.ClientKeyFile)) | ||
| { | ||
| var clientCertificate = X509Certificate2.CreateFromPemFile(this.ClientCertificateFile, this.ClientKeyFile); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Loading a client certificate from a PEM file with a private key should be done cautiously. How do you ensure or inform customers that the private key file should be stored securely and access-restricted? |
||
| handler.ClientCertificates.Add(clientCertificate); | ||
| } | ||
| #else | ||
| // Implement alternative methods for earlier .NET versions | ||
| throw new PlatformNotSupportedException("mTLS support requires .NET 6.0 or later."); | ||
| #endif | ||
|
|
||
| #pragma warning disable CS0162 // Unreachable code detected | ||
| return handler; | ||
| } | ||
|
|
||
| private static string GetUserAgentString() | ||
| { | ||
| var assembly = typeof(OtlpExporterOptions).Assembly; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.