Изучение ООП (python) и необходимость знать, использую ли я лучшие методы и концепции кодирования. Код рабочий

Привет. Приведенный ниже код является результатом моих попыток понять объектно-ориентированное программирование (ООП).

Я новичок в области программирования, поэтому, даже если это работает, это не означает, что я применяю правильные концепции и лучшие практики программирования, и это мои основные цели здесь.

Моя цель с этим кодом состояла в том, чтобы создать класс и вызвать его из других файлов .py.

Сам код работает, но я хотел бы знать, что можно улучшить или лучше реализовать.

Я написал другую программу только для вызова этого класса и его методов, и она сработала.

Я использую функции, которые будут работать только в Windows, потому что это упражнение, а не производственный код.

В моем коде есть только один класс (StudingClassFirstTime) и тринадцатый метод.

Он использует внешнюю библиотеку PyNaCl (https://pynacl.readthedocs.io/en/latest/)

Большое спасибо за ваше время и внимание.

import getpass # Allow user input a password without echo it on terminal
import os # To be able to use os.system('cls')
from nacl import pwhash, secret, utils, exceptions # Encrypt functions to be used


class StudingClassFirstTime:

    # Constructor mode: Here I will define the KDF (argon2id) parameters that will be used by default
    def __init__(self, kdf=pwhash.argon2id.kdf,
                 ops=pwhash.argon2id.OPSLIMIT_SENSITIVE,
                 mem=pwhash.argon2id.MEMLIMIT_SENSITIVE,
                 size=secret.SecretBox.KEY_SIZE):
        self.kdf = pwhash.argon2id.kdf
        self.ops = pwhash.argon2id.OPSLIMIT_SENSITIVE
        self.mem = pwhash.argon2id.MEMLIMIT_SENSITIVE
        self.size = secret.SecretBox.KEY_SIZE

    # Load an encryption key stored on file
    # This file should have only one encryption key inside it
    # If you have one file.key, you can encrypt as many files as you want, but, all of them will use the very same key
    # If you encrypt the same file twice with the same key, the output will not be equal thanks to NONCE
    def load_key(self, key_file):
        with open(key_file, 'rb') as my_key:
            key_of_file = my_key.read()
        return key_of_file

    # Generate an encryption key and return it's value(key)
    def generate_key(self):
        self.salt = utils.random(pwhash.argon2id.SALTBYTES) # 16 bytes == 128 bits
        password = self.password() # Ask a user to input a password
        key = self.kdf(self.size, password, self.salt, opslimit=self.ops, memlimit=self.mem)
        return key # Key have 32 bytes == 256 bits

    # Generate a raw 256 bits (32 bytes) as a key
    # Do not need any user password, or KDF function
    def generate_key_file(self):
        password = utils.random(32) # 32 bytes == 256 bits
        file_key = secret.SecretBox(password).encode()
        return file_key

    # Generate a encryption key and write it to a file
    def write_key_file(self, key_file):
        key = self.generate_key_file()
        with open(key_file, 'ab') as my_key:
            if os.path.getsize(key_file) > 0: # Check if the file have more than 0 bytes
                my_key.write(b'n' + key_file)   # If positive, first add a new line, then, write the key
            else:                             # If negative, just write the key, without add new line
                my_key.write(key_file)

    # Encrypt a message, generating an encryption key based on password.
    # The key will not be write to file.
    def encrypt(self, message):
        encrypt_key = self.generate_key()
        encrypt_box = secret.SecretBox(encrypt_key)
        encrypted = encrypt_box.encrypt(message)
        assert len(encrypted) == len(message) + encrypt_box.NONCE_SIZE + encrypt_box.MACBYTES  # NONCE_SIZE == 24 bytes // MACBYTES == 16 bytes
        return self.salt + encrypted

    # Encrypt a message with a key stored on file
    # To use it, first you should use generate_key_file() or already have a key contains your key.
    def encrypt_with_key(self, key_file, message):
        encrypt_key = self.load_key(key_file) # Load the encryption key stored on file
        encrypt_box = secret.SecretBox(encrypt_key)
        encrypted = encrypt_box.encrypt(message)
        assert len(encrypted) == len(message) + encrypt_box.NONCE_SIZE + encrypt_box.MACBYTES
        return encrypted

    # Decrypt a message, using a user supplied password
    def decrypt(self, message):
        try:
            password = getpass.getpass(prompt="nType password to DECRYPT: ", stream=None).encode() # Ask a user for the password to decrypt the message
            salt = message[:16] # The first 16 bytes of message [0 included : 16 not included] is the salt value
            decrypt_key = self.kdf(secret.SecretBox.KEY_SIZE, password, salt, opslimit=self.ops, memlimit=self.mem) # Derivate the enc. key from password
            decrypt_box = secret.SecretBox(decrypt_key)
            decrypted = decrypt_box.decrypt(message[16:])
            assert len(decrypted) == len(message[16:]) - decrypt_box.NONCE_SIZE - decrypt_box.MACBYTES
            return decrypted

        except exceptions.CryptoError:
            return 'error'

    # Decrypt a message with a key stored on file
    # To use it, first you should use encrypt_with_key() method.
    def decrypt_with_key(self, key_file, message):
        try:
            decrypt_key = self.load_key(key_file)
            decrypt_box = secret.SecretBox(decrypt_key)
            decrypted = decrypt_box.decrypt(message)
            assert len(decrypted) == len(message) - decrypt_box.NONCE_SIZE - decrypt_box.MACBYTES
            return decrypted

        except exceptions.CryptoError:
            return 'error'

    # Ask a user to type a password and verify it and return the password
    def password(self):
        while True:
            password1 = getpass.getpass(prompt="nType your password: ", stream=None).encode() # getpass do not echo the password on terminal
            password2 = getpass.getpass(prompt="Type your password again: ", stream=None).encode()
            if password1 != password2:
                input("nPassword does not match! Press any key to try again.")
                os.system('cls')
                continue
            else:
                return password1

    # Similar as write_key_file(), but, will not write the key on file.
    # This will be done by the function that call it.
    # The objective here is allow the caller (function) to store multiple keys in one single file, instead of, one file.key per encrypted file
    def key_db(self):
        key = self.generate_key_file()
        return key

    # Similar to encrypt_with_key(), but the key will be supplied.
    def encrypt_db(self, encrypt_key_db, message):
        encrypt_box = secret.SecretBox(encrypt_key_db)
        encrypted = encrypt_box.encrypt(message)
        assert len(encrypted) == len(message) + encrypt_box.NONCE_SIZE + encrypt_box.MACBYTES
        return salt + encrypted

    # The inverse of encrypt_db()
    def decrypt_db(self, key_db, message):
        try:
            decrypt_box = secret.SecretBox(bytes.fromhex(key_db))
            decrypted = decrypt_box.decrypt(message)
            assert len(decrypted) == len(message) - decrypt_box.NONCE_SIZE - decrypt_box.MACBYTES
            return decrypted

        except exceptions.CryptoError:
            return 'error'

'''
The first 56 bytes of encrypted text are:
[0:16] = SALT (16) # Will be present only if the user supply a password
[16:40] = NONCE (24) # Always present
[40:56] = MACBYTES (16) # Always present
[56:] = Encrypted Text
'''

3 ответа
3

Короткие комментарии в виде строк документации

Строки документации — это строки в тройных кавычках, которые появляются сразу после того, как вы определяете класс или функцию, и они особенные в том смысле, что Python может сказать, что эти строки должны рассматриваться как комментарии, которые документ код.

Для большинства ваших методов у вас есть короткий комментарий непосредственно перед функцией, и это здорово. Я предлагаю переместить его «внутрь» функции.

Например, вместо

    # Generate an encryption key and return it's value(key)
    def generate_key(self):
        self.salt = utils.random(pwhash.argon2id.SALTBYTES) # 16 bytes == 128 bits
        password = self.password() # Ask a user to input a password
        key = self.kdf(self.size, password, self.salt, opslimit=self.ops, memlimit=self.mem)
        return key # Key have 32 bytes == 256 bits

делать

    def generate_key(self):
        """Generate an encryption key and return it's value(key)."""
        self.salt = utils.random(pwhash.argon2id.SALTBYTES) # 16 bytes == 128 bits
        password = self.password() # Ask a user to input a password
        key = self.kdf(self.size, password, self.salt, opslimit=self.ops, memlimit=self.mem)
        return key # Key have 32 bytes == 256 bits

    Напишите как можно меньше кода внутри try

    Я вижу, что вы пишете часть своего кода в стиле EAFP (проще просить прощения, чем разрешения), а не в стиле LBYL (посмотрите, прежде чем прыгать). Это означает, что вы используете try: ... except: ... блоков, что прекрасно и часто поощряется в Python (статья о EAFP против LBYL в Python).

    Однако, когда вы пишете код EAFP, рекомендуется писать как можно меньше кода внутри try утверждение. Идея в том, что мы не хотим except чтобы поймать ошибку, отличную от той, которую мы пытаемся предотвратить. Итак, хотя я не знаком с используемыми вами библиотеками, вы, вероятно, можете взять что-то вроде

        # The inverse of encrypt_db()
        def decrypt_db(self, key_db, message):
            try:
                decrypt_box = secret.SecretBox(bytes.fromhex(key_db))
                decrypted = decrypt_box.decrypt(message)
                assert len(decrypted) == len(message) - decrypt_box.NONCE_SIZE - decrypt_box.MACBYTES
                return decrypted
    
            except exceptions.CryptoError:
                return 'error'
    

    и перепишите его как

        # The inverse of encrypt_db()
        def decrypt_db(self, key_db, message):
            try:
                decrypt_box = secret.SecretBox(bytes.fromhex(key_db))
                decrypted = decrypt_box.decrypt(message)
            except exceptions.CryptoError:
                return 'error'
    
            assert len(decrypted) == len(message) - decrypt_box.NONCE_SIZE - decrypt_box.MACBYTES
            return decrypted
    

    Теперь, в зависимости от того, какая линия может действительно поднять CryptoError, вы бы дополнительно изменили эту часть кода.

      Сначала давайте обсудим именование классов и методов:

      StudingClassFirstTime Прежде всего, слово Studing нет в моем словаре. Но, пожалуйста, просто назовите классы после того, что они должны делать, и не включайте слово class; мы знаем, что это класс.

      • load_key: это загружает 32-байтовый ключ из файла, что нормально, но не включает file в идентификаторе, как в более поздних методах;
      • generate_key: извлекает ключ из пароля; это не то же самое, что генерация ключа, что в криптографии обычно означает получение байтов из генератора случайных чисел;
      • generate_key_file: здесь предлагается файл, но файл не создается;
      • write_key_file: здесь файл ключей (или, как говорилось ранее, содержимое) создается и шифруется первым, чего нет в описании метода;
      • encrypt: Хорошо, хорошо, здесь вы выполняете шифрование, которое всегда использует ключ, однако параметр ключа отсутствует — вы просто предполагаете, что читатель ожидает, что ключ будет сгенерирован;
      • encrypt_with_key: можно было бы предположить, что это плеоназм — ключ нужен, чтобы что-либо зашифровать;
      • password: пароль — это не глагол, поэтому его следует называть ask_user_for_password например.
      • key_db: аналогичная проблема, что и выше, без глагола — требуемые обширные комментарии показывают, что это не очень хорошо продуманное имя;
      • encrypt_db: напрашивается вопрос: какая БД, мы где-то определяли БД?
      • decrypt_db: Хорошо, но комментарий здесь не обязателен, все должны знать, что дешифрование — это обратное шифрование.

      Для write_key_file это не симметрично с load_key чего обычно и следовало ожидать.

      Для encrypt_db, БД не присутствует в вызове метода, вместо этого есть параметр message, который мы должны догадаться, это БД. Более того, encrypt_key_db мне кажется, что существует база данных ключей, а не то, что она представляет собой ключ для шифрования БД.

      Единственная разница между encrypt_db и encrypt_with_key похоже ключ в файле или нет. Это не отражается в API.


      Теперь давайте поговорим о том, как структурирована программа / класс. Кажется, он делает много вещей одновременно. Во-первых, он сочетает в себе ввод-вывод файлов, ввод пользователя, очистку терминала, а также «бизнес-логику» выполнения шифрования / дешифрования и генерации ключей из паролей.

      Обычно мы стараемся максимально разделять эти три. Теперь вы определенно хотите сделать пользовательский интерфейс бизнес-логики зависимым. Это именно то, что делает класс при вызове getpass.getpass из бизнес-логики. Это можно было бы назвать «обращением контроля». Обычно существует класс контроллера, который выполняет бизнес-логику, которая, в свою очередь, контролируется классом пользовательского интерфейса. В лучшем случае класс контроллера позволяет классу пользовательского интерфейса вставлять функции обратного вызова (например, для обновления индикатора выполнения). Вместо этого класс пользовательского интерфейса или основной метод настроит пароль с помощью вызова метода или конструктора. Это делает код пригодным для повторного использования, например, для приложений с графическим интерфейсом пользователя вместо консольных приложений.


      Найти Argon для хеширования паролей — это, конечно, нормально и хороший выбор.


      Мне как пользователю непонятно, для чего предназначен этот класс. Да, он шифрует данные, но о чем это мне говорит? Зачем его использовать для шифрования, шифрования с помощью ключа или шифрования БД? Как правило, вы должны сосредоточиться только на одном «варианте использования», например, на шифровании БД.

      Как правило, программисты стремятся предоставить пользователю один интерфейс, когда дело доходит до написания одного или нескольких элементов. Наличие для этого отдельной функции, которая делает вещи немного по-другому, просто сбивает с толку. Один из способов избежать этого — иметь класс, специфичный для (защищенного паролем) ключа, который затем можно использовать для шифрования отдельных элементов, составляющих БД, с помощью отдельных вызовов методов.

      К сожалению, вариант использования / требования для этого класса достаточно неясны, чтобы я мог предложить для него лучший API.


      Что касается криптографии:

      • всегда указывайте протокол в описании класса; если у вас нет отдельного описания протокола, вам нужно будет указать его здесь;
      • ключ и пароль — это разные сущности: password = getpass.getpass хорошо, но password = utils.random(32) нет (32 случайных байта не читаются человеком и, следовательно, не являются паролем).

      Добавить комментарий

      Ваш адрес email не будет опубликован. Обязательные поля помечены *