Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Changed SSL Pinning Error to be NSURLErrorServerCertificateUntrusted #3191

Merged
merged 8 commits into from Dec 3, 2015
Merged

Changed SSL Pinning Error to be NSURLErrorServerCertificateUntrusted #3191

merged 8 commits into from Dec 3, 2015

Conversation

kcharwood
Copy link
Contributor

Cleaned up @0xced's work in #3169 and got the test to pass.

0xced and others added 8 commits December 3, 2015 11:05
@0xced @kcharwood
Better error when the server trust is invalid
7b4d617 
When the server trust is invalid, using `NSURLSessionAuthChallengeCancelAuthenticationChallenge` terminates the task with an error from `NSURLErrorDomain` with code `NSURLErrorCancelled` (-999) which is indistinguishable from the error you get when calling the `cancel` method on a `NSURLSessionTask`.

Using `NSURLSessionAuthChallengeRejectProtectionSpace` instead produces a much better error:
Error Domain:           NSURLErrorDomain
Code:                   NSURLErrorServerCertificateUntrusted (-1202)
NSLocalizedDescription: "The certificate for this server is invalid. You might be connecting to a server that is pretending to be “httpbin.org” which could put your confidential information at risk."

Fixes #3165
@0xced @kcharwood
Add some debug logs
86a1196 
Trying to figure out why the `testInvalidServerTrustProducesCorrectError` test passes on iOS 9 and tvOS but fails on iOS 8 and OS X here: https://travis-ci.org/AFNetworking/AFNetworking/builds/92142622
@0xced @kcharwood
Display verbose logs in case of failure
97eb278
@0xced @kcharwood
Log all NSURLSession delegate methods
2ce441e
@0xced @kcharwood
Log the session too
88080eb
@0xced @kcharwood
Add logs for NSURLSession(Data|Download)TaskDelegate
b14e740
@kcharwood
Cleaning up logs
2de951b
@kcharwood
Change https domain to test
9e38c8e
@kcharwood kcharwood added this to the 3.0.0 milestone Dec 3, 2015
@kcharwood kcharwood mentioned this pull request Dec 3, 2015
Closed
kcharwood added a commit that referenced this pull request Dec 3, 2015
@kcharwood
Merge pull request #3191 from AFNetworking/change/improve_pinning_error
f5388f0 
Changed SSL Pinning Error to be `NSURLErrorServerCertificateUntrusted`
@kcharwood kcharwood merged commit f5388f0 into 3_0_0_branch Dec 3, 2015
@kcharwood
Copy link
Contributor Author

🍻

@kcharwood kcharwood deleted the change/improve_pinning_error branch December 3, 2015 18:10
@0xced
Copy link
Contributor

0xced commented Dec 4, 2015

Excellent, thanks for fixing it.

Now there’s one problem remaining: the same code in the OS X Demo app returns NSURLErrorSecureConnectionFailed (-1200) instead of NSURLErrorServerCertificateUntrusted (-1202).

Maybe we should just assert error.code == NSURLErrorSecureConnectionFailed || error.code == NSURLErrorServerCertificateUntrusted in the unit test and move on?

@kcharwood
Copy link
Contributor Author

Added the after_failure step back in 6831f21

@0xced
Copy link
Contributor

0xced commented Dec 4, 2015

👍

@kcharwood
Copy link
Contributor Author

I just ran the code in the OS X demo app, and got -1202

po error
Error Domain=NSURLErrorDomain Code=-1202 "The certificate for this server is invalid. You might be connecting to a server that is pretending to be “apple.com” which could put your confidential information at risk." UserInfo={NSURLErrorFailingURLPeerTrustErrorKey=<SecTrust 0x10061a8f0 [0x7fff77785390]>, NSLocalizedRecoverySuggestion=Would you like to connect to the server anyway?, _kCFStreamErrorDomainKey=3, _kCFStreamErrorCodeKey=-9813, NSErrorPeerCertificateChainKey=(
    "<SecCertificate 0x100619d60 [0x7fff77785390]>",
    "<SecCertificate 0x100619fa0 [0x7fff77785390]>",
    "<SecCertificate 0x10061a580 [0x7fff77785390]>"
), NSUnderlyingError=0x60000024afb0 {Error Domain=kCFErrorDomainCFNetwork Code=-1202 "(null)" UserInfo={_kCFStreamPropertySSLClientCertificateState=0, kCFStreamPropertySSLPeerTrust=<SecTrust 0x10061a8f0 [0x7fff77785390]>, _kCFNetworkCFStreamSSLErrorOriginalValue=-9813, _kCFStreamErrorDomainKey=3, _kCFStreamErrorCodeKey=-9813, kCFStreamPropertySSLPeerCertificates=(
    "<SecCertificate 0x100619d60 [0x7fff77785390]>",
    "<SecCertificate 0x100619fa0 [0x7fff77785390]>",
    "<SecCertificate 0x10061a580 [0x7fff77785390]>"
)}}, NSLocalizedDescription=The certificate for this server is invalid. You might be connecting to a server that is pretending to be “apple.com” which could put your confidential information at risk., NSErrorFailingURLKey=https://apple.com/AFNetworking/AFNetworking, NSErrorFailingURLStringKey=https://apple.com/AFNetworking/AFNetworking, NSErrorClientCertificateStateKey=0}

@0xced
Copy link
Contributor

0xced commented Dec 10, 2015

Here is what I get with my pinning-error-in-demo-app branch:

po error
Error Domain=NSURLErrorDomain Code=-1200 "An SSL error has occurred and a secure connection to the server cannot be made." UserInfo={NSURLErrorFailingURLPeerTrustErrorKey=<SecTrust 0x10071ceb0 [0x7fff74e25390]>, NSLocalizedRecoverySuggestion=Would you like to connect to the server anyway?, _kCFStreamErrorDomainKey=3, _kCFStreamErrorCodeKey=-9802, NSErrorPeerCertificateChainKey=(
    "<SecCertificate 0x10071c5b0 [0x7fff74e25390]>",
    "<SecCertificate 0x10071c7f0 [0x7fff74e25390]>"
), NSUnderlyingError=0x61800004e8e0 {Error Domain=kCFErrorDomainCFNetwork Code=-1200 "(null)" UserInfo={_kCFStreamPropertySSLClientCertificateState=0, kCFStreamPropertySSLPeerTrust=<SecTrust 0x10071ceb0 [0x7fff74e25390]>, _kCFNetworkCFStreamSSLErrorOriginalValue=-9802, _kCFStreamErrorDomainKey=3, _kCFStreamErrorCodeKey=-9802, kCFStreamPropertySSLPeerCertificates=(
    "<SecCertificate 0x10071c5b0 [0x7fff74e25390]>",
    "<SecCertificate 0x10071c7f0 [0x7fff74e25390]>"
)}}, NSLocalizedDescription=An SSL error has occurred and a secure connection to the server cannot be made., NSErrorFailingURLKey=https://api.app.net/stream/0/posts/stream/global, NSErrorFailingURLStringKey=https://api.app.net/stream/0/posts/stream/global, NSErrorClientCertificateStateKey=0}

Running OS X 10.11.1 (15B42).

Can you try to run this branch and see if you get the -1200 or -1202 error?

@kcharwood
Copy link
Contributor Author

OOOOOKKKKK...

If I add App Transport Security (Allow Arbitrary Loads) settings to the Mac Example, I get -1202. Without it, I get -1200.

@0xced
Copy link
Contributor

0xced commented Dec 10, 2015

Of course, -1200 means an ATS issue, I should have known better! Thanks for clearing that up.

@brow brow mentioned this pull request Mar 30, 2016
Closed
@0xced 0xced mentioned this pull request Apr 4, 2016
Merged
@jshier jshier mentioned this pull request Dec 14, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
changed security
Projects
None yet
Milestone
3.0.0
2 participants
@kcharwood @0xced