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

AES cleanup fixes pt 1 for #179 #210

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

AES cleanup fixes pt 1 for #179 #210

wants to merge 6 commits into from

Conversation

jn80842
Copy link
Collaborator

@jn80842 jn80842 commented Dec 20, 2024

Three of the tasks from issue 179, cleaning up small issues to do with the AES spec.

Copy link
Contributor

@marsella marsella left a comment

Choose a reason for hiding this comment

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

This looks solid, thank you for addressing! I appreciate the coherent commits.

I have a few comments; the most important ones are about docs and naming in SBox -- the rest are mostly nitpicky.

One other top-level comments: please check off the boxes for the things you finished in #179!

sbox : SBox
sbox = [ transform (GF28::inverse b) | b <- [0 .. 255] ] where
sboxTable : SBox
sboxTable = [ transform (GF28::inverse b) | b <- [0 .. 255] ] where
Copy link
Contributor

Choose a reason for hiding this comment

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

todo: Since this table is not used anywhere except the sbox function, let's make it private. Also, please update the docs on this to point to Table 4 (my bad) and mention that it's pulled out here for efficiency (e.g. this lets us compute the table once for every access in AES, instead of recomputing every time).

Same comment on the inverse below.

Primitive/Symmetric/Cipher/Block/AES/SBox.cry Outdated Show resolved Hide resolved
// Equation 5.3.
transform b = GF28::add [b, (b >>> 4), (b >>> 5), (b >>> 6), (b >>> 7), c]
// The constant byte {01100011}.
c = 0x63
sbox : GF28 -> GF28
Copy link
Contributor

Choose a reason for hiding this comment

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

todo: this function needs docs. You can use the description that's currently on sboxTable.

Ditto for inverse.

Primitive/Symmetric/Cipher/Block/AES/Algorithm.cry Outdated Show resolved Hide resolved
Primitive/Symmetric/Cipher/Block/AES/Algorithm.cry Outdated Show resolved Hide resolved
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