Re: [PATCH 08a/30] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE

[Jiri Bohac]

On Tue, Jan 16, 2018 at 04:31:51PM +0000, David Howells wrote:
> I think that your code isn't quite right.  Looking at the patched code:
> 
>     #ifdef CONFIG_KEXEC_SIG
> 	sig_err = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
> 					   image->kernel_buf_len);
> 	if (sig_err)
> 		pr_debug("kernel signature verification failed.\n");
> 	else
> 		pr_debug("kernel signature verification successful.\n");
>     #endif
> 
> 	if (sig_err && IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
> 		ret = sig_err;
> 		goto out;
> 	}
> 
> If the signature check fails because the signature is bad, but
> CONFIG_KEXEC_SIG_FORCE=n then it now won't fail when it should.
> 
> If sig_err is -EKEYREJECTED, -EKEYEXPIRED or -EKEYREVOKED then it must fail,
> even if the signature check isn't forced.

It wasn't my intention to fail in these cases. What additional
security does this bring? If simply stripping an invalid
signature from the image before loading will make it pass, why
should the image with an invalid signature be rejected?

Indeed, the module signing code, the semantics of which I wanted
to mimic, also won't load modules with invalid signatures. It
will load modules without any signatuire, it will load  modules
with the MODULE_SIG_STRING modified and it will load modules with
either of MODULE_INIT_IGNORE_MODVERSIONS or
MODULE_INIT_IGNORE_VERMAGIC passed as flags to the finit_module
syscall. In all these cases, it will taint the kernel, which
might be something we want for kexec_file_load as well (?).

But I don't see why it distinguishes between ENOKEY and other
errors when it's so easy for the caller to strip the invalid
signature. And why kexec_file_load should do the same.

What am I missing?

Thanks,

-- 
Jiri Bohac <jbohac@xxxxxxx>
SUSE Labs, Prague, Czechia