From 995009121a4ad02fb9d8a3f2980ea9dd1693e396 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Thu, 18 Nov 2021 12:05:06 +0100 Subject: [PATCH] Revert "Avoid caching PEMs on disk (#213)" This reverts commit 00d908cc8925f26dce24c8282e2682383b60e045. --- CHANGELOG.md | 1 - Sources/CTunnelKitOpenVPNProtocol/TLSBox.m | 89 +++++-------------- .../include/TLSBox.h | 13 ++- .../OpenVPNTunnelProvider.swift | 2 +- .../OpenVPNSession.swift | 69 ++++++++++---- .../EncryptionTests.swift | 5 -- 6 files changed, 83 insertions(+), 96 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d2a1a3..1e65086 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Avoid caching PEMs on disk (roop). [#213](https://github.com/passepartoutvpn/tunnelkit/pull/213) - Upgrade OpenSSL to 1.1.1l. ## 3.4.0 (2021-08-07) diff --git a/Sources/CTunnelKitOpenVPNProtocol/TLSBox.m b/Sources/CTunnelKitOpenVPNProtocol/TLSBox.m index 10e4c6e..96810a5 100644 --- a/Sources/CTunnelKitOpenVPNProtocol/TLSBox.m +++ b/Sources/CTunnelKitOpenVPNProtocol/TLSBox.m @@ -58,9 +58,9 @@ int TLSBoxVerifyPeer(int ok, X509_STORE_CTX *ctx) { @interface TLSBox () -@property (nonatomic, strong) NSString *caPEM; -@property (nonatomic, strong) NSString *clientCertificatePEM; -@property (nonatomic, strong) NSString *clientKeyPEM; +@property (nonatomic, strong) NSString *caPath; +@property (nonatomic, strong) NSString *clientCertificatePath; +@property (nonatomic, strong) NSString *clientKeyPath; @property (nonatomic, assign) BOOL checksEKU; @property (nonatomic, assign) BOOL checksSANHost; @property (nonatomic, strong) NSString *hostname; @@ -76,10 +76,6 @@ int TLSBoxVerifyPeer(int ok, X509_STORE_CTX *ctx) { @end -static BIO *create_BIO_from_PEM(NSString *pem) { - return BIO_new_mem_buf([pem cStringUsingEncoding:NSASCIIStringEncoding], (int)[pem length]); -} - @implementation TLSBox + (NSString *)md5ForCertificatePath:(NSString *)path error:(NSError * _Nullable __autoreleasing * _Nullable)error @@ -109,31 +105,6 @@ static BIO *create_BIO_from_PEM(NSString *pem) { return hex; } -+ (NSString *)md5ForCertificatePEM:(NSString *)pem error:(NSError * _Nullable __autoreleasing * _Nullable)error -{ - const EVP_MD *alg = EVP_get_digestbyname("MD5"); - uint8_t md[16]; - unsigned int len; - - BIO *bio = create_BIO_from_PEM(pem); - X509 *cert = PEM_read_bio_X509(bio, NULL, NULL, NULL); - - if (!cert) { - BIO_free(bio); - return NULL; - } - X509_digest(cert, alg, md, &len); - X509_free(cert); - BIO_free(bio); - NSCAssert2(len == sizeof(md), @"Unexpected MD5 size (%d != %lu)", len, sizeof(md)); - - NSMutableString *hex = [[NSMutableString alloc] initWithCapacity:2 * sizeof(md)]; - for (int i = 0; i < sizeof(md); ++i) { - [hex appendFormat:@"%02x", md[i]]; - } - return hex; -} - + (NSString *)decryptedPrivateKeyFromPath:(NSString *)path passphrase:(NSString *)passphrase error:(NSError * _Nullable __autoreleasing *)error { BIO *bio; @@ -194,17 +165,17 @@ static BIO *create_BIO_from_PEM(NSString *pem) { return nil; } -- (instancetype)initWithCA:(nonnull NSString *)caPEM - clientCertificate:(nullable NSString *)clientCertificatePEM - clientKey:(nullable NSString *)clientKeyPEM - checksEKU:(BOOL)checksEKU - checksSANHost:(BOOL)checksSANHost - hostname:(nullable NSString *)hostname +- (instancetype)initWithCAPath:(NSString *)caPath + clientCertificatePath:(NSString *)clientCertificatePath + clientKeyPath:(NSString *)clientKeyPath + checksEKU:(BOOL)checksEKU + checksSANHost:(BOOL)checksSANHost + hostname:(nullable NSString *)hostname { if ((self = [super init])) { - self.caPEM = caPEM; - self.clientCertificatePEM = clientCertificatePEM; - self.clientKeyPEM = clientKeyPEM; + self.caPath = caPath; + self.clientCertificatePath = clientCertificatePath; + self.clientKeyPath = clientKeyPath; self.checksEKU = checksEKU; self.checksSANHost = checksSANHost; self.bufferCipherText = allocate_safely(TLSBoxMaxBufferLength); @@ -234,47 +205,31 @@ static BIO *create_BIO_from_PEM(NSString *pem) { self.ctx = SSL_CTX_new(TLS_client_method()); SSL_CTX_set_options(self.ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3 | SSL_OP_NO_COMPRESSION); SSL_CTX_set_verify(self.ctx, SSL_VERIFY_PEER, TLSBoxVerifyPeer); - - if (self.caPEM) { - BIO *bio = create_BIO_from_PEM(self.caPEM); - X509 *ca = PEM_read_bio_X509(bio, NULL, NULL, NULL); - BIO_free(bio); - X509_STORE *trustedStore = SSL_CTX_get_cert_store(self.ctx); - if (!X509_STORE_add_cert(trustedStore, ca)) { - ERR_print_errors_fp(stdout); - if (error) { - *error = TunnelKitErrorWithCode(TunnelKitErrorCodeTLSCertificateAuthority); - } - return NO; + if (!SSL_CTX_load_verify_locations(self.ctx, [self.caPath cStringUsingEncoding:NSASCIIStringEncoding], NULL)) { + ERR_print_errors_fp(stdout); + if (error) { + *error = TunnelKitErrorWithCode(TunnelKitErrorCodeTLSCertificateAuthority); } + return NO; } - - if (self.clientCertificatePEM) { - BIO *bio = create_BIO_from_PEM(self.clientCertificatePEM); - X509 *cert = PEM_read_bio_X509(bio, NULL, NULL, NULL); - BIO_free(bio); - if (!SSL_CTX_use_certificate(self.ctx, cert)) { + + if (self.clientCertificatePath) { + if (!SSL_CTX_use_certificate_file(self.ctx, [self.clientCertificatePath cStringUsingEncoding:NSASCIIStringEncoding], SSL_FILETYPE_PEM)) { ERR_print_errors_fp(stdout); if (error) { *error = TunnelKitErrorWithCode(TunnelKitErrorCodeTLSClientCertificate); } return NO; } - X509_free(cert); - if (self.clientKeyPEM) { - BIO *bio = create_BIO_from_PEM(self.clientKeyPEM); - EVP_PKEY *pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); - BIO_free(bio); - if (!SSL_CTX_use_PrivateKey(self.ctx, pkey)) { + if (self.clientKeyPath) { + if (!SSL_CTX_use_PrivateKey_file(self.ctx, [self.clientKeyPath cStringUsingEncoding:NSASCIIStringEncoding], SSL_FILETYPE_PEM)) { ERR_print_errors_fp(stdout); if (error) { *error = TunnelKitErrorWithCode(TunnelKitErrorCodeTLSClientKey); } return NO; - } - EVP_PKEY_free(pkey); } } diff --git a/Sources/CTunnelKitOpenVPNProtocol/include/TLSBox.h b/Sources/CTunnelKitOpenVPNProtocol/include/TLSBox.h index 50c803f..bc1f975 100644 --- a/Sources/CTunnelKitOpenVPNProtocol/include/TLSBox.h +++ b/Sources/CTunnelKitOpenVPNProtocol/include/TLSBox.h @@ -51,16 +51,15 @@ extern NSString *const TLSBoxPeerVerificationErrorNotification; @interface TLSBox : NSObject + (nullable NSString *)md5ForCertificatePath:(NSString *)path error:(NSError **)error; -+ (nullable NSString *)md5ForCertificatePEM:(NSString *)pem error:(NSError **)error; + (nullable NSString *)decryptedPrivateKeyFromPath:(NSString *)path passphrase:(NSString *)passphrase error:(NSError **)error; + (nullable NSString *)decryptedPrivateKeyFromPEM:(NSString *)pem passphrase:(NSString *)passphrase error:(NSError **)error; -- (instancetype)initWithCA:(nonnull NSString *)caPEM - clientCertificate:(nullable NSString *)clientCertificatePEM - clientKey:(nullable NSString *)clientKeyPEM - checksEKU:(BOOL)checksEKU - checksSANHost:(BOOL)checksSANHost - hostname:(nullable NSString *)hostname; +- (instancetype)initWithCAPath:(NSString *)caPath + clientCertificatePath:(nullable NSString *)clientCertificatePath + clientKeyPath:(nullable NSString *)clientKeyPath + checksEKU:(BOOL)checksEKU + checksSANHost:(BOOL)checksSANHost + hostname:(nullable NSString *)hostname; - (BOOL)startWithError:(NSError **)error; diff --git a/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift b/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift index e53086f..5cae71c 100644 --- a/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift +++ b/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift @@ -242,7 +242,7 @@ open class OpenVPNTunnelProvider: NEPacketTunnelProvider { let session: OpenVPNSession do { - session = try OpenVPNSession(queue: tunnelQueue, configuration: cfg.sessionConfiguration) + session = try OpenVPNSession(queue: tunnelQueue, configuration: cfg.sessionConfiguration, cachesURL: cachesURL) refreshDataCount() } catch let e { completionHandler(e) diff --git a/Sources/TunnelKitOpenVPNProtocol/OpenVPNSession.swift b/Sources/TunnelKitOpenVPNProtocol/OpenVPNSession.swift index 3d30fa2..2bc4791 100644 --- a/Sources/TunnelKitOpenVPNProtocol/OpenVPNSession.swift +++ b/Sources/TunnelKitOpenVPNProtocol/OpenVPNSession.swift @@ -72,6 +72,14 @@ public class OpenVPNSession: Session { case reconnect } + private struct Caches { + static let ca = "ca.pem" + + static let clientCertificate = "cert.pem" + + static let clientKey = "key.pem" + } + // MARK: Configuration /// The session base configuration. @@ -166,6 +174,22 @@ public class OpenVPNSession: Session { private var authenticator: OpenVPN.Authenticator? + // MARK: Caching + + private let cachesURL: URL + + private var caURL: URL { + return cachesURL.appendingPathComponent(Caches.ca) + } + + private var clientCertificateURL: URL { + return cachesURL.appendingPathComponent(Caches.clientCertificate) + } + + private var clientKeyURL: URL { + return cachesURL.appendingPathComponent(Caches.clientKey) + } + // MARK: Init /** @@ -174,13 +198,14 @@ public class OpenVPNSession: Session { - Parameter queue: The `DispatchQueue` where to run the session loop. - Parameter configuration: The `Configuration` to use for this session. */ - public init(queue: DispatchQueue, configuration: OpenVPN.Configuration) throws { - guard let _ = configuration.ca else { + public init(queue: DispatchQueue, configuration: OpenVPN.Configuration, cachesURL: URL) throws { + guard let ca = configuration.ca else { throw ConfigurationError.missingConfiguration(option: "ca") } self.queue = queue self.configuration = configuration + self.cachesURL = cachesURL withLocalOptions = true keys = [:] @@ -201,10 +226,25 @@ public class OpenVPNSession: Session { } else { controlChannel = OpenVPN.ControlChannel() } + + // cache PEMs locally (mandatory for OpenSSL) + let fm = FileManager.default + try ca.pem.write(to: caURL, atomically: true, encoding: .ascii) + if let container = configuration.clientCertificate { + try container.pem.write(to: clientCertificateURL, atomically: true, encoding: .ascii) + } else { + try? fm.removeItem(at: clientCertificateURL) + } + if let container = configuration.clientKey { + try container.pem.write(to: clientKeyURL, atomically: true, encoding: .ascii) + } else { + try? fm.removeItem(at: clientKeyURL) + } } deinit { cleanup() + cleanupCache() } // MARK: Session @@ -315,6 +355,13 @@ public class OpenVPNSession: Session { stopError = nil } + func cleanupCache() { + let fm = FileManager.default + for url in [caURL, clientCertificateURL, clientKeyURL] { + try? fm.removeItem(at: url) + } + } + // MARK: Loop // Ruby: start @@ -576,13 +623,9 @@ public class OpenVPNSession: Session { private func hardResetPayload() -> Data? { guard !(configuration.usesPIAPatches ?? false) else { - guard let ca = configuration.ca else { - log.error("Configuration doesn't have a CA") - return nil - } let caMD5: String do { - caMD5 = try TLSBox.md5(forCertificatePEM: ca.pem) + caMD5 = try TLSBox.md5(forCertificatePath: caURL.path) } catch { log.error("CA MD5 could not be computed, skipping custom HARD_RESET") return nil @@ -723,11 +766,6 @@ public class OpenVPNSession: Session { return } - guard let ca = configuration.ca else { - log.error("Configuration doesn't have a CA") - return - } - // start new TLS handshake if ((packet.code == .hardResetServerV2) && (negotiationKey.state == .hardReset)) || ((packet.code == .softResetV1) && (negotiationKey.state == .softReset)) { @@ -751,9 +789,9 @@ public class OpenVPNSession: Session { log.debug("Start TLS handshake") let tls = TLSBox( - ca: ca.pem, - clientCertificate: configuration.clientCertificate?.pem, - clientKey: configuration.clientKey?.pem, + caPath: caURL.path, + clientCertificatePath: (configuration.clientCertificate != nil) ? clientCertificateURL.path : nil, + clientKeyPath: (configuration.clientKey != nil) ? clientKeyURL.path : nil, checksEKU: configuration.checksEKU ?? false, checksSANHost: configuration.checksSANHost ?? false, hostname: configuration.sanHost @@ -1212,6 +1250,7 @@ public class OpenVPNSession: Session { switch method { case .shutdown: self?.doShutdown(error: error) + self?.cleanupCache() case .reconnect: self?.doReconnect(error: error) diff --git a/Tests/TunnelKitOpenVPNTests/EncryptionTests.swift b/Tests/TunnelKitOpenVPNTests/EncryptionTests.swift index 090e5fb..0502a4e 100644 --- a/Tests/TunnelKitOpenVPNTests/EncryptionTests.swift +++ b/Tests/TunnelKitOpenVPNTests/EncryptionTests.swift @@ -118,11 +118,6 @@ class EncryptionTests: XCTestCase { let exp = "e2fccccaba712ccc68449b1c56427ac1" print(md5) XCTAssertEqual(md5, exp) - - let pem = try! String(contentsOfFile: path, encoding: .ascii) - let md5FromPEM = try! TLSBox.md5(forCertificatePEM: pem) - print(md5FromPEM) - XCTAssertEqual(md5FromPEM, exp) } func testPrivateKeyDecryption() {