TLS configuration for Zipkin

Signed-off-by: Matthieu MOREL <[email protected]>

cmd/collector/app/builder_flags.go

@@ -50,6 +50,10 @@ var tlsHTTPFlagsConfig = tlscfg.ServerFlagsConfig{
 	Prefix: "collector.http",
 }
 
+var tlsZipkinFlagsConfig = tlscfg.ServerFlagsConfig{
+	Prefix: "collector.zipkin",
+}
+
 // CollectorOptions holds configuration for collector
 type CollectorOptions struct {
 	// DynQueueSizeMemory determines how much memory to use for the queue
@@ -66,6 +70,8 @@ type CollectorOptions struct {
 	TLSGRPC tlscfg.Options
 	// TLSHTTP configures secure transport for HTTP endpoint to collect spans
 	TLSHTTP tlscfg.Options
+	// TLSZipkin configures secure transport for Zipkin endpoint to collect spans
+	TLSZipkin tlscfg.Options
 	// CollectorTags is the string representing collector tags to append to each and every span
 	CollectorTags map[string]string
 	// CollectorZipkinHTTPHostPort is the host:port address that the Zipkin collector service listens in on for http requests
@@ -101,6 +107,7 @@ func AddFlags(flags *flag.FlagSet) {
 
 	tlsGRPCFlagsConfig.AddFlags(flags)
 	tlsHTTPFlagsConfig.AddFlags(flags)
+	tlsZipkinFlagsConfig.AddFlags(flags)
 }
 
 // InitFromViper initializes CollectorOptions with properties from viper
@@ -127,6 +134,11 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper) (*CollectorOptions,
 	} else {
 		return cOpts, fmt.Errorf("failed to parse HTTP TLS options: %w", err)
 	}
+	if tlsZipkin, err := tlsZipkinFlagsConfig.InitFromViper(v); err == nil {
+		cOpts.TLSZipkin = tlsZipkin
+	} else {
+		return cOpts, fmt.Errorf("failed to parse Zipkin TLS options: %w", err)
+	}
 
 	return cOpts, nil
 }

cmd/collector/app/builder_flags_test.go

@@ -80,6 +80,19 @@ func TestCollectorOptionsWithFailedGRPCFlags(t *testing.T) {
 	assert.Contains(t, err.Error(), "failed to parse gRPC TLS options")
 }
 
+func TestCollectorOptionsWithFailedZipkinFlags(t *testing.T) {
+	c := &CollectorOptions{}
+	v, command := config.Viperize(AddFlags)
+	err := command.ParseFlags([]string{
+		"--collector.zipkin.tls.enabled=false",
+		"--collector.zipkin.tls.cert=blah", // invalid unless tls.enabled
+	})
+	require.NoError(t, err)
+	_, err = c.InitFromViper(v)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "failed to parse Zipkin TLS options")
+}
+
 func TestCollectorOptionsWithFlags_CheckMaxReceiveMessageLength(t *testing.T) {
 	c := &CollectorOptions{}
 	v, command := config.Viperize(AddFlags)

cmd/collector/app/collector.go

@@ -46,11 +46,12 @@ type Collector struct {
 	spanHandlers   *SpanHandlers
 
 	// state, read only
-	hServer                  *http.Server
-	zkServer                 *http.Server
-	grpcServer               *grpc.Server
-	tlsGRPCCertWatcherCloser io.Closer
-	tlsHTTPCertWatcherCloser io.Closer
+	hServer                    *http.Server
+	zkServer                   *http.Server
+	grpcServer                 *grpc.Server
+	tlsGRPCCertWatcherCloser   io.Closer
+	tlsHTTPCertWatcherCloser   io.Closer
+	tlsZipkinCertWatcherCloser io.Closer
 }
 
 // CollectorParams to construct a new Jaeger Collector.
@@ -125,9 +126,11 @@ func (c *Collector) Start(builderOpts *CollectorOptions) error {
 
 	c.tlsGRPCCertWatcherCloser = &builderOpts.TLSGRPC
 	c.tlsHTTPCertWatcherCloser = &builderOpts.TLSHTTP
+	c.tlsZipkinCertWatcherCloser = &builderOpts.TLSZipkin
 	zkServer, err := server.StartZipkinServer(&server.ZipkinServerParams{
 		HostPort:       builderOpts.CollectorZipkinHTTPHostPort,
 		Handler:        c.spanHandlers.ZipkinSpansHandler,
+		TLSConfig:      builderOpts.TLSZipkin,
 		HealthCheck:    c.hCheck,
 		AllowedHeaders: builderOpts.CollectorZipkinAllowedHeaders,
 		AllowedOrigins: builderOpts.CollectorZipkinAllowedOrigins,
@@ -189,6 +192,7 @@ func (c *Collector) Close() error {
 	// watchers actually never return errors from Close
 	_ = c.tlsGRPCCertWatcherCloser.Close()
 	_ = c.tlsHTTPCertWatcherCloser.Close()
+	_ = c.tlsZipkinCertWatcherCloser.Close()
 
 	return nil
 }

cmd/collector/app/server/zipkin.go

@@ -27,13 +27,15 @@ import (
 
 	"github.com/jaegertracing/jaeger/cmd/collector/app/handler"
 	"github.com/jaegertracing/jaeger/cmd/collector/app/zipkin"
+	"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
 	"github.com/jaegertracing/jaeger/pkg/httpmetrics"
 	"github.com/jaegertracing/jaeger/pkg/recoveryhandler"
 )
 
 // ZipkinServerParams to construct a new Jaeger Collector Zipkin Server
 type ZipkinServerParams struct {
+	TLSConfig      tlscfg.Options
 	HostPort       string
 	Handler        handler.ZipkinSpansHandler
 	AllowedOrigins string
@@ -62,6 +64,13 @@ func StartZipkinServer(params *ZipkinServerParams) (*http.Server, error) {
 		Addr:     params.HostPort,
 		ErrorLog: errorLog,
 	}
+	if params.TLSConfig.Enabled {
+		tlsCfg, err := params.TLSConfig.Config(params.Logger) // This checks if the certificates are correctly provided
+		if err != nil {
+			return nil, err
+		}
+		server.TLSConfig = tlsCfg
+	}
 	serveZipkin(server, listener, params)
 
 	return server, nil
@@ -84,7 +93,13 @@ func serveZipkin(server *http.Server, listener net.Listener, params *ZipkinServe
 	recoveryHandler := recoveryhandler.NewRecoveryHandler(params.Logger, true)
 	server.Handler = cors.Handler(httpmetrics.Wrap(recoveryHandler(r), params.MetricsFactory))
 	go func(listener net.Listener, server *http.Server) {
-		if err := server.Serve(listener); err != nil {
+		var err error
+		if params.TLSConfig.Enabled {
+			err = server.ServeTLS(listener, "", "")
+		} else {
+			err = server.Serve(listener)
+		}
+		if err != nil {
 			if err != http.ErrServerClosed {
 				params.Logger.Error("Could not launch Zipkin server", zap.Error(err))
 			}

cmd/collector/app/server/zipkin_test.go

@@ -15,17 +15,23 @@
 package server
 
 import (
+	"crypto/tls"
+	"fmt"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"github.com/uber/jaeger-lib/metrics/metricstest"
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/cmd/collector/app/handler"
+	"github.com/jaegertracing/jaeger/pkg/config/tlscfg"
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
+	"github.com/jaegertracing/jaeger/ports"
 )
 
 // test wrong port number
@@ -57,3 +63,187 @@ func TestSpanCollectorZipkin(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NotNil(t, response)
 }
+
+func TestSpanCollectorZipkinTLS(t *testing.T) {
+	testCases := []struct {
+		name                  string
+		serverTLS             tlscfg.Options
+		clientTLS             tlscfg.Options
+		expectTLSClientErr    bool
+		expectZipkinClientErr bool
+		expectServerFail      bool
+	}{
+		{
+			name: "should fail with TLS client to untrusted TLS server",
+			serverTLS: tlscfg.Options{
+				Enabled:  true,
+				CertPath: testCertKeyLocation + "/example-server-cert.pem",
+				KeyPath:  testCertKeyLocation + "/example-server-key.pem",
+			},
+			clientTLS: tlscfg.Options{
+				Enabled:    true,
+				ServerName: "example.com",
+			},
+			expectTLSClientErr:    true,
+			expectZipkinClientErr: true,
+			expectServerFail:      false,
+		},
+		{
+			name: "should fail with TLS client to trusted TLS server with incorrect hostname",
+			serverTLS: tlscfg.Options{
+				Enabled:  true,
+				CertPath: testCertKeyLocation + "/example-server-cert.pem",
+				KeyPath:  testCertKeyLocation + "/example-server-key.pem",
+			},
+			clientTLS: tlscfg.Options{
+				Enabled:    true,
+				CAPath:     testCertKeyLocation + "/example-CA-cert.pem",
+				ServerName: "nonEmpty",
+			},
+			expectTLSClientErr:    true,
+			expectZipkinClientErr: true,
+			expectServerFail:      false,
+		},
+		{
+			name: "should pass with TLS client to trusted TLS server with correct hostname",
+			serverTLS: tlscfg.Options{
+				Enabled:  true,
+				CertPath: testCertKeyLocation + "/example-server-cert.pem",
+				KeyPath:  testCertKeyLocation + "/example-server-key.pem",
+			},
+			clientTLS: tlscfg.Options{
+				Enabled:    true,
+				CAPath:     testCertKeyLocation + "/example-CA-cert.pem",
+				ServerName: "example.com",
+			},
+			expectTLSClientErr:    false,
+			expectZipkinClientErr: false,
+			expectServerFail:      false,
+		},
+		{
+			name: "should fail with TLS client without cert to trusted TLS server requiring cert",
+			serverTLS: tlscfg.Options{
+				Enabled:      true,
+				CertPath:     testCertKeyLocation + "/example-server-cert.pem",
+				KeyPath:      testCertKeyLocation + "/example-server-key.pem",
+				ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
+			},
+			clientTLS: tlscfg.Options{
+				Enabled:    true,
+				CAPath:     testCertKeyLocation + "/example-CA-cert.pem",
+				ServerName: "example.com",
+			},
+			expectTLSClientErr:    false,
+			expectServerFail:      false,
+			expectZipkinClientErr: true,
+		},
+		{
+			name: "should pass with TLS client with cert to trusted TLS server requiring cert",
+			serverTLS: tlscfg.Options{
+				Enabled:      true,
+				CertPath:     testCertKeyLocation + "/example-server-cert.pem",
+				KeyPath:      testCertKeyLocation + "/example-server-key.pem",
+				ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
+			},
+			clientTLS: tlscfg.Options{
+				Enabled:    true,
+				CAPath:     testCertKeyLocation + "/example-CA-cert.pem",
+				ServerName: "example.com",
+				CertPath:   testCertKeyLocation + "/example-client-cert.pem",
+				KeyPath:    testCertKeyLocation + "/example-client-key.pem",
+			},
+			expectTLSClientErr:    false,
+			expectServerFail:      false,
+			expectZipkinClientErr: false,
+		},
+		{
+			name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA",
+			serverTLS: tlscfg.Options{
+				Enabled:      true,
+				CertPath:     testCertKeyLocation + "/example-server-cert.pem",
+				KeyPath:      testCertKeyLocation + "/example-server-key.pem",
+				ClientCAPath: testCertKeyLocation + "/wrong-CA-cert.pem", // NB: wrong CA
+			},
+			clientTLS: tlscfg.Options{
+				Enabled:    true,
+				CAPath:     testCertKeyLocation + "/example-CA-cert.pem",
+				ServerName: "example.com",
+				CertPath:   testCertKeyLocation + "/example-client-cert.pem",
+				KeyPath:    testCertKeyLocation + "/example-client-key.pem",
+			},
+			expectTLSClientErr:    false,
+			expectServerFail:      false,
+			expectZipkinClientErr: true,
+		},
+		{
+			name: "should fail with TLS client with cert to trusted TLS server with incorrect TLS min",
+			serverTLS: tlscfg.Options{
+				Enabled:      true,
+				CertPath:     testCertKeyLocation + "/example-server-cert.pem",
+				KeyPath:      testCertKeyLocation + "/example-server-key.pem",
+				ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem",
+				MinVersion:   "1.5",
+			},
+			clientTLS: tlscfg.Options{
+				Enabled:    true,
+				CAPath:     testCertKeyLocation + "/example-CA-cert.pem",
+				ServerName: "example.com",
+				CertPath:   testCertKeyLocation + "/example-client-cert.pem",
+				KeyPath:    testCertKeyLocation + "/example-client-key.pem",
+			},
+			expectTLSClientErr:    true,
+			expectServerFail:      true,
+			expectZipkinClientErr: false,
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			logger, _ := zap.NewDevelopment()
+			params := &ZipkinServerParams{
+				HostPort:       fmt.Sprintf(":%d", ports.CollectorZipkin),
+				Handler:        handler.NewZipkinSpanHandler(logger, nil, nil),
+				MetricsFactory: metricstest.NewFactory(time.Hour),
+				HealthCheck:    healthcheck.New(),
+				Logger:         logger,
+				TLSConfig:      test.serverTLS,
+			}
+
+			server, err := StartZipkinServer(params)
+
+			if test.expectServerFail {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+			defer server.Close()
+
+			clientTLSCfg, err0 := test.clientTLS.Config(zap.NewNop())
+			require.NoError(t, err0)
+			dialer := &net.Dialer{Timeout: 2 * time.Second}
+			conn, clientError := tls.DialWithDialer(dialer, "tcp", fmt.Sprintf("localhost:%d", ports.CollectorZipkin), clientTLSCfg)
+
+			if test.expectTLSClientErr {
+				require.Error(t, clientError)
+			} else {
+				require.NoError(t, clientError)
+				require.Nil(t, conn.Close())
+			}
+
+			client := &http.Client{
+				Transport: &http.Transport{
+					TLSClientConfig: clientTLSCfg,
+				},
+			}
+
+			response, requestError := client.Post(fmt.Sprintf("https://localhost:%d", ports.CollectorZipkin), "", nil)
+
+			if test.expectZipkinClientErr {
+				require.Error(t, requestError)
+			} else {
+				require.NoError(t, requestError)
+				require.NotNil(t, response)
+			}
+		})
+	}
+}

go.mod

@@ -8,7 +8,6 @@ require (
 	github.com/bsm/sarama-cluster v2.1.13+incompatible
 	github.com/crossdock/crossdock-go v0.0.0-20160816171116-049aabb0122b
 	github.com/dgraph-io/badger/v3 v3.2103.2
-	github.com/dgraph-io/ristretto v0.1.0 // indirect
 	github.com/fsnotify/fsnotify v1.5.4
 	github.com/go-openapi/errors v0.20.2
 	github.com/go-openapi/loads v0.21.1
@@ -45,6 +44,7 @@ require (
 	github.com/uber/jaeger-lib v2.4.1+incompatible
 	github.com/xdg-go/scram v1.1.1
 	go.opentelemetry.io/collector/model v0.49.0
+	go.opentelemetry.io/collector/pdata v0.49.0
 	go.uber.org/atomic v1.9.0
 	go.uber.org/automaxprocs v1.5.1
 	go.uber.org/zap v1.21.0
@@ -55,8 +55,6 @@ require (
 	gopkg.in/yaml.v2 v2.4.0
 )
 
-require go.opentelemetry.io/collector/pdata v0.49.0
-
 require (
 	github.com/HdrHistogram/hdrhistogram-go v1.0.1 // indirect
 	github.com/VividCortex/gohistogram v1.0.0 // indirect
@@ -67,6 +65,7 @@ require (
 	github.com/cespare/xxhash/v2 v2.1.2 // indirect
 	github.com/cpuguy83/go-md2man/v2 v2.0.1 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/dgraph-io/ristretto v0.1.0 // indirect
 	github.com/dustin/go-humanize v1.0.0 // indirect
 	github.com/eapache/go-resiliency v1.2.0 // indirect
 	github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21 // indirect

ports/ports.go

@@ -37,6 +37,8 @@ const (
 	CollectorHTTP = 14268
 	// CollectorAdminHTTP is the default admin HTTP port (health check, metrics, etc.)
 	CollectorAdminHTTP = 14269
+	// CollectorZipkin is the port for Zipkin server for sending spans
+	CollectorZipkin = 9411
 
 	// QueryGRPC is the default port of GRPC requests for Query trace retrieval
 	QueryGRPC = 16685