Skip to content

Conversation

boorad
Copy link
Collaborator

@boorad boorad commented May 8, 2025

Adds xsalsa20 cipher to the library

  • first cipher from libsodium, so there's also scaffolding around using that C++ lib
  • The libsodium Cocoapod is dated, so for now, the podspec has instructions on downloading source and compiling from that vendored folder 🤮
  • encrypt/decrypt test
  • encrypt/decrypt benchmark vs @noble/ciphers/salsa
ios android
cipher_benchmark_ios cipher_benchmark_android

@boorad boorad self-assigned this May 8, 2025
@boorad boorad marked this pull request as ready for review May 13, 2025 13:09
@boorad boorad requested review from mrousavy and renanmav May 13, 2025 18:14
Copy link
Member

@renanmav renanmav left a comment

Choose a reason for hiding this comment

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

@boorad haven't tested this locally, but I plan to before approving.

Just a few minor questions to get me started.

Can I get more context on why libsodium? I was not aware of it before reading this PR.

@boorad
Copy link
Collaborator Author

boorad commented May 13, 2025

Can I get more context on why libsodium? I was not aware of it before reading this PR.

I need to use xsalsa20 algorithm, and it's not in OpenSSL. Plus, libsodium has a more modern approach to cryptography according to their docs and online comments. I want to do a full diff on the two libs and what they provide, but this was the best one to add for xsalsa that also opened up newer algos in the future.

@boorad boorad requested a review from renanmav May 14, 2025 12:56
Copy link
Member

@mrousavy mrousavy left a comment

Choose a reason for hiding this comment

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

Honestly great code, I couldn't find any real problems. Nice work Brad 👍

(keep in mind I only looked at C++ code, no JS)

if (result != 0) {
throw std::runtime_error("XSalsa20Cipher: Failed to update");
}
return std::make_shared<NativeArrayBuffer>(output, native_data->size(), [=]() { delete[] output; });
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return std::make_shared<NativeArrayBuffer>(output, native_data->size(), [=]() { delete[] output; });
return ArrayBuffer::wrap(output, native_data->size(), [=]() { delete[] output; });

use the initializer functions instead of the constructor, kinda more explicit.

* xsalsa20 does not have a final step, returns empty buffer
*/
std::shared_ptr<ArrayBuffer> XSalsa20Cipher::final() {
return std::make_shared<NativeArrayBuffer>(nullptr, 0, nullptr);
Copy link
Member

Choose a reason for hiding this comment

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

Does that deallocate fine? Not sure if this calls delete[] at some point but maybe it's fine.

@boorad boorad merged commit aeb1d45 into main May 14, 2025
7 checks passed
@boorad boorad deleted the feat/xsalsa20 branch May 14, 2025 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants