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;
Message-ID: <924a29d81cc7e0d3e2f62f693a0d8fcef97b9779.camel@linux.ibm.com>
Subject: Re: [PATCH v4] KEYS: encrypted: fix key instantiation with
 user-provided data
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Nikolaus Voss <nv@vosn.de>, 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>
Cc:     linux-integrity@vger.kernel.org, keyrings@vger.kernel.org,
        linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
Date:   Thu, 13 Oct 2022 22:10:54 -0400
In-Reply-To: <20221013064308.857011E25@mail.steuer-voss.de>
References: <20221013064308.857011E25@mail.steuer-voss.de>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
Precedence: bulk

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 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?

> 
> Fixes: cd3bc044af48 ("KEYS: encrypted: Instantiate key with user-provided decrypted data")
> Cc: stable <stable@kernel.org>
> Link: https://lore.kernel.org/ltp/20221006081709.92303897@mail.steuer-voss.de/
> Signed-off-by: Nikolaus Voss <nikolaus.voss@haag-streit.com>

Otherwise,

Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>