Message-ID: <ea1a678bdbfe5c492aa494d93476bb18e6dfec5e.camel@debian.org>
Date:   Mon, 20 Nov 2023 19:43:17 +0000
From:   Luca Boccassi <bluca@...ian.org>
To:     Luis Chamberlain <mcgrof@...nel.org>,
        Alessandro Carminati <alessandro.carminati@...il.com>
Cc:     linux-modules@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jonathan Corbet <corbet@....net>, linux-doc@...r.kernel.org,
        linux-security-module@...r.kernel.org, jan.setjeeilers@...cle.com,
        linux-efi@...r.kernel.org, ardb@...nel.org
Subject: Re: [RFC PATCH 1/2] Modules: Introduce boot-time module signature
 flexibility

On Fri, 2023-11-17 at 10:33 -0800, Luis Chamberlain wrote:
> On Fri, Nov 17, 2023 at 02:56:53PM +0100, Alessandro Carminati wrote:
> > Il giorno gio 16 nov 2023 alle ore 18:35 Luis Chamberlain
> > <mcgrof@...nel.org> ha scritto:
> > > 
> > > I see the code which skips module signature verification and the knobs
> > > but I don't see the code which complete the promise to do the actual
> > > signature verification post initrd / initramfs state. What gives?
> > 
> > My initial intention wasn't centered around providing an automated solution.
> 
> It is not even an automated solution, it's *any* solution. So to be
> clear your patch simply disables module verification, it has no
> solution.
> 
> > Instead, I envisioned a design where users could manually restore module
> > verification during a specific point in their init scripts.
> > 
> > It might be plausible to restore module verification when the rootfs is
> > remounted. However, this seems limiting rather than advantageous.
> 
> The patch as-is describes a lofty world and does nothing other than
> disables module verification. If a patch disables module verification
> it should just do that and describe that. Nothing else. The subject
> of the patch tends to suggest some flexibility it provided but does
> nothing of being flexible, it outright disables module signature
> verification. The commit log and the patch subject description are
> describing something completely different than what the code actually
> does, and it gives me to the concern, to the point that if you didn't
> have a few commit logs in the kernel I would have thought your intent
> was test kernel developers with some AI type of code that does something
> stupid and very carefully crafted commit log.
> 
> Nacked-by: Luis Chamberlain <mcgrof@...nel.org>
> 
>   Luis

Hi,

On top of that, this change would go counter to the security posture
that is established by the Lockdown LSM, especially when UEFI Secure
Boot is used. While it would be great if initrds were only ever read-
only, signed, and verified, the reality is that on all general purpose
distributions in use today the initrd is writable by root, and the
kernel command line is also writable by root.

E.g.: on Debian, one just has to edit /etc/default/grub, run a command
and at the next reboot arbitrary kernel command line parameters will be
applied, including this one, and combined with adding an arbitrary
kernel module to the appropriate path this becomes, de-facto, a uid0-
to-ring0 privilege escalation avenue, which is something the secure
boot + shim + lockdown workflow tries very hard to avoid.

While there might be room for such features (e.g.:
https://docs.kernel.org/admin-guide/LSM/LoadPin.html ) in very specific
and defined environment or use cases, at the very least there needs to
be:

- the mechanism to lock it down again very early at boot as indicated
in the cover letter, which is missing as pointed out by Luis
- a kconfig governing the availability of the new kernel cmdline
option, disabled by default
- ideally a check or hook into the Lockdown LSM, so that it can stop
the new kernel command line option, if enabled at build time and if
specified, from actually working when secure boot is enabled, or at the
very, very minimum taint it

Then users wanting to use such a performance optimization will be able
to do so in their own special purpose builds, without negatively
affecting the security story of generalist distributions.

-- 
Kind regards,
Luca Boccassi

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists