Skip to content

Commit 22aa4fd

Browse files
committed
Improve flexibility of sandbox path rules, and make sandbox profile a struct that's easier to manage
Turn the stateless `Sandbox` enum into a `SandboxProfile` struct that's a bit more flexible: - there is a list of path rules allowing a mixed order of `writable`, `readonly`, and `noaccess` rules; this addresses an issue where writable directories were always applied after readable directories, preventing arrangements where, for example, a source directory inside the temporary directory was properly made readonly - the defaults are built into the initializer rather than a separate `strictness` parameter — this means that they can be queried once the SandboxProfile has been created and are expressed in terms of the choices available to all sandbox profiles - it's a bit more ergonomic (as a struct, it can be copied, mutated, and carried around in a property before being used), and an optional `SandboxProfile` can be used to indicate both whether or not to apply a profile and if so what to apply. Having the sandbox profile be a struct that generates the platform specifics as needed also could also provide a place with which to associate any cached/compiled representation for profiles that are largely static, for any platform that supports that. As cleanup, this commit also removes the pre-SwiftPM 5.3 specialities which were specific to running the package manifest in an interpreter rather than compiling and executing it. This functionality is no longer relevant since it isn't possible to run any manifest in the interpreter, no matter what tools version the package specifies. In this commit, the call sites have been adjusted so that they use the modified SandboxProfile API, but they still construct the profiles on-the-fly as before. A future change will make them instead be configured at an early point and then applied later when the sandboxed process is actually launched. Another future change could add the sandbox profile to `Process` as a property so that there is no API assumption that applying a sandbox necessarily involves just modifying the command line.
1 parent cde76ea commit 22aa4fd

File tree

9 files changed

+214
-192
lines changed

9 files changed

+214
-192
lines changed

Diff for: Sources/Basics/CMakeLists.txt

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# This source file is part of the Swift open source project
22
#
3-
# Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors
3+
# Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
44
# Licensed under Apache License v2.0 with Runtime Library Exception
55
#
66
# See http://swift.org/LICENSE.txt for license information
@@ -26,7 +26,7 @@ add_library(Basics
2626
NSLock+Extensions.swift
2727
Observability.swift
2828
SQLite.swift
29-
Sandbox.swift
29+
SandboxProfile.swift
3030
String+Extensions.swift
3131
Triple+Extensions.swift
3232
SwiftVersion.swift

Diff for: Sources/Basics/Sandbox.swift

-144
This file was deleted.

Diff for: Sources/Basics/SandboxProfile.swift

+95
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
/*
2+
This source file is part of the Swift.org open source project
3+
4+
Copyright (c) 2021 - 2022 Apple Inc. and the Swift project authors
5+
Licensed under Apache License v2.0 with Runtime Library Exception
6+
7+
See http://swift.org/LICENSE.txt for license information
8+
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
9+
*/
10+
11+
import Foundation
12+
import TSCBasic
13+
14+
/// A sandbox profile representing the desired restrictions. The implementation can vary between platforms. Currently
15+
/// the only control a client has is in the path rules, but in the future there should also be options for controlling
16+
/// blocking of network access and process launching.
17+
public struct SandboxProfile: Equatable {
18+
/// An ordered list of path rules, where the last rule to cover a particular path "wins". These will be resolved
19+
/// to absolute paths at the time the profile is applied. They are applied after any of the implicit directories
20+
/// referenced by other sandbox profile settings.
21+
public var pathAccessRules: [PathAccessRule]
22+
23+
/// Represents a rule for access to a path and everything under it.
24+
public enum PathAccessRule: Equatable {
25+
case noaccess(AbsolutePath)
26+
case readonly(AbsolutePath)
27+
case writable(AbsolutePath)
28+
}
29+
30+
/// Configures a SandboxProfile for blocking network access and writing to the file system except to specifically
31+
/// permitted locations.
32+
public init(_ pathAccessRules: [PathAccessRule] = []) {
33+
self.pathAccessRules = pathAccessRules
34+
}
35+
}
36+
37+
extension SandboxProfile {
38+
/// Applies the sandbox profile to the given command line (if the platform supports it), and returns the modified
39+
/// command line. On platforms that don't support sandboxing, the unmodified command line is returned.
40+
public func apply(to command: [String]) throws -> [String] {
41+
#if os(macOS)
42+
return ["/usr/bin/sandbox-exec", "-p", try self.generateMacOSSandboxProfileString()] + command
43+
#else
44+
// rdar://40235432, rdar://75636874 tracks implementing sandboxes for other platforms.
45+
return command
46+
#endif
47+
}
48+
}
49+
50+
// MARK: - macOS
51+
52+
#if os(macOS)
53+
fileprivate extension SandboxProfile {
54+
/// Private function that generates a Darwin sandbox profile suitable for passing to `sandbox-exec(1)`.
55+
func generateMacOSSandboxProfileString() throws -> String {
56+
var contents = "(version 1)\n"
57+
58+
// Deny everything by default.
59+
contents += "(deny default)\n"
60+
61+
// Import the system sandbox profile.
62+
contents += "(import \"system.sb\")\n"
63+
64+
// Allow operations on subprocesses.
65+
contents += "(allow process*)\n"
66+
67+
// Allow reading any file that isn't protected by TCC or permissions (ideally we'd only allow a specific set
68+
// of readable locations, and can maybe tighten this in the future).
69+
contents += "(allow file-read*)\n"
70+
71+
// Apply customized rules for specific file system locations. Everything is readonly by default, so we just
72+
// either allow or deny writing, in order. Later rules have precedence over earlier rules.
73+
for rule in pathAccessRules {
74+
switch rule {
75+
case .noaccess(let path):
76+
contents += "(deny file-* (subpath \(try resolveSymlinksAndQuotePath(path))))\n"
77+
case .readonly(let path):
78+
contents += "(deny file-write* (subpath \(try resolveSymlinksAndQuotePath(path))))\n"
79+
case .writable(let path):
80+
contents += "(allow file-write* (subpath \(try resolveSymlinksAndQuotePath(path))))\n"
81+
}
82+
}
83+
return contents
84+
}
85+
86+
/// Private helper function that resolves an AbsolutePath and returns it as a string quoted for use as a subpath
87+
/// in a .sb sandbox profile.
88+
func resolveSymlinksAndQuotePath(_ path: AbsolutePath) throws -> String {
89+
return try "\"" + resolveSymlinks(path).pathString
90+
.replacingOccurrences(of: "\\", with: "\\\\")
91+
.replacingOccurrences(of: "\"", with: "\\\"")
92+
+ "\""
93+
}
94+
}
95+
#endif

Diff for: Sources/Build/BuildOperation.swift

+5-1
Original file line numberDiff line numberDiff line change
@@ -554,7 +554,11 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
554554
// TODO: We need to also use any working directory, but that support isn't yet available on all platforms at a lower level.
555555
var commandLine = [command.configuration.executable.pathString] + command.configuration.arguments
556556
if !self.disableSandboxForPluginCommands {
557-
commandLine = try Sandbox.apply(command: commandLine, strictness: .writableTemporaryDirectory, writableDirectories: [pluginResult.pluginOutputDirectory])
557+
// Allow access to the plugin's output directory as well as to the local temporary directory.
558+
let sandboxProfile = SandboxProfile([
559+
.writable(pluginResult.pluginOutputDirectory),
560+
.writable(try self.fileSystem.tempDirectory)])
561+
commandLine = try sandboxProfile.apply(to: commandLine)
558562
}
559563
let processResult = try TSCBasic.Process.popen(arguments: commandLine, environment: command.configuration.environment)
560564
let output = try processResult.utf8Output() + processResult.utf8stderrOutput()

Diff for: Sources/Build/LLBuildManifestBuilder.swift

+6-1
Original file line numberDiff line numberDiff line change
@@ -621,8 +621,13 @@ extension LLBuildManifestBuilder {
621621
let uniquedName = ([execPath.pathString] + command.configuration.arguments).joined(separator: "|")
622622
let displayName = command.configuration.displayName ?? execPath.basename
623623
var commandLine = [execPath.pathString] + command.configuration.arguments
624+
625+
// Apply the sandbox, if appropriate.
624626
if !self.disableSandboxForPluginCommands {
625-
commandLine = try Sandbox.apply(command: commandLine, strictness: .writableTemporaryDirectory, writableDirectories: [result.pluginOutputDirectory])
627+
let sandboxProfile = SandboxProfile([
628+
.writable(result.pluginOutputDirectory),
629+
.writable(try self.fileSystem.tempDirectory)])
630+
commandLine = try sandboxProfile.apply(to: commandLine)
626631
}
627632
manifest.addShellCmd(
628633
name: displayName + "-" + ByteString(encodingAsUTF8: uniquedName).sha256Checksum,

Diff for: Sources/PackageLoading/ManifestLoader.swift

+21-3
Original file line numberDiff line numberDiff line change
@@ -635,10 +635,28 @@ public final class ManifestLoader: ManifestLoaderProtocol {
635635
// This provides some safety against arbitrary code execution when parsing manifest files.
636636
// We only allow the permissions which are absolutely necessary.
637637
if self.isManifestSandboxEnabled {
638-
let cacheDirectories = [self.databaseCacheDir, moduleCachePath].compactMap{ $0 }
639-
let strictness: Sandbox.Strictness = toolsVersion < .v5_3 ? .manifest_pre_53 : .default
640638
do {
641-
cmd = try Sandbox.apply(command: cmd, strictness: strictness, writableDirectories: cacheDirectories)
639+
var sandbox = SandboxProfile()
640+
641+
// Allow writing inside the temporary directory.
642+
try sandbox.pathAccessRules.append(.writable(localFileSystem.tempDirectory))
643+
644+
// Allow writing in the database cache directory, if we have one.
645+
if let databaseCacheDir = self.databaseCacheDir {
646+
sandbox.pathAccessRules.append(.writable(databaseCacheDir))
647+
}
648+
649+
// Allow writing in the module cache path, if there is one.
650+
if let moduleCachePath = moduleCachePath {
651+
sandbox.pathAccessRules.append(.writable(moduleCachePath))
652+
}
653+
654+
// But do not allow writing in the directory that contains the manifest, even if it is
655+
// inside one of the otherwise writable directories.
656+
sandbox.pathAccessRules.append(.readonly(manifestPath.parentDirectory))
657+
658+
// Finally apply the sandbox.
659+
cmd = try sandbox.apply(to: cmd)
642660
} catch {
643661
return completion(.failure(error))
644662
}

Diff for: Sources/Workspace/DefaultPluginScriptRunner.swift

+16-1
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,22 @@ public struct DefaultPluginScriptRunner: PluginScriptRunner, Cancellable {
423423
// Optionally wrap the command in a sandbox, which places some limits on what it can do. In particular, it blocks network access and restricts the paths to which the plugin can make file system changes. It does allow writing to temporary directories.
424424
if self.enableSandbox {
425425
do {
426-
command = try Sandbox.apply(command: command, strictness: .writableTemporaryDirectory, writableDirectories: writableDirectories + [self.cacheDir], readOnlyDirectories: readOnlyDirectories)
426+
var sandbox = SandboxProfile()
427+
428+
// Allow writing in the temporary directory (whether or not it's overridden by TMPDIR).
429+
sandbox.pathAccessRules.append(.writable(try self.fileSystem.tempDirectory))
430+
431+
// But prevent writing in any read-only directories.
432+
sandbox.pathAccessRules.append(contentsOf: readOnlyDirectories.map{ .readonly($0) })
433+
434+
// But allow writing in any writable directories.
435+
sandbox.pathAccessRules.append(contentsOf: writableDirectories.map{ .writable($0) })
436+
437+
// And always allow writing to the cache directory, even if it is inside one of the readonly directories.
438+
sandbox.pathAccessRules.append(.writable(self.cacheDir))
439+
440+
// Apply the sandbox to the command.
441+
command = try sandbox.apply(to: command)
427442
} catch {
428443
return callbackQueue.async {
429444
completion(.failure(error))

0 commit comments

Comments
 (0)