Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 14 Oct 2022 13:39:34 +0200 (CEST)
From:   Nikolaus Voss <nv@vosn.de>
To:     Mimi Zohar <zohar@linux.ibm.com>
cc:     David Howells <dhowells@redhat.com>,
        Jarkko Sakkinen <jarkko@kernel.org>,
        James Morris <jmorris@namei.org>,
        "Serge E. Hallyn" <serge@hallyn.com>, Yael Tzur <yaelt@google.com>,
        Cyril Hrubis <chrubis@suse.cz>, Petr Vorel <pvorel@suse.cz>,
        linux-integrity@vger.kernel.org, keyrings@vger.kernel.org,
        linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] KEYS: encrypted: fix key instantiation with user-provided
 data
In-Reply-To: <16fe5265c49fcecdf613fe9dd660efe4ae8d452e.camel@linux.ibm.com>
Message-ID: <1b621acf-a1f1-ec9b-21f6-d081d69ed74@vosn.de>
References: <20221013064308.857011E25@mail.steuer-voss.de>  <924a29d81cc7e0d3e2f62f693a0d8fcef97b9779.camel@linux.ibm.com>  <c620d6ed-d97f-b0c3-574-7b3cd63a7799@vosn.de> <16fe5265c49fcecdf613fe9dd660efe4ae8d452e.camel@linux.ibm.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-2142412134-1665747574=:33507"
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-2142412134-1665747574=:33507
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 8BIT

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
--8323329-2142412134-1665747574=:33507--