| Message ID | 20200618160133.937-2-roberto.sassu@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/11] evm: Execute evm_inode_init_security() only when the HMAC key is loaded | expand |
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 <roberto.sassu@huawei.com> > --- > 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:zohar@linux.ibm.com] > 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 <roberto.sassu@huawei.com> > > --- > > 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
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
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 <roberto.sassu@huawei.com> --- security/integrity/iint.c | 2 ++ security/integrity/ima/ima_init.c | 4 ++++ 2 files changed, 6 insertions(+)