AFHTTPSessionManager now throws exception if SSL pinning mode is set for non https sessions
#3687
Conversation
Current coverage is 86.75% (diff: 100%)@@ master #3687 diff @@
==========================================
Files 44 44
Lines 6067 6110 +43
Methods 1079 1088 +9
Messages 0 0
Branches 407 408 +1
==========================================
+ Hits 5256 5301 +45
+ Misses 808 806 -2
Partials 3 3
|
7c49173 to
6fc3e37
Compare
| [super setResponseSerializer:responseSerializer]; | ||
| } | ||
|
|
||
| - (void)setSecurityPolicy:(AFSecurityPolicy *)securityPolicy { |
There was a problem hiding this comment.
Should we add a note to the documentation declaring this behavior?
There was a problem hiding this comment.
Yes, that would be a good idea. The only issue is that this behaviour only applies at the AFHTTPSessionManager level (as AFURLSessionManager has no base URL). So, should we redefine the securityPolicy property on AFHTTPSessionManager for the sole purpose of augmenting its documentation? Or is it going to be more confusing?
There was a problem hiding this comment.
I think that would be a good idea, since that behavior only affects AFHTTPSessionManager
There was a problem hiding this comment.
OK, I just rebased on master and force-pushed with documentation.
e4d3f1b to
181d633
Compare
|
After much struggling, all tests finally pass! |
|
@0xced awesome! I think it would make sense to cherry-pick the tests stability into its on PR, so we can have a direct reference to that and it doesn't get lost in this changeset. Would you be able to do that? If not, I can take a stab at it. |
|
Yes, I will do it. |
424fd01 to
e698f45
Compare
|
Can you rebase this with master to get Xcode 8 coverage? |
…URLs ### Before this commit Setting a security policy configured with `AFSSLPinningModeCertificate` or `AFSSLPinningModePublicKey` on a AFHTTPSessionManager instance configured with an insecure `http` base URL was valid. Requests made with this manager would always succeed since the `-[AFURLSessionManager URLSession:didReceiveChallenge:completionHandler:]` would never be called and thus the security policy would never be evaluated. ### After this commit Setting a security policy configured with `AFSSLPinningModeCertificate` or `AFSSLPinningModePublicKey` on a AFHTTPSessionManager instance configured with an insecure `http` base URL will throw an exception. This will force the manager to be configured with a secure `https` URL. Note that properly configuring App Transport Security (ATS) would also solve this issue since insecure connections would fail anyway, but this is a *belt and suspenders* solution.
e698f45 to
f9ed2d2
Compare
|
I rebased, but I think you should be able to rebase and merge from the GitHub interface now. |
|
I wanted to see the green check before merging! |
|
I realized right after posting. 😄 |
AFHTTPSessionManager now throws exception if SSL pinning mode is set for non https sessions
Before this commit
Setting a security policy configured with
AFSSLPinningModeCertificateorAFSSLPinningModePublicKeyon a AFHTTPSessionManager instance configured with an insecurehttpbase URL was valid. Requests made with this manager would always succeed since the-[AFURLSessionManager URLSession:didReceiveChallenge:completionHandler:]would never be called and thus the security policy would never be evaluated.After this commit
Setting a security policy configured with
AFSSLPinningModeCertificateorAFSSLPinningModePublicKeyon a AFHTTPSessionManager instance configured with an insecurehttpbase URL will throw an exception. This will force the manager to be configured with a securehttpsURL.Note that properly configuring App Transport Security (ATS) would also solve this issue since insecure connections would fail anyway, but this is a belt and suspenders solution.