>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.
smime.p7s
Description: S/MIME cryptographic signature
_______________________________________________ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
