From 7b5cd4dda5bea482ea5b2ddbc5ab22ae993299c0 Mon Sep 17 00:00:00 2001
From: Etienne Perot <eperot@google.com>
Date: Thu, 23 Mar 2023 11:49:19 -0700
Subject: [PATCH] `runsc`: Prohibit runsc metrics from starting with the prefix
 "`meta_`".

This prefix is used by the metric server to synthesize its own metrics.

If the sandbox were to define metrics with the same name, they would conflict.

By having this prefix check, this prevents a malicious sandbox from defining
metrics that conflict with those that the metric server is trying to export.

PiperOrigin-RevId: 518922970
---
 pkg/prometheus/prometheus_test.go   | 22 ++++++++++++++++++++++
 pkg/prometheus/prometheus_verify.go |  8 ++++++++
 runsc/cmd/metric_server.go          |  2 +-
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/pkg/prometheus/prometheus_test.go b/pkg/prometheus/prometheus_test.go
index c455a4e7b..49e9bc0cc 100644
--- a/pkg/prometheus/prometheus_test.go
+++ b/pkg/prometheus/prometheus_test.go
@@ -364,6 +364,28 @@ func TestVerifier(t *testing.T) {
 			),
 			WantVerifierCreationErr: true,
 		},
+		{
+			Name: "Prometheus metric name starts with reserved prefix",
+			Registration: newMetricRegistration(&metricMetadata{
+				PB: &pb.MetricMetadata{
+					Name:           "metaFooBar",
+					PrometheusName: "meta_foo_bar",
+					Type:           pb.MetricMetadata_TYPE_UINT64,
+				}},
+			),
+			WantVerifierCreationErr: true,
+		},
+		{
+			Name: "Prometheus metric name does not starts with reserved prefix but non-Prometheus metric name does",
+			Registration: newMetricRegistration(&metricMetadata{
+				PB: &pb.MetricMetadata{
+					Name:           "metaFooBar",
+					PrometheusName: "not_meta_foo_bar",
+					Type:           pb.MetricMetadata_TYPE_UINT64,
+				}},
+			),
+			WantVerifierCreationErr: false,
+		},
 		{
 			Name: "no buckets",
 			Registration: newMetricRegistration(&metricMetadata{
diff --git a/pkg/prometheus/prometheus_verify.go b/pkg/prometheus/prometheus_verify.go
index 5cf85296f..78204487f 100644
--- a/pkg/prometheus/prometheus_verify.go
+++ b/pkg/prometheus/prometheus_verify.go
@@ -30,6 +30,11 @@ const (
 	// maxExportStaleness is the maximum allowed age of a snapshot when it is verified.
 	// Used to avoid exporting snapshots from bogus times from ages past.
 	maxExportStaleness = 10 * time.Second
+
+	// MetaMetricPrefix is a prefix used for metrics defined by the metric server,
+	// as opposed to metrics generated by each sandbox.
+	// For this reason, this prefix is not allowed to be used in sandbox metrics.
+	MetaMetricPrefix = "meta_"
 )
 
 // internedStringMap allows for interning strings.
@@ -260,6 +265,9 @@ func newVerifiableMetric(metadata *pb.MetricMetadata, verifier *Verifier) (*veri
 	if metadata.GetName() == "" || metadata.GetPrometheusName() == "" {
 		return nil, errors.New("metric has no name")
 	}
+	if strings.HasPrefix(metadata.GetPrometheusName(), MetaMetricPrefix) {
+		return nil, fmt.Errorf("metric name %q starts with %q which is a reserved prefix", metadata.GetPrometheusName(), "meta_")
+	}
 	if !unicode.IsLower(rune(metadata.GetPrometheusName()[0])) {
 		return nil, fmt.Errorf("invalid initial character in prometheus metric name: %q", metadata.GetPrometheusName())
 	}
diff --git a/runsc/cmd/metric_server.go b/runsc/cmd/metric_server.go
index 952d327d6..7512775b2 100644
--- a/runsc/cmd/metric_server.go
+++ b/runsc/cmd/metric_server.go
@@ -701,7 +701,7 @@ func (m *MetricServer) serveMetrics(w http.ResponseWriter, req *http.Request) ht
 	// Meanwhile, build the map of all snapshots we will be rendering.
 	snapshotsToOptions := make(map[*prometheus.Snapshot]prometheus.SnapshotExportOptions, numSandboxes+2)
 	snapshotsToOptions[selfMetrics] = prometheus.SnapshotExportOptions{
-		ExporterPrefix: fmt.Sprintf("%smeta_", m.exporterPrefix),
+		ExporterPrefix: fmt.Sprintf("%s%s", m.exporterPrefix, prometheus.MetaMetricPrefix),
 	}
 	processMetrics := prometheus.NewSnapshot()
 	processMetrics.Add(prometheus.NewFloatData(&ProcessStartTimeMetric, float64(m.startTime.Unix())+(float64(m.startTime.Nanosecond())/1e9)))