public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed
From: Adrian Vovk <adrianvovk@gmail.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Eric Biggers <ebiggers@kernel.org>,
	Md Sadre Alam <quic_mdalam@quicinc.com>,
	axboe@kernel.dk, song@kernel.org, yukuai3@huawei.com,
	agk@redhat.com, snitzer@kernel.org,
	Mikulas Patocka <mpatocka@redhat.com>,
	adrian.hunter@intel.com, quic_asutoshd@quicinc.com,
	ritesh.list@gmail.com, ulf.hansson@linaro.org,
	andersson@kernel.org, konradybcio@kernel.org, kees@kernel.org,
	gustavoars@kernel.org, linux-block@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-raid@vger.kernel.org,
	dm-devel@lists.linux.dev, linux-mmc@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-hardening@vger.kernel.org,
	quic_srichara@quicinc.com, quic_varada@quicinc.com
Subject: Re: [PATCH v2 1/3] dm-inlinecrypt: Add inline encryption support
Date: Thu, 24 Oct 2024 03:52:24 -0400	[thread overview]
Message-ID: <14126375-5F6F-484A-B34B-F0C011F3A9C5@gmail.com> (raw)
In-Reply-To: <Zxnl4VnD6K6No4UQ@infradead.org>



On October 24, 2024 2:14:57 AM EDT, Christoph Hellwig <hch@infradead.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

  reply	other threads:[~2024-10-24  7:52 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-16  8:57 [PATCH v2 0/3] Add inline encryption support Md Sadre Alam
2024-09-16  8:57 ` [PATCH v2 1/3] dm-inlinecrypt: " Md Sadre Alam
2024-09-17  5:05   ` kernel test robot
2024-09-17  6:38   ` kernel test robot
2024-09-18  5:08   ` kernel test robot
2024-09-21 18:55   ` Eric Biggers
2024-09-24  7:44     ` Christoph Hellwig
2024-09-24 22:04       ` Eric Biggers
2024-10-01  8:37         ` Christoph Hellwig
2024-10-18  3:26       ` Adrian Vovk
2024-10-18  5:22         ` Christoph Hellwig
     [not found]           ` <CAAdYy_mVy3uXPqWbjPzK_i8w7Okq73wKBQyc95TbnonE36rPgQ@mail.gmail.com>
2024-10-18  5:56             ` Christoph Hellwig
2024-10-18 15:03               ` Adrian Vovk
2024-10-23  6:57                 ` Christoph Hellwig
2024-10-24  2:52                   ` Adrian Vovk
2024-10-24  3:17                     ` Adrian Vovk
2024-10-24  6:14                     ` Christoph Hellwig
2024-10-24  7:52                       ` Adrian Vovk [this message]
2024-10-24  9:04                         ` Christoph Hellwig
2024-10-24 15:32                           ` Adrian Vovk
2024-10-24 15:59                             ` Christoph Hellwig
2024-10-24 16:23                               ` Adrian Vovk
2024-10-29 11:08                         ` Mikulas Patocka
2024-10-24  8:11                     ` Geoff Back
2024-10-24 15:28                       ` Adrian Vovk
2024-10-24 19:21                         ` John Stoffel
2024-10-24 20:45                           ` Adrian Vovk
2024-10-15 10:59   ` Mikulas Patocka
2024-09-16  8:57 ` [PATCH v2 2/3] mmc: cqhci: Add additional algo mode for inline encryption Md Sadre Alam
2024-09-16  8:57 ` [PATCH v2 3/3] mmc: sdhci-msm: " Md Sadre Alam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=14126375-5F6F-484A-B34B-F0C011F3A9C5@gmail.com \
    --to=adrianvovk@gmail.com \
    --cc=adrian.hunter@intel.com \
    --cc=agk@redhat.com \
    --cc=andersson@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=dm-devel@lists.linux.dev \
    --cc=ebiggers@kernel.org \
    --cc=gustavoars@kernel.org \
    --cc=hch@infradead.org \
    --cc=kees@kernel.org \
    --cc=konradybcio@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=mpatocka@redhat.com \
    --cc=quic_asutoshd@quicinc.com \
    --cc=quic_mdalam@quicinc.com \
    --cc=quic_srichara@quicinc.com \
    --cc=quic_varada@quicinc.com \
    --cc=ritesh.list@gmail.com \
    --cc=snitzer@kernel.org \
    --cc=song@kernel.org \
    --cc=ulf.hansson@linaro.org \
    --cc=yukuai3@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox