Re: [PATCH v3 2/2] ima: check signature enforcement against cmdline param instead of CONFIG

["Bruno E. O. Meneguele" <brdeoliv@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2608 bytes --]

On 24-10, Mimi Zohar wrote:
> On Tue, 2017-10-24 at 15:37 -0200, Bruno E. O. Meneguele wrote:
> > When the user requests MODULE_CHECK policy and its kernel is compiled
> > with CONFIG_MODULE_SIG_FORCE not set, all modules would not load, just
> > those loaded in initram time. One option the user would have would be
> > set a kernel cmdline param (module.sig_enforce) to true, but the IMA
> > module check code doesn't rely on this value, it checks just
> > CONFIG_MODULE_SIG_FORCE.
> > 
> > This patch solves this problem checking for the exported value of
> > module.sig_enforce cmdline param intead of CONFIG_MODULE_SIG_FORCE,
> > which holds the effective value (CONFIG || param).
> > 
> > Signed-off-by: Bruno E. O. Meneguele <brdeoliv@redhat.com>
> > ---
> >  security/integrity/ima/ima_main.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index e4ab8ef8016e..d11a7fcc5c8b 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -356,12 +356,12 @@ void ima_post_path_mknod(struct dentry *dentry)
> >   */
> >  int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
> >  {
> > +	bool sig_enforce = is_module_sig_enforced();
> > +
> >  	if (!file && read_id == READING_MODULE) {
> 
> The only reason for getting here is that you're using the old module
> load syscall.  Is there a reason for not using the new one, which
> passes the file descriptor?
> 
> thanks,
> 
> Mimi
> 

Basicaly because the way kmod handles compressed (gz/xz) modules. The
way it's today would require major changes in the code or some kind of
memfd_create() + xattrs reassignement in order to finit_module() be used
correctly.

Considering it would take some time to be accepted or even to figure out
the correct way to tackle it, the current IMA module check code works
aside kernel module signature validation, which is fine for now for me,
but has the problem that this patch tries to solve in the
CONFIG_MODULE_SIG_FORCE check (ignoring module.sig_enforce cmdline
param).

> > -#ifndef CONFIG_MODULE_SIG_FORCE
> > -		if ((ima_appraise & IMA_APPRAISE_MODULES) &&
> > +		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
> >  		    (ima_appraise & IMA_APPRAISE_ENFORCE))
> >  			return -EACCES;	/* INTEGRITY_UNKNOWN */
> > -#endif
> >  		return 0;	/* We rely on module signature checking */
> >  	}
> >  	return 0;
> 
> 

-- 
bmeneg 
PGP Key: http://bmeneg.com/pubkey.txt

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]