Avoid caching PEMs on disk (#213)

* TLSBox: Use OpenSSL calls that take in-memory cert / private key

* TLSBox: Add ability to compute MD5 hash for cert in memory

* OpenVPNSession: Remove disk caching of ca, cert and key

* Add test for computing MD5 hash for cert in memory

Co-authored-by: Davide De Rosa <keeshux@gmail.com>

CHANGELOG.md

@@ -9,6 +9,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.
 
 ## 3.4.0 (2021-08-07)

TunnelKit/Sources/Protocols/OpenVPN/AppExtension/OpenVPNTunnelProvider.swift

@@ -238,7 +238,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)

TunnelKit/Sources/Protocols/OpenVPN/OpenVPNSession.swift

@@ -70,14 +70,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.
@@ -172,22 +164,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
 
     /**
@@ -196,14 +172,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 {
+    public init(queue: DispatchQueue, configuration: OpenVPN.Configuration) throws {
-        guard let ca = configuration.ca else {
+        guard let _ = configuration.ca else {
             throw ConfigurationError.missingConfiguration(option: "ca")
         }
         
         self.queue = queue
         self.configuration = configuration
-        self.cachesURL = cachesURL
 
         withLocalOptions = true
         keys = [:]
@@ -224,26 +199,11 @@ 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)
-        }
     }
     
     /// :nodoc:
     deinit {
         cleanup()
-        cleanupCache()
     }
     
     // MARK: Session
@@ -354,13 +314,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
@@ -622,9 +575,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
@@ -765,6 +722,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)) {
@@ -788,9 +750,9 @@ public class OpenVPNSession: Session {
             log.debug("Start TLS handshake")
 
             let tls = TLSBox(
-                caPath: caURL.path,
+                ca: ca.pem,
-                clientCertificatePath: (configuration.clientCertificate != nil) ? clientCertificateURL.path : nil,
+                clientCertificate: configuration.clientCertificate?.pem,
-                clientKeyPath: (configuration.clientKey != nil) ? clientKeyURL.path : nil,
+                clientKey: configuration.clientKey?.pem,
                 checksEKU: configuration.checksEKU ?? false,
                 checksSANHost: configuration.checksSANHost ?? false,
                 hostname: configuration.sanHost
@@ -1252,7 +1214,6 @@ public class OpenVPNSession: Session {
             switch method {
             case .shutdown:
                 self?.doShutdown(error: error)
-                self?.cleanupCache()
                 
             case .reconnect:
                 self?.doReconnect(error: error)

TunnelKit/Sources/Protocols/OpenVPN/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
+- (instancetype)initWithCA:(nonnull NSString *)caPEM
-         clientCertificatePath:(nullable NSString *)clientCertificatePath
+         clientCertificate:(nullable NSString *)clientCertificatePEM
-                 clientKeyPath:(nullable NSString *)clientKeyPath
+                 clientKey:(nullable NSString *)clientKeyPEM
-                     checksEKU:(BOOL)checksEKU
+                 checksEKU:(BOOL)checksEKU
-                 checksSANHost:(BOOL)checksSANHost
+             checksSANHost:(BOOL)checksSANHost
-                      hostname:(nullable NSString *)hostname;
+                  hostname:(nullable NSString *)hostname;
 
 - (BOOL)startWithError:(NSError **)error;
 

TunnelKit/Sources/Protocols/OpenVPN/TLSBox.m

@@ -65,9 +65,9 @@ const NSInteger TLSBoxDefaultSecurityLevel = -1;
 
 @interface TLSBox ()
 
-@property (nonatomic, strong) NSString *caPath;
+@property (nonatomic, strong) NSString *caPEM;
-@property (nonatomic, strong) NSString *clientCertificatePath;
+@property (nonatomic, strong) NSString *clientCertificatePEM;
-@property (nonatomic, strong) NSString *clientKeyPath;
+@property (nonatomic, strong) NSString *clientKeyPEM;
 @property (nonatomic, assign) BOOL checksEKU;
 @property (nonatomic, assign) BOOL checksSANHost;
 @property (nonatomic, strong) NSString *hostname;
@@ -83,6 +83,10 @@ const NSInteger TLSBoxDefaultSecurityLevel = -1;
 
 @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
@@ -112,6 +116,31 @@ const NSInteger TLSBoxDefaultSecurityLevel = -1;
     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;
@@ -172,17 +201,17 @@ const NSInteger TLSBoxDefaultSecurityLevel = -1;
     return nil;
 }
 
-- (instancetype)initWithCAPath:(NSString *)caPath
+- (instancetype)initWithCA:(nonnull NSString *)caPEM
-         clientCertificatePath:(NSString *)clientCertificatePath
+         clientCertificate:(nullable NSString *)clientCertificatePEM
-                 clientKeyPath:(NSString *)clientKeyPath
+                 clientKey:(nullable NSString *)clientKeyPEM
-                     checksEKU:(BOOL)checksEKU
+                 checksEKU:(BOOL)checksEKU
-                 checksSANHost:(BOOL)checksSANHost
+             checksSANHost:(BOOL)checksSANHost
-                      hostname:(nullable NSString *)hostname
+                  hostname:(nullable NSString *)hostname
 {
     if ((self = [super init])) {
-        self.caPath = caPath;
+        self.caPEM = caPEM;
-        self.clientCertificatePath = clientCertificatePath;
+        self.clientCertificatePEM = clientCertificatePEM;
-        self.clientKeyPath = clientKeyPath;
+        self.clientKeyPEM = clientKeyPEM;
         self.checksEKU = checksEKU;
         self.checksSANHost = checksSANHost;
         self.bufferCipherText = allocate_safely(TLSBoxMaxBufferLength);
@@ -216,31 +245,47 @@ const NSInteger TLSBoxDefaultSecurityLevel = -1;
     if (self.securityLevel != TLSBoxDefaultSecurityLevel) {
         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 (self.caPEM) {
-        if (error) {
+        BIO *bio = create_BIO_from_PEM(self.caPEM);
-            *error = TunnelKitErrorWithCode(TunnelKitErrorCodeTLSCertificateAuthority);
+        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;
         }
-        return NO;
     }
 
-    if (self.clientCertificatePath) {
+    if (self.clientCertificatePEM) {
-        if (!SSL_CTX_use_certificate_file(self.ctx, [self.clientCertificatePath cStringUsingEncoding:NSASCIIStringEncoding], SSL_FILETYPE_PEM)) {
+        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)) {
             ERR_print_errors_fp(stdout);
             if (error) {
                 *error = TunnelKitErrorWithCode(TunnelKitErrorCodeTLSClientCertificate);
             }
             return NO;
         }
+        X509_free(cert);
 
-        if (self.clientKeyPath) {
+        if (self.clientKeyPEM) {
-            if (!SSL_CTX_use_PrivateKey_file(self.ctx, [self.clientKeyPath cStringUsingEncoding:NSASCIIStringEncoding], SSL_FILETYPE_PEM)) {
+            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)) {
                 ERR_print_errors_fp(stdout);
                 if (error) {
                     *error = TunnelKitErrorWithCode(TunnelKitErrorCodeTLSClientKey);
                 }
                 return NO;
+
             }
+            EVP_PKEY_free(pkey);
         }
     }
 

TunnelKit/Tests/OpenVPN/EncryptionTests.swift

@@ -117,6 +117,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() {