On Wed, Oct 07, 2020 at 12:42:09PM -0700, jaegeuk@xxxxxxxxxx wrote:
> Hi Satya,
>
> On 10/05, Satya Tangirala wrote:
> > This patch introduces two new options '-A' and '-M' for specifying metadata
> > crypt options. '-A' takes the desired metadata encryption algorithm as
> > argument. '-M' takes the linux key_serial of the metadata encryption key as
> > the argument. The keyring key provided must be of a key type that supports
> > reading the payload from userspace.
>
> Could you please update manpages as well?
>
Done
> > diff --git a/lib/f2fs_metadata_crypt.c b/lib/f2fs_metadata_crypt.c
> > new file mode 100644
> > index 0000000..faf399a
> > --- /dev/null
> > +++ b/lib/f2fs_metadata_crypt.c
> > @@ -0,0 +1,226 @@
> > +/**
> > + * f2fs_metadata_crypt.c
> > + *
> > + * Copyright (c) 2020 Google LLC
> > + *
> > + * Dual licensed under the GPL or LGPL version 2 licenses.
> > + */
> > +#include <string.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <sys/socket.h>
> > +#include <linux/if_alg.h>
> > +#include <linux/socket.h>
> > +#include <assert.h>
> > +#include <errno.h>
> > +#include <keyutils.h>
> > +
> > +#include "f2fs_fs.h"
> > +#include "f2fs_metadata_crypt.h"
> > +
> > +extern struct f2fs_configuration c;
> > +struct f2fs_crypt_mode {
> > + const char *friendly_name;
> > + const char *cipher_str;
> > + unsigned int keysize;
> > + unsigned int ivlen;
> > +} f2fs_crypt_modes[] = {
> > + [FSCRYPT_MODE_AES_256_XTS] = {
> > + .friendly_name = "AES-256-XTS",
> > + .cipher_str = "xts(aes)",
> > + .keysize = 64,
> > + .ivlen = 16,
> > + },
> > + [FSCRYPT_MODE_ADIANTUM] = {
> > + .friendly_name = "Adiantum",
> > + .cipher_str = "adiantum(xchacha12,aes)",
> > + .keysize = 32,
> > + .ivlen = 32,
> > + },
> > +};
> > +#define MAX_IV_LEN 32
> > +
> > +void f2fs_print_crypt_algs(void)
> > +{
> > + int i;
> > +
> > + for (i = 1; i <= __FSCRYPT_MODE_MAX; i++) {
> > + if (!f2fs_crypt_modes[i].friendly_name)
> > + continue;
> > + MSG(0, "\t%s\n", f2fs_crypt_modes[i].friendly_name);
> > + }
> > +}
> > +
> > +int f2fs_get_crypt_alg(const char *optarg)
> > +{
> > + int i;
> > +
> > + for (i = 1; i <= __FSCRYPT_MODE_MAX; i++) {
> > + if (f2fs_crypt_modes[i].friendly_name &&
> > + !strcmp(f2fs_crypt_modes[i].friendly_name, optarg)) {
> > + return i;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int f2fs_metadata_process_key(const char *key_serial_str)
> > +{
> > + key_serial_t key_serial = strtol(key_serial_str, NULL, 10);
> > +
> > + c.metadata_crypt_key_len =
> > + keyctl_read_alloc(key_serial, (void **)&c.metadata_crypt_key);
> > +
> > + if (c.metadata_crypt_key_len < 0)
> > + return errno;
> > +
> > + return 0;
> > +}
> > +
> > +int f2fs_metadata_verify_args(void)
> > +{
> > + /* If neither specified, nothing to do */
> > + if (!c.metadata_crypt_key && !c.metadata_crypt_alg)
> > + return 0;
> > +
> > + /* We need both specified */
> > + if (!c.metadata_crypt_key || !c.metadata_crypt_alg)
> > + return -EINVAL;
> > +
> > + if (c.metadata_crypt_key_len !=
> > + f2fs_crypt_modes[c.metadata_crypt_alg].keysize) {
> > + MSG(0, "\tMetadata encryption key length %d didn't match required size %d\n",
> > + c.metadata_crypt_key_len,
> > + f2fs_crypt_modes[c.metadata_crypt_alg].keysize);
> > +
> > + return -EINVAL;
> > + }
>
> Need to check sparse mode here?
>
I tried to support sparse mode with metadata encryption in v2 (that I
just sent out), but I haven't been able to even compile or test it yet.
Would you happen to know where I might find some info on how to compile
and test sparse mode?
> And, what about multiple partition case?
>
IIUC I think multiple devices are handled correctly by the code - is there
something I'm missing?
> > @@ -138,6 +147,14 @@ static void f2fs_parse_options(int argc, char *argv[])
> > case 'a':
> > c.heap = atoi(optarg);
> > break;
> > + case 'A':
> > + c.metadata_crypt_alg = f2fs_get_crypt_alg(optarg);
> > + if (c.metadata_crypt_alg < 0) {
> > + MSG(0, "Error: invalid crypt algorithm specified. The choices are:");
> > + f2fs_print_crypt_algs();
> > + exit(1);
> > + }
> > + break;
> > case 'c':
> > if (c.ndevs >= MAX_DEVICES) {
> > MSG(0, "Error: Too many devices\n");
> > @@ -178,6 +195,12 @@ static void f2fs_parse_options(int argc, char *argv[])
> > case 'm':
> > c.zoned_mode = 1;
> > break;
> > + case 'M':
> > + if (f2fs_metadata_process_key(optarg)) {
> > + MSG(0, "Error: Invalid metadata key\n");
> > + mkfs_usage();
> > + }
> > + break;
> > case 'o':
> > c.overprovision = atof(optarg);
> > break;
> > @@ -244,6 +267,14 @@ static void f2fs_parse_options(int argc, char *argv[])
> > }
> > }
> >
> > + if ((!!c.metadata_crypt_key) != (!!c.metadata_crypt_alg)) {
> > + MSG(0, "\tError: Both the metadata crypt key and crypt algorithm must be specified!");
> > + exit(1);
> > + }
> > +
> > + if (f2fs_metadata_verify_args())
> > + exit(1);
> > +
> > add_default_options();
>
> Need to check options after add_default_options()?
>
As in, you're suggesting moving the metadata_crypt_key and
metadata_crypt_alg check and the
+ if (f2fs_metadata_verify_args())
to below the add_default_options() call? If so, I'll do that in v3 of
this patch series
> Thanks,
>
> >
> > if (!(c.feature & cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR))) {
> > --
> > 2.28.0.806.g8561365e88-goog
|