[v7,07/12] ndctl: setup modprobe rules
diff mbox series

Message ID 154705647045.23227.7733373111052687524.stgit@djiang5-desk3.ch.intel.com
State Superseded
Headers show
Series
  • ndctl: add security support
Related show

Commit Message

Dave Jiang Jan. 9, 2019, 5:54 p.m. UTC
Adding reference config file for modprobe.d in order to trigger the
reference script that will inject keys associated with the nvdimms into
the kernel user ring for unlock.

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

Comments

Verma, Vishal L Jan. 9, 2019, 10:08 p.m. UTC | #1
On Wed, 2019-01-09 at 10:54 -0700, Dave Jiang wrote:
> Adding reference config file for modprobe.d in order to trigger the
> reference script that will inject keys associated with the nvdimms
> into
> the kernel user ring for unlock.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Makefile.am                  |   10 ++++++++++
>  contrib/ndctl-loadkeys.sh    |   25 +++++++++++++++++++++++++
>  contrib/nvdimm_modprobe.conf |    1 +
>  3 files changed, 36 insertions(+)
>  create mode 100755 contrib/ndctl-loadkeys.sh
>  create mode 100644 contrib/nvdimm_modprobe.conf
> 
> diff --git a/Makefile.am b/Makefile.am
> index e0c463a3..5a3f03aa 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -42,6 +42,16 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
>  dist_bashcompletion_DATA = contrib/ndctl
>  endif
>  
> +load_key_file = contrib/ndctl-loadkeys.sh
> +load_keydir = $(sysconfdir)/ndctl/
> +load_key_DATA = $(load_key_file)
> +EXTRA_DIST += $(load_key_file)
> +
> +modprobe_file = contrib/nvdimm_modprobe.conf
> +modprobedir = $(sysconfdir)/modprobe.d/
> +modprobe_DATA = $(modprobe_file)
> +EXTRA_DIST += $(modprobe_file)
> +

We're installing these files via the Makefile, but I think the spec is
missing them? Presumably the spec should also install them in the same
way?

>  noinst_LIBRARIES = libccan.a
>  libccan_a_SOURCES = \
>  	ccan/str/str.h \
> diff --git a/contrib/ndctl-loadkeys.sh b/contrib/ndctl-loadkeys.sh
> new file mode 100755
> index 00000000..bc2c94df
> --- /dev/null
> +++ b/contrib/ndctl-loadkeys.sh
> @@ -0,0 +1,25 @@
> +#!/bin/bash -Ex

I didn't catch this before, but this script doesn't need -E since we're
not setting up a trap.
If anything use -e, the regular version.
I'm also not sure -x is needed - it is just an example script right? I 
don't feel strongly about it either way, if having the extra debug here
might be helpful we can keep it.

> +
> +# This script assumes a single master key for all DIMMs
> +
> +key_path=/etc/ndctl/keys
> +tpmh_path="$key_path"/tpm.handle
> +key_type=""
> +tpm_handle=""
> +id=""
> +
> +if [ -f $tpmh_path ]; then
> +	key_type=trusted
> +	tpm_handle="keyhandle=$(cat $tpmh_path)"
> +else
> +	key_type=user
> +fi
> +
> +if ! keyctl search @u "$key_type" nvdimm-master; then
> +	keyctl add "$key_type" nvdimm-master "load $(cat
> $key_path/nvdimm-master.blob) $tpm_handle" @u > /dev/null
> +fi
> +
> +for file in "$key_path"/nvdimm_*; do
> +	id="$(cut -d'_' -f2 <<< "${file##*/}")"
> +	keyctl add encrypted nvdimm:"$id" "load $(cat "$file")" @u
> +done
> diff --git a/contrib/nvdimm_modprobe.conf
> b/contrib/nvdimm_modprobe.conf
> new file mode 100644
> index 00000000..b113d8d7
> --- /dev/null
> +++ b/contrib/nvdimm_modprobe.conf
> @@ -0,0 +1 @@
> +install libnvdimm /usr/sbin/ndctl-loadkeys.sh ; /sbin/modprobe --
> ignore-install libnvdimm $CMDLINE_OPTS

I'm not familiar with how modprobe.conf works, but is it looking for
ndctl-loadkeys.sh in /usr/sbin? If so are we installing it there? The
lines above in the Makefile seem to have it going into /etc/ndctl ?

>
Dan Williams Jan. 9, 2019, 10:59 p.m. UTC | #2
On Wed, Jan 9, 2019 at 9:55 AM Dave Jiang <dave.jiang@intel.com> wrote:
>
> Adding reference config file for modprobe.d in order to trigger the
> reference script that will inject keys associated with the nvdimms into
> the kernel user ring for unlock.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Makefile.am                  |   10 ++++++++++
>  contrib/ndctl-loadkeys.sh    |   25 +++++++++++++++++++++++++
>  contrib/nvdimm_modprobe.conf |    1 +
>  3 files changed, 36 insertions(+)
>  create mode 100755 contrib/ndctl-loadkeys.sh
>  create mode 100644 contrib/nvdimm_modprobe.conf

This file is installed to /etc/modprobe.d, so no need to duplicate
"modprobe" in the name. I'd prefer "nvidimm-security.conf" to make it
explicit.

>
> diff --git a/Makefile.am b/Makefile.am
> index e0c463a3..5a3f03aa 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -42,6 +42,16 @@ bashcompletiondir = $(BASH_COMPLETION_DIR)
>  dist_bashcompletion_DATA = contrib/ndctl
>  endif
>
> +load_key_file = contrib/ndctl-loadkeys.sh
> +load_keydir = $(sysconfdir)/ndctl/
> +load_key_DATA = $(load_key_file)
> +EXTRA_DIST += $(load_key_file)

No need for EXTRA_DIST,  _DATA will do this.

> +
> +modprobe_file = contrib/nvdimm_modprobe.conf
> +modprobedir = $(sysconfdir)/modprobe.d/
> +modprobe_DATA = $(modprobe_file)
> +EXTRA_DIST += $(modprobe_file)

ditto.

> +
>  noinst_LIBRARIES = libccan.a
>  libccan_a_SOURCES = \
>         ccan/str/str.h \
> diff --git a/contrib/ndctl-loadkeys.sh b/contrib/ndctl-loadkeys.sh
> new file mode 100755
> index 00000000..bc2c94df
> --- /dev/null
> +++ b/contrib/ndctl-loadkeys.sh
> @@ -0,0 +1,25 @@
> +#!/bin/bash -Ex
> +
> +# This script assumes a single master key for all DIMMs
> +
> +key_path=/etc/ndctl/keys

Hard coded path, this should be constructed from the variables from
the Makefile.

Where is this shell script installed? I think this should become an
actual ndctl command rather than an on-the side shell script. It
otherwise seems odd to name it under "contrib" since this seems
generic enough to be the "official" solution. I think contrib should
be reserved for things that are not fundamental to the operation of
the utility. This seems integral to the security implementation.

The git command harness had support for optionally calling built-in C
routines or shell scripts, would just need to resurrect the support to
route "ndctl load-keys" to this script, likely installed to
/usr/libexec/ndctl.

Patch
diff mbox series

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