Opened 8 months ago

Closed 3 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 7 months ago by asante

  • Cc Friedrich Wiemer added

comment:5 Changed 7 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 5 months ago by asante

  • Status changed from needs_review to positive_review

I'm setting this to positive review now.

comment:9 Changed 5 months ago by asante

  • Status changed from positive_review to needs_work

Actually I realised that I would like to have some design changes.

In principle, so that this is easier to do experiments with, it should be easy to modify the cipher. I have something in mind like the following:

sage: cipher = PRESENT(rounds=1, doLastLinearLayer=False)
sage: cipher.sbox = SBox(range(16))
sage: cipher.key_schedule = lambda x: [0, 0]  # return the 0 keys as round keys
sage: cipher.encrypt(plain=0x1234, key=0x0) == 0x1234
True

So this would require the implementation to split the encrypt function into a round function that uses an sboxlayer function and a linearlayer function, the sboxlayer uses the member variable sbox. All these should not be internal functions, but visible and documented, so that the user knows about them and that it is possible to modify them.

I hope this is clear, otherwise I can specify this further.

Last edited 5 months ago by asante (previous) (diff)

comment:10 Changed 5 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 5 months ago by gh-yhxnf

  • Milestone changed from sage-8.8 to sage-8.9

comment:12 Changed 4 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 4 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 3 months ago by asante

  • Status changed from needs_review to positive_review

LGTM

comment:24 Changed 3 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.