[RFC,0/3] xfstests: test adding filesystem-level fscrypt key via key_id
mbox series

Message ID 20191119223130.228341-1-ebiggers@kernel.org
Headers show
Series
  • xfstests: test adding filesystem-level fscrypt key via key_id
Related show

Message

Eric Biggers Nov. 19, 2019, 10:31 p.m. UTC
This series adds a test which tests adding a key to a filesystem's
fscrypt keyring via an "fscrypt-provisioning" keyring key.  This is an
alternative to the normal method where the raw key is given directly.

I'm sending this out for comment, but it shouldn't be merged until the
corresponding kernel patch has reached mainline.  For more details, see
the kernel patch:
https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/T/#u

This test depends on an xfs_io patch which adds the '-k' option to the
'add_enckey' command, e.g.:

	xfs_io -c "add_enckey -k KEY_ID" MOUNTPOINT

This test is skipped if the needed kernel or xfs_io support is absent.

This has been tested on ext4, f2fs, and ubifs.

To apply cleanly, my other xfstests patch series
"[RFC PATCH 0/5] xfstests: verify ciphertext of IV_INO_LBLK_64 encryption policies"
must be applied first.

This series can also be retrieved from
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
tag "fscrypt-provisioning_2019-11-19".

Eric Biggers (3):
  common/rc: handle option with argument in _require_xfs_io_command()
  common/encrypt: move constant test key to common code
  generic: test adding filesystem-level fscrypt key via key_id

 common/encrypt        |  95 +++++++++++++++++++++----
 common/rc             |   2 +-
 tests/generic/580     |  17 ++---
 tests/generic/806     | 156 ++++++++++++++++++++++++++++++++++++++++++
 tests/generic/806.out |  73 ++++++++++++++++++++
 tests/generic/group   |   1 +
 6 files changed, 316 insertions(+), 28 deletions(-)
 create mode 100644 tests/generic/806
 create mode 100644 tests/generic/806.out

Comments

Jarkko Sakkinen Nov. 27, 2019, 8:45 p.m. UTC | #1
On Tue, Nov 19, 2019 at 02:31:27PM -0800, Eric Biggers wrote:
> This series adds a test which tests adding a key to a filesystem's
> fscrypt keyring via an "fscrypt-provisioning" keyring key.  This is an
> alternative to the normal method where the raw key is given directly.
> 
> I'm sending this out for comment, but it shouldn't be merged until the
> corresponding kernel patch has reached mainline.  For more details, see
> the kernel patch:
> https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/T/#u
> 
> This test depends on an xfs_io patch which adds the '-k' option to the
> 'add_enckey' command, e.g.:
> 
> 	xfs_io -c "add_enckey -k KEY_ID" MOUNTPOINT
> 
> This test is skipped if the needed kernel or xfs_io support is absent.
> 
> This has been tested on ext4, f2fs, and ubifs.
> 
> To apply cleanly, my other xfstests patch series
> "[RFC PATCH 0/5] xfstests: verify ciphertext of IV_INO_LBLK_64 encryption policies"
> must be applied first.
> 
> This series can also be retrieved from
> https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
> tag "fscrypt-provisioning_2019-11-19".
> 
> Eric Biggers (3):
>   common/rc: handle option with argument in _require_xfs_io_command()
>   common/encrypt: move constant test key to common code
>   generic: test adding filesystem-level fscrypt key via key_id
> 
>  common/encrypt        |  95 +++++++++++++++++++++----
>  common/rc             |   2 +-
>  tests/generic/580     |  17 ++---
>  tests/generic/806     | 156 ++++++++++++++++++++++++++++++++++++++++++
>  tests/generic/806.out |  73 ++++++++++++++++++++
>  tests/generic/group   |   1 +
>  6 files changed, 316 insertions(+), 28 deletions(-)
>  create mode 100644 tests/generic/806
>  create mode 100644 tests/generic/806.out
> 
> -- 
> 2.24.0.432.g9d3f5f5b63-goog
> 

I'm newbie with fscrypt so I started by encrypting a directory without
the new feature

sudo tune2fs -O encrypt /dev/sda2
sudo fscrypt setup /
fscrypt encrypt foo

Worked.

Generally speaking I'd appreciate a usage example like here to the
commit message:

https://lwn.net/Articles/692514/

Is this doable?

I might consider trying out the XFS test suite some day but right now it
would be first nice to smoke test the feature quickly.

I think for this patch that would actually be mostly sufficient testing.

/Jarkko
Jarkko Sakkinen Nov. 27, 2019, 8:46 p.m. UTC | #2
On Wed, Nov 27, 2019 at 10:45:36PM +0200, Jarkko Sakkinen wrote:
> On Tue, Nov 19, 2019 at 02:31:27PM -0800, Eric Biggers wrote:
> > This series adds a test which tests adding a key to a filesystem's
> > fscrypt keyring via an "fscrypt-provisioning" keyring key.  This is an
> > alternative to the normal method where the raw key is given directly.
> > 
> > I'm sending this out for comment, but it shouldn't be merged until the
> > corresponding kernel patch has reached mainline.  For more details, see
> > the kernel patch:
> > https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/T/#u
> > 
> > This test depends on an xfs_io patch which adds the '-k' option to the
> > 'add_enckey' command, e.g.:
> > 
> > 	xfs_io -c "add_enckey -k KEY_ID" MOUNTPOINT
> > 
> > This test is skipped if the needed kernel or xfs_io support is absent.
> > 
> > This has been tested on ext4, f2fs, and ubifs.
> > 
> > To apply cleanly, my other xfstests patch series
> > "[RFC PATCH 0/5] xfstests: verify ciphertext of IV_INO_LBLK_64 encryption policies"
> > must be applied first.
> > 
> > This series can also be retrieved from
> > https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
> > tag "fscrypt-provisioning_2019-11-19".
> > 
> > Eric Biggers (3):
> >   common/rc: handle option with argument in _require_xfs_io_command()
> >   common/encrypt: move constant test key to common code
> >   generic: test adding filesystem-level fscrypt key via key_id
> > 
> >  common/encrypt        |  95 +++++++++++++++++++++----
> >  common/rc             |   2 +-
> >  tests/generic/580     |  17 ++---
> >  tests/generic/806     | 156 ++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/806.out |  73 ++++++++++++++++++++
> >  tests/generic/group   |   1 +
> >  6 files changed, 316 insertions(+), 28 deletions(-)
> >  create mode 100644 tests/generic/806
> >  create mode 100644 tests/generic/806.out
> > 
> > -- 
> > 2.24.0.432.g9d3f5f5b63-goog
> > 
> 
> I'm newbie with fscrypt so I started by encrypting a directory without
> the new feature
> 
> sudo tune2fs -O encrypt /dev/sda2
> sudo fscrypt setup /
> fscrypt encrypt foo
> 
> Worked.
> 
> Generally speaking I'd appreciate a usage example like here to the
> commit message:
> 
> https://lwn.net/Articles/692514/
> 
> Is this doable?
> 
> I might consider trying out the XFS test suite some day but right now it
> would be first nice to smoke test the feature quickly.
> 
> I think for this patch that would actually be mostly sufficient testing.

Sorry, this was meant for the kernel patch :-/

/Jarkko
Eric Biggers Nov. 27, 2019, 10:57 p.m. UTC | #3
On Wed, Nov 27, 2019 at 10:45:36PM +0200, Jarkko Sakkinen wrote:
> On Tue, Nov 19, 2019 at 02:31:27PM -0800, Eric Biggers wrote:
> > This series adds a test which tests adding a key to a filesystem's
> > fscrypt keyring via an "fscrypt-provisioning" keyring key.  This is an
> > alternative to the normal method where the raw key is given directly.
> > 
> > I'm sending this out for comment, but it shouldn't be merged until the
> > corresponding kernel patch has reached mainline.  For more details, see
> > the kernel patch:
> > https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/T/#u
> > 
> > This test depends on an xfs_io patch which adds the '-k' option to the
> > 'add_enckey' command, e.g.:
> > 
> > 	xfs_io -c "add_enckey -k KEY_ID" MOUNTPOINT
> > 
> > This test is skipped if the needed kernel or xfs_io support is absent.
> > 
> > This has been tested on ext4, f2fs, and ubifs.
> > 
> > To apply cleanly, my other xfstests patch series
> > "[RFC PATCH 0/5] xfstests: verify ciphertext of IV_INO_LBLK_64 encryption policies"
> > must be applied first.
> > 
> > This series can also be retrieved from
> > https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git
> > tag "fscrypt-provisioning_2019-11-19".
> > 
> > Eric Biggers (3):
> >   common/rc: handle option with argument in _require_xfs_io_command()
> >   common/encrypt: move constant test key to common code
> >   generic: test adding filesystem-level fscrypt key via key_id
> > 
> >  common/encrypt        |  95 +++++++++++++++++++++----
> >  common/rc             |   2 +-
> >  tests/generic/580     |  17 ++---
> >  tests/generic/806     | 156 ++++++++++++++++++++++++++++++++++++++++++
> >  tests/generic/806.out |  73 ++++++++++++++++++++
> >  tests/generic/group   |   1 +
> >  6 files changed, 316 insertions(+), 28 deletions(-)
> >  create mode 100644 tests/generic/806
> >  create mode 100644 tests/generic/806.out
> > 
> > -- 
> > 2.24.0.432.g9d3f5f5b63-goog
> > 
> 
> I'm newbie with fscrypt so I started by encrypting a directory without
> the new feature
> 
> sudo tune2fs -O encrypt /dev/sda2
> sudo fscrypt setup /
> fscrypt encrypt foo
> 
> Worked.
> 
> Generally speaking I'd appreciate a usage example like here to the
> commit message:
> 
> https://lwn.net/Articles/692514/
> 
> Is this doable?
> 
> I might consider trying out the XFS test suite some day but right now it
> would be first nice to smoke test the feature quickly.
> 
> I think for this patch that would actually be mostly sufficient testing.
> 

You could manually do what the xfstest does, which is more or less the following
(requires xfs_io patched with https://patchwork.kernel.org/patch/11252795/):

	mkfs.ext4 -O encrypt /dev/vdc
	mount /dev/vdc /vdc
	keyctl new_session
	payload='\x02\x00\x00\x00\x00\x00\x00\x00'
	for i in {1..64}; do
		payload+="\\x$(printf "%02x" $i)"
	done
	keyid=$(echo -ne "$payload" | keyctl padd fscrypt-provisioning desc @s)
	keyspec=$(xfs_io -c "add_enckey -k $keyid" /vdc | awk '{print $NF}')
	mkdir /vdc/edir
	xfs_io -c "set_encpolicy $keyspec" /vdc/edir
	echo contents > /vdc/edir/file

I'm not yet planning to use this feature in the 'fscrypt' userspace tool
https://github.com/google/fscrypt.  The user who actually wants this feature
(Android) isn't using the 'fscrypt' tool, but rather is using custom code.

Also, the 'fscrypt' tool isn't meant for testing or exposing every corner of the
fscrypt kernel API.  Rather, it's meant to be a user-friendly tool for creating
encrypted directories on "regular" Linux distros, supporting lots of higher
level userspace features like passphrase hashing with Argon2, multiple
protectors, and auto-unlocking directories with PAM.

So for now, the important thing is to have an xfstest that fully tests this new
API.  For that, like the other fscrypt tests, I'm using keyctl and xfs_io to
access the kernel API directly.

Later, if people really need the 'fscrypt' tool to do something that requires
this feature, we can add it.  This would likely take the form of a user-friendly
option to 'fscrypt unlock' rather than a direct translation of the kernel API.

- Eric
Jarkko Sakkinen Dec. 11, 2019, 9:50 a.m. UTC | #4
On Wed, Nov 27, 2019 at 02:57:59PM -0800, Eric Biggers wrote:
> You could manually do what the xfstest does, which is more or less the following
> (requires xfs_io patched with https://patchwork.kernel.org/patch/11252795/):

I postpone testing/reviewing this patch up until its depedencies are in
the mainline.

I'll add these to my tree as soon as we have addressed a critical bug
in tpm_tis:

1. KEYS: remove CONFIG_KEYS_COMPAT
2. KEYS: asymmetric: return ENOMEM if akcipher_request_alloc() fails

Just mentioning that I haven't forgotten them.

/Jarkko
Eric Biggers Dec. 11, 2019, 6 p.m. UTC | #5
Hi Jarkko,

On Wed, Dec 11, 2019 at 11:50:19AM +0200, Jarkko Sakkinen wrote:
> On Wed, Nov 27, 2019 at 02:57:59PM -0800, Eric Biggers wrote:
> > You could manually do what the xfstest does, which is more or less the following
> > (requires xfs_io patched with https://patchwork.kernel.org/patch/11252795/):
> 
> I postpone testing/reviewing this patch up until its depedencies are in
> the mainline.
> 
> I'll add these to my tree as soon as we have addressed a critical bug
> in tpm_tis:
> 
> 1. KEYS: remove CONFIG_KEYS_COMPAT
> 2. KEYS: asymmetric: return ENOMEM if akcipher_request_alloc() fails
> 
> Just mentioning that I haven't forgotten them.
> 

xfstests and xfsprogs are developed separately from the kernel, and their
maintainers don't apply patches that depend on non-mainlined features.  So
unless there are objections to the kernel patch [1], in a couple weeks I'll
apply it to the fscrypt tree for 5.6, and then once it's in mainline I'll resend
the patches for the test.  I've simply sent the test out early as an RFC, in
case it helps reviewing the kernel patch or in case there are early comments.

Again, while you're certainly welcome to manually test the kernel patch, it's
more important that we have test coverage of it in xfstests.

[1] https://lkml.kernel.org/linux-fscrypt/20191119222447.226853-1-ebiggers@kernel.org/

- Eric