tigertv / secretpy

Classical ciphers: Caesar, ADFGX, ROT13 and etc.
https://pypi.org/project/secretpy/
MIT License
61 stars 10 forks source link

Scytal cipher rewritten #7

Closed LuminousLizard closed 3 years ago

LuminousLizard commented 3 years ago

I have rewritten the algorithm for the scytale cipher. Also I have created the "test_scytale.py" script and tested it.

I found different specifications of what the key is:

I used the number of windings and have also specified this in the docstring. The algorithm should also be compatible with Python2. Alphabets are not necessary because it's solved purely via displacement.

But please review the code and test it. Sorry for the waste of time because of the first script.

LuminousLizard

tigertv commented 3 years ago

https://github.com/tigertv/secretpy/pull/7/checks?check_run_id=2685690496

tests/test_scytale.py:5: in <module>
    from secretpy import Scytale
E   ImportError: cannot import name 'Scytale' from 'secretpy' (/home/runner/work/secretpy/secretpy/secretpy/__init__.py)

Here we need to add import in __init__.py files.

https://github.com/tigertv/secretpy/pull/7/checks?check_run_id=2685690386

./secretpy/ciphers/scytale.py:10:25: E999 SyntaxError: invalid syntax
    def __enc(self, text: str, key: int):
                        ^
1     E999 SyntaxError: invalid syntax

Python2 doesn't like that syntax.

Note: You can check the code by tests on your machine by using command:

make test

Is that working or something incorrect happens? Let me know and I'll change something.

Also I have created the "test_scytale.py" script and tested it.

It's the right way.

Alphabets are not necessary because it's solved purely via displacement.

Maybe for the transposition cipher is appropriate, but we have to save the alphabet for the interface, otherwise the CryptMachine breaks down.

https://github.com/tigertv/secretpy/blob/8364253ecd3f33b1f0cd737e1171754d9228e8a7/secretpy/cryptmachine.py#L25-L26

I found different specifications of what the key is:

Yes, I've seen this. CryptOnline says on the tab "description" the following:

The diameter of the Scytale can be regarded as the key of the cipher.

Let's consider it. If we use number of turns of the band as the key then each time we send a message we need either a new key or rods of different diameters. Right? Size of message is variable, key is constant, exchange of keys should happen only once(or not frequently). What do you think? img

Sorry for the waste of time because of the first script.

Keine Sorge! Herzlich Willkommen! We don't waste time, we spent time.

LuminousLizard commented 3 years ago
tests/test_scytale.py:5: in <module>
    from secretpy import Scytale
E   ImportError: cannot import name 'Scytale' from 'secretpy' (/home/runner/work/secretpy/secretpy/secretpy/__init__.py)

Here we need to add import in init.py files.

I hope I have now added everything necessary. make test for my fork works.

https://github.com/tigertv/secretpy/pull/7/checks?check_run_id=2685690386

./secretpy/ciphers/scytale.py:10:25: E999 SyntaxError: invalid syntax
    def __enc(self, text: str, key: int):
                        ^
1     E999 SyntaxError: invalid syntax

Python2 doesn't like that syntax.

Fixed !

Alphabets are not necessary because it's solved purely via displacement.

Maybe for the transposition cipher is appropriate, but we have to save the alphabet for the interface, otherwise the CryptMachine breaks down.

Alphabet added in scytale.py and test_scytale.py

I found different specifications of what the key is:

Yes, I've seen this. CryptOnline says on the tab "description" the following:

The diameter of the Scytale can be regarded as the key of the cipher.

Let's consider it. If we use number of turns of the band as the key then each time we send a message we need either a new key or rods of different diameters. Right? Size of message is variable, key is constant, exchange of keys should happen only once(or not frequently). What do you think?

That's right ! Seen after that the key should be the number of chars per winding. -> WIP

LuminousLizard commented 3 years ago

Let's consider it. If we use number of turns of the band as the key then each time we send a message we need either a new key or rods of different diameters. Right? Size of message is variable, key is constant, exchange of keys should happen only once(or not frequently). What do you think?

Doesn't work:

1. because the text is filled in rows and the windings are not fixed, you can theoretically pass a 20 char in 2 rows multiple times, e.g with 10 windings:

w 1 w 2 w 3 w 4 w 5 w 6 w 7 w 8 w 9 w 10
I a m h u r t v e r
y b a d l y h e l p
and with 11 windings: w 1 w 2 w 3 w 4 w 5 w 6 w 7 w 8 w 9 w10 w 11
I a m h u r t v e r y
b a d l y h e l p

2. Some keys doesn't work:

Plaintext = "Iamhurtverybadlyhelp"

with 4 windings you have 5 rows with 3 windings the text must be set in 7 rows

=> 6 rows isn't possible, because 1 winding fills more than 1 row !

The only solution for that is an exception e.g. that the first 2 rows can have 4 windings and the the following only 3 windings: winding 1 winding 2 winding 3 winding 4
I a m h
u r t v
e r y
b a d
l y h
e l p
or the first 3 rows can have 4 windings and the the following only 3 windings: winding 1 winding 2 winding 3 winding 4
I a m h
u r t v
e r y b
a d l
y h e
l p

but no ... I don't want to code that.

You must fix the number of windings and then the number of rows depends on the text length. There are exceptions the other way around.

tigertv commented 3 years ago

That's right! We have the same problem for two cases. 😊 Maybe the right key is fixed rows and columns at the same time. But if you want we can merge it and I'll add an additional commit for the cipher and do refactoring. Or ... let me think. Merged. Thanks.

tigertv commented 3 years ago

@LuminousLizard, Refactored.