2021-02-10 12:08:33

by Mickaël Salaün

[permalink] [raw]
Subject: [PATCH v6 5/5] certs: Allow root user to append signed hashes to the blacklist keyring

From: Mickaël Salaün <[email protected]>

Add a kernel option SYSTEM_BLACKLIST_AUTH_UPDATE to enable the root user
to dynamically add new keys to the blacklist keyring. This enables to
invalidate new certificates, either from being loaded in a keyring, or
from being trusted in a PKCS#7 certificate chain. This also enables to
add new file hashes to be denied by the integrity infrastructure.

Being able to untrust a certificate which could have normaly been
trusted is a sensitive operation. This is why adding new hashes to the
blacklist keyring is only allowed when these hashes are signed and
vouched by the builtin trusted keyring. A blacklist hash is stored as a
key description. The PKCS#7 signature of this description must be
provided as the key payload.

Marking a certificate as untrusted should be enforced while the system
is running. It is then forbiden to remove such blacklist keys.

Update blacklist keyring, blacklist key and revoked certificate access rights:
* allows the root user to search for a specific blacklisted hash, which
make sense because the descriptions are already viewable;
* forbids key update (blacklist and asymmetric ones);
* restricts kernel rights on the blacklist keyring to align with the
root user rights.

See help in tools/certs/print-cert-tbs-hash.sh .

Cc: David Howells <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Eric Snowberg <[email protected]>
Cc: Jarkko Sakkinen <[email protected]>
Signed-off-by: Mickaël Salaün <[email protected]>
---

Changes since v5:
* Rebase on keys-next, fix Kconfig conflict, and update the asymmetric
key rights added to the blacklist keyring by the new
add_key_to_revocation_list(): align with blacklist key rights by
removing KEY_POS_WRITE as a safeguard, and add
KEY_ALLOC_BYPASS_RESTRICTION to not be subject to
restrict_link_for_blacklist() that only allows blacklist key types to
be added to the keyring.
* Change the return code for restrict_link_for_blacklist() from -EPERM
to -EOPNOTSUPP to align with asymmetric key keyrings.

Changes since v3:
* Update commit message for print-cert-tbs-hash.sh .

Changes since v2:
* Add comment for blacklist_key_instantiate().
---
certs/Kconfig | 10 +++++
certs/blacklist.c | 96 ++++++++++++++++++++++++++++++++++++-----------
2 files changed, 85 insertions(+), 21 deletions(-)

diff --git a/certs/Kconfig b/certs/Kconfig
index 139940b54207..555491fdde0e 100644
--- a/certs/Kconfig
+++ b/certs/Kconfig
@@ -94,4 +94,14 @@ config SYSTEM_REVOCATION_KEYS
containing X.509 certificates to be included in the default blacklist
keyring.

+config SYSTEM_BLACKLIST_AUTH_UPDATE
+ bool "Allow root to add signed blacklist keys"
+ depends on SYSTEM_BLACKLIST_KEYRING
+ depends on SYSTEM_DATA_VERIFICATION
+ help
+ If set, provide the ability to load new blacklist keys at run time if
+ they are signed and vouched by a certificate from the builtin trusted
+ keyring. The PKCS#7 signature of the description is set in the key
+ payload. Blacklist keys cannot be removed.
+
endmenu
diff --git a/certs/blacklist.c b/certs/blacklist.c
index 9fe79597bda7..731e8e5bb6d8 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -15,6 +15,7 @@
#include <linux/err.h>
#include <linux/seq_file.h>
#include <linux/uidgid.h>
+#include <linux/verification.h>
#include <keys/system_keyring.h>
#include "blacklist.h"
#include "common.h"
@@ -26,6 +27,9 @@
*/
#define MAX_HASH_LEN 128

+#define BLACKLIST_KEY_PERM (KEY_POS_SEARCH | KEY_POS_VIEW | \
+ KEY_USR_SEARCH | KEY_USR_VIEW)
+
static const char tbs_prefix[] = "tbs";
static const char bin_prefix[] = "bin";

@@ -78,19 +82,51 @@ static int blacklist_vet_description(const char *desc)
return 0;
}

-/*
- * The hash to be blacklisted is expected to be in the description. There will
- * be no payload.
- */
-static int blacklist_preparse(struct key_preparsed_payload *prep)
+static int blacklist_key_instantiate(struct key *key,
+ struct key_preparsed_payload *prep)
{
- if (prep->datalen > 0)
- return -EINVAL;
- return 0;
+#ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
+ int err;
+#endif
+
+ /* Sets safe default permissions for keys loaded by user space. */
+ key->perm = BLACKLIST_KEY_PERM;
+
+ /*
+ * Skips the authentication step for builtin hashes, they are not
+ * signed but still trusted.
+ */
+ if (key->flags & (1 << KEY_FLAG_BUILTIN))
+ goto out;
+
+#ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
+ /*
+ * Verifies the description's PKCS#7 signature against the builtin
+ * trusted keyring.
+ */
+ err = verify_pkcs7_signature(key->description,
+ strlen(key->description), prep->data, prep->datalen,
+ NULL, VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
+ if (err)
+ return err;
+#else
+ /*
+ * It should not be possible to come here because the keyring doesn't
+ * have KEY_USR_WRITE and the only other way to call this function is
+ * for builtin hashes.
+ */
+ WARN_ON_ONCE(1);
+ return -EPERM;
+#endif
+
+out:
+ return generic_key_instantiate(key, prep);
}

-static void blacklist_free_preparse(struct key_preparsed_payload *prep)
+static int blacklist_key_update(struct key *key,
+ struct key_preparsed_payload *prep)
{
+ return -EPERM;
}

static void blacklist_describe(const struct key *key, struct seq_file *m)
@@ -101,9 +137,8 @@ static void blacklist_describe(const struct key *key, struct seq_file *m)
static struct key_type key_type_blacklist = {
.name = "blacklist",
.vet_description = blacklist_vet_description,
- .preparse = blacklist_preparse,
- .free_preparse = blacklist_free_preparse,
- .instantiate = generic_key_instantiate,
+ .instantiate = blacklist_key_instantiate,
+ .update = blacklist_key_update,
.describe = blacklist_describe,
};

@@ -152,8 +187,7 @@ static int mark_raw_hash_blacklisted(const char *hash)
hash,
NULL,
0,
- ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
- KEY_USR_VIEW),
+ BLACKLIST_KEY_PERM,
KEY_ALLOC_NOT_IN_QUOTA |
KEY_ALLOC_BUILT_IN);
if (IS_ERR(key)) {
@@ -172,8 +206,10 @@ int add_key_to_revocation_list(const char *data, size_t size)
NULL,
data,
size,
- ((KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW),
- KEY_ALLOC_NOT_IN_QUOTA | KEY_ALLOC_BUILT_IN);
+ KEY_POS_VIEW | KEY_POS_READ | KEY_POS_SEARCH
+ | KEY_USR_VIEW,
+ KEY_ALLOC_NOT_IN_QUOTA | KEY_ALLOC_BUILT_IN
+ | KEY_ALLOC_BYPASS_RESTRICTION);

if (IS_ERR(key)) {
pr_err("Problem with revocation key (%ld)\n", PTR_ERR(key));
@@ -247,25 +283,43 @@ int is_binary_blacklisted(const u8 *hash, size_t hash_len)
}
EXPORT_SYMBOL_GPL(is_binary_blacklisted);

+static int restrict_link_for_blacklist(struct key *dest_keyring,
+ const struct key_type *type, const union key_payload *payload,
+ struct key *restrict_key)
+{
+ if (type == &key_type_blacklist)
+ return 0;
+ return -EOPNOTSUPP;
+}
+
/*
* Initialise the blacklist
*/
static int __init blacklist_init(void)
{
const char *const *bl;
+ struct key_restriction *restriction;

if (register_key_type(&key_type_blacklist) < 0)
panic("Can't allocate system blacklist key type\n");

+ restriction = kzalloc(sizeof(*restriction), GFP_KERNEL);
+ if (!restriction)
+ panic("Can't allocate blacklist keyring restriction\n");
+ restriction->check = restrict_link_for_blacklist;
+
blacklist_keyring =
keyring_alloc(".blacklist",
GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
- (KEY_POS_ALL & ~KEY_POS_SETATTR) |
- KEY_USR_VIEW | KEY_USR_READ |
- KEY_USR_SEARCH,
- KEY_ALLOC_NOT_IN_QUOTA |
+ KEY_POS_VIEW | KEY_POS_READ | KEY_POS_SEARCH |
+ KEY_POS_WRITE |
+ KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH
+#ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
+ | KEY_USR_WRITE
+#endif
+ , KEY_ALLOC_NOT_IN_QUOTA |
KEY_ALLOC_SET_KEEP,
- NULL, NULL);
+ restriction, NULL);
if (IS_ERR(blacklist_keyring))
panic("Can't allocate system blacklist keyring\n");

--
2.30.0


2021-02-12 11:49:32

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v6 5/5] certs: Allow root user to append signed hashes to the blacklist keyring

On Wed, Feb 10, 2021 at 01:04:10PM +0100, Micka?l Sala?n wrote:
> From: Micka?l Sala?n <[email protected]>
>
> Add a kernel option SYSTEM_BLACKLIST_AUTH_UPDATE to enable the root user
> to dynamically add new keys to the blacklist keyring. This enables to
> invalidate new certificates, either from being loaded in a keyring, or
> from being trusted in a PKCS#7 certificate chain. This also enables to
> add new file hashes to be denied by the integrity infrastructure.
>
> Being able to untrust a certificate which could have normaly been
> trusted is a sensitive operation. This is why adding new hashes to the
> blacklist keyring is only allowed when these hashes are signed and
> vouched by the builtin trusted keyring. A blacklist hash is stored as a
> key description. The PKCS#7 signature of this description must be
> provided as the key payload.
>
> Marking a certificate as untrusted should be enforced while the system
> is running. It is then forbiden to remove such blacklist keys.
>
> Update blacklist keyring, blacklist key and revoked certificate access rights:
> * allows the root user to search for a specific blacklisted hash, which
> make sense because the descriptions are already viewable;
> * forbids key update (blacklist and asymmetric ones);
> * restricts kernel rights on the blacklist keyring to align with the
> root user rights.
>
> See help in tools/certs/print-cert-tbs-hash.sh .
>
> Cc: David Howells <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Cc: Eric Snowberg <[email protected]>
> Cc: Jarkko Sakkinen <[email protected]>
> Signed-off-by: Micka?l Sala?n <[email protected]>

Tested-by: Jarkko Sakkinen <[email protected]>

/Jarkko

> ---
>
> Changes since v5:
> * Rebase on keys-next, fix Kconfig conflict, and update the asymmetric
> key rights added to the blacklist keyring by the new
> add_key_to_revocation_list(): align with blacklist key rights by
> removing KEY_POS_WRITE as a safeguard, and add
> KEY_ALLOC_BYPASS_RESTRICTION to not be subject to
> restrict_link_for_blacklist() that only allows blacklist key types to
> be added to the keyring.
> * Change the return code for restrict_link_for_blacklist() from -EPERM
> to -EOPNOTSUPP to align with asymmetric key keyrings.
>
> Changes since v3:
> * Update commit message for print-cert-tbs-hash.sh .
>
> Changes since v2:
> * Add comment for blacklist_key_instantiate().
> ---
> certs/Kconfig | 10 +++++
> certs/blacklist.c | 96 ++++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 85 insertions(+), 21 deletions(-)
>
> diff --git a/certs/Kconfig b/certs/Kconfig
> index 139940b54207..555491fdde0e 100644
> --- a/certs/Kconfig
> +++ b/certs/Kconfig
> @@ -94,4 +94,14 @@ config SYSTEM_REVOCATION_KEYS
> containing X.509 certificates to be included in the default blacklist
> keyring.
>
> +config SYSTEM_BLACKLIST_AUTH_UPDATE
> + bool "Allow root to add signed blacklist keys"
> + depends on SYSTEM_BLACKLIST_KEYRING
> + depends on SYSTEM_DATA_VERIFICATION
> + help
> + If set, provide the ability to load new blacklist keys at run time if
> + they are signed and vouched by a certificate from the builtin trusted
> + keyring. The PKCS#7 signature of the description is set in the key
> + payload. Blacklist keys cannot be removed.
> +
> endmenu
> diff --git a/certs/blacklist.c b/certs/blacklist.c
> index 9fe79597bda7..731e8e5bb6d8 100644
> --- a/certs/blacklist.c
> +++ b/certs/blacklist.c
> @@ -15,6 +15,7 @@
> #include <linux/err.h>
> #include <linux/seq_file.h>
> #include <linux/uidgid.h>
> +#include <linux/verification.h>
> #include <keys/system_keyring.h>
> #include "blacklist.h"
> #include "common.h"
> @@ -26,6 +27,9 @@
> */
> #define MAX_HASH_LEN 128
>
> +#define BLACKLIST_KEY_PERM (KEY_POS_SEARCH | KEY_POS_VIEW | \
> + KEY_USR_SEARCH | KEY_USR_VIEW)
> +
> static const char tbs_prefix[] = "tbs";
> static const char bin_prefix[] = "bin";
>
> @@ -78,19 +82,51 @@ static int blacklist_vet_description(const char *desc)
> return 0;
> }
>
> -/*
> - * The hash to be blacklisted is expected to be in the description. There will
> - * be no payload.
> - */
> -static int blacklist_preparse(struct key_preparsed_payload *prep)
> +static int blacklist_key_instantiate(struct key *key,
> + struct key_preparsed_payload *prep)
> {
> - if (prep->datalen > 0)
> - return -EINVAL;
> - return 0;
> +#ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
> + int err;
> +#endif
> +
> + /* Sets safe default permissions for keys loaded by user space. */
> + key->perm = BLACKLIST_KEY_PERM;
> +
> + /*
> + * Skips the authentication step for builtin hashes, they are not
> + * signed but still trusted.
> + */
> + if (key->flags & (1 << KEY_FLAG_BUILTIN))
> + goto out;
> +
> +#ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
> + /*
> + * Verifies the description's PKCS#7 signature against the builtin
> + * trusted keyring.
> + */
> + err = verify_pkcs7_signature(key->description,
> + strlen(key->description), prep->data, prep->datalen,
> + NULL, VERIFYING_UNSPECIFIED_SIGNATURE, NULL, NULL);
> + if (err)
> + return err;
> +#else
> + /*
> + * It should not be possible to come here because the keyring doesn't
> + * have KEY_USR_WRITE and the only other way to call this function is
> + * for builtin hashes.
> + */
> + WARN_ON_ONCE(1);
> + return -EPERM;
> +#endif
> +
> +out:
> + return generic_key_instantiate(key, prep);
> }
>
> -static void blacklist_free_preparse(struct key_preparsed_payload *prep)
> +static int blacklist_key_update(struct key *key,
> + struct key_preparsed_payload *prep)
> {
> + return -EPERM;
> }
>
> static void blacklist_describe(const struct key *key, struct seq_file *m)
> @@ -101,9 +137,8 @@ static void blacklist_describe(const struct key *key, struct seq_file *m)
> static struct key_type key_type_blacklist = {
> .name = "blacklist",
> .vet_description = blacklist_vet_description,
> - .preparse = blacklist_preparse,
> - .free_preparse = blacklist_free_preparse,
> - .instantiate = generic_key_instantiate,
> + .instantiate = blacklist_key_instantiate,
> + .update = blacklist_key_update,
> .describe = blacklist_describe,
> };
>
> @@ -152,8 +187,7 @@ static int mark_raw_hash_blacklisted(const char *hash)
> hash,
> NULL,
> 0,
> - ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> - KEY_USR_VIEW),
> + BLACKLIST_KEY_PERM,
> KEY_ALLOC_NOT_IN_QUOTA |
> KEY_ALLOC_BUILT_IN);
> if (IS_ERR(key)) {
> @@ -172,8 +206,10 @@ int add_key_to_revocation_list(const char *data, size_t size)
> NULL,
> data,
> size,
> - ((KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW),
> - KEY_ALLOC_NOT_IN_QUOTA | KEY_ALLOC_BUILT_IN);
> + KEY_POS_VIEW | KEY_POS_READ | KEY_POS_SEARCH
> + | KEY_USR_VIEW,
> + KEY_ALLOC_NOT_IN_QUOTA | KEY_ALLOC_BUILT_IN
> + | KEY_ALLOC_BYPASS_RESTRICTION);
>
> if (IS_ERR(key)) {
> pr_err("Problem with revocation key (%ld)\n", PTR_ERR(key));
> @@ -247,25 +283,43 @@ int is_binary_blacklisted(const u8 *hash, size_t hash_len)
> }
> EXPORT_SYMBOL_GPL(is_binary_blacklisted);
>
> +static int restrict_link_for_blacklist(struct key *dest_keyring,
> + const struct key_type *type, const union key_payload *payload,
> + struct key *restrict_key)
> +{
> + if (type == &key_type_blacklist)
> + return 0;
> + return -EOPNOTSUPP;
> +}
> +
> /*
> * Initialise the blacklist
> */
> static int __init blacklist_init(void)
> {
> const char *const *bl;
> + struct key_restriction *restriction;
>
> if (register_key_type(&key_type_blacklist) < 0)
> panic("Can't allocate system blacklist key type\n");
>
> + restriction = kzalloc(sizeof(*restriction), GFP_KERNEL);
> + if (!restriction)
> + panic("Can't allocate blacklist keyring restriction\n");
> + restriction->check = restrict_link_for_blacklist;
> +
> blacklist_keyring =
> keyring_alloc(".blacklist",
> GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
> - (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> - KEY_USR_VIEW | KEY_USR_READ |
> - KEY_USR_SEARCH,
> - KEY_ALLOC_NOT_IN_QUOTA |
> + KEY_POS_VIEW | KEY_POS_READ | KEY_POS_SEARCH |
> + KEY_POS_WRITE |
> + KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH
> +#ifdef CONFIG_SYSTEM_BLACKLIST_AUTH_UPDATE
> + | KEY_USR_WRITE
> +#endif
> + , KEY_ALLOC_NOT_IN_QUOTA |
> KEY_ALLOC_SET_KEEP,
> - NULL, NULL);
> + restriction, NULL);
> if (IS_ERR(blacklist_keyring))
> panic("Can't allocate system blacklist keyring\n");
>
> --
> 2.30.0
>
>