Opened 8 months ago

Closed 4 months ago

Last modified 3 months ago

#27573 closed enhancement (fixed)

PRESENT Block Cipher

Reported by: gh-yhxnf Owned by:
Priority: major Milestone: sage-8.9
Component: cryptography Keywords: PRESENT
Cc: asante Merged in:
Authors: Lukas Stennes Reviewers: Friedrich Wiemer
Report Upstream: N/A Work issues:
Branch: 798421b (Commits) Commit:
Dependencies: Stopgaps:

Description

Implementation of the ultra-lightweight block cipher PRESENT.

Change History (26)

comment:1 Changed 8 months ago by gh-yhxnf

  • Status changed from new to needs_review

comment:2 Changed 8 months ago by git

  • Commit changed from 193a8a0db685eed64988151bd1f960e127606b32 to 3c11154109eff1daa7829cfb57c5c9fa3e3cacdc

Branch pushed to git repo; I updated commit sha1. New commits:

3c11154removed ... in EXAMPLES section

comment:3 Changed 8 months ago by git

  • Commit changed from 3c11154109eff1daa7829cfb57c5c9fa3e3cacdc to 027af50aa0adf18c288da9efd2bf9078106359f0

Branch pushed to git repo; I updated commit sha1. New commits:

027af50added tests

comment:4 Changed 8 months ago by asante

  • Cc Friedrich Wiemer added

comment:5 Changed 8 months ago by asante

  • Cc asante added; Friedrich Wiemer removed

comment:6 Changed 7 months ago by asante

  • Reviewers set to Friedrich Wiemer

I would rename the doLastLinearLayer argument to doFinalRound, as this will fit/be more consistent, when implementing other block ciphers.

I also reviewed the ticket and it lgtm, but maybe someone else also want to take a look?

comment:7 Changed 7 months ago by git

  • Commit changed from 027af50aa0adf18c288da9efd2bf9078106359f0 to 559ceacfe6e9d435a3f3af7360ba52c17a8b862d

Branch pushed to git repo; I updated commit sha1. New commits:

0ab40c0fixed typos
fd64dbdrenamed doLastLinearLayer to doFinalRound
559ceacupdated disclaimer

comment:8 Changed 6 months ago by asante

  • Status changed from needs_review to positive_review

I'm setting this to positive review now.

comment:9 Changed 6 months ago by asante

  • Status changed from positive_review to needs_work

actually I realised that I would like to have some design changes. I'll add them in a minute

Version 0, edited 6 months ago by asante (next)

comment:10 Changed 6 months ago by git

  • Commit changed from 559ceacfe6e9d435a3f3af7360ba52c17a8b862d to b429007c16032b3ddfc484dd9ccb180af58dc11f

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

5407256moved determination of inputType out of _convert_to_vector
1a1b6feadded sbox_layer function
18592f8use little endian sbox
3c174e0expose sbox variable
c1e355cadded linear_layer function
d0ff01eadded round function
6c9bf2drenamed variables
55cb61eexpose key schedule
3fd87e1added example
b429007Merge branch 'u/gh-yhxnf/present_block_cipher' of trac.sagemath.org:sage into present

comment:11 Changed 6 months ago by gh-yhxnf

  • Milestone changed from sage-8.8 to sage-8.9

comment:12 Changed 5 months ago by git

  • Commit changed from b429007c16032b3ddfc484dd9ccb180af58dc11f to e692eaffb7c07f8c5c22a16f1f074d19f401aeeb

Branch pushed to git repo; I updated commit sha1. New commits:

01c2822do not store inverse sbox
3e9e673use big endian PRESENT sbox
e692eafadd present.round example

comment:13 Changed 5 months ago by asante

OK, LGTM with one last point:

If you do the following import in an example

sage: from sage.crypto.block_cipher.present import _convert_to_vector

I think the _convert_to_vector function should not be hidden by default.

comment:14 Changed 4 months ago by git

  • Commit changed from e692eaffb7c07f8c5c22a16f1f074d19f401aeeb to 32ec4a91a47e59b4889675acd710cf7f66d5e2ab

Branch pushed to git repo; I updated commit sha1. New commits:

32ec4a9unhide _convert_to_vector

comment:15 Changed 4 months ago by gh-yhxnf

  • Status changed from needs_work to needs_review

comment:16 Changed 4 months ago by asante

lgtm

However, I have problems building the documentation, it fails with some error in the categorie docs -- which should actually not be related to this ticket? Maybe we should merge the latest development branch, for which I am able to build the documentation.

comment:17 Changed 4 months ago by asante

  • Branch changed from u/gh-yhxnf/present_block_cipher to u/asante/present_block_cipher

comment:18 Changed 4 months ago by asante

  • Commit changed from 32ec4a91a47e59b4889675acd710cf7f66d5e2ab to 6f1bfa7621521a468467eeb3669e4347200321d9
  • Status changed from needs_review to positive_review

I just did the rebase on the latest develop branch.

Docs now build correctly.

LGTM


New commits:

6f1bfa7Merge branch 'develop' into t/27573/present_block_cipher

comment:19 follow-up: Changed 4 months ago by vbraun

  • Status changed from positive_review to needs_work

The PDF docs don't build

[docpdf] ! Package amsmath Error: \begin{aligned} allowed only in math mode.
[docpdf] 
[docpdf] See the amsmath package documentation for explanation.
[docpdf] Type  H <return>  for immediate help.
[docpdf]  ...                                              
[docpdf]                                                   
[docpdf] l.6727 \ 
[docpdf]          [k_{79}k_{78}\dots k_{1}k_{0}] &= [k_{18}k_{17}\dots k_{20}k_{19}] \\
[docpdf] ? 
[docpdf] ! Emergency stop.
[docpdf]  ...                                              
[docpdf]                                                   
[docpdf] l.6727 \ 
[docpdf]          [k_{79}k_{78}\dots k_{1}k_{0}] &= [k_{18}k_{17}\dots k_{20}k_{19}] \\
[docpdf] !  ==> Fatal error occurred, no output PDF file produced!
[docpdf] Transcript written on cryptography.log.

comment:20 Changed 4 months ago by gh-yhxnf

  • Branch changed from u/asante/present_block_cipher to u/gh-yhxnf/present_block_cipher
  • Commit changed from 6f1bfa7621521a468467eeb3669e4347200321d9 to 798421b817a008b12d74fa47a6ff40a10a0bdbee

New commits:

65839f3removed unneeded import
798421bremove :nowrap:

comment:21 in reply to: ↑ 19 Changed 4 months ago by gh-yhxnf

Replying to vbraun:

The PDF docs don't build

should be fixed now

comment:22 Changed 4 months ago by gh-yhxnf

  • Status changed from needs_work to needs_review

comment:23 Changed 4 months ago by asante

  • Status changed from needs_review to positive_review

LGTM

comment:24 Changed 4 months ago by vbraun

  • Branch changed from u/gh-yhxnf/present_block_cipher to 798421b817a008b12d74fa47a6ff40a10a0bdbee
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:25 Changed 3 months ago by jhpalmieri

  • Commit 798421b817a008b12d74fa47a6ff40a10a0bdbee deleted

This causes a doctest failure with Python 3. Follow up at #28403.

comment:26 Changed 3 months ago by chapoton

I recall that development should now be done with python3. This would prevent this to happen.

Note: See TracTickets for help on using tickets.