diff --git a/Sources/Containerization/BridgeManager.swift b/Sources/Containerization/BridgeManager.swift index c117d197..9b108716 100644 --- a/Sources/Containerization/BridgeManager.swift +++ b/Sources/Containerization/BridgeManager.swift @@ -72,6 +72,12 @@ public struct BridgeManager: Sendable { /// created without external connectivity, leaving host firewall /// policy untouched. /// - logger: optional logger. Defaults to a `bridge`-labeled logger. + /// + /// Use ``make(name:subnet:gateway:mtu:egressInterface:enableNAT:logger:)`` + /// to validate interface names eagerly and surface invalid input as a + /// ``ContainerizationError``. `create()` and `delete()` also validate + /// before touching host state, so existing `init` callers do not trap on + /// invalid names. public init( name: String, subnet: CIDRv4, @@ -81,11 +87,58 @@ public struct BridgeManager: Sendable { enableNAT: Bool = false, logger: Logger? = nil ) { - self.name = Self.validateInterfaceName(name) + self.name = name self.subnet = subnet self.gateway = gateway ?? subnet.gateway self.mtu = mtu - self.egressInterface = egressInterface.map(Self.validateInterfaceName) + self.egressInterface = egressInterface + self.enableNAT = enableNAT + self.log = logger ?? Logger(label: "com.apple.containerization.bridge") + } + + /// Create a manager after validating user-provided interface names. + /// + /// - Throws: ``ContainerizationError`` with code `.invalidArgument` when + /// `name` or `egressInterface` is empty, longer than 15 bytes, or + /// contains whitespace, `/`, `:`, or NUL. + public static func make( + name: String, + subnet: CIDRv4, + gateway: IPv4Address? = nil, + mtu: UInt32 = 1500, + egressInterface: String? = nil, + enableNAT: Bool = false, + logger: Logger? = nil + ) throws -> Self { + let validatedName = try Self.validateInterfaceName(name, parameter: "name") + let validatedEgress = try egressInterface.map { + try Self.validateInterfaceName($0, parameter: "egressInterface") + } + return Self( + validatedName: validatedName, + subnet: subnet, + gateway: gateway ?? subnet.gateway, + mtu: mtu, + validatedEgressInterface: validatedEgress, + enableNAT: enableNAT, + logger: logger + ) + } + + private init( + validatedName name: String, + subnet: CIDRv4, + gateway: IPv4Address, + mtu: UInt32, + validatedEgressInterface egressInterface: String?, + enableNAT: Bool, + logger: Logger? + ) { + self.name = name + self.subnet = subnet + self.gateway = gateway + self.mtu = mtu + self.egressInterface = egressInterface self.enableNAT = enableNAT self.log = logger ?? Logger(label: "com.apple.containerization.bridge") } @@ -94,38 +147,67 @@ public struct BridgeManager: Sendable { /// `iptables`. This is a defense-in-depth check; the kernel and /// `iptables` themselves will also reject pathological inputs, but doing /// it here surfaces the error in a callable Swift API rather than as a - /// netlink rc or iptables exit. Asserts (rather than throws) — these - /// constraints are static, so a violation is a programming error. - private static func validateInterfaceName(_ name: String) -> String { + /// netlink rc or iptables exit. + private static func validateInterfaceName(_ name: String, parameter: String) throws -> String { // IFNAMSIZ on Linux is 16 (15 usable + NUL). iptables itself caps - // at 15. Names with `/`, whitespace, or NUL are kernel-rejected. - precondition(!name.isEmpty, "interface name must be non-empty") - precondition(name.utf8.count <= 15, "interface name '\(name)' exceeds IFNAMSIZ-1 (15)") - precondition( - !name.contains(where: { $0.isWhitespace || $0 == "/" || $0 == "\0" || $0 == ":" }), - "interface name '\(name)' contains invalid characters" - ) + // at 15. Names with `/`, `:`, whitespace, or NUL are rejected. + guard !name.isEmpty else { + throw ContainerizationError( + .invalidArgument, + message: "\(parameter) must be non-empty" + ) + } + guard name.utf8.count <= 15 else { + throw ContainerizationError( + .invalidArgument, + message: "\(parameter) '\(name)' exceeds IFNAMSIZ-1 (15)" + ) + } + guard !name.contains(where: { $0.isWhitespace || $0 == "/" || $0 == "\0" || $0 == ":" }) else { + throw ContainerizationError( + .invalidArgument, + message: "\(parameter) '\(name)' contains invalid characters (whitespace, '/', ':', or NUL)" + ) + } return name } /// Idempotent create. public func create() throws { + let validated = try validated() try Self.ensureStateDirectory() - let lock = try FileLock(path: Self.lockPath(for: name)) + let lock = try FileLock(path: Self.lockPath(for: validated.name)) try lock.withExclusive { - try createLocked() + try validated.createLocked() } } /// Idempotent delete. No-op when the bridge does not exist. public func delete() throws { + let validated = try validated() try Self.ensureStateDirectory() - let lock = try FileLock(path: Self.lockPath(for: name)) + let lock = try FileLock(path: Self.lockPath(for: validated.name)) try lock.withExclusive { - try deleteLocked() + try validated.deleteLocked() } } + private func validated() throws -> Self { + let validatedName = try Self.validateInterfaceName(name, parameter: "name") + let validatedEgress = try egressInterface.map { + try Self.validateInterfaceName($0, parameter: "egressInterface") + } + return Self( + validatedName: validatedName, + subnet: subnet, + gateway: gateway, + mtu: mtu, + validatedEgressInterface: validatedEgress, + enableNAT: enableNAT, + logger: log + ) + } + private func createLocked() throws { let session = NetlinkSession(socket: try DefaultNetlinkSocket(), log: log) let stateURL = URL(fileURLWithPath: Self.statePath(for: name)) diff --git a/Sources/cctl/BridgeCommand.swift b/Sources/cctl/BridgeCommand.swift index 2714acc2..fb1a7c08 100644 --- a/Sources/cctl/BridgeCommand.swift +++ b/Sources/cctl/BridgeCommand.swift @@ -59,7 +59,7 @@ extension Application { func run() async throws { let cidr = try CIDRv4(subnet) let gw = try gateway.map { try IPv4Address($0) } - let mgr = BridgeManager( + let mgr = try BridgeManager.make( name: name, subnet: cidr, gateway: gw, @@ -86,7 +86,7 @@ extension Application { func run() async throws { let cidr = try CIDRv4(subnet) - let mgr = BridgeManager(name: name, subnet: cidr, logger: log) + let mgr = try BridgeManager.make(name: name, subnet: cidr, logger: log) try mgr.delete() } } diff --git a/Sources/cctl/RunCommand.swift b/Sources/cctl/RunCommand.swift index cc5a00a4..c5fb9974 100644 --- a/Sources/cctl/RunCommand.swift +++ b/Sources/cctl/RunCommand.swift @@ -403,7 +403,7 @@ extension Application { let subnetCIDR = try CIDRv4(subnet) let gw = try bridgeGateway.map { try IPv4Address($0) } - let mgr = BridgeManager( + let mgr = try BridgeManager.make( name: bridge, subnet: subnetCIDR, gateway: gw, diff --git a/Tests/ContainerizationTests/BridgeManagerTests.swift b/Tests/ContainerizationTests/BridgeManagerTests.swift new file mode 100644 index 00000000..aad7bd60 --- /dev/null +++ b/Tests/ContainerizationTests/BridgeManagerTests.swift @@ -0,0 +1,84 @@ +//===----------------------------------------------------------------------===// +// 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. +//===----------------------------------------------------------------------===// + +#if os(Linux) +import ContainerizationError +import ContainerizationExtras +import Testing + +@testable import Containerization + +@Suite("BridgeManager") +struct BridgeManagerTests { + @Test(arguments: [ + ("", "name must be non-empty"), + ("reallyreallylongbridgename", "exceeds IFNAMSIZ-1 (15)"), + ("bad/name", "contains invalid characters"), + ("bad name", "contains invalid characters"), + ("bad:name", "contains invalid characters"), + ("bad\u{0000}name", "contains invalid characters"), + ]) + func makeRejectsInvalidBridgeName(name: String, expectedMessage: String) throws { + let subnet = try CIDRv4("192.168.64.0/24") + + do { + _ = try BridgeManager.make(name: name, subnet: subnet) + #expect(Bool(false), "expected invalidArgument for bridge name '\(name)'") + } catch let error as ContainerizationError { + #expect(error.code == .invalidArgument) + #expect(error.message.contains(expectedMessage)) + } catch { + #expect(Bool(false), "unexpected error: \(error)") + } + } + + @Test(arguments: [ + "", + "reallyreallylongbridgename", + "bad/name", + "bad name", + ]) + func initDoesNotTrapOnInvalidBridgeName(name: String) throws { + let subnet = try CIDRv4("192.168.64.0/24") + let manager = BridgeManager(name: name, subnet: subnet) + + #expect(manager.name == name) + #expect(manager.subnet == subnet) + } + + @Test(arguments: [ + ("", "egressInterface must be non-empty"), + ("reallyreallylongegress", "exceeds IFNAMSIZ-1 (15)"), + ("bad/name", "contains invalid characters"), + ("bad name", "contains invalid characters"), + ("bad:name", "contains invalid characters"), + ("bad\u{0000}name", "contains invalid characters"), + ]) + func makeRejectsInvalidEgressInterface(name: String, expectedMessage: String) throws { + let subnet = try CIDRv4("192.168.64.0/24") + + do { + _ = try BridgeManager.make(name: "cz0", subnet: subnet, egressInterface: name) + #expect(Bool(false), "expected invalidArgument for egress interface '\(name)'") + } catch let error as ContainerizationError { + #expect(error.code == .invalidArgument) + #expect(error.message.contains(expectedMessage)) + } catch { + #expect(Bool(false), "unexpected error: \(error)") + } + } +} +#endif