2022-01-07 11:55:50

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH v3 0/6] KEXEC_SIG with appended signature

Hello,

This is a refresh of the KEXEC_SIG series.

This adds KEXEC_SIG support on powerpc and deduplicates the code dealing
with appended signatures in the kernel.

powerpc supports IMA_KEXEC but that's an exception rather than the norm.
On the other hand, KEXEC_SIG is portable across platforms.

For distributions to have uniform security features across platforms one
option should be used on all platforms.

Thanks

Michal

Previous revision: https://lore.kernel.org/linuxppc-dev/[email protected]/
Patched kernel tree: https://github.com/hramrach/kernel/tree/kexec_sig

Michal Suchanek (6):
s390/kexec_file: Don't opencode appended signature check.
powerpc/kexec_file: Add KEXEC_SIG support.
kexec_file: Don't opencode appended signature verification.
module: strip the signature marker in the verification function.
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/Kconfig | 16 +++++++
arch/powerpc/kexec/elf_64.c | 14 ++++++
arch/s390/Kconfig | 2 +-
arch/s390/kernel/machine_kexec_file.c | 43 ++----------------
crypto/asymmetric_keys/asymmetric_type.c | 1 +
include/linux/module_signature.h | 1 +
include/linux/verification.h | 4 ++
kernel/module-internal.h | 2 -
kernel/module.c | 12 +++--
kernel/module_signature.c | 56 +++++++++++++++++++++++-
kernel/module_signing.c | 33 +++++++-------
security/integrity/ima/ima_modsig.c | 22 ++--------
12 files changed, 119 insertions(+), 87 deletions(-)

--
2.31.1



2022-01-07 11:55:57

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH v3 1/6] s390/kexec_file: Don't opencode appended signature check.

Module verification already implements appeded signature check.

Reuse it for kexec_file.

Note: the kexec_file implementation uses EKEYREJECTED error in some
cases when there is no key and the common implementation uses ENOPKG or
EBADMSG instead.

Signed-off-by: Michal Suchanek <[email protected]>
Acked-by: Heiko Carstens <[email protected]>
---
v3: Philipp Rudo <[email protected]>: Update the commit with note about
change of return value
---
arch/s390/kernel/machine_kexec_file.c | 22 +++++-----------------
1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 8f43575a4dd3..c944d71316c7 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -31,6 +31,7 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
struct module_signature *ms;
unsigned long sig_len;
+ int ret;

/* Skip signature verification when not secure IPLed. */
if (!ipl_secure_flag)
@@ -45,25 +46,12 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
kernel_len -= marker_len;

ms = (void *)kernel + kernel_len - sizeof(*ms);
- kernel_len -= sizeof(*ms);
+ ret = mod_check_sig(ms, kernel_len, "kexec");
+ if (ret)
+ return ret;

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;
- }
+ kernel_len -= sizeof(*ms) + sig_len;

return verify_pkcs7_signature(kernel, kernel_len,
kernel + kernel_len, sig_len,
--
2.31.1


2022-01-07 11:55:59

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH v3 2/6] powerpc/kexec_file: Add KEXEC_SIG support.

Copy the code from s390x

Both powerpc and s390x use appended signature format (as opposed to EFI
based patforms using PE format).

Signed-off-by: Michal Suchanek <[email protected]>
---
v3: - Philipp Rudo <[email protected]>: Update the comit message with
explanation why the s390 code is usable on powerpc.
- Include correct header for mod_check_sig
- Nayna <[email protected]>: Mention additional IMA features
in kconfig text
---
arch/powerpc/Kconfig | 16 ++++++++++++++++
arch/powerpc/kexec/elf_64.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index dea74d7717c0..1cde9b6c5987 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -560,6 +560,22 @@ 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.
+
+ Note: on powerpc IMA_ARCH_POLICY also implements kexec'ed kernel
+ verification. In addition IMA adds kernel hashes to the measurement
+ list, extends IMA PCR in the TPM, and implements kernel image
+ blacklist by hash.
+
config RELOCATABLE
bool "Build a relocatable kernel"
depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index eeb258002d1e..98d1cb5135b4 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/module_signature.h>

static void *elf64_load(struct kimage *image, char *kernel_buf,
unsigned long kernel_len, char *initrd,
@@ -151,7 +152,42 @@ 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 kernel_len)
+{
+ const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
+ struct module_signature *ms;
+ unsigned long sig_len;
+ int ret;
+
+ 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;
+
+ ms = (void *)kernel + kernel_len - sizeof(*ms);
+ ret = mod_check_sig(ms, kernel_len, "kexec");
+ if (ret)
+ return ret;
+
+ sig_len = be32_to_cpu(ms->sig_len);
+ kernel_len -= sizeof(*ms) + sig_len;
+
+ return verify_pkcs7_signature(kernel, kernel_len,
+ kernel + kernel_len, sig_len,
+ VERIFY_USE_PLATFORM_KEYRING,
+ VERIFYING_MODULE_SIGNATURE,
+ NULL, NULL);
+}
+#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


2022-01-07 11:56:02

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH v3 4/6] module: strip the signature marker in the verification function.

It is stripped by each caller separately.

Note: this changes the error for kexec_file from EKEYREJECTED to ENODATA
when the signature marker is missing.

Signed-off-by: Michal Suchanek <[email protected]>
---
v3: - Philipp Rudo <[email protected]>: Update the commit with note about
change of raturn value
- the module_signature.h is now no longer needed for kexec_file
---
arch/powerpc/kexec/elf_64.c | 10 ----------
arch/s390/kernel/machine_kexec_file.c | 10 ----------
kernel/module.c | 7 +------
kernel/module_signing.c | 12 ++++++++++--
4 files changed, 11 insertions(+), 28 deletions(-)

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 9442666ca69d..e8dff6b23ac5 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -24,7 +24,6 @@
#include <linux/slab.h>
#include <linux/types.h>
#include <linux/verification.h>
-#include <linux/module_signature.h>

static void *elf64_load(struct kimage *image, char *kernel_buf,
unsigned long kernel_len, char *initrd,
@@ -157,15 +156,6 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
int elf64_verify_sig(const char *kernel, unsigned long length)
{
size_t kernel_len = length;
- const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-
- 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/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 75e0c17cf0eb..3e3bc7bcae86 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -12,7 +12,6 @@
#include <linux/elf.h>
#include <linux/errno.h>
#include <linux/kexec.h>
-#include <linux/module_signature.h>
#include <linux/verification.h>
#include <linux/vmalloc.h>
#include <asm/boot_data.h>
@@ -29,20 +28,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 8481933dfa92..d91ca0f93a40 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


2022-01-07 11:56:03

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH v3 5/6] module: Use key_being_used_for for log messages in verify_appended_signature

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 <[email protected]>
---
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 3e3bc7bcae86..18ba6df31d68 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -34,7 +34,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 d91ca0f93a40..0a359dc6b690 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


2022-01-07 11:56:05

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH v3 6/6] module: Move duplicate mod_check_sig users code to mod_parse_sig

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.

Note: This changes the error from ENOENT to ENODATA for ima_read_modsig
in the case the signature marker is missing.

Signed-off-by: Michal Suchanek <[email protected]>
---
v3: - Philipp Rudo <[email protected]>: Update the commit with note about
change of raturn value
- Preserve the EBADMSG error code while moving code araound
---
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..b8eb30182183 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 -EBADMSG;
+
+ 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


2022-01-07 11:56:24

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH v3 3/6] kexec_file: Don't opencode appended signature verification.

Module verification already implements appeded signature verification.

Reuse it for kexec_file.

Signed-off-by: Michal Suchanek <[email protected]>
---
v3: - Philipp Rudo <[email protected]>: Update the dependency on
MODULE_SIG_FORMAT to MODULE_SIG
- Include linux/verification.h - previously added in earlier patch
---
arch/powerpc/Kconfig | 2 +-
arch/powerpc/kexec/elf_64.c | 22 +++++-----------------
arch/s390/Kconfig | 2 +-
arch/s390/kernel/machine_kexec_file.c | 21 ++++-----------------
include/linux/verification.h | 3 +++
kernel/module-internal.h | 2 --
kernel/module.c | 4 +++-
kernel/module_signing.c | 24 +++++++++++++++---------
8 files changed, 32 insertions(+), 48 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1cde9b6c5987..4092187474ff 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -562,7 +562,7 @@ config ARCH_HAS_KEXEC_PURGATORY

config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
- depends on KEXEC_FILE && MODULE_SIG_FORMAT
+ depends on KEXEC_FILE && MODULE_SIG
help
This option makes kernel signature verification mandatory for
the kexec_file_load() syscall.
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 98d1cb5135b4..9442666ca69d 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>
#include <linux/module_signature.h>

static void *elf64_load(struct kimage *image, char *kernel_buf,
@@ -153,12 +154,10 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
}

#ifdef CONFIG_KEXEC_SIG
-int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
+int elf64_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;
- int ret;

if (marker_len > kernel_len)
return -EKEYREJECTED;
@@ -168,19 +167,8 @@ int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
return -EKEYREJECTED;
kernel_len -= marker_len;

- ms = (void *)kernel + kernel_len - sizeof(*ms);
- ret = mod_check_sig(ms, kernel_len, "kexec");
- if (ret)
- return ret;
-
- sig_len = be32_to_cpu(ms->sig_len);
- kernel_len -= sizeof(*ms) + sig_len;
-
- 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/arch/s390/Kconfig b/arch/s390/Kconfig
index 2a5bb4f29cfe..cece7152ea35 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -544,7 +544,7 @@ config ARCH_HAS_KEXEC_PURGATORY

config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
- depends on KEXEC_FILE && MODULE_SIG_FORMAT
+ depends on KEXEC_FILE && MODULE_SIG
help
This option makes kernel signature verification mandatory for
the kexec_file_load() syscall.
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index c944d71316c7..75e0c17cf0eb 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -26,12 +26,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;
- int ret;

/* Skip signature verification when not secure IPLed. */
if (!ipl_secure_flag)
@@ -45,19 +43,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);
- ret = mod_check_sig(ms, kernel_len, "kexec");
- if (ret)
- return ret;
-
- sig_len = be32_to_cpu(ms->sig_len);
- kernel_len -= sizeof(*ms) + sig_len;
-
- 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 84a9141a5e15..8481933dfa92 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


2022-01-07 18:38:37

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] kexec_file: Don't opencode appended signature verification.

Hi Michal,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on s390/features linus/master jeyu/modules-next v5.16-rc8 next-20220106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Michal-Suchanek/KEXEC_SIG-with-appended-signature/20220107-195818
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: hexagon-randconfig-r016-20220107 (https://download.01.org/0day-ci/archive/20220108/[email protected]/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project f3a344d2125fa37e59bae1b0874442c650a19607)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c59400c94a653abe5a5fbfd5bc166bd3ac1ebb41
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Michal-Suchanek/KEXEC_SIG-with-appended-signature/20220107-195818
git checkout c59400c94a653abe5a5fbfd5bc166bd3ac1ebb41
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

>> kernel/module.c:2898:40: error: incompatible pointer types passing 'unsigned long *' to parameter of type 'size_t *' (aka 'unsigned int *') [-Werror,-Wincompatible-pointer-types]
err = verify_appended_signature(mod, &info->len,
^~~~~~~~~~
include/linux/verification.h:63:57: note: passing argument to parameter 'len' here
int verify_appended_signature(const void *data, size_t *len, struct key *trusted_keys,
^
kernel/module.c:4804:6: warning: no previous prototype for function 'module_layout' [-Wmissing-prototypes]
void module_layout(struct module *mod,
^
kernel/module.c:4804:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void module_layout(struct module *mod,
^
static
1 warning and 1 error generated.


vim +2898 kernel/module.c

2880
2881 #ifdef CONFIG_MODULE_SIG
2882 static int module_sig_check(struct load_info *info, int flags)
2883 {
2884 int err = -ENODATA;
2885 const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
2886 const char *reason;
2887 const void *mod = info->hdr;
2888
2889 /*
2890 * Require flags == 0, as a module with version information
2891 * removed is no longer the module that was signed
2892 */
2893 if (flags == 0 &&
2894 info->len > markerlen &&
2895 memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
2896 /* We truncate the module to discard the signature */
2897 info->len -= markerlen;
> 2898 err = verify_appended_signature(mod, &info->len,
2899 VERIFY_USE_SECONDARY_KEYRING, "module");
2900 if (!err) {
2901 info->sig_ok = true;
2902 return 0;
2903 }
2904 }
2905
2906 /*
2907 * We don't permit modules to be loaded into the trusted kernels
2908 * without a valid signature on them, but if we're not enforcing,
2909 * certain errors are non-fatal.
2910 */
2911 switch (err) {
2912 case -ENODATA:
2913 reason = "unsigned module";
2914 break;
2915 case -ENOPKG:
2916 reason = "module with unsupported crypto";
2917 break;
2918 case -ENOKEY:
2919 reason = "module with unavailable key";
2920 break;
2921
2922 default:
2923 /*
2924 * All other errors are fatal, including lack of memory,
2925 * unparseable signatures, and signature check failures --
2926 * even if signatures aren't required.
2927 */
2928 return err;
2929 }
2930
2931 if (is_module_sig_enforced()) {
2932 pr_notice("Loading of %s is rejected\n", reason);
2933 return -EKEYREJECTED;
2934 }
2935
2936 return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
2937 }
2938 #else /* !CONFIG_MODULE_SIG */
2939 static int module_sig_check(struct load_info *info, int flags)
2940 {
2941 return 0;
2942 }
2943 #endif /* !CONFIG_MODULE_SIG */
2944

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

2022-01-08 15:00:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 3/6] kexec_file: Don't opencode appended signature verification.

Hi Michal,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on s390/features linus/master jeyu/modules-next v5.16-rc8 next-20220107]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Michal-Suchanek/KEXEC_SIG-with-appended-signature/20220107-195818
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: arc-randconfig-r043-20220107 (https://download.01.org/0day-ci/archive/20220108/[email protected]/config)
compiler: arceb-elf-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/c59400c94a653abe5a5fbfd5bc166bd3ac1ebb41
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Michal-Suchanek/KEXEC_SIG-with-appended-signature/20220107-195818
git checkout c59400c94a653abe5a5fbfd5bc166bd3ac1ebb41
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

kernel/module.c: In function 'module_sig_check':
>> kernel/module.c:2898:54: error: passing argument 2 of 'verify_appended_signature' from incompatible pointer type [-Werror=incompatible-pointer-types]
2898 | err = verify_appended_signature(mod, &info->len,
| ^~~~~~~~~~
| |
| long unsigned int *
In file included from kernel/module.c:60:
include/linux/verification.h:63:57: note: expected 'size_t *' {aka 'unsigned int *'} but argument is of type 'long unsigned int *'
63 | int verify_appended_signature(const void *data, size_t *len, struct key *trusted_keys,
| ~~~~~~~~^~~
cc1: some warnings being treated as errors


vim +/verify_appended_signature +2898 kernel/module.c

2880
2881 #ifdef CONFIG_MODULE_SIG
2882 static int module_sig_check(struct load_info *info, int flags)
2883 {
2884 int err = -ENODATA;
2885 const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
2886 const char *reason;
2887 const void *mod = info->hdr;
2888
2889 /*
2890 * Require flags == 0, as a module with version information
2891 * removed is no longer the module that was signed
2892 */
2893 if (flags == 0 &&
2894 info->len > markerlen &&
2895 memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
2896 /* We truncate the module to discard the signature */
2897 info->len -= markerlen;
> 2898 err = verify_appended_signature(mod, &info->len,
2899 VERIFY_USE_SECONDARY_KEYRING, "module");
2900 if (!err) {
2901 info->sig_ok = true;
2902 return 0;
2903 }
2904 }
2905
2906 /*
2907 * We don't permit modules to be loaded into the trusted kernels
2908 * without a valid signature on them, but if we're not enforcing,
2909 * certain errors are non-fatal.
2910 */
2911 switch (err) {
2912 case -ENODATA:
2913 reason = "unsigned module";
2914 break;
2915 case -ENOPKG:
2916 reason = "module with unsupported crypto";
2917 break;
2918 case -ENOKEY:
2919 reason = "module with unavailable key";
2920 break;
2921
2922 default:
2923 /*
2924 * All other errors are fatal, including lack of memory,
2925 * unparseable signatures, and signature check failures --
2926 * even if signatures aren't required.
2927 */
2928 return err;
2929 }
2930
2931 if (is_module_sig_enforced()) {
2932 pr_notice("Loading of %s is rejected\n", reason);
2933 return -EKEYREJECTED;
2934 }
2935
2936 return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
2937 }
2938 #else /* !CONFIG_MODULE_SIG */
2939 static int module_sig_check(struct load_info *info, int flags)
2940 {
2941 return 0;
2942 }
2943 #endif /* !CONFIG_MODULE_SIG */
2944

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]