* [PATCH v8 01/12] ndctl: add support for display security state
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
@ 2019-01-14 20:06 ` Dave Jiang
2019-01-16 1:13 ` Dan Williams
2019-01-14 20:06 ` [PATCH v8 02/12] ndctl: add passphrase update to ndctl Dave Jiang
` (10 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:06 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 79e2ca0a..e03135d9 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -587,3 +587,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 c81cc032..4255252c 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] 23+ messages in thread* Re: [PATCH v8 01/12] ndctl: add support for display security state
2019-01-14 20:06 ` [PATCH v8 01/12] ndctl: add support for display security state Dave Jiang
@ 2019-01-16 1:13 ` Dan Williams
0 siblings, 0 replies; 23+ messages in thread
From: Dan Williams @ 2019-01-16 1:13 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
Some comments below:
On Mon, Jan 14, 2019 at 12:07 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> 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 79e2ca0a..e03135d9 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -587,3 +587,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)
Lets just have this routine return an "enum ndctl_security_state"
rather than an in/out parameter. Also name it "enum ndctl_" to match
the other enum definitions in libndctl.h, and use NDCTL_ in the enum
value names.
> +{
> + 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;
We'll never get here because a missing "security" attribute is how the
kernel indicates "unsupported" and the sysfs_read_attr() call above
would have already failed with -ENOENT.
> + 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;
An error will be thrown before we get here, no need for
ND_SECURITY_INVALID or ND_SECURITY_UNSUPPORTED should be defined.
> +
> + 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 c81cc032..4255252c 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) {
Returning the state directly means we can lose a level of identation.
> + switch (sstate) {
> + case ND_SECURITY_UNSUPPORTED:
> + jobj = json_object_new_string("unsupported");
> + break;
> + case ND_SECURITY_DISABLED:
> + jobj = json_object_new_string("disabled");
> + break;
It's less lines to write an if / else tree rather than a switch.
> + 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);
> + }
> +
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v8 02/12] ndctl: add passphrase update to ndctl
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
2019-01-14 20:06 ` [PATCH v8 01/12] ndctl: add support for display security state Dave Jiang
@ 2019-01-14 20:06 ` Dave Jiang
2019-01-16 1:56 ` Dan Williams
2019-01-14 20:06 ` [PATCH v8 03/12] ndctl: add disable security support Dave Jiang
` (9 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:06 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 | 42 ++
Documentation/ndctl/ndctl-update-passphrase.txt | 38 ++
configure.ac | 19 +
ndctl.spec.in | 2
ndctl/Makefile.am | 3
ndctl/builtin.h | 2
ndctl/dimm.c | 94 +++++-
ndctl/lib/Makefile.am | 8
ndctl/lib/dimm.c | 39 ++
ndctl/lib/keys.c | 390 +++++++++++++++++++++++
ndctl/lib/libndctl.sym | 4
ndctl/libndctl.h | 35 ++
ndctl/ndctl.c | 2
14 files changed, 669 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..c14a206c
--- /dev/null
+++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-enable-passphrase(1)
+==========================
+
+NAME
+----
+ndctl-enable-passphrase - enable the security passphrase for a NVDIMM
+
+SYNOPSIS
+--------
+[verse]
+'ndctl enable-passphrase' <dimm> [<options>]
+
+DESCRIPTION
+-----------
+Provide a command to enable the security passphrase for the 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.
+ The format would be <key_type>:<master_key_name>
+ i.e.: trusted:master-nvdimm
+
+-p::
+--key-path=::
+ Path to where key related files resides. This parameter is optional
+ and the default is set to /etc/ndctl/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..dd6e4e4e
--- /dev/null
+++ b/Documentation/ndctl/ndctl-update-passphrase.txt
@@ -0,0 +1,38 @@
+// 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 command to update the 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.
+
+-p::
+--key-path=::
+ Path to where key related files resides. This parameter is optional
+ and the default is set to /etc/ndctl/keys.
+
+include::../copyright.txt[]
diff --git a/configure.ac b/configure.ac
index aa07ec7b..22efc871 100644
--- a/configure.ac
+++ b/configure.ac
@@ -154,6 +154,7 @@ fi
AC_SUBST([systemd_unitdir])
AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
+
ndctl_monitorconfdir=${sysconfdir}/ndctl
ndctl_monitorconf=monitor.conf
AC_SUBST([ndctl_monitorconfdir])
@@ -161,6 +162,24 @@ AC_SUBST([ndctl_monitorconf])
AC_DEFINE_UNQUOTED(NDCTL_CONF_FILE, ["$ndctl_monitorconfdir/$ndctl_monitorconf"],
[default ndctl monitor conf path])
+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"])
+ndctl_keysdir=${sysconfdir}/ndctl/keys
+AC_SUBST([ndctl_keysdir])
+AC_DEFINE_UNQUOTED(NDCTL_KEYS_DIR, ["$ndctl_keysdir"],
+ [default ndctl keys path])
+
my_CFLAGS="\
-Wall \
-Wchar-subscripts \
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 ff01e068..120941a4 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/builtin.h b/ndctl/builtin.h
index 17300df0..231fda25 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -32,4 +32,6 @@ int cmd_bat(int argc, const char **argv, struct ndctl_ctx *ctx);
#endif
int cmd_update_firmware(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_inject_smart(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_passphrase_setup(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_passphrase_update(int argc, const char **argv, struct ndctl_ctx *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index c717beeb..1ab6b29f 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -40,6 +40,20 @@ struct action_context {
struct update_context update;
};
+static struct parameters {
+ const char *bus;
+ const char *outfile;
+ const char *infile;
+ const char *labelversion;
+ const char *key_path;
+ 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 +838,31 @@ 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,
+ param.key_path);
+}
+
+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,
+ param.key_path);
+}
static int __action_init(struct ndctl_dimm *dimm,
enum ndctl_namespace_version version, int chk_only)
@@ -925,6 +953,12 @@ OPT_BOOLEAN('f', "force", ¶m.force, \
OPT_STRING('V', "label-version", ¶m.labelversion, "version-number", \
"namespace label specification version (default: 1.1)")
+#define KEY_OPTIONS() \
+OPT_STRING('m', "master-key", ¶m.master_key, "<key_type>:<key_name>", \
+ "master key for security"), \
+OPT_FILENAME('p', "key-path", ¶m.key_path, "key-path", \
+ "override the default key path")
+
static const struct option read_options[] = {
BASE_OPTIONS(),
READ_OPTIONS(),
@@ -954,6 +988,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, struct ndctl_ctx *ctx,
int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
const struct option *options, const char *usage)
@@ -1024,6 +1064,13 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *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);
@@ -1042,6 +1089,9 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx,
goto out;
}
+ if (!param.key_path)
+ param.key_path = strdup(NDCTL_KEYS_DIR);
+
rc = 0;
err = 0;
count = 0;
@@ -1181,3 +1231,25 @@ int cmd_update_firmware(int argc, const char **argv, struct ndctl_ctx *ctx)
count > 1 ? "s" : "");
return count >= 0 ? 0 : EXIT_FAILURE;
}
+
+int cmd_passphrase_update(int argc, const char **argv, struct ndctl_ctx *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, struct ndctl_ctx *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 e03135d9..b64c9568 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -624,3 +624,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..de18ddf7
--- /dev/null
+++ b/ndctl/lib/keys.c
@@ -0,0 +1,390 @@
+// 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"
+
+static int get_key_path(struct ndctl_dimm *dimm, char *path,
+ enum ndctl_key_type key_type, const char *keypath)
+{
+ struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+ char hostname[HOST_NAME_MAX];
+ int rc;
+
+ rc = gethostname(hostname, HOST_NAME_MAX);
+ 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",
+ keypath,
+ ndctl_dimm_get_unique_id(dimm),
+ hostname);
+ } else {
+ rc = sprintf(path, "%s/nvdimm_%s_%s.blob",
+ keypath,
+ ndctl_dimm_get_unique_id(dimm),
+ hostname);
+ }
+
+ if (rc < 0) {
+ 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, "error setting key description: %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, "Failed 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)
+{
+ char desc[ND_KEY_DESC_SIZE];
+ int rc;
+
+ rc = get_key_desc(dimm, desc, key_type);
+ if (rc < 0)
+ 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, const char *keypath)
+{
+ struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+ char desc[ND_KEY_DESC_SIZE];
+ char path[PATH_MAX];
+ char cmd[ND_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, "Error: key already present in user keyring\n");
+ return -EEXIST;
+ }
+
+ rc = get_key_path(dimm, path, ND_USER_KEY, keypath);
+ if (rc < 0)
+ return rc;
+
+ rc = stat(path, &st);
+ if (rc == 0) {
+ err(ctx, "%s already exists!\n", path);
+ return -EEXIST;
+ }
+
+ 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, "Unable to open file %s: %s\n",
+ path, strerror(errno));
+ free(buffer);
+ return rc;
+ }
+
+ wrote = fwrite(buffer, 1, size, fp);
+ if (wrote != size) {
+ if (wrote == -1)
+ rc = -errno;
+ else
+ rc = -EIO;
+ err(ctx, "Failed to write to %s: %s\n",
+ path, strerror(-rc));
+ 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, const char *keypath)
+{
+ struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+ key_serial_t key;
+ char desc[ND_KEY_DESC_SIZE];
+ char path[PATH_MAX];
+ 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, keypath);
+ 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,
+ const char *keypath)
+{
+ struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+ int rc;
+ key_serial_t key;
+ char old_path[PATH_MAX];
+ char new_path[PATH_MAX];
+
+ 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, keypath);
+ if (rc < 0)
+ return rc;
+
+ rc = get_key_path(dimm, new_path, ND_USER_OLD_KEY, keypath);
+ if (rc < 0)
+ return rc;
+
+ rc = rename(old_path, new_path);
+ if (rc < 0) {
+ err(ctx, "rename failed from %s to %s: %s\n",
+ old_path, new_path, strerror(errno));
+ return -errno;
+ }
+
+ return dimm_load_key(dimm, ND_USER_OLD_KEY, keypath);
+}
+
+static int dimm_remove_key(struct ndctl_dimm *dimm,
+ enum ndctl_key_type key_type, const char *keypath)
+{
+ struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+ key_serial_t key;
+ char path[PATH_MAX];
+ 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, keypath);
+ if (rc < 0)
+ return rc;
+
+ rc = unlink(path);
+ if (rc < 0) {
+ err(ctx, "delete file %s failed: %s\n",
+ path, strerror(errno));
+ return -errno;
+ }
+
+ return 0;
+}
+
+NDCTL_EXPORT int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
+ const char *master, const char *keypath)
+{
+ key_serial_t key;
+ int rc;
+
+ key = dimm_create_key(dimm, master, keypath);
+ if (key < 0)
+ return key;
+
+ rc = ndctl_dimm_update_passphrase(dimm, 0, key);
+ if (rc < 0) {
+ dimm_remove_key(dimm, ND_USER_KEY, keypath);
+ return rc;
+ }
+
+ return 0;
+}
+
+NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
+ const char *master, const char *keypath)
+{
+ 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 new key
+ * 5. remove old key
+ * 6. remove old key blob
+ */
+ old_key = move_key_to_old(dimm, keypath);
+ if (old_key < 0)
+ return old_key;
+
+ new_key = dimm_create_key(dimm, master, keypath);
+ /* need to create new key here */
+ if (new_key < 0) {
+ new_key = dimm_load_key(dimm, ND_USER_KEY, keypath);
+ 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, keypath);
+ 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 4255252c..50ca68f9 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_UUID
#include <uuid/uuid.h>
@@ -681,6 +682,10 @@ 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_KEY_DESC_LEN 22
+#define ND_KEY_DESC_PREFIX 7
+
enum nd_security_state {
ND_SECURITY_INVALID = -1,
ND_SECURITY_UNSUPPORTED = 0,
@@ -693,6 +698,36 @@ 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,
+ const char *keypath);
+int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master,
+ const char *keypath);
+#else
+static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
+ const char *master, const char *keypath)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
+ const char *master, const char *keypath)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
+#define ND_KEY_DESC_SIZE 128
+#define ND_KEY_CMD_SIZE 128
#ifdef __cplusplus
} /* extern "C" */
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index b01594e0..9d109b34 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] 23+ messages in thread* Re: [PATCH v8 02/12] ndctl: add passphrase update to ndctl
2019-01-14 20:06 ` [PATCH v8 02/12] ndctl: add passphrase update to ndctl Dave Jiang
@ 2019-01-16 1:56 ` Dan Williams
2019-01-16 17:43 ` Verma, Vishal L
0 siblings, 1 reply; 23+ messages in thread
From: Dan Williams @ 2019-01-16 1:56 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
Some comments below...
On Mon, Jan 14, 2019 at 12:06 PM Dave Jiang <dave.jiang@intel.com> 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 | 42 ++
> Documentation/ndctl/ndctl-update-passphrase.txt | 38 ++
> configure.ac | 19 +
> ndctl.spec.in | 2
> ndctl/Makefile.am | 3
> ndctl/builtin.h | 2
> ndctl/dimm.c | 94 +++++-
> ndctl/lib/Makefile.am | 8
> ndctl/lib/dimm.c | 39 ++
> ndctl/lib/keys.c | 390 +++++++++++++++++++++++
> ndctl/lib/libndctl.sym | 4
> ndctl/libndctl.h | 35 ++
> ndctl/ndctl.c | 2
> 14 files changed, 669 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..c14a206c
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
> @@ -0,0 +1,42 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +ndctl-enable-passphrase(1)
> +==========================
> +
> +NAME
> +----
> +ndctl-enable-passphrase - enable the security passphrase for a NVDIMM
*an NVDIMM
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl enable-passphrase' <dimm> [<options>]
Is this true, there are no other required options besides the nmem
device? No support for more than one nmem at a time?
> +
> +DESCRIPTION
> +-----------
> +Provide a command to enable the security passphrase for the NVDIMM.
No need to say "Provide a command" that's assumed for a man page named
after a binary.
So I'd say "Enable the security passphrase for one or more NVDIMMs.
> +It is expected that the master key has already been loaded into the user
Without listing the key-id as a required parameter it's not clear what
the usage should be.
> +key ring. The encrypted key blobs will be created in /etc/nvdimm directory
Is this stale, should be /etc/ndctl/keys, right? Given the directory
is changeable by the build system this source file should be built
with the key directory as a variable.
> +with the file name of "nvdimm-<hostname>-<dimm unique id>.blob".
That's quite a bit to type out? Is there a command to discover this
file name, or can we provide a short-hand?
> +
> +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.
I feel like this is missing a create-passphrase step, and/or that
there needs to be an example in the man page. The man page should show
sohow to create the pre-requisite material, or reference another
command that will handle the details. I don't think the user interface
should ever expose "nvdimm-<hostname>-<dimm unique id>.blob" as
something the user needs to type... if at all possible. Maybe a global
"set-kek" command to write out the key-encryption-key to a
configuration file that enable-passphrase can consult by default
rather than require it to be passed to every enable-passphrase
invocation?
> Do not touch the /etc/nvdimm
> +directory and let ndctl manage the keys, unless you know what you are doing.
I think the "unless you know what you are doing." sentiment goes
without saying for a man page. If anything I'd ship a README file in
/etc/ndctl/keys if you're worried about manual edits to that
directory.
> +
> +OPTIONS
> +-------
> +<dimm>::
> +include::xable-dimm-options.txt[]
> +
> +-m::
> +--master=::
> + 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
"master" is ambiguous when we have master passphrases and master keys.
Let's call it the KEK (key-encryption-key) and reserve "master" for
"master passphrase".
> +-p::
> +--key-path=::
> + Path to where key related files resides. This parameter is optional
> + and the default is set to /etc/ndctl/keys.
Is this flexibility needed? Seems like something that can be omitted
unless/until a need arises.
> +
> +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..dd6e4e4e
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-update-passphrase.txt
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +ndctl-update-passphrase(1)
> +==========================
> +
> +NAME
> +----
> +ndctl-update-passphrase - update the security passphrase for a NVDIMM
*an NVDIMM
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl update-passphrase' <dimm> [<options>]
Same comment about required options.
> +
> +DESCRIPTION
> +-----------
> +Provide a command to update the security key for NVDIMM.
Same comment about "Provide a command"
> +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.
> +
> +-p::
> +--key-path=::
> + Path to where key related files resides. This parameter is optional
> + and the default is set to /etc/ndctl/keys.
Same comment about an example and the need for key-path.
> +
> +include::../copyright.txt[]
> diff --git a/configure.ac b/configure.ac
> index aa07ec7b..22efc871 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -154,6 +154,7 @@ fi
> AC_SUBST([systemd_unitdir])
> AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
>
> +
> ndctl_monitorconfdir=${sysconfdir}/ndctl
> ndctl_monitorconf=monitor.conf
> AC_SUBST([ndctl_monitorconfdir])
> @@ -161,6 +162,24 @@ AC_SUBST([ndctl_monitorconf])
> AC_DEFINE_UNQUOTED(NDCTL_CONF_FILE, ["$ndctl_monitorconfdir/$ndctl_monitorconf"],
> [default ndctl monitor conf path])
>
> +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"])
> +ndctl_keysdir=${sysconfdir}/ndctl/keys
> +AC_SUBST([ndctl_keysdir])
> +AC_DEFINE_UNQUOTED(NDCTL_KEYS_DIR, ["$ndctl_keysdir"],
> + [default ndctl keys path])
My bad, apparently AC_DEFINE_UNQUOTED falls over if $ndctl_keysdir is
made up of components that need further expansion. Instead add
NDCTL_KEYS_DIR to the new autogenerated ndctl/config.h file. See this
pending patch where I killed off AC_DEFINE_UNQUOTED usage:
https://patchwork.kernel.org/patch/10763963/
> +
> my_CFLAGS="\
> -Wall \
> -Wchar-subscripts \
> 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 ff01e068..120941a4 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
This should be conditional on whether ndctl was built with keyutils support.
...reading ahead in the patch I don't think ndctl actually has a
direct dependency on keyutils, right? It's abstracted behind the
libndctl routines, so do we need this?
[..]
> diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
> index e03135d9..b64c9568 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -624,3 +624,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;
> +}
I think the need for this goes away when ndctl_dimm_get_security()
returns the state directly.
[..]
> diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
> index b01594e0..9d109b34 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 } },
Maybe call the command setup-passphrase. All the other enable commands
are just toggling dynamic state, this one is creating persistent
key-material.
> + { "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 [flat|nested] 23+ messages in thread* Re: [PATCH v8 02/12] ndctl: add passphrase update to ndctl
2019-01-16 1:56 ` Dan Williams
@ 2019-01-16 17:43 ` Verma, Vishal L
2019-01-16 17:47 ` Dave Jiang
0 siblings, 1 reply; 23+ messages in thread
From: Verma, Vishal L @ 2019-01-16 17:43 UTC (permalink / raw)
To: Williams, Dan J, Jiang, Dave; +Cc: linux-nvdimm
On Tue, 2019-01-15 at 17:56 -0800, Dan Williams wrote:
> Some comments below...
>
> On Mon, Jan 14, 2019 at 12:06 PM Dave Jiang <dave.jiang@intel.com> 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 | 42 ++
> > Documentation/ndctl/ndctl-update-passphrase.txt | 38 ++
> > configure.ac | 19 +
> > ndctl.spec.in | 2
> > ndctl/Makefile.am | 3
> > ndctl/builtin.h | 2
> > ndctl/dimm.c | 94 +++++-
> > ndctl/lib/Makefile.am | 8
> > ndctl/lib/dimm.c | 39 ++
> > ndctl/lib/keys.c | 390 +++++++++++++++++++++++
> > ndctl/lib/libndctl.sym | 4
> > ndctl/libndctl.h | 35 ++
> > ndctl/ndctl.c | 2
> > 14 files changed, 669 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..c14a206c
> > --- /dev/null
> > +++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
> > @@ -0,0 +1,42 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +ndctl-enable-passphrase(1)
> > +==========================
> > +
> > +NAME
> > +----
> > +ndctl-enable-passphrase - enable the security passphrase for a NVDIMM
>
> *an NVDIMM
>
> > +
> > +SYNOPSIS
> > +--------
> > +[verse]
> > +'ndctl enable-passphrase' <dimm> [<options>]
>
> Is this true, there are no other required options besides the nmem
> device? No support for more than one nmem at a time?
>
> > +
> > +DESCRIPTION
> > +-----------
> > +Provide a command to enable the security passphrase for the NVDIMM.
>
> No need to say "Provide a command" that's assumed for a man page named
> after a binary.
>
> So I'd say "Enable the security passphrase for one or more NVDIMMs.
>
> > +It is expected that the master key has already been loaded into the user
>
> Without listing the key-id as a required parameter it's not clear what
> the usage should be.
>
> > +key ring. The encrypted key blobs will be created in /etc/nvdimm directory
>
> Is this stale, should be /etc/ndctl/keys, right? Given the directory
> is changeable by the build system this source file should be built
> with the key directory as a variable.
>
> > +with the file name of "nvdimm-<hostname>-<dimm unique id>.blob".
>
> That's quite a bit to type out? Is there a command to discover this
> file name, or can we provide a short-hand?
I also noticed the actual file created was of the format:
"nvdimm-<dimm unique id>-<hostname>.blob"
One of them should be made consistent with the other..
>
> > +
> > +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.
>
> I feel like this is missing a create-passphrase step, and/or that
> there needs to be an example in the man page. The man page should show
> sohow to create the pre-requisite material, or reference another
> command that will handle the details. I don't think the user interface
> should ever expose "nvdimm-<hostname>-<dimm unique id>.blob" as
> something the user needs to type... if at all possible. Maybe a global
> "set-kek" command to write out the key-encryption-key to a
> configuration file that enable-passphrase can consult by default
> rather than require it to be passed to every enable-passphrase
> invocation?
>
> > Do not touch the /etc/nvdimm
> > +directory and let ndctl manage the keys, unless you know what you are doing.
>
> I think the "unless you know what you are doing." sentiment goes
> without saying for a man page. If anything I'd ship a README file in
> /etc/ndctl/keys if you're worried about manual edits to that
> directory.
>
> > +
> > +OPTIONS
> > +-------
> > +<dimm>::
> > +include::xable-dimm-options.txt[]
> > +
> > +-m::
> > +--master=::
> > + 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
>
> "master" is ambiguous when we have master passphrases and master keys.
> Let's call it the KEK (key-encryption-key) and reserve "master" for
> "master passphrase".
>
> > +-p::
> > +--key-path=::
> > + Path to where key related files resides. This parameter is optional
> > + and the default is set to /etc/ndctl/keys.
>
> Is this flexibility needed? Seems like something that can be omitted
> unless/until a need arises.
>
> > +
> > +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..dd6e4e4e
> > --- /dev/null
> > +++ b/Documentation/ndctl/ndctl-update-passphrase.txt
> > @@ -0,0 +1,38 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +ndctl-update-passphrase(1)
> > +==========================
> > +
> > +NAME
> > +----
> > +ndctl-update-passphrase - update the security passphrase for a NVDIMM
>
> *an NVDIMM
>
> > +
> > +SYNOPSIS
> > +--------
> > +[verse]
> > +'ndctl update-passphrase' <dimm> [<options>]
>
> Same comment about required options.
>
> > +
> > +DESCRIPTION
> > +-----------
> > +Provide a command to update the security key for NVDIMM.
>
> Same comment about "Provide a command"
>
> > +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.
> > +
> > +-p::
> > +--key-path=::
> > + Path to where key related files resides. This parameter is optional
> > + and the default is set to /etc/ndctl/keys.
>
> Same comment about an example and the need for key-path.
>
> > +
> > +include::../copyright.txt[]
> > diff --git a/configure.ac b/configure.ac
> > index aa07ec7b..22efc871 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -154,6 +154,7 @@ fi
> > AC_SUBST([systemd_unitdir])
> > AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
> >
> > +
> > ndctl_monitorconfdir=${sysconfdir}/ndctl
> > ndctl_monitorconf=monitor.conf
> > AC_SUBST([ndctl_monitorconfdir])
> > @@ -161,6 +162,24 @@ AC_SUBST([ndctl_monitorconf])
> > AC_DEFINE_UNQUOTED(NDCTL_CONF_FILE, ["$ndctl_monitorconfdir/$ndctl_monitorconf"],
> > [default ndctl monitor conf path])
> >
> > +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"])
> > +ndctl_keysdir=${sysconfdir}/ndctl/keys
> > +AC_SUBST([ndctl_keysdir])
> > +AC_DEFINE_UNQUOTED(NDCTL_KEYS_DIR, ["$ndctl_keysdir"],
> > + [default ndctl keys path])
>
> My bad, apparently AC_DEFINE_UNQUOTED falls over if $ndctl_keysdir is
> made up of components that need further expansion. Instead add
> NDCTL_KEYS_DIR to the new autogenerated ndctl/config.h file. See this
> pending patch where I killed off AC_DEFINE_UNQUOTED usage:
> https://patchwork.kernel.org/patch/10763963/
>
> > +
> > my_CFLAGS="\
> > -Wall \
> > -Wchar-subscripts \
> > 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 ff01e068..120941a4 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
>
> This should be conditional on whether ndctl was built with keyutils support.
>
> ...reading ahead in the patch I don't think ndctl actually has a
> direct dependency on keyutils, right? It's abstracted behind the
> libndctl routines, so do we need this?
>
> [..]
> > diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
> > index e03135d9..b64c9568 100644
> > --- a/ndctl/lib/dimm.c
> > +++ b/ndctl/lib/dimm.c
> > @@ -624,3 +624,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;
> > +}
>
> I think the need for this goes away when ndctl_dimm_get_security()
> returns the state directly.
>
> [..]
> > diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
> > index b01594e0..9d109b34 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 } },
>
> Maybe call the command setup-passphrase. All the other enable commands
> are just toggling dynamic state, this one is creating persistent
> key-material.
>
> > + { "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 [flat|nested] 23+ messages in thread* Re: [PATCH v8 02/12] ndctl: add passphrase update to ndctl
2019-01-16 17:43 ` Verma, Vishal L
@ 2019-01-16 17:47 ` Dave Jiang
0 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2019-01-16 17:47 UTC (permalink / raw)
To: Verma, Vishal L, Williams, Dan J; +Cc: linux-nvdimm
On 1/16/19 10:43 AM, Verma, Vishal L wrote:
>
> On Tue, 2019-01-15 at 17:56 -0800, Dan Williams wrote:
>> Some comments below...
>>
>> On Mon, Jan 14, 2019 at 12:06 PM Dave Jiang <dave.jiang@intel.com> 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 | 42 ++
>>> Documentation/ndctl/ndctl-update-passphrase.txt | 38 ++
>>> configure.ac | 19 +
>>> ndctl.spec.in | 2
>>> ndctl/Makefile.am | 3
>>> ndctl/builtin.h | 2
>>> ndctl/dimm.c | 94 +++++-
>>> ndctl/lib/Makefile.am | 8
>>> ndctl/lib/dimm.c | 39 ++
>>> ndctl/lib/keys.c | 390 +++++++++++++++++++++++
>>> ndctl/lib/libndctl.sym | 4
>>> ndctl/libndctl.h | 35 ++
>>> ndctl/ndctl.c | 2
>>> 14 files changed, 669 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..c14a206c
>>> --- /dev/null
>>> +++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
>>> @@ -0,0 +1,42 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +ndctl-enable-passphrase(1)
>>> +==========================
>>> +
>>> +NAME
>>> +----
>>> +ndctl-enable-passphrase - enable the security passphrase for a NVDIMM
>>
>> *an NVDIMM
>>
>>> +
>>> +SYNOPSIS
>>> +--------
>>> +[verse]
>>> +'ndctl enable-passphrase' <dimm> [<options>]
>>
>> Is this true, there are no other required options besides the nmem
>> device? No support for more than one nmem at a time?
>>
>>> +
>>> +DESCRIPTION
>>> +-----------
>>> +Provide a command to enable the security passphrase for the NVDIMM.
>>
>> No need to say "Provide a command" that's assumed for a man page named
>> after a binary.
>>
>> So I'd say "Enable the security passphrase for one or more NVDIMMs.
>>
>>> +It is expected that the master key has already been loaded into the user
>>
>> Without listing the key-id as a required parameter it's not clear what
>> the usage should be.
>>
>>> +key ring. The encrypted key blobs will be created in /etc/nvdimm directory
>>
>> Is this stale, should be /etc/ndctl/keys, right? Given the directory
>> is changeable by the build system this source file should be built
>> with the key directory as a variable.
>>
>>> +with the file name of "nvdimm-<hostname>-<dimm unique id>.blob".
>>
>> That's quite a bit to type out? Is there a command to discover this
>> file name, or can we provide a short-hand?
>
> I also noticed the actual file created was of the format:
>
> "nvdimm-<dimm unique id>-<hostname>.blob"
>
> One of them should be made consistent with the other..
I'll fix the documentation. I changed the format for ease of parsing
sometimes during the development and missed the update to the
documentation here.
>
>>
>>> +
>>> +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.
>>
>> I feel like this is missing a create-passphrase step, and/or that
>> there needs to be an example in the man page. The man page should show
>> sohow to create the pre-requisite material, or reference another
>> command that will handle the details. I don't think the user interface
>> should ever expose "nvdimm-<hostname>-<dimm unique id>.blob" as
>> something the user needs to type... if at all possible. Maybe a global
>> "set-kek" command to write out the key-encryption-key to a
>> configuration file that enable-passphrase can consult by default
>> rather than require it to be passed to every enable-passphrase
>> invocation?
>>
>>> Do not touch the /etc/nvdimm
>>> +directory and let ndctl manage the keys, unless you know what you are doing.
>>
>> I think the "unless you know what you are doing." sentiment goes
>> without saying for a man page. If anything I'd ship a README file in
>> /etc/ndctl/keys if you're worried about manual edits to that
>> directory.
>>
>>> +
>>> +OPTIONS
>>> +-------
>>> +<dimm>::
>>> +include::xable-dimm-options.txt[]
>>> +
>>> +-m::
>>> +--master=::
>>> + 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
>>
>> "master" is ambiguous when we have master passphrases and master keys.
>> Let's call it the KEK (key-encryption-key) and reserve "master" for
>> "master passphrase".
>>
>>> +-p::
>>> +--key-path=::
>>> + Path to where key related files resides. This parameter is optional
>>> + and the default is set to /etc/ndctl/keys.
>>
>> Is this flexibility needed? Seems like something that can be omitted
>> unless/until a need arises.
>>
>>> +
>>> +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..dd6e4e4e
>>> --- /dev/null
>>> +++ b/Documentation/ndctl/ndctl-update-passphrase.txt
>>> @@ -0,0 +1,38 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +ndctl-update-passphrase(1)
>>> +==========================
>>> +
>>> +NAME
>>> +----
>>> +ndctl-update-passphrase - update the security passphrase for a NVDIMM
>>
>> *an NVDIMM
>>
>>> +
>>> +SYNOPSIS
>>> +--------
>>> +[verse]
>>> +'ndctl update-passphrase' <dimm> [<options>]
>>
>> Same comment about required options.
>>
>>> +
>>> +DESCRIPTION
>>> +-----------
>>> +Provide a command to update the security key for NVDIMM.
>>
>> Same comment about "Provide a command"
>>
>>> +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.
>>> +
>>> +-p::
>>> +--key-path=::
>>> + Path to where key related files resides. This parameter is optional
>>> + and the default is set to /etc/ndctl/keys.
>>
>> Same comment about an example and the need for key-path.
>>
>>> +
>>> +include::../copyright.txt[]
>>> diff --git a/configure.ac b/configure.ac
>>> index aa07ec7b..22efc871 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -154,6 +154,7 @@ fi
>>> AC_SUBST([systemd_unitdir])
>>> AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
>>>
>>> +
>>> ndctl_monitorconfdir=${sysconfdir}/ndctl
>>> ndctl_monitorconf=monitor.conf
>>> AC_SUBST([ndctl_monitorconfdir])
>>> @@ -161,6 +162,24 @@ AC_SUBST([ndctl_monitorconf])
>>> AC_DEFINE_UNQUOTED(NDCTL_CONF_FILE, ["$ndctl_monitorconfdir/$ndctl_monitorconf"],
>>> [default ndctl monitor conf path])
>>>
>>> +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"])
>>> +ndctl_keysdir=${sysconfdir}/ndctl/keys
>>> +AC_SUBST([ndctl_keysdir])
>>> +AC_DEFINE_UNQUOTED(NDCTL_KEYS_DIR, ["$ndctl_keysdir"],
>>> + [default ndctl keys path])
>>
>> My bad, apparently AC_DEFINE_UNQUOTED falls over if $ndctl_keysdir is
>> made up of components that need further expansion. Instead add
>> NDCTL_KEYS_DIR to the new autogenerated ndctl/config.h file. See this
>> pending patch where I killed off AC_DEFINE_UNQUOTED usage:
>> https://patchwork.kernel.org/patch/10763963/
>>
>>> +
>>> my_CFLAGS="\
>>> -Wall \
>>> -Wchar-subscripts \
>>> 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 ff01e068..120941a4 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
>>
>> This should be conditional on whether ndctl was built with keyutils support.
>>
>> ...reading ahead in the patch I don't think ndctl actually has a
>> direct dependency on keyutils, right? It's abstracted behind the
>> libndctl routines, so do we need this?
>>
>> [..]
>>> diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
>>> index e03135d9..b64c9568 100644
>>> --- a/ndctl/lib/dimm.c
>>> +++ b/ndctl/lib/dimm.c
>>> @@ -624,3 +624,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;
>>> +}
>>
>> I think the need for this goes away when ndctl_dimm_get_security()
>> returns the state directly.
>>
>> [..]
>>> diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
>>> index b01594e0..9d109b34 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 } },
>>
>> Maybe call the command setup-passphrase. All the other enable commands
>> are just toggling dynamic state, this one is creating persistent
>> key-material.
>>
>>> + { "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 [flat|nested] 23+ messages in thread
* [PATCH v8 03/12] ndctl: add disable security support
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
2019-01-14 20:06 ` [PATCH v8 01/12] ndctl: add support for display security state Dave Jiang
2019-01-14 20:06 ` [PATCH v8 02/12] ndctl: add passphrase update to ndctl Dave Jiang
@ 2019-01-14 20:06 ` Dave Jiang
2019-01-16 4:41 ` Dan Williams
2019-01-14 20:06 ` [PATCH v8 04/12] ndctl: add support for freeze security Dave Jiang
` (8 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:06 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 | 33 +++++++++++++++++++
ndctl/builtin.h | 1 +
ndctl/dimm.c | 38 ++++++++++++++++++++--
ndctl/lib/dimm.c | 9 +++++
ndctl/lib/keys.c | 29 +++++++++++++++++
ndctl/lib/libndctl.sym | 2 +
ndctl/libndctl.h | 8 +++++
ndctl/ndctl.c | 1 +
9 files changed, 120 insertions(+), 4 deletions(-)
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..49f25898
--- /dev/null
+++ b/Documentation/ndctl/ndctl-disable-passphrase.txt
@@ -0,0 +1,33 @@
+// 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.
+
+Search the user key ring for the associated NVDIMM. If not found,
+attempt to load the key blob from the default location. After disabling
+the passphrase, remove the key and the key blob.
+
+OPTIONS
+-------
+<dimm>::
+include::xable-dimm-options.txt[]
+
+-p::
+--key-path=::
+ Path to where key related files resides. This parameter is optional
+ and the default is set to /etc/ndctl/keys.
+
+include::../copyright.txt[]
diff --git a/ndctl/builtin.h b/ndctl/builtin.h
index 231fda25..821ea690 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -34,4 +34,5 @@ int cmd_update_firmware(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_inject_smart(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_passphrase_setup(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_passphrase_update(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_disable_passphrase(int argc, const char **argv, struct ndctl_ctx *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 1ab6b29f..4f0466a1 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -864,6 +864,18 @@ static int action_key_update(struct ndctl_dimm *dimm,
param.key_path);
}
+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, param.key_path);
+}
+
static int __action_init(struct ndctl_dimm *dimm,
enum ndctl_namespace_version version, int chk_only)
{
@@ -953,12 +965,14 @@ OPT_BOOLEAN('f', "force", ¶m.force, \
OPT_STRING('V', "label-version", ¶m.labelversion, "version-number", \
"namespace label specification version (default: 1.1)")
-#define KEY_OPTIONS() \
-OPT_STRING('m', "master-key", ¶m.master_key, "<key_type>:<key_name>", \
- "master key for security"), \
+#define KEY_BASE_OPTIONS() \
OPT_FILENAME('p', "key-path", ¶m.key_path, "key-path", \
"override the default key path")
+#define KEY_OPTIONS() \
+OPT_STRING('m', "master-key", ¶m.master_key, "<key_type>:<key_name>", \
+ "master key for security")
+
static const struct option read_options[] = {
BASE_OPTIONS(),
READ_OPTIONS(),
@@ -988,8 +1002,15 @@ static const struct option init_options[] = {
OPT_END(),
};
+static const struct option key_base_options[] = {
+ BASE_OPTIONS(),
+ KEY_BASE_OPTIONS(),
+ OPT_END(),
+};
+
static const struct option key_options[] = {
BASE_OPTIONS(),
+ KEY_BASE_OPTIONS(),
KEY_OPTIONS(),
OPT_END(),
};
@@ -1253,3 +1274,14 @@ int cmd_passphrase_setup(int argc, const char **argv, struct ndctl_ctx *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,
+ key_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 b64c9568..076ccbf6 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -663,3 +663,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 de18ddf7..6dfbfd49 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -388,3 +388,32 @@ NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
return 0;
}
+
+NDCTL_EXPORT int ndctl_dimm_disable_key(struct ndctl_dimm *dimm,
+ const char *keypath)
+{
+ 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, keypath);
+ 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, keypath);
+ 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 50ca68f9..b1192960 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -701,6 +701,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,
@@ -712,6 +713,7 @@ int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master,
const char *keypath);
int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master,
const char *keypath);
+int ndctl_dimm_disable_key(struct ndctl_dimm *dimm, const char *keypath);
#else
static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
const char *master, const char *keypath)
@@ -724,6 +726,12 @@ static inline int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
{
return -EOPNOTSUPP;
}
+
+static inline int ndctl_dimm_disable_key(struct ndctl_dimm *dimm,
+ const char *keypath)
+{
+ return -EOPNOTSUPP;
+}
#endif
#define ND_KEY_DESC_SIZE 128
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 9d109b34..21f1b834 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] 23+ messages in thread* Re: [PATCH v8 03/12] ndctl: add disable security support
2019-01-14 20:06 ` [PATCH v8 03/12] ndctl: add disable security support Dave Jiang
@ 2019-01-16 4:41 ` Dan Williams
0 siblings, 0 replies; 23+ messages in thread
From: Dan Williams @ 2019-01-16 4:41 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Mon, Jan 14, 2019 at 12:07 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> 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 | 33 +++++++++++++++++++
> ndctl/builtin.h | 1 +
> ndctl/dimm.c | 38 ++++++++++++++++++++--
> ndctl/lib/dimm.c | 9 +++++
> ndctl/lib/keys.c | 29 +++++++++++++++++
> ndctl/lib/libndctl.sym | 2 +
> ndctl/libndctl.h | 8 +++++
> ndctl/ndctl.c | 1 +
> 9 files changed, 120 insertions(+), 4 deletions(-)
> 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..49f25898
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-disable-passphrase.txt
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +ndctl-disable-passphrase(1)
> +===========================
> +
> +NAME
> +----
> +ndctl-disable-passphrase - disabling passphrase for an NVDIMM
How about, "Stop a DIMM from locking at power-loss and requiring a
passphrase to access media"
Similar to how enable-passphrase seems like an awkward name given how
much work it is doing, how about calling this command
"remove-passphrase"?
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl disable-passphrase' <dimm> [<options>]
In the source it states:
"ndctl disable-passphrase <nmem0> [<nmem1>..<nmemN>] [<options>]"
...that multiple DIMMs are supported. Like patch2 it would be nice if
the optional options were indeed optional, or not required altogether
if the key-encryption-key details are stored in a configuration file.
> +
> +DESCRIPTION
> +-----------
> +Provide a generic interface for disabling passphrase for NVDIMM.
Drop this sentence since this is implied.
> +
> +Search the user key ring for the associated NVDIMM.
"NVDIMM key", perhaps?
> If not found,
> +attempt to load the key blob from the default location. After disabling
Maybe drop the "default location" qualifier since I'm not sure what
the use case is for allowing non-default key material locations. Just
consider /etc/ndctl/keys like libnvdimm-sysfs. There's only one path,
take it or leave it. If a user wants to do a custom key management
scheme they can just avoid ndctl altogether.
> +the passphrase, remove the key and the key blob.
Ah, see, it *is* a "remove-passphrase" command.
> +
> +OPTIONS
> +-------
> +<dimm>::
> +include::xable-dimm-options.txt[]
> +
> +-p::
> +--key-path=::
> + Path to where key related files resides. This parameter is optional
> + and the default is set to /etc/ndctl/keys.
> +
> +include::../copyright.txt[]
> diff --git a/ndctl/builtin.h b/ndctl/builtin.h
> index 231fda25..821ea690 100644
> --- a/ndctl/builtin.h
> +++ b/ndctl/builtin.h
> @@ -34,4 +34,5 @@ int cmd_update_firmware(int argc, const char **argv, struct ndctl_ctx *ctx);
> int cmd_inject_smart(int argc, const char **argv, struct ndctl_ctx *ctx);
> int cmd_passphrase_setup(int argc, const char **argv, struct ndctl_ctx *ctx);
> int cmd_passphrase_update(int argc, const char **argv, struct ndctl_ctx *ctx);
> +int cmd_disable_passphrase(int argc, const char **argv, struct ndctl_ctx *ctx);
> #endif /* _NDCTL_BUILTIN_H_ */
> diff --git a/ndctl/dimm.c b/ndctl/dimm.c
> index 1ab6b29f..4f0466a1 100644
> --- a/ndctl/dimm.c
> +++ b/ndctl/dimm.c
> @@ -864,6 +864,18 @@ static int action_key_update(struct ndctl_dimm *dimm,
> param.key_path);
> }
>
> +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, param.key_path);
> +}
> +
> static int __action_init(struct ndctl_dimm *dimm,
> enum ndctl_namespace_version version, int chk_only)
> {
> @@ -953,12 +965,14 @@ OPT_BOOLEAN('f', "force", ¶m.force, \
> OPT_STRING('V', "label-version", ¶m.labelversion, "version-number", \
> "namespace label specification version (default: 1.1)")
>
> -#define KEY_OPTIONS() \
> -OPT_STRING('m', "master-key", ¶m.master_key, "<key_type>:<key_name>", \
> - "master key for security"), \
> +#define KEY_BASE_OPTIONS() \
> OPT_FILENAME('p', "key-path", ¶m.key_path, "key-path", \
> "override the default key path")
>
> +#define KEY_OPTIONS() \
> +OPT_STRING('m', "master-key", ¶m.master_key, "<key_type>:<key_name>", \
> + "master key for security")
> +
Same key-encryption-key recommendation as patch 2.
> static const struct option read_options[] = {
> BASE_OPTIONS(),
> READ_OPTIONS(),
> @@ -988,8 +1002,15 @@ static const struct option init_options[] = {
> OPT_END(),
> };
>
> +static const struct option key_base_options[] = {
> + BASE_OPTIONS(),
> + KEY_BASE_OPTIONS(),
> + OPT_END(),
> +};
> +
> static const struct option key_options[] = {
> BASE_OPTIONS(),
> + KEY_BASE_OPTIONS(),
> KEY_OPTIONS(),
> OPT_END(),
> };
> @@ -1253,3 +1274,14 @@ int cmd_passphrase_setup(int argc, const char **argv, struct ndctl_ctx *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,
> + key_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 b64c9568..076ccbf6 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -663,3 +663,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 de18ddf7..6dfbfd49 100644
> --- a/ndctl/lib/keys.c
> +++ b/ndctl/lib/keys.c
> @@ -388,3 +388,32 @@ NDCTL_EXPORT int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
>
> return 0;
> }
> +
> +NDCTL_EXPORT int ndctl_dimm_disable_key(struct ndctl_dimm *dimm,
> + const char *keypath)
ndctl_dimm_remove_key()?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v8 04/12] ndctl: add support for freeze security
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
` (2 preceding siblings ...)
2019-01-14 20:06 ` [PATCH v8 03/12] ndctl: add disable security support Dave Jiang
@ 2019-01-14 20:06 ` Dave Jiang
2019-01-16 4:53 ` Dan Williams
2019-01-14 20:07 ` [PATCH v8 05/12] ndctl: add support for sanitize dimm Dave Jiang
` (7 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:06 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 ++++++++++++++++++
ndctl/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/ndctl/builtin.h b/ndctl/builtin.h
index 821ea690..f7469598 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -35,4 +35,5 @@ int cmd_inject_smart(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_passphrase_setup(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_passphrase_update(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_disable_passphrase(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_freeze_security(int argc, const char **argv, struct ndctl_ctx *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 4f0466a1..19301791 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -876,6 +876,24 @@ static int action_passphrase_disable(struct ndctl_dimm *dimm,
return ndctl_dimm_disable_key(dimm, param.key_path);
}
+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)
{
@@ -1285,3 +1303,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 076ccbf6..8f0f0486 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -672,3 +672,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 b1192960..3862bbfd 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -702,6 +702,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 21f1b834..da8dce11 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] 23+ messages in thread* Re: [PATCH v8 04/12] ndctl: add support for freeze security
2019-01-14 20:06 ` [PATCH v8 04/12] ndctl: add support for freeze security Dave Jiang
@ 2019-01-16 4:53 ` Dan Williams
0 siblings, 0 replies; 23+ messages in thread
From: Dan Williams @ 2019-01-16 4:53 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Mon, Jan 14, 2019 at 12:07 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> 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 ++++++++++++++++++
> ndctl/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
What is it "enabling"?
I would just say:
"Set the given DIMM(s) to reject future security operations"
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl freeze-security' <dimm>
Code says:
ndctl freeze-security <nmem0> [<nmem1>..<nmemN>] [<options>]
...I'm assuming the multiple nmem support is true, but there are no
extra options?
...and now that I say that out loud, I think all of these commands
should support -v/--verbose to turn on libndctl debug.
> +
> +DESCRIPTION
> +-----------
> +Provide a generic interface to freeze the security for NVDIMM.
That can go, it reads like a changelog, not a man page.
> Once security
> +is frozen, no other security operations can succeed until reboot happens.
"Prevent any further security operations on the given DIMMs until the
next reboot. This is used in scenarios where the administrator has
taken all expected security actions for the current boot and wants the
DIMM to enforce / lock the current state."
An example section might show some before and after "ndctl list" data
for the DIMM and perhaps the state changes of the /etc/ndctl/keys
directory.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v8 05/12] ndctl: add support for sanitize dimm
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
` (3 preceding siblings ...)
2019-01-14 20:06 ` [PATCH v8 04/12] ndctl: add support for freeze security Dave Jiang
@ 2019-01-14 20:07 ` Dave Jiang
2019-01-16 5:08 ` Dan Williams
2019-01-17 3:08 ` Jane Chu
2019-01-14 20:07 ` [PATCH v8 06/12] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
` (6 subsequent siblings)
11 siblings, 2 replies; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:07 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 | 38 ++++++++++++++++++++
ndctl/builtin.h | 1 +
ndctl/dimm.c | 52 +++++++++++++++++++++++++++
ndctl/lib/dimm.c | 8 ++++
ndctl/lib/keys.c | 21 +++++++++--
ndctl/lib/libndctl.sym | 2 +
ndctl/libndctl.h | 9 +++++
ndctl/ndctl.c | 1 +
9 files changed, 131 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..79629964
--- /dev/null
+++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
@@ -0,0 +1,38 @@
+// 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.
+
+Search the user key ring for the associated NVDIMM. If not found,
+attempt to load the key blob from the default location. After disabling
+the passphrase, remove the key and the key blob.
+
+OPTIONS
+-------
+<dimm>::
+include::xable-dimm-options.txt[]
+
+-p::
+--key-path=::
+ Path to where key related files resides. This parameter is optional
+ and the default is set to /etc/ndctl/keys.
+
+-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/ndctl/builtin.h b/ndctl/builtin.h
index f7469598..55bee47c 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -36,4 +36,5 @@ int cmd_passphrase_setup(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_passphrase_update(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_disable_passphrase(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_freeze_security(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_sanitize_dimm(int argc, const char **argv, struct ndctl_ctx *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 19301791..a91b40d5 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -47,6 +47,7 @@ static struct parameters {
const char *labelversion;
const char *key_path;
const char *master_key;
+ bool crypto_erase;
bool force;
bool json;
bool verbose;
@@ -894,6 +895,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, param.key_path);
+ if (rc < 0)
+ return rc;
+ }
+
+ return 0;
+}
+
static int __action_init(struct ndctl_dimm *dimm,
enum ndctl_namespace_version version, int chk_only)
{
@@ -991,6 +1021,10 @@ OPT_FILENAME('p', "key-path", ¶m.key_path, "key-path", \
OPT_STRING('m', "master-key", ¶m.master_key, "<key_type>:<key_name>", \
"master key for security")
+#define SANITIZE_OPTIONS() \
+OPT_BOOLEAN('c', "crypto-erase", ¶m.crypto_erase, \
+ "crypto erase a dimm")
+
static const struct option read_options[] = {
BASE_OPTIONS(),
READ_OPTIONS(),
@@ -1033,6 +1067,13 @@ static const struct option key_options[] = {
OPT_END(),
};
+static const struct option sanitize_options[] = {
+ BASE_OPTIONS(),
+ KEY_BASE_OPTIONS(),
+ SANITIZE_OPTIONS(),
+ OPT_END(),
+};
+
static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx,
int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
const struct option *options, const char *usage)
@@ -1313,3 +1354,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 8f0f0486..285e09ce 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -677,3 +677,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 6dfbfd49..42281394 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -389,7 +389,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,
const char *keypath)
{
struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
@@ -405,9 +406,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;
}
@@ -417,3 +418,17 @@ 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,
+ const char *keypath)
+{
+ return check_key_run_and_discard(dimm, ndctl_dimm_disable_passphrase,
+ "disable passphrase", keypath);
+}
+
+NDCTL_EXPORT int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
+ const char *keypath)
+{
+ return check_key_run_and_discard(dimm, ndctl_dimm_secure_erase,
+ "crypto erase", keypath);
+}
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 3862bbfd..bb4bab85 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -703,6 +703,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,
@@ -715,6 +716,8 @@ int ndctl_dimm_enable_key(struct ndctl_dimm *dimm, const char *master,
int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master,
const char *keypath);
int ndctl_dimm_disable_key(struct ndctl_dimm *dimm, const char *keypath);
+int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
+ const char *keypath);
#else
static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
const char *master, const char *keypath)
@@ -733,6 +736,12 @@ 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,
+ const char *keypath)
+{
+ return -EOPNOTSUPP;
+}
#endif
#define ND_KEY_DESC_SIZE 128
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index da8dce11..09af1317 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] 23+ messages in thread* Re: [PATCH v8 05/12] ndctl: add support for sanitize dimm
2019-01-14 20:07 ` [PATCH v8 05/12] ndctl: add support for sanitize dimm Dave Jiang
@ 2019-01-16 5:08 ` Dan Williams
2019-01-17 3:08 ` Jane Chu
1 sibling, 0 replies; 23+ messages in thread
From: Dan Williams @ 2019-01-16 5:08 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Mon, Jan 14, 2019 at 12:07 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> 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 | 38 ++++++++++++++++++++
> ndctl/builtin.h | 1 +
> ndctl/dimm.c | 52 +++++++++++++++++++++++++++
> ndctl/lib/dimm.c | 8 ++++
> ndctl/lib/keys.c | 21 +++++++++--
> ndctl/lib/libndctl.sym | 2 +
> ndctl/libndctl.h | 9 +++++
> ndctl/ndctl.c | 1 +
> 9 files changed, 131 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..79629964
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +ndctl-sanitize-dimm(1)
> +======================
> +
> +NAME
> +----
> +ndctl-sanitize-dimm - sanitize the data on the NVDIMM
Perform a cryptographic destruction or overwrite of the contents of
the given NVDIMM(s).
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl sanitize' <dimm> [<options>]
> +
> +DESCRIPTION
> +-----------
> +Provide a generic interface to crypto erase a NVDIMM.
Same "Provide" comment
> +
> +Search the user key ring for the associated NVDIMM. If not found,
> +attempt to load the key blob from the default location. After disabling
> +the passphrase, remove the key and the key blob.
Perhaps reference the standards document Robert pointed out that
clarifies the different levels of sanitation, and what this command
provides.
> +
> +OPTIONS
> +-------
> +<dimm>::
> +include::xable-dimm-options.txt[]
> +
> +-p::
> +--key-path=::
> + Path to where key related files resides. This parameter is optional
> + and the default is set to /etc/ndctl/keys.
> +
> +-c::
> +--crypto-erase::
> + Replaces encryption keys and securely erases the data.
This makes it sound like two actions are taken.
Replace the media encryption key causing all existing data to read as
cipher text with the new key.
> This does not
> + change label data. This is the default sanitize method.
> +
> +include::../copyright.txt[]
> diff --git a/ndctl/builtin.h b/ndctl/builtin.h
> index f7469598..55bee47c 100644
> --- a/ndctl/builtin.h
> +++ b/ndctl/builtin.h
> @@ -36,4 +36,5 @@ int cmd_passphrase_setup(int argc, const char **argv, struct ndctl_ctx *ctx);
> int cmd_passphrase_update(int argc, const char **argv, struct ndctl_ctx *ctx);
> int cmd_disable_passphrase(int argc, const char **argv, struct ndctl_ctx *ctx);
> int cmd_freeze_security(int argc, const char **argv, struct ndctl_ctx *ctx);
> +int cmd_sanitize_dimm(int argc, const char **argv, struct ndctl_ctx *ctx);
> #endif /* _NDCTL_BUILTIN_H_ */
> diff --git a/ndctl/dimm.c b/ndctl/dimm.c
> index 19301791..a91b40d5 100644
> --- a/ndctl/dimm.c
> +++ b/ndctl/dimm.c
> @@ -47,6 +47,7 @@ static struct parameters {
> const char *labelversion;
> const char *key_path;
> const char *master_key;
> + bool crypto_erase;
> bool force;
> bool json;
> bool verbose;
> @@ -894,6 +895,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
This comment reads like a note to a patch reviewer about a future
patch, not a comment for someone reading the code later. The future
tense is stale as soon as the overwrite support is added. I'd say
either change this patch to not assume overwrite support will be
added, or squash this with overwrite support and reflow the comments
to be present tense.
> + * 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, param.key_path);
> + if (rc < 0)
> + return rc;
> + }
> +
> + return 0;
> +}
> +
> static int __action_init(struct ndctl_dimm *dimm,
> enum ndctl_namespace_version version, int chk_only)
> {
> @@ -991,6 +1021,10 @@ OPT_FILENAME('p', "key-path", ¶m.key_path, "key-path", \
> OPT_STRING('m', "master-key", ¶m.master_key, "<key_type>:<key_name>", \
> "master key for security")
>
> +#define SANITIZE_OPTIONS() \
> +OPT_BOOLEAN('c', "crypto-erase", ¶m.crypto_erase, \
> + "crypto erase a dimm")
> +
> static const struct option read_options[] = {
> BASE_OPTIONS(),
> READ_OPTIONS(),
> @@ -1033,6 +1067,13 @@ static const struct option key_options[] = {
> OPT_END(),
> };
>
> +static const struct option sanitize_options[] = {
> + BASE_OPTIONS(),
> + KEY_BASE_OPTIONS(),
> + SANITIZE_OPTIONS(),
> + OPT_END(),
> +};
> +
> static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx,
> int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
> const struct option *options, const char *usage)
> @@ -1313,3 +1354,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 8f0f0486..285e09ce 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -677,3 +677,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 6dfbfd49..42281394 100644
> --- a/ndctl/lib/keys.c
> +++ b/ndctl/lib/keys.c
> @@ -389,7 +389,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,
> const char *keypath)
How about three separate helper functions?
check()
run()
discard()
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v8 05/12] ndctl: add support for sanitize dimm
2019-01-14 20:07 ` [PATCH v8 05/12] ndctl: add support for sanitize dimm Dave Jiang
2019-01-16 5:08 ` Dan Williams
@ 2019-01-17 3:08 ` Jane Chu
2019-01-17 16:58 ` Dave Jiang
1 sibling, 1 reply; 23+ messages in thread
From: Jane Chu @ 2019-01-17 3:08 UTC (permalink / raw)
To: Dave Jiang, vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
Hi, Dave,
On 1/14/2019 12:07 PM, Dave Jiang wrote:
> 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 | 38 ++++++++++++++++++++
> ndctl/builtin.h | 1 +
> ndctl/dimm.c | 52 +++++++++++++++++++++++++++
> ndctl/lib/dimm.c | 8 ++++
> ndctl/lib/keys.c | 21 +++++++++--
> ndctl/lib/libndctl.sym | 2 +
> ndctl/libndctl.h | 9 +++++
> ndctl/ndctl.c | 1 +
> 9 files changed, 131 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..79629964
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
> @@ -0,0 +1,38 @@
> +// 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>]
Did you actually mean
'ndctl sanitize-dimm' <dimm> [<options>]
?
thanks!
-jane
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v8 05/12] ndctl: add support for sanitize dimm
2019-01-17 3:08 ` Jane Chu
@ 2019-01-17 16:58 ` Dave Jiang
0 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2019-01-17 16:58 UTC (permalink / raw)
To: Jane Chu, vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
On 1/16/19 8:08 PM, Jane Chu wrote:
> Hi, Dave,
>
> On 1/14/2019 12:07 PM, Dave Jiang wrote:
>> 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 | 38 ++++++++++++++++++++
>> ndctl/builtin.h | 1 +
>> ndctl/dimm.c | 52
>> +++++++++++++++++++++++++++
>> ndctl/lib/dimm.c | 8 ++++
>> ndctl/lib/keys.c | 21 +++++++++--
>> ndctl/lib/libndctl.sym | 2 +
>> ndctl/libndctl.h | 9 +++++
>> ndctl/ndctl.c | 1 +
>> 9 files changed, 131 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..79629964
>> --- /dev/null
>> +++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
>> @@ -0,0 +1,38 @@
>> +// 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>]
>
> Did you actually mean
> 'ndctl sanitize-dimm' <dimm> [<options>]
> ?
Yes. Thanks for the catch!
>
> thanks!
> -jane
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v8 06/12] ndctl: add unit test for security ops (minus overwrite)
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
` (4 preceding siblings ...)
2019-01-14 20:07 ` [PATCH v8 05/12] ndctl: add support for sanitize dimm Dave Jiang
@ 2019-01-14 20:07 ` Dave Jiang
2019-01-16 1:02 ` Vishal Verma
2019-01-14 20:07 ` [PATCH v8 07/12] ndctl: add modprobe conf file and load-keys ndctl command Dave Jiang
` (5 subsequent siblings)
11 siblings, 1 reply; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:07 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 | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 201 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..9f69b481
--- /dev/null
+++ b/test/security.sh
@@ -0,0 +1,197 @@
+#!/bin/bash -Ex
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2018 Intel Corporation. All rights reserved.
+
+rc=77
+dev=""
+id=""
+dev_no=""
+keypath="/etc/ndctl/keys"
+masterkey="nvdimm-master-test"
+masterpath="$keypath/$masterkey"
+
+. ./common
+
+lockpath="/sys/devices/platform/${NFIT_TEST_BUS0}/nfit_test_dimm/test_dimm"
+
+trap 'err $LINENO' ERR
+
+setup()
+{
+ $NDCTL disable-region -b "$NFIT_TEST_BUS0" all
+}
+
+detect()
+{
+ dev="$($NDCTL list -b "$NFIT_TEST_BUS0" -D | jq -r .[0].dev)"
+ [ -n "$dev" ] || err "$LINENO"
+ id="$($NDCTL list -b "$NFIT_TEST_BUS0" -D | jq -r .[0].id)"
+ [ -n "$id" ] || err "$LINENO"
+}
+
+setup_keys()
+{
+ keyctl add user "$masterkey" "$(dd if=/dev/urandom bs=1 count=32 2>/dev/null)" @u
+ keyctl pipe "$(keyctl search @u user $masterkey)" > "$masterpath"
+}
+
+test_cleanup()
+{
+ if keyctl search @u encrypted nvdimm:"$id"; then
+ keyctl unlink "$(keyctl search @u encrypted nvdimm:"$id")"
+ fi
+
+ if keyctl search @u user "$masterkey"; then
+ keyctl unlink "$(keyctl search @u user $masterkey)"
+ fi
+
+ if [ -f "$keypath"/nvdimm_"$id"_"$(hostname)".blob ]; then
+ rm -f "$keypath"/nvdimm_"$id"_"$(hostname)".blob
+ fi
+
+ if [ -f $masterpath ]; then
+ rm -f "$masterpath"
+ fi
+}
+
+lock_dimm()
+{
+ $NDCTL disable-dimm "$dev"
+ dev_no="${dev#nmem}"
+ echo 1 > "${lockpath}${dev_no}/lock_dimm"
+ sstate="$(get_security_state)"
+ if [ "$sstate" != "locked" ]; then
+ echo "Incorrect security state: $sstate expected: disabled"
+ err $LINENO
+ fi
+}
+
+get_security_state()
+{
+ $NDCTL list -i -b "$NFIT_TEST_BUS0" -d "$dev" | jq -r .[].dimms[0].security
+}
+
+enable_passphrase()
+{
+ $NDCTL enable-passphrase -m user:"$masterkey" "$dev"
+ sstate="$(get_security_state)"
+ if [ "$sstate" != "unlocked" ]; then
+ echo "Incorrect security state: $sstate expected: unlocked"
+ err $LINENO
+ fi
+}
+
+disable_passphrase()
+{
+ $NDCTL disable-passphrase "$dev"
+ sstate="$(get_security_state)"
+ if [ "$sstate" != "disabled" ]; then
+ echo "Incorrect security state: $sstate expected: disabled"
+ err $LINENO
+ fi
+}
+
+erase_security()
+{
+ $NDCTL sanitize-dimm -c "$dev"
+ sstate="$(get_security_state)"
+ if [ "$sstate" != "disabled" ]; then
+ echo "Incorrect security state: $sstate expected: disabled"
+ err $LINENO
+ fi
+}
+
+update_security()
+{
+ $NDCTL update-passphrase -m user:"$masterkey" "$dev"
+ sstate="$(get_security_state)"
+ if [ "$sstate" != "unlocked" ]; then
+ echo "Incorrect security state: $sstate expected: unlocked"
+ err $LINENO
+ 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_unlock()
+{
+ enable_passphrase
+ lock_dimm
+ $NDCTL enable-dimm "$dev"
+ sstate="$(get_security_state)"
+ if [ "$sstate" != "unlocked" ]; then
+ echo "Incorrect security state: $sstate expected: unlocked"
+ err $LINENO
+ 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
+ sstate="$(get_security_state)"
+ if [ "$sstate" != "frozen" ]; then
+ echo "Incorrect security state: $sstate expected: frozen"
+ err $LINENO
+ fi
+ $NDCTL disable-passphrase "$dev" && { echo "disable succeed after frozen"; }
+ sstate="$(get_security_state)"
+ echo "$sstate"
+ if [ "$sstate" != "frozen" ]; then
+ echo "Incorrect security state: $sstate expected: disabled"
+ err $LINENO
+ fi
+}
+
+check_min_kver "5.0" || do_skip "may lack security handling"
+
+modprobe nfit_test
+setup
+check_prereq "keyctl"
+rc=1
+detect
+test_cleanup
+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, unlock dimm"
+test_4_security_unlock
+
+# 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] 23+ messages in thread* Re: [PATCH v8 06/12] ndctl: add unit test for security ops (minus overwrite)
2019-01-14 20:07 ` [PATCH v8 06/12] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
@ 2019-01-16 1:02 ` Vishal Verma
0 siblings, 0 replies; 23+ messages in thread
From: Vishal Verma @ 2019-01-16 1:02 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On 01/14, 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 | 197 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 201 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..9f69b481
> --- /dev/null
> +++ b/test/security.sh
> @@ -0,0 +1,197 @@
> +#!/bin/bash -Ex
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright(c) 2018 Intel Corporation. All rights reserved.
> +
> +rc=77
> +dev=""
[..]
> +
> +lock_dimm()
> +{
> + $NDCTL disable-dimm "$dev"
> + dev_no="${dev#nmem}"
> + echo 1 > "${lockpath}${dev_no}/lock_dimm"
This breaks setups where nfit_test.0 is not the first bus on the system.
The following patch should fix it (you can squash it into this patch):
8<----
>From 9365b1af15c3c8958f87f618494c3a52d09d8cbc Mon Sep 17 00:00:00 2001
From: Vishal Verma <vishal.l.verma@intel.com>
Date: Tue, 15 Jan 2019 17:49:23 -0700
Subject: [ndctl PATCH] test/security.sh: fix nmemX to nfit_test_dimm
translation
The lock_dimm helper used the nmemX number as is to find the test_dimm
path to trigger the dimm lock for nfit_test, which is incorrect. The
test_dimmY device numbring is relative to nfit_test only, and nmemX
is systemwide. Use the dimm handles to find the right test_dimm to
operate on in lock_dimm. If another user of this functionality shows up,
we can then refactor this into a helper routing in test/common.
While at it, and since this patch is just going to be squashed into the
original, fix up a few more quoting issues.
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
test/security.sh | 41 ++++++++++++++++++++++++++++-------------
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/test/security.sh b/test/security.sh
index 9f69b48..ea6b235 100755
--- a/test/security.sh
+++ b/test/security.sh
@@ -5,15 +5,12 @@
rc=77
dev=""
id=""
-dev_no=""
keypath="/etc/ndctl/keys"
masterkey="nvdimm-master-test"
masterpath="$keypath/$masterkey"
. ./common
-lockpath="/sys/devices/platform/${NFIT_TEST_BUS0}/nfit_test_dimm/test_dimm"
-
trap 'err $LINENO' ERR
setup()
@@ -57,12 +54,30 @@ test_cleanup()
lock_dimm()
{
$NDCTL disable-dimm "$dev"
- dev_no="${dev#nmem}"
- echo 1 > "${lockpath}${dev_no}/lock_dimm"
+
+ # convert nmemX --> test_dimmY
+ # for now this is the only user of such a conversion so we can leave it inline
+ # once a subsequent user arrives we can refactor this to a helper in test/common:
+ # get_test_dimm_path "nfit_test.0" "nmem3"
+ handle="$(ndctl list -b "$NFIT_TEST_BUS0" -d "$dev" -i | jq -r .[].dimms[0].handle)"
+ test_dimm_path=""
+ for test_dimm in /sys/devices/platform/"$NFIT_TEST_BUS0"/nfit_test_dimm/test_dimm*; do
+ td_handle_file="$test_dimm/handle"
+ test -e "$td_handle_file" || continue
+ td_handle="$(cat "$td_handle_file")"
+ if [[ "$td_handle" -eq "$handle" ]]; then
+ test_dimm_path="$test_dimm"
+ break
+ fi
+ done
+ test -d "$test_dimm_path"
+
+ # now lock the dimm
+ echo 1 > "${test_dimm_path}/lock_dimm"
sstate="$(get_security_state)"
if [ "$sstate" != "locked" ]; then
echo "Incorrect security state: $sstate expected: disabled"
- err $LINENO
+ err "$LINENO"
fi
}
@@ -77,7 +92,7 @@ enable_passphrase()
sstate="$(get_security_state)"
if [ "$sstate" != "unlocked" ]; then
echo "Incorrect security state: $sstate expected: unlocked"
- err $LINENO
+ err "$LINENO"
fi
}
@@ -87,7 +102,7 @@ disable_passphrase()
sstate="$(get_security_state)"
if [ "$sstate" != "disabled" ]; then
echo "Incorrect security state: $sstate expected: disabled"
- err $LINENO
+ err "$LINENO"
fi
}
@@ -97,7 +112,7 @@ erase_security()
sstate="$(get_security_state)"
if [ "$sstate" != "disabled" ]; then
echo "Incorrect security state: $sstate expected: disabled"
- err $LINENO
+ err "$LINENO"
fi
}
@@ -107,7 +122,7 @@ update_security()
sstate="$(get_security_state)"
if [ "$sstate" != "unlocked" ]; then
echo "Incorrect security state: $sstate expected: unlocked"
- err $LINENO
+ err "$LINENO"
fi
}
@@ -143,7 +158,7 @@ test_4_security_unlock()
sstate="$(get_security_state)"
if [ "$sstate" != "unlocked" ]; then
echo "Incorrect security state: $sstate expected: unlocked"
- err $LINENO
+ err "$LINENO"
fi
$NDCTL disable-region -b "$NFIT_TEST_BUS0" all
disable_passphrase
@@ -158,14 +173,14 @@ test_5_security_freeze()
sstate="$(get_security_state)"
if [ "$sstate" != "frozen" ]; then
echo "Incorrect security state: $sstate expected: frozen"
- err $LINENO
+ err "$LINENO"
fi
$NDCTL disable-passphrase "$dev" && { echo "disable succeed after frozen"; }
sstate="$(get_security_state)"
echo "$sstate"
if [ "$sstate" != "frozen" ]; then
echo "Incorrect security state: $sstate expected: disabled"
- err $LINENO
+ err "$LINENO"
fi
}
--
2.17.2
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v8 07/12] ndctl: add modprobe conf file and load-keys ndctl command
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
` (5 preceding siblings ...)
2019-01-14 20:07 ` [PATCH v8 06/12] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
@ 2019-01-14 20:07 ` Dave Jiang
2019-01-14 20:07 ` [PATCH v8 08/12] ndctl: add overwrite operation support Dave Jiang
` (4 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:07 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
Add load-keys command to ndctl. This will attempt to load the master key
and the related encrypted keys for nvdimms. Also add reference config file
for modprobe.d in order to call ndctl load-keys and inject keys associated
with the nvdimms into the kernel user ring for unlock.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
Documentation/ndctl/Makefile.am | 3
Makefile.am | 4 +
contrib/nvdimm-security.conf | 1
ndctl.spec.in | 1
ndctl/Makefile.am | 3
ndctl/builtin.h | 1
ndctl/lib/keys.c | 60 ++++++---
ndctl/lib/libndctl.sym | 1
ndctl/libndctl.h | 2
ndctl/load-keys.c | 260 +++++++++++++++++++++++++++++++++++++++
ndctl/ndctl.c | 1
11 files changed, 314 insertions(+), 23 deletions(-)
create mode 100644 contrib/nvdimm-security.conf
create mode 100644 ndctl/load-keys.c
diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index bbea9674..0224cccd 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-load-keys.1
CLEANFILES = $(man1_MANS)
diff --git a/Makefile.am b/Makefile.am
index e0c463a3..df8797ef 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,6 +42,10 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
dist_bashcompletion_DATA = contrib/ndctl
endif
+modprobe_file = contrib/nvdimm-security.conf
+modprobedir = $(sysconfdir)/modprobe.d/
+modprobe_DATA = $(modprobe_file)
+
noinst_LIBRARIES = libccan.a
libccan_a_SOURCES = \
ccan/str/str.h \
diff --git a/contrib/nvdimm-security.conf b/contrib/nvdimm-security.conf
new file mode 100644
index 00000000..e2bb7c0a
--- /dev/null
+++ b/contrib/nvdimm-security.conf
@@ -0,0 +1 @@
+install libnvdimm /usr/bin/ndctl load-keys ; /sbin/modprobe --ignore-install libnvdimm $CMDLINE_OPTS
diff --git a/ndctl.spec.in b/ndctl.spec.in
index 66466db6..afed8a43 100644
--- a/ndctl.spec.in
+++ b/ndctl.spec.in
@@ -121,6 +121,7 @@ make check
%{_sysconfdir}/ndctl/monitor.conf
%{_unitdir}/ndctl-monitor.service
%{_sysconfdir}/ndctl/keys/
+%{_sysconfdir}/modprobe.d/nvdimm-security.conf
%files -n daxctl
%defattr(-,root,root)
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 120941a4..eafae05d 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -16,7 +16,8 @@ ndctl_SOURCES = ndctl.c \
util/json-firmware.c \
inject-error.c \
inject-smart.c \
- monitor.c
+ monitor.c \
+ load-keys.c
if ENABLE_DESTRUCTIVE
ndctl_SOURCES += ../test/blk_namespaces.c \
diff --git a/ndctl/builtin.h b/ndctl/builtin.h
index 55bee47c..2cdc0590 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -37,4 +37,5 @@ int cmd_passphrase_update(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_disable_passphrase(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_freeze_security(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_sanitize_dimm(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_load_keys(int argc, const char **argv, struct ndctl_ctx *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
index 42281394..39c4143c 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -71,16 +71,23 @@ static int get_key_desc(struct ndctl_dimm *dimm, char *desc,
return 0;
}
-static char *load_key_blob(struct ndctl_ctx *ctx, const char *path, int *size)
+NDCTL_EXPORT char *ndctl_load_key_blob(struct ndctl_ctx *ctx,
+ const char *path, int *size, const char *postfix, int dirfd)
{
struct stat st;
- FILE *bfile = NULL;
- ssize_t read;
- int rc;
+ ssize_t read_bytes = 0;
+ int rc, fd;
char *blob, *pl;
char prefix[] = "load ";
- rc = stat(path, &st);
+ fd = openat(dirfd, path, O_RDONLY);
+ if (fd < 0) {
+ err(ctx, "failed to open file %s: %s\n",
+ path, strerror(errno));
+ return NULL;
+ }
+
+ rc = fstat(fd, &st);
if (rc < 0) {
err(ctx, "stat: %s\n", strerror(errno));
return NULL;
@@ -96,31 +103,42 @@ static char *load_key_blob(struct ndctl_ctx *ctx, const char *path, int *size)
}
*size = st.st_size + sizeof(prefix) - 1;
+ /*
+ * We need to increment postfix and space.
+ * "keyhandle=" is 10 bytes, plus null termination.
+ */
+ if (postfix)
+ *size += strlen(postfix) + 10 + 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, "Failed to read from blob file: %s\n",
- strerror(errno));
- free(blob);
- fclose(bfile);
- return NULL;
+
+ do {
+ rc = read(fd, pl, st.st_size);
+ if (rc < 0) {
+ err(ctx, "Failed to read from blob file: %s\n",
+ strerror(errno));
+ free(blob);
+ close(fd);
+ return NULL;
+ }
+ read_bytes += rc;
+ } while (read_bytes != st.st_size);
+
+ close(fd);
+
+ if (postfix) {
+ pl += read_bytes;
+ *pl = ' ';
+ pl++;
+ rc = sprintf(pl, "keyhandle=%s", postfix);
}
- fclose(bfile);
return blob;
}
@@ -250,7 +268,7 @@ static key_serial_t dimm_load_key(struct ndctl_dimm *dimm,
if (rc < 0)
return rc;
- blob = load_key_blob(ctx, path, &size);
+ blob = ndctl_load_key_blob(ctx, path, &size, NULL, -1);
if (!blob)
return -ENOMEM;
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 0e3aa5d9..bd85651e 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -388,6 +388,7 @@ global:
LIBNDCTL_19 {
global:
+ ndctl_load_key_blob;
ndctl_dimm_get_security;
ndctl_dimm_security_supported;
ndctl_dimm_enable_key;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index bb4bab85..9decc937 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -696,6 +696,8 @@ enum nd_security_state {
ND_SECURITY_OVERWRITE,
};
+char *ndctl_load_key_blob(struct ndctl_ctx *ctx,
+ const char *path, int *size, const char *postfix, int dirfd);
int ndctl_dimm_get_security(struct ndctl_dimm *dimm,
enum nd_security_state *sstate);
bool ndctl_dimm_security_supported(struct ndctl_dimm *dimm);
diff --git a/ndctl/load-keys.c b/ndctl/load-keys.c
new file mode 100644
index 00000000..802675a2
--- /dev/null
+++ b/ndctl/load-keys.c
@@ -0,0 +1,260 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2019 Intel Corporation. All rights reserved. */
+
+#include <stdio.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <limits.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <dirent.h>
+#include <fcntl.h>
+
+#include <util/json.h>
+#include <util/filter.h>
+#include <json-c/json.h>
+#include <ndctl/libndctl.h>
+#include <util/parse-options.h>
+#include <ccan/array_size/array_size.h>
+
+#include <ndctl.h>
+
+static struct parameters {
+ const char *key_path;
+ const char *tpm_handle;
+} param;
+
+enum key_type {
+ KEY_USER = 0,
+ KEY_TRUSTED,
+};
+
+static const char *key_names[] = {"user", "trusted"};
+
+static struct loadkeys {
+ enum key_type key_type;
+ DIR *dir;
+ int dirfd;
+} loadkey_ctx;
+
+static int load_master_key(struct ndctl_ctx *ctx, struct loadkeys *lk_ctx,
+ const char *keypath)
+{
+ key_serial_t key;
+ char *blob;
+ int size, rc;
+ char path[PATH_MAX];
+
+ rc = sprintf(path, "%s/nvdimm-master.blob", keypath);
+ if (rc < 0)
+ return -errno;
+
+ if (param.tpm_handle)
+ lk_ctx->key_type = KEY_TRUSTED;
+ else
+ lk_ctx->key_type = KEY_USER;
+
+ key = keyctl_search(KEY_SPEC_USER_KEYRING,
+ key_names[lk_ctx->key_type], "nvdimm-master", 0);
+ if (key > 0) /* check to see if key already loaded */
+ return 0;
+
+ if (key < 0 && errno != ENOKEY) {
+ fprintf(stderr, "keyctl_search() failed: %s\n",
+ strerror(errno));
+ return -errno;
+ }
+
+ blob = ndctl_load_key_blob(ctx, path, &size, param.tpm_handle, -1);
+ if (!blob)
+ return -ENOMEM;
+
+ key = add_key(key_names[lk_ctx->key_type], "nvdimm-master",
+ blob, size, KEY_SPEC_USER_KEYRING);
+ free(blob);
+ if (key < 0) {
+ fprintf(stderr, "add_key failed: %s\n", strerror(errno));
+ return -errno;
+ }
+
+ printf("nvdimm master key loaded.\n");
+
+ return 0;
+}
+
+static int load_dimm_keys(struct ndctl_ctx *ctx, struct loadkeys *lk_ctx)
+{
+ int rc;
+ struct dirent *dent;
+ char *fname, *id, *blob;
+ char desc[ND_KEY_DESC_SIZE];
+ int size, count = 0;
+ key_serial_t key;
+
+ while ((dent = readdir(lk_ctx->dir)) != NULL) {
+ if (dent->d_type != DT_REG)
+ continue;
+
+ fname = strdup(dent->d_name);
+ if (!fname) {
+ fprintf(stderr, "Unable to strdup %s\n",
+ dent->d_name);
+ return -ENOMEM;
+ }
+
+ /*
+ * We want to pick up the second member of the file name
+ * as the nvdimm id.
+ */
+ id = strtok(fname, "_");
+ if (!id)
+ continue;
+ if (strcmp(id, "nvdimm") != 0)
+ continue;
+ id = strtok(NULL, "_");
+ if (!id)
+ continue;
+
+ blob = ndctl_load_key_blob(ctx, dent->d_name, &size, NULL,
+ lk_ctx->dirfd);
+ if (!blob) {
+ free(fname);
+ continue;
+ }
+
+ rc = sprintf(desc, "nvdimm:%s", id);
+ if (rc < 0) {
+ free(fname);
+ free(blob);
+ continue;
+ }
+
+ key = add_key("encrypted", desc, blob, size,
+ KEY_SPEC_USER_KEYRING);
+ if (key < 0)
+ fprintf(stderr, "add_key failed: %s\n",
+ strerror(errno));
+ else
+ count++;
+ free(fname);
+ free(blob);
+ }
+
+ printf("%d nvdimm keys loaded\n", count);
+
+ return 0;
+}
+
+static int check_tpm_handle(struct ndctl_ctx *ctx, struct loadkeys *lk_ctx)
+{
+ int fd, rc;
+ FILE *fs;
+ char *buf;
+
+ fd = openat(lk_ctx->dirfd, "tpm.handle", O_RDONLY);
+ if (fd < 0) {
+ fprintf(stderr, "Fail to open TPM handle: %s\n",
+ strerror(errno));
+ return -errno;
+ }
+
+ fs = fdopen(fd, "r");
+ if (!fs) {
+ fprintf(stderr, "Fail to open file stream: %s\n",
+ strerror(errno));
+ return -errno;
+ }
+
+ rc = fscanf(fs, "%ms", &buf);
+ if (rc < 0) {
+ rc = -errno;
+ fprintf(stderr, "Fail to read file: %s\n", strerror(errno));
+ fclose(fs);
+ return rc;
+ }
+
+ param.tpm_handle = buf;
+ fclose(fs);
+ return 0;
+}
+
+static int load_keys(struct ndctl_ctx *ctx, struct loadkeys *lk_ctx,
+ const char *keypath, const char *tpmhandle)
+{
+ int rc;
+
+ rc = chdir(keypath);
+ if (rc < 0) {
+ rc = -errno;
+ fprintf(stderr, "Change current work dir to %s failed: %s\n",
+ param.key_path, strerror(errno));
+ rc = -errno;
+ goto erropen;
+ }
+
+ lk_ctx->dir = opendir(param.key_path);
+ if (!lk_ctx->dir) {
+ fprintf(stderr, "Unable to open dir %s: %s\n",
+ param.key_path, strerror(errno));
+ rc = -errno;
+ goto erropen;
+ }
+
+ lk_ctx->dirfd = open(param.key_path, O_DIRECTORY);
+ if (lk_ctx->dirfd < 0) {
+ fprintf(stderr, "Unable to open dir %s: %s\n",
+ param.key_path, strerror(errno));
+ rc = -errno;
+ goto erropen;
+ }
+
+ if (!tpmhandle) {
+ rc = check_tpm_handle(ctx, lk_ctx);
+ if (rc < 0) {
+ rc = -errno;
+ goto erropen;
+ }
+ }
+
+ rc = load_master_key(ctx, lk_ctx, param.key_path);
+ if (rc < 0)
+ goto out;
+
+ rc = load_dimm_keys(ctx, lk_ctx);
+ if (rc < 0)
+ goto out;
+
+ out:
+ close(lk_ctx->dirfd);
+ erropen:
+ closedir(lk_ctx->dir);
+ return rc;
+}
+
+int cmd_load_keys(int argc, const char **argv, struct ndctl_ctx *ctx)
+{
+ const struct option options[] = {
+ OPT_FILENAME('p', "key-path", ¶m.key_path, "key-path",
+ "override the default key path"),
+ OPT_STRING('t', "tpm-handle", ¶m.tpm_handle, "tpm-handle",
+ "TPM handle for trusted key"),
+ OPT_END(),
+ };
+ const char *const u[] = {
+ "ndctl load-keys [<options>]",
+ NULL
+ };
+ int i;
+
+ argc = parse_options(argc, argv, options, u, 0);
+ for (i = 0; i < argc; i++)
+ error("unknown parameter \"%s\"\n", argv[i]);
+ if (argc)
+ usage_with_options(u, options);
+
+ if (!param.key_path)
+ param.key_path = strdup(NDCTL_KEYS_DIR);
+
+ return load_keys(ctx, &loadkey_ctx, param.key_path, param.tpm_handle);
+}
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 09af1317..91b080d5 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 } },
+ { "load-keys", { cmd_load_keys } },
{ "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] 23+ messages in thread* [PATCH v8 08/12] ndctl: add overwrite operation support
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
` (6 preceding siblings ...)
2019-01-14 20:07 ` [PATCH v8 07/12] ndctl: add modprobe conf file and load-keys ndctl command Dave Jiang
@ 2019-01-14 20:07 ` Dave Jiang
2019-01-14 20:07 ` [PATCH v8 09/12] ndctl: add wait-overwrite support Dave Jiang
` (3 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:07 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 | 21 ++++++++++++++++-----
ndctl/lib/libndctl.sym | 2 ++
ndctl/libndctl.h | 8 ++++++++
6 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/Documentation/ndctl/ndctl-sanitize-dimm.txt b/Documentation/ndctl/ndctl-sanitize-dimm.txt
index 79629964..a1a43bdd 100644
--- a/Documentation/ndctl/ndctl-sanitize-dimm.txt
+++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
@@ -35,4 +35,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 a91b40d5..799823d6 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -48,6 +48,7 @@ static struct parameters {
const char *key_path;
const char *master_key;
bool crypto_erase;
+ bool overwrite;
bool force;
bool json;
bool verbose;
@@ -910,7 +911,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");
}
@@ -921,6 +922,12 @@ static int action_sanitize_dimm(struct ndctl_dimm *dimm,
return rc;
}
+ if (param.overwrite) {
+ rc = ndctl_dimm_overwrite_key(dimm, param.key_path);
+ if (rc < 0)
+ return rc;
+ }
+
return 0;
}
@@ -1023,7 +1030,9 @@ OPT_STRING('m', "master-key", ¶m.master_key, "<key_type>:<key_name>", \
#define SANITIZE_OPTIONS() \
OPT_BOOLEAN('c', "crypto-erase", ¶m.crypto_erase, \
- "crypto erase a dimm")
+ "crypto erase a dimm"), \
+OPT_BOOLEAN('o', "overwrite", ¶m.overwrite, \
+ "overwrite a dimm")
static const struct option read_options[] = {
BASE_OPTIONS(),
@@ -1361,7 +1370,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 285e09ce..f27b966c 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -685,3 +685,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 39c4143c..beb5ab3b 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -92,6 +92,7 @@ NDCTL_EXPORT char *ndctl_load_key_blob(struct ndctl_ctx *ctx,
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;
@@ -418,10 +419,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, keypath);
- 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);
@@ -431,9 +433,11 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
return rc;
}
- rc = dimm_remove_key(dimm, false, keypath);
- if (rc < 0)
- err(ctx, "Unable to cleanup key.\n");
+ if (key) {
+ rc = dimm_remove_key(dimm, false, keypath);
+ if (rc < 0)
+ err(ctx, "Unable to cleanup key.\n");
+ }
return 0;
}
@@ -450,3 +454,10 @@ 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", keypath);
}
+
+NDCTL_EXPORT int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm,
+ const char *keypath)
+{
+ return check_key_run_and_discard(dimm, ndctl_dimm_overwrite,
+ "overwrite", keypath);
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index bd85651e..ba9af99e 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -399,4 +399,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 9decc937..bcfcdfa5 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -706,6 +706,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,
@@ -720,6 +721,7 @@ int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master,
int ndctl_dimm_disable_key(struct ndctl_dimm *dimm, const char *keypath);
int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
const char *keypath);
+int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm, const char *keypath);
#else
static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
const char *master, const char *keypath)
@@ -744,6 +746,12 @@ static inline int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
{
return -EOPNOTSUPP;
}
+
+static inline int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm,
+ const char *keypath)
+{
+ return -EOPNOTSUPP;
+}
#endif
#define ND_KEY_DESC_SIZE 128
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v8 09/12] ndctl: add wait-overwrite support
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
` (7 preceding siblings ...)
2019-01-14 20:07 ` [PATCH v8 08/12] ndctl: add overwrite operation support Dave Jiang
@ 2019-01-14 20:07 ` Dave Jiang
2019-01-14 20:07 ` [PATCH v8 10/12] ndctl: master phassphrase management support Dave Jiang
` (2 subsequent siblings)
11 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:07 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
Add a blocking 'wait-overwrite' command to ndctl to let a user wait for an
overwrite operation on a dimm to complete.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
Documentation/ndctl/Makefile.am | 3 +
Documentation/ndctl/ndctl-wait-overwrite.txt | 31 ++++++++++
ndctl/builtin.h | 1
ndctl/dimm.c | 27 +++++++++
ndctl/lib/dimm.c | 78 ++++++++++++++++++++++++++
ndctl/lib/libndctl.sym | 1
ndctl/libndctl.h | 1
ndctl/ndctl.c | 1
8 files changed, 142 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 0224cccd..9c1c95b5 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -53,7 +53,8 @@ man1_MANS = \
ndctl-disable-passphrase.1 \
ndctl-freeze-security.1 \
ndctl-sanitize-dimm.1 \
- ndctl-load-keys.1
+ ndctl-load-keys.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..5d4c72ef
--- /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 kernel provides a POLL(2) capable sysfs file ('security') to indicate
+the state of overwrite. The 'ndctl wait-overwrite' operation waits for
+a change in the state of the 'security' file 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/ndctl/builtin.h b/ndctl/builtin.h
index 2cdc0590..ebc3f5c7 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -38,4 +38,5 @@ int cmd_disable_passphrase(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_freeze_security(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_sanitize_dimm(int argc, const char **argv, struct ndctl_ctx *ctx);
int cmd_load_keys(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_wait_overwrite(int argc, const char **argv, struct ndctl_ctx *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 799823d6..ce477981 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -931,6 +931,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_overwrite(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)
{
@@ -1378,3 +1396,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 f27b966c..eb950331 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";
@@ -693,3 +694,80 @@ 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_overwrite(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 ba9af99e..d2ca19ee 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -401,4 +401,5 @@ global:
ndctl_dimm_secure_erase_key;
ndctl_dimm_overwrite;
ndctl_dimm_overwrite_key;
+ ndctl_dimm_wait_overwrite;
} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index bcfcdfa5..9217e381 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -707,6 +707,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_overwrite(struct ndctl_dimm *dimm);
enum ndctl_key_type {
ND_USER_KEY,
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 91b080d5..d89b8fae 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -94,6 +94,7 @@ static struct cmd_struct commands[] = {
{ "freeze-security", { cmd_freeze_security } },
{ "sanitize-dimm", { cmd_sanitize_dimm } },
{ "load-keys", { cmd_load_keys } },
+ { "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] 23+ messages in thread* [PATCH v8 10/12] ndctl: master phassphrase management support
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
` (8 preceding siblings ...)
2019-01-14 20:07 ` [PATCH v8 09/12] ndctl: add wait-overwrite support Dave Jiang
@ 2019-01-14 20:07 ` Dave Jiang
2019-01-14 20:07 ` [PATCH v8 11/12] ndctl: add master secure erase support Dave Jiang
2019-01-14 20:07 ` [PATCH v8 12/12] ndctl: documentation for security and key management Dave Jiang
11 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:07 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 | 13 +-
ndctl/lib/dimm.c | 9 ++
ndctl/lib/keys.c | 127 ++++++++++++++++-------
ndctl/lib/libndctl.sym | 1
ndctl/libndctl.h | 14 ++-
7 files changed, 130 insertions(+), 48 deletions(-)
diff --git a/Documentation/ndctl/ndctl-enable-passphrase.txt b/Documentation/ndctl/ndctl-enable-passphrase.txt
index c14a206c..c025b1c3 100644
--- a/Documentation/ndctl/ndctl-enable-passphrase.txt
+++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
@@ -29,7 +29,7 @@ OPTIONS
include::xable-dimm-options.txt[]
-m::
---master=::
+--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
@@ -39,4 +39,9 @@ include::xable-dimm-options.txt[]
Path to where key related files resides. This parameter is optional
and the default is set to /etc/ndctl/keys.
+-M::
+--master-passphrase::
+ Indicates 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 dd6e4e4e..8b5dfe01 100644
--- a/Documentation/ndctl/ndctl-update-passphrase.txt
+++ b/Documentation/ndctl/ndctl-update-passphrase.txt
@@ -26,7 +26,7 @@ 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.
@@ -35,4 +35,9 @@ include::xable-dimm-options.txt[]
Path to where key related files resides. This parameter is optional
and the default is set to /etc/ndctl/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/ndctl/dimm.c b/ndctl/dimm.c
index ce477981..4875e414 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -49,6 +49,7 @@ static struct parameters {
const char *master_key;
bool crypto_erase;
bool overwrite;
+ bool master_pass;
bool force;
bool json;
bool verbose;
@@ -849,8 +850,8 @@ static int action_key_enable(struct ndctl_dimm *dimm,
return -EOPNOTSUPP;
}
- return ndctl_dimm_enable_key(dimm, param.master_key,
- param.key_path);
+ return ndctl_dimm_enable_key(dimm, param.master_key, param.key_path,
+ param.master_pass ? ND_MASTER_KEY : ND_USER_KEY);
}
static int action_key_update(struct ndctl_dimm *dimm,
@@ -862,8 +863,8 @@ static int action_key_update(struct ndctl_dimm *dimm,
return -EOPNOTSUPP;
}
- return ndctl_dimm_update_key(dimm, param.master_key,
- param.key_path);
+ return ndctl_dimm_update_key(dimm, param.master_key, param.key_path,
+ param.master_pass ? ND_MASTER_KEY : ND_USER_KEY);
}
static int action_passphrase_disable(struct ndctl_dimm *dimm,
@@ -1044,7 +1045,9 @@ OPT_FILENAME('p', "key-path", ¶m.key_path, "key-path", \
#define KEY_OPTIONS() \
OPT_STRING('m', "master-key", ¶m.master_key, "<key_type>:<key_name>", \
- "master key for security")
+ "master key for security"), \
+OPT_BOOLEAN('M', "master-passphrase", ¶m.master_pass, \
+ "use master passphrase")
#define SANITIZE_OPTIONS() \
OPT_BOOLEAN('c', "crypto-erase", ¶m.crypto_erase, \
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index eb950331..dc945296 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -771,3 +771,12 @@ NDCTL_EXPORT int ndctl_dimm_wait_overwrite(struct ndctl_dimm *dimm)
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 beb5ab3b..09ceeb57 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -17,7 +17,7 @@
#include "private.h"
static int get_key_path(struct ndctl_dimm *dimm, char *path,
- enum ndctl_key_type key_type, const char *keypath)
+ const char *keypath, enum ndctl_key_type key_type)
{
struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
char hostname[HOST_NAME_MAX];
@@ -29,16 +29,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",
- keypath,
- ndctl_dimm_get_unique_id(dimm),
+ switch (key_type) {
+ case ND_USER_OLD_KEY:
+ rc = sprintf(path, "%s/nvdimm-old_%s_%s.blob",
+ keypath, ndctl_dimm_get_unique_id(dimm),
hostname);
- } else {
+ break;
+ case ND_USER_KEY:
rc = sprintf(path, "%s/nvdimm_%s_%s.blob",
- keypath,
- ndctl_dimm_get_unique_id(dimm),
+ keypath, ndctl_dimm_get_unique_id(dimm),
hostname);
+ break;
+ case ND_MASTER_OLD_KEY:
+ rc = sprintf(path, "%s/nvdimm-master-old_%s_%s.blob",
+ keypath, ndctl_dimm_get_unique_id(dimm),
+ hostname);
+ break;
+ case ND_MASTER_KEY:
+ rc = sprintf(path, "%s/nvdimm-master_%s_%s.blob",
+ keypath, ndctl_dimm_get_unique_id(dimm),
+ hostname);
+ break;
+ default:
+ return -EINVAL;
}
if (rc < 0) {
@@ -55,12 +68,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, "error setting key description: %s\n",
@@ -157,7 +184,8 @@ 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 *keypath)
+ const char *master_key, const char *keypath,
+ enum ndctl_key_type key_type)
{
struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
char desc[ND_KEY_DESC_SIZE];
@@ -177,7 +205,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;
@@ -188,7 +216,7 @@ static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
return -EEXIST;
}
- rc = get_key_path(dimm, path, ND_USER_KEY, keypath);
+ rc = get_key_path(dimm, path, keypath, key_type);
if (rc < 0)
return rc;
@@ -198,7 +226,7 @@ static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
return -EEXIST;
}
- 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;
@@ -245,7 +273,7 @@ static key_serial_t dimm_create_key(struct ndctl_dimm *dimm,
}
static key_serial_t dimm_load_key(struct ndctl_dimm *dimm,
- enum ndctl_key_type key_type, const char *keypath)
+ const char *keypath, enum ndctl_key_type key_type)
{
struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
key_serial_t key;
@@ -265,7 +293,7 @@ static key_serial_t dimm_load_key(struct ndctl_dimm *dimm,
if (rc < 0)
return rc;
- rc = get_key_path(dimm, path, key_type, keypath);
+ rc = get_key_path(dimm, path, keypath, key_type);
if (rc < 0)
return rc;
@@ -290,13 +318,14 @@ static key_serial_t dimm_load_key(struct ndctl_dimm *dimm,
* ring.
*/
static key_serial_t move_key_to_old(struct ndctl_dimm *dimm,
- const char *keypath)
+ const char *keypath, 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_MAX];
char new_path[PATH_MAX];
+ enum ndctl_key_type okey_type;
if (ndctl_dimm_is_active(dimm)) {
err(ctx, "regions active on %s, op failed\n",
@@ -304,15 +333,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, keypath);
+ 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, keypath, key_type);
if (rc < 0)
return rc;
- rc = get_key_path(dimm, new_path, ND_USER_OLD_KEY, keypath);
+ rc = get_key_path(dimm, new_path, keypath, okey_type);
if (rc < 0)
return rc;
@@ -323,11 +359,11 @@ static key_serial_t move_key_to_old(struct ndctl_dimm *dimm,
return -errno;
}
- return dimm_load_key(dimm, ND_USER_OLD_KEY, keypath);
+ return dimm_load_key(dimm, keypath, okey_type);
}
-static int dimm_remove_key(struct ndctl_dimm *dimm,
- enum ndctl_key_type key_type, const char *keypath)
+static int dimm_remove_key(struct ndctl_dimm *dimm, const char *keypath,
+ enum ndctl_key_type key_type)
{
struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
key_serial_t key;
@@ -338,7 +374,7 @@ static int dimm_remove_key(struct ndctl_dimm *dimm,
if (key > 0)
keyctl_unlink(key, KEY_SPEC_USER_KEYRING);
- rc = get_key_path(dimm, path, key_type, keypath);
+ rc = get_key_path(dimm, path, keypath, key_type);
if (rc < 0)
return rc;
@@ -353,18 +389,22 @@ 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 *keypath)
+ const char *master_key, const char *keypath,
+ enum ndctl_key_type key_type)
{
key_serial_t key;
int rc;
- key = dimm_create_key(dimm, master, keypath);
+ key = dimm_create_key(dimm, master_key, keypath, key_type);
if (key < 0)
return 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, keypath);
+ dimm_remove_key(dimm, keypath, key_type);
return rc;
}
@@ -372,10 +412,19 @@ 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 *keypath)
+ const char *master_key, const char *keypath,
+ 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
@@ -385,23 +434,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, keypath);
+ old_key = move_key_to_old(dimm, keypath, key_type);
if (old_key < 0)
return old_key;
- new_key = dimm_create_key(dimm, master, keypath);
+ new_key = dimm_create_key(dimm, master_key, keypath, key_type);
/* need to create new key here */
if (new_key < 0) {
- new_key = dimm_load_key(dimm, ND_USER_KEY, keypath);
+ new_key = dimm_load_key(dimm, keypath, 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, keypath);
+ rc = dimm_remove_key(dimm, keypath, okey_type);
if (rc < 0)
return rc;
@@ -416,9 +469,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, keypath);
+ key = dimm_load_key(dimm, keypath, ND_USER_KEY);
if (key < 0 && run_op != ndctl_dimm_overwrite) {
err(ctx, "Unable to load key\n");
return -ENOKEY;
@@ -434,7 +487,7 @@ static int check_key_run_and_discard(struct ndctl_dimm *dimm,
}
if (key) {
- rc = dimm_remove_key(dimm, false, keypath);
+ rc = dimm_remove_key(dimm, keypath, 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 d2ca19ee..e4d89e06 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -402,4 +402,5 @@ global:
ndctl_dimm_overwrite;
ndctl_dimm_overwrite_key;
ndctl_dimm_wait_overwrite;
+ ndctl_dimm_update_master_passphrase;
} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 9217e381..0aa0640d 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -708,30 +708,36 @@ 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_overwrite(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,
- const char *keypath);
+ const char *keypath, enum ndctl_key_type key_type);
int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master,
- const char *keypath);
+ const char *keypath, enum ndctl_key_type key_type);
int ndctl_dimm_disable_key(struct ndctl_dimm *dimm, const char *keypath);
int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
const char *keypath);
int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm, const char *keypath);
#else
static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
- const char *master, const char *keypath)
+ const char *master_key, const char *keypath,
+ enum ndctl_key_type key_type)
{
return -EOPNOTSUPP;
}
static inline int ndctl_dimm_update_key(struct ndctl_dimm *dimm,
- const char *master, const char *keypath)
+ const char *master_key, const char *keypath,
+ 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] 23+ messages in thread* [PATCH v8 11/12] ndctl: add master secure erase support
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
` (9 preceding siblings ...)
2019-01-14 20:07 ` [PATCH v8 10/12] ndctl: master phassphrase management support Dave Jiang
@ 2019-01-14 20:07 ` Dave Jiang
2019-01-14 20:07 ` [PATCH v8 12/12] ndctl: documentation for security and key management Dave Jiang
11 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:07 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 | 28 +++++++++++++++++++--------
ndctl/lib/libndctl.sym | 1 +
ndctl/libndctl.h | 5 +++--
6 files changed, 51 insertions(+), 12 deletions(-)
diff --git a/Documentation/ndctl/ndctl-sanitize-dimm.txt b/Documentation/ndctl/ndctl-sanitize-dimm.txt
index a1a43bdd..3a09ae59 100644
--- a/Documentation/ndctl/ndctl-sanitize-dimm.txt
+++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
@@ -39,4 +39,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 4875e414..7f2d4873 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -908,6 +908,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.
@@ -918,7 +924,9 @@ static int action_sanitize_dimm(struct ndctl_dimm *dimm,
}
if (param.crypto_erase) {
- rc = ndctl_dimm_secure_erase_key(dimm, param.key_path);
+ rc = ndctl_dimm_secure_erase_key(dimm, param.key_path,
+ param.master_pass ?
+ ND_MASTER_KEY : ND_USER_KEY);
if (rc < 0)
return rc;
}
@@ -1053,7 +1061,9 @@ OPT_BOOLEAN('M', "master-passphrase", ¶m.master_pass, \
OPT_BOOLEAN('c', "crypto-erase", ¶m.crypto_erase, \
"crypto erase a dimm"), \
OPT_BOOLEAN('o', "overwrite", ¶m.overwrite, \
- "overwrite a dimm")
+ "overwrite a dimm"), \
+OPT_BOOLEAN('M', "master-passphrase", ¶m.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 dc945296..b9bf9cc2 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -780,3 +780,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 09ceeb57..c10dc436 100644
--- a/ndctl/lib/keys.c
+++ b/ndctl/lib/keys.c
@@ -463,13 +463,13 @@ 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,
- const char *keypath)
+ const char *keypath, 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, keypath, ND_USER_KEY);
if (key < 0 && run_op != ndctl_dimm_overwrite) {
@@ -486,8 +486,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, keypath, ND_USER_KEY);
+ rc = dimm_remove_key(dimm, keypath, key_type);
if (rc < 0)
err(ctx, "Unable to cleanup key.\n");
}
@@ -498,19 +502,27 @@ NDCTL_EXPORT int ndctl_dimm_disable_key(struct ndctl_dimm *dimm,
const char *keypath)
{
return check_key_run_and_discard(dimm, ndctl_dimm_disable_passphrase,
- "disable passphrase", keypath);
+ "disable passphrase", keypath, ND_USER_KEY);
}
NDCTL_EXPORT int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
- const char *keypath)
+ const char *keypath, enum ndctl_key_type key_type)
{
- return check_key_run_and_discard(dimm, ndctl_dimm_secure_erase,
- "crypto erase", keypath);
+ if (key_type == ND_MASTER_KEY)
+ return check_key_run_and_discard(dimm,
+ ndctl_dimm_master_secure_erase,
+ "master crypto erase", keypath, key_type);
+ else if (key_type == ND_USER_KEY)
+ return check_key_run_and_discard(dimm,
+ ndctl_dimm_secure_erase,
+ "crypto erase", keypath, key_type);
+ else
+ return -EINVAL;
}
NDCTL_EXPORT int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm,
const char *keypath)
{
return check_key_run_and_discard(dimm, ndctl_dimm_overwrite,
- "overwrite", keypath);
+ "overwrite", keypath, ND_USER_KEY);
}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index e4d89e06..d724f5b9 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -403,4 +403,5 @@ global:
ndctl_dimm_overwrite_key;
ndctl_dimm_wait_overwrite;
ndctl_dimm_update_master_passphrase;
+ ndctl_dimm_master_secure_erase;
} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 0aa0640d..17321242 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -710,6 +710,7 @@ int ndctl_dimm_overwrite(struct ndctl_dimm *dimm, long key);
int ndctl_dimm_wait_overwrite(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,
@@ -725,7 +726,7 @@ int ndctl_dimm_update_key(struct ndctl_dimm *dimm, const char *master,
const char *keypath, enum ndctl_key_type key_type);
int ndctl_dimm_disable_key(struct ndctl_dimm *dimm, const char *keypath);
int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
- const char *keypath);
+ const char *keypath, enum ndctl_key_type key_type);
int ndctl_dimm_overwrite_key(struct ndctl_dimm *dimm, const char *keypath);
#else
static inline int ndctl_dimm_enable_key(struct ndctl_dimm *dimm,
@@ -749,7 +750,7 @@ static inline int ndctl_dimm_disable_key(struct ndctl_dimm *dimm,
}
static inline int ndctl_dimm_secure_erase_key(struct ndctl_dimm *dimm,
- const char *keypath)
+ const char *keypath, 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] 23+ messages in thread* [PATCH v8 12/12] ndctl: documentation for security and key management
2019-01-14 20:06 [PATCH v8 00/12] ndctl: add security support Dave Jiang
` (10 preceding siblings ...)
2019-01-14 20:07 ` [PATCH v8 11/12] ndctl: add master secure erase support Dave Jiang
@ 2019-01-14 20:07 ` Dave Jiang
11 siblings, 0 replies; 23+ messages in thread
From: Dave Jiang @ 2019-01-14 20:07 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
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 | 140 ++++++++++++++++++++++
Documentation/ndctl/ndctl-disable-passphrase.txt | 2
Documentation/ndctl/ndctl-enable-passphrase.txt | 2
Documentation/ndctl/ndctl-freeze-security.txt | 2
Documentation/ndctl/ndctl-sanitize-dimm.txt | 2
Documentation/ndctl/ndctl-update-passphrase.txt | 2
6 files changed, 150 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..13ef34d0
--- /dev/null
+++ b/Documentation/ndctl/intel-nvdimm-security.txt
@@ -0,0 +1,140 @@
+// 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. The operations supported by
+ndctl are 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 'unlock' DSM is not supported by ndctl, that is left for the kernel to
+manage with some assistance from userspace.
+
+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's 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 key from the Trusted Platform
+Module (TPM), but a master key generated by the TPM can also
+be used. 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 minimum the master key is already
+in the kernel's 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
+
+Except for 'overwrite', all operations expect the relevant regions associated
+with the nvdimm are disabled before proceeding. For 'overwrite', in addition
+to the regions, the dimm itself is expected to be disabled.
+
+The following 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. This script script will inject the master key and the
+related encrypted keys into the kernel's user key ring. A reference modprobe
+config file and a setup script have been provided by ndctl. During the 'probe'
+of the nvdimm driver, 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's user key ring.
+3. Finally, create the unlock DSM, 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's user key ring and the master key name is passed to
+ndctl so it can do key management. An encrypted key with a 32 bytes 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's 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 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. The following
+operations are performed for update:
+1. Remove the associated encrypted key from the kernel's user key ring.
+2. Rename the key blob to old.
+3. Load the now old key blob into kernel's 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
+-----------------------
+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 passphrase-disable. The kernel will issue
+WBINVD before and after the operation to ensure no data corruption from a stale
+CPU cache. Use ndctl's sanitize-dimm command with the --crypto-erase option to
+perform this operation.
+
+OVERWRITE
+---------
+The overwrite operation wipes the entire nvdimm. The operation can take
+a significant amount of time. Issuing a command is very similar to
+"passphrase-disable" 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 option is used via ndctl.
+If both --crypto-erase and --overwrite options 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 49f25898..6a8ceba2 100644
--- a/Documentation/ndctl/ndctl-disable-passphrase.txt
+++ b/Documentation/ndctl/ndctl-disable-passphrase.txt
@@ -30,4 +30,6 @@ include::xable-dimm-options.txt[]
Path to where key related files resides. This parameter is optional
and the default is set to /etc/ndctl/keys.
+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 c025b1c3..54a2716e 100644
--- a/Documentation/ndctl/ndctl-enable-passphrase.txt
+++ b/Documentation/ndctl/ndctl-enable-passphrase.txt
@@ -44,4 +44,6 @@ include::xable-dimm-options.txt[]
Indicates 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 3a09ae59..bc2194f8 100644
--- a/Documentation/ndctl/ndctl-sanitize-dimm.txt
+++ b/Documentation/ndctl/ndctl-sanitize-dimm.txt
@@ -45,4 +45,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 8b5dfe01..dc5f33c0 100644
--- a/Documentation/ndctl/ndctl-update-passphrase.txt
+++ b/Documentation/ndctl/ndctl-update-passphrase.txt
@@ -40,4 +40,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] 23+ messages in thread