[PATCHv4] package: skip strip on signed kernel modules

[PATCHv4] package: skip strip on signed kernel modules

[omar.ocampo.coronado]

From: foocampo <omar.ocampo.coronado@intel.com>

Kernel module signatures are outside the defined ELF container,
executing strip action on kernel modules removes the signature.
In order to keep the signature on kernel modules, avoid any strip
action on signed modules.

Fore more information check kernel.org admin-guide/module-signing.

Signed-off-by: Omar Ocampo <omar.ocampo.coronado@intel.com>
---
 meta/lib/oe/package.py | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index fa3428ad61..e7eac19762 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -21,11 +21,15 @@ def runstrip(arg):
         os.chmod(file, newmode)
 
     stripcmd = [strip]
-
+    skip_strip = False
     # kernel module    
     if elftype & 16:
-        stripcmd.extend(["--strip-debug", "--remove-section=.comment",
-            "--remove-section=.note", "--preserve-dates"])
+        if is_kernel_module_signed(file):
+            bb.debug(1, "Skip strip on signed module %s" % file)
+            skip_strip = True
+        else:
+            stripcmd.extend(["--strip-debug", "--remove-section=.comment",
+                "--remove-section=.note", "--preserve-dates"])
     # .so and shared library
     elif ".so" in file and elftype & 8:
         stripcmd.extend(["--remove-section=.comment", "--remove-section=.note", "--strip-unneeded"])
@@ -36,7 +40,8 @@ def runstrip(arg):
     stripcmd.append(file)
     bb.debug(1, "runstrip: %s" % stripcmd)
 
-    output = subprocess.check_output(stripcmd, stderr=subprocess.STDOUT)
+    if not skip_strip:
+        output = subprocess.check_output(stripcmd, stderr=subprocess.STDOUT)
 
     if newmode:
         os.chmod(file, origmode)
@@ -46,6 +51,15 @@ def is_kernel_module(path):
     with open(path) as f:
         return mmap.mmap(f.fileno(), 0, prot=mmap.PROT_READ).find(b"vermagic=") >= 0
 
+# Detect if .ko module is signed
+def is_kernel_module_signed(path):
+    with open(path, "rb") as f:
+        # 'Module signature appended' the string confirms a signature is present.
+        # 28 bytes is the size of the string, regardless of the architecture.
+        f.seek(-28, 2)
+        module_tail = f.read()
+        return "Module signature appended" in "".join(chr(c) for c in bytearray(module_tail))
+
 # Return type (bits):
 # 0 - not elf
 # 1 - ELF
-- 
2.18.0

✗ patchtest: failure for package: skip strip on signed kernel modules (rev4)

[Patchwork]

== Series Details ==

Series: package: skip strip on signed kernel modules (rev4)
Revision: 4
URL   : https://patchwork.openembedded.org/series/13380/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at e2b8a3d5a1)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [PATCHv4] package: skip strip on signed kernel modules

[Andre McCurdy]

On Tue, Aug 14, 2018 at 3:33 PM,  <omar.ocampo.coronado@intel.com> wrote:
> From: foocampo <omar.ocampo.coronado@intel.com>
>
> Kernel module signatures are outside the defined ELF container,
> executing strip action on kernel modules removes the signature.
> In order to keep the signature on kernel modules, avoid any strip
> action on signed modules.
>
> Fore more information check kernel.org admin-guide/module-signing.

An earlier version of this patch has already been merged:

  http://git.openembedded.org/openembedded-core/commit/?id=4c47e5f171fa2603355e2f9183065ce8137a18c7

In general, you should always at least rebase to the latest upstream
master branch before (re)submitting a patch. Checking what's upcoming
in master-next is often useful too.

Also, when sending updates to a previously submitted patch, it's
helpful to provide a brief description of what's changed (via a
comment below the "---" line). In this case it looks like you only
added some comments and updated the commit message?

> Signed-off-by: Omar Ocampo <omar.ocampo.coronado@intel.com>

The Author and Signed-off-by lines in your patches are different.
Maybe double check your local git config before sending more patches.
Author should be your name, not a user name.

> ---
>  meta/lib/oe/package.py | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
> index fa3428ad61..e7eac19762 100644
> --- a/meta/lib/oe/package.py
> +++ b/meta/lib/oe/package.py
> @@ -21,11 +21,15 @@ def runstrip(arg):
>          os.chmod(file, newmode)
>
>      stripcmd = [strip]
> -
> +    skip_strip = False
>      # kernel module
>      if elftype & 16:
> -        stripcmd.extend(["--strip-debug", "--remove-section=.comment",
> -            "--remove-section=.note", "--preserve-dates"])
> +        if is_kernel_module_signed(file):
> +            bb.debug(1, "Skip strip on signed module %s" % file)
> +            skip_strip = True
> +        else:
> +            stripcmd.extend(["--strip-debug", "--remove-section=.comment",
> +                "--remove-section=.note", "--preserve-dates"])
>      # .so and shared library
>      elif ".so" in file and elftype & 8:
>          stripcmd.extend(["--remove-section=.comment", "--remove-section=.note", "--strip-unneeded"])
> @@ -36,7 +40,8 @@ def runstrip(arg):
>      stripcmd.append(file)
>      bb.debug(1, "runstrip: %s" % stripcmd)
>
> -    output = subprocess.check_output(stripcmd, stderr=subprocess.STDOUT)
> +    if not skip_strip:
> +        output = subprocess.check_output(stripcmd, stderr=subprocess.STDOUT)
>
>      if newmode:
>          os.chmod(file, origmode)
> @@ -46,6 +51,15 @@ def is_kernel_module(path):
>      with open(path) as f:
>          return mmap.mmap(f.fileno(), 0, prot=mmap.PROT_READ).find(b"vermagic=") >= 0
>
> +# Detect if .ko module is signed
> +def is_kernel_module_signed(path):
> +    with open(path, "rb") as f:
> +        # 'Module signature appended' the string confirms a signature is present.
> +        # 28 bytes is the size of the string, regardless of the architecture.
> +        f.seek(-28, 2)
> +        module_tail = f.read()
> +        return "Module signature appended" in "".join(chr(c) for c in bytearray(module_tail))
> +
>  # Return type (bits):
>  # 0 - not elf
>  # 1 - ELF
> --
> 2.18.0
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core