Is my cipher secure?

Question

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")
    o1 = o.read()
    #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
    p = "/Users/kyle/one_time_pad/"+base2
    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")
    encrypt = encrypt1.read()
    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")
    encrypt = encrypt1.read()
    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")
    cipher2 = cipher1.read()
    cipher3 = [ord(c) for c in cipher2]

    #opens the key and coverts it to decimal
    key = raw_input("key: ")
    key1 = open(key, "r")
    key2 = key1.read()
    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*"-"

Answer 1

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.

Answer 2

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