Bouke / HAP

Swift implementation of the Homekit Accessory Protocol
https://boukehaarsma.nl/HAP/
MIT License
364 stars 50 forks source link

Replace Libsodium with SwiftCrypto #117

Closed Bouke closed 3 years ago

Bouke commented 3 years ago

Work in progress. Idea is to drop Libsodium completely as a dependency. I find the API documentation hard to work with, it doesn't specify what primitives it uses and leaves a lot of guess work when implementing. Using the CryptoKit API, we can remove various low-level C calls.

Regarding compatibility with older macOS / iOS, we can fork swift-crypto to support that / force it to always use BoringSSL.


Update I cannot build BoringSSL on my Raspberry (ARM). Holding off on merging this.

sonarcloud[bot] commented 3 years ago

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

gbrooker commented 3 years ago

I've been revisiting this, looking through my original pull request and this one. While I still like the approach of putting all the Crypto calls into the Authenticator class in my implementation, I've no logical objections to doing it this way.

Work in progress. Idea is to drop Libsodium completely as a dependency. I find the API documentation hard to work with, it doesn't specify what primitives it uses and leaves a lot of guess work when implementing. Using the CryptoKit API, we can remove various low-level C calls.

Regarding compatibility with older macOS / iOS, we can fork swift-crypto to support that / force it to always use BoringSSL.

Yes that could work. I've just built and ran tests on a BoringSSL backed version on Big Sur (forcing the replacement of CrypoKit) and it seemed to work.

Update I cannot build BoringSSL on my Raspberry (ARM). Holding off on merging this.

While I know the original idea was to drop libsodium universally, if BoringSSL is posing a problem on your Pi, would it be useful to implement a version of Crypto with libsodium shim's, just for the small part of the api that we use in HAP ?

That would allow us to merge this PR into the main branch, use CryptoKit on BigSur/iOS 14, use SwiftCrypto with BoringSSL elsewhere (macOS earlier than Big Sur with ) and still cover platforms where SwiftCrypto does not yet work properly (Raspberry Pi), at least until that is fixed.

gbrooker commented 3 years ago

While I know the original idea was to drop libsodium universally, if BoringSSL is posing a problem on your Pi, would it be useful to implement a version of Crypto with libsodium shim's, just for the small part of the api that we use in HAP ?

That would allow us to merge this PR into the main branch, use CryptoKit on BigSur/iOS 14, use SwiftCrypto with BoringSSL elsewhere (macOS earlier than Big Sur with ) and still cover platforms where SwiftCrypto does not yet work properly (Raspberry Pi), at least until that is fixed.

What are your thoughts on this ?

gbrooker commented 3 years ago

I have added a branch working-crypto to my fork which rebases these changes to the current master

Bouke commented 3 years ago

What are your thoughts on this ?

There's great news regarding this. My Pi is now able to build swift-crypto (Swift 5.4.1 / swift-crypto 1.1.6). I'd like to reduce the amount of custom dependencies where possible, so I'm looking at targeting apple/swift-crypto instead of working off a fork. The reasoning for targeting a newer version is that it reduces maintenance burden on my part. Having to maintain a fork of a crypto-related package is not ideal. This would thus require macOS 11 / Swift 5.3 going forward. What's your thoughts on this?

(Note that I'm not merging this yet; we're now using Github Actions for CI which doesn't provide macOS 11 images yet)

gbrooker commented 3 years ago

That's great news. I'm all for moving this to main. I've actually been using a slight variation on this fork for some time now.

sonarcloud[bot] commented 3 years ago

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication