diff mbox series

lockdown: s390: kexec_file: don't skip signature verification when not secure IPLed

Message ID 20221121072715.836323-1-coxu@redhat.com
State New
Headers
Series lockdown: s390: kexec_file: don't skip signature verification when not secure IPLed |

Commit Message

Coiby Xu Nov. 21, 2022, 7:27 a.m. UTC 
  Currently for s390, lockdown doesn't prevent unsigned kernel image from
being kexec'ed when secure IPL is disabled. Fix it by always verifying
the signature regardless secure IPL is enabled or not.

Fixes: 155bdd30af17 ("kexec_file: Restrict at runtime if the kernel is locked down")

Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Cc: Jiri Bohac <jbohac@suse.cz>
Cc: David Howells <dhowells@redhat.com>
Cc: Philipp Rudo <prudo@redhat.com>
Cc: kexec@lists.infradead.org
Cc: linux-s390@vger.kernel.org
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 arch/s390/kernel/machine_kexec_file.c | 4 ----
 1 file changed, 4 deletions(-)

Comments

Vasily Gorbik Nov. 22, 2022, 3:15 p.m. UTC | #1 
On Mon, Nov 21, 2022 at 03:27:15PM +0800, Coiby Xu wrote:
> --- a/arch/s390/kernel/machine_kexec_file.c
> +++ b/arch/s390/kernel/machine_kexec_file.c
> @@ -33,10 +33,6 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
>  	unsigned long sig_len;
>  	int ret;
>  
> -	/* Skip signature verification when not secure IPLed. */
> -	if (!ipl_secure_flag)
> -		return 0;

Looking at s390_verify_sig() especially before commit 0828c4a39be5
("kexec, KEYS, s390: Make use of built-in and secondary keyring for
signature verification") I think this condition actually expresses
2 things:
1. the firmware is secure IPL capable and secure IPL keys are
   provided and present in platform keyring.
2. secure IPL is enabled.

Wouldn't this change have implications for machines with older firmware
which doesn't support secure IPL? In this case platform keyring won't
have any secure IPL keys (since firmware doesn't provide them)
and any properly signed kernels will be rejected for kexec in this
function. Unless secure IPL keys are also present in built-in or secondary
keyring (which is possible after commit 0828c4a39be5) - is that what
distributions normally do?
Coiby Xu Nov. 23, 2022, 12:52 a.m. UTC | #2 
On Tue, Nov 22, 2022 at 04:15:04PM +0100, Vasily Gorbik wrote:
>On Mon, Nov 21, 2022 at 03:27:15PM +0800, Coiby Xu wrote:
>> --- a/arch/s390/kernel/machine_kexec_file.c
>> +++ b/arch/s390/kernel/machine_kexec_file.c
>> @@ -33,10 +33,6 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
>>  	unsigned long sig_len;
>>  	int ret;
>>
>> -	/* Skip signature verification when not secure IPLed. */
>> -	if (!ipl_secure_flag)
>> -		return 0;
>
>Looking at s390_verify_sig() especially before commit 0828c4a39be5
>("kexec, KEYS, s390: Make use of built-in and secondary keyring for
>signature verification") I think this condition actually expresses
>2 things:
>1. the firmware is secure IPL capable and secure IPL keys are
>   provided and present in platform keyring.
>2. secure IPL is enabled.
>
>Wouldn't this change have implications for machines with older firmware
>which doesn't support secure IPL? In this case platform keyring won't
>have any secure IPL keys (since firmware doesn't provide them)
>and any properly signed kernels will be rejected for kexec in this
>function. Unless secure IPL keys are also present in built-in or secondary
>keyring (which is possible after commit 0828c4a39be5) - is that what
>distributions normally do?

Thanks for pointing me to the above commit and reminding me older
firmware doesn't support secure IPL! But I don't think this change will
break machines with older firmwares which doesn't support secure IPL.
Distributions like Fedora/RHEL have downstream-only patch that enable
lockdown automatically when secure boot is enabled. Since there is no
secure IPL, lockdown won't be enabled which means
kimage_validate_signature (kernel/kexec_file.c) doesn't enforce
signature verification (sorry I should change the commit subject which
is misleading). For the case where users manually enables lockdown, I
assume they know what lockdown means and expect signature verification
to be enforced instead to be silently bypassed.
diff mbox series

Patch

diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index fc6d5f58debe..627685426ac2 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -33,10 +33,6 @@  int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 	unsigned long sig_len;
 	int ret;
 
-	/* Skip signature verification when not secure IPLed. */
-	if (!ipl_secure_flag)
-		return 0;
-
 	if (marker_len > kernel_len)
 		return -EKEYREJECTED;