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

Add support for invalid/untrusted SSL certificates (for debugging) #204

Closed
wants to merge 1 commit into from
Closed

Add support for invalid/untrusted SSL certificates (for debugging) #204

wants to merge 1 commit into from

Conversation

steipete
Copy link
Contributor

@steipete steipete commented Feb 8, 2012

... if the constant AFNETWORKING_ALLOW_INVALID_SSL_CERTIFICATES is set.

This is useful if you debug on custom servers in SSL mode, or just want to use Charles to capture the packets in SSL. It's only a minor change and doesn't get compiled if the constant is not set.

…FNETWORKING_ALLOW_INVALID_SSL_CERTIFICATES_ is set.

This is useful if you debug on custom servers in SSL mode, or just want to use Charles to capture the packets in SSL. It's only a minor change and doesn't get compiled if the constant is not set.
@steipete
Copy link
Contributor Author

steipete commented Feb 8, 2012

There are some other ways to achieve that with private API, but this solution only uses public API. (It just really shouldn't be enabled in a release build)

@kcharwood
Copy link
Contributor

I might be misunderstanding the purpose of the authentication block. I thought @mattt added that specifically for this case in mind. I would be really interested to hear his feedback on this issue because I occasionally run into the bad cert issue with a development server, but haven't tried to solve the problem in any of my AF projects yet.

I imagined some sort of implementation like this in your AFHTTPClient subclass:

- (AFHTTPRequestOperation*)HTTPRequestOperationWithRequest:(NSURLRequest *)request success:(void (^)(AFHTTPRequestOperation *, id))success failure:(void (^)(AFHTTPRequestOperation *, NSError *))failure{
    AFHTTPRequestOperation * operation = [super HTTPRequestOperationWithRequest:request success:success failure:failure];
#ifdef DEBUG
    [operation setAuthenticationChallengeBlock:^(NSURLConnection *connection, NSURLAuthenticationChallenge *challenge) {
        if ([challenge.protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodServerTrust]) {
            [challenge.sender useCredential:[NSURLCredential credentialForTrust:challenge.protectionSpace.serverTrust] forAuthenticationChallenge:challenge];
        }
    }];
#endif
    return operation;
};

@steipete
Copy link
Contributor Author

steipete commented Feb 9, 2012

Without also overriding canAuthenticateAgainstProtectionSpace, the authentication challenge is never called. So while I could use this block, i'd still need to override/monkey patch AFURLConnectionOperation.

@kcharwood
Copy link
Contributor

Ah gotcha. Missed that. I think the best solution would be doing it without requiring a AF level #define to do it. I know ASI had a property you simply set to ON.

@steipete
Copy link
Contributor Author

steipete commented Feb 9, 2012

I'm really not sure what the best practice on this is - I feel the default setup (protecting you from man in the middle attacks etc) and this not even calling the auth-code is the best way of handling it - this protects you from possible mistakes on the actual auth challenge.

I really just want a solution that is (like in my app) automatically enabled in DEBUG but doesn't even get compiled into a release build.

@mattt
Copy link
Contributor

mattt commented Feb 9, 2012

I had planned to add another block property to intercept that protection space delegate method (maybe roll into the other auth method) for 0.9, but (and this is a pretty dumb reason) I couldn't figure out a good name for any of that.

A block is the right solution for the same reasons it works for the can authenticate method. If anyone has any opinions about how to package that up, I'm all ears.

On 2012/02/08, at 16:15, Peter Steinbergerreply@reply.github.com wrote:

I'm really not sure what the best practice on this is - I feel the default setup (protecting you from man in the middle attacks etc) and this not even calling the auth-code is the best way of handling it - this protects you from possible mistakes on the actual auth challenge.

I really just want a solution that is (like in my app) automatically enabled in DEBUG but doesn't even get compiled into a release build.


Reply to this email directly or view it on GitHub:
#204 (comment)

@steipete
Copy link
Contributor Author

Well, we could name it canAuthenticateBlock(NSURLProtectionSpace * protectionSpace) because it returns a BOOL.

Still, I would love to have sth like AFNETWORKING_ALLOW_INVALID_SSL_CERTIFICATES in the codebase as I'm sure there are more developers that will need this, and it's a lot easier to just set this define than adding blocks (again, those blocks may be already used for custom auth on some properties, so we would need additional logic) and also the devs first would need to look up how you allow invalid SSL certs in the first place.

The code needed e.g. "[protectionSpace.authenticationMethod isEqualToString:NSURLAuthenticationMethodServerTrust];" is not obvious at all.

@kcharwood
Copy link
Contributor

Would it make sense to have a BOOL property like ASI had, that encapsulated the logic needed to allow invalid SSL certs, while also still potentially exposing blocks for people to do more advanced things with the call backs? It could be modeled after the AFReachabilityCallback block which does some things automatically for the user before calling the user reachability block. There we could check to see if allowsInvalidCert is set to YES, automatically handle the code required for that situation, before passing on to the block. The allowsInvalidCert property probably gets added to AFURLRequestOperation, as well as the AFHTTPClient to allow the HTTPOperationWithMethod: method to set every operation that comes through it as well.

@steipete Is the code not even getting compiled into the app a deal breaker for you? I feel like surrounding the code that sets the flag to YES with a #ifdef DEBUG would be sufficient.

@kcharwood
Copy link
Contributor

I took an (untested) pass here at a potential implementation:

kcharwood@cbf81c0

The one issue to look at is the case of when a canAuthenticateProtectionSpace block is set AND the allowsInvalidSSLCertificates is set to YES. Currently the block gets ignored, and the logic used to handle invalid SSL certs is run, but perhaps that is backwards? Any comments on the implementation would be great.

EDIT: Doh realized I misread the docs on canAuthenticateProtectionSpace callback. This checkin should bring that implementation in line with the expected behavior.

kcharwood@7fad11d

@steipete
Copy link
Contributor Author

@kcharwood looks good to me! I don't really care that much if it gets compiled or not - as long as the default is NO it's ok. A property is even more visible to new developers, so +1 on that.

@kcharwood
Copy link
Contributor

Cool. I also couldn't come up with a good name, so maybe that changes as well. Be interested to hear @mattt's thoughts. Would be happy to update that with better names if anyone has options before submitting a pull for it.

@Shukuyen
Copy link

Just a thought on accepting invalid certificates: I think the most clean way (for a production app, not what @steipete wants to do) would be to present a "Trust this server?" dialog to the user. Once the server is trusted this will not appear again, until the certificate changes.

This way you could connect to servers with untrusted certificates (necessary in my app) but also hint the user when something strange is happening.

Is this something that should be considered for AFNetworking or should it be done in each app individually?

@underbjerg
Copy link

+1 for the option to ignore invalid certificates

It would be useful for testing against test-servers with self-signed certificates, and in one of our apps we need it to integrate with some of our customers who (unfortunately) are still using self-signed certificates for a couple of servers.

@kcharwood
Copy link
Contributor

@Shukuyen - I think 'clean' is a relative term. In your approach we'll need to pop a localized Alert View and persist the response locally about a particular certificate. I am guessing that is something that @mattt would probably be against (but I could be wrong!)

Your approach sounds like something that may be best done in the individual application. I think the use case most people have here is in a dev environment with a self signed cert that is not going to be used in production code.

@mattt
Copy link
Contributor

mattt commented Feb 14, 2012

Great contributions, @steipete & @kcharwood! I just pushed 17f5584, which borrows from both approaches to make what I think is a solid feature.

First, this commit adds setAuthenticationAgainstProtectionSpaceBlock:, which much like its authentication challenge block sister, intercepts a default implementation of the corresponding NSURLConnectionDelegate method.

Secondly, this commit also adds _AFNETWORKING_ALLOW_INVALID_SSL_CERTIFICATES_, which if defined, will change the behavior of both authentication challenge delegate methods. The idea is that this could be set either temporarily during development, but not for production apps. It's a feature flag: no code change other than setting or unsetting the flag is needed to turn on or off full SSL certificate support.

Thanks again, everyone!

@mattt mattt closed this Feb 14, 2012
@kcharwood
Copy link
Contributor

@mattt thanks for getting this pulled in!

I had a comment I wanted to make on the implementation. Since _AFNETWORKING_ALLOW_INVALID_SSL_CERTIFICATES_ is really all or nothing, we could potentially open a (admittedly small) corner case scenario. Lets say we have an application that interfaces with two different API's, and hence has two different AFHTTPClient's implemented. One API is already considered final, and perhaps even passes sensitive information back and forth with a valid SSL connection. The other API is early in development, and has an invalid SSL certificate. With the current approach, you must set _AFNETWORKING_ALLOW_INVALID_SSL_CERTIFICATES_ in order for the dev API to work, but now all AFURLConnectionOperation's will accept an invalid SSL, potentially exposing the sensitive information for the first API to be susceptible to a Man-in-the-middle attack.

If we choose to go the property route, we can specify which API should allow invalid SSL certs, while also making sure that other API's correctly fail if they run into an invalid SSL.

One more comment on my approach I made above - I added a property to AFHTTPClient as well since ignoring invalid SSL certs feels like a an action to take on the entire client. I realize that it is trivial to override HTTPRequestOperationWithRequest:success:failure:, but it feels lightweight enough to go ahead and add the property and implement this automatically for the user.

I'll reopen the issue just to get feedback on this comment. If @mattt feels this corner case isn't worth solving, then go ahead and close out again.

EDIT: Doh I don't have permission to reopen :(

@mattt
Copy link
Contributor

mattt commented Feb 27, 2012

@kcharwood I appreciate your concern about this issue.

The compromise of a compiler macro essentially offers two modes: regular mode, and "F it, I just want it to work right now while I'm developing". In the case that someone interacts with two APIs, developing in "F it" mode for both is alright, and each can eventually set their own auth blocks for the real implementation. That is to say, what is accomplished by the macro can be replicated by simply setting the correct auth block.

For the sake of simplicity, I prefer to keep as many properties about operations configured in HTTPRequestOperationWithRequest:success:failure: as possible. Even though SSL cert behavior is a client-wide behavior, a lot of other properties about operations could qualify as well, and I wouldn't want to have the API for AFHTTPClient to balloon to match parity for that.

@BinaryDennis
Copy link

Hi
Im trying to "tell" AFNetworking to ignore ssl warning with no success :(

neither of these defines work for me. What Im I missing? Should these defines be in a certain file?

#define AFNETWORKING_ALLOW_INVALID_SSL_CERTIFICATES
#define AFNETWORKING_ALLOW_INVALID_SSL_CERTIFICATES

Thanks in advance

@aprato
Copy link

aprato commented Oct 16, 2012

@idennis try putting it in your AppName-Prefix.pch like so:

#ifdef DEBUG
#define _AFNETWORKING_ALLOW_INVALID_SSL_CERTIFICATES_
#endif

@BinaryDennis
Copy link

Thanks!

@elsurudo
Copy link

Hmm, not working for me, even when I define the flag in my prefix file. Any other gotchas?

EDIT: For the record, my problem was that I have AFNetworking installed through CocoaPods, so setting the variable in my project's prefix file didn't do the trick. I needed to set it in CocoaPods' prefix file, instead.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
8 participants