nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 00/12] ndctl: add security support
@ 2018-12-14 21:09 Dave Jiang
  2018-12-14 21:09 ` [PATCH v6 01/12] ndctl: add support for display security state Dave Jiang
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

The following series implements mechanisms that utilize the sysfs knobs
provided by the kernel in order to support the Intel DSM v1.8 spec
that provides security to NVDIMM. The following abilities are added:
1. display security state
2. enable/update passphrase
3. disable passphrase
4. freeze security
5. secure erase
6. overwrite
7. master passphrase enable/update

v6:
- Fix spelling and grammar errors for documentation. (Jing)
- Change bool for indicate master passphrase and old passphrase to enum.
- Fix key load script master key name.
- Update to match v15 of kernel patch series.

v5:
- Updated to match latest kernel interface (encrypted keys)
- Added overwrite support
- Added support for DSM v1.8 master passphrase operations
- Removed upcall related code
- Moved security state to enum (Dan)
- Change security output "security_state" to just "security". (Dan)
- Break out enable and update passphrase operation. (Dan)
- Security build can be compiled out when keyutils does not exist. (Dan)
- Move all keyutils related operations to libndctl. (Dan)

v4:
- Updated to match latest kernel interface.
- Added unit test for all security calls

v3:
- Added support to inject keys in order to update nvdimm security.

v2:
- Fixup the upcall util to match recent kernel updates for nvdimm security.

---

Dave Jiang (12):
      ndctl: add support for display security state
      ndctl: add passphrase update to ndctl
      ndctl: add disable security support
      ndctl: add support for freeze security
      ndctl: add support for sanitize dimm
      ndctl: add unit test for security ops (minus overwrite)
      ndctl: setup modprobe rules
      ndctl: add overwrite operation support
      ndctl: add overwrite-wait support
      ndctl: master phassphrase management support
      ndctl: add master secure erase support
      ndctl: documentation for security and key management


 Documentation/ndctl/Makefile.am                  |    8 
 Documentation/ndctl/intel-nvdimm-security.txt    |  139 ++++++
 Documentation/ndctl/ndctl-disable-passphrase.txt |   29 +
 Documentation/ndctl/ndctl-enable-passphrase.txt  |   44 ++
 Documentation/ndctl/ndctl-freeze-security.txt    |   22 +
 Documentation/ndctl/ndctl-list.txt               |    8 
 Documentation/ndctl/ndctl-sanitize-dimm.txt      |   44 ++
 Documentation/ndctl/ndctl-update-passphrase.txt  |   40 ++
 Documentation/ndctl/ndctl-wait-overwrite.txt     |   31 +
 Makefile.am                                      |   10 
 builtin.h                                        |    6 
 configure.ac                                     |   14 +
 contrib/ndctl-loadkeys.sh                        |   24 +
 contrib/nvdimm_modprobe.conf                     |    1 
 ndctl.spec.in                                    |    2 
 ndctl/Makefile.am                                |    3 
 ndctl/dimm.c                                     |  242 ++++++++++-
 ndctl/lib/Makefile.am                            |    8 
 ndctl/lib/dimm.c                                 |  203 +++++++++
 ndctl/lib/keys.c                                 |  501 ++++++++++++++++++++++
 ndctl/lib/libndctl.sym                           |   19 +
 ndctl/libndctl.h                                 |   76 +++
 ndctl/ndctl.c                                    |    6 
 test/Makefile.am                                 |    4 
 test/security.sh                                 |  191 ++++++++
 util/json.c                                      |   31 +
 26 files changed, 1693 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/ndctl/intel-nvdimm-security.txt
 create mode 100644 Documentation/ndctl/ndctl-disable-passphrase.txt
 create mode 100644 Documentation/ndctl/ndctl-enable-passphrase.txt
 create mode 100644 Documentation/ndctl/ndctl-freeze-security.txt
 create mode 100644 Documentation/ndctl/ndctl-sanitize-dimm.txt
 create mode 100644 Documentation/ndctl/ndctl-update-passphrase.txt
 create mode 100644 Documentation/ndctl/ndctl-wait-overwrite.txt
 create mode 100755 contrib/ndctl-loadkeys.sh
 create mode 100644 contrib/nvdimm_modprobe.conf
 create mode 100644 ndctl/lib/keys.c
 create mode 100755 test/security.sh

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

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

* [PATCH v6 01/12] ndctl: add support for display security state
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2018-12-14 21:09 ` [PATCH v6 02/12] ndctl: add passphrase update to ndctl Dave Jiang
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Adding libndctl API call for retrieving security state for a DIMM and also
adding support to ndctl list for displaying security state.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/ndctl-list.txt |    8 ++++++++
 ndctl/lib/dimm.c                   |   37 ++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym             |    5 +++++
 ndctl/libndctl.h                   |   13 +++++++++++++
 util/json.c                        |   31 ++++++++++++++++++++++++++++++
 5 files changed, 94 insertions(+)

diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index e24c8f40..bdd69add 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -98,6 +98,14 @@ include::xable-region-options.txt[]
 -D::
 --dimms::
 	Include dimm info in the listing
+[verse]
+{
+  "dev":"nmem0",
+  "id":"cdab-0a-07e0-ffffffff",
+  "handle":0,
+  "phys_id":0,
+  "security:":"disabled"
+}
 
 -H::
 --health::
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 5e41734d..cd2895c9 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -583,3 +583,40 @@ NDCTL_EXPORT unsigned long ndctl_dimm_get_available_labels(
 
 	return strtoul(buf, NULL, 0);
 }
+
+NDCTL_EXPORT int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
+		enum nd_security_state *state)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	char *path = dimm->dimm_buf;
+	int len = dimm->buf_len;
+	char buf[64];
+	int rc;
+
+	if (snprintf(path, len, "%s/security", dimm->dimm_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				ndctl_dimm_get_devname(dimm));
+		return -ERANGE;
+	}
+
+	rc = sysfs_read_attr(ctx, path, buf);
+	if (rc < 0)
+		return rc;
+
+	if (strcmp(buf, "unsupported") == 0)
+		*state = ND_SECURITY_UNSUPPORTED;
+	else if (strcmp(buf, "disabled") == 0)
+		*state = ND_SECURITY_DISABLED;
+	else if (strcmp(buf, "unlocked") == 0)
+		*state = ND_SECURITY_UNLOCKED;
+	else if (strcmp(buf, "locked") == 0)
+		*state = ND_SECURITY_LOCKED;
+	else if (strcmp(buf, "frozen") == 0)
+		*state = ND_SECURITY_FROZEN;
+	else if (strcmp(buf, "overwrite") == 0)
+		*state = ND_SECURITY_OVERWRITE;
+	else
+		*state = ND_SECURITY_INVALID;
+
+	return 0;
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 6c4c8b4d..1bd63fa1 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -385,3 +385,8 @@ global:
 	ndctl_namespace_get_next_badblock;
 	ndctl_dimm_get_dirty_shutdown;
 } LIBNDCTL_17;
+
+LIBNDCTL_19 {
+global:
+	ndctl_dimm_get_security;
+} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 62cef9e8..a9f9167a 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -681,6 +681,19 @@ enum ND_FW_STATUS ndctl_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd);
 struct ndctl_cmd *ndctl_dimm_cmd_new_ack_shutdown_count(struct ndctl_dimm *dimm);
 int ndctl_dimm_fw_update_supported(struct ndctl_dimm *dimm);
 
+enum nd_security_state {
+	ND_SECURITY_INVALID = -1,
+	ND_SECURITY_UNSUPPORTED = 0,
+	ND_SECURITY_DISABLED,
+	ND_SECURITY_UNLOCKED,
+	ND_SECURITY_LOCKED,
+	ND_SECURITY_FROZEN,
+	ND_SECURITY_OVERWRITE,
+};
+
+int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
+		enum nd_security_state *sstate);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif
diff --git a/util/json.c b/util/json.c
index 5c3424e2..e3b9e72e 100644
--- a/util/json.c
+++ b/util/json.c
@@ -164,6 +164,7 @@ struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
 	unsigned int handle = ndctl_dimm_get_handle(dimm);
 	unsigned short phys_id = ndctl_dimm_get_phys_id(dimm);
 	struct json_object *jobj;
+	enum nd_security_state sstate;
 
 	if (!jdimm)
 		return NULL;
@@ -243,6 +244,36 @@ struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
 		json_object_object_add(jdimm, "flag_smart_event", jobj);
 	}
 
+	if (ndctl_dimm_get_security(dimm, &sstate) == 0) {
+		switch (sstate) {
+		case ND_SECURITY_UNSUPPORTED:
+			jobj = json_object_new_string("unsupported");
+			break;
+		case ND_SECURITY_DISABLED:
+			jobj = json_object_new_string("disabled");
+			break;
+		case ND_SECURITY_UNLOCKED:
+			jobj = json_object_new_string("unlocked");
+			break;
+		case ND_SECURITY_LOCKED:
+			jobj = json_object_new_string("locked");
+			break;
+		case ND_SECURITY_FROZEN:
+			jobj = json_object_new_string("frozen");
+			break;
+		case ND_SECURITY_OVERWRITE:
+			jobj = json_object_new_string("overwrite");
+			break;
+		case ND_SECURITY_INVALID:
+		default:
+			jobj = json_object_new_string("invalid");
+			break;
+		}
+		if (!jobj)
+			goto err;
+		json_object_object_add(jdimm, "security", jobj);
+	}
+
 	return jdimm;
  err:
 	json_object_put(jdimm);

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

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

* [PATCH v6 02/12] ndctl: add passphrase update to ndctl
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
  2018-12-14 21:09 ` [PATCH v6 01/12] ndctl: add support for display security state Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2019-01-05  0:07   ` Verma, Vishal L
  2018-12-14 21:09 ` [PATCH v6 03/12] ndctl: add disable security support Dave Jiang
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Add API call for triggering sysfs knob to update the security for a DIMM
in libndctl. Also add the ndctl "update-passphrase" to trigger the
operation.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/Makefile.am                 |    4 
 Documentation/ndctl/ndctl-enable-passphrase.txt |   35 ++
 Documentation/ndctl/ndctl-update-passphrase.txt |   33 ++
 builtin.h                                       |    2 
 configure.ac                                    |   14 +
 ndctl.spec.in                                   |    2 
 ndctl/Makefile.am                               |    3 
 ndctl/dimm.c                                    |   86 ++++-
 ndctl/lib/Makefile.am                           |    8 
 ndctl/lib/dimm.c                                |   39 ++
 ndctl/lib/keys.c                                |  389 +++++++++++++++++++++++
 ndctl/lib/libndctl.sym                          |    4 
 ndctl/libndctl.h                                |   31 ++
 ndctl/ndctl.c                                   |    2 
 14 files changed, 639 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/ndctl/ndctl-enable-passphrase.txt
 create mode 100644 Documentation/ndctl/ndctl-update-passphrase.txt
 create mode 100644 ndctl/lib/keys.c

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index a30b139b..7ad6666b 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -47,7 +47,9 @@ man1_MANS = \
 	ndctl-inject-smart.1 \
 	ndctl-update-firmware.1 \
 	ndctl-list.1 \
-	ndctl-monitor.1
+	ndctl-monitor.1 \
+	ndctl-enable-passphrase.1 \
+	ndctl-update-passphrase.1
 
 CLEANFILES = $(man1_MANS)
 
diff --git a/Documentation/ndctl/ndctl-enable-passphrase.txt b/Documentation/ndctl/ndctl-enable-passphrase.txt
new file mode 100644
index 00000000..8de5410c
--- /dev/null
+++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-enable-passphrase(1)
+==========================
+
+NAME
+----
+ndctl-enable-passphrase - enabling the security passphrase for a NVDIMM
+
+SYNOPSIS
+--------
+[verse]
+'ndctl enable-passphrase' <dimm> [<options>]
+
+DESCRIPTION
+-----------
+Provide a generic interface for enabling security for NVDIMM.
+It is expected that the master key has already been loaded into the user
+key ring. The encrypted key blobs will be created in /etc/nvdimm directory
+with the file name of "nvdimm-<hostname>-<dimm unique id>.blob".
+
+The command will fail if the nvdimm key is already in the user key ring and/or
+the key blob already resides in /etc/nvdimm. Do not touch the /etc/nvdimm
+directory and let ndctl manage the keys, unless you know what you are doing.
+
+OPTIONS
+-------
+<dimm>::
+include::xable-dimm-options.txt[]
+
+-m::
+--master=::
+	Key name for the master key used to seal the NVDIMM security keys.
+
+include::../copyright.txt[]
diff --git a/Documentation/ndctl/ndctl-update-passphrase.txt b/Documentation/ndctl/ndctl-update-passphrase.txt
new file mode 100644
index 00000000..9ed39cca
--- /dev/null
+++ b/Documentation/ndctl/ndctl-update-passphrase.txt
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-update-passphrase(1)
+==========================
+
+NAME
+----
+ndctl-update-passphrase - update the security passphrase for a NVDIMM
+
+SYNOPSIS
+--------
+[verse]
+'ndctl update-passphrase' <dimm> [<options>]
+
+DESCRIPTION
+-----------
+Provide a generic interface for updating security key for NVDIMM.
+It is expected that the current and the new (if new master key is desired)
+master key has already been loaded into the user key ring. The new encrypted
+key blobs will be created in /etc/nvdimm directory
+with the file name of "nvdimm-<hostname>-<dimm unique id>.blob".
+
+OPTIONS
+-------
+<dimm>::
+include::xable-dimm-options.txt[]
+
+-m::
+--master::
+	New key name for the master key to seal the new nvdimm key, or the
+	existing master key name. i.e trusted:master-key.
+
+include::../copyright.txt[]
diff --git a/builtin.h b/builtin.h
index 675a6ce7..ed018d96 100644
--- a/builtin.h
+++ b/builtin.h
@@ -48,4 +48,6 @@ int cmd_bat(int argc, const char **argv, void *ctx);
 #endif
 int cmd_update_firmware(int argc, const char **argv, void *ctx);
 int cmd_inject_smart(int argc, const char **argv, void *ctx);
+int cmd_passphrase_setup(int argc, const char **argv, void *ctx);
+int cmd_passphrase_update(int argc, const char **argv, void *ctx);
 #endif /* _NDCTL_BUILTIN_H_ */
diff --git a/configure.ac b/configure.ac
index bb6b0332..ac4f56c3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -153,6 +153,20 @@ fi
 AC_SUBST([systemd_unitdir])
 AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
 
+AC_ARG_WITH([keyutils],
+	    AS_HELP_STRING([--with-keyutils],
+			[Enable keyutils functionality (security).  @<:@default=yes@:>@]), [], [with_keyutils=yes])
+
+if test "x$with_keyutils" = "xyes"; then
+	AC_CHECK_HEADERS([keyutils.h],,[
+		AC_MSG_ERROR([keyutils.h not found, consider installing
+			      keyutils-libs-devel.])
+		])
+fi
+AS_IF([test "x$with_keyutils" = "xyes"],
+	[AC_DEFINE([ENABLE_KEYUTILS], [1], [Enable keyutils support])])
+AM_CONDITIONAL([ENABLE_KEYUTILS], [test "x$with_keyutils" = "xyes"])
+
 my_CFLAGS="\
 -D DEF_CONF_FILE='\"${sysconfdir}/ndctl/monitor.conf\"' \
 -Wall \
diff --git a/ndctl.spec.in b/ndctl.spec.in
index 26396d4a..66466db6 100644
--- a/ndctl.spec.in
+++ b/ndctl.spec.in
@@ -21,6 +21,7 @@ BuildRequires:	pkgconfig(uuid)
 BuildRequires:	pkgconfig(json-c)
 BuildRequires:	pkgconfig(bash-completion)
 BuildRequires:	pkgconfig(systemd)
+BuildRequires:	keyutils-libs-devel
 
 %description
 Utility library for managing the "libnvdimm" subsystem.  The "libnvdimm"
@@ -119,6 +120,7 @@ make check
 %{bashcompdir}/
 %{_sysconfdir}/ndctl/monitor.conf
 %{_unitdir}/ndctl-monitor.service
+%{_sysconfdir}/ndctl/keys/
 
 %files -n daxctl
 %defattr(-,root,root)
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 8a5e5f87..5b62251b 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -30,7 +30,8 @@ ndctl_LDADD =\
 	../libutil.a \
 	$(UUID_LIBS) \
 	$(KMOD_LIBS) \
-	$(JSON_LIBS)
+	$(JSON_LIBS) \
+	-lkeyutils
 
 if ENABLE_TEST
 ndctl_SOURCES += ../test/libndctl.c \
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 699ab57d..581be497 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -40,6 +40,19 @@ struct action_context {
 	struct update_context update;
 };
 
+static struct parameters {
+	const char *bus;
+	const char *outfile;
+	const char *infile;
+	const char *labelversion;
+	const char *master_key;
+	bool force;
+	bool json;
+	bool verbose;
+} param = {
+	.labelversion = "1.1",
+};
+
 static int action_disable(struct ndctl_dimm *dimm, struct action_context *actx)
 {
 	if (ndctl_dimm_is_active(dimm)) {
@@ -824,17 +837,29 @@ static int action_update(struct ndctl_dimm *dimm, struct action_context *actx)
 	return rc;
 }
 
-static struct parameters {
-	const char *bus;
-	const char *outfile;
-	const char *infile;
-	const char *labelversion;
-	bool force;
-	bool json;
-	bool verbose;
-} param = {
-	.labelversion = "1.1",
-};
+static int action_key_enable(struct ndctl_dimm *dimm,
+		struct action_context *actx)
+{
+	if (!ndctl_dimm_security_supported(dimm)) {
+		error("%s: security operation not supported\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EOPNOTSUPP;
+	}
+
+	return ndctl_dimm_enable_key(dimm, param.master_key);
+}
+
+static int action_key_update(struct ndctl_dimm *dimm,
+		struct action_context *actx)
+{
+	if (!ndctl_dimm_security_supported(dimm)) {
+		error("%s: security operation not supported\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EOPNOTSUPP;
+	}
+
+	return ndctl_dimm_update_key(dimm, param.master_key);
+}
 
 static int __action_init(struct ndctl_dimm *dimm,
 		enum ndctl_namespace_version version, int chk_only)
@@ -925,6 +950,10 @@ OPT_BOOLEAN('f', "force", &param.force, \
 OPT_STRING('V', "label-version", &param.labelversion, "version-number", \
 	"namespace label specification version (default: 1.1)")
 
+#define KEY_OPTIONS() \
+OPT_STRING('m', "master-key", &param.master_key, "<key_type>:<key_name>", \
+		"master key for security")
+
 static const struct option read_options[] = {
 	BASE_OPTIONS(),
 	READ_OPTIONS(),
@@ -954,6 +983,12 @@ static const struct option init_options[] = {
 	OPT_END(),
 };
 
+static const struct option key_options[] = {
+	BASE_OPTIONS(),
+	KEY_OPTIONS(),
+	OPT_END(),
+};
+
 static int dimm_action(int argc, const char **argv, void *ctx,
 		int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
 		const struct option *options, const char *usage)
@@ -1024,6 +1059,13 @@ static int dimm_action(int argc, const char **argv, void *ctx,
 		}
 	}
 
+	if (!param.master_key &&
+			(action == action_key_enable ||
+			 action == action_key_update)) {
+		usage_with_options(u, options);
+		return -EINVAL;
+	}
+
 	if (param.verbose)
 		ndctl_set_log_priority(ctx, LOG_DEBUG);
 
@@ -1181,3 +1223,25 @@ int cmd_update_firmware(int argc, const char **argv, void *ctx)
 			count > 1 ? "s" : "");
 	return count >= 0 ? 0 : EXIT_FAILURE;
 }
+
+int cmd_passphrase_update(int argc, const char **argv, void *ctx)
+{
+	int count = dimm_action(argc, argv, ctx, action_key_update,
+			key_options,
+			"ndctl update-passphrase <nmem0> [<nmem1>..<nmemN>] [<options>]");
+
+	fprintf(stderr, "passphrase updated for %d nmem%s.\n", count >= 0 ? count : 0,
+			count > 1 ? "s" : "");
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
+
+int cmd_passphrase_setup(int argc, const char **argv, void *ctx)
+{
+	int count = dimm_action(argc, argv, ctx, action_key_enable,
+			key_options,
+			"ndctl enable-passphrase <nmem0> [<nmem1>..<nmemN>] [<options>]");
+
+	fprintf(stderr, "passphrase enabled for %d nmem%s.\n", count >= 0 ? count : 0,
+			count > 1 ? "s" : "");
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
index 77970399..6b9bde43 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -24,12 +24,20 @@ libndctl_la_SOURCES =\
 	firmware.c \
 	libndctl.c
 
+if ENABLE_KEYUTILS
+libndctl_la_SOURCES += keys.c
+endif
+
 libndctl_la_LIBADD =\
 	../../daxctl/lib/libdaxctl.la \
 	$(UDEV_LIBS) \
 	$(UUID_LIBS) \
 	$(KMOD_LIBS)
 
+if ENABLE_KEYUTILS
+libndctl_la_LIBADD += -lkeyutils
+endif
+
 EXTRA_DIST += libndctl.sym
 
 libndctl_la_LDFLAGS = $(AM_LDFLAGS) \
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index cd2895c9..2da0d01a 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -620,3 +620,42 @@ NDCTL_EXPORT int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
 
 	return 0;
 }
+
+NDCTL_EXPORT bool ndctl_dimm_security_supported(struct ndctl_dimm *dimm)
+{
+	enum nd_security_state state;
+	int rc;
+
+	rc = ndctl_dimm_get_security(dimm, &state);
+	if (rc < 0)
+		return false;
+
+	if (state == ND_SECURITY_UNSUPPORTED)
+		return false;
+
+	return true;
+}
+
+static int write_security(struct ndctl_dimm *dimm, const char *cmd)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	char *path = dimm->dimm_buf;
+	int len = dimm->buf_len;
+
+	if (snprintf(path, len, "%s/security", dimm->dimm_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				ndctl_dimm_get_devname(dimm));
+		return -ERANGE;
+	}
+
+	return sysfs_write_attr(ctx, path, cmd);
+}
+
+NDCTL_EXPORT int ndctl_dimm_update_passphrase(struct ndctl_dimm *dimm,
+		long ckey, long nkey)
+{
+	char buf[SYSFS_ATTR_SIZE];
+
+	sprintf(buf, "update %ld %ld\n", ckey, nkey);
+	return write_security(dimm, buf);
+}
diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
new file mode 100644
index 00000000..a005c6a6
--- /dev/null
+++ b/ndctl/lib/keys.c
@@ -0,0 +1,389 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+
+#include <stdio.h>
+#include <errno.h>
+#include <string.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <sys/param.h>
+#include <keyutils.h>
+#include <syslog.h>
+
+#include <ndctl.h>
+#include <ndctl/libndctl.h>
+#include "private.h"
+
+#define PATH_SIZE	512
+#define HOSTNAME_SIZE	64
+#define DESC_SIZE	128
+#define KEY_CMD_SIZE	128
+
+static int get_key_path(struct ndctl_dimm *dimm, char *path,
+		enum ndctl_key_type key_type)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	char hostname[HOSTNAME_SIZE];
+	int rc;
+
+	rc = gethostname(hostname, HOSTNAME_SIZE-1);
+	if (rc < 0) {
+		err(ctx, "gethostname: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	if (key_type == ND_USER_OLD_KEY) {
+		rc = sprintf(path, "%s/nvdimmold_%s_%s.blob",
+				ND_PASS_PATH,
+				ndctl_dimm_get_unique_id(dimm),
+				hostname);
+	} else {
+		rc = sprintf(path, "%s/nvdimm_%s_%s.blob",
+				ND_PASS_PATH,
+				ndctl_dimm_get_unique_id(dimm),
+				hostname);
+	}
+
+	if (rc < 0) {
+		err(ctx, "sprintf: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	return 0;
+}
+
+static int get_key_desc(struct ndctl_dimm *dimm, char *desc,
+		enum ndctl_key_type key_type)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	int rc;
+
+	if (key_type == ND_USER_OLD_KEY)
+		rc = sprintf(desc, "nvdimm-old:%s",
+				ndctl_dimm_get_unique_id(dimm));
+	else
+		rc = sprintf(desc, "nvdimm:%s",
+				ndctl_dimm_get_unique_id(dimm));
+
+	if (rc < 0) {
+		err(ctx, "sprintf: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	return 0;
+}
+
+static char *load_key_blob(struct ndctl_ctx *ctx, const char *path, int *size)
+{
+	struct stat st;
+	FILE *bfile = NULL;
+	ssize_t read;
+	int rc;
+	char *blob, *pl;
+	char prefix[] = "load ";
+
+	rc = stat(path, &st);
+	if (rc < 0) {
+		err(ctx, "stat: %s\n", strerror(errno));
+		return NULL;
+	}
+	if ((st.st_mode & S_IFMT) != S_IFREG) {
+		err(ctx, "%s not a regular file\n", path);
+		return NULL;
+	}
+
+	if (st.st_size == 0 || st.st_size > 4096) {
+		err(ctx, "Invalid blob file size\n");
+		return NULL;
+	}
+
+	*size = st.st_size + sizeof(prefix) - 1;
+	blob = malloc(*size);
+	if (!blob) {
+		err(ctx, "Unable to allocate memory for blob\n");
+		return NULL;
+	}
+
+	bfile = fopen(path, "r");
+	if (!bfile) {
+		err(ctx, "Unable to open %s: %s\n", path, strerror(errno));
+		free(blob);
+		return NULL;
+	}
+
+	memcpy(blob, prefix, sizeof(prefix) - 1);
+	pl = blob + sizeof(prefix) - 1;
+	read = fread(pl, st.st_size, 1, bfile);
+	if (read < 0) {
+		err(ctx, "Fail to read from blob file: %s\n",
+				strerror(errno));
+		free(blob);
+		fclose(bfile);
+		return NULL;
+	}
+
+	fclose(bfile);
+	return blob;
+}
+
+static key_serial_t dimm_check_key(struct ndctl_dimm *dimm,
+		enum ndctl_key_type key_type)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	char desc[DESC_SIZE];
+	int rc;
+
+	rc = get_key_desc(dimm, desc, key_type);
+	if (rc < 0) {
+		err(ctx, "sprintf: %d\n", rc);
+		return rc;
+	}
+
+	return keyctl_search(KEY_SPEC_USER_KEYRING, "encrypted", desc, 0);
+}
+
+static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
+		const char *master)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	char desc[DESC_SIZE];
+	char path[PATH_SIZE];
+	char cmd[KEY_CMD_SIZE];
+	key_serial_t key;
+	void *buffer;
+	int rc;
+	ssize_t size;
+	FILE *fp;
+	ssize_t wrote;
+	struct stat st;
+
+	if (ndctl_dimm_is_active(dimm)) {
+		err(ctx, "regions active on %s, op failed\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EBUSY;
+	}
+
+	rc = get_key_desc(dimm, desc, ND_USER_KEY);
+	if (rc < 0)
+		return rc;
+
+	/* make sure it's not already in the key ring */
+	key = keyctl_search(KEY_SPEC_USER_KEYRING, "encrypted", desc, 0);
+	if (key > 0) {
+		err(ctx, "key already in user keyring, can't create new.\n");
+		return -EAGAIN;
+	}
+
+	rc = get_key_path(dimm, path, ND_USER_KEY);
+	if (rc < 0)
+		return rc;
+
+	rc = stat(path, &st);
+	if (rc == 0) {
+		err(ctx, "%s already exists!\n", path);
+		return -EAGAIN;
+	}
+
+	rc = sprintf(cmd, "new enc32 %s 32", master);
+	if (rc < 0) {
+		err(ctx, "sprintf: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	key = add_key("encrypted", desc, cmd, strlen(cmd),
+			KEY_SPEC_USER_KEYRING);
+	if (key < 0) {
+		err(ctx, "add_key failed: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	size = keyctl_read_alloc(key, &buffer);
+	if (size < 0) {
+		err(ctx, "keyctl_read_alloc failed: %ld\n", size);
+		keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
+		return rc;
+	}
+
+	fp = fopen(path, "w");
+	if (!fp) {
+		rc = -errno;
+		err(ctx, "fopen: %s\n", strerror(errno));
+		free(buffer);
+		return rc;
+	}
+
+	 wrote = fwrite(buffer, size, 1, fp);
+	 if (wrote < 0) {
+		 rc = -errno;
+		 err(ctx, "fwrite failed: %s\n", strerror(errno));
+		 free(buffer);
+		 return rc;
+	 }
+
+	 fclose(fp);
+	 free(buffer);
+	 return key;
+}
+
+static key_serial_t dimm_load_key(struct ndctl_dimm *dimm,
+		enum ndctl_key_type key_type)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	key_serial_t key;
+	char desc[DESC_SIZE];
+	char path[PATH_SIZE];
+	int rc;
+	char *blob;
+	int size;
+
+	if (ndctl_dimm_is_active(dimm)) {
+		err(ctx, "regions active on %s, op failed\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EBUSY;
+	}
+
+	rc = get_key_desc(dimm, desc, key_type);
+	if (rc < 0)
+		return rc;
+
+	rc = get_key_path(dimm, path, key_type);
+	if (rc < 0)
+		return rc;
+
+	blob = load_key_blob(ctx, path, &size);
+	if (!blob)
+		return -ENOMEM;
+
+	key = add_key("encrypted", desc, blob, size, KEY_SPEC_USER_KEYRING);
+	free(blob);
+	if (key < 0) {
+		err(ctx, "add_key failed: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	return key;
+}
+
+/*
+ * The function will check to see if the existing key is there and remove
+ * from user key ring if it is. Rename the existing key blob to old key
+ * blob, and then attempt to inject the key as old key into the user key
+ * ring.
+ */
+static key_serial_t move_key_to_old(struct ndctl_dimm *dimm)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	int rc;
+	key_serial_t key;
+	char old_path[PATH_SIZE];
+	char new_path[PATH_SIZE];
+
+	if (ndctl_dimm_is_active(dimm)) {
+		err(ctx, "regions active on %s, op failed\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EBUSY;
+	}
+
+	key = dimm_check_key(dimm, ND_USER_KEY);
+	if (key > 0)
+		keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
+
+	rc = get_key_path(dimm, old_path, ND_USER_KEY);
+	if (rc < 0)
+		return rc;
+
+	rc = get_key_path(dimm, new_path, ND_USER_OLD_KEY);
+	if (rc < 0)
+		return rc;
+
+	rc = rename(old_path, new_path);
+	if (rc < 0)
+		return -errno;
+
+	return dimm_load_key(dimm, ND_USER_OLD_KEY);
+}
+
+static int dimm_remove_key(struct ndctl_dimm *dimm,
+		enum ndctl_key_type key_type)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	key_serial_t key;
+	char path[PATH_SIZE];
+	int rc;
+
+	key = dimm_check_key(dimm, key_type);
+	if (key > 0)
+		keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
+
+	rc = get_key_path(dimm, path, key_type);
+	if (rc < 0) {
+		err(ctx, "get key file path failed: %d\n", rc);
+		return rc;
+	}
+
+	rc = unlink(path);
+	if (rc < 0) {
+		err(ctx, "unlink: %s\n", strerror(errno));
+		return -errno;
+	}
+
+	return 0;
+}
+
+NDCTL_EXPORT int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
+		const char *master)
+{
+	key_serial_t key;
+	int rc;
+
+	key = dimm_create_key(dimm, master);
+	if (key < 0)
+		return (int)key;
+
+	rc = ndctl_dimm_update_passphrase(dimm, 0, key);
+	if (rc < 0) {
+		dimm_remove_key(dimm, ND_USER_KEY);
+		return rc;
+	}
+
+	return 0;
+}
+
+NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
+		const char *master)
+{
+	int rc;
+	key_serial_t old_key, new_key;
+
+	/*
+	 * 1. check if current key is loaded and remove
+	 * 2. move current key blob to old key blob
+	 * 3. load old key blob
+	 * 4. trigger change key with old and key key
+	 * 5. remove old key
+	 * 6. remove old key blob
+	 */
+	old_key = move_key_to_old(dimm);
+	if (old_key < 0)
+		return old_key;
+
+	new_key = dimm_create_key(dimm, master);
+	/* need to create new key here */
+	if (new_key < 0) {
+		new_key = dimm_load_key(dimm, ND_USER_KEY);
+		if (new_key < 0)
+			return new_key;
+	}
+
+	rc = ndctl_dimm_update_passphrase(dimm, old_key, new_key);
+	if (rc < 0)
+		return rc;
+
+	rc = dimm_remove_key(dimm, ND_USER_OLD_KEY);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 1bd63fa1..a790b1ea 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -389,4 +389,8 @@ global:
 LIBNDCTL_19 {
 global:
 	ndctl_dimm_get_security;
+	ndctl_dimm_security_supported;
+	ndctl_dimm_enable_key;
+	ndctl_dimm_update_key;
+	ndctl_dimm_update_passphrase;
 } LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index a9f9167a..00ec1907 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -19,6 +19,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <limits.h>
+#include <keyutils.h>
 
 #ifdef HAVE_LIBUUID
 #include <uuid/uuid.h>
@@ -681,6 +682,11 @@ enum ND_FW_STATUS ndctl_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd);
 struct ndctl_cmd *ndctl_dimm_cmd_new_ack_shutdown_count(struct ndctl_dimm *dimm);
 int ndctl_dimm_fw_update_supported(struct ndctl_dimm *dimm);
 
+#define ND_PASSPHRASE_SIZE	32
+#define ND_PASS_PATH		"/etc/ndctl/keys"
+#define ND_KEY_DESC_LEN	22
+#define ND_KEY_DESC_PREFIX  7
+
 enum nd_security_state {
 	ND_SECURITY_INVALID = -1,
 	ND_SECURITY_UNSUPPORTED = 0,
@@ -693,6 +699,31 @@ enum nd_security_state {
 
 int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
 		enum nd_security_state *sstate);
+bool ndctl_dimm_security_supported(struct ndctl_dimm *dimm);
+int ndctl_dimm_update_passphrase(struct ndctl_dimm *dimm,
+		long ckey, long nkey);
+
+enum ndctl_key_type {
+	ND_USER_KEY,
+	ND_USER_OLD_KEY,
+};
+
+#ifdef ENABLE_KEYUTILS
+int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master);
+int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master);
+#else
+static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
+		const char *master)
+{
+	return -EOPNOTSUPP;
+}
+
+static inline int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
+		const char *master)
+{
+	return -EOPNOTSUPP;
+}
+#endif
 
 #ifdef __cplusplus
 } /* extern "C" */
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 73dabfac..4336c94c 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -88,6 +88,8 @@ static struct cmd_struct commands[] = {
 	{ "inject-smart", cmd_inject_smart },
 	{ "wait-scrub", cmd_wait_scrub },
 	{ "start-scrub", cmd_start_scrub },
+	{ "enable-passphrase", cmd_passphrase_setup },
+	{ "update-passphrase", cmd_passphrase_update },
 	{ "list", cmd_list },
 	{ "monitor", cmd_monitor},
 	{ "help", cmd_help },

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

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

* [PATCH v6 03/12] ndctl: add disable security support
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
  2018-12-14 21:09 ` [PATCH v6 01/12] ndctl: add support for display security state Dave Jiang
  2018-12-14 21:09 ` [PATCH v6 02/12] ndctl: add passphrase update to ndctl Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2018-12-14 21:09 ` [PATCH v6 04/12] ndctl: add support for freeze security Dave Jiang
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Add support for disable security to libndctl and also command line option
of "disable-passphrase" for ndctl. This provides a way to disable security
on the nvdimm.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/Makefile.am                  |    3 ++
 Documentation/ndctl/ndctl-disable-passphrase.txt |   27 +++++++++++++++++++++
 builtin.h                                        |    1 +
 ndctl/dimm.c                                     |   22 +++++++++++++++++
 ndctl/lib/dimm.c                                 |    9 +++++++
 ndctl/lib/keys.c                                 |   28 ++++++++++++++++++++++
 ndctl/lib/libndctl.sym                           |    2 ++
 ndctl/libndctl.h                                 |    7 ++++++
 ndctl/ndctl.c                                    |    1 +
 9 files changed, 99 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ndctl/ndctl-disable-passphrase.txt

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index 7ad6666b..31570a77 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -49,7 +49,8 @@ man1_MANS = \
 	ndctl-list.1 \
 	ndctl-monitor.1 \
 	ndctl-enable-passphrase.1 \
-	ndctl-update-passphrase.1
+	ndctl-update-passphrase.1 \
+	ndctl-disable-passphrase.1
 
 CLEANFILES = $(man1_MANS)
 
diff --git a/Documentation/ndctl/ndctl-disable-passphrase.txt b/Documentation/ndctl/ndctl-disable-passphrase.txt
new file mode 100644
index 00000000..e921eb23
--- /dev/null
+++ b/Documentation/ndctl/ndctl-disable-passphrase.txt
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-disable-passphrase(1)
+===========================
+
+NAME
+----
+ndctl-disable-passphrase - disabling passphrase for an NVDIMM
+
+SYNOPSIS
+--------
+[verse]
+'ndctl disable-passphrase' <dimm> [<options>]
+
+DESCRIPTION
+-----------
+Provide a generic interface for disabling passphrase for NVDIMM.
+Ndctl will search the user key ring for the associated DIMM. If no key is
+found, it will attempt to load the key blob from the expected location. Ndctl
+will remove the key and the key blob once passphrase is disabled.
+
+OPTIONS
+-------
+<dimm>::
+include::xable-dimm-options.txt[]
+
+include::../copyright.txt[]
diff --git a/builtin.h b/builtin.h
index ed018d96..8d8f61e2 100644
--- a/builtin.h
+++ b/builtin.h
@@ -50,4 +50,5 @@ int cmd_update_firmware(int argc, const char **argv, void *ctx);
 int cmd_inject_smart(int argc, const char **argv, void *ctx);
 int cmd_passphrase_setup(int argc, const char **argv, void *ctx);
 int cmd_passphrase_update(int argc, const char **argv, void *ctx);
+int cmd_disable_passphrase(int argc, const char **argv, void *ctx);
 #endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 581be497..edb4b9d7 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -861,6 +861,18 @@ static int action_key_update(struct ndctl_dimm *dimm,
 	return ndctl_dimm_update_key(dimm, param.master_key);
 }
 
+static int action_passphrase_disable(struct ndctl_dimm *dimm,
+		struct action_context *actx)
+{
+	if (!ndctl_dimm_security_supported(dimm)) {
+		error("%s: security operation not supported\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EOPNOTSUPP;
+	}
+
+	return ndctl_dimm_disable_key(dimm);
+}
+
 static int __action_init(struct ndctl_dimm *dimm,
 		enum ndctl_namespace_version version, int chk_only)
 {
@@ -1245,3 +1257,13 @@ int cmd_passphrase_setup(int argc, const char **argv, void *ctx)
 			count > 1 ? "s" : "");
 	return count >= 0 ? 0 : EXIT_FAILURE;
 }
+
+int cmd_disable_passphrase(int argc, const char **argv, void *ctx)
+{
+	int count = dimm_action(argc, argv, ctx, action_passphrase_disable, base_options,
+			"ndctl disable-passphrase <nmem0> [<nmem1>..<nmemN>] [<options>]");
+
+	fprintf(stderr, "passphrase disabled %d nmem%s.\n", count >= 0 ? count : 0,
+			count > 1 ? "s" : "");
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 2da0d01a..9cc22016 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -659,3 +659,12 @@ NDCTL_EXPORT int ndctl_dimm_update_passphrase(struct ndctl_dimm *dimm,
 	sprintf(buf, "update %ld %ld\n", ckey, nkey);
 	return write_security(dimm, buf);
 }
+
+NDCTL_EXPORT int ndctl_dimm_disable_passphrase(struct ndctl_dimm *dimm,
+		long key)
+{
+	char buf[SYSFS_ATTR_SIZE];
+
+	sprintf(buf, "disable %ld\n", key);
+	return write_security(dimm, buf);
+}
diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
index a005c6a6..779e82af 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -387,3 +387,31 @@ NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
 
 	return 0;
 }
+
+NDCTL_EXPORT int ndctl_dimm_disable_key(struct ndctl_dimm *dimm)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	key_serial_t key;
+	int rc;
+
+	key = dimm_check_key(dimm, false);
+	if (key < 0) {
+		key = dimm_load_key(dimm, false);
+		if (key < 0) {
+			err(ctx, "Unable to load key\n");
+			return -ENOKEY;
+		}
+	}
+
+	rc = ndctl_dimm_disable_passphrase(dimm, key);
+	if (rc < 0) {
+		err(ctx, "Failed to disable security for %s\n",
+				ndctl_dimm_get_devname(dimm));
+		return rc;
+	}
+
+	rc = dimm_remove_key(dimm, false);
+	if (rc < 0)
+		err(ctx, "Unable to cleanup key.\n");
+	return 0;
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index a790b1ea..90038e75 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -393,4 +393,6 @@ global:
 	ndctl_dimm_enable_key;
 	ndctl_dimm_update_key;
 	ndctl_dimm_update_passphrase;
+	ndctl_dimm_disable_passphrase;
+	ndctl_dimm_disable_key;
 } LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 00ec1907..18cccbb1 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -702,6 +702,7 @@ int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
 bool ndctl_dimm_security_supported(struct ndctl_dimm *dimm);
 int ndctl_dimm_update_passphrase(struct ndctl_dimm *dimm,
 		long ckey, long nkey);
+int ndctl_dimm_disable_passphrase(struct ndctl_dimm *dimm, long key);
 
 enum ndctl_key_type {
 	ND_USER_KEY,
@@ -711,6 +712,7 @@ enum ndctl_key_type {
 #ifdef ENABLE_KEYUTILS
 int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master);
 int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master);
+int ndctl_dimm_disable_key(struct ndctl_dimm *dimm);
 #else
 static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
 		const char *master)
@@ -723,6 +725,11 @@ static inline int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int ndctl_dimm_disable_key(struct ndctl_dimm *dimm)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 #ifdef __cplusplus
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 4336c94c..1e82ded0 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -90,6 +90,7 @@ static struct cmd_struct commands[] = {
 	{ "start-scrub", cmd_start_scrub },
 	{ "enable-passphrase", cmd_passphrase_setup },
 	{ "update-passphrase", cmd_passphrase_update },
+	{ "disable-passphrase", cmd_disable_passphrase },
 	{ "list", cmd_list },
 	{ "monitor", cmd_monitor},
 	{ "help", cmd_help },

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

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

* [PATCH v6 04/12] ndctl: add support for freeze security
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
                   ` (2 preceding siblings ...)
  2018-12-14 21:09 ` [PATCH v6 03/12] ndctl: add disable security support Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2018-12-14 21:09 ` [PATCH v6 05/12] ndctl: add support for sanitize dimm Dave Jiang
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Add support for freeze security to libndctl and also command line option
of "freeze-security" for ndctl. This will lock the ability to make changes
to the NVDIMM security.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/Makefile.am               |    3 ++-
 Documentation/ndctl/ndctl-freeze-security.txt |   20 ++++++++++++++++++
 builtin.h                                     |    1 +
 ndctl/dimm.c                                  |   28 +++++++++++++++++++++++++
 ndctl/lib/dimm.c                              |    5 ++++
 ndctl/lib/libndctl.sym                        |    1 +
 ndctl/libndctl.h                              |    1 +
 ndctl/ndctl.c                                 |    1 +
 8 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ndctl/ndctl-freeze-security.txt

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index 31570a77..a97f193d 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -50,7 +50,8 @@ man1_MANS = \
 	ndctl-monitor.1 \
 	ndctl-enable-passphrase.1 \
 	ndctl-update-passphrase.1 \
-	ndctl-disable-passphrase.1
+	ndctl-disable-passphrase.1 \
+	ndctl-freeze-security.1
 
 CLEANFILES = $(man1_MANS)
 
diff --git a/Documentation/ndctl/ndctl-freeze-security.txt b/Documentation/ndctl/ndctl-freeze-security.txt
new file mode 100644
index 00000000..4e9d2d61
--- /dev/null
+++ b/Documentation/ndctl/ndctl-freeze-security.txt
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-freeze-security(1)
+========================
+
+NAME
+----
+ndctl-freeze-security - enabling or freeze the security for an NVDIMM
+
+SYNOPSIS
+--------
+[verse]
+'ndctl freeze-security' <dimm>
+
+DESCRIPTION
+-----------
+Provide a generic interface to freeze the security for NVDIMM. Once security
+is frozen, no other security operations can succeed until reboot happens.
+
+include::../copyright.txt[]
diff --git a/builtin.h b/builtin.h
index 8d8f61e2..827295da 100644
--- a/builtin.h
+++ b/builtin.h
@@ -51,4 +51,5 @@ int cmd_inject_smart(int argc, const char **argv, void *ctx);
 int cmd_passphrase_setup(int argc, const char **argv, void *ctx);
 int cmd_passphrase_update(int argc, const char **argv, void *ctx);
 int cmd_disable_passphrase(int argc, const char **argv, void *ctx);
+int cmd_freeze_security(int argc, const char **argv, void *ctx);
 #endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index edb4b9d7..6476b6e5 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -873,6 +873,24 @@ static int action_passphrase_disable(struct ndctl_dimm *dimm,
 	return ndctl_dimm_disable_key(dimm);
 }
 
+static int action_security_freeze(struct ndctl_dimm *dimm,
+		struct action_context *actx)
+{
+	int rc;
+
+	if (!ndctl_dimm_security_supported(dimm)) {
+		error("%s: security operation not supported\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EOPNOTSUPP;
+	}
+
+	rc = ndctl_dimm_freeze_security(dimm);
+	if (rc < 0)
+		error("Failed to freeze security for %s\n",
+				ndctl_dimm_get_devname(dimm));
+	return rc;
+}
+
 static int __action_init(struct ndctl_dimm *dimm,
 		enum ndctl_namespace_version version, int chk_only)
 {
@@ -1267,3 +1285,13 @@ int cmd_disable_passphrase(int argc, const char **argv, void *ctx)
 			count > 1 ? "s" : "");
 	return count >= 0 ? 0 : EXIT_FAILURE;
 }
+
+int cmd_freeze_security(int argc, const char **argv, void *ctx)
+{
+	int count = dimm_action(argc, argv, ctx, action_security_freeze, base_options,
+			"ndctl freeze-security <nmem0> [<nmem1>..<nmemN>] [<options>]");
+
+	fprintf(stderr, "security freezed %d nmem%s.\n", count >= 0 ? count : 0,
+			count > 1 ? "s" : "");
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 9cc22016..227c7124 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -668,3 +668,8 @@ NDCTL_EXPORT int ndctl_dimm_disable_passphrase(struct ndctl_dimm *dimm,
 	sprintf(buf, "disable %ld\n", key);
 	return write_security(dimm, buf);
 }
+
+NDCTL_EXPORT int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm)
+{
+	return write_security(dimm, "freeze");
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 90038e75..a1c56060 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -395,4 +395,5 @@ global:
 	ndctl_dimm_update_passphrase;
 	ndctl_dimm_disable_passphrase;
 	ndctl_dimm_disable_key;
+	ndctl_dimm_freeze_security;
 } LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 18cccbb1..7bb7132c 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -703,6 +703,7 @@ bool ndctl_dimm_security_supported(struct ndctl_dimm *dimm);
 int ndctl_dimm_update_passphrase(struct ndctl_dimm *dimm,
 		long ckey, long nkey);
 int ndctl_dimm_disable_passphrase(struct ndctl_dimm *dimm, long key);
+int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm);
 
 enum ndctl_key_type {
 	ND_USER_KEY,
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 1e82ded0..d08fee4d 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -91,6 +91,7 @@ static struct cmd_struct commands[] = {
 	{ "enable-passphrase", cmd_passphrase_setup },
 	{ "update-passphrase", cmd_passphrase_update },
 	{ "disable-passphrase", cmd_disable_passphrase },
+	{ "freeze-security", cmd_freeze_security },
 	{ "list", cmd_list },
 	{ "monitor", cmd_monitor},
 	{ "help", cmd_help },

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

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

* [PATCH v6 05/12] ndctl: add support for sanitize dimm
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
                   ` (3 preceding siblings ...)
  2018-12-14 21:09 ` [PATCH v6 04/12] ndctl: add support for freeze security Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2018-12-14 21:09 ` [PATCH v6 06/12] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Add support to secure erase to libndctl and also command line option
of "sanitize-dimm" for ndctl. This will initiate the request to crypto
erase a DIMM.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/Makefile.am             |    3 +-
 Documentation/ndctl/ndctl-sanitize-dimm.txt |   32 +++++++++++++++++
 builtin.h                                   |    1 +
 ndctl/dimm.c                                |   51 +++++++++++++++++++++++++++
 ndctl/lib/dimm.c                            |    8 ++++
 ndctl/lib/keys.c                            |   19 ++++++++--
 ndctl/lib/libndctl.sym                      |    2 +
 ndctl/libndctl.h                            |    7 ++++
 ndctl/ndctl.c                               |    1 +
 9 files changed, 120 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/ndctl/ndctl-sanitize-dimm.txt

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index a97f193d..bbea9674 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -51,7 +51,8 @@ man1_MANS = \
 	ndctl-enable-passphrase.1 \
 	ndctl-update-passphrase.1 \
 	ndctl-disable-passphrase.1 \
-	ndctl-freeze-security.1
+	ndctl-freeze-security.1 \
+	ndctl-sanitize-dimm.1
 
 CLEANFILES = $(man1_MANS)
 
diff --git a/Documentation/ndctl/ndctl-sanitize-dimm.txt b/Documentation/ndctl/ndctl-sanitize-dimm.txt
new file mode 100644
index 00000000..a6802d30
--- /dev/null
+++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
@@ -0,0 +1,32 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-sanitize-dimm(1)
+======================
+
+NAME
+----
+ndctl-sanitize-dimm - sanitize the data on the NVDIMM
+
+SYNOPSIS
+--------
+[verse]
+'ndctl sanitize' <dimm> [<options>]
+
+DESCRIPTION
+-----------
+Provide a generic interface to crypto erase a NVDIMM.
+Ndctl will search the user key ring for the associated DIMM. If no key is
+found, it will attempt to load the key blob from the expected location. Ndctl
+will remove the key and the key blob once security is disabled.
+
+OPTIONS
+-------
+<dimm>::
+include::xable-dimm-options.txt[]
+
+-c::
+--crypto-erase::
+	Replaces encryption keys and securely erases the data. This does not
+	change label data. This is the default sanitize method.
+
+include::../copyright.txt[]
diff --git a/builtin.h b/builtin.h
index 827295da..db0f2858 100644
--- a/builtin.h
+++ b/builtin.h
@@ -52,4 +52,5 @@ int cmd_passphrase_setup(int argc, const char **argv, void *ctx);
 int cmd_passphrase_update(int argc, const char **argv, void *ctx);
 int cmd_disable_passphrase(int argc, const char **argv, void *ctx);
 int cmd_freeze_security(int argc, const char **argv, void *ctx);
+int cmd_sanitize_dimm(int argc, const char **argv, void *ctx);
 #endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 6476b6e5..ae674f78 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -46,6 +46,7 @@ static struct parameters {
 	const char *infile;
 	const char *labelversion;
 	const char *master_key;
+	bool crypto_erase;
 	bool force;
 	bool json;
 	bool verbose;
@@ -891,6 +892,35 @@ static int action_security_freeze(struct ndctl_dimm *dimm,
 	return rc;
 }
 
+static int action_sanitize_dimm(struct ndctl_dimm *dimm,
+		struct action_context *actx)
+{
+	int rc;
+
+	if (!ndctl_dimm_security_supported(dimm)) {
+		error("%s: security operation not supported\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EOPNOTSUPP;
+	}
+
+	/*
+	 * Setting crypto erase to be default. The other method will be
+	 * overwrite.
+	 */
+	if (!param.crypto_erase) {
+		param.crypto_erase = true;
+		printf("No santize method passed in, default to crypto-erase\n");
+	}
+
+	if (param.crypto_erase) {
+		rc = ndctl_dimm_secure_erase_key(dimm);
+		if (rc < 0)
+			return rc;
+	}
+
+	return 0;
+}
+
 static int __action_init(struct ndctl_dimm *dimm,
 		enum ndctl_namespace_version version, int chk_only)
 {
@@ -984,6 +1014,10 @@ OPT_STRING('V', "label-version", &param.labelversion, "version-number", \
 OPT_STRING('m', "master-key", &param.master_key, "<key_type>:<key_name>", \
 		"master key for security")
 
+#define SANITIZE_OPTIONS() \
+OPT_BOOLEAN('c', "crypto-erase", &param.crypto_erase, \
+		"crypto erase a dimm")
+
 static const struct option read_options[] = {
 	BASE_OPTIONS(),
 	READ_OPTIONS(),
@@ -1019,6 +1053,12 @@ static const struct option key_options[] = {
 	OPT_END(),
 };
 
+static const struct option sanitize_options[] = {
+	BASE_OPTIONS(),
+	SANITIZE_OPTIONS(),
+	OPT_END(),
+};
+
 static int dimm_action(int argc, const char **argv, void *ctx,
 		int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
 		const struct option *options, const char *usage)
@@ -1295,3 +1335,14 @@ int cmd_freeze_security(int argc, const char **argv, void *ctx)
 			count > 1 ? "s" : "");
 	return count >= 0 ? 0 : EXIT_FAILURE;
 }
+
+int cmd_sanitize_dimm(int argc, const char **argv, void *ctx)
+{
+	int count = dimm_action(argc, argv, ctx, action_sanitize_dimm,
+			sanitize_options,
+			"ndctl sanitize-dimm <nmem0> [<nmem1>..<nmemN>] [<options>]");
+
+	fprintf(stderr, "sanitized %d nmem%s.\n", count >= 0 ? count : 0,
+			count > 1 ? "s" : "");
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 227c7124..6da29c1e 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -673,3 +673,11 @@ NDCTL_EXPORT int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm)
 {
 	return write_security(dimm, "freeze");
 }
+
+NDCTL_EXPORT int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm, long key)
+{
+	char buf[SYSFS_ATTR_SIZE];
+
+	sprintf(buf, "erase %ld\n", key);
+	return write_security(dimm, buf);
+}
diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
index 779e82af..130c29ed 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -388,7 +388,8 @@ NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
 	return 0;
 }
 
-NDCTL_EXPORT int ndctl_dimm_disable_key(struct ndctl_dimm *dimm)
+static int check_key_run_and_discard(struct ndctl_dimm *dimm,
+		int (*run_op)(struct ndctl_dimm *, long), const char *name)
 {
 	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
 	key_serial_t key;
@@ -403,9 +404,9 @@ NDCTL_EXPORT int ndctl_dimm_disable_key(struct ndctl_dimm *dimm)
 		}
 	}
 
-	rc = ndctl_dimm_disable_passphrase(dimm, key);
+	rc = run_op(dimm, key);
 	if (rc < 0) {
-		err(ctx, "Failed to disable security for %s\n",
+		err(ctx, "Failed %s for %s\n", name,
 				ndctl_dimm_get_devname(dimm));
 		return rc;
 	}
@@ -415,3 +416,15 @@ NDCTL_EXPORT int ndctl_dimm_disable_key(struct ndctl_dimm *dimm)
 		err(ctx, "Unable to cleanup key.\n");
 	return 0;
 }
+
+NDCTL_EXPORT int ndctl_dimm_disable_key(struct ndctl_dimm *dimm)
+{
+	return check_key_run_and_discard(dimm, ndctl_dimm_disable_passphrase,
+			"disable passphrase");
+}
+
+NDCTL_EXPORT int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm)
+{
+	return check_key_run_and_discard(dimm, ndctl_dimm_secure_erase,
+			"crypto erase");
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index a1c56060..0e3aa5d9 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -396,4 +396,6 @@ global:
 	ndctl_dimm_disable_passphrase;
 	ndctl_dimm_disable_key;
 	ndctl_dimm_freeze_security;
+	ndctl_dimm_secure_erase;
+	ndctl_dimm_secure_erase_key;
 } LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 7bb7132c..19053a20 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -704,6 +704,7 @@ int ndctl_dimm_update_passphrase(struct ndctl_dimm *dimm,
 		long ckey, long nkey);
 int ndctl_dimm_disable_passphrase(struct ndctl_dimm *dimm, long key);
 int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm);
+int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm, long key);
 
 enum ndctl_key_type {
 	ND_USER_KEY,
@@ -714,6 +715,7 @@ enum ndctl_key_type {
 int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master);
 int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master);
 int ndctl_dimm_disable_key(struct ndctl_dimm *dimm);
+int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm);
 #else
 static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
 		const char *master)
@@ -731,6 +733,11 @@ static inline int ndctl_dimm_disable_key(struct ndctl_dimm *dimm)
 {
 	return -EOPNOTSUPP;
 }
+
+static inline int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 #ifdef __cplusplus
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index d08fee4d..40982f43 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -92,6 +92,7 @@ static struct cmd_struct commands[] = {
 	{ "update-passphrase", cmd_passphrase_update },
 	{ "disable-passphrase", cmd_disable_passphrase },
 	{ "freeze-security", cmd_freeze_security },
+	{ "sanitize-dimm", cmd_sanitize_dimm },
 	{ "list", cmd_list },
 	{ "monitor", cmd_monitor},
 	{ "help", cmd_help },

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

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

* [PATCH v6 06/12] ndctl: add unit test for security ops (minus overwrite)
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
                   ` (4 preceding siblings ...)
  2018-12-14 21:09 ` [PATCH v6 05/12] ndctl: add support for sanitize dimm Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2019-01-05  1:21   ` Verma, Vishal L
  2018-12-14 21:09 ` [PATCH v6 07/12] ndctl: setup modprobe rules Dave Jiang
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Add unit test for security enable, disable, update, erase, unlock, and
freeze.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 test/Makefile.am |    4 +
 test/security.sh |  191 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 195 insertions(+)
 create mode 100755 test/security.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index ebdd23f6..42009c31 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -27,6 +27,10 @@ TESTS =\
 	max_available_extent_ns.sh \
 	pfn-meta-errors.sh
 
+if ENABLE_KEYUTILS
+TESTS += security.sh
+endif
+
 check_PROGRAMS =\
 	libndctl \
 	dsm-fail \
diff --git a/test/security.sh b/test/security.sh
new file mode 100755
index 00000000..5238c9c4
--- /dev/null
+++ b/test/security.sh
@@ -0,0 +1,191 @@
+#!/bin/bash -Ex
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2018 Intel Corporation. All rights reserved.
+
+rc=77
+dev=""
+id=""
+dev_no=""
+sstate=""
+KEYPATH="/etc/ndctl/keys"
+UNLOCK="/sys/devices/platform/nfit_test.0/nfit_test_dimm/test_dimm"
+MASTERKEY="nvdimm-master-test"
+MASTERPATH="$KEYPATH/$MASTERKEY"
+
+. ./common
+
+trap 'err $LINENO' ERR
+
+setup()
+{
+	$NDCTL disable-region -b $NFIT_TEST_BUS0 all
+}
+
+detect()
+{
+	dev=$($NDCTL list -b $NFIT_TEST_BUS0 -D | jq .[0].dev | tr -d '"')
+	[ -n "$dev" ] || err "$LINENO"
+	id=$($NDCTL list -b $NFIT_TEST_BUS0 -D | jq .[0].id | tr -d '"')
+	[ -n "$id" ] || err "$LINENO"
+}
+
+setup_keys()
+{
+	if [ ! -f $MASTERPATH ]; then
+		keyctl add user $MASTERKEY "`dd if=/dev/urandom bs=1 count=32 2>/dev/null`" @u
+		keyctl pipe `keyctl search @u user $MASTERKEY` > $MASTERPATH
+	else
+		echo "Unclean setup. Please cleanup $MASTERPATH file."
+		exit 1
+	fi
+}
+
+test_cleanup()
+{
+	keyctl unlink `keyctl search @u encrypted nvdimm:$id`
+	keyctl unlink `keyctl search @u user $MASTERKEY`
+	rm -f $KEYPATH/nvdimm_$id\_`hostname`.blob
+	rm -f $MASTERPATH
+}
+
+locking_dimm()
+{
+	$NDCTL disable-dimm $dev
+	dev_no=$(echo $dev | cut -b 5-)
+	echo 1 > "$UNLOCK$dev_no/lock_dimm"
+	get_security_state
+	if [ "$sstate" != "locked" ]; then
+		echo "Incorrect security state: $sstate expected: disabled"
+		exit 1
+	fi
+}
+
+get_security_state()
+{
+	sstate=$($NDCTL list -i -b $NFIT_TEST_BUS0 -d $dev | jq .[].dimms[0].security | tr -d '"')
+	[ -n "$sstate" ] || err "$LINENO"
+}
+
+enable_passphrase()
+{
+	$NDCTL enable-passphrase -m user:$MASTERKEY $dev
+	get_security_state
+	if [ "$sstate" != "unlocked" ]; then
+		echo "Incorrect security state: $sstate expected: unlocked"
+		exit 1
+	fi
+}
+
+disable_passphrase()
+{
+	$NDCTL disable-passphrase $dev
+	get_security_state
+	if [ "$sstate" != "disabled" ]; then
+		echo "Incorrect security state: $sstate expected: disabled"
+		exit 1
+	fi
+}
+
+erase_security()
+{
+	$NDCTL sanitize-dimm -c $dev
+	get_security_state
+	if [ "$sstate" != "disabled" ]; then
+		echo "Incorrect security state: $sstate expected: disabled"
+		exit 1
+	fi
+}
+
+update_security()
+{
+	$NDCTL update-passphrase -m user:$MASTERKEY $dev
+	get_security_state
+	if [ "$sstate" != "unlocked" ]; then
+		echo "Incorrect security state: $sstate expected: unlocked"
+		exit 1
+	fi
+}
+
+freeze_security()
+{
+	$NDCTL freeze-security $dev
+}
+
+test_1_security_enable_and_disable()
+{
+	enable_passphrase
+	disable_passphrase
+}
+
+test_2_security_enable_and_update()
+{
+	enable_passphrase
+	update_security
+	disable_passphrase
+}
+
+test_3_security_enable_and_erase()
+{
+	enable_passphrase
+	erase_security
+}
+
+test_4_security_unlocking()
+{
+	enable_passphrase
+	locking_dimm
+	$NDCTL enable-dimm $dev
+	get_security_state
+	if [ "$sstate" != "unlocked" ]; then
+		echo "Incorrect security state: $sstate expected: unlocked"
+		exit 1
+	fi
+	$NDCTL disable-region -b $NFIT_TEST_BUS0 all
+	disable_passphrase
+}
+
+# this should always be the last test. with security frozen, nfit_test must
+# be removed and is no longer usable
+test_5_security_freeze()
+{
+	enable_passphrase
+	freeze_security
+	get_security_state
+	if [ "$sstate" != "frozen" ]; then
+		echo "Incorrect security state: $sstate expected: frozen"
+		exit 1
+	fi
+	$NDCTL disable-passphrase $dev && { echo "diable succeed after frozen"; exit 1; }
+	get_security_state
+	echo $sstate
+	if [ "$sstate" != "frozen" ]; then
+		echo "Incorrect security state: $sstate expected: disabled"
+		exit 1
+	fi
+}
+
+check_min_kver "4.21" || do_skip "may lack security test handling"
+
+modprobe nfit_test
+rc=1
+setup
+rc=2
+detect
+setup_keys
+echo "Test 1, security enable and disable"
+test_1_security_enable_and_disable
+echo "Test 2, security enable, update, and disable"
+test_2_security_enable_and_update
+echo "Test 3, security enable and erase"
+test_3_security_enable_and_erase
+echo "Test 4, unlocking dimm"
+test_4_security_unlocking
+
+# Freeze should always be run last because it locks security state and require
+# nfit_test module unload.
+echo "Test 5, freeze security"
+test_5_security_freeze
+
+test_cleanup
+_cleanup
+exit 0

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

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

* [PATCH v6 07/12] ndctl: setup modprobe rules
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
                   ` (5 preceding siblings ...)
  2018-12-14 21:09 ` [PATCH v6 06/12] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2019-01-05  1:40   ` Verma, Vishal L
  2018-12-14 21:09 ` [PATCH v6 08/12] ndctl: add overwrite operation support Dave Jiang
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Adding reference config file for modprobe.d in order to trigger the
reference script that will inject keys associated with the nvdimms into
the kernel user ring for unlock.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Makefile.am                  |   10 ++++++++++
 contrib/ndctl-loadkeys.sh    |   24 ++++++++++++++++++++++++
 contrib/nvdimm_modprobe.conf |    1 +
 3 files changed, 35 insertions(+)
 create mode 100755 contrib/ndctl-loadkeys.sh
 create mode 100644 contrib/nvdimm_modprobe.conf

diff --git a/Makefile.am b/Makefile.am
index e0c463a3..5a3f03aa 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,6 +42,16 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
 dist_bashcompletion_DATA = contrib/ndctl
 endif
 
+load_key_file = contrib/ndctl-loadkeys.sh
+load_keydir = $(sysconfdir)/ndctl/
+load_key_DATA = $(load_key_file)
+EXTRA_DIST += $(load_key_file)
+
+modprobe_file = contrib/nvdimm_modprobe.conf
+modprobedir = $(sysconfdir)/modprobe.d/
+modprobe_DATA = $(modprobe_file)
+EXTRA_DIST += $(modprobe_file)
+
 noinst_LIBRARIES = libccan.a
 libccan_a_SOURCES = \
 	ccan/str/str.h \
diff --git a/contrib/ndctl-loadkeys.sh b/contrib/ndctl-loadkeys.sh
new file mode 100755
index 00000000..dae0a88a
--- /dev/null
+++ b/contrib/ndctl-loadkeys.sh
@@ -0,0 +1,24 @@
+#!/bin/bash -Ex
+
+# This script assumes a single master key for all DIMMs
+
+KEY_PATH=/etc/ndctl/keys
+TPMH_PATH=$KEY_PATH/tpm.handle
+KEYTPE=""
+TPM_HANDLE=""
+id=""
+
+if [ -f $TPMH_PATH ]; then
+	KEYTYPE=trusted
+	TPM_HANDLE="keyhandle=`cat $TPMH_PATH`"
+else
+	KEYTYPE=user
+fi
+
+keyctl show | grep -q nvdimm-master || keyctl add $KEYTYPE nvdimm-master "load `cat $KEY_PATH/nvdimm-master.blob` $TPM_HANDLE" @u > /dev/null
+
+for i in `ls -1 $KEY_PATH/nvdimm_*.blob`;
+do
+	id=`echo $i | cut -d'_' -f2`
+	keyctl add encrypted nvdimm:$id "load `cat $i`" @u
+done
diff --git a/contrib/nvdimm_modprobe.conf b/contrib/nvdimm_modprobe.conf
new file mode 100644
index 00000000..b113d8d7
--- /dev/null
+++ b/contrib/nvdimm_modprobe.conf
@@ -0,0 +1 @@
+install libnvdimm /usr/sbin/ndctl-loadkeys.sh ; /sbin/modprobe --ignore-install libnvdimm $CMDLINE_OPTS

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

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

* [PATCH v6 08/12] ndctl: add overwrite operation support
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
                   ` (6 preceding siblings ...)
  2018-12-14 21:09 ` [PATCH v6 07/12] ndctl: setup modprobe rules Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2018-12-14 21:09 ` [PATCH v6 09/12] ndctl: add overwrite-wait support Dave Jiang
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Add support for overwrite to libndctl. The operation will be triggered
by the sanitize-dimm command with -o switch. This will initiate the request
to wipe the entire nvdimm. Success return of the command only indicate
overwrite has started and does not indicate completion of overwrite.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/ndctl-sanitize-dimm.txt |    4 ++++
 ndctl/dimm.c                                |   21 ++++++++++++++++----
 ndctl/lib/dimm.c                            |    8 ++++++++
 ndctl/lib/keys.c                            |   28 ++++++++++++++++-----------
 ndctl/lib/libndctl.sym                      |    2 ++
 ndctl/libndctl.h                            |    7 +++++++
 6 files changed, 55 insertions(+), 15 deletions(-)

diff --git a/Documentation/ndctl/ndctl-sanitize-dimm.txt b/Documentation/ndctl/ndctl-sanitize-dimm.txt
index a6802d30..beb4b2f9 100644
--- a/Documentation/ndctl/ndctl-sanitize-dimm.txt
+++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
@@ -29,4 +29,8 @@ include::xable-dimm-options.txt[]
 	Replaces encryption keys and securely erases the data. This does not
 	change label data. This is the default sanitize method.
 
+-o::
+--ovewrite::
+	Wipe the entire DIMM, including label data. Can take significant time.
+
 include::../copyright.txt[]
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index ae674f78..6b1ee47f 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -47,6 +47,7 @@ static struct parameters {
 	const char *labelversion;
 	const char *master_key;
 	bool crypto_erase;
+	bool overwrite;
 	bool force;
 	bool json;
 	bool verbose;
@@ -907,7 +908,7 @@ static int action_sanitize_dimm(struct ndctl_dimm *dimm,
 	 * Setting crypto erase to be default. The other method will be
 	 * overwrite.
 	 */
-	if (!param.crypto_erase) {
+	if (!param.crypto_erase && !param.overwrite) {
 		param.crypto_erase = true;
 		printf("No santize method passed in, default to crypto-erase\n");
 	}
@@ -918,6 +919,12 @@ static int action_sanitize_dimm(struct ndctl_dimm *dimm,
 			return rc;
 	}
 
+	if (param.overwrite) {
+		rc = ndctl_dimm_overwrite_key(dimm);
+		if (rc < 0)
+			return rc;
+	}
+
 	return 0;
 }
 
@@ -1016,7 +1023,9 @@ OPT_STRING('m', "master-key", &param.master_key, "<key_type>:<key_name>", \
 
 #define SANITIZE_OPTIONS() \
 OPT_BOOLEAN('c', "crypto-erase", &param.crypto_erase, \
-		"crypto erase a dimm")
+		"crypto erase a dimm"), \
+OPT_BOOLEAN('o', "overwrite", &param.overwrite, \
+		"overwrite a dimm")
 
 static const struct option read_options[] = {
 	BASE_OPTIONS(),
@@ -1342,7 +1351,11 @@ int cmd_sanitize_dimm(int argc, const char **argv, void *ctx)
 			sanitize_options,
 			"ndctl sanitize-dimm <nmem0> [<nmem1>..<nmemN>] [<options>]");
 
-	fprintf(stderr, "sanitized %d nmem%s.\n", count >= 0 ? count : 0,
-			count > 1 ? "s" : "");
+	if (param.overwrite)
+		fprintf(stderr, "overwrite issued for %d nmem%s.\n",
+				count >= 0 ? count : 0, count > 1 ? "s" : "");
+	else
+		fprintf(stderr, "sanitized %d nmem%s.\n",
+				count >= 0 ? count : 0, count > 1 ? "s" : "");
 	return count >= 0 ? 0 : EXIT_FAILURE;
 }
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 6da29c1e..2dff80f0 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -681,3 +681,11 @@ NDCTL_EXPORT int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm, long key)
 	sprintf(buf, "erase %ld\n", key);
 	return write_security(dimm, buf);
 }
+
+NDCTL_EXPORT int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key)
+{
+	char buf[SYSFS_ATTR_SIZE];
+
+	sprintf(buf, "overwrite %ld\n", key);
+	return write_security(dimm, buf);
+}
diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
index 130c29ed..fcf300bb 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -85,10 +85,9 @@ static char *load_key_blob(struct ndctl_ctx *ctx, const char *path, int *size)
 	char prefix[] = "load ";
 
 	rc = stat(path, &st);
-	if (rc < 0) {
-		err(ctx, "stat: %s\n", strerror(errno));
+	if (rc < 0)
 		return NULL;
-	}
+
 	if ((st.st_mode & S_IFMT) != S_IFREG) {
 		err(ctx, "%s not a regular file\n", path);
 		return NULL;
@@ -324,10 +323,8 @@ static int dimm_remove_key(struct ndctl_dimm *dimm,
 	}
 
 	rc = unlink(path);
-	if (rc < 0) {
-		err(ctx, "unlink: %s\n", strerror(errno));
+	if (rc < 0)
 		return -errno;
-	}
 
 	return 0;
 }
@@ -398,10 +395,11 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
 	key = dimm_check_key(dimm, false);
 	if (key < 0) {
 		key = dimm_load_key(dimm, false);
-		if (key < 0) {
+		if (key < 0 && run_op != ndctl_dimm_overwrite) {
 			err(ctx, "Unable to load key\n");
 			return -ENOKEY;
-		}
+		} else
+			key = 0;
 	}
 
 	rc = run_op(dimm, key);
@@ -411,9 +409,11 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
 		return rc;
 	}
 
-	rc = dimm_remove_key(dimm, false);
-	if (rc < 0)
-		err(ctx, "Unable to cleanup key.\n");
+	if (key) {
+		rc = dimm_remove_key(dimm, false);
+		if (rc < 0)
+			err(ctx, "Unable to cleanup key.\n");
+	}
 	return 0;
 }
 
@@ -428,3 +428,9 @@ NDCTL_EXPORT int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm)
 	return check_key_run_and_discard(dimm, ndctl_dimm_secure_erase,
 			"crypto erase");
 }
+
+NDCTL_EXPORT int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm)
+{
+	return check_key_run_and_discard(dimm, ndctl_dimm_overwrite,
+			"overwrite");
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 0e3aa5d9..0fedae1a 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -398,4 +398,6 @@ global:
 	ndctl_dimm_freeze_security;
 	ndctl_dimm_secure_erase;
 	ndctl_dimm_secure_erase_key;
+	ndctl_dimm_overwrite;
+	ndctl_dimm_overwrite_key;
 } LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 19053a20..fee35db7 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -705,6 +705,7 @@ int ndctl_dimm_update_passphrase(struct ndctl_dimm *dimm,
 int ndctl_dimm_disable_passphrase(struct ndctl_dimm *dimm, long key);
 int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm);
 int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm, long key);
+int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key);
 
 enum ndctl_key_type {
 	ND_USER_KEY,
@@ -716,6 +717,7 @@ int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master);
 int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master);
 int ndctl_dimm_disable_key(struct ndctl_dimm *dimm);
 int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm);
+int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm);
 #else
 static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
 		const char *master)
@@ -738,6 +740,11 @@ static inline int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm)
 {
 	return -EOPNOTSUPP;
 }
+
+int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm)
+{
+	return -EOPNOTSUPP;
+}
 #endif
 
 #ifdef __cplusplus

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

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

* [PATCH v6 09/12] ndctl: add overwrite-wait support
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
                   ` (7 preceding siblings ...)
  2018-12-14 21:09 ` [PATCH v6 08/12] ndctl: add overwrite operation support Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2019-01-05  1:54   ` Verma, Vishal L
  2018-12-14 21:09 ` [PATCH v6 10/12] ndctl: master phassphrase management support Dave Jiang
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Adding a monitoring command to ndctl in order to wait on the progress of
overwrite.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/Makefile.am              |    3 +
 Documentation/ndctl/ndctl-wait-overwrite.txt |   31 ++++++++++
 builtin.h                                    |    1 
 ndctl/dimm.c                                 |   27 +++++++++
 ndctl/lib/dimm.c                             |   79 ++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym                       |    1 
 ndctl/libndctl.h                             |    1 
 ndctl/ndctl.c                                |    1 
 8 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ndctl/ndctl-wait-overwrite.txt

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index bbea9674..a60a67e5 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -52,7 +52,8 @@ man1_MANS = \
 	ndctl-update-passphrase.1 \
 	ndctl-disable-passphrase.1 \
 	ndctl-freeze-security.1 \
-	ndctl-sanitize-dimm.1
+	ndctl-sanitize-dimm.1 \
+	ndctl-wait-overwrite.1
 
 CLEANFILES = $(man1_MANS)
 
diff --git a/Documentation/ndctl/ndctl-wait-overwrite.txt b/Documentation/ndctl/ndctl-wait-overwrite.txt
new file mode 100644
index 00000000..97583a36
--- /dev/null
+++ b/Documentation/ndctl/ndctl-wait-overwrite.txt
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-wait-overwrite(1)
+=======================
+
+NAME
+----
+ndctl-wait-overwrite - wait for nvdimm overwrite operation to complete
+
+SYNOPSIS
+--------
+[verse]
+'ndctl wait-overwrite' <dimm> [<options>]
+
+DESCRIPTION
+-----------
+The ernel provides a POLL(2) capable sysfs file ('security') to indicate
+the state of overwrite. The 'ndctl wait-overwrite' operation waits for
+'security', across all specified dimms.
+
+OPTIONS
+-------
+-v::
+--verbose::
+	Emit debug messages for the overwrite wait process
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkndctl:ndctl-sanitize-dimm[1]
diff --git a/builtin.h b/builtin.h
index db0f2858..32bee59e 100644
--- a/builtin.h
+++ b/builtin.h
@@ -53,4 +53,5 @@ int cmd_passphrase_update(int argc, const char **argv, void *ctx);
 int cmd_disable_passphrase(int argc, const char **argv, void *ctx);
 int cmd_freeze_security(int argc, const char **argv, void *ctx);
 int cmd_sanitize_dimm(int argc, const char **argv, void *ctx);
+int cmd_wait_overwrite(int argc, const char **argv, void *ctx);
 #endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 6b1ee47f..21ffea1e 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -928,6 +928,24 @@ static int action_sanitize_dimm(struct ndctl_dimm *dimm,
 	return 0;
 }
 
+static int action_wait_overwrite(struct ndctl_dimm *dimm,
+		struct action_context *actx)
+{
+	int rc;
+
+	if (!ndctl_dimm_security_supported(dimm)) {
+		error("%s: security operation not supported\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EOPNOTSUPP;
+	}
+
+	rc = ndctl_dimm_wait_for_overwrite_completion(dimm);
+	if (rc == 1)
+		printf("%s: overwrite completed.\n",
+				ndctl_dimm_get_devname(dimm));
+	return rc;
+}
+
 static int __action_init(struct ndctl_dimm *dimm,
 		enum ndctl_namespace_version version, int chk_only)
 {
@@ -1359,3 +1377,12 @@ int cmd_sanitize_dimm(int argc, const char **argv, void *ctx)
 				count >= 0 ? count : 0, count > 1 ? "s" : "");
 	return count >= 0 ? 0 : EXIT_FAILURE;
 }
+
+int cmd_wait_overwrite(int argc, const char **argv, void *ctx)
+{
+	int count = dimm_action(argc, argv, ctx, action_wait_overwrite,
+			base_options,
+			"ndctl wait-overwrite <nmem0> [<nmem1>..<nmemN>] [<options>]");
+
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 2dff80f0..d815b9fc 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -16,6 +16,7 @@
 #include <util/bitmap.h>
 #include <util/sysfs.h>
 #include <stdlib.h>
+#include <poll.h>
 #include "private.h"
 
 static const char NSINDEX_SIGNATURE[] = "NAMESPACE_INDEX\0";
@@ -689,3 +690,81 @@ NDCTL_EXPORT int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key)
 	sprintf(buf, "overwrite %ld\n", key);
 	return write_security(dimm, buf);
 }
+
+NDCTL_EXPORT int ndctl_dimm_wait_for_overwrite_completion(
+		struct ndctl_dimm *dimm)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	struct pollfd fds;
+	char buf[SYSFS_ATTR_SIZE];
+	int fd = 0, rc;
+	char *path = dimm->dimm_buf;
+	int len = dimm->buf_len;
+
+	if (snprintf(path, len, "%s/security", dimm->dimm_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				ndctl_dimm_get_devname(dimm));
+		return -ERANGE;
+	}
+
+	fd = open(path, O_RDONLY|O_CLOEXEC);
+	if (fd < 0) {
+		rc = -errno;
+		err(ctx, "open: %s\n", strerror(errno));
+		return rc;
+	}
+	memset(&fds, 0, sizeof(fds));
+	fds.fd = fd;
+
+	rc = sysfs_read_attr(ctx, path, buf);
+	if (rc < 0) {
+		rc = -EOPNOTSUPP;
+		goto out;
+	}
+	/* skipping if we aren't in overwrite state */
+	if (strcmp(buf, "overwrite") != 0) {
+		rc = 0;
+		goto out;
+	}
+
+	for (;;) {
+		rc = sysfs_read_attr(ctx, path, buf);
+		if (rc < 0) {
+			rc = -EOPNOTSUPP;
+			break;
+		}
+
+		if (strcmp(buf, "overwrite") == 0) {
+			rc = poll(&fds, 1, -1);
+			if (rc < 0) {
+				rc = -errno;
+				err(ctx, "poll error: %s\n", strerror(errno));
+				break;
+			}
+			dbg(ctx, "poll wake: revents: %d\n", fds.revents);
+			if (pread(fd, buf, 1, 0) == -1) {
+				rc = -errno;
+				break;
+			}
+			fds.revents = 0;
+		} else {
+			if (strcmp(buf, "disabled") == 0)
+				rc = 1;
+			break;
+		}
+	}
+
+	if (rc == 1)
+		dbg(ctx, "%s: overwrite complete\n",
+				ndctl_dimm_get_devname(dimm));
+	else if (rc == 0)
+		dbg(ctx, "%s: ovewrite skipped\n",
+				ndctl_dimm_get_devname(dimm));
+	else
+		dbg(ctx, "%s: overwrite error waiting for complete\n",
+				ndctl_dimm_get_devname(dimm));
+
+ out:
+	close(fd);
+	return rc;
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 0fedae1a..0da8b282 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -400,4 +400,5 @@ global:
 	ndctl_dimm_secure_erase_key;
 	ndctl_dimm_overwrite;
 	ndctl_dimm_overwrite_key;
+	ndctl_dimm_wait_for_overwrite_completion;
 } LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index fee35db7..5c34f2cb 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -706,6 +706,7 @@ int ndctl_dimm_disable_passphrase(struct ndctl_dimm *dimm, long key);
 int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm);
 int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm, long key);
 int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key);
+int ndctl_dimm_wait_for_overwrite_completion(struct ndctl_dimm *dimm);
 
 enum ndctl_key_type {
 	ND_USER_KEY,
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 40982f43..d16e1ed0 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -93,6 +93,7 @@ static struct cmd_struct commands[] = {
 	{ "disable-passphrase", cmd_disable_passphrase },
 	{ "freeze-security", cmd_freeze_security },
 	{ "sanitize-dimm", cmd_sanitize_dimm },
+	{ "wait-overwrite", cmd_wait_overwrite },
 	{ "list", cmd_list },
 	{ "monitor", cmd_monitor},
 	{ "help", cmd_help },

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

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

* [PATCH v6 10/12] ndctl: master phassphrase management support
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
                   ` (8 preceding siblings ...)
  2018-12-14 21:09 ` [PATCH v6 09/12] ndctl: add overwrite-wait support Dave Jiang
@ 2018-12-14 21:09 ` Dave Jiang
  2019-01-05  2:01   ` Verma, Vishal L
  2018-12-14 21:10 ` [PATCH v6 11/12] ndctl: add master secure erase support Dave Jiang
  2018-12-14 21:10 ` [PATCH v6 12/12] ndctl: documentation for security and key management Dave Jiang
  11 siblings, 1 reply; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:09 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Adding master passphrase enabling and update to ndctl. This is a new
feature from Intel DSM v1.8.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/ndctl-enable-passphrase.txt |    7 +
 Documentation/ndctl/ndctl-update-passphrase.txt |    7 +
 ndctl/dimm.c                                    |   11 ++
 ndctl/lib/dimm.c                                |    9 ++
 ndctl/lib/keys.c                                |  113 +++++++++++++++++------
 ndctl/lib/libndctl.sym                          |    1 
 ndctl/libndctl.h                                |   14 ++-
 7 files changed, 122 insertions(+), 40 deletions(-)

diff --git a/Documentation/ndctl/ndctl-enable-passphrase.txt b/Documentation/ndctl/ndctl-enable-passphrase.txt
index 8de5410c..6639ce8d 100644
--- a/Documentation/ndctl/ndctl-enable-passphrase.txt
+++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
@@ -29,7 +29,12 @@ OPTIONS
 include::xable-dimm-options.txt[]
 
 -m::
---master=::
+--master-key=::
 	Key name for the master key used to seal the NVDIMM security keys.
 
+-M::
+--master-passphrase::
+	Parameter to indicate that we are managing the master passphrase
+	instead of the user passphrase.
+
 include::../copyright.txt[]
diff --git a/Documentation/ndctl/ndctl-update-passphrase.txt b/Documentation/ndctl/ndctl-update-passphrase.txt
index 9ed39cca..e2ecacf5 100644
--- a/Documentation/ndctl/ndctl-update-passphrase.txt
+++ b/Documentation/ndctl/ndctl-update-passphrase.txt
@@ -26,8 +26,13 @@ OPTIONS
 include::xable-dimm-options.txt[]
 
 -m::
---master::
+--master-key=::
 	New key name for the master key to seal the new nvdimm key, or the
 	existing master key name. i.e trusted:master-key.
 
+-M::
+--master-passphrase::
+	Parameter to indicate that we are managing the master passphrase
+	instead of the user passphrase.
+
 include::../copyright.txt[]
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 21ffea1e..c60ef96e 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -48,6 +48,7 @@ static struct parameters {
 	const char *master_key;
 	bool crypto_erase;
 	bool overwrite;
+	bool master_pass;
 	bool force;
 	bool json;
 	bool verbose;
@@ -848,7 +849,8 @@ static int action_key_enable(struct ndctl_dimm *dimm,
 		return -EOPNOTSUPP;
 	}
 
-	return ndctl_dimm_enable_key(dimm, param.master_key);
+	return ndctl_dimm_enable_key(dimm, param.master_key,
+			param.master_pass ? ND_MASTER_KEY : ND_USER_KEY);
 }
 
 static int action_key_update(struct ndctl_dimm *dimm,
@@ -860,7 +862,8 @@ static int action_key_update(struct ndctl_dimm *dimm,
 		return -EOPNOTSUPP;
 	}
 
-	return ndctl_dimm_update_key(dimm, param.master_key);
+	return ndctl_dimm_update_key(dimm, param.master_key,
+			param.master_pass ? ND_MASTER_KEY : ND_USER_KEY);
 }
 
 static int action_passphrase_disable(struct ndctl_dimm *dimm,
@@ -1037,7 +1040,9 @@ OPT_STRING('V', "label-version", &param.labelversion, "version-number", \
 
 #define KEY_OPTIONS() \
 OPT_STRING('m', "master-key", &param.master_key, "<key_type>:<key_name>", \
-		"master key for security")
+		"master key for security"), \
+OPT_BOOLEAN('M', "master-passphrase", &param.master_pass, \
+		"use master passphrase")
 
 #define SANITIZE_OPTIONS() \
 OPT_BOOLEAN('c', "crypto-erase", &param.crypto_erase, \
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index d815b9fc..07513b4b 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -768,3 +768,12 @@ NDCTL_EXPORT int ndctl_dimm_wait_for_overwrite_completion(
 	close(fd);
 	return rc;
 }
+
+NDCTL_EXPORT int ndctl_dimm_update_master_passphrase(struct ndctl_dimm *dimm,
+		long ckey, long nkey)
+{
+	char buf[SYSFS_ATTR_SIZE];
+
+	sprintf(buf, "master_update %ld %ld\n", ckey, nkey);
+	return write_security(dimm, buf);
+}
diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
index fcf300bb..1d395b48 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -34,16 +34,29 @@ static int get_key_path(struct ndctl_dimm *dimm, char *path,
 		return -errno;
 	}
 
-	if (key_type == ND_USER_OLD_KEY) {
-		rc = sprintf(path, "%s/nvdimmold_%s_%s.blob",
-				ND_PASS_PATH,
-				ndctl_dimm_get_unique_id(dimm),
+	switch (key_type) {
+	case ND_USER_OLD_KEY:
+		rc = sprintf(path, "%s/nvdimm-old_%s_%s.blob",
+				ND_PASS_PATH, ndctl_dimm_get_unique_id(dimm),
 				hostname);
-	} else {
+		break;
+	case ND_USER_KEY:
 		rc = sprintf(path, "%s/nvdimm_%s_%s.blob",
-				ND_PASS_PATH,
-				ndctl_dimm_get_unique_id(dimm),
+				ND_PASS_PATH, ndctl_dimm_get_unique_id(dimm),
 				hostname);
+		break;
+	case ND_MASTER_OLD_KEY:
+		rc = sprintf(path, "%s/nvdimm-master-old_%s_%s.blob",
+				ND_PASS_PATH, ndctl_dimm_get_unique_id(dimm),
+				hostname);
+		break;
+	case ND_MASTER_KEY:
+		rc = sprintf(path, "%s/nvdimm-master_%s_%s.blob",
+				ND_PASS_PATH, ndctl_dimm_get_unique_id(dimm),
+				hostname);
+		break;
+	default:
+		return -EINVAL;
 	}
 
 	if (rc < 0) {
@@ -60,12 +73,26 @@ static int get_key_desc(struct ndctl_dimm *dimm, char *desc,
 	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
 	int rc;
 
-	if (key_type == ND_USER_OLD_KEY)
+	switch (key_type) {
+	case ND_USER_OLD_KEY:
 		rc = sprintf(desc, "nvdimm-old:%s",
 				ndctl_dimm_get_unique_id(dimm));
-	else
+		break;
+	case ND_USER_KEY:
 		rc = sprintf(desc, "nvdimm:%s",
 				ndctl_dimm_get_unique_id(dimm));
+		break;
+	case ND_MASTER_OLD_KEY:
+		rc = sprintf(desc, "nvdimm-master-old:%s",
+				ndctl_dimm_get_unique_id(dimm));
+		break;
+	case ND_MASTER_KEY:
+		rc = sprintf(desc, "nvdimm-master:%s",
+				ndctl_dimm_get_unique_id(dimm));
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	if (rc < 0) {
 		err(ctx, "sprintf: %s\n", strerror(errno));
@@ -144,7 +171,7 @@ static key_serial_t dimm_check_key(struct ndctl_dimm *dimm,
 }
 
 static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
-		const char *master)
+		const char *master_key, enum ndctl_key_type key_type)
 {
 	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
 	char desc[DESC_SIZE];
@@ -164,7 +191,7 @@ static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
 		return -EBUSY;
 	}
 
-	rc = get_key_desc(dimm, desc, ND_USER_KEY);
+	rc = get_key_desc(dimm, desc, key_type);
 	if (rc < 0)
 		return rc;
 
@@ -175,7 +202,7 @@ static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
 		return -EAGAIN;
 	}
 
-	rc = get_key_path(dimm, path, ND_USER_KEY);
+	rc = get_key_path(dimm, path, key_type);
 	if (rc < 0)
 		return rc;
 
@@ -185,7 +212,7 @@ static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
 		return -EAGAIN;
 	}
 
-	rc = sprintf(cmd, "new enc32 %s 32", master);
+	rc = sprintf(cmd, "new enc32 %s 32", master_key);
 	if (rc < 0) {
 		err(ctx, "sprintf: %s\n", strerror(errno));
 		return -errno;
@@ -271,13 +298,15 @@ static key_serial_t dimm_load_key(struct ndctl_dimm *dimm,
  * blob, and then attempt to inject the key as old key into the user key
  * ring.
  */
-static key_serial_t move_key_to_old(struct ndctl_dimm *dimm)
+static key_serial_t move_key_to_old(struct ndctl_dimm *dimm,
+		enum ndctl_key_type key_type)
 {
 	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
 	int rc;
 	key_serial_t key;
 	char old_path[PATH_SIZE];
 	char new_path[PATH_SIZE];
+	enum ndctl_key_type okey_type;
 
 	if (ndctl_dimm_is_active(dimm)) {
 		err(ctx, "regions active on %s, op failed\n",
@@ -285,15 +314,22 @@ static key_serial_t move_key_to_old(struct ndctl_dimm *dimm)
 		return -EBUSY;
 	}
 
-	key = dimm_check_key(dimm, ND_USER_KEY);
+	key = dimm_check_key(dimm, key_type);
 	if (key > 0)
 		keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
 
-	rc = get_key_path(dimm, old_path, ND_USER_KEY);
+	if (key_type == ND_USER_KEY)
+		okey_type = ND_USER_OLD_KEY;
+	else if (key_type == ND_MASTER_KEY)
+		okey_type = ND_MASTER_OLD_KEY;
+	else
+		return -EINVAL;
+
+	rc = get_key_path(dimm, old_path, key_type);
 	if (rc < 0)
 		return rc;
 
-	rc = get_key_path(dimm, new_path, ND_USER_OLD_KEY);
+	rc = get_key_path(dimm, new_path, okey_type);
 	if (rc < 0)
 		return rc;
 
@@ -301,7 +337,7 @@ static key_serial_t move_key_to_old(struct ndctl_dimm *dimm)
 	if (rc < 0)
 		return -errno;
 
-	return dimm_load_key(dimm, ND_USER_OLD_KEY);
+	return dimm_load_key(dimm, okey_type);
 }
 
 static int dimm_remove_key(struct ndctl_dimm *dimm,
@@ -330,18 +366,21 @@ static int dimm_remove_key(struct ndctl_dimm *dimm,
 }
 
 NDCTL_EXPORT int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
-		const char *master)
+		const char *master_key, enum ndctl_key_type key_type)
 {
 	key_serial_t key;
 	int rc;
 
-	key = dimm_create_key(dimm, master);
+	key = dimm_create_key(dimm, master_key, key_type);
 	if (key < 0)
 		return (int)key;
 
-	rc = ndctl_dimm_update_passphrase(dimm, 0, key);
+	if (key_type == ND_MASTER_KEY)
+		rc = ndctl_dimm_update_master_passphrase(dimm, 0, key);
+	else
+		rc = ndctl_dimm_update_passphrase(dimm, 0, key);
 	if (rc < 0) {
-		dimm_remove_key(dimm, ND_USER_KEY);
+		dimm_remove_key(dimm, key_type);
 		return rc;
 	}
 
@@ -349,10 +388,18 @@ NDCTL_EXPORT int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
 }
 
 NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
-		const char *master)
+		const char *master_key, enum ndctl_key_type key_type)
 {
 	int rc;
 	key_serial_t old_key, new_key;
+	enum ndctl_key_type okey_type;
+
+	if (key_type == ND_USER_KEY)
+		okey_type = ND_USER_OLD_KEY;
+	else if (key_type == ND_MASTER_KEY)
+		okey_type = ND_MASTER_OLD_KEY;
+	else
+		return -EINVAL;
 
 	/*
 	 * 1. check if current key is loaded and remove
@@ -362,23 +409,27 @@ NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
 	 * 5. remove old key
 	 * 6. remove old key blob
 	 */
-	old_key = move_key_to_old(dimm);
+	old_key = move_key_to_old(dimm, key_type);
 	if (old_key < 0)
 		return old_key;
 
-	new_key = dimm_create_key(dimm, master);
+	new_key = dimm_create_key(dimm, master_key, key_type);
 	/* need to create new key here */
 	if (new_key < 0) {
-		new_key = dimm_load_key(dimm, ND_USER_KEY);
+		new_key = dimm_load_key(dimm, key_type);
 		if (new_key < 0)
 			return new_key;
 	}
 
-	rc = ndctl_dimm_update_passphrase(dimm, old_key, new_key);
+	if (key_type == ND_MASTER_KEY)
+		rc = ndctl_dimm_update_master_passphrase(dimm,
+				old_key, new_key);
+	else
+		rc = ndctl_dimm_update_passphrase(dimm, old_key, new_key);
 	if (rc < 0)
 		return rc;
 
-	rc = dimm_remove_key(dimm, ND_USER_OLD_KEY);
+	rc = dimm_remove_key(dimm, okey_type);
 	if (rc < 0)
 		return rc;
 
@@ -392,9 +443,9 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
 	key_serial_t key;
 	int rc;
 
-	key = dimm_check_key(dimm, false);
+	key = dimm_check_key(dimm, ND_USER_KEY);
 	if (key < 0) {
-		key = dimm_load_key(dimm, false);
+		key = dimm_load_key(dimm, ND_USER_KEY);
 		if (key < 0 && run_op != ndctl_dimm_overwrite) {
 			err(ctx, "Unable to load key\n");
 			return -ENOKEY;
@@ -410,7 +461,7 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
 	}
 
 	if (key) {
-		rc = dimm_remove_key(dimm, false);
+		rc = dimm_remove_key(dimm, ND_USER_KEY);
 		if (rc < 0)
 			err(ctx, "Unable to cleanup key.\n");
 	}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 0da8b282..bd933eb2 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -401,4 +401,5 @@ global:
 	ndctl_dimm_overwrite;
 	ndctl_dimm_overwrite_key;
 	ndctl_dimm_wait_for_overwrite_completion;
+	ndctl_dimm_update_master_passphrase;
 } LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 5c34f2cb..a0b8a886 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -707,27 +707,33 @@ int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm);
 int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm, long key);
 int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key);
 int ndctl_dimm_wait_for_overwrite_completion(struct ndctl_dimm *dimm);
+int ndctl_dimm_update_master_passphrase(struct ndctl_dimm *dimm,
+		long ckey, long nkey);
 
 enum ndctl_key_type {
 	ND_USER_KEY,
 	ND_USER_OLD_KEY,
+	ND_MASTER_KEY,
+	ND_MASTER_OLD_KEY,
 };
 
 #ifdef ENABLE_KEYUTILS
-int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master);
-int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master);
+int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master_key,
+		enum ndctl_key_type key_type);
+int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master_key,
+		enum ndctl_key_type key_type);
 int ndctl_dimm_disable_key(struct ndctl_dimm *dimm);
 int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm);
 int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm);
 #else
 static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
-		const char *master)
+		const char *master_key, enum ndctl_key_type key_type);
 {
 	return -EOPNOTSUPP;
 }
 
 static inline int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
-		const char *master)
+		const char *master_key, enum ndctl_key_type key_type);
 {
 	return -EOPNOTSUPP;
 }

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

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

* [PATCH v6 11/12] ndctl: add master secure erase support
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
                   ` (9 preceding siblings ...)
  2018-12-14 21:09 ` [PATCH v6 10/12] ndctl: master phassphrase management support Dave Jiang
@ 2018-12-14 21:10 ` Dave Jiang
  2018-12-14 21:10 ` [PATCH v6 12/12] ndctl: documentation for security and key management Dave Jiang
  11 siblings, 0 replies; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:10 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Intel DSM v1.8 introduced the concept of master passphrase and allowing
nvdimm to be secure erased via the master passphrase in addition to the
user passphrase. Add ndctl support to provide master passphrase secure
erase.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/ndctl-sanitize-dimm.txt |    6 +++++
 ndctl/dimm.c                                |   14 +++++++++++--
 ndctl/lib/dimm.c                            |    9 ++++++++
 ndctl/lib/keys.c                            |   30 ++++++++++++++++++++-------
 ndctl/lib/libndctl.sym                      |    1 +
 ndctl/libndctl.h                            |    7 +++++-
 6 files changed, 55 insertions(+), 12 deletions(-)

diff --git a/Documentation/ndctl/ndctl-sanitize-dimm.txt b/Documentation/ndctl/ndctl-sanitize-dimm.txt
index beb4b2f9..7b036318 100644
--- a/Documentation/ndctl/ndctl-sanitize-dimm.txt
+++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
@@ -33,4 +33,10 @@ include::xable-dimm-options.txt[]
 --ovewrite::
 	Wipe the entire DIMM, including label data. Can take significant time.
 
+-M::
+--master_passphrase::
+	Parameter to indicate that we are managing the master passphrase
+	instead of the user passphrase. This only is applicable to the
+	crypto-erase option.
+
 include::../copyright.txt[]
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index c60ef96e..b2d50e28 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -907,6 +907,12 @@ static int action_sanitize_dimm(struct ndctl_dimm *dimm,
 		return -EOPNOTSUPP;
 	}
 
+	if (param.overwrite && param.master_pass) {
+		error("%s: overwrite does not support master passphrase\n",
+				ndctl_dimm_get_devname(dimm));
+		return -EINVAL;
+	}
+
 	/*
 	 * Setting crypto erase to be default. The other method will be
 	 * overwrite.
@@ -917,7 +923,9 @@ static int action_sanitize_dimm(struct ndctl_dimm *dimm,
 	}
 
 	if (param.crypto_erase) {
-		rc = ndctl_dimm_secure_erase_key(dimm);
+		rc = ndctl_dimm_secure_erase_key(dimm,
+				param.master_pass ?
+				ND_MASTER_KEY : ND_USER_KEY);
 		if (rc < 0)
 			return rc;
 	}
@@ -1048,7 +1056,9 @@ OPT_BOOLEAN('M', "master-passphrase", &param.master_pass, \
 OPT_BOOLEAN('c', "crypto-erase", &param.crypto_erase, \
 		"crypto erase a dimm"), \
 OPT_BOOLEAN('o', "overwrite", &param.overwrite, \
-		"overwrite a dimm")
+		"overwrite a dimm"), \
+OPT_BOOLEAN('M', "master-passphrase", &param.master_pass, \
+		"use master passphrase")
 
 static const struct option read_options[] = {
 	BASE_OPTIONS(),
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 07513b4b..a8013e4b 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -777,3 +777,12 @@ NDCTL_EXPORT int ndctl_dimm_update_master_passphrase(struct ndctl_dimm *dimm,
 	sprintf(buf, "master_update %ld %ld\n", ckey, nkey);
 	return write_security(dimm, buf);
 }
+
+NDCTL_EXPORT int ndctl_dimm_master_secure_erase(struct ndctl_dimm *dimm,
+		long key)
+{
+	char buf[SYSFS_ATTR_SIZE];
+
+	sprintf(buf, "master_erase %ld\n", key);
+	return write_security(dimm, buf);
+}
diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
index 1d395b48..51532c7a 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -437,13 +437,14 @@ NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
 }
 
 static int check_key_run_and_discard(struct ndctl_dimm *dimm,
-		int (*run_op)(struct ndctl_dimm *, long), const char *name)
+		int (*run_op)(struct ndctl_dimm *, long), const char *name,
+		enum ndctl_key_type key_type)
 {
 	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
 	key_serial_t key;
 	int rc;
 
-	key = dimm_check_key(dimm, ND_USER_KEY);
+	key = dimm_check_key(dimm, key_type);
 	if (key < 0) {
 		key = dimm_load_key(dimm, ND_USER_KEY);
 		if (key < 0 && run_op != ndctl_dimm_overwrite) {
@@ -460,8 +461,12 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
 		return rc;
 	}
 
+	/* we do not delete the key if master secure erase */
+	if (key_type == ND_MASTER_KEY)
+		return 0;
+
 	if (key) {
-		rc = dimm_remove_key(dimm, ND_USER_KEY);
+		rc = dimm_remove_key(dimm, key_type);
 		if (rc < 0)
 			err(ctx, "Unable to cleanup key.\n");
 	}
@@ -471,17 +476,26 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
 NDCTL_EXPORT int ndctl_dimm_disable_key(struct ndctl_dimm *dimm)
 {
 	return check_key_run_and_discard(dimm, ndctl_dimm_disable_passphrase,
-			"disable passphrase");
+			"disable passphrase", ND_USER_KEY);
 }
 
-NDCTL_EXPORT int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm)
+NDCTL_EXPORT int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
+		enum ndctl_key_type key_type)
 {
-	return check_key_run_and_discard(dimm, ndctl_dimm_secure_erase,
-			"crypto erase");
+	if (key_type == ND_MASTER_KEY)
+		return check_key_run_and_discard(dimm,
+				ndctl_dimm_master_secure_erase,
+				"master crypto erase", key_type);
+	else if (key_type == ND_USER_KEY)
+		return check_key_run_and_discard(dimm,
+				ndctl_dimm_secure_erase,
+				"crypto erase", key_type);
+	else
+		return -EINVAL;
 }
 
 NDCTL_EXPORT int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm)
 {
 	return check_key_run_and_discard(dimm, ndctl_dimm_overwrite,
-			"overwrite");
+			"overwrite", ND_USER_KEY);
 }
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index bd933eb2..f51f82fe 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -402,4 +402,5 @@ global:
 	ndctl_dimm_overwrite_key;
 	ndctl_dimm_wait_for_overwrite_completion;
 	ndctl_dimm_update_master_passphrase;
+	ndctl_dimm_master_secure_erase;
 } LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index a0b8a886..9b9fd34c 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -709,6 +709,7 @@ int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key);
 int ndctl_dimm_wait_for_overwrite_completion(struct ndctl_dimm *dimm);
 int ndctl_dimm_update_master_passphrase(struct ndctl_dimm *dimm,
 		long ckey, long nkey);
+int ndctl_dimm_master_secure_erase(struct ndctl_dimm *dimm, long key);
 
 enum ndctl_key_type {
 	ND_USER_KEY,
@@ -723,7 +724,8 @@ int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master_key,
 int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master_key,
 		enum ndctl_key_type key_type);
 int ndctl_dimm_disable_key(struct ndctl_dimm *dimm);
-int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm);
+int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
+		enum ndctl_key_type key_type);
 int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm);
 #else
 static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
@@ -743,7 +745,8 @@ static inline int ndctl_dimm_disable_key(struct ndctl_dimm *dimm)
 	return -EOPNOTSUPP;
 }
 
-static inline int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm)
+static inline int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
+		enum ndctl_key_type key_type);
 {
 	return -EOPNOTSUPP;
 }

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

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

* [PATCH v6 12/12] ndctl: documentation for security and key management
  2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
                   ` (10 preceding siblings ...)
  2018-12-14 21:10 ` [PATCH v6 11/12] ndctl: add master secure erase support Dave Jiang
@ 2018-12-14 21:10 ` Dave Jiang
  2019-01-05  2:31   ` Verma, Vishal L
  11 siblings, 1 reply; 19+ messages in thread
From: Dave Jiang @ 2018-12-14 21:10 UTC (permalink / raw)
  To: vishal.l.verma., dan.j.williams; +Cc: linux-nvdimm

Adding the theory of operation for Intel DSM operations.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/ndctl/intel-nvdimm-security.txt    |  139 ++++++++++++++++++++++
 Documentation/ndctl/ndctl-disable-passphrase.txt |    2 
 Documentation/ndctl/ndctl-enable-passphrase.txt  |    4 +
 Documentation/ndctl/ndctl-freeze-security.txt    |    2 
 Documentation/ndctl/ndctl-sanitize-dimm.txt      |    2 
 Documentation/ndctl/ndctl-update-passphrase.txt  |    2 
 6 files changed, 151 insertions(+)
 create mode 100644 Documentation/ndctl/intel-nvdimm-security.txt

diff --git a/Documentation/ndctl/intel-nvdimm-security.txt b/Documentation/ndctl/intel-nvdimm-security.txt
new file mode 100644
index 00000000..0c9a41a4
--- /dev/null
+++ b/Documentation/ndctl/intel-nvdimm-security.txt
@@ -0,0 +1,139 @@
+// SPDX-License-Identifier: GPL-2.0
+
+THEORY OF OPERATOIN
+-------------------
+With the introduction of Intel Device Specific Methods (DSM) specification
+v1.7 and v1.8 [1], security DSMs were introduced. Ndctl supports enable
+passhprase, update passphrase, disable security, freeze security, secure
+(crypto) erase, overwrite, master passphrase enable, master passphrase update,
+and master passphrase secure (crypto) erase. The DSM that is not supported by
+ndctl is the unlock DSM, which is left to the kernel to manage.
+
+The security management for nvdimm is composed of two parts. The front end
+utilizes the Linux key management framework (trusted and encrypted keys [2]).
+It uses the keyutils on the user side and Linux key management APIs in
+the kernel. The backend takes the decrypted payload of the key and passes the
+plaintext payload to the nvdimm for processing.
+
+Unlike typical DSMs, the security DSMs are managed through the 'security'
+sysfs attribute under the dimm devices rather than an ioctl call by libndctl.
+The relevant key id is written to the 'security' attribute and the kernel will
+pull that key from the kernel user key ring for processing.
+
+The entire security process starts with a master key that is used to seal the
+encrypted keys that are used to protect the passphrase for each nvdimm. We
+recommend using THE system master trusted key from the Trusted Platform
+Module (TPM). However, a trusted master key generated by the TPM can also
+be used. And for testing purposes, a user key with randomized payload can
+also be served as a master key. See [2] for details. To perform any security
+operations, it is expected that at the minimal the master key is already
+in the kernel user keyring as shown in example below:
+
+> keyctl show
+Session Keyring
+ 736023423 --alswrv      0     0  keyring: _ses
+ 675104189 --alswrv      0 65534   \_ keyring: _uid.0
+ 680187394 --alswrv      0     0       \_ trusted: nvdimm-master
+
+All operations expect the relevant regions associated to the nvdimm are
+disabled before proceeding, except overwrite. Overwrite expects the
+relevant nvdimm also be disabled.
+
+The follow on sections describe specifics of each security features.
+
+UNLOCK
+------
+Unlock is performed by the kernel, however a preparation step must happen
+before the unlock DSM can be issued by the kernel. The expectation is that
+during initramfs, a setup script is called before the libnvdimm module is
+loaded by modprobe. The said script will inject the master key and the
+related encrypted keys into the kernel user key ring. A reference modprobe
+config file and a setup script have been provided by ndctl. When the
+nvdimm driver probes, it will
+1. First, Check the security state of the device and see if the dimm is locked
+2. Request the associated encrypted key from the kernel user key ring.
+3. Finally, create the unlock DSM and copy the decrypted payload into the DSM
+   passphrase field, and issue the DSM to unlock the DIMM.
+
+If the DIMM is already unlocked, the kernel will attempt to revalidate the key.
+This can be overriden with a kernel module parameter. If we fail to revalidate
+the key, the kernel will freeze the security and disallow any further security
+configuration changes.
+
+ENABLE USER PASSPHRASE
+----------------------
+To enable the user passphrase for a DIMM, it is expected that the master key
+is already in the kernel user key ring and the master key name is passed to
+ndctl so it can do key management. An encrypted key with a 32byte payload
+and encrypted key format 'enc32' is created and sealed by the master key. Be
+aware that the passphrase is never provided by or visible to the user.
+The decrypted payload for the encrypted key will be randomly generated by the
+kernel and userspace does not have access to this decrypted payload. When the
+encrypted key is created, a binary blob of the encrypted key is written to the
+designated key blob storage directory (/etc/ndctl/keys as default). The user is
+responsible for backing up the dimm key blobs to a secure location. When a key
+is successfully loaded into the kernel user key ring, the payload is decrypted
+and can be accessed by the kernel.
+
+Key ids are written to the 'security' sysfs attribute with the command "update".
+A key id of 0 is provided for the old key id. The kernel will see that the
+update call is an enable call because the 0 old key id. A "passphrase update"
+DSM is issued by the kernel with the old passphrase as 0s.
+
+UPDATE USER PASSPHRASE
+----------------------
+The update user passphrase operation uses the same DSM command as enable user
+passphrase. Most of the work is done on the key management. The user will
+provide a new master key name for the new passphrase key or use the existing
+one. Ndctl will use whatever master key name that is passed in. The following
+operation is performed for update:
+1. Remove the associated encrypted key from the kernel user key ring.
+2. Rename the key blob to old.
+3. Load the now old key blob into kernel user key ring with "old" name.
+4. Create new encrypted key and seal with master key.
+5. Generate new key blob.
+6. Send DSM with old and new passphrase via the decrypted key payloads.
+7. On success, remove old key from key ring and old key blob.
+
+DISABLE USER PASSPHRASE
+-----------------------
+Ndctl will look up the key id for the current associated key and write to sysfs.
+Upon success of disabling, the key will be removed from the kernel user key
+ring and the related key blob will also be deleted.
+
+CRYPTO (SECURE) ERASE
+---------------------
+This operation is similar to disable the passphrase. The kernel will issue
+wbinvd before and after the operation to ensure no data corruption from stale
+CPU cache. The "sanitize-dimm" command with the --crypto-erase switch is used
+via ndctl.
+
+OVERWRITE
+---------
+The overwrite operation wipes the entire nvdimm. Therefore it can take
+a significant amount of time. To issue the command is very similar to disable
+passphrase and secure erase. However, when the command returns successfully
+it just means overwrite has been successfully started. The wait-overwrite
+command for ndctl can be used to wait on the nvdimms that are performing
+overwrite until the operation is completed. Upon successful completion of the
+operation, wbinvd will be issued by the kernel. The "sanitize-dimm" command
+with the --overwrite switch is used via ndctl. If both --crypto-erase and
+--overwrite switches are passed in, the crypt-erase will be issued first before
+overwrite.
+
+SECURITY FREEZE
+---------------
+This operation requires no key to succeed. Ndctl will issue the DSM command
+and upon completion, the security commands besides status query will be locked
+out until the next boot.
+
+MASTER PASSPHRASE ENABLE, UPDATE, and CRYPTO ERASE
+-----------------------------------------------------------
+These operations are similar to the user passphrase enable and update. The only
+difference is that a different encrypted key is being used for the master
+passphrase operations. Note that master passphrase has no relation to the
+master key for encryption.
+
+
+[1] http://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf
+[2] https://www.kernel.org/doc/Documentation/security/keys/trusted-encrypted.rst
diff --git a/Documentation/ndctl/ndctl-disable-passphrase.txt b/Documentation/ndctl/ndctl-disable-passphrase.txt
index e921eb23..df7401cb 100644
--- a/Documentation/ndctl/ndctl-disable-passphrase.txt
+++ b/Documentation/ndctl/ndctl-disable-passphrase.txt
@@ -24,4 +24,6 @@ OPTIONS
 <dimm>::
 include::xable-dimm-options.txt[]
 
+include::intel-nvdimm-security.txt[]
+
 include::../copyright.txt[]
diff --git a/Documentation/ndctl/ndctl-enable-passphrase.txt b/Documentation/ndctl/ndctl-enable-passphrase.txt
index 6639ce8d..9ff19927 100644
--- a/Documentation/ndctl/ndctl-enable-passphrase.txt
+++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
@@ -31,10 +31,14 @@ include::xable-dimm-options.txt[]
 -m::
 --master-key=::
 	Key name for the master key used to seal the NVDIMM security keys.
+	The format would be <key_type>:<master_key_name>
+	i.e.: trusted:master-nvdimm
 
 -M::
 --master-passphrase::
 	Parameter to indicate that we are managing the master passphrase
 	instead of the user passphrase.
 
+include::intel-nvdimm-security.txt[]
+
 include::../copyright.txt[]
diff --git a/Documentation/ndctl/ndctl-freeze-security.txt b/Documentation/ndctl/ndctl-freeze-security.txt
index 4e9d2d61..2ae21980 100644
--- a/Documentation/ndctl/ndctl-freeze-security.txt
+++ b/Documentation/ndctl/ndctl-freeze-security.txt
@@ -17,4 +17,6 @@ DESCRIPTION
 Provide a generic interface to freeze the security for NVDIMM. Once security
 is frozen, no other security operations can succeed until reboot happens.
 
+include::intel-nvdimm-security.txt[]
+
 include::../copyright.txt[]
diff --git a/Documentation/ndctl/ndctl-sanitize-dimm.txt b/Documentation/ndctl/ndctl-sanitize-dimm.txt
index 7b036318..a8cdca71 100644
--- a/Documentation/ndctl/ndctl-sanitize-dimm.txt
+++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
@@ -39,4 +39,6 @@ include::xable-dimm-options.txt[]
 	instead of the user passphrase. This only is applicable to the
 	crypto-erase option.
 
+include::intel-nvdimm-security.txt[]
+
 include::../copyright.txt[]
diff --git a/Documentation/ndctl/ndctl-update-passphrase.txt b/Documentation/ndctl/ndctl-update-passphrase.txt
index e2ecacf5..046fac6c 100644
--- a/Documentation/ndctl/ndctl-update-passphrase.txt
+++ b/Documentation/ndctl/ndctl-update-passphrase.txt
@@ -35,4 +35,6 @@ include::xable-dimm-options.txt[]
 	Parameter to indicate that we are managing the master passphrase
 	instead of the user passphrase.
 
+include::intel-nvdimm-security.txt[]
+
 include::../copyright.txt[]

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

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

* Re: [PATCH v6 02/12] ndctl: add passphrase update to ndctl
  2018-12-14 21:09 ` [PATCH v6 02/12] ndctl: add passphrase update to ndctl Dave Jiang
@ 2019-01-05  0:07   ` Verma, Vishal L
  0 siblings, 0 replies; 19+ messages in thread
From: Verma, Vishal L @ 2019-01-05  0:07 UTC (permalink / raw)
  To: Williams, Dan J, Jiang, Dave, Verma, Vishal L; +Cc: linux-nvdimm


On Fri, 2018-12-14 at 14:09 -0700, Dave Jiang wrote:
> Add API call for triggering sysfs knob to update the security for a DIMM
> in libndctl. Also add the ndctl "update-passphrase" to trigger the
> operation.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Documentation/ndctl/Makefile.am                 |    4 
>  Documentation/ndctl/ndctl-enable-passphrase.txt |   35 ++
>  Documentation/ndctl/ndctl-update-passphrase.txt |   33 ++
>  builtin.h                                       |    2 

This and a few other files conflict with Dan's recent monitor/builtin
cleanups. Would you mind rebasing upon those? I've pushed those out in
the pending branch.

>  configure.ac                                    |   14 +
>  ndctl.spec.in                                   |    2 
>  ndctl/Makefile.am                               |    3 
>  ndctl/dimm.c                                    |   86 ++++-
>  ndctl/lib/Makefile.am                           |    8 
>  ndctl/lib/dimm.c                                |   39 ++
>  ndctl/lib/keys.c                                |  389 +++++++++++++++++++++++
>  ndctl/lib/libndctl.sym                          |    4 
>  ndctl/libndctl.h                                |   31 ++
>  ndctl/ndctl.c                                   |    2 
>  14 files changed, 639 insertions(+), 13 deletions(-)
>  create mode 100644 Documentation/ndctl/ndctl-enable-passphrase.txt
>  create mode 100644 Documentation/ndctl/ndctl-update-passphrase.txt
>  create mode 100644 ndctl/lib/keys.c
> 
> diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
> index a30b139b..7ad6666b 100644
> --- a/Documentation/ndctl/Makefile.am
> +++ b/Documentation/ndctl/Makefile.am
> @@ -47,7 +47,9 @@ man1_MANS = \
>  	ndctl-inject-smart.1 \
>  	ndctl-update-firmware.1 \
>  	ndctl-list.1 \
> -	ndctl-monitor.1
> +	ndctl-monitor.1 \
> +	ndctl-enable-passphrase.1 \
> +	ndctl-update-passphrase.1
>  
>  CLEANFILES = $(man1_MANS)
>  
> diff --git a/Documentation/ndctl/ndctl-enable-passphrase.txt b/Documentation/ndctl/ndctl-enable-passphrase.txt
> new file mode 100644
> index 00000000..8de5410c
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
> @@ -0,0 +1,35 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +ndctl-enable-passphrase(1)
> +==========================
> +
> +NAME
> +----
> +ndctl-enable-passphrase - enabling the security passphrase for a NVDIMM
                             ^enable
for consistency

> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl enable-passphrase' <dimm> [<options>]
> +
> +DESCRIPTION
> +-----------
> +Provide a generic interface for enabling security for NVDIMM.

Consider changing to:
Provide a generic interface to enable the security passphrase for the
NVDIMM.

Also, "Provide a generic interface to" can probably be replaced by
"Command to"

Same comments apply to the update-passphrase man page below.

> +It is expected that the master key has already been loaded into the user
> +key ring. The encrypted key blobs will be created in /etc/nvdimm directory
> +with the file name of "nvdimm-<hostname>-<dimm unique id>.blob".
> +
> +The command will fail if the nvdimm key is already in the user key ring and/or
> +the key blob already resides in /etc/nvdimm. Do not touch the /etc/nvdimm
> +directory and let ndctl manage the keys, unless you know what you are doing.
> +
> +OPTIONS
> +-------
> +<dimm>::
> +include::xable-dimm-options.txt[]
> +
> +-m::
> +--master=::
> +	Key name for the master key used to seal the NVDIMM security keys.
> +
> +include::../copyright.txt[]
> diff --git a/Documentation/ndctl/ndctl-update-passphrase.txt b/Documentation/ndctl/ndctl-update-passphrase.txt
> new file mode 100644
> index 00000000..9ed39cca
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-update-passphrase.txt
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +ndctl-update-passphrase(1)
> +==========================
> +
> +NAME
> +----
> +ndctl-update-passphrase - update the security passphrase for a NVDIMM
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl update-passphrase' <dimm> [<options>]
> +
> +DESCRIPTION
> +-----------
> +Provide a generic interface for updating security key for NVDIMM.
> +It is expected that the current and the new (if new master key is desired)
> +master key has already been loaded into the user key ring. The new encrypted
> +key blobs will be created in /etc/nvdimm directory
> +with the file name of "nvdimm-<hostname>-<dimm unique id>.blob".
> +
> +OPTIONS
> +-------
> +<dimm>::
> +include::xable-dimm-options.txt[]
> +
> +-m::
> +--master::
> +	New key name for the master key to seal the new nvdimm key, or the
> +	existing master key name. i.e trusted:master-key.
> +
> +include::../copyright.txt[]
> diff --git a/builtin.h b/builtin.h
> index 675a6ce7..ed018d96 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -48,4 +48,6 @@ int cmd_bat(int argc, const char **argv, void *ctx);
>  #endif
>  int cmd_update_firmware(int argc, const char **argv, void *ctx);
>  int cmd_inject_smart(int argc, const char **argv, void *ctx);
> +int cmd_passphrase_setup(int argc, const char **argv, void *ctx);
> +int cmd_passphrase_update(int argc, const char **argv, void *ctx);
>  #endif /* _NDCTL_BUILTIN_H_ */
> diff --git a/configure.ac b/configure.ac
> index bb6b0332..ac4f56c3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -153,6 +153,20 @@ fi
>  AC_SUBST([systemd_unitdir])
>  AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
>  
> +AC_ARG_WITH([keyutils],
> +	    AS_HELP_STRING([--with-keyutils],
> +			[Enable keyutils functionality (security).  @<:@default=yes@:>@]), [], [with_keyutils=yes])
> +
> +if test "x$with_keyutils" = "xyes"; then
> +	AC_CHECK_HEADERS([keyutils.h],,[
> +		AC_MSG_ERROR([keyutils.h not found, consider installing
> +			      keyutils-libs-devel.])
> +		])
> +fi
> +AS_IF([test "x$with_keyutils" = "xyes"],
> +	[AC_DEFINE([ENABLE_KEYUTILS], [1], [Enable keyutils support])])
> +AM_CONDITIONAL([ENABLE_KEYUTILS], [test "x$with_keyutils" = "xyes"])
> +
>  my_CFLAGS="\
>  -D DEF_CONF_FILE='\"${sysconfdir}/ndctl/monitor.conf\"' \
>  -Wall \

We should be passing the /etc/ndctl/keys path here. Once you rebase
upon Dan's recent reworks, see how monitor.conf is now setup.

> diff --git a/ndctl.spec.in b/ndctl.spec.in
> index 26396d4a..66466db6 100644
> --- a/ndctl.spec.in
> +++ b/ndctl.spec.in
> @@ -21,6 +21,7 @@ BuildRequires:	pkgconfig(uuid)
>  BuildRequires:	pkgconfig(json-c)
>  BuildRequires:	pkgconfig(bash-completion)
>  BuildRequires:	pkgconfig(systemd)
> +BuildRequires:	keyutils-libs-devel
>  
>  %description
>  Utility library for managing the "libnvdimm" subsystem.  The "libnvdimm"
> @@ -119,6 +120,7 @@ make check
>  %{bashcompdir}/
>  %{_sysconfdir}/ndctl/monitor.conf
>  %{_unitdir}/ndctl-monitor.service
> +%{_sysconfdir}/ndctl/keys/
>  
>  %files -n daxctl
>  %defattr(-,root,root)
> 

<...>

> diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
> new file mode 100644
> index 00000000..a005c6a6
> --- /dev/null
> +++ b/ndctl/lib/keys.c
> @@ -0,0 +1,389 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
> +
> +#include <stdio.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <sys/param.h>
> +#include <keyutils.h>
> +#include <syslog.h>
> +
> +#include <ndctl.h>
> +#include <ndctl/libndctl.h>
> +#include "private.h"
> +
> +#define PATH_SIZE	512
> +#define HOSTNAME_SIZE	64
> +#define DESC_SIZE	128
> +#define KEY_CMD_SIZE	128
> +
> +static int get_key_path(struct ndctl_dimm *dimm, char *path,
> +		enum ndctl_key_type key_type)
> +{
> +	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +	char hostname[HOSTNAME_SIZE];
> +	int rc;
> +
> +	rc = gethostname(hostname, HOSTNAME_SIZE-1);

Spaces around the '-'

Also do we need to subtract 1? The man page says "gethostname() returns
the null-terminated hostname in the character array name, which has a
length of len bytes." Which means if the array was 64B it will use all
of it to fill in the hostname + NUL.

And finally, why redefine HOSTNAME_SIZE, instead of using
HOST_NAME_MAX. Perhaps malloc it since we don't know its size.

> +	if (rc < 0) {
> +		err(ctx, "gethostname: %s\n", strerror(errno));
> +		return -errno;
> +	}
> +
> +	if (key_type == ND_USER_OLD_KEY) {
> +		rc = sprintf(path, "%s/nvdimmold_%s_%s.blob",
> +				ND_PASS_PATH,
> +				ndctl_dimm_get_unique_id(dimm),
> +				hostname);
> +	} else {
> +		rc = sprintf(path, "%s/nvdimm_%s_%s.blob",
> +				ND_PASS_PATH,
> +				ndctl_dimm_get_unique_id(dimm),
> +				hostname);
> +	}
> +
> +	if (rc < 0) {
> +		err(ctx, "sprintf: %s\n", strerror(errno));

This applies to everywhere else - an error starting with "sprintf: " is
very debug centric, i.e. a user may not know what to make of it. It
might be friendlier to describe what went wrong. So in this case:

		err(ctx, "error setting path: %s\n", strerror(errno));

> +		return -errno;
> +	}
> +
> +	return 0;
> +}
> +
> +static int get_key_desc(struct ndctl_dimm *dimm, char *desc,
> +		enum ndctl_key_type key_type)
> +{
> +	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +	int rc;
> +
> +	if (key_type == ND_USER_OLD_KEY)
> +		rc = sprintf(desc, "nvdimm-old:%s",
> +				ndctl_dimm_get_unique_id(dimm));
> +	else
> +		rc = sprintf(desc, "nvdimm:%s",
> +				ndctl_dimm_get_unique_id(dimm));
> +
> +	if (rc < 0) {
> +		err(ctx, "sprintf: %s\n", strerror(errno));
> +		return -errno;
> +	}
> +
> +	return 0;
> +}
> +
> +static char *load_key_blob(struct ndctl_ctx *ctx, const char *path, int *size)
> +{
> +	struct stat st;
> +	FILE *bfile = NULL;
> +	ssize_t read;
> +	int rc;
> +	char *blob, *pl;
> +	char prefix[] = "load ";
> +
> +	rc = stat(path, &st);
> +	if (rc < 0) {
> +		err(ctx, "stat: %s\n", strerror(errno));
> +		return NULL;
> +	}
> +	if ((st.st_mode & S_IFMT) != S_IFREG) {
> +		err(ctx, "%s not a regular file\n", path);
> +		return NULL;
> +	}
> +
> +	if (st.st_size == 0 || st.st_size > 4096) {
> +		err(ctx, "Invalid blob file size\n");
> +		return NULL;
> +	}
> +
> +	*size = st.st_size + sizeof(prefix) - 1;
> +	blob = malloc(*size);
> +	if (!blob) {
> +		err(ctx, "Unable to allocate memory for blob\n");
> +		return NULL;
> +	}
> +
> +	bfile = fopen(path, "r");
> +	if (!bfile) {
> +		err(ctx, "Unable to open %s: %s\n", path, strerror(errno));
> +		free(blob);
> +		return NULL;
> +	}
> +
> +	memcpy(blob, prefix, sizeof(prefix) - 1);
> +	pl = blob + sizeof(prefix) - 1;
> +	read = fread(pl, st.st_size, 1, bfile);
> +	if (read < 0) {
> +		err(ctx, "Fail to read from blob file: %s\n",
			  ^Failed

> +				strerror(errno));
> +		free(blob);
> +		fclose(bfile);
> +		return NULL;
> +	}
> +
> +	fclose(bfile);
> +	return blob;
> +}
> +
> +static key_serial_t dimm_check_key(struct ndctl_dimm *dimm,
> +		enum ndctl_key_type key_type)
> +{
> +	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +	char desc[DESC_SIZE];
> +	int rc;
> +
> +	rc = get_key_desc(dimm, desc, key_type);
> +	if (rc < 0) {
> +		err(ctx, "sprintf: %d\n", rc);
> +		return rc;
> +	}

get_key_desc also prints the same error message, so this seems
redundant. This seems to be the only spot for this call with an extra
print.

> +
> +	return keyctl_search(KEY_SPEC_USER_KEYRING, "encrypted", desc, 0);
> +}
> +
> +static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
> +		const char *master)
> +{
> +	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +	char desc[DESC_SIZE];
> +	char path[PATH_SIZE];
> +	char cmd[KEY_CMD_SIZE];
> +	key_serial_t key;
> +	void *buffer;
> +	int rc;
> +	ssize_t size;
> +	FILE *fp;
> +	ssize_t wrote;
> +	struct stat st;
> +
> +	if (ndctl_dimm_is_active(dimm)) {
> +		err(ctx, "regions active on %s, op failed\n",
> +				ndctl_dimm_get_devname(dimm));
> +		return -EBUSY;
> +	}
> +
> +	rc = get_key_desc(dimm, desc, ND_USER_KEY);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* make sure it's not already in the key ring */
> +	key = keyctl_search(KEY_SPEC_USER_KEYRING, "encrypted", desc, 0);
> +	if (key > 0) {
> +		err(ctx, "key already in user keyring, can't create new.\n");

Perhaps just "Error: key is already present in the user keyring."

> +		return -EAGAIN;

Why EAGAIN? EAGAIN usually means "try again later", which doesn't make
sense in this case. EEXIST might be a better option. Same below.

> +	}
> +
> +	rc = get_key_path(dimm, path, ND_USER_KEY);
> +	if (rc < 0)
> +		return rc;
> +
> +	rc = stat(path, &st);
> +	if (rc == 0) {
> +		err(ctx, "%s already exists!\n", path);
> +		return -EAGAIN;
> +	}
> +
> +	rc = sprintf(cmd, "new enc32 %s 32", master);
> +	if (rc < 0) {
> +		err(ctx, "sprintf: %s\n", strerror(errno));
> +		return -errno;
> +	}
> +
> +	key = add_key("encrypted", desc, cmd, strlen(cmd),
> +			KEY_SPEC_USER_KEYRING);
> +	if (key < 0) {
> +		err(ctx, "add_key failed: %s\n", strerror(errno));
> +		return -errno;
> +	}
> +
> +	size = keyctl_read_alloc(key, &buffer);
> +	if (size < 0) {
> +		err(ctx, "keyctl_read_alloc failed: %ld\n", size);
> +		keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
> +		return rc;
> +	}
> +
> +	fp = fopen(path, "w");
> +	if (!fp) {
> +		rc = -errno;
> +		err(ctx, "fopen: %s\n", strerror(errno));

Perhaps print the error similar to the other fopen in this file - i.e.
"Unable to open %s: %s", path, strerror...

> +		free(buffer);
> +		return rc;
> +	}
> +
> +	 wrote = fwrite(buffer, size, 1, fp);

We're writing a character stream here yes? If so, we should make the
'size' argument '1', and 'nitems' as the number of characters (buffer
len). fwrite will return the number of items written, which could be
less than 'nitems', so we should subsequently check for (wrote <
nitems) to catch a partial write.

> +	 if (wrote < 0) {
> +		 rc = -errno;
> +		 err(ctx, "fwrite failed: %s\n", strerror(errno));

Similar to the cases, "Failed to write to <path>: error-string" is more
user friendly.

> +		 free(buffer);
> +		 return rc;
> +	 }
> +
> +	 fclose(fp);
> +	 free(buffer);
> +	 return key;
> +}
> +
> +static key_serial_t dimm_load_key(struct ndctl_dimm *dimm,
> +		enum ndctl_key_type key_type)
> +{
> +	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +	key_serial_t key;
> +	char desc[DESC_SIZE];
> +	char path[PATH_SIZE];
> +	int rc;
> +	char *blob;
> +	int size;
> +
> +	if (ndctl_dimm_is_active(dimm)) {
> +		err(ctx, "regions active on %s, op failed\n",
> +				ndctl_dimm_get_devname(dimm));
> +		return -EBUSY;
> +	}
> +
> +	rc = get_key_desc(dimm, desc, key_type);
> +	if (rc < 0)
> +		return rc;
> +
> +	rc = get_key_path(dimm, path, key_type);
> +	if (rc < 0)
> +		return rc;
> +
> +	blob = load_key_blob(ctx, path, &size);
> +	if (!blob)
> +		return -ENOMEM;
> +
> +	key = add_key("encrypted", desc, blob, size, KEY_SPEC_USER_KEYRING);
> +	free(blob);
> +	if (key < 0) {
> +		err(ctx, "add_key failed: %s\n", strerror(errno));
> +		return -errno;
> +	}
> +
> +	return key;
> +}
> +
> +/*
> + * The function will check to see if the existing key is there and remove
> + * from user key ring if it is. Rename the existing key blob to old key
> + * blob, and then attempt to inject the key as old key into the user key
> + * ring.
> + */
> +static key_serial_t move_key_to_old(struct ndctl_dimm *dimm)
> +{
> +	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +	int rc;
> +	key_serial_t key;
> +	char old_path[PATH_SIZE];
> +	char new_path[PATH_SIZE];
> +
> +	if (ndctl_dimm_is_active(dimm)) {
> +		err(ctx, "regions active on %s, op failed\n",
> +				ndctl_dimm_get_devname(dimm));
> +		return -EBUSY;
> +	}
> +
> +	key = dimm_check_key(dimm, ND_USER_KEY);
> +	if (key > 0)
> +		keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
> +
> +	rc = get_key_path(dimm, old_path, ND_USER_KEY);
> +	if (rc < 0)
> +		return rc;
> +
> +	rc = get_key_path(dimm, new_path, ND_USER_OLD_KEY);
> +	if (rc < 0)
> +		return rc;
> +
> +	rc = rename(old_path, new_path);

Probably add an err() here printing the strerror() for why the rename
failed.

> +	if (rc < 0)
> +		return -errno;
> +
> +	return dimm_load_key(dimm, ND_USER_OLD_KEY);
> +}
> +
> +static int dimm_remove_key(struct ndctl_dimm *dimm,
> +		enum ndctl_key_type key_type)
> +{
> +	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +	key_serial_t key;
> +	char path[PATH_SIZE];
> +	int rc;
> +
> +	key = dimm_check_key(dimm, key_type);
> +	if (key > 0)
> +		keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
> +
> +	rc = get_key_path(dimm, path, key_type);
> +	if (rc < 0) {
> +		err(ctx, "get key file path failed: %d\n", rc);
> +		return rc;
> +	}

get_key_path already prints the error.

> +
> +	rc = unlink(path);
> +	if (rc < 0) {
> +		err(ctx, "unlink: %s\n", strerror(errno));

Add 'path' into the error message.

> +		return -errno;
> +	}
> +
> +	return 0;
> +}
> +
> +NDCTL_EXPORT int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
> +		const char *master)
> +{
> +	key_serial_t key;
> +	int rc;
> +
> +	key = dimm_create_key(dimm, master);
> +	if (key < 0)
> +		return (int)key;

Is the typecast needed here?

> +
> +	rc = ndctl_dimm_update_passphrase(dimm, 0, key);
> +	if (rc < 0) {
> +		dimm_remove_key(dimm, ND_USER_KEY);
> +		return rc;
> +	}
> +
> +	return 0;
> +}
> +
> +NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
> +		const char *master)
> +{
> +	int rc;
> +	key_serial_t old_key, new_key;
> +
> +	/*
> +	 * 1. check if current key is loaded and remove
> +	 * 2. move current key blob to old key blob
> +	 * 3. load old key blob
> +	 * 4. trigger change key with old and key key

s/key key/new key/ ?

> +	 * 5. remove old key
> +	 * 6. remove old key blob
> +	 */
> +	old_key = move_key_to_old(dimm);
> +	if (old_key < 0)
> +		return old_key;
> +
> +	new_key = dimm_create_key(dimm, master);
> +	/* need to create new key here */
> +	if (new_key < 0) {
> +		new_key = dimm_load_key(dimm, ND_USER_KEY);
> +		if (new_key < 0)
> +			return new_key;
> +	}
> +
> +	rc = ndctl_dimm_update_passphrase(dimm, old_key, new_key);
> +	if (rc < 0)
> +		return rc;
> +
> +	rc = dimm_remove_key(dimm, ND_USER_OLD_KEY);
> +	if (rc < 0)
> +		return rc;
> +
> +	return 0;
> +}
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index 1bd63fa1..a790b1ea 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -389,4 +389,8 @@ global:
>  LIBNDCTL_19 {
>  global:
>  	ndctl_dimm_get_security;
> +	ndctl_dimm_security_supported;
> +	ndctl_dimm_enable_key;
> +	ndctl_dimm_update_key;
> +	ndctl_dimm_update_passphrase;
>  } LIBNDCTL_18;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index a9f9167a..00ec1907 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -19,6 +19,7 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <limits.h>
> +#include <keyutils.h>
>  
>  #ifdef HAVE_LIBUUID
>  #include <uuid/uuid.h>
> @@ -681,6 +682,11 @@ enum ND_FW_STATUS ndctl_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd);
>  struct ndctl_cmd *ndctl_dimm_cmd_new_ack_shutdown_count(struct ndctl_dimm *dimm);
>  int ndctl_dimm_fw_update_supported(struct ndctl_dimm *dimm);
>  
> +#define ND_PASSPHRASE_SIZE	32
> +#define ND_PASS_PATH		"/etc/ndctl/keys"

This should not be hard coded, should be derived from autoconf (I think
monitor now does this correctly, so could copy from there).

> +#define ND_KEY_DESC_LEN	22
> +#define ND_KEY_DESC_PREFIX  7
> +
>  enum nd_security_state {
>  	ND_SECURITY_INVALID = -1,
>  	ND_SECURITY_UNSUPPORTED = 0,
> @@ -693,6 +699,31 @@ enum nd_security_state {
>  
>  int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
>  		enum nd_security_state *sstate);
> +bool ndctl_dimm_security_supported(struct ndctl_dimm *dimm);
> +int ndctl_dimm_update_passphrase(struct ndctl_dimm *dimm,
> +		long ckey, long nkey);
> +
> +enum ndctl_key_type {
> +	ND_USER_KEY,
> +	ND_USER_OLD_KEY,
> +};
> +
> +#ifdef ENABLE_KEYUTILS
> +int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master);
> +int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master);
> +#else
> +static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
> +		const char *master)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static inline int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
> +		const char *master)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif
>  
>  #ifdef __cplusplus
>  } /* extern "C" */
> diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
> index 73dabfac..4336c94c 100644
> --- a/ndctl/ndctl.c
> +++ b/ndctl/ndctl.c
> @@ -88,6 +88,8 @@ static struct cmd_struct commands[] = {
>  	{ "inject-smart", cmd_inject_smart },
>  	{ "wait-scrub", cmd_wait_scrub },
>  	{ "start-scrub", cmd_start_scrub },
> +	{ "enable-passphrase", cmd_passphrase_setup },
> +	{ "update-passphrase", cmd_passphrase_update },
>  	{ "list", cmd_list },
>  	{ "monitor", cmd_monitor},
>  	{ "help", cmd_help },
> 
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

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

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

* Re: [PATCH v6 06/12] ndctl: add unit test for security ops (minus overwrite)
  2018-12-14 21:09 ` [PATCH v6 06/12] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
@ 2019-01-05  1:21   ` Verma, Vishal L
  0 siblings, 0 replies; 19+ messages in thread
From: Verma, Vishal L @ 2019-01-05  1:21 UTC (permalink / raw)
  To: Williams, Dan J, Jiang, Dave, Verma, Vishal L; +Cc: linux-nvdimm


On Fri, 2018-12-14 at 14:09 -0700, Dave Jiang wrote:
> Add unit test for security enable, disable, update, erase, unlock, and
> freeze.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  test/Makefile.am |    4 +
>  test/security.sh |  191 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 195 insertions(+)
>  create mode 100755 test/security.sh
> 
> diff --git a/test/Makefile.am b/test/Makefile.am
> index ebdd23f6..42009c31 100644
> --- a/test/Makefile.am
> +++ b/test/Makefile.am
> @@ -27,6 +27,10 @@ TESTS =\
>  	max_available_extent_ns.sh \
>  	pfn-meta-errors.sh
>  
> +if ENABLE_KEYUTILS
> +TESTS += security.sh
> +endif
> +
>  check_PROGRAMS =\
>  	libndctl \
>  	dsm-fail \
> diff --git a/test/security.sh b/test/security.sh
> new file mode 100755
> index 00000000..5238c9c4
> --- /dev/null
> +++ b/test/security.sh
> @@ -0,0 +1,191 @@
> +#!/bin/bash -Ex
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright(c) 2018 Intel Corporation. All rights reserved.
> +
> +rc=77
> +dev=""
> +id=""
> +dev_no=""
> +sstate=""
> +KEYPATH="/etc/ndctl/keys"
> +UNLOCK="/sys/devices/platform/nfit_test.0/nfit_test_dimm/test_dimm"
> +MASTERKEY="nvdimm-master-test"
> +MASTERPATH="$KEYPATH/$MASTERKEY"

Local-only variables don't need to be all uppercase. By convention,
only variables you export are uppercase.

> +
> +. ./common
> +
> +trap 'err $LINENO' ERR
> +
> +setup()
> +{
> +	$NDCTL disable-region -b $NFIT_TEST_BUS0 all

quote all "$expansions".
Generally, consider running the script through shellchek for styling
problems.

> +}
> +
> +detect()
> +{
> +	dev=$($NDCTL list -b $NFIT_TEST_BUS0 -D | jq .[0].dev | tr -d '"')
> +	[ -n "$dev" ] || err "$LINENO"
> +	id=$($NDCTL list -b $NFIT_TEST_BUS0 -D | jq .[0].id | tr -d '"')
> +	[ -n "$id" ] || err "$LINENO"
> +}
> +
> +setup_keys()
> +{
> +	if [ ! -f $MASTERPATH ]; then
> +		keyctl add user $MASTERKEY "`dd if=/dev/urandom bs=1 count=32 2>/dev/null`" @u

Applies everyehere in the script - prefer "$()" instead of backticks.
Also before using keyctl, lets make sure it is installed. test/common
provides a function for this - check_prereq()

> +		keyctl pipe `keyctl search @u user $MASTERKEY` > $MASTERPATH

Before the first use, would be a good idea to make sure keypath exists.

> +	else
> +		echo "Unclean setup. Please cleanup $MASTERPATH file."
> +		exit 1
> +	fi
> +}
> +
> +test_cleanup()
> +{
> +	keyctl unlink `keyctl search @u encrypted nvdimm:$id`
> +	keyctl unlink `keyctl search @u user $MASTERKEY`
> +	rm -f $KEYPATH/nvdimm_$id\_`hostname`.blob
> +	rm -f $MASTERPATH
> +}
> +
> +locking_dimm()

Do you mean "unlock_dimm()" ?

> +{
> +	$NDCTL disable-dimm $dev
> +	dev_no=$(echo $dev | cut -b 5-)
> +	echo 1 > "$UNLOCK$dev_no/lock_dimm"

Best to explicitly delineate each variable using ${}:
"${path}/${id}/lock_dimm"
Also 'unlock' isn't very descriptive, maybe 'unlock_path'? It also
contains "nfit_test.0" which should be obtained from the variable
"$NFIT_TEST_BUS0".


> +	get_security_state

See below

> +	if [ "$sstate" != "locked" ]; then
> +		echo "Incorrect security state: $sstate expected: disabled"
> +		exit 1
> +	fi
> +}
> +
> +get_security_state()
> +{
> +	sstate=$($NDCTL list -i -b $NFIT_TEST_BUS0 -d $dev | jq .[].dimms[0].security | tr -d '"')
> +	[ -n "$sstate" ] || err "$LINENO"

Avoid setting global variables, it is easy to lose track of what has
been set where. Instead make helper functions like this simply print
their result as the output. Then call them like so:
sstate="$(get_security_state)"
make the '-n' check the responsibility of the caller, and this function
could simply be:

	get_security_state()
	{
		$NDCTL list -i -b $NFIT_TEST_BUS0 -d $dev | jq .[].dimms[0].security | tr -d '"'
	}

In most of these cases, since you are checking for sstate to be exactly
"some-state", you don't even need a -n check.

> +}
> +
> +enable_passphrase()
> +{
> +	$NDCTL enable-passphrase -m user:$MASTERKEY $dev
> +	get_security_state
> +	if [ "$sstate" != "unlocked" ]; then
> +		echo "Incorrect security state: $sstate expected: unlocked"
> +		exit 1
> +	fi
> +}
> +
> +disable_passphrase()
> +{
> +	$NDCTL disable-passphrase $dev
> +	get_security_state
> +	if [ "$sstate" != "disabled" ]; then
> +		echo "Incorrect security state: $sstate expected: disabled"
> +		exit 1
> +	fi
> +}
> +
> +erase_security()
> +{
> +	$NDCTL sanitize-dimm -c $dev
> +	get_security_state
> +	if [ "$sstate" != "disabled" ]; then
> +		echo "Incorrect security state: $sstate expected: disabled"
> +		exit 1
> +	fi
> +}
> +
> +update_security()
> +{
> +	$NDCTL update-passphrase -m user:$MASTERKEY $dev
> +	get_security_state
> +	if [ "$sstate" != "unlocked" ]; then
> +		echo "Incorrect security state: $sstate expected: unlocked"
> +		exit 1
> +	fi
> +}
> +
> +freeze_security()
> +{
> +	$NDCTL freeze-security $dev
> +}
> +
> +test_1_security_enable_and_disable()
> +{
> +	enable_passphrase
> +	disable_passphrase
> +}
> +
> +test_2_security_enable_and_update()
> +{
> +	enable_passphrase
> +	update_security
> +	disable_passphrase
> +}
> +
> +test_3_security_enable_and_erase()
> +{
> +	enable_passphrase
> +	erase_security
> +}
> +
> +test_4_security_unlocking()
> +{
> +	enable_passphrase
> +	locking_dimm
> +	$NDCTL enable-dimm $dev
> +	get_security_state
> +	if [ "$sstate" != "unlocked" ]; then
> +		echo "Incorrect security state: $sstate expected: unlocked"
> +		exit 1
> +	fi
> +	$NDCTL disable-region -b $NFIT_TEST_BUS0 all
> +	disable_passphrase
> +}
> +
> +# this should always be the last test. with security frozen, nfit_test must
> +# be removed and is no longer usable
> +test_5_security_freeze()
> +{
> +	enable_passphrase
> +	freeze_security
> +	get_security_state
> +	if [ "$sstate" != "frozen" ]; then
> +		echo "Incorrect security state: $sstate expected: frozen"
> +		exit 1
> +	fi
> +	$NDCTL disable-passphrase $dev && { echo "diable succeed after frozen"; exit 1; }

What is the 'exit 1' here for? Shouldn't we just fall through and check
the  state explicitly anyway, even after a successful disable? And if
so, no need to echo the success message, the state will just get
checked later.

> +	get_security_state
> +	echo $sstate
> +	if [ "$sstate" != "frozen" ]; then
> +		echo "Incorrect security state: $sstate expected: disabled"
> +		exit 1
> +	fi
> +}
> +
> +check_min_kver "4.21" || do_skip "may lack security test handling"

"may lack security enabling" - its not just the test infrastructure
that was added in 4.21

> +
> +modprobe nfit_test
> +rc=1
> +setup
> +rc=2
> +detect
> +setup_keys
> +echo "Test 1, security enable and disable"
> +test_1_security_enable_and_disable
> +echo "Test 2, security enable, update, and disable"
> +test_2_security_enable_and_update
> +echo "Test 3, security enable and erase"
> +test_3_security_enable_and_erase
> +echo "Test 4, unlocking dimm"
> +test_4_security_unlocking
> +
> +# Freeze should always be run last because it locks security state and require
> +# nfit_test module unload.
> +echo "Test 5, freeze security"
> +test_5_security_freeze
> +
> +test_cleanup
> +_cleanup
> +exit 0
> 
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

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

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

* Re: [PATCH v6 07/12] ndctl: setup modprobe rules
  2018-12-14 21:09 ` [PATCH v6 07/12] ndctl: setup modprobe rules Dave Jiang
@ 2019-01-05  1:40   ` Verma, Vishal L
  0 siblings, 0 replies; 19+ messages in thread
From: Verma, Vishal L @ 2019-01-05  1:40 UTC (permalink / raw)
  To: Williams, Dan J, Jiang, Dave, Verma, Vishal L; +Cc: linux-nvdimm


On Fri, 2018-12-14 at 14:09 -0700, Dave Jiang wrote:
> Adding reference config file for modprobe.d in order to trigger the
> reference script that will inject keys associated with the nvdimms into
> the kernel user ring for unlock.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Makefile.am                  |   10 ++++++++++
>  contrib/ndctl-loadkeys.sh    |   24 ++++++++++++++++++++++++
>  contrib/nvdimm_modprobe.conf |    1 +
>  3 files changed, 35 insertions(+)
>  create mode 100755 contrib/ndctl-loadkeys.sh
>  create mode 100644 contrib/nvdimm_modprobe.conf
> 
> diff --git a/Makefile.am b/Makefile.am
> index e0c463a3..5a3f03aa 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -42,6 +42,16 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
>  dist_bashcompletion_DATA = contrib/ndctl
>  endif
>  
> +load_key_file = contrib/ndctl-loadkeys.sh
> +load_keydir = $(sysconfdir)/ndctl/
> +load_key_DATA = $(load_key_file)
> +EXTRA_DIST += $(load_key_file)
> +
> +modprobe_file = contrib/nvdimm_modprobe.conf
> +modprobedir = $(sysconfdir)/modprobe.d/
> +modprobe_DATA = $(modprobe_file)
> +EXTRA_DIST += $(modprobe_file)
> +
>  noinst_LIBRARIES = libccan.a
>  libccan_a_SOURCES = \
>  	ccan/str/str.h \
> diff --git a/contrib/ndctl-loadkeys.sh b/contrib/ndctl-loadkeys.sh
> new file mode 100755
> index 00000000..dae0a88a
> --- /dev/null
> +++ b/contrib/ndctl-loadkeys.sh
> @@ -0,0 +1,24 @@
> +#!/bin/bash -Ex
> +
> +# This script assumes a single master key for all DIMMs
> +
> +KEY_PATH=/etc/ndctl/keys
> +TPMH_PATH=$KEY_PATH/tpm.handle
> +KEYTPE=""
> +TPM_HANDLE=""
> +id=""
> +
> +if [ -f $TPMH_PATH ]; then
> +	KEYTYPE=trusted
> +	TPM_HANDLE="keyhandle=`cat $TPMH_PATH`"
> +else
> +	KEYTYPE=user
> +fi

Same comments as the previous script about uppercase variables,
backticks, and quoting.

> +
> +keyctl show | grep -q nvdimm-master || keyctl add $KEYTYPE nvdimm-master "load `cat $KEY_PATH/nvdimm-master.blob` $TPM_HANDLE" @u > /dev/null

Prefer:

if ! grep -q "nvdimm-master" <<< "$(keyctl show)"; then
	keyctl add ...
fi

In fact is it not possible to directly query keyctl for 'nvdimm-master' 
instead of show everything + grep?

> +
> +for i in `ls -1 $KEY_PATH/nvdimm_*.blob`;

/never/ loop through files using ls - it is fragile and broken..
http://mywiki.wooledge.org/ParsingLs

Use globbing instead - see below.

> +do
> +	id=`echo $i | cut -d'_' -f2`

Useless use of echo :)
id="$(cut -d'_' -f2 <<< $i)"

> +	keyctl add encrypted nvdimm:$id "load `cat $i`" @u
> +done

The whole thing then becomes:
for file in "$key_path"/nvdimm_*; do
	id="$(cut -d'_' -f2 <<< "${file##*/}")"
	keyctl add encrypted nvdimm:"$id" "load $(cat $i)" @u
done

> diff --git a/contrib/nvdimm_modprobe.conf b/contrib/nvdimm_modprobe.conf
> new file mode 100644
> index 00000000..b113d8d7
> --- /dev/null
> +++ b/contrib/nvdimm_modprobe.conf
> @@ -0,0 +1 @@
> +install libnvdimm /usr/sbin/ndctl-loadkeys.sh ; /sbin/modprobe --ignore-install libnvdimm $CMDLINE_OPTS
> 
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

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

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

* Re: [PATCH v6 09/12] ndctl: add overwrite-wait support
  2018-12-14 21:09 ` [PATCH v6 09/12] ndctl: add overwrite-wait support Dave Jiang
@ 2019-01-05  1:54   ` Verma, Vishal L
  0 siblings, 0 replies; 19+ messages in thread
From: Verma, Vishal L @ 2019-01-05  1:54 UTC (permalink / raw)
  To: Williams, Dan J, Jiang, Dave, Verma, Vishal L; +Cc: linux-nvdimm


On Fri, 2018-12-14 at 14:09 -0700, Dave Jiang wrote:
> Subject: [PATCH v6 09/12] ndctl: add overwrite-wait support

Consider s/overwite-wait/wait-overwrite/ in the commit subject since
the command is called wait-overwrite.

> Adding a monitoring command to ndctl in order to wait on the progress of

s/Adding/Add/
s/in order to/to/
In fact..

"Add a blocking 'wait-overwrite' command to ndctl to let a user wait
for an overwrite operation on a dimm to complete."

> overwrite.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Documentation/ndctl/Makefile.am              |    3 +
>  Documentation/ndctl/ndctl-wait-overwrite.txt |   31 ++++++++++
>  builtin.h                                    |    1 
>  ndctl/dimm.c                                 |   27 +++++++++
>  ndctl/lib/dimm.c                             |   79 ++++++++++++++++++++++++++
>  ndctl/lib/libndctl.sym                       |    1 
>  ndctl/libndctl.h                             |    1 
>  ndctl/ndctl.c                                |    1 
>  8 files changed, 143 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ndctl/ndctl-wait-overwrite.txt
> 
> diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
> index bbea9674..a60a67e5 100644
> --- a/Documentation/ndctl/Makefile.am
> +++ b/Documentation/ndctl/Makefile.am
> @@ -52,7 +52,8 @@ man1_MANS = \
>  	ndctl-update-passphrase.1 \
>  	ndctl-disable-passphrase.1 \
>  	ndctl-freeze-security.1 \
> -	ndctl-sanitize-dimm.1
> +	ndctl-sanitize-dimm.1 \
> +	ndctl-wait-overwrite.1
>  
>  CLEANFILES = $(man1_MANS)
>  
> diff --git a/Documentation/ndctl/ndctl-wait-overwrite.txt b/Documentation/ndctl/ndctl-wait-overwrite.txt
> new file mode 100644
> index 00000000..97583a36
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-wait-overwrite.txt
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +ndctl-wait-overwrite(1)
> +=======================
> +
> +NAME
> +----
> +ndctl-wait-overwrite - wait for nvdimm overwrite operation to complete
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl wait-overwrite' <dimm> [<options>]
> +
> +DESCRIPTION
> +-----------
> +The ernel provides a POLL(2) capable sysfs file ('security') to indicate
       ^kernel

> +the state of overwrite. The 'ndctl wait-overwrite' operation waits for
> +'security', across all specified dimms.

"waits for a change in state of the 'security' file"..

> +
> +OPTIONS
> +-------
> +-v::
> +--verbose::
> +	Emit debug messages for the overwrite wait process
> +
> +include::../copyright.txt[]
> +
> +SEE ALSO
> +--------
> +linkndctl:ndctl-sanitize-dimm[1]
> diff --git a/builtin.h b/builtin.h
> index db0f2858..32bee59e 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -53,4 +53,5 @@ int cmd_passphrase_update(int argc, const char **argv, void *ctx);
>  int cmd_disable_passphrase(int argc, const char **argv, void *ctx);
>  int cmd_freeze_security(int argc, const char **argv, void *ctx);
>  int cmd_sanitize_dimm(int argc, const char **argv, void *ctx);
> +int cmd_wait_overwrite(int argc, const char **argv, void *ctx);
>  #endif /* _NDCTL_BUILTIN_H_ */
> diff --git a/ndctl/dimm.c b/ndctl/dimm.c
> index 6b1ee47f..21ffea1e 100644
> --- a/ndctl/dimm.c
> +++ b/ndctl/dimm.c
> @@ -928,6 +928,24 @@ static int action_sanitize_dimm(struct ndctl_dimm *dimm,
>  	return 0;
>  }
>  
> +static int action_wait_overwrite(struct ndctl_dimm *dimm,
> +		struct action_context *actx)
> +{
> +	int rc;
> +
> +	if (!ndctl_dimm_security_supported(dimm)) {
> +		error("%s: security operation not supported\n",
> +				ndctl_dimm_get_devname(dimm));
> +		return -EOPNOTSUPP;
> +	}
> +
> +	rc = ndctl_dimm_wait_for_overwrite_completion(dimm);
> +	if (rc == 1)
> +		printf("%s: overwrite completed.\n",
> +				ndctl_dimm_get_devname(dimm));
> +	return rc;
> +}
> +
>  static int __action_init(struct ndctl_dimm *dimm,
>  		enum ndctl_namespace_version version, int chk_only)
>  {
> @@ -1359,3 +1377,12 @@ int cmd_sanitize_dimm(int argc, const char **argv, void *ctx)
>  				count >= 0 ? count : 0, count > 1 ? "s" : "");
>  	return count >= 0 ? 0 : EXIT_FAILURE;
>  }
> +
> +int cmd_wait_overwrite(int argc, const char **argv, void *ctx)
> +{
> +	int count = dimm_action(argc, argv, ctx, action_wait_overwrite,
> +			base_options,
> +			"ndctl wait-overwrite <nmem0> [<nmem1>..<nmemN>] [<options>]");
> +
> +	return count >= 0 ? 0 : EXIT_FAILURE;
> +}
> diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
> index 2dff80f0..d815b9fc 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -16,6 +16,7 @@
>  #include <util/bitmap.h>
>  #include <util/sysfs.h>
>  #include <stdlib.h>
> +#include <poll.h>
>  #include "private.h"
>  
>  static const char NSINDEX_SIGNATURE[] = "NAMESPACE_INDEX\0";
> @@ -689,3 +690,81 @@ NDCTL_EXPORT int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key)
>  	sprintf(buf, "overwrite %ld\n", key);
>  	return write_security(dimm, buf);
>  }
> +
> +NDCTL_EXPORT int ndctl_dimm_wait_for_overwrite_completion(
> +		struct ndctl_dimm *dimm)

Just a minor nit, but this could as easily be
'ndctl_dimm_wait_for_overwrite'. The completion is implied since we're
waiting.. In fact, I think 'ndctl_dimm_wait_overwrite' will be even
better.

> +{
> +	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +	struct pollfd fds;
> +	char buf[SYSFS_ATTR_SIZE];
> +	int fd = 0, rc;
> +	char *path = dimm->dimm_buf;
> +	int len = dimm->buf_len;
> +
> +	if (snprintf(path, len, "%s/security", dimm->dimm_path) >= len) {
> +		err(ctx, "%s: buffer too small!\n",
> +				ndctl_dimm_get_devname(dimm));
> +		return -ERANGE;
> +	}
> +
> +	fd = open(path, O_RDONLY|O_CLOEXEC);
> +	if (fd < 0) {
> +		rc = -errno;
> +		err(ctx, "open: %s\n", strerror(errno));
> +		return rc;
> +	}
> +	memset(&fds, 0, sizeof(fds));
> +	fds.fd = fd;
> +
> +	rc = sysfs_read_attr(ctx, path, buf);
> +	if (rc < 0) {
> +		rc = -EOPNOTSUPP;
> +		goto out;
> +	}
> +	/* skipping if we aren't in overwrite state */
> +	if (strcmp(buf, "overwrite") != 0) {
> +		rc = 0;
> +		goto out;
> +	}
> +
> +	for (;;) {
> +		rc = sysfs_read_attr(ctx, path, buf);
> +		if (rc < 0) {
> +			rc = -EOPNOTSUPP;
> +			break;
> +		}
> +
> +		if (strcmp(buf, "overwrite") == 0) {
> +			rc = poll(&fds, 1, -1);
> +			if (rc < 0) {
> +				rc = -errno;
> +				err(ctx, "poll error: %s\n", strerror(errno));
> +				break;
> +			}
> +			dbg(ctx, "poll wake: revents: %d\n", fds.revents);
> +			if (pread(fd, buf, 1, 0) == -1) {
> +				rc = -errno;
> +				break;
> +			}
> +			fds.revents = 0;
> +		} else {
> +			if (strcmp(buf, "disabled") == 0)
> +				rc = 1;
> +			break;
> +		}
> +	}
> +
> +	if (rc == 1)
> +		dbg(ctx, "%s: overwrite complete\n",
> +				ndctl_dimm_get_devname(dimm));
> +	else if (rc == 0)
> +		dbg(ctx, "%s: ovewrite skipped\n",
> +				ndctl_dimm_get_devname(dimm));
> +	else
> +		dbg(ctx, "%s: overwrite error waiting for complete\n",
> +				ndctl_dimm_get_devname(dimm));
> +
> + out:
> +	close(fd);
> +	return rc;
> +}
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index 0fedae1a..0da8b282 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -400,4 +400,5 @@ global:
>  	ndctl_dimm_secure_erase_key;
>  	ndctl_dimm_overwrite;
>  	ndctl_dimm_overwrite_key;
> +	ndctl_dimm_wait_for_overwrite_completion;
>  } LIBNDCTL_18;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index fee35db7..5c34f2cb 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -706,6 +706,7 @@ int ndctl_dimm_disable_passphrase(struct ndctl_dimm *dimm, long key);
>  int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm);
>  int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm, long key);
>  int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key);
> +int ndctl_dimm_wait_for_overwrite_completion(struct ndctl_dimm *dimm);
>  
>  enum ndctl_key_type {
>  	ND_USER_KEY,
> diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
> index 40982f43..d16e1ed0 100644
> --- a/ndctl/ndctl.c
> +++ b/ndctl/ndctl.c
> @@ -93,6 +93,7 @@ static struct cmd_struct commands[] = {
>  	{ "disable-passphrase", cmd_disable_passphrase },
>  	{ "freeze-security", cmd_freeze_security },
>  	{ "sanitize-dimm", cmd_sanitize_dimm },
> +	{ "wait-overwrite", cmd_wait_overwrite },
>  	{ "list", cmd_list },
>  	{ "monitor", cmd_monitor},
>  	{ "help", cmd_help },
> 
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

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

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

* Re: [PATCH v6 10/12] ndctl: master phassphrase management support
  2018-12-14 21:09 ` [PATCH v6 10/12] ndctl: master phassphrase management support Dave Jiang
@ 2019-01-05  2:01   ` Verma, Vishal L
  0 siblings, 0 replies; 19+ messages in thread
From: Verma, Vishal L @ 2019-01-05  2:01 UTC (permalink / raw)
  To: Williams, Dan J, Jiang, Dave, Verma, Vishal L; +Cc: linux-nvdimm


On Fri, 2018-12-14 at 14:09 -0700, Dave Jiang wrote:
> Adding master passphrase enabling and update to ndctl. This is a new
> feature from Intel DSM v1.8.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Documentation/ndctl/ndctl-enable-passphrase.txt |    7 +
>  Documentation/ndctl/ndctl-update-passphrase.txt |    7 +
>  ndctl/dimm.c                                    |   11 ++
>  ndctl/lib/dimm.c                                |    9 ++
>  ndctl/lib/keys.c                                |  113 +++++++++++++++++------
>  ndctl/lib/libndctl.sym                          |    1 
>  ndctl/libndctl.h                                |   14 ++-
>  7 files changed, 122 insertions(+), 40 deletions(-)
> 
> diff --git a/Documentation/ndctl/ndctl-enable-passphrase.txt b/Documentation/ndctl/ndctl-enable-passphrase.txt
> index 8de5410c..6639ce8d 100644
> --- a/Documentation/ndctl/ndctl-enable-passphrase.txt
> +++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
> @@ -29,7 +29,12 @@ OPTIONS
>  include::xable-dimm-options.txt[]
>  
>  -m::
> ---master=::
> +--master-key=::
>  	Key name for the master key used to seal the NVDIMM security keys.
>  
> +-M::
> +--master-passphrase::
> +	Parameter to indicate that we are managing the master passphrase

s/Parameter/Option/
Or even better, omit it entirely since we're in the options section.
"Indicate that we are.."

> +	instead of the user passphrase.
> +
>  include::../copyright.txt[]
> diff --git a/Documentation/ndctl/ndctl-update-passphrase.txt b/Documentation/ndctl/ndctl-update-passphrase.txt
> index 9ed39cca..e2ecacf5 100644
> --- a/Documentation/ndctl/ndctl-update-passphrase.txt
> +++ b/Documentation/ndctl/ndctl-update-passphrase.txt
> @@ -26,8 +26,13 @@ OPTIONS
>  include::xable-dimm-options.txt[]
>  
>  -m::
> ---master::
> +--master-key=::
>  	New key name for the master key to seal the new nvdimm key, or the
>  	existing master key name. i.e trusted:master-key.
>  
> +-M::
> +--master-passphrase::
> +	Parameter to indicate that we are managing the master passphrase
> +	instead of the user passphrase.
> +
>  include::../copyright.txt[]
> diff --git a/ndctl/dimm.c b/ndctl/dimm.c
> index 21ffea1e..c60ef96e 100644
> --- a/ndctl/dimm.c
> +++ b/ndctl/dimm.c
> @@ -48,6 +48,7 @@ static struct parameters {
>  	const char *master_key;
>  	bool crypto_erase;
>  	bool overwrite;
> +	bool master_pass;
>  	bool force;
>  	bool json;
>  	bool verbose;
> @@ -848,7 +849,8 @@ static int action_key_enable(struct ndctl_dimm *dimm,
>  		return -EOPNOTSUPP;
>  	}
>  
> -	return ndctl_dimm_enable_key(dimm, param.master_key);
> +	return ndctl_dimm_enable_key(dimm, param.master_key,
> +			param.master_pass ? ND_MASTER_KEY : ND_USER_KEY);
>  }
>  
>  static int action_key_update(struct ndctl_dimm *dimm,
> @@ -860,7 +862,8 @@ static int action_key_update(struct ndctl_dimm *dimm,
>  		return -EOPNOTSUPP;
>  	}
>  
> -	return ndctl_dimm_update_key(dimm, param.master_key);
> +	return ndctl_dimm_update_key(dimm, param.master_key,
> +			param.master_pass ? ND_MASTER_KEY : ND_USER_KEY);
>  }
>  
>  static int action_passphrase_disable(struct ndctl_dimm *dimm,
> @@ -1037,7 +1040,9 @@ OPT_STRING('V', "label-version", &param.labelversion, "version-number", \
>  
>  #define KEY_OPTIONS() \
>  OPT_STRING('m', "master-key", &param.master_key, "<key_type>:<key_name>", \
> -		"master key for security")
> +		"master key for security"), \
> +OPT_BOOLEAN('M', "master-passphrase", &param.master_pass, \
> +		"use master passphrase")
>  
>  #define SANITIZE_OPTIONS() \
>  OPT_BOOLEAN('c', "crypto-erase", &param.crypto_erase, \
> diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
> index d815b9fc..07513b4b 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -768,3 +768,12 @@ NDCTL_EXPORT int ndctl_dimm_wait_for_overwrite_completion(
>  	close(fd);
>  	return rc;
>  }
> +
> +NDCTL_EXPORT int ndctl_dimm_update_master_passphrase(struct ndctl_dimm *dimm,
> +		long ckey, long nkey)
> +{
> +	char buf[SYSFS_ATTR_SIZE];
> +
> +	sprintf(buf, "master_update %ld %ld\n", ckey, nkey);
> +	return write_security(dimm, buf);
> +}
> diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
> index fcf300bb..1d395b48 100644
> --- a/ndctl/lib/keys.c
> +++ b/ndctl/lib/keys.c
> @@ -34,16 +34,29 @@ static int get_key_path(struct ndctl_dimm *dimm, char *path,
>  		return -errno;
>  	}
>  
> -	if (key_type == ND_USER_OLD_KEY) {
> -		rc = sprintf(path, "%s/nvdimmold_%s_%s.blob",
> -				ND_PASS_PATH,
> -				ndctl_dimm_get_unique_id(dimm),
> +	switch (key_type) {
> +	case ND_USER_OLD_KEY:
> +		rc = sprintf(path, "%s/nvdimm-old_%s_%s.blob",
> +				ND_PASS_PATH, ndctl_dimm_get_unique_id(dimm),
>  				hostname);
> -	} else {
> +		break;
> +	case ND_USER_KEY:
>  		rc = sprintf(path, "%s/nvdimm_%s_%s.blob",
> -				ND_PASS_PATH,
> -				ndctl_dimm_get_unique_id(dimm),
> +				ND_PASS_PATH, ndctl_dimm_get_unique_id(dimm),
>  				hostname);
> +		break;
> +	case ND_MASTER_OLD_KEY:
> +		rc = sprintf(path, "%s/nvdimm-master-old_%s_%s.blob",
> +				ND_PASS_PATH, ndctl_dimm_get_unique_id(dimm),
> +				hostname);
> +		break;
> +	case ND_MASTER_KEY:
> +		rc = sprintf(path, "%s/nvdimm-master_%s_%s.blob",
> +				ND_PASS_PATH, ndctl_dimm_get_unique_id(dimm),
> +				hostname);
> +		break;
> +	default:
> +		return -EINVAL;
>  	}
>  
>  	if (rc < 0) {
> @@ -60,12 +73,26 @@ static int get_key_desc(struct ndctl_dimm *dimm, char *desc,
>  	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
>  	int rc;
>  
> -	if (key_type == ND_USER_OLD_KEY)
> +	switch (key_type) {
> +	case ND_USER_OLD_KEY:
>  		rc = sprintf(desc, "nvdimm-old:%s",
>  				ndctl_dimm_get_unique_id(dimm));
> -	else
> +		break;
> +	case ND_USER_KEY:
>  		rc = sprintf(desc, "nvdimm:%s",
>  				ndctl_dimm_get_unique_id(dimm));
> +		break;
> +	case ND_MASTER_OLD_KEY:
> +		rc = sprintf(desc, "nvdimm-master-old:%s",
> +				ndctl_dimm_get_unique_id(dimm));
> +		break;
> +	case ND_MASTER_KEY:
> +		rc = sprintf(desc, "nvdimm-master:%s",
> +				ndctl_dimm_get_unique_id(dimm));
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
>  
>  	if (rc < 0) {
>  		err(ctx, "sprintf: %s\n", strerror(errno));
> @@ -144,7 +171,7 @@ static key_serial_t dimm_check_key(struct ndctl_dimm *dimm,
>  }
>  
>  static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
> -		const char *master)
> +		const char *master_key, enum ndctl_key_type key_type)
>  {
>  	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
>  	char desc[DESC_SIZE];
> @@ -164,7 +191,7 @@ static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
>  		return -EBUSY;
>  	}
>  
> -	rc = get_key_desc(dimm, desc, ND_USER_KEY);
> +	rc = get_key_desc(dimm, desc, key_type);
>  	if (rc < 0)
>  		return rc;
>  
> @@ -175,7 +202,7 @@ static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
>  		return -EAGAIN;
>  	}
>  
> -	rc = get_key_path(dimm, path, ND_USER_KEY);
> +	rc = get_key_path(dimm, path, key_type);
>  	if (rc < 0)
>  		return rc;
>  
> @@ -185,7 +212,7 @@ static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
>  		return -EAGAIN;
>  	}
>  
> -	rc = sprintf(cmd, "new enc32 %s 32", master);
> +	rc = sprintf(cmd, "new enc32 %s 32", master_key);
>  	if (rc < 0) {
>  		err(ctx, "sprintf: %s\n", strerror(errno));
>  		return -errno;
> @@ -271,13 +298,15 @@ static key_serial_t dimm_load_key(struct ndctl_dimm *dimm,
>   * blob, and then attempt to inject the key as old key into the user key
>   * ring.
>   */
> -static key_serial_t move_key_to_old(struct ndctl_dimm *dimm)
> +static key_serial_t move_key_to_old(struct ndctl_dimm *dimm,
> +		enum ndctl_key_type key_type)
>  {
>  	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
>  	int rc;
>  	key_serial_t key;
>  	char old_path[PATH_SIZE];
>  	char new_path[PATH_SIZE];
> +	enum ndctl_key_type okey_type;
>  
>  	if (ndctl_dimm_is_active(dimm)) {
>  		err(ctx, "regions active on %s, op failed\n",
> @@ -285,15 +314,22 @@ static key_serial_t move_key_to_old(struct ndctl_dimm *dimm)
>  		return -EBUSY;
>  	}
>  
> -	key = dimm_check_key(dimm, ND_USER_KEY);
> +	key = dimm_check_key(dimm, key_type);
>  	if (key > 0)
>  		keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
>  
> -	rc = get_key_path(dimm, old_path, ND_USER_KEY);
> +	if (key_type == ND_USER_KEY)
> +		okey_type = ND_USER_OLD_KEY;
> +	else if (key_type == ND_MASTER_KEY)
> +		okey_type = ND_MASTER_OLD_KEY;
> +	else
> +		return -EINVAL;
> +
> +	rc = get_key_path(dimm, old_path, key_type);
>  	if (rc < 0)
>  		return rc;
>  
> -	rc = get_key_path(dimm, new_path, ND_USER_OLD_KEY);
> +	rc = get_key_path(dimm, new_path, okey_type);
>  	if (rc < 0)
>  		return rc;
>  
> @@ -301,7 +337,7 @@ static key_serial_t move_key_to_old(struct ndctl_dimm *dimm)
>  	if (rc < 0)
>  		return -errno;
>  
> -	return dimm_load_key(dimm, ND_USER_OLD_KEY);
> +	return dimm_load_key(dimm, okey_type);
>  }
>  
>  static int dimm_remove_key(struct ndctl_dimm *dimm,
> @@ -330,18 +366,21 @@ static int dimm_remove_key(struct ndctl_dimm *dimm,
>  }
>  
>  NDCTL_EXPORT int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
> -		const char *master)
> +		const char *master_key, enum ndctl_key_type key_type)
>  {
>  	key_serial_t key;
>  	int rc;
>  
> -	key = dimm_create_key(dimm, master);
> +	key = dimm_create_key(dimm, master_key, key_type);
>  	if (key < 0)
>  		return (int)key;
>  
> -	rc = ndctl_dimm_update_passphrase(dimm, 0, key);
> +	if (key_type == ND_MASTER_KEY)
> +		rc = ndctl_dimm_update_master_passphrase(dimm, 0, key);
> +	else
> +		rc = ndctl_dimm_update_passphrase(dimm, 0, key);
>  	if (rc < 0) {
> -		dimm_remove_key(dimm, ND_USER_KEY);
> +		dimm_remove_key(dimm, key_type);
>  		return rc;
>  	}
>  
> @@ -349,10 +388,18 @@ NDCTL_EXPORT int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
>  }
>  
>  NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
> -		const char *master)
> +		const char *master_key, enum ndctl_key_type key_type)
>  {
>  	int rc;
>  	key_serial_t old_key, new_key;
> +	enum ndctl_key_type okey_type;
> +
> +	if (key_type == ND_USER_KEY)
> +		okey_type = ND_USER_OLD_KEY;
> +	else if (key_type == ND_MASTER_KEY)
> +		okey_type = ND_MASTER_OLD_KEY;
> +	else
> +		return -EINVAL;
>  
>  	/*
>  	 * 1. check if current key is loaded and remove
> @@ -362,23 +409,27 @@ NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
>  	 * 5. remove old key
>  	 * 6. remove old key blob
>  	 */
> -	old_key = move_key_to_old(dimm);
> +	old_key = move_key_to_old(dimm, key_type);
>  	if (old_key < 0)
>  		return old_key;
>  
> -	new_key = dimm_create_key(dimm, master);
> +	new_key = dimm_create_key(dimm, master_key, key_type);
>  	/* need to create new key here */
>  	if (new_key < 0) {
> -		new_key = dimm_load_key(dimm, ND_USER_KEY);
> +		new_key = dimm_load_key(dimm, key_type);
>  		if (new_key < 0)
>  			return new_key;
>  	}
>  
> -	rc = ndctl_dimm_update_passphrase(dimm, old_key, new_key);
> +	if (key_type == ND_MASTER_KEY)
> +		rc = ndctl_dimm_update_master_passphrase(dimm,
> +				old_key, new_key);
> +	else
> +		rc = ndctl_dimm_update_passphrase(dimm, old_key, new_key);
>  	if (rc < 0)
>  		return rc;
>  
> -	rc = dimm_remove_key(dimm, ND_USER_OLD_KEY);
> +	rc = dimm_remove_key(dimm, okey_type);
>  	if (rc < 0)
>  		return rc;
>  
> @@ -392,9 +443,9 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
>  	key_serial_t key;
>  	int rc;
>  
> -	key = dimm_check_key(dimm, false);
> +	key = dimm_check_key(dimm, ND_USER_KEY);
>  	if (key < 0) {
> -		key = dimm_load_key(dimm, false);
> +		key = dimm_load_key(dimm, ND_USER_KEY);
>  		if (key < 0 && run_op != ndctl_dimm_overwrite) {
>  			err(ctx, "Unable to load key\n");
>  			return -ENOKEY;
> @@ -410,7 +461,7 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
>  	}
>  
>  	if (key) {
> -		rc = dimm_remove_key(dimm, false);
> +		rc = dimm_remove_key(dimm, ND_USER_KEY);
>  		if (rc < 0)
>  			err(ctx, "Unable to cleanup key.\n");
>  	}
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index 0da8b282..bd933eb2 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -401,4 +401,5 @@ global:
>  	ndctl_dimm_overwrite;
>  	ndctl_dimm_overwrite_key;
>  	ndctl_dimm_wait_for_overwrite_completion;
> +	ndctl_dimm_update_master_passphrase;
>  } LIBNDCTL_18;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index 5c34f2cb..a0b8a886 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -707,27 +707,33 @@ int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm);
>  int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm, long key);
>  int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key);
>  int ndctl_dimm_wait_for_overwrite_completion(struct ndctl_dimm *dimm);
> +int ndctl_dimm_update_master_passphrase(struct ndctl_dimm *dimm,
> +		long ckey, long nkey);
>  
>  enum ndctl_key_type {
>  	ND_USER_KEY,
>  	ND_USER_OLD_KEY,
> +	ND_MASTER_KEY,
> +	ND_MASTER_OLD_KEY,
>  };
>  
>  #ifdef ENABLE_KEYUTILS
> -int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master);
> -int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master);
> +int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master_key,
> +		enum ndctl_key_type key_type);
> +int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master_key,
> +		enum ndctl_key_type key_type);
>  int ndctl_dimm_disable_key(struct ndctl_dimm *dimm);
>  int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm);
>  int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm);
>  #else
>  static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
> -		const char *master)
> +		const char *master_key, enum ndctl_key_type key_type);
>  {
>  	return -EOPNOTSUPP;
>  }
>  
>  static inline int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
> -		const char *master)
> +		const char *master_key, enum ndctl_key_type key_type);
>  {
>  	return -EOPNOTSUPP;
>  }
> 
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

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

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

* Re: [PATCH v6 12/12] ndctl: documentation for security and key management
  2018-12-14 21:10 ` [PATCH v6 12/12] ndctl: documentation for security and key management Dave Jiang
@ 2019-01-05  2:31   ` Verma, Vishal L
  0 siblings, 0 replies; 19+ messages in thread
From: Verma, Vishal L @ 2019-01-05  2:31 UTC (permalink / raw)
  To: Williams, Dan J, Jiang, Dave, Verma, Vishal L; +Cc: linux-nvdimm


On Fri, 2018-12-14 at 14:10 -0700, Dave Jiang wrote:
> Adding the theory of operation for Intel DSM operations.

Add a "Theory of Operation" section describing the Intel DSM operations
to the relevant man pages.

> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Documentation/ndctl/intel-nvdimm-security.txt    |  139 ++++++++++++++++++++++
>  Documentation/ndctl/ndctl-disable-passphrase.txt |    2 
>  Documentation/ndctl/ndctl-enable-passphrase.txt  |    4 +
>  Documentation/ndctl/ndctl-freeze-security.txt    |    2 
>  Documentation/ndctl/ndctl-sanitize-dimm.txt      |    2 
>  Documentation/ndctl/ndctl-update-passphrase.txt  |    2 
>  6 files changed, 151 insertions(+)
>  create mode 100644 Documentation/ndctl/intel-nvdimm-security.txt
> 
> diff --git a/Documentation/ndctl/intel-nvdimm-security.txt b/Documentation/ndctl/intel-nvdimm-security.txt
> new file mode 100644
> index 00000000..0c9a41a4
> --- /dev/null
> +++ b/Documentation/ndctl/intel-nvdimm-security.txt
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +THEORY OF OPERATOIN
> +-------------------
> +With the introduction of Intel Device Specific Methods (DSM) specification
> +v1.7 and v1.8 [1], security DSMs were introduced. Ndctl supports enable

'ndctl' with an uppercase 'N' just looks very odd :)
Perhaps reword as: The operations supported by ndctl are..

> +passhprase, update passphrase, disable security, freeze security, secure
> +(crypto) erase, overwrite, master passphrase enable, master passphrase update,
> +and master passphrase secure (crypto) erase. The DSM that is not supported by
> +ndctl is the unlock DSM, which is left to the kernel to manage.

Repetition of 'DSM'. Perhaps:
The 'unlock' DSM is not supported by ndctl - that is left for the
kernel to manage.

> +
> +The security management for nvdimm is composed of two parts. The front end
> +utilizes the Linux key management framework (trusted and encrypted keys [2]).
> +It uses the keyutils on the user side and Linux key management APIs in
> +the kernel. The backend takes the decrypted payload of the key and passes the
> +plaintext payload to the nvdimm for processing.
> +
> +Unlike typical DSMs, the security DSMs are managed through the 'security'
> +sysfs attribute under the dimm devices rather than an ioctl call by libndctl.
> +The relevant key id is written to the 'security' attribute and the kernel will
> +pull that key from the kernel user key ring for processing.
> +
> +The entire security process starts with a master key that is used to seal the
> +encrypted keys that are used to protect the passphrase for each nvdimm. We
> +recommend using THE system master trusted key from the Trusted Platform
> +Module (TPM). However, a trusted master key generated by the TPM can also
> +be used.

Do these two bits say the same thing?

>  And for testing purposes, a user key with randomized payload can

Don't begin with "And..". Just say "For testing.."

> +also be served as a master key. See [2] for details. To perform any security
> +operations, it is expected that at the minimal the master key is already

"it is expected that at the minimum, the master key.."

> +in the kernel user keyring as shown in example below:

"in the kernel's.."

> +
> +> keyctl show
> +Session Keyring
> + 736023423 --alswrv      0     0  keyring: _ses
> + 675104189 --alswrv      0 65534   \_ keyring: _uid.0
> + 680187394 --alswrv      0     0       \_ trusted: nvdimm-master
> +
> +All operations expect the relevant regions associated to the nvdimm are

"associated with the.."

> +disabled before proceeding, except overwrite. Overwrite expects the

Don't tag 'except overwrite' at the end, instead have it at the start.

> +relevant nvdimm also be disabled.

Can be made clearer if you say something like:
"For 'overwrite', in addition to the regions, the dimm itself is
expected to be disabled"

> +
> +The follow on sections describe specifics of each security features.

"The following sections"

> +
> +UNLOCK
> +------
> +Unlock is performed by the kernel, however a preparation step must happen
> +before the unlock DSM can be issued by the kernel. The expectation is that
> +during initramfs, a setup script is called before the libnvdimm module is
> +loaded by modprobe. The said script will inject the master key and the

"This script will inject.."

> +related encrypted keys into the kernel user key ring. A reference modprobe

"kernel's"

> +config file and a setup script have been provided by ndctl. When the
> +nvdimm driver probes, it will

"During the 'probe' of the nvdimm driver.."

> +1. First, Check the security state of the device and see if the dimm is locked

s/Check/check/
dimm vs. DIMM can be made more consistent.

> +2. Request the associated encrypted key from the kernel user key ring.
> +3. Finally, create the unlock DSM and copy the decrypted payload into the DSM

s/and/,/

> +   passphrase field, and issue the DSM to unlock the DIMM.
> +
> +If the DIMM is already unlocked, the kernel will attempt to revalidate the key.
> +This can be overriden with a kernel module parameter. If we fail to revalidate
> +the key, the kernel will freeze the security and disallow any further security
> +configuration changes.
> +
> +ENABLE USER PASSPHRASE
> +----------------------
> +To enable the user passphrase for a DIMM, it is expected that the master key
> +is already in the kernel user key ring and the master key name is passed to
> +ndctl so it can do key management. An encrypted key with a 32byte payload

s/32byte/32 byte/

> +and encrypted key format 'enc32' is created and sealed by the master key. Be
> +aware that the passphrase is never provided by or visible to the user.
> +The decrypted payload for the encrypted key will be randomly generated by the
> +kernel and userspace does not have access to this decrypted payload. When the
> +encrypted key is created, a binary blob of the encrypted key is written to the
> +designated key blob storage directory (/etc/ndctl/keys as default). The user is
> +responsible for backing up the dimm key blobs to a secure location. When a key
> +is successfully loaded into the kernel user key ring, the payload is decrypted
> +and can be accessed by the kernel.
> +
> +Key ids are written to the 'security' sysfs attribute with the command "update".
> +A key id of 0 is provided for the old key id. The kernel will see that the
> +update call is an enable call because the 0 old key id. A "passphrase update"
> +DSM is issued by the kernel with the old passphrase as 0s.
> +
> +UPDATE USER PASSPHRASE
> +----------------------
> +The update user passphrase operation uses the same DSM command as enable user
> +passphrase. Most of the work is done on the key management. 

The last sentence seems incomplete? Perhaps "on the key management
side"?

> The user will
> +provide a new master key name for the new passphrase key or use the existing
> +one. Ndctl will use whatever master key name that is passed in.

Same nit with 'Ndctl'. Perhaps "The master key name that is passed in
will be used as-is by ndctl"

>  The following
> +operation is performed for update:

operations are

> +1. Remove the associated encrypted key from the kernel user key ring.
> +2. Rename the key blob to old.
> +3. Load the now old key blob into kernel user key ring with "old" name.
> +4. Create new encrypted key and seal with master key.
> +5. Generate new key blob.
> +6. Send DSM with old and new passphrase via the decrypted key payloads.
> +7. On success, remove old key from key ring and old key blob.
> +
> +DISABLE USER PASSPHRASE
> +-----------------------
> +Ndctl will look up the key id for the current associated key and write to sysfs.
> +Upon success of disabling, the key will be removed from the kernel user key
> +ring and the related key blob will also be deleted.

The current key id is written to sysfs by ndctl. Upon successfully
disabling the passphrase, the key is removed from the kernel's user
keyring, and the associated key blob is deleted.

> +
> +CRYPTO (SECURE) ERASE
> +---------------------
> +This operation is similar to disable the passphrase. The kernel will issue

similar to passphrase-disable.

> +wbinvd before and after the operation to ensure no data corruption from stale

'WBINVD'
"from a stale.."

> +CPU cache. The "sanitize-dimm" command with the --crypto-erase switch is used
> +via ndctl.

Use ndctl's sanitize-dimm command with the --crypto-erase option to
perform this operation.

> +
> +OVERWRITE
> +---------
> +The overwrite operation wipes the entire nvdimm. Therefore it can take

s/. Therefore it/ and/

> +a significant amount of time. To issue the command is very similar to disable
> +passphrase and secure erase.

Issuing the command is very similar to..

>  However, when the command returns successfully
> +it just means overwrite has been successfully started. The wait-overwrite
> +command for ndctl can be used to wait on the nvdimms that are performing
> +overwrite until the operation is completed. Upon successful completion of the
> +operation, wbinvd will be issued by the kernel. The "sanitize-dimm" command
> +with the --overwrite switch is used via ndctl. If both --crypto-erase and
> +--overwrite switches are passed in, the crypt-erase will be issued first before
> +overwrite.

s/switch/option/g

> +
> +SECURITY FREEZE
> +---------------
> +This operation requires no key to succeed. Ndctl will issue the DSM command

s/Ndctl/ndctl/ or reword

> +and upon completion, the security commands besides status query will be locked
> +out until the next boot.
> +
> +MASTER PASSPHRASE ENABLE, UPDATE, and CRYPTO ERASE
> +-----------------------------------------------------------
> +These operations are similar to the user passphrase enable and update. The only
> +difference is that a different encrypted key is being used for the master
> +passphrase operations. Note that master passphrase has no relation to the
> +master key for encryption.
> +
> +
> +[1] http://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf
> +[2] https://www.kernel.org/doc/Documentation/security/keys/trusted-encrypted.rst
> diff --git a/Documentation/ndctl/ndctl-disable-passphrase.txt b/Documentation/ndctl/ndctl-disable-passphrase.txt
> index e921eb23..df7401cb 100644
> --- a/Documentation/ndctl/ndctl-disable-passphrase.txt
> +++ b/Documentation/ndctl/ndctl-disable-passphrase.txt
> @@ -24,4 +24,6 @@ OPTIONS
>  <dimm>::
>  include::xable-dimm-options.txt[]
>  
> +include::intel-nvdimm-security.txt[]
> +
>  include::../copyright.txt[]
> diff --git a/Documentation/ndctl/ndctl-enable-passphrase.txt b/Documentation/ndctl/ndctl-enable-passphrase.txt
> index 6639ce8d..9ff19927 100644
> --- a/Documentation/ndctl/ndctl-enable-passphrase.txt
> +++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
> @@ -31,10 +31,14 @@ include::xable-dimm-options.txt[]
>  -m::
>  --master-key=::
>  	Key name for the master key used to seal the NVDIMM security keys.
> +	The format would be <key_type>:<master_key_name>
> +	i.e.: trusted:master-nvdimm

This hunk should've been in the original patch that added this?

>  
>  -M::
>  --master-passphrase::
>  	Parameter to indicate that we are managing the master passphrase
>  	instead of the user passphrase.
>  
> +include::intel-nvdimm-security.txt[]
> +
>  include::../copyright.txt[]
> diff --git a/Documentation/ndctl/ndctl-freeze-security.txt b/Documentation/ndctl/ndctl-freeze-security.txt
> index 4e9d2d61..2ae21980 100644
> --- a/Documentation/ndctl/ndctl-freeze-security.txt
> +++ b/Documentation/ndctl/ndctl-freeze-security.txt
> @@ -17,4 +17,6 @@ DESCRIPTION
>  Provide a generic interface to freeze the security for NVDIMM. Once security
>  is frozen, no other security operations can succeed until reboot happens.
>  
> +include::intel-nvdimm-security.txt[]
> +
>  include::../copyright.txt[]
> diff --git a/Documentation/ndctl/ndctl-sanitize-dimm.txt b/Documentation/ndctl/ndctl-sanitize-dimm.txt
> index 7b036318..a8cdca71 100644
> --- a/Documentation/ndctl/ndctl-sanitize-dimm.txt
> +++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
> @@ -39,4 +39,6 @@ include::xable-dimm-options.txt[]
>  	instead of the user passphrase. This only is applicable to the
>  	crypto-erase option.
>  
> +include::intel-nvdimm-security.txt[]
> +
>  include::../copyright.txt[]
> diff --git a/Documentation/ndctl/ndctl-update-passphrase.txt b/Documentation/ndctl/ndctl-update-passphrase.txt
> index e2ecacf5..046fac6c 100644
> --- a/Documentation/ndctl/ndctl-update-passphrase.txt
> +++ b/Documentation/ndctl/ndctl-update-passphrase.txt
> @@ -35,4 +35,6 @@ include::xable-dimm-options.txt[]
>  	Parameter to indicate that we are managing the master passphrase
>  	instead of the user passphrase.
>  
> +include::intel-nvdimm-security.txt[]
> +
>  include::../copyright.txt[]
> 
> _______________________________________________
> Linux-nvdimm mailing list
> Linux-nvdimm@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm

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

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

end of thread, other threads:[~2019-01-05  2:31 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-14 21:09 [PATCH v6 00/12] ndctl: add security support Dave Jiang
2018-12-14 21:09 ` [PATCH v6 01/12] ndctl: add support for display security state Dave Jiang
2018-12-14 21:09 ` [PATCH v6 02/12] ndctl: add passphrase update to ndctl Dave Jiang
2019-01-05  0:07   ` Verma, Vishal L
2018-12-14 21:09 ` [PATCH v6 03/12] ndctl: add disable security support Dave Jiang
2018-12-14 21:09 ` [PATCH v6 04/12] ndctl: add support for freeze security Dave Jiang
2018-12-14 21:09 ` [PATCH v6 05/12] ndctl: add support for sanitize dimm Dave Jiang
2018-12-14 21:09 ` [PATCH v6 06/12] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
2019-01-05  1:21   ` Verma, Vishal L
2018-12-14 21:09 ` [PATCH v6 07/12] ndctl: setup modprobe rules Dave Jiang
2019-01-05  1:40   ` Verma, Vishal L
2018-12-14 21:09 ` [PATCH v6 08/12] ndctl: add overwrite operation support Dave Jiang
2018-12-14 21:09 ` [PATCH v6 09/12] ndctl: add overwrite-wait support Dave Jiang
2019-01-05  1:54   ` Verma, Vishal L
2018-12-14 21:09 ` [PATCH v6 10/12] ndctl: master phassphrase management support Dave Jiang
2019-01-05  2:01   ` Verma, Vishal L
2018-12-14 21:10 ` [PATCH v6 11/12] ndctl: add master secure erase support Dave Jiang
2018-12-14 21:10 ` [PATCH v6 12/12] ndctl: documentation for security and key management Dave Jiang
2019-01-05  2:31   ` Verma, Vishal L

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