linux-kernel - Re: [PATCH v4] KEYS: encrypted: fix key instantiation with user-provided data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c620d6ed-d97f-b0c3-574-7b3cd63a7799@vosn.de>
Date:   Fri, 14 Oct 2022 08:40:10 +0200 (CEST)
From:   Nikolaus Voss <nv@...n.de>
To:     Mimi Zohar <zohar@...ux.ibm.com>
cc:     David Howells <dhowells@...hat.com>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>, Yael Tzur <yaelt@...gle.com>,
        Cyril Hrubis <chrubis@...e.cz>, Petr Vorel <pvorel@...e.cz>,
        linux-integrity@...r.kernel.org, keyrings@...r.kernel.org,
        linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4] KEYS: encrypted: fix key instantiation with user-provided
 data

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.

>
>> The corresponding test for the Linux Test Project ltp has also been
>> fixed (see link below).
>
> The LTP patch still needs to be revised, but the "Link" is a reference
> to the discussion.  Is the above statement necessary?

As long as the patch is not accepted the discussion is helpful. But feel 
free to delete it upon integration ;-).

>
>>
>> Fixes: cd3bc044af48 ("KEYS: encrypted: Instantiate key with user-provided decrypted data")
>> Cc: stable <stable@...nel.org>
>> Link: https://lore.kernel.org/ltp/20221006081709.92303897@mail.steuer-voss.de/
>> Signed-off-by: Nikolaus Voss <nikolaus.voss@...g-streit.com>
>
> Otherwise,
>
> Reviewed-by: Mimi Zohar <zohar@...ux.ibm.com>

Thanks Mimi!