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:

Status badges

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)

trac9674_fix_Sbox_init.patch (971 bytes) - added by ylchapuy 11 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 follow-up: Changed 11 years ago by ylchapuy

Does ticket #9366 fix your problem?

comment:2 in reply to: ↑ 1 Changed 11 years ago by nmouha

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 ylchapuy

  • 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 nmouha

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 ylchapuy

  • Authors set to Yann Laigle-Chapuy

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 ylchapuy

  • Cc malb added

Maybe Martin will be interrested in reviewing this...

comment:7 Changed 11 years ago by malb

  • Status changed from needs_review to positive_review

Patch looks good, doctests pass.

comment:8 Changed 11 years ago by mpatel

  • 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 mpatel

  • Reviewers set to Martin Albrecht

Changed 11 years ago by ylchapuy

comment:10 Changed 11 years ago by ylchapuy

  • 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 mpatel

Sure. Thanks!

comment:12 Changed 10 years ago by mpatel

  • Merged in set to sage-4.6.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.