* [PATCH 01/10] KEYS: Generalise system_verify_data() to provide access to internal content
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
@ 2015-10-21 15:13 ` David Howells
2015-10-21 15:13 ` [PATCH 02/10] PKCS#7: Make trust determination dependent on contents of trust keyring David Howells
` (9 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:13 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Generalise system_verify_data() to provide access to internal content
through a callback. This allows all the PKCS#7 stuff to be hidden inside
this function and removed from the PE file parser and the PKCS#7 test key.
If external content is not required, NULL should be passed as data to the
function. If the callback is not required, that can be set to NULL.
The function is now called verify_pkcs7_signature() to contrast with
verify_pefile_signature() and the definitions of both have been moved into
linux/verification.h along with the key_being_used_for enum.
Signed-off-by: David Howells <dhowells@redhat.com>
---
arch/x86/kernel/kexec-bzimage64.c | 18 ++-------
certs/system_keyring.c | 45 +++++++++++++++++-----
crypto/asymmetric_keys/Kconfig | 1
crypto/asymmetric_keys/mscode_parser.c | 21 +++-------
crypto/asymmetric_keys/pkcs7_key_type.c | 64 +++++++++++++++----------------
crypto/asymmetric_keys/pkcs7_parser.c | 21 +++++-----
crypto/asymmetric_keys/verify_pefile.c | 40 ++++---------------
crypto/asymmetric_keys/verify_pefile.h | 5 +-
include/crypto/pkcs7.h | 3 +
include/crypto/public_key.h | 14 -------
include/keys/asymmetric-type.h | 1
include/keys/system_keyring.h | 7 ---
include/linux/verification.h | 50 ++++++++++++++++++++++++
include/linux/verify_pefile.h | 22 -----------
kernel/module_signing.c | 5 +-
15 files changed, 156 insertions(+), 161 deletions(-)
create mode 100644 include/linux/verification.h
delete mode 100644 include/linux/verify_pefile.h
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 0f8a6bbaaa44..0b5da62eb203 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -19,8 +19,7 @@
#include <linux/kernel.h>
#include <linux/mm.h>
#include <linux/efi.h>
-#include <linux/verify_pefile.h>
-#include <keys/system_keyring.h>
+#include <linux/verification.h>
#include <asm/bootparam.h>
#include <asm/setup.h>
@@ -529,18 +528,9 @@ static int bzImage64_cleanup(void *loader_data)
#ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
static int bzImage64_verify_sig(const char *kernel, unsigned long kernel_len)
{
- bool trusted;
- int ret;
-
- ret = verify_pefile_signature(kernel, kernel_len,
- system_trusted_keyring,
- VERIFYING_KEXEC_PE_SIGNATURE,
- &trusted);
- if (ret < 0)
- return ret;
- if (!trusted)
- return -EKEYREJECTED;
- return 0;
+ return verify_pefile_signature(kernel, kernel_len,
+ NULL,
+ VERIFYING_KEXEC_PE_SIGNATURE);
}
#endif
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 2570598b784d..cf55bd3a072a 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -108,16 +108,25 @@ late_initcall(load_system_certificate_list);
#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
/**
- * Verify a PKCS#7-based signature on system data.
- * @data: The data to be verified.
+ * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
+ * @data: The data to be verified (NULL if expecting internal data).
* @len: Size of @data.
* @raw_pkcs7: The PKCS#7 message that is the signature.
* @pkcs7_len: The size of @raw_pkcs7.
+ * @trusted_keys: Trusted keys to use (NULL for system_trusted_keyring).
* @usage: The use to which the key is being put.
+ * @view_content: Callback to gain access to content.
+ * @ctx: Context for callback.
*/
-int system_verify_data(const void *data, unsigned long len,
- const void *raw_pkcs7, size_t pkcs7_len,
- enum key_being_used_for usage)
+int verify_pkcs7_signature(const void *data, size_t len,
+ const void *raw_pkcs7, size_t pkcs7_len,
+ struct key *trusted_keys,
+ int untrusted_error,
+ enum key_being_used_for usage,
+ int (*view_content)(void *ctx,
+ const void *data, size_t len,
+ size_t asn1hdrlen),
+ void *ctx)
{
struct pkcs7_message *pkcs7;
bool trusted;
@@ -128,7 +137,7 @@ int system_verify_data(const void *data, unsigned long len,
return PTR_ERR(pkcs7);
/* The data should be detached - so we need to supply it. */
- if (pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
+ if (data && pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
pr_err("PKCS#7 signature with non-detached data\n");
ret = -EBADMSG;
goto error;
@@ -138,13 +147,29 @@ int system_verify_data(const void *data, unsigned long len,
if (ret < 0)
goto error;
- ret = pkcs7_validate_trust(pkcs7, system_trusted_keyring, &trusted);
+ if (!trusted_keys)
+ trusted_keys = system_trusted_keyring;
+ ret = pkcs7_validate_trust(pkcs7, trusted_keys, &trusted);
if (ret < 0)
goto error;
- if (!trusted) {
+ if (!trusted && untrusted_error) {
pr_err("PKCS#7 signature not signed with a trusted key\n");
- ret = -ENOKEY;
+ ret = untrusted_error;
+ goto error;
+ }
+
+ if (view_content) {
+ size_t asn1hdrlen;
+
+ ret = pkcs7_get_content_data(pkcs7, &data, &len, &asn1hdrlen);
+ if (ret < 0) {
+ if (ret == -ENODATA)
+ pr_devel("PKCS#7 message does not contain data\n");
+ goto error;
+ }
+
+ ret = view_content(ctx, data, len, asn1hdrlen);
}
error:
@@ -152,6 +177,6 @@ error:
pr_devel("<==%s() = %d\n", __func__, ret);
return ret;
}
-EXPORT_SYMBOL_GPL(system_verify_data);
+EXPORT_SYMBOL_GPL(verify_pkcs7_signature);
#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index 4870f28403f5..d071989142b5 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -61,6 +61,7 @@ config PKCS7_TEST_KEY
config SIGNED_PE_FILE_VERIFICATION
bool "Support for PE file signature verification"
depends on PKCS7_MESSAGE_PARSER=y
+ depends on SYSTEM_DATA_VERIFICATION
select ASN1
select OID_REGISTRY
help
diff --git a/crypto/asymmetric_keys/mscode_parser.c b/crypto/asymmetric_keys/mscode_parser.c
index adcef59eec0b..b2fa9eea3ef6 100644
--- a/crypto/asymmetric_keys/mscode_parser.c
+++ b/crypto/asymmetric_keys/mscode_parser.c
@@ -21,19 +21,13 @@
/*
* Parse a Microsoft Individual Code Signing blob
*/
-int mscode_parse(struct pefile_context *ctx)
+int mscode_parse(void *_ctx, const void *content_data, size_t data_len,
+ size_t asn1hdrlen)
{
- const void *content_data;
- size_t data_len;
- int ret;
-
- ret = pkcs7_get_content_data(ctx->pkcs7, &content_data, &data_len, 1);
-
- if (ret) {
- pr_debug("PKCS#7 message does not contain data\n");
- return ret;
- }
+ struct pefile_context *ctx = _ctx;
+ content_data -= asn1hdrlen;
+ data_len += asn1hdrlen;
pr_devel("Data: %zu [%*ph]\n", data_len, (unsigned)(data_len),
content_data);
@@ -129,7 +123,6 @@ int mscode_note_digest(void *context, size_t hdrlen,
{
struct pefile_context *ctx = context;
- ctx->digest = value;
- ctx->digest_len = vlen;
- return 0;
+ ctx->digest = kmemdup(value, vlen, GFP_KERNEL);
+ return ctx->digest ? 0 : -ENOMEM;
}
diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c b/crypto/asymmetric_keys/pkcs7_key_type.c
index e2d0edbbc71a..240a5303ebb7 100644
--- a/crypto/asymmetric_keys/pkcs7_key_type.c
+++ b/crypto/asymmetric_keys/pkcs7_key_type.c
@@ -29,15 +29,37 @@ MODULE_PARM_DESC(pkcs7_usage,
"Usage to specify when verifying the PKCS#7 message");
/*
+ * Retrieve the PKCS#7 message content.
+ */
+static int pkcs7_view_content(void *ctx, const void *data, size_t len,
+ size_t asn1hdrlen)
+{
+ struct key_preparsed_payload *prep = ctx;
+ const void *saved_prep_data;
+ size_t saved_prep_datalen;
+ int ret;
+
+ kenter(",%zu", len);
+
+ saved_prep_data = prep->data;
+ saved_prep_datalen = prep->datalen;
+ prep->data = data;
+ prep->datalen = len;
+
+ ret = user_preparse(prep);
+
+ prep->data = saved_prep_data;
+ prep->datalen = saved_prep_datalen;
+ kleave(" = %d", ret);
+ return ret;
+}
+
+/*
* Preparse a PKCS#7 wrapped and validated data blob.
*/
static int pkcs7_preparse(struct key_preparsed_payload *prep)
{
enum key_being_used_for usage = pkcs7_usage;
- struct pkcs7_message *pkcs7;
- const void *data, *saved_prep_data;
- size_t datalen, saved_prep_datalen;
- bool trusted;
int ret;
kenter("");
@@ -47,37 +69,11 @@ static int pkcs7_preparse(struct key_preparsed_payload *prep)
return -EINVAL;
}
- saved_prep_data = prep->data;
- saved_prep_datalen = prep->datalen;
- pkcs7 = pkcs7_parse_message(saved_prep_data, saved_prep_datalen);
- if (IS_ERR(pkcs7)) {
- ret = PTR_ERR(pkcs7);
- goto error;
- }
-
- ret = pkcs7_verify(pkcs7, usage);
- if (ret < 0)
- goto error_free;
-
- ret = pkcs7_validate_trust(pkcs7, system_trusted_keyring, &trusted);
- if (ret < 0)
- goto error_free;
- if (!trusted)
- pr_warn("PKCS#7 message doesn't chain back to a trusted key\n");
-
- ret = pkcs7_get_content_data(pkcs7, &data, &datalen, false);
- if (ret < 0)
- goto error_free;
-
- prep->data = data;
- prep->datalen = datalen;
- ret = user_preparse(prep);
- prep->data = saved_prep_data;
- prep->datalen = saved_prep_datalen;
+ ret = verify_pkcs7_signature(NULL, 0,
+ prep->data, prep->datalen,
+ NULL, -ENOKEY, usage,
+ pkcs7_view_content, prep);
-error_free:
- pkcs7_free_message(pkcs7);
-error:
kleave(" = %d", ret);
return ret;
}
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index 758acabf2d81..7b69783cff99 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -165,24 +165,25 @@ EXPORT_SYMBOL_GPL(pkcs7_parse_message);
* @pkcs7: The preparsed PKCS#7 message to access
* @_data: Place to return a pointer to the data
* @_data_len: Place to return the data length
- * @want_wrapper: True if the ASN.1 object header should be included in the data
+ * @_headerlen: Size of ASN.1 header not included in _data
*
- * Get access to the data content of the PKCS#7 message, including, optionally,
- * the header of the ASN.1 object that contains it. Returns -ENODATA if the
- * data object was missing from the message.
+ * Get access to the data content of the PKCS#7 message. The size of the
+ * header of the ASN.1 object that contains it is also provided and can be used
+ * to adjust *_data and *_data_len to get the entire object.
+ *
+ * Returns -ENODATA if the data object was missing from the message.
*/
int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
const void **_data, size_t *_data_len,
- bool want_wrapper)
+ size_t *_headerlen)
{
- size_t wrapper;
-
if (!pkcs7->data)
return -ENODATA;
- wrapper = want_wrapper ? pkcs7->data_hdrlen : 0;
- *_data = pkcs7->data - wrapper;
- *_data_len = pkcs7->data_len + wrapper;
+ *_data = pkcs7->data;
+ *_data_len = pkcs7->data_len;
+ if (_headerlen)
+ *_headerlen = pkcs7->data_hdrlen;
return 0;
}
EXPORT_SYMBOL_GPL(pkcs7_get_content_data);
diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
index 897b734dabf9..443a00f9cd7a 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -16,7 +16,7 @@
#include <linux/err.h>
#include <linux/pe.h>
#include <linux/asn1.h>
-#include <crypto/pkcs7.h>
+#include <linux/verification.h>
#include <crypto/hash.h>
#include "verify_pefile.h"
@@ -392,9 +392,8 @@ error_no_desc:
* verify_pefile_signature - Verify the signature on a PE binary image
* @pebuf: Buffer containing the PE binary image
* @pelen: Length of the binary image
- * @trust_keyring: Signing certificates to use as starting points
+ * @trust_keys: Signing certificate(s) to use as starting points
* @usage: The use to which the key is being put.
- * @_trusted: Set to true if trustworth, false otherwise
*
* Validate that the certificate chain inside the PKCS#7 message inside the PE
* binary image intersects keys we already know and trust.
@@ -418,14 +417,10 @@ error_no_desc:
* May also return -ENOMEM.
*/
int verify_pefile_signature(const void *pebuf, unsigned pelen,
- struct key *trusted_keyring,
- enum key_being_used_for usage,
- bool *_trusted)
+ struct key *trusted_keys,
+ enum key_being_used_for usage)
{
- struct pkcs7_message *pkcs7;
struct pefile_context ctx;
- const void *data;
- size_t datalen;
int ret;
kenter("");
@@ -439,19 +434,10 @@ int verify_pefile_signature(const void *pebuf, unsigned pelen,
if (ret < 0)
return ret;
- pkcs7 = pkcs7_parse_message(pebuf + ctx.sig_offset, ctx.sig_len);
- if (IS_ERR(pkcs7))
- return PTR_ERR(pkcs7);
- ctx.pkcs7 = pkcs7;
-
- ret = pkcs7_get_content_data(ctx.pkcs7, &data, &datalen, false);
- if (ret < 0 || datalen == 0) {
- pr_devel("PKCS#7 message does not contain data\n");
- ret = -EBADMSG;
- goto error;
- }
-
- ret = mscode_parse(&ctx);
+ ret = verify_pkcs7_signature(NULL, 0,
+ pebuf + ctx.sig_offset, ctx.sig_len,
+ trusted_keys, -EKEYREJECTED, usage,
+ mscode_parse, &ctx);
if (ret < 0)
goto error;
@@ -462,16 +448,8 @@ int verify_pefile_signature(const void *pebuf, unsigned pelen,
* contents.
*/
ret = pefile_digest_pe(pebuf, pelen, &ctx);
- if (ret < 0)
- goto error;
-
- ret = pkcs7_verify(pkcs7, usage);
- if (ret < 0)
- goto error;
-
- ret = pkcs7_validate_trust(pkcs7, trusted_keyring, _trusted);
error:
- pkcs7_free_message(ctx.pkcs7);
+ kfree(ctx.digest);
return ret;
}
diff --git a/crypto/asymmetric_keys/verify_pefile.h b/crypto/asymmetric_keys/verify_pefile.h
index 55d5f7ebc45a..d6341d5406d4 100644
--- a/crypto/asymmetric_keys/verify_pefile.h
+++ b/crypto/asymmetric_keys/verify_pefile.h
@@ -9,7 +9,6 @@
* 2 of the Licence, or (at your option) any later version.
*/
-#include <linux/verify_pefile.h>
#include <crypto/pkcs7.h>
#include <crypto/hash_info.h>
@@ -23,7 +22,6 @@ struct pefile_context {
unsigned sig_offset;
unsigned sig_len;
const struct section_header *secs;
- struct pkcs7_message *pkcs7;
/* PKCS#7 MS Individual Code Signing content */
const void *digest; /* Digest */
@@ -39,4 +37,5 @@ struct pefile_context {
/*
* mscode_parser.c
*/
-extern int mscode_parse(struct pefile_context *ctx);
+extern int mscode_parse(void *_ctx, const void *content_data, size_t data_len,
+ size_t asn1hdrlen);
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 441aff9b5aa7..8323e3e57131 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -12,6 +12,7 @@
#ifndef _CRYPTO_PKCS7_H
#define _CRYPTO_PKCS7_H
+#include <linux/verification.h>
#include <crypto/public_key.h>
struct key;
@@ -26,7 +27,7 @@ extern void pkcs7_free_message(struct pkcs7_message *pkcs7);
extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
const void **_data, size_t *_datalen,
- bool want_wrapper);
+ size_t *_headerlen);
/*
* pkcs7_trust.c
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index cc2516df0efa..de50d026576d 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -39,20 +39,6 @@ enum pkey_id_type {
extern const char *const pkey_id_type_name[PKEY_ID_TYPE__LAST];
/*
- * The use to which an asymmetric key is being put.
- */
-enum key_being_used_for {
- VERIFYING_MODULE_SIGNATURE,
- VERIFYING_FIRMWARE_SIGNATURE,
- VERIFYING_KEXEC_PE_SIGNATURE,
- VERIFYING_KEY_SIGNATURE,
- VERIFYING_KEY_SELF_SIGNATURE,
- VERIFYING_UNSPECIFIED_SIGNATURE,
- NR__KEY_BEING_USED_FOR
-};
-extern const char *const key_being_used_for[NR__KEY_BEING_USED_FOR];
-
-/*
* Cryptographic data for the public-key subtype of the asymmetric key type.
*
* Note that this may include private part of the key as well as the public
diff --git a/include/keys/asymmetric-type.h b/include/keys/asymmetric-type.h
index 59c1df9cf922..72c18c1f3308 100644
--- a/include/keys/asymmetric-type.h
+++ b/include/keys/asymmetric-type.h
@@ -15,6 +15,7 @@
#define _KEYS_ASYMMETRIC_TYPE_H
#include <linux/key-type.h>
+#include <linux/verification.h>
extern struct key_type key_type_asymmetric;
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index b20cd885c1fd..18e9310b0a36 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -15,6 +15,7 @@
#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
#include <linux/key.h>
+#include <linux/verification.h>
#include <crypto/public_key.h>
extern struct key *system_trusted_keyring;
@@ -29,10 +30,4 @@ static inline struct key *get_system_trusted_keyring(void)
}
#endif
-#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
-extern int system_verify_data(const void *data, unsigned long len,
- const void *raw_pkcs7, size_t pkcs7_len,
- enum key_being_used_for usage);
-#endif
-
#endif /* _KEYS_SYSTEM_KEYRING_H */
diff --git a/include/linux/verification.h b/include/linux/verification.h
new file mode 100644
index 000000000000..bb0fcf941cb7
--- /dev/null
+++ b/include/linux/verification.h
@@ -0,0 +1,50 @@
+/* Signature verification
+ *
+ * Copyright (C) 2014 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _LINUX_VERIFICATION_H
+#define _LINUX_VERIFICATION_H
+
+/*
+ * The use to which an asymmetric key is being put.
+ */
+enum key_being_used_for {
+ VERIFYING_MODULE_SIGNATURE,
+ VERIFYING_FIRMWARE_SIGNATURE,
+ VERIFYING_KEXEC_PE_SIGNATURE,
+ VERIFYING_KEY_SIGNATURE,
+ VERIFYING_KEY_SELF_SIGNATURE,
+ VERIFYING_UNSPECIFIED_SIGNATURE,
+ NR__KEY_BEING_USED_FOR
+};
+extern const char *const key_being_used_for[NR__KEY_BEING_USED_FOR];
+
+#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
+
+struct key;
+
+extern int verify_pkcs7_signature(const void *data, size_t len,
+ const void *raw_pkcs7, size_t pkcs7_len,
+ struct key *trusted_keys,
+ int untrusted_error,
+ enum key_being_used_for usage,
+ int (*view_content)(void *ctx,
+ const void *data, size_t len,
+ size_t asn1hdrlen),
+ void *ctx);
+
+#ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
+extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
+ struct key *trusted_keys,
+ enum key_being_used_for usage);
+#endif
+
+#endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
+#endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/include/linux/verify_pefile.h b/include/linux/verify_pefile.h
deleted file mode 100644
index da2049b5161c..000000000000
--- a/include/linux/verify_pefile.h
+++ /dev/null
@@ -1,22 +0,0 @@
-/* Signed PE file verification
- *
- * Copyright (C) 2014 Red Hat, Inc. All Rights Reserved.
- * Written by David Howells (dhowells@redhat.com)
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public Licence
- * as published by the Free Software Foundation; either version
- * 2 of the Licence, or (at your option) any later version.
- */
-
-#ifndef _LINUX_VERIFY_PEFILE_H
-#define _LINUX_VERIFY_PEFILE_H
-
-#include <crypto/public_key.h>
-
-extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
- struct key *trusted_keyring,
- enum key_being_used_for usage,
- bool *_trusted);
-
-#endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 6528a79d998d..70cf0220efeb 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -73,6 +73,7 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
return -EBADMSG;
}
- return system_verify_data(mod, modlen, mod + modlen, sig_len,
- VERIFYING_MODULE_SIGNATURE);
+ return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
+ NULL, -ENOKEY, VERIFYING_MODULE_SIGNATURE,
+ NULL, NULL);
}
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 02/10] PKCS#7: Make trust determination dependent on contents of trust keyring
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
2015-10-21 15:13 ` [PATCH 01/10] KEYS: Generalise system_verify_data() to provide access to internal content David Howells
@ 2015-10-21 15:13 ` David Howells
2015-10-21 15:13 ` [PATCH 03/10] KEYS: Add facility to check key trustworthiness upon link creation David Howells
` (8 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:13 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Make the determination of the trustworthiness of a key dependent on whether
a key that can verify it is present in the ring of trusted keys rather than
whether or not the verifying key has KEY_FLAG_TRUSTED set.
Signed-off-by: David Howells <dhowells@redhat.com>
---
certs/system_keyring.c | 13 ++++---------
crypto/asymmetric_keys/pkcs7_key_type.c | 2 +-
crypto/asymmetric_keys/pkcs7_parser.h | 1 -
crypto/asymmetric_keys/pkcs7_trust.c | 16 +++-------------
crypto/asymmetric_keys/verify_pefile.c | 2 +-
crypto/asymmetric_keys/x509_parser.h | 1 -
include/crypto/pkcs7.h | 3 +--
include/linux/verification.h | 1 -
kernel/module_signing.c | 2 +-
9 files changed, 11 insertions(+), 30 deletions(-)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index cf55bd3a072a..e7f286413276 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -121,7 +121,6 @@ late_initcall(load_system_certificate_list);
int verify_pkcs7_signature(const void *data, size_t len,
const void *raw_pkcs7, size_t pkcs7_len,
struct key *trusted_keys,
- int untrusted_error,
enum key_being_used_for usage,
int (*view_content)(void *ctx,
const void *data, size_t len,
@@ -129,7 +128,6 @@ int verify_pkcs7_signature(const void *data, size_t len,
void *ctx)
{
struct pkcs7_message *pkcs7;
- bool trusted;
int ret;
pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
@@ -149,13 +147,10 @@ int verify_pkcs7_signature(const void *data, size_t len,
if (!trusted_keys)
trusted_keys = system_trusted_keyring;
- ret = pkcs7_validate_trust(pkcs7, trusted_keys, &trusted);
- if (ret < 0)
- goto error;
-
- if (!trusted && untrusted_error) {
- pr_err("PKCS#7 signature not signed with a trusted key\n");
- ret = untrusted_error;
+ ret = pkcs7_validate_trust(pkcs7, trusted_keys);
+ if (ret < 0) {
+ if (ret == -ENOKEY)
+ pr_err("PKCS#7 signature not signed with a trusted key\n");
goto error;
}
diff --git a/crypto/asymmetric_keys/pkcs7_key_type.c b/crypto/asymmetric_keys/pkcs7_key_type.c
index 240a5303ebb7..89b75477868d 100644
--- a/crypto/asymmetric_keys/pkcs7_key_type.c
+++ b/crypto/asymmetric_keys/pkcs7_key_type.c
@@ -71,7 +71,7 @@ static int pkcs7_preparse(struct key_preparsed_payload *prep)
ret = verify_pkcs7_signature(NULL, 0,
prep->data, prep->datalen,
- NULL, -ENOKEY, usage,
+ NULL, usage,
pkcs7_view_content, prep);
kleave(" = %d", ret);
diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
index a66b19ebcf47..c8159983ed8f 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.h
+++ b/crypto/asymmetric_keys/pkcs7_parser.h
@@ -22,7 +22,6 @@ struct pkcs7_signed_info {
struct pkcs7_signed_info *next;
struct x509_certificate *signer; /* Signing certificate (in msg->certs) */
unsigned index;
- bool trusted;
bool unsupported_crypto; /* T if not usable due to missing crypto */
/* Message digest - the digest of the Content Data (or NULL) */
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 90d6d47965b0..388007fed3b2 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -30,7 +30,6 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
struct public_key_signature *sig = &sinfo->sig;
struct x509_certificate *x509, *last = NULL, *p;
struct key *key;
- bool trusted;
int ret;
kenter(",%u,", sinfo->index);
@@ -42,10 +41,8 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
for (x509 = sinfo->signer; x509; x509 = x509->signer) {
if (x509->seen) {
- if (x509->verified) {
- trusted = x509->trusted;
+ if (x509->verified)
goto verified;
- }
kleave(" = -ENOKEY [cached]");
return -ENOKEY;
}
@@ -122,7 +119,6 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
matched:
ret = verify_signature(key, sig);
- trusted = test_bit(KEY_FLAG_TRUSTED, &key->flags);
key_put(key);
if (ret < 0) {
if (ret == -ENOMEM)
@@ -134,12 +130,9 @@ matched:
verified:
if (x509) {
x509->verified = true;
- for (p = sinfo->signer; p != x509; p = p->signer) {
+ for (p = sinfo->signer; p != x509; p = p->signer)
p->verified = true;
- p->trusted = trusted;
- }
}
- sinfo->trusted = trusted;
kleave(" = 0");
return 0;
}
@@ -148,7 +141,6 @@ verified:
* pkcs7_validate_trust - Validate PKCS#7 trust chain
* @pkcs7: The PKCS#7 certificate to validate
* @trust_keyring: Signing certificates to use as starting points
- * @_trusted: Set to true if trustworth, false otherwise
*
* Validate that the certificate chain inside the PKCS#7 message intersects
* keys we already know and trust.
@@ -170,8 +162,7 @@ verified:
* May also return -ENOMEM.
*/
int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
- struct key *trust_keyring,
- bool *_trusted)
+ struct key *trust_keyring)
{
struct pkcs7_signed_info *sinfo;
struct x509_certificate *p;
@@ -191,7 +182,6 @@ int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
cached_ret = -ENOPKG;
continue;
case 0:
- *_trusted |= sinfo->trusted;
cached_ret = 0;
continue;
default:
diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
index 443a00f9cd7a..4112f922cc66 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -436,7 +436,7 @@ int verify_pefile_signature(const void *pebuf, unsigned pelen,
ret = verify_pkcs7_signature(NULL, 0,
pebuf + ctx.sig_offset, ctx.sig_len,
- trusted_keys, -EKEYREJECTED, usage,
+ trusted_keys, usage,
mscode_parse, &ctx);
if (ret < 0)
goto error;
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index dbeed6018e63..36b7c47335b5 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -41,7 +41,6 @@ struct x509_certificate {
unsigned index;
bool seen; /* Infinite recursion prevention */
bool verified;
- bool trusted;
bool unsupported_crypto; /* T if can't be verified due to missing crypto */
};
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 8323e3e57131..583f199400a3 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -33,8 +33,7 @@ extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
* pkcs7_trust.c
*/
extern int pkcs7_validate_trust(struct pkcs7_message *pkcs7,
- struct key *trust_keyring,
- bool *_trusted);
+ struct key *trust_keyring);
/*
* pkcs7_verify.c
diff --git a/include/linux/verification.h b/include/linux/verification.h
index bb0fcf941cb7..a10549a6c7cd 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -33,7 +33,6 @@ struct key;
extern int verify_pkcs7_signature(const void *data, size_t len,
const void *raw_pkcs7, size_t pkcs7_len,
struct key *trusted_keys,
- int untrusted_error,
enum key_being_used_for usage,
int (*view_content)(void *ctx,
const void *data, size_t len,
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 70cf0220efeb..b3dafe4fd320 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -74,6 +74,6 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
}
return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
- NULL, -ENOKEY, VERIFYING_MODULE_SIGNATURE,
+ NULL, VERIFYING_MODULE_SIGNATURE,
NULL, NULL);
}
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 03/10] KEYS: Add facility to check key trustworthiness upon link creation
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
2015-10-21 15:13 ` [PATCH 01/10] KEYS: Generalise system_verify_data() to provide access to internal content David Howells
2015-10-21 15:13 ` [PATCH 02/10] PKCS#7: Make trust determination dependent on contents of trust keyring David Howells
@ 2015-10-21 15:13 ` David Howells
2015-10-21 15:13 ` [PATCH 04/10] KEYS: Allow authentication data to be stored in an asymmetric key David Howells
` (7 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:13 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Add a facility whereby if KEY_FLAG_TRUSTED_ONLY is set on the destination
keyring, the creation of a link to a candidate key will cause the
trustworthiness of that key to be evaluated against the already present
contents of that keyring. This affects operations like add_key(),
KEYCTL_LINK and KEYCTL_INSTANTIATE.
To this end:
(1) A new key type method is provided:
int (*verify_trust)(const union key_payload *payload,
struct key *keyring);
This is implemented by key types for which verification of one key by
another is appropriate. It is primarily intended for use with the
asymmetric key type.
When called, it is given the payload or prospective payload[*] of the
candidate key to verify and a pointer to the destination keyring. The
method is expected to search the keying for an appropriate key with
which to verify the candidate.
[*] If called during add_key(), preparse is called before this method,
but a key isn't actually allocated unless the verification is
successful.
(2) KEY_FLAG_TRUSTED is removed. A key is now trusted by virtue of being
contained in the trusted-only keyring being searched.
(3) KEY_ALLOC_TRUSTED now acts as an override. If this is passed to
key_create_or_update() then the ->verify_trust() method will be
ignored and the key will be added anyway.
Signed-off-by: David Howells <dhowells@redhat.com>
---
Documentation/security/keys.txt | 17 ++++++++++++
crypto/asymmetric_keys/x509_public_key.c | 6 ++--
include/linux/key-type.h | 10 ++++++-
include/linux/key.h | 12 +++++---
security/keys/key.c | 44 ++++++++++++++++++++++++------
security/keys/keyring.c | 18 +++++++++++-
6 files changed, 87 insertions(+), 20 deletions(-)
diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 8c183873b2b7..e7f3447ccd1b 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -1183,6 +1183,23 @@ The structure has a number of fields, some of which are mandatory:
successfully, even if instantiate() or update() succeed.
+ (*) int (*verify_trust)(const union key_payload *payload, struct key *keyring);
+
+ If the keyring to which a candidate key is being added/linked is marked as
+ KEY_FLAG_TRUSTED_ONLY then this function will get called in the candidate
+ key type to verify the key or proposed key based on its payload. It is
+ expected to use the contents of the supplied destination keyring to
+ determine whether the candidate key is to be trusted and added to the
+ keyring.
+
+ The method should return 0 to allow the addition and an error otherwise,
+ typically ENOKEY if there's no key in the keyring to verify this key and
+ EKEYREJECTED if the selected key fails to verify the candidate.
+
+ This method is optional. If it is not supplied, keys of this type cannot
+ be added to trusted-only keyrings and EPERM will be returned.
+
+
(*) int (*instantiate)(struct key *key, struct key_preparsed_payload *prep);
This method is called to attach a payload to a key during construction.
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 64d42981a8d7..76c211b31da7 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -318,10 +318,10 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
ret = x509_check_signature(cert->pub, cert); /* self-signed */
if (ret < 0)
goto error_free_cert;
- } else if (!prep->trusted) {
+ } else {
ret = x509_validate_trust(cert, get_system_trusted_keyring());
- if (!ret)
- prep->trusted = 1;
+ if (ret == -EKEYREJECTED)
+ goto error_free_cert;
}
/* Propose a description */
diff --git a/include/linux/key-type.h b/include/linux/key-type.h
index 7463355a198b..5d7cf5e7f8c6 100644
--- a/include/linux/key-type.h
+++ b/include/linux/key-type.h
@@ -45,7 +45,6 @@ struct key_preparsed_payload {
size_t datalen; /* Raw datalen */
size_t quotalen; /* Quota length for proposed payload */
time_t expiry; /* Expiry time of key */
- bool trusted; /* True if key is trusted */
};
typedef int (*request_key_actor_t)(struct key_construction *key,
@@ -95,6 +94,15 @@ struct key_type {
*/
void (*free_preparse)(struct key_preparsed_payload *prep);
+ /* Verify the trust on a key when added to a trusted-only keyring.
+ *
+ * If this method isn't provided then it is assumed that the concept of
+ * trust is irrelevant to keys of this type and an attempt to add one
+ * to a trusted-only keyring will be rejected.
+ */
+ int (*verify_trust)(const union key_payload *payload,
+ struct key *keyring);
+
/* instantiate a key of this type
* - this method should call key_payload_reserve() to determine if the
* user's quota will hold the payload
diff --git a/include/linux/key.h b/include/linux/key.h
index 66f705243985..19cb9283448a 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -173,10 +173,12 @@ struct key {
#define KEY_FLAG_NEGATIVE 5 /* set if key is negative */
#define KEY_FLAG_ROOT_CAN_CLEAR 6 /* set if key can be cleared by root without permission */
#define KEY_FLAG_INVALIDATED 7 /* set if key has been invalidated */
-#define KEY_FLAG_TRUSTED 8 /* set if key is trusted */
-#define KEY_FLAG_TRUSTED_ONLY 9 /* set if keyring only accepts links to trusted keys */
-#define KEY_FLAG_BUILTIN 10 /* set if key is builtin */
-#define KEY_FLAG_ROOT_CAN_INVAL 11 /* set if key can be invalidated by root without permission */
+#define KEY_FLAG_TRUSTED_ONLY 8 /* set to make keyring only accept links to keys that can
+ * be verified by one of the keys it already contains or
+ * if KEY_ALLOC_TRUSTED is flagged.
+ */
+#define KEY_FLAG_BUILTIN 9 /* set if key is builtin */
+#define KEY_FLAG_ROOT_CAN_INVAL 10 /* set if key can be invalidated by root without permission */
/* the key type and key description string
* - the desc is used to match a key against search criteria
@@ -217,7 +219,7 @@ extern struct key *key_alloc(struct key_type *type,
#define KEY_ALLOC_IN_QUOTA 0x0000 /* add to quota, reject if would overrun */
#define KEY_ALLOC_QUOTA_OVERRUN 0x0001 /* add to quota, permit even if overrun */
#define KEY_ALLOC_NOT_IN_QUOTA 0x0002 /* not in quota */
-#define KEY_ALLOC_TRUSTED 0x0004 /* Key should be flagged as trusted */
+#define KEY_ALLOC_TRUSTED 0x0004 /* Override the verification check on trusted keyrings */
extern void key_revoke(struct key *key);
extern void key_invalidate(struct key *key);
diff --git a/security/keys/key.c b/security/keys/key.c
index ab7997ded725..e081e3921397 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -294,8 +294,6 @@ struct key *key_alloc(struct key_type *type, const char *desc,
if (!(flags & KEY_ALLOC_NOT_IN_QUOTA))
key->flags |= 1 << KEY_FLAG_IN_QUOTA;
- if (flags & KEY_ALLOC_TRUSTED)
- key->flags |= 1 << KEY_FLAG_TRUSTED;
#ifdef KEY_DEBUGGING
key->magic = KEY_DEBUG_MAGIC;
@@ -478,6 +476,11 @@ int key_instantiate_and_link(struct key *key,
struct assoc_array_edit *edit;
int ret;
+ if (keyring &&
+ unlikely(test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags)) &&
+ !key->type->verify_trust)
+ return -EPERM;
+
memset(&prep, 0, sizeof(prep));
prep.data = data;
prep.datalen = datalen;
@@ -490,6 +493,13 @@ int key_instantiate_and_link(struct key *key,
}
if (keyring) {
+ if (unlikely(test_bit(KEY_FLAG_TRUSTED_ONLY,
+ &keyring->flags)) &&
+ key->type->verify_trust) {
+ ret = key->type->verify_trust(&prep.payload, keyring);
+ if (ret < 0)
+ goto error;
+ }
ret = __key_link_begin(keyring, &key->index_key, &edit);
if (ret < 0)
goto error;
@@ -545,8 +555,12 @@ int key_reject_and_link(struct key *key,
awaken = 0;
ret = -EBUSY;
- if (keyring)
+ if (keyring) {
+ if (unlikely(test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags)))
+ return -EPERM;
+
link_ret = __key_link_begin(keyring, &key->index_key, &edit);
+ }
mutex_lock(&key_construction_mutex);
@@ -786,6 +800,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
const struct cred *cred = current_cred();
struct key *keyring, *key = NULL;
key_ref_t key_ref;
+ bool verify_trust;
int ret;
/* look up the key type to see if it's one of the registered kernel
@@ -802,9 +817,18 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
goto error_put_type;
keyring = key_ref_to_ptr(keyring_ref);
-
key_check(keyring);
+ key_ref = ERR_PTR(-EPERM);
+ if (!test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags))
+ verify_trust = false;
+ else if (flags & KEY_ALLOC_TRUSTED)
+ verify_trust = false;
+ else if (index_key.type->verify_trust)
+ verify_trust = true;
+ else
+ goto error_put_type;
+
key_ref = ERR_PTR(-ENOTDIR);
if (keyring->type != &key_type_keyring)
goto error_put_type;
@@ -813,7 +837,6 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
prep.data = payload;
prep.datalen = plen;
prep.quotalen = index_key.type->def_datalen;
- prep.trusted = flags & KEY_ALLOC_TRUSTED;
prep.expiry = TIME_T_MAX;
if (index_key.type->preparse) {
ret = index_key.type->preparse(&prep);
@@ -829,10 +852,13 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
}
index_key.desc_len = strlen(index_key.description);
- key_ref = ERR_PTR(-EPERM);
- if (!prep.trusted && test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags))
- goto error_free_prep;
- flags |= prep.trusted ? KEY_ALLOC_TRUSTED : 0;
+ if (verify_trust) {
+ ret = index_key.type->verify_trust(&prep.payload, keyring);
+ if (ret < 0) {
+ key_ref = ERR_PTR(ret);
+ goto error_free_prep;
+ }
+ }
ret = __key_link_begin(keyring, &index_key, &edit);
if (ret < 0) {
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index f931ccfeefb0..01aa3837644a 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1191,6 +1191,18 @@ void __key_link_end(struct key *keyring,
up_write(&keyring->sem);
}
+/*
+ * Verify a trusted-only keyring link.
+ */
+static int __key_link_verify(struct key *keyring, struct key *key)
+{
+ if (!test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags))
+ return 0;
+ if (!key->type->verify_trust)
+ return -EPERM;
+ return key->type->verify_trust(&key->payload, keyring);
+}
+
/**
* key_link - Link a key to a keyring
* @keyring: The keyring to make the link in.
@@ -1222,13 +1234,15 @@ int key_link(struct key *keyring, struct key *key)
key_check(key);
if (test_bit(KEY_FLAG_TRUSTED_ONLY, &keyring->flags) &&
- !test_bit(KEY_FLAG_TRUSTED, &key->flags))
+ !key->type->verify_trust)
return -EPERM;
ret = __key_link_begin(keyring, &key->index_key, &edit);
if (ret == 0) {
kdebug("begun {%d,%d}", keyring->serial, atomic_read(&keyring->usage));
- ret = __key_link_check_live_key(keyring, key);
+ ret = __key_link_verify(keyring, key);
+ if (ret == 0)
+ ret = __key_link_check_live_key(keyring, key);
if (ret == 0)
__key_link(key, &edit);
__key_link_end(keyring, &key->index_key, edit);
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 04/10] KEYS: Allow authentication data to be stored in an asymmetric key
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
` (2 preceding siblings ...)
2015-10-21 15:13 ` [PATCH 03/10] KEYS: Add facility to check key trustworthiness upon link creation David Howells
@ 2015-10-21 15:13 ` David Howells
2015-10-21 15:14 ` [PATCH 05/10] KEYS: Add identifier pointers to public_key_signature struct David Howells
` (6 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:13 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Allow authentication data to be stored in an asymmetric key in the 4th
element of the key payload and provide a way for it to be destroyed.
For the public key subtype, this will be a public_key_signature struct.
Signed-off-by: David Howells <dhowells@redhat.com>
---
crypto/asymmetric_keys/asymmetric_type.c | 7 +++++--
crypto/asymmetric_keys/public_key.c | 22 +++++++++++++++++++---
crypto/asymmetric_keys/x509_cert_parser.c | 2 +-
include/crypto/public_key.h | 5 +++--
include/keys/asymmetric-subtype.h | 2 +-
include/keys/asymmetric-type.h | 7 ++++---
6 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index 9f2165b27d52..a79d30128821 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -331,7 +331,8 @@ static void asymmetric_key_free_preparse(struct key_preparsed_payload *prep)
pr_devel("==>%s()\n", __func__);
if (subtype) {
- subtype->destroy(prep->payload.data[asym_crypto]);
+ subtype->destroy(prep->payload.data[asym_crypto],
+ prep->payload.data[asym_auth]);
module_put(subtype->owner);
}
asymmetric_key_free_kids(kids);
@@ -346,13 +347,15 @@ static void asymmetric_key_destroy(struct key *key)
struct asymmetric_key_subtype *subtype = asymmetric_key_subtype(key);
struct asymmetric_key_ids *kids = key->payload.data[asym_key_ids];
void *data = key->payload.data[asym_crypto];
+ void *auth = key->payload.data[asym_auth];
key->payload.data[asym_crypto] = NULL;
key->payload.data[asym_subtype] = NULL;
key->payload.data[asym_key_ids] = NULL;
+ key->payload.data[asym_auth] = NULL;
if (subtype) {
- subtype->destroy(data);
+ subtype->destroy(data, auth);
module_put(subtype->owner);
}
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 6db4c01c6503..e537aaeafdbf 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -59,18 +59,34 @@ static void public_key_describe(const struct key *asymmetric_key,
/*
* Destroy a public key algorithm key.
*/
-void public_key_destroy(void *payload)
+void public_key_free(struct public_key *key,
+ struct public_key_signature *sig)
{
- struct public_key *key = payload;
int i;
if (key) {
for (i = 0; i < ARRAY_SIZE(key->mpi); i++)
mpi_free(key->mpi[i]);
kfree(key);
+ key = NULL;
}
+
+ if (sig) {
+ for (i = 0; i < ARRAY_SIZE(sig->mpi); i++)
+ mpi_free(sig->mpi[i]);
+ kfree(sig->digest);
+ kfree(sig);
+ }
+}
+EXPORT_SYMBOL_GPL(public_key_free);
+
+/*
+ * Destroy a public key algorithm key.
+ */
+static void public_key_destroy(void *payload0, void *payload3)
+{
+ public_key_free(payload0, payload3);
}
-EXPORT_SYMBOL_GPL(public_key_destroy);
/*
* Verify a signature using a public key.
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index af71878dc15b..430848445dd9 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -48,7 +48,7 @@ struct x509_parse_context {
void x509_free_certificate(struct x509_certificate *cert)
{
if (cert) {
- public_key_destroy(cert->pub);
+ public_key_free(cert->pub, NULL);
kfree(cert->issuer);
kfree(cert->subject);
kfree(cert->id);
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index de50d026576d..a3f8f8268e23 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -72,8 +72,6 @@ struct public_key {
};
};
-extern void public_key_destroy(void *payload);
-
/*
* Public key cryptography signature data
*/
@@ -95,6 +93,9 @@ struct public_key_signature {
};
};
+extern void public_key_free(struct public_key *key,
+ struct public_key_signature *sig);
+
struct key;
extern int verify_signature(const struct key *key,
const struct public_key_signature *sig);
diff --git a/include/keys/asymmetric-subtype.h b/include/keys/asymmetric-subtype.h
index 4915d40d3c3c..2480469ce8fb 100644
--- a/include/keys/asymmetric-subtype.h
+++ b/include/keys/asymmetric-subtype.h
@@ -32,7 +32,7 @@ struct asymmetric_key_subtype {
void (*describe)(const struct key *key, struct seq_file *m);
/* Destroy a key of this subtype */
- void (*destroy)(void *payload);
+ void (*destroy)(void *payload_crypto, void *payload_auth);
/* Verify the signature on a key of this subtype (optional) */
int (*verify_signature)(const struct key *key,
diff --git a/include/keys/asymmetric-type.h b/include/keys/asymmetric-type.h
index 72c18c1f3308..d1e23dda4363 100644
--- a/include/keys/asymmetric-type.h
+++ b/include/keys/asymmetric-type.h
@@ -24,9 +24,10 @@ extern struct key_type key_type_asymmetric;
* follows:
*/
enum asymmetric_payload_bits {
- asym_crypto,
- asym_subtype,
- asym_key_ids,
+ asym_crypto, /* The data representing the key */
+ asym_subtype, /* Pointer to an asymmetric_key_subtype struct */
+ asym_key_ids, /* Pointer to an asymmetric_key_ids struct */
+ asym_auth /* The key's authorisation (signature, parent key ID) */
};
/*
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 05/10] KEYS: Add identifier pointers to public_key_signature struct
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
` (3 preceding siblings ...)
2015-10-21 15:13 ` [PATCH 04/10] KEYS: Allow authentication data to be stored in an asymmetric key David Howells
@ 2015-10-21 15:14 ` David Howells
2015-10-21 15:14 ` [PATCH 06/10] X.509: Retain the key verification data David Howells
` (5 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:14 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Add key identifier pointers to public_key_signature struct so that they can
be used to retain the identifier of the key to be used to verify the
signature in both PKCS#7 and X.509.
Signed-off-by: David Howells <dhowells@redhat.com>
---
crypto/asymmetric_keys/public_key.c | 2 ++
include/crypto/public_key.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index e537aaeafdbf..f5b4824b7c77 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -72,6 +72,8 @@ void public_key_free(struct public_key *key,
}
if (sig) {
+ for (i = 0; i < ARRAY_SIZE(sig->auth_ids); i++)
+ kfree(sig->auth_ids[i]);
for (i = 0; i < ARRAY_SIZE(sig->mpi); i++)
mpi_free(sig->mpi[i]);
kfree(sig->digest);
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index a3f8f8268e23..ed86bfb23e89 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -76,6 +76,7 @@ struct public_key {
* Public key cryptography signature data
*/
struct public_key_signature {
+ struct asymmetric_key_id *auth_ids[2];
u8 *digest;
u8 digest_size; /* Number of bytes in digest */
u8 nr_mpi; /* Occupancy of mpi[] */
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 06/10] X.509: Retain the key verification data
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
` (4 preceding siblings ...)
2015-10-21 15:14 ` [PATCH 05/10] KEYS: Add identifier pointers to public_key_signature struct David Howells
@ 2015-10-21 15:14 ` David Howells
2015-10-21 15:14 ` [PATCH 07/10] X.509: Extract signature digest and make self-signed cert checks earlier David Howells
` (4 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:14 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Retain the key verification data (ie. the struct public_key_signature)
including the digest and the key identifiers.
Note that this means that we need to take a separate copy of the digest in
x509_get_sig_params() rather than lumping it in with the crypto layer data.
Signed-off-by: David Howells <dhowells@redhat.com>
---
crypto/asymmetric_keys/pkcs7_trust.c | 8 ++-
crypto/asymmetric_keys/pkcs7_verify.c | 20 +++++----
crypto/asymmetric_keys/x509_cert_parser.c | 43 +++++++++---------
crypto/asymmetric_keys/x509_parser.h | 4 --
crypto/asymmetric_keys/x509_public_key.c | 68 +++++++++++++++--------------
5 files changed, 72 insertions(+), 71 deletions(-)
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 388007fed3b2..7bb9389fd644 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -77,16 +77,16 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
might_sleep();
last = x509;
- sig = &last->sig;
+ sig = last->sig;
}
/* No match - see if the root certificate has a signer amongst the
* trusted keys.
*/
- if (last && (last->akid_id || last->akid_skid)) {
+ if (last && (last->sig->auth_ids[0] || last->sig->auth_ids[1])) {
key = x509_request_asymmetric_key(trust_keyring,
- last->akid_id,
- last->akid_skid,
+ last->sig->auth_ids[0],
+ last->sig->auth_ids[1],
false);
if (!IS_ERR(key)) {
x509 = last;
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index d20c0b4b880e..e225dccdf559 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -175,6 +175,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
struct pkcs7_signed_info *sinfo)
{
+ struct public_key_signature *sig;
struct x509_certificate *x509 = sinfo->signer, *p;
struct asymmetric_key_id *auth;
int ret;
@@ -194,14 +195,15 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
goto maybe_missing_crypto_in_x509;
pr_debug("- issuer %s\n", x509->issuer);
- if (x509->akid_id)
+ sig = x509->sig;
+ if (sig->auth_ids[0])
pr_debug("- authkeyid.id %*phN\n",
- x509->akid_id->len, x509->akid_id->data);
- if (x509->akid_skid)
+ sig->auth_ids[0]->len, sig->auth_ids[0]->data);
+ if (sig->auth_ids[1])
pr_debug("- authkeyid.skid %*phN\n",
- x509->akid_skid->len, x509->akid_skid->data);
+ sig->auth_ids[1]->len, sig->auth_ids[1]->data);
- if ((!x509->akid_id && !x509->akid_skid) ||
+ if ((!x509->sig->auth_ids[0] && !x509->sig->auth_ids[1]) ||
strcmp(x509->subject, x509->issuer) == 0) {
/* If there's no authority certificate specified, then
* the certificate must be self-signed and is the root
@@ -225,7 +227,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
/* Look through the X.509 certificates in the PKCS#7 message's
* list to see if the next one is there.
*/
- auth = x509->akid_id;
+ auth = sig->auth_ids[0];
if (auth) {
pr_debug("- want %*phN\n", auth->len, auth->data);
for (p = pkcs7->certs; p; p = p->next) {
@@ -235,7 +237,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
goto found_issuer_check_skid;
}
} else {
- auth = x509->akid_skid;
+ auth = sig->auth_ids[1];
pr_debug("- want %*phN\n", auth->len, auth->data);
for (p = pkcs7->certs; p; p = p->next) {
if (!p->skid)
@@ -255,8 +257,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
/* We matched issuer + serialNumber, but if there's an
* authKeyId.keyId, that must match the CA subjKeyId also.
*/
- if (x509->akid_skid &&
- !asymmetric_key_id_same(p->skid, x509->akid_skid)) {
+ if (sig->auth_ids[1] &&
+ !asymmetric_key_id_same(p->skid, sig->auth_ids[1])) {
pr_warn("Sig %u: X.509 chain contains auth-skid nonmatch (%u->%u)\n",
sinfo->index, x509->index, p->index);
return -EKEYREJECTED;
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 430848445dd9..09706bf112f3 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -48,15 +48,11 @@ struct x509_parse_context {
void x509_free_certificate(struct x509_certificate *cert)
{
if (cert) {
- public_key_free(cert->pub, NULL);
+ public_key_free(cert->pub, cert->sig);
kfree(cert->issuer);
kfree(cert->subject);
kfree(cert->id);
kfree(cert->skid);
- kfree(cert->akid_id);
- kfree(cert->akid_skid);
- kfree(cert->sig.digest);
- mpi_free(cert->sig.rsa.s);
kfree(cert);
}
}
@@ -79,6 +75,9 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
cert->pub = kzalloc(sizeof(struct public_key), GFP_KERNEL);
if (!cert->pub)
goto error_no_ctx;
+ cert->sig = kzalloc(sizeof(struct public_key_signature), GFP_KERNEL);
+ if (!cert->sig)
+ goto error_no_ctx;
ctx = kzalloc(sizeof(struct x509_parse_context), GFP_KERNEL);
if (!ctx)
goto error_no_ctx;
@@ -188,33 +187,33 @@ int x509_note_pkey_algo(void *context, size_t hdrlen,
return -ENOPKG; /* Unsupported combination */
case OID_md4WithRSAEncryption:
- ctx->cert->sig.pkey_hash_algo = HASH_ALGO_MD5;
- ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA;
+ ctx->cert->sig->pkey_hash_algo = HASH_ALGO_MD5;
+ ctx->cert->sig->pkey_algo = PKEY_ALGO_RSA;
break;
case OID_sha1WithRSAEncryption:
- ctx->cert->sig.pkey_hash_algo = HASH_ALGO_SHA1;
- ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA;
+ ctx->cert->sig->pkey_hash_algo = HASH_ALGO_SHA1;
+ ctx->cert->sig->pkey_algo = PKEY_ALGO_RSA;
break;
case OID_sha256WithRSAEncryption:
- ctx->cert->sig.pkey_hash_algo = HASH_ALGO_SHA256;
- ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA;
+ ctx->cert->sig->pkey_hash_algo = HASH_ALGO_SHA256;
+ ctx->cert->sig->pkey_algo = PKEY_ALGO_RSA;
break;
case OID_sha384WithRSAEncryption:
- ctx->cert->sig.pkey_hash_algo = HASH_ALGO_SHA384;
- ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA;
+ ctx->cert->sig->pkey_hash_algo = HASH_ALGO_SHA384;
+ ctx->cert->sig->pkey_algo = PKEY_ALGO_RSA;
break;
case OID_sha512WithRSAEncryption:
- ctx->cert->sig.pkey_hash_algo = HASH_ALGO_SHA512;
- ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA;
+ ctx->cert->sig->pkey_hash_algo = HASH_ALGO_SHA512;
+ ctx->cert->sig->pkey_algo = PKEY_ALGO_RSA;
break;
case OID_sha224WithRSAEncryption:
- ctx->cert->sig.pkey_hash_algo = HASH_ALGO_SHA224;
- ctx->cert->sig.pkey_algo = PKEY_ALGO_RSA;
+ ctx->cert->sig->pkey_hash_algo = HASH_ALGO_SHA224;
+ ctx->cert->sig->pkey_algo = PKEY_ALGO_RSA;
break;
}
@@ -550,7 +549,7 @@ int x509_decode_time(time64_t *_t, size_t hdrlen,
min < 0 || min > 59 ||
sec < 0 || sec > 59)
goto invalid_time;
-
+
*_t = mktime64(year, mon, day, hour, min, sec);
return 0;
@@ -593,14 +592,14 @@ int x509_akid_note_kid(void *context, size_t hdrlen,
pr_debug("AKID: keyid: %*phN\n", (int)vlen, value);
- if (ctx->cert->akid_skid)
+ if (ctx->cert->sig->auth_ids[1])
return 0;
kid = asymmetric_key_generate_id(value, vlen, "", 0);
if (IS_ERR(kid))
return PTR_ERR(kid);
pr_debug("authkeyid %*phN\n", kid->len, kid->data);
- ctx->cert->akid_skid = kid;
+ ctx->cert->sig->auth_ids[1] = kid;
return 0;
}
@@ -632,7 +631,7 @@ int x509_akid_note_serial(void *context, size_t hdrlen,
pr_debug("AKID: serial: %*phN\n", (int)vlen, value);
- if (!ctx->akid_raw_issuer || ctx->cert->akid_id)
+ if (!ctx->akid_raw_issuer || ctx->cert->sig->auth_ids[0])
return 0;
kid = asymmetric_key_generate_id(value,
@@ -643,6 +642,6 @@ int x509_akid_note_serial(void *context, size_t hdrlen,
return PTR_ERR(kid);
pr_debug("authkeyid %*phN\n", kid->len, kid->data);
- ctx->cert->akid_id = kid;
+ ctx->cert->sig->auth_ids[0] = kid;
return 0;
}
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 36b7c47335b5..63ff787c74b3 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -17,13 +17,11 @@ struct x509_certificate {
struct x509_certificate *next;
struct x509_certificate *signer; /* Certificate that signed this one */
struct public_key *pub; /* Public key details */
- struct public_key_signature sig; /* Signature parameters */
+ struct public_key_signature *sig; /* Signature parameters */
char *issuer; /* Name of certificate issuer */
char *subject; /* Name of certificate subject */
struct asymmetric_key_id *id; /* Issuer + Serial number */
struct asymmetric_key_id *skid; /* Subject + subjectKeyId (optional) */
- struct asymmetric_key_id *akid_id; /* CA AuthKeyId matching ->id (optional) */
- struct asymmetric_key_id *akid_skid; /* CA AuthKeyId matching ->skid (optional) */
time64_t valid_from;
time64_t valid_to;
const void *tbs; /* Signed data */
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 76c211b31da7..6b3711fa71d1 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -92,7 +92,7 @@ struct key *x509_request_asymmetric_key(struct key *keyring,
lookup = skid->data;
len = skid->len;
}
-
+
/* Construct an identifier "id:<keyid>". */
p = req = kmalloc(2 + 1 + len * 2 + 1, GFP_KERNEL);
if (!req)
@@ -141,7 +141,7 @@ struct key *x509_request_asymmetric_key(struct key *keyring,
goto reject;
}
}
-
+
pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key));
return key;
@@ -157,28 +157,28 @@ EXPORT_SYMBOL_GPL(x509_request_asymmetric_key);
*/
int x509_get_sig_params(struct x509_certificate *cert)
{
+ struct public_key_signature *sig = cert->sig;
struct crypto_shash *tfm;
struct shash_desc *desc;
- size_t digest_size, desc_size;
- void *digest;
+ size_t desc_size;
int ret;
pr_devel("==>%s()\n", __func__);
if (cert->unsupported_crypto)
return -ENOPKG;
- if (cert->sig.rsa.s)
+ if (sig->rsa.s)
return 0;
- cert->sig.rsa.s = mpi_read_raw_data(cert->raw_sig, cert->raw_sig_size);
- if (!cert->sig.rsa.s)
+ sig->rsa.s = mpi_read_raw_data(cert->raw_sig, cert->raw_sig_size);
+ if (!sig->rsa.s)
return -ENOMEM;
- cert->sig.nr_mpi = 1;
+ sig->nr_mpi = 1;
/* Allocate the hashing algorithm we're going to need and find out how
* big the hash operational data will be.
*/
- tfm = crypto_alloc_shash(hash_algo_name[cert->sig.pkey_hash_algo], 0, 0);
+ tfm = crypto_alloc_shash(hash_algo_name[sig->pkey_hash_algo], 0, 0);
if (IS_ERR(tfm)) {
if (PTR_ERR(tfm) == -ENOENT) {
cert->unsupported_crypto = true;
@@ -188,30 +188,29 @@ int x509_get_sig_params(struct x509_certificate *cert)
}
desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
- digest_size = crypto_shash_digestsize(tfm);
+ sig->digest_size = crypto_shash_digestsize(tfm);
- /* We allocate the hash operational data storage on the end of the
- * digest storage space.
- */
ret = -ENOMEM;
- digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
- if (!digest)
+ sig->digest = kmalloc(sig->digest_size, GFP_KERNEL);
+ if (!sig->digest)
goto error;
- cert->sig.digest = digest;
- cert->sig.digest_size = digest_size;
+ desc = kzalloc(desc_size, GFP_KERNEL);
+ if (!desc)
+ goto error;
- desc = digest + digest_size;
desc->tfm = tfm;
desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
ret = crypto_shash_init(desc);
if (ret < 0)
- goto error;
+ goto error_2;
might_sleep();
- ret = crypto_shash_finup(desc, cert->tbs, cert->tbs_size, digest);
-error:
+ ret = crypto_shash_finup(desc, cert->tbs, cert->tbs_size, sig->digest);
crypto_free_shash(tfm);
+error_2:
+ kfree(desc);
+error:
pr_devel("<==%s() = %d\n", __func__, ret);
return ret;
}
@@ -231,7 +230,7 @@ int x509_check_signature(const struct public_key *pub,
if (ret < 0)
return ret;
- ret = public_key_verify_signature(pub, &cert->sig);
+ ret = public_key_verify_signature(pub, cert->sig);
if (ret == -ENOPKG)
cert->unsupported_crypto = true;
pr_debug("Cert Verification: %d\n", ret);
@@ -251,17 +250,18 @@ EXPORT_SYMBOL_GPL(x509_check_signature);
static int x509_validate_trust(struct x509_certificate *cert,
struct key *trust_keyring)
{
+ struct public_key_signature *sig = cert->sig;
struct key *key;
int ret = 1;
if (!trust_keyring)
return -EOPNOTSUPP;
- if (ca_keyid && !asymmetric_key_id_partial(cert->akid_skid, ca_keyid))
+ if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
return -EPERM;
key = x509_request_asymmetric_key(trust_keyring,
- cert->akid_id, cert->akid_skid,
+ sig->auth_ids[0], sig->auth_ids[1],
false);
if (!IS_ERR(key)) {
if (!use_builtin_keys
@@ -293,11 +293,11 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
pr_devel("Cert Subject: %s\n", cert->subject);
if (cert->pub->pkey_algo >= PKEY_ALGO__LAST ||
- cert->sig.pkey_algo >= PKEY_ALGO__LAST ||
- cert->sig.pkey_hash_algo >= PKEY_HASH__LAST ||
+ cert->sig->pkey_algo >= PKEY_ALGO__LAST ||
+ cert->sig->pkey_hash_algo >= PKEY_HASH__LAST ||
!pkey_algo[cert->pub->pkey_algo] ||
- !pkey_algo[cert->sig.pkey_algo] ||
- !hash_algo_name[cert->sig.pkey_hash_algo]) {
+ !pkey_algo[cert->sig->pkey_algo] ||
+ !hash_algo_name[cert->sig->pkey_hash_algo]) {
ret = -ENOPKG;
goto error_free_cert;
}
@@ -305,16 +305,16 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
pr_devel("Cert Key Algo: %s\n", pkey_algo_name[cert->pub->pkey_algo]);
pr_devel("Cert Valid period: %lld-%lld\n", cert->valid_from, cert->valid_to);
pr_devel("Cert Signature: %s + %s\n",
- pkey_algo_name[cert->sig.pkey_algo],
- hash_algo_name[cert->sig.pkey_hash_algo]);
+ pkey_algo_name[cert->sig->pkey_algo],
+ hash_algo_name[cert->sig->pkey_hash_algo]);
cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
cert->pub->id_type = PKEY_ID_X509;
/* Check the signature on the key if it appears to be self-signed */
- if ((!cert->akid_skid && !cert->akid_id) ||
- asymmetric_key_id_same(cert->skid, cert->akid_skid) ||
- asymmetric_key_id_same(cert->id, cert->akid_id)) {
+ if ((!cert->sig->auth_ids[0] && !cert->sig->auth_ids[1]) ||
+ asymmetric_key_id_same(cert->skid, cert->sig->auth_ids[1]) ||
+ asymmetric_key_id_same(cert->id, cert->sig->auth_ids[0])) {
ret = x509_check_signature(cert->pub, cert); /* self-signed */
if (ret < 0)
goto error_free_cert;
@@ -356,6 +356,7 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
prep->payload.data[asym_subtype] = &public_key_subtype;
prep->payload.data[asym_key_ids] = kids;
prep->payload.data[asym_crypto] = cert->pub;
+ prep->payload.data[asym_auth] = cert->sig;
prep->description = desc;
prep->quotalen = 100;
@@ -363,6 +364,7 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
cert->pub = NULL;
cert->id = NULL;
cert->skid = NULL;
+ cert->sig = NULL;
desc = NULL;
ret = 0;
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 07/10] X.509: Extract signature digest and make self-signed cert checks earlier
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
` (5 preceding siblings ...)
2015-10-21 15:14 ` [PATCH 06/10] X.509: Retain the key verification data David Howells
@ 2015-10-21 15:14 ` David Howells
2015-10-21 15:14 ` [PATCH 08/10] PKCS#7: Make the signature a pointer rather than embedding it David Howells
` (3 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:14 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Extract the signature digest for an X.509 certificate earlier, at the end
of x509_cert_parse() rather than leaving it to the callers thereof.
Further, immediately after that, check the signature on self-signed
certificates, also rather in the callers of x509_cert_parse().
This we need to determine whether or not the X.509 cert requires crypto
that we don't support before we do the above two steps.
We note in the x509_certificate struct the following bits of information:
(1) Whether the signature is self-signed (even if we can't check the
signature due to missing crypto).
(2) Whether the key held in the certificate needs unsupported crypto to be
used. We may get a PKCS#7 message with X.509 certs that we can't make
use of - we just ignore them and give ENOPKG at the end it we couldn't
verify anything if at least one of these unusable certs are in the
chain of trust.
(3) Whether the signature held in the certificate needs unsupported crypto
to be checked. We can still use the key held in this certificate,
even if we can't check the signature on it - if it is held in the
system trusted keyring, for instance. We just can't add it to a ring
of trusted keys or follow it further up the chain of trust.
Making these checks earlier allows x509_check_signature() to be removed and
replaced with direct calls to public_key_verify_signature().
Signed-off-by: David Howells <dhowells@redhat.com>
---
crypto/asymmetric_keys/pkcs7_verify.c | 38 ++------
crypto/asymmetric_keys/x509_cert_parser.c | 10 ++
crypto/asymmetric_keys/x509_parser.h | 7 +
crypto/asymmetric_keys/x509_public_key.c | 139 ++++++++++++++++++++---------
4 files changed, 121 insertions(+), 73 deletions(-)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index e225dccdf559..1dede0199673 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -190,9 +190,8 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
x509->subject,
x509->raw_serial_size, x509->raw_serial);
x509->seen = true;
- ret = x509_get_sig_params(x509);
- if (ret < 0)
- goto maybe_missing_crypto_in_x509;
+ if (x509->unsupported_key)
+ goto unsupported_crypto_in_x509;
pr_debug("- issuer %s\n", x509->issuer);
sig = x509->sig;
@@ -203,22 +202,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
pr_debug("- authkeyid.skid %*phN\n",
sig->auth_ids[1]->len, sig->auth_ids[1]->data);
- if ((!x509->sig->auth_ids[0] && !x509->sig->auth_ids[1]) ||
- strcmp(x509->subject, x509->issuer) == 0) {
+ if (x509->self_signed) {
/* If there's no authority certificate specified, then
* the certificate must be self-signed and is the root
* of the chain. Likewise if the cert is its own
* authority.
*/
- pr_debug("- no auth?\n");
- if (x509->raw_subject_size != x509->raw_issuer_size ||
- memcmp(x509->raw_subject, x509->raw_issuer,
- x509->raw_issuer_size) != 0)
- return 0;
-
- ret = x509_check_signature(x509->pub, x509);
- if (ret < 0)
- goto maybe_missing_crypto_in_x509;
+ if (x509->unsupported_sig)
+ goto unsupported_crypto_in_x509;
x509->signer = x509;
pr_debug("- self-signed\n");
return 0;
@@ -270,7 +261,7 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
sinfo->index);
return 0;
}
- ret = x509_check_signature(p->pub, x509);
+ ret = public_key_verify_signature(p->pub, p->sig);
if (ret < 0)
return ret;
x509->signer = p;
@@ -282,16 +273,14 @@ static int pkcs7_verify_sig_chain(struct pkcs7_message *pkcs7,
might_sleep();
}
-maybe_missing_crypto_in_x509:
+unsupported_crypto_in_x509:
/* Just prune the certificate chain at this point if we lack some
* crypto module to go further. Note, however, we don't want to set
- * sinfo->missing_crypto as the signed info block may still be
+ * sinfo->unsupported_crypto as the signed info block may still be
* validatable against an X.509 cert lower in the chain that we have a
* trusted copy of.
*/
- if (ret == -ENOPKG)
- return 0;
- return ret;
+ return 0;
}
/*
@@ -378,9 +367,8 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
enum key_being_used_for usage)
{
struct pkcs7_signed_info *sinfo;
- struct x509_certificate *x509;
int enopkg = -ENOPKG;
- int ret, n;
+ int ret;
kenter("");
@@ -422,12 +410,6 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
return -EINVAL;
}
- for (n = 0, x509 = pkcs7->certs; x509; x509 = x509->next, n++) {
- ret = x509_get_sig_params(x509);
- if (ret < 0)
- return ret;
- }
-
for (sinfo = pkcs7->signed_infos; sinfo; sinfo = sinfo->next) {
ret = pkcs7_verify_one(pkcs7, sinfo);
if (ret < 0) {
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 09706bf112f3..f8cb4b01932a 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -108,6 +108,11 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
if (ret < 0)
goto error_decode;
+ /* Grab the signature bits */
+ ret = x509_get_sig_params(cert);
+ if (ret < 0)
+ goto error_decode;
+
/* Generate cert issuer + serial number key ID */
kid = asymmetric_key_generate_id(cert->raw_serial,
cert->raw_serial_size,
@@ -119,6 +124,11 @@ struct x509_certificate *x509_cert_parse(const void *data, size_t datalen)
}
cert->id = kid;
+ /* Detect self-signed certificates */
+ ret = x509_check_for_self_signed(cert);
+ if (ret < 0)
+ goto error_decode;
+
kfree(ctx);
return cert;
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 63ff787c74b3..05eef1c68881 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -39,7 +39,9 @@ struct x509_certificate {
unsigned index;
bool seen; /* Infinite recursion prevention */
bool verified;
- bool unsupported_crypto; /* T if can't be verified due to missing crypto */
+ bool self_signed; /* T if self-signed (check unsupported_sig too) */
+ bool unsupported_key; /* T if key uses unsupported crypto */
+ bool unsupported_sig; /* T if signature uses unsupported crypto */
};
/*
@@ -55,5 +57,4 @@ extern int x509_decode_time(time64_t *_t, size_t hdrlen,
* x509_public_key.c
*/
extern int x509_get_sig_params(struct x509_certificate *cert);
-extern int x509_check_signature(const struct public_key *pub,
- struct x509_certificate *cert);
+extern int x509_check_for_self_signed(struct x509_certificate *cert);
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 6b3711fa71d1..2b9e754e3ab9 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -165,10 +165,20 @@ int x509_get_sig_params(struct x509_certificate *cert)
pr_devel("==>%s()\n", __func__);
- if (cert->unsupported_crypto)
- return -ENOPKG;
- if (sig->rsa.s)
+ if (cert->pub->pkey_algo >= PKEY_ALGO__LAST ||
+ !pkey_algo[cert->pub->pkey_algo])
+ cert->unsupported_key = true;
+
+ if (sig->pkey_algo >= PKEY_ALGO__LAST ||
+ !pkey_algo[sig->pkey_algo])
+ cert->unsupported_sig = true;
+
+ /* We check the hash if we can - even if we can't then verify it */
+ if (sig->pkey_hash_algo >= PKEY_HASH__LAST ||
+ !hash_algo_name[sig->pkey_hash_algo]) {
+ cert->unsupported_sig = true;
return 0;
+ }
sig->rsa.s = mpi_read_raw_data(cert->raw_sig, cert->raw_sig_size);
if (!sig->rsa.s)
@@ -181,8 +191,8 @@ int x509_get_sig_params(struct x509_certificate *cert)
tfm = crypto_alloc_shash(hash_algo_name[sig->pkey_hash_algo], 0, 0);
if (IS_ERR(tfm)) {
if (PTR_ERR(tfm) == -ENOENT) {
- cert->unsupported_crypto = true;
- return -ENOPKG;
+ cert->unsupported_sig = true;
+ return 0;
}
return PTR_ERR(tfm);
}
@@ -217,26 +227,60 @@ error:
EXPORT_SYMBOL_GPL(x509_get_sig_params);
/*
- * Check the signature on a certificate using the provided public key
+ * Check for self-signedness in an X.509 cert and if found, check the signature
+ * immediately if we can.
*/
-int x509_check_signature(const struct public_key *pub,
- struct x509_certificate *cert)
+int x509_check_for_self_signed(struct x509_certificate *cert)
{
- int ret;
+ int ret = 0;
pr_devel("==>%s()\n", __func__);
- ret = x509_get_sig_params(cert);
- if (ret < 0)
- return ret;
+ if (cert->raw_subject_size != cert->raw_issuer_size ||
+ memcmp(cert->raw_subject, cert->raw_issuer,
+ cert->raw_issuer_size) != 0)
+ goto not_self_signed;
+
+ if (cert->sig->auth_ids[0] || cert->sig->auth_ids[1]) {
+ /* If the AKID is present it may have one or two parts. If
+ * both are supplied, both must match.
+ */
+ bool a = asymmetric_key_id_same(cert->skid, cert->sig->auth_ids[1]);
+ bool b = asymmetric_key_id_same(cert->id, cert->sig->auth_ids[0]);
+
+ if (!a && !b)
+ goto not_self_signed;
+
+ ret = -EKEYREJECTED;
+ if (((a && !b) || (b && !a)) &&
+ cert->sig->auth_ids[0] && cert->sig->auth_ids[1])
+ goto out;
+ }
+
+ ret = -EKEYREJECTED;
+ if (cert->pub->pkey_algo != cert->sig->pkey_algo)
+ goto out;
- ret = public_key_verify_signature(pub, cert->sig);
- if (ret == -ENOPKG)
- cert->unsupported_crypto = true;
- pr_debug("Cert Verification: %d\n", ret);
+ ret = public_key_verify_signature(cert->pub, cert->sig);
+ if (ret < 0) {
+ if (ret == -ENOPKG) {
+ cert->unsupported_sig = true;
+ ret = 0;
+ }
+ goto out;
+ }
+
+ pr_devel("Cert Self-signature verified");
+ cert->self_signed = true;
+
+out:
+ pr_devel("<==%s() = %d\n", __func__, ret);
return ret;
+
+not_self_signed:
+ pr_devel("<==%s() = 0 [not]\n", __func__);
+ return 0;
}
-EXPORT_SYMBOL_GPL(x509_check_signature);
/*
* Check the new certificate against the ones in the trust keyring. If one of
@@ -256,20 +300,25 @@ static int x509_validate_trust(struct x509_certificate *cert,
if (!trust_keyring)
return -EOPNOTSUPP;
-
if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
return -EPERM;
+ if (cert->unsupported_sig)
+ return -ENOPKG;
key = x509_request_asymmetric_key(trust_keyring,
sig->auth_ids[0], sig->auth_ids[1],
false);
- if (!IS_ERR(key)) {
- if (!use_builtin_keys
- || test_bit(KEY_FLAG_BUILTIN, &key->flags))
- ret = x509_check_signature(key->payload.data[asym_crypto],
- cert);
- key_put(key);
+ if (IS_ERR(key))
+ return PTR_ERR(key);
+
+ if (!use_builtin_keys ||
+ test_bit(KEY_FLAG_BUILTIN, &key->flags)) {
+ ret = public_key_verify_signature(
+ key->payload.data[asym_crypto], cert->sig);
+ if (ret == -ENOPKG)
+ cert->unsupported_sig = true;
}
+ key_put(key);
return ret;
}
@@ -292,36 +341,42 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
pr_devel("Cert Issuer: %s\n", cert->issuer);
pr_devel("Cert Subject: %s\n", cert->subject);
- if (cert->pub->pkey_algo >= PKEY_ALGO__LAST ||
- cert->sig->pkey_algo >= PKEY_ALGO__LAST ||
- cert->sig->pkey_hash_algo >= PKEY_HASH__LAST ||
- !pkey_algo[cert->pub->pkey_algo] ||
- !pkey_algo[cert->sig->pkey_algo] ||
- !hash_algo_name[cert->sig->pkey_hash_algo]) {
+ if (cert->unsupported_key) {
ret = -ENOPKG;
goto error_free_cert;
}
pr_devel("Cert Key Algo: %s\n", pkey_algo_name[cert->pub->pkey_algo]);
pr_devel("Cert Valid period: %lld-%lld\n", cert->valid_from, cert->valid_to);
- pr_devel("Cert Signature: %s + %s\n",
- pkey_algo_name[cert->sig->pkey_algo],
- hash_algo_name[cert->sig->pkey_hash_algo]);
cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
cert->pub->id_type = PKEY_ID_X509;
/* Check the signature on the key if it appears to be self-signed */
- if ((!cert->sig->auth_ids[0] && !cert->sig->auth_ids[1]) ||
- asymmetric_key_id_same(cert->skid, cert->sig->auth_ids[1]) ||
- asymmetric_key_id_same(cert->id, cert->sig->auth_ids[0])) {
- ret = x509_check_signature(cert->pub, cert); /* self-signed */
- if (ret < 0)
- goto error_free_cert;
+ if (cert->unsupported_sig) {
+ public_key_free(NULL, cert->sig);
+ cert->sig = NULL;
} else {
- ret = x509_validate_trust(cert, get_system_trusted_keyring());
- if (ret == -EKEYREJECTED)
- goto error_free_cert;
+ pr_devel("Cert Signature: %s + %s\n",
+ pkey_algo_name[cert->sig->pkey_algo],
+ hash_algo_name[cert->sig->pkey_hash_algo]);
+
+ if (cert->self_signed) {
+ /* Self-signed */
+ if (cert->unsupported_sig) {
+ ret = -ENOPKG;
+ goto error_free_cert;
+ }
+
+ /* There's no point retaining the signature */
+ public_key_free(NULL, cert->sig);
+ cert->sig = NULL;
+ } else {
+ ret = x509_validate_trust(cert,
+ get_system_trusted_keyring());
+ if (ret == -EKEYREJECTED)
+ goto error_free_cert;
+ }
}
/* Propose a description */
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 08/10] PKCS#7: Make the signature a pointer rather than embedding it
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
` (6 preceding siblings ...)
2015-10-21 15:14 ` [PATCH 07/10] X.509: Extract signature digest and make self-signed cert checks earlier David Howells
@ 2015-10-21 15:14 ` David Howells
2015-10-21 15:14 ` [PATCH 09/10] X.509: Move the trust validation code out to its own file David Howells
` (2 subsequent siblings)
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:14 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Point to the public_key_signature struct from the pkcs7_signed_info struct
rather than embedding it. This makes it easier to have it take an
arbitrary number of MPIs in future.
We also save a copy of the digest in the signature without sharing the
memory with the crypto layer metadata. This means we can use
public_key_free() to get rid of the signature record.
Signed-off-by: David Howells <dhowells@redhat.com>
---
crypto/asymmetric_keys/pkcs7_parser.c | 38 +++++++++++++++---------
crypto/asymmetric_keys/pkcs7_parser.h | 10 +++---
crypto/asymmetric_keys/pkcs7_trust.c | 4 +--
crypto/asymmetric_keys/pkcs7_verify.c | 52 +++++++++++++++++----------------
4 files changed, 56 insertions(+), 48 deletions(-)
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index 7b69783cff99..8454ae5b5aa8 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -44,9 +44,7 @@ struct pkcs7_parse_context {
static void pkcs7_free_signed_info(struct pkcs7_signed_info *sinfo)
{
if (sinfo) {
- mpi_free(sinfo->sig.mpi[0]);
- kfree(sinfo->sig.digest);
- kfree(sinfo->signing_cert_id);
+ public_key_free(NULL, sinfo->sig);
kfree(sinfo);
}
}
@@ -125,6 +123,10 @@ struct pkcs7_message *pkcs7_parse_message(const void *data, size_t datalen)
ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
if (!ctx->sinfo)
goto out_no_sinfo;
+ ctx->sinfo->sig = kzalloc(sizeof(struct public_key_signature),
+ GFP_KERNEL);
+ if (!ctx->sinfo->sig)
+ goto out_no_sig;
ctx->data = (unsigned long)data;
ctx->ppcerts = &ctx->certs;
@@ -150,6 +152,7 @@ out:
ctx->certs = cert->next;
x509_free_certificate(cert);
}
+out_no_sig:
pkcs7_free_signed_info(ctx->sinfo);
out_no_sinfo:
pkcs7_free_message(ctx->msg);
@@ -219,25 +222,25 @@ int pkcs7_sig_note_digest_algo(void *context, size_t hdrlen,
switch (ctx->last_oid) {
case OID_md4:
- ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_MD4;
+ ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_MD4;
break;
case OID_md5:
- ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_MD5;
+ ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_MD5;
break;
case OID_sha1:
- ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA1;
+ ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA1;
break;
case OID_sha256:
- ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA256;
+ ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA256;
break;
case OID_sha384:
- ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA384;
+ ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA384;
break;
case OID_sha512:
- ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA512;
+ ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA512;
break;
case OID_sha224:
- ctx->sinfo->sig.pkey_hash_algo = HASH_ALGO_SHA224;
+ ctx->sinfo->sig->pkey_hash_algo = HASH_ALGO_SHA224;
default:
printk("Unsupported digest algo: %u\n", ctx->last_oid);
return -ENOPKG;
@@ -256,7 +259,7 @@ int pkcs7_sig_note_pkey_algo(void *context, size_t hdrlen,
switch (ctx->last_oid) {
case OID_rsaEncryption:
- ctx->sinfo->sig.pkey_algo = PKEY_ALGO_RSA;
+ ctx->sinfo->sig->pkey_algo = PKEY_ALGO_RSA;
break;
default:
printk("Unsupported pkey algo: %u\n", ctx->last_oid);
@@ -617,16 +620,17 @@ int pkcs7_sig_note_signature(void *context, size_t hdrlen,
const void *value, size_t vlen)
{
struct pkcs7_parse_context *ctx = context;
+ struct public_key_signature *sig = ctx->sinfo->sig;
MPI mpi;
- BUG_ON(ctx->sinfo->sig.pkey_algo != PKEY_ALGO_RSA);
+ BUG_ON(sig->pkey_algo != PKEY_ALGO_RSA);
mpi = mpi_read_raw_data(value, vlen);
if (!mpi)
return -ENOMEM;
- ctx->sinfo->sig.mpi[0] = mpi;
- ctx->sinfo->sig.nr_mpi = 1;
+ sig->mpi[0] = mpi;
+ sig->nr_mpi = 1;
return 0;
}
@@ -662,12 +666,16 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
pr_devel("SINFO KID: %u [%*phN]\n", kid->len, kid->len, kid->data);
- sinfo->signing_cert_id = kid;
+ sinfo->sig->auth_ids[0] = kid;
sinfo->index = ++ctx->sinfo_index;
*ctx->ppsinfo = sinfo;
ctx->ppsinfo = &sinfo->next;
ctx->sinfo = kzalloc(sizeof(struct pkcs7_signed_info), GFP_KERNEL);
if (!ctx->sinfo)
return -ENOMEM;
+ ctx->sinfo->sig = kzalloc(sizeof(struct public_key_signature),
+ GFP_KERNEL);
+ if (!ctx->sinfo->sig)
+ return -ENOMEM;
return 0;
}
diff --git a/crypto/asymmetric_keys/pkcs7_parser.h b/crypto/asymmetric_keys/pkcs7_parser.h
index c8159983ed8f..f4e81074f5e0 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.h
+++ b/crypto/asymmetric_keys/pkcs7_parser.h
@@ -40,19 +40,17 @@ struct pkcs7_signed_info {
#define sinfo_has_ms_statement_type 5
time64_t signing_time;
- /* Issuing cert serial number and issuer's name [PKCS#7 or CMS ver 1]
- * or issuing cert's SKID [CMS ver 3].
- */
- struct asymmetric_key_id *signing_cert_id;
-
/* Message signature.
*
* This contains the generated digest of _either_ the Content Data or
* the Authenticated Attributes [RFC2315 9.3]. If the latter, one of
* the attributes contains the digest of the the Content Data within
* it.
+ *
+ * THis also contains the issuing cert serial number and issuer's name
+ * [PKCS#7 or CMS ver 1] or issuing cert's SKID [CMS ver 3].
*/
- struct public_key_signature sig;
+ struct public_key_signature *sig;
};
struct pkcs7_message {
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 7bb9389fd644..400ef359448a 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -27,7 +27,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
struct pkcs7_signed_info *sinfo,
struct key *trust_keyring)
{
- struct public_key_signature *sig = &sinfo->sig;
+ struct public_key_signature *sig = sinfo->sig;
struct x509_certificate *x509, *last = NULL, *p;
struct key *key;
int ret;
@@ -102,7 +102,7 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
* the signed info directly.
*/
key = x509_request_asymmetric_key(trust_keyring,
- sinfo->signing_cert_id,
+ sinfo->sig->auth_ids[0],
NULL,
false);
if (!IS_ERR(key)) {
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 1dede0199673..3b8124c2cd91 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -25,35 +25,38 @@
static int pkcs7_digest(struct pkcs7_message *pkcs7,
struct pkcs7_signed_info *sinfo)
{
+ struct public_key_signature *sig = sinfo->sig;
struct crypto_shash *tfm;
struct shash_desc *desc;
- size_t digest_size, desc_size;
- void *digest;
+ size_t desc_size;
int ret;
- kenter(",%u,%u", sinfo->index, sinfo->sig.pkey_hash_algo);
+ kenter(",%u,%u", sinfo->index, sig->pkey_hash_algo);
- if (sinfo->sig.pkey_hash_algo >= PKEY_HASH__LAST ||
- !hash_algo_name[sinfo->sig.pkey_hash_algo])
+ if (sig->pkey_hash_algo >= PKEY_HASH__LAST ||
+ !hash_algo_name[sig->pkey_hash_algo])
return -ENOPKG;
/* Allocate the hashing algorithm we're going to need and find out how
* big the hash operational data will be.
*/
- tfm = crypto_alloc_shash(hash_algo_name[sinfo->sig.pkey_hash_algo],
+ tfm = crypto_alloc_shash(hash_algo_name[sinfo->sig->pkey_hash_algo],
0, 0);
if (IS_ERR(tfm))
return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
- sinfo->sig.digest_size = digest_size = crypto_shash_digestsize(tfm);
+ sig->digest_size = crypto_shash_digestsize(tfm);
ret = -ENOMEM;
- digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
- if (!digest)
+ sig->digest = kmalloc(sig->digest_size, GFP_KERNEL);
+ if (!sig->digest)
+ goto error_no_desc;
+
+ desc = kzalloc(desc_size, GFP_KERNEL);
+ if (!desc)
goto error_no_desc;
- desc = digest + digest_size;
desc->tfm = tfm;
desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
@@ -61,10 +64,11 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
ret = crypto_shash_init(desc);
if (ret < 0)
goto error;
- ret = crypto_shash_finup(desc, pkcs7->data, pkcs7->data_len, digest);
+ ret = crypto_shash_finup(desc, pkcs7->data, pkcs7->data_len,
+ sig->digest);
if (ret < 0)
goto error;
- pr_devel("MsgDigest = [%*ph]\n", 8, digest);
+ pr_devel("MsgDigest = [%*ph]\n", 8, sig->digest);
/* However, if there are authenticated attributes, there must be a
* message digest attribute amongst them which corresponds to the
@@ -79,14 +83,15 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
goto error;
}
- if (sinfo->msgdigest_len != sinfo->sig.digest_size) {
+ if (sinfo->msgdigest_len != sig->digest_size) {
pr_debug("Sig %u: Invalid digest size (%u)\n",
sinfo->index, sinfo->msgdigest_len);
ret = -EBADMSG;
goto error;
}
- if (memcmp(digest, sinfo->msgdigest, sinfo->msgdigest_len) != 0) {
+ if (memcmp(sig->digest, sinfo->msgdigest,
+ sinfo->msgdigest_len) != 0) {
pr_debug("Sig %u: Message digest doesn't match\n",
sinfo->index);
ret = -EKEYREJECTED;
@@ -98,7 +103,7 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
* convert the attributes from a CONT.0 into a SET before we
* hash it.
*/
- memset(digest, 0, sinfo->sig.digest_size);
+ memset(sig->digest, 0, sig->digest_size);
ret = crypto_shash_init(desc);
if (ret < 0)
@@ -108,17 +113,14 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
if (ret < 0)
goto error;
ret = crypto_shash_finup(desc, sinfo->authattrs,
- sinfo->authattrs_len, digest);
+ sinfo->authattrs_len, sig->digest);
if (ret < 0)
goto error;
- pr_devel("AADigest = [%*ph]\n", 8, digest);
+ pr_devel("AADigest = [%*ph]\n", 8, sig->digest);
}
- sinfo->sig.digest = digest;
- digest = NULL;
-
error:
- kfree(digest);
+ kfree(desc);
error_no_desc:
crypto_free_shash(tfm);
kleave(" = %d", ret);
@@ -145,12 +147,12 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
* PKCS#7 message - but I can't be 100% sure of that. It's
* possible this will need element-by-element comparison.
*/
- if (!asymmetric_key_id_same(x509->id, sinfo->signing_cert_id))
+ if (!asymmetric_key_id_same(x509->id, sinfo->sig->auth_ids[0]))
continue;
pr_devel("Sig %u: Found cert serial match X.509[%u]\n",
sinfo->index, certix);
- if (x509->pub->pkey_algo != sinfo->sig.pkey_algo) {
+ if (x509->pub->pkey_algo != sinfo->sig->pkey_algo) {
pr_warn("Sig %u: X.509 algo and PKCS#7 sig algo don't match\n",
sinfo->index);
continue;
@@ -165,7 +167,7 @@ static int pkcs7_find_key(struct pkcs7_message *pkcs7,
*/
pr_debug("Sig %u: Issuing X.509 cert not found (#%*phN)\n",
sinfo->index,
- sinfo->signing_cert_id->len, sinfo->signing_cert_id->data);
+ sinfo->sig->auth_ids[0]->len, sinfo->sig->auth_ids[0]->data);
return 0;
}
@@ -324,7 +326,7 @@ static int pkcs7_verify_one(struct pkcs7_message *pkcs7,
}
/* Verify the PKCS#7 binary against the key */
- ret = public_key_verify_signature(sinfo->signer->pub, &sinfo->sig);
+ ret = public_key_verify_signature(sinfo->signer->pub, sinfo->sig);
if (ret < 0)
return ret;
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 09/10] X.509: Move the trust validation code out to its own file
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
` (7 preceding siblings ...)
2015-10-21 15:14 ` [PATCH 08/10] PKCS#7: Make the signature a pointer rather than embedding it David Howells
@ 2015-10-21 15:14 ` David Howells
2015-10-21 15:14 ` [PATCH 10/10] KEYS: Move the point of trust determination to __key_link() David Howells
2015-10-21 17:02 ` [PATCH 00/10] KEYS: Change how keys are determined to be trusted Mimi Zohar
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:14 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Move the X.509 trust validation code out to its own file so that it can be
generalised.
Signed-off-by: David Howells <dhowells@redhat.com>
---
crypto/asymmetric_keys/Makefile | 2
crypto/asymmetric_keys/public_key_trust.c | 192 +++++++++++++++++++++++++++++
crypto/asymmetric_keys/x509_parser.h | 6 +
crypto/asymmetric_keys/x509_public_key.c | 167 -------------------------
4 files changed, 199 insertions(+), 168 deletions(-)
create mode 100644 crypto/asymmetric_keys/public_key_trust.c
diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index cd1406f9b14a..bd07987c64e7 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -12,7 +12,7 @@ obj-$(CONFIG_PUBLIC_KEY_ALGO_RSA) += rsa.o
#
# X.509 Certificate handling
#
-obj-$(CONFIG_X509_CERTIFICATE_PARSER) += x509_key_parser.o
+obj-$(CONFIG_X509_CERTIFICATE_PARSER) += x509_key_parser.o public_key_trust.o
x509_key_parser-y := \
x509-asn1.o \
x509_akid-asn1.o \
diff --git a/crypto/asymmetric_keys/public_key_trust.c b/crypto/asymmetric_keys/public_key_trust.c
new file mode 100644
index 000000000000..753a413d479b
--- /dev/null
+++ b/crypto/asymmetric_keys/public_key_trust.c
@@ -0,0 +1,192 @@
+/* Instantiate a public key crypto key from an X.509 Certificate
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) "X.509: "fmt
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/mpi.h>
+#include <linux/asn1_decoder.h>
+#include <keys/asymmetric-subtype.h>
+#include <keys/asymmetric-parser.h>
+#include <keys/system_keyring.h>
+#include <crypto/hash.h>
+#include "asymmetric_keys.h"
+#include "public_key.h"
+#include "x509_parser.h"
+
+static bool use_builtin_keys;
+static struct asymmetric_key_id *ca_keyid;
+
+#ifndef MODULE
+static struct {
+ struct asymmetric_key_id id;
+ unsigned char data[10];
+} cakey;
+
+static int __init ca_keys_setup(char *str)
+{
+ if (!str) /* default system keyring */
+ return 1;
+
+ if (strncmp(str, "id:", 3) == 0) {
+ struct asymmetric_key_id *p = &cakey.id;
+ size_t hexlen = (strlen(str) - 3) / 2;
+ int ret;
+
+ if (hexlen == 0 || hexlen > sizeof(cakey.data)) {
+ pr_err("Missing or invalid ca_keys id\n");
+ return 1;
+ }
+
+ ret = __asymmetric_key_hex_to_key_id(str + 3, p, hexlen);
+ if (ret < 0)
+ pr_err("Unparsable ca_keys id hex string\n");
+ else
+ ca_keyid = p; /* owner key 'id:xxxxxx' */
+ } else if (strcmp(str, "builtin") == 0) {
+ use_builtin_keys = true;
+ }
+
+ return 1;
+}
+__setup("ca_keys=", ca_keys_setup);
+#endif
+
+/**
+ * x509_request_asymmetric_key - Request a key by X.509 certificate params.
+ * @keyring: The keys to search.
+ * @id: The issuer & serialNumber to look for or NULL.
+ * @skid: The subjectKeyIdentifier to look for or NULL.
+ * @partial: Use partial match if true, exact if false.
+ *
+ * Find a key in the given keyring by identifier. The preferred identifier is
+ * the issuer + serialNumber and the fallback identifier is the
+ * subjectKeyIdentifier. If both are given, the lookup is by the former, but
+ * the latter must also match.
+ */
+struct key *x509_request_asymmetric_key(struct key *keyring,
+ const struct asymmetric_key_id *id,
+ const struct asymmetric_key_id *skid,
+ bool partial)
+{
+ struct key *key;
+ key_ref_t ref;
+ const char *lookup;
+ char *req, *p;
+ int len;
+
+ if (id) {
+ lookup = id->data;
+ len = id->len;
+ } else {
+ lookup = skid->data;
+ len = skid->len;
+ }
+
+ /* Construct an identifier "id:<keyid>". */
+ p = req = kmalloc(2 + 1 + len * 2 + 1, GFP_KERNEL);
+ if (!req)
+ return ERR_PTR(-ENOMEM);
+
+ if (partial) {
+ *p++ = 'i';
+ *p++ = 'd';
+ } else {
+ *p++ = 'e';
+ *p++ = 'x';
+ }
+ *p++ = ':';
+ p = bin2hex(p, lookup, len);
+ *p = 0;
+
+ pr_debug("Look up: \"%s\"\n", req);
+
+ ref = keyring_search(make_key_ref(keyring, 1),
+ &key_type_asymmetric, req);
+ if (IS_ERR(ref))
+ pr_debug("Request for key '%s' err %ld\n", req, PTR_ERR(ref));
+ kfree(req);
+
+ if (IS_ERR(ref)) {
+ switch (PTR_ERR(ref)) {
+ /* Hide some search errors */
+ case -EACCES:
+ case -ENOTDIR:
+ case -EAGAIN:
+ return ERR_PTR(-ENOKEY);
+ default:
+ return ERR_CAST(ref);
+ }
+ }
+
+ key = key_ref_to_ptr(ref);
+ if (id && skid) {
+ const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
+ if (!kids->id[1]) {
+ pr_debug("issuer+serial match, but expected SKID missing\n");
+ goto reject;
+ }
+ if (!asymmetric_key_id_same(skid, kids->id[1])) {
+ pr_debug("issuer+serial match, but SKID does not\n");
+ goto reject;
+ }
+ }
+
+ pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key));
+ return key;
+
+reject:
+ key_put(key);
+ return ERR_PTR(-EKEYREJECTED);
+}
+EXPORT_SYMBOL_GPL(x509_request_asymmetric_key);
+
+/*
+ * Check the new certificate against the ones in the trust keyring. If one of
+ * those is the signing key and validates the new certificate, then mark the
+ * new certificate as being trusted.
+ *
+ * Return 0 if the new certificate was successfully validated, 1 if we couldn't
+ * find a matching parent certificate in the trusted list and an error if there
+ * is a matching certificate but the signature check fails.
+ */
+int x509_validate_trust(struct x509_certificate *cert,
+ struct key *trust_keyring)
+{
+ struct public_key_signature *sig = cert->sig;
+ struct key *key;
+ int ret = 1;
+
+ if (!trust_keyring)
+ return -EOPNOTSUPP;
+ if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
+ return -EPERM;
+ if (cert->unsupported_sig)
+ return -ENOPKG;
+
+ key = x509_request_asymmetric_key(trust_keyring,
+ sig->auth_ids[0], sig->auth_ids[1],
+ false);
+ if (IS_ERR(key))
+ return PTR_ERR(key);
+
+ if (!use_builtin_keys ||
+ test_bit(KEY_FLAG_BUILTIN, &key->flags)) {
+ ret = public_key_verify_signature(
+ key->payload.data[asym_crypto], cert->sig);
+ if (ret == -ENOPKG)
+ cert->unsupported_sig = true;
+ }
+ key_put(key);
+ return ret;
+}
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 05eef1c68881..7a802b09a509 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -58,3 +58,9 @@ extern int x509_decode_time(time64_t *_t, size_t hdrlen,
*/
extern int x509_get_sig_params(struct x509_certificate *cert);
extern int x509_check_for_self_signed(struct x509_certificate *cert);
+
+/*
+ * public_key_trust.c
+ */
+extern int x509_validate_trust(struct x509_certificate *cert,
+ struct key *trust_keyring);
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 2b9e754e3ab9..19f4abc762ee 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -24,133 +24,6 @@
#include "public_key.h"
#include "x509_parser.h"
-static bool use_builtin_keys;
-static struct asymmetric_key_id *ca_keyid;
-
-#ifndef MODULE
-static struct {
- struct asymmetric_key_id id;
- unsigned char data[10];
-} cakey;
-
-static int __init ca_keys_setup(char *str)
-{
- if (!str) /* default system keyring */
- return 1;
-
- if (strncmp(str, "id:", 3) == 0) {
- struct asymmetric_key_id *p = &cakey.id;
- size_t hexlen = (strlen(str) - 3) / 2;
- int ret;
-
- if (hexlen == 0 || hexlen > sizeof(cakey.data)) {
- pr_err("Missing or invalid ca_keys id\n");
- return 1;
- }
-
- ret = __asymmetric_key_hex_to_key_id(str + 3, p, hexlen);
- if (ret < 0)
- pr_err("Unparsable ca_keys id hex string\n");
- else
- ca_keyid = p; /* owner key 'id:xxxxxx' */
- } else if (strcmp(str, "builtin") == 0) {
- use_builtin_keys = true;
- }
-
- return 1;
-}
-__setup("ca_keys=", ca_keys_setup);
-#endif
-
-/**
- * x509_request_asymmetric_key - Request a key by X.509 certificate params.
- * @keyring: The keys to search.
- * @id: The issuer & serialNumber to look for or NULL.
- * @skid: The subjectKeyIdentifier to look for or NULL.
- * @partial: Use partial match if true, exact if false.
- *
- * Find a key in the given keyring by identifier. The preferred identifier is
- * the issuer + serialNumber and the fallback identifier is the
- * subjectKeyIdentifier. If both are given, the lookup is by the former, but
- * the latter must also match.
- */
-struct key *x509_request_asymmetric_key(struct key *keyring,
- const struct asymmetric_key_id *id,
- const struct asymmetric_key_id *skid,
- bool partial)
-{
- struct key *key;
- key_ref_t ref;
- const char *lookup;
- char *req, *p;
- int len;
-
- if (id) {
- lookup = id->data;
- len = id->len;
- } else {
- lookup = skid->data;
- len = skid->len;
- }
-
- /* Construct an identifier "id:<keyid>". */
- p = req = kmalloc(2 + 1 + len * 2 + 1, GFP_KERNEL);
- if (!req)
- return ERR_PTR(-ENOMEM);
-
- if (partial) {
- *p++ = 'i';
- *p++ = 'd';
- } else {
- *p++ = 'e';
- *p++ = 'x';
- }
- *p++ = ':';
- p = bin2hex(p, lookup, len);
- *p = 0;
-
- pr_debug("Look up: \"%s\"\n", req);
-
- ref = keyring_search(make_key_ref(keyring, 1),
- &key_type_asymmetric, req);
- if (IS_ERR(ref))
- pr_debug("Request for key '%s' err %ld\n", req, PTR_ERR(ref));
- kfree(req);
-
- if (IS_ERR(ref)) {
- switch (PTR_ERR(ref)) {
- /* Hide some search errors */
- case -EACCES:
- case -ENOTDIR:
- case -EAGAIN:
- return ERR_PTR(-ENOKEY);
- default:
- return ERR_CAST(ref);
- }
- }
-
- key = key_ref_to_ptr(ref);
- if (id && skid) {
- const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
- if (!kids->id[1]) {
- pr_debug("issuer+serial match, but expected SKID missing\n");
- goto reject;
- }
- if (!asymmetric_key_id_same(skid, kids->id[1])) {
- pr_debug("issuer+serial match, but SKID does not\n");
- goto reject;
- }
- }
-
- pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key));
- return key;
-
-reject:
- key_put(key);
- return ERR_PTR(-EKEYREJECTED);
-}
-EXPORT_SYMBOL_GPL(x509_request_asymmetric_key);
-
/*
* Set up the signature parameters in an X.509 certificate. This involves
* digesting the signed data and extracting the signature.
@@ -283,46 +156,6 @@ not_self_signed:
}
/*
- * Check the new certificate against the ones in the trust keyring. If one of
- * those is the signing key and validates the new certificate, then mark the
- * new certificate as being trusted.
- *
- * Return 0 if the new certificate was successfully validated, 1 if we couldn't
- * find a matching parent certificate in the trusted list and an error if there
- * is a matching certificate but the signature check fails.
- */
-static int x509_validate_trust(struct x509_certificate *cert,
- struct key *trust_keyring)
-{
- struct public_key_signature *sig = cert->sig;
- struct key *key;
- int ret = 1;
-
- if (!trust_keyring)
- return -EOPNOTSUPP;
- if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
- return -EPERM;
- if (cert->unsupported_sig)
- return -ENOPKG;
-
- key = x509_request_asymmetric_key(trust_keyring,
- sig->auth_ids[0], sig->auth_ids[1],
- false);
- if (IS_ERR(key))
- return PTR_ERR(key);
-
- if (!use_builtin_keys ||
- test_bit(KEY_FLAG_BUILTIN, &key->flags)) {
- ret = public_key_verify_signature(
- key->payload.data[asym_crypto], cert->sig);
- if (ret == -ENOPKG)
- cert->unsupported_sig = true;
- }
- key_put(key);
- return ret;
-}
-
-/*
* Attempt to parse a data blob for a key as an X509 certificate.
*/
static int x509_key_preparse(struct key_preparsed_payload *prep)
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 10/10] KEYS: Move the point of trust determination to __key_link()
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
` (8 preceding siblings ...)
2015-10-21 15:14 ` [PATCH 09/10] X.509: Move the trust validation code out to its own file David Howells
@ 2015-10-21 15:14 ` David Howells
2015-10-21 17:02 ` [PATCH 00/10] KEYS: Change how keys are determined to be trusted Mimi Zohar
10 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2015-10-21 15:14 UTC (permalink / raw)
To: keyrings; +Cc: dhowells, linux-security-module, linux-crypto, linux-kernel
Move the point at which a key is determined to be trustworthy to
__key_link() so that we use the contents of the keyring being linked in to
to determine whether the key being linked in is trusted or not.
What is 'trusted' then becomes a matter of what's in the keyring.
Currently, the test is done when the key is parsed, but given that at that
point we can only sensibly refer to the contents of the system trusted
keyring, we can only use that as the basis for working out the
trustworthiness of a new key.
With this change, a trusted keyring is a set of keys that once the
trusted-only flag is set cannot be added to except by verification through
one of the contained keys.
Further, adding a key into a trusted keyring, whilst it might grant
trustworthiness in the context of that keyring, does not automatically
grant trustworthiness in the context of a second keyring to which it could
be secondarily linked.
To accomplish this, the authentication data associated with the key source
must now be retained. For an X.509 cert, this means the contents of the
AuthorityKeyIdentifier and the signature data.
Signed-off-by: David Howells <dhowells@redhat.com>
---
certs/system_keyring.c | 3 +
crypto/asymmetric_keys/Makefile | 2 -
crypto/asymmetric_keys/asymmetric_keys.h | 2 +
crypto/asymmetric_keys/asymmetric_type.c | 15 +++++
crypto/asymmetric_keys/pkcs7_trust.c | 22 +++----
crypto/asymmetric_keys/public_key.c | 19 ++++++
crypto/asymmetric_keys/public_key.h | 6 ++
crypto/asymmetric_keys/public_key_trust.c | 94 +++++++++++++----------------
crypto/asymmetric_keys/x509_parser.h | 6 --
crypto/asymmetric_keys/x509_public_key.c | 6 --
include/crypto/public_key.h | 8 +-
include/keys/asymmetric-subtype.h | 4 +
security/integrity/digsig_asymmetric.c | 5 +-
13 files changed, 108 insertions(+), 84 deletions(-)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index e7f286413276..fbaaaea59f02 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -35,7 +35,8 @@ static __init int system_trusted_keyring_init(void)
keyring_alloc(".system_keyring",
KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
((KEY_POS_ALL & ~KEY_POS_SETATTR) |
- KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH),
+ KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH |
+ KEY_USR_WRITE),
KEY_ALLOC_NOT_IN_QUOTA, NULL);
if (IS_ERR(system_trusted_keyring))
panic("Can't allocate system trusted keyring\n");
diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index bd07987c64e7..69bcdc9a2ce6 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -6,7 +6,7 @@ obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += asymmetric_keys.o
asymmetric_keys-y := asymmetric_type.o signature.o
-obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o
+obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o public_key_trust.o
obj-$(CONFIG_PUBLIC_KEY_ALGO_RSA) += rsa.o
#
diff --git a/crypto/asymmetric_keys/asymmetric_keys.h b/crypto/asymmetric_keys/asymmetric_keys.h
index 1d450b580245..ca8e9ac34ce6 100644
--- a/crypto/asymmetric_keys/asymmetric_keys.h
+++ b/crypto/asymmetric_keys/asymmetric_keys.h
@@ -9,6 +9,8 @@
* 2 of the Licence, or (at your option) any later version.
*/
+#include <keys/asymmetric-type.h>
+
extern struct asymmetric_key_id *asymmetric_key_hex_to_key_id(const char *id);
extern int __asymmetric_key_hex_to_key_id(const char *id,
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index a79d30128821..e02cbd068151 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -362,10 +362,25 @@ static void asymmetric_key_destroy(struct key *key)
asymmetric_key_free_kids(kids);
}
+/*
+ * Verify the trust on an asymmetric key when added to a trusted-only keyring.
+ * The keyring provides a list of keys to check against.
+ */
+static int asymmetric_key_verify_trust(const union key_payload *payload,
+ struct key *keyring)
+{
+ struct asymmetric_key_subtype *subtype = payload->data[asym_subtype];
+
+ pr_devel("==>%s()\n", __func__);
+
+ return subtype->verify_trust(payload, keyring);
+}
+
struct key_type key_type_asymmetric = {
.name = "asymmetric",
.preparse = asymmetric_key_preparse,
.free_preparse = asymmetric_key_free_preparse,
+ .verify_trust = asymmetric_key_verify_trust,
.instantiate = generic_key_instantiate,
.match_preparse = asymmetric_key_match_preparse,
.match_free = asymmetric_key_match_free,
diff --git a/crypto/asymmetric_keys/pkcs7_trust.c b/crypto/asymmetric_keys/pkcs7_trust.c
index 400ef359448a..8760bc566902 100644
--- a/crypto/asymmetric_keys/pkcs7_trust.c
+++ b/crypto/asymmetric_keys/pkcs7_trust.c
@@ -51,9 +51,9 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
/* Look to see if this certificate is present in the trusted
* keys.
*/
- key = x509_request_asymmetric_key(trust_keyring,
- x509->id, x509->skid,
- false);
+ key = request_asymmetric_key(trust_keyring,
+ x509->id, x509->skid,
+ false);
if (!IS_ERR(key)) {
/* One of the X.509 certificates in the PKCS#7 message
* is apparently the same as one we already trust.
@@ -84,10 +84,10 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
* trusted keys.
*/
if (last && (last->sig->auth_ids[0] || last->sig->auth_ids[1])) {
- key = x509_request_asymmetric_key(trust_keyring,
- last->sig->auth_ids[0],
- last->sig->auth_ids[1],
- false);
+ key = request_asymmetric_key(trust_keyring,
+ last->sig->auth_ids[0],
+ last->sig->auth_ids[1],
+ false);
if (!IS_ERR(key)) {
x509 = last;
pr_devel("sinfo %u: Root cert %u signer is key %x\n",
@@ -101,10 +101,10 @@ static int pkcs7_validate_trust_one(struct pkcs7_message *pkcs7,
/* As a last resort, see if we have a trusted public key that matches
* the signed info directly.
*/
- key = x509_request_asymmetric_key(trust_keyring,
- sinfo->sig->auth_ids[0],
- NULL,
- false);
+ key = request_asymmetric_key(trust_keyring,
+ sinfo->sig->auth_ids[0],
+ NULL,
+ false);
if (!IS_ERR(key)) {
pr_devel("sinfo %u: Direct signer is key %x\n",
sinfo->index, key_serial(key));
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index f5b4824b7c77..7a6d2b3a7168 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -135,6 +135,24 @@ static int public_key_verify_signature_2(const struct key *key,
}
/*
+ * Verify the trust on an asymmetric key when added to a trusted-only keyring.
+ * The keyring provides a list of keys to check against.
+ */
+static int public_key_verify_trust(const union key_payload *payload,
+ struct key *keyring)
+{
+ const struct public_key_signature *sig = payload->data[asym_auth];
+ int ret;
+
+ pr_devel("==>%s()\n", __func__);
+
+ ret = public_key_validate_trust(sig, keyring);
+ if (ret == -ENOKEY)
+ ret = -EPERM;
+ return ret;
+}
+
+/*
* Public key algorithm asymmetric key subtype
*/
struct asymmetric_key_subtype public_key_subtype = {
@@ -143,6 +161,7 @@ struct asymmetric_key_subtype public_key_subtype = {
.name_len = sizeof("public_key") - 1,
.describe = public_key_describe,
.destroy = public_key_destroy,
+ .verify_trust = public_key_verify_trust,
.verify_signature = public_key_verify_signature_2,
};
EXPORT_SYMBOL_GPL(public_key_subtype);
diff --git a/crypto/asymmetric_keys/public_key.h b/crypto/asymmetric_keys/public_key.h
index 5c37a22a0637..2962025e1c09 100644
--- a/crypto/asymmetric_keys/public_key.h
+++ b/crypto/asymmetric_keys/public_key.h
@@ -34,3 +34,9 @@ extern const struct public_key_algorithm RSA_public_key_algorithm;
*/
extern int public_key_verify_signature(const struct public_key *pk,
const struct public_key_signature *sig);
+
+/*
+ * public_key_trust.c
+ */
+extern int public_key_validate_trust(const struct public_key_signature *sig,
+ struct key *trust_keyring);
diff --git a/crypto/asymmetric_keys/public_key_trust.c b/crypto/asymmetric_keys/public_key_trust.c
index 753a413d479b..285f9d6658d4 100644
--- a/crypto/asymmetric_keys/public_key_trust.c
+++ b/crypto/asymmetric_keys/public_key_trust.c
@@ -1,6 +1,6 @@
-/* Instantiate a public key crypto key from an X.509 Certificate
+/* Validate one public key against another to determine trust chaining.
*
- * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Copyright (C) 2015 Red Hat, Inc. All Rights Reserved.
* Written by David Howells (dhowells@redhat.com)
*
* This program is free software; you can redistribute it and/or
@@ -9,20 +9,12 @@
* 2 of the Licence, or (at your option) any later version.
*/
-#define pr_fmt(fmt) "X.509: "fmt
-#include <linux/module.h>
+#define pr_fmt(fmt) "PKEY: "fmt
#include <linux/kernel.h>
#include <linux/slab.h>
#include <linux/err.h>
-#include <linux/mpi.h>
-#include <linux/asn1_decoder.h>
-#include <keys/asymmetric-subtype.h>
-#include <keys/asymmetric-parser.h>
-#include <keys/system_keyring.h>
-#include <crypto/hash.h>
#include "asymmetric_keys.h"
#include "public_key.h"
-#include "x509_parser.h"
static bool use_builtin_keys;
static struct asymmetric_key_id *ca_keyid;
@@ -63,21 +55,20 @@ __setup("ca_keys=", ca_keys_setup);
#endif
/**
- * x509_request_asymmetric_key - Request a key by X.509 certificate params.
+ * request_asymmetric_key - Request a key by ID.
* @keyring: The keys to search.
- * @id: The issuer & serialNumber to look for or NULL.
- * @skid: The subjectKeyIdentifier to look for or NULL.
+ * @id_0: The first ID to look for or NULL.
+ * @id_1: The second ID to look for or NULL.
* @partial: Use partial match if true, exact if false.
*
* Find a key in the given keyring by identifier. The preferred identifier is
- * the issuer + serialNumber and the fallback identifier is the
- * subjectKeyIdentifier. If both are given, the lookup is by the former, but
- * the latter must also match.
+ * the id_0 and the fallback identifier is the id_1. If both are given, the
+ * lookup is by the former, but the latter must also match.
*/
-struct key *x509_request_asymmetric_key(struct key *keyring,
- const struct asymmetric_key_id *id,
- const struct asymmetric_key_id *skid,
- bool partial)
+struct key *request_asymmetric_key(struct key *keyring,
+ const struct asymmetric_key_id *id_0,
+ const struct asymmetric_key_id *id_1,
+ bool partial)
{
struct key *key;
key_ref_t ref;
@@ -85,12 +76,12 @@ struct key *x509_request_asymmetric_key(struct key *keyring,
char *req, *p;
int len;
- if (id) {
- lookup = id->data;
- len = id->len;
+ if (id_0) {
+ lookup = id_0->data;
+ len = id_0->len;
} else {
- lookup = skid->data;
- len = skid->len;
+ lookup = id_1->data;
+ len = id_1->len;
}
/* Construct an identifier "id:<keyid>". */
@@ -130,14 +121,15 @@ struct key *x509_request_asymmetric_key(struct key *keyring,
}
key = key_ref_to_ptr(ref);
- if (id && skid) {
+ if (id_0 && id_1) {
const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
- if (!kids->id[1]) {
- pr_debug("issuer+serial match, but expected SKID missing\n");
+
+ if (!kids->id[0]) {
+ pr_debug("First ID matches, but second is missing\n");
goto reject;
}
- if (!asymmetric_key_id_same(skid, kids->id[1])) {
- pr_debug("issuer+serial match, but SKID does not\n");
+ if (!asymmetric_key_id_same(id_1, kids->id[1])) {
+ pr_debug("First ID matches, but second does not\n");
goto reject;
}
}
@@ -149,44 +141,40 @@ reject:
key_put(key);
return ERR_PTR(-EKEYREJECTED);
}
-EXPORT_SYMBOL_GPL(x509_request_asymmetric_key);
+EXPORT_SYMBOL_GPL(request_asymmetric_key);
/*
* Check the new certificate against the ones in the trust keyring. If one of
* those is the signing key and validates the new certificate, then mark the
* new certificate as being trusted.
*
- * Return 0 if the new certificate was successfully validated, 1 if we couldn't
- * find a matching parent certificate in the trusted list and an error if there
- * is a matching certificate but the signature check fails.
+ * Return 0 if the new certificate was successfully validated, -ENOKEY if we
+ * couldn't find a matching parent certificate in the trusted list and some
+ * other error if there is a matching certificate but the signature check
+ * fails or cannot be performed.
*/
-int x509_validate_trust(struct x509_certificate *cert,
- struct key *trust_keyring)
+int public_key_validate_trust(const struct public_key_signature *sig,
+ struct key *trust_keyring)
{
- struct public_key_signature *sig = cert->sig;
struct key *key;
- int ret = 1;
+ int ret;
if (!trust_keyring)
return -EOPNOTSUPP;
if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
return -EPERM;
- if (cert->unsupported_sig)
- return -ENOPKG;
- key = x509_request_asymmetric_key(trust_keyring,
- sig->auth_ids[0], sig->auth_ids[1],
- false);
+ key = request_asymmetric_key(trust_keyring,
+ sig->auth_ids[0],
+ sig->auth_ids[1],
+ false);
if (IS_ERR(key))
- return PTR_ERR(key);
-
- if (!use_builtin_keys ||
- test_bit(KEY_FLAG_BUILTIN, &key->flags)) {
- ret = public_key_verify_signature(
- key->payload.data[asym_crypto], cert->sig);
- if (ret == -ENOPKG)
- cert->unsupported_sig = true;
- }
+ return -ENOKEY;
+
+ if (use_builtin_keys && !test_bit(KEY_FLAG_BUILTIN, &key->flags))
+ ret = -ENOKEY;
+ else
+ ret = verify_signature(key, sig);
key_put(key);
return ret;
}
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 7a802b09a509..05eef1c68881 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -58,9 +58,3 @@ extern int x509_decode_time(time64_t *_t, size_t hdrlen,
*/
extern int x509_get_sig_params(struct x509_certificate *cert);
extern int x509_check_for_self_signed(struct x509_certificate *cert);
-
-/*
- * public_key_trust.c
- */
-extern int x509_validate_trust(struct x509_certificate *cert,
- struct key *trust_keyring);
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 19f4abc762ee..df504f8ef783 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -97,7 +97,6 @@ error:
pr_devel("<==%s() = %d\n", __func__, ret);
return ret;
}
-EXPORT_SYMBOL_GPL(x509_get_sig_params);
/*
* Check for self-signedness in an X.509 cert and if found, check the signature
@@ -204,11 +203,6 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
/* There's no point retaining the signature */
public_key_free(NULL, cert->sig);
cert->sig = NULL;
- } else {
- ret = x509_validate_trust(cert,
- get_system_trusted_keyring());
- if (ret == -EKEYREJECTED)
- goto error_free_cert;
}
}
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index ed86bfb23e89..eaaf261d398a 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -102,9 +102,9 @@ extern int verify_signature(const struct key *key,
const struct public_key_signature *sig);
struct asymmetric_key_id;
-extern struct key *x509_request_asymmetric_key(struct key *keyring,
- const struct asymmetric_key_id *id,
- const struct asymmetric_key_id *skid,
- bool partial);
+extern struct key *request_asymmetric_key(struct key *keyring,
+ const struct asymmetric_key_id *id_0,
+ const struct asymmetric_key_id *id_1,
+ bool partial);
#endif /* _LINUX_PUBLIC_KEY_H */
diff --git a/include/keys/asymmetric-subtype.h b/include/keys/asymmetric-subtype.h
index 2480469ce8fb..5c8d056d163a 100644
--- a/include/keys/asymmetric-subtype.h
+++ b/include/keys/asymmetric-subtype.h
@@ -34,6 +34,10 @@ struct asymmetric_key_subtype {
/* Destroy a key of this subtype */
void (*destroy)(void *payload_crypto, void *payload_auth);
+ /* Verify the trust on a key against a keyring of keys */
+ int (*verify_trust)(const union key_payload *payload,
+ struct key *keyring);
+
/* Verify the signature on a key of this subtype (optional) */
int (*verify_signature)(const struct key *key,
const struct public_key_signature *sig);
diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
index 4fec1816a2b3..668e054e534b 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -23,7 +23,8 @@
/*
* Request an asymmetric key.
*/
-static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
+static struct key *ds_request_asymmetric_key(struct key *keyring,
+ uint32_t keyid)
{
struct key *key;
char name[12];
@@ -83,7 +84,7 @@ int asymmetric_verify(struct key *keyring, const char *sig,
if (hdr->hash_algo >= PKEY_HASH__LAST)
return -ENOPKG;
- key = request_asymmetric_key(keyring, __be32_to_cpu(hdr->keyid));
+ key = ds_request_asymmetric_key(keyring, __be32_to_cpu(hdr->keyid));
if (IS_ERR(key))
return PTR_ERR(key);
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 00/10] KEYS: Change how keys are determined to be trusted
2015-10-21 15:13 [PATCH 00/10] KEYS: Change how keys are determined to be trusted David Howells
` (9 preceding siblings ...)
2015-10-21 15:14 ` [PATCH 10/10] KEYS: Move the point of trust determination to __key_link() David Howells
@ 2015-10-21 17:02 ` Mimi Zohar
2015-10-21 17:21 ` Josh Boyer
2015-10-21 18:48 ` Petko Manolov
10 siblings, 2 replies; 17+ messages in thread
From: Mimi Zohar @ 2015-10-21 17:02 UTC (permalink / raw)
To: David Howells; +Cc: keyrings, linux-security-module, linux-crypto, linux-kernel
On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
> Here's a set of patches that changes how keys are determined to be trusted
> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
> it. A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
> indicates that only keys with this flag set may be added to that keyring.
>
> Further, any time an X.509 certificate is instantiated without this flag
> set, the certificate is judged against the contents of the system trusted
> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
>
> With these patches, KEY_FLAG_TRUSTED is removed. The kernel may add
> implicitly trusted keys to a trusted-only keyring by asserting
> KEY_ALLOC_TRUSTED when the key is created,
Ok, but only the x509 certificates built into the kernel image should be
automatically trusted and can be added to a trusted keyring, because the
kernel itself was signed (and verified). These certificates extend the
(UEFI) certificate chain of trust that is rooted in hardware to the OS.
Other keys that the kernel reads and loads should not automatically be
trusted (eg. ima_load_x509). They need to be validated against a
trusted key.
> but otherwise the key will only
> be allowed to be added to the keyring if it can be verified by a key
> already in that keyring. The system trusted keyring is not then special in
> this sense and other trusted keyrings can be set up that are wholly
> independent of it.
We already went down this path of "transitive trust" back when we first
introduced the concept of trusted keys and keyrings. Just because a
key is on a trusted keyring, doesn't imply that it should be permitted
to load other keys on the same trusted keyring. In the case of
IMA-appraisal, the key should only be used to verify the file data
signature, not other keys.
The trusted keys used for verifying other certificates should be stored
on a separate keyring, not the target keyring. Petko's patches define
a new IMA keyring named .ima_mok for this purpose.
Mimi
> To make this work, we have to retain sufficient data from the X.509
> certificate that we can then verify the signature at need.
>
> The patches can be found here also:
>
> http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-trust
>
> and are tagged with:
>
> keys-trust-20151021
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 00/10] KEYS: Change how keys are determined to be trusted
2015-10-21 17:02 ` [PATCH 00/10] KEYS: Change how keys are determined to be trusted Mimi Zohar
@ 2015-10-21 17:21 ` Josh Boyer
2015-10-21 18:11 ` Mimi Zohar
2015-10-21 18:48 ` Petko Manolov
1 sibling, 1 reply; 17+ messages in thread
From: Josh Boyer @ 2015-10-21 17:21 UTC (permalink / raw)
To: Mimi Zohar
Cc: David Howells, keyrings, linux-security-module, linux-crypto,
Linux-Kernel@Vger. Kernel. Org
On Wed, Oct 21, 2015 at 1:02 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
>> Here's a set of patches that changes how keys are determined to be trusted
>> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
>> it. A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
>> indicates that only keys with this flag set may be added to that keyring.
>>
>> Further, any time an X.509 certificate is instantiated without this flag
>> set, the certificate is judged against the contents of the system trusted
>> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
>>
>> With these patches, KEY_FLAG_TRUSTED is removed. The kernel may add
>> implicitly trusted keys to a trusted-only keyring by asserting
>> KEY_ALLOC_TRUSTED when the key is created,
>
> Ok, but only the x509 certificates built into the kernel image should be
> automatically trusted and can be added to a trusted keyring, because the
> kernel itself was signed (and verified). These certificates extend the
> (UEFI) certificate chain of trust that is rooted in hardware to the OS.
That doesn't sound accurate to me. The cert built into the kernel
image doesn't extend the UEFI certificates. In most cases, it is a
ephemeral cert that is automatically generated at kernel build time
and then discarded. It is not chained to or derived from any of the
UEFI certs stored in the db (or mok) variables. The built-in cert is
used for module loading verification. I agree that it should be
trusted, but not really for the reason you list. Perhaps you meant
the key that the PE image of the kernel is signed with? If so, the
kernel doesn't load that. Only shim (and grub2 via shim) read that
key.
However, that does bring up the UEFI db/mok certs and how to deal with
those. The out-of-tree patches we have add them to the system keyring
as trusted keys. We can modify the patches to use KEY_ALLOC_TRUSTED
to preserve that functionality I suppose.
josh
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 00/10] KEYS: Change how keys are determined to be trusted
2015-10-21 17:21 ` Josh Boyer
@ 2015-10-21 18:11 ` Mimi Zohar
2015-10-21 18:21 ` Josh Boyer
0 siblings, 1 reply; 17+ messages in thread
From: Mimi Zohar @ 2015-10-21 18:11 UTC (permalink / raw)
To: Josh Boyer
Cc: David Howells, keyrings, linux-security-module, linux-crypto,
Linux-Kernel@Vger. Kernel. Org
On Wed, 2015-10-21 at 13:21 -0400, Josh Boyer wrote:
> On Wed, Oct 21, 2015 at 1:02 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
> >> Here's a set of patches that changes how keys are determined to be trusted
> >> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
> >> it. A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
> >> indicates that only keys with this flag set may be added to that keyring.
> >>
> >> Further, any time an X.509 certificate is instantiated without this flag
> >> set, the certificate is judged against the contents of the system trusted
> >> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
> >>
> >> With these patches, KEY_FLAG_TRUSTED is removed. The kernel may add
> >> implicitly trusted keys to a trusted-only keyring by asserting
> >> KEY_ALLOC_TRUSTED when the key is created,
> >
> > Ok, but only the x509 certificates built into the kernel image should be
> > automatically trusted and can be added to a trusted keyring, because the
> > kernel itself was signed (and verified). These certificates extend the
> > (UEFI) certificate chain of trust that is rooted in hardware to the OS.
>
> That doesn't sound accurate to me. The cert built into the kernel
> image doesn't extend the UEFI certificates. In most cases, it is a
> ephemeral cert that is automatically generated at kernel build time
> and then discarded. It is not chained to or derived from any of the
> UEFI certs stored in the db (or mok) variables. The built-in cert is
> used for module loading verification. I agree that it should be
> trusted, but not really for the reason you list. Perhaps you meant
> the key that the PE image of the kernel is signed with? If so, the
> kernel doesn't load that. Only shim (and grub2 via shim) read that
> key.
This is similar to the concept of the MoK DB. Keys added to the MoK
aren't signed by a UEFI key, yet they extend the UEFI secure boot
certificate chain of trust. Similarly, the certificates built into the
kernel image don't need to be signed by a UEFI/MoK key for it to extend
the certificate chain of trust.
> However, that does bring up the UEFI db/mok certs and how to deal with
> those. The out-of-tree patches we have add them to the system keyring
> as trusted keys. We can modify the patches to use KEY_ALLOC_TRUSTED
> to preserve that functionality I suppose.
Certificates are use case specific. Just because a key was trusted at
the UEFI layer doesn't mean it should be trusted by the kernel (eg.
Microsoft key). To illustrate this point, David Howells/David Woodhouse
recently posted/upstreamed patches to differentiate how keys loaded onto
the system keyring may be used. (Reference needed.)
Mimi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 00/10] KEYS: Change how keys are determined to be trusted
2015-10-21 18:11 ` Mimi Zohar
@ 2015-10-21 18:21 ` Josh Boyer
2015-10-21 18:33 ` Mimi Zohar
0 siblings, 1 reply; 17+ messages in thread
From: Josh Boyer @ 2015-10-21 18:21 UTC (permalink / raw)
To: Mimi Zohar
Cc: David Howells, keyrings, linux-security-module, linux-crypto,
Linux-Kernel@Vger. Kernel. Org
On Wed, Oct 21, 2015 at 2:11 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2015-10-21 at 13:21 -0400, Josh Boyer wrote:
>> On Wed, Oct 21, 2015 at 1:02 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
>> >> Here's a set of patches that changes how keys are determined to be trusted
>> >> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
>> >> it. A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
>> >> indicates that only keys with this flag set may be added to that keyring.
>> >>
>> >> Further, any time an X.509 certificate is instantiated without this flag
>> >> set, the certificate is judged against the contents of the system trusted
>> >> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
>> >>
>> >> With these patches, KEY_FLAG_TRUSTED is removed. The kernel may add
>> >> implicitly trusted keys to a trusted-only keyring by asserting
>> >> KEY_ALLOC_TRUSTED when the key is created,
>> >
>> > Ok, but only the x509 certificates built into the kernel image should be
>> > automatically trusted and can be added to a trusted keyring, because the
>> > kernel itself was signed (and verified). These certificates extend the
>> > (UEFI) certificate chain of trust that is rooted in hardware to the OS.
>>
>> That doesn't sound accurate to me. The cert built into the kernel
>> image doesn't extend the UEFI certificates. In most cases, it is a
>> ephemeral cert that is automatically generated at kernel build time
>> and then discarded. It is not chained to or derived from any of the
>> UEFI certs stored in the db (or mok) variables. The built-in cert is
>> used for module loading verification. I agree that it should be
>> trusted, but not really for the reason you list. Perhaps you meant
>> the key that the PE image of the kernel is signed with? If so, the
>> kernel doesn't load that. Only shim (and grub2 via shim) read that
>> key.
>
> This is similar to the concept of the MoK DB. Keys added to the MoK
> aren't signed by a UEFI key, yet they extend the UEFI secure boot
> certificate chain of trust. Similarly, the certificates built into the
Right, because UEFI is verifying shim, which verifies grub2, which
verifies the kernel. I get that. However, it's irrelevant.
> kernel image don't need to be signed by a UEFI/MoK key for it to extend
> the certificate chain of trust.
The certificates built _into_ the kernel need to be trusted in all
cases. It is how module signing is done. So a user not using Secure
Boot, or even not using UEFI, still needs those embedded certs trusted
so that they can load modules. It has nothing to do with UEFI or some
single-root-of-trust.
At any rate, I believe we are both saying the embedded cert needs to
be trusted so there's little point in debating further. I just wanted
to point out that this need has nothing to do with UEFI.
josh
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 00/10] KEYS: Change how keys are determined to be trusted
2015-10-21 18:21 ` Josh Boyer
@ 2015-10-21 18:33 ` Mimi Zohar
0 siblings, 0 replies; 17+ messages in thread
From: Mimi Zohar @ 2015-10-21 18:33 UTC (permalink / raw)
To: Josh Boyer
Cc: David Howells, keyrings, linux-security-module, linux-crypto,
Linux-Kernel@Vger. Kernel. Org
On Wed, 2015-10-21 at 14:21 -0400, Josh Boyer wrote:
> On Wed, Oct 21, 2015 at 2:11 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2015-10-21 at 13:21 -0400, Josh Boyer wrote:
> >> On Wed, Oct 21, 2015 at 1:02 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> > On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
> >> >> Here's a set of patches that changes how keys are determined to be trusted
> >> >> - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
> >> >> it. A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
> >> >> indicates that only keys with this flag set may be added to that keyring.
> >> >>
> >> >> Further, any time an X.509 certificate is instantiated without this flag
> >> >> set, the certificate is judged against the contents of the system trusted
> >> >> keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
> >> >>
> >> >> With these patches, KEY_FLAG_TRUSTED is removed. The kernel may add
> >> >> implicitly trusted keys to a trusted-only keyring by asserting
> >> >> KEY_ALLOC_TRUSTED when the key is created,
> >> >
> >> > Ok, but only the x509 certificates built into the kernel image should be
> >> > automatically trusted and can be added to a trusted keyring, because the
> >> > kernel itself was signed (and verified). These certificates extend the
> >> > (UEFI) certificate chain of trust that is rooted in hardware to the OS.
> >>
> >> That doesn't sound accurate to me. The cert built into the kernel
> >> image doesn't extend the UEFI certificates. In most cases, it is a
> >> ephemeral cert that is automatically generated at kernel build time
> >> and then discarded. It is not chained to or derived from any of the
> >> UEFI certs stored in the db (or mok) variables. The built-in cert is
> >> used for module loading verification. I agree that it should be
> >> trusted, but not really for the reason you list. Perhaps you meant
> >> the key that the PE image of the kernel is signed with? If so, the
> >> kernel doesn't load that. Only shim (and grub2 via shim) read that
> >> key.
> >
> > This is similar to the concept of the MoK DB. Keys added to the MoK
> > aren't signed by a UEFI key, yet they extend the UEFI secure boot
> > certificate chain of trust. Similarly, the certificates built into the
>
> Right, because UEFI is verifying shim, which verifies grub2, which
> verifies the kernel. I get that. However, it's irrelevant.
>
> > kernel image don't need to be signed by a UEFI/MoK key for it to extend
> > the certificate chain of trust.
>
> The certificates built _into_ the kernel need to be trusted in all
> cases. It is how module signing is done. So a user not using Secure
> Boot, or even not using UEFI, still needs those embedded certs trusted
> so that they can load modules. It has nothing to do with UEFI or some
> single-root-of-trust.
>
> At any rate, I believe we are both saying the embedded cert needs to
> be trusted so there's little point in debating further. I just wanted
> to point out that this need has nothing to do with UEFI.
Right, the embedded certs need to trusted. But that trust needs to be
based on something. One method of establishing that trust is (UEFI)
secure boot, which verifies the kernel image signature, including the
embedded certs.
Mimi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 00/10] KEYS: Change how keys are determined to be trusted
2015-10-21 17:02 ` [PATCH 00/10] KEYS: Change how keys are determined to be trusted Mimi Zohar
2015-10-21 17:21 ` Josh Boyer
@ 2015-10-21 18:48 ` Petko Manolov
1 sibling, 0 replies; 17+ messages in thread
From: Petko Manolov @ 2015-10-21 18:48 UTC (permalink / raw)
To: Mimi Zohar
Cc: David Howells, keyrings, linux-security-module, linux-crypto,
linux-kernel, mdb
On 15-10-21 13:02:48, Mimi Zohar wrote:
> On Wed, 2015-10-21 at 16:13 +0100, David Howells wrote:
> > Here's a set of patches that changes how keys are determined to be trusted
> > - currently, that's a case of whether a key has KEY_FLAG_TRUSTED set upon
> > it. A keyring can then have a flag set (KEY_FLAG_TRUSTED ONLY) that
> > indicates that only keys with this flag set may be added to that keyring.
> >
> > Further, any time an X.509 certificate is instantiated without this flag
> > set, the certificate is judged against the contents of the system trusted
> > keyring to determine whether KEY_FLAG_TRUSTED should be set upon it.
> >
> > With these patches, KEY_FLAG_TRUSTED is removed. The kernel may add
> > implicitly trusted keys to a trusted-only keyring by asserting
> > KEY_ALLOC_TRUSTED when the key is created,
>
> Ok, but only the x509 certificates built into the kernel image should be
> automatically trusted and can be added to a trusted keyring, because the
> kernel itself was signed (and verified). These certificates extend the
> (UEFI) certificate chain of trust that is rooted in hardware to the OS.
>
> Other keys that the kernel reads and loads should not automatically be
> trusted (eg. ima_load_x509). They need to be validated against a
> trusted key.
>
> > but otherwise the key will only
> > be allowed to be added to the keyring if it can be verified by a key
> > already in that keyring. The system trusted keyring is not then special in
> > this sense and other trusted keyrings can be set up that are wholly
> > independent of it.
>
> We already went down this path of "transitive trust" back when we first
> introduced the concept of trusted keys and keyrings. Just because a key is on
> a trusted keyring, doesn't imply that it should be permitted to load other
> keys on the same trusted keyring. In the case of IMA-appraisal, the key
> should only be used to verify the file data signature, not other keys.
>
> The trusted keys used for verifying other certificates should be stored on a
> separate keyring, not the target keyring. Petko's patches define a new IMA
> keyring named .ima_mok for this purpose.
The concept is not new. Some embedded applications are multi-tenant and
typically have uptime measured in years. The current CA hierarchy model of the
kernel is somewhat limited in terms of dynamically adding trusted certificates
and trusted keys.
.ima_mok was introduced as an intermediate keyring storing CAs that are
themselves signed by CAs in the system keyring, which is trusted by default.
Only keys that have been signed by certificate in .system or .ima_mok may land
in .ima keyring. This:
.system ---> .ima_mok ---> .ima ---> actual.key
gives us the ability to extend the chain of trust and also cover the above
criteria. That said, .ima_mok may be used for a whole bunch of other cases.
Think of a kernel module that comes from one of the tenants or even the machine
owner. They obviously don't have access to the Manufacturer's signing key
(CA-M), but do have certificate (CA-O) that has been signed by it (CA-M).
This certificate (CA-O) can now go to .ima_mok (or whatever the name) and
successfully verify the kernel's module signature. CA-O may even sign another
certificate, CA-O2, and by the above rules it may also go into .ima_mok. And so
on...
I think that in general having an intermediate CA keyring adds a lot of
flexibility to the kernel's key management, although it's typical use does not
make this mandatory.
cheers,
Petko
^ permalink raw reply [flat|nested] 17+ messages in thread