From: Deven Bowers <[email protected]>
Allows author of IPE policy to indicate trust for a singular dm-verity
volume, identified by roothash, through "dmverity_roothash" and all
signed dm-verity volumes, through "dmverity_signature".
Signed-off-by: Deven Bowers <[email protected]>
---
Relevant changes since v6:
* Squash patch 08/12, 10/12 to [11/16]
---
security/ipe/eval.c | 5 ++
security/ipe/eval.h | 10 +++
security/ipe/hooks.c | 48 ++++++++++++++
security/ipe/hooks.h | 6 ++
security/ipe/ipe.c | 9 +++
security/ipe/ipe.h | 3 +
security/ipe/modules/Kconfig | 23 +++++++
security/ipe/modules/Makefile | 2 +
security/ipe/modules/dmverity_roothash.c | 80 +++++++++++++++++++++++
security/ipe/modules/dmverity_signature.c | 25 +++++++
10 files changed, 211 insertions(+)
create mode 100644 security/ipe/modules/dmverity_roothash.c
create mode 100644 security/ipe/modules/dmverity_signature.c
diff --git a/security/ipe/eval.c b/security/ipe/eval.c
index 361efccebad4..facc05c753f4 100644
--- a/security/ipe/eval.c
+++ b/security/ipe/eval.c
@@ -23,6 +23,7 @@ static struct super_block *pinned_sb;
static DEFINE_SPINLOCK(pin_lock);
#define FILE_SUPERBLOCK(f) ((f)->f_path.mnt->mnt_sb)
+#define FILE_BLOCK_DEV(f) (FILE_SUPERBLOCK(f)->s_bdev)
/**
* pin_sb: pin the underlying superblock of @f, marking it as trusted
@@ -95,6 +96,10 @@ static struct ipe_eval_ctx *build_ctx(const struct file *file,
ctx->hook = hook;
ctx->ci_ctx = ipe_current_ctx();
ctx->from_init_sb = from_pinned(file);
+ if (file) {
+ if (FILE_BLOCK_DEV(file))
+ ctx->ipe_bdev = ipe_bdev(FILE_BLOCK_DEV(file));
+ }
return ctx;
}
diff --git a/security/ipe/eval.h b/security/ipe/eval.h
index 42fb7fdf2599..25d2d8d55702 100644
--- a/security/ipe/eval.h
+++ b/security/ipe/eval.h
@@ -13,6 +13,14 @@
#include "hooks.h"
#include "policy.h"
+struct ipe_bdev {
+ const u8 *sigdata;
+ size_t siglen;
+
+ const u8 *hash;
+ size_t hashlen;
+};
+
struct ipe_eval_ctx {
enum ipe_hook hook;
enum ipe_operation op;
@@ -20,6 +28,8 @@ struct ipe_eval_ctx {
const struct file *file;
struct ipe_context *ci_ctx;
+ const struct ipe_bdev *ipe_bdev;
+
bool from_init_sb;
};
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index 2d4a4f0eead0..470fb48e490c 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -13,6 +13,7 @@
#include <linux/types.h>
#include <linux/refcount.h>
#include <linux/rcupdate.h>
+#include <linux/blk_types.h>
#include <linux/binfmts.h>
#include <linux/mman.h>
@@ -219,3 +220,50 @@ void ipe_sb_free_security(struct super_block *mnt_sb)
{
ipe_invalidate_pinned_sb(mnt_sb);
}
+
+/**
+ * ipe_bdev_free_security: free nested structures within IPE's LSM blob
+ * in block_devices
+ * @bdev: Supplies a pointer to a block_device that contains the structure
+ * to free.
+ */
+void ipe_bdev_free_security(struct block_device *bdev)
+{
+ struct ipe_bdev *blob = ipe_bdev(bdev);
+
+ kfree(blob->sigdata);
+}
+
+/**
+ * ipe_bdev_setsecurity: associate some data from the block device layer
+ * with IPE's LSM blob.
+ * @bdev: Supplies a pointer to a block_device that contains the LSM blob.
+ * @key: Supplies the string key that uniquely identifies the value.
+ * @value: Supplies the value to store.
+ * @len: The length of @value.
+ */
+int ipe_bdev_setsecurity(struct block_device *bdev, const char *key,
+ const void *value, size_t len)
+{
+ struct ipe_bdev *blob = ipe_bdev(bdev);
+
+ if (!strcmp(key, DM_VERITY_SIGNATURE_SEC_NAME)) {
+ blob->siglen = len;
+ blob->sigdata = kmemdup(value, len, GFP_KERNEL);
+ if (!blob->sigdata)
+ return -ENOMEM;
+
+ return 0;
+ }
+
+ if (!strcmp(key, DM_VERITY_ROOTHASH_SEC_NAME)) {
+ blob->hashlen = len;
+ blob->hash = kmemdup(value, len, GFP_KERNEL);
+ if (!blob->hash)
+ return -ENOMEM;
+
+ return 0;
+ }
+
+ return -ENOSYS;
+}
diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
index e7f107ab5620..285f35187188 100644
--- a/security/ipe/hooks.h
+++ b/security/ipe/hooks.h
@@ -10,6 +10,7 @@
#include <linux/sched.h>
#include <linux/binfmts.h>
#include <linux/security.h>
+#include <linux/device-mapper.h>
enum ipe_hook {
ipe_hook_exec = 0,
@@ -40,4 +41,9 @@ int ipe_on_kernel_load_data(enum kernel_load_data_id id, bool contents);
void ipe_sb_free_security(struct super_block *mnt_sb);
+void ipe_bdev_free_security(struct block_device *bdev);
+
+int ipe_bdev_setsecurity(struct block_device *bdev, const char *key,
+ const void *value, size_t len);
+
#endif /* IPE_HOOKS_H */
diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
index 1382d50078ec..215936cb4574 100644
--- a/security/ipe/ipe.c
+++ b/security/ipe/ipe.c
@@ -9,6 +9,7 @@
#include "ipe_parser.h"
#include "modules/ipe_module.h"
#include "modules.h"
+#include "eval.h"
#include <linux/fs.h>
#include <linux/sched.h>
@@ -20,8 +21,14 @@
struct lsm_blob_sizes ipe_blobs __lsm_ro_after_init = {
.lbs_task = sizeof(struct ipe_context __rcu *),
+ .lbs_bdev = sizeof(struct ipe_bdev),
};
+struct ipe_bdev *ipe_bdev(struct block_device *b)
+{
+ return b->security + ipe_blobs.lbs_bdev;
+}
+
static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(task_alloc, ipe_task_alloc),
LSM_HOOK_INIT(task_free, ipe_task_free),
@@ -31,6 +38,8 @@ static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
LSM_HOOK_INIT(kernel_read_file, ipe_on_kernel_read),
LSM_HOOK_INIT(kernel_load_data, ipe_on_kernel_load_data),
LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
+ LSM_HOOK_INIT(bdev_free_security, ipe_bdev_free_security),
+ LSM_HOOK_INIT(bdev_setsecurity, ipe_bdev_setsecurity),
};
/**
diff --git a/security/ipe/ipe.h b/security/ipe/ipe.h
index ad16d2bebfec..6b4c7e07f204 100644
--- a/security/ipe/ipe.h
+++ b/security/ipe/ipe.h
@@ -14,10 +14,13 @@
#include <linux/types.h>
#include <linux/sched.h>
+#include <linux/blk_types.h>
#include <linux/lsm_hooks.h>
extern struct lsm_blob_sizes ipe_blobs;
extern struct ipe_parser __start_ipe_parsers[], __end_ipe_parsers[];
extern struct ipe_module __start_ipe_modules[], __end_ipe_modules[];
+struct ipe_bdev *ipe_bdev(struct block_device *b);
+
#endif /* IPE_H */
diff --git a/security/ipe/modules/Kconfig b/security/ipe/modules/Kconfig
index fad96ba534e2..a6ea06cf0737 100644
--- a/security/ipe/modules/Kconfig
+++ b/security/ipe/modules/Kconfig
@@ -16,5 +16,28 @@ config IPE_PROP_BOOT_VERIFIED
If unsure, answer N.
+config IPE_PROP_DM_VERITY_SIGNATURE
+ bool "Enable support for signed dm-verity volumes"
+ depends on DM_VERITY_VERIFY_ROOTHASH_SIG
+ default Y
+ help
+ This option enables the property 'dmverity_signature' in
+ IPE policy. This property evaluates to TRUE when a file
+ is evaluated against a dm-verity volume that was mounted
+ with a signed root-hash.
+
+ If unsure, answer Y.
+
+config IPE_PROP_DM_VERITY_ROOTHASH
+ bool "Enable support for dm-verity volumes"
+ depends on DM_VERITY
+ default Y
+ help
+ This option enables the property 'dmverity_roothash' in
+ IPE policy. This property evaluates to TRUE when a file
+ is evaluated against a dm-verity volume whose root hash
+ matches the supplied value.
+
+ If unsure, answer Y.
endmenu
diff --git a/security/ipe/modules/Makefile b/security/ipe/modules/Makefile
index e0045ec65434..84fadce85193 100644
--- a/security/ipe/modules/Makefile
+++ b/security/ipe/modules/Makefile
@@ -6,3 +6,5 @@
#
obj-$(CONFIG_IPE_PROP_BOOT_VERIFIED) += boot_verified.o
+obj-$(CONFIG_IPE_PROP_DM_VERITY_SIGNATURE) += dmverity_signature.o
+obj-$(CONFIG_IPE_PROP_DM_VERITY_ROOTHASH) += dmverity_roothash.o
diff --git a/security/ipe/modules/dmverity_roothash.c b/security/ipe/modules/dmverity_roothash.c
new file mode 100644
index 000000000000..0f82bec3b842
--- /dev/null
+++ b/security/ipe/modules/dmverity_roothash.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include "ipe_module.h"
+
+#include <linux/fs.h>
+#include <linux/types.h>
+
+struct counted_array {
+ size_t len;
+ u8 *data;
+};
+
+static int dvrh_parse(const char *valstr, void **value)
+{
+ int rv = 0;
+ struct counted_array *arr;
+
+ arr = kzalloc(sizeof(*arr), GFP_KERNEL);
+ if (!arr) {
+ rv = -ENOMEM;
+ goto err;
+ }
+
+ arr->len = (strlen(valstr) / 2);
+
+ arr->data = kzalloc(arr->len, GFP_KERNEL);
+ if (!arr->data) {
+ rv = -ENOMEM;
+ goto err;
+ }
+
+ rv = hex2bin(arr->data, valstr, arr->len);
+ if (rv != 0)
+ goto err2;
+
+ *value = arr;
+ return rv;
+err2:
+ kfree(arr->data);
+err:
+ kfree(arr);
+ return rv;
+}
+
+static bool dvrh_eval(const struct ipe_eval_ctx *ctx, const void *val)
+{
+ const u8 *src;
+ struct counted_array *expect = (struct counted_array *)val;
+
+ if (!ctx->ipe_bdev)
+ return false;
+
+ if (ctx->ipe_bdev->hashlen != expect->len)
+ return false;
+
+ src = ctx->ipe_bdev->hash;
+
+ return !memcmp(expect->data, src, expect->len);
+}
+
+static int dvrh_free(void **val)
+{
+ struct counted_array *expect = (struct counted_array *)val;
+
+ kfree(expect->data);
+ kfree(expect);
+
+ return 0;
+}
+
+IPE_MODULE(dvrh) = {
+ .name = "dmverity_roothash",
+ .version = 1,
+ .parse = dvrh_parse,
+ .free = dvrh_free,
+ .eval = dvrh_eval,
+};
diff --git a/security/ipe/modules/dmverity_signature.c b/security/ipe/modules/dmverity_signature.c
new file mode 100644
index 000000000000..08746fcbcb3e
--- /dev/null
+++ b/security/ipe/modules/dmverity_signature.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include "ipe_module.h"
+
+#include <linux/fs.h>
+#include <linux/types.h>
+
+static bool dvv_eval(const struct ipe_eval_ctx *ctx, const void *val)
+{
+ bool expect = (bool)val;
+ bool eval = ctx->ipe_bdev && (!!ctx->ipe_bdev->sigdata);
+
+ return expect == eval;
+}
+
+IPE_MODULE(dvv) = {
+ .name = "dmverity_signature",
+ .version = 1,
+ .parse = ipe_bool_parse,
+ .free = NULL,
+ .eval = dvv_eval,
+};
--
2.33.0
> From: [email protected]
> [mailto:[email protected]]
> Sent: Wednesday, October 13, 2021 9:07 PM
> From: Deven Bowers <[email protected]>
>
> Allows author of IPE policy to indicate trust for a singular dm-verity
> volume, identified by roothash, through "dmverity_roothash" and all
> signed dm-verity volumes, through "dmverity_signature".
>
> Signed-off-by: Deven Bowers <[email protected]>
> ---
>
> Relevant changes since v6:
> * Squash patch 08/12, 10/12 to [11/16]
>
> ---
> security/ipe/eval.c | 5 ++
> security/ipe/eval.h | 10 +++
> security/ipe/hooks.c | 48 ++++++++++++++
> security/ipe/hooks.h | 6 ++
> security/ipe/ipe.c | 9 +++
> security/ipe/ipe.h | 3 +
> security/ipe/modules/Kconfig | 23 +++++++
> security/ipe/modules/Makefile | 2 +
> security/ipe/modules/dmverity_roothash.c | 80 +++++++++++++++++++++++
> security/ipe/modules/dmverity_signature.c | 25 +++++++
> 10 files changed, 211 insertions(+)
> create mode 100644 security/ipe/modules/dmverity_roothash.c
> create mode 100644 security/ipe/modules/dmverity_signature.c
>
> diff --git a/security/ipe/eval.c b/security/ipe/eval.c
> index 361efccebad4..facc05c753f4 100644
> --- a/security/ipe/eval.c
> +++ b/security/ipe/eval.c
> @@ -23,6 +23,7 @@ static struct super_block *pinned_sb;
> static DEFINE_SPINLOCK(pin_lock);
>
> #define FILE_SUPERBLOCK(f) ((f)->f_path.mnt->mnt_sb)
> +#define FILE_BLOCK_DEV(f) (FILE_SUPERBLOCK(f)->s_bdev)
>
> /**
> * pin_sb: pin the underlying superblock of @f, marking it as trusted
> @@ -95,6 +96,10 @@ static struct ipe_eval_ctx *build_ctx(const struct file *file,
> ctx->hook = hook;
> ctx->ci_ctx = ipe_current_ctx();
> ctx->from_init_sb = from_pinned(file);
> + if (file) {
> + if (FILE_BLOCK_DEV(file))
> + ctx->ipe_bdev = ipe_bdev(FILE_BLOCK_DEV(file));
> + }
>
> return ctx;
> }
> diff --git a/security/ipe/eval.h b/security/ipe/eval.h
> index 42fb7fdf2599..25d2d8d55702 100644
> --- a/security/ipe/eval.h
> +++ b/security/ipe/eval.h
> @@ -13,6 +13,14 @@
> #include "hooks.h"
> #include "policy.h"
>
> +struct ipe_bdev {
> + const u8 *sigdata;
> + size_t siglen;
> +
> + const u8 *hash;
> + size_t hashlen;
> +};
> +
> struct ipe_eval_ctx {
> enum ipe_hook hook;
> enum ipe_operation op;
> @@ -20,6 +28,8 @@ struct ipe_eval_ctx {
> const struct file *file;
> struct ipe_context *ci_ctx;
>
> + const struct ipe_bdev *ipe_bdev;
> +
> bool from_init_sb;
> };
>
> diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> index 2d4a4f0eead0..470fb48e490c 100644
> --- a/security/ipe/hooks.c
> +++ b/security/ipe/hooks.c
> @@ -13,6 +13,7 @@
> #include <linux/types.h>
> #include <linux/refcount.h>
> #include <linux/rcupdate.h>
> +#include <linux/blk_types.h>
> #include <linux/binfmts.h>
> #include <linux/mman.h>
>
> @@ -219,3 +220,50 @@ void ipe_sb_free_security(struct super_block *mnt_sb)
> {
> ipe_invalidate_pinned_sb(mnt_sb);
> }
> +
> +/**
> + * ipe_bdev_free_security: free nested structures within IPE's LSM blob
> + * in block_devices
> + * @bdev: Supplies a pointer to a block_device that contains the structure
> + * to free.
> + */
> +void ipe_bdev_free_security(struct block_device *bdev)
> +{
> + struct ipe_bdev *blob = ipe_bdev(bdev);
> +
> + kfree(blob->sigdata);
> +}
> +
> +/**
> + * ipe_bdev_setsecurity: associate some data from the block device layer
> + * with IPE's LSM blob.
> + * @bdev: Supplies a pointer to a block_device that contains the LSM blob.
> + * @key: Supplies the string key that uniquely identifies the value.
> + * @value: Supplies the value to store.
> + * @len: The length of @value.
> + */
> +int ipe_bdev_setsecurity(struct block_device *bdev, const char *key,
> + const void *value, size_t len)
> +{
> + struct ipe_bdev *blob = ipe_bdev(bdev);
> +
> + if (!strcmp(key, DM_VERITY_SIGNATURE_SEC_NAME)) {
> + blob->siglen = len;
> + blob->sigdata = kmemdup(value, len, GFP_KERNEL);
> + if (!blob->sigdata)
> + return -ENOMEM;
> +
> + return 0;
> + }
> +
> + if (!strcmp(key, DM_VERITY_ROOTHASH_SEC_NAME)) {
> + blob->hashlen = len;
> + blob->hash = kmemdup(value, len, GFP_KERNEL);
> + if (!blob->hash)
> + return -ENOMEM;
> +
> + return 0;
> + }
> +
> + return -ENOSYS;
> +}
> diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
> index e7f107ab5620..285f35187188 100644
> --- a/security/ipe/hooks.h
> +++ b/security/ipe/hooks.h
> @@ -10,6 +10,7 @@
> #include <linux/sched.h>
> #include <linux/binfmts.h>
> #include <linux/security.h>
> +#include <linux/device-mapper.h>
>
> enum ipe_hook {
> ipe_hook_exec = 0,
> @@ -40,4 +41,9 @@ int ipe_on_kernel_load_data(enum kernel_load_data_id
> id, bool contents);
>
> void ipe_sb_free_security(struct super_block *mnt_sb);
>
> +void ipe_bdev_free_security(struct block_device *bdev);
> +
> +int ipe_bdev_setsecurity(struct block_device *bdev, const char *key,
> + const void *value, size_t len);
> +
> #endif /* IPE_HOOKS_H */
> diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> index 1382d50078ec..215936cb4574 100644
> --- a/security/ipe/ipe.c
> +++ b/security/ipe/ipe.c
> @@ -9,6 +9,7 @@
> #include "ipe_parser.h"
> #include "modules/ipe_module.h"
> #include "modules.h"
> +#include "eval.h"
>
> #include <linux/fs.h>
> #include <linux/sched.h>
> @@ -20,8 +21,14 @@
>
> struct lsm_blob_sizes ipe_blobs __lsm_ro_after_init = {
> .lbs_task = sizeof(struct ipe_context __rcu *),
> + .lbs_bdev = sizeof(struct ipe_bdev),
> };
>
> +struct ipe_bdev *ipe_bdev(struct block_device *b)
> +{
> + return b->security + ipe_blobs.lbs_bdev;
> +}
> +
> static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
> LSM_HOOK_INIT(task_alloc, ipe_task_alloc),
> LSM_HOOK_INIT(task_free, ipe_task_free),
> @@ -31,6 +38,8 @@ static struct security_hook_list ipe_hooks[]
> __lsm_ro_after_init = {
> LSM_HOOK_INIT(kernel_read_file, ipe_on_kernel_read),
> LSM_HOOK_INIT(kernel_load_data, ipe_on_kernel_load_data),
> LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
> + LSM_HOOK_INIT(bdev_free_security, ipe_bdev_free_security),
> + LSM_HOOK_INIT(bdev_setsecurity, ipe_bdev_setsecurity),
> };
>
> /**
> diff --git a/security/ipe/ipe.h b/security/ipe/ipe.h
> index ad16d2bebfec..6b4c7e07f204 100644
> --- a/security/ipe/ipe.h
> +++ b/security/ipe/ipe.h
> @@ -14,10 +14,13 @@
>
> #include <linux/types.h>
> #include <linux/sched.h>
> +#include <linux/blk_types.h>
> #include <linux/lsm_hooks.h>
>
> extern struct lsm_blob_sizes ipe_blobs;
> extern struct ipe_parser __start_ipe_parsers[], __end_ipe_parsers[];
> extern struct ipe_module __start_ipe_modules[], __end_ipe_modules[];
>
> +struct ipe_bdev *ipe_bdev(struct block_device *b);
> +
> #endif /* IPE_H */
> diff --git a/security/ipe/modules/Kconfig b/security/ipe/modules/Kconfig
> index fad96ba534e2..a6ea06cf0737 100644
> --- a/security/ipe/modules/Kconfig
> +++ b/security/ipe/modules/Kconfig
> @@ -16,5 +16,28 @@ config IPE_PROP_BOOT_VERIFIED
>
> If unsure, answer N.
>
> +config IPE_PROP_DM_VERITY_SIGNATURE
> + bool "Enable support for signed dm-verity volumes"
> + depends on DM_VERITY_VERIFY_ROOTHASH_SIG
> + default Y
> + help
> + This option enables the property 'dmverity_signature' in
> + IPE policy. This property evaluates to TRUE when a file
> + is evaluated against a dm-verity volume that was mounted
> + with a signed root-hash.
> +
> + If unsure, answer Y.
> +
> +config IPE_PROP_DM_VERITY_ROOTHASH
> + bool "Enable support for dm-verity volumes"
> + depends on DM_VERITY
> + default Y
> + help
> + This option enables the property 'dmverity_roothash' in
> + IPE policy. This property evaluates to TRUE when a file
> + is evaluated against a dm-verity volume whose root hash
> + matches the supplied value.
> +
> + If unsure, answer Y.
>
> endmenu
> diff --git a/security/ipe/modules/Makefile b/security/ipe/modules/Makefile
> index e0045ec65434..84fadce85193 100644
> --- a/security/ipe/modules/Makefile
> +++ b/security/ipe/modules/Makefile
> @@ -6,3 +6,5 @@
> #
>
> obj-$(CONFIG_IPE_PROP_BOOT_VERIFIED) += boot_verified.o
> +obj-$(CONFIG_IPE_PROP_DM_VERITY_SIGNATURE) += dmverity_signature.o
> +obj-$(CONFIG_IPE_PROP_DM_VERITY_ROOTHASH) += dmverity_roothash.o
> diff --git a/security/ipe/modules/dmverity_roothash.c
> b/security/ipe/modules/dmverity_roothash.c
> new file mode 100644
> index 000000000000..0f82bec3b842
> --- /dev/null
> +++ b/security/ipe/modules/dmverity_roothash.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation. All rights reserved.
> + */
> +
> +#include "ipe_module.h"
> +
> +#include <linux/fs.h>
> +#include <linux/types.h>
> +
> +struct counted_array {
> + size_t len;
> + u8 *data;
> +};
> +
> +static int dvrh_parse(const char *valstr, void **value)
> +{
> + int rv = 0;
> + struct counted_array *arr;
> +
> + arr = kzalloc(sizeof(*arr), GFP_KERNEL);
> + if (!arr) {
> + rv = -ENOMEM;
> + goto err;
> + }
> +
> + arr->len = (strlen(valstr) / 2);
> +
> + arr->data = kzalloc(arr->len, GFP_KERNEL);
> + if (!arr->data) {
> + rv = -ENOMEM;
> + goto err;
> + }
> +
> + rv = hex2bin(arr->data, valstr, arr->len);
> + if (rv != 0)
> + goto err2;
> +
> + *value = arr;
> + return rv;
> +err2:
> + kfree(arr->data);
> +err:
> + kfree(arr);
> + return rv;
> +}
> +
> +static bool dvrh_eval(const struct ipe_eval_ctx *ctx, const void *val)
> +{
> + const u8 *src;
> + struct counted_array *expect = (struct counted_array *)val;
> +
> + if (!ctx->ipe_bdev)
> + return false;
> +
> + if (ctx->ipe_bdev->hashlen != expect->len)
> + return false;
> +
> + src = ctx->ipe_bdev->hash;
> +
> + return !memcmp(expect->data, src, expect->len);
Hi Deven
I was curious to see if determining the property at run-time
could apply also to dm-verity. It seems it could be done
(I omit some checks, I also keep the expected value in hex
format):
---
md = dm_get_md(file_inode(ctx->file)->i_sb->s_dev);
table = dm_get_live_table(md, &srcu_idx);
num_targets = dm_table_get_num_targets(table);
for (i = 0; i < num_targets; i++) {
struct dm_target *ti = dm_table_get_target(table, i);
if (strcmp(ti->type->name, "verity"))
continue;
ti->type->status(ti, STATUSTYPE_IMA, 0, result, sizeof(result));
}
dm_put_live_table(md, srcu_idx);
dm_put(md);
root_digest_ptr = strstr(result, "root_digest=");
return !strncmp(expect->data, root_digest_ptr + 12, expect->len);
---
Only dm_table_get_target() is not exported yet, but I guess it could
be. dm_table_get_num_targets() is exported.
With this code, you would not have to manage security blobs
outside IPE. Maybe you could add a blob for the super block, so
that you verify the dm-verity property just once per filesystem.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua
> +}
> +
> +static int dvrh_free(void **val)
> +{
> + struct counted_array *expect = (struct counted_array *)val;
> +
> + kfree(expect->data);
> + kfree(expect);
> +
> + return 0;
> +}
> +
> +IPE_MODULE(dvrh) = {
> + .name = "dmverity_roothash",
> + .version = 1,
> + .parse = dvrh_parse,
> + .free = dvrh_free,
> + .eval = dvrh_eval,
> +};
> diff --git a/security/ipe/modules/dmverity_signature.c
> b/security/ipe/modules/dmverity_signature.c
> new file mode 100644
> index 000000000000..08746fcbcb3e
> --- /dev/null
> +++ b/security/ipe/modules/dmverity_signature.c
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation. All rights reserved.
> + */
> +
> +#include "ipe_module.h"
> +
> +#include <linux/fs.h>
> +#include <linux/types.h>
> +
> +static bool dvv_eval(const struct ipe_eval_ctx *ctx, const void *val)
> +{
> + bool expect = (bool)val;
> + bool eval = ctx->ipe_bdev && (!!ctx->ipe_bdev->sigdata);
> +
> + return expect == eval;
> +}
> +
> +IPE_MODULE(dvv) = {
> + .name = "dmverity_signature",
> + .version = 1,
> + .parse = ipe_bool_parse,
> + .free = NULL,
> + .eval = dvv_eval,
> +};
> --
> 2.33.0
On 11/25/2021 1:37 AM, Roberto Sassu wrote:
>> From: [email protected]
>> [mailto:[email protected]]
>> Sent: Wednesday, October 13, 2021 9:07 PM
>> From: Deven Bowers <[email protected]>
..snip
>> diff --git a/security/ipe/modules/Makefile b/security/ipe/modules/Makefile
>> index e0045ec65434..84fadce85193 100644
>> --- a/security/ipe/modules/Makefile
>> +++ b/security/ipe/modules/Makefile
>> @@ -6,3 +6,5 @@
>> #
>>
>> obj-$(CONFIG_IPE_PROP_BOOT_VERIFIED) += boot_verified.o
>> +obj-$(CONFIG_IPE_PROP_DM_VERITY_SIGNATURE) += dmverity_signature.o
>> +obj-$(CONFIG_IPE_PROP_DM_VERITY_ROOTHASH) += dmverity_roothash.o
>> diff --git a/security/ipe/modules/dmverity_roothash.c
>> b/security/ipe/modules/dmverity_roothash.c
>> new file mode 100644
>> index 000000000000..0f82bec3b842
>> --- /dev/null
>> +++ b/security/ipe/modules/dmverity_roothash.c
>> @@ -0,0 +1,80 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) Microsoft Corporation. All rights reserved.
>> + */
>> +
>> +#include "ipe_module.h"
>> +
>> +#include <linux/fs.h>
>> +#include <linux/types.h>
>> +
>> +struct counted_array {
>> + size_t len;
>> + u8 *data;
>> +};
>> +
>> +static int dvrh_parse(const char *valstr, void **value)
>> +{
>> + int rv = 0;
>> + struct counted_array *arr;
>> +
>> + arr = kzalloc(sizeof(*arr), GFP_KERNEL);
>> + if (!arr) {
>> + rv = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + arr->len = (strlen(valstr) / 2);
>> +
>> + arr->data = kzalloc(arr->len, GFP_KERNEL);
>> + if (!arr->data) {
>> + rv = -ENOMEM;
>> + goto err;
>> + }
>> +
>> + rv = hex2bin(arr->data, valstr, arr->len);
>> + if (rv != 0)
>> + goto err2;
>> +
>> + *value = arr;
>> + return rv;
>> +err2:
>> + kfree(arr->data);
>> +err:
>> + kfree(arr);
>> + return rv;
>> +}
>> +
>> +static bool dvrh_eval(const struct ipe_eval_ctx *ctx, const void *val)
>> +{
>> + const u8 *src;
>> + struct counted_array *expect = (struct counted_array *)val;
>> +
>> + if (!ctx->ipe_bdev)
>> + return false;
>> +
>> + if (ctx->ipe_bdev->hashlen != expect->len)
>> + return false;
>> +
>> + src = ctx->ipe_bdev->hash;
>> +
>> + return !memcmp(expect->data, src, expect->len);
> Hi Deven
>
> I was curious to see if determining the property at run-time
> could apply also to dm-verity. It seems it could be done
> (I omit some checks, I also keep the expected value in hex
> format):
>
> ---
> md = dm_get_md(file_inode(ctx->file)->i_sb->s_dev);
> table = dm_get_live_table(md, &srcu_idx);
> num_targets = dm_table_get_num_targets(table);
>
> for (i = 0; i < num_targets; i++) {
> struct dm_target *ti = dm_table_get_target(table, i);
>
> if (strcmp(ti->type->name, "verity"))
> continue;
>
> ti->type->status(ti, STATUSTYPE_IMA, 0, result, sizeof(result));
> }
>
> dm_put_live_table(md, srcu_idx);
> dm_put(md);
>
> root_digest_ptr = strstr(result, "root_digest=");
> return !strncmp(expect->data, root_digest_ptr + 12, expect->len);
> ---
>
> Only dm_table_get_target() is not exported yet, but I guess it could
> be. dm_table_get_num_targets() is exported.
I had tried something similar in a very early draft of IPE. The issue
that comes with this is that when you compile device-mapper as a module
(CONFIG_BLK_DEV_DM=m) you start to get linking errors with this
approach.
Obviously, we can fix this in the IPE's module Kconfig by setting the
dependency to be =y, but it's something to highlight. My general
preference is to support the =m configuration by using these blobs.
The runtime approach does work with fs-verity, because fs-verity is a
file-system level feature that cannot be compiled as a module.
> With this code, you would not have to manage security blobs
> outside IPE. Maybe you could add a blob for the super block, so
> that you verify the dm-verity property just once per filesystem.
>
> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
>
>> +}
>> +
>> +static int dvrh_free(void **val)
>> +{
>> + struct counted_array *expect = (struct counted_array *)val;
>> +
>> + kfree(expect->data);
>> + kfree(expect);
>> +
>> + return 0;
>> +}
>> +
>> +IPE_MODULE(dvrh) = {
>> + .name = "dmverity_roothash",
>> + .version = 1,
>> + .parse = dvrh_parse,
>> + .free = dvrh_free,
>> + .eval = dvrh_eval,
>> +};
>> diff --git a/security/ipe/modules/dmverity_signature.c
>> b/security/ipe/modules/dmverity_signature.c
>> new file mode 100644
>> index 000000000000..08746fcbcb3e
>> --- /dev/null
>> +++ b/security/ipe/modules/dmverity_signature.c
>> @@ -0,0 +1,25 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) Microsoft Corporation. All rights reserved.
>> + */
>> +
>> +#include "ipe_module.h"
>> +
>> +#include <linux/fs.h>
>> +#include <linux/types.h>
>> +
>> +static bool dvv_eval(const struct ipe_eval_ctx *ctx, const void *val)
>> +{
>> + bool expect = (bool)val;
>> + bool eval = ctx->ipe_bdev && (!!ctx->ipe_bdev->sigdata);
>> +
>> + return expect == eval;
>> +}
>> +
>> +IPE_MODULE(dvv) = {
>> + .name = "dmverity_signature",
>> + .version = 1,
>> + .parse = ipe_bool_parse,
>> + .free = NULL,
>> + .eval = dvv_eval,
>> +};
>> --
>> 2.33.0
Users of the device mapper driver might want to obtain a device status,
with status types defined in the status_type_t enumerator.
If a function to get the status is exported by the device mapper, when
compiled as a module, it is not suitable to use by callers compiled builtin
in the kernel.
Introduce the real function to get the status, _dm_get_status(), in the
device mapper module, and add the stub dm_get_status() in dm-builtin.c, so
that it can be invoked by builtin callers.
The stub calls the real function if the device mapper is compiled builtin
or the module has been loaded. Calls to the real function are safely
disabled if the module is unloaded. The race condition is avoided by
incrementing the reference count of the module.
_dm_get_status() invokes the status() method for each device mapper table,
which writes a string to the supplied buffer as output. The buffer might
contain multiple strings concatenated together. If there is not enough
space available, the string is truncated and a termination character is put
at the end.
Signed-off-by: Roberto Sassu <[email protected]>
---
drivers/md/dm-builtin.c | 13 +++++++
drivers/md/dm-core.h | 5 +++
drivers/md/dm.c | 71 +++++++++++++++++++++++++++++++++++
include/linux/device-mapper.h | 3 ++
4 files changed, 92 insertions(+)
diff --git a/drivers/md/dm-builtin.c b/drivers/md/dm-builtin.c
index 8eb52e425141..cc1e9c27ab41 100644
--- a/drivers/md/dm-builtin.c
+++ b/drivers/md/dm-builtin.c
@@ -47,3 +47,16 @@ void dm_kobject_release(struct kobject *kobj)
}
EXPORT_SYMBOL(dm_kobject_release);
+
+dm_get_status_fn status_fn;
+EXPORT_SYMBOL(status_fn);
+
+ssize_t dm_get_status(dev_t dev, status_type_t type, const char *target_name,
+ u8 *buf, size_t buf_len)
+{
+ if (status_fn)
+ return status_fn(dev, type, target_name, buf, buf_len);
+
+ return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL(dm_get_status);
diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
index b855fef4f38a..6600ec260558 100644
--- a/drivers/md/dm-core.h
+++ b/drivers/md/dm-core.h
@@ -259,4 +259,9 @@ extern atomic_t dm_global_event_nr;
extern wait_queue_head_t dm_global_eventq;
void dm_issue_global_event(void);
+typedef ssize_t (*dm_get_status_fn)(dev_t dev, status_type_t type,
+ const char *target_name, u8 *buf,
+ size_t buf_len);
+
+extern dm_get_status_fn status_fn;
#endif
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 662742a310cb..55e59a4e3661 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -192,6 +192,74 @@ static unsigned dm_get_numa_node(void)
DM_NUMA_NODE, num_online_nodes() - 1);
}
+static ssize_t _dm_get_status(dev_t dev, status_type_t type,
+ const char *target_name, u8 *buf, size_t buf_len)
+{
+ struct mapped_device *md;
+ struct dm_table *table;
+ u8 *buf_ptr = buf;
+ ssize_t len, res = 0;
+ int srcu_idx, num_targets, i;
+
+ if (buf_len > INT_MAX)
+ return -EINVAL;
+
+ if (!buf_len)
+ return buf_len;
+
+ if (!try_module_get(THIS_MODULE))
+ return -EBUSY;
+
+ md = dm_get_md(dev);
+ if (!md) {
+ res = -ENOENT;
+ goto out_module;
+ }
+
+ table = dm_get_live_table(md, &srcu_idx);
+ if (!table) {
+ res = -ENOENT;
+ goto out_md;
+ }
+
+ memset(buf, 0, buf_len);
+
+ num_targets = dm_table_get_num_targets(table);
+
+ for (i = 0; i < num_targets; i++) {
+ struct dm_target *ti = dm_table_get_target(table, i);
+
+ if (!ti)
+ continue;
+
+ if (target_name && strcmp(ti->type->name, target_name))
+ continue;
+
+ if (!ti->type->status)
+ continue;
+
+ ti->type->status(ti, type, 0, buf_ptr, buf + buf_len - buf_ptr);
+
+ if (!*buf_ptr)
+ continue;
+
+ len = strlen(buf_ptr);
+ buf_ptr += len + 1;
+
+ if (buf_ptr == buf + buf_len)
+ break;
+
+ res += len + 1;
+ }
+
+ dm_put_live_table(md, srcu_idx);
+out_md:
+ dm_put(md);
+out_module:
+ module_put(THIS_MODULE);
+ return res;
+}
+
static int __init local_init(void)
{
int r;
@@ -275,6 +343,7 @@ static int __init dm_init(void)
goto bad;
}
+ status_fn = _dm_get_status;
return 0;
bad:
while (i--)
@@ -287,6 +356,8 @@ static void __exit dm_exit(void)
{
int i = ARRAY_SIZE(_exits);
+ status_fn = NULL;
+
while (i--)
_exits[i]();
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index a7df155ea49b..d97b296d3104 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -487,6 +487,9 @@ int dm_report_zones(struct block_device *bdev, sector_t start, sector_t sector,
struct dm_report_zones_args *args, unsigned int nr_zones);
#endif /* CONFIG_BLK_DEV_ZONED */
+ssize_t dm_get_status(dev_t dev, status_type_t type, const char *target_name,
+ u8 *buf, size_t buf_len);
+
/*
* Device mapper functions to parse and create devices specified by the
* parameter "dm-mod.create="
--
2.32.0
> From: Roberto Sassu
> Sent: Wednesday, December 1, 2021 5:37 PM
> Users of the device mapper driver might want to obtain a device status,
> with status types defined in the status_type_t enumerator.
>
> If a function to get the status is exported by the device mapper, when
> compiled as a module, it is not suitable to use by callers compiled builtin
> in the kernel.
>
> Introduce the real function to get the status, _dm_get_status(), in the
> device mapper module, and add the stub dm_get_status() in dm-builtin.c, so
> that it can be invoked by builtin callers.
>
> The stub calls the real function if the device mapper is compiled builtin
> or the module has been loaded. Calls to the real function are safely
> disabled if the module is unloaded. The race condition is avoided by
> incrementing the reference count of the module.
>
> _dm_get_status() invokes the status() method for each device mapper table,
> which writes a string to the supplied buffer as output. The buffer might
> contain multiple strings concatenated together. If there is not enough
> space available, the string is truncated and a termination character is put
> at the end.
>
> Signed-off-by: Roberto Sassu <[email protected]>
> ---
> drivers/md/dm-builtin.c | 13 +++++++
> drivers/md/dm-core.h | 5 +++
> drivers/md/dm.c | 71 +++++++++++++++++++++++++++++++++++
> include/linux/device-mapper.h | 3 ++
> 4 files changed, 92 insertions(+)
>
> diff --git a/drivers/md/dm-builtin.c b/drivers/md/dm-builtin.c
> index 8eb52e425141..cc1e9c27ab41 100644
> --- a/drivers/md/dm-builtin.c
> +++ b/drivers/md/dm-builtin.c
> @@ -47,3 +47,16 @@ void dm_kobject_release(struct kobject *kobj)
> }
>
> EXPORT_SYMBOL(dm_kobject_release);
> +
> +dm_get_status_fn status_fn;
> +EXPORT_SYMBOL(status_fn);
> +
> +ssize_t dm_get_status(dev_t dev, status_type_t type, const char
> *target_name,
> + u8 *buf, size_t buf_len)
> +{
> + if (status_fn)
> + return status_fn(dev, type, target_name, buf, buf_len);
> +
> + return -EOPNOTSUPP;
> +}
> +EXPORT_SYMBOL(dm_get_status);
> diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> index b855fef4f38a..6600ec260558 100644
> --- a/drivers/md/dm-core.h
> +++ b/drivers/md/dm-core.h
> @@ -259,4 +259,9 @@ extern atomic_t dm_global_event_nr;
> extern wait_queue_head_t dm_global_eventq;
> void dm_issue_global_event(void);
>
> +typedef ssize_t (*dm_get_status_fn)(dev_t dev, status_type_t type,
> + const char *target_name, u8 *buf,
> + size_t buf_len);
> +
> +extern dm_get_status_fn status_fn;
> #endif
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 662742a310cb..55e59a4e3661 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -192,6 +192,74 @@ static unsigned dm_get_numa_node(void)
> DM_NUMA_NODE,
> num_online_nodes() - 1);
> }
>
> +static ssize_t _dm_get_status(dev_t dev, status_type_t type,
> + const char *target_name, u8 *buf, size_t buf_len)
> +{
> + struct mapped_device *md;
> + struct dm_table *table;
> + u8 *buf_ptr = buf;
> + ssize_t len, res = 0;
> + int srcu_idx, num_targets, i;
> +
> + if (buf_len > INT_MAX)
> + return -EINVAL;
> +
> + if (!buf_len)
> + return buf_len;
> +
> + if (!try_module_get(THIS_MODULE))
> + return -EBUSY;
> +
> + md = dm_get_md(dev);
> + if (!md) {
> + res = -ENOENT;
> + goto out_module;
> + }
> +
> + table = dm_get_live_table(md, &srcu_idx);
> + if (!table) {
> + res = -ENOENT;
> + goto out_md;
> + }
> +
> + memset(buf, 0, buf_len);
> +
> + num_targets = dm_table_get_num_targets(table);
> +
> + for (i = 0; i < num_targets; i++) {
> + struct dm_target *ti = dm_table_get_target(table, i);
> +
> + if (!ti)
> + continue;
> +
> + if (target_name && strcmp(ti->type->name, target_name))
> + continue;
> +
> + if (!ti->type->status)
> + continue;
> +
> + ti->type->status(ti, type, 0, buf_ptr, buf + buf_len - buf_ptr);
> +
> + if (!*buf_ptr)
> + continue;
> +
> + len = strlen(buf_ptr);
> + buf_ptr += len + 1;
> +
> + if (buf_ptr == buf + buf_len)
> + break;
> +
> + res += len + 1;
The line above before the check.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua
> + }
> +
> + dm_put_live_table(md, srcu_idx);
> +out_md:
> + dm_put(md);
> +out_module:
> + module_put(THIS_MODULE);
> + return res;
> +}
> +
> static int __init local_init(void)
> {
> int r;
> @@ -275,6 +343,7 @@ static int __init dm_init(void)
> goto bad;
> }
>
> + status_fn = _dm_get_status;
> return 0;
> bad:
> while (i--)
> @@ -287,6 +356,8 @@ static void __exit dm_exit(void)
> {
> int i = ARRAY_SIZE(_exits);
>
> + status_fn = NULL;
> +
> while (i--)
> _exits[i]();
>
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index a7df155ea49b..d97b296d3104 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -487,6 +487,9 @@ int dm_report_zones(struct block_device *bdev,
> sector_t start, sector_t sector,
> struct dm_report_zones_args *args, unsigned int nr_zones);
> #endif /* CONFIG_BLK_DEV_ZONED */
>
> +ssize_t dm_get_status(dev_t dev, status_type_t type, const char
> *target_name,
> + u8 *buf, size_t buf_len);
> +
> /*
> * Device mapper functions to parse and create devices specified by the
> * parameter "dm-mod.create="
> --
> 2.32.0
On Wed, Dec 01, 2021 at 05:37:08PM +0100, Roberto Sassu wrote:
> Users of the device mapper driver might want to obtain a device status,
> with status types defined in the status_type_t enumerator.
The patch looks really odd. And without the corresponding user of your
new functionality it is entirely unreviewable anyway.
> From: Christoph Hellwig [mailto:[email protected]]
> Sent: Thursday, December 2, 2021 8:21 AM
> On Wed, Dec 01, 2021 at 05:37:08PM +0100, Roberto Sassu wrote:
> > Users of the device mapper driver might want to obtain a device status,
> > with status types defined in the status_type_t enumerator.
>
> The patch looks really odd. And without the corresponding user of your
> new functionality it is entirely unreviewable anyway.
Hi Christoph
ok, I will send it together with a patch for a not yet accepted
software, Integrity Policy Enforcement (IPE), that will be
the primary user of the introduced functionality.
Regarding the patch itself, could you please provide a more
detailed explanation?
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua
On Thu, Dec 02, 2021 at 07:59:38AM +0000, Roberto Sassu wrote:
> ok, I will send it together with a patch for a not yet accepted
> software, Integrity Policy Enforcement (IPE), that will be
> the primary user of the introduced functionality.
>
> Regarding the patch itself, could you please provide a more
> detailed explanation?
We don't build things into the kernel just as hooks. So in doubt you
need to restructured the code. And that a security module pokes into
a random block driver is a big hint that whatever you're trying to do is
completely broken.
> From: Christoph Hellwig [mailto:[email protected]]
> Sent: Thursday, December 2, 2021 9:44 AM
> On Thu, Dec 02, 2021 at 07:59:38AM +0000, Roberto Sassu wrote:
> > ok, I will send it together with a patch for a not yet accepted
> > software, Integrity Policy Enforcement (IPE), that will be
> > the primary user of the introduced functionality.
> >
> > Regarding the patch itself, could you please provide a more
> > detailed explanation?
>
> We don't build things into the kernel just as hooks. So in doubt you
> need to restructured the code. And that a security module pokes into
> a random block driver is a big hint that whatever you're trying to do is
> completely broken.
I will add more context to the discussion.
The problem being solved is how to grant access to files
which satisfy a property defined in the policy.
For example, a policy enforced by IPE could be:
policy_name="AllowDMVerityKmodules" policy_version=0.0.1
DEFAULT action=ALLOW
DEFAULT op=KMODULE action=DENY
op=KMODULE dmverity_roothash=3c64aae64ae5e8ca781df4d1fbff7c02cb78c4f18a79198263db192cc7f7ba11 action=ALLOW
This would require IPE to obtain somehow this property.
So far, there are two different approaches. The first one,
proposed by the IPE authors was to define a new LSM hook
for block devices, to append a security blob for those devices,
and to store the dm-verity root digest as soon as this information
can be determined. IPE will then access the security blob at
run-time and will match the blob content with the property
value in the policy.
The second one I'm proposing is to directly retrieve the
information at run-time, when files are accessed, and to
possibly cache the result of the evaluation per filesystem.
This would avoid to the introduction of a new LSM hook
and to append a security blob for the purpose of passing
information from the device mapper driver to IPE.
Security blobs are usually used to store LSM-specific
information such as a label (or a reference of it). Sometimes,
when the label must be stored persistently, the subsystem
responsible for this task, such as the VFS, uses subsystem-defined
methods to retrieve the label from the storage and copy it to
the security blob.
In this case, it is not an LSM-specific information but rather
an existing property of another subsystem IPE is interested in.
Since LSMs need anyway to inspect the object before making
the security decision, they could directly retrieve the information
that is already available, instead of making it redundant.
Even if I would prefer the second option, it would be fine for
me if the first is adopted.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua
On Thu, Dec 02, 2021 at 09:29:52AM +0000, Roberto Sassu wrote:
> The problem being solved is how to grant access to files
> which satisfy a property defined in the policy.
If you have want to enforce access to files in the block layer using
a specific stacking block driver you don't just have one layering
violation but a bunch of them. Please go back to the drawing board.
> From: Christoph Hellwig [mailto:[email protected]]
> Sent: Friday, December 3, 2021 7:52 AM
> On Thu, Dec 02, 2021 at 09:29:52AM +0000, Roberto Sassu wrote:
> > The problem being solved is how to grant access to files
> > which satisfy a property defined in the policy.
>
> If you have want to enforce access to files in the block layer using
> a specific stacking block driver you don't just have one layering
> violation but a bunch of them. Please go back to the drawing board.
Ok. I write my thoughts here, so that it is easier to align.
dm-verity provides block-level integrity, which means that
the block layer itself is responsible to not pass data to the
upper layer, the filesystem, if a block is found corrupted.
The dm-verity root digest represents the immutable state
of the block device. dm-verity is still responsible to enforce
accesses to the block device according to the root digest
passed at device setup time. Nothing changes, the block
layer still detects data corruption against the passed
reference value.
The task of the security layer is to decide whether or not
the root digest passed at device setup time is acceptable,
e.g. it represents a device containing genuine files coming
from a software vendor.
The mandatory policy can be enforced at different layers,
depending on whether the security controls are placed.
A possibility would be to deny mounting block devices that
don't satisfy the mandatory policy.
However, if the mandatory policy wants only to restrict
execution of approved files and allowing the rest, making
the decision at the block layer is too coarse and restrictive.
It would force the user to mount only approved block
devices. The security layer must operate on files to enforce
this policy.
Now probably there is the part where there is no agreement.
The integrity property of a block device applies also to the
files on the filesystem mounted from that device. User space
programs cannot access files in that filesystem coming from a
device with a different dm-verity root digest, or files stored
in a corrupted block device.
If what I wrote is correct, that the integrity property is preserved
across the layers, this would give enough flexibility to enforce
policies at a higher layer, although that property is guaranteed
by a lower layer.
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua
> From: Roberto Sassu [mailto:[email protected]]
> Sent: Friday, December 3, 2021 11:20 AM
> > From: Christoph Hellwig [mailto:[email protected]]
> > Sent: Friday, December 3, 2021 7:52 AM
> > On Thu, Dec 02, 2021 at 09:29:52AM +0000, Roberto Sassu wrote:
> > > The problem being solved is how to grant access to files
> > > which satisfy a property defined in the policy.
> >
> > If you have want to enforce access to files in the block layer using
> > a specific stacking block driver you don't just have one layering
> > violation but a bunch of them. Please go back to the drawing board.
>
> Ok. I write my thoughts here, so that it is easier to align.
>
> dm-verity provides block-level integrity, which means that
> the block layer itself is responsible to not pass data to the
> upper layer, the filesystem, if a block is found corrupted.
>
> The dm-verity root digest represents the immutable state
> of the block device. dm-verity is still responsible to enforce
> accesses to the block device according to the root digest
> passed at device setup time. Nothing changes, the block
> layer still detects data corruption against the passed
> reference value.
>
> The task of the security layer is to decide whether or not
> the root digest passed at device setup time is acceptable,
> e.g. it represents a device containing genuine files coming
> from a software vendor.
>
> The mandatory policy can be enforced at different layers,
> depending on whether the security controls are placed.
> A possibility would be to deny mounting block devices that
> don't satisfy the mandatory policy.
>
> However, if the mandatory policy wants only to restrict
> execution of approved files and allowing the rest, making
> the decision at the block layer is too coarse and restrictive.
> It would force the user to mount only approved block
> devices. The security layer must operate on files to enforce
> this policy.
>
> Now probably there is the part where there is no agreement.
>
> The integrity property of a block device applies also to the
> files on the filesystem mounted from that device. User space
> programs cannot access files in that filesystem coming from a
> device with a different dm-verity root digest, or files stored
> in a corrupted block device.
>
> If what I wrote is correct, that the integrity property is preserved
> across the layers, this would give enough flexibility to enforce
> policies at a higher layer, although that property is guaranteed
> by a lower layer.
Hi Christoph
did I address your concerns? If yes, I could send the new patch
set, including the patch that uses the new functionality.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua