Re: [PATCH ALT4 V3 1/2] audit: show fstype:pathname for entries with anonymous parents
On Thursday, November 9, 2017 3:52:46 PM EST Richard Guy Briggs wrote:
> >> > It might be simplest to just apply a corrective
patch over top of
> >> > this one so that you don't have to muck about with git branches
and
> >> > commit messages.
> >>
> >> A quick note on the "corrective patch": given we are just days
away
> >> from the merge window opening, it is *way* to late for something like
> >> that, at this point the only options are to leave it as-is or
> >> yank/revert and make another pass during the next development phase.
> >
> > Then yank it. I think that is overreacting but given the options you
> > presented its the only one that avoids changing a critical field
> > format.
>
> It's not overreacting Steve, there is simply no way we can test and
> adequately soak new changes in the few days we have left.
Its just moving the output of the information a few lines down further in the
code. 10 minutes of work, tops.
> Event yanks/reverts carry a risk at this stage, but I consider
that the
> less risky option for these patches. Neither is a great option, and that
> is why I'm rather annoyed.
I don't really see that this is my choice to include it or not. This is
the upstream maintainer's decision.
I can't say I'd be thrilled to have my name on something that stuffs up
the system though. It still isn't clear to me why an incomplete path
from some seemingly random place in the filesystem tree is preferable to
something that gives it an anchor point, at least to human interpreters.
The path should stay. Just the file system type needs decoupling and moving.
Adding an fstype to the record is an interesting idea, but then
creates
a void for all the rest of the properly formed records that don't need
it and will need more work to find it, wasting bandwidth with
"fstype=?".
I would let it optionally swing in and out at the end of the record. This
should never show up on a normal system because the rules will suppress
generating this information by default. So, it really won't be all that
visible.
How are the analysis tools stymied by a text prefix to a path that it
can't
find anyways?
Because they do not actually resolve anything in the file system. They take
the event as ground truth and use that. Also, the tools expect name=value.
This has been the way since the beginning. We do not lump multiple independent
values together. And then what if the path has a special character in it? The
whole value then has to be encoded. And I don't think the patch is using
untrusted string like it should.
Since we have a chance to fix it before it goes upstream, I think it
should either be yanked and respun, or add a corrective patch and submit
them together.
> >> As for the objection itself: ungh. There is really no good reason why
> >> you couldn't have seen this in the *several* *months* prior to this;
> >> Richard wrote a nice patch description which *included* sample audit
> >> events, and you were involved in discussions regarding this patchset.
> >> To say I'm disappointed would be an understatement.
> >
> > I am also disappointed to find that we are modifying a searchable field
> > that has been defined since 2005. The "name" field is very
important.
> > It's used in quite a few reports, its used in the text format, it's
> > searchable, and we have a dictionary that defines exactly what it is.
> > Fields that are searchable and used in common reports cannot be changed
> > without a whole lot of coordination. I'm also disappointed to have to
> > point out that new information should go in its own field. I thought
> > this was common knowledge. In any event, it was caught and problems can
> > be avoided.
So why does this make it unsearchable?
I didn't say it makes in unsearchable, but now that you mention it...it does
in one case. Searchable fields are more important. They typically are the
object or subject or some kind of special attribute that is commonly searched
on to group events. Searchable fields can either be partial or full word
match. By combining information in the same field, it will change this
behavior. The path name is the object of the event. By combining information
that is not the object with it, everyone will have to change and update their
software to handle this change in behavior.
I still don't understand any explanations that have been made so
far
Try ausearch --text on those events, or aureport --file.
-Steve