Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Michael Halcrow]

> Eric,
> 
> > On 04.10.2016, at 18:38, Eric Biggers <ebiggers@google.com> wrote:
> > 
> > On Tue, Oct 04, 2016 at 10:46:54AM +0200, Richard Weinberger wrote:
> >>> Also, currently this code *is* only supposed to be used for XTS.
> >>> There's a bug where a specially crafted filesystem can cause
> >>> this code path to be entered with CTS, but I have a patch
> >>> pending in the ext4 tree to fix that.
> >> 
> >> David and I are currently working on UBIFS encryption and we have
> >> to support other cipher modes than XTS. So, keeping fscrypto as
> >> generic as possible would be nice. :-)
> >> 
> > 
> > The problem was that the kernel supported reading a file whose
> > contents was encrypted with CTS, which is only supposed to be used
> > for filenames.  This was inconsistent with
> > FS_IOC_SET_ENCRYPTION_POLICY which currently only allows XTS for
> > contents and CTS for filenames.  So in other words I wanted to
> > eliminate a strange scenario that was not intended to happen and
> > was almost certainly never tested.
> > 
> > Either way, new modes can still be added if there is a good reason
> > to do so.  What new encryption modes are you thinking of adding,
> > would they be for contents or for filenames, and are you thinking
> > they would be offered by all filesystems (ext4 and f2fs too)?
> 
> We currently have one case where our embedded platform is only able
> to do AES-CBC in hardware, not AES-XTS. So switching to AES-CBC for
> file contents would yield far better performance while still being
> "secure enough".

Great to see more interest in file system encryption.  A few thoughts.

I'm concerned about the proliferation of storage encryption code in
the kernel.  Of course, I'm perhaps the worst instigator.  However
what's happening now is that we have several file systems that are
proposing their own encryption, as well as several attempts at support
for hardware encryption.

High-performance random access read/write block storage encryption
with authentication is hard to get right.  The way I see it, the ideal
solution would have these properties:

 * The actual cryptographic transform happens in as few places as
   possible -- preferably one place in software, with a sensible
   vendor-neutral API for defering to hardware.

 * All blocks in the file system, including both file contents and
   file system metadata, are cryptographically protected.

 * Encryption is authenticated and has versioning support to enforce
   consistency and defend against rollback.

 * File systems can select which keys protect which blocks.

 * Authentication of all storage chains back to Secure Boot.

To solve all of these simultaneously, it looks like we'll want to
consider changes to the kernel block API:


diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 436f43f..de3492a 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -19,6 +19,20 @@ typedef void (bio_end_io_t) (struct bio *);
 typedef void (bio_destructor_t) (struct bio *);
 
 #ifdef CONFIG_BLOCK
+
+#ifdef CONFIG_BLK_CRYPT
+struct bio_crypt_ctx {
+	unsigned int bc_flags;	/* Indicates union interpretation */
+	union {
+		struct key *bc_key;
+		unsigned int bc_key_type;	/* Implies size */
+		unsigned int bc_key_size;
+	};
+	atomic_t __bc_cnt;	/* Pin count */	
+	u8 bc_key[0];
+};
+#endif
+
 /*
  * main unit of I/O for the block layer and lower layers (ie drivers and
  * stacking drivers)
@@ -81,6 +95,10 @@ struct bio {
 
 	struct bio_set		*bi_pool;
 
+#ifdef CONFIG_BLK_CRYPT
+	struct bio_crypt_ctx	*bi_crypt_ctx;
+#endif
+
 	/*
 	 * We can inline a number of vecs at the end of the bio, to avoid
 	 * double allocations for a small number of bio_vecs. This member


>From here, we can delegate to dm-crypt to perform the block
transformation using the key in the bio.  Or we can defer to the block
storage driver to provision the key into the hardware encryption
element and tag requests to use that key.

This promises to get a big chunk of the file contents encryption logic
out of the file system layer.

If the file system doesn't provide a bi_crypt_ctx, then dm-crypt can
use the default key, which would be shared among all tenants of the
system.  That shared key can potentially be further protected by the
distro by leveraging a secure element like a TPM.

For user-specific file contents -- say, what's in the user's home
directory -- then that can be protected with a key that's only made
available after the user logs in (providing their credentials).  Other
tenants on the same device who can get at the shared key might still
get information like how many files other users have or what the
directory structure is, but at least they can't read the contents of
other users' files.  Meanwhile, the volume is comprehensively
protected against the "left in a taxi" scenario.

> Generally speaking though, it would be great to have encryption
> _and_ authentication for file contents.

Not good enough for me.  I want authenticated encryption for
everything, contents or metadata.

> AEAD modes like GCM or future finalists of the CAESAR competition
> come to mind.

GCM is problematic for block storage, primarily because it's
catastrophic to reuse a key/IV pair.

If you naively use the same key when writing more than 2^32 blocks
with a random IV, you've just stepped into the collision "danger
zone" (per NIST SP 800-38D).  We have a design that involves frequent
encryption key derivation in order to address the collision space
problem.  But that's just one piece of the solution to the whole
problem.

> IIRC the ext4 encryption design document mentions this, but it's
> unclear to me why AES-GCM wasn't used for file contents from the
> beginning. I'd guess it has to do with where to store the
> authentication tag and performance.

Comparatively, that's the easy part.  The hard part is ensuring
*consistency* between the ciphertext and the cryptographic metadata.
If you write out the ciphertext and don't get the IV you used for it
out to storage simultaneously, you've just lost the block.  And
vice-versa.

Then there's the problem of internal consistency.  Supposing you do
manage to get the blocks and their crypto metadata out together,
what's to stop an attacker from punching holes (for example)?  You
need an authenticated dictionary structure at that point, such as a
Merkle tree or an authenticated skiplist.

Now you have an additional data structure to maintain.  And you're
rebalancing a Merkle tree in the midst of modifications, or you're
producing an implementation of ASL in the Linux kernel (which, BTW, my
team does have a prototype for right now).

Once we have a root of an authenticated dictionary, we can look to a
high-performance secure hardware element to sign that root against a
monotonic counter to get rollback protection.

To protect the entire block device, we need the authentication data to
be consistent with the ciphertext at the block level.  So that means
something like copy-on-write or log-structured volume at the dm-
layer.  Right now the best shortcut I've been able to come up starts
with a loopback mount on btrfs.

> Does anybody have details on that?

Hopefully I've been able to shine some light on the reasons why
high-performance random access read/write block storage encryption
with authentication is a harder problem than it looks on the surface.

In the meantime, to address the CBC thing, I'd want to understand what
the hardware is doing exactly.  I wouldn't want the existence of code
that supports CBC in fs/crypto to be interpreted as some sort of
endorsement for using it rather than XTS (when unauthenticated
encryption is for some reason the only viable option) for new storage
encryption applications.

> Thanks,
> David

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Richard Weinberger]

Michael,

On 05.10.2016 20:23, Michael Halcrow wrote:
>> Eric,
>>
>>> On 04.10.2016, at 18:38, Eric Biggers <ebiggers@google.com> wrote:
>>>
>>> On Tue, Oct 04, 2016 at 10:46:54AM +0200, Richard Weinberger wrote:
>>>>> Also, currently this code *is* only supposed to be used for XTS.
>>>>> There's a bug where a specially crafted filesystem can cause
>>>>> this code path to be entered with CTS, but I have a patch
>>>>> pending in the ext4 tree to fix that.
>>>>
>>>> David and I are currently working on UBIFS encryption and we have
>>>> to support other cipher modes than XTS. So, keeping fscrypto as
>>>> generic as possible would be nice. :-)
>>>>
>>>
>>> The problem was that the kernel supported reading a file whose
>>> contents was encrypted with CTS, which is only supposed to be used
>>> for filenames.  This was inconsistent with
>>> FS_IOC_SET_ENCRYPTION_POLICY which currently only allows XTS for
>>> contents and CTS for filenames.  So in other words I wanted to
>>> eliminate a strange scenario that was not intended to happen and
>>> was almost certainly never tested.
>>>
>>> Either way, new modes can still be added if there is a good reason
>>> to do so.  What new encryption modes are you thinking of adding,
>>> would they be for contents or for filenames, and are you thinking
>>> they would be offered by all filesystems (ext4 and f2fs too)?
>>
>> We currently have one case where our embedded platform is only able
>> to do AES-CBC in hardware, not AES-XTS. So switching to AES-CBC for
>> file contents would yield far better performance while still being
>> "secure enough".
> 
> Great to see more interest in file system encryption.  A few thoughts.
> 
> I'm concerned about the proliferation of storage encryption code in
> the kernel.  Of course, I'm perhaps the worst instigator.  However
> what's happening now is that we have several file systems that are
> proposing their own encryption, as well as several attempts at support
> for hardware encryption.
> 
> High-performance random access read/write block storage encryption
> with authentication is hard to get right.  The way I see it, the ideal
> solution would have these properties:
> 
>  * The actual cryptographic transform happens in as few places as
>    possible -- preferably one place in software, with a sensible
>    vendor-neutral API for defering to hardware.
> 
>  * All blocks in the file system, including both file contents and
>    file system metadata, are cryptographically protected.
> 
>  * Encryption is authenticated and has versioning support to enforce
>    consistency and defend against rollback.
> 
>  * File systems can select which keys protect which blocks.
> 
>  * Authentication of all storage chains back to Secure Boot.
> 
> To solve all of these simultaneously, it looks like we'll want to
> consider changes to the kernel block API:

Not all filesystems use the block layer, hint: UBIFS.

> From here, we can delegate to dm-crypt to perform the block
> transformation using the key in the bio.  Or we can defer to the block
> storage driver to provision the key into the hardware encryption
> element and tag requests to use that key.
> 
> This promises to get a big chunk of the file contents encryption logic
> out of the file system layer.
> 
> If the file system doesn't provide a bi_crypt_ctx, then dm-crypt can
> use the default key, which would be shared among all tenants of the
> system.  That shared key can potentially be further protected by the
> distro by leveraging a secure element like a TPM.

No dm-crypt available in MTD land.

> For user-specific file contents -- say, what's in the user's home
> directory -- then that can be protected with a key that's only made
> available after the user logs in (providing their credentials).  Other
> tenants on the same device who can get at the shared key might still
> get information like how many files other users have or what the
> directory structure is, but at least they can't read the contents of
> other users' files.  Meanwhile, the volume is comprehensively
> protected against the "left in a taxi" scenario.
> 
>> Generally speaking though, it would be great to have encryption
>> _and_ authentication for file contents.
> 
> Not good enough for me.  I want authenticated encryption for
> everything, contents or metadata.

Well, let's focus first on file contents.
We have already the fscrypo framework.

What you suggest is completely different from what we have now.

>> AEAD modes like GCM or future finalists of the CAESAR competition
>> come to mind.
> 
> GCM is problematic for block storage, primarily because it's
> catastrophic to reuse a key/IV pair.
> 
> If you naively use the same key when writing more than 2^32 blocks
> with a random IV, you've just stepped into the collision "danger
> zone" (per NIST SP 800-38D).  We have a design that involves frequent
> encryption key derivation in order to address the collision space
> problem.  But that's just one piece of the solution to the whole
> problem.
> 
>> IIRC the ext4 encryption design document mentions this, but it's
>> unclear to me why AES-GCM wasn't used for file contents from the
>> beginning. I'd guess it has to do with where to store the
>> authentication tag and performance.
> 
> Comparatively, that's the easy part.  The hard part is ensuring
> *consistency* between the ciphertext and the cryptographic metadata.
> If you write out the ciphertext and don't get the IV you used for it
> out to storage simultaneously, you've just lost the block.  And
> vice-versa.
> 
> Then there's the problem of internal consistency.  Supposing you do
> manage to get the blocks and their crypto metadata out together,
> what's to stop an attacker from punching holes (for example)?  You
> need an authenticated dictionary structure at that point, such as a
> Merkle tree or an authenticated skiplist.
> 
> Now you have an additional data structure to maintain.  And you're
> rebalancing a Merkle tree in the midst of modifications, or you're
> producing an implementation of ASL in the Linux kernel (which, BTW, my
> team does have a prototype for right now).
> 
> Once we have a root of an authenticated dictionary, we can look to a
> high-performance secure hardware element to sign that root against a
> monotonic counter to get rollback protection.
> 
> To protect the entire block device, we need the authentication data to
> be consistent with the ciphertext at the block level.  So that means
> something like copy-on-write or log-structured volume at the dm-
> layer.  Right now the best shortcut I've been able to come up starts
> with a loopback mount on btrfs.
> 
>> Does anybody have details on that?
> 
> Hopefully I've been able to shine some light on the reasons why
> high-performance random access read/write block storage encryption
> with authentication is a harder problem than it looks on the surface.
> 
> In the meantime, to address the CBC thing, I'd want to understand what
> the hardware is doing exactly.  I wouldn't want the existence of code
> that supports CBC in fs/crypto to be interpreted as some sort of
> endorsement for using it rather than XTS (when unauthenticated
> encryption is for some reason the only viable option) for new storage
> encryption applications.

The hardware offers AES-CBC, accessible via the kernel crypto API.

Thanks,
//richard

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Michael Halcrow]

On Wed, Oct 05, 2016 at 08:44:09PM +0200, Richard Weinberger wrote:
> Michael,
> 
> On 05.10.2016 20:23, Michael Halcrow wrote:
> >> Eric,
> >>
> >>> On 04.10.2016, at 18:38, Eric Biggers <ebiggers@google.com> wrote:
> >>>
> >>> On Tue, Oct 04, 2016 at 10:46:54AM +0200, Richard Weinberger wrote:
> >>>>> Also, currently this code *is* only supposed to be used for XTS.
> >>>>> There's a bug where a specially crafted filesystem can cause
> >>>>> this code path to be entered with CTS, but I have a patch
> >>>>> pending in the ext4 tree to fix that.
> >>>>
> >>>> David and I are currently working on UBIFS encryption and we have
> >>>> to support other cipher modes than XTS. So, keeping fscrypto as
> >>>> generic as possible would be nice. :-)
> >>>>
> >>>
> >>> The problem was that the kernel supported reading a file whose
> >>> contents was encrypted with CTS, which is only supposed to be used
> >>> for filenames.  This was inconsistent with
> >>> FS_IOC_SET_ENCRYPTION_POLICY which currently only allows XTS for
> >>> contents and CTS for filenames.  So in other words I wanted to
> >>> eliminate a strange scenario that was not intended to happen and
> >>> was almost certainly never tested.
> >>>
> >>> Either way, new modes can still be added if there is a good reason
> >>> to do so.  What new encryption modes are you thinking of adding,
> >>> would they be for contents or for filenames, and are you thinking
> >>> they would be offered by all filesystems (ext4 and f2fs too)?
> >>
> >> We currently have one case where our embedded platform is only able
> >> to do AES-CBC in hardware, not AES-XTS. So switching to AES-CBC for
> >> file contents would yield far better performance while still being
> >> "secure enough".
> > 
> > Great to see more interest in file system encryption.  A few thoughts.
> > 
> > I'm concerned about the proliferation of storage encryption code in
> > the kernel.  Of course, I'm perhaps the worst instigator.  However
> > what's happening now is that we have several file systems that are
> > proposing their own encryption, as well as several attempts at support
> > for hardware encryption.
> > 
> > High-performance random access read/write block storage encryption
> > with authentication is hard to get right.  The way I see it, the ideal
> > solution would have these properties:
> > 
> >  * The actual cryptographic transform happens in as few places as
> >    possible -- preferably one place in software, with a sensible
> >    vendor-neutral API for defering to hardware.
> > 
> >  * All blocks in the file system, including both file contents and
> >    file system metadata, are cryptographically protected.
> > 
> >  * Encryption is authenticated and has versioning support to enforce
> >    consistency and defend against rollback.
> > 
> >  * File systems can select which keys protect which blocks.
> > 
> >  * Authentication of all storage chains back to Secure Boot.
> > 
> > To solve all of these simultaneously, it looks like we'll want to
> > consider changes to the kernel block API:
> 
> Not all filesystems use the block layer, hint: UBIFS.
> 
> > From here, we can delegate to dm-crypt to perform the block
> > transformation using the key in the bio.  Or we can defer to the block
> > storage driver to provision the key into the hardware encryption
> > element and tag requests to use that key.
> > 
> > This promises to get a big chunk of the file contents encryption logic
> > out of the file system layer.
> > 
> > If the file system doesn't provide a bi_crypt_ctx, then dm-crypt can
> > use the default key, which would be shared among all tenants of the
> > system.  That shared key can potentially be further protected by the
> > distro by leveraging a secure element like a TPM.
> 
> No dm-crypt available in MTD land.
> 
> > For user-specific file contents -- say, what's in the user's home
> > directory -- then that can be protected with a key that's only made
> > available after the user logs in (providing their credentials).  Other
> > tenants on the same device who can get at the shared key might still
> > get information like how many files other users have or what the
> > directory structure is, but at least they can't read the contents of
> > other users' files.  Meanwhile, the volume is comprehensively
> > protected against the "left in a taxi" scenario.
> > 
> >> Generally speaking though, it would be great to have encryption
> >> _and_ authentication for file contents.
> > 
> > Not good enough for me.  I want authenticated encryption for
> > everything, contents or metadata.
> 
> Well, let's focus first on file contents.
> We have already the fscrypo framework.
> 
> What you suggest is completely different from what we have now.
> 
> >> AEAD modes like GCM or future finalists of the CAESAR competition
> >> come to mind.
> > 
> > GCM is problematic for block storage, primarily because it's
> > catastrophic to reuse a key/IV pair.
> > 
> > If you naively use the same key when writing more than 2^32 blocks
> > with a random IV, you've just stepped into the collision "danger
> > zone" (per NIST SP 800-38D).  We have a design that involves frequent
> > encryption key derivation in order to address the collision space
> > problem.  But that's just one piece of the solution to the whole
> > problem.
> > 
> >> IIRC the ext4 encryption design document mentions this, but it's
> >> unclear to me why AES-GCM wasn't used for file contents from the
> >> beginning. I'd guess it has to do with where to store the
> >> authentication tag and performance.
> > 
> > Comparatively, that's the easy part.  The hard part is ensuring
> > *consistency* between the ciphertext and the cryptographic metadata.
> > If you write out the ciphertext and don't get the IV you used for it
> > out to storage simultaneously, you've just lost the block.  And
> > vice-versa.
> > 
> > Then there's the problem of internal consistency.  Supposing you do
> > manage to get the blocks and their crypto metadata out together,
> > what's to stop an attacker from punching holes (for example)?  You
> > need an authenticated dictionary structure at that point, such as a
> > Merkle tree or an authenticated skiplist.
> > 
> > Now you have an additional data structure to maintain.  And you're
> > rebalancing a Merkle tree in the midst of modifications, or you're
> > producing an implementation of ASL in the Linux kernel (which, BTW, my
> > team does have a prototype for right now).
> > 
> > Once we have a root of an authenticated dictionary, we can look to a
> > high-performance secure hardware element to sign that root against a
> > monotonic counter to get rollback protection.
> > 
> > To protect the entire block device, we need the authentication data to
> > be consistent with the ciphertext at the block level.  So that means
> > something like copy-on-write or log-structured volume at the dm-
> > layer.  Right now the best shortcut I've been able to come up starts
> > with a loopback mount on btrfs.
> > 
> >> Does anybody have details on that?
> > 
> > Hopefully I've been able to shine some light on the reasons why
> > high-performance random access read/write block storage encryption
> > with authentication is a harder problem than it looks on the surface.
> > 
> > In the meantime, to address the CBC thing, I'd want to understand what
> > the hardware is doing exactly.  I wouldn't want the existence of code
> > that supports CBC in fs/crypto to be interpreted as some sort of
> > endorsement for using it rather than XTS (when unauthenticated
> > encryption is for some reason the only viable option) for new storage
> > encryption applications.
> 
> The hardware offers AES-CBC, accessible via the kernel crypto API.

I presume your goal is to usually package up relatively large segments
of data you'd like to chain together under one key/IV?

Else, for random-access block storage, I would like to get on idea on
what the latency/throughput/power impact would be vs. just doing
AES-XTS on the CPU.

Regardless, if you need IV generation in fs/crypto, you can use ESSIV
from eCryptfs as an example.  Except you'll probably want to use
SHA-256 instead of MD5, if only for the sake of hygiene.

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Richard Weinberger]

Michael,

On 05.10.2016 23:11, Michael Halcrow wrote:
>>> In the meantime, to address the CBC thing, I'd want to understand what
>>> the hardware is doing exactly.  I wouldn't want the existence of code
>>> that supports CBC in fs/crypto to be interpreted as some sort of
>>> endorsement for using it rather than XTS (when unauthenticated
>>> encryption is for some reason the only viable option) for new storage
>>> encryption applications.
>>
>> The hardware offers AES-CBC, accessible via the kernel crypto API.
> 
> I presume your goal is to usually package up relatively large segments
> of data you'd like to chain together under one key/IV?

Yes. That's the plan.

> Else, for random-access block storage, I would like to get on idea on
> what the latency/throughput/power impact would be vs. just doing
> AES-XTS on the CPU.

Hopefully I can report some results soon. :-)

> Regardless, if you need IV generation in fs/crypto, you can use ESSIV
> from eCryptfs as an example.  Except you'll probably want to use
> SHA-256 instead of MD5, if only for the sake of hygiene.

Thanks for the pointer.

Thanks,
//richard

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Richard Weinberger]

Michael,

On 05.10.2016 20:44, Richard Weinberger wrote:
> Well, let's focus first on file contents.
> We have already the fscrypo framework.
> 
> What you suggest is completely different from what we have now.

To clarify that, I'm not saying that meta-data or block level authentication
is a bad idea. But let's start with small steps and consider file contents
authentication first. Of course this has some attack vectors but these
can be documented and for many use case these are acceptable.

Thanks,
//richard

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Dave Chinner]

On Wed, Oct 05, 2016 at 11:14:55PM +0200, Richard Weinberger wrote:
> Michael,
> 
> On 05.10.2016 20:44, Richard Weinberger wrote:
> > Well, let's focus first on file contents.
> > We have already the fscrypo framework.
> > 
> > What you suggest is completely different from what we have now.
> 
> To clarify that, I'm not saying that meta-data or block level authentication
> is a bad idea. But let's start with small steps and consider file contents
> authentication first. Of course this has some attack vectors but these
> can be documented and for many use case these are acceptable.

This is the sanest approach, because encrypting filesystem internal
metadata may have some unintended consequences. e.g being unable to
perform forensic analysis of corruption or data loss events, or an
inability for tools like fsck to work without also implementing all
the encryption code in userspace and being provided with all the
keys needed to decrypt the metadata.

i.e. it's not just the kernel code we have to consider here when
discussing this level of encryption in filesystems - the impact
on the entire support ecosystem needs to be considered. A weakness
in a fsck tool will be just as serious as a weakness in the kernel
code, and there's a much larger amount of widely dispersed code that
would need to be encryption enabled by going down this path.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Theodore Ts'o]

On Thu, Oct 06, 2016 at 12:17:15PM +1100, Dave Chinner wrote:
> This is the sanest approach, because encrypting filesystem internal
> metadata may have some unintended consequences. e.g being unable to
> perform forensic analysis of corruption or data loss events, or an
> inability for tools like fsck to work without also implementing all
> the encryption code in userspace and being provided with all the
> keys needed to decrypt the metadata.

Absolutely. 

> i.e. it's not just the kernel code we have to consider here when
> discussing this level of encryption in filesystems - the impact
> on the entire support ecosystem needs to be considered. A weakness
> in a fsck tool will be just as serious as a weakness in the kernel
> code, and there's a much larger amount of widely dispersed code that
> would need to be encryption enabled by going down this path.

An approach that works fairly well, which doesn't require any
userspace changes, is one where if you are using a hardware
accelerated, in-line crypto engine, and where you are sending the key
identifier to be used down through the block layer in the bio request,
is to define a particular key as the "default" key to be used if a key
is not specified in the bio request.

So if the file system doesn't send down an explicit key identifier for
its metadata reads/write requests, the block device essentially acts
like a dm-crypt device --- except the hardware is doing the encryption
so it's nice and fast.

This approach means that no changes are needed in the file system for
encrypting and decrypting the metadata blocks, and it also means that
no changes are needed in any of the e2fsprogs userspace utiliies
(which except for debugfs and fuse2fs, only read metadata blocks).
Well, resize2fs wouldn't work since it would try to move data blocks
around, but eMMC memory is generally not resizeable, so it all works
out at least for mobile devices, and if we really want to support this
for resizeable devices, we could teach resize2fs a way to signal to
the block device which blocks are data blocks and so the ICE layer
should be passed during a file system shrink operation.

So if we move to a model where we move the actual block
encryption/decryption into a dm-crypt like layer (instead of doing it
in fs/ext4/page_io.c as we currently do for ext4's fs crypto support),
we minimize the changes needed in the file systems --- since all we
need to do is to pass the key identifier into the bio layer, and key
identifier management can be done in the fs/crypto layer, and so on
top of the other benefits, we get the ability to support generalized
hardware ICE acceleration and metadata encryption for a relatively
tiny investment of effort.

Yes, this won't work for ubifs, but ubifs is a bit of an outlier, and
unless we think we want to support any other MTD file systems, we may
just simply need make things flexible enough so that ubifs can do its
own thing as far as hardware acceleration is concerned.

Cheers,

					- Ted

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Dave Chinner]

On Wed, Oct 05, 2016 at 10:16:25PM -0400, Theodore Ts'o wrote:
> On Thu, Oct 06, 2016 at 12:17:15PM +1100, Dave Chinner wrote:
> > This is the sanest approach, because encrypting filesystem internal
> > metadata may have some unintended consequences. e.g being unable to
> > perform forensic analysis of corruption or data loss events, or an
> > inability for tools like fsck to work without also implementing all
> > the encryption code in userspace and being provided with all the
> > keys needed to decrypt the metadata.
> 
> Absolutely. 
> 
> > i.e. it's not just the kernel code we have to consider here when
> > discussing this level of encryption in filesystems - the impact
> > on the entire support ecosystem needs to be considered. A weakness
> > in a fsck tool will be just as serious as a weakness in the kernel
> > code, and there's a much larger amount of widely dispersed code that
> > would need to be encryption enabled by going down this path.
> 
> An approach that works fairly well, which doesn't require any
> userspace changes, is one where if you are using a hardware
> accelerated, in-line crypto engine, and where you are sending the key
> identifier to be used down through the block layer in the bio request,
> is to define a particular key as the "default" key to be used if a key
> is not specified in the bio request.
> 
> So if the file system doesn't send down an explicit key identifier for
> its metadata reads/write requests, the block device essentially acts
> like a dm-crypt device --- except the hardware is doing the encryption
> so it's nice and fast.

At which point, I have to ask, why not just use dm-crypt and
implement a simple "use device key on bio contents if none has been
specified by higher layer" mechanism to direct the encryption?  It's
simple, you can still offload the crypto to your hardware, requires
minimal extra plumbing, and no userspace tooling changes. This
allows the user data on a dm-crypt to be encrypted with a user
specified key so even is the device key is compromised their data is
not (and vice versa).

As for non-block device filesystems like UBIFS, I'd suggest that
their "storage layer" that the filesytem sits on top of could do the
same thing - any request that doesn't have a key supplied gets
encrypted using the device key. i.e. the filesystem layers use the
same implementation, but the underlying storage device implements
the encryption interface according to the requirements of the
subsystem.

i.e. I don't think we should be trying to implement a single layer
that supports both block and non-block filesystems here. However, we
can still architect a solution where the filesystem level code is
independent of the underlying storage encryption implementation.  In
this sort of architecture, the filesystem only does key management
and deals with unencrypted data and metadata, whilst the underlying
storage layer implements the (hardware accelerated) encryption as
the data passes through...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Theodore Ts'o]

On Fri, Oct 07, 2016 at 09:23:51AM +1100, Dave Chinner wrote:
> At which point, I have to ask, why not just use dm-crypt and
> implement a simple "use device key on bio contents if none has been
> specified by higher layer" mechanism to direct the encryption?  It's
> simple, you can still offload the crypto to your hardware, requires
> minimal extra plumbing, and no userspace tooling changes. This
> allows the user data on a dm-crypt to be encrypted with a user
> specified key so even is the device key is compromised their data is
> not (and vice versa).

That's what I was suggesting.  There would still have to be a
userspace tool which specifies which key should be used for which
file, but the actual userspace changes beyond that are minimal.  We
have some changes in e2fsck to complain if the "this file encrypted"
when the encryption feature flag is not set, and so on.

Once Pixel and Pixel XL ship in a few weeks, and the kernel
repositories are published through AOSP, there will be some prototype
code for folks to look at --- although I'll be the first to admit it
is a bit ugly.  We were under a bit of time pressure, and so we took a
bunch of shortcuts that we knew we would have to clean up (and maybe
even reimplement) before the technology was ready to go upstream.

But, yes, that's the direction in which I want to evolve the fs/crypto
code.

> As for non-block device filesystems like UBIFS, I'd suggest that
> their "storage layer" that the filesytem sits on top of could do the
> same thing - any request that doesn't have a key supplied gets
> encrypted using the device key. i.e. the filesystem layers use the
> same implementation, but the underlying storage device implements
> the encryption interface according to the requirements of the
> subsystem.

Yes, the right place for this would be in the MTD layer.  I'm not sure
hardware designers will ever be interested in adding hardware
accelerated crypto in front of the "raw flash" interface (if they are
going to do that, why not just throw in the whole FTL and eMMC layers
as well?), but if it does happen, the MTD layer could handle that as
well.

> i.e. I don't think we should be trying to implement a single layer
> that supports both block and non-block filesystems here. However, we
> can still architect a solution where the filesystem level code is
> independent of the underlying storage encryption implementation.  In
> this sort of architecture, the filesystem only does key management
> and deals with unencrypted data and metadata, whilst the underlying
> storage layer implements the (hardware accelerated) encryption as
> the data passes through...

Yep, that's the idea --- with the added twist that the architecture
should be flexible enough that in some cases, the pieces of the key
management, currently in fs/crypto, might get moved into to secure
enclaves, such that even if the attacker completely compromises kernel
memory, none of cryptographic bits would be available.

Both ARM Trustzone and Intel's SGX are examples of trusted execution
environments which could be used for this purpose, so this is
something we'll eventually be able to use and test on x86 developer
workstations.

Cheers,

						- Ted

[PATCH] fscrypto: make XTS tweak initialization endian-independent

[Eric Biggers]

The XTS tweak (or IV) was initialized differently on little endian and
big endian systems.  Because the ciphertext depends on the XTS tweak, it
was not possible to use an encrypted filesystem created by a little
endian system on a big endian system and vice versa, even if they shared
the same PAGE_SIZE.  Fix this by always using little endian.

This will break hypothetical big endian users of ext4 or f2fs
encryption.  However, all users we are aware of are little endian, and
it's believed that "real" big endian users are unlikely to exist yet.
So this might as well be fixed now before it's too late.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/crypto.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 61057b7d..98f87fe 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -151,7 +151,10 @@ static int do_page_crypto(struct inode *inode,
 			struct page *src_page, struct page *dest_page,
 			gfp_t gfp_flags)
 {
-	u8 xts_tweak[FS_XTS_TWEAK_SIZE];
+	struct {
+		__le64 index;
+		u8 padding[FS_XTS_TWEAK_SIZE - sizeof(__le64)];
+	} xts_tweak;
 	struct skcipher_request *req = NULL;
 	DECLARE_FS_COMPLETION_RESULT(ecr);
 	struct scatterlist dst, src;
@@ -171,17 +174,15 @@ static int do_page_crypto(struct inode *inode,
 		req, CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
 		page_crypt_complete, &ecr);
 
-	BUILD_BUG_ON(FS_XTS_TWEAK_SIZE < sizeof(index));
-	memcpy(xts_tweak, &index, sizeof(index));
-	memset(&xts_tweak[sizeof(index)], 0,
-			FS_XTS_TWEAK_SIZE - sizeof(index));
+	BUILD_BUG_ON(sizeof(xts_tweak) != FS_XTS_TWEAK_SIZE);
+	xts_tweak.index = cpu_to_le64(index);
+	memset(xts_tweak.padding, 0, sizeof(xts_tweak.padding));
 
 	sg_init_table(&dst, 1);
 	sg_set_page(&dst, dest_page, PAGE_SIZE, 0);
 	sg_init_table(&src, 1);
 	sg_set_page(&src, src_page, PAGE_SIZE, 0);
-	skcipher_request_set_crypt(req, &src, &dst, PAGE_SIZE,
-					xts_tweak);
+	skcipher_request_set_crypt(req, &src, &dst, PAGE_SIZE, &xts_tweak);
 	if (rw == FS_DECRYPT)
 		res = crypto_skcipher_decrypt(req);
 	else
-- 
2.8.0.rc3.226.g39d4020


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Richard Weinberger]

Eric,

On Fri, Sep 30, 2016 at 7:58 PM, Eric Biggers <ebiggers@google.com> wrote:
> The XTS tweak (or IV) was initialized differently on little endian and
> big endian systems.  Because the ciphertext depends on the XTS tweak, it
> was not possible to use an encrypted filesystem created by a little
> endian system on a big endian system and vice versa, even if they shared
> the same PAGE_SIZE.  Fix this by always using little endian.
>
> This will break hypothetical big endian users of ext4 or f2fs
> encryption.  However, all users we are aware of are little endian, and
> it's believed that "real" big endian users are unlikely to exist yet.
> So this might as well be fixed now before it's too late.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/crypto/crypto.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index 61057b7d..98f87fe 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -151,7 +151,10 @@ static int do_page_crypto(struct inode *inode,
>                         struct page *src_page, struct page *dest_page,
>                         gfp_t gfp_flags)
>  {
> -       u8 xts_tweak[FS_XTS_TWEAK_SIZE];
> +       struct {
> +               __le64 index;
> +               u8 padding[FS_XTS_TWEAK_SIZE - sizeof(__le64)];
> +       } xts_tweak;

While we are here, wouldn't it make sense to rename the variable to "iv"?
In aes-xts mode the IV is used as tweak. But it is still an IV and passed
as IV parameter to the crypto API.

Especially when other cipher modes are used this is confusing.

-- 
Thanks,
//richard

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Eric Biggers]

On Sat, Oct 01, 2016 at 06:03:31PM +0200, Richard Weinberger wrote:
> Eric,
> 
> On Fri, Sep 30, 2016 at 7:58 PM, Eric Biggers <ebiggers@google.com> wrote:
> > The XTS tweak (or IV) was initialized differently on little endian and
> > big endian systems.  Because the ciphertext depends on the XTS tweak, it
> > was not possible to use an encrypted filesystem created by a little
> > endian system on a big endian system and vice versa, even if they shared
> > the same PAGE_SIZE.  Fix this by always using little endian.
> >
> > This will break hypothetical big endian users of ext4 or f2fs
> > encryption.  However, all users we are aware of are little endian, and
> > it's believed that "real" big endian users are unlikely to exist yet.
> > So this might as well be fixed now before it's too late.
> >
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >  fs/crypto/crypto.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> > index 61057b7d..98f87fe 100644
> > --- a/fs/crypto/crypto.c
> > +++ b/fs/crypto/crypto.c
> > @@ -151,7 +151,10 @@ static int do_page_crypto(struct inode *inode,
> >                         struct page *src_page, struct page *dest_page,
> >                         gfp_t gfp_flags)
> >  {
> > -       u8 xts_tweak[FS_XTS_TWEAK_SIZE];
> > +       struct {
> > +               __le64 index;
> > +               u8 padding[FS_XTS_TWEAK_SIZE - sizeof(__le64)];
> > +       } xts_tweak;
> 
> While we are here, wouldn't it make sense to rename the variable to "iv"?
> In aes-xts mode the IV is used as tweak. But it is still an IV and passed
> as IV parameter to the crypto API.
> 
> Especially when other cipher modes are used this is confusing.
> 

Good idea --- I agree that "iv" is a better name, so as to not tie the code to
XTS specifically.  But I think the renaming should be a separate patch.

Also, currently this code *is* only supposed to be used for XTS.  There's a bug
where a specially crafted filesystem can cause this code path to be entered with
CTS, but I have a patch pending in the ext4 tree to fix that.

Eric

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Richard Weinberger]

Eric,

On 03.10.2016 20:03, Eric Biggers wrote:
>>>  {
>>> -       u8 xts_tweak[FS_XTS_TWEAK_SIZE];
>>> +       struct {
>>> +               __le64 index;
>>> +               u8 padding[FS_XTS_TWEAK_SIZE - sizeof(__le64)];
>>> +       } xts_tweak;
>>
>> While we are here, wouldn't it make sense to rename the variable to "iv"?
>> In aes-xts mode the IV is used as tweak. But it is still an IV and passed
>> as IV parameter to the crypto API.
>>
>> Especially when other cipher modes are used this is confusing.
>>
> 
> Good idea --- I agree that "iv" is a better name, so as to not tie the code to
> XTS specifically.  But I think the renaming should be a separate patch.

Sure. I can do that.

> Also, currently this code *is* only supposed to be used for XTS.  There's a bug
> where a specially crafted filesystem can cause this code path to be entered with
> CTS, but I have a patch pending in the ext4 tree to fix that.

David and I are currently working on UBIFS encryption and we have to support other cipher
modes than XTS. So, keeping fscrypto as generic as possible would be nice. :-)

Thanks,
//richard

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Eric Biggers]

On Tue, Oct 04, 2016 at 10:46:54AM +0200, Richard Weinberger wrote:
> > Also, currently this code *is* only supposed to be used for XTS.  There's a bug
> > where a specially crafted filesystem can cause this code path to be entered with
> > CTS, but I have a patch pending in the ext4 tree to fix that.
> 
> David and I are currently working on UBIFS encryption and we have to support other cipher
> modes than XTS. So, keeping fscrypto as generic as possible would be nice. :-)
> 

The problem was that the kernel supported reading a file whose contents was
encrypted with CTS, which is only supposed to be used for filenames.  This was
inconsistent with FS_IOC_SET_ENCRYPTION_POLICY which currently only allows XTS
for contents and CTS for filenames.  So in other words I wanted to eliminate a
strange scenario that was not intended to happen and was almost certainly never
tested.

Either way, new modes can still be added if there is a good reason to do so.
What new encryption modes are you thinking of adding, would they be for contents
or for filenames, and are you thinking they would be offered by all filesystems
(ext4 and f2fs too)?

Eric

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[David Gstir]

Eric,

> On 04.10.2016, at 18:38, Eric Biggers <ebiggers@google.com> wrote:
> 
> On Tue, Oct 04, 2016 at 10:46:54AM +0200, Richard Weinberger wrote:
>>> Also, currently this code *is* only supposed to be used for XTS.  There's a bug
>>> where a specially crafted filesystem can cause this code path to be entered with
>>> CTS, but I have a patch pending in the ext4 tree to fix that.
>> 
>> David and I are currently working on UBIFS encryption and we have to support other cipher
>> modes than XTS. So, keeping fscrypto as generic as possible would be nice. :-)
>> 
> 
> The problem was that the kernel supported reading a file whose contents was
> encrypted with CTS, which is only supposed to be used for filenames.  This was
> inconsistent with FS_IOC_SET_ENCRYPTION_POLICY which currently only allows XTS
> for contents and CTS for filenames.  So in other words I wanted to eliminate a
> strange scenario that was not intended to happen and was almost certainly never
> tested.
> 
> Either way, new modes can still be added if there is a good reason to do so.
> What new encryption modes are you thinking of adding, would they be for contents
> or for filenames, and are you thinking they would be offered by all filesystems
> (ext4 and f2fs too)?

We currently have one case where our embedded platform is only able to do AES-CBC in hardware, not AES-XTS. So switching to AES-CBC for file contents would yield far better performance while still being "secure enough".

Generally speaking though, it would be great to have encryption _and_ authentication for file contents. AEAD modes like GCM or future finalists of the CAESAR competition come to mind. IIRC the ext4 encryption design document mentions this, but it's unclear to me why AES-GCM wasn't used for file contents from the beginning. I'd guess it has to do with where to store the authentication tag and performance.
Does anybody have details on that?

Thanks,
David

Re: [PATCH] fscrypto: make XTS tweak initialization endian-independent

[Theodore Ts'o]

On Fri, Sep 30, 2016 at 7:58 PM, Eric Biggers <ebiggers@google.com> wrote:
> The XTS tweak (or IV) was initialized differently on little endian and
> big endian systems.  Because the ciphertext depends on the XTS tweak, it
> was not possible to use an encrypted filesystem created by a little
> endian system on a big endian system and vice versa, even if they shared
> the same PAGE_SIZE.  Fix this by always using little endian.
>
> This will break hypothetical big endian users of ext4 or f2fs
> encryption.  However, all users we are aware of are little endian, and
> it's believed that "real" big endian users are unlikely to exist yet.
> So this might as well be fixed now before it's too late.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied.

					- Ted

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot