linux-kernel - Re: [PATCH v9 5/6] IMA: Add support to limit measuring keys

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <18b30666-7c44-f81e-8515-189052007e47@linux.microsoft.com>
Date:   Wed, 27 Nov 2019 16:44:22 -0800
From:   Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>
To:     Mimi Zohar <zohar@...ux.ibm.com>, linux-integrity@...r.kernel.org
Cc:     eric.snowberg@...cle.com, dhowells@...hat.com,
        matthewgarrett@...gle.com, sashal@...nel.org,
        jamorris@...ux.microsoft.com, linux-kernel@...r.kernel.org,
        keyrings@...r.kernel.org
Subject: Re: [PATCH v9 5/6] IMA: Add support to limit measuring keys

On 11/27/19 10:52 AM, Mimi Zohar wrote:

Hi Mimi,

>> +static bool ima_match_keyring(struct ima_rule_entry *rule,
>> +			      const char *keyring)
>> +{
>> +	/*
>> +	 * "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

strsep() modifies the source string (replaces the delimiter with '\0' 
and also updates the source string pointer). I am not sure it can be 
used for our scenario. Please correct me if I am wrong.

Initial IMA policy:
-------------------
measure func=KEY_CHECK 
keyrings=.ima|.evm|.builtin_trusted_keys|.blacklist template=ima-buf

Policy after adding a key to .ima keyring:
------------------------------------------
measure func=KEY_CHECK keyrings=.evm|.builtin_trusted_keys|.blacklist 
template=ima-buf

Policy after adding a key to a keyring that is not listed in the policy:
------------------------------------------------------------------------
measure func=KEY_CHECK keyrings= template=ima-buf

********************************************************************************

Please see the description from the man page for strsep():

http://man7.org/linux/man-pages/man3/strsep.3.html

char *strsep(char **stringp, const char *delim);

This function finds the first token in the string *stringp, that is 
delimited by one of the bytes in the string delim.  This token is 
terminated by overwriting the delimiter with a null byte ('\0'), and 
*stringp is updated to point past the token.

thanks,
  -lakshmi