On Tuesday, November 22, 2016 9:55:11 AM EST Stephen Smalley wrote: > On 11/22/2016 09:28 AM, Steve Grubb wrote: > > On Tuesday, November 22, 2016 8:56:57 AM EST Stephen Smalley wrote: > >> On 11/21/2016 04:50 PM, Paul Moore wrote: > >>> On Mon, Nov 21, 2016 at 12:30 PM, Steve Grubb <sgrubb@xxxxxxxxxx> wrote: > >>>> The AUDIT_MAC_POLICY_LOAD event has dangling text that means the same > >>>> thing as the event type and is missing the uid and results field. The > >>>> bigger issue is that in some failure cases no event is emitted. This > >>>> patch fixes the noted problems. > >> > >> A potential problem with this patch is that it changes the semantic > >> meaning of this audit record, from meaning "a policy was loaded into the > >> kernel" to "there was an attempt to load a policy, check the res= field > >> to determine whether it succeeded". So anything in userspace that used > >> the presence of this audit record to determine whether or not policy was > >> successfully loaded (e.g. audit2allow -l) will be confused. > > > > I really can't have implicit success. I need to have a field to point to > > that says yes/no. It can be hard coded to res=1 (success), but it needs > > to be there. > > Ok. Why is it you use res=1|0 in these records but success=yes|no in > syscall records? Success is only used in syscall records. It was created to disambiguate the exit field which can look like a failure on some arches but is actually OK. Originally the exit field was all that existed. We found that one or two arches actually have 2 return codes. The whole rest of the audit system uses res= to mean a crisp yes/no this did or didn't happen. This predates the creation of the success field. It could be argued success should have been res, but ausearch/report will use either to mean the same thing. The new auparse field classifier work will also map both to mean the same thing. That is how I found AUDIT_MAC_POLICY_LOAD missing any kind of success/fail indication. > >> While there were failure cases that would still generate the audit record > >> previously, those were all selinuxfs node creation failures; the policy > >> would nonetheless have been loaded into the kernel and would be active > >> at that point, so saying res=0 is somewhat misleading. > > > > OK. We can move the point where res=1 is set. But I would think that its a > > requirement to have an audit record that states that policy failed to > > load. > > FMT_MSA.3 Static Attribute Initialization. Auditable events: All > > modifications of the initial value of security attributes. I would think > > this means changes such as booleans, modifying labels, loading a new > > policy, or failure to load a policy. > > Failure to load a policy is not a modification to the initial value of > the security attribute, is it? Sure. If the state is intended to enabled and enforcing and its not, that would be a surprising result that there was no indication in the audit trail. Also, if for some reason it booted fine and a new policy was loaded at some point in the future and it failed, then we have a modification to initial state. > >> This overlapswith > >> https://github.com/SELinuxProject/selinux-kernel/issues/1, which > >> highlights the fact that we can end up in an intermediate state where > >> policy is loaded but selinuxfs (particularly booleans, class/*, and > >> policy_capabilities/*) has not been regenerated. > > > > I see. That should be an audited event. If you have a datacenter with a > > thousand machines, its best to get this in the audit trail so it can be > > alerted on at a central collector. > > > > So, what should we do about the patch? I'm willing to modify it. > > At present, we only generate AUDIT_MAC_STATUS, AUDIT_MAC_LOAD, and > AUDIT_MAC_CONFIG_CHANGE on success (or at least partial success). If > you truly need to audit failures, then it seems like you either need to > a) do it through syscall audit filters, which already provide a success= > field I can't imagine what to audit on. There is an open syscall that has a path. But I suspect that does not fail because policy has not be written. There is a write syscall but triggering on that is pretty generic. This is not ideal. > or b) add new audit message types for this purpose (e.g. > AUDIT_MAC_STATUS_FAIL, AUDIT_MAC_LOAD_FAIL, ...). To just add a res= > field to the existing ones and change them to always be generated is a > user-visible semantic change. OK. This is do-able. I'll hard code the 'res' field for each of them. -Steve _______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
