From 2b5d2349ede8112015f25109650481e126ad95cb Mon Sep 17 00:00:00 2001 From: csuzhangxc Date: Thu, 5 Nov 2026 14:43:36 +0800 Subject: [PATCH 1/3] reloader: drop privileges while preserving initializer compatibility --- .github/workflows/build-security.yaml | 4 + cmd/Dockerfile | 1 + cmd/datasource/k8s-datasource.yaml | 2 +- cmd/dm_rules_test.go | 78 +++++++- cmd/init.sh | 108 ++++++++--- cmd/persisted_rules_test.go | 140 ++++++++++++++ monitor-snapshot/master/operator/Dockerfile | 1 + .../operator/datasources/k8s-datasource.yaml | 2 +- monitor-snapshot/master/operator/init.sh | 108 ++++++++--- platform-monitoring/operator/Dockerfile | 1 + .../operator/datasource/k8s-datasource.yaml | 2 +- platform-monitoring/operator/init.sh | 108 ++++++++--- reload/CONTAINER_SECURITY.md | 77 ++++++++ reload/Dockerfile | 2 + reload/Dockerfile_buildx | 2 + reload/identity/identity_linux.go | 176 ++++++++++++++++++ reload/identity/identity_linux_test.go | 134 +++++++++++++ reload/identity/identity_other.go | 7 + reload/main.go | 7 + 19 files changed, 891 insertions(+), 69 deletions(-) create mode 100644 cmd/persisted_rules_test.go create mode 100644 reload/CONTAINER_SECURITY.md create mode 100644 reload/identity/identity_linux.go create mode 100644 reload/identity/identity_linux_test.go create mode 100644 reload/identity/identity_other.go diff --git a/.github/workflows/build-security.yaml b/.github/workflows/build-security.yaml index 722dc6c8..c6d671fe 100644 --- a/.github/workflows/build-security.yaml +++ b/.github/workflows/build-security.yaml @@ -6,6 +6,9 @@ on: - '.github/workflows/build-security.yaml' - '.github/workflows/reloader-dockerhub.yaml' - 'pkg/common/**' + - 'cmd/**' + - 'platform-monitoring/operator/**' + - 'monitor-snapshot/master/operator/**' - 'scripts/**' - 'go.mod' - 'go.sum' @@ -27,6 +30,7 @@ jobs: with: go-version-file: go.mod - run: go test ./pkg/common + - run: go test ./cmd/monitoring.go ./cmd/dm_rules_test.go ./cmd/persisted_rules_test.go - run: python3 -m unittest discover -s scripts -p 'test_build_security.py' -v reloader: diff --git a/cmd/Dockerfile b/cmd/Dockerfile index 68ed73a1..701c5b1f 100644 --- a/cmd/Dockerfile +++ b/cmd/Dockerfile @@ -3,6 +3,7 @@ FROM busybox # Add user and group to support run it with non-root. ARG PINGCAP_UID=1000 ARG PINGCAP_GID=2000 +ENV RELOADER_UID=${PINGCAP_UID} RELOADER_GID=${PINGCAP_GID} RUN addgroup -g ${PINGCAP_GID} pingcap && \ adduser -u ${PINGCAP_UID} -G pingcap -h /home/pingcap -D pingcap diff --git a/cmd/datasource/k8s-datasource.yaml b/cmd/datasource/k8s-datasource.yaml index 9b8f9694..ea99ca8f 100644 --- a/cmd/datasource/k8s-datasource.yaml +++ b/cmd/datasource/k8s-datasource.yaml @@ -1,7 +1,7 @@ apiVersion: 1 datasources: - access: proxy - editable: true + editable: false name: k8s-cluster orgId: 1 type: prometheus diff --git a/cmd/dm_rules_test.go b/cmd/dm_rules_test.go index 6a913de9..1313be2b 100644 --- a/cmd/dm_rules_test.go +++ b/cmd/dm_rules_test.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "io" "net/http" "os" @@ -17,6 +18,77 @@ import ( type dmTransport func(*http.Request) (*http.Response, error) +func TestInitializerPreparesRuleAccess(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("initializer requires POSIX permissions") + } + for _, source := range []string{"init.sh", "../platform-monitoring/operator/init.sh", "../monitor-snapshot/master/operator/init.sh"} { + t.Run(source, func(t *testing.T) { + script, err := os.ReadFile(source) + if err != nil { + t.Fatal(err) + } + prefix, _, ok := strings.Cut(string(script), "# Multiline replacements") + if !ok || !strings.Contains(prefix, "prepare_reloader_rules()") { + t.Fatal("initializer does not prepare reloader access") + } + dir := t.TempDir() + active := filepath.Join(dir, "active rules", "rules") + data := filepath.Join(dir, "data") + store := filepath.Join(data, "latest-rules", "tidb:test") + for _, path := range []string{active, store} { + if err := os.MkdirAll(path, 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(path, "test.yml"), []byte("groups: []"), 0444); err != nil { + t.Fatal(err) + } + } + unrelated := filepath.Join(data, "prometheus") + if err := os.Mkdir(unrelated, 0700); err != nil { + t.Fatal(err) + } + run := func() error { + cmd := exec.Command("sh", "-c", prefix+"\nprepare_reloader_rules tidb:test") + cmd.Env = append(os.Environ(), "PROM_CONFIG_PATH="+filepath.Dir(active), "PROM_PERSISTENT_DIR="+data, + fmt.Sprintf("RELOADER_UID=%d", os.Geteuid()), fmt.Sprintf("RELOADER_GID=%d", os.Getegid())) + output, err := cmd.CombinedOutput() + if err != nil { + return fmt.Errorf("%w: %s", err, output) + } + return nil + } + if err := run(); err != nil { + t.Fatal(err) + } + for _, path := range []string{active, store} { + info, err := os.Stat(filepath.Join(path, "test.yml")) + if err != nil || info.Mode().Perm() != 0644 { + t.Fatalf("rule permissions: %v, %v", info, err) + } + } + info, err := os.Stat(unrelated) + if err != nil || info.Mode().Perm() != 0700 { + t.Fatal("unrelated data directory permissions changed", err) + } + outside := filepath.Join(dir, "outside.yml") + if err := os.WriteFile(outside, []byte("private"), 0400); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(active, "alias.yml")); err != nil { + t.Fatal(err) + } + if err := run(); err == nil { + t.Fatal("symbolic-link rule accepted") + } + info, err = os.Stat(outside) + if err != nil || info.Mode().Perm() != 0400 { + t.Fatal("permissions changed through symlink", err) + } + }) + } +} + func (f dmTransport) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) } func TestDMRulesAreGenerated(t *testing.T) { @@ -120,7 +192,7 @@ func TestDMInitializerRejectsMissingRules(t *testing.T) { } } cmd := exec.Command("sh", scriptPath) - cmd.Env = append(os.Environ(), "DM_CLUSTER_NAME=test", "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana space"), "PROM_CONFIG_PATH="+filepath.Join(root, "prometheus space"), "PROM_PERSISTENT_DIR=", "GF_DATASOURCE_PATH="+datasource, "GF_DM_PROMETHEUS_URL=http://example.test/?a=1&b=2", "PATH="+extraPath+os.Getenv("PATH")) + cmd.Env = append(os.Environ(), fmt.Sprintf("RELOADER_UID=%d", os.Geteuid()), fmt.Sprintf("RELOADER_GID=%d", os.Getegid()), "DM_CLUSTER_NAME=test", "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana space"), "PROM_CONFIG_PATH="+filepath.Join(root, "prometheus space"), "PROM_PERSISTENT_DIR=", "GF_DATASOURCE_PATH="+datasource, "GF_DM_PROMETHEUS_URL=http://example.test/?a=1&b=2", "PATH="+extraPath+os.Getenv("PATH")) out, err := cmd.CombinedOutput() if !hasRules && err == nil { t.Errorf("missing rules reported success: %s", out) @@ -148,7 +220,7 @@ func TestInitializerRejectsMultilineReplacement(t *testing.T) { t.Run(source, func(t *testing.T) { root := t.TempDir() cmd := exec.Command("sh", source) - cmd.Env = append(os.Environ(), "DM_CLUSTER_NAME=test", "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana"), "PROM_CONFIG_PATH="+filepath.Join(root, "prometheus"), "GF_DM_PROMETHEUS_URL=bad\nreplacement") + cmd.Env = append(os.Environ(), fmt.Sprintf("RELOADER_UID=%d", os.Geteuid()), fmt.Sprintf("RELOADER_GID=%d", os.Getegid()), "DM_CLUSTER_NAME=test", "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana"), "PROM_CONFIG_PATH="+filepath.Join(root, "prometheus"), "GF_DM_PROMETHEUS_URL=bad\nreplacement") out, err := cmd.CombinedOutput() if err == nil || !strings.Contains(string(out), "must be single-line") { t.Fatalf("multiline value not rejected: %v %s", err, out) @@ -224,7 +296,7 @@ func TestTiDBInitializerPreservesDatasourceURLs(t *testing.T) { cmd.Dir = root k8sURL := "http://example.test/?a=1&b=2,tail" tidbURL := "http://example.test/tidb?a=1&b=2" - cmd.Env = append(os.Environ(), "DM_CLUSTER_NAME=", "TIDB_CLUSTER_NAME=test", "TIDB_CLUSTER_NAMESPACE=default", "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana space"), "PROM_CONFIG_PATH="+filepath.Join(root, "prometheus space"), "PROM_PERSISTENT_DIR=", "GF_DATASOURCE_PATH="+datasource, "GF_K8S_PROMETHEUS_URL="+k8sURL, "GF_TIDB_PROMETHEUS_URL="+tidbURL, "PATH="+extraPath+os.Getenv("PATH")) + cmd.Env = append(os.Environ(), fmt.Sprintf("RELOADER_UID=%d", os.Geteuid()), fmt.Sprintf("RELOADER_GID=%d", os.Getegid()), "DM_CLUSTER_NAME=", "TIDB_CLUSTER_NAME=test", "TIDB_CLUSTER_NAMESPACE=default", "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana space"), "PROM_CONFIG_PATH="+filepath.Join(root, "prometheus space"), "PROM_PERSISTENT_DIR=", "GF_DATASOURCE_PATH="+datasource, "GF_K8S_PROMETHEUS_URL="+k8sURL, "GF_TIDB_PROMETHEUS_URL="+tidbURL, "PATH="+extraPath+os.Getenv("PATH")) if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("TiDB initializer failed: %v %s", err, out) } diff --git a/cmd/init.sh b/cmd/init.sh index b15e7f47..585f6f1d 100644 --- a/cmd/init.sh +++ b/cmd/init.sh @@ -6,6 +6,85 @@ replace_template() { sed -i "s|$1|$replacement|g" -- "$3" } +# Run after copying/restoring rules on every Pod initialization. Do not chown /data. +prepare_reloader_rules() { + reloader_uid=${RELOADER_UID:-1000} + reloader_gid=${RELOADER_GID:-2000} + for reloader_id in "$reloader_uid" "$reloader_gid"; do + case "$reloader_id" in + ''|*[!0-9]*) echo "Reloader UID/GID must be numeric" >&2; return 1;; + esac + done + reloader_version=$1 + reloader_store= + set -- "$PROM_CONFIG_PATH/rules" + if [ -n "$PROM_PERSISTENT_DIR" ]; then + case "$reloader_version" in + ''|.|..|*/*) echo "Invalid persistent rules version" >&2; return 1;; + esac + reloader_store="$PROM_PERSISTENT_DIR/latest-rules/$reloader_version" + set -- "$@" "$PROM_PERSISTENT_DIR/latest-rules" "$reloader_store" + fi + for reloader_dir do + if [ -L "$reloader_dir" ] || { [ -e "$reloader_dir" ] && [ ! -d "$reloader_dir" ]; }; then + echo "Rules path must be a directory, not a symlink: $reloader_dir" >&2 + return 1 + fi + mkdir -p -- "$reloader_dir" && + chown "$reloader_uid:$reloader_gid" "$reloader_dir" && + chmod u+rwx,go-w "$reloader_dir" || return 1 + done + for reloader_dir in "$PROM_CONFIG_PATH/rules" "$reloader_store"; do + [ -n "$reloader_dir" ] || continue + for reloader_file in "$reloader_dir"/*.yml; do + [ -e "$reloader_file" ] || [ -L "$reloader_file" ] || continue + if [ -L "$reloader_file" ] || [ ! -f "$reloader_file" ] || [ -z "$(find "$reloader_file" -prune -type f -links 1 -print)" ]; then + echo "Rule must be a regular file: $reloader_file" >&2 + return 1 + fi + chown "$reloader_uid:$reloader_gid" "$reloader_file" && + chmod u+rw,go-w,a-x "$reloader_file" || return 1 + done + done +} + +# Restore current-version customizations automatically, as before. Validate the +# filesystem entries rather than treating all historical user edits as untrusted. +reset_rules() { + rules_dir="$PROM_CONFIG_PATH/rules" + if [ -L "$rules_dir" ] || { [ -e "$rules_dir" ] && [ ! -d "$rules_dir" ]; }; then + echo "Rules path must be a directory" >&2; return 1 + fi + mkdir -p -- "$rules_dir" || return 1 + for entry in "$rules_dir"/* "$rules_dir"/.[!.]* "$rules_dir"/..?*; do + [ -e "$entry" ] || [ -L "$entry" ] || continue + rm -rf -- "$entry" || return 1 + done +} + +restore_rules() { + [ -n "$PROM_PERSISTENT_DIR" ] || return 0 + case "$1" in + ''|.|..|*/*) echo "Invalid persisted rules version" >&2; return 1;; + esac + saved_root="$PROM_PERSISTENT_DIR/latest-rules" + saved_version="$saved_root/$1" + if [ -L "$PROM_PERSISTENT_DIR" ] || [ -L "$saved_root" ] || [ -L "$saved_version" ]; then + echo "Persisted rule directories must not be symbolic links" >&2; return 1 + fi + if [ -e "$saved_version" ] && [ ! -d "$saved_version" ]; then + echo "Persisted rules path must be a directory" >&2; return 1 + fi + [ -d "$saved_version" ] || return 0 + for rule in "$saved_version"/*.rules.yml; do + [ -e "$rule" ] || [ -L "$rule" ] || continue + if [ -L "$rule" ] || [ ! -f "$rule" ] || [ -z "$(find "$rule" -prune -type f -links 1 -print)" ]; then + echo "Persisted rule must be a regular file with one link: $rule" >&2; return 1 + fi + cp -f -- "$rule" "$PROM_CONFIG_PATH/rules/${rule##*/}" || return 1 + done +} + # Multiline replacements would become new sed commands or malformed configuration. for value in "$DM_CLUSTER_NAME" "$TIDB_CLUSTER_NAME" "$TIDB_CLUSTER_NAMESPACE" \ "$GF_DM_PROMETHEUS_URL" "$GF_K8S_PROMETHEUS_URL" "$GF_TIDB_PROMETHEUS_URL"; do @@ -34,9 +113,7 @@ if [ -n "$DM_CLUSTER_NAME" ]; then replace_template 'Test-Cluster-DM' "${DM_CLUSTER_NAME}-DM-Professional" "$GF_PROVISIONING_PATH/dashboards"/DM-Monitor-Professional.json # Rules - if [ ! -d "$PROM_CONFIG_PATH/rules" ]; then - mkdir -p -- "$PROM_CONFIG_PATH/rules" - fi + reset_rules || exit 1 echo $META_TYPE echo $META_INSTANCE @@ -47,12 +124,7 @@ if [ -n "$DM_CLUSTER_NAME" ]; then fi replace_template 'ENV_LABELS_ENV' "${DM_CLUSTER_NAME}" "$PROM_CONFIG_PATH/rules/dm_worker.rules.yml" || exit 1 - # Copy Persistent rules to override raw files - if [ -n "$PROM_PERSISTENT_DIR" ]; then - if [ -d "$PROM_PERSISTENT_DIR/latest-rules/${DM_VERSION##*/}" ]; then - cp -f -- "$PROM_PERSISTENT_DIR/latest-rules/${DM_VERSION##*/}"/dm_worker.rules.yml "$PROM_CONFIG_PATH/rules" - fi - fi + restore_rules "${DM_VERSION##*/}" || exit 1 # Data sources if [ -n "$GF_DATASOURCE_PATH" ]; then @@ -61,6 +133,7 @@ if [ -n "$DM_CLUSTER_NAME" ]; then fi cp -f -- /tmp/dm-cluster-datasource.yaml "$GF_DATASOURCE_PATH/" fi + prepare_reloader_rules "${DM_VERSION##*/}" || exit 1 echo "running for DM done" exit fi @@ -153,29 +226,22 @@ for f in "$GF_PROVISIONING_PATH/dashboards"/*.json; do done # Rules -if [ ! -d "$PROM_CONFIG_PATH/rules" ]; then - mkdir -p -- "$PROM_CONFIG_PATH/rules" -else - rm -rf -- "$PROM_CONFIG_PATH/rules"/* -fi +reset_rules || exit 1 echo $META_TYPE echo $META_INSTANCE echo $META_VALUE -cp -- /tmp/*.rules.yml "$PROM_CONFIG_PATH/rules" +cp -- /tmp/*.rules.yml "$PROM_CONFIG_PATH/rules" || exit 1 for file in "$PROM_CONFIG_PATH/rules"/*; do replace_template 'ENV_LABELS_ENV' "${TIDB_CLUSTER_NAME}" "$file" sed -i 's%job=%component=%g' "$file" sed -i -e 's%\(by\s(\)job\(,.*)\)%\1component\2%g' -e 's%\(by\s(.*\),job,\(.*)\)%\1,component,\2%g' -e 's%\(by\s(.*,\)job)%\1component)%g' -e 's%\(by\s(\)job)%\1component)%g' "$file" done -# Copy Persistent rules to override raw files -if [ -n "$PROM_PERSISTENT_DIR" ]; then - if [ -d "$PROM_PERSISTENT_DIR/latest-rules/${TIDB_VERSION##*/}" ]; then - cp -f -- "$PROM_PERSISTENT_DIR/latest-rules/${TIDB_VERSION##*/}"/*.rules.yml "$PROM_CONFIG_PATH/rules" - fi -fi +restore_rules "${TIDB_VERSION##*/}" || exit 1 # Datasources +prepare_reloader_rules "${TIDB_VERSION##*/}" || exit 1 + if [ -n "$GF_DATASOURCE_PATH" ]; then if [ -n "$GF_K8S_PROMETHEUS_URL" ]; then replace_template 'http://prometheus-k8s.monitoring.svc:9090' "${GF_K8S_PROMETHEUS_URL}" /tmp/k8s-datasource.yaml diff --git a/cmd/persisted_rules_test.go b/cmd/persisted_rules_test.go new file mode 100644 index 00000000..b4342c19 --- /dev/null +++ b/cmd/persisted_rules_test.go @@ -0,0 +1,140 @@ +package main + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "runtime" + "strings" + "testing" +) + +func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("initializer requires POSIX shell") + } + for _, source := range []string{"init.sh", "../platform-monitoring/operator/init.sh", "../monitor-snapshot/master/operator/init.sh"} { + for _, dm := range []bool{false, true} { + for _, scenario := range []string{"default", "custom-name", "symlink", "hardlink", "directory-link", "invalid-version"} { + t.Run(source+"/"+map[bool]string{false: "tidb", true: "dm"}[dm]+"/"+scenario, func(t *testing.T) { + root := t.TempDir() + assets, output, persistent := filepath.Join(root, "assets"), filepath.Join(root, "prometheus"), filepath.Join(root, "persistent") + for _, dir := range []string{assets, filepath.Join(output, "rules"), filepath.Join(persistent, "latest-rules", "v1")} { + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatal(err) + } + } + extraPath := "" + if runtime.GOOS == "darwin" { + gsed, err := exec.LookPath("gsed") + if err != nil { + t.Skip("GNU sed required on macOS") + } + bin := filepath.Join(root, "bin") + if err := os.Mkdir(bin, 0755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(gsed, filepath.Join(bin, "sed")); err != nil { + t.Fatal(err) + } + extraPath = bin + string(os.PathListSeparator) + } + script, err := os.ReadFile(source) + if err != nil { + t.Fatal(err) + } + for _, match := range regexp.MustCompile(`/tmp/([A-Za-z0-9_.-]+)`).FindAllStringSubmatch(string(script), -1) { + name := match[1] + if filepath.Ext(name) != ".json" && filepath.Ext(name) != ".yml" && filepath.Ext(name) != ".yaml" { + continue + } + if err := os.WriteFile(filepath.Join(assets, name), []byte("{}"), 0644); err != nil { + t.Fatal(err) + } + } + name := "test.rules.yml" + if dm { + name = "dm_worker.rules.yml" + } + bundled, restored := "groups: []\n# bundled\n", "groups: []\n# reviewed custom rules\n" + if err := os.WriteFile(filepath.Join(assets, name), []byte(bundled), 0644); err != nil { + t.Fatal(err) + } + old := filepath.Join(output, "rules", ".stale.rules.yml") + if err := os.WriteFile(old, []byte("groups: []"), 0644); err != nil { + t.Fatal(err) + } + saved := filepath.Join(persistent, "latest-rules", "v1", name) + if err := os.WriteFile(saved, []byte(restored), 0644); err != nil { + t.Fatal(err) + } + switch scenario { + case "custom-name": + if err := os.WriteFile(filepath.Join(filepath.Dir(saved), "foreign.rules.yml"), []byte(restored), 0644); err != nil { + t.Fatal(err) + } + case "symlink": + if err := os.Remove(saved); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(assets, name), saved); err != nil { + t.Fatal(err) + } + case "directory-link": + if err := os.Rename(filepath.Dir(saved), filepath.Join(root, "untrusted")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(root, "untrusted"), filepath.Dir(saved)); err != nil { + t.Fatal(err) + } + case "hardlink": + if err := os.Link(saved, filepath.Join(filepath.Dir(saved), "linked.rules.yml")); err != nil { + t.Fatal(err) + } + } + scriptPath := filepath.Join(root, "init.sh") + if err := os.WriteFile(scriptPath, []byte(strings.ReplaceAll(string(script), "/tmp/", assets+"/")), 0644); err != nil { + t.Fatal(err) + } + cluster := "" + if dm { + cluster = "test" + } + cmd := exec.Command("sh", scriptPath) + cmd.Env = append(os.Environ(), "DM_CLUSTER_NAME="+cluster, "TIDB_CLUSTER_NAME=test", "DM_VERSION="+map[bool]string{true: "..", false: "v1"}[scenario == "invalid-version"], "TIDB_VERSION="+map[bool]string{true: "..", false: "v1"}[scenario == "invalid-version"], "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana"), "PROM_CONFIG_PATH="+output, "PROM_PERSISTENT_DIR="+persistent, fmt.Sprintf("RELOADER_UID=%d", os.Geteuid()), fmt.Sprintf("RELOADER_GID=%d", os.Getegid()), "GF_DATASOURCE_PATH=", "PATH="+extraPath+os.Getenv("PATH")) + out, err := cmd.CombinedOutput() + success := scenario == "default" || scenario == "custom-name" + if success && err != nil { + t.Fatalf("initializer failed: %v %s", err, out) + } + if !success && err == nil { + t.Fatalf("unsafe restore succeeded: %s", out) + } + if !success { + return + } + want := restored + got, err := os.ReadFile(filepath.Join(output, "rules", name)) + if err != nil || string(got) != want { + t.Fatalf("rules = %q, want %q: %v", got, want, err) + } + if scenario == "custom-name" { + custom, err := os.ReadFile(filepath.Join(output, "rules", "foreign.rules.yml")) + if err != nil || string(custom) != restored { + t.Fatal("custom rule was not restored", err) + } + } + if _, err := os.Lstat(old); !os.IsNotExist(err) { + t.Fatalf("foreign active rule survived: %v", err) + } + got, err = os.ReadFile(saved) + if err != nil || string(got) != restored { + t.Fatalf("PVC data was modified: %q %v", got, err) + } + }) + } + } + } +} diff --git a/monitor-snapshot/master/operator/Dockerfile b/monitor-snapshot/master/operator/Dockerfile index 08314550..57bb4729 100644 --- a/monitor-snapshot/master/operator/Dockerfile +++ b/monitor-snapshot/master/operator/Dockerfile @@ -3,6 +3,7 @@ FROM busybox:1.37.0 # Add user and group to support run it with non-root. ARG PINGCAP_UID=1000 ARG PINGCAP_GID=2000 +ENV RELOADER_UID=${PINGCAP_UID} RELOADER_GID=${PINGCAP_GID} RUN addgroup -g ${PINGCAP_GID} pingcap && \ adduser -u ${PINGCAP_UID} -G pingcap -h /home/pingcap -D pingcap diff --git a/monitor-snapshot/master/operator/datasources/k8s-datasource.yaml b/monitor-snapshot/master/operator/datasources/k8s-datasource.yaml index 9b8f9694..ea99ca8f 100644 --- a/monitor-snapshot/master/operator/datasources/k8s-datasource.yaml +++ b/monitor-snapshot/master/operator/datasources/k8s-datasource.yaml @@ -1,7 +1,7 @@ apiVersion: 1 datasources: - access: proxy - editable: true + editable: false name: k8s-cluster orgId: 1 type: prometheus diff --git a/monitor-snapshot/master/operator/init.sh b/monitor-snapshot/master/operator/init.sh index a9774e9f..8b96af14 100755 --- a/monitor-snapshot/master/operator/init.sh +++ b/monitor-snapshot/master/operator/init.sh @@ -6,6 +6,85 @@ replace_template() { sed -i "s|$1|$replacement|g" -- "$3" } +# Run after copying/restoring rules on every Pod initialization. Do not chown /data. +prepare_reloader_rules() { + reloader_uid=${RELOADER_UID:-1000} + reloader_gid=${RELOADER_GID:-2000} + for reloader_id in "$reloader_uid" "$reloader_gid"; do + case "$reloader_id" in + ''|*[!0-9]*) echo "Reloader UID/GID must be numeric" >&2; return 1;; + esac + done + reloader_version=$1 + reloader_store= + set -- "$PROM_CONFIG_PATH/rules" + if [ -n "$PROM_PERSISTENT_DIR" ]; then + case "$reloader_version" in + ''|.|..|*/*) echo "Invalid persistent rules version" >&2; return 1;; + esac + reloader_store="$PROM_PERSISTENT_DIR/latest-rules/$reloader_version" + set -- "$@" "$PROM_PERSISTENT_DIR/latest-rules" "$reloader_store" + fi + for reloader_dir do + if [ -L "$reloader_dir" ] || { [ -e "$reloader_dir" ] && [ ! -d "$reloader_dir" ]; }; then + echo "Rules path must be a directory, not a symlink: $reloader_dir" >&2 + return 1 + fi + mkdir -p -- "$reloader_dir" && + chown "$reloader_uid:$reloader_gid" "$reloader_dir" && + chmod u+rwx,go-w "$reloader_dir" || return 1 + done + for reloader_dir in "$PROM_CONFIG_PATH/rules" "$reloader_store"; do + [ -n "$reloader_dir" ] || continue + for reloader_file in "$reloader_dir"/*.yml; do + [ -e "$reloader_file" ] || [ -L "$reloader_file" ] || continue + if [ -L "$reloader_file" ] || [ ! -f "$reloader_file" ] || [ -z "$(find "$reloader_file" -prune -type f -links 1 -print)" ]; then + echo "Rule must be a regular file: $reloader_file" >&2 + return 1 + fi + chown "$reloader_uid:$reloader_gid" "$reloader_file" && + chmod u+rw,go-w,a-x "$reloader_file" || return 1 + done + done +} + +# Restore current-version customizations automatically, as before. Validate the +# filesystem entries rather than treating all historical user edits as untrusted. +reset_rules() { + rules_dir="$PROM_CONFIG_PATH/rules" + if [ -L "$rules_dir" ] || { [ -e "$rules_dir" ] && [ ! -d "$rules_dir" ]; }; then + echo "Rules path must be a directory" >&2; return 1 + fi + mkdir -p -- "$rules_dir" || return 1 + for entry in "$rules_dir"/* "$rules_dir"/.[!.]* "$rules_dir"/..?*; do + [ -e "$entry" ] || [ -L "$entry" ] || continue + rm -rf -- "$entry" || return 1 + done +} + +restore_rules() { + [ -n "$PROM_PERSISTENT_DIR" ] || return 0 + case "$1" in + ''|.|..|*/*) echo "Invalid persisted rules version" >&2; return 1;; + esac + saved_root="$PROM_PERSISTENT_DIR/latest-rules" + saved_version="$saved_root/$1" + if [ -L "$PROM_PERSISTENT_DIR" ] || [ -L "$saved_root" ] || [ -L "$saved_version" ]; then + echo "Persisted rule directories must not be symbolic links" >&2; return 1 + fi + if [ -e "$saved_version" ] && [ ! -d "$saved_version" ]; then + echo "Persisted rules path must be a directory" >&2; return 1 + fi + [ -d "$saved_version" ] || return 0 + for rule in "$saved_version"/*.rules.yml; do + [ -e "$rule" ] || [ -L "$rule" ] || continue + if [ -L "$rule" ] || [ ! -f "$rule" ] || [ -z "$(find "$rule" -prune -type f -links 1 -print)" ]; then + echo "Persisted rule must be a regular file with one link: $rule" >&2; return 1 + fi + cp -f -- "$rule" "$PROM_CONFIG_PATH/rules/${rule##*/}" || return 1 + done +} + # Multiline replacements would become new sed commands or malformed configuration. for value in "$DM_CLUSTER_NAME" "$TIDB_CLUSTER_NAME" "$TIDB_CLUSTER_NAMESPACE" \ "$GF_DM_PROMETHEUS_URL" "$GF_K8S_PROMETHEUS_URL" "$GF_TIDB_PROMETHEUS_URL"; do @@ -34,9 +113,7 @@ if [ -n "$DM_CLUSTER_NAME" ]; then replace_template 'Test-Cluster-DM' "${DM_CLUSTER_NAME}-DM-Professional" "$GF_PROVISIONING_PATH/dashboards"/DM-Monitor-Professional.json # Rules - if [ ! -d "$PROM_CONFIG_PATH/rules" ]; then - mkdir -p -- "$PROM_CONFIG_PATH/rules" - fi + reset_rules || exit 1 echo $META_TYPE echo $META_INSTANCE @@ -47,12 +124,7 @@ if [ -n "$DM_CLUSTER_NAME" ]; then fi replace_template 'ENV_LABELS_ENV' "${DM_CLUSTER_NAME}" "$PROM_CONFIG_PATH/rules/dm_worker.rules.yml" || exit 1 - # Copy Persistent rules to override raw files - if [ -n "$PROM_PERSISTENT_DIR" ]; then - if [ -d "$PROM_PERSISTENT_DIR/latest-rules/${DM_VERSION##*/}" ]; then - cp -f -- "$PROM_PERSISTENT_DIR/latest-rules/${DM_VERSION##*/}"/dm_worker.rules.yml "$PROM_CONFIG_PATH/rules" - fi - fi + restore_rules "${DM_VERSION##*/}" || exit 1 # Data sources if [ -n "$GF_DATASOURCE_PATH" ]; then @@ -61,6 +133,7 @@ if [ -n "$DM_CLUSTER_NAME" ]; then fi cp -f -- /tmp/dm-cluster-datasource.yaml "$GF_DATASOURCE_PATH/" fi + prepare_reloader_rules "${DM_VERSION##*/}" || exit 1 echo "running for DM done" exit fi @@ -161,29 +234,22 @@ for f in "$GF_PROVISIONING_PATH/dashboards"/*.json; do done # Rules -if [ ! -d "$PROM_CONFIG_PATH/rules" ]; then - mkdir -p -- "$PROM_CONFIG_PATH/rules" -else - rm -rf -- "$PROM_CONFIG_PATH/rules"/* -fi +reset_rules || exit 1 echo $META_TYPE echo $META_INSTANCE echo $META_VALUE -cp -- /tmp/*.rules.yml "$PROM_CONFIG_PATH/rules" +cp -- /tmp/*.rules.yml "$PROM_CONFIG_PATH/rules" || exit 1 for file in "$PROM_CONFIG_PATH/rules"/*; do sed -i 's/ENV_LABELS_ENV/Cluster/g' "$file" sed -i 's%job=%component=%g' "$file" sed -i -e 's%\(by\s(\)job\(,.*)\)%\1component\2%g' -e 's%\(by\s(.*\),job,\(.*)\)%\1,component,\2%g' -e 's%\(by\s(.*,\)job)%\1component)%g' -e 's%\(by\s(\)job)%\1component)%g' "$file" done -# Copy Persistent rules to override raw files -if [ -n "$PROM_PERSISTENT_DIR" ]; then - if [ -d "$PROM_PERSISTENT_DIR/latest-rules/${TIDB_VERSION##*/}" ]; then - cp -f -- "$PROM_PERSISTENT_DIR/latest-rules/${TIDB_VERSION##*/}"/*.rules.yml "$PROM_CONFIG_PATH/rules" - fi -fi +restore_rules "${TIDB_VERSION##*/}" || exit 1 # Datasources +prepare_reloader_rules "${TIDB_VERSION##*/}" || exit 1 + if [ -n "$GF_DATASOURCE_PATH" ]; then if [ -n "$GF_K8S_PROMETHEUS_URL" ]; then replace_template 'http://prometheus-k8s.monitoring.svc:9090' "${GF_K8S_PROMETHEUS_URL}" /tmp/k8s-datasource.yaml diff --git a/platform-monitoring/operator/Dockerfile b/platform-monitoring/operator/Dockerfile index 08314550..57bb4729 100644 --- a/platform-monitoring/operator/Dockerfile +++ b/platform-monitoring/operator/Dockerfile @@ -3,6 +3,7 @@ FROM busybox:1.37.0 # Add user and group to support run it with non-root. ARG PINGCAP_UID=1000 ARG PINGCAP_GID=2000 +ENV RELOADER_UID=${PINGCAP_UID} RELOADER_GID=${PINGCAP_GID} RUN addgroup -g ${PINGCAP_GID} pingcap && \ adduser -u ${PINGCAP_UID} -G pingcap -h /home/pingcap -D pingcap diff --git a/platform-monitoring/operator/datasource/k8s-datasource.yaml b/platform-monitoring/operator/datasource/k8s-datasource.yaml index 9b8f9694..ea99ca8f 100644 --- a/platform-monitoring/operator/datasource/k8s-datasource.yaml +++ b/platform-monitoring/operator/datasource/k8s-datasource.yaml @@ -1,7 +1,7 @@ apiVersion: 1 datasources: - access: proxy - editable: true + editable: false name: k8s-cluster orgId: 1 type: prometheus diff --git a/platform-monitoring/operator/init.sh b/platform-monitoring/operator/init.sh index a9774e9f..8b96af14 100755 --- a/platform-monitoring/operator/init.sh +++ b/platform-monitoring/operator/init.sh @@ -6,6 +6,85 @@ replace_template() { sed -i "s|$1|$replacement|g" -- "$3" } +# Run after copying/restoring rules on every Pod initialization. Do not chown /data. +prepare_reloader_rules() { + reloader_uid=${RELOADER_UID:-1000} + reloader_gid=${RELOADER_GID:-2000} + for reloader_id in "$reloader_uid" "$reloader_gid"; do + case "$reloader_id" in + ''|*[!0-9]*) echo "Reloader UID/GID must be numeric" >&2; return 1;; + esac + done + reloader_version=$1 + reloader_store= + set -- "$PROM_CONFIG_PATH/rules" + if [ -n "$PROM_PERSISTENT_DIR" ]; then + case "$reloader_version" in + ''|.|..|*/*) echo "Invalid persistent rules version" >&2; return 1;; + esac + reloader_store="$PROM_PERSISTENT_DIR/latest-rules/$reloader_version" + set -- "$@" "$PROM_PERSISTENT_DIR/latest-rules" "$reloader_store" + fi + for reloader_dir do + if [ -L "$reloader_dir" ] || { [ -e "$reloader_dir" ] && [ ! -d "$reloader_dir" ]; }; then + echo "Rules path must be a directory, not a symlink: $reloader_dir" >&2 + return 1 + fi + mkdir -p -- "$reloader_dir" && + chown "$reloader_uid:$reloader_gid" "$reloader_dir" && + chmod u+rwx,go-w "$reloader_dir" || return 1 + done + for reloader_dir in "$PROM_CONFIG_PATH/rules" "$reloader_store"; do + [ -n "$reloader_dir" ] || continue + for reloader_file in "$reloader_dir"/*.yml; do + [ -e "$reloader_file" ] || [ -L "$reloader_file" ] || continue + if [ -L "$reloader_file" ] || [ ! -f "$reloader_file" ] || [ -z "$(find "$reloader_file" -prune -type f -links 1 -print)" ]; then + echo "Rule must be a regular file: $reloader_file" >&2 + return 1 + fi + chown "$reloader_uid:$reloader_gid" "$reloader_file" && + chmod u+rw,go-w,a-x "$reloader_file" || return 1 + done + done +} + +# Restore current-version customizations automatically, as before. Validate the +# filesystem entries rather than treating all historical user edits as untrusted. +reset_rules() { + rules_dir="$PROM_CONFIG_PATH/rules" + if [ -L "$rules_dir" ] || { [ -e "$rules_dir" ] && [ ! -d "$rules_dir" ]; }; then + echo "Rules path must be a directory" >&2; return 1 + fi + mkdir -p -- "$rules_dir" || return 1 + for entry in "$rules_dir"/* "$rules_dir"/.[!.]* "$rules_dir"/..?*; do + [ -e "$entry" ] || [ -L "$entry" ] || continue + rm -rf -- "$entry" || return 1 + done +} + +restore_rules() { + [ -n "$PROM_PERSISTENT_DIR" ] || return 0 + case "$1" in + ''|.|..|*/*) echo "Invalid persisted rules version" >&2; return 1;; + esac + saved_root="$PROM_PERSISTENT_DIR/latest-rules" + saved_version="$saved_root/$1" + if [ -L "$PROM_PERSISTENT_DIR" ] || [ -L "$saved_root" ] || [ -L "$saved_version" ]; then + echo "Persisted rule directories must not be symbolic links" >&2; return 1 + fi + if [ -e "$saved_version" ] && [ ! -d "$saved_version" ]; then + echo "Persisted rules path must be a directory" >&2; return 1 + fi + [ -d "$saved_version" ] || return 0 + for rule in "$saved_version"/*.rules.yml; do + [ -e "$rule" ] || [ -L "$rule" ] || continue + if [ -L "$rule" ] || [ ! -f "$rule" ] || [ -z "$(find "$rule" -prune -type f -links 1 -print)" ]; then + echo "Persisted rule must be a regular file with one link: $rule" >&2; return 1 + fi + cp -f -- "$rule" "$PROM_CONFIG_PATH/rules/${rule##*/}" || return 1 + done +} + # Multiline replacements would become new sed commands or malformed configuration. for value in "$DM_CLUSTER_NAME" "$TIDB_CLUSTER_NAME" "$TIDB_CLUSTER_NAMESPACE" \ "$GF_DM_PROMETHEUS_URL" "$GF_K8S_PROMETHEUS_URL" "$GF_TIDB_PROMETHEUS_URL"; do @@ -34,9 +113,7 @@ if [ -n "$DM_CLUSTER_NAME" ]; then replace_template 'Test-Cluster-DM' "${DM_CLUSTER_NAME}-DM-Professional" "$GF_PROVISIONING_PATH/dashboards"/DM-Monitor-Professional.json # Rules - if [ ! -d "$PROM_CONFIG_PATH/rules" ]; then - mkdir -p -- "$PROM_CONFIG_PATH/rules" - fi + reset_rules || exit 1 echo $META_TYPE echo $META_INSTANCE @@ -47,12 +124,7 @@ if [ -n "$DM_CLUSTER_NAME" ]; then fi replace_template 'ENV_LABELS_ENV' "${DM_CLUSTER_NAME}" "$PROM_CONFIG_PATH/rules/dm_worker.rules.yml" || exit 1 - # Copy Persistent rules to override raw files - if [ -n "$PROM_PERSISTENT_DIR" ]; then - if [ -d "$PROM_PERSISTENT_DIR/latest-rules/${DM_VERSION##*/}" ]; then - cp -f -- "$PROM_PERSISTENT_DIR/latest-rules/${DM_VERSION##*/}"/dm_worker.rules.yml "$PROM_CONFIG_PATH/rules" - fi - fi + restore_rules "${DM_VERSION##*/}" || exit 1 # Data sources if [ -n "$GF_DATASOURCE_PATH" ]; then @@ -61,6 +133,7 @@ if [ -n "$DM_CLUSTER_NAME" ]; then fi cp -f -- /tmp/dm-cluster-datasource.yaml "$GF_DATASOURCE_PATH/" fi + prepare_reloader_rules "${DM_VERSION##*/}" || exit 1 echo "running for DM done" exit fi @@ -161,29 +234,22 @@ for f in "$GF_PROVISIONING_PATH/dashboards"/*.json; do done # Rules -if [ ! -d "$PROM_CONFIG_PATH/rules" ]; then - mkdir -p -- "$PROM_CONFIG_PATH/rules" -else - rm -rf -- "$PROM_CONFIG_PATH/rules"/* -fi +reset_rules || exit 1 echo $META_TYPE echo $META_INSTANCE echo $META_VALUE -cp -- /tmp/*.rules.yml "$PROM_CONFIG_PATH/rules" +cp -- /tmp/*.rules.yml "$PROM_CONFIG_PATH/rules" || exit 1 for file in "$PROM_CONFIG_PATH/rules"/*; do sed -i 's/ENV_LABELS_ENV/Cluster/g' "$file" sed -i 's%job=%component=%g' "$file" sed -i -e 's%\(by\s(\)job\(,.*)\)%\1component\2%g' -e 's%\(by\s(.*\),job,\(.*)\)%\1,component,\2%g' -e 's%\(by\s(.*,\)job)%\1component)%g' -e 's%\(by\s(\)job)%\1component)%g' "$file" done -# Copy Persistent rules to override raw files -if [ -n "$PROM_PERSISTENT_DIR" ]; then - if [ -d "$PROM_PERSISTENT_DIR/latest-rules/${TIDB_VERSION##*/}" ]; then - cp -f -- "$PROM_PERSISTENT_DIR/latest-rules/${TIDB_VERSION##*/}"/*.rules.yml "$PROM_CONFIG_PATH/rules" - fi -fi +restore_rules "${TIDB_VERSION##*/}" || exit 1 # Datasources +prepare_reloader_rules "${TIDB_VERSION##*/}" || exit 1 + if [ -n "$GF_DATASOURCE_PATH" ]; then if [ -n "$GF_K8S_PROMETHEUS_URL" ]; then replace_template 'http://prometheus-k8s.monitoring.svc:9090' "${GF_K8S_PROMETHEUS_URL}" /tmp/k8s-datasource.yaml diff --git a/reload/CONTAINER_SECURITY.md b/reload/CONTAINER_SECURITY.md new file mode 100644 index 00000000..4cf9770c --- /dev/null +++ b/reload/CONTAINER_SECURITY.md @@ -0,0 +1,77 @@ +# Rule persistence and reloader identity + +Updating the reloader does not require a new initializer, new CR fields, a +restore opt-in, or a change to `alertManagerRulesVersion`. Keep the deployed +initializer and its version. Rules continue to be restored from the existing +current-version directory on the PVC. Authentication/TLS settings follow +[UPGRADING.md](UPGRADING.md); this change adds no web configuration requirements. + +## Existing initializer images + +The Linux reloader image starts `/bin/reload` with root privileges for local +filesystem preparation only. Before loading web credentials, starting watchers +or listening, it prepares the active rules directory and the current +`latest-rules/` directory for the image's existing `pingcap` account +(default UID 1000/GID 2000). It does not recursively change `/data`, Prometheus, +Grafana or other rule versions. Directories and regular `.yml` rule files lose +unnecessary write/execute permissions; existing read permissions are retained. +Symlink paths and symlink, hard-linked or non-regular rule files are rejected. + +The process clears supplementary groups, switches GID/UID, clears capabilities, +sets `no_new_privs`, and replaces itself before starting the service. There is no +root supervisor. A preparation or privilege-drop failure stops startup rather +than serving as root. The image intentionally retains root as its startup user: +this is **not** an image that is non-root for its entire lifetime. A scanner +checking only the Docker `USER` will still report root. + +Pod rebuilds rerun the old initializer and may recreate root-owned active files. +The reloader therefore repeats preparation each time, without changing rule +contents or choosing a different version. Upgrading the reloader alone does not +make a future fully non-root image compatible with every old initializer. + +## New initializer images and fully non-root deployments + +Current TiDB and DM initializer sources prepare the same rule permissions after +generating and restoring rules on every startup. Once deployed, they also allow +the reloader to start directly as UID 1000/GID 2000, including after Pod rebuilds. +An explicit non-root runtime user bypasses reloader ownership preparation; +volume permissions must already permit editing. No identity override is needed +for the compatible default startup path. + +Custom builds retain the existing `PINGCAP_UID`/`PINGCAP_GID` build arguments. +The reloader looks up the image's `pingcap` account; initializer permission +preparation uses matching `RELOADER_UID`/`RELOADER_GID` defaults or existing env +configuration (`spec.initializer.envs`, and `spec.dm.initializer.envs` for DM). +Explicit custom runtime identities must match the prepared ownership. Mounted +web credentials must be readable by the serving identity, as with any non-root +service; root-only custom Secret modes may require an access adjustment. + +The compatibility startup needs permission to chown/chmod rule files and change +UID/GID. Storage that prohibits ownership changes (for example root-squashed +NFS), or a security context dropping the required startup capabilities, needs +pre-provisioned permissions and an explicit non-root runtime identity. Existing +root-only configurations cannot be made universally non-root without those +filesystem permissions. + +## Rule restore behavior + +Current initializers continue to restore persisted `*.rules.yml` files for the +selected version automatically, including names not present in the image's +bundle. Both TiDB and DM first clear only the dedicated active rules directory, +including hidden leftovers, and then install bundled rules and restore saved +ones. Invalid paths, symlink/hard-linked/non-regular saved rules, and copy errors +stop initialization. The PVC copies are not deleted. There is no +`PROM_RESTORE_RULES` option and no cross-version directory discovery or migration. + +The existing version-selection semantics are unchanged: without an explicit +`alertManagerRulesVersion`, changing the initializer tag changes the rule store +version. Retaining customizations across that change uses the existing field; +this PR does not silently overlay rules from another version on a new bundle. + +Filesystem validation and authenticated editing do not establish the provenance +of rules saved before authentication was enabled. Existing customizations remain +in use; if compromise is suspected, review/replace them as part of incident +response. This change does not claim automatic detection of historical tampering. + +Provisioned k8s Grafana datasources are non-editable, consistent with their +sibling definitions. Change the provisioning source to update their settings. diff --git a/reload/Dockerfile b/reload/Dockerfile index 13c7e065..87273ef9 100644 --- a/reload/Dockerfile +++ b/reload/Dockerfile @@ -7,5 +7,7 @@ RUN addgroup -g ${PINGCAP_GID} pingcap && \ adduser -u ${PINGCAP_UID} -G pingcap -h /home/pingcap -D pingcap COPY --chmod=755 ./build/linux/reload /bin/reload +# /bin/reload prepares legacy volumes, then execs as the pingcap account before serving. +USER 0:0 ENTRYPOINT ["/bin/reload"] CMD ["--watch-path=/etc/prometheus/rules", "--prometheus-url=http://127.0.0.1:9090"] diff --git a/reload/Dockerfile_buildx b/reload/Dockerfile_buildx index 61ffd038..c818fd72 100644 --- a/reload/Dockerfile_buildx +++ b/reload/Dockerfile_buildx @@ -13,6 +13,8 @@ RUN addgroup -g ${PINGCAP_GID} pingcap && \ adduser -u ${PINGCAP_UID} -G pingcap -h /home/pingcap -D pingcap COPY --from=builder --chmod=755 /app/main /bin/reload +# /bin/reload prepares legacy volumes, then execs as the pingcap account before serving. +USER 0:0 ENTRYPOINT [ "/bin/reload" ] CMD [ "--watch-path=/etc/prometheus/rules", \ "--prometheus-url=http://127.0.0.1:9090" ] diff --git a/reload/identity/identity_linux.go b/reload/identity/identity_linux.go new file mode 100644 index 00000000..3d8c15f0 --- /dev/null +++ b/reload/identity/identity_linux.go @@ -0,0 +1,176 @@ +//go:build linux + +// Package identity prepares legacy rule volumes before starting the service as non-root. +package identity + +import ( + "fmt" + "os" + "os/user" + "path/filepath" + "runtime" + "strconv" + "strings" + "syscall" + + "golang.org/x/sys/unix" +) + +// Prepare runs before loading credentials, starting watchers or listening. An +// explicit non-root runtime identity is already authoritative and is left alone. +func Prepare(watchDir, storeRoot, version string) error { + if os.Geteuid() != 0 { + return nil + } + account, err := user.Lookup("pingcap") + if err != nil { + return fmt.Errorf("find non-root image account: %w", err) + } + uid, err := strconv.Atoi(account.Uid) + if err != nil || uid <= 0 { + return fmt.Errorf("invalid non-root UID %q", account.Uid) + } + gid, err := strconv.Atoi(account.Gid) + if err != nil || gid <= 0 { + return fmt.Errorf("invalid non-root GID %q", account.Gid) + } + if err := preparePaths(watchDir, storeRoot, version, uid, gid); err != nil { + return err + } + return dropAndExec(uid, gid) +} + +func preparePaths(watchDir, storeRoot, version string, uid, gid int) error { + if version != "" && (version != filepath.Base(version) || version == "." || version == "..") { + return fmt.Errorf("invalid rules version %q", version) + } + if err := prepareDirectory(watchDir, false, true, uid, gid); err != nil { + return err + } + if version == "" { + return nil + } + // The volume root must already exist. Only create and change dedicated rule directories. + root, err := openDirectory(storeRoot, false) + if err != nil { + return err + } + root.Close() + parent := filepath.Join(storeRoot, "latest-rules") + if err := prepareDirectory(parent, true, false, uid, gid); err != nil { + return err + } + return prepareDirectory(filepath.Join(parent, version), true, true, uid, gid) +} + +// Open each component with NOFOLLOW, retaining directory descriptors so renames +// cannot redirect ownership changes to a symlink target. +func openDirectory(path string, create bool) (*os.File, error) { + if !filepath.IsAbs(path) || filepath.Clean(path) == "/" { + return nil, fmt.Errorf("rules path must be an absolute non-root directory: %q", path) + } + fd, err := unix.Open("/", unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) + if err != nil { + return nil, err + } + parts := strings.Split(filepath.Clean(path)[1:], "/") + for i, part := range parts { + next, openErr := unix.Openat(fd, part, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) + if openErr == unix.ENOENT && create && i == len(parts)-1 { + if err := unix.Mkdirat(fd, part, 0755); err != nil && err != unix.EEXIST { + unix.Close(fd) + return nil, err + } + next, openErr = unix.Openat(fd, part, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) + } + unix.Close(fd) + if openErr != nil { + return nil, fmt.Errorf("open rules directory %q: %w", path, openErr) + } + fd = next + } + return os.NewFile(uintptr(fd), path), nil +} + +func setOwnerMode(file *os.File, uid, gid int, directory bool) error { + var st unix.Stat_t + if err := unix.Fstat(int(file.Fd()), &st); err != nil { + return err + } + if !directory && (st.Mode&unix.S_IFMT != unix.S_IFREG || st.Nlink != 1) { + return fmt.Errorf("rule must be a regular file with one link: %s", file.Name()) + } + if int(st.Uid) != uid || int(st.Gid) != gid { + if err := file.Chown(uid, gid); err != nil { + return err + } + } + mode := os.FileMode(st.Mode&0644) | 0600 + if directory { + mode = os.FileMode(st.Mode&0055) | 0700 + } + return file.Chmod(mode) +} + +func prepareDirectory(path string, create, rules bool, uid, gid int) error { + dir, err := openDirectory(path, create) + if err != nil { + return err + } + defer dir.Close() + if err := setOwnerMode(dir, uid, gid, true); err != nil { + return fmt.Errorf("prepare %s: %w", path, err) + } + if !rules { + return nil + } + names, err := dir.Readdirnames(-1) + if err != nil { + return err + } + for _, name := range names { + if filepath.Ext(name) != ".yml" { + continue + } + fd, err := unix.Openat(int(dir.Fd()), name, unix.O_RDONLY|unix.O_NONBLOCK|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) + if err != nil { + return fmt.Errorf("open rule %s: %w", name, err) + } + file := os.NewFile(uintptr(fd), filepath.Join(path, name)) + err = setOwnerMode(file, uid, gid, false) + file.Close() + if err != nil { + return err + } + } + return nil +} + +func dropAndExec(uid, gid int) error { + // Go's syscall credential wrappers update all runtime threads. Lock this + // thread for the thread-local capability/prctl operations and subsequent exec. + runtime.LockOSThread() + if err := unix.Prctl(unix.PR_SET_KEEPCAPS, 0, 0, 0, 0); err != nil { + return err + } + if err := unix.Prctl(unix.PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); err != nil { + return err + } + if err := syscall.Setgroups([]int{}); err != nil { + return err + } + if err := syscall.Setgid(gid); err != nil { + return err + } + if err := syscall.Setuid(uid); err != nil { + return err + } + header := unix.CapUserHeader{Version: unix.LINUX_CAPABILITY_VERSION_3} + data := [2]unix.CapUserData{} + if err := unix.Capset(&header, &data[0]); err != nil { + return err + } + // Replace the entire process: no root supervisor, bootstrap marker, extra + // arguments, or inherited rule descriptors remain in the serving process. + return syscall.Exec("/proc/self/exe", os.Args, os.Environ()) +} diff --git a/reload/identity/identity_linux_test.go b/reload/identity/identity_linux_test.go new file mode 100644 index 00000000..f6ba8272 --- /dev/null +++ b/reload/identity/identity_linux_test.go @@ -0,0 +1,134 @@ +//go:build linux + +package identity + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "testing" +) + +func TestPreparePaths(t *testing.T) { + for _, attack := range []string{"none", "symlink-file", "hardlink-file", "symlink-dir", "symlink-parent", "fifo", "version"} { + t.Run(attack, func(t *testing.T) { + root := t.TempDir() + active, data := filepath.Join(root, "active"), filepath.Join(root, "data") + for _, p := range []string{active, data} { + if err := os.Mkdir(p, 0755); err != nil { + t.Fatal(err) + } + } + outside := filepath.Join(root, "outside.yml") + if err := os.WriteFile(outside, []byte("private"), 0400); err != nil { + t.Fatal(err) + } + rule := filepath.Join(active, "custom.rules.yml") + version := "tidb:v1" + switch attack { + case "symlink-file": + os.Symlink(outside, rule) + case "hardlink-file": + os.Link(outside, rule) + case "symlink-dir": + os.Remove(active) + os.Symlink(data, active) + case "symlink-parent": + os.Symlink(root, filepath.Join(data, "latest-rules")) + case "fifo": + if err := syscall.Mkfifo(rule, 0600); err != nil { + t.Fatal(err) + } + case "version": + version = "../other" + default: + os.WriteFile(rule, []byte("groups: []"), 0444) + } + err := preparePaths(active, data, version, os.Geteuid(), os.Getegid()) + if attack != "none" { + if err == nil { + t.Fatal("unsafe path accepted") + } + info, e := os.Stat(outside) + if e != nil || info.Mode().Perm() != 0400 { + t.Fatal("outside target modified", e) + } + return + } + if err != nil { + t.Fatal(err) + } + info, err := os.Stat(rule) + if err != nil || info.Mode().Perm() != 0644 { + t.Fatal("rule access not prepared", err) + } + if _, err := os.Stat(filepath.Join(data, "latest-rules", version)); err != nil { + t.Fatal(err) + } + }) + } +} + +func TestDropAndExec(t *testing.T) { + // This helper runs only in a disposable Linux test container as root. + if os.Getenv("RELOADER_IDENTITY_TEST_CHILD") == "1" { + if os.Geteuid() == 0 { + if err := dropAndExec(1000, 2000); err != nil { + t.Fatal(err) + } + t.Fatal("exec returned") + } + if os.Getuid() != 1000 || os.Geteuid() != 1000 || os.Getgid() != 2000 || os.Getegid() != 2000 { + t.Fatal("wrong runtime identity") + } + if err := Prepare("/does-not-exist", "/does-not-exist", "invalid"); err != nil { + t.Fatal("non-root runtime attempted ownership preparation", err) + } + groups, err := os.Getgroups() + if err != nil || len(groups) != 0 { + t.Fatal("supplementary groups retained", groups, err) + } + status, err := os.ReadFile("/proc/self/status") + if err != nil { + t.Fatal(err) + } + for _, line := range strings.Split(string(status), "\n") { + fields := strings.Fields(line) + if len(fields) == 0 { + continue + } + switch fields[0] { + case "CapInh:", "CapPrm:", "CapEff:", "CapAmb:": + if fields[1] != "0000000000000000" { + t.Fatal("capabilities retained", line) + } + case "NoNewPrivs:": + if fields[1] != "1" { + t.Fatal("no_new_privs unset") + } + case "Uid:": + if strings.Join(fields[1:], " ") != "1000 1000 1000 1000" { + t.Fatal("saved uid retained", line) + } + case "Gid:": + if strings.Join(fields[1:], " ") != "2000 2000 2000 2000" { + t.Fatal("saved gid retained", line) + } + } + } + if syscall.Setuid(0) == nil { + t.Fatal("regained root") + } + return + } + if os.Geteuid() != 0 { + t.Skip("requires disposable root container") + } + cmd := exec.Command(os.Args[0], "-test.run=^TestDropAndExec$") + cmd.Env = append(os.Environ(), "RELOADER_IDENTITY_TEST_CHILD=1") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("%v: %s", err, out) + } +} diff --git a/reload/identity/identity_other.go b/reload/identity/identity_other.go new file mode 100644 index 00000000..45cd0fbc --- /dev/null +++ b/reload/identity/identity_other.go @@ -0,0 +1,7 @@ +//go:build !linux + +package identity + +// Container privilege preparation is Linux-specific; native development keeps +// the caller's identity and filesystem permissions. +func Prepare(watchDir, storeRoot, version string) error { return nil } diff --git a/reload/main.go b/reload/main.go index 5e6f24d4..c3e72aa3 100644 --- a/reload/main.go +++ b/reload/main.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "fmt" "github.com/gin-gonic/gin" + "github.com/pingcap/monitoring/reload/identity" "github.com/pingcap/monitoring/reload/server/apis" "github.com/pingcap/monitoring/reload/server/auth" "github.com/pingcap/monitoring/reload/server/bizlogic" @@ -37,6 +38,12 @@ func main() { var rootCmd = &cobra.Command{ Use: "reload", Run: func(co *cobra.Command, args []string) { + if subStorePath != "" && (extract(subStorePath) == "" || extract(subStorePath) == "." || extract(subStorePath) == "..") { + log.Fatal("invalid persistent rules version") + } + if err := identity.Prepare(watchDir, rootStorePath, extract(subStorePath)); err != nil { + log.Fatalf("prepare non-root reloader: %v", err) + } startServer() }, FParseErrWhitelist: cobra.FParseErrWhitelist{ From b4afd09ef7540336db2fa2d19058741b899492c2 Mon Sep 17 00:00:00 2001 From: csuzhangxc Date: Thu, 5 Nov 2026 15:10:12 +0800 Subject: [PATCH 2/3] initializer: preserve TiDB rules when adding DM monitoring --- cmd/init.sh | 32 +++-- cmd/persisted_rules_test.go | 144 ++++++++++++++++++++++- monitor-snapshot/master/operator/init.sh | 32 +++-- platform-monitoring/operator/init.sh | 32 +++-- reload/CONTAINER_SECURITY.md | 10 +- 5 files changed, 216 insertions(+), 34 deletions(-) diff --git a/cmd/init.sh b/cmd/init.sh index 585f6f1d..a8e0f915 100644 --- a/cmd/init.sh +++ b/cmd/init.sh @@ -6,6 +6,13 @@ replace_template() { sed -i "s|$1|$replacement|g" -- "$3" } +validate_rule_file() { + if [ -L "$1" ] || [ ! -f "$1" ] || [ -z "$(find "$1" -prune -type f -links 1 -print)" ]; then + echo "Rule must be a regular file with one link: $1" >&2 + return 1 + fi +} + # Run after copying/restoring rules on every Pod initialization. Do not chown /data. prepare_reloader_rules() { reloader_uid=${RELOADER_UID:-1000} @@ -38,10 +45,7 @@ prepare_reloader_rules() { [ -n "$reloader_dir" ] || continue for reloader_file in "$reloader_dir"/*.yml; do [ -e "$reloader_file" ] || [ -L "$reloader_file" ] || continue - if [ -L "$reloader_file" ] || [ ! -f "$reloader_file" ] || [ -z "$(find "$reloader_file" -prune -type f -links 1 -print)" ]; then - echo "Rule must be a regular file: $reloader_file" >&2 - return 1 - fi + validate_rule_file "$reloader_file" || return 1 chown "$reloader_uid:$reloader_gid" "$reloader_file" && chmod u+rw,go-w,a-x "$reloader_file" || return 1 done @@ -50,12 +54,16 @@ prepare_reloader_rules() { # Restore current-version customizations automatically, as before. Validate the # filesystem entries rather than treating all historical user edits as untrusted. -reset_rules() { +ensure_rules_directory() { rules_dir="$PROM_CONFIG_PATH/rules" if [ -L "$rules_dir" ] || { [ -e "$rules_dir" ] && [ ! -d "$rules_dir" ]; }; then echo "Rules path must be a directory" >&2; return 1 fi mkdir -p -- "$rules_dir" || return 1 +} + +reset_rules() { + ensure_rules_directory || return 1 for entry in "$rules_dir"/* "$rules_dir"/.[!.]* "$rules_dir"/..?*; do [ -e "$entry" ] || [ -L "$entry" ] || continue rm -rf -- "$entry" || return 1 @@ -78,8 +86,10 @@ restore_rules() { [ -d "$saved_version" ] || return 0 for rule in "$saved_version"/*.rules.yml; do [ -e "$rule" ] || [ -L "$rule" ] || continue - if [ -L "$rule" ] || [ ! -f "$rule" ] || [ -z "$(find "$rule" -prune -type f -links 1 -print)" ]; then - echo "Persisted rule must be a regular file with one link: $rule" >&2; return 1 + validate_rule_file "$rule" || return 1 + destination="$PROM_CONFIG_PATH/rules/${rule##*/}" + if [ -e "$destination" ] || [ -L "$destination" ]; then + validate_rule_file "$destination" || return 1 fi cp -f -- "$rule" "$PROM_CONFIG_PATH/rules/${rule##*/}" || return 1 done @@ -112,8 +122,12 @@ if [ -n "$DM_CLUSTER_NAME" ]; then cp -f -- /tmp/DM-Monitor-Professional.json "$GF_PROVISIONING_PATH/dashboards" replace_template 'Test-Cluster-DM' "${DM_CLUSTER_NAME}-DM-Professional" "$GF_PROVISIONING_PATH/dashboards"/DM-Monitor-Professional.json - # Rules - reset_rules || exit 1 + # Operator runs DM after TiDB against the same directory. Keep TiDB rules. + ensure_rules_directory || exit 1 + dm_rule="$PROM_CONFIG_PATH/rules/dm_worker.rules.yml" + if [ -e "$dm_rule" ] || [ -L "$dm_rule" ]; then + validate_rule_file "$dm_rule" || exit 1 + fi echo $META_TYPE echo $META_INSTANCE diff --git a/cmd/persisted_rules_test.go b/cmd/persisted_rules_test.go index b4342c19..b23f20bc 100644 --- a/cmd/persisted_rules_test.go +++ b/cmd/persisted_rules_test.go @@ -17,8 +17,11 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { } for _, source := range []string{"init.sh", "../platform-monitoring/operator/init.sh", "../monitor-snapshot/master/operator/init.sh"} { for _, dm := range []bool{false, true} { - for _, scenario := range []string{"default", "custom-name", "symlink", "hardlink", "directory-link", "invalid-version"} { + for _, scenario := range []string{"default", "custom-name", "symlink", "hardlink", "directory-link", "invalid-version", "active-symlink", "active-hardlink", "restore-destination-symlink"} { t.Run(source+"/"+map[bool]string{false: "tidb", true: "dm"}[dm]+"/"+scenario, func(t *testing.T) { + if !dm && (scenario == "active-symlink" || scenario == "active-hardlink" || scenario == "restore-destination-symlink") { + t.Skip("DM shares an existing active directory") + } root := t.TempDir() assets, output, persistent := filepath.Join(root, "assets"), filepath.Join(root, "prometheus"), filepath.Join(root, "persistent") for _, dir := range []string{assets, filepath.Join(output, "rules"), filepath.Join(persistent, "latest-rules", "v1")} { @@ -70,7 +73,26 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { if err := os.WriteFile(saved, []byte(restored), 0644); err != nil { t.Fatal(err) } + outside := filepath.Join(root, "outside.yml") + if err := os.WriteFile(outside, []byte("unchanged target"), 0644); err != nil { + t.Fatal(err) + } switch scenario { + case "active-symlink": + if err := os.Symlink(outside, filepath.Join(output, "rules", name)); err != nil { + t.Fatal(err) + } + case "active-hardlink": + if err := os.Link(outside, filepath.Join(output, "rules", name)); err != nil { + t.Fatal(err) + } + case "restore-destination-symlink": + if err := os.WriteFile(filepath.Join(filepath.Dir(saved), "custom.rules.yml"), []byte(restored), 0644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(output, "rules", "custom.rules.yml")); err != nil { + t.Fatal(err) + } case "custom-name": if err := os.WriteFile(filepath.Join(filepath.Dir(saved), "foreign.rules.yml"), []byte(restored), 0644); err != nil { t.Fatal(err) @@ -113,6 +135,10 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { t.Fatalf("unsafe restore succeeded: %s", out) } if !success { + got, readErr := os.ReadFile(outside) + if readErr != nil || string(got) != "unchanged target" { + t.Fatalf("outside target changed: %q %v", got, readErr) + } return } want := restored @@ -126,8 +152,8 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { t.Fatal("custom rule was not restored", err) } } - if _, err := os.Lstat(old); !os.IsNotExist(err) { - t.Fatalf("foreign active rule survived: %v", err) + if _, err := os.Lstat(old); dm && err != nil || !dm && !os.IsNotExist(err) { + t.Fatalf("unexpected shared rules cleanup: %v", err) } got, err = os.ReadFile(saved) if err != nil || string(got) != restored { @@ -138,3 +164,115 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { } } } + +// Operator runs these initializers in sequence, sharing the rules emptyDir/PVC. +func TestTiDBAndDMInitializeSharedRules(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("initializer requires POSIX shell") + } + for _, source := range []string{"init.sh", "../platform-monitoring/operator/init.sh", "../monitor-snapshot/master/operator/init.sh"} { + for _, persisted := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/persisted=%t", source, persisted), func(t *testing.T) { + root := t.TempDir() + output, data := filepath.Join(root, "prometheus"), filepath.Join(root, "persistent") + store := filepath.Join(data, "latest-rules", "tidb:v1") + if err := os.MkdirAll(store, 0755); err != nil { + t.Fatal(err) + } + script, err := os.ReadFile(source) + if err != nil { + t.Fatal(err) + } + path := os.Getenv("PATH") + if runtime.GOOS == "darwin" { + sed, err := exec.LookPath("gsed") + if err != nil { + t.Skip("GNU sed required on macOS") + } + bin := filepath.Join(root, "bin") + if err := os.Mkdir(bin, 0755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(sed, filepath.Join(bin, "sed")); err != nil { + t.Fatal(err) + } + path = bin + string(os.PathListSeparator) + path + } + expected, runs := map[string]string{}, map[string]string{} + for _, stage := range []string{"tidb", "dm"} { + assets := filepath.Join(root, stage+"-assets") + if err := os.Mkdir(assets, 0755); err != nil { + t.Fatal(err) + } + for _, match := range regexp.MustCompile(`/tmp/([A-Za-z0-9_.-]+)`).FindAllStringSubmatch(string(script), -1) { + name := match[1] + if filepath.Ext(name) != ".json" && filepath.Ext(name) != ".yaml" { + continue + } + if err := os.WriteFile(filepath.Join(assets, name), []byte("{}\n"), 0644); err != nil { + t.Fatal(err) + } + } + name := "tidb.rules.yml" + if stage == "dm" { + name = "dm_worker.rules.yml" + } + bundled := "groups: []\n# " + stage + " bundled\n" + if err := os.WriteFile(filepath.Join(assets, name), []byte(bundled), 0644); err != nil { + t.Fatal(err) + } + expected[name] = bundled + if persisted { + expected[name] = "groups: []\n# " + stage + " saved\n" + if err := os.WriteFile(filepath.Join(store, name), []byte(expected[name]), 0644); err != nil { + t.Fatal(err) + } + } + runs[stage] = filepath.Join(root, stage+".sh") + if err := os.WriteFile(runs[stage], []byte(strings.ReplaceAll(string(script), "/tmp/", assets+"/")), 0644); err != nil { + t.Fatal(err) + } + } + for round := 0; round < 2; round++ { + if round == 1 { + // Model edits saved to the PVC, followed by a new Pod's emptyDir. + for name := range expected { + expected[name] = "groups: []\n# edited " + name + "\n" + for _, dir := range []string{filepath.Join(output, "rules"), store} { + if err := os.WriteFile(filepath.Join(dir, name), []byte(expected[name]), 0644); err != nil { + t.Fatal(err) + } + } + } + if err := os.RemoveAll(output); err != nil { + t.Fatal(err) + } + } + for _, stage := range []string{"tidb", "dm"} { + cluster := "" + if stage == "dm" { + cluster = "test" + } + cmd := exec.Command("sh", runs[stage]) + cmd.Env = append(os.Environ(), "PATH="+path, "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana", stage), "PROM_CONFIG_PATH="+output, "PROM_PERSISTENT_DIR="+data, "TIDB_CLUSTER_NAME=test", "DM_CLUSTER_NAME="+cluster, "TIDB_VERSION=tidb:v1", "DM_VERSION=tidb:v1", "GF_DATASOURCE_PATH=", fmt.Sprintf("RELOADER_UID=%d", os.Geteuid()), fmt.Sprintf("RELOADER_GID=%d", os.Getegid())) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("round %d %s: %v %s", round, stage, err, out) + } + } + for name, want := range expected { + got, err := os.ReadFile(filepath.Join(output, "rules", name)) + if err != nil || string(got) != want { + t.Fatalf("%s = %q, want %q: %v", name, got, want, err) + } + if persisted || round == 1 { + got, err = os.ReadFile(filepath.Join(store, name)) + if err != nil || string(got) != want { + t.Fatalf("saved %s changed: %q %v", name, got, err) + } + } + } + } + }) + } + } +} diff --git a/monitor-snapshot/master/operator/init.sh b/monitor-snapshot/master/operator/init.sh index 8b96af14..a44eed1a 100755 --- a/monitor-snapshot/master/operator/init.sh +++ b/monitor-snapshot/master/operator/init.sh @@ -6,6 +6,13 @@ replace_template() { sed -i "s|$1|$replacement|g" -- "$3" } +validate_rule_file() { + if [ -L "$1" ] || [ ! -f "$1" ] || [ -z "$(find "$1" -prune -type f -links 1 -print)" ]; then + echo "Rule must be a regular file with one link: $1" >&2 + return 1 + fi +} + # Run after copying/restoring rules on every Pod initialization. Do not chown /data. prepare_reloader_rules() { reloader_uid=${RELOADER_UID:-1000} @@ -38,10 +45,7 @@ prepare_reloader_rules() { [ -n "$reloader_dir" ] || continue for reloader_file in "$reloader_dir"/*.yml; do [ -e "$reloader_file" ] || [ -L "$reloader_file" ] || continue - if [ -L "$reloader_file" ] || [ ! -f "$reloader_file" ] || [ -z "$(find "$reloader_file" -prune -type f -links 1 -print)" ]; then - echo "Rule must be a regular file: $reloader_file" >&2 - return 1 - fi + validate_rule_file "$reloader_file" || return 1 chown "$reloader_uid:$reloader_gid" "$reloader_file" && chmod u+rw,go-w,a-x "$reloader_file" || return 1 done @@ -50,12 +54,16 @@ prepare_reloader_rules() { # Restore current-version customizations automatically, as before. Validate the # filesystem entries rather than treating all historical user edits as untrusted. -reset_rules() { +ensure_rules_directory() { rules_dir="$PROM_CONFIG_PATH/rules" if [ -L "$rules_dir" ] || { [ -e "$rules_dir" ] && [ ! -d "$rules_dir" ]; }; then echo "Rules path must be a directory" >&2; return 1 fi mkdir -p -- "$rules_dir" || return 1 +} + +reset_rules() { + ensure_rules_directory || return 1 for entry in "$rules_dir"/* "$rules_dir"/.[!.]* "$rules_dir"/..?*; do [ -e "$entry" ] || [ -L "$entry" ] || continue rm -rf -- "$entry" || return 1 @@ -78,8 +86,10 @@ restore_rules() { [ -d "$saved_version" ] || return 0 for rule in "$saved_version"/*.rules.yml; do [ -e "$rule" ] || [ -L "$rule" ] || continue - if [ -L "$rule" ] || [ ! -f "$rule" ] || [ -z "$(find "$rule" -prune -type f -links 1 -print)" ]; then - echo "Persisted rule must be a regular file with one link: $rule" >&2; return 1 + validate_rule_file "$rule" || return 1 + destination="$PROM_CONFIG_PATH/rules/${rule##*/}" + if [ -e "$destination" ] || [ -L "$destination" ]; then + validate_rule_file "$destination" || return 1 fi cp -f -- "$rule" "$PROM_CONFIG_PATH/rules/${rule##*/}" || return 1 done @@ -112,8 +122,12 @@ if [ -n "$DM_CLUSTER_NAME" ]; then cp -f -- /tmp/DM-Monitor-Professional.json "$GF_PROVISIONING_PATH/dashboards" replace_template 'Test-Cluster-DM' "${DM_CLUSTER_NAME}-DM-Professional" "$GF_PROVISIONING_PATH/dashboards"/DM-Monitor-Professional.json - # Rules - reset_rules || exit 1 + # Operator runs DM after TiDB against the same directory. Keep TiDB rules. + ensure_rules_directory || exit 1 + dm_rule="$PROM_CONFIG_PATH/rules/dm_worker.rules.yml" + if [ -e "$dm_rule" ] || [ -L "$dm_rule" ]; then + validate_rule_file "$dm_rule" || exit 1 + fi echo $META_TYPE echo $META_INSTANCE diff --git a/platform-monitoring/operator/init.sh b/platform-monitoring/operator/init.sh index 8b96af14..a44eed1a 100755 --- a/platform-monitoring/operator/init.sh +++ b/platform-monitoring/operator/init.sh @@ -6,6 +6,13 @@ replace_template() { sed -i "s|$1|$replacement|g" -- "$3" } +validate_rule_file() { + if [ -L "$1" ] || [ ! -f "$1" ] || [ -z "$(find "$1" -prune -type f -links 1 -print)" ]; then + echo "Rule must be a regular file with one link: $1" >&2 + return 1 + fi +} + # Run after copying/restoring rules on every Pod initialization. Do not chown /data. prepare_reloader_rules() { reloader_uid=${RELOADER_UID:-1000} @@ -38,10 +45,7 @@ prepare_reloader_rules() { [ -n "$reloader_dir" ] || continue for reloader_file in "$reloader_dir"/*.yml; do [ -e "$reloader_file" ] || [ -L "$reloader_file" ] || continue - if [ -L "$reloader_file" ] || [ ! -f "$reloader_file" ] || [ -z "$(find "$reloader_file" -prune -type f -links 1 -print)" ]; then - echo "Rule must be a regular file: $reloader_file" >&2 - return 1 - fi + validate_rule_file "$reloader_file" || return 1 chown "$reloader_uid:$reloader_gid" "$reloader_file" && chmod u+rw,go-w,a-x "$reloader_file" || return 1 done @@ -50,12 +54,16 @@ prepare_reloader_rules() { # Restore current-version customizations automatically, as before. Validate the # filesystem entries rather than treating all historical user edits as untrusted. -reset_rules() { +ensure_rules_directory() { rules_dir="$PROM_CONFIG_PATH/rules" if [ -L "$rules_dir" ] || { [ -e "$rules_dir" ] && [ ! -d "$rules_dir" ]; }; then echo "Rules path must be a directory" >&2; return 1 fi mkdir -p -- "$rules_dir" || return 1 +} + +reset_rules() { + ensure_rules_directory || return 1 for entry in "$rules_dir"/* "$rules_dir"/.[!.]* "$rules_dir"/..?*; do [ -e "$entry" ] || [ -L "$entry" ] || continue rm -rf -- "$entry" || return 1 @@ -78,8 +86,10 @@ restore_rules() { [ -d "$saved_version" ] || return 0 for rule in "$saved_version"/*.rules.yml; do [ -e "$rule" ] || [ -L "$rule" ] || continue - if [ -L "$rule" ] || [ ! -f "$rule" ] || [ -z "$(find "$rule" -prune -type f -links 1 -print)" ]; then - echo "Persisted rule must be a regular file with one link: $rule" >&2; return 1 + validate_rule_file "$rule" || return 1 + destination="$PROM_CONFIG_PATH/rules/${rule##*/}" + if [ -e "$destination" ] || [ -L "$destination" ]; then + validate_rule_file "$destination" || return 1 fi cp -f -- "$rule" "$PROM_CONFIG_PATH/rules/${rule##*/}" || return 1 done @@ -112,8 +122,12 @@ if [ -n "$DM_CLUSTER_NAME" ]; then cp -f -- /tmp/DM-Monitor-Professional.json "$GF_PROVISIONING_PATH/dashboards" replace_template 'Test-Cluster-DM' "${DM_CLUSTER_NAME}-DM-Professional" "$GF_PROVISIONING_PATH/dashboards"/DM-Monitor-Professional.json - # Rules - reset_rules || exit 1 + # Operator runs DM after TiDB against the same directory. Keep TiDB rules. + ensure_rules_directory || exit 1 + dm_rule="$PROM_CONFIG_PATH/rules/dm_worker.rules.yml" + if [ -e "$dm_rule" ] || [ -L "$dm_rule" ]; then + validate_rule_file "$dm_rule" || exit 1 + fi echo $META_TYPE echo $META_INSTANCE diff --git a/reload/CONTAINER_SECURITY.md b/reload/CONTAINER_SECURITY.md index 4cf9770c..6121ba10 100644 --- a/reload/CONTAINER_SECURITY.md +++ b/reload/CONTAINER_SECURITY.md @@ -57,10 +57,12 @@ filesystem permissions. Current initializers continue to restore persisted `*.rules.yml` files for the selected version automatically, including names not present in the image's -bundle. Both TiDB and DM first clear only the dedicated active rules directory, -including hidden leftovers, and then install bundled rules and restore saved -ones. Invalid paths, symlink/hard-linked/non-regular saved rules, and copy errors -stop initialization. The PVC copies are not deleted. There is no +bundle. TiDB clears the dedicated active rules directory, including hidden +leftovers, before installing bundled rules and restoring saved ones. DM runs +after TiDB against the shared directory, preserving TiDB rules while installing +DM rules and restoring saved ones. Existing destination rule files are validated +before replacement. Invalid paths, symlink/hard-linked/non-regular saved rules, +and copy errors stop initialization. The PVC copies are not deleted. There is no `PROM_RESTORE_RULES` option and no cross-version directory discovery or migration. The existing version-selection semantics are unchanged: without an explicit From fe37c1959ca1b1ccb6c02e8e4767e83d9fe60c4e Mon Sep 17 00:00:00 2001 From: csuzhangxc Date: Thu, 5 Nov 2026 16:17:44 +0800 Subject: [PATCH 3/3] reloader: preserve Operator volume permissions across upgrades --- cmd/Dockerfile | 2 +- cmd/dm_rules_test.go | 25 ++- cmd/init.sh | 51 ++++-- cmd/persisted_rules_test.go | 41 ++++- monitor-snapshot/master/operator/Dockerfile | 2 +- monitor-snapshot/master/operator/init.sh | 51 ++++-- platform-monitoring/operator/Dockerfile | 2 +- platform-monitoring/operator/init.sh | 51 ++++-- reload/CONTAINER_SECURITY.md | 40 +++-- reload/identity/identity_linux.go | 168 ++++++++++++++++++-- reload/identity/identity_linux_test.go | 136 +++++++++++++++- reload/identity/identity_other.go | 2 + reload/main.go | 5 + reload/server/auth/auth.go | 13 ++ reload/server/auth/auth_test.go | 15 ++ 15 files changed, 532 insertions(+), 72 deletions(-) diff --git a/cmd/Dockerfile b/cmd/Dockerfile index 701c5b1f..903032ee 100644 --- a/cmd/Dockerfile +++ b/cmd/Dockerfile @@ -3,7 +3,7 @@ FROM busybox # Add user and group to support run it with non-root. ARG PINGCAP_UID=1000 ARG PINGCAP_GID=2000 -ENV RELOADER_UID=${PINGCAP_UID} RELOADER_GID=${PINGCAP_GID} +ENV RELOADER_UID=${PINGCAP_UID} RUN addgroup -g ${PINGCAP_GID} pingcap && \ adduser -u ${PINGCAP_UID} -G pingcap -h /home/pingcap -D pingcap diff --git a/cmd/dm_rules_test.go b/cmd/dm_rules_test.go index 1313be2b..cfa02713 100644 --- a/cmd/dm_rules_test.go +++ b/cmd/dm_rules_test.go @@ -19,6 +19,9 @@ import ( type dmTransport func(*http.Request) (*http.Response, error) func TestInitializerPreparesRuleAccess(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("ownership preparation requires a disposable root container") + } if runtime.GOOS == "windows" { t.Skip("initializer requires POSIX permissions") } @@ -32,7 +35,7 @@ func TestInitializerPreparesRuleAccess(t *testing.T) { if !ok || !strings.Contains(prefix, "prepare_reloader_rules()") { t.Fatal("initializer does not prepare reloader access") } - dir := t.TempDir() + dir := initializerTempDir(t) active := filepath.Join(dir, "active rules", "rules") data := filepath.Join(dir, "data") store := filepath.Join(data, "latest-rules", "tidb:test") @@ -43,6 +46,9 @@ func TestInitializerPreparesRuleAccess(t *testing.T) { if err := os.WriteFile(filepath.Join(path, "test.yml"), []byte("groups: []"), 0444); err != nil { t.Fatal(err) } + if err := os.Chmod(filepath.Join(path, "test.yml"), 0444); err != nil { + t.Fatal(err) + } } unrelated := filepath.Join(data, "prometheus") if err := os.Mkdir(unrelated, 0700); err != nil { @@ -115,7 +121,7 @@ func TestDMRulesAreGenerated(t *testing.T) { } oldTransport, oldAnsible, oldOperator := http.DefaultTransport, ansibleRuleDir, operatorRuleDir t.Cleanup(func() { http.DefaultTransport, ansibleRuleDir, operatorRuleDir = oldTransport, oldAnsible, oldOperator }) - ansibleRuleDir, operatorRuleDir = t.TempDir(), t.TempDir() + ansibleRuleDir, operatorRuleDir = initializerTempDir(t), initializerTempDir(t) http.DefaultTransport = dmTransport(func(*http.Request) (*http.Response, error) { return &http.Response{StatusCode: 200, Header: make(http.Header), Body: io.NopCloser(strings.NewReader(string(body)))}, nil }) @@ -139,7 +145,7 @@ func TestDMInitializerRejectsMissingRules(t *testing.T) { } for _, source := range []string{"init.sh", "../platform-monitoring/operator/init.sh", "../monitor-snapshot/master/operator/init.sh"} { t.Run(source, func(t *testing.T) { - root := t.TempDir() + root := initializerTempDir(t) assets := filepath.Join(root, "assets") extraPath := "" if runtime.GOOS == "darwin" { @@ -218,7 +224,7 @@ func TestInitializerRejectsMultilineReplacement(t *testing.T) { } for _, source := range []string{"init.sh", "../platform-monitoring/operator/init.sh", "../monitor-snapshot/master/operator/init.sh"} { t.Run(source, func(t *testing.T) { - root := t.TempDir() + root := initializerTempDir(t) cmd := exec.Command("sh", source) cmd.Env = append(os.Environ(), fmt.Sprintf("RELOADER_UID=%d", os.Geteuid()), fmt.Sprintf("RELOADER_GID=%d", os.Getegid()), "DM_CLUSTER_NAME=test", "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana"), "PROM_CONFIG_PATH="+filepath.Join(root, "prometheus"), "GF_DM_PROMETHEUS_URL=bad\nreplacement") out, err := cmd.CombinedOutput() @@ -239,7 +245,7 @@ func TestTiDBInitializerPreservesDatasourceURLs(t *testing.T) { } for _, source := range []string{"init.sh", "../platform-monitoring/operator/init.sh", "../monitor-snapshot/master/operator/init.sh"} { t.Run(source, func(t *testing.T) { - root := t.TempDir() + root := initializerTempDir(t) assets := filepath.Join(root, "assets") datasource := filepath.Join(root, "data sources") for _, dir := range []string{assets, datasource} { @@ -309,3 +315,12 @@ func TestTiDBInitializerPreservesDatasourceURLs(t *testing.T) { }) } } + +func initializerTempDir(t *testing.T) string { + t.Helper() + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + return dir +} diff --git a/cmd/init.sh b/cmd/init.sh index a8e0f915..71412463 100644 --- a/cmd/init.sh +++ b/cmd/init.sh @@ -6,6 +6,25 @@ replace_template() { sed -i "s|$1|$replacement|g" -- "$3" } +# Reject symlink ancestors before any rule deletion, copy or ownership changes. +validate_rule_path() { + case "$1" in + /*) ;; + *) echo "Rules path must be absolute: $1" >&2; return 1;; + esac + case "/$1/" in + */../*) echo "Rules path must not contain ..: $1" >&2; return 1;; + esac + checked_path=$1 + while [ "$checked_path" != / ]; do + if [ -L "$checked_path" ]; then + echo "Rules path must not contain symlinks: $1" >&2; return 1 + fi + checked_path=${checked_path%/*} + [ -n "$checked_path" ] || checked_path=/ + done +} + validate_rule_file() { if [ -L "$1" ] || [ ! -f "$1" ] || [ -z "$(find "$1" -prune -type f -links 1 -print)" ]; then echo "Rule must be a regular file with one link: $1" >&2 @@ -16,10 +35,10 @@ validate_rule_file() { # Run after copying/restoring rules on every Pod initialization. Do not chown /data. prepare_reloader_rules() { reloader_uid=${RELOADER_UID:-1000} - reloader_gid=${RELOADER_GID:-2000} - for reloader_id in "$reloader_uid" "$reloader_gid"; do + initializer_uid=$(id -u) || return 1 + for reloader_id in "$reloader_uid"; do case "$reloader_id" in - ''|*[!0-9]*) echo "Reloader UID/GID must be numeric" >&2; return 1;; + ''|*[!0-9]*) echo "Reloader UID must be numeric" >&2; return 1;; esac done reloader_version=$1 @@ -37,17 +56,26 @@ prepare_reloader_rules() { echo "Rules path must be a directory, not a symlink: $reloader_dir" >&2 return 1 fi - mkdir -p -- "$reloader_dir" && - chown "$reloader_uid:$reloader_gid" "$reloader_dir" && - chmod u+rwx,go-w "$reloader_dir" || return 1 + mkdir -p -- "$reloader_dir" || return 1 + if [ "$initializer_uid" = 0 ]; then + chown "$reloader_uid" "$reloader_dir" || return 1 + fi + if [ "$initializer_uid" = 0 ]; then + # Preserve the volume group and inherit it on atomic replacements. + chmod u+rwx,g+s,o-w "$reloader_dir" || return 1 + fi done for reloader_dir in "$PROM_CONFIG_PATH/rules" "$reloader_store"; do [ -n "$reloader_dir" ] || continue - for reloader_file in "$reloader_dir"/*.yml; do + for reloader_file in "$reloader_dir"/*.yml "$reloader_dir"/.*.yml "$reloader_dir"/.yml; do [ -e "$reloader_file" ] || [ -L "$reloader_file" ] || continue validate_rule_file "$reloader_file" || return 1 - chown "$reloader_uid:$reloader_gid" "$reloader_file" && + if [ "$initializer_uid" = 0 ]; then + chown "$reloader_uid" "$reloader_file" || return 1 + fi + if [ "$initializer_uid" = 0 ]; then chmod u+rw,go-w,a-x "$reloader_file" || return 1 + fi done done } @@ -84,7 +112,7 @@ restore_rules() { echo "Persisted rules path must be a directory" >&2; return 1 fi [ -d "$saved_version" ] || return 0 - for rule in "$saved_version"/*.rules.yml; do + for rule in "$saved_version"/*.rules.yml "$saved_version"/.rules.yml "$saved_version"/.*.rules.yml; do [ -e "$rule" ] || [ -L "$rule" ] || continue validate_rule_file "$rule" || return 1 destination="$PROM_CONFIG_PATH/rules/${rule##*/}" @@ -108,6 +136,11 @@ if [ -z "$GF_PROVISIONING_PATH" ] || [ -z "$PROM_CONFIG_PATH" ]; then exit 1 fi +validate_rule_path "$PROM_CONFIG_PATH/rules" || exit 1 +if [ -n "$PROM_PERSISTENT_DIR" ]; then + validate_rule_path "$PROM_PERSISTENT_DIR/latest-rules" || exit 1 +fi + if [ -n "$DM_CLUSTER_NAME" ]; then echo "running for DM" if [ ! -d "$GF_PROVISIONING_PATH/dashboards" ]; then diff --git a/cmd/persisted_rules_test.go b/cmd/persisted_rules_test.go index b23f20bc..36814234 100644 --- a/cmd/persisted_rules_test.go +++ b/cmd/persisted_rules_test.go @@ -17,12 +17,12 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { } for _, source := range []string{"init.sh", "../platform-monitoring/operator/init.sh", "../monitor-snapshot/master/operator/init.sh"} { for _, dm := range []bool{false, true} { - for _, scenario := range []string{"default", "custom-name", "symlink", "hardlink", "directory-link", "invalid-version", "active-symlink", "active-hardlink", "restore-destination-symlink"} { + for _, scenario := range []string{"default", "custom-name", "hidden-name", "symlink", "hardlink", "directory-link", "invalid-version", "config-link", "config-parent-link", "active-symlink", "active-hardlink", "restore-destination-symlink"} { t.Run(source+"/"+map[bool]string{false: "tidb", true: "dm"}[dm]+"/"+scenario, func(t *testing.T) { if !dm && (scenario == "active-symlink" || scenario == "active-hardlink" || scenario == "restore-destination-symlink") { t.Skip("DM shares an existing active directory") } - root := t.TempDir() + root := initializerTempDir(t) assets, output, persistent := filepath.Join(root, "assets"), filepath.Join(root, "prometheus"), filepath.Join(root, "persistent") for _, dir := range []string{assets, filepath.Join(output, "rules"), filepath.Join(persistent, "latest-rules", "v1")} { if err := os.MkdirAll(dir, 0755); err != nil { @@ -93,6 +93,26 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { if err := os.Symlink(outside, filepath.Join(output, "rules", "custom.rules.yml")); err != nil { t.Fatal(err) } + case "config-link": + actual := filepath.Join(root, "actual-prometheus") + if err := os.Rename(output, actual); err != nil { + t.Fatal(err) + } + if err := os.Symlink(actual, output); err != nil { + t.Fatal(err) + } + case "config-parent-link": + link := filepath.Join(root, "link") + if err := os.Symlink(root, link); err != nil { + t.Fatal(err) + } + output = filepath.Join(link, "prometheus") + case "hidden-name": + for _, hidden := range []string{".custom.rules.yml", ".rules.yml"} { + if err := os.WriteFile(filepath.Join(filepath.Dir(saved), hidden), []byte(restored), 0644); err != nil { + t.Fatal(err) + } + } case "custom-name": if err := os.WriteFile(filepath.Join(filepath.Dir(saved), "foreign.rules.yml"), []byte(restored), 0644); err != nil { t.Fatal(err) @@ -127,7 +147,7 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { cmd := exec.Command("sh", scriptPath) cmd.Env = append(os.Environ(), "DM_CLUSTER_NAME="+cluster, "TIDB_CLUSTER_NAME=test", "DM_VERSION="+map[bool]string{true: "..", false: "v1"}[scenario == "invalid-version"], "TIDB_VERSION="+map[bool]string{true: "..", false: "v1"}[scenario == "invalid-version"], "GF_PROVISIONING_PATH="+filepath.Join(root, "grafana"), "PROM_CONFIG_PATH="+output, "PROM_PERSISTENT_DIR="+persistent, fmt.Sprintf("RELOADER_UID=%d", os.Geteuid()), fmt.Sprintf("RELOADER_GID=%d", os.Getegid()), "GF_DATASOURCE_PATH=", "PATH="+extraPath+os.Getenv("PATH")) out, err := cmd.CombinedOutput() - success := scenario == "default" || scenario == "custom-name" + success := scenario == "default" || scenario == "custom-name" || scenario == "hidden-name" if success && err != nil { t.Fatalf("initializer failed: %v %s", err, out) } @@ -135,6 +155,11 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { t.Fatalf("unsafe restore succeeded: %s", out) } if !success { + if scenario == "config-link" || scenario == "config-parent-link" { + if _, err := os.Stat(old); err != nil { + t.Fatal("deleted files through a symlink ancestor", err) + } + } got, readErr := os.ReadFile(outside) if readErr != nil || string(got) != "unchanged target" { t.Fatalf("outside target changed: %q %v", got, readErr) @@ -146,6 +171,14 @@ func TestInitializerRestoresRulesWithoutOptIn(t *testing.T) { if err != nil || string(got) != want { t.Fatalf("rules = %q, want %q: %v", got, want, err) } + if scenario == "hidden-name" { + for _, hidden := range []string{".custom.rules.yml", ".rules.yml"} { + got, err := os.ReadFile(filepath.Join(output, "rules", hidden)) + if err != nil || string(got) != restored { + t.Fatalf("hidden rule not restored: %s %v", hidden, err) + } + } + } if scenario == "custom-name" { custom, err := os.ReadFile(filepath.Join(output, "rules", "foreign.rules.yml")) if err != nil || string(custom) != restored { @@ -173,7 +206,7 @@ func TestTiDBAndDMInitializeSharedRules(t *testing.T) { for _, source := range []string{"init.sh", "../platform-monitoring/operator/init.sh", "../monitor-snapshot/master/operator/init.sh"} { for _, persisted := range []bool{false, true} { t.Run(fmt.Sprintf("%s/persisted=%t", source, persisted), func(t *testing.T) { - root := t.TempDir() + root := initializerTempDir(t) output, data := filepath.Join(root, "prometheus"), filepath.Join(root, "persistent") store := filepath.Join(data, "latest-rules", "tidb:v1") if err := os.MkdirAll(store, 0755); err != nil { diff --git a/monitor-snapshot/master/operator/Dockerfile b/monitor-snapshot/master/operator/Dockerfile index 57bb4729..29ebf702 100644 --- a/monitor-snapshot/master/operator/Dockerfile +++ b/monitor-snapshot/master/operator/Dockerfile @@ -3,7 +3,7 @@ FROM busybox:1.37.0 # Add user and group to support run it with non-root. ARG PINGCAP_UID=1000 ARG PINGCAP_GID=2000 -ENV RELOADER_UID=${PINGCAP_UID} RELOADER_GID=${PINGCAP_GID} +ENV RELOADER_UID=${PINGCAP_UID} RUN addgroup -g ${PINGCAP_GID} pingcap && \ adduser -u ${PINGCAP_UID} -G pingcap -h /home/pingcap -D pingcap diff --git a/monitor-snapshot/master/operator/init.sh b/monitor-snapshot/master/operator/init.sh index a44eed1a..a166ca4c 100755 --- a/monitor-snapshot/master/operator/init.sh +++ b/monitor-snapshot/master/operator/init.sh @@ -6,6 +6,25 @@ replace_template() { sed -i "s|$1|$replacement|g" -- "$3" } +# Reject symlink ancestors before any rule deletion, copy or ownership changes. +validate_rule_path() { + case "$1" in + /*) ;; + *) echo "Rules path must be absolute: $1" >&2; return 1;; + esac + case "/$1/" in + */../*) echo "Rules path must not contain ..: $1" >&2; return 1;; + esac + checked_path=$1 + while [ "$checked_path" != / ]; do + if [ -L "$checked_path" ]; then + echo "Rules path must not contain symlinks: $1" >&2; return 1 + fi + checked_path=${checked_path%/*} + [ -n "$checked_path" ] || checked_path=/ + done +} + validate_rule_file() { if [ -L "$1" ] || [ ! -f "$1" ] || [ -z "$(find "$1" -prune -type f -links 1 -print)" ]; then echo "Rule must be a regular file with one link: $1" >&2 @@ -16,10 +35,10 @@ validate_rule_file() { # Run after copying/restoring rules on every Pod initialization. Do not chown /data. prepare_reloader_rules() { reloader_uid=${RELOADER_UID:-1000} - reloader_gid=${RELOADER_GID:-2000} - for reloader_id in "$reloader_uid" "$reloader_gid"; do + initializer_uid=$(id -u) || return 1 + for reloader_id in "$reloader_uid"; do case "$reloader_id" in - ''|*[!0-9]*) echo "Reloader UID/GID must be numeric" >&2; return 1;; + ''|*[!0-9]*) echo "Reloader UID must be numeric" >&2; return 1;; esac done reloader_version=$1 @@ -37,17 +56,26 @@ prepare_reloader_rules() { echo "Rules path must be a directory, not a symlink: $reloader_dir" >&2 return 1 fi - mkdir -p -- "$reloader_dir" && - chown "$reloader_uid:$reloader_gid" "$reloader_dir" && - chmod u+rwx,go-w "$reloader_dir" || return 1 + mkdir -p -- "$reloader_dir" || return 1 + if [ "$initializer_uid" = 0 ]; then + chown "$reloader_uid" "$reloader_dir" || return 1 + fi + if [ "$initializer_uid" = 0 ]; then + # Preserve the volume group and inherit it on atomic replacements. + chmod u+rwx,g+s,o-w "$reloader_dir" || return 1 + fi done for reloader_dir in "$PROM_CONFIG_PATH/rules" "$reloader_store"; do [ -n "$reloader_dir" ] || continue - for reloader_file in "$reloader_dir"/*.yml; do + for reloader_file in "$reloader_dir"/*.yml "$reloader_dir"/.*.yml "$reloader_dir"/.yml; do [ -e "$reloader_file" ] || [ -L "$reloader_file" ] || continue validate_rule_file "$reloader_file" || return 1 - chown "$reloader_uid:$reloader_gid" "$reloader_file" && + if [ "$initializer_uid" = 0 ]; then + chown "$reloader_uid" "$reloader_file" || return 1 + fi + if [ "$initializer_uid" = 0 ]; then chmod u+rw,go-w,a-x "$reloader_file" || return 1 + fi done done } @@ -84,7 +112,7 @@ restore_rules() { echo "Persisted rules path must be a directory" >&2; return 1 fi [ -d "$saved_version" ] || return 0 - for rule in "$saved_version"/*.rules.yml; do + for rule in "$saved_version"/*.rules.yml "$saved_version"/.rules.yml "$saved_version"/.*.rules.yml; do [ -e "$rule" ] || [ -L "$rule" ] || continue validate_rule_file "$rule" || return 1 destination="$PROM_CONFIG_PATH/rules/${rule##*/}" @@ -108,6 +136,11 @@ if [ -z "$GF_PROVISIONING_PATH" ] || [ -z "$PROM_CONFIG_PATH" ]; then exit 1 fi +validate_rule_path "$PROM_CONFIG_PATH/rules" || exit 1 +if [ -n "$PROM_PERSISTENT_DIR" ]; then + validate_rule_path "$PROM_PERSISTENT_DIR/latest-rules" || exit 1 +fi + if [ -n "$DM_CLUSTER_NAME" ]; then echo "running for DM" if [ ! -d "$GF_PROVISIONING_PATH/dashboards" ]; then diff --git a/platform-monitoring/operator/Dockerfile b/platform-monitoring/operator/Dockerfile index 57bb4729..29ebf702 100644 --- a/platform-monitoring/operator/Dockerfile +++ b/platform-monitoring/operator/Dockerfile @@ -3,7 +3,7 @@ FROM busybox:1.37.0 # Add user and group to support run it with non-root. ARG PINGCAP_UID=1000 ARG PINGCAP_GID=2000 -ENV RELOADER_UID=${PINGCAP_UID} RELOADER_GID=${PINGCAP_GID} +ENV RELOADER_UID=${PINGCAP_UID} RUN addgroup -g ${PINGCAP_GID} pingcap && \ adduser -u ${PINGCAP_UID} -G pingcap -h /home/pingcap -D pingcap diff --git a/platform-monitoring/operator/init.sh b/platform-monitoring/operator/init.sh index a44eed1a..a166ca4c 100755 --- a/platform-monitoring/operator/init.sh +++ b/platform-monitoring/operator/init.sh @@ -6,6 +6,25 @@ replace_template() { sed -i "s|$1|$replacement|g" -- "$3" } +# Reject symlink ancestors before any rule deletion, copy or ownership changes. +validate_rule_path() { + case "$1" in + /*) ;; + *) echo "Rules path must be absolute: $1" >&2; return 1;; + esac + case "/$1/" in + */../*) echo "Rules path must not contain ..: $1" >&2; return 1;; + esac + checked_path=$1 + while [ "$checked_path" != / ]; do + if [ -L "$checked_path" ]; then + echo "Rules path must not contain symlinks: $1" >&2; return 1 + fi + checked_path=${checked_path%/*} + [ -n "$checked_path" ] || checked_path=/ + done +} + validate_rule_file() { if [ -L "$1" ] || [ ! -f "$1" ] || [ -z "$(find "$1" -prune -type f -links 1 -print)" ]; then echo "Rule must be a regular file with one link: $1" >&2 @@ -16,10 +35,10 @@ validate_rule_file() { # Run after copying/restoring rules on every Pod initialization. Do not chown /data. prepare_reloader_rules() { reloader_uid=${RELOADER_UID:-1000} - reloader_gid=${RELOADER_GID:-2000} - for reloader_id in "$reloader_uid" "$reloader_gid"; do + initializer_uid=$(id -u) || return 1 + for reloader_id in "$reloader_uid"; do case "$reloader_id" in - ''|*[!0-9]*) echo "Reloader UID/GID must be numeric" >&2; return 1;; + ''|*[!0-9]*) echo "Reloader UID must be numeric" >&2; return 1;; esac done reloader_version=$1 @@ -37,17 +56,26 @@ prepare_reloader_rules() { echo "Rules path must be a directory, not a symlink: $reloader_dir" >&2 return 1 fi - mkdir -p -- "$reloader_dir" && - chown "$reloader_uid:$reloader_gid" "$reloader_dir" && - chmod u+rwx,go-w "$reloader_dir" || return 1 + mkdir -p -- "$reloader_dir" || return 1 + if [ "$initializer_uid" = 0 ]; then + chown "$reloader_uid" "$reloader_dir" || return 1 + fi + if [ "$initializer_uid" = 0 ]; then + # Preserve the volume group and inherit it on atomic replacements. + chmod u+rwx,g+s,o-w "$reloader_dir" || return 1 + fi done for reloader_dir in "$PROM_CONFIG_PATH/rules" "$reloader_store"; do [ -n "$reloader_dir" ] || continue - for reloader_file in "$reloader_dir"/*.yml; do + for reloader_file in "$reloader_dir"/*.yml "$reloader_dir"/.*.yml "$reloader_dir"/.yml; do [ -e "$reloader_file" ] || [ -L "$reloader_file" ] || continue validate_rule_file "$reloader_file" || return 1 - chown "$reloader_uid:$reloader_gid" "$reloader_file" && + if [ "$initializer_uid" = 0 ]; then + chown "$reloader_uid" "$reloader_file" || return 1 + fi + if [ "$initializer_uid" = 0 ]; then chmod u+rw,go-w,a-x "$reloader_file" || return 1 + fi done done } @@ -84,7 +112,7 @@ restore_rules() { echo "Persisted rules path must be a directory" >&2; return 1 fi [ -d "$saved_version" ] || return 0 - for rule in "$saved_version"/*.rules.yml; do + for rule in "$saved_version"/*.rules.yml "$saved_version"/.rules.yml "$saved_version"/.*.rules.yml; do [ -e "$rule" ] || [ -L "$rule" ] || continue validate_rule_file "$rule" || return 1 destination="$PROM_CONFIG_PATH/rules/${rule##*/}" @@ -108,6 +136,11 @@ if [ -z "$GF_PROVISIONING_PATH" ] || [ -z "$PROM_CONFIG_PATH" ]; then exit 1 fi +validate_rule_path "$PROM_CONFIG_PATH/rules" || exit 1 +if [ -n "$PROM_PERSISTENT_DIR" ]; then + validate_rule_path "$PROM_PERSISTENT_DIR/latest-rules" || exit 1 +fi + if [ -n "$DM_CLUSTER_NAME" ]; then echo "running for DM" if [ ! -d "$GF_PROVISIONING_PATH/dashboards" ]; then diff --git a/reload/CONTAINER_SECURITY.md b/reload/CONTAINER_SECURITY.md index 6121ba10..81baa140 100644 --- a/reload/CONTAINER_SECURITY.md +++ b/reload/CONTAINER_SECURITY.md @@ -4,7 +4,8 @@ Updating the reloader does not require a new initializer, new CR fields, a restore opt-in, or a change to `alertManagerRulesVersion`. Keep the deployed initializer and its version. Rules continue to be restored from the existing current-version directory on the PVC. Authentication/TLS settings follow -[UPGRADING.md](UPGRADING.md); this change adds no web configuration requirements. +[UPGRADING.md](UPGRADING.md). Mounted credentials must be readable by the serving +identity (default UID 1000/GID 2000); root-only custom modes prevent startup. ## Existing initializer images @@ -13,11 +14,14 @@ filesystem preparation only. Before loading web credentials, starting watchers or listening, it prepares the active rules directory and the current `latest-rules/` directory for the image's existing `pingcap` account (default UID 1000/GID 2000). It does not recursively change `/data`, Prometheus, -Grafana or other rule versions. Directories and regular `.yml` rule files lose -unnecessary write/execute permissions; existing read permissions are retained. +Grafana or other rule versions. Existing file/directory groups and reader access +are retained; setgid on rule directories makes atomic replacements inherit the +original group. Only the serving UID is assigned ownership. Files lose unnecessary +write/execute permissions. Symlink paths and symlink, hard-linked or non-regular rule files are rejected. -The process clears supplementary groups, switches GID/UID, clears capabilities, +The process retains existing Pod supplementary groups (including a configured +fsGroup 0), switches GID/UID, clears capabilities, sets `no_new_privs`, and replaces itself before starting the service. There is no root supervisor. A preparation or privilege-drop failure stops startup rather than serving as root. The image intentionally retains root as its startup user: @@ -35,16 +39,28 @@ Current TiDB and DM initializer sources prepare the same rule permissions after generating and restoring rules on every startup. Once deployed, they also allow the reloader to start directly as UID 1000/GID 2000, including after Pod rebuilds. An explicit non-root runtime user bypasses reloader ownership preparation; -volume permissions must already permit editing. No identity override is needed +volume permissions must already permit editing. Accessibility is checked under +the serving identity before listening, so an inaccessible PVC root fails startup +rather than allowing an active save followed by a persistence failure. When +an admin account enables editing, atomic saves also require a file group to +match the directory's inherited group or a Kubernetes-granted serving group. Inconsistent custom groups fail startup with +an actionable message; the process does not grant itself extra group access. +Watch-only and viewer-only services need only their existing read/traverse access; +read-only mounts do not trigger metadata writes. No identity override is needed for the compatible default startup path. Custom builds retain the existing `PINGCAP_UID`/`PINGCAP_GID` build arguments. The reloader looks up the image's `pingcap` account; initializer permission -preparation uses matching `RELOADER_UID`/`RELOADER_GID` defaults or existing env -configuration (`spec.initializer.envs`, and `spec.dm.initializer.envs` for DM). -Explicit custom runtime identities must match the prepared ownership. Mounted +preparation uses `RELOADER_UID` (default 1000) or existing env configuration +(`spec.initializer.envs`, and `spec.dm.initializer.envs` for DM). File groups are +inherited from the volumes, not reassigned to the image GID. Explicit runAsGroup +is honored. Non-root initializers retain their identity and do not attempt chown; +they leave existing ownership/modes untouched, including setgid directories +created by a previous bootstrap. Unprivileged chmod can otherwise clear setgid +when its inherited group is not one of the initializer's groups. Matching +explicit non-root initializer/reloader identities therefore need no new env overrides. Mounted web credentials must be readable by the serving identity, as with any non-root -service; root-only custom Secret modes may require an access adjustment. +service; root-only custom Secret modes must be adjusted or startup fails. The compatibility startup needs permission to chown/chmod rule files and change UID/GID. Storage that prohibits ownership changes (for example root-squashed @@ -56,9 +72,9 @@ filesystem permissions. ## Rule restore behavior Current initializers continue to restore persisted `*.rules.yml` files for the -selected version automatically, including names not present in the image's -bundle. TiDB clears the dedicated active rules directory, including hidden -leftovers, before installing bundled rules and restoring saved ones. DM runs +selected version automatically, including hidden names and names not present in +the image's bundle. TiDB clears the dedicated active rules directory, including +hidden leftovers, before installing bundled rules and restoring saved ones. DM runs after TiDB against the shared directory, preserving TiDB rules while installing DM rules and restoring saved ones. Existing destination rule files are validated before replacement. Invalid paths, symlink/hard-linked/non-regular saved rules, diff --git a/reload/identity/identity_linux.go b/reload/identity/identity_linux.go index 3d8c15f0..56d495e6 100644 --- a/reload/identity/identity_linux.go +++ b/reload/identity/identity_linux.go @@ -20,7 +20,7 @@ import ( // explicit non-root runtime identity is already authoritative and is left alone. func Prepare(watchDir, storeRoot, version string) error { if os.Geteuid() != 0 { - return nil + return checkPaths(watchDir, storeRoot, version, false) } account, err := user.Lookup("pingcap") if err != nil { @@ -34,17 +34,21 @@ func Prepare(watchDir, storeRoot, version string) error { if err != nil || gid <= 0 { return fmt.Errorf("invalid non-root GID %q", account.Gid) } - if err := preparePaths(watchDir, storeRoot, version, uid, gid); err != nil { + // Kubernetes may explicitly set runAsGroup independently of runAsUser. + if os.Getgid() != 0 { + gid = os.Getgid() + } + if err := preparePaths(watchDir, storeRoot, version, uid); err != nil { return err } return dropAndExec(uid, gid) } -func preparePaths(watchDir, storeRoot, version string, uid, gid int) error { +func preparePaths(watchDir, storeRoot, version string, uid int) error { if version != "" && (version != filepath.Base(version) || version == "." || version == "..") { return fmt.Errorf("invalid rules version %q", version) } - if err := prepareDirectory(watchDir, false, true, uid, gid); err != nil { + if err := prepareDirectory(watchDir, false, true, uid); err != nil { return err } if version == "" { @@ -57,10 +61,10 @@ func preparePaths(watchDir, storeRoot, version string, uid, gid int) error { } root.Close() parent := filepath.Join(storeRoot, "latest-rules") - if err := prepareDirectory(parent, true, false, uid, gid); err != nil { + if err := prepareDirectory(parent, true, false, uid); err != nil { return err } - return prepareDirectory(filepath.Join(parent, version), true, true, uid, gid) + return prepareDirectory(filepath.Join(parent, version), true, true, uid) } // Open each component with NOFOLLOW, retaining directory descriptors so renames @@ -69,6 +73,11 @@ func openDirectory(path string, create bool) (*os.File, error) { if !filepath.IsAbs(path) || filepath.Clean(path) == "/" { return nil, fmt.Errorf("rules path must be an absolute non-root directory: %q", path) } + for _, part := range strings.Split(path, "/") { + if part == ".." { + return nil, fmt.Errorf("rules path must not contain ..: %q", path) + } + } fd, err := unix.Open("/", unix.O_RDONLY|unix.O_DIRECTORY|unix.O_CLOEXEC, 0) if err != nil { return nil, err @@ -92,7 +101,7 @@ func openDirectory(path string, create bool) (*os.File, error) { return os.NewFile(uintptr(fd), path), nil } -func setOwnerMode(file *os.File, uid, gid int, directory bool) error { +func setOwnerMode(file *os.File, uid int, directory bool) error { var st unix.Stat_t if err := unix.Fstat(int(file.Fd()), &st); err != nil { return err @@ -100,25 +109,45 @@ func setOwnerMode(file *os.File, uid, gid int, directory bool) error { if !directory && (st.Mode&unix.S_IFMT != unix.S_IFREG || st.Nlink != 1) { return fmt.Errorf("rule must be a regular file with one link: %s", file.Name()) } - if int(st.Uid) != uid || int(st.Gid) != gid { - if err := file.Chown(uid, gid); err != nil { + ownershipChanged := int(st.Uid) != uid + if ownershipChanged { + if err := file.Chown(uid, -1); err != nil { return err } } mode := os.FileMode(st.Mode&0644) | 0600 if directory { - mode = os.FileMode(st.Mode&0055) | 0700 + // Retain existing reader/writer groups and let atomic replacements inherit + // that group, without granting the service additional group memberships. + mode = os.FileMode(st.Mode&0075) | 0700 | os.ModeSetgid + if st.Mode&unix.S_ISVTX != 0 { + mode |= os.ModeSticky + } + } + info, err := file.Stat() + if err != nil { + return err + } + if info.Mode()&(os.ModePerm|os.ModeSetgid|os.ModeSetuid|os.ModeSticky) == mode { + return nil } return file.Chmod(mode) } -func prepareDirectory(path string, create, rules bool, uid, gid int) error { +func prepareDirectory(path string, create, rules bool, uid int) error { dir, err := openDirectory(path, create) if err != nil { return err } defer dir.Close() - if err := setOwnerMode(dir, uid, gid, true); err != nil { + readOnly, err := readOnlyDirectory(dir) + if err != nil { + return err + } + if readOnly { + return nil + } + if err := setOwnerMode(dir, uid, true); err != nil { return fmt.Errorf("prepare %s: %w", path, err) } if !rules { @@ -137,7 +166,7 @@ func prepareDirectory(path string, create, rules bool, uid, gid int) error { return fmt.Errorf("open rule %s: %w", name, err) } file := os.NewFile(uintptr(fd), filepath.Join(path, name)) - err = setOwnerMode(file, uid, gid, false) + err = setOwnerMode(file, uid, false) file.Close() if err != nil { return err @@ -156,7 +185,13 @@ func dropAndExec(uid, gid int) error { if err := unix.Prctl(unix.PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); err != nil { return err } - if err := syscall.Setgroups([]int{}); err != nil { + // Keep groups already granted by the Pod, including a valid fsGroup 0. + // Group 0 does not grant UID 0 or capabilities; never infer new groups from files. + groups, err := os.Getgroups() + if err != nil { + return err + } + if err := syscall.Setgroups(groups); err != nil { return err } if err := syscall.Setgid(gid); err != nil { @@ -174,3 +209,108 @@ func dropAndExec(uid, gid int) error { // arguments, or inherited rule descriptors remain in the serving process. return syscall.Exec("/proc/self/exe", os.Args, os.Environ()) } + +// Rechecked after exec under the actual serving identity, before listening. +// This catches inaccessible PVC roots instead of failing after an active save. +// CheckWritable is called only when an administrator can edit rules. Watch-only +// and viewer-only services do not need ownership suitable for atomic saves. +func CheckWritable(watchDir, storeRoot, version string) error { + return checkPaths(watchDir, storeRoot, version, true) +} + +func checkPaths(watchDir, storeRoot, version string, writable bool) error { + paths := []string{watchDir} + if version != "" { + paths = append(paths, filepath.Join(storeRoot, "latest-rules"), filepath.Join(storeRoot, "latest-rules", version)) + } + for i, path := range paths { + dir, err := openDirectory(path, i > 0) + if err != nil { + return fmt.Errorf("access rules as serving user: %w", err) + } + access := uint32(unix.R_OK | unix.X_OK) + readOnly, err := readOnlyDirectory(dir) + if err != nil { + dir.Close() + return err + } + if writable && i != 1 && !readOnly { + access |= unix.W_OK + } + if err := unix.Faccessat(int(dir.Fd()), ".", access, unix.AT_EACCESS); err != nil { + dir.Close() + return fmt.Errorf("serving identity cannot access rules directory %s: %w", path, err) + } + if writable && i != 1 { + if err := checkReplacementGroups(dir); err != nil { + dir.Close() + return err + } + } + dir.Close() + } + return nil +} + +// Atomic saves preserve file groups. A non-root service can restore a different +// group only when Kubernetes has explicitly granted that group to the process. +func checkReplacementGroups(dir *os.File) error { + readOnly, err := readOnlyDirectory(dir) + if err != nil { + return err + } + if readOnly { + return nil + } // Watch-only read-only mounts need no metadata writes. + info, err := dir.Stat() + if err != nil { + return err + } + inherited := os.Getegid() + if info.Mode()&os.ModeSetgid != 0 { + inherited = int(info.Sys().(*syscall.Stat_t).Gid) + } + allowed, err := os.Getgroups() + if err != nil { + return err + } + allowed = append(allowed, os.Getegid(), inherited) + names, err := dir.Readdirnames(-1) + if err != nil { + return err + } + for _, name := range names { + if filepath.Ext(name) != ".yml" { + continue + } + var st unix.Stat_t + if err = unix.Fstatat(int(dir.Fd()), name, &st, unix.AT_SYMLINK_NOFOLLOW); err != nil { + return err + } + if st.Mode&unix.S_IFMT != unix.S_IFREG { + continue + } + if st.Uid != uint32(os.Geteuid()) { + return fmt.Errorf("rule %s owner %d cannot be preserved by serving UID %d; prepare ownership for the configured runtime identity", filepath.Join(dir.Name(), name), st.Uid, os.Geteuid()) + } + ok := false + for _, group := range allowed { + if uint32(group) == st.Gid { + ok = true + break + } + } + if !ok { + return fmt.Errorf("rule %s group %d cannot be preserved by serving identity; align the rule directory group or grant it with pod supplementalGroups", filepath.Join(dir.Name(), name), st.Gid) + } + } + return nil +} + +func readOnlyDirectory(dir *os.File) (bool, error) { + var stat unix.Statfs_t + if err := unix.Fstatfs(int(dir.Fd()), &stat); err != nil { + return false, err + } + return stat.Flags&unix.ST_RDONLY != 0, nil +} diff --git a/reload/identity/identity_linux_test.go b/reload/identity/identity_linux_test.go index f6ba8272..2c6bbd29 100644 --- a/reload/identity/identity_linux_test.go +++ b/reload/identity/identity_linux_test.go @@ -3,6 +3,7 @@ package identity import ( + "fmt" "os" "os/exec" "path/filepath" @@ -12,7 +13,7 @@ import ( ) func TestPreparePaths(t *testing.T) { - for _, attack := range []string{"none", "symlink-file", "hardlink-file", "symlink-dir", "symlink-parent", "fifo", "version"} { + for _, attack := range []string{"none", "symlink-file", "hardlink-file", "symlink-dir", "symlink-parent", "fifo", "version", "dotdot"} { t.Run(attack, func(t *testing.T) { root := t.TempDir() active, data := filepath.Join(root, "active"), filepath.Join(root, "data") @@ -41,12 +42,19 @@ func TestPreparePaths(t *testing.T) { if err := syscall.Mkfifo(rule, 0600); err != nil { t.Fatal(err) } + case "dotdot": + active = filepath.Join(root, "active") + "/../active" case "version": version = "../other" default: - os.WriteFile(rule, []byte("groups: []"), 0444) + if err := os.WriteFile(rule, []byte("groups: []"), 0444); err != nil { + t.Fatal(err) + } + if err := os.Chmod(rule, 0444); err != nil { + t.Fatal(err) + } } - err := preparePaths(active, data, version, os.Geteuid(), os.Getegid()) + err := preparePaths(active, data, version, os.Geteuid()) if attack != "none" { if err == nil { t.Fatal("unsafe path accepted") @@ -75,6 +83,13 @@ func TestDropAndExec(t *testing.T) { // This helper runs only in a disposable Linux test container as root. if os.Getenv("RELOADER_IDENTITY_TEST_CHILD") == "1" { if os.Geteuid() == 0 { + groups, err := os.Getgroups() + if err != nil { + t.Fatal(err) + } + if err := os.Setenv("RELOADER_IDENTITY_EXPECTED_GROUPS", fmt.Sprint(groups)); err != nil { + t.Fatal(err) + } if err := dropAndExec(1000, 2000); err != nil { t.Fatal(err) } @@ -83,12 +98,12 @@ func TestDropAndExec(t *testing.T) { if os.Getuid() != 1000 || os.Geteuid() != 1000 || os.Getgid() != 2000 || os.Getegid() != 2000 { t.Fatal("wrong runtime identity") } - if err := Prepare("/does-not-exist", "/does-not-exist", "invalid"); err != nil { - t.Fatal("non-root runtime attempted ownership preparation", err) + if err := Prepare("/does-not-exist", "/does-not-exist", "invalid"); err == nil { + t.Fatal("non-root runtime did not check inaccessible paths") } groups, err := os.Getgroups() - if err != nil || len(groups) != 0 { - t.Fatal("supplementary groups retained", groups, err) + if err != nil || fmt.Sprint(groups) != os.Getenv("RELOADER_IDENTITY_EXPECTED_GROUPS") { + t.Fatal("Kubernetes volume groups changed", groups, err) } status, err := os.ReadFile("/proc/self/status") if err != nil { @@ -132,3 +147,110 @@ func TestDropAndExec(t *testing.T) { t.Fatalf("%v: %s", err, out) } } + +func TestPreserveVolumeGroup(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("requires disposable root container") + } + root := t.TempDir() + active, data := filepath.Join(root, "active"), filepath.Join(root, "data") + for _, p := range []string{active, data} { + if err := os.Mkdir(p, 0750); err != nil { + t.Fatal(err) + } + if err := os.Chown(p, 0, 1234); err != nil { + t.Fatal(err) + } + if err := os.Chmod(p, 0750|os.ModeSticky); err != nil { + t.Fatal(err) + } + } + rule := filepath.Join(active, "custom.rules.yml") + if err := os.WriteFile(rule, []byte("groups: []"), 0640); err != nil { + t.Fatal(err) + } + if err := os.Chown(rule, 0, 1234); err != nil { + t.Fatal(err) + } + if err := os.Chmod(rule, 0640); err != nil { + t.Fatal(err) + } + if err := preparePaths(active, data, "v1", 1000); err != nil { + t.Fatal(err) + } + info, err := os.Stat(rule) + if err != nil { + t.Fatal(err) + } + if info.Sys().(*syscall.Stat_t).Gid != 1234 || info.Mode().Perm() != 0640 { + t.Fatal("reader's group access changed", info.Sys(), info.Mode()) + } + info, err = os.Stat(active) + if err != nil { + t.Fatal(err) + } + if info.Sys().(*syscall.Stat_t).Gid != 1234 || info.Mode()&os.ModeSetgid == 0 || info.Mode()&os.ModeSticky == 0 { + t.Fatal("atomic replacements will not inherit existing group") + } +} + +func TestRejectUnpreservableRuleGroup(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("requires disposable root container") + } + dir := t.TempDir() + name := filepath.Join(dir, "custom.rules.yml") + if err := os.WriteFile(name, []byte("groups: []"), 0640); err != nil { + t.Fatal(err) + } + if err := os.Chown(name, os.Geteuid(), 60001); err != nil { + t.Fatal(err) + } + opened, err := openDirectory(dir, false) + if err != nil { + t.Fatal(err) + } + defer opened.Close() + if err := checkReplacementGroups(opened); err == nil { + t.Fatal("unsupported group would fail only after saving active rules") + } +} + +func TestRejectUnpreservableRuleOwner(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("requires disposable root container") + } + dir := t.TempDir() + name := filepath.Join(dir, "custom.rules.yml") + if err := os.WriteFile(name, []byte("groups: []"), 0644); err != nil { + t.Fatal(err) + } + if err := os.Chown(name, 1000, os.Getegid()); err != nil { + t.Fatal(err) + } + opened, err := openDirectory(dir, false) + if err != nil { + t.Fatal(err) + } + defer opened.Close() + if err := checkReplacementGroups(opened); err == nil { + t.Fatal("unsupported owner would fail only after saving active rules") + } +} + +func TestWatchOnlyDoesNotRequireRuleOwnership(t *testing.T) { + if os.Geteuid() != 0 { + t.Skip("requires disposable root container") + } + dir := t.TempDir() + name := filepath.Join(dir, "custom.rules.yml") + if err := os.WriteFile(name, []byte("groups: []"), 0644); err != nil { + t.Fatal(err) + } + if err := os.Chown(name, 60001, 60001); err != nil { + t.Fatal(err) + } + if err := checkPaths(dir, "", "", false); err != nil { + t.Fatal("watch-only requires read/traverse, not replacement rights", err) + } +} diff --git a/reload/identity/identity_other.go b/reload/identity/identity_other.go index 45cd0fbc..a536ac47 100644 --- a/reload/identity/identity_other.go +++ b/reload/identity/identity_other.go @@ -5,3 +5,5 @@ package identity // Container privilege preparation is Linux-specific; native development keeps // the caller's identity and filesystem permissions. func Prepare(watchDir, storeRoot, version string) error { return nil } + +func CheckWritable(watchDir, storeRoot, version string) error { return nil } diff --git a/reload/main.go b/reload/main.go index c3e72aa3..ed6f881e 100644 --- a/reload/main.go +++ b/reload/main.go @@ -75,6 +75,11 @@ func startServer() { if err := validateWebOptions(listenAddress, tlsCertFile, tlsKeyFile, policy != nil, allowInsecure); err != nil { log.Fatal(err) } + if policy.HasAdmins() { + if err := identity.CheckWritable(watchDir, rootStorePath, extract(subStorePath)); err != nil { + log.Fatal(err) + } + } if policy == nil { log.Print("administrative API disabled: configure --web.auth-file to enable access") } diff --git a/reload/server/auth/auth.go b/reload/server/auth/auth.go index 4d892bd2..b3ede989 100644 --- a/reload/server/auth/auth.go +++ b/reload/server/auth/auth.go @@ -112,3 +112,16 @@ func Middleware(p *Policy) gin.HandlerFunc { c.Next() } } + +// HasAdmins reports whether this policy enables rule editing. +func (p *Policy) HasAdmins() bool { + if p == nil { + return false + } + for _, user := range p.users { + if user.Role == "admin" { + return true + } + } + return false +} diff --git a/reload/server/auth/auth_test.go b/reload/server/auth/auth_test.go index e75daf8a..05fd6cf1 100644 --- a/reload/server/auth/auth_test.go +++ b/reload/server/auth/auth_test.go @@ -20,3 +20,18 @@ func TestInvalidAuthFilesFailClosed(t *testing.T) { }) } } + +func TestHasAdmins(t *testing.T) { + var disabled *Policy + if disabled.HasAdmins() { + t.Fatal("disabled policy enables writes") + } + viewer := &Policy{users: map[string]User{"reader": {Role: "viewer"}}} + if viewer.HasAdmins() { + t.Fatal("viewer-only policy enables writes") + } + viewer.users["editor"] = User{Role: "admin"} + if !viewer.HasAdmins() { + t.Fatal("admin policy disables writes") + } +}