Re: RFC: sign the modules at install time

[Rusty Russell <rusty@rustcorp.com.au>]

Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, Oct 17, 2012 at 10:34 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>>
>> Hacking the keyid and signer-name to be extracted every time by
>> sign-file takes my modules_install time from 18.6 seconds to 19.1.  We'd
>> get that back easily by making sign-file a perl script anyway; it calls
>> out to perl 3 times already.
>
> Ok, that tiny slowdown seems worth the cleanup, especially if we'd get
> it back from somebody re-writing it in perl.
>
> Want to sign off on the two patches, or put them in your git tree?

Smerged them together: no point moving the x509keyid script now.
I dropped the optional dst arg, since we don't use it.

Thanks,
Rusty.
===
From: Rusty Russell <rusty@rustcorp.com.au>
Subject: [PATCH] kbuild: sign the modules at install time

Linus deleted the old code and put signing on the install command,
I fixed it to extract the keyid and signer-name within sign-file
and cleaned up that script now it always signs in-place.

Some enthusiast should convert sign-key to perl and pull
x509keyid into it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/Makefile b/Makefile
index 5be2ee8..8efe41d 100644
--- a/Makefile
+++ b/Makefile
@@ -717,6 +717,17 @@ endif # INSTALL_MOD_STRIP
 export mod_strip_cmd
 
 
+ifeq ($(CONFIG_MODULE_SIG),y)
+MODSECKEY = ./signing_key.priv
+MODPUBKEY = ./signing_key.x509
+export MODPUBKEY
+mod_sign_cmd = sh $(srctree)/scripts/sign-file $(MODSECKEY) $(MODPUBKEY) $(srctree)/scripts/x509keyid
+else
+mod_sign_cmd = true
+endif
+export mod_sign_cmd
+
+
 ifeq ($(KBUILD_EXTMOD),)
 core-y		+= kernel/ mm/ fs/ ipc/ security/ crypto/ block/
 
diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index 3d13d3a..dda4b2b 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -17,7 +17,7 @@ __modinst: $(modules)
 	@:
 
 quiet_cmd_modules_install = INSTALL $@
-      cmd_modules_install = mkdir -p $(2); cp $@ $(2) ; $(mod_strip_cmd) $(2)/$(notdir $@)
+      cmd_modules_install = mkdir -p $(2); cp $@ $(2) ; $(mod_strip_cmd) $(2)/$(notdir $@) ; $(mod_sign_cmd) $(2)/$(notdir $@)
 
 # Modules built outside the kernel source tree go into extra by default
 INSTALL_MOD_DIR ?= extra
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 0020891..a1cb022 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -14,8 +14,7 @@
 # 3)  create one <module>.mod.c file pr. module
 # 4)  create one Module.symvers file with CRC for all exported symbols
 # 5) compile all <module>.mod.c files
-# 6) final link of the module to a <module.ko> (or <module.unsigned>) file
-# 7) signs the modules to a <module.ko> file
+# 6) final link of the module to a <module.ko> file
 
 # Step 3 is used to place certain information in the module's ELF
 # section, including information such as:
@@ -33,8 +32,6 @@
 # Step 4 is solely used to allow module versioning in external modules,
 # where the CRC of each module is retrieved from the Module.symvers file.
 
-# Step 7 is dependent on CONFIG_MODULE_SIG being enabled.
-
 # KBUILD_MODPOST_WARN can be set to avoid error out in case of undefined
 # symbols in the final module linking stage
 # KBUILD_MODPOST_NOFINAL can be set to skip the final link of modules.
@@ -119,7 +116,6 @@ $(modules:.ko=.mod.o): %.mod.o: %.mod.c FORCE
 targets += $(modules:.ko=.mod.o)
 
 # Step 6), final link of the modules
-ifneq ($(CONFIG_MODULE_SIG),y)
 quiet_cmd_ld_ko_o = LD [M]  $@
       cmd_ld_ko_o = $(LD) -r $(LDFLAGS)                                 \
                              $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \
@@ -129,78 +125,7 @@ $(modules): %.ko :%.o %.mod.o FORCE
 	$(call if_changed,ld_ko_o)
 
 targets += $(modules)
-else
-quiet_cmd_ld_ko_unsigned_o = LD [M]  $@
-      cmd_ld_ko_unsigned_o =						\
-		$(LD) -r $(LDFLAGS)					\
-			 $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)	\
-			 -o $@ $(filter-out FORCE,$^)			\
-		$(if $(AFTER_LINK),; $(AFTER_LINK))
-
-$(modules:.ko=.ko.unsigned): %.ko.unsigned :%.o %.mod.o FORCE
-	$(call if_changed,ld_ko_unsigned_o)
-
-targets += $(modules:.ko=.ko.unsigned)
-
-# Step 7), sign the modules
-MODSECKEY = ./signing_key.priv
-MODPUBKEY = ./signing_key.x509
-
-ifeq ($(wildcard $(MODSECKEY))+$(wildcard $(MODPUBKEY)),$(MODSECKEY)+$(MODPUBKEY))
-ifeq ($(KBUILD_SRC),)
-	# no O= is being used
-	SCRIPTS_DIR := scripts
-else
-	SCRIPTS_DIR := $(KBUILD_SRC)/scripts
-endif
-SIGN_MODULES := 1
-else
-SIGN_MODULES := 0
-endif
-
-# only sign if it's an in-tree module
-ifneq ($(KBUILD_EXTMOD),)
-SIGN_MODULES := 0
-endif
 
-# We strip the module as best we can - note that using both strip and eu-strip
-# results in a smaller module than using either alone.
-EU_STRIP = $(shell which eu-strip || echo true)
-
-quiet_cmd_sign_ko_stripped_ko_unsigned = STRIP [M] $@
-      cmd_sign_ko_stripped_ko_unsigned = \
-		cp $< $@ && \
-		strip -x -g $@ && \
-		$(EU_STRIP) $@
-
-ifeq ($(SIGN_MODULES),1)
-
-quiet_cmd_genkeyid = GENKEYID $@
-      cmd_genkeyid = \
-		perl $(SCRIPTS_DIR)/x509keyid $< $<.signer $<.keyid
-
-%.signer %.keyid: %
-	$(call if_changed,genkeyid)
-
-KEYRING_DEP := $(MODSECKEY) $(MODPUBKEY) $(MODPUBKEY).signer $(MODPUBKEY).keyid
-quiet_cmd_sign_ko_ko_stripped = SIGN [M] $@
-      cmd_sign_ko_ko_stripped = \
-		sh $(SCRIPTS_DIR)/sign-file $(MODSECKEY) $(MODPUBKEY) $< $@
-else
-KEYRING_DEP :=
-quiet_cmd_sign_ko_ko_unsigned = NO SIGN [M] $@
-      cmd_sign_ko_ko_unsigned = \
-		cp $< $@
-endif
-
-$(modules): %.ko :%.ko.stripped $(KEYRING_DEP) FORCE
-	$(call if_changed,sign_ko_ko_stripped)
-
-$(patsubst %.ko,%.ko.stripped,$(modules)): %.ko.stripped :%.ko.unsigned FORCE
-	$(call if_changed,sign_ko_stripped_ko_unsigned)
-
-targets += $(modules)
-endif
 
 # Add FORCE to the prequisites of a target to force it to be always rebuilt.
 # ---------------------------------------------------------------------------
diff --git a/scripts/sign-file b/scripts/sign-file
index e58e34e..095a953 100644
--- a/scripts/sign-file
+++ b/scripts/sign-file
@@ -1,8 +1,8 @@
-#!/bin/sh
+#!/bin/bash
 #
 # Sign a module file using the given key.
 #
-# Format: sign-file <key> <x509> <src-file> <dst-file>
+# Format: sign-file <key> <x509> <keyid-script> <module>
 #
 
 scripts=`dirname $0`
@@ -15,8 +15,8 @@ fi
 
 key="$1"
 x509="$2"
-src="$3"
-dst="$4"
+keyid_script="$3"
+mod="$4"
 
 if [ ! -r "$key" ]
 then
@@ -29,16 +29,6 @@ then
     echo "Can't read X.509 certificate" >&2
     exit 2
 fi
-if [ ! -r "$x509.signer" ]
-then
-    echo "Can't read Signer name" >&2
-    exit 2;
-fi
-if [ ! -r "$x509.keyid" ]
-then
-    echo "Can't read Key identifier" >&2
-    exit 2;
-fi
 
 #
 # Signature parameters
@@ -83,33 +73,35 @@ fi
 
 (
 perl -e "binmode STDOUT; print pack(\"C*\", $prologue)" || exit $?
-openssl dgst $dgst -binary $src || exit $?
-) >$src.dig || exit $?
+openssl dgst $dgst -binary $mod || exit $?
+) >$mod.dig || exit $?
 
 #
 # Generate the binary signature, which will be just the integer that comprises
 # the signature with no metadata attached.
 #
-openssl rsautl -sign -inkey $key -keyform PEM -in $src.dig -out $src.sig || exit $?
-signerlen=`stat -c %s $x509.signer`
-keyidlen=`stat -c %s $x509.keyid`
-siglen=`stat -c %s $src.sig`
+openssl rsautl -sign -inkey $key -keyform PEM -in $mod.dig -out $mod.sig || exit $?
+
+SIGNER="`perl $keyid_script $x509 signer-name`"
+KEYID="`perl $keyid_script $x509 keyid`"
+keyidlen=${#KEYID}
+siglen=${#SIGNER}
 
 #
 # Build the signed binary
 #
 (
-    cat $src || exit $?
+    cat $mod || exit $?
     echo '~Module signature appended~' || exit $?
-    cat $x509.signer $x509.keyid || exit $?
+    echo -n "$SIGNER" || exit $?
+    echo -n "$KEYID" || exit $?
 
     # Preface each signature integer with a 2-byte BE length
     perl -e "binmode STDOUT; print pack(\"n\", $siglen)" || exit $?
-    cat $src.sig || exit $?
+    cat $mod.sig || exit $?
 
     # Generate the information block
     perl -e "binmode STDOUT; print pack(\"CCCCCxxxN\", $algo, $hash, $id_type, $signerlen, $keyidlen, $siglen + 2)" || exit $?
-) >$dst~ || exit $?
+) >$mod~ || exit $?
 
-# Permit in-place signing
-mv $dst~ $dst || exit $?
+mv $mod~ $mod || exit $?
diff --git a/scripts/x509keyid b/scripts/x509keyid
index c8e91a4..4241ec6 100755
--- a/scripts/x509keyid
+++ b/scripts/x509keyid
@@ -22,7 +22,7 @@ use strict;
 
 my $raw_data;
 
-die "Need three filenames\n" if ($#ARGV != 2);
+die "Need a filename [keyid|signer-name]\n" if ($#ARGV != 1);
 
 my $src = $ARGV[0];
 
@@ -259,10 +259,10 @@ die $src, ": ", "X.509: Couldn't find the Subject Key Identifier extension\n"
 
 my $id_key_id = asn1_retrieve($subject_key_id->[1]);
 
-open(OUTFD, ">$ARGV[1]") || die $ARGV[1];
-print OUTFD $id_name;
-close OUTFD || die $ARGV[1];
-
-open(OUTFD, ">$ARGV[2]") || die $ARGV[2];
-print OUTFD $id_key_id;
-close OUTFD || die $ARGV[2];
+if ($ARGV[1] eq "signer-name") {
+    print $id_name;
+} elsif ($ARGV[1] eq "keyid") {
+    print $id_key_id;
+} else {
+    die "Unknown arg";
+}