public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH 0/5] Add anti-rollback validation feature
@ 2023-09-12  9:47 seanedmond
  2023-09-12  9:47 ` [PATCH 1/8] drivers: rollback: Add rollback devices to driver model seanedmond
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: seanedmond @ 2023-09-12  9:47 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, stcarlso, ilias.apalodimas

From: Sean Edmond <seanedmond@microsoft.com>

Adds Add anti-rollback version protection. Images with an anti-rollback counter
value "rollback" declared in the kernel FDT will be compared against the current device 
anti-rollback counter value, and older images will not pass signature 
validation. If the image is newer, the device anti-rollback counter value will
be updated.

The "rollback" value is stored/retrieved using the newly added security driver.
A "TPM backed" and "sandbox backed" security driver have been provided as examples.

Adds new configs:
- CONFIG_DM_ROLLBACK : enable security device support
- CONFIG_ROLLBACK_SANDBOX : enables "rollback-sandbox" driver
- CONFIG_ROLLBACK_TPM : Enables "rollback-tpm" driver
- CONFIG_FIT_ROLLBACK_CHECK : enable enforcement of OS anti-rollback counter during image loading
- CONFIG_FIT_ROLLBACK_CHECK_GRACE : adds a one unit grace version to OS anti-rollback protection

changes in v2:
- arbvn -> rollback_idx
- rollback-tpm is a child of TPM device
- tpm_rollback_counter_init() tries to read NV index, defines and writes 0 if it fails
- tpm_rollback_counter_init() moved to tpm-v2.c
- Use tpm_auto_start()
- No error checking in rollback_idx_get()/rollback_idx_set() (intelligence is in fit_image_verify_rollback())
- assume "rollback" of 0 if FIT property not found
- "grace period" -> "grace version"
- drop "dm_" prefix in header
- Fix for tpm2_nv_define_space() (add "auth" parameter)
- Make NV index consistent across APIs (define/read/write/lock).  IS THIS CORRECT?!
- Add documentation

Sean Edmond (1):
  dm: test: Add a test for security driver

Stephen Carlson (4):
  drivers: security: Add security devices to driver model
  drivers: security: Add TPM2 implementation of security devices
  common: Add OS anti-rollback validation using security devices
  common: Add OS anti-rollback grace period

 MAINTAINERS                         |   9 ++
 arch/sandbox/dts/test.dts           |   8 ++
 boot/Kconfig                        |  19 +++
 boot/image-fit-sig.c                |  94 +++++++++++++++
 boot/image-fit.c                    |  23 ++++
 configs/sandbox_defconfig           |   3 +
 drivers/Kconfig                     |   2 +
 drivers/Makefile                    |   1 +
 drivers/security/Kconfig            |  25 ++++
 drivers/security/Makefile           |   7 ++
 drivers/security/sandbox_security.c |  65 +++++++++++
 drivers/security/security-tpm.c     | 173 ++++++++++++++++++++++++++++
 drivers/security/security-uclass.c  |  30 +++++
 include/dm-security.h               |  44 +++++++
 include/dm/uclass-id.h              |   1 +
 include/image.h                     |   4 +
 include/tpm-v2.h                    |   1 +
 test/dm/Makefile                    |   1 +
 test/dm/security.c                  |  78 +++++++++++++
 19 files changed, 588 insertions(+)
 create mode 100644 drivers/security/Kconfig
 create mode 100644 drivers/security/Makefile
 create mode 100644 drivers/security/sandbox_security.c
 create mode 100644 drivers/security/security-tpm.c
 create mode 100644 drivers/security/security-uclass.c
 create mode 100644 include/dm-security.h
 create mode 100644 test/dm/security.c

-- 
2.40.0


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

* [PATCH 1/8] drivers: rollback: Add rollback devices to driver model
  2023-09-12  9:47 [PATCH 0/5] Add anti-rollback validation feature seanedmond
@ 2023-09-12  9:47 ` seanedmond
  2023-12-01 14:16   ` Ilias Apalodimas
  2023-12-01 18:32   ` Simon Glass
  2023-09-12  9:47 ` [PATCH 2/8] drivers: rollback: Add TPM2 implementation of rollback devices seanedmond
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 13+ messages in thread
From: seanedmond @ 2023-09-12  9:47 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, stcarlso, ilias.apalodimas

From: Stephen Carlson <stcarlso@linux.microsoft.com>

Rollback devices currently implement operations to store an OS
anti-rollback monotonic counter. Existing devices such as the Trusted
Platform Module (TPM) already support this operation, but this uclass
provides abstraction for current and future devices that may support
different features.

- New Driver Model uclass UCLASS_ROLLBACK.
- New config CONFIG_DM_ROLLBACK to enable security device support.
- New driver rollback-sandbox matching "rollback,sandbox", enabled with
  new config CONFIG_ROLLBACK_SANDBOX.

Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 MAINTAINERS                         |  9 ++++
 drivers/Kconfig                     |  2 +
 drivers/Makefile                    |  1 +
 drivers/rollback/Kconfig            | 25 +++++++++++
 drivers/rollback/Makefile           |  6 +++
 drivers/rollback/rollback-sandbox.c | 65 +++++++++++++++++++++++++++++
 drivers/rollback/rollback-uclass.c  | 30 +++++++++++++
 include/dm/uclass-id.h              |  1 +
 include/rollback.h                  | 42 +++++++++++++++++++
 9 files changed, 181 insertions(+)
 create mode 100644 drivers/rollback/Kconfig
 create mode 100644 drivers/rollback/Makefile
 create mode 100644 drivers/rollback/rollback-sandbox.c
 create mode 100644 drivers/rollback/rollback-uclass.c
 create mode 100644 include/rollback.h

diff --git a/MAINTAINERS b/MAINTAINERS
index bf851cffd6..de14724c27 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1438,6 +1438,15 @@ F:	cmd/seama.c
 F:	doc/usage/cmd/seama.rst
 F:	test/cmd/seama.c
 
+ROLLBACK
+M:	Stephen Carlson <stcarlso@linux.microsoft.com>
+M:	Sean Edmond <seanedmond@microsoft.com>
+S:	Maintained
+F:	drivers/rollback/Kconfig
+F:	drivers/rollback/Makefile
+F:	drivers/rollback/rollback-sandbox.c
+F:	drivers/rollback/rollback-uclass.c
+
 SEMIHOSTING
 R:	Sean Anderson <sean.anderson@seco.com>
 S:	Orphaned
diff --git a/drivers/Kconfig b/drivers/Kconfig
index a25f6ae02f..faa7cbb72b 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -116,6 +116,8 @@ source "drivers/rtc/Kconfig"
 
 source "drivers/scsi/Kconfig"
 
+source "drivers/rollback/Kconfig"
+
 source "drivers/serial/Kconfig"
 
 source "drivers/smem/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index efc2a4afb2..f6cfb48cb6 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -98,6 +98,7 @@ obj-$(CONFIG_PCH) += pch/
 obj-$(CONFIG_DM_REBOOT_MODE) += reboot-mode/
 obj-y += rtc/
 obj-y += scsi/
+obj-y += rollback/
 obj-y += sound/
 obj-y += spmi/
 obj-y += watchdog/
diff --git a/drivers/rollback/Kconfig b/drivers/rollback/Kconfig
new file mode 100644
index 0000000000..31f5a3f56d
--- /dev/null
+++ b/drivers/rollback/Kconfig
@@ -0,0 +1,25 @@
+config DM_ROLLBACK
+	bool "Support rollback devices with driver model"
+	depends on DM
+	help
+	  This option enables support for the rollback uclass which supports
+	  devices intended to provide the anti-rollback feature during
+	  boot. These devices might encapsulate existing features of TPM
+	  or TEE devices, but can also be dedicated security processors
+	  implemented in specific hardware.
+
+config ROLLBACK_SANDBOX
+	bool "Enable sandbox rollback driver"
+	depends on DM_ROLLBACK
+	help
+	  This driver supports a simulated rollback device that uses volatile
+	  memory to store secure data and begins uninitialized. This
+	  implementation allows OS images with security requirements to be
+	  loaded in the sandbox environment.
+
+config ROLLBACK_TPM
+	bool "Enable TPM rollback driver"
+	depends on TPM && TPM_V2 && DM_ROLLBACK
+	help
+	  This driver supports a rollback device based on existing TPM
+	  functionality.
diff --git a/drivers/rollback/Makefile b/drivers/rollback/Makefile
new file mode 100644
index 0000000000..4e7fa46041
--- /dev/null
+++ b/drivers/rollback/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# (C) Copyright 2021 Microsoft, Inc.
+
+obj-$(CONFIG_DM_ROLLBACK) += rollback-uclass.o
+obj-$(CONFIG_ROLLBACK_SANDBOX) += rollback-sandbox.o
diff --git a/drivers/rollback/rollback-sandbox.c b/drivers/rollback/rollback-sandbox.c
new file mode 100644
index 0000000000..acbe6d2303
--- /dev/null
+++ b/drivers/rollback/rollback-sandbox.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021 Microsoft, Inc
+ * Written by Stephen Carlson <stcarlso@microsoft.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fdtdec.h>
+#include <rollback.h>
+
+static struct rollback_state {
+	u64 rollback_idx;
+};
+
+static int sb_rollback_idx_get(struct udevice *dev, u64 *rollback_idx)
+{
+	struct rollback_state *priv = dev_get_priv(dev);
+
+	if (!rollback_idx)
+		return -EINVAL;
+
+	*rollback_idx = priv->rollback_idx;
+	return 0;
+}
+
+static int sb_rollback_idx_set(struct udevice *dev, u64 rollback_idx)
+{
+	struct rollback_state *priv = dev_get_priv(dev);
+	u64 old_rollback_idx;
+
+	old_rollback_idx = priv->rollback_idx;
+	if (rollback_idx < old_rollback_idx)
+		return -EPERM;
+
+	priv->rollback_idx = rollback_idx;
+	return 0;
+}
+
+static const struct rollback_ops rollback_sandbox_ops = {
+	.rollback_idx_get		= sb_rollback_idx_get,
+	.rollback_idx_set		= sb_rollback_idx_set,
+};
+
+static int rollback_sandbox_probe(struct udevice *dev)
+{
+	struct rollback_state *priv = dev_get_priv(dev);
+
+	priv->rollback_idx = 0ULL;
+	return 0;
+}
+
+static const struct udevice_id rollback_sandbox_ids[] = {
+	{ .compatible = "sandbox,rollback" },
+	{ }
+};
+
+U_BOOT_DRIVER(rollback_sandbox) = {
+	.name	= "rollback_sandbox",
+	.id	= UCLASS_ROLLBACK,
+	.priv_auto = sizeof(struct rollback_state),
+	.of_match = rollback_sandbox_ids,
+	.probe	= rollback_sandbox_probe,
+	.ops	= &rollback_sandbox_ops,
+};
diff --git a/drivers/rollback/rollback-uclass.c b/drivers/rollback/rollback-uclass.c
new file mode 100644
index 0000000000..1fc5486b0d
--- /dev/null
+++ b/drivers/rollback/rollback-uclass.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021 Microsoft, Inc
+ * Written by Stephen Carlson <stcarlso@microsoft.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <rollback.h>
+
+int rollback_idx_get(struct udevice *dev, uint64_t *rollback_idx)
+{
+	if (!dev || !rollback_idx)
+		return -EINVAL;
+
+	return rollback_get_ops(dev)->rollback_idx_get(dev, rollback_idx);
+}
+
+int rollback_idx_set(struct udevice *dev, uint64_t rollback_idx)
+{
+	if (!dev)
+		return -EINVAL;
+
+	return rollback_get_ops(dev)->rollback_idx_set(dev, rollback_idx);
+}
+
+UCLASS_DRIVER(rollback) = {
+	.id		= UCLASS_ROLLBACK,
+	.name		= "rollback",
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 0432c95c9e..4b8b730d87 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -124,6 +124,7 @@ enum uclass_id {
 	UCLASS_RTC,		/* Real time clock device */
 	UCLASS_SCMI_AGENT,	/* Interface with an SCMI server */
 	UCLASS_SCSI,		/* SCSI device */
+	UCLASS_ROLLBACK,	/* Rollback device */
 	UCLASS_SERIAL,		/* Serial UART */
 	UCLASS_SIMPLE_BUS,	/* Bus with child devices */
 	UCLASS_SMEM,		/* Shared memory interface */
diff --git a/include/rollback.h b/include/rollback.h
new file mode 100644
index 0000000000..60dfd62dea
--- /dev/null
+++ b/include/rollback.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2021 Microsoft, Inc.
+ */
+
+#ifndef _ROLLBACK_H_
+#define _ROLLBACK_H_
+
+#include <stdint.h>
+
+/* Access the rollback operations for a device */
+#define rollback_get_ops(dev)	((struct rollback_ops *)(dev)->driver->ops)
+
+/**
+ * rollback_idx_get() Gets the OS anti-rollback version number
+ *
+ * @dev:     Device to check
+ * @rollback_idx:   The retrieved anti-rollback version
+ * @return   0 if OK, -ve on error
+ */
+int rollback_idx_get(struct udevice *dev, uint64_t *rollback_idx);
+
+/**
+ * rollback_idx_set() Sets the OS anti-rollback version number.
+ *
+ * @dev:     Device to modify
+ * @rollback_idx:   The new anti-rollback version to set
+ * @return   0 if OK, -ve on error
+ */
+int rollback_idx_set(struct udevice *dev, uint64_t rollback_idx);
+
+/**
+ * struct rollback_ops - Driver model rollback operations
+ *
+ * Refer to the functions above for the description of each operation.
+ */
+struct rollback_ops {
+	int (*rollback_idx_get)(struct udevice *dev, uint64_t *rollback_idx);
+	int (*rollback_idx_set)(struct udevice *dev, uint64_t rollback_idx);
+};
+
+#endif
-- 
2.40.0


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

* [PATCH 2/8] drivers: rollback: Add TPM2 implementation of rollback devices
  2023-09-12  9:47 [PATCH 0/5] Add anti-rollback validation feature seanedmond
  2023-09-12  9:47 ` [PATCH 1/8] drivers: rollback: Add rollback devices to driver model seanedmond
@ 2023-09-12  9:47 ` seanedmond
  2023-12-01 14:52   ` Ilias Apalodimas
  2023-09-12  9:47 ` [PATCH 3/8] common: Add OS anti-rollback validation using " seanedmond
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: seanedmond @ 2023-09-12  9:47 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, stcarlso, ilias.apalodimas

From: Stephen Carlson <stcarlso@linux.microsoft.com>

This implementation of the rollback uclass driver allows existing TPM2
devices declared in the device tree to be referenced for storing the OS
anti-rollback counter, using the TPM2 non-volatile storage API.  The
rollback device must be a child of the TPM device.  For example:

	tpm2 {
		compatible = "sandbox,tpm2";

		rollback@1 {
			compatible = "tpm,rollback";
			rollback-nv-index = <0x1001007>;
		};
	};

Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 MAINTAINERS                     |   1 +
 drivers/rollback/Makefile       |   1 +
 drivers/rollback/rollback-tpm.c | 117 ++++++++++++++++++++++++++++++++
 include/tpm-v2.h                |  17 +++++
 lib/tpm-v2.c                    |  48 +++++++++++++
 5 files changed, 184 insertions(+)
 create mode 100644 drivers/rollback/rollback-tpm.c

diff --git a/MAINTAINERS b/MAINTAINERS
index de14724c27..e5ac889db4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1446,6 +1446,7 @@ F:	drivers/rollback/Kconfig
 F:	drivers/rollback/Makefile
 F:	drivers/rollback/rollback-sandbox.c
 F:	drivers/rollback/rollback-uclass.c
+F:	drivers/security/rollback-tpm.c
 
 SEMIHOSTING
 R:	Sean Anderson <sean.anderson@seco.com>
diff --git a/drivers/rollback/Makefile b/drivers/rollback/Makefile
index 4e7fa46041..63c08863ca 100644
--- a/drivers/rollback/Makefile
+++ b/drivers/rollback/Makefile
@@ -4,3 +4,4 @@
 
 obj-$(CONFIG_DM_ROLLBACK) += rollback-uclass.o
 obj-$(CONFIG_ROLLBACK_SANDBOX) += rollback-sandbox.o
+obj-$(CONFIG_ROLLBACK_TPM) += rollback-tpm.o
\ No newline at end of file
diff --git a/drivers/rollback/rollback-tpm.c b/drivers/rollback/rollback-tpm.c
new file mode 100644
index 0000000000..3bb6214042
--- /dev/null
+++ b/drivers/rollback/rollback-tpm.c
@@ -0,0 +1,117 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2021 Microsoft, Inc
+ * Written by Stephen Carlson <stcarlso@microsoft.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <fdtdec.h>
+#include <rollback.h>
+#include <tpm_api.h>
+
+struct rollback_state {
+	u32 nv_index;
+	struct udevice *tpm_dev;
+};
+
+static int tpm_rollback_idx_get(struct udevice *dev, u64 *rollback_idx)
+{
+	struct rollback_state *priv = dev_get_priv(dev);
+	int ret;
+
+	if (!rollback_idx)
+		return -EINVAL;
+
+	ret = tpm2_nv_read_value(priv->tpm_dev, priv->nv_index, rollback_idx, sizeof(u64));
+	if (ret) {
+		log(UCLASS_ROLLBACK, LOGL_ERR,
+		    "Unable to read rollback number from TPM (ret=%d)\n", ret);
+		return ret;
+	}
+
+	return ret;
+}
+
+static int tpm_rollback_idx_set(struct udevice *dev, u64 rollback_idx)
+{
+	int ret;
+	struct rollback_state *priv = dev_get_priv(dev);
+
+	ret = tpm2_nv_write_value(priv->tpm_dev, priv->nv_index, &rollback_idx, sizeof(u64));
+	if (ret) {
+		log(UCLASS_ROLLBACK, LOGL_ERR,
+		    "Unable to write anti-rollback version to TPM (ret=%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct rollback_ops tpm_rollback_ops = {
+	.rollback_idx_get		= tpm_rollback_idx_get,
+	.rollback_idx_set		= tpm_rollback_idx_set,
+};
+
+static int tpm_rollback_probe(struct udevice *dev)
+{
+	struct rollback_state *priv = dev_get_priv(dev);
+	int ret;
+
+	/* initialize the TPM rollback counter NV index
+	 * and initial to 0.  Note, this driver provides
+	 * a NULL policy.
+	 */
+	ret = tpm_rollback_counter_init(priv->tpm_dev, priv->nv_index,
+					NULL, 0);
+	if (ret) {
+		log(UCLASS_ROLLBACK, LOGL_ERR,
+		    "TPM rollback init failed (ret=%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int tpm_rollback_remove(struct udevice *dev)
+{
+	struct rollback_state *priv = dev_get_priv(dev);
+
+	return tpm_close(priv->tpm_dev);
+}
+
+static int tpm_rollback_ofdata_to_platdata(struct udevice *dev)
+{
+	struct udevice *parent_dev;
+	struct rollback_state *priv = dev_get_priv(dev);
+
+	priv->nv_index = (u32)dev_read_u32_default(dev, "rollback-nv-index", 0);
+
+	parent_dev = dev_get_parent(dev);
+
+	if (parent_dev->driver->id == UCLASS_TPM) {
+		priv->tpm_dev = parent_dev;
+	} else {
+		log(UCLASS_ROLLBACK, LOGL_ERR,
+		    "TPM rollback must be a child node of a TPM\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct udevice_id tpm_rollback_ids[] = {
+	{ .compatible = "tpm,rollback" },
+	{ }
+};
+
+U_BOOT_DRIVER(rollback_tpm) = {
+	.name	= "rollback_tpm",
+	.id	= UCLASS_ROLLBACK,
+	.priv_auto = sizeof(struct rollback_state),
+	.of_match = tpm_rollback_ids,
+	.of_to_plat = tpm_rollback_ofdata_to_platdata,
+	.probe	= tpm_rollback_probe,
+	.remove	= tpm_rollback_remove,
+	.ops	= &tpm_rollback_ops,
+};
diff --git a/include/tpm-v2.h b/include/tpm-v2.h
index 2b6980e441..8c441066a0 100644
--- a/include/tpm-v2.h
+++ b/include/tpm-v2.h
@@ -321,6 +321,7 @@ enum tpm2_return_codes {
 	TPM2_RC_COMMAND_CODE	= TPM2_RC_VER1 + 0x0043,
 	TPM2_RC_AUTHSIZE	= TPM2_RC_VER1 + 0x0044,
 	TPM2_RC_AUTH_CONTEXT	= TPM2_RC_VER1 + 0x0045,
+	TPM2_RC_NV_UNINITIALIZED	= TPM2_RC_VER1 + 0x04a,
 	TPM2_RC_NV_DEFINED	= TPM2_RC_VER1 + 0x004c,
 	TPM2_RC_NEEDS_TEST	= TPM2_RC_VER1 + 0x0053,
 	TPM2_RC_WARN		= 0x0900,
@@ -706,4 +707,20 @@ u32 tpm2_enable_nvcommits(struct udevice *dev, uint vendor_cmd,
  */
 u32 tpm2_auto_start(struct udevice *dev);
 
+/**
+ * tpm_rollback_counter_init() - Initialize a TPM rollback counter
+ *                               (used by the TPM-backed rollback device)
+ *
+ * Will define the non-volitile index and initialize to 0 if it hasn't
+ * been created previously.
+ *
+ * @dev:	     TPM device
+ * @nv_index:	     NV index to initialize
+ * @nv_policy:       NV index policy (pass NULL for no policy)
+ * @nv_policy_size:  NV index policy size (pass 0 for no policy)
+ * Return: result of the operation
+ */
+int tpm_rollback_counter_init(struct udevice *dev, u32 nv_index,
+			      const u8 *nv_policy, size_t nv_policy_size);
+
 #endif /* __TPM_V2_H */
diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index 9ab5b46df1..c3c469eb35 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -742,3 +742,51 @@ u32 tpm2_enable_nvcommits(struct udevice *dev, uint vendor_cmd,
 
 	return 0;
 }
+
+int tpm_rollback_counter_init(struct udevice *dev, u32 nv_index,
+			      const u8 *nv_policy, size_t nv_policy_size)
+{
+	int ret;
+	u64 data;
+	u64 data0 = 0;
+
+	ret = tpm_open(dev);
+	if (ret == -EBUSY) {
+		log(UCLASS_ROLLBACK, LOGL_DEBUG,
+		    "Existing TPM session found, reusing\n");
+	} else {
+		if (ret) {
+			log(UCLASS_ROLLBACK, LOGL_ERR,
+			    "TPM initialization failed (ret=%d)\n", ret);
+			return ret;
+		}
+
+		ret = tpm2_auto_start(dev);
+		if (ret) {
+			log(UCLASS_ROLLBACK, LOGL_ERR,
+			    "TPM startup failed (ret=%d)\n", ret);
+			return ret;
+		}
+	}
+
+	if (ret) {
+		log_err("TPM startup failed\n");
+		return ret;
+	}
+
+	/* test reading NV index from TPM */
+	ret = tpm2_nv_read_value(dev, nv_index, &data, sizeof(u64));
+
+	if (ret) {
+		/*read failed.  Assume the NV index hasn't been defined yet */
+		ret = tpm2_nv_define_space(dev, nv_index, sizeof(u64), TPMA_NV_PPREAD |
+					   TPMA_NV_PPWRITE | TPMA_NV_PLATFORMCREATE,
+					   nv_policy, nv_policy_size);
+
+		/* initialize the rollback counter to 0 */
+		if (ret == TPM2_RC_NV_DEFINED || !ret)
+			ret = tpm2_nv_write_value(dev, nv_index, &data0, sizeof(u64));
+	}
+
+	return ret;
+}
-- 
2.40.0


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

* [PATCH 3/8] common: Add OS anti-rollback validation using rollback devices
  2023-09-12  9:47 [PATCH 0/5] Add anti-rollback validation feature seanedmond
  2023-09-12  9:47 ` [PATCH 1/8] drivers: rollback: Add rollback devices to driver model seanedmond
  2023-09-12  9:47 ` [PATCH 2/8] drivers: rollback: Add TPM2 implementation of rollback devices seanedmond
@ 2023-09-12  9:47 ` seanedmond
  2023-09-12  9:47 ` [PATCH 4/8] common: Add OS anti-rollback grace version seanedmond
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: seanedmond @ 2023-09-12  9:47 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, stcarlso, ilias.apalodimas

From: Stephen Carlson <stcarlso@linux.microsoft.com>

New config CONFIG_ROLLBACK_CHECK to enable enforcement of OS anti-rollback
counter during image loading.

Images with an anti-rollback counter value "rollback" declared in the FDT
will be compared against the current device anti-rollback counter value,
and older images will not pass signature validation. If the image is
newer, the device anti-rollback counter value will be updated.

Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 boot/Kconfig         |  9 +++++
 boot/image-fit-sig.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
 boot/image-fit.c     | 31 +++++++++++++++
 include/image.h      |  6 ++-
 4 files changed, 137 insertions(+), 1 deletion(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index e8fb03b801..9180a1c8dc 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -103,6 +103,15 @@ config FIT_CIPHER
 	  Enable the feature of data ciphering/unciphering in the tool mkimage
 	  and in the u-boot support of the FIT image.
 
+config FIT_ROLLBACK_CHECK
+	bool "Enable Anti rollback version check for FIT images"
+	depends on FIT_SIGNATURE
+	default n
+	help
+	  Enables FIT image anti-rollback protection. This feature is required
+	  when a platform needs to retire previous versions of FIT images due to
+	  security flaws and prevent devices from being reverted to them.
+
 config FIT_VERBOSE
 	bool "Show verbose messages when FIT images fail"
 	depends on FIT
diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index 12369896fe..91eaf4baa8 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -11,6 +11,8 @@
 #include <log.h>
 #include <malloc.h>
 #include <asm/global_data.h>
+#include <dm.h>
+#include <rollback.h>
 DECLARE_GLOBAL_DATA_PTR;
 #endif /* !USE_HOSTCC*/
 #include <fdt_region.h>
@@ -63,6 +65,44 @@ struct image_region *fit_region_make_list(const void *fit,
 	return region;
 }
 
+static int fit_image_verify_rollback(const void *fit, int image_noffset)
+{
+#if !defined(USE_HOSTCC)
+	u64 image_rollback;
+	u64 plat_rollback = 0ULL;
+	struct udevice *dev;
+	int ret;
+
+	ret = fit_image_get_rollback(fit, image_noffset, &image_rollback);
+
+	/* If the FIT doesn't contain the rollback property, assume an
+	 * anti-rollback version number of 0.  This ensures failure
+	 * if the platform anti-rollback version number is non-zero
+	 */
+	if (ret)
+		image_rollback = 0;
+
+	ret = uclass_first_device_err(UCLASS_ROLLBACK, &dev);
+	if (ret)
+		return ret;
+
+	ret = rollback_idx_get(dev, &plat_rollback);
+	if (ret)
+		return -EIO;
+
+	if (image_rollback < plat_rollback) {
+		return -EPERM;
+	} else if (image_rollback > plat_rollback) {
+		ret = rollback_idx_set(dev, image_rollback);
+		printf(" Updating OS anti-rollback to %llu from %llu\n",
+		       image_rollback, plat_rollback);
+		return ret;
+	}
+#endif
+
+	return 0;
+}
+
 static int fit_image_setup_verify(struct image_sign_info *info,
 				  const void *fit, int noffset,
 				  const void *key_blob, int required_keynode,
@@ -175,6 +215,16 @@ static int fit_image_verify_sig(const void *fit, int image_noffset,
 		goto error;
 	}
 
+	if (!tools_build()) {
+		if (FIT_IMAGE_ENABLE_ROLLBACK_CHECK && verified) {
+			ret = fit_image_verify_rollback(fit, image_noffset);
+			if (ret) {
+				err_msg = "Anti-rollback verification failed";
+				goto error;
+			}
+		}
+	}
+
 	return verified ? 0 : -EPERM;
 
 error:
@@ -385,6 +435,38 @@ static int fit_config_check_sig(const void *fit, int noffset, int conf_noffset,
 	return 0;
 }
 
+static int fit_config_verify_rollback(const void *fit, int conf_noffset,
+				      int sig_offset)
+{
+	static const char default_list[] = FIT_KERNEL_PROP "\0"
+			FIT_FDT_PROP;
+	int ret, len;
+	const char *prop, *iname, *end;
+	int image_noffset;
+
+	/* If there is "sign-images" property, use that */
+	prop = fdt_getprop(fit, sig_offset, "sign-images", &len);
+	if (!prop) {
+		prop = default_list;
+		len = sizeof(default_list);
+	}
+
+	/* Locate the images */
+	end = prop + len;
+	for (iname = prop; iname < end; iname += strlen(iname) + 1) {
+		image_noffset = fit_conf_get_prop_node(fit, conf_noffset,
+						       iname, IH_PHASE_NONE);
+		if (image_noffset < 0)
+			return -ENOENT;
+
+		ret = fit_image_verify_rollback(fit, image_noffset);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 /**
  * fit_config_verify_key() - Verify that a configuration is signed with a key
  *
@@ -444,6 +526,16 @@ static int fit_config_verify_key(const void *fit, int conf_noffset,
 		goto error;
 	}
 
+	if (!tools_build()) {
+		if (FIT_IMAGE_ENABLE_ROLLBACK_CHECK && verified) {
+			ret = fit_config_verify_rollback(fit, conf_noffset, noffset);
+			if (ret) {
+				err_msg = "Anti-rollback verification failed";
+				goto error;
+			}
+		}
+	}
+
 	if (verified)
 		return 0;
 
diff --git a/boot/image-fit.c b/boot/image-fit.c
index 3cc556b727..510cb51cd5 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -1084,6 +1084,37 @@ int fit_image_get_data_and_size(const void *fit, int noffset,
 	return ret;
 }
 
+/**
+ * fit_image_get_rollback - get anti-rollback counter
+ * @fit: pointer to the FIT image header
+ * @noffset: component image node offset
+ * @rollback: holds the rollback property value
+ *
+ * returns:
+ *     0, on success
+ *     -ENOENT if the property could not be found
+ */
+int fit_image_get_rollback(const void *fit, int noffset, uint64_t *rollback)
+{
+	const fdt64_t *val;
+	int len;
+
+	val = fdt_getprop(fit, noffset, FIT_ROLLBACK_PROP, &len);
+
+	if (!val) {
+		printf("error! Can't find property %s in FIT\n", FIT_ROLLBACK_PROP);
+		return -ENOENT;
+	}
+	if (len != sizeof(uint64_t)) {
+		printf("Property %s must be 64-bits\n", FIT_ROLLBACK_PROP);
+		return -ENOENT;
+	}
+
+	*rollback = fdt64_to_cpu(*val);
+
+	return 0;
+}
+
 /**
  * fit_image_hash_get_algo - get hash algorithm name
  * @fit: pointer to the FIT format image header
diff --git a/include/image.h b/include/image.h
index 01a6787d21..3283cd7717 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1024,6 +1024,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
 #define FIT_COMP_PROP		"compression"
 #define FIT_ENTRY_PROP		"entry"
 #define FIT_LOAD_PROP		"load"
+#define FIT_ROLLBACK_PROP	"rollback"
 
 /* configuration node */
 #define FIT_KERNEL_PROP		"kernel"
@@ -1105,6 +1106,7 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
 				       size_t *data_size);
 int fit_image_get_data_and_size(const void *fit, int noffset,
 				const void **data, size_t *size);
+int fit_image_get_rollback(const void *fit, int noffset, uint64_t *rollback_idx);
 
 /**
  * fit_get_data_node() - Get verified image data for an image
@@ -1389,6 +1391,7 @@ int calculate_hash(const void *data, int data_len, const char *algo,
  * device
  */
 #if defined(USE_HOSTCC)
+# define FIT_IMAGE_ENABLE_ROLLBACK_CHECK	0
 # if defined(CONFIG_FIT_SIGNATURE)
 #  define IMAGE_ENABLE_SIGN	1
 #  define FIT_IMAGE_ENABLE_VERIFY	1
@@ -1399,7 +1402,8 @@ int calculate_hash(const void *data, int data_len, const char *algo,
 # endif
 #else
 # define IMAGE_ENABLE_SIGN	0
-# define FIT_IMAGE_ENABLE_VERIFY	CONFIG_IS_ENABLED(FIT_SIGNATURE)
+# define FIT_IMAGE_ENABLE_VERIFY		CONFIG_IS_ENABLED(FIT_SIGNATURE)
+# define FIT_IMAGE_ENABLE_ROLLBACK_CHECK	CONFIG_IS_ENABLED(FIT_ROLLBACK_CHECK)
 #endif
 
 #ifdef USE_HOSTCC
-- 
2.40.0


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

* [PATCH 4/8] common: Add OS anti-rollback grace version
  2023-09-12  9:47 [PATCH 0/5] Add anti-rollback validation feature seanedmond
                   ` (2 preceding siblings ...)
  2023-09-12  9:47 ` [PATCH 3/8] common: Add OS anti-rollback validation using " seanedmond
@ 2023-09-12  9:47 ` seanedmond
  2023-09-12  9:47 ` [PATCH 5/8] dm: test: Add a test for rollback driver seanedmond
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: seanedmond @ 2023-09-12  9:47 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, stcarlso, ilias.apalodimas

From: Stephen Carlson <stcarlso@microsoft.com>

New config CONFIG_FIT_ROLLBACK_CHECK_GRACE to add a one unit grace version
to OS anti-rollback protection, allowing images with anti-rollback
counters exactly one less than the platform value to still be loaded. No
update to the platform anti-rollback counter will be performed in this
case.

Signed-off-by: Stephen Carlson <stcarlso@microsoft.com>
Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 boot/Kconfig         | 10 ++++++++++
 boot/image-fit-sig.c |  7 ++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index 9180a1c8dc..95a717765c 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -112,6 +112,16 @@ config FIT_ROLLBACK_CHECK
 	  when a platform needs to retire previous versions of FIT images due to
 	  security flaws and prevent devices from being reverted to them.
 
+config FIT_ROLLBACK_CHECK_GRACE
+	bool "Enable FIT Anti rollback grace version"
+	depends on FIT_ARBP
+	default n
+	help
+	  Enables a one unit grace version for FIT image anti-rollback protection,
+	  where anti-rollback protection will still accept a FIT image with an
+	  anti-rollback version one less than the current number, but will not
+	  update the platform anti-rollback counter in that case.
+
 config FIT_VERBOSE
 	bool "Show verbose messages when FIT images fail"
 	depends on FIT
diff --git a/boot/image-fit-sig.c b/boot/image-fit-sig.c
index 91eaf4baa8..5689a316b6 100644
--- a/boot/image-fit-sig.c
+++ b/boot/image-fit-sig.c
@@ -70,6 +70,7 @@ static int fit_image_verify_rollback(const void *fit, int image_noffset)
 #if !defined(USE_HOSTCC)
 	u64 image_rollback;
 	u64 plat_rollback = 0ULL;
+	u64 target_rollback;
 	struct udevice *dev;
 	int ret;
 
@@ -90,7 +91,11 @@ static int fit_image_verify_rollback(const void *fit, int image_noffset)
 	if (ret)
 		return -EIO;
 
-	if (image_rollback < plat_rollback) {
+	target_rollback = plat_rollback;
+	/* Calculate target anti-rollback version, including grace version if enabled */
+	if (CONFIG_IS_ENABLED(FIT_ROLLBACK_CHECK_GRACE) && plat_rollback > 0ULL)
+		target_rollback = plat_rollback - 1ULL;
+	if (image_rollback < target_rollback) {
 		return -EPERM;
 	} else if (image_rollback > plat_rollback) {
 		ret = rollback_idx_set(dev, image_rollback);
-- 
2.40.0


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

* [PATCH 5/8] dm: test: Add a test for rollback driver
  2023-09-12  9:47 [PATCH 0/5] Add anti-rollback validation feature seanedmond
                   ` (3 preceding siblings ...)
  2023-09-12  9:47 ` [PATCH 4/8] common: Add OS anti-rollback grace version seanedmond
@ 2023-09-12  9:47 ` seanedmond
  2023-09-12  9:47 ` [PATCH 6/8] tpm: Fix issues relating to NV Indexes seanedmond
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: seanedmond @ 2023-09-12  9:47 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, stcarlso, ilias.apalodimas

From: Sean Edmond <seanedmond@microsoft.com>

Adds a test for a sandbox and TPM backed rollback driver.

Allows for testing of anti-rollback version number get/set API using the
rollback driver.

Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 arch/sandbox/dts/test.dts |  9 +++++
 configs/sandbox_defconfig |  3 ++
 test/dm/Makefile          |  1 +
 test/dm/rollback.c        | 78 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 91 insertions(+)
 create mode 100644 test/dm/rollback.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index f351d5cb84..1226bad6a6 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1263,6 +1263,10 @@
 		backlight = <&backlight 0 100>;
 	};
 
+	rollback@0 {
+		compatible = "sandbox,rollback";
+	};
+
 	scsi {
 		compatible = "sandbox,scsi";
 		sandbox,filepath = "scsi.img";
@@ -1376,6 +1380,11 @@
 
 	tpm2 {
 		compatible = "sandbox,tpm2";
+
+		rollback@1 {
+			compatible = "tpm,rollback";
+			rollback-nv-index = <0x1001007>;
+		};
 	};
 
 	tpm {
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 1cd1c2ed7c..f2bd10fbf0 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -346,3 +346,6 @@ CONFIG_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
 CONFIG_ARM_FFA_TRANSPORT=y
+CONFIG_DM_ROLLBACK=y
+CONFIG_ROLLBACK_SANDBOX=y
+CONFIG_ROLLBACK_TPM=y
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 7ed00733c1..3875ec4f4b 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -104,6 +104,7 @@ obj-$(CONFIG_DM_RNG) += rng.o
 obj-$(CONFIG_DM_RTC) += rtc.o
 obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
 obj-$(CONFIG_SCSI) += scsi.o
+obj-$(CONFIG_DM_ROLLBACK) += security.o
 obj-$(CONFIG_DM_SERIAL) += serial.o
 obj-$(CONFIG_DM_SPI_FLASH) += sf.o
 obj-$(CONFIG_SIMPLE_BUS) += simple-bus.o
diff --git a/test/dm/rollback.c b/test/dm/rollback.c
new file mode 100644
index 0000000000..a2984797f9
--- /dev/null
+++ b/test/dm/rollback.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023 Microsoft Corporation
+ * Written by Sean Edmond <seanedmond@microsoft.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <rollback.h>
+#include <log.h>
+#include <dm/test.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+/*
+ * get_rollback() - Get a rollback driver of a given driver name
+ *
+ * @devp: Returns the rollback device
+ * @driver_name: Driver name to find
+ * Returns: 0 if OK, -ENODEV if not found
+ */
+static int get_rollback(struct udevice **devp, char *driver_name)
+{
+	struct udevice *dev;
+
+	uclass_foreach_dev_probe(UCLASS_ROLLBACK, dev) {
+		if (strcmp(dev->driver->name, driver_name) == 0) {
+			*devp = dev;
+			return 0;
+		}
+	}
+
+	return -ENODEV;
+}
+
+/* Basic test of rollback driver Anti rollback version number read/write */
+static int test_rollback_idx(struct unit_test_state *uts, char *driver_name)
+{
+	struct udevice *dev;
+	u64 rollback_idx;
+
+	/* get the rollback driver */
+	ut_assertok(get_rollback(&dev, driver_name));
+
+	/* ensure initial value is 0 */
+	rollback_idx_get(dev, &rollback_idx);
+	ut_asserteq(0, rollback_idx);
+
+	/* write 1 and ensure it's read back */
+	rollback_idx_set(dev, 1);
+	rollback_idx_get(dev, &rollback_idx);
+	ut_asserteq(1, rollback_idx);
+
+	/* write all ones and ensure it's read back */
+	rollback_idx_set(dev, 0xffffffffffffffffULL);
+	rollback_idx_get(dev, &rollback_idx);
+	ut_asserteq(0xffffffffffffffffULL, rollback_idx);
+
+	return 0;
+}
+
+static int dm_test_rollback_sandbox(struct unit_test_state *uts)
+{
+	ut_assertok(test_rollback_idx(uts, "rollback_sandbox"));
+
+	return 0;
+}
+
+DM_TEST(dm_test_rollback_sandbox, UT_TESTF_SCAN_FDT);
+
+static int dm_test_rollback_tpm(struct unit_test_state *uts)
+{
+	ut_assertok(test_rollback_idx(uts, "rollback_tpm"));
+
+	return 0;
+}
+
+DM_TEST(dm_test_rollback_tpm, UT_TESTF_SCAN_FDT);
-- 
2.40.0


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

* [PATCH 6/8] tpm: Fix issues relating to NV Indexes
  2023-09-12  9:47 [PATCH 0/5] Add anti-rollback validation feature seanedmond
                   ` (4 preceding siblings ...)
  2023-09-12  9:47 ` [PATCH 5/8] dm: test: Add a test for rollback driver seanedmond
@ 2023-09-12  9:47 ` seanedmond
  2023-09-12  9:47 ` [PATCH 7/8] sandbox: tpm: Fix TPM2_CC_NV_DEFINE_SPACE command seanedmond
  2023-09-12  9:47 ` [PATCH 8/8] doc: rollback: anti-rollback verification seanedmond
  7 siblings, 0 replies; 13+ messages in thread
From: seanedmond @ 2023-09-12  9:47 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, stcarlso, ilias.apalodimas

From: Sean Edmond <seanedmond@microsoft.com>

The TPM 2.0 command reference states that "auth" (type TPM2B_AUTH)
should come before "publicInfo" (type TPM2B_NV_PUBLIC) in the
"TPM2_NV_DefineSpace" command.  Let's add an empty "auth" (size 0), so
that this can work with compliant TPMs.

Make sure that NV index used in tpm2_nv_define_space() can be used
directly in the NV read/write/lock APIs.

Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 lib/tpm-v2.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/lib/tpm-v2.c b/lib/tpm-v2.c
index c3c469eb35..d3ecf556d2 100644
--- a/lib/tpm-v2.c
+++ b/lib/tpm-v2.c
@@ -109,7 +109,7 @@ u32 tpm2_nv_define_space(struct udevice *dev, u32 space_index,
 	const int platform_len = sizeof(u32);
 	const int session_hdr_len = 13;
 	const int message_len = 14;
-	uint offset = TPM2_HDR_LEN + platform_len + session_hdr_len +
+	uint offset = TPM2_HDR_LEN + platform_len + session_hdr_len + 2 +
 		message_len;
 	u8 command_v2[COMMAND_BUFFER_SIZE] = {
 		/* header 10 bytes */
@@ -127,6 +127,9 @@ u32 tpm2_nv_define_space(struct udevice *dev, u32 space_index,
 		0,				/* session_attrs */
 		tpm_u16(0),			/* auth_size */
 
+		/* auth value */
+		tpm_u16(0),
+
 		/* message 14 bytes + policy */
 		tpm_u16(message_len + nv_policy_size),	/* size */
 		tpm_u32(space_index),
@@ -206,7 +209,7 @@ u32 tpm2_nv_read_value(struct udevice *dev, u32 index, void *data, u32 count)
 
 		/* handles 8 bytes */
 		tpm_u32(TPM2_RH_PLATFORM),	/* Primary platform seed */
-		tpm_u32(HR_NV_INDEX + index),	/* Password authorisation */
+		tpm_u32(index),			/* nvIndex */
 
 		/* AUTH_SESSION */
 		tpm_u32(9),			/* Authorization size */
@@ -229,9 +232,14 @@ u32 tpm2_nv_read_value(struct udevice *dev, u32 index, void *data, u32 count)
 	ret = tpm_sendrecv_command(dev, command_v2, response, &response_len);
 	if (ret)
 		return log_msg_ret("read", ret);
+
+	const size_t tag_offset = 0;
+	const size_t size_offset = 2;
+	const size_t code_offset = 6;
+	const size_t data_offset = 16;
 	if (unpack_byte_string(response, response_len, "wdds",
-			       0, &tag, 2, &size, 6, &code,
-			       16, data, count))
+			       tag_offset, &tag, size_offset, &size, code_offset, &code,
+			       data_offset, data, count))
 		return TPM_LIB_ERROR;
 
 	return 0;
@@ -254,7 +262,7 @@ u32 tpm2_nv_write_value(struct udevice *dev, u32 index, const void *data,
 
 		/* handles 8 bytes */
 		tpm_u32(auth),			/* Primary platform seed */
-		tpm_u32(HR_NV_INDEX + index),	/* Password authorisation */
+		tpm_u32(index),			/* nvIndex */
 
 		/* AUTH_SESSION */
 		tpm_u32(9),			/* Authorization size */
@@ -643,7 +651,7 @@ u32 tpm2_write_lock(struct udevice *dev, u32 index)
 
 		/* handles 8 bytes */
 		tpm_u32(TPM2_RH_PLATFORM),	/* Primary platform seed */
-		tpm_u32(HR_NV_INDEX + index),	/* Password authorisation */
+		tpm_u32(index),			/* nvIndex */
 
 		/* session header 9 bytes */
 		tpm_u32(9),			/* Header size */
-- 
2.40.0


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

* [PATCH 7/8] sandbox: tpm: Fix TPM2_CC_NV_DEFINE_SPACE command
  2023-09-12  9:47 [PATCH 0/5] Add anti-rollback validation feature seanedmond
                   ` (5 preceding siblings ...)
  2023-09-12  9:47 ` [PATCH 6/8] tpm: Fix issues relating to NV Indexes seanedmond
@ 2023-09-12  9:47 ` seanedmond
  2023-09-12  9:47 ` [PATCH 8/8] doc: rollback: anti-rollback verification seanedmond
  7 siblings, 0 replies; 13+ messages in thread
From: seanedmond @ 2023-09-12  9:47 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, stcarlso, ilias.apalodimas

From: Sean Edmond <seanedmond@microsoft.com>

The TPM 2.0 command reference shows "auth" (type TPM2B_AUTH) before
"publicInfo" (type TPM2B_NV_PUBLIC).

The TPM v2 driver was updated to add this field.  The sandbox driver
needs to be updated to match the driver implementation.

Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 drivers/tpm/tpm2_tis_sandbox.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/tpm/tpm2_tis_sandbox.c b/drivers/tpm/tpm2_tis_sandbox.c
index e4004cfcca..3e38d28637 100644
--- a/drivers/tpm/tpm2_tis_sandbox.c
+++ b/drivers/tpm/tpm2_tis_sandbox.c
@@ -758,8 +758,10 @@ static int sandbox_tpm2_xfer(struct udevice *dev, const u8 *sendbuf,
 		break;
 	}
 	case TPM2_CC_NV_DEFINE_SPACE: {
-		int policy_size, index, seq;
+		int policy_size, auth_size, index, seq;
 
+		auth_size = get_unaligned_be16(sent);
+		sent += 2 + auth_size;
 		policy_size = get_unaligned_be16(sent + 12);
 		index = get_unaligned_be32(sent + 2);
 		sent += 14 + policy_size;
-- 
2.40.0


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

* [PATCH 8/8] doc: rollback: anti-rollback verification
  2023-09-12  9:47 [PATCH 0/5] Add anti-rollback validation feature seanedmond
                   ` (6 preceding siblings ...)
  2023-09-12  9:47 ` [PATCH 7/8] sandbox: tpm: Fix TPM2_CC_NV_DEFINE_SPACE command seanedmond
@ 2023-09-12  9:47 ` seanedmond
  7 siblings, 0 replies; 13+ messages in thread
From: seanedmond @ 2023-09-12  9:47 UTC (permalink / raw)
  To: u-boot; +Cc: sjg, stcarlso, ilias.apalodimas

From: Sean Edmond <seanedmond@microsoft.com>

Add documentation for anti-rollback verification, optional properties in
FIT image, and UCLASS_ROLLBACK device.

Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
---
 doc/develop/driver-model/index.rst         |  1 +
 doc/develop/driver-model/rollback-info.rst | 42 +++++++++++++++++
 doc/usage/fit/signature.rst                | 53 +++++++++++++++++++---
 doc/usage/fit/source_file_format.rst       | 21 ++++++++-
 4 files changed, 108 insertions(+), 9 deletions(-)
 create mode 100644 doc/develop/driver-model/rollback-info.rst

diff --git a/doc/develop/driver-model/index.rst b/doc/develop/driver-model/index.rst
index 8e12bbd936..bb2d2e8a5d 100644
--- a/doc/develop/driver-model/index.rst
+++ b/doc/develop/driver-model/index.rst
@@ -25,6 +25,7 @@ subsystems
    pci-info
    pmic-framework
    remoteproc-framework
+   rollback-info
    serial-howto
    soc-framework
    spi-howto
diff --git a/doc/develop/driver-model/rollback-info.rst b/doc/develop/driver-model/rollback-info.rst
new file mode 100644
index 0000000000..06ba4584a9
--- /dev/null
+++ b/doc/develop/driver-model/rollback-info.rst
@@ -0,0 +1,42 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Rollback Driver Guide
+=====================
+
+Rollback protection is required when there is a need to retire
+previous versions of FIT images due to security flaws in them.
+See :doc:`../../usage/fit/signature` for details on rollback protection.
+
+The rollback driver is intended to provide tamper-evident storage using
+a UCLASS_ROLLBACK device.  It exposes the following APIs which are
+used in the FIT verification process (if FIT_ROLLBACK_CHECK is set):
+- rollback_idx_get()
+- rollback_idx_set()
+
+A TPM based rollback device has been provided as an example.  Users could
+create their own UCLASS_ROLLBACK device to access proprietary secure storage.
+
+TPM 2.0 based rollback devices
+------------
+
+A TPM-based rollback device has been provided as a reference.  It can be
+enabled with:
+- DM_ROLLBACK
+- ROLLBACK_TPM
+
+The tpm based rollback device should be added as a child node of the TPM in
+the u-boot device tree.  You should provide the property "rollback-nv-index"
+to set the TPM's NV index (if no "rollback-nv-index" is provided, an NV index
+of 0 is assumed).  For example:
+
+	tpm2 {
+		compatible = "sandbox,tpm2";
+
+		rollback@1 {
+			compatible = "tpm,rollback";
+			rollback-nv-index = <0x1001007>;
+		};
+	};
+
+Note, the ROLLBACK_TPM device does not set any policy.  Consumers of this
+driver should add a policy to make it more secure.
\ No newline at end of file
diff --git a/doc/usage/fit/signature.rst b/doc/usage/fit/signature.rst
index 0804bffd1e..30dd529c29 100644
--- a/doc/usage/fit/signature.rst
+++ b/doc/usage/fit/signature.rst
@@ -674,14 +674,53 @@ Sign the fitImage with the hardware key::
     "model=PKCS%2315%20emulated;manufacturer=ZeitControl;serial=000xxxxxxxxx;token=OpenPGP%20card%20%28User%20PIN%20%28sig%29%29" \
     -K u-boot.dtb -N pkcs11 -r fitImage
 
+Roll-back Protection
+-----------------------------------------
 
-Future Work
------------
-
-- Roll-back protection using a TPM is done using the tpm command. This can
-  be scripted, but we might consider a default way of doing this, built into
-  bootm.
-
+Rollback protection is required when there is a need to retire
+previous versions of FIT images due to security flaws in them.
+
+This feature is realized by adding a rollback index as a property
+in the FIT image.  Every time a security flaw is discovered, the
+rollback index is incremented.  For example:
+
++-------------+----------------+
+| FIT version | Rollback index |
++=============+================+
+| 1.0         | 0              |
++-------------+----------------+
+| 1.1         | 0              |
++-------------+----------------+
+| 2.0         | 0              |
++-------------+----------------+
+| Security issue found!        |
++-------------+----------------+
+| 1.2         | 1              |
++-------------+----------------+
+| 2.1         | 1              |
++-------------+----------------+
+
+Each sub-image node inside /images node has an optional rollback rollback_index
+("rollback"). This version number is part of signed data and is incremented as
+security flaws are discovered and fixed. If no "rollback" property is present
+in the FIT image, a rollback index of 0 is assumed.
+
+U-Boot stores the last seen "rollback" for a given image type in platform
+specific tamper-evident storage (using a UCLASS_ROLLBACK device).
+See :doc:`../../develop/driver-model/rollback-info` for more information on
+rollback devices.
+
+As part of signature verification, U-Boot enforces rollback
+protection if enabled (with FIT_ROLLBACK_CHECK). The rollback index stored in secure
+storage is validated with "rollback" in the sub-image node. If the counter
+in the FIT image is lower than the counter in platform secure storage, image
+validation has failed. If both counters match or the image counter is
+higher than that in the platform secure storage, the image validation is
+successful. If the rollback index has increased, U-Boot stores the new
+value in secure storage.
+
+A grace version can be enabled with FIT_ROLLBACK_CHECK_GRACE, which will accept
+a FIT image with an anti-rollback version one less than number in secure storage.
 
 Possible Future Work
 --------------------
diff --git a/doc/usage/fit/source_file_format.rst b/doc/usage/fit/source_file_format.rst
index b2b1e42bd7..149447b856 100644
--- a/doc/usage/fit/source_file_format.rst
+++ b/doc/usage/fit/source_file_format.rst
@@ -389,6 +389,16 @@ signature-1
     Each signature sub-node represents separate signature
     calculated for node's data according to specified algorithm.
 
+Optional properties
+~~~~~~~~~~~~~~~~~~~~
+
+rollback
+    The rollback index (used for anti rollback protection) specified
+    as a 64-bit value.  For example a rollback index of 1 is sepcified as:
+
+    rollback = <0 1>
+
+    If no "rollback" property is present, a rollback index of 0 is assumed.
 
 Hash nodes
 ----------
@@ -507,7 +517,6 @@ padding
     The padding algorithm, it may be pkcs-1.5 or pss,
     if no value is provided we assume pkcs-1.5
 
-
 '/configurations' node
 ----------------------
 
@@ -524,12 +533,20 @@ The 'configurations' node has the following structure::
         ...
 
 
-Optional property
+Optional properties
 ~~~~~~~~~~~~~~~~~
 
 default
     Selects one of the configuration sub-nodes as a default configuration.
 
+rollback
+    The rollback index (used for anti rollback protection) specified
+    as a 64-bit value.  For example a rollback index of 1 is sepcified as:
+
+    rollback = <0 1>
+
+    If no "rollback" property is present, a rollback index of 0 is assumed.
+
 Mandatory nodes
 ~~~~~~~~~~~~~~~
 
-- 
2.40.0


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

* Re: [PATCH 1/8] drivers: rollback: Add rollback devices to driver model
  2023-09-12  9:47 ` [PATCH 1/8] drivers: rollback: Add rollback devices to driver model seanedmond
@ 2023-12-01 14:16   ` Ilias Apalodimas
  2023-12-01 18:32   ` Simon Glass
  1 sibling, 0 replies; 13+ messages in thread
From: Ilias Apalodimas @ 2023-12-01 14:16 UTC (permalink / raw)
  To: seanedmond; +Cc: u-boot, sjg, stcarlso

Hi Sean, 

Apologies for the very late reply.
Simon, can you have a look since this is mostly the DM part?

On Tue, Sep 12, 2023 at 02:47:24AM -0700, seanedmond@linux.microsoft.com wrote:
> From: Stephen Carlson <stcarlso@linux.microsoft.com>
> 
> Rollback devices currently implement operations to store an OS
> anti-rollback monotonic counter. Existing devices such as the Trusted
> Platform Module (TPM) already support this operation, but this uclass
> provides abstraction for current and future devices that may support
> different features.
> 
> - New Driver Model uclass UCLASS_ROLLBACK.
> - New config CONFIG_DM_ROLLBACK to enable security device support.
> - New driver rollback-sandbox matching "rollback,sandbox", enabled with
>   new config CONFIG_ROLLBACK_SANDBOX.
> 
> Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
> Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
> ---
>  MAINTAINERS                         |  9 ++++
>  drivers/Kconfig                     |  2 +
>  drivers/Makefile                    |  1 +
>  drivers/rollback/Kconfig            | 25 +++++++++++
>  drivers/rollback/Makefile           |  6 +++
>  drivers/rollback/rollback-sandbox.c | 65 +++++++++++++++++++++++++++++
>  drivers/rollback/rollback-uclass.c  | 30 +++++++++++++
>  include/dm/uclass-id.h              |  1 +
>  include/rollback.h                  | 42 +++++++++++++++++++
>  9 files changed, 181 insertions(+)
>  create mode 100644 drivers/rollback/Kconfig
>  create mode 100644 drivers/rollback/Makefile
>  create mode 100644 drivers/rollback/rollback-sandbox.c
>  create mode 100644 drivers/rollback/rollback-uclass.c
>  create mode 100644 include/rollback.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bf851cffd6..de14724c27 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1438,6 +1438,15 @@ F:	cmd/seama.c
>  F:	doc/usage/cmd/seama.rst
>  F:	test/cmd/seama.c
>  
> +ROLLBACK
> +M:	Stephen Carlson <stcarlso@linux.microsoft.com>
> +M:	Sean Edmond <seanedmond@microsoft.com>
> +S:	Maintained
> +F:	drivers/rollback/Kconfig
> +F:	drivers/rollback/Makefile
> +F:	drivers/rollback/rollback-sandbox.c
> +F:	drivers/rollback/rollback-uclass.c
> +
>  SEMIHOSTING
>  R:	Sean Anderson <sean.anderson@seco.com>
>  S:	Orphaned
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index a25f6ae02f..faa7cbb72b 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -116,6 +116,8 @@ source "drivers/rtc/Kconfig"
>  
>  source "drivers/scsi/Kconfig"
>  
> +source "drivers/rollback/Kconfig"
> +
>  source "drivers/serial/Kconfig"
>  
>  source "drivers/smem/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index efc2a4afb2..f6cfb48cb6 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -98,6 +98,7 @@ obj-$(CONFIG_PCH) += pch/
>  obj-$(CONFIG_DM_REBOOT_MODE) += reboot-mode/
>  obj-y += rtc/
>  obj-y += scsi/
> +obj-y += rollback/
>  obj-y += sound/
>  obj-y += spmi/
>  obj-y += watchdog/
> diff --git a/drivers/rollback/Kconfig b/drivers/rollback/Kconfig
> new file mode 100644
> index 0000000000..31f5a3f56d
> --- /dev/null
> +++ b/drivers/rollback/Kconfig
> @@ -0,0 +1,25 @@
> +config DM_ROLLBACK
> +	bool "Support rollback devices with driver model"
> +	depends on DM
> +	help
> +	  This option enables support for the rollback uclass which supports
> +	  devices intended to provide the anti-rollback feature during
> +	  boot. These devices might encapsulate existing features of TPM
> +	  or TEE devices, but can also be dedicated security processors
> +	  implemented in specific hardware.
> +
> +config ROLLBACK_SANDBOX
> +	bool "Enable sandbox rollback driver"
> +	depends on DM_ROLLBACK
> +	help
> +	  This driver supports a simulated rollback device that uses volatile
> +	  memory to store secure data and begins uninitialized. This
> +	  implementation allows OS images with security requirements to be
> +	  loaded in the sandbox environment.
> +
> +config ROLLBACK_TPM
> +	bool "Enable TPM rollback driver"
> +	depends on TPM && TPM_V2 && DM_ROLLBACK
> +	help
> +	  This driver supports a rollback device based on existing TPM
> +	  functionality.
> diff --git a/drivers/rollback/Makefile b/drivers/rollback/Makefile
> new file mode 100644
> index 0000000000..4e7fa46041
> --- /dev/null
> +++ b/drivers/rollback/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# (C) Copyright 2021 Microsoft, Inc.
> +
> +obj-$(CONFIG_DM_ROLLBACK) += rollback-uclass.o
> +obj-$(CONFIG_ROLLBACK_SANDBOX) += rollback-sandbox.o
> diff --git a/drivers/rollback/rollback-sandbox.c b/drivers/rollback/rollback-sandbox.c
> new file mode 100644
> index 0000000000..acbe6d2303
> --- /dev/null
> +++ b/drivers/rollback/rollback-sandbox.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2021 Microsoft, Inc
> + * Written by Stephen Carlson <stcarlso@microsoft.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <rollback.h>
> +
> +static struct rollback_state {
> +	u64 rollback_idx;
> +};
> +
> +static int sb_rollback_idx_get(struct udevice *dev, u64 *rollback_idx)
> +{
> +	struct rollback_state *priv = dev_get_priv(dev);
> +
> +	if (!rollback_idx)
> +		return -EINVAL;
> +
> +	*rollback_idx = priv->rollback_idx;
> +	return 0;
> +}
> +
> +static int sb_rollback_idx_set(struct udevice *dev, u64 rollback_idx)
> +{
> +	struct rollback_state *priv = dev_get_priv(dev);
> +	u64 old_rollback_idx;
> +
> +	old_rollback_idx = priv->rollback_idx;

Skip the assignment, if (rollback_idx < priv->rollback_idx) is pretty
straight forward to read 

> +	if (rollback_idx < old_rollback_idx)
> +		return -EPERM;
> +
> +	priv->rollback_idx = rollback_idx;
> +	return 0;
> +}
> +
> +static const struct rollback_ops rollback_sandbox_ops = {
> +	.rollback_idx_get		= sb_rollback_idx_get,
> +	.rollback_idx_set		= sb_rollback_idx_set,
> +};

nit, but I prefer 
.rollback_idx_get = sb_rollback_idx_get, etc
makes grepping a lot easier

> +
> +static int rollback_sandbox_probe(struct udevice *dev)
> +{
> +	struct rollback_state *priv = dev_get_priv(dev);
> +
> +	priv->rollback_idx = 0ULL;

Why do you need the integer constant here?

> +	return 0;
> +}
> +
> +static const struct udevice_id rollback_sandbox_ids[] = {
> +	{ .compatible = "sandbox,rollback" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(rollback_sandbox) = {
> +	.name	= "rollback_sandbox",
> +	.id	= UCLASS_ROLLBACK,
> +	.priv_auto = sizeof(struct rollback_state),
> +	.of_match = rollback_sandbox_ids,
> +	.probe	= rollback_sandbox_probe,
> +	.ops	= &rollback_sandbox_ops,
> +};
 
[...]

Thanks
/Ilias

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

* Re: [PATCH 2/8] drivers: rollback: Add TPM2 implementation of rollback devices
  2023-09-12  9:47 ` [PATCH 2/8] drivers: rollback: Add TPM2 implementation of rollback devices seanedmond
@ 2023-12-01 14:52   ` Ilias Apalodimas
  2023-12-01 18:32     ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: Ilias Apalodimas @ 2023-12-01 14:52 UTC (permalink / raw)
  To: seanedmond; +Cc: u-boot, sjg, stcarlso

Hi Sean,

On Tue, Sep 12, 2023 at 02:47:25AM -0700, seanedmond@linux.microsoft.com wrote:
> From: Stephen Carlson <stcarlso@linux.microsoft.com>
> 
> This implementation of the rollback uclass driver allows existing TPM2
> devices declared in the device tree to be referenced for storing the OS
> anti-rollback counter, using the TPM2 non-volatile storage API.  The
> rollback device must be a child of the TPM device.  For example:
> 
> 	tpm2 {
> 		compatible = "sandbox,tpm2";
> 
> 		rollback@1 {
> 			compatible = "tpm,rollback";
> 			rollback-nv-index = <0x1001007>;
> 		};
> 	};
> 
This node is part of the DT specification right? If we accept this, we
should figure out if we can add that to the specification.

[...]

> +/*
> + * Copyright (c) 2021 Microsoft, Inc
> + * Written by Stephen Carlson <stcarlso@microsoft.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <fdtdec.h>
> +#include <rollback.h>
> +#include <tpm_api.h>
> +
> +struct rollback_state {
> +	u32 nv_index;
> +	struct udevice *tpm_dev;
> +};
> +
> +static int tpm_rollback_idx_get(struct udevice *dev, u64 *rollback_idx)
> +{
> +	struct rollback_state *priv = dev_get_priv(dev);
> +	int ret;
> +
> +	if (!rollback_idx)
> +		return -EINVAL;
> +
> +	ret = tpm2_nv_read_value(priv->tpm_dev, priv->nv_index, rollback_idx, sizeof(u64));
> +	if (ret) {
> +		log(UCLASS_ROLLBACK, LOGL_ERR,

Wouldn't the init function below make sure the index is created? IOW
shouldn't this be a log_err()?

> +		    "Unable to read rollback number from TPM (ret=%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int tpm_rollback_idx_set(struct udevice *dev, u64 rollback_idx)
> +{
> +	int ret;
> +	struct rollback_state *priv = dev_get_priv(dev);
> +
> +	ret = tpm2_nv_write_value(priv->tpm_dev, priv->nv_index, &rollback_idx, sizeof(u64));
> +	if (ret) {
> +		log(UCLASS_ROLLBACK, LOGL_ERR,
> +		    "Unable to write anti-rollback version to TPM (ret=%d)\n", ret);

log_err()

> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct rollback_ops tpm_rollback_ops = {
> +	.rollback_idx_get		= tpm_rollback_idx_get,
> +	.rollback_idx_set		= tpm_rollback_idx_set,
> +};
> +
> +static int tpm_rollback_probe(struct udevice *dev)
> +{
> +	struct rollback_state *priv = dev_get_priv(dev);
> +	int ret;
> +
> +	/* initialize the TPM rollback counter NV index
> +	 * and initial to 0.  Note, this driver provides
> +	 * a NULL policy.
> +	 */
> +	ret = tpm_rollback_counter_init(priv->tpm_dev, priv->nv_index,
> +					NULL, 0);
> +	if (ret) {
> +		log(UCLASS_ROLLBACK, LOGL_ERR,
> +		    "TPM rollback init failed (ret=%d)\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int tpm_rollback_remove(struct udevice *dev)
> +{
> +	struct rollback_state *priv = dev_get_priv(dev);
> +
> +	return tpm_close(priv->tpm_dev);

I am not sure what this function is supposed to be doing. Why is it called
remove but only closes the tpm?

[...]

> +int tpm_rollback_counter_init(struct udevice *dev, u32 nv_index,
> +			      const u8 *nv_policy, size_t nv_policy_size)
> +{
> +	int ret;
> +	u64 data;
> +	u64 data0 = 0;
> +
> +	ret = tpm_open(dev);
> +	if (ret == -EBUSY) {
> +		log(UCLASS_ROLLBACK, LOGL_DEBUG,
> +		    "Existing TPM session found, reusing\n");
> +	} else {
> +		if (ret) {
> +			log(UCLASS_ROLLBACK, LOGL_ERR,
> +			    "TPM initialization failed (ret=%d)\n", ret);
> +			return ret;
> +		}
> +
> +		ret = tpm2_auto_start(dev);
> +		if (ret) {
> +			log(UCLASS_ROLLBACK, LOGL_ERR,
> +			    "TPM startup failed (ret=%d)\n", ret);
> +			return ret;
> +		}
> +	}
> +
> +	if (ret) {
> +		log_err("TPM startup failed\n");
> +		return ret;
> +	}
> +
> +	/* test reading NV index from TPM */
> +	ret = tpm2_nv_read_value(dev, nv_index, &data, sizeof(u64));
> +
> +	if (ret) {
> +		/*read failed.  Assume the NV index hasn't been defined yet */
> +		ret = tpm2_nv_define_space(dev, nv_index, sizeof(u64), TPMA_NV_PPREAD |
> +					   TPMA_NV_PPWRITE | TPMA_NV_PLATFORMCREATE,
> +					   nv_policy, nv_policy_size);
> +

Since the policy is NULL and no password is provided how do we trust this counter? 

The TPMs allow the creation of an 'NV Counter Index' which would be better
for what you are trying to do here.

> +		/* initialize the rollback counter to 0 */
> +		if (ret == TPM2_RC_NV_DEFINED || !ret)
> +			ret = tpm2_nv_write_value(dev, nv_index, &data0, sizeof(u64));
> +	}
> +
> +	return ret;
> +}
> -- 
> 2.40.0
> 

Thanks
/Ilias

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

* Re: [PATCH 2/8] drivers: rollback: Add TPM2 implementation of rollback devices
  2023-12-01 14:52   ` Ilias Apalodimas
@ 2023-12-01 18:32     ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2023-12-01 18:32 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: seanedmond, u-boot, stcarlso

Hi,

On Fri, 1 Dec 2023 at 07:52, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Sean,
>
> On Tue, Sep 12, 2023 at 02:47:25AM -0700, seanedmond@linux.microsoft.com wrote:
> > From: Stephen Carlson <stcarlso@linux.microsoft.com>
> >
> > This implementation of the rollback uclass driver allows existing TPM2
> > devices declared in the device tree to be referenced for storing the OS
> > anti-rollback counter, using the TPM2 non-volatile storage API.  The
> > rollback device must be a child of the TPM device.  For example:
> >
> >       tpm2 {
> >               compatible = "sandbox,tpm2";
> >
> >               rollback@1 {
> >                       compatible = "tpm,rollback";
> >                       rollback-nv-index = <0x1001007>;
> >               };
> >       };
> >
> This node is part of the DT specification right? If we accept this, we
> should figure out if we can add that to the specification.

For now I suggest adding a binding file to U-Boot and sending it upstream.

We still seem to be having extreme difficulty actually getting things
accepted upstream.

The logic of this driver needs a few changes, I believe:
- probe() should do nothing, to avoid probe failing due to bad counter, etc.
- we should have a startup() method to actually init the counter
- if that fails, we should have a setup() method to set up a new counter

That way the caller is in charge of things.

Of course you can put helper functions in the uclass.

Regards,
Simon

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

* Re: [PATCH 1/8] drivers: rollback: Add rollback devices to driver model
  2023-09-12  9:47 ` [PATCH 1/8] drivers: rollback: Add rollback devices to driver model seanedmond
  2023-12-01 14:16   ` Ilias Apalodimas
@ 2023-12-01 18:32   ` Simon Glass
  1 sibling, 0 replies; 13+ messages in thread
From: Simon Glass @ 2023-12-01 18:32 UTC (permalink / raw)
  To: seanedmond; +Cc: u-boot, stcarlso, ilias.apalodimas

Hi,

On Tue, 12 Sept 2023 at 03:47, <seanedmond@linux.microsoft.com> wrote:
>
> From: Stephen Carlson <stcarlso@linux.microsoft.com>
>
> Rollback devices currently implement operations to store an OS
> anti-rollback monotonic counter. Existing devices such as the Trusted
> Platform Module (TPM) already support this operation, but this uclass
> provides abstraction for current and future devices that may support
> different features.
>
> - New Driver Model uclass UCLASS_ROLLBACK.
> - New config CONFIG_DM_ROLLBACK to enable security device support.
> - New driver rollback-sandbox matching "rollback,sandbox", enabled with
>   new config CONFIG_ROLLBACK_SANDBOX.
>
> Signed-off-by: Stephen Carlson <stcarlso@linux.microsoft.com>
> Signed-off-by: Sean Edmond <seanedmond@microsoft.com>
> ---
>  MAINTAINERS                         |  9 ++++
>  drivers/Kconfig                     |  2 +
>  drivers/Makefile                    |  1 +
>  drivers/rollback/Kconfig            | 25 +++++++++++
>  drivers/rollback/Makefile           |  6 +++
>  drivers/rollback/rollback-sandbox.c | 65 +++++++++++++++++++++++++++++
>  drivers/rollback/rollback-uclass.c  | 30 +++++++++++++
>  include/dm/uclass-id.h              |  1 +
>  include/rollback.h                  | 42 +++++++++++++++++++
>  9 files changed, 181 insertions(+)
>  create mode 100644 drivers/rollback/Kconfig
>  create mode 100644 drivers/rollback/Makefile
>  create mode 100644 drivers/rollback/rollback-sandbox.c
>  create mode 100644 drivers/rollback/rollback-uclass.c
>  create mode 100644 include/rollback.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bf851cffd6..de14724c27 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1438,6 +1438,15 @@ F:       cmd/seama.c
>  F:     doc/usage/cmd/seama.rst
>  F:     test/cmd/seama.c
>
> +ROLLBACK
> +M:     Stephen Carlson <stcarlso@linux.microsoft.com>
> +M:     Sean Edmond <seanedmond@microsoft.com>
> +S:     Maintained
> +F:     drivers/rollback/Kconfig
> +F:     drivers/rollback/Makefile
> +F:     drivers/rollback/rollback-sandbox.c
> +F:     drivers/rollback/rollback-uclass.c
> +
>  SEMIHOSTING
>  R:     Sean Anderson <sean.anderson@seco.com>
>  S:     Orphaned
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index a25f6ae02f..faa7cbb72b 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -116,6 +116,8 @@ source "drivers/rtc/Kconfig"
>
>  source "drivers/scsi/Kconfig"
>
> +source "drivers/rollback/Kconfig"
> +
>  source "drivers/serial/Kconfig"
>
>  source "drivers/smem/Kconfig"
> diff --git a/drivers/Makefile b/drivers/Makefile
> index efc2a4afb2..f6cfb48cb6 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -98,6 +98,7 @@ obj-$(CONFIG_PCH) += pch/
>  obj-$(CONFIG_DM_REBOOT_MODE) += reboot-mode/
>  obj-y += rtc/
>  obj-y += scsi/
> +obj-y += rollback/
>  obj-y += sound/
>  obj-y += spmi/
>  obj-y += watchdog/
> diff --git a/drivers/rollback/Kconfig b/drivers/rollback/Kconfig
> new file mode 100644
> index 0000000000..31f5a3f56d
> --- /dev/null
> +++ b/drivers/rollback/Kconfig
> @@ -0,0 +1,25 @@
> +config DM_ROLLBACK

Could this be ROLLBACK ? We try to use DM_ only for migrations.

> +       bool "Support rollback devices with driver model"
> +       depends on DM
> +       help
> +         This option enables support for the rollback uclass which supports
> +         devices intended to provide the anti-rollback feature during
> +         boot. These devices might encapsulate existing features of TPM
> +         or TEE devices, but can also be dedicated security processors
> +         implemented in specific hardware.
> +
> +config ROLLBACK_SANDBOX
> +       bool "Enable sandbox rollback driver"
> +       depends on DM_ROLLBACK
> +       help
> +         This driver supports a simulated rollback device that uses volatile
> +         memory to store secure data and begins uninitialized. This
> +         implementation allows OS images with security requirements to be
> +         loaded in the sandbox environment.
> +
> +config ROLLBACK_TPM
> +       bool "Enable TPM rollback driver"
> +       depends on TPM && TPM_V2 && DM_ROLLBACK
> +       help
> +         This driver supports a rollback device based on existing TPM
> +         functionality.
> diff --git a/drivers/rollback/Makefile b/drivers/rollback/Makefile
> new file mode 100644
> index 0000000000..4e7fa46041
> --- /dev/null
> +++ b/drivers/rollback/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# (C) Copyright 2021 Microsoft, Inc.
> +
> +obj-$(CONFIG_DM_ROLLBACK) += rollback-uclass.o
> +obj-$(CONFIG_ROLLBACK_SANDBOX) += rollback-sandbox.o
> diff --git a/drivers/rollback/rollback-sandbox.c b/drivers/rollback/rollback-sandbox.c
> new file mode 100644
> index 0000000000..acbe6d2303
> --- /dev/null
> +++ b/drivers/rollback/rollback-sandbox.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2021 Microsoft, Inc
> + * Written by Stephen Carlson <stcarlso@microsoft.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <fdtdec.h>

Is that needed?

> +#include <rollback.h>
> +
> +static struct rollback_state {
> +       u64 rollback_idx;
> +};
> +
> +static int sb_rollback_idx_get(struct udevice *dev, u64 *rollback_idx)
> +{
> +       struct rollback_state *priv = dev_get_priv(dev);
> +
> +       if (!rollback_idx)
> +               return -EINVAL;
> +
> +       *rollback_idx = priv->rollback_idx;
> +       return 0;
> +}
> +
> +static int sb_rollback_idx_set(struct udevice *dev, u64 rollback_idx)
> +{
> +       struct rollback_state *priv = dev_get_priv(dev);
> +       u64 old_rollback_idx;
> +
> +       old_rollback_idx = priv->rollback_idx;
> +       if (rollback_idx < old_rollback_idx)
> +               return -EPERM;
> +
> +       priv->rollback_idx = rollback_idx;
> +       return 0;
> +}
> +
> +static const struct rollback_ops rollback_sandbox_ops = {
> +       .rollback_idx_get               = sb_rollback_idx_get,
> +       .rollback_idx_set               = sb_rollback_idx_set,
> +};
> +
> +static int rollback_sandbox_probe(struct udevice *dev)
> +{
> +       struct rollback_state *priv = dev_get_priv(dev);
> +
> +       priv->rollback_idx = 0ULL;

Driver model sets private data to zeroes, so you can drop this.

> +       return 0;
> +}
> +
> +static const struct udevice_id rollback_sandbox_ids[] = {
> +       { .compatible = "sandbox,rollback" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(rollback_sandbox) = {
> +       .name   = "rollback_sandbox",
> +       .id     = UCLASS_ROLLBACK,
> +       .priv_auto = sizeof(struct rollback_state),
> +       .of_match = rollback_sandbox_ids,
> +       .probe  = rollback_sandbox_probe,
> +       .ops    = &rollback_sandbox_ops,
> +};
> diff --git a/drivers/rollback/rollback-uclass.c b/drivers/rollback/rollback-uclass.c
> new file mode 100644
> index 0000000000..1fc5486b0d
> --- /dev/null
> +++ b/drivers/rollback/rollback-uclass.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2021 Microsoft, Inc
> + * Written by Stephen Carlson <stcarlso@microsoft.com>
> + */
> +
> +#include <common.h>
> +#include <dm.h>
> +#include <rollback.h>
> +
> +int rollback_idx_get(struct udevice *dev, uint64_t *rollback_idx)
> +{
> +       if (!dev || !rollback_idx)
> +               return -EINVAL;

We don't normally do this sort of checking, due to code size. It isn't
valid to pass NULL here.

> +
> +       return rollback_get_ops(dev)->rollback_idx_get(dev, rollback_idx);
> +}
> +
> +int rollback_idx_set(struct udevice *dev, uint64_t rollback_idx)
> +{
> +       if (!dev)
> +               return -EINVAL;
> +
> +       return rollback_get_ops(dev)->rollback_idx_set(dev, rollback_idx);
> +}
> +
> +UCLASS_DRIVER(rollback) = {
> +       .id             = UCLASS_ROLLBACK,
> +       .name           = "rollback",
> +};
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 0432c95c9e..4b8b730d87 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -124,6 +124,7 @@ enum uclass_id {
>         UCLASS_RTC,             /* Real time clock device */
>         UCLASS_SCMI_AGENT,      /* Interface with an SCMI server */
>         UCLASS_SCSI,            /* SCSI device */
> +       UCLASS_ROLLBACK,        /* Rollback device */
>         UCLASS_SERIAL,          /* Serial UART */
>         UCLASS_SIMPLE_BUS,      /* Bus with child devices */
>         UCLASS_SMEM,            /* Shared memory interface */
> diff --git a/include/rollback.h b/include/rollback.h
> new file mode 100644
> index 0000000000..60dfd62dea
> --- /dev/null
> +++ b/include/rollback.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2021 Microsoft, Inc.
> + */
> +
> +#ifndef _ROLLBACK_H_
> +#define _ROLLBACK_H_
> +
> +#include <stdint.h>
> +
> +/* Access the rollback operations for a device */
> +#define rollback_get_ops(dev)  ((struct rollback_ops *)(dev)->driver->ops)
> +
> +/**
> + * rollback_idx_get() Gets the OS anti-rollback version number
> + *
> + * @dev:     Device to check
> + * @rollback_idx:   The retrieved anti-rollback version
> + * @return   0 if OK, -ve on error

Can you document -EPERM as well?

> + */
> +int rollback_idx_get(struct udevice *dev, uint64_t *rollback_idx);
> +
> +/**
> + * rollback_idx_set() Sets the OS anti-rollback version number.

How do we lock it so that it cannot be set? That would be needed
before booting the OS, wouldn't it?

> + *
> + * @dev:     Device to modify
> + * @rollback_idx:   The new anti-rollback version to set
> + * @return   0 if OK, -ve on error
> + */
> +int rollback_idx_set(struct udevice *dev, uint64_t rollback_idx);
> +
> +/**
> + * struct rollback_ops - Driver model rollback operations
> + *
> + * Refer to the functions above for the description of each operation.
> + */
> +struct rollback_ops {
> +       int (*rollback_idx_get)(struct udevice *dev, uint64_t *rollback_idx);
> +       int (*rollback_idx_set)(struct udevice *dev, uint64_t rollback_idx);

Please do duplicate the comments here. Also these methods should not
have a rollback_ prefix.

> +};
> +
> +#endif
> --
> 2.40.0
>

Regards,
Simon

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

end of thread, other threads:[~2023-12-01 18:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-12  9:47 [PATCH 0/5] Add anti-rollback validation feature seanedmond
2023-09-12  9:47 ` [PATCH 1/8] drivers: rollback: Add rollback devices to driver model seanedmond
2023-12-01 14:16   ` Ilias Apalodimas
2023-12-01 18:32   ` Simon Glass
2023-09-12  9:47 ` [PATCH 2/8] drivers: rollback: Add TPM2 implementation of rollback devices seanedmond
2023-12-01 14:52   ` Ilias Apalodimas
2023-12-01 18:32     ` Simon Glass
2023-09-12  9:47 ` [PATCH 3/8] common: Add OS anti-rollback validation using " seanedmond
2023-09-12  9:47 ` [PATCH 4/8] common: Add OS anti-rollback grace version seanedmond
2023-09-12  9:47 ` [PATCH 5/8] dm: test: Add a test for rollback driver seanedmond
2023-09-12  9:47 ` [PATCH 6/8] tpm: Fix issues relating to NV Indexes seanedmond
2023-09-12  9:47 ` [PATCH 7/8] sandbox: tpm: Fix TPM2_CC_NV_DEFINE_SPACE command seanedmond
2023-09-12  9:47 ` [PATCH 8/8] doc: rollback: anti-rollback verification seanedmond

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox