Careful handling when loading files

lib/cgroup/cgroup.go

@@ -41,6 +41,7 @@ import (
 
 	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/lib/defaults"
+	"github.com/gravitational/teleport/lib/utils"
 )
 
 var log = logrus.WithFields(logrus.Fields{
@@ -171,7 +172,7 @@ func (s *Service) Place(sessionID string, pid int) error {
 // readPids returns a slice of PIDs from a file. Used to get list of all PIDs
 // within a cgroup.
 func readPids(path string) ([]string, error) {
-	f, err := os.Open(path)
+	f, err := utils.OpenFileNoUnsafeLinks(path)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

lib/config/configuration.go

@@ -271,7 +271,7 @@ func ReadConfigFile(cliConfigPath string) (*FileConfig, error) {
 
 // ReadResources loads a set of resources from a file.
 func ReadResources(filePath string) ([]types.Resource, error) {
-	reader, err := utils.OpenFile(filePath)
+	reader, err := utils.OpenFileAllowingUnsafeLinks(filePath)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}

lib/config/fileconf.go

@@ -104,7 +104,7 @@ type FileConfig struct {
 // ReadFromFile reads Teleport configuration from a file. Currently only YAML
 // format is supported
 func ReadFromFile(filePath string) (*FileConfig, error) {
-	f, err := os.Open(filePath)
+	f, err := utils.OpenFileAllowingUnsafeLinks(filePath)
 	if err != nil {
 		if errors.Is(err, fs.ErrPermission) {
 			return nil, trace.Wrap(err, "failed to open file for Teleport configuration: %v. Ensure that you are running as a user with appropriate permissions.", filePath)

lib/srv/exec.go

@@ -41,6 +41,7 @@ import (
 	apievents "github.com/gravitational/teleport/api/types/events"
 	"github.com/gravitational/teleport/lib/events"
 	"github.com/gravitational/teleport/lib/services"
+	"github.com/gravitational/teleport/lib/utils"
 )
 
 const (
@@ -526,7 +527,7 @@ func getDefaultEnvPath(uid string, loginDefsPath string) string {
 	envRootPath := defaultEnvRootPath
 
 	// open file, if it doesn't exist return a default path and move on
-	f, err := os.Open(loginDefsPath)
+	f, err := utils.OpenFileAllowingUnsafeLinks(loginDefsPath)
 	if err != nil {
 		if uid == "0" {
 			log.Infof("Unable to open %q: %v: returning default su path: %q", loginDefsPath, err, defaultEnvRootPath)

lib/tbot/config/config.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"io"
 	"net/url"
-	"os"
 	"reflect"
 	"strings"
 	"time"
@@ -663,7 +662,7 @@ func FromCLIConf(cf *CLIConf) (*BotConfig, error) {
 
 // ReadConfigFromFile reads and parses a YAML config from a file.
 func ReadConfigFromFile(filePath string, manualMigration bool) (*BotConfig, error) {
-	f, err := os.Open(filePath)
+	f, err := utils.OpenFileAllowingUnsafeLinks(filePath)
 	if err != nil {
 		return nil, trace.Wrap(err, fmt.Sprintf("failed to open file: %v", filePath))
 	}

lib/utils/environment.go

@@ -16,7 +16,6 @@ package utils
 
 import (
 	"bufio"
-	"os"
 	"strings"
 
 	log "github.com/sirupsen/logrus"
@@ -30,7 +29,7 @@ import (
 func ReadEnvironmentFile(filename string) ([]string, error) {
 	// open the users environment file. if we don't find a file, move on as
 	// having this file for the user is optional.
-	file, err := os.Open(filename)
+	file, err := OpenFileNoUnsafeLinks(filename)
 	if err != nil {
 		log.Warnf("Unable to open environment file %v: %v, skipping", filename, err)
 		return []string{}, nil

lib/utils/fs.go

@@ -23,6 +23,8 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"runtime"
+	"strings"
 	"time"
 
 	"github.com/gofrs/flock"
@@ -84,30 +86,69 @@ func IsDir(path string) bool {
 
 // NormalizePath normalises path, evaluating symlinks and converting local
 // paths to absolute
-func NormalizePath(path string) (string, error) {
+func NormalizePath(path string, evaluateSymlinks bool) (string, error) {
 	s, err := filepath.Abs(path)
 	if err != nil {
 		return "", trace.ConvertSystemError(err)
 	}
-	abs, err := filepath.EvalSymlinks(s)
-	if err != nil {
-		return "", trace.ConvertSystemError(err)
+	if evaluateSymlinks {
+		s, err = filepath.EvalSymlinks(s)
+		if err != nil {
+			return "", trace.ConvertSystemError(err)
+		}
 	}
-	return abs, nil
+	return s, nil
+}
+
+// OpenFileAllowingUnsafeLinks opens a file, if the path includes a symlink, the returned os.File will be resolved to
+// the actual file.  This will return an error if the file is not found or is a directory.
+func OpenFileAllowingUnsafeLinks(path string) (*os.File, error) {
+	return openFile(path, true /* allowSymlink */, true /* allowMultipleHardlinks */)
 }
 
-// OpenFile opens  file and returns file handle
-func OpenFile(path string) (*os.File, error) {
-	newPath, err := NormalizePath(path)
+// OpenFileNoUnsafeLinks opens a file, ensuring it's an actual file and not a directory or symlink.  Depending on
+// the os, it may also prevent hardlinks.  This is important because MacOS allows hardlinks without validating write
+// permissions (similar to a symlink in that regard).
+func OpenFileNoUnsafeLinks(path string) (*os.File, error) {
+	return openFile(path, false /* allowSymlink */, runtime.GOOS != "darwin" /* allowMultipleHardlinks */)
+}
+
+func openFile(path string, allowSymlink, allowMultipleHardlinks bool) (*os.File, error) {
+	newPath, err := NormalizePath(path, allowSymlink)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	fi, err := os.Stat(newPath)
-	if err != nil {
-		return nil, trace.ConvertSystemError(err)
+	var fi os.FileInfo
+	if allowSymlink {
+		fi, err = os.Stat(newPath)
+		if err != nil {
+			return nil, trace.ConvertSystemError(err)
+		}
+	} else {
+		components := strings.Split(newPath, string(os.PathSeparator))
+		var subPath string
+		for _, p := range components {
+			subPath = filepath.Join(subPath, p)
+			if subPath == "" {
+				subPath = string(os.PathSeparator)
+			}
+
+			fi, err = os.Lstat(subPath)
+			if err != nil {
+				return nil, trace.ConvertSystemError(err)
+			} else if fi.Mode().Type()&os.ModeSymlink != 0 {
+				return nil, trace.BadParameter("opening file %s, symlink not allowed in path: %s", path, subPath)
+			}
+		}
+	}
+	if !allowMultipleHardlinks {
+		// hardlinks can only exist at the end file, not for directories within the path
+		if linkCount, ok := getHardLinkCount(fi); ok && linkCount > 1 {
+			return nil, trace.BadParameter("file has hardlink count greater than 1: %s", path)
+		}
 	}
 	if fi.IsDir() {
-		return nil, trace.BadParameter("%v is not a file", path)
+		return nil, trace.BadParameter("%s is not a file", path)
 	}
 	f, err := os.Open(newPath)
 	if err != nil {
@@ -118,7 +159,7 @@ func OpenFile(path string) (*os.File, error) {
 
 // StatFile stats path, returns error if it exists but a directory.
 func StatFile(path string) (os.FileInfo, error) {
-	newPath, err := NormalizePath(path)
+	newPath, err := NormalizePath(path, true)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}