mbox series

[PULL,REQUEST] Lock down patches

Message ID CACdnJuvW47m3JvEcuEX1bsr+L2Ht9LDn_iCuPbHLOoaohOFW4Q@mail.gmail.com (mailing list archive)
State New, archived
Headers show
Series [PULL,REQUEST] Lock down patches | expand

Pull-request

https://github.com/mjg59/linux lock_down

Message

Matthew Garrett Feb. 28, 2019, 9:28 p.m. UTC
Hi James,

David is low on cycles at the moment, so I'm taking over for this time
round. This patchset introduces an optional kernel lockdown feature,
intended to strengthen the boundary between UID 0 and the kernel. When
enabled and active (by enabling the config option and passing the
"lockdown" option on the kernel command line), various pieces of
kernel functionality are restricted. Applications that rely on
low-level access to either hardware or the kernel may cease working as
a result - therefore this should not be enabled without appropriate
evaluation beforehand.

The majority of mainstream distributions have been carrying variants
of this patchset for many years now, so there's value in providing a
unified upstream implementation to reduce the delta. This PR probably
doesn't meet every distribution requirement, but gets us much closer
to not requiring external patches.

This PR is mostly the same as the previous attempt, but with the
following changes:

1) The integration between EFI secure boot and the lockdown state has
been removed
2) A new CONFIG_KERNEL_LOCK_DOWN_FORCE kconfig option has been added,
which will always enable lockdown regardless of the kernel command
line
3) The integration with IMA has been dropped for now. Requiring the
use of the IMA secure boot policy when lockdown is enabled isn't
practical for most distributions at the moment, as there's still not a
great deal of infrastructure for shipping packages with appropriate
IMA signatures, and it makes it complicated for end users to manage
custom IMA policies.

The following changes since commit a3b22b9f11d9fbc48b0291ea92259a5a810e9438:

  Linux 5.0-rc7 (2019-02-17 18:46:40 -0800)

are available in the Git repository at:

  https://github.com/mjg59/linux lock_down

for you to fetch changes up to 43e004ecae91bf9159b8e91cd1d613e58b8f63f8:

  lockdown: Print current->comm in restriction messages (2019-02-28
11:19:23 -0800)

----------------------------------------------------------------
Dave Young (1):
      Copy secure_boot flag in boot params across kexec reboot

David Howells (12):
      Add the ability to lock down access to the running kernel image
      Enforce module signatures if the kernel is locked down
      Prohibit PCMCIA CIS storage when the kernel is locked down
      Lock down TIOCSSERIAL
      Lock down module params that specify hardware parameters (eg. ioport)
      x86/mmiotrace: Lock down the testmmiotrace module
      Lock down /proc/kcore
      Lock down kprobes
      bpf: Restrict kernel image access functions when the kernel is locked down
      Lock down perf
      debugfs: Restrict debugfs when the kernel is locked down
      lockdown: Print current->comm in restriction messages

Jiri Bohac (2):
      kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
      kexec_file: Restrict at runtime if the kernel is locked down

Josh Boyer (2):
      hibernate: Disable when the kernel is locked down
      acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down

Kyle McMartin (1):
      Add a SysRq option to lift kernel lockdown

Linn Crosetto (2):
      acpi: Disable ACPI table override if the kernel is locked down
      acpi: Disable APEI error injection if the kernel is locked down

Matthew Garrett (7):
      Restrict /dev/{mem,kmem,port} when the kernel is locked down
      kexec_load: Disable at runtime if the kernel is locked down
      uswsusp: Disable when the kernel is locked down
      PCI: Lock down BAR access when the kernel is locked down
      x86: Lock down IO port access when the kernel is locked down
      x86/msr: Restrict MSR access when the kernel is locked down
      ACPI: Limit access to custom_method when the kernel is locked down

 arch/x86/Kconfig                       |  20 ++++++++++++-----
 arch/x86/include/asm/setup.h           |   2 ++
 arch/x86/kernel/ioport.c               |   6 ++++--
 arch/x86/kernel/kexec-bzimage64.c      |   1 +
 arch/x86/kernel/msr.c                  |  10 +++++++++
 arch/x86/mm/testmmiotrace.c            |   3 +++
 crypto/asymmetric_keys/verify_pefile.c |   4 +++-
 drivers/acpi/apei/einj.c               |   3 +++
 drivers/acpi/custom_method.c           |   3 +++
 drivers/acpi/osl.c                     |   2 +-
 drivers/acpi/tables.c                  |   5 +++++
 drivers/char/mem.c                     |   2 ++
 drivers/input/misc/uinput.c            |   1 +
 drivers/pci/pci-sysfs.c                |   9 ++++++++
 drivers/pci/proc.c                     |   9 +++++++-
 drivers/pci/syscall.c                  |   3 ++-
 drivers/pcmcia/cistpl.c                |   3 +++
 drivers/tty/serial/serial_core.c       |   6 ++++++
 drivers/tty/sysrq.c                    |  19 +++++++++++------
 fs/debugfs/file.c                      |  28 ++++++++++++++++++++++++
 fs/debugfs/inode.c                     |  30 ++++++++++++++++++++++++--
 fs/proc/kcore.c                        |   2 ++
 include/linux/ima.h                    |   6 ++++++
 include/linux/input.h                  |   5 +++++
 include/linux/kernel.h                 |  17 +++++++++++++++
 include/linux/kexec.h                  |   4 ++--
 include/linux/security.h               |   9 +++++++-
 include/linux/sysrq.h                  |   8 ++++++-
 kernel/bpf/syscall.c                   |   3 +++
 kernel/debug/kdb/kdb_main.c            |   2 +-
 kernel/events/core.c                   |   5 +++++
 kernel/kexec.c                         |   7 ++++++
 kernel/kexec_file.c                    |  56
++++++++++++++++++++++++++++++++++++++++++------
 kernel/kprobes.c                       |   3 +++
 kernel/module.c                        |  56
++++++++++++++++++++++++++++++++++++------------
 kernel/params.c                        |  26 ++++++++++++++++++-----
 kernel/power/hibernate.c               |   2 +-
 kernel/power/user.c                    |   3 +++
 security/Kconfig                       |  24 +++++++++++++++++++++
 security/Makefile                      |   3 +++
 security/lock_down.c                   | 106
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 41 files changed, 466 insertions(+), 50 deletions(-)
 create mode 100644 security/lock_down.c

Comments

Mimi Zohar Feb. 28, 2019, 10:20 p.m. UTC | #1
On Thu, 2019-02-28 at 13:28 -0800, Matthew Garrett wrote:
> Hi James,
> 
> David is low on cycles at the moment, so I'm taking over for this time
> round. This patchset introduces an optional kernel lockdown feature,
> intended to strengthen the boundary between UID 0 and the kernel. When
> enabled and active (by enabling the config option and passing the
> "lockdown" option on the kernel command line), various pieces of
> kernel functionality are restricted. Applications that rely on
> low-level access to either hardware or the kernel may cease working as
> a result - therefore this should not be enabled without appropriate
> evaluation beforehand.
> 
> The majority of mainstream distributions have been carrying variants
> of this patchset for many years now, so there's value in providing a
> unified upstream implementation to reduce the delta. This PR probably
> doesn't meet every distribution requirement, but gets us much closer
> to not requiring external patches.
> 
> This PR is mostly the same as the previous attempt, but with the
> following changes:

Where/when was this latest version of the patches posted?

> 
> 1) The integration between EFI secure boot and the lockdown state has
> been removed
> 2) A new CONFIG_KERNEL_LOCK_DOWN_FORCE kconfig option has been added,
> which will always enable lockdown regardless of the kernel command
> line
> 3) The integration with IMA has been dropped for now. Requiring the
> use of the IMA secure boot policy when lockdown is enabled isn't
> practical for most distributions at the moment, as there's still not a
> great deal of infrastructure for shipping packages with appropriate
> IMA signatures, and it makes it complicated for end users to manage
> custom IMA policies.

I'm all in favor of dropping the original attempt to coordinate
between the kexec PE and IMA kernel image signatures and the kernel
appended and IMA modules signatures, but there has been quite a bit of
work recently coordinating the different types of signatures.

Preventing systems which do use IMA for signature verification, should
not limit their ability to enable "lock down".  Does this version of
the "lock down" patches coordinate the different kexec kernel image
and kernel module signature verification methods? 

Mimi
Matthew Garrett Feb. 28, 2019, 11:13 p.m. UTC | #2
On Thu, Feb 28, 2019 at 2:20 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Thu, 2019-02-28 at 13:28 -0800, Matthew Garrett wrote:
> > This PR is mostly the same as the previous attempt, but with the
> > following changes:
>
> Where/when was this latest version of the patches posted?

They should have followed this, but git-send-email choked on some
reviewed-by: lines so I'm just trying to sort that out.

> >
> > 1) The integration between EFI secure boot and the lockdown state has
> > been removed
> > 2) A new CONFIG_KERNEL_LOCK_DOWN_FORCE kconfig option has been added,
> > which will always enable lockdown regardless of the kernel command
> > line
> > 3) The integration with IMA has been dropped for now. Requiring the
> > use of the IMA secure boot policy when lockdown is enabled isn't
> > practical for most distributions at the moment, as there's still not a
> > great deal of infrastructure for shipping packages with appropriate
> > IMA signatures, and it makes it complicated for end users to manage
> > custom IMA policies.
>
> I'm all in favor of dropping the original attempt to coordinate
> between the kexec PE and IMA kernel image signatures and the kernel
> appended and IMA modules signatures, but there has been quite a bit of
> work recently coordinating the different types of signatures.
>
> Preventing systems which do use IMA for signature verification, should
> not limit their ability to enable "lock down".  Does this version of
> the "lock down" patches coordinate the different kexec kernel image
> and kernel module signature verification methods?

It's a little more complicated than this. We can't just rely on IMA
appraisal - it has to be based on digital signatures, and the existing
patch only made that implicit by enabling the secure_boot policy. I
think we do want to integrate these, but there's a few things we need
to take into account:

1) An integrated solution can't depend on xattrs, both because of the
lagging support for distributing those signatures but also because we
need to support filesystems that don't support xattrs
2) An integrated solution can't depend on the current secure_boot
policy because that requires signed IMA policy updates, but
distributions have no way of knowing what IMA policy end users require

In any case, I do agree that we should aim to make this more
reasonable - having orthogonal signing code doesn't benefit anyone.
Once there's solid agreement on that we can extend this support.
Randy Dunlap Feb. 28, 2019, 11:24 p.m. UTC | #3
On 2/28/19 1:28 PM, Matthew Garrett wrote:
> Hi James,
> 
> David is low on cycles at the moment, so I'm taking over for this time
> round. This patchset introduces an optional kernel lockdown feature,
> intended to strengthen the boundary between UID 0 and the kernel. When
> enabled and active (by enabling the config option and passing the
> "lockdown" option on the kernel command line), various pieces of
> kernel functionality are restricted. Applications that rely on
> low-level access to either hardware or the kernel may cease working as
> a result - therefore this should not be enabled without appropriate
> evaluation beforehand.

Documentation/process/submitting-patches.rst says (IMO) that these
patches should also have Signed-of-by: <you>.

"The Signed-off-by: tag indicates that the signer was involved in the
development of the patch, or that he/she was in the patch's delivery path."

Also, the sysrq key usage should be documented in
Documentation/admin-guide/sysrq.rst.

> The majority of mainstream distributions have been carrying variants
> of this patchset for many years now, so there's value in providing a
> unified upstream implementation to reduce the delta. This PR probably
> doesn't meet every distribution requirement, but gets us much closer
> to not requiring external patches.
> 
> This PR is mostly the same as the previous attempt, but with the
> following changes:
> 
> 1) The integration between EFI secure boot and the lockdown state has
> been removed
> 2) A new CONFIG_KERNEL_LOCK_DOWN_FORCE kconfig option has been added,
> which will always enable lockdown regardless of the kernel command
> line
> 3) The integration with IMA has been dropped for now. Requiring the
> use of the IMA secure boot policy when lockdown is enabled isn't
> practical for most distributions at the moment, as there's still not a
> great deal of infrastructure for shipping packages with appropriate
> IMA signatures, and it makes it complicated for end users to manage
> custom IMA policies.
> 
> The following changes since commit a3b22b9f11d9fbc48b0291ea92259a5a810e9438:
> 
>   Linux 5.0-rc7 (2019-02-17 18:46:40 -0800)
> 
> are available in the Git repository at:
> 
>   https://github.com/mjg59/linux lock_down
> 
> for you to fetch changes up to 43e004ecae91bf9159b8e91cd1d613e58b8f63f8:
> 
>   lockdown: Print current->comm in restriction messages (2019-02-28
> 11:19:23 -0800)
> 
> ----------------------------------------------------------------
> Dave Young (1):
>       Copy secure_boot flag in boot params across kexec reboot
> 
> David Howells (12):
>       Add the ability to lock down access to the running kernel image
>       Enforce module signatures if the kernel is locked down
>       Prohibit PCMCIA CIS storage when the kernel is locked down
>       Lock down TIOCSSERIAL
>       Lock down module params that specify hardware parameters (eg. ioport)
>       x86/mmiotrace: Lock down the testmmiotrace module
>       Lock down /proc/kcore
>       Lock down kprobes
>       bpf: Restrict kernel image access functions when the kernel is locked down
>       Lock down perf
>       debugfs: Restrict debugfs when the kernel is locked down
>       lockdown: Print current->comm in restriction messages
> 
> Jiri Bohac (2):
>       kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
>       kexec_file: Restrict at runtime if the kernel is locked down
> 
> Josh Boyer (2):
>       hibernate: Disable when the kernel is locked down
>       acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down
> 
> Kyle McMartin (1):
>       Add a SysRq option to lift kernel lockdown
> 
> Linn Crosetto (2):
>       acpi: Disable ACPI table override if the kernel is locked down
>       acpi: Disable APEI error injection if the kernel is locked down
> 
> Matthew Garrett (7):
>       Restrict /dev/{mem,kmem,port} when the kernel is locked down
>       kexec_load: Disable at runtime if the kernel is locked down
>       uswsusp: Disable when the kernel is locked down
>       PCI: Lock down BAR access when the kernel is locked down
>       x86: Lock down IO port access when the kernel is locked down
>       x86/msr: Restrict MSR access when the kernel is locked down
>       ACPI: Limit access to custom_method when the kernel is locked down
> 
>  arch/x86/Kconfig                       |  20 ++++++++++++-----
>  arch/x86/include/asm/setup.h           |   2 ++
>  arch/x86/kernel/ioport.c               |   6 ++++--
>  arch/x86/kernel/kexec-bzimage64.c      |   1 +
>  arch/x86/kernel/msr.c                  |  10 +++++++++
>  arch/x86/mm/testmmiotrace.c            |   3 +++
>  crypto/asymmetric_keys/verify_pefile.c |   4 +++-
>  drivers/acpi/apei/einj.c               |   3 +++
>  drivers/acpi/custom_method.c           |   3 +++
>  drivers/acpi/osl.c                     |   2 +-
>  drivers/acpi/tables.c                  |   5 +++++
>  drivers/char/mem.c                     |   2 ++
>  drivers/input/misc/uinput.c            |   1 +
>  drivers/pci/pci-sysfs.c                |   9 ++++++++
>  drivers/pci/proc.c                     |   9 +++++++-
>  drivers/pci/syscall.c                  |   3 ++-
>  drivers/pcmcia/cistpl.c                |   3 +++
>  drivers/tty/serial/serial_core.c       |   6 ++++++
>  drivers/tty/sysrq.c                    |  19 +++++++++++------
>  fs/debugfs/file.c                      |  28 ++++++++++++++++++++++++
>  fs/debugfs/inode.c                     |  30 ++++++++++++++++++++++++--
>  fs/proc/kcore.c                        |   2 ++
>  include/linux/ima.h                    |   6 ++++++
>  include/linux/input.h                  |   5 +++++
>  include/linux/kernel.h                 |  17 +++++++++++++++
>  include/linux/kexec.h                  |   4 ++--
>  include/linux/security.h               |   9 +++++++-
>  include/linux/sysrq.h                  |   8 ++++++-
>  kernel/bpf/syscall.c                   |   3 +++
>  kernel/debug/kdb/kdb_main.c            |   2 +-
>  kernel/events/core.c                   |   5 +++++
>  kernel/kexec.c                         |   7 ++++++
>  kernel/kexec_file.c                    |  56
> ++++++++++++++++++++++++++++++++++++++++++------
>  kernel/kprobes.c                       |   3 +++
>  kernel/module.c                        |  56
> ++++++++++++++++++++++++++++++++++++------------
>  kernel/params.c                        |  26 ++++++++++++++++++-----
>  kernel/power/hibernate.c               |   2 +-
>  kernel/power/user.c                    |   3 +++
>  security/Kconfig                       |  24 +++++++++++++++++++++
>  security/Makefile                      |   3 +++
>  security/lock_down.c                   | 106
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  41 files changed, 466 insertions(+), 50 deletions(-)
>  create mode 100644 security/lock_down.c
>
Mimi Zohar March 1, 2019, 12:05 a.m. UTC | #4
On Thu, 2019-02-28 at 15:13 -0800, Matthew Garrett wrote:
> On Thu, Feb 28, 2019 at 2:20 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > On Thu, 2019-02-28 at 13:28 -0800, Matthew Garrett wrote:
> > > This PR is mostly the same as the previous attempt, but with the
> > > following changes:
> >
> > Where/when was this latest version of the patches posted?
> 
> They should have followed this, but git-send-email choked on some
> reviewed-by: lines so I'm just trying to sort that out.

I'm a little perplexed as to why you would send a pull request, before
re-posting the patches with the changes for review.

> 
> > >
> > > 1) The integration between EFI secure boot and the lockdown state has
> > > been removed
> > > 2) A new CONFIG_KERNEL_LOCK_DOWN_FORCE kconfig option has been added,
> > > which will always enable lockdown regardless of the kernel command
> > > line
> > > 3) The integration with IMA has been dropped for now. Requiring the
> > > use of the IMA secure boot policy when lockdown is enabled isn't
> > > practical for most distributions at the moment, as there's still not a
> > > great deal of infrastructure for shipping packages with appropriate
> > > IMA signatures, and it makes it complicated for end users to manage
> > > custom IMA policies.
> >
> > I'm all in favor of dropping the original attempt to coordinate
> > between the kexec PE and IMA kernel image signatures and the kernel
> > appended and IMA modules signatures, but there has been quite a bit of
> > work recently coordinating the different types of signatures.
> >
> > Preventing systems which do use IMA for signature verification, should
> > not limit their ability to enable "lock down".  Does this version of
> > the "lock down" patches coordinate the different kexec kernel image
> > and kernel module signature verification methods?
> 
> It's a little more complicated than this. We can't just rely on IMA
> appraisal - it has to be based on digital signatures, and the existing
> patch only made that implicit by enabling the secure_boot policy.  

Right, which is the reason the IMA architecture specific policy
requires file signatures. [1][2]

> I
> think we do want to integrate these, but there's a few things we need
> to take into account:
> 
> 1) An integrated solution can't depend on xattrs, both because of the
> lagging support for distributing those signatures but also because we
> need to support filesystems that don't support xattrs

That's not a valid reason for preventing systems that do use IMA for
verifying the kexec kernel image signature or kernel module signatures
from enabling "lock down".  This just means that there needs to be
some coordination between the different signature verification
methods. [1][2]

> 2) An integrated solution can't depend on the current secure_boot
> policy because that requires signed IMA policy updates, but
> distributions have no way of knowing what IMA policy end users require

Both the "CONFIG_IMA_APPRAISE_REQUIRE_KEXEC_SIGS" and the IMA
architecture policy rules persist after loading a custom policy.
 Neither of them require loading or signing a custom policy.

> In any case, I do agree that we should aim to make this more
> reasonable - having orthogonal signing code doesn't benefit anyone.
> Once there's solid agreement on that we can extend this support.
> 

Having multiple signature verification methods is going to be around
for a while.  The solution is to coordinate the signature verification
methods, without requiring both types of signatures. [1][2]

[For distros that do enable IMA, enabling the IMA architecture
specific policy enforces the kexec kernel image and kernel modules
signature verification.]

Mimi

[1] Subject: [PATCH v2 0/5] selftests/ima: add kexec and kernel module tests
[2] Patches available from the "next-queued-testing" branch
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/
Matthew Garrett March 1, 2019, 1:01 a.m. UTC | #5
On Thu, Feb 28, 2019 at 4:05 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2019-02-28 at 15:13 -0800, Matthew Garrett wrote:
> > On Thu, Feb 28, 2019 at 2:20 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > Where/when was this latest version of the patches posted?
> >
> > They should have followed this, but git-send-email choked on some
> > reviewed-by: lines so I'm just trying to sort that out.
>
> I'm a little perplexed as to why you would send a pull request, before
> re-posting the patches with the changes for review.

They should be there now. There's no substantive change to the
patches, other than having dropped a few from the series.

> > It's a little more complicated than this. We can't just rely on IMA
> > appraisal - it has to be based on digital signatures, and the existing
> > patch only made that implicit by enabling the secure_boot policy.
>
> Right, which is the reason the IMA architecture specific policy
> requires file signatures. [1][2]

The current patches seem to require ima signatures - shouldn't this
allow ima digests as long as there's an evm signature?

> > I
> > think we do want to integrate these, but there's a few things we need
> > to take into account:
> >
> > 1) An integrated solution can't depend on xattrs, both because of the
> > lagging support for distributing those signatures but also because we
> > need to support filesystems that don't support xattrs
>
> That's not a valid reason for preventing systems that do use IMA for
> verifying the kexec kernel image signature or kernel module signatures
> from enabling "lock down".  This just means that there needs to be
> some coordination between the different signature verification
> methods. [1][2]

I agree, but the current form of the integration makes it impossible
for anyone using an IMA-enabled kernel (but not using IMA) to do
anything unless they have IMA signatures. It's a problem we need to
solve, I just don't think it's a problem we need to solve before
merging the patchset.

> > 2) An integrated solution can't depend on the current secure_boot
> > policy because that requires signed IMA policy updates, but
> > distributions have no way of knowing what IMA policy end users require
>
> Both the "CONFIG_IMA_APPRAISE_REQUIRE_KEXEC_SIGS" and the IMA
> architecture policy rules persist after loading a custom policy.
>  Neither of them require loading or signing a custom policy.

The previous version of the lockdown patchset sets the secure_boot
policy when lockdown is enabled, which does require that any custom
policy be signed.

> > In any case, I do agree that we should aim to make this more
> > reasonable - having orthogonal signing code doesn't benefit anyone.
> > Once there's solid agreement on that we can extend this support.
> >
>
> Having multiple signature verification methods is going to be around
> for a while.  The solution is to coordinate the signature verification
> methods, without requiring both types of signatures. [1][2]

Agree, and once we have a solution to this we should integrate that
with lockdown. I don't think merging this first makes that any harder.
Importantly, this version of the patchset doesn't enable lockdown
automatically unless explicitly configured to do so, which means you
can build a lockdown kernel without interfering with IMA.
Mimi Zohar March 1, 2019, 1:44 a.m. UTC | #6
On Thu, 2019-02-28 at 17:01 -0800, Matthew Garrett wrote:

> > That's not a valid reason for preventing systems that do use IMA for
> > verifying the kexec kernel image signature or kernel module signatures
> > from enabling "lock down".  This just means that there needs to be
> > some coordination between the different signature verification
> > methods. [1][2]
> 
> I agree, but the current form of the integration makes it impossible
> for anyone using an IMA-enabled kernel (but not using IMA) to do
> anything unless they have IMA signatures. It's a problem we need to
> solve, I just don't think it's a problem we need to solve before
> merging the patchset.

That's simply not true.  Have you even looked at the IMA architecture
patches?

fcf338449af5 x86/ima: require signed kernel modules
d958083a8f64 x86/ima: define arch_get_ima_policy() for x86

Mimi
Matthew Garrett March 1, 2019, 3:33 a.m. UTC | #7
On Thu, Feb 28, 2019 at 5:45 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2019-02-28 at 17:01 -0800, Matthew Garrett wrote:
>
> > > That's not a valid reason for preventing systems that do use IMA for
> > > verifying the kexec kernel image signature or kernel module signatures
> > > from enabling "lock down".  This just means that there needs to be
> > > some coordination between the different signature verification
> > > methods. [1][2]
> >
> > I agree, but the current form of the integration makes it impossible
> > for anyone using an IMA-enabled kernel (but not using IMA) to do
> > anything unless they have IMA signatures. It's a problem we need to
> > solve, I just don't think it's a problem we need to solve before
> > merging the patchset.
>
> That's simply not true.  Have you even looked at the IMA architecture
> patches?

Sorry, I think we're talking at cross purposes - I was referring to
your patch "ima: require secure_boot rules in lockdown mode"
(https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=efi-lock-down&id=7fa3734bd31a4b3fe71358fcba8d4878e5005b7f).
If the goal is just to use the architecture rules then I don't see any
conflict, and as far as I can tell things would just work as is if I
drop the ima portion from "kexec_file: Restrict at runtime if the
kernel is locked down"? Apologies, I'd thought that the secure_boot
ruleset was still intended to be used in a lockdown environment.
Mimi Zohar March 1, 2019, 4:16 a.m. UTC | #8
On Thu, 2019-02-28 at 19:33 -0800, Matthew Garrett wrote:
> On Thu, Feb 28, 2019 at 5:45 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2019-02-28 at 17:01 -0800, Matthew Garrett wrote:
> >
> > > > That's not a valid reason for preventing systems that do use IMA for
> > > > verifying the kexec kernel image signature or kernel module signatures
> > > > from enabling "lock down".  This just means that there needs to be
> > > > some coordination between the different signature verification
> > > > methods. [1][2]
> > >
> > > I agree, but the current form of the integration makes it impossible
> > > for anyone using an IMA-enabled kernel (but not using IMA) to do
> > > anything unless they have IMA signatures. It's a problem we need to
> > > solve, I just don't think it's a problem we need to solve before
> > > merging the patchset.
> >
> > That's simply not true.  Have you even looked at the IMA architecture
> > patches?
> 
> Sorry, I think we're talking at cross purposes - I was referring to
> your patch "ima: require secure_boot rules in lockdown mode"
> (https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=efi-lock-down&id=7fa3734bd31a4b3fe71358fcba8d4878e5005b7f).

With the "secure_boot" rules it was difficult to coordinate the
different signature verification methods.  Plus they weren't
persistent after loading a custom policy.

> If the goal is just to use the architecture rules then I don't see any
> conflict, 

yes

> and as far as I can tell things would just work as is if I
> drop the ima portion from "kexec_file: Restrict at runtime if the
> kernel is locked down"?

That code is a remnant left over from when the "secure_boot" policy
was enabled.  However, dropping the IMA portion there would result in
allowing only PE signed kernel images.  (On Power, for example, there
aren't any PE signatures.)

My suggestion would be to drop this patch and require the architecture
specific policy in "lock down" mode.

>  Apologies, I'd thought that the secure_boot
> ruleset was still intended to be used in a lockdown environment.

No, not any longer.

Mimi
Matthew Garrett March 4, 2019, 10:10 p.m. UTC | #9
Hi James,

Based on feedback, I'm going to make a couple of small changes to this
patchset and then resend.