AES, is this method to say "The password you entered is wrong" secure? (TL;DR at the end)

Let's say the plaintext hello is encrypted with AES (CFB mode). I noticed that decryption works no matter the given password! Of course if the password is wrong, the output will be garbage, but no error will be thrown (example in Python):

import Crypto, Crypto.Random, Crypto.Hash, Crypto.Cipher.AES
p = b'hello'
iv = Crypto.Random.new().read(Crypto.Cipher.AES.block_size)
cipher = Crypto.Cipher.AES.new(Crypto.Hash.SHA256.new(b'password1').digest(),
                          Crypto.Cipher.AES.MODE_CFB, iv).encrypt(p)
print(Crypto.Cipher.AES.new(Crypto.Hash.SHA256.new(b'wrongpassword').digest(),
                          Crypto.Cipher.AES.MODE_CFB, iv).decrypt(cipher))
# b'\xfdQ\xd7\xa3\x1b'


I read that HMAC etc. can be used to ensure integrity, or AES in GCM mode. 

But to keep things simple, I wanted to know if this method is secure or not:

p = b'hello'
startingtag = b'504B0304'   # fixed constant, like "MZ" for DOS .exe files, or "%PDF-1.5" for PDF files
iv = Crypto.Random.new().read(Crypto.Cipher.AES.block_size)
cipher = Crypto.Cipher.AES.new(Crypto.Hash.SHA256.new(b'password1').digest(),
                          Crypto.Cipher.AES.MODE_CFB, iv).encrypt(startingtag + p)
s = Crypto.Cipher.AES.new(Crypto.Hash.SHA256.new(b'wrongpassword1').digest(),
                          Crypto.Cipher.AES.MODE_CFB, iv).decrypt(cipher)
if s[:len(startingtag)] == startingtag:
    print('ok good password, here is the plaintext:')
    print(s[len(startingtag):])
else:
    print('wrong password entered.')    


What does it do? A starting tag is encrypted along the plaintext. Then we check if the first bytes of the decrypted text matches this starting tag.

This is basically the method explained here. I had the same questionning than a comment there:



  if the first block of the plaintext is a known value, can't an attacker easily get the key from looking at the first block of the ciphertext? 

But on the other hand this seems true as well:


  Having a known header isn't a problem, otherwise encrypting something like a Microsoft Word document (that also has a known header) would be insecure.





TL;DR:


Is it secure to encrypt with AES CFB mode a plaintext for which the first bytes are known (e.g. %PDF-1.5 for a PDF document), 
or does this give too much information to an attacker? Then does this mean it's a bad to encrypt documents with known headers with AES?
Is it secure to encrypt with AES CFB mode a plaintext for which the first bytes are known (e.g. %PDF-1.5 for a PDF document)


Yes. Known (and even chosen) plaintext is a standard assumption in any moder cipher, including AES-CFB (and CTR, CBC, OFB...). It does not "give too much information to an attacker", and it is not "bad to encrypt documents with known headers with AES".

True, having a known plaintext at start to allow a test of the password makes password search a little simpler. But only a little, and we customarily ignore that little. One reason (stated in the question) is that actual plaintext typically contains recognizable sequences. And when not, it is typically compressible, when the incorrect plaintext obtained for the wrong password is not, which allows automated recognition of having the correct password anyway.

Another problem is that CFB is weak at detecting message alteration. Using a modern, authenticated encryption mode such as AES-GCM would solve that. It turns out this is readily available in PyCryptodome's Crypto.Cipher.

However, there is a different, serious problem with the code !

It has poor security because the Crypto.Hash.SHA256.new(b'password1') part of it builds the AES key from a password using a standard cryptographic hash (SHA-256). That conversion from password to key is fast, and that allows to test passwords at high speed. This makes the system very vulnerable to password crackers (which essentially test plausible passwords at high speed, including short ones, and those from or inspired from a dictionary of common passwords/words).


  I use a random Initialization Vector, so doesn't this help (against the above problem)?


No, because that does not appreciably slow down testing a password.

That issue has plagued password-based encryption since the origin of that, and progress in computing (faster CPUs, GPUs, FPGAs, ASICs..) directly worsen it, to the point that nowadays, very practically, if users can memorize a password, and a standard hash is used to turn it into a key, then the system is very weak (much to the pleasure of Three Letters Agencies and more casual/greedy attackers, which routinely use password crackers, and succeed).

In the 1990 we started to get iterated password-to-key derivation functions, like PBKDF2 of RFC&nbsp;2898, which essentially slow down conversion from password to key by requiring many hashes. Initially "many" was at least a thousand, but nowadays we'd need hundred thousands just to keep with the progress of technology. Until about 2015, that was the industrial state of the art (what your mobile phone or computer used).

We are slowly moving to memory-intensive (or memory-hard) password-to-key functions. The pioneer was bcrypt (which almost accidentally required sizable memory), then scrypt (theorizing the use of large memory and multiple CPUs when available). The Password Hashing Competition gave us Argon2 (which unfortunately fails to catch AFAIK, perhaps because it's complex and has many options). There is also Balloon (which does not catch either, perhaps because it was not in the PHC).

In any case, it is nowadays inadequate and grossly incompetent to turn a password into an encryption key using anything lesser than an iterated hash with many thousand iterations; and it is highly recommendable to additionally make that process require sizable memory.


  Could you include an example of code with which you would replace AES.new(SHA256.new(b'password1').digest(), AES.MODE_CFB, iv).encrypt(p) to make use of another hash?


Crypto.SE is not for code recommendations; and we need another kind of hash. One variously called entropy-stretching key derivation function, password-based hash, or some mix of that. It must be purposely as slow as possible for the attacker, yet fast enough for the application. Also, it will have a salt input, and it is a good idea to feed that with the IV/nonce, of at least server+user ID (this was missing in an earlier version of this answer).

The good news is that we no longer have to choose between several evils (see earlier version of this answer): PyCryptodome now has entropy-stretching key derivation functions, including scrypt which is memory-hard. This is the obvious choice. Be sure to up the N parameter as much as bearable.

Sample Python code there, illustrating how PyCryptodome's scrypt can turn a password into a key of AES-GCM, with GCM's nounce also scrypt's salt.

Caveat: nothing in this post should be construed as an endorsement of PyCryptodome's security.