Make CA non-optional

Fix up nullability qualifiers in TLSBox. Fixes #26
passepartoutvpn · Oct 6, 2018 · 0937745 · 0937745
1 parent d0a46fe
commit 0937745
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 59 deletions.
diff --git a/TunnelKit/Sources/AppExtension/TunnelKitProvider+Configuration.swift b/TunnelKit/Sources/AppExtension/TunnelKitProvider+Configuration.swift
@@ -164,8 +164,8 @@ extension TunnelKitProvider {
         /// The message digest algorithm.
         public var digest: SessionProxy.Digest
 
-        /// The optional CA certificate to validate server against. Set to `nil` to disable CA validation (default).
-        public var ca: CryptoContainer?
+        /// The CA certificate to validate server against.
+        public let ca: CryptoContainer
 
         /// The optional client certificate to authenticate with. Set to `nil` to disable client authentication (default).
         public var clientCertificate: CryptoContainer?
@@ -200,14 +200,16 @@ extension TunnelKitProvider {
 
         /**
          Default initializer.
+         
+         - Parameter ca: The CA certificate.
          */
-        public init() {
+        public init(ca: CryptoContainer) {
             prefersResolvedAddresses = false
             resolvedAddresses = nil
             endpointProtocols = [EndpointProtocol(.udp, 1194)]
             cipher = .aes128cbc
             digest = .sha1
-            ca = nil
+            self.ca = ca
             clientCertificate = nil
             clientKey = nil
             mtu = 1500
@@ -229,20 +231,19 @@ extension TunnelKitProvider {
                 throw ProviderError.configuration(field: "protocolConfiguration.providerConfiguration[\(S.digestAlgorithm)]")
             }
 
-            let ca: CryptoContainer?
+            let ca: CryptoContainer
             let clientCertificate: CryptoContainer?
             let clientKey: CryptoContainer?
-            if let pem = providerConfiguration[S.ca] as? String {
-                ca = CryptoContainer(pem: pem)
-            } else {
-                ca = nil
+            guard let caPEM = providerConfiguration[S.ca] as? String else {
+                throw ProviderError.configuration(field: "protocolConfiguration.providerConfiguration[\(S.ca)]")
             }
-            if let pem = providerConfiguration[S.clientCertificate] as? String {
+            ca = CryptoContainer(pem: caPEM)
+            if let clientPEM = providerConfiguration[S.clientCertificate] as? String {
                 guard let keyPEM = providerConfiguration[S.clientKey] as? String else {
                     throw ProviderError.configuration(field: "protocolConfiguration.providerConfiguration[\(S.clientKey)]")
                 }
 
-                clientCertificate = CryptoContainer(pem: pem)
+                clientCertificate = CryptoContainer(pem: clientPEM)
                 clientKey = CryptoContainer(pem: keyPEM)
             } else {
                 clientCertificate = nil
@@ -365,7 +366,7 @@ extension TunnelKitProvider {
         public let digest: SessionProxy.Digest
 
         /// - Seealso: `TunnelKitProvider.ConfigurationBuilder.ca`
-        public let ca: CryptoContainer?
+        public let ca: CryptoContainer
 
         /// - Seealso: `TunnelKitProvider.ConfigurationBuilder.clientCertificate`
         public let clientCertificate: CryptoContainer?
@@ -446,12 +447,10 @@ extension TunnelKitProvider {
                 S.endpointProtocols: endpointProtocols.map { $0.serialized() },
                 S.cipherAlgorithm: cipher.rawValue,
                 S.digestAlgorithm: digest.rawValue,
+                S.ca: ca.pem,
                 S.mtu: mtu,
                 S.debug: shouldDebug
             ]
-            if let ca = ca {
-                dict[S.ca] = ca.pem
-            }
             if let clientCertificate = clientCertificate {
                 dict[S.clientCertificate] = clientCertificate.pem
             }
@@ -514,11 +513,6 @@ extension TunnelKitProvider {
             log.info("\tProtocols: \(endpointProtocols)")
             log.info("\tCipher: \(cipher)")
             log.info("\tDigest: \(digest)")
-            if let _ = ca {
-                log.info("\tCA verification: enabled")
-            } else {
-                log.info("\tCA verification: disabled")
-            }
             if let _ = clientCertificate {
                 log.info("\tClient verification: enabled")
             } else {
@@ -551,11 +545,10 @@ extension TunnelKitProvider.Configuration: Equatable {
      - Returns: An editable `TunnelKitProvider.ConfigurationBuilder` initialized with this configuration.
      */
     public func builder() -> TunnelKitProvider.ConfigurationBuilder {
-        var builder = TunnelKitProvider.ConfigurationBuilder()
+        var builder = TunnelKitProvider.ConfigurationBuilder(ca: ca)
         builder.endpointProtocols = endpointProtocols
         builder.cipher = cipher
         builder.digest = digest
-        builder.ca = ca
         builder.clientCertificate = clientCertificate
         builder.clientKey = clientKey
         builder.mtu = mtu

diff --git a/TunnelKit/Sources/AppExtension/TunnelKitProvider.swift b/TunnelKit/Sources/AppExtension/TunnelKitProvider.swift
@@ -171,20 +171,16 @@ open class TunnelKitProvider: NEPacketTunnelProvider {
             return
         }
 
-        let caPath: String?
+        let caPath: String
         let clientCertificatePath: String?
         let clientKeyPath: String?
-        if let ca = cfg.ca {
-            do {
-                let url = temporaryURL(forKey: Configuration.Keys.ca)
-                try ca.write(to: url)
-                caPath = url.path
-            } catch {
-                completionHandler(ProviderError.certificateSerialization)
-                return
-            }
-        } else {
-            caPath = nil
+        do {
+            let url = temporaryURL(forKey: Configuration.Keys.ca)
+            try cfg.ca.write(to: url)
+            caPath = url.path
+        } catch {
+            completionHandler(ProviderError.certificateSerialization)
+            return
         }
         if let clientCertificate = cfg.clientCertificate {
             do {
@@ -214,10 +210,13 @@ open class TunnelKitProvider: NEPacketTunnelProvider {
         cfg.print(appVersion: appVersion)
 
 //        log.info("Temporary CA is stored to: \(caPath)")
-        var sessionConfiguration = SessionProxy.ConfigurationBuilder(username: endpoint.username, password: endpoint.password)
+        var sessionConfiguration = SessionProxy.ConfigurationBuilder(
+            username: endpoint.username,
+            password: endpoint.password,
+            caPath: caPath
+        )
         sessionConfiguration.cipher = cfg.cipher
         sessionConfiguration.digest = cfg.digest
-        sessionConfiguration.caPath = caPath
         sessionConfiguration.clientCertificatePath = clientCertificatePath
         sessionConfiguration.clientKeyPath = clientKeyPath
         sessionConfiguration.compressionFraming = cfg.compressionFraming

diff --git a/TunnelKit/Sources/Core/SessionProxy+Configuration.swift b/TunnelKit/Sources/Core/SessionProxy+Configuration.swift
@@ -124,8 +124,8 @@ extension SessionProxy {
         /// The digest algorithm for HMAC.
         public var digest: Digest
 
-        /// The path to the optional CA for TLS negotiation (PEM format).
-        public var caPath: String?
+        /// The path to the CA for TLS negotiation (PEM format).
+        public let caPath: String
 
         /// The path to the optional client certificate for TLS negotiation (PEM format).
         public var clientCertificatePath: String?
@@ -143,12 +143,12 @@ extension SessionProxy {
         public var renegotiatesAfter: TimeInterval?
 
         /// :nodoc:
-        public init(username: String, password: String) {
+        public init(username: String, password: String, caPath: String) {
             self.username = username
             self.password = password
             cipher = .aes128cbc
             digest = .sha1
-            caPath = nil
+            self.caPath = caPath
             clientCertificatePath = nil
             clientKeyPath = nil
             compressionFraming = .disabled
@@ -193,7 +193,7 @@ extension SessionProxy {
         public let digest: Digest
 
         /// - Seealso: `SessionProxy.ConfigurationBuilder.caPath`
-        public let caPath: String?
+        public let caPath: String
 
         /// - Seealso: `SessionProxy.ConfigurationBuilder.clientCertificatePath`
         public let clientCertificatePath: String?

diff --git a/TunnelKit/Sources/Core/TLSBox.h b/TunnelKit/Sources/Core/TLSBox.h
@@ -37,6 +37,8 @@
 
 #import <Foundation/Foundation.h>
 
+NS_ASSUME_NONNULL_BEGIN
+
 extern const NSInteger TLSBoxMaxBufferLength;
 
 extern NSString *const TLSBoxPeerVerificationErrorNotification;
@@ -50,12 +52,12 @@ extern NSString *const TLSBoxPeerVerificationErrorNotification;
 @interface TLSBox : NSObject
 
 - (instancetype)initWithCAPath:(NSString *)caPath
-                 clientCertificatePath:(NSString *)clientCertificatePath
-                         clientKeyPath:(NSString *)clientKeyPath;
+         clientCertificatePath:(nullable NSString *)clientCertificatePath
+                 clientKeyPath:(nullable NSString *)clientKeyPath;
 
 - (BOOL)startWithError:(NSError **)error;
 
-- (NSData *)pullCipherTextWithError:(NSError **)error;
+- (nullable NSData *)pullCipherTextWithError:(NSError **)error;
 // WARNING: text must be able to hold plain text output
 - (BOOL)pullRawPlainText:(uint8_t *)text length:(NSInteger *)length error:(NSError **)error;
 
@@ -67,3 +69,5 @@ extern NSString *const TLSBoxPeerVerificationErrorNotification;
 - (BOOL)isConnected;
 
 @end
+
+NS_ASSUME_NONNULL_END
diff --git a/TunnelKit/Sources/Core/TLSBox.m b/TunnelKit/Sources/Core/TLSBox.m
@@ -77,7 +77,8 @@ @implementation TLSBox
 
 - (instancetype)init
 {
-    return [self initWithCAPath:nil clientCertificatePath:nil clientKeyPath:nil];
+    [NSException raise:NSInvalidArgumentException format:@"Use initWithCAPath:clientCertificatePath:clientKeyPath:"];
+    return nil;
 }
 
 - (instancetype)initWithCAPath:(NSString *)caPath clientCertificatePath:(NSString *)clientCertificatePath clientKeyPath:(NSString *)clientKeyPath
@@ -115,18 +116,13 @@ - (BOOL)startWithError:(NSError *__autoreleasing *)error
 
     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);
-    if (self.caPath) {
-        SSL_CTX_set_verify(self.ctx, SSL_VERIFY_PEER, TLSBoxVerifyPeer);
-        if (!SSL_CTX_load_verify_locations(self.ctx, [self.caPath cStringUsingEncoding:NSASCIIStringEncoding], NULL)) {
-            ERR_print_errors_fp(stdout);
-            if (error) {
-                *error = TunnelKitErrorWithCode(TunnelKitErrorCodeTLSBoxCA);
-            }
-            return NO;
+    SSL_CTX_set_verify(self.ctx, SSL_VERIFY_PEER, TLSBoxVerifyPeer);
+    if (!SSL_CTX_load_verify_locations(self.ctx, [self.caPath cStringUsingEncoding:NSASCIIStringEncoding], NULL)) {
+        ERR_print_errors_fp(stdout);
+        if (error) {
+            *error = TunnelKitErrorWithCode(TunnelKitErrorCodeTLSBoxCA);
         }
-    }
-    else {
-        SSL_CTX_set_verify(self.ctx, SSL_VERIFY_NONE, NULL);
+        return NO;
     }
 
     if (self.clientCertificatePath) {

diff --git a/TunnelKitTests/AppExtensionTests.swift b/TunnelKitTests/AppExtensionTests.swift
@@ -63,12 +63,11 @@ class AppExtensionTests: XCTestCase {
             password: "bar"
         )
 
-        builder = TunnelKitProvider.ConfigurationBuilder()
+        builder = TunnelKitProvider.ConfigurationBuilder(ca: CryptoContainer(pem: "abcdef"))
         XCTAssertNotNil(builder)
 
         builder.cipher = .aes128cbc
         builder.digest = .sha256
-        builder.ca = CryptoContainer(pem: "abcdef")
         cfg = builder.build()
 
         let proto = try? cfg.generatedTunnelProtocol(withBundleIdentifier: identifier, appGroup: appGroup, endpoint: endpoint)
@@ -87,7 +86,7 @@ class AppExtensionTests: XCTestCase {
         XCTAssertEqual(proto?.providerConfiguration?[K.appGroup] as? String, appGroup)
         XCTAssertEqual(proto?.providerConfiguration?[K.cipherAlgorithm] as? String, cfg.cipher.rawValue)
         XCTAssertEqual(proto?.providerConfiguration?[K.digestAlgorithm] as? String, cfg.digest.rawValue)
-        XCTAssertEqual(proto?.providerConfiguration?[K.ca] as? String, cfg.ca?.pem)
+        XCTAssertEqual(proto?.providerConfiguration?[K.ca] as? String, cfg.ca.pem)
         XCTAssertEqual(proto?.providerConfiguration?[K.mtu] as? Int, cfg.mtu)
         XCTAssertEqual(proto?.providerConfiguration?[K.renegotiatesAfter] as? Int, cfg.renegotiatesAfterSeconds)
         XCTAssertEqual(proto?.providerConfiguration?[K.debug] as? Bool, cfg.shouldDebug)