| Message ID | 20200707082729.85058-1-flo@geekplace.eu |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] e4crypt: if salt is explicitly provided to add_key, then use it | expand |
On Tue, Jul 07, 2020 at 10:27:30AM +0200, Florian Schmaus wrote: > Providing -S and a path to 'add_key' previously exhibit an unintuitive exhibit => exhibited > behavior: instead of using the salt explicitly provided by the user, > e4crypt would use the salt obtained via EXT4_IOC_GET_ENCRYPTION_PWSALT > on the path. This was because set_policy() was still called with NULL > as salt. > > With this change we now remember the explicitly provided salt (if any) > and use it as argument for set_policy(). > > Eventually > > e4crypt add_key -S s:my-spicy-salt /foo > > will now actually use 'my-spicy-salt' and not something else as salt > for the policy set on /foo. > > Signed-off-by: Florian Schmaus <flo@geekplace.eu> > --- > > Notes: > - Clarify -S description in man page. > - Do not store a reference to salt_list entry, as it > could be reallocated causing a use-after-free. > - Only parse the salts of the path arguments if no > salt was explicitly specified. > > misc/e4crypt.8.in | 4 +++- > misc/e4crypt.c | 18 ++++++++++++++---- > 2 files changed, 17 insertions(+), 5 deletions(-) > > diff --git a/misc/e4crypt.8.in b/misc/e4crypt.8.in > index 75b968a0..fe9372cf 100644 > --- a/misc/e4crypt.8.in > +++ b/misc/e4crypt.8.in > @@ -48,7 +48,9 @@ values are 4, 8, 16, and 32. > If one or more directory paths are specified, e4crypt will try to > set the policy of those directories to use the key just added by the > .B add_key > -command. > +command. If a salt was explicitly specified, then it will be used > +to derive the encryption key of those directories. Otherwise a > +directory-specific default salt will be used. > .TP > .B e4crypt get_policy \fIpath\fR ... > Print the policy for the directories specified on the command line. > diff --git a/misc/e4crypt.c b/misc/e4crypt.c > index 2ae6254a..67d25d88 100644 > --- a/misc/e4crypt.c > +++ b/misc/e4crypt.c > @@ -26,6 +26,7 @@ > #include <getopt.h> > #include <dirent.h> > #include <errno.h> > +#include <stdbool.h> I'd like to use <stdbool.h> too, but I'm not sure if it's allowed in e2fsprogs; this would be the first use. Everywhere else seems to just use int, 0, and 1. Ted, is stdbool.h allowed in e2fsprogs? > + if (!explicit_salt) > + for (i = optind; i < argc; i++) > + parse_salt(argv[i], PARSE_FLAGS_FORCE_FN); There should be braces at the outer level (following Linux kernel coding style): if (!explicit_salt) { for (i = optind; i < argc; i++) parse_salt(argv[i], PARSE_FLAGS_FORCE_FN); } Otherwise this patch looks fine. Hopefully people aren't depending on this bug being present. - Eric
On Tue, Jul 07, 2020 at 10:27:30AM +0200, Florian Schmaus wrote: > Providing -S and a path to 'add_key' previously exhibit an unintuitive > behavior: instead of using the salt explicitly provided by the user, > e4crypt would use the salt obtained via EXT4_IOC_GET_ENCRYPTION_PWSALT > on the path. This was because set_policy() was still called with NULL > as salt. > > With this change we now remember the explicitly provided salt (if any) > and use it as argument for set_policy(). > > Eventually > > e4crypt add_key -S s:my-spicy-salt /foo > > will now actually use 'my-spicy-salt' and not something else as salt > for the policy set on /foo. > > Signed-off-by: Florian Schmaus <flo@geekplace.eu> Applied, with the spell correction Eric pointed out. - Ted
diff --git a/misc/e4crypt.8.in b/misc/e4crypt.8.in index 75b968a0..fe9372cf 100644 --- a/misc/e4crypt.8.in +++ b/misc/e4crypt.8.in @@ -48,7 +48,9 @@ values are 4, 8, 16, and 32. If one or more directory paths are specified, e4crypt will try to set the policy of those directories to use the key just added by the .B add_key -command. +command. If a salt was explicitly specified, then it will be used +to derive the encryption key of those directories. Otherwise a +directory-specific default salt will be used. .TP .B e4crypt get_policy \fIpath\fR ... Print the policy for the directories specified on the command line. diff --git a/misc/e4crypt.c b/misc/e4crypt.c index 2ae6254a..67d25d88 100644 --- a/misc/e4crypt.c +++ b/misc/e4crypt.c @@ -26,6 +26,7 @@ #include <getopt.h> #include <dirent.h> #include <errno.h> +#include <stdbool.h> #include <stdarg.h> #include <stdio.h> #include <stdlib.h> @@ -652,6 +653,7 @@ static void do_help(int argc, char **argv, const struct cmd_desc *cmd); static void do_add_key(int argc, char **argv, const struct cmd_desc *cmd) { struct salt *salt; + bool explicit_salt = false; char *keyring = NULL; int i, opt, pad = 4; unsigned j; @@ -666,8 +668,13 @@ static void do_add_key(int argc, char **argv, const struct cmd_desc *cmd) pad = atoi(optarg); break; case 'S': + if (explicit_salt) { + fputs("May only provide -S once\n", stderr); + exit(1); + } /* Salt value for passphrase. */ parse_salt(optarg, 0); + explicit_salt = true; break; case 'v': options |= OPT_VERBOSE; @@ -692,8 +699,9 @@ static void do_add_key(int argc, char **argv, const struct cmd_desc *cmd) exit(1); } validate_paths(argc, argv, optind); - for (i = optind; i < argc; i++) - parse_salt(argv[i], PARSE_FLAGS_FORCE_FN); + if (!explicit_salt) + for (i = optind; i < argc; i++) + parse_salt(argv[i], PARSE_FLAGS_FORCE_FN); printf("Enter passphrase (echo disabled): "); get_passphrase(in_passphrase, sizeof(in_passphrase)); for (j = 0, salt = salt_list; j < num_salt; j++, salt++) { @@ -702,8 +710,10 @@ static void do_add_key(int argc, char **argv, const struct cmd_desc *cmd) generate_key_ref_str(salt); insert_key_into_keyring(keyring, salt); } - if (optind != argc) - set_policy(NULL, pad, argc, argv, optind); + if (optind != argc) { + salt = explicit_salt ? salt_list : NULL; + set_policy(salt, pad, argc, argv, optind); + } clear_secrets(); exit(0); }
Providing -S and a path to 'add_key' previously exhibit an unintuitive behavior: instead of using the salt explicitly provided by the user, e4crypt would use the salt obtained via EXT4_IOC_GET_ENCRYPTION_PWSALT on the path. This was because set_policy() was still called with NULL as salt. With this change we now remember the explicitly provided salt (if any) and use it as argument for set_policy(). Eventually e4crypt add_key -S s:my-spicy-salt /foo will now actually use 'my-spicy-salt' and not something else as salt for the policy set on /foo. Signed-off-by: Florian Schmaus <flo@geekplace.eu> --- Notes: - Clarify -S description in man page. - Do not store a reference to salt_list entry, as it could be reallocated causing a use-after-free. - Only parse the salts of the path arguments if no salt was explicitly specified. misc/e4crypt.8.in | 4 +++- misc/e4crypt.c | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-)