linux-kernel - Re: [PATCH] MODSIGN: Warn when sign check fails due to -ENOKEY

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] [day] [month] [year] [list]

Message-ID: <50F115CF.8050000@csamuel.org>
Date:	Sat, 12 Jan 2013 18:50:39 +1100
From:	Chris Samuel <chris@...muel.org>
To:	Josh Boyer <jwboyer@...il.com>
CC:	linux-kernel@...r.kernel.org,
	Rusty Russell <rusty@...tcorp.com.au>, dhowells@...hat.com
Subject: Re: [PATCH] MODSIGN: Warn when sign check fails due to -ENOKEY

On 12/01/13 00:49, Josh Boyer wrote:

> On Fri, Jan 11, 2013 at 4:44 AM, Chris Samuel <chris@...muel.org> wrote:
 >
>> /* Please CC me in responses, I am not subscribed to LKML */
>>
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 250092c..27de534 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -2443,8 +2443,10 @@ static int module_sig_check(struct load_info *info)
>>          if (err < 0 && fips_enabled)
>>                  panic("Module verification failed with error %d in FIPS
>> mode\n",
>>                        err);
>> -       if (err == -ENOKEY && !sig_enforce)
>> +       if (err == -ENOKEY && !sig_enforce) {
>> +               printk_once(KERN_DEBUG "Module verification failed, required
>> key not present, tainting kernel\n");
>>                  err = 0;
>> +       }
>>          return err;
>
> I'd suggest putting the printk in load_module where we call the
> add_taint_module function instead.

I did ponder that, but I used module_sig_check() instead as here we know 
explicitly that the failure is -ENOKEY, that information doesn't seem to 
get propagated back to load_module().

Looking at the code again though it seems that any other reason will 
make module_sig_check() return non-zero and hence cause the module to 
fail to load, so currently we can infer that the reason was -ENOKEY.

I'm happy either way, just my inner pedant thought this was better as in 
future module_sig_check() may find another reason to have to return with 
a zero status when modules aren't signed and so we can no longer tell 
the user the reason the signature failed.

Rusty, which is your preference?

> Also, you might want to make the priority a bit higher if it's meant
> to be informative.  Something like KERN_INFO.

Yup, sounds good, I see Rusty suggested KERN_NOTICE so I'll use that.

cheers,
Chris
-- 
  Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC
--
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/