Re: [Fwd: [PATCH][RFC] SMACK : add logging support V1]
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