5
\$\begingroup\$

I have created an encryption algorithm for this app I am currently in the process of developing. I was wondering if my code looks "sharp", concise, and most importantly, succinct. I was also wondering if it is secure.

I know that 99% of the time, encryption algorithms that are invented are not secure, however I was wondering if this algorithm is in the 1% of the safe ones.

#Key Size: 128 bytes [recommended]

import random
import os
import time
import hashlib

def generate_key(key_size):
    key = os.urandom(key_size)
    return key;

def encrypt(_key, message):
    letter_ascii = []
    letter_ascii_final = []
    ascii_key_value = ''.join(str(ord(x)) for x in str(_key))
    for letter in message:
        letter_ascii.append(ord(letter))
    for _letter in letter_ascii:
        added = int(ascii_key_value) + int(_letter)
        letter_ascii_final.append(str(added))
    return letter_ascii_final;

def decrypt(_key, message):
    message_repieced = []
    ascii_key_value = ''.join(str(ord(x)) for x in str(_key))
    for item in message:
        _item = chr(int(item)-int(ascii_key_value))
        message_repieced.append(str(_item))
    return ''.join(message_repieced);
\$\endgroup\$
  • 4
    \$\begingroup\$ Are you matt? who asked almost exactly the same problem yesterday? \$\endgroup\$ – Oscar Smith Dec 24 '17 at 4:03
  • 6
    \$\begingroup\$ Yes, this question appears identical to this one and is assumbably a follow-up. If so, please declare it as such here. \$\endgroup\$ – Jamal Dec 24 '17 at 5:52
  • 3
    \$\begingroup\$ Your 99% lost a few nines after the decimal point - you should never, ever even try to invent your own cipher if it's meant to go to production. Unless you know what you are doing, but then you are not an application programmer... \$\endgroup\$ – NieDzejkob Dec 25 '17 at 17:36
31
\$\begingroup\$

No. This is just a worse Caeser cypher.

steps to break:

  1. Convert each string to an int in the output
  2. Subtract everything from the min output value - ord('A')
  3. Everything is now between 0 and 26
  4. If the message is readable, you are done (this will be true if the message contains an 'a')
  5. Otherwise, add 1 to the each character
  6. After repeating this between 1 and 24 times, the message will be decrypted.
\$\endgroup\$
  • 4
    \$\begingroup\$ Minor correction: the plaintext may contain characters with ASCII codes below 'A', such as punctuation and spaces, so there may be more than 24 possible decrypts to check. But not enough to matter; it's still hopelessly insecure. \$\endgroup\$ – Gordon Davisson Dec 25 '17 at 8:33
13
\$\begingroup\$

In addition to Oscar Smith's answer why this code is not secure, here are a few nitpicks on your code itself:

  1. While it is legal to end a line with ; in Python, it is not necessary and actively discouraged.
  2. You should use list comprehensions:
def encrypt(_key, message):
    key_value = int(''.join(str(ord(x)) for x in str(_key)))
    return [ord(letter) + key_value for letter in message]

def decrypt(_key, message):
    key_value = int(''.join(str(ord(x)) for x in str(_key)))
    return ''.join([chr(int(item) - key_value) for item in message])
  1. Move the comment about the recommended keysize into the docstring of the encrypt method:
def encrypt(_key, message):
    """
    Encrypt message using a Caesar shift determined by key.
    Key Size: 128 bytes [recommended]
    """
    key_value = int(''.join(str(ord(x)) for x in str(_key)))
    return [ord(letter) + key_value for letter in message]
\$\endgroup\$
5
\$\begingroup\$

Oscar already pointed out that this algorithm is a bad one, but I'd like to give you some advice on how to check code for some of the more obvious flaws in the future. As the most basic test of an encryption algorithm, the output it generates should at least look 'random' when fed any type of input. Patterns in the output are always bad, because they reveal something about the input. Your code applies the same transformation to every element in the input, so if the input was all 0s, the output will only contain 1 unique value as well. This is the same flaw which you can see with the famous ECB Penguin. ECB penguin

Every pixel in the penguin is modified with the same function, so outlines of regions are clearly visible even if every individual pixel has been modified in a complex way.

To improve on this, you're going to first need to learn a bit about cipher modes of operation and pseudo-random permutations. I would recommend starting by learning how CTR mode encryption works and trying to implement it because it is one of the simplest to understand.

\$\endgroup\$
  • 5
    \$\begingroup\$ Everything you said is correct, but I can't agree with your advice. Adopting a better mode of operation would not help the security of this encryption noticeably. The correct advice is to avoid either designing or implementing your own crypto, and instead use a well-audited crypto library (and follow the experts' advice on using it correctly). \$\endgroup\$ – Gordon Davisson Dec 25 '17 at 8:25
  • \$\begingroup\$ True enough. I don't think it's a good idea for anyone to roll their own encryption if they haven't spent years studying it and had their results peer reviewed, but everyone who eventually becomes good at crypto has to indulge their initial curiosity at some point. \$\endgroup\$ – klfwip Dec 26 '17 at 4:27
2
\$\begingroup\$

I have created an encryption algorithm

Aaaaaand you've already lost, right out of the gate.

Don't do it, ever, unless you hold a degree in advanced mathematics with a specialization in cryptography.

Plus, why not use AES (which is both secure and hardware-accelerated)?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.