2
\$\begingroup\$

I have created a simple authentication script in python that can add users and provides a login function. I've tried to thow as much security into this as possible, which probably wasn't the best idea.

To my knowledge, the code works fine and withstands as much invalid input as I can find (although I haven't written any proper tests). I can see straight away that I've repeated myself quite a lot, but I'm not entirely sure how to cut it down.

Usage:

login(username, password)
Checks if this username/password combination is valid.
Returns bool

add_user(username, password)
Adds a user to the password file.
Returns bool


import hashlib, random, string

USERNAME_CHARACTERS=string.digits+string.ascii_letters
PASSWORD_CHARACTERS=string.digits+string.ascii_letters+string.punctuation

SALT_CHARACTERS="0123456789abcdefghijklmnopqrstuv"
SALT_CHARACTER_BITS=5
SALT_LENGTH=16
PEPPER_LENGTH=1

FILE_PATH="./shad"
FILE_SEPERATOR=":"

def int_to_string(num):
    string=""
    while num!=0:
        c=num%(2**SALT_CHARACTER_BITS)
        string+=SALT_CHARACTERS[c]
        num-=c
        num//=2**SALT_CHARACTER_BITS
    return string
    
def generate_new_salt():
     return int_to_string(random.getrandbits(SALT_CHARACTER_BITS*SALT_LENGTH))
def generate_new_pepper():
    return int_to_string(random.getrandbits(SALT_CHARACTER_BITS*PEPPER_LENGTH))
def generate_all_peppers():
    for i in range(1,SALT_CHARACTER_BITS*PEPPER_LENGTH):
        yield int_to_string(i)

def do_hash(string):
    return hashlib.sha3_256(string.encode()).hexdigest()

def get_user_data(username):
    with open(FILE_PATH) as h:
        data=h.read()
    for line in data.split("\n"):
        l=line.split(FILE_SEPERATOR)
        if l[0]==username:
            return {"user":l[0],
                    "pass":l[1],
                    "salt":l[2]}
    raise ValueError("Username not found: "+username)


def is_valid_username(username):
    for i in username:
        if not(i in USERNAME_CHARACTERS):
            return False
    return True

def is_valid_password(username):
    for i in username:
        if not(i in PASSWORD_CHARACTERS):
            return False
    return True

def add_user(username, password):
    if not(is_valid_username(username)):
        print("Invalid username")
        return False
    elif not(is_valid_password(password)):
        print("Invalid password")
        return False
    with open(FILE_PATH) as h:
        data=h.read()
    for line in data.split("\n"):
        l=line.split(FILE_SEPERATOR)
        if l[0]==username:
            print("Username already exists")
            return False
        
    salt=generate_new_salt()
    data=FILE_SEPERATOR.join([username, do_hash(generate_new_pepper()+salt+password), salt])+"\n"
    with open(FILE_PATH, "a") as h:
        h.write(data)
    return True

def login(username, password):
    if not(is_valid_username(username)):
        print("Invalid username")
        return False
    elif not(is_valid_password(password)):
        print("Invalid password")
        return False
    data=get_user_data(username)
    for pepper in generate_all_peppers():
        if do_hash(pepper+data["salt"]+password)==data["pass"]:
            return True
    return False
\$\endgroup\$
3
  • 4
    \$\begingroup\$ You are using an uniterated hash that can be a problem as it is too fast. Although the characters allowed in passwords are very limited (allow all Unicode points, no need to restrict it). You should also make sure that the login function uses a constant time method to compare the password authenticator string. \$\endgroup\$
    – eckes
    Sep 24, 2017 at 21:58
  • 1
    \$\begingroup\$ Homebrew, Hashing and security are never a good solution. There is a reason to use brypt, don't be a dave \$\endgroup\$
    – Ludisposed
    Sep 25, 2017 at 7:20
  • \$\begingroup\$ @eckes Can you put answers in answer boxes, thanks. \$\endgroup\$
    – Peilonrayz
    Sep 25, 2017 at 8:19

2 Answers 2

2
\$\begingroup\$

Cryptography

As others have mentioned, I would use bcrypt

Format

You have used good function names and variable names. However, you can add doc strings for each function.

def add_user(username, password):
    """"Function to add a user. Returns True for successful, or False for failure"""
    #Function code here

Additionally, you could improve your compliance with PEP8 (the Python Style Guide) by adding white-space around your equals symbols. This would greatly improve the readability of your code. Also, a more relaxed vertical spacing would improve the look of your code.

__main__

Not to sure if you want to create an default function for your code. But you could add (if __name__ == "__main__":)

\$\endgroup\$
1
\$\begingroup\$

Code style

You mostly stick to PEP8 regarding variable and function naming.
Though you might want to implement a bit more of vertical spacing between the functions.

Homebrew cryprography

As already metioned by other users in the comments, homebrew crypto is - most of the time - a bad idea.
Instead you should rely on proven algorithms and libraries such as the metioned bcrypt or Argon2, which was elected winner in the Password Hashing Competition 2015.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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