diff --git a/server/embed/etcd.go b/server/embed/etcd.go index ce3b33986432..3f9bfcec885e 100644 --- a/server/embed/etcd.go +++ b/server/embed/etcd.go @@ -822,6 +822,24 @@ func (e *Etcd) pickGRPCGatewayServeContext(splitHTTP bool) *serveCtx { panic("Expect at least one context able to serve grpc") } +var ErrMissingClientTLSInfoForMetricsURL = errors.New("client TLS key/cert (--cert-file, --key-file) must be provided for metrics url") + +func (e *Etcd) createMetricsListener(murl url.URL) (net.Listener, error) { + tlsInfo := &e.cfg.ClientTLSInfo + switch murl.Scheme { + case "http": + tlsInfo = nil + case "https", "unixs": + if e.cfg.ClientTLSInfo.Empty() { + return nil, ErrMissingClientTLSInfoForMetricsURL + } + } + return transport.NewListenerWithOpts(murl.Host, murl.Scheme, + transport.WithTLSInfo(tlsInfo), + transport.WithSocketOpts(&e.cfg.SocketOpts), + ) +} + func (e *Etcd) serveMetrics() (err error) { if e.cfg.Metrics == "extensive" { grpc_prometheus.EnableHandlingTimeHistogram() @@ -833,14 +851,7 @@ func (e *Etcd) serveMetrics() (err error) { etcdhttp.HandleHealth(e.cfg.logger, metricsMux, e.Server) for _, murl := range e.cfg.ListenMetricsUrls { - tlsInfo := &e.cfg.ClientTLSInfo - if murl.Scheme == "http" { - tlsInfo = nil - } - ml, err := transport.NewListenerWithOpts(murl.Host, murl.Scheme, - transport.WithTLSInfo(tlsInfo), - transport.WithSocketOpts(&e.cfg.SocketOpts), - ) + ml, err := e.createMetricsListener(murl) if err != nil { return err } diff --git a/server/embed/etcd_test.go b/server/embed/etcd_test.go new file mode 100644 index 000000000000..793563999ac2 --- /dev/null +++ b/server/embed/etcd_test.go @@ -0,0 +1,24 @@ +package embed + +import ( + "net/url" + "testing" + + "go.etcd.io/etcd/client/pkg/v3/transport" +) + +func TestEmptyClientTLSInfo_createMetricsListener(t *testing.T) { + e := &Etcd{ + cfg: Config{ + ClientTLSInfo: transport.TLSInfo{}, + }, + } + + murl := url.URL{ + Scheme: "https", + Host: "localhost:8080", + } + if _, err := e.createMetricsListener(murl); err != ErrMissingClientTLSInfoForMetricsURL { + t.Fatalf("expected error %v, got %v", ErrMissingClientTLSInfoForMetricsURL, err) + } +} diff --git a/server/etcdmain/help.go b/server/etcdmain/help.go index 8633d7c2f6f5..d9bb14525e5c 100644 --- a/server/etcdmain/help.go +++ b/server/etcdmain/help.go @@ -238,7 +238,7 @@ Profiling and Monitoring: --metrics 'basic' Set level of detail for exported metrics, specify 'extensive' to include server side grpc histogram metrics. --listen-metrics-urls '' - List of URLs to listen on for the metrics and health endpoints. + List of URLs to listen on for the /metrics and /health endpoints. For https, the client URL TLS info is used. Logging: --logger 'zap' diff --git a/tests/e2e/etcd_config_test.go b/tests/e2e/etcd_config_test.go index a6da16b04eda..c455bc208e86 100644 --- a/tests/e2e/etcd_config_test.go +++ b/tests/e2e/etcd_config_test.go @@ -28,6 +28,7 @@ import ( "golang.org/x/sync/errgroup" "go.etcd.io/etcd/pkg/v3/expect" + "go.etcd.io/etcd/server/v3/embed" "go.etcd.io/etcd/tests/v3/framework/config" "go.etcd.io/etcd/tests/v3/framework/e2e" ) @@ -120,6 +121,60 @@ func TestEtcdUnixPeers(t *testing.T) { } } +// TestEtcdListenMetricsURLsWithMissingClientTLSInfo checks that the HTTPs listen metrics URL +// but without the client TLS info will fail its verification. +func TestEtcdListenMetricsURLsWithMissingClientTLSInfo(t *testing.T) { + e2e.SkipInShortMode(t) + + tempDir := t.TempDir() + defer os.RemoveAll(tempDir) + + caFile, certFiles, keyFiles, err := generateCertsForIPs(tempDir, []net.IP{net.ParseIP("127.0.0.1")}) + require.NoError(t, err) + + // non HTTP but metrics URL is HTTPS, invalid when the client TLS info is not provided + clientURL := fmt.Sprintf("http://localhost:%d", e2e.EtcdProcessBasePort) + peerURL := fmt.Sprintf("https://localhost:%d", e2e.EtcdProcessBasePort+1) + listenMetricsURL := fmt.Sprintf("https://localhost:%d", e2e.EtcdProcessBasePort+2) + + commonArgs := []string{ + e2e.BinPath.Etcd, + "--name", "e0", + "--data-dir", tempDir, + + "--listen-client-urls", clientURL, + "--advertise-client-urls", clientURL, + + "--initial-advertise-peer-urls", peerURL, + "--listen-peer-urls", peerURL, + + "--initial-cluster", "e0=" + peerURL, + + "--listen-metrics-urls", listenMetricsURL, + + "--peer-cert-file", certFiles[0], + "--peer-key-file", keyFiles[0], + "--peer-trusted-ca-file", caFile, + "--peer-client-cert-auth", + } + + proc, err := e2e.SpawnCmd(commonArgs, nil) + if err != nil { + t.Fatal(err) + } + defer func() { + if err := proc.Stop(); err != nil { + t.Error(err) + } + proc.Wait() // ensure the port has been released + _ = proc.Close() + }() + + if err := e2e.WaitReadyExpectProc(context.TODO(), proc, []string{embed.ErrMissingClientTLSInfoForMetricsURL.Error()}); err != nil { + t.Fatal(err) + } +} + // TestEtcdPeerCNAuth checks that the inter peer auth based on CN of cert is working correctly. func TestEtcdPeerCNAuth(t *testing.T) { e2e.SkipInShortMode(t)