Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#6841 closed enhancement (fixed)

[with patch, positive review] the shift cryptosystem

Reported by: mvngu Owned by: somebody
Priority: major Milestone: sage-4.1.2
Component: cryptography Keywords:
Cc: rbeezer Merged in: Sage 4.1.2.alpha4
Authors: Minh Van Nguyen Reviewers: Rob Beezer
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Implement the shift cryptosystem for educational purposes. This adds to the classical cryptosystems already implemented.

Attachments (1)

trac_6841-shift-cipher.patch (100.0 KB) - added by mvngu 12 years ago.
based on Sage 4.1.2.alpha2

Download all attachments as: .zip

Change History (12)

comment:1 Changed 12 years ago by mvngu

The patch trac_6841-shift-cipher.patch implements the shift cryptosystem. It also adds some doctests and documentation to existing crypto modules.

comment:2 Changed 12 years ago by mvngu

  • Authors set to Minh Van Nguyen

comment:3 Changed 12 years ago by mvngu

  • Summary changed from the shift cryptosystem to [with patch, needs review] the shift cryptosystem

comment:4 Changed 12 years ago by rbeezer

  • Cc rbeezer added

comment:5 Changed 12 years ago by mvngu

  • Summary changed from [with patch, needs review] the shift cryptosystem to [with patch, needs work] the shift cryptosystem

Here are some of rbeezer's comments on sanity checking the key value:

23:24 < mvngu-6971> rbeezer: Going back to shift cipher: The callable of 
                    ShiftCryptosystem (i.e. __call__) takes care of converting 
                    any value of key to 0 <= key < alphabet_size.
23:24 < mvngu-6971> rbeezer: Perhaps I should make that clear in the 
                    documentation.
23:27 < rbeezer> mvngu-6971: yes, I see it being Mod'ed, but I think you should 
                 disallow bad keys on input
23:27 < rbeezer> S(29) for "regular" alphabet just churns along, I'd prefer an 
                 error
23:27 < mvngu-6971> rbeezer: New patch coming up... in a hour or so. I'm still 
                    doing some release manage work...
23:28 < mvngu-6971> rbeezer: Thank you *very* much for the valuable feedback!
23:28 < rbeezer> For example with binary strings, my S(5) could have thrown an 
                 error, and I would have had to think about it right then, 
                 instead of looking for a positional shift of 5.  ;-)
23:29 < rbeezer> mvngu-6971: no rush, probably be tomorrow night before I get 
                 to it
23:29 < mvngu-6971> rbeezer: Cool.

Changed 12 years ago by mvngu

based on Sage 4.1.2.alpha2

comment:6 Changed 12 years ago by mvngu

  • Summary changed from [with patch, needs work] the shift cryptosystem to [with patch, needs review] the shift cryptosystem

The updated patch addresses rbeezer's comments on sanity checking the value of the encryption/decryption key. The sanity checking is done in the callable __call__() of the class ShiftCryptosystem. I have added doctests to that callable. Currently, methods beginning with an underscore "_" don't show up in the reference manual, so I have also added the doctests for checking key value to the docstring for the class ShiftCryptosystem. In this way, reading the reference manual for the shift cryptosystem should make it clear (hopefully) that the value of an encryption/decryption key must lie within the range of acceptable values, i.e. must be within the key space.

comment:7 follow-up: Changed 12 years ago by rbeezer

  • Reviewers set to Rob Beezer

Builds and runs on 4.1.2.alpha2. Passes tests, docs build without warnings. Great documentation.

Would you want to put similar input protections on inverse_key()? Your call.

Positive review, either way.

comment:8 Changed 12 years ago by rbeezer

  • Summary changed from [with patch, needs review] the shift cryptosystem to [with patch, positive review] the shift cryptosystem

comment:9 in reply to: ↑ 7 Changed 12 years ago by mvngu

Replying to rbeezer:

Would you want to put similar input protections on inverse_key()? Your call.

Yes, I like your idea: be consistent. Please see ticket #7010 for sanity checking the value of the inverse key.

comment:10 Changed 12 years ago by mvngu

  • Merged in set to Sage 4.1.2.alpha3
  • Resolution set to fixed
  • Status changed from new to closed

comment:11 Changed 12 years ago by mvngu

  • Merged in changed from Sage 4.1.2.alpha3 to Sage 4.1.2.alpha4

There is no 4.1.2.alpha3. Sage 4.1.2.alpha3 was William Stein's release for working on making the notebook a standalone package.

Note: See TracTickets for help on using tickets.