From 6574121b7f149059bb58650032bfdb168729b726 Mon Sep 17 00:00:00 2001 From: Alban Diquet Date: Mon, 8 Feb 2016 17:00:57 -0800 Subject: [PATCH] Simplify authentication handlers in delegate proxies --- .../TSKNSURLConnectionDelegateProxy.m | 33 +-------------- .../Swizzling/TSKNSURLSessionDelegateProxy.m | 41 ++----------------- 2 files changed, 6 insertions(+), 68 deletions(-) diff --git a/TrustKit/Swizzling/TSKNSURLConnectionDelegateProxy.m b/TrustKit/Swizzling/TSKNSURLConnectionDelegateProxy.m index 5d1a9f9f..07909ae5 100644 --- a/TrustKit/Swizzling/TSKNSURLConnectionDelegateProxy.m +++ b/TrustKit/Swizzling/TSKNSURLConnectionDelegateProxy.m @@ -206,37 +206,7 @@ - (void)connection:(NSURLConnection *)connection willSendRequestForAuthenticatio // Check the trust object against the pinning policy trustDecision = [TSKPinningValidator evaluateTrust:serverTrust forHostname:serverHostname]; _lastTrustDecision = trustDecision; - if (trustDecision == TSKTrustDecisionShouldAllowConnection) - { - // Success - don't do anything and forward the challenge to the original delegate - wasChallengeHandled = NO; - } - else if (trustDecision == TSKTrustDecisionDomainNotPinned) - { - if ([self forwardToOriginalDelegateAuthenticationChallenge:challenge forConnection:connection]) - { - // The original delegate handled the challenge and performed SSL validation itself - wasChallengeHandled = YES; - } - else - { - // The original delegate does not have authentication handlers for this challenge - // We need to do the default validation ourselves to avoid disabling SSL validation for all non pinned domains - TSKLog(@"Performing default certificate validation for %@", serverHostname); - SecTrustResultType trustResult = 0; - SecTrustEvaluate(serverTrust, &trustResult); - if ((trustResult != kSecTrustResultUnspecified) && (trustResult != kSecTrustResultProceed)) - { - // Default SSL validation failed - block the connection - CFDictionaryRef evaluationDetails = SecTrustCopyResult(serverTrust); - TSKLog(@"Error: default SSL validation failed: %@", evaluationDetails); - CFRelease(evaluationDetails); - wasChallengeHandled = YES; - [challenge.sender cancelAuthenticationChallenge:challenge]; - } - } - } - else + if (trustDecision == TSKTrustDecisionShouldBlockConnection) { // Pinning validation failed - block the connection wasChallengeHandled = YES; @@ -247,6 +217,7 @@ - (void)connection:(NSURLConnection *)connection willSendRequestForAuthenticatio // Forward all challenges (including client auth challenges) to the original delegate if (wasChallengeHandled == NO) { + // We will also get here if the pinning validation succeeded or the domain was not pinned if ([self forwardToOriginalDelegateAuthenticationChallenge:challenge forConnection:connection] == NO) { // The original delegate could not handle the challenge; use the default handler diff --git a/TrustKit/Swizzling/TSKNSURLSessionDelegateProxy.m b/TrustKit/Swizzling/TSKNSURLSessionDelegateProxy.m index 7f7ecb97..b60ed61d 100644 --- a/TrustKit/Swizzling/TSKNSURLSessionDelegateProxy.m +++ b/TrustKit/Swizzling/TSKNSURLSessionDelegateProxy.m @@ -184,37 +184,7 @@ - (void)URLSession:(NSURLSession * _Nonnull)session // Check the trust object against the pinning policy trustDecision = [TSKPinningValidator evaluateTrust:serverTrust forHostname:serverHostname]; _lastTrustDecision = trustDecision; - if (trustDecision == TSKTrustDecisionShouldAllowConnection) - { - // Success - don't do anything and forward the challenge to the original delegate - wasChallengeHandled = NO; - } - else if (trustDecision == TSKTrustDecisionDomainNotPinned) - { - if ([self forwardToOriginalDelegateAuthenticationChallenge:challenge completionHandler:completionHandler forSession:session]) - { - // The original delegate handled the challenge and performed SSL validation itself - wasChallengeHandled = YES; - } - else - { - // The original delegate does not have authentication handlers for this challenge - // We need to do the default validation ourselves to avoid disabling SSL validation for all non pinned domains - TSKLog(@"Performing default certificate validation for %@", serverHostname); - SecTrustResultType trustResult = 0; - SecTrustEvaluate(serverTrust, &trustResult); - if ((trustResult != kSecTrustResultUnspecified) && (trustResult != kSecTrustResultProceed)) - { - // Default SSL validation failed - block the connection - CFDictionaryRef evaluationDetails = SecTrustCopyResult(serverTrust); - TSKLog(@"Error: default SSL validation failed: %@", evaluationDetails); - CFRelease(evaluationDetails); - wasChallengeHandled = YES; - completionHandler(NSURLSessionAuthChallengeCancelAuthenticationChallenge, NULL); - } - } - } - else + if (trustDecision == TSKTrustDecisionShouldBlockConnection) { // Pinning validation failed - block the connection wasChallengeHandled = YES; @@ -225,6 +195,7 @@ - (void)URLSession:(NSURLSession * _Nonnull)session // Forward all challenges (including client auth challenges) to the original delegate if (wasChallengeHandled == NO) { + // We will also get here if the pinning validation succeeded or the domain was not pinned if ([self forwardToOriginalDelegateAuthenticationChallenge:challenge completionHandler:completionHandler forSession:session] == NO) { // The original delegate could not handle the challenge; use the default handler @@ -250,12 +221,7 @@ - (void)URLSession:(NSURLSession * _Nonnull)session trustDecision = [TSKPinningValidator evaluateTrust:challenge.protectionSpace.serverTrust forHostname:challenge.protectionSpace.host]; _lastTrustDecision = trustDecision; - if ((trustDecision == TSKTrustDecisionShouldAllowConnection) || (trustDecision == TSKTrustDecisionDomainNotPinned)) - { - // Don't do anything and forward the challenge to the original delegate - wasChallengeHandled = NO; - } - else + if (trustDecision == TSKTrustDecisionShouldBlockConnection) { // Pinning validation failed - block the connection wasChallengeHandled = YES; @@ -266,6 +232,7 @@ - (void)URLSession:(NSURLSession * _Nonnull)session // Forward all challenges (including client auth challenges) to the original delegate if (wasChallengeHandled == NO) { + // We will also get here if the pinning validation succeeded or the domain was not pinned // If we're in this delegate method (and not URLSession:didReceiveChallenge:completionHandler:) // it means the delegate definitely implements the handler method so we can call it directly [originalDelegate URLSession:session task:task didReceiveChallenge:challenge completionHandler:completionHandler];