[PATCH 0/6] security/keys/encrypted: Break module dependency chain

[PATCH 0/6] security/keys/encrypted: Break module dependency chain

[Dan Williams]

With v5.1-rc1 all the nvdimm sub-system regression tests started failing
because the libnvdimm module failed to load in the qemu-kvm test
environment.  Critically that environment does not have a TPM. Commit
240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
started to require a TPM to be present for the trusted.ko module to load
where there was no requirement for that before.

Rather than undo the "fail if no hardware" behavior James points out
that the module dependencies can be broken by looking up the key-type by
name. Remove the dependencies on the "key_type_trusted" and
"key_type_encrypted" symbol exports, and clean up other boilerplate that
supported those exports in different configurations.

---

Dan Williams (6):
      security/keys/encrypted: Allow operation without trusted.ko
      security/keys/encrypted: Clean up request_trusted_key()
      libnvdimm/security: Drop direct dependency on key_type_encrypted
      security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
      security/integrity/evm: Drop direct dependency on key_type_encrypted
      security/keys/encrypted: Drop export of key_type_encrypted


 drivers/nvdimm/security.c                        |   11 ++++-
 fs/ecryptfs/ecryptfs_kernel.h                    |   22 -----------
 fs/ecryptfs/keystore.c                           |   12 ++++++
 include/keys/encrypted-type.h                    |    2 -
 include/linux/key.h                              |    1 
 security/integrity/evm/evm_crypto.c              |    9 ++++
 security/keys/encrypted-keys/Makefile            |    3 -
 security/keys/encrypted-keys/encrypted.c         |   35 ++++++++++++++++-
 security/keys/encrypted-keys/encrypted.h         |   12 ------
 security/keys/encrypted-keys/masterkey_trusted.c |   46 ----------------------
 security/keys/internal.h                         |    2 -
 security/keys/key.c                              |    1 
 12 files changed, 65 insertions(+), 91 deletions(-)
 delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c

[PATCH 5/6] security/integrity/evm: Drop direct dependency on key_type_encrypted

[Dan Williams]

Lookup the key type by name and protect evm from encrypted_keys.ko
module load failures.

Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: <linux-integrity@vger.kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 security/integrity/evm/evm_crypto.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index c37d08118af5..5c65c3aef427 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -354,10 +354,15 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
 int evm_init_key(void)
 {
 	struct key *evm_key;
+	struct key_type *type;
 	struct encrypted_key_payload *ekp;
 	int rc;
 
-	evm_key = request_key(&key_type_encrypted, EVMKEY, NULL);
+	type = key_type_lookup("encrypted");
+	if (IS_ERR(type))
+		return PTR_ERR(type);
+
+	evm_key = request_key(type, EVMKEY, NULL);
 	if (IS_ERR(evm_key))
 		return -ENOENT;
 
@@ -372,3 +377,5 @@ int evm_init_key(void)
 	key_put(evm_key);
 	return rc;
 }
+
+MODULE_SOFTDEP("pre: encrypted_keys");

Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain

[Dan Williams]

On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> because the libnvdimm module failed to load in the qemu-kvm test
> environment.  Critically that environment does not have a TPM. Commit
> 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> started to require a TPM to be present for the trusted.ko module to load
> where there was no requirement for that before.
>
> Rather than undo the "fail if no hardware" behavior James points out
> that the module dependencies can be broken by looking up the key-type by
> name. Remove the dependencies on the "key_type_trusted" and
> "key_type_encrypted" symbol exports, and clean up other boilerplate that
> supported those exports in different configurations.

Any feedback? Was hoping to get at least patch1 in the queue for
v5.1-rc2 since this effectively disables the nvdimm driver on typical
configurations. Jarkko, would you be willing to merge it since the
regression came through your tree?

> Dan Williams (6):
>       security/keys/encrypted: Allow operation without trusted.ko
>       security/keys/encrypted: Clean up request_trusted_key()
>       libnvdimm/security: Drop direct dependency on key_type_encrypted
>       security/keys/ecryptfs: Drop direct dependency on key_type_encrypted
>       security/integrity/evm: Drop direct dependency on key_type_encrypted
>       security/keys/encrypted: Drop export of key_type_encrypted
>
>
>  drivers/nvdimm/security.c                        |   11 ++++-
>  fs/ecryptfs/ecryptfs_kernel.h                    |   22 -----------
>  fs/ecryptfs/keystore.c                           |   12 ++++++
>  include/keys/encrypted-type.h                    |    2 -
>  include/linux/key.h                              |    1
>  security/integrity/evm/evm_crypto.c              |    9 ++++
>  security/keys/encrypted-keys/Makefile            |    3 -
>  security/keys/encrypted-keys/encrypted.c         |   35 ++++++++++++++++-
>  security/keys/encrypted-keys/encrypted.h         |   12 ------
>  security/keys/encrypted-keys/masterkey_trusted.c |   46 ----------------------
>  security/keys/internal.h                         |    2 -
>  security/keys/key.c                              |    1
>  12 files changed, 65 insertions(+), 91 deletions(-)
>  delete mode 100644 security/keys/encrypted-keys/masterkey_trusted.c

Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain

[James Bottomley]

On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> om> wrote:
> > 
> > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > failing because the libnvdimm module failed to load in the qemu-kvm 
> > test environment.  Critically that environment does not have a TPM.
> > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > structure..." started to require a TPM to be present for the
> > trusted.ko module to load where there was no requirement for that
> > before.
> > 
> > Rather than undo the "fail if no hardware" behavior James points
> > out that the module dependencies can be broken by looking up the
> > key-type by name. Remove the dependencies on the "key_type_trusted"
> > and "key_type_encrypted" symbol exports, and clean up other
> > boilerplate that supported those exports in different
> > configurations.
> 
> Any feedback? Was hoping to get at least patch1 in the queue for
> v5.1-rc2 since this effectively disables the nvdimm driver on typical
> configurations. Jarkko, would you be willing to merge it since the
> regression came through your tree?

The reason I sent out the RFC was to see if the people who actually
wrote the code had any reasons they needed the current way of doing
things preserving.

I think your series looks fine except you need to export
key_type_lookup (patch 2) before you use it (patch 1) to preserve
bisectability of builds.

James

Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain

[Dan Williams]

On Tue, Mar 19, 2019 at 2:09 PM James Bottomley <jejb@linux.ibm.com> wrote:
>
> On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> > om> wrote:
> > >
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > > failing because the libnvdimm module failed to load in the qemu-kvm
> > > test environment.  Critically that environment does not have a TPM.
> > > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > > structure..." started to require a TPM to be present for the
> > > trusted.ko module to load where there was no requirement for that
> > > before.
> > >
> > > Rather than undo the "fail if no hardware" behavior James points
> > > out that the module dependencies can be broken by looking up the
> > > key-type by name. Remove the dependencies on the "key_type_trusted"
> > > and "key_type_encrypted" symbol exports, and clean up other
> > > boilerplate that supported those exports in different
> > > configurations.
> >
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
>
> The reason I sent out the RFC was to see if the people who actually
> wrote the code had any reasons they needed the current way of doing
> things preserving.
>
> I think your series looks fine except you need to export
> key_type_lookup (patch 2) before you use it (patch 1) to preserve
> bisectability of builds.

Good catch, I fumbled that around. I'll respin and let 0day chew on it
before sending out another version.

Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain

[Mimi Zohar]

On Tue, 2019-03-19 at 14:08 -0700, James Bottomley wrote:
> On Tue, 2019-03-19 at 14:01 -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.c
> > om> wrote:
> > > 
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started
> > > failing because the libnvdimm module failed to load in the qemu-kvm 
> > > test environment.  Critically that environment does not have a TPM.
> > > Commit 240730437deb "KEYS: trusted: explicitly use tpm_chip
> > > structure..." started to require a TPM to be present for the
> > > trusted.ko module to load where there was no requirement for that
> > > before.
> > > 
> > > Rather than undo the "fail if no hardware" behavior James points
> > > out that the module dependencies can be broken by looking up the
> > > key-type by name. Remove the dependencies on the "key_type_trusted"
> > > and "key_type_encrypted" symbol exports, and clean up other
> > > boilerplate that supported those exports in different
> > > configurations.
> > 
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
> 
> The reason I sent out the RFC was to see if the people who actually
> wrote the code had any reasons they needed the current way of doing
> things preserving.

No, it looks fine.  The error return codes will change, but I don't
think that is a problem.

Mimi

> 
> I think your series looks fine except you need to export
> key_type_lookup (patch 2) before you use it (patch 1) to preserve
> bisectability of builds.
> 
> James

Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain

[Jarkko Sakkinen]

On Tue, Mar 19, 2019 at 02:01:44PM -0700, Dan Williams wrote:
> On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
> >
> > With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> > because the libnvdimm module failed to load in the qemu-kvm test
> > environment.  Critically that environment does not have a TPM. Commit
> > 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> > started to require a TPM to be present for the trusted.ko module to load
> > where there was no requirement for that before.
> >
> > Rather than undo the "fail if no hardware" behavior James points out
> > that the module dependencies can be broken by looking up the key-type by
> > name. Remove the dependencies on the "key_type_trusted" and
> > "key_type_encrypted" symbol exports, and clean up other boilerplate that
> > supported those exports in different configurations.
> 
> Any feedback? Was hoping to get at least patch1 in the queue for
> v5.1-rc2 since this effectively disables the nvdimm driver on typical
> configurations. Jarkko, would you be willing to merge it since the
> regression came through your tree?

Yes, of course. The feedback has been extremely passive because I've
been sick leave for the early week :-)

Before I'm merging this I'm just thinking that would it be better
idea to merge a patch for trusted.c that reverts the old behavior
with cc to stable and fixes tags as I said in my earlier response.

It would less intrusive for stable kernels. Lets quickly sort out
the best strategy before merging.

/Jarkko

Re: [PATCH 0/6] security/keys/encrypted: Break module dependency chain

[Jarkko Sakkinen]

On Thu, Mar 21, 2019 at 03:45:49PM +0200, Jarkko Sakkinen wrote:
> On Tue, Mar 19, 2019 at 02:01:44PM -0700, Dan Williams wrote:
> > On Mon, Mar 18, 2019 at 11:18 PM Dan Williams <dan.j.williams@intel.com> wrote:
> > >
> > > With v5.1-rc1 all the nvdimm sub-system regression tests started failing
> > > because the libnvdimm module failed to load in the qemu-kvm test
> > > environment.  Critically that environment does not have a TPM. Commit
> > > 240730437deb "KEYS: trusted: explicitly use tpm_chip structure..."
> > > started to require a TPM to be present for the trusted.ko module to load
> > > where there was no requirement for that before.
> > >
> > > Rather than undo the "fail if no hardware" behavior James points out
> > > that the module dependencies can be broken by looking up the key-type by
> > > name. Remove the dependencies on the "key_type_trusted" and
> > > "key_type_encrypted" symbol exports, and clean up other boilerplate that
> > > supported those exports in different configurations.
> > 
> > Any feedback? Was hoping to get at least patch1 in the queue for
> > v5.1-rc2 since this effectively disables the nvdimm driver on typical
> > configurations. Jarkko, would you be willing to merge it since the
> > regression came through your tree?
> 
> Yes, of course. The feedback has been extremely passive because I've
> been sick leave for the early week :-)
> 
> Before I'm merging this I'm just thinking that would it be better
> idea to merge a patch for trusted.c that reverts the old behavior
> with cc to stable and fixes tags as I said in my earlier response.
> 
> It would less intrusive for stable kernels. Lets quickly sort out
> the best strategy before merging.

I.e. the way I see the situation:

1. Reverting the old behavior in the sense that missing TPM does
   not prevent init of trusted.ko should be done right now.
2. Your patch could be definitely merged but not as a bug fix.
3. At some point we could consider failing the init of trusted.ko
   if TPM is missing because that is kind of senseful anyway with
   better testing now that we understand the dependency context
   better.