Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754045AbaFBLkb (ORCPT ); Mon, 2 Jun 2014 07:40:31 -0400 Received: from mail-we0-f176.google.com ([74.125.82.176]:56782 "EHLO mail-we0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752784AbaFBLka (ORCPT ); Mon, 2 Jun 2014 07:40:30 -0400 MIME-Version: 1.0 In-Reply-To: <1401708833.15098.26.camel@dhcp-9-2-203-236.watson.ibm.com> References: <1401289784-31340-1-git-send-email-zohar@linux.vnet.ibm.com> <1401289784-31340-5-git-send-email-zohar@linux.vnet.ibm.com> <1401588848.22476.33.camel@dhcp-9-2-203-236.watson.ibm.com> <1401708833.15098.26.camel@dhcp-9-2-203-236.watson.ibm.com> Date: Mon, 2 Jun 2014 14:40:28 +0300 Message-ID: Subject: Re: [RFC PATCH v4 4/4] KEYS: define an owner trusted keyring From: Dmitry Kasatkin To: Mimi Zohar Cc: linux-security-module , Dmitry Kasatkin , David Howells , Josh Boyer , keyrings , linux-kernel Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2 June 2014 14:33, Mimi Zohar wrote: > On Mon, 2014-06-02 at 13:48 +0300, Dmitry Kasatkin wrote: >> On 1 June 2014 05:14, Mimi Zohar wrote: >> > On Sat, 2014-05-31 at 01:37 +0300, Dmitry Kasatkin wrote: >> >> On 28 May 2014 18:09, Mimi Zohar wrote: >> >> > (UEFI) secure boot provides a signature chain of trust rooted in >> >> > hardware. The signature chain of trust includes the Machine Owner >> >> > Keys(MOKs), which cannot be modified without physical presence. >> >> > >> >> > Instead of allowing public keys, with certificates signed by any >> >> > key on the system trusted keyring, to be added to a trusted >> >> > keyring, this patch further restricts the certificates to those >> >> > signed by the machine owner's key or chosen key. >> >> > >> >> > This patch defines an owner trusted keyring, defines a new boot >> >> > command line option 'keyring=' to designate the machine owner's >> >> > chosen key, and renames the function get_system_trusted_keyring() >> >> > to get_system_or_owner_trusted_keyring(). >> >> > >> >> > This patch permits the machine owner to safely identify their own >> >> > or chosen key, without requiring it to be builtin the kernel. >> >> > >> >> > Signed-off-by: Mimi Zohar >> >> > --- >> >> > crypto/asymmetric_keys/x509_public_key.c | 3 +- >> >> > include/keys/system_keyring.h | 15 ++++-- >> >> > include/linux/key.h | 4 ++ >> >> > kernel/system_keyring.c | 85 ++++++++++++++++++++++++++++++++ >> >> > security/keys/key.c | 20 ++++++++ >> >> > 5 files changed, 121 insertions(+), 6 deletions(-) >> >> > >> >> > diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c >> >> > index 1af8a30..6d52790 100644 >> >> > --- a/crypto/asymmetric_keys/x509_public_key.c >> >> > +++ b/crypto/asymmetric_keys/x509_public_key.c >> >> > @@ -237,7 +237,8 @@ static int x509_key_preparse(struct key_preparsed_payload *prep) >> >> > if (ret < 0) >> >> > goto error_free_cert; >> >> > } else { >> >> > - ret = x509_validate_trust(cert, get_system_trusted_keyring()); >> >> > + ret = x509_validate_trust(cert, >> >> > + get_system_or_owner_trusted_keyring()); >> >> > if (!ret) >> >> > prep->trusted = 1; >> >> > } >> >> > diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h >> >> > index 72665eb..f665c33 100644 >> >> > --- a/include/keys/system_keyring.h >> >> > +++ b/include/keys/system_keyring.h >> >> > @@ -17,15 +17,20 @@ >> >> > #include >> >> > >> >> > extern struct key *system_trusted_keyring; >> >> > -static inline struct key *get_system_trusted_keyring(void) >> >> > -{ >> >> > - return system_trusted_keyring; >> >> > -} >> >> > +extern struct key *owner_trusted_keyring; >> >> > + >> >> > +extern struct key *get_system_or_owner_trusted_keyring(void); >> >> > +extern void load_owner_identified_uefi_key(key_ref_t key); >> >> > + >> >> > #else >> >> > -static inline struct key *get_system_trusted_keyring(void) >> >> > +static inline struct key *get_system_or_owner_trusted_keyring(void) >> >> > { >> >> > return NULL; >> >> > } >> >> > + >> >> > +static void load_owner_identified_uefi_key(key_ref_t key) >> >> > +{ >> >> > +} >> >> > #endif >> >> > >> >> > #endif /* _KEYS_SYSTEM_KEYRING_H */ >> >> > diff --git a/include/linux/key.h b/include/linux/key.h >> >> > index cd0abb8..861843a 100644 >> >> > --- a/include/linux/key.h >> >> > +++ b/include/linux/key.h >> >> > @@ -267,6 +267,10 @@ extern int wait_for_key_construction(struct key *key, bool intr); >> >> > >> >> > extern int key_validate(const struct key *key); >> >> > >> >> > +extern int key_match(key_ref_t key, >> >> > + const char *type, >> >> > + const char *description); >> >> > + >> >> > extern key_ref_t key_create_or_update(key_ref_t keyring, >> >> > const char *type, >> >> > const char *description, >> >> > diff --git a/kernel/system_keyring.c b/kernel/system_keyring.c >> >> > index 52ebc70..e9b14ac 100644 >> >> > --- a/kernel/system_keyring.c >> >> > +++ b/kernel/system_keyring.c >> >> > @@ -19,11 +19,75 @@ >> >> > #include "module-internal.h" >> >> > >> >> > struct key *system_trusted_keyring; >> >> > +struct key *owner_trusted_keyring; >> >> > EXPORT_SYMBOL_GPL(system_trusted_keyring); >> >> > >> >> > extern __initconst const u8 system_certificate_list[]; >> >> > extern __initconst const unsigned long system_certificate_list_size; >> >> > >> >> > +static int use_owner_trusted_keyring; >> >> > + >> >> > +static char *owner_keyid; >> >> > +static int builtin_keyring; >> >> > +static int __init default_keyring_set(char *str) >> >> > +{ >> >> > + if (!str) /* default: builtin */ >> >> > + return 1; >> >> > + >> >> > + if (strcmp(str, "system") == 0) /* use system keyring */ >> >> > + ; >> >> > + else if (strcmp(str, "builtin") == 0) /* only builtin keys */ >> >> > + builtin_keyring = 1; >> >> > + else >> >> > + owner_keyid = str; /* owner local key 'id:xxxxxx' */ >> >> > + return 1; >> >> > +} >> >> > +__setup("keyring=", default_keyring_set); >> >> > + >> >> > +/* >> >> > + * Load the owner identified key on the 'owner' trusted keyring. >> >> > + */ >> >> > +void load_owner_identified_uefi_key(key_ref_t key) >> >> > +{ >> >> > + if (!owner_keyid || use_owner_trusted_keyring) >> >> > + return; >> >> > + >> >> > + if (!key_match(key, "asymmetric", owner_keyid)) >> >> > + return; >> >> > + >> >> > + if (key_link(owner_trusted_keyring, key_ref_to_ptr(key)) == 0) { >> >> > + set_bit(KEY_FLAG_TRUSTED_ONLY, &owner_trusted_keyring->flags); >> >> > + use_owner_trusted_keyring = 1; >> >> >> >> This is a bit strange... >> >> Linking any key forces to use owner trusted keyring... >> > >> > Wouldn't it be stranger to identify a specific key on the system keyring >> > and add it to the owner keyring, but then not use it? I don't >> > understand your concern. What is the problem? >> >> This patch technically specifies to use all keys from the system >> keyring or the one specified >> by the owner_keyid. Why the owner keyring is needed then at all? >> owner_keyid can identify the key to use from the system keyring.... > > Currently only the builtin keys are on the system keyring, but once > David and Josh's UEFI patches are upstreamed, the UEFI keys will also be > on the system keyring. At that point, we would want to differentiate > between the builtin keys and the remaining keys on the system keyring. > Splitting this patch definitely helps clarify what's happening and, more > importantly, why. > > Mimi > Ok. May be would should focus on patches for existing functionality. If something new comes from other side, it can be addressed by new another set of patches. - Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/