On 10/08/2012 03:03 PM, Reshetova, Elena wrote:
Hi,
Hi,
Following up our recent discussion, I have composed a first patch in order
not to talk about the abstract concepts only.
I have taken the less intrusive approach and tried to integrate the new
hooks and the notion of "security plugin"
into the code with least amount of changes for core rpm and for existing
collection hooks. This would also allow to leave the existing
SELinux collection hooks as they are and only move the integrated SElinux
functionality from rpm to the hooks.
Note that the existing hooks are by no means "sacred" for other than
documenting the needs of the SELinux collection. AFAIK the collections
aren't actually being used anywhere yet (its still marked experimental),
and the interfaces are internal-only so if there's something that
needs/wants to be changed, they can be changed.
Please note that this patch is very rough and I would like to post it only
for the purpose of asking if this approach is valid.
There are a number of decisions that I had to make about this patch that are
worth discussing:
- The patch defines a new type of rpm plugin, called security plugin and add
a rpmpluginsAddSecurityPlugin() function to initialize it.
This is done in the similar style as rpmpluginsAddCollectionPlugin()
function. The top level function that uses it is rpmteSetupSecurityPlugins()
from transaction.c. The main difference from the collection plugin case is
that this function can automatically discover the security plugins
that should be loaded based on the %__security_plugins macro from macros.in
that contains comma separated list of security plugins to be loaded.
I don't know how viable this approach is, but it seemed for me the easiest
way to make the plugins to be configured.
Macros the means through which rpm is generally configured, and that
kind of approach should make it reasonably simple to allow selective
enabling/disabling of them from cli (annoying but inevitable necessity).
Might be more annoying for other API users - currently disabling SELinux
is a simple matter of setting RPMTRANS_FLAG_NOCONTEXTS flag for the
transaction, with plugins it would be something ... quite different.
-Due to necessity of calling the plugins hooks with the plugin struct that
is stored in ts, some rpm functions have to pass the rpmts struct.
In addition since the ts->plugins is the actual needed parameter,
ts_internal.h has to be exposed to couple of more .c file. I find it to be
quite
ugly, but could not figure out the better way with the current plugin
definition. I was especially annoyed by the script functions that now have
to pass ts, too.
Any suggestion on this?
I think this was one of the prime reasons I had that look into the
"security manager" object thingie: to avoid having to pass the
transaction set to places where it really does not belong to.
OTOH it would seem to me (most of) the places could simply be passed a
handle to the plugins instead of ts, as the ts is pretty much only used
for ts->plugins.
- Instead of making a loop for each plugin hook to be called for each
plugin, I am iterating by the plugins and calling the hook for each plugin
inside the rpmpluginsCallxxx() functions. IMO it makes the hook code
scattered through the rpm cleaner, but this is just my opinion.
...and this is related to what was one of the issues the "security
manager" thing ran into. Assuming there can be more than one present:
how to meaningfully deal with errors? Returning an error if one or more
of them failed is a simple answer, but whether that's sufficient for the
callee(s) I dont know, I seem to recall some things wanting a better
idea of what failed and how (but that might well be just from staring
too much at the way things are currently done)
I haven't read through the patch too carefully, so just a few comments
on things that I happened to notice (on top of the issues/questions
already raised)
+
+ /* Run file updated hook for all plugins */
+ rc = rpmpluginsCallFileUpdated(ts->plugins, fsm->path, fsm->buf,
len);
Hmm. Looking at the MSM plugin (from
git://github.com/ereshetova/rpm.git), this is primarily used for
calculating yet another digest for the file being processed. This should
be avoidable, the rpmio FD_t type supports calculating more than one on
the digest on the fly, only the interfaces to use it are currently
buried in rpmio/rpmio_internal.h (but not actually hidden from ABI).
rc = fsmSetSELabel(sehandle, dn, mode);
-
+ if (!rc)
+ rc = rpmpluginsCallDirCreated(ts->plugins, dn,
mode);
This is a pretty ugly special case hack :) Part of the reason for
fsmSetSELabel() was to isolate the whole labeling thing into something
fairly generic that could be as well in a plugin. So this should be more
of a "post file/dir-creation hook" I think . Also access to such a thing
(or a way to avoid the need there completely...) would be needed in
rpmdbMoveDatabase() too.
if (xx == 0) {
- xx = execv(argv[0], argv);
+ /* Run script exec hook for all plugins */
+ if (rpmpluginsCallScriptExec(ts->plugins, argv) !=
RPMRC_FAIL) {
+ xx = execv(argv[0], argv);
+ }
Ah, this is one of the places where I remember pondering over with the
"security manager" experiment: my idea was to bury the execv() call
itself into the manager so it could be execv(), rpm_execcon() or
whatever depending on the manager type/what's loaded, but that didn't
fly at all :) This seems like a much more feasible approach, but the
hook name wants clarification as the hook MUST NOT actually perform the
exec, only setup any preliminaries it wants for the exec (eg
setexeccon() in case of selinux)
@@ -409,6 +411,9 @@ static void handleInstInstalledFile(const rpmts ts,
rpmte p, rpmfi fi, int fx,
if (rpmtsFilterFlags(ts) & RPMPROB_FILTER_REPLACEOLDFILES)
rConflicts = 0;
+ /* run file conflict hook for all plugins */
+ rpmpluginsCallFileConflict(ts->plugins, ts, p, fi, otherFi,
otherHeader);
+
This wont work in rpm >= 4.10 which no longer uses the rpmfi
"self-iterator" here - random access to the file info sets is needed and
it can even end up accessing the same file info set at two places
simultaneously (its possible for a package to have file conflicts within
itself via directory symlinks). So you'd have to actually pass indexes
for both fi and otherFi, but then the indexed accessors to rpmfi aren't
exported in the API currently and...
Overall I'm still not really comfortable of letting plugins mess around
with such a core functionality of rpm (I do remember the previous
discussion around this though)
- Panu -
_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint
