From 92d363c76d67f169c6b79b559e0381e34fc4a213 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Thu, 9 Jul 2020 10:37:50 +0530 Subject: [PATCH 01/25] Added TLS for HTTP (consumer-query) server Signed-off-by: rjs211 --- cmd/query/app/flags.go | 17 +++++++++++++---- cmd/query/app/server.go | 34 ++++++++++++++++++++++++++++------ 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 80f4e2ea152..e00774cdb26 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -48,12 +48,18 @@ const ( queryMaxClockSkewAdjust = "query.max-clock-skew-adjustment" ) -var tlsFlagsConfig = tlscfg.ServerFlagsConfig{ +var tlsGrpcFlagsConfig = tlscfg.ServerFlagsConfig{ Prefix: "query.grpc", ShowEnabled: true, ShowClientCA: true, } +var tlsHttpFlagsConfig = tlscfg.ServerFlagsConfig{ + Prefix: "query.http", + ShowEnabled: true, + ShowClientCA: true, +} + // QueryOptions holds configuration for query service type QueryOptions struct { // HostPort is the host:port address that the query service listens o n @@ -66,8 +72,10 @@ type QueryOptions struct { UIConfig string // BearerTokenPropagation activate/deactivate bearer token propagation to storage BearerTokenPropagation bool - // TLS configures secure transport - TLS tlscfg.Options + // TLS configures secure transport (Consumer to Query service gRPC APO) + TLSGrpc tlscfg.Options + // TLS configures secure transport (Consumer to Query service HTTP API) + TLSHttp tlscfg.Options // AdditionalHeaders AdditionalHeaders http.Header // MaxClockSkewAdjust is the maximum duration by which jaeger-query will adjust a span @@ -93,7 +101,8 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) *Qu qOpts.StaticAssets = v.GetString(queryStaticFiles) qOpts.UIConfig = v.GetString(queryUIConfig) qOpts.BearerTokenPropagation = v.GetBool(queryTokenPropagation) - qOpts.TLS = tlsFlagsConfig.InitFromViper(v) + qOpts.TLSGrpc = tlsGrpcFlagsConfig.InitFromViper(v) + qOpts.TLSGrpc = tlsHttpFlagsConfig.InitFromViper(v) qOpts.MaxClockSkewAdjust = v.GetDuration(queryMaxClockSkewAdjust) stringSlice := v.GetStringSlice(queryAdditionalHeaders) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 3e4b6756462..1ffc11e6973 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -17,6 +17,7 @@ package app import ( "net" "net/http" + "path/filepath" "strings" "github.com/gorilla/handlers" @@ -54,13 +55,18 @@ func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, options *Que return nil, err } + httpServer, err := createHTTPServer(querySvc, options, tracer, logger) + if err != nil { + return nil, err + } + return &Server{ logger: logger, querySvc: querySvc, queryOptions: options, tracer: tracer, grpcServer: grpcServer, - httpServer: createHTTPServer(querySvc, options, tracer, logger), + httpServer: httpServer, unavailableChannel: make(chan healthcheck.Status), }, nil } @@ -73,8 +79,8 @@ func (s Server) HealthCheckStatus() chan healthcheck.Status { func createGRPCServer(querySvc *querysvc.QueryService, options *QueryOptions, logger *zap.Logger, tracer opentracing.Tracer) (*grpc.Server, error) { var grpcOpts []grpc.ServerOption - if options.TLS.Enabled { - tlsCfg, err := options.TLS.Config() + if options.TLSGrpc.Enabled { + tlsCfg, err := options.TLSGrpc.Config() if err != nil { return nil, err } @@ -90,11 +96,19 @@ func createGRPCServer(querySvc *querysvc.QueryService, options *QueryOptions, lo return server, nil } -func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, tracer opentracing.Tracer, logger *zap.Logger) *http.Server { +func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, tracer opentracing.Tracer, logger *zap.Logger) (*http.Server, error) { apiHandlerOptions := []HandlerOption{ HandlerOptions.Logger(logger), HandlerOptions.Tracer(tracer), } + + if queryOpts.TLSHttp.Enabled { + _, err := queryOpts.TLSHttp.Config() // This checks if the certificates are correctly provided + if err != nil { + return nil, err + } + } + apiHandler := NewAPIHandler( querySvc, apiHandlerOptions...) @@ -114,7 +128,7 @@ func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true) return &http.Server{ Handler: recoveryHandler(handler), - } + }, nil } // Start http, GRPC and cmux servers concurrently @@ -146,13 +160,21 @@ func (s *Server) Start() error { go func() { s.logger.Info("Starting HTTP server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) + var err error + if s.queryOptions.TLSHttp.Enabled { + err = s.httpServer.ServeTLS(httpListener, filepath.Clean(s.queryOptions.TLSHttp.CertPath), filepath.Clean(s.queryOptions.TLSHttp.KeyPath)) - switch err := s.httpServer.Serve(httpListener); err { + } else { + err = s.httpServer.Serve(httpListener) + } + + switch err { case nil, http.ErrServerClosed, cmux.ErrListenerClosed: // normal exit, nothing to do default: s.logger.Error("Could not start HTTP server", zap.Error(err)) } + s.unavailableChannel <- healthcheck.Unavailable }() From 3bf07461a21ab305aee20d0bc6f7821b54769ea5 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Thu, 9 Jul 2020 11:25:02 +0530 Subject: [PATCH 02/25] Add testcase of error in TLS HTTP server creation Signed-off-by: rjs211 --- cmd/query/app/server_test.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 33f8a36b802..75dfbe726c4 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -44,7 +44,7 @@ func TestServerError(t *testing.T) { assert.Error(t, srv.Start()) } -func TestCreateTLSServerError(t *testing.T) { +func TestCreateTLSGrpcServerError(t *testing.T) { tlsCfg := tlscfg.Options{ Enabled: true, CertPath: "invalid/path", @@ -53,7 +53,20 @@ func TestCreateTLSServerError(t *testing.T) { } _, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, - &QueryOptions{TLS: tlsCfg}, opentracing.NoopTracer{}) + &QueryOptions{TLSGrpc: tlsCfg}, opentracing.NoopTracer{}) + assert.NotNil(t, err) +} + +func TestCreateTLSHttpServerError(t *testing.T) { + tlsCfg := tlscfg.Options{ + Enabled: true, + CertPath: "invalid/path", + KeyPath: "invalid/path", + ClientCAPath: "invalid/path", + } + + _, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, + &QueryOptions{TLSHttp: tlsCfg}, opentracing.NoopTracer{}) assert.NotNil(t, err) } From 050fb627da25bad20b496cac9c323bf24bf407cf Mon Sep 17 00:00:00 2001 From: rjs211 Date: Thu, 9 Jul 2020 19:07:44 +0530 Subject: [PATCH 03/25] Minor refactoring of properties and vars Signed-off-by: rjs211 --- cmd/query/app/flags.go | 12 ++++++------ cmd/query/app/server.go | 12 ++++++------ cmd/query/app/server_test.go | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index e00774cdb26..8ae152bc056 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -48,13 +48,13 @@ const ( queryMaxClockSkewAdjust = "query.max-clock-skew-adjustment" ) -var tlsGrpcFlagsConfig = tlscfg.ServerFlagsConfig{ +var tlsGRPCFlagsConfig = tlscfg.ServerFlagsConfig{ Prefix: "query.grpc", ShowEnabled: true, ShowClientCA: true, } -var tlsHttpFlagsConfig = tlscfg.ServerFlagsConfig{ +var tlsHTTPFlagsConfig = tlscfg.ServerFlagsConfig{ Prefix: "query.http", ShowEnabled: true, ShowClientCA: true, @@ -73,9 +73,9 @@ type QueryOptions struct { // BearerTokenPropagation activate/deactivate bearer token propagation to storage BearerTokenPropagation bool // TLS configures secure transport (Consumer to Query service gRPC APO) - TLSGrpc tlscfg.Options + TLSGRPC tlscfg.Options // TLS configures secure transport (Consumer to Query service HTTP API) - TLSHttp tlscfg.Options + TLSHTTP tlscfg.Options // AdditionalHeaders AdditionalHeaders http.Header // MaxClockSkewAdjust is the maximum duration by which jaeger-query will adjust a span @@ -101,8 +101,8 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) *Qu qOpts.StaticAssets = v.GetString(queryStaticFiles) qOpts.UIConfig = v.GetString(queryUIConfig) qOpts.BearerTokenPropagation = v.GetBool(queryTokenPropagation) - qOpts.TLSGrpc = tlsGrpcFlagsConfig.InitFromViper(v) - qOpts.TLSGrpc = tlsHttpFlagsConfig.InitFromViper(v) + qOpts.TLSGRPC = tlsGRPCFlagsConfig.InitFromViper(v) + qOpts.TLSHTTP = tlsHTTPFlagsConfig.InitFromViper(v) qOpts.MaxClockSkewAdjust = v.GetDuration(queryMaxClockSkewAdjust) stringSlice := v.GetStringSlice(queryAdditionalHeaders) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 1ffc11e6973..01b1bffa65c 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -79,8 +79,8 @@ func (s Server) HealthCheckStatus() chan healthcheck.Status { func createGRPCServer(querySvc *querysvc.QueryService, options *QueryOptions, logger *zap.Logger, tracer opentracing.Tracer) (*grpc.Server, error) { var grpcOpts []grpc.ServerOption - if options.TLSGrpc.Enabled { - tlsCfg, err := options.TLSGrpc.Config() + if options.TLSGRPC.Enabled { + tlsCfg, err := options.TLSGRPC.Config() if err != nil { return nil, err } @@ -102,8 +102,8 @@ func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, HandlerOptions.Tracer(tracer), } - if queryOpts.TLSHttp.Enabled { - _, err := queryOpts.TLSHttp.Config() // This checks if the certificates are correctly provided + if queryOpts.TLSHTTP.Enabled { + _, err := queryOpts.TLSHTTP.Config() // This checks if the certificates are correctly provided if err != nil { return nil, err } @@ -161,8 +161,8 @@ func (s *Server) Start() error { go func() { s.logger.Info("Starting HTTP server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) var err error - if s.queryOptions.TLSHttp.Enabled { - err = s.httpServer.ServeTLS(httpListener, filepath.Clean(s.queryOptions.TLSHttp.CertPath), filepath.Clean(s.queryOptions.TLSHttp.KeyPath)) + if s.queryOptions.TLSHTTP.Enabled { + err = s.httpServer.ServeTLS(httpListener, filepath.Clean(s.queryOptions.TLSHTTP.CertPath), filepath.Clean(s.queryOptions.TLSHTTP.KeyPath)) } else { err = s.httpServer.Serve(httpListener) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 75dfbe726c4..13019d2b084 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -53,7 +53,7 @@ func TestCreateTLSGrpcServerError(t *testing.T) { } _, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, - &QueryOptions{TLSGrpc: tlsCfg}, opentracing.NoopTracer{}) + &QueryOptions{TLSGRPC: tlsCfg}, opentracing.NoopTracer{}) assert.NotNil(t, err) } @@ -66,7 +66,7 @@ func TestCreateTLSHttpServerError(t *testing.T) { } _, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, - &QueryOptions{TLSHttp: tlsCfg}, opentracing.NoopTracer{}) + &QueryOptions{TLSHTTP: tlsCfg}, opentracing.NoopTracer{}) assert.NotNil(t, err) } From d02d85f4dfe14a3ee6126e1f6717d58b480d3958 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Thu, 9 Jul 2020 19:26:41 +0530 Subject: [PATCH 04/25] Exposing flags for HTTP and GRPC with TLS config Signed-off-by: rjs211 --- cmd/query/app/flags.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 8ae152bc056..41d8657efd6 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -92,6 +92,8 @@ func AddFlags(flagSet *flag.FlagSet) { flagSet.String(queryUIConfig, "", "The path to the UI configuration file in JSON format") flagSet.Bool(queryTokenPropagation, false, "Allow propagation of bearer token to be used by storage plugins") flagSet.Duration(queryMaxClockSkewAdjust, time.Second, "The maximum delta by which span timestamps may be adjusted in the UI due to clock skew; set to 0s to disable clock skew adjustments") + tlsGRPCFlagsConfig.AddFlags(flagSet) + tlsHTTPFlagsConfig.AddFlags(flagSet) } // InitFromViper initializes QueryOptions with properties from viper From e135064d7bfc6995053b3ecc26263505cf6cfd76 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Sat, 11 Jul 2020 12:14:11 +0530 Subject: [PATCH 05/25] minor refactoring of comments Signed-off-by: rjs211 --- cmd/query/app/flags.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 41d8657efd6..0feb473a186 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -72,9 +72,9 @@ type QueryOptions struct { UIConfig string // BearerTokenPropagation activate/deactivate bearer token propagation to storage BearerTokenPropagation bool - // TLS configures secure transport (Consumer to Query service gRPC APO) + // TLSGRPC configures secure transport (Consumer to Query service GRPC API) TLSGRPC tlscfg.Options - // TLS configures secure transport (Consumer to Query service HTTP API) + // TLSHTTP configures secure transport (Consumer to Query service HTTP API) TLSHTTP tlscfg.Options // AdditionalHeaders AdditionalHeaders http.Header From 657e0b4c75c015ba24967894f0eb7e9c614dbc16 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Sat, 11 Jul 2020 12:15:13 +0530 Subject: [PATCH 06/25] Changed TLS server to use tlsCfg instead of injection Signed-off-by: rjs211 --- cmd/query/app/server.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 01b1bffa65c..336309c3f2a 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -15,9 +15,10 @@ package app import ( + "crypto/rand" + "crypto/tls" "net" "net/http" - "path/filepath" "strings" "github.com/gorilla/handlers" @@ -162,7 +163,11 @@ func (s *Server) Start() error { s.logger.Info("Starting HTTP server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) var err error if s.queryOptions.TLSHTTP.Enabled { - err = s.httpServer.ServeTLS(httpListener, filepath.Clean(s.queryOptions.TLSHTTP.CertPath), filepath.Clean(s.queryOptions.TLSHTTP.KeyPath)) + tlsCfg, _ := s.queryOptions.TLSHTTP.Config() + tlsCfg.Rand = rand.Reader + tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) + + err = s.httpServer.Serve(tlsHTTPListener) } else { err = s.httpServer.Serve(httpListener) From 0f67d154667e516f973aa34bceca019d8590b976 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Sat, 11 Jul 2020 12:18:07 +0530 Subject: [PATCH 07/25] Create test for HTTP server with TLS and MTLS Signed-off-by: rjs211 --- cmd/query/app/server_test.go | 311 +++++++++++++++++++++++++++++++++++ 1 file changed, 311 insertions(+) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 13019d2b084..2584914b8c9 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -16,17 +16,24 @@ package app import ( "context" + "crypto/rand" + "crypto/tls" + "fmt" + "net" + "net/http" "testing" "time" opentracing "github.com/opentracing/opentracing-go" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "go.uber.org/zap" "go.uber.org/zap/zaptest/observer" "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" + "github.com/jaegertracing/jaeger/model" "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/healthcheck" "github.com/jaegertracing/jaeger/ports" @@ -70,6 +77,310 @@ func TestCreateTLSHttpServerError(t *testing.T) { assert.NotNil(t, err) } +func TestTLSHTTPServer(t *testing.T) { + tests := []struct { + name string + serverTLS tlscfg.Options + clientTLS tlscfg.Options + expectError bool + expectServerFail bool + }{ + // { + // name: "", + // serverTLS: tlscfg.Options{ + // Enabled: true, + // CertPath: "invalid/path", + // KeyPath: "invalid/path", + // ClientCAPath: "invalid/path", + // }, + // clientTLS: tlscfg.Options{ + // Enabled: true, + // CertPath: "invalid/path", + // KeyPath: "invalid/path", + // CAPath: "invalid/path", + // ServerName: "localhost", + // }, + // expectError: false, + // expectServerFail: false, + // }, + { + name: "Should pass with insecure connection", + serverTLS: tlscfg.Options{ + Enabled: false, + }, + clientTLS: tlscfg.Options{ + Enabled: false, + }, + expectError: false, + expectServerFail: false, + }, + { + name: "should fail with TLS client to untrusted TLS server", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: "testdata/serverCert.pem", + KeyPath: "testdata/serverkey.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + ServerName: "localhost", + }, + expectError: true, + expectServerFail: false, + }, + { + name: "should fail with TLS client to trusted TLS server with incorrect hostname", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: "testdata/serverCert.pem", + KeyPath: "testdata/serverkey.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: "testdata/CA-cert.pem", + ServerName: "nonEmpty", + }, + expectError: true, + expectServerFail: false, + }, + { + name: "should pass with TLS client to trusted TLS server with correct hostname", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: "testdata/serverCert.pem", + KeyPath: "testdata/serverkey.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: "testdata/CA-cert.pem", + ServerName: "localhost", + }, + expectError: false, + expectServerFail: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + serverOptions := &QueryOptions{TLSHTTP: test.serverTLS} + httpServer, err := createHTTPServer(&querysvc.QueryService{}, serverOptions, opentracing.NoopTracer{}, zap.NewNop()) + + if test.expectServerFail { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.NotNil(t, httpServer) + } + httpListener, err := net.Listen("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)) + if test.expectServerFail { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + if serverOptions.TLSHTTP.Enabled { + + tlsCfg, _ := serverOptions.TLSHTTP.Config() + tlsCfg.Rand = rand.Reader + tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) + go func() { + err := httpServer.Serve(tlsHTTPListener) + + if test.expectServerFail { + + assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) + + } + + }() + defer httpServer.Close() + // defer tlsHTTPListener.Close() + // defer httpListener.Close() + time.Sleep(10 * time.Millisecond) // wait for server to start serving + + clientTLSCfg, err0 := test.clientTLS.Config() + require.NoError(t, err0) + // fmt.Println(tlsCfg.ClientCAs != nil, tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert) + conn, err1 := tls.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) + + if test.expectError { + require.Error(t, err1) + } else { + require.NoError(t, err1) + defer conn.Close() + } + + } else { + go func() { + err := httpServer.Serve(httpListener) + + if test.expectServerFail { + + assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) + + } + // time.Sleep(2 * time.Second) + }() + + time.Sleep(10 * time.Millisecond) // wait for server to start serving + defer httpServer.Close() + conn, err2 := net.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)) + if test.expectError { + require.Error(t, err2) + } else { + require.NoError(t, err2) + defer conn.Close() + } + + } + + }) + } +} + +func TestTLSHTTPServerWithMTLS(t *testing.T) { + tests := []struct { + name string + serverTLS tlscfg.Options + clientTLS tlscfg.Options + expectError bool + expectServerFail bool + expectClientError bool + }{ + { + name: "should fail with TLS client without cert to trusted TLS server requiring cert", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: "testdata/serverCert.pem", + KeyPath: "testdata/serverkey.pem", + ClientCAPath: "testdata/CA-cert.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: "testdata/CA-cert.pem", + ServerName: "localhost", + }, + expectError: false, + expectServerFail: false, + expectClientError: true, + }, + { + name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: "testdata/serverCert.pem", + KeyPath: "testdata/serverkey.pem", + ClientCAPath: "testdata/testCA.pem", // NB: wrong CA + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: "testdata/CA-cert.pem", + ServerName: "localhost", + CertPath: "testdata/clientCert.pem", + KeyPath: "testdata/clientkey.pem", + }, + expectError: false, + expectServerFail: false, + expectClientError: true, + }, + { + name: "should pass with TLS client with cert to trusted TLS server requiring cert", + serverTLS: tlscfg.Options{ + Enabled: true, + CertPath: "testdata/serverCert.pem", + KeyPath: "testdata/serverkey.pem", + ClientCAPath: "testdata/CA-cert.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + CAPath: "testdata/CA-cert.pem", + ServerName: "localhost", + CertPath: "testdata/clientCert.pem", + KeyPath: "testdata/clientkey.pem", + }, + expectError: false, + expectServerFail: false, + expectClientError: false, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + serverOptions := &QueryOptions{TLSHTTP: test.serverTLS} + readMock := &spanstoremocks.Reader{} + serverQuerySvc := querysvc.NewQueryService(readMock, &depsmocks.Reader{}, querysvc.QueryServiceOptions{}) + + httpServer, err := createHTTPServer(serverQuerySvc, serverOptions, opentracing.NoopTracer{}, zap.NewNop()) + + if test.expectServerFail { + require.Error(t, err) + } else { + require.NoError(t, err) + assert.NotNil(t, httpServer) + } + httpListener, err := net.Listen("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)) + if test.expectServerFail { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + tlsCfg, _ := serverOptions.TLSHTTP.Config() + tlsCfg.Rand = rand.Reader + tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) + go func() { + err := httpServer.Serve(tlsHTTPListener) + + if test.expectServerFail { + + assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) + + } + + }() + defer httpServer.Close() + time.Sleep(10 * time.Millisecond) // wait for server to start serving + + clientTLSCfg, err0 := test.clientTLS.Config() + require.NoError(t, err0) + // fmt.Println(tlsCfg.ClientCAs != nil, tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert) + conn, err1 := tls.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) + + if test.expectError { + require.Error(t, err1) + + } else { + require.NoError(t, err1) + conn.Close() + } + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: clientTLSCfg, + }, + } + readMock.On("FindTraces", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("*spanstore.TraceQueryParameters")).Return([]*model.Trace{mockTrace}, nil).Once() + queryString := "/api/traces?service=service&start=0&end=0&operation=operation&limit=200&minDuration=20ms" + req, err := http.NewRequest("GET", "https://localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)+queryString, nil) + if err != nil { + return + } + req.Header.Add("Accept", "application/json") + + resp, err2 := client.Do(req) + + if test.expectClientError { + require.Error(t, err2) + return + + } + require.NoError(t, err2) + + defer resp.Body.Close() + + }) + } +} + func TestServer(t *testing.T) { flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() From 73867a58d8886d0bb04214cf81f12ac823fbdae6 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Sat, 11 Jul 2020 17:59:15 +0530 Subject: [PATCH 08/25] Removing checks to avoid race condition Signed-off-by: rjs211 --- cmd/query/app/server_test.go | 89 +++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 33 deletions(-) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 2584914b8c9..d06eb8ea278 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -184,16 +184,20 @@ func TestTLSHTTPServer(t *testing.T) { tlsCfg.Rand = rand.Reader tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) go func() { - err := httpServer.Serve(tlsHTTPListener) + // err + _ = httpServer.Serve(tlsHTTPListener) + // if err != nil { + // //pass + // } - if test.expectServerFail { + // if test.expectServerFail { - assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) + // //assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) - } + // } }() - defer httpServer.Close() + // defer httpServer.Close() // defer tlsHTTPListener.Close() // defer httpListener.Close() time.Sleep(10 * time.Millisecond) // wait for server to start serving @@ -207,33 +211,49 @@ func TestTLSHTTPServer(t *testing.T) { require.Error(t, err1) } else { require.NoError(t, err1) - defer conn.Close() + //defer conn.Close() + } + if conn != nil { + require.Nil(t, conn.Close()) + } + if httpServer != nil { + require.Nil(t, httpServer.Close()) } } else { go func() { - err := httpServer.Serve(httpListener) + // err : + _ = httpServer.Serve(httpListener) + // if err != nil { + // //pass + // } - if test.expectServerFail { + // if test.expectServerFail { - assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) + // //assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) - } + // } // time.Sleep(2 * time.Second) }() time.Sleep(10 * time.Millisecond) // wait for server to start serving - defer httpServer.Close() + //defer httpServer.Close() conn, err2 := net.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)) if test.expectError { require.Error(t, err2) } else { require.NoError(t, err2) - defer conn.Close() + //defer conn.Close() + } + if conn != nil { + require.Nil(t, conn.Close()) + } + if httpServer != nil { + require.Nil(t, httpServer.Close()) } } - + time.Sleep(50 * time.Millisecond) }) } } @@ -265,12 +285,12 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { expectClientError: true, }, { - name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", + name: "should pass with TLS client with cert to trusted TLS server requiring cert", serverTLS: tlscfg.Options{ Enabled: true, CertPath: "testdata/serverCert.pem", KeyPath: "testdata/serverkey.pem", - ClientCAPath: "testdata/testCA.pem", // NB: wrong CA + ClientCAPath: "testdata/CA-cert.pem", }, clientTLS: tlscfg.Options{ Enabled: true, @@ -281,15 +301,15 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { }, expectError: false, expectServerFail: false, - expectClientError: true, + expectClientError: false, }, { - name: "should pass with TLS client with cert to trusted TLS server requiring cert", + name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", serverTLS: tlscfg.Options{ Enabled: true, CertPath: "testdata/serverCert.pem", KeyPath: "testdata/serverkey.pem", - ClientCAPath: "testdata/CA-cert.pem", + ClientCAPath: "testdata/testCA.pem", // NB: wrong CA }, clientTLS: tlscfg.Options{ Enabled: true, @@ -300,7 +320,7 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { }, expectError: false, expectServerFail: false, - expectClientError: false, + expectClientError: true, }, } @@ -329,21 +349,25 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { tlsCfg.Rand = rand.Reader tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) go func() { - err := httpServer.Serve(tlsHTTPListener) + // err : + _ = httpServer.Serve(tlsHTTPListener) + // if err != nil { + // //pass + // } - if test.expectServerFail { + // if test.expectServerFail { - assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) + // assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) - } + // } }() - defer httpServer.Close() + // defer httpServer.Close() time.Sleep(10 * time.Millisecond) // wait for server to start serving clientTLSCfg, err0 := test.clientTLS.Config() require.NoError(t, err0) - // fmt.Println(tlsCfg.ClientCAs != nil, tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert) + fmt.Println(tlsCfg.ClientCAs != nil, tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert) conn, err1 := tls.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) if test.expectError { @@ -361,26 +385,25 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { readMock.On("FindTraces", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("*spanstore.TraceQueryParameters")).Return([]*model.Trace{mockTrace}, nil).Once() queryString := "/api/traces?service=service&start=0&end=0&operation=operation&limit=200&minDuration=20ms" req, err := http.NewRequest("GET", "https://localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)+queryString, nil) - if err != nil { - return - } + assert.Nil(t, err) req.Header.Add("Accept", "application/json") resp, err2 := client.Do(req) + if err2 == nil { + resp.Body.Close() + } + httpServer.Close() if test.expectClientError { require.Error(t, err2) - return + } else { + require.NoError(t, err2) } - require.NoError(t, err2) - - defer resp.Body.Close() }) } } - func TestServer(t *testing.T) { flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() From a763d9e84a0461b990c5a7436eca50de2bc5e74b Mon Sep 17 00:00:00 2001 From: rjs211 Date: Tue, 14 Jul 2020 15:31:09 +0530 Subject: [PATCH 09/25] Adding testdata of certificates and keys of CA, server & client Signed-off-by: rjs211 --- cmd/query/app/testdata/README.md | 35 +++++++++++++++++++ cmd/query/app/testdata/example-CA-cert.pem | 19 ++++++++++ .../app/testdata/example-client-cert.pem | 17 +++++++++ cmd/query/app/testdata/example-client-key.pem | 27 ++++++++++++++ .../app/testdata/example-server-cert.pem | 17 +++++++++ cmd/query/app/testdata/example-server-key.pem | 27 ++++++++++++++ cmd/query/app/testdata/wrong-CA-cert.pem | 18 ++++++++++ cmd/query/app/testdata/wrong-CA-key.pem | 27 ++++++++++++++ 8 files changed, 187 insertions(+) create mode 100644 cmd/query/app/testdata/README.md create mode 100644 cmd/query/app/testdata/example-CA-cert.pem create mode 100644 cmd/query/app/testdata/example-client-cert.pem create mode 100644 cmd/query/app/testdata/example-client-key.pem create mode 100644 cmd/query/app/testdata/example-server-cert.pem create mode 100644 cmd/query/app/testdata/example-server-key.pem create mode 100644 cmd/query/app/testdata/wrong-CA-cert.pem create mode 100644 cmd/query/app/testdata/wrong-CA-key.pem diff --git a/cmd/query/app/testdata/README.md b/cmd/query/app/testdata/README.md new file mode 100644 index 00000000000..f92acf70111 --- /dev/null +++ b/cmd/query/app/testdata/README.md @@ -0,0 +1,35 @@ +# Example Certificate Authority and Certificate creation for testing + +The following commands were used to create the CA, server and client's certificates and keys + +```bash + +# create CA +openssl genrsa -out example-CA-key.pem 2048 +openssl req -new -key example-CA-key.pem -x509 -days 3650 -out example-CA-cert.pem -subj /CN="example-CA" + +# create Wrong CA (a dummy CA which doesn't provide any certificate ) +openssl genrsa -out wrong-CA-key.pem 2048 +openssl req -new -key wrong-CA-key.pem -x509 -days 3650 -out wrong-CA-cert.pem -subj /CN="wrong-CA" + +# cerating client and server keys +openssl genrsa -out example-server-key.pem 2048 +openssl genrsa -out example-client-key.pem 2048 + +# creating certificate sign request using the above created keys and configuration given and commandline arguemnts +openssl req -new -nodes -key example-server-key.pem -out example-server.csr -subj /CN="example.com" # This server's name is provided as parameter during client's tls configuration +openssl req -new -nodes -key example-client-key.pem -out example-client.csr -subj /CN="example-client" + +# creating the client and server certificate +openssl x509 -req -in example-server.csr -CA example-CA-cert.pem -CAkey example-CA-key.pem -CAcreateserial -out example-server-cert.pem +openssl x509 -req -in example-client.csr -CA example-CA-cert.pem -CAkey example-CA-key.pem -CAcreateserial -out example-client-cert.pem + +# cleanup +rm example-CA-cert.srl +rm example-client.csr +rm example-server.csr +``` + +The server name (common name in the server certificate ) is `example.com` . (in accordance to [RFC 2006](https://tools.ietf.org/html/rfc2606) ) +The common name of the client is `example-client` (never actually unused). +The common name of the Certificate Authority (CA) is `example-CA` . diff --git a/cmd/query/app/testdata/example-CA-cert.pem b/cmd/query/app/testdata/example-CA-cert.pem new file mode 100644 index 00000000000..903848efbeb --- /dev/null +++ b/cmd/query/app/testdata/example-CA-cert.pem @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDADCCAeigAwIBAgIJAKkh9TujwvOJMA0GCSqGSIb3DQEBCwUAMBUxEzARBgNV +BAMMCmV4YW1wbGUtQ0EwHhcNMjAwNzE0MDg1MDM1WhcNMzAwNzEyMDg1MDM1WjAV +MRMwEQYDVQQDDApleGFtcGxlLUNBMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB +CgKCAQEAoDCMfImaVkVR71IqU6WK3wO4qJ+X+arH6W2kADrxjkTjp4zml6ti7wpT +YpDhRmc5/8ib8yd1YnaIFoM4Rx+RE3qrHM3m+3OwT1K33pdg8C4hkmiPQFGez4lE +Ci1O4RTedk4tkaoSYKOu6ktHxw1xezLP4CtCr0vUD5bry7/9An58Y18fRAY5JOb4 +9jOxIL2hzddZHQSRu/i6GqHTtzh6vXLTNGYaXA2IL88grt4B0dphevjVYc8MjGfW +8OEm30SeqY8GFJk3vrGbln5XHz6Jd2gnOdhPVGpS1IGf1zumJIRsYQz7VPd7Tp8j +N+HfbOKktkXjaxOEQShyUbU2eOT2UwIDAQABo1MwUTAdBgNVHQ4EFgQUSwc/w6qn +AnyrcT72YrvkrfUfd1wwHwYDVR0jBBgwFoAUSwc/w6qnAnyrcT72YrvkrfUfd1ww +DwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAUI3+SG+SMxboek63 +pa7Dapg/fRRcpIdUKb2oPStX8fH0YAHnrY3g2bUWwyZ7QxjYAi8P/QsAhSHgxhQm +WUwzl5NwdpanXwyjIgj72ydVIbaZ7YEeaE74accxF/uYIXmU53hf8T/XKQJhLMht +LmgH4HvXqPbj185XRRK41guSRoxmOkqPXTPG+m65NuunOAltImb5E2fmT9RW9WRd +ll2BgngA4Ue7ssed7Io5DCuVrZiamgGbfKZHiLxGTmCEiwi+5a+bBir7/EM75KJ6 +my2U81YEYU+T4latOsUVKNwokTOISx6WsQ/Lo37XiUsxjQqCi/U2woLxU7e90VV5 +NZHhDg== +-----END CERTIFICATE----- diff --git a/cmd/query/app/testdata/example-client-cert.pem b/cmd/query/app/testdata/example-client-cert.pem new file mode 100644 index 00000000000..7298ef97c67 --- /dev/null +++ b/cmd/query/app/testdata/example-client-cert.pem @@ -0,0 +1,17 @@ +-----BEGIN CERTIFICATE----- +MIICqjCCAZICCQDlbM3e9VUF1zANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDDApl +eGFtcGxlLUNBMB4XDTIwMDcxNDA4NTAzNloXDTIwMDgxMzA4NTAzNlowGTEXMBUG +A1UEAwwOZXhhbXBsZS1jbGllbnQwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK +AoIBAQDT/Mw0duVSuw0SNKm1KdmBJbSBZtAlgR8jNiD+Y5vIYN4LaPA9gB/LSH7h +ZYWodXbcgsYvbrgg+E1q6dLlAFaV9dHx6V9gUjaprEc0P2reXwb8xMV8PdEa107F +B0fTxWLGX3Gac3ILVOHn8jo2IeFmvlhWKC3EyPdxcbsbXOW9GzK9sF45C5jd0c6f +RSCvJuIpkc+H77isN6bc5wsbMJh2u+sWOXGZLJpaoKTEtDwn19eCLDBzRYZ95xGw +We/2IraAOqbbznfu6bCmcfOb4OR39ky1Ki7IxiNvDnJG7rIT1Nj4oUXg0hLKtzLW +8qWpesIcJBxBjumhQSLFdQhe804nAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAAqV +bGOpHZ+KpfD1vzYRLc9pOKOqvhqsVtnx+cFKI2N2sBnVFjNd9i/eojmkTSqIFYzD +ZXsMN13FSaIdeyufaCmQzFXFBttXd73M2dtKmvPS729s0P2197VTir32du+iCMdr +7erHGW1GJjLWaYoIQRNbNFTkSrRNrNTdPwKja59smj3DOMSCtmTqPcZVsBCsYtBl +TsJkEQLq5/yZAS9L0r7Ng57xJzM7hLzhZwkTVSuzXD8ZRQo/GpdPTQCLZPz+3Z78 +X7Ate8MJ3L30lYXr7m86WfvOnk7yc+SI7JmbpQQrunEGyY6IjGNUQd6vosz9bb6e +9HjFzmXUuLL5wAJWJ/U= +-----END CERTIFICATE----- diff --git a/cmd/query/app/testdata/example-client-key.pem b/cmd/query/app/testdata/example-client-key.pem new file mode 100644 index 00000000000..e407760f411 --- /dev/null +++ b/cmd/query/app/testdata/example-client-key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpQIBAAKCAQEA0/zMNHblUrsNEjSptSnZgSW0gWbQJYEfIzYg/mObyGDeC2jw +PYAfy0h+4WWFqHV23ILGL264IPhNaunS5QBWlfXR8elfYFI2qaxHND9q3l8G/MTF +fD3RGtdOxQdH08Vixl9xmnNyC1Th5/I6NiHhZr5YVigtxMj3cXG7G1zlvRsyvbBe +OQuY3dHOn0UgrybiKZHPh++4rDem3OcLGzCYdrvrFjlxmSyaWqCkxLQ8J9fXgiww +c0WGfecRsFnv9iK2gDqm28537umwpnHzm+Dkd/ZMtSouyMYjbw5yRu6yE9TY+KFF +4NISyrcy1vKlqXrCHCQcQY7poUEixXUIXvNOJwIDAQABAoIBAQCpfN8Pu1fSc3cZ +7T0aeWFLXogZRch5k+j/UBHFEoLFDJ27ZaVepofFEitPrsnUTEZCO9SUq+NPiPbs +2hIhwcl4kFfRIJN/AXxu4Cz12xZowHVntzNmCE6dNTPnV9DXwmMc08aShGbUJIOR +3RspmKEMcndSO0GMqlkcPKAqWnXcYOQHIzt7y5pnmzIqhacRW18cJ/ebLqaTEaCh +xRYJi4wMJ/WMTkND5io583NPHxH/eeagLz1VunbI9J3++NGhDgV2EuDJj+AMyBwJ +nIs5eSOzTuVP24m0f7bIe3mwVviS1Ce8whg9c1P+6j4eA9nQUgM3nq9egSWO1tH7 +3/laCnMRAoGBAOnObrlxJDZjeYGHYouFMcF6gPAidaqTkHYjM/slQH2rThNOSWaX +bOnBlh5X1HHOFDFGusuCcukvoXPskfEGHYl7nzppmJkw+b8huuUnllaFwZOKtsfw +jPuN33udQ0NsQ/7YVPN3aG/FqD/a1tIeNChCbWxzUy9ihfgjgrYHb+0zAoGBAOgc +KNJFdeipg2Tbz8ZZF6rLbUqXqUgAtlZgnYR2K2i5BRDkshq6aSxuBPWDXbLjDWwz +G88rlrQnbwXNcO4XpRJrfPjKREjlFKGDCBOYCvnq4mCBrPUMEC2dTO50qqLkrCGF +ZLxV6T5nH2YrG+9GO28/xCgYVjom/B9fYl0onxM9AoGAd7sAFrTq33NXcM5814fw +7+ylBNQQv7ZrnyPt5amw+en0xIwtqHzZCGfbUVSW9WScEJPw6nC2GdYLbwrxvaTt +EU+ePdJ/k8txycAjLmB0a4B5ne3GJmN4PwXrMSlanbdepq3v6vH2KG7m9zRBGD7h +ZNCHqqbwLW7dvjGpKUBrv+MCgYEAt/1KZpPlCU8F9rc/lm0dE6g5tD1QVOErqIDh +nBTICDVRzLhcbk+B+1Qi3jMJ3TmadEB3NNvKlHgFy8W0VSetFFLcN6KuSlI87mKd +zlphqQYF+JYSchTj5iSgzQzyDjnSMKg62AgJ4guAmk5l0GAN0PvtFBNmlqY8iOnl +YQ9IOckCgYEAoC4e4qfbChUBxZDcjM0abO39XjnngLHwdi0ihL+05lNxK68qSszF +F0KhEG2hwdNK92ENNVLT6y2hnL66TlShCJv+YgHzBPbciHL6Feo2g3nx44SeAeY1 +ezEJRIUrk4mX3Sf0RdeXrqlAWdMhuQJW2tFQ2paTXLbizGsBh6mn+68= +-----END RSA PRIVATE KEY----- diff --git a/cmd/query/app/testdata/example-server-cert.pem b/cmd/query/app/testdata/example-server-cert.pem new file mode 100644 index 00000000000..b8a967f0eed --- /dev/null +++ b/cmd/query/app/testdata/example-server-cert.pem @@ -0,0 +1,17 @@ +-----BEGIN CERTIFICATE----- +MIICpzCCAY8CCQDlbM3e9VUF1jANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDDApl +eGFtcGxlLUNBMB4XDTIwMDcxNDA4NTAzNloXDTIwMDgxMzA4NTAzNlowFjEUMBIG +A1UEAwwLZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB +AQDQVbU0dTlqdnzQCLq3gWCzd7dm7K3yYejjvRKFF9rSvZ03yyjUiNe+nuGu9nA7 +aaUl9lJGj7r9pS8TvKPd2QCK5qLJDYDxptvtRUSJ+ZNgt9trWYaQpJJpUvJ6ZDMU +ap6uTVj0NywGVx8xIkj25GSHRi59dbyREMiS4Tu52HFDRQ1utV8doidZfKTSrNxz +RbhQ0BMOirzE//urh18HSmJbCdL2JbCoFzFj7FLZX1vpqW785oJF0c2uumfXQYGD +sa3qHcYQkuddh2c/sC8pHHdRbACMlxEmvbYra0fIqLiHWrL4fvayjAIKNfQ1WpZc +aQaJ5Qu4/VTVWaOC08OzdrBLAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAHEcKTKo +Uu2DNCFXkNMzDvtIbnKw442aW3CyeKXT/fP3ArJWJLermLoTGLssRIWk6UuRBOPd +QIYPJnV3QpVkRRSfBQRTauvW0RVFoHr1XeEjXcrJclVQff5tFgIqd9t9HGUyg2Hd +59drgxwqTUnfY4Mf66GaVVi8BovJb48Wa57VzK247DAMUgAJ2sAmP8LH/2r3ee4d +6QZWEiSkb+AYX3LZ76GWglm1eyZnxbwl0StwMcKcPspPFevOFCaj7mqQ4zMVBvsV +J1224Qdfw1PU3p0nFBDsVK0t+fSTj7F/mrz4a5JF5kk4ERsc6GYG4PyRGTjePi8E +EW9BRhqJNkvBz14= +-----END CERTIFICATE----- diff --git a/cmd/query/app/testdata/example-server-key.pem b/cmd/query/app/testdata/example-server-key.pem new file mode 100644 index 00000000000..edebbe34994 --- /dev/null +++ b/cmd/query/app/testdata/example-server-key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEpAIBAAKCAQEA0FW1NHU5anZ80Ai6t4Fgs3e3Zuyt8mHo470ShRfa0r2dN8so +1IjXvp7hrvZwO2mlJfZSRo+6/aUvE7yj3dkAiuaiyQ2A8abb7UVEifmTYLfba1mG +kKSSaVLyemQzFGqerk1Y9DcsBlcfMSJI9uRkh0YufXW8kRDIkuE7udhxQ0UNbrVf +HaInWXyk0qzcc0W4UNATDoq8xP/7q4dfB0piWwnS9iWwqBcxY+xS2V9b6alu/OaC +RdHNrrpn10GBg7Gt6h3GEJLnXYdnP7AvKRx3UWwAjJcRJr22K2tHyKi4h1qy+H72 +sowCCjX0NVqWXGkGieULuP1U1VmjgtPDs3awSwIDAQABAoIBADw7y3wzODpzr3pm +S7Wqjk7VGg2D+HbPoAnYLHaWgwnTEJWFA3UPa3ENdqqjTaefz8O+B+OmwqV7ELVg +IPCPQPzn2cDPSqyTVZqwIDTUF6wq/CF5bNJ9Ame5N6nzNmF8wgwbCNDy8qOSpM4L +35SiS/5gaCiV1cMLWzI8NIxy66xK0O/XdqiSCHehCNjsBjLXK6QHUXHB4IVdp8zt +2gHOWTFUeFp1dPUuB+iVZZerYXMuFSU8i7/Ta5CBJdtS/HzOxatuPyVgZJfrhcoo +CONRuTkyvthS6v8GMM86zXe1dVKH20rgSDNyyCVqmV/nSbl+2qqaHmNpwVy1ihoa +owPCJ4ECgYEA86bjok+3atbc7XVKBq7ruD98lsAqutTCRX6Vfc6fLy/peZbLnqd5 +V7Gd4MbDsx5w2inXm8nJ+uzD9dj1iNt5SIgyWVNIOY5V0qnC9EiY5RGqQXGpyaVx +1eJyR19xzSCrw7R5yRPzNplrb5s8pMvZ3jDlM+ghAL0/ZPTENEW6UbECgYEA2uSe +XPx+CudWzmog3GAjVGKQ91jCqJ2+GedC9UabYQA/gkwWlDWlZtIQ3JOKZ5V0dMlO +w/I7qI0nRJdVihaxhrJxvW4gHuqXe1M2of/E9J8Iz974QlhDEcTX+N15iklgVP23 +bF3eFTTEQ+7motBgMT65W6jCUtFgIzbTcyHKRLsCgYEAlQMfOrhIM7Y+nZda86Vt +Elz5hHT4bRULz1awdKW2YzSJNMyNsXU5V/GP3dWSAG0Aldx7OZL6zVSaMDRFTjL0 +BPEO4eR2SoULZfBfA+mWYJoiJy4tqd5eNXHtdEsiHVL62ZD53okt+NlxhGtLdnoZ +v7Llqo4wCPS03bh4eoOehLECgYBW64GXGwxR+BsYTDxQRjzBPYKjNbPWkQzz7ElC +bI82rYqKivxMTiEn+zKG5G+JQmfEGEwWsxHNlH+LVSy6cNh9zE4uEpTesfkFF74S +BiaHnL87DCc48SNw4uGACTJCH3EJTtaPSZbuhM2KzEr5TBVmeKnSI+lGSa4LTbIZ +fcl8EQKBgQDMqSYeIlb5Rjv4LTdgX6QjySP+OxTCaOJW6+KQoGW+QGCvX2vcqLgs +AZRUpoRcdK3q2gCVbzzpzo968APmm7NbpR6aL7COXakdT829y1viS0MvrV86Z9ZB +O1yZaFy1mR0fICV8KrgrY2yZ6/FoK3LtYMtbRvqQJ/YO85MIXqaLYQ== +-----END RSA PRIVATE KEY----- diff --git a/cmd/query/app/testdata/wrong-CA-cert.pem b/cmd/query/app/testdata/wrong-CA-cert.pem new file mode 100644 index 00000000000..bb18ea2cfff --- /dev/null +++ b/cmd/query/app/testdata/wrong-CA-cert.pem @@ -0,0 +1,18 @@ +-----BEGIN CERTIFICATE----- +MIIC/DCCAeSgAwIBAgIJALgTdRDGjv3/MA0GCSqGSIb3DQEBCwUAMBMxETAPBgNV +BAMMCHdyb25nLUNBMB4XDTIwMDcxNDA4NTAzNloXDTMwMDcxMjA4NTAzNlowEzER +MA8GA1UEAwwId3JvbmctQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB +AQC4aWNDj6MEIqPDHowMl4eYgfibCABBPaHeD8EyCrHmN0UfV5Ot0YJz9vhByM+A +oVvpF2xNZMJfHd8SCpGUfnbfz1LhlWADrcLcXO6lEtbQ6vZqz+OQU66XJ5SMlOi2 +PntNqKOIX3i56+4Y20NJZiIYRulDceptnQU49Tw4IA28P1T2OWl+QFgF+Eg56QIu +DD1VRlS+xljz+zTwD9sT+Fb8ZS3b/e7kiI3IxX6UtibVEprulg6uAoanpU1NBl5W +bn5dQeD7Pa9RUyIuzcHBgaSmh5y0vGKTzg3rrjAicsu0/UsBPD0QERIyngVy/muo +Kh/o9Htgfgj+RDpmwYRGbZ0NAgMBAAGjUzBRMB0GA1UdDgQWBBTKAuMMcCvo528k +c2e4DDpllnlwajAfBgNVHSMEGDAWgBTKAuMMcCvo528kc2e4DDpllnlwajAPBgNV +HRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBuz4xu5DDN3moq00JL7qU7 +gH92bIbXgkWvn4CC4U75fDy2r1bZKYhNwi+N3NlPo02zrfY4+kkoZs63s+2Wcjv1 +yF5zDgUrRrnO/z8YVJwBKr2QFrdTUvRhN5MLET1dZX79xQWRNRlCZSHskSHHc8Z3 +8YU9UvuVgnKv95Ro2DczoF4uHWVRhesf9QpF3m6h4XDf3TRcp4JqdrIJav8AOUka +1/zsmMoHoRggbWszvZZtEiVzJtHFdAx9mjzTzYU3mW/6GnlufHvIzJrs5DtWQxCw +5kYho6ytKJiN7NMoGCxwld7jMUYX332P3MD47kAibli9nXUUuXWuqB4enyUcNu87 +-----END CERTIFICATE----- diff --git a/cmd/query/app/testdata/wrong-CA-key.pem b/cmd/query/app/testdata/wrong-CA-key.pem new file mode 100644 index 00000000000..16e626515cc --- /dev/null +++ b/cmd/query/app/testdata/wrong-CA-key.pem @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAuGljQ4+jBCKjwx6MDJeHmIH4mwgAQT2h3g/BMgqx5jdFH1eT +rdGCc/b4QcjPgKFb6RdsTWTCXx3fEgqRlH52389S4ZVgA63C3FzupRLW0Or2as/j +kFOulyeUjJTotj57TaijiF94uevuGNtDSWYiGEbpQ3HqbZ0FOPU8OCANvD9U9jlp +fkBYBfhIOekCLgw9VUZUvsZY8/s08A/bE/hW/GUt2/3u5IiNyMV+lLYm1RKa7pYO +rgKGp6VNTQZeVm5+XUHg+z2vUVMiLs3BwYGkpoectLxik84N664wInLLtP1LATw9 +EBESMp4Fcv5rqCof6PR7YH4I/kQ6ZsGERm2dDQIDAQABAoIBABrBD2mp2Risfd28 +/MyG32E82fiD/KV6b5Vr67d63bxsoFafugkpsCdq2TGSFKiwwEjc4RWJXLm397kC +V4SXpF9sELYxmbBu8SpPQUApr4i3zfBJINa7jUTYtTCAGhL2laT6pl8OmtE+bVil +1uk7LA8hmo3yZPRqJ2vQCGCMJfH9bub5A/ia7onJPGPXRACW1mBNtlqXUs8t7CXL +D2dS89C/OHU55YxezD5ysKiU8leUyd0kX37iJ4qoLEqb4uTkeftvC1b2KLwl53el +UNcG7khG8q+8P+3iXEg3XasdumvmS2cz0KqabYofi6rjwXW8gJSeckPUI+FJR21L +rNord3ECgYEA7W0GrOTxa6d4podeOEIQC3TZUQY3y9Ic0l0W7OPi7ZD8rz4qATgx +f81J40E2chWjhFemjnbPuN7IAS3LTmmtAhSyj4VeR24/XpoLbCmdADQbl9eNckMf +rSBzRPPMqxp80v0oox7ONpoU3mPQVuCk+4SjDq9xadI6eaMRP6Q0Z/cCgYEAxtai +xZoXe0QeUi/OGs646TP/ac+quw1i32eXUrbb7H9ALsy4mx1qGcuxeu2uJzemMOx+ +8D4L94zGtXv0Xw/9gQZMJa5EOFRpt8zZXmpXvLNUyLIh6oy3JqzCyKbbZZg4i3gW +1fklxnjX1Qjy8fPW0rnbarEXnWkSdRct5Q4hChsCgYAhwcyfLHX+3nLTCpAk44+w +cU6srHumaRtb9Yxa5hPPvzuOFwKV5c1z5FZLD2yUT+tN18CApPfnyxUYxdAQOAo3 +L4YrzcSX117/LlvNZyVBcCc0MZuU0WJhlSmOdjN1aHCy9veeKbUcIlAxRnPKxmnu +sO8WGW/Aoflabgr3x3sEBQKBgQCv1OoH16XgKXKutvuJlTjLcGHyzeNqmgHPESot +yOpeDoFRYVP2R2KmAQynajtRDtL1/IycIiIu/NxTbJPC5L8GM+1ufNZzAaKjGJE3 +/s4rXmIhP/TiFyF3H9r32SW2h4+pNb7r5PDUu9QQ3WMJNtnHavdvN79sQZhC1waM +wZvR9QKBgFpdsAmBd9GPYcUZ2B6gGMf+guPwo8HQyJAahC+9YGNX3BHh4/4wNmpD +AM8i/vzN8UpgEBEzOQpJCEDv1itY1RG/gLPwXP9wqVJapTaBgUsMlEpGN5/FdzLP +izqpeFWpFOSonCgbmz37ihe+FHx/QiKKLpY/ASk2ro0Jn3GvHPmG +-----END RSA PRIVATE KEY----- From 0a88eb5ae7959d4612d90c88878d81bd4d0ffa31 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Tue, 14 Jul 2020 15:34:21 +0530 Subject: [PATCH 10/25] Changing the names of keys and certificates Signed-off-by: rjs211 --- cmd/query/app/server_test.go | 62 ++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index d06eb8ea278..45f5cd3d5db 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -98,7 +98,7 @@ func TestTLSHTTPServer(t *testing.T) { // CertPath: "invalid/path", // KeyPath: "invalid/path", // CAPath: "invalid/path", - // ServerName: "localhost", + // ServerName: "example.com", // }, // expectError: false, // expectServerFail: false, @@ -118,12 +118,12 @@ func TestTLSHTTPServer(t *testing.T) { name: "should fail with TLS client to untrusted TLS server", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/serverCert.pem", - KeyPath: "testdata/serverkey.pem", + CertPath: "testdata/example-server-cert.pem", + KeyPath: "testdata/example-server-key.pem", }, clientTLS: tlscfg.Options{ Enabled: true, - ServerName: "localhost", + ServerName: "example.com", }, expectError: true, expectServerFail: false, @@ -132,12 +132,12 @@ func TestTLSHTTPServer(t *testing.T) { name: "should fail with TLS client to trusted TLS server with incorrect hostname", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/serverCert.pem", - KeyPath: "testdata/serverkey.pem", + CertPath: "testdata/example-server-cert.pem", + KeyPath: "testdata/example-server-key.pem", }, clientTLS: tlscfg.Options{ Enabled: true, - CAPath: "testdata/CA-cert.pem", + CAPath: "testdata/example-CA-cert.pem", ServerName: "nonEmpty", }, expectError: true, @@ -147,13 +147,13 @@ func TestTLSHTTPServer(t *testing.T) { name: "should pass with TLS client to trusted TLS server with correct hostname", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/serverCert.pem", - KeyPath: "testdata/serverkey.pem", + CertPath: "testdata/example-server-cert.pem", + KeyPath: "testdata/example-server-key.pem", }, clientTLS: tlscfg.Options{ Enabled: true, - CAPath: "testdata/CA-cert.pem", - ServerName: "localhost", + CAPath: "testdata/example-CA-cert.pem", + ServerName: "example.com", }, expectError: false, expectServerFail: false, @@ -271,14 +271,14 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { name: "should fail with TLS client without cert to trusted TLS server requiring cert", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/serverCert.pem", - KeyPath: "testdata/serverkey.pem", - ClientCAPath: "testdata/CA-cert.pem", + CertPath: "testdata/example-server-cert.pem", + KeyPath: "testdata/example-server-key.pem", + ClientCAPath: "testdata/example-CA-cert.pem", }, clientTLS: tlscfg.Options{ Enabled: true, - CAPath: "testdata/CA-cert.pem", - ServerName: "localhost", + CAPath: "testdata/example-CA-cert.pem", + ServerName: "example.com", }, expectError: false, expectServerFail: false, @@ -288,16 +288,16 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { name: "should pass with TLS client with cert to trusted TLS server requiring cert", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/serverCert.pem", - KeyPath: "testdata/serverkey.pem", - ClientCAPath: "testdata/CA-cert.pem", + CertPath: "testdata/example-server-cert.pem", + KeyPath: "testdata/example-server-key.pem", + ClientCAPath: "testdata/example-CA-cert.pem", }, clientTLS: tlscfg.Options{ Enabled: true, - CAPath: "testdata/CA-cert.pem", - ServerName: "localhost", - CertPath: "testdata/clientCert.pem", - KeyPath: "testdata/clientkey.pem", + CAPath: "testdata/example-CA-cert.pem", + ServerName: "example.com", + CertPath: "testdata/example-client-cert.pem", + KeyPath: "testdata/example-client-key.pem", }, expectError: false, expectServerFail: false, @@ -307,16 +307,16 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/serverCert.pem", - KeyPath: "testdata/serverkey.pem", - ClientCAPath: "testdata/testCA.pem", // NB: wrong CA + CertPath: "testdata/example-server-cert.pem", + KeyPath: "testdata/example-server-key.pem", + ClientCAPath: "testdata/wrong-CA-cert.pem", // NB: wrong CA }, clientTLS: tlscfg.Options{ Enabled: true, - CAPath: "testdata/CA-cert.pem", - ServerName: "localhost", - CertPath: "testdata/clientCert.pem", - KeyPath: "testdata/clientkey.pem", + CAPath: "testdata/example-CA-cert.pem", + ServerName: "example.com", + CertPath: "testdata/example-client-cert.pem", + KeyPath: "testdata/example-client-key.pem", }, expectError: false, expectServerFail: false, @@ -367,7 +367,7 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { clientTLSCfg, err0 := test.clientTLS.Config() require.NoError(t, err0) - fmt.Println(tlsCfg.ClientCAs != nil, tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert) + // fmt.Println(tlsCfg.ClientCAs != nil, tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert) conn, err1 := tls.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) if test.expectError { From 691ffeaed24c8e13bb5bd38bb1fcbb1a13573d53 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Wed, 15 Jul 2020 08:53:59 +0530 Subject: [PATCH 11/25] Coverage increase and cleanup Signed-off-by: rjs211 --- cmd/query/app/server.go | 10 ++++++---- cmd/query/app/server_test.go | 38 +----------------------------------- 2 files changed, 7 insertions(+), 41 deletions(-) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 336309c3f2a..b67805b8132 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -163,11 +163,13 @@ func (s *Server) Start() error { s.logger.Info("Starting HTTP server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) var err error if s.queryOptions.TLSHTTP.Enabled { - tlsCfg, _ := s.queryOptions.TLSHTTP.Config() - tlsCfg.Rand = rand.Reader - tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) + tlsCfg, err := s.queryOptions.TLSHTTP.Config() + if err == nil { + tlsCfg.Rand = rand.Reader + tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) - err = s.httpServer.Serve(tlsHTTPListener) + err = s.httpServer.Serve(tlsHTTPListener) + } } else { err = s.httpServer.Serve(httpListener) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 45f5cd3d5db..3de803c0855 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -186,32 +186,19 @@ func TestTLSHTTPServer(t *testing.T) { go func() { // err _ = httpServer.Serve(tlsHTTPListener) - // if err != nil { - // //pass - // } - - // if test.expectServerFail { - - // //assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) - - // } }() - // defer httpServer.Close() - // defer tlsHTTPListener.Close() - // defer httpListener.Close() + time.Sleep(10 * time.Millisecond) // wait for server to start serving clientTLSCfg, err0 := test.clientTLS.Config() require.NoError(t, err0) - // fmt.Println(tlsCfg.ClientCAs != nil, tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert) conn, err1 := tls.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) if test.expectError { require.Error(t, err1) } else { require.NoError(t, err1) - //defer conn.Close() } if conn != nil { require.Nil(t, conn.Close()) @@ -224,26 +211,14 @@ func TestTLSHTTPServer(t *testing.T) { go func() { // err : _ = httpServer.Serve(httpListener) - // if err != nil { - // //pass - // } - - // if test.expectServerFail { - - // //assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) - - // } - // time.Sleep(2 * time.Second) }() time.Sleep(10 * time.Millisecond) // wait for server to start serving - //defer httpServer.Close() conn, err2 := net.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)) if test.expectError { require.Error(t, err2) } else { require.NoError(t, err2) - //defer conn.Close() } if conn != nil { require.Nil(t, conn.Close()) @@ -351,23 +326,12 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { go func() { // err : _ = httpServer.Serve(tlsHTTPListener) - // if err != nil { - // //pass - // } - - // if test.expectServerFail { - - // assert.Equal(t, false, (err == nil) || (err == http.ErrServerClosed)) - - // } }() - // defer httpServer.Close() time.Sleep(10 * time.Millisecond) // wait for server to start serving clientTLSCfg, err0 := test.clientTLS.Config() require.NoError(t, err0) - // fmt.Println(tlsCfg.ClientCAs != nil, tlsCfg.ClientAuth == tls.RequireAndVerifyClientCert) conn, err1 := tls.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) if test.expectError { From 5cdf944592d4427298fd88aebe732a09c39a179f Mon Sep 17 00:00:00 2001 From: rjs211 Date: Wed, 15 Jul 2020 09:18:32 +0530 Subject: [PATCH 12/25] removing redundant certif/keys set and using previously available set Signed-off-by: rjs211 --- cmd/query/app/server.go | 6 ++- cmd/query/app/server_test.go | 50 ++++++++++--------- cmd/query/app/testdata/README.md | 35 ------------- cmd/query/app/testdata/example-CA-cert.pem | 19 ------- .../app/testdata/example-client-cert.pem | 17 ------- cmd/query/app/testdata/example-client-key.pem | 27 ---------- .../app/testdata/example-server-cert.pem | 17 ------- cmd/query/app/testdata/example-server-key.pem | 27 ---------- cmd/query/app/testdata/wrong-CA-cert.pem | 18 ------- cmd/query/app/testdata/wrong-CA-key.pem | 27 ---------- 10 files changed, 30 insertions(+), 213 deletions(-) delete mode 100644 cmd/query/app/testdata/README.md delete mode 100644 cmd/query/app/testdata/example-CA-cert.pem delete mode 100644 cmd/query/app/testdata/example-client-cert.pem delete mode 100644 cmd/query/app/testdata/example-client-key.pem delete mode 100644 cmd/query/app/testdata/example-server-cert.pem delete mode 100644 cmd/query/app/testdata/example-server-key.pem delete mode 100644 cmd/query/app/testdata/wrong-CA-cert.pem delete mode 100644 cmd/query/app/testdata/wrong-CA-key.pem diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index b67805b8132..fd28ea61dd7 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -163,12 +163,14 @@ func (s *Server) Start() error { s.logger.Info("Starting HTTP server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) var err error if s.queryOptions.TLSHTTP.Enabled { - tlsCfg, err := s.queryOptions.TLSHTTP.Config() - if err == nil { + tlsCfg, err1 := s.queryOptions.TLSHTTP.Config() + if err1 == nil { tlsCfg.Rand = rand.Reader tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) err = s.httpServer.Serve(tlsHTTPListener) + } else { + err = err1 } } else { diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 3de803c0855..9a413df060b 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -42,6 +42,8 @@ import ( spanstoremocks "github.com/jaegertracing/jaeger/storage/spanstore/mocks" ) +var testCertKeyLocation = "../../../pkg/config/tlscfg/testdata" + func TestServerError(t *testing.T) { srv := &Server{ queryOptions: &QueryOptions{ @@ -118,8 +120,8 @@ func TestTLSHTTPServer(t *testing.T) { name: "should fail with TLS client to untrusted TLS server", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/example-server-cert.pem", - KeyPath: "testdata/example-server-key.pem", + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", }, clientTLS: tlscfg.Options{ Enabled: true, @@ -132,12 +134,12 @@ func TestTLSHTTPServer(t *testing.T) { name: "should fail with TLS client to trusted TLS server with incorrect hostname", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/example-server-cert.pem", - KeyPath: "testdata/example-server-key.pem", + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", }, clientTLS: tlscfg.Options{ Enabled: true, - CAPath: "testdata/example-CA-cert.pem", + CAPath: testCertKeyLocation + "/example-CA-cert.pem", ServerName: "nonEmpty", }, expectError: true, @@ -147,12 +149,12 @@ func TestTLSHTTPServer(t *testing.T) { name: "should pass with TLS client to trusted TLS server with correct hostname", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/example-server-cert.pem", - KeyPath: "testdata/example-server-key.pem", + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", }, clientTLS: tlscfg.Options{ Enabled: true, - CAPath: "testdata/example-CA-cert.pem", + CAPath: testCertKeyLocation + "/example-CA-cert.pem", ServerName: "example.com", }, expectError: false, @@ -246,13 +248,13 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { name: "should fail with TLS client without cert to trusted TLS server requiring cert", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/example-server-cert.pem", - KeyPath: "testdata/example-server-key.pem", - ClientCAPath: "testdata/example-CA-cert.pem", + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", }, clientTLS: tlscfg.Options{ Enabled: true, - CAPath: "testdata/example-CA-cert.pem", + CAPath: testCertKeyLocation + "/example-CA-cert.pem", ServerName: "example.com", }, expectError: false, @@ -263,16 +265,16 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { name: "should pass with TLS client with cert to trusted TLS server requiring cert", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/example-server-cert.pem", - KeyPath: "testdata/example-server-key.pem", - ClientCAPath: "testdata/example-CA-cert.pem", + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", }, clientTLS: tlscfg.Options{ Enabled: true, - CAPath: "testdata/example-CA-cert.pem", + CAPath: testCertKeyLocation + "/example-CA-cert.pem", ServerName: "example.com", - CertPath: "testdata/example-client-cert.pem", - KeyPath: "testdata/example-client-key.pem", + CertPath: testCertKeyLocation + "/example-client-cert.pem", + KeyPath: testCertKeyLocation + "/example-client-key.pem", }, expectError: false, expectServerFail: false, @@ -282,16 +284,16 @@ func TestTLSHTTPServerWithMTLS(t *testing.T) { name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", serverTLS: tlscfg.Options{ Enabled: true, - CertPath: "testdata/example-server-cert.pem", - KeyPath: "testdata/example-server-key.pem", - ClientCAPath: "testdata/wrong-CA-cert.pem", // NB: wrong CA + 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: "testdata/example-CA-cert.pem", + CAPath: testCertKeyLocation + "/example-CA-cert.pem", ServerName: "example.com", - CertPath: "testdata/example-client-cert.pem", - KeyPath: "testdata/example-client-key.pem", + CertPath: testCertKeyLocation + "/example-client-cert.pem", + KeyPath: testCertKeyLocation + "/example-client-key.pem", }, expectError: false, expectServerFail: false, diff --git a/cmd/query/app/testdata/README.md b/cmd/query/app/testdata/README.md deleted file mode 100644 index f92acf70111..00000000000 --- a/cmd/query/app/testdata/README.md +++ /dev/null @@ -1,35 +0,0 @@ -# Example Certificate Authority and Certificate creation for testing - -The following commands were used to create the CA, server and client's certificates and keys - -```bash - -# create CA -openssl genrsa -out example-CA-key.pem 2048 -openssl req -new -key example-CA-key.pem -x509 -days 3650 -out example-CA-cert.pem -subj /CN="example-CA" - -# create Wrong CA (a dummy CA which doesn't provide any certificate ) -openssl genrsa -out wrong-CA-key.pem 2048 -openssl req -new -key wrong-CA-key.pem -x509 -days 3650 -out wrong-CA-cert.pem -subj /CN="wrong-CA" - -# cerating client and server keys -openssl genrsa -out example-server-key.pem 2048 -openssl genrsa -out example-client-key.pem 2048 - -# creating certificate sign request using the above created keys and configuration given and commandline arguemnts -openssl req -new -nodes -key example-server-key.pem -out example-server.csr -subj /CN="example.com" # This server's name is provided as parameter during client's tls configuration -openssl req -new -nodes -key example-client-key.pem -out example-client.csr -subj /CN="example-client" - -# creating the client and server certificate -openssl x509 -req -in example-server.csr -CA example-CA-cert.pem -CAkey example-CA-key.pem -CAcreateserial -out example-server-cert.pem -openssl x509 -req -in example-client.csr -CA example-CA-cert.pem -CAkey example-CA-key.pem -CAcreateserial -out example-client-cert.pem - -# cleanup -rm example-CA-cert.srl -rm example-client.csr -rm example-server.csr -``` - -The server name (common name in the server certificate ) is `example.com` . (in accordance to [RFC 2006](https://tools.ietf.org/html/rfc2606) ) -The common name of the client is `example-client` (never actually unused). -The common name of the Certificate Authority (CA) is `example-CA` . diff --git a/cmd/query/app/testdata/example-CA-cert.pem b/cmd/query/app/testdata/example-CA-cert.pem deleted file mode 100644 index 903848efbeb..00000000000 --- a/cmd/query/app/testdata/example-CA-cert.pem +++ /dev/null @@ -1,19 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIDADCCAeigAwIBAgIJAKkh9TujwvOJMA0GCSqGSIb3DQEBCwUAMBUxEzARBgNV -BAMMCmV4YW1wbGUtQ0EwHhcNMjAwNzE0MDg1MDM1WhcNMzAwNzEyMDg1MDM1WjAV -MRMwEQYDVQQDDApleGFtcGxlLUNBMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIB -CgKCAQEAoDCMfImaVkVR71IqU6WK3wO4qJ+X+arH6W2kADrxjkTjp4zml6ti7wpT -YpDhRmc5/8ib8yd1YnaIFoM4Rx+RE3qrHM3m+3OwT1K33pdg8C4hkmiPQFGez4lE -Ci1O4RTedk4tkaoSYKOu6ktHxw1xezLP4CtCr0vUD5bry7/9An58Y18fRAY5JOb4 -9jOxIL2hzddZHQSRu/i6GqHTtzh6vXLTNGYaXA2IL88grt4B0dphevjVYc8MjGfW -8OEm30SeqY8GFJk3vrGbln5XHz6Jd2gnOdhPVGpS1IGf1zumJIRsYQz7VPd7Tp8j -N+HfbOKktkXjaxOEQShyUbU2eOT2UwIDAQABo1MwUTAdBgNVHQ4EFgQUSwc/w6qn -AnyrcT72YrvkrfUfd1wwHwYDVR0jBBgwFoAUSwc/w6qnAnyrcT72YrvkrfUfd1ww -DwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAOCAQEAUI3+SG+SMxboek63 -pa7Dapg/fRRcpIdUKb2oPStX8fH0YAHnrY3g2bUWwyZ7QxjYAi8P/QsAhSHgxhQm -WUwzl5NwdpanXwyjIgj72ydVIbaZ7YEeaE74accxF/uYIXmU53hf8T/XKQJhLMht -LmgH4HvXqPbj185XRRK41guSRoxmOkqPXTPG+m65NuunOAltImb5E2fmT9RW9WRd -ll2BgngA4Ue7ssed7Io5DCuVrZiamgGbfKZHiLxGTmCEiwi+5a+bBir7/EM75KJ6 -my2U81YEYU+T4latOsUVKNwokTOISx6WsQ/Lo37XiUsxjQqCi/U2woLxU7e90VV5 -NZHhDg== ------END CERTIFICATE----- diff --git a/cmd/query/app/testdata/example-client-cert.pem b/cmd/query/app/testdata/example-client-cert.pem deleted file mode 100644 index 7298ef97c67..00000000000 --- a/cmd/query/app/testdata/example-client-cert.pem +++ /dev/null @@ -1,17 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICqjCCAZICCQDlbM3e9VUF1zANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDDApl -eGFtcGxlLUNBMB4XDTIwMDcxNDA4NTAzNloXDTIwMDgxMzA4NTAzNlowGTEXMBUG -A1UEAwwOZXhhbXBsZS1jbGllbnQwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK -AoIBAQDT/Mw0duVSuw0SNKm1KdmBJbSBZtAlgR8jNiD+Y5vIYN4LaPA9gB/LSH7h -ZYWodXbcgsYvbrgg+E1q6dLlAFaV9dHx6V9gUjaprEc0P2reXwb8xMV8PdEa107F -B0fTxWLGX3Gac3ILVOHn8jo2IeFmvlhWKC3EyPdxcbsbXOW9GzK9sF45C5jd0c6f -RSCvJuIpkc+H77isN6bc5wsbMJh2u+sWOXGZLJpaoKTEtDwn19eCLDBzRYZ95xGw -We/2IraAOqbbznfu6bCmcfOb4OR39ky1Ki7IxiNvDnJG7rIT1Nj4oUXg0hLKtzLW -8qWpesIcJBxBjumhQSLFdQhe804nAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAAqV -bGOpHZ+KpfD1vzYRLc9pOKOqvhqsVtnx+cFKI2N2sBnVFjNd9i/eojmkTSqIFYzD -ZXsMN13FSaIdeyufaCmQzFXFBttXd73M2dtKmvPS729s0P2197VTir32du+iCMdr -7erHGW1GJjLWaYoIQRNbNFTkSrRNrNTdPwKja59smj3DOMSCtmTqPcZVsBCsYtBl -TsJkEQLq5/yZAS9L0r7Ng57xJzM7hLzhZwkTVSuzXD8ZRQo/GpdPTQCLZPz+3Z78 -X7Ate8MJ3L30lYXr7m86WfvOnk7yc+SI7JmbpQQrunEGyY6IjGNUQd6vosz9bb6e -9HjFzmXUuLL5wAJWJ/U= ------END CERTIFICATE----- diff --git a/cmd/query/app/testdata/example-client-key.pem b/cmd/query/app/testdata/example-client-key.pem deleted file mode 100644 index e407760f411..00000000000 --- a/cmd/query/app/testdata/example-client-key.pem +++ /dev/null @@ -1,27 +0,0 @@ ------BEGIN RSA PRIVATE KEY----- -MIIEpQIBAAKCAQEA0/zMNHblUrsNEjSptSnZgSW0gWbQJYEfIzYg/mObyGDeC2jw -PYAfy0h+4WWFqHV23ILGL264IPhNaunS5QBWlfXR8elfYFI2qaxHND9q3l8G/MTF -fD3RGtdOxQdH08Vixl9xmnNyC1Th5/I6NiHhZr5YVigtxMj3cXG7G1zlvRsyvbBe -OQuY3dHOn0UgrybiKZHPh++4rDem3OcLGzCYdrvrFjlxmSyaWqCkxLQ8J9fXgiww -c0WGfecRsFnv9iK2gDqm28537umwpnHzm+Dkd/ZMtSouyMYjbw5yRu6yE9TY+KFF -4NISyrcy1vKlqXrCHCQcQY7poUEixXUIXvNOJwIDAQABAoIBAQCpfN8Pu1fSc3cZ -7T0aeWFLXogZRch5k+j/UBHFEoLFDJ27ZaVepofFEitPrsnUTEZCO9SUq+NPiPbs -2hIhwcl4kFfRIJN/AXxu4Cz12xZowHVntzNmCE6dNTPnV9DXwmMc08aShGbUJIOR -3RspmKEMcndSO0GMqlkcPKAqWnXcYOQHIzt7y5pnmzIqhacRW18cJ/ebLqaTEaCh -xRYJi4wMJ/WMTkND5io583NPHxH/eeagLz1VunbI9J3++NGhDgV2EuDJj+AMyBwJ -nIs5eSOzTuVP24m0f7bIe3mwVviS1Ce8whg9c1P+6j4eA9nQUgM3nq9egSWO1tH7 -3/laCnMRAoGBAOnObrlxJDZjeYGHYouFMcF6gPAidaqTkHYjM/slQH2rThNOSWaX -bOnBlh5X1HHOFDFGusuCcukvoXPskfEGHYl7nzppmJkw+b8huuUnllaFwZOKtsfw -jPuN33udQ0NsQ/7YVPN3aG/FqD/a1tIeNChCbWxzUy9ihfgjgrYHb+0zAoGBAOgc -KNJFdeipg2Tbz8ZZF6rLbUqXqUgAtlZgnYR2K2i5BRDkshq6aSxuBPWDXbLjDWwz -G88rlrQnbwXNcO4XpRJrfPjKREjlFKGDCBOYCvnq4mCBrPUMEC2dTO50qqLkrCGF -ZLxV6T5nH2YrG+9GO28/xCgYVjom/B9fYl0onxM9AoGAd7sAFrTq33NXcM5814fw -7+ylBNQQv7ZrnyPt5amw+en0xIwtqHzZCGfbUVSW9WScEJPw6nC2GdYLbwrxvaTt -EU+ePdJ/k8txycAjLmB0a4B5ne3GJmN4PwXrMSlanbdepq3v6vH2KG7m9zRBGD7h -ZNCHqqbwLW7dvjGpKUBrv+MCgYEAt/1KZpPlCU8F9rc/lm0dE6g5tD1QVOErqIDh -nBTICDVRzLhcbk+B+1Qi3jMJ3TmadEB3NNvKlHgFy8W0VSetFFLcN6KuSlI87mKd -zlphqQYF+JYSchTj5iSgzQzyDjnSMKg62AgJ4guAmk5l0GAN0PvtFBNmlqY8iOnl -YQ9IOckCgYEAoC4e4qfbChUBxZDcjM0abO39XjnngLHwdi0ihL+05lNxK68qSszF -F0KhEG2hwdNK92ENNVLT6y2hnL66TlShCJv+YgHzBPbciHL6Feo2g3nx44SeAeY1 -ezEJRIUrk4mX3Sf0RdeXrqlAWdMhuQJW2tFQ2paTXLbizGsBh6mn+68= ------END RSA PRIVATE KEY----- diff --git a/cmd/query/app/testdata/example-server-cert.pem b/cmd/query/app/testdata/example-server-cert.pem deleted file mode 100644 index b8a967f0eed..00000000000 --- a/cmd/query/app/testdata/example-server-cert.pem +++ /dev/null @@ -1,17 +0,0 @@ ------BEGIN CERTIFICATE----- -MIICpzCCAY8CCQDlbM3e9VUF1jANBgkqhkiG9w0BAQsFADAVMRMwEQYDVQQDDApl -eGFtcGxlLUNBMB4XDTIwMDcxNDA4NTAzNloXDTIwMDgxMzA4NTAzNlowFjEUMBIG -A1UEAwwLZXhhbXBsZS5jb20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB -AQDQVbU0dTlqdnzQCLq3gWCzd7dm7K3yYejjvRKFF9rSvZ03yyjUiNe+nuGu9nA7 -aaUl9lJGj7r9pS8TvKPd2QCK5qLJDYDxptvtRUSJ+ZNgt9trWYaQpJJpUvJ6ZDMU -ap6uTVj0NywGVx8xIkj25GSHRi59dbyREMiS4Tu52HFDRQ1utV8doidZfKTSrNxz -RbhQ0BMOirzE//urh18HSmJbCdL2JbCoFzFj7FLZX1vpqW785oJF0c2uumfXQYGD -sa3qHcYQkuddh2c/sC8pHHdRbACMlxEmvbYra0fIqLiHWrL4fvayjAIKNfQ1WpZc -aQaJ5Qu4/VTVWaOC08OzdrBLAgMBAAEwDQYJKoZIhvcNAQELBQADggEBAHEcKTKo -Uu2DNCFXkNMzDvtIbnKw442aW3CyeKXT/fP3ArJWJLermLoTGLssRIWk6UuRBOPd -QIYPJnV3QpVkRRSfBQRTauvW0RVFoHr1XeEjXcrJclVQff5tFgIqd9t9HGUyg2Hd -59drgxwqTUnfY4Mf66GaVVi8BovJb48Wa57VzK247DAMUgAJ2sAmP8LH/2r3ee4d -6QZWEiSkb+AYX3LZ76GWglm1eyZnxbwl0StwMcKcPspPFevOFCaj7mqQ4zMVBvsV -J1224Qdfw1PU3p0nFBDsVK0t+fSTj7F/mrz4a5JF5kk4ERsc6GYG4PyRGTjePi8E -EW9BRhqJNkvBz14= ------END CERTIFICATE----- diff --git a/cmd/query/app/testdata/example-server-key.pem b/cmd/query/app/testdata/example-server-key.pem deleted file mode 100644 index edebbe34994..00000000000 --- a/cmd/query/app/testdata/example-server-key.pem +++ /dev/null @@ -1,27 +0,0 @@ ------BEGIN RSA PRIVATE KEY----- -MIIEpAIBAAKCAQEA0FW1NHU5anZ80Ai6t4Fgs3e3Zuyt8mHo470ShRfa0r2dN8so -1IjXvp7hrvZwO2mlJfZSRo+6/aUvE7yj3dkAiuaiyQ2A8abb7UVEifmTYLfba1mG -kKSSaVLyemQzFGqerk1Y9DcsBlcfMSJI9uRkh0YufXW8kRDIkuE7udhxQ0UNbrVf -HaInWXyk0qzcc0W4UNATDoq8xP/7q4dfB0piWwnS9iWwqBcxY+xS2V9b6alu/OaC -RdHNrrpn10GBg7Gt6h3GEJLnXYdnP7AvKRx3UWwAjJcRJr22K2tHyKi4h1qy+H72 -sowCCjX0NVqWXGkGieULuP1U1VmjgtPDs3awSwIDAQABAoIBADw7y3wzODpzr3pm -S7Wqjk7VGg2D+HbPoAnYLHaWgwnTEJWFA3UPa3ENdqqjTaefz8O+B+OmwqV7ELVg -IPCPQPzn2cDPSqyTVZqwIDTUF6wq/CF5bNJ9Ame5N6nzNmF8wgwbCNDy8qOSpM4L -35SiS/5gaCiV1cMLWzI8NIxy66xK0O/XdqiSCHehCNjsBjLXK6QHUXHB4IVdp8zt -2gHOWTFUeFp1dPUuB+iVZZerYXMuFSU8i7/Ta5CBJdtS/HzOxatuPyVgZJfrhcoo -CONRuTkyvthS6v8GMM86zXe1dVKH20rgSDNyyCVqmV/nSbl+2qqaHmNpwVy1ihoa -owPCJ4ECgYEA86bjok+3atbc7XVKBq7ruD98lsAqutTCRX6Vfc6fLy/peZbLnqd5 -V7Gd4MbDsx5w2inXm8nJ+uzD9dj1iNt5SIgyWVNIOY5V0qnC9EiY5RGqQXGpyaVx -1eJyR19xzSCrw7R5yRPzNplrb5s8pMvZ3jDlM+ghAL0/ZPTENEW6UbECgYEA2uSe -XPx+CudWzmog3GAjVGKQ91jCqJ2+GedC9UabYQA/gkwWlDWlZtIQ3JOKZ5V0dMlO -w/I7qI0nRJdVihaxhrJxvW4gHuqXe1M2of/E9J8Iz974QlhDEcTX+N15iklgVP23 -bF3eFTTEQ+7motBgMT65W6jCUtFgIzbTcyHKRLsCgYEAlQMfOrhIM7Y+nZda86Vt -Elz5hHT4bRULz1awdKW2YzSJNMyNsXU5V/GP3dWSAG0Aldx7OZL6zVSaMDRFTjL0 -BPEO4eR2SoULZfBfA+mWYJoiJy4tqd5eNXHtdEsiHVL62ZD53okt+NlxhGtLdnoZ -v7Llqo4wCPS03bh4eoOehLECgYBW64GXGwxR+BsYTDxQRjzBPYKjNbPWkQzz7ElC -bI82rYqKivxMTiEn+zKG5G+JQmfEGEwWsxHNlH+LVSy6cNh9zE4uEpTesfkFF74S -BiaHnL87DCc48SNw4uGACTJCH3EJTtaPSZbuhM2KzEr5TBVmeKnSI+lGSa4LTbIZ -fcl8EQKBgQDMqSYeIlb5Rjv4LTdgX6QjySP+OxTCaOJW6+KQoGW+QGCvX2vcqLgs -AZRUpoRcdK3q2gCVbzzpzo968APmm7NbpR6aL7COXakdT829y1viS0MvrV86Z9ZB -O1yZaFy1mR0fICV8KrgrY2yZ6/FoK3LtYMtbRvqQJ/YO85MIXqaLYQ== ------END RSA PRIVATE KEY----- diff --git a/cmd/query/app/testdata/wrong-CA-cert.pem b/cmd/query/app/testdata/wrong-CA-cert.pem deleted file mode 100644 index bb18ea2cfff..00000000000 --- a/cmd/query/app/testdata/wrong-CA-cert.pem +++ /dev/null @@ -1,18 +0,0 @@ ------BEGIN CERTIFICATE----- -MIIC/DCCAeSgAwIBAgIJALgTdRDGjv3/MA0GCSqGSIb3DQEBCwUAMBMxETAPBgNV -BAMMCHdyb25nLUNBMB4XDTIwMDcxNDA4NTAzNloXDTMwMDcxMjA4NTAzNlowEzER -MA8GA1UEAwwId3JvbmctQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIB -AQC4aWNDj6MEIqPDHowMl4eYgfibCABBPaHeD8EyCrHmN0UfV5Ot0YJz9vhByM+A -oVvpF2xNZMJfHd8SCpGUfnbfz1LhlWADrcLcXO6lEtbQ6vZqz+OQU66XJ5SMlOi2 -PntNqKOIX3i56+4Y20NJZiIYRulDceptnQU49Tw4IA28P1T2OWl+QFgF+Eg56QIu -DD1VRlS+xljz+zTwD9sT+Fb8ZS3b/e7kiI3IxX6UtibVEprulg6uAoanpU1NBl5W -bn5dQeD7Pa9RUyIuzcHBgaSmh5y0vGKTzg3rrjAicsu0/UsBPD0QERIyngVy/muo -Kh/o9Htgfgj+RDpmwYRGbZ0NAgMBAAGjUzBRMB0GA1UdDgQWBBTKAuMMcCvo528k -c2e4DDpllnlwajAfBgNVHSMEGDAWgBTKAuMMcCvo528kc2e4DDpllnlwajAPBgNV -HRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQBuz4xu5DDN3moq00JL7qU7 -gH92bIbXgkWvn4CC4U75fDy2r1bZKYhNwi+N3NlPo02zrfY4+kkoZs63s+2Wcjv1 -yF5zDgUrRrnO/z8YVJwBKr2QFrdTUvRhN5MLET1dZX79xQWRNRlCZSHskSHHc8Z3 -8YU9UvuVgnKv95Ro2DczoF4uHWVRhesf9QpF3m6h4XDf3TRcp4JqdrIJav8AOUka -1/zsmMoHoRggbWszvZZtEiVzJtHFdAx9mjzTzYU3mW/6GnlufHvIzJrs5DtWQxCw -5kYho6ytKJiN7NMoGCxwld7jMUYX332P3MD47kAibli9nXUUuXWuqB4enyUcNu87 ------END CERTIFICATE----- diff --git a/cmd/query/app/testdata/wrong-CA-key.pem b/cmd/query/app/testdata/wrong-CA-key.pem deleted file mode 100644 index 16e626515cc..00000000000 --- a/cmd/query/app/testdata/wrong-CA-key.pem +++ /dev/null @@ -1,27 +0,0 @@ ------BEGIN RSA PRIVATE KEY----- -MIIEowIBAAKCAQEAuGljQ4+jBCKjwx6MDJeHmIH4mwgAQT2h3g/BMgqx5jdFH1eT -rdGCc/b4QcjPgKFb6RdsTWTCXx3fEgqRlH52389S4ZVgA63C3FzupRLW0Or2as/j -kFOulyeUjJTotj57TaijiF94uevuGNtDSWYiGEbpQ3HqbZ0FOPU8OCANvD9U9jlp -fkBYBfhIOekCLgw9VUZUvsZY8/s08A/bE/hW/GUt2/3u5IiNyMV+lLYm1RKa7pYO -rgKGp6VNTQZeVm5+XUHg+z2vUVMiLs3BwYGkpoectLxik84N664wInLLtP1LATw9 -EBESMp4Fcv5rqCof6PR7YH4I/kQ6ZsGERm2dDQIDAQABAoIBABrBD2mp2Risfd28 -/MyG32E82fiD/KV6b5Vr67d63bxsoFafugkpsCdq2TGSFKiwwEjc4RWJXLm397kC -V4SXpF9sELYxmbBu8SpPQUApr4i3zfBJINa7jUTYtTCAGhL2laT6pl8OmtE+bVil -1uk7LA8hmo3yZPRqJ2vQCGCMJfH9bub5A/ia7onJPGPXRACW1mBNtlqXUs8t7CXL -D2dS89C/OHU55YxezD5ysKiU8leUyd0kX37iJ4qoLEqb4uTkeftvC1b2KLwl53el -UNcG7khG8q+8P+3iXEg3XasdumvmS2cz0KqabYofi6rjwXW8gJSeckPUI+FJR21L -rNord3ECgYEA7W0GrOTxa6d4podeOEIQC3TZUQY3y9Ic0l0W7OPi7ZD8rz4qATgx -f81J40E2chWjhFemjnbPuN7IAS3LTmmtAhSyj4VeR24/XpoLbCmdADQbl9eNckMf -rSBzRPPMqxp80v0oox7ONpoU3mPQVuCk+4SjDq9xadI6eaMRP6Q0Z/cCgYEAxtai -xZoXe0QeUi/OGs646TP/ac+quw1i32eXUrbb7H9ALsy4mx1qGcuxeu2uJzemMOx+ -8D4L94zGtXv0Xw/9gQZMJa5EOFRpt8zZXmpXvLNUyLIh6oy3JqzCyKbbZZg4i3gW -1fklxnjX1Qjy8fPW0rnbarEXnWkSdRct5Q4hChsCgYAhwcyfLHX+3nLTCpAk44+w -cU6srHumaRtb9Yxa5hPPvzuOFwKV5c1z5FZLD2yUT+tN18CApPfnyxUYxdAQOAo3 -L4YrzcSX117/LlvNZyVBcCc0MZuU0WJhlSmOdjN1aHCy9veeKbUcIlAxRnPKxmnu -sO8WGW/Aoflabgr3x3sEBQKBgQCv1OoH16XgKXKutvuJlTjLcGHyzeNqmgHPESot -yOpeDoFRYVP2R2KmAQynajtRDtL1/IycIiIu/NxTbJPC5L8GM+1ufNZzAaKjGJE3 -/s4rXmIhP/TiFyF3H9r32SW2h4+pNb7r5PDUu9QQ3WMJNtnHavdvN79sQZhC1waM -wZvR9QKBgFpdsAmBd9GPYcUZ2B6gGMf+guPwo8HQyJAahC+9YGNX3BHh4/4wNmpD -AM8i/vzN8UpgEBEzOQpJCEDv1itY1RG/gLPwXP9wqVJapTaBgUsMlEpGN5/FdzLP -izqpeFWpFOSonCgbmz37ihe+FHx/QiKKLpY/ASk2ro0Jn3GvHPmG ------END RSA PRIVATE KEY----- From cca70e93fee84b8a8068de1fbd380246dd4f36c6 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Wed, 15 Jul 2020 11:30:49 +0530 Subject: [PATCH 13/25] Added helper function to serve HTTP server Signed-off-by: rjs211 --- cmd/query/app/server.go | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index fd28ea61dd7..36804a4d105 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -132,6 +132,25 @@ func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, }, nil } +func (s *Server) serveHTTP(httpListener net.Listener) error { + var err error + if s.queryOptions.TLSHTTP.Enabled { + tlsCfg, err1 := s.queryOptions.TLSHTTP.Config() + if err1 == nil { + tlsCfg.Rand = rand.Reader + tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) + + err = s.httpServer.Serve(tlsHTTPListener) + } else { + err = err1 + } + + } else { + err = s.httpServer.Serve(httpListener) + } + return err +} + // Start http, GRPC and cmux servers concurrently func (s *Server) Start() error { conn, err := net.Listen("tcp", s.queryOptions.HostPort) @@ -161,23 +180,8 @@ func (s *Server) Start() error { go func() { s.logger.Info("Starting HTTP server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) - var err error - if s.queryOptions.TLSHTTP.Enabled { - tlsCfg, err1 := s.queryOptions.TLSHTTP.Config() - if err1 == nil { - tlsCfg.Rand = rand.Reader - tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) - - err = s.httpServer.Serve(tlsHTTPListener) - } else { - err = err1 - } - - } else { - err = s.httpServer.Serve(httpListener) - } - switch err { + switch err := s.serveHTTP(httpListener); err { case nil, http.ErrServerClosed, cmux.ErrListenerClosed: // normal exit, nothing to do default: From fea7e14b50f6ecf198ab8a1659446d4fa312085b Mon Sep 17 00:00:00 2001 From: rjs211 Date: Thu, 16 Jul 2020 23:47:29 +0530 Subject: [PATCH 14/25] Modify cmux and tests for secure HTTP and GRPC Signed-off-by: rjs211 --- cmd/query/app/server.go | 112 +++++-- cmd/query/app/server_test.go | 600 +++++++++++++++++++++-------------- 2 files changed, 437 insertions(+), 275 deletions(-) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 36804a4d105..33b5301fcce 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -17,8 +17,11 @@ package app import ( "crypto/rand" "crypto/tls" + "fmt" + "io/ioutil" "net" "net/http" + "path/filepath" "strings" "github.com/gorilla/handlers" @@ -26,9 +29,9 @@ import ( "github.com/soheilhy/cmux" "go.uber.org/zap" "google.golang.org/grpc" - "google.golang.org/grpc/credentials" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" + "github.com/jaegertracing/jaeger/pkg/config/tlscfg" "github.com/jaegertracing/jaeger/pkg/healthcheck" "github.com/jaegertracing/jaeger/pkg/netutils" "github.com/jaegertracing/jaeger/pkg/recoveryhandler" @@ -78,19 +81,15 @@ func (s Server) HealthCheckStatus() chan healthcheck.Status { } func createGRPCServer(querySvc *querysvc.QueryService, options *QueryOptions, logger *zap.Logger, tracer opentracing.Tracer) (*grpc.Server, error) { - var grpcOpts []grpc.ServerOption if options.TLSGRPC.Enabled { - tlsCfg, err := options.TLSGRPC.Config() + _, err := options.TLSGRPC.Config() if err != nil { return nil, err } - creds := credentials.NewTLS(tlsCfg) - - grpcOpts = append(grpcOpts, grpc.Creds(creds)) } - server := grpc.NewServer(grpcOpts...) + server := grpc.NewServer() handler := NewGRPCHandler(querySvc, logger, tracer) api_v2.RegisterQueryServiceServer(server, handler) @@ -132,32 +131,93 @@ func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, }, nil } -func (s *Server) serveHTTP(httpListener net.Listener) error { - var err error - if s.queryOptions.TLSHTTP.Enabled { - tlsCfg, err1 := s.queryOptions.TLSHTTP.Config() - if err1 == nil { - tlsCfg.Rand = rand.Reader - tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) +func getTLSListener(listener net.Listener, tlsOptions tlscfg.Options, otherCA string) (net.Listener, error) { // takes otherCA so that the certPool will have the CA of both GRPC and HTTP clients. + tlsCfg, err := tlsOptions.Config() + if err != nil { + return nil, err + } + if otherCA != "" { + caPEM, err := ioutil.ReadFile(filepath.Clean(otherCA)) + if err != nil { + return nil, fmt.Errorf("failed to load CA %s: %w", otherCA, err) + } + if !tlsCfg.ClientCAs.AppendCertsFromPEM(caPEM) { + return nil, fmt.Errorf("failed to parse CA %s", otherCA) + } + } + tlsCfg.Rand = rand.Reader + tlsListener := tls.NewListener(listener, tlsCfg) + return tlsListener, err + +} + +func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { + var httpListener net.Listener + var grpcListener net.Listener + var cmux1 cmux.CMux + // var cmux2 cmux.CMux + var tlsOptions tlscfg.Options + conn, err := net.Listen("tcp", s.queryOptions.HostPort) + s.conn = conn + + if err != nil { + return nil, nil, nil, err + } + + if s.queryOptions.TLSHTTP.Enabled != s.queryOptions.TLSGRPC.Enabled { + cmux1 = cmux.New(conn) + + if !s.queryOptions.TLSHTTP.Enabled { + httpListener = cmux1.Match(cmux.HTTP1Fast()) + tlsOptions = s.queryOptions.TLSGRPC + } else { + grpcListener = cmux1.MatchWithWriters( + cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"), + cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc+proto"), + ) + tlsOptions = s.queryOptions.TLSHTTP + } + tlsListener := cmux1.Match(cmux.Any()) + tlsListener, err = getTLSListener(tlsListener, tlsOptions, "") + if err != nil { + return nil, nil, nil, err + } - err = s.httpServer.Serve(tlsHTTPListener) + // cmux2 = cmux.New(tlsListener) + if s.queryOptions.TLSHTTP.Enabled { + httpListener = tlsListener } else { - err = err1 + grpcListener = tlsListener } } else { - err = s.httpServer.Serve(httpListener) + var muxListener net.Listener + if s.queryOptions.TLSHTTP.Enabled { + muxListener, err = getTLSListener(conn, s.queryOptions.TLSHTTP, s.queryOptions.TLSGRPC.ClientCAPath) + if err != nil { + return nil, nil, nil, err + } + + } else { + muxListener = conn + } + cmux1 = cmux.New(muxListener) + grpcListener = cmux1.MatchWithWriters( + cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"), + cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc+proto"), + ) + httpListener = cmux1.Match(cmux.Any()) } - return err + + return cmux1, httpListener, grpcListener, err } // Start http, GRPC and cmux servers concurrently func (s *Server) Start() error { - conn, err := net.Listen("tcp", s.queryOptions.HostPort) + cmuxServer, httpListener, grpcListener, err := s.getCmux() if err != nil { return err } - s.conn = conn var tcpPort int if port, err := netutils.GetPort(s.conn.Addr()); err == nil { @@ -169,19 +229,11 @@ func (s *Server) Start() error { zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) - // cmux server acts as a reverse-proxy between HTTP and GRPC backends. - cmuxServer := cmux.New(s.conn) - - grpcListener := cmuxServer.MatchWithWriters( - cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"), - cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc+proto"), - ) - httpListener := cmuxServer.Match(cmux.Any()) - go func() { s.logger.Info("Starting HTTP server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HostPort)) + // s.serveHTTP(httpListener); - switch err := s.serveHTTP(httpListener); err { + switch err := s.httpServer.Serve(httpListener); err { case nil, http.ErrServerClosed, cmux.ErrListenerClosed: // normal exit, nothing to do default: diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 9a413df060b..09a92c5e19e 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -16,7 +16,6 @@ package app import ( "context" - "crypto/rand" "crypto/tls" "fmt" "net" @@ -30,6 +29,8 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/zap" "go.uber.org/zap/zaptest/observer" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials" "github.com/jaegertracing/jaeger/cmd/flags" "github.com/jaegertracing/jaeger/cmd/query/app/querysvc" @@ -79,297 +80,406 @@ func TestCreateTLSHttpServerError(t *testing.T) { assert.NotNil(t, err) } -func TestTLSHTTPServer(t *testing.T) { - tests := []struct { - name string - serverTLS tlscfg.Options - clientTLS tlscfg.Options - expectError bool - expectServerFail bool - }{ - // { - // name: "", - // serverTLS: tlscfg.Options{ - // Enabled: true, - // CertPath: "invalid/path", - // KeyPath: "invalid/path", - // ClientCAPath: "invalid/path", - // }, - // clientTLS: tlscfg.Options{ - // Enabled: true, - // CertPath: "invalid/path", - // KeyPath: "invalid/path", - // CAPath: "invalid/path", - // ServerName: "example.com", - // }, - // expectError: false, - // expectServerFail: false, - // }, - { - name: "Should pass with insecure connection", - serverTLS: tlscfg.Options{ - Enabled: false, - }, - clientTLS: tlscfg.Options{ - Enabled: false, - }, - expectError: false, - expectServerFail: false, +var testCases = []struct { + name string + HTTPTLS tlscfg.Options + GRPCTLS tlscfg.Options + clientTLS tlscfg.Options + expectError bool + expectClientError bool + expectServerFail bool +}{ + { + name: "Should pass with insecure connection", + HTTPTLS: tlscfg.Options{ + Enabled: false, }, - { - 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", - }, - expectError: true, - expectServerFail: false, + GRPCTLS: tlscfg.Options{ + Enabled: 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", - }, - expectError: true, - expectServerFail: false, + clientTLS: tlscfg.Options{ + Enabled: 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", - }, - expectError: false, - expectServerFail: false, + expectError: false, + expectClientError: false, + expectServerFail: false, + }, + { + name: "should fail with TLS client to untrusted TLS server", + HTTPTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", }, - } + GRPCTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + }, + clientTLS: tlscfg.Options{ + Enabled: true, + ServerName: "example.com", + }, + expectError: true, + expectClientError: true, + expectServerFail: false, + }, + { + name: "should fail with TLS client to trusted TLS server with incorrect hostname", + HTTPTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + }, + GRPCTLS: 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", + }, + expectError: true, + expectClientError: true, + expectServerFail: false, + }, + { + name: "should pass with TLS client to trusted TLS server with correct hostname", + HTTPTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + }, + GRPCTLS: 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", + }, + expectError: false, + expectClientError: false, + expectServerFail: false, + }, + { + name: "should fail with TLS client without cert to trusted TLS server requiring cert", + HTTPTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + }, + GRPCTLS: 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", + }, + expectError: false, + expectServerFail: false, + expectClientError: true, + }, + { + name: "should pass with TLS client with cert to trusted TLS server requiring cert", + HTTPTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + }, + GRPCTLS: 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", + }, + expectError: false, + expectServerFail: false, + expectClientError: false, + }, + { + name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", + HTTPTLS: 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 + }, + GRPCTLS: 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", + }, + expectError: false, + expectServerFail: false, + expectClientError: true, + }, + { + name: "should pass with TLS client with cert to trusted TLS HTTP server requiring cert and insecure GRPC server", + HTTPTLS: tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + }, + GRPCTLS: tlscfg.Options{ + Enabled: false, + }, + 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", + }, + expectError: false, + expectServerFail: false, + expectClientError: false, + }, + { + name: "should pass with TLS client with cert to trusted GRPC TLS server requiring cert and insecure HTTP server", + HTTPTLS: tlscfg.Options{ + Enabled: false, + }, + GRPCTLS: 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", + }, + expectError: false, + expectServerFail: false, + expectClientError: false, + }, +} + +func TestServerHTTPTLS(t *testing.T) { + tests := testCases + testlen := len(tests) + tests[testlen-1].clientTLS = tlscfg.Options{Enabled: false} + tests[testlen-1].name = "Should pass with insecure HTTP Client and insecure HTTP server with secure GRPC Server" for _, test := range tests { t.Run(test.name, func(t *testing.T) { - serverOptions := &QueryOptions{TLSHTTP: test.serverTLS} - httpServer, err := createHTTPServer(&querysvc.QueryService{}, serverOptions, opentracing.NoopTracer{}, zap.NewNop()) + serverOptions := &QueryOptions{TLSHTTP: test.HTTPTLS, TLSGRPC: test.GRPCTLS, HostPort: ports.PortToHostPort(ports.QueryHTTP), BearerTokenPropagation: true} + flagsSvc := flags.NewService(ports.QueryAdminHTTP) + flagsSvc.Logger = zap.NewNop() + + spanReader := &spanstoremocks.Reader{} + dependencyReader := &depsmocks.Reader{} + expectedServices := []string{"test"} + spanReader.On("GetServices", mock.AnythingOfType("*context.valueCtx")).Return(expectedServices, nil) + + querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{}) + server, err := NewServer(flagsSvc.Logger, querySvc, + serverOptions, + opentracing.NoopTracer{}) + assert.Nil(t, err) + assert.NoError(t, server.Start()) + go func() { + for s := range server.HealthCheckStatus() { + flagsSvc.SetHealthCheckStatus(s) + } + }() - if test.expectServerFail { - require.Error(t, err) - } else { - require.NoError(t, err) - assert.NotNil(t, httpServer) - } - httpListener, err := net.Listen("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)) - if test.expectServerFail { - require.Error(t, err) - } else { - require.NoError(t, err) - } + var clientError error + var clientClose func() error + var clientTLSCfg *tls.Config if serverOptions.TLSHTTP.Enabled { - tlsCfg, _ := serverOptions.TLSHTTP.Config() - tlsCfg.Rand = rand.Reader - tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) - go func() { - // err - _ = httpServer.Serve(tlsHTTPListener) - - }() + var err0 error - time.Sleep(10 * time.Millisecond) // wait for server to start serving - - clientTLSCfg, err0 := test.clientTLS.Config() + clientTLSCfg, err0 = test.clientTLS.Config() require.NoError(t, err0) - conn, err1 := tls.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) - - if test.expectError { - require.Error(t, err1) - } else { - require.NoError(t, err1) - } + dialer := &net.Dialer{Timeout: 2 * time.Second} + conn, err1 := tls.DialWithDialer(dialer, "tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) + clientError = err1 + clientClose = nil if conn != nil { - require.Nil(t, conn.Close()) + clientClose = conn.Close } - if httpServer != nil { - require.Nil(t, httpServer.Close()) + + } else { + + conn, err1 := net.DialTimeout("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), 2*time.Second) + clientError = err1 + clientClose = nil + if conn != nil { + clientClose = conn.Close } + } + if test.expectError { + require.Error(t, clientError) } else { - go func() { - // err : - _ = httpServer.Serve(httpListener) - }() - - time.Sleep(10 * time.Millisecond) // wait for server to start serving - conn, err2 := net.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)) - if test.expectError { + require.NoError(t, clientError) + } + if clientClose != nil { + require.Nil(t, clientClose()) + } + + // defer server.Close() + // fmt.Print(test.HTTPTLS.ClientCAPath == "" ); fmt.Println(test.HTTPTLS.ClientCAPath) + + if test.HTTPTLS.ClientCAPath != "" { + client := &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: clientTLSCfg, + }, + } + readMock := spanReader + readMock.On("FindTraces", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("*spanstore.TraceQueryParameters")).Return([]*model.Trace{mockTrace}, nil).Once() + queryString := "/api/traces?service=service&start=0&end=0&operation=operation&limit=200&minDuration=20ms" + req, err := http.NewRequest("GET", "https://localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)+queryString, nil) + assert.Nil(t, err) + req.Header.Add("Accept", "application/json") + + resp, err2 := client.Do(req) + if err2 == nil { + resp.Body.Close() + } + + if test.expectClientError { require.Error(t, err2) } else { require.NoError(t, err2) } - if conn != nil { - require.Nil(t, conn.Close()) - } - if httpServer != nil { - require.Nil(t, httpServer.Close()) + } + server.Close() + for i := 0; i < 10; i++ { + if flagsSvc.HC().Get() == healthcheck.Unavailable { + break } - + time.Sleep(1 * time.Millisecond) } - time.Sleep(50 * time.Millisecond) + assert.Equal(t, healthcheck.Unavailable, flagsSvc.HC().Get()) + }) } } -func TestTLSHTTPServerWithMTLS(t *testing.T) { - tests := []struct { - name string - serverTLS tlscfg.Options - clientTLS tlscfg.Options - expectError bool - expectServerFail bool - expectClientError bool - }{ - { - 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", - }, - expectError: false, - expectServerFail: false, - expectClientError: 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", - }, - expectError: false, - expectServerFail: false, - expectClientError: 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", - }, - expectError: false, - expectServerFail: false, - expectClientError: true, - }, +func newGRPCClientWithTLS(t *testing.T, addr string, creds credentials.TransportCredentials) *grpcClient { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + var conn *grpc.ClientConn + var err error + + if creds != nil { + conn, err = grpc.DialContext(ctx, addr, grpc.WithTransportCredentials(creds)) + } else { + conn, err = grpc.DialContext(ctx, addr, grpc.WithInsecure()) } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - serverOptions := &QueryOptions{TLSHTTP: test.serverTLS} - readMock := &spanstoremocks.Reader{} - serverQuerySvc := querysvc.NewQueryService(readMock, &depsmocks.Reader{}, querysvc.QueryServiceOptions{}) + require.NoError(t, err) + return &grpcClient{ + QueryServiceClient: api_v2.NewQueryServiceClient(conn), + conn: conn, + } +} - httpServer, err := createHTTPServer(serverQuerySvc, serverOptions, opentracing.NoopTracer{}, zap.NewNop()) +func TestServerGRPCTLS(t *testing.T) { + tests := testCases + testlen := len(tests) + tests[testlen-2].clientTLS = tlscfg.Options{Enabled: false} + tests[testlen-2].name = "Should pass with insecure GRPC Client and insecure GRPC server with secure HTTP Server" - if test.expectServerFail { - require.Error(t, err) - } else { - require.NoError(t, err) - assert.NotNil(t, httpServer) - } - httpListener, err := net.Listen("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)) - if test.expectServerFail { - require.Error(t, err) - } else { - require.NoError(t, err) - } - - tlsCfg, _ := serverOptions.TLSHTTP.Config() - tlsCfg.Rand = rand.Reader - tlsHTTPListener := tls.NewListener(httpListener, tlsCfg) + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + serverOptions := &QueryOptions{TLSHTTP: test.HTTPTLS, TLSGRPC: test.GRPCTLS, HostPort: ports.PortToHostPort(ports.QueryHTTP), BearerTokenPropagation: true} + flagsSvc := flags.NewService(ports.QueryAdminHTTP) + flagsSvc.Logger = zap.NewNop() + + spanReader := &spanstoremocks.Reader{} + dependencyReader := &depsmocks.Reader{} + expectedServices := []string{"test"} + spanReader.On("GetServices", mock.AnythingOfType("*context.valueCtx")).Return(expectedServices, nil) + + querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{}) + server, err := NewServer(flagsSvc.Logger, querySvc, + serverOptions, + opentracing.NoopTracer{}) + assert.Nil(t, err) + assert.NoError(t, server.Start()) go func() { - // err : - _ = httpServer.Serve(tlsHTTPListener) - + for s := range server.HealthCheckStatus() { + flagsSvc.SetHealthCheckStatus(s) + } }() - time.Sleep(10 * time.Millisecond) // wait for server to start serving - clientTLSCfg, err0 := test.clientTLS.Config() - require.NoError(t, err0) - conn, err1 := tls.Dial("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) + var clientError error + var client *grpcClient - if test.expectError { - require.Error(t, err1) + // time.Sleep(10 * time.Millisecond) // wait for server to start serving + + if serverOptions.TLSGRPC.Enabled { + clientTLSCfg, err0 := test.clientTLS.Config() + require.NoError(t, err0) + creds := credentials.NewTLS(clientTLSCfg) + client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryHTTP), creds) } else { - require.NoError(t, err1) - conn.Close() - } - client := &http.Client{ - Transport: &http.Transport{ - TLSClientConfig: clientTLSCfg, - }, + client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryHTTP), nil) } - readMock.On("FindTraces", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("*spanstore.TraceQueryParameters")).Return([]*model.Trace{mockTrace}, nil).Once() - queryString := "/api/traces?service=service&start=0&end=0&operation=operation&limit=200&minDuration=20ms" - req, err := http.NewRequest("GET", "https://localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)+queryString, nil) - assert.Nil(t, err) - req.Header.Add("Accept", "application/json") - resp, err2 := client.Do(req) - if err2 == nil { - resp.Body.Close() - } - httpServer.Close() + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + + res, clientError := client.GetServices(ctx, &api_v2.GetServicesRequest{}) if test.expectClientError { - require.Error(t, err2) + require.Error(t, clientError) } else { - require.NoError(t, err2) - + require.NoError(t, clientError) + assert.Equal(t, expectedServices, res.Services) } - + if client != nil { + require.Nil(t, client.conn.Close()) + } + server.Close() + for i := 0; i < 10; i++ { + if flagsSvc.HC().Get() == healthcheck.Unavailable { + break + } + time.Sleep(1 * time.Millisecond) + } + assert.Equal(t, healthcheck.Unavailable, flagsSvc.HC().Get()) }) } } + func TestServer(t *testing.T) { flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() From 31b31e704ab324d39fa7ee6bd698d2060aaf2efe Mon Sep 17 00:00:00 2001 From: rjs211 Date: Fri, 17 Jul 2020 07:14:14 +0530 Subject: [PATCH 15/25] Fixing testscases for safe re-use Signed-off-by: rjs211 --- cmd/query/app/server_test.go | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 09a92c5e19e..ba5fbc2b349 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -286,8 +286,19 @@ var testCases = []struct { } func TestServerHTTPTLS(t *testing.T) { - tests := testCases - testlen := len(tests) + testlen := len(testCases) + + tests := make([]struct { + name string + HTTPTLS tlscfg.Options + GRPCTLS tlscfg.Options + clientTLS tlscfg.Options + expectError bool + expectClientError bool + expectServerFail bool + }, testlen) + copy(tests, testCases) + tests[testlen-1].clientTLS = tlscfg.Options{Enabled: false} tests[testlen-1].name = "Should pass with insecure HTTP Client and insecure HTTP server with secure GRPC Server" @@ -411,10 +422,21 @@ func newGRPCClientWithTLS(t *testing.T, addr string, creds credentials.Transport } func TestServerGRPCTLS(t *testing.T) { - tests := testCases - testlen := len(tests) + testlen := len(testCases) + + tests := make([]struct { + name string + HTTPTLS tlscfg.Options + GRPCTLS tlscfg.Options + clientTLS tlscfg.Options + expectError bool + expectClientError bool + expectServerFail bool + }, testlen) + copy(tests, testCases) tests[testlen-2].clientTLS = tlscfg.Options{Enabled: false} - tests[testlen-2].name = "Should pass with insecure GRPC Client and insecure GRPC server with secure HTTP Server" + tests[testlen-2].name = "should pass with insecure GRPC Client and insecure GRPC server with secure HTTP Server" + fmt.Println(tests[testlen-1]) for _, test := range tests { t.Run(test.name, func(t *testing.T) { From affb566257971bf2d46319efc306e86e5033f065 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Fri, 17 Jul 2020 14:36:32 +0530 Subject: [PATCH 16/25] Use common certificate flags for GRPC and HTTP Signed-off-by: rjs211 --- cmd/query/app/flags.go | 32 ++++++++++++++++++-------------- cmd/query/app/server_test.go | 2 +- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 0feb473a186..6d0ebb6efe3 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -46,17 +46,13 @@ const ( queryTokenPropagation = "query.bearer-token-propagation" queryAdditionalHeaders = "query.additional-headers" queryMaxClockSkewAdjust = "query.max-clock-skew-adjustment" + queryHTTPTLSEnabled = "query.http.tls.enabled" + queryGRPCTLSEnabled = "query.gprc.tls.enabled" ) -var tlsGRPCFlagsConfig = tlscfg.ServerFlagsConfig{ - Prefix: "query.grpc", - ShowEnabled: true, - ShowClientCA: true, -} - -var tlsHTTPFlagsConfig = tlscfg.ServerFlagsConfig{ - Prefix: "query.http", - ShowEnabled: true, +var tlsFlagsConfig = tlscfg.ServerFlagsConfig{ + Prefix: "query.server", + ShowEnabled: false, ShowClientCA: true, } @@ -92,8 +88,16 @@ func AddFlags(flagSet *flag.FlagSet) { flagSet.String(queryUIConfig, "", "The path to the UI configuration file in JSON format") flagSet.Bool(queryTokenPropagation, false, "Allow propagation of bearer token to be used by storage plugins") flagSet.Duration(queryMaxClockSkewAdjust, time.Second, "The maximum delta by which span timestamps may be adjusted in the UI due to clock skew; set to 0s to disable clock skew adjustments") - tlsGRPCFlagsConfig.AddFlags(flagSet) - tlsHTTPFlagsConfig.AddFlags(flagSet) + flagSet.Bool(queryHTTPTLSEnabled, false, "Enable TLS when talking to the remote HTTP server") + flagSet.Bool(queryGRPCTLSEnabled, false, "Enable TLS when talking to the remote GRPC server") + tlsFlagsConfig.AddFlags(flagSet) +} + +func getOptions(enabled bool, qTLSOptions tlscfg.Options) tlscfg.Options { + if enabled { + return tlscfg.Options{Enabled: enabled, CertPath: qTLSOptions.CertPath, KeyPath: qTLSOptions.KeyPath, ClientCAPath: qTLSOptions.ClientCAPath} + } + return tlscfg.Options{Enabled: enabled} } // InitFromViper initializes QueryOptions with properties from viper @@ -103,9 +107,9 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) *Qu qOpts.StaticAssets = v.GetString(queryStaticFiles) qOpts.UIConfig = v.GetString(queryUIConfig) qOpts.BearerTokenPropagation = v.GetBool(queryTokenPropagation) - qOpts.TLSGRPC = tlsGRPCFlagsConfig.InitFromViper(v) - qOpts.TLSHTTP = tlsHTTPFlagsConfig.InitFromViper(v) - qOpts.MaxClockSkewAdjust = v.GetDuration(queryMaxClockSkewAdjust) + qTLSOptions := tlsFlagsConfig.InitFromViper(v) + qOpts.TLSGRPC = getOptions(v.GetBool(queryGRPCTLSEnabled), qTLSOptions) + qOpts.TLSHTTP = getOptions(v.GetBool(queryHTTPTLSEnabled), qTLSOptions) stringSlice := v.GetStringSlice(queryAdditionalHeaders) headers, err := stringSliceAsHeader(stringSlice) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index ba5fbc2b349..1dbfdc7ac37 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -436,7 +436,7 @@ func TestServerGRPCTLS(t *testing.T) { copy(tests, testCases) tests[testlen-2].clientTLS = tlscfg.Options{Enabled: false} tests[testlen-2].name = "should pass with insecure GRPC Client and insecure GRPC server with secure HTTP Server" - fmt.Println(tests[testlen-1]) + // fmt.Println(tests[testlen-1]) for _, test := range tests { t.Run(test.name, func(t *testing.T) { From 5def2de1b02e00a541f300836aecdfa5c8f99dc8 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Fri, 17 Jul 2020 15:03:33 +0530 Subject: [PATCH 17/25] Use common certificate flags for GRPC and HTTP Signed-off-by: rjs211 --- cmd/query/app/flags.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 6d0ebb6efe3..83f8ae52f40 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -107,10 +107,12 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) *Qu qOpts.StaticAssets = v.GetString(queryStaticFiles) qOpts.UIConfig = v.GetString(queryUIConfig) qOpts.BearerTokenPropagation = v.GetBool(queryTokenPropagation) + qTLSOptions := tlsFlagsConfig.InitFromViper(v) qOpts.TLSGRPC = getOptions(v.GetBool(queryGRPCTLSEnabled), qTLSOptions) qOpts.TLSHTTP = getOptions(v.GetBool(queryHTTPTLSEnabled), qTLSOptions) + qOpts.MaxClockSkewAdjust = v.GetDuration(queryMaxClockSkewAdjust) stringSlice := v.GetStringSlice(queryAdditionalHeaders) headers, err := stringSliceAsHeader(stringSlice) if err != nil { From 1d8133e253bb368b3836c31a8dcd2a5544c45ec0 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Sat, 18 Jul 2020 11:44:23 +0530 Subject: [PATCH 18/25] tempCommit Signed-off-by: rjs211 --- cmd/query/app/flags.go | 30 ++++++++++++++++-------------- cmd/query/app/server.go | 23 ++++++++++------------- pkg/config/tlscfg/flags.go | 6 ++++-- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 83f8ae52f40..3e895406a07 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -48,6 +48,7 @@ const ( queryMaxClockSkewAdjust = "query.max-clock-skew-adjustment" queryHTTPTLSEnabled = "query.http.tls.enabled" queryGRPCTLSEnabled = "query.gprc.tls.enabled" + queryServerClientCA = "query.server.tls.client-ca" ) var tlsFlagsConfig = tlscfg.ServerFlagsConfig{ @@ -68,14 +69,18 @@ type QueryOptions struct { UIConfig string // BearerTokenPropagation activate/deactivate bearer token propagation to storage BearerTokenPropagation bool - // TLSGRPC configures secure transport (Consumer to Query service GRPC API) - TLSGRPC tlscfg.Options - // TLSHTTP configures secure transport (Consumer to Query service HTTP API) - TLSHTTP tlscfg.Options + // HTTPTLSEnabled Enable/Disable secure HTTP connection + HTTPTLSEnabled bool + // GRPCTLSEnabled Enable/Disable secure CRPC connection + GRPCTLSEnabled bool + // TLS configures secure transport + TLS tlscfg.Options // AdditionalHeaders AdditionalHeaders http.Header // MaxClockSkewAdjust is the maximum duration by which jaeger-query will adjust a span MaxClockSkewAdjust time.Duration + + liststring []string } // AddFlags adds flags for QueryOptions @@ -93,13 +98,6 @@ func AddFlags(flagSet *flag.FlagSet) { tlsFlagsConfig.AddFlags(flagSet) } -func getOptions(enabled bool, qTLSOptions tlscfg.Options) tlscfg.Options { - if enabled { - return tlscfg.Options{Enabled: enabled, CertPath: qTLSOptions.CertPath, KeyPath: qTLSOptions.KeyPath, ClientCAPath: qTLSOptions.ClientCAPath} - } - return tlscfg.Options{Enabled: enabled} -} - // InitFromViper initializes QueryOptions with properties from viper func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) *QueryOptions { qOpts.HostPort = ports.GetAddressFromCLIOptions(v.GetInt(queryPort), v.GetString(queryHostPort)) @@ -108,9 +106,13 @@ func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) *Qu qOpts.UIConfig = v.GetString(queryUIConfig) qOpts.BearerTokenPropagation = v.GetBool(queryTokenPropagation) - qTLSOptions := tlsFlagsConfig.InitFromViper(v) - qOpts.TLSGRPC = getOptions(v.GetBool(queryGRPCTLSEnabled), qTLSOptions) - qOpts.TLSHTTP = getOptions(v.GetBool(queryHTTPTLSEnabled), qTLSOptions) + qOpts.TLS = tlsFlagsConfig.InitFromViper(v) + qOpts.TLS.Enabled = false + qOpts.GRPCTLSEnabled = v.GetBool(queryGRPCTLSEnabled) + qOpts.HTTPTLSEnabled = v.GetBool(queryHTTPTLSEnabled) + if qOpts.GRPCTLSEnabled || qOpts.HTTPTLSEnabled { + qOpts.TLS.Enabled = true + } qOpts.MaxClockSkewAdjust = v.GetDuration(queryMaxClockSkewAdjust) stringSlice := v.GetStringSlice(queryAdditionalHeaders) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 33b5301fcce..10f1820a527 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -82,8 +82,8 @@ func (s Server) HealthCheckStatus() chan healthcheck.Status { func createGRPCServer(querySvc *querysvc.QueryService, options *QueryOptions, logger *zap.Logger, tracer opentracing.Tracer) (*grpc.Server, error) { - if options.TLSGRPC.Enabled { - _, err := options.TLSGRPC.Config() + if options.GRPCTLSEnabled { + _, err := options.TLS.Config() if err != nil { return nil, err } @@ -102,8 +102,8 @@ func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, HandlerOptions.Tracer(tracer), } - if queryOpts.TLSHTTP.Enabled { - _, err := queryOpts.TLSHTTP.Config() // This checks if the certificates are correctly provided + if queryOpts.HTTPTLSEnabled { + _, err := queryOpts.TLS.Config() // This checks if the certificates are correctly provided if err != nil { return nil, err } @@ -156,7 +156,6 @@ func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { var grpcListener net.Listener var cmux1 cmux.CMux // var cmux2 cmux.CMux - var tlsOptions tlscfg.Options conn, err := net.Listen("tcp", s.queryOptions.HostPort) s.conn = conn @@ -164,27 +163,25 @@ func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { return nil, nil, nil, err } - if s.queryOptions.TLSHTTP.Enabled != s.queryOptions.TLSGRPC.Enabled { + if s.queryOptions.HTTPTLSEnabled != s.queryOptions.GRPCTLSEnabled { cmux1 = cmux.New(conn) - if !s.queryOptions.TLSHTTP.Enabled { + if !s.queryOptions.HTTPTLSEnabled { httpListener = cmux1.Match(cmux.HTTP1Fast()) - tlsOptions = s.queryOptions.TLSGRPC } else { grpcListener = cmux1.MatchWithWriters( cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"), cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc+proto"), ) - tlsOptions = s.queryOptions.TLSHTTP } tlsListener := cmux1.Match(cmux.Any()) - tlsListener, err = getTLSListener(tlsListener, tlsOptions, "") + tlsListener, err = getTLSListener(tlsListener, s.queryOptions.TLS, "") if err != nil { return nil, nil, nil, err } // cmux2 = cmux.New(tlsListener) - if s.queryOptions.TLSHTTP.Enabled { + if s.queryOptions.HTTPTLSEnabled { httpListener = tlsListener } else { grpcListener = tlsListener @@ -192,8 +189,8 @@ func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { } else { var muxListener net.Listener - if s.queryOptions.TLSHTTP.Enabled { - muxListener, err = getTLSListener(conn, s.queryOptions.TLSHTTP, s.queryOptions.TLSGRPC.ClientCAPath) + if s.queryOptions.HTTPTLSEnabled { + muxListener, err = getTLSListener(conn, s.queryOptions.TLS, s.queryOptions.TLSGRPC.ClientCAPath) if err != nil { return nil, nil, nil, err } diff --git a/pkg/config/tlscfg/flags.go b/pkg/config/tlscfg/flags.go index 8ea42fc0105..2b60df3934e 100644 --- a/pkg/config/tlscfg/flags.go +++ b/pkg/config/tlscfg/flags.go @@ -70,8 +70,10 @@ func (c ServerFlagsConfig) AddFlags(flags *flag.FlagSet) { } flags.String(c.Prefix+tlsCert, "", "Path to a TLS Certificate file, used to identify this server to clients") flags.String(c.Prefix+tlsKey, "", "Path to a TLS Private Key file, used to identify this server to clients") - flags.String(c.Prefix+tlsClientCA, "", "Path to a TLS CA (Certification Authority) file used to verify certificates presented by clients (if unset, all clients are permitted)") - flags.String(c.Prefix+tlsClientCAOld, "", "(deprecated) see --"+c.Prefix+tlsClientCA) + if c.ShowClientCA { + flags.String(c.Prefix+tlsClientCA, "", "Path to a TLS CA (Certification Authority) file used to verify certificates presented by clients (if unset, all clients are permitted)") + flags.String(c.Prefix+tlsClientCAOld, "", "(deprecated) see --"+c.Prefix+tlsClientCA) + } } // InitFromViper creates tls.Config populated with values retrieved from Viper. From 3c0b23c61a8a9dd984525bae13ebe90acdb0ad8a Mon Sep 17 00:00:00 2001 From: rjs211 Date: Sat, 18 Jul 2020 13:59:44 +0530 Subject: [PATCH 19/25] Using same tlsCfg structure for server Signed-off-by: rjs211 --- cmd/query/app/flags.go | 2 - cmd/query/app/server.go | 19 +----- cmd/query/app/server_test.go | 128 ++++++++++++++--------------------- 3 files changed, 55 insertions(+), 94 deletions(-) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 3e895406a07..b611aafd7de 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -79,8 +79,6 @@ type QueryOptions struct { AdditionalHeaders http.Header // MaxClockSkewAdjust is the maximum duration by which jaeger-query will adjust a span MaxClockSkewAdjust time.Duration - - liststring []string } // AddFlags adds flags for QueryOptions diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 10f1820a527..8f6ab1d7ece 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -17,11 +17,8 @@ package app import ( "crypto/rand" "crypto/tls" - "fmt" - "io/ioutil" "net" "net/http" - "path/filepath" "strings" "github.com/gorilla/handlers" @@ -131,20 +128,11 @@ func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, }, nil } -func getTLSListener(listener net.Listener, tlsOptions tlscfg.Options, otherCA string) (net.Listener, error) { // takes otherCA so that the certPool will have the CA of both GRPC and HTTP clients. +func getTLSListener(listener net.Listener, tlsOptions tlscfg.Options) (net.Listener, error) { // takes otherCA so that the certPool will have the CA of both GRPC and HTTP clients. tlsCfg, err := tlsOptions.Config() if err != nil { return nil, err } - if otherCA != "" { - caPEM, err := ioutil.ReadFile(filepath.Clean(otherCA)) - if err != nil { - return nil, fmt.Errorf("failed to load CA %s: %w", otherCA, err) - } - if !tlsCfg.ClientCAs.AppendCertsFromPEM(caPEM) { - return nil, fmt.Errorf("failed to parse CA %s", otherCA) - } - } tlsCfg.Rand = rand.Reader tlsListener := tls.NewListener(listener, tlsCfg) return tlsListener, err @@ -162,7 +150,6 @@ func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { if err != nil { return nil, nil, nil, err } - if s.queryOptions.HTTPTLSEnabled != s.queryOptions.GRPCTLSEnabled { cmux1 = cmux.New(conn) @@ -175,7 +162,7 @@ func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { ) } tlsListener := cmux1.Match(cmux.Any()) - tlsListener, err = getTLSListener(tlsListener, s.queryOptions.TLS, "") + tlsListener, err = getTLSListener(tlsListener, s.queryOptions.TLS) if err != nil { return nil, nil, nil, err } @@ -190,7 +177,7 @@ func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { } else { var muxListener net.Listener if s.queryOptions.HTTPTLSEnabled { - muxListener, err = getTLSListener(conn, s.queryOptions.TLS, s.queryOptions.TLSGRPC.ClientCAPath) + muxListener, err = getTLSListener(conn, s.queryOptions.TLS) if err != nil { return nil, nil, nil, err } diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 1dbfdc7ac37..ba7eedf7bc0 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -63,7 +63,7 @@ func TestCreateTLSGrpcServerError(t *testing.T) { } _, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, - &QueryOptions{TLSGRPC: tlsCfg}, opentracing.NoopTracer{}) + &QueryOptions{GRPCTLSEnabled: true, TLS: tlsCfg}, opentracing.NoopTracer{}) assert.NotNil(t, err) } @@ -76,25 +76,25 @@ func TestCreateTLSHttpServerError(t *testing.T) { } _, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, - &QueryOptions{TLSHTTP: tlsCfg}, opentracing.NoopTracer{}) + &QueryOptions{HTTPTLSEnabled: true, TLS: tlsCfg}, opentracing.NoopTracer{}) assert.NotNil(t, err) } var testCases = []struct { name string - HTTPTLS tlscfg.Options - GRPCTLS tlscfg.Options + TLS tlscfg.Options + HTTPTLSEnabled bool + GRPCTLSEnabled bool clientTLS tlscfg.Options expectError bool expectClientError bool expectServerFail bool }{ { - name: "Should pass with insecure connection", - HTTPTLS: tlscfg.Options{ - Enabled: false, - }, - GRPCTLS: tlscfg.Options{ + name: "Should pass with insecure connection", + HTTPTLSEnabled: false, + GRPCTLSEnabled: false, + TLS: tlscfg.Options{ Enabled: false, }, clientTLS: tlscfg.Options{ @@ -105,13 +105,10 @@ var testCases = []struct { expectServerFail: false, }, { - name: "should fail with TLS client to untrusted TLS server", - HTTPTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - }, - GRPCTLS: tlscfg.Options{ + name: "should fail with TLS client to untrusted TLS server", + HTTPTLSEnabled: true, + GRPCTLSEnabled: true, + TLS: tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", KeyPath: testCertKeyLocation + "/example-server-key.pem", @@ -125,13 +122,10 @@ var testCases = []struct { expectServerFail: false, }, { - name: "should fail with TLS client to trusted TLS server with incorrect hostname", - HTTPTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - }, - GRPCTLS: tlscfg.Options{ + name: "should fail with TLS client to trusted TLS server with incorrect hostname", + HTTPTLSEnabled: true, + GRPCTLSEnabled: true, + TLS: tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", KeyPath: testCertKeyLocation + "/example-server-key.pem", @@ -146,13 +140,10 @@ var testCases = []struct { expectServerFail: false, }, { - name: "should pass with TLS client to trusted TLS server with correct hostname", - HTTPTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - }, - GRPCTLS: tlscfg.Options{ + name: "should pass with TLS client to trusted TLS server with correct hostname", + HTTPTLSEnabled: true, + GRPCTLSEnabled: true, + TLS: tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", KeyPath: testCertKeyLocation + "/example-server-key.pem", @@ -167,14 +158,10 @@ var testCases = []struct { expectServerFail: false, }, { - name: "should fail with TLS client without cert to trusted TLS server requiring cert", - HTTPTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", - }, - GRPCTLS: tlscfg.Options{ + name: "should fail with TLS client without cert to trusted TLS server requiring cert", + HTTPTLSEnabled: true, + GRPCTLSEnabled: true, + TLS: tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", KeyPath: testCertKeyLocation + "/example-server-key.pem", @@ -190,14 +177,10 @@ var testCases = []struct { expectClientError: true, }, { - name: "should pass with TLS client with cert to trusted TLS server requiring cert", - HTTPTLS: tlscfg.Options{ - Enabled: true, - CertPath: testCertKeyLocation + "/example-server-cert.pem", - KeyPath: testCertKeyLocation + "/example-server-key.pem", - ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", - }, - GRPCTLS: tlscfg.Options{ + name: "should pass with TLS client with cert to trusted TLS server requiring cert", + HTTPTLSEnabled: true, + GRPCTLSEnabled: true, + TLS: tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", KeyPath: testCertKeyLocation + "/example-server-key.pem", @@ -215,14 +198,10 @@ var testCases = []struct { expectClientError: false, }, { - name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", - HTTPTLS: 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 - }, - GRPCTLS: tlscfg.Options{ + name: "should fail with TLS client without cert to trusted TLS server requiring cert from a different CA", + HTTPTLSEnabled: true, + GRPCTLSEnabled: true, + TLS: tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", KeyPath: testCertKeyLocation + "/example-server-key.pem", @@ -240,16 +219,15 @@ var testCases = []struct { expectClientError: true, }, { - name: "should pass with TLS client with cert to trusted TLS HTTP server requiring cert and insecure GRPC server", - HTTPTLS: tlscfg.Options{ + name: "should pass with TLS client with cert to trusted TLS HTTP server requiring cert and insecure GRPC server", + HTTPTLSEnabled: true, + GRPCTLSEnabled: false, + TLS: tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", KeyPath: testCertKeyLocation + "/example-server-key.pem", ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", }, - GRPCTLS: tlscfg.Options{ - Enabled: false, - }, clientTLS: tlscfg.Options{ Enabled: true, CAPath: testCertKeyLocation + "/example-CA-cert.pem", @@ -262,11 +240,10 @@ var testCases = []struct { expectClientError: false, }, { - name: "should pass with TLS client with cert to trusted GRPC TLS server requiring cert and insecure HTTP server", - HTTPTLS: tlscfg.Options{ - Enabled: false, - }, - GRPCTLS: tlscfg.Options{ + name: "should pass with TLS client with cert to trusted GRPC TLS server requiring cert and insecure HTTP server", + HTTPTLSEnabled: false, + GRPCTLSEnabled: true, + TLS: tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", KeyPath: testCertKeyLocation + "/example-server-key.pem", @@ -290,8 +267,9 @@ func TestServerHTTPTLS(t *testing.T) { tests := make([]struct { name string - HTTPTLS tlscfg.Options - GRPCTLS tlscfg.Options + TLS tlscfg.Options + HTTPTLSEnabled bool + GRPCTLSEnabled bool clientTLS tlscfg.Options expectError bool expectClientError bool @@ -304,7 +282,7 @@ func TestServerHTTPTLS(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - serverOptions := &QueryOptions{TLSHTTP: test.HTTPTLS, TLSGRPC: test.GRPCTLS, HostPort: ports.PortToHostPort(ports.QueryHTTP), BearerTokenPropagation: true} + serverOptions := &QueryOptions{HTTPTLSEnabled: test.HTTPTLSEnabled, GRPCTLSEnabled: test.GRPCTLSEnabled, TLS: test.TLS, HostPort: ports.PortToHostPort(ports.QueryHTTP), BearerTokenPropagation: true} flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() @@ -329,7 +307,7 @@ func TestServerHTTPTLS(t *testing.T) { var clientClose func() error var clientTLSCfg *tls.Config - if serverOptions.TLSHTTP.Enabled { + if serverOptions.HTTPTLSEnabled { var err0 error @@ -363,9 +341,7 @@ func TestServerHTTPTLS(t *testing.T) { } // defer server.Close() - // fmt.Print(test.HTTPTLS.ClientCAPath == "" ); fmt.Println(test.HTTPTLS.ClientCAPath) - - if test.HTTPTLS.ClientCAPath != "" { + if test.HTTPTLSEnabled && test.TLS.ClientCAPath != "" { client := &http.Client{ Transport: &http.Transport{ TLSClientConfig: clientTLSCfg, @@ -426,8 +402,9 @@ func TestServerGRPCTLS(t *testing.T) { tests := make([]struct { name string - HTTPTLS tlscfg.Options - GRPCTLS tlscfg.Options + TLS tlscfg.Options + HTTPTLSEnabled bool + GRPCTLSEnabled bool clientTLS tlscfg.Options expectError bool expectClientError bool @@ -436,11 +413,10 @@ func TestServerGRPCTLS(t *testing.T) { copy(tests, testCases) tests[testlen-2].clientTLS = tlscfg.Options{Enabled: false} tests[testlen-2].name = "should pass with insecure GRPC Client and insecure GRPC server with secure HTTP Server" - // fmt.Println(tests[testlen-1]) for _, test := range tests { t.Run(test.name, func(t *testing.T) { - serverOptions := &QueryOptions{TLSHTTP: test.HTTPTLS, TLSGRPC: test.GRPCTLS, HostPort: ports.PortToHostPort(ports.QueryHTTP), BearerTokenPropagation: true} + serverOptions := &QueryOptions{HTTPTLSEnabled: test.HTTPTLSEnabled, GRPCTLSEnabled: test.GRPCTLSEnabled, TLS: test.TLS, HostPort: ports.PortToHostPort(ports.QueryHTTP), BearerTokenPropagation: true} flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() @@ -466,7 +442,7 @@ func TestServerGRPCTLS(t *testing.T) { // time.Sleep(10 * time.Millisecond) // wait for server to start serving - if serverOptions.TLSGRPC.Enabled { + if serverOptions.GRPCTLSEnabled { clientTLSCfg, err0 := test.clientTLS.Config() require.NoError(t, err0) creds := credentials.NewTLS(clientTLSCfg) From 7d9e18fa61cd438d0077d1d1e6391dc3aa180941 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Thu, 30 Jul 2020 13:10:10 +0530 Subject: [PATCH 20/25] Removing reduntant code, added comments, using correct port for testing Signed-off-by: rjs211 --- cmd/query/app/flags.go | 5 ++--- cmd/query/app/server.go | 16 +++++++--------- cmd/query/app/server_test.go | 16 ++++++++-------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index b611aafd7de..a151f1facda 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -47,12 +47,11 @@ const ( queryAdditionalHeaders = "query.additional-headers" queryMaxClockSkewAdjust = "query.max-clock-skew-adjustment" queryHTTPTLSEnabled = "query.http.tls.enabled" - queryGRPCTLSEnabled = "query.gprc.tls.enabled" - queryServerClientCA = "query.server.tls.client-ca" + queryGRPCTLSEnabled = "query.grpc.tls.enabled" ) var tlsFlagsConfig = tlscfg.ServerFlagsConfig{ - Prefix: "query.server", + Prefix: "query", ShowEnabled: false, ShowClientCA: true, } diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 8f6ab1d7ece..8dbf62dfa39 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -143,31 +143,29 @@ func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { var httpListener net.Listener var grpcListener net.Listener var cmux1 cmux.CMux - // var cmux2 cmux.CMux conn, err := net.Listen("tcp", s.queryOptions.HostPort) s.conn = conn if err != nil { return nil, nil, nil, err } - if s.queryOptions.HTTPTLSEnabled != s.queryOptions.GRPCTLSEnabled { + if s.queryOptions.HTTPTLSEnabled != s.queryOptions.GRPCTLSEnabled { // exactly one of HTTP or GRPC has TLS enabled cmux1 = cmux.New(conn) - if !s.queryOptions.HTTPTLSEnabled { + if !s.queryOptions.HTTPTLSEnabled { // HTTPTLS disabled GRPCTLS enabled => match http request httpListener = cmux1.Match(cmux.HTTP1Fast()) - } else { - grpcListener = cmux1.MatchWithWriters( + } else { // HTTPTLS enabled GRPCTLS disabled + grpcListener = cmux1.MatchWithWriters( // HTTPTLS disabled GRPCTLS enabled => match GRPC request cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"), cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc+proto"), ) } - tlsListener := cmux1.Match(cmux.Any()) + tlsListener := cmux1.Match(cmux.Any()) // unmatched request are routed to TLS Listener tlsListener, err = getTLSListener(tlsListener, s.queryOptions.TLS) if err != nil { return nil, nil, nil, err } - // cmux2 = cmux.New(tlsListener) if s.queryOptions.HTTPTLSEnabled { httpListener = tlsListener } else { @@ -176,13 +174,13 @@ func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { } else { var muxListener net.Listener - if s.queryOptions.HTTPTLSEnabled { + if s.queryOptions.HTTPTLSEnabled { // Both HTTPTLS and GRPCTLS are enabled Using TLS listener. muxListener, err = getTLSListener(conn, s.queryOptions.TLS) if err != nil { return nil, nil, nil, err } - } else { + } else { // Both HTTPTLS and GRPCTLS are disabled muxListener = conn } cmux1 = cmux.New(muxListener) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index ba7eedf7bc0..ac81ae15cee 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -282,7 +282,7 @@ func TestServerHTTPTLS(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - serverOptions := &QueryOptions{HTTPTLSEnabled: test.HTTPTLSEnabled, GRPCTLSEnabled: test.GRPCTLSEnabled, TLS: test.TLS, HostPort: ports.PortToHostPort(ports.QueryHTTP), BearerTokenPropagation: true} + serverOptions := &QueryOptions{HTTPTLSEnabled: test.HTTPTLSEnabled, GRPCTLSEnabled: test.GRPCTLSEnabled, TLS: test.TLS, HostPort: ports.PortToHostPort(ports.QueryAdminHTTP), BearerTokenPropagation: true} flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() @@ -314,7 +314,7 @@ func TestServerHTTPTLS(t *testing.T) { clientTLSCfg, err0 = test.clientTLS.Config() require.NoError(t, err0) dialer := &net.Dialer{Timeout: 2 * time.Second} - conn, err1 := tls.DialWithDialer(dialer, "tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) + conn, err1 := tls.DialWithDialer(dialer, "tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryAdminHTTP), clientTLSCfg) clientError = err1 clientClose = nil if conn != nil { @@ -323,7 +323,7 @@ func TestServerHTTPTLS(t *testing.T) { } else { - conn, err1 := net.DialTimeout("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), 2*time.Second) + conn, err1 := net.DialTimeout("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryAdminHTTP), 2*time.Second) clientError = err1 clientClose = nil if conn != nil { @@ -350,7 +350,7 @@ func TestServerHTTPTLS(t *testing.T) { readMock := spanReader readMock.On("FindTraces", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("*spanstore.TraceQueryParameters")).Return([]*model.Trace{mockTrace}, nil).Once() queryString := "/api/traces?service=service&start=0&end=0&operation=operation&limit=200&minDuration=20ms" - req, err := http.NewRequest("GET", "https://localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)+queryString, nil) + req, err := http.NewRequest("GET", "https://localhost:"+fmt.Sprintf("%d", ports.QueryAdminHTTP)+queryString, nil) assert.Nil(t, err) req.Header.Add("Accept", "application/json") @@ -416,7 +416,7 @@ func TestServerGRPCTLS(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - serverOptions := &QueryOptions{HTTPTLSEnabled: test.HTTPTLSEnabled, GRPCTLSEnabled: test.GRPCTLSEnabled, TLS: test.TLS, HostPort: ports.PortToHostPort(ports.QueryHTTP), BearerTokenPropagation: true} + serverOptions := &QueryOptions{HTTPTLSEnabled: test.HTTPTLSEnabled, GRPCTLSEnabled: test.GRPCTLSEnabled, TLS: test.TLS, HostPort: ports.PortToHostPort(ports.QueryAdminHTTP), BearerTokenPropagation: true} flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() @@ -446,10 +446,10 @@ func TestServerGRPCTLS(t *testing.T) { clientTLSCfg, err0 := test.clientTLS.Config() require.NoError(t, err0) creds := credentials.NewTLS(clientTLSCfg) - client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryHTTP), creds) + client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryAdminHTTP), creds) } else { - client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryHTTP), nil) + client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryAdminHTTP), nil) } ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) @@ -481,7 +481,7 @@ func TestServerGRPCTLS(t *testing.T) { func TestServer(t *testing.T) { flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() - hostPort := ports.GetAddressFromCLIOptions(ports.QueryHTTP, "") + hostPort := ports.GetAddressFromCLIOptions(ports.QueryAdminHTTP, "") spanReader := &spanstoremocks.Reader{} dependencyReader := &depsmocks.Reader{} From 0ceb4e58f3b04145f7266091f57ca26dc015f73d Mon Sep 17 00:00:00 2001 From: rjs211 Date: Fri, 11 Sep 2020 23:31:01 +0530 Subject: [PATCH 21/25] modified test-cases for dedicated ports with TLS Signed-off-by: rjs211 --- cmd/query/app/flags_test.go | 12 ++++ cmd/query/app/server.go | 78 +++------------------- cmd/query/app/server_test.go | 125 ++++++++++++++++++++++++++--------- 3 files changed, 116 insertions(+), 99 deletions(-) diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go index c79fc2c66a8..d2ea31ecbfc 100644 --- a/cmd/query/app/flags_test.go +++ b/cmd/query/app/flags_test.go @@ -72,6 +72,18 @@ func TestQueryBuilderFlagsSeparatePorts(t *testing.T) { } func TestQueryBuilderFlagsSeparateNoPorts(t *testing.T) { + v, command := config.Viperize(AddFlags) + command.ParseFlags([]string{ + "--query.http.tls.enabled=true", + }) + qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) + + assert.Equal(t, ports.PortToHostPort(ports.QueryHTTP), qOpts.HTTPHostPort) + assert.Equal(t, ports.PortToHostPort(ports.QueryGRPC), qOpts.GRPCHostPort) + assert.NotEqual(t, qOpts.HTTPHostPort, qOpts.GRPCHostPort) +} + +func TestQueryBuilderFlagsSeparateNoPortsWithTLS(t *testing.T) { v, command := config.Viperize(AddFlags) command.ParseFlags([]string{}) qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 10c39d50ed7..4170e9c76af 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -64,7 +64,7 @@ func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, options *Que } if (options.TLSHTTP.Enabled || options.TLSGRPC.Enabled) && (grpcPort == httpPort) { - return nil, errors.New("Server with TLS enabled can not share host ports. Use dedicated HTTP and gRPC host ports instead") + return nil, errors.New("Server with TLS enabled can not use same host ports for gRPC and HTTP. Use dedicated HTTP and gRPC host ports instead") } grpcServer, err := createGRPCServer(querySvc, options, logger, tracer) @@ -148,78 +148,13 @@ func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, Handler: recoveryHandler(handler), TLSConfig: tlsCfg, }, nil + } return &http.Server{ Handler: recoveryHandler(handler), }, nil } -// func getTLSListener(listener net.Listener, tlsOptions tlscfg.Options) (net.Listener, error) { // takes otherCA so that the certPool will have the CA of both GRPC and HTTP clients. -// tlsCfg, err := tlsOptions.Config() -// if err != nil { -// return nil, err -// } -// tlsCfg.Rand = rand.Reader -// tlsListener := tls.NewListener(listener, tlsCfg) -// return tlsListener, err - -// } - -// func (s *Server) getCmux() (cmux.CMux, net.Listener, net.Listener, error) { -// var httpListener net.Listener -// var grpcListener net.Listener -// var cmux1 cmux.CMux -// conn, err := net.Listen("tcp", s.queryOptions.HostPort) -// s.conn = conn - -// if err != nil { -// return nil, nil, nil, err -// } -// if s.queryOptions.HTTPTLSEnabled != s.queryOptions.GRPCTLSEnabled { // exactly one of HTTP or GRPC has TLS enabled -// cmux1 = cmux.New(conn) - -// if !s.queryOptions.HTTPTLSEnabled { // HTTPTLS disabled GRPCTLS enabled => match http request -// httpListener = cmux1.Match(cmux.HTTP1Fast()) -// } else { // HTTPTLS enabled GRPCTLS disabled -// grpcListener = cmux1.MatchWithWriters( // HTTPTLS disabled GRPCTLS enabled => match GRPC request -// cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"), -// cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc+proto"), -// ) -// } -// tlsListener := cmux1.Match(cmux.Any()) // unmatched request are routed to TLS Listener -// tlsListener, err = getTLSListener(tlsListener, s.queryOptions.TLS) -// if err != nil { -// return nil, nil, nil, err -// } - -// if s.queryOptions.HTTPTLSEnabled { -// httpListener = tlsListener -// } else { -// grpcListener = tlsListener -// } - -// } else { -// var muxListener net.Listener -// if s.queryOptions.HTTPTLSEnabled { // Both HTTPTLS and GRPCTLS are enabled Using TLS listener. -// muxListener, err = getTLSListener(conn, s.queryOptions.TLS) -// if err != nil { -// return nil, nil, nil, err -// } - -// } else { // Both HTTPTLS and GRPCTLS are disabled -// muxListener = conn -// } -// cmux1 = cmux.New(muxListener) -// grpcListener = cmux1.MatchWithWriters( -// cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc"), -// cmux.HTTP2MatchHeaderFieldSendSettings("content-type", "application/grpc+proto"), -// ) -// httpListener = cmux1.Match(cmux.Any()) -// } - -// return cmux1, httpListener, grpcListener, err -// } - // initListener initialises listeners of the server func (s *Server) initListener() (cmux.CMux, error) { if s.separatePorts { // use separate ports and listeners each for gRPC and HTTP requests @@ -296,8 +231,13 @@ func (s *Server) Start() error { go func() { s.logger.Info("Starting HTTP server", zap.Int("port", httpPort), zap.String("addr", s.queryOptions.HTTPHostPort)) - - switch err := s.httpServer.Serve(s.httpConn); err { + var err error + if s.queryOptions.TLSHTTP.Enabled { + err = s.httpServer.ServeTLS(s.httpConn, "", "") + } else { + err = s.httpServer.Serve(s.httpConn) + } + switch err { case nil, http.ErrServerClosed, cmux.ErrListenerClosed: // normal exit, nothing to do default: diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 6f4043343bf..fcb9f769b75 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -55,6 +55,19 @@ func TestServerError(t *testing.T) { assert.Error(t, srv.Start()) } +func TestCreateTLSServerSinglePortError(t *testing.T) { + tlsCfg := tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + } + + _, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, + &QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8080", TLSGRPC: tlsCfg, TLSHTTP: tlsCfg}, opentracing.NoopTracer{}) + assert.NotNil(t, err) +} + func TestCreateTLSGrpcServerError(t *testing.T) { tlsCfg := tlscfg.Options{ Enabled: true, @@ -64,7 +77,7 @@ func TestCreateTLSGrpcServerError(t *testing.T) { } _, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, - &QueryOptions{GRPCTLSEnabled: true, TLS: tlsCfg}, opentracing.NoopTracer{}) + &QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSGRPC: tlsCfg}, opentracing.NoopTracer{}) assert.NotNil(t, err) } @@ -77,7 +90,7 @@ func TestCreateTLSHttpServerError(t *testing.T) { } _, err := NewServer(zap.NewNop(), &querysvc.QueryService{}, - &QueryOptions{HTTPTLSEnabled: true, TLS: tlsCfg}, opentracing.NoopTracer{}) + &QueryOptions{HTTPHostPort: ":8080", GRPCHostPort: ":8081", TLSHTTP: tlsCfg}, opentracing.NoopTracer{}) assert.NotNil(t, err) } @@ -280,10 +293,33 @@ func TestServerHTTPTLS(t *testing.T) { tests[testlen-1].clientTLS = tlscfg.Options{Enabled: false} tests[testlen-1].name = "Should pass with insecure HTTP Client and insecure HTTP server with secure GRPC Server" + tests[testlen-1].TLS = tlscfg.Options{ + Enabled: false, + } + + disabledTLSCfg := tlscfg.Options{ + Enabled: false, + } + enabledTLSCfg := tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - serverOptions := &QueryOptions{HTTPTLSEnabled: test.HTTPTLSEnabled, GRPCTLSEnabled: test.GRPCTLSEnabled, TLS: test.TLS, HostPort: ports.PortToHostPort(ports.QueryAdminHTTP), BearerTokenPropagation: true} + TLSGRPC := disabledTLSCfg + if test.GRPCTLSEnabled { + TLSGRPC = enabledTLSCfg + } + + serverOptions := &QueryOptions{ + GRPCHostPort: ports.GetAddressFromCLIOptions(ports.QueryGRPC, ""), + HTTPHostPort: ports.GetAddressFromCLIOptions(ports.QueryHTTP, ""), + TLSHTTP: test.TLS, + TLSGRPC: TLSGRPC, + BearerTokenPropagation: true} flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() @@ -298,9 +334,19 @@ func TestServerHTTPTLS(t *testing.T) { opentracing.NoopTracer{}) assert.Nil(t, err) assert.NoError(t, server.Start()) + + var wg sync.WaitGroup + wg.Add(1) + once := sync.Once{} + go func() { for s := range server.HealthCheckStatus() { - flagsSvc.SetHealthCheckStatus(s) + flagsSvc.HC().Set(s) + if s == healthcheck.Unavailable { + once.Do(func() { + wg.Done() + }) + } } }() @@ -308,14 +354,14 @@ func TestServerHTTPTLS(t *testing.T) { var clientClose func() error var clientTLSCfg *tls.Config - if serverOptions.HTTPTLSEnabled { + if serverOptions.TLSHTTP.Enabled { var err0 error - clientTLSCfg, err0 = test.clientTLS.Config() + clientTLSCfg, err0 = test.clientTLS.Config(zap.NewNop()) require.NoError(t, err0) dialer := &net.Dialer{Timeout: 2 * time.Second} - conn, err1 := tls.DialWithDialer(dialer, "tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryAdminHTTP), clientTLSCfg) + conn, err1 := tls.DialWithDialer(dialer, "tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), clientTLSCfg) clientError = err1 clientClose = nil if conn != nil { @@ -324,7 +370,7 @@ func TestServerHTTPTLS(t *testing.T) { } else { - conn, err1 := net.DialTimeout("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryAdminHTTP), 2*time.Second) + conn, err1 := net.DialTimeout("tcp", "localhost:"+fmt.Sprintf("%d", ports.QueryHTTP), 2*time.Second) clientError = err1 clientClose = nil if conn != nil { @@ -341,7 +387,6 @@ func TestServerHTTPTLS(t *testing.T) { require.Nil(t, clientClose()) } - // defer server.Close() if test.HTTPTLSEnabled && test.TLS.ClientCAPath != "" { client := &http.Client{ Transport: &http.Transport{ @@ -351,7 +396,7 @@ func TestServerHTTPTLS(t *testing.T) { readMock := spanReader readMock.On("FindTraces", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("*spanstore.TraceQueryParameters")).Return([]*model.Trace{mockTrace}, nil).Once() queryString := "/api/traces?service=service&start=0&end=0&operation=operation&limit=200&minDuration=20ms" - req, err := http.NewRequest("GET", "https://localhost:"+fmt.Sprintf("%d", ports.QueryAdminHTTP)+queryString, nil) + req, err := http.NewRequest("GET", "https://localhost:"+fmt.Sprintf("%d", ports.QueryHTTP)+queryString, nil) assert.Nil(t, err) req.Header.Add("Accept", "application/json") @@ -367,12 +412,7 @@ func TestServerHTTPTLS(t *testing.T) { } } server.Close() - for i := 0; i < 10; i++ { - if flagsSvc.HC().Get() == healthcheck.Unavailable { - break - } - time.Sleep(1 * time.Millisecond) - } + wg.Wait() assert.Equal(t, healthcheck.Unavailable, flagsSvc.HC().Get()) }) @@ -414,10 +454,32 @@ func TestServerGRPCTLS(t *testing.T) { copy(tests, testCases) tests[testlen-2].clientTLS = tlscfg.Options{Enabled: false} tests[testlen-2].name = "should pass with insecure GRPC Client and insecure GRPC server with secure HTTP Server" + tests[testlen-2].TLS = tlscfg.Options{ + Enabled: false, + } + + disabledTLSCfg := tlscfg.Options{ + Enabled: false, + } + enabledTLSCfg := tlscfg.Options{ + Enabled: true, + CertPath: testCertKeyLocation + "/example-server-cert.pem", + KeyPath: testCertKeyLocation + "/example-server-key.pem", + ClientCAPath: testCertKeyLocation + "/example-CA-cert.pem", + } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - serverOptions := &QueryOptions{HTTPTLSEnabled: test.HTTPTLSEnabled, GRPCTLSEnabled: test.GRPCTLSEnabled, TLS: test.TLS, HostPort: ports.PortToHostPort(ports.QueryAdminHTTP), BearerTokenPropagation: true} + TLSHTTP := disabledTLSCfg + if test.HTTPTLSEnabled { + TLSHTTP = enabledTLSCfg + } + serverOptions := &QueryOptions{ + GRPCHostPort: ports.GetAddressFromCLIOptions(ports.QueryGRPC, ""), + HTTPHostPort: ports.GetAddressFromCLIOptions(ports.QueryHTTP, ""), + TLSHTTP: TLSHTTP, + TLSGRPC: test.TLS, + BearerTokenPropagation: true} flagsSvc := flags.NewService(ports.QueryAdminHTTP) flagsSvc.Logger = zap.NewNop() @@ -432,25 +494,33 @@ func TestServerGRPCTLS(t *testing.T) { opentracing.NoopTracer{}) assert.Nil(t, err) assert.NoError(t, server.Start()) + + var wg sync.WaitGroup + wg.Add(1) + once := sync.Once{} + go func() { for s := range server.HealthCheckStatus() { - flagsSvc.SetHealthCheckStatus(s) + flagsSvc.HC().Set(s) + if s == healthcheck.Unavailable { + once.Do(func() { + wg.Done() + }) + } } }() var clientError error var client *grpcClient - // time.Sleep(10 * time.Millisecond) // wait for server to start serving - - if serverOptions.GRPCTLSEnabled { - clientTLSCfg, err0 := test.clientTLS.Config() + if serverOptions.TLSGRPC.Enabled { + clientTLSCfg, err0 := test.clientTLS.Config(zap.NewNop()) require.NoError(t, err0) creds := credentials.NewTLS(clientTLSCfg) - client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryAdminHTTP), creds) + client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryGRPC), creds) } else { - client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryAdminHTTP), nil) + client = newGRPCClientWithTLS(t, ports.PortToHostPort(ports.QueryGRPC), nil) } ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) @@ -468,12 +538,7 @@ func TestServerGRPCTLS(t *testing.T) { require.Nil(t, client.conn.Close()) } server.Close() - for i := 0; i < 10; i++ { - if flagsSvc.HC().Get() == healthcheck.Unavailable { - break - } - time.Sleep(1 * time.Millisecond) - } + wg.Wait() assert.Equal(t, healthcheck.Unavailable, flagsSvc.HC().Get()) }) } From 0119c1ea047143df32e6bbcaed1cf39f02909197 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Mon, 14 Sep 2020 14:56:42 +0530 Subject: [PATCH 22/25] remove redundant test, created error var Signed-off-by: rjs211 --- cmd/query/app/server.go | 6 +++++- cmd/query/app/server_test.go | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 4170e9c76af..9bf89ff81ea 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -51,6 +51,10 @@ type Server struct { unavailableChannel chan healthcheck.Status } +var ( + errSharedTLSPort = errors.New("server with TLS enabled can not use same host ports for gRPC and HTTP. Use dedicated HTTP and gRPC host ports instead") +) + // NewServer creates and initializes Server func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, options *QueryOptions, tracer opentracing.Tracer) (*Server, error) { @@ -64,7 +68,7 @@ func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, options *Que } if (options.TLSHTTP.Enabled || options.TLSGRPC.Enabled) && (grpcPort == httpPort) { - return nil, errors.New("Server with TLS enabled can not use same host ports for gRPC and HTTP. Use dedicated HTTP and gRPC host ports instead") + return nil, errSharedTLSPort } grpcServer, err := createGRPCServer(querySvc, options, logger, tracer) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index fcb9f769b75..80d7a0d9a86 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -105,6 +105,7 @@ var testCases = []struct { expectServerFail bool }{ { + // this is a cross test for the "dedicated ports" use case without TLS name: "Should pass with insecure connection", HTTPTLSEnabled: false, GRPCTLSEnabled: false, From 601c2699b911e55b211522f11d3cbcc58fe3654d Mon Sep 17 00:00:00 2001 From: rjs211 Date: Mon, 14 Sep 2020 15:04:38 +0530 Subject: [PATCH 23/25] remove redundant test, created error var Signed-off-by: rjs211 --- cmd/query/app/server_test.go | 47 ------------------------------------ 1 file changed, 47 deletions(-) diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index 80d7a0d9a86..a9120e939d6 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -633,53 +633,6 @@ func TestServer(t *testing.T) { assert.Equal(t, healthcheck.Unavailable, flagsSvc.HC().Get()) } -func TestServerWithDedicatedPorts(t *testing.T) { - flagsSvc := flags.NewService(ports.QueryAdminHTTP) - flagsSvc.Logger = zap.NewNop() - - spanReader := &spanstoremocks.Reader{} - dependencyReader := &depsmocks.Reader{} - expectedServices := []string{"test"} - spanReader.On("GetServices", mock.AnythingOfType("*context.valueCtx")).Return(expectedServices, nil) - - querySvc := querysvc.NewQueryService(spanReader, dependencyReader, querysvc.QueryServiceOptions{}) - - server, err := NewServer(flagsSvc.Logger, querySvc, - &QueryOptions{HTTPHostPort: "127.0.0.1:8080", GRPCHostPort: "127.0.0.1:8081", BearerTokenPropagation: true}, - opentracing.NoopTracer{}) - assert.Nil(t, err) - assert.NoError(t, server.Start()) - - var wg sync.WaitGroup - wg.Add(1) - once := sync.Once{} - - go func() { - for s := range server.HealthCheckStatus() { - flagsSvc.HC().Set(s) - if s == healthcheck.Unavailable { - once.Do(func() { - wg.Done() - }) - } - } - }() - - client := newGRPCClient(t, "127.0.0.1:8081") - defer client.conn.Close() - - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - - res, err := client.GetServices(ctx, &api_v2.GetServicesRequest{}) - assert.NoError(t, err) - assert.Equal(t, expectedServices, res.Services) - - server.Close() - wg.Wait() - assert.Equal(t, healthcheck.Unavailable, flagsSvc.HC().Get()) -} - func TestServerGracefulExit(t *testing.T) { flagsSvc := flags.NewService(ports.QueryAdminHTTP) From f0ac83ec415540f3ee1081f9af7b24dde138b0f8 Mon Sep 17 00:00:00 2001 From: rjs211 Date: Tue, 10 Nov 2020 23:59:46 +0530 Subject: [PATCH 24/25] removed code repitition, added comment Signed-off-by: rjs211 --- cmd/query/app/server.go | 19 +++++++------------ cmd/query/app/server_test.go | 1 + 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index cf728e96c35..37da8b9f59a 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -51,10 +51,6 @@ type Server struct { unavailableChannel chan healthcheck.Status } -var ( - errSharedTLSPort = errors.New("server with TLS enabled can not use same host ports for gRPC and HTTP. Use dedicated HTTP and gRPC host ports instead") -) - // NewServer creates and initializes Server func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, options *QueryOptions, tracer opentracing.Tracer) (*Server, error) { @@ -68,7 +64,7 @@ func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, options *Que } if (options.TLSHTTP.Enabled || options.TLSGRPC.Enabled) && (grpcPort == httpPort) { - return nil, errSharedTLSPort + return nil, errors.New("server with TLS enabled can not use same host ports for gRPC and HTTP. Use dedicated HTTP and gRPC host ports instead") } grpcServer, err := createGRPCServer(querySvc, options, logger, tracer) @@ -143,20 +139,19 @@ func createHTTPServer(querySvc *querysvc.QueryService, queryOpts *QueryOptions, handler = handlers.CompressHandler(handler) recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true) + server := &http.Server{ + Handler: recoveryHandler(handler), + } + if queryOpts.TLSHTTP.Enabled { tlsCfg, err := queryOpts.TLSHTTP.Config(logger) // This checks if the certificates are correctly provided if err != nil { return nil, err } - return &http.Server{ - Handler: recoveryHandler(handler), - TLSConfig: tlsCfg, - }, nil + server.TLSConfig = tlsCfg } - return &http.Server{ - Handler: recoveryHandler(handler), - }, nil + return server, nil } // initListener initialises listeners of the server diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go index b65c088eebf..18807fa38ad 100644 --- a/cmd/query/app/server_test.go +++ b/cmd/query/app/server_test.go @@ -56,6 +56,7 @@ func TestServerError(t *testing.T) { } func TestCreateTLSServerSinglePortError(t *testing.T) { + // When TLS is enabled, and the host-port of both servers are the same, this leads to error, as TLS-enabled server is required to run on dedicated port. tlsCfg := tlscfg.Options{ Enabled: true, CertPath: testCertKeyLocation + "/example-server-cert.pem", From c53af219c2f87dba7b038b02ea76023047ac057a Mon Sep 17 00:00:00 2001 From: rjs211 Date: Wed, 11 Nov 2020 00:05:52 +0530 Subject: [PATCH 25/25] added table-based tests for QueryOptions port allocation Signed-off-by: rjs211 --- cmd/query/app/flags.go | 4 +- cmd/query/app/flags_test.go | 145 ++++++++++++++++++++++++++++-------- 2 files changed, 115 insertions(+), 34 deletions(-) diff --git a/cmd/query/app/flags.go b/cmd/query/app/flags.go index 349986350bf..9adbf70a72f 100644 --- a/cmd/query/app/flags.go +++ b/cmd/query/app/flags.go @@ -119,8 +119,8 @@ func (qOpts *QueryOptions) InitPortsConfigFromViper(v *viper.Viper, logger *zap. return qOpts } - // --query.host-port flag is not set and at least one of --query.grpc-server.host-port or --query.http-server.host-port is set by the user with command line flags. - // user intends to use separate GRPC and HTTP host:port flags + // --query.host-port flag is not set and either or both of --query.grpc-server.host-port or --query.http-server.host-port is set by the user with command line flags. + // i.e. user intends to use separate GRPC and HTTP host:port flags if !(v.IsSet(queryHostPort) || v.IsSet(queryPort)) && (v.IsSet(queryHTTPHostPort) || v.IsSet(queryGRPCHostPort)) { return qOpts } diff --git a/cmd/query/app/flags_test.go b/cmd/query/app/flags_test.go index d2ea31ecbfc..5ed081cc9e1 100644 --- a/cmd/query/app/flags_test.go +++ b/cmd/query/app/flags_test.go @@ -61,38 +61,6 @@ func TestQueryBuilderFlags(t *testing.T) { assert.Equal(t, 10*time.Second, qOpts.MaxClockSkewAdjust) } -func TestQueryBuilderFlagsSeparatePorts(t *testing.T) { - v, command := config.Viperize(AddFlags) - command.ParseFlags([]string{ - "--query.http-server.host-port=127.0.0.1:8080", - }) - qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) - assert.Equal(t, "127.0.0.1:8080", qOpts.HTTPHostPort) - assert.Equal(t, ports.PortToHostPort(ports.QueryGRPC), qOpts.GRPCHostPort) -} - -func TestQueryBuilderFlagsSeparateNoPorts(t *testing.T) { - v, command := config.Viperize(AddFlags) - command.ParseFlags([]string{ - "--query.http.tls.enabled=true", - }) - qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) - - assert.Equal(t, ports.PortToHostPort(ports.QueryHTTP), qOpts.HTTPHostPort) - assert.Equal(t, ports.PortToHostPort(ports.QueryGRPC), qOpts.GRPCHostPort) - assert.NotEqual(t, qOpts.HTTPHostPort, qOpts.GRPCHostPort) -} - -func TestQueryBuilderFlagsSeparateNoPortsWithTLS(t *testing.T) { - v, command := config.Viperize(AddFlags) - command.ParseFlags([]string{}) - qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) - - assert.Equal(t, ports.PortToHostPort(ports.QueryHTTP), qOpts.HTTPHostPort) - assert.Equal(t, ports.PortToHostPort(ports.QueryHTTP), qOpts.GRPCHostPort) - assert.Equal(t, ports.PortToHostPort(ports.QueryHTTP), qOpts.HostPort) -} - func TestQueryBuilderBadHeadersFlags(t *testing.T) { v, command := config.Viperize(AddFlags) command.ParseFlags([]string{ @@ -157,3 +125,116 @@ func TestBuildQueryServiceOptions(t *testing.T) { assert.NotNil(t, qSvcOpts.ArchiveSpanReader) assert.NotNil(t, qSvcOpts.ArchiveSpanWriter) } + +func TestQueryOptionsPortAllocationFromFlags(t *testing.T) { + var flagPortCases = []struct { + name string + flagsArray []string + expectedHTTPHostPort string + expectedGRPCHostPort string + verifyCommonPort bool + expectedHostPort string + }{ + { + // Since TLS is enabled in atleast one server, the dedicated host-ports obtained from viper are used, even if common host-port is specified + name: "Atleast one dedicated host-port and common host-port is specified, atleast one of GRPC, HTTP TLS enabled", + flagsArray: []string{ + "--query.grpc.tls.enabled=true", + "--query.http-server.host-port=127.0.0.1:8081", + "--query.host-port=127.0.0.1:8080", + }, + expectedHTTPHostPort: "127.0.0.1:8081", + expectedGRPCHostPort: ports.PortToHostPort(ports.QueryGRPC), // fallback in viper + verifyCommonPort: false, + }, + { + // TLS is disabled in both servers, since common host-port is specified, common host-port is used + name: "Atleast one dedicated host-port is specified, common host-port is specified, both GRPC and HTTP TLS disabled", + flagsArray: []string{ + "--query.http-server.host-port=127.0.0.1:8081", + "--query.host-port=127.0.0.1:8080", + }, + expectedHTTPHostPort: "127.0.0.1:8080", + expectedGRPCHostPort: "127.0.0.1:8080", + verifyCommonPort: true, + expectedHostPort: "127.0.0.1:8080", + }, + { + // Since TLS is enabled in atleast one server, the dedicated host-ports obtained from viper are used + name: "Atleast one dedicated host-port is specified, common host-port is not specified, atleast one of GRPC, HTTP TLS enabled", + flagsArray: []string{ + "--query.grpc.tls.enabled=true", + "--query.http-server.host-port=127.0.0.1:8081", + }, + expectedHTTPHostPort: "127.0.0.1:8081", + expectedGRPCHostPort: ports.PortToHostPort(ports.QueryGRPC), // fallback in viper + verifyCommonPort: false, + }, + { + // TLS is disabled in both servers, since common host-port is not specified but atleast one dedicated port is specified, the dedicated host-ports obtained from viper are used + name: "Atleast one dedicated port, common port defined, both GRPC and HTTP TLS disabled", + flagsArray: []string{ + "--query.http-server.host-port=127.0.0.1:8081", + }, + expectedHTTPHostPort: "127.0.0.1:8081", + expectedGRPCHostPort: ports.PortToHostPort(ports.QueryGRPC), // fallback in viper + verifyCommonPort: false, + }, + { + // Since TLS is enabled in atleast one server, the dedicated host-ports obtained from viper are used, even if common host-port is specified and the dedicated host-port are not specified + name: "No dedicated host-port is specified, common host-port is specified, atleast one of GRPC, HTTP TLS enabled", + flagsArray: []string{ + "--query.grpc.tls.enabled=true", + "--query.host-port=127.0.0.1:8080", + }, + expectedHTTPHostPort: ports.PortToHostPort(ports.QueryHTTP), // fallback in viper + expectedGRPCHostPort: ports.PortToHostPort(ports.QueryGRPC), // fallback in viper + verifyCommonPort: false, + }, + { + // TLS is disabled in both servers, since only common host-port is specified, common host-port is used + name: "No dedicated host-port is specified, common host-port is specified, both GRPC and HTTP TLS disabled", + flagsArray: []string{ + "--query.host-port=127.0.0.1:8080", + }, + expectedHTTPHostPort: "127.0.0.1:8080", + expectedGRPCHostPort: "127.0.0.1:8080", + verifyCommonPort: true, + expectedHostPort: "127.0.0.1:8080", + }, + { + // Since TLS is enabled in atleast one server, the dedicated host-ports obtained from viper are used + name: "No dedicated host-port is specified, common host-port is not specified, atleast one of GRPC, HTTP TLS enabled", + flagsArray: []string{ + "--query.grpc.tls.enabled=true", + }, + expectedHTTPHostPort: ports.PortToHostPort(ports.QueryHTTP), // fallback in viper + expectedGRPCHostPort: ports.PortToHostPort(ports.QueryGRPC), // fallback in viper + verifyCommonPort: false, + }, + { + // TLS is disabled in both servers, since common host-port is not specified and neither dedicated ports are specified, common host-port from viper is used + name: "No dedicated host-port is specified, common host-port is not specified, both GRPC and HTTP TLS disabled", + flagsArray: []string{}, + expectedHTTPHostPort: ports.PortToHostPort(ports.QueryHTTP), + expectedGRPCHostPort: ports.PortToHostPort(ports.QueryHTTP), + verifyCommonPort: true, + expectedHostPort: ports.PortToHostPort(ports.QueryHTTP), // fallback in viper + }, + } + + for _, test := range flagPortCases { + t.Run(test.name, func(t *testing.T) { + v, command := config.Viperize(AddFlags) + command.ParseFlags(test.flagsArray) + qOpts := new(QueryOptions).InitFromViper(v, zap.NewNop()) + + assert.Equal(t, test.expectedHTTPHostPort, qOpts.HTTPHostPort) + assert.Equal(t, test.expectedGRPCHostPort, qOpts.GRPCHostPort) + if test.verifyCommonPort { + assert.Equal(t, test.expectedHostPort, qOpts.HostPort) + } + + }) + } +}