[Rpm-maint] Plugin ponderings

[Reshetova, Elena]

>Like said in some earlier email, there's not much hope getting the details
100% right the first time. The best way of finding out what works and what
doesn't is to try it out, so...
>I started looking into creating a syslog plugin for rpm. One could argue
that logging is fundamental enough to be in rpm core, but then having it in
a plugin allows nicely for alternatives (such as *gasp* native systemd
journal support) and is a nice and harmless way to test the plugin system.
Lets just say I ran >into issues real fast. No fingerpointing implied - I
didn't catch these on review either and some of the issues are direct
consequences from my own suggestions, doh :)

Yes, I have warned that I am a security person, nothing more :) So my head
is full of security use cases which are unfortunately far from covering all
potential use cases :( I think I will keep learning a lot about package
management for a long while still!

>For logging installs and erasures, one of the primary interests is
PLUGINHOOK_PSM_POST. As it is now, the hook doesn't get called if the
package failed to install, which is a case you'll certainly want to log:
>
>             /* Run post transaction element hook for all plugins */
>             if (!rc) rc = rpmpluginsCallPsmPost(ts->plugins, te);

>The other problem here is that the PSM_POST hook doesn't get passed the
result code of the install (otoh it doesn't get called on failure to begin
with so...). One could get the status from rpmteFailed(), but that element
failure is set only after rpmpsmRun() returns so its not available here.
>Similar issues are present in the TSM hooks: TSM_POST doesn't get called on
some error situations (file conflicts etc) where TSM_PRE is called, and the
transaction result code is not passed to plugins.

Yes, I guess it would make sense to make them all symmetric, like we are now
trying to get for scripts. And in this case of course return code is must. 

>Me thinks we seriously need to define a consistent convention for all
plugin hooks and then scrutinize the existing and future hook paths for
correctness in all possible situations.
Fully agree, just it will be probably also work in progress constantly as
getting hooks right :)

>I'd think part of the convention needs to be that whenever PRE-hooks get
called, the pairing POST hooks are guaranteed to get called too. Even if
some of the PRE-hooks returned errors: some plugin(s) might return an error
from its PRE-hook, but there could be other plugins which succeeded in their
own >business and might have allocated resources they need to free up in
POST.
Makes sense. 

>Then there's the issue of return codes: maybe all the POST hooks should
take an added 'rc' argument for the return code of the actual operation. 
I think this is good, we would just need to define for plugins what return
code means. Wonder if we can do it generic enough after all without going to
define a bunch of "InstallationSucededWithWarningsType1",
"InstallationSucededWithWarningsType2", "InstallationFailedReasonA",
"InstallationFailedReasonB",.... Like to tell that this hook will always get
RPMRC_OK, if package is installed (somehow, even with warnings) and FAIL, if
installation badly failed and package traces aren't present in filesystem. 
>That seems simple enough in principle, but there be dragons in the details:

>For example rpmpsmRun() returns RPMRC_FAIL only if the package didn't get
installed (or removed) at all. %pre and %preun scriptlets can prevent
install/erase from taking place, %post and %postun cannot (as the operations
cannot be rolled back) so RPMRC_OK is returned from
>rpmpsmRun() even if they fail: basically the operation succeeded with
warnings. This semantic needs to be taken into account with plugin hook
results too, so rpmpluginsCallPsmPost() can't really cause RPMRC_FAIL to be
returned from rpmpsmRun(). Or we need to revamp the whole damn return code
>system (which might well be the sanest option really, especially since its
just an internal API)
>Similar quirks are present in rpmtsRun() return codes, but that's a more
difficult case as its part of the public API (which everybody loves to hate
because of the nutty and useless return codes :)
>Another question is whether rpmRC codes are sufficient for the task. 
>It's used for a lot of things in rpm, but many of those uses are ugly
abuses in reality.

Return codes are really hard. Suppose we define some new ones for plugin
purposes, so that hooks can return more sane codes and potentially indicate
even type of failure. I would love to see some kind of security failure as
return code in this case and treat it more seriously :) But then for hooks
like rpmpluginsCallPsmPost() it doesn't matter anyway like you said: we
can't revert installation at this point, so even if we return
"EverythingIsReallyReallyBad", what can rpm do? I have noticed this at the
beginning when our initial patch was labelling the filesystem in
rpmpluginsCallPsmPost(), which made me think what if labelling fails at that
point? Plugin can return whatever, but it is too late to do anything about
it.  Actually this problem keeps me occupied for FileClosed() hook: file has
been already placed on filesystem at that time, we want to setup a nice
security context on it, but lsetattr() fails, what are the next steps apart
from panic? There is no way to scratch that file out of filesystem, so
initial labelling has to be secure by default (not usable for that file
probably, but secure). This brings to the fact that when file is written to
the filesystem, it should be placed with some kind of "Isolated label" or
isolated security context, meaning that we need to change rpm process
security context to have this isolated context/label, place the file, change
the label of rpm process back to rpm and then call the closed hook to label
the file. At least two more hooks :( But from security point of view, setup
will be safe: if labelling fails, then at least file is labelled with
Isolated label and can't harm. However, again, returning failure or any
other code that "OK" from FileClosed() hook doesn't make much sense.
All of this just to say that I am not sure if new return codes will add so
much value...

>P.S. I certainly dont mean you need to do all this, I'll help combing the
call- and error paths for correctness once we figure out what "correct"
actually is.
I am not against working with this at all! Just would like to understand
also what is the best way about this...

Best Regards,
Elena.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 7220 bytes
Desc: not available
URL: <http://lists.rpm.org/pipermail/rpm-maint/attachments/20121123/b5785bea/attachment-0001.p7s>