linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] device mapper target measurements using IMA
@ 2021-07-13  0:48 Tushar Sugandhi
  2021-07-13  0:48 ` [PATCH 1/7] dm: measure data on table load Tushar Sugandhi
                   ` (8 more replies)
  0 siblings, 9 replies; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-13  0:48 UTC (permalink / raw)
  To: dm-devel, agk, snitzer; +Cc: zohar, linux-integrity, nramas, tusharsu

For a given system, various external services/infrastructure tools
(including the attestation service) interact with it - both during the
setup and during rest of the system run-time.  They share sensitive data
and/or execute critical workload on that system.  The external services
may want to verify the current run-time state of the relevant kernel
subsystems before fully trusting the system with business-critical
data/workload.

Device mapper is one such kernel subsystem that plays a critical role on
a given system by providing various important functionalities to the
block devices with various target types like crypt, verity, integrity 
etc.  Each of these target types’ functionalities can be configured with
various attributes.  The attributes chosen to configure these target types
can significantly impact the security profile of the block device,
and in-turn, of the system itself.  For instance, the type of encryption
algorithm and the key size determines the strength of encryption for a
given block device.

Therefore, verifying the current state of various block devices as well
as their various target attributes is crucial for external services
before fully trusting the system with business-critical data/workload.

IMA provides the necessary functionality for device mapper to measure the
state and configuration of various block devices -
  - BY device mapper itself, from within the kernel,
  - in a tamper resistant way,
  - and re-measured - triggered on state/configuration change.

This patch series uses this IMA functionality, by calling the function
ima_measure_critical_data(), when a block device state is changed (e.g.
on device create, resume, rename, remove etc.)  It measures the device
state and configuration and stores it in IMA logs, so that it can be
used by external services for managing the system.


Tushar Sugandhi (7):
  dm: measure data on table load
  dm: measure data on device resume
  dm: measure data on device remove
  dm: measure data on table clear
  dm: measure data on device rename
  dm: update target specific status functions to measure data
  dm: add documentation for IMA measurement support

 .../admin-guide/device-mapper/dm-ima.rst      | 306 ++++++++
 .../admin-guide/device-mapper/index.rst       |   1 +
 drivers/md/Makefile                           |   2 +
 drivers/md/dm-cache-target.c                  |  24 +
 drivers/md/dm-clone-target.c                  |   7 +
 drivers/md/dm-core.h                          |   5 +
 drivers/md/dm-crypt.c                         |  29 +
 drivers/md/dm-delay.c                         |   4 +
 drivers/md/dm-dust.c                          |   4 +
 drivers/md/dm-ebs-target.c                    |   3 +
 drivers/md/dm-era-target.c                    |   4 +
 drivers/md/dm-flakey.c                        |   4 +
 drivers/md/dm-ima.c                           | 725 ++++++++++++++++++
 drivers/md/dm-ima.h                           |  56 ++
 drivers/md/dm-integrity.c                     |  24 +
 drivers/md/dm-ioctl.c                         |  24 +-
 drivers/md/dm-linear.c                        |   8 +
 drivers/md/dm-log-userspace-base.c            |   3 +
 drivers/md/dm-log-writes.c                    |   4 +
 drivers/md/dm-log.c                           |  10 +
 drivers/md/dm-mpath.c                         |  29 +
 drivers/md/dm-ps-historical-service-time.c    |   3 +
 drivers/md/dm-ps-io-affinity.c                |   3 +
 drivers/md/dm-ps-queue-length.c               |   3 +
 drivers/md/dm-ps-round-robin.c                |   4 +
 drivers/md/dm-ps-service-time.c               |   3 +
 drivers/md/dm-raid.c                          |  39 +
 drivers/md/dm-raid1.c                         |  18 +
 drivers/md/dm-snap-persistent.c               |   4 +
 drivers/md/dm-snap-transient.c                |   4 +
 drivers/md/dm-snap.c                          |  13 +
 drivers/md/dm-stripe.c                        |  16 +
 drivers/md/dm-switch.c                        |   4 +
 drivers/md/dm-thin.c                          |   8 +
 drivers/md/dm-unstripe.c                      |   4 +
 drivers/md/dm-verity-target.c                 |  45 ++
 drivers/md/dm-writecache.c                    |   3 +
 drivers/md/dm-zoned-target.c                  |   3 +
 drivers/md/dm.c                               |   3 +
 include/linux/device-mapper.h                 |   6 +-
 include/uapi/linux/dm-ioctl.h                 |   6 +
 41 files changed, 1464 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/admin-guide/device-mapper/dm-ima.rst
 create mode 100644 drivers/md/dm-ima.c
 create mode 100644 drivers/md/dm-ima.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/7] dm: measure data on table load
  2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
@ 2021-07-13  0:48 ` Tushar Sugandhi
  2021-07-21  2:12   ` Mimi Zohar
  2021-07-13  0:48 ` [PATCH 2/7] dm: measure data on device resume Tushar Sugandhi
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-13  0:48 UTC (permalink / raw)
  To: dm-devel, agk, snitzer; +Cc: zohar, linux-integrity, nramas, tusharsu

DM configures a block device with various target specific attributes
passed to it as a table.  DM loads the table, and calls each target’s
respective constructors with the attributes as input parameters.
Some of these attributes are critical to ensure the device meets
certain security bar.  Thus, IMA should measure these attributes, to
ensure they are not tampered with, during the lifetime of the device.
So that the external services can have high confidence in the
configuration of the block-devices on a given system.

Some devices may have large tables.  And a given device may change its
state (table-load, suspend, resume, rename, remove, table-clear etc.)
many times.  Measuring these attributes each time when the device 
changes its state will significantly increase the size of the IMA logs.
Further, once configured, these attributes are not expected to change
unless a new table is loaded, or a device is removed and recreated.
Therefore the clear-text of the attributes should only be measured
during table load, and the hash of the active/inactive table should be
measured for the remaining device state changes.

Measure device parameters, as well as target specific attributes, during
table load, using IMA function ima_measure_critical_data().  Compute the
hash of the inactive table and store it for measurements during future
state change.  If a load is called multiple times, update the inactive
table hash with the hash of the latest populated table.  So that the
correct inactive table hash is measured when the device transitions to
different states like resume, remove, rename etc.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 drivers/md/Makefile           |   2 +
 drivers/md/dm-core.h          |   5 +
 drivers/md/dm-ima.c           | 345 ++++++++++++++++++++++++++++++++++
 drivers/md/dm-ima.h           |  50 +++++
 drivers/md/dm-ioctl.c         |   7 +-
 drivers/md/dm.c               |   3 +
 include/linux/device-mapper.h |   2 +-
 include/uapi/linux/dm-ioctl.h |   6 +
 8 files changed, 418 insertions(+), 2 deletions(-)
 create mode 100644 drivers/md/dm-ima.c
 create mode 100644 drivers/md/dm-ima.h

diff --git a/drivers/md/Makefile b/drivers/md/Makefile
index ef7ddc27685c..af349b7cbf8b 100644
--- a/drivers/md/Makefile
+++ b/drivers/md/Makefile
@@ -92,6 +92,8 @@ ifeq ($(CONFIG_DM_UEVENT),y)
 dm-mod-objs			+= dm-uevent.o
 endif
 
+dm-mod-objs			+= dm-ima.o
+
 ifeq ($(CONFIG_DM_VERITY_FEC),y)
 dm-verity-objs			+= dm-verity-fec.o
 endif
diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
index 5953ff2bd260..f67f898803db 100644
--- a/drivers/md/dm-core.h
+++ b/drivers/md/dm-core.h
@@ -18,6 +18,7 @@
 #include <trace/events/block.h>
 
 #include "dm.h"
+#include "dm-ima.h"
 
 #define DM_RESERVED_MAX_IOS		1024
 
@@ -114,6 +115,10 @@ struct mapped_device {
 	bool init_tio_pdu:1;
 
 	struct srcu_struct io_barrier;
+
+#ifdef CONFIG_IMA
+	struct dm_ima_measurements ima;
+#endif
 };
 
 void disable_discard(struct mapped_device *md);
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
new file mode 100644
index 000000000000..b752517380c3
--- /dev/null
+++ b/drivers/md/dm-ima.c
@@ -0,0 +1,345 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 Microsoft Corporation
+ *
+ * Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>
+ *
+ * File: dm-ima.c
+ *       Enables IMA measurements for DM targets
+ */
+
+#include "dm-core.h"
+#include "dm-ima.h"
+
+#include <linux/ima.h>
+#include <linux/device-mapper.h>
+#include <crypto/hash.h>
+#include <linux/crypto.h>
+#include <crypto/hash_info.h>
+
+#define DM_MSG_PREFIX "dm-ima"
+
+#ifdef CONFIG_IMA
+/*
+ * Internal function to prefix separator characters in input buffer with escape
+ * character, so that they don't interfere with the construction of key-value pairs,
+ * and clients can split the key1=val1,key2=val2,key3=val3; pairs properly.
+ */
+static void fix_separator_chars(char **buf)
+{
+	int l = strlen(*buf);
+	int i, j, sp = 0;
+
+	for (i = 0; i < l; i++)
+		if ((*buf)[i] == '\\' || (*buf)[i] == ';' || (*buf)[i] == '=' || (*buf)[i] == ',')
+			sp++;
+
+	if (!sp)
+		return;
+
+	for (i = l-1, j = i+sp; i >= 0; i--) {
+		(*buf)[j--] = (*buf)[i];
+		if ((*buf)[i] == '\\' || (*buf)[i] == ';' || (*buf)[i] == '=' || (*buf)[i] == ',')
+			(*buf)[j--] = '\\';
+	}
+}
+
+/*
+ * Internal function to allocate memory for IMA measurements.
+ */
+static void *dm_ima_alloc(size_t len, gfp_t flags, bool noio)
+{
+	unsigned int noio_flag;
+	void *ptr;
+
+	if (noio)
+		noio_flag = memalloc_noio_save();
+
+	ptr = kzalloc(len, flags);
+
+	if (noio)
+		memalloc_noio_restore(noio_flag);
+
+	return ptr;
+}
+
+/*
+ * Internal function to allocate and copy name and uuid for IMA measurements.
+ */
+static int dm_ima_alloc_and_copy_name_uuid(struct mapped_device *md, char **dev_name,
+					   char **dev_uuid, bool noio)
+{
+	int r;
+	*dev_name = dm_ima_alloc(DM_NAME_LEN*2, GFP_KERNEL, noio);
+	if (!(*dev_name)) {
+		r = -ENOMEM;
+		goto error;
+	}
+
+	*dev_uuid = dm_ima_alloc(DM_UUID_LEN*2, GFP_KERNEL, noio);
+	if (!(*dev_uuid)) {
+		r = -ENOMEM;
+		goto error;
+	}
+
+	r = dm_copy_name_and_uuid(md, *dev_name, *dev_uuid);
+	if (r)
+		goto error;
+
+	fix_separator_chars(dev_name);
+	fix_separator_chars(dev_uuid);
+
+	return 0;
+error:
+	kfree(*dev_name);
+	kfree(*dev_uuid);
+	*dev_name = NULL;
+	*dev_uuid = NULL;
+	return r;
+}
+
+/*
+ * Internal function to allocate and copy device data for IMA measurements.
+ */
+static int dm_ima_alloc_and_copy_device_data(struct mapped_device *md, char **device_data,
+					     unsigned int num_targets, bool noio)
+{
+	char *dev_name = NULL, *dev_uuid = NULL;
+	int r;
+
+	r = dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio);
+	if (r)
+		return r;
+
+	*device_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, GFP_KERNEL, noio);
+	if (!(*device_data)) {
+		r = -ENOMEM;
+		goto error;
+	}
+
+	scnprintf(*device_data, DM_IMA_DEVICE_BUF_LEN,
+		  "name=%s,uuid=%s,major=%d,minor=%d,minor_count=%d,num_targets=%u;",
+		  dev_name, dev_uuid, md->disk->major, md->disk->first_minor,
+		  md->disk->minors, num_targets);
+
+error:
+	kfree(dev_name);
+	kfree(dev_uuid);
+	return r;
+}
+
+/*
+ * Internal wrapper function to call IMA to measure DM data.
+ */
+static void dm_ima_measure_data(const char *event_name, const void *buf, size_t buf_len,
+				bool noio)
+{
+	unsigned int noio_flag;
+
+	if (noio)
+		noio_flag = memalloc_noio_save();
+
+	ima_measure_critical_data(DM_NAME, event_name, buf, buf_len, false);
+
+	if (noio)
+		memalloc_noio_restore(noio_flag);
+}
+
+/*
+ * Initialize the dm ima related data structure variables.
+ */
+
+void dm_ima_reset_data(struct mapped_device *md)
+{
+	memset(&(md->ima), 0, sizeof(md->ima));
+}
+
+/*
+ * Build up the IMA data for each target, and finally measure.
+ */
+void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags)
+{
+	size_t device_data_buf_len, target_metadata_buf_len, target_data_buf_len, l = 0;
+	char *target_metadata_buf = NULL, *target_data_buf = NULL, *digest_buf = NULL;
+	char *ima_buf = NULL, *device_data_buf = NULL;
+	int digest_size, last_target_measured = -1, r;
+	status_type_t type = STATUSTYPE_IMA;
+	size_t cur_total_buf_len = 0;
+	unsigned int num_targets, i;
+	SHASH_DESC_ON_STACK(shash, NULL);
+	struct crypto_shash *tfm;
+	u8 *digest = NULL;
+	bool noio = false;
+
+	ima_buf = dm_ima_alloc(DM_IMA_MEASUREMENT_BUF_LEN, GFP_KERNEL, noio);
+	if (!ima_buf)
+		return;
+
+	target_metadata_buf = dm_ima_alloc(DM_IMA_TARGET_METADATA_BUF_LEN, GFP_KERNEL, noio);
+	if (!target_metadata_buf)
+		goto error;
+
+	target_data_buf = dm_ima_alloc(DM_IMA_TARGET_DATA_BUF_LEN, GFP_KERNEL, noio);
+	if (!target_data_buf)
+		goto error;
+
+	num_targets = dm_table_get_num_targets(table);
+
+	if (dm_ima_alloc_and_copy_device_data(table->md, &device_data_buf, num_targets, noio))
+		goto error;
+
+	tfm = crypto_alloc_shash("sha256", 0, 0);
+	if (IS_ERR(tfm))
+		goto error;
+
+	shash->tfm = tfm;
+	digest_size = crypto_shash_digestsize(tfm);
+	digest = dm_ima_alloc(digest_size, GFP_KERNEL, noio);
+	if (!digest)
+		goto error;
+
+	r = crypto_shash_init(shash);
+	if (r)
+		return;
+
+	device_data_buf_len = strlen(device_data_buf);
+	memcpy(ima_buf + l, device_data_buf, device_data_buf_len);
+	l += device_data_buf_len;
+
+	for (i = 0; i < num_targets; i++) {
+		struct dm_target *ti = dm_table_get_target(table, i);
+
+		if (!ti)
+			goto error;
+
+		last_target_measured = 0;
+
+		/*
+		 * First retrieve the target metadata.
+		 */
+		scnprintf(target_metadata_buf, DM_IMA_TARGET_METADATA_BUF_LEN,
+			  "target_index=%d,target_begin=%llu,target_len=%llu,",
+			  i, ti->begin, ti->len);
+		target_metadata_buf_len = strlen(target_metadata_buf);
+
+		/*
+		 * Then retrieve the actual target data.
+		 */
+		if (ti->type->status)
+			ti->type->status(ti, type, status_flags, target_data_buf,
+					 DM_IMA_TARGET_DATA_BUF_LEN);
+		else
+			target_data_buf[0] = '\0';
+
+		target_data_buf_len = strlen(target_data_buf);
+
+		/*
+		 * Check if the total data can fit into the IMA buffer.
+		 */
+		cur_total_buf_len = l + target_metadata_buf_len + target_data_buf_len;
+
+		/*
+		 * IMA measurements for DM targets are best-effort.
+		 * If the total data buffered so far, including the current target,
+		 * is too large to fit into DM_IMA_MEASUREMENT_BUF_LEN, measure what
+		 * we have in the current buffer, and continue measuring the remaining
+		 * targets by prefixing the device metadata again.
+		 */
+		if (unlikely(cur_total_buf_len >= DM_IMA_MEASUREMENT_BUF_LEN)) {
+			dm_ima_measure_data("table_load", ima_buf, l, noio);
+			r = crypto_shash_update(shash, (const u8 *)ima_buf, l);
+			if (r < 0)
+				goto error;
+
+			memset(ima_buf, 0, DM_IMA_MEASUREMENT_BUF_LEN);
+			l = 0;
+
+			/*
+			 * Each new "table_load" entry in IMA log should have device data prefix,
+			 * so that multiple records from the same table_load for a given device
+			 * can be linked together.
+			 */
+			memcpy(ima_buf + l, device_data_buf, device_data_buf_len);
+			l += device_data_buf_len;
+
+			/*
+			 * If this iteration of the for loop turns out to be the last target in the
+			 * table, dm_ima_measure_data("table_load", ...) doesn't need to be called
+			 * again, just the hash needs to be finalized. "last_target_measured" tracks
+			 * this state.
+			 */
+			last_target_measured = 1;
+		}
+
+		/*
+		 * Fill-in all the target metadata, so that multiple targets for the same device
+		 * can be linked together.
+		 */
+
+		memcpy(ima_buf + l, target_metadata_buf, target_metadata_buf_len);
+		l += target_metadata_buf_len;
+
+		memcpy(ima_buf + l, target_data_buf, target_data_buf_len);
+		l += target_data_buf_len;
+	}
+
+
+	if (!last_target_measured) {
+		dm_ima_measure_data("table_load", ima_buf, l, noio);
+
+		r = crypto_shash_update(shash, (const u8 *)ima_buf, l);
+		if (r < 0)
+			goto error;
+
+	}
+
+	/*
+	 * Finalize the table hash, and store it in table->md->ima.inactive_table.hash,
+	 * so that the table data can be verified against the future device state change
+	 * events, e.g. resume, rename, remove, table-clear etc.
+	 */
+
+	r = crypto_shash_final(shash, digest);
+	if (r < 0)
+		goto error;
+
+	digest_buf = dm_ima_alloc((digest_size*2)+1, GFP_KERNEL, noio);
+	if (!digest_buf)
+		goto error;
+
+	for (i = 0; i < digest_size; i++)
+		snprintf((digest_buf+(i*2)), 3, "%02x", digest[i]);
+
+	if (table->md->ima.active_table.hash != table->md->ima.inactive_table.hash)
+		kfree(table->md->ima.inactive_table.hash);
+
+	table->md->ima.inactive_table.hash = digest_buf;
+	table->md->ima.inactive_table.hash_len = strlen(digest_buf);
+	table->md->ima.inactive_table.num_targets = num_targets;
+
+	if (table->md->ima.active_table.device_metadata !=
+	    table->md->ima.inactive_table.device_metadata)
+		kfree(table->md->ima.inactive_table.device_metadata);
+
+	table->md->ima.inactive_table.device_metadata = device_data_buf;
+	table->md->ima.inactive_table.device_metadata_len = device_data_buf_len;
+
+	goto exit;
+error:
+	kfree(digest_buf);
+	kfree(device_data_buf);
+exit:
+	kfree(digest);
+	crypto_free_shash(tfm);
+	kfree(ima_buf);
+	kfree(target_metadata_buf);
+	kfree(target_data_buf);
+}
+
+#else
+void dm_ima_reset_data(struct mapped_device *md) {}
+void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags) {}
+#endif
+MODULE_AUTHOR("Tushar Sugandhi <tusharsu@linux.microsoft.com>");
+MODULE_DESCRIPTION("Enables IMA measurements for DM targets");
+MODULE_LICENSE("GPL");
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
new file mode 100644
index 000000000000..3c92a2523284
--- /dev/null
+++ b/drivers/md/dm-ima.h
@@ -0,0 +1,50 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2021 Microsoft Corporation
+ *
+ * Author: Tushar Sugandhi <tusharsu@linux.microsoft.com>
+ *
+ * File: dm-ima.h
+ *       Header file for device mapper IMA measurements.
+ */
+
+#ifndef DM_IMA_H
+#define DM_IMA_H
+
+#define DM_IMA_MEASUREMENT_BUF_LEN	4096
+#define DM_IMA_DEVICE_BUF_LEN		1024
+#define DM_IMA_TARGET_METADATA_BUF_LEN	128
+#define DM_IMA_TARGET_DATA_BUF_LEN	2048
+
+struct dm_ima_device_table_metadata {
+	/*
+	 * Contains data specific to the device which is common across
+	 * all the targets in the table.e.g. name, uuid, major, minor etc.
+	 * The values are stored in comma separated list of key1=val1,key2=val2; pairs
+	 * delimited by a semicolon at the end of the list.
+	 */
+	char *device_metadata;
+	unsigned int device_metadata_len;
+	unsigned int num_targets;
+
+	/*
+	 * Contains the sha256 hashs of the IMA measurements of the
+	 * target attributes key-value pairs from the active/inactive tables.
+	 */
+	char *hash;
+	unsigned int hash_len;
+
+};
+
+/*
+ * This structure contains device metadata, and table hash for
+ * active and inactive tables for ima measurements.
+ */
+struct dm_ima_measurements {
+	struct dm_ima_device_table_metadata active_table;
+	struct dm_ima_device_table_metadata inactive_table;
+};
+
+void dm_ima_reset_data(struct mapped_device *md);
+void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags);
+#endif /*DM_IMA_H*/
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 2209cbcd84db..e6e9fe74baf9 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -6,7 +6,7 @@
  */
 
 #include "dm-core.h"
-
+#include "dm-ima.h"
 #include <linux/module.h>
 #include <linux/vmalloc.h>
 #include <linux/miscdevice.h>
@@ -20,6 +20,7 @@
 #include <linux/compat.h>
 
 #include <linux/uaccess.h>
+#include <linux/ima.h>
 
 #define DM_MSG_PREFIX "ioctl"
 #define DM_DRIVER_EMAIL "dm-devel@redhat.com"
@@ -1224,6 +1225,8 @@ static void retrieve_status(struct dm_table *table,
 
 	if (param->flags & DM_STATUS_TABLE_FLAG)
 		type = STATUSTYPE_TABLE;
+	else if (param->flags & DM_IMA_MEASUREMENT_FLAG)
+		type = STATUSTYPE_IMA;
 	else
 		type = STATUSTYPE_INFO;
 
@@ -1425,6 +1428,8 @@ static int table_load(struct file *filp, struct dm_ioctl *param, size_t param_si
 	if (r)
 		goto err_unlock_md_type;
 
+	dm_ima_measure_on_table_load(t, STATUSTYPE_IMA);
+
 	immutable_target_type = dm_get_immutable_target_type(md);
 	if (immutable_target_type &&
 	    (immutable_target_type != dm_table_get_immutable_target_type(t)) &&
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index ca2aedd8ee7d..63c83f213fd4 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -8,6 +8,7 @@
 #include "dm-core.h"
 #include "dm-rq.h"
 #include "dm-uevent.h"
+#include "dm-ima.h"
 
 #include <linux/init.h>
 #include <linux/module.h>
@@ -2109,6 +2110,8 @@ int dm_create(int minor, struct mapped_device **result)
 		return r;
 	}
 
+	dm_ima_reset_data(md);
+
 	*result = md;
 	return 0;
 }
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index ff700fb6ce1d..738a7d023650 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -31,7 +31,7 @@ enum dm_queue_mode {
 	DM_TYPE_DAX_BIO_BASED	 = 3,
 };
 
-typedef enum { STATUSTYPE_INFO, STATUSTYPE_TABLE } status_type_t;
+typedef enum { STATUSTYPE_INFO, STATUSTYPE_TABLE, STATUSTYPE_IMA } status_type_t;
 
 union map_info {
 	void *ptr;
diff --git a/include/uapi/linux/dm-ioctl.h b/include/uapi/linux/dm-ioctl.h
index e5c6e458bdf7..c12ce30b52df 100644
--- a/include/uapi/linux/dm-ioctl.h
+++ b/include/uapi/linux/dm-ioctl.h
@@ -376,4 +376,10 @@ enum {
  */
 #define DM_INTERNAL_SUSPEND_FLAG	(1 << 18) /* Out */
 
+/*
+ * If set, returns in the in buffer passed by UM, the raw table information
+ * that would be measured by IMA subsystem on device state change.
+ */
+#define DM_IMA_MEASUREMENT_FLAG	(1 << 19) /* In */
+
 #endif				/* _LINUX_DM_IOCTL_H */
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 2/7] dm: measure data on device resume
  2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
  2021-07-13  0:48 ` [PATCH 1/7] dm: measure data on table load Tushar Sugandhi
@ 2021-07-13  0:48 ` Tushar Sugandhi
  2021-07-13  0:49 ` [PATCH 3/7] dm: measure data on device remove Tushar Sugandhi
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-13  0:48 UTC (permalink / raw)
  To: dm-devel, agk, snitzer; +Cc: zohar, linux-integrity, nramas, tusharsu

A given block device can load a table multiple times, with different
input parameters, before eventually resuming it.  Further, a device may
be suspended and then resumed.  The device may never resume after a
table-load.  Because of the above valid scenarios for a given device,
it is important to measure and log the device resume event using IMA.

Also, if the table is large, measuring it in clear-text each time the
device changes state, will unnecessarily increase the size of IMA log.
Since the table clear-text is already measured during table-load event,
measuring the hash during resume should be sufficient to validate the
table contents.

Measure the device parameters, and hash of the active table, when the
device is resumed.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 drivers/md/dm-ima.c   | 118 ++++++++++++++++++++++++++++++++++++++++++
 drivers/md/dm-ima.h   |   2 +
 drivers/md/dm-ioctl.c |   8 ++-
 3 files changed, 126 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index b752517380c3..1c545717adf4 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -145,6 +145,27 @@ static void dm_ima_measure_data(const char *event_name, const void *buf, size_t
 		memalloc_noio_restore(noio_flag);
 }
 
+/*
+ * Internal function to allocate and copy current device capacity for IMA measurements.
+ */
+
+static int dm_ima_alloc_and_copy_capacity_str(struct mapped_device *md, char **capacity_str,
+					      bool noio)
+{
+	sector_t capacity;
+
+	capacity = get_capacity(md->disk);
+
+	*capacity_str = dm_ima_alloc(DM_IMA_DEVICE_CAPACITY_BUF_LEN, GFP_KERNEL, noio);
+	if (!(*capacity_str))
+		return -ENOMEM;
+
+	scnprintf(*capacity_str, DM_IMA_DEVICE_BUF_LEN, "current_device_capacity=%llu;",
+		  capacity);
+
+	return 0;
+}
+
 /*
  * Initialize the dm ima related data structure variables.
  */
@@ -336,9 +357,106 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
 	kfree(target_data_buf);
 }
 
+/*
+ * Measure IMA data on device resume.
+ */
+void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
+{
+	char *device_table_data, *dev_name = NULL, *dev_uuid = NULL, *capacity_str = NULL;
+	char active[] = "active_table_hash=";
+	unsigned int active_len = strlen(active), capacity_len = 0;
+	unsigned int l = 0;
+	bool noio = true;
+	int r;
+
+	device_table_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, GFP_KERNEL, noio);
+	if (!device_table_data)
+		return;
+
+	r = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
+	if (r)
+		goto error;
+
+	if (swap) {
+		if (md->ima.active_table.hash != md->ima.inactive_table.hash)
+			kfree(md->ima.active_table.hash);
+
+		md->ima.active_table.hash = NULL;
+		md->ima.active_table.hash_len = 0;
+
+		if (md->ima.active_table.device_metadata !=
+		    md->ima.inactive_table.device_metadata)
+			kfree(md->ima.active_table.device_metadata);
+
+		md->ima.active_table.device_metadata = NULL;
+		md->ima.active_table.device_metadata_len = 0;
+		md->ima.active_table.num_targets = 0;
+
+		if (md->ima.inactive_table.hash) {
+			md->ima.active_table.hash = md->ima.inactive_table.hash;
+			md->ima.active_table.hash_len = md->ima.inactive_table.hash_len;
+			md->ima.inactive_table.hash = NULL;
+			md->ima.inactive_table.hash_len = 0;
+		}
+
+		if (md->ima.inactive_table.device_metadata) {
+			md->ima.active_table.device_metadata =
+				md->ima.inactive_table.device_metadata;
+			md->ima.active_table.device_metadata_len =
+				md->ima.inactive_table.device_metadata_len;
+			md->ima.active_table.num_targets = md->ima.inactive_table.num_targets;
+			md->ima.inactive_table.device_metadata = NULL;
+			md->ima.inactive_table.device_metadata_len = 0;
+			md->ima.inactive_table.num_targets = 0;
+		}
+	}
+
+	if (md->ima.active_table.device_metadata) {
+		l = md->ima.active_table.device_metadata_len;
+		memcpy(device_table_data, md->ima.active_table.device_metadata, l);
+	}
+
+	if (md->ima.active_table.hash) {
+		memcpy(device_table_data + l, active, active_len);
+		l += active_len;
+
+		memcpy(device_table_data + l, md->ima.active_table.hash,
+		       md->ima.active_table.hash_len);
+		l += md->ima.active_table.hash_len;
+
+		memcpy(device_table_data + l, ";", 1);
+		l++;
+	}
+
+	if (!l) {
+		r = dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio);
+		if (r)
+			goto error;
+
+		scnprintf(device_table_data, DM_IMA_DEVICE_BUF_LEN,
+			  "name=%s,uuid=%s;device_resume=no_data;",
+			  dev_name, dev_uuid);
+		l += strlen(device_table_data);
+
+	}
+
+	capacity_len = strlen(capacity_str);
+	memcpy(device_table_data + l, capacity_str, capacity_len);
+	l += capacity_len;
+
+	dm_ima_measure_data("device_resume", device_table_data, l, noio);
+
+	kfree(dev_name);
+	kfree(dev_uuid);
+error:
+	kfree(capacity_str);
+	kfree(device_table_data);
+}
+
 #else
 void dm_ima_reset_data(struct mapped_device *md) {}
 void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags) {}
+void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap) {}
 #endif
 MODULE_AUTHOR("Tushar Sugandhi <tusharsu@linux.microsoft.com>");
 MODULE_DESCRIPTION("Enables IMA measurements for DM targets");
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index 3c92a2523284..cafdcf5064c7 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -15,6 +15,7 @@
 #define DM_IMA_DEVICE_BUF_LEN		1024
 #define DM_IMA_TARGET_METADATA_BUF_LEN	128
 #define DM_IMA_TARGET_DATA_BUF_LEN	2048
+#define DM_IMA_DEVICE_CAPACITY_BUF_LEN	128
 
 struct dm_ima_device_table_metadata {
 	/*
@@ -47,4 +48,5 @@ struct dm_ima_measurements {
 
 void dm_ima_reset_data(struct mapped_device *md);
 void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags);
+void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap);
 #endif /*DM_IMA_H*/
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index e6e9fe74baf9..11af40f9b9c0 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1160,8 +1160,12 @@ static int do_resume(struct dm_ioctl *param)
 
 	if (dm_suspended_md(md)) {
 		r = dm_resume(md);
-		if (!r && !dm_kobject_uevent(md, KOBJ_CHANGE, param->event_nr))
-			param->flags |= DM_UEVENT_GENERATED_FLAG;
+		if (!r) {
+			dm_ima_measure_on_device_resume(md, new_map ? true : false);
+
+			if (!dm_kobject_uevent(md, KOBJ_CHANGE, param->event_nr))
+				param->flags |= DM_UEVENT_GENERATED_FLAG;
+		}
 	}
 
 	/*
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 3/7] dm: measure data on device remove
  2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
  2021-07-13  0:48 ` [PATCH 1/7] dm: measure data on table load Tushar Sugandhi
  2021-07-13  0:48 ` [PATCH 2/7] dm: measure data on device resume Tushar Sugandhi
@ 2021-07-13  0:49 ` Tushar Sugandhi
  2021-07-13  0:49 ` [PATCH 4/7] dm: measure data on table clear Tushar Sugandhi
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-13  0:49 UTC (permalink / raw)
  To: dm-devel, agk, snitzer; +Cc: zohar, linux-integrity, nramas, tusharsu

Presence of an active block-device, configured with expected parameters,
is important for an external attestation service to determine if a system
meets the attestation requirements.  Therefore it is important for DM to
measure the device remove events.

Measure device parameters and table hashes when the device is removed,
using either remove or remove_all.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 drivers/md/dm-ima.c   | 120 ++++++++++++++++++++++++++++++++++++++++++
 drivers/md/dm-ima.h   |   1 +
 drivers/md/dm-ioctl.c |   3 ++
 3 files changed, 124 insertions(+)

diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 1c545717adf4..47eca432a21a 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -453,10 +453,130 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap)
 	kfree(device_table_data);
 }
 
+/*
+ * Measure IMA data on remove.
+ */
+void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
+{
+	char *device_table_data, *dev_name = NULL, *dev_uuid = NULL, *capacity_str = NULL;
+	char active_table_str[] = "active_table_hash=";
+	char inactive_table_str[] = "inactive_table_hash=";
+	char device_active_str[] = "device_active_metadata=";
+	char device_inactive_str[] = "device_inactive_metadata=";
+	char remove_all_str[] = "remove_all=";
+	unsigned int active_table_len = strlen(active_table_str);
+	unsigned int inactive_table_len = strlen(inactive_table_str);
+	unsigned int device_active_len = strlen(device_active_str);
+	unsigned int device_inactive_len = strlen(device_inactive_str);
+	unsigned int remove_all_len = strlen(remove_all_str);
+	unsigned int capacity_len = 0;
+	unsigned int l = 0;
+	bool noio = true;
+	int r;
+
+	device_table_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN*2, GFP_KERNEL, noio);
+	if (!device_table_data)
+		goto exit;
+
+	r = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
+	if (r) {
+		kfree(device_table_data);
+		goto exit;
+	}
+
+	if (md->ima.active_table.device_metadata) {
+		memcpy(device_table_data + l, device_active_str, device_active_len);
+		l += device_active_len;
+
+		memcpy(device_table_data + l, md->ima.active_table.device_metadata,
+		       md->ima.active_table.device_metadata_len);
+		l += md->ima.active_table.device_metadata_len;
+	}
+
+	if (md->ima.inactive_table.device_metadata) {
+		memcpy(device_table_data + l, device_inactive_str, device_inactive_len);
+		l += device_inactive_len;
+
+		memcpy(device_table_data + l, md->ima.inactive_table.device_metadata,
+		       md->ima.inactive_table.device_metadata_len);
+		l += md->ima.inactive_table.device_metadata_len;
+	}
+
+	if (md->ima.active_table.hash) {
+		memcpy(device_table_data + l, active_table_str, active_table_len);
+		l += active_table_len;
+
+		memcpy(device_table_data + l, md->ima.active_table.hash,
+			   md->ima.active_table.hash_len);
+		l += md->ima.active_table.hash_len;
+
+		memcpy(device_table_data + l, ",", 1);
+		l++;
+	}
+
+	if (md->ima.inactive_table.hash) {
+		memcpy(device_table_data + l, inactive_table_str, inactive_table_len);
+		l += inactive_table_len;
+
+		memcpy(device_table_data + l, md->ima.inactive_table.hash,
+		       md->ima.inactive_table.hash_len);
+		l += md->ima.inactive_table.hash_len;
+
+		memcpy(device_table_data + l, ",", 1);
+		l++;
+	}
+	/*
+	 * In case both active and inactive tables, and corresponding
+	 * device metadata is cleared/missing - record the name and uuid
+	 * in IMA measurements.
+	 */
+	if (!l) {
+		if (dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio))
+			goto error;
+
+		scnprintf(device_table_data, DM_IMA_DEVICE_BUF_LEN,
+			  "name=%s,uuid=%s;device_remove=no_data;",
+			  dev_name, dev_uuid);
+		l += strlen(device_table_data);
+	}
+
+	memcpy(device_table_data + l, remove_all_str, remove_all_len);
+	l += remove_all_len;
+	memcpy(device_table_data + l, remove_all ? "y;" : "n;", 2);
+	l += 2;
+
+	capacity_len = strlen(capacity_str);
+	memcpy(device_table_data + l, capacity_str, capacity_len);
+	l += capacity_len;
+
+	dm_ima_measure_data("device_remove", device_table_data, l, noio);
+
+error:
+	kfree(device_table_data);
+	kfree(capacity_str);
+exit:
+	kfree(md->ima.active_table.device_metadata);
+
+	if (md->ima.active_table.device_metadata !=
+	    md->ima.inactive_table.device_metadata)
+		kfree(md->ima.inactive_table.device_metadata);
+
+	kfree(md->ima.active_table.hash);
+
+	if (md->ima.active_table.hash != md->ima.inactive_table.hash)
+		kfree(md->ima.inactive_table.hash);
+
+	dm_ima_reset_data(md);
+
+	kfree(dev_name);
+	kfree(dev_uuid);
+}
+
 #else
 void dm_ima_reset_data(struct mapped_device *md) {}
 void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags) {}
 void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap) {}
+void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all) {}
 #endif
 MODULE_AUTHOR("Tushar Sugandhi <tusharsu@linux.microsoft.com>");
 MODULE_DESCRIPTION("Enables IMA measurements for DM targets");
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index cafdcf5064c7..cb7b883ed35b 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -49,4 +49,5 @@ struct dm_ima_measurements {
 void dm_ima_reset_data(struct mapped_device *md);
 void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags);
 void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap);
+void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all);
 #endif /*DM_IMA_H*/
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 11af40f9b9c0..2d4475f6de7d 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -348,6 +348,7 @@ static void dm_hash_remove_all(bool keep_open_devices, bool mark_deferred, bool
 			dm_sync_table(md);
 			dm_table_destroy(t);
 		}
+		dm_ima_measure_on_device_remove(md, true);
 		dm_put(md);
 		if (likely(keep_open_devices))
 			dm_destroy(md);
@@ -982,6 +983,8 @@ static int dev_remove(struct file *filp, struct dm_ioctl *param, size_t param_si
 
 	param->flags &= ~DM_DEFERRED_REMOVE;
 
+	dm_ima_measure_on_device_remove(md, false);
+
 	if (!dm_kobject_uevent(md, KOBJ_REMOVE, param->event_nr))
 		param->flags |= DM_UEVENT_GENERATED_FLAG;
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 4/7] dm: measure data on table clear
  2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
                   ` (2 preceding siblings ...)
  2021-07-13  0:49 ` [PATCH 3/7] dm: measure data on device remove Tushar Sugandhi
@ 2021-07-13  0:49 ` Tushar Sugandhi
  2021-07-13  0:49 ` [PATCH 5/7] dm: measure data on device rename Tushar Sugandhi
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-13  0:49 UTC (permalink / raw)
  To: dm-devel, agk, snitzer; +Cc: zohar, linux-integrity, nramas, tusharsu

For a given block device, an inactive table slot contains the parameters
to configure the device with.  The inactive table can be cleared
multiple times, accidentally or maliciously, which may impact the
functionality of the device, and compromise the system.  Therefore it is
important to measure and log the event when a table is cleared.

Measure device parameters, and table hashes when the inactive table slot
is cleared.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 drivers/md/dm-ima.c   | 93 +++++++++++++++++++++++++++++++++++++++++++
 drivers/md/dm-ima.h   |  2 +
 drivers/md/dm-ioctl.c |  3 ++
 3 files changed, 98 insertions(+)

diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 47eca432a21a..b1e1cf6bb4e7 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -572,11 +572,104 @@ void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all)
 	kfree(dev_uuid);
 }
 
+/*
+ * Measure ima data on table clear.
+ */
+void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map)
+{
+	unsigned int l = 0, capacity_len = 0;
+	char *device_table_data = NULL, *dev_name = NULL, *dev_uuid = NULL, *capacity_str = NULL;
+	char inactive_str[] = "inactive_table_hash=";
+	unsigned int inactive_len = strlen(inactive_str);
+	bool noio = true;
+	int r;
+
+	device_table_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN, GFP_KERNEL, noio);
+	if (!device_table_data)
+		return;
+
+	r = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
+	if (r)
+		goto error1;
+
+	if (md->ima.inactive_table.device_metadata_len &&
+	    md->ima.inactive_table.hash_len) {
+		memcpy(device_table_data + l, md->ima.inactive_table.device_metadata,
+		       md->ima.inactive_table.device_metadata_len);
+		l += md->ima.inactive_table.device_metadata_len;
+
+		memcpy(device_table_data + l, inactive_str, inactive_len);
+		l += inactive_len;
+
+		memcpy(device_table_data + l, md->ima.inactive_table.hash,
+			   md->ima.inactive_table.hash_len);
+
+		l += md->ima.inactive_table.hash_len;
+
+		memcpy(device_table_data + l, ";", 1);
+		l++;
+	}
+
+	if (!l) {
+		if (dm_ima_alloc_and_copy_name_uuid(md, &dev_name, &dev_uuid, noio))
+			goto error2;
+
+		scnprintf(device_table_data, DM_IMA_DEVICE_BUF_LEN,
+			  "name=%s,uuid=%s;table_clear=no_data;", dev_name, dev_uuid);
+		l += strlen(device_table_data);
+	}
+
+	capacity_len = strlen(capacity_str);
+	memcpy(device_table_data + l, capacity_str, capacity_len);
+	l += capacity_len;
+
+	dm_ima_measure_data("table_clear", device_table_data, l, noio);
+
+	if (new_map) {
+		if (md->ima.inactive_table.hash &&
+		    md->ima.inactive_table.hash != md->ima.active_table.hash)
+			kfree(md->ima.inactive_table.hash);
+
+		md->ima.inactive_table.hash = NULL;
+		md->ima.inactive_table.hash_len = 0;
+
+		if (md->ima.inactive_table.device_metadata &&
+		    md->ima.inactive_table.device_metadata != md->ima.active_table.device_metadata)
+			kfree(md->ima.inactive_table.device_metadata);
+
+		md->ima.inactive_table.device_metadata = NULL;
+		md->ima.inactive_table.device_metadata_len = 0;
+		md->ima.inactive_table.num_targets = 0;
+
+		if (md->ima.active_table.hash) {
+			md->ima.inactive_table.hash = md->ima.active_table.hash;
+			md->ima.inactive_table.hash_len = md->ima.active_table.hash_len;
+		}
+
+		if (md->ima.active_table.device_metadata) {
+			md->ima.inactive_table.device_metadata =
+				md->ima.active_table.device_metadata;
+			md->ima.inactive_table.device_metadata_len =
+				md->ima.active_table.device_metadata_len;
+			md->ima.inactive_table.num_targets =
+				md->ima.active_table.num_targets;
+		}
+	}
+
+	kfree(dev_name);
+	kfree(dev_uuid);
+error2:
+	kfree(capacity_str);
+error1:
+	kfree(device_table_data);
+}
+
 #else
 void dm_ima_reset_data(struct mapped_device *md) {}
 void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags) {}
 void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap) {}
 void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all) {}
+void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map) {}
 #endif
 MODULE_AUTHOR("Tushar Sugandhi <tusharsu@linux.microsoft.com>");
 MODULE_DESCRIPTION("Enables IMA measurements for DM targets");
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index cb7b883ed35b..a17ae953dc67 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -50,4 +50,6 @@ void dm_ima_reset_data(struct mapped_device *md);
 void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags);
 void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap);
 void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all);
+void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map);
+
 #endif /*DM_IMA_H*/
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 2d4475f6de7d..b07c19037c7c 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1508,6 +1508,7 @@ static int table_clear(struct file *filp, struct dm_ioctl *param, size_t param_s
 	struct hash_cell *hc;
 	struct mapped_device *md;
 	struct dm_table *old_map = NULL;
+	bool has_new_map = false;
 
 	down_write(&_hash_lock);
 
@@ -1521,6 +1522,7 @@ static int table_clear(struct file *filp, struct dm_ioctl *param, size_t param_s
 	if (hc->new_map) {
 		old_map = hc->new_map;
 		hc->new_map = NULL;
+		has_new_map = true;
 	}
 
 	param->flags &= ~DM_INACTIVE_PRESENT_FLAG;
@@ -1532,6 +1534,7 @@ static int table_clear(struct file *filp, struct dm_ioctl *param, size_t param_s
 		dm_sync_table(md);
 		dm_table_destroy(old_map);
 	}
+	dm_ima_measure_on_table_clear(md, has_new_map);
 	dm_put(md);
 
 	return 0;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 5/7] dm: measure data on device rename
  2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
                   ` (3 preceding siblings ...)
  2021-07-13  0:49 ` [PATCH 4/7] dm: measure data on table clear Tushar Sugandhi
@ 2021-07-13  0:49 ` Tushar Sugandhi
  2021-07-13  0:49 ` [PATCH 6/7] dm: update target specific status functions to measure data Tushar Sugandhi
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-13  0:49 UTC (permalink / raw)
  To: dm-devel, agk, snitzer; +Cc: zohar, linux-integrity, nramas, tusharsu

A given block device is identified by it's name and UUID.  However, both
these parameters can be renamed.  For an external attestation service to
correctly attest a given device, it needs to keep track of these rename
events.

Update the device data with the new values for IMA measurements.  Measure
both old and new device name/UUID parameters in the same IMA measurement
event, so that the old and the new values can be connected later.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 drivers/md/dm-ima.c   | 49 +++++++++++++++++++++++++++++++++++++++++++
 drivers/md/dm-ima.h   |  1 +
 drivers/md/dm-ioctl.c |  3 +++
 3 files changed, 53 insertions(+)

diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index b1e1cf6bb4e7..36f99848825a 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -664,12 +664,61 @@ void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map)
 	kfree(device_table_data);
 }
 
+/*
+ * Measure IMA data on device rename.
+ */
+void dm_ima_measure_on_device_rename(struct mapped_device *md)
+{
+	char *old_device_data = NULL, *new_device_data = NULL, *combined_device_data = NULL;
+	char *new_dev_name = NULL, *new_dev_uuid = NULL, *capacity_str = NULL;
+	bool noio = true;
+	int r;
+
+	if (dm_ima_alloc_and_copy_device_data(md, &new_device_data,
+					      md->ima.active_table.num_targets, noio))
+		return;
+
+	if (dm_ima_alloc_and_copy_name_uuid(md, &new_dev_name, &new_dev_uuid, noio))
+		goto error;
+
+	combined_device_data = dm_ima_alloc(DM_IMA_DEVICE_BUF_LEN * 2, GFP_KERNEL, noio);
+	if (!combined_device_data)
+		goto error;
+
+	r = dm_ima_alloc_and_copy_capacity_str(md, &capacity_str, noio);
+	if (r)
+		goto error;
+
+	old_device_data = md->ima.active_table.device_metadata;
+
+	md->ima.active_table.device_metadata = new_device_data;
+	md->ima.active_table.device_metadata_len = strlen(new_device_data);
+
+	scnprintf(combined_device_data, DM_IMA_DEVICE_BUF_LEN * 2, "%snew_name=%s,new_uuid=%s;%s",
+		  old_device_data, new_dev_name, new_dev_uuid, capacity_str);
+
+	dm_ima_measure_data("device_rename", combined_device_data, strlen(combined_device_data),
+			    noio);
+
+	goto exit;
+
+error:
+	kfree(new_device_data);
+exit:
+	kfree(capacity_str);
+	kfree(combined_device_data);
+	kfree(old_device_data);
+	kfree(new_dev_name);
+	kfree(new_dev_uuid);
+}
+
 #else
 void dm_ima_reset_data(struct mapped_device *md) {}
 void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_flags) {}
 void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap) {}
 void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all) {}
 void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map) {}
+void dm_ima_measure_on_device_rename(struct mapped_device *md) {}
 #endif
 MODULE_AUTHOR("Tushar Sugandhi <tusharsu@linux.microsoft.com>");
 MODULE_DESCRIPTION("Enables IMA measurements for DM targets");
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index a17ae953dc67..b1cf014d538d 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -51,5 +51,6 @@ void dm_ima_measure_on_table_load(struct dm_table *table, unsigned int status_fl
 void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap);
 void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all);
 void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map);
+void dm_ima_measure_on_device_rename(struct mapped_device *md);
 
 #endif /*DM_IMA_H*/
diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index b07c19037c7c..e45f6c6ef84a 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -485,6 +485,9 @@ static struct mapped_device *dm_hash_rename(struct dm_ioctl *param,
 		param->flags |= DM_UEVENT_GENERATED_FLAG;
 
 	md = hc->md;
+
+	dm_ima_measure_on_device_rename(md);
+
 	up_write(&_hash_lock);
 	kfree(old_name);
 
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 6/7] dm: update target specific status functions to measure data
  2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
                   ` (4 preceding siblings ...)
  2021-07-13  0:49 ` [PATCH 5/7] dm: measure data on device rename Tushar Sugandhi
@ 2021-07-13  0:49 ` Tushar Sugandhi
  2021-07-13  1:06   ` Alasdair G Kergon
  2021-07-13  0:49 ` [PATCH 7/7] dm: add documentation for IMA measurement support Tushar Sugandhi
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-13  0:49 UTC (permalink / raw)
  To: dm-devel, agk, snitzer; +Cc: zohar, linux-integrity, nramas, tusharsu

For device mapper targets to take advantage of IMA's measurement
capabilities, the status functions for the individual targets need to be
updated to handle the status_type_t case for value STATUSTYPE_IMA.

Update status functions for the following target types, to log their
respective attributes to be measured using IMA.
 01. cache
 02. crypt
 03. integrity
 04. linear
 05. mirror
 06. multipath
 07. raid
 08. snapshot
 09. striped
 10. verity 

For rest of the targets, handle the STATUSTYPE_IMA case by setting the 
measurement buffer to NULL.

For IMA to measure the data on a given system, the IMA policy on the
system needs to be updated to have the following line, and the system 
needs to be restarted for the measurements to take effect.

/etc/ima/ima-policy
 measure func=CRITICAL_DATA label=device-mapper template=ima-buf

The measurements will be reflected in the IMA logs, which are located at:

/sys/kernel/security/integrity/ima/ascii_runtime_measurements
/sys/kernel/security/integrity/ima/binary_runtime_measurements

These IMA logs can later be consumed by various attestation clients
running on the system, and send them to external services for attesting
the system.

The DM target data measured by IMA subsystem can alternatively
be queried from userspace by setting DM_IMA_MEASUREMENT_FLAG with
DM_TABLE_STATUS_CMD.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 drivers/md/dm-cache-target.c               | 24 ++++++++++++
 drivers/md/dm-clone-target.c               |  7 ++++
 drivers/md/dm-crypt.c                      | 29 ++++++++++++++
 drivers/md/dm-delay.c                      |  4 ++
 drivers/md/dm-dust.c                       |  4 ++
 drivers/md/dm-ebs-target.c                 |  3 ++
 drivers/md/dm-era-target.c                 |  4 ++
 drivers/md/dm-flakey.c                     |  4 ++
 drivers/md/dm-integrity.c                  | 24 ++++++++++++
 drivers/md/dm-linear.c                     |  8 ++++
 drivers/md/dm-log-userspace-base.c         |  3 ++
 drivers/md/dm-log-writes.c                 |  4 ++
 drivers/md/dm-log.c                        | 10 +++++
 drivers/md/dm-mpath.c                      | 29 ++++++++++++++
 drivers/md/dm-ps-historical-service-time.c |  3 ++
 drivers/md/dm-ps-io-affinity.c             |  3 ++
 drivers/md/dm-ps-queue-length.c            |  3 ++
 drivers/md/dm-ps-round-robin.c             |  4 ++
 drivers/md/dm-ps-service-time.c            |  3 ++
 drivers/md/dm-raid.c                       | 39 +++++++++++++++++++
 drivers/md/dm-raid1.c                      | 18 +++++++++
 drivers/md/dm-snap-persistent.c            |  4 ++
 drivers/md/dm-snap-transient.c             |  4 ++
 drivers/md/dm-snap.c                       | 13 +++++++
 drivers/md/dm-stripe.c                     | 16 ++++++++
 drivers/md/dm-switch.c                     |  4 ++
 drivers/md/dm-thin.c                       |  8 ++++
 drivers/md/dm-unstripe.c                   |  4 ++
 drivers/md/dm-verity-target.c              | 45 ++++++++++++++++++++++
 drivers/md/dm-writecache.c                 |  3 ++
 drivers/md/dm-zoned-target.c               |  3 ++
 include/linux/device-mapper.h              |  4 ++
 32 files changed, 338 insertions(+)

diff --git a/drivers/md/dm-cache-target.c b/drivers/md/dm-cache-target.c
index 6ab01ff25747..f1299e481d9d 100644
--- a/drivers/md/dm-cache-target.c
+++ b/drivers/md/dm-cache-target.c
@@ -3192,6 +3192,30 @@ static void cache_status(struct dm_target *ti, status_type_t type,
 			DMEMIT(" %s", cache->ctr_args[i]);
 		if (cache->nr_ctr_args)
 			DMEMIT(" %s", cache->ctr_args[cache->nr_ctr_args - 1]);
+		break;
+
+	case STATUSTYPE_IMA:
+		DMEMIT_NAME_VERSION(ti->type);
+
+		if (get_cache_mode(cache) == CM_FAIL)
+			DMEMIT("metadata_mode=fail,");
+		else if (get_cache_mode(cache) == CM_READ_ONLY)
+			DMEMIT("metadata_mode=ro,");
+		else
+			DMEMIT("metadata_mode=rw,");
+
+		format_dev_t(buf, cache->metadata_dev->bdev->bd_dev);
+		DMEMIT("cache_metadata_device=%s,", buf);
+		format_dev_t(buf, cache->cache_dev->bdev->bd_dev);
+		DMEMIT("cache_device=%s,", buf);
+		format_dev_t(buf, cache->origin_dev->bdev->bd_dev);
+		DMEMIT("cache_origin_device=%s,", buf);
+		DMEMIT("writethrough=%c,", writethrough_mode(cache) ? 'y' : 'n');
+		DMEMIT("writeback=%c,", writeback_mode(cache) ? 'y' : 'n');
+		DMEMIT("passthrough=%c,", passthrough_mode(cache) ? 'y' : 'n');
+		DMEMIT("metadata2=%c,", cache->features.metadata_version == 2 ? 'y' : 'n');
+		DMEMIT("no_discard_passdown=%c;", cache->features.discard_passdown ? 'n' : 'y');
+		break;
 	}
 
 	return;
diff --git a/drivers/md/dm-clone-target.c b/drivers/md/dm-clone-target.c
index a90bdf9b2ca6..506d4d70b8b9 100644
--- a/drivers/md/dm-clone-target.c
+++ b/drivers/md/dm-clone-target.c
@@ -1499,6 +1499,13 @@ static void clone_status(struct dm_target *ti, status_type_t type,
 
 		for (i = 0; i < clone->nr_ctr_args; i++)
 			DMEMIT(" %s", clone->ctr_args[i]);
+
+		break;
+
+	case STATUSTYPE_IMA:
+		*result = '\0';
+
+		break;
 	}
 
 	return;
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index b0ab080f2567..b7b72f31a7c6 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -3473,6 +3473,35 @@ static void crypt_status(struct dm_target *ti, status_type_t type,
 				DMEMIT(" iv_large_sectors");
 		}
 
+		break;
+
+	case STATUSTYPE_IMA:
+		DMEMIT_NAME_VERSION(ti->type);
+
+		DMEMIT("allow_discards=%c,", ti->num_discard_bios ? 'y' : 'n');
+		DMEMIT("same_cpu=%c,", test_bit(DM_CRYPT_SAME_CPU, &cc->flags) ? 'y' : 'n');
+		DMEMIT("submit_from_crypt_cpus=%c,", test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags) ?
+		       'y' : 'n');
+		DMEMIT("no_read_workqueue=%c,", test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags) ?
+		       'y' : 'n');
+		DMEMIT("no_write_workqueue=%c,", test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags) ?
+		       'y' : 'n');
+		DMEMIT("iv_large_sectors=%c", test_bit(CRYPT_IV_LARGE_SECTORS, &cc->cipher_flags) ?
+		       'y' : 'n');
+
+		if (cc->on_disk_tag_size)
+			DMEMIT(",integrity_tag_size=%u,cipher_auth=%s",
+				cc->on_disk_tag_size, cc->cipher_auth);
+		if (cc->sector_size != (1 << SECTOR_SHIFT))
+			DMEMIT(",sector_size=%d", cc->sector_size);
+		if (cc->cipher_string)
+			DMEMIT(",cipher_string=%s", cc->cipher_string);
+
+		DMEMIT(",key_size=%u", cc->key_size);
+		DMEMIT(",key_parts=%u", cc->key_parts);
+		DMEMIT(",key_extra_size=%u", cc->key_extra_size);
+		DMEMIT(",key_mac_size=%u;", cc->key_mac_size);
+
 		break;
 	}
 }
diff --git a/drivers/md/dm-delay.c b/drivers/md/dm-delay.c
index 2628a832787b..59e51d285b0e 100644
--- a/drivers/md/dm-delay.c
+++ b/drivers/md/dm-delay.c
@@ -326,6 +326,10 @@ static void delay_status(struct dm_target *ti, status_type_t type,
 			DMEMIT_DELAY_CLASS(&dc->flush);
 		}
 		break;
+
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-dust.c b/drivers/md/dm-dust.c
index cbe1058ee589..3163e2b1418e 100644
--- a/drivers/md/dm-dust.c
+++ b/drivers/md/dm-dust.c
@@ -527,6 +527,10 @@ static void dust_status(struct dm_target *ti, status_type_t type,
 		DMEMIT("%s %llu %u", dd->dev->name,
 		       (unsigned long long)dd->start, dd->blksz);
 		break;
+
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-ebs-target.c b/drivers/md/dm-ebs-target.c
index 71475a2410be..ec8426611cc6 100644
--- a/drivers/md/dm-ebs-target.c
+++ b/drivers/md/dm-ebs-target.c
@@ -401,6 +401,9 @@ static void ebs_status(struct dm_target *ti, status_type_t type,
 		snprintf(result, maxlen, ec->u_bs_set ? "%s %llu %u %u" : "%s %llu %u",
 			 ec->dev->name, (unsigned long long) ec->start, ec->e_bs, ec->u_bs);
 		break;
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-era-target.c b/drivers/md/dm-era-target.c
index d9ac7372108c..08beb311e2e0 100644
--- a/drivers/md/dm-era-target.c
+++ b/drivers/md/dm-era-target.c
@@ -1640,6 +1640,10 @@ static void era_status(struct dm_target *ti, status_type_t type,
 		format_dev_t(buf, era->origin_dev->bdev->bd_dev);
 		DMEMIT("%s %u", buf, era->sectors_per_block);
 		break;
+
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 
 	return;
diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
index b7fee9936f05..8720e1a4873b 100644
--- a/drivers/md/dm-flakey.c
+++ b/drivers/md/dm-flakey.c
@@ -440,6 +440,10 @@ static void flakey_status(struct dm_target *ti, status_type_t type,
 			       fc->corrupt_bio_value, fc->corrupt_bio_flags);
 
 		break;
+
+	case STATUSTYPE_IMA:
+		result[0] = '\0';
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 20f2510db1f6..59f5618143fc 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -3306,6 +3306,30 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type,
 		EMIT_ALG(journal_mac_alg, "journal_mac");
 		break;
 	}
+	case STATUSTYPE_IMA:
+		DMEMIT_NAME_VERSION(ti->type);
+		DMEMIT("dev_name=%s,start=%llu,tag_size=%u,mode=%c",
+			ic->dev->name, ic->start, ic->tag_size, ic->mode);
+
+		if (ic->meta_dev)
+			DMEMIT(",meta_device=%s", ic->meta_dev->name);
+		if (ic->sectors_per_block != 1)
+			DMEMIT(",block_size=%u", ic->sectors_per_block << SECTOR_SHIFT);
+
+		DMEMIT(",recalculate=%c", (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) ?
+		       'y' : 'n');
+		DMEMIT(",allow_discards=%c", ic->discard ? 'y' : 'n');
+		DMEMIT(",fix_padding=%c",
+		       ((ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_PADDING)) != 0) ? 'y' : 'n');
+		DMEMIT(",fix_hmac=%c",
+		       ((ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_HMAC)) != 0) ? 'y' : 'n');
+		DMEMIT(",legacy_recalculate=%c", ic->legacy_recalculate ? 'y' : 'n');
+
+		DMEMIT(",journal_sectors=%u", ic->initial_sectors - SB_SECTORS);
+		DMEMIT(",interleave_sectors=%u", 1U << ic->sb->log2_interleave_sectors);
+		DMEMIT(",buffer_sectors=%u", 1U << ic->log2_buffer_sectors);
+		DMEMIT(",mode=%c;", ic->mode);
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
index 92db0f5e7f28..3c35370f33e1 100644
--- a/drivers/md/dm-linear.c
+++ b/drivers/md/dm-linear.c
@@ -106,6 +106,7 @@ static void linear_status(struct dm_target *ti, status_type_t type,
 			  unsigned status_flags, char *result, unsigned maxlen)
 {
 	struct linear_c *lc = (struct linear_c *) ti->private;
+	size_t sz = 0;
 
 	switch (type) {
 	case STATUSTYPE_INFO:
@@ -116,6 +117,13 @@ static void linear_status(struct dm_target *ti, status_type_t type,
 		snprintf(result, maxlen, "%s %llu", lc->dev->name,
 				(unsigned long long)lc->start);
 		break;
+
+	case STATUSTYPE_IMA:
+		DMEMIT_NAME_VERSION(ti->type);
+
+		DMEMIT("device_name=%s,start=%llu;", lc->dev->name,
+		       (unsigned long long)lc->start);
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-log-userspace-base.c b/drivers/md/dm-log-userspace-base.c
index 52090bee17c2..9ab93ebea889 100644
--- a/drivers/md/dm-log-userspace-base.c
+++ b/drivers/md/dm-log-userspace-base.c
@@ -820,6 +820,9 @@ static int userspace_status(struct dm_dirty_log *log, status_type_t status_type,
 			DMEMIT("integrated_flush ");
 		DMEMIT("%s ", table_args);
 		break;
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 	return (r) ? 0 : (int)sz;
 }
diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c
index 57882654ffee..d93a4db23512 100644
--- a/drivers/md/dm-log-writes.c
+++ b/drivers/md/dm-log-writes.c
@@ -834,6 +834,10 @@ static void log_writes_status(struct dm_target *ti, status_type_t type,
 	case STATUSTYPE_TABLE:
 		DMEMIT("%s %s", lc->dev->name, lc->logdev->name);
 		break;
+
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-log.c b/drivers/md/dm-log.c
index 33e71ea6cc14..1ecf75ef276a 100644
--- a/drivers/md/dm-log.c
+++ b/drivers/md/dm-log.c
@@ -793,6 +793,11 @@ static int core_status(struct dm_dirty_log *log, status_type_t status,
 		DMEMIT("%s %u %u ", log->type->name,
 		       lc->sync == DEFAULTSYNC ? 1 : 2, lc->region_size);
 		DMEMIT_SYNC;
+		break;
+
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 
 	return sz;
@@ -817,6 +822,11 @@ static int disk_status(struct dm_dirty_log *log, status_type_t status,
 		       lc->sync == DEFAULTSYNC ? 2 : 3, lc->log_dev->name,
 		       lc->region_size);
 		DMEMIT_SYNC;
+		break;
+
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 
 	return sz;
diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index bced42f082b0..408638395d9b 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1904,6 +1904,35 @@ static void multipath_status(struct dm_target *ti, status_type_t type,
 			}
 		}
 		break;
+
+	case STATUSTYPE_IMA:
+		DMEMIT_NAME_VERSION(ti->type);
+
+		list_for_each_entry(pg, &m->priority_groups, list) {
+			if (pg->bypassed)
+				state = 'D';	/* Disabled */
+			else if (pg == m->current_pg)
+				state = 'A';	/* Currently Active */
+			else
+				state = 'E';	/* Enabled */
+
+			DMEMIT("state=%c", state);
+
+			list_for_each_entry(p, &pg->pgpaths, list) {
+				DMEMIT(",path_name=%s,is_active=%c,fail_count=%u",
+					p->path.dev->name,
+					p->is_active ? 'A' : 'F',
+					p->fail_count);
+				if (pg->ps.type->status) {
+					DMEMIT(",path_selector_status=");
+					sz += pg->ps.type->status(&pg->ps,
+					      &p->path, type, result + sz,
+					      maxlen - sz);
+				}
+			}
+			DMEMIT(";");
+		}
+		break;
 	}
 
 	spin_unlock_irqrestore(&m->lock, flags);
diff --git a/drivers/md/dm-ps-historical-service-time.c b/drivers/md/dm-ps-historical-service-time.c
index 186f91e2752c..1856a1b125cc 100644
--- a/drivers/md/dm-ps-historical-service-time.c
+++ b/drivers/md/dm-ps-historical-service-time.c
@@ -255,6 +255,9 @@ static int hst_status(struct path_selector *ps, struct dm_path *path,
 		case STATUSTYPE_TABLE:
 			DMEMIT("0 ");
 			break;
+		case STATUSTYPE_IMA:
+			*result = '\0';
+			break;
 		}
 	}
 
diff --git a/drivers/md/dm-ps-io-affinity.c b/drivers/md/dm-ps-io-affinity.c
index 077655cd4fae..8eaa7b726387 100644
--- a/drivers/md/dm-ps-io-affinity.c
+++ b/drivers/md/dm-ps-io-affinity.c
@@ -171,6 +171,9 @@ static int ioa_status(struct path_selector *ps, struct dm_path *path,
 		pi = path->pscontext;
 		DMEMIT("%*pb ", cpumask_pr_args(pi->cpumask));
 		break;
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 
 	return sz;
diff --git a/drivers/md/dm-ps-queue-length.c b/drivers/md/dm-ps-queue-length.c
index 5fd018d18418..cef70657bbbc 100644
--- a/drivers/md/dm-ps-queue-length.c
+++ b/drivers/md/dm-ps-queue-length.c
@@ -102,6 +102,9 @@ static int ql_status(struct path_selector *ps, struct dm_path *path,
 		case STATUSTYPE_TABLE:
 			DMEMIT("%u ", pi->repeat_count);
 			break;
+		case STATUSTYPE_IMA:
+			*result = '\0';
+			break;
 		}
 	}
 
diff --git a/drivers/md/dm-ps-round-robin.c b/drivers/md/dm-ps-round-robin.c
index bdbb7e6e8212..27f44c5fa04e 100644
--- a/drivers/md/dm-ps-round-robin.c
+++ b/drivers/md/dm-ps-round-robin.c
@@ -100,6 +100,10 @@ static int rr_status(struct path_selector *ps, struct dm_path *path,
 			pi = path->pscontext;
 			DMEMIT("%u ", pi->repeat_count);
 			break;
+
+		case STATUSTYPE_IMA:
+			*result = '\0';
+			break;
 		}
 	}
 
diff --git a/drivers/md/dm-ps-service-time.c b/drivers/md/dm-ps-service-time.c
index 9cfda665e9eb..3ec9c33265c5 100644
--- a/drivers/md/dm-ps-service-time.c
+++ b/drivers/md/dm-ps-service-time.c
@@ -99,6 +99,9 @@ static int st_status(struct path_selector *ps, struct dm_path *path,
 			DMEMIT("%u %u ", pi->repeat_count,
 			       pi->relative_throughput);
 			break;
+		case STATUSTYPE_IMA:
+			result[0] = '\0';
+			break;
 		}
 	}
 
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index bf4a467fc73a..71fffafe3afa 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -3671,6 +3671,45 @@ static void raid_status(struct dm_target *ti, status_type_t type,
 		for (i = 0; i < rs->raid_disks; i++)
 			DMEMIT(" %s %s", __get_dev_name(rs->dev[i].meta_dev),
 					 __get_dev_name(rs->dev[i].data_dev));
+		break;
+
+	case STATUSTYPE_IMA:
+		rt = get_raid_type_by_ll(mddev->new_level, mddev->new_layout);
+		if (!rt)
+			return;
+
+		DMEMIT_NAME_VERSION(ti->type);
+
+		DMEMIT("raid_type=%s,raid_disks=%d,", rt->name, mddev->raid_disks);
+
+		/* Access most recent mddev properties for status output */
+		smp_rmb();
+		state = decipher_sync_action(mddev, recovery);
+		DMEMIT("raid_state=%s", sync_str(state));
+
+		for (i = 0; i < rs->raid_disks; i++) {
+			DMEMIT(",raid_device_%d_status=", i);
+			DMEMIT(__raid_dev_status(rs, &rs->dev[i].rdev));
+		}
+
+		if (rt_is_raid456(rt)) {
+			DMEMIT(",journal_dev_mode=");
+			switch (rs->journal_dev.mode) {
+			case R5C_JOURNAL_MODE_WRITE_THROUGH:
+				DMEMIT("%s",
+				       _raid456_journal_mode[R5C_JOURNAL_MODE_WRITE_THROUGH].param);
+				break;
+			case R5C_JOURNAL_MODE_WRITE_BACK:
+				DMEMIT("%s",
+				       _raid456_journal_mode[R5C_JOURNAL_MODE_WRITE_BACK].param);
+				break;
+			default:
+				DMEMIT("invalid");
+				break;
+			}
+		}
+		DMEMIT(";");
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-raid1.c b/drivers/md/dm-raid1.c
index b0a82f29a2e4..4d042c2abf88 100644
--- a/drivers/md/dm-raid1.c
+++ b/drivers/md/dm-raid1.c
@@ -1435,6 +1435,24 @@ static void mirror_status(struct dm_target *ti, status_type_t type,
 		}
 
 		break;
+
+	case STATUSTYPE_IMA:
+		DMEMIT_NAME_VERSION(ti->type);
+
+		DMEMIT("mirrors=%d,", ms->nr_mirrors);
+		for (m = 0; m < ms->nr_mirrors; m++) {
+			DMEMIT("mirror_device_%d=%s,", m, ms->mirror[m].dev->name);
+			DMEMIT("mirror_device_%d_status=%c,",
+			       m, device_status_char(&(ms->mirror[m])));
+		}
+
+		DMEMIT("handle_errors=%c,", errors_handled(ms) ? 'y' : 'n');
+		DMEMIT("keep_log=%c,", keep_log(ms) ? 'y' : 'n');
+
+		DMEMIT("log_type_status=");
+		sz += log->type->status(log, type, result+sz, maxlen-sz);
+		DMEMIT(";");
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-snap-persistent.c b/drivers/md/dm-snap-persistent.c
index 9ab4bf651ca9..3bb5cff5d6fc 100644
--- a/drivers/md/dm-snap-persistent.c
+++ b/drivers/md/dm-snap-persistent.c
@@ -908,6 +908,10 @@ static unsigned persistent_status(struct dm_exception_store *store,
 	case STATUSTYPE_TABLE:
 		DMEMIT(" %s %llu", store->userspace_supports_overflow ? "PO" : "P",
 		       (unsigned long long)store->chunk_size);
+		break;
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 
 	return sz;
diff --git a/drivers/md/dm-snap-transient.c b/drivers/md/dm-snap-transient.c
index 4d50a12cf00c..0e0ae4c36b37 100644
--- a/drivers/md/dm-snap-transient.c
+++ b/drivers/md/dm-snap-transient.c
@@ -95,6 +95,10 @@ static unsigned transient_status(struct dm_exception_store *store,
 		break;
 	case STATUSTYPE_TABLE:
 		DMEMIT(" N %llu", (unsigned long long)store->chunk_size);
+		break;
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 
 	return sz;
diff --git a/drivers/md/dm-snap.c b/drivers/md/dm-snap.c
index 751ec5ea1dbb..a407dbba0b5a 100644
--- a/drivers/md/dm-snap.c
+++ b/drivers/md/dm-snap.c
@@ -2390,6 +2390,16 @@ static void snapshot_status(struct dm_target *ti, status_type_t type,
 				DMEMIT(" discard_passdown_origin");
 		}
 		break;
+
+	case STATUSTYPE_IMA:
+		DMEMIT_NAME_VERSION(ti->type);
+		DMEMIT("snap_origin_name=%s,", snap->origin->name);
+		DMEMIT("snap_cow_name=%s,", snap->cow->name);
+		DMEMIT("snap_valid=%c,", snap->valid ? 'y' : 'n');
+		DMEMIT("snap_merge_failed=%c,", snap->merge_failed ? 'y' : 'n');
+		DMEMIT("snapshot_overflowed=%c;", snap->snapshot_overflowed ? 'y' : 'n');
+		break;
+
 	}
 }
 
@@ -2734,6 +2744,9 @@ static void origin_status(struct dm_target *ti, status_type_t type,
 	case STATUSTYPE_TABLE:
 		snprintf(result, maxlen, "%s", o->dev->name);
 		break;
+	case STATUSTYPE_IMA:
+		result[0] = '\0';
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
index df359d33cda8..82f3593f6a0e 100644
--- a/drivers/md/dm-stripe.c
+++ b/drivers/md/dm-stripe.c
@@ -428,6 +428,22 @@ static void stripe_status(struct dm_target *ti, status_type_t type,
 			DMEMIT(" %s %llu", sc->stripe[i].dev->name,
 			    (unsigned long long)sc->stripe[i].physical_start);
 		break;
+
+	case STATUSTYPE_IMA:
+		DMEMIT_NAME_VERSION(ti->type);
+
+		DMEMIT("stripes=%d,chunk_size=%llu", sc->stripes,
+		       (unsigned long long)sc->chunk_size);
+
+		for (i = 0; i < sc->stripes; i++) {
+			DMEMIT(",stripe_%d_device_name=%s", i, sc->stripe[i].dev->name);
+			DMEMIT(",stripe_%d_physical_start=%llu", i,
+			       (unsigned long long)sc->stripe[i].physical_start);
+			DMEMIT(",stripe_%d_status=%c", i,
+			       atomic_read(&(sc->stripe[i].error_count)) ? 'D' : 'A');
+		}
+		DMEMIT(";");
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c
index 262e2b0fd975..028a92ff6d57 100644
--- a/drivers/md/dm-switch.c
+++ b/drivers/md/dm-switch.c
@@ -504,6 +504,10 @@ static void switch_status(struct dm_target *ti, status_type_t type,
 			DMEMIT(" %s %llu", sctx->path_list[path_nr].dmdev->name,
 			       (unsigned long long)sctx->path_list[path_nr].start);
 		break;
+
+	case STATUSTYPE_IMA:
+		result[0] = '\0';
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index 985baee3a678..4c67b77c23c1 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -4012,6 +4012,10 @@ static void pool_status(struct dm_target *ti, status_type_t type,
 		       (unsigned long long)pt->low_water_blocks);
 		emit_flags(&pt->requested_pf, result, sz, maxlen);
 		break;
+
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 	return;
 
@@ -4423,6 +4427,10 @@ static void thin_status(struct dm_target *ti, status_type_t type,
 			if (tc->origin_dev)
 				DMEMIT(" %s", format_dev_t(buf, tc->origin_dev->bdev->bd_dev));
 			break;
+
+		case STATUSTYPE_IMA:
+			*result = '\0';
+			break;
 		}
 	}
 
diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
index 7357c1bd5863..fdc8921e5c19 100644
--- a/drivers/md/dm-unstripe.c
+++ b/drivers/md/dm-unstripe.c
@@ -156,6 +156,10 @@ static void unstripe_status(struct dm_target *ti, status_type_t type,
 		       uc->stripes, (unsigned long long)uc->chunk_size, uc->unstripe,
 		       uc->dev->name, (unsigned long long)uc->physical_start);
 		break;
+
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index d3e76aefc1a6..0cae12944352 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -772,6 +772,51 @@ static void verity_status(struct dm_target *ti, status_type_t type,
 			DMEMIT(" " DM_VERITY_ROOT_HASH_VERIFICATION_OPT_SIG_KEY
 				" %s", v->signature_key_desc);
 		break;
+
+	case STATUSTYPE_IMA:
+		DMEMIT_NAME_VERSION(ti->type);
+
+		DMEMIT("hash_failed=%c,", v->hash_failed ? 'C' : 'V');
+
+		DMEMIT("verity_version=%u,", v->version);
+		DMEMIT("data_device_name=%s,", v->data_dev->name);
+		DMEMIT("hash_device_name=%s,", v->hash_dev->name);
+		DMEMIT("verity_algorithm=%s,", v->alg_name);
+
+		DMEMIT("root_digest=");
+		for (x = 0; x < v->digest_size; x++)
+			DMEMIT("%02x", v->root_digest[x]);
+		DMEMIT(",");
+
+		DMEMIT("salt=");
+		if (!v->salt_size)
+			DMEMIT("-");
+		else
+			for (x = 0; x < v->salt_size; x++)
+				DMEMIT("%02x", v->salt[x]);
+		DMEMIT(",");
+
+		DMEMIT("ignore_zero_blocks=%c,", v->zero_digest ? 'y' : 'n');
+		DMEMIT("check_at_most_once=%c", v->validated_blocks ? 'y' : 'n');
+
+		if (v->mode != DM_VERITY_MODE_EIO) {
+			DMEMIT(",verity_mode=");
+			switch (v->mode) {
+			case DM_VERITY_MODE_LOGGING:
+				DMEMIT(DM_VERITY_OPT_LOGGING);
+				break;
+			case DM_VERITY_MODE_RESTART:
+				DMEMIT(DM_VERITY_OPT_RESTART);
+				break;
+			case DM_VERITY_MODE_PANIC:
+				DMEMIT(DM_VERITY_OPT_PANIC);
+				break;
+			default:
+				DMEMIT("invalid");
+			}
+		}
+		DMEMIT(";");
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
index aecc246ade26..a7516bd7cd04 100644
--- a/drivers/md/dm-writecache.c
+++ b/drivers/md/dm-writecache.c
@@ -2536,6 +2536,9 @@ static void writecache_status(struct dm_target *ti, status_type_t type,
 		if (wc->writeback_fua_set)
 			DMEMIT(" %sfua", wc->writeback_fua ? "" : "no");
 		break;
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 }
 
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 7e88df64d197..ae1bc48c0043 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -1119,6 +1119,9 @@ static void dmz_status(struct dm_target *ti, status_type_t type,
 			DMEMIT(" %s", buf);
 		}
 		break;
+	case STATUSTYPE_IMA:
+		*result = '\0';
+		break;
 	}
 	return;
 }
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 738a7d023650..d4cd9ef94063 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -596,6 +596,10 @@ void dm_destroy_keyslot_manager(struct blk_keyslot_manager *ksm);
 #define DMEMIT(x...) sz += ((sz >= maxlen) ? \
 			  0 : scnprintf(result + sz, maxlen - sz, x))
 
+#define DMEMIT_NAME_VERSION(y) \
+		DMEMIT("target_type_name=%s,target_type_version=%u.%u.%u,", \
+		       (y)->name, (y)->version[0], (y)->version[1], (y)->version[2])
+
 /*
  * Definitions of return values from target end_io function.
  */
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 7/7] dm: add documentation for IMA measurement support
  2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
                   ` (5 preceding siblings ...)
  2021-07-13  0:49 ` [PATCH 6/7] dm: update target specific status functions to measure data Tushar Sugandhi
@ 2021-07-13  0:49 ` Tushar Sugandhi
  2021-07-21  2:33   ` Mimi Zohar
  2021-07-14 11:32 ` [dm-devel] [PATCH 0/7] device mapper target measurements using IMA Thore Sommer
  2021-07-20 21:27 ` Mike Snitzer
  8 siblings, 1 reply; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-13  0:49 UTC (permalink / raw)
  To: dm-devel, agk, snitzer; +Cc: zohar, linux-integrity, nramas, tusharsu

To interpret various DM target measurement data in IMA logs,
a separate documentation page is needed under
Documentation/admin-guide/device-mapper.

Add documentation to help system administrators and attestation
client/server component owners to interpret the measurement
data generated by various DM targets, on various device/table state
changes.

Signed-off-by: Tushar Sugandhi <tusharsu@linux.microsoft.com>
---
 .../admin-guide/device-mapper/dm-ima.rst      | 306 ++++++++++++++++++
 .../admin-guide/device-mapper/index.rst       |   1 +
 2 files changed, 307 insertions(+)
 create mode 100644 Documentation/admin-guide/device-mapper/dm-ima.rst

diff --git a/Documentation/admin-guide/device-mapper/dm-ima.rst b/Documentation/admin-guide/device-mapper/dm-ima.rst
new file mode 100644
index 000000000000..1be2da7c6b6b
--- /dev/null
+++ b/Documentation/admin-guide/device-mapper/dm-ima.rst
@@ -0,0 +1,306 @@
+======
+dm-ima
+======
+
+For a given system, various external services/infrastructure tools
+(including the attestation service) interact with it - both during the
+setup and during rest of the system run-time.  They share sensitive data
+and/or execute critical workload on that system.  The external services
+may want to verify the current run-time state of the relevant kernel
+subsystems before fully trusting the system with business-critical
+data/workload.
+
+Device mapper plays a critical role on a given system by providing
+various important functionalities to the block devices using various
+target types like crypt, verity, integrity etc.  Each of these target
+types’ functionalities can be configured with various attributes.
+The attributes chosen to configure these target types can significantly
+impact the security profile of the block device, and in-turn, of the
+system itself.  For instance, the type of encryption algorithm and the
+key size determines the strength of encryption for a given block device.
+
+Therefore, verifying the current state of various block devices as well
+as their various target attributes is crucial for external services before
+fully trusting the system with business-critical data/workload.
+
+IMA kernel subsystem provides the necessary functionality for
+device mapper to measure the state and configuration of
+various block devices -
+  - BY device mapper itself, from within the kernel,
+  - in a tamper resistant way,
+  - and re-measured - triggered on state/configuration change.
+
+Setting the IMA Policy:
+=======================
+For IMA to measure the data on a given system, the IMA policy on the
+system needs to be updated to have following line, and the system needs
+to be restarted for the measurements to take effect.
+
+/etc/ima/ima-policy
+ measure func=CRITICAL_DATA label=device-mapper template=ima-buf
+
+The measurements will be reflected in the IMA logs, which are located at:
+
+/sys/kernel/security/integrity/ima/ascii_runtime_measurements
+/sys/kernel/security/integrity/ima/binary_runtime_measurements
+
+Then IMA ASCII measurement log has the following format:
+PCR TEMPLATE_DIGEST TEMPLATE ALG:EVENT_DIGEST EVENT_NAME EVENT_DATA
+
+PCR := Platform Configuration Register, in which the values are registered.
+       This is applicable if TPM chip is in use.
+TEMPLATE_DIGEST := Template digest of the IMA record.
+TEMPLATE := Template that registered the integrity value (e.g. ima-buf).
+ALG:EVENT_DIGEST = Algorithm to compute event digest, followed by digest of event data
+EVENT_NAME := Description of the event (e.g. 'table_load').
+EVENT_DATA := The event data to be measured.
+
+The DM target data measured by IMA subsystem can alternatively
+be queried from userspace by setting DM_IMA_MEASUREMENT_FLAG with
+DM_TABLE_STATUS_CMD.
+
+Supported Device States:
+========================
+Following device state changes will trigger IMA measurements.
+01. Table load
+02. Device resume
+03. Device remove
+04. Table clear
+05. Device rename
+
+01. Table load:
+---------------
+When a new table is loaded in a device's inactive table slot,
+the device information and target specific details from the
+targets in the table are measured.
+
+For instance, if a linear device is created with the following table entries,
+# dmsetup create linear1
+0 2 linear /dev/loop0 512
+2 2 linear /dev/loop0 512
+4 2 linear /dev/loop0 512
+6 2 linear /dev/loop0 512
+
+Then IMA ASCII measurement log will have an entry with:
+EVENT_NAME := table_load
+EVENT_DATA := [device_data];[target_data_row_1];[target_data_row_2];...[target_data_row_n];
+
+E.g.
+(converted from ASCII to text for readability)
+10 a8c5ff755561c7a28146389d1514c318592af49a ima-buf sha256:4d73481ecce5eadba8ab084640d85bb9ca899af4d0a122989252a76efadc5b72
+table_load
+name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=4;
+target_index=0,target_begin=0,target_len=2,target_type_name=linear,target_type_version=1.4.0,device_name=7:0,start=512;
+target_index=1,target_begin=2,target_len=2,target_type_name=linear,target_type_version=1.4.0,device_name=7:0,start=512;
+target_index=2,target_begin=4,target_len=2,target_type_name=linear,target_type_version=1.4.0,device_name=7:0,start=512;
+target_index=3,target_begin=6,target_len=2,target_type_name=linear,target_type_version=1.4.0,device_name=7:0,start=512;
+
+02. Device resume:
+------------------
+When a suspended device is resumed, the device information and a sha256 hash of the
+data from previous load of an active table are measured.
+
+For instance, if a linear device is resumed with the following command,
+#dmsetup resume linear1
+
+Then IMA ASCII measurement log will have an entry with:
+EVENT_NAME := device_resume
+EVENT_DATA := [device_data];active_table_hash=(sha256hash([device_data];[target_data_row_1];...[target_data_row_n]);
+              current_device_capacity=<N>;
+
+E.g.
+(converted from ASCII to text for readability)
+10 56c00cc062ffc24ccd9ac2d67d194af3282b934e ima-buf sha256:e7d12c03b958b4e0e53e7363a06376be88d98a1ac191fdbd3baf5e4b77f329b6
+device_resume
+name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=4;
+active_table_hash=4d73481ecce5eadba8ab084640d85bb9ca899af4d0a122989252a76efadc5b72;current_device_capacity=8;
+
+03. Device remove:
+------------------
+When a device is removed, the device information and a sha256 hash of the
+data from an active and inactive table are measured.
+
+For instance, if a linear device is removed with the following command,
+# dmsetup remove linear1
+
+Then IMA ASCII measurement log will have an entry with:
+EVENT_NAME := device_remove
+EVENT_DATA := [device_active_metadata];[device_inactive_metadata];
+              [active_table_hash=(sha256hash([device_active_metadata];[active_table_row_1];...[active_table_row_n]),
+              [inactive_table_hash=(sha256hash([device_inactive_metadata];[inactive_table_row_1];...[inactive_table_row_n]),
+              remove_all=[y|n];current_device_capacity=<N>;
+
+E.g
+(converted from ASCII to text for readability)
+10 499812b621b705061c4514d643894483e16d2619 ima-buf sha256:c3f26b02f09bf5b464925589454bdd4d354077ce430fd1e75c9e96ce29cd1cad
+device_remove
+device_active_metadata=name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=4;
+device_inactive_metadata=name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=2;
+active_table_hash=4d73481ecce5eadba8ab084640d85bb9ca899af4d0a122989252a76efadc5b72,
+inactive_table_hash=5596cc857b0e887fd0c5d58dc6382513284596b07f09fd37efae2da224bd521d,remove_all=n;
+current_device_capacity=8;
+
+
+04. Table clear:
+----------------
+When an inactive table is cleared from the device, the device information and a sha256 hash of the
+data from an inactive table are measured.
+
+For instance, if a linear device's inactive table is cleared with the following command,
+
+# dmsetup clear linear1
+
+Then IMA ASCII measurement log will have an entry with:
+EVENT_NAME := table_clear
+EVENT_DATA := [device_data];inactive_table_hash=(sha256hash([device_data];[inactive_table_row_1];...[inactive_table_row_n]);
+current_device_capacity=<N>;
+
+E.g.
+(converted from ASCII to text for readability)
+10 9c11e284d792875352d51c09f6643c96649484be ima-buf sha256:84b22b364ea4d8264fa33c38635c18ef448fa9077731fa7e5f969b1da2003ea4
+table_clear
+name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=2;
+inactive_table_hash=5596cc857b0e887fd0c5d58dc6382513284596b07f09fd37efae2da224bd521d;current_device_capacity=0;
+
+
+05. Device rename:
+------------------
+When an device's NAME or UUID is changed, the device information and the new NAME and UUID
+are measured.
+
+For instance, if a linear device's name is changed with the following command,
+
+#dmsetup rename linear1 linear=2
+Then IMA ASCII measurement log will have an entry with:
+EVENT_NAME := device_rename
+EVENT_DATA := [current_device_data];new_name=<new_name_value>;new_uuid=<new_uuid_value>;current_device_capacity=<N>;
+
+E.g 1:
+#dmsetup rename linear1 --setuuid 1234-5678
+
+IMA Log entry:
+(converted from ASCII to text for readability)
+10 7380ef4d1349fe1ebd74affa54e9fcc960e3cbf5 ima-buf sha256:9759e36a17a967ea43c1bf3455279395a40bd0401105ec5ad8edb9a52054efc7
+device_rename
+name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=1;new_name=linear1,new_uuid=1234-5678;current_device_capacity=2;
+
+E.g 2:
+# dmsetup rename linear1 linear=2
+10 092c8266fc36e44f74c59f123ecfe15310f249f4 ima-buf sha256:4cf8b85c81fa6fedaeb602b05019124dbbb0605dce58fcdeea56887a8a3874cd
+device_rename
+name=linear1,uuid=1234-5678,major=253,minor=0,minor_count=1,num_targets=1;new_name=linear\=2,new_uuid=1234-5678;current_device_capacity=2;
+
+
+Supported targets:
+==================
+Following targets are supported to measure their data using IMA.
+
+01. cache
+02. crypt
+03. integrity
+04. linear
+05. mirror
+06. multipath
+07. raid
+08. snapshot
+09. striped
+10. verity
+
+01. cache
+---------
+<<documenatation in progress>>
+
+02. crypt
+-----
+When a crypt target is loaded, then IMA ASCII measurement log will have an entry
+similar to the following, depicting what crypt attributes are measured in EVENT_DATA.
+
+(converted from ASCII to text for readability)
+10 fe3b80a35b155bd282df778e2625066c05fc068c ima-buf sha256:2d86ce9d6f16a4a97607318aa123ae816e0ceadefeea7903abf7f782f2cb78ad
+table_load
+name=test-crypt,uuid=,major=253,minor=0,minor_count=1,num_targets=1;
+target_index=0,target_begin=0,target_len=1953125,target_type_name=crypt,target_type_version=1.23.0,
+allow_discards=y,same_cpu=n,submit_from_crypt_cpus=n,no_read_workqueue=n,no_write_workqueue=n,
+iv_large_sectors=n,cipher_string=aes-xts-plain64,key_size=32,key_parts=1,key_extra_size=0,key_mac_size=0;
+
+03. integrity
+-------------
+<<documenatation in progress>>
+
+
+04. linear
+----------
+When a linear target is loaded, then IMA ASCII measurement log will have an entry
+similar to the following, depicting what linear attributes are measured in EVENT_DATA.
+
+(converted from ASCII to text for readability)
+10 a8c5ff755561c7a28146389d1514c318592af49a ima-buf sha256:4d73481ecce5eadba8ab084640d85bb9ca899af4d0a122989252a76efadc5b72
+table_load
+name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=4;
+target_index=0,target_begin=0,target_len=2,target_type_name=linear,target_type_version=1.4.0,device_name=7:0,start=512;
+target_index=1,target_begin=2,target_len=2,target_type_name=linear,target_type_version=1.4.0,device_name=7:0,start=512;
+target_index=2,target_begin=4,target_len=2,target_type_name=linear,target_type_version=1.4.0,device_name=7:0,start=512;
+target_index=3,target_begin=6,target_len=2,target_type_name=linear,target_type_version=1.4.0,device_name=7:0,start=512;
+
+05. mirror
+----------
+When a mirror target is loaded, then IMA ASCII measurement log will have an entry
+similar to the following, depicting what mirror attributes are measured in EVENT_DATA.
+
+(converted from ASCII to text for readability)
+10 90ff9113a00c367df823595dc347425ce3bfc50a ima-buf sha256:8da0678ed3bf616533573d9e61e5342f2bd16cb0b3145a08262641a743806c2e
+table_load
+name=test-mirror,uuid=,major=253,minor=4,minor_count=1,num_targets=1;
+target_index=0,target_begin=0,target_len=1953125,target_type_name=mirror,target_type_version=1.14.0,
+mirrors=2,mirror_device_0=253:2,mirror_device_0_status=A,mirror_device_1=253:3,mirror_device_1_status=A,
+handle_errors=y,keep_log=n,log_type_status=;
+
+06. multipath
+-------------
+<<documenatation in progress>>
+
+07. raid
+--------
+When a raid target is loaded, then IMA ASCII measurement log will have an entry
+similar to the following, depicting what raid attributes are measured in EVENT_DATA.
+
+(converted from ASCII to text for readability)
+10 76cb30d0cd0fe099966f20f5c82e3a2ac29b21a0 ima-buf sha256:52250f20b27376fcfb348bdfa1e1cf5acfd6646e0f3ad1a72952cffd9f818753
+table_load
+name=test-raid1,uuid=,major=253,minor=2,minor_count=1,num_targets=1;
+target_index=0,target_begin=0,target_len=1953125,target_type_name=raid,target_type_version=1.15.1,
+raid_type=raid1,raid_disks=2,raid_state=idle,raid_device_0_status=A,raid_device_1_status=A;
+
+08. snapshot
+------------
+<<documenatation in progress>>
+
+09. striped
+----------
+When a linear target is loaded, then IMA ASCII measurement log will have an entry
+similar to the following, depicting what linear attributes are measured in EVENT_DATA.
+
+(converted from ASCII to text for readability)
+10 7bd94fa8f799169b9f12d97b9dbdce4dc5509233 ima-buf sha256:0d148eda69887f7833f1a6042767b54359cd23b64fa941b9e1856879eee1f778
+table_load
+name=test-raid0,uuid=,major=253,minor=8,minor_count=1,num_targets=1;
+target_index=0,target_begin=0,target_len=7812096,target_type_name=striped,target_type_version=1.6.0,stripes=4,chunk_size=128,
+stripe_0_device_name=253:1,stripe_0_physical_start=0,stripe_0_status=A,
+stripe_1_device_name=253:3,stripe_1_physical_start=0,stripe_1_status=A,
+stripe_2_device_name=253:5,stripe_2_physical_start=0,stripe_2_status=A,
+stripe_3_device_name=253:7,stripe_3_physical_start=0,stripe_3_status=A;
+
+10. verity
+----------
+When a verity target is loaded, then IMA ASCII measurement log will have an entry
+similar to the following, depicting what verity attributes are measured in EVENT_DATA.
+
+(converted from ASCII to text for readability)
+10 fced5f575b140fc0efac302c88a635174cd663da ima-buf sha256:021370c1cc93929460b06922c606334fb1d7ea5ecf04f2384f3157a446894283
+table_load
+name=test-verity,uuid=,major=253,minor=2,minor_count=1,num_targets=1;
+target_index=0,target_begin=0,target_len=1953120,target_type_name=verity,target_type_version=1.8.0,hash_failed=V,
+verity_version=1,data_device_name=253:1,hash_device_name=253:0,verity_algorithm=sha256,
+root_digest=29cb87e60ce7b12b443ba6008266f3e41e93e403d7f298f8e3f316b29ff89c5e,
+salt=e48da609055204e89ae53b655ca2216dd983cf3cb829f34f63a297d106d53e2d,
+ignore_zero_blocks=n,check_at_most_once=n;
diff --git a/Documentation/admin-guide/device-mapper/index.rst b/Documentation/admin-guide/device-mapper/index.rst
index 6cf8adc86fa8..cde52cc09645 100644
--- a/Documentation/admin-guide/device-mapper/index.rst
+++ b/Documentation/admin-guide/device-mapper/index.rst
@@ -13,6 +13,7 @@ Device Mapper
     dm-dust
     dm-ebs
     dm-flakey
+    dm-ima
     dm-init
     dm-integrity
     dm-io
-- 
2.25.1


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/7] dm: update target specific status functions to measure data
  2021-07-13  0:49 ` [PATCH 6/7] dm: update target specific status functions to measure data Tushar Sugandhi
@ 2021-07-13  1:06   ` Alasdair G Kergon
  2021-07-14 20:23     ` Tushar Sugandhi
  0 siblings, 1 reply; 32+ messages in thread
From: Alasdair G Kergon @ 2021-07-13  1:06 UTC (permalink / raw)
  To: Tushar Sugandhi, dm-devel, agk, snitzer, zohar, linux-integrity, nramas

On Mon, Jul 12, 2021 at 05:49:03PM -0700, Tushar Sugandhi wrote:
> The DM target data measured by IMA subsystem can alternatively
> be queried from userspace by setting DM_IMA_MEASUREMENT_FLAG with
> DM_TABLE_STATUS_CMD.

I was able to try this out - as 'dmsetup measure' - by applying the quick
patch below to the upstream LVM2 tree.
 
Alasdair


diff --git a/libdm/.exported_symbols.DM_1_02_179 b/libdm/.exported_symbols.DM_1_02_179
new file mode 100644
index 000000000..4ab603b68
--- /dev/null
+++ b/libdm/.exported_symbols.DM_1_02_179
@@ -0,0 +1 @@
+dm_task_ima_measurement
diff --git a/libdm/dm-tools/dmsetup.c b/libdm/dm-tools/dmsetup.c
index a3d1248bc..3e5983fef 100644
--- a/libdm/dm-tools/dmsetup.c
+++ b/libdm/dm-tools/dmsetup.c
@@ -2446,6 +2446,9 @@ static int _status(CMD_ARGS)
 	if (_switches[NOFLUSH_ARG] && !dm_task_no_flush(dmt))
 		goto_out;
 
+	if (!dm_task_ima_measurement(dmt))
+		goto_out;
+
 	if (!_task_run(dmt))
 		goto_out;
 
@@ -6262,6 +6265,7 @@ static struct command _dmsetup_commands[] = {
 	{"stats", "<command> [<options>] [<device>...]", 1, -1, 1, 1, _stats},
 	{"status", "[<device>...] [--noflush] [--target <target_type>]", 0, -1, 2, 0, _status},
 	{"table", "[<device>...] [--concise] [--target <target_type>] [--showkeys]", 0, -1, 2, 0, _status},
+	{"measure", "[<device>...]", 0, -1, 2, 0, _status},
 	{"wait", "<device> [<event_nr>] [--noflush]", 0, 2, 0, 0, _wait},
 	{"mknodes", "[<device>...]", 0, -1, 1, 0, _mknodes},
 	{"mangle", "[<device>...]", 0, -1, 1, 0, _mangle},
diff --git a/libdm/ioctl/libdm-iface.c b/libdm/ioctl/libdm-iface.c
index 47f14398c..22cce8e76 100644
--- a/libdm/ioctl/libdm-iface.c
+++ b/libdm/ioctl/libdm-iface.c
@@ -929,6 +929,13 @@ int dm_task_secure_data(struct dm_task *dmt)
 	return 1;
 }
 
+int dm_task_ima_measurement(struct dm_task *dmt)
+{
+	dmt->ima_measurement = 1;
+
+	return 1;
+}
+
 int dm_task_retry_remove(struct dm_task *dmt)
 {
 	dmt->retry_remove = 1;
@@ -1286,7 +1293,14 @@ static struct dm_ioctl *_flatten(struct dm_task *dmt, unsigned repeat_count)
 		}
 		dmi->flags |= DM_UUID_FLAG;
 	}
-
+	if (dmt->ima_measurement) {
+		if (_dm_version_minor < 45) {
+			log_error("WARNING: IMA measurement unsupported by "
+				  "kernel.  Aborting operation.");
+			goto bad;
+		}
+		dmi->flags |= DM_IMA_MEASUREMENT_FLAG;
+	}
 	dmi->target_count = count;
 	dmi->event_nr = dmt->event_nr;
 
@@ -1487,6 +1501,7 @@ static int _create_and_load_v4(struct dm_task *dmt)
 	task->head = dmt->head;
 	task->tail = dmt->tail;
 	task->secure_data = dmt->secure_data;
+	task->ima_measurement = dmt->ima_measurement;
 
 	r = dm_task_run(task);
 
@@ -1875,7 +1890,7 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, unsigned command,
 	}
 
 	log_debug_activation("dm %s %s%s %s%s%s %s%.0d%s%.0d%s"
-			     "%s[ %s%s%s%s%s%s%s%s%s] %.0" PRIu64 " %s [%u] (*%u)",
+			     "%s[ %s%s%s%s%s%s%s%s%s%s] %.0" PRIu64 " %s [%u] (*%u)",
 			     _cmd_data_v4[dmt->type].name,
 			     dmt->new_uuid ? "UUID " : "",
 			     dmi->name, dmi->uuid, dmt->newname ? " " : "",
@@ -1893,6 +1908,7 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, unsigned command,
 			     dmt->retry_remove ? "retryremove " : "",
 			     dmt->deferred_remove ? "deferredremove " : "",
 			     dmt->secure_data ? "securedata " : "",
+			     dmt->ima_measurement ? "ima_measurement " : "",
 			     dmt->query_inactive_table ? "inactive " : "",
 			     dmt->enable_checks ? "enablechecks " : "",
 			     dmt->sector, _sanitise_message(dmt->message),
diff --git a/libdm/ioctl/libdm-targets.h b/libdm/ioctl/libdm-targets.h
index 294210d2b..022b02c72 100644
--- a/libdm/ioctl/libdm-targets.h
+++ b/libdm/ioctl/libdm-targets.h
@@ -69,6 +69,7 @@ struct dm_task {
 	int enable_checks;
 	int expected_errno;
 	int ioctl_errno;
+	int ima_measurement;
 
 	int record_timestamp;
 
diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h
index ac31b59da..e9412da7d 100644
--- a/libdm/libdevmapper.h
+++ b/libdm/libdevmapper.h
@@ -235,6 +235,7 @@ int dm_task_suppress_identical_reload(struct dm_task *dmt);
 int dm_task_secure_data(struct dm_task *dmt);
 int dm_task_retry_remove(struct dm_task *dmt);
 int dm_task_deferred_remove(struct dm_task *dmt);
+int dm_task_ima_measurement(struct dm_task *dmt);
 
 /*
  * Record timestamp immediately after the ioctl returns.
diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c
index 708414676..d123e3ddf 100644
--- a/libdm/libdm-common.c
+++ b/libdm/libdm-common.c
@@ -336,6 +336,7 @@ struct dm_task *dm_task_create(int type)
 	dmt->new_uuid = 0;
 	dmt->secure_data = 0;
 	dmt->record_timestamp = 0;
+	dmt->ima_measurement = 0;
 
 	return dmt;
 }
diff --git a/libdm/misc/dm-ioctl.h b/libdm/misc/dm-ioctl.h
index 55dee2148..2b442ab70 100644
--- a/libdm/misc/dm-ioctl.h
+++ b/libdm/misc/dm-ioctl.h
@@ -1,6 +1,7 @@
+/* SPDX-License-Identifier: LGPL-2.0+ WITH Linux-syscall-note */
 /*
  * Copyright (C) 2001 - 2003 Sistina Software (UK) Limited.
- * Copyright (C) 2004 - 2017 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004 - 2021 Red Hat, Inc. All rights reserved.
  *
  * This file is released under the LGPL.
  */
@@ -183,7 +184,7 @@ struct dm_target_spec {
 struct dm_target_deps {
 	uint32_t count;	/* Array size */
 	uint32_t padding;	/* unused */
-	uint64_t dev[];		/* out */
+	uint64_t dev[0];	/* out */
 };
 
 /*
@@ -193,9 +194,23 @@ struct dm_name_list {
 	uint64_t dev;
 	uint32_t next;		/* offset to the next record from
 				   the _start_ of this */
-	char name[];
+	char name[0];
+
+	/*
+	 * The following members can be accessed by taking a pointer that
+	 * points immediately after the terminating zero character in "name"
+	 * and aligning this pointer to next 8-byte boundary.
+	 * Uuid is present if the flag DM_NAME_LIST_FLAG_HAS_UUID is set.
+	 *
+	 * uint32_t event_nr;
+	 * uint32_t flags;
+	 * char uuid[0];
+	 */
 };
 
+#define DM_NAME_LIST_FLAG_HAS_UUID		1
+#define DM_NAME_LIST_FLAG_DOESNT_HAVE_UUID	2
+
 /*
  * Used to retrieve the target versions
  */
@@ -203,7 +218,7 @@ struct dm_target_versions {
         uint32_t next;
         uint32_t version[3];
 
-        char name[];
+        char name[0];
 };
 
 /*
@@ -212,7 +227,7 @@ struct dm_target_versions {
 struct dm_target_msg {
 	uint64_t sector;	/* Device sector */
 
-	char message[];
+	char message[0];
 };
 
 /*
@@ -267,15 +282,15 @@ enum {
 #define DM_TABLE_STATUS  _IOWR(DM_IOCTL, DM_TABLE_STATUS_CMD, struct dm_ioctl)
 
 #define DM_LIST_VERSIONS _IOWR(DM_IOCTL, DM_LIST_VERSIONS_CMD, struct dm_ioctl)
+#define DM_GET_TARGET_VERSION _IOWR(DM_IOCTL, DM_GET_TARGET_VERSION_CMD, struct dm_ioctl)
 
 #define DM_TARGET_MSG	 _IOWR(DM_IOCTL, DM_TARGET_MSG_CMD, struct dm_ioctl)
 #define DM_DEV_SET_GEOMETRY	_IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl)
-#define DM_GET_TARGET_VERSION	_IOWR(DM_IOCTL, DM_GET_TARGET_VERSION_CMD, struct dm_ioctl)
 
 #define DM_VERSION_MAJOR	4
-#define DM_VERSION_MINOR	36
+#define DM_VERSION_MINOR	45
 #define DM_VERSION_PATCHLEVEL	0
-#define DM_VERSION_EXTRA	"-ioctl (2017-06-09)"
+#define DM_VERSION_EXTRA	"-ioctl (2021-03-22)"
 
 /* Status bits */
 #define DM_READONLY_FLAG	(1 << 0) /* In/Out */
@@ -363,4 +378,10 @@ enum {
  */
 #define DM_INTERNAL_SUSPEND_FLAG	(1 << 18) /* Out */
 
+/*
+ * If set, returns in the in buffer passed by UM, the raw table information
+ * that would be measured by IMA subsystem on device state change.
+ */
+#define DM_IMA_MEASUREMENT_FLAG	(1 << 19) /* In */
+
 #endif				/* _LINUX_DM_IOCTL_H */


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
                   ` (6 preceding siblings ...)
  2021-07-13  0:49 ` [PATCH 7/7] dm: add documentation for IMA measurement support Tushar Sugandhi
@ 2021-07-14 11:32 ` Thore Sommer
  2021-07-14 20:20   ` Tushar Sugandhi
  2021-07-20 21:27 ` Mike Snitzer
  8 siblings, 1 reply; 32+ messages in thread
From: Thore Sommer @ 2021-07-14 11:32 UTC (permalink / raw)
  To: tusharsu; +Cc: agk, dm-devel, linux-integrity, nramas, snitzer, zohar

Thank you for bringing IMA support to device mapper. The addition of dm-verity
to IMA is very useful for the project I'm working on where we boot
our distribution from removable USB media.

One of our goals is to detect tampering of the root file system remotely.
Therefore we enabled dm-verity support but implementing remote attestation for
dm-verity from userland is not ideal which was our initial plan.

This patch set enables us to leverage to already implemented IMA attestation
infrastructure by the remote attestation service that we are using (Keylime)
without trying to roll a custom solution.

We tested the initial RFC patch set and will continue testing with this one to
see if it fully works in our environment and with our use case.

Thore Sommer


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-14 11:32 ` [dm-devel] [PATCH 0/7] device mapper target measurements using IMA Thore Sommer
@ 2021-07-14 20:20   ` Tushar Sugandhi
  2021-07-27 10:18     ` Thore Sommer
  0 siblings, 1 reply; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-14 20:20 UTC (permalink / raw)
  To: Thore Sommer; +Cc: agk, dm-devel, linux-integrity, nramas, snitzer, zohar

Hello Thore,
On 7/14/21 4:32 AM, Thore Sommer wrote:
> Thank you for bringing IMA support to device mapper. The addition of dm-verity
> to IMA is very useful for the project I'm working on where we boot
> our distribution from removable USB media.
Thank you for the positive ack. Appreciate it.
> One of our goals is to detect tampering of the root file system remotely.
> Therefore we enabled dm-verity support but implementing remote attestation for
> dm-verity from userland is not ideal which was our initial plan.
Yes, remote attestation from userland is not ideal.
> This patch set enables us to leverage to already implemented IMA attestation
> infrastructure by the remote attestation service that we are using (Keylime)
> without trying to roll a custom solution.
I am glad that DM-IMA functionality is useful for your scenario.
> We tested the initial RFC patch set and will continue testing with 
> this one to see if it fully works in our environment and with our use 
> case. 
Thank you for testing the RFC patch set.
Please let me know if you discover any bugs in this one, or have any 
other feedback.

Thanks again.

Regards,
Tushar
> Thore Sommer 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 6/7] dm: update target specific status functions to measure data
  2021-07-13  1:06   ` Alasdair G Kergon
@ 2021-07-14 20:23     ` Tushar Sugandhi
  0 siblings, 0 replies; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-14 20:23 UTC (permalink / raw)
  To: dm-devel, agk, snitzer, zohar, linux-integrity, nramas


On 7/12/21 6:06 PM, Alasdair G Kergon wrote:
> On Mon, Jul 12, 2021 at 05:49:03PM -0700, Tushar Sugandhi wrote:
>> The DM target data measured by IMA subsystem can alternatively
>> be queried from userspace by setting DM_IMA_MEASUREMENT_FLAG with
>> DM_TABLE_STATUS_CMD.
> I was able to try this out - as 'dmsetup measure' - by applying the quick
> patch below to the upstream LVM2 tree.
>   
> Alasdair

Thanks Alasdair for trying this out, and sharing the dmsetup sample code.

Regards,
Tushar
>
> diff --git a/libdm/.exported_symbols.DM_1_02_179 b/libdm/.exported_symbols.DM_1_02_179
> new file mode 100644
> index 000000000..4ab603b68
> --- /dev/null
> +++ b/libdm/.exported_symbols.DM_1_02_179
> @@ -0,0 +1 @@
> +dm_task_ima_measurement
> diff --git a/libdm/dm-tools/dmsetup.c b/libdm/dm-tools/dmsetup.c
> index a3d1248bc..3e5983fef 100644
> --- a/libdm/dm-tools/dmsetup.c
> +++ b/libdm/dm-tools/dmsetup.c
> @@ -2446,6 +2446,9 @@ static int _status(CMD_ARGS)
>   	if (_switches[NOFLUSH_ARG] && !dm_task_no_flush(dmt))
>   		goto_out;
>   
> +	if (!dm_task_ima_measurement(dmt))
> +		goto_out;
> +
>   	if (!_task_run(dmt))
>   		goto_out;
>   
> @@ -6262,6 +6265,7 @@ static struct command _dmsetup_commands[] = {
>   	{"stats", "<command> [<options>] [<device>...]", 1, -1, 1, 1, _stats},
>   	{"status", "[<device>...] [--noflush] [--target <target_type>]", 0, -1, 2, 0, _status},
>   	{"table", "[<device>...] [--concise] [--target <target_type>] [--showkeys]", 0, -1, 2, 0, _status},
> +	{"measure", "[<device>...]", 0, -1, 2, 0, _status},
>   	{"wait", "<device> [<event_nr>] [--noflush]", 0, 2, 0, 0, _wait},
>   	{"mknodes", "[<device>...]", 0, -1, 1, 0, _mknodes},
>   	{"mangle", "[<device>...]", 0, -1, 1, 0, _mangle},
> diff --git a/libdm/ioctl/libdm-iface.c b/libdm/ioctl/libdm-iface.c
> index 47f14398c..22cce8e76 100644
> --- a/libdm/ioctl/libdm-iface.c
> +++ b/libdm/ioctl/libdm-iface.c
> @@ -929,6 +929,13 @@ int dm_task_secure_data(struct dm_task *dmt)
>   	return 1;
>   }
>   
> +int dm_task_ima_measurement(struct dm_task *dmt)
> +{
> +	dmt->ima_measurement = 1;
> +
> +	return 1;
> +}
> +
>   int dm_task_retry_remove(struct dm_task *dmt)
>   {
>   	dmt->retry_remove = 1;
> @@ -1286,7 +1293,14 @@ static struct dm_ioctl *_flatten(struct dm_task *dmt, unsigned repeat_count)
>   		}
>   		dmi->flags |= DM_UUID_FLAG;
>   	}
> -
> +	if (dmt->ima_measurement) {
> +		if (_dm_version_minor < 45) {
> +			log_error("WARNING: IMA measurement unsupported by "
> +				  "kernel.  Aborting operation.");
> +			goto bad;
> +		}
> +		dmi->flags |= DM_IMA_MEASUREMENT_FLAG;
> +	}
>   	dmi->target_count = count;
>   	dmi->event_nr = dmt->event_nr;
>   
> @@ -1487,6 +1501,7 @@ static int _create_and_load_v4(struct dm_task *dmt)
>   	task->head = dmt->head;
>   	task->tail = dmt->tail;
>   	task->secure_data = dmt->secure_data;
> +	task->ima_measurement = dmt->ima_measurement;
>   
>   	r = dm_task_run(task);
>   
> @@ -1875,7 +1890,7 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, unsigned command,
>   	}
>   
>   	log_debug_activation("dm %s %s%s %s%s%s %s%.0d%s%.0d%s"
> -			     "%s[ %s%s%s%s%s%s%s%s%s] %.0" PRIu64 " %s [%u] (*%u)",
> +			     "%s[ %s%s%s%s%s%s%s%s%s%s] %.0" PRIu64 " %s [%u] (*%u)",
>   			     _cmd_data_v4[dmt->type].name,
>   			     dmt->new_uuid ? "UUID " : "",
>   			     dmi->name, dmi->uuid, dmt->newname ? " " : "",
> @@ -1893,6 +1908,7 @@ static struct dm_ioctl *_do_dm_ioctl(struct dm_task *dmt, unsigned command,
>   			     dmt->retry_remove ? "retryremove " : "",
>   			     dmt->deferred_remove ? "deferredremove " : "",
>   			     dmt->secure_data ? "securedata " : "",
> +			     dmt->ima_measurement ? "ima_measurement " : "",
>   			     dmt->query_inactive_table ? "inactive " : "",
>   			     dmt->enable_checks ? "enablechecks " : "",
>   			     dmt->sector, _sanitise_message(dmt->message),
> diff --git a/libdm/ioctl/libdm-targets.h b/libdm/ioctl/libdm-targets.h
> index 294210d2b..022b02c72 100644
> --- a/libdm/ioctl/libdm-targets.h
> +++ b/libdm/ioctl/libdm-targets.h
> @@ -69,6 +69,7 @@ struct dm_task {
>   	int enable_checks;
>   	int expected_errno;
>   	int ioctl_errno;
> +	int ima_measurement;
>   
>   	int record_timestamp;
>   
> diff --git a/libdm/libdevmapper.h b/libdm/libdevmapper.h
> index ac31b59da..e9412da7d 100644
> --- a/libdm/libdevmapper.h
> +++ b/libdm/libdevmapper.h
> @@ -235,6 +235,7 @@ int dm_task_suppress_identical_reload(struct dm_task *dmt);
>   int dm_task_secure_data(struct dm_task *dmt);
>   int dm_task_retry_remove(struct dm_task *dmt);
>   int dm_task_deferred_remove(struct dm_task *dmt);
> +int dm_task_ima_measurement(struct dm_task *dmt);
>   
>   /*
>    * Record timestamp immediately after the ioctl returns.
> diff --git a/libdm/libdm-common.c b/libdm/libdm-common.c
> index 708414676..d123e3ddf 100644
> --- a/libdm/libdm-common.c
> +++ b/libdm/libdm-common.c
> @@ -336,6 +336,7 @@ struct dm_task *dm_task_create(int type)
>   	dmt->new_uuid = 0;
>   	dmt->secure_data = 0;
>   	dmt->record_timestamp = 0;
> +	dmt->ima_measurement = 0;
>   
>   	return dmt;
>   }
> diff --git a/libdm/misc/dm-ioctl.h b/libdm/misc/dm-ioctl.h
> index 55dee2148..2b442ab70 100644
> --- a/libdm/misc/dm-ioctl.h
> +++ b/libdm/misc/dm-ioctl.h
> @@ -1,6 +1,7 @@
> +/* SPDX-License-Identifier: LGPL-2.0+ WITH Linux-syscall-note */
>   /*
>    * Copyright (C) 2001 - 2003 Sistina Software (UK) Limited.
> - * Copyright (C) 2004 - 2017 Red Hat, Inc. All rights reserved.
> + * Copyright (C) 2004 - 2021 Red Hat, Inc. All rights reserved.
>    *
>    * This file is released under the LGPL.
>    */
> @@ -183,7 +184,7 @@ struct dm_target_spec {
>   struct dm_target_deps {
>   	uint32_t count;	/* Array size */
>   	uint32_t padding;	/* unused */
> -	uint64_t dev[];		/* out */
> +	uint64_t dev[0];	/* out */
>   };
>   
>   /*
> @@ -193,9 +194,23 @@ struct dm_name_list {
>   	uint64_t dev;
>   	uint32_t next;		/* offset to the next record from
>   				   the _start_ of this */
> -	char name[];
> +	char name[0];
> +
> +	/*
> +	 * The following members can be accessed by taking a pointer that
> +	 * points immediately after the terminating zero character in "name"
> +	 * and aligning this pointer to next 8-byte boundary.
> +	 * Uuid is present if the flag DM_NAME_LIST_FLAG_HAS_UUID is set.
> +	 *
> +	 * uint32_t event_nr;
> +	 * uint32_t flags;
> +	 * char uuid[0];
> +	 */
>   };
>   
> +#define DM_NAME_LIST_FLAG_HAS_UUID		1
> +#define DM_NAME_LIST_FLAG_DOESNT_HAVE_UUID	2
> +
>   /*
>    * Used to retrieve the target versions
>    */
> @@ -203,7 +218,7 @@ struct dm_target_versions {
>           uint32_t next;
>           uint32_t version[3];
>   
> -        char name[];
> +        char name[0];
>   };
>   
>   /*
> @@ -212,7 +227,7 @@ struct dm_target_versions {
>   struct dm_target_msg {
>   	uint64_t sector;	/* Device sector */
>   
> -	char message[];
> +	char message[0];
>   };
>   
>   /*
> @@ -267,15 +282,15 @@ enum {
>   #define DM_TABLE_STATUS  _IOWR(DM_IOCTL, DM_TABLE_STATUS_CMD, struct dm_ioctl)
>   
>   #define DM_LIST_VERSIONS _IOWR(DM_IOCTL, DM_LIST_VERSIONS_CMD, struct dm_ioctl)
> +#define DM_GET_TARGET_VERSION _IOWR(DM_IOCTL, DM_GET_TARGET_VERSION_CMD, struct dm_ioctl)
>   
>   #define DM_TARGET_MSG	 _IOWR(DM_IOCTL, DM_TARGET_MSG_CMD, struct dm_ioctl)
>   #define DM_DEV_SET_GEOMETRY	_IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD, struct dm_ioctl)
> -#define DM_GET_TARGET_VERSION	_IOWR(DM_IOCTL, DM_GET_TARGET_VERSION_CMD, struct dm_ioctl)
>   
>   #define DM_VERSION_MAJOR	4
> -#define DM_VERSION_MINOR	36
> +#define DM_VERSION_MINOR	45
>   #define DM_VERSION_PATCHLEVEL	0
> -#define DM_VERSION_EXTRA	"-ioctl (2017-06-09)"
> +#define DM_VERSION_EXTRA	"-ioctl (2021-03-22)"
>   
>   /* Status bits */
>   #define DM_READONLY_FLAG	(1 << 0) /* In/Out */
> @@ -363,4 +378,10 @@ enum {
>    */
>   #define DM_INTERNAL_SUSPEND_FLAG	(1 << 18) /* Out */
>   
> +/*
> + * If set, returns in the in buffer passed by UM, the raw table information
> + * that would be measured by IMA subsystem on device state change.
> + */
> +#define DM_IMA_MEASUREMENT_FLAG	(1 << 19) /* In */
> +
>   #endif				/* _LINUX_DM_IOCTL_H */

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/7] device mapper target measurements using IMA
  2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
                   ` (7 preceding siblings ...)
  2021-07-14 11:32 ` [dm-devel] [PATCH 0/7] device mapper target measurements using IMA Thore Sommer
@ 2021-07-20 21:27 ` Mike Snitzer
  8 siblings, 0 replies; 32+ messages in thread
From: Mike Snitzer @ 2021-07-20 21:27 UTC (permalink / raw)
  To: Tushar Sugandhi; +Cc: dm-devel, agk, zohar, linux-integrity, nramas

On Mon, Jul 12 2021 at  8:48P -0400,
Tushar Sugandhi <tusharsu@linux.microsoft.com> wrote:

> For a given system, various external services/infrastructure tools
> (including the attestation service) interact with it - both during the
> setup and during rest of the system run-time.  They share sensitive data
> and/or execute critical workload on that system.  The external services
> may want to verify the current run-time state of the relevant kernel
> subsystems before fully trusting the system with business-critical
> data/workload.
> 
> Device mapper is one such kernel subsystem that plays a critical role on
> a given system by providing various important functionalities to the
> block devices with various target types like crypt, verity, integrity 
> etc.  Each of these target types’ functionalities can be configured with
> various attributes.  The attributes chosen to configure these target types
> can significantly impact the security profile of the block device,
> and in-turn, of the system itself.  For instance, the type of encryption
> algorithm and the key size determines the strength of encryption for a
> given block device.
> 
> Therefore, verifying the current state of various block devices as well
> as their various target attributes is crucial for external services
> before fully trusting the system with business-critical data/workload.
> 
> IMA provides the necessary functionality for device mapper to measure the
> state and configuration of various block devices -
>   - BY device mapper itself, from within the kernel,
>   - in a tamper resistant way,
>   - and re-measured - triggered on state/configuration change.
> 
> This patch series uses this IMA functionality, by calling the function
> ima_measure_critical_data(), when a block device state is changed (e.g.
> on device create, resume, rename, remove etc.)  It measures the device
> state and configuration and stores it in IMA logs, so that it can be
> used by external services for managing the system.

I needed to EXPORT_SYMBOL_GPL(ima_measure_critical_data); otherwise I
couldn't compile.. not sure but I can only imagine you compile DM
(and all targets) to be builtin?

In addition to fixing that (in first table load patch) I changed
various things along the way while I reviewed each patch.

Things that I recall are:
- moved #ifdef CONFIG_IMA from dm-ima.c to dm-ima.h
- fixed various typos and whitespace
- consistently prepend "," in STATUSTYPE_IMA's DMEMIT()s as opposed to
  having a mix or pre and postfix throughout targets
- fixed what seemed like malformed STATUSTYPE_IMA handling for
  dm-multipath -- it was DMEMIT(";") for each dm-mpath's pathgroup
- added some fields to dm-mpath, renamed some IMA names in name=value
  pairs to be more clear.

I've staged the result in linux-next via linux-dm.git's dm-5.15
branch, see:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/log/?h=dm-5.15

I've compiled tested both with and without CONFIG_IMA set.  But
haven't actually tested the code.

Please send any incremental fixes relative to the dm-5.15 branch and
I'll get them folded in where appropriate.

Thanks,
Mike


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/7] dm: measure data on table load
  2021-07-13  0:48 ` [PATCH 1/7] dm: measure data on table load Tushar Sugandhi
@ 2021-07-21  2:12   ` Mimi Zohar
  2021-07-21 15:42     ` Mike Snitzer
  0 siblings, 1 reply; 32+ messages in thread
From: Mimi Zohar @ 2021-07-21  2:12 UTC (permalink / raw)
  To: Tushar Sugandhi, dm-devel, agk, snitzer; +Cc: linux-integrity, nramas

Hi Tushar, Mike, 

On Mon, 2021-07-12 at 17:48 -0700, Tushar Sugandhi wrote:
> +struct dm_ima_device_table_metadata {
> +       /*
> +        * Contains data specific to the device which is common across
> +        * all the targets in the table.e.g. name, uuid, major, minor etc.
> +        * The values are stored in comma separated list of key1=val1,key2=val2; pairs
> +        * delimited by a semicolon at the end of the list.
> +        */
> +       char *device_metadata;
> +       unsigned int device_metadata_len;
> +       unsigned int num_targets;
> +
> +       /*
> +        * Contains the sha256 hashs of the IMA measurements of the
> +        * target attributes key-value pairs from the active/inactive tables.
> +        */

From past experience hard coding the hash algorithm is really not a
good idea.

Mimi

> +       char *hash;
> +       unsigned int hash_len;
> +
> +};



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 7/7] dm: add documentation for IMA measurement support
  2021-07-13  0:49 ` [PATCH 7/7] dm: add documentation for IMA measurement support Tushar Sugandhi
@ 2021-07-21  2:33   ` Mimi Zohar
  2021-07-24  7:25     ` Tushar Sugandhi
  0 siblings, 1 reply; 32+ messages in thread
From: Mimi Zohar @ 2021-07-21  2:33 UTC (permalink / raw)
  To: Tushar Sugandhi, dm-devel, agk, snitzer; +Cc: linux-integrity, nramas

Hi Tushar, Mike,

On Mon, 2021-07-12 at 17:49 -0700, Tushar Sugandhi wrote:
> +Then IMA ASCII measurement log has the following format:
> +PCR TEMPLATE_DIGEST TEMPLATE ALG:EVENT_DIGEST EVENT_NAME EVENT_DATA
> +
> +PCR := Platform Configuration Register, in which the values are registered.
> +       This is applicable if TPM chip is in use.
> +TEMPLATE_DIGEST := Template digest of the IMA record.

^TEMPLATE_DATA_DIGEST

> +TEMPLATE := Template that registered the integrity value (e.g. ima-buf).

^TEMPLATE_NAME

The template data format consists of:
> +ALG:EVENT_DIGEST = Algorithm to compute event digest, followed by digest of event data
> +EVENT_NAME := Description of the event (e.g. 'table_load').
> +EVENT_DATA := The event data to be measured.

Missing from the document is a way of validating the template data. 
For example, in the original case of file measurements, the template
data contains the file hash, which can be recalculated or verified
against an allow list.

Other than re-calculating the template data digest based on the
template data, and verifying it against the template data digest in the
measurement list, would an attestation server be able to verify the
template data itself?

thanks,

Mimi


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/7] dm: measure data on table load
  2021-07-21  2:12   ` Mimi Zohar
@ 2021-07-21 15:42     ` Mike Snitzer
  2021-07-21 16:07       ` Mimi Zohar
  0 siblings, 1 reply; 32+ messages in thread
From: Mike Snitzer @ 2021-07-21 15:42 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: Tushar Sugandhi, dm-devel, agk, linux-integrity, nramas

On Tue, Jul 20 2021 at 10:12P -0400,
Mimi Zohar <zohar@linux.ibm.com> wrote:

> Hi Tushar, Mike, 
> 
> On Mon, 2021-07-12 at 17:48 -0700, Tushar Sugandhi wrote:
> > +struct dm_ima_device_table_metadata {
> > +       /*
> > +        * Contains data specific to the device which is common across
> > +        * all the targets in the table.e.g. name, uuid, major, minor etc.
> > +        * The values are stored in comma separated list of key1=val1,key2=val2; pairs
> > +        * delimited by a semicolon at the end of the list.
> > +        */
> > +       char *device_metadata;
> > +       unsigned int device_metadata_len;
> > +       unsigned int num_targets;
> > +
> > +       /*
> > +        * Contains the sha256 hashs of the IMA measurements of the
> > +        * target attributes key-value pairs from the active/inactive tables.
> > +        */
> 
> From past experience hard coding the hash algorithm is really not a
> good idea.
> 
> Mimi
> 
> > +       char *hash;
> > +       unsigned int hash_len;
> > +
> > +};

Hi Mimi,

The dm-ima.c code is using SHASH_DESC_ON_STACK and then storing the
more opaque result via 'hash' and 'hash_len'.

So if/when the dm-ima.c hash algorithm were to change this detail
won't change the dm_ima_device_table_metadata structure at all right?
But even if changes were needed this is purely an implementation
detail correct?  Why might users care which algorithm is used by
dm-ima to generate the hashes?

Assuming there is a valid reason for users to care about this, we can
improve this aspect as follow-on work.. so I don't consider this a
blocker for this patchset at this point.  Please clarify if you feel
it should be a blocker.

Thanks,
Mike


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/7] dm: measure data on table load
  2021-07-21 15:42     ` Mike Snitzer
@ 2021-07-21 16:07       ` Mimi Zohar
  2021-07-21 21:17         ` Mimi Zohar
  0 siblings, 1 reply; 32+ messages in thread
From: Mimi Zohar @ 2021-07-21 16:07 UTC (permalink / raw)
  To: Mike Snitzer; +Cc: Tushar Sugandhi, dm-devel, agk, linux-integrity, nramas

On Wed, 2021-07-21 at 11:42 -0400, Mike Snitzer wrote:
> On Tue, Jul 20 2021 at 10:12P -0400,
> Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> > Hi Tushar, Mike, 
> > 
> > On Mon, 2021-07-12 at 17:48 -0700, Tushar Sugandhi wrote:
> > > +struct dm_ima_device_table_metadata {
> > > +       /*
> > > +        * Contains data specific to the device which is common across
> > > +        * all the targets in the table.e.g. name, uuid, major, minor etc.
> > > +        * The values are stored in comma separated list of key1=val1,key2=val2; pairs
> > > +        * delimited by a semicolon at the end of the list.
> > > +        */
> > > +       char *device_metadata;
> > > +       unsigned int device_metadata_len;
> > > +       unsigned int num_targets;
> > > +
> > > +       /*
> > > +        * Contains the sha256 hashs of the IMA measurements of the
> > > +        * target attributes key-value pairs from the active/inactive tables.
> > > +        */
> > 
> > From past experience hard coding the hash algorithm is really not a
> > good idea.
> > 
> > Mimi
> > 
> > > +       char *hash;
> > > +       unsigned int hash_len;
> > > +
> > > +};
> 
> Hi Mimi,
> 
> The dm-ima.c code is using SHASH_DESC_ON_STACK and then storing the
> more opaque result via 'hash' and 'hash_len'.
> 
> So if/when the dm-ima.c hash algorithm were to change this detail
> won't change the dm_ima_device_table_metadata structure at all right?
> But even if changes were needed this is purely an implementation
> detail correct?  Why might users care which algorithm is used by
> dm-ima to generate the hashes?
> 
> Assuming there is a valid reason for users to care about this, we can
> improve this aspect as follow-on work.. so I don't consider this a
> blocker for this patchset at this point.  Please clarify if you feel
> it should be a blocker.

This goes back to my question as to if or how the template data in
these DM critical data records are to be validated by the attestation
server.   Asumming the hash/hash_len is being stored in the IMA
measurement list, the less the attestation should need to know about
the specific kernel version the better.

thanks,

Mimi


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/7] dm: measure data on table load
  2021-07-21 16:07       ` Mimi Zohar
@ 2021-07-21 21:17         ` Mimi Zohar
  2021-07-29 19:58           ` Tushar Sugandhi
  0 siblings, 1 reply; 32+ messages in thread
From: Mimi Zohar @ 2021-07-21 21:17 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Tushar Sugandhi, dm-devel, agk, linux-integrity, Lakshmi Ramasubramanian

On Wed, 2021-07-21 at 12:07 -0400, Mimi Zohar wrote:
> On Wed, 2021-07-21 at 11:42 -0400, Mike Snitzer wrote:
> > On Tue, Jul 20 2021 at 10:12P -0400,
> > Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > > Hi Tushar, Mike, 
> > > 
> > > On Mon, 2021-07-12 at 17:48 -0700, Tushar Sugandhi wrote:
> > > > +struct dm_ima_device_table_metadata {
> > > > +       /*
> > > > +        * Contains data specific to the device which is common across
> > > > +        * all the targets in the table.e.g. name, uuid, major, minor etc.
> > > > +        * The values are stored in comma separated list of key1=val1,key2=val2; pairs
> > > > +        * delimited by a semicolon at the end of the list.
> > > > +        */
> > > > +       char *device_metadata;
> > > > +       unsigned int device_metadata_len;
> > > > +       unsigned int num_targets;
> > > > +
> > > > +       /*
> > > > +        * Contains the sha256 hashs of the IMA measurements of the
> > > > +        * target attributes key-value pairs from the active/inactive tables.
> > > > +        */
> > > 
> > > From past experience hard coding the hash algorithm is really not a
> > > good idea.
> > > 
> > > Mimi
> > > 
> > > > +       char *hash;
> > > > +       unsigned int hash_len;
> > > > +
> > > > +};
> > 
> > Hi Mimi,
> > 
> > The dm-ima.c code is using SHASH_DESC_ON_STACK and then storing the
> > more opaque result via 'hash' and 'hash_len'.
> > 
> > So if/when the dm-ima.c hash algorithm were to change this detail
> > won't change the dm_ima_device_table_metadata structure at all right?
> > But even if changes were needed this is purely an implementation
> > detail correct?  Why might users care which algorithm is used by
> > dm-ima to generate the hashes?
> > 
> > Assuming there is a valid reason for users to care about this, we can
> > improve this aspect as follow-on work.. so I don't consider this a
> > blocker for this patchset at this point.  Please clarify if you feel
> > it should be a blocker.
> 
> This goes back to my question as to if or how the template data in
> these DM critical data records are to be validated by the attestation
> server.   Asumming the hash/hash_len is being stored in the IMA
> measurement list, the less the attestation should need to know about
> the specific kernel version the better.

Hi Mike, Tushar,  Laskshmi,

Perhaps, when defining a new IMA "critical data" record, especially if
you know it's going to change, the critical data should contain a
version number.

thanks,

Mimi


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 7/7] dm: add documentation for IMA measurement support
  2021-07-21  2:33   ` Mimi Zohar
@ 2021-07-24  7:25     ` Tushar Sugandhi
  2021-07-26 16:33       ` Mimi Zohar
  0 siblings, 1 reply; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-24  7:25 UTC (permalink / raw)
  To: Mimi Zohar, dm-devel, agk, snitzer; +Cc: linux-integrity, nramas

Hi Mimi,

On 7/20/21 7:33 PM, Mimi Zohar wrote:
> Hi Tushar, Mike,
> 
> On Mon, 2021-07-12 at 17:49 -0700, Tushar Sugandhi wrote:
>> +Then IMA ASCII measurement log has the following format:
>> +PCR TEMPLATE_DIGEST TEMPLATE ALG:EVENT_DIGEST EVENT_NAME EVENT_DATA
>> +
>> +PCR := Platform Configuration Register, in which the values are registered.
>> +       This is applicable if TPM chip is in use.
>> +TEMPLATE_DIGEST := Template digest of the IMA record.
> 
> ^TEMPLATE_DATA_DIGEST
> 
Will do.

>> +TEMPLATE := Template that registered the integrity value (e.g. ima-buf).
> 
> ^TEMPLATE_NAME
>
Will do.

> The template data format consists of:
>> +ALG:EVENT_DIGEST = Algorithm to compute event digest, followed by digest of event data
>> +EVENT_NAME := Description of the event (e.g. 'table_load').
>> +EVENT_DATA := The event data to be measured.
> 
Thanks. I will add this to the dm-ima documentation.

> Missing from the document is a way of validating the template data.
> For example, in the original case of file measurements, the template
> data contains the file hash, which can be recalculated or verified
> against an allow list.
> 
> Other than re-calculating the template data digest based on the
> template data, and verifying it against the template data digest in the
> measurement list, would an attestation server be able to verify the
> template data itself?
>
Yes.
In the context of device-mapper, EVENT_DATA for 'table_load' would
contain the key-value pairs for various targets in the table
(crypt, verity, integrity etc.) which the attestation servers
should be able to verify against the allowed/expected
key-value pairs specified in the attestation policy.

To avoid bloating the IMA log with same data from table_load again,
we only measure hash of the loaded table in the EVENT_DATA -
when there is a state change for DM device.
e.g. when EVENT_NAME is 'device_resume', 'table_clear',
'device_remove' etc.

Since the table clear-text is already present in the EVENT_DATA
buffer for 'table_load', and is available to attestation servers,
verifying the corresponding hash values in the
EVENT_DATA in the subsequent DM events should be possible for
the attestation servers.

Please let us know if you need further info.

Thanks,
Tushar

> thanks,
> 
> Mimi
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 7/7] dm: add documentation for IMA measurement support
  2021-07-24  7:25     ` Tushar Sugandhi
@ 2021-07-26 16:33       ` Mimi Zohar
  2021-07-26 18:28         ` Tushar Sugandhi
  0 siblings, 1 reply; 32+ messages in thread
From: Mimi Zohar @ 2021-07-26 16:33 UTC (permalink / raw)
  To: Tushar Sugandhi, dm-devel, agk, snitzer; +Cc: linux-integrity, nramas

Hi Tushar,

On Sat, 2021-07-24 at 00:25 -0700, Tushar Sugandhi wrote:
> Hi Mimi,
> 
> 
> > Missing from the document is a way of validating the template data.
> > For example, in the original case of file measurements, the template
> > data contains the file hash, which can be recalculated or verified
> > against an allow list.
> > 
> > Other than re-calculating the template data digest based on the
> > template data, and verifying it against the template data digest in the
> > measurement list, would an attestation server be able to verify the
> > template data itself?
> >
> Yes.
> In the context of device-mapper, EVENT_DATA for 'table_load' would
> contain the key-value pairs for various targets in the table
> (crypt, verity, integrity etc.) which the attestation servers
> should be able to verify against the allowed/expected
> key-value pairs specified in the attestation policy.
> 
> To avoid bloating the IMA log with same data from table_load again,
> we only measure hash of the loaded table in the EVENT_DATA -
> when there is a state change for DM device.
> e.g. when EVENT_NAME is 'device_resume', 'table_clear',
> 'device_remove' etc.
> 
> Since the table clear-text is already present in the EVENT_DATA
> buffer for 'table_load', and is available to attestation servers,
> verifying the corresponding hash values in the
> EVENT_DATA in the subsequent DM events should be possible for
> the attestation servers.
> 
> Please let us know if you need further info.

For regular files with signatures, the file signature is verified
against the file hash, both contained within the template data.  For
the SELinux "critical-data", 
commit 2554a48f4437 ("selinux: measure state and policy capabilities")
contains that information.  Missing from this patch set is information
on how the attestation server could verify the DM critical data.

Does the DM record contain everything needed for the attestion server
to verify the template record?  Are things like the hash algorithm hard
coded?

thanks,

Mimi




^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 7/7] dm: add documentation for IMA measurement support
  2021-07-26 16:33       ` Mimi Zohar
@ 2021-07-26 18:28         ` Tushar Sugandhi
  0 siblings, 0 replies; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-26 18:28 UTC (permalink / raw)
  To: Mimi Zohar, dm-devel, agk, snitzer; +Cc: linux-integrity, nramas

Hi Mimi,

On 7/26/21 9:33 AM, Mimi Zohar wrote:
> Hi Tushar,
> 
> On Sat, 2021-07-24 at 00:25 -0700, Tushar Sugandhi wrote:
>> Hi Mimi,
>>
>>
>>> Missing from the document is a way of validating the template data.
>>> For example, in the original case of file measurements, the template
>>> data contains the file hash, which can be recalculated or verified
>>> against an allow list.
>>>
>>> Other than re-calculating the template data digest based on the
>>> template data, and verifying it against the template data digest in the
>>> measurement list, would an attestation server be able to verify the
>>> template data itself?
>>>
>> Yes.
>> In the context of device-mapper, EVENT_DATA for 'table_load' would
>> contain the key-value pairs for various targets in the table
>> (crypt, verity, integrity etc.) which the attestation servers
>> should be able to verify against the allowed/expected
>> key-value pairs specified in the attestation policy.
>>
>> To avoid bloating the IMA log with same data from table_load again,
>> we only measure hash of the loaded table in the EVENT_DATA -
>> when there is a state change for DM device.
>> e.g. when EVENT_NAME is 'device_resume', 'table_clear',
>> 'device_remove' etc.
>>
>> Since the table clear-text is already present in the EVENT_DATA
>> buffer for 'table_load', and is available to attestation servers,
>> verifying the corresponding hash values in the
>> EVENT_DATA in the subsequent DM events should be possible for
>> the attestation servers.
>>
>> Please let us know if you need further info.
> 
> For regular files with signatures, the file signature is verified
> against the file hash, both contained within the template data.  For
> the SELinux "critical-data",
> commit 2554a48f4437 ("selinux: measure state and policy capabilities")
> contains that information.  Missing from this patch set is information
> on how the attestation server could verify the DM critical data.
> 
> Does the DM record contain everything needed for the attestion server
> to verify the template record?  Are things like the hash algorithm hard
> coded?
> 
Thanks for the feedback.
I think you have a point (on documenting hash algo and version in the
IMA record).

For the individual targets (crypt/linear etc.) the target version
is already recorded during table_load [1].
So for new attribute add/update/delete to those targets in future,
they should be accompanied by bumping the relevant target version.
So that the attestation servers would know what attributes to expect,
based on the target's version number.
So individual targets are already taken care of.

But for overall DM records I think some versioning would be useful
to the attestation servers, to determine what to expect for the
attributes that are at device level (i.e. not part of any individual
targets e.g. "minor_count", "num_targets" etc.).

Further, for the DM records that are not "table_load" - what hashing
algo is used to hash active/inactive table is the implementation detail
specific to DM, as Mike called out; and it is orthogonal to IMA's hash
algo.

But I think it would be useful to record the active/inactive table's
hash algo as part of the overall DM record template, so that attestation 
servers would know to how to interpret the relevant data.
(i.e. "active_table_hash", "inactive_table_hash" in [2]).

Both the changes are simple enough.

I will prototype and propose them as incremental patches along with
other changes (pointed to dm-5.15 branch as per Mike's suggestion.)

Thanks for your suggestions again.

Regards,
Tushar

[1]
+10 a8c5ff755561c7a28146389d1514c318592af49a ima-buf 
sha256:4d73481ecce5eadba8ab084640d85bb9ca899af4d0a122989252a76efadc5b72
+table_load
+name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=1;
+target_index=0,target_begin=0,target_len=2,target_name=linear,target_version=1.4.0,device_name=7:0,start=512;


+table_load
+name=test-crypt,uuid=,major=253,minor=0,minor_count=1,num_targets=1;
+target_index=0,target_begin=0,target_len=1953125,target_name=crypt,target_version=1.23.0,


[2]
+device_remove
+device_active_metadata=name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=4;
+device_inactive_metadata=name=linear1,uuid=,major=253,minor=0,minor_count=1,num_targets=2;
+active_table_hash=4d73481ecce5eadba8ab084640d85bb9ca899af4d0a122989252a76efadc5b72,
+inactive_table_hash=5596cc857b0e887fd0c5d58dc6382513284596b07f09fd37efae2da224bd521d,remove_all=n;
+current_device_capacity=8;
> thanks,
> 
> Mimi
> 
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-14 20:20   ` Tushar Sugandhi
@ 2021-07-27 10:18     ` Thore Sommer
  2021-07-27 20:33       ` Alasdair G Kergon
                         ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Thore Sommer @ 2021-07-27 10:18 UTC (permalink / raw)
  To: tusharsu; +Cc: agk, dm-devel, linux-integrity, nramas, public, snitzer, zohar

Hello Tushar,

I've now tested your patches more in depth. I've used the latest changes from
the dm-5.15 branch. Here are some things that I noticed during testing with
dm-verity. I'm relative new to IMA and device mapper, so there are also some
more general questions.

No new IMA measurement is generated if dm-verity verification fails. This is
unfortunate because to make the dm-verity measurements useful we need to be
notified when hash_failed is set to true. We will need something like
"device_update" where we remeasure the device state if it has changed.

Creating a dm-verity device with mount then removing it and now if you create it
again no measurement is generated. Is that the expected behavior?  
I would expect that new measurements for "table_load" and "device_resume" are
created even if the entries are identical the other ones before.

There is no way to verify if the root hash was verified against a signature. We
have "root_hash_sig_key_desc SIGNATURE_DESCRIPTION" in the dm table.
"SIGNATURE_DESCRIPTION" itself is not really useful because it seems that we
cannot map it back to the certificate that was used for verification but the
presence of "root_hash_sig_key_desc" might be enough in combination with
measuring the keyring.

Is there a reason that suspend is not measured?

How is the measured uuid created? The format seems to be
"CRYPT-VERITY-UUID-NAME" where UUID is uuid from the verity device and NAME is
the device mapper name. Does this naming come from the kernel or libcryptsetup?

What can happen in between a "table_load" and "device_resume" that is security
relevant?

For remote attestation services it would be nice if we have clear indicator from
what component the "ima-buf" entry was generated. Prefixing all "n-ng" field
entries with something like "dm_" would make it easier for us to add different
validators for different measurements that use the "ima-buf" template. The
keyring measurements already use "ima-buf" and using some kind of naming scheme
to easily differentiate the entries would be nice.

Not directly device mapper related, but it would be nice to also measure the
mount points and a mapping to the device IDs. The reasoning is that we can
measure if the target is correct but not if it was mounted correctly.  
In our case we can verify the trust of our initramfs that creates the dm-verity
device and then mounts it and if this fails refuses to boot, but that might not
always be the case.

Regards, 
Thore

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-27 10:18     ` Thore Sommer
@ 2021-07-27 20:33       ` Alasdair G Kergon
  2021-07-28  3:10         ` Tushar Sugandhi
  2021-07-28 17:34         ` Thore Sommer
  2021-07-28 21:33       ` Alasdair G Kergon
  2021-07-29 19:24       ` Tushar Sugandhi
  2 siblings, 2 replies; 32+ messages in thread
From: Alasdair G Kergon @ 2021-07-27 20:33 UTC (permalink / raw)
  To: Thore Sommer
  Cc: tusharsu, agk, dm-devel, linux-integrity, nramas, snitzer, zohar

On Tue, Jul 27, 2021 at 12:18:02PM +0200, Thore Sommer wrote:
> No new IMA measurement is generated if dm-verity verification fails. This is
> unfortunate because to make the dm-verity measurements useful we need to be
> notified when hash_failed is set to true. We will need something like
> "device_update" where we remeasure the device state if it has changed.
 
Measurements in the current patchset are only triggered by ioctl calls
initiated by userspace.

Having other triggering mechanisms - such as hooking into internal
events when something unexpected happens - could be considered for
follow-on patches.

> Creating a dm-verity device with mount then removing it and now if you create it
> again no measurement is generated. Is that the expected behavior?  

Each of the relevant dm ioctls should be logged separately each time.  If that's
not happening it might need fixing.

> Is there a reason that suspend is not measured?

A suspend doesn't change the configuration so falls outside the criteria
for this patch set.  (If there is some need for it, it would be covered
by the need to measure internal events that I mentioned above.)
 
> What can happen in between a "table_load" and "device_resume" that is security
> relevant?

The table prepared by the load can be cleared.  That would change the
effect of the resume.
 
> Not directly device mapper related, but it would be nice to also measure the
> mount points and a mapping to the device IDs. 

Again, that would be for future patches building on these ones.

Alasdair


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-27 20:33       ` Alasdair G Kergon
@ 2021-07-28  3:10         ` Tushar Sugandhi
  2021-07-28 17:14           ` Thore Sommer
  2021-07-28 17:34         ` Thore Sommer
  1 sibling, 1 reply; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-28  3:10 UTC (permalink / raw)
  To: Thore Sommer, agk, dm-devel, linux-integrity, nramas, snitzer, zohar

Hi Thore,

On 7/27/21 1:33 PM, Alasdair G Kergon wrote:
>> Creating a dm-verity device with mount then removing it and now if you create it
>> again no measurement is generated. Is that the expected behavior?
> Each of the relevant dm ioctls should be logged separately each time.  If that's
> not happening it might need fixing.
> 
Most likely this is because you haven't set CONFIG_IMA_DISABLE_HTABLE=y.

See "IMA: support for duplicate measurement records" [1] for details.

Please let us know if you still see this behavior after setting
CONFIG_IMA_DISABLE_HTABLE=y.

Thanks,
Tushar

[1] 
https://github.com/torvalds/linux/commit/52c208397c246f0c31d031eb8c41f9c7e9fdec0e

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-28  3:10         ` Tushar Sugandhi
@ 2021-07-28 17:14           ` Thore Sommer
  2021-07-29 17:32             ` Tushar Sugandhi
  0 siblings, 1 reply; 32+ messages in thread
From: Thore Sommer @ 2021-07-28 17:14 UTC (permalink / raw)
  To: tusharsu; +Cc: agk, dm-devel, linux-integrity, nramas, public, snitzer, zohar

Hi Tushar,

> Most likely this is because you haven't set CONFIG_IMA_DISABLE_HTABLE=y.
Yes, that was the case. 

With CONFIG_IMA_DISABLE_HTABLE=y the behavior is as expected. Now a new
measurement is created if I create the same device twice.

Regards,
Thore

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-27 20:33       ` Alasdair G Kergon
  2021-07-28  3:10         ` Tushar Sugandhi
@ 2021-07-28 17:34         ` Thore Sommer
  1 sibling, 0 replies; 32+ messages in thread
From: Thore Sommer @ 2021-07-28 17:34 UTC (permalink / raw)
  To: agk; +Cc: dm-devel, linux-integrity, nramas, public, snitzer, tusharsu, zohar

Hi Alasdair,

thank you for answering my questions. The answers helped a lot.

> Having other triggering mechanisms - such as hooking into internal
> events when something unexpected happens - could be considered for
> follow-on patches.

I've written [1] a proof of concept implementation that provides a hook for
targets to remeasure their state. This was my first attempt at writing kernel
code, so there might be some serious bugs. With something like this implemented
remote attestation for state of dm-verity should be possible.

The output if a target calls that hook is:
device_update DEVICE_METADATA;TARGET_METADATA,TARGET_STATUS

Regards,
Thore

[1]
diff --git a/drivers/md/dm-ima.c b/drivers/md/dm-ima.c
index 2d796e439bee..b743a2e9add4 100644
--- a/drivers/md/dm-ima.c
+++ b/drivers/md/dm-ima.c
@@ -703,3 +703,84 @@ void dm_ima_measure_on_device_rename(struct mapped_device *md)
 	kfree(new_dev_name);
 	kfree(new_dev_uuid);
 }
+
+/*
+ * Give the option for targets to remeasure on state change.
+ */
+void dm_ima_measure_on_target_update(struct dm_target *ti){
+	char *ima_buf = NULL, *target_metadata_buf = NULL, *target_data_buf= NULL;
+	status_type_t type = STATUSTYPE_IMA;
+	size_t target_metadata_buf_len, target_data_buf_len;
+	unsigned int num_targets, i;
+	struct dm_table *table = ti->table;
+	struct mapped_device *md = table->md;
+	unsigned int  target_num = -1;
+	bool noio = true;
+	int l = 0;
+
+	ima_buf = dm_ima_alloc(DM_IMA_MEASUREMENT_BUF_LEN, GFP_KERNEL, noio);
+	if (!ima_buf)
+		return;
+
+	target_metadata_buf = dm_ima_alloc(DM_IMA_TARGET_METADATA_BUF_LEN, GFP_KERNEL, noio);
+	if (!target_metadata_buf)
+		goto exit;
+
+	target_data_buf = dm_ima_alloc(DM_IMA_TARGET_DATA_BUF_LEN, GFP_KERNEL, noio);
+	if (!target_data_buf)
+		goto exit;
+
+
+	num_targets = dm_table_get_num_targets(table);
+
+	// Get target number
+	for (i = 0; i < num_targets; i++){
+		struct dm_target *ti2 = dm_table_get_target(table, i);
+		if (!ti)
+			goto exit;
+		if (ti == ti2){
+			target_num = i;
+			break;
+		}
+	}
+	if (target_num == -1)
+		goto exit;
+
+	/*
+	 * First retrieve the target metadata.
+	 */
+	scnprintf(target_metadata_buf, DM_IMA_TARGET_METADATA_BUF_LEN,
+			  "target_index=%d,target_begin=%llu,target_len=%llu,",
+			  target_num, ti->begin, ti->len);
+	target_metadata_buf_len = strlen(target_metadata_buf);
+
+	/*
+	 * Then retrieve the actual target data.
+	 */
+	if (ti->type->status)
+		ti->type->status(ti, type, STATUSTYPE_IMA, target_data_buf,
+					DM_IMA_TARGET_DATA_BUF_LEN);
+	else
+		target_data_buf[0] = '\0';
+	target_data_buf_len = strlen(target_data_buf);
+
+	// Add the device metadata
+	memcpy(ima_buf + l, md->ima.active_table.device_metadata, md->ima.active_table.device_metadata_len);
+	l += md->ima.active_table.device_metadata_len;
+
+	// Add the target metadata
+	memcpy(ima_buf + l, target_metadata_buf, target_metadata_buf_len);
+	l += target_metadata_buf_len;
+
+	// Add the new target state
+	memcpy(ima_buf + l, target_data_buf, target_data_buf_len);
+
+	dm_ima_measure_data("device_update", ima_buf, strlen(ima_buf), noio);
+
+exit:
+	kfree(ima_buf);
+	kfree(target_data_buf);
+	kfree(target_metadata_buf);
+}
+
+EXPORT_SYMBOL_GPL(dm_ima_measure_on_target_update);
\ No newline at end of file
diff --git a/drivers/md/dm-ima.h b/drivers/md/dm-ima.h
index 6e6f18bf05b4..f90c4147277a 100644
--- a/drivers/md/dm-ima.h
+++ b/drivers/md/dm-ima.h
@@ -53,6 +53,7 @@ void dm_ima_measure_on_device_resume(struct mapped_device *md, bool swap);
 void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all);
 void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map);
 void dm_ima_measure_on_device_rename(struct mapped_device *md);
+void dm_ima_measure_on_target_update(struct dm_target *ti);
 
 #else
 
@@ -62,6 +63,7 @@ static inline void dm_ima_measure_on_device_resume(struct mapped_device *md, boo
 static inline void dm_ima_measure_on_device_remove(struct mapped_device *md, bool remove_all) {}
 static inline void dm_ima_measure_on_table_clear(struct mapped_device *md, bool new_map) {}
 static inline void dm_ima_measure_on_device_rename(struct mapped_device *md) {}
+static inline void dm_ima_measure_on_target_update(struct dm_target *ti) {}
 
 #endif /* CONFIG_IMA */
 
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index bfefa100c265..f0764dd8bd4d 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -16,6 +16,7 @@
 #include "dm-verity.h"
 #include "dm-verity-fec.h"
 #include "dm-verity-verify-sig.h"
+#include "dm-ima.h"
 #include <linux/module.h>
 #include <linux/reboot.h>
 
@@ -217,10 +218,15 @@ static int verity_handle_err(struct dm_verity *v, enum verity_block_type type,
 	char *envp[] = { verity_env, NULL };
 	const char *type_str = "";
 	struct mapped_device *md = dm_table_get_md(v->ti->table);
+	int old_hash_failed = v->hash_failed;
 
 	/* Corruption should be visible in device status in all modes */
 	v->hash_failed = 1;
 
+	if (!old_hash_failed){
+		dm_ima_measure_on_target_update(v->ti);
+	}
+
 	if (v->corrupted_errs >= DM_VERITY_MAX_CORRUPTED_ERRS)
 		goto out;
 
-- 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-27 10:18     ` Thore Sommer
  2021-07-27 20:33       ` Alasdair G Kergon
@ 2021-07-28 21:33       ` Alasdair G Kergon
  2021-08-02 10:45         ` Thore Sommer
  2021-07-29 19:24       ` Tushar Sugandhi
  2 siblings, 1 reply; 32+ messages in thread
From: Alasdair G Kergon @ 2021-07-28 21:33 UTC (permalink / raw)
  To: Thore Sommer
  Cc: tusharsu, agk, dm-devel, linux-integrity, nramas, snitzer, zohar

On Tue, Jul 27, 2021 at 12:18:02PM +0200, Thore Sommer wrote:
> How is the measured uuid created? The format seems to be
> "CRYPT-VERITY-UUID-NAME" where UUID is uuid from the verity device and NAME is
> the device mapper name. Does this naming come from the kernel or libcryptsetup?

See libdevmapper.h:
/*
 * Configure default UUID prefix string.
 * Conventionally this is a short capitalised prefix indicating the subsystem
 * that is managing the devices, e.g. "LVM-" or "MPATH-".
 * To support stacks of devices from different subsystems, recursive functions
 * stop recursing if they reach a device with a different prefix.
 */
int dm_set_uuid_prefix(const char *uuid_prefix);


Each device-mapper device may have a uuid of up to 128 characters plus
trailing NUL.  Whichever piece software activates the device assigns the
uuid (so userspace or kernel boot parameters).  By convention each such
piece of software uses a short prefix ending with a hyphen that
identifies that software as the "owner" (manager) of that dm device.
This means each piece of software can easily filter out the devices for
which it is responsible and ignore all the others etc.  It can use the
remainder of the UUID to identify the device uniquely to itself.
Another convention is that when one device is a 'wrapper' of some sort
around another, it may create the uuid by adding its prefix to the uuid
of the device it is wrapping - this might give you stacked prefixes.
When there's a complex one-composed-from-many device structure, suffices
may be used to identify the components.

Think of the 'name' as the human-friendly device name and the uuid as
a software-friendly internal name.

Alasdair


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-28 17:14           ` Thore Sommer
@ 2021-07-29 17:32             ` Tushar Sugandhi
  0 siblings, 0 replies; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-29 17:32 UTC (permalink / raw)
  To: Thore Sommer; +Cc: agk, dm-devel, linux-integrity, nramas, snitzer, zohar



On 7/28/21 10:14 AM, Thore Sommer wrote:
> Hi Tushar,
> 
>> Most likely this is because you haven't set CONFIG_IMA_DISABLE_HTABLE=y.
> Yes, that was the case.
> 
> With CONFIG_IMA_DISABLE_HTABLE=y the behavior is as expected. Now a new
> measurement is created if I create the same device twice.
> 
> Regards,
> Thore
> 
Thanks for confirming Thore. :)

Regards,
Tushar

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-27 10:18     ` Thore Sommer
  2021-07-27 20:33       ` Alasdair G Kergon
  2021-07-28 21:33       ` Alasdair G Kergon
@ 2021-07-29 19:24       ` Tushar Sugandhi
  2021-08-02 10:38         ` Thore Sommer
  2 siblings, 1 reply; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-29 19:24 UTC (permalink / raw)
  To: Thore Sommer; +Cc: agk, dm-devel, linux-integrity, nramas, snitzer, zohar

Hi Thore,
Replying to a few questions which were not already answered by me/Alasdair.


On 7/27/21 3:18 AM, Thore Sommer wrote:

> There is no way to verify if the root hash was verified against a signature. We
> have "root_hash_sig_key_desc SIGNATURE_DESCRIPTION" in the dm table.
> "SIGNATURE_DESCRIPTION" itself is not really useful because it seems that we
> cannot map it back to the certificate that was used for verification but the
> presence of "root_hash_sig_key_desc" might be enough in combination with
> measuring the keyring.

Thanks for the suggestion Thore.
I can update the verity_status() to measure if v->signature_key_desc is 
set.

Something like:
DMEMIT("signature_key_desc_present=%c,", v->signature_key_desc ? 'y' :
'n');

Alasdair, Mike,
Can you tell if this is needed and/or sufficient?
If it is needed, should we log the full string  v->signature_key_desc?
I am concerned about logging the full string as it is an unbounded
buffer (a char*) coming from UM. And at the same time, not sure if just
logging the presence is sufficient. Thoughts?

Thore,
Please note – even if we measure signature_key_desc (full string or just
its presence): in order to use it with the keyrings, the IMA policy also
needs to be set to measure key rings (using “measure func=KEY_CHECK
...”). It is independent from measuring the device mapper data (which is
measured when the policy is set to “measure func=CRITICAL_DATA
label=device-mapper ...”).

Therefore measuring keyrings together (i.e. in the same IMA log) with DM
data  is not always guaranteed, since it is dictated by how the IMA
policy is configured.

Just FYI.

> For remote attestation services it would be nice if we have clear indicator from
> what component the "ima-buf" entry was generated. Prefixing all "n-ng" field
> entries with something like "dm_" would make it easier for us to add different
> validators for different measurements that use the "ima-buf" template. The
> keyring measurements already use "ima-buf" and using some kind of naming scheme
> to easily differentiate the entries would be nice.
The event names typically come from kernel components that are doing the
measurement of critical data. So any duplicates should be caught in the
upstream review of the kernel patch.

But thanks for the suggestion. I will prefix the event names in this 
patch series with “dm_” to indicate they are related to device mapper.

Thanks,
Tushar
> Regards,
> Thore
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/7] dm: measure data on table load
  2021-07-21 21:17         ` Mimi Zohar
@ 2021-07-29 19:58           ` Tushar Sugandhi
  0 siblings, 0 replies; 32+ messages in thread
From: Tushar Sugandhi @ 2021-07-29 19:58 UTC (permalink / raw)
  To: Mimi Zohar, Mike Snitzer
  Cc: dm-devel, agk, linux-integrity, Lakshmi Ramasubramanian

Hi Mimi,

On 7/21/21 2:17 PM, Mimi Zohar wrote:
> On Wed, 2021-07-21 at 12:07 -0400, Mimi Zohar wrote:
>> On Wed, 2021-07-21 at 11:42 -0400, Mike Snitzer wrote:
>>> On Tue, Jul 20 2021 at 10:12P -0400,
>>> Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>
>>>> Hi Tushar, Mike,
>>>>
>>>> On Mon, 2021-07-12 at 17:48 -0700, Tushar Sugandhi wrote:
>>>>> +struct dm_ima_device_table_metadata {
>>>>> +       /*
>>>>> +        * Contains data specific to the device which is common across
>>>>> +        * all the targets in the table.e.g. name, uuid, major, minor etc.
>>>>> +        * The values are stored in comma separated list of key1=val1,key2=val2; pairs
>>>>> +        * delimited by a semicolon at the end of the list.
>>>>> +        */
>>>>> +       char *device_metadata;
>>>>> +       unsigned int device_metadata_len;
>>>>> +       unsigned int num_targets;
>>>>> +
>>>>> +       /*
>>>>> +        * Contains the sha256 hashs of the IMA measurements of the
>>>>> +        * target attributes key-value pairs from the active/inactive tables.
>>>>> +        */
>>>>
>>>>  From past experience hard coding the hash algorithm is really not a
>>>> good idea.
>>>>
>>>> Mimi
>>>>
>>>>> +       char *hash;
>>>>> +       unsigned int hash_len;
>>>>> +
>>>>> +};
>>>
>>> Hi Mimi,
>>>
>>> The dm-ima.c code is using SHASH_DESC_ON_STACK and then storing the
>>> more opaque result via 'hash' and 'hash_len'.
>>>
>>> So if/when the dm-ima.c hash algorithm were to change this detail
>>> won't change the dm_ima_device_table_metadata structure at all right?
>>> But even if changes were needed this is purely an implementation
>>> detail correct?  Why might users care which algorithm is used by
>>> dm-ima to generate the hashes?
>>>
>>> Assuming there is a valid reason for users to care about this, we can
>>> improve this aspect as follow-on work.. so I don't consider this a
>>> blocker for this patchset at this point.  Please clarify if you feel
>>> it should be a blocker.
>>
>> This goes back to my question as to if or how the template data in
>> these DM critical data records are to be validated by the attestation
>> server.   Asumming the hash/hash_len is being stored in the IMA
>> measurement list, the less the attestation should need to know about
>> the specific kernel version the better.
> 
> Hi Mike, Tushar,  Laskshmi,
> 
> Perhaps, when defining a new IMA "critical data" record, especially if
> you know it's going to change, the critical data should contain a
> version number.
>
Just to close the loop on this thread:

As I explained on the other thread in this patch series -

@the hash verification:
the clear-text for the active/inactive table hashes is already logged in
the 'table_load' event. And we will prefix the active/inactive table
hashes with hash_algo.  (e.g. sha256:<hash>) in the remaining events.
Together it should be sufficient for the attestation server to verify
the active/inactive table hashes without maintaining any list of
expected hashes or referring to kernel version etc.

@versioning:
We are already logging versions for individual targets. What was missing
was some versioning at device level. So thanks again for the suggestion.
We will add a version at device level in each of the events. Together
that should help attestation server to determine what attributes to 
expect in the event data - without relying on specific kernel version.

Thanks again for your feedback.

Regards,
Tushar

> thanks,
> 
> Mimi
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-29 19:24       ` Tushar Sugandhi
@ 2021-08-02 10:38         ` Thore Sommer
  0 siblings, 0 replies; 32+ messages in thread
From: Thore Sommer @ 2021-08-02 10:38 UTC (permalink / raw)
  To: tusharsu; +Cc: agk, dm-devel, linux-integrity, nramas, public, snitzer, zohar

Hi Tushar,

thank you for answering my questions and looking at my suggestions.

> I can update the verity_status() to measure if v->signature_key_desc is 
> set.
> 
> Something like:
> DMEMIT("signature_key_desc_present=%c,", v->signature_key_desc ? 'y' :
> 'n');

If my understanding that this entry is only set if the signature was validated
is correct then this should work.

> Please note – even if we measure signature_key_desc (full string or just
> its presence): in order to use it with the keyrings, the IMA policy also
> needs to be set to measure key rings (using "measure func=KEY_CHECK
> ..."). It is independent from measuring the device mapper data (which is
> measured when the policy is set to “measure func=CRITICAL_DATA
> label=device-mapper ...").
> 
> Therefore measuring keyrings together (i.e. in the same IMA log) with DM
> data  is not always guaranteed, since it is dictated by how the IMA
> policy is configured.

Thanks for pointing that out. Currently we don't measure the keyrings but when
we enable remote attestation for dm-verity we'll make sure that our IMA policy
also measures the keyrings.

Regards,
Thore

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [dm-devel] [PATCH 0/7] device mapper target measurements using IMA
  2021-07-28 21:33       ` Alasdair G Kergon
@ 2021-08-02 10:45         ` Thore Sommer
  0 siblings, 0 replies; 32+ messages in thread
From: Thore Sommer @ 2021-08-02 10:45 UTC (permalink / raw)
  To: agk; +Cc: dm-devel, linux-integrity, nramas, public, snitzer, tusharsu, zohar

Hi Alasdair,

thank you for the explanation and pointing me into the right direction.  
I was wondering how we can differentiate the different devices in the remote
attestation service and now I think that I understand the format of the uuids,
so that we can implement a parser/validator for that.

Regards,
Thore

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2021-08-02 10:45 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13  0:48 [PATCH 0/7] device mapper target measurements using IMA Tushar Sugandhi
2021-07-13  0:48 ` [PATCH 1/7] dm: measure data on table load Tushar Sugandhi
2021-07-21  2:12   ` Mimi Zohar
2021-07-21 15:42     ` Mike Snitzer
2021-07-21 16:07       ` Mimi Zohar
2021-07-21 21:17         ` Mimi Zohar
2021-07-29 19:58           ` Tushar Sugandhi
2021-07-13  0:48 ` [PATCH 2/7] dm: measure data on device resume Tushar Sugandhi
2021-07-13  0:49 ` [PATCH 3/7] dm: measure data on device remove Tushar Sugandhi
2021-07-13  0:49 ` [PATCH 4/7] dm: measure data on table clear Tushar Sugandhi
2021-07-13  0:49 ` [PATCH 5/7] dm: measure data on device rename Tushar Sugandhi
2021-07-13  0:49 ` [PATCH 6/7] dm: update target specific status functions to measure data Tushar Sugandhi
2021-07-13  1:06   ` Alasdair G Kergon
2021-07-14 20:23     ` Tushar Sugandhi
2021-07-13  0:49 ` [PATCH 7/7] dm: add documentation for IMA measurement support Tushar Sugandhi
2021-07-21  2:33   ` Mimi Zohar
2021-07-24  7:25     ` Tushar Sugandhi
2021-07-26 16:33       ` Mimi Zohar
2021-07-26 18:28         ` Tushar Sugandhi
2021-07-14 11:32 ` [dm-devel] [PATCH 0/7] device mapper target measurements using IMA Thore Sommer
2021-07-14 20:20   ` Tushar Sugandhi
2021-07-27 10:18     ` Thore Sommer
2021-07-27 20:33       ` Alasdair G Kergon
2021-07-28  3:10         ` Tushar Sugandhi
2021-07-28 17:14           ` Thore Sommer
2021-07-29 17:32             ` Tushar Sugandhi
2021-07-28 17:34         ` Thore Sommer
2021-07-28 21:33       ` Alasdair G Kergon
2021-08-02 10:45         ` Thore Sommer
2021-07-29 19:24       ` Tushar Sugandhi
2021-08-02 10:38         ` Thore Sommer
2021-07-20 21:27 ` Mike Snitzer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).