linux-kernel - Re: [RFC PATCH] fips: check whether a module registering an alg or template is signed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130206161557.GJ3751@redacted.bos.redhat.com>
Date:	Wed, 6 Feb 2013 11:15:57 -0500
From:	Kyle McMartin <kmcmarti@...hat.com>
To:	Stephan Mueller <stephan.mueller@...ec.com>
Cc:	linux-kernel@...r.kernel.org, David Howells <dhowells@...hat.com>,
	rusty@...tcorp.com.au, jstancek@...hat.com,
	herbert@...dor.hengli.com.au
Subject: Re: [RFC PATCH] fips: check whether a module registering an alg or
 template is signed

On Wed, Feb 06, 2013 at 09:02:46AM +0100, Stephan Mueller wrote:
> On 05.02.2013 23:58:30, +0100, Kyle McMartin <kyle@...hat.com> wrote:
> 
> Hi Kyle,
>

Thanks for the review, Stephan.
 
> Just reading this paragraph, there is one missing puzzle piece: the
> *entire* kernel crypto API must shut down, even if only one kernel
> module with one cipher (or block chaining mode, ...) has a broken signature.
> 
> The overall requirement is: if one self test fails, the entire FIPS
> 140-2 crypto module must become unavailable. (please note and do not get
> confused by the overload of the term "module" -- we have the KOs the
> kernel loads, and we have something called a FIPS 140-2 module which is
> the entire crypto "library" subject to a FIPS 140-2 validation)
> 
> This signature check is one self test required at runtime.
> 
> I added comments inline into the patch.
> 
> > 
> > crypto_sig_check should return 1 (and allow the registration) if any
> > of the following are true:
> > +	if (!crypto_sig_check(alg->cra_module))
> > +		return -EINVAL;
> 
> Instead of an EINVAL, the kernel either must panic(), or a global flag
> is introduced which is evaluated by every kernel crypto API call. If
> that flag is, say, false, none of the kernel crypto API calls must succeed.

Returning -EINVAL means the module does not successfully load, and
nothing is registered. I don't see why you would need to taint or panic,
if nothing untoward actually occured? I don't object to it, if it's
necessary, I just didn't think it was. If Herbert doesn't object to this
patch, I'd move the panic from kernel/module.c to here.

> > +
> >  	return crypto_set_driver_name(alg);
> >  }
> >  
> > @@ -435,6 +438,11 @@ int crypto_register_template(struct crypto_template *tmpl)
> 
> 
> I am wondering whether the modification of these two functions are
> sufficient. As I wrote in a previous email, there are a number of
> register functions the kernel crypto API exports and which are used.
> 

Between these two, I believe all codepaths that could bring in a
mode, cipher, or other cryptographic algorithm are covered.

> >  			goto out;
> >  	}

regards, Kyle
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/