| Message ID | 20191118153917.691813-1-seth.forshee@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,E/Unstable] UBUNTU: [Packaging] Fix module signing with older modinfo | expand |
Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
On 18.11.19 16:39, Seth Forshee wrote: > BugLink: https://bugs.launchpad.net/bugs/1852581 > > Not all versions of modinfo support the signer field; > specifically, the version in boinic does not. This leaves all > modules unsigned in hwe kernels based on eoan and later. Change > the check to look for the magic string at the end of the module, > which does not rely on any external tools being aware of module > signatures. > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > debian/rules.d/2-binary-arch.mk | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > index 050f867060cb..070478e010f7 100644 > --- a/debian/rules.d/2-binary-arch.mk > +++ b/debian/rules.d/2-binary-arch.mk > @@ -413,12 +413,12 @@ ifneq ($(skipdbg),true) > -name '*.ko' | while read path_module ; do \ > module="/lib/modules/$${path_module#*/lib/modules/}"; \ > if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \ > - signer=$$(/sbin/modinfo -F signer "$$path_module"); \ > + signature=$$(tail -c 28 "$$path_module"); \ > $(CROSS_COMPILE)objcopy \ > --add-gnu-debuglink=$(dbgpkgdir)/usr/lib/debug/$$module \ > $$path_module; \ > if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config && \ > - [ -n "$$signer" ]; then \ > + [ "$$signature" = "~Module signature appended~" ]; then \ > $(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \ > $(MODSECKEY) \ > $(MODPUBKEY) \ >
On 18.11.19 16:39, Seth Forshee wrote: > BugLink: https://bugs.launchpad.net/bugs/1852581 > > Not all versions of modinfo support the signer field; > specifically, the version in boinic does not. This leaves all > modules unsigned in hwe kernels based on eoan and later. Change > the check to look for the magic string at the end of the module, > which does not rely on any external tools being aware of module > signatures. > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > --- > debian/rules.d/2-binary-arch.mk | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > index 050f867060cb..070478e010f7 100644 > --- a/debian/rules.d/2-binary-arch.mk > +++ b/debian/rules.d/2-binary-arch.mk > @@ -413,12 +413,12 @@ ifneq ($(skipdbg),true) > -name '*.ko' | while read path_module ; do \ > module="/lib/modules/$${path_module#*/lib/modules/}"; \ > if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \ > - signer=$$(/sbin/modinfo -F signer "$$path_module"); \ > + signature=$$(tail -c 28 "$$path_module"); \ > $(CROSS_COMPILE)objcopy \ > --add-gnu-debuglink=$(dbgpkgdir)/usr/lib/debug/$$module \ > $$path_module; \ > if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config && \ > - [ -n "$$signer" ]; then \ > + [ "$$signature" = "~Module signature appended~" ]; then \ > $(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \ > $(MODSECKEY) \ > $(MODPUBKEY) \ > Applied to eoan/master-next branch. Thanks, Kleber
On 18.11.19 16:39, Seth Forshee wrote: > BugLink: https://bugs.launchpad.net/bugs/1852581 > > Not all versions of modinfo support the signer field; > specifically, the version in boinic does not. This leaves all > modules unsigned in hwe kernels based on eoan and later. Change > the check to look for the magic string at the end of the module, > which does not rely on any external tools being aware of module > signatures. > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > --- > debian/rules.d/2-binary-arch.mk | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > index 050f867060cb..070478e010f7 100644 > --- a/debian/rules.d/2-binary-arch.mk > +++ b/debian/rules.d/2-binary-arch.mk > @@ -413,12 +413,12 @@ ifneq ($(skipdbg),true) > -name '*.ko' | while read path_module ; do \ > module="/lib/modules/$${path_module#*/lib/modules/}"; \ > if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \ > - signer=$$(/sbin/modinfo -F signer "$$path_module"); \ > + signature=$$(tail -c 28 "$$path_module"); \ Hi Seth, With this patch, we get the following warnings when building the virtual box modules during the package build: [...] signing vboxguest.ko II: dkms-build override dkms-build--virtualbox-guest found, executing II: dkms-build build virtualbox-guest complete # Add .gnu_debuglink sections to each stripped .ko # pointing to unstripped verson find /<<PKGBUILDDIR>>/debian/linux-modules-5.3.0-23-generic \ /<<PKGBUILDDIR>>/debian/linux-modules-extra-5.3.0-23-generic \ -name '*.ko' | while read path_module ; do \ module="/lib/modules/${path_module#*/lib/modules/}"; \ if [[ -f "/<<PKGBUILDDIR>>/debian/linux-image-unsigned-5.3.0-23-generic-dbgsym/usr/lib/debug /$module" ]] ; then \ signature=$(tail -c 28 "$path_module"); \ objcopy \ --add-gnu-debuglink=/<<PKGBUILDDIR>>/debian/linux-image-unsigned-5.3.0-23-ge neric-dbgsym/usr/lib/debug/$module \ $path_module; \ if grep -q CONFIG_MODULE_SIG=y /<<PKGBUILDDIR>>/debian/build/build-generic/.config & & \ [ "$signature" = "~Module signature appended~" ]; then \ /<<PKGBUILDDIR>>/debian/build/build-generic/scripts/sign-file sha512 \ /<<PKGBUILDDIR>>/debian/build/build-generic/certs/signing_key.pem \ /<<PKGBUILDDIR>>/debian/build/build-generic/certs/signing_key.x509 \ $path_module; \ fi; \ else \ echo "WARNING: Missing debug symbols for module '$module'."; \ fi; \ done WARNING: Missing debug symbols for module '/lib/modules/5.3.0-23-generic/kernel/virtualbox-guest/vbo xsf.ko'. WARNING: Missing debug symbols for module '/lib/modules/5.3.0-23-generic/kernel/virtualbox-guest/vbo xguest.ko'. /bin/bash: line 5: warning: command substitution: ignored null byte in input /bin/bash: line 5: warning: command substitution: ignored null byte in input /bin/bash: line 5: warning: command substitution: ignored null byte in input [...] For the first one I'm not sure there's anything we can do, but the second one repeats hundreds of times and I suspect it's cause by the return of the 'tail' command. Kleber > $(CROSS_COMPILE)objcopy \ > --add-gnu-debuglink=$(dbgpkgdir)/usr/lib/debug/$$module \ > $$path_module; \ > if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config && \ > - [ -n "$$signer" ]; then \ > + [ "$$signature" = "~Module signature appended~" ]; then \ > $(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \ > $(MODSECKEY) \ > $(MODPUBKEY) \ >
On Fri, Nov 22, 2019 at 11:36:45AM +0100, Kleber Souza wrote: > On 18.11.19 16:39, Seth Forshee wrote: > > BugLink: https://bugs.launchpad.net/bugs/1852581 > > > > Not all versions of modinfo support the signer field; > > specifically, the version in boinic does not. This leaves all > > modules unsigned in hwe kernels based on eoan and later. Change > > the check to look for the magic string at the end of the module, > > which does not rely on any external tools being aware of module > > signatures. > > > > Signed-off-by: Seth Forshee <seth.forshee@canonical.com> > > --- > > debian/rules.d/2-binary-arch.mk | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk > > index 050f867060cb..070478e010f7 100644 > > --- a/debian/rules.d/2-binary-arch.mk > > +++ b/debian/rules.d/2-binary-arch.mk > > @@ -413,12 +413,12 @@ ifneq ($(skipdbg),true) > > -name '*.ko' | while read path_module ; do \ > > module="/lib/modules/$${path_module#*/lib/modules/}"; \ > > if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \ > > - signer=$$(/sbin/modinfo -F signer "$$path_module"); \ > > + signature=$$(tail -c 28 "$$path_module"); \ > > Hi Seth, > > With this patch, we get the following warnings when building the virtual box > modules during the package build: > > [...] > signing vboxguest.ko > II: dkms-build override dkms-build--virtualbox-guest found, executing > II: dkms-build build virtualbox-guest complete > # Add .gnu_debuglink sections to each stripped .ko > # pointing to unstripped verson > find /<<PKGBUILDDIR>>/debian/linux-modules-5.3.0-23-generic \ > /<<PKGBUILDDIR>>/debian/linux-modules-extra-5.3.0-23-generic \ > -name '*.ko' | while read path_module ; do \ > module="/lib/modules/${path_module#*/lib/modules/}"; \ > if [[ -f "/<<PKGBUILDDIR>>/debian/linux-image-unsigned-5.3.0-23-generic-dbgsym/usr/lib/debug > /$module" ]] ; then \ > signature=$(tail -c 28 "$path_module"); \ > objcopy \ > --add-gnu-debuglink=/<<PKGBUILDDIR>>/debian/linux-image-unsigned-5.3.0-23-ge > neric-dbgsym/usr/lib/debug/$module \ > $path_module; \ > if grep -q CONFIG_MODULE_SIG=y /<<PKGBUILDDIR>>/debian/build/build-generic/.config & > & \ > [ "$signature" = "~Module signature appended~" ]; then \ > /<<PKGBUILDDIR>>/debian/build/build-generic/scripts/sign-file sha512 \ > /<<PKGBUILDDIR>>/debian/build/build-generic/certs/signing_key.pem \ > /<<PKGBUILDDIR>>/debian/build/build-generic/certs/signing_key.x509 \ > $path_module; \ > fi; \ > else \ > echo "WARNING: Missing debug symbols for module '$module'."; \ > fi; \ > done > WARNING: Missing debug symbols for module '/lib/modules/5.3.0-23-generic/kernel/virtualbox-guest/vbo > xsf.ko'. > WARNING: Missing debug symbols for module '/lib/modules/5.3.0-23-generic/kernel/virtualbox-guest/vbo > xguest.ko'. > /bin/bash: line 5: warning: command substitution: ignored null byte in input > /bin/bash: line 5: warning: command substitution: ignored null byte in input > /bin/bash: line 5: warning: command substitution: ignored null byte in input > [...] > > For the first one I'm not sure there's anything we can do, That warning is printed by debian/rules.d/2-binary-arch.mk, but isn't a result of this patch. But of course if there's no utility to having it we could delete it. > but the second one repeats > hundreds of times and I suspect it's cause by the return of the 'tail' command. Yes, that seems to be the case, when we feed it modules without signatures. Every solution to this problem seems to have some kind of gotcha. I'll take another crack at it. Seth
diff --git a/debian/rules.d/2-binary-arch.mk b/debian/rules.d/2-binary-arch.mk index 050f867060cb..070478e010f7 100644 --- a/debian/rules.d/2-binary-arch.mk +++ b/debian/rules.d/2-binary-arch.mk @@ -413,12 +413,12 @@ ifneq ($(skipdbg),true) -name '*.ko' | while read path_module ; do \ module="/lib/modules/$${path_module#*/lib/modules/}"; \ if [[ -f "$(dbgpkgdir)/usr/lib/debug/$$module" ]] ; then \ - signer=$$(/sbin/modinfo -F signer "$$path_module"); \ + signature=$$(tail -c 28 "$$path_module"); \ $(CROSS_COMPILE)objcopy \ --add-gnu-debuglink=$(dbgpkgdir)/usr/lib/debug/$$module \ $$path_module; \ if grep -q CONFIG_MODULE_SIG=y $(builddir)/build-$*/.config && \ - [ -n "$$signer" ]; then \ + [ "$$signature" = "~Module signature appended~" ]; then \ $(builddir)/build-$*/scripts/sign-file $(MODHASHALGO) \ $(MODSECKEY) \ $(MODPUBKEY) \
BugLink: https://bugs.launchpad.net/bugs/1852581 Not all versions of modinfo support the signer field; specifically, the version in boinic does not. This leaves all modules unsigned in hwe kernels based on eoan and later. Change the check to look for the magic string at the end of the module, which does not rely on any external tools being aware of module signatures. Signed-off-by: Seth Forshee <seth.forshee@canonical.com> --- debian/rules.d/2-binary-arch.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)