On Fri, 2022-10-14 at 13:39 +0200, Nikolaus Voss wrote: > 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. Agreed the latter is better solution. Please update/replace the sentence "It is encouraged to switch to a new key because ..." based on this more complete explanation. thanks, Mimi