From fc269d0e1922e189cf8f74f676562763cd348242 Mon Sep 17 00:00:00 2001 From: Maxime Grenu Date: Thu, 5 Mar 2026 09:11:49 +0100 Subject: [PATCH 1/5] fix(dns): validate nameserver addresses are valid IP addresses Resolves apple/containerization#467. Previously, any arbitrary string could be passed as a nameserver in DNS configuration, which would silently result in an invalid /etc/resolv.conf inside the container. This change adds a DNS.validate() method that ensures every nameserver string is a valid IPv4 or IPv6 address (using the existing ContainerizationExtras parsers). The method is called from Vminitd.configureDNS() before applying the configuration. Tests added to DNSTests.swift covering valid IPv4, IPv6, mixed, empty nameserver lists, and invalid hostname/address rejection. Signed-off-by: Maxime Grenu --- .../Containerization/DNSConfiguration.swift | 22 ++++++++++++++ Sources/Containerization/Vminitd.swift | 1 + Tests/ContainerizationTests/DNSTests.swift | 30 +++++++++++++++++++ 3 files changed, 53 insertions(+) diff --git a/Sources/Containerization/DNSConfiguration.swift b/Sources/Containerization/DNSConfiguration.swift index 8c9e601f..dc0abe47 100644 --- a/Sources/Containerization/DNSConfiguration.swift +++ b/Sources/Containerization/DNSConfiguration.swift @@ -14,6 +14,8 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerizationExtras + /// DNS configuration for a container. The values will be used to /// construct /etc/resolv.conf for a given container. public struct DNS: Sendable { @@ -41,6 +43,26 @@ public struct DNS: Sendable { self.searchDomains = searchDomains self.options = options } + + /// Validates the DNS configuration. + /// + /// Ensures that all nameserver entries are valid IPv4 or IPv6 addresses. + /// Arbitrary hostnames are not permitted as nameservers. + /// + /// - Throws: ``ContainerizationError`` with code `.invalidArgument` if + /// any nameserver is not a valid IP address. + public func validate() throws { + for nameserver in nameservers { + let isValidIPv4 = (try? IPv4Address(nameserver)) != nil + let isValidIPv6 = (try? IPv6Address(nameserver)) != nil + if !isValidIPv4 && !isValidIPv6 { + throw ContainerizationError( + .invalidArgument, + message: "nameserver '\(nameserver)' is not a valid IPv4 or IPv6 address" + ) + } + } + } } extension DNS { diff --git a/Sources/Containerization/Vminitd.swift b/Sources/Containerization/Vminitd.swift index 0ac47b7a..02aab2e2 100644 --- a/Sources/Containerization/Vminitd.swift +++ b/Sources/Containerization/Vminitd.swift @@ -408,6 +408,7 @@ extension Vminitd { /// Configure DNS within the sandbox's environment. public func configureDNS(config: DNS, location: String) async throws { + try config.validate() _ = try await client.configureDns( .with { $0.location = location diff --git a/Tests/ContainerizationTests/DNSTests.swift b/Tests/ContainerizationTests/DNSTests.swift index a003b919..fe866b97 100644 --- a/Tests/ContainerizationTests/DNSTests.swift +++ b/Tests/ContainerizationTests/DNSTests.swift @@ -51,4 +51,34 @@ struct DNSTests { let expected = "nameserver 8.8.8.8\n" #expect(dns.resolvConf == expected) } + + @Test func dnsValidateAcceptsValidIPv4Nameservers() throws { + let dns = DNS(nameservers: ["8.8.8.8", "1.1.1.1"]) + #expect(throws: Never.self) { try dns.validate() } + } + + @Test func dnsValidateAcceptsValidIPv6Nameservers() throws { + let dns = DNS(nameservers: ["2001:4860:4860::8888", "::1"]) + #expect(throws: Never.self) { try dns.validate() } + } + + @Test func dnsValidateAcceptsMixedIPv4AndIPv6Nameservers() throws { + let dns = DNS(nameservers: ["8.8.8.8", "2001:4860:4860::8844"]) + #expect(throws: Never.self) { try dns.validate() } + } + + @Test func dnsValidateAcceptsEmptyNameservers() throws { + let dns = DNS(nameservers: []) + #expect(throws: Never.self) { try dns.validate() } + } + + @Test func dnsValidateRejectsHostname() { + let dns = DNS(nameservers: ["dns.example.com"]) + #expect(throws: (any Error).self) { try dns.validate() } + } + + @Test func dnsValidateRejectsInvalidAddress() { + let dns = DNS(nameservers: ["not-an-ip"]) + #expect(throws: (any Error).self) { try dns.validate() } + } } From caaab2524fa9835a4e4f41116331a38312084122 Mon Sep 17 00:00:00 2001 From: Maxime Grenu Date: Fri, 6 Mar 2026 16:03:41 +0100 Subject: [PATCH 2/5] fix(dns): add missing import ContainerizationError The validate() method uses ContainerizationError which lives in its own module and must be explicitly imported. Signed-off-by: Maxime Grenu --- Sources/Containerization/DNSConfiguration.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Containerization/DNSConfiguration.swift b/Sources/Containerization/DNSConfiguration.swift index dc0abe47..e87ed27d 100644 --- a/Sources/Containerization/DNSConfiguration.swift +++ b/Sources/Containerization/DNSConfiguration.swift @@ -14,6 +14,7 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import ContainerizationError import ContainerizationExtras /// DNS configuration for a container. The values will be used to From e673719a8b867c006250ededa8c8cbcfef89f8e0 Mon Sep 17 00:00:00 2001 From: Maxime Grenu Date: Tue, 17 Mar 2026 10:06:08 +0100 Subject: [PATCH 3/5] Fix DiffID computation to use uncompressed layer digest The OCI Image Specification requires DiffIDs to be the SHA256 digest of the uncompressed layer content. InitImage.create() was incorrectly using the digest of the compressed gzip layer. Add ContentWriter.diffID(of:) which decompresses the gzip stream and computes SHA256 of the raw content. The implementation parses the gzip header (handling FEXTRA, FNAME, FCOMMENT, FHCRC flags) and feeds the raw deflate stream to Apple's Compression framework. Signed-off-by: Maxime Grenu Signed-off-by: Maxime Grenu --- .../Containerization/Image/InitImage.swift | 6 +- .../Content/ContentWriter.swift | 110 +++++++++++++++++ .../DiffIDTests.swift | 115 ++++++++++++++++++ 3 files changed, 227 insertions(+), 4 deletions(-) create mode 100644 Tests/ContainerizationOCITests/DiffIDTests.swift diff --git a/Sources/Containerization/Image/InitImage.swift b/Sources/Containerization/Image/InitImage.swift index a63f0dce..e838198b 100644 --- a/Sources/Containerization/Image/InitImage.swift +++ b/Sources/Containerization/Image/InitImage.swift @@ -53,10 +53,8 @@ extension InitImage { var result = try writer.create(from: rootfs) let layerDescriptor = Descriptor(mediaType: ContainerizationOCI.MediaTypes.imageLayerGzip, digest: result.digest.digestString, size: result.size) - // TODO: compute and fill in the correct diffID for the above layer - // We currently put in the sha of the fully compressed layer, this needs to be replaced with - // the sha of the uncompressed layer. - let rootfsConfig = ContainerizationOCI.Rootfs(type: "layers", diffIDs: [result.digest.digestString]) + let diffID = try ContentWriter.diffID(of: rootfs) + let rootfsConfig = ContainerizationOCI.Rootfs(type: "layers", diffIDs: [diffID.digestString]) let runtimeConfig = ContainerizationOCI.ImageConfig(labels: labels) let imageConfig = ContainerizationOCI.Image(architecture: platform.architecture, os: platform.os, config: runtimeConfig, rootfs: rootfsConfig) result = try writer.create(from: imageConfig) diff --git a/Sources/ContainerizationOCI/Content/ContentWriter.swift b/Sources/ContainerizationOCI/Content/ContentWriter.swift index 0b8c662c..be44c51f 100644 --- a/Sources/ContainerizationOCI/Content/ContentWriter.swift +++ b/Sources/ContainerizationOCI/Content/ContentWriter.swift @@ -14,6 +14,7 @@ // limitations under the License. //===----------------------------------------------------------------------===// +import Compression import ContainerizationError import Crypto import Foundation @@ -60,6 +61,110 @@ public class ContentWriter { return try self.write(data) } + /// Computes the SHA256 digest of the uncompressed content of a gzip file. + /// + /// Per the OCI Image Specification, a DiffID is the SHA256 digest of the + /// uncompressed layer content. This method decompresses the gzip data and + /// hashes the result using a streaming approach for memory efficiency. + /// + /// - Parameter url: The URL of the gzip-compressed file. + /// - Returns: The SHA256 digest of the uncompressed content. + public static func diffID(of url: URL) throws -> SHA256.Digest { + let compressedData = try Data(contentsOf: url) + let decompressed = try Self.decompressGzip(compressedData) + return SHA256.hash(data: decompressed) + } + + /// Decompresses gzip data by stripping the gzip header and feeding the raw + /// deflate stream to Apple's Compression framework. + private static func decompressGzip(_ data: Data) throws -> Data { + let headerSize = try Self.gzipHeaderSize(data) + + var output = Data() + let bufferSize = 65_536 + let destinationBuffer = UnsafeMutablePointer.allocate(capacity: bufferSize) + defer { destinationBuffer.deallocate() } + + try data.withUnsafeBytes { rawBuffer in + guard let sourcePointer = rawBuffer.baseAddress?.assumingMemoryBound(to: UInt8.self) else { + throw ContentWriterError.decompressionFailed + } + + let stream = UnsafeMutablePointer.allocate(capacity: 1) + defer { stream.deallocate() } + + var status = compression_stream_init(stream, COMPRESSION_STREAM_DECODE, COMPRESSION_ZLIB) + guard status != COMPRESSION_STATUS_ERROR else { + throw ContentWriterError.decompressionFailed + } + defer { compression_stream_destroy(stream) } + + stream.pointee.src_ptr = sourcePointer.advanced(by: headerSize) + stream.pointee.src_size = data.count - headerSize + stream.pointee.dst_ptr = destinationBuffer + stream.pointee.dst_size = bufferSize + + repeat { + status = compression_stream_process(stream, 0) + + switch status { + case COMPRESSION_STATUS_OK: + let produced = bufferSize - stream.pointee.dst_size + output.append(destinationBuffer, count: produced) + stream.pointee.dst_ptr = destinationBuffer + stream.pointee.dst_size = bufferSize + + case COMPRESSION_STATUS_END: + let produced = bufferSize - stream.pointee.dst_size + if produced > 0 { + output.append(destinationBuffer, count: produced) + } + + default: + throw ContentWriterError.decompressionFailed + } + } while status == COMPRESSION_STATUS_OK + } + + return output + } + + /// Parses the gzip header to determine where the raw deflate stream begins. + private static func gzipHeaderSize(_ data: Data) throws -> Int { + guard data.count >= 10, + data[data.startIndex] == 0x1f, + data[data.startIndex + 1] == 0x8b + else { + throw ContentWriterError.invalidGzip + } + + let start = data.startIndex + let flags = data[start + 3] + var offset = 10 + + // FEXTRA + if flags & 0x04 != 0 { + guard data.count >= offset + 2 else { throw ContentWriterError.invalidGzip } + let extraLen = Int(data[start + offset]) | (Int(data[start + offset + 1]) << 8) + offset += 2 + extraLen + } + // FNAME + if flags & 0x08 != 0 { + while offset < data.count && data[start + offset] != 0 { offset += 1 } + offset += 1 + } + // FCOMMENT + if flags & 0x10 != 0 { + while offset < data.count && data[start + offset] != 0 { offset += 1 } + offset += 1 + } + // FHCRC + if flags & 0x02 != 0 { offset += 2 } + + guard offset < data.count else { throw ContentWriterError.invalidGzip } + return offset + } + /// Encodes the passed in type as a JSON blob and writes it to the base path. /// - Parameters: /// - content: The type to convert to JSON. @@ -69,3 +174,8 @@ public class ContentWriter { return try self.write(data) } } + +enum ContentWriterError: Error { + case invalidGzip + case decompressionFailed +} diff --git a/Tests/ContainerizationOCITests/DiffIDTests.swift b/Tests/ContainerizationOCITests/DiffIDTests.swift new file mode 100644 index 00000000..c79f80ad --- /dev/null +++ b/Tests/ContainerizationOCITests/DiffIDTests.swift @@ -0,0 +1,115 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025-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 Crypto +import Foundation +import Testing + +@testable import ContainerizationOCI + +struct DiffIDTests { + /// Helper to create a gzip-compressed temporary file from raw data. + private func createGzipFile(content: Data) throws -> URL { + let tempDir = FileManager.default.temporaryDirectory + let rawFile = tempDir.appendingPathComponent(UUID().uuidString) + let gzFile = tempDir.appendingPathComponent(UUID().uuidString + ".gz") + try content.write(to: rawFile) + defer { try? FileManager.default.removeItem(at: rawFile) } + + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/gzip") + process.arguments = ["-k", "-f", rawFile.path] + try process.run() + process.waitUntilExit() + + let gzPath = URL(fileURLWithPath: rawFile.path + ".gz") + if FileManager.default.fileExists(atPath: gzPath.path) { + try FileManager.default.moveItem(at: gzPath, to: gzFile) + } + return gzFile + } + + @Test func diffIDMatchesUncompressedSHA256() throws { + let content = Data("hello, oci layer content for diffid test".utf8) + let gzFile = try createGzipFile(content: content) + defer { try? FileManager.default.removeItem(at: gzFile) } + + let diffID = try ContentWriter.diffID(of: gzFile) + let expected = SHA256.hash(data: content) + + #expect(diffID.digestString == expected.digestString) + } + + @Test func diffIDIsDeterministic() throws { + let content = Data("deterministic diffid check".utf8) + let gzFile = try createGzipFile(content: content) + defer { try? FileManager.default.removeItem(at: gzFile) } + + let first = try ContentWriter.diffID(of: gzFile) + let second = try ContentWriter.diffID(of: gzFile) + + #expect(first.digestString == second.digestString) + } + + @Test func diffIDRejectsNonGzipData() throws { + let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try Data("this is not gzip".utf8).write(to: tempFile) + defer { try? FileManager.default.removeItem(at: tempFile) } + + #expect(throws: ContentWriterError.self) { + try ContentWriter.diffID(of: tempFile) + } + } + + @Test func diffIDRejectsEmptyFile() throws { + let tempFile = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try Data().write(to: tempFile) + defer { try? FileManager.default.removeItem(at: tempFile) } + + #expect(throws: ContentWriterError.self) { + try ContentWriter.diffID(of: tempFile) + } + } + + @Test func diffIDHandlesLargeContent() throws { + // 1MB of repeating data + let pattern = Data("ABCDEFGHIJKLMNOPQRSTUVWXYZ012345".utf8) + var large = Data() + for _ in 0..<(1_048_576 / pattern.count) { + large.append(pattern) + } + let gzFile = try createGzipFile(content: large) + defer { try? FileManager.default.removeItem(at: gzFile) } + + let diffID = try ContentWriter.diffID(of: gzFile) + let expected = SHA256.hash(data: large) + + #expect(diffID.digestString == expected.digestString) + } + + @Test func diffIDDigestStringFormat() throws { + let content = Data("format test".utf8) + let gzFile = try createGzipFile(content: content) + defer { try? FileManager.default.removeItem(at: gzFile) } + + let diffID = try ContentWriter.diffID(of: gzFile) + let digestString = diffID.digestString + + #expect(digestString.hasPrefix("sha256:")) + // sha256: prefix + 64 hex chars + #expect(digestString.count == 7 + 64) + } +} From 55a16d71f3e690ca9ab14538b7d2f4caf18e323b Mon Sep 17 00:00:00 2001 From: Maxime Grenu Date: Fri, 20 Mar 2026 10:23:31 +0100 Subject: [PATCH 4/5] fix: validate gzip trailer CRC32 and ISIZE in diffID computation Verify the gzip trailer (CRC32 checksum and original size) after decompression to reject malformed or truncated archives. Add tests for truncated gzip (missing trailer) and corrupted CRC. Signed-off-by: Maxime Grenu --- .../Content/ContentWriter.swift | 26 +++++++++++ .../DiffIDTests.swift | 44 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/Sources/ContainerizationOCI/Content/ContentWriter.swift b/Sources/ContainerizationOCI/Content/ContentWriter.swift index be44c51f..d551bac9 100644 --- a/Sources/ContainerizationOCI/Content/ContentWriter.swift +++ b/Sources/ContainerizationOCI/Content/ContentWriter.swift @@ -19,6 +19,7 @@ import ContainerizationError import Crypto import Foundation import NIOCore +import zlib /// Provides a context to write data into a directory. public class ContentWriter { @@ -126,6 +127,30 @@ public class ContentWriter { } while status == COMPRESSION_STATUS_OK } + // Validate the gzip trailer: last 8 bytes are CRC32 + ISIZE (both little-endian). + guard data.count >= 8 else { + throw ContentWriterError.gzipTrailerMismatch + } + let trailerStart = data.startIndex + data.count - 8 + let expectedCRC = UInt32(data[trailerStart]) + | (UInt32(data[trailerStart + 1]) << 8) + | (UInt32(data[trailerStart + 2]) << 16) + | (UInt32(data[trailerStart + 3]) << 24) + let expectedSize = UInt32(data[trailerStart + 4]) + | (UInt32(data[trailerStart + 5]) << 8) + | (UInt32(data[trailerStart + 6]) << 16) + | (UInt32(data[trailerStart + 7]) << 24) + + let actualCRC = output.withUnsafeBytes { buffer -> UInt32 in + let ptr = buffer.baseAddress!.assumingMemoryBound(to: Bytef.self) + return UInt32(crc32(0, ptr, uInt(buffer.count))) + } + let actualSize = UInt32(truncatingIfNeeded: output.count) + + guard expectedCRC == actualCRC, expectedSize == actualSize else { + throw ContentWriterError.gzipTrailerMismatch + } + return output } @@ -178,4 +203,5 @@ public class ContentWriter { enum ContentWriterError: Error { case invalidGzip case decompressionFailed + case gzipTrailerMismatch } diff --git a/Tests/ContainerizationOCITests/DiffIDTests.swift b/Tests/ContainerizationOCITests/DiffIDTests.swift index c79f80ad..88a5c308 100644 --- a/Tests/ContainerizationOCITests/DiffIDTests.swift +++ b/Tests/ContainerizationOCITests/DiffIDTests.swift @@ -100,6 +100,50 @@ struct DiffIDTests { #expect(diffID.digestString == expected.digestString) } + @Test func diffIDRejectsTruncatedGzip() throws { + // Build a valid gzip file, then chop off the 8-byte trailer (CRC32 + ISIZE) + // to produce a structurally malformed archive. + let content = Data("truncated gzip trailer test".utf8) + let gzFile = try createGzipFile(content: content) + defer { try? FileManager.default.removeItem(at: gzFile) } + + var gzData = try Data(contentsOf: gzFile) + guard gzData.count > 8 else { + Issue.record("Compressed file too small to truncate") + return + } + gzData.removeLast(8) + + let truncatedFile = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString + ".gz") + try gzData.write(to: truncatedFile) + defer { try? FileManager.default.removeItem(at: truncatedFile) } + + #expect(throws: ContentWriterError.self) { + try ContentWriter.diffID(of: truncatedFile) + } + } + + @Test func diffIDRejectsCorruptedCRC() throws { + // Flip a byte in the CRC32 field of an otherwise valid gzip file. + let content = Data("corrupted crc test".utf8) + let gzFile = try createGzipFile(content: content) + defer { try? FileManager.default.removeItem(at: gzFile) } + + var gzData = try Data(contentsOf: gzFile) + let crcOffset = gzData.count - 8 + gzData[crcOffset] ^= 0xFF + + let corruptedFile = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString + ".gz") + try gzData.write(to: corruptedFile) + defer { try? FileManager.default.removeItem(at: corruptedFile) } + + #expect(throws: ContentWriterError.self) { + try ContentWriter.diffID(of: corruptedFile) + } + } + @Test func diffIDDigestStringFormat() throws { let content = Data("format test".utf8) let gzFile = try createGzipFile(content: content) From 50b1194477058e27711098af930dad6d1adb7fcf Mon Sep 17 00:00:00 2001 From: Maxime Grenu Date: Sun, 22 Mar 2026 11:32:36 +0100 Subject: [PATCH 5/5] fix: validate gzip CM field and correct docstring Reject archives where the compression method byte is not 0x08 (deflate), per RFC 1952 section 2.3.1. Also corrects the diffID docstring which incorrectly claimed a streaming approach. Signed-off-by: Maxime Grenu --- Sources/ContainerizationOCI/Content/ContentWriter.swift | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Sources/ContainerizationOCI/Content/ContentWriter.swift b/Sources/ContainerizationOCI/Content/ContentWriter.swift index d551bac9..8f3575b4 100644 --- a/Sources/ContainerizationOCI/Content/ContentWriter.swift +++ b/Sources/ContainerizationOCI/Content/ContentWriter.swift @@ -65,8 +65,8 @@ public class ContentWriter { /// Computes the SHA256 digest of the uncompressed content of a gzip file. /// /// Per the OCI Image Specification, a DiffID is the SHA256 digest of the - /// uncompressed layer content. This method decompresses the gzip data and - /// hashes the result using a streaming approach for memory efficiency. + /// uncompressed layer content. This method loads the compressed file, + /// decompresses it, validates the gzip trailer, and hashes the result. /// /// - Parameter url: The URL of the gzip-compressed file. /// - Returns: The SHA256 digest of the uncompressed content. @@ -158,7 +158,8 @@ public class ContentWriter { private static func gzipHeaderSize(_ data: Data) throws -> Int { guard data.count >= 10, data[data.startIndex] == 0x1f, - data[data.startIndex + 1] == 0x8b + data[data.startIndex + 1] == 0x8b, + data[data.startIndex + 2] == 0x08 // CM must be 8 (deflate) per RFC 1952 else { throw ContentWriterError.invalidGzip }