diff mbox series

[v15,07/16] acpi/nfit, libnvdimm: Add unlock of nvdimm support for Intel DIMMs

Message ID 154474491540.64529.6730683963766405362.stgit@djiang5-desk3.ch.intel.com (mailing list archive)
State New, archived
Headers show
Series Adding security support for nvdimm | expand

Commit Message

Dave Jiang Dec. 13, 2018, 11:48 p.m. UTC
From: Dan Williams <dan.j.williams@intel.com>

Add support to unlock the dimm via the kernel key management APIs. The
passphrase is expected to be pulled from userspace through keyutils.
The key management and sysfs attributes are libnvdimm generic.

Encrypted keys are used to protect the nvdimm passphrase at rest. The
master key can be a trusted-key sealed in a TPM, preferred, or an
encrypted-key, more flexible, but more exposure to a potential attacker.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Co-developed-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit/intel.c   |  109 ++++++++++++++++++++++++++++++++
 drivers/nvdimm/Kconfig      |    4 +
 drivers/nvdimm/Makefile     |    1 
 drivers/nvdimm/dimm.c       |   16 ++++-
 drivers/nvdimm/nd.h         |    8 ++
 drivers/nvdimm/security.c   |  148 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h   |   12 +++
 tools/testing/nvdimm/Kbuild |    1 
 8 files changed, 298 insertions(+), 1 deletion(-)
 create mode 100644 drivers/nvdimm/security.c

Comments

Elliott, Robert (Servers) Jan. 15, 2019, 9:56 p.m. UTC | #1
> -----Original Message-----
> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of Dave Jiang
> Sent: Thursday, December 13, 2018 5:49 PM
> To: dan.j.williams@intel.com
> Cc: linux-nvdimm@lists.01.org
> Subject: [PATCH v15 07/16] acpi/nfit, libnvdimm: Add unlock of nvdimm support for Intel DIMMs
> 
> From: Dan Williams <dan.j.williams@intel.com>
> 
...
> +static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
> +{
> +	struct device *dev = &nvdimm->dev;
> +	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
> +	struct key *key = NULL;
> +	int rc;
> +
> +	/* The bus lock should be held at the top level of the call stack */
> +	lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
> +
> +	if (!nvdimm->sec.ops || !nvdimm->sec.ops->unlock
> +			|| nvdimm->sec.state < 0)
> +		return -EIO;
> +
> +	/*
> +	 * If the pre-OS has unlocked the DIMM, attempt to send the key
> +	 * from request_key() to the hardware for verification.  Failure
> +	 * to revalidate the key against the hardware results in a
> +	 * freeze of the security configuration. I.e. if the OS does not
> +	 * have the key, security is being managed pre-OS.
> +	 */
> +	if (nvdimm->sec.state == NVDIMM_SECURITY_UNLOCKED) {
> +		if (!key_revalidate)
> +			return 0;
> +
> +		key = nvdimm_key_revalidate(nvdimm);
> +		if (!key)
> +			return nvdimm_security_freeze(nvdimm);

If it's already unlocked, regardless of whether nvdimm_key_revalidate()
succeeds or fails, I'm not sure you should call nvdimm_security_freeze().  

The kernel doesn't know the key, so a hands-off policy seems prudent -
don't send any commands that change the security state.  Some other
software (pre-OS or application) must be handling the feature, and
sending freeze lock might interfere with that software.

Maybe you intended that to be "if (key)", intending to freeze if the
kernel does have the correct key (so no application can change it and
render the kernel out of sync)?


> +	} else
> +		key = nvdimm_request_key(nvdimm);
> +
> +	if (!key)
> +		return -ENOKEY;
> +
> +	rc = nvdimm->sec.ops->unlock(nvdimm, key_data(key));
> +	dev_dbg(dev, "key: %d unlock: %s\n", key_serial(key),
> +			rc == 0 ? "success" : "fail");
> +
> +	nvdimm_put_key(key);
> +	nvdimm->sec.state = nvdimm_security_state(nvdimm);
> +	return rc;
> +}
> +
Dave Jiang Jan. 15, 2019, 10:35 p.m. UTC | #2
On 1/15/19 2:56 PM, Elliott, Robert (Persistent Memory) wrote:
> 
> 
>> -----Original Message-----
>> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of Dave Jiang
>> Sent: Thursday, December 13, 2018 5:49 PM
>> To: dan.j.williams@intel.com
>> Cc: linux-nvdimm@lists.01.org
>> Subject: [PATCH v15 07/16] acpi/nfit, libnvdimm: Add unlock of nvdimm support for Intel DIMMs
>>
>> From: Dan Williams <dan.j.williams@intel.com>
>>
> ...
>> +static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
>> +{
>> +struct device *dev = &nvdimm->dev;
>> +struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
>> +struct key *key = NULL;
>> +int rc;
>> +
>> +/* The bus lock should be held at the top level of the call stack */
>> +lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
>> +
>> +if (!nvdimm->sec.ops || !nvdimm->sec.ops->unlock
>> +|| nvdimm->sec.state < 0)
>> +return -EIO;
>> +
>> +/*
>> + * If the pre-OS has unlocked the DIMM, attempt to send the key
>> + * from request_key() to the hardware for verification.  Failure
>> + * to revalidate the key against the hardware results in a
>> + * freeze of the security configuration. I.e. if the OS does not
>> + * have the key, security is being managed pre-OS.
>> + */
>> +if (nvdimm->sec.state == NVDIMM_SECURITY_UNLOCKED) {
>> +if (!key_revalidate)
>> +return 0;
>> +
>> +key = nvdimm_key_revalidate(nvdimm);
>> +if (!key)
>> +return nvdimm_security_freeze(nvdimm);
> 
> If it's already unlocked, regardless of whether nvdimm_key_revalidate()
> succeeds or fails, I'm not sure you should call nvdimm_security_freeze().
> 
> The kernel doesn't know the key, so a hands-off policy seems prudent -
> don't send any commands that change the security state.  Some other
> software (pre-OS or application) must be handling the feature, and
> sending freeze lock might interfere with that software.

The intention is that if we don't have the key or the key verification
fails, we want to prevent any additional interaction with security for
this particular boot session. At least for the Intel implementation, a
reboot will clear the freeze.

> 
> Maybe you intended that to be "if (key)", intending to freeze if the
> kernel does have the correct key (so no application can change it and
> render the kernel out of sync)?
> 
> 
>> +} else
>> +key = nvdimm_request_key(nvdimm);
>> +
>> +if (!key)
>> +return -ENOKEY;
>> +
>> +rc = nvdimm->sec.ops->unlock(nvdimm, key_data(key));
>> +dev_dbg(dev, "key: %d unlock: %s\n", key_serial(key),
>> +rc == 0 ? "success" : "fail");
>> +
>> +nvdimm_put_key(key);
>> +nvdimm->sec.state = nvdimm_security_state(nvdimm);
>> +return rc;
>> +}
>> +
>
diff mbox series

Patch

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index f98d680d1a39..38f2cb364853 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -3,6 +3,7 @@ 
 #include <linux/libnvdimm.h>
 #include <linux/ndctl.h>
 #include <linux/acpi.h>
+#include <asm/smp.h>
 #include "intel.h"
 #include "nfit.h"
 
@@ -75,8 +76,116 @@  static int intel_security_freeze(struct nvdimm *nvdimm)
 	return 0;
 }
 
+static int intel_security_change_key(struct nvdimm *nvdimm,
+		const struct nvdimm_key_data *old_data,
+		const struct nvdimm_key_data *new_data)
+{
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_set_passphrase cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_SET_PASSPHRASE,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE * 2,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+		},
+	};
+	int rc;
+
+	if (!test_bit(NVDIMM_INTEL_SET_PASSPHRASE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	if (old_data)
+		memcpy(nd_cmd.cmd.old_pass, old_data->data,
+				sizeof(nd_cmd.cmd.old_pass));
+	memcpy(nd_cmd.cmd.new_pass, new_data->data,
+			sizeof(nd_cmd.cmd.new_pass));
+	rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
+	if (rc < 0)
+		return rc;
+
+	switch (nd_cmd.cmd.status) {
+	case 0:
+		return 0;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		return -EINVAL;
+	case ND_INTEL_STATUS_NOT_SUPPORTED:
+		return -EOPNOTSUPP;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		return -EIO;
+	}
+}
+
+static void nvdimm_invalidate_cache(void);
+
+static int intel_security_unlock(struct nvdimm *nvdimm,
+		const struct nvdimm_key_data *key_data)
+{
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_unlock_unit cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_UNLOCK_UNIT,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+		},
+	};
+	int rc;
+
+	if (!test_bit(NVDIMM_INTEL_UNLOCK_UNIT, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	memcpy(nd_cmd.cmd.passphrase, key_data->data,
+			sizeof(nd_cmd.cmd.passphrase));
+	rc = nvdimm_ctl(nvdimm, ND_CMD_CALL, &nd_cmd, sizeof(nd_cmd), NULL);
+	if (rc < 0)
+		return rc;
+	switch (nd_cmd.cmd.status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		return -EINVAL;
+	default:
+		return -EIO;
+	}
+
+	/* DIMM unlocked, invalidate all CPU caches before we read it */
+	nvdimm_invalidate_cache();
+
+	return 0;
+}
+
+/*
+ * TODO: define a cross arch wbinvd equivalent when/if
+ * NVDIMM_FAMILY_INTEL command support arrives on another arch.
+ */
+#ifdef CONFIG_X86
+static void nvdimm_invalidate_cache(void)
+{
+	wbinvd_on_all_cpus();
+}
+#else
+static void nvdimm_invalidate_cache(void)
+{
+	WARN_ON_ONCE("cache invalidation required after unlock\n");
+}
+#endif
+
 static const struct nvdimm_security_ops __intel_security_ops = {
 	.state = intel_security_state,
 	.freeze = intel_security_freeze,
+	.change_key = intel_security_change_key,
+#ifdef CONFIG_X86
+	.unlock = intel_security_unlock,
+#endif
 };
+
 const struct nvdimm_security_ops *intel_security_ops = &__intel_security_ops;
diff --git a/drivers/nvdimm/Kconfig b/drivers/nvdimm/Kconfig
index 9d36473dc2a2..00f6325928f6 100644
--- a/drivers/nvdimm/Kconfig
+++ b/drivers/nvdimm/Kconfig
@@ -112,4 +112,8 @@  config OF_PMEM
 
 	  Select Y if unsure.
 
+config NVDIMM_KEYS
+        def_bool y
+        depends on KEYS
+
 endif
diff --git a/drivers/nvdimm/Makefile b/drivers/nvdimm/Makefile
index e8847045dac0..6f2a088afad6 100644
--- a/drivers/nvdimm/Makefile
+++ b/drivers/nvdimm/Makefile
@@ -27,3 +27,4 @@  libnvdimm-$(CONFIG_ND_CLAIM) += claim.o
 libnvdimm-$(CONFIG_BTT) += btt_devs.o
 libnvdimm-$(CONFIG_NVDIMM_PFN) += pfn_devs.o
 libnvdimm-$(CONFIG_NVDIMM_DAX) += dax_devs.o
+libnvdimm-$(CONFIG_NVDIMM_KEYS) += security.o
diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
index 9899c97138a3..1b3d9e7b2ffe 100644
--- a/drivers/nvdimm/dimm.c
+++ b/drivers/nvdimm/dimm.c
@@ -34,7 +34,11 @@  static int nvdimm_probe(struct device *dev)
 		return rc;
 	}
 
-	/* reset locked, to be validated below... */
+	/*
+	 * The locked status bit reflects explicit status codes from the
+	 * label reading commands, revalidate it each time the driver is
+	 * activated and re-reads the label area.
+	 */
 	nvdimm_clear_locked(dev);
 
 	ndd = kzalloc(sizeof(*ndd), GFP_KERNEL);
@@ -51,6 +55,16 @@  static int nvdimm_probe(struct device *dev)
 	get_device(dev);
 	kref_init(&ndd->kref);
 
+	/*
+	 * Attempt to unlock, if the DIMM supports security commands,
+	 * otherwise the locked indication is determined by explicit
+	 * status codes from the label reading commands.
+	 */
+	rc = nvdimm_security_unlock(dev);
+	if (rc < 0)
+		dev_err(dev, "failed to unlock dimm: %d\n", rc);
+
+
 	/*
 	 * EACCES failures reading the namespace label-area-properties
 	 * are interpreted as the DIMM capacity being locked but the
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index e79cc8e5c114..cfde992684e7 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -250,6 +250,14 @@  long nvdimm_clear_poison(struct device *dev, phys_addr_t phys,
 void nvdimm_set_aliasing(struct device *dev);
 void nvdimm_set_locked(struct device *dev);
 void nvdimm_clear_locked(struct device *dev);
+#if IS_ENABLED(CONFIG_NVDIMM_KEYS)
+int nvdimm_security_unlock(struct device *dev);
+#else
+static inline int nvdimm_security_unlock(struct device *dev)
+{
+	return 0;
+}
+#endif
 struct nd_btt *to_nd_btt(struct device *dev);
 
 struct nd_gen_sb {
diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c
new file mode 100644
index 000000000000..51d77a67a9fb
--- /dev/null
+++ b/drivers/nvdimm/security.c
@@ -0,0 +1,148 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/ndctl.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/mm.h>
+#include <linux/cred.h>
+#include <linux/key.h>
+#include <linux/key-type.h>
+#include <keys/user-type.h>
+#include <keys/encrypted-type.h>
+#include "nd-core.h"
+#include "nd.h"
+
+static bool key_revalidate = true;
+module_param(key_revalidate, bool, 0444);
+MODULE_PARM_DESC(key_revalidate, "Require key validation at init.");
+
+static void *key_data(struct key *key)
+{
+	struct encrypted_key_payload *epayload = dereference_key_locked(key);
+
+	lockdep_assert_held_read(&key->sem);
+
+	return epayload->decrypted_data;
+}
+
+static void nvdimm_put_key(struct key *key)
+{
+	up_read(&key->sem);
+	key_put(key);
+}
+
+/*
+ * Retrieve kernel key for DIMM and request from user space if
+ * necessary. Returns a key held for read and must be put by
+ * nvdimm_put_key() before the usage goes out of scope.
+ */
+static struct key *nvdimm_request_key(struct nvdimm *nvdimm)
+{
+	struct key *key = NULL;
+	static const char NVDIMM_PREFIX[] = "nvdimm:";
+	char desc[NVDIMM_KEY_DESC_LEN + sizeof(NVDIMM_PREFIX)];
+	struct device *dev = &nvdimm->dev;
+
+	sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id);
+	key = request_key(&key_type_encrypted, desc, "");
+	if (IS_ERR(key)) {
+		if (PTR_ERR(key) == -ENOKEY)
+			dev_warn(dev, "request_key() found no key\n");
+		else
+			dev_warn(dev, "request_key() upcall failed\n");
+		key = NULL;
+	} else {
+		struct encrypted_key_payload *epayload;
+
+		down_read(&key->sem);
+		epayload = dereference_key_locked(key);
+		if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) {
+			up_read(&key->sem);
+			key_put(key);
+			key = NULL;
+		}
+	}
+
+	return key;
+}
+
+static struct key *nvdimm_key_revalidate(struct nvdimm *nvdimm)
+{
+	struct key *key;
+	int rc;
+
+	if (!nvdimm->sec.ops->change_key)
+		return NULL;
+
+	key = nvdimm_request_key(nvdimm);
+	if (!key)
+		return NULL;
+
+	/*
+	 * Send the same key to the hardware as new and old key to
+	 * verify that the key is good.
+	 */
+	rc = nvdimm->sec.ops->change_key(nvdimm, key_data(key), key_data(key));
+	if (rc < 0) {
+		nvdimm_put_key(key);
+		key = NULL;
+	}
+	return key;
+}
+
+static int __nvdimm_security_unlock(struct nvdimm *nvdimm)
+{
+	struct device *dev = &nvdimm->dev;
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key = NULL;
+	int rc;
+
+	/* The bus lock should be held at the top level of the call stack */
+	lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
+
+	if (!nvdimm->sec.ops || !nvdimm->sec.ops->unlock
+			|| nvdimm->sec.state < 0)
+		return -EIO;
+
+	/*
+	 * If the pre-OS has unlocked the DIMM, attempt to send the key
+	 * from request_key() to the hardware for verification.  Failure
+	 * to revalidate the key against the hardware results in a
+	 * freeze of the security configuration. I.e. if the OS does not
+	 * have the key, security is being managed pre-OS.
+	 */
+	if (nvdimm->sec.state == NVDIMM_SECURITY_UNLOCKED) {
+		if (!key_revalidate)
+			return 0;
+
+		key = nvdimm_key_revalidate(nvdimm);
+		if (!key)
+			return nvdimm_security_freeze(nvdimm);
+	} else
+		key = nvdimm_request_key(nvdimm);
+
+	if (!key)
+		return -ENOKEY;
+
+	rc = nvdimm->sec.ops->unlock(nvdimm, key_data(key));
+	dev_dbg(dev, "key: %d unlock: %s\n", key_serial(key),
+			rc == 0 ? "success" : "fail");
+
+	nvdimm_put_key(key);
+	nvdimm->sec.state = nvdimm_security_state(nvdimm);
+	return rc;
+}
+
+int nvdimm_security_unlock(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	int rc;
+
+	nvdimm_bus_lock(dev);
+	rc = __nvdimm_security_unlock(nvdimm);
+	nvdimm_bus_unlock(dev);
+	return rc;
+}
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 42c815f97c02..0f0ab276134e 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -163,9 +163,21 @@  enum nvdimm_security_state {
 	NVDIMM_SECURITY_OVERWRITE,
 };
 
+#define NVDIMM_PASSPHRASE_LEN		32
+#define NVDIMM_KEY_DESC_LEN		22
+
+struct nvdimm_key_data {
+	u8 data[NVDIMM_PASSPHRASE_LEN];
+};
+
 struct nvdimm_security_ops {
 	enum nvdimm_security_state (*state)(struct nvdimm *nvdimm);
 	int (*freeze)(struct nvdimm *nvdimm);
+	int (*change_key)(struct nvdimm *nvdimm,
+			const struct nvdimm_key_data *old_data,
+			const struct nvdimm_key_data *new_data);
+	int (*unlock)(struct nvdimm *nvdimm,
+			const struct nvdimm_key_data *key_data);
 };
 
 void badrange_init(struct badrange *badrange);
diff --git a/tools/testing/nvdimm/Kbuild b/tools/testing/nvdimm/Kbuild
index 4a2f3cff2a75..33ea40777205 100644
--- a/tools/testing/nvdimm/Kbuild
+++ b/tools/testing/nvdimm/Kbuild
@@ -80,6 +80,7 @@  libnvdimm-$(CONFIG_ND_CLAIM) += $(NVDIMM_SRC)/claim.o
 libnvdimm-$(CONFIG_BTT) += $(NVDIMM_SRC)/btt_devs.o
 libnvdimm-$(CONFIG_NVDIMM_PFN) += $(NVDIMM_SRC)/pfn_devs.o
 libnvdimm-$(CONFIG_NVDIMM_DAX) += $(NVDIMM_SRC)/dax_devs.o
+libnvdimm-$(CONFIG_NVDIMM_KEYS) += $(NVDIMM_SRC)/security.o
 libnvdimm-y += libnvdimm_test.o
 libnvdimm-y += config_check.o