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

voice: use xsalsa20poly1305 crate for encryption #884

Merged
merged 1 commit into from
Jun 12, 2020
Merged

voice: use xsalsa20poly1305 crate for encryption #884

merged 1 commit into from
Jun 12, 2020

Conversation

tarcieri
Copy link
Contributor

This commit replaces the sodiumoxide crate, a wrapper for the libsodium C library, with the pure Rust xsalsa20poly1305 crate from the RustCrypto project:

https://github.com/RustCrypto/AEADs/tree/master/xsalsa20poly1305

This should reduce compile times as libsodium is a somewhat large C library and you are only using one algorithm from it. The xsalsa20poly1305 crate, on the other hand, is a lightweight pure Rust dependency that implements only the algorithm you are using.

Note that the advertised MSRV of this crate is 1.41+, however earlier today new release of the dependency causing that, generic-array, reduced the MSRV to 1.36.0:

fizyk20/generic-array#102

You can read more about RustCrypto and our recent releases in this post:

https://www.reddit.com/r/rust/comments/h0em5n/ann_new_rustcrypto_releases_aead_blockcipher/

@arqunis arqunis requested a review from FelixMcFelix June 11, 2020 19:33
@arqunis arqunis added enhancement An improvement to Serenity. voice Related to the `voice` module and `serenity_voice_model` crate. labels Jun 11, 2020
Copy link
Member

@FelixMcFelix FelixMcFelix 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 the PR, looks good (and should remove occasional external library issues to boot).

Other than a few nits, it's a welcome change; API mostly matches libsodium (including in-place/detached modes, which we're using in the rewrite, so we can migrate that too). I don't believe this is a breaking change since the crate is entirely internal, and the minimum compiler version is now below current's 1.39. Will test manually in the morning since CI for examples is broken on current.

Cargo.toml Show resolved Hide resolved
src/voice/connection.rs Outdated Show resolved Hide resolved
src/voice/connection.rs Show resolved Hide resolved
This commit replaces the `sodiumoxide` crate, a wrapper for the
libsodium C library, with the pure Rust `xsalsa20poly1305` crate from
the RustCrypto project:

https://github.com/RustCrypto/AEADs/tree/master/xsalsa20poly1305

This should reduce compile times as `libsodium` is a somewhat large
C library and you are only using one algorithm from it. The
`xsalsa20poly1305` crate, on the other hand, is a lightweight pure Rust
dependency that implements only the algorithm you are using.

Note that the advertised MSRV of this crate is 1.41+, however earlier
today new release of the dependency causing that, `generic-array`,
reduced the MSRV to 1.36.0:

fizyk20/generic-array#102

You can read more about RustCrypto and our recent releases in this post:

https://www.reddit.com/r/rust/comments/h0em5n/ann_new_rustcrypto_releases_aead_blockcipher/
@FelixMcFelix
Copy link
Member

Tested locally, runs fine for me. Thanks!

@arqunis arqunis merged commit 3965e00 into serenity-rs:current Jun 12, 2020
arqunis pushed a commit to arqunis/serenity that referenced this pull request Jun 12, 2020
…#884)

This commit replaces the `sodiumoxide` crate, a wrapper for the
libsodium C library, with the pure Rust `xsalsa20poly1305` crate from
the RustCrypto project: 

https://github.com/RustCrypto/AEADs/tree/master/xsalsa20poly1305
@tarcieri tarcieri deleted the use-xsalsa20poly1305-crate branch June 12, 2020 13:38
FelixMcFelix added a commit to FelixMcFelix/serenity that referenced this pull request Jun 12, 2020
This uses the `encrypt/decrypt_in_place` functions exposed by this
library. Migration occurred due to decisions outlined in serenity-rs#884.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement to Serenity. voice Related to the `voice` module and `serenity_voice_model` crate.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants