config: add path validation

add validation and restrictions about configuration paths,
to reduce the chance of path traversal vulnerabilities.

Most notably:
- config root path must be one of the allowed subpaths
- configlet are expected not to escape the configuratio
  directories

Signed-off-by: Francesco Romani <[email protected]>

pkg/config/cfgfile.go

@@ -44,11 +44,19 @@ func FixExtraConfigPath(configRoot string) string {
 }
 
 func FromFiles(pArgs *ProgArgs, configRoot, extraConfigPath string) error {
-	err := fromDaemonFiles(pArgs, configRoot)
+	cfgRoot, err := validateConfigRootPath(configRoot)
+	if err != nil {
+		return err
+	}
+	extraCfgPath, err := validateConfigRootPath(extraConfigPath)
+	if err != nil {
+		return err
+	}
+	err = fromDaemonFiles(pArgs, cfgRoot)
 	if err != nil && !errors.Is(err, os.ErrNotExist) {
 		return err
 	}
-	return fromExtraFile(pArgs, extraConfigPath)
+	return fromExtraFile(pArgs, extraCfgPath)
 }
 
 func fromExtraFile(pArgs *ProgArgs, extraConfigPath string) error {
@@ -94,17 +102,21 @@ func fromDaemonFiles(pArgs *ProgArgs, configPathRoot string) error {
 
 	// this directory may be missing, that's expected and fine
 	configletDir := filepath.Join(configPathRoot, "daemon", "config.yaml.d")
-	if configLets, err := os.ReadDir(configletDir); err == nil {
-		for _, configLet := range configLets {
-			if !configLet.Type().IsRegular() {
-				klog.Infof("configlet %q not regular file: ignored", configLet.Name())
+	if configlets, err := os.ReadDir(configletDir); err == nil {
+		for _, configlet := range configlets {
+			if !configlet.Type().IsRegular() {
+				klog.Infof("configlet %q not regular file: ignored", configlet.Name())
+				continue
+			}
+			configletPath, err := validateConfigLetPath(configletDir, configlet.Name())
+			if err != nil {
+				klog.Infof("could not load: %v", err)
 				continue
 			}
-			configLetPath := filepath.Join(configletDir, configLet.Name())
 			if pArgs.Global.Debug {
-				klog.Infof("loading configlet: %q", configLetPath)
+				klog.Infof("loading configlet: %q", configletPath)
 			}
-			err = loadConfiglet(confObj, configLetPath)
+			err = loadConfiglet(confObj, configletPath)
 			if err != nil {
 				return err
 			}

pkg/config/cfgfile_test.go

@@ -27,10 +27,10 @@ import (
 )
 
 func TestReadResourceExclude(t *testing.T) {
-	closer := setupTest(t)
+	testDir, closer := setupTest(t)
 	t.Cleanup(closer)
 
-	cfg, err := os.CreateTemp("", "exclude-list")
+	cfg, err := os.CreateTemp(testDir, "exclude-list")
 	if err != nil {
 		t.Fatalf("unexpected error creating temp file: %v", err)
 	}
@@ -69,6 +69,9 @@ func TestReadResourceExclude(t *testing.T) {
 }
 
 func TestFromFiles(t *testing.T) {
+	testDir, closer := setupTest(t)
+	t.Cleanup(closer)
+
 	type testCase struct {
 		name string
 	}
@@ -79,7 +82,7 @@ func TestFromFiles(t *testing.T) {
 		},
 	} {
 		t.Run(tcase.name, func(t *testing.T) {
-			confRoot := filepath.Join(testDataDir, "conftree", tcase.name)
+			confRoot := filepath.Join(testDir, "conftree", tcase.name)
 			extraPath := FixExtraConfigPath(confRoot)
 
 			var pArgs ProgArgs

pkg/config/config.go

@@ -18,8 +18,10 @@ package config
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
+	"path/filepath"
 
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/yaml"
@@ -29,6 +31,10 @@ import (
 	"github.com/k8stopologyawareschedwg/resource-topology-exporter/pkg/resourcetopologyexporter"
 )
 
+var (
+	SkipDirectory = errors.New("skip config directory")
+)
+
 type GlobalArgs struct {
 	KubeConfig string `json:"kubeConfig,omitempty"`
 	Debug      bool   `json:"debug,omitempty"`
@@ -150,3 +156,16 @@ func Finalize(pArgs *ProgArgs) error {
 	}
 	return err
 }
+
+func UserRunDir() (string, error) {
+	return filepath.Join("/run", "user", fmt.Sprintf("%d", os.Getuid()), "rte"), nil
+}
+
+func UserHomeDir() (string, error) {
+	homeDir, ok := os.LookupEnv("HOME")
+	if !ok || homeDir == "" {
+		// can happen in CI
+		return "", SkipDirectory
+	}
+	return filepath.Join(homeDir, ".rte"), nil
+}

pkg/config/config_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
 	"strings"
@@ -26,6 +27,9 @@ import (
 )
 
 func TestLoadArgs(t *testing.T) {
+	_, closer := setupTest(t)
+	t.Cleanup(closer)
+
 	type testCase struct {
 		name string
 	}
@@ -39,7 +43,8 @@ func TestLoadArgs(t *testing.T) {
 		},
 	} {
 		t.Run(tcase.name, func(t *testing.T) {
-			confRoot := filepath.Join(testDataDir, "conftree", tcase.name)
+			userDir, _ := UserRunDir()
+			confRoot := filepath.Join(userDir, "conftree", tcase.name)
 
 			environ := filepath.Join(confRoot, "_env", "vars.yaml")
 			setupEnviron(t, environ)
@@ -69,6 +74,14 @@ func TestLoadArgs(t *testing.T) {
 	}
 }
 
+func TestUserHomeDirWithoutEnv(t *testing.T) {
+	t.Setenv("HOME", "")
+	_, err := UserHomeDir()
+	if !errors.Is(err, SkipDirectory) {
+		t.Fatalf("returned unexpected error: %v", err)
+	}
+}
+
 func setupEnviron(t *testing.T, envDefsPath string) {
 	t.Helper()
 	data, err := os.ReadFile(envDefsPath)

pkg/config/defaults_test.go

@@ -25,7 +25,7 @@ import (
 )
 
 func TestSetDefaults(t *testing.T) {
-	closer := setupTest(t)
+	_, closer := setupTest(t)
 	t.Cleanup(closer)
 
 	pArgs := ProgArgs{}

pkg/config/environ_test.go

@@ -26,7 +26,7 @@ func TestFromEnv(t *testing.T) {
 	tmPol := "restricted"
 	tmScope := "pod"
 
-	closer := setupTestWithEnv(t, map[string]string{
+	_, closer := setupTestWithEnv(t, map[string]string{
 		"TOPOLOGY_MANAGER_POLICY":  tmPol,
 		"TOPOLOGY_MANAGER_SCOPE":   tmScope,
 		"NODE_NAME":                nodeName,

pkg/config/flags_test.go

@@ -19,6 +19,7 @@ package config
 import (
 	"flag"
 	"fmt"
+	"log"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -43,7 +44,7 @@ var (
 )
 
 func TestOneshot(t *testing.T) {
-	closer := setupTest(t)
+	_, closer := setupTest(t)
 	t.Cleanup(closer)
 
 	pArgs, err := LoadArgs("--oneshot", "--no-publish")
@@ -59,7 +60,7 @@ func TestOneshot(t *testing.T) {
 }
 
 func TestReferenceContainer(t *testing.T) {
-	closer := setupTest(t)
+	_, closer := setupTest(t)
 	t.Cleanup(closer)
 
 	pArgs, err := LoadArgs("--reference-container=ns/pod/cont")
@@ -74,7 +75,7 @@ func TestReferenceContainer(t *testing.T) {
 }
 
 func TestRefreshNodeResources(t *testing.T) {
-	closer := setupTest(t)
+	_, closer := setupTest(t)
 	t.Cleanup(closer)
 
 	pArgs, err := LoadArgs("--refresh-node-resources")
@@ -87,7 +88,7 @@ func TestRefreshNodeResources(t *testing.T) {
 }
 
 func TestLoadDefaults(t *testing.T) {
-	closer := setupTest(t)
+	_, closer := setupTest(t)
 	t.Cleanup(closer)
 
 	pArgs, err := LoadArgs()
@@ -119,7 +120,8 @@ func TestLoadDefaults(t *testing.T) {
 	}
 }
 
-func setupTest(t *testing.T) func() {
+func setupTest(t *testing.T) (string, func()) {
+	t.Helper()
 	return setupTestWithEnv(t, map[string]string{
 		"NODE_NAME":                node,
 		"REFERENCE_NAMESPACE":      namespace,
@@ -128,7 +130,8 @@ func setupTest(t *testing.T) func() {
 	})
 }
 
-func setupTestWithEnv(t *testing.T, envs map[string]string) func() {
+func setupTestWithEnv(t *testing.T, envs map[string]string) (string, func()) {
+	t.Helper()
 	_, file, _, ok := runtime.Caller(0)
 	if !ok {
 		t.Fatal("Cannot retrieve tests directory")
@@ -137,10 +140,24 @@ func setupTestWithEnv(t *testing.T, envs map[string]string) func() {
 	baseDir = filepath.Dir(file)
 	testDataDir = filepath.Clean(filepath.Join(baseDir, "..", "..", "test", "data"))
 
-	return envSetter(envs)
+	userDir, err := UserRunDir()
+	if err == nil {
+		err2 := os.CopyFS(userDir, os.DirFS(testDataDir))
+		log.Printf("copyfs %q -> %q err=%v", testDataDir, userDir, err2)
+	}
+
+	closer := envSetter(envs)
+	return userDir, func() {
+		closer()
+		if err != nil {
+			return
+		}
+		err2 := os.RemoveAll(userDir)
+		log.Printf("cleaned up %q err=%v", userDir, err2)
+	}
 }
 
-func envSetter(envs map[string]string) (closer func()) {
+func envSetter(envs map[string]string) func() {
 	originalEnvs := map[string]string{}
 
 	for name, value := range envs {

pkg/config/validation.go

@@ -17,6 +17,12 @@ limitations under the License.
 package config
 
 import (
+	"errors"
+	"fmt"
+	"io/fs"
+	"path/filepath"
+	"strings"
+
 	metricssrv "github.com/k8stopologyawareschedwg/resource-topology-exporter/pkg/metrics/server"
 	"github.com/k8stopologyawareschedwg/resource-topology-exporter/pkg/resourcemonitor"
 )
@@ -36,3 +42,75 @@ func Validate(pArgs *ProgArgs) error {
 
 	return nil
 }
+
+func validateConfigLetPath(configletDir, configletName string) (string, error) {
+	origConfigletPath := filepath.Clean(filepath.Join(configletDir, configletName))
+	if filepath.Dir(origConfigletPath) != configletDir {
+		return "", fmt.Errorf("configlet %q is not a proper subtree of %q", configletName, configletDir)
+	}
+	configletPath, err := filepath.EvalSymlinks(origConfigletPath)
+	if err != nil {
+		return "", err
+	}
+	if configletPath != origConfigletPath {
+		return "", fmt.Errorf("configlet %q seems to be a symlink which is not allowed", configletName)
+	}
+	return configletPath, nil
+}
+
+func validateConfigRootPath(configRoot string) (string, error) {
+	if configRoot == "" {
+		return "", fmt.Errorf("configRoot is not allowed to be an empty string")
+	}
+
+	configRoot = filepath.Clean(configRoot)
+	cfgRoot, err := filepath.EvalSymlinks(configRoot)
+	if err != nil {
+		if errors.Is(err, fs.ErrNotExist) {
+			// reset to original value, it somehow passed the symlink check
+			cfgRoot = configRoot
+		} else {
+			return "", fmt.Errorf("failed to validate configRoot path: %w", err)
+		}
+	}
+	// else either success or checking a non-existing path. Which can be still OK.
+
+	pattern, err := IsConfigRootAllowed(cfgRoot, UserRunDir, UserHomeDir)
+	if err != nil {
+		return "", err
+	}
+	if pattern == "" {
+		return "", fmt.Errorf("failed to validate configRoot path %q: does not match any allowed pattern", cfgRoot)
+	}
+	return cfgRoot, nil
+}
+
+// IsConfigRootAllowed checks if an *already cleaned and canonicalized* path is among the allowed list.
+// use `addDirFns` to inject user-dependent paths (e.g. $HOME). Returns the matched pattern, if any,
+// and error describing the failure. The error is only relevant if failed to inject user-provided paths.
+func IsConfigRootAllowed(cfgPath string, addDirFns ...func() (string, error)) (string, error) {
+	allowedPatterns := []string{
+		"/etc/rte",
+		"/run/rte",
+		"/var/rte",
+		"/usr/local/etc/rte",
+		"/etc/resource-topology-exporter", // legacy, but still supported
+	}
+	for _, addDirFn := range addDirFns {
+		userDir, err := addDirFn()
+		if errors.Is(err, SkipDirectory) {
+			continue
+		}
+		if err != nil {
+			return "", err
+		}
+		allowedPatterns = append(allowedPatterns, userDir)
+	}
+
+	for _, pattern := range allowedPatterns {
+		if strings.HasPrefix(cfgPath, pattern) {
+			return pattern, nil
+		}
+	}
+	return "", nil
+}