Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v9 5/6] IMA: Add support to limit measuring keys
From:   Mimi Zohar <zohar@linux.ibm.com>
To:     Lakshmi Ramasubramanian <nramas@linux.microsoft.com>,
        linux-integrity@vger.kernel.org
Cc:     eric.snowberg@oracle.com, dhowells@redhat.com,
        matthewgarrett@google.com, sashal@kernel.org,
        jamorris@linux.microsoft.com, linux-kernel@vger.kernel.org,
        keyrings@vger.kernel.org
Date:   Wed, 27 Nov 2019 13:52:21 -0500
In-Reply-To: <20191127015654.3744-6-nramas@linux.microsoft.com>
References: <20191127015654.3744-1-nramas@linux.microsoft.com>
         <20191127015654.3744-6-nramas@linux.microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Message-Id: <1574880741.4793.292.camel@linux.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -79,6 +79,7 @@ struct ima_rule_entry {
>  		int type;	/* audit type */
>  	} lsm[MAX_LSM_RULES];
>  	char *fsname;
> +	char *keyrings; /* Measure keys added to these keyrings */
>  	struct ima_template_desc *template;
>  };
>  
> @@ -356,6 +357,55 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
>  	return NOTIFY_OK;
>  }
>  
> +/**
> + * ima_match_keyring - determine whether the keyring matches the measure rule
> + * @rule: a pointer to a rule
> + * @keyring: name of the keyring to match against the measure rule
> + *
> + * If the measure action for KEY_CHECK does not specify keyrings=
> + * option then return true (Measure all keys).
> + * Else, return true if the given keyring name is present in
> + * the keyrings= option. False, otherwise.

This is suppose to be a comment, not code or pseudo code.  Please
refer to the section "Comments" in Documentation/process/coding-
style.rst.
 
> + */
> +static bool ima_match_keyring(struct ima_rule_entry *rule,
> +			      const char *keyring)
> +{
> +	const char *p;
> +
> +	/* If "keyrings=" is not specified all keys are measured. */
> +	if (!rule->keyrings)
> +		return true;
> +
> +	if (!keyring)
> +		return false;
> +
> +	/*
> +	 * "keyrings=" is specified in the policy in the format below:
> +	 *   keyrings=.builtin_trusted_keys|.ima|.evm
> +	 *
> +	 * Each keyring name in the option is separated by a '|' and
> +	 * the last keyring name is null terminated.
> +	 *
> +	 * The given keyring is considered matched only if
> +	 * the whole keyring name matched a keyring name specified
> +	 * in the "keyrings=" option.
> +	 */
> +	p = strstr(rule->keyrings, keyring);
> +	if (p) {
> +		/*
> +		 * Found a substring match. Check if the character
> +		 * at the end of the keyring name is | (keyring name
> +		 * separator) or is the terminating null character.
> +		 * If yes, we have a whole string match.
> +		 */
> +		p += strlen(keyring);
> +		if (*p == '|' || *p == '\0')
> +			return true;
> +	}
> +

Using "while strsep()" would simplify this code, removing the need for
such a long comment.

Mimi

> +	return false;
> +}
> +