From: Dave Young <dyoung@redhat.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: Yannik Sembritzki <yannik@sembritzki.me>,
Linus Torvalds <torvalds@linux-foundation.org>,
David Howells <dhowells@redhat.com>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Peter Anvin <hpa@zytor.com>,
the arch/x86 maintainers <x86@kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Baoquan He <bhe@redhat.com>,
"Justin M. Forbes" <jforbes@redhat.com>
Subject: Re: [PATCH] Fix kexec forbidding kernels signed with custom platform keys to boot
Date: Thu, 16 Aug 2018 08:52:06 +0800 [thread overview]
Message-ID: <20180816005206.GA5915@dhcp-128-65.nay.redhat.com> (raw)
In-Reply-To: <20180815174247.GB29541@redhat.com>
On 08/15/18 at 01:42pm, Vivek Goyal wrote:
> On Wed, Aug 15, 2018 at 07:27:33PM +0200, Yannik Sembritzki wrote:
> > Would this be okay?
>
> [ CC dave young, Baoquan, Justin Forbes]
>
> Hi Yannik,
>
> I am reading that bug and wondering that what broke it. It used to work,
> so some change broke it.
>
> Justin said that we have been signing fedora kernels with fedora keys so
> looks like no change there.
>
> Previously, I think all the keys used to go in system keyring and it
> used to work. Is it somehow because of split in builtin keyring and
> secondary system keyring. Could it be that fedora key used to show
> up in system keyring previously and it worked but now it shows up
> in secondary system keyring and by default we don't use keys from
> that keyring for signature verification?
There was a Fedora bug below:
https://bugzilla.redhat.com/show_bug.cgi?id=1470995
I posted a fix here but bobody responsed, I think I obviously did not
consider the "trust build system only" point from Linus:
http://lists.infradead.org/pipermail/kexec/2017-November/019632.html
But either above patch or defining a macro for the "1UL" in cert header
file works.
Since nobody reviewed my patch so later I submitted a Fedora only patch
which is similar with Yannik's and merged in Fedora tree:
https://bugzilla.redhat.com/attachment.cgi?id=1450772&action=edit
>
> Thanks
> Vivek
>
> >
> > diff --git a/arch/x86/kernel/kexec-bzimage64.c
> > b/arch/x86/kernel/kexec-bzimage64.c
> > index 7326078e..2ba47e24 100644
> > --- a/arch/x86/kernel/kexec-bzimage64.c
> > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > @@ -41,6 +41,9 @@
> > #define MIN_KERNEL_LOAD_ADDR 0x100000
> > #define MIN_INITRD_LOAD_ADDR 0x1000000
> >
> > +// Allow both builtin trusted keys and secondary trusted keys
> > +#define TRUST_FULL_KEYRING (void *)1UL
> > +
> > /*
> > * This is a place holder for all boot loader specific data structure which
> > * gets allocated in one call but gets freed much later during cleanup
> > @@ -532,7 +535,7 @@ static int bzImage64_cleanup(void *loader_data)
> > static int bzImage64_verify_sig(const char *kernel, unsigned long
> > kernel_len)
> > {
> > return verify_pefile_signature(kernel, kernel_len,
> > - NULL,
> > + TRUST_FULL_KEYRING,
> > VERIFYING_KEXEC_PE_SIGNATURE);
> > }
> > #endif
> > --
> >
> > On 15.08.2018 18:54, Linus Torvalds wrote:
> > > This needs more people involved, and at least a sign-off.
> > >
> > > It looks ok, but I think we need a #define for the magical (void *)1UL
> > > thing. I see the use in verify_pkcs7_signature(), but still.
> > >
> > > Linus
> > >
> > >
> > >
> > > On Wed, Aug 15, 2018 at 3:11 AM Yannik Sembritzki <yannik@sembritzki.me> wrote:
> > >> ---
> > >> arch/x86/kernel/kexec-bzimage64.c | 2 +-
> > >> 1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> > >> index 7326078e..eaaa125d 100644
> > >> --- a/arch/x86/kernel/kexec-bzimage64.c
> > >> +++ b/arch/x86/kernel/kexec-bzimage64.c
> > >> @@ -532,7 +532,7 @@ static int bzImage64_cleanup(void *loader_data)
> > >> static int bzImage64_verify_sig(const char *kernel, unsigned long kernel_len)
> > >> {
> > >> return verify_pefile_signature(kernel, kernel_len,
> > >> - NULL,
> > >> + (void *)1UL,
> > >> VERIFYING_KEXEC_PE_SIGNATURE);
> > >> }
> > >> #endif
> > >> --
> > >> 2.17.1
> > >>
> > >> The exact scenario under which this issue occurs is described here:
> > >> https://bugzilla.redhat.com/show_bug.cgi?id=1554113
> > >>
> >
Thanks
Dave
next prev parent reply other threads:[~2018-08-16 0:52 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-15 10:00 [PATCH] Fix kexec forbidding kernels signed with custom platform keys to boot Yannik Sembritzki
2018-08-15 16:54 ` Linus Torvalds
2018-08-15 17:27 ` Yannik Sembritzki
2018-08-15 17:37 ` Yannik Sembritzki
2018-08-15 17:42 ` Vivek Goyal
2018-08-15 18:44 ` Yannik Sembritzki
2018-08-15 18:58 ` Vivek Goyal
2018-08-15 19:06 ` Yannik Sembritzki
2018-08-15 19:49 ` Vivek Goyal
2018-08-15 20:47 ` Linus Torvalds
2018-08-15 20:53 ` James Bottomley
2018-08-15 21:08 ` Yannik Sembritzki
2018-08-15 21:13 ` James Bottomley
2018-08-15 21:31 ` Yannik Sembritzki
2018-08-15 21:40 ` James Bottomley
2018-08-15 21:50 ` Yannik Sembritzki
2018-08-15 21:57 ` Vivek Goyal
2018-08-15 22:14 ` Yannik Sembritzki
2018-08-15 21:52 ` Vivek Goyal
2018-08-15 21:57 ` James Bottomley
2018-08-15 21:14 ` Linus Torvalds
2018-08-16 13:51 ` David Howells
2018-08-16 15:16 ` James Bottomley
2018-08-16 15:42 ` James Bottomley
2018-08-16 15:49 ` David Howells
2018-08-16 15:56 ` James Bottomley
2018-08-16 16:56 ` David Laight
2018-08-16 17:15 ` James Bottomley
2018-08-16 20:31 ` David Howells
2018-08-17 0:07 ` James Bottomley
2018-08-17 8:24 ` David Howells
2018-08-17 14:58 ` James Bottomley
2018-08-17 15:42 ` Justin Forbes
2018-08-17 16:02 ` James Bottomley
2018-08-16 0:52 ` Dave Young [this message]
2018-08-16 0:55 ` Dave Young
2018-08-16 12:13 ` David Howells
2018-08-16 14:22 ` James Bottomley
2018-08-16 14:43 ` David Howells
2018-08-16 14:59 ` James Bottomley
2018-08-17 17:00 ` Alan Cox
2018-08-15 17:45 ` Linus Torvalds
2018-08-15 18:19 ` Yannik Sembritzki
2018-08-15 18:22 ` Linus Torvalds
2018-08-15 19:42 ` [PATCH 0/2] Fix kexec forbidding kernels signed with keys in the secondary keyring " Yannik Sembritzki
2018-08-16 18:02 ` Linus Torvalds
2018-08-15 19:42 ` [PATCH 1/2] " Yannik Sembritzki
2018-08-15 19:42 ` [PATCH 2/2] Replace magic for trusting the secondary keyring with #define Yannik Sembritzki
2018-08-15 21:14 ` kbuild test robot
2018-08-15 21:19 ` [PATCH 2/2] [FIXED] " Yannik Sembritzki
2018-08-15 22:01 ` Linus Torvalds
2018-08-15 22:07 ` [PATCH 2/2] [FIXED v2] " Yannik Sembritzki
2018-08-16 1:11 ` Dave Young
2018-08-16 7:43 ` Yannik Sembritzki
2018-08-16 8:02 ` Dave Young
2018-08-16 8:20 ` Greg Kroah-Hartman
2018-08-16 12:46 ` Vivek Goyal
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=20180816005206.GA5915@dhcp-128-65.nay.redhat.com \
--to=dyoung@redhat.com \
--cc=bhe@redhat.com \
--cc=dhowells@redhat.com \
--cc=hpa@zytor.com \
--cc=jforbes@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=vgoyal@redhat.com \
--cc=x86@kernel.org \
--cc=yannik@sembritzki.me \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).