* [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set @ 2013-01-22 18:43 Kyle McMartin 2013-01-22 23:17 ` Rusty Russell 2013-01-23 11:26 ` David Howells 0 siblings, 2 replies; 22+ messages in thread From: Kyle McMartin @ 2013-01-22 18:43 UTC (permalink / raw) To: linux-kernel; +Cc: dhowells, rusty, jstancek Commit 1d0059f3a added a test to check if the system is booted in fips mode, and if so, panic the system if an unsigned module is loaded. However the wording of the changelog "in signature enforcing mode" leads one to assume that sig_enforce should be set for the panic to occur and that these two tests are transposed. Move the test for -ENOKEY && !sig_enforce before the test of fips_mode, so that err will be 0, and the panic will not trigger unless we've explicitly disabled unsigned modules with sig_enforce set, so that systemtap and 3rd party modules will work in fips mode. (This also matches the behaviour by Red Hat Enterprise Linux 6.) Things which need to deny module loading such as secure boot already set sig_enforce, so there's no issue here. Reported-by: Jan Stancek <jstancek@redhat.com> Signed-off-by: Kyle McMartin <kyle@redhat.com> --- a/kernel/module.c +++ b/kernel/module.c @@ -2460,11 +2460,11 @@ static int module_sig_check(struct load_info *info) } /* Not having a signature is only an error if we're strict. */ + if (err == -ENOKEY && !sig_enforce) + err = 0; if (err < 0 && fips_enabled) panic("Module verification failed with error %d in FIPS mode\n", err); - if (err == -ENOKEY && !sig_enforce) - err = 0; return err; } ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set 2013-01-22 18:43 [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set Kyle McMartin @ 2013-01-22 23:17 ` Rusty Russell 2013-01-23 11:26 ` David Howells 1 sibling, 0 replies; 22+ messages in thread From: Rusty Russell @ 2013-01-22 23:17 UTC (permalink / raw) To: Kyle McMartin, linux-kernel; +Cc: dhowells, jstancek Kyle McMartin <kyle@redhat.com> writes: > Commit 1d0059f3a added a test to check if the system is booted in fips > mode, and if so, panic the system if an unsigned module is loaded. > However the wording of the changelog "in signature enforcing mode" leads > one to assume that sig_enforce should be set for the panic to occur and > that these two tests are transposed. > > Move the test for -ENOKEY && !sig_enforce before the test of fips_mode, > so that err will be 0, and the panic will not trigger unless we've > explicitly disabled unsigned modules with sig_enforce set, so that > systemtap and 3rd party modules will work in fips mode. (This also > matches the behaviour by Red Hat Enterprise Linux 6.) > > Things which need to deny module loading such as secure boot already set > sig_enforce, so there's no issue here. > > Reported-by: Jan Stancek <jstancek@redhat.com> > Signed-off-by: Kyle McMartin <kyle@redhat.com> Seems reasonable, but I'll want David Howells' Ack. Thanks, Rusty. > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -2460,11 +2460,11 @@ static int module_sig_check(struct load_info *info) > } > > /* Not having a signature is only an error if we're strict. */ > + if (err == -ENOKEY && !sig_enforce) > + err = 0; > if (err < 0 && fips_enabled) > panic("Module verification failed with error %d in FIPS mode\n", > err); > - if (err == -ENOKEY && !sig_enforce) > - err = 0; > > return err; > } ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set 2013-01-22 18:43 [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set Kyle McMartin 2013-01-22 23:17 ` Rusty Russell @ 2013-01-23 11:26 ` David Howells 2013-01-23 15:18 ` Stephan Mueller 1 sibling, 1 reply; 22+ messages in thread From: David Howells @ 2013-01-23 11:26 UTC (permalink / raw) To: Kyle McMartin; +Cc: dhowells, linux-kernel, rusty, jstancek, Stephan Mueller Kyle McMartin <kyle@redhat.com> wrote: > Commit 1d0059f3a added a test to check if the system is booted in fips > mode, and if so, panic the system if an unsigned module is loaded. > However the wording of the changelog "in signature enforcing mode" leads > one to assume that sig_enforce should be set for the panic to occur and > that these two tests are transposed. > > Move the test for -ENOKEY && !sig_enforce before the test of fips_mode, > so that err will be 0, and the panic will not trigger unless we've > explicitly disabled unsigned modules with sig_enforce set, so that > systemtap and 3rd party modules will work in fips mode. (This also > matches the behaviour by Red Hat Enterprise Linux 6.) > > Things which need to deny module loading such as secure boot already set > sig_enforce, so there's no issue here. > > Reported-by: Jan Stancek <jstancek@redhat.com> > Signed-off-by: Kyle McMartin <kyle@redhat.com> Fine by me, but adding Stephan Mueller for his input. David > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -2460,11 +2460,11 @@ static int module_sig_check(struct load_info *info) > } > > /* Not having a signature is only an error if we're strict. */ > + if (err == -ENOKEY && !sig_enforce) > + err = 0; > if (err < 0 && fips_enabled) > panic("Module verification failed with error %d in FIPS mode\n", > err); > - if (err == -ENOKEY && !sig_enforce) > - err = 0; > > return err; > } ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set 2013-01-23 11:26 ` David Howells @ 2013-01-23 15:18 ` Stephan Mueller 2013-01-24 14:59 ` Kyle McMartin ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Stephan Mueller @ 2013-01-23 15:18 UTC (permalink / raw) To: David Howells; +Cc: Kyle McMartin, linux-kernel, rusty, jstancek On 23.01.2013 12:26:15, +0100, David Howells <dhowells@redhat.com> wrote: Hi David, > Kyle McMartin <kyle@redhat.com> wrote: > >> Commit 1d0059f3a added a test to check if the system is booted in fips >> mode, and if so, panic the system if an unsigned module is loaded. >> However the wording of the changelog "in signature enforcing mode" leads >> one to assume that sig_enforce should be set for the panic to occur and >> that these two tests are transposed. >> >> Move the test for -ENOKEY && !sig_enforce before the test of fips_mode, >> so that err will be 0, and the panic will not trigger unless we've >> explicitly disabled unsigned modules with sig_enforce set, so that >> systemtap and 3rd party modules will work in fips mode. (This also >> matches the behaviour by Red Hat Enterprise Linux 6.) >> >> Things which need to deny module loading such as secure boot already set >> sig_enforce, so there's no issue here. >> >> Reported-by: Jan Stancek <jstancek@redhat.com> >> Signed-off-by: Kyle McMartin <kyle@redhat.com> > > Fine by me, but adding Stephan Mueller for his input. FIPS requires the module (in our case the static kernel binary with its kernel crypto API plus all the crypto kernel modules) to be unavailable if the module signature fails. That is an unconditional requirement. Now, all is a name game from here on. With your patch, the FIPS mode is only enabled if both flags, i.e. fips_enabled *and* sig_enforce are set! IMHO this is very misleading of the fips_enabled flag which is intended to be the only trigger for the FIPS mode. Hence, I would NACK the patch -- only from this point of view (i.e. I do not have a technical argument against the patch)! The solution to the problem IMHO is rather to somehow identify crypto modules, i.e. modules that hook themselves into the kernel crypto API and only panic the kernel when those integrity checks fail. Therefore, to remove the panic() call in the module loading function when fips_enabled is set would entail to: 1. load and sig check the module as it is done now 2. remember whether the signature check passed or failed for the loaded module 3. in the cipher initialization code of the crypto API (i.e. the one behind crypto_register_alg()), you check the signature check flag -- panic the kernel when the flag shows that the signature check failed This way you limit the panic on signature checks in FIPS mode to the crypto modules. > > David > >> --- a/kernel/module.c >> +++ b/kernel/module.c >> @@ -2460,11 +2460,11 @@ static int module_sig_check(struct load_info *info) >> } >> >> /* Not having a signature is only an error if we're strict. */ >> + if (err == -ENOKEY && !sig_enforce) >> + err = 0; >> if (err < 0 && fips_enabled) >> panic("Module verification failed with error %d in FIPS mode\n", >> err); >> - if (err == -ENOKEY && !sig_enforce) >> - err = 0; >> >> return err; >> } > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set 2013-01-23 15:18 ` Stephan Mueller @ 2013-01-24 14:59 ` Kyle McMartin 2013-01-25 11:28 ` Stephan Mueller 2013-01-24 19:06 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned Kyle McMartin 2013-01-25 0:14 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned David Howells 2 siblings, 1 reply; 22+ messages in thread From: Kyle McMartin @ 2013-01-24 14:59 UTC (permalink / raw) To: Stephan Mueller; +Cc: David Howells, linux-kernel, rusty, jstancek On Wed, Jan 23, 2013 at 04:18:32PM +0100, Stephan Mueller wrote: > 3. in the cipher initialization code of the crypto API (i.e. the one > behind crypto_register_alg()), you check the signature check flag -- > panic the kernel when the flag shows that the signature check failed > > This way you limit the panic on signature checks in FIPS mode to the > crypto modules. > I was hoping we could just do what we do for driver/staging and set a flag in modpost for crypto modules, but it looks like since we have crypto modules outside of crypto/ for things like aesni, that won't work. Maybe that is a better choice, but it seems like an awful kludge. --Kyle ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set 2013-01-24 14:59 ` Kyle McMartin @ 2013-01-25 11:28 ` Stephan Mueller 0 siblings, 0 replies; 22+ messages in thread From: Stephan Mueller @ 2013-01-25 11:28 UTC (permalink / raw) To: Kyle McMartin; +Cc: David Howells, linux-kernel, rusty, jstancek On 24.01.2013 15:59:07, +0100, Kyle McMartin <kmcmarti@redhat.com> wrote: Hi Kyle, > On Wed, Jan 23, 2013 at 04:18:32PM +0100, Stephan Mueller wrote: >> 3. in the cipher initialization code of the crypto API (i.e. the one >> behind crypto_register_alg()), you check the signature check flag -- >> panic the kernel when the flag shows that the signature check failed >> >> This way you limit the panic on signature checks in FIPS mode to the >> crypto modules. >> > > I was hoping we could just do what we do for driver/staging and set a > flag in modpost for crypto modules, but it looks like since we have > crypto modules outside of crypto/ for things like aesni, that won't > work. Maybe that is a better choice, but it seems like an awful kludge. There is a completely different possibility though: The reason for the panic is the requirement that if one of the integrity tests for the crypto components fail in FIPS mode, the crypto module (i.e. the entire kernel crypto API) must be unavailable. Thus, another possibility is to add a global flag that indicates whether such a test failed. That flag is evaluated by *every* kernel crypto API interface function and this function rejects to do anything when that flag is set. This way, we can get rid of the panic. But then, we still have the problem of loading unsigned kernel modules in FIPS mode that are not related to crypto: those would still trigger setting that global flag if the module loader is unable to identify a crypto module. Hence, you idea with a flag may be the way to go: add such a crypto flag similar to the MODULE_LICENSE("GPL") for modules. Now, the module loader should only panic or set such a global flag for modules that are marked with something like MODULE_TYPE("CRYPTO"). Now, all modules in crypto/ and arch/*/crypto/* (or whereever) would have such a flag added. > > --Kyle > Ciao Stephan ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-23 15:18 ` Stephan Mueller 2013-01-24 14:59 ` Kyle McMartin @ 2013-01-24 19:06 ` Kyle McMartin 2013-01-24 19:21 ` Kyle McMartin ` (2 more replies) 2013-01-25 0:14 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned David Howells 2 siblings, 3 replies; 22+ messages in thread From: Kyle McMartin @ 2013-01-24 19:06 UTC (permalink / raw) To: linux-kernel; +Cc: David Howells, rusty, jstancek, Stephan Mueller After thinking about it a while, this seems like the best way to solve the problem, although it does still kind of offend my delicate sensibilities... Doing this check in the crypto layer seems kind of like a layering violation to me (and, to be honest, I think it'd be a gross-hack getting from the callee back to the caller module.) Instead, doing it in kernel/module.c and looking at the undefined symbol list again looks to me like an ordering problem since we're doing the signature check before we've even done the elf header check. We'd have to move the panic to a part of the module code that may not necessarily make sense. Whereas checking the undefined symbol list at modpost time is fairly logical, and adding a new MODULE_INFO entry in the CONFIG_CRYPTO_FIPS case is a well understood thing to do, and should catch all the crypto registrations regardless of where they exist in-tree... Seems to build and work with both values of CONFIG_CRYPTO_FIPS. Thoughts? Signed-off-by: Kyle McMartin <kyle@redhat.com> --- a/kernel/module.c +++ b/kernel/module.c @@ -2459,8 +2459,10 @@ static int module_sig_check(struct load_info *info) return 0; } - /* Not having a signature is only an error if we're strict. */ - if (err < 0 && fips_enabled) + /* Not having a signature is only an error if we're strict, and + * the module registers a crypto algorithm (checked in modpost) + */ + if (err < 0 && fips_enabled && !get_modinfo(info, "crypto_fips")) panic("Module verification failed with error %d in FIPS mode\n", err); if (err == -ENOKEY && !sig_enforce) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index ff36c50..79f46e2 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -1888,6 +1888,23 @@ static void add_staging_flag(struct buffer *b, const char *name) buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n"); } +static void add_crypto_flag(struct buffer *b, struct module *mod) +{ +#if defined(CONFIG_CRYPTO_FIPS) + struct symbol *s; + + /* iterate unresolved symbols looking for... + * - crypto_register_algs + */ + for (s = mod->unres; s; s = s->next) { + if (strcmp("crypto_register_algs", s->name) == 0) + buf_printf(b, "\nMODULE_INFO(crypto_fips, \"Y\");\n"); + } +#else + return; +#endif /*CONFIG_CRYPTO_FIPS*/ +} + /** * Record CRCs for unresolved symbols **/ @@ -2202,6 +2219,7 @@ int main(int argc, char **argv) add_header(&buf, mod); add_intree_flag(&buf, !external_module); add_staging_flag(&buf, mod->name); + add_crypto_flag(&buf, mod); err |= add_versions(&buf, mod); add_depends(&buf, mod, modules); add_moddevtable(&buf, mod); ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-24 19:06 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned Kyle McMartin @ 2013-01-24 19:21 ` Kyle McMartin 2013-01-24 23:36 ` Rusty Russell 2013-01-25 12:18 ` Stephan Mueller 2 siblings, 0 replies; 22+ messages in thread From: Kyle McMartin @ 2013-01-24 19:21 UTC (permalink / raw) To: linux-kernel On Thu, Jan 24, 2013 at 02:06:10PM -0500, Kyle McMartin wrote: > + if (err < 0 && fips_enabled && !get_modinfo(info, "crypto_fips")) Sigh, that should be get_modinfo(...) if (err < 0 && fips_enabled && get_modinfo(info, "crypto_fips")) Thinko when converting from flagging things as "nocrypto" to the above, since it touches far fewer .ko --Kyle ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-24 19:06 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned Kyle McMartin 2013-01-24 19:21 ` Kyle McMartin @ 2013-01-24 23:36 ` Rusty Russell 2013-01-25 5:45 ` Kyle McMartin ` (2 more replies) 2013-01-25 12:18 ` Stephan Mueller 2 siblings, 3 replies; 22+ messages in thread From: Rusty Russell @ 2013-01-24 23:36 UTC (permalink / raw) To: Kyle McMartin, linux-kernel; +Cc: David Howells, jstancek, Stephan Mueller Kyle McMartin <kyle@redhat.com> writes: > After thinking about it a while, this seems like the best way to solve > the problem, although it does still kind of offend my delicate > sensibilities... You're far too polite. This patch was horrible, partial and ugly. Stephan Mueller <stephan.mueller@atsec.com> wrote: > FIPS requires the module (in our case the static kernel binary with its > kernel crypto API plus all the crypto kernel modules) to be unavailable > if the module signature fails. That is an unconditional requirement. "the module signature" here being the signature of any crypto module, I'm guessing from Kyle's awful patch. Any crypto module, or just some? Presumably any module used by any crypto module, too? Because you can panic when a !sig_ok module registers a crypto algorithm. Or you can panic when anyone registers a crypto algorithm after any module has failed the signature check. But it doesn't make much sense to pick on the crypto modules, since they're not well isolated from the rest of the kernel. Thanks, Rusty. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-24 23:36 ` Rusty Russell @ 2013-01-25 5:45 ` Kyle McMartin 2013-01-25 12:42 ` Stephan Mueller 2013-01-25 12:46 ` Stephan Mueller 2 siblings, 0 replies; 22+ messages in thread From: Kyle McMartin @ 2013-01-25 5:45 UTC (permalink / raw) To: Rusty Russell; +Cc: linux-kernel, David Howells, jstancek, Stephan Mueller On Fri, Jan 25, 2013 at 10:06:01AM +1030, Rusty Russell wrote: > Kyle McMartin <kyle@redhat.com> writes: > > After thinking about it a while, this seems like the best way to solve > > the problem, although it does still kind of offend my delicate > > sensibilities... > > You're far too polite. This patch was horrible, partial and ugly. > Yeah, fair enough. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-24 23:36 ` Rusty Russell 2013-01-25 5:45 ` Kyle McMartin @ 2013-01-25 12:42 ` Stephan Mueller 2013-02-03 23:34 ` Rusty Russell 2013-01-25 12:46 ` Stephan Mueller 2 siblings, 1 reply; 22+ messages in thread From: Stephan Mueller @ 2013-01-25 12:42 UTC (permalink / raw) To: Rusty Russell; +Cc: Kyle McMartin, linux-kernel, David Howells, jstancek On 25.01.2013 00:36:01, +0100, Rusty Russell <rusty@rustcorp.com.au> wrote: Hi Rusty, > Kyle McMartin <kyle@redhat.com> writes: >> After thinking about it a while, this seems like the best way to solve >> the problem, although it does still kind of offend my delicate >> sensibilities... > > You're far too polite. This patch was horrible, partial and ugly. Well, in another email I suggested you may want to think about some marker that the code of the module would contain, similar to the GPL flag for the module (whose absence sets the tainted flag). > > Stephan Mueller <stephan.mueller@atsec.com> wrote: >> FIPS requires the module (in our case the static kernel binary with its >> kernel crypto API plus all the crypto kernel modules) to be unavailable >> if the module signature fails. That is an unconditional requirement. > > "the module signature" here being the signature of any crypto module, > I'm guessing from Kyle's awful patch. Any crypto module, or just some? > Presumably any module used by any crypto module, too? Any module loading into the kernel crypto API must be caught and its signature enforced. Thus Kyle's approach to catch the kernel crypto API register function would be appropriate, if indeed we would catch all crypto KOs that we want to catch -- see my remark to Kyle. > > Because you can panic when a !sig_ok module registers a crypto > algorithm. Or you can panic when anyone registers a crypto algorithm > after any module has failed the signature check. I do not see the difference from a FIPS perspective. The crypto KO is unavailable to any crypto user until it called the kernel crypto API register function. Thus, if a KO is loaded, its signature check failed and now lingers in the kernel, I do not see how the main FIPS requirement is offended. Only when the code in the KO registers with the crypto API, that is the latest point when the kernel must stop that KO whose signature check failed -- again from a FIPS perspective. > > But it doesn't make much sense to pick on the crypto modules, since > they're not well isolated from the rest of the kernel. Technically, I am totally with you. That business of integrity check of a subset of code that is loaded into the kernel realm is hard to grasp, if you want to stop an attacker. But that is not the focus of the FIPS test here. That test shall counter accidental modifications (how unlikely they are). And I am fully aware of the fact that this FIPS requirement does not make too much sense in software implementations. Note, FIPS 140-2 mainly focuses on hardware and has some requirements which are totally bogus for software -- this is one of them. Well, but if we want to be FIPS 140-2 compliant, either we meet that requirement, or, well, you are not compliant. It is that easy. :-) Ciao Stephan > > Thanks, > Rusty. > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-25 12:42 ` Stephan Mueller @ 2013-02-03 23:34 ` Rusty Russell 0 siblings, 0 replies; 22+ messages in thread From: Rusty Russell @ 2013-02-03 23:34 UTC (permalink / raw) To: Stephan Mueller; +Cc: Kyle McMartin, linux-kernel, David Howells, jstancek Stephan Mueller <stephan.mueller@atsec.com> writes: > On 25.01.2013 00:36:01, +0100, Rusty Russell <rusty@rustcorp.com.au> wrote: >> "the module signature" here being the signature of any crypto module, >> I'm guessing from Kyle's awful patch. Any crypto module, or just some? >> Presumably any module used by any crypto module, too? > > Any module loading into the kernel crypto API must be caught and its > signature enforced. Thus Kyle's approach to catch the kernel crypto API > register function would be appropriate, if indeed we would catch all > crypto KOs that we want to catch -- see my remark to Kyle. OK, so perhaps in fips mode we should fail the various crypto register calls if the kernel is tainted? > But that is not the focus of the FIPS test here. That test shall counter > accidental modifications (how unlikely they are). And I am fully aware > of the fact that this FIPS requirement does not make too much sense in > software implementations. Note, FIPS 140-2 mainly focuses on hardware > and has some requirements which are totally bogus for software -- this > is one of them. > > Well, but if we want to be FIPS 140-2 compliant, either we meet that > requirement, or, well, you are not compliant. It is that easy. :-) Two important principles here: 1) Ugliness and craziness must be contained in the subsystem which cares. 2) Minimize effort spent on craziness. Principle #1 means I want this in the crypto subsystem, not the module subsystem. Cheers, Rusty. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-24 23:36 ` Rusty Russell 2013-01-25 5:45 ` Kyle McMartin 2013-01-25 12:42 ` Stephan Mueller @ 2013-01-25 12:46 ` Stephan Mueller 2 siblings, 0 replies; 22+ messages in thread From: Stephan Mueller @ 2013-01-25 12:46 UTC (permalink / raw) To: Rusty Russell; +Cc: Kyle McMartin, linux-kernel, David Howells, jstancek On 25.01.2013 00:36:01, +0100, Rusty Russell <rusty@rustcorp.com.au> wrote: Hi Rusty at al, while we are at FIPS discussions, may I propose a slight fix because the FIPS mode is not covering the FIPS 200 (a management system set of requirements), but FIPS 140-2 covering implementation requirements for cryptography. Signed-off-by: Stephan Mueller <smueller@chronox.de> --- Kconfig.orig 2013-01-25 13:42:54.649705380 +0100 +++ Kconfig 2013-01-25 13:43:16.737705712 +0100 @@ -22,11 +22,11 @@ comment "Crypto core or helper" config CRYPTO_FIPS - bool "FIPS 200 compliance" + bool "FIPS 140-2 compliance" depends on CRYPTO_ANSI_CPRNG && !CRYPTO_MANAGER_DISABLE_TESTS help This options enables the fips boot option which is - required if you want to system to operate in a FIPS 200 + required if you want to system to operate in a FIPS 140-2 certification. You should say no unless you know what this is. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-24 19:06 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned Kyle McMartin 2013-01-24 19:21 ` Kyle McMartin 2013-01-24 23:36 ` Rusty Russell @ 2013-01-25 12:18 ` Stephan Mueller 2013-02-05 22:58 ` [RFC PATCH] fips: check whether a module registering an alg or template is signed Kyle McMartin 2 siblings, 1 reply; 22+ messages in thread From: Stephan Mueller @ 2013-01-25 12:18 UTC (permalink / raw) To: Kyle McMartin; +Cc: linux-kernel, David Howells, rusty, jstancek On 24.01.2013 20:06:10, +0100, Kyle McMartin <kyle@redhat.com> wrote: Hi Kyle, > After thinking about it a while, this seems like the best way to solve > the problem, although it does still kind of offend my delicate > sensibilities... > > Doing this check in the crypto layer seems kind of like a layering > violation to me (and, to be honest, I think it'd be a gross-hack getting > from the callee back to the caller module.) > > Instead, doing it in kernel/module.c and looking at the undefined symbol > list again looks to me like an ordering problem since we're doing the > signature check before we've even done the elf header check. We'd have > to move the panic to a part of the module code that may not necessarily > make sense. > > Whereas checking the undefined symbol list at modpost time is fairly > logical, and adding a new MODULE_INFO entry in the CONFIG_CRYPTO_FIPS > case is a well understood thing to do, and should catch all the crypto > registrations regardless of where they exist in-tree... > > Seems to build and work with both values of CONFIG_CRYPTO_FIPS. > > Thoughts? This patch is absolutely ok with FIPS requirements. But I think that patch falls short, because the crypto API offers other register functions: $ pwd .../include/crypto $ grep -r crypto_register * algapi.h:int crypto_register_template(struct crypto_template *tmpl); algapi.h:int crypto_register_instance(struct crypto_template *tmpl, internal/compress.h:extern int crypto_register_pcomp(struct pcomp_alg *alg); internal/hash.h:int crypto_register_ahash(struct ahash_alg *alg); internal/hash.h:int crypto_register_shash(struct shash_alg *alg); internal/hash.h:int crypto_register_shashes(struct shash_alg *algs, int count); $ pwd ...crypto $ grep -r crypto_register algapi.c | grep EXPORT EXPORT_SYMBOL_GPL(crypto_register_alg); EXPORT_SYMBOL_GPL(crypto_register_algs); EXPORT_SYMBOL_GPL(crypto_register_template); EXPORT_SYMBOL_GPL(crypto_register_instance); EXPORT_SYMBOL_GPL(crypto_register_notifier); So, please add these calls to your code too. Though, I have one concern that it does not catches all circumstances. What happens, if a crypto algo implementation consists of two KOs where one KO has the register function and the other KO implements other aspects of the cipher? In this case, both KOs must be covered with the check. Can that happen in general? Also, currently lib/sha1.c and lib/md5.c are statically compiled -- so we are fine. Though, is it possible that they may be offloaded into KOs in the future? If yes, they would need to be covered too. Ciao Stephan > > Signed-off-by: Kyle McMartin <kyle@redhat.com> > > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -2459,8 +2459,10 @@ static int module_sig_check(struct load_info *info) > return 0; > } > > - /* Not having a signature is only an error if we're strict. */ > - if (err < 0 && fips_enabled) > + /* Not having a signature is only an error if we're strict, and > + * the module registers a crypto algorithm (checked in modpost) > + */ > + if (err < 0 && fips_enabled && !get_modinfo(info, "crypto_fips")) > panic("Module verification failed with error %d in FIPS mode\n", > err); > if (err == -ENOKEY && !sig_enforce) > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index ff36c50..79f46e2 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -1888,6 +1888,23 @@ static void add_staging_flag(struct buffer *b, const char *name) > buf_printf(b, "\nMODULE_INFO(staging, \"Y\");\n"); > } > > +static void add_crypto_flag(struct buffer *b, struct module *mod) > +{ > +#if defined(CONFIG_CRYPTO_FIPS) > + struct symbol *s; > + > + /* iterate unresolved symbols looking for... > + * - crypto_register_algs > + */ > + for (s = mod->unres; s; s = s->next) { > + if (strcmp("crypto_register_algs", s->name) == 0) > + buf_printf(b, "\nMODULE_INFO(crypto_fips, \"Y\");\n"); > + } > +#else > + return; > +#endif /*CONFIG_CRYPTO_FIPS*/ > +} > + > /** > * Record CRCs for unresolved symbols > **/ > @@ -2202,6 +2219,7 @@ int main(int argc, char **argv) > add_header(&buf, mod); > add_intree_flag(&buf, !external_module); > add_staging_flag(&buf, mod->name); > + add_crypto_flag(&buf, mod); > err |= add_versions(&buf, mod); > add_depends(&buf, mod, modules); > add_moddevtable(&buf, mod); > ^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC PATCH] fips: check whether a module registering an alg or template is signed 2013-01-25 12:18 ` Stephan Mueller @ 2013-02-05 22:58 ` Kyle McMartin 2013-02-06 8:02 ` Stephan Mueller 0 siblings, 1 reply; 22+ messages in thread From: Kyle McMartin @ 2013-02-05 22:58 UTC (permalink / raw) To: Stephan Mueller; +Cc: linux-kernel, David Howells, rusty, jstancek, herbert fips mode needs to prevent unsigned modules from registering crypto algorithms, and currently panics if an unsigned module is attempted to be loaded. Instead, forbid (by returning -EINVAL) registering a crypto alg or template if fips mode is enabled and the module signature is not valid. crypto_sig_check should return 1 (and allow the registration) if any of the following are true: 1/ fips is not enabled (but CONFIG_CRYPTO_FIPS is enabled.) 2/ the algorithm is built into the kernel (THIS_MODULE == NULL) 3/ the algorithm is in a module, and the module sig check passes and fail in any of the other cases. Checking in crypto_check_alg and crypto_register_template seems to hit the callpoints as far as I can see. Signed-off-by: Kyle McMartin <kyle@redhat.com> --- rusty, How about something like this? It keeps the FIPS mess in the crypto/fips.c file (aside from something that goes away entirely in the !CONFIG_CRYPTO_FIPS case.) regards, Kyle --- a/crypto/algapi.c +++ b/crypto/algapi.c @@ -52,6 +52,9 @@ static int crypto_check_alg(struct crypto_alg *alg) if (alg->cra_priority < 0) return -EINVAL; + if (!crypto_sig_check(alg->cra_module)) + return -EINVAL; + return crypto_set_driver_name(alg); } @@ -435,6 +438,11 @@ int crypto_register_template(struct crypto_template *tmpl) goto out; } + if (!crypto_sig_check(tmpl->module)) { + err = -EINVAL; + goto out; + } + list_add(&tmpl->list, &crypto_template_list); crypto_notify(CRYPTO_MSG_TMPL_REGISTER, tmpl); err = 0; diff --git a/crypto/fips.c b/crypto/fips.c index 5539700..2ebbe0f 100644 --- a/crypto/fips.c +++ b/crypto/fips.c @@ -15,6 +15,19 @@ int fips_enabled; EXPORT_SYMBOL_GPL(fips_enabled); +/* forbid loading modules in fips mode if the module is not signed */ +int crypto_sig_check(struct module *m) +{ +#if defined(CONFIG_MODULE_SIG) + if (!fips_enabled || !m || (m && m->sig_ok)) + return 1; + else + return 0; +#else + return 1; +#endif +} + /* Process kernel command-line parameter at boot time. fips=0 or fips=1 */ static int fips_enable(char *str) { diff --git a/crypto/internal.h b/crypto/internal.h index 9ebedae..937bfaf 100644 --- a/crypto/internal.h +++ b/crypto/internal.h @@ -139,5 +139,14 @@ static inline void crypto_notify(unsigned long val, void *v) blocking_notifier_call_chain(&crypto_chain, val, v); } +#if defined(CONFIG_CRYPTO_FIPS) +int crypto_sig_check(struct module *m); +#else +static inline int crypto_sig_check(struct module *m) +{ + return 1; +} +#endif + #endif /* _CRYPTO_INTERNAL_H */ ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] fips: check whether a module registering an alg or template is signed 2013-02-05 22:58 ` [RFC PATCH] fips: check whether a module registering an alg or template is signed Kyle McMartin @ 2013-02-06 8:02 ` Stephan Mueller 2013-02-06 16:15 ` Kyle McMartin 0 siblings, 1 reply; 22+ messages in thread From: Stephan Mueller @ 2013-02-06 8:02 UTC (permalink / raw) To: Kyle McMartin; +Cc: linux-kernel, David Howells, rusty, jstancek, herbert On 05.02.2013 23:58:30, +0100, Kyle McMartin <kyle@redhat.com> wrote: Hi Kyle, > fips mode needs to prevent unsigned modules from registering crypto > algorithms, and currently panics if an unsigned module is attempted to > be loaded. Instead, forbid (by returning -EINVAL) registering a crypto > alg or template if fips mode is enabled and the module signature is not > valid. 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: > 1/ fips is not enabled (but CONFIG_CRYPTO_FIPS is enabled.) > 2/ the algorithm is built into the kernel (THIS_MODULE == NULL) > 3/ the algorithm is in a module, and the module sig check passes > and fail in any of the other cases. > > Checking in crypto_check_alg and crypto_register_template seems to hit > the callpoints as far as I can see. > > Signed-off-by: Kyle McMartin <kyle@redhat.com> > > --- > > rusty, > > How about something like this? It keeps the FIPS mess in the > crypto/fips.c file (aside from something that goes away entirely in the > !CONFIG_CRYPTO_FIPS case.) > > regards, Kyle > > --- a/crypto/algapi.c > +++ b/crypto/algapi.c > @@ -52,6 +52,9 @@ static int crypto_check_alg(struct crypto_alg *alg) > if (alg->cra_priority < 0) > return -EINVAL; > > + 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. > + > 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. > goto out; > } > > + if (!crypto_sig_check(tmpl->module)) { > + err = -EINVAL; > + goto out; > + } > + > list_add(&tmpl->list, &crypto_template_list); > crypto_notify(CRYPTO_MSG_TMPL_REGISTER, tmpl); > err = 0; > diff --git a/crypto/fips.c b/crypto/fips.c > index 5539700..2ebbe0f 100644 > --- a/crypto/fips.c > +++ b/crypto/fips.c > @@ -15,6 +15,19 @@ > int fips_enabled; > EXPORT_SYMBOL_GPL(fips_enabled); > > +/* forbid loading modules in fips mode if the module is not signed */ > +int crypto_sig_check(struct module *m) > +{ > +#if defined(CONFIG_MODULE_SIG) > + if (!fips_enabled || !m || (m && m->sig_ok)) > + return 1; > + else > + return 0; This code looks good. > +#else > + return 1; > +#endif > +} > + > /* Process kernel command-line parameter at boot time. fips=0 or fips=1 */ > static int fips_enable(char *str) > { > diff --git a/crypto/internal.h b/crypto/internal.h > index 9ebedae..937bfaf 100644 > --- a/crypto/internal.h > +++ b/crypto/internal.h > @@ -139,5 +139,14 @@ static inline void crypto_notify(unsigned long val, void *v) > blocking_notifier_call_chain(&crypto_chain, val, v); > } > > +#if defined(CONFIG_CRYPTO_FIPS) > +int crypto_sig_check(struct module *m); > +#else > +static inline int crypto_sig_check(struct module *m) > +{ > + return 1; > +} > +#endif > + > #endif /* _CRYPTO_INTERNAL_H */ > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] fips: check whether a module registering an alg or template is signed 2013-02-06 8:02 ` Stephan Mueller @ 2013-02-06 16:15 ` Kyle McMartin 2013-02-06 17:45 ` Stephan Mueller 0 siblings, 1 reply; 22+ messages in thread From: Kyle McMartin @ 2013-02-06 16:15 UTC (permalink / raw) To: Stephan Mueller; +Cc: linux-kernel, David Howells, rusty, jstancek, herbert On Wed, Feb 06, 2013 at 09:02:46AM +0100, Stephan Mueller wrote: > On 05.02.2013 23:58:30, +0100, Kyle McMartin <kyle@redhat.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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] fips: check whether a module registering an alg or template is signed 2013-02-06 16:15 ` Kyle McMartin @ 2013-02-06 17:45 ` Stephan Mueller 2013-02-06 18:18 ` Kyle McMartin 0 siblings, 1 reply; 22+ messages in thread From: Stephan Mueller @ 2013-02-06 17:45 UTC (permalink / raw) To: Kyle McMartin; +Cc: linux-kernel, David Howells, rusty, jstancek, herbert On 06.02.2013 17:15:57, +0100, Kyle McMartin <kmcmarti@redhat.com> wrote: Hi Kyle, > On Wed, Feb 06, 2013 at 09:02:46AM +0100, Stephan Mueller wrote: >> On 05.02.2013 23:58:30, +0100, Kyle McMartin <kyle@redhat.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 Unfortunately there has already something terrible happened: an additional piece of code loaded into the FIPS 140-2 module could not be loaded because a self test failed. This is a terrible accident in FIPS 140-2 speak. :-) That means, the FIPS 140-2 module, aka kernel crypto API must become unavailable. As one self test failed, the entire module must become unavailable. I am sorry, but there is no way around it. Just to quote the relevant part from the FIPS 140-2 specification, section 4.9: If a cryptographic module fails a self-test, the module shall enter an error state and output an error indicator via the status output interface. The cryptographic module shall not perform any cryptographic operations while in an error state. All data output via the data output interface shall be inhibited when an error state exists. ==> the signature test we are discussing here is one of these self tests, in particular a conditional self test defined in section 4.9.2 of the FIPS 140-2 standard. > 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. I am perfectly happy with the move of the code. > >>> + >>> 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. Ok, I believe you as I did not trace the code. I just wanted to point out this issue :-) But note, if a real FIPS 140-2 validation is conducted, we will trace the code ;-) > >>> goto out; >>> } > > regards, Kyle > Ciao Stephan ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH] fips: check whether a module registering an alg or template is signed 2013-02-06 17:45 ` Stephan Mueller @ 2013-02-06 18:18 ` Kyle McMartin 0 siblings, 0 replies; 22+ messages in thread From: Kyle McMartin @ 2013-02-06 18:18 UTC (permalink / raw) To: Stephan Mueller; +Cc: linux-kernel, David Howells, rusty, jstancek, herbert On Wed, Feb 06, 2013 at 06:45:45PM +0100, Stephan Mueller wrote: > Unfortunately there has already something terrible happened: an > additional piece of code loaded into the FIPS 140-2 module could not be > loaded because a self test failed. This is a terrible accident in FIPS > 140-2 speak. :-) > > That means, the FIPS 140-2 module, aka kernel crypto API must become > unavailable. As one self test failed, the entire module must become > unavailable. > > I am sorry, but there is no way around it. Just to quote the relevant > part from the FIPS 140-2 specification, section 4.9: > > If a cryptographic module fails a self-test, the module shall enter an > error state and output an error indicator > via the status output interface. The cryptographic module shall not > perform any cryptographic operations > while in an error state. All data output via the data output interface > shall be inhibited when an error state > exists. > OK. If Herbert and Rusty are ok with this, I'll send an additional patch moving the panic which should satisfy this requirement. > > ==> the signature test we are discussing here is one of these self > tests, in particular a conditional self test defined in section 4.9.2 of > the FIPS 140-2 standard. > > > 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. > > I am perfectly happy with the move of the code. > regards, Kyle ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-23 15:18 ` Stephan Mueller 2013-01-24 14:59 ` Kyle McMartin 2013-01-24 19:06 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned Kyle McMartin @ 2013-01-25 0:14 ` David Howells 2013-01-25 3:20 ` Matthew Garrett 2 siblings, 1 reply; 22+ messages in thread From: David Howells @ 2013-01-25 0:14 UTC (permalink / raw) To: Kyle McMartin; +Cc: dhowells, linux-kernel, rusty, jstancek, Stephan Mueller Kyle McMartin <kyle@redhat.com> wrote: > After thinking about it a while, this seems like the best way to solve > the problem, although it does still kind of offend my delicate > sensibilities... > > Doing this check in the crypto layer seems kind of like a layering > violation to me (and, to be honest, I think it'd be a gross-hack getting > from the callee back to the caller module.) > > Instead, doing it in kernel/module.c and looking at the undefined symbol > list again looks to me like an ordering problem since we're doing the > signature check before we've even done the elf header check. We'd have > to move the panic to a part of the module code that may not necessarily > make sense. > > Whereas checking the undefined symbol list at modpost time is fairly > logical, and adding a new MODULE_INFO entry in the CONFIG_CRYPTO_FIPS > case is a well understood thing to do, and should catch all the crypto > registrations regardless of where they exist in-tree... > > Seems to build and work with both values of CONFIG_CRYPTO_FIPS. > > Thoughts? You can't rely on someone trying to sneak a dodgy crypto module in to set the flag when they build it. The detection thus needs to be done in the kernel during the module load. Can you search the module image for "crypto_register_" I wonder? If that's there, it's a crypto module. David ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-25 0:14 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned David Howells @ 2013-01-25 3:20 ` Matthew Garrett 2013-01-25 12:23 ` Stephan Mueller 0 siblings, 1 reply; 22+ messages in thread From: Matthew Garrett @ 2013-01-25 3:20 UTC (permalink / raw) To: David Howells Cc: Kyle McMartin, linux-kernel, rusty, jstancek, Stephan Mueller On Fri, Jan 25, 2013 at 12:14:54AM +0000, David Howells wrote: > You can't rely on someone trying to sneak a dodgy crypto module in to set the > flag when they build it. The detection thus needs to be done in the kernel > during the module load. > > Can you search the module image for "crypto_register_" I wonder? If that's > there, it's a crypto module. If you're trying to protect against malice rather than accident, what's going to stop the module from just finding and modifying data structures itself? If you want to panic if you've just loaded something that might compromise your crypto implementations, you've got to panic on all unsigned module loads. -- Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned 2013-01-25 3:20 ` Matthew Garrett @ 2013-01-25 12:23 ` Stephan Mueller 0 siblings, 0 replies; 22+ messages in thread From: Stephan Mueller @ 2013-01-25 12:23 UTC (permalink / raw) To: Matthew Garrett Cc: David Howells, Kyle McMartin, linux-kernel, rusty, jstancek On 25.01.2013 04:20:07, +0100, Matthew Garrett <mjg59@srcf.ucam.org> wrote: Hi Matthew, > On Fri, Jan 25, 2013 at 12:14:54AM +0000, David Howells wrote: > >> You can't rely on someone trying to sneak a dodgy crypto module in to set the >> flag when they build it. The detection thus needs to be done in the kernel >> during the module load. >> >> Can you search the module image for "crypto_register_" I wonder? If that's >> there, it's a crypto module. > > If you're trying to protect against malice rather than accident, what's > going to stop the module from just finding and modifying data structures > itself? If you want to panic if you've just loaded something that might > compromise your crypto implementations, you've got to panic on all > unsigned module loads. That is the issue here. We want to protect against accidental changes and modifications. Malicious attacks will never be caught by the FIPS requirements where a module is allowed to check itself. If an attacker is able to load any kind of kernel module, we have lost. Period. Thus, from a FIPS point of view the latest patch from Kyle is also appropriate, provided the concerns I raised there are covered. Ciao Stephan ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2013-02-06 18:18 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-01-22 18:43 [PATCH] MODSIGN: only panic in fips mode if sig_enforce is set Kyle McMartin 2013-01-22 23:17 ` Rusty Russell 2013-01-23 11:26 ` David Howells 2013-01-23 15:18 ` Stephan Mueller 2013-01-24 14:59 ` Kyle McMartin 2013-01-25 11:28 ` Stephan Mueller 2013-01-24 19:06 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned Kyle McMartin 2013-01-24 19:21 ` Kyle McMartin 2013-01-24 23:36 ` Rusty Russell 2013-01-25 5:45 ` Kyle McMartin 2013-01-25 12:42 ` Stephan Mueller 2013-02-03 23:34 ` Rusty Russell 2013-01-25 12:46 ` Stephan Mueller 2013-01-25 12:18 ` Stephan Mueller 2013-02-05 22:58 ` [RFC PATCH] fips: check whether a module registering an alg or template is signed Kyle McMartin 2013-02-06 8:02 ` Stephan Mueller 2013-02-06 16:15 ` Kyle McMartin 2013-02-06 17:45 ` Stephan Mueller 2013-02-06 18:18 ` Kyle McMartin 2013-01-25 0:14 ` [PATCH] MODSIGN: flag modules that use cryptoapi and only panic if those are unsigned David Howells 2013-01-25 3:20 ` Matthew Garrett 2013-01-25 12:23 ` Stephan Mueller
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.