diff --git a/lib/fsutil/api.go b/lib/fsutil/api.go index 0e1e5d6a..97abb30d 100644 --- a/lib/fsutil/api.go +++ b/lib/fsutil/api.go @@ -32,13 +32,25 @@ func AppendFile(destFilename, sourceFilename string) error { return appendFile(destFilename, sourceFilename) } +// AppendFileWithRoot extends AppendFile with safe symlink evaluation. Relative +// symlinks are clamped to the root boundary, and absolute symlinks are rebased +// against the root using chroot-style semantics. +func AppendFileWithRoot(rootFd int, destRelPath, sourcePath string) error { + return appendFileWithRoot(rootFd, destRelPath, sourcePath) +} + // AppendTree recursively merges sourceDir into destDir. -// It appends contents to existing files or copies new ones -// while preserving permissions. -// Directory structures are mirrored. -// Returns an error if symlinks or non-regular files are encountered. +// Existing regular files will have data appended. Files which do not exist in +// destDir will be copied with the source file permissions. +// Directory structures will be mirrored. An error is returned if symlinks or +// non-regular files are encountered in sourceDir. If a destination path is a +// symlink, it is resolved within destDir using chroot-style semantics: +// absolute targets are anchored at destDir and ".." is clamped at its root. +// Dangling symlinks cause an error. +// Valid symlink targets within destDir will have data appended +// to the resolved file; the symlink itself is preserved. func AppendTree(destDir, sourceDir string) error { - return appendTree(destDir, sourceDir, AppendFile) + return appendTree(destDir, sourceDir, AppendFileWithRoot) } // CompareFile will read and compare the content of a file and buffer and will diff --git a/lib/fsutil/append.go b/lib/fsutil/append.go index d861dd34..f70925e3 100644 --- a/lib/fsutil/append.go +++ b/lib/fsutil/append.go @@ -7,6 +7,8 @@ import ( "io/fs" "os" "path/filepath" + + "golang.org/x/sys/unix" ) func appendToFile(destFilename string, reader io.Reader, @@ -40,6 +42,7 @@ func appendFile(destFilename, sourceFilename string) error { } return copyFile(destFilename, sourceFilename, mode, false) } + return err } sourceFile, err := os.Open(sourceFilename) if err != nil { @@ -50,28 +53,54 @@ func appendFile(destFilename, sourceFilename string) error { return appendToFile(destFilename, sourceFile, 0) } +func appendFileWithRoot(rootFd int, destRelPath, sourcePath string) error { + mode, err := getFilePerms(sourcePath) + if err != nil { + return err + } + destFile, err := secureOpenFile(rootFd, destRelPath, uint32(mode)) + if err != nil { + return err + } + defer destFile.Close() + sourceFile, err := os.Open(sourcePath) + if err != nil { + return errors.New(sourcePath + ": " + err.Error()) + } + _, err = io.Copy(destFile, sourceFile) + if err != nil { + return fmt.Errorf( + "error copying contents from source %q to dest %q: %w", + sourcePath, destRelPath, err) + } + return nil +} + func appendTree(destDir, sourceDir string, - appendFunc func(dest, src string) error) error { + appendFunc func(rootFd int, destRelPath, sourcePath string) error) error { + rootFd, err := openRoot(destDir) + if err != nil { + return err + } + defer unix.Close(rootFd) return filepath.WalkDir(sourceDir, func(path string, d fs.DirEntry, err error) error { if err != nil { return err } + if path == sourceDir { + return nil + } relPath, err := filepath.Rel(sourceDir, path) if err != nil { return err } - destFilename := filepath.Join(destDir, relPath) fileType := d.Type() switch { case fileType.IsDir(): - // If path is a directory, create directory and return. - // WalkDir will automatically visit the children next. - if err := os.MkdirAll(destFilename, DirPerms); err != nil { - return err - } + return secureMkdir(rootFd, relPath, DirPerms) case fileType.IsRegular(): - if err := appendFunc(destFilename, path); err != nil { + if err := appendFunc(rootFd, relPath, path); err != nil { return err } case fileType&fs.ModeSymlink != 0: diff --git a/lib/fsutil/append_test.go b/lib/fsutil/append_test.go index 564ba25f..30cba937 100644 --- a/lib/fsutil/append_test.go +++ b/lib/fsutil/append_test.go @@ -3,9 +3,11 @@ package fsutil import ( "bytes" "errors" + "fmt" "io" "os" "path/filepath" + "strings" "testing" ) @@ -18,6 +20,7 @@ func createBaseDirectory(t *testing.T, path string, perms os.FileMode) { if !info.IsDir() { t.Fatalf("path exists but is a file: %s", dir) } + return } if !os.IsNotExist(err) { t.Fatal(err.Error()) @@ -29,30 +32,30 @@ func createBaseDirectory(t *testing.T, path string, perms os.FileMode) { } func TestAppendFileNonExistingDestFile(t *testing.T) { - const ( - sourceFileName = "dir1/source" - destFileName = "dir2/dir3/dest" - ) - // setup source file. - tmp := t.TempDir() + // Setup source file. + sourceTmp := t.TempDir() + destTmp := t.TempDir() var ( + sourceDir = filepath.Join(sourceTmp, "source/dir1") + destDir = filepath.Join(destTmp, "dest/dir2/dir3") + filename = "test.txt" sourceFileData = []byte( "#/usr/bin/bash\nVAR1=$(which bash)\necho $VAR1\nthis is \n\ttest data\n", ) expectedDestFileData = sourceFileData filePerms os.FileMode = 0600 ) - sourceFilePath := filepath.Join(tmp, sourceFileName) - destFilePath := filepath.Join(tmp, destFileName) + sourceFilePath := filepath.Join(sourceDir, filename) + destFilePath := filepath.Join(destDir, filename) createBaseDirectory(t, sourceFilePath, 0755) - // create source file with data. + // Create source file with data. if err := copyToFile(sourceFilePath, 0600, bytes.NewReader(sourceFileData), 0); err != nil { t.Fatalf("error creating source file %s: %s\n", sourceFilePath, err.Error()) } - // skipping creation of dest file path. - // check dest file doesn't exist before append. + // Skipping creation of dest file path. + // Check dest file doesn't exist before append. _, err := os.Stat(destFilePath) if err == nil || !errors.Is(err, os.ErrNotExist) { t.Fatal("destfile exists already\n") @@ -61,14 +64,11 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { filepath.Dir(sourceFilePath)); err != nil { t.Fatalf("error appending to file: %s\n", err.Error()) } - // Since Destination file is not present, the entire Tree will be - // created with source file. - finalDestPath := filepath.Join(tmp, "dir2/dir3/source") - f, _ := os.OpenFile(finalDestPath, os.O_RDONLY, 0) + f, _ := os.OpenFile(destFilePath, os.O_RDONLY, 0) d, _ := io.ReadAll(f) t.Logf("file content is \n%s\n", string(d)) - // check file perm of dest, it should be same as source. - mode, err := getFilePerms(finalDestPath) + // Check file perm of dest, it should be same as source. + mode, err := getFilePerms(destFilePath) if err != nil { t.Fatalf("error getting dest file perms %s\n", err.Error()) } @@ -79,8 +79,8 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { filePerms, ) } - // dest file should exist. - same, err := CompareFile(expectedDestFileData, finalDestPath) + // Dest file should exist. + same, err := CompareFile(expectedDestFileData, destFilePath) if err != nil { t.Fatalf("error appending to file: %s\n", err.Error()) } @@ -90,24 +90,26 @@ func TestAppendFileNonExistingDestFile(t *testing.T) { } func TestAppendFileWithExistingDestFile(t *testing.T) { - // setup source file. - const ( - sourceFileName = "dir1/dir2/dir3/source" - destFileName = "dir4/dir2/dir3/dest" - ) - tmp := t.TempDir() + // Setup source file. + sourceTmp := t.TempDir() + destTmp := t.TempDir() var ( + sourceDir = filepath.Join(sourceTmp, "source/dir1") + destDir = filepath.Join(destTmp, "dest/dir2/dir3") + filename = "test.txt" sourceFileData = []byte( "#/usr/bin/bash\nVAR1=$(which bash)\necho $VAR1\nthis is \n\ttest data\n", ) - destFileData = []byte("#/usr/bin/python\necho 'this is test data'\n") + destFileData = []byte( + "#/usr/bin/python\necho 'this is test data'\n", + ) expectedDestFileData = append(destFileData, sourceFileData...) ) - sourceFilePath := filepath.Join(tmp, sourceFileName) - destFilePath := filepath.Join(tmp, destFileName) + sourceFilePath := filepath.Join(sourceDir, filename) + destFilePath := filepath.Join(destDir, filename) createBaseDirectory(t, sourceFilePath, 0755) createBaseDirectory(t, destFilePath, 0755) - // create source file with data. + // Create source file with data. if err := copyToFile( sourceFilePath, PublicFilePerms, @@ -117,7 +119,7 @@ func TestAppendFileWithExistingDestFile(t *testing.T) { t.Fatalf("error creating source file %s: %s\n", sourceFilePath, err.Error()) } - // create dest file with data. + // Create dest file with data. if err := copyToFile( destFilePath, PublicFilePerms, @@ -126,13 +128,14 @@ func TestAppendFileWithExistingDestFile(t *testing.T) { ); err != nil { t.Fatalf("error creating dest file %s: %s\n", destFilePath, err.Error()) } - if err := AppendFile(destFilePath, sourceFilePath); err != nil { + if err := AppendTree(filepath.Dir(destFilePath), + filepath.Dir(sourceFilePath)); err != nil { t.Fatalf("error appending to file: %s\n", err.Error()) } f, _ := os.OpenFile(destFilePath, os.O_RDONLY, 0) d, _ := io.ReadAll(f) t.Logf("file content is \n%s\n", string(d)) - // dest file should exist. + // Dest file should exist. same, err := CompareFile(expectedDestFileData, destFilePath) if err != nil { t.Fatalf("error appending to file: %s\n", err.Error()) @@ -141,3 +144,278 @@ func TestAppendFileWithExistingDestFile(t *testing.T) { t.Fatalf("contents mismatch after append") } } + +func TestAppendFileWithDanglingDestSymlinks(t *testing.T) { + sourceTmp := t.TempDir() + destTmp := t.TempDir() + var ( + sourceDir = filepath.Join(sourceTmp, "etc/config") + destDir = filepath.Join(destTmp, "etc/config") + filename = "test.txt" + sourceFilePath = filepath.Join(sourceDir, filename) + destFilePath = filepath.Join(destDir, filename) + ) + danglingSymlinkPaths := + []struct { + name, + danglingSymlinkPath, + // Resolved path in root with clamping. + resolvedDanglingSymlinkPath string + }{ + { + name: "relative_path_1", + danglingSymlinkPath: "../run/systemd/test-service.txt", + resolvedDanglingSymlinkPath: filepath.Join( + destTmp, + "etc/run/systemd/test-service.txt", + ), + }, + { + name: "relative_path_2", + danglingSymlinkPath: "../../run/systemd/test-service.txt", + resolvedDanglingSymlinkPath: filepath.Join( + destTmp, + "run/systemd/test-service.txt", + ), + }, + { + name: "relative_path_clamp_root", + danglingSymlinkPath: "../../../../../../run/systemd/test-service.txt", + resolvedDanglingSymlinkPath: filepath.Join( + destTmp, + "run/systemd/test-service.txt", + ), + }, + } + createBaseDirectory(t, sourceFilePath, 0755) + createBaseDirectory(t, destFilePath, 0755) + // Create source file with data. + if err := copyToFile( + sourceFilePath, + PublicFilePerms, + bytes.NewReader([]byte{}), + 0, + ); err != nil { + t.Fatalf("error creating source file %s: %s\n", + sourceFilePath, err.Error()) + } + for _, danglingSymlink := range danglingSymlinkPaths { + t.Run(danglingSymlink.name, func(t *testing.T) { + // Setup dangling symlink at destFile. + if err := os.Symlink(danglingSymlink.danglingSymlinkPath, + destFilePath); err != nil { + t.Fatalf("error creating dangling symlink: %s", err) + } + defer func() { + err := os.Remove(destFilePath) + if err != nil { + t.Fatalf("error removing symlink: %s", err) + } + }() + err := AppendTree(destTmp, sourceTmp) + if err == nil { + t.Fatalf("expected error for dangling symlinks") + } + fmt.Println(err.Error()) + if !strings.EqualFold(err.Error(), + fmt.Sprintf( + "dangling symlink: %q resolves to missing target %q", + destFilePath, + danglingSymlink.resolvedDanglingSymlinkPath, + ), + ) { + t.Fatalf("unexpected error") + } + }) + } +} + +func TestAppendFileWithClampingTargetSymlinks(t *testing.T) { + sourceTmp := t.TempDir() + destTmp := t.TempDir() + var ( + sourceDir = filepath.Join(sourceTmp, "etc/config") + destDir = filepath.Join(destTmp, "etc/config") + filename = "test.txt" + sourceFilePath = filepath.Join(sourceDir, filename) + sourceData = []byte( + "#/usr/bin/bash\n\tThis is test data from source\n", + ) + destData = []byte( + "#/usr/bin/bash\n\tThis is test data from dest\n", + ) + destFilePath = filepath.Join(destDir, filename) + danglingSymlinkPath = "../../../run/systemd/test-service.txt" + expectedData = append(destData, sourceData...) + ) + createBaseDirectory(t, sourceFilePath, 0755) + createBaseDirectory(t, destFilePath, 0755) + symlinkTargetFullPath := filepath.Clean( + filepath.Join(destDir, + strings.TrimPrefix( + danglingSymlinkPath, + ".."+string(filepath.Separator), + ), + ), + ) + fmt.Println(symlinkTargetFullPath) + createBaseDirectory(t, symlinkTargetFullPath, 0755) + // Create source file with data. + if err := copyToFile( + sourceFilePath, + PublicFilePerms, + bytes.NewReader(sourceData), + 0, + ); err != nil { + t.Fatalf("error creating source file %s: %s\n", + sourceFilePath, err.Error()) + } + if err := copyToFile( + symlinkTargetFullPath, + PublicFilePerms, + bytes.NewReader(destData), + 0, + ); err != nil { + t.Fatalf("error creating symlink target file %s: %s\n", + symlinkTargetFullPath, err.Error()) + } + if err := os.Symlink(danglingSymlinkPath, destFilePath); err != nil { + t.Fatalf("error creating dangling symlink: %s", err) + } + err := AppendTree(destTmp, sourceTmp) + if err != nil { + t.Fatalf("unexpected error in AppendTree: %s", err) + } + f, err := os.OpenFile(symlinkTargetFullPath, os.O_RDONLY, 0) + if err != nil { + t.Fatalf("error opening %s: %s", symlinkTargetFullPath, err) + } + d, err := io.ReadAll(f) + if err != nil { + t.Fatalf("error reading data from %s: %s", symlinkTargetFullPath, err) + } + t.Logf("file content is \n%s\n", string(d)) + same, err := compareFile(expectedData, symlinkTargetFullPath) + if err != nil { + t.Fatalf("error comparing to file %s: %s", symlinkTargetFullPath, err) + } + if !same { + t.Fatalf( + "mismatched contents, present: %s\nexpected: %s", + expectedData, d, + ) + } + //Check if symlink is intact. + expectedSymlinkPath, err := os.Readlink(destFilePath) + if err != nil { + t.Fatalf("unexpected error with symlink %s: %s", destFilePath, err) + } + if expectedSymlinkPath != danglingSymlinkPath { + t.Fatalf("symlink is broken.") + } +} + +func TestAppendFileWithExistingTargetSymlinks(t *testing.T) { + var ( + filename = "test.txt" + symlinkPaths = map[string]string{ + "relPathTest": "../../run/systemd/new-rel-test-file.txt", + "absPathTest": "/var/run/systemd/new-abs-test-file.txt", + } + sourceData = []byte("This is from source\n") + destData = []byte( + "#/usr/bin/bash\necho 'hello world'\n\tThis is from symlink", + ) + expectedData = append(destData, sourceData...) + ) + for name, symlinkPath := range symlinkPaths { + t.Run(name, func(t *testing.T) { + sourceTmp := t.TempDir() + destTmp := t.TempDir() + var ( + sourceDir = filepath.Join(sourceTmp, "etc/config") + destDir = filepath.Join(destTmp, "etc/config") + sourceFilePath = filepath.Join(sourceDir, filename) + destFilePath = filepath.Join(destDir, filename) + ) + createBaseDirectory(t, sourceFilePath, 0755) + createBaseDirectory(t, destFilePath, 0755) + var rootDir string + if !filepath.IsAbs(symlinkPath) { + rootDir = destDir + } else { + rootDir = destTmp + } + symlinkTargetFullPath := filepath.Clean( + filepath.Join(rootDir, symlinkPath), + ) + createBaseDirectory(t, symlinkTargetFullPath, 0755) + // Create source file with data. + if err := copyToFile( + sourceFilePath, + PublicFilePerms, + bytes.NewReader(sourceData), + 0, + ); err != nil { + t.Fatalf("error creating source file %s: %s\n", + sourceFilePath, err.Error()) + } + // Create symlink target path. + if err := copyToFile( + symlinkTargetFullPath, + PublicFilePerms, + bytes.NewReader(destData), + 0, + ); err != nil { + t.Fatalf("error creating symlink target file %s: %s\n", + symlinkTargetFullPath, err.Error()) + } + // Create symlink for destFilePath to targetPath. + if err := os.Symlink(symlinkPath, destFilePath); err != nil { + t.Fatalf("error creating dangling symlink: %s", err) + } + err := AppendTree(destTmp, sourceTmp) + if err != nil { + t.Fatalf("unexpected error in appendTree: %s", err) + } + // If symlink target is absolute, we need to append rootDir + // for validating data. + var expectedEvaluatedPath string + if !filepath.IsAbs(symlinkPath) { + expectedEvaluatedPath = destFilePath + } else { + expectedEvaluatedPath = symlinkTargetFullPath + } + // Check if contents match. + f, err := os.OpenFile(expectedEvaluatedPath, os.O_RDONLY, 0) + if err != nil { + t.Fatalf("error opening expected file: %s", err) + } + d, err := io.ReadAll(f) + if err != nil { + t.Fatalf("error reading file contents: %s", err) + } + t.Log("file contents is", string(d)) + same, err := compareFile(expectedData, expectedEvaluatedPath) + if err != nil { + t.Fatalf("error comparing to file: %s\n", err.Error()) + } + if !same { + t.Fatalf("contents mismatch after append") + } + // Check if symlink stays intact. + linkPath, err := os.Readlink(destFilePath) + if err != nil { + t.Fatalf( + "error checking target %s of symlink %s: %s", + destFilePath, + symlinkPath, + err, + ) + } + if linkPath != symlinkPath { + t.Fatalf("symlink targets don't match") + } + }) + } +} diff --git a/lib/fsutil/copy.go b/lib/fsutil/copy.go index 74c44ae2..b3981553 100644 --- a/lib/fsutil/copy.go +++ b/lib/fsutil/copy.go @@ -7,6 +7,8 @@ import ( "os" "path" + "golang.org/x/sys/unix" + "github.com/Cloud-Foundations/Dominator/lib/wsyscall" ) @@ -76,6 +78,16 @@ func copyToWriter(writer io.Writer, filename string, reader io.Reader, } func copyTree(destDir, sourceDir string, allTypes bool, + copyFunc func(destFilename, sourceFilename string, + mode os.FileMode) error) error { + rootFd, err := openRoot(destDir) + if err != nil { + return err + } + return copyTreeWithRoot(rootFd, ".", sourceDir, allTypes, copyFunc) +} + +func copyTreeWithRoot(rootFd int, destRelDir, sourceDir string, allTypes bool, copyFunc func(destFilename, sourceFilename string, mode os.FileMode) error) error { file, err := os.Open(sourceDir) @@ -92,19 +104,25 @@ func copyTree(destDir, sourceDir string, allTypes bool, } for _, name := range names { sourceFilename := path.Join(sourceDir, name) - destFilename := path.Join(destDir, name) + destFilename := path.Join(destRelDir, name) var stat wsyscall.Stat_t if err := wsyscall.Lstat(sourceFilename, &stat); err != nil { return errors.New(sourceFilename + ": " + err.Error()) } switch stat.Mode & wsyscall.S_IFMT { case wsyscall.S_IFDIR: - if err := os.Mkdir(destFilename, DirPerms); err != nil { - if !os.IsExist(err) { + if err := secureMkdir(rootFd, destFilename, DirPerms); err != nil { + if err != unix.ENOENT || !os.IsExist(err) { return err } } - err := copyTree(destFilename, sourceFilename, allTypes, copyFunc) + err = copyTreeWithRoot( + rootFd, + destFilename, + sourceFilename, + allTypes, + copyFunc, + ) if err != nil { return err } diff --git a/lib/fsutil/virtualFileSystem_linux.go b/lib/fsutil/virtualFileSystem_linux.go new file mode 100644 index 00000000..1fedfacb --- /dev/null +++ b/lib/fsutil/virtualFileSystem_linux.go @@ -0,0 +1,71 @@ +//go:build linux + +package fsutil + +import ( + "fmt" + "os" + "path/filepath" + + "golang.org/x/sys/unix" +) + +func openRoot(path string) (int, error) { + return unix.Open(path, unix.O_DIRECTORY|unix.O_PATH|unix.O_CLOEXEC, 0) +} + +func secureMkdir(rootFd int, relPath string, mode uint32) error { + dir, file := filepath.Split(relPath) + parentFd, err := unix.Openat2(rootFd, dir, &unix.OpenHow{ + Flags: unix.O_DIRECTORY | unix.O_PATH | unix.O_CLOEXEC, + Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS, + }) + if err != nil { + return fmt.Errorf("resolving parent directory %q: %w", dir, err) + } + err = unix.Mkdirat(parentFd, file, mode) + if err != nil && err != unix.EEXIST { + return fmt.Errorf("mkdir %q:%w", relPath, err) + } + return nil +} + +func secureOpenFile(rootFd int, relPath string, mode uint32) (*os.File, error) { + fileFd, err := unix.Openat2(rootFd, relPath, &unix.OpenHow{ + Flags: unix.O_RDWR | unix.O_APPEND | unix.O_CLOEXEC, + Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS, + }) + if err == nil { + // file already exists safely. + return os.NewFile(uintptr(fileFd), relPath), nil + } + if err != unix.ENOENT { + return nil, fmt.Errorf("error resolving file %q securely: %w", + relPath, err) + } + // ENOENT encountered, could be missing file or dangling symlink, + // check if parent directory exists. + parentFd, parentErr := unix.Openat2( + rootFd, + filepath.Dir(relPath), + &unix.OpenHow{ + Flags: unix.O_DIRECTORY | unix.O_PATH | unix.O_CLOEXEC, + Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS, + }, + ) + if parentErr != nil { + return nil, fmt.Errorf("dangling symlink detected in path: %q", relPath) + } + if err := unix.Close(parentFd); err != nil { + return nil, err + } + fileFd, err = unix.Openat2(rootFd, relPath, &unix.OpenHow{ + Flags: unix.O_RDWR | unix.O_CREAT | unix.O_APPEND | unix.O_CLOEXEC, + Mode: uint64(mode), + Resolve: unix.RESOLVE_IN_ROOT | unix.RESOLVE_NO_MAGICLINKS, + }) + if err != nil { + return nil, fmt.Errorf("creating/appending file %q: %w", relPath, err) + } + return os.NewFile(uintptr(fileFd), relPath), nil +} diff --git a/lib/fsutil/virtualFileSystem_others.go b/lib/fsutil/virtualFileSystem_others.go new file mode 100644 index 00000000..4f944876 --- /dev/null +++ b/lib/fsutil/virtualFileSystem_others.go @@ -0,0 +1,20 @@ +//go:build !linux + +package fsutil + +import ( + "errors" + "os" +) + +func openRoot(path string) (int, error) { + return 0, errors.New("openRoot is supported in Linux only") +} + +func secureMkdir(rootFd int, relPath string, mode uint32) error { + return errors.New("secureMkdir is supported in Linux only") +} + +func secureOpenFile(rootFd int, relPath string, mode uint32) (*os.File, error) { + return nil, errors.New("secureOpenFile is supported in Linux only") +}