Hello,
The main highlight in this version is that it's not necessary to appraise
the file before storing its measurement anymore. This is possible due to a
new approach that Mimi suggested: we decide whether the modsig should be
used or not at the time it is read from the file, while before we would
only make that decision when trying to verify its signature (i.e., at
appraisal time).
Now the modsig is only ignored if it references a signature that is not
present in IMA's keyring (or if there's a parsing error, obviously). If the
signature verification fails, then appraisal fails and the xattr signature
(if there is one) is not used as a fallback. With this change, we already
know which signature will be used for appraisal at the time the measurement
is stored in the measurement list.
Also, now IMA first tries to use the xattr signature and only if it doesn't
exist or uses a key which isn't in IMA's keyring it will look for a modsig.
This is because an xattr sig was most likely placed there by the system
admin, while the modsig most likely came from the OS vendor and thus the
former should be given preference. Also, since modsig is only allowed in
hooks which aren't in any hotpath, there's no practical gain in speed by
avoiding to read the xattr.
There's also a new template field called 'd-sig' which will store the
digest used for verification of the modsig in the measurement list.
Finally, I dropped the patches removing superfluous parentheses from
expressions, since they add a lot of churn and whether they're an actual
improvement or not is subjective.
These patches apply on top of today's linux-integrity/next-integrity.
They also require patch 4/4 from my cleanups series I posted a few days
ago, which isn't in next-integrity yet:
https://lkml.org/lkml/2018/3/14/1029
For convenience, I pushed these patches to the following branch:
https://github.com/bauermann/linux.git ima-modsig
Original cover letter:
On the OpenPOWER platform, secure boot and trusted boot are being
implemented using IMA for taking measurements and verifying signatures.
Since the kernel image on Power servers is an ELF binary, kernels are
signed using the scripts/sign-file tool and thus use the same signature
format as signed kernel modules.
This patch series adds support in IMA for verifying those signatures.
It adds flexibility to OpenPOWER secure boot, because it allows it to boot
kernels with the signature appended to them as well as kernels where the
signature is stored in the IMA extended attribute.
Changes since v5:
- Patch "ima: Remove some superfluous parentheses"
- Dropped.
- Patch "evm, ima: Remove superfluous parentheses"
- Dropped.
- Patch "evm, ima: Remove more superfluous parentheses"
- Dropped.
- Patch "ima: Don't pass xattr value to EVM xattr verification."
- Dropped.
- Patch "ima: Store measurement after appraisal"
- Dropped.
- Patch "MODSIGN: Export module signature definitions"
- Reduced changes to the code that was moved into validate_module_sig()
to the minimum necessary (suggested by Mimi Zohar).
- Added SPDX license identifier.
- Patch "PKCS#7: Introduce pkcs7_get_message_sig() and verify_pkcs7_message_sig()"
- In the hypothetical case that there's more than one sinfo, changed
pkcs7_get_message_sig() to return NULL instead of the first sinfo's sig.
- Dropped Mimi's Reviewed-by because of the code change above.
- Patch "PKCS#7: Introduce pkcs7_get_digest()"
- New patch.
- Patch "integrity: Introduce integrity_keyring_from_id"
- Add stub in case CONFIG_INTEGRITY_SIGNATURE isn't set.
- Patch "integrity: Introduce asymmetric_sig_has_known_key()"
- New patch.
- Patch "ima: Introduce is_ima_sig"
- New patch, with code from "ima: Improvements in ima_appraise_measurement"
- Patch "ima: Add modsig appraise_type option for module-style appended signatures"
- Changed appraise_type to accept "imasig|modsig" instead of
"modsig|imasig" to reflect the fact that now IMA only looks for
the modsig after failing to find a suitable imasig stored in the xattr.
- Added SPDX license identifier.
- Patch "ima: Add functions to read and verify a modsig signature"
- Changed ima_read_modsig() to abort loading the modsig if it uses a key
which isn't known to IMA.
- Changed ima_get_modsig_hash() to use pkcs7_get_digest().
- Patch "ima: Implement support for module-style appended signatures"
- Added ima_xattr_sig_known_key() auxiliary function.
- Call ima_read_modsig() directly from process_measurement() instead of
from ima_appraise_measurement(), and only if there's no xattr signature
or if the xattr signature uses a key which isn't known to IMA.
- hash_algo in process_measurement() is always obtained from the xattr
signature, never from the modsig.
- Changes to ima_appraise_measurement() are a lot simpler now, and don't
involve going back to the main switch statement a second time.
- Pass xattr_value to evm_verifyxattr() unless xattr_value is a modsig.
- Patch "ima: Write modsig to the measurement list"
- Since now we determine whether we'll use an xattr sig or a modsig
at the time they are read, there's no need to store a measurement
again in the modsig case. Thus, this patch doesn't need to change
ima_store_measurement() nor process_measurement() anymore.
- Define new "d-sig" template field which holds the digest that is
expected to match the one contained in the modsig.
- Moved addition of ima_modsig_serialize_data() to patch "ima: Add
functions to read and verify a modsig signature".
- Increase MAX_TEMPLATE_NAME_LEN to 24.
Changes since v4:
- Patch "ima: Remove redundant conditional operator"
- New patch.
- Patch "ima: Remove some superfluous parentheses"
- New patch.
- Patch "evm, ima: Remove superfluous parentheses"
- New patch.
- Patch "evm, ima: Remove more superfluous parentheses"
- New patch.
- Patch "ima: Simplify ima_eventsig_init"
- New patch.
- Patch "ima: Improvements in ima_appraise_measurement"
- New patch.
- Patch "ima: Don't pass xattr value to EVM xattr verification."
- New patch.
- Patch "ima: Export func_tokens"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- Patch "ima: Add modsig appraise_type option for module-style appended
signatures"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- Mention modsig option in Documentation/ABI/testing/ima_policy
(suggested by Mimi Zohar).
- Patch "ima: Add functions to read and verify a modsig signature"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- Patch "ima: Implement support for module-style appended signatures"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- In ima_appraise_measurement, change the logic of dealing with xattr
errors in case the modsig verification fails. With this,
process_xattr_error isn't needed anymore.
- Patch "ima: Write modsig to the measurement list"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- Added ima_current_template_has_sig function.
- Removed hdr parameter from ima_modsig_serialize_data.
- In ima_store_measurement, continue processing even if the given PCR
is already measured if it's for a modsig.
- In process_measurement, add exception to store measurement even if
IMA_MEASURE is not set when appraising a modsig (suggested by
Mimi Zohar).
- Call is_ima_sig in ima_eventsig_init.
Changes since v3:
- Patch "integrity: Introduce struct evm_hmac_xattr"
- Renamed new struct to evm_xattr.
- Define struct evm_xattr using struct evm_ima_xattr_data, and moved it
from evm.h to integrity.h (suggested by Mimi Zohar).
- Patch "PKCS#7: Introduce verify_pkcs7_message_sig"
- Also introduce pkcs7_get_message_sig.
- Patch "ima: Support appended signatures for appraisal"
- Moved check for buffer presence and size from ima_appraise_measurement
to ima_read_modsig (suggested by Mimi Zohar).
- Factored out handling of ima_read_xattr return value into
process_xattr_error in ima_appraise_measurement so that it can be used
if the modsig verification fails.
- Pass NULL xattr_value to evm_verifyxattr even in the case of xattr
signature in ima_appraise_measurement (suggested by Mimi Zohar).
- Use switch statement provided by Mimi Zohar to check result of
evm_verifyxattr.
- If the modsig verification succeeds, copy the hash calculated during
the verification to the iint cache (suggested by Mimi Zohar).
- Substitute recursion in ima_appraise_measurement by a goto statement
back to the main switch statement (suggested by Mimi Zohar).
Thiago Jung Bauermann (12):
MODSIGN: Export module signature definitions
PKCS#7: Introduce pkcs7_get_message_sig() and
verify_pkcs7_message_sig()
PKCS#7: Introduce pkcs7_get_digest()
ima: Introduce is_ima_sig()
integrity: Introduce integrity_keyring_from_id()
integrity: Introduce asymmetric_sig_has_known_key()
integrity: Select CONFIG_KEYS instead of depending on it
ima: Export func_tokens
ima: Add modsig appraise_type option for module-style appended
signatures
ima: Add functions to read and verify a modsig signature
ima: Implement support for module-style appended signatures
ima: Write modsig to the measurement list
Documentation/ABI/testing/ima_policy | 6 +-
Documentation/security/IMA-templates.rst | 5 +
certs/system_keyring.c | 61 ++++++---
crypto/asymmetric_keys/pkcs7_parser.c | 16 +++
crypto/asymmetric_keys/pkcs7_verify.c | 25 ++++
include/crypto/pkcs7.h | 5 +
include/linux/module.h | 3 -
include/linux/module_signature.h | 44 +++++++
include/linux/verification.h | 10 ++
init/Kconfig | 6 +-
kernel/Makefile | 2 +-
kernel/module.c | 1 +
kernel/module_signing.c | 77 +++++------
security/integrity/Kconfig | 2 +-
security/integrity/digsig.c | 28 +++-
security/integrity/digsig_asymmetric.c | 44 +++++--
security/integrity/ima/Kconfig | 13 ++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 66 ++++++++++
security/integrity/ima/ima_appraise.c | 60 +++++++--
security/integrity/ima/ima_main.c | 21 ++-
security/integrity/ima/ima_modsig.c | 212 ++++++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 24 ++--
security/integrity/ima/ima_template.c | 4 +-
security/integrity/ima/ima_template_lib.c | 49 ++++++-
security/integrity/ima/ima_template_lib.h | 2 +
security/integrity/integrity.h | 16 +++
27 files changed, 693 insertions(+), 110 deletions(-)
create mode 100644 include/linux/module_signature.h
create mode 100644 security/integrity/ima/ima_modsig.c
IMA will need to know the key that signed a given PKCS#7 message, so add
pkcs7_get_message_sig().
It will also need to verify an already parsed PKCS#7 message. For this
purpose, add verify_pkcs7_message_sig() which takes a struct pkcs7_message
for verification instead of the raw bytes that verify_pkcs7_signature()
takes.
Signed-off-by: Thiago Jung Bauermann <[email protected]>
Cc: David Howells <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: "David S. Miller" <[email protected]>
---
certs/system_keyring.c | 61 ++++++++++++++++++++++++++---------
crypto/asymmetric_keys/pkcs7_parser.c | 16 +++++++++
include/crypto/pkcs7.h | 2 ++
include/linux/verification.h | 10 ++++++
4 files changed, 73 insertions(+), 16 deletions(-)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 6251d1b27f0c..7ddc8b7a3062 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -190,33 +190,27 @@ late_initcall(load_system_certificate_list);
#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
/**
- * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
+ * verify_pkcs7_message_sig - Verify a PKCS#7-based signature on system data.
* @data: The data to be verified (NULL if expecting internal data).
* @len: Size of @data.
- * @raw_pkcs7: The PKCS#7 message that is the signature.
- * @pkcs7_len: The size of @raw_pkcs7.
+ * @pkcs7: The PKCS#7 message that is the signature.
* @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
* (void *)1UL for all trusted keys).
* @usage: The use to which the key is being put.
* @view_content: Callback to gain access to content.
* @ctx: Context for callback.
*/
-int verify_pkcs7_signature(const void *data, size_t len,
- const void *raw_pkcs7, size_t pkcs7_len,
- struct key *trusted_keys,
- enum key_being_used_for usage,
- int (*view_content)(void *ctx,
- const void *data, size_t len,
- size_t asn1hdrlen),
- void *ctx)
+int verify_pkcs7_message_sig(const void *data, size_t len,
+ struct pkcs7_message *pkcs7,
+ struct key *trusted_keys,
+ enum key_being_used_for usage,
+ int (*view_content)(void *ctx,
+ const void *data, size_t len,
+ size_t asn1hdrlen),
+ void *ctx)
{
- struct pkcs7_message *pkcs7;
int ret;
- pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
- if (IS_ERR(pkcs7))
- return PTR_ERR(pkcs7);
-
/* The data should be detached - so we need to supply it. */
if (data && pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
pr_err("PKCS#7 signature with non-detached data\n");
@@ -258,6 +252,41 @@ int verify_pkcs7_signature(const void *data, size_t len,
}
error:
+ pr_devel("<==%s() = %d\n", __func__, ret);
+ return ret;
+}
+
+/**
+ * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
+ * @data: The data to be verified (NULL if expecting internal data).
+ * @len: Size of @data.
+ * @raw_pkcs7: The PKCS#7 message that is the signature.
+ * @pkcs7_len: The size of @raw_pkcs7.
+ * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
+ * (void *)1UL for all trusted keys).
+ * @usage: The use to which the key is being put.
+ * @view_content: Callback to gain access to content.
+ * @ctx: Context for callback.
+ */
+int verify_pkcs7_signature(const void *data, size_t len,
+ const void *raw_pkcs7, size_t pkcs7_len,
+ struct key *trusted_keys,
+ enum key_being_used_for usage,
+ int (*view_content)(void *ctx,
+ const void *data, size_t len,
+ size_t asn1hdrlen),
+ void *ctx)
+{
+ struct pkcs7_message *pkcs7;
+ int ret;
+
+ pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
+ if (IS_ERR(pkcs7))
+ return PTR_ERR(pkcs7);
+
+ ret = verify_pkcs7_message_sig(data, len, pkcs7, trusted_keys, usage,
+ view_content, ctx);
+
pkcs7_free_message(pkcs7);
pr_devel("<==%s() = %d\n", __func__, ret);
return ret;
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index a6dcaa659aa8..456b803972b5 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -683,3 +683,19 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
return -ENOMEM;
return 0;
}
+
+/**
+ * pkcs7_get_message_sig - get signature in @pkcs7
+ */
+const struct public_key_signature *pkcs7_get_message_sig(
+ const struct pkcs7_message *pkcs7)
+{
+ /*
+ * This function doesn't support messages with more than one signature,
+ * so don't return anything in that case.
+ */
+ if (pkcs7->signed_infos == NULL || pkcs7->signed_infos->next != NULL)
+ return NULL;
+
+ return pkcs7->signed_infos->sig;
+}
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 583f199400a3..6f51d0cb6d12 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -28,6 +28,8 @@ extern void pkcs7_free_message(struct pkcs7_message *pkcs7);
extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
const void **_data, size_t *_datalen,
size_t *_headerlen);
+extern const struct public_key_signature *pkcs7_get_message_sig(
+ const struct pkcs7_message *pkcs7);
/*
* pkcs7_trust.c
diff --git a/include/linux/verification.h b/include/linux/verification.h
index a10549a6c7cd..f04dac2728ec 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -29,6 +29,7 @@ extern const char *const key_being_used_for[NR__KEY_BEING_USED_FOR];
#ifdef CONFIG_SYSTEM_DATA_VERIFICATION
struct key;
+struct pkcs7_message;
extern int verify_pkcs7_signature(const void *data, size_t len,
const void *raw_pkcs7, size_t pkcs7_len,
@@ -38,6 +39,15 @@ extern int verify_pkcs7_signature(const void *data, size_t len,
const void *data, size_t len,
size_t asn1hdrlen),
void *ctx);
+extern int verify_pkcs7_message_sig(const void *data, size_t len,
+ struct pkcs7_message *pkcs7,
+ struct key *trusted_keys,
+ enum key_being_used_for usage,
+ int (*view_content)(void *ctx,
+ const void *data,
+ size_t len,
+ size_t asn1hdrlen),
+ void *ctx);
#ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
IMA will use the module_signature format for append signatures, so export
the relevant definitions and factor out the code which verifies that the
appended signature trailer is valid.
Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
and be able to use validate_module_sig() without having to depend on
CONFIG_MODULE_SIG.
Signed-off-by: Thiago Jung Bauermann <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
Cc: Jessica Yu <[email protected]>
---
include/linux/module.h | 3 --
include/linux/module_signature.h | 44 +++++++++++++++++++++++
init/Kconfig | 6 +++-
kernel/Makefile | 2 +-
kernel/module.c | 1 +
kernel/module_signing.c | 77 ++++++++++++++++++----------------------
6 files changed, 85 insertions(+), 48 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index d44df9b2c131..275cbc2579eb 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -24,9 +24,6 @@
#include <linux/percpu.h>
#include <asm/module.h>
-/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
-#define MODULE_SIG_STRING "~Module signature appended~\n"
-
/* Not Yet Implemented */
#define MODULE_SUPPORTED_DEVICE(name)
diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
new file mode 100644
index 000000000000..890135437b6b
--- /dev/null
+++ b/include/linux/module_signature.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Module signature handling.
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells ([email protected])
+ */
+
+#ifndef _LINUX_MODULE_SIGNATURE_H
+#define _LINUX_MODULE_SIGNATURE_H
+
+/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
+#define MODULE_SIG_STRING "~Module signature appended~\n"
+
+enum pkey_id_type {
+ PKEY_ID_PGP, /* OpenPGP generated key ID */
+ PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
+ PKEY_ID_PKCS7, /* Signature in PKCS#7 message */
+};
+
+/*
+ * Module signature information block.
+ *
+ * The constituents of the signature section are, in order:
+ *
+ * - Signer's name
+ * - Key identifier
+ * - Signature data
+ * - Information block
+ */
+struct module_signature {
+ u8 algo; /* Public-key crypto algorithm [0] */
+ u8 hash; /* Digest algorithm [0] */
+ u8 id_type; /* Key identifier type [PKEY_ID_PKCS7] */
+ u8 signer_len; /* Length of signer's name [0] */
+ u8 key_id_len; /* Length of key identifier [0] */
+ u8 __pad[3];
+ __be32 sig_len; /* Length of signature data */
+};
+
+int validate_module_sig(const struct module_signature *ms, size_t file_len);
+int mod_verify_sig(const void *mod, unsigned long *_modlen);
+
+#endif /* _LINUX_MODULE_SIGNATURE_H */
diff --git a/init/Kconfig b/init/Kconfig
index e37f4b2a6445..56a9679671b5 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1760,7 +1760,7 @@ config MODULE_SRCVERSION_ALL
config MODULE_SIG
bool "Module signature verification"
depends on MODULES
- select SYSTEM_DATA_VERIFICATION
+ select MODULE_SIG_FORMAT
help
Check modules for valid signatures upon load: the signature
is simply appended to the module. For more information see
@@ -1775,6 +1775,10 @@ config MODULE_SIG
debuginfo strip done by some packagers (such as rpmbuild) and
inclusion into an initramfs that wants the module size reduced.
+config MODULE_SIG_FORMAT
+ def_bool n
+ select SYSTEM_DATA_VERIFICATION
+
config MODULE_SIG_FORCE
bool "Require modules to be validly signed"
depends on MODULE_SIG
diff --git a/kernel/Makefile b/kernel/Makefile
index f85ae5dfa474..a4c7a7a0cce7 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -58,7 +58,7 @@ obj-y += up.o
endif
obj-$(CONFIG_UID16) += uid16.o
obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o
+obj-$(CONFIG_MODULE_SIG_FORMAT) += module_signing.o
obj-$(CONFIG_KALLSYMS) += kallsyms.o
obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
obj-$(CONFIG_CRASH_CORE) += crash_core.o
diff --git a/kernel/module.c b/kernel/module.c
index ad2d420024f6..292eee54e390 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -19,6 +19,7 @@
#include <linux/export.h>
#include <linux/extable.h>
#include <linux/moduleloader.h>
+#include <linux/module_signature.h>
#include <linux/trace_events.h>
#include <linux/init.h>
#include <linux/kallsyms.h>
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 937c844bee4a..a7c2c461c598 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,36 +11,41 @@
#include <linux/kernel.h>
#include <linux/errno.h>
+#include <linux/module_signature.h>
#include <linux/string.h>
#include <linux/verification.h>
#include <crypto/public_key.h>
#include "module-internal.h"
-enum pkey_id_type {
- PKEY_ID_PGP, /* OpenPGP generated key ID */
- PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
- PKEY_ID_PKCS7, /* Signature in PKCS#7 message */
-};
-
-/*
- * Module signature information block.
+/**
+ * validate_module_sig - validate that the given signature is sane
*
- * The constituents of the signature section are, in order:
- *
- * - Signer's name
- * - Key identifier
- * - Signature data
- * - Information block
+ * @ms: Signature to validate.
+ * @file_len: Size of the file to which @ms is appended.
*/
-struct module_signature {
- u8 algo; /* Public-key crypto algorithm [0] */
- u8 hash; /* Digest algorithm [0] */
- u8 id_type; /* Key identifier type [PKEY_ID_PKCS7] */
- u8 signer_len; /* Length of signer's name [0] */
- u8 key_id_len; /* Length of key identifier [0] */
- u8 __pad[3];
- __be32 sig_len; /* Length of signature data */
-};
+int validate_module_sig(const struct module_signature *ms, size_t file_len)
+{
+ if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
+ return -EBADMSG;
+
+ if (ms->id_type != PKEY_ID_PKCS7) {
+ pr_err("Module is not signed with expected PKCS#7 message\n");
+ return -ENOPKG;
+ }
+
+ 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) {
+ pr_err("PKCS#7 signature info has unexpected non-zero params\n");
+ return -EBADMSG;
+ }
+
+ return 0;
+}
/*
* Verify the signature on a module.
@@ -49,6 +54,7 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
{
struct module_signature ms;
size_t modlen = *_modlen, sig_len;
+ int ret;
pr_devel("==>%s(,%zu)\n", __func__, modlen);
@@ -56,30 +62,15 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
return -EBADMSG;
memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
- modlen -= sizeof(ms);
+
+ ret = validate_module_sig(&ms, modlen);
+ if (ret)
+ return ret;
sig_len = be32_to_cpu(ms.sig_len);
- if (sig_len >= modlen)
- return -EBADMSG;
- modlen -= sig_len;
+ modlen -= sig_len + sizeof(ms);
*_modlen = modlen;
- if (ms.id_type != PKEY_ID_PKCS7) {
- pr_err("Module is not signed with expected PKCS#7 message\n");
- return -ENOPKG;
- }
-
- 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) {
- pr_err("PKCS#7 signature info has unexpected non-zero params\n");
- return -EBADMSG;
- }
-
return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
NULL, VERIFYING_MODULE_SIGNATURE,
NULL, NULL);
ima_read_modsig() will need it so that it can show an error message.
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_policy.c | 12 ++++++------
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 4bafa6a97967..99f2bab15009 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -196,6 +196,8 @@ enum ima_hooks {
__ima_hooks(__ima_hook_enumify)
};
+extern const char *const func_tokens[];
+
/* LIM API function definitions */
int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
int mask, enum ima_hooks func, int *pcr);
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 51a4cd999a49..833da24c7009 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -992,6 +992,12 @@ void ima_delete_rules(void)
}
}
+#define __ima_hook_stringify(str) (#str),
+
+const char *const func_tokens[] = {
+ __ima_hooks(__ima_hook_stringify)
+};
+
#ifdef CONFIG_IMA_READ_POLICY
enum {
mask_exec = 0, mask_write, mask_read, mask_append
@@ -1004,12 +1010,6 @@ static const char *const mask_tokens[] = {
"MAY_APPEND"
};
-#define __ima_hook_stringify(str) (#str),
-
-static const char *const func_tokens[] = {
- __ima_hooks(__ima_hook_stringify)
-};
-
void *ima_policy_start(struct seq_file *m, loff_t *pos)
{
loff_t l = *pos;
This patch introduces the modsig keyword to the IMA policy syntax to
specify that a given hook should expect the file to have the IMA signature
appended to it. Here is how it can be used in a rule:
appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
With this rule, IMA will accept either a signature stored in the extended
attribute or an appended signature.
For now, the rule above will behave exactly the same as if
appraise_type=imasig was specified because the actual modsig implementation
will be introduced in a separate patch.
Suggested-by: Mimi Zohar <[email protected]>
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
Documentation/ABI/testing/ima_policy | 6 +++++-
security/integrity/ima/Kconfig | 10 ++++++++++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 9 +++++++++
security/integrity/ima/ima_modsig.c | 31 +++++++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 12 ++++++++++--
security/integrity/integrity.h | 1 +
7 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index b8465e00ba5f..835d9da9b26e 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -37,7 +37,7 @@ Description:
euid:= decimal value
fowner:= decimal value
lsm: are LSM specific
- option: appraise_type:= [imasig]
+ option: appraise_type:= [imasig] [imasig|modsig]
pcr:= decimal value
default policy:
@@ -103,3 +103,7 @@ Description:
measure func=KEXEC_KERNEL_CHECK pcr=4
measure func=KEXEC_INITRAMFS_CHECK pcr=5
+
+ Example of appraise rule allowing modsig appended signatures:
+
+ appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 6a8f67714c83..ee278189e0bb 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -164,6 +164,16 @@ config IMA_APPRAISE_BOOTPARAM
This option enables the different "ima_appraise=" modes
(eg. fix, log) from the boot command line.
+config IMA_APPRAISE_MODSIG
+ bool "Support module-style signatures for appraisal"
+ depends on IMA_APPRAISE
+ default n
+ help
+ Adds support for signatures appended to files. The format of the
+ appended signature is the same used for signed kernel modules.
+ The modsig keyword can be used in the IMA policy to allow a hook
+ to accept such signatures.
+
config IMA_TRUSTED_KEYRING
bool "Require all keys on the .ima keyring be signed (deprecated)"
depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index d921dc4f9eb0..31d57cdf2421 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -9,5 +9,6 @@ obj-$(CONFIG_IMA) += ima.o
ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
ima_policy.o ima_template.o ima_template_lib.o
ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
+ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o
ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 99f2bab15009..c61d8fc5190d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -299,6 +299,15 @@ static inline int ima_read_xattr(struct dentry *dentry,
#endif /* CONFIG_IMA_APPRAISE */
+#ifdef CONFIG_IMA_APPRAISE_MODSIG
+bool ima_hook_supports_modsig(enum ima_hooks func);
+#else
+static inline bool ima_hook_supports_modsig(enum ima_hooks func)
+{
+ return false;
+}
+#endif /* CONFIG_IMA_APPRAISE_MODSIG */
+
/* LSM based policy rules require audit */
#ifdef CONFIG_IMA_LSM_RULES
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
new file mode 100644
index 000000000000..d8ea811b6f74
--- /dev/null
+++ b/security/integrity/ima/ima_modsig.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IMA support for appraising module-style appended signatures.
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Author:
+ * Thiago Jung Bauermann <[email protected]>
+ */
+
+#include "ima.h"
+
+/**
+ * ima_hook_supports_modsig - can the policy allow modsig for this hook?
+ *
+ * modsig is only supported by hooks using ima_post_read_file, because only they
+ * preload the contents of the file in a buffer. FILE_CHECK does that in some
+ * cases, but not when reached from vfs_open. POLICY_CHECK can support it, but
+ * it's not useful in practice because it's a text file so deny.
+ */
+bool ima_hook_supports_modsig(enum ima_hooks func)
+{
+ switch (func) {
+ case FIRMWARE_CHECK:
+ case KEXEC_KERNEL_CHECK:
+ case KEXEC_INITRAMFS_CHECK:
+ return true;
+ default:
+ return false;
+ }
+}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 833da24c7009..e7421fd94277 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -887,6 +887,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
ima_log_string(ab, "appraise_type", args[0].from);
if ((strcmp(args[0].from, "imasig")) == 0)
entry->flags |= IMA_DIGSIG_REQUIRED;
+ else if (ima_hook_supports_modsig(entry->func) &&
+ strcmp(args[0].from, "imasig|modsig") == 0)
+ entry->flags |= IMA_DIGSIG_REQUIRED
+ | IMA_MODSIG_ALLOWED;
else
result = -EINVAL;
break;
@@ -1176,8 +1180,12 @@ int ima_policy_show(struct seq_file *m, void *v)
}
}
}
- if (entry->flags & IMA_DIGSIG_REQUIRED)
- seq_puts(m, "appraise_type=imasig ");
+ if (entry->flags & IMA_DIGSIG_REQUIRED) {
+ if (entry->flags & IMA_MODSIG_ALLOWED)
+ seq_puts(m, "appraise_type=imasig|modsig ");
+ else
+ seq_puts(m, "appraise_type=imasig ");
+ }
if (entry->flags & IMA_PERMIT_DIRECTIO)
seq_puts(m, "permit_directio ");
rcu_read_unlock();
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 4c381b992e11..6643c6550787 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -35,6 +35,7 @@
#define IMA_NEW_FILE 0x04000000
#define EVM_IMMUTABLE_DIGSIG 0x08000000
#define IMA_FAIL_UNVERIFIABLE_SIGS 0x10000000
+#define IMA_MODSIG_ALLOWED 0x20000000
#define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
IMA_HASH | IMA_APPRAISE_SUBMASK)
With the introduction of another IMA signature type (modsig), some places
will need to check for both of them. It is cleaner to do that if there's a
helper function to tell whether an xattr_value represents an IMA
signature.
Suggested-by: Mimi Zohar <[email protected]>
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
security/integrity/ima/ima.h | 5 +++++
security/integrity/ima/ima_appraise.c | 7 +++----
security/integrity/ima/ima_template_lib.c | 2 +-
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 35fe91aa1fc9..4bafa6a97967 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -155,6 +155,11 @@ unsigned long ima_get_binary_runtime_size(void);
int ima_init_template(void);
void ima_init_template_list(void);
+static inline bool is_ima_sig(const struct evm_ima_xattr_data *xattr_value)
+{
+ return xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG;
+}
+
/*
* used to protect h_table and sha_table
*/
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index a6b2995b7d0b..01172eab297b 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -325,15 +325,14 @@ int ima_appraise_measurement(enum ima_hooks func,
} else if (status != INTEGRITY_PASS) {
/* Fix mode, but don't replace file signatures. */
if ((ima_appraise & IMA_APPRAISE_FIX) &&
- (!xattr_value ||
- xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
+ !is_ima_sig(xattr_value)) {
if (!ima_fix_xattr(dentry, iint))
status = INTEGRITY_PASS;
}
/* Permit new files with file signatures, but without data. */
if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
- xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
+ is_ima_sig(xattr_value)) {
status = INTEGRITY_PASS;
}
@@ -448,7 +447,7 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
return -EINVAL;
ima_reset_appraise_flags(d_backing_inode(dentry),
- xvalue->type == EVM_IMA_XATTR_DIGSIG);
+ is_ima_sig(xvalue));
result = 0;
}
return result;
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 5afaa53decc5..afb52a90e532 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -380,7 +380,7 @@ int ima_eventsig_init(struct ima_event_data *event_data,
{
struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
- if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG))
+ if (!is_ima_sig(xattr_value))
return 0;
return ima_write_template_field_data(xattr_value, event_data->xattr_len,
This avoids a dependency cycle in CONFIG_IMA_APPRAISE_MODSIG (introduced by
a later patch in this series): it will select CONFIG_MODULE_SIG_FORMAT
which in turn selects CONFIG_KEYS. Kconfig then complains that
CONFIG_INTEGRITY_SIGNATURE depends on CONFIG_KEYS.
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
security/integrity/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index da9565891738..0d642e0317c7 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -17,8 +17,8 @@ if INTEGRITY
config INTEGRITY_SIGNATURE
bool "Digital signature verification using multiple keyrings"
- depends on KEYS
default n
+ select KEYS
select SIGNATURE
help
This option enables digital signature verification support
This patch actually implements the appraise_type=imasig|modsig option,
allowing IMA to read and verify modsig signatures.
In case both are present in the same file, IMA will first check whether the
key used by the xattr signature is present in the kernel keyring. If not,
it will try the appended signature.
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
security/integrity/ima/ima.h | 11 +++++++-
security/integrity/ima/ima_appraise.c | 53 +++++++++++++++++++++++++++++++----
security/integrity/ima/ima_main.c | 21 +++++++++++---
3 files changed, 74 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 49aef56dc96d..c11ccb7c5bfb 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -157,7 +157,8 @@ void ima_init_template_list(void);
static inline bool is_ima_sig(const struct evm_ima_xattr_data *xattr_value)
{
- return xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG;
+ return xattr_value && (xattr_value->type == EVM_IMA_XATTR_DIGSIG ||
+ xattr_value->type == IMA_MODSIG);
}
/*
@@ -253,6 +254,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
enum ima_hooks func);
enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
int xattr_len);
+bool ima_xattr_sig_known_key(const struct evm_ima_xattr_data *xattr_value,
+ int xattr_len);
int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value);
@@ -291,6 +294,12 @@ ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len)
return ima_hash_algo;
}
+static inline bool ima_xattr_sig_known_key(const struct evm_ima_xattr_data
+ *xattr_value, int xattr_len)
+{
+ return false;
+}
+
static inline int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value)
{
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 01172eab297b..84e0fd5a19c8 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -189,6 +189,22 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
return ima_hash_algo;
}
+bool ima_xattr_sig_known_key(const struct evm_ima_xattr_data *xattr_value,
+ int xattr_len)
+{
+ struct key *keyring;
+
+ if (xattr_value->type != EVM_IMA_XATTR_DIGSIG)
+ return false;
+
+ keyring = integrity_keyring_from_id(INTEGRITY_KEYRING_IMA);
+ if (IS_ERR(keyring))
+ return false;
+
+ return asymmetric_sig_has_known_key(keyring, (const char *) xattr_value,
+ xattr_len);
+}
+
int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value)
{
@@ -221,8 +237,12 @@ int ima_appraise_measurement(enum ima_hooks func,
struct inode *inode = d_backing_inode(dentry);
enum integrity_status status = INTEGRITY_UNKNOWN;
int rc = xattr_len, hash_start = 0;
+ size_t xattr_contents_len;
+ void *xattr_contents;
- if (!(inode->i_opflags & IOP_XATTR))
+ /* If not appraising a modsig, we need an xattr. */
+ if ((xattr_value == NULL || xattr_value->type != IMA_MODSIG) &&
+ !(inode->i_opflags & IOP_XATTR))
return INTEGRITY_UNKNOWN;
if (rc <= 0) {
@@ -241,13 +261,29 @@ int ima_appraise_measurement(enum ima_hooks func,
goto out;
}
- status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
+ /*
+ * If it's a modsig, we don't have the xattr contents to pass to
+ * evm_verifyxattr().
+ */
+ if (xattr_value->type == IMA_MODSIG) {
+ xattr_contents = NULL;
+ xattr_contents_len = 0;
+ } else {
+ xattr_contents = xattr_value;
+ xattr_contents_len = xattr_len;
+ }
+
+ status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_contents,
+ xattr_contents_len, iint);
switch (status) {
case INTEGRITY_PASS:
case INTEGRITY_PASS_IMMUTABLE:
case INTEGRITY_UNKNOWN:
break;
case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */
+ /* It's fine not to have xattrs when using a modsig. */
+ if (xattr_value->type == IMA_MODSIG)
+ break;
case INTEGRITY_NOLABEL: /* No security.evm xattr. */
cause = "missing-HMAC";
goto out;
@@ -288,11 +324,16 @@ int ima_appraise_measurement(enum ima_hooks func,
status = INTEGRITY_PASS;
break;
case EVM_IMA_XATTR_DIGSIG:
+ case IMA_MODSIG:
set_bit(IMA_DIGSIG, &iint->atomic_flags);
- rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
- (const char *)xattr_value, rc,
- iint->ima_hash->digest,
- iint->ima_hash->length);
+ if (xattr_value->type == EVM_IMA_XATTR_DIGSIG)
+ rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+ (const char *)xattr_value,
+ rc, iint->ima_hash->digest,
+ iint->ima_hash->length);
+ else
+ rc = ima_modsig_verify(INTEGRITY_KEYRING_IMA,
+ xattr_value);
if (rc == -EOPNOTSUPP) {
status = INTEGRITY_UNKNOWN;
} else if (rc) {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5d122daf5c8a..1b11c10f09df 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -183,7 +183,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
struct evm_ima_xattr_data *xattr_value = NULL;
int xattr_len = 0;
bool violation_check;
- enum hash_algo hash_algo;
+ enum hash_algo hash_algo = HASH_ALGO__LAST;
if (!ima_policy_flag || !S_ISREG(inode->i_mode))
return 0;
@@ -277,11 +277,24 @@ static int process_measurement(struct file *file, const struct cred *cred,
template_desc = ima_template_desc_current();
if ((action & IMA_APPRAISE_SUBMASK) ||
- strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
+ strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) {
/* read 'security.ima' */
xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
+ if (iint->flags & IMA_MODSIG_ALLOWED &&
+ (xattr_len <= 0 || !ima_xattr_sig_known_key(xattr_value,
+ xattr_len))) {
+ /*
+ * Even if we end up using a modsig, hash_algo should
+ * come from the xattr (or even the default hash algo).
+ */
+ hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
+ ima_read_modsig(func, buf, size, &xattr_value,
+ &xattr_len);
+ }
+ }
- hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
+ if (hash_algo == HASH_ALGO__LAST)
+ hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
rc = ima_collect_measurement(iint, file, buf, size, hash_algo);
if (rc != 0 && rc != -EBADF && rc != -EINVAL)
@@ -309,7 +322,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
!(iint->flags & IMA_NEW_FILE))
rc = -EACCES;
mutex_unlock(&iint->mutex);
- kfree(xattr_value);
+ ima_free_xattr_data(xattr_value);
out:
if (pathbuf)
__putname(pathbuf);
Define new "d-sig" template field which holds the digest that is expected
to match the one contained in the modsig.
Also add modsig support to the "sig" template field, allowing the the
contents of the modsig to be included in the measurement list.
Suggested-by: Mimi Zohar <[email protected]>
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
Documentation/security/IMA-templates.rst | 5 ++++
security/integrity/ima/ima_template.c | 4 ++-
security/integrity/ima/ima_template_lib.c | 47 +++++++++++++++++++++++++++++--
security/integrity/ima/ima_template_lib.h | 2 ++
4 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 2cd0e273cc9a..f2a0f4225857 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -68,6 +68,11 @@ descriptors by adding their identifier to the format string
- 'd-ng': the digest of the event, calculated with an arbitrary hash
algorithm (field format: [<hash algo>:]digest, where the digest
prefix is shown only if the hash algorithm is not SHA1 or MD5);
+ - 'd-sig': the digest of the event for files that have an appended modsig. This
+ field is calculated without including the modsig and thus will differ from
+ the total digest of the file, but it is what should match the digest
+ contained in the modsig (if it doesn't, the signature is invalid). It is
+ shown in the same format as 'd-ng';
- 'n-ng': the name of the event, without size limitations;
- 'sig': the file signature.
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 30db39b23804..36fc32f538b5 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -43,8 +43,10 @@ static struct ima_template_field supported_fields[] = {
.field_show = ima_show_template_string},
{.field_id = "sig", .field_init = ima_eventsig_init,
.field_show = ima_show_template_sig},
+ {.field_id = "d-sig", .field_init = ima_eventdigest_sig_init,
+ .field_show = ima_show_template_digest_ng},
};
-#define MAX_TEMPLATE_NAME_LEN 15
+#define MAX_TEMPLATE_NAME_LEN 24
static struct ima_template_desc *ima_template;
static struct ima_template_desc *lookup_template_desc(const char *name);
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index afb52a90e532..1dca082cce43 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -220,7 +220,8 @@ int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
return 0;
}
-static int ima_eventdigest_init_common(u8 *digest, u32 digestsize, u8 hash_algo,
+static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
+ u8 hash_algo,
struct ima_field_data *field_data)
{
/*
@@ -323,6 +324,35 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
hash_algo, field_data);
}
+/*
+ * This function writes the digest of the file which is expected to match the
+ * digest contained in the file's embedded signature.
+ */
+int ima_eventdigest_sig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data)
+{
+ struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
+ enum hash_algo hash_algo = HASH_ALGO_SHA1;
+ const u8 *cur_digest = NULL;
+ u8 cur_digestsize = 0;
+ int ret;
+
+ if (!xattr_value || xattr_value->type != IMA_MODSIG)
+ return 0;
+
+ if (event_data->violation) /* recording a violation. */
+ goto out;
+
+ ret = ima_get_modsig_hash(xattr_value, &hash_algo, &cur_digest,
+ &cur_digestsize);
+ if (ret)
+ return ret;
+
+ out:
+ return ima_eventdigest_init_common(cur_digest, cur_digestsize,
+ hash_algo, field_data);
+}
+
static int ima_eventname_init_common(struct ima_event_data *event_data,
struct ima_field_data *field_data,
bool size_limit)
@@ -379,10 +409,23 @@ int ima_eventsig_init(struct ima_event_data *event_data,
struct ima_field_data *field_data)
{
struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
+ int xattr_len = event_data->xattr_len;
if (!is_ima_sig(xattr_value))
return 0;
- return ima_write_template_field_data(xattr_value, event_data->xattr_len,
+ /*
+ * The xattr_value for IMA_MODSIG is a runtime structure containing
+ * pointers. Get its raw data instead.
+ */
+ if (xattr_value->type == IMA_MODSIG) {
+ int rc;
+
+ rc = ima_modsig_serialize_data(&xattr_value, &xattr_len);
+ if (rc)
+ return rc;
+ }
+
+ return ima_write_template_field_data(xattr_value, xattr_len,
DATA_FMT_HEX, field_data);
}
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index 6a3d8b831deb..3cd353e83f73 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -38,6 +38,8 @@ int ima_eventname_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventdigest_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
+int ima_eventdigest_sig_init(struct ima_event_data *event_data,
+ struct ima_field_data *field_data);
int ima_eventname_ng_init(struct ima_event_data *event_data,
struct ima_field_data *field_data);
int ima_eventsig_init(struct ima_event_data *event_data,
IMA will need to obtain the keyring used to verify file signatures so that
it can verify the module-style signature appended to files.
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
security/integrity/digsig.c | 28 +++++++++++++++++++++-------
security/integrity/integrity.h | 6 ++++++
2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 6f9e4ce568cd..e641a67b9fc7 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -48,11 +48,10 @@ static bool init_keyring __initdata;
#define restrict_link_to_ima restrict_link_by_builtin_trusted
#endif
-int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
- const char *digest, int digestlen)
+struct key *integrity_keyring_from_id(const unsigned int id)
{
- if (id >= INTEGRITY_KEYRING_MAX || siglen < 2)
- return -EINVAL;
+ if (id >= INTEGRITY_KEYRING_MAX)
+ return ERR_PTR(-EINVAL);
if (!keyring[id]) {
keyring[id] =
@@ -61,17 +60,32 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
int err = PTR_ERR(keyring[id]);
pr_err("no %s keyring: %d\n", keyring_name[id], err);
keyring[id] = NULL;
- return err;
+ return ERR_PTR(err);
}
}
+ return keyring[id];
+}
+
+int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
+ const char *digest, int digestlen)
+{
+ struct key *keyring;
+
+ if (siglen < 2)
+ return -EINVAL;
+
+ keyring = integrity_keyring_from_id(id);
+ if (IS_ERR(keyring))
+ return PTR_ERR(keyring);
+
switch (sig[1]) {
case 1:
/* v1 API expect signature without xattr type */
- return digsig_verify(keyring[id], sig + 1, siglen - 1,
+ return digsig_verify(keyring, sig + 1, siglen - 1,
digest, digestlen);
case 2:
- return asymmetric_verify(keyring[id], sig, siglen,
+ return asymmetric_verify(keyring, sig, siglen,
digest, digestlen);
}
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 79799a0d9195..2d245f44ca26 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -150,6 +150,7 @@ int integrity_kernel_read(struct file *file, loff_t offset,
#ifdef CONFIG_INTEGRITY_SIGNATURE
+struct key *integrity_keyring_from_id(const unsigned int id);
int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
const char *digest, int digestlen);
@@ -157,6 +158,11 @@ int __init integrity_init_keyring(const unsigned int id);
int __init integrity_load_x509(const unsigned int id, const char *path);
#else
+static inline struct key *integrity_keyring_from_id(const unsigned int id)
+{
+ return ERR_PTR(-EINVAL);
+}
+
static inline int integrity_digsig_verify(const unsigned int id,
const char *sig, int siglen,
const char *digest, int digestlen)
IMA will only look for a modsig if the xattr sig references a key which is
not in the expected kernel keyring. To that end, introduce
asymmetric_sig_has_known_key().
The logic of extracting the key used in the xattr sig is factored out from
asymmetric_verify() so that it can be used by the new function.
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
security/integrity/digsig_asymmetric.c | 44 +++++++++++++++++++++++++---------
security/integrity/integrity.h | 8 +++++++
2 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
index ab6a029062a1..241647970c19 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -79,26 +79,48 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
return key;
}
-int asymmetric_verify(struct key *keyring, const char *sig,
- int siglen, const char *data, int datalen)
+static struct key *asymmetric_key_from_sig(struct key *keyring, const char *sig,
+ int siglen)
{
- struct public_key_signature pks;
- struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
- struct key *key;
- int ret = -ENOMEM;
+ const struct signature_v2_hdr *hdr = (struct signature_v2_hdr *) sig;
if (siglen <= sizeof(*hdr))
- return -EBADMSG;
+ return ERR_PTR(-EBADMSG);
siglen -= sizeof(*hdr);
if (siglen != be16_to_cpu(hdr->sig_size))
- return -EBADMSG;
+ return ERR_PTR(-EBADMSG);
if (hdr->hash_algo >= HASH_ALGO__LAST)
- return -ENOPKG;
+ return ERR_PTR(-ENOPKG);
+
+ return request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
+}
+
+bool asymmetric_sig_has_known_key(struct key *keyring, const char *sig,
+ int siglen)
+{
+ struct key *key;
+
+ key = asymmetric_key_from_sig(keyring, sig, siglen);
+ if (IS_ERR_OR_NULL(key))
+ return false;
+
+ key_put(key);
+
+ return true;
+}
+
+int asymmetric_verify(struct key *keyring, const char *sig,
+ int siglen, const char *data, int datalen)
+{
+ struct public_key_signature pks;
+ struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
+ struct key *key;
+ int ret = -ENOMEM;
- key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
+ key = asymmetric_key_from_sig(keyring, sig, siglen);
if (IS_ERR(key))
return PTR_ERR(key);
@@ -109,7 +131,7 @@ int asymmetric_verify(struct key *keyring, const char *sig,
pks.digest = (u8 *)data;
pks.digest_size = datalen;
pks.s = hdr->sig;
- pks.s_size = siglen;
+ pks.s_size = siglen - sizeof(*hdr);
ret = verify_signature(key, &pks);
key_put(key);
pr_debug("%s() = %d\n", __func__, ret);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 2d245f44ca26..4c381b992e11 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -179,12 +179,20 @@ static inline int integrity_init_keyring(const unsigned int id)
#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
int asymmetric_verify(struct key *keyring, const char *sig,
int siglen, const char *data, int datalen);
+bool asymmetric_sig_has_known_key(struct key *keyring, const char *sig,
+ int siglen);
#else
static inline int asymmetric_verify(struct key *keyring, const char *sig,
int siglen, const char *data, int datalen)
{
return -EOPNOTSUPP;
}
+
+static inline bool asymmetric_sig_has_known_key(struct key *keyring,
+ const char *sig, int siglen)
+{
+ return false;
+}
#endif
#ifdef CONFIG_IMA_LOAD_X509
IMA will need to access the digest of the PKCS7 message (as calculated by
the kernel) before the signature is verified, so introduce
pkcs7_get_digest() for that purpose.
Also, modify pkcs7_digest() to detect when the digest was already
calculated so that it doesn't have to do redundant work. Verifying that
sinfo->sig->digest isn't NULL is sufficient because both places which
allocate sinfo->sig (pkcs7_parse_message() and pkcs7_note_signed_info())
use kzalloc() so sig->digest is always initialized to zero.
Signed-off-by: Thiago Jung Bauermann <[email protected]>
Cc: David Howells <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: "David S. Miller" <[email protected]>
---
crypto/asymmetric_keys/pkcs7_verify.c | 25 +++++++++++++++++++++++++
include/crypto/pkcs7.h | 3 +++
2 files changed, 28 insertions(+)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 39e6de0c2761..bd02360f8be5 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -33,6 +33,10 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
kenter(",%u,%s", sinfo->index, sinfo->sig->hash_algo);
+ /* The digest was calculated already. */
+ if (sig->digest)
+ return 0;
+
if (!sinfo->sig->hash_algo)
return -ENOPKG;
@@ -122,6 +126,27 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
return ret;
}
+int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf, u8 *len)
+{
+ struct pkcs7_signed_info *sinfo = pkcs7->signed_infos;
+ int ret;
+
+ /*
+ * This function doesn't support messages with more than one signature.
+ */
+ if (sinfo == NULL || sinfo->next != NULL)
+ return -EBADMSG;
+
+ ret = pkcs7_digest(pkcs7, sinfo);
+ if (ret)
+ return ret;
+
+ *buf = sinfo->sig->digest;
+ *len = sinfo->sig->digest_size;
+
+ return 0;
+}
+
/*
* Find the key (X.509 certificate) to use to verify a PKCS#7 message. PKCS#7
* uses the issuer's name and the issuing certificate serial number for
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 6f51d0cb6d12..cfaea9c37f4a 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -46,4 +46,7 @@ extern int pkcs7_verify(struct pkcs7_message *pkcs7,
extern int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
const void *data, size_t datalen);
+extern int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf,
+ u8 *len);
+
#endif /* _CRYPTO_PKCS7_H */
This is the code needed by IMA-appraise to work with modsig signatures.
It will be used by the next two patches.
Signed-off-by: Thiago Jung Bauermann <[email protected]>
---
security/integrity/ima/Kconfig | 3 +
security/integrity/ima/ima.h | 41 ++++++++
security/integrity/ima/ima_modsig.c | 181 ++++++++++++++++++++++++++++++++++++
security/integrity/integrity.h | 1 +
4 files changed, 226 insertions(+)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index ee278189e0bb..306601d62f0b 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -167,6 +167,9 @@ config IMA_APPRAISE_BOOTPARAM
config IMA_APPRAISE_MODSIG
bool "Support module-style signatures for appraisal"
depends on IMA_APPRAISE
+ depends on INTEGRITY_ASYMMETRIC_KEYS
+ select PKCS7_MESSAGE_PARSER
+ select MODULE_SIG_FORMAT
default n
help
Adds support for signatures appended to files. The format of the
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c61d8fc5190d..49aef56dc96d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -301,11 +301,52 @@ static inline int ima_read_xattr(struct dentry *dentry,
#ifdef CONFIG_IMA_APPRAISE_MODSIG
bool ima_hook_supports_modsig(enum ima_hooks func);
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+ struct evm_ima_xattr_data **xattr_value,
+ int *xattr_len);
+int ima_get_modsig_hash(struct evm_ima_xattr_data *hdr, enum hash_algo *algo,
+ const u8 **hash, u8 *len);
+int ima_modsig_serialize_data(struct evm_ima_xattr_data **data, int *data_len);
+int ima_modsig_verify(const unsigned int keyring_id,
+ struct evm_ima_xattr_data *hdr);
+void ima_free_xattr_data(struct evm_ima_xattr_data *hdr);
#else
static inline bool ima_hook_supports_modsig(enum ima_hooks func)
{
return false;
}
+
+static inline int ima_read_modsig(enum ima_hooks func, const void *buf,
+ loff_t buf_len,
+ struct evm_ima_xattr_data **xattr_value,
+ int *xattr_len)
+{
+ return -ENOTSUPP;
+}
+
+static inline int ima_get_modsig_hash(struct evm_ima_xattr_data *hdr,
+ enum hash_algo *algo, const u8 **hash,
+ u8 *len)
+{
+ return -ENOTSUPP;
+}
+
+static inline int ima_modsig_serialize_data(struct evm_ima_xattr_data **data,
+ int *data_len)
+{
+ return -ENOTSUPP;
+}
+
+static inline int ima_modsig_verify(const unsigned int keyring_id,
+ struct evm_ima_xattr_data *hdr)
+{
+ return -ENOTSUPP;
+}
+
+static inline void ima_free_xattr_data(struct evm_ima_xattr_data *hdr)
+{
+ kfree(hdr);
+}
#endif /* CONFIG_IMA_APPRAISE_MODSIG */
/* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index d8ea811b6f74..105fd04d585e 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -8,8 +8,25 @@
* Thiago Jung Bauermann <[email protected]>
*/
+#include <linux/types.h>
+#include <linux/module_signature.h>
+#include <keys/asymmetric-type.h>
+#include <crypto/pkcs7.h>
+
#include "ima.h"
+struct modsig_hdr {
+ uint8_t type; /* Should be IMA_MODSIG. */
+ struct pkcs7_message *pkcs7_msg;
+ int raw_pkcs7_len;
+
+ /*
+ * This is what will go to the measurement list if the template requires
+ * storing the signature.
+ */
+ struct evm_ima_xattr_data raw_pkcs7;
+};
+
/**
* ima_hook_supports_modsig - can the policy allow modsig for this hook?
*
@@ -29,3 +46,167 @@ bool ima_hook_supports_modsig(enum ima_hooks func)
return false;
}
}
+
+static bool modsig_has_known_key(struct modsig_hdr *hdr)
+{
+ const struct public_key_signature *pks;
+ struct key *keyring;
+ struct key *key;
+
+ keyring = integrity_keyring_from_id(INTEGRITY_KEYRING_IMA);
+ if (IS_ERR(keyring))
+ return false;
+
+ pks = pkcs7_get_message_sig(hdr->pkcs7_msg);
+ if (!pks)
+ return false;
+
+ key = find_asymmetric_key(keyring, pks->auth_ids[0], NULL, false);
+ if (IS_ERR(key))
+ return false;
+
+ key_put(key);
+
+ return true;
+}
+
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+ struct evm_ima_xattr_data **xattr_value,
+ int *xattr_len)
+{
+ const size_t marker_len = sizeof(MODULE_SIG_STRING) - 1;
+ const struct module_signature *sig;
+ struct modsig_hdr *hdr;
+ size_t sig_len;
+ const void *p;
+ int rc;
+
+ /*
+ * Not supposed to happen. Hooks that support modsig are whitelisted
+ * when parsing the policy using ima_hooks_supports_modsig().
+ */
+ if (!buf || !buf_len) {
+ WARN_ONCE(true, "%s doesn't support modsig\n",
+ func_tokens[func]);
+ return -ENOENT;
+ } else 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 = validate_module_sig(sig, buf_len);
+ 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 = kmalloc(sizeof(*hdr) + sig_len, GFP_KERNEL);
+ if (!hdr)
+ return -ENOMEM;
+
+ hdr->pkcs7_msg = pkcs7_parse_message(buf + buf_len, sig_len);
+ if (IS_ERR(hdr->pkcs7_msg)) {
+ rc = PTR_ERR(hdr->pkcs7_msg);
+ goto err_no_msg;
+ }
+
+ rc = pkcs7_supply_detached_data(hdr->pkcs7_msg, buf, buf_len);
+ if (rc)
+ goto err;
+
+ if (!modsig_has_known_key(hdr)) {
+ rc = -ENOKEY;
+ goto err;
+ }
+
+ memcpy(hdr->raw_pkcs7.data, buf + buf_len, sig_len);
+ hdr->raw_pkcs7_len = sig_len + 1;
+ hdr->raw_pkcs7.type = IMA_MODSIG;
+
+ hdr->type = IMA_MODSIG;
+
+ *xattr_value = (typeof(*xattr_value)) hdr;
+ *xattr_len = sizeof(*hdr);
+
+ return 0;
+
+ err:
+ pkcs7_free_message(hdr->pkcs7_msg);
+ err_no_msg:
+ kfree(hdr);
+ return rc;
+}
+
+int ima_get_modsig_hash(struct evm_ima_xattr_data *hdr, enum hash_algo *algo,
+ const u8 **hash, u8 *len)
+{
+ struct modsig_hdr *modsig = (typeof(modsig)) hdr;
+ const struct public_key_signature *pks;
+ int i;
+
+ if (!hdr || hdr->type != IMA_MODSIG)
+ return -EINVAL;
+
+ pks = pkcs7_get_message_sig(modsig->pkcs7_msg);
+ if (!pks)
+ return -EBADMSG;
+
+ for (i = 0; i < HASH_ALGO__LAST; i++)
+ if (!strcmp(hash_algo_name[i], pks->hash_algo))
+ break;
+
+ *algo = i;
+
+ return pkcs7_get_digest(modsig->pkcs7_msg, hash, len);
+}
+
+int ima_modsig_serialize_data(struct evm_ima_xattr_data **data, int *data_len)
+{
+ struct modsig_hdr *modsig = (struct modsig_hdr *) *data;
+
+ if (!*data || (*data)->type != IMA_MODSIG)
+ return -EINVAL;
+
+ *data = &modsig->raw_pkcs7;
+ *data_len = modsig->raw_pkcs7_len;
+
+ return 0;
+}
+
+int ima_modsig_verify(const unsigned int keyring_id,
+ struct evm_ima_xattr_data *hdr)
+{
+ struct modsig_hdr *modsig = (struct modsig_hdr *) hdr;
+ struct key *keyring;
+
+ if (!hdr || hdr->type != IMA_MODSIG)
+ return -EINVAL;
+
+ keyring = integrity_keyring_from_id(keyring_id);
+ if (IS_ERR(keyring))
+ return PTR_ERR(keyring);
+
+ return verify_pkcs7_message_sig(NULL, 0, modsig->pkcs7_msg, keyring,
+ VERIFYING_MODULE_SIGNATURE, NULL, NULL);
+}
+
+void ima_free_xattr_data(struct evm_ima_xattr_data *hdr)
+{
+ if (!hdr)
+ return;
+
+ if (hdr->type == IMA_MODSIG) {
+ struct modsig_hdr *modsig = (struct modsig_hdr *) hdr;
+
+ pkcs7_free_message(modsig->pkcs7_msg);
+ }
+
+ kfree(hdr);
+}
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 6643c6550787..4acb1fb86b3b 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -74,6 +74,7 @@ enum evm_ima_xattr_type {
EVM_IMA_XATTR_DIGSIG,
IMA_XATTR_DIGEST_NG,
EVM_XATTR_PORTABLE_DIGSIG,
+ IMA_MODSIG,
IMA_XATTR_LAST
};
Thiago Jung Bauermann <[email protected]> writes:
> Now the modsig is only ignored if it references a signature that is not
> present in IMA's keyring (or if there's a parsing error, obviously). If the
The above should read "Now the modsig is only ignored if it references a
*key* that is not present in IMA's keyring".
Sorry, I only noticed the mistake after sending the emails.
--
Thiago Jung Bauermann
IBM Linux Technology Center
On Fri, 2018-03-16 at 17:38 -0300, Thiago Jung Bauermann wrote:
> IMA will need to obtain the keyring used to verify file signatures so that
> it can verify the module-style signature appended to files.
>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
> ---
> security/integrity/digsig.c | 28 +++++++++++++++++++++-------
> security/integrity/integrity.h | 6 ++++++
> 2 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 6f9e4ce568cd..e641a67b9fc7 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -48,11 +48,10 @@ static bool init_keyring __initdata;
> #define restrict_link_to_ima restrict_link_by_builtin_trusted
> #endif
>
> -int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> - const char *digest, int digestlen)
> +struct key *integrity_keyring_from_id(const unsigned int id)
> {
> - if (id >= INTEGRITY_KEYRING_MAX || siglen < 2)
> - return -EINVAL;
> + if (id >= INTEGRITY_KEYRING_MAX)
> + return ERR_PTR(-EINVAL);
>
> if (!keyring[id]) {
> keyring[id] =
> @@ -61,17 +60,32 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> int err = PTR_ERR(keyring[id]);
> pr_err("no %s keyring: %d\n", keyring_name[id], err);
> keyring[id] = NULL;
> - return err;
> + return ERR_PTR(err);
> }
> }
>
> + return keyring[id];
> +}
> +
> +int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> + const char *digest, int digestlen)
> +{
> + struct key *keyring;
> +
> + if (siglen < 2)
> + return -EINVAL;
> +
> + keyring = integrity_keyring_from_id(id);
> + if (IS_ERR(keyring))
> + return PTR_ERR(keyring);
> +
> switch (sig[1]) {
> case 1:
> /* v1 API expect signature without xattr type */
> - return digsig_verify(keyring[id], sig + 1, siglen - 1,
> + return digsig_verify(keyring, sig + 1, siglen - 1,
> digest, digestlen);
> case 2:
> - return asymmetric_verify(keyring[id], sig, siglen,
> + return asymmetric_verify(keyring, sig, siglen,
> digest, digestlen);
> }
>
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 79799a0d9195..2d245f44ca26 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -150,6 +150,7 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>
> #ifdef CONFIG_INTEGRITY_SIGNATURE
>
> +struct key *integrity_keyring_from_id(const unsigned int id);
> int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> const char *digest, int digestlen);
>
> @@ -157,6 +158,11 @@ int __init integrity_init_keyring(const unsigned int id);
> int __init integrity_load_x509(const unsigned int id, const char *path);
> #else
>
> +static inline struct key *integrity_keyring_from_id(const unsigned int id)
> +{
> + return ERR_PTR(-EINVAL);
> +}
> +
> static inline int integrity_digsig_verify(const unsigned int id,
> const char *sig, int siglen,
> const char *digest, int digestlen)
>
On Fri, 2018-03-16 at 17:38 -0300, Thiago Jung Bauermann wrote:
> IMA will only look for a modsig if the xattr sig references a key which is
> not in the expected kernel keyring. To that end, introduce
> asymmetric_sig_has_known_key().
>
> The logic of extracting the key used in the xattr sig is factored out from
> asymmetric_verify() so that it can be used by the new function.
>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
> ---
> security/integrity/digsig_asymmetric.c | 44 +++++++++++++++++++++++++---------
> security/integrity/integrity.h | 8 +++++++
> 2 files changed, 41 insertions(+), 11 deletions(-)
>
> diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
> index ab6a029062a1..241647970c19 100644
> --- a/security/integrity/digsig_asymmetric.c
> +++ b/security/integrity/digsig_asymmetric.c
> @@ -79,26 +79,48 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
> return key;
> }
>
> -int asymmetric_verify(struct key *keyring, const char *sig,
> - int siglen, const char *data, int datalen)
> +static struct key *asymmetric_key_from_sig(struct key *keyring, const char *sig,
> + int siglen)
> {
> - struct public_key_signature pks;
> - struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
> - struct key *key;
> - int ret = -ENOMEM;
> + const struct signature_v2_hdr *hdr = (struct signature_v2_hdr *) sig;
>
> if (siglen <= sizeof(*hdr))
> - return -EBADMSG;
> + return ERR_PTR(-EBADMSG);
>
> siglen -= sizeof(*hdr);
>
> if (siglen != be16_to_cpu(hdr->sig_size))
> - return -EBADMSG;
> + return ERR_PTR(-EBADMSG);
>
> if (hdr->hash_algo >= HASH_ALGO__LAST)
> - return -ENOPKG;
> + return ERR_PTR(-ENOPKG);
> +
> + return request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
> +}
> +
> +bool asymmetric_sig_has_known_key(struct key *keyring, const char *sig,
> + int siglen)
> +{
> + struct key *key;
> +
> + key = asymmetric_key_from_sig(keyring, sig, siglen);
> + if (IS_ERR_OR_NULL(key))
> + return false;
> +
> + key_put(key);
> +
> + return true;
> +}
> +
> +int asymmetric_verify(struct key *keyring, const char *sig,
> + int siglen, const char *data, int datalen)
> +{
> + struct public_key_signature pks;
> + struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
> + struct key *key;
> + int ret = -ENOMEM;
>
> - key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
> + key = asymmetric_key_from_sig(keyring, sig, siglen);
> if (IS_ERR(key))
> return PTR_ERR(key);
>
> @@ -109,7 +131,7 @@ int asymmetric_verify(struct key *keyring, const char *sig,
> pks.digest = (u8 *)data;
> pks.digest_size = datalen;
> pks.s = hdr->sig;
> - pks.s_size = siglen;
> + pks.s_size = siglen - sizeof(*hdr);
> ret = verify_signature(key, &pks);
> key_put(key);
> pr_debug("%s() = %d\n", __func__, ret);
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 2d245f44ca26..4c381b992e11 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -179,12 +179,20 @@ static inline int integrity_init_keyring(const unsigned int id)
> #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
> int asymmetric_verify(struct key *keyring, const char *sig,
> int siglen, const char *data, int datalen);
> +bool asymmetric_sig_has_known_key(struct key *keyring, const char *sig,
> + int siglen);
> #else
> static inline int asymmetric_verify(struct key *keyring, const char *sig,
> int siglen, const char *data, int datalen)
> {
> return -EOPNOTSUPP;
> }
> +
> +static inline bool asymmetric_sig_has_known_key(struct key *keyring,
> + const char *sig, int siglen)
> +{
> + return false;
> +}
> #endif
>
> #ifdef CONFIG_IMA_LOAD_X509
>
On Fri, 2018-03-16 at 17:38 -0300, Thiago Jung Bauermann wrote:
> This avoids a dependency cycle in CONFIG_IMA_APPRAISE_MODSIG (introduced by
> a later patch in this series): it will select CONFIG_MODULE_SIG_FORMAT
> which in turn selects CONFIG_KEYS. Kconfig then complains that
> CONFIG_INTEGRITY_SIGNATURE depends on CONFIG_KEYS.
>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
> ---
> security/integrity/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
> index da9565891738..0d642e0317c7 100644
> --- a/security/integrity/Kconfig
> +++ b/security/integrity/Kconfig
> @@ -17,8 +17,8 @@ if INTEGRITY
>
> config INTEGRITY_SIGNATURE
> bool "Digital signature verification using multiple keyrings"
> - depends on KEYS
> default n
> + select KEYS
> select SIGNATURE
> help
> This option enables digital signature verification support
>
Hi Thiago,
On Fri, 2018-03-16 at 17:38 -0300, Thiago Jung Bauermann wrote:
> IMA will need to know the key that signed a given PKCS#7 message, so add
> pkcs7_get_message_sig().
>
> It will also need to verify an already parsed PKCS#7 message. For this
> purpose, add verify_pkcs7_message_sig() which takes a struct pkcs7_message
> for verification instead of the raw bytes that verify_pkcs7_signature()
> takes.
The title "PKCS#7: refactor verify_pkcs7_signature()" might be more
appropriate. The patch description would then explain why it needs to
be refactored. In this case, verify_pkcs7_signature() verifies the
signature using keys on the builtin and secondary keyrings. IMA-
appraisal needs to verify the signature using keys on its keyring.
The patch itself looks good!
Reviewed-by: Mimi Zohar <[email protected]>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
> Cc: David Howells <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Cc: Herbert Xu <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> ---
> certs/system_keyring.c | 61 ++++++++++++++++++++++++++---------
> crypto/asymmetric_keys/pkcs7_parser.c | 16 +++++++++
> include/crypto/pkcs7.h | 2 ++
> include/linux/verification.h | 10 ++++++
> 4 files changed, 73 insertions(+), 16 deletions(-)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 6251d1b27f0c..7ddc8b7a3062 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -190,33 +190,27 @@ late_initcall(load_system_certificate_list);
> #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
>
> /**
> - * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
> + * verify_pkcs7_message_sig - Verify a PKCS#7-based signature on system data.
> * @data: The data to be verified (NULL if expecting internal data).
> * @len: Size of @data.
> - * @raw_pkcs7: The PKCS#7 message that is the signature.
> - * @pkcs7_len: The size of @raw_pkcs7.
> + * @pkcs7: The PKCS#7 message that is the signature.
> * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
> * (void *)1UL for all trusted keys).
> * @usage: The use to which the key is being put.
> * @view_content: Callback to gain access to content.
> * @ctx: Context for callback.
> */
> -int verify_pkcs7_signature(const void *data, size_t len,
> - const void *raw_pkcs7, size_t pkcs7_len,
> - struct key *trusted_keys,
> - enum key_being_used_for usage,
> - int (*view_content)(void *ctx,
> - const void *data, size_t len,
> - size_t asn1hdrlen),
> - void *ctx)
> +int verify_pkcs7_message_sig(const void *data, size_t len,
> + struct pkcs7_message *pkcs7,
> + struct key *trusted_keys,
> + enum key_being_used_for usage,
> + int (*view_content)(void *ctx,
> + const void *data, size_t len,
> + size_t asn1hdrlen),
> + void *ctx)
> {
> - struct pkcs7_message *pkcs7;
> int ret;
>
> - pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
> - if (IS_ERR(pkcs7))
> - return PTR_ERR(pkcs7);
> -
> /* The data should be detached - so we need to supply it. */
> if (data && pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
> pr_err("PKCS#7 signature with non-detached data\n");
> @@ -258,6 +252,41 @@ int verify_pkcs7_signature(const void *data, size_t len,
> }
>
> error:
> + pr_devel("<==%s() = %d\n", __func__, ret);
> + return ret;
> +}
> +
> +/**
> + * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
> + * @data: The data to be verified (NULL if expecting internal data).
> + * @len: Size of @data.
> + * @raw_pkcs7: The PKCS#7 message that is the signature.
> + * @pkcs7_len: The size of @raw_pkcs7.
> + * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
> + * (void *)1UL for all trusted keys).
> + * @usage: The use to which the key is being put.
> + * @view_content: Callback to gain access to content.
> + * @ctx: Context for callback.
> + */
> +int verify_pkcs7_signature(const void *data, size_t len,
> + const void *raw_pkcs7, size_t pkcs7_len,
> + struct key *trusted_keys,
> + enum key_being_used_for usage,
> + int (*view_content)(void *ctx,
> + const void *data, size_t len,
> + size_t asn1hdrlen),
> + void *ctx)
> +{
> + struct pkcs7_message *pkcs7;
> + int ret;
> +
> + pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
> + if (IS_ERR(pkcs7))
> + return PTR_ERR(pkcs7);
> +
> + ret = verify_pkcs7_message_sig(data, len, pkcs7, trusted_keys, usage,
> + view_content, ctx);
> +
> pkcs7_free_message(pkcs7);
> pr_devel("<==%s() = %d\n", __func__, ret);
> return ret;
> diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
> index a6dcaa659aa8..456b803972b5 100644
> --- a/crypto/asymmetric_keys/pkcs7_parser.c
> +++ b/crypto/asymmetric_keys/pkcs7_parser.c
> @@ -683,3 +683,19 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
> return -ENOMEM;
> return 0;
> }
> +
> +/**
> + * pkcs7_get_message_sig - get signature in @pkcs7
> + */
> +const struct public_key_signature *pkcs7_get_message_sig(
> + const struct pkcs7_message *pkcs7)
> +{
> + /*
> + * This function doesn't support messages with more than one signature,
> + * so don't return anything in that case.
> + */
> + if (pkcs7->signed_infos == NULL || pkcs7->signed_infos->next != NULL)
> + return NULL;
> +
> + return pkcs7->signed_infos->sig;
> +}
> diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> index 583f199400a3..6f51d0cb6d12 100644
> --- a/include/crypto/pkcs7.h
> +++ b/include/crypto/pkcs7.h
> @@ -28,6 +28,8 @@ extern void pkcs7_free_message(struct pkcs7_message *pkcs7);
> extern int pkcs7_get_content_data(const struct pkcs7_message *pkcs7,
> const void **_data, size_t *_datalen,
> size_t *_headerlen);
> +extern const struct public_key_signature *pkcs7_get_message_sig(
> + const struct pkcs7_message *pkcs7);
>
> /*
> * pkcs7_trust.c
> diff --git a/include/linux/verification.h b/include/linux/verification.h
> index a10549a6c7cd..f04dac2728ec 100644
> --- a/include/linux/verification.h
> +++ b/include/linux/verification.h
> @@ -29,6 +29,7 @@ extern const char *const key_being_used_for[NR__KEY_BEING_USED_FOR];
> #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
>
> struct key;
> +struct pkcs7_message;
>
> extern int verify_pkcs7_signature(const void *data, size_t len,
> const void *raw_pkcs7, size_t pkcs7_len,
> @@ -38,6 +39,15 @@ extern int verify_pkcs7_signature(const void *data, size_t len,
> const void *data, size_t len,
> size_t asn1hdrlen),
> void *ctx);
> +extern int verify_pkcs7_message_sig(const void *data, size_t len,
> + struct pkcs7_message *pkcs7,
> + struct key *trusted_keys,
> + enum key_being_used_for usage,
> + int (*view_content)(void *ctx,
> + const void *data,
> + size_t len,
> + size_t asn1hdrlen),
> + void *ctx);
>
> #ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
> extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
>
On Fri, 2018-03-16 at 17:38 -0300, Thiago Jung Bauermann wrote:
> IMA will need to access the digest of the PKCS7 message (as calculated by
> the kernel) before the signature is verified, so introduce
> pkcs7_get_digest() for that purpose.
>
> Also, modify pkcs7_digest() to detect when the digest was already
> calculated so that it doesn't have to do redundant work. Verifying that
> sinfo->sig->digest isn't NULL is sufficient because both places which
> allocate sinfo->sig (pkcs7_parse_message() and pkcs7_note_signed_info())
> use kzalloc() so sig->digest is always initialized to zero.
>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
> Cc: David Howells <[email protected]>
> Cc: Herbert Xu <[email protected]>
> Cc: "David S. Miller" <[email protected]>
Reviewed-by: Mimi Zohar <[email protected]>
> ---
> crypto/asymmetric_keys/pkcs7_verify.c | 25 +++++++++++++++++++++++++
> include/crypto/pkcs7.h | 3 +++
> 2 files changed, 28 insertions(+)
>
> diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
> index 39e6de0c2761..bd02360f8be5 100644
> --- a/crypto/asymmetric_keys/pkcs7_verify.c
> +++ b/crypto/asymmetric_keys/pkcs7_verify.c
> @@ -33,6 +33,10 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
>
> kenter(",%u,%s", sinfo->index, sinfo->sig->hash_algo);
>
> + /* The digest was calculated already. */
> + if (sig->digest)
> + return 0;
> +
> if (!sinfo->sig->hash_algo)
> return -ENOPKG;
>
> @@ -122,6 +126,27 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
> return ret;
> }
>
> +int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf, u8 *len)
> +{
> + struct pkcs7_signed_info *sinfo = pkcs7->signed_infos;
> + int ret;
> +
> + /*
> + * This function doesn't support messages with more than one signature.
> + */
> + if (sinfo == NULL || sinfo->next != NULL)
> + return -EBADMSG;
> +
> + ret = pkcs7_digest(pkcs7, sinfo);
> + if (ret)
> + return ret;
> +
> + *buf = sinfo->sig->digest;
> + *len = sinfo->sig->digest_size;
> +
> + return 0;
> +}
> +
> /*
> * Find the key (X.509 certificate) to use to verify a PKCS#7 message. PKCS#7
> * uses the issuer's name and the issuing certificate serial number for
> diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
> index 6f51d0cb6d12..cfaea9c37f4a 100644
> --- a/include/crypto/pkcs7.h
> +++ b/include/crypto/pkcs7.h
> @@ -46,4 +46,7 @@ extern int pkcs7_verify(struct pkcs7_message *pkcs7,
> extern int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
> const void *data, size_t datalen);
>
> +extern int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf,
> + u8 *len);
> +
> #endif /* _CRYPTO_PKCS7_H */
>
On Fri, 2018-03-16 at 17:38 -0300, Thiago Jung Bauermann wrote:
> Define new "d-sig" template field which holds the digest that is expected
> to match the one contained in the modsig.
>
> Also add modsig support to the "sig" template field, allowing the the
> contents of the modsig to be included in the measurement list.
Although including the appended signature in the template data doesn't
make sense on its own, as the file digest (without the appended
signature) is needed to validate the appended signature, defining a
new template field and its usage should be independent of other
changes.
Mimi
>
> Suggested-by: Mimi Zohar <[email protected]>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
> ---
> Documentation/security/IMA-templates.rst | 5 ++++
> security/integrity/ima/ima_template.c | 4 ++-
> security/integrity/ima/ima_template_lib.c | 47 +++++++++++++++++++++++++++++--
> security/integrity/ima/ima_template_lib.h | 2 ++
> 4 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
> index 2cd0e273cc9a..f2a0f4225857 100644
> --- a/Documentation/security/IMA-templates.rst
> +++ b/Documentation/security/IMA-templates.rst
> @@ -68,6 +68,11 @@ descriptors by adding their identifier to the format string
> - 'd-ng': the digest of the event, calculated with an arbitrary hash
> algorithm (field format: [<hash algo>:]digest, where the digest
> prefix is shown only if the hash algorithm is not SHA1 or MD5);
> + - 'd-sig': the digest of the event for files that have an appended modsig. This
> + field is calculated without including the modsig and thus will differ from
> + the total digest of the file, but it is what should match the digest
> + contained in the modsig (if it doesn't, the signature is invalid). It is
> + shown in the same format as 'd-ng';
> - 'n-ng': the name of the event, without size limitations;
> - 'sig': the file signature.
>
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 30db39b23804..36fc32f538b5 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -43,8 +43,10 @@ static struct ima_template_field supported_fields[] = {
> .field_show = ima_show_template_string},
> {.field_id = "sig", .field_init = ima_eventsig_init,
> .field_show = ima_show_template_sig},
> + {.field_id = "d-sig", .field_init = ima_eventdigest_sig_init,
> + .field_show = ima_show_template_digest_ng},
> };
> -#define MAX_TEMPLATE_NAME_LEN 15
> +#define MAX_TEMPLATE_NAME_LEN 24
>
> static struct ima_template_desc *ima_template;
> static struct ima_template_desc *lookup_template_desc(const char *name);
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index afb52a90e532..1dca082cce43 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -220,7 +220,8 @@ int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
> return 0;
> }
>
> -static int ima_eventdigest_init_common(u8 *digest, u32 digestsize, u8 hash_algo,
> +static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
> + u8 hash_algo,
> struct ima_field_data *field_data)
> {
> /*
> @@ -323,6 +324,35 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
> hash_algo, field_data);
> }
>
> +/*
> + * This function writes the digest of the file which is expected to match the
> + * digest contained in the file's embedded signature.
> + */
> +int ima_eventdigest_sig_init(struct ima_event_data *event_data,
> + struct ima_field_data *field_data)
> +{
> + struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
> + enum hash_algo hash_algo = HASH_ALGO_SHA1;
> + const u8 *cur_digest = NULL;
> + u8 cur_digestsize = 0;
> + int ret;
> +
> + if (!xattr_value || xattr_value->type != IMA_MODSIG)
> + return 0;
> +
> + if (event_data->violation) /* recording a violation. */
> + goto out;
> +
> + ret = ima_get_modsig_hash(xattr_value, &hash_algo, &cur_digest,
> + &cur_digestsize);
> + if (ret)
> + return ret;
> +
> + out:
> + return ima_eventdigest_init_common(cur_digest, cur_digestsize,
> + hash_algo, field_data);
> +}
> +
> static int ima_eventname_init_common(struct ima_event_data *event_data,
> struct ima_field_data *field_data,
> bool size_limit)
> @@ -379,10 +409,23 @@ int ima_eventsig_init(struct ima_event_data *event_data,
> struct ima_field_data *field_data)
> {
> struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
> + int xattr_len = event_data->xattr_len;
>
> if (!is_ima_sig(xattr_value))
> return 0;
>
> - return ima_write_template_field_data(xattr_value, event_data->xattr_len,
> + /*
> + * The xattr_value for IMA_MODSIG is a runtime structure containing
> + * pointers. Get its raw data instead.
> + */
> + if (xattr_value->type == IMA_MODSIG) {
> + int rc;
> +
> + rc = ima_modsig_serialize_data(&xattr_value, &xattr_len);
> + if (rc)
> + return rc;
> + }
> +
> + return ima_write_template_field_data(xattr_value, xattr_len,
> DATA_FMT_HEX, field_data);
> }
> diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
> index 6a3d8b831deb..3cd353e83f73 100644
> --- a/security/integrity/ima/ima_template_lib.h
> +++ b/security/integrity/ima/ima_template_lib.h
> @@ -38,6 +38,8 @@ int ima_eventname_init(struct ima_event_data *event_data,
> struct ima_field_data *field_data);
> int ima_eventdigest_ng_init(struct ima_event_data *event_data,
> struct ima_field_data *field_data);
> +int ima_eventdigest_sig_init(struct ima_event_data *event_data,
> + struct ima_field_data *field_data);
> int ima_eventname_ng_init(struct ima_event_data *event_data,
> struct ima_field_data *field_data);
> int ima_eventsig_init(struct ima_event_data *event_data,
>
On Fri, 2018-03-16 at 17:38 -0300, Thiago Jung Bauermann wrote:
> This patch actually implements the appraise_type=imasig|modsig option,
> allowing IMA to read and verify modsig signatures.
>
> In case both are present in the same file, IMA will first check whether the
> key used by the xattr signature is present in the kernel keyring. If not,
> it will try the appended signature.
Yes, this sounds right.
>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
> ---
> security/integrity/ima/ima.h | 11 +++++++-
> security/integrity/ima/ima_appraise.c | 53 +++++++++++++++++++++++++++++++----
> security/integrity/ima/ima_main.c | 21 +++++++++++---
> 3 files changed, 74 insertions(+), 11 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 49aef56dc96d..c11ccb7c5bfb 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -157,7 +157,8 @@ void ima_init_template_list(void);
>
> static inline bool is_ima_sig(const struct evm_ima_xattr_data *xattr_value)
> {
> - return xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG;
> + return xattr_value && (xattr_value->type == EVM_IMA_XATTR_DIGSIG ||
> + xattr_value->type == IMA_MODSIG);
> }
>
> /*
> @@ -253,6 +254,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
> enum ima_hooks func);
> enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
> int xattr_len);
> +bool ima_xattr_sig_known_key(const struct evm_ima_xattr_data *xattr_value,
> + int xattr_len);
> int ima_read_xattr(struct dentry *dentry,
> struct evm_ima_xattr_data **xattr_value);
>
> @@ -291,6 +294,12 @@ ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len)
> return ima_hash_algo;
> }
>
> +static inline bool ima_xattr_sig_known_key(const struct evm_ima_xattr_data
> + *xattr_value, int xattr_len)
> +{
> + return false;
> +}
> +
> static inline int ima_read_xattr(struct dentry *dentry,
> struct evm_ima_xattr_data **xattr_value)
> {
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 01172eab297b..84e0fd5a19c8 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -189,6 +189,22 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
> return ima_hash_algo;
> }
>
> +bool ima_xattr_sig_known_key(const struct evm_ima_xattr_data *xattr_value,
> + int xattr_len)
> +{
> + struct key *keyring;
> +
> + if (xattr_value->type != EVM_IMA_XATTR_DIGSIG)
> + return false;
> +
> + keyring = integrity_keyring_from_id(INTEGRITY_KEYRING_IMA);
> + if (IS_ERR(keyring))
> + return false;
> +
> + return asymmetric_sig_has_known_key(keyring, (const char *) xattr_value,
> + xattr_len);
> +}
> +
> int ima_read_xattr(struct dentry *dentry,
> struct evm_ima_xattr_data **xattr_value)
> {
> @@ -221,8 +237,12 @@ int ima_appraise_measurement(enum ima_hooks func,
> struct inode *inode = d_backing_inode(dentry);
> enum integrity_status status = INTEGRITY_UNKNOWN;
> int rc = xattr_len, hash_start = 0;
> + size_t xattr_contents_len;
> + void *xattr_contents;
>
> - if (!(inode->i_opflags & IOP_XATTR))
> + /* If not appraising a modsig, we need an xattr. */
> + if ((xattr_value == NULL || xattr_value->type != IMA_MODSIG) &&
> + !(inode->i_opflags & IOP_XATTR))
> return INTEGRITY_UNKNOWN;
>
> if (rc <= 0) {
> @@ -241,13 +261,29 @@ int ima_appraise_measurement(enum ima_hooks func,
> goto out;
> }
>
> - status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
> + /*
> + * If it's a modsig, we don't have the xattr contents to pass to
> + * evm_verifyxattr().
> + */
> + if (xattr_value->type == IMA_MODSIG) {
> + xattr_contents = NULL;
> + xattr_contents_len = 0;
> + } else {
> + xattr_contents = xattr_value;
> + xattr_contents_len = xattr_len;
> + }
> +
> + status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_contents,
> + xattr_contents_len, iint);
> switch (status) {
> case INTEGRITY_PASS:
> case INTEGRITY_PASS_IMMUTABLE:
> case INTEGRITY_UNKNOWN:
> break;
> case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */
> + /* It's fine not to have xattrs when using a modsig. */
> + if (xattr_value->type == IMA_MODSIG)
> + break;
> case INTEGRITY_NOLABEL: /* No security.evm xattr. */
> cause = "missing-HMAC";
> goto out;
> @@ -288,11 +324,16 @@ int ima_appraise_measurement(enum ima_hooks func,
> status = INTEGRITY_PASS;
> break;
> case EVM_IMA_XATTR_DIGSIG:
> + case IMA_MODSIG:
> set_bit(IMA_DIGSIG, &iint->atomic_flags);
> - rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> - (const char *)xattr_value, rc,
> - iint->ima_hash->digest,
> - iint->ima_hash->length);
> + if (xattr_value->type == EVM_IMA_XATTR_DIGSIG)
> + rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> + (const char *)xattr_value,
> + rc, iint->ima_hash->digest,
> + iint->ima_hash->length);
> + else
> + rc = ima_modsig_verify(INTEGRITY_KEYRING_IMA,
> + xattr_value);
> if (rc == -EOPNOTSUPP) {
> status = INTEGRITY_UNKNOWN;
> } else if (rc) {
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 5d122daf5c8a..1b11c10f09df 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -183,7 +183,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> struct evm_ima_xattr_data *xattr_value = NULL;
> int xattr_len = 0;
> bool violation_check;
> - enum hash_algo hash_algo;
> + enum hash_algo hash_algo = HASH_ALGO__LAST;
>
> if (!ima_policy_flag || !S_ISREG(inode->i_mode))
> return 0;
> @@ -277,11 +277,24 @@ static int process_measurement(struct file *file, const struct cred *cred,
>
> template_desc = ima_template_desc_current();
> if ((action & IMA_APPRAISE_SUBMASK) ||
> - strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
> + strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) {
> /* read 'security.ima' */
> xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
> + if (iint->flags & IMA_MODSIG_ALLOWED &&
> + (xattr_len <= 0 || !ima_xattr_sig_known_key(xattr_value,
> + xattr_len))) {
> + /*
> + * Even if we end up using a modsig, hash_algo should
> + * come from the xattr (or even the default hash algo).
> + */
> + hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
> + ima_read_modsig(func, buf, size, &xattr_value,
> + &xattr_len);
> + }
> + }
>
> - hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
> + if (hash_algo == HASH_ALGO__LAST)
> + hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
Previous versions needed to calculate the file hash based on the
modsig hash algorithm. With the introduction of the digest signature
template field ('d-sig'), the file digest field ('d-ng') is always
calculated based on either the xattr hash algorithm, if one exists, or
the IMA default hash algorithm.
Mimi
>
> rc = ima_collect_measurement(iint, file, buf, size, hash_algo);
> if (rc != 0 && rc != -EBADF && rc != -EINVAL)
> @@ -309,7 +322,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
> !(iint->flags & IMA_NEW_FILE))
> rc = -EACCES;
> mutex_unlock(&iint->mutex);
> - kfree(xattr_value);
> + ima_free_xattr_data(xattr_value);
> out:
> if (pathbuf)
> __putname(pathbuf);
>
On Fri, 2018-03-16 at 17:38 -0300, Thiago Jung Bauermann wrote:
> With the introduction of another IMA signature type (modsig), some places
> will need to check for both of them. It is cleaner to do that if there's a
> helper function to tell whether an xattr_value represents an IMA
> signature.
Initially the function name "is_ima_sig" is fine, since it reflects
the 'imasig' type. Having a more generic function name would be
better when adding 'modsig' support. As long as the function is
locally define, we can drop 'ima' from the name. Perhaps something
like has_signature or is_signed() would be preferable.
Mimi
>
> Suggested-by: Mimi Zohar <[email protected]>
> Signed-off-by: Thiago Jung Bauermann <[email protected]>
> ---
> security/integrity/ima/ima.h | 5 +++++
> security/integrity/ima/ima_appraise.c | 7 +++----
> security/integrity/ima/ima_template_lib.c | 2 +-
> 3 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 35fe91aa1fc9..4bafa6a97967 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -155,6 +155,11 @@ unsigned long ima_get_binary_runtime_size(void);
> int ima_init_template(void);
> void ima_init_template_list(void);
>
> +static inline bool is_ima_sig(const struct evm_ima_xattr_data *xattr_value)
> +{
> + return xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG;
> +}
> +
> /*
> * used to protect h_table and sha_table
> */
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index a6b2995b7d0b..01172eab297b 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -325,15 +325,14 @@ int ima_appraise_measurement(enum ima_hooks func,
> } else if (status != INTEGRITY_PASS) {
> /* Fix mode, but don't replace file signatures. */
> if ((ima_appraise & IMA_APPRAISE_FIX) &&
> - (!xattr_value ||
> - xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
> + !is_ima_sig(xattr_value)) {
> if (!ima_fix_xattr(dentry, iint))
> status = INTEGRITY_PASS;
> }
>
> /* Permit new files with file signatures, but without data. */
> if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
> - xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
> + is_ima_sig(xattr_value)) {
> status = INTEGRITY_PASS;
> }
>
> @@ -448,7 +447,7 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
> if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
> return -EINVAL;
> ima_reset_appraise_flags(d_backing_inode(dentry),
> - xvalue->type == EVM_IMA_XATTR_DIGSIG);
> + is_ima_sig(xvalue));
> result = 0;
> }
> return result;
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 5afaa53decc5..afb52a90e532 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -380,7 +380,7 @@ int ima_eventsig_init(struct ima_event_data *event_data,
> {
> struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
>
> - if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG))
> + if (!is_ima_sig(xattr_value))
> return 0;
>
> return ima_write_template_field_data(xattr_value, event_data->xattr_len,
>