diff mbox series

[v2,7/8] xfs_io/encrypt: add 'rm_enckey' command

Message ID 20190920001822.257411-8-ebiggers@kernel.org (mailing list archive)
State Superseded
Headers show
Series xfsprogs: support fscrypt API additions in xfs_io | expand

Commit Message

Eric Biggers Sept. 20, 2019, 12:18 a.m. UTC
From: Eric Biggers <ebiggers@google.com>

Add a 'rm_enckey' command to xfs_io, to provide a command-line interface
to the FS_IOC_REMOVE_ENCRYPTION_KEY and
FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS ioctls.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 io/encrypt.c      | 75 +++++++++++++++++++++++++++++++++++++++++++++++
 man/man8/xfs_io.8 | 15 ++++++++++
 2 files changed, 90 insertions(+)

Comments

Eric Sandeen Sept. 24, 2019, 10:39 p.m. UTC | #1
On 9/19/19 7:18 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Add a 'rm_enckey' command to xfs_io, to provide a command-line interface
> to the FS_IOC_REMOVE_ENCRYPTION_KEY and
> FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS ioctls.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  io/encrypt.c      | 75 +++++++++++++++++++++++++++++++++++++++++++++++
>  man/man8/xfs_io.8 | 15 ++++++++++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/io/encrypt.c b/io/encrypt.c
> index d38ac595..7531c4ad 100644
> --- a/io/encrypt.c
> +++ b/io/encrypt.c
> @@ -139,6 +139,7 @@ struct fscrypt_get_key_status_arg {
>  static cmdinfo_t get_encpolicy_cmd;
>  static cmdinfo_t set_encpolicy_cmd;
>  static cmdinfo_t add_enckey_cmd;
> +static cmdinfo_t rm_enckey_cmd;
>  
>  static void
>  get_encpolicy_help(void)
> @@ -200,6 +201,21 @@ add_enckey_help(void)
>  "\n"));
>  }
>  
> +static void
> +rm_enckey_help(void)
> +{
> +	printf(_(
> +"\n"
> +" remove an encryption key from the filesystem\n"
> +"\n"
> +" Examples:\n"
> +" 'rm_enckey 0000111122223333' - remove key for v1 policies w/ given descriptor\n"
> +" 'rm_enckey 00001111222233334444555566667777' - remove key for v2 policies w/ given identifier\n"
> +"\n"
> +" -a -- remove key for all users who have added it (privileged operation)\n"
> +"\n"));
> +}
> +
>  static const struct {
>  	__u8 mode;
>  	const char *name;
> @@ -693,6 +709,54 @@ out:
>  	return 0;
>  }
>  
> +static int
> +rm_enckey_f(int argc, char **argv)
> +{
> +	int c;
> +	struct fscrypt_remove_key_arg arg;
> +	int ioc = FS_IOC_REMOVE_ENCRYPTION_KEY;
> +
> +	memset(&arg, 0, sizeof(arg));
> +
> +	while ((c = getopt(argc, argv, "a")) != EOF) {
> +		switch (c) {
> +		case 'a':
> +			ioc = FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS;
> +			break;
> +		default:
> +			return command_usage(&rm_enckey_cmd);
> +		}
> +	}
> +	argc -= optind;
> +	argv += optind;
> +
> +	if (argc != 1)
> +		return command_usage(&rm_enckey_cmd);
> +
> +	if (str2keyspec(argv[0], -1, &arg.key_spec) < 0)
> +		return 0;
> +
> +	if (ioctl(file->fd, ioc, &arg) != 0) {
> +		fprintf(stderr, "Error removing encryption key: %s\n",
> +			strerror(errno));
> +		exitcode = 1;
> +		return 0;
> +	}
> +	if (arg.removal_status_flags &
> +	    FSCRYPT_KEY_REMOVAL_STATUS_FLAG_OTHER_USERS) {
> +		printf("Removed user's claim to encryption key with %s %s\n",
> +		       keyspectype(&arg.key_spec), keyspec2str(&arg.key_spec));
> +	} else if (arg.removal_status_flags &
> +		   FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY) {
> +		printf("Removed encryption key with %s %s, but files still busy\n",
> +		       keyspectype(&arg.key_spec), keyspec2str(&arg.key_spec));
> +	} else {
> +		printf("Removed encryption key with %s %s\n",
> +		       keyspectype(&arg.key_spec), keyspec2str(&arg.key_spec));
> +	}
> +	return 0;
> +}
> +
>  void
>  encrypt_init(void)
>  {
> @@ -726,7 +790,18 @@ encrypt_init(void)
>  	add_enckey_cmd.oneline = _("add an encryption key to the filesystem");
>  	add_enckey_cmd.help = add_enckey_help;
>  
> +	rm_enckey_cmd.name = "rm_enckey";
> +	rm_enckey_cmd.cfunc = rm_enckey_f;
> +	rm_enckey_cmd.args = _("keyspec");

can you add "-a" to the args for the shorthelp/args here please?

I don't know anything about this stuff.  :(  Is it to be used as:

rm_enckey -a <keyspec> ?

> +	rm_enckey_cmd.argmin = 0;
> +	rm_enckey_cmd.argmax = -1;

I wonder if min/max should be 1/2 but eh, I think you catch wrong counts
in the function itself, just without the explicit message the built-in
checkers would give.

> +	rm_enckey_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> +	rm_enckey_cmd.oneline =
> +		_("remove an encryption key from the filesystem");
> +	rm_enckey_cmd.help = rm_enckey_help;
> +
>  	add_command(&get_encpolicy_cmd);
>  	add_command(&set_encpolicy_cmd);
>  	add_command(&add_enckey_cmd);
> +	add_command(&rm_enckey_cmd);
>  }
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index 7d6a23fe..a6894778 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -764,6 +764,21 @@ Otherwise, the key is added as a v2 policy key, and on success the resulting
>  .RE
>  .PD
>  .TP
> +.BI "rm_enckey " keyspec

show [-a] here as well?

> +On filesystems that support encryption, remove an encryption key from the
> +filesystem containing the currently open file.
> +.I keyspec
> +is a hex string specifying the key to remove, as a 16-character "key descriptor"
> +or a 32-character "key identifier".
> +.RS 1.0i
> +.PD 0
> +.TP 0.4i
> +.BI \-a
> +Remove the key for all users who have added it, not just the current user.  This
> +is a privileged operation.
> +.RE
> +.PD
> +.TP
>  .BR lsattr " [ " \-R " | " \-D " | " \-a " | " \-v " ]"
>  List extended inode flags on the currently open file. If the
>  .B \-R
>
Eric Biggers Sept. 25, 2019, 11:11 p.m. UTC | #2
On Tue, Sep 24, 2019 at 05:39:02PM -0500, Eric Sandeen wrote:
> On 9/19/19 7:18 PM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Add a 'rm_enckey' command to xfs_io, to provide a command-line interface
> > to the FS_IOC_REMOVE_ENCRYPTION_KEY and
> > FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS ioctls.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >  io/encrypt.c      | 75 +++++++++++++++++++++++++++++++++++++++++++++++
> >  man/man8/xfs_io.8 | 15 ++++++++++
> >  2 files changed, 90 insertions(+)
> > 
> > diff --git a/io/encrypt.c b/io/encrypt.c
> > index d38ac595..7531c4ad 100644
> > --- a/io/encrypt.c
> > +++ b/io/encrypt.c
> > @@ -139,6 +139,7 @@ struct fscrypt_get_key_status_arg {
> >  static cmdinfo_t get_encpolicy_cmd;
> >  static cmdinfo_t set_encpolicy_cmd;
> >  static cmdinfo_t add_enckey_cmd;
> > +static cmdinfo_t rm_enckey_cmd;
> >  
> >  static void
> >  get_encpolicy_help(void)
> > @@ -200,6 +201,21 @@ add_enckey_help(void)
> >  "\n"));
> >  }
> >  
> > +static void
> > +rm_enckey_help(void)
> > +{
> > +	printf(_(
> > +"\n"
> > +" remove an encryption key from the filesystem\n"
> > +"\n"
> > +" Examples:\n"
> > +" 'rm_enckey 0000111122223333' - remove key for v1 policies w/ given descriptor\n"
> > +" 'rm_enckey 00001111222233334444555566667777' - remove key for v2 policies w/ given identifier\n"
> > +"\n"
> > +" -a -- remove key for all users who have added it (privileged operation)\n"
> > +"\n"));
> > +}
> > +
> >  static const struct {
> >  	__u8 mode;
> >  	const char *name;
> > @@ -693,6 +709,54 @@ out:
> >  	return 0;
> >  }
> >  
> > +static int
> > +rm_enckey_f(int argc, char **argv)
> > +{
> > +	int c;
> > +	struct fscrypt_remove_key_arg arg;
> > +	int ioc = FS_IOC_REMOVE_ENCRYPTION_KEY;
> > +
> > +	memset(&arg, 0, sizeof(arg));
> > +
> > +	while ((c = getopt(argc, argv, "a")) != EOF) {
> > +		switch (c) {
> > +		case 'a':
> > +			ioc = FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS;
> > +			break;
> > +		default:
> > +			return command_usage(&rm_enckey_cmd);
> > +		}
> > +	}
> > +	argc -= optind;
> > +	argv += optind;
> > +
> > +	if (argc != 1)
> > +		return command_usage(&rm_enckey_cmd);
> > +
> > +	if (str2keyspec(argv[0], -1, &arg.key_spec) < 0)
> > +		return 0;
> > +
> > +	if (ioctl(file->fd, ioc, &arg) != 0) {
> > +		fprintf(stderr, "Error removing encryption key: %s\n",
> > +			strerror(errno));
> > +		exitcode = 1;
> > +		return 0;
> > +	}
> > +	if (arg.removal_status_flags &
> > +	    FSCRYPT_KEY_REMOVAL_STATUS_FLAG_OTHER_USERS) {
> > +		printf("Removed user's claim to encryption key with %s %s\n",
> > +		       keyspectype(&arg.key_spec), keyspec2str(&arg.key_spec));
> > +	} else if (arg.removal_status_flags &
> > +		   FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY) {
> > +		printf("Removed encryption key with %s %s, but files still busy\n",
> > +		       keyspectype(&arg.key_spec), keyspec2str(&arg.key_spec));
> > +	} else {
> > +		printf("Removed encryption key with %s %s\n",
> > +		       keyspectype(&arg.key_spec), keyspec2str(&arg.key_spec));
> > +	}
> > +	return 0;
> > +}
> > +
> >  void
> >  encrypt_init(void)
> >  {
> > @@ -726,7 +790,18 @@ encrypt_init(void)
> >  	add_enckey_cmd.oneline = _("add an encryption key to the filesystem");
> >  	add_enckey_cmd.help = add_enckey_help;
> >  
> > +	rm_enckey_cmd.name = "rm_enckey";
> > +	rm_enckey_cmd.cfunc = rm_enckey_f;
> > +	rm_enckey_cmd.args = _("keyspec");
> 
> can you add "-a" to the args for the shorthelp/args here please?

Will do.

> 
> I don't know anything about this stuff.  :(  Is it to be used as:
> 
> rm_enckey -a <keyspec> ?

Yes, -a is just an optional argument.  Nothing special.

> 
> > +	rm_enckey_cmd.argmin = 0;
> > +	rm_enckey_cmd.argmax = -1;
> 
> I wonder if min/max should be 1/2 but eh, I think you catch wrong counts
> in the function itself, just without the explicit message the built-in
> checkers would give.

Well since there's only one option, we *could* pretend that it's an optional
positional argument, and do argmin=1, argmax=2, and

	if (argc == 2) {
		if (strcmp(argv[1], "-a") != 0)
			return command_usage(&rm_enckey_cmd);
		ioc = FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS;
		argv++;
	}

But all that would need to be changed if/when a second option is added.
IMO it's better to handle it in the standard way.

> 
> > +	rm_enckey_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> > +	rm_enckey_cmd.oneline =
> > +		_("remove an encryption key from the filesystem");
> > +	rm_enckey_cmd.help = rm_enckey_help;
> > +
> >  	add_command(&get_encpolicy_cmd);
> >  	add_command(&set_encpolicy_cmd);
> >  	add_command(&add_enckey_cmd);
> > +	add_command(&rm_enckey_cmd);
> >  }
> > diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> > index 7d6a23fe..a6894778 100644
> > --- a/man/man8/xfs_io.8
> > +++ b/man/man8/xfs_io.8
> > @@ -764,6 +764,21 @@ Otherwise, the key is added as a v2 policy key, and on success the resulting
> >  .RE
> >  .PD
> >  .TP
> > +.BI "rm_enckey " keyspec
> 
> show [-a] here as well?
> 

Will do.

- Eric
diff mbox series

Patch

diff --git a/io/encrypt.c b/io/encrypt.c
index d38ac595..7531c4ad 100644
--- a/io/encrypt.c
+++ b/io/encrypt.c
@@ -139,6 +139,7 @@  struct fscrypt_get_key_status_arg {
 static cmdinfo_t get_encpolicy_cmd;
 static cmdinfo_t set_encpolicy_cmd;
 static cmdinfo_t add_enckey_cmd;
+static cmdinfo_t rm_enckey_cmd;
 
 static void
 get_encpolicy_help(void)
@@ -200,6 +201,21 @@  add_enckey_help(void)
 "\n"));
 }
 
+static void
+rm_enckey_help(void)
+{
+	printf(_(
+"\n"
+" remove an encryption key from the filesystem\n"
+"\n"
+" Examples:\n"
+" 'rm_enckey 0000111122223333' - remove key for v1 policies w/ given descriptor\n"
+" 'rm_enckey 00001111222233334444555566667777' - remove key for v2 policies w/ given identifier\n"
+"\n"
+" -a -- remove key for all users who have added it (privileged operation)\n"
+"\n"));
+}
+
 static const struct {
 	__u8 mode;
 	const char *name;
@@ -693,6 +709,54 @@  out:
 	return 0;
 }
 
+static int
+rm_enckey_f(int argc, char **argv)
+{
+	int c;
+	struct fscrypt_remove_key_arg arg;
+	int ioc = FS_IOC_REMOVE_ENCRYPTION_KEY;
+
+	memset(&arg, 0, sizeof(arg));
+
+	while ((c = getopt(argc, argv, "a")) != EOF) {
+		switch (c) {
+		case 'a':
+			ioc = FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS;
+			break;
+		default:
+			return command_usage(&rm_enckey_cmd);
+		}
+	}
+	argc -= optind;
+	argv += optind;
+
+	if (argc != 1)
+		return command_usage(&rm_enckey_cmd);
+
+	if (str2keyspec(argv[0], -1, &arg.key_spec) < 0)
+		return 0;
+
+	if (ioctl(file->fd, ioc, &arg) != 0) {
+		fprintf(stderr, "Error removing encryption key: %s\n",
+			strerror(errno));
+		exitcode = 1;
+		return 0;
+	}
+	if (arg.removal_status_flags &
+	    FSCRYPT_KEY_REMOVAL_STATUS_FLAG_OTHER_USERS) {
+		printf("Removed user's claim to encryption key with %s %s\n",
+		       keyspectype(&arg.key_spec), keyspec2str(&arg.key_spec));
+	} else if (arg.removal_status_flags &
+		   FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY) {
+		printf("Removed encryption key with %s %s, but files still busy\n",
+		       keyspectype(&arg.key_spec), keyspec2str(&arg.key_spec));
+	} else {
+		printf("Removed encryption key with %s %s\n",
+		       keyspectype(&arg.key_spec), keyspec2str(&arg.key_spec));
+	}
+	return 0;
+}
+
 void
 encrypt_init(void)
 {
@@ -726,7 +790,18 @@  encrypt_init(void)
 	add_enckey_cmd.oneline = _("add an encryption key to the filesystem");
 	add_enckey_cmd.help = add_enckey_help;
 
+	rm_enckey_cmd.name = "rm_enckey";
+	rm_enckey_cmd.cfunc = rm_enckey_f;
+	rm_enckey_cmd.args = _("keyspec");
+	rm_enckey_cmd.argmin = 0;
+	rm_enckey_cmd.argmax = -1;
+	rm_enckey_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	rm_enckey_cmd.oneline =
+		_("remove an encryption key from the filesystem");
+	rm_enckey_cmd.help = rm_enckey_help;
+
 	add_command(&get_encpolicy_cmd);
 	add_command(&set_encpolicy_cmd);
 	add_command(&add_enckey_cmd);
+	add_command(&rm_enckey_cmd);
 }
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 7d6a23fe..a6894778 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -764,6 +764,21 @@  Otherwise, the key is added as a v2 policy key, and on success the resulting
 .RE
 .PD
 .TP
+.BI "rm_enckey " keyspec
+On filesystems that support encryption, remove an encryption key from the
+filesystem containing the currently open file.
+.I keyspec
+is a hex string specifying the key to remove, as a 16-character "key descriptor"
+or a 32-character "key identifier".
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.BI \-a
+Remove the key for all users who have added it, not just the current user.  This
+is a privileged operation.
+.RE
+.PD
+.TP
 .BR lsattr " [ " \-R " | " \-D " | " \-a " | " \-v " ]"
 List extended inode flags on the currently open file. If the
 .B \-R