On Thu, Jun 13, 2019 at 02:00:29PM -0400, Stephen Smalley wrote:

On 6/11/19 6:55 PM, Xing, Cedric wrote:

quoted

*_noaudit() is exactly what I wanted. But I couldn't find
selinux_file_mprotect_noaudit()/file_has_perm_noaudit(), and I'm reluctant
to duplicate code. Any suggestions?

I would have no objection to adding _noaudit() variants of these, either
duplicating code (if sufficiently small/simple) or creating a common helper
with a bool audit flag that gets used for both. But the larger issue would
be to resolve how to ultimately ensure that a denial is audited later if the
denied permission is actually requested and blocked via sgxsec_mprotect().

I too would like to see a solution to the auditing issue.  I wrongly
assumed Cedric's approach (option #3) didn't suffer the same auditing
problem as Andy's dynamic tracking proposal (option #2).  After reading
through the code more carefully (trying to steal ideas to finish off an
implementation of #2), I've come to realize options #2 (Andy) and #3
(Cedric) are basically identical concepts, the only difference being who
tracks state.

We can use the f_security blob sizes to identify which LSM denied
something, but I haven't the faintest idea how to track the auditing
information in a sane fashion.  We'd basically have to do a deep copy on
struct common_audit_data, or pre-generate and store the audit message.
For SELinux, a deep copy is somewhat feasible because selinux_audit_data
distills everything down to basic types.  AppArmor on the other hand has
'struct aa_label *label', which at a glance all but requires pre-generating
the audit message, and since AppArmor logs denials from every profile, it's 
possible the "sgx audit blob" will consume a non-trivial amount of data.

Even if we figure out a way to store the audit messages without exploding
memory consumption or making things horrendously complex, we still have a
problem of reporting state info.  Any number of things could be removed or
modified by the time the audit is actually triggered, e.g. files removed,
AppArmor profiles modified, etc...  Any such change means we're logging
garbage.