* [PATCH v4 0/7] ndctl: add security support
@ 2018-10-12 22:28 Dave Jiang
2018-10-12 22:28 ` [PATCH v4 1/7] ndctl: add support for display security state Dave Jiang
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Dave Jiang @ 2018-10-12 22:28 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
The following series implements mechanisms that utilize the sysfs knobs
provided by the kernel in order to support the Intel DSM v1.7 spec
that provides security to NVDIMM. The following abilities are added:
1. display security state
2. update security
3. disable security
4. freeze security
5. secure erase
6. kernel unlock upcall support
v4:
- Updated to match latest kernel interface.
- Added unit test for all security calls
v3:
- Added support to inject keys in order to update nvdimm security.
v2:
- Fixup the upcall util to match recent kernel updates for nvdimm security.
---
Dave Jiang (7):
ndctl: add support for display security state
ndctl: add update to security support
ndctl: add disable security support
ndctl: add support for freeze security
ndctl: add support for sanitize dimm
ndctl: add request-key upcall reference app
ndctl: add unit test for security ops (minus overwrite)
Documentation/ndctl/Makefile.am | 7 +
Documentation/ndctl/ndctl-disable-security.txt | 48 +++++
Documentation/ndctl/ndctl-freeze-security.txt | 21 ++
Documentation/ndctl/ndctl-list.txt | 8 +
Documentation/ndctl/ndctl-sanitize.txt | 52 +++++
Documentation/ndctl/ndctl-update-security.txt | 56 ++++++
Documentation/ndctl/nvdimm-upcall.txt | 33 +++
Makefile.am | 5 +
builtin.h | 4
configure.ac | 5 +
contrib/nvdimm.conf | 1
ndctl.spec.in | 3
ndctl/Makefile.am | 7 +
ndctl/dimm.c | 228 +++++++++++++++++++++++-
ndctl/lib/Makefile.am | 4
ndctl/lib/dimm.c | 63 +++++++
ndctl/lib/keys.c | 139 +++++++++++++++
ndctl/lib/libndctl.sym | 11 +
ndctl/libndctl.h | 18 ++
ndctl/ndctl.c | 4
ndctl/nvdimm-upcall.c | 154 ++++++++++++++++
test/Makefile.am | 3
test/security.sh | 187 ++++++++++++++++++++
util/json.c | 8 +
24 files changed, 1054 insertions(+), 15 deletions(-)
create mode 100644 Documentation/ndctl/ndctl-disable-security.txt
create mode 100644 Documentation/ndctl/ndctl-freeze-security.txt
create mode 100644 Documentation/ndctl/ndctl-sanitize.txt
create mode 100644 Documentation/ndctl/ndctl-update-security.txt
create mode 100644 Documentation/ndctl/nvdimm-upcall.txt
create mode 100644 contrib/nvdimm.conf
create mode 100644 ndctl/lib/keys.c
create mode 100644 ndctl/nvdimm-upcall.c
create mode 100755 test/security.sh
--
Signature
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 1/7] ndctl: add support for display security state
2018-10-12 22:28 [PATCH v4 0/7] ndctl: add security support Dave Jiang
@ 2018-10-12 22:28 ` Dave Jiang
2018-10-13 17:57 ` Dan Williams
2018-10-15 22:12 ` Dan Williams
2018-10-12 22:28 ` [PATCH v4 2/7] ndctl: add update to security support Dave Jiang
` (5 subsequent siblings)
6 siblings, 2 replies; 13+ messages in thread
From: Dave Jiang @ 2018-10-12 22:28 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 | 16 ++++++++++++++++
ndctl/lib/libndctl.sym | 5 +++++
ndctl/libndctl.h | 1 +
util/json.c | 8 ++++++++
5 files changed, 38 insertions(+)
diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index e24c8f40..a4a712a0 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_state:":"disabled"
+}
-H::
--health::
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 5e41734d..4470d5fe 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -583,3 +583,19 @@ NDCTL_EXPORT unsigned long ndctl_dimm_get_available_labels(
return strtoul(buf, NULL, 0);
}
+
+NDCTL_EXPORT int ndctl_dimm_get_security_state(struct ndctl_dimm *dimm,
+ char *state)
+{
+ 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_read_attr(ctx, path, state);
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 6c4c8b4d..d3c1e018 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_state;
+} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 62cef9e8..29fc6603 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -680,6 +680,7 @@ unsigned long long ndctl_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd);
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);
+int ndctl_dimm_get_security_state(struct ndctl_dimm *dimm, char *state);
#ifdef __cplusplus
} /* extern "C" */
diff --git a/util/json.c b/util/json.c
index 5c3424e2..2d46f95f 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;
+ char security_state[32];
if (!jdimm)
return NULL;
@@ -243,6 +244,13 @@ 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_state(dimm, security_state) == 0) {
+ jobj = json_object_new_string(security_state);
+ if (!jobj)
+ goto err;
+ json_object_object_add(jdimm, "security_state", 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] 13+ messages in thread
* [PATCH v4 2/7] ndctl: add update to security support
2018-10-12 22:28 [PATCH v4 0/7] ndctl: add security support Dave Jiang
2018-10-12 22:28 ` [PATCH v4 1/7] ndctl: add support for display security state Dave Jiang
@ 2018-10-12 22:28 ` Dave Jiang
2018-10-16 0:59 ` Dan Williams
2018-10-12 22:28 ` [PATCH v4 3/7] ndctl: add disable " Dave Jiang
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Dave Jiang @ 2018-10-12 22:28 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-security" to trigger that as well.
ndctl does not actually handle the passphrase file. It only initiates the
update and expects all the necessary mechanisms are already in place.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
Documentation/ndctl/Makefile.am | 3 -
Documentation/ndctl/ndctl-update-security.txt | 56 ++++++++++
builtin.h | 1
configure.ac | 5 +
ndctl.spec.in | 1
ndctl/Makefile.am | 3 -
ndctl/dimm.c | 90 ++++++++++++++--
ndctl/lib/Makefile.am | 4 +
ndctl/lib/dimm.c | 24 ++++
ndctl/lib/keys.c | 139 +++++++++++++++++++++++++
ndctl/lib/libndctl.sym | 3 +
ndctl/libndctl.h | 14 +++
ndctl/ndctl.c | 1
13 files changed, 330 insertions(+), 14 deletions(-)
create mode 100644 Documentation/ndctl/ndctl-update-security.txt
create mode 100644 ndctl/lib/keys.c
diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index a30b139b..2c064c3a 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -47,7 +47,8 @@ man1_MANS = \
ndctl-inject-smart.1 \
ndctl-update-firmware.1 \
ndctl-list.1 \
- ndctl-monitor.1
+ ndctl-monitor.1 \
+ ndctl-update-security.1
CLEANFILES = $(man1_MANS)
diff --git a/Documentation/ndctl/ndctl-update-security.txt b/Documentation/ndctl/ndctl-update-security.txt
new file mode 100644
index 00000000..96d519b1
--- /dev/null
+++ b/Documentation/ndctl/ndctl-update-security.txt
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-update-security(1)
+========================
+
+NAME
+----
+ndctl-update-security - enabling or update the security passphrase for a
+NVDIMM
+
+SYNOPSIS
+--------
+[verse]
+'ndctl update-security' <dimm> [<options>]
+
+DESCRIPTION
+-----------
+Provide a generic interface for enabling or updating security passphrase for
+NVDIMM. The use of this depends on support from the underlying libndctl,
+kernel, as well as the platform itself.
+
+For the reference passphrase setup, /etc/nvdimm.passwd is read for passphrase
+retrieval:
+
+The nvdimm.passwd is formatted as:
+<description id>:<passphrase with padded 0 to 32bytes>
+cdab-0a-07e0-feffffff:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+To update a DIMM that has passphrase already enabled, the format is done as:
+<description id>:<new passphrase with padded 0 to 32bytes>:<old passphrase with padded 0
+to 32 bytes>
+
+cdab-0a-07e0-feffffff:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+The accepted key will replace the old payload with the new payload in the
+cached key that resides in the kernel key ring.
+
+OPTIONS
+-------
+<dimm>::
+include::xable-dimm-options.txt[]
+
+-i::
+--insecure::
+ Using the default reference support to parse the nvdimm passphrase
+ file, inject the key, and initiate update operation. This is labeled
+ as insecure as it just provides a reference to how to inject keys
+ for the nvdimm. The passphrase is in clear text and is not considered
+ as secure as it can be.
+
+-e::
+--exec::
+ The external binary module that would inject the passphrase and
+ initiate the update operation. Use this or -i, not both.
+
+include::../copyright.txt[]
diff --git a/builtin.h b/builtin.h
index 675a6ce7..37404c79 100644
--- a/builtin.h
+++ b/builtin.h
@@ -48,4 +48,5 @@ int cmd_bat(int argc, const char **argv, void *ctx);
#endif
int cmd_update_firmware(int argc, const char **argv, void *ctx);
int cmd_inject_smart(int argc, const char **argv, void *ctx);
+int cmd_key_update(int argc, const char **argv, void *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/configure.ac b/configure.ac
index bb6b0332..1cfd9718 100644
--- a/configure.ac
+++ b/configure.ac
@@ -153,6 +153,11 @@ fi
AC_SUBST([systemd_unitdir])
AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
+AC_CHECK_HEADERS([keyutils.h],,[
+ AC_MSG_ERROR([keyutils.h not found, consider installing
+ keyutils-libs-devel.])
+ ])
+
my_CFLAGS="\
-D DEF_CONF_FILE='\"${sysconfdir}/ndctl/monitor.conf\"' \
-Wall \
diff --git a/ndctl.spec.in b/ndctl.spec.in
index 26396d4a..f57b4fd5 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
%description
Utility library for managing the "libnvdimm" subsystem. The "libnvdimm"
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 8a5e5f87..5b62251b 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -30,7 +30,8 @@ ndctl_LDADD =\
../libutil.a \
$(UUID_LIBS) \
$(KMOD_LIBS) \
- $(JSON_LIBS)
+ $(JSON_LIBS) \
+ -lkeyutils
if ENABLE_TEST
ndctl_SOURCES += ../test/libndctl.c \
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 699ab57d..62a102b4 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_exec;
+ bool force;
+ bool json;
+ bool verbose;
+ bool key_insecure;
+} 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,44 @@ 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(struct ndctl_dimm *dimm,
+ struct action_context *actx)
+{
+ int rc, po_valid, pn_valid;
+ char old_payload[ND_PASSPHRASE_SIZE];
+ char new_payload[ND_PASSPHRASE_SIZE];
+ key_serial_t old_key, new_key;
+ char *po;
+
+ if (param.key_exec)
+ return execl(param.key_exec, ndctl_dimm_get_devname(dimm),
+ ndctl_dimm_get_unique_id(dimm), (char *)NULL);
+
+ if (!param.key_insecure)
+ return -EINVAL;
+
+ rc = ndctl_dimm_get_key_payload(dimm, new_payload, &pn_valid,
+ old_payload, &po_valid);
+ if (rc < 0)
+ return rc;
+
+ /*
+ * If we are turning on security rather than updating, then
+ * we would not have an old key.
+ */
+ po = po_valid ? old_payload : NULL;
+ rc = ndctl_dimm_add_keys(dimm, &new_key, new_payload,
+ &old_key, po);
+ if (rc < 0)
+ return rc;
+
+ /* now do actual writing to update */
+ rc = ndctl_dimm_set_change_key(dimm, old_key, new_key);
+ if (rc < 0)
+ return rc;
+
+ return rc;
+}
static int __action_init(struct ndctl_dimm *dimm,
enum ndctl_namespace_version version, int chk_only)
@@ -925,6 +966,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_BOOLEAN('i', "insecure", ¶m.key_insecure, \
+ "insecure reference passphrase update method"), \
+OPT_STRING('e', "exec", ¶m.key_exec, "external-exec", \
+ "external exec module for passphrase update")
+
static const struct option read_options[] = {
BASE_OPTIONS(),
READ_OPTIONS(),
@@ -954,6 +1001,12 @@ static const struct option init_options[] = {
OPT_END(),
};
+static const struct option key_options[] = {
+ BASE_OPTIONS(),
+ KEY_OPTIONS(),
+ OPT_END(),
+};
+
static int dimm_action(int argc, const char **argv, void *ctx,
int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
const struct option *options, const char *usage)
@@ -1024,6 +1077,11 @@ static int dimm_action(int argc, const char **argv, void *ctx,
}
}
+ if (action == action_key && param.key_insecure && param.key_exec) {
+ usage_with_options(u, options);
+ return -EINVAL;
+ }
+
if (param.verbose)
ndctl_set_log_priority(ctx, LOG_DEBUG);
@@ -1181,3 +1239,13 @@ int cmd_update_firmware(int argc, const char **argv, void *ctx)
count > 1 ? "s" : "");
return count >= 0 ? 0 : EXIT_FAILURE;
}
+
+int cmd_key_update(int argc, const char **argv, void *ctx)
+{
+ int count = dimm_action(argc, argv, ctx, action_key, key_options,
+ "ndctl update-security <nmem0> [<nmem1>..<nmemN>] [<options>]");
+
+ fprintf(stderr, "security updated 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..245b78a9 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -22,13 +22,15 @@ libndctl_la_SOURCES =\
msft.c \
ars.c \
firmware.c \
+ keys.c \
libndctl.c
libndctl_la_LIBADD =\
../../daxctl/lib/libdaxctl.la \
$(UDEV_LIBS) \
$(UUID_LIBS) \
- $(KMOD_LIBS)
+ $(KMOD_LIBS) \
+ $(KEYUTILS_LIBS)
EXTRA_DIST += libndctl.sym
diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
index 4470d5fe..c9ef894a 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -599,3 +599,27 @@ NDCTL_EXPORT int ndctl_dimm_get_security_state(struct ndctl_dimm *dimm,
return sysfs_read_attr(ctx, path, state);
}
+
+static int ndctl_dimm_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_set_change_key(struct ndctl_dimm *dimm,
+ key_serial_t ckey, key_serial_t nkey)
+{
+ char buf[SYSFS_ATTR_SIZE];
+
+ sprintf(buf, "update %d %d\n", ckey, nkey);
+ return ndctl_dimm_write_security(dimm, buf);
+}
diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
new file mode 100644
index 00000000..b21d11f0
--- /dev/null
+++ b/ndctl/lib/keys.c
@@ -0,0 +1,139 @@
+/* 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"
+
+NDCTL_EXPORT int ndctl_dimm_get_key_payload(struct ndctl_dimm *dimm,
+ char *pass, int *pass_valid,
+ char *oldpass, int *oldpass_valid)
+{
+ struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+ ssize_t rc = 0;
+ static FILE *fp;
+ const char *dimm_id;
+ size_t size = 0, i;
+ char *line = NULL, *desc, *pass1, *pass2;
+ int found = 0, comment = 0;
+
+ *pass_valid = 0;
+ *oldpass_valid = 0;
+
+ dimm_id = ndctl_dimm_get_unique_id(dimm);
+ if (!dimm_id)
+ return -EINVAL;
+
+ fp = fopen(PASS_PATH, "r+");
+ if (!fp) {
+ err(ctx, "fopen: %s\n", strerror(errno));
+ return -errno;
+ }
+
+ while ((rc = getline(&line, &size, fp)) != -1) {
+ /* skip comments */
+ for (i = 0; i < size; i++) {
+ if (isspace(line[i]))
+ continue;
+ if (line[i] == '#')
+ comment = 1;
+ }
+
+ if (comment) {
+ comment = 0;
+ continue;
+ }
+
+ desc = strtok(line, ":");
+ if (!desc)
+ break;
+ if (strcmp(desc, dimm_id) == 0) {
+ found = 1;
+ rc = 0;
+ break;
+ }
+ }
+
+ memset(oldpass, 0, ND_PASSPHRASE_SIZE);
+ memset(pass, 0, ND_PASSPHRASE_SIZE);
+ if (rc == 0 && found) {
+ pass1 = strtok(NULL, ":");
+ pass2 = strtok(NULL, ":");
+ if (!pass1) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if (strlen(pass1) > ND_PASSPHRASE_SIZE)
+ pass1[ND_PASSPHRASE_SIZE] = '\0';
+
+ if (pass2 && strlen(pass2) > ND_PASSPHRASE_SIZE)
+ pass2[ND_PASSPHRASE_SIZE] = '\0';
+
+ memcpy(pass, pass1, strlen(pass1));
+ *pass_valid = 1;
+
+ if (pass2) {
+ memcpy(oldpass, pass2, strlen(pass2));
+ *oldpass_valid = 1;
+ }
+ } else
+ rc = -ENXIO;
+
+out:
+ free(line);
+ fclose(fp);
+ return rc;
+}
+
+NDCTL_EXPORT int ndctl_dimm_add_keys(struct ndctl_dimm *dimm,
+ key_serial_t *new_key, const char *payload1,
+ key_serial_t *old_key, const char *payload2)
+{
+ int rc;
+ char desc[ND_KEY_DESC_LEN + ND_KEY_DESC_PREFIX];
+ char odesc[ND_KEY_DESC_LEN + 4 + ND_KEY_DESC_PREFIX];
+ struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+
+ rc = sprintf(desc, "nvdimm:%s", ndctl_dimm_get_unique_id(dimm));
+ if (rc < 0) {
+ err(ctx, "sprintf: %s\n", strerror(errno));
+ return -errno;
+ }
+
+ rc = sprintf(odesc, "nvdimm_old:%s", ndctl_dimm_get_unique_id(dimm));
+ if (rc < 0) {
+ err(ctx, "sprintf: %s\n", strerror(errno));
+ return -errno;
+ }
+
+ *new_key = add_key("logon", desc, payload1, ND_PASSPHRASE_SIZE,
+ KEY_SPEC_USER_KEYRING);
+ if (*new_key == -1) {
+ err(ctx, "add_key: %s\n", strerror(errno));
+ return -errno;
+ }
+
+ if (payload2) {
+ *old_key = add_key("logon", odesc, payload2,
+ ND_PASSPHRASE_SIZE, KEY_SPEC_USER_KEYRING);
+ if (*old_key == -1) {
+ err(ctx, "add_key: %s\n", strerror(errno));
+ return -errno;
+ }
+ } else
+ *old_key = 0;
+
+ return 0;
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index d3c1e018..a37e7389 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -389,4 +389,7 @@ global:
LIBNDCTL_19 {
global:
ndctl_dimm_get_security_state;
+ ndctl_dimm_get_key_payload;
+ ndctl_dimm_add_keys;
+ ndctl_dimm_set_change_key;
} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 29fc6603..4fee6467 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -19,6 +19,7 @@
#include <unistd.h>
#include <errno.h>
#include <limits.h>
+#include <keyutils.h>
#ifdef HAVE_LIBUUID
#include <uuid/uuid.h>
@@ -680,7 +681,20 @@ unsigned long long ndctl_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd);
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 PASS_PATH "/etc/nvdimm.passwd"
+#define ND_KEY_DESC_LEN 22
+#define ND_KEY_DESC_PREFIX 7
+
int ndctl_dimm_get_security_state(struct ndctl_dimm *dimm, char *state);
+int ndctl_dimm_set_change_key(struct ndctl_dimm *dimm, key_serial_t ckey,
+ key_serial_t nkey);
+int ndctl_dimm_get_key_payload(struct ndctl_dimm *dimm, char *pass,
+ int *pass_valid, char *oldpass, int *oldpass_valid);
+int ndctl_dimm_add_keys(struct ndctl_dimm *dimm, key_serial_t *new_key,
+ const char *new_payload, key_serial_t *old_key,
+ const char *old_payload);
#ifdef __cplusplus
} /* extern "C" */
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 73dabfac..c7096dfd 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -88,6 +88,7 @@ static struct cmd_struct commands[] = {
{ "inject-smart", cmd_inject_smart },
{ "wait-scrub", cmd_wait_scrub },
{ "start-scrub", cmd_start_scrub },
+ { "update-security", cmd_key_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] 13+ messages in thread
* [PATCH v4 3/7] ndctl: add disable security support
2018-10-12 22:28 [PATCH v4 0/7] ndctl: add security support Dave Jiang
2018-10-12 22:28 ` [PATCH v4 1/7] ndctl: add support for display security state Dave Jiang
2018-10-12 22:28 ` [PATCH v4 2/7] ndctl: add update to security support Dave Jiang
@ 2018-10-12 22:28 ` Dave Jiang
2018-10-12 22:28 ` [PATCH v4 4/7] ndctl: add support for freeze security Dave Jiang
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Dave Jiang @ 2018-10-12 22:28 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-security" for ndctl. This provides a way to disable security
on the nvdimm. ndctl does not handle the actual processing of the
passphrase. It only starts the request.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
Documentation/ndctl/Makefile.am | 3 +-
Documentation/ndctl/ndctl-disable-security.txt | 48 ++++++++++++++++++++++++
builtin.h | 1 +
ndctl/dimm.c | 46 +++++++++++++++++++++++
ndctl/lib/dimm.c | 9 +++++
ndctl/lib/libndctl.sym | 1 +
ndctl/libndctl.h | 1 +
ndctl/ndctl.c | 1 +
8 files changed, 109 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ndctl/ndctl-disable-security.txt
diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index 2c064c3a..faeb0d9b 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -48,7 +48,8 @@ man1_MANS = \
ndctl-update-firmware.1 \
ndctl-list.1 \
ndctl-monitor.1 \
- ndctl-update-security.1
+ ndctl-update-security.1 \
+ ndctl-disable-security.1
CLEANFILES = $(man1_MANS)
diff --git a/Documentation/ndctl/ndctl-disable-security.txt b/Documentation/ndctl/ndctl-disable-security.txt
new file mode 100644
index 00000000..e51717f2
--- /dev/null
+++ b/Documentation/ndctl/ndctl-disable-security.txt
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-disable-security(1)
+========================
+
+NAME
+----
+ndctl-disable-security - enabling or disable security for an NVDIMM
+
+SYNOPSIS
+--------
+[verse]
+'ndctl disable-security' <dimm> [<options>]
+
+DESCRIPTION
+-----------
+Provide a generic interface for disabling security for NVDIMM. The use of this
+depends on support from the underlying libndctl, kernel, as well as the
+platform itself.
+
+For the reference passphrase setup, /etc/nvdimm.passwd is read for passphrase
+retrieval:
+
+The nvdimm.passwd is formatted as:
+<description id>:<passphrase with padded 0 to 32bytes>
+cdab-0a-07e0-feffffff:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+OPTIONS
+-------
+<dimm>::
+include::xable-dimm-options.txt[]
+
+-i::
+--insecure::
+ Using the default reference support to parse the nvdimm passphrase
+ file, inject the key, and initiate disable operation. This is labeled
+ as insecure as it just provides a reference to how to inject keys
+ for the nvdimm. The passphrase is in clear text and is not considered
+ as secure as it can be.
+
+-e::
+--exec::
+ The external binary module that would inject the passphrase and
+ initiate the disable operation. Use this or -i, not both.
+
+
+
+include::../copyright.txt[]
diff --git a/builtin.h b/builtin.h
index 37404c79..39e4e38d 100644
--- a/builtin.h
+++ b/builtin.h
@@ -49,4 +49,5 @@ int cmd_bat(int argc, const char **argv, void *ctx);
int cmd_update_firmware(int argc, const char **argv, void *ctx);
int cmd_inject_smart(int argc, const char **argv, void *ctx);
int cmd_key_update(int argc, const char **argv, void *ctx);
+int cmd_disable_security(int argc, const char **argv, void *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 62a102b4..1de145b1 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -877,6 +877,42 @@ static int action_key(struct ndctl_dimm *dimm,
return rc;
}
+static int action_security_disable(struct ndctl_dimm *dimm,
+ struct action_context *actx)
+{
+ int rc, po_valid, pn_valid;
+ char old_payload[ND_PASSPHRASE_SIZE];
+ char new_payload[ND_PASSPHRASE_SIZE];
+ key_serial_t old_key, new_key;
+
+ if (param.key_exec)
+ return execl(param.key_exec, ndctl_dimm_get_devname(dimm),
+ ndctl_dimm_get_unique_id(dimm), (char *)NULL);
+
+ if (!param.key_insecure)
+ return -EINVAL;
+
+ rc = ndctl_dimm_get_key_payload(dimm, new_payload, &pn_valid,
+ old_payload, &po_valid);
+ if (rc < 0)
+ return rc;
+
+ /*
+ * We will ignore the "old" key. The "new" key is actually the
+ * current key, which we will pass to the kernel.
+ */
+ rc = ndctl_dimm_add_keys(dimm, &new_key, new_payload, &old_key, NULL);
+ if (rc < 0)
+ return rc;
+
+ rc = ndctl_dimm_disable_security(dimm, new_key);
+ if (rc < 0)
+ error("Failed to disable 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)
{
@@ -1249,3 +1285,13 @@ int cmd_key_update(int argc, const char **argv, void *ctx)
count > 1 ? "s" : "");
return count >= 0 ? 0 : EXIT_FAILURE;
}
+
+int cmd_disable_security(int argc, const char **argv, void *ctx)
+{
+ int count = dimm_action(argc, argv, ctx, action_security_disable, key_options,
+ "ndctl disable-security <nmem0> [<nmem1>..<nmemN>] [<options>]");
+
+ fprintf(stderr, "security 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 c9ef894a..16cf2a79 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -623,3 +623,12 @@ NDCTL_EXPORT int ndctl_dimm_set_change_key(struct ndctl_dimm *dimm,
sprintf(buf, "update %d %d\n", ckey, nkey);
return ndctl_dimm_write_security(dimm, buf);
}
+
+NDCTL_EXPORT int ndctl_dimm_disable_security(struct ndctl_dimm *dimm,
+ key_serial_t key)
+{
+ char buf[SYSFS_ATTR_SIZE];
+
+ sprintf(buf, "disable %d\n", key);
+ return ndctl_dimm_write_security(dimm, buf);
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index a37e7389..b84f0063 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -392,4 +392,5 @@ global:
ndctl_dimm_get_key_payload;
ndctl_dimm_add_keys;
ndctl_dimm_set_change_key;
+ ndctl_dimm_disable_security;
} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 4fee6467..72ec1763 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -695,6 +695,7 @@ int ndctl_dimm_get_key_payload(struct ndctl_dimm *dimm, char *pass,
int ndctl_dimm_add_keys(struct ndctl_dimm *dimm, key_serial_t *new_key,
const char *new_payload, key_serial_t *old_key,
const char *old_payload);
+int ndctl_dimm_disable_security(struct ndctl_dimm *dimm, key_serial_t key);
#ifdef __cplusplus
} /* extern "C" */
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index c7096dfd..2dab02d2 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -89,6 +89,7 @@ static struct cmd_struct commands[] = {
{ "wait-scrub", cmd_wait_scrub },
{ "start-scrub", cmd_start_scrub },
{ "update-security", cmd_key_update },
+ { "disable-security", cmd_disable_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] 13+ messages in thread
* [PATCH v4 4/7] ndctl: add support for freeze security
2018-10-12 22:28 [PATCH v4 0/7] ndctl: add security support Dave Jiang
` (2 preceding siblings ...)
2018-10-12 22:28 ` [PATCH v4 3/7] ndctl: add disable " Dave Jiang
@ 2018-10-12 22:28 ` Dave Jiang
2018-10-12 22:29 ` [PATCH v4 5/7] ndctl: add support for sanitize dimm Dave Jiang
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Dave Jiang @ 2018-10-12 22:28 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 | 21 +++++++++++++++++++++
builtin.h | 1 +
ndctl/dimm.c | 22 ++++++++++++++++++++++
ndctl/lib/dimm.c | 5 +++++
ndctl/lib/libndctl.sym | 1 +
ndctl/libndctl.h | 1 +
ndctl/ndctl.c | 1 +
8 files changed, 54 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 faeb0d9b..3a761ba0 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -49,7 +49,8 @@ man1_MANS = \
ndctl-list.1 \
ndctl-monitor.1 \
ndctl-update-security.1 \
- ndctl-disable-security.1
+ ndctl-disable-security.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..343bb019
--- /dev/null
+++ b/Documentation/ndctl/ndctl-freeze-security.txt
@@ -0,0 +1,21 @@
+// 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. The use of this
+depends on support from the underlying libndctl, kernel, as well as the
+platform itself.
+
+include::../copyright.txt[]
diff --git a/builtin.h b/builtin.h
index 39e4e38d..7b970e10 100644
--- a/builtin.h
+++ b/builtin.h
@@ -50,4 +50,5 @@ int cmd_update_firmware(int argc, const char **argv, void *ctx);
int cmd_inject_smart(int argc, const char **argv, void *ctx);
int cmd_key_update(int argc, const char **argv, void *ctx);
int cmd_disable_security(int argc, const char **argv, void *ctx);
+int cmd_freeze_security(int argc, const char **argv, void *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 1de145b1..3ab15094 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -913,6 +913,18 @@ static int action_security_disable(struct ndctl_dimm *dimm,
return rc;
}
+static int action_security_freeze(struct ndctl_dimm *dimm,
+ struct action_context *actx)
+{
+ int rc;
+
+ 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)
{
@@ -1295,3 +1307,13 @@ int cmd_disable_security(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 16cf2a79..fc0c2e57 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -632,3 +632,8 @@ NDCTL_EXPORT int ndctl_dimm_disable_security(struct ndctl_dimm *dimm,
sprintf(buf, "disable %d\n", key);
return ndctl_dimm_write_security(dimm, buf);
}
+
+NDCTL_EXPORT int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm)
+{
+ return ndctl_dimm_write_security(dimm, "freeze");
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index b84f0063..6fd1d3a9 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -393,4 +393,5 @@ global:
ndctl_dimm_add_keys;
ndctl_dimm_set_change_key;
ndctl_dimm_disable_security;
+ ndctl_dimm_freeze_security;
} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 72ec1763..19c4974d 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -696,6 +696,7 @@ int ndctl_dimm_add_keys(struct ndctl_dimm *dimm, key_serial_t *new_key,
const char *new_payload, key_serial_t *old_key,
const char *old_payload);
int ndctl_dimm_disable_security(struct ndctl_dimm *dimm, key_serial_t key);
+int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm);
#ifdef __cplusplus
} /* extern "C" */
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 2dab02d2..7a242aab 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -90,6 +90,7 @@ static struct cmd_struct commands[] = {
{ "start-scrub", cmd_start_scrub },
{ "update-security", cmd_key_update },
{ "disable-security", cmd_disable_security },
+ { "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] 13+ messages in thread
* [PATCH v4 5/7] ndctl: add support for sanitize dimm
2018-10-12 22:28 [PATCH v4 0/7] ndctl: add security support Dave Jiang
` (3 preceding siblings ...)
2018-10-12 22:28 ` [PATCH v4 4/7] ndctl: add support for freeze security Dave Jiang
@ 2018-10-12 22:29 ` Dave Jiang
2018-10-16 1:25 ` Dan Williams
2018-10-12 22:29 ` [PATCH v4 6/7] ndctl: add request-key upcall reference app Dave Jiang
2018-10-12 22:29 ` [PATCH v4 7/7] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
6 siblings, 1 reply; 13+ messages in thread
From: Dave Jiang @ 2018-10-12 22:29 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" for ndctl. This will initiate the request to crypto
erase a DIMM. ndctl does not actually handle the verification of the
security. That is handled by the kernel and the key upcall mechanism.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
Documentation/ndctl/Makefile.am | 3 +
Documentation/ndctl/ndctl-sanitize.txt | 52 ++++++++++++++++++++++++
builtin.h | 1
ndctl/dimm.c | 70 ++++++++++++++++++++++++++++++++
ndctl/lib/dimm.c | 9 ++++
ndctl/lib/libndctl.sym | 1
ndctl/libndctl.h | 1
ndctl/ndctl.c | 1
8 files changed, 137 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ndctl/ndctl-sanitize.txt
diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index 3a761ba0..8c171ecb 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -50,7 +50,8 @@ man1_MANS = \
ndctl-monitor.1 \
ndctl-update-security.1 \
ndctl-disable-security.1 \
- ndctl-freeze-security.1
+ ndctl-freeze-security.1 \
+ ndctl-sanitize.1
CLEANFILES = $(man1_MANS)
diff --git a/Documentation/ndctl/ndctl-sanitize.txt b/Documentation/ndctl/ndctl-sanitize.txt
new file mode 100644
index 00000000..a02b4b31
--- /dev/null
+++ b/Documentation/ndctl/ndctl-sanitize.txt
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-sanitize(1)
+=================
+
+NAME
+----
+ndctl-sanitize - sanitize the data on the NVDIMM
+
+SYNOPSIS
+--------
+[verse]
+'ndctl sanitize' <dimm> [<options>]
+
+DESCRIPTION
+-----------
+Provide a generic interface to crypto erase a NVDIMM.
+The use of this depends on support from the underlying
+libndctl, kernel, as well as the platform itself.
+
+For the reference passphrase setup, /etc/nvdimm.passwd is read for passphrase
+retrieval:
+
+The nvdimm.passwd is formatted as:
+<description id>:<passphrase with padded 0 to 32bytes>
+cdab-0a-07e0-feffffff:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+OPTIONS
+-------
+<dimm>::
+include::xable-dimm-options.txt[]
+
+-m::
+--method::
+ The method for sanitizing the dimm content.
+
+ crypto-erase: replaces encryption keys. This does not change label data.
+
+-i::
+--insecure::
+ Using the default reference support to parse the nvdimm passphrase
+ file, inject the key, and initiate disable operation. This is labeled
+ as insecure as it just provides a reference to how to inject keys
+ for the nvdimm. The passphrase is in clear text and is not considered
+ as secure as it can be.
+
+-e::
+--exec::
+ The external binary module that would inject the passphrase and
+ initiate the disable operation. Use this or -i, not both.
+
+include::../copyright.txt[]
diff --git a/builtin.h b/builtin.h
index 7b970e10..f43988af 100644
--- a/builtin.h
+++ b/builtin.h
@@ -51,4 +51,5 @@ int cmd_inject_smart(int argc, const char **argv, void *ctx);
int cmd_key_update(int argc, const char **argv, void *ctx);
int cmd_disable_security(int argc, const char **argv, void *ctx);
int cmd_freeze_security(int argc, const char **argv, void *ctx);
+int cmd_sanitize(int argc, const char **argv, void *ctx);
#endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 3ab15094..e99c7dcf 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -46,6 +46,7 @@ static struct parameters {
const char *infile;
const char *labelversion;
const char *key_exec;
+ const char *sanitize_method;
bool force;
bool json;
bool verbose;
@@ -925,6 +926,54 @@ static int action_security_freeze(struct ndctl_dimm *dimm,
return rc;
}
+static int action_sanitize(struct ndctl_dimm *dimm,
+ struct action_context *actx)
+{
+ int rc, po_valid, pn_valid;
+ char old_payload[ND_PASSPHRASE_SIZE];
+ char new_payload[ND_PASSPHRASE_SIZE];
+ key_serial_t old_key, new_key;
+
+ if (!param.sanitize_method) {
+ error("No sanitize_method passed in\n");
+ return -EINVAL;
+ }
+
+ if (param.key_exec)
+ return execl(param.key_exec, ndctl_dimm_get_devname(dimm),
+ ndctl_dimm_get_unique_id(dimm),
+ "-m ", param.sanitize_method,
+ (char *)NULL);
+
+ if (!param.key_insecure)
+ return -EINVAL;
+
+ rc = ndctl_dimm_get_key_payload(dimm, new_payload, &pn_valid,
+ old_payload, &po_valid);
+ if (rc < 0)
+ return rc;
+
+ /*
+ * We will ignore the "old" key. The "new" key is actually the
+ * current key, which we will pass to the kernel.
+ */
+ rc = ndctl_dimm_add_keys(dimm, &new_key, new_payload, &old_key, NULL);
+ if (rc < 0)
+ return rc;
+
+ if (strcmp(param.sanitize_method, "crypto-erase") == 0) {
+ rc = ndctl_dimm_secure_erase(dimm, new_key);
+ if (rc < 0)
+ error("Failed to secure erase for %s\n",
+ ndctl_dimm_get_devname(dimm));
+ } else {
+ error("Incorrect sanitize method passed in.\n");
+ return -EINVAL;
+ }
+
+ return rc;
+}
+
static int __action_init(struct ndctl_dimm *dimm,
enum ndctl_namespace_version version, int chk_only)
{
@@ -1020,6 +1069,10 @@ OPT_BOOLEAN('i', "insecure", ¶m.key_insecure, \
OPT_STRING('e', "exec", ¶m.key_exec, "external-exec", \
"external exec module for passphrase update")
+#define SANITIZE_OPTIONS() \
+OPT_STRING('m', "method", ¶m.sanitize_method, "sanitize-method", \
+ "method for sanitize a dimm")
+
static const struct option read_options[] = {
BASE_OPTIONS(),
READ_OPTIONS(),
@@ -1055,6 +1108,13 @@ static const struct option key_options[] = {
OPT_END(),
};
+static const struct option sanitize_options[] = {
+ BASE_OPTIONS(),
+ KEY_OPTIONS(),
+ SANITIZE_OPTIONS(),
+ OPT_END(),
+};
+
static int dimm_action(int argc, const char **argv, void *ctx,
int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
const struct option *options, const char *usage)
@@ -1317,3 +1377,13 @@ int cmd_freeze_security(int argc, const char **argv, void *ctx)
count > 1 ? "s" : "");
return count >= 0 ? 0 : EXIT_FAILURE;
}
+
+int cmd_sanitize(int argc, const char **argv, void *ctx)
+{
+ int count = dimm_action(argc, argv, ctx, action_sanitize, sanitize_options,
+ "ndctl sanitize <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 fc0c2e57..dcff84c8 100644
--- a/ndctl/lib/dimm.c
+++ b/ndctl/lib/dimm.c
@@ -637,3 +637,12 @@ NDCTL_EXPORT int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm)
{
return ndctl_dimm_write_security(dimm, "freeze");
}
+
+NDCTL_EXPORT int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm,
+ key_serial_t key)
+{
+ char buf[SYSFS_ATTR_SIZE];
+
+ sprintf(buf, "erase %d\n", key);
+ return ndctl_dimm_write_security(dimm, buf);
+}
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 6fd1d3a9..155cf3c2 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -394,4 +394,5 @@ global:
ndctl_dimm_set_change_key;
ndctl_dimm_disable_security;
ndctl_dimm_freeze_security;
+ ndctl_dimm_secure_erase;
} LIBNDCTL_18;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 19c4974d..ba5af487 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -697,6 +697,7 @@ int ndctl_dimm_add_keys(struct ndctl_dimm *dimm, key_serial_t *new_key,
const char *old_payload);
int ndctl_dimm_disable_security(struct ndctl_dimm *dimm, key_serial_t key);
int ndctl_dimm_freeze_security(struct ndctl_dimm *dimm);
+int ndctl_dimm_secure_erase(struct ndctl_dimm *dimm, key_serial_t key);
#ifdef __cplusplus
} /* extern "C" */
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 7a242aab..07af7305 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -91,6 +91,7 @@ static struct cmd_struct commands[] = {
{ "update-security", cmd_key_update },
{ "disable-security", cmd_disable_security },
{ "freeze-security", cmd_freeze_security },
+ { "sanitize", cmd_sanitize },
{ "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] 13+ messages in thread
* [PATCH v4 6/7] ndctl: add request-key upcall reference app
2018-10-12 22:28 [PATCH v4 0/7] ndctl: add security support Dave Jiang
` (4 preceding siblings ...)
2018-10-12 22:29 ` [PATCH v4 5/7] ndctl: add support for sanitize dimm Dave Jiang
@ 2018-10-12 22:29 ` Dave Jiang
2018-10-16 3:06 ` Dan Williams
2018-10-12 22:29 ` [PATCH v4 7/7] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
6 siblings, 1 reply; 13+ messages in thread
From: Dave Jiang @ 2018-10-12 22:29 UTC (permalink / raw)
To: vishal.l.verma, dan.j.williams; +Cc: linux-nvdimm
Adding a reference upcall helper for request-key in order to retrieve the
security passphrase from userspace to provide to the kernel. The reference
app uses keyutils API to respond to the upcall from the kernel and is
invoked by /sbin/request-key of the keyutils.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
Documentation/ndctl/Makefile.am | 3 -
Documentation/ndctl/nvdimm-upcall.txt | 33 +++++++
Makefile.am | 5 +
contrib/nvdimm.conf | 1
ndctl.spec.in | 2
ndctl/Makefile.am | 4 +
ndctl/nvdimm-upcall.c | 154 +++++++++++++++++++++++++++++++++
7 files changed, 201 insertions(+), 1 deletion(-)
create mode 100644 Documentation/ndctl/nvdimm-upcall.txt
create mode 100644 contrib/nvdimm.conf
create mode 100644 ndctl/nvdimm-upcall.c
diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index 8c171ecb..f4e8b24c 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -51,7 +51,8 @@ man1_MANS = \
ndctl-update-security.1 \
ndctl-disable-security.1 \
ndctl-freeze-security.1 \
- ndctl-sanitize.1
+ ndctl-sanitize.1 \
+ nvdimm-upcall.1
CLEANFILES = $(man1_MANS)
diff --git a/Documentation/ndctl/nvdimm-upcall.txt b/Documentation/ndctl/nvdimm-upcall.txt
new file mode 100644
index 00000000..bbf52fd1
--- /dev/null
+++ b/Documentation/ndctl/nvdimm-upcall.txt
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+
+nvdimm-upcall(1)
+================
+
+NAME
+----
+nvdimm-upcall - upcall helper for keyutils
+
+SYNOPSIS
+--------
+[verse]
+'nvdimm-upcall' [key_id]
+
+DESCRIPTION
+-----------
+nvdimm-upcall is called by request-key from keyutils and not meant to be used
+directly by the user. It expects to read from /etc/nvdimm.passwd to retrieve
+the description and passphrase for the NVDIMM key.
+
+The nvdimm.passwd is formatted as:
+<description id>:<passphrase with padded 0 to 32bytes>
+cdab-0a-07e0-feffffff:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+
+In order for this util to be called, /etc/request-key.conf must be appended
+with the following line:
+create logon nvdimm* * /usr/sbin/nvdimm-upcall %k
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+http://pmem.io/documents/NVDIMM_DSM_Interface-V1.7.pdf
diff --git a/Makefile.am b/Makefile.am
index e0c463a3..73aa2645 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -42,6 +42,11 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
dist_bashcompletion_DATA = contrib/ndctl
endif
+key_config_file = contrib/nvdimm.conf
+key_configdir = $(sysconfdir)/request-key.d/
+key_config_DATA = $(key_config_file)
+EXTRA_DIST += $(key_config_file)
+
noinst_LIBRARIES = libccan.a
libccan_a_SOURCES = \
ccan/str/str.h \
diff --git a/contrib/nvdimm.conf b/contrib/nvdimm.conf
new file mode 100644
index 00000000..e1f9a28b
--- /dev/null
+++ b/contrib/nvdimm.conf
@@ -0,0 +1 @@
+create logon nvdimm:* * /usr/sbin/nvdimm-upcall %k
diff --git a/ndctl.spec.in b/ndctl.spec.in
index f57b4fd5..ef591615 100644
--- a/ndctl.spec.in
+++ b/ndctl.spec.in
@@ -120,6 +120,8 @@ make check
%{bashcompdir}/
%{_sysconfdir}/ndctl/monitor.conf
%{_unitdir}/ndctl-monitor.service
+%{_sbindir}/nvdimm-upcall
+%{_sysconfdir}/request-key.d/nvdimm.conf
%files -n daxctl
%defattr(-,root,root)
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 5b62251b..24ca98c7 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -1,6 +1,7 @@
include $(top_srcdir)/Makefile.am.in
bin_PROGRAMS = ndctl
+sbin_PROGRAMS = nvdimm-upcall
ndctl_SOURCES = ndctl.c \
bus.c \
@@ -52,3 +53,6 @@ EXTRA_DIST += $(monitor_config_file)
if ENABLE_SYSTEMD_UNITS
systemd_unit_DATA = ndctl-monitor.service
endif
+
+nvdimm_upcall_SOURCES = nvdimm-upcall.c
+nvdimm_upcall_LDADD = -lkeyutils
diff --git a/ndctl/nvdimm-upcall.c b/ndctl/nvdimm-upcall.c
new file mode 100644
index 00000000..c8248359
--- /dev/null
+++ b/ndctl/nvdimm-upcall.c
@@ -0,0 +1,154 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+
+/*
+ * Used by /sbin/request-key for handling nvdimm upcall of key requests
+ * for security DSMs.
+ */
+
+
+#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 <ctype.h>
+
+#define PASSPHRASE_SIZE 32
+#define PASS_PATH "/etc/nvdimm.passwd"
+
+static FILE *fp;
+
+static int get_passphrase_from_id(const char *id, char *pass)
+{
+ ssize_t rc = 0;
+ size_t size = 0, i;
+ char *line = NULL;
+ char *desc, *pass1;
+ int found = 0, comment = 0;
+
+ fp = fopen(PASS_PATH, "r+");
+ if (!fp) {
+ syslog(LOG_ERR, "fopen: %s\n", strerror(errno));
+ return -errno;
+ }
+
+ while ((rc = getline(&line, &size, fp)) != -1) {
+ /* skip comments */
+ for (i = 0; i < size; i++) {
+ if (isspace(line[i]))
+ continue;
+ if (line[i] == '#')
+ comment = 1;
+ }
+
+ if (comment) {
+ comment = 0;
+ continue;
+ }
+
+ desc = strtok(line, ":");
+ if (!desc)
+ break;
+ if (strcmp(desc, id) == 0) {
+ found = 1;
+ rc = 0;
+ break;
+ }
+ }
+
+ if (rc == 0 && found) {
+ pass1 = strtok(NULL, ":");
+ if (!pass1) {
+ rc = -EINVAL;
+ goto out;
+ }
+ memset(pass, 0, PASSPHRASE_SIZE);
+ memcpy(pass, pass1, MIN(strlen(pass1), PASSPHRASE_SIZE));
+ } else
+ rc = -ENXIO;
+
+out:
+ free(line);
+ fclose(fp);
+ return rc;
+}
+
+static char *get_key_desc(char *buf)
+{
+ char *tmp = &buf[0];
+ int count = 0;
+
+ while (*tmp != '\0') {
+ if (*tmp == ';')
+ count++;
+ if (count == 4) {
+ tmp++;
+ return tmp;
+ }
+ tmp++;
+ }
+
+ return NULL;
+}
+
+int main(int argc, const char **argv)
+{
+ key_serial_t key;
+ int rc;
+ char *buf, *desc, *dimm_id;
+ char pass[PASSPHRASE_SIZE];
+
+ if (argc < 2) {
+ syslog(LOG_ERR, "Incorrect number of arguments\n");
+ return -EINVAL;
+ }
+
+ syslog(LOG_DEBUG, "key passed in: %s\n", argv[1]);
+ key = strtol(argv[1], NULL, 10);
+ if (key < 0) {
+ syslog(LOG_ERR, "Invalid key format: %s\n", strerror(errno));
+ return -errno;
+ }
+
+ rc = keyctl_describe_alloc(key, &buf);
+ if (rc < 0) {
+ syslog(LOG_ERR, "keyctl_describe_alloc failed: %s\n",
+ strerror(errno));
+ rc = -errno;
+ goto out;
+ }
+
+ desc = get_key_desc(buf);
+ if (!desc) {
+ syslog(LOG_ERR, "Can't find key description\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ strtok_r(desc, ":", &dimm_id);
+ rc = get_passphrase_from_id(dimm_id, pass);
+ if (rc < 0) {
+ syslog(LOG_ERR, "failed to retrieve passphrase\n");
+ goto out;
+ }
+
+ rc = keyctl_instantiate(key, pass, PASSPHRASE_SIZE, 0);
+ if (rc < 0) {
+ syslog(LOG_ERR, "keyctl_instantiate failed: %s\n",
+ strerror(errno));
+ rc = -errno;
+ goto out;
+ }
+
+ out:
+ if (rc < 0)
+ keyctl_negate(key, 1, KEY_REQKEY_DEFL_DEFAULT);
+
+ return rc;
+}
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 7/7] ndctl: add unit test for security ops (minus overwrite)
2018-10-12 22:28 [PATCH v4 0/7] ndctl: add security support Dave Jiang
` (5 preceding siblings ...)
2018-10-12 22:29 ` [PATCH v4 6/7] ndctl: add request-key upcall reference app Dave Jiang
@ 2018-10-12 22:29 ` Dave Jiang
6 siblings, 0 replies; 13+ messages in thread
From: Dave Jiang @ 2018-10-12 22:29 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 | 3 +
test/security.sh | 187 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 189 insertions(+), 1 deletion(-)
create mode 100755 test/security.sh
diff --git a/test/Makefile.am b/test/Makefile.am
index ebdd23f6..68adfdee 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -25,7 +25,8 @@ TESTS =\
inject-smart.sh \
monitor.sh \
max_available_extent_ns.sh \
- pfn-meta-errors.sh
+ pfn-meta-errors.sh \
+ security.sh
check_PROGRAMS =\
libndctl \
diff --git a/test/security.sh b/test/security.sh
new file mode 100755
index 00000000..07d9dd7d
--- /dev/null
+++ b/test/security.sh
@@ -0,0 +1,187 @@
+#!/bin/bash -Ex
+# SPDX-License-Identifier: GPL-2.0
+# Copyright(c) 2018 Intel Corporation. All rights reserved.
+
+rc=77
+dev=""
+id=""
+dev_no=""
+sstate=""
+PASSWD="/etc/nvdimm.passwd"
+PASSWD_BACKUP="/etc/nvdimm.passwd.ndctl.backup"
+PASS1="aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
+PASS2="bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"
+UNLOCK="/sys/devices/platform/nfit_test.0/nfit_test_dimm/test_dimm"
+
+. ./common
+
+trap 'err $LINENO' ERR
+
+setup()
+{
+ $NDCTL disable-region -b $NFIT_TEST_BUS0 all
+}
+
+detect()
+{
+ dev=$($NDCTL list -b $NFIT_TEST_BUS0 -D | jq .[0].dev | tr -d '"')
+ [ -n "$dev" ] || err "$LINENO"
+ id=$($NDCTL list -b $NFIT_TEST_BUS0 -D | jq .[0].id | tr -d '"')
+ [ -n "$id" ] || err "$LINENO"
+}
+
+setup_passwd()
+{
+ if [ ! -f $PASSWD_BACKUP ]; then
+ cp $PASSWD $PASSWD_BACKUP
+ echo "$id:$PASS1" > $PASSWD
+ else
+ echo "Unclean setup. Please cleanup $PASSWD_BACKUP file."
+ exit 1
+ fi
+}
+
+test_restore()
+{
+ if [ -f $PASSWD_BACKUP ]; then
+ mv $PASSWD.ndctl.backup $PASSWD
+ fi
+}
+
+locking_dimm()
+{
+ $NDCTL disable-dimm $dev
+ dev_no=$(echo $dev | cut -b 5-)
+ echo 1 > "$UNLOCK$dev_no/lock_dimm"
+ get_security_state
+ if [ "$sstate" != "locked" ]; then
+ echo "Incorrect security state: $sstate expected: disabled"
+ exit 1
+ fi
+}
+
+get_security_state()
+{
+ sstate=$($NDCTL list -i -b $NFIT_TEST_BUS0 -d $dev | jq .[].dimms[0].security_state | tr -d '"')
+ [ -n "$sstate" ] || err "$LINENO"
+}
+
+enable_security()
+{
+ $NDCTL update-security -i $dev
+ get_security_state
+ if [ "$sstate" != "unlocked" ]; then
+ echo "Incorrect security state: $sstate expected: unlocked"
+ exit 1
+ fi
+}
+
+disable_security()
+{
+ $NDCTL disable-security -i $dev
+ get_security_state
+ if [ "$sstate" != "disabled" ]; then
+ echo "Incorrect security state: $sstate expected: disabled"
+ exit 1
+ fi
+}
+
+erase_security()
+{
+ $NDCTL sanitize -m crypto-erase -i $dev
+ get_security_state
+ if [ "$sstate" != "disabled" ]; then
+ echo "Incorrect security state: $sstate expected: disabled"
+ exit 1
+ fi
+}
+
+update_security()
+{
+ if [ -f $PASSWD_BACKUP ]; then
+ echo "$id:$PASS2:$PASS1" > $PASSWD
+ fi
+ enable_security
+ echo "$id:$PASS2" > $PASSWD
+}
+
+freeze_security()
+{
+ $NDCTL freeze-security $dev
+}
+
+test_1_security_enable_and_disable()
+{
+ enable_security
+ disable_security
+}
+
+test_2_security_enable_and_update()
+{
+ enable_security
+ update_security
+ disable_security
+}
+
+test_3_security_enable_and_erase()
+{
+ enable_security
+ erase_security
+}
+
+test_4_security_unlocking()
+{
+ enable_security
+ locking_dimm
+ $NDCTL enable-dimm $dev
+ get_security_state
+ if [ "$sstate" != "unlocked" ]; then
+ echo "Incorrect security state: $sstate expected: unlocked"
+ exit 1
+ fi
+ $NDCTL disable-region -b $NFIT_TEST_BUS0 all
+ disable_security
+}
+
+# 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_security
+ freeze_security
+ get_security_state
+ if [ "$sstate" != "frozen" ]; then
+ echo "Incorrect security state: $sstate expected: frozen"
+ exit 1
+ fi
+ $NDCTL disable-security -i $dev && { echo "diable succeed after frozen"; exit 1; }
+ get_security_state
+ echo $sstate
+ if [ "$sstate" != "frozen" ]; then
+ echo "Incorrect security state: $sstate expected: disabled"
+ exit 1
+ fi
+}
+
+check_min_kver "4.20" || do_skip "may lack security test handling"
+
+modprobe nfit_test
+rc=1
+setup
+rc=2
+detect
+setup_passwd
+echo "Test 1, security enable and disable"
+test_1_security_enable_and_disable
+echo "Test 2, security enable, update, and disable"
+test_2_security_enable_and_update
+echo "Test 3, security enable and erase"
+test_3_security_enable_and_erase
+echo "Test 4, unlocking dimm"
+test_4_security_unlocking
+echo "Test 5, freeze security"
+test_5_security_freeze
+
+test_restore
+_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] 13+ messages in thread
* Re: [PATCH v4 1/7] ndctl: add support for display security state
2018-10-12 22:28 ` [PATCH v4 1/7] ndctl: add support for display security state Dave Jiang
@ 2018-10-13 17:57 ` Dan Williams
2018-10-15 22:12 ` Dan Williams
1 sibling, 0 replies; 13+ messages in thread
From: Dan Williams @ 2018-10-13 17:57 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Fri, Oct 12, 2018 at 3:28 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 | 16 ++++++++++++++++
> ndctl/lib/libndctl.sym | 5 +++++
> ndctl/libndctl.h | 1 +
> util/json.c | 8 ++++++++
> 5 files changed, 38 insertions(+)
>
> diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
> index e24c8f40..a4a712a0 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_state:":"disabled"
> +}
>
> -H::
> --health::
> diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
> index 5e41734d..4470d5fe 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -583,3 +583,19 @@ NDCTL_EXPORT unsigned long ndctl_dimm_get_available_labels(
>
> return strtoul(buf, NULL, 0);
> }
> +
> +NDCTL_EXPORT int ndctl_dimm_get_security_state(struct ndctl_dimm *dimm,
> + char *state)
> +{
> + 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_read_attr(ctx, path, state);
This should return a security state enum value instead of a string.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/7] ndctl: add support for display security state
2018-10-12 22:28 ` [PATCH v4 1/7] ndctl: add support for display security state Dave Jiang
2018-10-13 17:57 ` Dan Williams
@ 2018-10-15 22:12 ` Dan Williams
1 sibling, 0 replies; 13+ messages in thread
From: Dan Williams @ 2018-10-15 22:12 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Fri, Oct 12, 2018 at 3:28 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 | 16 ++++++++++++++++
> ndctl/lib/libndctl.sym | 5 +++++
> ndctl/libndctl.h | 1 +
> util/json.c | 8 ++++++++
> 5 files changed, 38 insertions(+)
>
> diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
> index e24c8f40..a4a712a0 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_state:":"disabled"
Lets drop the _state suffix.
> +}
>
> -H::
> --health::
> diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
> index 5e41734d..4470d5fe 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -583,3 +583,19 @@ NDCTL_EXPORT unsigned long ndctl_dimm_get_available_labels(
>
> return strtoul(buf, NULL, 0);
> }
> +
> +NDCTL_EXPORT int ndctl_dimm_get_security_state(struct ndctl_dimm *dimm,
> + char *state)
> +{
Same here, lets drop the _state suffix, and combined with the last
comment about returning an enum makes it clearer what this function is
returning.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/7] ndctl: add update to security support
2018-10-12 22:28 ` [PATCH v4 2/7] ndctl: add update to security support Dave Jiang
@ 2018-10-16 0:59 ` Dan Williams
0 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2018-10-16 0:59 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Fri, Oct 12, 2018 at 3:28 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-security" to trigger that as well.
> ndctl does not actually handle the passphrase file. It only initiates the
> update and expects all the necessary mechanisms are already in place.
One of the mistakes of create-namespace was trying to combine multiple
verbs ('create' and 'reconfigure') into one command. Since this is
operating on the passphrase state of the DIMM and we may have
non-passphrase based security in the future lets make the command set:
enable-passphrase
disable-passphrase
change-passphrase
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> Documentation/ndctl/Makefile.am | 3 -
> Documentation/ndctl/ndctl-update-security.txt | 56 ++++++++++
> builtin.h | 1
> configure.ac | 5 +
> ndctl.spec.in | 1
> ndctl/Makefile.am | 3 -
> ndctl/dimm.c | 90 ++++++++++++++--
> ndctl/lib/Makefile.am | 4 +
> ndctl/lib/dimm.c | 24 ++++
> ndctl/lib/keys.c | 139 +++++++++++++++++++++++++
> ndctl/lib/libndctl.sym | 3 +
> ndctl/libndctl.h | 14 +++
> ndctl/ndctl.c | 1
> 13 files changed, 330 insertions(+), 14 deletions(-)
> create mode 100644 Documentation/ndctl/ndctl-update-security.txt
> create mode 100644 ndctl/lib/keys.c
>
> diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
> index a30b139b..2c064c3a 100644
> --- a/Documentation/ndctl/Makefile.am
> +++ b/Documentation/ndctl/Makefile.am
> @@ -47,7 +47,8 @@ man1_MANS = \
> ndctl-inject-smart.1 \
> ndctl-update-firmware.1 \
> ndctl-list.1 \
> - ndctl-monitor.1
> + ndctl-monitor.1 \
> + ndctl-update-security.1
>
> CLEANFILES = $(man1_MANS)
>
> diff --git a/Documentation/ndctl/ndctl-update-security.txt b/Documentation/ndctl/ndctl-update-security.txt
> new file mode 100644
> index 00000000..96d519b1
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-update-security.txt
> @@ -0,0 +1,56 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +ndctl-update-security(1)
> +========================
> +
> +NAME
> +----
> +ndctl-update-security - enabling or update the security passphrase for a
> +NVDIMM
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl update-security' <dimm> [<options>]
> +
> +DESCRIPTION
> +-----------
> +Provide a generic interface for enabling or updating security passphrase for
> +NVDIMM. The use of this depends on support from the underlying libndctl,
> +kernel, as well as the platform itself.
No need to mention "underlying libndctl". The packaging guarantees
that the library version is always installed from the same package as
the binary. I.e. you can't update the binaries without it force
updating to the latest library.
I'd rewrite this to be in terms of something they can detect. I.e.
something like "if 'ndctl list -D' shows a 'security:' attribute then
passphrase management is available".
Hmm, now as I type that I wonder if we should call all of this
'passphrase' in all the user facing names? Since 'security' mechanisms
can change from one DIMM vendor to the next.
> +
> +For the reference passphrase setup, /etc/nvdimm.passwd is read for passphrase
I assume that's where the upcall binary is going to look? Lets convert
that to /etc/ndctl/passphrase.d so we can support a file per dimm
device.
I'd also drop the mention of "reference" and instead provide a way to
read passphrase material over stdio or optionally a file.
I think we need a:
generate-passphrase
...helper that takes an nmemX list and spits out properly formatted /
randomly generated passphrase for the given DIMM.
That way you can do something like:
ndctl generate-passphrase nmem{0,1,2,3} > /etc/ndctl/passphrase.d/keys.conf
ndctl enable-passphrase < /etc/ndctl/passphrase.d/keys.conf
...where enable-passphrase should be able to identify the DIMM to
operate on by description-id.
> +retrieval:
> +
> +The nvdimm.passwd is formatted as:
> +<description id>:<passphrase with padded 0 to 32bytes>
> +cdab-0a-07e0-feffffff:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +
> +To update a DIMM that has passphrase already enabled, the format is done as:
> +<description id>:<new passphrase with padded 0 to 32bytes>:<old passphrase with padded 0
> +to 32 bytes>
> +
> +cdab-0a-07e0-feffffff:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +
> +The accepted key will replace the old payload with the new payload in the
> +cached key that resides in the kernel key ring.
I think having an extended format for the change key operation is
awkward. We could just make the requirement that when you want to
change the key input stream needs to list the key exactly twice. The
first occurrence is the old key and the second the new one. Then you
could do something like:
cat old-keys.conf keys.conf | ndctl change-passphrase
> +OPTIONS
> +-------
> +<dimm>::
> +include::xable-dimm-options.txt[]
> +
> +-i::
> +--insecure::
> + Using the default reference support to parse the nvdimm passphrase
> + file, inject the key, and initiate update operation. This is labeled
> + as insecure as it just provides a reference to how to inject keys
> + for the nvdimm. The passphrase is in clear text and is not considered
> + as secure as it can be.
If the command supports taking input over stdio then we're secure as
we can be unless / until we can update the kernel implementation to
take TPM or other wrapped keys.
> +-e::
> +--exec::
> + The external binary module that would inject the passphrase and
> + initiate the update operation. Use this or -i, not both.
Does stdio support negate the need for this option?
> +
> +include::../copyright.txt[]
> diff --git a/builtin.h b/builtin.h
> index 675a6ce7..37404c79 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -48,4 +48,5 @@ int cmd_bat(int argc, const char **argv, void *ctx);
> #endif
> int cmd_update_firmware(int argc, const char **argv, void *ctx);
> int cmd_inject_smart(int argc, const char **argv, void *ctx);
> +int cmd_key_update(int argc, const char **argv, void *ctx);
> #endif /* _NDCTL_BUILTIN_H_ */
> diff --git a/configure.ac b/configure.ac
> index bb6b0332..1cfd9718 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -153,6 +153,11 @@ fi
> AC_SUBST([systemd_unitdir])
> AM_CONDITIONAL([ENABLE_SYSTEMD_UNITS], [test "x$with_systemd" = "xyes"])
>
> +AC_CHECK_HEADERS([keyutils.h],,[
> + AC_MSG_ERROR([keyutils.h not found, consider installing
> + keyutils-libs-devel.])
> + ])
It shouldn't be an error to build without keyutils support. I'd gate
this with an optional "--with=keyutils" specified to the configure
script.
> +
> my_CFLAGS="\
> -D DEF_CONF_FILE='\"${sysconfdir}/ndctl/monitor.conf\"' \
> -Wall \
> diff --git a/ndctl.spec.in b/ndctl.spec.in
> index 26396d4a..f57b4fd5 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
'keyutils' does not provide any build material. Looking at nfs-utils
it only has "Requires: keyutils", and looking even closer it was only
required there because of
https://bugzilla.redhat.com/show_bug.cgi?id=769724
So I think make this "BuildRequires: keyutils-libs-devel".
>
> %description
> Utility library for managing the "libnvdimm" subsystem. The "libnvdimm"
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index 8a5e5f87..5b62251b 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -30,7 +30,8 @@ ndctl_LDADD =\
> ../libutil.a \
> $(UUID_LIBS) \
> $(KMOD_LIBS) \
> - $(JSON_LIBS)
> + $(JSON_LIBS) \
> + -lkeyutils
Should be gated on a conditional if keyutils support is enabled.
>
> if ENABLE_TEST
> ndctl_SOURCES += ../test/libndctl.c \
> diff --git a/ndctl/dimm.c b/ndctl/dimm.c
> index 699ab57d..62a102b4 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_exec;
> + bool force;
> + bool json;
> + bool verbose;
> + bool key_insecure;
> +} 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,44 @@ 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(struct ndctl_dimm *dimm,
> + struct action_context *actx)
> +{
> + int rc, po_valid, pn_valid;
> + char old_payload[ND_PASSPHRASE_SIZE];
> + char new_payload[ND_PASSPHRASE_SIZE];
> + key_serial_t old_key, new_key;
> + char *po;
> +
> + if (param.key_exec)
> + return execl(param.key_exec, ndctl_dimm_get_devname(dimm),
> + ndctl_dimm_get_unique_id(dimm), (char *)NULL);
> +
> + if (!param.key_insecure)
> + return -EINVAL;
> +
> + rc = ndctl_dimm_get_key_payload(dimm, new_payload, &pn_valid,
> + old_payload, &po_valid);
> + if (rc < 0)
> + return rc;
> +
> + /*
> + * If we are turning on security rather than updating, then
> + * we would not have an old key.
> + */
> + po = po_valid ? old_payload : NULL;
> + rc = ndctl_dimm_add_keys(dimm, &new_key, new_payload,
> + &old_key, po);
> + if (rc < 0)
> + return rc;
> +
> + /* now do actual writing to update */
> + rc = ndctl_dimm_set_change_key(dimm, old_key, new_key);
> + if (rc < 0)
> + return rc;
> +
> + return rc;
> +}
>
> static int __action_init(struct ndctl_dimm *dimm,
> enum ndctl_namespace_version version, int chk_only)
> @@ -925,6 +966,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_BOOLEAN('i', "insecure", ¶m.key_insecure, \
> + "insecure reference passphrase update method"), \
> +OPT_STRING('e', "exec", ¶m.key_exec, "external-exec", \
> + "external exec module for passphrase update")
> +
> static const struct option read_options[] = {
> BASE_OPTIONS(),
> READ_OPTIONS(),
> @@ -954,6 +1001,12 @@ static const struct option init_options[] = {
> OPT_END(),
> };
>
> +static const struct option key_options[] = {
> + BASE_OPTIONS(),
> + KEY_OPTIONS(),
> + OPT_END(),
> +};
> +
> static int dimm_action(int argc, const char **argv, void *ctx,
> int (*action)(struct ndctl_dimm *dimm, struct action_context *actx),
> const struct option *options, const char *usage)
> @@ -1024,6 +1077,11 @@ static int dimm_action(int argc, const char **argv, void *ctx,
> }
> }
>
> + if (action == action_key && param.key_insecure && param.key_exec) {
> + usage_with_options(u, options);
> + return -EINVAL;
> + }
> +
> if (param.verbose)
> ndctl_set_log_priority(ctx, LOG_DEBUG);
>
> @@ -1181,3 +1239,13 @@ int cmd_update_firmware(int argc, const char **argv, void *ctx)
> count > 1 ? "s" : "");
> return count >= 0 ? 0 : EXIT_FAILURE;
> }
> +
> +int cmd_key_update(int argc, const char **argv, void *ctx)
> +{
> + int count = dimm_action(argc, argv, ctx, action_key, key_options,
> + "ndctl update-security <nmem0> [<nmem1>..<nmemN>] [<options>]");
> +
> + fprintf(stderr, "security updated 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..245b78a9 100644
> --- a/ndctl/lib/Makefile.am
> +++ b/ndctl/lib/Makefile.am
> @@ -22,13 +22,15 @@ libndctl_la_SOURCES =\
> msft.c \
> ars.c \
> firmware.c \
> + keys.c \
> libndctl.c
>
> libndctl_la_LIBADD =\
> ../../daxctl/lib/libdaxctl.la \
> $(UDEV_LIBS) \
> $(UUID_LIBS) \
> - $(KMOD_LIBS)
> + $(KMOD_LIBS) \
> + $(KEYUTILS_LIBS)
Does this work? I thought these _LIBS variables were generated by the
autotools pkgconfig support.
>
> EXTRA_DIST += libndctl.sym
>
> diff --git a/ndctl/lib/dimm.c b/ndctl/lib/dimm.c
> index 4470d5fe..c9ef894a 100644
> --- a/ndctl/lib/dimm.c
> +++ b/ndctl/lib/dimm.c
> @@ -599,3 +599,27 @@ NDCTL_EXPORT int ndctl_dimm_get_security_state(struct ndctl_dimm *dimm,
>
> return sysfs_read_attr(ctx, path, state);
> }
> +
> +static int ndctl_dimm_write_security(struct ndctl_dimm *dimm, const char *cmd)
If it's static, it does not need the ndctl_dimm_ prefix.
> +{
> + 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_set_change_key(struct ndctl_dimm *dimm,
> + key_serial_t ckey, key_serial_t nkey)
> +{
> + char buf[SYSFS_ATTR_SIZE];
> +
> + sprintf(buf, "update %d %d\n", ckey, nkey);
> + return ndctl_dimm_write_security(dimm, buf);
> +}
I don't think we need to have an ndctl api that exposes the raw
keyutils details. I think the ndctl apis should only deal in
passphrases and DIMMs.
It might be useful to have a helper api that takes a file stream and
converts it to a key list as that seems to be a common operation that
the ndctl passphrase operations will want to do.
> diff --git a/ndctl/lib/keys.c b/ndctl/lib/keys.c
> new file mode 100644
> index 00000000..b21d11f0
> --- /dev/null
> +++ b/ndctl/lib/keys.c
> @@ -0,0 +1,139 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
Checkpatch complained to me that the "official" way to do SPDX in C
code was with a C++ // style comment. Who knew?
> +/* 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"
> +
> +NDCTL_EXPORT int ndctl_dimm_get_key_payload(struct ndctl_dimm *dimm,
> + char *pass, int *pass_valid,
> + char *oldpass, int *oldpass_valid)
What's the intended use case of this api, and the next?
> +{
> + struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> + ssize_t rc = 0;
> + static FILE *fp;
> + const char *dimm_id;
> + size_t size = 0, i;
> + char *line = NULL, *desc, *pass1, *pass2;
> + int found = 0, comment = 0;
> +
> + *pass_valid = 0;
> + *oldpass_valid = 0;
> +
> + dimm_id = ndctl_dimm_get_unique_id(dimm);
> + if (!dimm_id)
> + return -EINVAL;
> +
> + fp = fopen(PASS_PATH, "r+");
> + if (!fp) {
> + err(ctx, "fopen: %s\n", strerror(errno));
> + return -errno;
> + }
> +
> + while ((rc = getline(&line, &size, fp)) != -1) {
> + /* skip comments */
> + for (i = 0; i < size; i++) {
> + if (isspace(line[i]))
> + continue;
> + if (line[i] == '#')
> + comment = 1;
> + }
> +
> + if (comment) {
> + comment = 0;
> + continue;
> + }
> +
> + desc = strtok(line, ":");
> + if (!desc)
> + break;
> + if (strcmp(desc, dimm_id) == 0) {
> + found = 1;
> + rc = 0;
> + break;
> + }
> + }
> +
> + memset(oldpass, 0, ND_PASSPHRASE_SIZE);
> + memset(pass, 0, ND_PASSPHRASE_SIZE);
> + if (rc == 0 && found) {
> + pass1 = strtok(NULL, ":");
> + pass2 = strtok(NULL, ":");
> + if (!pass1) {
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + if (strlen(pass1) > ND_PASSPHRASE_SIZE)
> + pass1[ND_PASSPHRASE_SIZE] = '\0';
> +
> + if (pass2 && strlen(pass2) > ND_PASSPHRASE_SIZE)
> + pass2[ND_PASSPHRASE_SIZE] = '\0';
> +
> + memcpy(pass, pass1, strlen(pass1));
> + *pass_valid = 1;
> +
> + if (pass2) {
> + memcpy(oldpass, pass2, strlen(pass2));
> + *oldpass_valid = 1;
> + }
> + } else
> + rc = -ENXIO;
> +
> +out:
> + free(line);
> + fclose(fp);
> + return rc;
> +}
> +
> +NDCTL_EXPORT int ndctl_dimm_add_keys(struct ndctl_dimm *dimm,
> + key_serial_t *new_key, const char *payload1,
> + key_serial_t *old_key, const char *payload2)
> +{
> + int rc;
> + char desc[ND_KEY_DESC_LEN + ND_KEY_DESC_PREFIX];
> + char odesc[ND_KEY_DESC_LEN + 4 + ND_KEY_DESC_PREFIX];
> + struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
> +
> + rc = sprintf(desc, "nvdimm:%s", ndctl_dimm_get_unique_id(dimm));
> + if (rc < 0) {
> + err(ctx, "sprintf: %s\n", strerror(errno));
> + return -errno;
> + }
> +
> + rc = sprintf(odesc, "nvdimm_old:%s", ndctl_dimm_get_unique_id(dimm));
> + if (rc < 0) {
> + err(ctx, "sprintf: %s\n", strerror(errno));
> + return -errno;
> + }
> +
> + *new_key = add_key("logon", desc, payload1, ND_PASSPHRASE_SIZE,
> + KEY_SPEC_USER_KEYRING);
> + if (*new_key == -1) {
> + err(ctx, "add_key: %s\n", strerror(errno));
> + return -errno;
> + }
> +
> + if (payload2) {
> + *old_key = add_key("logon", odesc, payload2,
> + ND_PASSPHRASE_SIZE, KEY_SPEC_USER_KEYRING);
> + if (*old_key == -1) {
> + err(ctx, "add_key: %s\n", strerror(errno));
> + return -errno;
> + }
> + } else
> + *old_key = 0;
> +
> + return 0;
> +}
I think we need a family of apis that makes it straightforward for an
application to handle passphrases.
ndctl_dimm_set_current_passphrase - cache a passphrase on the DIMM object
ndctl_dimm_set_next_passphrase - cache a passphrase on the DIMM object
ndctl_dimm_get_current_passphrase - return the cached passphrase
ndctl_dimm_get_next_passphrase - return the cached next passphrase
ndctl_dimm_enable_passphrase - try to turn on security with the cached
passphrase
ndctl_dimm_disable_passphrase - disable, clear out cached passphrase
ndctl_dimm_change_passphrase - use cached current and next passphrases
to perform key update to kernel, cache the next passphrase as current
ndctl_dimm_dump_current_passphrase - write the current configuration
ndctl_dimm_dump_next_passphrase - write the next
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 5/7] ndctl: add support for sanitize dimm
2018-10-12 22:29 ` [PATCH v4 5/7] ndctl: add support for sanitize dimm Dave Jiang
@ 2018-10-16 1:25 ` Dan Williams
0 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2018-10-16 1:25 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Fri, Oct 12, 2018 at 3:29 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> Add support to secure erase to libndctl and also command line option
> of "sanitize" for ndctl. This will initiate the request to crypto
> erase a DIMM. ndctl does not actually handle the verification of the
> security. That is handled by the kernel and the key upcall mechanism.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> Documentation/ndctl/Makefile.am | 3 +
> Documentation/ndctl/ndctl-sanitize.txt | 52 ++++++++++++++++++++++++
> builtin.h | 1
> ndctl/dimm.c | 70 ++++++++++++++++++++++++++++++++
> ndctl/lib/dimm.c | 9 ++++
> ndctl/lib/libndctl.sym | 1
> ndctl/libndctl.h | 1
> ndctl/ndctl.c | 1
> 8 files changed, 137 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/ndctl/ndctl-sanitize.txt
>
> diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
> index 3a761ba0..8c171ecb 100644
> --- a/Documentation/ndctl/Makefile.am
> +++ b/Documentation/ndctl/Makefile.am
> @@ -50,7 +50,8 @@ man1_MANS = \
> ndctl-monitor.1 \
> ndctl-update-security.1 \
> ndctl-disable-security.1 \
> - ndctl-freeze-security.1
> + ndctl-freeze-security.1 \
> + ndctl-sanitize.1
>
> CLEANFILES = $(man1_MANS)
>
> diff --git a/Documentation/ndctl/ndctl-sanitize.txt b/Documentation/ndctl/ndctl-sanitize.txt
> new file mode 100644
> index 00000000..a02b4b31
> --- /dev/null
> +++ b/Documentation/ndctl/ndctl-sanitize.txt
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +ndctl-sanitize(1)
> +=================
> +
> +NAME
> +----
> +ndctl-sanitize - sanitize the data on the NVDIMM
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'ndctl sanitize' <dimm> [<options>]
Lets call it secure-erase-dimm since to leave room for per-namespace
security commands in the future.
> +DESCRIPTION
> +-----------
> +Provide a generic interface to crypto erase a NVDIMM.
> +The use of this depends on support from the underlying
> +libndctl, kernel, as well as the platform itself.
Similar comment about dependencies as patch 2 I think it goes without saying.
> +
> +For the reference passphrase setup, /etc/nvdimm.passwd is read for passphrase
> +retrieval:
> +
> +The nvdimm.passwd is formatted as:
> +<description id>:<passphrase with padded 0 to 32bytes>
> +cdab-0a-07e0-feffffff:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +
> +OPTIONS
> +-------
> +<dimm>::
> +include::xable-dimm-options.txt[]
> +
> +-m::
> +--method::
> + The method for sanitizing the dimm content.
> +
> + crypto-erase: replaces encryption keys. This does not change label data.
I assume this is to differentiate secure-erase vs overwrite? Given
overwrite is such an odd mechanism that needs to be monitored for
completion I'd put that off in its own command.
> +
> +-i::
> +--insecure::
> + Using the default reference support to parse the nvdimm passphrase
> + file, inject the key, and initiate disable operation. This is labeled
> + as insecure as it just provides a reference to how to inject keys
> + for the nvdimm. The passphrase is in clear text and is not considered
> + as secure as it can be.
> +
> +-e::
> +--exec::
> + The external binary module that would inject the passphrase and
> + initiate the disable operation. Use this or -i, not both.
Same comments about taking key material over stdio.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 6/7] ndctl: add request-key upcall reference app
2018-10-12 22:29 ` [PATCH v4 6/7] ndctl: add request-key upcall reference app Dave Jiang
@ 2018-10-16 3:06 ` Dan Williams
0 siblings, 0 replies; 13+ messages in thread
From: Dan Williams @ 2018-10-16 3:06 UTC (permalink / raw)
To: Dave Jiang; +Cc: linux-nvdimm
On Fri, Oct 12, 2018 at 3:29 PM Dave Jiang <dave.jiang@intel.com> wrote:
>
> Adding a reference upcall helper for request-key in order to retrieve the
> security passphrase from userspace to provide to the kernel. The reference
> app uses keyutils API to respond to the upcall from the kernel and is
> invoked by /sbin/request-key of the keyutils.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> Documentation/ndctl/Makefile.am | 3 -
> Documentation/ndctl/nvdimm-upcall.txt | 33 +++++++
> Makefile.am | 5 +
> contrib/nvdimm.conf | 1
> ndctl.spec.in | 2
> ndctl/Makefile.am | 4 +
> ndctl/nvdimm-upcall.c | 154 +++++++++++++++++++++++++++++++++
> 7 files changed, 201 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/ndctl/nvdimm-upcall.txt
> create mode 100644 contrib/nvdimm.conf
> create mode 100644 ndctl/nvdimm-upcall.c
>
> diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
> index 8c171ecb..f4e8b24c 100644
> --- a/Documentation/ndctl/Makefile.am
> +++ b/Documentation/ndctl/Makefile.am
> @@ -51,7 +51,8 @@ man1_MANS = \
> ndctl-update-security.1 \
> ndctl-disable-security.1 \
> ndctl-freeze-security.1 \
> - ndctl-sanitize.1
> + ndctl-sanitize.1 \
> + nvdimm-upcall.1
Any particular reason to not make this an ndctl command? It makes it
discoverable with --list-cmds and the man page can be had via --help.
I'd call it 'ndctl request-key'.
>
> CLEANFILES = $(man1_MANS)
>
> diff --git a/Documentation/ndctl/nvdimm-upcall.txt b/Documentation/ndctl/nvdimm-upcall.txt
> new file mode 100644
> index 00000000..bbf52fd1
> --- /dev/null
> +++ b/Documentation/ndctl/nvdimm-upcall.txt
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +nvdimm-upcall(1)
> +================
> +
> +NAME
> +----
> +nvdimm-upcall - upcall helper for keyutils
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'nvdimm-upcall' [key_id]
> +
> +DESCRIPTION
> +-----------
> +nvdimm-upcall is called by request-key from keyutils and not meant to be used
> +directly by the user. It expects to read from /etc/nvdimm.passwd to retrieve
> +the description and passphrase for the NVDIMM key.
> +
> +The nvdimm.passwd is formatted as:
> +<description id>:<passphrase with padded 0 to 32bytes>
> +cdab-0a-07e0-feffffff:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
> +
> +In order for this util to be called, /etc/request-key.conf must be appended
> +with the following line:
> +create logon nvdimm* * /usr/sbin/nvdimm-upcall %k
> +
> +include::../copyright.txt[]
> +
> +SEE ALSO
> +--------
> +http://pmem.io/documents/NVDIMM_DSM_Interface-V1.7.pdf
> diff --git a/Makefile.am b/Makefile.am
> index e0c463a3..73aa2645 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -42,6 +42,11 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
> dist_bashcompletion_DATA = contrib/ndctl
> endif
>
> +key_config_file = contrib/nvdimm.conf
> +key_configdir = $(sysconfdir)/request-key.d/
> +key_config_DATA = $(key_config_file)
> +EXTRA_DIST += $(key_config_file)
> +
> noinst_LIBRARIES = libccan.a
> libccan_a_SOURCES = \
> ccan/str/str.h \
> diff --git a/contrib/nvdimm.conf b/contrib/nvdimm.conf
> new file mode 100644
> index 00000000..e1f9a28b
> --- /dev/null
> +++ b/contrib/nvdimm.conf
> @@ -0,0 +1 @@
> +create logon nvdimm:* * /usr/sbin/nvdimm-upcall %k
> diff --git a/ndctl.spec.in b/ndctl.spec.in
> index f57b4fd5..ef591615 100644
> --- a/ndctl.spec.in
> +++ b/ndctl.spec.in
> @@ -120,6 +120,8 @@ make check
> %{bashcompdir}/
> %{_sysconfdir}/ndctl/monitor.conf
> %{_unitdir}/ndctl-monitor.service
> +%{_sbindir}/nvdimm-upcall
> +%{_sysconfdir}/request-key.d/nvdimm.conf
>
> %files -n daxctl
> %defattr(-,root,root)
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index 5b62251b..24ca98c7 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -1,6 +1,7 @@
> include $(top_srcdir)/Makefile.am.in
>
> bin_PROGRAMS = ndctl
> +sbin_PROGRAMS = nvdimm-upcall
>
> ndctl_SOURCES = ndctl.c \
> bus.c \
> @@ -52,3 +53,6 @@ EXTRA_DIST += $(monitor_config_file)
> if ENABLE_SYSTEMD_UNITS
> systemd_unit_DATA = ndctl-monitor.service
> endif
> +
> +nvdimm_upcall_SOURCES = nvdimm-upcall.c
> +nvdimm_upcall_LDADD = -lkeyutils
> diff --git a/ndctl/nvdimm-upcall.c b/ndctl/nvdimm-upcall.c
> new file mode 100644
> index 00000000..c8248359
> --- /dev/null
> +++ b/ndctl/nvdimm-upcall.c
> @@ -0,0 +1,154 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
> +
> +/*
> + * Used by /sbin/request-key for handling nvdimm upcall of key requests
> + * for security DSMs.
> + */
> +
> +
> +#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 <ctype.h>
> +
> +#define PASSPHRASE_SIZE 32
> +#define PASS_PATH "/etc/nvdimm.passwd"
> +
> +static FILE *fp;
> +
> +static int get_passphrase_from_id(const char *id, char *pass)
> +{
> + ssize_t rc = 0;
> + size_t size = 0, i;
> + char *line = NULL;
> + char *desc, *pass1;
> + int found = 0, comment = 0;
> +
> + fp = fopen(PASS_PATH, "r+");
> + if (!fp) {
> + syslog(LOG_ERR, "fopen: %s\n", strerror(errno));
> + return -errno;
> + }
> +
> + while ((rc = getline(&line, &size, fp)) != -1) {
> + /* skip comments */
> + for (i = 0; i < size; i++) {
> + if (isspace(line[i]))
> + continue;
> + if (line[i] == '#')
> + comment = 1;
> + }
> +
> + if (comment) {
> + comment = 0;
> + continue;
> + }
> +
> + desc = strtok(line, ":");
> + if (!desc)
> + break;
> + if (strcmp(desc, id) == 0) {
> + found = 1;
> + rc = 0;
> + break;
> + }
> + }
> +
> + if (rc == 0 && found) {
> + pass1 = strtok(NULL, ":");
> + if (!pass1) {
> + rc = -EINVAL;
> + goto out;
> + }
> + memset(pass, 0, PASSPHRASE_SIZE);
> + memcpy(pass, pass1, MIN(strlen(pass1), PASSPHRASE_SIZE));
> + } else
> + rc = -ENXIO;
> +
> +out:
> + free(line);
> + fclose(fp);
> + return rc;
> +}
> +
> +static char *get_key_desc(char *buf)
> +{
> + char *tmp = &buf[0];
> + int count = 0;
> +
> + while (*tmp != '\0') {
> + if (*tmp == ';')
> + count++;
> + if (count == 4) {
> + tmp++;
> + return tmp;
> + }
> + tmp++;
> + }
> +
> + return NULL;
> +}
> +
> +int main(int argc, const char **argv)
Could you maybe add a comment or link to documentation about what the
keyutils helper binary is supposed to do with its inputs?
> +{
> + key_serial_t key;
> + int rc;
> + char *buf, *desc, *dimm_id;
> + char pass[PASSPHRASE_SIZE];
> +
> + if (argc < 2) {
> + syslog(LOG_ERR, "Incorrect number of arguments\n");
> + return -EINVAL;
> + }
> +
> + syslog(LOG_DEBUG, "key passed in: %s\n", argv[1]);
> + key = strtol(argv[1], NULL, 10);
> + if (key < 0) {
> + syslog(LOG_ERR, "Invalid key format: %s\n", strerror(errno));
> + return -errno;
> + }
> +
> + rc = keyctl_describe_alloc(key, &buf);
> + if (rc < 0) {
> + syslog(LOG_ERR, "keyctl_describe_alloc failed: %s\n",
> + strerror(errno));
> + rc = -errno;
> + goto out;
> + }
> +
> + desc = get_key_desc(buf);
> + if (!desc) {
> + syslog(LOG_ERR, "Can't find key description\n");
> + rc = -EINVAL;
> + goto out;
> + }
> +
> + strtok_r(desc, ":", &dimm_id);
> + rc = get_passphrase_from_id(dimm_id, pass);
> + if (rc < 0) {
> + syslog(LOG_ERR, "failed to retrieve passphrase\n");
> + goto out;
> + }
> +
> + rc = keyctl_instantiate(key, pass, PASSPHRASE_SIZE, 0);
> + if (rc < 0) {
> + syslog(LOG_ERR, "keyctl_instantiate failed: %s\n",
> + strerror(errno));
> + rc = -errno;
> + goto out;
> + }
> +
> + out:
> + if (rc < 0)
> + keyctl_negate(key, 1, KEY_REQKEY_DEFL_DEFAULT);
> +
> + return rc;
> +}
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-10-16 3:06 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-12 22:28 [PATCH v4 0/7] ndctl: add security support Dave Jiang
2018-10-12 22:28 ` [PATCH v4 1/7] ndctl: add support for display security state Dave Jiang
2018-10-13 17:57 ` Dan Williams
2018-10-15 22:12 ` Dan Williams
2018-10-12 22:28 ` [PATCH v4 2/7] ndctl: add update to security support Dave Jiang
2018-10-16 0:59 ` Dan Williams
2018-10-12 22:28 ` [PATCH v4 3/7] ndctl: add disable " Dave Jiang
2018-10-12 22:28 ` [PATCH v4 4/7] ndctl: add support for freeze security Dave Jiang
2018-10-12 22:29 ` [PATCH v4 5/7] ndctl: add support for sanitize dimm Dave Jiang
2018-10-16 1:25 ` Dan Williams
2018-10-12 22:29 ` [PATCH v4 6/7] ndctl: add request-key upcall reference app Dave Jiang
2018-10-16 3:06 ` Dan Williams
2018-10-12 22:29 ` [PATCH v4 7/7] ndctl: add unit test for security ops (minus overwrite) Dave Jiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).