diff mbox series

[v6,05/11] nfit/libnvdimm: add set passphrase support for Intel nvdimms

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

Commit Message

Dave Jiang July 25, 2018, 8:58 p.m. UTC
Add 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  |   68 +++++++++++++++++++++++++
 drivers/nvdimm/dimm_devs.c |  120 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libnvdimm.h  |    5 ++
 3 files changed, 193 insertions(+)

Comments

David Howells Aug. 2, 2018, noon UTC | #1
Dave Jiang <dave.jiang@intel.com> wrote:

> +	/* request new key from userspace */
> +	key = nvdimm_request_key(dev, update);
> +	if (!key) {
> +		dev_dbg(dev, "%s: failed to acquire new key\n", __func__);
> +		rc = -ENXIO;
> +		goto out;
> +	}

I still think you're better taking two explicit key IDs as part of the command
rather than using request_key() to *hopefully* pick the right target.

David
Dave Jiang Aug. 2, 2018, 10:29 p.m. UTC | #2
On 08/02/2018 05:00 AM, David Howells wrote:
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> +	/* request new key from userspace */
>> +	key = nvdimm_request_key(dev, update);
>> +	if (!key) {
>> +		dev_dbg(dev, "%s: failed to acquire new key\n", __func__);
>> +		rc = -ENXIO;
>> +		goto out;
>> +	}
> 
> I still think you're better taking two explicit key IDs as part of the command
> rather than using request_key() to *hopefully* pick the right target.

In order to do this, I would need to do a key_add() in userspace to add
a new key with the new payload before I can initiate update correct? So
for an update it would look something like:
1. (user) add key with new payload
2. (user) lookup old key
3. (user) write to sysfs update attrib: "update:<old id>:<new id>"
4. (kernel) check old_id against cached key and make sure they match
5. (kernel) check new key desc against old key and make sure they match
6. (kernel) update to hardware
6. (kernel) when success, link the new key to the kernel keyring and
it'll replace the old key?
Dave Jiang Aug. 3, 2018, 12:28 a.m. UTC | #3
On 08/02/2018 03:29 PM, Dave Jiang wrote:
> 
> 
> On 08/02/2018 05:00 AM, David Howells wrote:
>> Dave Jiang <dave.jiang@intel.com> wrote:
>>
>>> +	/* request new key from userspace */
>>> +	key = nvdimm_request_key(dev, update);
>>> +	if (!key) {
>>> +		dev_dbg(dev, "%s: failed to acquire new key\n", __func__);
>>> +		rc = -ENXIO;
>>> +		goto out;
>>> +	}
>>
>> I still think you're better taking two explicit key IDs as part of the command
>> rather than using request_key() to *hopefully* pick the right target.
> 
> In order to do this, I would need to do a key_add() in userspace to add
> a new key with the new payload before I can initiate update correct? So
> for an update it would look something like:
> 1. (user) add key with new payload
> 2. (user) lookup old key
> 3. (user) write to sysfs update attrib: "update:<old id>:<new id>"
> 4. (kernel) check old_id against cached key and make sure they match
> 5. (kernel) check new key desc against old key and make sure they match
> 6. (kernel) update to hardware
> 6. (kernel) when success, link the new key to the kernel keyring and
> it'll replace the old key?

I think I'm a little confused on what to do once I pass in the new key
id through update. How do I retrieve the new key in kernel that I added
from userspace and not use key_lookup()? request_key() or something
else? Wouldn't request_key come up with the old key since it has the
same description?
David Howells Aug. 3, 2018, 8:28 a.m. UTC | #4
Dave Jiang <dave.jiang@intel.com> wrote:

> In order to do this, I would need to do a key_add() in userspace to add

Well, add_key().

> a new key with the new payload before I can initiate update correct? So
> for an update it would look something like:
> 1. (user) add key with new payload
> 2. (user) lookup old key

You don't technically need the old key - just a key with the old password in
it.  It doesn't need to have any useful description since you're providing it
directly.

> 3. (user) write to sysfs update attrib: "update:<old id>:<new id>"
> 4. (kernel) check old_id against cached key and make sure they match
> 5. (kernel) check new key desc against old key and make sure they match
> 6. (kernel) update to hardware
> 6. (kernel) when success, link the new key to the kernel keyring and
> it'll replace the old key?

Yep - provided it has the same description.  A keyring can only keep one key
of any {type, description} at any one time.  Adding a second will displace the
first.

David
David Howells Aug. 3, 2018, 8:32 a.m. UTC | #5
Dave Jiang <dave.jiang@intel.com> wrote:

> I think I'm a little confused on what to do once I pass in the new key
> id through update. How do I retrieve the new key in kernel that I added
> from userspace and not use key_lookup()? request_key() or something
> else? Wouldn't request_key come up with the old key since it has the
> same description?

As I keep saying, you need to use lookup_user_key():

	key = lookup_user_key(keyid, 0, KEY_NEED_SEARCH);

You need to add it to linux/key.h and export it.

David
Dave Jiang Aug. 3, 2018, 4:07 p.m. UTC | #6
On 08/03/2018 01:32 AM, David Howells wrote:
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> I think I'm a little confused on what to do once I pass in the new key
>> id through update. How do I retrieve the new key in kernel that I added
>> from userspace and not use key_lookup()? request_key() or something
>> else? Wouldn't request_key come up with the old key since it has the
>> same description?
> 
> As I keep saying, you need to use lookup_user_key():
> 
> 	key = lookup_user_key(keyid, 0, KEY_NEED_SEARCH);
> 
> You need to add it to linux/key.h and export it.

Thanks! I got confused with key_user_lookup() and got stuck there.
diff mbox series

Patch

diff --git a/drivers/acpi/nfit/intel.c b/drivers/acpi/nfit/intel.c
index 4bfc1c1da339..314eae7e02d7 100644
--- a/drivers/acpi/nfit/intel.c
+++ b/drivers/acpi/nfit/intel.c
@@ -18,6 +18,73 @@ 
 #include "intel.h"
 #include "nfit.h"
 
+/*
+ * The update passphrase takes the old passphrase and the new passphrase
+ * and send those to the nvdimm. The nvdimm will verify the old
+ * passphrase and then update it with the new passphrase if pending
+ * verification. The function will pass in a zeroed passphrase field
+ * if the old passphrase is NULL. This typically happens when we are
+ * enabling security from the disabled state.
+ */
+static int intel_dimm_security_update_passphrase(
+		struct nvdimm_bus *nvdimm_bus, struct nvdimm *nvdimm,
+		const struct nvdimm_key_data *old_data,
+		const struct nvdimm_key_data *new_data)
+{
+	struct nvdimm_bus_descriptor *nd_desc = to_nd_desc(nvdimm_bus);
+	int cmd_rc, rc = 0;
+	struct nfit_mem *nfit_mem = nvdimm_provider_data(nvdimm);
+	struct {
+		struct nd_cmd_pkg pkg;
+		struct nd_intel_set_passphrase cmd;
+	} nd_cmd = {
+		.pkg = {
+			.nd_command = NVDIMM_INTEL_SET_PASSPHRASE,
+			.nd_family = NVDIMM_FAMILY_INTEL,
+			.nd_size_in = ND_INTEL_PASSPHRASE_SIZE * 2,
+			.nd_size_out = ND_INTEL_STATUS_SIZE,
+			.nd_fw_size = ND_INTEL_STATUS_SIZE,
+		},
+		.cmd = {
+			.status = 0,
+		},
+	};
+
+	if (!test_bit(NVDIMM_INTEL_SET_PASSPHRASE, &nfit_mem->dsm_mask))
+		return -ENOTTY;
+
+	if (old_data)
+		memcpy(nd_cmd.cmd.old_pass, old_data->data,
+				sizeof(nd_cmd.cmd.old_pass));
+	else
+		memset(nd_cmd.cmd.old_pass, 0, sizeof(nd_cmd.cmd.old_pass));
+	memcpy(nd_cmd.cmd.new_pass, new_data->data,
+			sizeof(nd_cmd.cmd.new_pass));
+	rc = nd_desc->ndctl(nd_desc, nvdimm, ND_CMD_CALL, &nd_cmd,
+			sizeof(nd_cmd), &cmd_rc);
+	if (rc < 0)
+		goto out;
+	if (cmd_rc < 0) {
+		rc = cmd_rc;
+		goto out;
+	}
+
+	switch (nd_cmd.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:
+	return rc;
+}
+
 static int intel_dimm_security_unlock(struct nvdimm_bus *nvdimm_bus,
 		struct nvdimm *nvdimm, const struct nvdimm_key_data *nkey)
 {
@@ -149,4 +216,5 @@  static int intel_dimm_security_state(struct nvdimm_bus *nvdimm_bus,
 const 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 5b247f94807a..c6029e3f329e 100644
--- a/drivers/nvdimm/dimm_devs.c
+++ b/drivers/nvdimm/dimm_devs.c
@@ -156,6 +156,79 @@  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;
+	void *old_data, *new_data;
+	bool update = false;
+	struct user_key_payload *payload;
+
+	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: old key: %#x\n",
+				__func__, key_serial(old_key));
+		update = true;
+	}
+
+	/* request new key from userspace */
+	key = nvdimm_request_key(dev, update);
+	if (!key) {
+		dev_dbg(dev, "%s: failed to acquire new key\n", __func__);
+		rc = -ENXIO;
+		goto out;
+	}
+
+	dev_dbg(dev, "%s: new key: %#x\n", __func__, key_serial(key));
+
+	rc = nvdimm_check_key_len(key->datalen, update);
+	if (rc < 0)
+		goto out;
+
+	payload = key->payload.data[0];
+	if (!update) {
+		old_data = NULL;
+		new_data = payload->data;
+	} else {
+		new_data = payload->data;
+		old_data = new_data + NVDIMM_PASSPHRASE_LEN;
+	}
+
+	down_read(&key->sem);
+	rc = nvdimm->security_ops->change_key(nvdimm_bus, nvdimm, old_data,
+			new_data);
+	/* copy new payload to old payload */
+	if (rc == 0) {
+		if (update)
+			key_update(make_key_ref(old_key, 1), new_data,
+					old_key->datalen);
+		else
+			nvdimm->key_id = key_serial(key);
+	} else
+		dev_warn(dev, "key update failed\n");
+	up_read(&key->sem);
+
+	if (update)
+		key_invalidate(key);
+	nvdimm_security_get_state(dev);
+
+ out:
+	if (old_key)
+		key_put(old_key);
+	if (key)
+		key_put(key);
+	return rc;
+}
+
 /*
  * Retrieve bus and dimm handle and return if this bus supports
  * get_config_data commands
@@ -513,11 +586,58 @@  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)
+
+{
+	struct nvdimm *nvdimm = to_nvdimm(dev);
+	struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev);
+	ssize_t rc = -EINVAL;
+
+        wait_nvdimm_bus_probe_idle(&nvdimm_bus->dev);
+        if (atomic_read(&nvdimm->busy))
+                return -EBUSY;
+
+	if (sysfs_streq(buf, "update"))
+		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 257ff2637ce1..bd6a413164ee 100644
--- a/include/linux/libnvdimm.h
+++ b/include/linux/libnvdimm.h
@@ -160,6 +160,7 @@  enum nvdimm_security_state {
 	NVDIMM_SECURITY_DISABLED,
 	NVDIMM_SECURITY_UNLOCKED,
 	NVDIMM_SECURITY_LOCKED,
+	NVDIMM_SECURITY_FROZEN,
 	NVDIMM_SECURITY_UNSUPPORTED,
 };
 
@@ -177,6 +178,10 @@  struct nvdimm_security_ops {
 	int (*unlock)(struct nvdimm_bus *nvdimm_bus,
 			struct nvdimm *nvdimm,
 			const struct nvdimm_key_data *nkey);
+	int (*change_key)(struct nvdimm_bus *nvdimm_bus,
+			struct nvdimm *nvdimm,
+			const struct nvdimm_key_data *old_data,
+			const struct nvdimm_key_data *new_data);
 };
 
 void badrange_init(struct badrange *badrange);