From mboxrd@z Thu Jan 1 00:00:00 1970 From: Richard Guy Briggs Subject: Re: [PATCH] audit: restore AUDIT_LOGINUID unset ABI Date: Fri, 12 Dec 2014 00:19:44 -0500 Message-ID: <20141212051944.GM29175@madcap2.tricolour.ca> References: <12023790.4atqQrk5y0@sifl> <20141209163014.GA29998@madcap2.tricolour.ca> <3178834.SIVRIbvQQR@sifl> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <3178834.SIVRIbvQQR@sifl> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Paul Moore Cc: linux-audit@redhat.com, ebiederm@xmission.com List-Id: linux-audit@redhat.com On 14/12/09, Paul Moore wrote: > On Tuesday, December 09, 2014 11:30:14 AM Richard Guy Briggs wrote: > > On 14/12/08, Paul Moore wrote: > > > As I understand it, when old userspace would set a filter with > > > AUDIT_LOGINUID but when it listed the audit rules in the kernel it would > > > see AUDIT_LOGINUID_SET, yes? This patch attempts to fix this by marking a > > > legacy userspace with the AUDIT_LOGINUID_LEGACY bitmask on the internal > > > kernel representation so that when the rules are dumped to userspace the > > > AUDIT_LOGINUID_SET rule can be rewritten as AUDIT_LOGINUID, yes? > > > > Correct. > > > > > However, there are some things that are not immediately obvious to me: > > > > > > * Why are we using a bit in audit_field->type to indicate the legacy > > > nature of userspace? > > > > Convenience. Adding a new member to audit_field or audit_krule seemed > > unnecessary memory overhead (however, it then complicates other code...). > > > > > * Why are we reusing the AUDIT_NEGATE bit in the type field to indicate a > > > legacy userspace? > > > > It wasn't reaped when commit 18900909 went through... (first introduced > > with original audit in b7b0074c, 2004-04-11). It would have been more > > clear if I had sent a first patch to remove AUDIT_NEGATE altogether and > > re-introduce it with a new name in this patch. > > The problem is that AUDIT_NEGATE lives in the userspace visible header file > which means it needs to live there for pretty much forever. While I would > like to see us remote it for clarity's sake, I think we're stuck with it. Ok, fair enough, the same goes for AUDIT_{LIST,ADD,DEL} since they are no longer used, but still remain in the API. I can use another value. > > > * Why are we not using something in audit_krule? Without looking to in > > > depth it would appear that there are multiple fields which might be > > > useful, e.g. "vers_ops", "flags"? > > > > audit_krule applies to the set of all fields for this rule. I wanted > > something that localized it very unambiguously to this one field. > > You can only add or delete rules, right? Not modify? If you can only add or > delete a rule, then if one of the fields in that rule is sent from legacy > userspace I think it is safe to set an indicator in one of the audit_krule > fields. I understand your point, but I'm not sure it is something to worry > too much about; I'd rather see the legacy indicator here than in the > audit_field->type field where we might have to contend with userspace usage at > some point. > > I'd like to explore the idea of not using audit_field->type; I picked > "vers_ops" and "flags" since they seemed like reasonable places to start. The > "vers_ops" field in particular appears to be almost unused in the current code > and it seems like a good way to track userspace versions perhaps, e.g. 1 = > legacy, 2 = now current, etc.? I'm curious if this sounds reasonable to you. vers_ops appears to be assigned, copied and never otherwise read. And in fact *that* should have been removed with commit 18900909. I'll just prepare a patch to rip it out. flags looks like a better choice... and I'll have to do some similar filtering that I did for type and as has been already done for AUDIT_FILTER_PREPEND... In fact, the inverse of AUDIT_FILTER_PREPEND. And this patch is simpler than the one you are critiquing. Bonus. > paul moore - RGB -- Richard Guy Briggs Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat Remote, Ottawa, Canada Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545