| Message ID | 20220406015337.4000739-7-eric.snowberg@oracle.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Add CA enforcement keyring restrictions | expand |
Hi Eric,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on 3123109284176b1532874591f7c81f3837bbdc17]
url: https://github.com/intel-lab-lkp/linux/commits/Eric-Snowberg/Add-CA-enforcement-keyring-restrictions/20220407-003209
base: 3123109284176b1532874591f7c81f3837bbdc17
config: um-i386_defconfig (https://download.01.org/0day-ci/archive/20220407/202204070929.nFQNU3B8-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-19) 11.2.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/b0858df3dd6d627f8fa75cc973f55516372a5c98
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Eric-Snowberg/Add-CA-enforcement-keyring-restrictions/20220407-003209
git checkout b0858df3dd6d627f8fa75cc973f55516372a5c98
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=um SUBARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from fs/file_table.c:28:
>> include/linux/ima.h:189:56: warning: 'union key_payload' declared inside parameter list will not be visible outside of this definition or declaration
189 | const union key_payload *payload,
| ^~~~~~~~~~~
>> include/linux/ima.h:188:57: warning: 'struct key_type' declared inside parameter list will not be visible outside of this definition or declaration
188 | const struct key_type *type,
| ^~~~~~~~
vim +189 include/linux/ima.h
179
180 #ifdef CONFIG_ASYMMETRIC_KEY_TYPE
181 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
182 #define ima_validate_builtin_rot restrict_link_by_rot_builtin_and_secondary_trusted
183 #else
184 #define ima_validate_builtin_rot restrict_link_by_rot_builtin_trusted
185 #endif
186 #else
187 static inline int ima_validate_builtin_rot(struct key *dest_keyring,
> 188 const struct key_type *type,
> 189 const union key_payload *payload,
190 struct key *unused){
191 return -EPERM;
192 }
193 #endif
194
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c index 7290e765f46b..9052dd761ea3 100644 --- a/crypto/asymmetric_keys/x509_public_key.c +++ b/crypto/asymmetric_keys/x509_public_key.c @@ -215,8 +215,18 @@ static int x509_key_preparse(struct key_preparsed_payload *prep) prep->payload.data[asym_auth] = cert->sig; prep->description = desc; prep->quotalen = 100; - if (cert->is_kcs_set && cert->self_signed && cert->is_root_ca) - prep->payload_flags |= KEY_ALLOC_ROT; + if (cert->is_kcs_set) { + if (cert->self_signed && cert->is_root_ca) + prep->payload_flags |= KEY_ALLOC_ROT; + /* + * In this case it could be an Intermediate CA. Set + * KEY_MAYBE_ROT for now. If the restriction check + * passes later, the key will be allocated with the + * correct ROT flag. + */ + else if (!cert->self_signed && !cert->is_root_ca) + prep->payload_flags |= KEY_MAYBE_ROT; + } /* We've finished with the certificate */ cert->pub = NULL; diff --git a/include/linux/ima.h b/include/linux/ima.h index 426b1744215e..3f23bccf880a 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -12,6 +12,7 @@ #include <linux/security.h> #include <linux/kexec.h> #include <crypto/hash_info.h> +#include <keys/system_keyring.h> struct linux_binprm; #ifdef CONFIG_IMA @@ -176,6 +177,21 @@ static inline void ima_post_key_create_or_update(struct key *keyring, bool create) {} #endif /* CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS */ +#ifdef CONFIG_ASYMMETRIC_KEY_TYPE +#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING +#define ima_validate_builtin_rot restrict_link_by_rot_builtin_and_secondary_trusted +#else +#define ima_validate_builtin_rot restrict_link_by_rot_builtin_trusted +#endif +#else +static inline int ima_validate_builtin_rot(struct key *dest_keyring, + const struct key_type *type, + const union key_payload *payload, + struct key *unused){ + return -EPERM; +} +#endif + #ifdef CONFIG_IMA_APPRAISE extern bool is_ima_appraise_enabled(void); extern void ima_inode_post_setattr(struct user_namespace *mnt_userns, diff --git a/include/linux/key-type.h b/include/linux/key-type.h index ed0aaad3849b..da09e68903e2 100644 --- a/include/linux/key-type.h +++ b/include/linux/key-type.h @@ -38,6 +38,7 @@ struct key_preparsed_payload { time64_t expiry; /* Expiry time of key */ unsigned int payload_flags; /* Proposed payload flags */ #define KEY_ALLOC_ROT 0x0001 /* Proposed Root of Trust (ROT) key */ +#define KEY_MAYBE_ROT 0x0002 /* Proposed possible Root of Trust key */ } __randomize_layout; typedef int (*request_key_actor_t)(struct key *auth_key, void *aux); diff --git a/security/keys/key.c b/security/keys/key.c index 732bb837fc51..c553040dcc02 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -900,6 +900,11 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, } } + /* Previous restriction check passed therefore try to validate root of trust */ + if ((prep.payload_flags & KEY_MAYBE_ROT) && + !(ima_validate_builtin_rot(keyring, index_key.type, &prep.payload, NULL))) + prep.payload_flags |= KEY_ALLOC_ROT; + /* if we're going to allocate a new key, we're going to have * to modify the keyring */ ret = key_permission(keyring_ref, KEY_NEED_WRITE);
Currently X.509 Intermediate CA certs do not have the builtin root of trust key flag set. Allow intermediate CA certs to be added. Requirements for an intermediate CA include: Usage extension defined as keyCertSign, Basic Constrains for CA is false, and Intermediate CA cert is signed by a current builtin ROT key. Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com> --- crypto/asymmetric_keys/x509_public_key.c | 14 ++++++++++++-- include/linux/ima.h | 16 ++++++++++++++++ include/linux/key-type.h | 1 + security/keys/key.c | 5 +++++ 4 files changed, 34 insertions(+), 2 deletions(-)