Re: [PATCH v3 09/10] certs: Allow root user to append signed hashes to the blacklist keyring
On 20/01/2021 06:23, Jarkko Sakkinen wrote: > On Thu, Jan 14, 2021 at 04:19:08PM +0100, Mickaël Salaün wrote: >> From: Mickaël Salaün >> >> 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 and blacklist key 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; >> * restricts kernel rights on the blacklist keyring to align with the >> root user rights. >> >> See the help in tools/certs/print-cert-tbs-hash.sh provided by a >> following commit. > > Please re-order patches in a way that print-cert-tbs-hash.sh is > available before this. That way we get rid of this useless remark. OK > >> Cc: David Howells >> Cc: David Woodhouse >> Signed-off-by: Mickaël Salaün > > /Jarkko >
Re: [PATCH v3 09/10] certs: Allow root user to append signed hashes to the blacklist keyring
On Thu, Jan 14, 2021 at 04:19:08PM +0100, Mickaël Salaün wrote: > From: Mickaël Salaün > > 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 and blacklist key 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; > * restricts kernel rights on the blacklist keyring to align with the > root user rights. > > See the help in tools/certs/print-cert-tbs-hash.sh provided by a > following commit. Please re-order patches in a way that print-cert-tbs-hash.sh is available before this. That way we get rid of this useless remark. > Cc: David Howells > Cc: David Woodhouse > Signed-off-by: Mickaël Salaün /Jarkko
Re: [PATCH v3 09/10] certs: Allow root user to append signed hashes to the blacklist keyring
Hi Mickaël, On Thu, 2021-01-14 at 16:19 +0100, Mickaël Salaün wrote: > From: Mickaël Salaün > > 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 and blacklist key 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; > * restricts kernel rights on the blacklist keyring to align with the > root user rights. > > See the help in tools/certs/print-cert-tbs-hash.sh provided by a > following commit. The design looks good. I'm hoping to review/test at least this patch next week. thanks, Mimi
[PATCH v3 09/10] certs: Allow root user to append signed hashes to the blacklist keyring
From: Mickaël Salaün
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 and blacklist key 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;
* restricts kernel rights on the blacklist keyring to align with the
root user rights.
See the help in tools/certs/print-cert-tbs-hash.sh provided by a
following commit.
Cc: David Howells
Cc: David Woodhouse
Signed-off-by: Mickaël Salaün
---
Changes since v2:
* Add comment for blacklist_key_instantiate().
---
certs/Kconfig | 10 ++
certs/blacklist.c | 90 +--
2 files changed, 81 insertions(+), 19 deletions(-)
diff --git a/certs/Kconfig b/certs/Kconfig
index c94e93d8bccf..35fe9989e7b9 100644
--- a/certs/Kconfig
+++ b/certs/Kconfig
@@ -83,4 +83,14 @@ config SYSTEM_BLACKLIST_HASH_LIST
wrapper to incorporate the list into the kernel. Each should
be a string of hex digits.
+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 1e63971bea94..07c592ae5307 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include "blacklist.h"
@@ -25,6 +26,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";
@@ -74,19 +78,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)
@@ -97,9 +133,8 @@ static void blacklist_describe(const struct key *key, struct
seq_file *m)
static struct key_type key_type_blacklist = {
.name = "blacklist",
.vet_description=
