Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't validate domain name without using public key nor pinned certificates? #2560

Closed
jcayzac opened this issue Feb 13, 2015 · 5 comments

Comments

Projects
None yet
4 participants
@jcayzac
Copy link
Contributor

commented Feb 13, 2015

In AFSecurityPolicy:

    NSMutableArray *policies = [NSMutableArray array];
    if (self.validatesDomainName) {
        [policies addObject:(__bridge_transfer id)SecPolicyCreateSSL(true, (__bridge CFStringRef)domain)];
    }
    [...]
    if (self.SSLPinningMode != AFSSLPinningModeNone && !AFServerTrustIsValid(serverTrust) && !self.allowInvalidCertificates) {
        return NO;
    }
    [...]
    switch (self.SSLPinningMode) {
        case AFSSLPinningModeNone:
            return YES; <<< ?

…why do you always return YES even if validatesDomainName is set? IMHO this last line should read
return AFServerTrustIsValid(serverTrust), with serverTrust only containing the domain policy set above.

@jakubvano

This comment has been minimized.

Copy link

commented Feb 17, 2015

I have similar issue with current implementation of -evaluateServerTrust:forDomain:

When SSLPinningMode == AFSSLPinningModeNone, no validation of server trust is performed and -evaluateServerTrust:forDomain: always returns YES.

I believe issue was introduced by commit 104ce04.
It states that change was performed due to failing test, but for me tests succeed with or without the change.

Proposed solution is to revert mentioned commit.

@jcayzac

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2015

I agree 104ce04 looks like a bad solution for the test it was supposed to fix.

@jcayzac

This comment has been minimized.

Copy link
Contributor Author

commented Feb 28, 2015

Note that the security implication for this is that by default AFNetworking happily connects to https servers with invalid certificates. The company I work for has several dozen apps that just had to be updated to work around this ーit's an e-commerce company, so a few people there were quite stressed as you can guess 👯

@mattt

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2015

Fixed by 90bd342. See #2611 for discussion.

@mattt mattt closed this Mar 26, 2015

@peterpaulis

This comment has been minimized.

Copy link

commented Sep 23, 2015

to go around this add

    // security policy
    {
        AFSecurityPolicy* policy = [AFSecurityPolicy policyWithPinningMode:AFSSLPinningModeNone];
        [policy setValidatesDomainName:NO];
        [policy setAllowInvalidCertificates:NO];
        self.requestOperationManager.securityPolicy = policy;
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.