| Message ID | 154705647045.23227.7733373111052687524.stgit@djiang5-desk3.ch.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ndctl: add security support | expand |
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 ? >
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.
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
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