Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-16 Thread joeyli
On Thu, Mar 15, 2018 at 07:30:26AM -0700, James Bottomley wrote:
> On Thu, 2018-03-15 at 14:16 +0800, joeyli wrote:
> > On Wed, Mar 14, 2018 at 07:19:25AM -0700, James Bottomley wrote:
> > > 
> > > On Wed, 2018-03-14 at 14:08 +0800, joeyli wrote:
> > > > 
> > > > On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> > > > > 
> > > > > 
> > > > > On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > > > > > 
> > > > > > 
> > > > > > This patch adds the logic for checking the kernel module's
> > > > > > hash base on blacklist. The hash must be generated by sha256
> > > > > > and enrolled to dbx/mokx.
> > > > > > 
> > > > > > For example:
> > > > > > sha256sum sample.ko
> > > > > > mokutil --mokx --import-hash $HASH_RESULT
> > > > > > 
> > > > > > Whether the signature on ko file is stripped or not, the hash
> > > > > > can be compared by kernel.
> > > > > 
> > > > > What's the use case for this?  We're already in trouble from
> > > > > the ODMs for the size of dbx and its consumption of the
> > > > > extremely limited variable space, so do we really have a use
> > > > > case for adding module blacklist hashes to the UEFI variables
> > > > > given the space constraints (as in one we can't do any other
> > > > > way)?
> > > > > 
> > > > 
> > > > The dbx is a authenticated variable that it can only be updated
> > > > by manufacturer. The mokx gives a flexible way for distro to
> > > > revoke a key or a signed module. Then we don't need to touch shim
> > > > or bother manufacturer to deliver new db. Currently it doesn't
> > > > have real use case yet. 
> > > > 
> > > > I knew that the NVRAM has limited space. But distro needs a
> > > > backup solution for emergency.
> > > 
> > > I wasn't asking why the variable, I was asking why the mechanism.
> > > 
> > > OK, let me try to ask the question in a different way:
> > > 
> > > Why would the distribution need to blacklist a module in this way?
> > >  For
> > 
> > This way is a new option for user to blacklist a module but not the
> > only way.
> 
> So this is for the *user* not the distribution?
> 
> >  MOK has this ability because shim implements the mokx by signature
> > database format (EFI_SIGNATURE_DATA in UEFI spec). This format
> > supports both hash signature and x.509 certificate.
> > 
> > > 
> > > the distro to execute the script to add this blacklist, means the
> > > system is getting automated or manual updates ... can't those
> > > updates just remove the module?
> > > 
> > Yes, we can just remove or update the module in kernel rpm or kmp.
> > But user may re-install distro with old kernel or install a old kmp.
> > If the blacklist hash was stored in variable, then kernel can prevent
> > to load the module.
> > 
> > On the other hand, for enrolling mokx, user must reboots system and
> > deals with shim-mokmanager UI. It's more secure because user should
> > really know what he does. And user can choice not to enroll the hash
> > if they still want to use the module.
> 
> OK, so now the use case is the user needs to roll back but doesn't want
> a module to load ... I've got to say that in that case I'd just remove
> it before reload.
> 
> > > The point is that module sha sums are pretty ephemeral in our model
> > > (they change with every kernel), so it seems to be a mismatch to
> > > place them in a permanent blacklist, particularly when we have very
> > > limited space for that list.
> > > 
> > Normally we run a serious process for signing a kernel module before
> > shipping it to customer. The SUSE's "Partner Linux Driver Program”
> > (PLDP) is an example. So the module sha sums are not too ephemeral.
> 
> Ephemeral isn't about the signing process it means that the sum is
> short lived because every time you create a module for a specific
> kernel its sum changes (because of the interface versioning) so your
> blacklist only applies to one module and specific kernel combination.
>  Once you compile it for a different kernel you need a different
> blacklist sum for it.
>

I agree with you that the sum is ephemeral. I will remove this patch
from the mokx series. The certificate in mokx will be loaded to
blacklist keyring. Which means that we still can use mokx to revoke
public key. But kernel will not check the blacklisted hash before
loading kernel module.

Thanks a lot!
Joey Lee


Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-16 Thread joeyli
On Thu, Mar 15, 2018 at 07:30:26AM -0700, James Bottomley wrote:
> On Thu, 2018-03-15 at 14:16 +0800, joeyli wrote:
> > On Wed, Mar 14, 2018 at 07:19:25AM -0700, James Bottomley wrote:
> > > 
> > > On Wed, 2018-03-14 at 14:08 +0800, joeyli wrote:
> > > > 
> > > > On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> > > > > 
> > > > > 
> > > > > On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > > > > > 
> > > > > > 
> > > > > > This patch adds the logic for checking the kernel module's
> > > > > > hash base on blacklist. The hash must be generated by sha256
> > > > > > and enrolled to dbx/mokx.
> > > > > > 
> > > > > > For example:
> > > > > > sha256sum sample.ko
> > > > > > mokutil --mokx --import-hash $HASH_RESULT
> > > > > > 
> > > > > > Whether the signature on ko file is stripped or not, the hash
> > > > > > can be compared by kernel.
> > > > > 
> > > > > What's the use case for this?  We're already in trouble from
> > > > > the ODMs for the size of dbx and its consumption of the
> > > > > extremely limited variable space, so do we really have a use
> > > > > case for adding module blacklist hashes to the UEFI variables
> > > > > given the space constraints (as in one we can't do any other
> > > > > way)?
> > > > > 
> > > > 
> > > > The dbx is a authenticated variable that it can only be updated
> > > > by manufacturer. The mokx gives a flexible way for distro to
> > > > revoke a key or a signed module. Then we don't need to touch shim
> > > > or bother manufacturer to deliver new db. Currently it doesn't
> > > > have real use case yet. 
> > > > 
> > > > I knew that the NVRAM has limited space. But distro needs a
> > > > backup solution for emergency.
> > > 
> > > I wasn't asking why the variable, I was asking why the mechanism.
> > > 
> > > OK, let me try to ask the question in a different way:
> > > 
> > > Why would the distribution need to blacklist a module in this way?
> > >  For
> > 
> > This way is a new option for user to blacklist a module but not the
> > only way.
> 
> So this is for the *user* not the distribution?
> 
> >  MOK has this ability because shim implements the mokx by signature
> > database format (EFI_SIGNATURE_DATA in UEFI spec). This format
> > supports both hash signature and x.509 certificate.
> > 
> > > 
> > > the distro to execute the script to add this blacklist, means the
> > > system is getting automated or manual updates ... can't those
> > > updates just remove the module?
> > > 
> > Yes, we can just remove or update the module in kernel rpm or kmp.
> > But user may re-install distro with old kernel or install a old kmp.
> > If the blacklist hash was stored in variable, then kernel can prevent
> > to load the module.
> > 
> > On the other hand, for enrolling mokx, user must reboots system and
> > deals with shim-mokmanager UI. It's more secure because user should
> > really know what he does. And user can choice not to enroll the hash
> > if they still want to use the module.
> 
> OK, so now the use case is the user needs to roll back but doesn't want
> a module to load ... I've got to say that in that case I'd just remove
> it before reload.
> 
> > > The point is that module sha sums are pretty ephemeral in our model
> > > (they change with every kernel), so it seems to be a mismatch to
> > > place them in a permanent blacklist, particularly when we have very
> > > limited space for that list.
> > > 
> > Normally we run a serious process for signing a kernel module before
> > shipping it to customer. The SUSE's "Partner Linux Driver Program”
> > (PLDP) is an example. So the module sha sums are not too ephemeral.
> 
> Ephemeral isn't about the signing process it means that the sum is
> short lived because every time you create a module for a specific
> kernel its sum changes (because of the interface versioning) so your
> blacklist only applies to one module and specific kernel combination.
>  Once you compile it for a different kernel you need a different
> blacklist sum for it.
>

I agree with you that the sum is ephemeral. I will remove this patch
from the mokx series. The certificate in mokx will be loaded to
blacklist keyring. Which means that we still can use mokx to revoke
public key. But kernel will not check the blacklisted hash before
loading kernel module.

Thanks a lot!
Joey Lee


Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-15 Thread James Bottomley
On Thu, 2018-03-15 at 14:16 +0800, joeyli wrote:
> On Wed, Mar 14, 2018 at 07:19:25AM -0700, James Bottomley wrote:
> > 
> > On Wed, 2018-03-14 at 14:08 +0800, joeyli wrote:
> > > 
> > > On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> > > > 
> > > > 
> > > > On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > > > > 
> > > > > 
> > > > > This patch adds the logic for checking the kernel module's
> > > > > hash base on blacklist. The hash must be generated by sha256
> > > > > and enrolled to dbx/mokx.
> > > > > 
> > > > > For example:
> > > > >   sha256sum sample.ko
> > > > >   mokutil --mokx --import-hash $HASH_RESULT
> > > > > 
> > > > > Whether the signature on ko file is stripped or not, the hash
> > > > > can be compared by kernel.
> > > > 
> > > > What's the use case for this?  We're already in trouble from
> > > > the ODMs for the size of dbx and its consumption of the
> > > > extremely limited variable space, so do we really have a use
> > > > case for adding module blacklist hashes to the UEFI variables
> > > > given the space constraints (as in one we can't do any other
> > > > way)?
> > > > 
> > > 
> > > The dbx is a authenticated variable that it can only be updated
> > > by manufacturer. The mokx gives a flexible way for distro to
> > > revoke a key or a signed module. Then we don't need to touch shim
> > > or bother manufacturer to deliver new db. Currently it doesn't
> > > have real use case yet. 
> > > 
> > > I knew that the NVRAM has limited space. But distro needs a
> > > backup solution for emergency.
> > 
> > I wasn't asking why the variable, I was asking why the mechanism.
> > 
> > OK, let me try to ask the question in a different way:
> > 
> > Why would the distribution need to blacklist a module in this way?
> >  For
> 
> This way is a new option for user to blacklist a module but not the
> only way.

So this is for the *user* not the distribution?

>  MOK has this ability because shim implements the mokx by signature
> database format (EFI_SIGNATURE_DATA in UEFI spec). This format
> supports both hash signature and x.509 certificate.
> 
> > 
> > the distro to execute the script to add this blacklist, means the
> > system is getting automated or manual updates ... can't those
> > updates just remove the module?
> > 
> Yes, we can just remove or update the module in kernel rpm or kmp.
> But user may re-install distro with old kernel or install a old kmp.
> If the blacklist hash was stored in variable, then kernel can prevent
> to load the module.
> 
> On the other hand, for enrolling mokx, user must reboots system and
> deals with shim-mokmanager UI. It's more secure because user should
> really know what he does. And user can choice not to enroll the hash
> if they still want to use the module.

OK, so now the use case is the user needs to roll back but doesn't want
a module to load ... I've got to say that in that case I'd just remove
it before reload.

> > The point is that module sha sums are pretty ephemeral in our model
> > (they change with every kernel), so it seems to be a mismatch to
> > place them in a permanent blacklist, particularly when we have very
> > limited space for that list.
> > 
> Normally we run a serious process for signing a kernel module before
> shipping it to customer. The SUSE's "Partner Linux Driver Program”
> (PLDP) is an example. So the module sha sums are not too ephemeral.

Ephemeral isn't about the signing process it means that the sum is
short lived because every time you create a module for a specific
kernel its sum changes (because of the interface versioning) so your
blacklist only applies to one module and specific kernel combination.
 Once you compile it for a different kernel you need a different
blacklist sum for it.

James



Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-15 Thread James Bottomley
On Thu, 2018-03-15 at 14:16 +0800, joeyli wrote:
> On Wed, Mar 14, 2018 at 07:19:25AM -0700, James Bottomley wrote:
> > 
> > On Wed, 2018-03-14 at 14:08 +0800, joeyli wrote:
> > > 
> > > On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> > > > 
> > > > 
> > > > On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > > > > 
> > > > > 
> > > > > This patch adds the logic for checking the kernel module's
> > > > > hash base on blacklist. The hash must be generated by sha256
> > > > > and enrolled to dbx/mokx.
> > > > > 
> > > > > For example:
> > > > >   sha256sum sample.ko
> > > > >   mokutil --mokx --import-hash $HASH_RESULT
> > > > > 
> > > > > Whether the signature on ko file is stripped or not, the hash
> > > > > can be compared by kernel.
> > > > 
> > > > What's the use case for this?  We're already in trouble from
> > > > the ODMs for the size of dbx and its consumption of the
> > > > extremely limited variable space, so do we really have a use
> > > > case for adding module blacklist hashes to the UEFI variables
> > > > given the space constraints (as in one we can't do any other
> > > > way)?
> > > > 
> > > 
> > > The dbx is a authenticated variable that it can only be updated
> > > by manufacturer. The mokx gives a flexible way for distro to
> > > revoke a key or a signed module. Then we don't need to touch shim
> > > or bother manufacturer to deliver new db. Currently it doesn't
> > > have real use case yet. 
> > > 
> > > I knew that the NVRAM has limited space. But distro needs a
> > > backup solution for emergency.
> > 
> > I wasn't asking why the variable, I was asking why the mechanism.
> > 
> > OK, let me try to ask the question in a different way:
> > 
> > Why would the distribution need to blacklist a module in this way?
> >  For
> 
> This way is a new option for user to blacklist a module but not the
> only way.

So this is for the *user* not the distribution?

>  MOK has this ability because shim implements the mokx by signature
> database format (EFI_SIGNATURE_DATA in UEFI spec). This format
> supports both hash signature and x.509 certificate.
> 
> > 
> > the distro to execute the script to add this blacklist, means the
> > system is getting automated or manual updates ... can't those
> > updates just remove the module?
> > 
> Yes, we can just remove or update the module in kernel rpm or kmp.
> But user may re-install distro with old kernel or install a old kmp.
> If the blacklist hash was stored in variable, then kernel can prevent
> to load the module.
> 
> On the other hand, for enrolling mokx, user must reboots system and
> deals with shim-mokmanager UI. It's more secure because user should
> really know what he does. And user can choice not to enroll the hash
> if they still want to use the module.

OK, so now the use case is the user needs to roll back but doesn't want
a module to load ... I've got to say that in that case I'd just remove
it before reload.

> > The point is that module sha sums are pretty ephemeral in our model
> > (they change with every kernel), so it seems to be a mismatch to
> > place them in a permanent blacklist, particularly when we have very
> > limited space for that list.
> > 
> Normally we run a serious process for signing a kernel module before
> shipping it to customer. The SUSE's "Partner Linux Driver Program”
> (PLDP) is an example. So the module sha sums are not too ephemeral.

Ephemeral isn't about the signing process it means that the sum is
short lived because every time you create a module for a specific
kernel its sum changes (because of the interface versioning) so your
blacklist only applies to one module and specific kernel combination.
 Once you compile it for a different kernel you need a different
blacklist sum for it.

James



Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-15 Thread joeyli
On Wed, Mar 14, 2018 at 07:19:25AM -0700, James Bottomley wrote:
> On Wed, 2018-03-14 at 14:08 +0800, joeyli wrote:
> > On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> > > 
> > > On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > > > 
> > > > This patch adds the logic for checking the kernel module's hash
> > > > base on blacklist. The hash must be generated by sha256 and
> > > > enrolled
> > > > to dbx/mokx.
> > > > 
> > > > For example:
> > > > sha256sum sample.ko
> > > > mokutil --mokx --import-hash $HASH_RESULT
> > > > 
> > > > Whether the signature on ko file is stripped or not, the hash can
> > > > be
> > > > compared by kernel.
> > > 
> > > What's the use case for this?  We're already in trouble from the
> > > ODMs for the size of dbx and its consumption of the extremely
> > > limited variable space, so do we really have a use case for adding
> > > module blacklist hashes to the UEFI variables given the space
> > > constraints (as in one we can't do any other way)?
> > > 
> > 
> > The dbx is a authenticated variable that it can only be updated by
> > manufacturer. The mokx gives a flexible way for distro to revoke a
> > key or a signed module. Then we don't need to touch shim or bother
> > manufacturer to deliver new db. Currently it doesn't have real use
> > case yet. 
> > 
> > I knew that the NVRAM has limited space. But distro needs a backup
> > solution for emergency.
> 
> I wasn't asking why the variable, I was asking why the mechanism.
> 
> OK, let me try to ask the question in a different way:
> 
> Why would the distribution need to blacklist a module in this way?  For

This way is a new option for user to blacklist a module but not the only
way. MOK has this ability because shim implements the mokx by signature
database format (EFI_SIGNATURE_DATA in UEFI spec). This format supports
both hash signature and x.509 certificate.

> the distro to execute the script to add this blacklist, means the
> system is getting automated or manual updates ... can't those updates
> just remove the module?
>
Yes, we can just remove or update the module in kernel rpm or kmp. But
user may re-install distro with old kernel or install a old kmp. If
the blacklist hash was stored in variable, then kernel can prevent
to load the module.

On the other hand, for enrolling mokx, user must reboots system and
deals with shim-mokmanager UI. It's more secure because user should
really know what he does. And user can choice not to enroll the hash
if they still want to use the module.

> The point is that module sha sums are pretty ephemeral in our model
> (they change with every kernel), so it seems to be a mismatch to place
> them in a permanent blacklist, particularly when we have very limited
> space for that list.
>
Normally we run a serious process for signing a kernel module before
shipping it to customer. The SUSE's "Partner Linux Driver Program” (PLDP)
is an example. So the module sha sums are not too ephemeral.

I agree with you for the space is limit. But the mokx gives a option to
distro or user to blacklist kernel module. They can choice to use this
mechanism or not. 

Thanks a lot!
Joey Lee


Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-15 Thread joeyli
On Wed, Mar 14, 2018 at 07:19:25AM -0700, James Bottomley wrote:
> On Wed, 2018-03-14 at 14:08 +0800, joeyli wrote:
> > On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> > > 
> > > On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > > > 
> > > > This patch adds the logic for checking the kernel module's hash
> > > > base on blacklist. The hash must be generated by sha256 and
> > > > enrolled
> > > > to dbx/mokx.
> > > > 
> > > > For example:
> > > > sha256sum sample.ko
> > > > mokutil --mokx --import-hash $HASH_RESULT
> > > > 
> > > > Whether the signature on ko file is stripped or not, the hash can
> > > > be
> > > > compared by kernel.
> > > 
> > > What's the use case for this?  We're already in trouble from the
> > > ODMs for the size of dbx and its consumption of the extremely
> > > limited variable space, so do we really have a use case for adding
> > > module blacklist hashes to the UEFI variables given the space
> > > constraints (as in one we can't do any other way)?
> > > 
> > 
> > The dbx is a authenticated variable that it can only be updated by
> > manufacturer. The mokx gives a flexible way for distro to revoke a
> > key or a signed module. Then we don't need to touch shim or bother
> > manufacturer to deliver new db. Currently it doesn't have real use
> > case yet. 
> > 
> > I knew that the NVRAM has limited space. But distro needs a backup
> > solution for emergency.
> 
> I wasn't asking why the variable, I was asking why the mechanism.
> 
> OK, let me try to ask the question in a different way:
> 
> Why would the distribution need to blacklist a module in this way?  For

This way is a new option for user to blacklist a module but not the only
way. MOK has this ability because shim implements the mokx by signature
database format (EFI_SIGNATURE_DATA in UEFI spec). This format supports
both hash signature and x.509 certificate.

> the distro to execute the script to add this blacklist, means the
> system is getting automated or manual updates ... can't those updates
> just remove the module?
>
Yes, we can just remove or update the module in kernel rpm or kmp. But
user may re-install distro with old kernel or install a old kmp. If
the blacklist hash was stored in variable, then kernel can prevent
to load the module.

On the other hand, for enrolling mokx, user must reboots system and
deals with shim-mokmanager UI. It's more secure because user should
really know what he does. And user can choice not to enroll the hash
if they still want to use the module.

> The point is that module sha sums are pretty ephemeral in our model
> (they change with every kernel), so it seems to be a mismatch to place
> them in a permanent blacklist, particularly when we have very limited
> space for that list.
>
Normally we run a serious process for signing a kernel module before
shipping it to customer. The SUSE's "Partner Linux Driver Program” (PLDP)
is an example. So the module sha sums are not too ephemeral.

I agree with you for the space is limit. But the mokx gives a option to
distro or user to blacklist kernel module. They can choice to use this
mechanism or not. 

Thanks a lot!
Joey Lee


Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-14 Thread James Bottomley
On Wed, 2018-03-14 at 14:08 +0800, joeyli wrote:
> On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> > 
> > On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > > 
> > > This patch adds the logic for checking the kernel module's hash
> > > base on blacklist. The hash must be generated by sha256 and
> > > enrolled
> > > to dbx/mokx.
> > > 
> > > For example:
> > >   sha256sum sample.ko
> > >   mokutil --mokx --import-hash $HASH_RESULT
> > > 
> > > Whether the signature on ko file is stripped or not, the hash can
> > > be
> > > compared by kernel.
> > 
> > What's the use case for this?  We're already in trouble from the
> > ODMs for the size of dbx and its consumption of the extremely
> > limited variable space, so do we really have a use case for adding
> > module blacklist hashes to the UEFI variables given the space
> > constraints (as in one we can't do any other way)?
> > 
> 
> The dbx is a authenticated variable that it can only be updated by
> manufacturer. The mokx gives a flexible way for distro to revoke a
> key or a signed module. Then we don't need to touch shim or bother
> manufacturer to deliver new db. Currently it doesn't have real use
> case yet. 
> 
> I knew that the NVRAM has limited space. But distro needs a backup
> solution for emergency.

I wasn't asking why the variable, I was asking why the mechanism.

OK, let me try to ask the question in a different way:

Why would the distribution need to blacklist a module in this way?  For
the distro to execute the script to add this blacklist, means the
system is getting automated or manual updates ... can't those updates
just remove the module?

The point is that module sha sums are pretty ephemeral in our model
(they change with every kernel), so it seems to be a mismatch to place
them in a permanent blacklist, particularly when we have very limited
space for that list.

James



Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-14 Thread James Bottomley
On Wed, 2018-03-14 at 14:08 +0800, joeyli wrote:
> On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> > 
> > On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > > 
> > > This patch adds the logic for checking the kernel module's hash
> > > base on blacklist. The hash must be generated by sha256 and
> > > enrolled
> > > to dbx/mokx.
> > > 
> > > For example:
> > >   sha256sum sample.ko
> > >   mokutil --mokx --import-hash $HASH_RESULT
> > > 
> > > Whether the signature on ko file is stripped or not, the hash can
> > > be
> > > compared by kernel.
> > 
> > What's the use case for this?  We're already in trouble from the
> > ODMs for the size of dbx and its consumption of the extremely
> > limited variable space, so do we really have a use case for adding
> > module blacklist hashes to the UEFI variables given the space
> > constraints (as in one we can't do any other way)?
> > 
> 
> The dbx is a authenticated variable that it can only be updated by
> manufacturer. The mokx gives a flexible way for distro to revoke a
> key or a signed module. Then we don't need to touch shim or bother
> manufacturer to deliver new db. Currently it doesn't have real use
> case yet. 
> 
> I knew that the NVRAM has limited space. But distro needs a backup
> solution for emergency.

I wasn't asking why the variable, I was asking why the mechanism.

OK, let me try to ask the question in a different way:

Why would the distribution need to blacklist a module in this way?  For
the distro to execute the script to add this blacklist, means the
system is getting automated or manual updates ... can't those updates
just remove the module?

The point is that module sha sums are pretty ephemeral in our model
(they change with every kernel), so it seems to be a mismatch to place
them in a permanent blacklist, particularly when we have very limited
space for that list.

James



Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-14 Thread joeyli
On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > This patch adds the logic for checking the kernel module's hash
> > base on blacklist. The hash must be generated by sha256 and enrolled
> > to dbx/mokx.
> > 
> > For example:
> > sha256sum sample.ko
> > mokutil --mokx --import-hash $HASH_RESULT
> > 
> > Whether the signature on ko file is stripped or not, the hash can be
> > compared by kernel.
> 
> What's the use case for this?  We're already in trouble from the ODMs
> for the size of dbx and its consumption of the extremely limited
> variable space, so do we really have a use case for adding module
> blacklist hashes to the UEFI variables given the space constraints (as
> in one we can't do any other way)?
>

The dbx is a authenticated variable that it can only be updated by
manufacturer. The mokx gives a flexible way for distro to revoke a key
or a signed module. Then we don't need to touch shim or bother
manufacturer to deliver new db. Currently it doesn't have real use
case yet. 

I knew that the NVRAM has limited space. But distro needs a backup
solution for emergency.

Thanks a lot!
Joey Lee 


Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-14 Thread joeyli
On Tue, Mar 13, 2018 at 10:18:35AM -0700, James Bottomley wrote:
> On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> > This patch adds the logic for checking the kernel module's hash
> > base on blacklist. The hash must be generated by sha256 and enrolled
> > to dbx/mokx.
> > 
> > For example:
> > sha256sum sample.ko
> > mokutil --mokx --import-hash $HASH_RESULT
> > 
> > Whether the signature on ko file is stripped or not, the hash can be
> > compared by kernel.
> 
> What's the use case for this?  We're already in trouble from the ODMs
> for the size of dbx and its consumption of the extremely limited
> variable space, so do we really have a use case for adding module
> blacklist hashes to the UEFI variables given the space constraints (as
> in one we can't do any other way)?
>

The dbx is a authenticated variable that it can only be updated by
manufacturer. The mokx gives a flexible way for distro to revoke a key
or a signed module. Then we don't need to touch shim or bother
manufacturer to deliver new db. Currently it doesn't have real use
case yet. 

I knew that the NVRAM has limited space. But distro needs a backup
solution for emergency.

Thanks a lot!
Joey Lee 


Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-13 Thread James Bottomley
On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> This patch adds the logic for checking the kernel module's hash
> base on blacklist. The hash must be generated by sha256 and enrolled
> to dbx/mokx.
> 
> For example:
>   sha256sum sample.ko
>   mokutil --mokx --import-hash $HASH_RESULT
> 
> Whether the signature on ko file is stripped or not, the hash can be
> compared by kernel.

What's the use case for this?  We're already in trouble from the ODMs
for the size of dbx and its consumption of the extremely limited
variable space, so do we really have a use case for adding module
blacklist hashes to the UEFI variables given the space constraints (as
in one we can't do any other way)?

James



Re: [PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-13 Thread James Bottomley
On Tue, 2018-03-13 at 18:38 +0800, Lee, Chun-Yi wrote:
> This patch adds the logic for checking the kernel module's hash
> base on blacklist. The hash must be generated by sha256 and enrolled
> to dbx/mokx.
> 
> For example:
>   sha256sum sample.ko
>   mokutil --mokx --import-hash $HASH_RESULT
> 
> Whether the signature on ko file is stripped or not, the hash can be
> compared by kernel.

What's the use case for this?  We're already in trouble from the ODMs
for the size of dbx and its consumption of the extremely limited
variable space, so do we really have a use case for adding module
blacklist hashes to the UEFI variables given the space constraints (as
in one we can't do any other way)?

James



[PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-13 Thread Lee, Chun-Yi
This patch adds the logic for checking the kernel module's hash
base on blacklist. The hash must be generated by sha256 and enrolled
to dbx/mokx.

For example:
sha256sum sample.ko
mokutil --mokx --import-hash $HASH_RESULT

Whether the signature on ko file is stripped or not, the hash can be
compared by kernel.

Cc: David Howells 
Cc: Josh Boyer 
Cc: James Bottomley 
Signed-off-by: "Lee, Chun-Yi" 
---
 kernel/module_signing.c | 62 +++--
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index d3d6f95..d30ac74 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,9 +11,12 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "module-internal.h"
 
 enum pkey_id_type {
@@ -42,19 +45,67 @@ struct module_signature {
__be32  sig_len;/* Length of signature data */
 };
 
+static int mod_is_hash_blacklisted(const void *mod, size_t verifylen)
+{
+   struct crypto_shash *tfm;
+   struct shash_desc *desc;
+   size_t digest_size, desc_size;
+   u8 *digest;
+   int ret = 0;
+
+   tfm = crypto_alloc_shash("sha256", 0, 0);
+   if (IS_ERR(tfm))
+   goto error_return;
+
+   desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
+   digest_size = crypto_shash_digestsize(tfm);
+   digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
+   if (!digest) {
+   pr_err("digest memory buffer allocate fail\n");
+   ret = -ENOMEM;
+   goto error_digest;
+   }
+   desc = (void *)digest + digest_size;
+   desc->tfm = tfm;
+   desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+   ret = crypto_shash_init(desc);
+   if (ret < 0)
+   goto error_shash;
+
+   ret = crypto_shash_finup(desc, mod, verifylen, digest);
+   if (ret < 0)
+   goto error_shash;
+
+   pr_debug("%ld digest: %*phN\n", verifylen, (int) digest_size, digest);
+
+   ret = is_hash_blacklisted(digest, digest_size, "bin");
+   if (ret == -EKEYREJECTED)
+   pr_err("Module hash %*phN is blacklisted\n",
+  (int) digest_size, digest);
+
+error_shash:
+   kfree(digest);
+error_digest:
+   crypto_free_shash(tfm);
+error_return:
+   return ret;
+}
+
 /*
  * Verify the signature on a module.
  */
 int mod_verify_sig(const void *mod, unsigned long *_modlen)
 {
struct module_signature ms;
-   size_t modlen = *_modlen, sig_len;
+   size_t modlen = *_modlen, sig_len, wholelen;
+   int ret;
 
pr_devel("==>%s(,%zu)\n", __func__, modlen);
 
if (modlen <= sizeof(ms))
return -EBADMSG;
 
+   wholelen = modlen + sizeof(MODULE_SIG_STRING) - 1;
memcpy(, mod + (modlen - sizeof(ms)), sizeof(ms));
modlen -= sizeof(ms);
 
@@ -80,7 +131,14 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
return -EBADMSG;
}
 
-   return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
+   ret = verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
  (void *)1UL, VERIFYING_MODULE_SIGNATURE,
  NULL, NULL);
+   pr_devel("verify_pkcs7_signature() = %d\n", ret);
+
+   /* checking hash of module is in blacklist */
+   if (!ret)
+   ret = mod_is_hash_blacklisted(mod, wholelen);
+
+   return ret;
 }
-- 
2.10.2



[PATCH 4/5] MODSIGN: checking the blacklisted hash before loading a kernel module

2018-03-13 Thread Lee, Chun-Yi
This patch adds the logic for checking the kernel module's hash
base on blacklist. The hash must be generated by sha256 and enrolled
to dbx/mokx.

For example:
sha256sum sample.ko
mokutil --mokx --import-hash $HASH_RESULT

Whether the signature on ko file is stripped or not, the hash can be
compared by kernel.

Cc: David Howells 
Cc: Josh Boyer 
Cc: James Bottomley 
Signed-off-by: "Lee, Chun-Yi" 
---
 kernel/module_signing.c | 62 +++--
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index d3d6f95..d30ac74 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,9 +11,12 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "module-internal.h"
 
 enum pkey_id_type {
@@ -42,19 +45,67 @@ struct module_signature {
__be32  sig_len;/* Length of signature data */
 };
 
+static int mod_is_hash_blacklisted(const void *mod, size_t verifylen)
+{
+   struct crypto_shash *tfm;
+   struct shash_desc *desc;
+   size_t digest_size, desc_size;
+   u8 *digest;
+   int ret = 0;
+
+   tfm = crypto_alloc_shash("sha256", 0, 0);
+   if (IS_ERR(tfm))
+   goto error_return;
+
+   desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
+   digest_size = crypto_shash_digestsize(tfm);
+   digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
+   if (!digest) {
+   pr_err("digest memory buffer allocate fail\n");
+   ret = -ENOMEM;
+   goto error_digest;
+   }
+   desc = (void *)digest + digest_size;
+   desc->tfm = tfm;
+   desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
+   ret = crypto_shash_init(desc);
+   if (ret < 0)
+   goto error_shash;
+
+   ret = crypto_shash_finup(desc, mod, verifylen, digest);
+   if (ret < 0)
+   goto error_shash;
+
+   pr_debug("%ld digest: %*phN\n", verifylen, (int) digest_size, digest);
+
+   ret = is_hash_blacklisted(digest, digest_size, "bin");
+   if (ret == -EKEYREJECTED)
+   pr_err("Module hash %*phN is blacklisted\n",
+  (int) digest_size, digest);
+
+error_shash:
+   kfree(digest);
+error_digest:
+   crypto_free_shash(tfm);
+error_return:
+   return ret;
+}
+
 /*
  * Verify the signature on a module.
  */
 int mod_verify_sig(const void *mod, unsigned long *_modlen)
 {
struct module_signature ms;
-   size_t modlen = *_modlen, sig_len;
+   size_t modlen = *_modlen, sig_len, wholelen;
+   int ret;
 
pr_devel("==>%s(,%zu)\n", __func__, modlen);
 
if (modlen <= sizeof(ms))
return -EBADMSG;
 
+   wholelen = modlen + sizeof(MODULE_SIG_STRING) - 1;
memcpy(, mod + (modlen - sizeof(ms)), sizeof(ms));
modlen -= sizeof(ms);
 
@@ -80,7 +131,14 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
return -EBADMSG;
}
 
-   return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
+   ret = verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
  (void *)1UL, VERIFYING_MODULE_SIGNATURE,
  NULL, NULL);
+   pr_devel("verify_pkcs7_signature() = %d\n", ret);
+
+   /* checking hash of module is in blacklist */
+   if (!ret)
+   ret = mod_is_hash_blacklisted(mod, wholelen);
+
+   return ret;
 }
-- 
2.10.2