diff mbox series

[v4,8/8] fscrypt: Add HCTR2 support for filename encryption

Message ID 20220412172816.917723-9-nhuck@google.com (mailing list archive)
State New, archived
Headers show
Series crypto: HCTR2 support | expand

Commit Message

Nathan Huckleberry April 12, 2022, 5:28 p.m. UTC
HCTR2 is a tweakable, length-preserving encryption mode.  It has the
same security guarantees as Adiantum, but is intended for use on CPUs
with dedicated crypto instructions.  It fixes a known weakness with
filename encryption: when two filenames in the same directory share a
prefix of >= 16 bytes, with CTS-CBC their encrypted filenames share a
common substring, leaking information.  HCTR2 does not have this
problem.

More information on HCTR2 can be found here: Length-preserving
encryption with HCTR2: https://eprint.iacr.org/2021/1441.pdf

Signed-off-by: Nathan Huckleberry <nhuck@google.com>
---
 Documentation/filesystems/fscrypt.rst | 19 ++++++++++++++-----
 fs/crypto/fscrypt_private.h           |  2 +-
 fs/crypto/keysetup.c                  |  7 +++++++
 fs/crypto/policy.c                    |  4 ++++
 include/uapi/linux/fscrypt.h          |  3 ++-
 tools/include/uapi/linux/fscrypt.h    |  3 ++-
 6 files changed, 30 insertions(+), 8 deletions(-)

Comments

Eric Biggers April 13, 2022, 6:10 a.m. UTC | #1
On Tue, Apr 12, 2022 at 05:28:16PM +0000, Nathan Huckleberry wrote:
> HCTR2 is a tweakable, length-preserving encryption mode.  It has the
> same security guarantees as Adiantum, but is intended for use on CPUs
> with dedicated crypto instructions.  It fixes a known weakness with
> filename encryption: when two filenames in the same directory share a
> prefix of >= 16 bytes, with CTS-CBC their encrypted filenames share a
> common substring, leaking information.  HCTR2 does not have this
> problem.
> 
> More information on HCTR2 can be found here: Length-preserving
> encryption with HCTR2: https://eprint.iacr.org/2021/1441.pdf

Please quote titles to distinguish them from the surrounding text.  E.g.

More information on HCTR2 can be found in the paper "Length-preserving
encryption with HCTR2" (https://eprint.iacr.org/2021/1441.pdf)


> 
> Signed-off-by: Nathan Huckleberry <nhuck@google.com>
> ---
>  Documentation/filesystems/fscrypt.rst | 19 ++++++++++++++-----
>  fs/crypto/fscrypt_private.h           |  2 +-
>  fs/crypto/keysetup.c                  |  7 +++++++
>  fs/crypto/policy.c                    |  4 ++++
>  include/uapi/linux/fscrypt.h          |  3 ++-
>  tools/include/uapi/linux/fscrypt.h    |  3 ++-
>  6 files changed, 30 insertions(+), 8 deletions(-)

Can you make sure that all fscrypt patches are Cc'ed to the linux-fscrypt
mailing list?  In this case, just Cc the whole series to there.

> 
> diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
> index 4d5d50dca65c..09915086abd8 100644
> --- a/Documentation/filesystems/fscrypt.rst
> +++ b/Documentation/filesystems/fscrypt.rst
> @@ -337,6 +337,7 @@ Currently, the following pairs of encryption modes are supported:
>  - AES-256-XTS for contents and AES-256-CTS-CBC for filenames
>  - AES-128-CBC for contents and AES-128-CTS-CBC for filenames
>  - Adiantum for both contents and filenames
> +- AES-256-XTS for contents and AES-256-HCTR2 for filenames
>  
>  If unsure, you should use the (AES-256-XTS, AES-256-CTS-CBC) pair.
>  
> @@ -357,6 +358,14 @@ To use Adiantum, CONFIG_CRYPTO_ADIANTUM must be enabled.  Also, fast
>  implementations of ChaCha and NHPoly1305 should be enabled, e.g.
>  CONFIG_CRYPTO_CHACHA20_NEON and CONFIG_CRYPTO_NHPOLY1305_NEON for ARM.
>  
> +AES-256-HCTR2 is another true wide-block encryption mode.  It has the same
> +security guarantees as Adiantum, but is intended for use on CPUs with dedicated
> +crypto instructions. See the paper "Length-preserving encryption with HCTR2"
> +(https://eprint.iacr.org/2021/1441.pdf) for more details. To use HCTR2,
> +CONFIG_CRYPTO_HCTR2 must be enabled. Also, fast implementations of XCTR and
> +POLYVAL should be enabled, e.g. CRYPTO_POLYVAL_ARM64_CE and
> +CRYPTO_AES_ARM64_CE_BLK for ARM64.

"same security guarantees as Adiantum" is not really correct.  Both Adiantum and
HCTR2 are secure super-pseudorandom permutations if their underlying primitives
are secure.  So their security guarantees are pretty similar, but not literally
the same.  Can you reword this?  This potentially-misleading claim also showed
up in the commit message.

> diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
> index ed3d623724cd..fa8bdb8c76b7 100644
> --- a/fs/crypto/policy.c
> +++ b/fs/crypto/policy.c
> @@ -54,6 +54,10 @@ static bool fscrypt_valid_enc_modes(u32 contents_mode, u32 filenames_mode)
>  	    filenames_mode == FSCRYPT_MODE_ADIANTUM)
>  		return true;
>  
> +	if (contents_mode == FSCRYPT_MODE_AES_256_XTS &&
> +	    filenames_mode == FSCRYPT_MODE_AES_256_HCTR2)
> +		return true;
> +
>  	return false;
>  }

This is allowing HCTR2 for both v1 and v2 encryption policies.  I don't think we
should add any new features to v1 encryption policies, as they are deprecated.
How about allowing HCTR2 for v2 encryption policies only?  This is the first new
encryption mode where this issue has come up, but this could be handled easily
by splitting fscrypt_valid_enc_modes() into fscrypt_valid_enc_modes_v1() and
fscrypt_valid_enc_modes_v2().  The v2 one can call the v1 one to share code.

> diff --git a/tools/include/uapi/linux/fscrypt.h b/tools/include/uapi/linux/fscrypt.h
> index 9f4428be3e36..a756b29afcc2 100644
> --- a/tools/include/uapi/linux/fscrypt.h
> +++ b/tools/include/uapi/linux/fscrypt.h
> @@ -27,7 +27,8 @@
>  #define FSCRYPT_MODE_AES_128_CBC		5
>  #define FSCRYPT_MODE_AES_128_CTS		6
>  #define FSCRYPT_MODE_ADIANTUM			9
> -/* If adding a mode number > 9, update FSCRYPT_MODE_MAX in fscrypt_private.h */
> +#define FSCRYPT_MODE_AES_256_HCTR2		10
> +/* If adding a mode number > 10, update FSCRYPT_MODE_MAX in fscrypt_private.h */
>  

As far as I know, you don't actually need to update the copy of UAPI headers in
tools/.  The people who maintain those files handle that.  It doesn't make sense
to have copies of files in the source tree anyway.

- Eric
Ard Biesheuvel April 13, 2022, 6:16 a.m. UTC | #2
On Wed, 13 Apr 2022 at 08:10, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Apr 12, 2022 at 05:28:16PM +0000, Nathan Huckleberry wrote:
> > HCTR2 is a tweakable, length-preserving encryption mode.  It has the
> > same security guarantees as Adiantum, but is intended for use on CPUs
> > with dedicated crypto instructions.  It fixes a known weakness with
> > filename encryption: when two filenames in the same directory share a
> > prefix of >= 16 bytes, with CTS-CBC their encrypted filenames share a
> > common substring, leaking information.  HCTR2 does not have this
> > problem.
> >
> > More information on HCTR2 can be found here: Length-preserving
> > encryption with HCTR2: https://eprint.iacr.org/2021/1441.pdf
>
> Please quote titles to distinguish them from the surrounding text.  E.g.
>
> More information on HCTR2 can be found in the paper "Length-preserving
> encryption with HCTR2" (https://eprint.iacr.org/2021/1441.pdf)
>
>
> >
> > Signed-off-by: Nathan Huckleberry <nhuck@google.com>
> > ---
> >  Documentation/filesystems/fscrypt.rst | 19 ++++++++++++++-----
> >  fs/crypto/fscrypt_private.h           |  2 +-
> >  fs/crypto/keysetup.c                  |  7 +++++++
> >  fs/crypto/policy.c                    |  4 ++++
> >  include/uapi/linux/fscrypt.h          |  3 ++-
> >  tools/include/uapi/linux/fscrypt.h    |  3 ++-
> >  6 files changed, 30 insertions(+), 8 deletions(-)
>
> Can you make sure that all fscrypt patches are Cc'ed to the linux-fscrypt
> mailing list?  In this case, just Cc the whole series to there.
>
> >
> > diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
> > index 4d5d50dca65c..09915086abd8 100644
> > --- a/Documentation/filesystems/fscrypt.rst
> > +++ b/Documentation/filesystems/fscrypt.rst
> > @@ -337,6 +337,7 @@ Currently, the following pairs of encryption modes are supported:
> >  - AES-256-XTS for contents and AES-256-CTS-CBC for filenames
> >  - AES-128-CBC for contents and AES-128-CTS-CBC for filenames
> >  - Adiantum for both contents and filenames
> > +- AES-256-XTS for contents and AES-256-HCTR2 for filenames
> >
> >  If unsure, you should use the (AES-256-XTS, AES-256-CTS-CBC) pair.
> >
> > @@ -357,6 +358,14 @@ To use Adiantum, CONFIG_CRYPTO_ADIANTUM must be enabled.  Also, fast
> >  implementations of ChaCha and NHPoly1305 should be enabled, e.g.
> >  CONFIG_CRYPTO_CHACHA20_NEON and CONFIG_CRYPTO_NHPOLY1305_NEON for ARM.
> >
> > +AES-256-HCTR2 is another true wide-block encryption mode.  It has the same
> > +security guarantees as Adiantum, but is intended for use on CPUs with dedicated
> > +crypto instructions. See the paper "Length-preserving encryption with HCTR2"
> > +(https://eprint.iacr.org/2021/1441.pdf) for more details. To use HCTR2,
> > +CONFIG_CRYPTO_HCTR2 must be enabled. Also, fast implementations of XCTR and
> > +POLYVAL should be enabled, e.g. CRYPTO_POLYVAL_ARM64_CE and
> > +CRYPTO_AES_ARM64_CE_BLK for ARM64.
>
> "same security guarantees as Adiantum" is not really correct.  Both Adiantum and
> HCTR2 are secure super-pseudorandom permutations if their underlying primitives
> are secure.  So their security guarantees are pretty similar, but not literally
> the same.  Can you reword this?  This potentially-misleading claim also showed
> up in the commit message.
>
> > diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
> > index ed3d623724cd..fa8bdb8c76b7 100644
> > --- a/fs/crypto/policy.c
> > +++ b/fs/crypto/policy.c
> > @@ -54,6 +54,10 @@ static bool fscrypt_valid_enc_modes(u32 contents_mode, u32 filenames_mode)
> >           filenames_mode == FSCRYPT_MODE_ADIANTUM)
> >               return true;
> >
> > +     if (contents_mode == FSCRYPT_MODE_AES_256_XTS &&
> > +         filenames_mode == FSCRYPT_MODE_AES_256_HCTR2)
> > +             return true;
> > +
> >       return false;
> >  }
>
> This is allowing HCTR2 for both v1 and v2 encryption policies.  I don't think we
> should add any new features to v1 encryption policies, as they are deprecated.
> How about allowing HCTR2 for v2 encryption policies only?  This is the first new
> encryption mode where this issue has come up, but this could be handled easily
> by splitting fscrypt_valid_enc_modes() into fscrypt_valid_enc_modes_v1() and
> fscrypt_valid_enc_modes_v2().  The v2 one can call the v1 one to share code.
>
> > diff --git a/tools/include/uapi/linux/fscrypt.h b/tools/include/uapi/linux/fscrypt.h
> > index 9f4428be3e36..a756b29afcc2 100644
> > --- a/tools/include/uapi/linux/fscrypt.h
> > +++ b/tools/include/uapi/linux/fscrypt.h
> > @@ -27,7 +27,8 @@
> >  #define FSCRYPT_MODE_AES_128_CBC             5
> >  #define FSCRYPT_MODE_AES_128_CTS             6
> >  #define FSCRYPT_MODE_ADIANTUM                        9
> > -/* If adding a mode number > 9, update FSCRYPT_MODE_MAX in fscrypt_private.h */
> > +#define FSCRYPT_MODE_AES_256_HCTR2           10
> > +/* If adding a mode number > 10, update FSCRYPT_MODE_MAX in fscrypt_private.h */
> >
>
> As far as I know, you don't actually need to update the copy of UAPI headers in
> tools/.  The people who maintain those files handle that.  It doesn't make sense
> to have copies of files in the source tree anyway.
>

Doesn't the x86 build emit a warning if these go out of sync?
Eric Biggers April 14, 2022, 7:12 a.m. UTC | #3
On Wed, Apr 13, 2022 at 08:16:25AM +0200, Ard Biesheuvel wrote:
> > > diff --git a/tools/include/uapi/linux/fscrypt.h b/tools/include/uapi/linux/fscrypt.h
> > > index 9f4428be3e36..a756b29afcc2 100644
> > > --- a/tools/include/uapi/linux/fscrypt.h
> > > +++ b/tools/include/uapi/linux/fscrypt.h
> > > @@ -27,7 +27,8 @@
> > >  #define FSCRYPT_MODE_AES_128_CBC             5
> > >  #define FSCRYPT_MODE_AES_128_CTS             6
> > >  #define FSCRYPT_MODE_ADIANTUM                        9
> > > -/* If adding a mode number > 9, update FSCRYPT_MODE_MAX in fscrypt_private.h */
> > > +#define FSCRYPT_MODE_AES_256_HCTR2           10
> > > +/* If adding a mode number > 10, update FSCRYPT_MODE_MAX in fscrypt_private.h */
> > >
> >
> > As far as I know, you don't actually need to update the copy of UAPI headers in
> > tools/.  The people who maintain those files handle that.  It doesn't make sense
> > to have copies of files in the source tree anyway.
> >
> 
> Doesn't the x86 build emit a warning if these go out of sync?

The warning is emitted when building tools/perf/, not the kernel itself.

According to https://lore.kernel.org/r/20191001185741.GD13904@kernel.org, the
perf maintainers actually prefer that their files are *not* updated for them.
And I'd like to push back against having duplicate source files in the tree
anyway, for obvious reasons.  So I think we shouldn't update this file.

- Eric
Ard Biesheuvel April 14, 2022, 7:15 a.m. UTC | #4
On Thu, 14 Apr 2022 at 09:12, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, Apr 13, 2022 at 08:16:25AM +0200, Ard Biesheuvel wrote:
> > > > diff --git a/tools/include/uapi/linux/fscrypt.h b/tools/include/uapi/linux/fscrypt.h
> > > > index 9f4428be3e36..a756b29afcc2 100644
> > > > --- a/tools/include/uapi/linux/fscrypt.h
> > > > +++ b/tools/include/uapi/linux/fscrypt.h
> > > > @@ -27,7 +27,8 @@
> > > >  #define FSCRYPT_MODE_AES_128_CBC             5
> > > >  #define FSCRYPT_MODE_AES_128_CTS             6
> > > >  #define FSCRYPT_MODE_ADIANTUM                        9
> > > > -/* If adding a mode number > 9, update FSCRYPT_MODE_MAX in fscrypt_private.h */
> > > > +#define FSCRYPT_MODE_AES_256_HCTR2           10
> > > > +/* If adding a mode number > 10, update FSCRYPT_MODE_MAX in fscrypt_private.h */
> > > >
> > >
> > > As far as I know, you don't actually need to update the copy of UAPI headers in
> > > tools/.  The people who maintain those files handle that.  It doesn't make sense
> > > to have copies of files in the source tree anyway.
> > >
> >
> > Doesn't the x86 build emit a warning if these go out of sync?
>
> The warning is emitted when building tools/perf/, not the kernel itself.
>
> According to https://lore.kernel.org/r/20191001185741.GD13904@kernel.org, the
> perf maintainers actually prefer that their files are *not* updated for them.
> And I'd like to push back against having duplicate source files in the tree
> anyway, for obvious reasons.  So I think we shouldn't update this file.
>

Fair enough.
Eric Biggers April 18, 2022, 6:05 p.m. UTC | #5
On Tue, Apr 12, 2022 at 05:28:16PM +0000, Nathan Huckleberry wrote:
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index eede186b04ce..ae24b581d3d7 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -53,6 +53,13 @@ struct fscrypt_mode fscrypt_modes[] = {
>  		.ivsize = 32,
>  		.blk_crypto_mode = BLK_ENCRYPTION_MODE_ADIANTUM,
>  	},
> +	[FSCRYPT_MODE_AES_256_HCTR2] = {
> +		.friendly_name = "HCTR2",

Can you use "AES-256-HCTR2" here instead of just "HCTR2"?  This would be similar
to how FSCRYPT_MODE_AES_256_XTS uses .friendly_name = "AES-256-XTS".

- Eric
diff mbox series

Patch

diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 4d5d50dca65c..09915086abd8 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -337,6 +337,7 @@  Currently, the following pairs of encryption modes are supported:
 - AES-256-XTS for contents and AES-256-CTS-CBC for filenames
 - AES-128-CBC for contents and AES-128-CTS-CBC for filenames
 - Adiantum for both contents and filenames
+- AES-256-XTS for contents and AES-256-HCTR2 for filenames
 
 If unsure, you should use the (AES-256-XTS, AES-256-CTS-CBC) pair.
 
@@ -357,6 +358,14 @@  To use Adiantum, CONFIG_CRYPTO_ADIANTUM must be enabled.  Also, fast
 implementations of ChaCha and NHPoly1305 should be enabled, e.g.
 CONFIG_CRYPTO_CHACHA20_NEON and CONFIG_CRYPTO_NHPOLY1305_NEON for ARM.
 
+AES-256-HCTR2 is another true wide-block encryption mode.  It has the same
+security guarantees as Adiantum, but is intended for use on CPUs with dedicated
+crypto instructions. See the paper "Length-preserving encryption with HCTR2"
+(https://eprint.iacr.org/2021/1441.pdf) for more details. To use HCTR2,
+CONFIG_CRYPTO_HCTR2 must be enabled. Also, fast implementations of XCTR and
+POLYVAL should be enabled, e.g. CRYPTO_POLYVAL_ARM64_CE and
+CRYPTO_AES_ARM64_CE_BLK for ARM64.
+
 New encryption modes can be added relatively easily, without changes
 to individual filesystems.  However, authenticated encryption (AE)
 modes are not currently supported because of the difficulty of dealing
@@ -404,11 +413,11 @@  alternatively has the file's nonce (for `DIRECT_KEY policies`_) or
 inode number (for `IV_INO_LBLK_64 policies`_) included in the IVs.
 Thus, IV reuse is limited to within a single directory.
 
-With CTS-CBC, the IV reuse means that when the plaintext filenames
-share a common prefix at least as long as the cipher block size (16
-bytes for AES), the corresponding encrypted filenames will also share
-a common prefix.  This is undesirable.  Adiantum does not have this
-weakness, as it is a wide-block encryption mode.
+With CTS-CBC, the IV reuse means that when the plaintext filenames share a
+common prefix at least as long as the cipher block size (16 bytes for AES), the
+corresponding encrypted filenames will also share a common prefix.  This is
+undesirable.  Adiantum and HCTR2 do not have this weakness, as they are
+wide-block encryption modes.
 
 All supported filenames encryption modes accept any plaintext length
 >= 16 bytes; cipher block alignment is not required.  However,
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 5b0a9e6478b5..d8617d01f7bd 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -31,7 +31,7 @@ 
 #define FSCRYPT_CONTEXT_V2	2
 
 /* Keep this in sync with include/uapi/linux/fscrypt.h */
-#define FSCRYPT_MODE_MAX	FSCRYPT_MODE_ADIANTUM
+#define FSCRYPT_MODE_MAX	FSCRYPT_MODE_AES_256_HCTR2
 
 struct fscrypt_context_v1 {
 	u8 version; /* FSCRYPT_CONTEXT_V1 */
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index eede186b04ce..ae24b581d3d7 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -53,6 +53,13 @@  struct fscrypt_mode fscrypt_modes[] = {
 		.ivsize = 32,
 		.blk_crypto_mode = BLK_ENCRYPTION_MODE_ADIANTUM,
 	},
+	[FSCRYPT_MODE_AES_256_HCTR2] = {
+		.friendly_name = "HCTR2",
+		.cipher_str = "hctr2(aes)",
+		.keysize = 32,
+		.security_strength = 32,
+		.ivsize = 32,
+	},
 };
 
 static DEFINE_MUTEX(fscrypt_mode_key_setup_mutex);
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index ed3d623724cd..fa8bdb8c76b7 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -54,6 +54,10 @@  static bool fscrypt_valid_enc_modes(u32 contents_mode, u32 filenames_mode)
 	    filenames_mode == FSCRYPT_MODE_ADIANTUM)
 		return true;
 
+	if (contents_mode == FSCRYPT_MODE_AES_256_XTS &&
+	    filenames_mode == FSCRYPT_MODE_AES_256_HCTR2)
+		return true;
+
 	return false;
 }
 
diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index 9f4428be3e36..a756b29afcc2 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -27,7 +27,8 @@ 
 #define FSCRYPT_MODE_AES_128_CBC		5
 #define FSCRYPT_MODE_AES_128_CTS		6
 #define FSCRYPT_MODE_ADIANTUM			9
-/* If adding a mode number > 9, update FSCRYPT_MODE_MAX in fscrypt_private.h */
+#define FSCRYPT_MODE_AES_256_HCTR2		10
+/* If adding a mode number > 10, update FSCRYPT_MODE_MAX in fscrypt_private.h */
 
 /*
  * Legacy policy version; ad-hoc KDF and no key verification.
diff --git a/tools/include/uapi/linux/fscrypt.h b/tools/include/uapi/linux/fscrypt.h
index 9f4428be3e36..a756b29afcc2 100644
--- a/tools/include/uapi/linux/fscrypt.h
+++ b/tools/include/uapi/linux/fscrypt.h
@@ -27,7 +27,8 @@ 
 #define FSCRYPT_MODE_AES_128_CBC		5
 #define FSCRYPT_MODE_AES_128_CTS		6
 #define FSCRYPT_MODE_ADIANTUM			9
-/* If adding a mode number > 9, update FSCRYPT_MODE_MAX in fscrypt_private.h */
+#define FSCRYPT_MODE_AES_256_HCTR2		10
+/* If adding a mode number > 10, update FSCRYPT_MODE_MAX in fscrypt_private.h */
 
 /*
  * Legacy policy version; ad-hoc KDF and no key verification.