Re: [PATCH 3rd revision] Add SELinux context support to AUDIT target
On Wed, Jun 8, 2011 at 2:13 PM, Casey Schaufler <casey(a)schaufler-ca.com> wrote:
On 6/8/2011 7:49 AM, Steve Grubb wrote:
> On Tuesday, June 07, 2011 06:32:35 AM Mr Dash Four wrote:
>> Add SELinux context support to AUDIT target - 3rd revision (style-type
>> changes made *only* since 2nd revision of this patch). Typical (raw
>> auditd) output after applying this patch would be:
> <snip>
>
>> @@ -163,6 +170,15 @@ audit_tg(struct sk_buff *skb, const struct
>> xt_action_param *par) break;
>> }
>>
>> +#ifdef CONFIG_NF_CONNTRACK_SECMARK
>> + if (skb->secmark) {
>> + if (!security_secid_to_secctx(skb->secmark, &secctx,
&len)) {
>> + audit_log_format(ab, " obj=%s", secctx);
>> + security_release_secctx(secctx, len);
>> + }
> else
> audit_log_format(ab, " osid=%u", skb->secmark);
>
> _All_ audit code records the number on a failed conversion.
But it really shouldn't. An unconvertible secid is indicative
of a serious, unrecoverable failure within the LSM. It's every
bit as bad as an invalid pointer.
I'm with Casey and Mr Dash Four. The right way to do this is to make
a helper function in the audit code which takes care of the LSM calls
(rather than making LSM calls up here). The function should throw an
error of one sort or another if it cannot convert the context. My
suggestion:
int audit_log_secctx(struct auditbuffer *ab, u32 secid)
{
int len, rc;
char *ctx;
rc = security_secid_to_secctx(sid, &ctx, &len);
if (rc) {
audit_panic("Cannot convert secid to context");
} else {
audit_log_format(ab, " subj=%s", ctx);
security_release_secctx(ctx, len);
}
return rc;
}
Such a function could be used a couple of places in the audit code itself.
-Eric