Re: [RFC] module: signature infrastructure

2012-09-06 Thread Rusty Russell
Lucas De Marchi  writes:
> Sorry to come up with this suggestion only now (and after you have
> already talked to me at LPC). Only after seeing this implementation I
> thought about the implications of having the module signed in this
> format.
...
> I'm worried about performance here. Module loading can take a fair
> amount of boot time. It may not be critical for servers or desktops
> that we rarely boot, but it is for embedded uses.
...
> Letting it in be32 is the simplest solution IMO. it's way simpler then
> the loop above.
...
>> Scanning the module is the least of our issues since we've just copied
>> it and we're about to SHA it.
>
> Yeah, but I don't think we need to scan it one more time. On every
> boot. For every module

Regretfully, I don't have Linus' talent for flamage.

There's no measurable performance impact.  Scanning 1k takes about
5usec; we've wasted about enough time on this subject to load a billion
kernel modules.

I know this.  Not because I'm brilliant, but because I *measured* it.  I
even pulled out my original module signature signing check code, and
that was both faster and simpler.  See below.

Your assertion that the length-prepended version is "way simpler" is
wrong.  Again, I know this because I *read the code*:


https://git.kernel.org/?p=linux/kernel/git/kasatkin/linux-digsig.git;a=commitdiff;h=19eef6e4e529ccf2b3a6ab5c19dd40f2eaf8fcaf

Don't send any more lazy, unthoughtful mails to the list.  It's
disrespectful and makes me grumpy.

Rusty.
PS.  Pushed updated version to my kernel.org linux.git/module-signing branch.

#ifdef CONFIG_MODULE_SIG
static int module_sig_check(struct load_info *info,
const void *mod, unsigned long *len)
{
int err = 0;
const unsigned long markerlen = strlen(MODULE_SIG_STRING);
const void *p = mod, *end = mod + *len;

/* Poor man's memmem. */
while ((p = memchr(p, MODULE_SIG_STRING[0], end - p))) {
if (p + markerlen > end)
break;

if (memcmp(p, MODULE_SIG_STRING, markerlen) == 0) {
const void *sig = p + markerlen;
/* Truncate module up to signature. */
*len = p - mod;
err = mod_verify_sig(mod, *len,
 sig, end - sig,
 >sig_ok);
break;
}
p++;
}

/* Not having a signature is only an error if we're strict. */
if (!err && !info->sig_ok && sig_enforce)
err = -EKEYREJECTED;
return err;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-06 Thread Rusty Russell
Lucas De Marchi lucas.de.mar...@gmail.com writes:
 Sorry to come up with this suggestion only now (and after you have
 already talked to me at LPC). Only after seeing this implementation I
 thought about the implications of having the module signed in this
 format.
...
 I'm worried about performance here. Module loading can take a fair
 amount of boot time. It may not be critical for servers or desktops
 that we rarely boot, but it is for embedded uses.
...
 Letting it in be32 is the simplest solution IMO. it's way simpler then
 the loop above.
...
 Scanning the module is the least of our issues since we've just copied
 it and we're about to SHA it.

 Yeah, but I don't think we need to scan it one more time. On every
 boot. For every module

Regretfully, I don't have Linus' talent for flamage.

There's no measurable performance impact.  Scanning 1k takes about
5usec; we've wasted about enough time on this subject to load a billion
kernel modules.

I know this.  Not because I'm brilliant, but because I *measured* it.  I
even pulled out my original module signature signing check code, and
that was both faster and simpler.  See below.

Your assertion that the length-prepended version is way simpler is
wrong.  Again, I know this because I *read the code*:


https://git.kernel.org/?p=linux/kernel/git/kasatkin/linux-digsig.git;a=commitdiff;h=19eef6e4e529ccf2b3a6ab5c19dd40f2eaf8fcaf

Don't send any more lazy, unthoughtful mails to the list.  It's
disrespectful and makes me grumpy.

Rusty.
PS.  Pushed updated version to my kernel.org linux.git/module-signing branch.

#ifdef CONFIG_MODULE_SIG
static int module_sig_check(struct load_info *info,
const void *mod, unsigned long *len)
{
int err = 0;
const unsigned long markerlen = strlen(MODULE_SIG_STRING);
const void *p = mod, *end = mod + *len;

/* Poor man's memmem. */
while ((p = memchr(p, MODULE_SIG_STRING[0], end - p))) {
if (p + markerlen  end)
break;

if (memcmp(p, MODULE_SIG_STRING, markerlen) == 0) {
const void *sig = p + markerlen;
/* Truncate module up to signature. */
*len = p - mod;
err = mod_verify_sig(mod, *len,
 sig, end - sig,
 info-sig_ok);
break;
}
p++;
}

/* Not having a signature is only an error if we're strict. */
if (!err  !info-sig_ok  sig_enforce)
err = -EKEYREJECTED;
return err;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Rusty Russell
Mimi Zohar  writes:

> On Wed, 2012-09-05 at 09:59 +0930, Rusty Russell wrote:
>> "Kasatkin, Dmitry"  writes:
>> > Hi,
>> >
>> > Please read bellow...
>> >
>> > On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell  
>> > wrote:
>> >> OK, I took a look at the module.c parts of David and Dmitry's patchsets,
>> >> and didn't really like either, but I stole parts of David's to make
>> >> this.
>> >>
>> >> So, here's the module.c part of module signing.  I hope you two got time
>> >> to discuss the signature format details?  Mimi suggested a scheme where
>> >> the private key would never be saved on disk (even temporarily), but I
>> >> didn't see patches.  Frankly it's something we can do later; let's aim
>> >> at getting the format right for the next merge window.
>> >
>> > In our patches key is stored on the disc in encrypted format...
>> 
>> Oh, I missed that twist.  Thanks for the explanation.
>> 
>> On consideration, I prefer signing to be the final part of the "modules"
>> target rather than modules_install.  I run the latter as root, and that
>> is wrong for doing any code generation.
>
> Agreed, but keep in mind that 'modules_install' could subsequently strip
> the module.

That had better be part of your signing step then!

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Lucas De Marchi
On Tue, Sep 4, 2012 at 9:19 PM, Rusty Russell  wrote:
> Lucas De Marchi  writes:
>> Hi Rusty,
>>
>> On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell  wrote:
>>> @@ -2399,7 +2437,50 @@ static inline void kmemleak_load_module(const struct 
>>> module *mod,
>>>  }
>>>  #endif
>>>
>>> -/* Sets info->hdr and info->len. */
>>> +#ifdef CONFIG_MODULE_SIG
>>> +static int module_sig_check(struct load_info *info,
>>> +   void *mod, unsigned long *len)
>>> +{
>>> +   int err;
>>> +   unsigned long i, siglen;
>>> +   char *sig = NULL;
>>> +
>>> +   /* This is not a valid module: ELF header is larger anyway. */
>>> +   if (*len < sizeof(MODULE_SIG_STRING))
>>> +   return -ENOEXEC;
>>> +
>>> +   for (i = 0; i < *len - (sizeof(MODULE_SIG_STRING)-1); i++) {
>>> +   /* Our memcmp is dumb, speed it up a little. */
>>> +   if (((char *)mod)[i] != MODULE_SIG_STRING[0])
>>> +   continue;
>>
>> Since the signature is appended to the module, why don't you go
>> backwards, starting from *len - strlen(sizeof(MODULE_SIG_STRING)) and
>> making this first comparison?
>
> We've had this discussion multiple times.  Simple wins.  It's so
> marginal, I don't really care, but I've changed it to:

Sorry to come up with this suggestion only now (and after you have
already talked to me at LPC). Only after seeing this implementation I
thought about the implications of having the module signed in this
format.

I'm worried about performance here. Module loading can take a fair
amount of boot time. It may not be critical for servers or desktops
that we rarely boot, but it is for embedded uses.

> int err;
> unsigned long i, siglen, markerlen;
> char *sig = NULL;
>
> markerlen = strlen(MODULE_SIG_STRING);
> /* This is not a valid module: ELF header is larger anyway. */
> if (*len < markerlen)
> return -ENOEXEC;
>
> for (i = *len - markerlen; i > 0; i--) {
> /* Our memcmp is dumb, speed it up a little. */
> if (((char *)mod)[i] != MODULE_SIG_STRING[0])
> continue;
> if (memcmp(mod+i, MODULE_SIG_STRING, markerlen))
> continue;
>
> sig = mod + i + markerlen;
> siglen = *len - i - markerlen;
> *len = i;
> break;
> }
>
> We could also implement memrchr(), or memrmem().  Hell, if we had
> memmem() in the kernel I'd gladly use it.
>
>> Or let the magic string as the last thing in the module and store the
>> signature length, too. In this case no scanning is needed
>
> Yes, they did that too, but append is simpler.  I don't even have to
> think about endianness (Dmitry chose be32) or parsing (David chose
> 5-digit ascii numeric encoding).

Letting it in be32 is the simplest solution IMO. it's way simpler then
the loop above. You have to check exactly 1 byte to have a first
decision if module is not signed (as opposed to scanning the entire
module). Then you compare the memory area with MODULE_SIG_STRING and
have a final decision. To get the signature length it is just a matter
of converting it to host endiannes.  And we do have functions for
doing that. If it's for simplicity in kernel side, this one could be
implemented in half of the code above.

>
> Scanning the module is the least of our issues since we've just copied
> it and we're about to SHA it.

Yeah, but I don't think we need to scan it one more time. On every
boot. For every module


Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Mimi Zohar
On Wed, 2012-09-05 at 09:59 +0930, Rusty Russell wrote:
> "Kasatkin, Dmitry"  writes:
> > Hi,
> >
> > Please read bellow...
> >
> > On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell  wrote:
> >> OK, I took a look at the module.c parts of David and Dmitry's patchsets,
> >> and didn't really like either, but I stole parts of David's to make
> >> this.
> >>
> >> So, here's the module.c part of module signing.  I hope you two got time
> >> to discuss the signature format details?  Mimi suggested a scheme where
> >> the private key would never be saved on disk (even temporarily), but I
> >> didn't see patches.  Frankly it's something we can do later; let's aim
> >> at getting the format right for the next merge window.
> >
> > In our patches key is stored on the disc in encrypted format...
> 
> Oh, I missed that twist.  Thanks for the explanation.
> 
> On consideration, I prefer signing to be the final part of the "modules"
> target rather than modules_install.  I run the latter as root, and that
> is wrong for doing any code generation.

Agreed, but keep in mind that 'modules_install' could subsequently strip
the module.

Mimi

> >> +   for (i = 0; i < *len - (sizeof(MODULE_SIG_STRING)-1); i++) {
> >> +   /* Our memcmp is dumb, speed it up a little. */
> >> +   if (((char *)mod)[i] != MODULE_SIG_STRING[0])
> >> +   continue;
> >> +   if (memcmp(mod, MODULE_SIG_STRING, 
> >> strlen(MODULE_SIG_STRING)))
> >
> > should be (mod+i)?
> 
> Yes, indeed.  Thanks, fixed.
> 
> >> +   continue;
> >> +
> >> +   sig = mod + i + strlen(MODULE_SIG_STRING);
> >> +   siglen = *len - i - strlen(MODULE_SIG_STRING);
> >> +   *len = i;
> >> +   break;
> >> +   }
> >
> > In general please clarify why do you need such parsing at all?
> > Why not to have MODULE_SIG_STRING as a last octets of the module and
> > have signature length field before?
> > Then it is easy to get the signature and rest of the module?
> > That will be super fast...
> >
> > Please clarify.
> 
> Ignore performance, it's just not an issue here.  So the simplest code
> wins.
> 
> And it's also simpler to sign a module this way.
> 
> (echo '~Module signature appended~'; gpg --sign ) >> mod.ko
> 
> Cheers,
> Rusty.
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-security-module" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Rusty Russell
Lucas De Marchi  writes:
> Hi Rusty,
>
> On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell  wrote:
>> @@ -2399,7 +2437,50 @@ static inline void kmemleak_load_module(const struct 
>> module *mod,
>>  }
>>  #endif
>>
>> -/* Sets info->hdr and info->len. */
>> +#ifdef CONFIG_MODULE_SIG
>> +static int module_sig_check(struct load_info *info,
>> +   void *mod, unsigned long *len)
>> +{
>> +   int err;
>> +   unsigned long i, siglen;
>> +   char *sig = NULL;
>> +
>> +   /* This is not a valid module: ELF header is larger anyway. */
>> +   if (*len < sizeof(MODULE_SIG_STRING))
>> +   return -ENOEXEC;
>> +
>> +   for (i = 0; i < *len - (sizeof(MODULE_SIG_STRING)-1); i++) {
>> +   /* Our memcmp is dumb, speed it up a little. */
>> +   if (((char *)mod)[i] != MODULE_SIG_STRING[0])
>> +   continue;
>
> Since the signature is appended to the module, why don't you go
> backwards, starting from *len - strlen(sizeof(MODULE_SIG_STRING)) and
> making this first comparison?

We've had this discussion multiple times.  Simple wins.  It's so
marginal, I don't really care, but I've changed it to:

int err;
unsigned long i, siglen, markerlen;
char *sig = NULL;

markerlen = strlen(MODULE_SIG_STRING);
/* This is not a valid module: ELF header is larger anyway. */
if (*len < markerlen)
return -ENOEXEC;

for (i = *len - markerlen; i > 0; i--) {
/* Our memcmp is dumb, speed it up a little. */
if (((char *)mod)[i] != MODULE_SIG_STRING[0])
continue;
if (memcmp(mod+i, MODULE_SIG_STRING, markerlen))
continue;

sig = mod + i + markerlen;
siglen = *len - i - markerlen;
*len = i;
break;
}

We could also implement memrchr(), or memrmem().  Hell, if we had
memmem() in the kernel I'd gladly use it.

> Or let the magic string as the last thing in the module and store the
> signature length, too. In this case no scanning is needed

Yes, they did that too, but append is simpler.  I don't even have to
think about endianness (Dmitry chose be32) or parsing (David chose
5-digit ascii numeric encoding).

Scanning the module is the least of our issues since we've just copied
it and we're about to SHA it.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Rusty Russell
"Kasatkin, Dmitry"  writes:
> Hi,
>
> Please read bellow...
>
> On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell  wrote:
>> OK, I took a look at the module.c parts of David and Dmitry's patchsets,
>> and didn't really like either, but I stole parts of David's to make
>> this.
>>
>> So, here's the module.c part of module signing.  I hope you two got time
>> to discuss the signature format details?  Mimi suggested a scheme where
>> the private key would never be saved on disk (even temporarily), but I
>> didn't see patches.  Frankly it's something we can do later; let's aim
>> at getting the format right for the next merge window.
>
> In our patches key is stored on the disc in encrypted format...

Oh, I missed that twist.  Thanks for the explanation.

On consideration, I prefer signing to be the final part of the "modules"
target rather than modules_install.  I run the latter as root, and that
is wrong for doing any code generation.

>> +   for (i = 0; i < *len - (sizeof(MODULE_SIG_STRING)-1); i++) {
>> +   /* Our memcmp is dumb, speed it up a little. */
>> +   if (((char *)mod)[i] != MODULE_SIG_STRING[0])
>> +   continue;
>> +   if (memcmp(mod, MODULE_SIG_STRING, 
>> strlen(MODULE_SIG_STRING)))
>
> should be (mod+i)?

Yes, indeed.  Thanks, fixed.

>> +   continue;
>> +
>> +   sig = mod + i + strlen(MODULE_SIG_STRING);
>> +   siglen = *len - i - strlen(MODULE_SIG_STRING);
>> +   *len = i;
>> +   break;
>> +   }
>
> In general please clarify why do you need such parsing at all?
> Why not to have MODULE_SIG_STRING as a last octets of the module and
> have signature length field before?
> Then it is easy to get the signature and rest of the module?
> That will be super fast...
>
> Please clarify.

Ignore performance, it's just not an issue here.  So the simplest code
wins.

And it's also simpler to sign a module this way.

(echo '~Module signature appended~'; gpg --sign ) >> mod.ko

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Lucas De Marchi
On Tue, Sep 4, 2012 at 9:19 PM, Rusty Russell ru...@rustcorp.com.au wrote:
 Lucas De Marchi lucas.de.mar...@gmail.com writes:
 Hi Rusty,

 On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell ru...@rustcorp.com.au wrote:
 @@ -2399,7 +2437,50 @@ static inline void kmemleak_load_module(const struct 
 module *mod,
  }
  #endif

 -/* Sets info-hdr and info-len. */
 +#ifdef CONFIG_MODULE_SIG
 +static int module_sig_check(struct load_info *info,
 +   void *mod, unsigned long *len)
 +{
 +   int err;
 +   unsigned long i, siglen;
 +   char *sig = NULL;
 +
 +   /* This is not a valid module: ELF header is larger anyway. */
 +   if (*len  sizeof(MODULE_SIG_STRING))
 +   return -ENOEXEC;
 +
 +   for (i = 0; i  *len - (sizeof(MODULE_SIG_STRING)-1); i++) {
 +   /* Our memcmp is dumb, speed it up a little. */
 +   if (((char *)mod)[i] != MODULE_SIG_STRING[0])
 +   continue;

 Since the signature is appended to the module, why don't you go
 backwards, starting from *len - strlen(sizeof(MODULE_SIG_STRING)) and
 making this first comparison?

 We've had this discussion multiple times.  Simple wins.  It's so
 marginal, I don't really care, but I've changed it to:

Sorry to come up with this suggestion only now (and after you have
already talked to me at LPC). Only after seeing this implementation I
thought about the implications of having the module signed in this
format.

I'm worried about performance here. Module loading can take a fair
amount of boot time. It may not be critical for servers or desktops
that we rarely boot, but it is for embedded uses.

 int err;
 unsigned long i, siglen, markerlen;
 char *sig = NULL;

 markerlen = strlen(MODULE_SIG_STRING);
 /* This is not a valid module: ELF header is larger anyway. */
 if (*len  markerlen)
 return -ENOEXEC;

 for (i = *len - markerlen; i  0; i--) {
 /* Our memcmp is dumb, speed it up a little. */
 if (((char *)mod)[i] != MODULE_SIG_STRING[0])
 continue;
 if (memcmp(mod+i, MODULE_SIG_STRING, markerlen))
 continue;

 sig = mod + i + markerlen;
 siglen = *len - i - markerlen;
 *len = i;
 break;
 }

 We could also implement memrchr(), or memrmem().  Hell, if we had
 memmem() in the kernel I'd gladly use it.

 Or let the magic string as the last thing in the module and store the
 signature length, too. In this case no scanning is needed

 Yes, they did that too, but append is simpler.  I don't even have to
 think about endianness (Dmitry chose be32) or parsing (David chose
 5-digit ascii numeric encoding).

Letting it in be32 is the simplest solution IMO. it's way simpler then
the loop above. You have to check exactly 1 byte to have a first
decision if module is not signed (as opposed to scanning the entire
module). Then you compare the memory area with MODULE_SIG_STRING and
have a final decision. To get the signature length it is just a matter
of converting it to host endiannes.  And we do have functions for
doing that. If it's for simplicity in kernel side, this one could be
implemented in half of the code above.


 Scanning the module is the least of our issues since we've just copied
 it and we're about to SHA it.

Yeah, but I don't think we need to scan it one more time. On every
boot. For every module


Lucas De Marchi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Rusty Russell
Mimi Zohar zo...@linux.vnet.ibm.com writes:

 On Wed, 2012-09-05 at 09:59 +0930, Rusty Russell wrote:
 Kasatkin, Dmitry dmitry.kasat...@intel.com writes:
  Hi,
 
  Please read bellow...
 
  On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell ru...@rustcorp.com.au 
  wrote:
  OK, I took a look at the module.c parts of David and Dmitry's patchsets,
  and didn't really like either, but I stole parts of David's to make
  this.
 
  So, here's the module.c part of module signing.  I hope you two got time
  to discuss the signature format details?  Mimi suggested a scheme where
  the private key would never be saved on disk (even temporarily), but I
  didn't see patches.  Frankly it's something we can do later; let's aim
  at getting the format right for the next merge window.
 
  In our patches key is stored on the disc in encrypted format...
 
 Oh, I missed that twist.  Thanks for the explanation.
 
 On consideration, I prefer signing to be the final part of the modules
 target rather than modules_install.  I run the latter as root, and that
 is wrong for doing any code generation.

 Agreed, but keep in mind that 'modules_install' could subsequently strip
 the module.

That had better be part of your signing step then!

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Rusty Russell
Kasatkin, Dmitry dmitry.kasat...@intel.com writes:
 Hi,

 Please read bellow...

 On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell ru...@rustcorp.com.au wrote:
 OK, I took a look at the module.c parts of David and Dmitry's patchsets,
 and didn't really like either, but I stole parts of David's to make
 this.

 So, here's the module.c part of module signing.  I hope you two got time
 to discuss the signature format details?  Mimi suggested a scheme where
 the private key would never be saved on disk (even temporarily), but I
 didn't see patches.  Frankly it's something we can do later; let's aim
 at getting the format right for the next merge window.

 In our patches key is stored on the disc in encrypted format...

Oh, I missed that twist.  Thanks for the explanation.

On consideration, I prefer signing to be the final part of the modules
target rather than modules_install.  I run the latter as root, and that
is wrong for doing any code generation.

 +   for (i = 0; i  *len - (sizeof(MODULE_SIG_STRING)-1); i++) {
 +   /* Our memcmp is dumb, speed it up a little. */
 +   if (((char *)mod)[i] != MODULE_SIG_STRING[0])
 +   continue;
 +   if (memcmp(mod, MODULE_SIG_STRING, 
 strlen(MODULE_SIG_STRING)))

 should be (mod+i)?

Yes, indeed.  Thanks, fixed.

 +   continue;
 +
 +   sig = mod + i + strlen(MODULE_SIG_STRING);
 +   siglen = *len - i - strlen(MODULE_SIG_STRING);
 +   *len = i;
 +   break;
 +   }

 In general please clarify why do you need such parsing at all?
 Why not to have MODULE_SIG_STRING as a last octets of the module and
 have signature length field before?
 Then it is easy to get the signature and rest of the module?
 That will be super fast...

 Please clarify.

Ignore performance, it's just not an issue here.  So the simplest code
wins.

And it's also simpler to sign a module this way.

(echo '~Module signature appended~'; gpg --sign )  mod.ko

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Rusty Russell
Lucas De Marchi lucas.de.mar...@gmail.com writes:
 Hi Rusty,

 On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell ru...@rustcorp.com.au wrote:
 @@ -2399,7 +2437,50 @@ static inline void kmemleak_load_module(const struct 
 module *mod,
  }
  #endif

 -/* Sets info-hdr and info-len. */
 +#ifdef CONFIG_MODULE_SIG
 +static int module_sig_check(struct load_info *info,
 +   void *mod, unsigned long *len)
 +{
 +   int err;
 +   unsigned long i, siglen;
 +   char *sig = NULL;
 +
 +   /* This is not a valid module: ELF header is larger anyway. */
 +   if (*len  sizeof(MODULE_SIG_STRING))
 +   return -ENOEXEC;
 +
 +   for (i = 0; i  *len - (sizeof(MODULE_SIG_STRING)-1); i++) {
 +   /* Our memcmp is dumb, speed it up a little. */
 +   if (((char *)mod)[i] != MODULE_SIG_STRING[0])
 +   continue;

 Since the signature is appended to the module, why don't you go
 backwards, starting from *len - strlen(sizeof(MODULE_SIG_STRING)) and
 making this first comparison?

We've had this discussion multiple times.  Simple wins.  It's so
marginal, I don't really care, but I've changed it to:

int err;
unsigned long i, siglen, markerlen;
char *sig = NULL;

markerlen = strlen(MODULE_SIG_STRING);
/* This is not a valid module: ELF header is larger anyway. */
if (*len  markerlen)
return -ENOEXEC;

for (i = *len - markerlen; i  0; i--) {
/* Our memcmp is dumb, speed it up a little. */
if (((char *)mod)[i] != MODULE_SIG_STRING[0])
continue;
if (memcmp(mod+i, MODULE_SIG_STRING, markerlen))
continue;

sig = mod + i + markerlen;
siglen = *len - i - markerlen;
*len = i;
break;
}

We could also implement memrchr(), or memrmem().  Hell, if we had
memmem() in the kernel I'd gladly use it.

 Or let the magic string as the last thing in the module and store the
 signature length, too. In this case no scanning is needed

Yes, they did that too, but append is simpler.  I don't even have to
think about endianness (Dmitry chose be32) or parsing (David chose
5-digit ascii numeric encoding).

Scanning the module is the least of our issues since we've just copied
it and we're about to SHA it.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-05 Thread Mimi Zohar
On Wed, 2012-09-05 at 09:59 +0930, Rusty Russell wrote:
 Kasatkin, Dmitry dmitry.kasat...@intel.com writes:
  Hi,
 
  Please read bellow...
 
  On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell ru...@rustcorp.com.au wrote:
  OK, I took a look at the module.c parts of David and Dmitry's patchsets,
  and didn't really like either, but I stole parts of David's to make
  this.
 
  So, here's the module.c part of module signing.  I hope you two got time
  to discuss the signature format details?  Mimi suggested a scheme where
  the private key would never be saved on disk (even temporarily), but I
  didn't see patches.  Frankly it's something we can do later; let's aim
  at getting the format right for the next merge window.
 
  In our patches key is stored on the disc in encrypted format...
 
 Oh, I missed that twist.  Thanks for the explanation.
 
 On consideration, I prefer signing to be the final part of the modules
 target rather than modules_install.  I run the latter as root, and that
 is wrong for doing any code generation.

Agreed, but keep in mind that 'modules_install' could subsequently strip
the module.

Mimi

  +   for (i = 0; i  *len - (sizeof(MODULE_SIG_STRING)-1); i++) {
  +   /* Our memcmp is dumb, speed it up a little. */
  +   if (((char *)mod)[i] != MODULE_SIG_STRING[0])
  +   continue;
  +   if (memcmp(mod, MODULE_SIG_STRING, 
  strlen(MODULE_SIG_STRING)))
 
  should be (mod+i)?
 
 Yes, indeed.  Thanks, fixed.
 
  +   continue;
  +
  +   sig = mod + i + strlen(MODULE_SIG_STRING);
  +   siglen = *len - i - strlen(MODULE_SIG_STRING);
  +   *len = i;
  +   break;
  +   }
 
  In general please clarify why do you need such parsing at all?
  Why not to have MODULE_SIG_STRING as a last octets of the module and
  have signature length field before?
  Then it is easy to get the signature and rest of the module?
  That will be super fast...
 
  Please clarify.
 
 Ignore performance, it's just not an issue here.  So the simplest code
 wins.
 
 And it's also simpler to sign a module this way.
 
 (echo '~Module signature appended~'; gpg --sign )  mod.ko
 
 Cheers,
 Rusty.
 --
 To unsubscribe from this list: send the line unsubscribe 
 linux-security-module in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-04 Thread Kasatkin, Dmitry
On Wed, Sep 5, 2012 at 1:51 AM, David Howells  wrote:
> Lucas De Marchi  wrote:
>
>> Or let the magic string as the last thing in the module and store the
>> signature length, too. In this case no scanning is needed
>
> Indeed.  This is the better way.
>
> The main problem is rendering the length from a shell script.  It's trivial to
> do as ASCII (there's a printf program), but a pain to render to binary.  I'm
> sure it can be done with perl or python without the need to compile anything.
>
> David

That is very easy to do from script as well.
See script in my tree.

http://git.kernel.org/?p=linux/kernel/git/kasatkin/linux-digsig.git;a=blob;f=scripts/modsig.sh;h=4e997c3996d71d8e1afeb3a7afe23b3f303b9f63;hb=59f1d5352969166f2f32f84e07e20dd1b30a890f

110 # add signature length - big endian
111 dec2hex $(stat --printf %s $sigfile) 4 | hex2bin $sigfile
112 echo -n "This Is A Crypto Signed Module" >>$sigfile

well. I have couple of small functions in the script. dec2hex and hex2bin

- Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-04 Thread David Howells
Lucas De Marchi  wrote:

> Or let the magic string as the last thing in the module and store the
> signature length, too. In this case no scanning is needed

Indeed.  This is the better way.

The main problem is rendering the length from a shell script.  It's trivial to
do as ASCII (there's a printf program), but a pain to render to binary.  I'm
sure it can be done with perl or python without the need to compile anything.

David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-04 Thread Kasatkin, Dmitry
On Tue, Sep 4, 2012 at 5:25 PM, Lucas De Marchi
 wrote:
> Hi Rusty,
>
> On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell  wrote:
>> OK, I took a look at the module.c parts of David and Dmitry's patchsets,
>> and didn't really like either, but I stole parts of David's to make
>> this.
>>
>> So, here's the module.c part of module signing.  I hope you two got time
>> to discuss the signature format details?  Mimi suggested a scheme where
>> the private key would never be saved on disk (even temporarily), but I
>> didn't see patches.  Frankly it's something we can do later; let's aim
>> at getting the format right for the next merge window.
>>
>> Cheers,
>> Rusty.
>>
>> ---
>> This patch doesn't compile: we need to implement:
>>
>> int mod_verify_sig(const void *mod, unsigned long modlen,
>>const void *sig, unsigned long siglen,
>>bool *sig_ok);
>>
>> Also, we need to actually append the signatures during build.
>>
>> diff --git a/Documentation/kernel-parameters.txt 
>> b/Documentation/kernel-parameters.txt
>> index ad7e2e5..9b2b8d3 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1582,6 +1582,12 @@ bytes respectively. Such letter suffixes can also be 
>> entirely omitted.
>> log everything. Information is printed at KERN_DEBUG
>> so loglevel=8 may also need to be specified.
>>
>> +   module.sig_enforce
>> +   [KNL] When CONFIG_MODULE_SIG is set, this means that
>> +   modules without (valid) signatures will fail to load.
>> +   Note that if CONFIG_MODULE_SIG_ENFORCE is set, that
>> +   is always true, so this option does nothing.
>> +
>> mousedev.tap_time=
>> [MOUSE] Maximum time between finger touching and
>> leaving touchpad surface for touch to be considered
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index fbcafe2..7760c6d 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -21,6 +21,9 @@
>>  #include 
>>  #include 
>>
>> +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
>> +#define MODULE_SIG_STRING "~Module signature appended~\n"
>> +
>>  /* Not Yet Implemented */
>>  #define MODULE_SUPPORTED_DEVICE(name)
>>
>> @@ -260,6 +263,11 @@ struct module
>> const unsigned long *unused_gpl_crcs;
>>  #endif
>>
>> +#ifdef CONFIG_MODULE_SIG
>> +   /* Signature was verified. */
>> +   bool sig_ok;
>> +#endif
>> +
>> /* symbols that will be GPL-only in the near future. */
>> const struct kernel_symbol *gpl_future_syms;
>> const unsigned long *gpl_future_crcs;
>> diff --git a/init/Kconfig b/init/Kconfig
>> index af6c7f8..7452e19 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1585,6 +1585,20 @@ config MODULE_SRCVERSION_ALL
>>   the version).  With this option, such a "srcversion" field
>>   will be created for all modules.  If unsure, say N.
>>
>> +config MODULE_SIG
>> +   bool "Module signature verification"
>> +   depends on MODULES
>> +   help
>> + Check modules for valid signatures upon load: the signature
>> + is simply appended to the module. For more information see
>> + Documentation/module-signing.txt.
>> +
>> +config MODULE_SIG_FORCE
>> +   bool "Require modules to be validly signed"
>> +   depends on MODULE_SIG
>> +   help
>> + Reject unsigned modules or signed modules for which we don't have a
>> + key.  Without this, such modules will simply taint the kernel.
>>  endif # MODULES
>>
>>  config INIT_ALL_POSSIBLE
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 4edbd9c..3cbd1a4 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -102,6 +102,43 @@ static LIST_HEAD(modules);
>>  struct list_head *kdb_modules =  /* kdb needs the list of modules 
>> */
>>  #endif /* CONFIG_KGDB_KDB */
>>
>> +#ifdef CONFIG_MODULE_SIG
>> +#ifdef CONFIG_MODULE_SIG_ENFORCE
>> +static bool sig_enforce = true;
>> +#else
>> +static bool sig_enforce = false;
>> +
>> +static int param_set_bool_enable_only(const char *val,
>> + const struct kernel_param *kp)
>> +{
>> +   int err;
>> +   bool test;
>> +   struct kernel_param dummy_kp = *kp;
>> +
>> +   dummy_kp.arg = 
>> +
>> +   err = param_set_bool(val, _kp);
>> +   if (err)
>> +   return err;
>> +
>> +   /* Don't let them unset it once it's set! */
>> +   if (!test && sig_enforce)
>> +   return -EROFS;
>> +
>> +   if (test)
>> +   sig_enforce = true;
>> +   return 0;
>> +}
>> +
>> +static const struct kernel_param_ops param_ops_bool_enable_only = {
>> +   .set = param_set_bool_enable_only,
>> +   .get = param_get_bool,
>> +};
>> +#define 

Re: [RFC] module: signature infrastructure

2012-09-04 Thread Lucas De Marchi
Hi Rusty,

On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell  wrote:
> OK, I took a look at the module.c parts of David and Dmitry's patchsets,
> and didn't really like either, but I stole parts of David's to make
> this.
>
> So, here's the module.c part of module signing.  I hope you two got time
> to discuss the signature format details?  Mimi suggested a scheme where
> the private key would never be saved on disk (even temporarily), but I
> didn't see patches.  Frankly it's something we can do later; let's aim
> at getting the format right for the next merge window.
>
> Cheers,
> Rusty.
>
> ---
> This patch doesn't compile: we need to implement:
>
> int mod_verify_sig(const void *mod, unsigned long modlen,
>const void *sig, unsigned long siglen,
>bool *sig_ok);
>
> Also, we need to actually append the signatures during build.
>
> diff --git a/Documentation/kernel-parameters.txt 
> b/Documentation/kernel-parameters.txt
> index ad7e2e5..9b2b8d3 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1582,6 +1582,12 @@ bytes respectively. Such letter suffixes can also be 
> entirely omitted.
> log everything. Information is printed at KERN_DEBUG
> so loglevel=8 may also need to be specified.
>
> +   module.sig_enforce
> +   [KNL] When CONFIG_MODULE_SIG is set, this means that
> +   modules without (valid) signatures will fail to load.
> +   Note that if CONFIG_MODULE_SIG_ENFORCE is set, that
> +   is always true, so this option does nothing.
> +
> mousedev.tap_time=
> [MOUSE] Maximum time between finger touching and
> leaving touchpad surface for touch to be considered
> diff --git a/include/linux/module.h b/include/linux/module.h
> index fbcafe2..7760c6d 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -21,6 +21,9 @@
>  #include 
>  #include 
>
> +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
> +#define MODULE_SIG_STRING "~Module signature appended~\n"
> +
>  /* Not Yet Implemented */
>  #define MODULE_SUPPORTED_DEVICE(name)
>
> @@ -260,6 +263,11 @@ struct module
> const unsigned long *unused_gpl_crcs;
>  #endif
>
> +#ifdef CONFIG_MODULE_SIG
> +   /* Signature was verified. */
> +   bool sig_ok;
> +#endif
> +
> /* symbols that will be GPL-only in the near future. */
> const struct kernel_symbol *gpl_future_syms;
> const unsigned long *gpl_future_crcs;
> diff --git a/init/Kconfig b/init/Kconfig
> index af6c7f8..7452e19 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1585,6 +1585,20 @@ config MODULE_SRCVERSION_ALL
>   the version).  With this option, such a "srcversion" field
>   will be created for all modules.  If unsure, say N.
>
> +config MODULE_SIG
> +   bool "Module signature verification"
> +   depends on MODULES
> +   help
> + Check modules for valid signatures upon load: the signature
> + is simply appended to the module. For more information see
> + Documentation/module-signing.txt.
> +
> +config MODULE_SIG_FORCE
> +   bool "Require modules to be validly signed"
> +   depends on MODULE_SIG
> +   help
> + Reject unsigned modules or signed modules for which we don't have a
> + key.  Without this, such modules will simply taint the kernel.
>  endif # MODULES
>
>  config INIT_ALL_POSSIBLE
> diff --git a/kernel/module.c b/kernel/module.c
> index 4edbd9c..3cbd1a4 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -102,6 +102,43 @@ static LIST_HEAD(modules);
>  struct list_head *kdb_modules =  /* kdb needs the list of modules */
>  #endif /* CONFIG_KGDB_KDB */
>
> +#ifdef CONFIG_MODULE_SIG
> +#ifdef CONFIG_MODULE_SIG_ENFORCE
> +static bool sig_enforce = true;
> +#else
> +static bool sig_enforce = false;
> +
> +static int param_set_bool_enable_only(const char *val,
> + const struct kernel_param *kp)
> +{
> +   int err;
> +   bool test;
> +   struct kernel_param dummy_kp = *kp;
> +
> +   dummy_kp.arg = 
> +
> +   err = param_set_bool(val, _kp);
> +   if (err)
> +   return err;
> +
> +   /* Don't let them unset it once it's set! */
> +   if (!test && sig_enforce)
> +   return -EROFS;
> +
> +   if (test)
> +   sig_enforce = true;
> +   return 0;
> +}
> +
> +static const struct kernel_param_ops param_ops_bool_enable_only = {
> +   .set = param_set_bool_enable_only,
> +   .get = param_get_bool,
> +};
> +#define param_check_bool_enable_only param_check_bool
> +
> +module_param(sig_enforce, bool_enable_only, 0644);
> +#endif /* !CONFIG_MODULE_SIG_ENFORCE */
> +#endif /* CONFIG_MODULE_SIG */
>
>  /* Block module 

Re: [RFC] module: signature infrastructure

2012-09-04 Thread Mimi Zohar
On Tue, 2012-09-04 at 15:21 +0300, Kasatkin, Dmitry wrote:
> On Tue, Sep 4, 2012 at 3:07 PM, Kasatkin, Dmitry
>  wrote:
> > Hi,
> >
> > Please read bellow...
> >
> > On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell  wrote:
> >> OK, I took a look at the module.c parts of David and Dmitry's patchsets,
> >> and didn't really like either, but I stole parts of David's to make
> >> this.
> >>
> >> So, here's the module.c part of module signing.  I hope you two got time
> >> to discuss the signature format details?  

The integrity subsystem currently defines 3 extended attribute formats
in security/integrity.h.

enum evm_ima_xattr_type {
IMA_XATTR_DIGEST = 0x01,
EVM_XATTR_HMAC,
EVM_IMA_XATTR_DIGSIG,
};

integrity_digsig_verify() is called to appraise EVM signatures stored as
EVM_IMA_XATTR_DIGSIG.  In Dmitry's patches, this same call is used to
appraise modules.  If you decide to define a new format, it should be
included here as well.

> Mimi suggested a scheme where
> >> the private key would never be saved on disk (even temporarily), but I
> >> didn't see patches.  Frankly it's something we can do later; let's aim
> >> at getting the format right for the next merge window.

Right, the key is a build issue, which doesn't affect the format.

> > In our patches key is stored on the disc in encrypted format...

An updated version of Dmitry's patches are in the 'modsig' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig. For
now, although commit 59f1d53 "modsig: build rules and scripts to
generate keys and sign modules" writes the ephemeral key to disk, it is
encrypted.

thanks,

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-04 Thread Kasatkin, Dmitry
On Tue, Sep 4, 2012 at 3:07 PM, Kasatkin, Dmitry
 wrote:
> Hi,
>
> Please read bellow...
>
> On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell  wrote:
>> OK, I took a look at the module.c parts of David and Dmitry's patchsets,
>> and didn't really like either, but I stole parts of David's to make
>> this.
>>
>> So, here's the module.c part of module signing.  I hope you two got time
>> to discuss the signature format details?  Mimi suggested a scheme where
>> the private key would never be saved on disk (even temporarily), but I
>> didn't see patches.  Frankly it's something we can do later; let's aim
>> at getting the format right for the next merge window.
>
> In our patches key is stored on the disc in encrypted format...
>
> More bellow..
>
>>
>> Cheers,
>> Rusty.
>>
>> ---
>> This patch doesn't compile: we need to implement:
>>
>> int mod_verify_sig(const void *mod, unsigned long modlen,
>>const void *sig, unsigned long siglen,
>>bool *sig_ok);
>>
>> Also, we need to actually append the signatures during build.
>>
>> diff --git a/Documentation/kernel-parameters.txt 
>> b/Documentation/kernel-parameters.txt
>> index ad7e2e5..9b2b8d3 100644
>> --- a/Documentation/kernel-parameters.txt
>> +++ b/Documentation/kernel-parameters.txt
>> @@ -1582,6 +1582,12 @@ bytes respectively. Such letter suffixes can also be 
>> entirely omitted.
>> log everything. Information is printed at KERN_DEBUG
>> so loglevel=8 may also need to be specified.
>>
>> +   module.sig_enforce
>> +   [KNL] When CONFIG_MODULE_SIG is set, this means that
>> +   modules without (valid) signatures will fail to load.
>> +   Note that if CONFIG_MODULE_SIG_ENFORCE is set, that
>> +   is always true, so this option does nothing.
>> +
>> mousedev.tap_time=
>> [MOUSE] Maximum time between finger touching and
>> leaving touchpad surface for touch to be considered
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index fbcafe2..7760c6d 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -21,6 +21,9 @@
>>  #include 
>>  #include 
>>
>> +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
>> +#define MODULE_SIG_STRING "~Module signature appended~\n"
>> +
>>  /* Not Yet Implemented */
>>  #define MODULE_SUPPORTED_DEVICE(name)
>>
>> @@ -260,6 +263,11 @@ struct module
>> const unsigned long *unused_gpl_crcs;
>>  #endif
>>
>> +#ifdef CONFIG_MODULE_SIG
>> +   /* Signature was verified. */
>> +   bool sig_ok;
>> +#endif
>> +
>> /* symbols that will be GPL-only in the near future. */
>> const struct kernel_symbol *gpl_future_syms;
>> const unsigned long *gpl_future_crcs;
>> diff --git a/init/Kconfig b/init/Kconfig
>> index af6c7f8..7452e19 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -1585,6 +1585,20 @@ config MODULE_SRCVERSION_ALL
>>   the version).  With this option, such a "srcversion" field
>>   will be created for all modules.  If unsure, say N.
>>
>> +config MODULE_SIG
>> +   bool "Module signature verification"
>> +   depends on MODULES
>> +   help
>> + Check modules for valid signatures upon load: the signature
>> + is simply appended to the module. For more information see
>> + Documentation/module-signing.txt.
>> +
>> +config MODULE_SIG_FORCE
>> +   bool "Require modules to be validly signed"
>> +   depends on MODULE_SIG
>> +   help
>> + Reject unsigned modules or signed modules for which we don't have a
>> + key.  Without this, such modules will simply taint the kernel.
>>  endif # MODULES
>>
>>  config INIT_ALL_POSSIBLE
>> diff --git a/kernel/module.c b/kernel/module.c
>> index 4edbd9c..3cbd1a4 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
>> @@ -102,6 +102,43 @@ static LIST_HEAD(modules);
>>  struct list_head *kdb_modules =  /* kdb needs the list of modules 
>> */
>>  #endif /* CONFIG_KGDB_KDB */
>>
>> +#ifdef CONFIG_MODULE_SIG
>> +#ifdef CONFIG_MODULE_SIG_ENFORCE
>> +static bool sig_enforce = true;
>> +#else
>> +static bool sig_enforce = false;
>> +
>> +static int param_set_bool_enable_only(const char *val,
>> + const struct kernel_param *kp)
>> +{
>> +   int err;
>> +   bool test;
>> +   struct kernel_param dummy_kp = *kp;
>> +
>> +   dummy_kp.arg = 
>> +
>> +   err = param_set_bool(val, _kp);
>> +   if (err)
>> +   return err;
>> +
>> +   /* Don't let them unset it once it's set! */
>> +   if (!test && sig_enforce)
>> +   return -EROFS;
>> +
>> +   if (test)
>> +   sig_enforce = true;
>> +   return 0;
>> +}
>> +
>> +static const struct kernel_param_ops param_ops_bool_enable_only = {
>> +   

Re: [RFC] module: signature infrastructure

2012-09-04 Thread Kasatkin, Dmitry
Hi,

Please read bellow...

On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell  wrote:
> OK, I took a look at the module.c parts of David and Dmitry's patchsets,
> and didn't really like either, but I stole parts of David's to make
> this.
>
> So, here's the module.c part of module signing.  I hope you two got time
> to discuss the signature format details?  Mimi suggested a scheme where
> the private key would never be saved on disk (even temporarily), but I
> didn't see patches.  Frankly it's something we can do later; let's aim
> at getting the format right for the next merge window.

In our patches key is stored on the disc in encrypted format...

More bellow..

>
> Cheers,
> Rusty.
>
> ---
> This patch doesn't compile: we need to implement:
>
> int mod_verify_sig(const void *mod, unsigned long modlen,
>const void *sig, unsigned long siglen,
>bool *sig_ok);
>
> Also, we need to actually append the signatures during build.
>
> diff --git a/Documentation/kernel-parameters.txt 
> b/Documentation/kernel-parameters.txt
> index ad7e2e5..9b2b8d3 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1582,6 +1582,12 @@ bytes respectively. Such letter suffixes can also be 
> entirely omitted.
> log everything. Information is printed at KERN_DEBUG
> so loglevel=8 may also need to be specified.
>
> +   module.sig_enforce
> +   [KNL] When CONFIG_MODULE_SIG is set, this means that
> +   modules without (valid) signatures will fail to load.
> +   Note that if CONFIG_MODULE_SIG_ENFORCE is set, that
> +   is always true, so this option does nothing.
> +
> mousedev.tap_time=
> [MOUSE] Maximum time between finger touching and
> leaving touchpad surface for touch to be considered
> diff --git a/include/linux/module.h b/include/linux/module.h
> index fbcafe2..7760c6d 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -21,6 +21,9 @@
>  #include 
>  #include 
>
> +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
> +#define MODULE_SIG_STRING "~Module signature appended~\n"
> +
>  /* Not Yet Implemented */
>  #define MODULE_SUPPORTED_DEVICE(name)
>
> @@ -260,6 +263,11 @@ struct module
> const unsigned long *unused_gpl_crcs;
>  #endif
>
> +#ifdef CONFIG_MODULE_SIG
> +   /* Signature was verified. */
> +   bool sig_ok;
> +#endif
> +
> /* symbols that will be GPL-only in the near future. */
> const struct kernel_symbol *gpl_future_syms;
> const unsigned long *gpl_future_crcs;
> diff --git a/init/Kconfig b/init/Kconfig
> index af6c7f8..7452e19 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1585,6 +1585,20 @@ config MODULE_SRCVERSION_ALL
>   the version).  With this option, such a "srcversion" field
>   will be created for all modules.  If unsure, say N.
>
> +config MODULE_SIG
> +   bool "Module signature verification"
> +   depends on MODULES
> +   help
> + Check modules for valid signatures upon load: the signature
> + is simply appended to the module. For more information see
> + Documentation/module-signing.txt.
> +
> +config MODULE_SIG_FORCE
> +   bool "Require modules to be validly signed"
> +   depends on MODULE_SIG
> +   help
> + Reject unsigned modules or signed modules for which we don't have a
> + key.  Without this, such modules will simply taint the kernel.
>  endif # MODULES
>
>  config INIT_ALL_POSSIBLE
> diff --git a/kernel/module.c b/kernel/module.c
> index 4edbd9c..3cbd1a4 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -102,6 +102,43 @@ static LIST_HEAD(modules);
>  struct list_head *kdb_modules =  /* kdb needs the list of modules */
>  #endif /* CONFIG_KGDB_KDB */
>
> +#ifdef CONFIG_MODULE_SIG
> +#ifdef CONFIG_MODULE_SIG_ENFORCE
> +static bool sig_enforce = true;
> +#else
> +static bool sig_enforce = false;
> +
> +static int param_set_bool_enable_only(const char *val,
> + const struct kernel_param *kp)
> +{
> +   int err;
> +   bool test;
> +   struct kernel_param dummy_kp = *kp;
> +
> +   dummy_kp.arg = 
> +
> +   err = param_set_bool(val, _kp);
> +   if (err)
> +   return err;
> +
> +   /* Don't let them unset it once it's set! */
> +   if (!test && sig_enforce)
> +   return -EROFS;
> +
> +   if (test)
> +   sig_enforce = true;
> +   return 0;
> +}
> +
> +static const struct kernel_param_ops param_ops_bool_enable_only = {
> +   .set = param_set_bool_enable_only,
> +   .get = param_get_bool,
> +};
> +#define param_check_bool_enable_only param_check_bool
> +
> +module_param(sig_enforce, bool_enable_only, 0644);
> +#endif /* 

Re: [RFC] module: signature infrastructure

2012-09-04 Thread Kasatkin, Dmitry
Hi,

Please read bellow...

On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell ru...@rustcorp.com.au wrote:
 OK, I took a look at the module.c parts of David and Dmitry's patchsets,
 and didn't really like either, but I stole parts of David's to make
 this.

 So, here's the module.c part of module signing.  I hope you two got time
 to discuss the signature format details?  Mimi suggested a scheme where
 the private key would never be saved on disk (even temporarily), but I
 didn't see patches.  Frankly it's something we can do later; let's aim
 at getting the format right for the next merge window.

In our patches key is stored on the disc in encrypted format...

More bellow..


 Cheers,
 Rusty.

 ---
 This patch doesn't compile: we need to implement:

 int mod_verify_sig(const void *mod, unsigned long modlen,
const void *sig, unsigned long siglen,
bool *sig_ok);

 Also, we need to actually append the signatures during build.

 diff --git a/Documentation/kernel-parameters.txt 
 b/Documentation/kernel-parameters.txt
 index ad7e2e5..9b2b8d3 100644
 --- a/Documentation/kernel-parameters.txt
 +++ b/Documentation/kernel-parameters.txt
 @@ -1582,6 +1582,12 @@ bytes respectively. Such letter suffixes can also be 
 entirely omitted.
 log everything. Information is printed at KERN_DEBUG
 so loglevel=8 may also need to be specified.

 +   module.sig_enforce
 +   [KNL] When CONFIG_MODULE_SIG is set, this means that
 +   modules without (valid) signatures will fail to load.
 +   Note that if CONFIG_MODULE_SIG_ENFORCE is set, that
 +   is always true, so this option does nothing.
 +
 mousedev.tap_time=
 [MOUSE] Maximum time between finger touching and
 leaving touchpad surface for touch to be considered
 diff --git a/include/linux/module.h b/include/linux/module.h
 index fbcafe2..7760c6d 100644
 --- a/include/linux/module.h
 +++ b/include/linux/module.h
 @@ -21,6 +21,9 @@
  #include linux/percpu.h
  #include asm/module.h

 +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
 +#define MODULE_SIG_STRING ~Module signature appended~\n
 +
  /* Not Yet Implemented */
  #define MODULE_SUPPORTED_DEVICE(name)

 @@ -260,6 +263,11 @@ struct module
 const unsigned long *unused_gpl_crcs;
  #endif

 +#ifdef CONFIG_MODULE_SIG
 +   /* Signature was verified. */
 +   bool sig_ok;
 +#endif
 +
 /* symbols that will be GPL-only in the near future. */
 const struct kernel_symbol *gpl_future_syms;
 const unsigned long *gpl_future_crcs;
 diff --git a/init/Kconfig b/init/Kconfig
 index af6c7f8..7452e19 100644
 --- a/init/Kconfig
 +++ b/init/Kconfig
 @@ -1585,6 +1585,20 @@ config MODULE_SRCVERSION_ALL
   the version).  With this option, such a srcversion field
   will be created for all modules.  If unsure, say N.

 +config MODULE_SIG
 +   bool Module signature verification
 +   depends on MODULES
 +   help
 + Check modules for valid signatures upon load: the signature
 + is simply appended to the module. For more information see
 + Documentation/module-signing.txt.
 +
 +config MODULE_SIG_FORCE
 +   bool Require modules to be validly signed
 +   depends on MODULE_SIG
 +   help
 + Reject unsigned modules or signed modules for which we don't have a
 + key.  Without this, such modules will simply taint the kernel.
  endif # MODULES

  config INIT_ALL_POSSIBLE
 diff --git a/kernel/module.c b/kernel/module.c
 index 4edbd9c..3cbd1a4 100644
 --- a/kernel/module.c
 +++ b/kernel/module.c
 @@ -102,6 +102,43 @@ static LIST_HEAD(modules);
  struct list_head *kdb_modules = modules; /* kdb needs the list of modules */
  #endif /* CONFIG_KGDB_KDB */

 +#ifdef CONFIG_MODULE_SIG
 +#ifdef CONFIG_MODULE_SIG_ENFORCE
 +static bool sig_enforce = true;
 +#else
 +static bool sig_enforce = false;
 +
 +static int param_set_bool_enable_only(const char *val,
 + const struct kernel_param *kp)
 +{
 +   int err;
 +   bool test;
 +   struct kernel_param dummy_kp = *kp;
 +
 +   dummy_kp.arg = test;
 +
 +   err = param_set_bool(val, dummy_kp);
 +   if (err)
 +   return err;
 +
 +   /* Don't let them unset it once it's set! */
 +   if (!test  sig_enforce)
 +   return -EROFS;
 +
 +   if (test)
 +   sig_enforce = true;
 +   return 0;
 +}
 +
 +static const struct kernel_param_ops param_ops_bool_enable_only = {
 +   .set = param_set_bool_enable_only,
 +   .get = param_get_bool,
 +};
 +#define param_check_bool_enable_only param_check_bool
 +
 +module_param(sig_enforce, bool_enable_only, 0644);
 +#endif /* !CONFIG_MODULE_SIG_ENFORCE */
 +#endif /* CONFIG_MODULE_SIG */

  /* Block module 

Re: [RFC] module: signature infrastructure

2012-09-04 Thread Kasatkin, Dmitry
On Tue, Sep 4, 2012 at 3:07 PM, Kasatkin, Dmitry
dmitry.kasat...@intel.com wrote:
 Hi,

 Please read bellow...

 On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell ru...@rustcorp.com.au wrote:
 OK, I took a look at the module.c parts of David and Dmitry's patchsets,
 and didn't really like either, but I stole parts of David's to make
 this.

 So, here's the module.c part of module signing.  I hope you two got time
 to discuss the signature format details?  Mimi suggested a scheme where
 the private key would never be saved on disk (even temporarily), but I
 didn't see patches.  Frankly it's something we can do later; let's aim
 at getting the format right for the next merge window.

 In our patches key is stored on the disc in encrypted format...

 More bellow..


 Cheers,
 Rusty.

 ---
 This patch doesn't compile: we need to implement:

 int mod_verify_sig(const void *mod, unsigned long modlen,
const void *sig, unsigned long siglen,
bool *sig_ok);

 Also, we need to actually append the signatures during build.

 diff --git a/Documentation/kernel-parameters.txt 
 b/Documentation/kernel-parameters.txt
 index ad7e2e5..9b2b8d3 100644
 --- a/Documentation/kernel-parameters.txt
 +++ b/Documentation/kernel-parameters.txt
 @@ -1582,6 +1582,12 @@ bytes respectively. Such letter suffixes can also be 
 entirely omitted.
 log everything. Information is printed at KERN_DEBUG
 so loglevel=8 may also need to be specified.

 +   module.sig_enforce
 +   [KNL] When CONFIG_MODULE_SIG is set, this means that
 +   modules without (valid) signatures will fail to load.
 +   Note that if CONFIG_MODULE_SIG_ENFORCE is set, that
 +   is always true, so this option does nothing.
 +
 mousedev.tap_time=
 [MOUSE] Maximum time between finger touching and
 leaving touchpad surface for touch to be considered
 diff --git a/include/linux/module.h b/include/linux/module.h
 index fbcafe2..7760c6d 100644
 --- a/include/linux/module.h
 +++ b/include/linux/module.h
 @@ -21,6 +21,9 @@
  #include linux/percpu.h
  #include asm/module.h

 +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
 +#define MODULE_SIG_STRING ~Module signature appended~\n
 +
  /* Not Yet Implemented */
  #define MODULE_SUPPORTED_DEVICE(name)

 @@ -260,6 +263,11 @@ struct module
 const unsigned long *unused_gpl_crcs;
  #endif

 +#ifdef CONFIG_MODULE_SIG
 +   /* Signature was verified. */
 +   bool sig_ok;
 +#endif
 +
 /* symbols that will be GPL-only in the near future. */
 const struct kernel_symbol *gpl_future_syms;
 const unsigned long *gpl_future_crcs;
 diff --git a/init/Kconfig b/init/Kconfig
 index af6c7f8..7452e19 100644
 --- a/init/Kconfig
 +++ b/init/Kconfig
 @@ -1585,6 +1585,20 @@ config MODULE_SRCVERSION_ALL
   the version).  With this option, such a srcversion field
   will be created for all modules.  If unsure, say N.

 +config MODULE_SIG
 +   bool Module signature verification
 +   depends on MODULES
 +   help
 + Check modules for valid signatures upon load: the signature
 + is simply appended to the module. For more information see
 + Documentation/module-signing.txt.
 +
 +config MODULE_SIG_FORCE
 +   bool Require modules to be validly signed
 +   depends on MODULE_SIG
 +   help
 + Reject unsigned modules or signed modules for which we don't have a
 + key.  Without this, such modules will simply taint the kernel.
  endif # MODULES

  config INIT_ALL_POSSIBLE
 diff --git a/kernel/module.c b/kernel/module.c
 index 4edbd9c..3cbd1a4 100644
 --- a/kernel/module.c
 +++ b/kernel/module.c
 @@ -102,6 +102,43 @@ static LIST_HEAD(modules);
  struct list_head *kdb_modules = modules; /* kdb needs the list of modules 
 */
  #endif /* CONFIG_KGDB_KDB */

 +#ifdef CONFIG_MODULE_SIG
 +#ifdef CONFIG_MODULE_SIG_ENFORCE
 +static bool sig_enforce = true;
 +#else
 +static bool sig_enforce = false;
 +
 +static int param_set_bool_enable_only(const char *val,
 + const struct kernel_param *kp)
 +{
 +   int err;
 +   bool test;
 +   struct kernel_param dummy_kp = *kp;
 +
 +   dummy_kp.arg = test;
 +
 +   err = param_set_bool(val, dummy_kp);
 +   if (err)
 +   return err;
 +
 +   /* Don't let them unset it once it's set! */
 +   if (!test  sig_enforce)
 +   return -EROFS;
 +
 +   if (test)
 +   sig_enforce = true;
 +   return 0;
 +}
 +
 +static const struct kernel_param_ops param_ops_bool_enable_only = {
 +   .set = param_set_bool_enable_only,
 +   .get = param_get_bool,
 +};
 +#define param_check_bool_enable_only param_check_bool
 +
 +module_param(sig_enforce, bool_enable_only, 0644);
 +#endif 

Re: [RFC] module: signature infrastructure

2012-09-04 Thread Mimi Zohar
On Tue, 2012-09-04 at 15:21 +0300, Kasatkin, Dmitry wrote:
 On Tue, Sep 4, 2012 at 3:07 PM, Kasatkin, Dmitry
 dmitry.kasat...@intel.com wrote:
  Hi,
 
  Please read bellow...
 
  On Tue, Sep 4, 2012 at 8:55 AM, Rusty Russell ru...@rustcorp.com.au wrote:
  OK, I took a look at the module.c parts of David and Dmitry's patchsets,
  and didn't really like either, but I stole parts of David's to make
  this.
 
  So, here's the module.c part of module signing.  I hope you two got time
  to discuss the signature format details?  

The integrity subsystem currently defines 3 extended attribute formats
in security/integrity.h.

enum evm_ima_xattr_type {
IMA_XATTR_DIGEST = 0x01,
EVM_XATTR_HMAC,
EVM_IMA_XATTR_DIGSIG,
};

integrity_digsig_verify() is called to appraise EVM signatures stored as
EVM_IMA_XATTR_DIGSIG.  In Dmitry's patches, this same call is used to
appraise modules.  If you decide to define a new format, it should be
included here as well.

 Mimi suggested a scheme where
  the private key would never be saved on disk (even temporarily), but I
  didn't see patches.  Frankly it's something we can do later; let's aim
  at getting the format right for the next merge window.

Right, the key is a build issue, which doesn't affect the format.

  In our patches key is stored on the disc in encrypted format...

An updated version of Dmitry's patches are in the 'modsig' branch of
git://git.kernel.org/pub/scm/linux/kernel/git/kasatkin/linux-digsig. For
now, although commit 59f1d53 modsig: build rules and scripts to
generate keys and sign modules writes the ephemeral key to disk, it is
encrypted.

thanks,

Mimi

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-04 Thread Lucas De Marchi
Hi Rusty,

On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell ru...@rustcorp.com.au wrote:
 OK, I took a look at the module.c parts of David and Dmitry's patchsets,
 and didn't really like either, but I stole parts of David's to make
 this.

 So, here's the module.c part of module signing.  I hope you two got time
 to discuss the signature format details?  Mimi suggested a scheme where
 the private key would never be saved on disk (even temporarily), but I
 didn't see patches.  Frankly it's something we can do later; let's aim
 at getting the format right for the next merge window.

 Cheers,
 Rusty.

 ---
 This patch doesn't compile: we need to implement:

 int mod_verify_sig(const void *mod, unsigned long modlen,
const void *sig, unsigned long siglen,
bool *sig_ok);

 Also, we need to actually append the signatures during build.

 diff --git a/Documentation/kernel-parameters.txt 
 b/Documentation/kernel-parameters.txt
 index ad7e2e5..9b2b8d3 100644
 --- a/Documentation/kernel-parameters.txt
 +++ b/Documentation/kernel-parameters.txt
 @@ -1582,6 +1582,12 @@ bytes respectively. Such letter suffixes can also be 
 entirely omitted.
 log everything. Information is printed at KERN_DEBUG
 so loglevel=8 may also need to be specified.

 +   module.sig_enforce
 +   [KNL] When CONFIG_MODULE_SIG is set, this means that
 +   modules without (valid) signatures will fail to load.
 +   Note that if CONFIG_MODULE_SIG_ENFORCE is set, that
 +   is always true, so this option does nothing.
 +
 mousedev.tap_time=
 [MOUSE] Maximum time between finger touching and
 leaving touchpad surface for touch to be considered
 diff --git a/include/linux/module.h b/include/linux/module.h
 index fbcafe2..7760c6d 100644
 --- a/include/linux/module.h
 +++ b/include/linux/module.h
 @@ -21,6 +21,9 @@
  #include linux/percpu.h
  #include asm/module.h

 +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
 +#define MODULE_SIG_STRING ~Module signature appended~\n
 +
  /* Not Yet Implemented */
  #define MODULE_SUPPORTED_DEVICE(name)

 @@ -260,6 +263,11 @@ struct module
 const unsigned long *unused_gpl_crcs;
  #endif

 +#ifdef CONFIG_MODULE_SIG
 +   /* Signature was verified. */
 +   bool sig_ok;
 +#endif
 +
 /* symbols that will be GPL-only in the near future. */
 const struct kernel_symbol *gpl_future_syms;
 const unsigned long *gpl_future_crcs;
 diff --git a/init/Kconfig b/init/Kconfig
 index af6c7f8..7452e19 100644
 --- a/init/Kconfig
 +++ b/init/Kconfig
 @@ -1585,6 +1585,20 @@ config MODULE_SRCVERSION_ALL
   the version).  With this option, such a srcversion field
   will be created for all modules.  If unsure, say N.

 +config MODULE_SIG
 +   bool Module signature verification
 +   depends on MODULES
 +   help
 + Check modules for valid signatures upon load: the signature
 + is simply appended to the module. For more information see
 + Documentation/module-signing.txt.
 +
 +config MODULE_SIG_FORCE
 +   bool Require modules to be validly signed
 +   depends on MODULE_SIG
 +   help
 + Reject unsigned modules or signed modules for which we don't have a
 + key.  Without this, such modules will simply taint the kernel.
  endif # MODULES

  config INIT_ALL_POSSIBLE
 diff --git a/kernel/module.c b/kernel/module.c
 index 4edbd9c..3cbd1a4 100644
 --- a/kernel/module.c
 +++ b/kernel/module.c
 @@ -102,6 +102,43 @@ static LIST_HEAD(modules);
  struct list_head *kdb_modules = modules; /* kdb needs the list of modules */
  #endif /* CONFIG_KGDB_KDB */

 +#ifdef CONFIG_MODULE_SIG
 +#ifdef CONFIG_MODULE_SIG_ENFORCE
 +static bool sig_enforce = true;
 +#else
 +static bool sig_enforce = false;
 +
 +static int param_set_bool_enable_only(const char *val,
 + const struct kernel_param *kp)
 +{
 +   int err;
 +   bool test;
 +   struct kernel_param dummy_kp = *kp;
 +
 +   dummy_kp.arg = test;
 +
 +   err = param_set_bool(val, dummy_kp);
 +   if (err)
 +   return err;
 +
 +   /* Don't let them unset it once it's set! */
 +   if (!test  sig_enforce)
 +   return -EROFS;
 +
 +   if (test)
 +   sig_enforce = true;
 +   return 0;
 +}
 +
 +static const struct kernel_param_ops param_ops_bool_enable_only = {
 +   .set = param_set_bool_enable_only,
 +   .get = param_get_bool,
 +};
 +#define param_check_bool_enable_only param_check_bool
 +
 +module_param(sig_enforce, bool_enable_only, 0644);
 +#endif /* !CONFIG_MODULE_SIG_ENFORCE */
 +#endif /* CONFIG_MODULE_SIG */

  /* Block module loading/unloading? */
  int modules_disabled = 0;
 @@ -136,6 +173,7 @@ struct load_info {
 

Re: [RFC] module: signature infrastructure

2012-09-04 Thread Kasatkin, Dmitry
On Tue, Sep 4, 2012 at 5:25 PM, Lucas De Marchi
lucas.de.mar...@gmail.com wrote:
 Hi Rusty,

 On Tue, Sep 4, 2012 at 2:55 AM, Rusty Russell ru...@rustcorp.com.au wrote:
 OK, I took a look at the module.c parts of David and Dmitry's patchsets,
 and didn't really like either, but I stole parts of David's to make
 this.

 So, here's the module.c part of module signing.  I hope you two got time
 to discuss the signature format details?  Mimi suggested a scheme where
 the private key would never be saved on disk (even temporarily), but I
 didn't see patches.  Frankly it's something we can do later; let's aim
 at getting the format right for the next merge window.

 Cheers,
 Rusty.

 ---
 This patch doesn't compile: we need to implement:

 int mod_verify_sig(const void *mod, unsigned long modlen,
const void *sig, unsigned long siglen,
bool *sig_ok);

 Also, we need to actually append the signatures during build.

 diff --git a/Documentation/kernel-parameters.txt 
 b/Documentation/kernel-parameters.txt
 index ad7e2e5..9b2b8d3 100644
 --- a/Documentation/kernel-parameters.txt
 +++ b/Documentation/kernel-parameters.txt
 @@ -1582,6 +1582,12 @@ bytes respectively. Such letter suffixes can also be 
 entirely omitted.
 log everything. Information is printed at KERN_DEBUG
 so loglevel=8 may also need to be specified.

 +   module.sig_enforce
 +   [KNL] When CONFIG_MODULE_SIG is set, this means that
 +   modules without (valid) signatures will fail to load.
 +   Note that if CONFIG_MODULE_SIG_ENFORCE is set, that
 +   is always true, so this option does nothing.
 +
 mousedev.tap_time=
 [MOUSE] Maximum time between finger touching and
 leaving touchpad surface for touch to be considered
 diff --git a/include/linux/module.h b/include/linux/module.h
 index fbcafe2..7760c6d 100644
 --- a/include/linux/module.h
 +++ b/include/linux/module.h
 @@ -21,6 +21,9 @@
  #include linux/percpu.h
  #include asm/module.h

 +/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
 +#define MODULE_SIG_STRING ~Module signature appended~\n
 +
  /* Not Yet Implemented */
  #define MODULE_SUPPORTED_DEVICE(name)

 @@ -260,6 +263,11 @@ struct module
 const unsigned long *unused_gpl_crcs;
  #endif

 +#ifdef CONFIG_MODULE_SIG
 +   /* Signature was verified. */
 +   bool sig_ok;
 +#endif
 +
 /* symbols that will be GPL-only in the near future. */
 const struct kernel_symbol *gpl_future_syms;
 const unsigned long *gpl_future_crcs;
 diff --git a/init/Kconfig b/init/Kconfig
 index af6c7f8..7452e19 100644
 --- a/init/Kconfig
 +++ b/init/Kconfig
 @@ -1585,6 +1585,20 @@ config MODULE_SRCVERSION_ALL
   the version).  With this option, such a srcversion field
   will be created for all modules.  If unsure, say N.

 +config MODULE_SIG
 +   bool Module signature verification
 +   depends on MODULES
 +   help
 + Check modules for valid signatures upon load: the signature
 + is simply appended to the module. For more information see
 + Documentation/module-signing.txt.
 +
 +config MODULE_SIG_FORCE
 +   bool Require modules to be validly signed
 +   depends on MODULE_SIG
 +   help
 + Reject unsigned modules or signed modules for which we don't have a
 + key.  Without this, such modules will simply taint the kernel.
  endif # MODULES

  config INIT_ALL_POSSIBLE
 diff --git a/kernel/module.c b/kernel/module.c
 index 4edbd9c..3cbd1a4 100644
 --- a/kernel/module.c
 +++ b/kernel/module.c
 @@ -102,6 +102,43 @@ static LIST_HEAD(modules);
  struct list_head *kdb_modules = modules; /* kdb needs the list of modules 
 */
  #endif /* CONFIG_KGDB_KDB */

 +#ifdef CONFIG_MODULE_SIG
 +#ifdef CONFIG_MODULE_SIG_ENFORCE
 +static bool sig_enforce = true;
 +#else
 +static bool sig_enforce = false;
 +
 +static int param_set_bool_enable_only(const char *val,
 + const struct kernel_param *kp)
 +{
 +   int err;
 +   bool test;
 +   struct kernel_param dummy_kp = *kp;
 +
 +   dummy_kp.arg = test;
 +
 +   err = param_set_bool(val, dummy_kp);
 +   if (err)
 +   return err;
 +
 +   /* Don't let them unset it once it's set! */
 +   if (!test  sig_enforce)
 +   return -EROFS;
 +
 +   if (test)
 +   sig_enforce = true;
 +   return 0;
 +}
 +
 +static const struct kernel_param_ops param_ops_bool_enable_only = {
 +   .set = param_set_bool_enable_only,
 +   .get = param_get_bool,
 +};
 +#define param_check_bool_enable_only param_check_bool
 +
 +module_param(sig_enforce, bool_enable_only, 0644);
 +#endif /* !CONFIG_MODULE_SIG_ENFORCE */
 +#endif /* CONFIG_MODULE_SIG */

  /* Block module 

Re: [RFC] module: signature infrastructure

2012-09-04 Thread David Howells
Lucas De Marchi lucas.de.mar...@gmail.com wrote:

 Or let the magic string as the last thing in the module and store the
 signature length, too. In this case no scanning is needed

Indeed.  This is the better way.

The main problem is rendering the length from a shell script.  It's trivial to
do as ASCII (there's a printf program), but a pain to render to binary.  I'm
sure it can be done with perl or python without the need to compile anything.

David
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] module: signature infrastructure

2012-09-04 Thread Kasatkin, Dmitry
On Wed, Sep 5, 2012 at 1:51 AM, David Howells dhowe...@redhat.com wrote:
 Lucas De Marchi lucas.de.mar...@gmail.com wrote:

 Or let the magic string as the last thing in the module and store the
 signature length, too. In this case no scanning is needed

 Indeed.  This is the better way.

 The main problem is rendering the length from a shell script.  It's trivial to
 do as ASCII (there's a printf program), but a pain to render to binary.  I'm
 sure it can be done with perl or python without the need to compile anything.

 David

That is very easy to do from script as well.
See script in my tree.

http://git.kernel.org/?p=linux/kernel/git/kasatkin/linux-digsig.git;a=blob;f=scripts/modsig.sh;h=4e997c3996d71d8e1afeb3a7afe23b3f303b9f63;hb=59f1d5352969166f2f32f84e07e20dd1b30a890f

110 # add signature length - big endian
111 dec2hex $(stat --printf %s $sigfile) 4 | hex2bin $sigfile
112 echo -n This Is A Crypto Signed Module $sigfile

well. I have couple of small functions in the script. dec2hex and hex2bin

- Dmitry
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/