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

ChaCha20 CTR Encryption #169

Merged
merged 22 commits into from
Apr 18, 2023
Merged

ChaCha20 CTR Encryption #169

merged 22 commits into from
Apr 18, 2023

Conversation

btoms20
Copy link
Contributor

@btoms20 btoms20 commented Apr 11, 2023

Added a method to interact with BoringSSL's CCryptoBoringSSL_CRYPTO_chacha_20()

Checklist

  • I've run tests to see all new and existing tests pass
  • I've followed the code style of the rest of the project
  • I've read the Contribution Guidelines
  • I've updated the documentation if necessary

Motivation:

Having the ability to explicitly set the Counter in the ChaCha20 Cipher allows us to generate 'Header Protection Masks' as described in RFC 9001 - Using TLS to Secure QUIC and issue #168

Modifications:

Added a single encryption method under the Insecure.ChaCha20CTR enum that lets the user of this library perform single block encryptions with an explicitly defined Counter and Nonce / IV.

The ChaCha20CTR enum described above is defined in the _CryptoExtras product.

Result:

After including both Crypto and _CryptoExtras in your project you'll have access to the ChaCha20CTR enum and the associated encryption method that enables one off encryption operations.

import Crypto
import _CryptoExtras

Insecure.ChaCha20CTR.encrypt(
   message: DataProtocol,
   using: SymmetricKey,
   counter: Insecure.ChaCha20CTR.Counter,
   nonce: Insecure.ChaCha20CTR.Nonce
) throws -> Data { ... }

See the included ChaCha20CTRTests.swift file for usage examples.

Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I've left some notes in the diff.

@btoms20 btoms20 requested a review from Lukasa April 12, 2023 20:55
…e the withContiguousStorageIfAvailable method. If our DataProtocol is contiguous we encrypt directly, otherwise we consolidate before encrypting. Removed inLen param from chacha20CTR call.
@btoms20 btoms20 requested a review from Lukasa April 13, 2023 21:20
Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, this is getting really close to ready to land. I've left a few more notes, but they're getting very trivial now.

Sources/_CryptoExtras/ChaCha20CTR/ChaCha20CTR.swift Outdated Show resolved Hide resolved
Sources/_CryptoExtras/ChaCha20CTR/ChaCha20CTR.swift Outdated Show resolved Hide resolved
Tests/_CryptoExtrasTests/ChaCha20CTRTests.swift Outdated Show resolved Hide resolved
Tests/_CryptoExtrasTests/ChaCha20CTRTests.swift Outdated Show resolved Hide resolved
@Lukasa
Copy link
Collaborator

Lukasa commented Apr 14, 2023

@swift-server-bot test this please

@Lukasa
Copy link
Collaborator

Lukasa commented Apr 17, 2023

@swift-server-bot test this please

@Lukasa
Copy link
Collaborator

Lukasa commented Apr 17, 2023

Excellent, one quick fixup: can you run scripts/update_cmakelists.sh and commit the changes?

@Lukasa
Copy link
Collaborator

Lukasa commented Apr 17, 2023

@swift-server-bot test this please

@btoms20
Copy link
Contributor Author

btoms20 commented Apr 17, 2023

Awesome! Thanks for guiding me through this PR, I really appreciate all the comments / help. Sorry for taking up so much of your time! 😅

@Lukasa Lukasa added the 🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0 label Apr 18, 2023
Copy link
Collaborator

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've got nothing to apologise for! This has been a great patch, and there's nothing wrong with going through a few review cycles to get to a final state we're all happy with. Thanks for doing the work!

@Lukasa Lukasa merged commit 65f8c60 into apple:main Apr 18, 2023
@btoms20 btoms20 deleted the feature/ChaCha20+Counter branch May 1, 2023 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔼 needs-minor-version-bump For PRs that when merged cause a bump of the minor version, ie. 1.x.0 -> 1.(x+1).0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants