Feature request: let client compare self-signed server cert against local embedded version #81

kkieffer · 2019-08-17T17:05:17Z

Added an optional URL with a path to a local DER-encocded certificate. When allowing self-signed certificates, the embedded certificate is loaded as an Anchor Certificate and validated using SecTrustEvaluateWithError()

Description

For the user, the only interface change is an optional URL in the Configuration init() method. If not set, the previous behavior is maintained. When the URL is set to a file, and the user has allowed self-signed certificates and is a client, then during the SSLHandshake the server presented trust is validated against the embedded certificate. The certificate must be a DER encoded X.509 certificate.

Motivation and Context

I am using BlueSSLService on iOS to connect securely to a server that uses its own root certificate, but I would like to validate the certificate. Previously, you can only accept any server supplied certificate, or the server certificate must be trusted by iOS. (There's no way to programmatically add the certificate to iOS keychain, you must go through a process to have the user install from a browser). If I have the public certificate of the server embedded in the app, I can use this feature to validate it.

How Has This Been Tested?

This is an iOS feature only - not implemented on Linux. I verified that when the URL is set to nil, the behavior is unchanged - I can successfully connect to my server when self-signed is true, and I get an errSSLXCertChainInvalid error when self-signed is false. If I embed by server certificate in the app and load it as follows

let path = Bundle.main.url(forResource: "cert", withExtension: ".der")
Then it successfully establishes the SSL connection. If I use a different (dummy) certificate, I again get the errSSLXCertChainInvalid error.

Note that in order to compile I needed to use

`swift build -Xswiftc "-target" -Xswiftc "x86_64-apple-macosx10.14"

because SecTrustEvaluateWithError is a 10.14 feature.

Checklist:

I have submitted a CLA form
If applicable, I have updated the documentation accordingly.
If applicable, I have added tests to cover my changes.

… check the server certificate using an embedded local certificate

CLAassistant · 2019-08-17T17:05:33Z

All committers have signed the CLA.

billabt · 2019-08-27T14:40:55Z

This can't be merged until you resolve the following issue that is causing a CI failure...

>> Building Swift package...
Fetching https://github.com/IBM-Swift/BlueSocket.git
Cloning https://github.com/IBM-Swift/BlueSocket.git
Resolving https://github.com/IBM-Swift/BlueSocket.git at 1.0.48
Compile Swift Module 'Socket' (3 sources)
Compile Swift Module 'SSLService' (2 sources)
/Users/travis/build/IBM-Swift/BlueSSLService/Sources/SSLService/SSLService.swift:1261:24: error: use of unresolved identifier 'SecTrustEvaluateWithError'
                    if SecTrustEvaluateWithError(trust, nil) == false {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~
error: terminated(1): /Applications/Xcode-.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift-build-tool -f /Users/travis/build/IBM-Swift/BlueSSLService/.build/debug.yaml main
The command "./Package-Builder/build-package.sh -projectDir $TRAVIS_BUILD_DIR" exited with 1.

The API, SecTrustEvaluateWithError is not available on all versions of OS that are currently supported by BlueSSLService. In order to accept this PR, you'll have to make changes so that the applicable code is only built when building on macOS 10.14 or higher. This can be accomplished using the if #available() notation. This link goes to the documentation for the #available notation.

You have to be able to build without any additional command line parameters.

Thanks for the contribution.

…heck

kkieffer · 2019-08-27T17:09:43Z

Thanks. Looks like it's passing CI now.

billabt · 2019-08-29T22:18:45Z

Any chance you can add a test using that case to SSLServiceTests.swift? That'd be extremely helpful. Thanks.

…client connection. Added test for embedded certificate on OSX. Adding cert.der (converted from existing cert.pem) to use as embedded server cert.

kkieffer · 2019-09-02T14:41:51Z

I added test test case to the SSLServiceTests.swift, but I noticed that the SSL option was turned off for Apple platforms. I turned it back on. All tests are working correctly for me building locally on OSX with SSL, but the Travis CI seems to hang when the client connects. Can you help? Here's the last output before hang...

Test Case '-[SSLServiceTests.SSLServiceTests testSecureReadWriteEmbedded]' started.
Listening on port: 1337
Using embedded cert: file:///Users/travis/build/IBM-Swift/BlueSSLService/Tests/SSLServiceTests/certs/cert.der
Attempting to connect to host: 127.0.0.1:1337...

…st will pass Travis CI.

kkieffer · 2019-09-10T14:49:13Z

I changed the test order so the original test could be executed first, but it still hangs on connect. I suspect this might be an issue with the Travis CI environment. All tests pass for me locally on OSX 10.14.6.

Merge remote-tracking branch 'upstream/master' into embeddedServerCert

billabt · 2019-09-10T17:06:08Z

@djones: Any insight on the CI failure? Thanks.

kkieffer · 2019-09-12T13:43:53Z

For what it's worth here's the output of my local test:

billabt · 2019-09-12T14:53:37Z

Thanks. The last time we had this type of failure it turned out to be a real bug that only seemed to show up in CI. That’s why I asked @djones to take a look. He’s our CI guru. 😉

kkieffer · 2019-09-12T14:58:20Z

Well in the master source, the MacOS tests were turned off - you modified in Dec 2018. So it was never being tested...

billabt · 2019-09-12T15:02:34Z

I’ll look into it more.

billabt · 2019-09-16T15:22:15Z

Disable SSL when testing on macOS so that it passes CI. This is a definitely a CI problem. After I merge, I'll test locally on macOS to ensure it passes the new test before doing the release. Thanks for your patience...

djones · 2019-09-16T19:14:17Z

@billabt just FYI you have the wrong @djones here. I'm not part of this project.

kkieffer · 2019-09-16T19:15:55Z

Done. Passing CI now.

billabt · 2019-09-18T13:32:48Z

@djones6: Can you take a look at the CI failure when we turn on testing for macOS? It passes locally but I want to make sure we’re dealing with a CI issue before I merge the changes. Thanks.

djones6 · 2019-09-27T15:30:25Z

@billabt I'm afraid I am by no means an expert on this, but my guess would be that the hang is to do with unlocking the keychain. I have successfully used a trick in the past in a headless environment (not Travis) which creates a new keychain (and unlocks it) prior to running the tests:
https://github.com/IBM-Swift/Kitura-Benchmarks/blob/master/Bench-Kitura-Core/scripts/ssl_setup.sh

You could add a script like this as osx/before_tests.sh which Package-Builder will then execute just before it runs swift test.

I'd be somewhat surprised however if this solves this issue, because we're successfully running Kitura's CI with SSL enabled in Travis without any such tricks. Worth a try though.

kkieffer · 2019-09-28T01:15:46Z

It looks like the statements in @djones6 ssl_setup.sh script are already in the osx/before_tests.sh script. Is it possible Travis CI isn't running this script?

djones6 · 2019-10-02T09:26:34Z

The Travis log shows that the script is running, and doesn't indicate there being any problems with it:

>> Testing Swift package...
security: SecKeychainDelete: The specified keychain could not be found.
Keychain SSLService.keychain does not exist.
SSLService.keychain created...
Keychain unlocked, ready to set timeout...
Keychain timeout set. Ready for import...
Keychain "SSLService.keychain" timeout=3600s
1 identity imported.
Import complete.  Ready for testing...
>> Completed osx pre-tests steps.

If this doesn't work, then I'm afraid I don't know.

For the sake of this PR, the CI is sufficient to demonstrate that it builds successfully in each of the environments. If we can also demonstrate that local testing of the feature is successful, I think that'll have to be good enough.

billabt · 2019-10-02T13:39:59Z

I agree. I'll run the checks here to confirm.

kkieffer · 2019-10-15T18:28:11Z

Any issues running the checks? Let me know if I can help.

billabt · 2019-11-04T20:43:22Z

@kkieffer Sorry I haven't merged this yet. Spent a bit of time in the hospital and then a bit of recovery time. I should be able to wrap it up this week. Again, sorry...

kkieffer · 2019-11-05T19:53:01Z

No worries, sorry to hear you were in the hospital.

kkieffer · 2019-11-22T20:28:42Z

Any updates?

billabt · 2019-11-23T19:30:22Z

Sorry this took so long. Ran into a problem when publishing on cocoa pods. Had to change the if available statement to include tvOS 12.0 as well. Also had to make changes to the Xcode project file (for those that use it) so that it can run the test when enabled.

It's now released as Version 1.0.52 on both Github and Cocoapods.

kkieffer · 2019-11-23T22:07:29Z

Awesome - thank you!

Adding feature when using self signed certificates to allow client to…

2c5e6e7

… check the server certificate using an embedded local certificate

kkieffer added 2 commits August 27, 2019 11:28

Added #available for MacOS 10.14, iOS 12.0 for embedded certificate c…

64cd0d2

…heck

Support embedded cert only in Swift >4.2

150fadd

kkieffer added 3 commits September 2, 2019 09:50

Adding ability to use multiple embedded certificates

8b6785e

Enabling SSL for OSX tests, previous configuration did not work with …

df7a411

…client connection. Added test for embedded certificate on OSX. Adding cert.der (converted from existing cert.pem) to use as embedded server cert.

Adding printout when attempting to connect to server

b8eb3a2

Changed order of tests using A, B in test names to see if original te…

09a37f8

…st will pass Travis CI.

Updating to upstream changes

b33c86a

Merge remote-tracking branch 'upstream/master' into embeddedServerCert

kkieffer added 2 commits September 16, 2019 14:29

Turning off SSL for MacOS because of TravisCI issue.

45e11e6

Fixing test names for Linux.

5370e73

billabt merged commit eea45c3 into Kitura:master Nov 23, 2019

kkieffer mentioned this pull request Nov 23, 2019

Feature Request: Allow SSL Authentication with Postgres Server, optionally using embedded certificates codewinsdotcom/PostgresClientKit#17

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature request: let client compare self-signed server cert against local embedded version #81

Feature request: let client compare self-signed server cert against local embedded version #81

kkieffer commented Aug 17, 2019

CLAassistant commented Aug 17, 2019 •

edited

Loading

billabt commented Aug 27, 2019

kkieffer commented Aug 27, 2019

billabt commented Aug 29, 2019 •

edited

Loading

kkieffer commented Sep 2, 2019

kkieffer commented Sep 10, 2019

billabt commented Sep 10, 2019

kkieffer commented Sep 12, 2019

billabt commented Sep 12, 2019

kkieffer commented Sep 12, 2019

billabt commented Sep 12, 2019

billabt commented Sep 16, 2019

djones commented Sep 16, 2019

kkieffer commented Sep 16, 2019

billabt commented Sep 18, 2019

djones6 commented Sep 27, 2019

kkieffer commented Sep 28, 2019

djones6 commented Oct 2, 2019

billabt commented Oct 2, 2019

kkieffer commented Oct 15, 2019

billabt commented Nov 4, 2019

kkieffer commented Nov 5, 2019

kkieffer commented Nov 22, 2019

billabt commented Nov 23, 2019

kkieffer commented Nov 23, 2019

Feature request: let client compare self-signed server cert against local embedded version #81

Feature request: let client compare self-signed server cert against local embedded version #81

Conversation

kkieffer commented Aug 17, 2019

Description

Motivation and Context

How Has This Been Tested?

Checklist:

CLAassistant commented Aug 17, 2019 • edited Loading

billabt commented Aug 27, 2019

kkieffer commented Aug 27, 2019

billabt commented Aug 29, 2019 • edited Loading

kkieffer commented Sep 2, 2019

kkieffer commented Sep 10, 2019

billabt commented Sep 10, 2019

kkieffer commented Sep 12, 2019

billabt commented Sep 12, 2019

kkieffer commented Sep 12, 2019

billabt commented Sep 12, 2019

billabt commented Sep 16, 2019

djones commented Sep 16, 2019

kkieffer commented Sep 16, 2019

billabt commented Sep 18, 2019

djones6 commented Sep 27, 2019

kkieffer commented Sep 28, 2019

djones6 commented Oct 2, 2019

billabt commented Oct 2, 2019

kkieffer commented Oct 15, 2019

billabt commented Nov 4, 2019

kkieffer commented Nov 5, 2019

kkieffer commented Nov 22, 2019

billabt commented Nov 23, 2019

kkieffer commented Nov 23, 2019

CLAassistant commented Aug 17, 2019 •

edited

Loading

billabt commented Aug 29, 2019 •

edited

Loading