Messages in this thread | | | Date | Fri, 14 Oct 2022 13:39:34 +0200 (CEST) | From | Nikolaus Voss <> | Subject | Re: [PATCH v4] KEYS: encrypted: fix key instantiation with user-provided data |
| |
On Fri, 14 Oct 2022, Mimi Zohar wrote: > On Fri, 2022-10-14 at 08:40 +0200, Nikolaus Voss wrote: >> On Thu, 13 Oct 2022, Mimi Zohar wrote: >>> On Thu, 2022-10-13 at 08:39 +0200, Nikolaus Voss wrote: >>>> Commit cd3bc044af48 ("KEYS: encrypted: Instantiate key with user-provided >>>> decrypted data") added key instantiation with user provided decrypted data. >>>> The user data is hex-ascii-encoded but was just memcpy'ed to the binary buffer. >>>> Fix this to use hex2bin instead. >>>> >>>> Old keys created from user provided decrypted data saved with "keyctl pipe" >>>> are still valid, however if the key is recreated from decrypted data the >>>> old key must be converted to the correct format. This can be done with a >>>> small shell script, e.g.: >>>> >>>> BROKENKEY=abcdefABCDEF1234567890aaaaaaaaaa >>>> NEWKEY=$(echo -ne $BROKENKEY | xxd -p -c32) >>>> keyctl add user masterkey "$(cat masterkey.bin)" @u >>>> keyctl add encrypted testkey "new user:masterkey 32 $NEWKEY" @u >>>> >>>> It is encouraged to switch to a new key because the effective key size >>>> of the old keys is only half of the specified size. >>> >>> Both the old and new decrypted data size is 32 bytes. Is the above >>> statement necessary, especially since the Documentation example does >>> the equivalent? >> >> The old key has the same byte size but all bytes must be within the >> hex-ascíi range of characters, otherwise it is refused by the kernel. >> So if you wanted a 32 bytes key you get 16 effective bytes for the key. >> In the above example the string size of the $BROKENKEY is 32, while >> the string size of the $NEWKEY is 64. >> >> If you do >> >> $ echo $NEWKEY >> 6162636465664142434445463132333435363738393061616161616161616161 >> >> for the example, the range problem is obvious, so $NEWKEY is still broken. >> That's why it should only be used to recover data which should be >> reencypted with a new key. If you count exactly, the effective key size is >> _slightly_ longer than half of the specified size, but it is still a >> severe security problem. > > So the issue with NEWKEY isn't the "effective key size of the old keys > is only half of the specified size", but that the old key, itself, is > limited to the hex-ascii range of characters.
The latter resulting in the former. If for BROKENKEY 32 bytes were specified, a brute force attacker knowing the key properties would only need to try at most 2^(16*8) keys, as if the key was only 16 bytes long. This is what I mean with "effective size" in contrast to the key's byte size which is 32 in my example.
The security issue is a result of the combination of limiting the input range to hex-ascii and using memcpy() instead of hex2bin(). It could have been fixed either by allowing binary input or using hex2bin() (and doubling the ascii input key length). I chose the latter.
Niko | |