[PATCH] crypto/fips: only panic on bad/missing crypto mod signatures

[PATCH] crypto/fips: only panic on bad/missing crypto mod signatures

[Jarod Wilson]

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@gondor.apana.org.au>
CC: "David S. Miller" <davem@davemloft.net>
CC: Rusty Russell <rusty@rustcorp.com.au>
CC: Stephan Mueller <stephan.mueller@atsec.com>
CC: linux-crypto@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.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
+
 	if (alg->cra_alignmask & (alg->cra_alignmask + 1))
 		return -EINVAL;
 
@@ -430,6 +436,12 @@ int crypto_register_template(struct crypto_template *tmpl)
 
 	down_write(&crypto_alg_sem);
 
+#ifdef CONFIG_CRYPTO_FIPS
+	if (fips_enabled && tmpl->module && !tmpl->module->sig_ok)
+		panic("Module %s signature verification failed in FIPS mode\n",
+		      tmpl->module->name);
+#endif
+
 	list_for_each_entry(q, &crypto_template_list, list) {
 		if (q == tmpl)
 			goto out;
@@ -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);
diff --git a/kernel/module.c b/kernel/module.c
index 81e727c..ec801d5 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2448,9 +2448,6 @@ static int module_sig_check(struct load_info *info)
 	}
 
 	/* Not having a signature is only an error if we're strict. */
-	if (err < 0 && fips_enabled)
-		panic("Module verification failed with error %d in FIPS mode\n",
-		      err);
 	if (err == -ENOKEY && !sig_enforce)
 		err = 0;
 
-- 
1.8.3.1

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

[Jarod Wilson]

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.
...
> --- a/crypto/algapi.c
> +++ b/crypto/algapi.c
...
> @@ -430,6 +436,12 @@ int crypto_register_template(struct crypto_template *tmpl)
>  
>  	down_write(&crypto_alg_sem);
>  
> +#ifdef CONFIG_CRYPTO_FIPS
> +	if (fips_enabled && tmpl->module && !tmpl->module->sig_ok)
> +		panic("Module %s signature verification failed in FIPS mode\n",
> +		      tmpl->module->name);
> +#endif
> +

Forgot to mention: the panic locations within the functions don't really
matter a whole lot right this moment, but Stephan pointed out the
possibility of a future FIPS standard that might not require a panic, thus
the crypto_register_template check being done after the down_write() so
that you could do a goto out; instead of a panic here and have things more
or less behave.

-- 
Jarod Wilson
jarod@redhat.com

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

[Herbert Xu]

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@gondor.apana.org.au>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Rusty Russell <rusty@rustcorp.com.au>
> CC: Stephan Mueller <stephan.mueller@atsec.com>
> CC: linux-crypto@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.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.

> @@ -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.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

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

[Jarod Wilson]

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@gondor.apana.org.au>
> > CC: "David S. Miller" <davem@davemloft.net>
> > CC: Rusty Russell <rusty@rustcorp.com.au>
> > CC: Stephan Mueller <stephan.mueller@atsec.com>
> > CC: linux-crypto@vger.kernel.org
> > Signed-off-by: Jarod Wilson <jarod@redhat.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@redhat.com

[PATCH v2] crypto/fips: only panic on bad/missing crypto mod signatures

[Jarod Wilson]

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.

v2: remove extraneous blank line, perform checks in static inline
function, drop no longer necessary fips.h include.

CC: Herbert Xu <herbert@gondor.apana.org.au>
CC: "David S. Miller" <davem@davemloft.net>
CC: Rusty Russell <rusty@rustcorp.com.au>
CC: Stephan Mueller <stephan.mueller@atsec.com>
CC: linux-crypto@vger.kernel.org
Signed-off-by: Jarod Wilson <jarod@redhat.com>
---
 crypto/Kconfig  |  1 +
 crypto/algapi.c | 14 ++++++++++++++
 kernel/module.c |  4 ----
 3 files changed, 15 insertions(+), 4 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..e8d3a7d 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -41,8 +41,20 @@ static inline int crypto_set_driver_name(struct crypto_alg *alg)
 	return 0;
 }
 
+static inline void crypto_check_module_sig(struct module *mod)
+{
+#ifdef CONFIG_CRYPTO_FIPS
+	if (fips_enabled && mod && !mod->sig_ok)
+		panic("Module %s signature verification failed in FIPS mode\n",
+		      mod->name);
+#endif
+	return;
+}
+
 static int crypto_check_alg(struct crypto_alg *alg)
 {
+	crypto_check_module_sig(alg->cra_module);
+
 	if (alg->cra_alignmask & (alg->cra_alignmask + 1))
 		return -EINVAL;
 
@@ -430,6 +442,8 @@ int crypto_register_template(struct crypto_template *tmpl)
 
 	down_write(&crypto_alg_sem);
 
+	crypto_check_module_sig(tmpl->module);
+
 	list_for_each_entry(q, &crypto_template_list, list) {
 		if (q == tmpl)
 			goto out;
diff --git a/kernel/module.c b/kernel/module.c
index 81e727c..ae79ce6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -60,7 +60,6 @@
 #include <linux/jump_label.h>
 #include <linux/pfn.h>
 #include <linux/bsearch.h>
-#include <linux/fips.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
 
@@ -2448,9 +2447,6 @@ static int module_sig_check(struct load_info *info)
 	}
 
 	/* Not having a signature is only an error if we're strict. */
-	if (err < 0 && fips_enabled)
-		panic("Module verification failed with error %d in FIPS mode\n",
-		      err);
 	if (err == -ENOKEY && !sig_enforce)
 		err = 0;
 
-- 
1.8.3.1

Re: [PATCH v2] crypto/fips: only panic on bad/missing crypto mod signatures

[Neil Horman]

On Wed, Jul 02, 2014 at 03:37:30PM -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.
> 
> v2: remove extraneous blank line, perform checks in static inline
> function, drop no longer necessary fips.h include.
> 
> CC: Herbert Xu <herbert@gondor.apana.org.au>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Rusty Russell <rusty@rustcorp.com.au>
> CC: Stephan Mueller <stephan.mueller@atsec.com>
> CC: linux-crypto@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [PATCH v2] crypto/fips: only panic on bad/missing crypto mod signatures

[Stephan Mueller]

Am Donnerstag, 3. Juli 2014, 07:18:06 schrieb Neil Horman:
>On Wed, Jul 02, 2014 at 03:37:30PM -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.
>> 
>> v2: remove extraneous blank line, perform checks in static inline
>> function, drop no longer necessary fips.h include.
>> 
>> CC: Herbert Xu <herbert@gondor.apana.org.au>
>> CC: "David S. Miller" <davem@davemloft.net>
>> CC: Rusty Russell <rusty@rustcorp.com.au>
>> CC: Stephan Mueller <stephan.mueller@atsec.com>
>> CC: linux-crypto@vger.kernel.org
>> Signed-off-by: Jarod Wilson <jarod@redhat.com>
>
>Acked-by: Neil Horman <nhorman@tuxdriver.com>
Acked-by: Stephan Mueller <stephan.mueller@atsec.com>

Re: [PATCH v2] crypto/fips: only panic on bad/missing crypto mod signatures

[Herbert Xu]

On Wed, Jul 02, 2014 at 03:37:30PM -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.
> 
> v2: remove extraneous blank line, perform checks in static inline
> function, drop no longer necessary fips.h include.
> 
> CC: Herbert Xu <herbert@gondor.apana.org.au>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Rusty Russell <rusty@rustcorp.com.au>
> CC: Stephan Mueller <stephan.mueller@atsec.com>
> CC: linux-crypto@vger.kernel.org
> Signed-off-by: Jarod Wilson <jarod@redhat.com>

Patch applied.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt