From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lakshmi Ramasubramanian Date: Wed, 06 Nov 2019 23:52:31 +0000 Subject: Re: [PATCH v4 08/10] IMA: Defined functions to queue and dequeue keys for measurement Message-Id: <8b2fd578-7429-f5b8-4286-1face91e1ae6@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset="ibm852" Content-Transfer-Encoding: base64 List-Id: References: <20191106190116.2578-1-nramas@linux.microsoft.com> <20191106190116.2578-9-nramas@linux.microsoft.com> <1573080281.5028.314.camel@linux.ibm.com> In-Reply-To: <1573080281.5028.314.camel@linux.ibm.com> To: Mimi Zohar , dhowells@redhat.com, matthewgarrett@google.com, sashal@kernel.org, jamorris@linux.microsoft.com, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, keyrings@vger.kernel.org, linux-kernel@vger.kernel.org Ck9uIDExLzYvMjAxOSAyOjQ0IFBNLCBNaW1pIFpvaGFyIHdyb3RlOgoKPj4gK2ludCBpbWFfcXVl dWVfb3JfcHJvY2Vzc19rZXlfZm9yX21lYXN1cmVtZW50KHN0cnVjdCBrZXkgKmtleXJpbmcsCj4+ ICsJCQkJCSAgICAgc3RydWN0IGtleSAqa2V5KQo+PiArewo+PiArCWludCByYyA9IDA7Cj4+ICsJ c3RydWN0IGltYV9tZWFzdXJlX2tleV9lbnRyeSAqZW50cnkgPSBOVUxMOwo+PiArCWNvbnN0IHN0 cnVjdCBwdWJsaWNfa2V5ICpwazsKPj4gKwo+PiArCWlmIChrZXktPnR5cGUgIT0gJmtleV90eXBl X2FzeW1tZXRyaWMpCj4+ICsJCXJldHVybiAwOwo+PiArCj4+ICsJbXV0ZXhfbG9jaygmaW1hX21l YXN1cmVfa2V5c19tdXRleCk7Cgo+IAo+IFVubGVzcyB0aGUga2V5IGlzIGJlaW5nIHF1ZXVlZCwg dGhlcmUncyBubyByZWFzb24gdG8gdGFrZSB0aGUgbG9jay4KClJlYXNvbiB0aGUgbG9jayBpcyB0 YWtlbiBldmVuIGluIHRoZSBjYXNlIHRoZSBrZXkgaXMgbm90IHF1ZXVlZCBpcyB0byAKYXZvaWQg dGhlIGZvbGxvd2luZyByYWNlIGNvbmRpdGlvbjoKCiAgPT4gaW1hX2luaXQoKSBzZXRzIGltYV9p bml0aWFsaXplZCBmbGFnIGFuZCBjYWxscyB0aGUgZGVxdWV1ZSBmdW5jdGlvbgoKICA9PiBJZiBJ TUEgaG9vayBjaGVja3MgaW1hX2luaXRpYWxpemVkIGZsYWcgb3V0c2lkZSB0aGUgbG9jayBhbmQg c2VlcyAKdGhlIGZsYWcgaXMgbm90IHNldCwgaXQgd2lsbCBjYWxsIHRoZSBxdWV1ZSBmdW5jdGlv bi4KCiAgPT4gSWYgdGhlIGFib3ZlIHR3byBzdGVwcyByYWNlLCB0aGUga2V5IGNvdWxkIGdldCBh ZGRlZCB0byB0aGUgcXVldWUgCmFmdGVyIGltYV9pbml0KCkgaGFzIHByb2Nlc3NlZCB0aGUgcXVl dWVkIGtleXMuCgpUaGF0J3MgdGhlIHJlYXNvbiBJIG5hbWVkIHRoZSBmdW5jdGlvbiBjYWxsZWQg YnkgdGhlIElNQSBob29rIHRvIAppbWFfcXVldWVfb3JfcHJvY2Vzc19rZXlfZm9yX21lYXN1cmVt ZW50KCkuCgpCdXQgSSBjYW4gbWFrZSB0aGUgZm9sbG93aW5nIGNoYW5nZToKCiAgPT4gSU1BIGhv b2sgY2hlY2tzIHRoZSBmbGFnLgogID0+IElmIGl0IGlzIHNldCwgcHJvY2VzcyBrZXkgaW1tZWRp YXRlbHkKICA9PiBJZiB0aGUgZmxhZyBpcyBub3Qgc2V0LCBjYWxsIGltYV9xdWV1ZV9vcl9wcm9j ZXNzX2tleV9mb3JfbWVhc3VyZW1lbnQoKQoKaW1hX3F1ZXVlX29yX3Byb2Nlc3Nfa2V5X2Zvcl9t ZWFzdXJlbWVudCgpIHdpbGwgZG8gdGhlIGZvbGxvd2luZzoKCiAgPT4gV2l0aCB0aGUgbG9jayBo ZWxkIGNoZWNrIGltYV9pbml0aWFsaXplZCBmbGFnCiAgPT4gSWYgdHJ1ZSByZWxlYXNlIHRoZSBs b2NrIGFuZCBjYWxsIHByb2Nlc3NfYnVmZmVyX21lYXN1cmVtZW50KCkKICA9PiBJZiBmYWxzZSwg cXVldWUgdGhlIGtleSBhbmQgdGhlbiByZWxlYXNlIHRoZSBsb2NrCgpXb3VsZCB0aGF0IGJlIGFj Y2VwdGFibGU/Cgo+IE1lYXN1cmluZyB0aGUga2V5IHNob3VsZCBiZSBkb25lIGluIGltYV9wb3N0 X2tleV9jcmVhdGVfb3JfdXBkYXRlKCkKPiB1bmxlc3MsIGl0IGlzIGJlaW5nIGRlZmVycmVkLiDC oFBsZWFzZSB1cGRhdGUgdGhlIGZ1bmN0aW9uIG5hbWUgdG8KPiByZWZsZWN0IHRoaXMuCgpKdXN0 IHdhbnRlZCB0byBjb25maXJtOgpSZW5hbWUgaW1hX3Bvc3Rfa2V5X2NyZWF0ZV9vcl91cGRhdGUo KSB0byBhIG1vcmUgYXBwcm9wcmlhdGUgbmFtZT8KCkFub3RoZXIgcmVhc29uIGZvciBkb2luZyBh bGwga2V5IHJlbGF0ZWQgb3BlcmF0aW9ucyBpbiAKaW1hX3F1ZXVlX29yX3Byb2Nlc3Nfa2V5X2Zv cl9tZWFzdXJlbWVudCgpIGlzIHRvIGlzb2xhdGUga2V5IHJlbGF0ZWQgCmNvZGUgaW4gYSBzZXBh cmF0ZSBDIGZpbGUgYW5kIGJ1aWxkIGl0IGlmIGFuZCBvbmx5IGlmIHRoZSBDT05GSUcgCmRlcGVu ZGVuY2llcyBhcmUgbWV0LgoKV2l0aCByZXNwZWN0IHRvIGxvYWRpbmcgY3VzdG9tIHBvbGljeSwg SSB3aWxsIHRha2UgYSBsb29rIGF0IGhvdyB0byAKaGFuZGxlIHRoYXQgY2FzZS4gVGhhbmtzIGZv ciBwb2ludGluZyB0aGF0IG91dC4KCj4gTWltaQoKdGhhbmtzLAogIC1sYWtzaG1p From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C48D4FC6195 for ; Wed, 6 Nov 2019 23:52:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 9660D2173E for ; Wed, 6 Nov 2019 23:52:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=linux.microsoft.com header.i=@linux.microsoft.com header.b="Gu1APcN4" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732310AbfKFXwM (ORCPT ); Wed, 6 Nov 2019 18:52:12 -0500 Received: from linux.microsoft.com ([13.77.154.182]:53368 "EHLO linux.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725937AbfKFXwM (ORCPT ); Wed, 6 Nov 2019 18:52:12 -0500 Received: from [10.137.112.111] (unknown [131.107.147.111]) by linux.microsoft.com (Postfix) with ESMTPSA id 2FC6F20B7192; Wed, 6 Nov 2019 15:52:11 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 2FC6F20B7192 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1573084331; bh=7+TITNWnuuqtobgXFF9AfJ8kAcAt4vZ2t9MtwS6OXX4=; h=Subject:To:References:From:Date:In-Reply-To:From; b=Gu1APcN4pyax71gcqYaAGdCtpIOmXf+wnCOL+xZC42h5YzThdJ0hzJk82wr3IR8sr 1OMthYH/qbPa2X011zafPUPpOwABgK8hFxmOm96D/wJbfa5DVYlr44SFXuITIHb7CI 6HV2JMReS+0R5u4OtMJlXQ+E29piD77UmskPLmck= Subject: Re: [PATCH v4 08/10] IMA: Defined functions to queue and dequeue keys for measurement To: Mimi Zohar , dhowells@redhat.com, matthewgarrett@google.com, sashal@kernel.org, jamorris@linux.microsoft.com, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, keyrings@vger.kernel.org, linux-kernel@vger.kernel.org References: <20191106190116.2578-1-nramas@linux.microsoft.com> <20191106190116.2578-9-nramas@linux.microsoft.com> <1573080281.5028.314.camel@linux.ibm.com> From: Lakshmi Ramasubramanian Message-ID: <8b2fd578-7429-f5b8-4286-1face91e1ae6@linux.microsoft.com> Date: Wed, 6 Nov 2019 15:52:31 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.2.1 MIME-Version: 1.0 In-Reply-To: <1573080281.5028.314.camel@linux.ibm.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/6/2019 2:44 PM, Mimi Zohar wrote: >> +int ima_queue_or_process_key_for_measurement(struct key *keyring, >> + struct key *key) >> +{ >> + int rc = 0; >> + struct ima_measure_key_entry *entry = NULL; >> + const struct public_key *pk; >> + >> + if (key->type != &key_type_asymmetric) >> + return 0; >> + >> + mutex_lock(&ima_measure_keys_mutex); > > Unless the key is being queued, there's no reason to take the lock. Reason the lock is taken even in the case the key is not queued is to avoid the following race condition: => ima_init() sets ima_initialized flag and calls the dequeue function => If IMA hook checks ima_initialized flag outside the lock and sees the flag is not set, it will call the queue function. => If the above two steps race, the key could get added to the queue after ima_init() has processed the queued keys. That's the reason I named the function called by the IMA hook to ima_queue_or_process_key_for_measurement(). But I can make the following change: => IMA hook checks the flag. => If it is set, process key immediately => If the flag is not set, call ima_queue_or_process_key_for_measurement() ima_queue_or_process_key_for_measurement() will do the following: => With the lock held check ima_initialized flag => If true release the lock and call process_buffer_measurement() => If false, queue the key and then release the lock Would that be acceptable? > Measuring the key should be done in ima_post_key_create_or_update() > unless, it is being deferred.  Please update the function name to > reflect this. Just wanted to confirm: Rename ima_post_key_create_or_update() to a more appropriate name? Another reason for doing all key related operations in ima_queue_or_process_key_for_measurement() is to isolate key related code in a separate C file and build it if and only if the CONFIG dependencies are met. With respect to loading custom policy, I will take a look at how to handle that case. Thanks for pointing that out. > Mimi thanks, -lakshmi