arosspope / cipher-crypt

A cryptographic tomb of ciphers forgotten by time.
Apache License 2.0
42 stars 8 forks source link

Trailing whitespace in `Columnar Transposition` Cipher #30

Closed avastmick closed 6 years ago

avastmick commented 6 years ago

The current implementation of the Columnar Transposition cipher pads uneven columns with whitespace ' ' to make sure that the resulting columns are easier to handle on decryption.

loop {
       if let Some(c) = chars.next() {
            key[i].1.push(c);
        } else if i > 0 {
            // This can cause issues ->
            key[i].1.push(' '); //We must add padding characters
        } else {
            break;
        }

This is is a good idea and makes the code much simpler for decrypt, however, in some cases the pad comes on the final transposed column and the whitespace may exist at the end of the encrypted text.

See the implementation for the ADFGVX cipher and the tests, where there is a trailing whitespace char. Kept in to ensure there is a viable fix created.

I suggest that the ColumnarTransposition code is changed to remove the whitespace padding and handle the code complexity in the decrypt function.

As an example, see pycipher/columnartransposition.py for an implementation that manages this. However, note that this implementation also removes all non-alphanumeric chars when encoding. Not sure this is ideal, as it seems to lose niceties, but may be worth a look on how to handle the uneven columns.

avastmick commented 6 years ago

Researching the implementations of the cipher, it looks as if padding is optional, according to usage. Where padding, as termed, Nulls are included, they seem to be determined as part of the plantext encoding, or as infrequent characters. See, Wikipedia, for example.

Accordingly, I am changing the ColumnarTransposition code to optionally allow Nulls. The Null character will be given as a key parameter in the constructor; if the null_characters is empty, then use_nulls will be false (new function); if null_characters is (for example) whitespace char, then the functionality is the same function as existing in version 0.11.0.

arosspope commented 6 years ago

As a side note, i think the scytale cipher also has this issue. If you're feeling adventurous perhaps you could implement the same fix for that cipher as well?

avastmick commented 6 years ago

As per your advice @arosspope, I should have implemented the entry into the code using the Option<> type in the first place! As I said, I'm new to Rust. The new code will be much cleaner.

The default will be no nulls (padding), with only nulls if the user wants to use them. There are good reasons for including them in ciphers as they may add to the ciphercode complexity and obfuscate the output.

arosspope commented 6 years ago

See PR #32