| Message ID | 154180163666.70506.8805433934495072699.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Additional patches for nvdimm security support | expand |
On Fri, Nov 9, 2018 at 2:13 PM Dave Jiang <dave.jiang@intel.com> wrote: > > In order to make nvdimm more secure, encrypted keys will be used instead of > clear text keys. A master key will be created to seal encrypted nvdimm > keys. The master key can be a trusted key generated from TPM 2.0 or a less > secure user key. > > In the process of this conversion, the kernel cached key will be removed > in order to simplify the verification process. The hardware will be used to > verify the decrypted user payload directly. > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > Documentation/nvdimm/security.txt | 29 ++- > drivers/nvdimm/dimm.c | 3 > drivers/nvdimm/dimm_devs.c | 2 > drivers/nvdimm/nd-core.h | 3 > drivers/nvdimm/nd.h | 5 - > drivers/nvdimm/security.c | 316 ++++++++++--------------------------- > 6 files changed, 108 insertions(+), 250 deletions(-) Remove twice the amount of code that it adds and gains features / security, nice!
On Fri, 2018-11-09 at 15:13 -0700, Dave Jiang wrote: > In order to make nvdimm more secure, encrypted keys will be used instead of > clear text keys. A master key will be created to seal encrypted nvdimm > keys. The master key can be a trusted key generated from TPM 2.0 or a less > secure user key. Trusted keys also work for TPM 1.2. Are you intentionally limiting the master key to TPM 2.0? Traditionally there is a single master key for the system, which would be sealed to a set of boot time PCR values. After decrypting all of the encrypted keys, the master key would be removed from the keyring and a PCR extended. Extending a PCR would prevent the master key from being unsealed again and used to decrypt encrypted keys, without rebooting the system. Normally this would be done before pivoting root. If you're not referring to the system master key and are intentionally limiting usage to TPM 2.0, more details on the master key security requirements should be included. Using trusted keys that are encrypted/decrypted using a user key should really be limited to testing environments. > > In the process of this conversion, the kernel cached key will be removed > in order to simplify the verification process. The hardware will be used to > verify the decrypted user payload directly. Making this sort of change implies there is no concern in breaking existing userspace. Either the code hasn't yet been upstreamed or there are not any users. If the code hasn't been upstreamed, it would make more sense to squash the git history: - making code review easier - making the git history bisect safe Mimi > > Signed-off-by: Dave Jiang <dave.jiang@intel.com> > --- > Documentation/nvdimm/security.txt | 29 ++- > drivers/nvdimm/dimm.c | 3 > drivers/nvdimm/dimm_devs.c | 2 > drivers/nvdimm/nd-core.h | 3 > drivers/nvdimm/nd.h | 5 - > drivers/nvdimm/security.c | 316 ++++++++++--------------------------- > 6 files changed, 108 insertions(+), 250 deletions(-) > > diff --git a/Documentation/nvdimm/security.txt b/Documentation/nvdimm/security.txt > index 50cbb6cb96a1..11240ce48755 100644 > --- a/Documentation/nvdimm/security.txt > +++ b/Documentation/nvdimm/security.txt > @@ -39,34 +39,39 @@ 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 > +key "passphrase" is expected to be 32bytes long. 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. > > -A nvdimm user logon key has the description format of: > +A nvdimm encrypted key has the description format of: > nvdimm:<bus-provider-specific-unique-id> > > +See Documentation/security/keys/trusted-encrypted.rst for details on encrypted > +keys. The encrypted key for the DIMM is generated from a master key that is > +either a trusted key created via TPM2 or a less secure user key. The payload > +for the nvdimm encrypted key is randomly generated by the kernel and is not > +visible to the user. The user is responsible for retaining the encrypted key > +blob generated from the encrypted key to be loaded at the next boot session. > + > 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. > > 5. Update > --------- > -When doing an update, it is expected that the new key with the 64bit payload of > -format described above is added via the keyutils API or utility. The update > -command written to the sysfs attribute will be with the format: > +When doing an update, it is expected that the new key is added via the > +keyutils API or utility. The update command written to the sysfs attribute > +will be with the format: > update <old keyid> <new keyid> > > -It is expected that a user logon key has been injected via keyutils to provide > -the payload for the update operation. The kernel will take the new user key, > -attempt the update operation with the nvdimm, and replace the existing key's > -payload with the new passphrase. > +It is expected that a encrypted key has been injected via keyutils to provide > +the payload for the update operation. The kernel will take the new user key > +and the old user key to attempt the update operation. > > If there is no old key id due to a security enabling, then a 0 should be > passed in. If a nvdimm has an existing passphrase, then an "old" key should > @@ -80,7 +85,7 @@ frozen by a user with root privelege. > 7. Disable > ---------- > The security disable command format is: > -disable <old keyid> > +disable <current keyid> > > An "old" key with the passphrase payload that is tied to the nvdimm should be > injected with a key description that does not have the "nvdimm:" prefix and > @@ -89,7 +94,7 @@ its keyid should be passed in via sysfs. > 8. Secure Erase > --------------- > The command format for doing a secure erase is: > -erase <old keyid> > +erase <current keyid> > > An "old" key with the passphrase payload that is tied to the nvdimm should be > injected with a key description that does not have the "nvdimm:" prefix and > diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c > index 225a8474c44a..c3886a22b5e4 100644 > --- a/drivers/nvdimm/dimm.c > +++ b/drivers/nvdimm/dimm.c > @@ -112,9 +112,6 @@ static int nvdimm_probe(struct device *dev) > static int nvdimm_remove(struct device *dev) > { > struct nvdimm_drvdata *ndd = dev_get_drvdata(dev); > - struct nvdimm *nvdimm = to_nvdimm(dev); > - > - nvdimm_security_release(nvdimm); > > if (!ndd) > return 0; > diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c > index 5d34251c4f3b..269f401a5e68 100644 > --- a/drivers/nvdimm/dimm_devs.c > +++ b/drivers/nvdimm/dimm_devs.c > @@ -485,7 +485,7 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_dat > nvdimm->num_flush = num_flush; > nvdimm->flush_wpq = flush_wpq; > atomic_set(&nvdimm->busy, 0); > - mutex_init(&nvdimm->key_mutex); > + mutex_init(&nvdimm->sec_mutex); > dev = &nvdimm->dev; > dev_set_name(dev, "nmem%d", nvdimm->id); > dev->parent = &nvdimm_bus->dev; > diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h > index 6e2877996a9b..cfb87f71db8d 100644 > --- a/drivers/nvdimm/nd-core.h > +++ b/drivers/nvdimm/nd-core.h > @@ -45,8 +45,7 @@ struct nvdimm { > const char *dimm_id; > const struct nvdimm_security_ops *security_ops; > enum nvdimm_security_state state; > - struct key *key; > - struct mutex key_mutex; > + struct mutex sec_mutex; > }; > > /** > diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h > index 892f127bfd38..f8d8f0a2a40d 100644 > --- a/drivers/nvdimm/nd.h > +++ b/drivers/nvdimm/nd.h > @@ -428,7 +428,6 @@ bool pmem_should_map_pages(struct device *dev); > > #ifdef CONFIG_NVDIMM_SECURITY > int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm); > -void nvdimm_security_release(struct nvdimm *nvdimm); > int nvdimm_security_get_state(struct nvdimm *nvdimm); > int nvdimm_security_change_key(struct nvdimm *nvdimm, unsigned int old_keyid, > unsigned int new_keyid); > @@ -441,10 +440,6 @@ static inline int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) > return 0; > } > > -static inline void nvdimm_security_release(struct nvdimm *nvdimm) > -{ > -} > - > static inline int nvdimm_security_get_state(struct nvdimm *nvdimm) > { > return -EOPNOTSUPP; > diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c > index 5aacc590b4c0..ee741199d623 100644 > --- a/drivers/nvdimm/security.c > +++ b/drivers/nvdimm/security.c > @@ -10,43 +10,10 @@ > #include <linux/key.h> > #include <linux/key-type.h> > #include <keys/user-type.h> > +#include <keys/encrypted-type.h> > #include "nd-core.h" > #include "nd.h" > > -/* > - * Replacing the user key with a kernel key. The function expects that > - * we hold the sem for the key passed in. The function will release that > - * sem when done process. We will also hold the sem for the valid new key > - * returned. > - */ > -static struct key *make_kernel_key(struct key *key) > -{ > - struct key *new_key; > - struct user_key_payload *payload; > - int rc; > - > - new_key = key_alloc(&key_type_logon, key->description, > - GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(), > - KEY_POS_ALL & ~KEY_POS_SETATTR, > - KEY_ALLOC_NOT_IN_QUOTA, NULL); > - if (IS_ERR(new_key)) > - return NULL; > - > - payload = key->payload.data[0]; > - rc = key_instantiate_and_link(new_key, payload->data, > - payload->datalen, NULL, NULL); > - up_read(&key->sem); > - if (rc < 0) { > - key_put(new_key); > - return NULL; > - } > - > - key_put(key); > - > - down_read(&new_key->sem); > - return new_key; > -} > - > /* > * Retrieve user injected key > */ > @@ -61,6 +28,10 @@ static struct key *nvdimm_lookup_user_key(struct device *dev, > return NULL; > > key = key_ref_to_ptr(keyref); > + if (key->type != &key_type_encrypted) { > + key_put(key); > + return NULL; > + } > dev_dbg(dev, "%s: key found: %#x\n", __func__, key_serial(key)); > > return key; > @@ -76,7 +47,7 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm) > struct device *dev = &nvdimm->dev; > > sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id); > - key = request_key(&key_type_logon, desc, ""); > + key = request_key(&key_type_encrypted, desc, ""); > if (IS_ERR(key)) { > if (PTR_ERR(key) == -ENOKEY) > dev_warn(dev, "request_key() found no key\n"); > @@ -88,52 +59,6 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm) > return key; > } > > -struct key *nvdimm_get_and_verify_key(struct nvdimm *nvdimm, > - unsigned int user_key_id) > -{ > - int rc; > - struct key *user_key, *key; > - struct device *dev = &nvdimm->dev; > - struct user_key_payload *upayload, *payload; > - > - lockdep_assert_held(&nvdimm->key_mutex); > - key = nvdimm->key; > - if (!key) { > - dev_dbg(dev, "No cached kernel key\n"); > - return ERR_PTR(-EAGAIN);; > - } > - dev_dbg(dev, "cached_key: %#x\n", key_serial(key)); > - > - user_key = nvdimm_lookup_user_key(dev, user_key_id); > - if (!user_key) { > - dev_dbg(dev, "Old user key lookup failed\n"); > - return ERR_PTR(-EINVAL); > - } > - dev_dbg(dev, "user_key: %#x\n", key_serial(user_key)); > - > - down_read(&key->sem); > - down_read(&user_key->sem); > - payload = key->payload.data[0]; > - upayload = user_key->payload.data[0]; > - > - rc = memcmp(payload->data, upayload->data, NVDIMM_PASSPHRASE_LEN); > - up_read(&user_key->sem); > - key_put(user_key); > - up_read(&key->sem); > - > - if (rc != 0) { > - dev_warn(dev, "Supplied old user key fails check.\n"); > - return ERR_PTR(-EINVAL); > - } > - return key; > -} > - > -static void key_destroy(struct key *key) > -{ > - key_invalidate(key); > - key_put(key); > -} > - > int nvdimm_security_get_state(struct nvdimm *nvdimm) > { > if (!nvdimm->security_ops) > @@ -146,14 +71,15 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid) > { > int rc = 0; > struct key *key; > - struct user_key_payload *payload; > + struct encrypted_key_payload *epayload; > struct device *dev = &nvdimm->dev; > + void *data; > > if (!nvdimm->security_ops) > return -EOPNOTSUPP; > > nvdimm_bus_lock(dev); > - mutex_lock(&nvdimm->key_mutex); > + mutex_lock(&nvdimm->sec_mutex); > if (atomic_read(&nvdimm->busy)) { > dev_warn(dev, "Unable to secure erase while DIMM active.\n"); > rc = -EBUSY; > @@ -166,26 +92,23 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid) > goto out; > } > > - /* look for a key from cached key if exists */ > - key = nvdimm_get_and_verify_key(nvdimm, keyid); > - if (IS_ERR(key)) { > + key = nvdimm_lookup_user_key(dev, keyid); > + if (!key) { > dev_dbg(dev, "Unable to get and verify key\n"); > - rc = PTR_ERR(key); > + rc = -ENOKEY; > goto out; > } > > down_read(&key->sem); > - payload = key->payload.data[0]; > + epayload = dereference_key_locked(key); > + data = epayload->decrypted_data; > rc = nvdimm->security_ops->erase(nvdimm, > - (const struct nvdimm_key_data *)payload->data); > + (const struct nvdimm_key_data *)data); > up_read(&key->sem); > - > - /* remove key since secure erase kills the passphrase */ > - key_destroy(key); > - nvdimm->key = NULL; > + key_put(key); > > out: > - mutex_unlock(&nvdimm->key_mutex); > + mutex_unlock(&nvdimm->sec_mutex); > nvdimm_bus_unlock(dev); > nvdimm_security_get_state(nvdimm); > return rc; > @@ -201,11 +124,15 @@ int nvdimm_security_freeze_lock(struct nvdimm *nvdimm) > if (nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED) > return -EOPNOTSUPP; > > + mutex_lock(&nvdimm->sec_mutex); > rc = nvdimm->security_ops->freeze_lock(nvdimm); > - if (rc < 0) > + if (rc < 0) { > + mutex_unlock(&nvdimm->sec_mutex); > return rc; > + } > > nvdimm_security_get_state(nvdimm); > + mutex_unlock(&nvdimm->sec_mutex); > return 0; > } > > @@ -213,8 +140,9 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid) > { > int rc; > struct key *key; > - struct user_key_payload *payload; > + struct encrypted_key_payload *epayload; > struct device *dev = &nvdimm->dev; > + void *data; > > if (!nvdimm->security_ops) > return -EOPNOTSUPP; > @@ -222,57 +150,52 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid) > if (nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED) > return -EOPNOTSUPP; > > - mutex_lock(&nvdimm->key_mutex); > + mutex_lock(&nvdimm->sec_mutex); > /* look for a key from cached key */ > - key = nvdimm_get_and_verify_key(nvdimm, keyid); > - if (IS_ERR(key)) { > - mutex_unlock(&nvdimm->key_mutex); > - return PTR_ERR(key); > + key = nvdimm_lookup_user_key(dev, keyid); > + if (!key) { > + mutex_unlock(&nvdimm->sec_mutex); > + return -ENOKEY; > } > > down_read(&key->sem); > - payload = key->payload.data[0]; > + epayload = dereference_key_locked(key); > + data = epayload->decrypted_data; > > rc = nvdimm->security_ops->disable(nvdimm, > - (const struct nvdimm_key_data *)payload->data); > + (const struct nvdimm_key_data *)data); > up_read(&key->sem); > - if (rc < 0) { > + if (rc < 0) > dev_warn(dev, "security disable failed\n"); > - goto out; > - } > > - /* If we succeed then remove the key */ > - key_destroy(key); > - nvdimm->key = NULL; > - > - out: > - mutex_unlock(&nvdimm->key_mutex); > + key_put(key); > nvdimm_security_get_state(nvdimm); > + mutex_unlock(&nvdimm->sec_mutex); > return rc; > } > > -static int nvdimm_self_verify_key(struct nvdimm *nvdimm) > +static struct key *nvdimm_self_verify_key(struct nvdimm *nvdimm) > { > struct key *key; > - struct user_key_payload *payload; > + struct encrypted_key_payload *epayload; > void *data; > int rc; > > - lockdep_assert_held(&nvdimm->key_mutex); > - > + lockdep_assert_held(&nvdimm->sec_mutex); > key = nvdimm_request_key(nvdimm); > if (!key) > - return -ENOKEY; > + return NULL; > + > + down_read(&key->sem); > + epayload = dereference_key_locked(key); > + data = epayload->decrypted_data; > > - if (key->datalen != NVDIMM_PASSPHRASE_LEN) { > + if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) { > key_put(key); > - return -EINVAL; > + key = NULL; > + goto out; > } > > - down_read(&key->sem); > - payload = key->payload.data[0]; > - data = payload->data; > - > /* > * We send the same key to the hardware as new and old key to > * verify that the key is good. > @@ -280,19 +203,21 @@ static int nvdimm_self_verify_key(struct nvdimm *nvdimm) > rc = nvdimm->security_ops->change_key(nvdimm, data, data); > if (rc < 0) { > key_put(key); > - return rc; > + key = NULL; > } > + > +out: > up_read(&key->sem); > - nvdimm->key = key; > - return 0; > + return key; > } > > int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) > { > - struct key *key; > - int rc = -ENXIO; > - struct user_key_payload *payload; > + struct key *key = NULL; > + int rc; > + struct encrypted_key_payload *epayload; > struct device *dev = &nvdimm->dev; > + void *data; > > if (!nvdimm->security_ops) > return 0; > @@ -301,7 +226,7 @@ int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) > nvdimm->state == NVDIMM_SECURITY_DISABLED) > return 0; > > - mutex_lock(&nvdimm->key_mutex); > + mutex_lock(&nvdimm->sec_mutex); > /* > * If the pre-OS has unlocked the DIMM, we will attempt to send > * the key from request_key() to the hardware for verification. > @@ -310,60 +235,50 @@ int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) > * other security operations. > */ > if (nvdimm->state == NVDIMM_SECURITY_UNLOCKED) { > - rc = nvdimm_self_verify_key(nvdimm); > - if (rc < 0) { > + key = nvdimm_self_verify_key(nvdimm); > + if (!key) { > rc = nvdimm_security_freeze_lock(nvdimm); > - mutex_unlock(&nvdimm->key_mutex); > - return rc; > + mutex_unlock(&nvdimm->sec_mutex); > + return -ENOKEY; > } > } > > - key = nvdimm->key; > - if (!key) { > + if (!key) > key = nvdimm_request_key(nvdimm); > - if (key && key->datalen != NVDIMM_PASSPHRASE_LEN) { > - key_put(key); > - key = NULL; > - rc = -EINVAL; > - } > - } > + > if (!key) { > - mutex_unlock(&nvdimm->key_mutex); > - return rc; > + mutex_unlock(&nvdimm->sec_mutex); > + return -ENOKEY; > } > > dev_dbg(dev, "%s: key: %#x\n", __func__, key_serial(key)); > down_read(&key->sem); > - payload = key->payload.data[0]; > + epayload = dereference_key_locked(key); > + data = epayload->decrypted_data; > + > + if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) { > + up_read(&key->sem); > + key_put(key); > + mutex_unlock(&nvdimm->sec_mutex); > + return -EINVAL; > + } > + > rc = nvdimm->security_ops->unlock(nvdimm, > - (const struct nvdimm_key_data *)payload->data); > + (const struct nvdimm_key_data *)data); > up_read(&key->sem); > > if (rc == 0) { > - if (!nvdimm->key) > - nvdimm->key = key; > nvdimm->state = NVDIMM_SECURITY_UNLOCKED; > dev_dbg(dev, "DIMM %s unlocked\n", dev_name(dev)); > - } else { > - key_invalidate(key); > - key_put(key); > - nvdimm->key = NULL; > + } else > dev_warn(dev, "Failed to unlock dimm: %s\n", dev_name(dev)); > - } > > - mutex_unlock(&nvdimm->key_mutex); > + key_put(key); > nvdimm_security_get_state(nvdimm); > + mutex_unlock(&nvdimm->sec_mutex); > return rc; > } > > -void nvdimm_security_release(struct nvdimm *nvdimm) > -{ > - mutex_lock(&nvdimm->key_mutex); > - key_put(nvdimm->key); > - nvdimm->key = NULL; > - mutex_unlock(&nvdimm->key_mutex); > -} > - > int nvdimm_security_change_key(struct nvdimm *nvdimm, > unsigned int old_keyid, unsigned int new_keyid) > { > @@ -371,7 +286,7 @@ int nvdimm_security_change_key(struct nvdimm *nvdimm, > struct key *key, *old_key; > void *old_data = NULL, *new_data; > struct device *dev = &nvdimm->dev; > - struct user_key_payload *payload, *old_payload; > + struct encrypted_key_payload *epayload, *old_epayload; > > if (!nvdimm->security_ops) > return -EOPNOTSUPP; > @@ -379,15 +294,10 @@ int nvdimm_security_change_key(struct nvdimm *nvdimm, > if (nvdimm->state == NVDIMM_SECURITY_FROZEN) > return -EBUSY; > > - mutex_lock(&nvdimm->key_mutex); > + mutex_lock(&nvdimm->sec_mutex); > /* look for a key from cached key if exists */ > - old_key = nvdimm_get_and_verify_key(nvdimm, old_keyid); > - if (IS_ERR(old_key) && PTR_ERR(old_key) == -EAGAIN) > - old_key = NULL; > - else if (IS_ERR(old_key)) { > - mutex_unlock(&nvdimm->key_mutex); > - return PTR_ERR(old_key); > - } else > + old_key = nvdimm_lookup_user_key(dev, old_keyid); > + if (old_key) > dev_dbg(dev, "%s: old key: %#x\n", __func__, > key_serial(old_key)); > > @@ -402,76 +312,28 @@ int nvdimm_security_change_key(struct nvdimm *nvdimm, > dev_dbg(dev, "%s: new key: %#x\n", __func__, key_serial(key)); > > down_read(&key->sem); > - payload = key->payload.data[0]; > - if (payload->datalen != NVDIMM_PASSPHRASE_LEN) { > - rc = -EINVAL; > - up_read(&key->sem); > - goto out; > - } > - > - /* > - * Since there is no existing key this user key will become the > - * kernel's key. > - */ > - if (!old_key) { > - key = make_kernel_key(key); > - if (!key) { > - rc = -ENOMEM; > - goto out; > - } > - } > - > - /* > - * We don't need to release key->sem here because > - * make_kernel_key() will have upgraded the user key to kernel > - * and handled the semaphore handoff. > - */ > - payload = key->payload.data[0]; > + epayload = dereference_key_locked(key); > + new_data = epayload->decrypted_data; > > if (old_key) { > down_read(&old_key->sem); > - old_payload = old_key->payload.data[0]; > - old_data = old_payload->data; > + old_epayload = dereference_key_locked(old_key); > + old_data = old_epayload->decrypted_data; > } > > - new_data = payload->data; > - > rc = nvdimm->security_ops->change_key(nvdimm, old_data, > new_data); > if (rc) > dev_warn(dev, "key update failed: %d\n", rc); > > - if (old_key) { > + if (old_key) > up_read(&old_key->sem); > - /* > - * With the key update done via hardware, we no longer need > - * the old payload and need to replace it with the new > - * payload. key_update() will acquire write sem of the > - * old key and update with new data. > - */ > - if (rc == 0) { > - rc = key_update(make_key_ref(old_key, 1), new_data, > - old_key->datalen); > - if (rc < 0) { > - dev_warn(dev, > - "kernel key update failed: %d\n", rc); > - key_destroy(old_key); > - nvdimm->key = NULL; > - } > - } > - } > up_read(&key->sem); > - > - if (!old_key) { > - if (rc == 0) { > - dev_dbg(dev, "key cached: %#x\n", key_serial(key)); > - nvdimm->key = key; > - } else > - key_destroy(key); > - } > nvdimm_security_get_state(nvdimm); > > out: > - mutex_unlock(&nvdimm->key_mutex); > + mutex_unlock(&nvdimm->sec_mutex); > + key_put(old_key); > + key_put(key); > return rc; > } >
[ add keyrings and lkml ] On Sun, Nov 11, 2018 at 9:28 AM Mimi Zohar <zohar@linux.ibm.com> wrote: > > On Fri, 2018-11-09 at 15:13 -0700, Dave Jiang wrote: > > In order to make nvdimm more secure, encrypted keys will be used instead of > > clear text keys. A master key will be created to seal encrypted nvdimm > > keys. The master key can be a trusted key generated from TPM 2.0 or a less > > secure user key. > > Trusted keys also work for TPM 1.2. Are you intentionally limiting > the master key to TPM 2.0? TPM 1.2 is supported from a software perspective, however the intersection of hardware platforms deploying security enabled NVDIMMs and TPM 1.2 might be a null set. > Traditionally there is a single master key for the system, which would > be sealed to a set of boot time PCR values. After decrypting all of > the encrypted keys, the master key would be removed from the keyring > and a PCR extended. Extending a PCR would prevent the master key from > being unsealed again and used to decrypt encrypted keys, without > rebooting the system. Normally this would be done before pivoting > root. > > If you're not referring to the system master key and are intentionally > limiting usage to TPM 2.0, more details on the master key security > requirements should be included. Oh, interesting point. I think we had been assuming a local + unsealed-at-runtime nvdimm master key rather than a system-wide master key. Yes, we need to rethink this in terms of supporting a sealed system-key. This would seem to limit security actions, outside of unlock, to always requiring a reboot. I.e. the nominal case is that we boot up and unlock the DIMMs, but any subsequent security operation like erase, or change-passphrase would require rebooting into an environment where the system-master key is unsealed. I do think re-provisioning keys and erasing DIMM contents are sufficiently exceptional events that a reboot requirement is tolerable. Is there already existing tooling around this to be able to schedule master-key related actions to be deferred to an initrd environment? > Using trusted keys that are encrypted/decrypted using a user key > should really be limited to testing environments. Makes sense. > > > > In the process of this conversion, the kernel cached key will be removed > > in order to simplify the verification process. The hardware will be used to > > verify the decrypted user payload directly. > > Making this sort of change implies there is no concern in breaking > existing userspace. Either the code hasn't yet been upstreamed or > there are not any users. If the code hasn't been upstreamed, it would > make more sense to squash the git history: > > - making code review easier > - making the git history bisect safe Yes, the old scheme is not upstream. I'll do the squash once we've finalized the key-management details. Thanks for the help Mimi.
> > Traditionally there is a single master key for the system, which would > > be sealed to a set of boot time PCR values. After decrypting all of > > the encrypted keys, the master key would be removed from the keyring > > and a PCR extended. Extending a PCR would prevent the master key from > > being unsealed again and used to decrypt encrypted keys, without > > rebooting the system. Normally this would be done before pivoting > > root. > > > > If you're not referring to the system master key and are intentionally > > limiting usage to TPM 2.0, more details on the master key security > > requirements should be included. > > Oh, interesting point. I think we had been assuming a local + > unsealed-at-runtime nvdimm master key rather than a system-wide master > key. Yes, we need to rethink this in terms of supporting a sealed > system-key. This would seem to limit security actions, outside of > unlock, to always requiring a reboot. I.e. the nominal case is that we > boot up and unlock the DIMMs, but any subsequent security operation > like erase, or change-passphrase would require rebooting into an > environment where the system-master key is unsealed. I do think > re-provisioning keys and erasing DIMM contents are sufficiently > exceptional events that a reboot requirement is tolerable. > Is there already existing tooling around this to be able to schedule > master-key related actions to be deferred to an initrd environment? There's the original dracut support for loading a masterkey, which is used by the EVM and ecryptfs dracut modules. After the last usage, the masterkey needs to be removed from the keyring. Different people over the years have wanted to add support for calculating the boot time expected PCRs values in order to reseal keys (trusted key update), but I haven't looked to see if there are any open source tools available. Mimi
On 11/11/2018 1:09 PM, Mimi Zohar wrote: >>> Traditionally there is a single master key for the system, which would >>> be sealed to a set of boot time PCR values. After decrypting all of >>> the encrypted keys, the master key would be removed from the keyring >>> and a PCR extended. Extending a PCR would prevent the master key from >>> being unsealed again and used to decrypt encrypted keys, without >>> rebooting the system. Normally this would be done before pivoting >>> root. >>> >>> If you're not referring to the system master key and are intentionally >>> limiting usage to TPM 2.0, more details on the master key security >>> requirements should be included. >> Oh, interesting point. I think we had been assuming a local + >> unsealed-at-runtime nvdimm master key rather than a system-wide master >> key. Yes, we need to rethink this in terms of supporting a sealed >> system-key. This would seem to limit security actions, outside of >> unlock, to always requiring a reboot. I.e. the nominal case is that we >> boot up and unlock the DIMMs, but any subsequent security operation >> like erase, or change-passphrase would require rebooting into an >> environment where the system-master key is unsealed. I do think >> re-provisioning keys and erasing DIMM contents are sufficiently >> exceptional events that a reboot requirement is tolerable. >> Is there already existing tooling around this to be able to schedule >> master-key related actions to be deferred to an initrd environment? > There's the original dracut support for loading a masterkey, which is > used by the EVM and ecryptfs dracut modules. After the last usage, > the masterkey needs to be removed from the keyring. How does one generate new encrypted keys with the system masterkey removed from the keyring? > > Different people over the years have wanted to add support for > calculating the boot time expected PCRs values in order to reseal keys > (trusted key update), but I haven't looked to see if there are any > open source tools available. > > Mimi >
On 11/11/2018 10:27 AM, Mimi Zohar wrote: > On Fri, 2018-11-09 at 15:13 -0700, Dave Jiang wrote: >> In order to make nvdimm more secure, encrypted keys will be used instead of >> clear text keys. A master key will be created to seal encrypted nvdimm >> keys. The master key can be a trusted key generated from TPM 2.0 or a less >> secure user key. > Trusted keys also work for TPM 1.2. Are you intentionally limiting > the master key to TPM 2.0? > > Traditionally there is a single master key for the system, which would > be sealed to a set of boot time PCR values. After decrypting all of > the encrypted keys, the master key would be removed from the keyring > and a PCR extended. Extending a PCR would prevent the master key from > being unsealed again and used to decrypt encrypted keys, without > rebooting the system. Normally this would be done before pivoting > root. > > If you're not referring to the system master key and are intentionally > limiting usage to TPM 2.0, more details on the master key security > requirements should be included. > > Using trusted keys that are encrypted/decrypted using a user key > should really be limited to testing environments. Do you have any recommendation for systems that do not support TPM? > >> In the process of this conversion, the kernel cached key will be removed >> in order to simplify the verification process. The hardware will be used to >> verify the decrypted user payload directly. > Making this sort of change implies there is no concern in breaking > existing userspace. Either the code hasn't yet been upstreamed or > there are not any users. If the code hasn't been upstreamed, it would > make more sense to squash the git history: > > - making code review easier > - making the git history bisect safe > > Mimi > >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> --- >> Documentation/nvdimm/security.txt | 29 ++- >> drivers/nvdimm/dimm.c | 3 >> drivers/nvdimm/dimm_devs.c | 2 >> drivers/nvdimm/nd-core.h | 3 >> drivers/nvdimm/nd.h | 5 - >> drivers/nvdimm/security.c | 316 ++++++++++--------------------------- >> 6 files changed, 108 insertions(+), 250 deletions(-) >> >> diff --git a/Documentation/nvdimm/security.txt b/Documentation/nvdimm/security.txt >> index 50cbb6cb96a1..11240ce48755 100644 >> --- a/Documentation/nvdimm/security.txt >> +++ b/Documentation/nvdimm/security.txt >> @@ -39,34 +39,39 @@ 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 >> +key "passphrase" is expected to be 32bytes long. 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. >> >> -A nvdimm user logon key has the description format of: >> +A nvdimm encrypted key has the description format of: >> nvdimm:<bus-provider-specific-unique-id> >> >> +See Documentation/security/keys/trusted-encrypted.rst for details on encrypted >> +keys. The encrypted key for the DIMM is generated from a master key that is >> +either a trusted key created via TPM2 or a less secure user key. The payload >> +for the nvdimm encrypted key is randomly generated by the kernel and is not >> +visible to the user. The user is responsible for retaining the encrypted key >> +blob generated from the encrypted key to be loaded at the next boot session. >> + >> 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. >> >> 5. Update >> --------- >> -When doing an update, it is expected that the new key with the 64bit payload of >> -format described above is added via the keyutils API or utility. The update >> -command written to the sysfs attribute will be with the format: >> +When doing an update, it is expected that the new key is added via the >> +keyutils API or utility. The update command written to the sysfs attribute >> +will be with the format: >> update <old keyid> <new keyid> >> >> -It is expected that a user logon key has been injected via keyutils to provide >> -the payload for the update operation. The kernel will take the new user key, >> -attempt the update operation with the nvdimm, and replace the existing key's >> -payload with the new passphrase. >> +It is expected that a encrypted key has been injected via keyutils to provide >> +the payload for the update operation. The kernel will take the new user key >> +and the old user key to attempt the update operation. >> >> If there is no old key id due to a security enabling, then a 0 should be >> passed in. If a nvdimm has an existing passphrase, then an "old" key should >> @@ -80,7 +85,7 @@ frozen by a user with root privelege. >> 7. Disable >> ---------- >> The security disable command format is: >> -disable <old keyid> >> +disable <current keyid> >> >> An "old" key with the passphrase payload that is tied to the nvdimm should be >> injected with a key description that does not have the "nvdimm:" prefix and >> @@ -89,7 +94,7 @@ its keyid should be passed in via sysfs. >> 8. Secure Erase >> --------------- >> The command format for doing a secure erase is: >> -erase <old keyid> >> +erase <current keyid> >> >> An "old" key with the passphrase payload that is tied to the nvdimm should be >> injected with a key description that does not have the "nvdimm:" prefix and >> diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c >> index 225a8474c44a..c3886a22b5e4 100644 >> --- a/drivers/nvdimm/dimm.c >> +++ b/drivers/nvdimm/dimm.c >> @@ -112,9 +112,6 @@ static int nvdimm_probe(struct device *dev) >> static int nvdimm_remove(struct device *dev) >> { >> struct nvdimm_drvdata *ndd = dev_get_drvdata(dev); >> - struct nvdimm *nvdimm = to_nvdimm(dev); >> - >> - nvdimm_security_release(nvdimm); >> >> if (!ndd) >> return 0; >> diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c >> index 5d34251c4f3b..269f401a5e68 100644 >> --- a/drivers/nvdimm/dimm_devs.c >> +++ b/drivers/nvdimm/dimm_devs.c >> @@ -485,7 +485,7 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_dat >> nvdimm->num_flush = num_flush; >> nvdimm->flush_wpq = flush_wpq; >> atomic_set(&nvdimm->busy, 0); >> - mutex_init(&nvdimm->key_mutex); >> + mutex_init(&nvdimm->sec_mutex); >> dev = &nvdimm->dev; >> dev_set_name(dev, "nmem%d", nvdimm->id); >> dev->parent = &nvdimm_bus->dev; >> diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h >> index 6e2877996a9b..cfb87f71db8d 100644 >> --- a/drivers/nvdimm/nd-core.h >> +++ b/drivers/nvdimm/nd-core.h >> @@ -45,8 +45,7 @@ struct nvdimm { >> const char *dimm_id; >> const struct nvdimm_security_ops *security_ops; >> enum nvdimm_security_state state; >> - struct key *key; >> - struct mutex key_mutex; >> + struct mutex sec_mutex; >> }; >> >> /** >> diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h >> index 892f127bfd38..f8d8f0a2a40d 100644 >> --- a/drivers/nvdimm/nd.h >> +++ b/drivers/nvdimm/nd.h >> @@ -428,7 +428,6 @@ bool pmem_should_map_pages(struct device *dev); >> >> #ifdef CONFIG_NVDIMM_SECURITY >> int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm); >> -void nvdimm_security_release(struct nvdimm *nvdimm); >> int nvdimm_security_get_state(struct nvdimm *nvdimm); >> int nvdimm_security_change_key(struct nvdimm *nvdimm, unsigned int old_keyid, >> unsigned int new_keyid); >> @@ -441,10 +440,6 @@ static inline int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) >> return 0; >> } >> >> -static inline void nvdimm_security_release(struct nvdimm *nvdimm) >> -{ >> -} >> - >> static inline int nvdimm_security_get_state(struct nvdimm *nvdimm) >> { >> return -EOPNOTSUPP; >> diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c >> index 5aacc590b4c0..ee741199d623 100644 >> --- a/drivers/nvdimm/security.c >> +++ b/drivers/nvdimm/security.c >> @@ -10,43 +10,10 @@ >> #include <linux/key.h> >> #include <linux/key-type.h> >> #include <keys/user-type.h> >> +#include <keys/encrypted-type.h> >> #include "nd-core.h" >> #include "nd.h" >> >> -/* >> - * Replacing the user key with a kernel key. The function expects that >> - * we hold the sem for the key passed in. The function will release that >> - * sem when done process. We will also hold the sem for the valid new key >> - * returned. >> - */ >> -static struct key *make_kernel_key(struct key *key) >> -{ >> - struct key *new_key; >> - struct user_key_payload *payload; >> - int rc; >> - >> - new_key = key_alloc(&key_type_logon, key->description, >> - GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(), >> - KEY_POS_ALL & ~KEY_POS_SETATTR, >> - KEY_ALLOC_NOT_IN_QUOTA, NULL); >> - if (IS_ERR(new_key)) >> - return NULL; >> - >> - payload = key->payload.data[0]; >> - rc = key_instantiate_and_link(new_key, payload->data, >> - payload->datalen, NULL, NULL); >> - up_read(&key->sem); >> - if (rc < 0) { >> - key_put(new_key); >> - return NULL; >> - } >> - >> - key_put(key); >> - >> - down_read(&new_key->sem); >> - return new_key; >> -} >> - >> /* >> * Retrieve user injected key >> */ >> @@ -61,6 +28,10 @@ static struct key *nvdimm_lookup_user_key(struct device *dev, >> return NULL; >> >> key = key_ref_to_ptr(keyref); >> + if (key->type != &key_type_encrypted) { >> + key_put(key); >> + return NULL; >> + } >> dev_dbg(dev, "%s: key found: %#x\n", __func__, key_serial(key)); >> >> return key; >> @@ -76,7 +47,7 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm) >> struct device *dev = &nvdimm->dev; >> >> sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id); >> - key = request_key(&key_type_logon, desc, ""); >> + key = request_key(&key_type_encrypted, desc, ""); >> if (IS_ERR(key)) { >> if (PTR_ERR(key) == -ENOKEY) >> dev_warn(dev, "request_key() found no key\n"); >> @@ -88,52 +59,6 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm) >> return key; >> } >> >> -struct key *nvdimm_get_and_verify_key(struct nvdimm *nvdimm, >> - unsigned int user_key_id) >> -{ >> - int rc; >> - struct key *user_key, *key; >> - struct device *dev = &nvdimm->dev; >> - struct user_key_payload *upayload, *payload; >> - >> - lockdep_assert_held(&nvdimm->key_mutex); >> - key = nvdimm->key; >> - if (!key) { >> - dev_dbg(dev, "No cached kernel key\n"); >> - return ERR_PTR(-EAGAIN);; >> - } >> - dev_dbg(dev, "cached_key: %#x\n", key_serial(key)); >> - >> - user_key = nvdimm_lookup_user_key(dev, user_key_id); >> - if (!user_key) { >> - dev_dbg(dev, "Old user key lookup failed\n"); >> - return ERR_PTR(-EINVAL); >> - } >> - dev_dbg(dev, "user_key: %#x\n", key_serial(user_key)); >> - >> - down_read(&key->sem); >> - down_read(&user_key->sem); >> - payload = key->payload.data[0]; >> - upayload = user_key->payload.data[0]; >> - >> - rc = memcmp(payload->data, upayload->data, NVDIMM_PASSPHRASE_LEN); >> - up_read(&user_key->sem); >> - key_put(user_key); >> - up_read(&key->sem); >> - >> - if (rc != 0) { >> - dev_warn(dev, "Supplied old user key fails check.\n"); >> - return ERR_PTR(-EINVAL); >> - } >> - return key; >> -} >> - >> -static void key_destroy(struct key *key) >> -{ >> - key_invalidate(key); >> - key_put(key); >> -} >> - >> int nvdimm_security_get_state(struct nvdimm *nvdimm) >> { >> if (!nvdimm->security_ops) >> @@ -146,14 +71,15 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid) >> { >> int rc = 0; >> struct key *key; >> - struct user_key_payload *payload; >> + struct encrypted_key_payload *epayload; >> struct device *dev = &nvdimm->dev; >> + void *data; >> >> if (!nvdimm->security_ops) >> return -EOPNOTSUPP; >> >> nvdimm_bus_lock(dev); >> - mutex_lock(&nvdimm->key_mutex); >> + mutex_lock(&nvdimm->sec_mutex); >> if (atomic_read(&nvdimm->busy)) { >> dev_warn(dev, "Unable to secure erase while DIMM active.\n"); >> rc = -EBUSY; >> @@ -166,26 +92,23 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid) >> goto out; >> } >> >> - /* look for a key from cached key if exists */ >> - key = nvdimm_get_and_verify_key(nvdimm, keyid); >> - if (IS_ERR(key)) { >> + key = nvdimm_lookup_user_key(dev, keyid); >> + if (!key) { >> dev_dbg(dev, "Unable to get and verify key\n"); >> - rc = PTR_ERR(key); >> + rc = -ENOKEY; >> goto out; >> } >> >> down_read(&key->sem); >> - payload = key->payload.data[0]; >> + epayload = dereference_key_locked(key); >> + data = epayload->decrypted_data; >> rc = nvdimm->security_ops->erase(nvdimm, >> - (const struct nvdimm_key_data *)payload->data); >> + (const struct nvdimm_key_data *)data); >> up_read(&key->sem); >> - >> - /* remove key since secure erase kills the passphrase */ >> - key_destroy(key); >> - nvdimm->key = NULL; >> + key_put(key); >> >> out: >> - mutex_unlock(&nvdimm->key_mutex); >> + mutex_unlock(&nvdimm->sec_mutex); >> nvdimm_bus_unlock(dev); >> nvdimm_security_get_state(nvdimm); >> return rc; >> @@ -201,11 +124,15 @@ int nvdimm_security_freeze_lock(struct nvdimm *nvdimm) >> if (nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED) >> return -EOPNOTSUPP; >> >> + mutex_lock(&nvdimm->sec_mutex); >> rc = nvdimm->security_ops->freeze_lock(nvdimm); >> - if (rc < 0) >> + if (rc < 0) { >> + mutex_unlock(&nvdimm->sec_mutex); >> return rc; >> + } >> >> nvdimm_security_get_state(nvdimm); >> + mutex_unlock(&nvdimm->sec_mutex); >> return 0; >> } >> >> @@ -213,8 +140,9 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid) >> { >> int rc; >> struct key *key; >> - struct user_key_payload *payload; >> + struct encrypted_key_payload *epayload; >> struct device *dev = &nvdimm->dev; >> + void *data; >> >> if (!nvdimm->security_ops) >> return -EOPNOTSUPP; >> @@ -222,57 +150,52 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid) >> if (nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED) >> return -EOPNOTSUPP; >> >> - mutex_lock(&nvdimm->key_mutex); >> + mutex_lock(&nvdimm->sec_mutex); >> /* look for a key from cached key */ >> - key = nvdimm_get_and_verify_key(nvdimm, keyid); >> - if (IS_ERR(key)) { >> - mutex_unlock(&nvdimm->key_mutex); >> - return PTR_ERR(key); >> + key = nvdimm_lookup_user_key(dev, keyid); >> + if (!key) { >> + mutex_unlock(&nvdimm->sec_mutex); >> + return -ENOKEY; >> } >> >> down_read(&key->sem); >> - payload = key->payload.data[0]; >> + epayload = dereference_key_locked(key); >> + data = epayload->decrypted_data; >> >> rc = nvdimm->security_ops->disable(nvdimm, >> - (const struct nvdimm_key_data *)payload->data); >> + (const struct nvdimm_key_data *)data); >> up_read(&key->sem); >> - if (rc < 0) { >> + if (rc < 0) >> dev_warn(dev, "security disable failed\n"); >> - goto out; >> - } >> >> - /* If we succeed then remove the key */ >> - key_destroy(key); >> - nvdimm->key = NULL; >> - >> - out: >> - mutex_unlock(&nvdimm->key_mutex); >> + key_put(key); >> nvdimm_security_get_state(nvdimm); >> + mutex_unlock(&nvdimm->sec_mutex); >> return rc; >> } >> >> -static int nvdimm_self_verify_key(struct nvdimm *nvdimm) >> +static struct key *nvdimm_self_verify_key(struct nvdimm *nvdimm) >> { >> struct key *key; >> - struct user_key_payload *payload; >> + struct encrypted_key_payload *epayload; >> void *data; >> int rc; >> >> - lockdep_assert_held(&nvdimm->key_mutex); >> - >> + lockdep_assert_held(&nvdimm->sec_mutex); >> key = nvdimm_request_key(nvdimm); >> if (!key) >> - return -ENOKEY; >> + return NULL; >> + >> + down_read(&key->sem); >> + epayload = dereference_key_locked(key); >> + data = epayload->decrypted_data; >> >> - if (key->datalen != NVDIMM_PASSPHRASE_LEN) { >> + if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) { >> key_put(key); >> - return -EINVAL; >> + key = NULL; >> + goto out; >> } >> >> - down_read(&key->sem); >> - payload = key->payload.data[0]; >> - data = payload->data; >> - >> /* >> * We send the same key to the hardware as new and old key to >> * verify that the key is good. >> @@ -280,19 +203,21 @@ static int nvdimm_self_verify_key(struct nvdimm *nvdimm) >> rc = nvdimm->security_ops->change_key(nvdimm, data, data); >> if (rc < 0) { >> key_put(key); >> - return rc; >> + key = NULL; >> } >> + >> +out: >> up_read(&key->sem); >> - nvdimm->key = key; >> - return 0; >> + return key; >> } >> >> int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) >> { >> - struct key *key; >> - int rc = -ENXIO; >> - struct user_key_payload *payload; >> + struct key *key = NULL; >> + int rc; >> + struct encrypted_key_payload *epayload; >> struct device *dev = &nvdimm->dev; >> + void *data; >> >> if (!nvdimm->security_ops) >> return 0; >> @@ -301,7 +226,7 @@ int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) >> nvdimm->state == NVDIMM_SECURITY_DISABLED) >> return 0; >> >> - mutex_lock(&nvdimm->key_mutex); >> + mutex_lock(&nvdimm->sec_mutex); >> /* >> * If the pre-OS has unlocked the DIMM, we will attempt to send >> * the key from request_key() to the hardware for verification. >> @@ -310,60 +235,50 @@ int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) >> * other security operations. >> */ >> if (nvdimm->state == NVDIMM_SECURITY_UNLOCKED) { >> - rc = nvdimm_self_verify_key(nvdimm); >> - if (rc < 0) { >> + key = nvdimm_self_verify_key(nvdimm); >> + if (!key) { >> rc = nvdimm_security_freeze_lock(nvdimm); >> - mutex_unlock(&nvdimm->key_mutex); >> - return rc; >> + mutex_unlock(&nvdimm->sec_mutex); >> + return -ENOKEY; >> } >> } >> >> - key = nvdimm->key; >> - if (!key) { >> + if (!key) >> key = nvdimm_request_key(nvdimm); >> - if (key && key->datalen != NVDIMM_PASSPHRASE_LEN) { >> - key_put(key); >> - key = NULL; >> - rc = -EINVAL; >> - } >> - } >> + >> if (!key) { >> - mutex_unlock(&nvdimm->key_mutex); >> - return rc; >> + mutex_unlock(&nvdimm->sec_mutex); >> + return -ENOKEY; >> } >> >> dev_dbg(dev, "%s: key: %#x\n", __func__, key_serial(key)); >> down_read(&key->sem); >> - payload = key->payload.data[0]; >> + epayload = dereference_key_locked(key); >> + data = epayload->decrypted_data; >> + >> + if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) { >> + up_read(&key->sem); >> + key_put(key); >> + mutex_unlock(&nvdimm->sec_mutex); >> + return -EINVAL; >> + } >> + >> rc = nvdimm->security_ops->unlock(nvdimm, >> - (const struct nvdimm_key_data *)payload->data); >> + (const struct nvdimm_key_data *)data); >> up_read(&key->sem); >> >> if (rc == 0) { >> - if (!nvdimm->key) >> - nvdimm->key = key; >> nvdimm->state = NVDIMM_SECURITY_UNLOCKED; >> dev_dbg(dev, "DIMM %s unlocked\n", dev_name(dev)); >> - } else { >> - key_invalidate(key); >> - key_put(key); >> - nvdimm->key = NULL; >> + } else >> dev_warn(dev, "Failed to unlock dimm: %s\n", dev_name(dev)); >> - } >> >> - mutex_unlock(&nvdimm->key_mutex); >> + key_put(key); >> nvdimm_security_get_state(nvdimm); >> + mutex_unlock(&nvdimm->sec_mutex); >> return rc; >> } >> >> -void nvdimm_security_release(struct nvdimm *nvdimm) >> -{ >> - mutex_lock(&nvdimm->key_mutex); >> - key_put(nvdimm->key); >> - nvdimm->key = NULL; >> - mutex_unlock(&nvdimm->key_mutex); >> -} >> - >> int nvdimm_security_change_key(struct nvdimm *nvdimm, >> unsigned int old_keyid, unsigned int new_keyid) >> { >> @@ -371,7 +286,7 @@ int nvdimm_security_change_key(struct nvdimm *nvdimm, >> struct key *key, *old_key; >> void *old_data = NULL, *new_data; >> struct device *dev = &nvdimm->dev; >> - struct user_key_payload *payload, *old_payload; >> + struct encrypted_key_payload *epayload, *old_epayload; >> >> if (!nvdimm->security_ops) >> return -EOPNOTSUPP; >> @@ -379,15 +294,10 @@ int nvdimm_security_change_key(struct nvdimm *nvdimm, >> if (nvdimm->state == NVDIMM_SECURITY_FROZEN) >> return -EBUSY; >> >> - mutex_lock(&nvdimm->key_mutex); >> + mutex_lock(&nvdimm->sec_mutex); >> /* look for a key from cached key if exists */ >> - old_key = nvdimm_get_and_verify_key(nvdimm, old_keyid); >> - if (IS_ERR(old_key) && PTR_ERR(old_key) == -EAGAIN) >> - old_key = NULL; >> - else if (IS_ERR(old_key)) { >> - mutex_unlock(&nvdimm->key_mutex); >> - return PTR_ERR(old_key); >> - } else >> + old_key = nvdimm_lookup_user_key(dev, old_keyid); >> + if (old_key) >> dev_dbg(dev, "%s: old key: %#x\n", __func__, >> key_serial(old_key)); >> >> @@ -402,76 +312,28 @@ int nvdimm_security_change_key(struct nvdimm *nvdimm, >> dev_dbg(dev, "%s: new key: %#x\n", __func__, key_serial(key)); >> >> down_read(&key->sem); >> - payload = key->payload.data[0]; >> - if (payload->datalen != NVDIMM_PASSPHRASE_LEN) { >> - rc = -EINVAL; >> - up_read(&key->sem); >> - goto out; >> - } >> - >> - /* >> - * Since there is no existing key this user key will become the >> - * kernel's key. >> - */ >> - if (!old_key) { >> - key = make_kernel_key(key); >> - if (!key) { >> - rc = -ENOMEM; >> - goto out; >> - } >> - } >> - >> - /* >> - * We don't need to release key->sem here because >> - * make_kernel_key() will have upgraded the user key to kernel >> - * and handled the semaphore handoff. >> - */ >> - payload = key->payload.data[0]; >> + epayload = dereference_key_locked(key); >> + new_data = epayload->decrypted_data; >> >> if (old_key) { >> down_read(&old_key->sem); >> - old_payload = old_key->payload.data[0]; >> - old_data = old_payload->data; >> + old_epayload = dereference_key_locked(old_key); >> + old_data = old_epayload->decrypted_data; >> } >> >> - new_data = payload->data; >> - >> rc = nvdimm->security_ops->change_key(nvdimm, old_data, >> new_data); >> if (rc) >> dev_warn(dev, "key update failed: %d\n", rc); >> >> - if (old_key) { >> + if (old_key) >> up_read(&old_key->sem); >> - /* >> - * With the key update done via hardware, we no longer need >> - * the old payload and need to replace it with the new >> - * payload. key_update() will acquire write sem of the >> - * old key and update with new data. >> - */ >> - if (rc == 0) { >> - rc = key_update(make_key_ref(old_key, 1), new_data, >> - old_key->datalen); >> - if (rc < 0) { >> - dev_warn(dev, >> - "kernel key update failed: %d\n", rc); >> - key_destroy(old_key); >> - nvdimm->key = NULL; >> - } >> - } >> - } >> up_read(&key->sem); >> - >> - if (!old_key) { >> - if (rc == 0) { >> - dev_dbg(dev, "key cached: %#x\n", key_serial(key)); >> - nvdimm->key = key; >> - } else >> - key_destroy(key); >> - } >> nvdimm_security_get_state(nvdimm); >> >> out: >> - mutex_unlock(&nvdimm->key_mutex); >> + mutex_unlock(&nvdimm->sec_mutex); >> + key_put(old_key); >> + key_put(key); >> return rc; >> } >>
On Mon, 2018-11-12 at 08:42 -0700, Dave Jiang wrote: > How does one generate new encrypted keys with the system masterkey > removed from the keyring? I don't think you can. Mimi
On Mon, 2018-11-12 at 08:45 -0700, Dave Jiang wrote: > > Using trusted keys that are encrypted/decrypted using a user key > > should really be limited to testing environments. > > Do you have any recommendation for systems that do not support TPM? The TPM provides certain security guarantees, which "user" type keys do not. If the system doesn't provide either a discrete or firmware TPM, use a software TPM. Mimi "
On 11/12/18 11:49 AM, Mimi Zohar wrote: > On Mon, 2018-11-12 at 08:42 -0700, Dave Jiang wrote: > >> How does one generate new encrypted keys with the system masterkey >> removed from the keyring? > > I don't think you can. Are there any documentations available showing system master-key usages? So far the docs (i.e. EVM setup) I've seen generates a trusted master key and then create the encrypted keys from that. I'm missing the understanding of how to generate encrypted keys from the system master key initially during setup. Thanks!
diff --git a/Documentation/nvdimm/security.txt b/Documentation/nvdimm/security.txt index 50cbb6cb96a1..11240ce48755 100644 --- a/Documentation/nvdimm/security.txt +++ b/Documentation/nvdimm/security.txt @@ -39,34 +39,39 @@ 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 +key "passphrase" is expected to be 32bytes long. 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. -A nvdimm user logon key has the description format of: +A nvdimm encrypted key has the description format of: nvdimm:<bus-provider-specific-unique-id> +See Documentation/security/keys/trusted-encrypted.rst for details on encrypted +keys. The encrypted key for the DIMM is generated from a master key that is +either a trusted key created via TPM2 or a less secure user key. The payload +for the nvdimm encrypted key is randomly generated by the kernel and is not +visible to the user. The user is responsible for retaining the encrypted key +blob generated from the encrypted key to be loaded at the next boot session. + 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. 5. Update --------- -When doing an update, it is expected that the new key with the 64bit payload of -format described above is added via the keyutils API or utility. The update -command written to the sysfs attribute will be with the format: +When doing an update, it is expected that the new key is added via the +keyutils API or utility. The update command written to the sysfs attribute +will be with the format: update <old keyid> <new keyid> -It is expected that a user logon key has been injected via keyutils to provide -the payload for the update operation. The kernel will take the new user key, -attempt the update operation with the nvdimm, and replace the existing key's -payload with the new passphrase. +It is expected that a encrypted key has been injected via keyutils to provide +the payload for the update operation. The kernel will take the new user key +and the old user key to attempt the update operation. If there is no old key id due to a security enabling, then a 0 should be passed in. If a nvdimm has an existing passphrase, then an "old" key should @@ -80,7 +85,7 @@ frozen by a user with root privelege. 7. Disable ---------- The security disable command format is: -disable <old keyid> +disable <current keyid> An "old" key with the passphrase payload that is tied to the nvdimm should be injected with a key description that does not have the "nvdimm:" prefix and @@ -89,7 +94,7 @@ its keyid should be passed in via sysfs. 8. Secure Erase --------------- The command format for doing a secure erase is: -erase <old keyid> +erase <current keyid> An "old" key with the passphrase payload that is tied to the nvdimm should be injected with a key description that does not have the "nvdimm:" prefix and diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c index 225a8474c44a..c3886a22b5e4 100644 --- a/drivers/nvdimm/dimm.c +++ b/drivers/nvdimm/dimm.c @@ -112,9 +112,6 @@ static int nvdimm_probe(struct device *dev) static int nvdimm_remove(struct device *dev) { struct nvdimm_drvdata *ndd = dev_get_drvdata(dev); - struct nvdimm *nvdimm = to_nvdimm(dev); - - nvdimm_security_release(nvdimm); if (!ndd) return 0; diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c index 5d34251c4f3b..269f401a5e68 100644 --- a/drivers/nvdimm/dimm_devs.c +++ b/drivers/nvdimm/dimm_devs.c @@ -485,7 +485,7 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus, void *provider_dat nvdimm->num_flush = num_flush; nvdimm->flush_wpq = flush_wpq; atomic_set(&nvdimm->busy, 0); - mutex_init(&nvdimm->key_mutex); + mutex_init(&nvdimm->sec_mutex); dev = &nvdimm->dev; dev_set_name(dev, "nmem%d", nvdimm->id); dev->parent = &nvdimm_bus->dev; diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h index 6e2877996a9b..cfb87f71db8d 100644 --- a/drivers/nvdimm/nd-core.h +++ b/drivers/nvdimm/nd-core.h @@ -45,8 +45,7 @@ struct nvdimm { const char *dimm_id; const struct nvdimm_security_ops *security_ops; enum nvdimm_security_state state; - struct key *key; - struct mutex key_mutex; + struct mutex sec_mutex; }; /** diff --git a/drivers/nvdimm/nd.h b/drivers/nvdimm/nd.h index 892f127bfd38..f8d8f0a2a40d 100644 --- a/drivers/nvdimm/nd.h +++ b/drivers/nvdimm/nd.h @@ -428,7 +428,6 @@ bool pmem_should_map_pages(struct device *dev); #ifdef CONFIG_NVDIMM_SECURITY int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm); -void nvdimm_security_release(struct nvdimm *nvdimm); int nvdimm_security_get_state(struct nvdimm *nvdimm); int nvdimm_security_change_key(struct nvdimm *nvdimm, unsigned int old_keyid, unsigned int new_keyid); @@ -441,10 +440,6 @@ static inline int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) return 0; } -static inline void nvdimm_security_release(struct nvdimm *nvdimm) -{ -} - static inline int nvdimm_security_get_state(struct nvdimm *nvdimm) { return -EOPNOTSUPP; diff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c index 5aacc590b4c0..ee741199d623 100644 --- a/drivers/nvdimm/security.c +++ b/drivers/nvdimm/security.c @@ -10,43 +10,10 @@ #include <linux/key.h> #include <linux/key-type.h> #include <keys/user-type.h> +#include <keys/encrypted-type.h> #include "nd-core.h" #include "nd.h" -/* - * Replacing the user key with a kernel key. The function expects that - * we hold the sem for the key passed in. The function will release that - * sem when done process. We will also hold the sem for the valid new key - * returned. - */ -static struct key *make_kernel_key(struct key *key) -{ - struct key *new_key; - struct user_key_payload *payload; - int rc; - - new_key = key_alloc(&key_type_logon, key->description, - GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(), - KEY_POS_ALL & ~KEY_POS_SETATTR, - KEY_ALLOC_NOT_IN_QUOTA, NULL); - if (IS_ERR(new_key)) - return NULL; - - payload = key->payload.data[0]; - rc = key_instantiate_and_link(new_key, payload->data, - payload->datalen, NULL, NULL); - up_read(&key->sem); - if (rc < 0) { - key_put(new_key); - return NULL; - } - - key_put(key); - - down_read(&new_key->sem); - return new_key; -} - /* * Retrieve user injected key */ @@ -61,6 +28,10 @@ static struct key *nvdimm_lookup_user_key(struct device *dev, return NULL; key = key_ref_to_ptr(keyref); + if (key->type != &key_type_encrypted) { + key_put(key); + return NULL; + } dev_dbg(dev, "%s: key found: %#x\n", __func__, key_serial(key)); return key; @@ -76,7 +47,7 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm) struct device *dev = &nvdimm->dev; sprintf(desc, "%s%s", NVDIMM_PREFIX, nvdimm->dimm_id); - key = request_key(&key_type_logon, desc, ""); + key = request_key(&key_type_encrypted, desc, ""); if (IS_ERR(key)) { if (PTR_ERR(key) == -ENOKEY) dev_warn(dev, "request_key() found no key\n"); @@ -88,52 +59,6 @@ static struct key *nvdimm_request_key(struct nvdimm *nvdimm) return key; } -struct key *nvdimm_get_and_verify_key(struct nvdimm *nvdimm, - unsigned int user_key_id) -{ - int rc; - struct key *user_key, *key; - struct device *dev = &nvdimm->dev; - struct user_key_payload *upayload, *payload; - - lockdep_assert_held(&nvdimm->key_mutex); - key = nvdimm->key; - if (!key) { - dev_dbg(dev, "No cached kernel key\n"); - return ERR_PTR(-EAGAIN);; - } - dev_dbg(dev, "cached_key: %#x\n", key_serial(key)); - - user_key = nvdimm_lookup_user_key(dev, user_key_id); - if (!user_key) { - dev_dbg(dev, "Old user key lookup failed\n"); - return ERR_PTR(-EINVAL); - } - dev_dbg(dev, "user_key: %#x\n", key_serial(user_key)); - - down_read(&key->sem); - down_read(&user_key->sem); - payload = key->payload.data[0]; - upayload = user_key->payload.data[0]; - - rc = memcmp(payload->data, upayload->data, NVDIMM_PASSPHRASE_LEN); - up_read(&user_key->sem); - key_put(user_key); - up_read(&key->sem); - - if (rc != 0) { - dev_warn(dev, "Supplied old user key fails check.\n"); - return ERR_PTR(-EINVAL); - } - return key; -} - -static void key_destroy(struct key *key) -{ - key_invalidate(key); - key_put(key); -} - int nvdimm_security_get_state(struct nvdimm *nvdimm) { if (!nvdimm->security_ops) @@ -146,14 +71,15 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid) { int rc = 0; struct key *key; - struct user_key_payload *payload; + struct encrypted_key_payload *epayload; struct device *dev = &nvdimm->dev; + void *data; if (!nvdimm->security_ops) return -EOPNOTSUPP; nvdimm_bus_lock(dev); - mutex_lock(&nvdimm->key_mutex); + mutex_lock(&nvdimm->sec_mutex); if (atomic_read(&nvdimm->busy)) { dev_warn(dev, "Unable to secure erase while DIMM active.\n"); rc = -EBUSY; @@ -166,26 +92,23 @@ int nvdimm_security_erase(struct nvdimm *nvdimm, unsigned int keyid) goto out; } - /* look for a key from cached key if exists */ - key = nvdimm_get_and_verify_key(nvdimm, keyid); - if (IS_ERR(key)) { + key = nvdimm_lookup_user_key(dev, keyid); + if (!key) { dev_dbg(dev, "Unable to get and verify key\n"); - rc = PTR_ERR(key); + rc = -ENOKEY; goto out; } down_read(&key->sem); - payload = key->payload.data[0]; + epayload = dereference_key_locked(key); + data = epayload->decrypted_data; rc = nvdimm->security_ops->erase(nvdimm, - (const struct nvdimm_key_data *)payload->data); + (const struct nvdimm_key_data *)data); up_read(&key->sem); - - /* remove key since secure erase kills the passphrase */ - key_destroy(key); - nvdimm->key = NULL; + key_put(key); out: - mutex_unlock(&nvdimm->key_mutex); + mutex_unlock(&nvdimm->sec_mutex); nvdimm_bus_unlock(dev); nvdimm_security_get_state(nvdimm); return rc; @@ -201,11 +124,15 @@ int nvdimm_security_freeze_lock(struct nvdimm *nvdimm) if (nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED) return -EOPNOTSUPP; + mutex_lock(&nvdimm->sec_mutex); rc = nvdimm->security_ops->freeze_lock(nvdimm); - if (rc < 0) + if (rc < 0) { + mutex_unlock(&nvdimm->sec_mutex); return rc; + } nvdimm_security_get_state(nvdimm); + mutex_unlock(&nvdimm->sec_mutex); return 0; } @@ -213,8 +140,9 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid) { int rc; struct key *key; - struct user_key_payload *payload; + struct encrypted_key_payload *epayload; struct device *dev = &nvdimm->dev; + void *data; if (!nvdimm->security_ops) return -EOPNOTSUPP; @@ -222,57 +150,52 @@ int nvdimm_security_disable(struct nvdimm *nvdimm, unsigned int keyid) if (nvdimm->state == NVDIMM_SECURITY_UNSUPPORTED) return -EOPNOTSUPP; - mutex_lock(&nvdimm->key_mutex); + mutex_lock(&nvdimm->sec_mutex); /* look for a key from cached key */ - key = nvdimm_get_and_verify_key(nvdimm, keyid); - if (IS_ERR(key)) { - mutex_unlock(&nvdimm->key_mutex); - return PTR_ERR(key); + key = nvdimm_lookup_user_key(dev, keyid); + if (!key) { + mutex_unlock(&nvdimm->sec_mutex); + return -ENOKEY; } down_read(&key->sem); - payload = key->payload.data[0]; + epayload = dereference_key_locked(key); + data = epayload->decrypted_data; rc = nvdimm->security_ops->disable(nvdimm, - (const struct nvdimm_key_data *)payload->data); + (const struct nvdimm_key_data *)data); up_read(&key->sem); - if (rc < 0) { + if (rc < 0) dev_warn(dev, "security disable failed\n"); - goto out; - } - /* If we succeed then remove the key */ - key_destroy(key); - nvdimm->key = NULL; - - out: - mutex_unlock(&nvdimm->key_mutex); + key_put(key); nvdimm_security_get_state(nvdimm); + mutex_unlock(&nvdimm->sec_mutex); return rc; } -static int nvdimm_self_verify_key(struct nvdimm *nvdimm) +static struct key *nvdimm_self_verify_key(struct nvdimm *nvdimm) { struct key *key; - struct user_key_payload *payload; + struct encrypted_key_payload *epayload; void *data; int rc; - lockdep_assert_held(&nvdimm->key_mutex); - + lockdep_assert_held(&nvdimm->sec_mutex); key = nvdimm_request_key(nvdimm); if (!key) - return -ENOKEY; + return NULL; + + down_read(&key->sem); + epayload = dereference_key_locked(key); + data = epayload->decrypted_data; - if (key->datalen != NVDIMM_PASSPHRASE_LEN) { + if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) { key_put(key); - return -EINVAL; + key = NULL; + goto out; } - down_read(&key->sem); - payload = key->payload.data[0]; - data = payload->data; - /* * We send the same key to the hardware as new and old key to * verify that the key is good. @@ -280,19 +203,21 @@ static int nvdimm_self_verify_key(struct nvdimm *nvdimm) rc = nvdimm->security_ops->change_key(nvdimm, data, data); if (rc < 0) { key_put(key); - return rc; + key = NULL; } + +out: up_read(&key->sem); - nvdimm->key = key; - return 0; + return key; } int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) { - struct key *key; - int rc = -ENXIO; - struct user_key_payload *payload; + struct key *key = NULL; + int rc; + struct encrypted_key_payload *epayload; struct device *dev = &nvdimm->dev; + void *data; if (!nvdimm->security_ops) return 0; @@ -301,7 +226,7 @@ int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) nvdimm->state == NVDIMM_SECURITY_DISABLED) return 0; - mutex_lock(&nvdimm->key_mutex); + mutex_lock(&nvdimm->sec_mutex); /* * If the pre-OS has unlocked the DIMM, we will attempt to send * the key from request_key() to the hardware for verification. @@ -310,60 +235,50 @@ int nvdimm_security_unlock_dimm(struct nvdimm *nvdimm) * other security operations. */ if (nvdimm->state == NVDIMM_SECURITY_UNLOCKED) { - rc = nvdimm_self_verify_key(nvdimm); - if (rc < 0) { + key = nvdimm_self_verify_key(nvdimm); + if (!key) { rc = nvdimm_security_freeze_lock(nvdimm); - mutex_unlock(&nvdimm->key_mutex); - return rc; + mutex_unlock(&nvdimm->sec_mutex); + return -ENOKEY; } } - key = nvdimm->key; - if (!key) { + if (!key) key = nvdimm_request_key(nvdimm); - if (key && key->datalen != NVDIMM_PASSPHRASE_LEN) { - key_put(key); - key = NULL; - rc = -EINVAL; - } - } + if (!key) { - mutex_unlock(&nvdimm->key_mutex); - return rc; + mutex_unlock(&nvdimm->sec_mutex); + return -ENOKEY; } dev_dbg(dev, "%s: key: %#x\n", __func__, key_serial(key)); down_read(&key->sem); - payload = key->payload.data[0]; + epayload = dereference_key_locked(key); + data = epayload->decrypted_data; + + if (epayload->decrypted_datalen != NVDIMM_PASSPHRASE_LEN) { + up_read(&key->sem); + key_put(key); + mutex_unlock(&nvdimm->sec_mutex); + return -EINVAL; + } + rc = nvdimm->security_ops->unlock(nvdimm, - (const struct nvdimm_key_data *)payload->data); + (const struct nvdimm_key_data *)data); up_read(&key->sem); if (rc == 0) { - if (!nvdimm->key) - nvdimm->key = key; nvdimm->state = NVDIMM_SECURITY_UNLOCKED; dev_dbg(dev, "DIMM %s unlocked\n", dev_name(dev)); - } else { - key_invalidate(key); - key_put(key); - nvdimm->key = NULL; + } else dev_warn(dev, "Failed to unlock dimm: %s\n", dev_name(dev)); - } - mutex_unlock(&nvdimm->key_mutex); + key_put(key); nvdimm_security_get_state(nvdimm); + mutex_unlock(&nvdimm->sec_mutex); return rc; } -void nvdimm_security_release(struct nvdimm *nvdimm) -{ - mutex_lock(&nvdimm->key_mutex); - key_put(nvdimm->key); - nvdimm->key = NULL; - mutex_unlock(&nvdimm->key_mutex); -} - int nvdimm_security_change_key(struct nvdimm *nvdimm, unsigned int old_keyid, unsigned int new_keyid) { @@ -371,7 +286,7 @@ int nvdimm_security_change_key(struct nvdimm *nvdimm, struct key *key, *old_key; void *old_data = NULL, *new_data; struct device *dev = &nvdimm->dev; - struct user_key_payload *payload, *old_payload; + struct encrypted_key_payload *epayload, *old_epayload; if (!nvdimm->security_ops) return -EOPNOTSUPP; @@ -379,15 +294,10 @@ int nvdimm_security_change_key(struct nvdimm *nvdimm, if (nvdimm->state == NVDIMM_SECURITY_FROZEN) return -EBUSY; - mutex_lock(&nvdimm->key_mutex); + mutex_lock(&nvdimm->sec_mutex); /* look for a key from cached key if exists */ - old_key = nvdimm_get_and_verify_key(nvdimm, old_keyid); - if (IS_ERR(old_key) && PTR_ERR(old_key) == -EAGAIN) - old_key = NULL; - else if (IS_ERR(old_key)) { - mutex_unlock(&nvdimm->key_mutex); - return PTR_ERR(old_key); - } else + old_key = nvdimm_lookup_user_key(dev, old_keyid); + if (old_key) dev_dbg(dev, "%s: old key: %#x\n", __func__, key_serial(old_key)); @@ -402,76 +312,28 @@ int nvdimm_security_change_key(struct nvdimm *nvdimm, dev_dbg(dev, "%s: new key: %#x\n", __func__, key_serial(key)); down_read(&key->sem); - payload = key->payload.data[0]; - if (payload->datalen != NVDIMM_PASSPHRASE_LEN) { - rc = -EINVAL; - up_read(&key->sem); - goto out; - } - - /* - * Since there is no existing key this user key will become the - * kernel's key. - */ - if (!old_key) { - key = make_kernel_key(key); - if (!key) { - rc = -ENOMEM; - goto out; - } - } - - /* - * We don't need to release key->sem here because - * make_kernel_key() will have upgraded the user key to kernel - * and handled the semaphore handoff. - */ - payload = key->payload.data[0]; + epayload = dereference_key_locked(key); + new_data = epayload->decrypted_data; if (old_key) { down_read(&old_key->sem); - old_payload = old_key->payload.data[0]; - old_data = old_payload->data; + old_epayload = dereference_key_locked(old_key); + old_data = old_epayload->decrypted_data; } - new_data = payload->data; - rc = nvdimm->security_ops->change_key(nvdimm, old_data, new_data); if (rc) dev_warn(dev, "key update failed: %d\n", rc); - if (old_key) { + if (old_key) up_read(&old_key->sem); - /* - * With the key update done via hardware, we no longer need - * the old payload and need to replace it with the new - * payload. key_update() will acquire write sem of the - * old key and update with new data. - */ - if (rc == 0) { - rc = key_update(make_key_ref(old_key, 1), new_data, - old_key->datalen); - if (rc < 0) { - dev_warn(dev, - "kernel key update failed: %d\n", rc); - key_destroy(old_key); - nvdimm->key = NULL; - } - } - } up_read(&key->sem); - - if (!old_key) { - if (rc == 0) { - dev_dbg(dev, "key cached: %#x\n", key_serial(key)); - nvdimm->key = key; - } else - key_destroy(key); - } nvdimm_security_get_state(nvdimm); out: - mutex_unlock(&nvdimm->key_mutex); + mutex_unlock(&nvdimm->sec_mutex); + key_put(old_key); + key_put(key); return rc; }
In order to make nvdimm more secure, encrypted keys will be used instead of clear text keys. A master key will be created to seal encrypted nvdimm keys. The master key can be a trusted key generated from TPM 2.0 or a less secure user key. In the process of this conversion, the kernel cached key will be removed in order to simplify the verification process. The hardware will be used to verify the decrypted user payload directly. Signed-off-by: Dave Jiang <dave.jiang@intel.com> --- Documentation/nvdimm/security.txt | 29 ++- drivers/nvdimm/dimm.c | 3 drivers/nvdimm/dimm_devs.c | 2 drivers/nvdimm/nd-core.h | 3 drivers/nvdimm/nd.h | 5 - drivers/nvdimm/security.c | 316 ++++++++++--------------------------- 6 files changed, 108 insertions(+), 250 deletions(-)