My observations are as follows:
- Storing the value of
l is pointless. You should store
n instead. You should be aware that PBKDF2 doesn't produce different sequences for different values of
l, but rather extends the sequence longer.
- Generating 1024 bytes for
k and 512 bytes for
P seems odd, since AES accepts no larger than a 256-bit (32 byte) key.
- In step 5 of the encryption, you say that
k is encrypted using AES-CBC, but do not mention how the IV is selected. This isn't hugely important here, since
k won't have duplicate blocks, but it's good practice to use the cipher mode properly.
- In step 5 of the decryption, you again mention AES-CBC without specifying the IV. This is critical - improperly selecting the IV may completely break the system.
- You provide no authenticity on the file or the stored metadata. This is actually important, because AES-CBC is malleable (i.e. ciphertext can be modified in ways that affect the plaintext, without needing to know the key) and your construction could be particularly susceptible.
Your general construction is actually alright, but it's missing the details that separate a secure construction from a weak one.
Here's how I'd do it:
- let p be the user's password.
- let kd (data key) be a random 128-bit value.
- let s (salt) be a random be a random 128-bit value.
- let c be the iteration count, e.g. 1,000,000.
- let km (master key) and ka (authenticity key) be computed as two 16 byte (128-bit) halves of PBKDF2(p, s, c, 32), i.e. PBKDF2 of the user password and salt, with the defined iteration count, and an output length of 32 bytes.
- let k'd (encrypted data key) be computed as AES(kd, km), using 128-bit AES in ECB mode.
- let IV be the initialisation vector for use when encrypting the file data.
- let Q (metadata) be the concatenated values s | c | k'd | IV.
- let aQ be the authenticity record for Q, defined as H(Q, ka), where H is a cryptographic hash function in a HMAC construction, e.g. HMAC-SHA256.
- store Q and aQ in the file header.
Since 128-bit keys are being used, and AES has a block size of 128 bits, only one block need be encrypted when computing k'd and therefore AES in ECB mode is safe; the additional complexity of using CBC or another mode is unnecessary.
Providing an authenticity record for the metadata helps prevent an attacker from "tweaking" the parameters needed to decrypt the file. Most critically, it provides authenticity of the IV, which is a prime target for modification as xor'ing it with a value will result in the first block of plaintext also being xor'ed with that value upon decryption. This property is known as malleability. For partially known (e.g. structured) plaintexts, this can have devastating effects.
You should be able to extrapolate the rest of the file encryption and decryption process from here. One addition I would make is to compute a HMAC hash of the encrypted data, then store that in the header, to ensure authenticity of the encrypted data.