Skip to content

feat: chacha20 + chacha20-poly1305 + tests #711

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Jun 2, 2025
Merged

feat: chacha20 + chacha20-poly1305 + tests #711

merged 13 commits into from
Jun 2, 2025

Conversation

@renanmav
Copy link
Contributor

@renanmav renanmav commented May 27, 2025 •
edited
Loading

In this PR:

  • Adds specific cipher C++ implementations for ChaCha20 and ChaCha20-Poly1305 algorithms.
  • Adds tests to ChaCha20 and ChaCha20-Poly1305 algorithms.

References:

@renanmav renanmav changed the title test: chacha with and without poly1305 feat: chacha20 + chacha20-poly1305 + tests May 28, 2025
@renanmav renanmav marked this pull request as ready for review May 28, 2025 17:01
@renanmav renanmav requested review from boorad and mrousavy May 28, 2025 17:05
boorad
boorad reviewed Jun 2, 2025
View reviewed changes
Copy link
Collaborator

@boorad boorad left a comment

Choose a reason for hiding this comment

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

changes to tests ❤️

packages/react-native-quick-crypto/cpp/cipher/ChaCha20Cipher.cpp

namespace margelo::nitro::crypto {

void ChaCha20Cipher::init(const std::shared_ptr<ArrayBuffer> cipher_key, const std::shared_ptr<ArrayBuffer> iv) {
Copy link
Collaborator

@boorad boorad Jun 2, 2025

Choose a reason for hiding this comment

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

Is it worth consolidating the similarities in the base class somehow, keeping only the EVP_chacha20() and size checks?

Copy link
Contributor Author

@renanmav renanmav Jun 2, 2025

Choose a reason for hiding this comment

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

@boorad an alternative I see for this is 9175d3e

But to achieve that, other cipher implementations would have to be refactored too, and I think that's out of scope for this PR.

Maybe in a follow up, check #712

@renanmav renanmav requested a review from boorad June 2, 2025 15:05
@renanmav renanmav merged commit 0a88709 into main Jun 2, 2025
7 checks passed
@renanmav renanmav deleted the renan/chacha-test branch June 2, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@boorad boorad boorad approved these changes

@mrousavy mrousavy Awaiting requested review from mrousavy

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@renanmav @boorad