| Message ID | 20201102223800.12181-3-ardb@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | wire up IMA secure boot for arm64 | expand |
Hi Ard, On Mon, Nov 02, 2020 at 11:37:59PM +0100, Ard Biesheuvel wrote: > From: Chester Lin <clin@suse.com> > > Move the x86 IMA arch code into security/integrity/ima/ima_efi.c, > so that we will be able to wire it up for arm64 in a future patch. > > Co-developed-by: Chester Lin <clin@suse.com> > Signed-off-by: Chester Lin <clin@suse.com> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > arch/x86/include/asm/efi.h | 3 ++ > arch/x86/kernel/Makefile | 2 - > security/integrity/ima/Makefile | 4 ++ > arch/x86/kernel/ima_arch.c => security/integrity/ima/ima_efi.c | 45 ++++++-------------- > 4 files changed, 19 insertions(+), 35 deletions(-) > > diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h > index 7673dc833232..c98f78330b09 100644 > --- a/arch/x86/include/asm/efi.h > +++ b/arch/x86/include/asm/efi.h > @@ -380,4 +380,7 @@ static inline void efi_fake_memmap_early(void) > } > #endif > > +#define arch_ima_efi_boot_mode \ > + ({ extern struct boot_params boot_params; boot_params.secure_boot; }) > + > #endif /* _ASM_X86_EFI_H */ > diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile > index 68608bd892c0..5eeb808eb024 100644 > --- a/arch/x86/kernel/Makefile > +++ b/arch/x86/kernel/Makefile > @@ -161,5 +161,3 @@ ifeq ($(CONFIG_X86_64),y) > obj-$(CONFIG_MMCONF_FAM10H) += mmconf-fam10h_64.o > obj-y += vsmp_64.o > endif > - > -obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o > diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile > index 67dabca670e2..2499f2485c04 100644 > --- a/security/integrity/ima/Makefile > +++ b/security/integrity/ima/Makefile > @@ -14,3 +14,7 @@ ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o > ima-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o > ima-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o > ima-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.o > + > +ifeq ($(CONFIG_EFI),y) > +ima-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_efi.o > +endif > diff --git a/arch/x86/kernel/ima_arch.c b/security/integrity/ima/ima_efi.c > similarity index 60% > rename from arch/x86/kernel/ima_arch.c > rename to security/integrity/ima/ima_efi.c > index 7dfb1e808928..233627a9d4b8 100644 > --- a/arch/x86/kernel/ima_arch.c > +++ b/security/integrity/ima/ima_efi.c > @@ -5,50 +5,29 @@ > #include <linux/efi.h> > #include <linux/module.h> > #include <linux/ima.h> > +#include <asm/efi.h> > > -extern struct boot_params boot_params; > +#ifndef arch_ima_efi_boot_mode > +#define arch_ima_efi_boot_mode efi_secureboot_mode_unknown I think this should be "efi_secureboot_mode_unset" otherwise the get_sb_mode() will never be called. The others look good to me, thanks for your help. Regards, Chester > +#endif > > static enum efi_secureboot_mode get_sb_mode(void) > { > - efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID; > - efi_status_t status; > - unsigned long size; > - u8 secboot, setupmode; > - > - size = sizeof(secboot); > + enum efi_secureboot_mode mode; > > if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) { > pr_info("ima: secureboot mode unknown, no efi\n"); > return efi_secureboot_mode_unknown; > } > > - /* Get variable contents into buffer */ > - status = efi.get_variable(L"SecureBoot", &efi_variable_guid, > - NULL, &size, &secboot); > - if (status == EFI_NOT_FOUND) { > + mode = efi_get_secureboot_mode(efi.get_variable); > + if (mode == efi_secureboot_mode_disabled) > pr_info("ima: secureboot mode disabled\n"); > - return efi_secureboot_mode_disabled; > - } > - > - if (status != EFI_SUCCESS) { > + else if (mode == efi_secureboot_mode_unknown) > pr_info("ima: secureboot mode unknown\n"); > - return efi_secureboot_mode_unknown; > - } > - > - size = sizeof(setupmode); > - status = efi.get_variable(L"SetupMode", &efi_variable_guid, > - NULL, &size, &setupmode); > - > - if (status != EFI_SUCCESS) /* ignore unknown SetupMode */ > - setupmode = 0; > - > - if (secboot == 0 || setupmode == 1) { > - pr_info("ima: secureboot mode disabled\n"); > - return efi_secureboot_mode_disabled; > - } > - > - pr_info("ima: secureboot mode enabled\n"); > - return efi_secureboot_mode_enabled; > + else > + pr_info("ima: secureboot mode enabled\n"); > + return mode; > } > > bool arch_ima_get_secureboot(void) > @@ -57,7 +36,7 @@ bool arch_ima_get_secureboot(void) > static bool initialized; > > if (!initialized && efi_enabled(EFI_BOOT)) { > - sb_mode = boot_params.secure_boot; > + sb_mode = arch_ima_efi_boot_mode; > > if (sb_mode == efi_secureboot_mode_unset) > sb_mode = get_sb_mode(); > -- > 2.17.1 >
On Fri, 6 Nov 2020 at 04:41, Chester Lin <clin@suse.com> wrote: > > Hi Ard, > > On Mon, Nov 02, 2020 at 11:37:59PM +0100, Ard Biesheuvel wrote: > > From: Chester Lin <clin@suse.com> > > > > Move the x86 IMA arch code into security/integrity/ima/ima_efi.c, > > so that we will be able to wire it up for arm64 in a future patch. > > > > Co-developed-by: Chester Lin <clin@suse.com> > > Signed-off-by: Chester Lin <clin@suse.com> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > arch/x86/include/asm/efi.h | 3 ++ > > arch/x86/kernel/Makefile | 2 - > > security/integrity/ima/Makefile | 4 ++ > > arch/x86/kernel/ima_arch.c => security/integrity/ima/ima_efi.c | 45 ++++++-------------- > > 4 files changed, 19 insertions(+), 35 deletions(-) > > > > diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h > > index 7673dc833232..c98f78330b09 100644 > > --- a/arch/x86/include/asm/efi.h > > +++ b/arch/x86/include/asm/efi.h > > @@ -380,4 +380,7 @@ static inline void efi_fake_memmap_early(void) > > } > > #endif > > > > +#define arch_ima_efi_boot_mode \ > > + ({ extern struct boot_params boot_params; boot_params.secure_boot; }) > > + > > #endif /* _ASM_X86_EFI_H */ > > diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile > > index 68608bd892c0..5eeb808eb024 100644 > > --- a/arch/x86/kernel/Makefile > > +++ b/arch/x86/kernel/Makefile > > @@ -161,5 +161,3 @@ ifeq ($(CONFIG_X86_64),y) > > obj-$(CONFIG_MMCONF_FAM10H) += mmconf-fam10h_64.o > > obj-y += vsmp_64.o > > endif > > - > > -obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o > > diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile > > index 67dabca670e2..2499f2485c04 100644 > > --- a/security/integrity/ima/Makefile > > +++ b/security/integrity/ima/Makefile > > @@ -14,3 +14,7 @@ ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o > > ima-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o > > ima-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o > > ima-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.o > > + > > +ifeq ($(CONFIG_EFI),y) > > +ima-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_efi.o > > +endif > > diff --git a/arch/x86/kernel/ima_arch.c b/security/integrity/ima/ima_efi.c > > similarity index 60% > > rename from arch/x86/kernel/ima_arch.c > > rename to security/integrity/ima/ima_efi.c > > index 7dfb1e808928..233627a9d4b8 100644 > > --- a/arch/x86/kernel/ima_arch.c > > +++ b/security/integrity/ima/ima_efi.c > > @@ -5,50 +5,29 @@ > > #include <linux/efi.h> > > #include <linux/module.h> > > #include <linux/ima.h> > > +#include <asm/efi.h> > > > > -extern struct boot_params boot_params; > > +#ifndef arch_ima_efi_boot_mode > > +#define arch_ima_efi_boot_mode efi_secureboot_mode_unknown > > I think this should be "efi_secureboot_mode_unset" otherwise the get_sb_mode() > will never be called. The others look good to me, thanks for your help. > Thanks Chester! I will fix that up. > > +#endif > > > > static enum efi_secureboot_mode get_sb_mode(void) > > { > > - efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID; > > - efi_status_t status; > > - unsigned long size; > > - u8 secboot, setupmode; > > - > > - size = sizeof(secboot); > > + enum efi_secureboot_mode mode; > > > > if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) { > > pr_info("ima: secureboot mode unknown, no efi\n"); > > return efi_secureboot_mode_unknown; > > } > > > > - /* Get variable contents into buffer */ > > - status = efi.get_variable(L"SecureBoot", &efi_variable_guid, > > - NULL, &size, &secboot); > > - if (status == EFI_NOT_FOUND) { > > + mode = efi_get_secureboot_mode(efi.get_variable); > > + if (mode == efi_secureboot_mode_disabled) > > pr_info("ima: secureboot mode disabled\n"); > > - return efi_secureboot_mode_disabled; > > - } > > - > > - if (status != EFI_SUCCESS) { > > + else if (mode == efi_secureboot_mode_unknown) > > pr_info("ima: secureboot mode unknown\n"); > > - return efi_secureboot_mode_unknown; > > - } > > - > > - size = sizeof(setupmode); > > - status = efi.get_variable(L"SetupMode", &efi_variable_guid, > > - NULL, &size, &setupmode); > > - > > - if (status != EFI_SUCCESS) /* ignore unknown SetupMode */ > > - setupmode = 0; > > - > > - if (secboot == 0 || setupmode == 1) { > > - pr_info("ima: secureboot mode disabled\n"); > > - return efi_secureboot_mode_disabled; > > - } > > - > > - pr_info("ima: secureboot mode enabled\n"); > > - return efi_secureboot_mode_enabled; > > + else > > + pr_info("ima: secureboot mode enabled\n"); > > + return mode; > > } > > > > bool arch_ima_get_secureboot(void) > > @@ -57,7 +36,7 @@ bool arch_ima_get_secureboot(void) > > static bool initialized; > > > > if (!initialized && efi_enabled(EFI_BOOT)) { > > - sb_mode = boot_params.secure_boot; > > + sb_mode = arch_ima_efi_boot_mode; > > > > if (sb_mode == efi_secureboot_mode_unset) > > sb_mode = get_sb_mode(); > > -- > > 2.17.1 > > >
diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h index 7673dc833232..c98f78330b09 100644 --- a/arch/x86/include/asm/efi.h +++ b/arch/x86/include/asm/efi.h @@ -380,4 +380,7 @@ static inline void efi_fake_memmap_early(void) } #endif +#define arch_ima_efi_boot_mode \ + ({ extern struct boot_params boot_params; boot_params.secure_boot; }) + #endif /* _ASM_X86_EFI_H */ diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index 68608bd892c0..5eeb808eb024 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -161,5 +161,3 @@ ifeq ($(CONFIG_X86_64),y) obj-$(CONFIG_MMCONF_FAM10H) += mmconf-fam10h_64.o obj-y += vsmp_64.o endif - -obj-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_arch.o diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile index 67dabca670e2..2499f2485c04 100644 --- a/security/integrity/ima/Makefile +++ b/security/integrity/ima/Makefile @@ -14,3 +14,7 @@ ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o ima-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o ima-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o ima-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.o + +ifeq ($(CONFIG_EFI),y) +ima-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_efi.o +endif diff --git a/arch/x86/kernel/ima_arch.c b/security/integrity/ima/ima_efi.c similarity index 60% rename from arch/x86/kernel/ima_arch.c rename to security/integrity/ima/ima_efi.c index 7dfb1e808928..233627a9d4b8 100644 --- a/arch/x86/kernel/ima_arch.c +++ b/security/integrity/ima/ima_efi.c @@ -5,50 +5,29 @@ #include <linux/efi.h> #include <linux/module.h> #include <linux/ima.h> +#include <asm/efi.h> -extern struct boot_params boot_params; +#ifndef arch_ima_efi_boot_mode +#define arch_ima_efi_boot_mode efi_secureboot_mode_unknown +#endif static enum efi_secureboot_mode get_sb_mode(void) { - efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID; - efi_status_t status; - unsigned long size; - u8 secboot, setupmode; - - size = sizeof(secboot); + enum efi_secureboot_mode mode; if (!efi_rt_services_supported(EFI_RT_SUPPORTED_GET_VARIABLE)) { pr_info("ima: secureboot mode unknown, no efi\n"); return efi_secureboot_mode_unknown; } - /* Get variable contents into buffer */ - status = efi.get_variable(L"SecureBoot", &efi_variable_guid, - NULL, &size, &secboot); - if (status == EFI_NOT_FOUND) { + mode = efi_get_secureboot_mode(efi.get_variable); + if (mode == efi_secureboot_mode_disabled) pr_info("ima: secureboot mode disabled\n"); - return efi_secureboot_mode_disabled; - } - - if (status != EFI_SUCCESS) { + else if (mode == efi_secureboot_mode_unknown) pr_info("ima: secureboot mode unknown\n"); - return efi_secureboot_mode_unknown; - } - - size = sizeof(setupmode); - status = efi.get_variable(L"SetupMode", &efi_variable_guid, - NULL, &size, &setupmode); - - if (status != EFI_SUCCESS) /* ignore unknown SetupMode */ - setupmode = 0; - - if (secboot == 0 || setupmode == 1) { - pr_info("ima: secureboot mode disabled\n"); - return efi_secureboot_mode_disabled; - } - - pr_info("ima: secureboot mode enabled\n"); - return efi_secureboot_mode_enabled; + else + pr_info("ima: secureboot mode enabled\n"); + return mode; } bool arch_ima_get_secureboot(void) @@ -57,7 +36,7 @@ bool arch_ima_get_secureboot(void) static bool initialized; if (!initialized && efi_enabled(EFI_BOOT)) { - sb_mode = boot_params.secure_boot; + sb_mode = arch_ima_efi_boot_mode; if (sb_mode == efi_secureboot_mode_unset) sb_mode = get_sb_mode();