# Is my cipher secure?

This is a program that I made for encrypting text files, it uses a one time pad cipher to encrypt the files, but I do not know if there are any holes in my program that could be a vulnerability.

import os
q = 1
while q == 1:
e = raw_input("file to encypt: ")
#This will open a file for encryption
o = open(e, "r")
#This is the plain text to encrypt
#'The quick brown fox jumps over the lazy dog'
plain = o1
#This will measure the length of the plain text
f3 = len(plain)
#generate random chacters as long as the text
a1 = os.urandom(f3)
#makes the random characters tuple format
b = list(a1)
b2 = list(plain)
s = plain
#gives the ascii value of the charters
L = [ord(c) for c in s]
s = a1
a = [ord(c) for c in s]
b = [ord(c) for c in plain]
#adds the random digits and the plain text
c = map(sum, zip(a,b))
#uses Modular arithmetic if the sum is greater than 256
x=c
z = []
for y in x:
z.append(y-256 if y>=256 else y)
z = [y-256 if y >= 256 else y for y in x]
#converts the sum back to charter form
cipher_text = ''.join(chr(i) for i in z)
#makes a folder for the files
base1 = os.path.basename(e)
base2 = os.path.splitext(base1)[0]
#makes a folder for the output
print p
if os.path.exists(p):
print
else:
os.mkdir(p)

key = a1
#makes a file containg the key
p = p + "/"
f2 = p+"key.txt"
#print f2
if os.path.exists(f2):
f1 = file(f2, "w")
f1 = open(f2, "w")
f1.write(key)
f1.close()
else:
f1 = file(f2, "w")
f1 = open(f2, "w")
f1.write(key)
f1.close()

#makes a file containg the cipher text
f3 = p+"cipher_text.txt"
if os.path.exists(f3):
f1 = file(f3, "w")
f1 = open(f3, "w")
f1.write(cipher_text)
f1.close()
else:
f1 = file(f3, "w")
f1 = open(f3, "w")
f1.write(cipher_text)
f1.close()

f4 = p+"encrypt.py"
encrypt1 = open("/Users/kyle/encrypt.py", "r")
if os.path.exists(f4):
f1 = file(f4, "w")
f1 = open(f4, "w")
f1.write(encrypt)
f1.close()
else:
f1 = file(f4, "w")
f1 = open(f4, "w")
f1.write(encrypt)
f1.close()

f5 = p+"decrypt.py"
encrypt1 = open("/Users/kyle/decrypt.py", "r")
if os.path.exists(f5):
f1 = file(f5, "w")
f1 = open(f5, "w")
f1.write(encrypt)
f1.close()
else:
f1 = file(f5, "w")
f1 = open(f5, "w")
f1.write(encrypt)
f1.close()

print 50*"-"


This is the code that I use for decryption:

import os

q = 1
while q == 1:
#opens the cipher text and it converts it to decimal
cipher = raw_input("cipher text: ")
cipher1 = open(cipher, "r")
cipher3 = [ord(c) for c in cipher2]

#opens the key and coverts it to decimal
key = raw_input("key: ")
key1 = open(key, "r")
key3 = [ord(c) for c in key2]

#subtracts the key from the cipher
a = cipher3
b = key3
c = map(lambda x: (x[0]-x[1]) % 256, zip(a,b))

#prints out the decrypted plain text
decrypt = ''.join(map(chr,c))
#makes a file with the decrypted output
path1 = raw_input("out folder: ")
name = "plain_text.txt"
path2 = path1 + "/" + name
if os.path.exists(path2):
f1 = file(path2, "a")
f1 = open(path2, "a")
f1.write(decrypt)
f1.close()
else:
f1 = file(path2, "w")
f1 = open(path2, "w")
f1.write(decrypt)
f1.close()

print 50*"-"

• Crypto is a very specialized topic. General programers will be able to critique your code style not your crypto strength. Go to a specialized group for that: crypto.stackexchange.com – Martin York Jun 28 '13 at 2:45
• At minimum your code is silly. The provable security of a one time pad relies on truly random data, so you can't use /dev/urandom. If you don't need provable security (hint: you don't), then a simple stream cipher is just as strong as a OTP like construction, but far nicer to use. – CodesInChaos Aug 31 '13 at 17:53

As you are no doubt aware, one-time pads are cryptographically secure according to Kerckhoff's principle, assuming the key is sufficiently random and secret. From a brief glance, your implementation of OTP looks correct. (But I might of course have missed something.)

However, whether the program itself has any vulnerabilities would not be relevant as long as you write the key to a file. Anyone with access to your memory or harddisk would be able to intercept the file, and therefore the key. This means the question boils down to "Is my implementation correct?", and the answer to that is "Yes", as far as I can tell.

I might be wrong, however, because your program is not very readable. Some of the things you can do to clean it up are:

1: Improve variable names. Several of your variable names are single letters. plaintext_length is way more readable than f3.

2: Split your code into logical units. Refactor into functions, and group code that logically belongs together by separating such groups with a single empty line.

3: Why on earth are you doing this?!

if os.path.exists(f2):
f1 = file(f2, "w")
f1 = open(f2, "w")
f1.write(key)
f1.close()
else:
f1 = file(f2, "w")
f1 = open(f2, "w")
f1.write(key)
f1.close()


As far as I can see, the two blocks are identical, and one of them will be executed regardless. And as Anthon points out, the first two lines in each block do the same.

• Thank you for the advice, i should Improve variable names it will make debugging more easy, and at a second look i do not know why i am handling the files in that way, thanks for the advice. – kyle k Jun 29 '13 at 6:15

I am not going to comment on possible vulnarabilities (that question does not belong here). But your Python code has several issues.

In the code:

f1 = file(path2, "w")
f1 = open(path2, "w")
f1.write(decrypt)
f1.close()


the first two lines do the same, opening path2 twice. You should leave out the first, or better use:

with open(path2, "w") as f1:
f1.write(decrypt)


unless you have an old version of python without with. This closes the file automatically on exit of the scope of the with statement.

You should also put your code (apart from the import) in a function, so it can be more easily reused (the decryption file):

import os

def decrypt():
q = 1
while q == 1:
#opens the cipher text and it converts it to decimal
.
.
.
print 50*"-"

if __name__ == '__main__':
decrypt()