| Message ID | 20180725233200.761-4-erichte@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for architecture-specific IMA policies | expand |
On Wed, Jul 25, 2018 at 06:31:59PM -0500, Eric Richter wrote: > IMA can verify the signature of kernel images loaded with kexec_file_load, > but can not verify images loaded with the regular kexec_load syscall. > Therefore, the appraisal will automatically fail during kexec_load when an > appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used > to effectively disable the kexec_load syscall, while still allowing the > kexec_file_load to operate so long as the target kernel image is signed. > > However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is > enabled and there is an appraise rule set, then the target kernel would > have to be verifiable by both IMA and the architecture specific kernel > verification procedure. > > This patch adds a new func= for IMA appraisal specifically for the original > kexec_load syscall. Therefore, the kexec_load syscall can be effectively > disabled via IMA policy, leaving the kexec_file_load syscall able to do its > own signature verification, and not require it to be signed via IMA. To > retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is > unchanged, and thus enables appraisal for both kexec syscalls. This seems like a roundabout way to disallow the kexec_load syscall. Wouldn't it make more sense to simply disallow kexec_load any time CONFIG_KEXEC_VERIFY_SIG is enabled, since it effectively renders that option impotent? Or has that idea already been rejected? Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2018-08-03 at 08:11 -0500, Seth Forshee wrote: > On Wed, Jul 25, 2018 at 06:31:59PM -0500, Eric Richter wrote: > > IMA can verify the signature of kernel images loaded with kexec_file_load, > > but can not verify images loaded with the regular kexec_load syscall. > > Therefore, the appraisal will automatically fail during kexec_load when an > > appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used > > to effectively disable the kexec_load syscall, while still allowing the > > kexec_file_load to operate so long as the target kernel image is signed. > > > > However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is > > enabled and there is an appraise rule set, then the target kernel would > > have to be verifiable by both IMA and the architecture specific kernel > > verification procedure. > > > > This patch adds a new func= for IMA appraisal specifically for the original > > kexec_load syscall. Therefore, the kexec_load syscall can be effectively > > disabled via IMA policy, leaving the kexec_file_load syscall able to do its > > own signature verification, and not require it to be signed via IMA. To > > retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is > > unchanged, and thus enables appraisal for both kexec syscalls. > > This seems like a roundabout way to disallow the kexec_load syscall. > Wouldn't it make more sense to simply disallow kexec_load any time > CONFIG_KEXEC_VERIFY_SIG is enabled, since it effectively renders that > option impotent? Or has that idea already been rejected? Agreed! We can modify the "case LOADING_KEXEC_IMAGE" in ima_load_data() to prevent the kexec_load based on CONFIG_KEXEC_VERIFY_SIG. The architecture specific policy would only include the IMA appraise rule if CONFIG_KEXEC_VERIFY_SIG was not defined. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 03, 2018 at 10:54:59AM -0400, Mimi Zohar wrote: > On Fri, 2018-08-03 at 08:11 -0500, Seth Forshee wrote: > > On Wed, Jul 25, 2018 at 06:31:59PM -0500, Eric Richter wrote: > > > IMA can verify the signature of kernel images loaded with kexec_file_load, > > > but can not verify images loaded with the regular kexec_load syscall. > > > Therefore, the appraisal will automatically fail during kexec_load when an > > > appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used > > > to effectively disable the kexec_load syscall, while still allowing the > > > kexec_file_load to operate so long as the target kernel image is signed. > > > > > > However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is > > > enabled and there is an appraise rule set, then the target kernel would > > > have to be verifiable by both IMA and the architecture specific kernel > > > verification procedure. > > > > > > This patch adds a new func= for IMA appraisal specifically for the original > > > kexec_load syscall. Therefore, the kexec_load syscall can be effectively > > > disabled via IMA policy, leaving the kexec_file_load syscall able to do its > > > own signature verification, and not require it to be signed via IMA. To > > > retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is > > > unchanged, and thus enables appraisal for both kexec syscalls. > > > > This seems like a roundabout way to disallow the kexec_load syscall. > > Wouldn't it make more sense to simply disallow kexec_load any time > > CONFIG_KEXEC_VERIFY_SIG is enabled, since it effectively renders that > > option impotent? Or has that idea already been rejected? > > Agreed! We can modify the "case LOADING_KEXEC_IMAGE" in > ima_load_data() to prevent the kexec_load based on > CONFIG_KEXEC_VERIFY_SIG. > > The architecture specific policy would only include the IMA appraise > rule if CONFIG_KEXEC_VERIFY_SIG was not defined. After looking at this some more I'm having second thoughts about my suggestion. As a distro we produce a kernel that needs to be flexible enough for a variety of scenarios, and if we completely close off the ability to load an unsigned kernel for kexec that's almost certainly going to end up breaking some use cases. So I think it is necessary to make this a run-time decision rather than a compile-time decision. The patch as provided does this based on whether or not the kernel was booted under secure boot. That might be reasonable, though I still find this mechanism kind of awkward. It seems like ideally there would instead be some logic that would accept the image if the KEXEC_VERIFY_SIG verification had passed, and otherwise require IMA signature verification. Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2018-08-03 at 11:16 -0500, Seth Forshee wrote: > On Fri, Aug 03, 2018 at 10:54:59AM -0400, Mimi Zohar wrote: > > On Fri, 2018-08-03 at 08:11 -0500, Seth Forshee wrote: > > > On Wed, Jul 25, 2018 at 06:31:59PM -0500, Eric Richter wrote: > > > > IMA can verify the signature of kernel images loaded with kexec_file_load, > > > > but can not verify images loaded with the regular kexec_load syscall. > > > > Therefore, the appraisal will automatically fail during kexec_load when an > > > > appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used > > > > to effectively disable the kexec_load syscall, while still allowing the > > > > kexec_file_load to operate so long as the target kernel image is signed. > > > > > > > > However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is > > > > enabled and there is an appraise rule set, then the target kernel would > > > > have to be verifiable by both IMA and the architecture specific kernel > > > > verification procedure. > > > > > > > > This patch adds a new func= for IMA appraisal specifically for the original > > > > kexec_load syscall. Therefore, the kexec_load syscall can be effectively > > > > disabled via IMA policy, leaving the kexec_file_load syscall able to do its > > > > own signature verification, and not require it to be signed via IMA. To > > > > retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is > > > > unchanged, and thus enables appraisal for both kexec syscalls. > > > > > > This seems like a roundabout way to disallow the kexec_load syscall. > > > Wouldn't it make more sense to simply disallow kexec_load any time > > > CONFIG_KEXEC_VERIFY_SIG is enabled, since it effectively renders that > > > option impotent? Or has that idea already been rejected? > > > > Agreed! We can modify the "case LOADING_KEXEC_IMAGE" in > > ima_load_data() to prevent the kexec_load based on > > CONFIG_KEXEC_VERIFY_SIG. > > > > The architecture specific policy would only include the IMA appraise > > rule if CONFIG_KEXEC_VERIFY_SIG was not defined. > > After looking at this some more I'm having second thoughts about my > suggestion. As a distro we produce a kernel that needs to be flexible > enough for a variety of scenarios, and if we completely close off the > ability to load an unsigned kernel for kexec that's almost certainly > going to end up breaking some use cases. > > So I think it is necessary to make this a run-time decision rather than > a compile-time decision. The patch as provided does this based on > whether or not the kernel was booted under secure boot. That might be > reasonable, though I still find this mechanism kind of awkward. Right, the above change is almost right. Instead of preventing the kexec_load syscall based on CONFIG_KEXEC_VERIFY_SIG it should be based on a runtime secure boot flag. Only if there is an arch specific secure boot function and the secure boot flag is enabled, would the kexec_load be disabled. Without an architecture specific secure boot function, the existing IMA code would fail the kexec_load syscall. > It seems > like ideally there would instead be some logic that would accept the > image if the KEXEC_VERIFY_SIG verification had passed, and otherwise > require IMA signature verification. True, but for now to coordinate between the two signature verification methods, only if CONFIG_KEXEC_VERIFY_SIG is not enabled would an IMA architecture specific rule be defined. Mimi -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 74c6702de74..031417779ec 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -29,6 +29,7 @@ Description: base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] [FIRMWARE_CHECK] [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK] + [KEXEC_ORIG_KERNEL_CHECK] mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND] [[^]MAY_EXEC] fsmagic:= hex value diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 6e5fa7c4280..c76e53c982b 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -181,6 +181,7 @@ static inline unsigned long ima_hash_key(u8 *digest) hook(MODULE_CHECK) \ hook(FIRMWARE_CHECK) \ hook(KEXEC_KERNEL_CHECK) \ + hook(KEXEC_ORIG_KERNEL_CHECK) \ hook(KEXEC_INITRAMFS_CHECK) \ hook(POLICY_CHECK) \ hook(MAX_CHECK) @@ -233,6 +234,7 @@ int ima_policy_show(struct seq_file *m, void *v); #define IMA_APPRAISE_FIRMWARE 0x10 #define IMA_APPRAISE_POLICY 0x20 #define IMA_APPRAISE_KEXEC 0x40 +#define IMA_APPRAISE_ORIG_KEXEC 0x80 #ifdef CONFIG_IMA_APPRAISE int ima_appraise_measurement(enum ima_hooks func, diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index dce0a8a217b..a7b4220043d 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -512,7 +512,8 @@ int ima_load_data(enum kernel_load_data_id id) switch (id) { case LOADING_KEXEC_IMAGE: - if (ima_appraise & IMA_APPRAISE_KEXEC) { + if (ima_appraise & + (IMA_APPRAISE_ORIG_KEXEC | IMA_APPRAISE_KEXEC)) { pr_err("impossible to appraise a kernel image without a file descriptor; try using kexec_file_load syscall.\n"); return -EACCES; /* INTEGRITY_UNKNOWN */ } diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 402e5bd1093..7a33e3f6eca 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -475,6 +475,8 @@ static int ima_appraise_flag(enum ima_hooks func) return IMA_APPRAISE_POLICY; else if (func == KEXEC_KERNEL_CHECK) return IMA_APPRAISE_KEXEC; + else if (func == KEXEC_ORIG_KERNEL_CHECK) + return IMA_APPRAISE_ORIG_KEXEC; return 0; } @@ -879,6 +881,9 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) else if (strcmp(args[0].from, "KEXEC_KERNEL_CHECK") == 0) entry->func = KEXEC_KERNEL_CHECK; + else if (strcmp(args[0].from, + "KEXEC_ORIG_KERNEL_CHECK") == 0) + entry->func = KEXEC_ORIG_KERNEL_CHECK; else if (strcmp(args[0].from, "KEXEC_INITRAMFS_CHECK") == 0) entry->func = KEXEC_INITRAMFS_CHECK;
IMA can verify the signature of kernel images loaded with kexec_file_load, but can not verify images loaded with the regular kexec_load syscall. Therefore, the appraisal will automatically fail during kexec_load when an appraise policy rule is set for func=KEXEC_KERNEL_CHECK. This can be used to effectively disable the kexec_load syscall, while still allowing the kexec_file_load to operate so long as the target kernel image is signed. However, this conflicts with CONFIG_KEXEC_VERIFY_SIG. If that option is enabled and there is an appraise rule set, then the target kernel would have to be verifiable by both IMA and the architecture specific kernel verification procedure. This patch adds a new func= for IMA appraisal specifically for the original kexec_load syscall. Therefore, the kexec_load syscall can be effectively disabled via IMA policy, leaving the kexec_file_load syscall able to do its own signature verification, and not require it to be signed via IMA. To retain compatibility, the existing func=KEXEC_KERNEL_CHECK flag is unchanged, and thus enables appraisal for both kexec syscalls. Signed-off-by: Eric Richter <erichte@linux.vnet.ibm.com> --- Documentation/ABI/testing/ima_policy | 1 + security/integrity/ima/ima.h | 2 ++ security/integrity/ima/ima_main.c | 3 ++- security/integrity/ima/ima_policy.c | 5 +++++ 4 files changed, 10 insertions(+), 1 deletion(-)