lkml.org 
[lkml]   [2019]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] security/keys/trusted: Allow operation without hardware TPM
    From
    Date
    On Tue, 2019-03-19 at 18:55 -0700, Dan Williams wrote:
    > On Mon, Mar 18, 2019 at 5:56 PM James Bottomley <jejb@linux.ibm.com>
    > wrote:
    > >
    > > On Mon, 2019-03-18 at 17:30 -0700, Dan Williams wrote:
    > > > On Mon, Mar 18, 2019 at 5:24 PM James Bottomley
    > > > <jejb@linux.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;
    >
    >
    > This falls over with the need to pin the module while any key that
    > needs service from the hosting key_type operations might be live in
    > the system.
    >
    > I could hang a "struct module *" off of the key_type so the host
    > module can be pinned, but that requires teaching all consumers of the
    > key_type module lifetime. Not impossible, but I think too big for a
    > fix, and I'd rather go with this local fixup to drop the dependency
    > on tpm_default_chip() successfully enumerating a TPM.

    Heh, well this proved to be a can of worms and no mistake.
    Unfortunately all of this does need fixing otherwise the keyctl syscall
    has exactly the same problem. But I think I agree it's getting way out
    of scope for the bug you found.

    James

    \
     
     \ /
      Last update: 2019-03-20 03:20    [W:5.215 / U:0.848 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site