[PATCH v3 0/3] Add file-system authentication to BTRFS

[PATCH v3 0/3] Add file-system authentication to BTRFS

[Johannes Thumshirn]

From: Johannes Thumshirn <johannes.thumshirn@wdc.com>

This series adds file-system authentication to BTRFS. 

Unlike other verified file-system techniques like fs-verity the
authenticated version of BTRFS does not need extra meta-data on disk.

This works because in BTRFS every on-disk block has a checksum, for meta-data
the checksum is in the header of each meta-data item. For data blocks, a
separate checksum tree exists, which holds the checksums for each block.

Currently BRTFS supports CRC32C, XXHASH64, SHA256 and Blake2b for checksumming
these blocks. This series adds a new checksum algorithm, HMAC(SHA-256), which
does need an authentication key. When no, or an incoreect authentication key
is supplied no valid checksum can be generated and a read, fsck or scrub
operation would detect invalid or tampered blocks once the file-system is
mounted again with the correct key. 

Getting the key inside the kernel is out of scope of this implementation, the
file-system driver assumes the key is already in the kernel's keyring at mount
time.

There was interest in also using keyed Blake2b from the community, but this
support is not yet included.

I have CCed Eric Biggers and Richard Weinberger in the submission, as they
previously have worked on filesystem authentication and I hope we can get
input from them as well.

Example usage:
Create a file-system with authentication key 0123456
mkfs.btrfs --csum "hmac(sha256)" --auth-key 0123456 /dev/disk

Add the key to the kernel's keyring as keyid 'btrfs:foo'
keyctl add logon btrfs:foo 0123456 @u

Mount the fs using the 'btrfs:foo' key
mount -t btrfs -o auth_key=btrfs:foo,auth_hash_name="hmac(sha256)" /dev/disk /mnt/point

Note, this is a re-base of the work I did when I was still at SUSE, hence the
S-o-b being my SUSE address, while the Author being with my WDC address (to
not generate bouncing mails).

Changes since v2:
- Select CONFIG_CRYPTO_HMAC and CONFIG_KEYS (kbuild robot)
- Fix double free in error path
- Fix memory leak in error path
- Disallow nodatasum and nodatacow when authetication is use (Eric)
- Pass in authentication algorithm as mount option (Eric)
- Don't use the work "replay" in the documentation, as it is wrong and
  harmful in this context (Eric)
- Force key name to begin with 'btrfs:' (Eric)
- Use '4' as on-disk checksum type for HMAC(SHA256) to not have holes in the
  checksum types array.

Changes since v1:
- None, only rebased the series

Johannes Thumshirn (3):
  btrfs: rename btrfs_parse_device_options back to
    btrfs_parse_early_options
  btrfs: add authentication support
  btrfs: document btrfs authentication

 .../filesystems/btrfs-authentication.rst      | 168 ++++++++++++++++++
 fs/btrfs/Kconfig                              |   2 +
 fs/btrfs/ctree.c                              |  22 ++-
 fs/btrfs/ctree.h                              |   5 +-
 fs/btrfs/disk-io.c                            |  71 +++++++-
 fs/btrfs/ioctl.c                              |   7 +-
 fs/btrfs/super.c                              |  65 ++++++-
 include/uapi/linux/btrfs_tree.h               |   1 +
 8 files changed, 326 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/filesystems/btrfs-authentication.rst

-- 
2.26.1

[PATCH v3 1/3] btrfs: rename btrfs_parse_device_options back to btrfs_parse_early_options

[Johannes Thumshirn]

From: Johannes Thumshirn <johannes.thumshirn@wdc.com>

As btrfs_parse_device_options() now doesn't only parse the -o device mount
option but -o auth_key as well, it makes sense to rename it back to
btrfs_parse_early_options().

This reverts commit fa59f27c8c35bbe00af8eff23de446a7f4b048b0.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 fs/btrfs/super.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 438ecba26557..07cec0d16348 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -482,8 +482,9 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 		case Opt_subvolrootid:
 		case Opt_device:
 			/*
-			 * These are parsed by btrfs_parse_subvol_options or
-			 * btrfs_parse_device_options and can be ignored here.
+			 * These are parsed by btrfs_parse_subvol_options
+			 * and btrfs_parse_early_options
+			 * and can be happily ignored here.
 			 */
 			break;
 		case Opt_nodatasum:
@@ -919,8 +920,9 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
  * All other options will be parsed on much later in the mount process and
  * only when we need to allocate a new super block.
  */
-static int btrfs_parse_device_options(const char *options, fmode_t flags,
-				      void *holder)
+static int btrfs_parse_early_options(struct btrfs_fs_info *info,
+				     const char *options, fmode_t flags,
+				     void *holder)
 {
 	substring_t args[MAX_OPT_ARGS];
 	char *device_name, *opts, *orig, *p;
@@ -987,7 +989,7 @@ static int btrfs_parse_subvol_options(const char *options, char **subvol_name,
 
 	/*
 	 * strsep changes the string, duplicate it because
-	 * btrfs_parse_device_options gets called later
+	 * btrfs_parse_early_options gets called later
 	 */
 	opts = kstrdup(options, GFP_KERNEL);
 	if (!opts)
@@ -1551,7 +1553,7 @@ static struct dentry *btrfs_mount_root(struct file_system_type *fs_type,
 	}
 
 	mutex_lock(&uuid_mutex);
-	error = btrfs_parse_device_options(data, mode, fs_type);
+	error = btrfs_parse_early_options(fs_info, data, mode, fs_type);
 	if (error) {
 		mutex_unlock(&uuid_mutex);
 		goto error_fs_info;
-- 
2.26.1

[PATCH v3 2/3] btrfs: add authentication support

[Johannes Thumshirn]

From: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Add authentication support for a BTRFS file-system.

This works, because in BTRFS every meta-data block as well as every
data-block has a own checksum. For meta-data the checksum is in the
meta-data node itself. For data blocks, the checksums are stored in the
checksum tree.

When replacing the checksum algorithm with a keyed hash, like HMAC(SHA256),
a key is needed to mount a verified file-system. This key also needs to be
used at file-system creation time.

We have to used a keyed hash scheme, in contrast to doing a normal
cryptographic hash, to guarantee integrity of the file system, as a
potential attacker could just generate the corresponding cryptographic
hash for forged file-system operations and the changes would go unnoticed.

Having a keyed hash only on the topmost Node of a tree or even just in the
super-block and using cryptographic hashes on the normal meta-data nodes
and checksum tree entries doesn't work either, as the BTRFS B-Tree's Nodes
do not include the checksums of their respective child nodes, but only the
block pointers and offsets where to find them on disk.

Also note, we do not need a incompat R/O flag for this, because if an old
kernel tries to mount an authenticated file-system it will fail the
initial checksum type verification and thus refuses to mount.

The key has to be supplied by the kernel's keyring and the method of
getting the key securely into the kernel is not subject of this patch.

Example usage:
Create a file-system with authentication key 0123456
mkfs.btrfs --csum "hmac(sha256)" --auth-key 0123456 /dev/disk

Add the key to the kernel's keyring as keyid 'btrfs:foo'
keyctl add logon btrfs:foo 0123456 @u

Mount the fs using the 'btrfs:foo' key
mount -o auth_key=btrfs:foo,auth_hash_name="hmac(sha256)" /dev/disk /mnt/point

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>

---
 fs/btrfs/Kconfig                |  2 +
 fs/btrfs/ctree.c                | 22 +++++++++-
 fs/btrfs/ctree.h                |  5 ++-
 fs/btrfs/disk-io.c              | 71 +++++++++++++++++++++++++++++++--
 fs/btrfs/ioctl.c                |  7 +++-
 fs/btrfs/super.c                | 51 ++++++++++++++++++++++-
 include/uapi/linux/btrfs_tree.h |  1 +
 7 files changed, 150 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/Kconfig b/fs/btrfs/Kconfig
index 575636f6491e..b7e9ce25b622 100644
--- a/fs/btrfs/Kconfig
+++ b/fs/btrfs/Kconfig
@@ -8,6 +8,8 @@ config BTRFS_FS
 	select CRYPTO_XXHASH
 	select CRYPTO_SHA256
 	select CRYPTO_BLAKE2B
+	select CRYPTO_HMAC
+	select KEYS
 	select ZLIB_INFLATE
 	select ZLIB_DEFLATE
 	select LZO_COMPRESS
diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
index 746dec22f250..2907bd054dd6 100644
--- a/fs/btrfs/ctree.c
+++ b/fs/btrfs/ctree.c
@@ -31,7 +31,7 @@ static void del_ptr(struct btrfs_root *root, struct btrfs_path *path,
 
 static const struct btrfs_csums {
 	u16		size;
-	const char	name[10];
+	const char	name[12];
 	const char	driver[12];
 } btrfs_csums[] = {
 	[BTRFS_CSUM_TYPE_CRC32] = { .size = 4, .name = "crc32c" },
@@ -39,6 +39,7 @@ static const struct btrfs_csums {
 	[BTRFS_CSUM_TYPE_SHA256] = { .size = 32, .name = "sha256" },
 	[BTRFS_CSUM_TYPE_BLAKE2] = { .size = 32, .name = "blake2b",
 				     .driver = "blake2b-256" },
+	[BTRFS_CSUM_TYPE_HMAC_SHA256] = { .size = 32, .name = "hmac(sha256)" }
 };
 
 int btrfs_super_csum_size(const struct btrfs_super_block *s)
@@ -56,12 +57,29 @@ const char *btrfs_super_csum_name(u16 csum_type)
 	return btrfs_csums[csum_type].name;
 }
 
+static const char *btrfs_auth_csum_driver(struct btrfs_fs_info *fs_info)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(btrfs_csums); i++) {
+		if (!strncmp(fs_info->auth_hash_name, btrfs_csums[i].name,
+			     strlen(btrfs_csums[i].name)))
+			return btrfs_csums[i].driver[0] ?
+				btrfs_csums[i].driver : btrfs_csums[i].name;
+	}
+
+	return NULL;
+}
+
 /*
  * Return driver name if defined, otherwise the name that's also a valid driver
  * name
  */
-const char *btrfs_super_csum_driver(u16 csum_type)
+const char *btrfs_super_csum_driver(struct btrfs_fs_info *info, u16 csum_type)
 {
+	if (btrfs_test_opt(info, AUTH_KEY))
+		return btrfs_auth_csum_driver(info);
+
 	/* csum type is validated at mount time */
 	return btrfs_csums[csum_type].driver[0] ?
 		btrfs_csums[csum_type].driver :
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0a1fa1526c43..b9e4cf5b9fd3 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -931,6 +931,8 @@ struct btrfs_fs_info {
 	struct rb_root swapfile_pins;
 
 	struct crypto_shash *csum_shash;
+	char *auth_key_name;
+	char *auth_hash_name;
 
 	/*
 	 * Number of send operations in progress.
@@ -1239,6 +1241,7 @@ static inline u32 BTRFS_MAX_XATTR_SIZE(const struct btrfs_fs_info *info)
 #define BTRFS_MOUNT_NOLOGREPLAY		(1 << 27)
 #define BTRFS_MOUNT_REF_VERIFY		(1 << 28)
 #define BTRFS_MOUNT_DISCARD_ASYNC	(1 << 29)
+#define BTRFS_MOUNT_AUTH_KEY		(1 << 30)
 
 #define BTRFS_DEFAULT_COMMIT_INTERVAL	(30)
 #define BTRFS_DEFAULT_MAX_INLINE	(2048)
@@ -2186,7 +2189,7 @@ BTRFS_SETGET_STACK_FUNCS(super_uuid_tree_generation, struct btrfs_super_block,
 
 int btrfs_super_csum_size(const struct btrfs_super_block *s);
 const char *btrfs_super_csum_name(u16 csum_type);
-const char *btrfs_super_csum_driver(u16 csum_type);
+const char *btrfs_super_csum_driver(struct btrfs_fs_info *info, u16 csum_type);
 size_t __const btrfs_get_num_csums(void);
 
 
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 714b57553ed6..6ed5d1191cdf 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -17,6 +17,7 @@
 #include <linux/error-injection.h>
 #include <linux/crc32c.h>
 #include <linux/sched/mm.h>
+#include <keys/user-type.h>
 #include <asm/unaligned.h>
 #include <crypto/hash.h>
 #include "ctree.h"
@@ -100,8 +101,10 @@ void __cold btrfs_end_io_wq_exit(void)
 
 static void btrfs_free_csum_hash(struct btrfs_fs_info *fs_info)
 {
-	if (fs_info->csum_shash)
+	if (fs_info->csum_shash) {
 		crypto_free_shash(fs_info->csum_shash);
+		fs_info->csum_shash = NULL;
+	}
 }
 
 /*
@@ -339,6 +342,7 @@ static bool btrfs_supported_super_csum(u16 csum_type)
 	case BTRFS_CSUM_TYPE_XXHASH:
 	case BTRFS_CSUM_TYPE_SHA256:
 	case BTRFS_CSUM_TYPE_BLAKE2:
+	case BTRFS_CSUM_TYPE_HMAC_SHA256:
 		return true;
 	default:
 		return false;
@@ -1509,6 +1513,8 @@ void btrfs_free_fs_info(struct btrfs_fs_info *fs_info)
 	percpu_counter_destroy(&fs_info->dio_bytes);
 	percpu_counter_destroy(&fs_info->dev_replace.bio_counter);
 	btrfs_free_csum_hash(fs_info);
+	kfree(fs_info->auth_key_name);
+	kfree(fs_info->auth_hash_name);
 	btrfs_free_stripe_hash_table(fs_info);
 	btrfs_free_ref_cache(fs_info);
 	kfree(fs_info->balance_ctl);
@@ -2178,10 +2184,16 @@ static int btrfs_init_workqueues(struct btrfs_fs_info *fs_info,
 static int btrfs_init_csum_hash(struct btrfs_fs_info *fs_info, u16 csum_type)
 {
 	struct crypto_shash *csum_shash;
-	const char *csum_driver = btrfs_super_csum_driver(csum_type);
+	const char *csum_driver;
+	struct key *key;
+	const struct user_key_payload *ukp;
+	int err = -EINVAL;
 
-	csum_shash = crypto_alloc_shash(csum_driver, 0, 0);
+	csum_driver = btrfs_super_csum_driver(fs_info, csum_type);
+	if (!csum_driver)
+		return err;
 
+	csum_shash = crypto_alloc_shash(csum_driver, 0, 0);
 	if (IS_ERR(csum_shash)) {
 		btrfs_err(fs_info, "error allocating %s hash for checksum",
 			  csum_driver);
@@ -2190,7 +2202,57 @@ static int btrfs_init_csum_hash(struct btrfs_fs_info *fs_info, u16 csum_type)
 
 	fs_info->csum_shash = csum_shash;
 
-	return 0;
+	/*
+	 * If we're not doing authentication, we're done by now. If we use
+	 * authentication and the auth_hash_name was bogus crypt_alloc_shash
+	 * would have dropped out by now. Validation that both auth_hash_name
+	 * and auth_key_name have been supplied is done in
+	 * btrfs_parse_early_options(), so we should be good to go from here on
+	 * and start authenticating the file-system.
+	 */
+	if (!btrfs_test_opt(fs_info, AUTH_KEY))
+		return 0;
+
+	if (strncmp(fs_info->auth_key_name, "btrfs:", 6)) {
+		btrfs_err(fs_info,
+			  "authentication key must start with 'btrfs:'");
+		goto out_free_hash;
+	}
+
+	key = request_key(&key_type_logon, fs_info->auth_key_name, NULL);
+	if (IS_ERR(key)) {
+		err = PTR_ERR(key);
+		goto out_free_hash;
+	}
+
+	down_read(&key->sem);
+
+	ukp = user_key_payload_locked(key);
+	if (!ukp) {
+		btrfs_err(fs_info, "error getting payload for key %s",
+			  fs_info->auth_key_name);
+		err = -EKEYREVOKED;
+		goto out;
+	}
+
+	err = crypto_shash_setkey(fs_info->csum_shash, ukp->data, ukp->datalen);
+	if (err)
+		btrfs_err(fs_info, "error setting key %s for verification",
+			  fs_info->auth_key_name);
+
+out:
+	if (err) {
+		btrfs_free_csum_hash(fs_info);
+	}
+
+	up_read(&key->sem);
+	key_put(key);
+
+	return err;
+
+out_free_hash:
+	btrfs_free_csum_hash(fs_info);
+	return err;
 }
 
 static int btrfs_replay_log(struct btrfs_fs_info *fs_info,
@@ -3371,6 +3433,7 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device
 	btrfs_stop_all_workers(fs_info);
 	btrfs_free_block_groups(fs_info);
 fail_alloc:
+	btrfs_free_csum_hash(fs_info);
 	btrfs_mapping_tree_free(&fs_info->mapping_tree);
 
 	iput(fs_info->btree_inode);
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 40b729dce91c..59bbb2c860d5 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -242,7 +242,12 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
 	else
 		binode_flags &= ~BTRFS_INODE_DIRSYNC;
 	if (fsflags & FS_NOCOW_FL) {
-		if (S_ISREG(inode->i_mode)) {
+		if (btrfs_test_opt(fs_info, AUTH_KEY)) {
+			btrfs_err(fs_info,
+				  "Cannot set nodatacow or nodatasum on authenticated file-system");
+			ret = -EPERM;
+			goto out_unlock;
+		} else if (S_ISREG(inode->i_mode)) {
 			/*
 			 * It's safe to turn csums off here, no extents exist.
 			 * Otherwise we want the flag to reflect the real COW
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 07cec0d16348..88164b916c28 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -342,6 +342,8 @@ enum {
 	Opt_treelog, Opt_notreelog,
 	Opt_usebackuproot,
 	Opt_user_subvol_rm_allowed,
+	Opt_auth_key,
+	Opt_auth_hash_name,
 
 	/* Deprecated options */
 	Opt_alloc_start,
@@ -410,6 +412,8 @@ static const match_table_t tokens = {
 	{Opt_notreelog, "notreelog"},
 	{Opt_usebackuproot, "usebackuproot"},
 	{Opt_user_subvol_rm_allowed, "user_subvol_rm_allowed"},
+	{Opt_auth_key, "auth_key=%s"},
+	{Opt_auth_hash_name, "auth_hash_name=%s"},
 
 	/* Deprecated options */
 	{Opt_alloc_start, "alloc_start=%s"},
@@ -488,6 +492,11 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 			 */
 			break;
 		case Opt_nodatasum:
+			if (btrfs_test_opt(info, AUTH_KEY)) {
+				btrfs_info(info,
+					   "nodatasum not supported on an authnticated file-system");
+				break;
+			}
 			btrfs_set_and_info(info, NODATASUM,
 					   "setting nodatasum");
 			break;
@@ -503,6 +512,11 @@ int btrfs_parse_options(struct btrfs_fs_info *info, char *options,
 			btrfs_clear_opt(info->mount_opt, NODATASUM);
 			break;
 		case Opt_nodatacow:
+			if (btrfs_test_opt(info, AUTH_KEY)) {
+				btrfs_info(info,
+					   "nodatacow not supported on an authnticated file-system");
+				break;
+			}
 			if (!btrfs_test_opt(info, NODATACOW)) {
 				if (!btrfs_test_opt(info, COMPRESS) ||
 				    !btrfs_test_opt(info, FORCE_COMPRESS)) {
@@ -950,7 +964,8 @@ static int btrfs_parse_early_options(struct btrfs_fs_info *info,
 			continue;
 
 		token = match_token(p, tokens, args);
-		if (token == Opt_device) {
+		switch (token) {
+		case Opt_device:
 			device_name = match_strdup(&args[0]);
 			if (!device_name) {
 				error = -ENOMEM;
@@ -963,9 +978,40 @@ static int btrfs_parse_early_options(struct btrfs_fs_info *info,
 				error = PTR_ERR(device);
 				goto out;
 			}
+			break;
+		case Opt_auth_key:
+			info->auth_key_name = match_strdup(&args[0]);
+			if (!info->auth_key_name) {
+				error = -ENOMEM;
+				goto out;
+			}
+			break;
+		case Opt_auth_hash_name:
+			info->auth_hash_name = match_strdup(&args[0]);
+			if (!info->auth_hash_name) {
+				error = -ENOMEM;
+				goto out;
+			}
+			break;
+		default:
+			break;
 		}
 	}
 
+	/*
+	 * Check that both auth_key_name and auth_hash_name are either present
+	 * or absent and error out if only one of them is set.
+	 */
+	if (info->auth_key_name && info->auth_hash_name) {
+		btrfs_info(info, "doing authentication");
+		btrfs_set_opt(info->mount_opt, AUTH_KEY);
+	} else if ((info->auth_key_name && !info->auth_hash_name) ||
+		   (!info->auth_key_name && info->auth_hash_name)) {
+		btrfs_err(info,
+			  "auth_key and auth_hash_name must be supplied together");
+		error = -EINVAL;
+	}
+
 out:
 	kfree(orig);
 	return error;
@@ -1405,6 +1451,8 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry)
 #endif
 	if (btrfs_test_opt(info, REF_VERIFY))
 		seq_puts(seq, ",ref_verify");
+	if (btrfs_test_opt(info, AUTH_KEY))
+		seq_printf(seq, ",auth_key=%s", info->auth_key_name);
 	seq_printf(seq, ",subvolid=%llu",
 		  BTRFS_I(d_inode(dentry))->root->root_key.objectid);
 	seq_puts(seq, ",subvol=");
@@ -2526,4 +2574,5 @@ MODULE_LICENSE("GPL");
 MODULE_SOFTDEP("pre: crc32c");
 MODULE_SOFTDEP("pre: xxhash64");
 MODULE_SOFTDEP("pre: sha256");
+MODULE_SOFTDEP("pre: hmac");
 MODULE_SOFTDEP("pre: blake2b-256");
diff --git a/include/uapi/linux/btrfs_tree.h b/include/uapi/linux/btrfs_tree.h
index a3f3975df0de..dfc22b995f60 100644
--- a/include/uapi/linux/btrfs_tree.h
+++ b/include/uapi/linux/btrfs_tree.h
@@ -305,6 +305,7 @@ enum btrfs_csum_type {
 	BTRFS_CSUM_TYPE_XXHASH	= 1,
 	BTRFS_CSUM_TYPE_SHA256	= 2,
 	BTRFS_CSUM_TYPE_BLAKE2	= 3,
+	BTRFS_CSUM_TYPE_HMAC_SHA256 = 4,
 };
 
 /*
-- 
2.26.1

Re: [PATCH v3 2/3] btrfs: add authentication support

[David Sterba]

On Thu, May 14, 2020 at 11:24:14AM +0200, Johannes Thumshirn wrote:
> From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Example usage:
> Create a file-system with authentication key 0123456
> mkfs.btrfs --csum "hmac(sha256)" --auth-key 0123456 /dev/disk
> 
> Add the key to the kernel's keyring as keyid 'btrfs:foo'
> keyctl add logon btrfs:foo 0123456 @u
> 
> Mount the fs using the 'btrfs:foo' key
> mount -o auth_key=btrfs:foo,auth_hash_name="hmac(sha256)" /dev/disk /mnt/point

I tried to follow the example but the filesystem does not mount. But
what almost shocked me was the way the key is specified on the userspace
side.

$ mkfs.btrfs --csum "hmac(sha256)" --auth-key 0123456 /dev/disk

"0123456" are the raw bytes of the key? Seriously?

And how it's passed to the hmac code:

 gcry_mac_hd_t mac;
 gcry_mac_open(&mac, GCRY_MAC_HMAC_SHA256, 0, NULL);
 gcry_mac_setkey(mac, fs_info->auth_key, strlen(fs_info->auth_key));
 gcry_mac_write(mac, buf, length);
 gcry_mac_read(mac, out, &length);

Strlen means the key must avoid char 0 and I don't think we want do any
decoding from ascii-hex format, when there's the whole keyctl
infrastructure.

The key for all userspace commands needs to be specified the same way as
for kernel, ie. "--auth-key btrfs:foo" and use the appropriate ioctls to
read the key bytes.

Re: [PATCH v3 2/3] btrfs: add authentication support

[Johannes Thumshirn]

On 27/05/2020 15:25, David Sterba wrote:
> On Thu, May 14, 2020 at 11:24:14AM +0200, Johannes Thumshirn wrote:
>> From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>> Example usage:
>> Create a file-system with authentication key 0123456
>> mkfs.btrfs --csum "hmac(sha256)" --auth-key 0123456 /dev/disk
>>
>> Add the key to the kernel's keyring as keyid 'btrfs:foo'
>> keyctl add logon btrfs:foo 0123456 @u
>>
>> Mount the fs using the 'btrfs:foo' key
>> mount -o auth_key=btrfs:foo,auth_hash_name="hmac(sha256)" /dev/disk /mnt/point
> 
> I tried to follow the example but the filesystem does not mount. But
> what almost shocked me was the way the key is specified on the userspace
> side.
> 
> $ mkfs.btrfs --csum "hmac(sha256)" --auth-key 0123456 /dev/disk
> 
> "0123456" are the raw bytes of the key? Seriously?
> 
> And how it's passed to the hmac code:
> 
>  gcry_mac_hd_t mac;
>  gcry_mac_open(&mac, GCRY_MAC_HMAC_SHA256, 0, NULL);
>  gcry_mac_setkey(mac, fs_info->auth_key, strlen(fs_info->auth_key));
>  gcry_mac_write(mac, buf, length);
>  gcry_mac_read(mac, out, &length);
> 
> Strlen means the key must avoid char 0 and I don't think we want do any
> decoding from ascii-hex format, when there's the whole keyctl
> infrastructure.
> 
> The key for all userspace commands needs to be specified the same way as
> for kernel, ie. "--auth-key btrfs:foo" and use the appropriate ioctls to
> read the key bytes.
> 

Hohum?

Here's what I just did:

rapido1:/# keyctl add logon btrfs:foo 0123456 @u
1020349071
rapido1:/# mkfs.btrfs --csum "hmac(sha256)" --auth-key 0123456 /dev/zram1
btrfs-progs v5.6
See http://btrfs.wiki.kernel.org for more information.

Detected a SSD, turning off metadata duplication.  Mkfs with -m dup if you want to force metadata duplication.
Label:              (null)
UUID:               56ae43ac-f333-4ed4-933a-356aed534115
[   31.005743] BTRFS: device fsid 56ae43ac-f333-4ed4-933a-356aed534115 devid 1 transid 5 /dev/zram1 scanned by mkfs.btrfs (241)

Sector size:        4096
Filesystem size:    3.00GiB
Block group profiles:
  Data:             single            8.00MiB
  Metadata:         single            8.00MiB
  System:           single            4.00MiB
SSD detected:       yes
Incompat features:  extref, skinny-metadata
Checksum:           hmac-sha256
Number of devices:  1
Devices:
   ID        SIZE  PATH
    1     3.00GiB  /dev/zram1


rapido1:/# mount -o auth_key=btrfs:foo,auth_hash_name="hmac(sha256)" /dev/zram1 /mnt/       
[   65.959465] BTRFS info (device (efault)): doing authentication
[   65.963204] BTRFS info (device zram1): disk space caching is enabled
[   65.964137] BTRFS info (device zram1): has skinny extents
[   65.964912] BTRFS info (device zram1): flagging fs with big metadata feature
[   65.968302] BTRFS info (device zram1): enabling ssd optimizations
[   65.969551] BTRFS info (device zram1): checking UUID tree
rapido1:/#

Re: [PATCH v3 2/3] btrfs: add authentication support

[Johannes Thumshirn]

On 27/05/2020 15:55, Johannes Thumshirn wrote:
> On 27/05/2020 15:25, David Sterba wrote:
>> On Thu, May 14, 2020 at 11:24:14AM +0200, Johannes Thumshirn wrote:
>>> From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>>> Example usage:
>>> Create a file-system with authentication key 0123456
>>> mkfs.btrfs --csum "hmac(sha256)" --auth-key 0123456 /dev/disk
>>>
>>> Add the key to the kernel's keyring as keyid 'btrfs:foo'
>>> keyctl add logon btrfs:foo 0123456 @u
>>>
>>> Mount the fs using the 'btrfs:foo' key
>>> mount -o auth_key=btrfs:foo,auth_hash_name="hmac(sha256)" /dev/disk /mnt/point
>>
>> I tried to follow the example but the filesystem does not mount. But
>> what almost shocked me was the way the key is specified on the userspace
>> side.

OK I think I know what happened. Did I forget to send a v2 of the progs side?
I changed the csum_type number to not have holes in the array, this obviously needs
to be done in progs as well.

Re: [PATCH v3 2/3] btrfs: add authentication support

[Johannes Thumshirn]

On 27/05/2020 15:25, David Sterba wrote:
> The key for all userspace commands needs to be specified the same way as
> for kernel, ie. "--auth-key btrfs:foo" and use the appropriate ioctls to
> read the key bytes.

Up to now I haven't been able to add a key to the kernel's keyring which 
can be read back to user-space.

How about passing in a key file, like it is done in UBIFS? Should be doable
both with libsodium and libgcrypt.

Re: [PATCH v3 2/3] btrfs: add authentication support

[David Sterba]

On Wed, May 27, 2020 at 06:04:57PM +0000, Johannes Thumshirn wrote:
> On 27/05/2020 15:25, David Sterba wrote:
> > The key for all userspace commands needs to be specified the same way as
> > for kernel, ie. "--auth-key btrfs:foo" and use the appropriate ioctls to
> > read the key bytes.
> 
> Up to now I haven't been able to add a key to the kernel's keyring which 
> can be read back to user-space.

This needs permissions on the key and I think keys from some keyrings
cannot be read back even with the permissions set. There's an ioctl
equivalent of 'keyctl read' which I used to emulate the reading. Setting
the permissions is cumbersome, as it needs to manually craft the hexa
value, but otherwise would seems a better way than either specifying the
key payload on command line or storing it in a file.

I've looked at other projects using keys, eg. ecryptfs-utils, it uses
keyctl_read_alloc, so that seems to be the preferred way.

> How about passing in a key file, like it is done in UBIFS? Should be doable
> both with libsodium and libgcrypt.

Keyfile would be better, that's what dm-crypt uses, but still.

Re: [PATCH v3 2/3] btrfs: add authentication support

[David Sterba]

On Wed, May 27, 2020 at 06:04:57PM +0000, Johannes Thumshirn wrote:
> On 27/05/2020 15:25, David Sterba wrote:
> > The key for all userspace commands needs to be specified the same way as
> > for kernel, ie. "--auth-key btrfs:foo" and use the appropriate ioctls to
> > read the key bytes.
> 
> Up to now I haven't been able to add a key to the kernel's keyring which 
> can be read back to user-space.

I was researching a possibility to use libkcapi, the API to use kernel
crypto implementaion, in order to avoid passing the raw key to userspace
completely. Basically, setting up what hash and key to use, pass the
buffer and get back the hash. API-wise it's just one more line to
specify the key -- by the numerical id. But no such interface is there,
only the raw bytes translating the request to the .setkey callback.

[PATCH v3 3/3] btrfs: document btrfs authentication

[Johannes Thumshirn]

From: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Document the design, guarantees and limitations of an authenticated BTRFS
file-system.

Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 .../filesystems/btrfs-authentication.rst      | 168 ++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100644 Documentation/filesystems/btrfs-authentication.rst

diff --git a/Documentation/filesystems/btrfs-authentication.rst b/Documentation/filesystems/btrfs-authentication.rst
new file mode 100644
index 000000000000..f13cab248fc0
--- /dev/null
+++ b/Documentation/filesystems/btrfs-authentication.rst
@@ -0,0 +1,168 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+:orphan:
+
+.. BTRFS Authentication
+.. Western Digital or it's affiliates
+.. 2020
+
+Introduction
+============
+
+This document describes an approach get file contents _and_ full meta-data
+authentication for BTRFS.
+
+This is possible because BTRFS uses checksums embedded in its on-disk
+meta-data structures as well as checksums for each individual extent of data.
+The primary intent of these checksums was to detect bit-flips of data at rest.
+But this mechanism can be extended to provide authentication of all on-disk
+data when the checksum algorithm is replaced by a cryptographically secure
+keyed hash.
+
+BTRFS Data Structures
+---------------------
+
+BTRFS utilizes a special copy-on-write b-tree to store all contents of the
+file-system on disk.
+
+Meta-Data
+~~~~~~~~~
+
+On-disk meta-data in BTRFS is stored in copy-on-write b-trees. These b-trees
+are build using two data structures, ``struct btrfs_node`` and ``struct
+btrfs_leaf``. Both of these structures start with a ``struct btrfs_header``.
+This structure has amongst other fields a checksum field, protecting it's
+contents. As the checksum is the first entry in the structure, the whole
+structure is protected by this checksum. The superblock (``struct
+btrfs_super_block``) is the first on-disk structure which is read on mount and
+it as well starts with a checksum field protecting the rest of the structure.
+The super block is also needed to read the addresses of the other file system
+b-trees, so their location on disk is protected by the checksum.
+
+::
+
+          BTRFS Header
+          +------+------+--------+-------+-----------------+----+-------+---------+
+          | csum | fsid | bytenr | flags | chunk_tree_uuid | gen| owner | nritems |
+          +------+------+--------+-------+-----------------+----+-------+---------+
+          BTRFS Node
+          +--------+-------------+-------------+-----+
+          | Header | key pointer | key pointer | ... |
+          +--------+-------------+-------------+-----+
+          BTRFS Leaf
+          +--------+------+------+-----+
+          | Header | item | item | ... |
+          +--------+------+------+-----+
+
+            Figure 1: BTRFS Header, Node and Leaf data structures
+
+User-Data
+~~~~~~~~~
+
+User data in BRTFS is also protected by checksums, but this checksum is not
+stored alongside the data, as it is with meta-data, but stored in a separate
+b-tree, the checksum tree. The leafs of this tree store the checksums of the
+user-data.  The tree nodes and leafs are of ``struct btrfs_node`` or ``struct
+btrfs_leaf``, so integrity of this tree is protected as well.
+
+BTRFS Authentication
+====================
+
+This chapter introduces BTRFS authentication which enables BTRFS to verify
+the authenticity and integrity of metadata and file contents stored on disk.
+
+Threat Model
+------------
+
+BTRFS authentication enables detection of offline data modification. While it
+does not prevent it, it enables (trusted) code to check the integrity and
+authenticity of on-disk file contents and filesystem metadata. This covers
+attacks where file contents are swapped.
+
+BTRFS authentication will not protect against a rollback of full disk
+contents. Ie. an attacker can still dump the disk and restore it at a later
+time without detection. It will also not protect against a rollback of one
+transaction. That means an attacker is able to partially undo changes. This is
+possible, because BTRFS does not immediately overwrite obsolete versions of
+its meta-data but keeps older generations until they get garbage collected.
+
+BTRFS authentication does not cover attacks where an attacker is able to
+execute code on the device after the authentication key was provided.
+Additional measures like secure boot and trusted boot have to be taken to
+ensure that only trusted code is executed on a device.
+
+As the file-system authentication key is also needed to update data structures
+on disk, the key has to be in the kernel's keyring for the whole time the
+file-system is mounted. An attacker that is able to compromise the kernel can
+be able to extract the key from the kernel's keyring and thus can gain the
+ability to modify the file-system later on.
+
+Authentication
+--------------
+
+To be able to fully trust data read from disk, all BTRFS data structures
+stored on disk are authenticated. That is:
+
+- The super blocks
+- The file-system b-trees
+- The user-data
+
+
+Super-block
+~~~~~~~~~~~
+
+In order to be able to authenticate the file-system's super-block, the
+checksum stored in the checksum field at the beginning of ``struct
+btrfs_super_block`` protecting its contents is replaced by a
+cryptographically secure keyed hash. In order to generate a valid super-block
+or to validate the super-block, one has to provide a key as an additional
+input for the hash function. The super-block is the starting point to read all
+on disk tree structures, so if we cannot trust the authenticity of the
+super-block anymore, we cannot trust the whole file-system.
+
+B-Trees
+~~~~~~~
+
+Starting from the super-block's root-tree root, the root tree holds the b-tree
+roots of all other on disk b-trees. All other file-system meta-data can be
+derived from the trees stored in this tree. As all b-trees in BTRFS are built
+using ``struct btrfs_node`` and ``struct btrfs_leaf`` each building block of
+each tree is checksummed. These checksums are replaced with the cryptographically
+secure keyed hash algorithm and the authentication key used to verify the
+super-block in the mount phase. Without this key it is impossible to alter any
+of the file-system structure without generating invalid hashes.
+
+User-data
+~~~~~~~~~
+
+The checksums for the user or file-data are stored in a separate b-tree, the
+checksum tree. As this tree in itself is authenticated, only the data stored
+in it needs to be authenticated. This is done by replacing the checksums
+stored on disk by the cryptographically secure keyed hash algorithm used for
+the super-block and other meta-data. So each written file block will get
+checksummed with the authentication key and without supplying the correct key
+it is impossible to write data on disk, which can be read back without
+failing the authentication test. If this test is failed, an I/O error is
+reported back to the user.
+
+Key Management
+--------------
+
+For simplicity, BTRFS authentication uses a single key to compute the keyed
+hashes of the super-block, b-tree nodes and leafs as well as file-blocks. This
+key has to be available on creation of the file-system (`mkfs.btrfs`) to
+authenticate all b-tree elements and the super-blocks. Further, it has to be
+available on mount of the file-system to verify the meta-data and user-data
+stored in the file-system.
+
+Limitations
+-----------
+
+As some optional features of BTRFS disable the generation of checksums, these
+features are incompatible with an authenticated BTRFS.
+These features are:
+- nodatacow
+- nodatasum
+
+As well as any offline modifications to the file-system, like setting an FS
+label while the FS is unmounted.
-- 
2.26.1

Re: [PATCH v3 3/3] btrfs: document btrfs authentication

[Jonathan Corbet]

On Thu, 14 May 2020 11:24:15 +0200
Johannes Thumshirn <jth@kernel.org> wrote:

Quick question...

> Document the design, guarantees and limitations of an authenticated BTRFS
> file-system.
> 
> Cc: Jonathan Corbet <corbet@lwn.net>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  .../filesystems/btrfs-authentication.rst      | 168 ++++++++++++++++++
>  1 file changed, 168 insertions(+)
>  create mode 100644 Documentation/filesystems/btrfs-authentication.rst
> 
> diff --git a/Documentation/filesystems/btrfs-authentication.rst b/Documentation/filesystems/btrfs-authentication.rst
> new file mode 100644
> index 000000000000..f13cab248fc0
> --- /dev/null
> +++ b/Documentation/filesystems/btrfs-authentication.rst
> @@ -0,0 +1,168 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +:orphan:
> +

Why mark this "orphan" rather than just adding it to index.rst so it gets
built with the rest of the docs?

Thanks,

jon

Re: [PATCH v3 3/3] btrfs: document btrfs authentication

[Johannes Thumshirn]

On 14/05/2020 14:26, Jonathan Corbet wrote:
> On Thu, 14 May 2020 11:24:15 +0200
> Johannes Thumshirn <jth@kernel.org> wrote:
> 
> Quick question...
> 
>> Document the design, guarantees and limitations of an authenticated BTRFS
>> file-system.
>>
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>> ---
>>  .../filesystems/btrfs-authentication.rst      | 168 ++++++++++++++++++
>>  1 file changed, 168 insertions(+)
>>  create mode 100644 Documentation/filesystems/btrfs-authentication.rst
>>
>> diff --git a/Documentation/filesystems/btrfs-authentication.rst b/Documentation/filesystems/btrfs-authentication.rst
>> new file mode 100644
>> index 000000000000..f13cab248fc0
>> --- /dev/null
>> +++ b/Documentation/filesystems/btrfs-authentication.rst
>> @@ -0,0 +1,168 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +:orphan:
>> +
> 
> Why mark this "orphan" rather than just adding it to index.rst so it gets
> built with the rest of the docs?
>
I've no idea of rst and the ubifs-authentication.rst which I had open at the
time did have this as well, so I blindly copied it. Thanks for spotting, will
remove in the next iteration.

Re: [PATCH v3 3/3] btrfs: document btrfs authentication

[Richard Weinberger]

----- Ursprüngliche Mail -----
>> Why mark this "orphan" rather than just adding it to index.rst so it gets
>> built with the rest of the docs?
>>
> I've no idea of rst and the ubifs-authentication.rst which I had open at the
> time did have this as well, so I blindly copied it. Thanks for spotting, will
> remove in the next iteration.

Well, the original ubifs-authentication documentation was written in in markdown
(which is IMHO muss less pain to write), later it was converted to rst by:

commit 09f4c750a8c7d1fc0b7bb3a7aa1de55de897a375
Author: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Date:   Fri Jul 26 09:51:14 2019 -0300

    docs: ubifs-authentication.md: convert to ReST
    
    The documentation standard is ReST and not markdown.
    
    Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
    Acked-by: Rob Herring <robh@kernel.org>
    Signed-off-by: Jonathan Corbet <corbet@lwn.net>

But I have no idea what this orphan thingy is.

Thanks,
//richard

Re: [PATCH v3 3/3] btrfs: document btrfs authentication

[Jonathan Corbet]

On Thu, 14 May 2020 17:14:36 +0200 (CEST)
Richard Weinberger <richard@nod.at> wrote:

> But I have no idea what this orphan thingy is.

It suppresses a warning from Sphinx that the file is not included in the
docs build.  Mauro did that with a lot of his conversions just to make his
life easier at the time, but it's not really something we want going
forward.

jon

Re: [PATCH v3 3/3] btrfs: document btrfs authentication

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Jonathan Corbet" <corbet@lwn.net>
> An: "richard" <richard@nod.at>
> CC: "Johannes Thumshirn" <Johannes.Thumshirn@wdc.com>, "Johannes Thumshirn" <jth@kernel.org>, "David Sterba"
> <dsterba@suse.cz>, "linux-fsdevel" <linux-fsdevel@vger.kernel.org>, "linux-btrfs" <linux-btrfs@vger.kernel.org>, "Eric
> Biggers" <ebiggers@google.com>
> Gesendet: Donnerstag, 14. Mai 2020 18:00:18
> Betreff: Re: [PATCH v3 3/3] btrfs: document btrfs authentication

> On Thu, 14 May 2020 17:14:36 +0200 (CEST)
> Richard Weinberger <richard@nod.at> wrote:
> 
>> But I have no idea what this orphan thingy is.
> 
> It suppresses a warning from Sphinx that the file is not included in the
> docs build.  Mauro did that with a lot of his conversions just to make his
> life easier at the time, but it's not really something we want going
> forward.

Ahh, thanks for explaining. :-)

Thanks,
//richard

Re: [PATCH v3 3/3] btrfs: document btrfs authentication

[David Sterba]

On Thu, May 14, 2020 at 11:24:15AM +0200, Johannes Thumshirn wrote:
> +User-data
> +~~~~~~~~~
> +
> +The checksums for the user or file-data are stored in a separate b-tree, the
> +checksum tree. As this tree in itself is authenticated, only the data stored
> +in it needs to be authenticated. This is done by replacing the checksums
> +stored on disk by the cryptographically secure keyed hash algorithm used for
> +the super-block and other meta-data. So each written file block will get
> +checksummed with the authentication key and without supplying the correct key
> +it is impossible to write data on disk, which can be read back without
> +failing the authentication test. If this test is failed, an I/O error is
> +reported back to the user.

With same key K and same contents of data block B, the keyed hash on two
different filesystems is the same. Ie. there's no per-filesystem salt
(like a UUID) or per-transaction salt (generation, block address).

For metadata the per-transaction salt is inherently there as the hash is
calculated with the header included (containing the increasing
generation) and the filesystem UUID (available via blkid) or chunk tree
UUID (not so easy to user to read).

So there's an obvious discrepancy in the additional data besides the
variable contents of the data and metadata blocks.

The weakness of the data blocks may aid some attacks (I don't have a
concrete suggestion where and how exatly).

Suggested fix is to have a data block "header", with similar contents as
the metadata blocks, eg.

struct btrfs_hash_header {
	u8 fsid[BTRFS_FSID_SIZE];
	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
	__le64 generation;
};

Perhaps also with some extra item for future extensions, set to zeros
for now.

Re: [PATCH v3 3/3] btrfs: document btrfs authentication

[Johannes Thumshirn]

On 24/05/2020 21:56, David Sterba wrote:
> On Thu, May 14, 2020 at 11:24:15AM +0200, Johannes Thumshirn wrote:
>> +User-data
>> +~~~~~~~~~
>> +
>> +The checksums for the user or file-data are stored in a separate b-tree, the
>> +checksum tree. As this tree in itself is authenticated, only the data stored
>> +in it needs to be authenticated. This is done by replacing the checksums
>> +stored on disk by the cryptographically secure keyed hash algorithm used for
>> +the super-block and other meta-data. So each written file block will get
>> +checksummed with the authentication key and without supplying the correct key
>> +it is impossible to write data on disk, which can be read back without
>> +failing the authentication test. If this test is failed, an I/O error is
>> +reported back to the user.
> 
> With same key K and same contents of data block B, the keyed hash on two
> different filesystems is the same. Ie. there's no per-filesystem salt
> (like a UUID) or per-transaction salt (generation, block address).

Correct.

> 
> For metadata the per-transaction salt is inherently there as the hash is
> calculated with the header included (containing the increasing
> generation) and the filesystem UUID (available via blkid) or chunk tree
> UUID (not so easy to user to read).
> 
> So there's an obvious discrepancy in the additional data besides the
> variable contents of the data and metadata blocks.
> 
> The weakness of the data blocks may aid some attacks (I don't have a
> concrete suggestion where and how exatly).

Yes but wouldn't this also need a hash that is prone to a known plaintext
attack or that has known collisions? But it would probably help in 
brute-forcing the key K of the filesystem. OTOH fsid, generation and the 
chunk-tree UUID can be read in plaintext from the FS as well so this would
only mitigate a rainbow table like attack, wouldn't it?

> 
> Suggested fix is to have a data block "header", with similar contents as
> the metadata blocks, eg.
> 
> struct btrfs_hash_header {
> 	u8 fsid[BTRFS_FSID_SIZE];
> 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
> 	__le64 generation;
> };
> 
> Perhaps also with some extra item for future extensions, set to zeros
> for now.
> 

This addition would be possible, yes. But if we'd add this header to every
checksum in the checksum tree it would be an incompatible on-disk format
change.

We could add this only for authenticated filesystems though, but would this
deviation make sense? I need to think more about it (and actually look at the
code to see how this could be done).

Re: [PATCH v3 3/3] btrfs: document btrfs authentication

[David Sterba]

On Mon, May 25, 2020 at 10:57:13AM +0000, Johannes Thumshirn wrote:
> On 24/05/2020 21:56, David Sterba wrote:
> > On Thu, May 14, 2020 at 11:24:15AM +0200, Johannes Thumshirn wrote:
> > For metadata the per-transaction salt is inherently there as the hash is
> > calculated with the header included (containing the increasing
> > generation) and the filesystem UUID (available via blkid) or chunk tree
> > UUID (not so easy to user to read).
> > 
> > So there's an obvious discrepancy in the additional data besides the
> > variable contents of the data and metadata blocks.
> > 
> > The weakness of the data blocks may aid some attacks (I don't have a
> > concrete suggestion where and how exatly).
> 
> Yes but wouldn't this also need a hash that is prone to a known plaintext
> attack or that has known collisions? But it would probably help in 
> brute-forcing the key K of the filesystem. OTOH fsid, generation and the 
> chunk-tree UUID can be read in plaintext from the FS as well so this would
> only mitigate a rainbow table like attack, wouldn't it?

The goal here is to make attacks harder at a small cost.

> > Suggested fix is to have a data block "header", with similar contents as
> > the metadata blocks, eg.
> > 
> > struct btrfs_hash_header {
> > 	u8 fsid[BTRFS_FSID_SIZE];
> > 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
> > 	__le64 generation;
> > };
> > 
> > Perhaps also with some extra item for future extensions, set to zeros
> > for now.
> 
> This addition would be possible, yes. But if we'd add this header to every
> checksum in the checksum tree it would be an incompatible on-disk format
> change.

No. It's only in-memory and is built from known pieces of information
exactly to avoid storing it on disk.

Re: [PATCH v3 3/3] btrfs: document btrfs authentication

[Johannes Thumshirn]

On 25/05/2020 13:27, David Sterba wrote:
>>> Suggested fix is to have a data block "header", with similar contents as
>>> the metadata blocks, eg.
>>>
>>> struct btrfs_hash_header {
>>> 	u8 fsid[BTRFS_FSID_SIZE];
>>> 	u8 chunk_tree_uuid[BTRFS_UUID_SIZE];
>>> 	__le64 generation;
>>> };
>>>
>>> Perhaps also with some extra item for future extensions, set to zeros
>>> for now.
>>
>> This addition would be possible, yes. But if we'd add this header to every
>> checksum in the checksum tree it would be an incompatible on-disk format
>> change.
> 
> No. It's only in-memory and is built from known pieces of information
> exactly to avoid storing it on disk.
> 

Ah OK, now I get what you meant. This should then be only for the authenticated 
FS I guess.

Re: [PATCH v3 0/3] Add file-system authentication to BTRFS

[David Sterba]

On Thu, May 14, 2020 at 11:24:12AM +0200, Johannes Thumshirn wrote:
> From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 
> This series adds file-system authentication to BTRFS. 
> 
> Unlike other verified file-system techniques like fs-verity the
> authenticated version of BTRFS does not need extra meta-data on disk.
> 
> This works because in BTRFS every on-disk block has a checksum, for meta-data
> the checksum is in the header of each meta-data item. For data blocks, a
> separate checksum tree exists, which holds the checksums for each block.
> 
> Currently BRTFS supports CRC32C, XXHASH64, SHA256 and Blake2b for checksumming
> these blocks. This series adds a new checksum algorithm, HMAC(SHA-256), which
> does need an authentication key. When no, or an incoreect authentication key
> is supplied no valid checksum can be generated and a read, fsck or scrub
> operation would detect invalid or tampered blocks once the file-system is
> mounted again with the correct key. 

As mentioned in the discussion under LWN article, https://lwn.net/Articles/818842/
ZFS implements split hash where one half is (partial) authenticated hash
and the other half is a checksum. This allows to have at least some sort
of verification when the auth key is not available. This applies to the
fixed size checksum area of metadata blocks, for data we can afford to
store both hashes in full.

I like this idea, however it brings interesting design decisions, "what
if" and corner cases:

- what hashes to use for the plain checksum, and thus what's the split
- what if one hash matches and the other not
- increased checksum calculation time due to doubled block read
- whether to store the same parital hash+checksum for data too

As the authenticated hash is the main usecase, I'd reserve most of the
32 byte buffer to it and use a weak hash for checksum: 24 bytes for HMAC
and 8 bytes for checksum. As an example: sha256+xxhash or
blake2b+xxhash.

I'd outright skip crc32c for the checksum so we have only small number
of authenticated checksums and avoid too many options, eg.
hmac-sha256-crc32c etc. The result will be still 2 authenticated hashes
with the added checksum hardcoded to xxhash.

Re: [PATCH v3 0/3] Add file-system authentication to BTRFS

[Johannes Thumshirn]

On 25/05/2020 15:11, David Sterba wrote:
> On Thu, May 14, 2020 at 11:24:12AM +0200, Johannes Thumshirn wrote:
>> From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>>
>> This series adds file-system authentication to BTRFS. 
>>
>> Unlike other verified file-system techniques like fs-verity the
>> authenticated version of BTRFS does not need extra meta-data on disk.
>>
>> This works because in BTRFS every on-disk block has a checksum, for meta-data
>> the checksum is in the header of each meta-data item. For data blocks, a
>> separate checksum tree exists, which holds the checksums for each block.
>>
>> Currently BRTFS supports CRC32C, XXHASH64, SHA256 and Blake2b for checksumming
>> these blocks. This series adds a new checksum algorithm, HMAC(SHA-256), which
>> does need an authentication key. When no, or an incoreect authentication key
>> is supplied no valid checksum can be generated and a read, fsck or scrub
>> operation would detect invalid or tampered blocks once the file-system is
>> mounted again with the correct key. 
> 
> As mentioned in the discussion under LWN article, https://lwn.net/Articles/818842/
> ZFS implements split hash where one half is (partial) authenticated hash
> and the other half is a checksum. This allows to have at least some sort
> of verification when the auth key is not available. This applies to the
> fixed size checksum area of metadata blocks, for data we can afford to
> store both hashes in full.
> 
> I like this idea, however it brings interesting design decisions, "what
> if" and corner cases:
> 
> - what hashes to use for the plain checksum, and thus what's the split
> - what if one hash matches and the other not
> - increased checksum calculation time due to doubled block read
> - whether to store the same parital hash+checksum for data too
> 
> As the authenticated hash is the main usecase, I'd reserve most of the
> 32 byte buffer to it and use a weak hash for checksum: 24 bytes for HMAC
> and 8 bytes for checksum. As an example: sha256+xxhash or
> blake2b+xxhash.
> 
> I'd outright skip crc32c for the checksum so we have only small number
> of authenticated checksums and avoid too many options, eg.
> hmac-sha256-crc32c etc. The result will be still 2 authenticated hashes
> with the added checksum hardcoded to xxhash.
> 

Hmm I'm really not a fan of this. We would have to use something like 
sha2-224 to get the room for the 2nd checksum. So we're using a weaker
hash just so we can add a second checksum. On the other hand you've asked 
me to add the known pieces of information into the hashes as a salt to
"make attacks harder at a small cost".

Re: [PATCH v3 0/3] Add file-system authentication to BTRFS

[David Sterba]

On Tue, May 26, 2020 at 07:50:53AM +0000, Johannes Thumshirn wrote:
> On 25/05/2020 15:11, David Sterba wrote:
> > On Thu, May 14, 2020 at 11:24:12AM +0200, Johannes Thumshirn wrote:
> > As mentioned in the discussion under LWN article, https://lwn.net/Articles/818842/
> > ZFS implements split hash where one half is (partial) authenticated hash
> > and the other half is a checksum. This allows to have at least some sort
> > of verification when the auth key is not available. This applies to the
> > fixed size checksum area of metadata blocks, for data we can afford to
> > store both hashes in full.
> > 
> > I like this idea, however it brings interesting design decisions, "what
> > if" and corner cases:
> > 
> > - what hashes to use for the plain checksum, and thus what's the split
> > - what if one hash matches and the other not
> > - increased checksum calculation time due to doubled block read
> > - whether to store the same parital hash+checksum for data too
> > 
> > As the authenticated hash is the main usecase, I'd reserve most of the
> > 32 byte buffer to it and use a weak hash for checksum: 24 bytes for HMAC
> > and 8 bytes for checksum. As an example: sha256+xxhash or
> > blake2b+xxhash.
> > 
> > I'd outright skip crc32c for the checksum so we have only small number
> > of authenticated checksums and avoid too many options, eg.
> > hmac-sha256-crc32c etc. The result will be still 2 authenticated hashes
> > with the added checksum hardcoded to xxhash.
> 
> Hmm I'm really not a fan of this. We would have to use something like 
> sha2-224 to get the room for the 2nd checksum. So we're using a weaker
> hash just so we can add a second checksum.

The idea is to calculate full hash (32 bytes) and store only the part
(24 bytes). Yes this means there's some information loss and weakening,
but enables a usecase.

> On the other hand you've asked 
> me to add the known pieces of information into the hashes as a salt to
> "make attacks harder at a small cost".

Yes and this makes it harder to attack the hash, it should be there
regardless of the additional checksums.

Re: [PATCH v3 0/3] Add file-system authentication to BTRFS

[Johannes Thumshirn]

On 26/05/2020 13:54, David Sterba wrote:
> On Tue, May 26, 2020 at 07:50:53AM +0000, Johannes Thumshirn wrote:
>> On 25/05/2020 15:11, David Sterba wrote:
>>> On Thu, May 14, 2020 at 11:24:12AM +0200, Johannes Thumshirn wrote:
>>> As mentioned in the discussion under LWN article, https://lwn.net/Articles/818842/
>>> ZFS implements split hash where one half is (partial) authenticated hash
>>> and the other half is a checksum. This allows to have at least some sort
>>> of verification when the auth key is not available. This applies to the
>>> fixed size checksum area of metadata blocks, for data we can afford to
>>> store both hashes in full.
>>>
>>> I like this idea, however it brings interesting design decisions, "what
>>> if" and corner cases:
>>>
>>> - what hashes to use for the plain checksum, and thus what's the split
>>> - what if one hash matches and the other not
>>> - increased checksum calculation time due to doubled block read
>>> - whether to store the same parital hash+checksum for data too
>>>
>>> As the authenticated hash is the main usecase, I'd reserve most of the
>>> 32 byte buffer to it and use a weak hash for checksum: 24 bytes for HMAC
>>> and 8 bytes for checksum. As an example: sha256+xxhash or
>>> blake2b+xxhash.
>>>
>>> I'd outright skip crc32c for the checksum so we have only small number
>>> of authenticated checksums and avoid too many options, eg.
>>> hmac-sha256-crc32c etc. The result will be still 2 authenticated hashes
>>> with the added checksum hardcoded to xxhash.
>>
>> Hmm I'm really not a fan of this. We would have to use something like 
>> sha2-224 to get the room for the 2nd checksum. So we're using a weaker
>> hash just so we can add a second checksum.
> 
> The idea is to calculate full hash (32 bytes) and store only the part
> (24 bytes). Yes this means there's some information loss and weakening,
> but enables a usecase.

I'm not enough a security expert to be able to judge this. Eric can I hear 
your opinion on this?

Thanks,
	Johannes

Re: [PATCH v3 0/3] Add file-system authentication to BTRFS

[David Sterba]

On Tue, May 26, 2020 at 12:44:28PM +0000, Johannes Thumshirn wrote:
> On 26/05/2020 13:54, David Sterba wrote:
> > On Tue, May 26, 2020 at 07:50:53AM +0000, Johannes Thumshirn wrote:
> >> On 25/05/2020 15:11, David Sterba wrote:
> >>> I'd outright skip crc32c for the checksum so we have only small number
> >>> of authenticated checksums and avoid too many options, eg.
> >>> hmac-sha256-crc32c etc. The result will be still 2 authenticated hashes
> >>> with the added checksum hardcoded to xxhash.
> >>
> >> Hmm I'm really not a fan of this. We would have to use something like 
> >> sha2-224 to get the room for the 2nd checksum. So we're using a weaker
> >> hash just so we can add a second checksum.
> > 
> > The idea is to calculate full hash (32 bytes) and store only the part
> > (24 bytes). Yes this means there's some information loss and weakening,
> > but enables a usecase.
> 
> I'm not enough a security expert to be able to judge this. Eric can I hear 
> your opinion on this?

Given that this has implications on strength and the usecases, I'd
rather let the filesystem provide the options and let the user choose
and not make the decision for their behalf.

This would increase number of authenticated hashes to 4 in the end:

1. authenticated with 32byte/256bit hash (sha256, blake2b)
   + full strength
   - no way to verify checksums without the key

2. authenticated with 24bytes/192bit hash (sha256, blake2b)
   where the last 8 bytes are xxhash64
   ~ weakened strength but should be still sufficient
   + possibility to verify checksums without the key
   - slight perf cost for the 2nd hash

As option 2 needs some evaluation and reasoning whether it does not
compromise the security, I don't insist on having it implemented in the
first phase. I have a prototype code for that so it might live in
linux-next for some time before we'd merge it.

Regarding backward compatibility, the checksums are easy compared to
other features. The supported status can be deteremined directly from
superblock so adding new types of checksum do not require compat bits
and the code for that.

Re: [PATCH v3 0/3] Add file-system authentication to BTRFS

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 4011 bytes --]



On 2020/5/14 下午5:24, Johannes Thumshirn wrote:
> From: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 
> This series adds file-system authentication to BTRFS. 
> 
> Unlike other verified file-system techniques like fs-verity the
> authenticated version of BTRFS does not need extra meta-data on disk.
> 
> This works because in BTRFS every on-disk block has a checksum, for meta-data
> the checksum is in the header of each meta-data item. For data blocks, a
> separate checksum tree exists, which holds the checksums for each block.
> 
> Currently BRTFS supports CRC32C, XXHASH64, SHA256 and Blake2b for checksumming
> these blocks. This series adds a new checksum algorithm, HMAC(SHA-256), which
> does need an authentication key. When no, or an incoreect authentication key
> is supplied no valid checksum can be generated and a read, fsck or scrub
> operation would detect invalid or tampered blocks once the file-system is
> mounted again with the correct key. 
> 
> Getting the key inside the kernel is out of scope of this implementation, the
> file-system driver assumes the key is already in the kernel's keyring at mount
> time.
> 
> There was interest in also using keyed Blake2b from the community, but this
> support is not yet included.
> 
> I have CCed Eric Biggers and Richard Weinberger in the submission, as they
> previously have worked on filesystem authentication and I hope we can get
> input from them as well.
> 
> Example usage:
> Create a file-system with authentication key 0123456
> mkfs.btrfs --csum "hmac(sha256)" --auth-key 0123456 /dev/disk
> 
> Add the key to the kernel's keyring as keyid 'btrfs:foo'
> keyctl add logon btrfs:foo 0123456 @u
> 
> Mount the fs using the 'btrfs:foo' key
> mount -t btrfs -o auth_key=btrfs:foo,auth_hash_name="hmac(sha256)" /dev/disk /mnt/point
> 
> Note, this is a re-base of the work I did when I was still at SUSE, hence the
> S-o-b being my SUSE address, while the Author being with my WDC address (to
> not generate bouncing mails).
> 
> Changes since v2:
> - Select CONFIG_CRYPTO_HMAC and CONFIG_KEYS (kbuild robot)
> - Fix double free in error path
> - Fix memory leak in error path
> - Disallow nodatasum and nodatacow when authetication is use (Eric)

Since we're disabling NODATACOW usages, can we also disable the
following features?
- v1 space cache
  V1 space cache uses NODATACOW file to store space cache, althouhg it
  has inline csum, but it's fixed to crc32c. So attacker can easily
  utilize this hole to mess space cache, and do some DoS attack.

- fallocate
  I'm not 100% sure about this, but since nodatacow is already a second
  class citizen in btrfs, maybe not supporting fallocate is not a
  strange move.

Thanks,
Qu

> - Pass in authentication algorithm as mount option (Eric)
> - Don't use the work "replay" in the documentation, as it is wrong and
>   harmful in this context (Eric)
> - Force key name to begin with 'btrfs:' (Eric)
> - Use '4' as on-disk checksum type for HMAC(SHA256) to not have holes in the
>   checksum types array.
> 
> Changes since v1:
> - None, only rebased the series
> 
> Johannes Thumshirn (3):
>   btrfs: rename btrfs_parse_device_options back to
>     btrfs_parse_early_options
>   btrfs: add authentication support
>   btrfs: document btrfs authentication
> 
>  .../filesystems/btrfs-authentication.rst      | 168 ++++++++++++++++++
>  fs/btrfs/Kconfig                              |   2 +
>  fs/btrfs/ctree.c                              |  22 ++-
>  fs/btrfs/ctree.h                              |   5 +-
>  fs/btrfs/disk-io.c                            |  71 +++++++-
>  fs/btrfs/ioctl.c                              |   7 +-
>  fs/btrfs/super.c                              |  65 ++++++-
>  include/uapi/linux/btrfs_tree.h               |   1 +
>  8 files changed, 326 insertions(+), 15 deletions(-)
>  create mode 100644 Documentation/filesystems/btrfs-authentication.rst
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 0/3] Add file-system authentication to BTRFS

[David Sterba]

On Wed, May 27, 2020 at 10:08:06AM +0800, Qu Wenruo wrote:
> > Changes since v2:
> > - Select CONFIG_CRYPTO_HMAC and CONFIG_KEYS (kbuild robot)
> > - Fix double free in error path
> > - Fix memory leak in error path
> > - Disallow nodatasum and nodatacow when authetication is use (Eric)
> 
> Since we're disabling NODATACOW usages, can we also disable the
> following features?
> - v1 space cache
>   V1 space cache uses NODATACOW file to store space cache, althouhg it
>   has inline csum, but it's fixed to crc32c. So attacker can easily
>   utilize this hole to mess space cache, and do some DoS attack.

That's a good point.

The v1 space cache will be phased out but it won't be in a timeframe
we'll get in the authentication. At this point we don't even have a way
to select v2 at mkfs time (it's work in progress though), so it would be
required to switch to v2 on the first mount.

> - fallocate
>   I'm not 100% sure about this, but since nodatacow is already a second
>   class citizen in btrfs, maybe not supporting fallocate is not a
>   strange move.

Fallocate is a standard file operation, not supporting would be quite
strange. What's the problem with fallocate and authentication?

Re: [PATCH v3 0/3] Add file-system authentication to BTRFS

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1728 bytes --]



On 2020/5/27 下午7:27, David Sterba wrote:
> On Wed, May 27, 2020 at 10:08:06AM +0800, Qu Wenruo wrote:
>>> Changes since v2:
>>> - Select CONFIG_CRYPTO_HMAC and CONFIG_KEYS (kbuild robot)
>>> - Fix double free in error path
>>> - Fix memory leak in error path
>>> - Disallow nodatasum and nodatacow when authetication is use (Eric)
>>
>> Since we're disabling NODATACOW usages, can we also disable the
>> following features?
>> - v1 space cache
>>   V1 space cache uses NODATACOW file to store space cache, althouhg it
>>   has inline csum, but it's fixed to crc32c. So attacker can easily
>>   utilize this hole to mess space cache, and do some DoS attack.
> 
> That's a good point.
> 
> The v1 space cache will be phased out but it won't be in a timeframe
> we'll get in the authentication. At this point we don't even have a way
> to select v2 at mkfs time (it's work in progress though), so it would be
> required to switch to v2 on the first mount.
> 
>> - fallocate
>>   I'm not 100% sure about this, but since nodatacow is already a second
>>   class citizen in btrfs, maybe not supporting fallocate is not a
>>   strange move.
> 
> Fallocate is a standard file operation, not supporting would be quite
> strange. What's the problem with fallocate and authentication?
> 
As said, I'm not that sure about preallocate, but that's the remaining
user of nodatacow.
Although it's a pretty common interface, but in btrfs it doesn't really
make much sense.
In case like fallocate then snapshot use case, there is really no
benefit from writing into fallocated range.

Not to mention the extra cross-ref check involved when writing into
possible preallocated range.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 0/3] Add file-system authentication to BTRFS

[David Sterba]

On Wed, May 27, 2020 at 10:08:06AM +0800, Qu Wenruo wrote:
> > Changes since v2:
> > - Select CONFIG_CRYPTO_HMAC and CONFIG_KEYS (kbuild robot)
> > - Fix double free in error path
> > - Fix memory leak in error path
> > - Disallow nodatasum and nodatacow when authetication is use (Eric)
> 
> Since we're disabling NODATACOW usages, can we also disable the
> following features?
> - v1 space cache
>   V1 space cache uses NODATACOW file to store space cache, althouhg it
>   has inline csum, but it's fixed to crc32c. So attacker can easily
>   utilize this hole to mess space cache, and do some DoS attack.
> 
> - fallocate
>   I'm not 100% sure about this, but since nodatacow is already a second
>   class citizen in btrfs, maybe not supporting fallocate is not a
>   strange move.

- swapfile
  NODATACOW is required for swapfile, so authentication and swapfile are
  mutualy exclusive.