Re: [PATCH 5.15 0/6] arm64: kexec_file: use more system keyrings to verify kernel image signature + dependencies

["Michal Suchánek" <msuchanek@suse.de>]

On Tue, Sep 27, 2022 at 11:39:52AM +0900, AKASHI Takahiro wrote:
> On Mon, Sep 26, 2022 at 09:40:25AM +0200, Michal Such??nek wrote:
> > On Mon, Sep 26, 2022 at 08:47:32AM +0200, Greg Kroah-Hartman wrote:
> > > On Sat, Sep 24, 2022 at 01:55:23PM +0200, Michal Suchánek wrote:
> > > > On Sat, Sep 24, 2022 at 12:13:34PM +0200, Greg Kroah-Hartman wrote:
> > > > > On Sat, Sep 24, 2022 at 11:45:21AM +0200, Michal Suchánek wrote:
> > > > > > On Sat, Sep 24, 2022 at 11:19:19AM +0200, Greg Kroah-Hartman wrote:
> > > > > > > On Fri, Sep 23, 2022 at 07:10:28PM +0200, Michal Suchanek wrote:
> > > > > > > > Hello,
> > > > > > > > 
> > > > > > > > this is backport of commit 0d519cadf751
> > > > > > > > ("arm64: kexec_file: use more system keyrings to verify kernel image signature")
> > > > > > > > to table 5.15 tree including the preparatory patches.
> > > > > > > 
> > > > > > > This feels to me like a new feature for arm64, one that has never worked
> > > > > > > before and you are just making it feature-parity with x86, right?
> > > > > > > 
> > > > > > > Or is this a regression fix somewhere?  Why is this needed in 5.15.y and
> > > > > > > why can't people who need this new feature just use a newer kernel
> > > > > > > version (5.19?)
> > > > > > 
> > > > > > It's half-broken implementation of the kexec kernel verification. At the time
> > > > > > it was implemented for arm64 we had the platform and secondary keyrings
> > > > > > and x86 was using them but on arm64 the initial implementation ignores
> > > > > > them.
> > > > > 
> > > > > Ok, so it's something that never worked.  Adding support to get it to
> > > > > work doesn't really fall into the stable kernel rules, right?
> > > > 
> > > > Not sure. It was defective, not using the facilities available at the
> > > > time correctly. Which translates to kernels that can be kexec'd on x86
> > > > failing to kexec on arm64 without any explanation (signed with same key,
> > > > built for the appropriate arch).
> > > 
> > > Feature parity across architectures is not a "regression", but rather a
> > > "this feature is not implemented for this architecture yet" type of
> > > thing.
> > 
> > That depends on the view - before kexec verification you could boot any
> > kernel, now you can boot some kernels signed with a valid key, but not
> > others - the initial implementation is buggy, probably because it
> > is based on an old version of the x86 code.
> 
> Buggy?
> The feature of supporting platform ring had been slipped in just before
> I submitted the latest patch series which was eventually merged.
> (I should have noticed it though.)

It's difficult to notice another in-flight patch that does not conflict
with yours, and is for a different architecture. That's why we have
followup patches and Fixes tags.

However, the support for secondary keyring was added in 4.19 by commit
ea93102f3224 ("Fix kexec forbidding kernels signed with keys in the
secondary keyring to boot") which was not supported by the arm64 code
either.

> Looking at changes in the commit 278311e417be ("kexec, KEYS: Make use of platform
> keyring for signature verify"), it seems to be obvious that it is a new feature
> because it introduced a new Kconfig option, CONFIG_INTEGRITY_PLATFORM_KEYRING,
> which allows for enabling/disabling platform ring support.

Yes, and that feature exists since 5.1, and we are talking about 5.15
here. Not making use of the keyring that is supported by the kernel
results in inability to kexec kernels that are signed by a valid key,
arguably a bug.

Thanks

Michal