Coding best practices
It's best to move statements like os.urandom(64)
inside of a function, so they are reusable:
def generate_key():
return os.urandom(64)
As is, if I import the module, I get a bunch of text printed to the console. Those are some nasty side effects. It's best to assure that part of the code only runs when being run from the command line. Here's where a name guard comes in handy:
if __name__ == "__main__":
encrypted = encrypt(key, "HEHHEHEHHE")
print(encrypted[0])
decrypted = decrypt(key, encrypted[0], encrypted[1])
print(decrypted)
You have some unused imports (random
and time
).
Apart from that, your code is mostly clean. There's an official style guide for Python called PEP-8 which I recommend you take a look at.
Security
First the good part: you're using os.urandom()
, which is good, because it is cryptographically random.
Now the bad part: you're rolling your own crypto implementation. As much fun as it is to come up with your own 'secure protocols', 99% of them are broken from the start. As @Oscar Smith mentioned, you should use be using cryptography, which provides the high level Fernet protocol.
I'm going to dissect your code to explain why it is so insecure.
def encrypt(_key, message):
letter_map_message = []
ascii_msg_value = ''.join(str(ord(c)) for c in message)
for c in message:
''.join(str(ord(c)))
letter_map_message.append(ord(c))
We can use a list comprehension to our advantage here:
def encrypt(key, message):
letter_map_message = [ord(c) for c in message]
ascii_msg_value = ''.join(str(ord(c)) for c in message)
Note that I removed ''.join(str(ord(c)))
, because the statement result is immediately discarded anyway.
Suppose we feed a key of "foo"
and a message of "bar"
to the function. We now have:
letter_map_message := [102, 111, 111]
ascii_msg_value := "9897114"
ascii_key_value = ''.join(str(ord(x)) for x in str(_key))
new_msg_encrypted = int(ascii_msg_value) * int(ascii_key_value)
returned = (new_msg_encrypted, letter_map_message)
return returned
Simplifying this to:
ascii_key_value = ''.join(str(ord(x)) for x in str(_key))
new_msg_encrypted = int(ascii_msg_value) * int(ascii_key_value)
return new_msg_encrypted, letter_map_message
This gives us:
ascii_key_value := "102111111"
new_msg_encrypted := 1010605306233654
Thus returning:
(1010605306233654, [102, 111, 111])
So, in effect, encrypt()
returns an ambiguous number, and an ASCII bytearray, in plaintext.
Since encrypt()
returns a list of the code points for the message, decrypt()
* could be a one-liner:
def decrypt(_key, message, letter_map):
return "".join(chr(x) for x in letter_map)
Yet, for some reason, your function is 9 lines long. Let's see why.
def decrypt(_key, message, letter_map):
ascii_key_value = ''.join(str(ord(x)) for x in str(_key))
msg_ascii_divided = int(message)/int(ascii_key_value)
Gives us:
ascii_key_value := "102111111"
msg_ascii_divided := 1010605306233654 / 102111111
= 9897114.0
letters = []
for c in range(len(letter_map)):
li = letter_map[c]
c = int(li)
letter = chr(c)
letters.append(letter)
new_msg_decrypted = ''.join(letters)
return new_msg_decrypted
This is an overly verbose way of saying:
letters = [chr(c) for c in letter_map]
return "".join(letters)
Note how _key
and message
are not at all involved in the decryption process. Strange, don't you think? It goes without saying that this is insecure.
Finally, here are encrypt()
and decrypt()
, as one-liners:
def encrypt(message):
return [ord(char) for char in message]
def decrypt(message_map):
return "".join(chr(x) for x in message_map)
Uh-oh.
* You can't call this encryption, it's obfuscation.