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] 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-department. can even think of passing script content to it, if we want to scan it for malware patterns, but this is probably too futuristic), and post hook can be used... hm... (trying to think of security reason) maybe just simply for verifying how did it go and cleaning up (can't find really any specific reason now) Well, not everything has to be related to security anyhow :) For example a logging plugin will certainly be interested in the post-script result. What do you think? Should I then change the patch to add two hooks in addition to current one and move them to
Re: [Rpm-maint] Script hooks patch
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... 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. 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 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 can even think of passing script content to it, if we want to scan it for malware patterns, but this is probably too futuristic), and post hook can be used... hm... (trying to think of security reason) maybe just simply for verifying how did it go and cleaning up (can't find really any specific reason now) What do you think? Should I then change the patch to add two hooks in addition to current one and move them to rpmScriptRun() for clean view? 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
[Rpm-maint] Script hooks patch
Hi, In the attachment promised patch for the script hooks: changing to have two hooks instead of one and calling them for lua case, too. Let me know what needs to be fixed J Panu, I will reply to your other email tomorrow morning: I am falling asleep already with this darkness around L Best Regards, Elena. 0001-Improving-script-related-rpm-plugin-hooks.patch Description: Binary data 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
Hi, Please ignore the previous path: I should have just went to sleep yesterday L Somehow it got locked in my head that case when execv might fail is a special case (which of course it is, but not for this) and we need to call the hook there also to indicate this. Complete nonsense when you look at it with fresh head. The corrected patch attached! Best Regards, Elena. From: Reshetova, Elena Sent: Thursday, November 22, 2012 9:42 PM To: rpm-maint@lists.rpm.org; Panu Matilainen Subject: Script hooks patch Hi, In the attachment promised patch for the script hooks: changing to have two hooks instead of one and calling them for lua case, too. Let me know what needs to be fixed J Panu, I will reply to your other email tomorrow morning: I am falling asleep already with this darkness around L Best Regards, Elena. 0001-Improving-script-related-rpm-plugin-hooks.patch Description: Binary data 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/23/2012 07:32 AM, Reshetova, Elena wrote: Hi, Please ignore the previous path: I should have just went to sleep yesterday Heh, I know the feeling :) Somehow it got locked in my head that case when execv might fail is a special case (which of course it is, but not for this) and we need to call the hook there also to indicate this. Complete nonsense when you look at it with fresh head. The corrected patch attached! I see a couple of further problems with this: - runLuaScript() will leak memory and a file descriptor if rpmpluginsCallScriptPre() fails. - 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. 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... 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. 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. 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... - Panu - ___ Rpm-maint mailing list Rpm-maint@lists.rpm.org http://lists.rpm.org/mailman/listinfo/rpm-maint
