On Fri, 2005-11-25 at 09:35 +0000, Tim Waugh wrote:
Forwarding this to a wider SELinux-knowledgeable audience. James
had
asked me what patches we apply to findutils, so I mentioned SELinux
and pointed him to the patch for an initial review.
IIRC, the findutils SELinux patch was originally created by a person
from MITRE who is no longer involved in the project. Dan later rewrote
it for the new SELinux API in Linux 2.6.
1. I notice that SELinux-enablement creates two new tests,
"-context"
and "--context". I would eliminate "--context", leaving only
"-context". Find predicates aren't GNU-style long options. Find
is confusing enough with options as well as predicates, and so I
definitely don't want to further blur that line.
This suggestion seems fine.
2. I see that if you use "find -context" on an
SELinux-enabled version
of find on a system where SELinux is not enabled, find will
immediately perform a fatal exit with an explanatory message. I
won't say that that is definitely the wrong behaviour, but why did
you choose not to have -context simply always return false (and
perhaps issue one warning message)?
This may be a legacy issue. Since getfilecon is implemented via
getxattr(2), and getxattr(2) does not require SELinux to be enabled, we
might want to drop this restriction. The older SELinux API (before
migrating to using xattrs) would only have worked if SELinux was
enabled.
3. Security contexts are long multi-part strings. Matching with
glob
patterns is a reasonable approach. Nevertheless, does the use of
glob patterns to match them reflect practice on other systems?
I'm not sure what we would compare against in terms of other systems
(Trusted Solaris?), or exactly how they would compare. Some kind of
pattern matching (regex or glob patterns) seems necessary to deal with
searching for e.g. a particular TE type without being concerned about
other fields.
4. I notice that your patch slips in a definition for FNM_CASEFOLD.
That looks wrong to me. gl_FUNC_FNMATCH_GNU should call
AC_GNU_SOURCE to ensure that <fnmatch.h> #defines FNM_CASEFOLD for
you. This should be called by gl_INIT. Is this (a) not working
correctly on some of your build systems, (b) not working on any of
your build systems, implying a findutils bug, or (c) there for
'historical reasons'?
I don't know - looks like you added these lines earlier this year upon
merging 4.2.15.
5. Findutils should probably default to compiling with SELinux
support
on systems where <selinux/selinux.h> exists or --with-selinux is
specifified to configure, and compile without such support
otherwise.
Seems reasonable.
6. Is there a reason you turn off Automake gnits rules checking in
the
SELinux patch?
No ideas here - looks like a change by Dan very recently from the cvs
log.
7. The configure magic for selinux can be fully generic and
self-contained I think: we could just have an selinux.m4 file.
That would also make it much easier to reuse it.
Seems fine.
8. Are the DejaGnu test cases for the SELinux patch in another patch
file somewhere else? Perhaps it's in the same patch file as the
update to the Texinfo documentation. :)
Likely doesn't exist yet.
--
Stephen Smalley
National Security Agency