Re: [Rpm-maint] Script hooks patch
Hi Panu, I am working on the next version of script patch and will send it hopefully today. One thing that I noticed currently: I think header.c file missing errno.h inclusion. It doesn't compile for me without it. Or am I doing smth wrong? Best Regards, Elena. -Original Message- From: Panu Matilainen [mailto:pmati...@laiskiainen.org] Sent: Saturday, November 24, 2012 12:57 PM To: Reshetova, Elena Cc: rpm-maint@lists.rpm.org Subject: Re: Script hooks patch On 11/23/2012 02:29 PM, Reshetova, Elena wrote: >> I see a couple of further problems with this: > >> - runLuaScript() will leak memory and a file descriptor if >> rpmpluginsCallScriptPre() fails. > > Sorry, will fix. > >> - For external scripts the hooks run on different sides of the fork(): >> pre-hook runs in the forked child, post-hook in the parent, so parent >> will > be unaware of anything occurring in the pre-hook. > > Yes, but we can't run it in the parent, because how then we setup the > security context on forked process? > >> I'd prefer the hooks called from rpmScriptRun() already so there >> would be > exactly one place where to deal with them, but whether that would > actually work is a different question entirely... > Would look nice from code point of view for sure, but I can't figure > out a way to do it currently... It's sooo annoying when practicalities clash with simple and beautiful code... :P > >> External scripts have a lengthy setup sequence in the child between >> the >> fork() and exec(), including a call to rpmExpand() on > %_install_script_path. Which should not, but technically could, have a > shell-invocation in it, which would cause a fork() + exec() of its own > which would cause (at least in case of SELinux) the exec context setup > to go to wrong address. It should be possible >to move the rpmExpand() > into parent and just pass the path as an argument to doScriptExec() > though but just goes to show how the kind of minefield this is. > > Oh, shell invocation isn't good at all in this case: it will be > invoked with all rpm privileges :( Is it an intended feature or smth > that we can try to limit? I have been thinking of this just as a way > to load configuration, but it seems to be a worse case. Technically any rpmExpand() and rpmExpandNumeric() call can result in shell invocation (and any arbitrary command through that). For example the %_install_script_path is by default defined as: %_install_script_path /sbin:/bin:/usr/sbin:/usr/bin:/usr/X11R6/bin It'd be perfectly legal (if dumb) to write this as: %_install_script_path %(echo /sbin:/bin:/usr/sbin:/usr/bin:/usr/X11R6/bin) ...and of course you can inject all sorts of "surprises" in there too, like %_install_script_path %(rm -rf /home; echo /sbin:/bin:/usr/sbin:/usr/bin:/usr/X11R6/bin) Besides shell, any macro can also be written in Lua, which can also invoke arbitrary commands, define further macros etc. Oh and "regular" macros can also define (and override) and undefine other macros. So yes, its quite nasty, and somehow limiting the power that macros can have would be good. I've been thinking of some kind of no-exec, read-only mode to the macro engine for quite some time now, but there are many open questions in that area. Such as what to do about the "violators" - neither rpmExpand() or rpmExpandNumeric() can return errors, and even if they could, its a non-trivial job to figure out how to handle them in all the relevant places. >> Internal scripts present different problems: as it is now, there's >> not much > difference between calling hooks from runLuaScript() and > rpmScriptRun(), but if we assume they'll get forked in the future, any > security context switch would have to happen after the fork. Which > inevitably causes the same >> asymmetry with pre- and post-hooks as there is now with external scripts. > True, but how much does the parent needs to know about it? > >> One way to address it would be having an "unpaired" post-fork (or >> pre-exec > if you like) hook that explicitly runs in the child process, which > basically is what we have now in the script setup hook. Which then begs the > question: > what exactly are the new script pre- and post-hooks supposed to be used for? >> Ok, I can imagine eg logging plugin wanting to do something in both >> of > those, but... > > Yes, I am starting to lean towards 3 set of hooks for scripts: > symmetrical pre-post in parent (nicely combined in rpmScriptRun) and > separate Setup hook in child. The purpose of child hook would be to > setup security context, the Security context or some other execution environment things, like environment variables present in the scripts (I can't think of practical use-cases for that atm but that doesn't mean they dont exist :) > pre-hook can be used for initial screening (IMO important part too), > like if we even want this particular script from this particular > package to run (I Ah yup, that's a reasonable use-case in the security-depa
Re: [Rpm-maint] Script hooks patch
On 11/26/2012 12:52 PM, Reshetova, Elena wrote: Hi Panu, I am working on the next version of script patch and will send it hopefully today. One thing that I noticed currently: I think header.c file missing errno.h inclusion. It doesn't compile for me without it. Or am I doing smth wrong? Oh, the include was missing alright, fixed now. This was somehow masked when using --with-selinux to compile (which I usually have), not the first time either... - Panu - ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] Script hooks patch
Thanks! I have noticed that you included the script hooks also? Was it intentional? I was planning to send you the new patch instead, since the previous one wasn't really quite right :( -Original Message- From: Panu Matilainen [mailto:pmati...@laiskiainen.org] Sent: Monday, November 26, 2012 1:38 PM To: Reshetova, Elena Cc: rpm-maint@lists.rpm.org Subject: Re: Script hooks patch On 11/26/2012 12:52 PM, Reshetova, Elena wrote: > Hi Panu, > > I am working on the next version of script patch and will send it > hopefully today. > One thing that I noticed currently: I think header.c file missing > errno.h inclusion. It doesn't compile for me without it. > Or am I doing smth wrong? Oh, the include was missing alright, fixed now. This was somehow masked when using --with-selinux to compile (which I usually have), not the first time either... - Panu - smime.p7s Description: S/MIME cryptographic signature ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] Script hooks patch
On 11/26/2012 01:51 PM, Reshetova, Elena wrote: Thanks! I have noticed that you included the script hooks also? Was it intentional? I was planning to send you the new patch instead, since the previous one wasn't really quite right :( Doh. Certainly not intentional, just had it committed in my local clone from looking at it on Friday and forgot all about it. I blame Monday :-/ Reverted now, thanks for pointing this out. - Panu - ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
Re: [Rpm-maint] Plugin ponderings
On 11/23/2012 03:08 PM, Reshetova, Elena wrote: 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! Oh, I haven't been looking at this outside the SELinux aspect either. Which is far too narrow view into a generic plugin interface - getting a wider perspective was a part of the reason for playing around with something completely different. 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. Ok, lets try to stick this for now then. Almost certainly there will be further curveballs (things that dont fit into the nice and clean paired hooks idiom) to deal with sooner or later, but having some kind of baseline convention should help anyway. 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. Trying to enumerate all the possible failure causes and then interpreting them in the callers is indeed doomed to fail. It'll need to be far more generic than that, my point was simply that I dunno if rpmRC is sufficient - just something to think about. 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
Re: [Rpm-maint] Plugin ponderings
Hi, I am attaching next version of the scriptlet-related patch. Next I will go back to current hooks and make a small patch to make sure they are called as we agreed. Will do this tomorrow. Also some comments below: > There's eg RPMRC_NOTTRUSTED that could be (ab)used for certain types of security-related failures, but what if we have several plugins on a hook, one returning RPMRC_OK, another one RPMRC_FAILED and the third one RPMRC_NOTTRUSTED - which >one should the CallFooHook() function return? >One of the failures sure but... The security theory is very easy for this case: the most restrictive case/result is applied/returned. So, in this case I would return that we failed on policy and indicate it with " RPMRC_NOTTRUSTED ". In this way rpm can always enforce the strictest rule, for example if it wants to abort installation after receiving RPMRC_NOTTRUSTED. > One possibility could be a bitfield type return code, where each plugin can raise relevant bits of fairly generic categories. There's no telling which plugin raised what bits, but at least it'd be possible to reflect more than one kind of failure per hook then. >But again, mostly just something to keep in mind / think about for now. This would be cool, especially if we get LSM stacking accepted upstream! Now there are active discussions on security mailing list on Casey's patches and I can imagine that in the some distant future we can have a number of simultaneous LSM working with rpm and a number of plugins that can use it. And for each plugin we can log what happens and make rpm to be more aware. But you are right: this is just future so far... > We could just make the hook return "void", but perhaps its better to let the hooks return errors codes and let rpm filter out (and perhaps log a >warning) what it cant handle: its possible that some things it can't handle now can be handled at some point in future. Agree, I think it is good to give plugins possibility to complain, even if the complains aren't heard at the moment. I just meant that maybe no reason to invent new return codes just yet. > Note that rpm always creates files (but not directories) with a temporary extension, and they're only rename()'d to final location (which might replace existing files on upgrades) after everything was successfully unpacked to the temporary locations. Rpm >doesn't currently do a proper job of cleaning up the cruft in case of failure, but there is a chance to undo at least some of the things before the fsm "commits" >the files to final destination. Oh, this is actually very good, but I am not sure this is full solution. I guess I need to study fsm machine more deeply. It is the hardest one for me to understand so far :( Let's imagine we install a package and while putting its files to temp. location security labelling fails on one of them (maybe even not the first file and this is the bad thing). Plugin returns failure on that file in file closed hook (which in this case would need to be moved before FSM commit). What would be the proper behaviour? IMO we need to revert the whole package installation unfortunately. Because if file is left on filesystem but not properly labelled, then it might not be usable, and this might also lead to app/service/daemon/whateverinstalledfrom package not usable too. Do we want such package on the filesystem then? But then the problem how do we remove the files that has already been commited or if this file happens to be a dir? >At least in SELinux, rename() doesn't affect the security labeling so it'd be possible to set the final contexts on the temporary files and only if that succeeds for all files, commit the result. Currently the labelling is done on the final destination so there's not >much chance of undoing in case of failure. Hm, isn't rename just a sub-case of mv? I think in this case, xattrs might not be preserved (for example for cp if you don't use --preserve=xattrs, then you might lose your xattrs, including security labels (applies for both SELinux and any LSM using the xattrs)). I would need to setup a small test to remember this. But there should be a way for sure to move the files from temporal location to final one while preserving all attributes, given that rpm has needed privileges (which it does). I wonder if rpm could first unpack all files and dirs to some temp location and then do one FSM commit (if everything went fine) to commit them all one by one. This would solve the problem of how to remove already installed files. > Yup... although some of this could be perhaps dealt with by rearranging the way file permissions etc are laid down: currently they're all done on the final destination, but if the labels, capabilities, modes etc were staged in it would allow dealing with, well, >not all but at least more failure cases. Would likely require quite some changes to fsm, but that's not exactly a bad thing (we've been slowly rewriting the damn thing anyway :) Yes, exactly my though
