All of lore.kernel.org
 help / color / mirror / Atom feed
From: Coiby Xu <coxu@redhat.com>
To: Mimi Zohar <zohar@linux.ibm.com>
Cc: kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org,
	Michal Suchanek <msuchanek@suse.de>, Baoquan He <bhe@redhat.com>,
	Dave Young <dyoung@redhat.com>, Will Deacon <will@kernel.org>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	Chun-Yi Lee <jlee@suse.com>
Subject: Re: [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
Date: Thu, 16 Jun 2022 09:15:06 +0800	[thread overview]
Message-ID: <20220616011506.ymbz2xuhw3refasw@Rk> (raw)
In-Reply-To: <b3c87ce140b8d46659adcf07142e1d01ffbb2267.camel@linux.ibm.com>

Hi Mimi,

Thanks for carefully reviewing the covert letter and patches and
suggesting various improvements! And sorry for the late reply as I need
some time to learn more about secure boot, lockdown and IMA to better
make sense of what you mean. 

On Fri, May 27, 2022 at 12:45:54PM -0400, Mimi Zohar wrote:
>
>> new cover letter here to collect new feedback from you thus we
>> can avoid unnecessary rounds of patch set.
>
>Agreed.   Much better.  Just a couple of nits.

>
>> Currently when loading a kernel image via the kexec_file_load() system
>> call, x86 can make use of three keyrings i.e. the .builtin_trusted_keys,
>> .secondary_trusted_keys and .platform keyrings to verify signature.
>
>Either "a signature" or "signatures".
>
>> However, arm64 and s390 can only use the .builtin_trusted_keys and
>> .platform keyring respectively. For example, one resulting problem is
>> kexec'ing a kernel image  would be rejected with the error "Lockdown:
>> kexec: kexec of unsigned images is restricted; see man
>> kernel_lockdown.7".
>>
>> This patch set enables arm64 and s390 to make use of the same keyrings
>> as x86 to very the signature kexec'ed kernel image.
>
>  Fix "very".  Perhaps "verify the kexec'ed kernel image signature".
>>
>> The recently introduced .machine keyring impacts the roots of trust by
>> linking the .machine keyring to the .secondary keyring. The roots of
>> trust of different keyring are described as follows,
>>
>"of ... keyring" -> "for the ... keyrings"

Thanks for catching those typos and improving the wording!

>
>> .builtin_trusted_keys:
>>
>> Keys may be built into the kernel during build or inserted into memory
>> reserved for keys post build. The root of trust is the kernel build i.e.
>> a Linux distribution vendor. On a physical system in a secure boot
>> environment, this trust is rooted in hardware.
>
>Please look at my response to your question below.
>
>>
>> .machine:
>>
>> If the end-users choose to trust the keys provided by first-stage UEFI
>> bootloader shim i.e. Machine Owner Keys (MOK keys), the keys will be
>> added to this keyring and this keyring is linked to the
>
> Grammatically "and this"  needs to be fixed.

How about 
"the keys will be added to this keyring which  is linked to the..."?

>
>> .secondary_trusted_keys keyring as same as the .builtin_trusted_keys
>> keyring. Shim has built-in keys from a Linux distribution or the
>> end-users-enrolled keys. So the root of trust of this keyring is either
>> a Linux distribution vendor or the end-users.
>>
>> .secondary_trusted_keys:
>>
>> Certificates signed by keys on the .builtin_trusted_keys, .machine, or
>> existing keys on the .secondary_trusted_keys keryings may be loaded
>> onto the .secondary_trusted_keys keyring. This establishes a signature
>> chain of trust based on keys loaded on either the .builtin_trusted_keys
>> or .machine keyrings, if configured and enabled.
>>
>> .platform:
>>
>> The .platform keyring consist of UEFI db and MOK keys which are used by
>> shim to verify the first boot kernel's image signature. If end-users
>> choose to trust MOK keys and the kernel has the .machine keyring
>> enabled, the .platform keyring only consists of UEFI db keys since the
>> MOK keys are added to the .machine keyring instead. Because the
>> end-users could also enroll there own MOK keys, the root of trust could
>
>"there" -> "their"
>
>> be hardware or the end-users.
>
>It's always "hardware".  "or" -> "and"?

Thanks for catching these issues as well!

>
><snip>
>
>> >>
>> >> The root of trusts of the keys in the %.builtin_trusted_keys and
>> >> secondary_trusted_keys keyring is a Linux distribution vendor.
>> >
>> >The root of trust for each keyring should be described separately.
>> >
>> >.builtin_trusted_keys:
>> >
>> >For example,
>> >
>> >Keys may be built into the kernel during build or inserted into memory
>> >reserved for keys post build.  In both of these cases, trust is based
>> >on verification of the kernel image signature.
>>
>> Correct me if I'm wrong, without secure boot, there is no verification
>> of the kernel image signature so the root of trust should be trust on
>> the kernel builder.
>
>No, basing the signature verification on secure boot could not have
>been upstreamed.  

Thanks for correcting me! I was a bit confused by secure boot and
lockdown and also forgot enabling lockdown automatically when secure
boot is enabled is a downstream feature. Btw, when testing the 4th s390
patch, I found s390 skip signature validation when secure boot is not
enabled, is this a mistake?

// arch/s390/kernel/machine_kexec_file.c
#ifdef CONFIG_KEXEC_SIG
int s390_verify_sig(const char *kernel, unsigned long kernel_len)
{
	/* Skip signature verification when not secure IPLed. */
	if (!ipl_secure_flag)
		return 0;

> IMA is based on policy, regardeless of the secure
>boot mode.  A builtin policy may be specified on the boot command line,
>but should be replaced with a more constrained custom policy [1].
>Unlike the builtin policy rules, the architecture specific rules are
>persistent[2].   The architecture specific rules are normally tied to
>the secure boot modes.  On OpenPOWER, the architecture specific
>"measure" rules are dependent on the trusted boot mode.
>
>The current IMA policy rules can be viewed by cat'ing
><securityfs>/ima/policy.

Thanks for explaining IMA to me! There is still the question of what's
the root of trust for .builtin_trusted_keys when there is no real
signature verification. For example, when CONFIG_KEXEC_SIG is enabled,
the default IMA policy is to not appraise kexec image. Since lockdown is
not enabled by default, there is no real verification as
kimage_validate_signature succeeds even when kexec_image_verify_sig
fails. 

>
>[1] The builtin policies are not LSM aware.  The policy rules need to
>be constrained to avoid integrity violations.
>
>[2] arch specific policy rules:
>security/integrity/ima/ima_efi.c, arch/powerpc/kernel/ima_arch.c
>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Coiby Xu <coxu@redhat.com>
To: Mimi Zohar <zohar@linux.ibm.com>
Cc: kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org,
	Michal Suchanek <msuchanek@suse.de>, Baoquan He <bhe@redhat.com>,
	Dave Young <dyoung@redhat.com>, Will Deacon <will@kernel.org>,
	"Eric W . Biederman" <ebiederm@xmission.com>,
	Chun-Yi Lee <jlee@suse.com>
Subject: Re: [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
Date: Thu, 16 Jun 2022 09:15:06 +0800	[thread overview]
Message-ID: <20220616011506.ymbz2xuhw3refasw@Rk> (raw)
In-Reply-To: <b3c87ce140b8d46659adcf07142e1d01ffbb2267.camel@linux.ibm.com>

Hi Mimi,

Thanks for carefully reviewing the covert letter and patches and
suggesting various improvements! And sorry for the late reply as I need
some time to learn more about secure boot, lockdown and IMA to better
make sense of what you mean. 

On Fri, May 27, 2022 at 12:45:54PM -0400, Mimi Zohar wrote:
>
>> new cover letter here to collect new feedback from you thus we
>> can avoid unnecessary rounds of patch set.
>
>Agreed.   Much better.  Just a couple of nits.

>
>> Currently when loading a kernel image via the kexec_file_load() system
>> call, x86 can make use of three keyrings i.e. the .builtin_trusted_keys,
>> .secondary_trusted_keys and .platform keyrings to verify signature.
>
>Either "a signature" or "signatures".
>
>> However, arm64 and s390 can only use the .builtin_trusted_keys and
>> .platform keyring respectively. For example, one resulting problem is
>> kexec'ing a kernel image  would be rejected with the error "Lockdown:
>> kexec: kexec of unsigned images is restricted; see man
>> kernel_lockdown.7".
>>
>> This patch set enables arm64 and s390 to make use of the same keyrings
>> as x86 to very the signature kexec'ed kernel image.
>
>  Fix "very".  Perhaps "verify the kexec'ed kernel image signature".
>>
>> The recently introduced .machine keyring impacts the roots of trust by
>> linking the .machine keyring to the .secondary keyring. The roots of
>> trust of different keyring are described as follows,
>>
>"of ... keyring" -> "for the ... keyrings"

Thanks for catching those typos and improving the wording!

>
>> .builtin_trusted_keys:
>>
>> Keys may be built into the kernel during build or inserted into memory
>> reserved for keys post build. The root of trust is the kernel build i.e.
>> a Linux distribution vendor. On a physical system in a secure boot
>> environment, this trust is rooted in hardware.
>
>Please look at my response to your question below.
>
>>
>> .machine:
>>
>> If the end-users choose to trust the keys provided by first-stage UEFI
>> bootloader shim i.e. Machine Owner Keys (MOK keys), the keys will be
>> added to this keyring and this keyring is linked to the
>
> Grammatically "and this"  needs to be fixed.

How about 
"the keys will be added to this keyring which  is linked to the..."?

>
>> .secondary_trusted_keys keyring as same as the .builtin_trusted_keys
>> keyring. Shim has built-in keys from a Linux distribution or the
>> end-users-enrolled keys. So the root of trust of this keyring is either
>> a Linux distribution vendor or the end-users.
>>
>> .secondary_trusted_keys:
>>
>> Certificates signed by keys on the .builtin_trusted_keys, .machine, or
>> existing keys on the .secondary_trusted_keys keryings may be loaded
>> onto the .secondary_trusted_keys keyring. This establishes a signature
>> chain of trust based on keys loaded on either the .builtin_trusted_keys
>> or .machine keyrings, if configured and enabled.
>>
>> .platform:
>>
>> The .platform keyring consist of UEFI db and MOK keys which are used by
>> shim to verify the first boot kernel's image signature. If end-users
>> choose to trust MOK keys and the kernel has the .machine keyring
>> enabled, the .platform keyring only consists of UEFI db keys since the
>> MOK keys are added to the .machine keyring instead. Because the
>> end-users could also enroll there own MOK keys, the root of trust could
>
>"there" -> "their"
>
>> be hardware or the end-users.
>
>It's always "hardware".  "or" -> "and"?

Thanks for catching these issues as well!

>
><snip>
>
>> >>
>> >> The root of trusts of the keys in the %.builtin_trusted_keys and
>> >> secondary_trusted_keys keyring is a Linux distribution vendor.
>> >
>> >The root of trust for each keyring should be described separately.
>> >
>> >.builtin_trusted_keys:
>> >
>> >For example,
>> >
>> >Keys may be built into the kernel during build or inserted into memory
>> >reserved for keys post build.  In both of these cases, trust is based
>> >on verification of the kernel image signature.
>>
>> Correct me if I'm wrong, without secure boot, there is no verification
>> of the kernel image signature so the root of trust should be trust on
>> the kernel builder.
>
>No, basing the signature verification on secure boot could not have
>been upstreamed.  

Thanks for correcting me! I was a bit confused by secure boot and
lockdown and also forgot enabling lockdown automatically when secure
boot is enabled is a downstream feature. Btw, when testing the 4th s390
patch, I found s390 skip signature validation when secure boot is not
enabled, is this a mistake?

// arch/s390/kernel/machine_kexec_file.c
#ifdef CONFIG_KEXEC_SIG
int s390_verify_sig(const char *kernel, unsigned long kernel_len)
{
	/* Skip signature verification when not secure IPLed. */
	if (!ipl_secure_flag)
		return 0;

> IMA is based on policy, regardeless of the secure
>boot mode.  A builtin policy may be specified on the boot command line,
>but should be replaced with a more constrained custom policy [1].
>Unlike the builtin policy rules, the architecture specific rules are
>persistent[2].   The architecture specific rules are normally tied to
>the secure boot modes.  On OpenPOWER, the architecture specific
>"measure" rules are dependent on the trusted boot mode.
>
>The current IMA policy rules can be viewed by cat'ing
><securityfs>/ima/policy.

Thanks for explaining IMA to me! There is still the question of what's
the root of trust for .builtin_trusted_keys when there is no real
signature verification. For example, when CONFIG_KEXEC_SIG is enabled,
the default IMA policy is to not appraise kexec image. Since lockdown is
not enabled by default, there is no real verification as
kimage_validate_signature succeeds even when kexec_image_verify_sig
fails. 

>
>[1] The builtin policies are not LSM aware.  The policy rules need to
>be constrained to avoid integrity violations.
>
>[2] arch specific policy rules:
>security/integrity/ima/ima_efi.c, arch/powerpc/kernel/ima_arch.c
>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2022-06-16  1:20 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-12  7:01 [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Coiby Xu
2022-05-12  7:01 ` Coiby Xu
2022-05-12  7:01 ` [PATCH v8 1/4] kexec: clean up arch_kexec_kernel_verify_sig Coiby Xu
2022-05-12  7:01   ` Coiby Xu
2022-05-12  7:01   ` Coiby Xu
2022-06-09 21:57   ` Mimi Zohar
2022-06-09 21:57     ` Mimi Zohar
2022-06-09 21:57     ` Mimi Zohar
2022-05-12  7:01 ` [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic Coiby Xu
2022-05-12  7:01   ` Coiby Xu
2022-05-12  7:01   ` Coiby Xu
2022-05-12  7:21   ` Baoquan He
2022-05-12  7:21     ` Baoquan He
2022-05-12  7:21     ` Baoquan He
2022-06-09 22:18   ` Mimi Zohar
2022-06-09 22:18     ` Mimi Zohar
2022-06-09 22:18     ` Mimi Zohar
2022-06-16  1:47     ` Coiby Xu
2022-06-16  1:47       ` Coiby Xu
2022-06-16  1:47       ` Coiby Xu
2022-05-12  7:01 ` [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature Coiby Xu
2022-05-12  7:01   ` Coiby Xu
2022-05-12  7:01   ` Coiby Xu
2022-06-09 23:15   ` Mimi Zohar
2022-06-09 23:15     ` Mimi Zohar
2022-06-09 23:15     ` Mimi Zohar
2022-06-16  1:22     ` Coiby Xu
2022-06-16  1:22       ` Coiby Xu
2022-06-16  1:22       ` Coiby Xu
2022-06-17  9:34     ` Michal Suchánek
2022-06-17  9:34       ` Michal Suchánek
2022-06-17  9:34       ` Michal Suchánek
2022-05-12  7:01 ` [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification Coiby Xu
2022-05-12  7:01   ` Coiby Xu
2022-05-12  7:01   ` Coiby Xu
2022-05-18 11:29   ` Heiko Carstens
2022-05-18 11:29     ` Heiko Carstens
2022-05-18 11:29     ` Heiko Carstens
2022-05-19  0:39     ` Baoquan He
2022-05-19  0:39       ` Baoquan He
2022-05-19  0:39       ` Baoquan He
2022-05-19 11:56       ` Mimi Zohar
2022-05-19 11:56         ` Mimi Zohar
2022-05-19 11:56         ` Mimi Zohar
2022-05-19 14:22         ` Baoquan He
2022-05-19 14:22           ` Baoquan He
2022-05-19 14:22           ` Baoquan He
2022-05-19 17:11           ` Michal Suchánek
2022-05-19 17:11             ` Michal =?unknown-8bit?q?Such=C3=A1nek?=
2022-05-19 17:11             ` Michal Suchánek
2022-06-16  1:46             ` Coiby Xu
2022-06-16  1:46               ` Coiby Xu
2022-06-16  1:46               ` Coiby Xu
2022-05-20 17:04 ` [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Mimi Zohar
2022-05-20 17:04   ` Mimi Zohar
2022-05-25  9:59   ` Coiby Xu
2022-05-25  9:59     ` Coiby Xu
2022-05-25 13:30     ` Mimi Zohar
2022-05-25 13:30       ` Mimi Zohar
2022-05-27 13:43       ` Coiby Xu
2022-05-27 13:43         ` Coiby Xu
2022-05-27 16:45         ` Mimi Zohar
2022-05-27 16:45           ` Mimi Zohar
2022-06-16  1:15           ` Coiby Xu [this message]
2022-06-16  1:15             ` Coiby Xu
2022-06-17  3:57             ` Coiby Xu
2022-06-17  3:57               ` Coiby Xu
2022-06-17 11:58               ` Mimi Zohar
2022-06-17 11:58                 ` Mimi Zohar
2022-06-20 13:14                 ` Coiby Xu
2022-06-20 13:14                   ` Coiby Xu
2022-06-09 15:35         ` Mimi Zohar
2022-06-09 15:35           ` Mimi Zohar
2022-06-16  1:21           ` Coiby Xu
2022-06-16  1:21             ` Coiby Xu
2022-06-17 12:06             ` Mimi Zohar
2022-06-17 12:06               ` Mimi Zohar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220616011506.ymbz2xuhw3refasw@Rk \
    --to=coxu@redhat.com \
    --cc=bhe@redhat.com \
    --cc=dyoung@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=jlee@suse.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=msuchanek@suse.de \
    --cc=will@kernel.org \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.