Public keys do not need to be appraised by IMA as the restriction on the
IMA/EVM keyrings ensures that a key is loaded only if it is signed with a
key in the primary or secondary keyring.
However, when evm_load_x509() is loaded, appraisal is already enabled and
a valid IMA signature must be added to the EVM key to pass verification.
Since the restriction is applied on both IMA and EVM keyrings, it is safe
to disable appraisal also when the EVM key is loaded. This patch calls
evm_load_x509() inside ima_load_x509() if CONFIG_IMA_LOAD_X509 is defined.
Signed-off-by: Roberto Sassu <[email protected]>
---
security/integrity/iint.c | 2 ++
security/integrity/ima/ima_init.c | 4 ++++
2 files changed, 6 insertions(+)
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index e12c4900510f..4765a266ba96 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -212,7 +212,9 @@ int integrity_kernel_read(struct file *file, loff_t offset,
void __init integrity_load_keys(void)
{
ima_load_x509();
+#ifndef CONFIG_IMA_LOAD_X509
evm_load_x509();
+#endif
}
static int __init integrity_fs_init(void)
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 4902fe7bd570..9d29a1680da8 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -106,6 +106,10 @@ void __init ima_load_x509(void)
ima_policy_flag &= ~unset_flags;
integrity_load_x509(INTEGRITY_KEYRING_IMA, CONFIG_IMA_X509_PATH);
+
+ /* load also EVM key to avoid appraisal */
+ evm_load_x509();
+
ima_policy_flag |= unset_flags;
}
#endif
--
2.17.1
On Thu, 2020-06-18 at 18:01 +0200, Roberto Sassu wrote:
> Public keys do not need to be appraised by IMA as the restriction on the
> IMA/EVM keyrings ensures that a key is loaded only if it is signed with a
> key in the primary or secondary keyring.
>
> However, when evm_load_x509() is loaded, appraisal is already enabled and
> a valid IMA signature must be added to the EVM key to pass verification.
>
> Since the restriction is applied on both IMA and EVM keyrings, it is safe
> to disable appraisal also when the EVM key is loaded. This patch calls
> evm_load_x509() inside ima_load_x509() if CONFIG_IMA_LOAD_X509 is defined.
>
> Signed-off-by: Roberto Sassu <[email protected]>
> ---
> security/integrity/iint.c | 2 ++
> security/integrity/ima/ima_init.c | 4 ++++
> 2 files changed, 6 insertions(+)
>
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index e12c4900510f..4765a266ba96 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -212,7 +212,9 @@ int integrity_kernel_read(struct file *file, loff_t offset,
> void __init integrity_load_keys(void)
> {
> ima_load_x509();
> +#ifndef CONFIG_IMA_LOAD_X509
> evm_load_x509();
> +#endif
> }
>
> static int __init integrity_fs_init(void)
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 4902fe7bd570..9d29a1680da8 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -106,6 +106,10 @@ void __init ima_load_x509(void)
>
> ima_policy_flag &= ~unset_flags;
> integrity_load_x509(INTEGRITY_KEYRING_IMA, CONFIG_IMA_X509_PATH);
> +
> + /* load also EVM key to avoid appraisal */
> + evm_load_x509();
> +
> ima_policy_flag |= unset_flags;
> }
> #endif
As much as possible IMA and EVM should remain independent of each
other. Modifying integrity_load_x509() doesn't help. This looks like
a good reason for calling another EVM function from within IMA.
Mimi
> From: Mimi Zohar [mailto:[email protected]]
> Sent: Friday, August 21, 2020 8:45 PM
> On Thu, 2020-06-18 at 18:01 +0200, Roberto Sassu wrote:
> > Public keys do not need to be appraised by IMA as the restriction on the
> > IMA/EVM keyrings ensures that a key is loaded only if it is signed with a
> > key in the primary or secondary keyring.
> >
> > However, when evm_load_x509() is loaded, appraisal is already enabled
> and
> > a valid IMA signature must be added to the EVM key to pass verification.
> >
> > Since the restriction is applied on both IMA and EVM keyrings, it is safe
> > to disable appraisal also when the EVM key is loaded. This patch calls
> > evm_load_x509() inside ima_load_x509() if CONFIG_IMA_LOAD_X509 is
> defined.
> >
> > Signed-off-by: Roberto Sassu <[email protected]>
> > ---
> > security/integrity/iint.c | 2 ++
> > security/integrity/ima/ima_init.c | 4 ++++
> > 2 files changed, 6 insertions(+)
> >
> > diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> > index e12c4900510f..4765a266ba96 100644
> > --- a/security/integrity/iint.c
> > +++ b/security/integrity/iint.c
> > @@ -212,7 +212,9 @@ int integrity_kernel_read(struct file *file, loff_t
> offset,
> > void __init integrity_load_keys(void)
> > {
> > ima_load_x509();
> > +#ifndef CONFIG_IMA_LOAD_X509
> > evm_load_x509();
> > +#endif
> > }
> >
> > static int __init integrity_fs_init(void)
> > diff --git a/security/integrity/ima/ima_init.c
> b/security/integrity/ima/ima_init.c
> > index 4902fe7bd570..9d29a1680da8 100644
> > --- a/security/integrity/ima/ima_init.c
> > +++ b/security/integrity/ima/ima_init.c
> > @@ -106,6 +106,10 @@ void __init ima_load_x509(void)
> >
> > ima_policy_flag &= ~unset_flags;
> > integrity_load_x509(INTEGRITY_KEYRING_IMA,
> CONFIG_IMA_X509_PATH);
> > +
> > + /* load also EVM key to avoid appraisal */
> > + evm_load_x509();
> > +
> > ima_policy_flag |= unset_flags;
> > }
> > #endif
>
> As much as possible IMA and EVM should remain independent of each
> other. Modifying integrity_load_x509() doesn't help. This looks like
> a good reason for calling another EVM function from within IMA.
Can I add your Reviewed-by?
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
On Mon, 2020-08-31 at 09:44 +0000, Roberto Sassu wrote:
>
> > As much as possible IMA and EVM should remain independent of each
> > other. Modifying integrity_load_x509() doesn't help. This looks like
> > a good reason for calling another EVM function from within IMA.
>
> Can I add your Reviewed-by?
Yes, that's fine.
Mimi