Opened 4 years ago
Closed 4 years ago
#22988 closed enhancement (fixed)
Add SBox Instances
Reported by:  asante  Owned by:  asante 

Priority:  major  Milestone:  sage8.1 
Component:  cryptography  Keywords:  sbox, crypto 
Cc:  leo.perrin@…  Merged in:  
Authors:  Friedrich Wiemer  Reviewers:  Martin Albrecht, David Roe 
Report Upstream:  N/A  Work issues:  
Branch:  31aa2db (Commits, GitHub, GitLab)  Commit:  31aa2db99d891e82645cadc7cf942332942db560 
Dependencies:  #22986  Stopgaps: 
Description (last modified by )
Leo Perrin collected a big amount of SBoxes used in block ciphers and hash functions during his PhD. We can add them to sage, to easily access them for experiments or so. This branch has Leo's list and references to where the SBoxes (cipher/hash functions) were initially described.
Change History (56)
comment:1 Changed 4 years ago by
 Component changed from PLEASE CHANGE to cryptography
 Keywords sbox crypto added
 Owner changed from (none) to asante
 Type changed from PLEASE CHANGE to enhancement
comment:2 Changed 4 years ago by
 Branch set to u/asante/add_sbox_instances
comment:3 Changed 4 years ago by
 Commit set to 95a8845e53458601a97c06e0da6accf434d9276f
 Status changed from new to needs_review
comment:4 Changed 4 years ago by
 Cc leo.perrin@… added
comment:5 Changed 4 years ago by
 Commit changed from 95a8845e53458601a97c06e0da6accf434d9276f to e1cb6427d2750eca1d42bbabe22aa1ac3e197875
Branch pushed to git repo; I updated commit sha1. New commits:
e1cb642  add dictionary of all sboxes for iteration purposes

comment:6 Changed 4 years ago by
Hi Friedrich,
Its also important to put URL link to the papers where these SBoxes are originally described.
Cheers
comment:7 Changed 4 years ago by
 Commit changed from e1cb6427d2750eca1d42bbabe22aa1ac3e197875 to 15afd28ca04de1a070136333eaa9ac2eca4544d8
comment:8 Changed 4 years ago by
I started to add references for the sboxes. Can you check if this is the correct format for references?
comment:9 Changed 4 years ago by
 Commit changed from 15afd28ca04de1a070136333eaa9ac2eca4544d8 to a600bb8264da370d93472a8e10d99bde45bb721a
Branch pushed to git repo; I updated commit sha1. New commits:
a600bb8  add more bibliography entries (some are still missing)

comment:10 Changed 4 years ago by
 Dependencies set to #22986
comment:11 Changed 4 years ago by
 Commit changed from a600bb8264da370d93472a8e10d99bde45bb721a to 358ced632ea4db5c1100bdf54614b6ae12ae7b95
Branch pushed to git repo; I updated commit sha1. New commits:
358ced6  moved references in global bibliography

comment:12 Changed 4 years ago by
 Commit changed from 358ced632ea4db5c1100bdf54614b6ae12ae7b95 to 9e42c1253805a9bac8499d3e9d06bc3dfc6f9841
Branch pushed to git repo; I updated commit sha1. New commits:
9e42c12  add more references for the sbox list

comment:13 Changed 4 years ago by
 Commit changed from 9e42c1253805a9bac8499d3e9d06bc3dfc6f9841 to b2f2a2364b3c5eda55fc20e4b457ec06480f777e
Branch pushed to git repo; I updated commit sha1. New commits:
b2f2a23  added last missing references

comment:14 Changed 4 years ago by
 Commit changed from b2f2a2364b3c5eda55fc20e4b457ec06480f777e to 9f526f724aed8201365bcffa8749fd876e1e3447
Branch pushed to git repo; I updated commit sha1. New commits:
b6fc04a  removed SBox from sage.crypto.all and use lazy imports

e42b860  Merge branch 'develop' into t/22986/refactor_sbox_code__move_from_crypto_mq_to_crypto_

9f526f7  Merge branch 't/22986/refactor_sbox_code__move_from_crypto_mq_to_crypto_' into t/22988/add_sbox_instances

comment:15 Changed 4 years ago by
I'd replace the long manual dict construction with something like:
for k, v in sage.crypto.sboxes__dict__.iteritems(): if isistance(sage.crypto.mq.sbox.SBox): sboxes[k] = v
This should make maintenance easier.
comment:16 Changed 4 years ago by
 Commit changed from 9f526f724aed8201365bcffa8749fd876e1e3447 to dd95e317d91a5d78c3c1da907954be67ddd804bd
Branch pushed to git repo; I updated commit sha1. New commits:
dd95e31  changed manually created dict of sboxes to autogenerated

comment:17 Changed 4 years ago by
Nice hint malb! This was actually what I was looking for, but I didn't find the dict object :)
I'm not sure if this
import sys sys.modules[__name__].__dict__
is the correct way to access the modul objects in itself, but I'm not aware of any other way.
comment:18 Changed 4 years ago by
Doesn't simply __dict__
work?
comment:19 Changed 4 years ago by
No, doesn't seem so:
sage: import sage.crypto.sboxes as sboxes  NameError Traceback (most recent call last) <ipythoninput1a6ff8351c3ae> in <module>() > 1 import sage.crypto.sboxes as sboxes /home/asante/werkstatt/werkbank/sage/local/lib/python2.7/sitepackages/sage/crypto/sboxes.py in <module>() 1286 sboxes = {} 1287 import sys > 1288 for k, v in list(__dict__.iteritems()): 1289 if isinstance(v, SBox): 1290 sboxes[k] = v NameError: name '__dict__' is not defined
I also tried sage.crypto.sboxes.__dict__
, .__dict__
, but only the sys.modules
approach worked for me.
comment:20 Changed 4 years ago by
Can you add a ticket description? People are more likely to review the ticket if they know what it does and once it is incorporated it is good to have a description too.
comment:22 Changed 4 years ago by
 Status changed from needs_review to needs_work
There's now a merge conflict.
I'm sorry that it's taken a while to review this. I'm happy to take a look once I get back from the mountains (in a week and a half) if nobody else has done so by then.
comment:23 Changed 4 years ago by
 Commit changed from dd95e317d91a5d78c3c1da907954be67ddd804bd to b1993ea245c2f152d0891a5448dcaeead32a6ff7
comment:24 Changed 4 years ago by
 Status changed from needs_work to needs_review
fixed merge conflicts
comment:25 Changed 4 years ago by
 Commit changed from b1993ea245c2f152d0891a5448dcaeead32a6ff7 to a8428e475e398c1cb2cc4422ddf916ea0e96e611
Branch pushed to git repo; I updated commit sha1. New commits:
a8428e4  add SBox from CHES paper 'GIFT'

comment:26 Changed 4 years ago by
Hi Friedrich,
I still do not understand the reason why the dictionary "sboxes" is needed here. Importing whatever SBox object in this module should be sufficient, e.g. (from sage.crypto.sboxes import AES). Could you explain the rationale behind having the dictionary "sboxes" ?
 Rusydi
comment:27 followup: ↓ 28 Changed 4 years ago by
Hi Rusydi,
this is for a convenient way of iterating over all collected Sboxes. Say you want to test some property for every Sbox, you can then just iterate over this dictionary. Without this dictionary such an iteration would not be that easy.
I already used this dictionary several times and find it really handy to have it.
comment:28 in reply to: ↑ 27 ; followup: ↓ 30 Changed 4 years ago by
In that case, I would suggest to give a explanation and an example that demonstrate this usefulness.
Replying to asante:
Hi Rusydi,
this is for a convenient way of iterating over all collected Sboxes. Say you want to test some property for every Sbox, you can then just iterate over this dictionary. Without this dictionary such an iteration would not be that easy.
I already used this dictionary several times and find it really handy to have it.
comment:29 Changed 4 years ago by
 Commit changed from a8428e475e398c1cb2cc4422ddf916ea0e96e611 to d544924e22efcc99f27e453d43fb885ea66fe298
comment:30 in reply to: ↑ 28 Changed 4 years ago by
Oh, of course, you are absolutly right, this dictionary should be mentioned in the documenation. I added an corresponding example.
Replying to ruhm:
In that case, I would suggest to give a explanation and an example that demonstrate this usefulness.
Additionally, I changed the creation of the dictionary to a way which is hopefully compatible with python 3.
comment:31 Changed 4 years ago by
 Commit changed from d544924e22efcc99f27e453d43fb885ea66fe298 to 09e1c4a8cd81538037e059948274163e15606161
Branch pushed to git repo; I updated commit sha1. New commits:
09e1c4a  iteritems not py3 compatible > items

comment:32 Changed 4 years ago by
 Milestone changed from sage8.0 to sage8.1
 Reviewers set to Martin Albrecht
 Status changed from needs_review to positive_review
LGTM
comment:34 Changed 4 years ago by
 Commit changed from 09e1c4a8cd81538037e059948274163e15606161 to c223d517244491ce3e90e73f66ab8d0f3752c092
comment:35 Changed 4 years ago by
 Status changed from needs_work to needs_review
fixed merge conflicts.
comment:36 Changed 4 years ago by
 Status changed from needs_review to positive_review
comment:37 followup: ↓ 38 Changed 4 years ago by
 Status changed from positive_review to needs_work
Docbuild fails
comment:38 in reply to: ↑ 37 ; followup: ↓ 39 Changed 4 years ago by
Replying to vbraun:
Docbuild fails
Are you sure that this error is due to my patch? I cannot docbuild the develop branch with the same error.
(Not sure if I get the same error than you:
OSError: [manifolds] /home/asante/werkstatt/werkbank/sage/src/doc/en/reference/manifolds/index.rst:4: WARNING: undefined label: tensorsonfreemodules (if the link has no caption the label must precede a section header)
plus some missing labels, but all these seem to be in the references rst document)
comment:39 in reply to: ↑ 38 ; followup: ↓ 40 Changed 4 years ago by
OK I think this was due to an error on my side to not clean old build things up.
Replying to asante:
Replying to vbraun:
Docbuild fails
Are you sure that this error is due to my patch? I cannot docbuild the develop branch with the same error.
(Not sure if I get the same error than you:
OSError: [manifolds] /home/asante/werkstatt/werkbank/sage/src/doc/en/reference/manifolds/index.rst:4: WARNING: undefined label: tensorsonfreemodules (if the link has no caption the label must precede a section header)plus some missing labels, but all these seem to be in the references rst document)
comment:40 in reply to: ↑ 39 Changed 4 years ago by
Replying to vbraun:
Docbuild fails
Nevertheless, I cannot reproduce the docbuild error. When running
SAGE_ROOT=`pwd` make clean SAGE_ROOT=`pwd` make SAGE_ROOT=`pwd` ./sage b SAGE_ROOT=`pwd` ./sage docbuild reference html
the build finishes successfully.
comment:41 followup: ↓ 43 Changed 4 years ago by
l.78 of sbox.py
there is a reference to [PRESENT07] that you renamed.
comment:42 Changed 4 years ago by
 Commit changed from c223d517244491ce3e90e73f66ab8d0f3752c092 to 43edbb8a56b615144e53b771ff5a4b209d855170
Branch pushed to git repo; I updated commit sha1. New commits:
43edbb8  fixed broken PRESENT reference

comment:43 in reply to: ↑ 41 Changed 4 years ago by
 Status changed from needs_work to needs_review
Replying to ylchapuy:
l.78 of
sbox.py
there is a reference to [PRESENT07] that you renamed.
Thanks ylchapuy! I totally oversaw this one :/ Sorry for the inconvenience.
Did I just not see the error in the build output, or did I miss a part of the docbuild?
comment:44 Changed 4 years ago by
 Commit changed from 43edbb8a56b615144e53b771ff5a4b209d855170 to dfbd71de53301eea115b3e717334b9dccd76ff3d
Branch pushed to git repo; I updated commit sha1. New commits:
dfbd71d  fix merge conflict

comment:45 Changed 4 years ago by
 Commit changed from dfbd71de53301eea115b3e717334b9dccd76ff3d to 6b8e8b1745403e1abdab6742044bcc1f9ba4c644
Branch pushed to git repo; I updated commit sha1. New commits:
6b8e8b1  the qarma block cipher actually defines three sboxes, one was missing

comment:46 Changed 4 years ago by
 Commit changed from 6b8e8b1745403e1abdab6742044bcc1f9ba4c644 to 9b5786a98d5977da8ab896f745211677d14754d2
Branch pushed to git repo; I updated commit sha1. New commits:
9b5786a  Merge branch 'develop' into t/22988/add_sbox_instances

comment:47 Changed 4 years ago by
 Reviewers changed from Martin Albrecht to Martin Albrecht, David Roe
 Status changed from needs_review to positive_review
Looks good!
comment:48 Changed 4 years ago by
 Commit changed from 9b5786a98d5977da8ab896f745211677d14754d2 to 9515fe6ba82d950a6b7845894bb7cf17083d21e2
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
9515fe6  fixes patchbot errors

comment:49 Changed 4 years ago by
 Status changed from needs_review to positive_review
Thanks for catching that. The changes are fine with me.
comment:51 Changed 4 years ago by
vbraun: I cannot see what merge conflict occurs here, would it be possible for you to check again?
comment:52 Changed 4 years ago by
please merge in the next beta
comment:53 Changed 4 years ago by
 Branch changed from u/asante/add_sbox_instances to u/roed/add_sbox_instances
comment:54 Changed 4 years ago by
 Commit changed from 9515fe6ba82d950a6b7845894bb7cf17083d21e2 to 31aa2db99d891e82645cadc7cf942332942db560
 Status changed from needs_work to positive_review
I fixed the merge.
New commits:
31aa2db  Merge branch 'u/asante/add_sbox_instances' of git://trac.sagemath.org/sage into t/22988/add_sbox_instances

comment:55 Changed 4 years ago by
It would be nice to get this merged early, since the global references file is kind of a nightmare for merge conflicts.
comment:56 Changed 4 years ago by
 Branch changed from u/roed/add_sbox_instances to 31aa2db99d891e82645cadc7cf942332942db560
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
refactored SBox code by moving to crypto package
updated documentation on SBoxes to reflect new path
add sbox instances of cryprographic schemes available in the literature
added alias for Midori_Sb0 = MANTIS