Encryption program for a messenger app [duplicate]

Question

I am building an encryption program for a messenger app I am developing.

I was wondering if the code looks clean and succinct, and at the same times is safe and secure:

import random
import os
import time

key = os.urandom(64)

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))
    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;

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)
    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;

encrypted = encrypt(key, "HEHHEHEHHE")
print(encrypted[0])
decrypted = decrypt(key, encrypted[0], encrypted[1])
print(decrypted)

Answer 1

This is not a secure system.

I haven't actually read your code, but unless you have a team of at least 10 security researchers who have been doing nothing other than testing this encryption scheme, you should not use it in production code. You should be using a crypto library built by someone that knows what they are doing. I would recommend cryptography. Docs are here https://cryptography.io/en/latest/. Good luck

Answer 2

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))

1. 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.

2. 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

3. 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
   

4. This gives us:

   ascii_key_value   := "102111111"
   new_msg_encrypted := 1010605306233654
   

5. 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)

6. 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

7. 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.