On 5/10/2021 4:52 PM, Paul Moore wrote:

On Mon, May 10, 2021 at 12:30 PM Casey Schaufler [off-list ref] wrote:

quoted

On 5/7/2021 6:54 PM, Richard Guy Briggs wrote:

quoted

On 2021-05-07 14:03, Casey Schaufler wrote:

quoted

On 5/7/2021 12:55 PM, Richard Guy Briggs wrote:

quoted

The *setxattr syscalls take 5 arguments.  The SYSCALL record only lists
four arguments and only lists pointers of string values.  The xattr name
string, value string and flags (5th arg) are needed by audit given the
syscall's main purpose.

Add the auxiliary record AUDIT_XATTR (1336) to record the details not
available in the SYSCALL record including the name string, value string
and flags.

Notes about field names:
- name is too generic, use xattr precedent from ima
- val is already generic value field name
- flags used by mmap, xflags new name

Sample event with new record:
type=PROCTITLE msg=audit(05/07/2021 12:58:42.176:189) : proctitle=filecap /tmp/ls dac_override
type=PATH msg=audit(05/07/2021 12:58:42.176:189) : item=0 name=(null) inode=25 dev=00:1e mode=file,755 ouid=root ogid=root rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=NORMAL cap_fp=none cap_fi=none cap_fe=0 cap_fver=0 cap_frootid=0
type=CWD msg=audit(05/07/2021 12:58:42.176:189) : cwd=/root
type=XATTR msg=audit(05/07/2021 12:58:42.176:189) : xattr="security.capability" val=01 xflags=0x0

Would it be sensible to break out the namespace from the attribute?

     attrspace="security" attrname="capability"

Do xattrs always follow this nomenclature?  Or only the ones we care
about?

Xattrs always have a namespace (man 7 xattr) of "user", "trusted",
"system" or "security". It's possible that additional namespaces will
be created in the future, although it seems unlikely given that only
"security" is widely used today.

Why should audit care about separating the name into two distinct
fields, e.g. "attrspace" and "attrname", instead of just a single
"xattr" field with a value that follows the "namespace.attribute"
format that is commonly seen by userspace?

I asked if it would be sensible. I don't much care myself.

quoted

quoted

quoted

Why isn't val= quoted?

Good question.  I guessed it should have been since it used
audit_log_untrustedstring(), but even the raw output is unquoted unless
it was converted by auditd to unquoted before being stored to disk due
to nothing offensive found in it since audit_log_n_string() does add
quotes.  (hmmm, bit of a run-on sentence there...)

quoted

The attribute value can be a .jpg or worse. I could even see it being an eBPF
program (although That Would Be Wrong) so including it in an audit record could
be a bit of a problem.

In these cases it would almost certainly get caught by the control
character test audit_string_contains_control() in
audit_log_n_untrustedstring() called from audit_log_untrustedstring()
and deliver it as hex.

In that case I'm more concerned with the potential size than with
quoting. One of original use cases proposed for xattrs (back in the
SGI Irix days) was to attach a bitmap to be used as the icon in file
browsers as an xattr. Another was to attach the build instructions
and source used to create a binary. None of that is information you'd
want to see in a audit record. On the other hand, if the xattr was an
eBPF program used to make access control decisions, you would want at
least a reference to it in the audit record.

It would be interesting to see how this code would handle arbitrarily
large xattr values, or at the very least large enough values to blow
up the audit record size.

As pointed out elsewhere in this thread, and brought up again above
(albeit indirectly), I'm guessing we don't really care about *all*
xattrs, just the "special" xattrs that are security relevant, in which
case I think we need to reconsider how we collect this data.

Right. And you can't know in advance which xattrs are relevant in the
face of "security=". We might want something like

	bool security_auditable_attribute(struct xattr *xattr)

which returns true if the passed xattr is one that an LSM in the stack
considers relevant. Much like security_ismaclabel(). I don't think that
we can just reuse security_ismaclabel() because there are xattrs that
are security relevant but are not MAC labels. SMACK64TRANSMUTE is one.
File capability sets are another. I also suggest passing the struct xattr
rather than the name because it seems reasonable that an LSM might
consider "user.execbyroot=never" relevant while "user.execbyroot=possible"
isn't.

I think it would also be very good to see what requirements may be
driving this work.  Is it purely a "gee, this seems important" or is
it a hard requirement in a security certification for example.