Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Mimi Zohar <zohar@linux.ibm.com>
To: Tyler Hicks <tyhicks@linux.microsoft.com>,
	Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	Lakshmi Ramasubramanian <nramas@linux.microsoft.com>,
	Prakhar Srivastava <prsriva02@gmail.com>,
	linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: Re: [PATCH v2 09/11] ima: Move validation of the keyrings conditional into ima_validate_rule()
Date: Tue, 30 Jun 2020 19:07:29 -0400
Message-ID: <1593558449.5057.12.camel@linux.ibm.com> (raw)
In-Reply-To: <20200626223900.253615-10-tyhicks@linux.microsoft.com>

On Fri, 2020-06-26 at 17:38 -0500, Tyler Hicks wrote:
> Use ima_validate_rule() to ensure that the combination of a hook
> function and the keyrings conditional is valid and that the keyrings
> conditional is not specified without an explicit KEY_CHECK func
> conditional. This is a code cleanup and has no user-facing change.
> 
> Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> ---
> 
> * v2
>   - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO,
>     IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be
>     present in the rule entry flags for non-buffer hook functions.
> 
>  security/integrity/ima/ima_policy.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 8cdca2399d59..43d49ad958fb 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
>  		case KEXEC_KERNEL_CHECK:
>  		case KEXEC_INITRAMFS_CHECK:
>  		case POLICY_CHECK:
> +			if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
> +					     IMA_UID | IMA_FOWNER | IMA_FSUUID |
> +					     IMA_INMASK | IMA_EUID | IMA_PCR |
> +					     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
> +					     IMA_PERMIT_DIRECTIO |
> +					     IMA_MODSIG_ALLOWED |
> +					     IMA_CHECK_BLACKLIST))

Other than KEYRINGS, this patch should continue to behave the same.
 However, this list gives the impressions that all of these flags are
permitted on all of the above flags, which isn't true.

For example, both IMA_MODSIG_ALLOWED & IMA_CHECK_BLACKLIST are limited
to appended signatures, meaning KERNEL_CHECK and KEXEC_KERNEL_CHECK.
 Both should only be allowed on APPRAISE action rules.

IMA_PCR should be limited to MEASURE action rules.

IMA_DIGSIG_REQUIRED should be limited to APPRAISE action rules.

> +				return false;
> +
>  			break;
>  		case KEXEC_CMDLINE:
>  			if (entry->action & ~(MEASURE | DONT_MEASURE))
> @@ -1027,7 +1036,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
>  		default:
>  			return false;
>  		}
> -	}
> +	} else if (entry->flags & IMA_KEYRINGS)
> +		return false;

IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST need to be added here as
well.

Mimi

>  
>  	return true;
>  }
> @@ -1209,7 +1219,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  			keyrings_len = strlen(args[0].from) + 1;
>  
>  			if ((entry->keyrings) ||
> -			    (entry->func != KEY_CHECK) ||
>  			    (keyrings_len < 2)) {
>  				result = -EINVAL;
>  				break;


  parent reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-26 22:38 [PATCH v2 00/11] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 01/11] ima: Have the LSM free its audit rule Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 02/11] ima: Free the entire rule when deleting a list of rules Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 03/11] ima: Free the entire rule if it fails to parse Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 04/11] ima: Fail rule parsing when buffer hook functions have an invalid action Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 05/11] ima: Fail rule parsing when the KEXEC_CMDLINE hook is combined with an invalid cond Tyler Hicks
2020-06-27 23:40   ` Lakshmi Ramasubramanian
2020-06-26 22:38 ` [PATCH v2 06/11] ima: Fail rule parsing when the KEY_CHECK " Tyler Hicks
2020-06-27 23:39   ` Lakshmi Ramasubramanian
2020-06-26 22:38 ` [PATCH v2 07/11] ima: Shallow copy the args_p member of ima_rule_entry.lsm elements Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 08/11] ima: Use correct type for " Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 09/11] ima: Move validation of the keyrings conditional into ima_validate_rule() Tyler Hicks
2020-06-27 23:49   ` Lakshmi Ramasubramanian
2020-06-29 14:16     ` Tyler Hicks
2020-06-30 23:07   ` Mimi Zohar [this message]
2020-07-02 22:16     ` Tyler Hicks
2020-07-03 14:15       ` Mimi Zohar
2020-06-26 22:38 ` [PATCH v2 10/11] ima: Use the common function to detect LSM conditionals in a rule Tyler Hicks
2020-06-26 22:39 ` [PATCH v2 11/11] ima: Support additional conditionals in the KEXEC_CMDLINE hook function Tyler Hicks
2020-06-28  0:03   ` Lakshmi Ramasubramanian
2020-07-01  8:04   ` Dave Young
2020-07-01 14:38     ` Tyler Hicks
2020-07-01  0:29 ` [PATCH v2 00/11] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support Mimi Zohar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1593558449.5057.12.camel@linux.ibm.com \
    --to=zohar@linux.ibm.com \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=jmorris@namei.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=nramas@linux.microsoft.com \
    --cc=prsriva02@gmail.com \
    --cc=serge@hallyn.com \
    --cc=tyhicks@linux.microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git