| Message ID | 20191018195328.6704-1-nramas@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v0] KEYS: Security LSM Hook for key_create_or_update | expand |
On 10/18/2019 12:53 PM, Lakshmi Ramasubramanian wrote: > Problem Statement: > key_create_or_update function currently does not have > a security LSM hook. The hook is needed to allow security > subsystems to use key create or update information. What security module(s) do you expect to use this? > security_key_alloc LSM hook that is currently available is not > providing enough information about the key (the key payload, > the target keyring, etc.). Also, this LSM hook is only available > for key create. > > Changes made: > Adding a new LSM hook for key key_create_or_update, > security_key_create_or_update, which is called after > => A newly created key is instantiated and linked to the target > keyring (__key_instantiate_and_link). > => An existing key is updated with a new payload (__key_update) > > security_key_create_or_update is passed the target keyring, key, > cred, key creation flags, and a boolean flag indicating whether > the key was newly created or updated. > > Security subsystems can use this hook for handling key create or update. > For example, IMA subsystem can measure the key when it is created or > updated. IMA is not a Linux Security Module. > Testing performed: > * Booted the kernel with this change. > * Executed keyctl tests from the Linux Test Project (LTP) > * Added a new key to a keyring and verified "key create" code path. > => In this case added a key to builtin_trusted_keys keyring. > * Added the same key again and verified "key update" code path. > => Add the same key to builtin_trusted_keys keyring. > => find_key_to_update found the key and LSM hook was > called for key update (create flag set to false). > * Forced the LSM hook (security_key_create_or_update) to > return an error and verified that the key was not added to > the keyring ("keyctl list <keyring>" does not list the key). > > Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> > --- > include/linux/lsm_hooks.h | 13 +++++++ > include/linux/security.h | 10 +++++ > security/keys/key.c | 78 ++++++++++++++++++++++++++++++++++----- > security/security.c | 8 ++++ You don't have a security module that provides this hook. We don't accept interfaces without users. > 4 files changed, 100 insertions(+), 9 deletions(-) > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > index df1318d85f7d..2f2e95df62f3 100644 > --- a/include/linux/lsm_hooks.h > +++ b/include/linux/lsm_hooks.h > @@ -1066,6 +1066,15 @@ > * > * Security hooks affecting all Key Management operations > * > + * @key_create_or_update: > + * Notification of key create or update. > + * @keyring points to the keyring to which the key belongs > + * @key points to the key being created or updated > + * @cred current cred > + * @flags is the allocation flags > + * @create flag set to true if the key was created. > + * flag set to false if the key was updated. > + * Return 0 if permission is granted, -ve error otherwise. > * @key_alloc: > * Permit allocation of a key and assign security data. Note that key does > * not have a serial number assigned at this point. > @@ -1781,6 +1790,9 @@ union security_list_options { > > /* key management security hooks */ > #ifdef CONFIG_KEYS > + int (*key_create_or_update)(struct key *keyring, struct key *key, > + const struct cred *cred, > + unsigned long flags, bool create); > int (*key_alloc)(struct key *key, const struct cred *cred, > unsigned long flags); > void (*key_free)(struct key *key); > @@ -2026,6 +2038,7 @@ struct security_hook_heads { > struct hlist_head xfrm_decode_session; > #endif /* CONFIG_SECURITY_NETWORK_XFRM */ > #ifdef CONFIG_KEYS > + struct hlist_head key_create_or_update; > struct hlist_head key_alloc; > struct hlist_head key_free; > struct hlist_head key_permission; > diff --git a/include/linux/security.h b/include/linux/security.h > index 5f7441abbf42..27e1c0a3057b 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -1672,6 +1672,9 @@ static inline int security_path_chroot(const struct path *path) > #ifdef CONFIG_KEYS > #ifdef CONFIG_SECURITY > > +int security_key_create_or_update(struct key *keyring, struct key *key, > + const struct cred *cred, > + unsigned long flags, bool create); > int security_key_alloc(struct key *key, const struct cred *cred, unsigned long flags); > void security_key_free(struct key *key); > int security_key_permission(key_ref_t key_ref, > @@ -1680,6 +1683,13 @@ int security_key_getsecurity(struct key *key, char **_buffer); > > #else > > +int security_key_create_or_update(struct key *keyring, struct key *key, > + const struct cred *cred, > + unsigned long flags, bool create) > +{ > + return 0; > +} > + > static inline int security_key_alloc(struct key *key, > const struct cred *cred, > unsigned long flags) > diff --git a/security/keys/key.c b/security/keys/key.c > index 764f4c57913e..b913feaf196e 100644 > --- a/security/keys/key.c > +++ b/security/keys/key.c > @@ -781,7 +781,7 @@ static inline key_ref_t __key_update(key_ref_t key_ref, > } > > /** > - * key_create_or_update - Update or create and instantiate a key. > + * __key_create_or_update - Update or create and instantiate a key. > * @keyring_ref: A pointer to the destination keyring with possession flag. > * @type: The type of key. > * @description: The searchable description for the key. > @@ -789,6 +789,8 @@ static inline key_ref_t __key_update(key_ref_t key_ref, > * @plen: The length of @payload. > * @perm: The permissions mask for a new key. > * @flags: The quota flags for a new key. > + * @create: Set to true if the key was newly created. > + * Set to false if the key was updated. > * > * Search the destination keyring for a key of the same description and if one > * is found, update it, otherwise create and instantiate a new one and create a > @@ -805,13 +807,14 @@ static inline key_ref_t __key_update(key_ref_t key_ref, > * On success, the possession flag from the keyring ref will be tacked on to > * the key ref before it is returned. > */ > -key_ref_t key_create_or_update(key_ref_t keyring_ref, > - const char *type, > - const char *description, > - const void *payload, > - size_t plen, > - key_perm_t perm, > - unsigned long flags) > +static key_ref_t __key_create_or_update(key_ref_t keyring_ref, > + const char *type, > + const char *description, > + const void *payload, > + size_t plen, > + key_perm_t perm, > + unsigned long flags, > + bool *create) > { > struct keyring_index_key index_key = { > .description = description, > @@ -936,6 +939,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, > goto error_link_end; > } > > + *create = true; > key_ref = make_key_ref(key, is_key_possessed(keyring_ref)); > > error_link_end: > @@ -948,7 +952,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, > error: > return key_ref; > > - found_matching_key: > +found_matching_key: > /* we found a matching key, so we're going to try to update it > * - we can drop the locks first as we have the key pinned > */ > @@ -964,9 +968,65 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, > } > } > > + *create = false; > key_ref = __key_update(key_ref, &prep); > goto error_free_prep; > } > + > +/** > + * key_create_or_update - Update or create and instantiate a key. > + * @keyring_ref: A pointer to the destination keyring with possession flag. > + * @type: The type of key. > + * @description: The searchable description for the key. > + * @payload: The data to use to instantiate or update the key. > + * @plen: The length of @payload. > + * @perm: The permissions mask for a new key. > + * @flags: The quota flags for a new key. > + * > + * Calls the internal function __key_create_or_update. > + * If successful calls the security LSM hook. > + */ > +key_ref_t key_create_or_update(key_ref_t keyring_ref, > + const char *type, > + const char *description, > + const void *payload, > + size_t plen, > + key_perm_t perm, > + unsigned long flags) > +{ > + key_ref_t key_ref; > + struct key *keyring, *key = NULL; > + int ret = 0; > + bool create = false; > + > + key_ref = __key_create_or_update(keyring_ref, type, description, > + payload, plen, perm, flags, > + &create); > + if (IS_ERR(key_ref)) > + goto out; > + > + keyring = key_ref_to_ptr(keyring_ref); > + key = key_ref_to_ptr(key_ref); > + > + /* let the security module know about > + * the created or updated key. > + */ > + ret = security_key_create_or_update(keyring, key, > + current_cred(), > + flags, create); > + if (ret < 0) > + goto security_error; > + else > + goto out; > + > +security_error: > + key_unlink(keyring, key); > + key_put(key); > + key_ref = ERR_PTR(ret); > + > +out: > + return key_ref; > +} > EXPORT_SYMBOL(key_create_or_update); > > /** > diff --git a/security/security.c b/security/security.c > index 250ee2d76406..fc1e4984fb53 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -2280,6 +2280,14 @@ EXPORT_SYMBOL(security_skb_classify_flow); > > #ifdef CONFIG_KEYS > > +int security_key_create_or_update(struct key *keyring, struct key *key, > + const struct cred *cred, > + unsigned long flags, bool create) > +{ > + return call_int_hook(key_create_or_update, 0, > + keyring, key, cred, flags, create); > +} > + > int security_key_alloc(struct key *key, const struct cred *cred, > unsigned long flags) > {
On 10/18/19 1:25 PM, Casey Schaufler wrote: >> Problem Statement: >> key_create_or_update function currently does not have >> a security LSM hook. The hook is needed to allow security >> subsystems to use key create or update information. > > What security module(s) do you expect to use this? SELinux is one that I can think of - it has hooks for key_alloc, key_free, etc. But does not have one for key_create_or_update. > IMA is not a Linux Security Module. Agree. But ima utilizes LSM to hook into system operations (such as read_file given below). int security_kernel_post_read_file(struct file *file, char *buf, loff_t size, enum kernel_read_file_id id) { int ret; ret = call_int_hook(kernel_post_read_file, 0, file, buf, size, id); if (ret) return ret; return ima_post_read_file(file, buf, size, id); } I am currently working on an ima function to measure keys. The change set I have submitted today is in preparation for that. > You don't have a security module that provides this hook. > We don't accept interfaces without users. Like I have mentioned above, that change in ima will be submitted for review shortly. If you have suggestions for a better way to hook into key create\update that ima can use to measure keys, I'll be happy to investigate that. thanks, -lakshmi
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index df1318d85f7d..2f2e95df62f3 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1066,6 +1066,15 @@ * * Security hooks affecting all Key Management operations * + * @key_create_or_update: + * Notification of key create or update. + * @keyring points to the keyring to which the key belongs + * @key points to the key being created or updated + * @cred current cred + * @flags is the allocation flags + * @create flag set to true if the key was created. + * flag set to false if the key was updated. + * Return 0 if permission is granted, -ve error otherwise. * @key_alloc: * Permit allocation of a key and assign security data. Note that key does * not have a serial number assigned at this point. @@ -1781,6 +1790,9 @@ union security_list_options { /* key management security hooks */ #ifdef CONFIG_KEYS + int (*key_create_or_update)(struct key *keyring, struct key *key, + const struct cred *cred, + unsigned long flags, bool create); int (*key_alloc)(struct key *key, const struct cred *cred, unsigned long flags); void (*key_free)(struct key *key); @@ -2026,6 +2038,7 @@ struct security_hook_heads { struct hlist_head xfrm_decode_session; #endif /* CONFIG_SECURITY_NETWORK_XFRM */ #ifdef CONFIG_KEYS + struct hlist_head key_create_or_update; struct hlist_head key_alloc; struct hlist_head key_free; struct hlist_head key_permission; diff --git a/include/linux/security.h b/include/linux/security.h index 5f7441abbf42..27e1c0a3057b 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1672,6 +1672,9 @@ static inline int security_path_chroot(const struct path *path) #ifdef CONFIG_KEYS #ifdef CONFIG_SECURITY +int security_key_create_or_update(struct key *keyring, struct key *key, + const struct cred *cred, + unsigned long flags, bool create); int security_key_alloc(struct key *key, const struct cred *cred, unsigned long flags); void security_key_free(struct key *key); int security_key_permission(key_ref_t key_ref, @@ -1680,6 +1683,13 @@ int security_key_getsecurity(struct key *key, char **_buffer); #else +int security_key_create_or_update(struct key *keyring, struct key *key, + const struct cred *cred, + unsigned long flags, bool create) +{ + return 0; +} + static inline int security_key_alloc(struct key *key, const struct cred *cred, unsigned long flags) diff --git a/security/keys/key.c b/security/keys/key.c index 764f4c57913e..b913feaf196e 100644 --- a/security/keys/key.c +++ b/security/keys/key.c @@ -781,7 +781,7 @@ static inline key_ref_t __key_update(key_ref_t key_ref, } /** - * key_create_or_update - Update or create and instantiate a key. + * __key_create_or_update - Update or create and instantiate a key. * @keyring_ref: A pointer to the destination keyring with possession flag. * @type: The type of key. * @description: The searchable description for the key. @@ -789,6 +789,8 @@ static inline key_ref_t __key_update(key_ref_t key_ref, * @plen: The length of @payload. * @perm: The permissions mask for a new key. * @flags: The quota flags for a new key. + * @create: Set to true if the key was newly created. + * Set to false if the key was updated. * * Search the destination keyring for a key of the same description and if one * is found, update it, otherwise create and instantiate a new one and create a @@ -805,13 +807,14 @@ static inline key_ref_t __key_update(key_ref_t key_ref, * On success, the possession flag from the keyring ref will be tacked on to * the key ref before it is returned. */ -key_ref_t key_create_or_update(key_ref_t keyring_ref, - const char *type, - const char *description, - const void *payload, - size_t plen, - key_perm_t perm, - unsigned long flags) +static key_ref_t __key_create_or_update(key_ref_t keyring_ref, + const char *type, + const char *description, + const void *payload, + size_t plen, + key_perm_t perm, + unsigned long flags, + bool *create) { struct keyring_index_key index_key = { .description = description, @@ -936,6 +939,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, goto error_link_end; } + *create = true; key_ref = make_key_ref(key, is_key_possessed(keyring_ref)); error_link_end: @@ -948,7 +952,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, error: return key_ref; - found_matching_key: +found_matching_key: /* we found a matching key, so we're going to try to update it * - we can drop the locks first as we have the key pinned */ @@ -964,9 +968,65 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref, } } + *create = false; key_ref = __key_update(key_ref, &prep); goto error_free_prep; } + +/** + * key_create_or_update - Update or create and instantiate a key. + * @keyring_ref: A pointer to the destination keyring with possession flag. + * @type: The type of key. + * @description: The searchable description for the key. + * @payload: The data to use to instantiate or update the key. + * @plen: The length of @payload. + * @perm: The permissions mask for a new key. + * @flags: The quota flags for a new key. + * + * Calls the internal function __key_create_or_update. + * If successful calls the security LSM hook. + */ +key_ref_t key_create_or_update(key_ref_t keyring_ref, + const char *type, + const char *description, + const void *payload, + size_t plen, + key_perm_t perm, + unsigned long flags) +{ + key_ref_t key_ref; + struct key *keyring, *key = NULL; + int ret = 0; + bool create = false; + + key_ref = __key_create_or_update(keyring_ref, type, description, + payload, plen, perm, flags, + &create); + if (IS_ERR(key_ref)) + goto out; + + keyring = key_ref_to_ptr(keyring_ref); + key = key_ref_to_ptr(key_ref); + + /* let the security module know about + * the created or updated key. + */ + ret = security_key_create_or_update(keyring, key, + current_cred(), + flags, create); + if (ret < 0) + goto security_error; + else + goto out; + +security_error: + key_unlink(keyring, key); + key_put(key); + key_ref = ERR_PTR(ret); + +out: + return key_ref; +} EXPORT_SYMBOL(key_create_or_update); /** diff --git a/security/security.c b/security/security.c index 250ee2d76406..fc1e4984fb53 100644 --- a/security/security.c +++ b/security/security.c @@ -2280,6 +2280,14 @@ EXPORT_SYMBOL(security_skb_classify_flow); #ifdef CONFIG_KEYS +int security_key_create_or_update(struct key *keyring, struct key *key, + const struct cred *cred, + unsigned long flags, bool create) +{ + return call_int_hook(key_create_or_update, 0, + keyring, key, cred, flags, create); +} + int security_key_alloc(struct key *key, const struct cred *cred, unsigned long flags) {
Problem Statement: key_create_or_update function currently does not have a security LSM hook. The hook is needed to allow security subsystems to use key create or update information. security_key_alloc LSM hook that is currently available is not providing enough information about the key (the key payload, the target keyring, etc.). Also, this LSM hook is only available for key create. Changes made: Adding a new LSM hook for key key_create_or_update, security_key_create_or_update, which is called after => A newly created key is instantiated and linked to the target keyring (__key_instantiate_and_link). => An existing key is updated with a new payload (__key_update) security_key_create_or_update is passed the target keyring, key, cred, key creation flags, and a boolean flag indicating whether the key was newly created or updated. Security subsystems can use this hook for handling key create or update. For example, IMA subsystem can measure the key when it is created or updated. Testing performed: * Booted the kernel with this change. * Executed keyctl tests from the Linux Test Project (LTP) * Added a new key to a keyring and verified "key create" code path. => In this case added a key to builtin_trusted_keys keyring. * Added the same key again and verified "key update" code path. => Add the same key to builtin_trusted_keys keyring. => find_key_to_update found the key and LSM hook was called for key update (create flag set to false). * Forced the LSM hook (security_key_create_or_update) to return an error and verified that the key was not added to the keyring ("keyctl list <keyring>" does not list the key). Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> --- include/linux/lsm_hooks.h | 13 +++++++ include/linux/security.h | 10 +++++ security/keys/key.c | 78 ++++++++++++++++++++++++++++++++++----- security/security.c | 8 ++++ 4 files changed, 100 insertions(+), 9 deletions(-)