linux-kernel - Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1552956989.2785.31.camel@linux.ibm.com>
Date:   Mon, 18 Mar 2019 17:56:29 -0700
From:   James Bottomley <jejb@...ux.ibm.com>
To:     Dan Williams <dan.j.williams@...el.com>
Cc:     Jarkko Sakkinen <jarkko.sakkinen@...ux.intel.com>,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        Roberto Sassu <roberto.sassu@...wei.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mimi Zohar <zohar@...ux.ibm.com>,
        David Howells <dhowells@...hat.com>, keyrings@...r.kernel.org
Subject: Re: [PATCH] security/keys/trusted: Allow operation without hardware
 TPM

On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote:
> On Mon, Mar 18, 2019 at 5:24 PM James Bottomley <jejb@...ux.ibm.com>
> wrote:
> > 
> > On Mon, 2019-03-18 at 16:45 -0700, Dan Williams wrote:
> > > Rather than fail initialization of the trusted.ko module, arrange
> > > for the module to load, but rely on trusted_instantiate() to fail
> > > trusted-key operations.
> > 
> > What actual problem is this fixing?  To me it would seem like an
> > enhancement to make the trusted module fail at load time if there's
> > no TPM rather than waiting until first use to find out it can never
> > work. Is there some piece of user code that depends on the
> > successful insertion of trusted.ko?
> 
> The module dependency chain relies on it. If that can be broken that
> would also be an acceptable fix.
> 
> I found this through the following dependency chain: libnvdimm.ko ->
> encrypted_keys.ko -> trusted.ko.
> 
> "key_type_trusted" is the symbol that encrypted_keys needs regardless
> of whether the tpm is present.

That's a nasty dependency caused by every key type module exporting a
symbol for its key type.  It really seems that key types should be
looked up by name not symbol to prevent more of these dependency issues
from spreading.  Something like this (untested and definitely won't
work without doing an EXPORT_SYMBOL on key_type_lookup).

If it does look acceptable we can also disentangle the nasty module
dependencies in the encrypted key code around masterkey which alone
should be a huge improvement because that code is too hacky to live.

James

---
diff --git a/security/keys/encrypted-keys/masterkey_trusted.c b/security/keys/encrypted-keys/masterkey_trusted.c
index dc3d18cae642..b98416f091e2 100644
--- a/security/keys/encrypted-keys/masterkey_trusted.c
+++ b/security/keys/encrypted-keys/masterkey_trusted.c
@@ -19,6 +19,7 @@
 #include <keys/trusted-type.h>
 #include <keys/encrypted-type.h>
 #include "encrypted.h"
+#include "../internal.h"
 
 /*
  * request_trusted_key - request the trusted key
@@ -32,8 +33,14 @@ struct key *request_trusted_key(const char *trusted_desc,
 {
 	struct trusted_key_payload *tpayload;
 	struct key *tkey;
+	struct key_type *type;
 
-	tkey = request_key(&key_type_trusted, trusted_desc, NULL);
+	type = key_type_lookup("trusted");
+	if (IS_ERR(type)) {
+		tkey = (struct key *)type;
+		goto error;
+	}
+	tkey = request_key(type, trusted_desc, NULL);
 	if (IS_ERR(tkey))
 		goto error;