Creating Original Database Algorithm--Login System

Question

For fun, I have been making a very basic login system in Python. I was going to look up an algorithm or something for user info storage and transfer, but then decided it would be more fun to come up with and implement it myself. What I would like to know is:

1. Is there anyway to make it secure (i.e.-encrypting the password without revealing the algorithm in the file)?

2. Can the algorithm I used be made any more efficient?

3. Is it cleanly written? I tried to take into account the comments on my last question regarding simplifying code and any comments about its readability would be appreciated--by secondary goal is beautiful code.

#This program will do a couple of things.
#The first time it is run, it will request that you add a password. Once you do, it will change the check_e$
#Every subsequent time, it will check the password in the other file and then match user input.
#If user input is correct it will display a joke.
#If user input is incorrect it will exit the program.

#Function to help the user pick a password
def pick_password():
        file = '/home/vhx/Documents/code/pydata_test/password_dbs/pswd.txt'
        print 'Please pick a password.'
        password = raw_input()
        target = open(file, 'w')
        target.write(password)
        file = '/home/vhx/Documents/code/pydata_test/password_dbs/existence_check.txt'
        target = open(file, 'w')
        target.write('YES')
#       file.close()

#Function to check the password with the password located in pswd.txt
def password_check():
        file = '/home/vhx/Documents/code/pydata_test/password_dbs/pswd.txt'
        pwd_check = open(file).read()
        userpass = raw_input('Please input a password.\n')
        if userpass == pwd_check:
                print 'Password accepted!'
                print 'Ready for the joke?'
                raw_input()
                print 'Why did the fly fly? Because the spider spied her!'
        elif userpass != pwd_check:
                print 'Sorry, wrong password.\n'
                exit()
        else:
                print 'Invalid syntax.'
                exit()

#location of password existence check file
EC = '/home/vhx/Documents/code/pydata_test/password_dbs/existence_check.txt'
PWD = '/home/vhx/Documents/code/pydata_test/password_dbs/pswd.txt' #Location of password file

pswd_exist = open(EC).read() #Checking to see if the password exists
if pswd_exist == 'YES':
        pass
else:
        pick_password() #If it doesn't, user will pick a password

#Checking for password
password_check()

Note: If you want to run it, it requires two files --pswd.txt and existence_check.txt--which you then must link (and replace all necessary file locations).

If you input the correct password, the program tells a joke, though whether its funny is debatable.

Answer 1

In your function pick_password, you should close the file at the end of the file because this frees up any system resources that were being devoted to working with that file.

For some reason, at the end of that function, you commented out the line

f.close()

You should un-comment this.

---

In python, it is good practice to open a file using the with keyword. Using with, you can easily access the file object and the file is automatically closed at the end of the with statement.

Here is how you should open the file:

with open(file, "w") as f:
    # whatever you want to do to the file f

Source

---

In your code, you have three separate variables that are all set to

'/home/vhx/Documents/code/pydata_test/password_dbs/pswd.txt'

You should create a single global constant variable that holds this value. Here is what that would look like:

PASSWORDS = '/home/vhx/Documents/code/pydata_test/password_dbs/pswd.txt'

I put the variable name in all-caps because constants are usually all capital letters.

---

In your function password_check, you make an exit call in a few places. However, you don't pass anything to it.

The number you pass to an exit call is called the exit code. This number is to show how execution went when the program was running to external processes.

Most commonly, an exit code of 0 means that execution was perfect, and an exit code of any non-zero number means that something went wrong.

I think that you should exit your program with 1 to show that something went wrong the user was entering their password (they entered the password incorrectly).

An advantage to doing it this way would be say, for example, you wrote another program and you only wanted to give access to it if the user entered the password correctly. In this program, you could read the exit code of this password program and if it's a 1, than your new program now knows that the user entered the password incorrectly.

P.S. Your interpreter is probably already doing this for you, but at the top of your code you should have from sys import exit because the sys library holds the exit function that you are calling.

Thanks to janos for correcting me on error exit codes.

---

I believe that it is fairly difficult to make this more secure.

1. The user can view the passwords file

2. If you do encrypt the file, this voids the above reason but the user will always be able to see your python source.

However, as BlueRaja - Danny Pflughoeft commented,

-1 for "impossible to make this more secure" - since he's storing passwords in plain-text, I'd argue it's impossible to make this less secure. Even if you need the passwords to be reversible (which is usually not the case), you should be encrypting them in a way that makes them unreadable even with the Python source (see eg. LastPass, which encrypts all passwords using a single master password)

---

Answer 2

It looks like passwords are stored in plain text form in the pswd.txt file. Never ever store passwords in plain text form. Store passwords salted and cryptographically hashed. That way, if an attacker gains access to the file, he still has to crack the password, which can be extremely difficult if the password is strong enough.

To verify a password, apply the same algorithm to the user input as used when creating the salted and cryptographically hashed version. The result will only match the stored password if the user entered the correct password.

@Boris left a great comment, quoting it verbatim:

Encrypted passwords are a bad idea as the encryption key needs to be stored in the code, then then it's just a question of looking at the code to gain the key. Hashing is irreversible and using a good, random, unique, salt per password and a purpose built cryptographic hashing algorithm is the only acceptable way of storing passwords - the best idea is to use something like bcrypt which is industry standard and rolls all this together.

Answer 3

• in unix/linux error messages are printed to stderr and not to stdin
• it is unclear to me, why you need the 'existence_check.txt'. If the 'pswd.txt' file exists, it contains a valid password.
• you use the same literals (filenames) on different places. Use the variables EC and PWD instead of.
• the name PWD has a special meaning in the unix context. maybe you shouk use another name.
• the filenames should not be part of the program but they shoul be read from a configuration file.
• you should put your login procedure in a function.
• if you create a file that stores such sensible information like passwords then you shoul ensure that the file that you create has the approptiate permissions.
• I don't like jokes in login procedures. Especially if they make it necessary to press the return key again.
• I am not familiar enough with python. Is the file automatcally closed after pswd_exist = open(EC).read()?
• Do not store passwords. Check how password algorithms work. Most procedures for entering the password don't display the user input. And therefore the user has to input the password twice and the two inputs are compared. Your password cannot be changed, no minimal requirements. So if a user presses erroneously the return key when he shoul pick a password, the empty password is set forever,

Answer 4

The standard way to open a file in Python is to use a with ... as ... context manager. By using a with ... as ... context manager, you can ensure that the file is properly closed, and there aren't any memory issues. Here's how you'd do that:

with open("path/to/myfile.txt", "r") as my_file:
    ...

Context managers also allow for multiple values as well. Here's an example of that:

with open("path/to/myfile.txt", "r") as my_file, ...:
    ...

You should also be using docstrings to describe your functions, not regular inline comments. Here's an example of a docstring:

def my_func( ... ):
    """
    Describe your function and it's arguments
    in this docstring.
    """

Finally, this is not a good way to do password checking at all. Preferably, rather than creating your own system without any security, you should use a library, like OpenSSL.

Answer 5

The security aspects have been pretty well-covered, so here are a few comments on your general Python style:

• Rather than using block comments to document functions and/or scripts, it's better to use docstrings. This is a string at the top of a function and/or module (within the function definition) that explains how the function works.

  See PEP 257: Docstring conventions for more details.

• Don't use file as a variable name; this is the name of a builtin function. Using the same variable name as a builtin is liable to be confusing, and potentially a source of subtle bugs.

• The Python convention (see PEP 8: Indentation) is that there are four spaces per indentation level, not 8.

• There is no need to have explicit exit() statements; the script will yield control flow naturally. If you really want to be explicit about leaving the function here, use return. Using exit() just makes this function harder to reuse.

• There's a common path for storing your files:

  /home/vhx/Documents/code/pydata_test/password_dbs/<filename>
  

  which appears file times in your code, with the only thing that changes being the filename. It would be better to pull the directory name into a variable, and then if it ever needs to change, you only need to do so in one place.