nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] Adding security support for nvdimm
@ 2018-07-02 23:39 Dave Jiang
  2018-07-02 23:39 ` [PATCH 01/11] nfit: adding support for Intel DSM 1.7 commands Dave Jiang
                   ` (10 more replies)
  0 siblings, 11 replies; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

The following series implements security support for nvdimm. Mostly adding
new security DSM support from the Intel NVDIMM DSM spec v1.7, but also
adding generic support libnvdimm for other vendors. The most important
security features are unlocking locked nvdimms, and updating/setting security
passphrase to nvdimms.

Security folks, thanks in advance for taking a look at my key management
implementation and making sure that I'm doing something sane. Mainly you'll
want to review patches 2, 4, and 5 as most relevant ones that need scrutiny.

---

Dave Jiang (11):
      nfit: adding support for Intel DSM 1.7 commands
      libnvdimm: create keyring to store security keys
      nfit/libnvdimm: store dimm id as a member to struct nvdimm
      nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
      nfit/libnvdimm: add set passphrase support for Intel nvdimms
      nfit/libnvdimm: add disable passphrase support to Intel nvdimm.
      nfit/libnvdimm: add freeze security support to Intel nvdimm
      nfit/libnvdimm: add support for issue secure erase DSM to Intel nvdimm
      nfit_test: adding context to dimm_dev for nfit_test
      nfit_test: adding test support for Intel nvdimm security DSMs
      libnvdimm: adding documentation for nvdimm security support


 Documentation/nvdimm/security    |   70 ++++++++
 drivers/acpi/nfit/Makefile       |    2 
 drivers/acpi/nfit/core.c         |   61 +++++--
 drivers/acpi/nfit/intel.c        |  348 ++++++++++++++++++++++++++++++++++++++
 drivers/acpi/nfit/intel.h        |  104 +++++++++++
 drivers/acpi/nfit/nfit.h         |   24 ---
 drivers/nvdimm/bus.c             |    2 
 drivers/nvdimm/dimm.c            |   97 +++++++++++
 drivers/nvdimm/dimm_devs.c       |  332 ++++++++++++++++++++++++++++++++++++
 drivers/nvdimm/nd-core.h         |    4 
 drivers/nvdimm/nd.h              |    2 
 include/linux/libnvdimm.h        |   35 ++++
 tools/testing/nvdimm/Kbuild      |    1 
 tools/testing/nvdimm/test/nfit.c |  226 +++++++++++++++++++++++--
 14 files changed, 1254 insertions(+), 54 deletions(-)
 create mode 100644 Documentation/nvdimm/security
 create mode 100644 drivers/acpi/nfit/intel.c
 create mode 100644 drivers/acpi/nfit/intel.h

--
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 01/11] nfit: adding support for Intel DSM 1.7 commands
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
@ 2018-07-02 23:39 ` Dave Jiang
  2018-07-02 23:49   ` Dan Williams
  2018-07-02 23:39 ` [PATCH 02/11] libnvdimm: create keyring to store security keys Dave Jiang
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

Adding command definition for security commands defined in Intel DSM
specification v1.7. This includes "get security state", "set passphrase",
"unlock unit", "freeze lock", "secure erase", "ovewrite", and
"overwrite query". Since we are adding a lot of Intel definitions, moving
the relevant bits to its own header. Also, we don't want those commands
to be issued from user space through ioctls. Reason being some of the
security commands require kernel involvement to flush all CPU caches that
can't be done in userspace and the result can cause system crash. So
blocking security commands in the ioctl path.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/nfit/core.c  |   27 +++++++++++-
 drivers/acpi/nfit/intel.h |  102 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/nfit/nfit.h  |   24 -----------
 drivers/nvdimm/bus.c      |    2 -
 include/linux/libnvdimm.h |    2 -
 5 files changed, 131 insertions(+), 26 deletions(-)
 create mode 100644 drivers/acpi/nfit/intel.h

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 2be8373153ed..70351b610b3d 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -377,6 +377,14 @@ static u8 nfit_dsm_revid(unsigned family, unsigned func)
 			[NVDIMM_INTEL_QUERY_FWUPDATE] = 2,
 			[NVDIMM_INTEL_SET_THRESHOLD] = 2,
 			[NVDIMM_INTEL_INJECT_ERROR] = 2,
+			[NVDIMM_INTEL_GET_SECURITY_STATE] = 2,
+			[NVDIMM_INTEL_SET_PASSPHRASE] = 2,
+			[NVDIMM_INTEL_DISABLE_PASSPHRASE] = 2,
+			[NVDIMM_INTEL_UNLOCK_UNIT] = 2,
+			[NVDIMM_INTEL_FREEZE_LOCK] = 2,
+			[NVDIMM_INTEL_SECURE_ERASE] = 2,
+			[NVDIMM_INTEL_OVERWRITE] = 2,
+			[NVDIMM_INTEL_QUERY_OVERWRITE] = 2,
 		},
 	};
 	u8 id;
@@ -3200,7 +3208,7 @@ static int acpi_nfit_flush_probe(struct nvdimm_bus_descriptor *nd_desc)
 	return 0;
 }
 
-static int acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
+static int __acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
 		struct nvdimm *nvdimm, unsigned int cmd)
 {
 	struct acpi_nfit_desc *acpi_desc = to_acpi_nfit_desc(nd_desc);
@@ -3222,6 +3230,23 @@ static int acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
 	return 0;
 }
 
+/* prevent security commands from being issued via ioctl */
+static int acpi_nfit_clear_to_send(struct nvdimm_bus_descriptor *nd_desc,
+		struct nvdimm *nvdimm, unsigned int cmd, void *buf)
+{
+	struct nd_cmd_pkg *call_pkg = buf;
+	unsigned int func;
+
+	if (nvdimm && cmd == ND_CMD_CALL &&
+			call_pkg->nd_family == NVDIMM_FAMILY_INTEL) {
+		func = call_pkg->nd_command;
+		if ((1 << func) & NVDIMM_INTEL_SECURITY_CMDMASK)
+			return -EOPNOTSUPP;
+	}
+
+	return __acpi_nfit_clear_to_send(nd_desc, nvdimm, cmd);
+}
+
 int acpi_nfit_ars_rescan(struct acpi_nfit_desc *acpi_desc, unsigned long flags)
 {
 	struct device *dev = acpi_desc->dev;
diff --git a/drivers/acpi/nfit/intel.h b/drivers/acpi/nfit/intel.h
new file mode 100644
index 000000000000..a351f451b42a
--- /dev/null
+++ b/drivers/acpi/nfit/intel.h
@@ -0,0 +1,102 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+/*
+ * Intel specific definitions for NVDIMM Firmware Interface Table - NFIT
+ */
+#ifndef _NFIT_INTEL_H_
+#define _NFIT_INTEL_H_
+
+/*
+ * Command numbers that the kernel needs to know about to handle
+ * non-default DSM revision ids
+ */
+enum nvdimm_family_cmds {
+	NVDIMM_INTEL_LATCH_SHUTDOWN = 10,
+	NVDIMM_INTEL_GET_MODES = 11,
+	NVDIMM_INTEL_GET_FWINFO = 12,
+	NVDIMM_INTEL_START_FWUPDATE = 13,
+	NVDIMM_INTEL_SEND_FWUPDATE = 14,
+	NVDIMM_INTEL_FINISH_FWUPDATE = 15,
+	NVDIMM_INTEL_QUERY_FWUPDATE = 16,
+	NVDIMM_INTEL_SET_THRESHOLD = 17,
+	NVDIMM_INTEL_INJECT_ERROR = 18,
+	NVDIMM_INTEL_GET_SECURITY_STATE = 19,
+	NVDIMM_INTEL_SET_PASSPHRASE = 20,
+	NVDIMM_INTEL_DISABLE_PASSPHRASE = 21,
+	NVDIMM_INTEL_UNLOCK_UNIT = 22,
+	NVDIMM_INTEL_FREEZE_LOCK = 23,
+	NVDIMM_INTEL_SECURE_ERASE = 24,
+	NVDIMM_INTEL_OVERWRITE = 25,
+	NVDIMM_INTEL_QUERY_OVERWRITE = 26,
+};
+
+#define NVDIMM_INTEL_SECURITY_CMDMASK \
+(1 << NVDIMM_INTEL_GET_SECURITY_STATE | 1 << NVDIMM_INTEL_SET_PASSPHRASE \
+| 1 << NVDIMM_INTEL_DISABLE_PASSPHRASE | 1 << NVDIMM_INTEL_UNLOCK_UNIT \
+| 1 << NVDIMM_INTEL_FREEZE_LOCK | 1 << NVDIMM_INTEL_SECURE_ERASE \
+| 1 << NVDIMM_INTEL_OVERWRITE | 1 << NVDIMM_INTEL_QUERY_OVERWRITE)
+
+#define NVDIMM_INTEL_CMDMASK \
+(NVDIMM_STANDARD_CMDMASK | 1 << NVDIMM_INTEL_GET_MODES \
+| 1 << NVDIMM_INTEL_GET_FWINFO | 1 << NVDIMM_INTEL_START_FWUPDATE \
+| 1 << NVDIMM_INTEL_SEND_FWUPDATE | 1 << NVDIMM_INTEL_FINISH_FWUPDATE \
+| 1 << NVDIMM_INTEL_QUERY_FWUPDATE | 1 << NVDIMM_INTEL_SET_THRESHOLD \
+| 1 << NVDIMM_INTEL_INJECT_ERROR | 1 << NVDIMM_INTEL_LATCH_SHUTDOWN \
+| NVDIMM_INTEL_SECURITY_CMDMASK)
+
+#define ND_INTEL_STATUS_SIZE		4
+#define ND_INTEL_PASSPHRASE_SIZE	32
+
+#define ND_INTEL_STATUS_RETRY		5
+#define ND_INTEL_STATUS_NOT_READY	9
+#define ND_INTEL_STATUS_INVALID_STATE	10
+#define ND_INTEL_STATUS_INVALID_PASS	11
+
+#define ND_INTEL_SEC_STATE_ENABLED	0x02
+#define ND_INTEL_SEC_STATE_LOCKED	0x04
+#define ND_INTEL_SEC_STATE_FROZEN	0x08
+#define ND_INTEL_SEC_STATE_PLIMIT	0x10
+#define ND_INTEL_SEC_STATE_UNSUPPORTED	0x20
+
+struct nd_intel_get_security_state {
+	u32 status;
+	u32 reserved;
+	u8 state;
+	u8 reserved1[3];
+} __packed;
+
+struct nd_intel_set_passphrase {
+	u8 old_pass[32];
+	u8 new_pass[32];
+	u32 status;
+} __packed;
+
+struct nd_intel_unlock_unit {
+	u8 passphrase[32];
+	u32 status;
+} __packed;
+
+struct nd_intel_disable_passphrase {
+	u8 passphrase[32];
+	u32 status;
+} __packed;
+
+struct nd_intel_freeze_lock {
+	u32 status;
+} __packed;
+
+struct nd_intel_secure_erase {
+	u8 passphrase[32];
+	u32 status;
+} __packed;
+
+struct nd_intel_overwrite {
+	u8 passphrase[32];
+	u32 status;
+} __packed;
+
+struct nd_intel_query_overwrite {
+	u32 status;
+} __packed;
+
+#endif
diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
index 7d15856a739f..3b3f59828632 100644
--- a/drivers/acpi/nfit/nfit.h
+++ b/drivers/acpi/nfit/nfit.h
@@ -20,6 +20,7 @@
 #include <linux/types.h>
 #include <linux/acpi.h>
 #include <acpi/acuuid.h>
+#include "intel.h"
 
 /* ACPI 6.1 */
 #define UUID_NFIT_BUS "2f10e7a4-9e91-11e4-89d3-123b93f75cba"
@@ -46,29 +47,6 @@
  | 1 << ND_CMD_SET_CONFIG_DATA | 1 << ND_CMD_VENDOR_EFFECT_LOG_SIZE \
  | 1 << ND_CMD_VENDOR_EFFECT_LOG | 1 << ND_CMD_VENDOR)
 
-/*
- * Command numbers that the kernel needs to know about to handle
- * non-default DSM revision ids
- */
-enum nvdimm_family_cmds {
-	NVDIMM_INTEL_LATCH_SHUTDOWN = 10,
-	NVDIMM_INTEL_GET_MODES = 11,
-	NVDIMM_INTEL_GET_FWINFO = 12,
-	NVDIMM_INTEL_START_FWUPDATE = 13,
-	NVDIMM_INTEL_SEND_FWUPDATE = 14,
-	NVDIMM_INTEL_FINISH_FWUPDATE = 15,
-	NVDIMM_INTEL_QUERY_FWUPDATE = 16,
-	NVDIMM_INTEL_SET_THRESHOLD = 17,
-	NVDIMM_INTEL_INJECT_ERROR = 18,
-};
-
-#define NVDIMM_INTEL_CMDMASK \
-(NVDIMM_STANDARD_CMDMASK | 1 << NVDIMM_INTEL_GET_MODES \
- | 1 << NVDIMM_INTEL_GET_FWINFO | 1 << NVDIMM_INTEL_START_FWUPDATE \
- | 1 << NVDIMM_INTEL_SEND_FWUPDATE | 1 << NVDIMM_INTEL_FINISH_FWUPDATE \
- | 1 << NVDIMM_INTEL_QUERY_FWUPDATE | 1 << NVDIMM_INTEL_SET_THRESHOLD \
- | 1 << NVDIMM_INTEL_INJECT_ERROR | 1 << NVDIMM_INTEL_LATCH_SHUTDOWN)
-
 enum nfit_uuids {
 	/* for simplicity alias the uuid index with the family id */
 	NFIT_DEV_DIMM = NVDIMM_FAMILY_INTEL,
diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 27902a8799b1..de2af49bbf08 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -894,7 +894,7 @@ static int nd_cmd_clear_to_send(struct nvdimm_bus *nvdimm_bus,
 
 	/* ask the bus provider if it would like to block this request */
 	if (nd_desc->clear_to_send) {
-		int rc = nd_desc->clear_to_send(nd_desc, nvdimm, cmd);
+		int rc = nd_desc->clear_to_send(nd_desc, nvdimm, cmd, data);
 
 		if (rc)
 			return rc;
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 097072c5a852..472171af7f60 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -87,7 +87,7 @@ struct nvdimm_bus_descriptor {
 	ndctl_fn ndctl;
 	int (*flush_probe)(struct nvdimm_bus_descriptor *nd_desc);
 	int (*clear_to_send)(struct nvdimm_bus_descriptor *nd_desc,
-			struct nvdimm *nvdimm, unsigned int cmd);
+			struct nvdimm *nvdimm, unsigned int cmd, void *data);
 };
 
 struct nd_cmd_desc {

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 02/11] libnvdimm: create keyring to store security keys
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
  2018-07-02 23:39 ` [PATCH 01/11] nfit: adding support for Intel DSM 1.7 commands Dave Jiang
@ 2018-07-02 23:39 ` Dave Jiang
  2018-07-03 22:16   ` Dan Williams
  2018-07-02 23:39 ` [PATCH 03/11] nfit/libnvdimm: store dimm id as a member to struct nvdimm Dave Jiang
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

Prepping the libnvdimm to support security management by adding a keyring
in order to provide passphrase management through the kernel key management
APIs.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/nvdimm/dimm.c     |   90 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h |    5 +++
 2 files changed, 95 insertions(+)

diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
index 6c8fb7590838..ee0c68efa82a 100644
--- a/drivers/nvdimm/dimm.c
+++ b/drivers/nvdimm/dimm.c
@@ -18,9 +18,46 @@
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/nd.h>
+#include <linux/cred.h>
+#include <keys/user-type.h>
+#include <linux/key-type.h>
+#include <linux/keyctl.h>
 #include "label.h"
 #include "nd.h"
 
+const struct cred *nvdimm_cred;
+static int nvdimm_key_instantiate(struct key *key,
+		struct key_preparsed_payload *prep);
+static void nvdimm_key_destroy(struct key *key);
+
+struct key_type nvdimm_key_type = {
+	.name = "nvdimm",
+	.instantiate = nvdimm_key_instantiate,
+	.destroy = nvdimm_key_destroy,
+	.describe = user_describe,
+	.def_datalen = NVDIMM_DEFAULT_PASSPHRASE_LEN * 2,
+};
+
+static int nvdimm_key_instantiate(struct key *key,
+		struct key_preparsed_payload *prep)
+{
+	char *payload;
+
+	payload = kzalloc(nvdimm_key_type.def_datalen, GFP_KERNEL);
+	if (!payload)
+		return -ENOMEM;
+
+	key->datalen = min(nvdimm_key_type.def_datalen, prep->datalen);
+	memcpy(payload, prep->data, key->datalen);
+	key->payload.data[0] = payload;
+	return 0;
+}
+
+static void nvdimm_key_destroy(struct key *key)
+{
+	kfree(key->payload.data[0]);
+}
+
 static int nvdimm_probe(struct device *dev)
 {
 	struct nvdimm_drvdata *ndd;
@@ -129,13 +166,66 @@ static struct nd_device_driver nvdimm_driver = {
 	.type = ND_DRIVER_DIMM,
 };
 
+static int nvdimm_register_keyring(void)
+{
+	struct cred *cred;
+	struct key *keyring;
+	int rc;
+
+	rc = register_key_type(&nvdimm_key_type);
+	if (rc < 0)
+		return rc;
+
+	cred = prepare_kernel_cred(NULL);
+	if (!cred) {
+		rc = -ENOMEM;
+		goto failed_cred;
+	}
+
+	keyring = keyring_alloc(".nvdimm",
+			GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred,
+			(KEY_POS_ALL & ~KEY_POS_SETATTR) |
+			(KEY_USR_ALL & ~KEY_USR_SETATTR),
+			KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
+	if (IS_ERR(keyring)) {
+		rc = PTR_ERR(keyring);
+		goto failed_keyring;
+	}
+
+	set_bit(KEY_FLAG_ROOT_CAN_CLEAR, &keyring->flags);
+	cred->thread_keyring = keyring;
+	cred->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING;
+	nvdimm_cred = cred;
+	return 0;
+
+ failed_cred:
+	unregister_key_type(&nvdimm_key_type);
+ failed_keyring:
+	put_cred(cred);
+	return rc;
+}
+
+static void nvdimm_unregister_keyring(void)
+{
+	key_revoke(nvdimm_cred->thread_keyring);
+	unregister_key_type(&nvdimm_key_type);
+	put_cred(nvdimm_cred);
+}
+
 int __init nvdimm_init(void)
 {
+	int rc;
+
+	rc = nvdimm_register_keyring();
+	if (rc < 0)
+		return rc;
+
 	return nd_driver_register(&nvdimm_driver);
 }
 
 void nvdimm_exit(void)
 {
+	nvdimm_unregister_keyring();
 	driver_unregister(&nvdimm_driver.drv);
 }
 
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 472171af7f60..a250ff2a30df 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -155,6 +155,11 @@ static inline struct nd_blk_region_desc *to_blk_region_desc(
 
 }
 
+extern struct key_type nvdimm_key_type;
+
+#define NVDIMM_DEFAULT_PASSPHRASE_LEN		32
+#define NVDIMM_DEFAULT_DESC_LEN			32
+
 void badrange_init(struct badrange *badrange);
 int badrange_add(struct badrange *badrange, u64 addr, u64 length);
 void badrange_forget(struct badrange *badrange, phys_addr_t start,

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 03/11] nfit/libnvdimm: store dimm id as a member to struct nvdimm
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
  2018-07-02 23:39 ` [PATCH 01/11] nfit: adding support for Intel DSM 1.7 commands Dave Jiang
  2018-07-02 23:39 ` [PATCH 02/11] libnvdimm: create keyring to store security keys Dave Jiang
@ 2018-07-02 23:39 ` Dave Jiang
  2018-07-03  1:04   ` Dan Williams
  2018-07-02 23:39 ` [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs Dave Jiang
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

The generated dimm id is needed for the sysfs attribute as well as being
used as the identifier/description for the security key. Since it's
constant and should never change, store it as a member of struct nvdimm.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/nfit/core.c   |   33 ++++++++++++++++++++++-----------
 drivers/nvdimm/dimm_devs.c |    4 +++-
 drivers/nvdimm/nd-core.h   |    1 +
 include/linux/libnvdimm.h  |    2 +-
 4 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index 70351b610b3d..a6fb336da79d 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -70,6 +70,9 @@ struct nfit_table_prev {
 
 static guid_t nfit_uuid[NFIT_UUID_MAX];
 
+static int acpi_nfit_get_dimm_id(struct acpi_nfit_control_region *dcr,
+		char *buf);
+
 const guid_t *to_nfit_uuid(enum nfit_uuids id)
 {
 	return &nfit_uuid[id];
@@ -1571,16 +1574,7 @@ static ssize_t id_show(struct device *dev,
 {
 	struct acpi_nfit_control_region *dcr = to_nfit_dcr(dev);
 
-	if (dcr->valid_fields & ACPI_NFIT_CONTROL_MFG_INFO_VALID)
-		return sprintf(buf, "%04x-%02x-%04x-%08x\n",
-				be16_to_cpu(dcr->vendor_id),
-				dcr->manufacturing_location,
-				be16_to_cpu(dcr->manufacturing_date),
-				be32_to_cpu(dcr->serial_number));
-	else
-		return sprintf(buf, "%04x-%08x\n",
-				be16_to_cpu(dcr->vendor_id),
-				be32_to_cpu(dcr->serial_number));
+	return acpi_nfit_get_dimm_id(dcr, buf);
 }
 static DEVICE_ATTR_RO(id);
 
@@ -1827,6 +1821,21 @@ static void shutdown_dimm_notify(void *data)
 	mutex_unlock(&acpi_desc->init_mutex);
 }
 
+static int acpi_nfit_get_dimm_id(struct acpi_nfit_control_region *dcr,
+		char *buf)
+{
+	if (dcr->valid_fields & ACPI_NFIT_CONTROL_MFG_INFO_VALID)
+		return sprintf(buf, "%04x-%02x-%04x-%08x",
+				be16_to_cpu(dcr->vendor_id),
+				dcr->manufacturing_location,
+				be16_to_cpu(dcr->manufacturing_date),
+				be32_to_cpu(dcr->serial_number));
+	else
+		return sprintf(buf, "%04x-%08x",
+				be16_to_cpu(dcr->vendor_id),
+				be32_to_cpu(dcr->serial_number));
+}
+
 static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
 {
 	struct nfit_mem *nfit_mem;
@@ -1839,6 +1848,7 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
 		struct nfit_memdev *nfit_memdev;
 		u32 device_handle;
 		u16 mem_flags;
+		char dimm_id[32];
 
 		device_handle = __to_nfit_memdev(nfit_mem)->device_handle;
 		nvdimm = acpi_nfit_dimm_by_handle(acpi_desc, device_handle);
@@ -1893,10 +1903,11 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
 
 		flush = nfit_mem->nfit_flush ? nfit_mem->nfit_flush->flush
 			: NULL;
+		acpi_nfit_get_dimm_id(nfit_mem->dcr, dimm_id);
 		nvdimm = nvdimm_create(acpi_desc->nvdimm_bus, nfit_mem,
 				acpi_nfit_dimm_attribute_groups,
 				flags, cmd_mask, flush ? flush->hint_count : 0,
-				nfit_mem->flush_wpq);
+				nfit_mem->flush_wpq, dimm_id);
 		if (!nvdimm)
 			return -ENOMEM;
 
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 8d348b22ba45..0ab06fd87992 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -398,7 +398,7 @@ EXPORT_SYMBOL_GPL(nvdimm_attribute_group);
 struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 		const struct attribute_group **groups, unsigned long flags,
 		unsigned long cmd_mask, int num_flush,
-		struct resource *flush_wpq)
+		struct resource *flush_wpq, const char *id)
 {
 	struct nvdimm *nvdimm = kzalloc(sizeof(*nvdimm), GFP_KERNEL);
 	struct device *dev;
@@ -411,6 +411,8 @@ struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 		kfree(nvdimm);
 		return NULL;
 	}
+
+	memcpy(nvdimm->dimm_id, id, NVDIMM_DEFAULT_DESC_LEN);
 	nvdimm->provider_data = provider_data;
 	nvdimm->flags = flags;
 	nvdimm->cmd_mask = cmd_mask;
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 79274ead54fb..0e9c4fd1234f 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -42,6 +42,7 @@ struct nvdimm {
 	atomic_t busy;
 	int id, num_flush;
 	struct resource *flush_wpq;
+	char dimm_id[NVDIMM_DEFAULT_DESC_LEN];
 };
 
 /**
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index a250ff2a30df..74c6fb315bf0 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -183,7 +183,7 @@ void *nvdimm_provider_data(struct nvdimm *nvdimm);
 struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 		const struct attribute_group **groups, unsigned long flags,
 		unsigned long cmd_mask, int num_flush,
-		struct resource *flush_wpq);
+		struct resource *flush_wpq, const char *dimm_id);
 const struct nd_cmd_desc *nd_cmd_dimm_desc(int cmd);
 const struct nd_cmd_desc *nd_cmd_bus_desc(int cmd);
 u32 nd_cmd_in_size(struct nvdimm *nvdimm, int cmd,

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
                   ` (2 preceding siblings ...)
  2018-07-02 23:39 ` [PATCH 03/11] nfit/libnvdimm: store dimm id as a member to struct nvdimm Dave Jiang
@ 2018-07-02 23:39 ` Dave Jiang
  2018-07-03  1:45   ` Elliott, Robert (Persistent Memory)
  2018-07-03  3:48   ` Dan Williams
  2018-07-02 23:39 ` [PATCH 05/11] nfit/libnvdimm: add set passphrase support for Intel nvdimms Dave Jiang
                   ` (6 subsequent siblings)
  10 siblings, 2 replies; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

Adding support to allow query the security status of the Intel nvdimms and
also unlock the dimm via the kernel key management APIs. The passphrase is
expected to be pulled from userspace through keyutils. Moving the Intel
related bits to its own source file as well.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/nfit/Makefile |    2 -
 drivers/acpi/nfit/core.c   |    3 +
 drivers/acpi/nfit/intel.c  |  142 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/nfit/intel.h  |    2 +
 drivers/nvdimm/dimm.c      |    7 ++
 drivers/nvdimm/dimm_devs.c |  110 ++++++++++++++++++++++++++++++++++
 drivers/nvdimm/nd-core.h   |    3 +
 drivers/nvdimm/nd.h        |    2 +
 include/linux/libnvdimm.h  |   22 ++++++-
 9 files changed, 287 insertions(+), 6 deletions(-)
 create mode 100644 drivers/acpi/nfit/intel.c

diff --git a/drivers/acpi/nfit/Makefile b/drivers/acpi/nfit/Makefile
index a407e769f103..8287005f9226 100644
--- a/drivers/acpi/nfit/Makefile
+++ b/drivers/acpi/nfit/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_ACPI_NFIT) := nfit.o
-nfit-y := core.o
+nfit-y := core.o intel.o
 nfit-$(CONFIG_X86_MCE) += mce.o
diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
index a6fb336da79d..638bb7fbe565 100644
--- a/drivers/acpi/nfit/core.c
+++ b/drivers/acpi/nfit/core.c
@@ -1907,7 +1907,8 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
 		nvdimm = nvdimm_create(acpi_desc->nvdimm_bus, nfit_mem,
 				acpi_nfit_dimm_attribute_groups,
 				flags, cmd_mask, flush ? flush->hint_count : 0,
-				nfit_mem->flush_wpq, dimm_id);
+				nfit_mem->flush_wpq, dimm_id,
+				&intel_security_ops);
 		if (!nvdimm)
 			return -ENOMEM;
 
diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
new file mode 100644
index 000000000000..1bdb493694e0
--- /dev/null
+++ b/drivers/acpi/nfit/intel.c
@@ -0,0 +1,142 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+/*
+ * Intel specific NFIT ops
+ */
+#include <linux/libnvdimm.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/ndctl.h>
+#include <linux/sysfs.h>
+#include <linux/delay.h>
+#include <linux/acpi.h>
+#include <linux/io.h>
+#include <linux/nd.h>
+#include <asm/cacheflush.h>
+#include <acpi/nfit.h>
+#include "nfit.h"
+
+static int intel_dimm_security_unlock(struct nvdimm_bus *nvdimm_bus,
+		struct nvdimm *nvdimm, const char *passphrase)
+{
+	struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus);
+	int cmd_rc, rc = 0, pkg_size;
+	struct nd_intel_unlock_unit *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+
+	if (!test_bit(NVDIMM_INTEL_UNLOCK_UNIT, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	pkg_size = sizeof(*pkg) + sizeof(*cmd);
+	pkg = kzalloc(pkg_size, GFP_KERNEL);
+	if (!pkg)
+		return -ENOMEM;
+
+	pkg->nd_command = NVDIMM_INTEL_UNLOCK_UNIT;
+	pkg->nd_family = NVDIMM_FAMILY_INTEL;
+	pkg->nd_size_in = ND_INTEL_PASSPHRASE_SIZE;
+	pkg->nd_size_out = ND_INTEL_STATUS_SIZE;
+	pkg->nd_fw_size = ND_INTEL_STATUS_SIZE;
+	cmd = (struct nd_intel_unlock_unit *)&pkg->nd_payload;
+	memcpy(cmd->passphrase, passphrase, ND_INTEL_PASSPHRASE_SIZE);
+	rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, pkg,
+			sizeof(pkg_size), &cmd_rc);
+	if (rc < 0)
+		goto out;
+	if (cmd_rc < 0) {
+		rc = cmd_rc;
+		goto out;
+	}
+
+	switch (cmd->status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		rc = -EINVAL;
+		goto out;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		rc = -ENXIO;
+		goto out;
+	}
+
+	/* DIMM unlocked, invalidate all CPU caches before we read it */
+	wbinvd();
+
+ out:
+	kfree(pkg);
+	return rc;
+}
+
+static int intel_dimm_security_state(struct nvdimm_bus *nvdimm_bus,
+		struct nvdimm *nvdimm, enum nvdimm_security_state *state)
+{
+	struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus);
+	int cmd_rc, rc = 0, pkg_size;
+	struct nd_intel_get_security_state *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+
+	if (!test_bit(NVDIMM_INTEL_GET_SECURITY_STATE, &nfit_mem->dsm_mask)) {
+		*state = NVDIMM_SECURITY_UNSUPPORTED;
+		return 0;
+	}
+
+	pkg_size = sizeof(*pkg) + sizeof(*cmd);
+	pkg = kzalloc(pkg_size, GFP_KERNEL);
+	if (!pkg)
+		return -ENOMEM;
+
+	*state = NVDIMM_SECURITY_DISABLED;
+
+	pkg->nd_command = NVDIMM_INTEL_GET_SECURITY_STATE;
+	pkg->nd_family = NVDIMM_FAMILY_INTEL;
+	pkg->nd_size_in = 0;
+	pkg->nd_size_out = sizeof(*cmd);
+	pkg->nd_fw_size = pkg->nd_size_out;
+	cmd = (struct nd_intel_get_security_state *)&pkg->nd_payload;
+
+	rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, pkg, pkg_size,
+			&cmd_rc);
+	if (rc < 0)
+		goto out;
+	if (cmd_rc < 0) {
+		rc = cmd_rc;
+		goto out;
+	}
+
+	switch (cmd->status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_RETRY:
+		rc = -EAGAIN;
+		goto out;
+	case ND_INTEL_STATUS_NOT_READY:
+	default:
+		rc = -ENXIO;
+		goto out;
+	}
+
+	/* check and see if security is enabled and locked */
+	if (cmd->state & ND_INTEL_SEC_STATE_UNSUPPORTED)
+		*state = NVDIMM_SECURITY_UNSUPPORTED;
+	else if (cmd->state & ND_INTEL_SEC_STATE_ENABLED) {
+		if (cmd->state & ND_INTEL_SEC_STATE_LOCKED)
+			*state = NVDIMM_SECURITY_LOCKED;
+		else
+			*state = NVDIMM_SECURITY_UNLOCKED;
+	} else
+		*state = NVDIMM_SECURITY_DISABLED;
+
+ out:
+	kfree(pkg);
+	if (rc < 0)
+		*state = NVDIMM_SECURITY_INVALID;
+	return rc;
+}
+
+struct nvdimm_security_ops intel_security_ops = {
+	.state = intel_dimm_security_state,
+	.unlock = intel_dimm_security_unlock,
+};
diff --git a/drivers/acpi/nfit/intel.h b/drivers/acpi/nfit/intel.h
index a351f451b42a..774ceb65b1b8 100644
--- a/drivers/acpi/nfit/intel.h
+++ b/drivers/acpi/nfit/intel.h
@@ -6,6 +6,8 @@
 #ifndef _NFIT_INTEL_H_
 #define _NFIT_INTEL_H_
 
+extern struct nvdimm_security_ops intel_security_ops;
+
 /*
  * Command numbers that the kernel needs to know about to handle
  * non-default DSM revision ids
diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
index ee0c68efa82a..a572e6fbd229 100644
--- a/drivers/nvdimm/dimm.c
+++ b/drivers/nvdimm/dimm.c
@@ -88,6 +88,13 @@ static int nvdimm_probe(struct device *dev)
 	get_device(dev);
 	kref_init(&ndd->kref);
 
+	nvdimm_security_get_state(dev);
+
+	/* unlock DIMM here before touch label */
+	rc = nvdimm_security_unlock_dimm(dev);
+	if (rc < 0)
+		dev_warn(dev, "failed to unlock dimm %s\n", dev_name(dev));
+
 	/*
 	 * EACCES failures reading the namespace label-area-properties
 	 * are interpreted as the DIMM capacity being locked but the
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 0ab06fd87992..f92bbe78e0df 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -18,6 +18,8 @@
 #include <linux/io.h>
 #include <linux/fs.h>
 #include <linux/mm.h>
+#include <linux/cred.h>
+#include <linux/key-type.h>
 #include "nd-core.h"
 #include "label.h"
 #include "pmem.h"
@@ -25,6 +27,110 @@
 
 static DEFINE_IDA(dimm_ida);
 
+/*
+ * Find key in kernel keyring
+ */
+static struct key *nvdimm_search_key(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	char *desc;
+	key_ref_t keyref;
+	struct key *key = NULL;
+
+	if (!nvdimm->security_ops)
+		return NULL;
+
+	desc = kzalloc(NVDIMM_DEFAULT_DESC_LEN, GFP_KERNEL);
+	if (!desc)
+		return NULL;
+
+	keyref = keyring_search(make_key_ref(nvdimm_cred->thread_keyring, 1),
+			&nvdimm_key_type, nvdimm->dimm_id);
+	if (IS_ERR(keyref))
+		key = NULL;
+	else
+		key = key_ref_to_ptr(keyref);
+
+	kfree(desc);
+	return key;
+}
+
+/*
+ * Retrieve kernel key for DIMM and request from user space if necessary.
+ */
+static struct key *nvdimm_request_key(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	struct key *key = NULL;
+
+	if (!nvdimm->security_ops)
+		return NULL;
+
+	key = request_key(&nvdimm_key_type, nvdimm->dimm_id, nvdimm->dimm_id);
+	if (IS_ERR(key))
+		key = NULL;
+
+	return key;
+}
+
+int nvdimm_security_get_state(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+
+	if (!nvdimm->security_ops)
+		return 0;
+
+	return nvdimm->security_ops->state(nvdimm_bus, nvdimm,
+			&nvdimm->state);
+}
+
+int nvdimm_security_unlock_dimm(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key;
+	int rc;
+	char *payload;
+	bool cached_key = false;
+
+	if (!nvdimm->security_ops)
+		return 0;
+
+	if (nvdimm->state == NVDIMM_SECURITY_UNLOCKED ||
+			nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED ||
+			nvdimm->state == NVDIMM_SECURITY_DISABLED)
+		return 0;
+
+	key = nvdimm_search_key(dev);
+	if (!key)
+		key = nvdimm_request_key(dev);
+	else
+		cached_key = true;
+	if (!key)
+		return -ENXIO;
+
+	dev_dbg(dev, "%s: key: %#x\n", __func__, key->serial);
+	down_read(&key->sem);
+	payload = key->payload.data[0];
+	rc = nvdimm->security_ops->unlock(nvdimm_bus, nvdimm, payload);
+	up_read(&key->sem);
+
+	if (rc == 0) {
+		if (!cached_key)
+			key_link(nvdimm_cred->thread_keyring, key);
+		nvdimm->state = NVDIMM_SECURITY_UNLOCKED;
+		dev_info(dev, "DIMM %s unlocked\n", dev_name(dev));
+	} else {
+		key_invalidate(key);
+		dev_warn(dev, "Failed to unlock dimm: %s\n", dev_name(dev));
+	}
+
+	key_put(key);
+	nvdimm_security_get_state(dev);
+	return rc;
+}
+
 /*
  * Retrieve bus and dimm handle and return if this bus supports
  * get_config_data commands
@@ -398,7 +504,8 @@ EXPORT_SYMBOL_GPL(nvdimm_attribute_group);
 struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 		const struct attribute_group **groups, unsigned long flags,
 		unsigned long cmd_mask, int num_flush,
-		struct resource *flush_wpq, const char *id)
+		struct resource *flush_wpq, const char *id,
+		struct nvdimm_security_ops *sec_ops)
 {
 	struct nvdimm *nvdimm = kzalloc(sizeof(*nvdimm), GFP_KERNEL);
 	struct device *dev;
@@ -413,6 +520,7 @@ struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 	}
 
 	memcpy(nvdimm->dimm_id, id, NVDIMM_DEFAULT_DESC_LEN);
+	nvdimm->security_ops = sec_ops;
 	nvdimm->provider_data = provider_data;
 	nvdimm->flags = flags;
 	nvdimm->cmd_mask = cmd_mask;
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 0e9c4fd1234f..6f1ce86f8e93 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -22,6 +22,7 @@
 extern struct list_head nvdimm_bus_list;
 extern struct mutex nvdimm_bus_list_mutex;
 extern int nvdimm_major;
+extern const struct cred *nvdimm_cred;
 
 struct nvdimm_bus {
 	struct nvdimm_bus_descriptor *nd_desc;
@@ -43,6 +44,8 @@ struct nvdimm {
 	int id, num_flush;
 	struct resource *flush_wpq;
 	char dimm_id[NVDIMM_DEFAULT_DESC_LEN];
+	struct nvdimm_security_ops *security_ops;
+	enum nvdimm_security_state state;
 };
 
 /**
diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h
index 9d17abd9f8d0..9c80e0f8c327 100644
--- a/drivers/nvdimm/nd.h
+++ b/drivers/nvdimm/nd.h
@@ -424,4 +424,6 @@ static inline bool is_bad_pmem(struct badblocks *bb, sector_t sector,
 resource_size_t nd_namespace_blk_validate(struct nd_namespace_blk *nsblk);
 const u8 *nd_dev_to_uuid(struct device *dev);
 bool pmem_should_map_pages(struct device *dev);
+int nvdimm_security_unlock_dimm(struct device *dev);
+int nvdimm_security_get_state(struct device *dev);
 #endif /* __ND_H__ */
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 74c6fb315bf0..be98afb5f27e 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -157,8 +157,23 @@ static inline struct nd_blk_region_desc *to_blk_region_desc(
 
 extern struct key_type nvdimm_key_type;
 
-#define NVDIMM_DEFAULT_PASSPHRASE_LEN		32
-#define NVDIMM_DEFAULT_DESC_LEN			32
+enum nvdimm_security_state {
+	NVDIMM_SECURITY_INVALID = 0,
+	NVDIMM_SECURITY_DISABLED,
+	NVDIMM_SECURITY_UNLOCKED,
+	NVDIMM_SECURITY_LOCKED,
+	NVDIMM_SECURITY_UNSUPPORTED,
+};
+
+#define NVDIMM_DEFAULT_PASSPHRASE_LEN	32
+#define NVDIMM_DEFAULT_DESC_LEN		32
+struct nvdimm_security_ops {
+	int (*state)(struct nvdimm_bus *nvdimm_bus,
+			struct nvdimm *nvdimm,
+			enum nvdimm_security_state *state);
+	int (*unlock)(struct nvdimm_bus *nvdimm_bus,
+			struct nvdimm *nvdimm, const char *passphrase);
+};
 
 void badrange_init(struct badrange *badrange);
 int badrange_add(struct badrange *badrange, u64 addr, u64 length);
@@ -183,7 +198,8 @@ void *nvdimm_provider_data(struct nvdimm *nvdimm);
 struct nvdimm *nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_data,
 		const struct attribute_group **groups, unsigned long flags,
 		unsigned long cmd_mask, int num_flush,
-		struct resource *flush_wpq, const char *dimm_id);
+		struct resource *flush_wpq, const char *dimm_id,
+		struct nvdimm_security_ops *sec_ops);
 const struct nd_cmd_desc *nd_cmd_dimm_desc(int cmd);
 const struct nd_cmd_desc *nd_cmd_bus_desc(int cmd);
 u32 nd_cmd_in_size(struct nvdimm *nvdimm, int cmd,

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 05/11] nfit/libnvdimm: add set passphrase support for Intel nvdimms
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
                   ` (3 preceding siblings ...)
  2018-07-02 23:39 ` [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs Dave Jiang
@ 2018-07-02 23:39 ` Dave Jiang
  2018-07-03 22:48   ` Dan Williams
  2018-07-02 23:39 ` [PATCH 06/11] nfit/libnvdimm: add disable passphrase support to Intel nvdimm Dave Jiang
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

Adding support for setting and/or updating passphrase on the Intel nvdimms.
The passphrase is pulled from userspace through the kernel key management.
We trigger the update via writing "update" to the sysfs attribute
"security". The state of the security can also be read via the "security"
attribute. libnvdimm will generically support the key_change API call.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/nfit/intel.c  |   54 ++++++++++++++++++++++
 drivers/nvdimm/dimm_devs.c |  110 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h  |    4 ++
 3 files changed, 168 insertions(+)

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index 1bdb493694e0..792a5c694a9e 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -16,6 +16,59 @@
 #include <acpi/nfit.h>
 #include "nfit.h"
 
+static int intel_dimm_security_update_passphrase(
+		struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
+		const char *old_pass, const char *new_pass)
+{
+	struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus);
+	int cmd_rc, rc = 0, pkg_size;
+	struct nd_intel_set_passphrase *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+
+	if (!test_bit(NVDIMM_INTEL_SET_PASSPHRASE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	pkg_size = sizeof(*pkg) + sizeof(*cmd);
+	pkg = kzalloc(pkg_size, GFP_KERNEL);
+	if (!pkg)
+		return -ENOMEM;
+
+	pkg->nd_command = NVDIMM_INTEL_SET_PASSPHRASE;
+	pkg->nd_family = NVDIMM_FAMILY_INTEL;
+	pkg->nd_size_in = ND_INTEL_PASSPHRASE_SIZE * 2;
+	pkg->nd_size_out = ND_INTEL_STATUS_SIZE;
+	pkg->nd_fw_size = pkg->nd_size_out;
+	cmd = (struct nd_intel_set_passphrase *)&pkg->nd_payload;
+	if (old_pass)
+		memcpy(cmd->old_pass, old_pass, ND_INTEL_PASSPHRASE_SIZE);
+	memcpy(cmd->new_pass, new_pass, ND_INTEL_PASSPHRASE_SIZE);
+	rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, pkg,
+			sizeof(pkg_size), &cmd_rc);
+	if (rc < 0)
+		goto out;
+	if (cmd_rc < 0) {
+		rc = cmd_rc;
+		goto out;
+	}
+
+	switch (cmd->status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		rc = -EINVAL;
+		goto out;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		rc = -ENXIO;
+		goto out;
+	}
+
+ out:
+	kfree(pkg);
+	return rc;
+}
+
 static int intel_dimm_security_unlock(struct nvdimm_bus *nvdimm_bus,
 		struct nvdimm *nvdimm, const char *passphrase)
 {
@@ -139,4 +192,5 @@ static int intel_dimm_security_state(struct nvdimm_bus *nvdimm_bus,
 struct nvdimm_security_ops intel_security_ops = {
 	.state = intel_dimm_security_state,
 	.unlock = intel_dimm_security_unlock,
+	.change_key = intel_dimm_security_update_passphrase,
 };
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index f92bbe78e0df..d3d738c77adb 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -131,6 +131,75 @@ int nvdimm_security_unlock_dimm(struct device *dev)
 	return rc;
 }
 
+static int nvdimm_security_change_key(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key = NULL, *old_key = NULL;
+	int rc;
+	char *old_pass, *new_pass;
+
+	if (!nvdimm->security_ops)
+		return 0;
+
+	if (nvdimm->state == NVDIMM_SECURITY_FROZEN)
+		return -EBUSY;
+
+	/* look for a key from keyring if exists and remove */
+	old_key = nvdimm_search_key(dev);
+	if (old_key) {
+		dev_dbg(dev, "%s: killing old key: %#x\n",
+				__func__, old_key->serial);
+		key_invalidate(old_key);
+		key_put(old_key);
+		/* need key garbage collection to take effect */
+		cond_resched();
+	}
+
+	/* request new key from userspace */
+	key = nvdimm_request_key(dev);
+	if (!key) {
+		dev_dbg(dev, "%s: failed to acquire new key\n", __func__);
+		return -ENXIO;
+	}
+
+	dev_dbg(dev, "%s: new key: %#x\n", __func__, key->serial);
+
+	if (key->datalen == NVDIMM_DEFAULT_PASSPHRASE_LEN) {
+		old_pass = NULL;
+		new_pass = key->payload.data[0];
+	} else if (key->datalen == (NVDIMM_DEFAULT_PASSPHRASE_LEN * 2)) {
+		old_pass = key->payload.data[0];
+		new_pass = old_pass + NVDIMM_DEFAULT_PASSPHRASE_LEN;
+	} else {
+		key_invalidate(key);
+		key_put(key);
+		dev_warn(dev, "Incorrect key payload size for update: %u\n",
+				key->datalen);
+		return -EINVAL;
+	}
+
+	down_read(&key->sem);
+	rc = nvdimm->security_ops->change_key(nvdimm_bus, nvdimm, old_pass,
+			new_pass);
+	if (rc == 0 && key->datalen == (NVDIMM_DEFAULT_PASSPHRASE_LEN * 2))
+		memcpy(old_pass, new_pass, NVDIMM_DEFAULT_PASSPHRASE_LEN);
+	up_read(&key->sem);
+
+	/*
+	 * If we successfully update, link the new key to our keyring,
+	 * otherwise, nuke the key.
+	 */
+	if (rc == 0)
+		key_link(nvdimm_cred->thread_keyring, key);
+	else
+		key_invalidate(key);
+
+	key_put(key);
+	nvdimm_security_get_state(dev);
+	return rc;
+}
+
 /*
  * Retrieve bus and dimm handle and return if this bus supports
  * get_config_data commands
@@ -488,11 +557,52 @@ static ssize_t available_slots_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(available_slots);
 
+static ssize_t security_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+
+	switch (nvdimm->state) {
+	case NVDIMM_SECURITY_DISABLED:
+		return sprintf(buf, "disabled\n");
+	case NVDIMM_SECURITY_UNLOCKED:
+		return sprintf(buf, "unlocked\n");
+	case NVDIMM_SECURITY_LOCKED:
+		return sprintf(buf, "locked\n");
+	case NVDIMM_SECURITY_FROZEN:
+		return sprintf(buf, "frozen\n");
+	case NVDIMM_SECURITY_UNSUPPORTED:
+	default:
+		return sprintf(buf, "unsupported\n");
+	}
+
+	return -ENOTTY;
+}
+
+static ssize_t security_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t len)
+
+{
+	ssize_t rc = -EINVAL;
+
+	if (strcmp(buf, "update") == 0 || strcmp(buf, "update\n") == 0)
+		rc = nvdimm_security_change_key(dev);
+	else
+		return -EINVAL;
+
+	if (rc == 0)
+		rc = len;
+
+	return rc;
+}
+static DEVICE_ATTR_RW(security);
+
 static struct attribute *nvdimm_attributes[] = {
 	&dev_attr_state.attr,
 	&dev_attr_flags.attr,
 	&dev_attr_commands.attr,
 	&dev_attr_available_slots.attr,
+	&dev_attr_security.attr,
 	NULL,
 };
 
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index be98afb5f27e..7b609409042c 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -162,6 +162,7 @@ enum nvdimm_security_state {
 	NVDIMM_SECURITY_DISABLED,
 	NVDIMM_SECURITY_UNLOCKED,
 	NVDIMM_SECURITY_LOCKED,
+	NVDIMM_SECURITY_FROZEN,
 	NVDIMM_SECURITY_UNSUPPORTED,
 };
 
@@ -173,6 +174,9 @@ struct nvdimm_security_ops {
 			enum nvdimm_security_state *state);
 	int (*unlock)(struct nvdimm_bus *nvdimm_bus,
 			struct nvdimm *nvdimm, const char *passphrase);
+	int (*change_key)(struct nvdimm_bus *nvdimm_bus,
+			struct nvdimm *nvdimm, const char *old_pass,
+			const char *new_pass);
 };
 
 void badrange_init(struct badrange *badrange);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 06/11] nfit/libnvdimm: add disable passphrase support to Intel nvdimm.
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
                   ` (4 preceding siblings ...)
  2018-07-02 23:39 ` [PATCH 05/11] nfit/libnvdimm: add set passphrase support for Intel nvdimms Dave Jiang
@ 2018-07-02 23:39 ` Dave Jiang
  2018-07-02 23:39 ` [PATCH 07/11] nfit/libnvdimm: add freeze security " Dave Jiang
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

Adding support to disable passphrase (security) for the Intel nvdimm. The
passphrase used for disabling is pulled from userspace via the kernel
key management. The action is triggered by writing "disable" to the sysfs
attribute "security". libnvdimm will support the generic disable API call.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/nfit/intel.c  |   51 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvdimm/dimm_devs.c |   41 +++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h  |    2 ++
 3 files changed, 94 insertions(+)

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index 792a5c694a9e..6d73493f02cc 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -16,6 +16,56 @@
 #include <acpi/nfit.h>
 #include "nfit.h"
 
+static int intel_dimm_security_disable(struct nvdimm_bus *nvdimm_bus,
+		struct nvdimm *nvdimm, const char *pass)
+{
+	struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus);
+	int cmd_rc, rc = 0, pkg_size;
+	struct nd_intel_disable_passphrase *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+
+	if (!test_bit(NVDIMM_INTEL_DISABLE_PASSPHRASE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	pkg_size = sizeof(*pkg) + sizeof(*cmd);
+	pkg = kzalloc(pkg_size, GFP_KERNEL);
+	if (!pkg)
+		return -ENOMEM;
+
+	pkg->nd_command = NVDIMM_INTEL_DISABLE_PASSPHRASE;
+	pkg->nd_family = NVDIMM_FAMILY_INTEL;
+	pkg->nd_size_in = ND_INTEL_PASSPHRASE_SIZE;
+	pkg->nd_size_out = ND_INTEL_STATUS_SIZE;
+	pkg->nd_fw_size = pkg->nd_size_out;
+	cmd = (struct nd_intel_disable_passphrase *)&pkg->nd_payload;
+	memcpy(cmd->passphrase, pass, ND_INTEL_PASSPHRASE_SIZE);
+	rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, pkg,
+			sizeof(pkg_size), &cmd_rc);
+	if (rc < 0)
+		goto out;
+	if (cmd_rc < 0) {
+		rc = cmd_rc;
+		goto out;
+	}
+
+	switch (cmd->status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		rc = -EINVAL;
+		goto out;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		rc = -ENXIO;
+		goto out;
+	}
+
+ out:
+	kfree(pkg);
+	return rc;
+}
+
 static int intel_dimm_security_update_passphrase(
 		struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
 		const char *old_pass, const char *new_pass)
@@ -193,4 +243,5 @@ struct nvdimm_security_ops intel_security_ops = {
 	.state = intel_dimm_security_state,
 	.unlock = intel_dimm_security_unlock,
 	.change_key = intel_dimm_security_update_passphrase,
+	.disable = intel_dimm_security_disable,
 };
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index d3d738c77adb..070811cb4cdc 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -85,6 +85,45 @@ int nvdimm_security_get_state(struct device *dev)
 			&nvdimm->state);
 }
 
+static int nvdimm_security_disable(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key;
+	int rc;
+	char *payload;
+	bool cached_key = false;
+
+	if (!nvdimm->security_ops)
+		return 0;
+
+	if (nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED)
+		return 0;
+
+	key = nvdimm_search_key(dev);
+	if (!key)
+		key = nvdimm_request_key(dev);
+	else
+		cached_key = true;
+	if (!key)
+		return -ENXIO;
+
+	down_read(&key->sem);
+	payload = key->payload.data[0];
+	rc = nvdimm->security_ops->disable(nvdimm_bus, nvdimm, payload);
+	up_read(&key->sem);
+	if (rc < 0)
+		goto out;
+
+	/* If we succeed then remove the key */
+	key_invalidate(key);
+
+ out:
+	key_put(key);
+	nvdimm_security_get_state(dev);
+	return rc;
+}
+
 int nvdimm_security_unlock_dimm(struct device *dev)
 {
 	struct nvdimm *nvdimm = to_nvdimm(dev);
@@ -587,6 +626,8 @@ static ssize_t security_store(struct device *dev,
 
 	if (strcmp(buf, "update") == 0 || strcmp(buf, "update\n") == 0)
 		rc = nvdimm_security_change_key(dev);
+	else if (strcmp(buf, "disable") == 0 || strcmp(buf, "disable\n") == 0)
+		rc = nvdimm_security_disable(dev);
 	else
 		return -EINVAL;
 
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 7b609409042c..0990dfd5a0a3 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -177,6 +177,8 @@ struct nvdimm_security_ops {
 	int (*change_key)(struct nvdimm_bus *nvdimm_bus,
 			struct nvdimm *nvdimm, const char *old_pass,
 			const char *new_pass);
+	int (*disable)(struct nvdimm_bus *nvdimm_bus,
+			struct nvdimm *nvdimm, const char *pass);
 };
 
 void badrange_init(struct badrange *badrange);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 07/11] nfit/libnvdimm: add freeze security support to Intel nvdimm
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
                   ` (5 preceding siblings ...)
  2018-07-02 23:39 ` [PATCH 06/11] nfit/libnvdimm: add disable passphrase support to Intel nvdimm Dave Jiang
@ 2018-07-02 23:39 ` Dave Jiang
  2018-07-02 23:39 ` [PATCH 08/11] nfit/libnvdimm: add support for issue secure erase DSM " Dave Jiang
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

Adding support for freeze security on Intel nvdimm. This locks out any
changes to security for the DIMM unless a reboot is done. This is triggered
by writing "freeze" to the "security" sysfs attribute. libnvdimm will
support the generic freeze_lock API call.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/nfit/intel.c  |   47 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvdimm/dimm_devs.c |   22 +++++++++++++++++++++
 include/linux/libnvdimm.h  |    2 ++
 3 files changed, 71 insertions(+)

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index 6d73493f02cc..41602be2a33b 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -16,6 +16,52 @@
 #include <acpi/nfit.h>
 #include "nfit.h"
 
+static int intel_dimm_security_freeze_lock(struct nvdimm_bus *nvdimm_bus,
+		struct nvdimm *nvdimm)
+{
+	struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus);
+	int cmd_rc, rc = 0, pkg_size;
+	struct nd_intel_freeze_lock *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+
+	if (!test_bit(NVDIMM_INTEL_FREEZE_LOCK, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	pkg_size = sizeof(*pkg) + sizeof(*cmd);
+	pkg = kzalloc(pkg_size, GFP_KERNEL);
+	if (!pkg)
+		return -ENOMEM;
+
+	pkg->nd_command = NVDIMM_INTEL_FREEZE_LOCK;
+	pkg->nd_family = NVDIMM_FAMILY_INTEL;
+	pkg->nd_size_in = 0;
+	pkg->nd_size_out = ND_INTEL_STATUS_SIZE;
+	pkg->nd_fw_size = pkg->nd_size_out;
+	cmd = (struct nd_intel_freeze_lock *)&pkg->nd_payload;
+	rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, pkg,
+			sizeof(pkg_size), &cmd_rc);
+	if (rc < 0)
+		goto out;
+	if (cmd_rc < 0) {
+		rc = cmd_rc;
+		goto out;
+	}
+
+	switch (cmd->status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		rc = -ENXIO;
+		goto out;
+	}
+
+ out:
+	kfree(pkg);
+	return rc;
+}
+
 static int intel_dimm_security_disable(struct nvdimm_bus *nvdimm_bus,
 		struct nvdimm *nvdimm, const char *pass)
 {
@@ -244,4 +290,5 @@ struct nvdimm_security_ops intel_security_ops = {
 	.unlock = intel_dimm_security_unlock,
 	.change_key = intel_dimm_security_update_passphrase,
 	.disable = intel_dimm_security_disable,
+	.freeze_lock = intel_dimm_security_freeze_lock,
 };
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index 070811cb4cdc..cd0624663621 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -85,6 +85,26 @@ int nvdimm_security_get_state(struct device *dev)
 			&nvdimm->state);
 }
 
+static int nvdimm_security_freeze_lock(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	int rc;
+
+	if (!nvdimm->security_ops)
+		return 0;
+
+	if (nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED)
+		return 0;
+
+	rc = nvdimm->security_ops->freeze_lock(nvdimm_bus, nvdimm);
+	if (rc < 0)
+		return rc;
+
+	nvdimm_security_get_state(dev);
+	return 0;
+}
+
 static int nvdimm_security_disable(struct device *dev)
 {
 	struct nvdimm *nvdimm = to_nvdimm(dev);
@@ -628,6 +648,8 @@ static ssize_t security_store(struct device *dev,
 		rc = nvdimm_security_change_key(dev);
 	else if (strcmp(buf, "disable") == 0 || strcmp(buf, "disable\n") == 0)
 		rc = nvdimm_security_disable(dev);
+	else if (strcmp(buf, "freeze") == 0 || strcmp(buf, "freeze\n") == 0)
+		rc = nvdimm_security_freeze_lock(dev);
 	else
 		return -EINVAL;
 
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 0990dfd5a0a3..683e4cadc7f6 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -179,6 +179,8 @@ struct nvdimm_security_ops {
 			const char *new_pass);
 	int (*disable)(struct nvdimm_bus *nvdimm_bus,
 			struct nvdimm *nvdimm, const char *pass);
+	int (*freeze_lock)(struct nvdimm_bus *nvdimm_bus,
+			struct nvdimm *nvdimm);
 };
 
 void badrange_init(struct badrange *badrange);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 08/11] nfit/libnvdimm: add support for issue secure erase DSM to Intel nvdimm
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
                   ` (6 preceding siblings ...)
  2018-07-02 23:39 ` [PATCH 07/11] nfit/libnvdimm: add freeze security " Dave Jiang
@ 2018-07-02 23:39 ` Dave Jiang
  2018-07-02 23:39 ` [PATCH 09/11] nfit_test: adding context to dimm_dev for nfit_test Dave Jiang
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

Adding support to issue a secure erase DSM to the Intel nvdimm. The
required passphrase is acquired from userspace through the kernel key
management. To trigger the action, "erase" is written to the "security"
sysfs attribute.  libnvdimm will support the erase generic API call.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/acpi/nfit/intel.c  |   54 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/nvdimm/dimm_devs.c |   47 ++++++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h  |    2 ++
 3 files changed, 103 insertions(+)

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index 41602be2a33b..4000db2d5ed8 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -16,6 +16,59 @@
 #include <acpi/nfit.h>
 #include "nfit.h"
 
+static int intel_dimm_security_erase(struct nvdimm_bus *nvdimm_bus,
+		struct nvdimm *nvdimm, const char *passphrase)
+{
+	struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus);
+	int cmd_rc, rc = 0, pkg_size;
+	struct nd_intel_secure_erase *cmd;
+	struct nd_cmd_pkg *pkg;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+
+	if (!test_bit(NVDIMM_INTEL_SECURE_ERASE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	pkg_size = sizeof(*pkg) + sizeof(*cmd);
+	pkg = kzalloc(pkg_size, GFP_KERNEL);
+	if (!pkg)
+		return -ENOMEM;
+
+	pkg->nd_command = NVDIMM_INTEL_SECURE_ERASE;
+	pkg->nd_family = NVDIMM_FAMILY_INTEL;
+	pkg->nd_size_in = ND_INTEL_PASSPHRASE_SIZE;
+	pkg->nd_size_out = ND_INTEL_STATUS_SIZE;
+	pkg->nd_fw_size = pkg->nd_size_out;
+	cmd = (struct nd_intel_secure_erase *)&pkg->nd_payload;
+	memcpy(cmd->passphrase, passphrase, ND_INTEL_PASSPHRASE_SIZE);
+	rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, pkg,
+			sizeof(pkg_size), &cmd_rc);
+	if (rc < 0)
+		goto out;
+	if (cmd_rc < 0) {
+		rc = cmd_rc;
+		goto out;
+	}
+
+	switch (cmd->status) {
+	case 0:
+		break;
+	case ND_INTEL_STATUS_INVALID_PASS:
+		rc = -EINVAL;
+		goto out;
+	case ND_INTEL_STATUS_INVALID_STATE:
+	default:
+		rc = -ENXIO;
+		goto out;
+	}
+
+	/* DIMM unlocked, invalidate all CPU caches before we read it */
+	wbinvd();
+
+ out:
+	kfree(pkg);
+	return rc;
+}
+
 static int intel_dimm_security_freeze_lock(struct nvdimm_bus *nvdimm_bus,
 		struct nvdimm *nvdimm)
 {
@@ -291,4 +344,5 @@ struct nvdimm_security_ops intel_security_ops = {
 	.change_key = intel_dimm_security_update_passphrase,
 	.disable = intel_dimm_security_disable,
 	.freeze_lock = intel_dimm_security_freeze_lock,
+	.erase = intel_dimm_security_erase,
 };
diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c
index cd0624663621..b66bde25bfe6 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -85,6 +85,51 @@ int nvdimm_security_get_state(struct device *dev)
 			&nvdimm->state);
 }
 
+static int nvdimm_security_erase(struct device *dev)
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	struct key *key;
+	char *payload;
+	int rc = 0;
+
+	if (!nvdimm->security_ops)
+		return 0;
+
+	/* lock the device and disallow driver bind */
+	device_lock(dev);
+	/* No driver data means dimm is disabled. Proceed if so. */
+	if (dev_get_drvdata(dev)) {
+		dev_warn(dev, "Unable to secure erase while DIMM active.\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED)
+		goto out;
+
+	key = nvdimm_search_key(dev);
+	if (!key)
+		key = nvdimm_request_key(dev);
+	if (!key) {
+		rc = -ENXIO;
+		goto out;
+	}
+
+	down_read(&key->sem);
+	payload = key->payload.data[0];
+	rc = nvdimm->security_ops->erase(nvdimm_bus, nvdimm, payload);
+	up_read(&key->sem);
+	/* remove key since secure erase kills the passphrase */
+	key_invalidate(key);
+	key_put(key);
+
+ out:
+	device_unlock(dev);
+	nvdimm_security_get_state(dev);
+	return rc;
+}
+
 static int nvdimm_security_freeze_lock(struct device *dev)
 {
 	struct nvdimm *nvdimm = to_nvdimm(dev);
@@ -650,6 +695,8 @@ static ssize_t security_store(struct device *dev,
 		rc = nvdimm_security_disable(dev);
 	else if (strcmp(buf, "freeze") == 0 || strcmp(buf, "freeze\n") == 0)
 		rc = nvdimm_security_freeze_lock(dev);
+	else if (strcmp(buf, "erase") == 0 || strcmp(buf, "erase\n") == 0)
+		rc = nvdimm_security_erase(dev);
 	else
 		return -EINVAL;
 
diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
index 683e4cadc7f6..e330a452238e 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -181,6 +181,8 @@ struct nvdimm_security_ops {
 			struct nvdimm *nvdimm, const char *pass);
 	int (*freeze_lock)(struct nvdimm_bus *nvdimm_bus,
 			struct nvdimm *nvdimm);
+	int (*erase)(struct nvdimm_bus *nvdimm_bus,
+			struct nvdimm *nvdimm, const char *passphrase);
 };
 
 void badrange_init(struct badrange *badrange);

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 09/11] nfit_test: adding context to dimm_dev for nfit_test
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
                   ` (7 preceding siblings ...)
  2018-07-02 23:39 ` [PATCH 08/11] nfit/libnvdimm: add support for issue secure erase DSM " Dave Jiang
@ 2018-07-02 23:39 ` Dave Jiang
  2018-07-02 23:40 ` [PATCH 10/11] nfit_test: adding test support for Intel nvdimm security DSMs Dave Jiang
  2018-07-02 23:40 ` [PATCH 11/11] libnvdimm: adding documentation for nvdimm security support Dave Jiang
  10 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:39 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

In order to access the nfit_test context via sideband sysfs knobs, the
dimm_dev needs to be more than struct device in order to point back to
struct nfit_test. Wrapping the original struct device with a struct
nfit_dimm_dev and saving the nfit_test as private driver data. Also
changing the nfit_mem to be a member of struct nfit_dimm_dev instead of
saving as private driver data of that device. This is in preparation for
adding security DSM support and allowing the locking of DIMMs for testing
via sideband.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/nvdimm/test/nfit.c |   41 +++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 18157b0c0d0d..7b3ce2194889 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -150,6 +150,11 @@ struct nfit_test_fw {
 	u64 end_time;
 };
 
+struct nfit_dimm_dev {
+	struct device dev;
+	struct nfit_mem *nfit_mem;
+};
+
 struct nfit_test {
 	struct acpi_nfit_desc acpi_desc;
 	struct platform_device pdev;
@@ -181,7 +186,7 @@ struct nfit_test {
 		unsigned long deadline;
 		spinlock_t lock;
 	} ars_state;
-	struct device *dimm_dev[NUM_DCR];
+	struct nfit_dimm_dev dimm_dev[NUM_DCR];
 	struct nd_intel_smart *smart;
 	struct nd_intel_smart_threshold *smart_threshold;
 	struct badrange badrange;
@@ -981,7 +986,8 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 						&t->smart_threshold[i -
 							t->dcr_idx],
 						&t->smart[i - t->dcr_idx],
-						&t->pdev.dev, t->dimm_dev[i]);
+						&t->pdev.dev,
+						&t->dimm_dev[i].dev);
 				break;
 			case ND_INTEL_SMART_INJECT:
 				rc = nfit_test_cmd_smart_inject(buf,
@@ -989,7 +995,8 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 						&t->smart_threshold[i -
 							t->dcr_idx],
 						&t->smart[i - t->dcr_idx],
-						&t->pdev.dev, t->dimm_dev[i]);
+						&t->pdev.dev,
+						&t->dimm_dev[i].dev);
 				break;
 			default:
 				return -ENOTTY;
@@ -1187,8 +1194,7 @@ static void put_dimms(void *data)
 	int i;
 
 	for (i = 0; i < t->num_dcr; i++)
-		if (t->dimm_dev[i])
-			device_unregister(t->dimm_dev[i]);
+		device_unregister(&t->dimm_dev[i].dev);
 }
 
 static struct class *nfit_test_dimm;
@@ -1290,19 +1296,27 @@ static const struct attribute_group *nfit_test_dimm_attribute_groups[] = {
 	NULL,
 };
 
+static void dimm_dev_release(struct device *dev)
+{
+}
+
 static int nfit_test_dimm_init(struct nfit_test *t)
 {
-	int i;
+	int i, rc;
 
 	if (devm_add_action_or_reset(&t->pdev.dev, put_dimms, t))
 		return -ENOMEM;
 	for (i = 0; i < t->num_dcr; i++) {
-		t->dimm_dev[i] = device_create_with_groups(nfit_test_dimm,
-				&t->pdev.dev, 0, NULL,
-				nfit_test_dimm_attribute_groups,
-				"test_dimm%d", i + t->dcr_idx);
-		if (!t->dimm_dev[i])
-			return -ENOMEM;
+		t->dimm_dev[i].dev.parent = &t->pdev.dev;
+		dev_set_name(&t->dimm_dev[i].dev, "test_dimm%d",
+				i + t->dcr_idx);
+		t->dimm_dev[i].dev.class = nfit_test_dimm;
+		t->dimm_dev[i].dev.groups = nfit_test_dimm_attribute_groups;
+		t->dimm_dev[i].dev.release = dimm_dev_release;
+		rc = device_register(&t->dimm_dev[i].dev);
+		if (rc < 0)
+			return rc;
+		dev_set_drvdata(&t->dimm_dev[i].dev, t);
 	}
 	return 0;
 }
@@ -2665,8 +2679,7 @@ static int nfit_test_probe(struct platform_device *pdev)
 
 		for (i = 0; i < NUM_DCR; i++)
 			if (nfit_handle == handle[i])
-				dev_set_drvdata(nfit_test->dimm_dev[i],
-						nfit_mem);
+				nfit_test->dimm_dev[i].nfit_mem = nfit_mem;
 	}
 	mutex_unlock(&acpi_desc->init_mutex);
 

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 10/11] nfit_test: adding test support for Intel nvdimm security DSMs
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
                   ` (8 preceding siblings ...)
  2018-07-02 23:39 ` [PATCH 09/11] nfit_test: adding context to dimm_dev for nfit_test Dave Jiang
@ 2018-07-02 23:40 ` Dave Jiang
  2018-07-02 23:40 ` [PATCH 11/11] libnvdimm: adding documentation for nvdimm security support Dave Jiang
  10 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:40 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

Adding nfit_test support for DSM functions "Get Security State",
"Set Passphrase", "Disable Passphrase", "Unlock Unit", "Freeze Lock",
and "Secure Erase" for the fake DIMMs.

Also adding a sysfs knob in order to put the DIMMs in "locked" state. The
order of testing DIMM unlocking would be.
1a. Set Passphrase to DIMM X.
1b. Disable DIMM X.
2. Write to
/sys/devices/platform/nfit_test.0/nfit_test_dimm/test_dimmX/lock_dimm
3. Renable DIMM X
4. Check DIMM X state via sysfs "security" attribute for nmemX.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 tools/testing/nvdimm/Kbuild      |    1 
 tools/testing/nvdimm/test/nfit.c |  185 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 186 insertions(+)

diff --git a/tools/testing/nvdimm/Kbuild b/tools/testing/nvdimm/Kbuild
index 0392153a0009..29e43d64077f 100644
--- a/tools/testing/nvdimm/Kbuild
+++ b/tools/testing/nvdimm/Kbuild
@@ -36,6 +36,7 @@ obj-$(CONFIG_DEV_DAX) += device_dax.o
 obj-$(CONFIG_DEV_DAX_PMEM) += dax_pmem.o
 
 nfit-y := $(ACPI_SRC)/core.o
+nfit-y += $(ACPI_SRC)/intel.o
 nfit-$(CONFIG_X86_MCE) += $(ACPI_SRC)/mce.o
 nfit-y += acpi_nfit_test.o
 nfit-y += config_check.o
diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
index 7b3ce2194889..85a64d0e73de 100644
--- a/tools/testing/nvdimm/test/nfit.c
+++ b/tools/testing/nvdimm/test/nfit.c
@@ -155,6 +155,11 @@ struct nfit_dimm_dev {
 	struct nfit_mem *nfit_mem;
 };
 
+struct nfit_test_sec {
+	u8 state;
+	u8 passphrase[32];
+};
+
 struct nfit_test {
 	struct acpi_nfit_desc acpi_desc;
 	struct platform_device pdev;
@@ -192,6 +197,7 @@ struct nfit_test {
 	struct badrange badrange;
 	struct work_struct work;
 	struct nfit_test_fw *fw;
+	struct nfit_test_sec *sec;
 };
 
 static struct workqueue_struct *nfit_wq;
@@ -899,6 +905,138 @@ static int override_return_code(int dimm, unsigned int func, int rc)
 	return rc;
 }
 
+static int nd_intel_test_cmd_security_status(struct nfit_test *t,
+		struct nd_intel_get_security_state *nd_cmd,
+		unsigned int buf_len, int idx)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &t->sec[idx];
+
+	nd_cmd->status = 0;
+	nd_cmd->state = sec->state;
+	dev_dbg(dev, "security state (%#x) returned\n", nd_cmd->state);
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_unlock_unit(struct nfit_test *t,
+		struct nd_intel_unlock_unit *nd_cmd,
+		unsigned int buf_len, int idx)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &t->sec[idx];
+
+	if (!(sec->state & ND_INTEL_SEC_STATE_LOCKED) ||
+			(sec->state & ND_INTEL_SEC_STATE_FROZEN)) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "unlock unit: invalid state: %#x\n",
+				sec->state);
+	} else if (memcmp(nd_cmd->passphrase, sec->passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "unlock unit: invalid passphrase\n");
+	} else {
+		nd_cmd->status = 0;
+		sec->state = ND_INTEL_SEC_STATE_ENABLED;
+		dev_dbg(dev, "Unit unlocked\n");
+	}
+
+	dev_dbg(dev, "unlocking status returned: %#x\n", nd_cmd->status);
+	return 0;
+}
+
+static int nd_intel_test_cmd_set_pass(struct nfit_test *t,
+		struct nd_intel_set_passphrase *nd_cmd,
+		unsigned int buf_len, int idx)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &t->sec[idx];
+
+	if (sec->state & ND_INTEL_SEC_STATE_FROZEN) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "set passphrase: wrong security state\n");
+	} else if (memcmp(nd_cmd->old_pass, sec->passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "set passphrase: wrong passphrase\n");
+	} else {
+		memcpy(sec->passphrase, nd_cmd->new_pass,
+				ND_INTEL_PASSPHRASE_SIZE);
+		sec->state |= ND_INTEL_SEC_STATE_ENABLED;
+		nd_cmd->status = 0;
+		dev_dbg(dev, "passphrase updated\n");
+	}
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_freeze_lock(struct nfit_test *t,
+		struct nd_intel_freeze_lock *nd_cmd,
+		unsigned int buf_len, int idx)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &t->sec[idx];
+
+	if (!(sec->state & ND_INTEL_SEC_STATE_ENABLED)) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "freeze lock: wrong security state\n");
+	} else {
+		sec->state |= ND_INTEL_SEC_STATE_FROZEN;
+		nd_cmd->status = 0;
+		dev_dbg(dev, "security frozen\n");
+	}
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_disable_pass(struct nfit_test *t,
+		struct nd_intel_disable_passphrase *nd_cmd,
+		unsigned int buf_len, int idx)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &t->sec[idx];
+
+	if (!(sec->state & ND_INTEL_SEC_STATE_ENABLED) ||
+			(sec->state & ND_INTEL_SEC_STATE_FROZEN)) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "disable passphrase: wrong security state\n");
+	} else if (memcmp(nd_cmd->passphrase, sec->passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "disable passphrase: wrong passphrase\n");
+	} else {
+		memset(sec->passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
+		sec->state = 0;
+		dev_dbg(dev, "disable passphrase: done\n");
+	}
+
+	return 0;
+}
+
+static int nd_intel_test_cmd_secure_erase(struct nfit_test *t,
+		struct nd_intel_secure_erase *nd_cmd,
+		unsigned int buf_len, int idx)
+{
+	struct device *dev = &t->pdev.dev;
+	struct nfit_test_sec *sec = &t->sec[idx];
+
+	if (!(sec->state & ND_INTEL_SEC_STATE_ENABLED) ||
+			(sec->state & ND_INTEL_SEC_STATE_FROZEN)) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_STATE;
+		dev_dbg(dev, "secure erase: wrong security state\n");
+	} else if (memcmp(nd_cmd->passphrase, sec->passphrase,
+				ND_INTEL_PASSPHRASE_SIZE) != 0) {
+		nd_cmd->status = ND_INTEL_STATUS_INVALID_PASS;
+		dev_dbg(dev, "secure erase: wrong passphrase\n");
+	} else {
+		memset(sec->passphrase, 0, ND_INTEL_PASSPHRASE_SIZE);
+		sec->state = 0;
+		dev_dbg(dev, "secure erase: done\n");
+	}
+
+	return 0;
+}
+
 static int get_dimm(struct nfit_mem *nfit_mem, unsigned int func)
 {
 	int i;
@@ -946,6 +1084,30 @@ static int nfit_test_ctl(struct nvdimm_bus_descriptor *nd_desc,
 				return i;
 
 			switch (func) {
+			case NVDIMM_INTEL_GET_SECURITY_STATE:
+				rc = nd_intel_test_cmd_security_status(t,
+						buf, buf_len, i - t->dcr_idx);
+				break;
+			case NVDIMM_INTEL_UNLOCK_UNIT:
+				rc = nd_intel_test_cmd_unlock_unit(t,
+						buf, buf_len, i - t->dcr_idx);
+				break;
+			case NVDIMM_INTEL_SET_PASSPHRASE:
+				rc = nd_intel_test_cmd_set_pass(t,
+						buf, buf_len, i - t->dcr_idx);
+				break;
+			case NVDIMM_INTEL_DISABLE_PASSPHRASE:
+				rc = nd_intel_test_cmd_disable_pass(t,
+						buf, buf_len, i - t->dcr_idx);
+				break;
+			case NVDIMM_INTEL_FREEZE_LOCK:
+				rc = nd_intel_test_cmd_freeze_lock(t,
+						buf, buf_len, i - t->dcr_idx);
+				break;
+			case NVDIMM_INTEL_SECURE_ERASE:
+				rc = nd_intel_test_cmd_secure_erase(t,
+						buf, buf_len, i - t->dcr_idx);
+				break;
 			case ND_INTEL_ENABLE_LSS_STATUS:
 				rc = nd_intel_test_cmd_set_lss_status(t,
 						buf, buf_len);
@@ -1280,10 +1442,23 @@ static ssize_t fail_cmd_code_store(struct device *dev, struct device_attribute *
 }
 static DEVICE_ATTR_RW(fail_cmd_code);
 
+static ssize_t lock_dimm_store(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t size)
+{
+	int dimm = dimm_name_to_id(dev);
+	struct nfit_test *t = dev_get_drvdata(dev);
+	struct nfit_test_sec *sec = &t->sec[dimm];
+
+	sec->state = ND_INTEL_SEC_STATE_ENABLED | ND_INTEL_SEC_STATE_LOCKED;
+	return size;
+}
+static DEVICE_ATTR_WO(lock_dimm);
+
 static struct attribute *nfit_test_dimm_attributes[] = {
 	&dev_attr_fail_cmd.attr,
 	&dev_attr_fail_cmd_code.attr,
 	&dev_attr_handle.attr,
+	&dev_attr_lock_dimm.attr,
 	NULL,
 };
 
@@ -2192,6 +2367,14 @@ static void nfit_test0_setup(struct nfit_test *t)
 	set_bit(ND_INTEL_FW_FINISH_UPDATE, &acpi_desc->dimm_cmd_force_en);
 	set_bit(ND_INTEL_FW_FINISH_QUERY, &acpi_desc->dimm_cmd_force_en);
 	set_bit(ND_INTEL_ENABLE_LSS_STATUS, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_GET_SECURITY_STATE,
+			&acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_SET_PASSPHRASE, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_DISABLE_PASSPHRASE,
+			&acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_UNLOCK_UNIT, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_FREEZE_LOCK, &acpi_desc->dimm_cmd_force_en);
+	set_bit(NVDIMM_INTEL_SECURE_ERASE, &acpi_desc->dimm_cmd_force_en);
 }
 
 static void nfit_test1_setup(struct nfit_test *t)
@@ -2611,6 +2794,8 @@ static int nfit_test_probe(struct platform_device *pdev)
 				GFP_KERNEL);
 		nfit_test->fw = devm_kcalloc(dev, num,
 				sizeof(struct nfit_test_fw), GFP_KERNEL);
+		nfit_test->sec = devm_kcalloc(dev, num,
+				sizeof(struct nfit_test_sec), GFP_KERNEL);
 		if (nfit_test->dimm && nfit_test->dimm_dma && nfit_test->label
 				&& nfit_test->label_dma && nfit_test->dcr
 				&& nfit_test->dcr_dma && nfit_test->flush

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* [PATCH 11/11] libnvdimm: adding documentation for nvdimm security support
  2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
                   ` (9 preceding siblings ...)
  2018-07-02 23:40 ` [PATCH 10/11] nfit_test: adding test support for Intel nvdimm security DSMs Dave Jiang
@ 2018-07-02 23:40 ` Dave Jiang
  10 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2018-07-02 23:40 UTC (permalink / raw)
  To: dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm

Adding theory of operation for the security support that's going into
libnvdimm.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/nvdimm/security |   70 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 70 insertions(+)
 create mode 100644 Documentation/nvdimm/security

diff --git a/Documentation/nvdimm/security b/Documentation/nvdimm/security
new file mode 100644
index 000000000000..756205c9aa4c
--- /dev/null
+++ b/Documentation/nvdimm/security
@@ -0,0 +1,70 @@
+NVDIMM SECURITY
+===============
+
+1. Introduction
+---------------
+
+With the introduction of Intel DSM v1.7 specification [1], security DSMs are
+introduced. The spec added the following security DSMs: "get security state",
+"set passphrase", "disable passphrase", "unlock unit", "freeze lock",
+"secure erase", and "overwrite". A security_ops data structure has been
+added to struct dimm in order to support the security operations and generic
+APIs are exposed to allow vendor neutral operations.
+
+2. Sysfs Interface
+------------------
+The "security" sysfs attribute is provided in the nvdimm sysfs directory. For
+example:
+/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/nmem0/security
+
+The "show" function of that attribute will display the security state for
+that DIMM. The following states are available: disabled, unlocked, locked,
+frozen, and unsupported.
+
+The "store" function takes several commands when the attribute is written to
+in order to support some of the security functionalities:
+update - enable security. Add or update current key.
+disable - disable enabled security and remove key.
+freeze - freeze changing of security states.
+erase - generate new ecryption key for DIMM and crypto-scrambles all existing
+	user data.
+
+3. Key Management
+-----------------
+
+The key is associted to the payload by the DIMM id. For example:
+# cat /sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/nmem0/nfit/id
+8089-a2-1740-00000133
+The DIMM id would be provided along with the key payload (passphrase) to
+the kernel.
+
+The security keys are managed on the basis of a single key per DIMM. The
+key "passphrase" is expected to be 32bytes long or padded to 32bytes. This is
+similar to the ATA security specification [2]. A key is initially acquired
+via the request_key() kernel API call and retrieved from userspace. It is up to
+the user to provide an upcall application to retrieve the key in whatever
+fashion meets their security requirements.
+
+The payload provided to the key can be a 32bytes payload or 64bytes payload
+when doing an "update". The payload is viewed as 64 bytes in the following
+format:
+[32 bytes current key data zero padded][32 bytes new key data zero padded]
+However, a 32bytes payload can be provided and will be assumed as the old
+key to be 32 bytes of 0s and the provided 32bytes payload is the new key.
+It is up to the user upcall function how that's presented as the key payload
+to the kernel.
+
+All the other security functions that require a provided key can accept a
+32bytes payload or 64bytes. If the payload is 64bytes, then second 32bytes
+will be ignored and the first 32bytes contains the expected "passphrase".
+
+4. Unlocking
+------------
+When the DIMMs are being enumerated by the kernel, the kernel will attempt to
+retrieve the key from its keyring. If that fails, it will attempt to
+acquire the key from the userspace upcall function. This is the only time
+a locked DIMM can be unlocked. Once unlocked, the DIMM will remain unlocked
+until reboot.
+
+[1]: http://pmem.io/documents/NVDIMM_DSM_Interface-V1.7.pdf
+[2]: http://www.t13.org/documents/UploadedDocuments/docs2006/e05179r4-ACS-SecurityClarifications.pdf

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 01/11] nfit: adding support for Intel DSM 1.7 commands
  2018-07-02 23:39 ` [PATCH 01/11] nfit: adding support for Intel DSM 1.7 commands Dave Jiang
@ 2018-07-02 23:49   ` Dan Williams
  0 siblings, 0 replies; 23+ messages in thread
From: Dan Williams @ 2018-07-02 23:49 UTC (permalink / raw)
  To: Dave Jiang
  Cc: David Howells, Schofield,
	Alison  <alison.schofield@intel.com>,
	keyrings@vger.kernel.org, Kees Cook, linux-nvdimm

On Mon, Jul 2, 2018 at 4:39 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Adding command definition for security commands defined in Intel DSM
> specification v1.7. This includes "get security state", "set passphrase",
> "unlock unit", "freeze lock", "secure erase", "ovewrite", and
> "overwrite query". Since we are adding a lot of Intel definitions, moving
> the relevant bits to its own header. Also, we don't want those commands
> to be issued from user space through ioctls. Reason being some of the
> security commands require kernel involvement to flush all CPU caches that
> can't be done in userspace and the result can cause system crash. So
> blocking security commands in the ioctl path.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---

Looks good, one nit.

[..]
> diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h
> index 7d15856a739f..3b3f59828632 100644
> --- a/drivers/acpi/nfit/nfit.h
> +++ b/drivers/acpi/nfit/nfit.h
> @@ -20,6 +20,7 @@
>  #include <linux/types.h>
>  #include <linux/acpi.h>
>  #include <acpi/acuuid.h>
> +#include "intel.h"

Since nothing in nfit.h requires intel.h this include should move to
core.c directly.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 03/11] nfit/libnvdimm: store dimm id as a member to struct nvdimm
  2018-07-02 23:39 ` [PATCH 03/11] nfit/libnvdimm: store dimm id as a member to struct nvdimm Dave Jiang
@ 2018-07-03  1:04   ` Dan Williams
  0 siblings, 0 replies; 23+ messages in thread
From: Dan Williams @ 2018-07-03  1:04 UTC (permalink / raw)
  To: Dave Jiang
  Cc: David Howells, Schofield,
	Alison  <alison.schofield@intel.com>,
	keyrings@vger.kernel.org, Kees Cook, linux-nvdimm

On Mon, Jul 2, 2018 at 4:39 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> The generated dimm id is needed for the sysfs attribute as well as being
> used as the identifier/description for the security key. Since it's
> constant and should never change, store it as a member of struct nvdimm.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/acpi/nfit/core.c   |   33 ++++++++++++++++++++++-----------
>  drivers/nvdimm/dimm_devs.c |    4 +++-
>  drivers/nvdimm/nd-core.h   |    1 +
>  include/linux/libnvdimm.h  |    2 +-
>  4 files changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 70351b610b3d..a6fb336da79d 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
[..]
> +static int acpi_nfit_get_dimm_id(struct acpi_nfit_control_region *dcr,
> +               char *buf)
> +{
> +       if (dcr->valid_fields & ACPI_NFIT_CONTROL_MFG_INFO_VALID)
> +               return sprintf(buf, "%04x-%02x-%04x-%08x",
> +                               be16_to_cpu(dcr->vendor_id),
> +                               dcr->manufacturing_location,
> +                               be16_to_cpu(dcr->manufacturing_date),
> +                               be32_to_cpu(dcr->serial_number));
> +       else
> +               return sprintf(buf, "%04x-%08x",
> +                               be16_to_cpu(dcr->vendor_id),
> +                               be32_to_cpu(dcr->serial_number));
> +}

Let's just add a 24-byte string to 'struct nfit_mem' to house the id.
It's static data, so no need to do another sprintf() once it is
initialized.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* RE: [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
  2018-07-02 23:39 ` [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs Dave Jiang
@ 2018-07-03  1:45   ` Elliott, Robert (Persistent Memory)
  2018-07-03  3:46     ` Dan Williams
  2018-07-03  3:48   ` Dan Williams
  1 sibling, 1 reply; 23+ messages in thread
From: Elliott, Robert (Persistent Memory) @ 2018-07-03  1:45 UTC (permalink / raw)
  To: 'Dave Jiang', dan.j.williams
  Cc: dhowells, alison.schofield, keyrings, keescook, linux-nvdimm



> -----Original Message-----
> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of Dave Jiang
> Sent: Monday, July 2, 2018 6:39 PM
> To: dan.j.williams@intel.com
> Cc: dhowells@redhat.com; alison.schofield@intel.com; keyrings@vger.kernel.org; keescook@chromium.org;
> linux-nvdimm@lists.01.org
> Subject: [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
> 
> Adding support to allow query the security status of the Intel nvdimms and
> also unlock the dimm via the kernel key management APIs. The passphrase is
> expected to be pulled from userspace through keyutils. Moving the Intel
> related bits to its own source file as well.
> 
...
> +static int intel_dimm_security_unlock(struct nvdimm_bus *nvdimm_bus,
> +		struct nvdimm *nvdimm, const char *passphrase)
> +{
+	struct nd_intel_unlock_unit *cmd;
+	struct nd_cmd_pkg *pkg;
...
> +	pkg_size = sizeof(*pkg) + sizeof(*cmd);
> +	pkg = kzalloc(pkg_size, GFP_KERNEL);
> +	if (!pkg)
> +		return -ENOMEM;
> +
> +	pkg->nd_command = NVDIMM_INTEL_UNLOCK_UNIT;
> +	pkg->nd_family = NVDIMM_FAMILY_INTEL;
> +	pkg->nd_size_in = ND_INTEL_PASSPHRASE_SIZE;
> +	pkg->nd_size_out = ND_INTEL_STATUS_SIZE;
> +	pkg->nd_fw_size = ND_INTEL_STATUS_SIZE;
> +	cmd = (struct nd_intel_unlock_unit *)&pkg->nd_payload;
> +	memcpy(cmd->passphrase, passphrase, ND_INTEL_PASSPHRASE_SIZE);
> +	rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, pkg,
> +			sizeof(pkg_size), &cmd_rc);
...
> +	kfree(pkg);

Since it contains a high-value password, I recommend zeroing cmd->passphrase
before calling kfree() so that data isn't seen by a subsequent kmalloc()
caller (and make sure the compiler cannot optimize away the clearing code).
clear)

Also, check if the ndctl() call chain makes any copies of cmd->passphrase
and ensure they are cleared.

---
Robert Elliott, HPE Persistent Memory



_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
  2018-07-03  1:45   ` Elliott, Robert (Persistent Memory)
@ 2018-07-03  3:46     ` Dan Williams
  2018-07-03  4:58       ` Elliott, Robert (Persistent Memory)
  2018-07-03 18:00       ` James Morris
  0 siblings, 2 replies; 23+ messages in thread
From: Dan Williams @ 2018-07-03  3:46 UTC (permalink / raw)
  To: Elliott, Robert (Persistent Memory)
  Cc: Schofield, Alison, keescook, linux-nvdimm, dhowells, keyrings

On Mon, Jul 2, 2018 at 6:45 PM, Elliott, Robert (Persistent Memory)
<elliott@hpe.com> wrote:
>
>
>> -----Original Message-----
>> From: Linux-nvdimm [mailto:linux-nvdimm-bounces@lists.01.org] On Behalf Of Dave Jiang
>> Sent: Monday, July 2, 2018 6:39 PM
>> To: dan.j.williams@intel.com
>> Cc: dhowells@redhat.com; alison.schofield@intel.com; keyrings@vger.kernel.org; keescook@chromium.org;
>> linux-nvdimm@lists.01.org
>> Subject: [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
>>
>> Adding support to allow query the security status of the Intel nvdimms and
>> also unlock the dimm via the kernel key management APIs. The passphrase is
>> expected to be pulled from userspace through keyutils. Moving the Intel
>> related bits to its own source file as well.
>>
> ...
>> +static int intel_dimm_security_unlock(struct nvdimm_bus *nvdimm_bus,
>> +struct nvdimm *nvdimm, const char *passphrase)
>> +{
> +struct nd_intel_unlock_unit *cmd;
> +struct nd_cmd_pkg *pkg;
> ...
>> +pkg_size = sizeof(*pkg) + sizeof(*cmd);
>> +pkg = kzalloc(pkg_size, GFP_KERNEL);
>> +if (!pkg)
>> +return -ENOMEM;
>> +
>> +pkg->nd_command = NVDIMM_INTEL_UNLOCK_UNIT;
>> +pkg->nd_family = NVDIMM_FAMILY_INTEL;
>> +pkg->nd_size_in = ND_INTEL_PASSPHRASE_SIZE;
>> +pkg->nd_size_out = ND_INTEL_STATUS_SIZE;
>> +pkg->nd_fw_size = ND_INTEL_STATUS_SIZE;
>> +cmd = (struct nd_intel_unlock_unit *)&pkg->nd_payload;
>> +memcpy(cmd->passphrase, passphrase, ND_INTEL_PASSPHRASE_SIZE);
>> +rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, pkg,
>> +sizeof(pkg_size), &cmd_rc);
> ...
>> +kfree(pkg);
>
> Since it contains a high-value password, I recommend zeroing cmd->passphrase
> before calling kfree() so that data isn't seen by a subsequent kmalloc()
> caller (and make sure the compiler cannot optimize away the clearing code).
> clear)
>
> Also, check if the ndctl() call chain makes any copies of cmd->passphrase
> and ensure they are cleared.

If an attacker can run arbitrary code in the kernel they can get the
key from the ring directly, or turn on ACPI debug. A platform could
arrange for the DIMMs to be unlocked pre-OS to minimize passphrase
exposure, but once you need to unlock from the OS at runtime there is
this exposure. Now, there may be ways we could protect the key the TPM
to minimize exposure, but there would always be the in-flight risk,
especially with ACPI debug.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
  2018-07-02 23:39 ` [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs Dave Jiang
  2018-07-03  1:45   ` Elliott, Robert (Persistent Memory)
@ 2018-07-03  3:48   ` Dan Williams
  1 sibling, 0 replies; 23+ messages in thread
From: Dan Williams @ 2018-07-03  3:48 UTC (permalink / raw)
  To: Dave Jiang
  Cc: David Howells, Schofield,
	Alison  <alison.schofield@intel.com>,
	keyrings@vger.kernel.org, Kees Cook, linux-nvdimm

On Mon, Jul 2, 2018 at 4:39 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Adding support to allow query the security status of the Intel nvdimms and
> also unlock the dimm via the kernel key management APIs. The passphrase is
> expected to be pulled from userspace through keyutils. Moving the Intel
> related bits to its own source file as well.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/acpi/nfit/Makefile |    2 -
>  drivers/acpi/nfit/core.c   |    3 +
>  drivers/acpi/nfit/intel.c  |  142 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/nfit/intel.h  |    2 +
>  drivers/nvdimm/dimm.c      |    7 ++
>  drivers/nvdimm/dimm_devs.c |  110 ++++++++++++++++++++++++++++++++++
>  drivers/nvdimm/nd-core.h   |    3 +
>  drivers/nvdimm/nd.h        |    2 +
>  include/linux/libnvdimm.h  |   22 ++++++-
>  9 files changed, 287 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/acpi/nfit/intel.c
>
> diff --git a/drivers/acpi/nfit/Makefile b/drivers/acpi/nfit/Makefile
> index a407e769f103..8287005f9226 100644
> --- a/drivers/acpi/nfit/Makefile
> +++ b/drivers/acpi/nfit/Makefile
> @@ -1,3 +1,3 @@
>  obj-$(CONFIG_ACPI_NFIT) := nfit.o
> -nfit-y := core.o
> +nfit-y := core.o intel.o
>  nfit-$(CONFIG_X86_MCE) += mce.o
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index a6fb336da79d..638bb7fbe565 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -1907,7 +1907,8 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc)
>                 nvdimm = nvdimm_create(acpi_desc->nvdimm_bus, nfit_mem,
>                                 acpi_nfit_dimm_attribute_groups,
>                                 flags, cmd_mask, flush ? flush->hint_count : 0,
> -                               nfit_mem->flush_wpq, dimm_id);
> +                               nfit_mem->flush_wpq, dimm_id,
> +                               &intel_security_ops);

I would expect there to be a routine that returns the security ops
based on the 'nvdimm family'. I.e. that this ops structure should be
NULL for everything except NVDIMM_FAMILY_INTEL presently.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* RE: [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
  2018-07-03  3:46     ` Dan Williams
@ 2018-07-03  4:58       ` Elliott, Robert (Persistent Memory)
  2018-07-03  5:03         ` Dan Williams
  2018-07-03 18:00       ` James Morris
  1 sibling, 1 reply; 23+ messages in thread
From: Elliott, Robert (Persistent Memory) @ 2018-07-03  4:58 UTC (permalink / raw)
  To: Dan Williams
  Cc: Schofield, Alison, keescook, linux-nvdimm, dhowells, keyrings


> > Since it contains a high-value password, I recommend zeroing
> > cmd->passphrase before calling kfree() so that data isn't seen
> > by a subsequent kmalloc() caller (and make sure the compiler
> > cannot optimize away the clearing code).
> >
> > Also, check if the ndctl() call chain makes any copies of cmd-
> > passphrase and ensure they are cleared.
> 
> If an attacker can run arbitrary code in the kernel they can get the
> key from the ring directly, or turn on ACPI debug. A platform could
> arrange for the DIMMs to be unlocked pre-OS to minimize passphrase
> exposure, but once you need to unlock from the OS at runtime there is
> this exposure. Now, there may be ways we could protect the key the
> TPM
> to minimize exposure, but there would always be the in-flight risk,
> especially with ACPI debug.

TCG Opal poses the same problems, discussed before in this thread:
http://lists.infradead.org/pipermail/linux-nvme/2016-May/004646.html

Ultimately we might need to rely on code running in encrypted memory
(e.g., SGX) to unpack the password from TPM-based storage, wrap it with
a temporary session key (e.g., based on TCG PSK secure sessions, or
public/private keys), and only pass that non-reusable bundle through
the kernel. That'll require more complex devices, though.





_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
  2018-07-03  4:58       ` Elliott, Robert (Persistent Memory)
@ 2018-07-03  5:03         ` Dan Williams
  0 siblings, 0 replies; 23+ messages in thread
From: Dan Williams @ 2018-07-03  5:03 UTC (permalink / raw)
  To: Elliott, Robert (Persistent Memory)
  Cc: Schofield, Alison, keescook, linux-nvdimm, dhowells, keyrings

On Mon, Jul 2, 2018 at 9:58 PM, Elliott, Robert (Persistent Memory)
<elliott@hpe.com> wrote:
>
>> > Since it contains a high-value password, I recommend zeroing
>> > cmd->passphrase before calling kfree() so that data isn't seen
>> > by a subsequent kmalloc() caller (and make sure the compiler
>> > cannot optimize away the clearing code).
>> >
>> > Also, check if the ndctl() call chain makes any copies of cmd-
>> > passphrase and ensure they are cleared.
>>
>> If an attacker can run arbitrary code in the kernel they can get the
>> key from the ring directly, or turn on ACPI debug. A platform could
>> arrange for the DIMMs to be unlocked pre-OS to minimize passphrase
>> exposure, but once you need to unlock from the OS at runtime there is
>> this exposure. Now, there may be ways we could protect the key the
>> TPM
>> to minimize exposure, but there would always be the in-flight risk,
>> especially with ACPI debug.
>
> TCG Opal poses the same problems, discussed before in this thread:
> http://lists.infradead.org/pipermail/linux-nvme/2016-May/004646.html
>
> Ultimately we might need to rely on code running in encrypted memory
> (e.g., SGX) to unpack the password from TPM-based storage, wrap it with
> a temporary session key (e.g., based on TCG PSK secure sessions, or
> public/private keys), and only pass that non-reusable bundle through
> the kernel. That'll require more complex devices, though.

Agreed. At least this implementation avoids sedutil and reuses the
keyctl api directly.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
  2018-07-03  3:46     ` Dan Williams
  2018-07-03  4:58       ` Elliott, Robert (Persistent Memory)
@ 2018-07-03 18:00       ` James Morris
  2018-07-03 19:21         ` Dan Williams
  1 sibling, 1 reply; 23+ messages in thread
From: James Morris @ 2018-07-03 18:00 UTC (permalink / raw)
  To: Dan Williams
  Cc: Schofield, Alison, keescook, linux-nvdimm, dhowells, keyrings

On Mon, 2 Jul 2018, Dan Williams wrote:

> If an attacker can run arbitrary code in the kernel they can get the
> key from the ring directly, or turn on ACPI debug. A platform could
> arrange for the DIMMs to be unlocked pre-OS to minimize passphrase
> exposure, 

So, either from within UEFI secure boot, or via the bootloader?

-- 
James Morris
<jmorris@namei.org>

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs
  2018-07-03 18:00       ` James Morris
@ 2018-07-03 19:21         ` Dan Williams
  0 siblings, 0 replies; 23+ messages in thread
From: Dan Williams @ 2018-07-03 19:21 UTC (permalink / raw)
  To: James Morris
  Cc: Schofield, Alison, keescook, linux-nvdimm, dhowells, keyrings

On Tue, Jul 3, 2018 at 11:00 AM, James Morris <jmorris@namei.org> wrote:
> On Mon, 2 Jul 2018, Dan Williams wrote:
>
>> If an attacker can run arbitrary code in the kernel they can get the
>> key from the ring directly, or turn on ACPI debug. A platform could
>> arrange for the DIMMs to be unlocked pre-OS to minimize passphrase
>> exposure,
>
> So, either from within UEFI secure boot, or via the bootloader?

Correct. The ATA security model that these commands are based on
assumes a laptop/desktop style interactive input of the hardware
passphrase. However, for servers that do unattended boots and
potentially retrieve the key from a hosted data center key service,
the proposal is to use the kernel's keyctl service to communicate the
passphrase to the kernel.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 02/11] libnvdimm: create keyring to store security keys
  2018-07-02 23:39 ` [PATCH 02/11] libnvdimm: create keyring to store security keys Dave Jiang
@ 2018-07-03 22:16   ` Dan Williams
  0 siblings, 0 replies; 23+ messages in thread
From: Dan Williams @ 2018-07-03 22:16 UTC (permalink / raw)
  To: Dave Jiang
  Cc: David Howells, Schofield,
	Alison  <alison.schofield@intel.com>,
	keyrings@vger.kernel.org, Kees Cook, linux-nvdimm

On Mon, Jul 2, 2018 at 4:39 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Prepping the libnvdimm to support security management by adding a keyring
> in order to provide passphrase management through the kernel key management
> APIs.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/nvdimm/dimm.c     |   90 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/libnvdimm.h |    5 +++
>  2 files changed, 95 insertions(+)
>
> diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c
> index 6c8fb7590838..ee0c68efa82a 100644
> --- a/drivers/nvdimm/dimm.c
> +++ b/drivers/nvdimm/dimm.c
> @@ -18,9 +18,46 @@
>  #include <linux/slab.h>
>  #include <linux/mm.h>
>  #include <linux/nd.h>
> +#include <linux/cred.h>
> +#include <keys/user-type.h>
> +#include <linux/key-type.h>
> +#include <linux/keyctl.h>
>  #include "label.h"
>  #include "nd.h"
>
> +const struct cred *nvdimm_cred;

I assume this can be marked static?

> +static int nvdimm_key_instantiate(struct key *key,
> +               struct key_preparsed_payload *prep);
> +static void nvdimm_key_destroy(struct key *key);
> +
> +struct key_type nvdimm_key_type = {
> +       .name = "nvdimm",
> +       .instantiate = nvdimm_key_instantiate,
> +       .destroy = nvdimm_key_destroy,
> +       .describe = user_describe,
> +       .def_datalen = NVDIMM_DEFAULT_PASSPHRASE_LEN * 2,

Perhaps a comment on why the "* 2"?

> +};
> +
> +static int nvdimm_key_instantiate(struct key *key,
> +               struct key_preparsed_payload *prep)
> +{
> +       char *payload;
> +
> +       payload = kzalloc(nvdimm_key_type.def_datalen, GFP_KERNEL);
> +       if (!payload)
> +               return -ENOMEM;
> +
> +       key->datalen = min(nvdimm_key_type.def_datalen, prep->datalen);
> +       memcpy(payload, prep->data, key->datalen);
> +       key->payload.data[0] = payload;
> +       return 0;
> +}
> +
> +static void nvdimm_key_destroy(struct key *key)
> +{
> +       kfree(key->payload.data[0]);
> +}
> +

The above appears to be generic infrastructure less associated with
the dimm driver and more the libnvdimm core. I think it should live in
drivers/nvdimm/dimm_devs.c.

>  static int nvdimm_probe(struct device *dev)
>  {
>         struct nvdimm_drvdata *ndd;
> @@ -129,13 +166,66 @@ static struct nd_device_driver nvdimm_driver = {
>         .type = ND_DRIVER_DIMM,
>  };
>
> +static int nvdimm_register_keyring(void)
> +{
> +       struct cred *cred;
> +       struct key *keyring;
> +       int rc;
> +
> +       rc = register_key_type(&nvdimm_key_type);
> +       if (rc < 0)
> +               return rc;
> +
> +       cred = prepare_kernel_cred(NULL);
> +       if (!cred) {
> +               rc = -ENOMEM;
> +               goto failed_cred;
> +       }
> +
> +       keyring = keyring_alloc(".nvdimm",
> +                       GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, cred,
> +                       (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> +                       (KEY_USR_ALL & ~KEY_USR_SETATTR),
> +                       KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
> +       if (IS_ERR(keyring)) {
> +               rc = PTR_ERR(keyring);
> +               goto failed_keyring;
> +       }
> +
> +       set_bit(KEY_FLAG_ROOT_CAN_CLEAR, &keyring->flags);
> +       cred->thread_keyring = keyring;
> +       cred->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING;
> +       nvdimm_cred = cred;
> +       return 0;
> +
> + failed_cred:
> +       unregister_key_type(&nvdimm_key_type);
> + failed_keyring:
> +       put_cred(cred);
> +       return rc;
> +}
> +
> +static void nvdimm_unregister_keyring(void)
> +{
> +       key_revoke(nvdimm_cred->thread_keyring);
> +       unregister_key_type(&nvdimm_key_type);
> +       put_cred(nvdimm_cred);
> +}
> +
>  int __init nvdimm_init(void)
>  {
> +       int rc;
> +
> +       rc = nvdimm_register_keyring();
> +       if (rc < 0)
> +               return rc;
> +
>         return nd_driver_register(&nvdimm_driver);
>  }
>
>  void nvdimm_exit(void)
>  {
> +       nvdimm_unregister_keyring();
>         driver_unregister(&nvdimm_driver.drv);
>  }

Yeah, the keyring registration infrastructure looks like details that
should be moved to drivers/nvdimm/dimm_devs.c. Would need to introduce
nvdimm_devs_init() and call it from libnvdimm_init(), but at least
nvdimm_devs_exit() already exists.

>
> diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h
> index 472171af7f60..a250ff2a30df 100644
> --- a/include/linux/libnvdimm.h
> +++ b/include/linux/libnvdimm.h
> @@ -155,6 +155,11 @@ static inline struct nd_blk_region_desc *to_blk_region_desc(
>
>  }
>
> +extern struct key_type nvdimm_key_type;
> +
> +#define NVDIMM_DEFAULT_PASSPHRASE_LEN          32
> +#define NVDIMM_DEFAULT_DESC_LEN                        32

Why the "_DEFAULT" designation? I.e. just shorten to
NVDIMM_PASSPHRASE_LEN and NVDIMM_KEY_DESC_LEN.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

* Re: [PATCH 05/11] nfit/libnvdimm: add set passphrase support for Intel nvdimms
  2018-07-02 23:39 ` [PATCH 05/11] nfit/libnvdimm: add set passphrase support for Intel nvdimms Dave Jiang
@ 2018-07-03 22:48   ` Dan Williams
  0 siblings, 0 replies; 23+ messages in thread
From: Dan Williams @ 2018-07-03 22:48 UTC (permalink / raw)
  To: Dave Jiang
  Cc: David Howells, Schofield,
	Alison  <alison.schofield@intel.com>,
	keyrings@vger.kernel.org, Kees Cook, linux-nvdimm

On Mon, Jul 2, 2018 at 4:39 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Adding support for setting and/or updating passphrase on the Intel nvdimms.
> The passphrase is pulled from userspace through the kernel key management.
> We trigger the update via writing "update" to the sysfs attribute
> "security". The state of the security can also be read via the "security"
> attribute. libnvdimm will generically support the key_change API call.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/acpi/nfit/intel.c  |   54 ++++++++++++++++++++++
>  drivers/nvdimm/dimm_devs.c |  110 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/libnvdimm.h  |    4 ++
>  3 files changed, 168 insertions(+)
>
> diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
> index 1bdb493694e0..792a5c694a9e 100644
> --- a/drivers/acpi/nfit/intel.c
> +++ b/drivers/acpi/nfit/intel.c
> @@ -16,6 +16,59 @@
>  #include <acpi/nfit.h>
>  #include "nfit.h"
>
> +static int intel_dimm_security_update_passphrase(
> +               struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
> +               const char *old_pass, const char *new_pass)

Raw strings concern me especially if we have future security
implementations that are not string passphrase based. How about having
this routine and the other security ops take 'struct nvdimm_key_data
*' arguments where:

struct nvdmm_key_data {
    u8 data[NVDIMM_PASSPHRASE_LEN];
};

> +{
> +       struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus);
> +       int cmd_rc, rc = 0, pkg_size;
> +       struct nd_intel_set_passphrase *cmd;
> +       struct nd_cmd_pkg *pkg;
> +       struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
> +
> +       if (!test_bit(NVDIMM_INTEL_SET_PASSPHRASE, &nfit_mem->dsm_mask))
> +               return -ENOTTY;
> +
> +       pkg_size = sizeof(*pkg) + sizeof(*cmd);
> +       pkg = kzalloc(pkg_size, GFP_KERNEL);
> +       if (!pkg)
> +               return -ENOMEM;

I think these commands are small enough to allocate on the stack, but
you would need to wrap it.

struct {
    struct nd_cmd_pkg pkg;
    struct nd_intel_set_passphrase cmd;
} cmd = {
    .pkg = {
    ...
    },
    .cmd = {
    ...
    },
};

This would save the memory allocation and the cast of nd_payload.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

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

end of thread, other threads:[~2018-07-03 22:48 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-02 23:39 [PATCH 00/11] Adding security support for nvdimm Dave Jiang
2018-07-02 23:39 ` [PATCH 01/11] nfit: adding support for Intel DSM 1.7 commands Dave Jiang
2018-07-02 23:49   ` Dan Williams
2018-07-02 23:39 ` [PATCH 02/11] libnvdimm: create keyring to store security keys Dave Jiang
2018-07-03 22:16   ` Dan Williams
2018-07-02 23:39 ` [PATCH 03/11] nfit/libnvdimm: store dimm id as a member to struct nvdimm Dave Jiang
2018-07-03  1:04   ` Dan Williams
2018-07-02 23:39 ` [PATCH 04/11] nfit/libnvdimm: add unlock of nvdimm support for Intel DIMMs Dave Jiang
2018-07-03  1:45   ` Elliott, Robert (Persistent Memory)
2018-07-03  3:46     ` Dan Williams
2018-07-03  4:58       ` Elliott, Robert (Persistent Memory)
2018-07-03  5:03         ` Dan Williams
2018-07-03 18:00       ` James Morris
2018-07-03 19:21         ` Dan Williams
2018-07-03  3:48   ` Dan Williams
2018-07-02 23:39 ` [PATCH 05/11] nfit/libnvdimm: add set passphrase support for Intel nvdimms Dave Jiang
2018-07-03 22:48   ` Dan Williams
2018-07-02 23:39 ` [PATCH 06/11] nfit/libnvdimm: add disable passphrase support to Intel nvdimm Dave Jiang
2018-07-02 23:39 ` [PATCH 07/11] nfit/libnvdimm: add freeze security " Dave Jiang
2018-07-02 23:39 ` [PATCH 08/11] nfit/libnvdimm: add support for issue secure erase DSM " Dave Jiang
2018-07-02 23:39 ` [PATCH 09/11] nfit_test: adding context to dimm_dev for nfit_test Dave Jiang
2018-07-02 23:40 ` [PATCH 10/11] nfit_test: adding test support for Intel nvdimm security DSMs Dave Jiang
2018-07-02 23:40 ` [PATCH 11/11] libnvdimm: adding documentation for nvdimm security support Dave Jiang

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