-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat: DES encryption #101
feat: DES encryption #101
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is beautiful, seriously! Amazing work. The README is so helpful, the code is well structured. I had to try hard to come up with suggestions, but I left a few for you if you want. By no means do you have to take them, I'm okay with merging this as-is.
@@ -0,0 +1,256 @@ | |||
//! Contains implementation of DES encryption | |||
#[doc = include_str!("./README.md")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really nice touch. We should add this everywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added to other modules as well
pub fn encrypt(&self, message: &Block) -> Block { Self::des(message, self.subkeys.iter()) } | ||
|
||
/// Decrypt a ciphertext of size [`Block`] | ||
/// | ||
/// ## Example | ||
/// ```rust | ||
/// use rand::{thread_rng, Rng}; | ||
/// use ronkathon::primitives::symmetric::encryption::des::DES; | ||
/// let mut rng = thread_rng(); | ||
/// let secret_key = rng.gen(); | ||
/// | ||
/// let des = DES::new(secret_key); | ||
/// | ||
/// let message = rng.gen(); | ||
/// let encrypted = des.encrypt(&message); | ||
/// let decrypted = des.decrypt(&encrypted); | ||
/// ``` | ||
pub fn decrypt(&self, ciphertext: &Block) -> Block { | ||
Self::des(ciphertext, self.subkeys.iter().rev()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps these should be part of a trait?
Would be really cool if we could have a proof that decrypt(encrypt(msg)) = msg
, but, alas, we are in Rust land not Lean land.
src/primitives/symmetric/mod.rs
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this module we could add a trait for Encryption
or something. Though if we do that, it's a little odd to have symmetric::encryption
and perhaps we should reverse into encryption::symmetric
and define an encryption trait in encryption
module. Perhaps we ultimately have subtraits for symmetric/asymmetric? Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
totally makes sense. i'll restructure according to this, and think on the encryption trait
@Autoparallel added a wdyt? can it be improved further? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good to me, i like the way this module is shaping up. Nice work!
This PR fixes/closes issue #100
It changes the following: