From f079391ab9534e8882542d16e4e7ba778b4c5902 Mon Sep 17 00:00:00 2001 From: Evan Haug <90289817+Evan-Haug@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:52:48 -0500 Subject: [PATCH] setupCACerts: write the aggregate atomically and never reuse an empty or short one The aggregate cacerts file was created empty under its final name and then filled, and an existing file was reused without any check. A process death or write failure in between left an empty or truncated file that was trusted on every later launch, so libcurl had no roots and every https request failed. Write to a temporary file in the same folder, synchronize it and rename it into place. Reuse an existing aggregate only when it is non-empty and at least as large as the sum of the selected certificate files. Add Android tests: complete bundle, empty bundle rebuilt, truncated bundle rebuilt, valid bundle left untouched. --- Sources/AndroidNative/AndroidNative.swift | 32 ++++-- .../AndroidNativeTests.swift | 99 +++++++++++++++++++ 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/Sources/AndroidNative/AndroidNative.swift b/Sources/AndroidNative/AndroidNative.swift index caa7f4e..4d42913 100644 --- a/Sources/AndroidNative/AndroidNative.swift +++ b/Sources/AndroidNative/AndroidNative.swift @@ -27,6 +27,8 @@ import Foundation public class AndroidBootstrap { /// Collects all the certificate files from the Android certificate store and writes them to a single `cacerts.pem` file that can be used by libcurl, /// which is communicated through the `URLSessionCertificateAuthorityInfoFile` environment property + /// The aggregate is written atomically. + /// An existing aggregate is reused only when it is at least as large as the sum of the system certificate files. /// /// See https://android.googlesource.com/platform/frameworks/base/+/8b192b19f264a8829eac2cfaf0b73f6fc188d933%5E%21/#F0 /// See https://github.com/apple/swift-nio-ssl/blob/d1088ebe0789d9eea231b40741831f37ab654b61/Sources/NIOSSL/AndroidCABundle.swift#L30 @@ -65,10 +67,13 @@ public class AndroidBootstrap { // generate a checksum of all the certificate URL names and their sizes and modification times in order to define the aggregate file name // we do this so was can safely cache the aggregate certificate file without re-creating it every time var urlSummary = "" + var expectedMinimumBytes: Int = 0 for certURL in certURLs { + let fileSize = try? certURL.resourceValues(forKeys: [.fileSizeKey]).fileSize + expectedMinimumBytes += fileSize ?? 0 urlSummary.append(certURL.path) urlSummary.append("|") - urlSummary.append((try? certURL.resourceValues(forKeys: [.fileSizeKey]).fileSize?.description) ?? "") + urlSummary.append(fileSize?.description ?? "") urlSummary.append("|") urlSummary.append((try? certURL.resourceValues(forKeys: [.contentModificationDateKey]).contentModificationDate?.timeIntervalSince1970.description) ?? "") urlSummary.append("|") @@ -83,22 +88,29 @@ public class AndroidBootstrap { let generatedCacertsURL = cacheFolder.appendingPathComponent("cacerts-aggregate-\(checksum).pem") if FileManager.default.fileExists(atPath: generatedCacertsURL.path) { - // cached aggregate file already exists; just re-use + // re-use only a non-empty cached aggregate file of sufficient size if !force { - setenv("URLSessionCertificateAuthorityInfoFile", generatedCacertsURL.path, 1) - return + let size = (try? FileManager.default.attributesOfItem(atPath: generatedCacertsURL.path)[.size] as? NSNumber)?.intValue ?? 0 + if size > 0 && size >= expectedMinimumBytes { + setenv("URLSessionCertificateAuthorityInfoFile", generatedCacertsURL.path, 1) + return + } } // clear any previous generated certificates file that may have been created by this app try FileManager.default.removeItem(atPath: generatedCacertsURL.path) } - _ = FileManager.default.createFile(atPath: generatedCacertsURL.path, contents: nil) - let fs = try FileHandle(forWritingTo: generatedCacertsURL) + let tempPath = generatedCacertsURL.path + ".\(UUID().uuidString).tmp" + defer { try? FileManager.default.removeItem(atPath: tempPath) } + guard FileManager.default.createFile(atPath: tempPath, contents: nil) else { + throw CocoaError(.fileWriteUnknown) + } + let fs = try FileHandle(forWritingTo: URL(fileURLWithPath: tempPath)) defer { try? fs.close() } // write a header - fs.write( + try fs.write(contentsOf: """ ## Bundle of CA Root Certificates ## Auto-generated on \(Date()) @@ -115,6 +127,12 @@ public class AndroidBootstrap { try fs.write(contentsOf: try Data(contentsOf: certURL)) } + try fs.synchronize() + try fs.close() + guard rename(tempPath, generatedCacertsURL.path) == 0 else { + throw CocoaError(.fileWriteUnknown) + } + setenv("URLSessionCertificateAuthorityInfoFile", generatedCacertsURL.path, 1) } diff --git a/Tests/AndroidNativeTests/AndroidNativeTests.swift b/Tests/AndroidNativeTests/AndroidNativeTests.swift index 56a938d..fe5ab61 100644 --- a/Tests/AndroidNativeTests/AndroidNativeTests.swift +++ b/Tests/AndroidNativeTests/AndroidNativeTests.swift @@ -15,6 +15,9 @@ import Testing import AndroidNative import Foundation +#if os(Android) +import Android +#endif #if canImport(FoundationNetworking) import FoundationEssentials import FoundationNetworking @@ -25,6 +28,102 @@ private struct RetryableError: Error { } struct AndroidNativeTests { + #if os(Android) + private static let caCertsLock = NSLock() + + @Test func testSetupCACertsBuildsCompleteBundle() throws { + try withCACertsFixture { _, expectedMinimumBytes, aggregateURL in + #expect(FileManager.default.fileExists(atPath: aggregateURL.path)) + try expectCompleteCACerts(at: aggregateURL, expectedMinimumBytes: expectedMinimumBytes) + } + } + + @Test func testSetupCACertsRebuildsEmptyBundle() throws { + try withCACertsFixture { certsFolder, expectedMinimumBytes, aggregateURL in + try Data().write(to: aggregateURL) + unsetenv("URLSessionCertificateAuthorityInfoFile") + try AndroidBootstrap.setupCACerts(force: false, fromCertficateFolders: [certsFolder.path]) + let path = try #require(getenv("URLSessionCertificateAuthorityInfoFile")) + #expect(String(cString: path) == aggregateURL.path) + try expectCompleteCACerts(at: aggregateURL, expectedMinimumBytes: expectedMinimumBytes) + } + } + + @Test func testSetupCACertsRebuildsTruncatedBundle() throws { + try withCACertsFixture { certsFolder, expectedMinimumBytes, aggregateURL in + let completeBundle = try Data(contentsOf: aggregateURL) + // Retain only 1 or 100 bytes, both below the certificate-size minimum. + for truncatedSize in [1, 100] { + try completeBundle.prefix(truncatedSize).write(to: aggregateURL) + unsetenv("URLSessionCertificateAuthorityInfoFile") + try AndroidBootstrap.setupCACerts(force: false, fromCertficateFolders: [certsFolder.path]) + let path = try #require(getenv("URLSessionCertificateAuthorityInfoFile")) + #expect(String(cString: path) == aggregateURL.path) + try expectCompleteCACerts(at: aggregateURL, expectedMinimumBytes: expectedMinimumBytes) + } + + let attributes = try FileManager.default.attributesOfItem(atPath: aggregateURL.path) + let size = (try #require(attributes[.size] as? NSNumber)).intValue + let modificationDate = try #require(attributes[.modificationDate] as? Date) + unsetenv("URLSessionCertificateAuthorityInfoFile") + try AndroidBootstrap.setupCACerts(force: false, fromCertficateFolders: [certsFolder.path]) + let path = try #require(getenv("URLSessionCertificateAuthorityInfoFile")) + #expect(String(cString: path) == aggregateURL.path) + let reusedAttributes = try FileManager.default.attributesOfItem(atPath: aggregateURL.path) + #expect((reusedAttributes[.size] as? NSNumber)?.intValue == size) + #expect(reusedAttributes[.modificationDate] as? Date == modificationDate) + } + } + + private func withCACertsFixture(_ body: (URL, Int, URL) throws -> Void) throws { + // These tests share a process-wide environment variable. + Self.caCertsLock.lock() + defer { Self.caCertsLock.unlock() } + let previousPath = getenv("URLSessionCertificateAuthorityInfoFile").map { String(cString: $0) } + defer { + if let previousPath { + setenv("URLSessionCertificateAuthorityInfoFile", previousPath, 1) + } else { + unsetenv("URLSessionCertificateAuthorityInfoFile") + } + } + + let certsFolder = URL.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try FileManager.default.createDirectory(at: certsFolder, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: certsFolder) } + var expectedMinimumBytes = 0 + for name in ["aaaaaaaa.0", "bbbbbbbb.0", "cccccccc.0", "dddddddd.0"] { + let certificate = """ + -----BEGIN CERTIFICATE----- + YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXo= + MDEyMzQ1Njc4OWFiY2RlZmdoaWprbG1ub3A= + cXJzdHV2d3h5ejAxMjM0NTY3ODlhYmNkZWY= + -----END CERTIFICATE----- + Test certificate metadata for \(name) + + """.data(using: .utf8)! + expectedMinimumBytes += certificate.count + try certificate.write(to: certsFolder.appendingPathComponent(name)) + } + + unsetenv("URLSessionCertificateAuthorityInfoFile") + try AndroidBootstrap.setupCACerts(force: false, fromCertficateFolders: [certsFolder.path]) + let path = try #require(getenv("URLSessionCertificateAuthorityInfoFile")) + let aggregateURL = URL(fileURLWithPath: String(cString: path)) + defer { try? FileManager.default.removeItem(at: aggregateURL) } + try body(certsFolder, expectedMinimumBytes, aggregateURL) + } + + private func expectCompleteCACerts(at aggregateURL: URL, expectedMinimumBytes: Int) throws { + let bundle = try Data(contentsOf: aggregateURL) + #expect(bundle.count >= expectedMinimumBytes) + let text = try #require(String(data: bundle, encoding: .utf8)) + #expect(text.components(separatedBy: "-----BEGIN CERTIFICATE-----").count - 1 == 4) + let files = try FileManager.default.contentsOfDirectory(atPath: aggregateURL.deletingLastPathComponent().path) + #expect(!files.contains { $0.hasSuffix(".tmp") }) + } + #endif + @Test(.disabled("temporarily disabled on Android due to hang")) func testNetwork() async throws { #if os(Android)