From 7a85d3cac7b7332983bae5aaf9e2878277d2450b Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Thu, 25 Nov 2021 12:36:17 +0100 Subject: [PATCH] Restore and fix former PEM caching PR (#235) This reverts commit 995009121a4ad02fb9d8a3f2980ea9dd1693e396. * Improve error handling * Trust intermediate CA * Update CHANGELOG --- CHANGELOG.md | 5 + .../CTunnelKitOpenVPNCore/include/Errors.h | 18 ++- Sources/CTunnelKitOpenVPNProtocol/TLSBox.m | 147 ++++++++++++++---- .../include/TLSBox.h | 13 +- .../OpenVPNTunnelProvider.swift | 6 +- .../OpenVPNSession.swift | 69 ++------ .../EncryptionTests.swift | 5 + 7 files changed, 165 insertions(+), 98 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb0f18a..99d77b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Revert to OpenSSL. [#233](https://github.com/passepartoutvpn/tunnelkit/pull/233) +### Fixed + +- Restore and fix "Avoid caching PEMs on disk" (roop). [#213](https://github.com/passepartoutvpn/tunnelkit/pull/213) + ## 4.0.1 (2021-11-18) ### Fixed @@ -33,6 +37,7 @@ 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. ### Fixed diff --git a/Sources/CTunnelKitOpenVPNCore/include/Errors.h b/Sources/CTunnelKitOpenVPNCore/include/Errors.h index fb5d18f..7d59fbc 100644 --- a/Sources/CTunnelKitOpenVPNCore/include/Errors.h +++ b/Sources/CTunnelKitOpenVPNCore/include/Errors.h @@ -44,13 +44,17 @@ typedef NS_ENUM(NSInteger, OpenVPNErrorCode) { OpenVPNErrorCodeCryptoHMAC = 102, OpenVPNErrorCodeCryptoEncryption = 103, OpenVPNErrorCodeCryptoAlgorithm = 104, - OpenVPNErrorCodeTLSCertificateAuthority = 201, - OpenVPNErrorCodeTLSHandshake = 202, - OpenVPNErrorCodeTLSClientCertificate = 204, - OpenVPNErrorCodeTLSClientKey = 205, - OpenVPNErrorCodeTLSServerCertificate = 206, - OpenVPNErrorCodeTLSServerEKU = 207, - OpenVPNErrorCodeTLSServerHost = 208, + OpenVPNErrorCodeTLSCARead = 201, + OpenVPNErrorCodeTLSCAUse = 202, + OpenVPNErrorCodeTLSCAPeerVerification = 203, + OpenVPNErrorCodeTLSClientCertificateRead = 204, + OpenVPNErrorCodeTLSClientCertificateUse = 205, + OpenVPNErrorCodeTLSClientKeyRead = 206, + OpenVPNErrorCodeTLSClientKeyUse = 207, + OpenVPNErrorCodeTLSHandshake = 210, + OpenVPNErrorCodeTLSServerCertificate = 211, + OpenVPNErrorCodeTLSServerEKU = 212, + OpenVPNErrorCodeTLSServerHost = 213, OpenVPNErrorCodeDataPathOverflow = 301, OpenVPNErrorCodeDataPathPeerIdMismatch = 302, OpenVPNErrorCodeDataPathCompression = 303 diff --git a/Sources/CTunnelKitOpenVPNProtocol/TLSBox.m b/Sources/CTunnelKitOpenVPNProtocol/TLSBox.m index 7673dca..f8928d2 100644 --- a/Sources/CTunnelKitOpenVPNProtocol/TLSBox.m +++ b/Sources/CTunnelKitOpenVPNProtocol/TLSBox.m @@ -50,7 +50,7 @@ static const char *const TLSBoxServerEKU = "TLS Web Server Authentication"; int TLSBoxVerifyPeer(int ok, X509_STORE_CTX *ctx) { if (!ok) { - NSError *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSCertificateAuthority); + NSError *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSCAPeerVerification); [[NSNotificationCenter defaultCenter] postNotificationName:TLSBoxPeerVerificationErrorNotification object:nil userInfo:@{OpenVPNErrorKey: error}]; @@ -62,9 +62,9 @@ const NSInteger TLSBoxDefaultSecurityLevel = 0; @interface TLSBox () -@property (nonatomic, strong) NSString *caPath; -@property (nonatomic, strong) NSString *clientCertificatePath; -@property (nonatomic, strong) NSString *clientKeyPath; +@property (nonatomic, strong) NSString *caPEM; +@property (nonatomic, strong) NSString *clientCertificatePEM; +@property (nonatomic, strong) NSString *clientKeyPEM; @property (nonatomic, assign) BOOL checksEKU; @property (nonatomic, assign) BOOL checksSANHost; @property (nonatomic, strong) NSString *hostname; @@ -80,6 +80,10 @@ const NSInteger TLSBoxDefaultSecurityLevel = 0; @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,6 +113,33 @@ const NSInteger TLSBoxDefaultSecurityLevel = 0; 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); + if (!bio) { + return NULL; + } + 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; @@ -169,17 +200,17 @@ const NSInteger TLSBoxDefaultSecurityLevel = 0; return nil; } -- (instancetype)initWithCAPath:(NSString *)caPath - clientCertificatePath:(NSString *)clientCertificatePath - clientKeyPath:(NSString *)clientKeyPath - checksEKU:(BOOL)checksEKU - checksSANHost:(BOOL)checksSANHost - hostname:(nullable NSString *)hostname +- (instancetype)initWithCA:(nonnull NSString *)caPEM + clientCertificate:(nullable NSString *)clientCertificatePEM + clientKey:(nullable NSString *)clientKeyPEM + checksEKU:(BOOL)checksEKU + checksSANHost:(BOOL)checksSANHost + hostname:(nullable NSString *)hostname { if ((self = [super init])) { - self.caPath = caPath; - self.clientCertificatePath = clientCertificatePath; - self.clientKeyPath = clientKeyPath; + self.caPEM = caPEM; + self.clientCertificatePEM = clientCertificatePEM; + self.clientKeyPEM = clientKeyPEM; self.checksEKU = checksEKU; self.checksSANHost = checksSANHost; self.bufferCipherText = allocate_safely(TLSBoxMaxBufferLength); @@ -214,31 +245,89 @@ const NSInteger TLSBoxDefaultSecurityLevel = 0; SSL_CTX_set_security_level(self.ctx, (int)self.securityLevel); } - if (!SSL_CTX_load_verify_locations(self.ctx, [self.caPath cStringUsingEncoding:NSASCIIStringEncoding], NULL)) { - ERR_print_errors_fp(stdout); - if (error) { - *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSCertificateAuthority); - } - return NO; - } - - if (self.clientCertificatePath) { - if (!SSL_CTX_use_certificate_file(self.ctx, [self.clientCertificatePath cStringUsingEncoding:NSASCIIStringEncoding], SSL_FILETYPE_PEM)) { - ERR_print_errors_fp(stdout); + if (self.caPEM) { + BIO *bio = create_BIO_from_PEM(self.caPEM); + if (!bio) { if (error) { - *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSClientCertificate); + *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSCARead); } return NO; } + X509 *ca = PEM_read_bio_X509_AUX(bio, NULL, NULL, NULL); + if (!ca) { + if (error) { + *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSCARead); + } + BIO_free(bio); + return NO; + } + BIO_free(bio); + X509_STORE *trustedStore = SSL_CTX_get_cert_store(self.ctx); + X509_STORE_set_flags(trustedStore, X509_V_FLAG_PARTIAL_CHAIN); + if (!X509_STORE_add_cert(trustedStore, ca)) { + ERR_print_errors_fp(stdout); + if (error) { + *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSCAUse); + } + X509_free(ca); + return NO; + } + X509_free(ca); + } - if (self.clientKeyPath) { - if (!SSL_CTX_use_PrivateKey_file(self.ctx, [self.clientKeyPath cStringUsingEncoding:NSASCIIStringEncoding], SSL_FILETYPE_PEM)) { - ERR_print_errors_fp(stdout); + if (self.clientCertificatePEM) { + BIO *bio = create_BIO_from_PEM(self.clientCertificatePEM); + if (!bio) { + if (error) { + *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSClientCertificateRead); + } + return NO; + } + X509 *cert = PEM_read_bio_X509(bio, NULL, NULL, NULL); + if (!cert) { + if (error) { + *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSClientCertificateRead); + } + BIO_free(bio); + return NO; + } + BIO_free(bio); + if (!SSL_CTX_use_certificate(self.ctx, cert)) { + ERR_print_errors_fp(stdout); + if (error) { + *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSClientCertificateUse); + } + X509_free(cert); + return NO; + } + X509_free(cert); + + if (self.clientKeyPEM) { + BIO *bio = create_BIO_from_PEM(self.clientKeyPEM); + if (!bio) { if (error) { - *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSClientKey); + *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSClientKeyRead); } return NO; } + EVP_PKEY *pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL); + if (!pkey) { + if (error) { + *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSClientKeyRead); + } + EVP_PKEY_free(pkey); + return NO; + } + BIO_free(bio); + if (!SSL_CTX_use_PrivateKey(self.ctx, pkey)) { + ERR_print_errors_fp(stdout); + if (error) { + *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSClientKeyUse); + } + EVP_PKEY_free(pkey); + return NO; + } + EVP_PKEY_free(pkey); } } diff --git a/Sources/CTunnelKitOpenVPNProtocol/include/TLSBox.h b/Sources/CTunnelKitOpenVPNProtocol/include/TLSBox.h index 91ebd25..3d9e54d 100644 --- a/Sources/CTunnelKitOpenVPNProtocol/include/TLSBox.h +++ b/Sources/CTunnelKitOpenVPNProtocol/include/TLSBox.h @@ -55,15 +55,16 @@ extern const NSInteger TLSBoxDefaultSecurityLevel; @property (nonatomic, assign) NSInteger securityLevel; // TLSBoxDefaultSecurityLevel for default + (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)initWithCAPath:(NSString *)caPath - clientCertificatePath:(nullable NSString *)clientCertificatePath - clientKeyPath:(nullable NSString *)clientKeyPath - checksEKU:(BOOL)checksEKU - checksSANHost:(BOOL)checksSANHost - hostname:(nullable NSString *)hostname; +- (instancetype)initWithCA:(nonnull NSString *)caPEM + clientCertificate:(nullable NSString *)clientCertificatePEM + clientKey:(nullable NSString *)clientKeyPEM + 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 ec7f395..9b96227 100644 --- a/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift +++ b/Sources/TunnelKitOpenVPNAppExtension/OpenVPNTunnelProvider.swift @@ -241,7 +241,7 @@ open class OpenVPNTunnelProvider: NEPacketTunnelProvider { let session: OpenVPNSession do { - session = try OpenVPNSession(queue: tunnelQueue, configuration: cfg.sessionConfiguration, cachesURL: cachesURL) + session = try OpenVPNSession(queue: tunnelQueue, configuration: cfg.sessionConfiguration) refreshDataCount() } catch let e { completionHandler(e) @@ -903,7 +903,9 @@ extension OpenVPNTunnelProvider { case .cryptoEncryption, .cryptoHMAC: return .encryptionData - case .tlsCertificateAuthority, .tlsClientCertificate, .tlsClientKey: + case .tlscaRead, .tlscaUse, .tlscaPeerVerification, + .tlsClientCertificateRead, .tlsClientCertificateUse, + .tlsClientKeyRead, .tlsClientKeyUse: return .tlsInitialization case .tlsServerCertificate, .tlsServerEKU, .tlsServerHost: diff --git a/Sources/TunnelKitOpenVPNProtocol/OpenVPNSession.swift b/Sources/TunnelKitOpenVPNProtocol/OpenVPNSession.swift index 99ecb40..35bade5 100644 --- a/Sources/TunnelKitOpenVPNProtocol/OpenVPNSession.swift +++ b/Sources/TunnelKitOpenVPNProtocol/OpenVPNSession.swift @@ -72,14 +72,6 @@ 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. @@ -174,22 +166,6 @@ 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 /** @@ -198,14 +174,13 @@ 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, cachesURL: URL) throws { - guard let ca = configuration.ca else { + public init(queue: DispatchQueue, configuration: OpenVPN.Configuration) throws { + guard let _ = configuration.ca else { throw ConfigurationError.missingConfiguration(option: "ca") } self.queue = queue self.configuration = configuration - self.cachesURL = cachesURL withLocalOptions = true keys = [:] @@ -226,25 +201,10 @@ 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 @@ -355,13 +315,6 @@ 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 @@ -623,9 +576,13 @@ 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(forCertificatePath: caURL.path) + caMD5 = try TLSBox.md5(forCertificatePEM: ca.pem) } catch { log.error("CA MD5 could not be computed, skipping custom HARD_RESET") return nil @@ -766,6 +723,11 @@ 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)) { @@ -789,9 +751,9 @@ public class OpenVPNSession: Session { log.debug("Start TLS handshake") let tls = TLSBox( - caPath: caURL.path, - clientCertificatePath: (configuration.clientCertificate != nil) ? clientCertificateURL.path : nil, - clientKeyPath: (configuration.clientKey != nil) ? clientKeyURL.path : nil, + ca: ca.pem, + clientCertificate: configuration.clientCertificate?.pem, + clientKey: configuration.clientKey?.pem, checksEKU: configuration.checksEKU ?? false, checksSANHost: configuration.checksSANHost ?? false, hostname: configuration.sanHost @@ -1253,7 +1215,6 @@ 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 0502a4e..090e5fb 100644 --- a/Tests/TunnelKitOpenVPNTests/EncryptionTests.swift +++ b/Tests/TunnelKitOpenVPNTests/EncryptionTests.swift @@ -118,6 +118,11 @@ 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() {