[OS-BUILD PATCHv5 0/2] redhat/kernel.spec: add uki_addons to create UKI kernel cmdline addons

[Emanuele Giuseppe Esposito (via Email Bridge)]

From: Emanuele Giuseppe Esposito on gitlab.com
Merge Request: https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917

We want to enable kernel.spec to optionally ship UKI addons defined in a
common config file in redhat folder.
The folder redhat/addons will contain all addons configs specifying the
UKI kernel cmdline addons to be created in the next build. An addon
config is simply a .addon plain text file, where any line
is taken as kernel cmdline, except for the ones starting with '#',
which will be automatically ignored.

redhat/scripts/uki_addons.py will take care of parsing all configs and
folders in redhat/addons and call 'ukify' to create the actual addons.
The output addon filename will be a concatenation of all folders in
redhat/addons that are part of the addon config path.

For example, an addon config redhat/addons/1/2/3/test.addon will result
in generating redhat/addons_output/1-2-3-test.addon.efi.

It is also possible to add .sbat to all the generated addons, by
populating redhat/addons/sbat/sbat.conf. Syntax is same as the addons
config.

At build time, Makefile will create a tar.gz archive (addons.tar.gz)
containing all the files in redhat/addons. It will then passed to the
kernel specfile that will extract the addons from it and generate the
UKI kernel cmdline addons.

As an example of this feature, add the fips addon to optionally enable fips
(https://issues.redhat.com/browse/RHEL-23049).

---
 redhat/addons/sbat/.gitkeep  |    0 
 redhat/addons/fips.addon     |    2 +
 redhat/scripts/uki_addons.py |  124 +++++++++++++++++++++++++++++++++++++++++++
 redhat/Makefile              |    3 +
 redhat/kernel.spec.template  |   18 ++++++
 5 files changed, 147 insertions(+), 0 deletions(-)

[OS-BUILD PATCHv5 1/2] redhat/kernel.spec: add uki_addons to create UKI kernel cmdline addons

[Emanuele Giuseppe Esposito (via Email Bridge)]

From: Emanuele Giuseppe Esposito

redhat/kernel.spec: add uki_addons to create UKI kernel cmdline addons

Upstream Status: RHEL-Only

The folder redhat/addons will contain all addons configs specifying the
UKI kernel cmdline addons to be created in the next build. An addon
config is simply a .addon plain text file, where any line
is taken as kernel cmdline, except for the ones starting with '#',
which will be automatically ignored.

redhat/scripts/uki_addons.py will take care of parsing all configs and
folders in redhat/addons and call 'ukify' to create the actual addons.
The output addon filename will be a concatenation of all folders in
redhat/addons that are part of the addon config path.

For example, an addon config redhat/addons/1/2/3/test.addon will result
in generating 1-2-3-test.addon.efi.

It is also possible to add .sbat to all the generated addons, by
populating redhat/addons/sbat/sbat.conf. Syntax is same as the addons
config.

At build time, Makefile will create a tar.gz archive (addons.tar.gz)
containing all the files in redhat/addons. It will then passed to the
kernel specfile that will extract the addons from it and generate the
UKI kernel cmdline addons.

Signed-off-by: Emanuele Giuseppe Esposito

diff --git a/redhat/Makefile b/redhat/Makefile
index blahblah..blahblah 100644
--- a/redhat/Makefile
+++ b/redhat/Makefile
@@ -679,6 +679,7 @@ sources-rh: $(TARBALL) generate-testpatch-tmp setup-source dist-configs-check
    @sed -e "s/%%SPECKVERSION%%/$(SPECKVERSION)/" \
    	-e "s/%%SPECKPATCHLEVEL%%/$(SPECKPATCHLEVEL)/" \
    	scripts/gating/rpminspect.yaml > $(SOURCES)/rpminspect.yaml
+	@tar -czf addons.tar.gz addons
    @cp scripts/kernel-tools/kvm_stat.logrotate \
    	keys/rhel*.x509 \
    	kabi/check-kabi \
@@ -690,6 +691,7 @@ sources-rh: $(TARBALL) generate-testpatch-tmp setup-source dist-configs-check
    	scripts/mod/mod-partner.list \
    	scripts/mod/mod-sign.sh \
    	scripts/mod/mod-kvm.list \
+		scripts/uki_addons.py \
    	configs/flavors \
    	configs/generate_all_configs.sh \
    	configs/merge.py \
@@ -698,6 +700,7 @@ sources-rh: $(TARBALL) generate-testpatch-tmp setup-source dist-configs-check
    	README.rst \
    	kernel-local \
    	dracut-virt.conf \
+		addons.tar.gz \
    	$(SOURCES)/
    @cat $$(ls -1 $(SPECPACKAGE_NAME).changelog-* | sort -t '.' -k 3 -n -r) \
    	> $(SOURCES)/kernel.changelog
diff --git a/redhat/addons/sbat/.gitkeep b/redhat/addons/sbat/.gitkeep
new file mode 100644
index blahblah..blahblah 100644
--- /dev/null
+++ b/redhat/addons/sbat/.gitkeep
diff --git a/redhat/kernel.spec.template b/redhat/kernel.spec.template
index blahblah..blahblah 100644
--- a/redhat/kernel.spec.template
+++ b/redhat/kernel.spec.template
@@ -792,6 +792,8 @@ BuildRequires: binutils
 BuildRequires: lvm2
 BuildRequires: systemd-boot-unsigned
 # For systemd-stub and systemd-pcrphase
+BuildRequires: systemd-ukify
+# For UKI kernel cmdline addons
 BuildRequires: systemd-udev >= 252-1
 # For TPM operations in UKI initramfs
 BuildRequires: tpm2-tools
@@ -933,6 +935,9 @@ Source86: dracut-virt.conf

Source87: flavors

+Source151: uki_addons.py
+Source152: addons.tar.gz
+
 Source100: rheldup3.x509
 Source101: rhelkpatch1.x509

@@ -2537,6 +2542,18 @@ BuildKernel() {
     	fi
     	mv $KernelUnifiedImage.signed $KernelUnifiedImage

+      tar -xvf %{SOURCE152} -C $KernelUnifiedImageDir
+      KernelAddonsDir="$KernelUnifiedImageDir/addons"
+      KernelAddonsDirOut="$RPM_BUILD_ROOT/usr/lib/linux/extra.d"
+      mkdir -p $KernelAddonsDirOut
+      python3 %{SOURCE151} $KernelAddonsDir $KernelAddonsDirOut
+      for addon in "$KernelAddonsDirOut"/*; do
+        %pesign -s -i $addon -o $addon.signed -a %{secureboot_ca_1} -c %{secureboot_key_1} -n %{pesign_name_1}
+        rm -f $addon
+        mv $addon.signed $addon
+      done
+      rm -rf $KernelAddonsDir
+
 # signkernel
 %endif

@@ -3692,6 +3709,7 @@ fi\
 /lib/modules/%{KVERREL}%{?3:+%{3}}/config\
 /lib/modules/%{KVERREL}%{?3:+%{3}}/modules.builtin*\
 %attr(0644, root, root) /lib/modules/%{KVERREL}%{?3:+%{3}}/%{?-k:%{-k*}}%{!?-k:vmlinuz}-virt.efi\
+/usr/lib/linux/extra.d/*.addon.efi\
 %ghost /%{image_install_path}/efi/EFI/Linux/%{?-k:%{-k*}}%{!?-k:*}-%{KVERREL}%{?3:+%{3}}.efi\
 %endif\
 %endif\
diff --git a/redhat/scripts/uki_addons.py b/redhat/scripts/uki_addons.py
new file mode 100644
index blahblah..blahblah 100644
--- /dev/null
+++ b/redhat/scripts/uki_addons.py
@@ -0,0 +1,124 @@
+#!/usr/bin/env python3
+#
+# This script recursively inspects a given uki directory, and creates an addon
+# for each file found.
+#
+# Usage: python uki_addons.py input_dir out_dir
+#
+# This tool requires the systemd-ukify and systemd-boot yum packages.
+#
+# Addon file
+#-----------
+# Each addon terminates with .addon
+# Each addon contains only two types of lines:
+# Lines beginning with '#' are description and thus ignored
+# All other lines are command line to be added.
+# The name of the end resulting addon is taken from the folder hierarchy.
+# For example, and addon in input_dir/test/test2/hello.addon will result in
+# an UKI addon file generated in out_dir called hello-test-test2.addon.efi
+#
+# sbat.conf
+#----------
+# This file is containing the sbat string for *all* addons being created.
+# This file is optional, but when used has to be put in out_dir/sbat/sbat.conf.
+# It follows the same syntax as the addon files, meaning '#' is comment and
+# the rest is taken as sbat string and feed to ukify.
+
+import os
+import sys
+import collections
+import subprocess
+
+SYSTEMD_STUB_PATH = '/usr/lib/systemd/boot/efi/addonx64.efi.stub'
+UKIFY_PATH = '/usr/lib/systemd/ukify'
+SBAT_LOCATION = '/sbat/sbat.conf'
+
+def usage(err):
+    print(f'Usage: {os.path.basename(__file__)} input_dir output_dir')
+    print(f'Error:{err}')
+    sys.exit(1)
+
+def check_clean_arguments(in_dir, out_dir):
+    # Remove end '/'
+    if in_dir[-1:] == '/':
+        in_dir = in_dir[:-1]
+    if out_dir[-1:] == '/':
+        out_dir = out_dir[:-1]
+    if not os.path.isdir(in_dir):
+        usage(f'in_dir {in_dir} is not a dir, or does not exist!')
+    if not os.path.isdir(out_dir):
+        usage(f'out_dir_dir {out_dir} is not a dir, or does not exist!')
+    return in_dir, out_dir
+
+UKICmdlineAddon = collections.namedtuple('UKICmdlineAddon', ['name', 'cmdline'])
+uki_addon_list = []
+
+def parse_addon_conf(path, rstrip=True):
+    with open(path, 'r') as addons:
+        lines = addons.readlines()
+    cmdline = ''
+    for l in lines:
+        l = l.lstrip()
+        if not l:
+            continue
+        if l[0] == '#':
+            continue
+        if rstrip:
+            l = l.rstrip() + ' '
+        cmdline += l
+    if cmdline == '':
+        return ''
+    return cmdline
+
+def parse_in_dir(in_dir):
+    for (dirpath, _, filenames) in os.walk(in_dir):
+        # remove in_dir from path, don't use it for the addon name
+        addon_name = dirpath.replace(in_dir, "", 1)
+        # remove the '/' added by walk after in_dir
+        addon_name = addon_name.replace("/", "",1)
+        # transform remaining '/' in '-'
+        addon_name = addon_name.replace("/", "-")
+        for addon_f in filenames:
+            if addon_f.endswith('.addon'):
+                addon_full_name = addon_name
+                if addon_full_name != '':
+                    addon_full_name += '-'
+                addon_full_name += addon_f + '.efi'
+                cmdline = parse_addon_conf(dirpath+'/'+addon_f)
+                if cmdline:
+                    uki_addon_list.append(UKICmdlineAddon(addon_full_name, cmdline))
+
+def parse_sbat(in_dir):
+    sbat_loc = in_dir + SBAT_LOCATION
+    if not os.path.isfile(sbat_loc):
+        return
+    return parse_addon_conf(sbat_loc, rstrip=False)
+
+def main(in_dir, out_dir):
+    sbat_string = parse_sbat(in_dir)
+    for uki_addon in uki_addon_list:
+        out_path = out_dir + '/' + uki_addon.name
+        cmd = [
+            f'{UKIFY_PATH}', 'build',
+            f'--stub={SYSTEMD_STUB_PATH}',
+            f'--cmdline="{uki_addon.cmdline}"',
+            f'--output={out_path}']
+        if sbat_string:
+            cmd.append('--sbat="' + sbat_string.rstrip() +'"')
+
+        print(cmd)
+        subprocess.check_call(cmd, text=True)
+
+if __name__ == "__main__":
+    argc = len(sys.argv) - 1
+    if argc != 2:
+        usage('too few or too many parameters!')
+
+    in_dir = sys.argv[1]
+    out_dir = sys.argv[2]
+
+    in_dir, out_dir = check_clean_arguments(in_dir, out_dir)
+    parse_in_dir(in_dir)
+    main(in_dir, out_dir)
+
+

--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917

[OS-BUILD PATCHv5 2/2] redhat/uki_cmdline_addons.conf: add FIPS addon

[Emanuele Giuseppe Esposito (via Email Bridge)]

From: Emanuele Giuseppe Esposito

redhat/uki_cmdline_addons.conf: add FIPS addon

Upstream Status: RHEL-Only

The fips addon simply enable fips in the kernel command line.

Signed-off-by: Emanuele Giuseppe Esposito

diff --git a/redhat/addons/fips.addon b/redhat/addons/fips.addon
new file mode 100644
index blahblah..blahblah 100644
--- /dev/null
+++ b/redhat/addons/fips.addon
@@ -0,0 +1,2 @@
+# Enable fips in RHEL (https://issues.redhat.com/browse/RHEL-23049)
+fips=1

--
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917

[Emanuele Giuseppe Esposito (via Email Bridge)]

From: Emanuele Giuseppe Esposito on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1776187...

I would like some opinion here. The place where I store the addons rn is
wrong. According to the spec, they will need to go in either

/usr/lib/linux/extra.d/ for global addons
    /usr/lib/linux/<uname>/<UKI_NAME>.efi.extra.d/ for local addons

The addons generated in kernel.spec are global or local addons?
- If they are global (meaning used by all UKIs), then the MR is fine as it is.
- If it's local addons (only used by the UKI built), then kernel-install needs
to be extended, because the UKI name is the machine ID, and it's provided to
kernel-install via some env variable that I don't think can control in the
kernel.specfile. So in that case this MR would need to be waiting that I
extend kernel-install first.

[pmendezh (via Email Bridge)]

From: pmendezh on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1776264...

Maybe remove the reference to `yum` here?

[Philipp Rudo (via Email Bridge)]

From: Philipp Rudo on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1789653...

What spec are you referring to? I couldn't find the two paths you are
mentioning neither in the uki spec nor any of the related systemd man pages.

[Philipp Rudo (via Email Bridge)]

From: Philipp Rudo on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1789653...

Hi Emanuele,

your implementation is different to what I originally suggested. But I must
admit that your implementation is better. With my suggestion it would have
been hard to get a reasonable name for the final addon.

What I don't understand is why you are building the exact same set of addons
for every kernel without adding a .uname section. The way I see it we either
ship one set of addons for every arch/variant. But then it would be better to
have a separate noarch rpm so we don't need to rebuild it over and over again.
Or we want to be able to ship arch/variant-specific addons. But then you need
to set the .uname section and a way to tell uki_addon.py to build addons only
for certain archs/variants. Personally I would again simply copy what is done
for the kernel configs and re-use the directory structure there (if we choose
to go this way).

[Emanuele Giuseppe Esposito (via Email Bridge)]

From: Emanuele Giuseppe Esposito on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1801106...

You're right, I thought it was merged but it's still there:
https://github.com/uapi-group/specifications/pull/91
Anyways I don't think this will change anytime soon.

[Emanuele Giuseppe Esposito (via Email Bridge)]

From: Emanuele Giuseppe Esposito on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1801117...

Hi Philipp,

Regarding why we are doing this here and not in a separate rpm, @vkuznets
might have a more detailed answer but the main point is that it would be
easier to do it here, because addons are very lightweight and honestly we
don't think we will ship many of them. In addition creating a rpm could be a
more complicated process.

Regarding kernel configs, you mean redhat/build_configs.sh? I can definitely
look at it. I see your point about arch specific, alternatively we could
simply use some RPM %arch flag or something like that to just follow and
compile addons for the arch folder.

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1801301...

I don't really see the benefit is going via a tarball for addons. Right now it
only contains a single file,  and even down the road the number of addons
doesn't seem likely to exceed single digits. Just listing each addon as a
`SourceNNNN` entry is simpler and clearer to review IMHO. Just pick a distinct
number range for addons, as has been done for the many other types of kernel
`SourceNNN` entries.

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1801314...

This is packaging non-versioned (ie global) addons, in the versioned sub-RPM
for the virt UKI. IMHO this is undesirable for a few reasons

* It should be possible to install multiple versions of the UKI RPM in
parallel, but since the addon filenames clash, parallel installs are no longer
possible after this change.
* Global add-ons are not specific to a kernel version. The same addon should
be usable with past/current/future UKI versions until we have reason to revoke
the addon with SBAT
* Global add-ons are not specific to the `virt` UKI. If other UKIs are created
in future, they should be able to use the same addons.

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1801354...
> Global add-ons are not specific to the virt UKI. If other UKIs are created

in future, they should be able to use the same addons.

That said, the signing key we're using was assigned specific for the the virt
UKI. So if we have separate UKIs in future with different signing keys,
they'll likely need separate addons.

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1801355...
> Regarding why we are doing this here and not in a separate rpm, @vkuznets

might have a more detailed answer but the main point is that it would be
easier to do it here, because addons are very lightweight and honestly we
don't think we will ship many of them. In addition creating a rpm could be a
more complicated process.

It seems unlikely that we will have kernel version specific addons. The use
cases discussed thus far all look like global addons. As such I think it is
preferable to treat these as global addons until someone comes along with a
clear use case for a version specific addon.

If we treat them as global addons, then at the very least they need to be in a
separate sub-RPM, so we can parallel install multiple UKI RPMs without the
addons clashing.

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1801367...

Consider if in future we have multiple UKIs created, each with their own
SecureBoot signing key. We might need to have the same addon for different
UKIs.

One option is that we have multiple signatures on the same addon binary. The
other option is to namespace things, eg 'virt-fips.addon.efi' and 'something-
fips.add.efi' for a "something" UKI, each signed with their respective
SecureBoot keys.

[Philipp Rudo (via Email Bridge)]

From: Philipp Rudo on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1803294...
> Regarding why we are doing this here and not in a separate rpm, @vkuznets

might have a more detailed answer but the main point is that it would be
easier to do it here, because addons are very lightweight and honestly we
don't think we will ship many of them. In addition creating a rpm could be a
more complicated process.

I agree, that handling them here is easier. But I doubt that there will only
be a few addons. There are a ton of debug parameters that at some point in
time will be needed. Plus there are bugs that can be worked around by "setting
the right parameter". Furthermore, we need the ability to disable memory
hungry parameters for the kdump kernel. All in all my expectation is that the
number will increase rapidly once UKIs get used more.
> Regarding kernel configs, you mean redhat/build_configs.sh?

Yes. Or just the directory structure in general.
> It seems unlikely that we will have kernel version specific addons. The use

cases discussed thus far all look like global addons. As such I think it is
preferable to treat these as global addons until someone comes along with a
clear use case for a version specific addon.

Not sure if that is a valid scenario for CVMs, but one use case is when you
have multiple UKIs installed that require different `crashkernel=` parameters.
Then you need to be able to install different addons for each UKI. A similar
scenario is when one of the UKIs has a bug that can be worked around by
"setting the right parameter" that is otherwise beneficial. In that case you
want to have version specific addons as well.

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1803375...
> I agree, that handling them here is easier. But I doubt that there will only

be a few addons. There are a ton of debug parameters that at some point in
time will be needed. Plus there are bugs that can be worked around by "setting
the right parameter". Furthermore, we need the ability to disable memory
hungry parameters for the kdump kernel. All in all my expectation is that the
number will increase rapidly once UKIs get used more.

We need to be very careful about creating signed addons for debug parameters.
The goal of UKIs is to close the huge SecureBoot security hole in existing
systems where neither the initrd nor cmdline were covered by the SecureBoot
signatures.

With the UKIs now the cmdline is signed, we have a fully trustworthy kernel
environment booted. There are parameters for the kernel that could potentially
undermine the security of the system such that we do NOT want to sign addons
with distro keys.

For adhoc debugging options, I think the expectation is that users would be
able to create and sign addons themselves, using a MOK key they have locally
enrolled with shim, rather than having the distro vendor (Red Hat) signing
them.

[Emanuele Giuseppe Esposito (via Email Bridge)]

From: Emanuele Giuseppe Esposito on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1803387...

Or we might ship unsigned debug addons directly?

[Emanuele Giuseppe Esposito (via Email Bridge)]

From: Emanuele Giuseppe Esposito on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1803395...

I mean being ready to scale without having to create a lot of SourceNNN does
not seem bad IMHO.
Also we need to figure whether to ship unsigned addons too. I think it could
be a good idea, so that the user doesn't have to (at least for the generic
case).

[Emanuele Giuseppe Esposito (via Email Bridge)]

From: Emanuele Giuseppe Esposito on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805489...

Ok so to make a summary, this could be a possible solution. We want to create
sub-rpms so that:
* `kernel-uki-virt-$uname.$distro.$arch` has the UKI. This is already
implemented.
Now, suppose we have an UKI `$UKI`. In the concrete example, it will be
`virt`. We would have the following sub-rpms:
* `kernel-addons-$UKI-common-$uname.$distro.$arch` has the $UKI global addons
signed by $UKI key.
* `kernel-addons-$UKI-$uname.$distro.$arch` has the $UKI local addons signed
by $UKI key. But since at the moment we don't care about local addons, let's
leave this as TODO.
* `kernel-addons-$UKI-debug-$uname.$distro.$arch` has the unsigned debug
addons. Or alternatively `kernel-addons-$UKI-unsigned-$uname.$distro.$arch`.
Or this can be shared by all UKIs, we can discuss about this later. Also this
as TODO.

Therefore we are left with $UKI global addons.

We have a config-like structure, which could be
`redhat/addons/$distro/$UKI/global or local/$arch/addon_name.conf` (file
content will stay the same as it is now, line starting with # are comment and
the rest is command line). `redhat/addons/$distro/$UKI` will be packed as
SourceNNN. When processing the $UKI package, kernel.spec fetches
SourceNNN/global/$arch. My script takes the files in there and creates
`addon_name-global-$UKI.$distro.$arch.addon.efi` to be packed in `kernel-
addons-$UKI-common-$uname.$distro.$arch` that will install it in
`/usr/lib/linux/`

Example: `virt` UKI and `something` UKI. Resulting theoretical sub-rpms:
* kernel-uki-virt
* kernel-addons-virt-common
* kernel-addons-virt
* kernel-uki-virt-debug
* kernel-something-virt
* kernel-addons-something-common
* kernel-addons-something
* kernel-uki-something-debug

Resulting actual sub-rpms after this MR:
* kernel-uki-virt
* kernel-addons-virt-common
* kernel-something-virt
* kernel-addons-something-common

And concretely now we only have the `virt` UKI.

Does this sound correct? @berrange @vkuznets @prudo1

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805563...
> kernel-uki-virt
> kernel-addons-virt-common
> kernel-addons-virt
> kernel-uki-virt-debug

My only thought is around naming to be a common name prefix:

* kernel-uki-virt
* kernel-uki-virt-addons-common
* kernel-uki-virt-addons
* kernel-uki-virt-debug

[Vitaly Kuznetsov (via Email Bridge)]

From: Vitaly Kuznetsov on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805606...

@eesposit @berrange personally, I'd suggest we don't over-complicate stuff
**now**. Namely, we have only one UKI and only one signing SB key for it
(which, in case of Fedora, is the same key as for the standard kernel). We
don't know yet whether we will be building more UKIs and if yes, whether these
UKIs will use different SB keys. Hopefully not, as to get distinct PCR7
fingerprints we will have to list all of them in SecureBoot DB. We can, of
course, start by creating "kernel-uki-virt-addons-common" but in the absence
of "kernel-uki-virt-addons" the "common" suffix is going to be a bit weird.
Note, creating "-common" sub-package still doesn't give us a way to create
kernel-version-independent addons, we will have to introduce a new srpm for
that.

Addons are, however, tiny, this means that if we have 3 copies of them when we
have three UKIs installed, it is not a big deal. What is more important, is if
we get an automated way to update them on the ESP with kernel updates. I.e. if
I have an active addon for my UKI and I'm installing new UKI version (both
kernel-uki-virt and kernel-uki-virt-addons), will the addon get updated on the
ESP. In case the addon is global, this means the the cmdline gets updated for
the already present UKIs is this is likely desirable.

At the end, it may make sense to make all addons tied to the specific UKI
version and teach kernel-install how to update the ESP when a new UKI version
is installed.

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805628...
> Note, creating "-common" sub-package still doesn't give us a way to create

kernel-version-independent addons, we will have to introduce a new srpm for
that.

No we don't. It merely means that the `kernel-uki-virt-addons-common` package
will be re-built on every kernel update, even if there were no addon changes.
The `-common` package from version `x.y.z` will be usable with UKI
`x.y.(z-1)`, and vica-versa.

The reason for the split is to cope with the different install scenarios. For
the global addons (in `-common`) a host will only ever need 1 copy of the RPM
installed, and the versin of the `-common` package installed need not match
the version of the UKI currently booted.  For the per-version addons, a host
will need one or more copies of the RPM installed - one copy matching each
`kernel-uki-virt` that's installed.

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805632...

That said, have a separate srpm is not an inherently bad idea. For that matter
there's a potential case to be made that the UKI itself could also be in a
separate srpm, since if you have a systemd-stub CVE you might want to rebuild
& ship the UKI without rebuilding the whole kernel package.  Having the UKI in
the kernel srpm is just a convenience.

[Vitaly Kuznetsov (via Email Bridge)]

From: Vitaly Kuznetsov on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805638...

What I'm afraid of with "-common" addons is that at some point we will realize
that the particular addon is not really "-common" and we want to update it
only for new ukis but not for the already existing ones. I can't probably make
a real world example but I can imagine something changing in the kernel in an
incompatible way so the new addon won't work for the old kernel and vice
versa. To avoid the need to make a judgement whether an addon is **really**
common or not, we can just slash them for now.

[Daniel P. Berrangé (via Email Bridge)]

From: Daniel P. Berrangé on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805650...

One could ask what benefit "global" addons actually provide ?  Addons are
tiny, so the saving from only having 1 copy is lost in the noise. Possibly the
only real advantage of "global" addons is precisely when the addon is being
provided by a separate SRPM, so that SRPM doesn't need rebuilding on every
kernel update.  IOW, by building the addons in the same package as the UKI,
there's zero benefit of creating a global addon.

[Vitaly Kuznetsov (via Email Bridge)]

From: Vitaly Kuznetsov on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805661...

Exactly: we may want to have an infrastructure for 'external' addons and these
should likely be global, but everything built from kernel srpm can just be
versioned.

[Philipp Rudo (via Email Bridge)]

From: Philipp Rudo on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805835...

@berrange I find not having any debug addon way too restrictive. We do need
ways to debug problems that will occur and we really should avoid asking users
to install and maintain their own MOKs. Not only is managing MOKs way less
user friendly but it also leads to much bigger security problems if a MOK
falls into the wrong hands. Much worse than any "wrong" addon could cause.
Especially as every addon is measured in the TPM so users do have a way to
verify that only the allowed set of addons were loaded.

[Philipp Rudo (via Email Bridge)]

From: Philipp Rudo on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1805849...

Thanks for the pointer

[Vitaly Kuznetsov (via Email Bridge)]

From: Vitaly Kuznetsov on gitlab.com
https://gitlab.com/cki-project/kernel-ark/-/merge_requests/2917#note_1807239...

I think it would make sense to make it possible for an addon to target
'standard' UKI / 'debug' UKI / both in the infrastructure which Emanuele is
trying to create. Personally, I would not be overthinking security issues of
debug addons just **now** and wait until we get a request for an addon which
we will consider 'unsafe'. As command line addons are currently rigid, i.e.
it's impossible to have something like 'root=*', I don't think we have that
many kernel options which require denylisting. For the extreme cases with very
strong security requirements, adding other PCRs (e.g. PCR4) to the sealing
policy might be a good option.