[PATCH v2 1/3] ndctl: fix load-keys for user master-key

[PATCH v2 1/3] ndctl: fix load-keys for user master-key

[Dave Jiang]

load-keys incorrectly assumes that all keys have TPM handles. TPM handle is
only for trusted-keys. Fix in order to allow user master-key to operate.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---

v2:
- Make output go to stderr. (Dan)


 ndctl/load-keys.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/ndctl/load-keys.c b/ndctl/load-keys.c
index 8e4998f2..7d86a94b 100644
--- a/ndctl/load-keys.c
+++ b/ndctl/load-keys.c
@@ -213,10 +213,8 @@ static int load_keys(struct loadkeys *lk_ctx, const char *keypath,
 
 	if (!tpmhandle) {
 		rc = check_tpm_handle(lk_ctx);
-		if (rc < 0) {
-			rc = -errno;
-			goto erropen;
-		}
+		if (rc < 0)
+			fprintf(stderr, "No TPM handle discovered.\n");
 	}
 
 	rc = load_master_key(lk_ctx, param.key_path);

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

[PATCH v2 2/3] ndctl: fix key blob loading for user keys

[Dave Jiang]

The syntax for loading user master key is different than loading a trusted
key. Fix so we can load user key properly.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---

v2: No change

 ndctl/load-keys.c |   13 +++++--------
 ndctl/util/keys.c |   20 +++++++++++++++-----
 ndctl/util/keys.h |   10 ++++++++--
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/ndctl/load-keys.c b/ndctl/load-keys.c
index 7d86a94b..981f80f1 100644
--- a/ndctl/load-keys.c
+++ b/ndctl/load-keys.c
@@ -25,12 +25,7 @@ static struct parameters {
 	const char *tpm_handle;
 } param;
 
-enum key_type {
-	KEY_USER = 0,
-	KEY_TRUSTED,
-};
-
-static const char *key_names[] = {"user", "trusted"};
+static const char *key_names[] = {"user", "trusted", "encrypted"};
 
 static struct loadkeys {
 	enum key_type key_type;
@@ -44,6 +39,7 @@ static int load_master_key(struct loadkeys *lk_ctx, const char *keypath)
 	char *blob;
 	int size, rc;
 	char path[PATH_MAX];
+	enum key_type;
 
 	rc = sprintf(path, "%s/nvdimm-master.blob", keypath);
 	if (rc < 0)
@@ -65,7 +61,8 @@ static int load_master_key(struct loadkeys *lk_ctx, const char *keypath)
 		return -errno;
 	}
 
-	blob = ndctl_load_key_blob(path, &size, param.tpm_handle, -1);
+	blob = ndctl_load_key_blob(path, &size, param.tpm_handle, -1,
+			lk_ctx->key_type);
 	if (!blob)
 		return -ENOMEM;
 
@@ -122,7 +119,7 @@ static int load_dimm_keys(struct loadkeys *lk_ctx)
 		}
 
 		blob = ndctl_load_key_blob(dent->d_name, &size, NULL,
-				lk_ctx->dirfd);
+				lk_ctx->dirfd, KEY_ENCRYPTED);
 		if (!blob) {
 			free(fname);
 			continue;
diff --git a/ndctl/util/keys.c b/ndctl/util/keys.c
index 622533d7..a621a5f5 100644
--- a/ndctl/util/keys.c
+++ b/ndctl/util/keys.c
@@ -103,13 +103,17 @@ static int get_key_desc(struct ndctl_dimm *dimm, char *desc,
 }
 
 char *ndctl_load_key_blob(const char *path, int *size, const char *postfix,
-		int dirfd)
+		int dirfd, enum key_type key_type)
 {
 	struct stat st;
 	ssize_t read_bytes = 0;
 	int rc, fd;
 	char *blob, *pl, *rdptr;
 	char prefix[] = "load ";
+	bool need_prefix = false;
+
+	if (key_type == KEY_ENCRYPTED || key_type == KEY_TRUSTED)
+		need_prefix = true;
 
 	fd = openat(dirfd, path, O_RDONLY);
 	if (fd < 0) {
@@ -133,7 +137,10 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix,
 		return NULL;
 	}
 
-	*size = st.st_size + sizeof(prefix) - 1;
+	*size = st.st_size;
+	if (need_prefix)
+		*size += strlen(prefix);
+
 	/*
 	 * We need to increment postfix and space.
 	 * "keyhandle=" is 10 bytes, plus null termination.
@@ -146,8 +153,11 @@ char *ndctl_load_key_blob(const char *path, int *size, const char *postfix,
 		return NULL;
 	}
 
-	memcpy(blob, prefix, sizeof(prefix) - 1);
-	pl = blob + sizeof(prefix) - 1;
+	if (need_prefix) {
+		memcpy(blob, prefix, strlen(prefix));
+		pl = blob + strlen(prefix);
+	} else
+		pl = blob;
 
 	rdptr = pl;
 	do {
@@ -300,7 +310,7 @@ static key_serial_t dimm_load_key(struct ndctl_dimm *dimm,
 	if (rc < 0)
 		return rc;
 
-	blob = ndctl_load_key_blob(path, &size, NULL, -1);
+	blob = ndctl_load_key_blob(path, &size, NULL, -1, KEY_ENCRYPTED);
 	if (!blob)
 		return -ENOMEM;
 
diff --git a/ndctl/util/keys.h b/ndctl/util/keys.h
index eab78d2f..9bc995ac 100644
--- a/ndctl/util/keys.h
+++ b/ndctl/util/keys.h
@@ -12,9 +12,15 @@ enum ndctl_key_type {
 	ND_ZERO_KEY,
 };
 
+enum key_type {
+	KEY_USER = 0,
+	KEY_TRUSTED,
+	KEY_ENCRYPTED,
+};
+
 #ifdef ENABLE_KEYUTILS
 char *ndctl_load_key_blob(const char *path, int *size, const char *postfix,
-		int dirfd);
+		int dirfd, enum key_type key_type);
 int ndctl_dimm_setup_key(struct ndctl_dimm *dimm, const char *kek,
 				enum ndctl_key_type key_type);
 int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *kek,
@@ -25,7 +31,7 @@ int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
 int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm);
 #else
 char *ndctl_load_key_blob(const char *path, int *size, const char *postfix,
-		int dirfd)
+		int dirfd, enum key_type key_type)
 {
 	return NULL;
 }

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

[PATCH v2 3/3] ndctl: add unit test for load-keys

[Dave Jiang]

Add to security.sh to test load-keys for user keys.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---

V2:
- Add quotes around $masterkey. (Vishal)
- Change fail to failed in output. (Vishal)

 test/security.sh |   68 ++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 61 insertions(+), 7 deletions(-)

diff --git a/test/security.sh b/test/security.sh
index 1b7a9a1a..8a36265f 100755
--- a/test/security.sh
+++ b/test/security.sh
@@ -6,8 +6,10 @@ rc=77
 dev=""
 id=""
 keypath="/etc/ndctl/keys"
-masterkey="nvdimm-master-test"
-masterpath="$keypath/$masterkey"
+masterkey="nvdimm-master"
+masterpath="$keypath/$masterkey.blob"
+backup_key=0
+backup_handle=0
 
 . ./common
 
@@ -32,6 +34,15 @@ setup_keys()
 		mkdir -p "$keypath"
 	fi
 
+	if [ -f "$masterpath" ]; then
+		mv "$masterpath" "$masterpath.bak"
+		$backup_key=1
+	fi
+	if [ -f "$keypath/tpm.handle" ]; then
+		mv "$keypath/tpm.handle" "$keypath/tmp.handle.bak"
+		$backup_handle=1
+	fi
+
 	dd if=/dev/urandom bs=1 count=32 2>/dev/null | keyctl padd user "$masterkey" @u
 	keyctl pipe "$(keyctl search @u user $masterkey)" > "$masterpath"
 }
@@ -43,16 +54,25 @@ test_cleanup()
 	fi
 
 	if keyctl search @u user "$masterkey"; then
-		keyctl unlink "$(keyctl search @u user $masterkey)"
+		keyctl unlink "$(keyctl search @u user "$masterkey")"
 	fi
 
 	if [ -f "$keypath"/nvdimm_"$id"_"$(hostname)".blob ]; then
 		rm -f "$keypath"/nvdimm_"$id"_"$(hostname)".blob
 	fi
+}
 
+post_cleanup()
+{
 	if [ -f $masterpath ]; then
 		rm -f "$masterpath"
 	fi
+	if [ "$backup_key" -eq 1 ]; then
+		mv "$masterpath.bak" "$masterpath"
+	fi
+	if [ "$backup_handle" -eq 1 ]; then
+		mv "$keypath/tpm.handle.bak" "$keypath/tmp.handle"
+	fi
 }
 
 lock_dimm()
@@ -168,8 +188,8 @@ test_4_security_unlock()
 	remove_passphrase
 }
 
-# this should always be the last test. with security frozen, nfit_test must
-# be removed and is no longer usable
+# This should always be the last nvdimm security test.
+# with security frozen, nfit_test must be removed and is no longer usable
 test_5_security_freeze()
 {
 	setup_passphrase
@@ -188,6 +208,33 @@ test_5_security_freeze()
 	fi
 }
 
+test_6_load_keys()
+{
+	if keyctl search @u encrypted nvdimm:"$id"; then
+		keyctl unlink "$(keyctl search @u encrypted nvdimm:"$id")"
+	fi
+
+	if keyctl search @u user "$masterkey"; then
+		keyctl unlink "$(keyctl search @u user "$masterkey")"
+	fi
+
+	$NDCTL load-keys
+
+	if keyctl search @u user "$masterkey"; then
+		echo "master key loaded"
+	else
+		echo "master key failed to loaded"
+		err "$LINENO"
+	fi
+
+	if keyctl search @u encrypted nvdimm:"$id"; then
+		echo "dimm key loaded"
+	else
+		echo "dimm key failed to load"
+		err "$LINENO"
+	fi
+}
+
 check_min_kver "5.0" || do_skip "may lack security handling"
 uid="$(keyctl show | grep -Eo "_uid.[0-9]+" | head -1 | cut -d. -f2-)"
 if [ "$uid" -ne 0 ]; then
@@ -210,11 +257,18 @@ test_3_security_setup_and_erase
 echo "Test 4, unlock dimm"
 test_4_security_unlock
 
-# Freeze should always be run last because it locks security state and require
-# nfit_test module unload.
+# Freeze should always be the last nvdimm security test because it locks
+# security state and require nfit_test module unload. However, this does
+# not impact any key management testing via libkeyctl.
 echo "Test 5, freeze security"
 test_5_security_freeze
 
+# Load-keys is independent of actual nvdimm security and is part of key
+# mangement testing.
+echo "Test 6, test load-keys"
+test_6_load_keys
+
 test_cleanup
+post_cleanup
 _cleanup
 exit 0

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