[ptxdist] [PATCH] kernel: do not strip signed modules

[ptxdist] [PATCH] kernel: do not strip signed modules

[Denis Osterland-Heim]

[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]



Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht
Nürnberg: HRB 32315

________________________________

Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht.
Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.

- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:

https://www.diehl.com/group/de/transparenz-und-informationspflichten/

The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

- For general information on data protection and your respective rights please visit:

https://www.diehl.com/group/en/transparency-and-information-obligations/



[-- Attachment #2: 1617084308.Vfd01Idfd786M787564.mbox --]
[-- Type: application/mbox, Size: 2475 bytes --]

[-- Attachment #3: Type: text/plain, Size: 181 bytes --]

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

Re: [ptxdist] [PATCH] kernel: do not strip signed modules

[Michael Olbrich]

On Tue, Mar 30, 2021 at 06:08:10AM +0000, Denis Osterland-Heim wrote:
> If CONFIG_MODULE_SIG_ALL is set in kernelconfig then modules will be
> automatically signed during the modules_install phase of a kernel build.
> 
> Signed modules are BRITTLE as the signature is outside of the defined ELF
> container. Thus they MAY NOT be stripped once the signature is computed
> and attached. Note the entire module is the signed payload, including any
> and all debug information present at the time of signing.

Hmm, we had the same  issue at some point. The solution was a local copy of
the shell code that does the stripping and installing. I think we added
some code to sign the Modules again.
The result was nice, but the whole thing was rather invasive and makes
assumptions on how the module signing works internally in the kernel.
So it was not something that I wanted to merge mainline that way.

In general, I like this approach better. But there are two issues with it.

1. There are redundant options and if the uses gets it wrong then it wont
fail at build time. We can get this from the kernelconfig:
'$(shell ptxd_get_kconfig $(KERNEL_CONFIG) CONFIG_MODULE_SIG_ALL)' I think.
But we have to make sure that it's not evaluated too early. To avoid
slowing down ptxdist startup.

2. The modules are not stripped at all. So we should set
INSTALL_MOD_STRIP=1 in this case.

Marc, what do you think?

Michael

> See: https://www.kernel.org/doc/html/latest/admin-guide/module-signing.html
> Signed-off-by: Denis Osterland-Heim <denis.osterland@diehl.com>
> ---
>  platforms/kernel.in | 7 +++++++
>  rules/kernel.make   | 3 ++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/platforms/kernel.in b/platforms/kernel.in
> index 68899c0f7..8b9473b03 100644
> --- a/platforms/kernel.in
> +++ b/platforms/kernel.in
> @@ -32,6 +32,13 @@ config KERNEL_MODULES
>  	default y
>  	prompt "build kernel-modules"
>  
> +config KERNEL_MODULES_SIGNED
> +	bool "kernel-modules are signed on install"
> +	depends on KERNEL_MODULES
> +	help
> +	  Set this to y if CONFIG_MODULE_SIG_ALL is y in kernelconfig.
> +	  Otherwise the strip would damage automatically generated signature.
> +
>  config KERNEL_MODULES_INSTALL
>  	bool
>  	default y
> diff --git a/rules/kernel.make b/rules/kernel.make
> index ea748fc8a..c964bd672 100644
> --- a/rules/kernel.make
> +++ b/rules/kernel.make
> @@ -313,7 +313,8 @@ ifdef PTXCONF_KERNEL_MODULES_INSTALL
>  	@$(call install_fixup, kernel-modules, AUTHOR,"Robert Schwebel <r.schwebel@pengutronix.de>")
>  	@$(call install_fixup, kernel-modules, DESCRIPTION,missing)
>  
> -	@$(call install_glob, kernel-modules, 0, 0, -, /lib/modules, *.ko,, k)
> +	@$(call install_glob, kernel-modules, 0, 0, -, /lib/modules, *.ko,, \
> +		$(call ptx/ifdef,PTXCONF_KERNEL_MODULES_SIGNED,n,k))
>  	@$(call install_glob, kernel-modules, 0, 0, -, /lib/modules,, *.ko */build */source, n)
>  
>  	@$(call install_finish, kernel-modules)
> -- 
> 2.31.1
> 

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


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

Re: [ptxdist] [PATCH] kernel: do not strip signed modules

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 3281 bytes --]

On 30.03.2021 09:53:46, Michael Olbrich wrote:
> On Tue, Mar 30, 2021 at 06:08:10AM +0000, Denis Osterland-Heim wrote:
> > If CONFIG_MODULE_SIG_ALL is set in kernelconfig then modules will be
> > automatically signed during the modules_install phase of a kernel build.
> > 
> > Signed modules are BRITTLE as the signature is outside of the defined ELF
> > container. Thus they MAY NOT be stripped once the signature is computed
> > and attached. Note the entire module is the signed payload, including any
> > and all debug information present at the time of signing.
> 
> Hmm, we had the same  issue at some point. The solution was a local copy of
> the shell code that does the stripping and installing. I think we added
> some code to sign the Modules again.
> The result was nice, but the whole thing was rather invasive and makes
> assumptions on how the module signing works internally in the kernel.
> So it was not something that I wanted to merge mainline that way.
> 
> In general, I like this approach better. But there are two issues with it.
> 
> 1. There are redundant options and if the uses gets it wrong then it wont
> fail at build time. We can get this from the kernelconfig:
> '$(shell ptxd_get_kconfig $(KERNEL_CONFIG) CONFIG_MODULE_SIG_ALL)' I think.
> But we have to make sure that it's not evaluated too early. To avoid
> slowing down ptxdist startup.
> 
> 2. The modules are not stripped at all. So we should set
> INSTALL_MOD_STRIP=1 in this case.
> 
> Marc, what do you think?

The current diff for module re-signing looks like this:

|  # $1: file to strip
|  #
|  # $strip: k for kernel modules
| @@ -425,8 +15,26 @@ ptxd_install_file_strip() {
|      esac
|  
|      files=( "${sdirs[@]/%/${dst}}" )
|      install -d "${files[0]%/*}" &&
|      "${strip_cmd[@]}" -o "${files[0]}" "${src}" &&
| +    if [ "${strip}" = "k" -a -e "${pkg_build_dir}/scripts/sign-file" -a -n "${CONFIG_MODULE_SIG_KEY}" ]; then
| +       local sig_hash
| +
| +       sig_hash="$(ptxd_get_kconfig "${pkg_config}" CONFIG_MODULE_SIG_HASH)" || return
| +
| +       echo "sign module:"
| +       echo "  file=$(ptxd_print_path ${files[0]})"
| +       echo "  key=${CONFIG_MODULE_SIG_KEY}"
| +       echo "  cert=$(ptxd_print_path ${pkg_build_dir}/certs/signing_key.x509)"
| +       echo "  hash=${sig_hash}"
| +       echo
| +       "${pkg_build_dir}/scripts/sign-file" \
| +           "${sig_hash}" \
| +           "${CONFIG_MODULE_SIG_KEY}" \
| +           "${pkg_build_dir}/certs/signing_key.x509" \
| +           "${files[0]}" || return
| +    fi
| +
|      for (( i=1 ; ${i} < ${#files[@]} ; i=$[i+1] )); do
|         install -m "${mod_rw}" -D "${files[0]}" "${files[${i}]}" || return
|      done &&
| @@ -443,658 +51,3 @@ ptxd_install_file_strip() {
|      fi
|  }

If we don't want to re-sign, we can use the above "if [ "${strip}" = "k"
-a ... ]" and don't strip. This means no changes in the kernel.make
files are needed.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 181 bytes --]

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

Re: [ptxdist] [PATCH] kernel: do not strip signed modules

[Michael Olbrich]

On Tue, Mar 30, 2021 at 10:22:40AM +0200, Marc Kleine-Budde wrote:
> On 30.03.2021 09:53:46, Michael Olbrich wrote:
> > On Tue, Mar 30, 2021 at 06:08:10AM +0000, Denis Osterland-Heim wrote:
> > > If CONFIG_MODULE_SIG_ALL is set in kernelconfig then modules will be
> > > automatically signed during the modules_install phase of a kernel build.
> > > 
> > > Signed modules are BRITTLE as the signature is outside of the defined ELF
> > > container. Thus they MAY NOT be stripped once the signature is computed
> > > and attached. Note the entire module is the signed payload, including any
> > > and all debug information present at the time of signing.
> > 
> > Hmm, we had the same  issue at some point. The solution was a local copy of
> > the shell code that does the stripping and installing. I think we added
> > some code to sign the Modules again.
> > The result was nice, but the whole thing was rather invasive and makes
> > assumptions on how the module signing works internally in the kernel.
> > So it was not something that I wanted to merge mainline that way.
> > 
> > In general, I like this approach better. But there are two issues with it.
> > 
> > 1. There are redundant options and if the uses gets it wrong then it wont
> > fail at build time. We can get this from the kernelconfig:
> > '$(shell ptxd_get_kconfig $(KERNEL_CONFIG) CONFIG_MODULE_SIG_ALL)' I think.
> > But we have to make sure that it's not evaluated too early. To avoid
> > slowing down ptxdist startup.
> > 
> > 2. The modules are not stripped at all. So we should set
> > INSTALL_MOD_STRIP=1 in this case.
> > 
> > Marc, what do you think?
> 
> The current diff for module re-signing looks like this:
> 
> |  # $1: file to strip
> |  #
> |  # $strip: k for kernel modules
> | @@ -425,8 +15,26 @@ ptxd_install_file_strip() {
> |      esac
> |  
> |      files=( "${sdirs[@]/%/${dst}}" )
> |      install -d "${files[0]%/*}" &&
> |      "${strip_cmd[@]}" -o "${files[0]}" "${src}" &&
> | +    if [ "${strip}" = "k" -a -e "${pkg_build_dir}/scripts/sign-file" -a -n "${CONFIG_MODULE_SIG_KEY}" ]; then
> | +       local sig_hash
> | +
> | +       sig_hash="$(ptxd_get_kconfig "${pkg_config}" CONFIG_MODULE_SIG_HASH)" || return
> | +
> | +       echo "sign module:"
> | +       echo "  file=$(ptxd_print_path ${files[0]})"
> | +       echo "  key=${CONFIG_MODULE_SIG_KEY}"
> | +       echo "  cert=$(ptxd_print_path ${pkg_build_dir}/certs/signing_key.x509)"
> | +       echo "  hash=${sig_hash}"
> | +       echo
> | +       "${pkg_build_dir}/scripts/sign-file" \
> | +           "${sig_hash}" \
> | +           "${CONFIG_MODULE_SIG_KEY}" \
> | +           "${pkg_build_dir}/certs/signing_key.x509" \
> | +           "${files[0]}" || return
> | +    fi
> | +
> |      for (( i=1 ; ${i} < ${#files[@]} ; i=$[i+1] )); do
> |         install -m "${mod_rw}" -D "${files[0]}" "${files[${i}]}" || return
> |      done &&
> | @@ -443,658 +51,3 @@ ptxd_install_file_strip() {
> |      fi
> |  }
> 
> If we don't want to re-sign, we can use the above "if [ "${strip}" = "k"
> -a ... ]" and don't strip. This means no changes in the kernel.make
> files are needed.

I'm pretty sure, that we modified something to get CONFIG_MODULE_SIG_KEY
defined in this context.
And depending on scripts/sign-file is definitely not something I want to do
here.

Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://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

Re: [ptxdist] [PATCH] kernel: do not strip signed modules

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 4048 bytes --]

On 30.03.2021 10:39:41, Michael Olbrich wrote:
> On Tue, Mar 30, 2021 at 10:22:40AM +0200, Marc Kleine-Budde wrote:
> > On 30.03.2021 09:53:46, Michael Olbrich wrote:
> > > On Tue, Mar 30, 2021 at 06:08:10AM +0000, Denis Osterland-Heim wrote:
> > > > If CONFIG_MODULE_SIG_ALL is set in kernelconfig then modules will be
> > > > automatically signed during the modules_install phase of a kernel build.
> > > > 
> > > > Signed modules are BRITTLE as the signature is outside of the defined ELF
> > > > container. Thus they MAY NOT be stripped once the signature is computed
> > > > and attached. Note the entire module is the signed payload, including any
> > > > and all debug information present at the time of signing.
> > > 
> > > Hmm, we had the same  issue at some point. The solution was a local copy of
> > > the shell code that does the stripping and installing. I think we added
> > > some code to sign the Modules again.
> > > The result was nice, but the whole thing was rather invasive and makes
> > > assumptions on how the module signing works internally in the kernel.
> > > So it was not something that I wanted to merge mainline that way.
> > > 
> > > In general, I like this approach better. But there are two issues with it.
> > > 
> > > 1. There are redundant options and if the uses gets it wrong then it wont
> > > fail at build time. We can get this from the kernelconfig:
> > > '$(shell ptxd_get_kconfig $(KERNEL_CONFIG) CONFIG_MODULE_SIG_ALL)' I think.
> > > But we have to make sure that it's not evaluated too early. To avoid
> > > slowing down ptxdist startup.
> > > 
> > > 2. The modules are not stripped at all. So we should set
> > > INSTALL_MOD_STRIP=1 in this case.
> > > 
> > > Marc, what do you think?
> > 
> > The current diff for module re-signing looks like this:
> > 
> > |  # $1: file to strip
> > |  #
> > |  # $strip: k for kernel modules
> > | @@ -425,8 +15,26 @@ ptxd_install_file_strip() {
> > |      esac
> > |  
> > |      files=( "${sdirs[@]/%/${dst}}" )
> > |      install -d "${files[0]%/*}" &&
> > |      "${strip_cmd[@]}" -o "${files[0]}" "${src}" &&
> > | +    if [ "${strip}" = "k" -a -e "${pkg_build_dir}/scripts/sign-file" -a -n "${CONFIG_MODULE_SIG_KEY}" ]; then
> > | +       local sig_hash
> > | +
> > | +       sig_hash="$(ptxd_get_kconfig "${pkg_config}" CONFIG_MODULE_SIG_HASH)" || return
> > | +
> > | +       echo "sign module:"
> > | +       echo "  file=$(ptxd_print_path ${files[0]})"
> > | +       echo "  key=${CONFIG_MODULE_SIG_KEY}"
> > | +       echo "  cert=$(ptxd_print_path ${pkg_build_dir}/certs/signing_key.x509)"
> > | +       echo "  hash=${sig_hash}"
> > | +       echo
> > | +       "${pkg_build_dir}/scripts/sign-file" \
> > | +           "${sig_hash}" \
> > | +           "${CONFIG_MODULE_SIG_KEY}" \
> > | +           "${pkg_build_dir}/certs/signing_key.x509" \
> > | +           "${files[0]}" || return
> > | +    fi
> > | +
> > |      for (( i=1 ; ${i} < ${#files[@]} ; i=$[i+1] )); do
> > |         install -m "${mod_rw}" -D "${files[0]}" "${files[${i}]}" || return
> > |      done &&
> > | @@ -443,658 +51,3 @@ ptxd_install_file_strip() {
> > |      fi
> > |  }
> > 
> > If we don't want to re-sign, we can use the above "if [ "${strip}" = "k"
> > -a ... ]" and don't strip. This means no changes in the kernel.make
> > files are needed.
> 
> I'm pretty sure, that we modified something to get CONFIG_MODULE_SIG_KEY
> defined in this context.

KERNEL_CONF_ENV = \
        $(CODE_SIGNING_ENV) \
        CONFIG_MODULE_SIG_KEY='$(shell cs_get_uri evm)'
        
> And depending on scripts/sign-file is definitely not something I want to do
> here.

We have to call scripts/sign-file to sign modules :)

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 181 bytes --]

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

Re: [ptxdist] [PATCH] kernel: do not strip signed modules

[Denis Osterland-Heim]

Hi,

Thanks for the input.

>$(mod_strip_cmd) $(2)/$(notdir $@) ; \
>$(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) ; \
from scripts/Makefile.modinst

As far as I got it, with INSTALL_MOD_STRIP="--strip-debug -R .GCC.command.line" in KERNEL_MAKE_OPT
and always strip=n in targetinstall it should work identically with signed modules and without.

What do you and Marc think?

Regards, Denis

Am Dienstag, den 30.03.2021, 09:53 +0200 schrieb Michael Olbrich:
> On Tue, Mar 30, 2021 at 06:08:10AM +0000, Denis Osterland-Heim wrote:
> > If CONFIG_MODULE_SIG_ALL is set in kernelconfig then modules will be
> > automatically signed during the modules_install phase of a kernel build.
> >
> > Signed modules are BRITTLE as the signature is outside of the defined ELF
> > container. Thus they MAY NOT be stripped once the signature is computed
> > and attached. Note the entire module is the signed payload, including any
> > and all debug information present at the time of signing.
>
> Hmm, we had the same  issue at some point. The solution was a local copy of
> the shell code that does the stripping and installing. I think we added
> some code to sign the Modules again.
> The result was nice, but the whole thing was rather invasive and makes
> assumptions on how the module signing works internally in the kernel.
> So it was not something that I wanted to merge mainline that way.
>
> In general, I like this approach better. But there are two issues with it.
>
> 1. There are redundant options and if the uses gets it wrong then it wont
> fail at build time. We can get this from the kernelconfig:
> '$(shell ptxd_get_kconfig $(KERNEL_CONFIG) CONFIG_MODULE_SIG_ALL)' I think.
> But we have to make sure that it's not evaluated too early. To avoid
> slowing down ptxdist startup.
>
> 2. The modules are not stripped at all. So we should set
> INSTALL_MOD_STRIP=1 in this case.
>
> Marc, what do you think?
>
> Michael
>
> > See: https://www.kernel.org/doc/html/latest/admin-guide/module-signing.html
> > Signed-off-by: Denis Osterland-Heim <denis.osterland@diehl.com>
> > ---
> >  platforms/kernel.in | 7 +++++++
> >  rules/kernel.make   | 3 ++-
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/platforms/kernel.in b/platforms/kernel.in
> > index 68899c0f7..8b9473b03 100644
> > --- a/platforms/kernel.in
> > +++ b/platforms/kernel.in
> > @@ -32,6 +32,13 @@ config KERNEL_MODULES
> >  default y
> >  prompt "build kernel-modules"
> >
> > +config KERNEL_MODULES_SIGNED
> > +bool "kernel-modules are signed on install"
> > +depends on KERNEL_MODULES
> > +help
> > +  Set this to y if CONFIG_MODULE_SIG_ALL is y in kernelconfig.
> > +  Otherwise the strip would damage automatically generated signature.
> > +
> >  config KERNEL_MODULES_INSTALL
> >  bool
> >  default y
> > diff --git a/rules/kernel.make b/rules/kernel.make
> > index ea748fc8a..c964bd672 100644
> > --- a/rules/kernel.make
> > +++ b/rules/kernel.make
> > @@ -313,7 +313,8 @@ ifdef PTXCONF_KERNEL_MODULES_INSTALL
> >  @$(call install_fixup, kernel-modules, AUTHOR,"Robert Schwebel <r.schwebel@pengutronix.de>")
> >  @$(call install_fixup, kernel-modules, DESCRIPTION,missing)
> >
> > -@$(call install_glob, kernel-modules, 0, 0, -, /lib/modules, *.ko,, k)
> > +@$(call install_glob, kernel-modules, 0, 0, -, /lib/modules, *.ko,, \
> > +$(call ptx/ifdef,PTXCONF_KERNEL_MODULES_SIGNED,n,k))
> >  @$(call install_glob, kernel-modules, 0, 0, -, /lib/modules,, *.ko */build */source, n)
> >
> >  @$(call install_finish, kernel-modules)
> > --
> > 2.31.1
> >
> > _______________________________________________
> > ptxdist mailing list
> > ptxdist@pengutronix.de
> > To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de
Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht
Nürnberg: HRB 32315

________________________________

Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht.
Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.

- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:

https://www.diehl.com/group/de/transparenz-und-informationspflichten/

The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

- For general information on data protection and your respective rights please visit:

https://www.diehl.com/group/en/transparency-and-information-obligations/


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

Re: [ptxdist] [PATCH] kernel: do not strip signed modules

[m.olbrich]

On Tue, Mar 30, 2021 at 09:53:25AM +0000, Denis Osterland-Heim wrote:
> Hi,
> 
> Thanks for the input.
> 
> >$(mod_strip_cmd) $(2)/$(notdir $@) ; \
> >$(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) ; \
> from scripts/Makefile.modinst
> 
> As far as I got it, with INSTALL_MOD_STRIP="--strip-debug -R .GCC.command.line" in KERNEL_MAKE_OPT
> and always strip=n in targetinstall it should work identically with signed modules and without.

We should add TARGET_COMPILER_RECORD_SWITCHES to the kernel blacklist. Then
.GCC.command.line won't show up at all. I've seen the kernel complain about
it anyways, but I keep forgetting it.

At that point only '--strip-debug' is necessary and that's the default, so
INSTALL_MOD_STRIP=1 works too.

And about always stripping: I think we should do this. In the past, we hat
a separate debug nfsroot directory but that's long gone. And for kernel
modules we don't actually keep a separate debug file. So we don't actually
keep a unstripped version. So stripping early is fine. I think, the
unstripped version remains in the build tree, so if someone needs the debug
symbols, they can look there.

> What do you and Marc think?

I'm not a kernel developer, so Marc, does that make sense to you?

Michael

> Am Dienstag, den 30.03.2021, 09:53 +0200 schrieb Michael Olbrich:
> > On Tue, Mar 30, 2021 at 06:08:10AM +0000, Denis Osterland-Heim wrote:
> > > If CONFIG_MODULE_SIG_ALL is set in kernelconfig then modules will be
> > > automatically signed during the modules_install phase of a kernel build.
> > >
> > > Signed modules are BRITTLE as the signature is outside of the defined ELF
> > > container. Thus they MAY NOT be stripped once the signature is computed
> > > and attached. Note the entire module is the signed payload, including any
> > > and all debug information present at the time of signing.
> >
> > Hmm, we had the same  issue at some point. The solution was a local copy of
> > the shell code that does the stripping and installing. I think we added
> > some code to sign the Modules again.
> > The result was nice, but the whole thing was rather invasive and makes
> > assumptions on how the module signing works internally in the kernel.
> > So it was not something that I wanted to merge mainline that way.
> >
> > In general, I like this approach better. But there are two issues with it.
> >
> > 1. There are redundant options and if the uses gets it wrong then it wont
> > fail at build time. We can get this from the kernelconfig:
> > '$(shell ptxd_get_kconfig $(KERNEL_CONFIG) CONFIG_MODULE_SIG_ALL)' I think.
> > But we have to make sure that it's not evaluated too early. To avoid
> > slowing down ptxdist startup.
> >
> > 2. The modules are not stripped at all. So we should set
> > INSTALL_MOD_STRIP=1 in this case.
> >
> > Marc, what do you think?
> >
> > Michael
> >
> > > See: https://www.kernel.org/doc/html/latest/admin-guide/module-signing.html
> > > Signed-off-by: Denis Osterland-Heim <denis.osterland@diehl.com>
> > > ---
> > >  platforms/kernel.in | 7 +++++++
> > >  rules/kernel.make   | 3 ++-
> > >  2 files changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/platforms/kernel.in b/platforms/kernel.in
> > > index 68899c0f7..8b9473b03 100644
> > > --- a/platforms/kernel.in
> > > +++ b/platforms/kernel.in
> > > @@ -32,6 +32,13 @@ config KERNEL_MODULES
> > >  default y
> > >  prompt "build kernel-modules"
> > >
> > > +config KERNEL_MODULES_SIGNED
> > > +bool "kernel-modules are signed on install"
> > > +depends on KERNEL_MODULES
> > > +help
> > > +  Set this to y if CONFIG_MODULE_SIG_ALL is y in kernelconfig.
> > > +  Otherwise the strip would damage automatically generated signature.
> > > +
> > >  config KERNEL_MODULES_INSTALL
> > >  bool
> > >  default y
> > > diff --git a/rules/kernel.make b/rules/kernel.make
> > > index ea748fc8a..c964bd672 100644
> > > --- a/rules/kernel.make
> > > +++ b/rules/kernel.make
> > > @@ -313,7 +313,8 @@ ifdef PTXCONF_KERNEL_MODULES_INSTALL
> > >  @$(call install_fixup, kernel-modules, AUTHOR,"Robert Schwebel <r.schwebel@pengutronix.de>")
> > >  @$(call install_fixup, kernel-modules, DESCRIPTION,missing)
> > >
> > > -@$(call install_glob, kernel-modules, 0, 0, -, /lib/modules, *.ko,, k)
> > > +@$(call install_glob, kernel-modules, 0, 0, -, /lib/modules, *.ko,, \
> > > +$(call ptx/ifdef,PTXCONF_KERNEL_MODULES_SIGNED,n,k))
> > >  @$(call install_glob, kernel-modules, 0, 0, -, /lib/modules,, *.ko */build */source, n)
> > >
> > >  @$(call install_finish, kernel-modules)
> > > --
> > > 2.31.1
> > >
> > > _______________________________________________
> > > ptxdist mailing list
> > > ptxdist@pengutronix.de
> > > To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de
> Diehl Connectivity Solutions GmbH
> Geschäftsführung: Horst Leonberger
> Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht
> Nürnberg: HRB 32315
> 
> ________________________________
> 
> Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
> Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht.
> Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.
> 
> - Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:
> 
> https://www.diehl.com/group/de/transparenz-und-informationspflichten/
> 
> The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
> mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.
> 
> - For general information on data protection and your respective rights please visit:
> 
> https://www.diehl.com/group/en/transparency-and-information-obligations/
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://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