Re: [ptxdist] [PATCH] kernel: add support for kernel module signing

[Roland Hieber <rhi@pengutronix.de>]

On Fri, Jul 23, 2021 at 12:39:28PM +0200, Michael Olbrich wrote:
> On Fri, Jul 23, 2021 at 12:17:36PM +0200, Roland Hieber wrote:
> > On Wed, Jul 21, 2021 at 10:54:53AM +0200, Michael Olbrich wrote:
> > > On Mon, Jul 19, 2021 at 08:30:53PM +0200, Roland Hieber wrote:
> > > > diff --git a/platforms/kernel.in b/platforms/kernel.in
> > > > index 68899c0f7dcc..d7bff2656fd9 100644
> > > > --- a/platforms/kernel.in
> > > > +++ b/platforms/kernel.in
> > > > @@ -3,6 +3,7 @@
> > > >  menuconfig KERNEL
> > > >  	bool
> > > >  	default y
> > > > +	select CODE_SIGNING		if KERNEL_MODULES_SIGN
> > > >  	select HOST_U_BOOT_TOOLS	if KERNEL_IMAGE_U || (KERNEL_IMAGE_SIMPLE && ARCH_MICROBLAZE)
> > > >  	select HOST_ZSTD		if KERNEL_ZSTD
> > > >  	select HOST_XZ			if KERNEL_XZ
> > > > @@ -38,6 +39,22 @@ config KERNEL_MODULES_INSTALL
> > > >  	prompt "Install modules into /lib/modules"
> > > >  	depends on KERNEL_MODULES
> > > >  
> > > > +config KERNEL_MODULES_SIGN
> > > > +	bool
> > > > +	depends on KERNEL_MODULES
> > > > +	select KERNEL_MODULES_INSTALL
> > > > +	select KERNEL_OPENSSL
> > > > +	prompt "sign modules"
> > > > +	help
> > > > +	  If enabled, kernel modules are signed during the install stage with
> > > > +	  the key specified by the code signing provider in the "kernel-module"
> > > > +	  role. Additionally, the CA specified in the "kernel-module" role is
> > > > +	  added to the kernel's trust root.
> > > > +
> > > > +	  See the section "Kernel module signing" in the "Daily Work" chapter in
> > > > +	  the PTXdist manual for use cases and more infos about what needs to be
> > > > +	  enabled in the kernel config file.
> > > > +
> > > >  config KERNEL_VERSION
> > > >  	prompt "kernel version"
> > > >  	string
> > > > diff --git a/rules/kernel.make b/rules/kernel.make
> > > > index f43c1bb8de89..750a68efc6fa 100644
> > > > --- a/rules/kernel.make
> > > > +++ b/rules/kernel.make
> > > > @@ -53,18 +53,24 @@ endef
> > > >  # check for old kernel modules rules
> > > >  KERNEL_MAKEVARS = $(call kernel/deprecated, KERNEL_MAKEVARS)
> > > >  
> > > > +KERNEL_SIGN_OPT	= \
> > > > +	CONFIG_MODULE_SIG_KEY='"$(shell cs_get_uri kernel-modules)"' \
> > > > +	$(if $(shell cs_get_ca kernel-trusted), \
> > > > +		CONFIG_SYSTEM_TRUSTED_KEYS=$(shell cs_get_ca kernel-trusted))
> > > 
> > > ... and here 'kernel-trusted' is used to import the CA.
> > > 
> > > I think the documentation should be changed. From what I understand,
> > > trusted keys are useful for more than just module signature verification.
> > 
> > Yes.
> > 
> > > And I think this should only be used if PTXCONF_KERNEL_MODULES_SIGN is
> > > enabled.
> > 
> > Hmm. I would add the 'kernel-trusted' lines regardless of whether module
> > singing is enabled; it already has a check anyway and is only added if
> > there are any CAs, and as you said it is useful for other things besides
> > module signing.
> 
> Right, of course.

OK, I put it behind a new KERNEL_CODE_SIGNING option now, we should make
sure that the cs_* functions are only used if they are actually
available… :)

> 
> > > > +
> > > >  # like kernel-opts but with different CROSS_COMPILE=
> > > >  KERNEL_BASE_OPT		:= \
> > > >  	$(call kernel-opts, KERNEL,$(KERNEL_CROSS_COMPILE)) \
> > > >  	$(call remove_quotes,$(PTXCONF_KERNEL_EXTRA_MAKEVARS))
> > > >  
> > > >  # Intermediate option. This will be used by kernel module packages.
> > > > -KERNEL_MODULE_OPT	:= \
> > > > +KERNEL_MODULE_OPT	= \
> > > >  	-C $(KERNEL_DIR) \
> > > >  	O=$(KERNEL_BUILD_DIR) \
> > > > +	$(KERNEL_SIGN_OPT) \
> > > 
> > > So we have the variable KERNEL_MODULE_OPT (separate from KERNEL_SHARED_OPT)
> > > as something that is also used by out-of-tree kernel modules.
> > > 
> > > I expect, that we don't need CONFIG_SYSTEM_TRUSTED_KEYS here. But we do
> > > need CONFIG_MODULE_SIG_KEY to sign those modules, right?
> > 
> > Yes, I think so too.
> > 
> > > I don't mind adding both here for simplicity, but I want to make sure that
> > > they should be added here at all and not just to KERNEL_SHARED_OPT.
> > 
> > I think it's best to add module signing and add trusted keys in two
> > separate patches, put CONFIG_SYSTEM_TRUSTED_KEYS into KERNEL_BASE_OPT
> > (with its if condition), and only have CONFIG_MODULE_SIG_KEY in
> > KERNEL_SIGN_OPT (behind a ifdef PTXCONF_KERNEL_MODULES_SIGN).
> 
> Hmm, I've been thinking about this some more. I think both should go in
> KERNEL_BASE_OPT. Where setting CONFIG_* options here, overwriting stuff
> from .config. And we should keep that consistent across all steps.

OK, seems fine to me. This now makes most options use = instead of :=
because the include KERNEL_BASE_OPT, but I've left the _IIO_OPT and
_PERF_OPT variables as := as they don't need code signing (I think…)

 - Roland

> But definitely conditional with an option each.
> 
> Michael
> 
> > > >  	$(KERNEL_BASE_OPT)
> > > >  
> > > > -KERNEL_SHARED_OPT	:= \
> > > > +KERNEL_SHARED_OPT	= \
> > > >  	$(KERNEL_MODULE_OPT)
> > > >  
> > > >  ifndef PTXCONF_KERNEL_GCC_PLUGINS
> > > > @@ -166,6 +172,7 @@ $(STATEDIR)/kernel.tags:
> > > >  
> > > >  KERNEL_MAKE_OPT		= \
> > > >  	$(call kernel/deprecated, KERNEL_MAKE_OPT) \
> > > > +	$(KERNEL_SIGN_OPT) \
> > > >  	$(KERNEL_SHARED_OPT) \
> > > >  	$(KERNEL_IMAGE) \
> > > >  	$(call ptx/ifdef, PTXCONF_KERNEL_MODULES,modules)
> > > > @@ -231,7 +238,8 @@ endif
> > > >  # Install
> > > >  # ----------------------------------------------------------------------------
> > > >  
> > > > -KERNEL_INSTALL_OPT := \
> > > > +KERNEL_INSTALL_OPT = \
> > > > +	$(KERNEL_SIGN_OPT) \
> > > >  	$(KERNEL_BASE_OPT) \
> > > >  	modules_install
> > > >  
> > > > -- 
> > > > 2.30.2

-- 
Roland Hieber, Pengutronix e.K.          | r.hieber@pengutronix.de     |
Steuerwalder Str. 21                     | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686         | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de