From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Berger Subject: Re: [PATCH] audit: add containerid support for IMA-audit Date: Mon, 21 May 2018 17:57:29 -0400 Message-ID: References: <1520257393.10396.291.camel@linux.vnet.ibm.com> <2397631.78oLu0QVqb@x2> <21646a72-e782-e33a-9e75-5cc98b241f36@linux.vnet.ibm.com> <4015765.rtofcNpGHU@x2> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <4015765.rtofcNpGHU@x2> Content-Language: en-MW List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Steve Grubb Cc: paul-r2n+y4ga6xFZroRs9YW3xA@public.gmane.org, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, LKML , Linux-Audit Mailing List , linux-integrity , Mimi Zohar List-Id: containers.vger.kernel.org T24gMDUvMjEvMjAxOCAwMjozMCBQTSwgU3RldmUgR3J1YmIgd3JvdGU6Cj4gSGVsbG8gU3RlZmFu LAo+Cj4gT24gTW9uZGF5LCBNYXkgMjEsIDIwMTggMTo1MzowNCBQTSBFRFQgU3RlZmFuIEJlcmdl ciB3cm90ZToKPj4gT24gMDUvMjEvMjAxOCAxMjo1OCBQTSwgU3RldmUgR3J1YmIgd3JvdGU6Cj4+ PiBPbiBUaHVyc2RheSwgTWF5IDE3LCAyMDE4IDEwOjE4OjEzIEFNIEVEVCBTdGVmYW4gQmVyZ2Vy IHdyb3RlOgo+Pj4+PiBhdWRpdF9sb2dfY29udGFpbmVyX2luZm8oKSB0aGVuIHJlbGVhc2luZyB0 aGUgbG9jYWwgY29udGV4dC4gIFRoaXMKPj4+Pj4gdmVyc2lvbiBvZiB0aGUgcmVjb3JkIGhhcyBh ZGRpdGlvbmFsIGNvbmNlcm5zIGNvdmVyZWQgaGVyZToKPj4+Pj4gaHR0cHM6Ly9naXRodWIuY29t L2xpbnV4LWF1ZGl0L2F1ZGl0LWtlcm5lbC9pc3N1ZXMvNTIKPj4+PiBGb2xsb3dpbmcgdGhlIGRp c2N1c3Npb24gdGhlcmUgYW5kIHRoZSBjb25jZXJuIHdpdGggYnJlYWtpbmcgdXNlciBzcGFjZSwK Pj4+PiBob3cgY2FuIHdlIHNwbGl0IHVwIHRoZSBBVURJVF9JTlRFR1JJVFlfUlVMRSB0aGF0IGlz IHVzZWQgaW4KPj4+PiBpbWFfYXVkaXRfbWVhc3VyZW1lbnQoKSBhbmQgaW1hX3BhcnNlX3J1bGUo KSwgd2l0aG91dCAnYnJlYWtpbmcgdXNlcgo+Pj4+IHNwYWNlJz8KPj4+Pgo+Pj4+IEEgbWVzc2Fn ZSBwcm9kdWNlZCBieSBpbWFfcGFyc2VfcnVsZSgpIGxvb2tzIGxpa2UgdGhpcyBoZXJlOgo+Pj4+ Cj4+Pj4gdHlwZT1JTlRFR1JJVFlfUlVMRSBtc2c9YXVkaXQoMTUyNjU2NjIxMy44NzA6MzA1KTog YWN0aW9uPSJkb250X21lYXN1cmUiCj4+Pj4gZnNtYWdpYz0iMHg5ZmEwIiByZXM9MQo+Pj4gV2h5 IGlzIGFjdGlvbiBhbmQgZnNtYWdpYyBiZWluZyBsb2dnZWQgYXMgdW50cnVzdGVkIHN0cmluZ3M/ IFVudHJ1c3RlZAo+Pj4gc3RyaW5ncyBhcmUgdXNlZCB3aGVuIGFuIHVucHJpdmlsZWdlZCB1c2Vy IGNhbiBhZmZlY3QgdGhlIGNvbnRlbnRzIG9mIHRoZQo+Pj4gZmllbGQgc3VjaCBhcyBjcmVhdGlu ZyBhIGZpbGUgd2l0aCBzcGFjZSBvciBzcGVjaWFsIGNoYXJhY3RlcnMgaW4gdGhlCj4+PiBuYW1l Lgo+Pj4KPj4+IEFsc28sIHN1YmplY3QgYW5kIG9iamVjdCBpbmZvcm1hdGlvbiBpcyBtaXNzaW5n LiBXaG8gbG9hZGVkIHRoaXMgcnVsZT8KPj4+Cj4+Pj4gaW4gY29udHJhc3QgdG8gdGhhdCBhbiBJ TlRFR1JJVFlfUENSIHJlY29yZCB0eXBlOgo+Pj4+Cj4+Pj4gdHlwZT1JTlRFR1JJVFlfUENSIG1z Zz1hdWRpdCgxNTI2NTY2MjM1LjE5MzozMzQpOiBwaWQ9MTYxNSB1aWQ9MCBhdWlkPTAKPj4+PiBz ZXM9MiBzdWJqPXVuY29uZmluZWRfdTp1bmNvbmZpbmVkX3I6dW5jb25maW5lZF90OnMwLXMwOmMw LmMxMDIzCj4+Pj4gb3A9ImludmFsaWRfcGNyIiBjYXVzZT0ib3Blbl93cml0ZXJzIiBjb21tPSJz Y3AiCj4+Pj4gbmFtZT0iL3Zhci9sb2cvYXVkaXQvYXVkaXQubG9nIiBkZXY9ImRtLTAiIGlubz0x OTYyNjI1IHJlcz0xCj4+PiBXaHkgaXMgb3AgJiBjYXVzZSBiZWluZyBsb2dnZWQgYXMgYW4gdW50 cnVzdGVkIHN0cmluZz8gVGhpcyBhbHNvIGhhcwo+Pj4gaW5jb21wbGV0ZSBzdWJqZWN0IGluZm9y bWF0aW9uLgo+PiBJdCdzIGNhbGxpbmcgYXVkaXRfbG9nX3N0cmluZygpIGluIGJvdGggY2FzZXM6 Cj4+Cj4+IGh0dHBzOi8vZWxpeGlyLmJvb3RsaW4uY29tL2xpbnV4L2xhdGVzdC9zb3VyY2Uvc2Vj dXJpdHkvaW50ZWdyaXR5L2ludGVncml0eQo+PiBfYXVkaXQuYyNMNDgKPiBJIHNlZS4gTG9va2lu ZyB0aGluZ3Mgb3ZlciwgSSBzZWUgdGhhdCBpdCBzZWVtcyBsaWtlIGl0IHNob3VsZCBkbyB0aGUg cmlnaHQKPiB0aGluZy4gQnV0IHRoZSBvcmlnaW5hbCBwdXJwb3NlIGZvciB0aGlzIGZ1bmN0aW9u IGlzIGhlcmU6Cj4KPiBodHRwczovL2VsaXhpci5ib290bGluLmNvbS9saW51eC9sYXRlc3Qvc291 cmNlL2tlcm5lbC9hdWRpdC5jI0wxOTQ0Cj4KPiBUaGlzIGlzIHdoZXJlIGl0IGlzIGxvZ2dpbmcg YW4gdW50cnVzdGVkIHN0cmluZyBhbmQgaGFzIHRvIGRlY2lkZSB0byBlbmNvZGUKPiBpdCBvciBq dXN0IHBsYWNlIGl0IGluIHF1b3Rlcy4gSWYgaXQgaGFzIHF1b3RlcywgdGhhdCBtZWFucyBpdCdz IGFuIHVudHJ1c3RlZAo+IHN0cmluZyBidXQgaGFzIG5vIGNvbnRyb2wgY2hhcmFjdGVycyBpbiBp dC4gSSB0aGluayB5b3Ugd2FudCB0byB1c2UKPiBhdWRpdF9sb2dfZm9ybWF0KCkgZm9yIGFueSBz dHJpbmcgdGhhdCBpcyB0cnVzdHdvcnRoeS4KClJlcGxhY2luZyBhbGwgb2NjdXJyZW5jZXMgKGlu IElNQSkgb2YgYXVkaXRfbG9nX3N0cmluZygpIHdpdGggCmF1ZGl0X2xvZ19mb3JtYXQoKS4KPgo+ Cj4gQXMgYW4gYXNpZGUsIEkgd29uZGVyIHdoeSBhdWRpdF9sb2dfc3RyaW5nKCkgaXMgaW4gdGhl IEFQSSB3aGVuIGl0IGlzIGEKPiBoZWxwZXIgdG8gYXVkaXRfbG9nX3VudHJ1c3RlZHN0cmluZygp ID8gIFdpdGhvdXQgdW5kZXJzdGFuZGluZyB0aGUgcnVsZXMgb2YKPiB1bnRydXN0ZWQgc3RyaW5n cywgaXQgY2FuJ3QgYmUgdXNlZCBjb3JyZWN0bHkgd2l0aG91dCByZS1pbnZlbnRpbmcKPiBhdWRp dF9sb2dfdW50cnVzdGVkc3RyaW5nKCkgYnkgaGFuZC4KPgo+Cj4+Pj4gU2hvdWxkIHNvbWUgb2Yg dGhlIGZpZWxkcyBmcm9tIElOVEVHUklUWV9QQ1IgYWxzbyBhcHBlYXIgaW4KPj4+PiBJTlRFR1JJ VFlfUlVMRT8gSWYgc28sIHdoaWNoIG9uZXM/Cj4+PiBwaWQsIHVpZCwgYXVpZCwgdHR5LCBzZXNz aW9uLCBzdWJqLCBjb21tLCBleGUsIHJlcy4gIDwtIHRoZXNlIGFyZQo+Pj4gcmVxdWlyZWQgdG8g YmUgc2VhcmNoYWJsZQo+Pj4KPj4+PiBXZSBjb3VsZCBwcm9iYWJseSByZWZhY3RvciB0aGUgY3Vy cmVudCAgaW50ZWdyaXR5X2F1ZGl0X21lc3NhZ2UoKSBhbmQKPj4+PiBoYXZlIGltYV9wYXJzZV9y dWxlKCkgY2FsbCBpbnRvIGl0IHRvIGdldCB0aG9zZSBmaWVsZHMgYXMgd2VsbC4gSQo+Pj4+IHN1 cHBvc2UgYWRkaW5nIG5ldyBmaWVsZHMgdG8gaXQgd291bGRuJ3QgYmUgY29uc2lkZXJlZCBicmVh a2luZyB1c2VyCj4+Pj4gc3BhY2U/Cj4+PiBUaGUgYXVkaXQgdXNlciBzcGFjZSB1dGlsaXRpZXMg cHJldHR5IG11Y2ggZXhwZWN0cyB0aG9zZSBmaWVsZHMgaW4gdGhhdAo+Pj4gb3JkZXIgZm9yIGFu eSBJTUEgb3JpZ2luYXRpbmcgZXZlbnRzLiBZb3UgY2FuIGFkZCB0aGluZ3MgbGlrZSBvcCBvcgo+ Pj4gY2F1c2UgYmVmb3JlCj4+IFdlIHdpbGwgY2FsbCBpbnRvIGF1ZGl0X2xvZ190YXNrLCB3aGlj aCB3aWxsIHB1dCB0aGUgcGFyYW1ldGVycyBpbnRvCj4+IGNvcnJlY3Qgb3JkZXI6Cj4+Cj4+IGF1 aWQgdWlkIGdpZCBzZXMgc3ViaiBwaWQgY29tbSBleGUKPiBJJ20gdGVsbGluZyB5b3Ugd2hhdCB0 aGUgY29ycmVjdCBvcmRlciBpcy4gIDotKSAgQSBsb25nIHRpbWUgYWdvLCB0aGUgSU1BCgo6LSkg VGhhbmtzLiBXYXMgZ2V0dGluZyBjb25mdXNlZC4KCj4gc3lzdGVtIGhhZCBhdWRpdCBldmVudHMg d2l0aCB0aGUgb3JkZXIgSSdtIG1lbnRpb25pbmcuIEZvciBleGFtcGxlLCBoZXJlJ3MKPiBvbmUg ZnJvbSBhIGxvZyBJIGNvbGxlY3RlZCBiYWNrIGluIDIwMTM6Cj4KPiB0eXBlPUlOVEVHUklUWV9Q Q1IgbXNnPWF1ZGl0KDEzMjc0MDkwMjEuODEzOjIxKTogcGlkPTEgdWlkPTAgYXVpZD00Mjk0OTY3 Mjk1Cj4gc2VzPTQyOTQ5NjcyOTUgc3Viaj1rZXJuZWwgb3A9ImFkZF90ZW1wbGF0ZV9tZWFzdXJl IiBjYXVzZT0iaGFzaF9hZGRlZCIKPiBjb21tPSJpbml0IiBuYW1lPSIwMXBhcnNlLWtlcm5lbC5z aCIgZGV2PXJvb3RmcyBpbm89NTQxMyByZXM9MAo+Cj4gaXQgd2FzIG1pc3NpbmcgInR0eSIgYW5k ICJleGUiLCBidXQgdGhlIG9yZGVyIGlzIGFzIEkgbWVudGlvbmVkLiBUaGUKPiBleHBlY3RhdGlv biBpcyB0aGF0IElOVEVHUklUWSBldmVudHMgbWFpbnRhaW4gdGhpcyBlc3RhYmxpc2hlZCBvcmRl ciBhY3Jvc3MKPiBhbGwgZXZlbnRzLgoKSSBhbSAqYXBwZW5kaW5nKiBleGU9IGFuZCB0dHk9IG5v dzoKCnR5cGU9SU5URUdSSVRZX1BDUiBtc2c9YXVkaXQoMTUyNjkzOTA0Ny44MDk6MzA1KTogcGlk PTE2MDkgdWlkPTAgYXVpZD0wIApzZXM9MiBzdWJqPXVuY29uZmluZWRfdTp1bmNvbmZpbmVkX3I6 dW5jb25maW5lZF90OnMwLXMwOmMwLmMxMDIzIApvcD0iaW52YWxpZF9wY3IiIGNhdXNlPSJvcGVu X3dyaXRlcnMiIGNvbW09InNzaCIgCm5hbWU9Ii92YXIvbGliL3Nzcy9tYy9wYXNzd2QiIGRldj0i ZG0tMCIgaW5vPTE5NjI2NzkgcmVzPTEgCmV4ZT0iL3Vzci9iaW4vc3NoIiB0dHk9dHR5MgoKCiDC oMKgIFN0ZWZhbgoKCj4KPiBUaGFua3MsCj4gLVN0ZXZlCj4KPgo+PiBodHRwczovL2VsaXhpci5i b290bGluLmNvbS9saW51eC9sYXRlc3Qvc291cmNlL2tlcm5lbC9hdWRpdHNjLmMjTDI0MzMKPj4K Pj4+IHRoYXQuIFRoZSByZWFzb24gd2h5IHlvdSBjYW4gZG8gdGhhdCBpcyB0aG9zZSBhZGRpdGlv bmFsIGZpZWxkcyBhcmUgbm90Cj4+PiByZXF1aXJlZCB0byBiZSBzZWFyY2hhYmxlIGJ5IGNvbW1v biBjcml0ZXJpYS4KPj4+Cj4+PiAtU3RldmUKPgo+Cj4KCl9fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fCkNvbnRhaW5lcnMgbWFpbGluZyBsaXN0CkNvbnRhaW5l cnNAbGlzdHMubGludXgtZm91bmRhdGlvbi5vcmcKaHR0cHM6Ly9saXN0cy5saW51eGZvdW5kYXRp b24ub3JnL21haWxtYW4vbGlzdGluZm8vY29udGFpbmVycw== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754427AbeEUV5m (ORCPT ); Mon, 21 May 2018 17:57:42 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:43948 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754104AbeEUV5h (ORCPT ); Mon, 21 May 2018 17:57:37 -0400 Subject: Re: [PATCH] audit: add containerid support for IMA-audit To: Steve Grubb Cc: Richard Guy Briggs , Mimi Zohar , containers@lists.linux-foundation.org, Linux-Audit Mailing List , linux-integrity , LKML , paul@paul-moore.com References: <1520257393.10396.291.camel@linux.vnet.ibm.com> <2397631.78oLu0QVqb@x2> <21646a72-e782-e33a-9e75-5cc98b241f36@linux.vnet.ibm.com> <4015765.rtofcNpGHU@x2> From: Stefan Berger Date: Mon, 21 May 2018 17:57:29 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <4015765.rtofcNpGHU@x2> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-MW X-TM-AS-GCONF: 00 x-cbid: 18052121-8235-0000-0000-00000D8AC16A X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00009062; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000261; SDB=6.01035711; UDB=6.00529775; IPR=6.00814840; MB=3.00021229; MTD=3.00000008; XFM=3.00000015; UTC=2018-05-21 21:57:33 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18052121-8236-0000-0000-00004111C787 Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-05-21_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1805210255 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/21/2018 02:30 PM, Steve Grubb wrote: > Hello Stefan, > > On Monday, May 21, 2018 1:53:04 PM EDT Stefan Berger wrote: >> On 05/21/2018 12:58 PM, Steve Grubb wrote: >>> On Thursday, May 17, 2018 10:18:13 AM EDT Stefan Berger wrote: >>>>> audit_log_container_info() then releasing the local context. This >>>>> version of the record has additional concerns covered here: >>>>> https://github.com/linux-audit/audit-kernel/issues/52 >>>> Following the discussion there and the concern with breaking user space, >>>> how can we split up the AUDIT_INTEGRITY_RULE that is used in >>>> ima_audit_measurement() and ima_parse_rule(), without 'breaking user >>>> space'? >>>> >>>> A message produced by ima_parse_rule() looks like this here: >>>> >>>> type=INTEGRITY_RULE msg=audit(1526566213.870:305): action="dont_measure" >>>> fsmagic="0x9fa0" res=1 >>> Why is action and fsmagic being logged as untrusted strings? Untrusted >>> strings are used when an unprivileged user can affect the contents of the >>> field such as creating a file with space or special characters in the >>> name. >>> >>> Also, subject and object information is missing. Who loaded this rule? >>> >>>> in contrast to that an INTEGRITY_PCR record type: >>>> >>>> type=INTEGRITY_PCR msg=audit(1526566235.193:334): pid=1615 uid=0 auid=0 >>>> ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 >>>> op="invalid_pcr" cause="open_writers" comm="scp" >>>> name="/var/log/audit/audit.log" dev="dm-0" ino=1962625 res=1 >>> Why is op & cause being logged as an untrusted string? This also has >>> incomplete subject information. >> It's calling audit_log_string() in both cases: >> >> https://elixir.bootlin.com/linux/latest/source/security/integrity/integrity >> _audit.c#L48 > I see. Looking things over, I see that it seems like it should do the right > thing. But the original purpose for this function is here: > > https://elixir.bootlin.com/linux/latest/source/kernel/audit.c#L1944 > > This is where it is logging an untrusted string and has to decide to encode > it or just place it in quotes. If it has quotes, that means it's an untrusted > string but has no control characters in it. I think you want to use > audit_log_format() for any string that is trustworthy. Replacing all occurrences (in IMA) of audit_log_string() with audit_log_format(). > > > As an aside, I wonder why audit_log_string() is in the API when it is a > helper to audit_log_untrustedstring() ? Without understanding the rules of > untrusted strings, it can't be used correctly without re-inventing > audit_log_untrustedstring() by hand. > > >>>> Should some of the fields from INTEGRITY_PCR also appear in >>>> INTEGRITY_RULE? If so, which ones? >>> pid, uid, auid, tty, session, subj, comm, exe, res. <- these are >>> required to be searchable >>> >>>> We could probably refactor the current integrity_audit_message() and >>>> have ima_parse_rule() call into it to get those fields as well. I >>>> suppose adding new fields to it wouldn't be considered breaking user >>>> space? >>> The audit user space utilities pretty much expects those fields in that >>> order for any IMA originating events. You can add things like op or >>> cause before >> We will call into audit_log_task, which will put the parameters into >> correct order: >> >> auid uid gid ses subj pid comm exe > I'm telling you what the correct order is. :-) A long time ago, the IMA :-) Thanks. Was getting confused. > system had audit events with the order I'm mentioning. For example, here's > one from a log I collected back in 2013: > > type=INTEGRITY_PCR msg=audit(1327409021.813:21): pid=1 uid=0 auid=4294967295 > ses=4294967295 subj=kernel op="add_template_measure" cause="hash_added" > comm="init" name="01parse-kernel.sh" dev=rootfs ino=5413 res=0 > > it was missing "tty" and "exe", but the order is as I mentioned. The > expectation is that INTEGRITY events maintain this established order across > all events. I am *appending* exe= and tty= now: type=INTEGRITY_PCR msg=audit(1526939047.809:305): pid=1609 uid=0 auid=0 ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op="invalid_pcr" cause="open_writers" comm="ssh" name="/var/lib/sss/mc/passwd" dev="dm-0" ino=1962679 res=1 exe="/usr/bin/ssh" tty=tty2    Stefan > > Thanks, > -Steve > > >> https://elixir.bootlin.com/linux/latest/source/kernel/auditsc.c#L2433 >> >>> that. The reason why you can do that is those additional fields are not >>> required to be searchable by common criteria. >>> >>> -Steve > > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:54038 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754596AbeEUV5h (ORCPT ); Mon, 21 May 2018 17:57:37 -0400 Received: from pps.filterd (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w4LLmst3126433 for ; Mon, 21 May 2018 17:57:36 -0400 Received: from e31.co.us.ibm.com (e31.co.us.ibm.com [32.97.110.149]) by mx0a-001b2d01.pphosted.com with ESMTP id 2j44mfbwaf-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 21 May 2018 17:57:36 -0400 Received: from localhost by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 21 May 2018 15:57:35 -0600 Subject: Re: [PATCH] audit: add containerid support for IMA-audit To: Steve Grubb Cc: Richard Guy Briggs , Mimi Zohar , containers@lists.linux-foundation.org, Linux-Audit Mailing List , linux-integrity , LKML , paul@paul-moore.com References: <1520257393.10396.291.camel@linux.vnet.ibm.com> <2397631.78oLu0QVqb@x2> <21646a72-e782-e33a-9e75-5cc98b241f36@linux.vnet.ibm.com> <4015765.rtofcNpGHU@x2> From: Stefan Berger Date: Mon, 21 May 2018 17:57:29 -0400 MIME-Version: 1.0 In-Reply-To: <4015765.rtofcNpGHU@x2> Content-Type: text/plain; charset=utf-8; format=flowed Message-Id: Sender: linux-integrity-owner@vger.kernel.org List-ID: On 05/21/2018 02:30 PM, Steve Grubb wrote: > Hello Stefan, > > On Monday, May 21, 2018 1:53:04 PM EDT Stefan Berger wrote: >> On 05/21/2018 12:58 PM, Steve Grubb wrote: >>> On Thursday, May 17, 2018 10:18:13 AM EDT Stefan Berger wrote: >>>>> audit_log_container_info() then releasing the local context. This >>>>> version of the record has additional concerns covered here: >>>>> https://github.com/linux-audit/audit-kernel/issues/52 >>>> Following the discussion there and the concern with breaking user space, >>>> how can we split up the AUDIT_INTEGRITY_RULE that is used in >>>> ima_audit_measurement() and ima_parse_rule(), without 'breaking user >>>> space'? >>>> >>>> A message produced by ima_parse_rule() looks like this here: >>>> >>>> type=INTEGRITY_RULE msg=audit(1526566213.870:305): action="dont_measure" >>>> fsmagic="0x9fa0" res=1 >>> Why is action and fsmagic being logged as untrusted strings? Untrusted >>> strings are used when an unprivileged user can affect the contents of the >>> field such as creating a file with space or special characters in the >>> name. >>> >>> Also, subject and object information is missing. Who loaded this rule? >>> >>>> in contrast to that an INTEGRITY_PCR record type: >>>> >>>> type=INTEGRITY_PCR msg=audit(1526566235.193:334): pid=1615 uid=0 auid=0 >>>> ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 >>>> op="invalid_pcr" cause="open_writers" comm="scp" >>>> name="/var/log/audit/audit.log" dev="dm-0" ino=1962625 res=1 >>> Why is op & cause being logged as an untrusted string? This also has >>> incomplete subject information. >> It's calling audit_log_string() in both cases: >> >> https://elixir.bootlin.com/linux/latest/source/security/integrity/integrity >> _audit.c#L48 > I see. Looking things over, I see that it seems like it should do the right > thing. But the original purpose for this function is here: > > https://elixir.bootlin.com/linux/latest/source/kernel/audit.c#L1944 > > This is where it is logging an untrusted string and has to decide to encode > it or just place it in quotes. If it has quotes, that means it's an untrusted > string but has no control characters in it. I think you want to use > audit_log_format() for any string that is trustworthy. Replacing all occurrences (in IMA) of audit_log_string() with audit_log_format(). > > > As an aside, I wonder why audit_log_string() is in the API when it is a > helper to audit_log_untrustedstring() ? Without understanding the rules of > untrusted strings, it can't be used correctly without re-inventing > audit_log_untrustedstring() by hand. > > >>>> Should some of the fields from INTEGRITY_PCR also appear in >>>> INTEGRITY_RULE? If so, which ones? >>> pid, uid, auid, tty, session, subj, comm, exe, res. <- these are >>> required to be searchable >>> >>>> We could probably refactor the current integrity_audit_message() and >>>> have ima_parse_rule() call into it to get those fields as well. I >>>> suppose adding new fields to it wouldn't be considered breaking user >>>> space? >>> The audit user space utilities pretty much expects those fields in that >>> order for any IMA originating events. You can add things like op or >>> cause before >> We will call into audit_log_task, which will put the parameters into >> correct order: >> >> auid uid gid ses subj pid comm exe > I'm telling you what the correct order is. :-) A long time ago, the IMA :-) Thanks. Was getting confused. > system had audit events with the order I'm mentioning. For example, here's > one from a log I collected back in 2013: > > type=INTEGRITY_PCR msg=audit(1327409021.813:21): pid=1 uid=0 auid=4294967295 > ses=4294967295 subj=kernel op="add_template_measure" cause="hash_added" > comm="init" name="01parse-kernel.sh" dev=rootfs ino=5413 res=0 > > it was missing "tty" and "exe", but the order is as I mentioned. The > expectation is that INTEGRITY events maintain this established order across > all events. I am *appending* exe= and tty= now: type=INTEGRITY_PCR msg=audit(1526939047.809:305): pid=1609 uid=0 auid=0 ses=2 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op="invalid_pcr" cause="open_writers" comm="ssh" name="/var/lib/sss/mc/passwd" dev="dm-0" ino=1962679 res=1 exe="/usr/bin/ssh" tty=tty2 Stefan > > Thanks, > -Steve > > >> https://elixir.bootlin.com/linux/latest/source/kernel/auditsc.c#L2433 >> >>> that. The reason why you can do that is those additional fields are not >>> required to be searchable by common criteria. >>> >>> -Steve > > >