Cryptokit

[gbrooker]

This commit drops the requirement for libsodium on platforms where CryptoKit is available (macOS 10.15, iOS 13, *). If For compatibility with earlier versions of macOS or iOS, libsodium is still linked on intel platforms, but it is entirely removed as a dependency on App Silicon.

All of the sodium or SRP dependant code has been moved to the security classes. HKDF would have been replaced too, except the method used in HAP to pass a shared secret from pairVerify() to the Cryptographer requires it to be encoded and passed through a text header. Unfortunately the CryptoKit SharedSecret can't be encoded and recreated without the privateKey, so I had to use the existing HKDF function instead. If there is a way to pass the SharedSecret object directly to the cryptographer without using text coded headers, that limitation can be removed.

(I fixed the commit history issue with a rebase)

[gbrooker]

OK cleared all the travis build checks

[Bouke]

Dropping the libsodium package would be great, and only using CryptoKit would be ideal. However instead of dropping libsodium, this PR is only dropping it partially as CryptoKit is not available on Linux. So this means that the codebase becomes substantially more complex (diff shows 1000 lines added, 200 removed) and thus more difficult to maintain. This PR introduces 14x "#if os() / canImport()" and 11x "if #available". That's quite a lot of different branches and combinations to support and as a result the code becomes hard to read. I think a good middle ground would be to add SwiftCrypto as a dependency: on macOS it'll forward to CryptoKit and on Linux it'll use BoringSSL (vendored).

[gbrooker]

Dropping the libsodium package would be great, and only using CryptoKit would be ideal. However instead of dropping libsodium, this PR is only dropping it partially as CryptoKit is not available on Linux. So this means that the codebase becomes substantially more complex (diff shows 1000 lines added, 200 removed) and thus more difficult to maintain. This PR introduces 14x "#if os() / canImport()" and 11x "if #available". That's quite a lot of different branches and combinations to support and as a result the code becomes hard to read. I think a good middle ground would be to add SwiftCrypto as a dependency: on macOS it'll forward to CryptoKit and on Linux it'll use BoringSSL (vendored).

SwiftCrypto is unfortunately only half-baked. It provides a CryptoKit like API to linux, and but it requires CryptoKit on macOS/iOS, meaning it is not compatible with macOS before 10.15. HAP is compatible with macOS 10.13 and 10.14 (High Sierra and Mojave), but SwiftCrypto is not.

This PR is not only compatible with linux and macOS 10.15+ (Catalina, Big Sur), but also with earlier macOS versions. On macOS ARM, it will build with only CryptoKit. i.e these three build scenarios are available

• On linux - only libsodium
• On macOS (intel) - when available CryptoKit, otherwise libsodium
• On macOS (arm) - only CryptoKit

The #if's are required to support multiplatform compatibility, whether they are in HAP or in a third party library like SwiftCrypto. While the count may be high, the use follows a similar template for each function.

One goal of this PR was to reduce the surface area of the external crypto library from around 6 classes to only 3 classes (well two classes and an enum), concentrated in the Security folder.

An alternative organisation which may be easier to read would be to merge those three objects into a single HAPCrypto enum and split the implementation of HAPCrypto over multiple files, with each file providing the implementation in the three bullets mentioned above. Let me know if you'd prefer me to re-organise along those lines.

[Bouke]

meaning it is not compatible with macOS before 10.15.

Is compatibility with macOS <10.15 important to you?

[gbrooker]

Yes it is, I need support back to 10.13

Sent from my iPhone

On 17 Aug 2020, at 2:35 PM, Bouke Haarsma notifications@github.com wrote:

 meaning it is not compatible with macOS before 10.15.

Is compatibility with macOS <10.15 important to you?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[Bouke]

In https://github.com/Bouke/HAP/pull/114 I've started experimenting using swift-crypto on supported macOS versions, and providing an implementation of the same Crypto API using Libsodium for older macOS versions / Linux. I've migrated Ed25519 (Curve25519) over, and that seems to be working great. Doing the same for ChaCha20Poly1305 and the CLibSodium dependency can be dropped from the HAP target.

I'm thinking about dropping the HKDF dependency and inline it, as the implementation is rather straight-forward and doesn't really need a module. However, that's something for another time.

[gbrooker]

That seems even more complicated that what I had developed here. So it would use CryptoKit on macOS 10.15+, libsodium on 10.14- and boringssl on linux (via SwiftCrypto) ?

The footprint that HAP needs is pretty small, so doesn't need the full capability of swift-crypto, and I would suggest it would be preferable to avoid adding yet another external library dependancy (swift-crypto + boringssl).

The only stumbling block I ran into for replacing HKDF with CryptoKit's version was as I mentioned at the top, the way a shared secret is passed between stages of the NIO pipeline in HAP, which currently requires it to be serialised in a base64 string to pass through a pseudo HTTP header. To recreate a CtryptoKit SharedSecret on the otherside, it also needs the private key to be transmitted. That could be added to the encoding, however a preferable solution would be to pass the object directly rather than serialising/deserialising in the same app...

[Bouke]

That seems even more complicated that what I had developed here. So it would use CryptoKit on macOS 10.15+, libsodium on 10.14- and boringssl on linux (via SwiftCrypto) ?

This is not the case, as I've made it conditional on canImport. So it'll only use CryptoKit / Libsodium, and not BoringSSL:

• macOS 10.15 and after: CryptoKit through SwiftCrypto
• macOS 10.14 and before: Libsodium (no SwiftCrypto)
• Linux: Libsodium (no SwiftCrypto)

[gbrooker]

... it'll only use CryptoKit / Libsodium, and not BoringSSL:

• macOS 10.15 and after: CryptoKit through SwiftCrypto
• macOS 10.14 and before: Libsodium (no SwiftCrypto)
• Linux: Libsodium (no SwiftCrypto)

I see, but what advantage does SwiftCrypto bring ? This PR does all of the above already, but doesn't introduce another package dependancy.

[gbrooker]

I've just posted an alternative organisation of the same functions, grouping all the cryptographic functions into one protocol HAPCryptoProtocol, implemented by one enum HAPCrypto. Two supporting files provide extensions for the CryptoKit and libsodium implementations.

Really no need to introduce SwiftCrypto.

Any thoughts on how to pass a shared secret between stages of the NIO pipeline directly, without encoding and decoding through a pseudo HTTP header ?

[Bouke]

Any thoughts on how to pass a shared secret between stages of the NIO pipeline directly, without encoding and decoding through a pseudo HTTP header ?

NIO has the notion of "events" which we're already using for setting the Cryptographer. We might be able to do this through the RequestContext that is passed to the Responder. However it needs to send the event after the body has been sent: encryption is started after verification has completed. This breaks the abstraction, but it was leaky already. So if we can come up with a proper design for this, we can stop abusing the HTTP header for this.

Really no need to introduce SwiftCrypto.

Working with Libsodium has been a mixed bag for me recently. I can't seem to understand what primitives it is using by reading the documentation. So there's a lot of guesswork involved and manually comparing seeds / byte arrays trying to find where I messed up. That is no fun pastime for me.

I'm also not interested in maintaining crypto related code, I'd rather offload those mental models to other libraries. I think the CryptoKit API is fine for this, and as an added benefit we can drop some other dependencies (HKDF, Cryptor). SwiftCrypto doesn't need a system library as it ships with BoringSSL, making installation of this package even simpler.

We probably need to fork SwiftCrypto to allow using it on macOS < 10.15 as you might that for your users. See #116 where I've done most of the migration to SwiftCrypto. Cryptor hasn't been dropped yet, as its being used in SRP. I intend to change SRP over to SwiftCrypto as well.

[Bouke]

Unfortunately the CryptoKit SharedSecret can't be encoded and recreated without the privateKey, so I had to use the existing HKDF function instead. If there is a way to pass the SharedSecret object directly to the cryptographer without using text coded headers, that limitation can be removed.

A SymmetricKey can be recreated, then you can use the HKDF<SHA512>.deriveKey directly. This API is however only available from Big Sur.

[gbrooker]

Unfortunately the CryptoKit SharedSecret can't be encoded and recreated without the privateKey, so I had to use the existing HKDF function instead. If there is a way to pass the SharedSecret object directly to the cryptographer without using text coded headers, that limitation can be removed.

A SymmetricKey can be recreated, then you can use the HKDF<SHA512>.deriveKey directly. This API is however only available from Big Sur.

No need for macOS 11, we can just recreate a shared secret from the private key and client public key, and encode both in the headers. See last commit for a small mod which which works on both CryptoKit and libsodium. It would be preferable to pass the object directly though if possible, rather than via this encoding scheme.

[gbrooker]

Really no need to introduce SwiftCrypto.

Working with Libsodium has been a mixed bag for me recently. I can't seem to understand what primitives it is using by reading the documentation. So there's a lot of guesswork involved and manually comparing seeds / byte arrays trying to find where I messed up. That is no fun pastime for me.

I know where you are coming from, it took me ages to work back through the libsodium implementation to get the CryptoKit equivalent. They are both working smoothly and interoperable now, so job done !

I'm also not interested in maintaining crypto related code, I'd rather offload those mental models to other libraries. I think the CryptoKit API is fine for this, and as an added benefit we can drop some other dependencies (HKDF, Cryptor). SwiftCrypto doesn't need a system library as it ships with BoringSSL, making installation of this package even simpler.

We probably need to fork SwiftCrypto to allow using it on macOS < 10.15 as you might that for your users. See #116 where I've done most of the migration to SwiftCrypto.

I still don't see the benefit, as I understand you are porting libsodium under SwiftCrypto, so you'll still have it to maintain, perhaps even more so if SwiftCrypto evolves which require changes. This PR avoids adding the additional layer of complexity of adding a dependancy on SwiftCrypto. The calls to sodium or CrypoKit are quite limited in scope, and are well debugged and tested.

Cryptor hasn't been dropped yet, as its being used in SRP. I intend to change SRP over to SwiftCrypto as well.

I went down that route, but ran into a roadblock implementing the verifySession method of SRP to verify the keys. I didn't work out how to do that in CryptoKit without going back to the same BigInt math as used by SRP. In the end I backed out my changes as they were not adding any benefit and just left SRP and Cryptor in the build because they didn't need libsodium.

[Bouke]

I know where you are coming from, it took me ages to work back through the libsodium implementation to get the CryptoKit equivalent. They are both working smoothly and interoperable now, so job done !

Yes job well done indeed, I was able to reuse most of your CryptoKit usage in my branch. I've referred to your input in the commit message.

I still don't see the benefit, as I understand you are porting libsodium under SwiftCrypto, so you'll still have it to maintain, perhaps even more so if SwiftCrypto evolves which require changes.

That was my original plan (#114), but that is indeed a big undertaking with little gains. In #116 I've now completely removed both Libsodium and BlueCryptor (and traversely openssl) as dependencies from HAP and SRP. I've patched SwiftCrypto to always use BoringSSL as you've also found that dynamically switching between CryptoKit and some fallback (Libsodium / BoringSSL) is quite a hassle.

I'm quite happy with how #116 is coming along. We're removing most (if not all) pointer handling in crypto functions and using a suitable API (CryptoKit) as replacement. Also as Libsodium is no longer needed, installation instructions are way simpler.

I didn't work out how to do that in CryptoKit without going back to the same BigInt math as used by SRP.

I've kept BigInt in place. Hopefully Swift will provide some Big Number in the standard lib. For now I think it's fine to keep that dependency in place.

[gbrooker]

I still don't see the benefit, as I understand you are porting libsodium under SwiftCrypto, so you'll still have it to maintain, perhaps even more so if SwiftCrypto evolves which require changes.

That was my original plan (#114), but that is indeed a big undertaking with little gains. In #116 I've now completely removed both Libsodium and BlueCryptor (and traversely openssl) as dependencies from HAP and SRP. I've patched SwiftCrypto to always use BoringSSL as you've also found that dynamically switching between CryptoKit and some fallback (Libsodium / BoringSSL) is quite a hassle.

I'm quite happy with how #116 is coming along. We're removing most (if not all) pointer handling in crypto functions and using a suitable API (CryptoKit) as replacement. Also as Libsodium is no longer needed, installation instructions are way simpler.

OK so in #116 libsodium support is dropped completely, but it looks like the dependancy is now on your own fork of swift-crypto. Is that not introducing a new hassle ?

If I understand the changes in the swift-crypto fork correctly, it will use BoringSSL on all platforms, even if CryptoKit is available, is that correct ?

Which versions of macOS have you been able to test it on ? You might want to pull over the CryptoKitTests.swift from this PR, which tests each of the individual CryptoKit functions we use with some pre-validated data. It'll need a few minor mods.

In #116 I see there are still 10 swift files importing Crypto. One of my goals in this PR was to reduce significantly the number of classes dependant on the crypto library. If you really don't like my PR, then can I suggest we make some attempt to concentrate the use of the external Crypto library to just a few classes ?

I would really like to drop as many external dependancies as possible, and therefore I would suggest that we use SwiftCrypto only on macOS Intel and linux. We can replace import Crypto with import CryptoKit with #if (os(macOS) || os(iOS) || os(tvOS) || os(watchOS)) && arch(arm64), so iOS and macOS arm builds don't need it. The legacy support is important for macOS for such a server library. Not important for iOS or macOS on arm.

I didn't work out how to do that in CryptoKit without going back to the same BigInt math as used by SRP.

I've kept BigInt in place. Hopefully Swift will provide some Big Number in the standard lib. For now I think it's fine to keep that dependency in place.

OK, yes that's another solution, it's good to drop Cryptor.

[Bouke]

but it looks like the dependancy is now on your own fork of swift-crypto. Is that not introducing a new hassle ?

Well yes. Sadly they decided to not support anything before macOS 10.15. The patch is [rather simple](https://github.com/apple/swift-crypto/compare/master...Bouke:master], but it is an annoyance that requires maintenance. Ideally we find some way of making this work without a patch, but that would require upstream changes. Currently supporting previous versions is a non-goal, so I don't expect any changes anytime soon. Also note that for HKDF macOS 11 (beta) is required. For now; I think we can just use the fork. It's probably less work to maintain the fork than to keep using libsodium.

Which versions of macOS have you been able to test it on ?

I only have access to macOS 10.15.6. Travis runs the tests against macOS 10.14.

You might want to pull over the CryptoKitTests.swift from this PR, which tests each of the individual CryptoKit functions we use with some pre-validated data. It'll need a few minor mods.

Thanks, I'll look into that.

One of my goals in this PR was to reduce significantly the number of classes dependant on the crypto library. If you really don't like my PR, then can I suggest we make some attempt to concentrate the use of the external Crypto library to just a few classes ?

The classes / functions currently importing Crypto are also very much involved with crypto (either doing SRP, key-exchange, signature validation). I have my concerns about readability when stripping that away. We could shift some responsibility to the responders, but I feel the controllers should remain in control over crypto. Having said that, the readability of the controllers could use an upgrade, so that is something that we might want to address.

I would really like to drop as many external dependancies as possible, and therefore I would suggest that we use SwiftCrypto only on macOS Intel and linux.

I agree -- however we need to have proper CI (testing: Github actions / Travis) of directly relying on CryptoKit as well. Neither Github Actions nor Travis currently have images with macOS 11 betas. So I suggest we put that on hold after such images become available.

[gbrooker]

OK I'll try out #116 on macOS 11 DTK.

Also note that for HKDF macOS 11 (beta) is required.

I was able to get HKDF working in 10.15 CryptoKit, so I'll take a look at that too.

the readability of the controllers could use an upgrade,

Did you see the Authenticator I added in #111 ? It strips out the cryptographic parts of the controller, by replacing the embedded Session, with the key steps in individual functions.

[gbrooker]

Closed in favour of #116