Hi Jan,
All in all the code looks fine to me. Nevertheless I don't think UKI support
should be added at the moment. This is because IMHO you basically reinterpret
the kexec_file systemcall and thus add a new uapi to the kernel. Once
introduced it is extremely hard to remove or change an uapi again. The problem
I see is that the spec you based your work on is in such a poor shape that I
don't feel comfortable to add any new uapi based on it.
For example there are two definitions for the UKI which contradict each other.
The dedicated one [1] you have cited earlier and the one in the BLS for type #2
entries [2]. In [1] the .linux and .initrd sections are mandatory and the
.osrel and .cmdline sections are optional while in [2] it is the other way
round. Which definition should the kernel follow?
Furthermore, I absolutely don't understand how the spec should be read. All
the spec does is defining some file formats. There is no word about which
component in the boot chain is supposed to handle them and what exactly this
component is supposed to do with it. But that is crucial if we want to add UKI
support for kexec as the kexec systemcall will replace the stub. So we need to
know what tasks the stub is supposed to perform. Currently this is only some
implementation detail of the systemd-stub [3] that can change any moment and I
strongly oppose to base any uapi on it.
In the end the only benefit this series brings is to extend the signature
checking on the whole UKI except of just the kernel image. Everything else can
also be done in user space. Compared to the problems described above this is a
very small gain for me.
Until the spec got fixed I don't see a chance to add UKI support for kexec.
Thanks
Philipp
[1] https://uapi-group.org/specifications/specs/unified_kernel_image/
[2] https://uapi-group.org/specifications/specs/boot_loader_specification/#type-2-efi-unified-kernel-images
[3] https://www.freedesktop.org/software/systemd/man/systemd-stub.html
On Mon, 11 Sep 2023 07:25:33 +0200
Jan Hendrik Farr <[email protected]> wrote:
> Hello,
>
> this patch (v2) implements UKI support for kexec_file_load. It will require
> support in the kexec-tools userspace utility. For testing purposes the
> following can be used: https://github.com/Cydox/kexec-test/
>
> Creating UKIs for testing can be done with ukify (included in systemd),
> sbctl, and mkinitcpio, etc.
>
> There has been discussion on this topic in an issue on GitHub that is linked
> below for reference.
>
> Changes for v2:
> - .cmdline section is now optional
> - moving pefile_parse_binary is now in a separate commit for clarity
> - parse_pefile.c is now in /lib instead of arch/x86/kernel (not sure if
> this is the best location, but it definetly shouldn't have been in an
> architecture specific location)
> - parse_pefile.h is now in include/kernel instead of architecture
> specific location
> - if initrd or cmdline is manually supplied EPERM is returned instead of
> being silently ignored
> - formatting tweaks
>
>
> Some links:
> - Related discussion: https://github.com/systemd/systemd/issues/28538
> - Documentation of UKIs: https://uapi-group.org/specifications/specs/unified_kernel_image/
>
> Jan Hendrik Farr (2):
> move pefile_parse_binary to its own file
> x86/kexec: UKI support
>
> arch/x86/include/asm/kexec-uki.h | 7 ++
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/kexec-uki.c | 126 +++++++++++++++++++++++++
> arch/x86/kernel/machine_kexec_64.c | 2 +
> crypto/asymmetric_keys/mscode_parser.c | 2 +-
> crypto/asymmetric_keys/verify_pefile.c | 110 +++------------------
> crypto/asymmetric_keys/verify_pefile.h | 16 ----
> include/linux/parse_pefile.h | 32 +++++++
> lib/Makefile | 3 +
> lib/parse_pefile.c | 109 +++++++++++++++++++++
> 10 files changed, 292 insertions(+), 116 deletions(-)
> create mode 100644 arch/x86/include/asm/kexec-uki.h
> create mode 100644 arch/x86/kernel/kexec-uki.c
> create mode 100644 include/linux/parse_pefile.h
> create mode 100644 lib/parse_pefile.c
>
On Wed, Sep 13, 2023, at 4:00 PM, Philipp Rudo wrote:
> Hi Jan,
>
> All in all the code looks fine to me. Nevertheless I don't think UKI
> support should be added at the moment. This is because IMHO you
> basically reinterpret the kexec_file systemcall and thus add a new
> uapi to the kernel. Once introduced it is extremely hard to remove or
> change an uapi again. The problem I see is that the spec you based
> your work on is in such a poor shape that I don't feel comfortable to
> add any new uapi based on it.
>
> For example there are two definitions for the UKI which contradict
> each other. The dedicated one [1] you have cited earlier and the one
> in the BLS for type #2 entries [2]. In [1] the .linux and .initrd
> sections are mandatory and the .osrel and .cmdline sections are
> optional while in [2] it is the other way round. Which definition
> should the kernel follow?
>
> Furthermore, I absolutely don't understand how the spec should be
> read. All the spec does is defining some file formats. There is no
> word about which component in the boot chain is supposed to handle
> them and what exactly this component is supposed to do with it. But
> that is crucial if we want to add UKI support for kexec as the kexec
> systemcall will replace the stub. So we need to know what tasks the
> stub is supposed to perform. Currently this is only some
> implementation detail of the systemd-stub [3] that can change any
> moment and I strongly oppose to base any uapi on it.
>
I think I have to agree with you on this one.
I kinda experienced this first hand. My initial patch required a kernel
command line, because the spec in [1] didn't say it was optional. Then
I got a response saying that it's actually optional. And the spec got
changed in PR [2] (I have a WIP v3 that allows a manual cmdline if the
.cmdline section is empty as explained by Lennart in [2] and now
also in the spec since [3]). So there's already 2 changes in the spec
in the last few days related to my patch. That shows that the spec is
not stable.
What's even worse IMO is that the reason given in [2] to change the spec
is simply that systemd-stub has a different behavior than the spec and
therefore the spec should be updated. At this point it's not really a
specification but simply a flawed documentation of the behavior of systemd-
stub / ukify.
I also discovered that ukify treats the initrd as optional [4]. So is
this the next change in the spec?
In [5] Luca writes:
> [...] we fully intend for the UKI format to be an open and stable
> specification, that anybody can support and rely on.
But that is unfortunately not where the format is at this point.
What is annoying though is where this leaves a user that actually
wants this feature. They can carry a patch or they might have to wait
a long time.
Can you indicate what it would take for the kernel community to consider
this spec as stable enough?
> In the end the only benefit this series brings is to extend the
> signature checking on the whole UKI except of just the kernel image.
> Everything else can also be done in user space. Compared to the
> problems described above this is a very small gain for me.
Correct. That is the benefit of pulling the UKI apart in the
kernel. However having to sign the kernel inside the UKI defeats
the whole point.
[1] https://uapi-group.org/specifications/specs/unified_kernel_image/
[2] https://github.com/uapi-group/specifications/pull/72
[3] https://github.com/uapi-group/specifications/pull/73
[4] https://github.com/uapi-group/specifications/issues/74
[5] https://github.com/systemd/systemd/issues/28538
On Mi, 13.09.23 16:00, Philipp Rudo ([email protected]) wrote:
> For example there are two definitions for the UKI which contradict each other.
> The dedicated one [1] you have cited earlier and the one in the BLS for type #2
> entries [2]. In [1] the .linux and .initrd sections are mandatory and the
> .osrel and .cmdline sections are optional while in [2] it is the other way
> round. Which definition should the kernel follow?
>
> Furthermore, I absolutely don't understand how the spec should be read. All
> the spec does is defining some file formats. There is no word about which
> component in the boot chain is supposed to handle them and what exactly this
> component is supposed to do with it. But that is crucial if we want to add UKI
> support for kexec as the kexec systemcall will replace the stub. So we need to
> know what tasks the stub is supposed to perform. Currently this is only some
> implementation detail of the systemd-stub [3] that can change any moment and I
> strongly oppose to base any uapi on it.
>
> In the end the only benefit this series brings is to extend the signature
> checking on the whole UKI except of just the kernel image. Everything else can
> also be done in user space. Compared to the problems described above this is a
> very small gain for me.
>
> Until the spec got fixed I don't see a chance to add UKI support for kexec.
So that spec is initially just a generalization of what
systemd-stub/systemd-boot/ukify does. The descrepancies between the
cited specs mostly come from the that generalization. If you want to
enumerate kernels and order them the ".osrel" stuff for example is
necessary, hence the boot loader spec really wants it. If you don't
care about the boot loader spec though and just want to register the
kernel UKI PE directly in BootXXX efi vars for example, then there's
no need to include .osrel. That all said we should certainly make the
two specs align better, and clarify the situation. Suggestions/patches
more than welcome.
Ultimately, I think a spec written as description with a single
implementation in mind (i.e. systemd) is a generally a bad spec. Hence
if kexec in the Linux kernel wants to add support for it, that'd be
great but I'd see that as an opportunity to adjust the spec to the
needs of the Linux kernel in this area, so that it reflects well more
than just one backend implementation.
Hence, seeing the spec as set in stone and as inherently low quality
is the wrong way to see it I am sure. Instead, the goal here is to
adjust the spec to make it work really nicely for *both* systemd and
the kernel.
Lennart
--
Lennart Poettering, Berlin
On Thu Sep 14, 2023 at 12:32 PM EEST, Lennart Poettering wrote:
> On Mi, 13.09.23 16:00, Philipp Rudo ([email protected]) wrote:
>
> > For example there are two definitions for the UKI which contradict each other.
> > The dedicated one [1] you have cited earlier and the one in the BLS for type #2
> > entries [2]. In [1] the .linux and .initrd sections are mandatory and the
> > .osrel and .cmdline sections are optional while in [2] it is the other way
> > round. Which definition should the kernel follow?
> >
> > Furthermore, I absolutely don't understand how the spec should be read. All
> > the spec does is defining some file formats. There is no word about which
> > component in the boot chain is supposed to handle them and what exactly this
> > component is supposed to do with it. But that is crucial if we want to add UKI
> > support for kexec as the kexec systemcall will replace the stub. So we need to
> > know what tasks the stub is supposed to perform. Currently this is only some
> > implementation detail of the systemd-stub [3] that can change any moment and I
> > strongly oppose to base any uapi on it.
> >
> > In the end the only benefit this series brings is to extend the signature
> > checking on the whole UKI except of just the kernel image. Everything else can
> > also be done in user space. Compared to the problems described above this is a
> > very small gain for me.
> >
> > Until the spec got fixed I don't see a chance to add UKI support for kexec.
>
> So that spec is initially just a generalization of what
> systemd-stub/systemd-boot/ukify does. The descrepancies between the
> cited specs mostly come from the that generalization. If you want to
> enumerate kernels and order them the ".osrel" stuff for example is
> necessary, hence the boot loader spec really wants it. If you don't
> care about the boot loader spec though and just want to register the
> kernel UKI PE directly in BootXXX efi vars for example, then there's
> no need to include .osrel. That all said we should certainly make the
> two specs align better, and clarify the situation. Suggestions/patches
> more than welcome.
>
> Ultimately, I think a spec written as description with a single
> implementation in mind (i.e. systemd) is a generally a bad spec. Hence
> if kexec in the Linux kernel wants to add support for it, that'd be
> great but I'd see that as an opportunity to adjust the spec to the
> needs of the Linux kernel in this area, so that it reflects well more
> than just one backend implementation.
>
> Hence, seeing the spec as set in stone and as inherently low quality
> is the wrong way to see it I am sure. Instead, the goal here is to
> adjust the spec to make it work really nicely for *both* systemd and
> the kernel.
Bringing better backing story [1] would also help the spec. Immeditaly
when there's some reflection surface, also the possible faults it the
spec become more apparent. Also this makes spec refinement less boring,
which can be boring and tedious if you write it isolated by yourself or
in a small group :-)
I need to check if I could with some effort extend my current testing
environment for UKI [2]. Need to study this better at some point.
> Lennart
>
> --
> Lennart Poettering, Berlin
[1] https://social.kernel.org/notice/AZklKOsIYBZXDL9Bya
[2] https://github.com/jarkkojs/buildroot-tpmdd/compare/master...linux-6.5.y
BR, JKarkko
On Thu Sep 14, 2023 at 3:26 PM EEST, Jarkko Sakkinen wrote:
> On Thu Sep 14, 2023 at 12:32 PM EEST, Lennart Poettering wrote:
> > On Mi, 13.09.23 16:00, Philipp Rudo ([email protected]) wrote:
> >
> > > For example there are two definitions for the UKI which contradict each other.
> > > The dedicated one [1] you have cited earlier and the one in the BLS for type #2
> > > entries [2]. In [1] the .linux and .initrd sections are mandatory and the
> > > .osrel and .cmdline sections are optional while in [2] it is the other way
> > > round. Which definition should the kernel follow?
> > >
> > > Furthermore, I absolutely don't understand how the spec should be read. All
> > > the spec does is defining some file formats. There is no word about which
> > > component in the boot chain is supposed to handle them and what exactly this
> > > component is supposed to do with it. But that is crucial if we want to add UKI
> > > support for kexec as the kexec systemcall will replace the stub. So we need to
> > > know what tasks the stub is supposed to perform. Currently this is only some
> > > implementation detail of the systemd-stub [3] that can change any moment and I
> > > strongly oppose to base any uapi on it.
> > >
> > > In the end the only benefit this series brings is to extend the signature
> > > checking on the whole UKI except of just the kernel image. Everything else can
> > > also be done in user space. Compared to the problems described above this is a
> > > very small gain for me.
> > >
> > > Until the spec got fixed I don't see a chance to add UKI support for kexec.
> >
> > So that spec is initially just a generalization of what
> > systemd-stub/systemd-boot/ukify does. The descrepancies between the
> > cited specs mostly come from the that generalization. If you want to
> > enumerate kernels and order them the ".osrel" stuff for example is
> > necessary, hence the boot loader spec really wants it. If you don't
> > care about the boot loader spec though and just want to register the
> > kernel UKI PE directly in BootXXX efi vars for example, then there's
> > no need to include .osrel. That all said we should certainly make the
> > two specs align better, and clarify the situation. Suggestions/patches
> > more than welcome.
> >
> > Ultimately, I think a spec written as description with a single
> > implementation in mind (i.e. systemd) is a generally a bad spec. Hence
> > if kexec in the Linux kernel wants to add support for it, that'd be
> > great but I'd see that as an opportunity to adjust the spec to the
> > needs of the Linux kernel in this area, so that it reflects well more
> > than just one backend implementation.
> >
> > Hence, seeing the spec as set in stone and as inherently low quality
> > is the wrong way to see it I am sure. Instead, the goal here is to
> > adjust the spec to make it work really nicely for *both* systemd and
> > the kernel.
>
> Bringing better backing story [1] would also help the spec. Immeditaly
> when there's some reflection surface, also the possible faults it the
> spec become more apparent. Also this makes spec refinement less boring,
> which can be boring and tedious if you write it isolated by yourself or
> in a small group :-)
>
> I need to check if I could with some effort extend my current testing
> environment for UKI [2]. Need to study this better at some point.
>
> > Lennart
> >
> > --
> > Lennart Poettering, Berlin
>
> [1] https://social.kernel.org/notice/AZklKOsIYBZXDL9Bya
> [2] https://github.com/jarkkojs/buildroot-tpmdd/compare/master...linux-6.5.y
>
> BR, JKarkko
I need to revisit one some months olds patch set from Matthew Garrett.
It was about encrypted hibernate.
I don't recall exactly what was the problem with PCR sealing but want
to check. This is not hunch that this would affect the current patch
set in review. Just want to revisit to remember why it did not go
through in the end.
BTW, would not be a bad idea to extend CC list to at least Matthew and
James Bottomley on this patch.
BR, Jarkko
> BTW, would not be a bad idea to extend CC list to at least Matthew and
> James Bottomley on this patch.
Sure. Added Matthew and James in CC
Also, I already made some minor changes. cmdline is now used from the syscall if there is no .cmdline section included in the UKI. find_section now returns the section_header as an ERR_PTR. You can find them in the uki-v3-wip branch at https://github.com/Cydox/linux/commits/uki-v3-wip
On Thu Sep 14, 2023 at 7:11 PM EEST, Jan Hendrik Farr wrote:
> > BTW, would not be a bad idea to extend CC list to at least Matthew and
> > James Bottomley on this patch.
>
> Sure. Added Matthew and James in CC
>
> Also, I already made some minor changes. cmdline is now used from the
> syscall if there is no .cmdline section included in the UKI.
> find_section now returns the section_header as an ERR_PTR. You can
> find them in the uki-v3-wip branch at
> https://github.com/Cydox/linux/commits/uki-v3-wip
Hey, I discussed about IKU at the Linux Linux security module
maintainers monthly meeting and we concluded that it would be nice if
this had a spread to linux-integrity and linux-security-module mailing
lists.
It is x86 feature at this point but obviously that will work as
reference model to other architectures too. So it would be nice
if those mailing lists would be also included to the loop.
I do not have time to check if this is relevant but this I think
the last version seen of encrypted hibernate:
https://lore.kernel.org/linux-integrity/[email protected]/
Just adding it as a reference since I mentioned it earlier.
I'm on holiday for the next week but will look forward to the next
version after I'm back.
BR, Jarkko
Hi Lennart,
On Thu, 14 Sep 2023 11:32:20 +0200
Lennart Poettering <[email protected]> wrote:
> On Mi, 13.09.23 16:00, Philipp Rudo ([email protected]) wrote:
>
> > For example there are two definitions for the UKI which contradict each other.
> > The dedicated one [1] you have cited earlier and the one in the BLS for type #2
> > entries [2]. In [1] the .linux and .initrd sections are mandatory and the
> > .osrel and .cmdline sections are optional while in [2] it is the other way
> > round. Which definition should the kernel follow?
> >
> > Furthermore, I absolutely don't understand how the spec should be read. All
> > the spec does is defining some file formats. There is no word about which
> > component in the boot chain is supposed to handle them and what exactly this
> > component is supposed to do with it. But that is crucial if we want to add UKI
> > support for kexec as the kexec systemcall will replace the stub. So we need to
> > know what tasks the stub is supposed to perform. Currently this is only some
> > implementation detail of the systemd-stub [3] that can change any moment and I
> > strongly oppose to base any uapi on it.
> >
> > In the end the only benefit this series brings is to extend the signature
> > checking on the whole UKI except of just the kernel image. Everything else can
> > also be done in user space. Compared to the problems described above this is a
> > very small gain for me.
> >
> > Until the spec got fixed I don't see a chance to add UKI support for kexec.
>
> So that spec is initially just a generalization of what
> systemd-stub/systemd-boot/ukify does. The descrepancies between the
> cited specs mostly come from the that generalization. If you want to
> enumerate kernels and order them the ".osrel" stuff for example is
> necessary, hence the boot loader spec really wants it. If you don't
> care about the boot loader spec though and just want to register the
> kernel UKI PE directly in BootXXX efi vars for example, then there's
> no need to include .osrel. That all said we should certainly make the
> two specs align better, and clarify the situation. Suggestions/patches
> more than welcome.
Thanks for the explanation. I know that writing a spec isn't easy and
no matter how hard you try it will never be "perfect". That's why I
think it's important to have a proper Introduction at the beginning of
the spec that gives a context to the reader on the problem the spec is
trying to solve, the different use cases/environments considered
while writing the spec, the different components involved and how they
interact with each other, which limitations there are, etc.. The BLS
sort of has it (if you also consider the "Additional discussion") but
for the UKI the introduction basically boils down to "it's a file that
contains stuff", which isn't very helpful. Having such a context not
only makes it easier to understand where such contradictions come from
but also help to prevent problems where people from different
backgrounds interpret the spec differently because they look at it from
their context.
An other thing I don't understand is, why the Extension Images (I
assume describe the "Companion Files" in the systmd-stub man pages) are
a separate spec. With the initrd and cmdline being part of the UKI and
thus fixed you take away a lot of flexibility users have today. These
extensions bring back part of the flexibility which IMHO is needed by
general purpose distros as for them a simple one-size-fits-all solution
doesn't work. That's why for me both belong together and thus should be
described in the same spec.
The extensions are also a good example why you need to properly define
the different components and their responsibility. In a secureboot
environment these extensions need to be signed and verified during
boot. But wich component is responsible to check the signature? Is it
the stub? The kernel? or even the initrd? If you don't define that in
the spec you will eventually end up in situations where nobody checks
the signature because everybody is sure it's "someone else's job".
> Ultimately, I think a spec written as description with a single
> implementation in mind (i.e. systemd) is a generally a bad spec. Hence
> if kexec in the Linux kernel wants to add support for it, that'd be
> great but I'd see that as an opportunity to adjust the spec to the
> needs of the Linux kernel in this area, so that it reflects well more
> than just one backend implementation.
Fully agree. I must admit my first mail sounds pretty negative. But I
don't oppose the UKI. All I wanted to say that at the moment the spec
doesn't work for the kernel. But it can (and should) be fixed.
In this context I hope it is also clear to you that when more and more
people rely on the spec you need a more formal process when including
changes. Especially when the change might break the implementation of
others. So no more making the .cmdline optional and allowing it to be
overwritten all on the same day.
Having that said, what does "local override" exactly mean? Does that
mean a distro can allow a user to freely choose the cmdline without
checking any signatures? I.e. does that mean we can get rid of this
https://github.com/systemd/systemd/issues/24539
> Hence, seeing the spec as set in stone and as inherently low quality
> is the wrong way to see it I am sure. Instead, the goal here is to
> adjust the spec to make it work really nicely for *both* systemd and
> the kernel.
Sorry, I never wanted to intend that the spec inherently low quality.
Just that it doesn't meat my expectations, yet. But that is fine. The
spec isn't even a year old and there's only a single implementation,
yet. So it's more documentation rather than a spec.
Furthermore I don't expect the spec to be ever "set in stone". A spec
always needs to adjust to an ever changing world. If it doesn't it's
dead. But once other people rely on it you mustn't break backward
compatibility. Meaning the more people rely on it the more careful you
have to be which changes you make.
Anyway, I hope I could make clear what my pain points are and by
that help to make it work for both sides.
Thanks
Philipp
> Lennart
>
> --
> Lennart Poettering, Berlin
>
> _______________________________________________
> kexec mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/kexec
>
Hi Jan,
On Wed, 13 Sep 2023 16:42:33 +0200
"Jan Hendrik Farr" <[email protected]> wrote:
> On Wed, Sep 13, 2023, at 4:00 PM, Philipp Rudo wrote:
[...]
> In [5] Luca writes:
> > [...] we fully intend for the UKI format to be an open and stable
> > specification, that anybody can support and rely on.
> But that is unfortunately not where the format is at this point.
>
> What is annoying though is where this leaves a user that actually
> wants this feature. They can carry a patch or they might have to wait
> a long time.
>
> Can you indicate what it would take for the kernel community to consider
> this spec as stable enough?
I don't think there is a good answer to that question. In fact I
believe if you ask 10 people from the community you will get 20+
different answers.
My guess is that either (1) the spec is moved to some official standard
committee where people spend decades to polish it before it makes it
into the kernel or (2) there's a big flamewar on LKML until Linus had
enough and passes his judgment on it. So definitely (2) ;-)
Thanks
Philipp
>
>
> > In the end the only benefit this series brings is to extend the
> > signature checking on the whole UKI except of just the kernel image.
> > Everything else can also be done in user space. Compared to the
> > problems described above this is a very small gain for me.
>
> Correct. That is the benefit of pulling the UKI apart in the
> kernel. However having to sign the kernel inside the UKI defeats
> the whole point.
>
>
> [1] https://uapi-group.org/specifications/specs/unified_kernel_image/
> [2] https://github.com/uapi-group/specifications/pull/72
> [3] https://github.com/uapi-group/specifications/pull/73
> [4] https://github.com/uapi-group/specifications/issues/74
> [5] https://github.com/systemd/systemd/issues/28538
>
On Thu, Sep 14, 2023, at 8:51 PM, Philipp Rudo wrote:
> [...]
>
> In this context I hope it is also clear to you that when more and more
> people rely on the spec you need a more formal process when including
> changes. Especially when the change might break the implementation of
> others. So no more making the .cmdline optional and allowing it to be
> overwritten all on the same day.
>
> Having that said, what does "local override" exactly mean? Does that
> mean a distro can allow a user to freely choose the cmdline without
> checking any signatures?
The behavior of systemd-stub is to allow the bootloader (or whatever
called sd-stub) supplied cmdline when there is no .cmdline section in
the UKI. That's how I understand "local override" here. For WIP v3 of
this patch the behavior is to use the cmdline supplied by userspace to
the kexec_file_load syscall if no .cmdline section is in the UKI.
empty .cmdline section -> empty cmdline always passed to kernel
.cmdline section -> use bootloader/user supplied cmdline (which would
be empty by default)
This setup does not make sense for a locked down / secure system though.
Maybe the word "override" is not ideal. There is nothing actually being
overridden as there is no cmdline in the UKI in the first place.
sd-stub also allows the bootloader supplied cmdline if not using secure
boot. So maybe the kernel could allow user supplied cmdline if not in
lockdown mode for kexec maybe? If not in lockdown mode somebody can just
kexec an unsigned kernel + unsigned cmdline using the kexec_load syscall
anyways. For this case the word "override" makes sense.
The logic for all of this in sd-stub is in [1].
> I.e. does that mean we can get rid of this
> https://github.com/systemd/systemd/issues/24539
This is a different usecase IMO.
>> Hence, seeing the spec as set in stone and as inherently low quality
>> is the wrong way to see it I am sure. Instead, the goal here is to
>> adjust the spec to make it work really nicely for *both* systemd and
>> the kernel.
>
> Sorry, I never wanted to intend that the spec inherently low quality.
> Just that it doesn't meat my expectations, yet. But that is fine. The
> spec isn't even a year old and there's only a single implementation,
> yet. So it's more documentation rather than a spec.
Let's make it happen.
[1] https://github.com/systemd/systemd/blob/5898cef22a35ceefa068d5f46929eced2baab0ed/src/boot/efi/stub.c#L140
Hi Jan,
On Thu, 14 Sep 2023 23:04:32 +0200
"Jan Hendrik Farr" <[email protected]> wrote:
> On Thu, Sep 14, 2023, at 8:51 PM, Philipp Rudo wrote:
> > [...]
> >
> > In this context I hope it is also clear to you that when more and more
> > people rely on the spec you need a more formal process when including
> > changes. Especially when the change might break the implementation of
> > others. So no more making the .cmdline optional and allowing it to be
> > overwritten all on the same day.
> >
> > Having that said, what does "local override" exactly mean? Does that
> > mean a distro can allow a user to freely choose the cmdline without
> > checking any signatures?
>
> The behavior of systemd-stub is to allow the bootloader (or whatever
> called sd-stub) supplied cmdline when there is no .cmdline section in
> the UKI. That's how I understand "local override" here. For WIP v3 of
> this patch the behavior is to use the cmdline supplied by userspace to
> the kexec_file_load syscall if no .cmdline section is in the UKI.
>
> empty .cmdline section -> empty cmdline always passed to kernel
> .cmdline section -> use bootloader/user supplied cmdline (which would
> be empty by default)
>
> This setup does not make sense for a locked down / secure system though.
>
> Maybe the word "override" is not ideal. There is nothing actually being
> overridden as there is no cmdline in the UKI in the first place.
>
> sd-stub also allows the bootloader supplied cmdline if not using secure
> boot. So maybe the kernel could allow user supplied cmdline if not in
> lockdown mode for kexec maybe? If not in lockdown mode somebody can just
> kexec an unsigned kernel + unsigned cmdline using the kexec_load syscall
> anyways. For this case the word "override" makes sense.
>
> The logic for all of this in sd-stub is in [1].
>
> > I.e. does that mean we can get rid of this
> > https://github.com/systemd/systemd/issues/24539
>
> This is a different usecase IMO.
Yeah, I expected that. The whole question was meant to be rhetorical.
The point I wanted to make was that when a spec uses terms like "local
override" it needs to explain what it means.
Thanks
Philipp
> >> Hence, seeing the spec as set in stone and as inherently low quality
> >> is the wrong way to see it I am sure. Instead, the goal here is to
> >> adjust the spec to make it work really nicely for *both* systemd and
> >> the kernel.
> >
> > Sorry, I never wanted to intend that the spec inherently low quality.
> > Just that it doesn't meat my expectations, yet. But that is fine. The
> > spec isn't even a year old and there's only a single implementation,
> > yet. So it's more documentation rather than a spec.
>
> Let's make it happen.
>
>
> [1] https://github.com/systemd/systemd/blob/5898cef22a35ceefa068d5f46929eced2baab0ed/src/boot/efi/stub.c#L140
>
> > In the end the only benefit this series brings is to extend the
> > signature checking on the whole UKI except of just the kernel image.
> > Everything else can also be done in user space. Compared to the
> > problems described above this is a very small gain for me.
>
> Correct. That is the benefit of pulling the UKI apart in the
> kernel. However having to sign the kernel inside the UKI defeats
> the whole point.
Pingfan added the zboot load support in kexec-tools, I know that he is
trying to sign the zboot image and the inside kernel twice. So
probably there are some common areas which can be discussed.
Added Ard and Pingfan in cc.
http://lists.infradead.org/pipermail/kexec/2023-August/027674.html
Thanks
Dave
On Wed, 20 Sept 2023 at 15:43, Dave Young <[email protected]> wrote:
>
> > > In the end the only benefit this series brings is to extend the
> > > signature checking on the whole UKI except of just the kernel image.
> > > Everything else can also be done in user space. Compared to the
> > > problems described above this is a very small gain for me.
> >
> > Correct. That is the benefit of pulling the UKI apart in the
> > kernel. However having to sign the kernel inside the UKI defeats
> > the whole point.
>
>
> Pingfan added the zboot load support in kexec-tools, I know that he is
> trying to sign the zboot image and the inside kernel twice. So
> probably there are some common areas which can be discussed.
> Added Ard and Pingfan in cc.
> http://lists.infradead.org/pipermail/kexec/2023-August/027674.html
>
Here is another thread of the initial try in kernel with a few more
options eg. some fake efi service helpers.
https://lore.kernel.org/linux-arm-kernel/[email protected]/T/#m42abb0ad3c10126b8b3bfae8a596deb707d6f76e
>
> Thanks
> Dave