Message ID | CACdnJuvW47m3JvEcuEX1bsr+L2Ht9LDn_iCuPbHLOoaohOFW4Q@mail.gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [PULL,REQUEST] Lock down patches | expand |
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
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.
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 >
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/
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.
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
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.
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
Hi James, Based on feedback, I'm going to make a couple of small changes to this patchset and then resend.