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

AFHTTPSessionManager now throws exception if SSL pinning mode is set for non https sessions #3687

Merged
merged 1 commit into from
Oct 6, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions AFNetworking/AFHTTPSessionManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,15 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property (nonatomic, strong) AFHTTPResponseSerializer <AFURLResponseSerialization> * responseSerializer;

///-------------------------------
/// @name Managing Security Policy
///-------------------------------

/**
The security policy used by created session to evaluate server trust for secure connections. `AFURLSessionManager` uses the `defaultPolicy` unless otherwise specified. A security policy configured with `AFSSLPinningModePublicKey` or `AFSSLPinningModeCertificate` can only be applied on a session manager initialized with a secure base URL (i.e. https). Applying a security policy with pinning enabled on an insecure session manager throws an `Invalid Security Policy` exception.
*/
@property (nonatomic, strong) AFSecurityPolicy *securityPolicy;

///---------------------
/// @name Initialization
///---------------------
Expand Down
17 changes: 17 additions & 0 deletions AFNetworking/AFHTTPSessionManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,23 @@ - (void)setResponseSerializer:(AFHTTPResponseSerializer <AFURLResponseSerializat
[super setResponseSerializer:responseSerializer];
}

@dynamic securityPolicy;

- (void)setSecurityPolicy:(AFSecurityPolicy *)securityPolicy {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a note to the documentation declaring this behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be a good idea, since that behavior only affects AFHTTPSessionManager

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I just rebased on master and force-pushed with documentation.

if (securityPolicy.SSLPinningMode != AFSSLPinningModeNone && ![self.baseURL.scheme isEqualToString:@"https"]) {
NSString *pinningMode = @"Unknown Pinning Mode";
switch (securityPolicy.SSLPinningMode) {
case AFSSLPinningModeNone: pinningMode = @"AFSSLPinningModeNone"; break;
case AFSSLPinningModeCertificate: pinningMode = @"AFSSLPinningModeCertificate"; break;
case AFSSLPinningModePublicKey: pinningMode = @"AFSSLPinningModePublicKey"; break;
}
NSString *reason = [NSString stringWithFormat:@"A security policy configured with `%@` can only be applied on a manager with a secure base URL (i.e. https)", pinningMode];
@throw [NSException exceptionWithName:@"Invalid Security Policy" reason:reason userInfo:nil];
}

[super setSecurityPolicy:securityPolicy];
}

#pragma mark -

- (NSURLSessionDataTask *)GET:(NSString *)URLString
Expand Down
50 changes: 50 additions & 0 deletions Tests/Tests/AFHTTPSessionManagerTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,56 @@ - (void)testHiddenBasicAuthentication {
[self waitForExpectationsWithCommonTimeoutUsingHandler:nil];
}

# pragma mark - Security Policy

- (void)testValidSecureNoPinningSecurityPolicy {
AFHTTPSessionManager *manager = [[AFHTTPSessionManager alloc] initWithBaseURL:[NSURL URLWithString:@"https://example.com"]];
AFSecurityPolicy *securityPolicy = [AFSecurityPolicy policyWithPinningMode:AFSSLPinningModeNone];
XCTAssertNoThrow(manager.securityPolicy = securityPolicy);
}

- (void)testValidInsecureNoPinningSecurityPolicy {
AFHTTPSessionManager *manager = [[AFHTTPSessionManager alloc] initWithBaseURL:[NSURL URLWithString:@"http://example.com"]];
AFSecurityPolicy *securityPolicy = [AFSecurityPolicy policyWithPinningMode:AFSSLPinningModeNone];
XCTAssertNoThrow(manager.securityPolicy = securityPolicy);
}

- (void)testValidCertificatePinningSecurityPolicy {
AFHTTPSessionManager *manager = [[AFHTTPSessionManager alloc] initWithBaseURL:[NSURL URLWithString:@"https://example.com"]];
AFSecurityPolicy *securityPolicy = [AFSecurityPolicy policyWithPinningMode:AFSSLPinningModeCertificate];
XCTAssertNoThrow(manager.securityPolicy = securityPolicy);
}

- (void)testInvalidCertificatePinningSecurityPolicy {
AFHTTPSessionManager *manager = [[AFHTTPSessionManager alloc] initWithBaseURL:[NSURL URLWithString:@"http://example.com"]];
AFSecurityPolicy *securityPolicy = [AFSecurityPolicy policyWithPinningMode:AFSSLPinningModeCertificate];
XCTAssertThrowsSpecificNamed(manager.securityPolicy = securityPolicy, NSException, @"Invalid Security Policy");
}

- (void)testValidPublicKeyPinningSecurityPolicy {
AFHTTPSessionManager *manager = [[AFHTTPSessionManager alloc] initWithBaseURL:[NSURL URLWithString:@"https://example.com"]];
AFSecurityPolicy *securityPolicy = [AFSecurityPolicy policyWithPinningMode:AFSSLPinningModePublicKey];
XCTAssertNoThrow(manager.securityPolicy = securityPolicy);
}

- (void)testInvalidPublicKeyPinningSecurityPolicy {
AFHTTPSessionManager *manager = [[AFHTTPSessionManager alloc] initWithBaseURL:[NSURL URLWithString:@"http://example.com"]];
AFSecurityPolicy *securityPolicy = [AFSecurityPolicy policyWithPinningMode:AFSSLPinningModePublicKey];
XCTAssertThrowsSpecificNamed(manager.securityPolicy = securityPolicy, NSException, @"Invalid Security Policy");
}

- (void)testInvalidCertificatePinningSecurityPolicyWithoutBaseURL {
AFHTTPSessionManager *manager = [[AFHTTPSessionManager alloc] init];
AFSecurityPolicy *securityPolicy = [AFSecurityPolicy policyWithPinningMode:AFSSLPinningModeCertificate];
XCTAssertThrowsSpecificNamed(manager.securityPolicy = securityPolicy, NSException, @"Invalid Security Policy");
}

- (void)testInvalidPublicKeyPinningSecurityPolicyWithoutBaseURL {
AFHTTPSessionManager *manager = [[AFHTTPSessionManager alloc] init];
AFSecurityPolicy *securityPolicy = [AFSecurityPolicy policyWithPinningMode:AFSSLPinningModePublicKey];
XCTAssertThrowsSpecificNamed(manager.securityPolicy = securityPolicy, NSException, @"Invalid Security Policy");
}

# pragma mark - Server Trust

- (void)testInvalidServerTrustProducesCorrectErrorForCertificatePinning {
Expand Down