On Tue, 2019-10-15 at 16:17 -0700, Lakshmi Ramasubramanian wrote:
> Add a LSM Hook for key_create_or_update function. The motive behind
> this change is enable subsystems to receive notification when
> a new key is created or updated.
As per Documentation/process/submitting-patches.rst section "2)
Describe your changes", please begin the patch description by
describing the problem.
>
> IMA will be one of the subsystems that will use this hook to measure
> the newly created key.
>
> This change set includes helper functions to check if the keyring
> (to which a new key is being added, for example) is one of
> the trusted keys keyring (builtin, secondary, or platform).
>
> The change set also includes an IMA function that will be called
> from the LSM hook to notify of the newly created key and the keyring
> to which the key is being added to.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@xxxxxxxxxxxxxxxxxxx>
This patch should be broken up even further.[1] In this case to
simplify review, separate defining the new LSM hook from any IMA code.
Different maintainers need to Ack/sign off on these patches.
The new LSM hook patch, with a clear well written patch description,
should be posted on the LSM mailing list as well.
[1] Refer to Documentation/process/submitting-patches.rst section 3)
"Separate your changes".
> ---
> certs/system_keyring.c | 23 +++++++++++++++++++++++
> include/keys/system_keyring.h | 4 ++++
> include/linux/ima.h | 8 ++++++++
> include/linux/lsm_hooks.h | 14 ++++++++++++++
> include/linux/security.h | 10 ++++++++++
> security/integrity/ima/ima.h | 1 +
> security/integrity/ima/ima_main.c | 19 +++++++++++++++++++
> security/keys/key.c | 8 ++++++++
> security/security.c | 11 +++++++++++
> 9 files changed, 98 insertions(+)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 1eba08a1af82..a89d23fb5d9d 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -283,3 +283,26 @@ void __init set_platform_trusted_keys(struct key *keyring)
> platform_trusted_keys = keyring;
> }
> #endif
> +
> +inline bool is_builtin_trusted_keyring(struct key *keyring)
> +{
> + return (keyring == builtin_trusted_keys);
> +}
> +
> +inline bool is_secondary_trusted_keyring(struct key *keyring)
> +{
> + #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> + return (keyring == secondary_trusted_keys);
> + #else
> + return false;
> + #endif
> +}
> +
> +inline bool is_platform_trusted_keyring(struct key *keyring)
> +{
> + #ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
> + return (keyring == platform_trusted_keys);
> + #else
> + return false;
> + #endif
> +}
Why are these functions defined in a new LSM hook patch? Before
posting a patch, please review the patch line by line, making sure
that there isn't anything extraneous.
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index c1a96fdf598b..2517181d8d6c 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -66,4 +66,8 @@ static inline void set_platform_trusted_keys(struct key *keyring)
> }
> #endif
>
> +extern bool is_builtin_trusted_keyring(struct key *keyring);
> +extern bool is_secondary_trusted_keyring(struct key *keyring);
> +extern bool is_platform_trusted_keyring(struct key *keyring);
> +
Not needed in this patch.
Mimi
