KEYS: Restrict additions to 'trusted' keyrings [ver #3]
Subsystems (12)
David Howells
maintainercover KEYS: Restrict additions to 'trusted' keyrings [ver #3]
Here's a set of patches that changes how certificates/keys are determined to be trusted. That's currently a two-step process:
(1) Up until recently, when an X.509 certificate was parsed - no matter the source - it was judged against the keys in .system_keyring, assuming those keys to be trusted if they have KEY_FLAG_TRUSTED set upon them.
This has just been changed such that any key in the .ima_mok keyring, if configured, may also be used to judge the trustworthiness of a new certificate, whether or not the .ima_mok keyring is meant to be consulted for whatever process is being undertaken.
If a certificate is determined to be trustworthy, KEY_FLAG_TRUSTED will be set upon a key it is loaded into (if it is loaded into one), no matter what the key is going to be loaded for.
(2) If an X.509 certificate is loaded into a key, then that key - if KEY_FLAG_TRUSTED gets set upon it - can be linked into any keyring with KEY_FLAG_TRUSTED_ONLY set upon it. This was meant to be the system keyring only, but has been extended to various IMA keyrings.
A user can at will link any key marked KEY_FLAG_TRUSTED into any keyring marked KEY_FLAG_TRUSTED_ONLY if the relevant permissions masks permit it.
These patches change that:
(1) Trust becomes a matter of consulting the ring of trusted keys supplied when the trust is evaluated only.
(3) Every keyring can be supplied with its own manager function to restrict what may be added to that keyring. This is called whenever a key is to be linked into the keyring to guard against a key being created in one keyring and then linked across.
This function is supplied with the keyring and the key type and payload[*] of the key being linked in for use in its evaluation. It is permitted to use other data also, such as the contents of other keyrings such as the system keyrings.
[*] The type and payload are supplied instead of a key because as an
optimisation this function may be called whilst creating a key and so may reject the proposed key between preparse and allocation.
(4) A default manager function is provided that permits keys to be restricted to only asymmetric keys that are vouched for by the contents of the system keyring.
A second manager function is provided that just rejects with EPERM.
(5) A key allocation flag, KEY_ALLOC_BYPASS_RESTRICTION, is made available so that the kernel can initialise keyrings with keys that form the root of the trust relationship.
(6) KEY_FLAG_TRUSTED and KEY_FLAG_TRUSTED_ONLY are removed, along with key_preparsed_payload::trusted.
This change also makes it possible in future for userspace to create a private set of trusted keys and then to have it sealed by setting a manager function where the private set is wholly independent of the kernel's trust relationships.
Further changes in the set involve extracting certain IMA special keyrings and making them generally global:
(*) .system_keyring is renamed to .builtin_trusted_keys and remains read
only. It carries only keys built in to the kernel. It may be where UEFI keys should be loaded - though that could better be the new secondary keyring (see below) or a separate UEFI keyring.
(*) An optional secondary system keyring (called .secondary_trusted_keys)
is added to replace the IMA MOK keyring.
(*) Keys can be added to the secondary keyring by root if the keys can
be vouched for by either ring of system keys.
(*) Module signing and kexec only use .builtin_trusted_keys and do not use
the new secondary keyring.
(*) Config option SYSTEM_TRUSTED_KEYS now depends on ASYMMETRIC_KEY_TYPE as
that's the only type currently permitted on the system keyrings.
(*) A new config option, IMA_PERMIT_ADD_TO_IMA_KEYRINGS, is provided to allow
keys to be added to IMA keyrings - subject to the restriction that such keys are validly signed by a key already in the system keyrings.
If IMA_PERMIT_ADD_TO_IMA_KEYRINGS is enabled, but secondary keyrings aren't, additions to the IMA keyrings will be restricted to signatures verifiable by keys in the builtin system keyring only.
The patches can be found here also:
http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-trust
This patchset is based on top of
[RFC PATCH 0/7] KEYS: Adjust public key signature handling
which is on branch keys-sig in the GIT repo mentioned above.
David
---
David Howells (12):
KEYS: Generalise system_verify_data() to provide access to internal content PKCS#7: Make trust determination dependent on contents of trust keyring
KEYS: Add a facility to restrict new links into a keyring
KEYS: Move x509_request_asymmetric_key() to asymmetric_type.c
KEYS: Generalise x509_request_asymmetric_key()
X.509: Use verify_signature() if we have a struct key * to use X.509: Move the trust validation code out to its own file
KEYS: Make the system trusted keyring depend on the asymmetric key type
KEYS: Move the point of trust determination to __key_link()
KEYS: Remove KEY_FLAG_TRUSTED and KEY_ALLOC_TRUSTED
certs: Add a secondary system keyring that can be added to dynamically
IMA: Use the the system trusted keyrings instead of .ima_mok
Documentation/security/keys.txt | 22 +++ arch/x86/kernel/kexec-bzimage64.c | 18 +-- certs/Kconfig | 9 + certs/system_keyring.c | 139 ++++++++++++++++++---- crypto/asymmetric_keys/Kconfig | 6 - crypto/asymmetric_keys/Makefile | 5 + crypto/asymmetric_keys/asymmetric_keys.h | 2 crypto/asymmetric_keys/asymmetric_type.c | 89 ++++++++++++++ crypto/asymmetric_keys/mscode_parser.c | 21 +-- crypto/asymmetric_keys/pkcs7_key_type.c | 72 ++++------- crypto/asymmetric_keys/pkcs7_parser.c | 21 ++- crypto/asymmetric_keys/pkcs7_parser.h | 1 crypto/asymmetric_keys/pkcs7_trust.c | 35 ++--- crypto/asymmetric_keys/restrict.c | 108 +++++++++++++++++ crypto/asymmetric_keys/verify_pefile.c | 40 +----- crypto/asymmetric_keys/verify_pefile.h | 5 - crypto/asymmetric_keys/x509_parser.h | 1 crypto/asymmetric_keys/x509_public_key.c | 191 ------------------------------ fs/cifs/cifsacl.c | 2 fs/nfs/nfs4idmap.c | 2 include/crypto/pkcs7.h | 6 - include/crypto/public_key.h | 27 +--- include/keys/asymmetric-type.h | 6 + include/keys/system_keyring.h | 37 ++---- include/linux/key-type.h | 1 include/linux/key.h | 44 +++++-- include/linux/verification.h | 49 ++++++++ include/linux/verify_pefile.h | 22 --- kernel/module_signing.c | 7 + net/dns_resolver/dns_key.c | 2 net/rxrpc/ar-key.c | 4 - security/integrity/digsig.c | 18 ++- security/integrity/ima/Kconfig | 62 ++++++++-- security/integrity/ima/ima_mok.c | 21 +-- security/keys/key.c | 42 +++++-- security/keys/keyring.c | 46 ++++++- security/keys/persistent.c | 4 - security/keys/process_keys.c | 16 ++- security/keys/request_key.c | 4 - security/keys/request_key_auth.c | 2 40 files changed, 699 insertions(+), 510 deletions(-) create mode 100644 crypto/asymmetric_keys/restrict.c create mode 100644 include/linux/verification.h delete mode 100644 include/linux/verify_pefile.h
David Howells
maintainerRFC PATCH 1/12 KEYS: Generalise system_verify_data() to provide access to internal content [ver #3]
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.
---
arch/x86/kernel/kexec-bzimage64.c | 18 ++------ certs/system_keyring.c | 45 +++++++++++++++---- crypto/asymmetric_keys/Kconfig | 4 +- crypto/asymmetric_keys/mscode_parser.c | 21 +++------ crypto/asymmetric_keys/pkcs7_key_type.c | 72 ++++++++++++------------------- 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, 155 insertions(+), 173 deletions(-) create mode 100644 include/linux/verification.h delete mode 100644 include/linux/verify_pefile.h
arch/x86/kernel/kexec-bzimage64.c +4 -14
@@ -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
certs/system_keyring.c +35 -10
77 lines not on this page · raw message
crypto/asymmetric_keys/Kconfig +2 -2
@@ -40,8 +40,7 @@ config PKCS7_MESSAGE_PARSER config PKCS7_TEST_KEY tristate "PKCS#7 testing key type" - depends on PKCS7_MESSAGE_PARSER - select SYSTEM_TRUSTED_KEYRING + depends on SYSTEM_DATA_VERIFICATION help This option provides a type of key that can be loaded up from a PKCS#7 message - provided the message is signed by a trusted key. If
@@ -54,6 +53,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
crypto/asymmetric_keys/mscode_parser.c +7 -14
@@ -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 ; }
crypto/asymmetric_keys/pkcs7_key_type.c +28 -44
100 lines not on this page · raw message
crypto/asymmetric_keys/pkcs7_parser.c +11 -10
@@ -168,24 +168,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);
crypto/asymmetric_keys/verify_pefile.c +9 -31
@@ -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; }
crypto/asymmetric_keys/verify_pefile.h +2 -3
@@ -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);
include/crypto/pkcs7.h +2 -1
@@ -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
include/crypto/public_key.h +0 -14
@@ -15,20 +15,6 @@#define _LINUX_PUBLIC_KEY_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 ]; - -/* * 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
include/keys/asymmetric-type.h +1 -0
@@ -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;
include/keys/system_keyring.h +1 -6
@@ -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,12 +30,6 @@ 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 -#ifdef CONFIG_IMA_MOK_KEYRING extern struct key *ima_mok_keyring;extern struct key *ima_blacklist_keyring;
include/linux/verification.h new file +50 -0
50 lines not on this page · raw message
include/linux/verify_pefile.h deleted +0 -22
@@ -1,22 +0,0 @@ -/* Signed PE file verification -* -* Copyright (C) 2014 Red Hat, Inc. All Rights Reserved. -* Written by David Howells ([email protected]) -* -* 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 */
kernel/module_signing.c +3 -2
@@ -80,6 +80,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 ); }
David Howells
maintainerRFC PATCH 2/12 PKCS#7: Make trust determination dependent on contents of trust keyring [ver #3]
In reply to David HowellsMake the determination of the trustworthiness of a key dependent on whether a key that can verify it is present in the supplied ring of trusted keys rather than whether or not the verifying key has KEY_FLAG_TRUSTED set.
verify_pkcs7_signature() will return -ENOKEY if the PKCS#7 message trust chain cannot be verified.
---
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(-)
certs/system_keyring.c +4 -9
@@ -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; }
crypto/asymmetric_keys/pkcs7_key_type.c +1 -1
@@ -62,7 +62,7 @@ static int pkcs7_preparse(struct key_preparsed_payload *prep)return verify_pkcs7_signature (NULL ,0 , prep->data, prep->datalen, -NULL , -ENOKEY , usage, +NULL , usage, pkcs7_view_content, prep); }
crypto/asymmetric_keys/pkcs7_parser.h +0 -1
@@ -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) */
crypto/asymmetric_keys/pkcs7_trust.c +3 -13
@@ -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 :
crypto/asymmetric_keys/verify_pefile.c +1 -1
@@ -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;
crypto/asymmetric_keys/x509_parser.h +0 -1
@@ -39,7 +39,6 @@ struct x509_certificate {unsigned index;bool seen;/* Infinite recursion prevention */ bool verified; -bool trusted;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 */
include/crypto/pkcs7.h +1 -2
@@ -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
include/linux/verification.h +0 -1
@@ -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,
kernel/module_signing.c +1 -1
@@ -81,6 +81,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 ); }
David Howells
maintainerRFC PATCH 3/12 KEYS: Add a facility to restrict new links into a keyring [ver #3]
In reply to David HowellsAdd a facility whereby proposed new links to be added to a keyring can be vetted, permitting them to be rejected if necessary. This can be used to block public keys from which the signature cannot be verified or for which the signature verification fails. It could also be used to provide blacklisting.
This affects operations like add_key(), KEYCTL_LINK and KEYCTL_INSTANTIATE.
To this end:
(1) A function pointer is added to the key struct that, if set, points to the vetting function. This is called as:
int (*restrict_link)(struct key *keyring,
const struct key_type *key_type, unsigned long key_flags, const union key_payload *key_payload),
where 'keyring' will be the keyring being added to, key_type and key_payload will describe the key being added and key_flags[*] can be AND'ed with KEY_FLAG_TRUSTED.
[*] This parameter will be removed in a later patch when
KEY_FLAG_TRUSTED is removed.
The function should return 0 to allow the link to take place or an error (typically -ENOKEY, -ENOPKG or -EKEYREJECTED) to reject the link.
The pointer should not be set directly, but rather should be set through keyring_alloc().
Note that if called during add_key(), preparse is called before this method, but a key isn't actually allocated until after this function is called.
(2) KEY_ALLOC_BYPASS_RESTRICTION is added. This can be passed to key_create_or_update() or key_instantiate_and_link() to bypass the restriction check.
(3) KEY_FLAG_TRUSTED_ONLY is removed. The entire contents of a keyring with this restriction emplaced can be considered 'trustworthy' by virtue of being in the keyring when that keyring is consulted.
(4) key_alloc() and keyring_alloc() take an extra argument that will be used to set restrict_link in the new key. This ensures that the pointer is set before the key is published, thus preventing a window of unrestrictedness. Normally this argument will be NULL.
(5) As a temporary affair, keyring_restrict_trusted_only() is added. It should be passed to keyring_alloc() as the extra argument instead of setting KEY_FLAG_TRUSTED_ONLY on a keyring. This will be replaced in a later patch with functions that look in the appropriate places for authoritative keys.
---
Documentation/security/keys.txt | 22 +++++++++++ certs/system_keyring.c | 8 ++-- fs/cifs/cifsacl.c | 2 + fs/nfs/nfs4idmap.c | 2 + include/linux/key.h | 53 ++++++++++++++++++++++------ net/dns_resolver/dns_key.c | 2 + net/rxrpc/ar-key.c | 4 +- security/integrity/digsig.c | 7 ++-- security/integrity/ima/ima_mok.c | 8 ++-- security/keys/key.c | 43 +++++++++++++++++++--- security/keys/keyring.c | 73 ++++++++++++++++++++++++++++++++++---- security/keys/persistent.c | 4 +- security/keys/process_keys.c | 16 +++++--- security/keys/request_key.c | 4 +- security/keys/request_key_auth.c | 2 + 15 files changed, 198 insertions(+), 52 deletions(-)
Documentation/security/keys.txt +22 -0
@@ -999,6 +999,10 @@ payload contents" for more information. struct key *keyring_alloc(const char *description, uid_t uid, gid_t gid, const struct cred *cred, key_perm_t perm, + int (*restrict_link)(struct key *, + const struct key_type *, + unsigned long, + const union key_payload *), unsigned long flags, struct key *dest);
@@ -1010,6 +1014,24 @@ payload contents" for more information. KEY_ALLOC_NOT_IN_QUOTA in flags if the keyring shouldn't be accounted towards the user's quota). Error ENOMEM can also be returned. + If restrict_link not NULL, it should point to a function that will be + called each time an attempt is made to link a key into the new keyring. + This function is called to check whether a key may be added into the keying + or not. Callers of key_create_or_update() within the kernel can pass + KEY_ALLOC_BYPASS_RESTRICTION to suppress the check. An example of using + this is to manage rings of cryptographic keys that are set up when the + kernel boots where userspace is also permitted to add keys - provided they + can be verified by a key the kernel already has. + + When called, the restriction function will be passed the keyring being + added to, the key flags value and the type and payload of the key being + added. Note that when a new key is being created, this is called between + payload preparsing and actual key creation. The function should return 0 + to allow the link or an error to reject it. + + A convenience function, restrict_link_reject, exists to always return + -EPERM to in this case. + (*) To check the validity of a key, this function can be called:
certs/system_keyring.c +4 -4
@@ -36,11 +36,10 @@ static __init int system_trusted_keyring_init(void)KUIDT_INIT (0 ),KGIDT_INIT (0 ),current_cred (), ((KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ |KEY_USR_SEARCH ), - KEY_ALLOC_NOT_IN_QUOTA, NULL); +KEY_ALLOC_NOT_IN_QUOTA , + keyring_restrict_trusted_only, NULL);if (IS_ERR (system_trusted_keyring))panic ("Can't allocate system trusted keyring\n" ); - -set_bit (KEY_FLAG_TRUSTED_ONLY , &system_trusted_keyring->flags);return 0 ; }
@@ -85,7 +84,8 @@ static __init int load_system_certificate_list(void) KEY_USR_VIEW | KEY_USR_READ),KEY_ALLOC_NOT_IN_QUOTA |KEY_ALLOC_TRUSTED | - KEY_ALLOC_BUILT_IN); +KEY_ALLOC_BUILT_IN | + KEY_ALLOC_BYPASS_RESTRICTION);if (IS_ERR (key)) {pr_err ("Problem loading in-kernel X.509 certificate (%ld)\n" ,PTR_ERR (key));
fs/cifs/cifsacl.c +1 -1
@@ -360,7 +360,7 @@ init_cifs_idmap(void)GLOBAL_ROOT_UID ,GLOBAL_ROOT_GID , cred, (KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ , - KEY_ALLOC_NOT_IN_QUOTA, NULL); + KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);if (IS_ERR (keyring)) { ret =PTR_ERR (keyring); goto failed_put_cred;
fs/nfs/nfs4idmap.c +1 -1
@@ -201,7 +201,7 @@ int nfs_idmap_init(void)GLOBAL_ROOT_UID ,GLOBAL_ROOT_GID , cred, (KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ , - KEY_ALLOC_NOT_IN_QUOTA, NULL); + KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);if (IS_ERR (keyring)) { ret =PTR_ERR (keyring); goto failed_put_cred;
include/linux/key.h +43 -10
81 lines not on this page · raw message
net/dns_resolver/dns_key.c +1 -1
@@ -281,7 +281,7 @@ static int __init init_dns_resolver(void)GLOBAL_ROOT_UID ,GLOBAL_ROOT_GID , cred, (KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ , - KEY_ALLOC_NOT_IN_QUOTA, NULL); + KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);if (IS_ERR (keyring)) { ret =PTR_ERR (keyring); goto failed_put_cred;
net/rxrpc/ar-key.c +2 -2
@@ -965,7 +965,7 @@ int rxrpc_get_server_data_key(struct rxrpc_connection *conn, key =key_alloc (&key_type_rxrpc,"x" ,GLOBAL_ROOT_UID ,GLOBAL_ROOT_GID , cred,0 , -KEY_ALLOC_NOT_IN_QUOTA ); +KEY_ALLOC_NOT_IN_QUOTA ,NULL );if (IS_ERR (key)) {_leave (" = -ENOMEM [alloc %ld]" ,PTR_ERR (key));return -ENOMEM ;
@@ -1012,7 +1012,7 @@ struct key *rxrpc_get_null_key(const char *keyname) key =key_alloc (&key_type_rxrpc, keyname,GLOBAL_ROOT_UID ,GLOBAL_ROOT_GID , cred, -KEY_POS_SEARCH ,KEY_ALLOC_NOT_IN_QUOTA ); +KEY_POS_SEARCH ,KEY_ALLOC_NOT_IN_QUOTA ,NULL );if (IS_ERR (key))return key;
security/integrity/digsig.c +3 -4
@@ -83,10 +83,9 @@ int __init integrity_init_keyring(const unsigned int id) ((KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ |KEY_USR_WRITE |KEY_USR_SEARCH ), - KEY_ALLOC_NOT_IN_QUOTA, NULL); -if (!IS_ERR (keyring[id])) -set_bit (KEY_FLAG_TRUSTED_ONLY , &keyring[id]->flags); -else { +KEY_ALLOC_NOT_IN_QUOTA , + NULL, NULL); +if (IS_ERR (keyring[id])) { err =PTR_ERR (keyring[id]);pr_info ("Can't allocate %s keyring (%d)\n" , keyring_name[id], err);
security/integrity/ima/ima_mok.c +4 -4
@@ -35,20 +35,20 @@ __init int ima_mok_init(void) (KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ |KEY_USR_WRITE |KEY_USR_SEARCH , - KEY_ALLOC_NOT_IN_QUOTA, NULL); +KEY_ALLOC_NOT_IN_QUOTA , + keyring_restrict_trusted_only, NULL); ima_blacklist_keyring =keyring_alloc (".ima_blacklist" ,KUIDT_INIT (0 ),KGIDT_INIT (0 ),current_cred (), (KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ |KEY_USR_WRITE |KEY_USR_SEARCH , -KEY_ALLOC_NOT_IN_QUOTA ,NULL ); +KEY_ALLOC_NOT_IN_QUOTA , + keyring_restrict_trusted_only,NULL );if (IS_ERR (ima_mok_keyring) ||IS_ERR (ima_blacklist_keyring))panic ("Can't allocate IMA MOK or blacklist keyrings." ); -set_bit (KEY_FLAG_TRUSTED_ONLY , &ima_mok_keyring->flags); -set_bit (KEY_FLAG_TRUSTED_ONLY , &ima_blacklist_keyring->flags);set_bit (KEY_FLAG_KEEP , &ima_blacklist_keyring->flags);return 0 ; }
security/keys/key.c +36 -7
98 lines not on this page · raw message
security/keys/keyring.c +66 -7
105 lines not on this page · raw message
security/keys/persistent.c +2 -2
@@ -26,7 +26,7 @@ static int key_create_persistent_register(struct user_namespace *ns)current_cred (), ((KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ ), - KEY_ALLOC_NOT_IN_QUOTA, NULL); + KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);if (IS_ERR (reg))return PTR_ERR (reg);
@@ -60,7 +60,7 @@ static key_ref_t key_create_persistent(struct user_namespace *ns, kuid_t uid, uid,INVALID_GID ,current_cred (), ((KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ ), -KEY_ALLOC_NOT_IN_QUOTA , +KEY_ALLOC_NOT_IN_QUOTA ,NULL , ns->persistent_keyring_register);if (IS_ERR (persistent))return ERR_CAST (persistent);
security/keys/process_keys.c +10 -6
@@ -76,7 +76,8 @@ int install_user_keyrings(void)if (IS_ERR (uid_keyring)) { uid_keyring =keyring_alloc (buf, user->uid,INVALID_GID , cred, user_keyring_perm, -KEY_ALLOC_IN_QUOTA ,NULL ); +KEY_ALLOC_IN_QUOTA , +NULL ,NULL );if (IS_ERR (uid_keyring)) { ret =PTR_ERR (uid_keyring); goto error;
@@ -92,7 +93,8 @@ int install_user_keyrings(void) session_keyring =keyring_alloc (buf, user->uid,INVALID_GID , cred, user_keyring_perm, -KEY_ALLOC_IN_QUOTA ,NULL ); +KEY_ALLOC_IN_QUOTA , +NULL ,NULL );if (IS_ERR (session_keyring)) { ret =PTR_ERR (session_keyring); goto error_release;
@@ -134,7 +136,8 @@ int install_thread_keyring_to_cred(struct cred *new) keyring =keyring_alloc ("_tid" , new->uid, new->gid, new,KEY_POS_ALL |KEY_USR_VIEW , -KEY_ALLOC_QUOTA_OVERRUN ,NULL ); +KEY_ALLOC_QUOTA_OVERRUN , +NULL ,NULL );if (IS_ERR (keyring))return PTR_ERR (keyring);
@@ -180,7 +183,8 @@ int install_process_keyring_to_cred(struct cred *new) keyring =keyring_alloc ("_pid" , new->uid, new->gid, new,KEY_POS_ALL |KEY_USR_VIEW , -KEY_ALLOC_QUOTA_OVERRUN ,NULL ); +KEY_ALLOC_QUOTA_OVERRUN , +NULL ,NULL );if (IS_ERR (keyring))return PTR_ERR (keyring);
@@ -231,7 +235,7 @@ int install_session_keyring_to_cred(struct cred *cred, struct key *keyring) keyring =keyring_alloc ("_ses" , cred->uid, cred->gid, cred,KEY_POS_ALL |KEY_USR_VIEW |KEY_USR_READ , - flags,NULL ); + flags,NULL ,NULL );if (IS_ERR (keyring))return PTR_ERR (keyring); }else {
@@ -785,7 +789,7 @@ long join_session_keyring(const char *name) keyring =keyring_alloc ( name, old->uid, old->gid, old,KEY_POS_ALL |KEY_USR_VIEW |KEY_USR_READ |KEY_USR_LINK , -KEY_ALLOC_IN_QUOTA ,NULL ); +KEY_ALLOC_IN_QUOTA ,NULL ,NULL );if (IS_ERR (keyring)) { ret =PTR_ERR (keyring); goto error2;
security/keys/request_key.c +2 -2
@@ -116,7 +116,7 @@ static int call_sbin_request_key(struct key_construction *cons, cred =get_current_cred (); keyring =keyring_alloc (desc, cred->fsuid, cred->fsgid, cred,KEY_POS_ALL |KEY_USR_VIEW |KEY_USR_READ , -KEY_ALLOC_QUOTA_OVERRUN ,NULL ); +KEY_ALLOC_QUOTA_OVERRUN ,NULL ,NULL );put_cred (cred);if (IS_ERR (keyring)) { ret =PTR_ERR (keyring);
@@ -355,7 +355,7 @@ static int construct_alloc_key(struct keyring_search_context *ctx, key =key_alloc (ctx->index_key.type, ctx->index_key.description, ctx->cred->fsuid, ctx->cred->fsgid, ctx->cred, - perm, flags); + perm, flags,NULL );if (IS_ERR (key)) goto alloc_failed;
security/keys/request_key_auth.c +1 -1
@@ -202,7 +202,7 @@ struct key *request_key_auth_new(struct key *target, const void *callout_info, authkey =key_alloc (&key_type_request_key_auth, desc, cred->fsuid, cred->fsgid, cred,KEY_POS_VIEW |KEY_POS_READ |KEY_POS_SEARCH | -KEY_USR_VIEW ,KEY_ALLOC_NOT_IN_QUOTA ); +KEY_USR_VIEW ,KEY_ALLOC_NOT_IN_QUOTA ,NULL );if (IS_ERR (authkey)) { ret =PTR_ERR (authkey); goto error_alloc;
David Howells
maintainerRFC PATCH 4/12 KEYS: Move x509_request_asymmetric_key() to asymmetric_type.c [ver #3]
In reply to David HowellsMove x509_request_asymmetric_key() to asymmetric_type.c so that it can be generalised.
---
crypto/asymmetric_keys/asymmetric_type.c | 89 ++++++++++++++++++++++++++++++ crypto/asymmetric_keys/x509_public_key.c | 89 ------------------------------ include/crypto/public_key.h | 6 -- include/keys/asymmetric-type.h | 5 ++ 4 files changed, 94 insertions(+), 95 deletions(-)
crypto/asymmetric_keys/asymmetric_type.c +89 -0
@@ -35,6 +35,95 @@ static LIST_HEAD(asymmetric_key_parsers);static DECLARE_RWSEM (asymmetric_key_parsers_sem);/** +* 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); + +/** * asymmetric_key_generate_id: Construct an asymmetric key ID * @val_1: First binary blob * @len_1: Length of first binary blob
crypto/asymmetric_keys/x509_public_key.c +0 -89
95 lines not on this page · raw message
include/crypto/public_key.h +0 -6
@@ -50,12 +50,6 @@ struct key;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); -int public_key_verify_signature (const struct public_key *pkey,const struct public_key_signature *sig);
include/keys/asymmetric-type.h +5 -0
@@ -76,6 +76,11 @@ const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key)return key->payload.data[asym_key_ids]; } +extern struct key *x509_request_asymmetric_key (struct key *keyring, +const struct asymmetric_key_id *id, +const struct asymmetric_key_id *skid, +bool partial); +/* * The payload is at the discretion of the subtype. */
David Howells
maintainerRFC PATCH 5/12 KEYS: Generalise x509_request_asymmetric_key() [ver #3]
In reply to David HowellsGeneralise x509_request_asymmetric_key(). It doesn't really have any dependencies on X.509 features as it uses generalised IDs and the public_key structs that contain data extracted from X.509.
---
crypto/asymmetric_keys/asymmetric_keys.h | 2 + crypto/asymmetric_keys/asymmetric_type.c | 42 +++++++++++++++--------------- crypto/asymmetric_keys/pkcs7_trust.c | 19 ++++++-------- crypto/asymmetric_keys/x509_public_key.c | 5 +--- include/keys/asymmetric-type.h | 8 +++--- 5 files changed, 37 insertions(+), 39 deletions(-)
crypto/asymmetric_keys/asymmetric_keys.h +2 -0
@@ -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,
crypto/asymmetric_keys/asymmetric_type.c +21 -21
75 lines not on this page · raw message
crypto/asymmetric_keys/pkcs7_trust.c +8 -11
@@ -51,9 +51,8 @@ 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 =find_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 +83,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 =find_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 +100,8 @@ 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 =find_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));
crypto/asymmetric_keys/x509_public_key.c +2 -3
@@ -213,9 +213,8 @@ static int x509_validate_trust(struct x509_certificate *cert,if (cert->unsupported_sig)return -ENOPKG ; - key =x509_request_asymmetric_key (trust_keyring, - sig->auth_ids[0 ], sig->auth_ids[1 ], - false); + key =find_asymmetric_key (trust_keyring, + sig->auth_ids[0 ], sig->auth_ids[1 ], false);if (IS_ERR (key))return PTR_ERR (key);
include/keys/asymmetric-type.h +4 -4
@@ -76,10 +76,10 @@ const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key)return key->payload.data[asym_key_ids]; } -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 *find_asymmetric_key (struct key *keyring, +const struct asymmetric_key_id *id_0, +const struct asymmetric_key_id *id_1, +bool partial);/* * The payload is at the discretion of the subtype.
David Howells
maintainerRFC PATCH 6/12 X.509: Use verify_signature() if we have a struct key * to use [ver #3]
In reply to David HowellsWe should call verify_signature() rather than directly calling public_key_verify_signature() if we have a struct key to use as we shouldn't be poking around in the private data of the key struct as that's subtype dependent.
---
crypto/asymmetric_keys/x509_public_key.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
crypto/asymmetric_keys/x509_public_key.c +1 -2
@@ -220,8 +220,7 @@ static int x509_validate_trust(struct x509_certificate *cert,if (!use_builtin_keys ||test_bit (KEY_FLAG_BUILTIN , &key->flags)) { - ret =public_key_verify_signature ( - key->payload.data[asym_crypto], cert->sig); + ret =verify_signature (key, cert->sig);if (ret == -ENOPKG ) cert->unsupported_sig = true; }
David Howells
maintainerRFC PATCH 7/12 X.509: Move the trust validation code out to its own file [ver #3]
In reply to David HowellsMove the X.509 trust validation code out to its own file so that it can be generalised.
---
crypto/asymmetric_keys/Makefile | 5 + crypto/asymmetric_keys/restrict.c | 106 ++++++++++++++++++++++++++++++ crypto/asymmetric_keys/x509_parser.h | 6 ++ crypto/asymmetric_keys/x509_public_key.c | 79 ---------------------- 4 files changed, 116 insertions(+), 80 deletions(-) create mode 100644 crypto/asymmetric_keys/restrict.c
crypto/asymmetric_keys/Makefile +4 -1
@@ -4,7 +4,10 @@ obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += asymmetric_keys.o -asymmetric_keys-y := asymmetric_type.o signature.o +asymmetric_keys-y := \ + asymmetric_type.o \ + restrict.o \ + signature.o obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o
crypto/asymmetric_keys/restrict.c new file +106 -0
106 lines not on this page · raw message
crypto/asymmetric_keys/x509_parser.h +6 -0
@@ -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);
crypto/asymmetric_keys/x509_public_key.c +0 -79
91 lines not on this page · raw message
David Howells
maintainerRFC PATCH 8/12 KEYS: Make the system trusted keyring depend on the asymmetric key type [ver #3]
In reply to David HowellsMake the system trusted keyring depend on the asymmetric key type as there's not a lot of point having it if you can't then load asymmetric keys onto it.
This requires the ASYMMETRIC_KEY_TYPE to be made a bool, not a tristate, as the Kconfig language doesn't then correctly force ASYMMETRIC_KEY_TYPE to 'y' rather than 'm' if SYSTEM_TRUSTED_KEYRING is 'y'.
Making SYSTEM_TRUSTED_KEYRING *select* ASYMMETRIC_KEY_TYPE instead doesn't work as the Kconfig interpreter then wrongly complains about dependency loops.
---
certs/Kconfig | 1 + crypto/asymmetric_keys/Kconfig | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
certs/Kconfig +1 -0
@@ -17,6 +17,7 @@ config MODULE_SIG_KEY config SYSTEM_TRUSTED_KEYRING bool "Provide system-wide ring of trusted keys" depends on KEYS + depends on ASYMMETRIC_KEY_TYPE help Provide a system keyring to which trusted keys can be added. Keys in the keyring are considered to be trusted. Keys may be added at will
crypto/asymmetric_keys/Kconfig +1 -1
@@ -1,5 +1,5 @@ menuconfig ASYMMETRIC_KEY_TYPE - tristate "Asymmetric (public-key cryptographic) key type" + bool "Asymmetric (public-key cryptographic) key type" depends on KEYS help This option provides support for a key type that holds the data for
David Howells
maintainerRFC PATCH 9/12 KEYS: Move the point of trust determination to __key_link() [ver #3]
In reply to David HowellsMove 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.
If system keyrings are disabled then restrict_link_by_builtin_trusted() resolves to restrict_link_reject(). The integrity digital signature code still works correctly with this as it was previously using KEY_FLAG_TRUSTED_ONLY, which doesn't permit anything to be added if there is no system keyring against which trust can be determined.
---
certs/system_keyring.c | 20 ++++++++-- crypto/asymmetric_keys/restrict.c | 62 +++++++++++++++--------------- crypto/asymmetric_keys/x509_parser.h | 6 --- crypto/asymmetric_keys/x509_public_key.c | 21 ---------- include/crypto/public_key.h | 7 +++ include/keys/system_keyring.h | 19 +++------ kernel/module_signing.c | 2 - security/integrity/digsig.c | 33 +++++++++++++++- security/integrity/ima/ima_mok.c | 6 +-- 9 files changed, 100 insertions(+), 76 deletions(-)
certs/system_keyring.c +17 -3
@@ -18,12 +18,26 @@#include <keys/system_keyring.h> #include <crypto/pkcs7.h> -struct key *system_trusted_keyring; -EXPORT_SYMBOL_GPL (system_trusted_keyring); +static struct key *system_trusted_keyring;extern __initconst const u8 system_certificate_list[];extern __initconst const unsigned long system_certificate_list_size; +/** +* restrict_link_by_builtin_trusted - Restrict keyring addition by system CA +* +* Restrict the addition of keys into a keyring based on the key-to-be-added +* being vouched for by a key in the system keyring. +*/ +int restrict_link_by_builtin_trusted (struct key *keyring, +const struct key_type *type, +unsigned long flags, +const union key_payload *payload) +{ +return restrict_link_by_signature (system_trusted_keyring, + type, payload); +} +/* * Load the compiled-in keys */
@@ -37,7 +51,7 @@ static __init int system_trusted_keyring_init(void) ((KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ |KEY_USR_SEARCH ),KEY_ALLOC_NOT_IN_QUOTA , - keyring_restrict_trusted_only, NULL); + restrict_link_by_builtin_trusted, NULL);if (IS_ERR (system_trusted_keyring))panic ("Can't allocate system trusted keyring\n" );return 0 ;
crypto/asymmetric_keys/restrict.c +32 -30
103 lines not on this page · raw message
crypto/asymmetric_keys/x509_parser.h +0 -6
@@ -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);
crypto/asymmetric_keys/x509_public_key.c +1 -20
@@ -178,31 +178,12 @@ static int x509_key_preparse(struct key_preparsed_payload *prep) cert->pub->id_type ="X509" ; -/* See if we can derive the trustability of this certificate. -* -* When it comes to self-signed certificates, we cannot evaluate -* trustedness except by the fact that we obtained it from a trusted -* location. So we just rely on x509_validate_trust() failing in this -* case. -* -* Note that there's a possibility of a self-signed cert matching a -* cert that we have (most likely a duplicate that we already trust) - -* in which case it will be marked trusted. -*/ -if (cert->unsupported_sig || cert->self_signed) { +if (cert->unsupported_sig) {public_key_signature_free (cert->sig); cert->sig =NULL ; }else {pr_devel ("Cert Signature: %s + %s\n" , cert->sig->pkey_algo, cert->sig->hash_algo); - - ret =x509_validate_trust (cert,get_system_trusted_keyring ()); -if (ret) - ret =x509_validate_trust (cert,get_ima_mok_keyring ()); -if (ret == -EKEYREJECTED ) - goto error_free_cert; -if (!ret) - prep->trusted = true; }/* Propose a description */
include/crypto/public_key.h +7 -0
@@ -47,6 +47,13 @@ extern void public_key_signature_free(struct public_key_signature *sig);extern struct asymmetric_key_subtype public_key_subtype;struct key ; +struct key_type ; +union key_payload ; + +extern int restrict_link_by_signature (struct key *trust_keyring, +const struct key_type *type, +const union key_payload *payload); +extern int verify_signature (const struct key *key,const struct public_key_signature *sig);
include/keys/system_keyring.h +7 -12
@@ -12,22 +12,17 @@#ifndef _KEYS_SYSTEM_KEYRING_H#define _KEYS_SYSTEM_KEYRING_H +#include <linux/key.h> +#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING -#include <linux/key.h> -#include <linux/verification.h> -#include <crypto/public_key.h> +extern int restrict_link_by_builtin_trusted (struct key *keyring, +const struct key_type *type, +unsigned long flags, +const union key_payload *payload); -extern struct key *system_trusted_keyring; -static inline struct key *get_system_trusted_keyring (void ) -{ -return system_trusted_keyring; -}#else -static inline struct key *get_system_trusted_keyring (void ) -{ -return NULL ; -} +#define restrict_link_by_builtin_trusted restrict_link_reject#endif #ifdef CONFIG_IMA_MOK_KEYRING
kernel/module_signing.c +1 -1
@@ -12,7 +12,7 @@#include <linux/kernel.h> #include <linux/errno.h> #include <linux/string.h> -#include <keys/system_keyring.h> +#include <linux/verification.h> #include <crypto/public_key.h> #include "module-internal.h"
security/integrity/digsig.c +32 -1
@@ -18,6 +18,8 @@#include <linux/cred.h> #include <linux/key-type.h> #include <linux/digsig.h> +#include <crypto/public_key.h> +#include <keys/system_keyring.h> #include "integrity.h"
@@ -40,6 +42,35 @@ static bool init_keyring __initdata = true;static bool init_keyring__initdata ;#endif +#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING +/* +* Restrict the addition of keys into the IMA keyring. +* +* Any key that needs to go in .ima keyring must be signed by CA in +* either .system or .ima_mok keyrings. +*/ +static int restrict_link_by_ima_mok (struct key *keyring, +const struct key_type *type, +unsigned long flags, +const union key_payload *payload) +{ +int ret; + + ret =restrict_link_by_builtin_trusted (keyring, type, flags, payload); +if (ret != -ENOKEY ) +return ret; + +return restrict_link_by_signature (get_ima_mok_keyring (), + type, payload); +} +#else +/* +* If there's no system trusted keyring, then keys cannot be loaded into +* .ima_mok and added keys cannot be marked trusted. +*/ +#define restrict_link_by_ima_mok restrict_link_reject +#endif +int integrity_digsig_verify (const unsigned int id,const char *sig,int siglen,const char *digest,int digestlen) {
@@ -84,7 +115,7 @@ int __init integrity_init_keyring(const unsigned int id)KEY_USR_VIEW |KEY_USR_READ | KEY_USR_WRITE | KEY_USR_SEARCH),KEY_ALLOC_NOT_IN_QUOTA , - NULL, NULL); + restrict_link_by_ima_mok, NULL);if (IS_ERR (keyring[id])) { err =PTR_ERR (keyring[id]); pr_info("Can't allocate %s keyring (%d)\n",
security/integrity/ima/ima_mok.c +3 -3
@@ -17,7 +17,7 @@#include <linux/cred.h> #include <linux/err.h> #include <linux/init.h> -#include <keys/asymmetric-type.h> +#include <keys/system_keyring.h> struct key *ima_mok_keyring;
@@ -36,7 +36,7 @@ __init int ima_mok_init(void)KEY_USR_VIEW |KEY_USR_READ |KEY_USR_WRITE |KEY_USR_SEARCH ,KEY_ALLOC_NOT_IN_QUOTA , - keyring_restrict_trusted_only, NULL); + restrict_link_by_builtin_trusted, NULL); ima_blacklist_keyring = keyring_alloc(".ima_blacklist", KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
@@ -44,7 +44,7 @@ __init int ima_mok_init(void)KEY_USR_VIEW |KEY_USR_READ |KEY_USR_WRITE |KEY_USR_SEARCH ,KEY_ALLOC_NOT_IN_QUOTA , - keyring_restrict_trusted_only, NULL); + restrict_link_by_builtin_trusted, NULL);if (IS_ERR (ima_mok_keyring) ||IS_ERR (ima_blacklist_keyring))panic ("Can't allocate IMA MOK or blacklist keyrings." );
David Howells
maintainerRFC PATCH 10/12 KEYS: Remove KEY_FLAG_TRUSTED and KEY_ALLOC_TRUSTED [ver #3]
In reply to David HowellsRemove KEY_FLAG_TRUSTED and KEY_ALLOC_TRUSTED as they're no longer meaningful. Also we can drop the trusted flag from the preparse structure.
Given this, we no longer need to pass the key flags through to restrict_link().
Further, we can now get rid of keyring_restrict_trusted_only() also.
---
certs/system_keyring.c | 2 -- include/keys/system_keyring.h | 1 - include/linux/key-type.h | 1 - include/linux/key.h | 21 +++++---------------- security/integrity/digsig.c | 3 +-- security/keys/key.c | 11 ++--------- security/keys/keyring.c | 29 +---------------------------- 7 files changed, 9 insertions(+), 59 deletions(-)
certs/system_keyring.c +0 -2
@@ -31,7 +31,6 @@ extern __initconst const unsigned long system_certificate_list_size;*/ int restrict_link_by_builtin_trusted (struct key *keyring,const struct key_type *type, -unsigned long flags,const union key_payload *payload) { return restrict_link_by_signature(system_trusted_keyring,
@@ -97,7 +96,6 @@ static __init int load_system_certificate_list(void) ((KEY_POS_ALL & ~KEY_POS_SETATTR ) |KEY_USR_VIEW |KEY_USR_READ ),KEY_ALLOC_NOT_IN_QUOTA | -KEY_ALLOC_TRUSTED |KEY_ALLOC_BUILT_IN | KEY_ALLOC_BYPASS_RESTRICTION);if (IS_ERR (key)) {
include/keys/system_keyring.h +0 -1
@@ -18,7 +18,6 @@extern int restrict_link_by_builtin_trusted (struct key *keyring,const struct key_type *type, -unsigned long flags,const union key_payload *payload);#else
include/linux/key-type.h +0 -1
@@ -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,
include/linux/key.h +5 -16
@@ -173,10 +173,9 @@ 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_BUILTIN 9/* set if key is built in to the kernel */ -#define KEY_FLAG_ROOT_CAN_INVAL 10/* set if key can be invalidated by root without permission */ -#define KEY_FLAG_KEEP 11/* set if key should not be removed */ +#define KEY_FLAG_BUILTIN 8/* set if key is built in to the kernel */ +#define KEY_FLAG_ROOT_CAN_INVAL 9/* set if key can be invalidated by root without permission */ +#define KEY_FLAG_KEEP 10/* set if key should not be removed */ /* the key type and key description string * - the desc is used to match a key against search criteria
@@ -217,7 +216,6 @@ struct key {*/ int (*restrict_link)(struct key *keyring,const struct key_type *type, -unsigned long flags,const union key_payload *payload); };
@@ -229,16 +227,14 @@ extern struct key *key_alloc(struct key_type *type,unsigned long flags, int (*restrict_link)(struct key *,const struct key_type *, -unsigned long , const union key_payload *));#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_BUILT_IN 0x0008/* Key is built into kernel */ -#define KEY_ALLOC_BYPASS_RESTRICTION 0x0010/* Override the check on restricted keyrings */ +#define KEY_ALLOC_BUILT_IN 0x0004/* Key is built into kernel */ +#define KEY_ALLOC_BYPASS_RESTRICTION 0x0008/* Override the check on restricted keyrings */ extern void key_revoke (struct key *key);extern void key_invalidate (struct key *key);
@@ -309,18 +305,11 @@ extern struct key *keyring_alloc(const char *description, kuid_t uid, kgid_t gidunsigned long flags, int (*restrict_link)(struct key *,const struct key_type *, -unsigned long ,const union key_payload *), struct key *dest); -extern int keyring_restrict_trusted_only (struct key *keyring, -const struct key_type *type, -unsigned long , -const union key_payload *payload); -extern int restrict_link_reject (struct key *keyring,const struct key_type *type, -unsigned long flags,const union key_payload *payload);extern int keyring_clear (struct key *keyring);
security/integrity/digsig.c +1 -2
@@ -51,12 +51,11 @@ static bool init_keyring __initdata;*/ static int restrict_link_by_ima_mok (struct key *keyring,const struct key_type *type, -unsigned long flags,const union key_payload *payload) {int ret; - ret =restrict_link_by_builtin_trusted (keyring, type, flags, payload); + ret =restrict_link_by_builtin_trusted (keyring, type, payload);if (ret != -ENOKEY )return ret;
security/keys/key.c +2 -9
@@ -227,7 +227,6 @@ struct key *key_alloc(struct key_type *type, const char *desc, key_perm_t perm, unsigned long flags, int (*restrict_link)(struct key *,const struct key_type *, -unsigned long , const union key_payload *)) { struct key_user *user = NULL;
@@ -300,8 +299,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 ;if (flags &KEY_ALLOC_BUILT_IN ) key->flags |=1 <<KEY_FLAG_BUILTIN ;
@@ -504,7 +501,7 @@ int key_instantiate_and_link(struct key *key,if (keyring) {if (keyring->restrict_link) { ret = keyring->restrict_link (keyring, key->type, - key->flags, &prep.payload); + &prep.payload);if (ret <0 ) goto error; }
@@ -811,7 +808,6 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,int ret;int (*restrict_link)(struct key *,const struct key_type *, -unsigned long ,const union key_payload *) =NULL ;/* look up the key type to see if it's one of the registered kernel
@@ -843,7 +839,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);
@@ -860,9 +855,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, index_key.desc_len =strlen (index_key.description);if (restrict_link) { -unsigned long kflags = prep.trusted ?KEY_FLAG_TRUSTED :0 ; - ret =restrict_link (keyring, - index_key.type, kflags, &prep.payload); + ret =restrict_link (keyring, index_key.type, &prep.payload);if (ret <0 ) { key_ref =ERR_PTR (ret); goto error_free_prep;
security/keys/keyring.c +1 -28
@@ -494,7 +494,6 @@ struct key *keyring_alloc(const char *description, kuid_t uid, kgid_t gid,unsigned long flags, int (*restrict_link)(struct key *,const struct key_type *, -unsigned long ,const union key_payload *), struct key *dest) {
@@ -516,33 +515,9 @@ struct key *keyring_alloc(const char *description, kuid_t uid, kgid_t gid,EXPORT_SYMBOL (keyring_alloc);/** -* keyring_restrict_trusted_only - Restrict additions to a keyring to trusted keys only -* @keyring: The keyring being added to. -* @type: The type of key being added. -* @flags: The key flags. -* @payload: The payload of the key intended to be added. -* -* Reject the addition of any links to a keyring that point to keys that aren't -* marked as being trusted. It can be overridden by passing -* KEY_ALLOC_BYPASS_RESTRICTION to key_instantiate_and_link() when adding a key -* to a keyring. -* -* This is meant to be passed as the restrict_link parameter to -* keyring_alloc(). -*/ -int keyring_restrict_trusted_only (struct key *keyring, -const struct key_type *type, -unsigned long flags, -const union key_payload *payload) -{ -return flags &KEY_FLAG_TRUSTED ?0 : -EPERM ; -} - -/** * restrict_link_reject - Give -EPERM to restrict link * @keyring: The keyring being added to. * @type: The type of key being added. -* @flags: The key flags. * @payload: The payload of the key intended to be added. * * Reject the addition of any links to a keyring. It can be overridden by
@@ -554,7 +529,6 @@ int keyring_restrict_trusted_only(struct key *keyring,*/ int restrict_link_reject (struct key *keyring,const struct key_type *type, -unsigned long flags,const union key_payload *payload) {return -EPERM ;
@@ -1248,8 +1222,7 @@ static int __key_link_check_restriction(struct key *keyring, struct key *key) {if (!keyring->restrict_link)return 0 ; -return keyring->restrict_link (keyring, - key->type, key->flags, &key->payload); +return keyring->restrict_link (keyring, key->type, &key->payload); }/**
David Howells
maintainerRFC PATCH 11/12 certs: Add a secondary system keyring that can be added to dynamically [ver #3]
In reply to David HowellsAdd a secondary system keyring that can be added to by root whilst the system is running - provided the key being added is vouched for by a key built into the kernel or already added to the secondary keyring.
Rename .system_keyring to .builtin_trusted_keys to distinguish it more obviously from the new keyring (called .secondary_trusted_keys).
The new keyring needs to be enabled with CONFIG_SECONDARY_TRUSTED_KEYRING.
If the secondary keyring is enabled, a link is created from that to .builtin_trusted_keys so that the the latter will automatically be searched too if the secondary keyring is searched.
---
certs/Kconfig | 8 ++++ certs/system_keyring.c | 87 +++++++++++++++++++++++++++++++++-------- include/keys/system_keyring.h | 9 ++++ 3 files changed, 88 insertions(+), 16 deletions(-)
certs/Kconfig +8 -0
@@ -56,4 +56,12 @@ config SYSTEM_EXTRA_CERTIFICATE_SIZE This is the number of bytes reserved in the kernel image for a certificate to be inserted. +config SECONDARY_TRUSTED_KEYRING + bool "Provide a keyring to which extra trustable keys may be added" + depends on SYSTEM_TRUSTED_KEYRING + help + If set, provide a keyring to which extra keys may be added, provided + those keys are not blacklisted and are vouched for by a key built + into the kernel or already in the secondary trusted keyring. + endmenu
certs/system_keyring.c +71 -16
134 lines not on this page · raw message
include/keys/system_keyring.h +9 -0
@@ -24,6 +24,15 @@ extern int restrict_link_by_builtin_trusted(struct key *keyring,#define restrict_link_by_builtin_trusted restrict_link_reject#endif +#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING +extern int restrict_link_by_builtin_and_secondary_trusted ( +struct key *keyring, +const struct key_type *type, +const union key_payload *payload); +#else +#define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted +#endif +#ifdef CONFIG_IMA_MOK_KEYRING extern struct key *ima_mok_keyring;extern struct key *ima_blacklist_keyring;
Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #3]
Quoted 14 lines · David Howells
certs/system_keyring.c +14
-0+#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING +/** +* restrict_link_by_builtin_and_secondary_trusted - Restrict keyring +* addition by both builtin and secondary keyrings +* +* Restrict the addition of keys into a keyring based on the key-to-be-added +* being vouched for by a key in either the built-in or the secondary system +* keyrings. +*/ +int restrict_link_by_builtin_and_secondary_trusted ( +struct key *keyring, +const struct key_type *type, +const union key_payload *payload) +{
certs/system_keyring.c line 65
Quoted 10 lines
certs/system_keyring.c +10
-0+/* If we have a secondary trusted keyring, then that contains a link +* through to the builtin keyring and the search will follow that link. +*/ +if (type == &key_type_keyring && + keyring == secondary_trusted_keys && + payload == &builtin_trusted_keys->payload) +/* Allow the builtin keyring to be added to the secondary */ +return 0 ; + +return restrict_link_by_signature (builtin_trusted_keys, type, payload);
Shouldn't thi be secondary_trusted_keys?
Mimi
Quoted 2 lines
(quoted diff) +2
-0+} +#endif
David Howells
maintainerRe: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #3]
Quoted 3 lines · Mimi Zohar
Review
Quoted 1 line
(quoted diff) +1
-0+ return restrict_link_by_signature(builtin_trusted_keys, type, payload);Shouldn't thi be
secondary_trusted_keys?
Yeah. Good catch, thanks!
David
David Howells
maintainerRFC PATCH 12/12 IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
In reply to David HowellsProvide a config option (IMA_PERMIT_ADD_TO_IMA_KEYRINGS) that, when enabled, allows keys to be added to the IMA keyrings by userspace - with the restriction that each must be signed by a key in the system trusted keyrings.
EPERM will be returned if this option is disabled, ENOKEY will be returned if no authoritative key can be found and EKEYREJECTED will be returned if the signature doesn't match. Other errors such as ENOPKG may also be returned.
If this new option is enabled, the builtin system keyring is searched, as is the secondary system keyring if that is also enabled. Intermediate keys between the builtin system keyring and the key being added can be added to the secondary keyring (which replaces .ima_mok) to form a trust chain - provided they are also validly signed by a key in one of the trusted keyrings.
The .ima_mok keyring is removed.
---
include/keys/system_keyring.h | 9 ------ security/integrity/digsig.c | 33 ++++---------------- security/integrity/ima/Kconfig | 62 +++++++++++++++++++++++++++++++------- security/integrity/ima/ima_mok.c | 15 ++------- 4 files changed, 60 insertions(+), 59 deletions(-)
include/keys/system_keyring.h +0 -9
@@ -34,22 +34,13 @@ extern int restrict_link_by_builtin_and_secondary_trusted(#endif #ifdef CONFIG_IMA_MOK_KEYRING -extern struct key *ima_mok_keyring;extern struct key *ima_blacklist_keyring; -static inline struct key *get_ima_mok_keyring (void ) -{ -return ima_mok_keyring; -}static inline struct key *get_ima_blacklist_keyring (void ) {return ima_blacklist_keyring; }#else -static inline struct key *get_ima_mok_keyring (void ) -{ -return NULL ; -}static inline struct key *get_ima_blacklist_keyring (void ) {return NULL ;
security/integrity/digsig.c +7 -26
@@ -42,32 +42,12 @@ static bool init_keyring __initdata = true;static bool init_keyring__initdata ;#endif -#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING -/* -* Restrict the addition of keys into the IMA keyring. -* -* Any key that needs to go in .ima keyring must be signed by CA in -* either .system or .ima_mok keyrings. -*/ -static int restrict_link_by_ima_mok (struct key *keyring, -const struct key_type *type, -const union key_payload *payload) -{ -int ret; - - ret =restrict_link_by_builtin_trusted (keyring, type, payload); -if (ret != -ENOKEY ) -return ret; - -return restrict_link_by_signature (get_ima_mok_keyring (), - type, payload); -} +#if defined(CONFIG_IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN ) +#define restrict_link_to_ima restrict_link_by_builtin_trusted +#elif defined(CONFIG_IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN_OR_SECONDARY ) +#define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted#else -/* -* If there's no system trusted keyring, then keys cannot be loaded into -* .ima_mok and added keys cannot be marked trusted. -*/ -#define restrict_link_by_ima_mok restrict_link_reject +#define restrict_link_to_ima restrict_link_reject#endif int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
@@ -114,7 +94,8 @@ int __init integrity_init_keyring(const unsigned int id)KEY_USR_VIEW |KEY_USR_READ | KEY_USR_WRITE | KEY_USR_SEARCH),KEY_ALLOC_NOT_IN_QUOTA , - restrict_link_by_ima_mok, NULL); + restrict_link_to_ima, + NULL);if (IS_ERR (keyring[id])) { err =PTR_ERR (keyring[id]); pr_info("Can't allocate %s keyring (%d)\n",
security/integrity/ima/Kconfig +50 -12
72 lines not on this page · raw message
security/integrity/ima/ima_mok.c +3 -12
@@ -20,23 +20,14 @@#include <keys/system_keyring.h> -struct key *ima_mok_keyring;struct key *ima_blacklist_keyring;/* -* Allocate the IMA MOK and blacklist keyrings +* Allocate the IMA blacklist keyring */ __init int ima_mok_init (void ) { -pr_notice ("Allocating IMA MOK and blacklist keyrings.\n" ); - - ima_mok_keyring =keyring_alloc (".ima_mok" , -KUIDT_INIT (0 ),KGIDT_INIT (0 ),current_cred (), - (KEY_POS_ALL & ~KEY_POS_SETATTR ) | -KEY_USR_VIEW |KEY_USR_READ | -KEY_USR_WRITE |KEY_USR_SEARCH , -KEY_ALLOC_NOT_IN_QUOTA , - restrict_link_by_builtin_trusted,NULL ); +pr_notice ("Allocating IMA blacklist keyrings.\n" ); ima_blacklist_keyring = keyring_alloc(".ima_blacklist", KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
@@ -46,7 +37,7 @@ __init int ima_mok_init(void)KEY_ALLOC_NOT_IN_QUOTA , restrict_link_by_builtin_trusted, NULL); -if (IS_ERR (ima_mok_keyring) ||IS_ERR (ima_blacklist_keyring)) +if (IS_ERR (ima_blacklist_keyring))panic ("Can't allocate IMA MOK or blacklist keyrings." );set_bit (KEY_FLAG_KEEP , &ima_blacklist_keyring->flags);
Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Hi David,
Quoted 9 lines · David Howells
Provide a config option (
IMA_PERMIT_ADD_TO_IMA_KEYRINGS) that, when enabled, allows keys to be added to the IMA keyrings by userspace - with the restriction that each must be signed by a key in the system trusted keyrings.EPERM will be returned if this option is disabled, ENOKEY will be returned if no authoritative key can be found and EKEYREJECTED will be returned if the signature doesn't match. Other errors such as ENOPKG may also be returned.
Quoted 7 lines
If this new option is enabled, the builtin system keyring is searched, as is the secondary system keyring if that is also enabled. Intermediate keys between the builtin system keyring and the key being added can be added to the secondary keyring (which replaces .ima_mok) to form a trust chain - provided they are also validly signed by a key in one of the trusted keyrings.
The .ima_mok keyring is removed.
This patch adds new Kconfig options, when it should be limited to
removing the .ima_mok keyring. Lets change the title to "IMA: use thesecondary_trusted_keys instead of .ima_mok" and limit the new Kconfig option to using the secondary_trusted_keys.
Quoted 93 lines
Signed-off-by David Howells <[email protected]>---
include/keys/system_keyring.h | 9 ------ security/integrity/digsig.c | 33 ++++---------------- security/integrity/ima/Kconfig | 62 +++++++++++++++++++++++++++++++------- security/integrity/ima/ima_mok.c | 15 ++------- 4 files changed, 60 insertions(+), 59 deletions(-)include/keys/system_keyring.h +0
-9@@ -34,22 +34,13 @@ extern int restrict_link_by_builtin_and_secondary_trusted(#endif #ifdef CONFIG_IMA_MOK_KEYRING -extern struct key *ima_mok_keyring;extern struct key *ima_blacklist_keyring; -static inline struct key *get_ima_mok_keyring (void ) -{ -return ima_mok_keyring; -}static inline struct key *get_ima_blacklist_keyring (void ) {return ima_blacklist_keyring; }#else -static inline struct key *get_ima_mok_keyring (void ) -{ -return NULL ; -}static inline struct key *get_ima_blacklist_keyring (void ) {return NULL ;security/integrity/digsig.c +7
-26@@ -42,32 +42,12 @@ static bool init_keyring __initdata = true;static bool init_keyring__initdata ;#endif -#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING -/* -* Restrict the addition of keys into the IMA keyring. -* -* Any key that needs to go in .ima keyring must be signed by CA in -* either .system or .ima_mok keyrings. -*/ -static int restrict_link_by_ima_mok (struct key *keyring, -const struct key_type *type, -const union key_payload *payload) -{ -int ret; - - ret =restrict_link_by_builtin_trusted (keyring, type, payload); -if (ret != -ENOKEY ) -return ret; - -return restrict_link_by_signature (get_ima_mok_keyring (), - type, payload); -} +#if defined(CONFIG_IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN ) +#define restrict_link_to_ima restrict_link_by_builtin_trusted +#elif defined(CONFIG_IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN_OR_SECONDARY ) +#define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted#else -/* -* If there's no system trusted keyring, then keys cannot be loaded into -* .ima_mok and added keys cannot be marked trusted. -*/ -#define restrict_link_by_ima_mok restrict_link_reject +#define restrict_link_to_ima restrict_link_reject#endif int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,@@ -114,7 +94,8 @@ int __init integrity_init_keyring(const unsigned int id)KEY_USR_VIEW |KEY_USR_READ | KEY_USR_WRITE | KEY_USR_SEARCH),KEY_ALLOC_NOT_IN_QUOTA , - restrict_link_by_ima_mok, NULL); + restrict_link_to_ima, + NULL);if (IS_ERR (keyring[id])) { err =PTR_ERR (keyring[id]); pr_info("Can't allocate %s keyring (%d)\n",security/integrity/ima/Kconfig +0
-0
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 158
Quoted 3 lines
security/integrity/ima/Kconfig +0
-0@@ -156,22 +156,60 @@ config IMA_TRUSTED_KEYRING This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING config IMA_MOK_KEYRING
As we're left with only the IMA blacklist, please change the IMA_MOK_KEYRING to IMA_BLACKLIST_KEYRING.
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 159
Quoted 2 lines
security/integrity/ima/Kconfig +1
-1- bool "Create IMA machine owner keys (MOK) and blacklist keyrings" + bool "Create IMA machine owner blacklist keyrings"
Both the .ima_mok and .ima_blacklist keyrings should have been annotated
as experimental. Please change this to "Create IMA machine owner
blacklist keyrings (EXPERIMENTAL)".
Quoted 15 lines
security/integrity/ima/Kconfig +0
-11depends on SYSTEM_TRUSTED_KEYRING depends on IMA_TRUSTED_KEYRING default n help - This option creates IMA MOK and blacklist keyrings. IMA MOK is an - intermediate keyring that sits between .system and .ima keyrings, - effectively forming a simple CA hierarchy. To successfully import a - key into .ima_mok it must be signed by a key which CA is in .system - keyring. On turn any key that needs to go in .ima keyring must be - signed by CA in either .system or .ima_mok keyrings. IMA MOK is empty - at kernel boot. - - IMA blacklist keyring contains all revoked IMA keys. It is consulted - before any other keyring. If the search is successful the requested - operation is rejected and error is returned to the caller.
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 173
Quoted 10 lines
security/integrity/ima/Kconfig +10
-0+ This option creates IMA blacklist keyring. This contains all + revoked IMA keys. It is consulted before any other keyring. If the + search is successful the requested operation is rejected and error + is returned to the caller. + +choice + prompt "Allow keys to be added to the ima keyrings by userspace?" + depends on IMA_APPRAISE + depends on INTEGRITY_ASYMMETRIC_KEYS + default IMA_NO_ADD_TO_IMA_KEYRINGS
In this patch, the choice should be between checking just the builtin trusted keys or both the builtin trusted and secondary keys.
if IMA is enabled, I'm not sure what IMA_NO_ADD_TO_IMA_KEYRINGS means.
Quoted 4 lines
security/integrity/ima/Kconfig +4
-0+ help + This option selects whether keys may be added to the ima keyrings + using add_key() or KEYCTL_LINK, and, if so, under what restrictions + the key being added will be placed.
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 187
Quoted 10 lines
security/integrity/ima/Kconfig +10
-0+ +config IMA_KEYRINGS_COMPILE_LOAD_ONLY + bool "No runtime key addition" + help + No keys may be added to the IMA keyrings by userspace in the running + kernel. Keys may still be added by including X.509 certificates in + the kernel image at compile time. + + Attempts to add to the ima keyrings will be rejected with EPERM. +
This could be useful for namespacing IMA.
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 198
Quoted 11 lines
security/integrity/ima/Kconfig +11
-0+config IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN + bool "Keys may be added at runtime if validly signed by a built-in CA cert" + depends on SYSTEM_TRUSTED_KEYRING + select INTEGRITY_TRUSTED_KEYRING + help + keys may be added to the IMA keyrings by userspace in the running + kernel if the keys to be added are validly signed by a CA cert in the + system built-in trusted keyring. + +config IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN_OR_SECONDARY + bool "Keys may be added at runtime if validly signed by a built-in or secondary CA cert"
Please add "(EXPERIMENTAL)" to this option as well.
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING
Quoted 12 lines
security/integrity/ima/Kconfig +11
-0+ depends on SYSTEM_TRUSTED_KEYRING + depends on SECONDARY_TRUSTED_KEYRING + select INTEGRITY_TRUSTED_KEYRING + help + keys may be added to the IMA keyrings by userspace in the running + kernel if the keys to be added are validly signed by a CA cert in the + system built-in or secondary trusted keyrings. + Intermediate keys between those the kernel has compiled in and the + IMA keys to be added may be added to the system secondary keyring, + provided they are validly signed by a key already resident in the + built-in or secondary trusted keyrings.
A reminder should be included here indicating that not only IMA specific keys are on the secondary keyring.
Thanks!
Mimi
Quoted 49 lines
security/integrity/ima/Kconfig +1
-0+endchoice config IMA_LOAD_X509 bool "Load X509 certificate onto the '.ima' trusted keyring"security/integrity/ima/ima_mok.c +3
-13@@ -20,23 +20,14 @@#include <keys/system_keyring.h> -struct key *ima_mok_keyring;struct key *ima_blacklist_keyring;/* -* Allocate the IMA MOK and blacklist keyrings +* Allocate the IMA blacklist keyring */ __init int ima_mok_init (void ) { -pr_notice ("Allocating IMA MOK and blacklist keyrings.\n" ); - - ima_mok_keyring =keyring_alloc (".ima_mok" , -KUIDT_INIT (0 ),KGIDT_INIT (0 ),current_cred (), - (KEY_POS_ALL & ~KEY_POS_SETATTR ) | -KEY_USR_VIEW |KEY_USR_READ | -KEY_USR_WRITE |KEY_USR_SEARCH , -KEY_ALLOC_NOT_IN_QUOTA , - restrict_link_by_builtin_trusted,NULL ); +pr_notice ("Allocating IMA blacklist keyrings.\n" ); ima_blacklist_keyring = keyring_alloc(".ima_blacklist", KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),@@ -46,7 +37,7 @@ __init int ima_mok_init(void)KEY_ALLOC_NOT_IN_QUOTA , restrict_link_by_builtin_trusted, NULL); -if (IS_ERR (ima_mok_keyring) ||IS_ERR (ima_blacklist_keyring)) +if (IS_ERR (ima_blacklist_keyring))panic ("Can't allocate IMA MOK or blacklist keyrings." );set_bit (KEY_FLAG_KEEP , &ima_blacklist_keyring->flags); --To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
David Howells
maintainerRe: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 8 lines · Mimi Zohar
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 173
Quoted 5 lines
security/integrity/ima/Kconfig +5
-0+choice + prompt "Allow keys to be added to the ima keyrings by userspace?" + depends on IMA_APPRAISE + depends on INTEGRITY_ASYMMETRIC_KEYS + default IMA_NO_ADD_TO_IMA_KEYRINGSIn this patch, the choice should be between checking just the builtin trusted keys or both the builtin trusted and secondary keys.
The third option I've added is that you can't add to .ima at all. You only get what's included at build time. You don't want that option?
Quoted 1 line
if IMA is enabled, I'm not sure what
IMA_NO_ADD_TO_IMA_KEYRINGSmeans.
Oops. that should be IMA_KEYRINGS_COMPILE_LOAD_ONLY.
"IMA_NO_ADD_TO_IMA_KEYRINGS" seemed to be phrased too clunkily, but the Kconfig parser warn you about the undefined symbol.
Quoted 4 lines
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 180
Quoted 2 lines
security/integrity/ima/Kconfig +2
-0+config IMA_KEYRINGS_COMPILE_LOAD_ONLY + bool "No runtime key addition"...
This could be useful for namespacing IMA.
You said you didn't want this option above (to quote: In this patch, the choice should be between checking just the builtin trusted keys or both the builtin trusted and secondary keys.)
David
Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 11 lines · David Howells
Quoted 8 lines · Mimi Zohar
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 173
Quoted 5 lines
security/integrity/ima/Kconfig +5
-0+choice + prompt "Allow keys to be added to the ima keyrings by userspace?" + depends on IMA_APPRAISE + depends on INTEGRITY_ASYMMETRIC_KEYS + default IMA_NO_ADD_TO_IMA_KEYRINGSIn this patch, the choice should be between checking just the builtin trusted keys or both the builtin trusted and secondary keys.
Quoted 2 lines
The third option I've added is that you can't add to .ima at all. You only get what's included at build time. You don't want that option?
Adding keys directly to the IMA keyring is a new feature. There's enough changes as it is, that this patch should be limited to replicating existing usage, not adding new features.
Quoted 3 lines
Quoted 1 line
if IMA is enabled, I'm not sure what
IMA_NO_ADD_TO_IMA_KEYRINGSmeans.Oops. that should be
IMA_KEYRINGS_COMPILE_LOAD_ONLY.
The default should be to validate keys against the builtin trusted keys, like it is currently.
Quoted 11 lines
"
IMA_NO_ADD_TO_IMA_KEYRINGS" seemed to be phrased too clunkily, but theKconfigparser warn you about the undefined symbol.Quoted 4 lines
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 180
Quoted 2 lines
security/integrity/ima/Kconfig +2
-0+config IMA_KEYRINGS_COMPILE_LOAD_ONLY + bool "No runtime key addition"...
This could be useful for namespacing IMA.You said you didn't want this option above (to quote: In this patch, the choice should be between checking just the builtin trusted keys or both the builtin trusted and secondary keys.)
Does the ability of adding builtin X.509 certificates directly to the IMA keyring already exist or is it something that still needs to be
done? Assuming the latter, this option would be added with the ability
of adding X.509 certificates directly to the IMA keyring.
Mimi
David Howells
maintainerRe: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 6 lines · Mimi Zohar
Quoted 2 lines
The third option I've added is that you can't add to .ima at all. You only get what's included at build time. You don't want that option?
Adding keys directly to the IMA keyring is a new feature. There's enough changes as it is, that this patch should be limited to replicating existing usage, not adding new features.
The three choice options I implemented don't exactly provide new features. Firstly:
config IMA_LOAD_X509
allow keys to be loaded in at compile time, secondly,
config INTEGRITY_SIGNATURE
allows keys to be inserted whilst the kernel is running if those keys are "trusted", and, thirdly,
config IMA_MOK_KEYRING
modifies what it means for a key to be "trusted" by allowing intermediate levels of CA certificates to be chained.
So, of the three choice options I'm providing,
config IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN
is equivalent to setting INTEGRITY_SIGNATURE plus IMA_LOAD_X509 if you set it; then
config IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN_OR_SECONDARY
is the equivalent of turning on IMA_MOK_KEYRING as well.
Fair enough, you can argue that:
config IMA_KEYRINGS_COMPILE_LOAD_ONLY
allowing you to disable addition of keys whilst the kernel is running is a 'new feature'. I disagree, I think it should be there for completeness.
Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
security/integrity/ima/Kconfig config IMA_TRUSTED_KEYRING · line 180
Quoted 2 lines
security/integrity/ima/Kconfig +2
-0+config IMA_KEYRINGS_COMPILE_LOAD_ONLY + bool "No runtime key addition"...
This could be useful for namespacing IMA.
Quoted 8 lines
Quoted 3 lines
You said you didn't want this option above (to quote: In this patch, the choice should be between checking just the builtin trusted keys or both the builtin trusted and secondary keys.)
Does the ability of adding builtin X.509 certificates directly to the IMA keyring already exist or is it something that still needs to be
done? Assuming the latter, this option would be added with the ability
of adding X.509 certificates directly to the IMA keyring.
I don't know what you mean by "directly" here. Even without this patch, you can already add X.509 certs to the .ima keyring, both at compile time and at runtime. This patch doesn't take that away - except by explicitly setting the option to prevent runtime addition.
David
Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 6 lines · David Howells
Quoted 4 lines · Mimi Zohar
Quoted 3 lines
You said you didn't want this option above (to quote: In this patch, the choice should be between checking just the builtin trusted keys or both the builtin trusted and secondary keys.)
Quoted 7 lines
Quoted 4 lines
Does the ability of adding builtin X.509 certificates directly to the IMA keyring already exist or is it something that still needs to be
done? Assuming the latter, this option would be added with the ability
of adding X.509 certificates directly to the IMA keyring.I don't know what you mean by "directly" here. Even without this patch, you can already add X.509 certs to the .ima keyring, both at compile time
I'm must be missing something obvious here. I need to review the patches again. The builtin keys are loaded onto
the .builtin_trusted_keys keyring, not the IMA keyring.
Mimi
Quoted 6 lines
and at
runtime. This patch doesn't take that away - except by explicitly setting the option to prevent runtime addition.David
Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 12 lines · Mimi Zohar
Quoted 11 lines · David Howells
Quoted 8 lines · Mimi Zohar
Quoted 3 lines
You said you didn't want this option above (to quote: In this patch, the choice should be between checking just the builtin trusted keys or both the builtin trusted and secondary keys.)
Does the ability of adding builtin X.509 certificates directly to the IMA keyring already exist or is it something that still needs to be
done? Assuming the latter, this option would be added with the ability
of adding X.509 certificates directly to the IMA keyring.
Quoted 6 lines
Quoted 2 lines
I don't know what you mean by "directly" here. Even without this patch, you can already add X.509 certs to the .ima keyring, both at compile time
I'm must be missing something obvious here. I need to review the patches again. The builtin keys are loaded onto
the .builtin_trusted_keys keyring, not the IMA keyring.is the
The only place where "KEY_ALLOC_BYPASS_RESTRICTION" is specified is in load_system_certificate_list(), when adding keys to
the .builtin_trusted_keys keyring. There is no other set of keys builtin and added to the IMA keyring.
Mimi
David Howells
maintainerRe: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 4 lines · Mimi Zohar
The only place where "
KEY_ALLOC_BYPASS_RESTRICTION" is specified is inload_system_certificate_list(), when adding keys to
the .builtin_trusted_keys keyring. There is no other set of keys builtin and added to the IMA keyring.
Are the keys loaded by integrity_load_x509() required to be validly signed by the builtin/secondary keys? Or is that unnecessary given that they are loaded and thus protected through integrity_read_file()?
David
Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 10 lines · David Howells
Quoted 4 lines · Mimi Zohar
The only place where "
KEY_ALLOC_BYPASS_RESTRICTION" is specified is inload_system_certificate_list(), when adding keys to
the .builtin_trusted_keys keyring. There is no other set of keys builtin and added to the IMA keyring.Are the keys loaded by
integrity_load_x509()required to be validly signed by the builtin/secondary keys? Or is that unnecessary given that they are loaded and thus protected throughintegrity_read_file()?
Loading keys on the IMA keyring is safe, because the certificates must be signed by a key on the builtin keyring or the secondary keyring, if it is Kconfig enabled.
Mimi
David Howells
maintainerRe: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
In reply to David HowellsQuoted 6 lines · David Howells
The three choice options I implemented don't exactly provide new features. Firstly:
config
IMA_LOAD_X509allow keys to be loaded in at compile time,
Ah - I think I'm labouring under a slight misapprehension here. IMA_LOAD_X509 doesn't load keys at compile time, but rather the kernel loads a file by name when booting, right?
David
Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 5 lines · David Howells
Quoted 3 lines · David Howells
The three choice options I implemented don't exactly provide new features. Firstly:
Quoted 7 lines
Quoted 3 lines
config
IMA_LOAD_X509allow keys to be loaded in at compile time,
Ah - I think I'm labouring under a slight misapprehension here.
IMA_LOAD_X509doesn't load keys at compile time, but rather the kernel loads a file by name when booting, right?
Right, all certificates must be signed by a key on the builtin (or
secondary keyring) before being added to the IMA keyring. Similarly,
dracut (modules/98integrity/ and systemd (ima-setup.c) have support for loading signed certificates on the IMA keyring.
Mimi
Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
In reply to David HowellsQuoted 14 lines · David Howells
security/integrity/digsig.c +0
-14-#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING -/* -* Restrict the addition of keys into the IMA keyring. -* -* Any key that needs to go in .ima keyring must be signed by CA in -* either .system or .ima_mok keyrings. -*/ -static int restrict_link_by_ima_mok (struct key *keyring, -const struct key_type *type, -const union key_payload *payload) -{ -int ret; - - ret =restrict_link_by_builtin_trusted (keyring, type, payload);
security/integrity/digsig.c static bool init_keyring __initdata = true; · line 48
Quoted 10 lines
security/integrity/digsig.c +4
-6-if (ret != -ENOKEY ) -return ret; - -return restrict_link_by_signature (get_ima_mok_keyring (), - type, payload); -} +#if defined(CONFIG_IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN ) +#define restrict_link_to_ima restrict_link_by_builtin_trusted +#elif defined(CONFIG_IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN_OR_SECONDARY ) +#define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
FYI, restrict_link_by_ima_mok() allows keys to be added to the IMA keyring signed by a key on the .ima_mok keyring, but restrict_link_by_builtin_and_secondary_trusted() results in "errno:
Required key not available (126)".
Mimi
David Howells
maintainerRe: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 4 lines · Mimi Zohar
FYI,
restrict_link_by_ima_mok()allows keys to be added to the IMA keyring signed by a key on the .ima_mok keyring, butrestrict_link_by_builtin_and_secondary_trusted()results in "errno:
Required key not available (126)".
Is that fixed by fixing restrict_link_by_builtin_and_secondary_trusted() to check the right keyring?
David
Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #3]
Quoted 9 lines · David Howells
Quoted 4 lines · Mimi Zohar
FYI,
restrict_link_by_ima_mok()allows keys to be added to the IMA keyring signed by a key on the .ima_mok keyring, butrestrict_link_by_builtin_and_secondary_trusted()results in "errno:
Required key not available (126)".Is that fixed by fixing
restrict_link_by_builtin_and_secondary_trusted()to check the right keyring?
Yes