linux-kernel - Re: [PATCH 1/2] trusted-key: allow overwriting the migratable flag

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]

Date:	Wed, 02 Nov 2011 18:37:37 +0100
From:	Roberto Sassu <roberto.sassu@...ito.it>
To:	David Safford <safford@...son.ibm.com>
CC:	keyrings@...ux-nfs.org, linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org, zohar@...ibm.com,
	dhowells@...hat.com, jmorris@...ei.org
Subject: Re: [PATCH 1/2] trusted-key: allow overwriting the migratable flag

On 11/02/2011 05:58 PM, David Safford wrote:
> On Wed, 2011-11-02 at 13:41 +0100, Roberto Sassu wrote:
>> The migratable should be modifiable during the key update() method. This
>> allows for example to update a migratable trusted key, wrapped by a TPM
>> key, to a a non-migratable one sealed under the SRK with a PCR set.
>>
>> Signed-off-by: Roberto Sassu<roberto.sassu@...ito.it>
>
> I can see a use case for updating a migratable key to a non-migratable
> one - such as keeping a migratable master on a flash drive, and keeping
> only the non-migratable copy on-line. I certainly don't want the
> ability to change a non-migratable to migratable, as that would defeat
> the entire purpose of non-migratable.
>
> I don't think this patch actually does either, though.
>
>> ---
>>   security/keys/trusted.c |    1 -
>>   1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
>> index 0c33e2e..8777015 100644
>> --- a/security/keys/trusted.c
>> +++ b/security/keys/trusted.c
>> @@ -1036,7 +1036,6 @@ static int trusted_update(struct key *key, const void *data, size_t datalen)
>>   		goto out;
>>   	}
>>   	/* copy old key values, and reseal with new pcrs */
>> -	new_p->migratable = p->migratable;
>
> Taking out this line appears only to remove a redundant assignment.
> We can only get here if the old key is already migratable, and the
> earlier trusted_payload_alloc() initializes the new copy to
> migratable by default. I don't see how the flag can be changed
> with this patch. Perhaps I'm missing something or this was just
> the start, and there is more to come?
>

Hi Dave

i think this line should be removed because it overwrites
the assignment made in datablob_parse() -> getoptions().

You can see this behaviour by executing the following commands:

1) keyctl add trusted kmk "new 32" @u
2) keyctl update $(keyctl search @u trusted kmk) "update migratable=0"
3) keyctl update $(keyctl search @u trusted kmk) "update migratable=0"

The third operation should fail because of the lines at the
begin of trusted_update():

---------
	if (!p->migratable)
		return -EPERM;
---------

but instead, without the patch, it is performed successfully.

Thanks

Roberto Sassu


> dave
>
>>   	new_p->key_len = p->key_len;
>>   	memcpy(new_p->key, p->key, p->key_len);
>>   	dump_payload(p);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/