From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Paris Subject: Re: [Fwd: [PATCH][RFC] SMACK : add logging support V1] Date: Mon, 30 Mar 2009 15:31:28 -0400 Message-ID: <1238441488.18717.54.camel@localhost.localdomain> References: <49B3AB1A.8040201@numericable.fr> <49CDB485.5010809@schaufler-ca.com> <1238422822.16684.74.camel@localhost.localdomain> <49D10FC5.4060504@numericable.fr> Mime-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <49D10FC5.4060504@numericable.fr> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: Etienne Basset Cc: linux-audit@redhat.com, jmorris@namei.org List-Id: linux-audit@redhat.com On Mon, 2009-03-30 at 20:30 +0200, Etienne Basset wrote: > > I didn't look much at all in the SMACK code only the more generic audit > > code. Are you sure you got the capability no audit stuff right? I > > guess you would know it pretty quickly since capabilities for memory > > allocation are checked a LOT, but on OOM and FS full they are checked > > too. > > > I'm not quite sure what you're referring to. You mean a way to not log too much frequent events? SMACK doesn't have it's own capabilities enforcement (it just calls cap_capable()) so this doesn't matter to you at all. Just pretend I didn't say anything. > > You don't have to but I'd like to see some reworking if you are willing. > > Any code that you stole line for line from SELinux, lets put that stuff > > in a single place. I think that's going to mean pretty much all of > > smack_logging.h could be reasonably moved to a generic header for both > > of you to use (just make it generic, i'm not asking you to port selinux > > et al. to the new header.) > > OK, would it be something like include/linux/lsm_common_audit.h ? James? lsm_audit.h? security_audit.h? Should I just shut up and not try to work towards a generic lsm audit framework? I'm just seeing a lot of code that I've seen before. > > I also feel like we could do better reusing > > code from smack_log and avc_audit but maybe that's beyond what you are > > willing to try to to take on > > > > you mean doing : > smack:smack_log(...) > smack_audit_specific_data() > ... > common_security_audit(struct common_audit_data *) > > selinux:avc_audit(...) > selinux_specific_data() > ... > common_security_audit(struct common_audit_data *); > > It would make sense I guess I believe they both output the same set of information right? (except selinux has a tclass smack doesn't?) . But since we can't really fix/change SELinux logging right now maybe this isn't worth doing too much work over. If you see an easy way to make more of the code generic, do it. If not, don't worry about it. The real difference between the two is in security/selinux/avc.c::avc_dump_query() vs the beginning of smack_log(). If it's reasonable to somehow combine most/all of the other code, i say we do it. This might mean something like struct selinux_audit_struct { u32 ssid u32 tsid u16 tclass u32 requested struct av_decision *avd int result } struct smack_audit_struct { char *function int result char *subj char *obj char *requested } common_audit_struct { [everything we have today] union { struct selinux_audit_struct selinux_audit; struct smack_audit_struct smack_audit; } void (*lsm_audit)(struct audit_buffer*, void *); } So we could call some new generic function that does all of the buffer allocation and reporting, then call back into the private selinux/smack function with the data in the union. Maybe this isn't as easy as it sounded in my head... I just don't think we want a 3,4,5,6 implementations of lsm auditing. > > I pretty strongly detest %s these days. Using it on the left side of an > > = is ok if you are REALLY careful. Using it on the right makes me > > cringe. Can smack labels have characters which are not ascii letters > > (spaces?) > > > no, smack basically do the same tests when importing smack label than you do > in kernel/audit.c:audit_string_contains_control > except smack allows the '"' character How did you plan to handle a SMACK label with a ' ? Using the audit string functions and being given a label with a " is going to give you the hex output. (which might someday turn into better encoding, but I'm still waiting to see some code to do it better) Can I suggest if you write userspace tools to do anything with these audit records that you use libauparse? So if we do make changes, SMACK tools keep working (this is the main problem with changing how SELinux uses audit, the userspace tools don't use libauparse so we can't make changes in just the kernel+library...) -Eric