Skip to content

Feat/aes128#719

Closed
jplaui wants to merge 3 commits intoConsensys:masterfrom
jplaui:feat/aes128
Closed

Feat/aes128#719
jplaui wants to merge 3 commits intoConsensys:masterfrom
jplaui:feat/aes128

Conversation

@jplaui
Copy link
Copy Markdown

@jplaui jplaui commented Jun 8, 2023

Please find my initial aes128 and aes128 gcm implementations in this pull request.
I have implemented the naive lookup of s-box values. possible s-box lookup optimizations can be found here: https://github.com/akosba/jsnark/tree/master/JsnarkCircuitBuilder/src/examples/gadgets/blockciphers/sbox

@ivokub
Copy link
Copy Markdown
Collaborator

ivokub commented Sep 6, 2023

Please find my initial aes128 and aes128 gcm implementations in this pull request. I have implemented the naive lookup of s-box values. possible s-box lookup optimizations can be found here: https://github.com/akosba/jsnark/tree/master/JsnarkCircuitBuilder/src/examples/gadgets/blockciphers/sbox

Thanks for the contribution. Unfortunately right now AES in not on gnark roadmap and it would take quite some time to review the PR is correct (and most importantly, sound). I think AES is definitely worth it and allows interesting applications. Lets keep the PR open until we have more time to get back to it.

@ivokub ivokub added the src: community Community originating PRs and issues label Sep 11, 2025
@ivokub ivokub self-requested a review March 3, 2026 09:20
@ivokub
Copy link
Copy Markdown
Collaborator

ivokub commented Mar 31, 2026

Thanks for the PR and sorry for not being able to review it earlier. However I do not think it would not suit gnark standard library as is. Main issues:

  • arithmetic is done only using frontend.Variable, not uints.U8. The frontend.Variable types are unconstrained, meaning that we do not assert that the inputs are bytes. The uints package performs all check automatically, preventing any possible soundness issues due to under-/overflow
  • The interfaces stray from the pattern we use in rest of standard gadgets. It is a bit subjective, but there are exposed public methods etc.
  • The block mode is defined to be GCM, but it is actually a CTR-mode. In practice this is unsafe due to malleability.
  • Lacking standard AES test vectors etc.
  • Lacking package example.

In short, it would be quicker to write the gadget from scratch, but considering that it hasn't really been requested, then I don't anticipate that we would include it as is in gnark right now.

@ivokub ivokub closed this Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: P3-low Issue priority: low. proposition src: community Community originating PRs and issues type: new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants