Support for PKCS#7 module signing.

Support for PKCS#7 module signing.

[Wouter van Kesteren]

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

Hello,

I asked the following on irc, where it was suggested that i take it to
this mailing list instead.

Commit https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=bc1c373dd2a5113800360f7152be729c9da996cc
introduced a new method of signing modules.

I attempted to make a patch to support this new method of signing. But
whilst doing so i came to the conclusion that a lot of things that are
in the appended struct are now set to zero.
Infact, everything except id_type (which is 2) and sig_len is set to
zero. Instead this information seems to be embedded in the signature
blob instead.

Now my question is, how do we want to go from here? If this
information is too important to just leave out we need to learn kmod
how to parse the PKCS#7 blob. But is that desireable? And if so how
should we go about doing that, use a crypto library, which one?

Attached is a WIP of the work i have done sofar. Currently it just
returns "" to library users (i figured that was better than suddenly
handing them nullpointers out of nowhere) and the modinfo frontend
just omits empty things.

[-- Attachment #2: kmod-pkcs7-wip.patch --]
[-- Type: text/x-patch, Size: 10047 bytes --]

From 4c6648aadf7625adb9003340a922c054eaa579a6 Mon Sep 17 00:00:00 2001
From: Wouter van Kesteren <woutershep@gmail.com>
Date: Fri, 8 Jan 2016 18:07:11 +0100
Subject: [PATCH 1/3] teach module to eat empties

---
 libkmod/libkmod-module.c | 82 ++++++++++++++++++++++++++++++------------------
 1 file changed, 52 insertions(+), 30 deletions(-)

diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index 50b2ff9..e92fd56 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -2247,40 +2247,62 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_
 		struct kmod_list *n;
 		char *key_hex;
 
-		n = kmod_module_info_append(list, "signer", strlen("signer"),
-				sig_info.signer, sig_info.signer_len);
-		if (n == NULL)
-			goto list_error;
-		count++;
+		if (sig_info.signer_len) {
+			n = kmod_module_info_append(list, "signer", strlen("signer"),
+					sig_info.signer, sig_info.signer_len);
+			if (n == NULL)
+				goto list_error;
+			count++;
+		}
 
-		/* Display the key id as 01:12:DE:AD:BE:EF:... */
-		key_hex = malloc(sig_info.key_id_len * 3);
-		if (key_hex == NULL)
-			goto list_error;
-		for (i = 0; i < (int)sig_info.key_id_len; i++) {
-			sprintf(key_hex + i * 3, "%02X",
-					(unsigned char)sig_info.key_id[i]);
-			if (i < (int)sig_info.key_id_len - 1)
-				key_hex[i * 3 + 2] = ':';
+		if (sig_info.key_id_len) {
+			/* Display the key id as 01:12:DE:AD:BE:EF:... */
+			key_hex = malloc(sig_info.key_id_len * 3);
+			if (key_hex == NULL)
+				goto list_error;
+			for (i = 0; i < (int)sig_info.key_id_len; i++) {
+				sprintf(key_hex + i * 3, "%02X",
+						(unsigned char)sig_info.key_id[i]);
+				if (i < (int)sig_info.key_id_len - 1)
+					key_hex[i * 3 + 2] = ':';
+			}
+			n = kmod_module_info_append(list, "sig_key", strlen("sig_key"),
+					key_hex, sig_info.key_id_len * 3 - 1);
+			free(key_hex);
+			if (n == NULL)
+				goto list_error;
+			count++;
 		}
-		n = kmod_module_info_append(list, "sig_key", strlen("sig_key"),
-				key_hex, sig_info.key_id_len * 3 - 1);
-		free(key_hex);
-		if (n == NULL)
-			goto list_error;
-		count++;
 
-		n = kmod_module_info_append(list,
-				"sig_hashalgo", strlen("sig_hashalgo"),
-				sig_info.hash_algo, strlen(sig_info.hash_algo));
-		if (n == NULL)
-			goto list_error;
-		count++;
+		size_t algo_len = strlen(sig_info.algo);
+		if (algo_len) {
+			n = kmod_module_info_append(list,
+					"sig_algo", strlen("sig_algo"),
+					sig_info.algo, algo_len);
+			if (n == NULL)
+				goto list_error;
+			count++;
+		}
 
-		/*
-		 * Omit sig_info.id_type and sig_info.algo for now, as these
-		 * are currently constant.
-		 */
+		size_t hash_algo_len = strlen(sig_info.hash_algo);
+		if (hash_algo_len) {
+			n = kmod_module_info_append(list,
+					"sig_hashalgo", strlen("sig_hashalgo"),
+					sig_info.hash_algo, hash_algo_len);
+			if (n == NULL)
+				goto list_error;
+			count++;
+		}
+
+		size_t id_type_len = strlen(sig_info.id_type);
+		if (id_type_len) {
+			n = kmod_module_info_append(list,
+					"sig_id_type", strlen("sig_id_type"),
+					sig_info.id_type, id_type_len);
+			if (n == NULL)
+				goto list_error;
+			count++;
+		}
 	}
 	ret = count;
 
-- 
2.7.0


From 1438da552f6c92fa57c436eaebd90fb6ad76a6bd Mon Sep 17 00:00:00 2001
From: Wouter van Kesteren <woutershep@gmail.com>
Date: Fri, 8 Jan 2016 18:13:29 +0100
Subject: [PATCH 2/3] teach libkmod-signature to eat pkcs7

---
 libkmod/libkmod-signature.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/libkmod/libkmod-signature.c b/libkmod/libkmod-signature.c
index 6fc06fc..8afc8d1 100644
--- a/libkmod/libkmod-signature.c
+++ b/libkmod/libkmod-signature.c
@@ -69,12 +69,14 @@ const char *const pkey_hash_algo[PKEY_HASH__LAST] = {
 enum pkey_id_type {
 	PKEY_ID_PGP,		/* OpenPGP generated key ID */
 	PKEY_ID_X509,		/* X.509 arbitrary subjectKeyIdentifier */
+	PKEY_ID_PKCS7,
 	PKEY_ID_TYPE__LAST
 };
 
 const char *const pkey_id_type[PKEY_ID_TYPE__LAST] = {
 	[PKEY_ID_PGP]		= "PGP",
 	[PKEY_ID_X509]		= "X509",
+	[PKEY_ID_PKCS7]		= "PKCS#7",
 };
 
 /*
@@ -132,16 +134,26 @@ bool kmod_module_signature_info(const struct kmod_file *file, struct kmod_signat
 	    size < (int64_t)(modsig->signer_len + modsig->key_id_len + sig_len))
 		return false;
 
-	size -= modsig->key_id_len + sig_len;
-	sig_info->key_id = mem + size;
-	sig_info->key_id_len = modsig->key_id_len;
+	if (modsig->key_id_len == PKEY_ID_PKCS7) {
+		sig_info->key_id = "";
+		sig_info->key_id_len = 0;
+		sig_info->signer = "";
+		sig_info->signer_len = 0;
+		sig_info->algo = "";
+		sig_info->hash_algo = "";
+	}else {
+		size -= modsig->key_id_len + sig_len;
+		sig_info->key_id = mem + size;
+		sig_info->key_id_len = modsig->key_id_len;
+
+		size -= modsig->signer_len;
+		sig_info->signer = mem + size;
+		sig_info->signer_len = modsig->signer_len;
+
+		sig_info->algo = pkey_algo[modsig->algo];
+		sig_info->hash_algo = pkey_hash_algo[modsig->hash];
+	}
 
-	size -= modsig->signer_len;
-	sig_info->signer = mem + size;
-	sig_info->signer_len = modsig->signer_len;
-
-	sig_info->algo = pkey_algo[modsig->algo];
-	sig_info->hash_algo = pkey_hash_algo[modsig->hash];
 	sig_info->id_type = pkey_id_type[modsig->id_type];
 
 	return true;
-- 
2.7.0


From 78fde9ba51fb88906f4fe1c11a9032960ac52d69 Mon Sep 17 00:00:00 2001
From: Wouter van Kesteren <woutershep@gmail.com>
Date: Fri, 8 Jan 2016 16:15:52 +0100
Subject: [PATCH 3/3] testsuitestuff

---
 testsuite/module-playground/dummy.sha512cms               | Bin 0 -> 710 bytes
 testsuite/populate-modules.sh                             |   9 +++++++++
 .../rootfs-pristine/test-modinfo/correct-sig_hashalgo.txt |   1 +
 .../rootfs-pristine/test-modinfo/correct-sig_key.txt      |   1 +
 testsuite/rootfs-pristine/test-modinfo/correct-signer.txt |   1 +
 testsuite/test-modinfo.c                                  |   3 ++-
 6 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 testsuite/module-playground/dummy.sha512cms

diff --git a/testsuite/module-playground/dummy.sha512cms b/testsuite/module-playground/dummy.sha512cms
new file mode 100644
index 0000000000000000000000000000000000000000..f70f6b7455aa99581f28adfbb07f02884525e636
GIT binary patch
literal 710
zcmXqLVw%OqsnzDu_MMlJooPW6Q@24AQ#%tQqam*WHydX{n+IbmGYb<lhyzo`XxPM*
z0#ud&RAyjppl7IKpvA@<%EHW}>{ObWlcG?PnVYJRSXz>wo|>0hlvt9QqL7_hl$V+V
zq$-&>8Ty6T7VdVKVTfu2FWd%3Mpg!vCMJeE(gt6C-*9J9NxXhT*R}F<&@7$?X5pXd
z^ABidUcY~0*Uv56&nhr|@M)c7sPM~Q+a~v<wc-Z$$kTzD(bY0tY;)|`j4U&p^p?-;
z(fP73`(Rm@US6W_-NwkjOEi?sj=j0MW$t&j%$iQqKK+Ug0Ua9-^+WMeJReJ)x9okf
zKWAx#h~A>TvG1R6och^-#WZba+p0@~g$JB3r+<AATYT9z_ex2!%x>Ap`LPy$fAknv
z1$M`a%#l2O=lj|(Kc;Q^7cUsk8v0Yy<IT2j<)2m>h9)Xzda6I;vnh^f_$<crsoC|a
z%N?7Qhnftsc4)=#OisSG_RP#s&%H-gub6Jus-5&@6N}%rs#jlC&NDD&_xOcO^t%+f
z{+;e*{fg;VS1WU$I&|@+h0C=d_v;(`!WAnIX8C-6!p!|urF7GcKGtZrkURf<FIyyj
zc>XC>ZaS~xD#00Q@i)9?c^qAR<P*DN<o`L#HMXDJptX70alNA#EvpixnkLVD*8Au7
z<jwOp{uT-{-XLRn{C#>rdU(LwZBf@hhAjAR9{$26R{xdgML+p9zf6<PJKs0{DL>I(
zLH%Lz{!25GtY*C8_$TyqUbXkHS8pPI-o934Ta(}r;Wfkm#0GB1jQ6%F8=lUTZaIFE
zO+3?o{;vPsJTG-EIk&n#;*(ToYfE3OoZqo5Z1d7dxsD&(&S<Ymmn-wx9(#s?fr$YE
inC8{_=BJeAq$(6=rspM=lokOKSV2K*UP@|89Txx@9WOZm

literal 0
HcmV?d00001

diff --git a/testsuite/populate-modules.sh b/testsuite/populate-modules.sh
index 409a6de..d1f8e8d 100755
--- a/testsuite/populate-modules.sh
+++ b/testsuite/populate-modules.sh
@@ -42,6 +42,7 @@ map=(
     ["test-modinfo/mod-simple-sparc64.ko"]="mod-simple-sparc64.ko"
     ["test-modinfo/mod-simple-sha1.ko"]="mod-simple.ko"
     ["test-modinfo/mod-simple-sha256.ko"]="mod-simple.ko"
+    ["test-modinfo/mod-simple-sha512cms.ko"]="mod-simple.ko"
     ["test-tools/insert/lib/modules/4.4.4/kernel/"]="mod-simple.ko"
     ["test-tools/remove/lib/modules/4.4.4/kernel/"]="mod-simple.ko"
 )
@@ -60,6 +61,10 @@ attach_sha1_array=(
     "test-modinfo/mod-simple-sha1.ko"
     )
 
+attach_sha512cms_array=(
+    "test-modinfo/mod-simple-sha512cms.ko"
+    )
+
 for k in ${!map[@]}; do
     dst=${ROOTFS}/$k
     src=${MODULE_PLAYGROUND}/${map[$k]}
@@ -86,3 +91,7 @@ done
 for m in "${attach_sha256_array[@]}"; do
     cat ${MODULE_PLAYGROUND}/dummy.sha256 >> ${ROOTFS}/$m
 done
+
+for m in "${attach_sha512cms_array[@]}"; do
+    cat ${MODULE_PLAYGROUND}/dummy.sha512cms >> ${ROOTFS}/$m
+done
diff --git a/testsuite/rootfs-pristine/test-modinfo/correct-sig_hashalgo.txt b/testsuite/rootfs-pristine/test-modinfo/correct-sig_hashalgo.txt
index 6d0223e..d3719ab 100644
--- a/testsuite/rootfs-pristine/test-modinfo/correct-sig_hashalgo.txt
+++ b/testsuite/rootfs-pristine/test-modinfo/correct-sig_hashalgo.txt
@@ -1,3 +1,4 @@
 sha1
 sha256
+sha512
 
diff --git a/testsuite/rootfs-pristine/test-modinfo/correct-sig_key.txt b/testsuite/rootfs-pristine/test-modinfo/correct-sig_key.txt
index 7dc4c6a..0a39b3c 100644
--- a/testsuite/rootfs-pristine/test-modinfo/correct-sig_key.txt
+++ b/testsuite/rootfs-pristine/test-modinfo/correct-sig_key.txt
@@ -1,3 +1,4 @@
 E3:C8:FC:A7:3F:B3:1D:DE:84:81:EF:38:E3:4C:DE:4B:0C:FD:1B:F9
 E3:C8:FC:A7:3F:B3:1D:DE:84:81:EF:38:E3:4C:DE:4B:0C:FD:1B:F9
+0B:1F:A8:03:46:54:95:62:51:45:72:D6:39:78:F3:A0:60:23:EF:2D
 
diff --git a/testsuite/rootfs-pristine/test-modinfo/correct-signer.txt b/testsuite/rootfs-pristine/test-modinfo/correct-signer.txt
index afe83df..f9cd69b 100644
--- a/testsuite/rootfs-pristine/test-modinfo/correct-signer.txt
+++ b/testsuite/rootfs-pristine/test-modinfo/correct-signer.txt
@@ -1,3 +1,4 @@
 Magrathea: Glacier signing key
 Magrathea: Glacier signing key
+Build time autogenerated kernel key
 
diff --git a/testsuite/test-modinfo.c b/testsuite/test-modinfo.c
index 4877502..fa0aa5b 100644
--- a/testsuite/test-modinfo.c
+++ b/testsuite/test-modinfo.c
@@ -56,7 +56,8 @@ DEFINE_TEST(test_modinfo_##_field, \
 #define DEFINE_MODINFO_SIGN_TEST(_field) \
 	DEFINE_MODINFO_TEST(_field, \
 			    "/mod-simple-sha1.ko", \
-			    "/mod-simple-sha256.ko")
+			    "/mod-simple-sha256.ko", \
+			    "/mod-simple-sha512cms.ko")
 
 DEFINE_MODINFO_GENERIC_TEST(filename);
 DEFINE_MODINFO_GENERIC_TEST(author);
-- 
2.7.0

Re: Support for PKCS#7 module signing.

[Lucas De Marchi]

Hi Wouter,


Sorry for the delay.

On Sun, Jan 10, 2016 at 10:15 PM, Wouter van Kesteren
<woutershep@gmail.com> wrote:
> Hello,
>
> I asked the following on irc, where it was suggested that i take it to
> this mailing list instead.
>
> Commit https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=bc1c373dd2a5113800360f7152be729c9da996cc
> introduced a new method of signing modules.
>
> I attempted to make a patch to support this new method of signing. But
> whilst doing so i came to the conclusion that a lot of things that are
> in the appended struct are now set to zero.
> Infact, everything except id_type (which is 2) and sig_len is set to
> zero. Instead this information seems to be embedded in the signature
> blob instead.

That struct should be filled by the tool signing the module:

/*
 * Module signature information block.
 */
struct module_signature {
        uint8_t algo;        /* Public-key crypto algorithm [enum pkey_algo] */
        uint8_t hash;        /* Digest algorithm [enum pkey_hash_algo] */
        uint8_t id_type;     /* Key identifier type [enum pkey_id_type] */
        uint8_t signer_len;  /* Length of signer's name */
        uint8_t key_id_len;  /* Length of key identifier */
        uint8_t __pad[3];
        uint32_t sig_len;    /* Length of signature data (big endian) */
};


I'm not sure why it was decided to omit this information in the commit
you mentioned and embed it inside the signature blob. I'm CC'ing
Michael who worked on the support for signature in kmod and David who
did that commit.  Ideally kmod would not link to any crypto library.


thanks

Lucas De Marchi

Re: Support for PKCS#7 module signing.

[Michal Marek]

On 2016-01-14 20:43, Lucas De Marchi wrote:
> Hi Wouter,
> 
> 
> Sorry for the delay.

Hi,

sorry for the even longer delay.


> On Sun, Jan 10, 2016 at 10:15 PM, Wouter van Kesteren
> <woutershep@gmail.com> wrote:
>> Hello,
>>
>> I asked the following on irc, where it was suggested that i take it to
>> this mailing list instead.
>>
>> Commit https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=bc1c373dd2a5113800360f7152be729c9da996cc
>> introduced a new method of signing modules.
>>
>> I attempted to make a patch to support this new method of signing. But
>> whilst doing so i came to the conclusion that a lot of things that are
>> in the appended struct are now set to zero.
>> Infact, everything except id_type (which is 2) and sig_len is set to
>> zero. Instead this information seems to be embedded in the signature
>> blob instead.
> 
> That struct should be filled by the tool signing the module:
[...]
> I'm not sure why it was decided to omit this information in the commit
> you mentioned and embed it inside the signature blob.

The kernel now retrieves the hash and algorithm from the PKCS#7 message.

> Ideally kmod would not link to any crypto library.

Right. What we can do easily is to print signature: PKCS#7 to at least
let the user know that the module has a signature appended.

Michal