From 536ec5b8893d5c67fd3c993f99f7d7b1f0bc37a5 Mon Sep 17 00:00:00 2001 From: John Logan Date: Wed, 7 Oct 2026 12:23:15 -0700 Subject: [PATCH 1/8] FileDescriptorOps: close-on-exec descriptors and stricter relative paths First step toward making FileDescriptorOps a complete, secure-by-default set of path traversal primitives. No change to the public API surface. - Open every descriptor close-on-exec (O_CLOEXEC on each openat) and duplicate with fcntl(F_DUPFD_CLOEXEC) instead of dup(2), which clears the flag. These are directory handles, and vminitd spawns processes, so a child must not inherit a handle to an extraction root. - Close the duplicated descriptor in unlinkRecursive when fdopendir fails, which previously leaked it. - validateRelativePath now rejects absolute paths as well as paths with a ".." component. An empty path is still allowed and means "the directory itself". Previously an absolute path was silently re-anchored under the root, which hid caller bugs. - ArchiveReader now strips a leading "/" from member names explicitly, matching the default behavior of bsdtar, so extraction behavior is unchanged. Names are still reported as they appear in the archive. ArchiveReader's root and file descriptors are close-on-exec too. - Correct the doc comments: mkdir does not reject symlinks, it replaces whatever is in the way, recursively for a non-empty directory. Adds tests for absolute-path rejection, close-on-exec descriptors from mkdir and enumerate, and unlinkRecursive not following symlinks. --- .../ArchiveReader.swift | 29 +++++- .../FileDescriptorOps.swift | 60 +++++++++--- .../FileDescriptorOpsTests.swift | 91 +++++++++++++++++++ 3 files changed, 162 insertions(+), 18 deletions(-) diff --git a/Sources/ContainerizationArchive/ArchiveReader.swift b/Sources/ContainerizationArchive/ArchiveReader.swift index 1840d9a82..792c4d58c 100644 --- a/Sources/ContainerizationArchive/ArchiveReader.swift +++ b/Sources/ContainerizationArchive/ArchiveReader.swift @@ -262,22 +262,28 @@ extension ArchiveReader { /// Rejects member paths that escape the root directory or traverse /// symbolic links, and uses a "last entry wins" replacement policy /// for an existing file at a path to be extracted. + /// + /// Absolute member names are extracted beneath `directory` with the leading + /// slash removed, matching the default behavior of `bsdtar`. Member names + /// containing a `..` component are rejected and reported in the returned list. public func extractContents(to directory: URL) throws -> [String] { // Create the root directory with standard permissions // and create a FileDescriptor for secure path traversal. let fm = FileManager.default let rootFilePath = FilePath(directory.path) try fm.createDirectory(atPath: directory.path, withIntermediateDirectories: true) - let rootFileDescriptor = try FileDescriptor.open(rootFilePath, .readOnly) + let rootFileDescriptor = try FileDescriptor.open(rootFilePath, .readOnly, options: [.closeOnExec]) defer { try? rootFileDescriptor.close() } // Iterate and extract archive entries, collecting rejected paths. var foundEntry = false var rejectedPaths = [String]() for (entry, dataReader) in self.makeStreamingIterator() { - guard let memberPath = (entry.path.map { FilePath($0) }) else { + guard let entryPath = entry.path else { continue } + let originalPath = FilePath(entryPath) + let memberPath = Self.relativeMemberPath(entryPath) foundEntry = true // Try to extract the entry, catching path validation errors @@ -289,7 +295,8 @@ extension ArchiveReader { ) if !extracted { - rejectedPaths.append(memberPath.string) + // Report the name as it appears in the archive. + rejectedPaths.append(originalPath.string) } } try throwIfStreamFailed() @@ -321,6 +328,20 @@ extension ArchiveReader { throw ArchiveError.failedToExtractArchive(" \(path) not found in archive") } + /// Turns an archive member name into a path relative to the extraction root. + /// + /// Archive formats allow absolute member names. Like the default behavior of + /// `bsdtar`, this strips the leading slash so the member is extracted beneath + /// the root instead of being rejected. It does not remove `..` components: + /// `FileDescriptorOps` rejects those. + private static func relativeMemberPath(_ name: String) -> FilePath { + let path = FilePath(name) + guard path.isAbsolute else { + return path + } + return FilePath(root: nil, path.components) + } + /// Extracts a single archive entry. /// Returns false if the entry was rejected due to path validation errors. /// Throws on system errors. @@ -345,7 +366,7 @@ extension ArchiveReader { // Open file for writing using openat with O_NOFOLLOW to prevent TOC-TOU attacks let fileMode = entry.permissions & 0o777 // Mask to permission bits only - let fileFd = openat(fd.rawValue, lastComponent.string, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, fileMode) + let fileFd = openat(fd.rawValue, lastComponent.string, O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, fileMode) guard fileFd >= 0 else { throw ArchiveError.failedToExtractArchive("failed to create file: \(memberPath)") } diff --git a/Sources/ContainerizationOS/FileDescriptorOps.swift b/Sources/ContainerizationOS/FileDescriptorOps.swift index 4ade9a53f..59dc6eb6d 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps.swift @@ -18,7 +18,6 @@ import SystemPackage #if canImport(Darwin) import Darwin -private let os_dup = Darwin.dup private let os_S_IFMT = mode_t(Darwin.S_IFMT) private let os_S_IFREG = mode_t(Darwin.S_IFREG) private let os_S_IFDIR = mode_t(Darwin.S_IFDIR) @@ -26,26 +25,33 @@ private let os_S_IFLNK = mode_t(Darwin.S_IFLNK) #elseif canImport(Musl) import CSystem import Musl -private let os_dup = Musl.dup private let os_S_IFMT = Musl.S_IFMT private let os_S_IFREG = Musl.S_IFREG private let os_S_IFDIR = Musl.S_IFDIR private let os_S_IFLNK = Musl.S_IFLNK #elseif canImport(Glibc) import Glibc -private let os_dup = Glibc.dup private let os_S_IFMT = mode_t(Glibc.S_IFMT) private let os_S_IFREG = mode_t(Glibc.S_IFREG) private let os_S_IFDIR = mode_t(Glibc.S_IFDIR) private let os_S_IFLNK = mode_t(Glibc.S_IFLNK) #endif +/// Duplicates `fd` with `FD_CLOEXEC` set on the new descriptor. Plain `dup(2)` +/// clears the flag, which would let a child process inherit a handle to a +/// directory that was opened close-on-exec. +private func dupCloseOnExec(_ fd: Int32) -> Int32 { + fcntl(fd, F_DUPFD_CLOEXEC, 0) +} + /// Static utility functions for secure, symlink-safe filesystem operations /// anchored to a file descriptor. /// /// All operations use `openat`/`mkdirat`/`unlinkat` anchored to the supplied -/// file descriptor, preventing path traversal and TOCTOU races. The type is -/// never instantiated; it exists solely as a namespace. +/// file descriptor. Every path component is opened with `O_NOFOLLOW`, so a +/// symlink is never followed while walking a path, and every descriptor this +/// type opens or duplicates is close-on-exec so it is not inherited by child +/// processes. The type is never instantiated; it exists solely as a namespace. public enum FileDescriptorOps { // MARK: - Nested types @@ -85,7 +91,18 @@ public enum FileDescriptorOps { // MARK: - Public API - /// Creates a directory relative to `fd`, rejecting paths that traverse symlinks. + /// Creates a directory relative to `fd` without following symlinks. + /// + /// Each component of `relativePath` is opened with `O_NOFOLLOW|O_DIRECTORY` + /// relative to the previous one, so a symlink in the path is never followed. + /// An existing directory is reused. Anything else that is in the way, including + /// a symlink, is **removed and replaced** with a directory. Removal is + /// recursive for a non-empty directory. This replace behavior only applies to + /// the components `mkdir` visits, and a missing intermediate is created only + /// when `makeIntermediates` is true. + /// + /// An empty path runs `completion` with `fd` itself. `relativePath` must be + /// relative and must not contain a `..` component. /// /// - Parameters: /// - fd: An open file descriptor for the parent directory. @@ -113,6 +130,9 @@ public enum FileDescriptorOps { /// Recursively removes a direct child of the directory at `fd`. /// + /// Symlinks are removed, not followed. `.` and `..` are ignored. A child that + /// does not exist is not an error. + /// /// - Parameters: /// - fd: An open file descriptor for the parent directory. /// - filename: The name of the child to remove. @@ -134,7 +154,7 @@ public enum FileDescriptorOps { throw Error.systemError("file removal during file descriptor unlink", errno) } - let componentFd = openat(fd.rawValue, filename.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY) + let componentFd = openat(fd.rawValue, filename.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) guard componentFd >= 0 else { throw Error.systemError("directory open during file descriptor unlink", errno) } @@ -142,9 +162,14 @@ public enum FileDescriptorOps { defer { try? componentFileDescriptor.close() } // Open the directory stream using a duplicate fd that closedir() will close. - let ownedFd = os_dup(componentFd) + let ownedFd = dupCloseOnExec(componentFd) + guard ownedFd >= 0 else { + throw Error.systemError("directory dup during file descriptor unlink", errno) + } guard let dir = fdopendir(ownedFd) else { - throw Error.systemError("directory opendir during file descriptor unlink", errno) + let savedErrno = errno + close(ownedFd) + throw Error.systemError("directory opendir during file descriptor unlink", savedErrno) } defer { closedir(dir) } @@ -183,7 +208,7 @@ public enum FileDescriptorOps { /// for the directory that contains the entry. The last component of `path` /// is the entry's filename; together with `parentFd` it allows the body to /// open the entry via - /// `openat(parentFd.rawValue, path.lastComponent!.string, O_NOFOLLOW …)` + /// `openat(parentFd.rawValue, path.lastComponent!.string, O_NOFOLLOW | O_CLOEXEC …)` /// without reconstructing an absolute path, preserving the TOCTOU safety /// of the traversal end-to-end. `parentFd` must not be closed within the /// body call, or used after the call returns. Throw to abort. @@ -237,7 +262,7 @@ public enum FileDescriptorOps { } let childComponents = FilePath.ComponentView(relativeComponents.dropFirst()) - var componentFd = openat(fd.rawValue, currentComponent.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY) + var componentFd = openat(fd.rawValue, currentComponent.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) if componentFd < 0 { guard makeIntermediates || childComponents.isEmpty else { throw Error.invalidPathComponent @@ -250,7 +275,7 @@ public enum FileDescriptorOps { throw Error.systemError("directory creation during file descriptor mkdir", errno) } - componentFd = openat(fd.rawValue, currentComponent.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY) + componentFd = openat(fd.rawValue, currentComponent.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) guard componentFd >= 0 else { throw Error.systemError("directory open during file descriptor mkdir", errno) } @@ -276,7 +301,7 @@ public enum FileDescriptorOps { ) throws { // fdopendir takes ownership of the fd passed to it and closes it via // closedir. Duplicate so the caller's fd remains open. - let dupFd = os_dup(fd.rawValue) + let dupFd = dupCloseOnExec(fd.rawValue) guard dupFd >= 0 else { throw Error.systemError("dup during file descriptor enumerate", errno) } @@ -309,7 +334,7 @@ public enum FileDescriptorOps { // Open the child directory with O_NOFOLLOW to guarantee we are // entering a real directory and not a symlink that was swapped in // between readdir and here. - let childFd = openat(fd.rawValue, name, O_NOFOLLOW | O_RDONLY | O_DIRECTORY) + let childFd = openat(fd.rawValue, name, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) guard childFd >= 0 else { throw Error.systemError("openat during file descriptor enumerate", errno) } @@ -338,7 +363,14 @@ public enum FileDescriptorOps { } } + /// Rejects anything that is not a plain relative path: an absolute path, or a + /// path with a `..` component. An empty path is allowed and means "the + /// directory itself". Callers that accept untrusted names, such as archive + /// member names, decide whether to strip or reject before calling in. private static func validateRelativePath(_ path: FilePath) throws { + guard !path.isAbsolute else { + throw Error.invalidRelativePath + } guard !(path.components.contains { $0 == ".." }) else { throw Error.invalidRelativePath } diff --git a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift index 6ac47410c..b907e2733 100644 --- a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift +++ b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift @@ -569,6 +569,97 @@ struct FileDescriptorPathSecureTests { #expect(FileManager.default.fileExists(atPath: tempPath.string)) } + @Test("Test mkdir rejects an absolute path and creates nothing") + func testMkdirRejectsAbsolutePath() throws { + let basePath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: basePath.string) } + + let rootPath = basePath.appending("root") + let targetPath = basePath.appending("absolute-target") + try FileManager.default.createDirectory(atPath: rootPath.string, withIntermediateDirectories: false) + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + for makeIntermediates in [false, true] { + var completionCalled = false + #expect(throws: FileDescriptorOps.Error.invalidRelativePath) { + try FileDescriptorOps.mkdir(rootFd, targetPath.appending("child"), makeIntermediates: makeIntermediates) { _ in + completionCalled = true + } + } + #expect(!completionCalled) + } + + // Nothing is created at the absolute location, or re-anchored under the root. + #expect(!FileManager.default.fileExists(atPath: targetPath.string)) + #expect(try FileManager.default.contentsOfDirectory(atPath: rootPath.string).isEmpty) + } + + @Test("Test directory descriptors opened by mkdir are close-on-exec") + func testMkdirDescriptorsAreCloseOnExec() throws { + let rootPath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: rootPath.string) } + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + var completionCalled = false + try FileDescriptorOps.mkdir(rootFd, FilePath("a/b/c"), makeIntermediates: true) { dirFd in + completionCalled = true + let flags = fcntl(dirFd.rawValue, F_GETFD) + #expect(flags >= 0) + #expect(flags & FD_CLOEXEC != 0, "descriptor handed to completion must be close-on-exec") + } + #expect(completionCalled) + } + + @Test("Test directory descriptors opened by enumerate are close-on-exec") + func testEnumerateDescriptorsAreCloseOnExec() throws { + let rootPath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: rootPath.string) } + try FileManager.default.createDirectory( + atPath: rootPath.appending("a/b").string, withIntermediateDirectories: true) + try Data("x".utf8).write(to: URL(fileURLWithPath: rootPath.appending("a/b/file.txt").string)) + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + // Entries below the top level are reported with a descriptor that enumerate opened itself. + var checked = 0 + try FileDescriptorOps.enumerate(rootFd) { path, _, parentFd in + guard path.components.count > 1 else { return } + let flags = fcntl(parentFd.rawValue, F_GETFD) + #expect(flags >= 0) + #expect(flags & FD_CLOEXEC != 0, "descriptor for \(path.string) must be close-on-exec") + checked += 1 + } + #expect(checked == 2) // a/b and a/b/file.txt + } + + @Test("Test unlinkRecursive removes a nested tree without following symlinks") + func testUnlinkRecursiveDoesNotFollowSymlinks() throws { + let basePath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: basePath.string) } + + let rootPath = basePath.appending("root") + let outsidePath = basePath.appending("outside") + try FileManager.default.createDirectory(atPath: rootPath.appending("tree/sub").string, withIntermediateDirectories: true) + try FileManager.default.createDirectory(atPath: outsidePath.string, withIntermediateDirectories: false) + try Data("keep".utf8).write(to: URL(fileURLWithPath: outsidePath.appending("keep.txt").string)) + try FileManager.default.createSymbolicLink( + atPath: rootPath.appending("tree/sub/link").string, withDestinationPath: outsidePath.string) + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + let name = FilePath.Component("tree") + try FileDescriptorOps.unlinkRecursive(rootFd, filename: name) + + #expect(!FileManager.default.fileExists(atPath: rootPath.appending("tree").string)) + #expect(FileManager.default.fileExists(atPath: outsidePath.appending("keep.txt").string)) + } + @Test("Test mkdir with empty path calls completion with parent") func testMkdirEmptyPath() throws { let rootPath = try createTempDirectory() From a2a0e50877ed5540425e7f8e06d02fecdccfa0bc Mon Sep 17 00:00:00 2001 From: John Logan Date: Wed, 7 Oct 2026 12:41:30 -0700 Subject: [PATCH 2/8] FileDescriptorOps: split primitives from composites Second step toward a complete, secure-by-default set of path traversal primitives. Breaking change to the FileDescriptorOps.Error enum, which gains cases. Primitives (FileDescriptorOps.swift). Each does one thing relative to a directory descriptor, never follows a symlink, and never removes or replaces anything it was not asked to: - entryType: the kind of an entry, via fstatat(AT_SYMLINK_NOFOLLOW). - openDirectory: open an existing directory with O_NOFOLLOW. Throws notFound, cannotFollowSymlink, or conflict(type) so callers do not depend on platform-specific errno values for "a symlink is in the way". - makeDirectory: mkdirat, throwing alreadyExists on any existing entry. - unlink: remove a non-directory only. New Error cases: notFound, alreadyExists, conflict(EntryType). cannotFollowSymlink is now actually thrown. Composites (FileDescriptorOps+Composite.swift). mkdir moves here and is rebuilt from the public primitives only, so a path walk is written once. It behaves as before: a file or symlink in the way of a directory it needs is replaced, and a symlink is removed and never followed. One change: it never removes a directory. Previously any failure to open a component other than ENOENT, such as EACCES on a directory without permissions, led to a recursive delete of that directory. It now fails with a system error and leaves the directory in place. ArchiveReader only needs to handle the new error cases. Adds tests for each primitive, for mkdir replacing a file or symlink without writing through it, for mkdir without makeIntermediates leaving what is in the way alone, and for an unopenable directory not being removed. --- .../ArchiveReader.swift | 7 +- .../FileDescriptorOps+Composite.swift | 122 ++++++++++ .../FileDescriptorOps.swift | 191 +++++++++------- .../FileDescriptorOpsTests.swift | 215 ++++++++++++++++++ 4 files changed, 452 insertions(+), 83 deletions(-) create mode 100644 Sources/ContainerizationOS/FileDescriptorOps+Composite.swift diff --git a/Sources/ContainerizationArchive/ArchiveReader.swift b/Sources/ContainerizationArchive/ArchiveReader.swift index 792c4d58c..7f2c161dd 100644 --- a/Sources/ContainerizationArchive/ArchiveReader.swift +++ b/Sources/ContainerizationArchive/ArchiveReader.swift @@ -402,10 +402,11 @@ extension ArchiveReader { } catch let error as FileDescriptorOps.Error { // Just reject path validation errors, don't fail the extraction switch error { - case .systemError: - // Fail for system errors + case .systemError, .notFound, .alreadyExists: + // Fail for system errors, and for entries that appeared or disappeared + // underneath us, which means something else is modifying the tree. throw error - case .invalidRelativePath, .invalidPathComponent, .cannotFollowSymlink: + case .invalidRelativePath, .invalidPathComponent, .cannotFollowSymlink, .conflict: return false } } diff --git a/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift new file mode 100644 index 000000000..34280a87a --- /dev/null +++ b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift @@ -0,0 +1,122 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the Containerization project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import SystemPackage + +// Composite operations. +// +// These combine the primitives in `FileDescriptorOps.swift` into sequences that +// several callers need and that are easy to get wrong. They never follow a +// symlink, and their documentation says what, if anything, they replace. +// +// Only the primitives may be used here. Do not call system calls directly, so +// that every path walk goes through code that is written and tested once. + +extension FileDescriptorOps { + /// Opens, and where needed creates, the directory at `relativePath` below `fd`, + /// then runs `completion` with a descriptor for it. + /// + /// Each component is opened relative to the previous one without following + /// symlinks, so a symlink in the path can never redirect the walk. Existing + /// directories are reused. The descriptor passed to `completion` is + /// close-on-exec and is closed when `completion` returns. + /// + /// By default only the last component is created. Pass `makeIntermediates` + /// to create missing parents too. + /// + /// **This replaces what is in the way.** A file, a symlink, or anything else that is not a + /// directory, in the place of a directory this call needs, is removed and replaced by one. A + /// symlink is removed and not followed. A directory is never removed, even one that cannot + /// be opened. + /// + /// An empty `relativePath` runs `completion` with `fd` itself. + /// + /// - Parameters: + /// - fd: An open file descriptor for the directory to start from. + /// - relativePath: The directory to open or create. It must be relative and must not contain a `..` component. + /// - permissions: The permissions for each directory this call creates (default 0o755). + /// - makeIntermediates: Also create missing intermediate directories. + /// - completion: A function that operates on the directory descriptor. + /// - Throws: ``Error/invalidRelativePath`` for an absolute path or one containing `..`; + /// ``Error/invalidPathComponent`` if an intermediate component is missing, or is not a + /// directory, and `makeIntermediates` is false; and ``Error/systemError(_:_:)`` for anything + /// else. Errors thrown by `completion` are propagated. + public static func mkdir( + _ fd: FileDescriptor, + _ relativePath: FilePath, + permissions: FilePermissions? = nil, + makeIntermediates: Bool = false, + completion: (FileDescriptor) throws -> Void = { _ in } + ) throws { + try validateRelativePath(relativePath) + + let components = Array(relativePath.components) + var current = fd + var ownsCurrent = false + defer { + if ownsCurrent { + try? current.close() + } + } + + for (index, component) in components.enumerated() { + let isLast = index == components.count - 1 + let next = try openOrCreateDirectory( + current, + component, + permissions: permissions, + allowCreate: makeIntermediates || isLast + ) + if ownsCurrent { + try? current.close() + } + current = next + ownsCurrent = true + } + + try completion(current) + } + + private static func openOrCreateDirectory( + _ parent: FileDescriptor, + _ name: FilePath.Component, + permissions: FilePermissions?, + allowCreate: Bool + ) throws -> FileDescriptor { + do { + return try openDirectory(parent, name) + } catch let error as Error { + switch error { + case .notFound: + guard allowCreate else { + throw Error.invalidPathComponent + } + case .cannotFollowSymlink, .conflict: + // Something that is not a directory is in the way. Replace it. This removes a symlink + // and not its target, and it never removes a directory. + guard allowCreate else { + throw Error.invalidPathComponent + } + try unlink(parent, name) + default: + throw error + } + } + + try makeDirectory(parent, name, permissions: permissions) + return try openDirectory(parent, name) + } +} diff --git a/Sources/ContainerizationOS/FileDescriptorOps.swift b/Sources/ContainerizationOS/FileDescriptorOps.swift index 59dc6eb6d..ed7dffa00 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps.swift @@ -57,9 +57,18 @@ public enum FileDescriptorOps { // MARK: - Nested types public enum Error: Swift.Error, CustomStringConvertible, Equatable { + /// The path is not a plain relative path: it is absolute, or contains a `..` component. case invalidRelativePath + /// An intermediate path component is missing or is not a directory. case invalidPathComponent + /// The entry is a symlink, which these operations never follow. case cannotFollowSymlink + /// The entry does not exist. + case notFound + /// The entry already exists. + case alreadyExists + /// A non-directory entry of the given type is in the way of an operation that needs a directory. + case conflict(EntryType) case systemError(String, Int32) public var description: String { @@ -70,6 +79,12 @@ public enum FileDescriptorOps { return "an intermediate path component is missing or is not a directory" case .cannotFollowSymlink: return "cannot follow a symlink in a file descriptor operation" + case .notFound: + return "no such entry in file descriptor operation" + case .alreadyExists: + return "entry already exists in file descriptor operation" + case .conflict(let type): + return "a \(type) entry is in the way of a file descriptor operation" case .systemError(let operation, let err): return "\(operation) returned error: \(err)" } @@ -89,43 +104,100 @@ public enum FileDescriptorOps { case other } - // MARK: - Public API + // MARK: - Primitives + // + // Each primitive does one thing relative to a directory descriptor, never + // follows a symlink, and never removes or replaces anything it was not asked + // to. Operations that combine primitives, or that have to choose what to do + // when something is in the way, live in `FileDescriptorOps+Composite.swift`. - /// Creates a directory relative to `fd` without following symlinks. + /// Returns the type of the entry `name` in the directory `fd`, without following + /// a symlink, or `nil` if there is no such entry. /// - /// Each component of `relativePath` is opened with `O_NOFOLLOW|O_DIRECTORY` - /// relative to the previous one, so a symlink in the path is never followed. - /// An existing directory is reused. Anything else that is in the way, including - /// a symlink, is **removed and replaced** with a directory. Removal is - /// recursive for a non-empty directory. This replace behavior only applies to - /// the components `mkdir` visits, and a missing intermediate is created only - /// when `makeIntermediates` is true. + /// - Parameters: + /// - fd: An open file descriptor for a directory. + /// - name: The name of a direct child of that directory. + /// - Throws: `FileDescriptorOps.Error.systemError` if the entry cannot be inspected. + public static func entryType(_ fd: FileDescriptor, _ name: FilePath.Component) throws -> EntryType? { + var stbuf = stat() + guard fstatat(fd.rawValue, name.string, &stbuf, AT_SYMLINK_NOFOLLOW) == 0 else { + if errno == ENOENT { + return nil + } + throw Error.systemError("stat during file descriptor entry type lookup", errno) + } + return entryType(forMode: stbuf.st_mode) + } + + /// Opens the existing directory `name` in the directory `fd`, without following + /// a symlink. The returned descriptor is close-on-exec and the caller must close it. /// - /// An empty path runs `completion` with `fd` itself. `relativePath` must be - /// relative and must not contain a `..` component. + /// - Parameters: + /// - fd: An open file descriptor for the parent directory. + /// - name: The name of a direct child of that directory. + /// - Throws: ``Error/notFound`` if there is no such entry, ``Error/cannotFollowSymlink`` + /// if it is a symlink, ``Error/conflict(_:)`` if it is some other kind of + /// non-directory, and ``Error/systemError(_:_:)`` for anything else. + public static func openDirectory(_ fd: FileDescriptor, _ name: FilePath.Component) throws -> FileDescriptor { + let newFd = openat(fd.rawValue, name.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) + if newFd >= 0 { + return FileDescriptor(rawValue: newFd) + } + + // The failure code for "a symlink or file is in the way" differs between + // platforms, so look at what is actually there. + let openErrno = errno + switch try entryType(fd, name) { + case nil: + throw Error.notFound + case .symlink: + throw Error.cannotFollowSymlink + case .regular: + throw Error.conflict(.regular) + case .other: + throw Error.conflict(.other) + case .directory: + throw Error.systemError("directory open during file descriptor open", openErrno) + } + } + + /// Creates the directory `name` in the directory `fd`. /// /// - Parameters: /// - fd: An open file descriptor for the parent directory. - /// - relativePath: The path to create, relative to `fd`. + /// - name: The name of the directory to create. /// - permissions: The permissions to give the directory (default 0o755). - /// - makeIntermediates: Create or replace intermediate components as needed. - /// - completion: A function that operates on the new directory fd. - /// - Throws: `FileDescriptorOps.Error` if path validation or system errors occur. - public static func mkdir( + /// - Throws: ``Error/alreadyExists`` if anything with that name exists, including a + /// symlink, and ``Error/systemError(_:_:)`` for anything else. + public static func makeDirectory( _ fd: FileDescriptor, - _ relativePath: FilePath, - permissions: FilePermissions? = nil, - makeIntermediates: Bool = false, - completion: (FileDescriptor) throws -> Void = { _ in } + _ name: FilePath.Component, + permissions: FilePermissions? = nil ) throws { - try validateRelativePath(relativePath) - try mkdir( - fd, - relativePath.components, - permissions: permissions, - makeIntermediates: makeIntermediates, - completion: completion - ) + guard mkdirat(fd.rawValue, name.string, permissions?.rawValue ?? 0o755) == 0 else { + if errno == EEXIST { + throw Error.alreadyExists + } + throw Error.systemError("directory creation during file descriptor mkdir", errno) + } + } + + /// Removes the non-directory entry `name` from the directory `fd`. A symlink is + /// removed, not followed. This never removes a directory: use + /// ``unlinkRecursive(_:filename:)`` for that. + /// + /// - Parameters: + /// - fd: An open file descriptor for the parent directory. + /// - name: The name of the entry to remove. + /// - Throws: ``Error/notFound`` if there is no such entry, and + /// ``Error/systemError(_:_:)`` for anything else, including when the entry is a directory. + public static func unlink(_ fd: FileDescriptor, _ name: FilePath.Component) throws { + guard unlinkat(fd.rawValue, name.string, 0) == 0 else { + if errno == ENOENT { + throw Error.notFound + } + throw Error.systemError("entry removal during file descriptor unlink", errno) + } } /// Recursively removes a direct child of the directory at `fd`. @@ -249,51 +321,6 @@ public enum FileDescriptorOps { // MARK: - Private helpers - private static func mkdir( - _ fd: FileDescriptor, - _ relativeComponents: FilePath.ComponentView, - permissions: FilePermissions? = nil, - makeIntermediates: Bool, - completion: (FileDescriptor) throws -> Void - ) throws { - guard let currentComponent = relativeComponents.first else { - try completion(fd) - return - } - let childComponents = FilePath.ComponentView(relativeComponents.dropFirst()) - - var componentFd = openat(fd.rawValue, currentComponent.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) - if componentFd < 0 { - guard makeIntermediates || childComponents.isEmpty else { - throw Error.invalidPathComponent - } - if errno != ENOENT { - try unlinkRecursive(fd, filename: currentComponent) - } - - guard mkdirat(fd.rawValue, currentComponent.string, permissions?.rawValue ?? 0o755) == 0 else { - throw Error.systemError("directory creation during file descriptor mkdir", errno) - } - - componentFd = openat(fd.rawValue, currentComponent.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) - guard componentFd >= 0 else { - throw Error.systemError("directory open during file descriptor mkdir", errno) - } - } - - let componentFileDescriptor = FileDescriptor(rawValue: componentFd) - defer { try? componentFileDescriptor.close() } - - guard !childComponents.isEmpty else { - try completion(componentFileDescriptor) - return - } - - try mkdir( - componentFileDescriptor, childComponents, - permissions: permissions, makeIntermediates: makeIntermediates, completion: completion) - } - private static func enumerateHelper( _ fd: FileDescriptor, relativePath: FilePath, @@ -353,12 +380,16 @@ public enum FileDescriptorOps { // Some filesystems (NFS, ext2/3) report DT_UNKNOWN; fall back to fstatat. var stbuf = stat() guard fstatat(parentFd, name, &stbuf, AT_SYMLINK_NOFOLLOW) == 0 else { return .other } - switch stbuf.st_mode & os_S_IFMT { - case os_S_IFREG: return .regular - case os_S_IFDIR: return .directory - case os_S_IFLNK: return .symlink - default: return .other - } + return entryType(forMode: stbuf.st_mode) + default: return .other + } + } + + private static func entryType(forMode mode: mode_t) -> EntryType { + switch mode & os_S_IFMT { + case os_S_IFREG: return .regular + case os_S_IFDIR: return .directory + case os_S_IFLNK: return .symlink default: return .other } } @@ -367,7 +398,7 @@ public enum FileDescriptorOps { /// path with a `..` component. An empty path is allowed and means "the /// directory itself". Callers that accept untrusted names, such as archive /// member names, decide whether to strip or reject before calling in. - private static func validateRelativePath(_ path: FilePath) throws { + static func validateRelativePath(_ path: FilePath) throws { guard !path.isAbsolute else { throw Error.invalidRelativePath } diff --git a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift index b907e2733..4392211f3 100644 --- a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift +++ b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift @@ -569,6 +569,221 @@ struct FileDescriptorPathSecureTests { #expect(FileManager.default.fileExists(atPath: tempPath.string)) } + // MARK: - Primitives + + @Test("Test entryType reports each kind without following symlinks") + func testEntryType() throws { + let rootPath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: rootPath.string) } + try Data("x".utf8).write(to: URL(fileURLWithPath: rootPath.appending("file").string)) + try FileManager.default.createDirectory(atPath: rootPath.appending("dir").string, withIntermediateDirectories: false) + try FileManager.default.createSymbolicLink(atPath: rootPath.appending("link").string, withDestinationPath: "dir") + #expect(mkfifo(rootPath.appending("fifo").string, 0o644) == 0) + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + let file: FileDescriptorOps.EntryType? = try FileDescriptorOps.entryType(rootFd, "file") + let dir: FileDescriptorOps.EntryType? = try FileDescriptorOps.entryType(rootFd, "dir") + let link: FileDescriptorOps.EntryType? = try FileDescriptorOps.entryType(rootFd, "link") + let fifo: FileDescriptorOps.EntryType? = try FileDescriptorOps.entryType(rootFd, "fifo") + let missing: FileDescriptorOps.EntryType? = try FileDescriptorOps.entryType(rootFd, "missing") + #expect(file == .regular) + #expect(dir == .directory) + #expect(link == .symlink, "a symlink to a directory is reported as a symlink") + #expect(fifo == .other) + #expect(missing == nil) + } + + @Test("Test openDirectory opens directories and classifies what is in the way") + func testOpenDirectory() throws { + let basePath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: basePath.string) } + let rootPath = basePath.appending("root") + try FileManager.default.createDirectory(atPath: rootPath.appending("dir").string, withIntermediateDirectories: true) + try FileManager.default.createDirectory(atPath: basePath.appending("outside").string, withIntermediateDirectories: false) + try Data("x".utf8).write(to: URL(fileURLWithPath: rootPath.appending("file").string)) + try FileManager.default.createSymbolicLink(atPath: rootPath.appending("link").string, withDestinationPath: "../outside") + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + let opened = try FileDescriptorOps.openDirectory(rootFd, "dir") + defer { try? opened.close() } + #expect(fcntl(opened.rawValue, F_GETFD) & FD_CLOEXEC != 0) + + #expect(throws: FileDescriptorOps.Error.notFound) { _ = try FileDescriptorOps.openDirectory(rootFd, "missing") } + #expect(throws: FileDescriptorOps.Error.cannotFollowSymlink) { _ = try FileDescriptorOps.openDirectory(rootFd, "link") } + #expect(throws: FileDescriptorOps.Error.conflict(.regular)) { _ = try FileDescriptorOps.openDirectory(rootFd, "file") } + } + + @Test("Test makeDirectory creates a directory and refuses to touch anything that exists") + func testMakeDirectory() throws { + let basePath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: basePath.string) } + let rootPath = basePath.appending("root") + try FileManager.default.createDirectory(atPath: rootPath.appending("dir").string, withIntermediateDirectories: true) + try FileManager.default.createDirectory(atPath: basePath.appending("outside").string, withIntermediateDirectories: false) + try Data("x".utf8).write(to: URL(fileURLWithPath: rootPath.appending("file").string)) + try FileManager.default.createSymbolicLink(atPath: rootPath.appending("link").string, withDestinationPath: "../outside") + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + try FileDescriptorOps.makeDirectory(rootFd, "new", permissions: FilePermissions(rawValue: 0o700)) + var isDirectory: ObjCBool = false + #expect(FileManager.default.fileExists(atPath: rootPath.appending("new").string, isDirectory: &isDirectory) && isDirectory.boolValue) + + for name in ["dir", "file", "link"] as [FilePath.Component] { + #expect(throws: FileDescriptorOps.Error.alreadyExists) { try FileDescriptorOps.makeDirectory(rootFd, name) } + } + #expect(try FileManager.default.destinationOfSymbolicLink(atPath: rootPath.appending("link").string) == "../outside") + #expect(try FileManager.default.contentsOfDirectory(atPath: basePath.appending("outside").string).isEmpty) + } + + @Test("Test unlink removes files and symlinks but never directories or symlink targets") + func testUnlink() throws { + let basePath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: basePath.string) } + let rootPath = basePath.appending("root") + let outsidePath = basePath.appending("outside") + try FileManager.default.createDirectory(atPath: rootPath.appending("dir").string, withIntermediateDirectories: true) + try FileManager.default.createDirectory(atPath: outsidePath.string, withIntermediateDirectories: false) + try Data("keep".utf8).write(to: URL(fileURLWithPath: outsidePath.appending("target").string)) + try Data("x".utf8).write(to: URL(fileURLWithPath: rootPath.appending("file").string)) + try FileManager.default.createSymbolicLink(atPath: rootPath.appending("link").string, withDestinationPath: "../outside/target") + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + try FileDescriptorOps.unlink(rootFd, "file") + try FileDescriptorOps.unlink(rootFd, "link") + #expect(!FileManager.default.fileExists(atPath: rootPath.appending("file").string)) + #expect(try FileManager.default.contentsOfDirectory(atPath: rootPath.string) == ["dir"]) + #expect(FileManager.default.fileExists(atPath: outsidePath.appending("target").string), "the target of a removed symlink must be untouched") + + #expect(throws: FileDescriptorOps.Error.notFound) { try FileDescriptorOps.unlink(rootFd, "missing") } + #expect(throws: (any Swift.Error).self) { try FileDescriptorOps.unlink(rootFd, "dir") } + #expect(FileManager.default.fileExists(atPath: rootPath.appending("dir").string), "unlink must not remove a directory") + } + + // MARK: - mkdir replaces what is in the way + + @Test("Test mkdir replaces a file or symlink that is in the way, and never writes through a symlink") + func testMkdirReplacesWhatIsInTheWay() throws { + struct Case { + let name: String + let path: String + let setup: (_ root: FilePath, _ outside: FilePath) throws -> Void + } + let writeFile: (FilePath, FilePath) throws -> Void = { root, _ in + try Data("old".utf8).write(to: URL(fileURLWithPath: root.appending("f").string)) + } + let writeLink: (FilePath, FilePath) throws -> Void = { root, outside in + try FileManager.default.createSymbolicLink(atPath: root.appending("l").string, withDestinationPath: outside.string) + } + let cases = [ + Case(name: "file at the last component", path: "f", setup: writeFile), + Case(name: "symlink at the last component", path: "l", setup: writeLink), + Case(name: "file at an intermediate component", path: "f/x", setup: writeFile), + Case(name: "symlink at an intermediate component", path: "l/x", setup: writeLink), + ] + + for testCase in cases { + let basePath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: basePath.string) } + let rootPath = basePath.appending("root") + let outsidePath = basePath.appending("outside") + try FileManager.default.createDirectory(atPath: rootPath.string, withIntermediateDirectories: false) + try FileManager.default.createDirectory(atPath: outsidePath.string, withIntermediateDirectories: false) + try testCase.setup(rootPath, outsidePath) + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + var completionCalled = false + try FileDescriptorOps.mkdir(rootFd, FilePath(testCase.path), makeIntermediates: true) { _ in + completionCalled = true + } + + #expect(completionCalled, "\(testCase.name)") + let first = String(testCase.path.split(separator: "/")[0]) + let kind = try FileManager.default.attributesOfItem(atPath: rootPath.appending(first).string)[.type] as? FileAttributeType + #expect(kind == .typeDirectory, "\(testCase.name): what was in the way must be replaced by a directory") + #expect(try FileManager.default.contentsOfDirectory(atPath: outsidePath.string).isEmpty, "\(testCase.name): nothing may be created outside the root") + } + } + + @Test("Test mkdir without makeIntermediates leaves what is in the way of an intermediate directory alone") + func testMkdirWithoutIntermediatesLeavesWhatIsInTheWay() throws { + let basePath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: basePath.string) } + let rootPath = basePath.appending("root") + let outsidePath = basePath.appending("outside") + try FileManager.default.createDirectory(atPath: rootPath.string, withIntermediateDirectories: false) + try FileManager.default.createDirectory(atPath: outsidePath.string, withIntermediateDirectories: false) + try Data("old".utf8).write(to: URL(fileURLWithPath: rootPath.appending("f").string)) + try FileManager.default.createSymbolicLink(atPath: rootPath.appending("l").string, withDestinationPath: outsidePath.string) + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + for path in ["f/x", "l/x"] { + #expect(throws: FileDescriptorOps.Error.invalidPathComponent, "\(path)") { + try FileDescriptorOps.mkdir(rootFd, FilePath(path)) + } + } + + #expect(try String(contentsOfFile: rootPath.appending("f").string, encoding: .utf8) == "old") + #expect(try FileManager.default.destinationOfSymbolicLink(atPath: rootPath.appending("l").string) == outsidePath.string) + #expect(try FileManager.default.contentsOfDirectory(atPath: outsidePath.string).isEmpty) + } + + @Test("Test mkdir replaces a symlink with a directory and never writes through it") + func testMkdirReplacesSymlinkAndNeverWritesThrough() throws { + let basePath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: basePath.string) } + let rootPath = basePath.appending("root") + let outsidePath = basePath.appending("outside") + try FileManager.default.createDirectory(atPath: rootPath.string, withIntermediateDirectories: false) + try FileManager.default.createDirectory(atPath: outsidePath.string, withIntermediateDirectories: false) + try FileManager.default.createSymbolicLink(atPath: rootPath.appending("l").string, withDestinationPath: outsidePath.string) + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + try FileDescriptorOps.mkdir(rootFd, FilePath("l/x"), makeIntermediates: true) { dirFd in + let fd = openat(dirFd.rawValue, "stub", O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, 0o644) + #expect(fd >= 0) + if fd >= 0 { close(fd) } + } + + #expect(FileManager.default.fileExists(atPath: rootPath.appending("l/x/stub").string)) + #expect(try FileManager.default.attributesOfItem(atPath: rootPath.appending("l").string)[.type] as? FileAttributeType == .typeDirectory) + #expect(try FileManager.default.contentsOfDirectory(atPath: outsidePath.string).isEmpty, "nothing may be written through the replaced symlink") + } + + @Test("Test mkdir never removes a directory it cannot open", .enabled(if: geteuid() != 0)) + func testMkdirDoesNotRemoveUnopenableDirectory() throws { + let rootPath = try createTempDirectory() + defer { try? FileManager.default.removeItem(atPath: rootPath.string) } + let lockedPath = rootPath.appending("locked") + try FileManager.default.createDirectory(atPath: lockedPath.string, withIntermediateDirectories: false) + try Data("keep".utf8).write(to: URL(fileURLWithPath: lockedPath.appending("keep").string)) + try FileManager.default.setAttributes([.posixPermissions: 0o000], ofItemAtPath: lockedPath.string) + defer { try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: lockedPath.string) } + + let rootFd = try FileDescriptor.open(rootPath, .readOnly, options: [.directory]) + defer { try? rootFd.close() } + + #expect(throws: (any Swift.Error).self) { + try FileDescriptorOps.mkdir(rootFd, FilePath("locked/child"), makeIntermediates: true) + } + + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: lockedPath.string) + #expect(FileManager.default.fileExists(atPath: lockedPath.appending("keep").string), "a directory that cannot be opened must not be removed") + } + @Test("Test mkdir rejects an absolute path and creates nothing") func testMkdirRejectsAbsolutePath() throws { let basePath = try createTempDirectory() From b8fa33d54d42464dd47a1ce7b3e650491992fb48 Mon Sep 17 00:00:00 2001 From: John Logan Date: Wed, 7 Oct 2026 13:02:16 -0700 Subject: [PATCH 3/8] FileDescriptorOps: add leaf primitives and read-beneath composites Third step toward a complete, secure-by-default set of path traversal primitives. Purely additive: no change to existing behavior. Primitives (FileDescriptorOps.swift), none of which follows a symlink: - status(of:) and status(_:_:): metadata via fstat / fstatat, returned as a new FileStatus (type, permissions, size, uid, gid, mtime). - openFile: open a regular file for reading with O_NOFOLLOW. The file is opened non-blocking and checked with fstat once open, so a FIFO, device or socket in its place is refused instead of hanging the caller. - createFile: create a file exclusively (O_CREAT|O_EXCL|O_NOFOLLOW), so it never writes through or over anything that exists. - makeSymlink and readSymlink: create a symlink without resolving its target, and read one back without following it. Error.conflict now also covers "this entry is not the type the operation needs", for example a directory where a file is read. Composites (FileDescriptorOps+Composite.swift), built from the primitives only: - openFile(_:relativePath:): walk the parents without following symlinks and open the file, so a symlink anywhere in the path, including the last component, is refused, and swapping a component for a symlink while this runs cannot redirect the open. This is the call to use when opening a file named by an untrusted path. - status(_:relativePath:): the same walk, returning nil if the entry or a parent does not exist. Adds tests for each primitive and composite, including a concurrency test that swaps both the last component and the parent directory between real entries and symlinks to an outside file while another thread opens the path, and asserts the outside file is never read. --- .../FileDescriptorOps+Composite.swift | 78 ++++++ .../FileDescriptorOps.swift | 192 ++++++++++++- .../FileDescriptorOpsTests.swift | 252 ++++++++++++++++++ 3 files changed, 521 insertions(+), 1 deletion(-) diff --git a/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift index 34280a87a..9ab9051f3 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift @@ -90,6 +90,84 @@ extension FileDescriptorOps { try completion(current) } + /// Opens the existing regular file at `relativePath` below `fd` for reading. + /// + /// Every component is opened relative to the previous one without following + /// symlinks, and the file is checked after it is open. A symlink anywhere in the + /// path, including the last component, is refused, and a swap of any component + /// for a symlink while this runs cannot redirect the open. The returned descriptor + /// is close-on-exec and the caller must close it. + /// + /// - Parameters: + /// - fd: An open file descriptor for the directory to start from. + /// - relativePath: The file to open. It must be relative, must not contain a `..` component, and must name something. + /// - Throws: ``Error/invalidRelativePath`` for an absolute path, one containing `..`, or an empty path; + /// ``Error/notFound`` if the file or a parent does not exist; ``Error/cannotFollowSymlink`` if a symlink + /// is in the path; ``Error/conflict(_:)`` if a parent is not a directory or the file is not a regular + /// file; and ``Error/systemError(_:_:)`` for anything else. + public static func openFile(_ fd: FileDescriptor, relativePath: FilePath) throws -> FileDescriptor { + try validateRelativePath(relativePath) + var components = Array(relativePath.components) + guard let name = components.popLast() else { + throw Error.invalidRelativePath + } + return try withDirectory(fd, components) { parent in + try openFile(parent, name) + } + } + + /// Returns the metadata of the entry at `relativePath` below `fd`, without following + /// symlinks, or `nil` if the entry or one of its parents does not exist. For a symlink this + /// is the metadata of the link itself. An empty path describes `fd` itself. + /// + /// A symlink in a parent position is refused, so the answer always describes an entry + /// that is really beneath `fd`. + /// + /// - Throws: ``Error/invalidRelativePath`` for an absolute path or one containing `..`; + /// ``Error/cannotFollowSymlink`` if a symlink is in a parent position; ``Error/conflict(_:)`` if a + /// parent is not a directory; and ``Error/systemError(_:_:)`` for anything else. + public static func status(_ fd: FileDescriptor, relativePath: FilePath) throws -> FileStatus? { + try validateRelativePath(relativePath) + var components = Array(relativePath.components) + guard let name = components.popLast() else { + return try status(of: fd) + } + do { + return try withDirectory(fd, components) { parent in + try status(parent, name) + } + } catch Error.notFound { + return nil + } + } + + /// Runs `body` with a descriptor for the directory reached by walking `components` from + /// `fd`, without following symlinks. With no components, `body` gets `fd` itself. + private static func withDirectory( + _ fd: FileDescriptor, + _ components: [FilePath.Component], + _ body: (FileDescriptor) throws -> T + ) throws -> T { + var current = fd + var ownsCurrent = false + defer { + if ownsCurrent { + try? current.close() + } + } + + for component in components { + let next = try openDirectory(current, component) + if ownsCurrent { + try? current.close() + } + current = next + ownsCurrent = true + } + + return try body(current) + } + private static func openOrCreateDirectory( _ parent: FileDescriptor, _ name: FilePath.Component, diff --git a/Sources/ContainerizationOS/FileDescriptorOps.swift b/Sources/ContainerizationOS/FileDescriptorOps.swift index ed7dffa00..6d0883862 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps.swift @@ -67,7 +67,8 @@ public enum FileDescriptorOps { case notFound /// The entry already exists. case alreadyExists - /// A non-directory entry of the given type is in the way of an operation that needs a directory. + /// The entry has a type the operation cannot use, for example a non-directory + /// where a directory is needed, or anything but a regular file where a file is read. case conflict(EntryType) case systemError(String, Int32) @@ -104,6 +105,18 @@ public enum FileDescriptorOps { case other } + /// The metadata of a directory entry, as reported by `stat(2)` without following symlinks. + public struct FileStatus: Sendable, Equatable { + public var type: EntryType + /// The permission bits, including setuid, setgid and sticky. + public var permissions: FilePermissions + public var size: Int64 + public var userID: UInt32 + public var groupID: UInt32 + public var modificationSeconds: Int64 + public var modificationNanoseconds: Int + } + // MARK: - Primitives // // Each primitive does one thing relative to a directory descriptor, never @@ -200,6 +213,166 @@ public enum FileDescriptorOps { } } + /// Returns the metadata of an open descriptor. + /// + /// - Throws: ``Error/systemError(_:_:)`` if the descriptor cannot be inspected. + public static func status(of fd: FileDescriptor) throws -> FileStatus { + var stbuf = stat() + guard fstat(fd.rawValue, &stbuf) == 0 else { + throw Error.systemError("stat during file descriptor status", errno) + } + return fileStatus(from: stbuf) + } + + /// Returns the metadata of the entry `name` in the directory `fd`, without following + /// a symlink, or `nil` if there is no such entry. For a symlink this is the metadata of + /// the link itself. + /// + /// - Throws: ``Error/systemError(_:_:)`` if the entry cannot be inspected. + public static func status(_ fd: FileDescriptor, _ name: FilePath.Component) throws -> FileStatus? { + var stbuf = stat() + guard fstatat(fd.rawValue, name.string, &stbuf, AT_SYMLINK_NOFOLLOW) == 0 else { + if errno == ENOENT { + return nil + } + throw Error.systemError("stat during file descriptor status", errno) + } + return fileStatus(from: stbuf) + } + + /// Opens the existing regular file `name` in the directory `fd` for reading, without + /// following a symlink. The returned descriptor is close-on-exec and the caller must close it. + /// + /// The file is opened non-blocking and checked after it is open, so a FIFO, device or + /// socket in its place is refused instead of blocking the caller or being read. + /// + /// - Throws: ``Error/notFound`` if there is no such entry, ``Error/cannotFollowSymlink`` + /// if it is a symlink, ``Error/conflict(_:)`` if it is not a regular file, and + /// ``Error/systemError(_:_:)`` for anything else. + public static func openFile(_ fd: FileDescriptor, _ name: FilePath.Component) throws -> FileDescriptor { + let newFd = openat(fd.rawValue, name.string, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_NOCTTY | O_CLOEXEC) + guard newFd >= 0 else { + let openErrno = errno + switch try entryType(fd, name) { + case nil: + throw Error.notFound + case .symlink: + throw Error.cannotFollowSymlink + case .other: + throw Error.conflict(.other) + case .regular, .directory: + throw Error.systemError("file open during file descriptor open", openErrno) + } + } + + let opened = FileDescriptor(rawValue: newFd) + do { + let type = try status(of: opened).type + guard type == .regular else { + throw Error.conflict(type) + } + return opened + } catch { + try? opened.close() + throw error + } + } + + /// Creates the new regular file `name` in the directory `fd`, open for writing. The + /// returned descriptor is close-on-exec and the caller must close it. + /// + /// The file is created exclusively and without following a symlink, so this never writes + /// through, or over, anything that already exists. + /// + /// - Parameters: + /// - fd: An open file descriptor for the parent directory. + /// - name: The name of the file to create. + /// - permissions: The permissions to give the file (default 0o644), subject to the umask. + /// - Throws: ``Error/alreadyExists`` if anything with that name exists, including a + /// symlink, and ``Error/systemError(_:_:)`` for anything else. + public static func createFile( + _ fd: FileDescriptor, + _ name: FilePath.Component, + permissions: FilePermissions? = nil + ) throws -> FileDescriptor { + let newFd = openat( + fd.rawValue, + name.string, + O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, + permissions?.rawValue ?? 0o644 + ) + guard newFd >= 0 else { + if errno == EEXIST { + throw Error.alreadyExists + } + throw Error.systemError("file creation during file descriptor create", errno) + } + return FileDescriptor(rawValue: newFd) + } + + /// Creates the symlink `name` in the directory `fd`, pointing at `target`. + /// + /// `target` is stored as given and is never resolved, so it may be absolute or contain `..`. + /// + /// - Throws: ``Error/alreadyExists`` if anything with that name exists, and + /// ``Error/systemError(_:_:)`` for anything else. + public static func makeSymlink(_ fd: FileDescriptor, _ name: FilePath.Component, target: String) throws { + guard symlinkat(target, fd.rawValue, name.string) == 0 else { + if errno == EEXIST { + throw Error.alreadyExists + } + throw Error.systemError("symlink creation during file descriptor symlink", errno) + } + } + + /// The longest symlink target ``readSymlink(_:_:)`` will read, in bytes. + /// + /// This is a sanity bound, not the limit of any particular platform, and `PATH_MAX` is the wrong + /// number to use for it. `PATH_MAX` is only the longest path that the system calls accept as an + /// argument (4096 on Linux, 1024 on macOS). It is not a promise about what a filesystem can store + /// or report, so a filesystem that is not bound by it would have links refused that really exist. + /// 16 KiB is four times the largest `PATH_MAX` of the platforms supported here, so every link those + /// platforms can create is read in full, while a filesystem that misbehaves still cannot make + /// this allocate much. + private static let maximumSymlinkTargetLength = 16 * 1024 + + /// Returns the target of the symlink `name` in the directory `fd`, without following it. + /// The target is decoded as UTF-8, and invalid sequences are replaced. A target longer than + /// 16 KiB is refused. + /// + /// - Throws: ``Error/notFound`` if there is no such entry, ``Error/conflict(_:)`` if it is + /// not a symlink, and ``Error/systemError(_:_:)`` for anything else, including a target that is too long. + public static func readSymlink(_ fd: FileDescriptor, _ name: FilePath.Component) throws -> String { + // Grow the buffer until the target fits, instead of asking for its length first (the `st_size` of + // `fstatat`) and sizing one buffer to match. A symlink is not locked, so it can be replaced between + // the two calls, and a buffer sized for the old target would silently cut off a longer new one. + // Here a read that fills the buffer is never trusted, so a target that is returned was read in full. + var capacity = 256 + while true { + var buffer = [CChar](repeating: 0, count: capacity) + let count = readlinkat(fd.rawValue, name.string, &buffer, capacity) + guard count >= 0 else { + let readlinkErrno = errno + switch try entryType(fd, name) { + case nil: + throw Error.notFound + case .symlink: + throw Error.systemError("symlink read during file descriptor readlink", readlinkErrno) + case let type?: + throw Error.conflict(type) + } + } + if count < capacity { + return String(decoding: buffer.prefix(count).map { UInt8(bitPattern: $0) }, as: UTF8.self) + } + // The buffer was filled, so the target may have been cut off. Retry with a larger one. + guard capacity < maximumSymlinkTargetLength else { + throw Error.systemError("symlink read during file descriptor readlink", ENAMETOOLONG) + } + capacity *= 2 + } + } + /// Recursively removes a direct child of the directory at `fd`. /// /// Symlinks are removed, not followed. `.` and `..` are ignored. A child that @@ -394,6 +567,23 @@ public enum FileDescriptorOps { } } + private static func fileStatus(from stbuf: stat) -> FileStatus { + #if canImport(Darwin) + let mtime = stbuf.st_mtimespec + #else + let mtime = stbuf.st_mtim + #endif + return FileStatus( + type: entryType(forMode: stbuf.st_mode), + permissions: FilePermissions(rawValue: stbuf.st_mode & 0o7777), + size: Int64(stbuf.st_size), + userID: UInt32(stbuf.st_uid), + groupID: UInt32(stbuf.st_gid), + modificationSeconds: Int64(mtime.tv_sec), + modificationNanoseconds: Int(mtime.tv_nsec) + ) + } + /// Rejects anything that is not a plain relative path: an absolute path, or a /// path with a `..` component. An empty path is allowed and means "the /// directory itself". Callers that accept untrusted names, such as archive diff --git a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift index 4392211f3..2513952e7 100644 --- a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift +++ b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift @@ -667,6 +667,258 @@ struct FileDescriptorPathSecureTests { #expect(FileManager.default.fileExists(atPath: rootPath.appending("dir").string), "unlink must not remove a directory") } + // MARK: - Leaf primitives + + private struct Sandbox { + let base: FilePath + let root: FilePath + let outside: FilePath + let rootFd: FileDescriptor + + func cleanup() { + try? rootFd.close() + try? FileManager.default.removeItem(atPath: base.string) + } + } + + /// A temporary `root` to work in, with a sibling `outside` that nothing may touch. + private func makeSandbox() throws -> Sandbox { + let base = try createTempDirectory() + let root = base.appending("root") + let outside = base.appending("outside") + try FileManager.default.createDirectory(atPath: root.string, withIntermediateDirectories: false) + try FileManager.default.createDirectory(atPath: outside.string, withIntermediateDirectories: false) + let rootFd = try FileDescriptor.open(root, .readOnly, options: [.directory]) + return Sandbox(base: base, root: root, outside: outside, rootFd: rootFd) + } + + private func writeText(_ text: String, to path: FilePath) throws { + try Data(text.utf8).write(to: URL(fileURLWithPath: path.string)) + } + + @Test("Test status reports metadata without following symlinks") + func testStatus() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try writeText("hello", to: sandbox.root.appending("file")) + try FileManager.default.setAttributes([.posixPermissions: 0o640], ofItemAtPath: sandbox.root.appending("file").string) + try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("link").string, withDestinationPath: "file") + + let file = try #require(try FileDescriptorOps.status(sandbox.rootFd, "file")) + #expect(file.type == .regular) + #expect(file.size == 5) + #expect(file.permissions.rawValue & 0o777 == 0o640) + #expect(file.userID == geteuid()) + #expect(file.modificationSeconds > 0) + + let link = try #require(try FileDescriptorOps.status(sandbox.rootFd, "link")) + #expect(link.type == .symlink, "status describes the link itself, not its target") + #expect(try FileDescriptorOps.status(sandbox.rootFd, "missing") == nil) + + let rootStatus = try FileDescriptorOps.status(of: sandbox.rootFd) + #expect(rootStatus.type == .directory) + } + + @Test("Test openFile reads a regular file and refuses everything else") + func testOpenFile() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try writeText("secret", to: sandbox.outside.appending("target")) + try writeText("hello", to: sandbox.root.appending("file")) + try FileManager.default.createDirectory(atPath: sandbox.root.appending("dir").string, withIntermediateDirectories: false) + try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("link").string, withDestinationPath: "../outside/target") + #expect(mkfifo(sandbox.root.appending("fifo").string, 0o644) == 0) + + let fd = try FileDescriptorOps.openFile(sandbox.rootFd, "file") + defer { try? fd.close() } + #expect(fcntl(fd.rawValue, F_GETFD) & FD_CLOEXEC != 0) + var buffer = [UInt8](repeating: 0, count: 16) + let count = read(fd.rawValue, &buffer, buffer.count) + #expect(String(decoding: buffer.prefix(max(count, 0)), as: UTF8.self) == "hello") + + #expect(throws: FileDescriptorOps.Error.notFound) { _ = try FileDescriptorOps.openFile(sandbox.rootFd, "missing") } + #expect(throws: FileDescriptorOps.Error.cannotFollowSymlink) { _ = try FileDescriptorOps.openFile(sandbox.rootFd, "link") } + #expect(throws: FileDescriptorOps.Error.conflict(.directory)) { _ = try FileDescriptorOps.openFile(sandbox.rootFd, "dir") } + // Opening a FIFO for reading would block forever if it were opened blocking. + #expect(throws: FileDescriptorOps.Error.conflict(.other)) { _ = try FileDescriptorOps.openFile(sandbox.rootFd, "fifo") } + } + + @Test("Test createFile creates exclusively and never writes through or over anything") + func testCreateFile() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try writeText("keep", to: sandbox.outside.appending("target")) + try writeText("old", to: sandbox.root.appending("file")) + try FileManager.default.createDirectory(atPath: sandbox.root.appending("dir").string, withIntermediateDirectories: false) + try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("link").string, withDestinationPath: "../outside/target") + + let fd = try FileDescriptorOps.createFile(sandbox.rootFd, "new", permissions: FilePermissions(rawValue: 0o600)) + defer { try? fd.close() } + #expect(fcntl(fd.rawValue, F_GETFD) & FD_CLOEXEC != 0) + #expect(write(fd.rawValue, "data", 4) == 4) + #expect(try String(contentsOfFile: sandbox.root.appending("new").string, encoding: .utf8) == "data") + + for name in ["file", "dir", "link"] as [FilePath.Component] { + #expect(throws: FileDescriptorOps.Error.alreadyExists) { _ = try FileDescriptorOps.createFile(sandbox.rootFd, name) } + } + #expect(try String(contentsOfFile: sandbox.root.appending("file").string, encoding: .utf8) == "old") + #expect(try String(contentsOfFile: sandbox.outside.appending("target").string, encoding: .utf8) == "keep", "nothing may be written through a symlink") + } + + @Test("Test makeSymlink and readSymlink round-trip any target without following it") + func testSymlinks() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try writeText("x", to: sandbox.root.appending("file")) + let longTarget = String(repeating: "a/", count: 300) + + let targets = ["relative", "../outside/escape", "/etc/passwd", longTarget] + for (index, target) in targets.enumerated() { + let name = try #require(FilePath.Component("link\(index)")) + try FileDescriptorOps.makeSymlink(sandbox.rootFd, name, target: target) + #expect(try FileDescriptorOps.readSymlink(sandbox.rootFd, name) == target) + #expect(try FileDescriptorOps.entryType(sandbox.rootFd, name) == .symlink) + } + + #expect(throws: FileDescriptorOps.Error.alreadyExists) { try FileDescriptorOps.makeSymlink(sandbox.rootFd, "file", target: "y") } + #expect(throws: FileDescriptorOps.Error.alreadyExists) { try FileDescriptorOps.makeSymlink(sandbox.rootFd, "link0", target: "y") } + #expect(throws: FileDescriptorOps.Error.conflict(.regular)) { _ = try FileDescriptorOps.readSymlink(sandbox.rootFd, "file") } + #expect(throws: FileDescriptorOps.Error.notFound) { _ = try FileDescriptorOps.readSymlink(sandbox.rootFd, "missing") } + } + + // MARK: - Reading beneath a directory + + @Test("Test openFile(relativePath:) reads nested files and refuses symlinks, parents that are not directories, and bad paths") + func testOpenFileRelativePath() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try writeText("secret", to: sandbox.outside.appending("target")) + try FileManager.default.createDirectory(atPath: sandbox.outside.appending("dir").string, withIntermediateDirectories: false) + try writeText("secret", to: sandbox.outside.appending("dir/target")) + try FileManager.default.createDirectory(atPath: sandbox.root.appending("a/b").string, withIntermediateDirectories: true) + try writeText("nested", to: sandbox.root.appending("a/b/file")) + try writeText("plain", to: sandbox.root.appending("plain")) + try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("filelink").string, withDestinationPath: "../outside/target") + try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("dirlink").string, withDestinationPath: "../outside/dir") + try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("a/up").string, withDestinationPath: "../../outside/dir") + + let fd = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "a/b/file") + defer { try? fd.close() } + var buffer = [UInt8](repeating: 0, count: 16) + let count = read(fd.rawValue, &buffer, buffer.count) + #expect(String(decoding: buffer.prefix(max(count, 0)), as: UTF8.self) == "nested") + + let cases: [(String, FileDescriptorOps.Error)] = [ + ("filelink", .cannotFollowSymlink), + ("dirlink/target", .cannotFollowSymlink), + ("a/up/target", .cannotFollowSymlink), + ("plain/x", .conflict(.regular)), + ("a/b/missing", .notFound), + ("missing/file", .notFound), + ("a/b", .conflict(.directory)), + ("../outside/target", .invalidRelativePath), + ("a/../../outside/target", .invalidRelativePath), + ("/etc/hosts", .invalidRelativePath), + ("", .invalidRelativePath), + ] + for (path, expected) in cases { + #expect(throws: expected, "\(path)") { _ = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: FilePath(path)) } + } + } + + @Test("Test status(relativePath:) describes entries beneath a directory and refuses symlinks in parent positions") + func testStatusRelativePath() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try FileManager.default.createDirectory(atPath: sandbox.outside.appending("dir").string, withIntermediateDirectories: false) + try writeText("x", to: sandbox.outside.appending("dir/target")) + try FileManager.default.createDirectory(atPath: sandbox.root.appending("a/b").string, withIntermediateDirectories: true) + try writeText("nested", to: sandbox.root.appending("a/b/file")) + try writeText("plain", to: sandbox.root.appending("plain")) + try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("dirlink").string, withDestinationPath: "../outside/dir") + + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b/file")?.type == .regular) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b/file")?.size == 6) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b")?.type == .directory) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "dirlink")?.type == .symlink, "the link itself") + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "")?.type == .directory, "an empty path is the directory itself") + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b/missing") == nil) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "missing/file") == nil) + + #expect(throws: FileDescriptorOps.Error.cannotFollowSymlink) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "dirlink/target") } + #expect(throws: FileDescriptorOps.Error.conflict(.regular)) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "plain/x") } + #expect(throws: FileDescriptorOps.Error.invalidRelativePath) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "../outside") } + #expect(throws: FileDescriptorOps.Error.invalidRelativePath) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "/etc") } + } + + private final class StopFlag: @unchecked Sendable { + private let lock = NSLock() + private var value = false + + var isSet: Bool { + lock.lock() + defer { lock.unlock() } + return value + } + + func set() { + lock.lock() + defer { lock.unlock() } + value = true + } + } + + @Test("Test openFile(relativePath:) never reads outside the root while components are swapped for symlinks") + func testOpenFileRaceNeverEscapes() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try writeText("SECRET", to: sandbox.outside.appending("bait")) + try FileManager.default.createDirectory(atPath: sandbox.root.appending("dir").string, withIntermediateDirectories: false) + try writeText("SAFE", to: sandbox.root.appending("dir/bait")) + + let root = sandbox.root.string + let outside = sandbox.outside.string + let stop = StopFlag() + let finished = DispatchSemaphore(value: 0) + DispatchQueue.global().async { + let fm = FileManager.default + while !stop.isSet { + // Swap the last component between a regular file and a symlink to the secret. + try? fm.removeItem(atPath: "\(root)/dir/bait") + try? fm.createSymbolicLink(atPath: "\(root)/dir/bait", withDestinationPath: "\(outside)/bait") + try? fm.removeItem(atPath: "\(root)/dir/bait") + try? Data("SAFE".utf8).write(to: URL(fileURLWithPath: "\(root)/dir/bait")) + // Swap the parent directory between a real directory and a symlink to the outside. + try? fm.moveItem(atPath: "\(root)/dir", toPath: "\(root)/dir.hold") + try? fm.createSymbolicLink(atPath: "\(root)/dir", withDestinationPath: outside) + try? fm.removeItem(atPath: "\(root)/dir") + try? fm.moveItem(atPath: "\(root)/dir.hold", toPath: "\(root)/dir") + } + finished.signal() + } + + var leaked = false + var opened = 0 + let deadline = Date().addingTimeInterval(1.0) + while Date() < deadline { + guard let fd = try? FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "dir/bait") else { + continue + } + var buffer = [UInt8](repeating: 0, count: 16) + let count = read(fd.rawValue, &buffer, buffer.count) + try? fd.close() + if String(decoding: buffer.prefix(max(count, 0)), as: UTF8.self) == "SECRET" { + leaked = true + } + opened += 1 + } + stop.set() + finished.wait() + + #expect(!leaked, "a file outside the root was read through a swapped symlink") + #expect(opened > 0, "the test never managed to open the file, so it proved nothing") + } + // MARK: - mkdir replaces what is in the way @Test("Test mkdir replaces a file or symlink that is in the way, and never writes through a symlink") From 2b39c94dda52d24640c17b11fda111214bf78fb3 Mon Sep 17 00:00:00 2001 From: John Logan Date: Wed, 7 Oct 2026 13:55:11 -0700 Subject: [PATCH 4/8] FileDescriptorOps: add an opt-in mode that follows symlinks beneath a directory Fifth step of the FileDescriptorOps series. openFile(_:relativePath:) and status(_:relativePath:) now require a SymlinkPolicy, so every caller states how symlinks should be treated. They were added one commit ago and have no callers outside the tests. SymlinkPolicy: - .refuse is the previous behavior: any symlink in the path is an error. - .followBeneath follows a symlink, but only to a place beneath the directory the walk started from. Some trees legitimately contain relative symlinks. For example, before v25 `docker save` stored a layer shared by several images once and linked to it from the others, as `/layer.tar -> ..//layer.tar`, so a loader for those archives cannot refuse every symlink. .followBeneath resolves links in userspace, one component at a time, against directory descriptors that are already open. Every component is still opened with O_NOFOLLOW; the kernel never follows a link. A target is resolved relative to the directory that holds the link. ".." in a target pops the stack of open directories and is refused at the bottom, which is what keeps every followed link beneath the starting directory. These are refused as cannotFollowSymlink: - an absolute or empty target - a target that would leave the starting directory - a chain of more than maximumSymlinksFollowed (40) links, which also stops loops openFile follows a symlink in the last position too. status follows links in parent positions only and describes a final symlink itself. Adds tests for the legacy docker-archive layout, links in parent positions, chains, ".." inside targets, each refusal including a chain of exactly the allowed length, links that end at a directory or nothing, and the concurrent swap test now runs under both policies with the swapped-in links pointing outside through a relative path. --- .../FileDescriptorOps+Composite.swift | 145 ++++++++++---- .../FileDescriptorOpsTests.swift | 181 ++++++++++++++++-- 2 files changed, 263 insertions(+), 63 deletions(-) diff --git a/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift index 9ab9051f3..a5cef1ff7 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift @@ -90,82 +90,143 @@ extension FileDescriptorOps { try completion(current) } + /// What to do when a symlink is found while resolving a path. + public enum SymlinkPolicy: Sendable, Equatable { + /// Refuse any symlink in the path, including the last component. This is the safe choice + /// for anything that does not need symlinks to work. + case refuse + + /// Follow a symlink, but only to a place beneath the directory the walk started from. + /// + /// Symlinks are resolved here, one component at a time, against directory descriptors + /// that are already open, and never by the kernel. A relative target may use `..` as + /// long as the result stays beneath the starting directory. An absolute target, a target + /// that would leave the starting directory, and a chain of more than + /// ``maximumSymlinksFollowed`` links are all refused as ``Error/cannotFollowSymlink``. + /// + /// Use it for trees that legitimately contain relative symlinks, such as a legacy + /// docker-archive where a layer shared between images is a link to another image's copy. + case followBeneath + } + + /// The most symlinks that ``SymlinkPolicy/followBeneath`` follows while resolving one path. + public static let maximumSymlinksFollowed = 40 + /// Opens the existing regular file at `relativePath` below `fd` for reading. /// - /// Every component is opened relative to the previous one without following - /// symlinks, and the file is checked after it is open. A symlink anywhere in the - /// path, including the last component, is refused, and a swap of any component - /// for a symlink while this runs cannot redirect the open. The returned descriptor - /// is close-on-exec and the caller must close it. + /// Every component is opened relative to the previous one with `O_NOFOLLOW`, and the file is + /// checked after it is open, so a swap of any component for a symlink while this runs + /// cannot redirect the open. How symlinks that are already there are treated is up to `symlinks`. + /// The returned descriptor is close-on-exec and the caller must close it. /// /// - Parameters: /// - fd: An open file descriptor for the directory to start from. /// - relativePath: The file to open. It must be relative, must not contain a `..` component, and must name something. + /// - symlinks: What to do about symlinks in the path, including the last component. There is no default. /// - Throws: ``Error/invalidRelativePath`` for an absolute path, one containing `..`, or an empty path; /// ``Error/notFound`` if the file or a parent does not exist; ``Error/cannotFollowSymlink`` if a symlink - /// is in the path; ``Error/conflict(_:)`` if a parent is not a directory or the file is not a regular - /// file; and ``Error/systemError(_:_:)`` for anything else. - public static func openFile(_ fd: FileDescriptor, relativePath: FilePath) throws -> FileDescriptor { + /// is refused or cannot be followed beneath `fd`; ``Error/conflict(_:)`` if a parent is not a directory or + /// the path does not end at a regular file; and ``Error/systemError(_:_:)`` for anything else. + public static func openFile(_ fd: FileDescriptor, relativePath: FilePath, symlinks: SymlinkPolicy) throws -> FileDescriptor { try validateRelativePath(relativePath) - var components = Array(relativePath.components) - guard let name = components.popLast() else { + let components = Array(relativePath.components) + guard !components.isEmpty else { throw Error.invalidRelativePath } - return try withDirectory(fd, components) { parent in - try openFile(parent, name) - } + return try resolveBeneath( + fd, components, symlinks: symlinks, + atEntry: { parent, name in try openFile(parent, name) }, + atDirectory: { _ in throw Error.conflict(.directory) }) } - /// Returns the metadata of the entry at `relativePath` below `fd`, without following - /// symlinks, or `nil` if the entry or one of its parents does not exist. For a symlink this - /// is the metadata of the link itself. An empty path describes `fd` itself. + /// Returns the metadata of the entry at `relativePath` below `fd`, or `nil` if the entry or + /// one of its parents does not exist. An empty path describes `fd` itself. /// - /// A symlink in a parent position is refused, so the answer always describes an entry - /// that is really beneath `fd`. + /// The entry itself is never followed: for a symlink this is the metadata of the link. How + /// symlinks in the parent positions are treated is up to `symlinks`, so the answer always + /// describes an entry that is really beneath `fd`. /// /// - Throws: ``Error/invalidRelativePath`` for an absolute path or one containing `..`; - /// ``Error/cannotFollowSymlink`` if a symlink is in a parent position; ``Error/conflict(_:)`` if a - /// parent is not a directory; and ``Error/systemError(_:_:)`` for anything else. - public static func status(_ fd: FileDescriptor, relativePath: FilePath) throws -> FileStatus? { + /// ``Error/cannotFollowSymlink`` if a symlink in a parent position is refused or cannot be followed + /// beneath `fd`; ``Error/conflict(_:)`` if a parent is not a directory; and + /// ``Error/systemError(_:_:)`` for anything else. + public static func status(_ fd: FileDescriptor, relativePath: FilePath, symlinks: SymlinkPolicy) throws -> FileStatus? { try validateRelativePath(relativePath) - var components = Array(relativePath.components) - guard let name = components.popLast() else { + let components = Array(relativePath.components) + guard !components.isEmpty else { return try status(of: fd) } do { - return try withDirectory(fd, components) { parent in - try status(parent, name) - } + return try resolveBeneath( + fd, components, symlinks: symlinks, + atEntry: { parent, name in try status(parent, name) }, + atDirectory: { directory in try status(of: directory) }) } catch Error.notFound { return nil } } - /// Runs `body` with a descriptor for the directory reached by walking `components` from - /// `fd`, without following symlinks. With no components, `body` gets `fd` itself. - private static func withDirectory( - _ fd: FileDescriptor, + /// Walks `components` from `root`, opening each parent with `O_NOFOLLOW`, and calls `atEntry` with + /// the directory that holds the last component. If `atEntry` throws ``Error/cannotFollowSymlink`` + /// because the last component is a symlink, and `symlinks` allows it, the link is followed. + /// + /// The walk keeps a stack of open directory descriptors. `..` in a symlink target pops the stack and is + /// refused at the bottom, which is what keeps every followed link beneath `root`. If the path resolves + /// to a directory without ever reaching a last component, for example a link to `..`, `atDirectory` runs. + private static func resolveBeneath( + _ root: FileDescriptor, _ components: [FilePath.Component], - _ body: (FileDescriptor) throws -> T + symlinks: SymlinkPolicy, + atEntry: (_ parent: FileDescriptor, _ name: FilePath.Component) throws -> T, + atDirectory: (_ directory: FileDescriptor) throws -> T ) throws -> T { - var current = fd - var ownsCurrent = false + var pending = Array(components.reversed()) + var directories = [root] + var symlinksFollowed = 0 defer { - if ownsCurrent { - try? current.close() + // The first entry is the caller's descriptor. + for directory in directories.dropFirst() { + try? directory.close() } } - for component in components { - let next = try openDirectory(current, component) - if ownsCurrent { - try? current.close() + func follow(_ name: FilePath.Component, in parent: FileDescriptor) throws { + symlinksFollowed += 1 + guard symlinksFollowed <= maximumSymlinksFollowed else { + throw Error.cannotFollowSymlink + } + let target = FilePath(try readSymlink(parent, name)) + guard !target.isAbsolute, !target.components.isEmpty else { + throw Error.cannotFollowSymlink + } + // The target is resolved next, relative to the directory that holds the link. + pending.append(contentsOf: target.components.reversed()) + } + + while let name = pending.popLast() { + if name.string == "." { + continue + } + if name.string == ".." { + guard directories.count > 1, let popped = directories.popLast() else { + throw Error.cannotFollowSymlink + } + try? popped.close() + continue + } + + let parent = directories[directories.count - 1] + do { + if pending.isEmpty { + return try atEntry(parent, name) + } + directories.append(try openDirectory(parent, name)) + } catch Error.cannotFollowSymlink where symlinks == .followBeneath { + try follow(name, in: parent) } - current = next - ownsCurrent = true } - return try body(current) + return try atDirectory(directories[directories.count - 1]) } private static func openOrCreateDirectory( diff --git a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift index 2513952e7..6a2c1765c 100644 --- a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift +++ b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift @@ -802,7 +802,7 @@ struct FileDescriptorPathSecureTests { try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("dirlink").string, withDestinationPath: "../outside/dir") try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("a/up").string, withDestinationPath: "../../outside/dir") - let fd = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "a/b/file") + let fd = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "a/b/file", symlinks: .refuse) defer { try? fd.close() } var buffer = [UInt8](repeating: 0, count: 16) let count = read(fd.rawValue, &buffer, buffer.count) @@ -822,7 +822,7 @@ struct FileDescriptorPathSecureTests { ("", .invalidRelativePath), ] for (path, expected) in cases { - #expect(throws: expected, "\(path)") { _ = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: FilePath(path)) } + #expect(throws: expected, "\(path)") { _ = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: FilePath(path), symlinks: .refuse) } } } @@ -837,18 +837,155 @@ struct FileDescriptorPathSecureTests { try writeText("plain", to: sandbox.root.appending("plain")) try FileManager.default.createSymbolicLink(atPath: sandbox.root.appending("dirlink").string, withDestinationPath: "../outside/dir") - #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b/file")?.type == .regular) - #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b/file")?.size == 6) - #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b")?.type == .directory) - #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "dirlink")?.type == .symlink, "the link itself") - #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "")?.type == .directory, "an empty path is the directory itself") - #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b/missing") == nil) - #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "missing/file") == nil) - - #expect(throws: FileDescriptorOps.Error.cannotFollowSymlink) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "dirlink/target") } - #expect(throws: FileDescriptorOps.Error.conflict(.regular)) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "plain/x") } - #expect(throws: FileDescriptorOps.Error.invalidRelativePath) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "../outside") } - #expect(throws: FileDescriptorOps.Error.invalidRelativePath) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "/etc") } + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b/file", symlinks: .refuse)?.type == .regular) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b/file", symlinks: .refuse)?.size == 6) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b", symlinks: .refuse)?.type == .directory) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "dirlink", symlinks: .refuse)?.type == .symlink, "the link itself") + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "", symlinks: .refuse)?.type == .directory, "an empty path is the directory itself") + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "a/b/missing", symlinks: .refuse) == nil) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "missing/file", symlinks: .refuse) == nil) + + #expect(throws: FileDescriptorOps.Error.cannotFollowSymlink) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "dirlink/target", symlinks: .refuse) } + #expect(throws: FileDescriptorOps.Error.conflict(.regular)) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "plain/x", symlinks: .refuse) } + #expect(throws: FileDescriptorOps.Error.invalidRelativePath) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "../outside", symlinks: .refuse) } + #expect(throws: FileDescriptorOps.Error.invalidRelativePath) { _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "/etc", symlinks: .refuse) } + } + + // MARK: - Following symlinks beneath a directory + + private func link(_ name: String, to target: String, in directory: FilePath) throws { + try FileManager.default.createSymbolicLink(atPath: directory.appending(name).string, withDestinationPath: target) + } + + private func readAll(_ fd: FileDescriptor) -> String { + var buffer = [UInt8](repeating: 0, count: 64) + let count = read(fd.rawValue, &buffer, buffer.count) + return String(decoding: buffer.prefix(max(count, 0)), as: UTF8.self) + } + + @Test("Test followBeneath resolves the symlinks of a legacy docker archive and refuse does not") + func testFollowBeneathLegacyDockerArchiveLayout() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + // `docker save` before v25 stored a layer shared by several images once, and linked to it. + try FileManager.default.createDirectory(atPath: sandbox.root.appending("aaa").string, withIntermediateDirectories: false) + try FileManager.default.createDirectory(atPath: sandbox.root.appending("bbb").string, withIntermediateDirectories: false) + try writeText("layer-a", to: sandbox.root.appending("aaa/layer.tar")) + try link("layer.tar", to: "../aaa/layer.tar", in: sandbox.root.appending("bbb")) + + let fd = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "bbb/layer.tar", symlinks: .followBeneath) + defer { try? fd.close() } + #expect(readAll(fd) == "layer-a") + #expect(fcntl(fd.rawValue, F_GETFD) & FD_CLOEXEC != 0) + + #expect(throws: FileDescriptorOps.Error.cannotFollowSymlink) { + _ = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "bbb/layer.tar", symlinks: .refuse) + } + } + + @Test("Test followBeneath follows links in parent positions, chains of links, and links that use ..") + func testFollowBeneathResolvesLinks() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try FileManager.default.createDirectory(atPath: sandbox.root.appending("real/deep").string, withIntermediateDirectories: true) + try FileManager.default.createDirectory(atPath: sandbox.root.appending("other").string, withIntermediateDirectories: false) + try writeText("deep-file", to: sandbox.root.appending("real/deep/file")) + try writeText("other-file", to: sandbox.root.appending("other/file")) + try link("dirlink", to: "real", in: sandbox.root) // a directory link in a parent position + try link("c", to: "real/deep/file", in: sandbox.root) // c -> b -> a -> the file + try link("b", to: "c", in: sandbox.root) + try link("a", to: "b", in: sandbox.root) + try link("up", to: "../other", in: sandbox.root.appending("real")) // real/up -> ../other, stays beneath root + try link("dot", to: "./real/./deep/../deep/file", in: sandbox.root) + + let expectations = [ + ("dirlink/deep/file", "deep-file"), + ("a", "deep-file"), + ("real/up/file", "other-file"), + ("dirlink/up/file", "other-file"), + ("dot", "deep-file"), + ] + for (path, expected) in expectations { + let fd = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: FilePath(path), symlinks: .followBeneath) + defer { try? fd.close() } + #expect(readAll(fd) == expected, "\(path)") + } + } + + @Test("Test followBeneath refuses links that leave the directory, absolute links, loops, and long chains") + func testFollowBeneathRefusals() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try writeText("SECRET", to: sandbox.outside.appending("target")) + try FileManager.default.createDirectory(atPath: sandbox.root.appending("sub/deep").string, withIntermediateDirectories: true) + try writeText("ok", to: sandbox.root.appending("sub/deep/file")) + try link("abs", to: sandbox.outside.appending("target").string, in: sandbox.root) // absolute target + try link("escape", to: "../outside/target", in: sandbox.root) // one level above the root + try link("deepescape", to: "sub/deep/../../../outside/target", in: sandbox.root) // stays inside until the last .. + try link("direscape", to: "../outside", in: sandbox.root) // a directory link out + try link("loop1", to: "loop2", in: sandbox.root) + try link("loop2", to: "loop1", in: sandbox.root) + try link("self", to: "self", in: sandbox.root) + // A chain one link longer than allowed. + let limit = FileDescriptorOps.maximumSymlinksFollowed + for index in 0...limit { + try link("chain\(index)", to: index == limit ? "sub/deep/file" : "chain\(index + 1)", in: sandbox.root) + } + // A chain exactly as long as allowed still works. + try link("short\(limit - 1)", to: "sub/deep/file", in: sandbox.root) + for index in 0..<(limit - 1) { + try link("short\(index)", to: "short\(index + 1)", in: sandbox.root) + } + + for path in ["abs", "escape", "deepescape", "direscape/target", "loop1", "self", "chain0"] { + #expect(throws: FileDescriptorOps.Error.cannotFollowSymlink, "\(path)") { + _ = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: FilePath(path), symlinks: .followBeneath) + } + } + let fd = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "short0", symlinks: .followBeneath) + defer { try? fd.close() } + #expect(readAll(fd) == "ok", "a chain of exactly \(limit) links is allowed") + } + + @Test("Test followBeneath reports links that lead to a directory, or to nothing, as errors") + func testFollowBeneathNonFileTargets() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try FileManager.default.createDirectory(atPath: sandbox.root.appending("sub").string, withIntermediateDirectories: false) + try link("todir", to: "sub", in: sandbox.root) + try link("todot", to: ".", in: sandbox.root.appending("sub")) + try link("dangling", to: "missing", in: sandbox.root) + + #expect(throws: FileDescriptorOps.Error.conflict(.directory)) { + _ = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "todir", symlinks: .followBeneath) + } + #expect(throws: FileDescriptorOps.Error.conflict(.directory)) { + _ = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "sub/todot", symlinks: .followBeneath) + } + #expect(throws: FileDescriptorOps.Error.notFound) { + _ = try FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "dangling", symlinks: .followBeneath) + } + } + + @Test("Test status with followBeneath follows parents but still describes a final symlink itself") + func testStatusFollowBeneath() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try FileManager.default.createDirectory(atPath: sandbox.root.appending("real").string, withIntermediateDirectories: false) + try writeText("hello", to: sandbox.root.appending("real/file")) + try link("dirlink", to: "real", in: sandbox.root) + try link("filelink", to: "real/file", in: sandbox.root) + try link("escape", to: "../outside", in: sandbox.root) + + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "dirlink/file", symlinks: .followBeneath)?.size == 5) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "filelink", symlinks: .followBeneath)?.type == .symlink) + #expect(try FileDescriptorOps.status(sandbox.rootFd, relativePath: "dirlink/missing", symlinks: .followBeneath) == nil) + #expect(throws: FileDescriptorOps.Error.cannotFollowSymlink) { + _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "escape/anything", symlinks: .followBeneath) + } + #expect(throws: FileDescriptorOps.Error.cannotFollowSymlink) { + _ = try FileDescriptorOps.status(sandbox.rootFd, relativePath: "dirlink/file", symlinks: .refuse) + } } private final class StopFlag: @unchecked Sendable { @@ -868,8 +1005,10 @@ struct FileDescriptorPathSecureTests { } } - @Test("Test openFile(relativePath:) never reads outside the root while components are swapped for symlinks") - func testOpenFileRaceNeverEscapes() throws { + @Test( + "Test openFile(relativePath:) never reads outside the root while components are swapped for symlinks", + arguments: [FileDescriptorOps.SymlinkPolicy.refuse, .followBeneath]) + func testOpenFileRaceNeverEscapes(symlinks: FileDescriptorOps.SymlinkPolicy) throws { let sandbox = try makeSandbox() defer { sandbox.cleanup() } try writeText("SECRET", to: sandbox.outside.appending("bait")) @@ -877,20 +1016,20 @@ struct FileDescriptorPathSecureTests { try writeText("SAFE", to: sandbox.root.appending("dir/bait")) let root = sandbox.root.string - let outside = sandbox.outside.string let stop = StopFlag() let finished = DispatchSemaphore(value: 0) DispatchQueue.global().async { let fm = FileManager.default while !stop.isSet { - // Swap the last component between a regular file and a symlink to the secret. + // Swap the last component between a regular file and a symlink to the secret. The link is relative, + // so it is the kind that `followBeneath` would follow if it stayed beneath the root. try? fm.removeItem(atPath: "\(root)/dir/bait") - try? fm.createSymbolicLink(atPath: "\(root)/dir/bait", withDestinationPath: "\(outside)/bait") + try? fm.createSymbolicLink(atPath: "\(root)/dir/bait", withDestinationPath: "../../outside/bait") try? fm.removeItem(atPath: "\(root)/dir/bait") try? Data("SAFE".utf8).write(to: URL(fileURLWithPath: "\(root)/dir/bait")) // Swap the parent directory between a real directory and a symlink to the outside. try? fm.moveItem(atPath: "\(root)/dir", toPath: "\(root)/dir.hold") - try? fm.createSymbolicLink(atPath: "\(root)/dir", withDestinationPath: outside) + try? fm.createSymbolicLink(atPath: "\(root)/dir", withDestinationPath: "../outside") try? fm.removeItem(atPath: "\(root)/dir") try? fm.moveItem(atPath: "\(root)/dir.hold", toPath: "\(root)/dir") } @@ -901,7 +1040,7 @@ struct FileDescriptorPathSecureTests { var opened = 0 let deadline = Date().addingTimeInterval(1.0) while Date() < deadline { - guard let fd = try? FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "dir/bait") else { + guard let fd = try? FileDescriptorOps.openFile(sandbox.rootFd, relativePath: "dir/bait", symlinks: symlinks) else { continue } var buffer = [UInt8](repeating: 0, count: 16) From cb47142fed4ec3699ec62879bfc6878cd671cb28 Mon Sep 17 00:00:00 2001 From: John Logan Date: Wed, 7 Oct 2026 14:16:29 -0700 Subject: [PATCH 5/8] FileDescriptorOps: add rename and an atomic file replace Sixth step of the FileDescriptorOps series. Purely additive. - rename (primitive): renameat between two directory descriptors. It is atomic and does not follow a symlink on either side: a symlink is moved, not its target. Like rename(2), it replaces an existing non-directory destination, which the documentation states plainly. Callers who must not replace anything should check first or create the destination exclusively instead. - replaceFile (composite, built from createFile, rename and unlink): writes a file so that a reader sees either the old contents or the new contents, never a partly written file. The data goes to an exclusively created temporary file in the same directory, which is then renamed over the target. A symlink at the target is replaced and its target is never written to. If the writer throws, the temporary file is removed and the old file is left as it was. The new file does not inherit the permissions or ownership of the file it replaces, and the write is not flushed to disk. A move or clone primitive for large files, and a hard-link primitive, are not included: nothing needs them yet, and cloning is APFS-specific. Adds tests for rename (same directory, across directories, replacing a file, moving a symlink without touching its target, replacing a symlink destination, a missing source) and for replaceFile (create, overwrite, replacing a symlink, permissions, a failing writer), plus a concurrency test where a writer repeatedly replaces a 256 KiB file while a reader checks that every read is complete and uniform. --- .../FileDescriptorOps+Composite.swift | 44 ++++++ .../FileDescriptorOps.swift | 28 +++- .../FileDescriptorOpsTests.swift | 140 ++++++++++++++++++ 3 files changed, 211 insertions(+), 1 deletion(-) diff --git a/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift index a5cef1ff7..009ca62f5 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift @@ -229,6 +229,50 @@ extension FileDescriptorOps { return try atDirectory(directories[directories.count - 1]) } + /// Writes the file `name` in `directory` so that a reader sees either the old contents or the new + /// contents, never a partly written file. + /// + /// The new contents are written to a temporary file in the same directory, which is created + /// exclusively, and then renamed over `name`. If `name` is a symlink, the link itself is replaced + /// and its target is never written to. If `write` throws, the temporary file is removed and + /// whatever was at `name` is left as it was. + /// + /// The new file does not inherit the permissions or ownership of the file it replaces. It + /// is atomic with respect to other readers, but it does not flush to disk. + /// + /// - Parameters: + /// - directory: An open file descriptor for the directory that holds the file. + /// - name: The name of the file to write. + /// - permissions: The permissions to give the new file (default 0o644), subject to the umask. + /// - write: Writes the new contents to the descriptor it is given. It must not close the descriptor. + /// - Throws: Errors from `write`, and ``Error/systemError(_:_:)`` if the temporary file cannot be + /// created or renamed into place, for example because `name` is a non-empty directory. + public static func replaceFile( + in directory: FileDescriptor, + named name: FilePath.Component, + permissions: FilePermissions? = nil, + write: (FileDescriptor) throws -> Void + ) throws { + guard let temporaryName = FilePath.Component(".tmp-" + String(UInt64.random(in: .min ... .max), radix: 16)) else { + throw Error.invalidPathComponent + } + + let file = try createFile(directory, temporaryName, permissions: permissions) + var isOpen = true + do { + try write(file) + isOpen = false + try file.close() + try rename(directory, temporaryName, to: directory, name) + } catch { + if isOpen { + try? file.close() + } + try? unlink(directory, temporaryName) + throw error + } + } + private static func openOrCreateDirectory( _ parent: FileDescriptor, _ name: FilePath.Component, diff --git a/Sources/ContainerizationOS/FileDescriptorOps.swift b/Sources/ContainerizationOS/FileDescriptorOps.swift index 6d0883862..d163d3a9b 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps.swift @@ -45,7 +45,8 @@ private func dupCloseOnExec(_ fd: Int32) -> Int32 { } /// Static utility functions for secure, symlink-safe filesystem operations -/// anchored to a file descriptor. +/// anchored to a file descriptor, with consistent semantics for both +/// Darwin and Linux. /// /// All operations use `openat`/`mkdirat`/`unlinkat` anchored to the supplied /// file descriptor. Every path component is opened with `O_NOFOLLOW`, so a @@ -213,6 +214,31 @@ public enum FileDescriptorOps { } } + /// Renames the entry `fromName` in the directory `fromDirectory` to `toName` in the directory + /// `toDirectory`. The two directories may be the same. + /// + /// The rename is atomic, and neither name is followed: a symlink is renamed, not its target. + /// + /// **This replaces an existing destination.** If `toName` exists and is not a directory, it is + /// replaced in one step, and a symlink is replaced rather than written through. As for + /// `rename(2)`, a directory may only replace an empty directory, and not the reverse. + /// + /// - Throws: ``Error/notFound`` if `fromName` does not exist, and ``Error/systemError(_:_:)`` + /// for anything else. + public static func rename( + _ fromDirectory: FileDescriptor, + _ fromName: FilePath.Component, + to toDirectory: FileDescriptor, + _ toName: FilePath.Component + ) throws { + guard renameat(fromDirectory.rawValue, fromName.string, toDirectory.rawValue, toName.string) == 0 else { + if errno == ENOENT { + throw Error.notFound + } + throw Error.systemError("rename during file descriptor rename", errno) + } + } + /// Returns the metadata of an open descriptor. /// /// - Throws: ``Error/systemError(_:_:)`` if the descriptor cannot be inspected. diff --git a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift index 6a2c1765c..541fe6f07 100644 --- a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift +++ b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift @@ -1058,6 +1058,146 @@ struct FileDescriptorPathSecureTests { #expect(opened > 0, "the test never managed to open the file, so it proved nothing") } + // MARK: - rename and replaceFile + + private func writeAll(_ bytes: [UInt8], to fd: FileDescriptor) throws { + var offset = 0 + while offset < bytes.count { + let written = bytes[offset...].withUnsafeBytes { write(fd.rawValue, $0.baseAddress, $0.count) } + guard written > 0 else { + throw Errno(rawValue: errno) + } + offset += written + } + } + + private func readEverything(_ fd: FileDescriptor) -> [UInt8] { + var result = [UInt8]() + var buffer = [UInt8](repeating: 0, count: 65536) + while true { + let count = read(fd.rawValue, &buffer, buffer.count) + guard count > 0 else { + return result + } + result.append(contentsOf: buffer.prefix(count)) + } + } + + @Test("Test rename moves entries atomically, replaces a non-directory destination, and never follows a symlink") + func testRename() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try FileManager.default.createDirectory(atPath: sandbox.root.appending("sub").string, withIntermediateDirectories: false) + try writeText("one", to: sandbox.root.appending("a")) + try writeText("old", to: sandbox.root.appending("b")) + try writeText("target", to: sandbox.outside.appending("target")) + try link("l", to: "../outside/target", in: sandbox.root) + try writeText("two", to: sandbox.root.appending("c")) + let subFd = try FileDescriptorOps.openDirectory(sandbox.rootFd, "sub") + defer { try? subFd.close() } + + // Within one directory, replacing an existing file. + try FileDescriptorOps.rename(sandbox.rootFd, "a", to: sandbox.rootFd, "b") + #expect(try String(contentsOfFile: sandbox.root.appending("b").string, encoding: .utf8) == "one") + #expect(!FileManager.default.fileExists(atPath: sandbox.root.appending("a").string)) + + // Across directories. + try FileDescriptorOps.rename(sandbox.rootFd, "c", to: subFd, "moved") + #expect(try String(contentsOfFile: sandbox.root.appending("sub/moved").string, encoding: .utf8) == "two") + + // A symlink is moved, not its target, and a symlink destination is replaced, not written through. + try FileDescriptorOps.rename(sandbox.rootFd, "l", to: subFd, "l2") + #expect(try FileDescriptorOps.readSymlink(subFd, "l2") == "../outside/target") + try FileDescriptorOps.rename(subFd, "moved", to: subFd, "l2") + #expect(try FileDescriptorOps.entryType(subFd, "l2") == .regular) + #expect(try String(contentsOfFile: sandbox.outside.appending("target").string, encoding: .utf8) == "target") + + #expect(throws: FileDescriptorOps.Error.notFound) { try FileDescriptorOps.rename(sandbox.rootFd, "missing", to: sandbox.rootFd, "x") } + } + + @Test("Test replaceFile creates a file, replaces a file or symlink, and cleans up when the writer fails") + func testReplaceFile() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + try writeText("keep", to: sandbox.outside.appending("target")) + try writeText("old", to: sandbox.root.appending("existing")) + try link("viaLink", to: "../outside/target", in: sandbox.root) + + try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "fresh", permissions: FilePermissions(rawValue: 0o600)) { fd in + try writeAll(Array("brand new".utf8), to: fd) + } + #expect(try String(contentsOfFile: sandbox.root.appending("fresh").string, encoding: .utf8) == "brand new") + let freshStatus = try #require(try FileDescriptorOps.status(sandbox.rootFd, "fresh")) + #expect(freshStatus.permissions.rawValue & 0o777 == 0o600) + + try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "existing") { fd in try writeAll(Array("updated".utf8), to: fd) } + #expect(try String(contentsOfFile: sandbox.root.appending("existing").string, encoding: .utf8) == "updated") + + // The link is replaced, and its target is never written to. + try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "viaLink") { fd in try writeAll(Array("PWNED".utf8), to: fd) } + #expect(try FileDescriptorOps.entryType(sandbox.rootFd, "viaLink") == .regular) + #expect(try String(contentsOfFile: sandbox.outside.appending("target").string, encoding: .utf8) == "keep") + + struct WriterFailed: Swift.Error {} + #expect(throws: WriterFailed.self) { + try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "existing") { fd in + try writeAll(Array("partial".utf8), to: fd) + throw WriterFailed() + } + } + #expect(try String(contentsOfFile: sandbox.root.appending("existing").string, encoding: .utf8) == "updated", "a failed write leaves the old file") + let leftovers = try FileManager.default.contentsOfDirectory(atPath: sandbox.root.string).filter { $0.hasPrefix(".tmp-") } + #expect(leftovers.isEmpty, "temporary files must be removed: \(leftovers)") + } + + @Test("Test replaceFile never lets a reader see a partly written file") + func testReplaceFileIsAtomicForReaders() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + let size = 256 * 1024 + try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "data") { fd in try writeAll([UInt8](repeating: 65, count: size), to: fd) } + + let stop = StopFlag() + let finished = DispatchSemaphore(value: 0) + let rootFd = sandbox.rootFd + DispatchQueue.global().async { + var letter: UInt8 = 66 + while !stop.isSet { + try? FileDescriptorOps.replaceFile(in: rootFd, named: "data") { fd in + var offset = 0 + let bytes = [UInt8](repeating: letter, count: size) + while offset < bytes.count { + let written = bytes[offset...].withUnsafeBytes { write(fd.rawValue, $0.baseAddress, $0.count) } + if written <= 0 { throw Errno(rawValue: errno) } + offset += written + } + } + letter = letter == 66 ? 65 : 66 + } + finished.signal() + } + + var torn = 0 + var reads = 0 + let deadline = Date().addingTimeInterval(1.0) + while Date() < deadline { + guard let fd = try? FileDescriptorOps.openFile(rootFd, relativePath: "data", symlinks: .refuse) else { + continue + } + let bytes = readEverything(fd) + try? fd.close() + reads += 1 + if bytes.count != size || Set(bytes).count != 1 { + torn += 1 + } + } + stop.set() + finished.wait() + + #expect(torn == 0, "\(torn) of \(reads) reads saw a partly written or mixed file") + #expect(reads > 0) + } + // MARK: - mkdir replaces what is in the way @Test("Test mkdir replaces a file or symlink that is in the way, and never writes through a symlink") From e08521ab26756e6a7e2d8e5e0ad9c77367dbb09f Mon Sep 17 00:00:00 2001 From: John Logan Date: Wed, 7 Oct 2026 15:37:01 -0700 Subject: [PATCH 6/8] FileDescriptorOps: pass O_RESOLVE_BENEATH where available, and document the type Seventh step of the FileDescriptorOps series. Where the kernel honors it, every openat in FileDescriptorOps now also passes O_RESOLVE_BENEATH, through one helper (resolveBeneathFlag). The kernel then refuses a path that is absolute or would leave the directory it is relative to. Every path given to openat here is already a single validated component, so this changes nothing when the code is correct. It is defense in depth: a bug in the path validation becomes a failed open instead of an escape. Per-component O_NOFOLLOW against a pinned directory descriptor remains the mechanism that provides the safety. Availability, checked against the xnu sources: - open(2) and openat(2) honor the flag from macOS 15.4 (xnu-11417.101.15), where vn_open_auth turns O_RESOLVE_BENEATH into the lookup flag. A refused path fails with EACCES there, and with ENOTCAPABLE from macOS 26.0. - It is absent from macOS 15.0 to 15.3, and the package supports macOS 15.0. On those kernels its value, 0x1000, is FMARK, so the flag must not be passed to them. The helper returns it only under #available(macOS 15.4, *), and returns 0 everywhere else, including on Linux. - The value is defined in the source and not taken from the SDK, so this builds with an SDK that predates the flag. Rewrites the FileDescriptorOps type documentation to state what the type guarantees (everything is relative to a directory descriptor, symlinks are never followed unless the caller opts in, the primitives never remove or replace anything unless asked and a composite documents what it replaces, descriptors are close-on-exec, errors are typed), how the primitives and the composites differ, and the platform notes above. Adds tests that the flag is used exactly on kernels that support it, and that the kernel honors the value defined here: an openat of a path that leaves the directory fails with the expected errno with the flag and succeeds without it. --- .../FileDescriptorOps.swift | 89 ++++++++++++++++--- .../FileDescriptorOpsTests.swift | 40 +++++++++ 2 files changed, 116 insertions(+), 13 deletions(-) diff --git a/Sources/ContainerizationOS/FileDescriptorOps.swift b/Sources/ContainerizationOS/FileDescriptorOps.swift index d163d3a9b..0430b22ef 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps.swift @@ -44,15 +44,56 @@ private func dupCloseOnExec(_ fd: Int32) -> Int32 { fcntl(fd, F_DUPFD_CLOEXEC, 0) } -/// Static utility functions for secure, symlink-safe filesystem operations -/// anchored to a file descriptor, with consistent semantics for both -/// Darwin and Linux. +/// Secure, symlink-safe filesystem operations anchored to a directory file descriptor, with +/// the same semantics on Darwin and Linux. /// -/// All operations use `openat`/`mkdirat`/`unlinkat` anchored to the supplied -/// file descriptor. Every path component is opened with `O_NOFOLLOW`, so a -/// symlink is never followed while walking a path, and every descriptor this -/// type opens or duplicates is close-on-exec so it is not inherited by child -/// processes. The type is never instantiated; it exists solely as a namespace. +/// Use these in place of path-based `FileManager` or `open(2)` calls whenever any part of a +/// path comes from somewhere you do not control, such as the member names of an archive or +/// the requests of a remote peer. A path-based call resolves the whole path again each time +/// it is used, and follows whatever symlinks it finds, so a symlink planted in the path, or +/// swapped in between a check and a use, can redirect it. +/// +/// ## Guarantees +/// +/// - Every operation is relative to a directory descriptor. Nothing here resolves a path from +/// the root of the file system. +/// - A symlink is never followed unless the caller explicitly asks for it with +/// ``SymlinkPolicy/followBeneath``. Even then, the link is resolved in this type, one +/// component at a time, against directory descriptors that are already open, and it can +/// never lead above the directory the walk started from. +/// - The primitives never remove or replace anything unless the caller asks for it by name: +/// ``unlink(_:_:)``, ``unlinkRecursive(_:filename:)``, or ``rename(_:_:to:_:)``. A composite says in +/// its documentation what it replaces. For example, ``mkdir(_:_:permissions:makeIntermediates:completion:)`` +/// replaces a file or symlink that is in the way of a directory it needs, and never removes a directory. +/// - Every descriptor this type opens or duplicates is close-on-exec, so it is not inherited +/// by child processes. +/// - Problems are reported as typed ``Error`` values, not as `errno` values that differ +/// between platforms. +/// +/// ## Two layers +/// +/// The **primitives**, in `FileDescriptorOps.swift`, each do one thing relative to a +/// directory descriptor and take no policy. If something is in the way, they report what. +/// +/// The **composites**, in `FileDescriptorOps+Composite.swift`, combine primitives into +/// sequences that several callers need and that are easy to get wrong, such as creating a +/// path and then working inside it, or replacing a file atomically. Where a composite has to +/// choose what to do about a symlink, it takes that choice as an explicit argument +/// (``SymlinkPolicy``). Composites use only the public primitives. +/// +/// ## Platform notes +/// +/// Safety here comes from opening each path component with `O_NOFOLLOW` against a pinned +/// directory descriptor. That works the same way everywhere. +/// +/// Where the kernel supports `O_RESOLVE_BENEATH`, it is also passed to `openat`, as a second +/// line of defense against a bug in this type's own path validation. It is available from +/// macOS 15.4 (`xnu-11417.101.15`) and not before, and this package supports macOS 15.0 and +/// later. Because its value is shared with another flag on older kernels, it is only ever +/// passed after an `#available` check. It is not used on Linux, where `openat2(2)` with +/// `RESOLVE_BENEATH` (Linux 5.6 and later) would be the equivalent. +/// +/// The type is never instantiated; it exists solely as a namespace. public enum FileDescriptorOps { // MARK: - Nested types @@ -153,7 +194,7 @@ public enum FileDescriptorOps { /// if it is a symlink, ``Error/conflict(_:)`` if it is some other kind of /// non-directory, and ``Error/systemError(_:_:)`` for anything else. public static func openDirectory(_ fd: FileDescriptor, _ name: FilePath.Component) throws -> FileDescriptor { - let newFd = openat(fd.rawValue, name.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) + let newFd = openat(fd.rawValue, name.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC | resolveBeneathFlag) if newFd >= 0 { return FileDescriptor(rawValue: newFd) } @@ -276,7 +317,7 @@ public enum FileDescriptorOps { /// if it is a symlink, ``Error/conflict(_:)`` if it is not a regular file, and /// ``Error/systemError(_:_:)`` for anything else. public static func openFile(_ fd: FileDescriptor, _ name: FilePath.Component) throws -> FileDescriptor { - let newFd = openat(fd.rawValue, name.string, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_NOCTTY | O_CLOEXEC) + let newFd = openat(fd.rawValue, name.string, O_RDONLY | O_NOFOLLOW | O_NONBLOCK | O_NOCTTY | O_CLOEXEC | resolveBeneathFlag) guard newFd >= 0 else { let openErrno = errno switch try entryType(fd, name) { @@ -324,7 +365,7 @@ public enum FileDescriptorOps { let newFd = openat( fd.rawValue, name.string, - O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, + O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC | resolveBeneathFlag, permissions?.rawValue ?? 0o644 ) guard newFd >= 0 else { @@ -425,7 +466,7 @@ public enum FileDescriptorOps { throw Error.systemError("file removal during file descriptor unlink", errno) } - let componentFd = openat(fd.rawValue, filename.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) + let componentFd = openat(fd.rawValue, filename.string, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC | resolveBeneathFlag) guard componentFd >= 0 else { throw Error.systemError("directory open during file descriptor unlink", errno) } @@ -520,6 +561,28 @@ public enum FileDescriptorOps { // MARK: - Private helpers + /// `O_RESOLVE_BENEATH` where the kernel supports it, and 0 everywhere else. + /// + /// With this flag the kernel refuses an `openat` whose path is absolute or would leave the + /// directory it is relative to. Every path passed to `openat` here is a single validated + /// component, so it changes nothing when the code is correct. It exists to turn a bug in + /// the validation into a failed open instead of an escape. + /// + /// `open(2)` and `openat(2)` honor the flag from macOS 15.4 (`xnu-11417.101.15`), and + /// not before. A refused path fails with `EACCES` on macOS 15.4 and later 15.x releases, and + /// with `ENOTCAPABLE` from macOS 26.0 (`xnu-12377.1.9`). Its value, `0x1000`, is `FMARK` on + /// older kernels, so the flag must not be passed to one: the `#available` check is the only + /// thing that makes it safe. The value is defined here, and not taken from the SDK, so this + /// builds with an SDK that predates the flag. + static var resolveBeneathFlag: Int32 { + #if canImport(Darwin) + if #available(macOS 15.4, *) { + return 0x1000 + } + #endif + return 0 + } + private static func enumerateHelper( _ fd: FileDescriptor, relativePath: FilePath, @@ -560,7 +623,7 @@ public enum FileDescriptorOps { // Open the child directory with O_NOFOLLOW to guarantee we are // entering a real directory and not a symlink that was swapped in // between readdir and here. - let childFd = openat(fd.rawValue, name, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC) + let childFd = openat(fd.rawValue, name, O_NOFOLLOW | O_RDONLY | O_DIRECTORY | O_CLOEXEC | resolveBeneathFlag) guard childFd >= 0 else { throw Error.systemError("openat during file descriptor enumerate", errno) } diff --git a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift index 541fe6f07..81c853750 100644 --- a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift +++ b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift @@ -31,6 +31,17 @@ import Glibc let os_close = Glibc.close #endif +#if canImport(Darwin) +/// Whether this macOS honors `O_RESOLVE_BENEATH` in `open`, which it does from macOS 15.4. +private let kernelHasResolveBeneath = ProcessInfo.processInfo.isOperatingSystemAtLeast( + OperatingSystemVersion(majorVersion: 15, minorVersion: 4, patchVersion: 0)) + +/// The errno a path refused by `O_RESOLVE_BENEATH` fails with: `EACCES` before macOS 26.0, and `ENOTCAPABLE` from it. +private let resolveBeneathRefusalErrno = + ProcessInfo.processInfo.isOperatingSystemAtLeast(OperatingSystemVersion(majorVersion: 26, minorVersion: 0, patchVersion: 0)) + ? ENOTCAPABLE : EACCES +#endif + struct FileDescriptorPathSecureTests { @Test( "Test creation of stub file under directory successfully created by secure mkdir", @@ -1198,6 +1209,35 @@ struct FileDescriptorPathSecureTests { #expect(reads > 0) } + // MARK: - O_RESOLVE_BENEATH + + #if canImport(Darwin) + @Test("Test the O_RESOLVE_BENEATH flag is used exactly where the kernel supports it") + func testResolveBeneathFlagMatchesOSVersion() { + #expect((FileDescriptorOps.resolveBeneathFlag != 0) == kernelHasResolveBeneath) + } + + @Test("Test the O_RESOLVE_BENEATH value defined here is the kernel's", .enabled(if: kernelHasResolveBeneath)) + func testResolveBeneathValueIsHonoredByTheKernel() throws { + let sandbox = try makeSandbox() + defer { sandbox.cleanup() } + let flag = FileDescriptorOps.resolveBeneathFlag + #expect(flag != 0) + + // A path that leaves the directory is refused by the kernel... + let escaped = openat(sandbox.rootFd.rawValue, "../outside", O_RDONLY | O_DIRECTORY | flag) + let escapedErrno = errno + if escaped >= 0 { close(escaped) } + #expect(escaped < 0) + #expect(escapedErrno == resolveBeneathRefusalErrno, "unexpected errno \(escapedErrno)") + + // ...and the same open without the flag works, so the refusal is due to the flag. + let control = openat(sandbox.rootFd.rawValue, "../outside", O_RDONLY | O_DIRECTORY) + #expect(control >= 0) + if control >= 0 { close(control) } + } + #endif + // MARK: - mkdir replaces what is in the way @Test("Test mkdir replaces a file or symlink that is in the way, and never writes through a symlink") From d2b17a35a0366dfaf60cbcb399efa3952e466668 Mon Sep 17 00:00:00 2001 From: John Logan Date: Fri, 9 Oct 2026 13:04:42 -0700 Subject: [PATCH 7/8] FileDescriptorOps: implement entryType in terms of status Review feedback on #957: entryType repeated the fstatat call and ENOENT handling already in status(_:_:), so delegate to it. --- Sources/ContainerizationOS/FileDescriptorOps.swift | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/Sources/ContainerizationOS/FileDescriptorOps.swift b/Sources/ContainerizationOS/FileDescriptorOps.swift index 0430b22ef..10cc3314a 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps.swift @@ -174,14 +174,7 @@ public enum FileDescriptorOps { /// - name: The name of a direct child of that directory. /// - Throws: `FileDescriptorOps.Error.systemError` if the entry cannot be inspected. public static func entryType(_ fd: FileDescriptor, _ name: FilePath.Component) throws -> EntryType? { - var stbuf = stat() - guard fstatat(fd.rawValue, name.string, &stbuf, AT_SYMLINK_NOFOLLOW) == 0 else { - if errno == ENOENT { - return nil - } - throw Error.systemError("stat during file descriptor entry type lookup", errno) - } - return entryType(forMode: stbuf.st_mode) + try status(fd, name)?.type } /// Opens the existing directory `name` in the directory `fd`, without following From 66c961736c88ba9141e076ae21bedba10013e2e8 Mon Sep 17 00:00:00 2001 From: John Logan Date: Fri, 9 Oct 2026 13:15:42 -0700 Subject: [PATCH 8/8] FileDescriptorOps: remove rename and replaceFile Review feedback on #957: replaceFile has no caller. The intended users (ArchiveReader, BuildFSSync, and docker-archive conversion) only create into fresh directories or read, so atomic replace is not needed yet. rename existed only to support replaceFile. Both can be added back when a caller needs them. --- .../FileDescriptorOps+Composite.swift | 44 ------ .../FileDescriptorOps.swift | 29 +--- .../FileDescriptorOpsTests.swift | 140 ------------------ 3 files changed, 2 insertions(+), 211 deletions(-) diff --git a/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift index 009ca62f5..a5cef1ff7 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps+Composite.swift @@ -229,50 +229,6 @@ extension FileDescriptorOps { return try atDirectory(directories[directories.count - 1]) } - /// Writes the file `name` in `directory` so that a reader sees either the old contents or the new - /// contents, never a partly written file. - /// - /// The new contents are written to a temporary file in the same directory, which is created - /// exclusively, and then renamed over `name`. If `name` is a symlink, the link itself is replaced - /// and its target is never written to. If `write` throws, the temporary file is removed and - /// whatever was at `name` is left as it was. - /// - /// The new file does not inherit the permissions or ownership of the file it replaces. It - /// is atomic with respect to other readers, but it does not flush to disk. - /// - /// - Parameters: - /// - directory: An open file descriptor for the directory that holds the file. - /// - name: The name of the file to write. - /// - permissions: The permissions to give the new file (default 0o644), subject to the umask. - /// - write: Writes the new contents to the descriptor it is given. It must not close the descriptor. - /// - Throws: Errors from `write`, and ``Error/systemError(_:_:)`` if the temporary file cannot be - /// created or renamed into place, for example because `name` is a non-empty directory. - public static func replaceFile( - in directory: FileDescriptor, - named name: FilePath.Component, - permissions: FilePermissions? = nil, - write: (FileDescriptor) throws -> Void - ) throws { - guard let temporaryName = FilePath.Component(".tmp-" + String(UInt64.random(in: .min ... .max), radix: 16)) else { - throw Error.invalidPathComponent - } - - let file = try createFile(directory, temporaryName, permissions: permissions) - var isOpen = true - do { - try write(file) - isOpen = false - try file.close() - try rename(directory, temporaryName, to: directory, name) - } catch { - if isOpen { - try? file.close() - } - try? unlink(directory, temporaryName) - throw error - } - } - private static func openOrCreateDirectory( _ parent: FileDescriptor, _ name: FilePath.Component, diff --git a/Sources/ContainerizationOS/FileDescriptorOps.swift b/Sources/ContainerizationOS/FileDescriptorOps.swift index 10cc3314a..e72d105d5 100644 --- a/Sources/ContainerizationOS/FileDescriptorOps.swift +++ b/Sources/ContainerizationOS/FileDescriptorOps.swift @@ -62,7 +62,7 @@ private func dupCloseOnExec(_ fd: Int32) -> Int32 { /// component at a time, against directory descriptors that are already open, and it can /// never lead above the directory the walk started from. /// - The primitives never remove or replace anything unless the caller asks for it by name: -/// ``unlink(_:_:)``, ``unlinkRecursive(_:filename:)``, or ``rename(_:_:to:_:)``. A composite says in +/// ``unlink(_:_:)`` or ``unlinkRecursive(_:filename:)``. A composite says in /// its documentation what it replaces. For example, ``mkdir(_:_:permissions:makeIntermediates:completion:)`` /// replaces a file or symlink that is in the way of a directory it needs, and never removes a directory. /// - Every descriptor this type opens or duplicates is close-on-exec, so it is not inherited @@ -77,7 +77,7 @@ private func dupCloseOnExec(_ fd: Int32) -> Int32 { /// /// The **composites**, in `FileDescriptorOps+Composite.swift`, combine primitives into /// sequences that several callers need and that are easy to get wrong, such as creating a -/// path and then working inside it, or replacing a file atomically. Where a composite has to +/// path and then working inside it, or opening a file beneath a directory. Where a composite has to /// choose what to do about a symlink, it takes that choice as an explicit argument /// (``SymlinkPolicy``). Composites use only the public primitives. /// @@ -248,31 +248,6 @@ public enum FileDescriptorOps { } } - /// Renames the entry `fromName` in the directory `fromDirectory` to `toName` in the directory - /// `toDirectory`. The two directories may be the same. - /// - /// The rename is atomic, and neither name is followed: a symlink is renamed, not its target. - /// - /// **This replaces an existing destination.** If `toName` exists and is not a directory, it is - /// replaced in one step, and a symlink is replaced rather than written through. As for - /// `rename(2)`, a directory may only replace an empty directory, and not the reverse. - /// - /// - Throws: ``Error/notFound`` if `fromName` does not exist, and ``Error/systemError(_:_:)`` - /// for anything else. - public static func rename( - _ fromDirectory: FileDescriptor, - _ fromName: FilePath.Component, - to toDirectory: FileDescriptor, - _ toName: FilePath.Component - ) throws { - guard renameat(fromDirectory.rawValue, fromName.string, toDirectory.rawValue, toName.string) == 0 else { - if errno == ENOENT { - throw Error.notFound - } - throw Error.systemError("rename during file descriptor rename", errno) - } - } - /// Returns the metadata of an open descriptor. /// /// - Throws: ``Error/systemError(_:_:)`` if the descriptor cannot be inspected. diff --git a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift index 81c853750..23e91e432 100644 --- a/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift +++ b/Tests/ContainerizationOSTests/FileDescriptorOpsTests.swift @@ -1069,146 +1069,6 @@ struct FileDescriptorPathSecureTests { #expect(opened > 0, "the test never managed to open the file, so it proved nothing") } - // MARK: - rename and replaceFile - - private func writeAll(_ bytes: [UInt8], to fd: FileDescriptor) throws { - var offset = 0 - while offset < bytes.count { - let written = bytes[offset...].withUnsafeBytes { write(fd.rawValue, $0.baseAddress, $0.count) } - guard written > 0 else { - throw Errno(rawValue: errno) - } - offset += written - } - } - - private func readEverything(_ fd: FileDescriptor) -> [UInt8] { - var result = [UInt8]() - var buffer = [UInt8](repeating: 0, count: 65536) - while true { - let count = read(fd.rawValue, &buffer, buffer.count) - guard count > 0 else { - return result - } - result.append(contentsOf: buffer.prefix(count)) - } - } - - @Test("Test rename moves entries atomically, replaces a non-directory destination, and never follows a symlink") - func testRename() throws { - let sandbox = try makeSandbox() - defer { sandbox.cleanup() } - try FileManager.default.createDirectory(atPath: sandbox.root.appending("sub").string, withIntermediateDirectories: false) - try writeText("one", to: sandbox.root.appending("a")) - try writeText("old", to: sandbox.root.appending("b")) - try writeText("target", to: sandbox.outside.appending("target")) - try link("l", to: "../outside/target", in: sandbox.root) - try writeText("two", to: sandbox.root.appending("c")) - let subFd = try FileDescriptorOps.openDirectory(sandbox.rootFd, "sub") - defer { try? subFd.close() } - - // Within one directory, replacing an existing file. - try FileDescriptorOps.rename(sandbox.rootFd, "a", to: sandbox.rootFd, "b") - #expect(try String(contentsOfFile: sandbox.root.appending("b").string, encoding: .utf8) == "one") - #expect(!FileManager.default.fileExists(atPath: sandbox.root.appending("a").string)) - - // Across directories. - try FileDescriptorOps.rename(sandbox.rootFd, "c", to: subFd, "moved") - #expect(try String(contentsOfFile: sandbox.root.appending("sub/moved").string, encoding: .utf8) == "two") - - // A symlink is moved, not its target, and a symlink destination is replaced, not written through. - try FileDescriptorOps.rename(sandbox.rootFd, "l", to: subFd, "l2") - #expect(try FileDescriptorOps.readSymlink(subFd, "l2") == "../outside/target") - try FileDescriptorOps.rename(subFd, "moved", to: subFd, "l2") - #expect(try FileDescriptorOps.entryType(subFd, "l2") == .regular) - #expect(try String(contentsOfFile: sandbox.outside.appending("target").string, encoding: .utf8) == "target") - - #expect(throws: FileDescriptorOps.Error.notFound) { try FileDescriptorOps.rename(sandbox.rootFd, "missing", to: sandbox.rootFd, "x") } - } - - @Test("Test replaceFile creates a file, replaces a file or symlink, and cleans up when the writer fails") - func testReplaceFile() throws { - let sandbox = try makeSandbox() - defer { sandbox.cleanup() } - try writeText("keep", to: sandbox.outside.appending("target")) - try writeText("old", to: sandbox.root.appending("existing")) - try link("viaLink", to: "../outside/target", in: sandbox.root) - - try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "fresh", permissions: FilePermissions(rawValue: 0o600)) { fd in - try writeAll(Array("brand new".utf8), to: fd) - } - #expect(try String(contentsOfFile: sandbox.root.appending("fresh").string, encoding: .utf8) == "brand new") - let freshStatus = try #require(try FileDescriptorOps.status(sandbox.rootFd, "fresh")) - #expect(freshStatus.permissions.rawValue & 0o777 == 0o600) - - try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "existing") { fd in try writeAll(Array("updated".utf8), to: fd) } - #expect(try String(contentsOfFile: sandbox.root.appending("existing").string, encoding: .utf8) == "updated") - - // The link is replaced, and its target is never written to. - try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "viaLink") { fd in try writeAll(Array("PWNED".utf8), to: fd) } - #expect(try FileDescriptorOps.entryType(sandbox.rootFd, "viaLink") == .regular) - #expect(try String(contentsOfFile: sandbox.outside.appending("target").string, encoding: .utf8) == "keep") - - struct WriterFailed: Swift.Error {} - #expect(throws: WriterFailed.self) { - try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "existing") { fd in - try writeAll(Array("partial".utf8), to: fd) - throw WriterFailed() - } - } - #expect(try String(contentsOfFile: sandbox.root.appending("existing").string, encoding: .utf8) == "updated", "a failed write leaves the old file") - let leftovers = try FileManager.default.contentsOfDirectory(atPath: sandbox.root.string).filter { $0.hasPrefix(".tmp-") } - #expect(leftovers.isEmpty, "temporary files must be removed: \(leftovers)") - } - - @Test("Test replaceFile never lets a reader see a partly written file") - func testReplaceFileIsAtomicForReaders() throws { - let sandbox = try makeSandbox() - defer { sandbox.cleanup() } - let size = 256 * 1024 - try FileDescriptorOps.replaceFile(in: sandbox.rootFd, named: "data") { fd in try writeAll([UInt8](repeating: 65, count: size), to: fd) } - - let stop = StopFlag() - let finished = DispatchSemaphore(value: 0) - let rootFd = sandbox.rootFd - DispatchQueue.global().async { - var letter: UInt8 = 66 - while !stop.isSet { - try? FileDescriptorOps.replaceFile(in: rootFd, named: "data") { fd in - var offset = 0 - let bytes = [UInt8](repeating: letter, count: size) - while offset < bytes.count { - let written = bytes[offset...].withUnsafeBytes { write(fd.rawValue, $0.baseAddress, $0.count) } - if written <= 0 { throw Errno(rawValue: errno) } - offset += written - } - } - letter = letter == 66 ? 65 : 66 - } - finished.signal() - } - - var torn = 0 - var reads = 0 - let deadline = Date().addingTimeInterval(1.0) - while Date() < deadline { - guard let fd = try? FileDescriptorOps.openFile(rootFd, relativePath: "data", symlinks: .refuse) else { - continue - } - let bytes = readEverything(fd) - try? fd.close() - reads += 1 - if bytes.count != size || Set(bytes).count != 1 { - torn += 1 - } - } - stop.set() - finished.wait() - - #expect(torn == 0, "\(torn) of \(reads) reads saw a partly written or mixed file") - #expect(reads > 0) - } - // MARK: - O_RESOLVE_BENEATH #if canImport(Darwin)