MODSIGN: Warn when sign check fails due to -ENOKEY

PATCH In review Chris Samuel 3 participants 4 messages Jan 11, 2013 – Jan 12, 2013

lore

Share & tools

lore

Chris Samuel

author

2013-01-11 09:54 UTC #0 lore raw

PATCH 1/1 MODSIGN: Warn when sign check fails due to -ENOKEY

/* Please CC me in responses, I am not subscribed to LKML */

Currently if a signature check fails on module load due to not having the appropriate key (-ENOKEY) and we are not doing strict checking there is no information provided to the user other than the lock debug taint warning:

Disabling lock debugging due to kernel taint

This patch causes a single warning to be emitted to explain why the kernel is being tainted, before the above taint warning occurs.

Module verification failed, required key not present, tainting kernel

Found whilst trying to work out why all the 3.8 development kernels I was building and testing were warning about taints and why all modules were listed as forced load (F) in /proc/modules when that wasn't the case in the 3.5, 3.6 or 3.7 kernels I'd tried.

Signed-off-by Christopher Samuel <[email protected]>

---
 kernel/module.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

kernel/module.c +3 -1

index 250092c..27de534 100644
@@ -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;
 }

Signature · 1 line

1.7.10.4

Josh Boyer

2013-01-11 13:49 UTC #1 lore raw

Quoted 30 lines · Chris Samuel

/* Please CC me in responses, I am not subscribed to LKML */
Currently if a signature check fails on module load due to not having the appropriate key (-ENOKEY) and we are not doing strict checking there is no information provided to the user other than the lock debug taint warning:
Disabling lock debugging due to kernel taint
This patch causes a single warning to be emitted to explain why the kernel is being tainted, before the above taint warning occurs.
Module verification failed, required key not present, tainting kernel
Found whilst trying to work out why all the 3.8 development kernels I was building and testing were warning about taints and why all modules were listed as forced load (F) in /proc/modules when that wasn't the case in the 3.5, 3.6 or 3.7 kernels I'd tried.
Signed-off-by Christopher Samuel <[email protected]>
---
 kernel/module.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
kernel/module.c +0 -0
index 250092c..27de534 100644
@@ -2443,8 +2443,10 @@ static int module_sig_check(struct load_info *info)
        if (err < 0 && fips_enabled)

kernel/module.c static int module_sig_check(struct load_info *info) · line 2452

Quoted 10 lines

kernel/module.c +3 -1

                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. Also, you might want to make the priority a bit higher if it's meant to be informative. Something like KERN_INFO.

josh

Rusty Russell

2013-01-12 01:17 UTC #2 lore raw

Quoted 38 lines · Josh Boyer

Quoted 37 lines · Chris Samuel
/* Please CC me in responses, I am not subscribed to LKML */
Currently if a signature check fails on module load due to not having the appropriate key (-ENOKEY) and we are not doing strict checking there is no information provided to the user other than the lock debug taint warning:
Disabling lock debugging due to kernel taint
This patch causes a single warning to be emitted to explain why the kernel is being tainted, before the above taint warning occurs.
Module verification failed, required key not present, tainting kernel
Found whilst trying to work out why all the 3.8 development kernels I was building and testing were warning about taints and why all modules were listed as forced load (F) in /proc/modules when that wasn't the case in the 3.5, 3.6 or 3.7 kernels I'd tried.
Signed-off-by Christopher Samuel <[email protected]>
---
 kernel/module.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
kernel/module.c +2 -1
index 250092c..27de534 100644
@@ -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");

Quoted 8 lines

kernel/module.c static int module_sig_check(struct load_info *info) · line 2450
Quoted 3 lines
kernel/module.c +1 -0
                err = 0;
+       }
        return err;
I'd suggest putting the printk in load_module where we call the add_taint_module function instead. Also, you might want to make the priority a bit higher if it's meant to be informative. Something like KERN_INFO.

Agreed. KERN_NOTICE, I think: we really want to see if someone's inserting an unsigned module!

Cheers,
Rusty.

Chris Samuel

author

2013-01-12 07:51 UTC #3 lore raw

In reply to Josh Boyer

Quoted 15 lines · Josh Boyer

Quoted 13 lines · Chris Samuel

/* Please CC me in responses, I am not subscribed to LKML */

kernel/module.c +1 -1

index 250092c..27de534 100644
@@ -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) {

Quoted 8 lines

kernel/module.c static int module_sig_check(struct load_info *info) · line 2450
Quoted 5 lines
kernel/module.c +2 -0
+               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?

Quoted 2 lines

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

Signature · 1 line

 Chris Samuel  :  http://www.csamuel.org/  :  Melbourne, VIC