Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1625619 > unrolled thread

[PATCH 0/6] Appended signatures support for IMA appraisal

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/6] Appended signatures support for IMA appraisal Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-18 22:20 +0200
    [PATCH 5/6] MODSIGN: Export module signature definitions. Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-18 22:20 +0200
      Re: [PATCH 5/6] MODSIGN: Export module signature definitions. Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-04-20 14:40 +0200
        Re: [PATCH 5/6] MODSIGN: Export module signature definitions. David Howells <dhowells@redhat.com> - 2017-04-20 16:40 +0200
          Re: [PATCH 5/6] MODSIGN: Export module signature definitions. Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-20 23:10 +0200
    [PATCH 2/6] ima: Tidy up constant strings Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-18 22:20 +0200
    [PATCH 3/6] ima: Simplify policy_func_show. Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-18 22:20 +0200
      Re: [PATCH 3/6] ima: Simplify policy_func_show. Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-04-20 14:20 +0200
        Re: [PATCH 3/6] ima: Simplify policy_func_show. Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-20 22:50 +0200
          Re: [PATCH 3/6] ima: Simplify policy_func_show. Mimi Zohar <zohar@linux.vnet.ibm.com> - 2017-04-21 16:00 +0200
            Re: [PATCH 3/6] ima: Simplify policy_func_show. Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-24 19:20 +0200
    [PATCH 1/6] integrity: Small code improvements Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-18 22:20 +0200
    [PATCH 4/6] ima: Log the same audit cause whenever a file has no signature Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-18 22:30 +0200
    [PATCH 6/6] ima: Support appended signatures for appraisal Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-18 22:30 +0200
      Re: [PATCH 6/6] ima: Support appended signatures for appraisal kbuild test robot <lkp@intel.com> - 2017-04-20 05:10 +0200
        Re: [PATCH 6/6] ima: Support appended signatures for appraisal Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com> - 2017-04-21 01:50 +0200

#1625619 — [PATCH 0/6] Appended signatures support for IMA appraisal

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 can boot kernels
with the signature appended to them as well as kernels where the signature
is stored in the IMA extended attribute.

The first four patches are cleanups and improvements that can be taken
independently from the others (and from each other as well). The last two
are the ones actually focused on this feature.

These patches apply on top of today's linux-security/next.

Thiago Jung Bauermann (6):
  integrity: Small code improvements
  ima: Tidy up constant strings
  ima: Simplify policy_func_show.
  ima: Log the same audit cause whenever a file has no signature
  MODSIGN: Export module signature definitions.
  ima: Support appended signatures for appraisal

 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 crypto/asymmetric_keys/pkcs7_parser.c    | 12 +++++
 crypto/asymmetric_keys/pkcs7_verify.c    | 13 +++++
 include/crypto/pkcs7.h                   |  3 ++
 include/linux/module_signature.h         | 45 ++++++++++++++++
 include/linux/verification.h             |  1 +
 init/Kconfig                             |  6 ++-
 kernel/Makefile                          |  2 +-
 kernel/module_signing.c                  | 74 +++++++++++----------------
 security/integrity/Kconfig               |  2 +-
 security/integrity/digsig_asymmetric.c   |  4 +-
 security/integrity/iint.c                |  2 +-
 security/integrity/ima/Kconfig           | 13 +++++
 security/integrity/ima/ima.h             |  8 +++
 security/integrity/ima/ima_appraise.c    | 86 ++++++++++++++++++++++++++++++-
 security/integrity/ima/ima_init.c        |  2 +-
 security/integrity/ima/ima_main.c        | 30 +++++++++--
 security/integrity/ima/ima_policy.c      | 88 ++++++++++++--------------------
 security/integrity/integrity.h           | 27 ++++++----
 19 files changed, 302 insertions(+), 117 deletions(-)
 create mode 100644 include/linux/module_signature.h

-- 
2.7.4

[toc] | [next] | [standalone]

#1625620 — [PATCH 5/6] MODSIGN: Export module signature definitions.

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_signature without having to depend on
CONFIG_MODULE_SIG.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 include/linux/module_signature.h | 45 ++++++++++++++++++++++++
 init/Kconfig                     |  6 +++-
 kernel/Makefile                  |  2 +-
 kernel/module_signing.c          | 74 +++++++++++++++++-----------------------
 4 files changed, 82 insertions(+), 45 deletions(-)

diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
new file mode 100644
index 000000000000..b04f16559b47
--- /dev/null
+++ b/include/linux/module_signature.h
@@ -0,0 +1,45 @@
+/* Module signature handling.
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _LINUX_MODULE_SIGNATURE_H
+#define _LINUX_MODULE_SIGNATURE_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.
+ *
+ * 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_signature(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 a92f27da4a27..891325e5aeff 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2024,7 +2024,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
@@ -2039,6 +2039,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 b302b4731d16..4451bbc70a08 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -56,7 +56,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_KEXEC_CORE) += kexec_core.o
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 937c844bee4a..421e3e668714 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,36 +11,38 @@
 
 #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.
- *
- * The constituents of the signature section are, in order:
+/**
+ * validate_module_signature - validate that the given signature is sane
  *
- *	- 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_signature(const struct module_signature *ms, size_t file_len)
+{
+	if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
+		return -EBADMSG;
+	else if (ms->id_type != PKEY_ID_PKCS7) {
+		pr_err("Module is not signed with expected PKCS#7 message\n");
+		return -ENOPKG;
+	} else 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 +51,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 +59,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_signature(&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);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1627411 — Re: [PATCH 5/6] MODSIGN: Export module signature definitions.

On Tue, 2017-04-18 at 17:17 -0300, Thiago Jung Bauermann wrote:
> 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_signature without having to depend on
> CONFIG_MODULE_SIG.

Basically we want to generalize the concept of an appended signature.
 Referring to it as a "module signature format" seems a bit confusing.

David, would you have a problem with changing the appended string from
"~Module signature appended~\n" to something more generic?  The
appended signature format could be used by anything calling
kernel_read_file() (eg. kexec kernel image/initramfs).

Mimi

> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>  include/linux/module_signature.h | 45 ++++++++++++++++++++++++
>  init/Kconfig                     |  6 +++-
>  kernel/Makefile                  |  2 +-
>  kernel/module_signing.c          | 74 +++++++++++++++++-----------------------
>  4 files changed, 82 insertions(+), 45 deletions(-)
> 
> diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
> new file mode 100644
> index 000000000000..b04f16559b47
> --- /dev/null
> +++ b/include/linux/module_signature.h
> @@ -0,0 +1,45 @@
> +/* Module signature handling.
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public Licence
> + * as published by the Free Software Foundation; either version
> + * 2 of the Licence, or (at your option) any later version.
> + */
> +
> +#ifndef _LINUX_MODULE_SIGNATURE_H
> +#define _LINUX_MODULE_SIGNATURE_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.
> + *
> + * 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_signature(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 a92f27da4a27..891325e5aeff 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2024,7 +2024,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
> @@ -2039,6 +2039,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 b302b4731d16..4451bbc70a08 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -56,7 +56,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_KEXEC_CORE) += kexec_core.o
> diff --git a/kernel/module_signing.c b/kernel/module_signing.c
> index 937c844bee4a..421e3e668714 100644
> --- a/kernel/module_signing.c
> +++ b/kernel/module_signing.c
> @@ -11,36 +11,38 @@
> 
>  #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.
> - *
> - * The constituents of the signature section are, in order:
> +/**
> + * validate_module_signature - validate that the given signature is sane
>   *
> - *	- 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_signature(const struct module_signature *ms, size_t file_len)
> +{
> +	if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
> +		return -EBADMSG;
> +	else if (ms->id_type != PKEY_ID_PKCS7) {
> +		pr_err("Module is not signed with expected PKCS#7 message\n");
> +		return -ENOPKG;
> +	} else 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 +51,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 +59,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_signature(&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);

[toc] | [prev] | [next] | [standalone]

#1627540 — Re: [PATCH 5/6] MODSIGN: Export module signature definitions.

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> On Tue, 2017-04-18 at 17:17 -0300, Thiago Jung Bauermann wrote:
> > 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_signature without having to depend on
> > CONFIG_MODULE_SIG.
> 
> Basically we want to generalize the concept of an appended signature.
>  Referring to it as a "module signature format" seems a bit confusing.
> 
> David, would you have a problem with changing the appended string from
> "~Module signature appended~\n" to something more generic?

Conceptually, no.  Is it possible that doing so could break someone's module
that they load on multiple versions of the kernel?  Say a module that only
exports things and doesn't use anything from the core or any other module.

Also, it needs to reasonably long and distinct enough to prevent a false
positive match.

David

[toc] | [prev] | [next] | [standalone]

#1627826 — Re: [PATCH 5/6] MODSIGN: Export module signature definitions.

Am Donnerstag, 20. April 2017, 15:37:37 BRT schrieb David Howells:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Tue, 2017-04-18 at 17:17 -0300, Thiago Jung Bauermann wrote:
> > > 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_signature without having to depend on
> > > CONFIG_MODULE_SIG.
> > 
> > Basically we want to generalize the concept of an appended signature.
> >  Referring to it as a "module signature format" seems a bit confusing.
> > 
> > David, would you have a problem with changing the appended string from
> > "~Module signature appended~\n" to something more generic?
> 
> Conceptually, no.  Is it possible that doing so could break someone's module
> that they load on multiple versions of the kernel?  Say a module that only
> exports things and doesn't use anything from the core or any other module.

I think that changing the appended string has limited value because very few 
people actually see them. It's just a marker. We could s/module_signature/
appended_signature/ in the code but keep the actual string unchanged. What do 
you think?

Alternatively, we could change the string but accept both the old and the new 
string for backwards compatibility.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

[toc] | [prev] | [next] | [standalone]

#1625622 — [PATCH 2/6] ima: Tidy up constant strings

Strictly speaking, boot_aggregate_name is a constant string, not a
modifiable pointer to a constant string.

Also, constify mask_tokens and func_tokens arrays.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_init.c   | 2 +-
 security/integrity/ima/ima_policy.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 2967d497a665..24c703366ba8 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -25,7 +25,7 @@
 #include "ima.h"
 
 /* name for boot aggregate entry */
-static const char *boot_aggregate_name = "boot_aggregate";
+static const char boot_aggregate_name[] = "boot_aggregate";
 int ima_used_chip;
 
 /* Add the boot aggregate to the IMA measurement list and extend
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index aed47b777a57..cfda5d7b17ec 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -889,7 +889,7 @@ enum {
 	mask_exec = 0, mask_write, mask_read, mask_append
 };
 
-static char *mask_tokens[] = {
+static const char *const mask_tokens[] = {
 	"MAY_EXEC",
 	"MAY_WRITE",
 	"MAY_READ",
@@ -903,7 +903,7 @@ enum {
 	func_policy
 };
 
-static char *func_tokens[] = {
+static const char *const func_tokens[] = {
 	"FILE_CHECK",
 	"MMAP_CHECK",
 	"BPRM_CHECK",
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1625623 — [PATCH 3/6] ima: Simplify policy_func_show.

If the func_tokens array uses the same indices as enum ima_hooks,
policy_func_show can be a lot simpler, and the func_* enum becomes
unnecessary.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_policy.c | 47 ++++++-------------------------------
 1 file changed, 7 insertions(+), 40 deletions(-)

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index cfda5d7b17ec..158eafef64e8 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -896,20 +896,14 @@ static const char *const mask_tokens[] = {
 	"MAY_APPEND"
 };
 
-enum {
-	func_file = 0, func_mmap, func_bprm,
-	func_module, func_firmware, func_post,
-	func_kexec_kernel, func_kexec_initramfs,
-	func_policy
-};
-
 static const char *const func_tokens[] = {
+	NULL,
 	"FILE_CHECK",
 	"MMAP_CHECK",
 	"BPRM_CHECK",
+	"POST_SETATTR",
 	"MODULE_CHECK",
 	"FIRMWARE_CHECK",
-	"POST_SETATTR",
 	"KEXEC_KERNEL_CHECK",
 	"KEXEC_INITRAMFS_CHECK",
 	"POLICY_CHECK"
@@ -949,48 +943,21 @@ void ima_policy_stop(struct seq_file *m, void *v)
 
 #define pt(token)	policy_tokens[token + Opt_err].pattern
 #define mt(token)	mask_tokens[token]
-#define ft(token)	func_tokens[token]
 
 /*
  * policy_func_show - display the ima_hooks policy rule
  */
 static void policy_func_show(struct seq_file *m, enum ima_hooks func)
 {
-	char tbuf[64] = {0,};
+	if (func > 0 && func < MAX_CHECK)
+		seq_printf(m, pt(Opt_func), func_tokens[func]);
+	else {
+		char tbuf[64] = {0,};
 
-	switch (func) {
-	case FILE_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_file));
-		break;
-	case MMAP_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_mmap));
-		break;
-	case BPRM_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_bprm));
-		break;
-	case MODULE_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_module));
-		break;
-	case FIRMWARE_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_firmware));
-		break;
-	case POST_SETATTR:
-		seq_printf(m, pt(Opt_func), ft(func_post));
-		break;
-	case KEXEC_KERNEL_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_kexec_kernel));
-		break;
-	case KEXEC_INITRAMFS_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_kexec_initramfs));
-		break;
-	case POLICY_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_policy));
-		break;
-	default:
 		snprintf(tbuf, sizeof(tbuf), "%d", func);
 		seq_printf(m, pt(Opt_func), tbuf);
-		break;
 	}
+
 	seq_puts(m, " ");
 }
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1627405 — Re: [PATCH 3/6] ima: Simplify policy_func_show.

On Tue, 2017-04-18 at 17:17 -0300, Thiago Jung Bauermann wrote:
> If the func_tokens array uses the same indices as enum ima_hooks,
> policy_func_show can be a lot simpler, and the func_* enum becomes
> unnecessary.

My main concern with separating the enumeration from the string
definition is that they might become out of sync.  Perhaps using
macros, similar to those used for kernel_read_file_id_str(), would be
better?

> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>  security/integrity/ima/ima_policy.c | 47 ++++++-------------------------------
>  1 file changed, 7 insertions(+), 40 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index cfda5d7b17ec..158eafef64e8 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -896,20 +896,14 @@ static const char *const mask_tokens[] = {
>  	"MAY_APPEND"
>  };
> 
> -enum {
> -	func_file = 0, func_mmap, func_bprm,
> -	func_module, func_firmware, func_post,
> -	func_kexec_kernel, func_kexec_initramfs,
> -	func_policy
> -};
> -

At least, add a comment here and near the ima_hooks enumeration to
prevent them from becoming out of sync.

Mimi

>  static const char *const func_tokens[] = {
> +	NULL,
>  	"FILE_CHECK",
>  	"MMAP_CHECK",
>  	"BPRM_CHECK",
> +	"POST_SETATTR",
>  	"MODULE_CHECK",
>  	"FIRMWARE_CHECK",
> -	"POST_SETATTR",
>  	"KEXEC_KERNEL_CHECK",
>  	"KEXEC_INITRAMFS_CHECK",
>  	"POLICY_CHECK"
> @@ -949,48 +943,21 @@ void ima_policy_stop(struct seq_file *m, void *v)
> 
>  #define pt(token)	policy_tokens[token + Opt_err].pattern
>  #define mt(token)	mask_tokens[token]
> -#define ft(token)	func_tokens[token]
> 
>  /*
>   * policy_func_show - display the ima_hooks policy rule
>   */
>  static void policy_func_show(struct seq_file *m, enum ima_hooks func)
>  {
> -	char tbuf[64] = {0,};
> +	if (func > 0 && func < MAX_CHECK)
> +		seq_printf(m, pt(Opt_func), func_tokens[func]);
> +	else {
> +		char tbuf[64] = {0,};
> 
> -	switch (func) {
> -	case FILE_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_file));
> -		break;
> -	case MMAP_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_mmap));
> -		break;
> -	case BPRM_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_bprm));
> -		break;
> -	case MODULE_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_module));
> -		break;
> -	case FIRMWARE_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_firmware));
> -		break;
> -	case POST_SETATTR:
> -		seq_printf(m, pt(Opt_func), ft(func_post));
> -		break;
> -	case KEXEC_KERNEL_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_kexec_kernel));
> -		break;
> -	case KEXEC_INITRAMFS_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_kexec_initramfs));
> -		break;
> -	case POLICY_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_policy));
> -		break;
> -	default:
>  		snprintf(tbuf, sizeof(tbuf), "%d", func);
>  		seq_printf(m, pt(Opt_func), tbuf);
> -		break;
>  	}
> +
>  	seq_puts(m, " ");
>  }
> 

[toc] | [prev] | [next] | [standalone]

#1627813 — Re: [PATCH 3/6] ima: Simplify policy_func_show.

Am Donnerstag, 20. April 2017, 08:13:23 BRT schrieb Mimi Zohar:
> On Tue, 2017-04-18 at 17:17 -0300, Thiago Jung Bauermann wrote:
> > If the func_tokens array uses the same indices as enum ima_hooks,
> > policy_func_show can be a lot simpler, and the func_* enum becomes
> > unnecessary.
> 
> My main concern with separating the enumeration from the string
> definition is that they might become out of sync.  Perhaps using
> macros, similar to those used for kernel_read_file_id_str(), would be
> better?

I agree that it would be better. Is the patch below what you had in mind?

I also noticed that policy_func_show can be even simpler if we stop using the 
printf format string from the policy_tokens table. What do you think?

-- 
Thiago Jung Bauermann
IBM Linux Technology Center


From 594628c94f5dd7c6d2624944a76b6a01f9668128 Mon Sep 17 00:00:00 2001
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Date: Mon, 10 Apr 2017 14:59:44 -0300
Subject: [PATCH 3/6] ima: Simplify policy_func_show.

If the func_tokens array uses the same indices as enum ima_hooks,
policy_func_show can be a lot simpler, and the func_* enum becomes
unnecessary.

Also, if we use the same macro trick used by kernel_read_file_id_str we can
use one hooks list for both the enum and the string array, making sure they
are always in sync (suggested by Mimi Zohar).

Finally, by using the printf pattern for the function token directly
instead of using the pt macro we can simplify policy_func_show even further
and avoid the need of having a temporary buffer. Since the only use of
Opt_func's printf pattern in policy_tokens was in policy_func_show, we
don't need it at all anymore so remove it.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h        | 25 +++++++++-------
 security/integrity/ima/ima_policy.c | 60 +++++--------------------------------
 2 files changed, 22 insertions(+), 63 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index b563fbd4d122..51ef805cf7f3 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -172,17 +172,22 @@ static inline unsigned long ima_hash_key(u8 *digest)
 	return hash_long(*digest, IMA_HASH_BITS);
 }
 
+#define __ima_hooks(hook)		\
+	hook(NONE)			\
+	hook(FILE_CHECK)		\
+	hook(MMAP_CHECK)		\
+	hook(BPRM_CHECK)		\
+	hook(POST_SETATTR)		\
+	hook(MODULE_CHECK)		\
+	hook(FIRMWARE_CHECK)		\
+	hook(KEXEC_KERNEL_CHECK)	\
+	hook(KEXEC_INITRAMFS_CHECK)	\
+	hook(POLICY_CHECK)		\
+	hook(MAX_CHECK)
+#define __ima_hook_enumify(ENUM)	ENUM,
+
 enum ima_hooks {
-	FILE_CHECK = 1,
-	MMAP_CHECK,
-	BPRM_CHECK,
-	POST_SETATTR,
-	MODULE_CHECK,
-	FIRMWARE_CHECK,
-	KEXEC_KERNEL_CHECK,
-	KEXEC_INITRAMFS_CHECK,
-	POLICY_CHECK,
-	MAX_CHECK
+	__ima_hooks(__ima_hook_enumify)
 };
 
 /* LIM API function definitions */
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index cfda5d7b17ec..39d43a5beb5a 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -503,7 +503,7 @@ static match_table_t policy_tokens = {
 	{Opt_subj_user, "subj_user=%s"},
 	{Opt_subj_role, "subj_role=%s"},
 	{Opt_subj_type, "subj_type=%s"},
-	{Opt_func, "func=%s"},
+	{Opt_func, NULL},
 	{Opt_mask, "mask=%s"},
 	{Opt_fsmagic, "fsmagic=%s"},
 	{Opt_fsuuid, "fsuuid=%s"},
@@ -896,23 +896,10 @@ static const char *const mask_tokens[] = {
 	"MAY_APPEND"
 };
 
-enum {
-	func_file = 0, func_mmap, func_bprm,
-	func_module, func_firmware, func_post,
-	func_kexec_kernel, func_kexec_initramfs,
-	func_policy
-};
+#define __ima_hook_stringify(str)	#str,
 
 static const char *const func_tokens[] = {
-	"FILE_CHECK",
-	"MMAP_CHECK",
-	"BPRM_CHECK",
-	"MODULE_CHECK",
-	"FIRMWARE_CHECK",
-	"POST_SETATTR",
-	"KEXEC_KERNEL_CHECK",
-	"KEXEC_INITRAMFS_CHECK",
-	"POLICY_CHECK"
+	__ima_hooks(__ima_hook_stringify)
 };
 
 void *ima_policy_start(struct seq_file *m, loff_t *pos)
@@ -949,49 +936,16 @@ void ima_policy_stop(struct seq_file *m, void *v)
 
 #define pt(token)	policy_tokens[token + Opt_err].pattern
 #define mt(token)	mask_tokens[token]
-#define ft(token)	func_tokens[token]
 
 /*
  * policy_func_show - display the ima_hooks policy rule
  */
 static void policy_func_show(struct seq_file *m, enum ima_hooks func)
 {
-	char tbuf[64] = {0,};
-
-	switch (func) {
-	case FILE_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_file));
-		break;
-	case MMAP_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_mmap));
-		break;
-	case BPRM_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_bprm));
-		break;
-	case MODULE_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_module));
-		break;
-	case FIRMWARE_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_firmware));
-		break;
-	case POST_SETATTR:
-		seq_printf(m, pt(Opt_func), ft(func_post));
-		break;
-	case KEXEC_KERNEL_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_kexec_kernel));
-		break;
-	case KEXEC_INITRAMFS_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_kexec_initramfs));
-		break;
-	case POLICY_CHECK:
-		seq_printf(m, pt(Opt_func), ft(func_policy));
-		break;
-	default:
-		snprintf(tbuf, sizeof(tbuf), "%d", func);
-		seq_printf(m, pt(Opt_func), tbuf);
-		break;
-	}
-	seq_puts(m, " ");
+	if (func > 0 && func < MAX_CHECK)
+		seq_printf(m, "func=%s ", func_tokens[func]);
+	else
+		seq_printf(m, "func=%d ", func);
 }
 
 int ima_policy_show(struct seq_file *m, void *v)
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1628273 — Re: [PATCH 3/6] ima: Simplify policy_func_show.

On Thu, 2017-04-20 at 17:40 -0300, Thiago Jung Bauermann wrote:
> Am Donnerstag, 20. April 2017, 08:13:23 BRT schrieb Mimi Zohar:
> > On Tue, 2017-04-18 at 17:17 -0300, Thiago Jung Bauermann wrote:
> > > If the func_tokens array uses the same indices as enum ima_hooks,
> > > policy_func_show can be a lot simpler, and the func_* enum becomes
> > > unnecessary.
> > 
> > My main concern with separating the enumeration from the string
> > definition is that they might become out of sync.  Perhaps using
> > macros, similar to those used for kernel_read_file_id_str(), would be
> > better?
> 
> I agree that it would be better. Is the patch below what you had in mind?

Yes, I haven't tested it yet, but it looks right.
> 
> I also noticed that policy_func_show can be even simpler if we stop using the 
> printf format string from the policy_tokens table. What do you think?
> 
> -- 
> Thiago Jung Bauermann
> IBM Linux Technology Center
> 
> 
> From 594628c94f5dd7c6d2624944a76b6a01f9668128 Mon Sep 17 00:00:00 2001
> From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> Date: Mon, 10 Apr 2017 14:59:44 -0300
> Subject: [PATCH 3/6] ima: Simplify policy_func_show.
> 
> If the func_tokens array uses the same indices as enum ima_hooks,
> policy_func_show can be a lot simpler, and the func_* enum becomes
> unnecessary.
> 
> Also, if we use the same macro trick used by kernel_read_file_id_str we can
> use one hooks list for both the enum and the string array, making sure they
> are always in sync (suggested by Mimi Zohar).

> Finally, by using the printf pattern for the function token directly
> instead of using the pt macro we can simplify policy_func_show even further
> and avoid the need of having a temporary buffer. Since the only use of
> Opt_func's printf pattern in policy_tokens was in policy_func_show, we
> don't need it at all anymore so remove it.
> 
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> ---
>  security/integrity/ima/ima.h        | 25 +++++++++-------
>  security/integrity/ima/ima_policy.c | 60 +++++--------------------------------
>  2 files changed, 22 insertions(+), 63 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index b563fbd4d122..51ef805cf7f3 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -172,17 +172,22 @@ static inline unsigned long ima_hash_key(u8 *digest)
>  	return hash_long(*digest, IMA_HASH_BITS);
>  }
> 
> +#define __ima_hooks(hook)		\
> +	hook(NONE)			\
> +	hook(FILE_CHECK)		\
> +	hook(MMAP_CHECK)		\
> +	hook(BPRM_CHECK)		\
> +	hook(POST_SETATTR)		\
> +	hook(MODULE_CHECK)		\
> +	hook(FIRMWARE_CHECK)		\
> +	hook(KEXEC_KERNEL_CHECK)	\
> +	hook(KEXEC_INITRAMFS_CHECK)	\
> +	hook(POLICY_CHECK)		\
> +	hook(MAX_CHECK)
> +#define __ima_hook_enumify(ENUM)	ENUM,
> +
>  enum ima_hooks {
> -	FILE_CHECK = 1,
> -	MMAP_CHECK,
> -	BPRM_CHECK,
> -	POST_SETATTR,
> -	MODULE_CHECK,
> -	FIRMWARE_CHECK,
> -	KEXEC_KERNEL_CHECK,
> -	KEXEC_INITRAMFS_CHECK,
> -	POLICY_CHECK,
> -	MAX_CHECK
> +	__ima_hooks(__ima_hook_enumify)
>  };
> 
>  /* LIM API function definitions */
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index cfda5d7b17ec..39d43a5beb5a 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -503,7 +503,7 @@ static match_table_t policy_tokens = {
>  	{Opt_subj_user, "subj_user=%s"},
>  	{Opt_subj_role, "subj_role=%s"},
>  	{Opt_subj_type, "subj_type=%s"},
> -	{Opt_func, "func=%s"},
> +	{Opt_func, NULL},
>  	{Opt_mask, "mask=%s"},
>  	{Opt_fsmagic, "fsmagic=%s"},
>  	{Opt_fsuuid, "fsuuid=%s"},
> @@ -896,23 +896,10 @@ static const char *const mask_tokens[] = {
>  	"MAY_APPEND"
>  };
> 
> -enum {
> -	func_file = 0, func_mmap, func_bprm,
> -	func_module, func_firmware, func_post,
> -	func_kexec_kernel, func_kexec_initramfs,
> -	func_policy
> -};
> +#define __ima_hook_stringify(str)	#str,
> 
>  static const char *const func_tokens[] = {
> -	"FILE_CHECK",
> -	"MMAP_CHECK",
> -	"BPRM_CHECK",
> -	"MODULE_CHECK",
> -	"FIRMWARE_CHECK",
> -	"POST_SETATTR",
> -	"KEXEC_KERNEL_CHECK",
> -	"KEXEC_INITRAMFS_CHECK",
> -	"POLICY_CHECK"
> +	__ima_hooks(__ima_hook_stringify)
>  };
> 
>  void *ima_policy_start(struct seq_file *m, loff_t *pos)
> @@ -949,49 +936,16 @@ void ima_policy_stop(struct seq_file *m, void *v)
> 
>  #define pt(token)	policy_tokens[token + Opt_err].pattern
>  #define mt(token)	mask_tokens[token]
> -#define ft(token)	func_tokens[token]
> 
>  /*
>   * policy_func_show - display the ima_hooks policy rule
>   */
>  static void policy_func_show(struct seq_file *m, enum ima_hooks func)
>  {
> -	char tbuf[64] = {0,};
> -
> -	switch (func) {
> -	case FILE_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_file));
> -		break;
> -	case MMAP_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_mmap));
> -		break;
> -	case BPRM_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_bprm));
> -		break;
> -	case MODULE_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_module));
> -		break;
> -	case FIRMWARE_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_firmware));
> -		break;
> -	case POST_SETATTR:
> -		seq_printf(m, pt(Opt_func), ft(func_post));
> -		break;
> -	case KEXEC_KERNEL_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_kexec_kernel));
> -		break;
> -	case KEXEC_INITRAMFS_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_kexec_initramfs));
> -		break;
> -	case POLICY_CHECK:
> -		seq_printf(m, pt(Opt_func), ft(func_policy));
> -		break;
> -	default:
> -		snprintf(tbuf, sizeof(tbuf), "%d", func);
> -		seq_printf(m, pt(Opt_func), tbuf);
> -		break;
> -	}
> -	seq_puts(m, " ");
> +	if (func > 0 && func < MAX_CHECK)
> +		seq_printf(m, "func=%s ", func_tokens[func]);
> +	else
> +		seq_printf(m, "func=%d ", func);

The only time this can happen is when __kernel_read_file_id() is
updated without updating the read_idmap[].  Perhaps we can display the
number and the appropriate __kernel_read_file_id string.

Mimi

>  }
> 
>  int ima_policy_show(struct seq_file *m, void *v)

[toc] | [prev] | [next] | [standalone]

#1629804 — Re: [PATCH 3/6] ima: Simplify policy_func_show.

Am Freitag, 21. April 2017, 09:57:56 BRT schrieb Mimi Zohar:
> On Thu, 2017-04-20 at 17:40 -0300, Thiago Jung Bauermann wrote:
> > @@ -949,49 +936,16 @@ void ima_policy_stop(struct seq_file *m, void *v)
> > 
> >  #define pt(token)	policy_tokens[token + Opt_err].pattern
> >  #define mt(token)	mask_tokens[token]
> > 
> > -#define ft(token)	func_tokens[token]
> > 
> >  /*
> >  
> >   * policy_func_show - display the ima_hooks policy rule
> >   */
> >  
> >  static void policy_func_show(struct seq_file *m, enum ima_hooks func)
> >  {
> > 
> > -	char tbuf[64] = {0,};
> > -
> > -	switch (func) {
> > -	case FILE_CHECK:
> > -		seq_printf(m, pt(Opt_func), ft(func_file));
> > -		break;
> > -	case MMAP_CHECK:
> > -		seq_printf(m, pt(Opt_func), ft(func_mmap));
> > -		break;
> > -	case BPRM_CHECK:
> > -		seq_printf(m, pt(Opt_func), ft(func_bprm));
> > -		break;
> > -	case MODULE_CHECK:
> > -		seq_printf(m, pt(Opt_func), ft(func_module));
> > -		break;
> > -	case FIRMWARE_CHECK:
> > -		seq_printf(m, pt(Opt_func), ft(func_firmware));
> > -		break;
> > -	case POST_SETATTR:
> > -		seq_printf(m, pt(Opt_func), ft(func_post));
> > -		break;
> > -	case KEXEC_KERNEL_CHECK:
> > -		seq_printf(m, pt(Opt_func), ft(func_kexec_kernel));
> > -		break;
> > -	case KEXEC_INITRAMFS_CHECK:
> > -		seq_printf(m, pt(Opt_func), ft(func_kexec_initramfs));
> > -		break;
> > -	case POLICY_CHECK:
> > -		seq_printf(m, pt(Opt_func), ft(func_policy));
> > -		break;
> > -	default:
> > -		snprintf(tbuf, sizeof(tbuf), "%d", func);
> > -		seq_printf(m, pt(Opt_func), tbuf);
> > -		break;
> > -	}
> > -	seq_puts(m, " ");
> > +	if (func > 0 && func < MAX_CHECK)
> > +		seq_printf(m, "func=%s ", func_tokens[func]);
> > +	else
> > +		seq_printf(m, "func=%d ", func);
> 
> The only time this can happen is when __kernel_read_file_id() is
> updated without updating the read_idmap[].  Perhaps we can display the
> number and the appropriate __kernel_read_file_id string.

From what I understood of the code func comes from ima_parse_rule, so that 
condition would only happen if ima_parse_rule got out of sync with 
func_tokens. Since that code only initializes func with constants from enum 
ima_hooks and this patch makes ima_hooks automatically sync with func_tokens, 
the else branch is more like a "can't happen" safety net.

read_idmap is only used in ima_post_read_file, and I couldn't see a relation 
between that code path and the one for ima_policy_show.

-- 
Thiago Jung Bauermann
IBM Linux Technology Center

[toc] | [prev] | [next] | [standalone]

#1625628 — [PATCH 1/6] integrity: Small code improvements

The keyid and sig_size members of struct signature_v2_hdr are in BE format,
so use a type that makes this assumption explicit. Also, use beXX_to_cpu
instead of __beXX_to_cpu to read them.

Change integrity_kernel_read to take a void * buffer instead of char *
buffer, so that callers don't have to use a cast if they provide a buffer
that isn't a char *.

Also, add missing fall through comment in ima_appraise.c.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 security/integrity/digsig_asymmetric.c | 4 ++--
 security/integrity/iint.c              | 2 +-
 security/integrity/ima/ima_appraise.c  | 1 +
 security/integrity/integrity.h         | 7 ++++---
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
index 80052ed8d467..ab6a029062a1 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -92,13 +92,13 @@ int asymmetric_verify(struct key *keyring, const char *sig,
 
 	siglen -= sizeof(*hdr);
 
-	if (siglen != __be16_to_cpu(hdr->sig_size))
+	if (siglen != be16_to_cpu(hdr->sig_size))
 		return -EBADMSG;
 
 	if (hdr->hash_algo >= HASH_ALGO__LAST)
 		return -ENOPKG;
 
-	key = request_asymmetric_key(keyring, __be32_to_cpu(hdr->keyid));
+	key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
 	if (IS_ERR(key))
 		return PTR_ERR(key);
 
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index c710d22042f9..6fc888ca468e 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -182,7 +182,7 @@ security_initcall(integrity_iintcache_init);
  *
  */
 int integrity_kernel_read(struct file *file, loff_t offset,
-			  char *addr, unsigned long count)
+			  void *addr, unsigned long count)
 {
 	mm_segment_t old_fs;
 	char __user *buf = (char __user *)addr;
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 1fd9539a969d..427a896bc806 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -227,6 +227,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 	case IMA_XATTR_DIGEST_NG:
 		/* first byte contains algorithm id */
 		hash_start = 1;
+		/* fall through */
 	case IMA_XATTR_DIGEST:
 		if (iint->flags & IMA_DIGSIG_REQUIRED) {
 			cause = "IMA-signature-required";
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 24520b4ef3b0..a53e7e4ab06c 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -92,8 +92,8 @@ struct signature_v2_hdr {
 	uint8_t type;		/* xattr type */
 	uint8_t version;	/* signature format version */
 	uint8_t	hash_algo;	/* Digest algorithm [enum hash_algo] */
-	uint32_t keyid;		/* IMA key identifier - not X509/PGP specific */
-	uint16_t sig_size;	/* signature size */
+	__be32 keyid;		/* IMA key identifier - not X509/PGP specific */
+	__be16 sig_size;	/* signature size */
 	uint8_t sig[0];		/* signature payload */
 } __packed;
 
@@ -118,7 +118,8 @@ struct integrity_iint_cache {
 struct integrity_iint_cache *integrity_iint_find(struct inode *inode);
 
 int integrity_kernel_read(struct file *file, loff_t offset,
-			  char *addr, unsigned long count);
+			  void *addr, unsigned long count);
+
 int __init integrity_read_file(const char *path, char **data);
 
 #define INTEGRITY_KEYRING_EVM		0
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1625632 — [PATCH 4/6] ima: Log the same audit cause whenever a file has no signature

If the file doesn't have an xattr, ima_appraise_measurement sets cause to
"missing-hash" while if there's an xattr but it's a digest instead of a
signature it sets cause to "IMA-signature-required".

Fix it by setting cause to "IMA-signature-required" in both cases.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_appraise.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 427a896bc806..8c8030a29117 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -205,7 +205,8 @@ int ima_appraise_measurement(enum ima_hooks func,
 		if (rc && rc != -ENODATA)
 			goto out;
 
-		cause = "missing-hash";
+		cause = iint->flags & IMA_DIGSIG_REQUIRED ?
+				"IMA-signature-required" : "missing-hash";
 		status = INTEGRITY_NOLABEL;
 		if (opened & FILE_CREATED) {
 			iint->flags |= IMA_NEW_FILE;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1625639 — [PATCH 6/6] ima: Support appended signatures for appraisal

This patch introduces the appended_imasig 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=appended_imasig
appraise func=KEXEC_KERNEL_CHECK appraise_type=appended_imasig|imasig

In the second form, IMA will accept either an appended signature or a
signature stored in the extended attribute. In that case, it will first
check whether there is an appended signature, and if not it will read it
from the extended attribute.

The format of the appended signature is the same used for signed kernel
modules. This means that the file can be signed with the scripts/sign-file
tool, with a command line such as this:

$ sign-file sha256 privkey_ima.pem x509_ima.der vmlinux

This code only works for files that are hashed from a memory buffer, not
for files that are read from disk at the time of hash calculation. In other
words, only hooks that use kernel_read_file can support appended
signatures.

The change in CONFIG_INTEGRITY_SIGNATURE to select CONFIG_KEYS instead of
depending on it is to avoid a dependency recursion in
CONFIG_IMA_APPRAISE_APPENDED_SIG, because CONFIG_MODULE_SIG_FORMAT selects
CONFIG_KEYS and Kconfig complains that CONFIG_INTEGRITY_SIGNATURE depends
on it.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 crypto/asymmetric_keys/pkcs7_parser.c    | 12 +++++
 crypto/asymmetric_keys/pkcs7_verify.c    | 13 +++++
 include/crypto/pkcs7.h                   |  3 ++
 include/linux/verification.h             |  1 +
 security/integrity/Kconfig               |  2 +-
 security/integrity/ima/Kconfig           | 13 +++++
 security/integrity/ima/ima.h             |  8 +++
 security/integrity/ima/ima_appraise.c    | 84 +++++++++++++++++++++++++++++++-
 security/integrity/ima/ima_main.c        | 30 ++++++++++--
 security/integrity/ima/ima_policy.c      | 43 ++++++++++------
 security/integrity/integrity.h           | 20 +++++---
 12 files changed, 204 insertions(+), 26 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index e4b0ed386bc8..0d58ecfba2ea 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -28,6 +28,7 @@ const char *const key_being_used_for[NR__KEY_BEING_USED_FOR] = {
 	[VERIFYING_KEXEC_PE_SIGNATURE]		= "kexec PE sig",
 	[VERIFYING_KEY_SIGNATURE]		= "key sig",
 	[VERIFYING_KEY_SELF_SIGNATURE]		= "key self sig",
+	[VERIFYING_KEXEC_CMS_SIGNATURE]		= "kexec CMS sig",
 	[VERIFYING_UNSPECIFIED_SIGNATURE]	= "unspec sig",
 };
 EXPORT_SYMBOL_GPL(key_being_used_for);
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index af4cd8649117..e41beda297a8 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -673,3 +673,15 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
 		return -ENOMEM;
 	return 0;
 }
+
+/**
+ * pkcs7_get_message_sig - get signature in @pkcs7
+ *
+ * This function doesn't meaningfully support messages with more than one
+ * signature. It will always return the first signature.
+ */
+const struct public_key_signature *pkcs7_get_message_sig(
+					const struct pkcs7_message *pkcs7)
+{
+	return pkcs7->signed_infos ? pkcs7->signed_infos->sig : NULL;
+}
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 2d93d9eccb4d..eee78074580a 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -420,6 +420,19 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
 			return -EKEYREJECTED;
 		}
 		break;
+	case VERIFYING_KEXEC_CMS_SIGNATURE:
+		/* Shipping certificates in the CMS message is not allowed. */
+		if (pkcs7->certs) {
+			pr_warn("Signature isn't allowed to contain certificates.\n");
+			return -EBADMSG;
+		}
+
+		/* Shipping CRLs in the CMS message is not allowed. */
+		if (pkcs7->crl) {
+			pr_warn("Signature isn't allowed to contain CRLs.\n");
+			return -EBADMSG;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 583f199400a3..a05a0d7214e6 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -29,6 +29,9 @@ 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..b7298d804440 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -22,6 +22,7 @@ enum key_being_used_for {
 	VERIFYING_KEY_SIGNATURE,
 	VERIFYING_KEY_SELF_SIGNATURE,
 	VERIFYING_UNSPECIFIED_SIGNATURE,
+	VERIFYING_KEXEC_CMS_SIGNATURE,
 	NR__KEY_BEING_USED_FOR
 };
 extern const char *const key_being_used_for[NR__KEY_BEING_USED_FOR];
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
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 370eb2f4dd37..13662043bf90 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -155,6 +155,19 @@ config IMA_APPRAISE
 	  <http://linux-ima.sourceforge.net>
 	  If unsure, say N.
 
+config IMA_APPRAISE_APPENDED_SIG
+	bool "Support appended 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
+	   appended signature is the same used for signed kernel modules.
+	   The appended_imasig keyword can be used in the IMA policy for this
+	   purpose.
+
 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/ima.h b/security/integrity/ima/ima.h
index b563fbd4d122..63ce5a1179c3 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -185,6 +185,8 @@ enum ima_hooks {
 	MAX_CHECK
 };
 
+extern const char *const func_tokens[];
+
 /* LIM API function definitions */
 int ima_get_action(struct inode *inode, int mask,
 		   enum ima_hooks func, int *pcr);
@@ -243,6 +245,12 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
 int ima_read_xattr(struct dentry *dentry,
 		   struct evm_ima_xattr_data **xattr_value);
 
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+void ima_read_appended_sig(const void *buf, loff_t *buf_len,
+			  struct evm_ima_xattr_data **xattr_value,
+			  int *xattr_len);
+#endif
+
 #else
 static inline int ima_appraise_measurement(enum ima_hooks func,
 					   struct integrity_iint_cache *iint,
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 8c8030a29117..64a42f16c4ee 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -9,12 +9,15 @@
  * the Free Software Foundation, version 2 of the License.
  */
 #include <linux/module.h>
+#include <linux/module_signature.h>
 #include <linux/file.h>
 #include <linux/fs.h>
 #include <linux/xattr.h>
 #include <linux/magic.h>
 #include <linux/ima.h>
 #include <linux/evm.h>
+#include <keys/asymmetric-type.h>
+#include <crypto/pkcs7.h>
 
 #include "ima.h"
 
@@ -177,6 +180,85 @@ int ima_read_xattr(struct dentry *dentry,
 	return ret;
 }
 
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+void ima_read_appended_sig(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 public_key_signature *pks;
+	const struct module_signature *sig;
+	struct signature_v2_hdr *hdr;
+	struct pkcs7_message *pkcs7;
+	int i, hdr_len;
+	loff_t file_len = *buf_len;
+	size_t sig_len;
+	const void *p;
+
+	if (file_len <= marker_len + sizeof(*sig))
+		return;
+
+	p = buf + file_len - marker_len;
+	if (memcmp(p, MODULE_SIG_STRING, marker_len))
+		return;
+
+	file_len -= marker_len;
+	sig = (const struct module_signature *) (p - sizeof(*sig));
+
+	if (validate_module_signature(sig, file_len))
+		return;
+
+	sig_len = be32_to_cpu(sig->sig_len);
+	file_len -= sig_len + sizeof(*sig);
+
+	pkcs7 = pkcs7_parse_message(buf + file_len, sig_len);
+	if (IS_ERR(pkcs7))
+		return;
+
+	if (pkcs7_verify(pkcs7, VERIFYING_KEXEC_CMS_SIGNATURE))
+		goto out;
+
+	pks = pkcs7_get_message_sig(pkcs7);
+	if (!pks)
+		goto out;
+
+	/* IMA only supports RSA keys. */
+	if (strcmp(pks->pkey_algo, "rsa"))
+		goto out;
+
+	if (!pks->auth_ids[0])
+		goto out;
+
+	for (i = 0; i < HASH_ALGO__LAST; i++)
+		if (!strcmp(hash_algo_name[i], pks->hash_algo))
+			break;
+
+	if (i == HASH_ALGO__LAST)
+		goto out;
+
+	hdr_len = sizeof(*hdr) + pks->s_size;
+	hdr = kmalloc(hdr_len, GFP_KERNEL);
+	if (!hdr)
+		goto out;
+
+	hdr->type = EVM_IMA_XATTR_DIGSIG;
+	hdr->version = 2;
+	hdr->hash_algo = i;
+	memcpy(hdr->sig, pks->s, pks->s_size);
+	hdr->sig_size = cpu_to_be16(pks->s_size);
+
+	p = pks->auth_ids[0]->data + pks->auth_ids[0]->len - sizeof(hdr->keyid);
+	memcpy(&hdr->keyid, p, sizeof(hdr->keyid));
+
+	*xattr_value = (typeof(*xattr_value)) hdr;
+	*xattr_len = hdr_len;
+	*buf_len = file_len;
+
+ out:
+	pkcs7_free_message(pkcs7);
+}
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
+
 /*
  * ima_appraise_measurement - appraise file measurement
  *
@@ -205,7 +287,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 		if (rc && rc != -ENODATA)
 			goto out;
 
-		cause = iint->flags & IMA_DIGSIG_REQUIRED ?
+		cause = iint->flags & IMA_DIGSIG_REQUIRED_MASK ?
 				"IMA-signature-required" : "missing-hash";
 		status = INTEGRITY_NOLABEL;
 		if (opened & FILE_CREATED) {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2aebb7984437..994ee420b2ec 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -16,6 +16,9 @@
  *	implements the IMA hooks: ima_bprm_check, ima_file_mmap,
  *	and ima_file_check.
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/file.h>
 #include <linux/binfmts.h>
@@ -228,9 +231,30 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 
 	template_desc = ima_template_desc_current();
 	if ((action & IMA_APPRAISE_SUBMASK) ||
-		    strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
-		/* read 'security.ima' */
-		xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
+		    strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) {
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+		unsigned long digsig_req;
+
+		if (iint->flags & IMA_APPENDED_DIGSIG_REQUIRED) {
+			if (!buf || !size)
+				pr_err("%s doesn't support appended_imasig\n",
+				       func_tokens[func]);
+			else
+				ima_read_appended_sig(buf, &size, &xattr_value,
+						      &xattr_len);
+		}
+
+		/*
+		 * Don't try to read the xattr if we require an appended
+		 * signature but failed to get one.
+		 */
+		digsig_req = iint->flags & IMA_DIGSIG_REQUIRED_MASK;
+		if (!xattr_len && digsig_req != IMA_APPENDED_DIGSIG_REQUIRED)
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
+			/* read 'security.ima' */
+			xattr_len = ima_read_xattr(file_dentry(file),
+						   &xattr_value);
+	}
 
 	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
 
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 158eafef64e8..96601b5ba411 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -777,8 +777,15 @@ 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)
+			if (!strcmp(args[0].from, "imasig"))
 				entry->flags |= IMA_DIGSIG_REQUIRED;
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+			else if (!strcmp(args[0].from, "appended_imasig"))
+				entry->flags |= IMA_APPENDED_DIGSIG_REQUIRED;
+			else if (!strcmp(args[0].from, "appended_imasig|imasig"))
+				entry->flags |= IMA_DIGSIG_REQUIRED
+						| IMA_APPENDED_DIGSIG_REQUIRED;
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
 			else
 				result = -EINVAL;
 			break;
@@ -884,19 +891,7 @@ void ima_delete_rules(void)
 	}
 }
 
-#ifdef	CONFIG_IMA_READ_POLICY
-enum {
-	mask_exec = 0, mask_write, mask_read, mask_append
-};
-
-static const char *const mask_tokens[] = {
-	"MAY_EXEC",
-	"MAY_WRITE",
-	"MAY_READ",
-	"MAY_APPEND"
-};
-
-static const char *const func_tokens[] = {
+const char *const func_tokens[] = {
 	NULL,
 	"FILE_CHECK",
 	"MMAP_CHECK",
@@ -909,6 +904,18 @@ static const char *const func_tokens[] = {
 	"POLICY_CHECK"
 };
 
+#ifdef	CONFIG_IMA_READ_POLICY
+enum {
+	mask_exec = 0, mask_write, mask_read, mask_append
+};
+
+static const char *const mask_tokens[] = {
+	"MAY_EXEC",
+	"MAY_WRITE",
+	"MAY_READ",
+	"MAY_APPEND"
+};
+
 void *ima_policy_start(struct seq_file *m, loff_t *pos)
 {
 	loff_t l = *pos;
@@ -1062,7 +1069,13 @@ int ima_policy_show(struct seq_file *m, void *v)
 			}
 		}
 	}
-	if (entry->flags & IMA_DIGSIG_REQUIRED)
+	if ((entry->flags & IMA_DIGSIG_REQUIRED_MASK) == IMA_DIGSIG_REQUIRED_MASK)
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+		seq_puts(m, "appraise_type=appended_imasig|imasig ");
+	else if (entry->flags & IMA_APPENDED_DIGSIG_REQUIRED)
+		seq_puts(m, "appraise_type=appended_imasig ");
+	else if (entry->flags & IMA_DIGSIG_REQUIRED)
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
 		seq_puts(m, "appraise_type=imasig ");
 	if (entry->flags & IMA_PERMIT_DIRECTIO)
 		seq_puts(m, "permit_directio ");
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index a53e7e4ab06c..de2a666740c1 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -27,12 +27,20 @@
 #define IMA_AUDITED		0x00000080
 
 /* iint cache flags */
-#define IMA_ACTION_FLAGS	0xff000000
-#define IMA_ACTION_RULE_FLAGS	0x06000000
-#define IMA_DIGSIG		0x01000000
-#define IMA_DIGSIG_REQUIRED	0x02000000
-#define IMA_PERMIT_DIRECTIO	0x04000000
-#define IMA_NEW_FILE		0x08000000
+#define IMA_ACTION_FLAGS		0xff000000
+#define IMA_ACTION_RULE_FLAGS		0x16000000
+#define IMA_DIGSIG			0x01000000
+#define IMA_DIGSIG_REQUIRED		0x02000000
+#define IMA_PERMIT_DIRECTIO		0x04000000
+#define IMA_NEW_FILE			0x08000000
+#define IMA_APPENDED_DIGSIG_REQUIRED	0x10000000
+
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+#define IMA_DIGSIG_REQUIRED_MASK	(IMA_DIGSIG_REQUIRED | \
+					 IMA_APPENDED_DIGSIG_REQUIRED)
+#else
+#define IMA_DIGSIG_REQUIRED_MASK	IMA_DIGSIG_REQUIRED
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_APPRAISE_SUBMASK)
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1626947 — Re: [PATCH 6/6] ima: Support appended signatures for appraisal

[Multipart message — attachments visible in raw view] — view raw

Hi Thiago,

[auto build test ERROR on security/next]
[also build test ERROR on v4.11-rc7 next-20170419]
[cannot apply to integrity/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Thiago-Jung-Bauermann/Appended-signatures-support-for-IMA-appraisal/20170419-053422
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next
config: i386-randconfig-h0-04201028 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   In file included from security/integrity/ima/ima_appraise.c:19:0:
   include/keys/asymmetric-type.h: In function 'asymmetric_key_ids':
>> include/keys/asymmetric-type.h:76:12: error: dereferencing pointer to incomplete type 'const struct key'
     return key->payload.data[asym_key_ids];
               ^~

vim +76 include/keys/asymmetric-type.h

7901c1a8 David Howells 2014-09-16  70  							    size_t len_1,
7901c1a8 David Howells 2014-09-16  71  							    const void *val_2,
7901c1a8 David Howells 2014-09-16  72  							    size_t len_2);
146aa8b1 David Howells 2015-10-21  73  static inline
146aa8b1 David Howells 2015-10-21  74  const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key)
146aa8b1 David Howells 2015-10-21  75  {
146aa8b1 David Howells 2015-10-21 @76  	return key->payload.data[asym_key_ids];
146aa8b1 David Howells 2015-10-21  77  }
7901c1a8 David Howells 2014-09-16  78  
9eb02989 David Howells 2016-04-06  79  extern struct key *find_asymmetric_key(struct key *keyring,

:::::: The code at line 76 was first introduced by commit
:::::: 146aa8b1453bd8f1ff2304ffb71b4ee0eb9acdcc KEYS: Merge the type-specific data with the payload data

:::::: TO: David Howells <dhowells@redhat.com>
:::::: CC: David Howells <dhowells@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1627872 — Re: [PATCH 6/6] ima: Support appended signatures for appraisal

Am Donnerstag, 20. April 2017, 11:04:40 BRT schrieb kbuild test robot:
>    In file included from security/integrity/ima/ima_appraise.c:19:0:
> 
>    include/keys/asymmetric-type.h: In function 'asymmetric_key_ids':
> >> include/keys/asymmetric-type.h:76:12: error: dereferencing pointer to
> >> incomplete type 'const struct key'
>      return key->payload.data[asym_key_ids];
>                ^~

This happens with CONFIG_IMA_APPRAISE=y and CONFIG_KEYS=n.
Fixed by only including the new header files in ima_appraise.c if 
CONFIG_IMA_APPRAISE_APPENDED_SIG=y

-- 
Thiago Jung Bauermann
IBM Linux Technology Center


From e2d3143b13d6601f6a1f4936dfb569a04a1ce0cc Mon Sep 17 00:00:00 2001
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Date: Mon, 20 Mar 2017 11:08:21 -0300
Subject: [PATCH 6/6] ima: Support appended signatures for appraisal

This patch introduces the appended_imasig 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=appended_imasig
appraise func=KEXEC_KERNEL_CHECK appraise_type=appended_imasig|imasig

In the second form, IMA will accept either an appended signature or a
signature stored in the extended attribute. In that case, it will first
check whether there is an appended signature, and if not it will read it
from the extended attribute.

The format of the appended signature is the same used for signed kernel
modules. This means that the file can be signed with the scripts/sign-file
tool, with a command line such as this:

$ sign-file sha256 privkey_ima.pem x509_ima.der vmlinux

This code only works for files that are hashed from a memory buffer, not
for files that are read from disk at the time of hash calculation. In other
words, only hooks that use kernel_read_file can support appended
signatures.

The change in CONFIG_INTEGRITY_SIGNATURE to select CONFIG_KEYS instead of
depending on it is to avoid a dependency recursion in
CONFIG_IMA_APPRAISE_APPENDED_SIG, because CONFIG_MODULE_SIG_FORMAT selects
CONFIG_KEYS and Kconfig complains that CONFIG_INTEGRITY_SIGNATURE depends
on it.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 crypto/asymmetric_keys/pkcs7_parser.c    | 12 +++++
 crypto/asymmetric_keys/pkcs7_verify.c    | 13 +++++
 include/crypto/pkcs7.h                   |  3 ++
 include/linux/verification.h             |  1 +
 security/integrity/Kconfig               |  2 +-
 security/integrity/ima/Kconfig           | 13 +++++
 security/integrity/ima/ima.h             |  8 +++
 security/integrity/ima/ima_appraise.c    | 86 +++++++++++++++++++++++++++++++-
 security/integrity/ima/ima_main.c        | 30 +++++++++--
 security/integrity/ima/ima_policy.c      | 29 ++++++++---
 security/integrity/integrity.h           | 20 +++++---
 12 files changed, 199 insertions(+), 19 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index e4b0ed386bc8..0d58ecfba2ea 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -28,6 +28,7 @@ const char *const key_being_used_for[NR__KEY_BEING_USED_FOR] = {
 	[VERIFYING_KEXEC_PE_SIGNATURE]		= "kexec PE sig",
 	[VERIFYING_KEY_SIGNATURE]		= "key sig",
 	[VERIFYING_KEY_SELF_SIGNATURE]		= "key self sig",
+	[VERIFYING_KEXEC_CMS_SIGNATURE]		= "kexec CMS sig",
 	[VERIFYING_UNSPECIFIED_SIGNATURE]	= "unspec sig",
 };
 EXPORT_SYMBOL_GPL(key_being_used_for);
diff --git a/crypto/asymmetric_keys/pkcs7_parser.c b/crypto/asymmetric_keys/pkcs7_parser.c
index af4cd8649117..e41beda297a8 100644
--- a/crypto/asymmetric_keys/pkcs7_parser.c
+++ b/crypto/asymmetric_keys/pkcs7_parser.c
@@ -673,3 +673,15 @@ int pkcs7_note_signed_info(void *context, size_t hdrlen,
 		return -ENOMEM;
 	return 0;
 }
+
+/**
+ * pkcs7_get_message_sig - get signature in @pkcs7
+ *
+ * This function doesn't meaningfully support messages with more than one
+ * signature. It will always return the first signature.
+ */
+const struct public_key_signature *pkcs7_get_message_sig(
+					const struct pkcs7_message *pkcs7)
+{
+	return pkcs7->signed_infos ? pkcs7->signed_infos->sig : NULL;
+}
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 2d93d9eccb4d..eee78074580a 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -420,6 +420,19 @@ int pkcs7_verify(struct pkcs7_message *pkcs7,
 			return -EKEYREJECTED;
 		}
 		break;
+	case VERIFYING_KEXEC_CMS_SIGNATURE:
+		/* Shipping certificates in the CMS message is not allowed. */
+		if (pkcs7->certs) {
+			pr_warn("Signature isn't allowed to contain certificates.\n");
+			return -EBADMSG;
+		}
+
+		/* Shipping CRLs in the CMS message is not allowed. */
+		if (pkcs7->crl) {
+			pr_warn("Signature isn't allowed to contain CRLs.\n");
+			return -EBADMSG;
+		}
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 583f199400a3..a05a0d7214e6 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -29,6 +29,9 @@ 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..b7298d804440 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -22,6 +22,7 @@ enum key_being_used_for {
 	VERIFYING_KEY_SIGNATURE,
 	VERIFYING_KEY_SELF_SIGNATURE,
 	VERIFYING_UNSPECIFIED_SIGNATURE,
+	VERIFYING_KEXEC_CMS_SIGNATURE,
 	NR__KEY_BEING_USED_FOR
 };
 extern const char *const key_being_used_for[NR__KEY_BEING_USED_FOR];
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
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 370eb2f4dd37..13662043bf90 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -155,6 +155,19 @@ config IMA_APPRAISE
 	  <http://linux-ima.sourceforge.net>
 	  If unsure, say N.
 
+config IMA_APPRAISE_APPENDED_SIG
+	bool "Support appended 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
+	   appended signature is the same used for signed kernel modules.
+	   The appended_imasig keyword can be used in the IMA policy for this
+	   purpose.
+
 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/ima.h b/security/integrity/ima/ima.h
index 51ef805cf7f3..60a823630d2a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -190,6 +190,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, int mask,
 		   enum ima_hooks func, int *pcr);
@@ -248,6 +250,12 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
 int ima_read_xattr(struct dentry *dentry,
 		   struct evm_ima_xattr_data **xattr_value);
 
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+void ima_read_appended_sig(const void *buf, loff_t *buf_len,
+			  struct evm_ima_xattr_data **xattr_value,
+			  int *xattr_len);
+#endif
+
 #else
 static inline int ima_appraise_measurement(enum ima_hooks func,
 					   struct integrity_iint_cache *iint,
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 8c8030a29117..789bcb1b2d89 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -15,6 +15,11 @@
 #include <linux/magic.h>
 #include <linux/ima.h>
 #include <linux/evm.h>
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+#include <linux/module_signature.h>
+#include <keys/asymmetric-type.h>
+#include <crypto/pkcs7.h>
+#endif
 
 #include "ima.h"
 
@@ -177,6 +182,85 @@ int ima_read_xattr(struct dentry *dentry,
 	return ret;
 }
 
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+void ima_read_appended_sig(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 public_key_signature *pks;
+	const struct module_signature *sig;
+	struct signature_v2_hdr *hdr;
+	struct pkcs7_message *pkcs7;
+	int i, hdr_len;
+	loff_t file_len = *buf_len;
+	size_t sig_len;
+	const void *p;
+
+	if (file_len <= marker_len + sizeof(*sig))
+		return;
+
+	p = buf + file_len - marker_len;
+	if (memcmp(p, MODULE_SIG_STRING, marker_len))
+		return;
+
+	file_len -= marker_len;
+	sig = (const struct module_signature *) (p - sizeof(*sig));
+
+	if (validate_module_signature(sig, file_len))
+		return;
+
+	sig_len = be32_to_cpu(sig->sig_len);
+	file_len -= sig_len + sizeof(*sig);
+
+	pkcs7 = pkcs7_parse_message(buf + file_len, sig_len);
+	if (IS_ERR(pkcs7))
+		return;
+
+	if (pkcs7_verify(pkcs7, VERIFYING_KEXEC_CMS_SIGNATURE))
+		goto out;
+
+	pks = pkcs7_get_message_sig(pkcs7);
+	if (!pks)
+		goto out;
+
+	/* IMA only supports RSA keys. */
+	if (strcmp(pks->pkey_algo, "rsa"))
+		goto out;
+
+	if (!pks->auth_ids[0])
+		goto out;
+
+	for (i = 0; i < HASH_ALGO__LAST; i++)
+		if (!strcmp(hash_algo_name[i], pks->hash_algo))
+			break;
+
+	if (i == HASH_ALGO__LAST)
+		goto out;
+
+	hdr_len = sizeof(*hdr) + pks->s_size;
+	hdr = kmalloc(hdr_len, GFP_KERNEL);
+	if (!hdr)
+		goto out;
+
+	hdr->type = EVM_IMA_XATTR_DIGSIG;
+	hdr->version = 2;
+	hdr->hash_algo = i;
+	memcpy(hdr->sig, pks->s, pks->s_size);
+	hdr->sig_size = cpu_to_be16(pks->s_size);
+
+	p = pks->auth_ids[0]->data + pks->auth_ids[0]->len - sizeof(hdr->keyid);
+	memcpy(&hdr->keyid, p, sizeof(hdr->keyid));
+
+	*xattr_value = (typeof(*xattr_value)) hdr;
+	*xattr_len = hdr_len;
+	*buf_len = file_len;
+
+ out:
+	pkcs7_free_message(pkcs7);
+}
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
+
 /*
  * ima_appraise_measurement - appraise file measurement
  *
@@ -205,7 +289,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 		if (rc && rc != -ENODATA)
 			goto out;
 
-		cause = iint->flags & IMA_DIGSIG_REQUIRED ?
+		cause = iint->flags & IMA_DIGSIG_REQUIRED_MASK ?
 				"IMA-signature-required" : "missing-hash";
 		status = INTEGRITY_NOLABEL;
 		if (opened & FILE_CREATED) {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2aebb7984437..994ee420b2ec 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -16,6 +16,9 @@
  *	implements the IMA hooks: ima_bprm_check, ima_file_mmap,
  *	and ima_file_check.
  */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #include <linux/module.h>
 #include <linux/file.h>
 #include <linux/binfmts.h>
@@ -228,9 +231,30 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 
 	template_desc = ima_template_desc_current();
 	if ((action & IMA_APPRAISE_SUBMASK) ||
-		    strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
-		/* read 'security.ima' */
-		xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
+		    strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) {
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+		unsigned long digsig_req;
+
+		if (iint->flags & IMA_APPENDED_DIGSIG_REQUIRED) {
+			if (!buf || !size)
+				pr_err("%s doesn't support appended_imasig\n",
+				       func_tokens[func]);
+			else
+				ima_read_appended_sig(buf, &size, &xattr_value,
+						      &xattr_len);
+		}
+
+		/*
+		 * Don't try to read the xattr if we require an appended
+		 * signature but failed to get one.
+		 */
+		digsig_req = iint->flags & IMA_DIGSIG_REQUIRED_MASK;
+		if (!xattr_len && digsig_req != IMA_APPENDED_DIGSIG_REQUIRED)
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
+			/* read 'security.ima' */
+			xattr_len = ima_read_xattr(file_dentry(file),
+						   &xattr_value);
+	}
 
 	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
 
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 39d43a5beb5a..fb652f5e3999 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -777,8 +777,15 @@ 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)
+			if (!strcmp(args[0].from, "imasig"))
 				entry->flags |= IMA_DIGSIG_REQUIRED;
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+			else if (!strcmp(args[0].from, "appended_imasig"))
+				entry->flags |= IMA_APPENDED_DIGSIG_REQUIRED;
+			else if (!strcmp(args[0].from, "appended_imasig|imasig"))
+				entry->flags |= IMA_DIGSIG_REQUIRED
+						| IMA_APPENDED_DIGSIG_REQUIRED;
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
 			else
 				result = -EINVAL;
 			break;
@@ -884,6 +891,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
@@ -896,12 +909,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;
@@ -1049,7 +1056,13 @@ int ima_policy_show(struct seq_file *m, void *v)
 			}
 		}
 	}
-	if (entry->flags & IMA_DIGSIG_REQUIRED)
+	if ((entry->flags & IMA_DIGSIG_REQUIRED_MASK) == IMA_DIGSIG_REQUIRED_MASK)
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+		seq_puts(m, "appraise_type=appended_imasig|imasig ");
+	else if (entry->flags & IMA_APPENDED_DIGSIG_REQUIRED)
+		seq_puts(m, "appraise_type=appended_imasig ");
+	else if (entry->flags & IMA_DIGSIG_REQUIRED)
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
 		seq_puts(m, "appraise_type=imasig ");
 	if (entry->flags & IMA_PERMIT_DIRECTIO)
 		seq_puts(m, "permit_directio ");
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index a53e7e4ab06c..de2a666740c1 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -27,12 +27,20 @@
 #define IMA_AUDITED		0x00000080
 
 /* iint cache flags */
-#define IMA_ACTION_FLAGS	0xff000000
-#define IMA_ACTION_RULE_FLAGS	0x06000000
-#define IMA_DIGSIG		0x01000000
-#define IMA_DIGSIG_REQUIRED	0x02000000
-#define IMA_PERMIT_DIRECTIO	0x04000000
-#define IMA_NEW_FILE		0x08000000
+#define IMA_ACTION_FLAGS		0xff000000
+#define IMA_ACTION_RULE_FLAGS		0x16000000
+#define IMA_DIGSIG			0x01000000
+#define IMA_DIGSIG_REQUIRED		0x02000000
+#define IMA_PERMIT_DIRECTIO		0x04000000
+#define IMA_NEW_FILE			0x08000000
+#define IMA_APPENDED_DIGSIG_REQUIRED	0x10000000
+
+#ifdef CONFIG_IMA_APPRAISE_APPENDED_SIG
+#define IMA_DIGSIG_REQUIRED_MASK	(IMA_DIGSIG_REQUIRED | \
+					 IMA_APPENDED_DIGSIG_REQUIRED)
+#else
+#define IMA_DIGSIG_REQUIRED_MASK	IMA_DIGSIG_REQUIRED
+#endif /* CONFIG_IMA_APPRAISE_APPENDED_SIG */
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_APPRAISE_SUBMASK)
-- 
2.7.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web