Skip to content
This repository

Pinning certificates will now trust all derived certificates. #882

Merged
merged 3 commits into from over 1 year ago

4 participants

Oliver Letterer Mattt Thompson rob phillips Esad Hajdarevic
Oliver Letterer

With this pull request, AFNetworking will trust all pinned certificates and it's derived certificates as discussed in #877. This code is backward compatible but if a user want's to only pin and trust root certificates (like Google Chrome does), this is now possible.

Oliver Letterer OliverLetterer referenced this pull request
Merged

Fix SSL Pinning #900

Esad Hajdarevic

SecTrustCreateWithCertificates expects CFArrayRef as first argument. I'm not sure why it compiles fine on iOS, but on OS X this code results in a compilation error.

Oliver Letterer

I was just passing the raw certificate as first argument because the iOS header states:

/*!
    @function SecTrustCreateWithCertificates
    @abstract Creates a trust object based on the given certificates and
    policies.
    @param certificates The group of certificates to verify.  This can either
    be a CFArrayRef of SecCertificateRef objects or a single SecCertificateRef
    @param policies An array of one or more policies. You may pass a
    SecPolicyRef to represent a single policy.
    @param trust On return, a pointer to the trust management reference.
    @result A result code.  See "Security Error Codes" (SecBase.h).
    @discussion If multiple policies are passed in, all policies must verify
    for the chain to be considered valid.
*/
OSStatus SecTrustCreateWithCertificates(CFTypeRef certificates,
    CFTypeRef policies, SecTrustRef *trust)
    __OSX_AVAILABLE_STARTING(__MAC_10_3, __IPHONE_2_0);

Didn't know it breaks on Mac.

Mattt Thompson
Owner
mattt commented

It seems like this won't merge cleanly after #900. Would you be able to rebase off of master with this branch, @OliverLetterer? Is there anything else you would need to land this patch?

Oliver Letterer

@mattt I added one missing SecTrustEvaluate call for iOS 5 and hope its good to go now.

Mattt Thompson mattt merged commit 24dd68f into from
Mattt Thompson mattt closed this
Mattt Thompson
Owner
mattt commented

Great. Thanks, @OliverLetterer!

rob phillips iwasrobbed referenced this pull request in iwasrobbed/Forecastr
Closed

Request "cancelled" using latest w/ SSL certificate #11

rob phillips

@OliverLetterer Am I understanding this correctly: If I pin a certificate that is validated against VeriSign, etc., will this (and the current AFNetworking 2.0) implementation check not only that pinned certificate, but also all underlying certificates all the way down to the root OS certificates? I just don't want to have to worry about the certificate expiring every year and then having to update the app, but I'm unsure I understand how to pin against the root OS certificates as a failsafe.

Pinning against the public key has resulted in some intermittent errors after the service updated their SSL certificate, which I'm still trying to understand.

Oliver Letterer OliverLetterer deleted the branch
Oliver Letterer

my implementation checks the pinned certificate and trusts all derived certificates. so if you pin certificate A and your webservices certificate B is derived from A, it will be trusted.

rob phillips

@OliverLetterer So in that example, if an SSL certificate is issued by DigiCert, then pinning against the original DigiCert certificate that it was derived from is best practice or is there a way to pin against root OS certificates? I guess the main question is: How do you prevent having to update your app every time a certificate is updated? Sorry for all the questions, just trying to wrap my head around SSL.

Oliver Letterer

i think stack overflow is more suitable for these kinds of discussions. github issues are for reporting issues only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.

Showing 1 changed file with 101 additions and 70 deletions. Show diff stats Hide diff stats

  1. +101 70 AFNetworking/AFURLConnectionOperation.m
171 AFNetworking/AFURLConnectionOperation.m
@@ -175,7 +175,7 @@ + (NSThread *)networkRequestThread {
175 175 _networkRequestThread = [[NSThread alloc] initWithTarget:self selector:@selector(networkRequestThreadEntryPoint:) object:nil];
176 176 [_networkRequestThread start];
177 177 });
178   -
  178 +
179 179 return _networkRequestThread;
180 180 }
181 181
@@ -186,7 +186,7 @@ + (NSArray *)pinnedCertificates {
186 186 dispatch_once(&onceToken, ^{
187 187 NSBundle *bundle = [NSBundle bundleForClass:[self class]];
188 188 NSArray *paths = [bundle pathsForResourcesOfType:@"cer" inDirectory:@"."];
189   -
  189 +
190 190 NSMutableArray *certificates = [NSMutableArray arrayWithCapacity:[paths count]];
191 191 for (NSString *path in paths) {
192 192 NSData *certificateData = [NSData dataWithContentsOfFile:path];
@@ -195,7 +195,7 @@ + (NSArray *)pinnedCertificates {
195 195
196 196 _pinnedCertificates = [[NSArray alloc] initWithArray:certificates];
197 197 });
198   -
  198 +
199 199 return _pinnedCertificates;
200 200 }
201 201
@@ -209,7 +209,7 @@ + (NSArray *)pinnedPublicKeys {
209 209 for (NSData *data in pinnedCertificates) {
210 210 SecCertificateRef allowedCertificate = SecCertificateCreateWithData(NULL, (__bridge CFDataRef)data);
211 211 NSCParameterAssert(allowedCertificate);
212   -
  212 +
213 213 SecCertificateRef allowedCertificates[] = {allowedCertificate};
214 214 CFArrayRef certificates = CFArrayCreate(NULL, (const void **)allowedCertificates, 1, NULL);
215 215
@@ -225,13 +225,13 @@ + (NSArray *)pinnedPublicKeys {
225 225 SecKeyRef allowedPublicKey = SecTrustCopyPublicKey(allowedTrust);
226 226 NSCParameterAssert(allowedPublicKey);
227 227 [publicKeys addObject:(__bridge_transfer id)allowedPublicKey];
228   -
  228 +
229 229 CFRelease(allowedTrust);
230 230 CFRelease(policy);
231 231 CFRelease(certificates);
232 232 CFRelease(allowedCertificate);
233 233 }
234   -
  234 +
235 235 _pinnedPublicKeys = [[NSArray alloc] initWithArray:publicKeys];
236 236 });
237 237
@@ -244,20 +244,20 @@ - (id)initWithRequest:(NSURLRequest *)urlRequest {
244 244 if (!self) {
245 245 return nil;
246 246 }
247   -
  247 +
248 248 self.lock = [[NSRecursiveLock alloc] init];
249 249 self.lock.name = kAFNetworkingLockName;
250   -
  250 +
251 251 self.runLoopModes = [NSSet setWithObject:NSRunLoopCommonModes];
252   -
  252 +
253 253 self.request = urlRequest;
254   -
  254 +
255 255 self.shouldUseCredentialStorage = YES;
256   -
  256 +
257 257 self.outputStream = [NSOutputStream outputStreamToMemory];
258   -
  258 +
259 259 self.state = AFOperationReadyState;
260   -
  260 +
261 261 return self;
262 262 }
263 263
@@ -266,7 +266,7 @@ - (void)dealloc {
266 266 [_outputStream close];
267 267 _outputStream = nil;
268 268 }
269   -
  269 +
270 270 #if defined(__IPHONE_OS_VERSION_MIN_REQUIRED)
271 271 if (_backgroundTaskIdentifier) {
272 272 [[UIApplication sharedApplication] endBackgroundTask:_backgroundTaskIdentifier];
@@ -287,7 +287,7 @@ - (void)setCompletionBlock:(void (^)(void))block {
287 287 __weak __typeof(&*self)weakSelf = self;
288 288 [super setCompletionBlock:^ {
289 289 __strong __typeof(&*weakSelf)strongSelf = weakSelf;
290   -
  290 +
291 291 block();
292 292 [strongSelf setCompletionBlock:nil];
293 293 }];
@@ -311,7 +311,7 @@ - (void)setOutputStream:(NSOutputStream *)outputStream {
311 311 if (outputStream == _outputStream) {
312 312 return;
313 313 }
314   -
  314 +
315 315 [self willChangeValueForKey:@"outputStream"];
316 316 if (_outputStream) {
317 317 [_outputStream close];
@@ -328,14 +328,14 @@ - (void)setShouldExecuteAsBackgroundTaskWithExpirationHandler:(void (^)(void))ha
328 328 __weak __typeof(&*self)weakSelf = self;
329 329 self.backgroundTaskIdentifier = [application beginBackgroundTaskWithExpirationHandler:^{
330 330 __strong __typeof(&*weakSelf)strongSelf = weakSelf;
331   -
  331 +
332 332 if (handler) {
333 333 handler();
334 334 }
335   -
  335 +
336 336 if (strongSelf) {
337 337 [strongSelf cancel];
338   -
  338 +
339 339 [application endBackgroundTask:strongSelf.backgroundTaskIdentifier];
340 340 strongSelf.backgroundTaskIdentifier = UIBackgroundTaskInvalid;
341 341 }
@@ -392,7 +392,7 @@ - (NSString *)responseString {
392 392 self.responseString = [[NSString alloc] initWithData:self.responseData encoding:self.responseStringEncoding];
393 393 }
394 394 [self.lock unlock];
395   -
  395 +
396 396 return _responseString;
397 397 }
398 398
@@ -406,11 +406,11 @@ - (NSStringEncoding)responseStringEncoding {
406 406 stringEncoding = CFStringConvertEncodingToNSStringEncoding(IANAEncoding);
407 407 }
408 408 }
409   -
  409 +
410 410 self.responseStringEncoding = stringEncoding;
411 411 }
412 412 [self.lock unlock];
413   -
  413 +
414 414 return _responseStringEncoding;
415 415 }
416 416
@@ -418,20 +418,20 @@ - (void)pause {
418 418 if ([self isPaused] || [self isFinished] || [self isCancelled]) {
419 419 return;
420 420 }
421   -
  421 +
422 422 [self.lock lock];
423   -
  423 +
424 424 if ([self isExecuting]) {
425 425 [self.connection performSelector:@selector(cancel) onThread:[[self class] networkRequestThread] withObject:nil waitUntilDone:NO modes:[self.runLoopModes allObjects]];
426   -
  426 +
427 427 dispatch_async(dispatch_get_main_queue(), ^{
428 428 NSNotificationCenter *notificationCenter = [NSNotificationCenter defaultCenter];
429 429 [notificationCenter postNotificationName:AFNetworkingOperationDidFinishNotification object:self];
430 430 });
431 431 }
432   -
  432 +
433 433 self.state = AFOperationPausedState;
434   -
  434 +
435 435 [self.lock unlock];
436 436 }
437 437
@@ -443,10 +443,10 @@ - (void)resume {
443 443 if (![self isPaused]) {
444 444 return;
445 445 }
446   -
  446 +
447 447 [self.lock lock];
448 448 self.state = AFOperationReadyState;
449   -
  449 +
450 450 [self start];
451 451 [self.lock unlock];
452 452 }
@@ -473,7 +473,7 @@ - (void)start {
473 473 [self.lock lock];
474 474 if ([self isReady]) {
475 475 self.state = AFOperationExecutingState;
476   -
  476 +
477 477 [self performSelector:@selector(operationDidStart) onThread:[[self class] networkRequestThread] withObject:nil waitUntilDone:NO modes:[self.runLoopModes allObjects]];
478 478 }
479 479 [self.lock unlock];
@@ -518,7 +518,7 @@ - (void)cancel {
518 518 _cancelled = YES;
519 519 [super cancel];
520 520 [self didChangeValueForKey:@"isCancelled"];
521   -
  521 +
522 522 // Cancel the connection on the thread it runs on to prevent race conditions
523 523 [self performSelector:@selector(cancelConnection) onThread:[[self class] networkRequestThread] withObject:nil waitUntilDone:NO modes:[self.runLoopModes allObjects]];
524 524 }
@@ -531,10 +531,10 @@ - (void)cancelConnection {
531 531 userInfo = [NSDictionary dictionaryWithObject:[self.request URL] forKey:NSURLErrorFailingURLErrorKey];
532 532 }
533 533 self.error = [NSError errorWithDomain:NSURLErrorDomain code:NSURLErrorCancelled userInfo:userInfo];
534   -
  534 +
535 535 if (self.connection) {
536 536 [self.connection cancel];
537   -
  537 +
538 538 // Manually send this delegate message since `[self.connection cancel]` causes the connection to never send another message to its delegate
539 539 [self performSelector:@selector(connection:didFailWithError:) withObject:self.connection withObject:self.error];
540 540 }
@@ -548,30 +548,61 @@ - (void)connection:(NSURLConnection *)connection
548 548 {
549 549 if ([challenge.protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodServerTrust]) {
550 550 SecTrustRef serverTrust = challenge.protectionSpace.serverTrust;
  551 +
  552 + SecPolicyRef policy = SecPolicyCreateBasicX509();
  553 + CFIndex certificateCount = SecTrustGetCertificateCount(serverTrust);
  554 + NSMutableArray *trustChain = [NSMutableArray arrayWithCapacity:certificateCount];
  555 +
  556 + for (CFIndex i = 0; i < certificateCount; i++) {
  557 + SecCertificateRef certificate = SecTrustGetCertificateAtIndex(serverTrust, i);
  558 +
  559 + if (self.SSLPinningMode == AFSSLPinningModeCertificate) {
  560 + [trustChain addObject:(__bridge_transfer NSData *)SecCertificateCopyData(certificate)];
  561 + } else if (self.SSLPinningMode == AFSSLPinningModePublicKey) {
  562 + SecCertificateRef someCertificates[] = {certificate};
  563 + CFArrayRef certificates = CFArrayCreate(NULL, (const void **)someCertificates, 1, NULL);
  564 +
  565 + SecTrustRef trust = NULL;
  566 +
  567 + OSStatus status = SecTrustCreateWithCertificates(certificates, policy, &trust);
  568 + NSAssert(status == noErr, @"SecTrustCreateWithCertificates error: %ld", (long int)status);
  569 +
  570 + SecTrustResultType result;
  571 + status = SecTrustEvaluate(trust, &result);
  572 + NSAssert(status == noErr, @"SecTrustEvaluate error: %ld", (long int)status);
  573 +
  574 + [trustChain addObject:(__bridge_transfer id)SecTrustCopyPublicKey(trust)];
  575 +
  576 + CFRelease(trust);
  577 + CFRelease(certificates);
  578 + }
  579 + }
  580 +
  581 + CFRelease(policy);
  582 +
551 583 switch (self.SSLPinningMode) {
552 584 case AFSSLPinningModePublicKey: {
553   - id publicKey = (__bridge_transfer id)SecTrustCopyPublicKey(serverTrust);
554   -
555   - if ([[self.class pinnedPublicKeys] containsObject:publicKey]) {
556   - NSURLCredential *credential = [NSURLCredential credentialForTrust:serverTrust];
557   - [[challenge sender] useCredential:credential forAuthenticationChallenge:challenge];
558   - } else {
559   - [[challenge sender] cancelAuthenticationChallenge:challenge];
  585 + for (id publicKey in trustChain) {
  586 + if ([[self.class pinnedPublicKeys] containsObject:publicKey]) {
  587 + NSURLCredential *credential = [NSURLCredential credentialForTrust:serverTrust];
  588 + [[challenge sender] useCredential:credential forAuthenticationChallenge:challenge];
  589 + return;
  590 + }
560 591 }
561 592
  593 + [[challenge sender] cancelAuthenticationChallenge:challenge];
562 594 break;
563 595 }
564 596 case AFSSLPinningModeCertificate: {
565   - SecCertificateRef serverCertificate = SecTrustGetCertificateAtIndex(serverTrust, 0);
566   - NSData *serverCertificateData = (__bridge_transfer NSData *)SecCertificateCopyData(serverCertificate);
567   -
568   - if ([[[self class] pinnedCertificates] containsObject:serverCertificateData]) {
569   - NSURLCredential *credential = [NSURLCredential credentialForTrust:serverTrust];
570   - [[challenge sender] useCredential:credential forAuthenticationChallenge:challenge];
571   - } else {
572   - [[challenge sender] cancelAuthenticationChallenge:challenge];
  597 + for (id serverCertificateData in trustChain) {
  598 + if ([[self.class pinnedCertificates] containsObject:serverCertificateData]) {
  599 + NSURLCredential *credential = [NSURLCredential credentialForTrust:serverTrust];
  600 + [[challenge sender] useCredential:credential forAuthenticationChallenge:challenge];
  601 + return;
  602 + }
573 603 }
574 604
  605 + [[challenge sender] cancelAuthenticationChallenge:challenge];
575 606 break;
576 607 }
577 608 case AFSSLPinningModeNone: {
@@ -607,7 +638,7 @@ - (BOOL)connection:(NSURLConnection *)connection
607 638 return YES;
608 639 }
609 640 #endif
610   -
  641 +
611 642 if (self.authenticationAgainstProtectionSpace) {
612 643 return self.authenticationAgainstProtectionSpace(connection, protectionSpace);
613 644 } else if ([protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodServerTrust] || [protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodClientCertificate]) {
@@ -626,16 +657,16 @@ - (void)connection:(NSURLConnection *)connection
626 657 return;
627 658 }
628 659 #endif
629   -
  660 +
630 661 if (self.authenticationChallenge) {
631 662 self.authenticationChallenge(connection, challenge);
632 663 } else {
633 664 if ([challenge previousFailureCount] == 0) {
634 665 NSURLCredential *credential = nil;
635   -
  666 +
636 667 NSString *user = [[self.request URL] user];
637 668 NSString *password = [[self.request URL] password];
638   -
  669 +
639 670 if (user && password) {
640 671 credential = [NSURLCredential credentialWithUser:user password:password persistence:NSURLCredentialPersistenceNone];
641 672 } else if (user) {
@@ -643,11 +674,11 @@ - (void)connection:(NSURLConnection *)connection
643 674 } else {
644 675 credential = [[NSURLCredentialStorage sharedCredentialStorage] defaultCredentialForProtectionSpace:[challenge protectionSpace]];
645 676 }
646   -
  677 +
647 678 if (!credential) {
648 679 credential = self.credential;
649 680 }
650   -
  681 +
651 682 if (credential) {
652 683 [[challenge sender] useCredential:credential forAuthenticationChallenge:challenge];
653 684 } else {
@@ -669,7 +700,7 @@ - (NSInputStream *)connection:(NSURLConnection __unused *)connection
669 700 if ([request.HTTPBodyStream conformsToProtocol:@protocol(NSCopying)]) {
670 701 return [request.HTTPBodyStream copy];
671 702 }
672   -
  703 +
673 704 return nil;
674 705 }
675 706
@@ -700,7 +731,7 @@ - (void)connection:(NSURLConnection __unused *)connection
700 731 didReceiveResponse:(NSURLResponse *)response
701 732 {
702 733 self.response = response;
703   -
  734 +
704 735 [self.outputStream open];
705 736 }
706 737
@@ -723,11 +754,11 @@ - (void)connection:(NSURLConnection __unused *)connection
723 754
724 755 - (void)connectionDidFinishLoading:(NSURLConnection __unused *)connection {
725 756 self.responseData = [self.outputStream propertyForKey:NSStreamDataWrittenToMemoryStreamKey];
726   -
  757 +
727 758 [self.outputStream close];
728   -
  759 +
729 760 [self finish];
730   -
  761 +
731 762 self.connection = nil;
732 763 }
733 764
@@ -735,11 +766,11 @@ - (void)connection:(NSURLConnection __unused *)connection
735 766 didFailWithError:(NSError *)error
736 767 {
737 768 self.error = error;
738   -
  769 +
739 770 [self.outputStream close];
740   -
  771 +
741 772 [self finish];
742   -
  773 +
743 774 self.connection = nil;
744 775 }
745 776
@@ -752,7 +783,7 @@ - (NSCachedURLResponse *)connection:(NSURLConnection *)connection
752 783 if ([self isCancelled]) {
753 784 return nil;
754 785 }
755   -
  786 +
756 787 return cachedResponse;
757 788 }
758 789 }
@@ -761,27 +792,27 @@ - (NSCachedURLResponse *)connection:(NSURLConnection *)connection
761 792
762 793 - (id)initWithCoder:(NSCoder *)aDecoder {
763 794 NSURLRequest *request = [aDecoder decodeObjectForKey:@"request"];
764   -
  795 +
765 796 self = [self initWithRequest:request];
766 797 if (!self) {
767 798 return nil;
768 799 }
769   -
  800 +
770 801 self.state = (AFOperationState)[aDecoder decodeIntegerForKey:@"state"];
771 802 self.cancelled = [aDecoder decodeBoolForKey:@"isCancelled"];
772 803 self.response = [aDecoder decodeObjectForKey:@"response"];
773 804 self.error = [aDecoder decodeObjectForKey:@"error"];
774 805 self.responseData = [aDecoder decodeObjectForKey:@"responseData"];
775 806 self.totalBytesRead = [[aDecoder decodeObjectForKey:@"totalBytesRead"] longLongValue];
776   -
  807 +
777 808 return self;
778 809 }
779 810
780 811 - (void)encodeWithCoder:(NSCoder *)aCoder {
781 812 [self pause];
782   -
  813 +
783 814 [aCoder encodeObject:self.request forKey:@"request"];
784   -
  815 +
785 816 switch (self.state) {
786 817 case AFOperationExecutingState:
787 818 case AFOperationPausedState:
@@ -791,7 +822,7 @@ - (void)encodeWithCoder:(NSCoder *)aCoder {
791 822 [aCoder encodeInteger:self.state forKey:@"state"];
792 823 break;
793 824 }
794   -
  825 +
795 826 [aCoder encodeBool:[self isCancelled] forKey:@"isCancelled"];
796 827 [aCoder encodeObject:self.response forKey:@"response"];
797 828 [aCoder encodeObject:self.error forKey:@"error"];
@@ -803,7 +834,7 @@ - (void)encodeWithCoder:(NSCoder *)aCoder {
803 834
804 835 - (id)copyWithZone:(NSZone *)zone {
805 836 AFURLConnectionOperation *operation = [(AFURLConnectionOperation *)[[self class] allocWithZone:zone] initWithRequest:self.request];
806   -
  837 +
807 838 operation.uploadProgress = self.uploadProgress;
808 839 operation.downloadProgress = self.downloadProgress;
809 840 operation.authenticationAgainstProtectionSpace = self.authenticationAgainstProtectionSpace;

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.