From 093774535dfd91e13470f79f8fb3b0a5aba93c6b Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Fri, 5 Oct 2018 17:35:40 +0200 Subject: [PATCH] Make CA non-optional Fix up nullability qualifiers in TLSBox. Fixes #26 --- .../TunnelKitProvider+Configuration.swift | 37 ++++++++----------- .../AppExtension/TunnelKitProvider.swift | 27 +++++++------- .../Core/SessionProxy+Configuration.swift | 10 ++--- TunnelKit/Sources/Core/TLSBox.h | 10 +++-- TunnelKit/Sources/Core/TLSBox.m | 20 ++++------ TunnelKitTests/AppExtensionTests.swift | 5 +-- 6 files changed, 50 insertions(+), 59 deletions(-) diff --git a/TunnelKit/Sources/AppExtension/TunnelKitProvider+Configuration.swift b/TunnelKit/Sources/AppExtension/TunnelKitProvider+Configuration.swift index 0f43f78..7437c40 100644 --- 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 index 69102ae..2fa9076 100644 --- 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 index 07d0c1e..9e82558 100644 --- 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 index 7895d93..9daf5f1 100644 --- a/TunnelKit/Sources/Core/TLSBox.h +++ b/TunnelKit/Sources/Core/TLSBox.h @@ -37,6 +37,8 @@ #import +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 index 078cc7a..d867a4e 100644 --- a/TunnelKit/Sources/Core/TLSBox.m +++ b/TunnelKit/Sources/Core/TLSBox.m @@ -77,7 +77,8 @@ int TLSBoxVerifyPeer(int ok, X509_STORE_CTX *ctx) { - (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 @@ int TLSBoxVerifyPeer(int ok, X509_STORE_CTX *ctx) { 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 index d02bfed..49445d1 100644 --- 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)