Opened 11 years ago
Closed 10 years ago
#9674 closed defect (fixed)
fix SBox __init__ (again)
Reported by: | nmouha | Owned by: | mhansen |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6 |
Component: | cryptography | Keywords: | |
Cc: | malb | Merged in: | sage-4.6.alpha1 |
Authors: | Yann Laigle-Chapuy | Reviewers: | Martin Albrecht |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
Revision sage/crypto/mq/sbox.py [11673:11b2f556827a:12294:d7533ae4895e] (explacing log_b() by exact_log()) causes the following problems:
- difference_distribution_matrix() (in crypto/mq/sbox.py) crashes when an n-to-m bit S-box does not contain the element 2<sup>m-1</sup> (the wrong calculation of m results in an array index going out of bounds).
- the statement length != int(length) is never executed, because exact_log() always outputs an integer
Attachments (1)
Change History (13)
comment:1 follow-up: ↓ 2 Changed 11 years ago by
comment:2 in reply to: ↑ 1 Changed 11 years ago by
Replying to ylchapuy:
Does ticket #9366 fix your problem?
Thank you for looking into this. You patch fixes the second bullet point. But the main problem is this line: "self.n = ZZ(max(S)+1).exact_log(2)".
Try this, and see what happens: S = mq.SBox(5,6,0,3,4,2,1,2); S.difference_distribution_matrix();
comment:3 Changed 11 years ago by
- Milestone set to sage-4.5.3
- Status changed from new to needs_review
- Summary changed from Please revert sage/crypto/mq/sbox.py [11673:11b2f556827a:12294:d7533ae4895e] to fix SBox __init__ (again)
You might review the following patch replacing exact_log
with nbits
.
comment:4 Changed 11 years ago by
Your proposed patch fixes the problem. I also think it's a cleaner solution than reversing the patch by mhansen. Thanks for your time.
comment:5 Changed 11 years ago by
If you are satisfied with the patch, and checked that all the doctests are still ok, then the procedure is to check the action "positive review" (bottom of the ticket page if you are logged in) and add yourself to the reviewers. This allows the release manager to consider the merging of this ticket in the next release.
comment:6 Changed 11 years ago by
- Cc malb added
Maybe Martin will be interrested in reviewing this...
comment:7 Changed 11 years ago by
- Status changed from needs_review to positive_review
Patch looks good, doctests pass.
comment:8 Changed 11 years ago by
- Status changed from positive_review to needs_work
Please put the ticket number in the first line of the patch commit string.
comment:9 Changed 11 years ago by
- Reviewers set to Martin Albrecht
Changed 11 years ago by
comment:10 Changed 11 years ago by
- Status changed from needs_work to positive_review
Done, I put it back as positive review directly, I hope it's ok.
comment:11 Changed 11 years ago by
Sure. Thanks!
comment:12 Changed 10 years ago by
- Merged in set to sage-4.6.alpha1
- Resolution set to fixed
- Status changed from positive_review to closed
Does ticket #9366 fix your problem?