mullvad · buggmagnet · Apr 17, 2024 · Apr 8, 2024
diff --git a/ios/MullvadLogging/Date+LogFormat.swift b/ios/MullvadLogging/Date+LogFormat.swift
@@ -15,4 +15,11 @@ extension Date {
 
         return formatter.string(from: self)
     }
+
+    public func logFormatFilename() -> String {
+        let formatter = DateFormatter()
+        formatter.dateFormat = "dd-MM-yyyy'T'HH:mm:ss"
+
+        return formatter.string(from: self)
+    }
 }
diff --git a/ios/MullvadLogging/LogRotation.swift b/ios/MullvadLogging/LogRotation.swift
@@ -10,48 +10,96 @@ import Foundation
 import MullvadTypes
 
 public enum LogRotation {
+    private struct LogData {
+        var path: URL
+        var size: UInt64
+        var creationDate: Date
+    }
+
+    public struct Options {
+        let storageSizeLimit: Int
+        let oldestAllowedDate: Date
+
+        /// Options for log rotation, defining how logs should be retained.
+        ///
+        /// - Parameter storageSizeLimit: Storage size limit in bytes.
+        /// - Parameter oldestAllowedDate: Oldest allowed date.
+        public init(storageSizeLimit: Int, oldestAllowedDate: Date) {
+            self.storageSizeLimit = storageSizeLimit
+            self.oldestAllowedDate = oldestAllowedDate
+        }
+    }
+
     public enum Error: LocalizedError, WrappingError {
-        case noSourceLogFile
-        case moveSourceLogFile(Swift.Error)
+        case rotateLogFiles(Swift.Error)
 
         public var errorDescription: String? {
             switch self {
-            case .noSourceLogFile:
-                return "Source log file does not exist."
-            case .moveSourceLogFile:
-                return "Failure to move the source log file to backup."
+            case .rotateLogFiles:
+                return "Failure to rotate the source log file to backup."
             }
         }
 
         public var underlyingError: Swift.Error? {
             switch self {
-            case .noSourceLogFile:
-                return nil
-            case let .moveSourceLogFile(error):
+            case let .rotateLogFiles(error):
                 return error
             }
         }
     }
 
-    public static func rotateLog(logsDirectory: URL, logFileName: String) throws {
-        let source = logsDirectory.appendingPathComponent(logFileName)
-        let backup = source.deletingPathExtension().appendingPathExtension("old.log")
+    public static func rotateLogs(logDirectory: URL, options: Options) throws {
+        let fileManager = FileManager.default
 
         do {
-            _ = try FileManager.default.replaceItemAt(backup, withItemAt: source)
-        } catch {
-            // FileManager returns a very obscure error chain so we need to traverse it to find
-            // the root cause of the error.
-            for case let fileError as CocoaError in error.underlyingErrorChain {
-                // .fileNoSuchFile is returned when both backup and source log files do not exist
-                // .fileReadNoSuchFile is returned when backup exists but source log file does not
-                if fileError.code == .fileNoSuchFile || fileError.code == .fileReadNoSuchFile,
-                   fileError.url == source {
-                    throw Error.noSourceLogFile
+            // Filter out all log files in directory.
+            let logPaths: [URL] = (try fileManager.contentsOfDirectory(
+                atPath: logDirectory.relativePath
+            )).compactMap { file in
+                if file.split(separator: ".").last == "log" {
+                    logDirectory.appendingPathComponent(file)
+                } else {
+                    nil
                 }
             }
 
-            throw Error.moveSourceLogFile(error)
+            // Convert logs into objects with necessary meta data.
+            let logs = try logPaths.map { logPath in
+                let attributes = try fileManager.attributesOfItem(atPath: logPath.relativePath)
+                let size = (attributes[.size] as? UInt64) ?? 0
+                let creationDate = (attributes[.creationDate] as? Date) ?? Date.distantPast
+
+                return LogData(path: logPath, size: size, creationDate: creationDate)
+            }.sorted { log1, log2 in
+                log1.creationDate > log2.creationDate
+            }
+
+            try deleteLogsOlderThan(options.oldestAllowedDate, in: logs)
+            try deleteLogsWithCombinedSizeLargerThan(options.storageSizeLimit, in: logs)
+        } catch {
+            throw Error.rotateLogFiles(error)
+        }
+    }
+
+    private static func deleteLogsOlderThan(_ dateThreshold: Date, in logs: [LogData]) throws {
+        let fileManager = FileManager.default
+
+        for log in logs where log.creationDate < dateThreshold {
+            try fileManager.removeItem(at: log.path)
+        }
+    }
+
+    private static func deleteLogsWithCombinedSizeLargerThan(_ sizeThreshold: Int, in logs: [LogData]) throws {
+        let fileManager = FileManager.default
+
+        // Delete all logs outside maximum capacity (ordered newest to oldest).
+        var fileSizes = UInt64.zero
+        for log in logs {
+            fileSizes += log.size
+
+            if fileSizes > sizeThreshold {
+                try fileManager.removeItem(at: log.path)
+            }
         }
     }
 }
diff --git a/ios/MullvadLogging/Logging.swift b/ios/MullvadLogging/Logging.swift
@@ -8,6 +8,7 @@
 
 import Foundation
 @_exported import Logging
+import MullvadTypes
 
 private enum LoggerOutput {
     case fileOutput(_ fileOutput: LogFileOutputStream)
@@ -24,7 +25,6 @@ public struct LoggerBuilder {
     public init() {}
 
     public mutating func addFileOutput(fileURL: URL) {
-        let logFileName = fileURL.lastPathComponent
         let logsDirectoryURL = fileURL.deletingLastPathComponent()
 
         try? FileManager.default.createDirectory(
@@ -34,7 +34,10 @@ public struct LoggerBuilder {
         )
 
         do {
-            try LogRotation.rotateLog(logsDirectory: logsDirectoryURL, logFileName: logFileName)
+            try LogRotation.rotateLogs(logDirectory: logsDirectoryURL, options: LogRotation.Options(
+                storageSizeLimit: 5_242_880, // 5 MB
+                oldestAllowedDate: Date(timeIntervalSinceNow: Duration.days(7).timeInterval)
+            ))
         } catch {
             logRotationErrors.append(error)
         }

diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj
@@ -582,6 +582,7 @@
 		7A9CCCC42A96302800DD6A34 /* TunnelCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A9CCCB22A96302800DD6A34 /* TunnelCoordinator.swift */; };
 		7A9FA1422A2E3306000B728D /* CheckboxView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A9FA1412A2E3306000B728D /* CheckboxView.swift */; };
 		7A9FA1442A2E3FE5000B728D /* CheckableSettingsCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A9FA1432A2E3FE5000B728D /* CheckableSettingsCell.swift */; };
+		7AA513862BC91C6B00D081A4 /* LogRotationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AA513852BC91C6B00D081A4 /* LogRotationTests.swift */; };
 		7AB2B6702BA1EB8C00B03E3B /* ListCustomListViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB2B66E2BA1EB8C00B03E3B /* ListCustomListViewController.swift */; };
 		7AB2B6712BA1EB8C00B03E3B /* ListCustomListCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB2B66F2BA1EB8C00B03E3B /* ListCustomListCoordinator.swift */; };
 		7AB4CCB92B69097E006037F5 /* IPOverrideTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7AB4CCB82B69097E006037F5 /* IPOverrideTests.swift */; };
@@ -1841,6 +1842,7 @@
 		7A9CCCB22A96302800DD6A34 /* TunnelCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TunnelCoordinator.swift; sourceTree = "<group>"; };
 		7A9FA1412A2E3306000B728D /* CheckboxView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CheckboxView.swift; sourceTree = "<group>"; };
 		7A9FA1432A2E3FE5000B728D /* CheckableSettingsCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CheckableSettingsCell.swift; sourceTree = "<group>"; };
+		7AA513852BC91C6B00D081A4 /* LogRotationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LogRotationTests.swift; sourceTree = "<group>"; };
 		7AB2B66E2BA1EB8C00B03E3B /* ListCustomListViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ListCustomListViewController.swift; sourceTree = "<group>"; };
 		7AB2B66F2BA1EB8C00B03E3B /* ListCustomListCoordinator.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ListCustomListCoordinator.swift; sourceTree = "<group>"; };
 		7AB4CCB82B69097E006037F5 /* IPOverrideTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IPOverrideTests.swift; sourceTree = "<group>"; };
@@ -2959,6 +2961,7 @@
 				7A5869C22B5820CE00640D27 /* IPOverrideRepositoryTests.swift */,
 				7AB4CCB82B69097E006037F5 /* IPOverrideTests.swift */,
 				7A516C3B2B712F0B00BBD33D /* IPOverrideWrapperTests.swift */,
+				7AA513852BC91C6B00D081A4 /* LogRotationTests.swift */,
 				A9B6AC172ADE8F4300F7802A /* MigrationManagerTests.swift */,
 				58C3FA652A38549D006A450A /* MockFileCache.swift */,
 				F09D04B42AE93CB6003D4F89 /* OutgoingConnectionProxy+Stub.swift */,
@@ -4973,6 +4976,7 @@
 				A9A5FA2D2ACB05160083449F /* DurationTests.swift in Sources */,
 				A9A5FA2E2ACB05160083449F /* FileCacheTests.swift in Sources */,
 				A9A5FA2F2ACB05160083449F /* FixedWidthIntegerArithmeticsTests.swift in Sources */,
+				7AA513862BC91C6B00D081A4 /* LogRotationTests.swift in Sources */,
 				F04413622BA45CE30018A6EE /* CustomListLocationNodeBuilder.swift in Sources */,
 				A9A5FA302ACB05160083449F /* InputTextFormatterTests.swift in Sources */,
 				F0B0E6972AFE6E7E001DC66B /* XCTest+Async.swift in Sources */,

diff --git a/ios/MullvadVPN/AppDelegate.swift b/ios/MullvadVPN/AppDelegate.swift
@@ -353,7 +353,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
 
     private func configureLogging() {
         var loggerBuilder = LoggerBuilder()
-        loggerBuilder.addFileOutput(fileURL: ApplicationConfiguration.logFileURL(for: .mainApp))
+        loggerBuilder.addFileOutput(fileURL: ApplicationConfiguration.newLogFileURL(for: .mainApp))
         #if DEBUG
         loggerBuilder.addOSLogOutput(subsystem: ApplicationTarget.mainApp.bundleIdentifier)
         #endif

diff --git a/ios/MullvadVPN/Classes/ConsolidatedApplicationLog.swift b/ios/MullvadVPN/Classes/ConsolidatedApplicationLog.swift
@@ -47,17 +47,9 @@ class ConsolidatedApplicationLog: TextOutputStreamable {
             }
     }
 
-    func addLogFile(fileURL: URL, includeLogBackup: Bool) {
-        addSingleLogFile(fileURL)
-        if includeLogBackup {
-            let oldLogFileURL = fileURL.deletingPathExtension().appendingPathExtension("old.log")
-            addSingleLogFile(oldLogFileURL)
-        }
-    }
-
-    func addLogFiles(fileURLs: [URL], includeLogBackups: Bool) {
+    func addLogFiles(fileURLs: [URL]) {
         for fileURL in fileURLs {
-            addLogFile(fileURL: fileURL, includeLogBackup: includeLogBackups)
+            addSingleLogFile(fileURL)
         }
     }
 

diff --git a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportInteractor.swift
@@ -24,9 +24,8 @@ final class ProblemReportInteractor {
             redactContainerPathsForSecurityGroupIdentifiers: [securityGroupIdentifier]
         )
 
-        let logFileURLs = ApplicationTarget.allCases.map { ApplicationConfiguration.logFileURL(for: $0) }
-
-        report.addLogFiles(fileURLs: logFileURLs, includeLogBackups: true)
+        let logFileURLs = ApplicationTarget.allCases.flatMap { ApplicationConfiguration.logFileURLs(for: $0) }
+        report.addLogFiles(fileURLs: logFileURLs)
 
         return report
     }()

diff --git a/ios/MullvadVPNTests/LogRotationTests.swift b/ios/MullvadVPNTests/LogRotationTests.swift
@@ -0,0 +1,98 @@
+//
+//  LogRotationTests.swift
+//  MullvadVPNTests
+//
+//  Created by Jon Petersson on 2024-04-12.
+//  Copyright © 2024 Mullvad VPN AB. All rights reserved.
+//
+
+import MullvadLogging
+import XCTest
+
+final class LogRotationTests: XCTestCase {
+    let fileManager = FileManager.default
+    let directoryPath = FileManager.default.temporaryDirectory.appendingPathComponent("LogRotationTests")
+
+    override func setUpWithError() throws {
+        try? fileManager.createDirectory(
+            at: directoryPath,
+            withIntermediateDirectories: false
+        )
+    }
+
+    override func tearDownWithError() throws {
+        try fileManager.removeItem(atPath: directoryPath.relativePath)
+    }
+
+    func testRotateLogsByStorageSizeLimit() throws {
+        let logPaths = [
+            directoryPath.appendingPathComponent("test1.log"),
+            directoryPath.appendingPathComponent("test2.log"),
+            directoryPath.appendingPathComponent("test3.log"),
+            directoryPath.appendingPathComponent("test4.log"),
+            directoryPath.appendingPathComponent("test5.log"),
+        ]
+
+        try logPaths.forEach { logPath in
+            try writeDataToDisk(path: logPath, fileSize: 1000)
+        }
+
+        try LogRotation.rotateLogs(logDirectory: directoryPath, options: LogRotation.Options(
+            storageSizeLimit: 5000,
+            oldestAllowedDate: .distantPast)
+        )
+        var logFileCount = try fileManager.contentsOfDirectory(atPath: directoryPath.relativePath).count
+        XCTAssertEqual(logFileCount, 5)
+
+        try LogRotation.rotateLogs(logDirectory: directoryPath, options: LogRotation.Options(
+            storageSizeLimit: 3999,
+            oldestAllowedDate: .distantPast)
+        )
+        logFileCount = try fileManager.contentsOfDirectory(atPath: directoryPath.relativePath).count
+        XCTAssertEqual(logFileCount, 3)
+    }
+
+    func testRotateLogsByOldestAllowedDate() throws {
+        let firstBatchOflogPaths = [
+            directoryPath.appendingPathComponent("test1.log"),
+            directoryPath.appendingPathComponent("test2.log"),
+            directoryPath.appendingPathComponent("test3.log"),
+        ]
+
+        let secondBatchOflogPaths = [
+            directoryPath.appendingPathComponent("test4.log"),
+            directoryPath.appendingPathComponent("test5.log"),
+        ]
+
+        let oldestDateAllowedForFirstBatch = Date()
+        try firstBatchOflogPaths.forEach { logPath in
+            try writeDataToDisk(path: logPath, fileSize: 1000)
+        }
+
+        let oldestDateAllowedForSecondBatch = Date()
+        try secondBatchOflogPaths.forEach { logPath in
+            try writeDataToDisk(path: logPath, fileSize: 1000)
+        }
+
+        try LogRotation.rotateLogs(
+            logDirectory: directoryPath,
+            options: LogRotation.Options(storageSizeLimit: .max, oldestAllowedDate: oldestDateAllowedForFirstBatch)
+        )
+        var logFileCount = try fileManager.contentsOfDirectory(atPath: directoryPath.relativePath).count
+        XCTAssertEqual(logFileCount, 5)
+
+        try LogRotation.rotateLogs(
+            logDirectory: directoryPath,
+            options: LogRotation.Options(storageSizeLimit: .max, oldestAllowedDate: oldestDateAllowedForSecondBatch)
+        )
+        logFileCount = try fileManager.contentsOfDirectory(atPath: directoryPath.relativePath).count
+        XCTAssertEqual(logFileCount, 2)
+    }
+}
+
+extension LogRotationTests {
+    private func writeDataToDisk(path: URL, fileSize: Int) throws {
+        let data = Data((0 ..< fileSize).map { UInt8($0 & 0xff) })
+        try data.write(to: path)
+    }
+}
diff --git a/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider/PacketTunnelProvider.swift
@@ -163,7 +163,7 @@
         var loggerBuilder = LoggerBuilder()
         let pid = ProcessInfo.processInfo.processIdentifier
         loggerBuilder.metadata["pid"] = .string("\(pid)")
-        loggerBuilder.addFileOutput(fileURL: ApplicationConfiguration.logFileURL(for: .packetTunnel))
+        loggerBuilder.addFileOutput(fileURL: ApplicationConfiguration.newLogFileURL(for: .packetTunnel))
         #if DEBUG
         loggerBuilder.addOSLogOutput(subsystem: ApplicationTarget.packetTunnel.bundleIdentifier)
         #endif
@@ -279,7 +279,7 @@

 extension PacketTunnelProvider: PostQuantumKeyReceiving {
    func receivePostQuantumKey(_ key: PreSharedKey) {
        // TODO: send the key to the actor
        actor.replacePreSharedKey(key)
    }
 }
diff --git a/ios/Shared/ApplicationConfiguration.swift b/ios/Shared/ApplicationConfiguration.swift
@@ -21,9 +21,25 @@ enum ApplicationConfiguration {
         FileManager.default.containerURL(forSecurityApplicationGroupIdentifier: securityGroupIdentifier)!
     }
 
-    /// Returns URL for log file associated with application target and located within shared container.
-    static func logFileURL(for target: ApplicationTarget) -> URL {
-        containerURL.appendingPathComponent("\(target.bundleIdentifier).log", isDirectory: false)
+    /// Returns URL for new log file associated with application target and located within shared container.
+    static func newLogFileURL(for target: ApplicationTarget) -> URL {
+        containerURL.appendingPathComponent(
+            "\(target.bundleIdentifier)_\(Date().logFormatFilename()).log",
+            isDirectory: false
+        )
+    }
+
+    /// Returns URLs for log files associated with application target and located within shared container.
+    static func logFileURLs(for target: ApplicationTarget) -> [URL] {
+        let containerUrl = containerURL
+
+        return (try? FileManager.default.contentsOfDirectory(atPath: containerURL.relativePath))?.compactMap { file in
+            if file.split(separator: ".").last == "log" {
+                containerUrl.appendingPathComponent(file)
+            } else {
+                nil
+            }
+        }.sorted { $0.relativePath > $1.relativePath } ?? []
     }
 
     /// Privacy policy URL.