From 3c7f2d8180192b06b15597a7ef66bc16e38919b9 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Thu, 23 Feb 2023 18:03:28 -0800 Subject: [PATCH 01/14] add TlsConfigHelper for additional TLS configurability and wire up internal builders. --- .../exporter/internal/TlsConfigHelper.java | 107 ++++++++++++++++++ .../internal/grpc/GrpcExporterBuilder.java | 33 ++---- .../okhttp/OkHttpExporterBuilder.java | 49 ++++---- .../jaeger/JaegerGrpcSpanExporterBuilder.java | 4 +- .../OtlpHttpMetricExporterBuilder.java | 4 +- .../trace/OtlpHttpSpanExporterBuilder.java | 4 +- .../OtlpGrpcMetricExporterBuilder.java | 4 +- .../trace/OtlpGrpcSpanExporterBuilder.java | 4 +- .../OtlpHttpLogRecordExporterBuilder.java | 4 +- .../OtlpGrpcLogRecordExporterBuilder.java | 4 +- 10 files changed, 152 insertions(+), 65 deletions(-) create mode 100644 exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java new file mode 100644 index 00000000000..d2cf04b44d8 --- /dev/null +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java @@ -0,0 +1,107 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.exporter.internal; + +import java.util.logging.Logger; +import javax.annotation.Nullable; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.X509KeyManager; +import javax.net.ssl.X509TrustManager; +import okhttp3.OkHttpClient; + +/** + * Utility class to help with management of TLS related components. This class is ultimately + * responsible for enabling TLS with the OkHttpClientBuilder by setting the sslSocketFactory. This + * class is only intended for internal OpenTelemetry exporter usage and should not be used by + * end-users. + */ +public class TlsConfigHelper { + + private static final Logger logger = Logger.getLogger(TlsConfigHelper.class.getName()); + + @Nullable private X509KeyManager keyManager; + @Nullable private X509TrustManager trustManager; + @Nullable private SSLSocketFactory sslSocketFactory; + + public TlsConfigHelper setTrustManager(X509TrustManager trustManager) { + this.trustManager = trustManager; + return this; + } + + public TlsConfigHelper configureTrustManager(byte[] trustedCertsPem) { + try { + this.trustManager = TlsUtil.trustManager(trustedCertsPem); + } catch (SSLException e) { + throw new IllegalStateException( + "Error creating trust manager for OTLP HTTP connection with provided certs. Are they valid X.509 in PEM format?", + e); + } + return this; + } + + public TlsConfigHelper configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) { + try { + if (keyManager != null) { + logger.warning( + "Previous X509 Key manager is being replaced. This is probably an error and should only be set once."); + } + keyManager = TlsUtil.keyManager(privateKeyPem, certificatePem); + return this; + } catch (SSLException e) { + throw new IllegalStateException( + "Error creating key manager for TLS configuration of OTLP HTTP connection with provided certs. Are they valid X.509 in PEM format?", + e); + } + } + + public TlsConfigHelper setKeyManager(X509KeyManager keyManager) { + if (this.keyManager != null) { + logger.warning( + "Previous X509 Key manager is being replaced. This is probably an error and should only be set once."); + } + this.keyManager = keyManager; + return this; + } + + public TlsConfigHelper setSslSocketFactory(SSLSocketFactory sslSocketFactory) { + this.sslSocketFactory = sslSocketFactory; + return this; + } + + public void configure(OkHttpClient.Builder clientBuilder) { + if (trustManager == null) { + if (warnIfOtherComponentsConfigured()) { + logger.warning("An X509TrustManager must be configured for TLS to be configured."); + } + return; + } + + try { + SSLSocketFactory sslSocketFactory = this.sslSocketFactory; + if (sslSocketFactory == null) { + sslSocketFactory = TlsUtil.sslSocketFactory(keyManager, trustManager); + } + clientBuilder.sslSocketFactory(sslSocketFactory, trustManager); + } catch (SSLException e) { + throw new IllegalStateException( + "Could not set trusted certificate for OTLP HTTP connection, are they valid X.509 in PEM format?", + e); + } + } + + private boolean warnIfOtherComponentsConfigured() { + if (sslSocketFactory != null) { + logger.warning("sslSocketFactory has been configured without an X509TrustManager."); + return true; + } + if (keyManager != null) { + logger.warning("An X509KeyManager has been configured without an X509TrustManager."); + return true; + } + return false; + } +} diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java index b77d825e324..f039b956180 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java @@ -14,7 +14,7 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.metrics.MeterProvider; import io.opentelemetry.exporter.internal.ExporterBuilderUtil; -import io.opentelemetry.exporter.internal.TlsUtil; +import io.opentelemetry.exporter.internal.TlsConfigHelper; import io.opentelemetry.exporter.internal.marshal.Marshaler; import io.opentelemetry.exporter.internal.okhttp.OkHttpUtil; import io.opentelemetry.exporter.internal.retry.RetryInterceptor; @@ -29,9 +29,6 @@ import java.util.function.BiFunction; import java.util.function.Supplier; import javax.annotation.Nullable; -import javax.net.ssl.SSLException; -import javax.net.ssl.X509KeyManager; -import javax.net.ssl.X509TrustManager; import okhttp3.Headers; import okhttp3.OkHttpClient; import okhttp3.Protocol; @@ -55,9 +52,7 @@ public class GrpcExporterBuilder { private URI endpoint; private boolean compressionEnabled = false; private final Map headers = new HashMap<>(); - @Nullable private byte[] trustedCertificatesPem; - @Nullable private byte[] privateKeyPem; - @Nullable private byte[] certificatePem; + private final TlsConfigHelper tlsConfigHelper = new TlsConfigHelper(); @Nullable private RetryPolicy retryPolicy; private Supplier meterProviderSupplier = GlobalOpenTelemetry::getMeterProvider; @@ -103,14 +98,13 @@ public GrpcExporterBuilder setCompression(String compressionMethod) { return this; } - public GrpcExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { - this.trustedCertificatesPem = trustedCertificatesPem; + public GrpcExporterBuilder configureTrustManager(byte[] trustedCertificatesPem) { + tlsConfigHelper.configureTrustManager(trustedCertificatesPem); return this; } - public GrpcExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - this.privateKeyPem = privateKeyPem; - this.certificatePem = certificatePem; + public GrpcExporterBuilder configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) { + tlsConfigHelper.configureKeyManager(privateKeyPem, certificatePem); return this; } @@ -139,20 +133,7 @@ public GrpcExporter build() { clientBuilder.callTimeout(Duration.ofNanos(timeoutNanos)); - if (trustedCertificatesPem != null) { - try { - X509TrustManager trustManager = TlsUtil.trustManager(trustedCertificatesPem); - X509KeyManager keyManager = null; - if (privateKeyPem != null && certificatePem != null) { - keyManager = TlsUtil.keyManager(privateKeyPem, certificatePem); - } - clientBuilder.sslSocketFactory( - TlsUtil.sslSocketFactory(keyManager, trustManager), trustManager); - } catch (SSLException e) { - throw new IllegalStateException( - "Could not set trusted certificates, are they valid X.509 in PEM format?", e); - } - } + tlsConfigHelper.configure(clientBuilder); String endpoint = this.endpoint.resolve(grpcEndpointPath).toString(); if (endpoint.startsWith("http://")) { diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java index 37c7090fc0e..fdeb800ebff 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java @@ -8,7 +8,7 @@ import io.opentelemetry.api.GlobalOpenTelemetry; import io.opentelemetry.api.metrics.MeterProvider; import io.opentelemetry.exporter.internal.ExporterBuilderUtil; -import io.opentelemetry.exporter.internal.TlsUtil; +import io.opentelemetry.exporter.internal.TlsConfigHelper; import io.opentelemetry.exporter.internal.auth.Authenticator; import io.opentelemetry.exporter.internal.marshal.Marshaler; import io.opentelemetry.exporter.internal.retry.RetryInterceptor; @@ -18,7 +18,7 @@ import java.util.concurrent.TimeUnit; import java.util.function.Supplier; import javax.annotation.Nullable; -import javax.net.ssl.SSLException; +import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509KeyManager; import javax.net.ssl.X509TrustManager; import okhttp3.Headers; @@ -44,9 +44,8 @@ public final class OkHttpExporterBuilder { private boolean compressionEnabled = false; private boolean exportAsJson = false; @Nullable private Headers.Builder headersBuilder; - @Nullable private byte[] trustedCertificatesPem; - @Nullable private byte[] privateKeyPem; - @Nullable private byte[] certificatePem; + + private final TlsConfigHelper tlsConfigHelper = new TlsConfigHelper(); @Nullable private RetryPolicy retryPolicy; private Supplier meterProviderSupplier = GlobalOpenTelemetry::getMeterProvider; @Nullable private Authenticator authenticator; @@ -91,14 +90,28 @@ public OkHttpExporterBuilder setAuthenticator(Authenticator authenticator) { return this; } - public OkHttpExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { - this.trustedCertificatesPem = trustedCertificatesPem; + public OkHttpExporterBuilder configureTrustManager(byte[] trustedCertificatesPem) { + tlsConfigHelper.configureTrustManager(trustedCertificatesPem); + return this; + } + + public OkHttpExporterBuilder setTrustManager(X509TrustManager trustManager) { + tlsConfigHelper.setTrustManager(trustManager); return this; } - public OkHttpExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - this.privateKeyPem = privateKeyPem; - this.certificatePem = certificatePem; + public OkHttpExporterBuilder configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) { + tlsConfigHelper.configureKeyManager(privateKeyPem, certificatePem); + return this; + } + + public OkHttpExporterBuilder setKeyManager(X509KeyManager keyManager) { + tlsConfigHelper.setKeyManager(keyManager); + return this; + } + + public OkHttpExporterBuilder setSslSocketFactory(SSLSocketFactory sslSocketFactory) { + tlsConfigHelper.setSslSocketFactory(sslSocketFactory); return this; } @@ -123,21 +136,7 @@ public OkHttpExporter build() { .dispatcher(OkHttpUtil.newDispatcher()) .callTimeout(Duration.ofNanos(timeoutNanos)); - if (trustedCertificatesPem != null) { - try { - X509TrustManager trustManager = TlsUtil.trustManager(trustedCertificatesPem); - X509KeyManager keyManager = null; - if (privateKeyPem != null && certificatePem != null) { - keyManager = TlsUtil.keyManager(privateKeyPem, certificatePem); - } - clientBuilder.sslSocketFactory( - TlsUtil.sslSocketFactory(keyManager, trustManager), trustManager); - } catch (SSLException e) { - throw new IllegalStateException( - "Could not set trusted certificate for OTLP HTTP connection, are they valid X.509 in PEM format?", - e); - } - } + tlsConfigHelper.configure(clientBuilder); Headers headers = headersBuilder == null ? null : headersBuilder.build(); diff --git a/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java b/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java index c30ad9f1651..8c8d9671ea0 100644 --- a/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java +++ b/exporters/jaeger/src/main/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterBuilder.java @@ -109,13 +109,13 @@ public JaegerGrpcSpanExporterBuilder setTimeout(Duration timeout) { * use the system default trusted certificates. */ public JaegerGrpcSpanExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { - delegate.setTrustedCertificates(trustedCertificatesPem); + delegate.configureTrustManager(trustedCertificatesPem); return this; } /** Sets the client key and chain to use for verifying servers when mTLS is enabled. */ public JaegerGrpcSpanExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - delegate.setClientTls(privateKeyPem, certificatePem); + delegate.configureKeyManager(privateKeyPem, certificatePem); return this; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java index 83c19c14534..1ab0f5c8e05 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/metrics/OtlpHttpMetricExporterBuilder.java @@ -99,7 +99,7 @@ public OtlpHttpMetricExporterBuilder addHeader(String key, String value) { * use the system default trusted certificates. */ public OtlpHttpMetricExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { - delegate.setTrustedCertificates(trustedCertificatesPem); + delegate.configureTrustManager(trustedCertificatesPem); return this; } @@ -108,7 +108,7 @@ public OtlpHttpMetricExporterBuilder setTrustedCertificates(byte[] trustedCertif * The key must be PKCS8, and both must be in PEM format. */ public OtlpHttpMetricExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - delegate.setClientTls(privateKeyPem, certificatePem); + delegate.configureKeyManager(privateKeyPem, certificatePem); return this; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java index 6ce05a36482..3d8ddbc68df 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/http/trace/OtlpHttpSpanExporterBuilder.java @@ -87,7 +87,7 @@ public OtlpHttpSpanExporterBuilder addHeader(String key, String value) { * use the system default trusted certificates. */ public OtlpHttpSpanExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { - delegate.setTrustedCertificates(trustedCertificatesPem); + delegate.configureTrustManager(trustedCertificatesPem); return this; } @@ -96,7 +96,7 @@ public OtlpHttpSpanExporterBuilder setTrustedCertificates(byte[] trustedCertific * The key must be PKCS8, and both must be in PEM format. */ public OtlpHttpSpanExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - delegate.setClientTls(privateKeyPem, certificatePem); + delegate.configureKeyManager(privateKeyPem, certificatePem); return this; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java index 3bc796b36e9..5197c17887e 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/metrics/OtlpGrpcMetricExporterBuilder.java @@ -131,7 +131,7 @@ public OtlpGrpcMetricExporterBuilder setCompression(String compressionMethod) { * use the system default trusted certificates. */ public OtlpGrpcMetricExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { - delegate.setTrustedCertificates(trustedCertificatesPem); + delegate.configureTrustManager(trustedCertificatesPem); return this; } @@ -140,7 +140,7 @@ public OtlpGrpcMetricExporterBuilder setTrustedCertificates(byte[] trustedCertif * The key must be PKCS8, and both must be in PEM format. */ public OtlpGrpcMetricExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - delegate.setClientTls(privateKeyPem, certificatePem); + delegate.configureKeyManager(privateKeyPem, certificatePem); return this; } diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java index 505f8e56dcc..8ead941f91d 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/trace/OtlpGrpcSpanExporterBuilder.java @@ -115,7 +115,7 @@ public OtlpGrpcSpanExporterBuilder setCompression(String compressionMethod) { * use the system default trusted certificates. */ public OtlpGrpcSpanExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { - delegate.setTrustedCertificates(trustedCertificatesPem); + delegate.configureTrustManager(trustedCertificatesPem); return this; } @@ -124,7 +124,7 @@ public OtlpGrpcSpanExporterBuilder setTrustedCertificates(byte[] trustedCertific * The key must be PKCS8, and both must be in PEM format. */ public OtlpGrpcSpanExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - delegate.setClientTls(privateKeyPem, certificatePem); + delegate.configureKeyManager(privateKeyPem, certificatePem); return this; } diff --git a/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java b/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java index 4a201104e8d..657b2dd819b 100644 --- a/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java +++ b/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogRecordExporterBuilder.java @@ -83,7 +83,7 @@ public OtlpHttpLogRecordExporterBuilder addHeader(String key, String value) { * use the system default trusted certificates. */ public OtlpHttpLogRecordExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { - delegate.setTrustedCertificates(trustedCertificatesPem); + delegate.configureTrustManager(trustedCertificatesPem); return this; } @@ -93,7 +93,7 @@ public OtlpHttpLogRecordExporterBuilder setTrustedCertificates(byte[] trustedCer */ public OtlpHttpLogRecordExporterBuilder setClientTls( byte[] privateKeyPem, byte[] certificatePem) { - delegate.setClientTls(privateKeyPem, certificatePem); + delegate.configureKeyManager(privateKeyPem, certificatePem); return this; } diff --git a/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java b/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java index 2802bbdc5f0..f6ec0e0b908 100644 --- a/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java +++ b/exporters/otlp/logs/src/main/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogRecordExporterBuilder.java @@ -115,7 +115,7 @@ public OtlpGrpcLogRecordExporterBuilder setCompression(String compressionMethod) * use the system default trusted certificates. */ public OtlpGrpcLogRecordExporterBuilder setTrustedCertificates(byte[] trustedCertificatesPem) { - delegate.setTrustedCertificates(trustedCertificatesPem); + delegate.configureTrustManager(trustedCertificatesPem); return this; } @@ -125,7 +125,7 @@ public OtlpGrpcLogRecordExporterBuilder setTrustedCertificates(byte[] trustedCer */ public OtlpGrpcLogRecordExporterBuilder setClientTls( byte[] privateKeyPem, byte[] certificatePem) { - delegate.setClientTls(privateKeyPem, certificatePem); + delegate.configureKeyManager(privateKeyPem, certificatePem); return this; } From b77ec4215e5a0ff2a30c66069af22e80c440af0f Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 24 Feb 2023 09:20:12 -0800 Subject: [PATCH 02/14] add javadoc and spotless --- .../exporter/internal/TlsConfigHelper.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java index d2cf04b44d8..6b1c81c85db 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java @@ -32,6 +32,12 @@ public TlsConfigHelper setTrustManager(X509TrustManager trustManager) { return this; } + /** + * Creates a new X509TrustManager from the given cert content. + * + * @param trustedCertsPem Certificate in PEM format. + * @return this + */ public TlsConfigHelper configureTrustManager(byte[] trustedCertsPem) { try { this.trustManager = TlsUtil.trustManager(trustedCertsPem); @@ -43,6 +49,13 @@ public TlsConfigHelper configureTrustManager(byte[] trustedCertsPem) { return this; } + /** + * Creates a new X509KeyManager from the given private key and certificate, both in PEM format. + * + * @param privateKeyPem Private key content in PEM format. + * @param certificatePem Certificate content in PEM format. + * @return this + */ public TlsConfigHelper configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) { try { if (keyManager != null) { @@ -58,6 +71,11 @@ public TlsConfigHelper configureKeyManager(byte[] privateKeyPem, byte[] certific } } + /** + * Assigns the X509KeyManager. + * + * @return this + */ public TlsConfigHelper setKeyManager(X509KeyManager keyManager) { if (this.keyManager != null) { logger.warning( @@ -72,6 +90,7 @@ public TlsConfigHelper setSslSocketFactory(SSLSocketFactory sslSocketFactory) { return this; } + /** Configures TLS for the provided OkHttp client builder by setting the SSL Socket Factory. */ public void configure(OkHttpClient.Builder clientBuilder) { if (trustManager == null) { if (warnIfOtherComponentsConfigured()) { From 67e23236f44a259c4953364cbcd21c9df7414a85 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 24 Feb 2023 10:30:39 -0800 Subject: [PATCH 03/14] add keys for "validity" testing. --- .../exporter/internal/TlsUtil.java | 2 +- .../jaeger/JaegerGrpcSpanExporterTest.java | 30 ++++++----- .../jaeger/src/test/resources/tls-test.key | 52 +++++++++++++++++++ .../jaeger/src/test/resources/tls-test.pem | 28 ++++++++++ 4 files changed, 98 insertions(+), 14 deletions(-) create mode 100644 exporters/jaeger/src/test/resources/tls-test.key create mode 100644 exporters/jaeger/src/test/resources/tls-test.pem diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsUtil.java index 83d9d89f2f1..2be05aed694 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsUtil.java @@ -89,7 +89,7 @@ public static SSLSocketFactory sslSocketFactory( } /** - * Creates {@link KeyManager} initiaded by keystore containing single private key with matching + * Creates {@link KeyManager} initiated by keystore containing single private key with matching * certificate chain. */ public static X509KeyManager keyManager(byte[] privateKeyPem, byte[] certificatePem) diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java index 0d364bdbc82..d1fb768bd25 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java @@ -38,8 +38,10 @@ import io.opentelemetry.sdk.trace.data.StatusData; import io.opentelemetry.semconv.resource.attributes.ResourceAttributes; import java.net.InetAddress; +import java.net.URI; import java.net.URISyntaxException; -import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -285,22 +287,19 @@ private static SpanData testSpanData(Resource resource, String spanName) { } @Test - void validTrustedConfig() { - assertThatCode( - () -> - JaegerGrpcSpanExporter.builder() - .setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8))) + void validTrustedConfig() throws Exception { + byte[] cert = readResource("tls-test.pem"); + assertThatCode(() -> JaegerGrpcSpanExporter.builder().setTrustedCertificates(cert)) .doesNotThrowAnyException(); } @Test - void validClientKeyConfig() { - assertThatCode( - () -> - JaegerGrpcSpanExporter.builder() - .setClientTls( - "foobar".getBytes(StandardCharsets.UTF_8), - "foobar".getBytes(StandardCharsets.UTF_8))) + void validClientKeyConfig() throws Exception { + + byte[] key = readResource("tls-test.key"); + byte[] cert = readResource("tls-test.pem"); + + assertThatCode(() -> JaegerGrpcSpanExporter.builder().setClientTls(key, cert)) .doesNotThrowAnyException(); } @@ -399,4 +398,9 @@ void shutdown() { assertThat(exporter.shutdown().join(1, TimeUnit.SECONDS).isSuccess()).isTrue(); logs.assertContains("Calling shutdown() multiple times."); } + + byte[] readResource(String name) throws Exception { + URI uri = Thread.currentThread().getContextClassLoader().getResource(name).toURI(); + return Files.readAllBytes(Paths.get(uri)); + } } diff --git a/exporters/jaeger/src/test/resources/tls-test.key b/exporters/jaeger/src/test/resources/tls-test.key new file mode 100644 index 00000000000..e19fd059ca5 --- /dev/null +++ b/exporters/jaeger/src/test/resources/tls-test.key @@ -0,0 +1,52 @@ +-----BEGIN PRIVATE KEY----- +MIIJQwIBADANBgkqhkiG9w0BAQEFAASCCS0wggkpAgEAAoICAQCzY+EHeeEVxBRc +EE2ZeMA907V6tRP3G2c/EJ39VfsvR93QaBz12g0r7P8u5QMi+23yqcmvIVmO51zD +IrqjU0VQJf0FC+3IEsykN3AvatPEflxrpQosSnUXkmlTrNyBdw/v7AXjrA+EkQPn +ybXapiQYsCSoGNvpExY2Vm24jnBGIRjShXNA2FNBV4m47cVYA9Yeis0ATqlJA+Pz +4VAkgIAIwnRUXqB0BpVRYTG1EwJaWa3RCsPjk/bQJmF9vRsw0/CBQEYP03JqRROp +/xSZDk+lMFQpuoTwUNk54F8PBmgm//bYqnd4kHx/d3VjqHiAivbJTma0FCF8LBSe ++K3xiOLt8FKXOoVhmEwQ/LRSQvF7afitYpzBeL87Vkdek9l6ZEk1gkQMp5BQW8AY +Mcq+mXoVbCzYfeEocXo6219ca5kj6Fwjx6g1Nduz5x4QJ9gmrcgbLWV+Q0RVYzcb +4YuvGkCD6iDU0xWoQQpjBAHDOq1yxu69KjK5sy1V/zkV/vilYxkHBdYdIO4Kkp+Z +aGg9iQfqzsp0HJbSpEroRvvlXoTb4PKSYNhKUwWZ9CdPsqRj7nFs6dHV8DArSVYt +liF2dWW0oO38ftDWpasMk616Eu6Bjb+Q67KuYKphcpdiGlA5DeNMGzmcm5qJ7K8b +hdzNHFHw7e1CAxQ5yXjUJaffre4PgQIDAQABAoICAEFjyfYgrjRx6f35D2A/DMw8 +rK1S7jkn6wWo9/4NZmdAqhE6sjvHxP9a/1XHstIAln5a96J35J5bpYuX0DKSuvRR +NJ6vT3ibqa6+ao1OQm0gpm2ylje42F/ERdJzc1lZ3hxVi3wHDw0Ck0rdUwWlhAjf +xCzmCts10uXxsUMR4MkfbV3nuTBM6mQqFZCsU0AW/ejaLYnMIpbz7RbL7Zuwn3sR +7reGWrSQZcNpS2Y2zKfEgsBtTSRaniAXrXfZA91TlBp5JFDGgbFNBrLzwT1dYxjS +ugsP6z9yKQyYxe3DJxn4ObGWyb/P4yHPmKHAGlS40uZkRrmjkOUjaEzeVwFxJU57 +iBG1RVNbbAQN54pZoOmSqBIYkgzshg1Jt2GV3UwKuFg2BBnSiXwyKo+Zzf5VRGqO +Ix6eadJuOAHSxRE7RlslGEE5gisEysqH2aRq8oHuX7Y/zjfpkRo+F40OBL/gwh4E +2TduiiIdq6Tg1IysB/Pza31YAbxVr2YKmRCNN7Bamz7MHmEVgycfMmldVY1vApMy +KdB7tyHye2n0WCw4SfwTvawjLncZihLLGi2Rf+xGoUve+pfbl8++hzx7SdCOBvqe +FD/CbwQOpg7RBPwRPUHxufroZKJJf+e8av/A1HpLOlmv7+qD3v/YeiwWQR4fe0lM +Lvx/G2FLx1wp309xQW8xAoIBAQDohuSbogAZ4GtGBSL3pj4fyn3kQVi58sRdgCmE +QIKkOaoURo3lR3Fs52mu17jcRDO/EM7qo6DHduXYkcWI7iqoVki+304BPbWssZNC +k1kYYNg328esnr06jvs/9k2qiy/8SSbn8qZbV9e78JHxxXIpT57wlXzDwLEHpQdf +WDd3qQgEHQO/e/o8IsWxweH90feqvciJ+bxNByOnLo2IjXtIfSoNyuZdET16fJPl +uqdXHV/6+ZjRhbBjd7a/Kvog3+hptD5KWbqrBSAo9ddxGGlpLOfCK6r3NFoBNJyy +rs0W58ylIUtqVSi2X8xl1WVnAkjojd5TNF1scUP04Jy6P+vlAoIBAQDFf8rIOGhK +ZmFrgOuImUBb9mOGVBAw4kj1cu597t69Bh13cEdGL/raPeCmGla6xRWcoaZkB9fi +p4EW1TgmeU9knVC7suAqV6M4GLSdGIOh1og+plbc2UUjWru4l+Zomn8uHay+CWu2 +HrUyxPkrG8MRg0phnfaL6pWfGqFC6wIZvGNGS1diyxOlOYimJdGkLfFKKsEjiIbE +2xDZ8m/IGOpKk2SQiRs4DOXYP6kapK8Hd6/0vo2/J933QD0uRobgatQqoqrVWbWc +TRz15t8Z+PPFq7gg+FSFi9WF8/7gtfPkgC/pYmo/0XGikKERSrsrqCSBoo0rJQIg +LP7i/rF2WDNtAoIBAQCurvf6+178EZ5FQgKc7WTqWSLuYTEYsIpYe7m0wwnh+fgN +t1tODbfmSosYcLNNtmbxeN0ZZMeaUXYmloxBbDrbr8aCrhtZz1Q2Ykjw0OUz7T1B +6Xg99BrQbS3PRWffYSUxcHsr8RyMrpdBd18MtHopEsmij4rlPFNCHPZG6GzzFicR +gAKazbaVisHd4nu2RRC8UkrfbJ/LdjuQkuZ82Vzufikjp3QcIqF+7Svmf6zLQyQh +1XS6oqZ6cyr0voKeQ9fD4UaScIpNhFI+s0GgkuhNvRB2zHtprwiWs0Gs8qjcNlOF +Elgnj8ZADAPv1LqTAiJnonyNIV+14fldc4gKW48JAoIBAFL5o6qG+Fn5tYhC+7HC +ISYB2EKBYuQGzfxBMy25g98KiKq7g1Zbphq20IE4o6OIVdIeoBd6rBrPoQ6Eujh/ +0vlElrylJETl2O8igg9WU0dVKJyZqfjTV5bI8gFV4DmmXzRyxzZ4Isa7FAmmc8DG +wyfCbyegBAs4nW/g8kt5DMLfQR4xXR9mjnRmPaEAVpmRq8QrPkW9gFGIcAYBMzFw +Nk7qqd+HmuXrlU1LQbKt6dNLV6ONw2PCTiEo3gtpGhWDDGx1Nyy3qfNm+nnda3Iw +A8rSoGNJsckBvM2wey7RgwxHRtnYvSbeyc0w0lq/eO+Yju3f60trGv+Fp/iaZNiB +qekCggEBANoazggEv+EltSrSd4MJngnzdkWhqYxASttjfUymUAsqDz4g1VFvF8Ne +NgoAE0lgkcq1YxXghQoetUEv7hxLL2urAkuLkOMj10gW8rpfxEGvR5EhOxKhc5dD +L7e2cFQYVIustH4fJL6d/p6nY1sE2pU65Mls4m9npWvQCAJF2w11ROGEM1XF+0+5 +OUOvfTS88TU7SlDz9AMCLxSfP8W4n/7SzekQ3w0vdGYpxPmN0u9duYPJezOnJHgL +U3+VJJU7KAeCWlV595qAvkUG1jxrH4V76r9NjZbPIezdsJgvnCLyAqBehKIhbPhe +EOvS8wKB2dVM2ck1O39XTAz9yMNRIKs= +-----END PRIVATE KEY----- diff --git a/exporters/jaeger/src/test/resources/tls-test.pem b/exporters/jaeger/src/test/resources/tls-test.pem new file mode 100644 index 00000000000..f6da368f063 --- /dev/null +++ b/exporters/jaeger/src/test/resources/tls-test.pem @@ -0,0 +1,28 @@ +-----BEGIN CERTIFICATE----- +MIIEtDCCApwCCQDAUcLm8z1qxDANBgkqhkiG9w0BAQsFADAcMQswCQYDVQQGEwJV +UzENMAsGA1UECgwEb3RlbDAeFw0yMzAyMjQxNzQwMjdaFw0yNDAyMjQxNzQwMjda +MBwxCzAJBgNVBAYTAlVTMQ0wCwYDVQQKDARvdGVsMIICIjANBgkqhkiG9w0BAQEF +AAOCAg8AMIICCgKCAgEAs2PhB3nhFcQUXBBNmXjAPdO1erUT9xtnPxCd/VX7L0fd +0Ggc9doNK+z/LuUDIvtt8qnJryFZjudcwyK6o1NFUCX9BQvtyBLMpDdwL2rTxH5c +a6UKLEp1F5JpU6zcgXcP7+wF46wPhJED58m12qYkGLAkqBjb6RMWNlZtuI5wRiEY +0oVzQNhTQVeJuO3FWAPWHorNAE6pSQPj8+FQJICACMJ0VF6gdAaVUWExtRMCWlmt +0QrD45P20CZhfb0bMNPwgUBGD9NyakUTqf8UmQ5PpTBUKbqE8FDZOeBfDwZoJv/2 +2Kp3eJB8f3d1Y6h4gIr2yU5mtBQhfCwUnvit8Yji7fBSlzqFYZhMEPy0UkLxe2n4 +rWKcwXi/O1ZHXpPZemRJNYJEDKeQUFvAGDHKvpl6FWws2H3hKHF6OttfXGuZI+hc +I8eoNTXbs+ceECfYJq3IGy1lfkNEVWM3G+GLrxpAg+og1NMVqEEKYwQBwzqtcsbu +vSoyubMtVf85Ff74pWMZBwXWHSDuCpKfmWhoPYkH6s7KdByW0qRK6Eb75V6E2+Dy +kmDYSlMFmfQnT7KkY+5xbOnR1fAwK0lWLZYhdnVltKDt/H7Q1qWrDJOtehLugY2/ +kOuyrmCqYXKXYhpQOQ3jTBs5nJuaieyvG4XczRxR8O3tQgMUOcl41CWn363uD4EC +AwEAATANBgkqhkiG9w0BAQsFAAOCAgEARUvETIQYHxJTjy2sTopJQhgwjlYER3uY +rKi0fOu8bfH8IZW6KNJkHtNCwW062IDRUVqp1oBQLUJTN0ztf/5dUFW8f0gAEoam +hgOSV+3dS61CNk5TzimPwA0RlOxvlFbll1f92Hr6Zb3Y2vPE81dbg4JSqw1T5e40 +doSgOpvHzTjsoE3CsMGIAsr44mSIv9F722tseVBKKy0LwcNmeA/U0xNUhGD5tGiX +ZkvdgeLR3L6EIc8GyiA3ToPsKUDQSdTI+YXxOC3Vzwurb8+7TzE1PN4Kn2v69CVz +iDZdpiXQ2LM1/EFkX/g2saGT98wvtZx+MvIyyiwfLDSxeDIrEtH1ZeWCAyHu1mK3 +ysTWMiCaKuz6/NuG/+TvSRavICW9Rn2PsNVE3RzAFiZD6WZbF2eDZBMZDpvaK+M3 +bIHbk+ZIdDEkfM3vNLxn4SPX1Cb1cximt4Lg0W/G35CquZTj6AwAY/BZgGjz6usf +X4bUwjy7GY6603sohFmh8S7NLdUWVYPx3qIizAAJHlmj+f7z1S8COdnv9oYgn5QM +cfeSRAX3Tx3uWo4fvC9oC/tXhvpRuKrjeTscG3FeIuFW5MfxT5E0PF+zLUh+iFvB +jP4mKlwzd6rdNX9XmUYI/PJUkVEACwTwG0ZgMnMydZrXrtJa8LUr3L1tztnm1WUs +G2KD0lgGVqg= +-----END CERTIFICATE----- From 9b7ac2752f324c8fc15872a647ca15fa5f435bad Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 24 Feb 2023 11:01:37 -0800 Subject: [PATCH 04/14] fix tests --- .../internal/AbstractHttpTelemetryExporterTest.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java index 47f26524435..9d23420220c 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java @@ -368,10 +368,9 @@ void tls_badCert() { () -> exporterBuilder() .setEndpoint(server.httpsUri() + path) - .setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8)) - .build()) + .setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8))) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Could not set trusted certificate"); + .hasMessageContaining("Error creating trust manager for OTLP HTTP connection"); } @ParameterizedTest @@ -555,8 +554,7 @@ void validConfig() { .doesNotThrowAnyException(); assertThatCode( - () -> - exporterBuilder().setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8))) + () -> exporterBuilder().setTrustedCertificates(certificate.certificate().getEncoded())) .doesNotThrowAnyException(); } From 9465c6783a91cb99280b18bf651b6c393ab07973 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 24 Feb 2023 12:27:11 -0800 Subject: [PATCH 05/14] fix tests --- .../internal/AbstractGrpcTelemetryExporterTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java index ea7001f602f..326159e221e 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java @@ -337,10 +337,9 @@ void tls_badCert() { () -> exporterBuilder() .setEndpoint(server.httpsUri().toString()) - .setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8)) - .build()) + .setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8))) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Could not set trusted certificates"); + .hasMessageContaining("Error creating trust manager for OTLP HTTP connection"); } @ParameterizedTest @@ -653,7 +652,7 @@ void validConfig() { assertThatCode( () -> - exporterBuilder().setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8))) + exporterBuilder().setTrustedCertificates(certificate.certificate().getEncoded())) .doesNotThrowAnyException(); } From fb22e0560ff7833121c942951335351a5e825dc2 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 24 Feb 2023 15:46:43 -0800 Subject: [PATCH 06/14] address code review comments --- .../exporter/internal/TlsConfigHelper.java | 79 ++++++++++++++----- .../internal/grpc/GrpcExporterBuilder.java | 6 +- .../internal/grpc/ManagedChannelUtil.java | 73 ++++++++--------- .../okhttp/OkHttpExporterBuilder.java | 6 +- .../jaeger/JaegerGrpcSpanExporterTest.java | 31 ++++---- .../jaeger/src/test/resources/tls-test.key | 52 ------------ .../jaeger/src/test/resources/tls-test.pem | 28 ------- .../AbstractGrpcTelemetryExporterTest.java | 9 ++- .../AbstractHttpTelemetryExporterTest.java | 2 +- ...anagedChannelTelemetryExporterBuilder.java | 25 +++--- .../sampler/DefaultGrpcServiceBuilder.java | 26 ++---- 11 files changed, 138 insertions(+), 199 deletions(-) delete mode 100644 exporters/jaeger/src/test/resources/tls-test.key delete mode 100644 exporters/jaeger/src/test/resources/tls-test.pem diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java index 6b1c81c85db..70625e4e71a 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java @@ -11,13 +11,14 @@ import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509KeyManager; import javax.net.ssl.X509TrustManager; -import okhttp3.OkHttpClient; /** * Utility class to help with management of TLS related components. This class is ultimately - * responsible for enabling TLS with the OkHttpClientBuilder by setting the sslSocketFactory. This - * class is only intended for internal OpenTelemetry exporter usage and should not be used by - * end-users. + * responsible for enabling TLS via callbacks passed to the configure[...]() methods. This class is + * only intended for internal OpenTelemetry exporter usage and should not be used by end-users. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. */ public class TlsConfigHelper { @@ -38,12 +39,12 @@ public TlsConfigHelper setTrustManager(X509TrustManager trustManager) { * @param trustedCertsPem Certificate in PEM format. * @return this */ - public TlsConfigHelper configureTrustManager(byte[] trustedCertsPem) { + public TlsConfigHelper createTrustManager(byte[] trustedCertsPem) { try { this.trustManager = TlsUtil.trustManager(trustedCertsPem); } catch (SSLException e) { throw new IllegalStateException( - "Error creating trust manager for OTLP HTTP connection with provided certs. Are they valid X.509 in PEM format?", + "Error creating X509TrustManager with provided certs. Are they valid X.509 in PEM format?", e); } return this; @@ -56,7 +57,7 @@ public TlsConfigHelper configureTrustManager(byte[] trustedCertsPem) { * @param certificatePem Certificate content in PEM format. * @return this */ - public TlsConfigHelper configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) { + public TlsConfigHelper createKeyManager(byte[] privateKeyPem, byte[] certificatePem) { try { if (keyManager != null) { logger.warning( @@ -66,7 +67,7 @@ public TlsConfigHelper configureKeyManager(byte[] privateKeyPem, byte[] certific return this; } catch (SSLException e) { throw new IllegalStateException( - "Error creating key manager for TLS configuration of OTLP HTTP connection with provided certs. Are they valid X.509 in PEM format?", + "Error creating X509KeyManager with provided certs. Are they valid X.509 in PEM format?", e); } } @@ -90,12 +91,47 @@ public TlsConfigHelper setSslSocketFactory(SSLSocketFactory sslSocketFactory) { return this; } - /** Configures TLS for the provided OkHttp client builder by setting the SSL Socket Factory. */ - public void configure(OkHttpClient.Builder clientBuilder) { + /** + * Functional wrapper type used in configure methods. Exists primarily to declare checked + * SSLException. + */ + public interface SslSocketFactoryConfigurer { + void configure(SSLSocketFactory sslSocketFactory, X509TrustManager trustManager) + throws SSLException; + } + + /** + * Functional wrapper type used in configure methods. Exists primarily to declare checked + * SSLException. + */ + public interface KeyManagerConfigurer { + void configure(X509TrustManager trustManager, X509KeyManager keyManager) throws SSLException; + } + + /** + * Configures TLS by invoking the given callback with the X509TrustManager and X509KeyManager. + * If the trust manager or key manager have not yet been configured, this method + * does nothing. + */ + public void configureWithKeyManager(KeyManagerConfigurer configureMethod) { + if (trustManager == null || keyManager == null) { + return; + } + try { + configureMethod.configure(trustManager, keyManager); + } catch (SSLException e) { + wrapException(e); + } + } + + /** + * Configures TLS by invoking the provided consumer with a new SSLSocketFactory and the + * preconfigured X509TrustManager. If the trust manager has not been configured, this + * method is effecting a noop. + */ + public void configureWithSocketFactory(SslSocketFactoryConfigurer configureMethod) { if (trustManager == null) { - if (warnIfOtherComponentsConfigured()) { - logger.warning("An X509TrustManager must be configured for TLS to be configured."); - } + warnIfOtherComponentsConfigured(); return; } @@ -104,23 +140,24 @@ public void configure(OkHttpClient.Builder clientBuilder) { if (sslSocketFactory == null) { sslSocketFactory = TlsUtil.sslSocketFactory(keyManager, trustManager); } - clientBuilder.sslSocketFactory(sslSocketFactory, trustManager); + configureMethod.configure(sslSocketFactory, trustManager); } catch (SSLException e) { - throw new IllegalStateException( - "Could not set trusted certificate for OTLP HTTP connection, are they valid X.509 in PEM format?", - e); + wrapException(e); } } - private boolean warnIfOtherComponentsConfigured() { + private static void wrapException(SSLException e) { + throw new IllegalStateException( + "Could not configure TLS connection, are certs in valid X.509 in PEM format?", e); + } + + private void warnIfOtherComponentsConfigured() { if (sslSocketFactory != null) { logger.warning("sslSocketFactory has been configured without an X509TrustManager."); - return true; + return; } if (keyManager != null) { logger.warning("An X509KeyManager has been configured without an X509TrustManager."); - return true; } - return false; } } diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java index f039b956180..cade0f018ca 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/GrpcExporterBuilder.java @@ -99,12 +99,12 @@ public GrpcExporterBuilder setCompression(String compressionMethod) { } public GrpcExporterBuilder configureTrustManager(byte[] trustedCertificatesPem) { - tlsConfigHelper.configureTrustManager(trustedCertificatesPem); + tlsConfigHelper.createTrustManager(trustedCertificatesPem); return this; } public GrpcExporterBuilder configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) { - tlsConfigHelper.configureKeyManager(privateKeyPem, certificatePem); + tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem); return this; } @@ -133,7 +133,7 @@ public GrpcExporter build() { clientBuilder.callTimeout(Duration.ofNanos(timeoutNanos)); - tlsConfigHelper.configure(clientBuilder); + tlsConfigHelper.configureWithSocketFactory(clientBuilder::sslSocketFactory); String endpoint = this.endpoint.resolve(grpcEndpointPath).toString(); if (endpoint.startsWith("http://")) { diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ManagedChannelUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ManagedChannelUtil.java index ce8972b6c61..2666f9562b4 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ManagedChannelUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ManagedChannelUtil.java @@ -12,6 +12,7 @@ import io.grpc.ManagedChannelBuilder; import io.grpc.netty.GrpcSslContexts; import io.grpc.netty.NettyChannelBuilder; +import io.netty.handler.ssl.SslContext; import io.opentelemetry.exporter.internal.TlsUtil; import io.opentelemetry.exporter.internal.retry.RetryPolicy; import io.opentelemetry.exporter.internal.retry.RetryUtil; @@ -23,7 +24,6 @@ import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; -import javax.annotation.Nullable; import javax.net.ssl.SSLException; import javax.net.ssl.X509KeyManager; import javax.net.ssl.X509TrustManager; @@ -45,48 +45,45 @@ public final class ManagedChannelUtil { * @throws SSLException if error occur processing the certificates */ public static void setClientKeysAndTrustedCertificatesPem( - ManagedChannelBuilder managedChannelBuilder, - @Nullable byte[] privateKeyPem, - @Nullable byte[] certificatePem, - byte[] trustedCertificatesPem) + ManagedChannelBuilder managedChannelBuilder, X509TrustManager tmf, X509KeyManager kmf) throws SSLException { requireNonNull(managedChannelBuilder, "managedChannelBuilder"); - requireNonNull(trustedCertificatesPem, "trustedCertificatesPem"); - - X509TrustManager tmf = TlsUtil.trustManager(trustedCertificatesPem); - X509KeyManager kmf = null; - if (privateKeyPem != null && certificatePem != null) { - kmf = TlsUtil.keyManager(privateKeyPem, certificatePem); - } + requireNonNull(kmf, "X509KeyManager"); + requireNonNull(tmf, "X509TrustManager"); // gRPC does not abstract TLS configuration so we need to check the implementation and act // accordingly. - if (managedChannelBuilder.getClass().getName().equals("io.grpc.netty.NettyChannelBuilder")) { - NettyChannelBuilder nettyBuilder = (NettyChannelBuilder) managedChannelBuilder; - nettyBuilder.sslContext( - GrpcSslContexts.forClient().keyManager(kmf).trustManager(tmf).build()); - } else if (managedChannelBuilder - .getClass() - .getName() - .equals("io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder")) { - io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder nettyBuilder = - (io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder) managedChannelBuilder; - nettyBuilder.sslContext( - io.grpc.netty.shaded.io.grpc.netty.GrpcSslContexts.forClient() - .trustManager(tmf) - .keyManager(kmf) - .build()); - } else if (managedChannelBuilder - .getClass() - .getName() - .equals("io.grpc.okhttp.OkHttpChannelBuilder")) { - io.grpc.okhttp.OkHttpChannelBuilder okHttpBuilder = - (io.grpc.okhttp.OkHttpChannelBuilder) managedChannelBuilder; - okHttpBuilder.sslSocketFactory(TlsUtil.sslSocketFactory(kmf, tmf)); - } else { - throw new SSLException( - "TLS certificate configuration not supported for unrecognized ManagedChannelBuilder " - + managedChannelBuilder.getClass().getName()); + String channelBuilderClassName = managedChannelBuilder.getClass().getName(); + switch (channelBuilderClassName) { + case "io.grpc.netty.NettyChannelBuilder": + { + NettyChannelBuilder nettyBuilder = (NettyChannelBuilder) managedChannelBuilder; + SslContext sslContext = + GrpcSslContexts.forClient().keyManager(kmf).trustManager(tmf).build(); + nettyBuilder.sslContext(sslContext); + break; + } + case "io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder": + { + io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder nettyBuilder = + (io.grpc.netty.shaded.io.grpc.netty.NettyChannelBuilder) managedChannelBuilder; + io.grpc.netty.shaded.io.netty.handler.ssl.SslContext sslContext = + io.grpc.netty.shaded.io.grpc.netty.GrpcSslContexts.forClient() + .trustManager(tmf) + .keyManager(kmf) + .build(); + nettyBuilder.sslContext(sslContext); + break; + } + case "io.grpc.okhttp.OkHttpChannelBuilder": + io.grpc.okhttp.OkHttpChannelBuilder okHttpBuilder = + (io.grpc.okhttp.OkHttpChannelBuilder) managedChannelBuilder; + okHttpBuilder.sslSocketFactory(TlsUtil.sslSocketFactory(kmf, tmf)); + break; + default: + throw new SSLException( + "TLS certificate configuration not supported for unrecognized ManagedChannelBuilder " + + channelBuilderClassName); } } diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java index fdeb800ebff..39bc659d9d9 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/okhttp/OkHttpExporterBuilder.java @@ -91,7 +91,7 @@ public OkHttpExporterBuilder setAuthenticator(Authenticator authenticator) { } public OkHttpExporterBuilder configureTrustManager(byte[] trustedCertificatesPem) { - tlsConfigHelper.configureTrustManager(trustedCertificatesPem); + tlsConfigHelper.createTrustManager(trustedCertificatesPem); return this; } @@ -101,7 +101,7 @@ public OkHttpExporterBuilder setTrustManager(X509TrustManager trustManager) { } public OkHttpExporterBuilder configureKeyManager(byte[] privateKeyPem, byte[] certificatePem) { - tlsConfigHelper.configureKeyManager(privateKeyPem, certificatePem); + tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem); return this; } @@ -136,7 +136,7 @@ public OkHttpExporter build() { .dispatcher(OkHttpUtil.newDispatcher()) .callTimeout(Duration.ofNanos(timeoutNanos)); - tlsConfigHelper.configure(clientBuilder); + tlsConfigHelper.configureWithSocketFactory(clientBuilder::sslSocketFactory); Headers headers = headersBuilder == null ? null : headersBuilder.build(); diff --git a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java index d1fb768bd25..d9b0e66f1db 100644 --- a/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java +++ b/exporters/jaeger/src/test/java/io/opentelemetry/exporter/jaeger/JaegerGrpcSpanExporterTest.java @@ -14,6 +14,7 @@ import com.linecorp.armeria.server.ServerBuilder; import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.grpc.protocol.AbstractUnaryGrpcService; +import com.linecorp.armeria.testing.junit5.server.SelfSignedCertificateExtension; import com.linecorp.armeria.testing.junit5.server.ServerExtension; import io.github.netmikey.logunit.api.LogCapturer; import io.opentelemetry.api.common.AttributeKey; @@ -38,10 +39,7 @@ import io.opentelemetry.sdk.trace.data.StatusData; import io.opentelemetry.semconv.resource.attributes.ResourceAttributes; import java.net.InetAddress; -import java.net.URI; import java.net.URISyntaxException; -import java.nio.file.Files; -import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -90,6 +88,12 @@ protected CompletionStage handleMessage( @RegisterExtension LogCapturer logs = LogCapturer.create().captureForType(OkHttpGrpcExporter.class); + @RegisterExtension + static final SelfSignedCertificateExtension serverTls = new SelfSignedCertificateExtension(); + + @RegisterExtension + static final SelfSignedCertificateExtension clientTls = new SelfSignedCertificateExtension(); + private static JaegerGrpcSpanExporter exporter; @BeforeAll @@ -288,18 +292,20 @@ private static SpanData testSpanData(Resource resource, String spanName) { @Test void validTrustedConfig() throws Exception { - byte[] cert = readResource("tls-test.pem"); - assertThatCode(() -> JaegerGrpcSpanExporter.builder().setTrustedCertificates(cert)) + assertThatCode( + () -> + JaegerGrpcSpanExporter.builder() + .setTrustedCertificates(serverTls.certificate().getEncoded())) .doesNotThrowAnyException(); } @Test void validClientKeyConfig() throws Exception { - - byte[] key = readResource("tls-test.key"); - byte[] cert = readResource("tls-test.pem"); - - assertThatCode(() -> JaegerGrpcSpanExporter.builder().setClientTls(key, cert)) + assertThatCode( + () -> + JaegerGrpcSpanExporter.builder() + .setClientTls( + clientTls.privateKey().getEncoded(), serverTls.certificate().getEncoded())) .doesNotThrowAnyException(); } @@ -398,9 +404,4 @@ void shutdown() { assertThat(exporter.shutdown().join(1, TimeUnit.SECONDS).isSuccess()).isTrue(); logs.assertContains("Calling shutdown() multiple times."); } - - byte[] readResource(String name) throws Exception { - URI uri = Thread.currentThread().getContextClassLoader().getResource(name).toURI(); - return Files.readAllBytes(Paths.get(uri)); - } } diff --git a/exporters/jaeger/src/test/resources/tls-test.key b/exporters/jaeger/src/test/resources/tls-test.key deleted file mode 100644 index e19fd059ca5..00000000000 --- a/exporters/jaeger/src/test/resources/tls-test.key +++ /dev/null @@ -1,52 +0,0 @@ ------BEGIN PRIVATE KEY----- -MIIJQwIBADANBgkqhkiG9w0BAQEFAASCCS0wggkpAgEAAoICAQCzY+EHeeEVxBRc -EE2ZeMA907V6tRP3G2c/EJ39VfsvR93QaBz12g0r7P8u5QMi+23yqcmvIVmO51zD -IrqjU0VQJf0FC+3IEsykN3AvatPEflxrpQosSnUXkmlTrNyBdw/v7AXjrA+EkQPn -ybXapiQYsCSoGNvpExY2Vm24jnBGIRjShXNA2FNBV4m47cVYA9Yeis0ATqlJA+Pz -4VAkgIAIwnRUXqB0BpVRYTG1EwJaWa3RCsPjk/bQJmF9vRsw0/CBQEYP03JqRROp -/xSZDk+lMFQpuoTwUNk54F8PBmgm//bYqnd4kHx/d3VjqHiAivbJTma0FCF8LBSe -+K3xiOLt8FKXOoVhmEwQ/LRSQvF7afitYpzBeL87Vkdek9l6ZEk1gkQMp5BQW8AY -Mcq+mXoVbCzYfeEocXo6219ca5kj6Fwjx6g1Nduz5x4QJ9gmrcgbLWV+Q0RVYzcb -4YuvGkCD6iDU0xWoQQpjBAHDOq1yxu69KjK5sy1V/zkV/vilYxkHBdYdIO4Kkp+Z -aGg9iQfqzsp0HJbSpEroRvvlXoTb4PKSYNhKUwWZ9CdPsqRj7nFs6dHV8DArSVYt -liF2dWW0oO38ftDWpasMk616Eu6Bjb+Q67KuYKphcpdiGlA5DeNMGzmcm5qJ7K8b -hdzNHFHw7e1CAxQ5yXjUJaffre4PgQIDAQABAoICAEFjyfYgrjRx6f35D2A/DMw8 -rK1S7jkn6wWo9/4NZmdAqhE6sjvHxP9a/1XHstIAln5a96J35J5bpYuX0DKSuvRR -NJ6vT3ibqa6+ao1OQm0gpm2ylje42F/ERdJzc1lZ3hxVi3wHDw0Ck0rdUwWlhAjf -xCzmCts10uXxsUMR4MkfbV3nuTBM6mQqFZCsU0AW/ejaLYnMIpbz7RbL7Zuwn3sR -7reGWrSQZcNpS2Y2zKfEgsBtTSRaniAXrXfZA91TlBp5JFDGgbFNBrLzwT1dYxjS -ugsP6z9yKQyYxe3DJxn4ObGWyb/P4yHPmKHAGlS40uZkRrmjkOUjaEzeVwFxJU57 -iBG1RVNbbAQN54pZoOmSqBIYkgzshg1Jt2GV3UwKuFg2BBnSiXwyKo+Zzf5VRGqO -Ix6eadJuOAHSxRE7RlslGEE5gisEysqH2aRq8oHuX7Y/zjfpkRo+F40OBL/gwh4E -2TduiiIdq6Tg1IysB/Pza31YAbxVr2YKmRCNN7Bamz7MHmEVgycfMmldVY1vApMy -KdB7tyHye2n0WCw4SfwTvawjLncZihLLGi2Rf+xGoUve+pfbl8++hzx7SdCOBvqe -FD/CbwQOpg7RBPwRPUHxufroZKJJf+e8av/A1HpLOlmv7+qD3v/YeiwWQR4fe0lM -Lvx/G2FLx1wp309xQW8xAoIBAQDohuSbogAZ4GtGBSL3pj4fyn3kQVi58sRdgCmE -QIKkOaoURo3lR3Fs52mu17jcRDO/EM7qo6DHduXYkcWI7iqoVki+304BPbWssZNC -k1kYYNg328esnr06jvs/9k2qiy/8SSbn8qZbV9e78JHxxXIpT57wlXzDwLEHpQdf -WDd3qQgEHQO/e/o8IsWxweH90feqvciJ+bxNByOnLo2IjXtIfSoNyuZdET16fJPl -uqdXHV/6+ZjRhbBjd7a/Kvog3+hptD5KWbqrBSAo9ddxGGlpLOfCK6r3NFoBNJyy -rs0W58ylIUtqVSi2X8xl1WVnAkjojd5TNF1scUP04Jy6P+vlAoIBAQDFf8rIOGhK -ZmFrgOuImUBb9mOGVBAw4kj1cu597t69Bh13cEdGL/raPeCmGla6xRWcoaZkB9fi -p4EW1TgmeU9knVC7suAqV6M4GLSdGIOh1og+plbc2UUjWru4l+Zomn8uHay+CWu2 -HrUyxPkrG8MRg0phnfaL6pWfGqFC6wIZvGNGS1diyxOlOYimJdGkLfFKKsEjiIbE -2xDZ8m/IGOpKk2SQiRs4DOXYP6kapK8Hd6/0vo2/J933QD0uRobgatQqoqrVWbWc -TRz15t8Z+PPFq7gg+FSFi9WF8/7gtfPkgC/pYmo/0XGikKERSrsrqCSBoo0rJQIg -LP7i/rF2WDNtAoIBAQCurvf6+178EZ5FQgKc7WTqWSLuYTEYsIpYe7m0wwnh+fgN -t1tODbfmSosYcLNNtmbxeN0ZZMeaUXYmloxBbDrbr8aCrhtZz1Q2Ykjw0OUz7T1B -6Xg99BrQbS3PRWffYSUxcHsr8RyMrpdBd18MtHopEsmij4rlPFNCHPZG6GzzFicR -gAKazbaVisHd4nu2RRC8UkrfbJ/LdjuQkuZ82Vzufikjp3QcIqF+7Svmf6zLQyQh -1XS6oqZ6cyr0voKeQ9fD4UaScIpNhFI+s0GgkuhNvRB2zHtprwiWs0Gs8qjcNlOF -Elgnj8ZADAPv1LqTAiJnonyNIV+14fldc4gKW48JAoIBAFL5o6qG+Fn5tYhC+7HC -ISYB2EKBYuQGzfxBMy25g98KiKq7g1Zbphq20IE4o6OIVdIeoBd6rBrPoQ6Eujh/ -0vlElrylJETl2O8igg9WU0dVKJyZqfjTV5bI8gFV4DmmXzRyxzZ4Isa7FAmmc8DG -wyfCbyegBAs4nW/g8kt5DMLfQR4xXR9mjnRmPaEAVpmRq8QrPkW9gFGIcAYBMzFw -Nk7qqd+HmuXrlU1LQbKt6dNLV6ONw2PCTiEo3gtpGhWDDGx1Nyy3qfNm+nnda3Iw -A8rSoGNJsckBvM2wey7RgwxHRtnYvSbeyc0w0lq/eO+Yju3f60trGv+Fp/iaZNiB -qekCggEBANoazggEv+EltSrSd4MJngnzdkWhqYxASttjfUymUAsqDz4g1VFvF8Ne -NgoAE0lgkcq1YxXghQoetUEv7hxLL2urAkuLkOMj10gW8rpfxEGvR5EhOxKhc5dD -L7e2cFQYVIustH4fJL6d/p6nY1sE2pU65Mls4m9npWvQCAJF2w11ROGEM1XF+0+5 -OUOvfTS88TU7SlDz9AMCLxSfP8W4n/7SzekQ3w0vdGYpxPmN0u9duYPJezOnJHgL -U3+VJJU7KAeCWlV595qAvkUG1jxrH4V76r9NjZbPIezdsJgvnCLyAqBehKIhbPhe -EOvS8wKB2dVM2ck1O39XTAz9yMNRIKs= ------END PRIVATE KEY----- diff --git a/exporters/jaeger/src/test/resources/tls-test.pem b/exporters/jaeger/src/test/resources/tls-test.pem deleted file mode 100644 index f6da368f063..00000000000 --- a/exporters/jaeger/src/test/resources/tls-test.pem +++ /dev/null @@ -1,28 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIEtDCCApwCCQDAUcLm8z1qxDANBgkqhkiG9w0BAQsFADAcMQswCQYDVQQGEwJV -UzENMAsGA1UECgwEb3RlbDAeFw0yMzAyMjQxNzQwMjdaFw0yNDAyMjQxNzQwMjda -MBwxCzAJBgNVBAYTAlVTMQ0wCwYDVQQKDARvdGVsMIICIjANBgkqhkiG9w0BAQEF -AAOCAg8AMIICCgKCAgEAs2PhB3nhFcQUXBBNmXjAPdO1erUT9xtnPxCd/VX7L0fd -0Ggc9doNK+z/LuUDIvtt8qnJryFZjudcwyK6o1NFUCX9BQvtyBLMpDdwL2rTxH5c -a6UKLEp1F5JpU6zcgXcP7+wF46wPhJED58m12qYkGLAkqBjb6RMWNlZtuI5wRiEY -0oVzQNhTQVeJuO3FWAPWHorNAE6pSQPj8+FQJICACMJ0VF6gdAaVUWExtRMCWlmt -0QrD45P20CZhfb0bMNPwgUBGD9NyakUTqf8UmQ5PpTBUKbqE8FDZOeBfDwZoJv/2 -2Kp3eJB8f3d1Y6h4gIr2yU5mtBQhfCwUnvit8Yji7fBSlzqFYZhMEPy0UkLxe2n4 -rWKcwXi/O1ZHXpPZemRJNYJEDKeQUFvAGDHKvpl6FWws2H3hKHF6OttfXGuZI+hc -I8eoNTXbs+ceECfYJq3IGy1lfkNEVWM3G+GLrxpAg+og1NMVqEEKYwQBwzqtcsbu -vSoyubMtVf85Ff74pWMZBwXWHSDuCpKfmWhoPYkH6s7KdByW0qRK6Eb75V6E2+Dy -kmDYSlMFmfQnT7KkY+5xbOnR1fAwK0lWLZYhdnVltKDt/H7Q1qWrDJOtehLugY2/ -kOuyrmCqYXKXYhpQOQ3jTBs5nJuaieyvG4XczRxR8O3tQgMUOcl41CWn363uD4EC -AwEAATANBgkqhkiG9w0BAQsFAAOCAgEARUvETIQYHxJTjy2sTopJQhgwjlYER3uY -rKi0fOu8bfH8IZW6KNJkHtNCwW062IDRUVqp1oBQLUJTN0ztf/5dUFW8f0gAEoam -hgOSV+3dS61CNk5TzimPwA0RlOxvlFbll1f92Hr6Zb3Y2vPE81dbg4JSqw1T5e40 -doSgOpvHzTjsoE3CsMGIAsr44mSIv9F722tseVBKKy0LwcNmeA/U0xNUhGD5tGiX -ZkvdgeLR3L6EIc8GyiA3ToPsKUDQSdTI+YXxOC3Vzwurb8+7TzE1PN4Kn2v69CVz -iDZdpiXQ2LM1/EFkX/g2saGT98wvtZx+MvIyyiwfLDSxeDIrEtH1ZeWCAyHu1mK3 -ysTWMiCaKuz6/NuG/+TvSRavICW9Rn2PsNVE3RzAFiZD6WZbF2eDZBMZDpvaK+M3 -bIHbk+ZIdDEkfM3vNLxn4SPX1Cb1cximt4Lg0W/G35CquZTj6AwAY/BZgGjz6usf -X4bUwjy7GY6603sohFmh8S7NLdUWVYPx3qIizAAJHlmj+f7z1S8COdnv9oYgn5QM -cfeSRAX3Tx3uWo4fvC9oC/tXhvpRuKrjeTscG3FeIuFW5MfxT5E0PF+zLUh+iFvB -jP4mKlwzd6rdNX9XmUYI/PJUkVEACwTwG0ZgMnMydZrXrtJa8LUr3L1tztnm1WUs -G2KD0lgGVqg= ------END CERTIFICATE----- diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java index 326159e221e..6b45c8a0f9c 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractGrpcTelemetryExporterTest.java @@ -304,8 +304,10 @@ void withHeaders() { void tls() throws Exception { TelemetryExporter exporter = exporterBuilder() - .setEndpoint(server.httpsUri().toString()) .setTrustedCertificates(Files.readAllBytes(certificate.certificateFile().toPath())) + .setClientTls( + certificate.privateKey().getEncoded(), certificate.certificate().getEncoded()) + .setEndpoint(server.httpsUri().toString()) .build(); try { CompletableResultCode result = @@ -339,7 +341,7 @@ void tls_badCert() { .setEndpoint(server.httpsUri().toString()) .setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8))) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Error creating trust manager for OTLP HTTP connection"); + .hasMessageContaining("Error creating X509TrustManager with provided certs."); } @ParameterizedTest @@ -651,8 +653,7 @@ void validConfig() { .doesNotThrowAnyException(); assertThatCode( - () -> - exporterBuilder().setTrustedCertificates(certificate.certificate().getEncoded())) + () -> exporterBuilder().setTrustedCertificates(certificate.certificate().getEncoded())) .doesNotThrowAnyException(); } diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java index 9d23420220c..10e6b3d1fdf 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/AbstractHttpTelemetryExporterTest.java @@ -370,7 +370,7 @@ void tls_badCert() { .setEndpoint(server.httpsUri() + path) .setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8))) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Error creating trust manager for OTLP HTTP connection"); + .hasMessageContaining("Error creating X509TrustManager with provided certs"); } @ParameterizedTest diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java index 7ef720419f6..e04e2ced7c5 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java @@ -9,6 +9,7 @@ import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; +import io.opentelemetry.exporter.internal.TlsConfigHelper; import io.opentelemetry.exporter.internal.grpc.ManagedChannelUtil; import io.opentelemetry.exporter.internal.otlp.OtlpUserAgent; import io.opentelemetry.exporter.internal.retry.RetryPolicy; @@ -18,7 +19,6 @@ import java.util.Collection; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; -import javax.net.ssl.SSLException; /** * Wraps a {@link TelemetryExporterBuilder}, delegating methods to upstream gRPC's {@link @@ -40,9 +40,7 @@ private ManagedChannelTelemetryExporterBuilder(TelemetryExporterBuilder deleg @Nullable private ManagedChannelBuilder channelBuilder; - @Nullable private byte[] privateKeyPem; - @Nullable private byte[] certificatePem; - @Nullable private byte[] trustedCertificatesPem; + private final TlsConfigHelper tlsConfigHelper = new TlsConfigHelper(); @Override public TelemetryExporterBuilder setEndpoint(String endpoint) { @@ -89,14 +87,13 @@ public TelemetryExporterBuilder addHeader(String key, String value) { @Override public TelemetryExporterBuilder setTrustedCertificates(byte[] certificates) { - this.trustedCertificatesPem = certificates; + tlsConfigHelper.createTrustManager(certificates); return this; } @Override public TelemetryExporterBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - this.privateKeyPem = privateKeyPem; - this.certificatePem = certificatePem; + tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem); return this; } @@ -126,15 +123,11 @@ public TelemetryExporterBuilder setChannel(ManagedChannel channel) { @Override public TelemetryExporter build() { requireNonNull(channelBuilder, "channel"); - if (trustedCertificatesPem != null) { - try { - ManagedChannelUtil.setClientKeysAndTrustedCertificatesPem( - channelBuilder, privateKeyPem, certificatePem, trustedCertificatesPem); - } catch (SSLException e) { - throw new IllegalStateException( - "Could not set trusted certificates, are they valid X.509 in PEM format?", e); - } - } + + tlsConfigHelper.configureWithKeyManager( + (tm, km) -> + ManagedChannelUtil.setClientKeysAndTrustedCertificatesPem(channelBuilder, tm, km)); + ManagedChannel channel = channelBuilder.build(); delegate.setChannel(channel); TelemetryExporter delegateExporter = delegate.build(); diff --git a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilder.java b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilder.java index b7e7a398c4b..e0d39cfa23b 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilder.java +++ b/sdk-extensions/jaeger-remote-sampler/src/main/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilder.java @@ -16,6 +16,7 @@ import io.grpc.Metadata; import io.grpc.stub.MetadataUtils; import io.opentelemetry.exporter.internal.ExporterBuilderUtil; +import io.opentelemetry.exporter.internal.TlsConfigHelper; import io.opentelemetry.exporter.internal.grpc.ManagedChannelUtil; import io.opentelemetry.exporter.internal.grpc.MarshalerServiceStub; import io.opentelemetry.exporter.internal.marshal.Marshaler; @@ -25,7 +26,6 @@ import java.util.concurrent.TimeUnit; import java.util.function.Function; import javax.annotation.Nullable; -import javax.net.ssl.SSLException; final class DefaultGrpcServiceBuilder implements GrpcServiceBuilder { @@ -39,10 +39,8 @@ final class DefaultGrpcServiceBuilder setCompression(String compressionMe public DefaultGrpcServiceBuilder setTrustedCertificates( byte[] trustedCertificatesPem) { requireNonNull(trustedCertificatesPem, "trustedCertificatesPem"); - this.trustedCertificatesPem = trustedCertificatesPem; + tlsConfigHelper.createTrustManager(trustedCertificatesPem); return this; } @Override public GrpcServiceBuilder setClientTls(byte[] privateKeyPem, byte[] certificatePem) { - this.privateKeyPem = privateKeyPem; - this.certificatePem = certificatePem; + tlsConfigHelper.createKeyManager(privateKeyPem, certificatePem); return this; } @@ -147,17 +144,10 @@ public GrpcService build() { managedChannelBuilder.intercept(MetadataUtils.newAttachHeadersInterceptor(metadata)); } - if (trustedCertificatesPem != null) { - try { - ManagedChannelUtil.setClientKeysAndTrustedCertificatesPem( - managedChannelBuilder, privateKeyPem, certificatePem, trustedCertificatesPem); - } catch (SSLException e) { - throw new IllegalStateException( - "Could not set trusted certificates for gRPC TLS connection, are they valid " - + "X.509 in PEM format?", - e); - } - } + tlsConfigHelper.configureWithKeyManager( + (tm, km) -> + ManagedChannelUtil.setClientKeysAndTrustedCertificatesPem( + managedChannelBuilder, tm, km)); if (retryPolicy != null) { managedChannelBuilder.defaultServiceConfig(toServiceConfig(grpcServiceName, retryPolicy)); From db8d08bfdc66c058ea4f65e4ea5b62b4661993e2 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 24 Feb 2023 16:21:14 -0800 Subject: [PATCH 07/14] fix typo --- .../opentelemetry/exporter/internal/TlsConfigHelper.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java index 70625e4e71a..c165dcb30ef 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java @@ -109,9 +109,8 @@ public interface KeyManagerConfigurer { } /** - * Configures TLS by invoking the given callback with the X509TrustManager and X509KeyManager. - * If the trust manager or key manager have not yet been configured, this method - * does nothing. + * Configures TLS by invoking the given callback with the X509TrustManager and X509KeyManager. If + * the trust manager or key manager have not yet been configured, this method does nothing. */ public void configureWithKeyManager(KeyManagerConfigurer configureMethod) { if (trustManager == null || keyManager == null) { @@ -126,8 +125,8 @@ public void configureWithKeyManager(KeyManagerConfigurer configureMethod) { /** * Configures TLS by invoking the provided consumer with a new SSLSocketFactory and the - * preconfigured X509TrustManager. If the trust manager has not been configured, this - * method is effecting a noop. + * preconfigured X509TrustManager. If the trust manager has not been configured, this method is + * effectively a noop. */ public void configureWithSocketFactory(SslSocketFactoryConfigurer configureMethod) { if (trustManager == null) { From cb56d613f47cdb5176f0bc60e88946a9f2fdeb18 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 24 Feb 2023 17:35:36 -0800 Subject: [PATCH 08/14] allow keymanager to be nullable. --- .../io/opentelemetry/exporter/internal/TlsConfigHelper.java | 5 +++-- .../exporter/internal/grpc/ManagedChannelUtil.java | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java index c165dcb30ef..6bb51e8319f 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java @@ -105,7 +105,8 @@ void configure(SSLSocketFactory sslSocketFactory, X509TrustManager trustManager) * SSLException. */ public interface KeyManagerConfigurer { - void configure(X509TrustManager trustManager, X509KeyManager keyManager) throws SSLException; + void configure(X509TrustManager trustManager, @Nullable X509KeyManager keyManager) + throws SSLException; } /** @@ -113,7 +114,7 @@ public interface KeyManagerConfigurer { * the trust manager or key manager have not yet been configured, this method does nothing. */ public void configureWithKeyManager(KeyManagerConfigurer configureMethod) { - if (trustManager == null || keyManager == null) { + if (trustManager == null) { return; } try { diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ManagedChannelUtil.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ManagedChannelUtil.java index 2666f9562b4..df2b3141d78 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ManagedChannelUtil.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/grpc/ManagedChannelUtil.java @@ -24,6 +24,7 @@ import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; +import javax.annotation.Nullable; import javax.net.ssl.SSLException; import javax.net.ssl.X509KeyManager; import javax.net.ssl.X509TrustManager; @@ -45,10 +46,11 @@ public final class ManagedChannelUtil { * @throws SSLException if error occur processing the certificates */ public static void setClientKeysAndTrustedCertificatesPem( - ManagedChannelBuilder managedChannelBuilder, X509TrustManager tmf, X509KeyManager kmf) + ManagedChannelBuilder managedChannelBuilder, + X509TrustManager tmf, + @Nullable X509KeyManager kmf) throws SSLException { requireNonNull(managedChannelBuilder, "managedChannelBuilder"); - requireNonNull(kmf, "X509KeyManager"); requireNonNull(tmf, "X509TrustManager"); // gRPC does not abstract TLS configuration so we need to check the implementation and act From f86c4db63bbfd6abf52de44ba8432d9d6c371e6d Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 24 Feb 2023 17:49:38 -0800 Subject: [PATCH 09/14] fix test --- .../jaeger/sampler/DefaultGrpcServiceBuilderTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilderTest.java b/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilderTest.java index dd8d231e03c..efe3bc193b6 100644 --- a/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilderTest.java +++ b/sdk-extensions/jaeger-remote-sampler/src/testGrpcNetty/java/io/opentelemetry/sdk/extension/trace/jaeger/sampler/DefaultGrpcServiceBuilderTest.java @@ -8,15 +8,19 @@ import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import com.linecorp.armeria.testing.junit5.server.SelfSignedCertificateExtension; import io.opentelemetry.exporter.internal.retry.RetryPolicy; import java.net.URI; -import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; class DefaultGrpcServiceBuilderTest { + @RegisterExtension + static final SelfSignedCertificateExtension serverTls = new SelfSignedCertificateExtension(); + private static DefaultGrpcServiceBuilder< SamplingStrategyParametersMarshaler, SamplingStrategyResponseUnMarshaler> exporterBuilder() { @@ -52,8 +56,7 @@ void validConfig() { .doesNotThrowAnyException(); assertThatCode( - () -> - exporterBuilder().setTrustedCertificates("foobar".getBytes(StandardCharsets.UTF_8))) + () -> exporterBuilder().setTrustedCertificates(serverTls.certificate().getEncoded())) .doesNotThrowAnyException(); assertThatCode(() -> exporterBuilder().addRetryPolicy(RetryPolicy.getDefault())) From ac97ec834136bf63edb913122347501075269d98 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Fri, 24 Feb 2023 18:05:27 -0800 Subject: [PATCH 10/14] so....much...null......away --- .../internal/ManagedChannelTelemetryExporterBuilder.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java index e04e2ced7c5..7ff40964207 100644 --- a/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java +++ b/exporters/otlp/testing-internal/src/main/java/io/opentelemetry/exporter/otlp/testing/internal/ManagedChannelTelemetryExporterBuilder.java @@ -125,8 +125,10 @@ public TelemetryExporter build() { requireNonNull(channelBuilder, "channel"); tlsConfigHelper.configureWithKeyManager( - (tm, km) -> - ManagedChannelUtil.setClientKeysAndTrustedCertificatesPem(channelBuilder, tm, km)); + (tm, km) -> { + requireNonNull(channelBuilder, "channel"); + ManagedChannelUtil.setClientKeysAndTrustedCertificatesPem(channelBuilder, tm, km); + }); ManagedChannel channel = channelBuilder.build(); delegate.setChannel(channel); From e45fb1baa47fb8a7e03386877ec02c69d5ac5e1c Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Mon, 27 Feb 2023 13:42:26 -0800 Subject: [PATCH 11/14] backfill tests --- .../exporter/internal/TlsConfigHelper.java | 35 +++- .../internal/TlsConfigHelperTest.java | 163 ++++++++++++++++++ 2 files changed, 195 insertions(+), 3 deletions(-) create mode 100644 exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java index 6bb51e8319f..1e18110248b 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java @@ -5,6 +5,7 @@ package io.opentelemetry.exporter.internal; +import com.google.common.annotations.VisibleForTesting; import java.util.logging.Logger; import javax.annotation.Nullable; import javax.net.ssl.SSLException; @@ -24,10 +25,21 @@ public class TlsConfigHelper { private static final Logger logger = Logger.getLogger(TlsConfigHelper.class.getName()); + private final TlsUtility tlsUtil; + @Nullable private X509KeyManager keyManager; @Nullable private X509TrustManager trustManager; @Nullable private SSLSocketFactory sslSocketFactory; + public TlsConfigHelper() { + this(new TlsUtility() {}); + } + + @VisibleForTesting + TlsConfigHelper(TlsUtility tlsUtil) { + this.tlsUtil = tlsUtil; + } + public TlsConfigHelper setTrustManager(X509TrustManager trustManager) { this.trustManager = trustManager; return this; @@ -41,7 +53,7 @@ public TlsConfigHelper setTrustManager(X509TrustManager trustManager) { */ public TlsConfigHelper createTrustManager(byte[] trustedCertsPem) { try { - this.trustManager = TlsUtil.trustManager(trustedCertsPem); + this.trustManager = tlsUtil.trustManager(trustedCertsPem); } catch (SSLException e) { throw new IllegalStateException( "Error creating X509TrustManager with provided certs. Are they valid X.509 in PEM format?", @@ -63,7 +75,7 @@ public TlsConfigHelper createKeyManager(byte[] privateKeyPem, byte[] certificate logger.warning( "Previous X509 Key manager is being replaced. This is probably an error and should only be set once."); } - keyManager = TlsUtil.keyManager(privateKeyPem, certificatePem); + keyManager = tlsUtil.keyManager(privateKeyPem, certificatePem); return this; } catch (SSLException e) { throw new IllegalStateException( @@ -138,7 +150,7 @@ public void configureWithSocketFactory(SslSocketFactoryConfigurer configureMetho try { SSLSocketFactory sslSocketFactory = this.sslSocketFactory; if (sslSocketFactory == null) { - sslSocketFactory = TlsUtil.sslSocketFactory(keyManager, trustManager); + sslSocketFactory = tlsUtil.sslSocketFactory(keyManager, trustManager); } configureMethod.configure(sslSocketFactory, trustManager); } catch (SSLException e) { @@ -160,4 +172,21 @@ private void warnIfOtherComponentsConfigured() { logger.warning("An X509KeyManager has been configured without an X509TrustManager."); } } + + // Exists for testing + interface TlsUtility { + default SSLSocketFactory sslSocketFactory( + @Nullable X509KeyManager keyManager, X509TrustManager trustManager) throws SSLException { + return TlsUtil.sslSocketFactory(keyManager, trustManager); + } + + default X509TrustManager trustManager(byte[] trustedCertificatesPem) throws SSLException { + return TlsUtil.trustManager(trustedCertificatesPem); + } + + default X509KeyManager keyManager(byte[] privateKeyPem, byte[] certificatePem) + throws SSLException { + return TlsUtil.keyManager(privateKeyPem, certificatePem); + } + } } diff --git a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java new file mode 100644 index 00000000000..7de97f21815 --- /dev/null +++ b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java @@ -0,0 +1,163 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.exporter.internal; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.linecorp.armeria.testing.junit5.server.SelfSignedCertificateExtension; +import java.util.concurrent.atomic.AtomicReference; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.X509KeyManager; +import javax.net.ssl.X509TrustManager; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class TlsConfigHelperTest { + @RegisterExtension + static final SelfSignedCertificateExtension serverTls = new SelfSignedCertificateExtension(); + + @Mock TlsConfigHelper.TlsUtility tlsUtil; + + @Test + void createTrustManager_exceptionMapped() throws Exception { + when(tlsUtil.trustManager(any())).thenThrow(new SSLException("boom")); + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + assertThrows( + IllegalStateException.class, + () -> { + helper.createTrustManager(serverTls.certificate().getEncoded()); + }); + } + + @Test + void createKeymanager_exceptionMapped() throws Exception { + when(tlsUtil.keyManager(any(), any())).thenThrow(new SSLException("boom")); + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + assertThrows( + IllegalStateException.class, + () -> { + helper.createKeyManager( + serverTls.privateKey().getEncoded(), serverTls.certificate().getEncoded()); + }); + } + + @Test + void socketFactory_happyPathWithBytes() throws Exception { + + byte[] cert = serverTls.certificate().getEncoded(); + byte[] key = serverTls.privateKey().getEncoded(); + AtomicReference seenTrustManager = new AtomicReference<>(); + AtomicReference seenSocketFactory = new AtomicReference<>(); + + X509TrustManager trustManager = mock(X509TrustManager.class); + X509KeyManager keyManager = mock(X509KeyManager.class); + SSLSocketFactory sslSocketFactory = mock(SSLSocketFactory.class); + + when(tlsUtil.trustManager(cert)).thenReturn(trustManager); + when(tlsUtil.keyManager(key, cert)).thenReturn(keyManager); + when(tlsUtil.sslSocketFactory(keyManager, trustManager)).thenReturn(sslSocketFactory); + + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + + helper.createKeyManager(key, cert); + helper.createTrustManager(cert); + helper.configureWithSocketFactory( + (socketFactory, tManager) -> { + seenTrustManager.set(tManager); + seenSocketFactory.set(socketFactory); + }); + assertSame(trustManager, seenTrustManager.get()); + assertSame(sslSocketFactory, seenSocketFactory.get()); + } + + @Test + void socketFactory_noTrustManager() { + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + helper.configureWithSocketFactory( + (sf, tm) -> { + fail("Should not have called due to missing trust manager"); + }); + verifyNoInteractions(tlsUtil); + } + + @Test + void socketFactoryCallbackExceptionHandled() { + X509TrustManager trustManager = mock(X509TrustManager.class); + + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + + helper.setTrustManager(trustManager); + assertThrows( + IllegalStateException.class, + () -> { + helper.configureWithSocketFactory( + (sf, tm) -> { + throw new SSLException("bad times"); + }); + }); + } + + @Test + void configureWithKeyManagerHappyPath() { + AtomicReference seenTrustManager = new AtomicReference<>(); + AtomicReference seenKeyManager = new AtomicReference<>(); + + X509TrustManager trustManager = mock(X509TrustManager.class); + X509KeyManager keyManager = mock(X509KeyManager.class); + + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + + helper.setTrustManager(trustManager); + helper.setKeyManager(keyManager); + helper.configureWithKeyManager( + (tm, km) -> { + seenTrustManager.set(tm); + seenKeyManager.set(km); + }); + assertThat(seenTrustManager.get()).isSameAs(trustManager); + assertThat(seenKeyManager.get()).isSameAs(keyManager); + } + + @Test + void configureWithKeyManagerExceptionHandled() { + X509TrustManager trustManager = mock(X509TrustManager.class); + X509KeyManager keyManager = mock(X509KeyManager.class); + + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + + helper.setTrustManager(trustManager); + helper.setKeyManager(keyManager); + assertThrows( + IllegalStateException.class, + () -> + helper.configureWithKeyManager( + (trustManager1, keyManager1) -> { + throw new SSLException("ouchey"); + })); + } + + @Test + void configureWithKeyManagerNoTrustManager() { + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + helper.configureWithKeyManager( + (tm, km) -> { + fail(); + }); + verifyNoInteractions(tlsUtil); + } +} From f70e7928971146878fc1bb1e80188d29dd6aca60 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Mon, 27 Feb 2023 13:59:21 -0800 Subject: [PATCH 12/14] checkstyle --- .../exporter/internal/TlsConfigHelperTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java index 7de97f21815..f7477df84a8 100644 --- a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java +++ b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java @@ -77,9 +77,9 @@ void socketFactory_happyPathWithBytes() throws Exception { helper.createKeyManager(key, cert); helper.createTrustManager(cert); helper.configureWithSocketFactory( - (socketFactory, tManager) -> { - seenTrustManager.set(tManager); - seenSocketFactory.set(socketFactory); + (sf, tm) -> { + seenTrustManager.set(tm); + seenSocketFactory.set(sf); }); assertSame(trustManager, seenTrustManager.get()); assertSame(sslSocketFactory, seenSocketFactory.get()); From e30cbff901f5a229d7b8d721669204c243cd20d6 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Mon, 27 Feb 2023 14:20:22 -0800 Subject: [PATCH 13/14] test coverage --- .../internal/TlsConfigHelperTest.java | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java index f7477df84a8..ccdac920265 100644 --- a/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java +++ b/exporters/common/src/test/java/io/opentelemetry/exporter/internal/TlsConfigHelperTest.java @@ -15,6 +15,7 @@ import static org.mockito.Mockito.when; import com.linecorp.armeria.testing.junit5.server.SelfSignedCertificateExtension; +import io.github.netmikey.logunit.api.LogCapturer; import java.util.concurrent.atomic.AtomicReference; import javax.net.ssl.SSLException; import javax.net.ssl.SSLSocketFactory; @@ -31,6 +32,9 @@ class TlsConfigHelperTest { @RegisterExtension static final SelfSignedCertificateExtension serverTls = new SelfSignedCertificateExtension(); + @RegisterExtension + LogCapturer logs = LogCapturer.create().captureForLogger(TlsConfigHelper.class.getName()); + @Mock TlsConfigHelper.TlsUtility tlsUtil; @Test @@ -160,4 +164,38 @@ void configureWithKeyManagerNoTrustManager() { }); verifyNoInteractions(tlsUtil); } + + @Test + void setKeyManagerReplacesAndWarns() { + X509KeyManager keyManager1 = mock(X509KeyManager.class); + X509KeyManager keyManager2 = mock(X509KeyManager.class); + + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + + helper.setTrustManager(mock(X509TrustManager.class)); + helper.setKeyManager(keyManager1); + helper.setKeyManager(keyManager2); + + helper.configureWithKeyManager((tm, km) -> assertSame(km, keyManager2)); + logs.assertContains("Previous X509 Key manager is being replaced"); + } + + @Test + void createKeyManagerReplacesAndWarns() throws Exception { + X509KeyManager keyManager1 = mock(X509KeyManager.class); + X509KeyManager keyManager2 = mock(X509KeyManager.class); + + byte[] cert = serverTls.certificate().getEncoded(); + byte[] key = serverTls.privateKey().getEncoded(); + + when(tlsUtil.keyManager(key, cert)).thenReturn(keyManager2); + TlsConfigHelper helper = new TlsConfigHelper(tlsUtil); + + helper.setTrustManager(mock(X509TrustManager.class)); + helper.setKeyManager(keyManager1); + helper.createKeyManager(key, cert); + + helper.configureWithKeyManager((tm, km) -> assertSame(km, keyManager2)); + logs.assertContains("Previous X509 Key manager is being replaced"); + } } From 0446b73f1ba13fd0fa6658f0de5e699e8724c038 Mon Sep 17 00:00:00 2001 From: Jason Plumb Date: Thu, 2 Mar 2023 15:03:30 -0800 Subject: [PATCH 14/14] address code review comments --- .../exporter/internal/TlsConfigHelper.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java index 1e18110248b..ce5555e83d7 100644 --- a/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java +++ b/exporters/common/src/main/java/io/opentelemetry/exporter/internal/TlsConfigHelper.java @@ -40,6 +40,7 @@ public TlsConfigHelper() { this.tlsUtil = tlsUtil; } + /** Sets the X509TrustManager. */ public TlsConfigHelper setTrustManager(X509TrustManager trustManager) { this.trustManager = trustManager; return this; @@ -98,6 +99,10 @@ public TlsConfigHelper setKeyManager(X509KeyManager keyManager) { return this; } + /** + * Sets the SSLSocketFactory, which is passed into the callback within + * configureWithSocketFactory(). + */ public TlsConfigHelper setSslSocketFactory(SSLSocketFactory sslSocketFactory) { this.sslSocketFactory = sslSocketFactory; return this; @@ -125,12 +130,12 @@ void configure(X509TrustManager trustManager, @Nullable X509KeyManager keyManage * Configures TLS by invoking the given callback with the X509TrustManager and X509KeyManager. If * the trust manager or key manager have not yet been configured, this method does nothing. */ - public void configureWithKeyManager(KeyManagerConfigurer configureMethod) { + public void configureWithKeyManager(KeyManagerConfigurer configurer) { if (trustManager == null) { return; } try { - configureMethod.configure(trustManager, keyManager); + configurer.configure(trustManager, keyManager); } catch (SSLException e) { wrapException(e); } @@ -138,10 +143,10 @@ public void configureWithKeyManager(KeyManagerConfigurer configureMethod) { /** * Configures TLS by invoking the provided consumer with a new SSLSocketFactory and the - * preconfigured X509TrustManager. If the trust manager has not been configured, this method is - * effectively a noop. + * preconfigured X509TrustManager. If the trust manager has not been configured, this method does + * nothing. */ - public void configureWithSocketFactory(SslSocketFactoryConfigurer configureMethod) { + public void configureWithSocketFactory(SslSocketFactoryConfigurer configurer) { if (trustManager == null) { warnIfOtherComponentsConfigured(); return; @@ -152,7 +157,7 @@ public void configureWithSocketFactory(SslSocketFactoryConfigurer configureMetho if (sslSocketFactory == null) { sslSocketFactory = tlsUtil.sslSocketFactory(keyManager, trustManager); } - configureMethod.configure(sslSocketFactory, trustManager); + configurer.configure(sslSocketFactory, trustManager); } catch (SSLException e) { wrapException(e); }