Skip to content

Commit 3ec29e7

Browse files
authored
[OTLP] refactor: trust custom CA only (open-telemetry#6804)
1 parent 20d6fc8 commit 3ec29e7

File tree

2 files changed

+49
-29
lines changed

2 files changed

+49
-29
lines changed

src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/OtlpCertificateManager.cs

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -226,40 +226,42 @@ internal static bool ValidateServerCertificate(
226226
{
227227
try
228228
{
229-
// If there are no SSL policy errors, accept the certificate
230-
if (sslPolicyErrors == SslPolicyErrors.None)
229+
// Fail fast for any non-chain-related SSL policy errors (e.g., name mismatch).
230+
var nonChainErrors = sslPolicyErrors & ~SslPolicyErrors.RemoteCertificateChainErrors;
231+
if (nonChainErrors != SslPolicyErrors.None)
231232
{
232-
return true;
233+
OpenTelemetryProtocolExporterEventSource.Log.MtlsServerCertificateValidationFailed(
234+
serverCert.Subject,
235+
sslPolicyErrors.ToString());
236+
return false;
233237
}
234238

235-
// If the only error is an untrusted root, validate against our CA
236-
if (sslPolicyErrors.HasFlag(SslPolicyErrors.RemoteCertificateChainErrors))
239+
// Always validate against the provided CA to enforce custom trust restrictions.
240+
chain.ChainPolicy.ExtraStore.Clear();
241+
chain.ChainPolicy.CustomTrustStore.Clear();
242+
chain.ChainPolicy.ExtraStore.Add(caCertificate);
243+
chain.ChainPolicy.VerificationFlags =
244+
X509VerificationFlags.AllowUnknownCertificateAuthority;
245+
chain.ChainPolicy.CustomTrustStore.Add(caCertificate);
246+
chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
247+
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
248+
chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot;
249+
250+
bool isValid = chain.Build(serverCert);
251+
252+
if (isValid)
237253
{
238-
// Add our CA certificate to the chain
239-
chain.ChainPolicy.ExtraStore.Add(caCertificate);
240-
chain.ChainPolicy.VerificationFlags =
241-
X509VerificationFlags.AllowUnknownCertificateAuthority;
242-
chain.ChainPolicy.CustomTrustStore.Add(caCertificate);
243-
chain.ChainPolicy.TrustMode = X509ChainTrustMode.CustomRootTrust;
244-
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
245-
chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot;
246-
247-
bool isValid = chain.Build(serverCert);
248-
249-
if (isValid)
254+
// Verify that the chain terminates with our CA
255+
var rootCert = chain.ChainElements[^1].Certificate;
256+
if (
257+
string.Equals(
258+
rootCert.Thumbprint,
259+
caCertificate.Thumbprint,
260+
StringComparison.OrdinalIgnoreCase))
250261
{
251-
// Verify that the chain terminates with our CA
252-
var rootCert = chain.ChainElements[^1].Certificate;
253-
if (
254-
string.Equals(
255-
rootCert.Thumbprint,
256-
caCertificate.Thumbprint,
257-
StringComparison.OrdinalIgnoreCase))
258-
{
259-
OpenTelemetryProtocolExporterEventSource.Log.MtlsServerCertificateValidated(
260-
serverCert.Subject);
261-
return true;
262-
}
262+
OpenTelemetryProtocolExporterEventSource.Log.MtlsServerCertificateValidated(
263+
serverCert.Subject);
264+
return true;
263265
}
264266
}
265267

test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/OtlpSecureHttpClientFactoryTests.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,24 @@ public void ValidateServerCertificate_ReturnsFalse_WhenCaDoesNotMatch()
257257
Assert.False(result);
258258
}
259259

260+
[Fact]
261+
public void ValidateServerCertificate_ReturnsFalse_WhenCaDoesNotMatch_EvenIfSslPolicyErrorsNone()
262+
{
263+
using var caCertificate = CreateCertificateAuthority();
264+
using var otherCaCertificate = CreateCertificateAuthority();
265+
using var serverCertificate = CreateServerCertificate(caCertificate);
266+
using var chain = new X509Chain();
267+
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
268+
269+
var result = OpenTelemetryProtocol.Implementation.OtlpCertificateManager.ValidateServerCertificate(
270+
serverCertificate,
271+
chain,
272+
SslPolicyErrors.None,
273+
otherCaCertificate);
274+
275+
Assert.False(result);
276+
}
277+
260278
[Fact]
261279
public void ValidateCertificateChain_ReturnsFalseForExpiredCertificate()
262280
{

0 commit comments

Comments
 (0)