Skip to content

Commit

Permalink
config: add path validation
Browse files Browse the repository at this point in the history
WIP TBD

Signed-off-by: Francesco Romani <[email protected]>
  • Loading branch information
ffromani committed Nov 27, 2024
1 parent a08c327 commit bf48a27
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 12 deletions.
18 changes: 11 additions & 7 deletions pkg/config/cfgfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,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
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/config/cfgfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ func TestReadResourceExclude(t *testing.T) {
}

func TestFromFiles(t *testing.T) {
closer := setupTest(t)
t.Cleanup(closer)

type testCase struct {
name string
}
Expand Down
23 changes: 21 additions & 2 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"encoding/json"
"fmt"
"os"
"path/filepath"

"k8s.io/klog/v2"
"sigs.k8s.io/yaml"
Expand Down Expand Up @@ -107,13 +108,19 @@ func LoadArgs(args ...string) (ProgArgs, error) {
return pArgs, err
}

// this needs to be done as early as possible
cfgRoot, err := validateConfigRootPath(configRoot)
if err != nil {
return pArgs, err
}

// now the real processing begins. From now on we waste nothing
if pArgs.Global.Debug {
klog.Infof("configRoot=%q extraConfigPath=%q", configRoot, extraConfigPath)
klog.Infof("configRoot=%q extraConfigPath=%q", cfgRoot, extraConfigPath)
klog.Infof("config defaults:{{\n%s}}", pArgs.ToYAMLString())
}

err = FromFiles(&pArgs, configRoot, extraConfigPath)
err = FromFiles(&pArgs, cfgRoot, extraConfigPath)
if err != nil {
return pArgs, err
}
Expand Down Expand Up @@ -150,3 +157,15 @@ 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, err := os.UserHomeDir()
if err != nil {
return "", err
}
return filepath.Join(homeDir, ".rte"), nil
}
6 changes: 5 additions & 1 deletion pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ import (
)

func TestLoadArgs(t *testing.T) {
closer := setupTest(t)
t.Cleanup(closer)

type testCase struct {
name string
}
Expand All @@ -39,7 +42,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)
Expand Down
19 changes: 17 additions & 2 deletions pkg/config/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package config
import (
"flag"
"fmt"
"log"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -137,10 +138,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 func() {
closer()
if err != nil {
return
}
err2 := os.Remove(userDir)
log.Printf("unlinked %q -> %q err=%v", testDataDir, 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 {
Expand Down
70 changes: 70 additions & 0 deletions pkg/config/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -36,3 +42,67 @@ func Validate(pArgs *ProgArgs) error {

return nil
}

func validateConfigLetPath(configletDir, configletName string) (string, error) {
configletPath, err := filepath.EvalSymlinks(filepath.Clean(filepath.Join(configletDir, configletName)))
if err != nil {
return "", err
}
if filepath.Dir(configletPath) != configletDir {
return "", fmt.Errorf("configlet %q is not within %q", configletName, configletDir)
}
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 "", errors.New("failed to validate configRoot path: not matches any allowed pattern")
}

// clear pattern to make the tool happy
relPath := strings.TrimPrefix(cfgRoot, pattern)
return filepath.Abs(filepath.Clean(filepath.Join(pattern, relPath)))
}

func IsConfigRootAllowed(cfgPath string, addDirFns ...func() (string, error)) (string, error) {
allowedPatterns := []string{
"/etc/rte",
"/run/rte",
"/var/rte",
"/usr/local/etc/rte",
}
for _, addDirFn := range addDirFns {
userDir, err := addDirFn()
if err != nil {
return "", err
}
allowedPatterns = append(allowedPatterns, userDir)
}

for _, pattern := range allowedPatterns {
if strings.HasPrefix(cfgPath, pattern) {
return pattern, nil
}
}
return "", nil
}

0 comments on commit bf48a27

Please sign in to comment.