Message-ID: <14126375-5F6F-484A-B34B-F0C011F3A9C5@gmail.com>
Date: Thu, 24 Oct 2024 03:52:24 -0400
From: Adrian Vovk <adrianvovk@...il.com>
To: Christoph Hellwig <hch@...radead.org>
CC: Eric Biggers <ebiggers@...nel.org>, Md Sadre Alam <quic_mdalam@...cinc.com>,
 axboe@...nel.dk, song@...nel.org, yukuai3@...wei.com, agk@...hat.com,
 snitzer@...nel.org, Mikulas Patocka <mpatocka@...hat.com>,
 adrian.hunter@...el.com, quic_asutoshd@...cinc.com, ritesh.list@...il.com,
 ulf.hansson@...aro.org, andersson@...nel.org, konradybcio@...nel.org,
 kees@...nel.org, gustavoars@...nel.org, linux-block@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-raid@...r.kernel.org,
 dm-devel@...ts.linux.dev, linux-mmc@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, linux-hardening@...r.kernel.org,
 quic_srichara@...cinc.com, quic_varada@...cinc.com
Subject: Re: [PATCH v2 1/3] dm-inlinecrypt: Add inline encryption support



On October 24, 2024 2:14:57 AM EDT, Christoph Hellwig <hch@...radead.org> wrote:
>On Wed, Oct 23, 2024 at 10:52:06PM -0400, Adrian Vovk wrote:
>> > Why do you assume the encryption would happen twice?
>> 
>> I'm not assuming. That's the behavior of dm-crypt without passthrough.
>> It just encrypts everything that moves through it. If I stack two
>> layers of dm-crypt on top of each other my data is encrypted twice.
>
>Sure.  But why would you do that?

As mentioned earlier in the thread: I don't have a usecase specifically for this and it was an example of a situation where passthrough is necessary and no filesystem is involved at all. Though, as I also pointed out, a usecase where you're putting encrypted virtual partitions on an encrypted LVM setup isn't all that absurd.

In my real-world case, I'm putting encrypted loop devices on top of a filesystem that holds its own sensitive data. Each loop device has dm-crypt inside and uses a unique key, but the filesystem needs to be encrypted too (because, again, it has its own sensitive data outside of the loop devices). The loop devices cannot be put onto their own separate partition because there's no good way to know ahead of time how much space either of the partitions would need: sometimes the loop devices need to take up loads of space on the partition, and other times the non-loop-device data needs to take up that space. And to top it all off, the distribution of allocated space needs to change dynamically.

The current Linux kernel does not support this use-case without double encryption. The loop devices are encrypted once with their own dm-crypt instance. Then that same data is encrypted a second time over by the partition.

Actually this scenario is simplified. We actually want to use fscrypt inside of the loopback file too. So actually, without the passthrough mechanism some data would be encrypted three distinct times.

>> > No one knows that it actually is encryped.  The lower layer just knows
>> > the skip encryption flag was set, but it has zero assurance data
>> > actually was encrypted.
>> 
>> I think it makes sense to require that the data is actually encrypted
>> whenever the flag is set. Of course there's no way to enforce that
>> programmatically, but code that sets the flag without making sure the
>> data gets encrypted some other way wouldn't pass review.
>
>You have a lot of trusted in reviers. But even that doesn't help as
>the kernel can load code that never passed review.

Ultimately, I'm unsure what the concern is here.

It's a glaringly loud opt-in marker that encryption was already performed or is otherwise intentionally unnecessary. The flag existing isn't what punches through the security model. It's the use of the flag that does. I can't imagine anything setting the flag by accident. So what are you actually concerned about? How are you expecting this flag to actually be misused?

As for third party modules that might punch holes, so what? 3rd party modules aren't the kernel's responsibility or problem

>> Alternatively, if I recall correctly it should be possible to just
>> check if the bio has an attached encryption context. If it has one,
>> then just pass-through. If it doesn't, then attach your own. No flag
>> required this way, and dm-default-key would only add encryption iff
>> the data isn't already encrypted.
>
>That at least sounds a little better. 

Please see my follow up. This is actually not feasible because it doesn't work. Sometimes, fscrypt will just ask to move encrypted blocks around without providing an encryption context; the data doesn't need to be decrypted to be reshuffled on disk. This flag-less approach I describe would actually just break: it it would unintentionally encrypt that data during shuffling.

> But it still doesn't answer
>why we need this hack instead always encrypting at one layer instead
>of splitting it up.

In my loopback file scenario, what would be the one layer that could handle the encryption?

- the loopback files are just regular files that happen to have encrypted data inside of them. Doing it a different way changes the semantics: with a loopback file, I'm able to move it into a basic FAT filesystem and back without losing the encryption on the data

- the filesystem is completely unaware of any encryption. The loopback files are just files with random content inside. The filesystem itself is encrypted from below by the block layer. So, there's nothing for it to do.

- the underlying instance of dm-crypt is encrypting a single opaque blob of data, and so without explicit communication from above it cannot possibly know how to handle this.

Thus, I don't see a single layer that can handle this. The only solution is for upper layers to communicate downward.

Best,
Adrian 

Powered by blists - more mailing lists