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

Verifying self-signed certs and hostnames #408

Closed
eliburke opened this issue Oct 16, 2017 · 3 comments
Closed

Verifying self-signed certs and hostnames #408

eliburke opened this issue Oct 16, 2017 · 3 comments

Comments

@eliburke
Copy link

I am using self signed certs. I've been setting disableSSLCertValidation, per PR #41. It was recently brought to my attention that the hostname was not being validated, so it is possible to use a known cert with any hostname or IP address. This isn't catastrophic, but it isn't great either.

The problem appears to lie with FoundationStream.sslTrust() method. This line is always returning nil:
let domain = outputStream!.property(forKey: kCFStreamSSLPeerName as Stream.PropertyKey) as? String
I have tried querying both the input and output stream. I have tried with disableSSLCertValidation true and false (I know the stock code NULLs it out in one of these cases, keep reading 😄 ). It never returns a valid domain for the stream. The one thing I have not tried is a connection to a site with a valid system-verifiable certificate. Can anyone confirm that it DOES work for this scenario or is it entirely broken? Apple's docs have conflicting statements, so I'm not clear on whether it can be used to retrieve the domain name or not.

Also as part of PR #41, the peer name is being set to NULL when validation is disabled. This is explicitly counter to Apple's suggestion that it simply be overridden to match the desired value: Apple Docs

I've updated SSLSettings() and the connect() call (happy to submit a PR) It allows independent control over the domain vs disabling system SSL checks. The hostname can be set to a known value or it can still be nulled out to disable hostname checking entirely.

            var settings = [NSObject: NSObject]()
            if ssl.disableCertValidation {
                #if os(watchOS) //watchOS us unfortunately is missing the kCFStream properties to make this work
                #else
                settings[kCFStreamSSLValidatesCertificateChain] = NSNumber(value: false)
                #endif
            }
            if ssl.overrideTrustDomain {
                if let domain = ssl.desiredTrustDomain {
                    settings[kCFStreamSSLPeerName] = domain as NSString
                } else {
                    settings[kCFStreamSSLPeerName] = kCFNull
                }
            }
            inStream.setProperty(settings, forKey: kCFStreamPropertySSLSettings as Stream.PropertyKey)
            outStream.setProperty(settings, forKey: kCFStreamPropertySSLSettings as Stream.PropertyKey)
@eliburke
Copy link
Author

Hey @daltoniam I know you don't use the self-signed cert code, but I think there is something going on with WebSocket.sslTrust() even with legit certificates. I am printing out the value of "domain" and it never seems to be set. Not in the simulator, not on device, not for iOS 11 or iOS 10. Is there any chance you can debug a connection and confirm what I am seeing?

@eliburke
Copy link
Author

Ok, I found some docs for another method of getting the peer name from the CFStream. I created a PR so you can take a look: #412

disableCertValidation now ONLY disables the system's cert validation, without clearing the peer's hostname

overrideTrustHostname allows you to set the peer's name to another value (for example, if the host is presenting a certificate for another host, but you still want to trust it), or set it to nil which disables checking.

@daltoniam
Copy link
Owner

3.0.3 has been released with your PR improvement. Thanks again for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants