Verify CA from on-disk file (#237)

* Verify CA from on-disk file

Revert part of #213 again, because `SSL_CTX_load_verify_locations`
is just more reliable at setting up the trust store.

It looks like it's able to reference the .pem multiple times in
those cases where the root issuer of the CA is also embedded in
the file (which is the case with e.g. Let's Encrypt).

This is better than the current implementation, and I couldn't
easily find a way to do the same in-memory. I'd rather use the
standard API here.

See 7a85d3cac7
This commit is contained in:
Davide De Rosa 2021-11-27 12:32:30 +01:00 committed by GitHub
parent 9f46054f04
commit 9c63b856cb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 48 additions and 76 deletions

View File

@ -62,7 +62,7 @@ const NSInteger TLSBoxDefaultSecurityLevel = 0;
@interface TLSBox () @interface TLSBox ()
@property (nonatomic, strong) NSString *caPEM; @property (nonatomic, strong) NSString *caPath;
@property (nonatomic, strong) NSString *clientCertificatePEM; @property (nonatomic, strong) NSString *clientCertificatePEM;
@property (nonatomic, strong) NSString *clientKeyPEM; @property (nonatomic, strong) NSString *clientKeyPEM;
@property (nonatomic, assign) BOOL checksEKU; @property (nonatomic, assign) BOOL checksEKU;
@ -113,33 +113,6 @@ static BIO *create_BIO_from_PEM(NSString *pem) {
return hex; 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 + (NSString *)decryptedPrivateKeyFromPath:(NSString *)path passphrase:(NSString *)passphrase error:(NSError * _Nullable __autoreleasing *)error
{ {
BIO *bio; BIO *bio;
@ -200,15 +173,15 @@ static BIO *create_BIO_from_PEM(NSString *pem) {
return nil; return nil;
} }
- (instancetype)initWithCA:(nonnull NSString *)caPEM - (instancetype)initWithCAPath:(nonnull NSString *)caPath
clientCertificate:(nullable NSString *)clientCertificatePEM clientCertificate:(nullable NSString *)clientCertificatePEM
clientKey:(nullable NSString *)clientKeyPEM 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])) { if ((self = [super init])) {
self.caPEM = caPEM; self.caPath = caPath;
self.clientCertificatePEM = clientCertificatePEM; self.clientCertificatePEM = clientCertificatePEM;
self.clientKeyPEM = clientKeyPEM; self.clientKeyPEM = clientKeyPEM;
self.checksEKU = checksEKU; self.checksEKU = checksEKU;
@ -245,34 +218,14 @@ static BIO *create_BIO_from_PEM(NSString *pem) {
SSL_CTX_set_security_level(self.ctx, (int)self.securityLevel); SSL_CTX_set_security_level(self.ctx, (int)self.securityLevel);
} }
if (self.caPEM) { if (self.caPath) {
BIO *bio = create_BIO_from_PEM(self.caPEM); if (!SSL_CTX_load_verify_locations(self.ctx, [self.caPath cStringUsingEncoding:NSASCIIStringEncoding], NULL)) {
if (!bio) {
if (error) {
*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); ERR_print_errors_fp(stdout);
if (error) { if (error) {
*error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSCAUse); *error = OpenVPNErrorWithCode(OpenVPNErrorCodeTLSCAUse);
} }
X509_free(ca);
return NO; return NO;
} }
X509_free(ca);
} }
if (self.clientCertificatePEM) { if (self.clientCertificatePEM) {

View File

@ -55,16 +55,15 @@ extern const NSInteger TLSBoxDefaultSecurityLevel;
@property (nonatomic, assign) NSInteger securityLevel; // TLSBoxDefaultSecurityLevel for default @property (nonatomic, assign) NSInteger securityLevel; // TLSBoxDefaultSecurityLevel for default
+ (nullable NSString *)md5ForCertificatePath:(NSString *)path error:(NSError **)error; + (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 *)decryptedPrivateKeyFromPath:(NSString *)path passphrase:(NSString *)passphrase error:(NSError **)error;
+ (nullable NSString *)decryptedPrivateKeyFromPEM:(NSString *)pem passphrase:(NSString *)passphrase error:(NSError **)error; + (nullable NSString *)decryptedPrivateKeyFromPEM:(NSString *)pem passphrase:(NSString *)passphrase error:(NSError **)error;
- (instancetype)initWithCA:(nonnull NSString *)caPEM - (instancetype)initWithCAPath:(nonnull NSString *)caPath
clientCertificate:(nullable NSString *)clientCertificatePEM clientCertificate:(nullable NSString *)clientCertificatePEM
clientKey:(nullable NSString *)clientKeyPEM 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; - (BOOL)startWithError:(NSError **)error;

View File

@ -241,7 +241,7 @@ open class OpenVPNTunnelProvider: NEPacketTunnelProvider {
let session: OpenVPNSession let session: OpenVPNSession
do { do {
session = try OpenVPNSession(queue: tunnelQueue, configuration: cfg.sessionConfiguration) session = try OpenVPNSession(queue: tunnelQueue, configuration: cfg.sessionConfiguration, cachesURL: cachesURL)
refreshDataCount() refreshDataCount()
} catch let e { } catch let e {
completionHandler(e) completionHandler(e)

View File

@ -72,6 +72,10 @@ public class OpenVPNSession: Session {
case reconnect case reconnect
} }
private struct Caches {
static let ca = "ca.pem"
}
// MARK: Configuration // MARK: Configuration
/// The session base configuration. /// The session base configuration.
@ -166,6 +170,14 @@ public class OpenVPNSession: Session {
private var authenticator: OpenVPN.Authenticator? private var authenticator: OpenVPN.Authenticator?
// MARK: Caching
private let cachesURL: URL
private var caURL: URL {
return cachesURL.appendingPathComponent(Caches.ca)
}
// MARK: Init // MARK: Init
/** /**
@ -174,13 +186,14 @@ public class OpenVPNSession: Session {
- Parameter queue: The `DispatchQueue` where to run the session loop. - Parameter queue: The `DispatchQueue` where to run the session loop.
- Parameter configuration: The `Configuration` to use for this session. - Parameter configuration: The `Configuration` to use for this session.
*/ */
public init(queue: DispatchQueue, configuration: OpenVPN.Configuration) throws { public init(queue: DispatchQueue, configuration: OpenVPN.Configuration, cachesURL: URL) throws {
guard let _ = configuration.ca else { guard let ca = configuration.ca else {
throw ConfigurationError.missingConfiguration(option: "ca") throw ConfigurationError.missingConfiguration(option: "ca")
} }
self.queue = queue self.queue = queue
self.configuration = configuration self.configuration = configuration
self.cachesURL = cachesURL
withLocalOptions = true withLocalOptions = true
keys = [:] keys = [:]
@ -201,10 +214,14 @@ public class OpenVPNSession: Session {
} else { } else {
controlChannel = OpenVPN.ControlChannel() controlChannel = OpenVPN.ControlChannel()
} }
// cache CA locally (mandatory for OpenSSL)
try ca.pem.write(to: caURL, atomically: true, encoding: .ascii)
} }
deinit { deinit {
cleanup() cleanup()
cleanupCache()
} }
// MARK: Session // MARK: Session
@ -315,6 +332,13 @@ public class OpenVPNSession: Session {
stopError = nil stopError = nil
} }
func cleanupCache() {
let fm = FileManager.default
for url in [caURL] {
try? fm.removeItem(at: url)
}
}
// MARK: Loop // MARK: Loop
// Ruby: start // Ruby: start
@ -576,13 +600,13 @@ public class OpenVPNSession: Session {
private func hardResetPayload() -> Data? { private func hardResetPayload() -> Data? {
guard !(configuration.usesPIAPatches ?? false) else { guard !(configuration.usesPIAPatches ?? false) else {
guard let ca = configuration.ca else { guard let _ = configuration.ca else {
log.error("Configuration doesn't have a CA") log.error("Configuration doesn't have a CA")
return nil return nil
} }
let caMD5: String let caMD5: String
do { do {
caMD5 = try TLSBox.md5(forCertificatePEM: ca.pem) caMD5 = try TLSBox.md5(forCertificatePath: caURL.path)
} catch { } catch {
log.error("CA MD5 could not be computed, skipping custom HARD_RESET") log.error("CA MD5 could not be computed, skipping custom HARD_RESET")
return nil return nil
@ -723,7 +747,7 @@ public class OpenVPNSession: Session {
return return
} }
guard let ca = configuration.ca else { guard let _ = configuration.ca else {
log.error("Configuration doesn't have a CA") log.error("Configuration doesn't have a CA")
return return
} }
@ -751,7 +775,7 @@ public class OpenVPNSession: Session {
log.debug("Start TLS handshake") log.debug("Start TLS handshake")
let tls = TLSBox( let tls = TLSBox(
ca: ca.pem, caPath: caURL.path,
clientCertificate: configuration.clientCertificate?.pem, clientCertificate: configuration.clientCertificate?.pem,
clientKey: configuration.clientKey?.pem, clientKey: configuration.clientKey?.pem,
checksEKU: configuration.checksEKU ?? false, checksEKU: configuration.checksEKU ?? false,
@ -1215,6 +1239,7 @@ public class OpenVPNSession: Session {
switch method { switch method {
case .shutdown: case .shutdown:
self?.doShutdown(error: error) self?.doShutdown(error: error)
self?.cleanupCache()
case .reconnect: case .reconnect:
self?.doReconnect(error: error) self?.doReconnect(error: error)

View File

@ -118,11 +118,6 @@ class EncryptionTests: XCTestCase {
let exp = "e2fccccaba712ccc68449b1c56427ac1" let exp = "e2fccccaba712ccc68449b1c56427ac1"
print(md5) print(md5)
XCTAssertEqual(md5, exp) 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() { func testPrivateKeyDecryption() {