Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 3/4] dm crypt: switch to EBOIV crypto API template
To:     Eric Biggers <ebiggers@kernel.org>,
        Milan Broz <gmazyland@gmail.com>
Cc:     Gilad Ben-Yossef <gilad@benyossef.com>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        "David S. Miller" <davem@davemloft.net>,
        Alasdair Kergon <agk@redhat.com>,
        Mike Snitzer <snitzer@redhat.com>, dm-devel@redhat.com,
        Song Liu <song@kernel.org>, Ofir Drang <ofir.drang@arm.com>,
        linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-raid@vger.kernel.org
References: <20201026130450.6947-1-gilad@benyossef.com>
 <20201026130450.6947-4-gilad@benyossef.com>
 <20201026175231.GG858@sol.localdomain>
 <d07b062c-1405-4d72-b907-1c4dfa97aecb@gmail.com>
 <20201026183936.GJ858@sol.localdomain>
From:   Milan Broz <gmazyland@gmail.com>
Message-ID: <fd5e46ce-a4bf-8025-05ea-e20d35485446@gmail.com>
Date:   Mon, 26 Oct 2020 20:04:13 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.4.0
MIME-Version: 1.0
In-Reply-To: <20201026183936.GJ858@sol.localdomain>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 26/10/2020 19:39, Eric Biggers wrote:
> On Mon, Oct 26, 2020 at 07:29:57PM +0100, Milan Broz wrote:
>> On 26/10/2020 18:52, Eric Biggers wrote:
>>> On Mon, Oct 26, 2020 at 03:04:46PM +0200, Gilad Ben-Yossef wrote:
>>>> Replace the explicit EBOIV handling in the dm-crypt driver with calls
>>>> into the crypto API, which now possesses the capability to perform
>>>> this processing within the crypto subsystem.
>>>>
>>>> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>>>>
>>>> ---
>>>>  drivers/md/Kconfig    |  1 +
>>>>  drivers/md/dm-crypt.c | 61 ++++++++++++++-----------------------------
>>>>  2 files changed, 20 insertions(+), 42 deletions(-)
>>>>
>>>> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
>>>> index 30ba3573626c..ca6e56a72281 100644
>>>> --- a/drivers/md/Kconfig
>>>> +++ b/drivers/md/Kconfig
>>>> @@ -273,6 +273,7 @@ config DM_CRYPT
>>>>  	select CRYPTO
>>>>  	select CRYPTO_CBC
>>>>  	select CRYPTO_ESSIV
>>>> +	select CRYPTO_EBOIV
>>>>  	help
>>>>  	  This device-mapper target allows you to create a device that
>>>>  	  transparently encrypts the data on it. You'll need to activate
>>>
>>> Can CRYPTO_EBOIV please not be selected by default?  If someone really wants
>>> Bitlocker compatibility support, they can select this option themselves.
>>
>> Please no! Until this move of IV to crypto API, we can rely on
>> support in dm-crypt (if it is not supported, it is just a very old kernel).
>> (Actually, this was the first thing I checked in this patchset - if it is
>> unconditionally enabled for compatibility once dmcrypt is selected.)
>>
>> People already use removable devices with BitLocker.
>> It was the whole point that it works out-of-the-box without enabling anything.
>>
>> If you insist on this to be optional, please better keep this IV inside dmcrypt.
>> (EBOIV has no other use than for disk encryption anyway.)
>>
>> Or maybe another option would be to introduce option under dm-crypt Kconfig that
>> defaults to enabled (like support for foreign/legacy disk encryption schemes) and that
>> selects these IVs/modes.
>> But requiring some random switch in crypto API will only confuse users.
> 
> CONFIG_DM_CRYPT can either select every weird combination of algorithms anyone
> can ever be using, or it can select some defaults and require any other needed
> algorithms to be explicitly selected.
> 
> In reality, dm-crypt has never even selected any particular block ciphers, even
> AES.  Nor has it ever selected XTS.  So it's actually always made users (or
> kernel distributors) explicitly select algorithms.  Why the Bitlocker support
> suddenly different?
> 
> I'd think a lot of dm-crypt users don't want to bloat their kernels with random
> legacy algorithms.

Yes, but IV is in reality not a cryptographic algorithm, it is kind-of a configuration
"option" of sector encryption mode here.

We had all of disk-IV inside dmcrypt before - but once it is partially moved into crypto API
(ESSIV, EBOIV for now), it becomes much more complicated for user to select what he needs.

I think we have no way to check that IV is available from userspace - it
will report the same error as if block cipher is not available, not helping user much
with the error.

But then I also think we should add abstract dm-crypt options here (Legacy TrueCrypt modes,
Bitlocker modes) that will select these crypto API configuration switches.
Otherwise it will be only a complicated matrix of crypto API options...

Milan