From b457315eb02a724614d54a5f39e7c6718554c550 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 09:07:45 +0200 Subject: [PATCH 01/10] Add a note about encrypted cert key --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index e994a480..88805650 100644 --- a/README.md +++ b/README.md @@ -70,14 +70,12 @@ Passepartout can import .ovpn configuration files. This way you can fine-tune en Unsupported: - UDP fragmentation, i.e. `--fragment` - -Unsupported (probably ever): - - Compression - `--comp-lzo` other than `no` - `--compress` other than empty - Proxy - External file references (inline `` only) +- Encrypted certificate private key (will raise error TunnelKitNative Code=205) Ignored: From 4388dfe6aecc18dc41fa371b9169827e3fc73bb4 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 00:29:12 +0200 Subject: [PATCH 02/10] Lower log level --- Passepartout/Sources/AppConstants.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Passepartout/Sources/AppConstants.swift b/Passepartout/Sources/AppConstants.swift index d314cec4..8c22f975 100644 --- a/Passepartout/Sources/AppConstants.swift +++ b/Passepartout/Sources/AppConstants.swift @@ -108,7 +108,7 @@ class AppConstants { static func configure() { let console = ConsoleDestination() console.useNSLog = true - console.minLevel = .verbose + console.minLevel = .debug SwiftyBeaver.addDestination(console) } } From 821393af70f70c0ff22dd127a21bc274e19e1cd1 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 00:08:48 +0200 Subject: [PATCH 03/10] Refactor configurations in service extension Reuse same directories of ConnectionService for storing configuration files. --- .../Organizer/WizardHostViewController.swift | 4 +- .../Scenes/ServiceViewController.swift | 2 +- Passepartout.xcodeproj/project.pbxproj | 8 +- .../ConnectionService+Configurations.swift | 49 +++++++++++ .../Sources/Model/ConnectionService.swift | 14 ++-- .../Model/ProfileConfigurationFactory.swift | 82 ------------------- 6 files changed, 64 insertions(+), 95 deletions(-) create mode 100644 Passepartout/Sources/Model/ConnectionService+Configurations.swift delete mode 100644 Passepartout/Sources/Model/ProfileConfigurationFactory.swift diff --git a/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift b/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift index dd2e731d..2fb94b15 100644 --- a/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift @@ -159,8 +159,8 @@ class WizardHostViewController: UITableViewController, TableModelHost, Wizard { } if let url = parsedFile?.url { do { - let savedUrl = try ProfileConfigurationFactory.shared.save(url: url, for: profile) - log.debug("Associated .ovpn configuration file to profile '\(profile.id)': \(savedUrl)") + let savedURL = try TransientStore.shared.service.save(configurationURL: url, for: profile) + log.debug("Associated .ovpn configuration file to profile '\(profile.id)': \(savedURL)") } catch let e { log.error("Could not associate .ovpn configuration file to profile: \(e)") } diff --git a/Passepartout-iOS/Scenes/ServiceViewController.swift b/Passepartout-iOS/Scenes/ServiceViewController.swift index 36349032..be20305f 100644 --- a/Passepartout-iOS/Scenes/ServiceViewController.swift +++ b/Passepartout-iOS/Scenes/ServiceViewController.swift @@ -154,7 +154,7 @@ class ServiceViewController: UIViewController, TableModelHost { let vc = destination as? ConfigurationViewController vc?.title = L10n.Service.Cells.Host.Parameters.caption vc?.initialConfiguration = uncheckedHostProfile.parameters.sessionConfiguration - vc?.originalConfigurationURL = ProfileConfigurationFactory.shared.configurationURL(for: uncheckedHostProfile) + vc?.originalConfigurationURL = service.configurationURL(for: uncheckedHostProfile) vc?.delegate = self case .debugLogSegueIdentifier: diff --git a/Passepartout.xcodeproj/project.pbxproj b/Passepartout.xcodeproj/project.pbxproj index 5172f742..7ae60691 100644 --- a/Passepartout.xcodeproj/project.pbxproj +++ b/Passepartout.xcodeproj/project.pbxproj @@ -20,7 +20,7 @@ 0E1D72B4213C118500BA1586 /* ConfigurationViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E1D72B3213C118500BA1586 /* ConfigurationViewController.swift */; }; 0E2B494020FCFF990094784C /* Theme+Titles.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E2B493F20FCFF990094784C /* Theme+Titles.swift */; }; 0E2B494220FD16540094784C /* TransientStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E2B494120FD16540094784C /* TransientStore.swift */; }; - 0E2D11BA217DBEDE0096822C /* ProfileConfigurationFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E2D11B9217DBEDE0096822C /* ProfileConfigurationFactory.swift */; }; + 0E2D11BA217DBEDE0096822C /* ConnectionService+Configurations.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E2D11B9217DBEDE0096822C /* ConnectionService+Configurations.swift */; }; 0E39BCF0214B9EF10035E9DE /* WebServices.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E39BCEF214B9EF10035E9DE /* WebServices.swift */; }; 0E39BCF3214DA9310035E9DE /* AppConstants.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E39BCF2214DA9310035E9DE /* AppConstants.swift */; }; 0E3DA371215CB5BF00B40FC9 /* VersionViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E3DA370215CB5BF00B40FC9 /* VersionViewController.swift */; }; @@ -137,7 +137,7 @@ 0E1D72B3213C118500BA1586 /* ConfigurationViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConfigurationViewController.swift; sourceTree = ""; }; 0E2B493F20FCFF990094784C /* Theme+Titles.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Theme+Titles.swift"; sourceTree = ""; }; 0E2B494120FD16540094784C /* TransientStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TransientStore.swift; sourceTree = ""; }; - 0E2D11B9217DBEDE0096822C /* ProfileConfigurationFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProfileConfigurationFactory.swift; sourceTree = ""; }; + 0E2D11B9217DBEDE0096822C /* ConnectionService+Configurations.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ConnectionService+Configurations.swift"; sourceTree = ""; }; 0E39BCEF214B9EF10035E9DE /* WebServices.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebServices.swift; sourceTree = ""; }; 0E39BCF2214DA9310035E9DE /* AppConstants.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppConstants.swift; sourceTree = ""; }; 0E3DA370215CB5BF00B40FC9 /* VersionViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VersionViewController.swift; sourceTree = ""; }; @@ -382,12 +382,12 @@ 0EBE3AA2213DC1B000BFA2F5 /* Profiles */, 0EBE3A9E213DC1A100BFA2F5 /* ConnectionProfile.swift */, 0EBE3A9F213DC1A100BFA2F5 /* ConnectionService.swift */, + 0E2D11B9217DBEDE0096822C /* ConnectionService+Configurations.swift */, 0EBBE8F42182361700106008 /* ConnectionService+Migration.swift */, 0EDE8DE620C93945004C739C /* Credentials.swift */, 0EC7F20420E24308004EA58E /* DebugLog.swift */, 0ED38AE621404F100004D387 /* EndpointDataSource.swift */, 0E89DFC4213DF7AE00741BA1 /* Preferences.swift */, - 0E2D11B9217DBEDE0096822C /* ProfileConfigurationFactory.swift */, 0E89DFC7213E8FC500741BA1 /* SessionProxy+Communication.swift */, 0E2B494120FD16540094784C /* TransientStore.swift */, 0E4C9CB820DB9BC600A0C59C /* TrustedNetworks.swift */, @@ -842,7 +842,7 @@ 0ED31C1220CF0ABA0027975F /* Infrastructure.swift in Sources */, 0EC7F20520E24308004EA58E /* DebugLog.swift in Sources */, 0E4FD7E120D3E4C5002221FF /* MockVPNProvider.swift in Sources */, - 0E2D11BA217DBEDE0096822C /* ProfileConfigurationFactory.swift in Sources */, + 0E2D11BA217DBEDE0096822C /* ConnectionService+Configurations.swift in Sources */, 0EBE3A90213C6F4000BFA2F5 /* TrustPolicy.swift in Sources */, 0E6BE13F20CFBAB300A6DD36 /* DebugLogViewController.swift in Sources */, 0E89DFC8213E8FC500741BA1 /* SessionProxy+Communication.swift in Sources */, diff --git a/Passepartout/Sources/Model/ConnectionService+Configurations.swift b/Passepartout/Sources/Model/ConnectionService+Configurations.swift new file mode 100644 index 00000000..c22f1dc8 --- /dev/null +++ b/Passepartout/Sources/Model/ConnectionService+Configurations.swift @@ -0,0 +1,49 @@ +// +// ConnectionService+Configurations.swift +// Passepartout +// +// Created by Davide De Rosa on 10/22/18. +// Copyright (c) 2018 Davide De Rosa. All rights reserved. +// +// https://github.com/keeshux +// +// This file is part of Passepartout. +// +// Passepartout is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Passepartout is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Passepartout. If not, see . +// + +import Foundation + +extension ConnectionService { + func save(configurationURL: URL, for profile: ConnectionProfile) throws -> URL { + let destinationURL = targetConfigurationURL(for: profile) + let fm = FileManager.default + try? fm.removeItem(at: destinationURL) + try fm.copyItem(at: configurationURL, to: destinationURL) + return destinationURL + } + + func configurationURL(for profile: ConnectionProfile) -> URL? { + let url = targetConfigurationURL(for: profile) + guard FileManager.default.fileExists(atPath: url.path) else { + return nil + } + return url + } + + private func targetConfigurationURL(for profile: ConnectionProfile) -> URL { + let contextURL = ConnectionService.ProfileKey(profile).contextURL(in: self) + return contextURL.appendingPathComponent("\(profile.id).ovpn") + } +} diff --git a/Passepartout/Sources/Model/ConnectionService.swift b/Passepartout/Sources/Model/ConnectionService.swift index 03502b2a..cf04e423 100644 --- a/Passepartout/Sources/Model/ConnectionService.swift +++ b/Passepartout/Sources/Model/ConnectionService.swift @@ -64,19 +64,21 @@ class ConnectionService: Codable { id = profile.id } - fileprivate func profileURL(in service: ConnectionService) -> URL { - let contextURL: URL + func contextURL(in service: ConnectionService) -> URL { switch context { case .provider: - contextURL = service.providersURL + return service.providersURL case .host: - contextURL = service.hostsURL + return service.hostsURL } - return ConnectionService.url(in: contextURL, forProfileId: id) + } + + func profileURL(in service: ConnectionService) -> URL { + return ConnectionService.url(in: contextURL(in: service), forProfileId: id) } - fileprivate func profileData(in service: ConnectionService) throws -> Data { + func profileData(in service: ConnectionService) throws -> Data { return try Data(contentsOf: profileURL(in: service)) } diff --git a/Passepartout/Sources/Model/ProfileConfigurationFactory.swift b/Passepartout/Sources/Model/ProfileConfigurationFactory.swift deleted file mode 100644 index c97d3ce1..00000000 --- a/Passepartout/Sources/Model/ProfileConfigurationFactory.swift +++ /dev/null @@ -1,82 +0,0 @@ -// -// ProfileConfigurationFactory.swift -// Passepartout -// -// Created by Davide De Rosa on 10/22/18. -// Copyright (c) 2018 Davide De Rosa. All rights reserved. -// -// https://github.com/keeshux -// -// This file is part of Passepartout. -// -// Passepartout is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Passepartout is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Passepartout. If not, see . -// - -import Foundation - -protocol ProfileConfigurationSource { - var id: String { get } - - var profileDirectory: String { get } -} - -extension ProfileConfigurationSource { - var profileConfigurationPath: String { - return "\(profileDirectory)/\(id).ovpn" - } -} - -extension ProviderConnectionProfile: ProfileConfigurationSource { - var profileDirectory: String { - return AppConstants.Store.providersDirectory - } -} - -extension HostConnectionProfile: ProfileConfigurationSource { - var profileDirectory: String { - return AppConstants.Store.hostsDirectory - } -} - -class ProfileConfigurationFactory { - static let shared = ProfileConfigurationFactory() - - private let configurationsPath: URL - - private init() { - let fm = FileManager.default - configurationsPath = fm.userURL(for: .documentDirectory, appending: nil) - try? fm.createDirectory(at: configurationsPath, withIntermediateDirectories: false, attributes: nil) - } - - func save(url: URL, for profile: ProfileConfigurationSource) throws -> URL { - let savedUrl = targetConfigurationURL(for: profile) - let fm = FileManager.default - try? fm.removeItem(at: savedUrl) - try fm.copyItem(at: url, to: savedUrl) - return savedUrl - } - - func configurationURL(for profile: ProfileConfigurationSource) -> URL? { - let url = targetConfigurationURL(for: profile) - guard FileManager.default.fileExists(atPath: url.path) else { - return nil - } - return url - } - - private func targetConfigurationURL(for profile: ProfileConfigurationSource) -> URL { - return configurationsPath.appendingPathComponent(profile.profileConfigurationPath) - } -} From dfde9c51e317b7d32cb3524c1d7bcda6930b997c Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 23 Oct 2018 10:25:41 +0200 Subject: [PATCH 04/10] Fine-grain report attachments - Debug log - .ovpn profile (if any) --- Passepartout-iOS/Global/IssueReporter.swift | 41 +++++++++++++++---- .../Scenes/DebugLogViewController.swift | 2 +- .../Scenes/ServiceViewController.swift | 3 +- .../Resources/en.lproj/Localizable.strings | 2 +- Passepartout/Sources/AppConstants.swift | 24 +++++++---- Passepartout/Sources/SwiftGen+Strings.swift | 2 +- 6 files changed, 53 insertions(+), 21 deletions(-) diff --git a/Passepartout-iOS/Global/IssueReporter.swift b/Passepartout-iOS/Global/IssueReporter.swift index b4fd850d..5b2d0373 100644 --- a/Passepartout-iOS/Global/IssueReporter.swift +++ b/Passepartout-iOS/Global/IssueReporter.swift @@ -27,6 +27,22 @@ import Foundation import MessageUI class IssueReporter: NSObject { + struct Attachments { + let debugLog: Bool + + let configurationURL: URL? + + init(debugLog: Bool, configurationURL: URL?) { + self.debugLog = debugLog + self.configurationURL = configurationURL + } + + init(debugLog: Bool, profile: ConnectionProfile) { + let url = TransientStore.shared.service.configurationURL(for: profile) + self.init(debugLog: debugLog, configurationURL: url) + } + } + static let shared = IssueReporter() private weak var viewController: UIViewController? @@ -35,7 +51,7 @@ class IssueReporter: NSObject { super.init() } - func present(in viewController: UIViewController) { + func present(in viewController: UIViewController, withAttachments attachments: Attachments) { guard MFMailComposeViewController.canSendMail() else { let alert = Macros.alert(L10n.IssueReporter.title, L10n.IssueReporter.Alerts.EmailNotConfigured.message) alert.addCancelAction(L10n.Global.ok) @@ -45,17 +61,21 @@ class IssueReporter: NSObject { self.viewController = viewController - let alert = Macros.alert(L10n.IssueReporter.title, L10n.IssueReporter.message) - alert.addDefaultAction(L10n.IssueReporter.Buttons.accept) { - VPN.shared.requestDebugLog(fallback: AppConstants.Log.debugSnapshot) { - self.composeEmail(withDebugLog: $0) + if attachments.debugLog { + let alert = Macros.alert(L10n.IssueReporter.title, L10n.IssueReporter.message) + alert.addDefaultAction(L10n.IssueReporter.Buttons.accept) { + VPN.shared.requestDebugLog(fallback: AppConstants.Log.debugSnapshot) { + self.composeEmail(withDebugLog: $0, configurationURL: attachments.configurationURL) + } } + alert.addCancelAction(L10n.Global.cancel) + viewController.present(alert, animated: true, completion: nil) + } else { + composeEmail(withDebugLog: nil, configurationURL: attachments.configurationURL) } - alert.addCancelAction(L10n.Global.cancel) - viewController.present(alert, animated: true, completion: nil) } - private func composeEmail(withDebugLog debugLog: String?) { + private func composeEmail(withDebugLog debugLog: String?, configurationURL: URL?) { let metadata = DebugLog(raw: "--").decoratedString() let vc = MFMailComposeViewController() @@ -64,7 +84,10 @@ class IssueReporter: NSObject { vc.setMessageBody(L10n.IssueReporter.Email.body(metadata), isHTML: false) if let raw = debugLog { let attachment = DebugLog(raw: raw).decoratedData() - vc.addAttachmentData(attachment, mimeType: AppConstants.IssueReporter.attachmentMIME, fileName: AppConstants.Log.debugFilename) + vc.addAttachmentData(attachment, mimeType: AppConstants.IssueReporter.MIME.debugLog, fileName: AppConstants.IssueReporter.Filenames.debugLog) + } + if let cfg = configurationURL, let attachment = try? Data(contentsOf: cfg) { + vc.addAttachmentData(attachment, mimeType: AppConstants.IssueReporter.MIME.configuration, fileName: AppConstants.IssueReporter.Filenames.configuration) } vc.mailComposeDelegate = self vc.apply(Theme.current) diff --git a/Passepartout-iOS/Scenes/DebugLogViewController.swift b/Passepartout-iOS/Scenes/DebugLogViewController.swift index 34bc5b57..7ed14daa 100644 --- a/Passepartout-iOS/Scenes/DebugLogViewController.swift +++ b/Passepartout-iOS/Scenes/DebugLogViewController.swift @@ -64,7 +64,7 @@ class DebugLogViewController: UIViewController { } let data = DebugLog(raw: raw).decoratedData() - let path = NSTemporaryDirectory().appending(AppConstants.Log.debugFilename) + let path = NSTemporaryDirectory().appending(AppConstants.IssueReporter.Filenames.debugLog) let url = URL(fileURLWithPath: path) do { try data.write(to: url) diff --git a/Passepartout-iOS/Scenes/ServiceViewController.swift b/Passepartout-iOS/Scenes/ServiceViewController.swift index be20305f..e3b27951 100644 --- a/Passepartout-iOS/Scenes/ServiceViewController.swift +++ b/Passepartout-iOS/Scenes/ServiceViewController.swift @@ -361,7 +361,8 @@ class ServiceViewController: UIViewController, TableModelHost { } private func reportConnectivityIssue() { - IssueReporter.shared.present(in: self) + let attach = IssueReporter.Attachments(debugLog: true, profile: uncheckedProfile) + IssueReporter.shared.present(in: self, withAttachments: attach) } // MARK: Notifications diff --git a/Passepartout/Resources/en.lproj/Localizable.strings b/Passepartout/Resources/en.lproj/Localizable.strings index b1ec559b..67e26879 100644 --- a/Passepartout/Resources/en.lproj/Localizable.strings +++ b/Passepartout/Resources/en.lproj/Localizable.strings @@ -174,7 +174,7 @@ "issue_reporter.message" = "The debug log of your latest connections is crucial to resolve your connectivity issues and is completely anonymous."; "issue_reporter.buttons.accept" = "I understand"; "issue_reporter.alerts.email_not_configured.message" = "No e-mail account is configured."; -"issue_reporter.email.subject" = "%@ - Debug log"; +"issue_reporter.email.subject" = "%@ - Report issue"; "issue_reporter.email.body" = "Hi,\n\ndescription of the issue:\n\n%@\n\nRegards"; "about.title" = "About"; diff --git a/Passepartout/Sources/AppConstants.swift b/Passepartout/Sources/AppConstants.swift index 8c22f975..b04ce544 100644 --- a/Passepartout/Sources/AppConstants.swift +++ b/Passepartout/Sources/AppConstants.swift @@ -96,13 +96,6 @@ class AppConstants { static var debugSnapshot: () -> String = { TransientStore.shared.service.vpnLog } - static var debugFilename: String { - let fmt = DateFormatter() - fmt.dateFormat = "yyyyMMdd-HHmmss" - let iso = fmt.string(from: Date()) - return "debug-\(iso).txt" - } - static let viewerRefreshInterval: TimeInterval = 3.0 static func configure() { @@ -116,7 +109,22 @@ class AppConstants { class IssueReporter { static let recipient = "issues@\(Domain.name)" - static let attachmentMIME = "text/plain" + class Filenames { + static var debugLog: String { + let fmt = DateFormatter() + fmt.dateFormat = "yyyyMMdd-HHmmss" + let iso = fmt.string(from: Date()) + return "debug-\(iso).txt" + } + + static let configuration = "profile.ovpn" + } + + class MIME { + static let debugLog = "text/plain" + + static let configuration = "application/x-openvpn-profile" + } } class URLs { diff --git a/Passepartout/Sources/SwiftGen+Strings.swift b/Passepartout/Sources/SwiftGen+Strings.swift index 24dad087..14840762 100644 --- a/Passepartout/Sources/SwiftGen+Strings.swift +++ b/Passepartout/Sources/SwiftGen+Strings.swift @@ -324,7 +324,7 @@ internal enum L10n { internal static func body(_ p1: String) -> String { return L10n.tr("Localizable", "issue_reporter.email.body", p1) } - /// %@ - Debug log + /// %@ - Report issue internal static func subject(_ p1: String) -> String { return L10n.tr("Localizable", "issue_reporter.email.subject", p1) } From a69c7c57331a1f858338f6eedcee734184c5d984 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 23 Oct 2018 10:39:50 +0200 Subject: [PATCH 05/10] Attach .ovpn as plain text Can easily inspect via email. --- Passepartout/Sources/AppConstants.swift | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Passepartout/Sources/AppConstants.swift b/Passepartout/Sources/AppConstants.swift index b04ce544..ed6f908a 100644 --- a/Passepartout/Sources/AppConstants.swift +++ b/Passepartout/Sources/AppConstants.swift @@ -117,13 +117,15 @@ class AppConstants { return "debug-\(iso).txt" } - static let configuration = "profile.ovpn" +// static let configuration = "profile.ovpn" + static let configuration = "profile.ovpn.txt" } class MIME { static let debugLog = "text/plain" - static let configuration = "application/x-openvpn-profile" +// static let configuration = "application/x-openvpn-profile" + static let configuration = "text/plain" } } From c7639daf0d4e029ea682b7bc3a57053105686e07 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 00:17:28 +0200 Subject: [PATCH 06/10] Strip configuration file before attaching Of sensitive or private data. --- Passepartout-iOS/Global/IssueReporter.swift | 12 ++++++-- .../TunnelKitProvider+FileConfiguration.swift | 30 +++++++++++++++++-- .../FileConfigurationTests.swift | 7 +++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/Passepartout-iOS/Global/IssueReporter.swift b/Passepartout-iOS/Global/IssueReporter.swift index 5b2d0373..4718da90 100644 --- a/Passepartout-iOS/Global/IssueReporter.swift +++ b/Passepartout-iOS/Global/IssueReporter.swift @@ -24,6 +24,7 @@ // import Foundation +import TunnelKit import MessageUI class IssueReporter: NSObject { @@ -86,8 +87,15 @@ class IssueReporter: NSObject { let attachment = DebugLog(raw: raw).decoratedData() vc.addAttachmentData(attachment, mimeType: AppConstants.IssueReporter.MIME.debugLog, fileName: AppConstants.IssueReporter.Filenames.debugLog) } - if let cfg = configurationURL, let attachment = try? Data(contentsOf: cfg) { - vc.addAttachmentData(attachment, mimeType: AppConstants.IssueReporter.MIME.configuration, fileName: AppConstants.IssueReporter.Filenames.configuration) + if let url = configurationURL { + var lines: [String] = [] + do { + _ = try TunnelKitProvider.Configuration.parsed(from: url, stripped: &lines) + if let attachment = lines.joined(separator: "\n").data(using: .utf8) { + vc.addAttachmentData(attachment, mimeType: AppConstants.IssueReporter.MIME.configuration, fileName: AppConstants.IssueReporter.Filenames.configuration) + } + } catch { + } } vc.mailComposeDelegate = self vc.apply(Theme.current) diff --git a/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift b/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift index b3d29946..c46f55dc 100644 --- a/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift +++ b/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift @@ -62,7 +62,7 @@ extension TunnelKitProvider.Configuration { static let blockEnd = Utils.regex("^<\\/[\\w\\-]+>") } - static func parsed(from url: URL) throws -> (String, TunnelKitProvider.Configuration) { + static func parsed(from url: URL, stripped: UnsafeMutablePointer<[String]>? = nil) throws -> (String, TunnelKitProvider.Configuration) { let lines = try String(contentsOf: url).trimmedLines() var defaultProto: TunnelKitProvider.SocketType? @@ -90,7 +90,16 @@ extension TunnelKitProvider.Configuration { for line in lines { log.verbose(line) + var isHandled = false + var strippedLine = line + defer { + if isHandled { + stripped?.pointee.append(strippedLine) + } + } + Regex.blockBegin.enumerateComponents(in: line) { + isHandled = true let tag = $0.first! let from = tag.index(after: tag.startIndex) let to = tag.index(before: tag.endIndex) @@ -99,6 +108,7 @@ extension TunnelKitProvider.Configuration { currentBlock = [] } Regex.blockEnd.enumerateComponents(in: line) { + isHandled = true let tag = $0.first! let from = tag.index(tag.startIndex, offsetBy: 2) let to = tag.index(before: tag.endIndex) @@ -138,8 +148,9 @@ extension TunnelKitProvider.Configuration { currentBlock.append(line) continue } - + Regex.proto.enumerateArguments(in: line) { + isHandled = true guard let str = $0.first else { return } @@ -149,26 +160,35 @@ extension TunnelKitProvider.Configuration { } } Regex.port.enumerateArguments(in: line) { + isHandled = true guard let str = $0.first else { return } defaultPort = UInt16(str) } Regex.remote.enumerateArguments(in: line) { + isHandled = true guard let hostname = $0.first else { return } var port: UInt16? var proto: TunnelKitProvider.SocketType? + var strippedComponents = ["remote", ""] if $0.count > 1 { port = UInt16($0[1]) + strippedComponents.append($0[1]) } if $0.count > 2 { proto = TunnelKitProvider.SocketType(protoString: $0[2]) + strippedComponents.append($0[2]) } remotes.append((hostname, port, proto)) + + // replace private data + strippedLine = strippedComponents.joined(separator: " ") } Regex.cipher.enumerateArguments(in: line) { + isHandled = true guard let rawValue = $0.first else { return } @@ -178,6 +198,7 @@ extension TunnelKitProvider.Configuration { } } Regex.auth.enumerateArguments(in: line) { + isHandled = true guard let rawValue = $0.first else { return } @@ -187,24 +208,29 @@ extension TunnelKitProvider.Configuration { } } Regex.compLZO.enumerateComponents(in: line) { _ in + isHandled = true compressionFraming = .compLZO } Regex.compress.enumerateComponents(in: line) { _ in + isHandled = true compressionFraming = .compress } Regex.keyDirection.enumerateArguments(in: line) { + isHandled = true guard let arg = $0.first, let value = Int(arg) else { return } keyDirection = StaticKey.Direction(rawValue: value) } Regex.ping.enumerateArguments(in: line) { + isHandled = true guard let arg = $0.first else { return } keepAliveSeconds = TimeInterval(arg) } Regex.renegSec.enumerateArguments(in: line) { + isHandled = true guard let arg = $0.first else { return } diff --git a/PassepartoutTests-iOS/FileConfigurationTests.swift b/PassepartoutTests-iOS/FileConfigurationTests.swift index 82b82035..ead8d5b5 100644 --- a/PassepartoutTests-iOS/FileConfigurationTests.swift +++ b/PassepartoutTests-iOS/FileConfigurationTests.swift @@ -44,6 +44,13 @@ class FileConfigurationTests: XCTestCase { XCTAssertEqual(cfg.sessionConfiguration.digest, .sha1) } + func testStripped() throws { + var lines: [String] = [] + _ = try TunnelKitProvider.Configuration.parsed(from: url(withName: "pia-hungary"), stripped: &lines) + let cfg = lines.joined(separator: "\n") + print(cfg) + } + private func url(withName name: String) -> URL { return Bundle(for: FileConfigurationTests.self).url(forResource: name, withExtension: "ovpn")! } From 73e09fefb1bb4156e81b58c80a1c1a500d58e0dd Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 09:43:30 +0200 Subject: [PATCH 07/10] Retain unhandled mtu/mssfix lines in stripped --- .../Sources/VPN/TunnelKitProvider+FileConfiguration.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift b/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift index c46f55dc..fd526e56 100644 --- a/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift +++ b/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift @@ -245,6 +245,9 @@ extension TunnelKitProvider.Configuration { Regex.externalFiles.enumerateArguments(in: line) { (_) in unsupportedError = ApplicationError.unsupportedConfiguration(option: "external file: \"\(line)\"") } + if line.contains("mtu") || line.contains("mssfix") { + isHandled = true + } if let error = unsupportedError { throw error From 4b075bcc95f503cc9dbcf1abb728db44e8418aa5 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Tue, 23 Oct 2018 10:49:35 +0200 Subject: [PATCH 08/10] Improve alerts on configuration import error Issue reporting is currently disabled because un unparsed .ovpn may contain sensitive data. --- Passepartout-iOS/AppDelegate.swift | 24 +++++++--- Passepartout-iOS/Global/IssueReporter.swift | 10 +++-- .../Organizer/WizardHostViewController.swift | 2 +- .../Resources/en.lproj/Localizable.strings | 11 +++-- Passepartout/Sources/ApplicationError.swift | 4 +- Passepartout/Sources/SwiftGen+Strings.swift | 45 ++++++++++++++----- .../TunnelKitProvider+FileConfiguration.swift | 4 +- 7 files changed, 69 insertions(+), 31 deletions(-) diff --git a/Passepartout-iOS/AppDelegate.swift b/Passepartout-iOS/AppDelegate.swift index 31688920..f8ddbce8 100644 --- a/Passepartout-iOS/AppDelegate.swift +++ b/Passepartout-iOS/AppDelegate.swift @@ -118,17 +118,29 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDele } nav.modalPresentationStyle = .formSheet root.present(nav, animated: true, completion: nil) + } catch ApplicationError.missingConfiguration(let option) { + let message = L10n.Wizards.Host.Alerts.Missing.message(option) + alertConfigurationImportError(url: url, in: root, withMessage: message) } catch ApplicationError.unsupportedConfiguration(let option) { - let alert = Macros.alert(L10n.Organizer.Sections.Hosts.header, L10n.Wizards.Host.Alerts.unsupported(option)) - alert.addCancelAction(L10n.Global.ok) - root.present(alert, animated: true, completion: nil) + let message = L10n.Wizards.Host.Alerts.Unsupported.message(option) + alertConfigurationImportError(url: url, in: root, withMessage: message) } catch let e { - let alert = Macros.alert(L10n.Organizer.Sections.Hosts.header, L10n.Wizards.Host.Alerts.parsing(e.localizedDescription)) - alert.addCancelAction(L10n.Global.ok) - root.present(alert, animated: true, completion: nil) + let message = L10n.Wizards.Host.Alerts.Parsing.message(e.localizedDescription) + alertConfigurationImportError(url: url, in: root, withMessage: message) } return true } + + private func alertConfigurationImportError(url: URL, in vc: UIViewController, withMessage message: String) { + let alert = Macros.alert(L10n.Organizer.Sections.Hosts.header, message) +// alert.addDefaultAction(L10n.Wizards.Host.Alerts.Buttons.report) { +// var attach = IssueReporter.Attachments(debugLog: false, configurationURL: url) +// attach.description = message +// IssueReporter.shared.present(in: vc, withAttachments: attach) +// } + alert.addCancelAction(L10n.Global.cancel) + vc.present(alert, animated: true, completion: nil) + } } extension UISplitViewController { diff --git a/Passepartout-iOS/Global/IssueReporter.swift b/Passepartout-iOS/Global/IssueReporter.swift index 4718da90..473e8a5b 100644 --- a/Passepartout-iOS/Global/IssueReporter.swift +++ b/Passepartout-iOS/Global/IssueReporter.swift @@ -33,6 +33,8 @@ class IssueReporter: NSObject { let configurationURL: URL? + var description: String? + init(debugLog: Bool, configurationURL: URL?) { self.debugLog = debugLog self.configurationURL = configurationURL @@ -66,23 +68,23 @@ class IssueReporter: NSObject { let alert = Macros.alert(L10n.IssueReporter.title, L10n.IssueReporter.message) alert.addDefaultAction(L10n.IssueReporter.Buttons.accept) { VPN.shared.requestDebugLog(fallback: AppConstants.Log.debugSnapshot) { - self.composeEmail(withDebugLog: $0, configurationURL: attachments.configurationURL) + self.composeEmail(withDebugLog: $0, configurationURL: attachments.configurationURL, description: attachments.description) } } alert.addCancelAction(L10n.Global.cancel) viewController.present(alert, animated: true, completion: nil) } else { - composeEmail(withDebugLog: nil, configurationURL: attachments.configurationURL) + composeEmail(withDebugLog: nil, configurationURL: attachments.configurationURL, description: attachments.description) } } - private func composeEmail(withDebugLog debugLog: String?, configurationURL: URL?) { + private func composeEmail(withDebugLog debugLog: String?, configurationURL: URL?, description: String?) { let metadata = DebugLog(raw: "--").decoratedString() let vc = MFMailComposeViewController() vc.setToRecipients([AppConstants.IssueReporter.recipient]) vc.setSubject(L10n.IssueReporter.Email.subject(GroupConstants.App.name)) - vc.setMessageBody(L10n.IssueReporter.Email.body(metadata), isHTML: false) + vc.setMessageBody(L10n.IssueReporter.Email.body(description ?? L10n.IssueReporter.Email.description, metadata), isHTML: false) if let raw = debugLog { let attachment = DebugLog(raw: raw).decoratedData() vc.addAttachmentData(attachment, mimeType: AppConstants.IssueReporter.MIME.debugLog, fileName: AppConstants.IssueReporter.Filenames.debugLog) diff --git a/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift b/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift index 2fb94b15..70be468c 100644 --- a/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift @@ -134,7 +134,7 @@ class WizardHostViewController: UITableViewController, TableModelHost, Wizard { profile.parameters = file.configuration guard !TransientStore.shared.service.containsProfile(profile) else { - let alert = Macros.alert(title, L10n.Wizards.Host.Alerts.existing) + let alert = Macros.alert(title, L10n.Wizards.Host.Alerts.Existing.message) alert.addDefaultAction(L10n.Global.ok) { self.next(withProfile: profile) } diff --git a/Passepartout/Resources/en.lproj/Localizable.strings b/Passepartout/Resources/en.lproj/Localizable.strings index 67e26879..da3f17c1 100644 --- a/Passepartout/Resources/en.lproj/Localizable.strings +++ b/Passepartout/Resources/en.lproj/Localizable.strings @@ -51,9 +51,11 @@ "wizards.host.cells.title_input.caption" = "Title"; "wizards.host.cells.title_input.placeholder" = "My Profile"; "wizards.host.sections.existing.header" = "Existing profiles"; -"wizards.host.alerts.existing" = "A host profile with the same title already exists. Replace it?"; -"wizards.host.alerts.unsupported" = "The configuration file contains an unsupported option (%@)."; -"wizards.host.alerts.parsing" = "Unable to parse the provided configuration file (%@)."; +"wizards.host.alerts.existing.message" = "A host profile with the same title already exists. Replace it?"; +"wizards.host.alerts.missing.message" = "The configuration file lacks a required option (%@)."; +"wizards.host.alerts.unsupported.message" = "The configuration file contains an unsupported option (%@)."; +"wizards.host.alerts.parsing.message" = "Unable to parse the provided configuration file (%@)."; +"wizards.host.alerts.buttons.report" = "Report an issue"; "service.welcome.message" = "Welcome to Passepartout!\n\nUse the organizer to add a new profile."; "service.sections.general.header" = "General"; @@ -175,7 +177,8 @@ "issue_reporter.buttons.accept" = "I understand"; "issue_reporter.alerts.email_not_configured.message" = "No e-mail account is configured."; "issue_reporter.email.subject" = "%@ - Report issue"; -"issue_reporter.email.body" = "Hi,\n\ndescription of the issue:\n\n%@\n\nRegards"; +"issue_reporter.email.body" = "Hi,\n\n%@\n\n%@\n\nRegards"; +"issue_reporter.email.description" = "description of the issue:"; "about.title" = "About"; "about.sections.info.header" = "General"; diff --git a/Passepartout/Sources/ApplicationError.swift b/Passepartout/Sources/ApplicationError.swift index 0d1815bc..ad91657c 100644 --- a/Passepartout/Sources/ApplicationError.swift +++ b/Passepartout/Sources/ApplicationError.swift @@ -30,10 +30,8 @@ enum ApplicationError: Error { case missingCredentials - case missingCA + case missingConfiguration(option: String) - case emptyRemotes - case unsupportedConfiguration(option: String) case migration diff --git a/Passepartout/Sources/SwiftGen+Strings.swift b/Passepartout/Sources/SwiftGen+Strings.swift index 14840762..5edc8d28 100644 --- a/Passepartout/Sources/SwiftGen+Strings.swift +++ b/Passepartout/Sources/SwiftGen+Strings.swift @@ -320,10 +320,12 @@ internal enum L10n { } internal enum Email { - /// Hi,\n\ndescription of the issue:\n\n%@\n\nRegards - internal static func body(_ p1: String) -> String { - return L10n.tr("Localizable", "issue_reporter.email.body", p1) + /// Hi,\n\n%@\n\n%@\n\nRegards + internal static func body(_ p1: String, _ p2: String) -> String { + return L10n.tr("Localizable", "issue_reporter.email.body", p1, p2) } + /// description of the issue: + internal static let description = L10n.tr("Localizable", "issue_reporter.email.description") /// %@ - Report issue internal static func subject(_ p1: String) -> String { return L10n.tr("Localizable", "issue_reporter.email.subject", p1) @@ -731,15 +733,36 @@ internal enum L10n { internal enum Host { internal enum Alerts { - /// A host profile with the same title already exists. Replace it? - internal static let existing = L10n.tr("Localizable", "wizards.host.alerts.existing") - /// Unable to parse the provided configuration file (%@). - internal static func parsing(_ p1: String) -> String { - return L10n.tr("Localizable", "wizards.host.alerts.parsing", p1) + + internal enum Buttons { + /// Report an issue + internal static let report = L10n.tr("Localizable", "wizards.host.alerts.buttons.report") } - /// The configuration file contains an unsupported option (%@). - internal static func unsupported(_ p1: String) -> String { - return L10n.tr("Localizable", "wizards.host.alerts.unsupported", p1) + + internal enum Existing { + /// A host profile with the same title already exists. Replace it? + internal static let message = L10n.tr("Localizable", "wizards.host.alerts.existing.message") + } + + internal enum Missing { + /// The configuration file lacks a required option (%@). + internal static func message(_ p1: String) -> String { + return L10n.tr("Localizable", "wizards.host.alerts.missing.message", p1) + } + } + + internal enum Parsing { + /// Unable to parse the provided configuration file (%@). + internal static func message(_ p1: String) -> String { + return L10n.tr("Localizable", "wizards.host.alerts.parsing.message", p1) + } + } + + internal enum Unsupported { + /// The configuration file contains an unsupported option (%@). + internal static func message(_ p1: String) -> String { + return L10n.tr("Localizable", "wizards.host.alerts.unsupported.message", p1) + } } } diff --git a/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift b/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift index fd526e56..7c70d081 100644 --- a/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift +++ b/Passepartout/Sources/VPN/TunnelKitProvider+FileConfiguration.swift @@ -255,13 +255,13 @@ extension TunnelKitProvider.Configuration { } guard let ca = optCA else { - throw ApplicationError.missingCA + throw ApplicationError.missingConfiguration(option: "ca") } // XXX: only reads first remote // hostnames = remotes.map { $0.0 } guard !remotes.isEmpty else { - throw ApplicationError.emptyRemotes + throw ApplicationError.missingConfiguration(option: "remote") } let hostname = remotes[0].0 From e00129c8a5631bbba63d0406b79e4e10af3ab8ce Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 00:47:28 +0200 Subject: [PATCH 09/10] Adjust issue alert to new .ovpn attachment --- Passepartout/Resources/en.lproj/Localizable.strings | 2 +- Passepartout/Sources/SwiftGen+Strings.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Passepartout/Resources/en.lproj/Localizable.strings b/Passepartout/Resources/en.lproj/Localizable.strings index da3f17c1..aec45ff9 100644 --- a/Passepartout/Resources/en.lproj/Localizable.strings +++ b/Passepartout/Resources/en.lproj/Localizable.strings @@ -173,7 +173,7 @@ "vpn.errors.network" = "Network changed"; "issue_reporter.title" = "Report issue"; -"issue_reporter.message" = "The debug log of your latest connections is crucial to resolve your connectivity issues and is completely anonymous."; +"issue_reporter.message" = "The debug log of your latest connections is crucial to resolve your connectivity issues and is completely anonymous.\n\nThe .ovpn configuration file, if any, is attached stripped of any sensitive data.\n\nPlease double check the email attachments if unsure."; "issue_reporter.buttons.accept" = "I understand"; "issue_reporter.alerts.email_not_configured.message" = "No e-mail account is configured."; "issue_reporter.email.subject" = "%@ - Report issue"; diff --git a/Passepartout/Sources/SwiftGen+Strings.swift b/Passepartout/Sources/SwiftGen+Strings.swift index 5edc8d28..e6827a50 100644 --- a/Passepartout/Sources/SwiftGen+Strings.swift +++ b/Passepartout/Sources/SwiftGen+Strings.swift @@ -301,7 +301,7 @@ internal enum L10n { } internal enum IssueReporter { - /// The debug log of your latest connections is crucial to resolve your connectivity issues and is completely anonymous. + /// The debug log of your latest connections is crucial to resolve your connectivity issues and is completely anonymous.\n\nThe .ovpn configuration file, if any, is attached stripped of any sensitive data.\n\nPlease double check the email attachments if unsure. internal static let message = L10n.tr("Localizable", "issue_reporter.message") /// Report issue internal static let title = L10n.tr("Localizable", "issue_reporter.title") From 22e3a1f3a5122924b1f441a29eae7c4602a8cf44 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 00:55:08 +0200 Subject: [PATCH 10/10] Update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0a81789..d475f196 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Added + +- Attach .ovpn when reporting a connectivity issue, stripped of sensitive data. [#13](https://github.com/keeshux/passepartout-ios/pull/13) + ## 1.0 beta 1107 (2018-10-26) ### Changed