Comment 5 for bug 1004845

Revision history for this message
Darsey Litzenberger (dlitz) wrote : Re: python-keyring doesn't work with python-crypto 2.6-1 (ValueError: IV must be 16 bytes long)

Hmm. It looks like python-keyring's CryptedFileKeyring uses weak cryptography, and your pull request effectively asks me to change PyCrypto in order to hide that fact. Obviously, I'm not going to do that, so I've rejected your patch.

This is a terrible way to initialize a cipher from a password:

    def _init_crypter(self):
        """Init the crypter(using the password of the keyring).
        """
        # check the password file
        if not self._check_file():
            self._init_file()

        password = self._getpass("Please input your password for the keyring")

        if not self._auth(password):
            sys.stderr.write("Wrong password for the keyring.\n")
            raise ValueError("Wrong password")

        # init the cipher with the password
        from Crypto.Cipher import AES
        # pad to _BLOCK_SIZE bytes
        password = password + (_BLOCK_SIZE - len(password) % _BLOCK_SIZE) * \
                                                                    _PADDING
        return AES.new(password, AES.MODE_CFB)

There's no salt, there's no iteration, and users can't even get the full 256 bits of entropy because their passphrases are limited to 32 printable characters. Digging further, it's trivial to determine the length of each password, and since there's no per-password salting, the storage format reveals which passwords are duplicates of each other.

This code looks like it was written by someone who is unfamiliar with password-based encryption. I suggest that anyone working on secure password-storage software should---at a minimum---read the PKCS#5 v2 standard: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-5v2/pkcs5v2_1.pdf

CryptedFileKeyring should store all the passwords together as a single ciphertext, with a unique salt generated every time the file is saved, and the key derived from the user's passphrase (which must not be limited to 32 characters) using a standard salted password-based key derivation function like PBKDF2. (PyCrypto now provides Crypto.Protocol.KDF.PBKDF2).