linux-kernel - Re: [PATCH] crypto/fips: only panic on bad/missing crypto mod signatures

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 2 Jul 2014 09:53:59 -0400
From:	Jarod Wilson <jarod@...hat.com>
To:	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	linux-kernel@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Rusty Russell <rusty@...tcorp.com.au>,
	Stephan Mueller <stephan.mueller@...ec.com>,
	linux-crypto@...r.kernel.org
Subject: Re: [PATCH] crypto/fips: only panic on bad/missing crypto mod
 signatures

On Wed, Jul 02, 2014 at 08:38:50PM +0800, Herbert Xu wrote:
> On Fri, Jun 27, 2014 at 03:12:54PM -0400, Jarod Wilson wrote:
> > Per further discussion with NIST, the requirements for FIPS state that
> > we only need to panic the system on failed kernel module signature checks
> > for crypto subsystem modules. This moves the fips-mode-only module
> > signature check out of the generic module loading code, into the crypto
> > subsystem, at points where we can catch both algorithm module loads and
> > mode module loads. At the same time, make CONFIG_CRYPTO_FIPS dependent on
> > CONFIG_MODULE_SIG, as this is entirely necessary for FIPS mode.
> > 
> > CC: Herbert Xu <herbert@...dor.apana.org.au>
> > CC: "David S. Miller" <davem@...emloft.net>
> > CC: Rusty Russell <rusty@...tcorp.com.au>
> > CC: Stephan Mueller <stephan.mueller@...ec.com>
> > CC: linux-crypto@...r.kernel.org
> > Signed-off-by: Jarod Wilson <jarod@...hat.com>
> > ---
> >  crypto/Kconfig  |  1 +
> >  crypto/algapi.c | 13 +++++++++++++
> >  kernel/module.c |  3 ---
> >  3 files changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > index ce4012a..36402e5 100644
> > --- a/crypto/Kconfig
> > +++ b/crypto/Kconfig
> > @@ -24,6 +24,7 @@ comment "Crypto core or helper"
> >  config CRYPTO_FIPS
> >  	bool "FIPS 200 compliance"
> >  	depends on CRYPTO_ANSI_CPRNG && !CRYPTO_MANAGER_DISABLE_TESTS
> > +	depends on MODULE_SIG
> >  	help
> >  	  This options enables the fips boot option which is
> >  	  required if you want to system to operate in a FIPS 200
> > diff --git a/crypto/algapi.c b/crypto/algapi.c
> > index 7a1ae87..7d228ff 100644
> > --- a/crypto/algapi.c
> > +++ b/crypto/algapi.c
> > @@ -43,6 +43,12 @@ static inline int crypto_set_driver_name(struct crypto_alg *alg)
> >  
> >  static int crypto_check_alg(struct crypto_alg *alg)
> >  {
> > +#ifdef CONFIG_CRYPTO_FIPS
> > +	if (fips_enabled && alg->cra_module && !alg->cra_module->sig_ok)
> > +		panic("Module %s signature verification failed in FIPS mode\n",
> > +		      alg->cra_module->name);
> > +#endif
> 
> Please hide the ugly ifdef in a helper inline function.

Will do.

> > @@ -437,6 +449,7 @@ int crypto_register_template(struct crypto_template *tmpl)
> >  
> >  	list_add(&tmpl->list, &crypto_template_list);
> >  	crypto_notify(CRYPTO_MSG_TMPL_REGISTER, tmpl);
> > +
> >  	err = 0;
> >  out:
> >  	up_write(&crypto_alg_sem);
> 
> While I have no problems with you adding a blank line please don't
> mix such additions with changes of substance.

Accidental, sorry, was sloppy when moving the check from one place in the
function to another. Will remedy that too. v2 coming after some quick
testing.

-- 
Jarod Wilson
jarod@...hat.com

--
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/