From 9ce0e9085acc6b091eb628605fd5b0e47f272905 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Fri, 1 Mar 2019 18:30:53 +0900 Subject: [PATCH 01/25] add certificate pinner method in HttpSender Signed-off-by: Iori Yoneji --- .../java/io/jaegertracing/Configuration.java | 15 ++++++++++++ .../thrift/internal/senders/HttpSender.java | 23 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/jaeger-core/src/main/java/io/jaegertracing/Configuration.java b/jaeger-core/src/main/java/io/jaegertracing/Configuration.java index b75bc77e4..ea78d3cb2 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/Configuration.java +++ b/jaeger-core/src/main/java/io/jaegertracing/Configuration.java @@ -614,6 +614,11 @@ public static class SenderConfiguration { */ private String authPassword; + /** + * The comma-separated certificate list for the endpoint which is self-signed, for example + */ + private String certificates; + public SenderConfiguration() { } @@ -647,6 +652,16 @@ public SenderConfiguration withAuthPassword(String password) { return this; } + public SenderConfiguration withCertificate(String certs) { + this.certificates = certs; + return this; + } + + public SenderConfiguration withTls(String password) { + this.authPassword = password; + return this; + } + /** * Returns a sender if one was given when creating the configuration, or attempts to create a sender based on the * configuration's state. diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index ba627d4d8..e6dbec9e0 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -19,8 +19,10 @@ import io.jaegertracing.thriftjava.Process; import io.jaegertracing.thriftjava.Span; import java.io.IOException; +import java.net.URI; import java.util.List; import lombok.ToString; +import okhttp3.CertificatePinner; import okhttp3.Credentials; import okhttp3.HttpUrl; import okhttp3.Interceptor; @@ -46,6 +48,7 @@ protected HttpSender(Builder builder) { if (collectorUrl == null) { throw new IllegalArgumentException("Could not parse url."); } + this.httpClient = builder.clientBuilder.build(); this.requestBuilder = new Request.Builder().url(collectorUrl); } @@ -88,6 +91,8 @@ public void send(Process process, List spans) throws SenderException { public static class Builder { private final String endpoint; + private CertificatePinner.Builder certificatePinnerBuilder = new CertificatePinner.Builder(); + private boolean tls; private int maxPacketSize = ONE_MB_IN_BYTES; private Interceptor authInterceptor; private OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder(); @@ -119,10 +124,28 @@ public Builder withAuth(String authToken) { return this; } + public Builder withCertificate(String sha256certs /* comma separated */) { + String hostname; + try { + URI uri = new URI(endpoint); + hostname = uri.getHost(); + tls = true; + String certs[] = sha256certs.split(","); + for (String cert: certs) { + certificatePinnerBuilder.add(hostname, String.format("sha256/%s", cert)); + } + } finally { + return this; + } + } + public HttpSender build() { if (authInterceptor != null) { clientBuilder.addInterceptor(authInterceptor); } + if (tls) { + clientBuilder.certificatePinner(certificatePinnerBuilder.build()); + } return new HttpSender(this); } From 1d492c82dadf21c312188f0a0297d25e023cafd0 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Mon, 4 Mar 2019 17:49:34 +0900 Subject: [PATCH 02/25] add senderConfiguration option to specify the endpoint certificate Signed-off-by: Iori Yoneji --- .../main/java/io/jaegertracing/Configuration.java | 13 ++++++++----- .../thrift/internal/senders/HttpSender.java | 14 ++++++++------ .../internal/senders/ThriftSenderFactory.java | 4 ++++ 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/jaeger-core/src/main/java/io/jaegertracing/Configuration.java b/jaeger-core/src/main/java/io/jaegertracing/Configuration.java index ea78d3cb2..a9e64711a 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/Configuration.java +++ b/jaeger-core/src/main/java/io/jaegertracing/Configuration.java @@ -152,6 +152,12 @@ public class Configuration { */ public static final String JAEGER_TRACEID_128BIT = JAEGER_PREFIX + "TRACEID_128BIT"; + /** + * TLS certificates (in comma-separated BASE64 SHA256 Hash) for certificates pinning, + * used in case of HTTPS communication to the endpoint. + */ + public static final String JAEGER_TLS_CERTIFICATES = JAEGER_PREFIX + "TLS_CERTIFICATES"; + /** * The supported trace context propagation formats. */ @@ -657,11 +663,6 @@ public SenderConfiguration withCertificate(String certs) { return this; } - public SenderConfiguration withTls(String password) { - this.authPassword = password; - return this; - } - /** * Returns a sender if one was given when creating the configuration, or attempts to create a sender based on the * configuration's state. @@ -686,6 +687,7 @@ public static SenderConfiguration fromEnv() { String authToken = getProperty(JAEGER_AUTH_TOKEN); String authUsername = getProperty(JAEGER_USER); String authPassword = getProperty(JAEGER_PASSWORD); + String certificates = getProperty(JAEGER_TLS_CERTIFICATES); return new SenderConfiguration() .withAgentHost(agentHost) @@ -693,6 +695,7 @@ public static SenderConfiguration fromEnv() { .withEndpoint(collectorEndpoint) .withAuthToken(authToken) .withAuthUsername(authUsername) + .withCertificate(certificates) .withAuthPassword(authPassword); } } diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index e6dbec9e0..f8476f6dc 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -124,15 +124,17 @@ public Builder withAuth(String authToken) { return this; } - public Builder withCertificate(String sha256certs /* comma separated */) { + public Builder withCertificates(String sha256certs /* comma separated */) { String hostname; try { URI uri = new URI(endpoint); - hostname = uri.getHost(); - tls = true; - String certs[] = sha256certs.split(","); - for (String cert: certs) { - certificatePinnerBuilder.add(hostname, String.format("sha256/%s", cert)); + if (uri.getScheme() == "https") { + hostname = uri.getHost(); + this.tls = true; + String certs[] = sha256certs.split(","); + for (String cert: certs) { + certificatePinnerBuilder.add(hostname, String.format("sha256/%s", cert)); + } } } finally { return this; diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactory.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactory.java index 419105748..bf5038ede 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactory.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactory.java @@ -26,6 +26,10 @@ public Sender getSender(Configuration.SenderConfiguration conf) { httpSenderBuilder.withAuth(conf.getAuthToken()); } + if (null != conf.getCertificates() && !conf.getCertificates().isEmpty()) { + httpSenderBuilder.withCertificates(conf.getCertificates()); + } + log.debug("Using the HTTP Sender to send spans directly to the endpoint."); return httpSenderBuilder.build(); } From c99e96fc0d13e273f890b9bd85e94f7e9f61c1a7 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Tue, 5 Mar 2019 20:51:59 +0900 Subject: [PATCH 03/25] add unsafeNoopVerifier Signed-off-by: Iori Yoneji --- .../thrift/internal/senders/HttpSender.java | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index f8476f6dc..6a1f2528f 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -21,6 +21,12 @@ import java.io.IOException; import java.net.URI; import java.util.List; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocketFactory; +import javax.net.ssl.X509TrustManager; +import javax.net.ssl.TrustManager; import lombok.ToString; import okhttp3.CertificatePinner; import okhttp3.Credentials; @@ -92,7 +98,7 @@ public void send(Process process, List spans) throws SenderException { public static class Builder { private final String endpoint; private CertificatePinner.Builder certificatePinnerBuilder = new CertificatePinner.Builder(); - private boolean tls; + private boolean pinning; private int maxPacketSize = ONE_MB_IN_BYTES; private Interceptor authInterceptor; private OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder(); @@ -124,13 +130,13 @@ public Builder withAuth(String authToken) { return this; } - public Builder withCertificates(String sha256certs /* comma separated */) { + public Builder withCertificatePinning(String sha256certs /* comma separated */) { String hostname; try { - URI uri = new URI(endpoint); + final URI uri = new URI(endpoint); if (uri.getScheme() == "https") { hostname = uri.getHost(); - this.tls = true; + this.pinning = true; String certs[] = sha256certs.split(","); for (String cert: certs) { certificatePinnerBuilder.add(hostname, String.format("sha256/%s", cert)); @@ -141,11 +147,32 @@ public Builder withCertificates(String sha256certs /* comma separated */) { } } + public Builder disableCertVerification() { + try { + final TrustManager[] unsafeNoopVerificator = new TrustManager[] { + new X509TrustManager() { + @Override public void checkServerTrusted(java.security.cert.X509Certificate[] chain, String authType) throws CertificateException {} + @Override public void checkClientTrusted(java.security.cert.X509Certificate[] chain, String authType) throws CertificateException {} + @Override public java.security.cert.X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; + } + } + }; + final SSLContext sslContext = SSLContext.getInstance("TLS"); + sslContext.init(null, unsafeNoopVerificator, null); + clientBuilder.sslSocketFactory(sslContext.getSocketFactory(), (X509TrustManager) unsafeNoopVerificator[0]); + + } catch (Exception e) { + throw new RuntimeException(e); + } + return this; + } + public HttpSender build() { if (authInterceptor != null) { clientBuilder.addInterceptor(authInterceptor); } - if (tls) { + if (pinning) { clientBuilder.certificatePinner(certificatePinnerBuilder.build()); } return new HttpSender(this); From 7a02c673fc5f256bcebb881029f6e3af4c8c83b3 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Tue, 5 Mar 2019 20:59:50 +0900 Subject: [PATCH 04/25] change server certificate pinning interfaces Signed-off-by: Iori Yoneji --- .../java/io/jaegertracing/Configuration.java | 14 ++++--- .../thrift/internal/senders/HttpSender.java | 38 +++++++++++-------- .../internal/senders/ThriftSenderFactory.java | 4 +- .../internal/senders/HttpSenderTest.java | 1 + 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/jaeger-core/src/main/java/io/jaegertracing/Configuration.java b/jaeger-core/src/main/java/io/jaegertracing/Configuration.java index a9e64711a..45166fa75 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/Configuration.java +++ b/jaeger-core/src/main/java/io/jaegertracing/Configuration.java @@ -156,7 +156,7 @@ public class Configuration { * TLS certificates (in comma-separated BASE64 SHA256 Hash) for certificates pinning, * used in case of HTTPS communication to the endpoint. */ - public static final String JAEGER_TLS_CERTIFICATES = JAEGER_PREFIX + "TLS_CERTIFICATES"; + public static final String JAEGER_TLS_CERTIFICATE_PINNING = JAEGER_PREFIX + "TLS_CERTIFICATE_PINNING"; /** * The supported trace context propagation formats. @@ -623,7 +623,7 @@ public static class SenderConfiguration { /** * The comma-separated certificate list for the endpoint which is self-signed, for example */ - private String certificates; + private String[] serverCertificateHashes; public SenderConfiguration() { } @@ -658,8 +658,10 @@ public SenderConfiguration withAuthPassword(String password) { return this; } - public SenderConfiguration withCertificate(String certs) { - this.certificates = certs; + public SenderConfiguration withCertificatePinning(String certs) { + if (certs != null) { + this.serverCertificateHashes = certs.split(","); + } return this; } @@ -687,7 +689,7 @@ public static SenderConfiguration fromEnv() { String authToken = getProperty(JAEGER_AUTH_TOKEN); String authUsername = getProperty(JAEGER_USER); String authPassword = getProperty(JAEGER_PASSWORD); - String certificates = getProperty(JAEGER_TLS_CERTIFICATES); + String certHashes = getProperty(JAEGER_TLS_CERTIFICATE_PINNING); return new SenderConfiguration() .withAgentHost(agentHost) @@ -695,7 +697,7 @@ public static SenderConfiguration fromEnv() { .withEndpoint(collectorEndpoint) .withAuthToken(authToken) .withAuthUsername(authUsername) - .withCertificate(certificates) + .withCertificatePinning(certHashes) .withAuthPassword(authPassword); } } diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 6a1f2528f..6843a617f 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -98,7 +98,8 @@ public void send(Process process, List spans) throws SenderException { public static class Builder { private final String endpoint; private CertificatePinner.Builder certificatePinnerBuilder = new CertificatePinner.Builder(); - private boolean pinning; + private boolean pinning = false; + private boolean disableVerification = false; private int maxPacketSize = ONE_MB_IN_BYTES; private Interceptor authInterceptor; private OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder(); @@ -130,15 +131,14 @@ public Builder withAuth(String authToken) { return this; } - public Builder withCertificatePinning(String sha256certs /* comma separated */) { + public Builder withCertificatePinning(String sha256certs[] /* comma separated */) { String hostname; try { final URI uri = new URI(endpoint); if (uri.getScheme() == "https") { hostname = uri.getHost(); this.pinning = true; - String certs[] = sha256certs.split(","); - for (String cert: certs) { + for (String cert: sha256certs) { certificatePinnerBuilder.add(hostname, String.format("sha256/%s", cert)); } } @@ -148,6 +148,25 @@ public Builder withCertificatePinning(String sha256certs /* comma separated */) } public Builder disableCertVerification() { + /* This dangerous operation will only take effect if pinning is used. */ + this.disableVerification = true; + return this; + } + + public HttpSender build() { + if (authInterceptor != null) { + clientBuilder.addInterceptor(authInterceptor); + } + if (pinning) { + clientBuilder.certificatePinner(certificatePinnerBuilder.build()); + if (disableVerification) { + disableCertVerification(clientBuilder); + } + } + return new HttpSender(this); + } + + private void disableCertVerification(OkHttpClient.Builder clientBuilder) { try { final TrustManager[] unsafeNoopVerificator = new TrustManager[] { new X509TrustManager() { @@ -165,17 +184,6 @@ public Builder disableCertVerification() { } catch (Exception e) { throw new RuntimeException(e); } - return this; - } - - public HttpSender build() { - if (authInterceptor != null) { - clientBuilder.addInterceptor(authInterceptor); - } - if (pinning) { - clientBuilder.certificatePinner(certificatePinnerBuilder.build()); - } - return new HttpSender(this); } private Interceptor getAuthInterceptor(final String headerValue) { diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactory.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactory.java index bf5038ede..083edf6f5 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactory.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/ThriftSenderFactory.java @@ -26,8 +26,8 @@ public Sender getSender(Configuration.SenderConfiguration conf) { httpSenderBuilder.withAuth(conf.getAuthToken()); } - if (null != conf.getCertificates() && !conf.getCertificates().isEmpty()) { - httpSenderBuilder.withCertificates(conf.getCertificates()); + if (null != conf.getServerCertificateHashes() && 0 != conf.getServerCertificateHashes().length) { + httpSenderBuilder.withCertificatePinning(conf.getServerCertificateHashes()); } log.debug("Using the HTTP Sender to send spans directly to the endpoint."); diff --git a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java index c02db1257..acbf5b69e 100644 --- a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java +++ b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java @@ -52,6 +52,7 @@ public void reset() { System.clearProperty(Configuration.JAEGER_AUTH_TOKEN); System.clearProperty(Configuration.JAEGER_USER); System.clearProperty(Configuration.JAEGER_PASSWORD); + System.clearProperty(Configuration.JAEGER_TLS_CERTIFICATE_PINNING); } @Override From 59b1b0f3849192c6aaf64b66500acb076ce8bd6a Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Thu, 7 Mar 2019 16:31:00 +0900 Subject: [PATCH 05/25] xdock test Signed-off-by: Iori Yoneji --- jaeger-crossdock/docker-compose.yml | 27 +++++++++++++++++-- jaeger-crossdock/https-proxy/gen.sh | 24 +++++++++++++++++ .../https-proxy/proxy.template.conf | 12 +++++++++ jaeger-crossdock/rules.mk | 12 ++++++--- .../jaegertracing/crossdock/JerseyServer.java | 8 ++++-- 5 files changed, 75 insertions(+), 8 deletions(-) create mode 100755 jaeger-crossdock/https-proxy/gen.sh create mode 100644 jaeger-crossdock/https-proxy/proxy.template.conf diff --git a/jaeger-crossdock/docker-compose.yml b/jaeger-crossdock/docker-compose.yml index 3c73d9519..e02292195 100644 --- a/jaeger-crossdock/docker-compose.yml +++ b/jaeger-crossdock/docker-compose.yml @@ -8,10 +8,11 @@ services: - go - java-udp - java-http + - java-https - python - nodejs environment: - - WAIT_FOR=test_driver,go,java-udp,java-http,python,nodejs + - WAIT_FOR=test_driver,go,java-udp,java-http,java-https,python,nodejs - WAIT_FOR_TIMEOUT=60s - CALL_TIMEOUT=60s @@ -27,7 +28,7 @@ services: - BEHAVIOR_TRACE=client,s1name,sampled,s2name,s2transport,s3name,s3transport - AXIS_TESTDRIVER=test_driver - - AXIS_SERVICES=java-udp,java-http + - AXIS_SERVICES=java-udp,java-http,java-https - BEHAVIOR_ENDTOEND=testdriver,services @@ -64,11 +65,33 @@ services: environment: - SENDER=http + java-https: + build: . + ports: + - "8080-8082" + environment: + - SENDER=https + depends_on: + - jaeger-collector + + jaeger-collector-https-proxy: + image: nginx + ports: + - "14443:14443" + volumes: + - ./https-proxy/build/proxy.conf:/etc/nginx/conf.d/proxy.conf + - ./https-proxy/build/proxy.crt:/etc/nginx/proxy.crt + - ./https-proxy/build/proxy.key:/etc/nginx/proxy.key + depends_on: + - jaeger-collector + command: ["nginx", "-g", "daemon off;"] + test_driver: image: jaegertracing/test-driver depends_on: - jaeger-query - jaeger-collector - jaeger-agent + - jaeger-collector-https-proxy ports: - "8080" diff --git a/jaeger-crossdock/https-proxy/gen.sh b/jaeger-crossdock/https-proxy/gen.sh new file mode 100755 index 000000000..249399bc1 --- /dev/null +++ b/jaeger-crossdock/https-proxy/gen.sh @@ -0,0 +1,24 @@ +#!/usr/bin/env bash + +set -e + +if [[ $# -ne 2 ]]; then + echo usage: $0 ' ' + exit 1 +fi + +SERVER=$1 +FORWARD=$2 +TARGET=build/proxy + +# generate secrets +cd $(dirname $0) +mkdir -p ./build +openssl ecparam -genkey -name prime256v1 -out ${TARGET}.key +openssl req -new -sha256 -key ${TARGET}.key -out ${TARGET}.csr -subj \ + "/CN=${SERVER}/" +openssl req -x509 -sha256 -days 1 -key ${TARGET}.key -in ${TARGET}.csr -out ${TARGET}.crt +chmod 644 ${TARGET}.key + +# generate nginx settings +SERVER=${SERVER} FORWARD=${FORWARD} envsubst < "proxy.template.conf" > build/proxy.conf diff --git a/jaeger-crossdock/https-proxy/proxy.template.conf b/jaeger-crossdock/https-proxy/proxy.template.conf new file mode 100644 index 000000000..8e999876e --- /dev/null +++ b/jaeger-crossdock/https-proxy/proxy.template.conf @@ -0,0 +1,12 @@ +server { + listen 14443 ssl; + server_name ${SERVER} + + ssl_protocols TLSv1 TLSv1.1 TLSv1.2; + ssl_certificate /etc/nginx/proxy.crt; + ssl_certificate_key /etc/nginx/proxy.key; + ssl_prefer_server_ciphers on; + location / { + proxy_pass http://${FORWARD}; + } +} diff --git a/jaeger-crossdock/rules.mk b/jaeger-crossdock/rules.mk index a6c083717..75202f508 100644 --- a/jaeger-crossdock/rules.mk +++ b/jaeger-crossdock/rules.mk @@ -5,10 +5,10 @@ JAEGER_COMPOSE_URL=https://raw.githubusercontent.com/jaegertracing/jaeger/master XDOCK_JAEGER_YAML=$(PROJECT)/jaeger-docker-compose.yml .PHONY: crossdock -crossdock: gradle-compile crossdock-download-jaeger - docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) kill java-udp java-http - docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) rm -f java-udp java-http - docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) build java-udp java-http +crossdock: gradle-compile proxy-secret-gen crossdock-download-jaeger + docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) kill java-udp java-http java-https + docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) rm -f java-udp java-http java-https + docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) build java-udp java-http java-https docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) run crossdock .PHONY: crossdock-fresh @@ -30,3 +30,7 @@ crossdock-clean: .PHONY: crossdock-download-jaeger crossdock-download-jaeger: curl -o $(XDOCK_JAEGER_YAML) $(JAEGER_COMPOSE_URL) + +.PHONY: proxy-secret-gen +proxy-secret-gen: + $(PROJECT)/https-proxy/gen.sh jaeger-collector-https-proxy jaeger-collector:14268 diff --git a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java index d412e285d..0d824acdf 100644 --- a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java +++ b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java @@ -125,6 +125,7 @@ public static void main(String[] args) throws Exception { new EndToEndBehaviorResource(new EndToEndBehavior(getEvn(SAMPLING_HOST_PORT, "jaeger-agent:5778"), "crossdock-" + serviceName, senderFromEnv(getEvn(COLLECTOR_HOST_PORT, "jaeger-collector:14268"), + getEvn(COLLECTOR_HOST_PORT, "jaeger-collector-https-proxy:14443"), getEvn(AGENT_HOST, "jaeger-agent")))), new HealthResource())); @@ -141,17 +142,20 @@ private static String getEvn(String envName, String defaultValue) { return env; } - private static Sender senderFromEnv(String collectorHostPort, String agentHost) { + private static Sender senderFromEnv(String collectorHostPort, String collectorHttpsHostPort, String agentHost) { String senderEnvVar = System.getenv(Constants.ENV_PROP_SENDER_TYPE); if ("http".equalsIgnoreCase(senderEnvVar)) { return new HttpSender.Builder(String.format("http://%s/api/traces", collectorHostPort)) .build(); + } else if ("https".equalsIgnoreCase(senderEnvVar)) { + return new HttpSender.Builder(String.format("https://%s/api/traces", collectorHttpsHostPort)) + .build(); } else if ("udp".equalsIgnoreCase(senderEnvVar) || senderEnvVar == null || senderEnvVar.isEmpty()) { return new UdpSender(agentHost, 0, 0); } throw new IllegalStateException("Env variable " + Constants.ENV_PROP_SENDER_TYPE - + ", is not valid, choose 'udp' or 'http'"); + + ", is not valid, choose 'udp', 'http' or 'https'"); } private static String serviceNameFromEnv() { From 9240b2226ddfbbd3315931b6292708748d3d0ba7 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Thu, 7 Mar 2019 23:18:09 +0900 Subject: [PATCH 06/25] allow disabling certificate verification for integration test Signed-off-by: Iori Yoneji --- .../io/jaegertracing/crossdock/JerseyServer.java | 1 + .../thrift/internal/senders/HttpSender.java | 13 ++----------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java index 0d824acdf..4453a2000 100644 --- a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java +++ b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java @@ -149,6 +149,7 @@ private static Sender senderFromEnv(String collectorHostPort, String collectorHt .build(); } else if ("https".equalsIgnoreCase(senderEnvVar)) { return new HttpSender.Builder(String.format("https://%s/api/traces", collectorHttpsHostPort)) + .disableCertVerification() .build(); } else if ("udp".equalsIgnoreCase(senderEnvVar) || senderEnvVar == null || senderEnvVar.isEmpty()) { return new UdpSender(agentHost, 0, 0); diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 6843a617f..d22678625 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -99,7 +99,6 @@ public static class Builder { private final String endpoint; private CertificatePinner.Builder certificatePinnerBuilder = new CertificatePinner.Builder(); private boolean pinning = false; - private boolean disableVerification = false; private int maxPacketSize = ONE_MB_IN_BYTES; private Interceptor authInterceptor; private OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder(); @@ -147,26 +146,17 @@ public Builder withCertificatePinning(String sha256certs[] /* comma separated */ } } - public Builder disableCertVerification() { - /* This dangerous operation will only take effect if pinning is used. */ - this.disableVerification = true; - return this; - } - public HttpSender build() { if (authInterceptor != null) { clientBuilder.addInterceptor(authInterceptor); } if (pinning) { clientBuilder.certificatePinner(certificatePinnerBuilder.build()); - if (disableVerification) { - disableCertVerification(clientBuilder); - } } return new HttpSender(this); } - private void disableCertVerification(OkHttpClient.Builder clientBuilder) { + public Builder disableCertVerification() { try { final TrustManager[] unsafeNoopVerificator = new TrustManager[] { new X509TrustManager() { @@ -184,6 +174,7 @@ private void disableCertVerification(OkHttpClient.Builder clientBuilder) { } catch (Exception e) { throw new RuntimeException(e); } + return this; } private Interceptor getAuthInterceptor(final String headerValue) { From 81b429aac2f276737cb70c029cf70a9e12bf7034 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Thu, 7 Mar 2019 23:50:26 +0900 Subject: [PATCH 07/25] add test-purpose CA key and enforce "verification-disabling" feature with Pinning Signed-off-by: Iori Yoneji --- jaeger-crossdock/docker-compose.yml | 1 + jaeger-crossdock/https-proxy/authority.crt | 10 ++++++++++ jaeger-crossdock/https-proxy/authority.key | 8 ++++++++ jaeger-crossdock/https-proxy/gen.sh | 2 +- .../io/jaegertracing/crossdock/JerseyServer.java | 5 ++++- .../thrift/internal/senders/HttpSender.java | 15 ++++++++++++--- 6 files changed, 36 insertions(+), 5 deletions(-) create mode 100644 jaeger-crossdock/https-proxy/authority.crt create mode 100644 jaeger-crossdock/https-proxy/authority.key diff --git a/jaeger-crossdock/docker-compose.yml b/jaeger-crossdock/docker-compose.yml index e02292195..572a9e833 100644 --- a/jaeger-crossdock/docker-compose.yml +++ b/jaeger-crossdock/docker-compose.yml @@ -71,6 +71,7 @@ services: - "8080-8082" environment: - SENDER=https + - COLLETCOR_HOST_PIN="sha256/n6Ovey/sJws9vKJpESmWyQf9Oocak9J51mmPKGm4S0E=" depends_on: - jaeger-collector diff --git a/jaeger-crossdock/https-proxy/authority.crt b/jaeger-crossdock/https-proxy/authority.crt new file mode 100644 index 000000000..18991aabe --- /dev/null +++ b/jaeger-crossdock/https-proxy/authority.crt @@ -0,0 +1,10 @@ +-----BEGIN CERTIFICATE----- +MIIBbTCCARSgAwIBAgIJAJXCC2VdhrxqMAoGCCqGSM49BAMCMBIxEDAOBgNVBAMM +B1RFU1QgQ0EwHhcNMTkwMzA3MTQzODIwWhcNMjkwMzA3MTQzODIwWjASMRAwDgYD +VQQDDAdURVNUIENBMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEAhYnw9zYU0G3 +VZ48nNlT5jAs096pX0zeHM/yxiJe+DS5Yj0EJXM/0A1Of2zqxbyJpaEIFcqTmTTy +TXCE7I6B6aNTMFEwHQYDVR0OBBYEFLXSxii6ZFvw427zs7ct/B+eHv2QMB8GA1Ud +IwQYMBaAFLXSxii6ZFvw427zs7ct/B+eHv2QMA8GA1UdEwEB/wQFMAMBAf8wCgYI +KoZIzj0EAwIDRwAwRAIgBrX7CX8zNoRLAZ48jGcqI8RuNlpkj0S+UShIQjwez3AC +IGuqhnGb9JZSiZmIQaYdSE6T/sQaX7iDnwEgKGMI8OB7 +-----END CERTIFICATE----- diff --git a/jaeger-crossdock/https-proxy/authority.key b/jaeger-crossdock/https-proxy/authority.key new file mode 100644 index 000000000..a30db3f38 --- /dev/null +++ b/jaeger-crossdock/https-proxy/authority.key @@ -0,0 +1,8 @@ +-----BEGIN EC PARAMETERS----- +BggqhkjOPQMBBw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIAA24MB8sxrLG1an0nG1DCH6J32iqrtborxFOjdqWNCmoAoGCCqGSM49 +AwEHoUQDQgAEAhYnw9zYU0G3VZ48nNlT5jAs096pX0zeHM/yxiJe+DS5Yj0EJXM/ +0A1Of2zqxbyJpaEIFcqTmTTyTXCE7I6B6Q== +-----END EC PRIVATE KEY----- diff --git a/jaeger-crossdock/https-proxy/gen.sh b/jaeger-crossdock/https-proxy/gen.sh index 249399bc1..11b1682f3 100755 --- a/jaeger-crossdock/https-proxy/gen.sh +++ b/jaeger-crossdock/https-proxy/gen.sh @@ -17,7 +17,7 @@ mkdir -p ./build openssl ecparam -genkey -name prime256v1 -out ${TARGET}.key openssl req -new -sha256 -key ${TARGET}.key -out ${TARGET}.csr -subj \ "/CN=${SERVER}/" -openssl req -x509 -sha256 -days 1 -key ${TARGET}.key -in ${TARGET}.csr -out ${TARGET}.crt +openssl x509 -req -sha256 -days 1 -CA authority.crt -CAkey authority.key -CAcreateserial -in ${TARGET}.csr -out ${TARGET}.crt chmod 644 ${TARGET}.key # generate nginx settings diff --git a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java index 4453a2000..8a2751bd8 100644 --- a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java +++ b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java @@ -53,6 +53,7 @@ public class JerseyServer { private static final String SAMPLING_HOST_PORT = "SAMPLING_HOST_PORT"; private static final String AGENT_HOST = "AGENT_HOST"; private static final String COLLECTOR_HOST_PORT = "COLLECTOR_HOST_PORT"; + private static final String COLLECTOR_HOST_PIN = "COLLECTOR_HOST_PIN"; // TODO should not be static, should be final public static Client client; @@ -126,6 +127,7 @@ public static void main(String[] args) throws Exception { "crossdock-" + serviceName, senderFromEnv(getEvn(COLLECTOR_HOST_PORT, "jaeger-collector:14268"), getEvn(COLLECTOR_HOST_PORT, "jaeger-collector-https-proxy:14443"), + getEvn(COLLECTOR_HOST_PIN, "sha256/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA="), getEvn(AGENT_HOST, "jaeger-agent")))), new HealthResource())); @@ -142,13 +144,14 @@ private static String getEvn(String envName, String defaultValue) { return env; } - private static Sender senderFromEnv(String collectorHostPort, String collectorHttpsHostPort, String agentHost) { + private static Sender senderFromEnv(String collectorHostPort, String collectorHttpsHostPort, String collectorHttpsPin, String agentHost) { String senderEnvVar = System.getenv(Constants.ENV_PROP_SENDER_TYPE); if ("http".equalsIgnoreCase(senderEnvVar)) { return new HttpSender.Builder(String.format("http://%s/api/traces", collectorHostPort)) .build(); } else if ("https".equalsIgnoreCase(senderEnvVar)) { return new HttpSender.Builder(String.format("https://%s/api/traces", collectorHttpsHostPort)) + .withCertificatePinning(new String[] {collectorHttpsPin}) .disableCertVerification() .build(); } else if ("udp".equalsIgnoreCase(senderEnvVar) || senderEnvVar == null || senderEnvVar.isEmpty()) { diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index d22678625..72367dd0d 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -98,7 +98,8 @@ public void send(Process process, List spans) throws SenderException { public static class Builder { private final String endpoint; private CertificatePinner.Builder certificatePinnerBuilder = new CertificatePinner.Builder(); - private boolean pinning = false; + private boolean pinning; + private boolean disableVerification = false; private int maxPacketSize = ONE_MB_IN_BYTES; private Interceptor authInterceptor; private OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder(); @@ -146,17 +147,26 @@ public Builder withCertificatePinning(String sha256certs[] /* comma separated */ } } + public Builder disableCertVerification() { + /* This dangerous operation will only take effect if pinning is used. */ + this.disableVerification = true; + return this; + } + public HttpSender build() { if (authInterceptor != null) { clientBuilder.addInterceptor(authInterceptor); } if (pinning) { clientBuilder.certificatePinner(certificatePinnerBuilder.build()); + if (disableVerification) { + disableCertVerification(clientBuilder); + } } return new HttpSender(this); } - public Builder disableCertVerification() { + private void disableCertVerification(OkHttpClient.Builder clientBuilder) { try { final TrustManager[] unsafeNoopVerificator = new TrustManager[] { new X509TrustManager() { @@ -174,7 +184,6 @@ public Builder disableCertVerification() { } catch (Exception e) { throw new RuntimeException(e); } - return this; } private Interceptor getAuthInterceptor(final String headerValue) { From 264db825de3b65d54a921b9d040c00933d89f2ef Mon Sep 17 00:00:00 2001 From: Iori YONEJI Date: Fri, 8 Mar 2019 01:15:25 +0900 Subject: [PATCH 08/25] fix crossdock checkstyle failure Signed-off-by: Iori YONEJI --- .../src/main/java/io/jaegertracing/crossdock/JerseyServer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java index 8a2751bd8..57ce314ad 100644 --- a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java +++ b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java @@ -144,7 +144,8 @@ private static String getEvn(String envName, String defaultValue) { return env; } - private static Sender senderFromEnv(String collectorHostPort, String collectorHttpsHostPort, String collectorHttpsPin, String agentHost) { + private static Sender senderFromEnv(String collectorHostPort, + String collectorHttpsHostPort, String collectorHttpsPin, String agentHost) { String senderEnvVar = System.getenv(Constants.ENV_PROP_SENDER_TYPE); if ("http".equalsIgnoreCase(senderEnvVar)) { return new HttpSender.Builder(String.format("http://%s/api/traces", collectorHostPort)) From d5127ff1f13dc957ab5bea9c55804e404982f322 Mon Sep 17 00:00:00 2001 From: Iori YONEJI Date: Fri, 8 Mar 2019 02:39:53 +0900 Subject: [PATCH 09/25] re-allow to disable certificate verification Signed-off-by: Iori YONEJI --- jaeger-crossdock/docker-compose.yml | 1 + jaeger-crossdock/https-proxy/proxy.template.conf | 3 +-- jaeger-crossdock/rules.mk | 6 +++--- .../jaegertracing/thrift/internal/senders/HttpSender.java | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/jaeger-crossdock/docker-compose.yml b/jaeger-crossdock/docker-compose.yml index 572a9e833..9f5de076c 100644 --- a/jaeger-crossdock/docker-compose.yml +++ b/jaeger-crossdock/docker-compose.yml @@ -83,6 +83,7 @@ services: - ./https-proxy/build/proxy.conf:/etc/nginx/conf.d/proxy.conf - ./https-proxy/build/proxy.crt:/etc/nginx/proxy.crt - ./https-proxy/build/proxy.key:/etc/nginx/proxy.key + restart: on-failure depends_on: - jaeger-collector command: ["nginx", "-g", "daemon off;"] diff --git a/jaeger-crossdock/https-proxy/proxy.template.conf b/jaeger-crossdock/https-proxy/proxy.template.conf index 8e999876e..02178ef1a 100644 --- a/jaeger-crossdock/https-proxy/proxy.template.conf +++ b/jaeger-crossdock/https-proxy/proxy.template.conf @@ -1,11 +1,10 @@ server { listen 14443 ssl; - server_name ${SERVER} + server_name ${SERVER}; ssl_protocols TLSv1 TLSv1.1 TLSv1.2; ssl_certificate /etc/nginx/proxy.crt; ssl_certificate_key /etc/nginx/proxy.key; - ssl_prefer_server_ciphers on; location / { proxy_pass http://${FORWARD}; } diff --git a/jaeger-crossdock/rules.mk b/jaeger-crossdock/rules.mk index 75202f508..db319f7cc 100644 --- a/jaeger-crossdock/rules.mk +++ b/jaeger-crossdock/rules.mk @@ -5,7 +5,7 @@ JAEGER_COMPOSE_URL=https://raw.githubusercontent.com/jaegertracing/jaeger/master XDOCK_JAEGER_YAML=$(PROJECT)/jaeger-docker-compose.yml .PHONY: crossdock -crossdock: gradle-compile proxy-secret-gen crossdock-download-jaeger +crossdock: gradle-compile crossdock-proxy-secret-gen crossdock-download-jaeger docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) kill java-udp java-http java-https docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) rm -f java-udp java-http java-https docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) build java-udp java-http java-https @@ -31,6 +31,6 @@ crossdock-clean: crossdock-download-jaeger: curl -o $(XDOCK_JAEGER_YAML) $(JAEGER_COMPOSE_URL) -.PHONY: proxy-secret-gen -proxy-secret-gen: +.PHONY: crossdock-proxy-secret-gen +crossdock-proxy-secret-gen: $(PROJECT)/https-proxy/gen.sh jaeger-collector-https-proxy jaeger-collector:14268 diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 72367dd0d..532998d70 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -159,9 +159,9 @@ public HttpSender build() { } if (pinning) { clientBuilder.certificatePinner(certificatePinnerBuilder.build()); - if (disableVerification) { - disableCertVerification(clientBuilder); - } + } + if (disableVerification) { + disableCertVerification(clientBuilder); } return new HttpSender(this); } From aec7e1ea6176ca4cc35c1b56651393203b568297 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Mon, 11 Mar 2019 17:01:57 +0900 Subject: [PATCH 10/25] fix two simple typoes involving with CertPinning configuration Signed-off-by: Iori Yoneji --- jaeger-crossdock/docker-compose.yml | 2 +- .../io/jaegertracing/thrift/internal/senders/HttpSender.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jaeger-crossdock/docker-compose.yml b/jaeger-crossdock/docker-compose.yml index 9f5de076c..06b63f7cf 100644 --- a/jaeger-crossdock/docker-compose.yml +++ b/jaeger-crossdock/docker-compose.yml @@ -71,7 +71,7 @@ services: - "8080-8082" environment: - SENDER=https - - COLLETCOR_HOST_PIN="sha256/n6Ovey/sJws9vKJpESmWyQf9Oocak9J51mmPKGm4S0E=" + - COLLECTOR_HOST_PIN="sha256/n6Ovey/sJws9vKJpESmWyQf9Oocak9J51mmPKGm4S0E=" depends_on: - jaeger-collector diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 532998d70..6fcf1218f 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -135,7 +135,7 @@ public Builder withCertificatePinning(String sha256certs[] /* comma separated */ String hostname; try { final URI uri = new URI(endpoint); - if (uri.getScheme() == "https") { + if ("https".equals(uri.getScheme())) { hostname = uri.getHost(); this.pinning = true; for (String cert: sha256certs) { From 5e9e6e13d9bef6f02261d06ccbbeac01d1284dbd Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Mon, 11 Mar 2019 19:20:01 +0900 Subject: [PATCH 11/25] restrict and rename relaxation option of HTTPS cert verification in HTTPSender Signed-off-by: Iori Yoneji --- jaeger-crossdock/docker-compose.yml | 2 +- .../jaegertracing/crossdock/JerseyServer.java | 2 +- .../thrift/internal/senders/HttpSender.java | 72 +++++++++++++------ 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/jaeger-crossdock/docker-compose.yml b/jaeger-crossdock/docker-compose.yml index 06b63f7cf..848e5ca69 100644 --- a/jaeger-crossdock/docker-compose.yml +++ b/jaeger-crossdock/docker-compose.yml @@ -71,7 +71,7 @@ services: - "8080-8082" environment: - SENDER=https - - COLLECTOR_HOST_PIN="sha256/n6Ovey/sJws9vKJpESmWyQf9Oocak9J51mmPKGm4S0E=" + - COLLECTOR_HOST_PIN=sha256/9I2vVk9bdSZF5Oy0AfvTDpOYhGMB9jc4RwyL4vWjix0= depends_on: - jaeger-collector diff --git a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java index 57ce314ad..96a33a85b 100644 --- a/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java +++ b/jaeger-crossdock/src/main/java/io/jaegertracing/crossdock/JerseyServer.java @@ -153,7 +153,7 @@ private static Sender senderFromEnv(String collectorHostPort, } else if ("https".equalsIgnoreCase(senderEnvVar)) { return new HttpSender.Builder(String.format("https://%s/api/traces", collectorHttpsHostPort)) .withCertificatePinning(new String[] {collectorHttpsPin}) - .disableCertVerification() + .acceptSelfSigned() .build(); } else if ("udp".equalsIgnoreCase(senderEnvVar) || senderEnvVar == null || senderEnvVar.isEmpty()) { return new UdpSender(agentHost, 0, 0); diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 6fcf1218f..0a48f7161 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -75,6 +75,8 @@ public void send(Process process, List spans) throws SenderException { try { response = httpClient.newCall(request).execute(); } catch (IOException e) { + e.printStackTrace(); + System.out.println("Test:" + e.toString() + e.getMessage() + e.getCause()); throw new SenderException(String.format("Could not send %d spans", spans.size()), e, spans.size()); } @@ -92,23 +94,38 @@ public void send(Process process, List spans) throws SenderException { String exceptionMessage = String.format("Could not send %d spans, response %d: %s", spans.size(), response.code(), responseBody); + throw new SenderException(exceptionMessage, null, spans.size()); } public static class Builder { private final String endpoint; private CertificatePinner.Builder certificatePinnerBuilder = new CertificatePinner.Builder(); - private boolean pinning; - private boolean disableVerification = false; private int maxPacketSize = ONE_MB_IN_BYTES; private Interceptor authInterceptor; private OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder(); + private boolean pinning; + private boolean acceptSelfSigned = false; + private final String hostname; /** * @param endpoint jaeger-collector HTTP endpoint e.g. http://localhost:14268/api/traces */ public Builder(String endpoint) { this.endpoint = endpoint; + String hostname = ""; + try { + // Just obtain hostname for SSL feature. Failure is OK if plain http is used. + final URI uri = new URI(endpoint); + if ("https".equals(uri.getScheme())) { + hostname = uri.getHost(); + } + } catch (Exception e) { + // Won't validate the endpoint name. + // True moment comes at Sender's Build. + } finally { + this.hostname = hostname; + } } public Builder withClient(OkHttpClient client) { @@ -132,24 +149,19 @@ public Builder withAuth(String authToken) { } public Builder withCertificatePinning(String sha256certs[] /* comma separated */) { - String hostname; - try { - final URI uri = new URI(endpoint); - if ("https".equals(uri.getScheme())) { - hostname = uri.getHost(); - this.pinning = true; - for (String cert: sha256certs) { - certificatePinnerBuilder.add(hostname, String.format("sha256/%s", cert)); - } + if (!"".equals(hostname)) { + this.pinning = true; + for (String cert: sha256certs) { + certificatePinnerBuilder.add(hostname, String.format("%s", cert)); + System.out.println(String.format("hostname: %s, cert: %s", hostname, cert)); } - } finally { - return this; } + return this; } - public Builder disableCertVerification() { + public Builder acceptSelfSigned() { /* This dangerous operation will only take effect if pinning is used. */ - this.disableVerification = true; + this.acceptSelfSigned = true; return this; } @@ -160,22 +172,36 @@ public HttpSender build() { if (pinning) { clientBuilder.certificatePinner(certificatePinnerBuilder.build()); } - if (disableVerification) { - disableCertVerification(clientBuilder); + if (acceptSelfSigned) { + acceptSelfSigned(clientBuilder, hostname); } return new HttpSender(this); } - private void disableCertVerification(OkHttpClient.Builder clientBuilder) { + private void acceptSelfSigned(OkHttpClient.Builder clientBuilder, final String hostname) { try { final TrustManager[] unsafeNoopVerificator = new TrustManager[] { - new X509TrustManager() { - @Override public void checkServerTrusted(java.security.cert.X509Certificate[] chain, String authType) throws CertificateException {} - @Override public void checkClientTrusted(java.security.cert.X509Certificate[] chain, String authType) throws CertificateException {} - @Override public java.security.cert.X509Certificate[] getAcceptedIssuers() { - return new X509Certificate[0]; + new X509TrustManager() { + private final String subjectCN = "CN=" + hostname; + @Override public void checkServerTrusted(java.security.cert.X509Certificate[] chain, String authType) throws CertificateException { + for (java.security.cert.X509Certificate cert: chain) { + System.out.println("pin:" + CertificatePinner.pin(cert)); + String[] subject = cert.getSubjectDN().getName().split("/"); + for (String name: subject) { + if (subjectCN.equals(name)) { + return; + } } + } + throw new CertificateException(); + } + @Override public void checkClientTrusted(java.security.cert.X509Certificate[] chain, String authType) throws CertificateException { + throw new CertificateException(); // Nothing will be accepted + } + @Override public java.security.cert.X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; } + } }; final SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(null, unsafeNoopVerificator, null); From cdd4c79b329902253ae7799674eb814a2b3254c5 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Mon, 11 Mar 2019 20:05:53 +0900 Subject: [PATCH 12/25] remove debug code with HTTPS from HTTPSender Signed-off-by: Iori Yoneji --- .../io/jaegertracing/thrift/internal/senders/HttpSender.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 0a48f7161..ac3dbdc7c 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -76,7 +76,6 @@ public void send(Process process, List spans) throws SenderException { response = httpClient.newCall(request).execute(); } catch (IOException e) { e.printStackTrace(); - System.out.println("Test:" + e.toString() + e.getMessage() + e.getCause()); throw new SenderException(String.format("Could not send %d spans", spans.size()), e, spans.size()); } @@ -153,7 +152,6 @@ public Builder withCertificatePinning(String sha256certs[] /* comma separated */ this.pinning = true; for (String cert: sha256certs) { certificatePinnerBuilder.add(hostname, String.format("%s", cert)); - System.out.println(String.format("hostname: %s, cert: %s", hostname, cert)); } } return this; @@ -185,7 +183,6 @@ private void acceptSelfSigned(OkHttpClient.Builder clientBuilder, final String h private final String subjectCN = "CN=" + hostname; @Override public void checkServerTrusted(java.security.cert.X509Certificate[] chain, String authType) throws CertificateException { for (java.security.cert.X509Certificate cert: chain) { - System.out.println("pin:" + CertificatePinner.pin(cert)); String[] subject = cert.getSubjectDN().getName().split("/"); for (String name: subject) { if (subjectCN.equals(name)) { From 9892e8d38ce73d8c0824f4dda497d8514a79e657 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Mon, 11 Mar 2019 22:23:54 +0900 Subject: [PATCH 13/25] add hand-check pinning verifier for self-signed certification without any trust anchor Signed-off-by: Iori Yoneji --- jaeger-crossdock/docker-compose.yml | 2 +- jaeger-crossdock/https-proxy/gen.sh | 4 +- .../thrift/internal/senders/HttpSender.java | 63 ++++++++++++------- 3 files changed, 46 insertions(+), 23 deletions(-) diff --git a/jaeger-crossdock/docker-compose.yml b/jaeger-crossdock/docker-compose.yml index 848e5ca69..b63ff624b 100644 --- a/jaeger-crossdock/docker-compose.yml +++ b/jaeger-crossdock/docker-compose.yml @@ -71,7 +71,7 @@ services: - "8080-8082" environment: - SENDER=https - - COLLECTOR_HOST_PIN=sha256/9I2vVk9bdSZF5Oy0AfvTDpOYhGMB9jc4RwyL4vWjix0= + - COLLECTOR_HOST_PIN=sha256/n6Ovey/sJws9vKJpESmWyQf9Oocak9J51mmPKGm4S0E= depends_on: - jaeger-collector diff --git a/jaeger-crossdock/https-proxy/gen.sh b/jaeger-crossdock/https-proxy/gen.sh index 11b1682f3..a02ac40e3 100755 --- a/jaeger-crossdock/https-proxy/gen.sh +++ b/jaeger-crossdock/https-proxy/gen.sh @@ -20,5 +20,7 @@ openssl req -new -sha256 -key ${TARGET}.key -out ${TARGET}.csr -subj \ openssl x509 -req -sha256 -days 1 -CA authority.crt -CAkey authority.key -CAcreateserial -in ${TARGET}.csr -out ${TARGET}.crt chmod 644 ${TARGET}.key +cat authority.crt >> ${TARGET}.crt + # generate nginx settings -SERVER=${SERVER} FORWARD=${FORWARD} envsubst < "proxy.template.conf" > build/proxy.conf +SERVER=${SERVER} FORWARD=${FORWARD} envsubst < "proxy.template.conf" > ${TARGET}.conf diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index ac3dbdc7c..3437dd4aa 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -20,6 +20,8 @@ import io.jaegertracing.thriftjava.Span; import java.io.IOException; import java.net.URI; +import java.util.Arrays; +import java.util.ArrayList; import java.util.List; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; @@ -103,8 +105,8 @@ public static class Builder { private int maxPacketSize = ONE_MB_IN_BYTES; private Interceptor authInterceptor; private OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder(); - private boolean pinning; - private boolean acceptSelfSigned = false; + private List pins = new ArrayList(); + private boolean selfSigned = false; private final String hostname; /** @@ -147,19 +149,14 @@ public Builder withAuth(String authToken) { return this; } - public Builder withCertificatePinning(String sha256certs[] /* comma separated */) { - if (!"".equals(hostname)) { - this.pinning = true; - for (String cert: sha256certs) { - certificatePinnerBuilder.add(hostname, String.format("%s", cert)); - } - } + public Builder withCertificatePinning(String[] sha256certs /* comma separated */) { + pins.addAll(Arrays.asList(sha256certs)); return this; } public Builder acceptSelfSigned() { - /* This dangerous operation will only take effect if pinning is used. */ - this.acceptSelfSigned = true; + // This dangerous operation will only take effect if pinning is used. + this.selfSigned = true; return this; } @@ -167,35 +164,59 @@ public HttpSender build() { if (authInterceptor != null) { clientBuilder.addInterceptor(authInterceptor); } - if (pinning) { + if (!selfSigned && !pins.isEmpty()) { + // Pinning Certificate issued by public CA + for (String cert: pins) { + certificatePinnerBuilder.add(hostname, String.format("%s", cert)); + } clientBuilder.certificatePinner(certificatePinnerBuilder.build()); - } - if (acceptSelfSigned) { - acceptSelfSigned(clientBuilder, hostname); + } else if (selfSigned && !pins.isEmpty()) { + /* Issued by private CA, OkHttp's pinner is unable to verify the pins. + * Instead, check the sha256 hash value by hand. */ + acceptSelfSigned(clientBuilder, hostname, pins); } return new HttpSender(this); } - private void acceptSelfSigned(OkHttpClient.Builder clientBuilder, final String hostname) { + private void acceptSelfSigned(OkHttpClient.Builder clientBuilder, final String hostname, final List pinlist) { try { final TrustManager[] unsafeNoopVerificator = new TrustManager[] { new X509TrustManager() { private final String subjectCN = "CN=" + hostname; - @Override public void checkServerTrusted(java.security.cert.X509Certificate[] chain, String authType) throws CertificateException { - for (java.security.cert.X509Certificate cert: chain) { + private final List pins = pinlist; + + private boolean check(X509Certificate[] chain) { + // Intersection of pins and every cert in this chain + for (X509Certificate cert: chain) { + String hash = CertificatePinner.pin(cert); + for (String pin: pins) { + if (hash.equals(pin)) { + return true; + } + } + } + return false; + } + + @Override public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { + for (X509Certificate cert: chain) { String[] subject = cert.getSubjectDN().getName().split("/"); for (String name: subject) { if (subjectCN.equals(name)) { - return; + // Found endpoint's hostname. This chain is going to be tested. + if (check(chain)) { + return; + } } } } + // TODO: For TOFU (trust on first use) usecase, print the chain throw new CertificateException(); } - @Override public void checkClientTrusted(java.security.cert.X509Certificate[] chain, String authType) throws CertificateException { + @Override public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { throw new CertificateException(); // Nothing will be accepted } - @Override public java.security.cert.X509Certificate[] getAcceptedIssuers() { + @Override public X509Certificate[] getAcceptedIssuers() { return new X509Certificate[0]; } } From 9479bb3cc8b5cb0364f8db19e202b012a57aa6f0 Mon Sep 17 00:00:00 2001 From: Iori YONEJI Date: Wed, 13 Mar 2019 02:01:29 +0900 Subject: [PATCH 14/25] crossdock fix (wait for nginx proxy that waits for collector) Signed-off-by: Iori YONEJI --- jaeger-crossdock/docker-compose.yml | 7 ++++--- jaeger-crossdock/https-proxy/proxy.template.conf | 9 +++++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/jaeger-crossdock/docker-compose.yml b/jaeger-crossdock/docker-compose.yml index b63ff624b..6259ec581 100644 --- a/jaeger-crossdock/docker-compose.yml +++ b/jaeger-crossdock/docker-compose.yml @@ -12,7 +12,7 @@ services: - python - nodejs environment: - - WAIT_FOR=test_driver,go,java-udp,java-http,java-https,python,nodejs + - WAIT_FOR=test_driver,go,java-udp,java-http,java-https,python,nodejs,jaeger-collector-https-proxy - WAIT_FOR_TIMEOUT=60s - CALL_TIMEOUT=60s @@ -76,8 +76,9 @@ services: - jaeger-collector jaeger-collector-https-proxy: - image: nginx + image: nginx:alpine ports: + - "8080:8080" - "14443:14443" volumes: - ./https-proxy/build/proxy.conf:/etc/nginx/conf.d/proxy.conf @@ -86,7 +87,7 @@ services: restart: on-failure depends_on: - jaeger-collector - command: ["nginx", "-g", "daemon off;"] + command: ["sh", "-c", "while ! { wget --spider -S http://jaeger-collector:14269; }; do echo waiting...; sleep 10; done; nginx -g 'daemon off;'"] test_driver: image: jaegertracing/test-driver diff --git a/jaeger-crossdock/https-proxy/proxy.template.conf b/jaeger-crossdock/https-proxy/proxy.template.conf index 02178ef1a..c5bdf4e19 100644 --- a/jaeger-crossdock/https-proxy/proxy.template.conf +++ b/jaeger-crossdock/https-proxy/proxy.template.conf @@ -1,3 +1,12 @@ +server { + listen 8080; + server_name ${SERVER}; + location / { + root /usr/share/nginx/html/; + index index.html; + } +} + server { listen 14443 ssl; server_name ${SERVER}; From 59342f12636efb4e692f5d16586853bee4f4aa5a Mon Sep 17 00:00:00 2001 From: Iori YONEJI Date: Wed, 13 Mar 2019 02:50:54 +0900 Subject: [PATCH 15/25] fix crossdock test avoiding ambiguous volume behavior Signed-off-by: Iori YONEJI --- jaeger-crossdock/docker-compose.yml | 11 +++-------- jaeger-crossdock/https-proxy/Dockerfile | 8 ++++++++ jaeger-crossdock/rules.mk | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) create mode 100644 jaeger-crossdock/https-proxy/Dockerfile diff --git a/jaeger-crossdock/docker-compose.yml b/jaeger-crossdock/docker-compose.yml index 6259ec581..4a30f951b 100644 --- a/jaeger-crossdock/docker-compose.yml +++ b/jaeger-crossdock/docker-compose.yml @@ -76,18 +76,13 @@ services: - jaeger-collector jaeger-collector-https-proxy: - image: nginx:alpine + build: https-proxy ports: - - "8080:8080" - - "14443:14443" - volumes: - - ./https-proxy/build/proxy.conf:/etc/nginx/conf.d/proxy.conf - - ./https-proxy/build/proxy.crt:/etc/nginx/proxy.crt - - ./https-proxy/build/proxy.key:/etc/nginx/proxy.key + - "8080" + - "14443" restart: on-failure depends_on: - jaeger-collector - command: ["sh", "-c", "while ! { wget --spider -S http://jaeger-collector:14269; }; do echo waiting...; sleep 10; done; nginx -g 'daemon off;'"] test_driver: image: jaegertracing/test-driver diff --git a/jaeger-crossdock/https-proxy/Dockerfile b/jaeger-crossdock/https-proxy/Dockerfile new file mode 100644 index 000000000..fbe2ae2bc --- /dev/null +++ b/jaeger-crossdock/https-proxy/Dockerfile @@ -0,0 +1,8 @@ +FROM nginx:alpine + +ADD build/proxy.conf /etc/nginx/conf.d/proxy.conf +ADD build/proxy.crt /etc/nginx/proxy.crt +ADD build/proxy.key /etc/nginx/proxy.key + +EXPOSE 8080 14443 +CMD ["sh", "-c", "while ! { wget --spider -S http://jaeger-collector:14269; }; do echo waiting...; sleep 1; done; nginx -g 'daemon off;'"] diff --git a/jaeger-crossdock/rules.mk b/jaeger-crossdock/rules.mk index db319f7cc..90bb6f3ea 100644 --- a/jaeger-crossdock/rules.mk +++ b/jaeger-crossdock/rules.mk @@ -12,7 +12,7 @@ crossdock: gradle-compile crossdock-proxy-secret-gen crossdock-download-jaeger docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) run crossdock .PHONY: crossdock-fresh -crossdock-fresh: gradle-compile crossdock-download-jaeger +crossdock-fresh: gradle-compile crossdock-proxy-secret-gen crossdock-download-jaeger docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) down --rmi all docker-compose -f $(XDOCK_YAML) -f $(XDOCK_JAEGER_YAML) run crossdock From da96565b5441d6317d43b356738cdd0cbfe5cf7e Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Wed, 13 Mar 2019 17:41:36 +0900 Subject: [PATCH 16/25] add test for HTTPSender's new feature (pinning) Signed-off-by: Iori Yoneji --- .../io/jaegertracing/ConfigurationTest.java | 1 + .../thrift/internal/senders/HttpSender.java | 47 +++++++++---------- .../internal/senders/HttpSenderTest.java | 11 +++++ 3 files changed, 33 insertions(+), 26 deletions(-) diff --git a/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java b/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java index 0d61541c1..71705fcd1 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/ConfigurationTest.java @@ -70,6 +70,7 @@ public void clearProperties() throws NoSuchFieldException, IllegalAccessExceptio System.clearProperty(Configuration.JAEGER_TAGS); System.clearProperty(Configuration.JAEGER_ENDPOINT); System.clearProperty(Configuration.JAEGER_AUTH_TOKEN); + System.clearProperty(Configuration.JAEGER_TLS_CERTIFICATE_PINNING); System.clearProperty(Configuration.JAEGER_USER); System.clearProperty(Configuration.JAEGER_PASSWORD); System.clearProperty(Configuration.JAEGER_PROPAGATION); diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 3437dd4aa..53d73f84a 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -107,26 +107,12 @@ public static class Builder { private OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder(); private List pins = new ArrayList(); private boolean selfSigned = false; - private final String hostname; /** * @param endpoint jaeger-collector HTTP endpoint e.g. http://localhost:14268/api/traces */ public Builder(String endpoint) { this.endpoint = endpoint; - String hostname = ""; - try { - // Just obtain hostname for SSL feature. Failure is OK if plain http is used. - final URI uri = new URI(endpoint); - if ("https".equals(uri.getScheme())) { - hostname = uri.getHost(); - } - } catch (Exception e) { - // Won't validate the endpoint name. - // True moment comes at Sender's Build. - } finally { - this.hostname = hostname; - } } public Builder withClient(OkHttpClient client) { @@ -164,23 +150,32 @@ public HttpSender build() { if (authInterceptor != null) { clientBuilder.addInterceptor(authInterceptor); } - if (!selfSigned && !pins.isEmpty()) { - // Pinning Certificate issued by public CA - for (String cert: pins) { - certificatePinnerBuilder.add(hostname, String.format("%s", cert)); + try { + // Just obtain hostname for SSL feature. Failure is OK if plain http is used. + final URI uri = new URI(endpoint); + String hostname = hostname = uri.getHost(); + + if (!selfSigned && !pins.isEmpty()) { + // Pinning Certificate issued by public CA + for (String cert: pins) { + certificatePinnerBuilder.add(hostname, String.format("%s", cert)); + } + clientBuilder.certificatePinner(certificatePinnerBuilder.build()); + } else if (selfSigned && !pins.isEmpty()) { + /* Issued by private CA, OkHttp's pinner is unable to verify the pins. + * Instead, check the sha256 hash value by custom verifier. */ + acceptSelfSigned(clientBuilder, hostname, pins); } - clientBuilder.certificatePinner(certificatePinnerBuilder.build()); - } else if (selfSigned && !pins.isEmpty()) { - /* Issued by private CA, OkHttp's pinner is unable to verify the pins. - * Instead, check the sha256 hash value by hand. */ - acceptSelfSigned(clientBuilder, hostname, pins); + } catch (Exception e) { + // Won't validate the endpoint name. + // True moment comes at Sender's Build. } return new HttpSender(this); } private void acceptSelfSigned(OkHttpClient.Builder clientBuilder, final String hostname, final List pinlist) { try { - final TrustManager[] unsafeNoopVerificator = new TrustManager[] { + final TrustManager[] selfSignedServerTrustManager = new TrustManager[] { new X509TrustManager() { private final String subjectCN = "CN=" + hostname; private final List pins = pinlist; @@ -222,8 +217,8 @@ private boolean check(X509Certificate[] chain) { } }; final SSLContext sslContext = SSLContext.getInstance("TLS"); - sslContext.init(null, unsafeNoopVerificator, null); - clientBuilder.sslSocketFactory(sslContext.getSocketFactory(), (X509TrustManager) unsafeNoopVerificator[0]); + sslContext.init(null, selfSignedServerTrustManager, null); + clientBuilder.sslSocketFactory(sslContext.getSocketFactory(), (X509TrustManager) selfSignedServerTrustManager[0]); } catch (Exception e) { throw new RuntimeException(e); diff --git a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java index acbf5b69e..bbbf5f0cf 100644 --- a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java +++ b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java @@ -125,6 +125,17 @@ public void sendWithTokenAuth() throws Exception { sender.send(new Process("robotrock"), generateSpans()); } + @Test + public void setTLSCertificatePinning() throws Exception { + System.setProperty(Configuration.JAEGER_ENDPOINT, target("/api/traces").getUri().toString()); + // Just confirm this is settable. Crossdock is used for TLS-level test. + System.setProperty(Configuration.JAEGER_TLS_CERTIFICATE_PINNING, + "sha256/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=,sha256/BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB="); + + HttpSender sender = (HttpSender) Configuration.SenderConfiguration.fromEnv().getSender(); + sender.send(new Process("robotrock"), generateSpans()); + } + @Test public void sanityTestForTokenAuthTest() throws Exception { System.setProperty(Configuration.JAEGER_ENDPOINT, target("/api/bearer").getUri().toString()); From 386b53752381079420f2244fef4e128847154127 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Wed, 13 Mar 2019 17:49:30 +0900 Subject: [PATCH 17/25] increase crossdock timeout Signed-off-by: Iori Yoneji --- jaeger-crossdock/docker-compose.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jaeger-crossdock/docker-compose.yml b/jaeger-crossdock/docker-compose.yml index 4a30f951b..c66cf29fc 100644 --- a/jaeger-crossdock/docker-compose.yml +++ b/jaeger-crossdock/docker-compose.yml @@ -13,9 +13,9 @@ services: - nodejs environment: - WAIT_FOR=test_driver,go,java-udp,java-http,java-https,python,nodejs,jaeger-collector-https-proxy - - WAIT_FOR_TIMEOUT=60s + - WAIT_FOR_TIMEOUT=90s - - CALL_TIMEOUT=60s + - CALL_TIMEOUT=90s - AXIS_CLIENT=go - AXIS_S1NAME=go,java-udp,python,nodejs From a0607f48cd7370959b8f937a208fdc517a03b82f Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Fri, 15 Mar 2019 15:15:22 +0900 Subject: [PATCH 18/25] refine comments and remove debug print as review in #602 Signed-off-by: Iori Yoneji --- jaeger-crossdock/https-proxy/Dockerfile | 8 ++++++-- .../jaegertracing/thrift/internal/senders/HttpSender.java | 5 +++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/jaeger-crossdock/https-proxy/Dockerfile b/jaeger-crossdock/https-proxy/Dockerfile index fbe2ae2bc..29d3976f6 100644 --- a/jaeger-crossdock/https-proxy/Dockerfile +++ b/jaeger-crossdock/https-proxy/Dockerfile @@ -1,8 +1,12 @@ FROM nginx:alpine -ADD build/proxy.conf /etc/nginx/conf.d/proxy.conf +# Overwrite default configuration +ADD build/proxy.conf /etc/nginx/conf.d/default.conf +# Install generated certificates; run `gen.sh` first ADD build/proxy.crt /etc/nginx/proxy.crt ADD build/proxy.key /etc/nginx/proxy.key EXPOSE 8080 14443 -CMD ["sh", "-c", "while ! { wget --spider -S http://jaeger-collector:14269; }; do echo waiting...; sleep 1; done; nginx -g 'daemon off;'"] + +# Poll healthcheck port until it returns 2xx or 3xx. +CMD ["sh", "-c", "while ! { wget --spider -S http://jaeger-collector:14269; }; do echo waiting for healthcheck; sleep 1; done; nginx -g 'daemon off;'"] diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 53d73f84a..b8d7ade1d 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -77,7 +77,6 @@ public void send(Process process, List spans) throws SenderException { try { response = httpClient.newCall(request).execute(); } catch (IOException e) { - e.printStackTrace(); throw new SenderException(String.format("Could not send %d spans", spans.size()), e, spans.size()); } @@ -140,8 +139,10 @@ public Builder withCertificatePinning(String[] sha256certs /* comma separated */ return this; } + /** + * Enable accepting self-signed certificates. This will only take effect if pinning is provided. + */ public Builder acceptSelfSigned() { - // This dangerous operation will only take effect if pinning is used. this.selfSigned = true; return this; } From bc856fdad9afcf82219b5406db4b94e5a5cc5292 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Wed, 20 Mar 2019 23:10:08 +0900 Subject: [PATCH 19/25] add more careful error handling in HttpSender Signed-off-by: Iori Yoneji --- .../thrift/internal/senders/HttpSender.java | 36 ++++++++++++------- .../internal/senders/HttpSenderTest.java | 5 +++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index b8d7ade1d..a381373d8 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -156,20 +156,22 @@ public HttpSender build() { final URI uri = new URI(endpoint); String hostname = hostname = uri.getHost(); - if (!selfSigned && !pins.isEmpty()) { - // Pinning Certificate issued by public CA - for (String cert: pins) { - certificatePinnerBuilder.add(hostname, String.format("%s", cert)); + if (uri.getScheme().equals("https")) { + if (!selfSigned && !pins.isEmpty()) { + // Pinning Certificate issued by public CA + for (String cert: pins) { + certificatePinnerBuilder.add(hostname, String.format("%s", cert)); + } + clientBuilder.certificatePinner(certificatePinnerBuilder.build()); + } else if (selfSigned && !pins.isEmpty()) { + /* Issued by private CA, OkHttp's pinner is unable to verify the pins. + * Instead, check the sha256 hash value by custom verifier. */ + acceptSelfSigned(clientBuilder, hostname, pins); } - clientBuilder.certificatePinner(certificatePinnerBuilder.build()); - } else if (selfSigned && !pins.isEmpty()) { - /* Issued by private CA, OkHttp's pinner is unable to verify the pins. - * Instead, check the sha256 hash value by custom verifier. */ - acceptSelfSigned(clientBuilder, hostname, pins); } - } catch (Exception e) { - // Won't validate the endpoint name. - // True moment comes at Sender's Build. + } catch (java.net.URISyntaxException e) { + // Early but similar validation & exception as what happens when HttpSender is called. + throw new IllegalArgumentException("Could not parse endpoint.", e); } return new HttpSender(this); } @@ -221,7 +223,15 @@ private boolean check(X509Certificate[] chain) { sslContext.init(null, selfSignedServerTrustManager, null); clientBuilder.sslSocketFactory(sslContext.getSocketFactory(), (X509TrustManager) selfSignedServerTrustManager[0]); - } catch (Exception e) { + } catch (java.security.NoSuchAlgorithmException e) { + // TLS is hardcoded abave. No occasion to come here. + throw new RuntimeException(e); + } catch (java.security.KeyManagementException e) { + /* KeyManagementException will not occurs because sslContext uses default KeyManager (first argument). + * + * > Either of the first two parameters may be null in which case the installed security providers + * > will be searched for the highest priority implementation of the appropriate factory. + */ throw new RuntimeException(e); } } diff --git a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java index bbbf5f0cf..9f9619722 100644 --- a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java +++ b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java @@ -84,6 +84,11 @@ public void misconfiguredUrl() throws Exception { new HttpSender.Builder("misconfiguredUrl").build(); } + @Test(expected = IllegalArgumentException.class) + public void misconfiguredQuery() throws Exception { + HttpSender sender = new HttpSender.Builder("http://some-server/api/traces?there=is&another=query") + .build(); + } @Test(expected = Exception.class) public void serverDoesntExist() throws Exception { HttpSender sender = new HttpSender.Builder("http://some-server/api/traces") From 85302785d4fb6c162a57d719b6a4a44bc4ce2474 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Wed, 20 Mar 2019 23:25:51 +0900 Subject: [PATCH 20/25] print pins if pinning failed for self-signed server Signed-off-by: Iori Yoneji --- .../thrift/internal/senders/HttpSender.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index a381373d8..2480d2cb7 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -29,6 +29,7 @@ import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509TrustManager; import javax.net.ssl.TrustManager; +import lombok.extern.slf4j.Slf4j; import lombok.ToString; import okhttp3.CertificatePinner; import okhttp3.Credentials; @@ -98,6 +99,7 @@ public void send(Process process, List spans) throws SenderException { throw new SenderException(exceptionMessage, null, spans.size()); } + @Slf4j public static class Builder { private final String endpoint; private CertificatePinner.Builder certificatePinnerBuilder = new CertificatePinner.Builder(); @@ -196,6 +198,14 @@ private boolean check(X509Certificate[] chain) { return false; } + private String describePins(X509Certificate[] chain) { + String message = "No pins matched with remote certificate chain. The pins are:"; + for (X509Certificate cert: chain) { + message = message + "\n\t" + CertificatePinner.pin(cert); + } + return message + "\n"; + } + @Override public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { for (X509Certificate cert: chain) { String[] subject = cert.getSubjectDN().getName().split("/"); @@ -204,11 +214,13 @@ private boolean check(X509Certificate[] chain) { // Found endpoint's hostname. This chain is going to be tested. if (check(chain)) { return; + } else { + // For TOFU (trust on first use) usecase, print the chain + log.warn(describePins(chain)); } } } } - // TODO: For TOFU (trust on first use) usecase, print the chain throw new CertificateException(); } @Override public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { From 17b1e7b497700292c3b7a15a0740a07c2333ba42 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Wed, 20 Mar 2019 23:46:59 +0900 Subject: [PATCH 21/25] add note for pre-generated, for-crossdock certificate and fix its conf Signed-off-by: Iori Yoneji --- jaeger-crossdock/https-proxy/gen.sh | 23 +++++++++++++++++-- .../https-proxy/proxy.template.conf | 8 +++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/jaeger-crossdock/https-proxy/gen.sh b/jaeger-crossdock/https-proxy/gen.sh index a02ac40e3..404cdbf22 100755 --- a/jaeger-crossdock/https-proxy/gen.sh +++ b/jaeger-crossdock/https-proxy/gen.sh @@ -9,6 +9,25 @@ fi SERVER=$1 FORWARD=$2 + +# Authority.crt & authority.key is pre-generated with hash code of sha256/n6Ovey/sJws9vKJpESmWyQf9Oocak9J51mmPKGm4S0E= +# About this file: +# CN: TEST CA +# Algorithm: ECDSA-SHA256 +# Validity: +# Not Before: Mar 7 14:38:20 2019 GMT +# Not After: Mar 7 14:38:20 2029 GMT +# +# To generate key: (not needed for cert renewal) +# openssl ecparam -genkey -name prime256v1 -out authority.key +# +# To regenerate: +# openssl req -new -sha256 -key authority.key -out authority.csr -subj "/CN=TEST CA/" +# openssl x509 -trustout -signkey authority.key -days 3652 -req -in authority.csr -out authority.crt +# +# To check pin: +# openssl ec -in authority.key -outform der -pubout | openssl dgst -sha256 -binary | openssl enc -base64 +AUTHORITY=authority TARGET=build/proxy # generate secrets @@ -17,10 +36,10 @@ mkdir -p ./build openssl ecparam -genkey -name prime256v1 -out ${TARGET}.key openssl req -new -sha256 -key ${TARGET}.key -out ${TARGET}.csr -subj \ "/CN=${SERVER}/" -openssl x509 -req -sha256 -days 1 -CA authority.crt -CAkey authority.key -CAcreateserial -in ${TARGET}.csr -out ${TARGET}.crt +openssl x509 -req -sha256 -days 1 -CA ${AUTHORITY}.crt -CAkey ${AUTHORITY}.key -CAcreateserial -in ${TARGET}.csr -out ${TARGET}.crt chmod 644 ${TARGET}.key -cat authority.crt >> ${TARGET}.crt +cat ${AUTHORITY}.crt >> ${TARGET}.crt # generate nginx settings SERVER=${SERVER} FORWARD=${FORWARD} envsubst < "proxy.template.conf" > ${TARGET}.conf diff --git a/jaeger-crossdock/https-proxy/proxy.template.conf b/jaeger-crossdock/https-proxy/proxy.template.conf index c5bdf4e19..c9f5f37c2 100644 --- a/jaeger-crossdock/https-proxy/proxy.template.conf +++ b/jaeger-crossdock/https-proxy/proxy.template.conf @@ -3,17 +3,21 @@ server { server_name ${SERVER}; location / { root /usr/share/nginx/html/; - index index.html; + index index.html; } } server { - listen 14443 ssl; + listen 14443 ssl http2; server_name ${SERVER}; ssl_protocols TLSv1 TLSv1.1 TLSv1.2; + ssl_session_cache shared:SSL:10m; ssl_certificate /etc/nginx/proxy.crt; ssl_certificate_key /etc/nginx/proxy.key; + ssl_prefer_server_ciphers on; + ssl_ciphers "EECDH+ECDSA+AESGCM EECDH+aRSA+AESGCM EECDH+ECDSA+SHA384 EECDH+ECDSA+SHA256 EECDH+aRSA+SHA384 EECDH+aRSA+SHA256 EECDH !EDH+aRSA !RC4 !aNULL !eNULL !LOW !3DES !MD5 !EXP !PSK !SRP !DSS"; + location / { proxy_pass http://${FORWARD}; } From b10ab02019ce6ffee8211c57d60e77fc7c425c40 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Thu, 21 Mar 2019 17:38:24 +0900 Subject: [PATCH 22/25] Log SenderException in RemoteReporter, including Trust verification Signed-off-by: Iori Yoneji --- .../internal/reporters/RemoteReporter.java | 1 + .../thrift/internal/senders/HttpSender.java | 111 +++++++++--------- .../internal/senders/HttpSenderTest.java | 3 +- 3 files changed, 59 insertions(+), 56 deletions(-) diff --git a/jaeger-core/src/main/java/io/jaegertracing/internal/reporters/RemoteReporter.java b/jaeger-core/src/main/java/io/jaegertracing/internal/reporters/RemoteReporter.java index 57c18ed22..0f2b225eb 100644 --- a/jaeger-core/src/main/java/io/jaegertracing/internal/reporters/RemoteReporter.java +++ b/jaeger-core/src/main/java/io/jaegertracing/internal/reporters/RemoteReporter.java @@ -175,6 +175,7 @@ public void run() { try { command.execute(); } catch (SenderException e) { + log.warn("RemoteReporter failed to send:", e); metrics.reporterFailure.inc(e.getDroppedSpanCount()); } } catch (Exception e) { diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 2480d2cb7..2c474533e 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -29,7 +29,6 @@ import javax.net.ssl.SSLSocketFactory; import javax.net.ssl.X509TrustManager; import javax.net.ssl.TrustManager; -import lombok.extern.slf4j.Slf4j; import lombok.ToString; import okhttp3.CertificatePinner; import okhttp3.Credentials; @@ -99,7 +98,6 @@ public void send(Process process, List spans) throws SenderException { throw new SenderException(exceptionMessage, null, spans.size()); } - @Slf4j public static class Builder { private final String endpoint; private CertificatePinner.Builder certificatePinnerBuilder = new CertificatePinner.Builder(); @@ -158,7 +156,7 @@ public HttpSender build() { final URI uri = new URI(endpoint); String hostname = hostname = uri.getHost(); - if (uri.getScheme().equals("https")) { + if (uri.getScheme() != null && uri.getScheme().equals("https")) { if (!selfSigned && !pins.isEmpty()) { // Pinning Certificate issued by public CA for (String cert: pins) { @@ -180,57 +178,7 @@ public HttpSender build() { private void acceptSelfSigned(OkHttpClient.Builder clientBuilder, final String hostname, final List pinlist) { try { - final TrustManager[] selfSignedServerTrustManager = new TrustManager[] { - new X509TrustManager() { - private final String subjectCN = "CN=" + hostname; - private final List pins = pinlist; - - private boolean check(X509Certificate[] chain) { - // Intersection of pins and every cert in this chain - for (X509Certificate cert: chain) { - String hash = CertificatePinner.pin(cert); - for (String pin: pins) { - if (hash.equals(pin)) { - return true; - } - } - } - return false; - } - - private String describePins(X509Certificate[] chain) { - String message = "No pins matched with remote certificate chain. The pins are:"; - for (X509Certificate cert: chain) { - message = message + "\n\t" + CertificatePinner.pin(cert); - } - return message + "\n"; - } - - @Override public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { - for (X509Certificate cert: chain) { - String[] subject = cert.getSubjectDN().getName().split("/"); - for (String name: subject) { - if (subjectCN.equals(name)) { - // Found endpoint's hostname. This chain is going to be tested. - if (check(chain)) { - return; - } else { - // For TOFU (trust on first use) usecase, print the chain - log.warn(describePins(chain)); - } - } - } - } - throw new CertificateException(); - } - @Override public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { - throw new CertificateException(); // Nothing will be accepted - } - @Override public X509Certificate[] getAcceptedIssuers() { - return new X509Certificate[0]; - } - } - }; + final TrustManager[] selfSignedServerTrustManager = new TrustManager[] { new SelfSignedTrustManager(hostname, pinlist) }; final SSLContext sslContext = SSLContext.getInstance("TLS"); sslContext.init(null, selfSignedServerTrustManager, null); clientBuilder.sslSocketFactory(sslContext.getSocketFactory(), (X509TrustManager) selfSignedServerTrustManager[0]); @@ -261,5 +209,60 @@ public Response intercept(Chain chain) throws IOException { } }; } + + private static class SelfSignedTrustManager implements javax.net.ssl.X509TrustManager { + private final String subjectCN; + private final List pins; + + protected SelfSignedTrustManager(String hostname, List pins) { + this.subjectCN = "CN=" + hostname; + this.pins = pins; + } + + private boolean check(X509Certificate[] chain) { + // Intersection of pins and every cert in this chain + for (X509Certificate cert: chain) { + String hash = CertificatePinner.pin(cert); + for (String pin: pins) { + if (hash.equals(pin)) { + return true; + } + } + } + return false; + } + + private String describePins(X509Certificate[] chain) { + String message = "No pins matched with remote certificate chain. The pins are:"; + for (X509Certificate cert: chain) { + message = message + "\n\t" + CertificatePinner.pin(cert); + } + return message + "\n"; + } + + public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException { + for (X509Certificate cert: chain) { + String[] subject = cert.getSubjectDN().getName().split("/"); + for (String name: subject) { + if (subjectCN.equals(name)) { + // Found endpoint's hostname. This chain is going to be tested. + if (check(chain)) { + return; + } else { + // For TOFU (trust on first use) usecase, print the chain + throw new RuntimeException(describePins(chain)); + } + } + } + } + throw new CertificateException(); + } + public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException { + throw new CertificateException(); // Nothing will be accepted + } + public X509Certificate[] getAcceptedIssuers() { + return new X509Certificate[0]; + } + } } } diff --git a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java index 9f9619722..2ba2009fd 100644 --- a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java +++ b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java @@ -86,8 +86,7 @@ public void misconfiguredUrl() throws Exception { @Test(expected = IllegalArgumentException.class) public void misconfiguredQuery() throws Exception { - HttpSender sender = new HttpSender.Builder("http://some-server/api/traces?there=is&another=query") - .build(); + new HttpSender.Builder("http://some-server/api/traces^there=is&another=query").build(); } @Test(expected = Exception.class) public void serverDoesntExist() throws Exception { From f45ed78a653bf49ef2d4dd70ab201312fe7b46d9 Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Thu, 21 Mar 2019 19:55:13 +0900 Subject: [PATCH 23/25] add a test "testReporterWithSenderException" for RemoteReporter Signed-off-by: Iori Yoneji --- .../reporters/RemoteReporterTest.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/jaeger-core/src/test/java/io/jaegertracing/internal/reporters/RemoteReporterTest.java b/jaeger-core/src/test/java/io/jaegertracing/internal/reporters/RemoteReporterTest.java index 5e3b616ce..dde94045b 100644 --- a/jaeger-core/src/test/java/io/jaegertracing/internal/reporters/RemoteReporterTest.java +++ b/jaeger-core/src/test/java/io/jaegertracing/internal/reporters/RemoteReporterTest.java @@ -31,6 +31,7 @@ import io.jaegertracing.internal.samplers.ConstSampler; import io.jaegertracing.internal.senders.InMemorySender; import io.jaegertracing.spi.Reporter; +import io.jaegertracing.spi.Sender; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -328,4 +329,56 @@ public int append(JaegerSpan span) { private JaegerSpan newSpan() { return tracer.buildSpan("x").start(); } + + private static class FailingSender implements Sender { + private static boolean invoked = false; + + @Override + public int append(JaegerSpan span) throws SenderException { + invoked = true; + throw new SenderException("FailingSender is to fail", 1); + } + + @Override + public int flush() throws SenderException { + throw new SenderException("FailingSender is to fail", 1); + } + + @Override + public int close() throws SenderException { + throw new SenderException("FailingSender is to fail", 1); + } + + public boolean invoked() { + return invoked; + } + } + + @Test + public void testReporterWithSenderException() { + FailingSender failingSender = new FailingSender(); + Reporter failingReporter = new RemoteReporter.Builder() + .withSender(failingSender) + .withFlushInterval(flushInterval) + .withMaxQueueSize(maxQueueSize) + .withMetrics(metrics) + .build(); + JaegerTracer failingTracer = new JaegerTracer.Builder("test-failing-reporter") + .withReporter(failingReporter) + .withSampler(new ConstSampler(true)) + .withMetrics(metrics) + .build(); + JaegerSpan span = failingTracer.buildSpan("raza").start(); + failingReporter.report(span); + // do sleep until automatic flush happens on 'reporter' + // added 20ms on top of 'flushInterval' to avoid corner cases + await() + .with() + .pollInterval(1, TimeUnit.MILLISECONDS) + .atMost(flushInterval + 20, TimeUnit.MILLISECONDS) + .until(() -> failingSender.invoked()); + + assertEquals(true, failingSender.invoked()); + assertEquals(1, metricsFactory.getCounter("jaeger_tracer_reporter_spans", "result=err")); + } } From 55ac341ee0ea4c5118bc73103ed1b59babf2baed Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Fri, 21 Jun 2019 20:21:31 +0900 Subject: [PATCH 24/25] fix as review comments in pullrequestreview-232113179 https://github.com/jaegertracing/jaeger-client-java/pull/602#pullrequestreview-232113179 Signed-off-by: Iori Yoneji --- .../thrift/internal/senders/HttpSender.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java index 2c474533e..2112b7b00 100644 --- a/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java +++ b/jaeger-thrift/src/main/java/io/jaegertracing/thrift/internal/senders/HttpSender.java @@ -23,6 +23,8 @@ import java.util.Arrays; import java.util.ArrayList; import java.util.List; +import java.security.NoSuchAlgorithmException; +import java.security.KeyManagementException; import java.security.cert.CertificateException; import java.security.cert.X509Certificate; import javax.net.ssl.SSLContext; @@ -134,7 +136,7 @@ public Builder withAuth(String authToken) { return this; } - public Builder withCertificatePinning(String[] sha256certs /* comma separated */) { + public Builder withCertificatePinning(String[] sha256certs) { pins.addAll(Arrays.asList(sha256certs)); return this; } @@ -156,7 +158,7 @@ public HttpSender build() { final URI uri = new URI(endpoint); String hostname = hostname = uri.getHost(); - if (uri.getScheme() != null && uri.getScheme().equals("https")) { + if ("https".equals(uri.getScheme())) { if (!selfSigned && !pins.isEmpty()) { // Pinning Certificate issued by public CA for (String cert: pins) { @@ -183,10 +185,10 @@ private void acceptSelfSigned(OkHttpClient.Builder clientBuilder, final String h sslContext.init(null, selfSignedServerTrustManager, null); clientBuilder.sslSocketFactory(sslContext.getSocketFactory(), (X509TrustManager) selfSignedServerTrustManager[0]); - } catch (java.security.NoSuchAlgorithmException e) { - // TLS is hardcoded abave. No occasion to come here. + } catch (NoSuchAlgorithmException e) { + // TLS is hardcoded above. No occasion to come here. throw new RuntimeException(e); - } catch (java.security.KeyManagementException e) { + } catch (KeyManagementException e) { /* KeyManagementException will not occurs because sslContext uses default KeyManager (first argument). * * > Either of the first two parameters may be null in which case the installed security providers @@ -210,7 +212,7 @@ public Response intercept(Chain chain) throws IOException { }; } - private static class SelfSignedTrustManager implements javax.net.ssl.X509TrustManager { + private static class SelfSignedTrustManager implements X509TrustManager { private final String subjectCN; private final List pins; From 4e0a70498c33d46de7fef18cca71a2900467ffaa Mon Sep 17 00:00:00 2001 From: Iori Yoneji Date: Mon, 24 Jun 2019 19:44:06 +0900 Subject: [PATCH 25/25] change a test case name setTLSCeritificatePinning -> sendPlainHttpWithCertificatePinning Signed-off-by: Iori Yoneji --- .../jaegertracing/thrift/internal/senders/HttpSenderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java index 2ba2009fd..bcd2b6430 100644 --- a/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java +++ b/jaeger-thrift/src/test/java/io/jaegertracing/thrift/internal/senders/HttpSenderTest.java @@ -130,7 +130,7 @@ public void sendWithTokenAuth() throws Exception { } @Test - public void setTLSCertificatePinning() throws Exception { + public void sendPlainHttpWithCertificatePinning() throws Exception { System.setProperty(Configuration.JAEGER_ENDPOINT, target("/api/traces").getUri().toString()); // Just confirm this is settable. Crossdock is used for TLS-level test. System.setProperty(Configuration.JAEGER_TLS_CERTIFICATE_PINNING,