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

Remove rust_sodium and replace encryptions with AES #255

Merged
merged 2 commits into from Dec 2, 2019

Conversation

@Yoga07
Copy link
Member

Yoga07 commented Nov 27, 2019

Resolves #254

- Aes128 is used along with CBC block cipher mode of operation
@Yoga07 Yoga07 requested review from nbaksalyar, m-cat and lionel1704 Nov 27, 2019
@Yoga07 Yoga07 requested a review from ustulation as a code owner Nov 27, 2019
@@ -8,18 +8,46 @@

// TODO(dirvine) Look at aessafe 256X8 cbc it should be very much faster :01/03/2015

This comment has been minimized.

Copy link
@dirvine

dirvine Nov 27, 2019

Member

We can remove this comment now.

where
E: StorageError,
{
let cipher = Aes128Cbc::new_var(key.0.as_ref(), iv.0.as_ref()).unwrap();

This comment has been minimized.

Copy link
@dirvine

dirvine Nov 27, 2019

Member

We should not unwrap here ? is better

This comment has been minimized.

Copy link
@Yoga07

Yoga07 Nov 27, 2019

Author Member

Ah! Missed it in the rush! Thanks! :)


pub const KEY_SIZE: usize = KEYBYTES;
pub const IV_SIZE: usize = NONCEBYTES;
// Buffer size is set to max chunk size + 100 bytes for padding.

This comment has been minimized.

Copy link
@dirvine

dirvine Nov 27, 2019

Member

Do we need to consider 100 bytes here for padding. It should be no more than 15 bytes I think, but taken care of by the block_modes crate AFAIK.

This comment has been minimized.

Copy link
@madadam

madadam Nov 27, 2019

Contributor

Or use encrypt_vec (and decrypt_vec) and you don't have to worry about the padding at all.

{
let cipher = Aes128Cbc::new_var(key.0.as_ref(), iv.0.as_ref()).unwrap();
let pos = data.len();
let mut buffer = [0u8; BUFFER_SIZE];

This comment has been minimized.

Copy link
@madadam

madadam Nov 27, 2019

Contributor

This can overflow the stack, depending on the platform, compiler, etc... Better to use Vec.

This comment has been minimized.

Copy link
@madadam

madadam Nov 27, 2019

Contributor

...but as I wrote in the other comment, better to use encrypt_vec, then you don't have to worry about any of this.


pub const KEY_SIZE: usize = KEYBYTES;
pub const IV_SIZE: usize = NONCEBYTES;
// Buffer size is set to max chunk size + 100 bytes for padding.

This comment has been minimized.

Copy link
@madadam

madadam Nov 27, 2019

Contributor

Or use encrypt_vec (and decrypt_vec) and you don't have to worry about the padding at all.

Copy link

jeanphilippeD left a comment

Looks reasonable, I think I agree with @madadam 's comments.

tests/lib.rs Show resolved Hide resolved
@Yoga07 Yoga07 requested review from madadam and jeanphilippeD Nov 27, 2019
.map_err(|_| SelfEncryptionError::Encryption)
let cipher = Aes128Cbc::new_var(key.0.as_ref(), iv.0.as_ref())
.map_err(|e| SelfEncryptionError::Cipher(format!("{:?}", e)))?;
Ok(cipher.encrypt_vec(data))

This comment has been minimized.

Copy link
@dirvine

dirvine Nov 27, 2019

Member

Much nicer

@lionel1704 lionel1704 merged commit 761d790 into maidsafe:master Dec 2, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.