3
\$\begingroup\$

I simply made this for practice. But I want to know if it can be used in the realworld or not. Please point out any bad programming practices and how I can change them and look out for them next time. Your advice or tip will be the invaluable rain on my deadly isolated programming land.

import os
import hashlib
from base64 import b64encode, b64decode


def convert_to_bytes(value, value_name):
    if isinstance(value, str):
        return value.encode('UTF-8')
    else:
        raise ValueError('%s must be string or bytes' % value_name)


def derive_passhash(password, hash_name='sha512', salt=None, iterations=100000):
    '''This function returns str type password_hash.
    hash contains hash_name, iterations, salt, derived_key
    -> [hash_name] $ [iterations] $$ [salt] $s_k$ [derived_key] '''
    if salt is None:
        salt = os.urandom(32)
    if not isinstance(salt, bytes):
        salt = convert_to_bytes(salt, 'salt')
    if not isinstance(password, bytes):
        password = convert_to_bytes(password, 'password')
    if not isinstance(iterations, int):
        if isinstance(iterations, str) and iterations.isnumeric():
            iterations = int(iterations)
        else:
            print('iterations must be integer.')
            raise ValueError
    # derive passhash
    try:
        d_key = hashlib.pbkdf2_hmac(hash_name, password, salt, iterations)
        del password    # maybe it can help to make more secure?
    except ValueError as error:
        print('[!] Error cused %s' % error)
        # log(error)

    # put hash_name, salt, iterations and derived key together
    # and encode to base64 format.
    pre_v = '$'.join((hash_name, str(iterations))).encode('utf-8')
    end_v = b'$s_k$'.join((salt, d_key))
    total_value = b'$$'.join((pre_v, end_v))

    user_hash = b64encode(total_value).decode(encoding='utf-8')
    return user_hash


def check_password(user_hash, asserted_pw):
    if not isinstance(asserted_pw, bytes):
        asserted_pw = convert_to_bytes(asserted_pw, 'password')

    user_hash = b64decode(user_hash.encode('utf-8'))

    h_i, s_k = user_hash.split(b'$$')
    hash_name, iterations = h_i.decode('utf-8').split('$')
    salt, key = s_k.split(b'$s_k$')

    try:
        asserted_key = hashlib.pbkdf2_hmac(hash_name, asserted_pw,
                                           salt, int(iterations))
        del asserted_pw
    except ValueError as error:
        print('[!] Error cused %s' % error)

    if key == asserted_key:
        return True
    else:
        return False
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Typo

Error cused -> Error caused

Memory security

del password    # maybe it can help to make more secure?

It's not harmful, but it probably also won't help much. Your theory seems to be that if an outside attacker gains access to your program's memory (which is already instant doom), that calling del will have removed the plaintext password from memory thus reducing the impact of an attack. Have a read through an explanatory article like Python Garbage Collector, which says that

you can use the del statement that removes a variable and its reference (not the object itself) [...] The del statement removes the references to our objects (i.e., decreases reference count by 1). After Python executes the del statement, our objects are no longer accessible from Python code. However, such objects are still sitting in the memory

del does not force garbage collection. To do that, you would in theory need to call gc.collect().

The most that del would immediately do is mitigate an attack on access to the locals dictionary. Again, if such an attack is possible, you have much bigger problems on your hands.

Boolean expressions

if key == asserted_key:
    return True
else:
    return False

can be

return key == asserted_key
\$\endgroup\$

Your Answer

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

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