[PATCH 0/3] KEXEC_SIG with appended signature

[PATCH 0/3] KEXEC_SIG with appended signature

[Michal Suchanek]

S390 uses appended signature for kernel but implements the check
separately from module loader.

Support for secure boot on powerpc with appended signature is planned -
grub patches submitted upstream but not yet merged.

This is an attempt at unified appended signature verification.

Thanks

Michal

Michal Suchanek (3):
  s390/kexec_file: Don't opencode appended signature verification.
  module: strip the signature marker in the verification function.
  powerpc/kexec_file: Add KEXEC_SIG support.

 arch/powerpc/Kconfig                  | 11 +++++++
 arch/powerpc/kexec/elf_64.c           | 14 +++++++++
 arch/s390/kernel/machine_kexec_file.c | 42 +++------------------------
 include/linux/verification.h          |  3 ++
 kernel/module-internal.h              |  2 --
 kernel/module.c                       | 11 +++----
 kernel/module_signing.c               | 32 ++++++++++++++------
 7 files changed, 59 insertions(+), 56 deletions(-)

-- 
2.31.1

[PATCH 1/3] s390/kexec_file: Don't opencode appended signature verification.

[Michal Suchanek]

Module verification already implements appeded signature verification.

Reuse it for kexec_file.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/s390/kernel/machine_kexec_file.c | 33 ++++-----------------------
 include/linux/verification.h          |  3 +++
 kernel/module-internal.h              |  2 --
 kernel/module.c                       |  4 +++-
 kernel/module_signing.c               | 24 +++++++++++--------
 5 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index f9e4baa64b67..634e641cd8aa 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -23,11 +23,10 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
 };
 
 #ifdef CONFIG_KEXEC_SIG
-int s390_verify_sig(const char *kernel, unsigned long kernel_len)
+int s390_verify_sig(const char *kernel, unsigned long length)
 {
+	size_t kernel_len = length;
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-	struct module_signature *ms;
-	unsigned long sig_len;
 
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
@@ -41,32 +40,8 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 		return -EKEYREJECTED;
 	kernel_len -= marker_len;
 
-	ms = (void *)kernel + kernel_len - sizeof(*ms);
-	kernel_len -= sizeof(*ms);
-
-	sig_len = be32_to_cpu(ms->sig_len);
-	if (sig_len >= kernel_len)
-		return -EKEYREJECTED;
-	kernel_len -= sig_len;
-
-	if (ms->id_type != PKEY_ID_PKCS7)
-		return -EKEYREJECTED;
-
-	if (ms->algo != 0 ||
-	    ms->hash != 0 ||
-	    ms->signer_len != 0 ||
-	    ms->key_id_len != 0 ||
-	    ms->__pad[0] != 0 ||
-	    ms->__pad[1] != 0 ||
-	    ms->__pad[2] != 0) {
-		return -EBADMSG;
-	}
-
-	return verify_pkcs7_signature(kernel, kernel_len,
-				      kernel + kernel_len, sig_len,
-				      VERIFY_USE_PLATFORM_KEYRING,
-				      VERIFYING_MODULE_SIGNATURE,
-				      NULL, NULL);
+	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
+					"kexec_file");
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/include/linux/verification.h b/include/linux/verification.h
index a655923335ae..c1cf0582012a 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -60,5 +60,8 @@ extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
 				   enum key_being_used_for usage);
 #endif
 
+int verify_appended_signature(const void *data, size_t *len, struct key *trusted_keys,
+			      const char *what);
+
 #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
 #endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/kernel/module-internal.h b/kernel/module-internal.h
index 33783abc377b..80461e14bf29 100644
--- a/kernel/module-internal.h
+++ b/kernel/module-internal.h
@@ -27,5 +27,3 @@ struct load_info {
 		unsigned int sym, str, mod, vers, info, pcpu;
 	} index;
 };
-
-extern int mod_verify_sig(const void *mod, struct load_info *info);
diff --git a/kernel/module.c b/kernel/module.c
index 5c26a76e800b..137b3661be75 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -57,6 +57,7 @@
 #include <linux/bsearch.h>
 #include <linux/dynamic_debug.h>
 #include <linux/audit.h>
+#include <linux/verification.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
 
@@ -2894,7 +2895,8 @@ static int module_sig_check(struct load_info *info, int flags)
 	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
 		/* We truncate the module to discard the signature */
 		info->len -= markerlen;
-		err = mod_verify_sig(mod, info);
+		err = verify_appended_signature(mod, &info->len,
+						VERIFY_USE_SECONDARY_KEYRING, "module");
 		if (!err) {
 			info->sig_ok = true;
 			return 0;
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 8723ae70ea1f..f492e410564d 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -14,13 +14,19 @@
 #include <crypto/public_key.h>
 #include "module-internal.h"
 
-/*
- * Verify the signature on a module.
+/**
+ * verify_appended_signature - Verify the signature on a module with the
+ * signature marker stripped.
+ * @data: The data to be verified
+ * @len: Size of @data.
+ * @trusted_keys: Keyring to use for verification
+ * @what: Informational string for log messages
  */
-int mod_verify_sig(const void *mod, struct load_info *info)
+int verify_appended_signature(const void *data, size_t *len,
+			      struct key *trusted_keys, const char *what)
 {
 	struct module_signature ms;
-	size_t sig_len, modlen = info->len;
+	size_t sig_len, modlen = *len;
 	int ret;
 
 	pr_devel("==>%s(,%zu)\n", __func__, modlen);
@@ -28,18 +34,18 @@ int mod_verify_sig(const void *mod, struct load_info *info)
 	if (modlen <= sizeof(ms))
 		return -EBADMSG;
 
-	memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
+	memcpy(&ms, data + (modlen - sizeof(ms)), sizeof(ms));
 
-	ret = mod_check_sig(&ms, modlen, "module");
+	ret = mod_check_sig(&ms, modlen, what);
 	if (ret)
 		return ret;
 
 	sig_len = be32_to_cpu(ms.sig_len);
 	modlen -= sig_len + sizeof(ms);
-	info->len = modlen;
+	*len = modlen;
 
-	return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
-				      VERIFY_USE_SECONDARY_KEYRING,
+	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
+				      trusted_keys,
 				      VERIFYING_MODULE_SIGNATURE,
 				      NULL, NULL);
 }
-- 
2.31.1

[PATCH 2/3] module: strip the signature marker in the verification function.

[Michal Suchanek]

It is stripped by each caller separately.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/s390/kernel/machine_kexec_file.c |  9 ---------
 kernel/module.c                       |  7 +------
 kernel/module_signing.c               | 12 ++++++++++--
 3 files changed, 11 insertions(+), 17 deletions(-)

diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 634e641cd8aa..82260bb61060 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -26,20 +26,11 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
 int s390_verify_sig(const char *kernel, unsigned long length)
 {
 	size_t kernel_len = length;
-	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
 
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
 		return 0;
 
-	if (marker_len > kernel_len)
-		return -EKEYREJECTED;
-
-	if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
-		   marker_len))
-		return -EKEYREJECTED;
-	kernel_len -= marker_len;
-
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
 					"kexec_file");
 }
diff --git a/kernel/module.c b/kernel/module.c
index 137b3661be75..1c421b0442e3 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2882,7 +2882,6 @@ static inline void kmemleak_load_module(const struct module *mod,
 static int module_sig_check(struct load_info *info, int flags)
 {
 	int err = -ENODATA;
-	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	const char *reason;
 	const void *mod = info->hdr;
 
@@ -2890,11 +2889,7 @@ static int module_sig_check(struct load_info *info, int flags)
 	 * Require flags == 0, as a module with version information
 	 * removed is no longer the module that was signed
 	 */
-	if (flags == 0 &&
-	    info->len > markerlen &&
-	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
-		/* We truncate the module to discard the signature */
-		info->len -= markerlen;
+	if (flags == 0) {
 		err = verify_appended_signature(mod, &info->len,
 						VERIFY_USE_SECONDARY_KEYRING, "module");
 		if (!err) {
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index f492e410564d..4c28cb55275f 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -15,8 +15,7 @@
 #include "module-internal.h"
 
 /**
- * verify_appended_signature - Verify the signature on a module with the
- * signature marker stripped.
+ * verify_appended_signature - Verify the signature on a module
  * @data: The data to be verified
  * @len: Size of @data.
  * @trusted_keys: Keyring to use for verification
@@ -25,12 +24,21 @@
 int verify_appended_signature(const void *data, size_t *len,
 			      struct key *trusted_keys, const char *what)
 {
+	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature ms;
 	size_t sig_len, modlen = *len;
 	int ret;
 
 	pr_devel("==>%s(,%zu)\n", __func__, modlen);
 
+	if (markerlen > modlen)
+		return -ENODATA;
+
+	if (memcmp(data + modlen - markerlen, MODULE_SIG_STRING,
+		   markerlen))
+		return -ENODATA;
+	modlen -= markerlen;
+
 	if (modlen <= sizeof(ms))
 		return -EBADMSG;
 
-- 
2.31.1

[PATCH 3/3] powerpc/kexec_file: Add KEXEC_SIG support.

[Michal Suchanek]

Use the module verifier for the kernel image verification.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/Kconfig        | 11 +++++++++++
 arch/powerpc/kexec/elf_64.c | 14 ++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 743c9783c64f..27bffafa9e79 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -558,6 +558,17 @@ config KEXEC_FILE
 config ARCH_HAS_KEXEC_PURGATORY
 	def_bool KEXEC_FILE
 
+config KEXEC_SIG
+	bool "Verify kernel signature during kexec_file_load() syscall"
+	depends on KEXEC_FILE && MODULE_SIG_FORMAT
+	help
+	  This option makes kernel signature verification mandatory for
+	  the kexec_file_load() syscall.
+
+	  In addition to that option, you need to enable signature
+	  verification for the corresponding kernel image type being
+	  loaded in order for this to work.
+
 config PPC64_BUILD_ELF_V2_ABI
 	bool
 
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index eeb258002d1e..e8dff6b23ac5 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -23,6 +23,7 @@
 #include <linux/of_fdt.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/verification.h>
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
 			unsigned long kernel_len, char *initrd,
@@ -151,7 +152,20 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 	return ret ? ERR_PTR(ret) : NULL;
 }
 
+#ifdef CONFIG_KEXEC_SIG
+int elf64_verify_sig(const char *kernel, unsigned long length)
+{
+	size_t kernel_len = length;
+
+	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
+					 "kexec_file");
+}
+#endif /* CONFIG_KEXEC_SIG */
+
 const struct kexec_file_ops kexec_elf64_ops = {
 	.probe = kexec_elf_probe,
 	.load = elf64_load,
+#ifdef CONFIG_KEXEC_SIG
+	.verify_sig = elf64_verify_sig,
+#endif
 };
-- 
2.31.1

[PATCH 0/2] Additional appended signature cleanup

[Michal Suchanek]

There is one more copy of the code checking appended signarutes.

Merge the common code to module_signature.c

Thanks

Michal

Michal Suchanek (2):
  module: Use key_being_used_for for log messages in
    verify_appended_signature
  module: Move duplicate mod_check_sig users code to mod_parse_sig

 arch/powerpc/kexec/elf_64.c              |  2 +-
 arch/s390/kernel/machine_kexec_file.c    |  2 +-
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 include/linux/module_signature.h         |  1 +
 include/linux/verification.h             |  3 +-
 kernel/module.c                          |  3 +-
 kernel/module_signature.c                | 56 +++++++++++++++++++++++-
 kernel/module_signing.c                  | 33 ++++----------
 security/integrity/ima/ima_modsig.c      | 22 ++--------
 9 files changed, 74 insertions(+), 49 deletions(-)

-- 
2.31.1

[PATCH 1/2] module: Use key_being_used_for for log messages in verify_appended_signature

[Michal Suchanek]

Add value for kexec appended signature and pass in key_being_used_for
enum rather than a string to verify_appended_signature to produce log
messages about the signature.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/kexec/elf_64.c              |  2 +-
 arch/s390/kernel/machine_kexec_file.c    |  2 +-
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 include/linux/verification.h             |  3 ++-
 kernel/module.c                          |  3 ++-
 kernel/module_signing.c                  | 11 ++++++-----
 6 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index e8dff6b23ac5..3aa5269f6e0f 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -158,7 +158,7 @@ int elf64_verify_sig(const char *kernel, unsigned long length)
 	size_t kernel_len = length;
 
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
-					 "kexec_file");
+					 VERIFYING_KEXEC_APPENDED_SIGNATURE);
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 82260bb61060..37fcbb149368 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -32,7 +32,7 @@ int s390_verify_sig(const char *kernel, unsigned long length)
 		return 0;
 
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
-					"kexec_file");
+					VERIFYING_KEXEC_APPENDED_SIGNATURE);
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index ad8af3d70ac0..6fd20eec3882 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -25,6 +25,7 @@ const char *const key_being_used_for[NR__KEY_BEING_USED_FOR] = {
 	[VERIFYING_KEXEC_PE_SIGNATURE]		= "kexec PE sig",
 	[VERIFYING_KEY_SIGNATURE]		= "key sig",
 	[VERIFYING_KEY_SELF_SIGNATURE]		= "key self sig",
+	[VERIFYING_KEXEC_APPENDED_SIGNATURE]	= "kexec appended sig",
 	[VERIFYING_UNSPECIFIED_SIGNATURE]	= "unspec sig",
 };
 EXPORT_SYMBOL_GPL(key_being_used_for);
diff --git a/include/linux/verification.h b/include/linux/verification.h
index c1cf0582012a..23748feb9e03 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -26,6 +26,7 @@ enum key_being_used_for {
 	VERIFYING_KEXEC_PE_SIGNATURE,
 	VERIFYING_KEY_SIGNATURE,
 	VERIFYING_KEY_SELF_SIGNATURE,
+	VERIFYING_KEXEC_APPENDED_SIGNATURE,
 	VERIFYING_UNSPECIFIED_SIGNATURE,
 	NR__KEY_BEING_USED_FOR
 };
@@ -61,7 +62,7 @@ extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
 #endif
 
 int verify_appended_signature(const void *data, size_t *len, struct key *trusted_keys,
-			      const char *what);
+			      enum key_being_used_for purpose);
 
 #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
 #endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/kernel/module.c b/kernel/module.c
index 1c421b0442e3..5f1cf989e1cf 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2891,7 +2891,8 @@ static int module_sig_check(struct load_info *info, int flags)
 	 */
 	if (flags == 0) {
 		err = verify_appended_signature(mod, &info->len,
-						VERIFY_USE_SECONDARY_KEYRING, "module");
+						VERIFY_USE_SECONDARY_KEYRING,
+						VERIFYING_MODULE_SIGNATURE);
 		if (!err) {
 			info->sig_ok = true;
 			return 0;
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 4c28cb55275f..cef72a6f6b5d 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -19,17 +19,18 @@
  * @data: The data to be verified
  * @len: Size of @data.
  * @trusted_keys: Keyring to use for verification
- * @what: Informational string for log messages
+ * @purpose: The use to which the key is being put
  */
 int verify_appended_signature(const void *data, size_t *len,
-			      struct key *trusted_keys, const char *what)
+			      struct key *trusted_keys,
+			      enum key_being_used_for purpose)
 {
 	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature ms;
 	size_t sig_len, modlen = *len;
 	int ret;
 
-	pr_devel("==>%s(,%zu)\n", __func__, modlen);
+	pr_devel("==>%s %s(,%zu)\n", __func__, key_being_used_for[purpose], modlen);
 
 	if (markerlen > modlen)
 		return -ENODATA;
@@ -44,7 +45,7 @@ int verify_appended_signature(const void *data, size_t *len,
 
 	memcpy(&ms, data + (modlen - sizeof(ms)), sizeof(ms));
 
-	ret = mod_check_sig(&ms, modlen, what);
+	ret = mod_check_sig(&ms, modlen, key_being_used_for[purpose]);
 	if (ret)
 		return ret;
 
@@ -54,6 +55,6 @@ int verify_appended_signature(const void *data, size_t *len,
 
 	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
 				      trusted_keys,
-				      VERIFYING_MODULE_SIGNATURE,
+				      purpose,
 				      NULL, NULL);
 }
-- 
2.31.1

[PATCH 2/2] module: Move duplicate mod_check_sig users code to mod_parse_sig

[Michal Suchanek]

Multiple users of mod_check_sig check for the marker, then call
mod_check_sig, extract signature length, and remove the signature.

Put this code in one place together with mod_check_sig.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 include/linux/module_signature.h    |  1 +
 kernel/module_signature.c           | 56 ++++++++++++++++++++++++++++-
 kernel/module_signing.c             | 26 +++-----------
 security/integrity/ima/ima_modsig.c | 22 ++----------
 4 files changed, 63 insertions(+), 42 deletions(-)

diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
index 7eb4b00381ac..1343879b72b3 100644
--- a/include/linux/module_signature.h
+++ b/include/linux/module_signature.h
@@ -42,5 +42,6 @@ struct module_signature {
 
 int mod_check_sig(const struct module_signature *ms, size_t file_len,
 		  const char *name);
+int mod_parse_sig(const void *data, size_t *len, size_t *sig_len, const char *name);
 
 #endif /* _LINUX_MODULE_SIGNATURE_H */
diff --git a/kernel/module_signature.c b/kernel/module_signature.c
index 00132d12487c..784b40575ee4 100644
--- a/kernel/module_signature.c
+++ b/kernel/module_signature.c
@@ -8,14 +8,36 @@
 
 #include <linux/errno.h>
 #include <linux/printk.h>
+#include <linux/string.h>
 #include <linux/module_signature.h>
 #include <asm/byteorder.h>
 
+/**
+ * mod_check_sig_marker - check that the given data has signature marker at the end
+ *
+ * @data:	Data with appended signature
+ * @len:	Length of data. Signature marker length is subtracted on success.
+ */
+static inline int mod_check_sig_marker(const void *data, size_t *len)
+{
+	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+
+	if (markerlen > *len)
+		return -ENODATA;
+
+	if (memcmp(data + *len - markerlen, MODULE_SIG_STRING,
+		   markerlen))
+		return -ENODATA;
+
+	*len -= markerlen;
+	return 0;
+}
+
 /**
  * mod_check_sig - check that the given signature is sane
  *
  * @ms:		Signature to check.
- * @file_len:	Size of the file to which @ms is appended.
+ * @file_len:	Size of the file to which @ms is appended (without the marker).
  * @name:	What is being checked. Used for error messages.
  */
 int mod_check_sig(const struct module_signature *ms, size_t file_len,
@@ -44,3 +66,35 @@ int mod_check_sig(const struct module_signature *ms, size_t file_len,
 
 	return 0;
 }
+
+/**
+ * mod_parse_sig - check that the given signature is sane and determine signature length
+ *
+ * @data:	Data with appended signature.
+ * @len:	Length of data. Signature and marker length is subtracted on success.
+ * @sig_len:	Length of signature. Filled on success.
+ * @name:	What is being checked. Used for error messages.
+ */
+int mod_parse_sig(const void *data, size_t *len, size_t *sig_len, const char *name)
+{
+	const struct module_signature *sig;
+	int rc;
+
+	rc = mod_check_sig_marker(data, len);
+	if (rc)
+		return rc;
+
+	if (*len < sizeof(*sig))
+		return -ENODATA;
+
+	sig = (const struct module_signature *)(data + (*len - sizeof(*sig)));
+
+	rc = mod_check_sig(sig, *len, name);
+	if (rc)
+		return rc;
+
+	*sig_len = be32_to_cpu(sig->sig_len);
+	*len -= *sig_len + sizeof(*sig);
+
+	return 0;
+}
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index cef72a6f6b5d..02bbca90f467 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -25,35 +25,17 @@ int verify_appended_signature(const void *data, size_t *len,
 			      struct key *trusted_keys,
 			      enum key_being_used_for purpose)
 {
-	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature ms;
-	size_t sig_len, modlen = *len;
+	size_t sig_len;
 	int ret;
 
-	pr_devel("==>%s %s(,%zu)\n", __func__, key_being_used_for[purpose], modlen);
+	pr_devel("==>%s %s(,%zu)\n", __func__, key_being_used_for[purpose], *len);
 
-	if (markerlen > modlen)
-		return -ENODATA;
-
-	if (memcmp(data + modlen - markerlen, MODULE_SIG_STRING,
-		   markerlen))
-		return -ENODATA;
-	modlen -= markerlen;
-
-	if (modlen <= sizeof(ms))
-		return -EBADMSG;
-
-	memcpy(&ms, data + (modlen - sizeof(ms)), sizeof(ms));
-
-	ret = mod_check_sig(&ms, modlen, key_being_used_for[purpose]);
+	ret = mod_parse_sig(data, len, &sig_len, key_being_used_for[purpose]);
 	if (ret)
 		return ret;
 
-	sig_len = be32_to_cpu(ms.sig_len);
-	modlen -= sig_len + sizeof(ms);
-	*len = modlen;
-
-	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
+	return verify_pkcs7_signature(data, *len, data + *len, sig_len,
 				      trusted_keys,
 				      purpose,
 				      NULL, NULL);
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index fb25723c65bc..46917eb37fd8 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -37,33 +37,17 @@ struct modsig {
  *
  * Return: 0 on success, error code otherwise.
  */
-int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t len,
 		    struct modsig **modsig)
 {
-	const size_t marker_len = strlen(MODULE_SIG_STRING);
-	const struct module_signature *sig;
 	struct modsig *hdr;
-	size_t sig_len;
-	const void *p;
+	size_t sig_len, buf_len = len;
 	int rc;
 
-	if (buf_len <= marker_len + sizeof(*sig))
-		return -ENOENT;
-
-	p = buf + buf_len - marker_len;
-	if (memcmp(p, MODULE_SIG_STRING, marker_len))
-		return -ENOENT;
-
-	buf_len -= marker_len;
-	sig = (const struct module_signature *)(p - sizeof(*sig));
-
-	rc = mod_check_sig(sig, buf_len, func_tokens[func]);
+	rc = mod_parse_sig(buf, &buf_len, &sig_len, func_tokens[func]);
 	if (rc)
 		return rc;
 
-	sig_len = be32_to_cpu(sig->sig_len);
-	buf_len -= sig_len + sizeof(*sig);
-
 	/* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */
 	hdr = kzalloc(sizeof(*hdr) + sig_len, GFP_KERNEL);
 	if (!hdr)
-- 
2.31.1

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Daniel Axtens]

Michal Suchanek <msuchanek@suse.de> writes:

> S390 uses appended signature for kernel but implements the check
> separately from module loader.
>
> Support for secure boot on powerpc with appended signature is planned -
> grub patches submitted upstream but not yet merged.

Power Non-Virtualised / OpenPower already supports secure boot via kexec
with signature verification via IMA. I think you have now sent a
follow-up series that merges some of the IMA implementation, I just
wanted to make sure it was clear that we actually already have support
for this in the kernel, it's just grub that is getting new support.

> This is an attempt at unified appended signature verification.

I am always in favour of fewer reimplementations of the same feature in
the kernel :)

Regards,
Daniel

>
> Thanks
>
> Michal
>
> Michal Suchanek (3):
>   s390/kexec_file: Don't opencode appended signature verification.
>   module: strip the signature marker in the verification function.
>   powerpc/kexec_file: Add KEXEC_SIG support.
>
>  arch/powerpc/Kconfig                  | 11 +++++++
>  arch/powerpc/kexec/elf_64.c           | 14 +++++++++
>  arch/s390/kernel/machine_kexec_file.c | 42 +++------------------------
>  include/linux/verification.h          |  3 ++
>  kernel/module-internal.h              |  2 --
>  kernel/module.c                       | 11 +++----
>  kernel/module_signing.c               | 32 ++++++++++++++------
>  7 files changed, 59 insertions(+), 56 deletions(-)
>
> -- 
> 2.31.1

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Michal Suchánek]

On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote:
> Michal Suchanek <msuchanek@suse.de> writes:
> 
> > S390 uses appended signature for kernel but implements the check
> > separately from module loader.
> >
> > Support for secure boot on powerpc with appended signature is planned -
> > grub patches submitted upstream but not yet merged.
> 
> Power Non-Virtualised / OpenPower already supports secure boot via kexec
> with signature verification via IMA. I think you have now sent a
> follow-up series that merges some of the IMA implementation, I just
> wanted to make sure it was clear that we actually already have support

So is IMA_KEXEC and KEXEC_SIG redundant?

I see some architectures have both. I also see there is a lot of overlap
between the IMA framework and the KEXEC_SIG and MODULE_SIg.

Thanks

Michal

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Daniel Axtens]

Michal Suchánek <msuchanek@suse.de> writes:

> On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote:
>> Michal Suchanek <msuchanek@suse.de> writes:
>> 
>> > S390 uses appended signature for kernel but implements the check
>> > separately from module loader.
>> >
>> > Support for secure boot on powerpc with appended signature is planned -
>> > grub patches submitted upstream but not yet merged.
>> 
>> Power Non-Virtualised / OpenPower already supports secure boot via kexec
>> with signature verification via IMA. I think you have now sent a
>> follow-up series that merges some of the IMA implementation, I just
>> wanted to make sure it was clear that we actually already have support
>
> So is IMA_KEXEC and KEXEC_SIG redundant?
>
> I see some architectures have both. I also see there is a lot of overlap
> between the IMA framework and the KEXEC_SIG and MODULE_SIg.


Mimi would be much better placed than me to answer this.

The limits of my knowledge are basically that signature verification for
modules and kexec kernels can be enforced by IMA policies.

For example a secure booted powerpc kernel with module support will have
the following IMA policy set at the arch level:

"appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig",
(in arch/powerpc/kernel/ima_arch.c)

Module signature enforcement can be set with either IMA (policy like
"appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig" )
or with CONFIG_MODULE_SIG_FORCE/module.sig_enforce=1.

Sometimes this leads to arguably unexpected interactions - for example
commit fa4f3f56ccd2 ("powerpc/ima: Fix secure boot rules in ima arch
policy"), so it might be interesting to see if we can make things easier
to understand.  I suspect another amusing interaction is that if the IMA
verification is used, the signature could be in an xattr rather than an
appended signature.

Kind regards,
Daniel

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Michal Suchánek]

Hello,

On Mon, Nov 08, 2021 at 09:18:56AM +1100, Daniel Axtens wrote:
> Michal Suchánek <msuchanek@suse.de> writes:
> 
> > On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote:
> >> Michal Suchanek <msuchanek@suse.de> writes:
> >> 
> >> > S390 uses appended signature for kernel but implements the check
> >> > separately from module loader.
> >> >
> >> > Support for secure boot on powerpc with appended signature is planned -
> >> > grub patches submitted upstream but not yet merged.
> >> 
> >> Power Non-Virtualised / OpenPower already supports secure boot via kexec
> >> with signature verification via IMA. I think you have now sent a
> >> follow-up series that merges some of the IMA implementation, I just
> >> wanted to make sure it was clear that we actually already have support
> >
> > So is IMA_KEXEC and KEXEC_SIG redundant?
> >
> > I see some architectures have both. I also see there is a lot of overlap
> > between the IMA framework and the KEXEC_SIG and MODULE_SIg.
> 
> 
> Mimi would be much better placed than me to answer this.
> 
> The limits of my knowledge are basically that signature verification for
> modules and kexec kernels can be enforced by IMA policies.
> 
> For example a secure booted powerpc kernel with module support will have
> the following IMA policy set at the arch level:
> 
> "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig",
> (in arch/powerpc/kernel/ima_arch.c)
> 
> Module signature enforcement can be set with either IMA (policy like
> "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig" )
> or with CONFIG_MODULE_SIG_FORCE/module.sig_enforce=1.
> 
> Sometimes this leads to arguably unexpected interactions - for example
> commit fa4f3f56ccd2 ("powerpc/ima: Fix secure boot rules in ima arch
> policy"), so it might be interesting to see if we can make things easier
> to understand.

I suspect that is the root of the problem here. Until distributions pick
up IMA and properly document step by step in detail how to implement,
enable, and debug it the _SIG options are required for users to be able
to make use of signatures.

The other part is that distributions apply 'lockdown' patches that change
the security policy depending on secure boot status which were rejected
by upstream which only hook into the _SIG options, and not into the IMA_
options. Of course, I expect this to change when the IMA options are
universally available across architectures and the support picked up by
distributions.

Which brings the third point: IMA features vary across architectures,
and KEXEC_SIG is more common than IMA_KEXEC.

config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y
config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y

config/arm64/default:CONFIG_KEXEC_SIG=y
config/s390x/default:CONFIG_KEXEC_SIG=y
config/x86_64/default:CONFIG_KEXEC_SIG=y

KEXEC_SIG makes it much easier to get uniform features across
architectures.

Thanks

Michal

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Nayna]

On 11/5/21 09:14, Michal Suchánek wrote:
> On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote:
>> Michal Suchanek <msuchanek@suse.de> writes:
>>
>>> S390 uses appended signature for kernel but implements the check
>>> separately from module loader.
>>>
>>> Support for secure boot on powerpc with appended signature is planned -
>>> grub patches submitted upstream but not yet merged.
>> Power Non-Virtualised / OpenPower already supports secure boot via kexec
>> with signature verification via IMA. I think you have now sent a
>> follow-up series that merges some of the IMA implementation, I just
>> wanted to make sure it was clear that we actually already have support
> So is IMA_KEXEC and KEXEC_SIG redundant?
>
> I see some architectures have both. I also see there is a lot of overlap
> between the IMA framework and the KEXEC_SIG and MODULE_SIg.

Originally, KEXEC_SIG was meant for PECOFF based signatures, while 
IMA_KEXEC mainly supported xattr based signatures.

Power (Non-virtualized/OpenPOWER) doesn't support PECOFF. Extended 
attributes based signature verification doesn't work with netboot. 
That's when appended signature support was added to IMA.

Using IMA_KEXEC has the benefit of being able to enable both signature 
verification and measurement of the kernel image.

Thanks & Regards,

      - Nayna

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Nayna]

On 11/8/21 07:05, Michal Suchánek wrote:
> Hello,
>
> On Mon, Nov 08, 2021 at 09:18:56AM +1100, Daniel Axtens wrote:
>> Michal Suchánek <msuchanek@suse.de> writes:
>>
>>> On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote:
>>>> Michal Suchanek <msuchanek@suse.de> writes:
>>>>
>>>>> S390 uses appended signature for kernel but implements the check
>>>>> separately from module loader.
>>>>>
>>>>> Support for secure boot on powerpc with appended signature is planned -
>>>>> grub patches submitted upstream but not yet merged.
>>>> Power Non-Virtualised / OpenPower already supports secure boot via kexec
>>>> with signature verification via IMA. I think you have now sent a
>>>> follow-up series that merges some of the IMA implementation, I just
>>>> wanted to make sure it was clear that we actually already have support
>>> So is IMA_KEXEC and KEXEC_SIG redundant?
>>>
>>> I see some architectures have both. I also see there is a lot of overlap
>>> between the IMA framework and the KEXEC_SIG and MODULE_SIg.
>>
>> Mimi would be much better placed than me to answer this.
>>
>> The limits of my knowledge are basically that signature verification for
>> modules and kexec kernels can be enforced by IMA policies.
>>
>> For example a secure booted powerpc kernel with module support will have
>> the following IMA policy set at the arch level:
>>
>> "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig",
>> (in arch/powerpc/kernel/ima_arch.c)
>>
>> Module signature enforcement can be set with either IMA (policy like
>> "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig" )
>> or with CONFIG_MODULE_SIG_FORCE/module.sig_enforce=1.
>>
>> Sometimes this leads to arguably unexpected interactions - for example
>> commit fa4f3f56ccd2 ("powerpc/ima: Fix secure boot rules in ima arch
>> policy"), so it might be interesting to see if we can make things easier
>> to understand.
> I suspect that is the root of the problem here. Until distributions pick
> up IMA and properly document step by step in detail how to implement,
> enable, and debug it the _SIG options are required for users to be able
> to make use of signatures.

For secureboot, IMA appraisal policies are configured in kernel at boot 
time based on secureboot state of the system, refer 
arch/powerpc/kernel/ima_arch.c and security/integrity/ima/ima_efi.c. 
This doesn't require any user configuration. Yes, I agree it would be 
helpful to update kernel documentation specifying steps to sign the 
kernel image using sign-file.

>
> The other part is that distributions apply 'lockdown' patches that change
> the security policy depending on secure boot status which were rejected
> by upstream which only hook into the _SIG options, and not into the IMA_
> options. Of course, I expect this to change when the IMA options are
> universally available across architectures and the support picked up by
> distributions.
>
> Which brings the third point: IMA features vary across architectures,
> and KEXEC_SIG is more common than IMA_KEXEC.
>
> config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y
> config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y
>
> config/arm64/default:CONFIG_KEXEC_SIG=y
> config/s390x/default:CONFIG_KEXEC_SIG=y
> config/x86_64/default:CONFIG_KEXEC_SIG=y
>
> KEXEC_SIG makes it much easier to get uniform features across
> architectures.

Architectures use KEXEC_SIG vs IMA_KEXEC based on their requirement. 
IMA_KEXEC is for the kernel images signed using sign-file (appended 
signatures, not PECOFF), provides measurement along with verification, 
and is tied to secureboot state of the system at boot time.

Thanks & Regards,

       - Nayna

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Michal Suchánek]

Hello,

On Thu, Nov 11, 2021 at 05:26:41PM -0500, Nayna wrote:
> 
> On 11/8/21 07:05, Michal Suchánek wrote:
> > Hello,
> > 
> > On Mon, Nov 08, 2021 at 09:18:56AM +1100, Daniel Axtens wrote:
> > > Michal Suchánek <msuchanek@suse.de> writes:
> > > 
> > > > On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote:
> > > > > Michal Suchanek <msuchanek@suse.de> writes:
> > > > > 
> > > > > > S390 uses appended signature for kernel but implements the check
> > > > > > separately from module loader.
> > > > > > 
> > > > > > Support for secure boot on powerpc with appended signature is planned -
> > > > > > grub patches submitted upstream but not yet merged.
> > > > > Power Non-Virtualised / OpenPower already supports secure boot via kexec
> > > > > with signature verification via IMA. I think you have now sent a
> > > > > follow-up series that merges some of the IMA implementation, I just
> > > > > wanted to make sure it was clear that we actually already have support
> > > > So is IMA_KEXEC and KEXEC_SIG redundant?
> > > > 
> > > > I see some architectures have both. I also see there is a lot of overlap
> > > > between the IMA framework and the KEXEC_SIG and MODULE_SIg.
> > > 
> > > Mimi would be much better placed than me to answer this.
> > > 
> > > The limits of my knowledge are basically that signature verification for
> > > modules and kexec kernels can be enforced by IMA policies.
> > > 
> > > For example a secure booted powerpc kernel with module support will have
> > > the following IMA policy set at the arch level:
> > > 
> > > "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig",
> > > (in arch/powerpc/kernel/ima_arch.c)
> > > 
> > > Module signature enforcement can be set with either IMA (policy like
> > > "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig" )
> > > or with CONFIG_MODULE_SIG_FORCE/module.sig_enforce=1.
> > > 
> > > Sometimes this leads to arguably unexpected interactions - for example
> > > commit fa4f3f56ccd2 ("powerpc/ima: Fix secure boot rules in ima arch
> > > policy"), so it might be interesting to see if we can make things easier
> > > to understand.
> > I suspect that is the root of the problem here. Until distributions pick
> > up IMA and properly document step by step in detail how to implement,
> > enable, and debug it the _SIG options are required for users to be able
> > to make use of signatures.
> 
> For secureboot, IMA appraisal policies are configured in kernel at boot time
> based on secureboot state of the system, refer
> arch/powerpc/kernel/ima_arch.c and security/integrity/ima/ima_efi.c. This
> doesn't require any user configuration. Yes, I agree it would be helpful to
> update kernel documentation specifying steps to sign the kernel image using
> sign-file.
> 
> > 
> > The other part is that distributions apply 'lockdown' patches that change
> > the security policy depending on secure boot status which were rejected
> > by upstream which only hook into the _SIG options, and not into the IMA_
> > options. Of course, I expect this to change when the IMA options are
> > universally available across architectures and the support picked up by
> > distributions.
> > 
> > Which brings the third point: IMA features vary across architectures,
> > and KEXEC_SIG is more common than IMA_KEXEC.
> > 
> > config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y
> > config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y
> > 
> > config/arm64/default:CONFIG_KEXEC_SIG=y
> > config/s390x/default:CONFIG_KEXEC_SIG=y
> > config/x86_64/default:CONFIG_KEXEC_SIG=y
> > 
> > KEXEC_SIG makes it much easier to get uniform features across
> > architectures.
> 
> Architectures use KEXEC_SIG vs IMA_KEXEC based on their requirement.
> IMA_KEXEC is for the kernel images signed using sign-file (appended
> signatures, not PECOFF), provides measurement along with verification, and

That's certainly not the case. S390 uses appended signatures with
KEXEC_SIG, arm64 uses PECOFF with both KEXEC_SIG and IMA_KEXEC.

> is tied to secureboot state of the system at boot time.

In distrubutions it's also the case with KEXEC_SIG, it's only upstream
where this is different. I don't know why Linux upstream has rejected
this support for KEXEC_SIG.

Anyway, sounds like the difference is that IMA provides measurement but
if you don't use it it does not makes any difference except more comlex
code.

Thanks

Michal

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Nayna]

On 11/12/21 03:30, Michal Suchánek wrote:
> Hello,
>
> On Thu, Nov 11, 2021 at 05:26:41PM -0500, Nayna wrote:
>> On 11/8/21 07:05, Michal Suchánek wrote:
>>> Hello,
>>>
>>> On Mon, Nov 08, 2021 at 09:18:56AM +1100, Daniel Axtens wrote:
>>>> Michal Suchánek <msuchanek@suse.de> writes:
>>>>
>>>>> On Fri, Nov 05, 2021 at 09:55:52PM +1100, Daniel Axtens wrote:
>>>>>> Michal Suchanek <msuchanek@suse.de> writes:
>>>>>>
>>>>>>> S390 uses appended signature for kernel but implements the check
>>>>>>> separately from module loader.
>>>>>>>
>>>>>>> Support for secure boot on powerpc with appended signature is planned -
>>>>>>> grub patches submitted upstream but not yet merged.
>>>>>> Power Non-Virtualised / OpenPower already supports secure boot via kexec
>>>>>> with signature verification via IMA. I think you have now sent a
>>>>>> follow-up series that merges some of the IMA implementation, I just
>>>>>> wanted to make sure it was clear that we actually already have support
>>>>> So is IMA_KEXEC and KEXEC_SIG redundant?
>>>>>
>>>>> I see some architectures have both. I also see there is a lot of overlap
>>>>> between the IMA framework and the KEXEC_SIG and MODULE_SIg.
>>>> Mimi would be much better placed than me to answer this.
>>>>
>>>> The limits of my knowledge are basically that signature verification for
>>>> modules and kexec kernels can be enforced by IMA policies.
>>>>
>>>> For example a secure booted powerpc kernel with module support will have
>>>> the following IMA policy set at the arch level:
>>>>
>>>> "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig",
>>>> (in arch/powerpc/kernel/ima_arch.c)
>>>>
>>>> Module signature enforcement can be set with either IMA (policy like
>>>> "appraise func=MODULE_CHECK appraise_flag=check_blacklist appraise_type=imasig|modsig" )
>>>> or with CONFIG_MODULE_SIG_FORCE/module.sig_enforce=1.
>>>>
>>>> Sometimes this leads to arguably unexpected interactions - for example
>>>> commit fa4f3f56ccd2 ("powerpc/ima: Fix secure boot rules in ima arch
>>>> policy"), so it might be interesting to see if we can make things easier
>>>> to understand.
>>> I suspect that is the root of the problem here. Until distributions pick
>>> up IMA and properly document step by step in detail how to implement,
>>> enable, and debug it the _SIG options are required for users to be able
>>> to make use of signatures.
>> For secureboot, IMA appraisal policies are configured in kernel at boot time
>> based on secureboot state of the system, refer
>> arch/powerpc/kernel/ima_arch.c and security/integrity/ima/ima_efi.c. This
>> doesn't require any user configuration. Yes, I agree it would be helpful to
>> update kernel documentation specifying steps to sign the kernel image using
>> sign-file.
>>
>>> The other part is that distributions apply 'lockdown' patches that change
>>> the security policy depending on secure boot status which were rejected
>>> by upstream which only hook into the _SIG options, and not into the IMA_
>>> options. Of course, I expect this to change when the IMA options are
>>> universally available across architectures and the support picked up by
>>> distributions.
>>>
>>> Which brings the third point: IMA features vary across architectures,
>>> and KEXEC_SIG is more common than IMA_KEXEC.
>>>
>>> config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y
>>> config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y
>>>
>>> config/arm64/default:CONFIG_KEXEC_SIG=y
>>> config/s390x/default:CONFIG_KEXEC_SIG=y
>>> config/x86_64/default:CONFIG_KEXEC_SIG=y
>>>
>>> KEXEC_SIG makes it much easier to get uniform features across
>>> architectures.
>> Architectures use KEXEC_SIG vs IMA_KEXEC based on their requirement.
>> IMA_KEXEC is for the kernel images signed using sign-file (appended
>> signatures, not PECOFF), provides measurement along with verification, and
> That's certainly not the case. S390 uses appended signatures with
> KEXEC_SIG, arm64 uses PECOFF with both KEXEC_SIG and IMA_KEXEC.

Yes, S390 uses appended signature, but they also do not support 
measurements.

On the other hand for arm64/x86, PECOFF works only with KEXEC_SIG. Look 
at the KEXEC_IMAGE_VERIFY_SIG config dependencies in arch/arm64/Kconfig 
and KEXEC_BZIMAGE_VERIFY_SIG config dependencies in arch/x86/Kconfig. 
Now, if KEXEC_SIG is not enabled, then IMA appraisal policies are 
enforced if secure boot is enabled, refer to 
security/integrity/ima_efi.c . IMA would fail verification if kernel is 
not signed with module sig appended signatures or signature verification 
fails.

In short, IMA is used to enforce the existence of a policy if secure 
boot is enabled. If they don't support module sig appended signatures, 
by definition it fails. Thus PECOFF doesn't work with both KEXEC_SIG and 
IMA_KEXEC, but only with KEXEC_SIG.

Lakshmi, do you agree with my reasoning ?

>
>> is tied to secureboot state of the system at boot time.
> In distrubutions it's also the case with KEXEC_SIG, it's only upstream
> where this is different. I don't know why Linux upstream has rejected
> this support for KEXEC_SIG.
>
> Anyway, sounds like the difference is that IMA provides measurement but
> if you don't use it it does not makes any difference except more comlex
> code.
I am unsure what do you mean by "complex code" here. Can you please 
elaborate ? IMA policies support for secureboot already exists and can 
be used as it is without adding any extra work as in 
arch/powerpc/kernel/ima_arch.c.

Also, if my analysis is right, I think I understand arm64/x86 support 
for both KEXEC_SIG and IMA_KEXEC as it can support two signature formats 
- PECOFF/module sig appended signature.

I am not clear from the patch descriptions on the need to add KEXEC_SIG 
support on POWER when that will also be based on module sig appended 
signatures like IMA_KEXEC.

Thanks & Regards,

       - Nayna

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Michal Suchánek]

On Mon, Nov 15, 2021 at 06:53:53PM -0500, Nayna wrote:
> 
> On 11/12/21 03:30, Michal Suchánek wrote:
> > Hello,
> > 
> > On Thu, Nov 11, 2021 at 05:26:41PM -0500, Nayna wrote:
> > > On 11/8/21 07:05, Michal Suchánek wrote:
> > > > Hello,
> > > > 

> > > > The other part is that distributions apply 'lockdown' patches that change
> > > > the security policy depending on secure boot status which were rejected
> > > > by upstream which only hook into the _SIG options, and not into the IMA_
> > > > options. Of course, I expect this to change when the IMA options are
> > > > universally available across architectures and the support picked up by
> > > > distributions.
> > > > 
> > > > Which brings the third point: IMA features vary across architectures,
> > > > and KEXEC_SIG is more common than IMA_KEXEC.
> > > > 
> > > > config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y
> > > > config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y
> > > > 
> > > > config/arm64/default:CONFIG_KEXEC_SIG=y
> > > > config/s390x/default:CONFIG_KEXEC_SIG=y
> > > > config/x86_64/default:CONFIG_KEXEC_SIG=y
> > > > 
> > > > KEXEC_SIG makes it much easier to get uniform features across
> > > > architectures.
> > > Architectures use KEXEC_SIG vs IMA_KEXEC based on their requirement.
> > > IMA_KEXEC is for the kernel images signed using sign-file (appended
> > > signatures, not PECOFF), provides measurement along with verification, and
> > That's certainly not the case. S390 uses appended signatures with
> > KEXEC_SIG, arm64 uses PECOFF with both KEXEC_SIG and IMA_KEXEC.
> 
> Yes, S390 uses appended signature, but they also do not support
> measurements.
> 
> On the other hand for arm64/x86, PECOFF works only with KEXEC_SIG. Look at
> the KEXEC_IMAGE_VERIFY_SIG config dependencies in arch/arm64/Kconfig and
> KEXEC_BZIMAGE_VERIFY_SIG config dependencies in arch/x86/Kconfig. Now, if
> KEXEC_SIG is not enabled, then IMA appraisal policies are enforced if secure
> boot is enabled, refer to security/integrity/ima_efi.c . IMA would fail
> verification if kernel is not signed with module sig appended signatures or
> signature verification fails.
> 
> In short, IMA is used to enforce the existence of a policy if secure boot is
> enabled. If they don't support module sig appended signatures, by definition
> it fails. Thus PECOFF doesn't work with both KEXEC_SIG and IMA_KEXEC, but
> only with KEXEC_SIG.

Then IMA_KEXEC is a no-go. It is not supported on all architectures and
it principially cannot be supported because it does not support PECOFF
which is needed to boot the kernel on EFI platforms. To get feature
parity across architectures KEXEC_SIG is required.

> > 
> > > is tied to secureboot state of the system at boot time.
> > In distrubutions it's also the case with KEXEC_SIG, it's only upstream
> > where this is different. I don't know why Linux upstream has rejected
> > this support for KEXEC_SIG.
> > 
> > Anyway, sounds like the difference is that IMA provides measurement but
> > if you don't use it it does not makes any difference except more comlex
> > code.
> I am unsure what do you mean by "complex code" here. Can you please
> elaborate ? IMA policies support for secureboot already exists and can be
> used as it is without adding any extra work as in
> arch/powerpc/kernel/ima_arch.c.

The code exists but using it to replace KEXEC_SIG also requires
understanding the code and the implications of using it. At a glance the
IMA codebase is much bigger and more convoluted compared to KEXEC_SIG
and MODULE_SIG.

Thanks

Michal

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Nayna]

On 11/16/21 04:53, Michal Suchánek wrote:
> On Mon, Nov 15, 2021 at 06:53:53PM -0500, Nayna wrote:
>> On 11/12/21 03:30, Michal Suchánek wrote:
>>> Hello,
>>>
>>> On Thu, Nov 11, 2021 at 05:26:41PM -0500, Nayna wrote:
>>>> On 11/8/21 07:05, Michal Suchánek wrote:
>>>>> Hello,
>>>>>
>>>>> The other part is that distributions apply 'lockdown' patches that change
>>>>> the security policy depending on secure boot status which were rejected
>>>>> by upstream which only hook into the _SIG options, and not into the IMA_
>>>>> options. Of course, I expect this to change when the IMA options are
>>>>> universally available across architectures and the support picked up by
>>>>> distributions.
>>>>>
>>>>> Which brings the third point: IMA features vary across architectures,
>>>>> and KEXEC_SIG is more common than IMA_KEXEC.
>>>>>
>>>>> config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y
>>>>> config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y
>>>>>
>>>>> config/arm64/default:CONFIG_KEXEC_SIG=y
>>>>> config/s390x/default:CONFIG_KEXEC_SIG=y
>>>>> config/x86_64/default:CONFIG_KEXEC_SIG=y
>>>>>
>>>>> KEXEC_SIG makes it much easier to get uniform features across
>>>>> architectures.
>>>> Architectures use KEXEC_SIG vs IMA_KEXEC based on their requirement.
>>>> IMA_KEXEC is for the kernel images signed using sign-file (appended
>>>> signatures, not PECOFF), provides measurement along with verification, and
>>> That's certainly not the case. S390 uses appended signatures with
>>> KEXEC_SIG, arm64 uses PECOFF with both KEXEC_SIG and IMA_KEXEC.
>> Yes, S390 uses appended signature, but they also do not support
>> measurements.
>>
>> On the other hand for arm64/x86, PECOFF works only with KEXEC_SIG. Look at
>> the KEXEC_IMAGE_VERIFY_SIG config dependencies in arch/arm64/Kconfig and
>> KEXEC_BZIMAGE_VERIFY_SIG config dependencies in arch/x86/Kconfig. Now, if
>> KEXEC_SIG is not enabled, then IMA appraisal policies are enforced if secure
>> boot is enabled, refer to security/integrity/ima_efi.c . IMA would fail
>> verification if kernel is not signed with module sig appended signatures or
>> signature verification fails.
>>
>> In short, IMA is used to enforce the existence of a policy if secure boot is
>> enabled. If they don't support module sig appended signatures, by definition
>> it fails. Thus PECOFF doesn't work with both KEXEC_SIG and IMA_KEXEC, but
>> only with KEXEC_SIG.
> Then IMA_KEXEC is a no-go. It is not supported on all architectures and
> it principially cannot be supported because it does not support PECOFF
> which is needed to boot the kernel on EFI platforms. To get feature
> parity across architectures KEXEC_SIG is required.

I would not say "a no-go", it is based on user requirements.

The key takeaway from this discussion is that both KEXEC_SIG and 
IMA_KEXEC support functionality with some small degree of overlap, and 
that documenting the differences is needed.  This will help kernel 
consumers to understand the difference and enable the appropriate 
functionality for their environment.

As per my understanding:

KEXEC_SIG:
* Supports kernel image verification
* Linked with secureboot state using downstream patch
* Supports PECOFF and module sig appended signature format
* Supports blocklisting of keys

IMA_KEXEC:
* Supports kernel image verification
* Linked with secureboot state in upstream
* Supports module sig appended signature format and signatures in 
extended attribute.
* Supports blocklisting of keys
* Supports blocklisting single kernel binary
* Supports measurements for attestation
* Supports audit log

Users can enable the option based on their requirements.

Thanks for the good discussion and enabling KEXEC_SIG for POWER as well. 
It would be good to have updated kernel documentation to go along with 
KEXEC_SIG support in the patchset.

Thanks & Regards,
     - Nayna

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Michal Suchánek]

Hello,

On Thu, Nov 18, 2021 at 05:34:01PM -0500, Nayna wrote:
> 
> On 11/16/21 04:53, Michal Suchánek wrote:
> > On Mon, Nov 15, 2021 at 06:53:53PM -0500, Nayna wrote:
> > > On 11/12/21 03:30, Michal Suchánek wrote:
> > > > Hello,
> > > > 
> > > > On Thu, Nov 11, 2021 at 05:26:41PM -0500, Nayna wrote:
> > > > > On 11/8/21 07:05, Michal Suchánek wrote:
> > > > > > Hello,
> > > > > > 
> > > > > > The other part is that distributions apply 'lockdown' patches that change
> > > > > > the security policy depending on secure boot status which were rejected
> > > > > > by upstream which only hook into the _SIG options, and not into the IMA_
> > > > > > options. Of course, I expect this to change when the IMA options are
> > > > > > universally available across architectures and the support picked up by
> > > > > > distributions.
> > > > > > 
> > > > > > Which brings the third point: IMA features vary across architectures,
> > > > > > and KEXEC_SIG is more common than IMA_KEXEC.
> > > > > > 
> > > > > > config/arm64/default:CONFIG_HAVE_IMA_KEXEC=y
> > > > > > config/ppc64le/default:CONFIG_HAVE_IMA_KEXEC=y
> > > > > > 
> > > > > > config/arm64/default:CONFIG_KEXEC_SIG=y
> > > > > > config/s390x/default:CONFIG_KEXEC_SIG=y
> > > > > > config/x86_64/default:CONFIG_KEXEC_SIG=y
> > > > > > 
> > > > > > KEXEC_SIG makes it much easier to get uniform features across
> > > > > > architectures.
> > > > > Architectures use KEXEC_SIG vs IMA_KEXEC based on their requirement.
> > > > > IMA_KEXEC is for the kernel images signed using sign-file (appended
> > > > > signatures, not PECOFF), provides measurement along with verification, and
> > > > That's certainly not the case. S390 uses appended signatures with
> > > > KEXEC_SIG, arm64 uses PECOFF with both KEXEC_SIG and IMA_KEXEC.
> > > Yes, S390 uses appended signature, but they also do not support
> > > measurements.
> > > 
> > > On the other hand for arm64/x86, PECOFF works only with KEXEC_SIG. Look at
> > > the KEXEC_IMAGE_VERIFY_SIG config dependencies in arch/arm64/Kconfig and
> > > KEXEC_BZIMAGE_VERIFY_SIG config dependencies in arch/x86/Kconfig. Now, if
> > > KEXEC_SIG is not enabled, then IMA appraisal policies are enforced if secure
> > > boot is enabled, refer to security/integrity/ima_efi.c . IMA would fail
> > > verification if kernel is not signed with module sig appended signatures or
> > > signature verification fails.
> > > 
> > > In short, IMA is used to enforce the existence of a policy if secure boot is
> > > enabled. If they don't support module sig appended signatures, by definition
> > > it fails. Thus PECOFF doesn't work with both KEXEC_SIG and IMA_KEXEC, but
> > > only with KEXEC_SIG.
> > Then IMA_KEXEC is a no-go. It is not supported on all architectures and
> > it principially cannot be supported because it does not support PECOFF
> > which is needed to boot the kernel on EFI platforms. To get feature
> > parity across architectures KEXEC_SIG is required.
> 
> I would not say "a no-go", it is based on user requirements.
> 
> The key takeaway from this discussion is that both KEXEC_SIG and IMA_KEXEC
> support functionality with some small degree of overlap, and that
> documenting the differences is needed.  This will help kernel consumers to
> understand the difference and enable the appropriate functionality for their
> environment.

Maybe I was not clear enough. If you happen to focus on an architecture
that supports IMA fully it's great.

My point of view is maintaining multiple architectures. Both end users
and people conecerend with security are rarely familiar with
architecture specifics. Portability of documentation and debugging
instructions across architectures is a concern.

IMA has large number of options with varying availablitily across
architectures for no apparent reason. The situation is complex and hard
to grasp.

In comparison the *_SIG options are widely available. The missing
support for KEXEC_SIG on POWER is trivial to add by cut&paste from s390.
With that all the documentation that exists already is also trivially
applicable to POWER. Any additional code cleanup is a bonus but not
really needed to enable the kexec lockdown on POWER.

Thanks

Michal

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Mimi Zohar]

On Fri, 2021-11-19 at 12:18 +0100, Michal Suchánek wrote:
> Maybe I was not clear enough. If you happen to focus on an architecture
> that supports IMA fully it's great.
> 
> My point of view is maintaining multiple architectures. Both end users
> and people conecerend with security are rarely familiar with
> architecture specifics. Portability of documentation and debugging
> instructions across architectures is a concern.
> 
> IMA has large number of options with varying availablitily across
> architectures for no apparent reason. The situation is complex and hard
> to grasp.

IMA measures, verifies, and audits the integrity of files based on a
system wide policy.  The known "good" integrity value may be stored in
the security.ima xattr or more recently as an appended signature.

With both IMA kexec appraise and measurement policy rules, not only is
the kernel image signature verified and the file hash included in the
IMA measurement list, but the signature used to verify the integrity of
the kexec kernel image is also included in the IMA measurement list
(ima_template=ima-sig).

Even without PECOFF support in IMA, IMA kexec measurement policy rules
can be defined to supplement the KEXEC_SIG signature verfication.

> 
> In comparison the *_SIG options are widely available. The missing
> support for KEXEC_SIG on POWER is trivial to add by cut&paste from s390.
> With that all the documentation that exists already is also trivially
> applicable to POWER. Any additional code cleanup is a bonus but not
> really needed to enable the kexec lockdown on POWER.

Before lockdown was upstreamed, Matthew made sure that IMA signature
verification could co-exist.   Refer to commit 29d3c1c8dfe7 ("kexec:
Allow kexec_file() with appropriate IMA policy when locked down").   If
there is a problem with the downstream kexec lockdown patches, they
should be fixed.

The kexec kselftest might provide some insight into how the different
signature verification methods and lockdown co-exist.

As for adding KEXEC_SIG appended signature support on PowerPC based on
the s390 code, it sounds reasonable.

thanks,

Mimi

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Philipp Rudo]

Hi Mimi,

On Fri, 19 Nov 2021 13:16:20 -0500
Mimi Zohar <zohar@linux.ibm.com> wrote:

> On Fri, 2021-11-19 at 12:18 +0100, Michal Suchánek wrote:
> > Maybe I was not clear enough. If you happen to focus on an architecture
> > that supports IMA fully it's great.
> > 
> > My point of view is maintaining multiple architectures. Both end users
> > and people conecerend with security are rarely familiar with
> > architecture specifics. Portability of documentation and debugging
> > instructions across architectures is a concern.
> > 
> > IMA has large number of options with varying availablitily across
> > architectures for no apparent reason. The situation is complex and hard
> > to grasp.  
> 
> IMA measures, verifies, and audits the integrity of files based on a
> system wide policy.  The known "good" integrity value may be stored in
> the security.ima xattr or more recently as an appended signature.
> 
> With both IMA kexec appraise and measurement policy rules, not only is
> the kernel image signature verified and the file hash included in the
> IMA measurement list, but the signature used to verify the integrity of
> the kexec kernel image is also included in the IMA measurement list
> (ima_template=ima-sig).
> 
> Even without PECOFF support in IMA, IMA kexec measurement policy rules
> can be defined to supplement the KEXEC_SIG signature verfication.
>
> > In comparison the *_SIG options are widely available. The missing
> > support for KEXEC_SIG on POWER is trivial to add by cut&paste from s390.
> > With that all the documentation that exists already is also trivially
> > applicable to POWER. Any additional code cleanup is a bonus but not
> > really needed to enable the kexec lockdown on POWER.  
> 
> Before lockdown was upstreamed, Matthew made sure that IMA signature
> verification could co-exist.   Refer to commit 29d3c1c8dfe7 ("kexec:
> Allow kexec_file() with appropriate IMA policy when locked down").   If
> there is a problem with the downstream kexec lockdown patches, they
> should be fixed.
> 
> The kexec kselftest might provide some insight into how the different
> signature verification methods and lockdown co-exist.
> 
> As for adding KEXEC_SIG appended signature support on PowerPC based on
> the s390 code, it sounds reasonable.

Heiko contacted me as you apparently requested that someone from s390
takes part in this discussion. I now spend over a day trying to make
any sens from this discussion but failed. The way I see it is.

On one hand there is KEXEC_SIG which is specifically designed to verify
the signatures of kernel images in kexec_file_load. From the beginning
it was designed in a way that every architecture (in fact even every
image type) can define its own callback function as needed. It's design
is simple and easy to extend and thus was adopted by all architectures,
except ppc, so far.

On the other hand there is IMA which is much more general and also
includes the same functionality like KEXEC_SIG but only on some
architectures in some special cases and without proper documentation.

Now Michal wants to adapt KEXEC_SIG for ppc too so distros can rely on all
architectures using the same mechanism and thus reduce maintenance cost.
On the way there he even makes some absolutely reasonable improvements
for everybody.

Why is that so controversial? What is the real problem that should be
discussed here?

Thanks
Philipp

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Mimi Zohar]

On Wed, 2021-11-24 at 12:09 +0100, Philipp Rudo wrote:
> Now Michal wants to adapt KEXEC_SIG for ppc too so distros can rely on all
> architectures using the same mechanism and thus reduce maintenance cost.
> On the way there he even makes some absolutely reasonable improvements
> for everybody.
> 
> Why is that so controversial? What is the real problem that should be
> discussed here?

Nothing is controversial with what Michal wants to do.  I've already
said, "As for adding KEXEC_SIG appended signature support on PowerPC
based on the s390 code, it sounds reasonable."

thanks,

Mimi

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Michal Suchánek]

On Wed, Nov 24, 2021 at 08:10:10AM -0500, Mimi Zohar wrote:
> On Wed, 2021-11-24 at 12:09 +0100, Philipp Rudo wrote:
> > Now Michal wants to adapt KEXEC_SIG for ppc too so distros can rely on all
> > architectures using the same mechanism and thus reduce maintenance cost.
> > On the way there he even makes some absolutely reasonable improvements
> > for everybody.
> > 
> > Why is that so controversial? What is the real problem that should be
> > discussed here?
> 
> Nothing is controversial with what Michal wants to do.  I've already
> said, "As for adding KEXEC_SIG appended signature support on PowerPC
> based on the s390 code, it sounds reasonable."

Ok, I will resend the series with the arch-specific changes first to be
independent of the core cleanup.

Thanks

Michal

Re: [PATCH 0/3] KEXEC_SIG with appended signature

[Philipp Rudo]

Hi Michal,

On Wed, 24 Nov 2021 14:27:16 +0100
Michal Suchánek <msuchanek@suse.de> wrote:

> On Wed, Nov 24, 2021 at 08:10:10AM -0500, Mimi Zohar wrote:
> > On Wed, 2021-11-24 at 12:09 +0100, Philipp Rudo wrote:  
> > > Now Michal wants to adapt KEXEC_SIG for ppc too so distros can rely on all
> > > architectures using the same mechanism and thus reduce maintenance cost.
> > > On the way there he even makes some absolutely reasonable improvements
> > > for everybody.
> > > 
> > > Why is that so controversial? What is the real problem that should be
> > > discussed here?  
> > 
> > Nothing is controversial with what Michal wants to do.  I've already
> > said, "As for adding KEXEC_SIG appended signature support on PowerPC
> > based on the s390 code, it sounds reasonable."  
> 
> Ok, I will resend the series with the arch-specific changes first to be
> independent of the core cleanup.

could you please add the kexec@lists.infradead.org to Cc when you
resend the series. As this is kexec related I think it makes sense to
give them a heads up, too.

Thanks
Philipp