From mboxrd@z Thu Jan 1 00:00:00 1970 From: William Roberts Subject: [RFC] Comments on audit command line failure Date: Mon, 6 Jan 2014 19:38:02 -0800 Message-ID: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: "linux-audit@redhat.com" Cc: Richard Guy Briggs List-Id: linux-audit@redhat.com I've been doing some testing of the recent audit cmdline patches, notably as many as the error paths as I can. On a failure, the field is populated with null, like when key is null. However, it has quotes, should I drop the quotes... Example: Now: cmdline="(null)" key=(null) Proposed: cmdline=(null) key=(null) I noticed that tty if its null also does not have quotes. -- Respectfully, William C Roberts From mboxrd@z Thu Jan 1 00:00:00 1970 From: William Roberts Subject: Re: [RFC] Comments on audit command line failure Date: Tue, 7 Jan 2014 07:54:11 -0800 Message-ID: References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8793821215880547828==" Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: linux-audit@redhat.com Cc: Richard Guy Briggs List-Id: linux-audit@redhat.com --===============8793821215880547828== Content-Type: multipart/alternative; boundary=047d7bf0d1042b95c604ef6363a5 --047d7bf0d1042b95c604ef6363a5 Content-Type: text/plain; charset=ISO-8859-1 On Jan 6, 2014 7:38 PM, "William Roberts" wrote: > > I've been doing some testing of the recent audit cmdline patches, > notably as many as the error paths as I can. > > On a failure, the field is populated with null, like when key is null. > However, it has quotes, should I drop the quotes... > > Example: > > Now: > cmdline="(null)" key=(null) > > Proposed: > cmdline=(null) key=(null) > > > I noticed that tty if its null also does not have quotes. Also, I was ensuring that the end of cmdline was null terminated before adding it to the audit buffer. That code is something like: buf[res-1]=\0 Suppose buf is just a single character. Its within bounds but now gets clobbered with a null byte? Should I handle this case? --047d7bf0d1042b95c604ef6363a5 Content-Type: text/html; charset=ISO-8859-1


On Jan 6, 2014 7:38 PM, "William Roberts" <bill.c.roberts@gmail.com> wrote:
>
> I've been doing some testing of the recent audit cmdline patches,
> notably as many as the error paths as I can.
>
> On a failure, the field is populated with null, like when key is null.
> However, it has quotes, should I drop the quotes...
>
> Example:
>
> Now:
> cmdline="(null)" key=(null)
>
> Proposed:
> cmdline=(null) key=(null)
>
>
> I noticed that tty if its null also does not have quotes.

Also, I was ensuring that the end of cmdline was null terminated before adding it to the audit buffer. That code is something like:
buf[res-1]=\0

Suppose buf is just a single character. Its within bounds but now gets clobbered with a null byte? Should I handle this case?

--047d7bf0d1042b95c604ef6363a5-- --===============8793821215880547828== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline --===============8793821215880547828==-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Grubb Subject: Re: [RFC] Comments on audit command line failure Date: Tue, 07 Jan 2014 11:34:23 -0500 Message-ID: <1653590.FkWMjMVmQ0@localhost.localdomain> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from localhost.localdomain (vpn-56-178.rdu2.redhat.com [10.10.56.178]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id s07GYN3C025759 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 7 Jan 2014 11:34:24 -0500 In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: linux-audit@redhat.com List-Id: linux-audit@redhat.com On Monday, January 06, 2014 07:38:02 PM William Roberts wrote: > I've been doing some testing of the recent audit cmdline patches, > notably as many as the error paths as I can. > > On a failure, the field is populated with null, like when key is null. But (null) for a key field is normal rather than a failure. > However, it has quotes, should I drop the quotes... > > Example: > > Now: > cmdline="(null)" key=(null) > > Proposed: > cmdline=(null) key=(null) The audit event format cannot change. EVER! If it has been changed due some patches, it must be changed back as fast as possible. Tools parse the log files and any format change can cause something important to be missed. Even the order of fields is important because some tools skip around taking advantage of the order to speed searches. So, the correct thing is to make sure events are the same before and after the patches. > I noticed that tty if its null also does not have quotes. Quotes are only used when user space has influenced the value. We can't allow a crafty user/admin to setup fields that will cause a parsing error that hides and event from tools. -Steve From mboxrd@z Thu Jan 1 00:00:00 1970 From: William Roberts Subject: Re: [RFC] Comments on audit command line failure Date: Tue, 7 Jan 2014 11:36:04 -0800 Message-ID: References: <1653590.FkWMjMVmQ0@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1653590.FkWMjMVmQ0@localhost.localdomain> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Steve Grubb Cc: "linux-audit@redhat.com" List-Id: linux-audit@redhat.com On Tue, Jan 7, 2014 at 8:34 AM, Steve Grubb wrote: > On Monday, January 06, 2014 07:38:02 PM William Roberts wrote: >> I've been doing some testing of the recent audit cmdline patches, >> notably as many as the error paths as I can. >> >> On a failure, the field is populated with null, like when key is null. > > But (null) for a key field is normal rather than a failure. > > >> However, it has quotes, should I drop the quotes... >> >> Example: >> >> Now: >> cmdline="(null)" key=(null) >> >> Proposed: >> cmdline=(null) key=(null) > > The audit event format cannot change. EVER! If it has been changed due some > patches, it must be changed back as fast as possible. I doubt the audit event format has stayed 100% static since it's inception, so i'm inclined to disagree with you there. I have also posted the patches numerous times, and am working on LKML as well as this list to get all the relevant parts merged. As of right now nothing has been merged. Tools parse the log files > and any format change can cause something important to be missed. Even the > order of fields is important because some tools skip around taking advantage of > the order to speed searches. > > So, the correct thing is to make sure events are the same before and after the > patches. Not with what I am doing. > > >> I noticed that tty if its null also does not have quotes. > > Quotes are only used when user space has influenced the value. We can't allow a ok I see that quotes are added as part of audit_log_untrustedstring(). > crafty user/admin to setup fields that will cause a parsing error that hides > and event from tools. > > -Steve > What I think I can take away here is this: 1. Any changes to the audit record should append, rather then change orderings 2. The quotes were actually as designed Any userspace tool that cannot handle the addition of a field in my mind would be broken. All of the tools I have been using so far just ignore the extra field and work fine. Bill