feat(ciphers): AES encryption #102

eightfilms · 2024-06-26T15:58:32Z

Partially addresses #95

It changes the following:

Adds an implementation of AES encryption using the SymmetricEncryption trait
note that decryption remains unimplemented - will add it as a follow up PR after this undergoes review and is merged

TODOs:

doc comments where necessary
richer readme like in other places
cleanup code, debug statements, etc.

eightfilms · 2024-06-28T01:35:22Z

Going to rebase my work based on #101

eightfilms · 2024-06-29T13:26:47Z

Ok, I think this should be ready for a look. Preparing the docs took a bit longer than expected - let me know if anything is unclear

Autoparallel · 2024-06-29T19:25:56Z

Excited to review this. May not be able to until Sunday night or Monday.

src/encryption/symmetric/aes/mod.rs

0xJepsen · 2024-06-30T23:31:25Z

src/encryption/symmetric/aes/README.md

+
+For decryption, we take the inverses of these routines:
+
+TODO


Complete or add more detail.

Ah yeah, was thinking to leave decryption for a followup PR - apologies for not being clear about that, but I could also complete it within the same PR (if that's preferable)

I'm fine either way

Filed an issue for decryption: #119

0xJepsen

This looks great to me! I would maybe add this to the root README

0xJepsen · 2024-06-30T23:33:06Z

src/encryption/symmetric/aes/README.md

+
+The **KeyExpansion** algorithm takes a 128/192/156-bit key and turns it into 11/13/15 round keys respectively of 16 bytes each. The main trick to key expansion is the fact that if 1 bit of the encryption key is changed, it should affect the round keys for several rounds.
+
+Using different keys for each round protects against _[slide attacks]_.


Can we link a resource on slide attacks? Would be nice to write a test for them at some point if it's not too crazy

Yes i 100% agree. An example of this would be amazing!

If you don't do it in this PR (which, by no means, do you have to) can you make an issue?

Filed an issue: #120 (comment)

0xJepsen · 2024-06-30T23:34:39Z

src/encryption/symmetric/aes/README.md

+
+### Decryption
+
+TODO


Either address the todo or elaborate in a comment.

Filed an issue for decryption: #119

0xJepsen · 2024-06-30T23:35:26Z

src/encryption/symmetric/aes/README.md

+the key and the ciphertext as part of the *confusion* property.
+
+During substitution, a byte is interpreted as a polynomial and
+mapped to its multiplicative inverse in [Rijndael's finite field][Rijndael ff]: GF(2^8) = GF(2)[x]/(x^8 + x^4 + x^3 + x + 1).


src/encryption/symmetric/aes/mod.rs

Autoparallel

I'm good to merge this! Would love to see the decryption done :)

Autoparallel · 2024-07-01T02:04:35Z

src/encryption/symmetric/aes/README.md

+
+The **KeyExpansion** algorithm takes a 128/192/156-bit key and turns it into 11/13/15 round keys respectively of 16 bytes each. The main trick to key expansion is the fact that if 1 bit of the encryption key is changed, it should affect the round keys for several rounds.
+
+Using different keys for each round protects against _[slide attacks]_.


Yes i 100% agree. An example of this would be amazing!

If you don't do it in this PR (which, by no means, do you have to) can you make an issue?

src/encryption/symmetric/aes/mod.rs

Autoparallel · 2024-07-01T02:12:22Z

src/encryption/symmetric/aes/mod.rs

+impl<const N: usize> std::ops::Deref for Key<N>
+where [(); N / 8]:
+{
+  type Target = [u8; N / 8];
+
+  fn deref(&self) -> &Self::Target { &self.inner }


Why do we need Deref?

Ah it's really just so we don't have to do key.inner. I think we only use it twice so I can actually remove this impl, it isn't more convenient and adds unnecessary lines

All good! Keep it in if you want, I just wasn't quite sure where it was being used.

lonerapier

Amazing work sir! Just one question, are we adding BlockCipher trait in a followup PR?

lonerapier · 2024-07-01T10:51:59Z

src/encryption/symmetric/aes/mod.rs

+{
+  /// Performs the cipher, with key size of `N` (in bits), as seen in Figure 5 of the document
+  /// linked in the front-page.
+  fn aes_encrypt(plaintext: &[u8; 16], key: &Key<N>, num_rounds: usize) -> Block {


It would be good to have result instead of assert (totally optional)

we can, but imo some of the asserts can/should coexist alongside Result usage, since they're asserting some of the invariants in the procedure (such as round keys being used up by the end of the encryption). Will see how I can refactor to have less unwrap()s.

eightfilms · 2024-07-01T15:50:45Z

Thanks for the reviews! Will implement decryption and an example of a slide attack within this same PR and request again very soon. Alternatively we can merge and leave that for a followup PR (whichever is preferable)

Autoparallel · 2024-07-01T17:49:54Z

@eightfilms I'm game to merge this and then handle a followup PR addressing our comments here.

Can you:

Make issues based off those comments
Resolve conflicts here?

Then I will merge :)

expanded key is not really a good name; since the key expansion actually outputs a vector of `Word`s, where each 4-word chunk is used as a round key.

eightfilms · 2024-07-01T23:53:50Z

Oops, I messed up the commit history (i blame my unfamiliarity with jj), but all I really did was resolve the merge conflict. Sorry for the mess, i'll see if i can fix it

0xJepsen · 2024-07-02T00:34:24Z

Oops, I messed up the commit history (i blame my unfamiliarity with jj), but all I really did was resolve the merge conflict. Sorry for the mess, i'll see if i can fix it

I think it's okay! If thats all thats left we can git this one in

eightfilms · 2024-07-02T00:35:53Z

Yeah sounds good! Thanks @0xJepsen

eightfilms force-pushed the bing/aes-encryption branch 2 times, most recently from d201f85 to 408c607 Compare June 28, 2024 01:31

eightfilms force-pushed the bing/aes-encryption branch 2 times, most recently from b5d929c to b4cefae Compare June 29, 2024 11:53

eightfilms marked this pull request as ready for review June 29, 2024 12:02

0xJepsen requested review from lonerapier, devloper, Autoparallel and 0xJepsen June 29, 2024 15:32

lonerapier mentioned this pull request Jun 30, 2024

feat: stream cipher trait and chacha encryption #103

Merged

eightfilms commented Jun 30, 2024

View reviewed changes

src/encryption/symmetric/aes/mod.rs Show resolved Hide resolved

0xJepsen reviewed Jun 30, 2024

View reviewed changes

0xJepsen approved these changes Jul 1, 2024

View reviewed changes

Autoparallel approved these changes Jul 1, 2024

View reviewed changes

lonerapier approved these changes Jul 1, 2024

View reviewed changes

eightfilms force-pushed the bing/aes-encryption branch from 1a0d696 to d6b7220 Compare July 1, 2024 23:39

eightfilms added 9 commits July 2, 2024 07:40

feat(ciphers): AES encryption

4d7a820

cleanup BlockCipher trait

7ca5c8f

feat: impl for 192-bit and 256-bit keys

99ed9fb

doc: frontpages

8fa05c5

test: remove mix column test

3720215

feat(key): use KEY_LEN_BYTES for assertions

f2d5536

feat(key): impl Deref and use

3d17aa9

remove unused Key128 type

30fb784

define and use ExpandedKey to represent expanded key

f0c09dc

eightfilms added 22 commits July 2, 2024 07:40

docs: add link to source

dbbe0f2

shift around comments

7cd117b

use same fn for all encryption of different bits

b5cbc80

move AES work to encryption crate

d93a822

docs: doc for Word

c7e50b5

cleanup

9665d74

split tests:

126b88c

don't use pretty_assertions

7bb965a

use remainder

103e9f3

docs: README skeleton

8e9e2d5

update example

cdf9ed0

README

add5456

doc: fix links

e4908da

remove ExpandedKey

c54a7c7

clippy

3a5022b

cleanup

1ee1209

SBOX cleanup

ed9d66a

replace unimplemented with more explicit panic

c3b924a

doc comment for key expansion

6469574

fix des link

a59cab2

better mix columns docs

c1d200b

rename expanded key

f0e38d6

expanded key is not really a good name; since the key expansion actually outputs a vector of `Word`s, where each 4-word chunk is used as a round key.

eightfilms force-pushed the bing/aes-encryption branch from d6b7220 to f0e38d6 Compare July 1, 2024 23:42

This was referenced Jul 1, 2024

feat: AES decryption #119

Closed

feat: slide attacks for AES #120

Open

0xJepsen merged commit 55fb8b2 into pluto:main Jul 2, 2024
5 checks passed

eightfilms deleted the bing/aes-encryption branch July 2, 2024 08:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ciphers): AES encryption #102

feat(ciphers): AES encryption #102

eightfilms commented Jun 26, 2024 •

edited

Loading

eightfilms commented Jun 28, 2024

eightfilms commented Jun 29, 2024

Autoparallel commented Jun 29, 2024

0xJepsen Jun 30, 2024

eightfilms Jul 1, 2024

Autoparallel Jul 1, 2024

eightfilms Jul 1, 2024

0xJepsen left a comment

0xJepsen Jun 30, 2024

Autoparallel Jul 1, 2024

eightfilms Jul 1, 2024

0xJepsen Jun 30, 2024

eightfilms Jul 1, 2024

0xJepsen Jun 30, 2024

Autoparallel left a comment

Autoparallel Jul 1, 2024

Autoparallel Jul 1, 2024

eightfilms Jul 1, 2024

Autoparallel Jul 1, 2024

lonerapier left a comment

lonerapier Jul 1, 2024

eightfilms Jul 1, 2024

eightfilms commented Jul 1, 2024

Autoparallel commented Jul 1, 2024

eightfilms commented Jul 1, 2024 •

edited

Loading

0xJepsen commented Jul 2, 2024

eightfilms commented Jul 2, 2024


		The KeyExpansion algorithm takes a 128/192/156-bit key and turns it into 11/13/15 round keys respectively of 16 bytes each. The main trick to key expansion is the fact that if 1 bit of the encryption key is changed, it should affect the round keys for several rounds.

		Using different keys for each round protects against _[slide attacks]_.


		### Decryption

		TODO

feat(ciphers): AES encryption #102

feat(ciphers): AES encryption #102

Conversation

eightfilms commented Jun 26, 2024 • edited Loading

eightfilms commented Jun 28, 2024

eightfilms commented Jun 29, 2024

Autoparallel commented Jun 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

0xJepsen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Autoparallel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lonerapier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eightfilms commented Jul 1, 2024

Autoparallel commented Jul 1, 2024

eightfilms commented Jul 1, 2024 • edited Loading

0xJepsen commented Jul 2, 2024

eightfilms commented Jul 2, 2024

eightfilms commented Jun 26, 2024 •

edited

Loading

eightfilms commented Jul 1, 2024 •

edited

Loading