Limit serving of insecure metrics by allowing configurable IP

Currently we are serving insecure metrics on all IPv4
routable addresses on the local machine (0.0.0.0).

In this PR, we make the metric IP configurable in order to ensure
that we listen for insecure metrics port only on one IP to reduce
security vulnerability.

Signed-off-by: Swati Sehgal <[email protected]>

cmd/resource-topology-exporter/main.go

@@ -87,7 +87,7 @@ func main() {
 	if err != nil {
 		klog.Fatalf("failed to setup metrics: %v", err)
 	}
-	err = metricssrv.Setup(parsedArgs.RTE.MetricsMode, metricssrv.NewConfig(parsedArgs.RTE.MetricsPort, parsedArgs.RTE.MetricsTLSCfg))
+	err = metricssrv.Setup(parsedArgs.RTE.MetricsMode, metricssrv.NewConfig(parsedArgs.RTE.MetricsAddress, parsedArgs.RTE.MetricsPort, parsedArgs.RTE.MetricsTLSCfg))
 	if err != nil {
 		klog.Fatalf("failed to setup metrics server: %v", err)
 	}

hack/get-manifests.sh

@@ -11,4 +11,5 @@ export RTE_VERBOSE="${RTE_VERBOSE:-5}"
 export RTE_METRICS_MODE="${RTE_METRICS_MODE:-disabled}"
 export RTE_METRICS_CLI_AUTH="${RTE_METRICS_CLI_AUTH:-true}"
 export METRICS_PORT="${METRICS_PORT:-2112}"
+export METRICS_ADDRESS="${METRICS_ADDRESS:-127.0.0.1}"
 envsubst < ${DIRNAME}/../manifests/resource-topology-exporter.yaml

manifests/resource-topology-exporter-ds-notif-file.yaml

@@ -42,6 +42,8 @@ spec:
           value: shared-pool-container
         - name: METRICS_PORT
           value: "${METRICS_PORT}"
+        - name: METRICS_ADDRESS
+          value: "${METRICS_ADDRESS}"
         volumeMounts:
           - name: host-sys
             mountPath: "/host-sys"

manifests/resource-topology-exporter.yaml

@@ -114,6 +114,8 @@ spec:
           value: shared-pool-container
         - name: METRICS_PORT
           value: "${METRICS_PORT}"
+        - name: METRICS_ADDRESS
+          value: "${METRICS_ADDRESS}"
         volumeMounts:
           - name: host-sys
             mountPath: "/host-sys"

pkg/config/cfgdispatch.go

@@ -126,6 +126,7 @@ func dispatchConfObj(obj map[string]interface{}, pArgs *ProgArgs) error {
 		{key: "topologyExporter.addNRTOwnerEnable", out: &pArgs.RTE.AddNRTOwnerEnable},
 		{key: "topologyExporter.metricsMode", out: &pArgs.RTE.MetricsMode},
 		{key: "topologyExporter.metricsPort", out: &pArgs.RTE.MetricsPort},
+		{key: "topologyExporter.MetricsAddress", out: &pArgs.RTE.MetricsAddress},
 		{key: "topologyExporter.metricsTLS.certsDir", out: &pArgs.RTE.MetricsTLSCfg.CertsDir},
 		{key: "topologyExporter.metricsTLS.certFile", out: &pArgs.RTE.MetricsTLSCfg.CertFile},
 		{key: "topologyExporter.metricsTLS.keyFile", out: &pArgs.RTE.MetricsTLSCfg.KeyFile},

pkg/config/defaults.go

@@ -39,6 +39,7 @@ func SetDefaults(pArgs *ProgArgs) {
 	pArgs.RTE.AddNRTOwnerEnable = true
 	pArgs.RTE.MetricsMode = metricssrv.ServingDefault
 	pArgs.RTE.MetricsPort = metricssrv.PortDefault
+	pArgs.RTE.MetricsAddress = metricssrv.AddressDefault
 	pArgs.RTE.MetricsTLSCfg = metricssrv.NewDefaultTLSConfig()
 	pArgs.RTE.MaxEventsPerTimeUnit = 1
 	pArgs.RTE.TimeUnitToLimitEvents = time.Second

pkg/config/environ.go

@@ -63,4 +63,7 @@ func FromEnv(pArgs *ProgArgs) {
 	if pArgs.RTE.MetricsPort == 0 {
 		pArgs.RTE.MetricsPort = metricssrv.PortFromEnv()
 	}
+	if pArgs.RTE.MetricsAddress == "" {
+		pArgs.RTE.MetricsAddress = metricssrv.IPFromEnv()
+	}
 }

pkg/config/flags.go

@@ -63,6 +63,7 @@ func FromFlags(pArgs *ProgArgs, args ...string) (string, string, error) {
 	flags.BoolVar(&pArgs.RTE.AddNRTOwnerEnable, "add-nrt-owner", pArgs.RTE.AddNRTOwnerEnable, "RTE will inject NRT's related node as OwnerReference to ensure cleanup if the node is deleted.")
 	flags.StringVar(&pArgs.RTE.MetricsMode, "metrics-mode", pArgs.RTE.MetricsMode, fmt.Sprintf("Select the mode to expose metrics endpoint. Valid options: %s", metricssrv.ServingModeSupported()))
 	flags.IntVar(&pArgs.RTE.MetricsPort, "metrics-port", pArgs.RTE.MetricsPort, "Select the port to listen for the metrics endpoint.")
+	flags.StringVar(&pArgs.RTE.MetricsAddress, "metrics-ip", pArgs.RTE.MetricsAddress, "Select the IP to listen for the metrics endpoint.")
 	flags.StringVar(&pArgs.RTE.MetricsTLSCfg.CertsDir, "metrics-certs-dir", pArgs.RTE.MetricsTLSCfg.CertsDir, "certificates directory for TLS metrics serving")
 	flags.StringVar(&pArgs.RTE.MetricsTLSCfg.CertFile, "metrics-cert-file", pArgs.RTE.MetricsTLSCfg.CertFile, "certificate file name for TLS metrics serving")
 	flags.StringVar(&pArgs.RTE.MetricsTLSCfg.KeyFile, "metrics-key-file", pArgs.RTE.MetricsTLSCfg.KeyFile, "key file name for TLS metrics serving")

pkg/metrics/server/setup.go

@@ -29,7 +29,8 @@ import (
 )
 
 const (
-	PortDefault = 2112
+	PortDefault    = 2112
+	AddressDefault = "0.0.0.0"
 
 	TLSRootDir = "/etc/secrets/rte"
 
@@ -60,19 +61,21 @@ type TLSConfig struct {
 }
 
 type Config struct {
+	IP   string
 	Port int
 	TLS  TLSConfig
 }
 
-func NewConfig(port int, tlsConf TLSConfig) Config {
+func NewConfig(ip string, port int, tlsConf TLSConfig) Config {
 	return Config{
+		IP:   ip,
 		Port: port,
 		TLS:  tlsConf,
 	}
 }
 
 func NewDefaultConfig() Config {
-	return NewConfig(PortDefault, NewDefaultTLSConfig())
+	return NewConfig(AddressDefault, PortDefault, NewDefaultTLSConfig())
 }
 
 func (conf TLSConfig) Clone() TLSConfig {
@@ -91,7 +94,7 @@ func (conf Config) Validate() error {
 }
 
 func (conf Config) BindAddress() string {
-	return fmt.Sprintf(":%d", conf.Port)
+	return fmt.Sprintf("%s:%d", conf.IP, conf.Port)
 }
 
 func ServingModeIsSupported(value string) (string, error) {
@@ -130,6 +133,14 @@ func PortFromEnv() int {
 	return port
 }
 
+func IPFromEnv() string {
+	ip, ok := os.LookupEnv("METRICS_ADDRESS")
+	if !ok {
+		return ""
+	}
+	return ip
+}
+
 func Setup(mode string, conf Config) error {
 	if mode == ServingDisabled {
 		klog.Infof("metrics endpoint disabled")

pkg/resourcetopologyexporter/resourcetopologyexporter.go

@@ -34,6 +34,7 @@ type Args struct {
 	AddNRTOwnerEnable      bool                           `json:"addNRTOwnerEnable,omitempty"`
 	MetricsMode            string                         `json:"metricsMode,omitempty"`
 	MetricsPort            int                            `json:"metricsPort,omitempty"`
+	MetricsAddress         string                         `json:"metricsAddress,omitempty"`
 	MetricsTLSCfg          metricssrv.TLSConfig           `json:"metricsTLS,omitempty"`
 }
 
@@ -52,6 +53,7 @@ func (args Args) Clone() Args {
 		AddNRTOwnerEnable:      args.AddNRTOwnerEnable,
 		MetricsMode:            args.MetricsMode,
 		MetricsPort:            args.MetricsPort,
+		MetricsAddress:         args.MetricsAddress,
 		MetricsTLSCfg:          args.MetricsTLSCfg.Clone(),
 	}
 }

test/data/TestLoadDefaults.expected.json

@@ -1 +1 @@
-{"global":{"verbose":2},"nrtUpdater":{"hostname":"TEST_NODE"},"resourceMonitor":{"sysfsRoot":"/sys","podSetFingerprint":true,"podSetFingerprintMethod":"with-exclusive-resources"},"topologyExporter":{"referenceContainer":{"namespace":"TEST_NS","podName":"TEST_POD","containerName":"TEST_CONT"},"kubeletConfigFile":"/podresources/config.yaml","podResourcesSocketPath":"unix:///podresources/kubelet.sock","sleepInterval":60000000000,"podReadinessEnable":true,"maxEventPerTimeUnit":1,"timeUnitToLimitEvents":1000000000,"addNRTOwnerEnable":true,"metricsMode":"disabled","metricsPort":2112,"metricsTLS":{"certsDir":"/etc/secrets/rte","certFile":"tls.crt","keyFile":"tls.key"}}}
+{"global":{"verbose":2},"nrtUpdater":{"hostname":"TEST_NODE"},"resourceMonitor":{"sysfsRoot":"/sys","podSetFingerprint":true,"podSetFingerprintMethod":"with-exclusive-resources"},"topologyExporter":{"referenceContainer":{"namespace":"TEST_NS","podName":"TEST_POD","containerName":"TEST_CONT"},"kubeletConfigFile":"/podresources/config.yaml","podResourcesSocketPath":"unix:///podresources/kubelet.sock","sleepInterval":60000000000,"podReadinessEnable":true,"maxEventPerTimeUnit":1,"timeUnitToLimitEvents":1000000000,"addNRTOwnerEnable":true,"metricsMode":"disabled","metricsPort":2112,"metricsAddress":"0.0.0.0","metricsTLS":{"certsDir":"/etc/secrets/rte","certFile":"tls.crt","keyFile":"tls.key"}}}

test/data/TestSetDefaults.expected.json

@@ -1 +1 @@
-{"global":{"verbose":2},"nrtUpdater":{},"resourceMonitor":{"sysfsRoot":"/sys","podSetFingerprint":true,"podSetFingerprintMethod":"with-exclusive-resources"},"topologyExporter":{"kubeletConfigFile":"/podresources/config.yaml","podResourcesSocketPath":"unix:///podresources/kubelet.sock","sleepInterval":60000000000,"podReadinessEnable":true,"maxEventPerTimeUnit":1,"timeUnitToLimitEvents":1000000000,"addNRTOwnerEnable":true,"metricsMode":"disabled","metricsPort":2112,"metricsTLS":{"certsDir":"/etc/secrets/rte","certFile":"tls.crt","keyFile":"tls.key"}}}
+{"global":{"verbose":2},"nrtUpdater":{},"resourceMonitor":{"sysfsRoot":"/sys","podSetFingerprint":true,"podSetFingerprintMethod":"with-exclusive-resources"},"topologyExporter":{"kubeletConfigFile":"/podresources/config.yaml","podResourcesSocketPath":"unix:///podresources/kubelet.sock","sleepInterval":60000000000,"podReadinessEnable":true,"maxEventPerTimeUnit":1,"timeUnitToLimitEvents":1000000000,"addNRTOwnerEnable":true,"metricsMode":"disabled","metricsPort":2112,"metricsAddress":"0.0.0.0","metricsTLS":{"certsDir":"/etc/secrets/rte","certFile":"tls.crt","keyFile":"tls.key"}}}

test/data/conftree/00-full/_output/output.yaml

@@ -20,6 +20,7 @@ resourceMonitor:
 topologyExporter:
   kubeletConfigFile: /podresources/config.yaml
   maxEventPerTimeUnit: 1
+  metricsAddress: 0.0.0.0
   metricsMode: disabled
   metricsPort: 2112
   metricsTLS:

test/data/conftree/05-full-env/_output/output.yaml

@@ -20,6 +20,7 @@ resourceMonitor:
 topologyExporter:
   kubeletConfigFile: /podresources/config.yaml
   maxEventPerTimeUnit: 1
+  metricsAddress: 0.0.0.0
   metricsMode: disabled
   metricsPort: 2112
   metricsTLS:

test/data/conftree/06-full-env-args/_output/output.yaml

@@ -20,6 +20,7 @@ resourceMonitor:
 topologyExporter:
   kubeletConfigFile: /podresources/config.yaml
   maxEventPerTimeUnit: 1
+  metricsAddress: 0.0.0.0
   metricsMode: disabled
   metricsPort: 2112
   metricsTLS:

test/e2e/rte/metrics.go

@@ -52,6 +52,7 @@ var _ = ginkgo.Describe("[RTE][Monitoring] metrics", func() {
 		hasMetrics          bool
 		metricsMode         string
 		metricsPort         int
+		MetricsAddress      string
 		rtePod              *corev1.Pod
 		workerNodes         []corev1.Node
 		topologyUpdaterNode *corev1.Node
@@ -99,6 +100,8 @@ var _ = ginkgo.Describe("[RTE][Monitoring] metrics", func() {
 			rtePod = &pods.Items[0]
 
 			if hasMetrics {
+				MetricsAddress, err = e2ertepod.FindMetricsAddress(rtePod)
+				gomega.Expect(err).ToNot(gomega.HaveOccurred())
 				metricsPort, err = e2ertepod.FindMetricsPort(rtePod)
 				gomega.Expect(err).ToNot(gomega.HaveOccurred())
 			}
@@ -120,7 +123,7 @@ var _ = ginkgo.Describe("[RTE][Monitoring] metrics", func() {
 			}
 			rteContainerName, err := e2ertepod.FindRTEContainerName(rtePod)
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
-			cmd := []string{"curl", "-k", "-L", fmt.Sprintf("https://127.0.0.1:%d/metrics", metricsPort)}
+			cmd := []string{"curl", "-k", "-L", fmt.Sprintf("https://%s:%d/metrics", MetricsAddress, metricsPort)}
 			key := client.ObjectKeyFromObject(rtePod)
 			klog.Infof("executing cmd: %s on pod %q", cmd, key.String())
 			var stdout, stderr []byte
@@ -139,7 +142,7 @@ var _ = ginkgo.Describe("[RTE][Monitoring] metrics", func() {
 			}
 			rteContainerName, err := e2ertepod.FindRTEContainerName(rtePod)
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
-			cmd := []string{"curl", "-L", fmt.Sprintf("http://127.0.0.1:%d/metrics", metricsPort)}
+			cmd := []string{"curl", "-L", fmt.Sprintf("https://%s:%d/metrics", MetricsAddress, metricsPort)}
 			key := client.ObjectKeyFromObject(rtePod)
 			klog.Infof("executing cmd: %s on pod %q", cmd, key.String())
 			var stdout, stderr []byte
@@ -173,7 +176,7 @@ var _ = ginkgo.Describe("[RTE][Monitoring] metrics", func() {
 			rteContainerName, err := e2ertepod.FindRTEContainerName(rtePod)
 			gomega.Expect(err).ToNot(gomega.HaveOccurred())
 
-			cmd := []string{"curl", "-L", fmt.Sprintf("http://127.0.0.1:%d/metrics", metricsPort)}
+			cmd := []string{"curl", "-L", fmt.Sprintf("https://%s:%d/metrics", MetricsAddress, metricsPort)}
 			key := client.ObjectKeyFromObject(rtePod)
 			klog.Infof("executing cmd: %s on pod %q", cmd, key.String())
 			var stdout, stderr []byte

test/e2e/utils/pods/rtepod/rtepod.go

@@ -51,6 +51,22 @@ func FindMetricsPort(rtePod *corev1.Pod) (int, error) {
 	return 0, fmt.Errorf("cannot find METRICS_PORT environment variable")
 }
 
+func FindMetricsAddress(rtePod *corev1.Pod) (string, error) {
+	for idx := 0; idx < len(rtePod.Spec.Containers); idx++ {
+		cnt := rtePod.Spec.Containers[idx] // shortcut
+		if !isRTEContainer(cnt) {
+			continue
+		}
+
+		for _, envVar := range cnt.Env {
+			if envVar.Name == "METRICS_ADDRESS" {
+				return envVar.Value, nil
+			}
+		}
+	}
+	return "", fmt.Errorf("cannot find METRICS_ADDRESS environment variable")
+}
+
 func FindRTEContainerName(rtePod *corev1.Pod) (string, error) {
 	for idx := 0; idx < len(rtePod.Spec.Containers); idx++ {
 		cnt := rtePod.Spec.Containers[idx] // shortcut