[Rpm-maint] The proper way to bootstrap rpm plugin data
Hi Panu, After a while break I am back with new problems J with regards to the plugins. The use case now is like this: Security plugin has a policy file (generalizing to any plugin, some configuration file), which is complex enough that can be created/hardcoded in plugin itself (even in some default values). This file might come with the plugin or from some special package (plugin configuration package): reason to keep it a separate package in our case is that policy can be very different and it is easier to manage it separately from the actual plugin. Currently plugin has hardcoded path to where find the policy when it starts (it checks in the INIT hook) and if policy is missing, it stops the installation because it is meaningless to proceed without it. I never liked the hardcoded policy path at the first place, but now it bring even bigger problem when you run rpm with --root option on the clean dir. Rpm itself would recover from this quite nicely by creating a clean rpmdb and proceeding fine, but plugin obviously runs into troubles. Solving this would seem to have two parts from my understanding: - Plugin able to detect when we are in this specific state (run with -root option, database needs to be created) - Plugin able to obtain the correct path of policy file outside of -root and then nicely set it (I suppose by simply copying to the -root dir). And here I would like to be able not to hardcode any path, but have it configurable (that there is a possibility to use other policy that main policy outside of the -root with hardcoded path) For the first part, I guess the most correct solution like many people advised is to add a new hook inside rpmtsInitDB() that would indicate to a plugin the moment it needs to do smth about its policy file. Do you see any problems if we add such hook? The place can be just after we initialized rpm db itself after this line: rc = rpmdbInit(ts-rootDir, dbmode); One thing that I can see if that this new hook would be called after plugin INIT hook and in the INIT hook I can't longer simply check that my policy is missing and abort, I have to still return ok and then maybe hoping that it is the initDB hook that would be called and create missing policy. And if it doesn't, then I have to keep checking for missing policy and do its initialization in pre_tsm hook for example. So, kind of not that clean anymore compare to having it all in one INIT hook. The second part is more troublesome. What do you think should be a proper way of passing the info to a plugin about where its policy file is? Should it be done via macro configuration? 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
Re: [Rpm-maint] FSM hooks for rpm plugin
Should we do the stat passing then for fsmCommit hooks? I am attaching the current state of my fsmCommit hooks just to show the place where I was thinking to add them. I haven't checked the tabs and other stuff, so this is just to give idea. I was thinking that instead of passing mode_t to the both hooks (in this patch it still does that), the whole stat stuct can be passed and this would give at least these hooks enough info. What do you think? Or if is too bad/assymetric to have mode_t in other hooks and stat here? I think we better leave the stat struct out of the picture for now. Its not just that the information in the stat struct would be more than a bit fuzzy, but also its not really sufficient either. For example a %config management plugin would need to know whether a file is a %config or not, and that's currently not available to plugins without jumping through a lot of hoops. What we'd really need is being able to pass rpmfi objects to the hooks (in addition to some other things), but that requires largish changes in the rpm internals... But I've talked about that long enough maybe its time to actually do it. It shouldn't be a particularly *difficult* change, just fairly large and tedious one. Ok, I will then stick with mode_t for now and then we can get a proper rpmfi object later. As for the preliminary patch, yes that's what I was thinking of as well - in particular, calling the pre-commit hook before fsmBackup() so in case a backup is needed, the pre-commit hook can grab the contents before its moved out of the way. There are some open questions here too however: if there's a failure, the post-commit hook doesn't really know whether it was the backup that failed or the actual commit. True, but question is does it need to differentiate? If backup has failed, commit can't be successful, isn't it? And then there's the special case of directory replacing something else, in which case the backup is (and needs to be) taken in rpmPackageFilesInstall() already. And then there's the whole erasure business to deal with... Just wondering if there actually should be *yet another* hook for backups, which would allow avoiding the ambiguity in fsmCommit() and make plugins aware of all the scenarios in which backups are taken. So many hooks, sigh... dunno. Hm... I am not against new hooks, but they are staring to multiply quite fast... I will try to think more about it... In general I think we should not introduce the hooks unless good use case is present, otherwise we will lose track of them and create interfaces that noone else is using. So, the question is does a backup operation is smth plugin should be aware and understand or it is now fully internal to rpm operation. At least we're not in danger of running out of supported hooks bits anymore :) I changed the plugin initialization + hook-calling system fairly radically over the weekend as discussed. Nice :) I will take a look on it after I am back from travelling in 1.5 weeks and then will be also able to continue on the patch! 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
Re: [Rpm-maint] Plugin findings
Hi, There's really nothing like field-testing when it comes to finding issues... Now that I simply have to run with the selinux plugin enabled (otherwise I'd have a very broken system real fast), hacking on new plugins revealed a some fairly nasty issues in the plugin system (and caused a fair bit of head-scratching early in the morning :) that's gone merrily unnoticed so far: I have actually only tested and use the hooks with one plugin (msm), so I guess some issues could have slipped though, indeed :( When there are more than one plugins present, an unsupported hook (or errors like not finding the hook) in one plugin will cause all the remaining plugins to be skipped on that hook. This is because RPMPLUGINS_SET_HOOK_FUNC() does 'return rc' on several conditions, which doesn't go very well with for-loops... Oh, yes, I see this... One way would be to change it to a normal function that would return the state, but won't break the loop. The question is how to differentiate between hook returned RPMRC_FAILED and hook isn't supported, which are indeed very very different things. Another related thing is that RPMPLUGINS_SET_HOOK_FUNC() is executed several times per each file in the transaction, times number of plugins loaded. It didn't matter for the collection plugins as they are so different in nature, but now with several hooks per each file its just terribly wasteful if nothing else. Hm.. How do you plan to avoid this? If you have let's say a security (selinux or msm) plugin and a log plugin, or two different security plugins (future LSM stacking), each plugin would need to get a hook called. So... I'm planning on doing some fairly major surgery on the whole thing. Just checking whether you have some work-in-progress in this area, IIRC you were planning to look into changing the plugin initialization (move it much earlier etc) and I dont want to clash with that work if you've already started it. I have put aside the initialization work so far, since we were concentrating on fsm hooks, so there is nothing to clash with for sure! FWIW the kind of thing I have in mind is make plugins into objects that hold their own data (like name, symbol handle etc) and hooks are function pointers in the struct that are initialized when the plugin is first loaded so we dont have to rediscover the hook functions on each and every round, etc. This sounds good, would make it easier indeed! Best Regards, Elena. -Original Message- From: Panu Matilainen [mailto:pmati...@laiskiainen.org] Sent: Thursday, March 28, 2013 12:32 PM To: Reshetova, Elena Cc: rpm-maint@lists.rpm.org Subject: Plugin findings Hi, There's really nothing like field-testing when it comes to finding issues... Now that I simply have to run with the selinux plugin enabled (otherwise I'd have a very broken system real fast), hacking on new plugins revealed a some fairly nasty issues in the plugin system (and caused a fair bit of head-scratching early in the morning :) that's gone merrily unnoticed so far: When there are more than one plugins present, an unsupported hook (or errors like not finding the hook) in one plugin will cause all the remaining plugins to be skipped on that hook. This is because RPMPLUGINS_SET_HOOK_FUNC() does 'return rc' on several conditions, which doesn't go very well with for-loops... Another related thing is that RPMPLUGINS_SET_HOOK_FUNC() is executed several times per each file in the transaction, times number of plugins loaded. It didn't matter for the collection plugins as they are so different in nature, but now with several hooks per each file its just terribly wasteful if nothing else. So... I'm planning on doing some fairly major surgery on the whole thing. Just checking whether you have some work-in-progress in this area, IIRC you were planning to look into changing the plugin initialization (move it much earlier etc) and I dont want to clash with that work if you've already started it. FWIW the kind of thing I have in mind is make plugins into objects that hold their own data (like name, symbol handle etc) and hooks are function pointers in the struct that are initialized when the plugin is first loaded so we dont have to rediscover the hook functions on each and every round, etc. - 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] FSM hooks for rpm plugin
After far too much pondering... I went ahead and added the prepare hook + some related bits and pieces. And actually ripped out SELinux support from rpm core while at it, replaced by a simple SELinux plugin. Wohoo. Looks cool :) Hope it works ;) I think I'll leave the commit-hooks to you though :) Ok, I think I will be able to send you a version for review today, but I have got one question. I was under impression that we at some point agreed to pass to hooks the whole stat structure as opposite for just mode_t. This would allow plugins to make checks on things like st_nlink and other useful info about the file. Do I remember this wrongly? 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
Re: [Rpm-maint] FSM hooks for rpm plugin
On 03/14/2013 03:01 PM, Reshetova, Elena wrote: Sure, I'm not suggesting delaying everything until I someday get around to fixing it, just that we could try thinking ahead for that model to hopefully avoid having to change the plugin interfaces later. I pushed a bunch of fsm changes yesterday, the two more interesting ones that we already talked about being: 1) Reflect the hardlink count in st_nlink so the real files vs hardlinks can be easily detected 2) Set permissions before committing to the rename to final destination. With 2) in place, we might be able to model the hooks in a way that doesn't require changing later. The question (again) just is, what the hooks should actually be. I think we'd want those pre- and post-commit hooks in any case: for example a %config versioning system plugin would want to know whether a file is being replaced and if it actually succeeded. The pre-commit hook could of course be used for setting additional permissions, content checking etc as well, but in the alleged new model of unpack + set permissions on all files first and only then commit, I think one would want to abort the whole thing as early as possible. Not that it matters all that much if we really are able to undo the whole thing. So I guess we'll just go with the pre- and post-commit hooks for now to be able to move forward with this. At least no-one can say this hasn't been thoroughly discussed :) I just went through your yesterday's changes. I think it now slowly falls together nicely. I think it is right that we need pre and post fsm hooks because even if we were able to unpack everything and successfully set all permissions on all files in tmp location, it isn't a guarantee that committing the whole thing to the final location would be successful. It is always possible that preserving security labels might fail or anything else might happen. And when you change a fsm model to new one, we can just add a new hook that would be called after each file is unpacked to tpm location: this would be primary hook for setting additional metadata on file and good time to scan the content of a file, too (so that we can revert the whole thing and delete a file if malware is found in it). Yup, and its this part I'm still pondering about: should we just add that post-unpack hook (or whatever its called) already and go with that for SELinux and all from the start, because that's what they really want. That's kinda what the setmetadata hook idea, but perhaps a more generic name would be appropriate now that it wouldn't be limited to that. Maybe something like file pre- and post-process, which can cover metadata, malware scanning and whatnot, both for install and erase (which needs the perhaps otherwise unnecessary pre-hook) Hm... I guess you are right: even without changing fsm, such hook can be introduced now and started to be used right away. But do you want to have one hook (for install and erase) or two hooks (pre- and post-process)? I think having pre- hook isn’t very beneficial, but post-unpack is the needed one. Maybe we can call it smth like FilePrepare or so to indicate that file is tmp location and hook is called to setup the necessary attributes and check the file? The only thing that I can't find so far a usage for pre commit hook for future: it would be kind of called on the same context (file is unpacked in tpm dir) and the future metadata/content screening hook One idea can be that for security needs, plugins can actually use pre- and post hooks to verify that permissions were preserved (and set) correctly and abort if they see some mismatch. But maybe this is too paranoid again :) That's perhaps slightly paranoid, yes :) But then its not my job to say what the plugins should be used... for some purpose having yet another layer of verifying might be reasonable. One use-case I see for the pre- and post-commit hooks is a plugin keeping track of config file contents: in pre-commit it would stage the change of content (think of git apply), and in post-commit it would either commit or abort (think of git commit or git reset --hard) depending on whether fsm commit succeeded or not. Yes, I think this might be a better use case than a paranoid plugin :) So, then we have 3 hooks: pre- and post- commit and some kind of filePrepare after unpack. I think this should be smth we will be able to leave with for quite a while even when fsm is changed. 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
Re: [Rpm-maint] FSM hooks for rpm plugin
Sure, I'm not suggesting delaying everything until I someday get around to fixing it, just that we could try thinking ahead for that model to hopefully avoid having to change the plugin interfaces later. I pushed a bunch of fsm changes yesterday, the two more interesting ones that we already talked about being: 1) Reflect the hardlink count in st_nlink so the real files vs hardlinks can be easily detected 2) Set permissions before committing to the rename to final destination. With 2) in place, we might be able to model the hooks in a way that doesn't require changing later. The question (again) just is, what the hooks should actually be. I think we'd want those pre- and post-commit hooks in any case: for example a %config versioning system plugin would want to know whether a file is being replaced and if it actually succeeded. The pre-commit hook could of course be used for setting additional permissions, content checking etc as well, but in the alleged new model of unpack + set permissions on all files first and only then commit, I think one would want to abort the whole thing as early as possible. Not that it matters all that much if we really are able to undo the whole thing. So I guess we'll just go with the pre- and post-commit hooks for now to be able to move forward with this. At least no-one can say this hasn't been thoroughly discussed :) I just went through your yesterday's changes. I think it now slowly falls together nicely. I think it is right that we need pre and post fsm hooks because even if we were able to unpack everything and successfully set all permissions on all files in tmp location, it isn't a guarantee that committing the whole thing to the final location would be successful. It is always possible that preserving security labels might fail or anything else might happen. And when you change a fsm model to new one, we can just add a new hook that would be called after each file is unpacked to tpm location: this would be primary hook for setting additional metadata on file and good time to scan the content of a file, too (so that we can revert the whole thing and delete a file if malware is found in it). The only thing that I can't find so far a usage for pre commit hook for future: it would be kind of called on the same context (file is unpacked in tpm dir) and the future metadata/content screening hook One idea can be that for security needs, plugins can actually use pre- and post hooks to verify that permissions were preserved (and set) correctly and abort if they see some mismatch. But maybe this is too paranoid again :) 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
Re: [Rpm-maint] FSM hooks for rpm plugin
Do you want to do the changes? I can also try to do it tomorrow if they aren't objections. I probably should merge (at least some of) the study and link count patches first, as those change the landscape quite a bit. I'll try to do that as soon as the caffeine kicks in for good. Sure, I will wait for changes. On a somewhat related note, I'm pondering about changing fsm to do staged removals too, ie rename before actually removing. It doesn't make much difference as things are now, but I've also started seriously thinking about changing the fsm to the model we discussed earlier where unpacking and setting permissions etc is first done for all files, and only if that succeeds completely we actually commit to renaming them all to the final target, and undo the whole lot if anything in unpacking failed. I think this would be the safest way not only from security, but also from correctness and also makes installation more robust in case of sudden power cuts and etc. ...which of course would actually fundamentally change the landscape again: if commit is changed to consist only of renaming a file, then commit hooks would no longer the right place to do security labeling etc. Argh! :) In that model we'd be back to the set metadata hook, or actually two of them to preserve the possibility of doing something after rpm did its own business. And in that model, both pre and post metadata hooks should get the temp and final path as separate arguments. Yeah, but I guess maybe we can first finish with the current system and check that it works for whatever test cases we have (I can start using new hooks in msm plugin) and then change it when you move rpm to a new fsm model. I think this would be a big change for fsm, so won't be possible to do it fast anyway. 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
Re: [Rpm-maint] FSM hooks for rpm plugin
A file is a hard-link if (S_ISREG(st-st_mode) st-st_nlink 1) is true. When erasing, we get this info from filesystem so that remains accurate (the last one would be seen as the real file). On installation the stat struct of a file is made up by rpm, so we can pass whatever we want in there. Currently st_nlink for hardlinks equals the total number of links a file will have, but we can easily change that to the number of *current* links so that it better matches reality. Ie the first one will have st_nlink == 1 so it will be seen as the real file, the rest will st_nlink++ each. See attached patch for a quick implementation of this. Oh, I guess I just wanted to say that after file is installed there is no way to determine where is the initial file and where is the hard link, but indeed in rpm case for installation, it can be indicated (as your patch does) by rpm, so I guess it is all fine, then :) 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
Re: [Rpm-maint] FSM hooks for rpm plugin
On 03/06/2013 05:01 PM, Reshetova, Elena wrote: What's missing is that another call to the hook is needed in fsmMkdirs() for the unowned directories, and there we should perhaps pass in the unowned aspect somehow. Having a separate argument for that seems like an overkill though... maybe we could pass that piece of info in the action argument instead. One possibility could be adding a new action, eg FA_CREATEUNOWNED that could be used for unowned directories (and files, but there aren't any currently). Or we could define the action as a partial bitfield: leave the existing actions alone but reserve the upper byte for special bits, such as unknown. I had some other use-case for turning it into (partial) bitfield but can't remember what it was right now. I think bitfield would be better in this case: if one start introduce actions FA_CREATEUNOWNED, then why not have FA_CREATELINK and etc. I will add hook calling for mkdirs, but I think it is better that you do a change of actions in a separate commit. Sure, and I agree bitfield seems like the better option as it'll allow cramming a whole lot more information in there. There's a whole lot of redundancy in the current actions (all the skip cases for example) and some of the values are totally unused as well. As the file actions aren't really exported in the API in a way that somebody could actually be using them, we might be able to get away with just redefining the whole thing as a bitfield and add the old symbols as defines or'ed together from the bits. But I'll need to think about it some more, in any case such a total conversion is not required in order to add a handful of bits right now. Ok, I think if you can add an unowned bit to it for a start, this can be a good beginning and then even the existing pre/post hooks can get one less argument, which is great. Continue on bit field idea, it would be great for plugins to get the basic info about the file acted upon from action also: so , if we are adding unowned bit, should the basic bits indicating hard or sym link be also supplied in action? Like action could point that it is creation of symlink or removal of hard link. Is it too bad idea? Hmm, but we already pass the mode (and planning to pass the whole stat struct) around, so you can tell whether its a symlink, directory or something else. Hardlinks are the more special case but you can tell them apart from others by st_nlinks from the stat struct. Except that you dont necessarily know which one is real file without extra tracking... unless st_nlinks is 1 for real files (including the first hardlink) and more for the actual links. I'm not sure if that's the case even already, but should be possible to arrange at least. I am quite sure you can't tell it from stat struct which is real file and which is hard link: the value for st_nlinks is stored in inode and not in dentry in my understanding, that's why it isn't really easy for plugin to detect the hard links, so indicator would be a plus... 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
Re: [Rpm-maint] FSM hooks for rpm plugin
Another issue I just realized is that with this patch, the metadata hook call
is not under the setmeta condition so it will get called always, ie in cases
like hardlinks when it specifically should *not* be called :)
I think the fsm-part should be something like this instead:
@@ -1569,6 +1569,11 @@ static int fsmCommit(FSM_t fsm, int ix, int setmeta)
}
if (setmeta) {
+ /* Run fsm metadata hook for all plugins */
+ if (!rc) {
+ rc = rpmpluginsCallFsmFileMetadata(fsm-plugins,
+ fsm-path, st, fsm-action);
+ }
/* Set file security context (if enabled) */
if (!rc !getuid()) {
rc = fsmSetSELabel(fsm-sehandle, fsm-path, fsm-sb.st_mode);
Ups, indeed, will fix!
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] FSM hooks for rpm plugin
I have been thinking about it now and I think having a hook for setting file
meta data is a good idea in any case (even if we decide to keep pre/post hooks
for some other purpose). It shows much clearer the purpose of the hook and it
can be placed nicely exactly where metadata is set (and together with your
latest commit it will be setting things right for hardlinks and etc.).
Another thing is if we still need some kind of pre/post hooks for files
separately... I was trying to think of use cases beyond just a logging plugin
that you were referring before. One more concrete use case that I now need to
look to is using plugin interface for having a package virus scanner. The
idea would be that plugin would scan the selected content from the package
(native executables, maybe some scripts) and if any malicious pattern is
detected, then do smth about it (preferably don't install the content at all
to avoid it to be started even unintentially). The difficulty here is that
plugin can't scan the code by itself (especially on mobile device) since it
doesn’t have a knowledge to do so, so it would need to pass the content of a
file in chunks to the actual scanning engine on the platform and get result:
malware detected or not. Ideally if malware is detected, as a last layer of
defence it would be very good not to call fsmcommit on that file and abort the
installation with error to avoid file to be placed in real filesystem, but I
guess it would be even better if the scan can happen even earlier that we
don't need to abort the package installation in a such nasty way. If this can
be done earlier, then I guess there is no need to pre/post hooks, but if not,
they might be very much needed together with the update hook that we have
already dismissed (it was used only for checksum calculation).
Actually while thinking more about it, I think the cleanest way is to process
the file when it has been unpacked to tpm location, but hasn't been committed
yet. The file is already on filesystem, but since it is in tpm location and
without proper attributes, it is quite limited on what it can do. This would
make a need of one more hook somewhere just before fsmCommit happens. I would
put it just after this piece of code in fsm.c to make sure it is called only
for real files and not dirs, symlinks and etc.:
1721 if (rc == CPIOERR_ENOENT) {
1722 rc = expandRegular(fsm, psm, archive, nodigest);
I think this hook can be called smth like FsmFileCheck() or smth like this and
it can be used for content checking or any other checks on a file before it is
transferred to a final location.
Do you think it make sense?
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
Re: [Rpm-maint] FSM hooks for rpm plugin
Looking at this, I just realized that rpm is currently doing chmod(), chown() and all for each hardlink it creates, which just doesn't make sense because ... by the very definition of a hardlink, it doesn't. Probably worth fixing regardless of what we end up doing with hooks, eg additional setmeta argument to fsmCommit() whether it should just create or set the additional metadata as well, and have pre/post-commit hooks get that so plugins get notified of all file creations but also can avoid redundant setting of labels etc. FWIW, this part is now pushed to git master, ie for hardlink sets the metadata (permissions etc) is only set once. I think this part looks good and clear, but indeed doesn't help with the below part. I'm going to poke around with this a bit to see what would make most sense, now that I have sufficient selinux plugin code to test it with. Like said, I'm starting to have second thoughts on the skipped files, so I'll probably look at changing the existing hooks to the commit model rather than add more: for actually created (and removed) files, the hook semantics would be rather obvious. With skipped (and some delayed and whatnot) files it gets far more convoluted. If it turns out the plugins *really* need the skipped file info as well, we can always add (back) more hooks later on :) Attached is one study into this direction, ie hooks are only called for files that are actually acted upon. This is a total diff of multiple commits with some semi-unrelated changes, so its more useful to look at the resulting code more than the diff itself. I'm not going to push this stuff until further discussion + thought, if at all: the more I look and think about this stuff, something about it all just doesn't feel quite right :) It doesn’t look anything bad to me, but you are right below that we are now more like trying to fit these hooks somewhere to keep symmetry without even being sure we need it. Perhaps the problem is the hooks are too generic for their own good now. One possibility (that's perhaps more clear with the other cleanup work in the patch) is to have a set additional file metadata hook, as *that* is what our current use-cases (SELinux and MSSF) want to do. Which should actually be as simple as adding something like this after all the fsmChown(), fsmChmod() etc calls: if (!rc) rc = rpmpluginsCallFsmFileMeta(fsm-plugins, fsm-path, ...) And actually that brings it right next to where fsmSetSELabel() is currently getting called. I've a feeling a symmetric pre/post-hook pair doesn't actually make sense for this particular purpose, it only complicates things unnecessarily. Thoughts? I have been thinking about it now and I think having a hook for setting file meta data is a good idea in any case (even if we decide to keep pre/post hooks for some other purpose). It shows much clearer the purpose of the hook and it can be placed nicely exactly where metadata is set (and together with your latest commit it will be setting things right for hardlinks and etc.). Another thing is if we still need some kind of pre/post hooks for files separately... I was trying to think of use cases beyond just a logging plugin that you were referring before. One more concrete use case that I now need to look to is using plugin interface for having a package virus scanner. The idea would be that plugin would scan the selected content from the package (native executables, maybe some scripts) and if any malicious pattern is detected, then do smth about it (preferably don't install the content at all to avoid it to be started even unintentially). The difficulty here is that plugin can't scan the code by itself (especially on mobile device) since it doesn’t have a knowledge to do so, so it would need to pass the content of a file in chunks to the actual scanning engine on the platform and get result: malware detected or not. Ideally if malware is detected, as a last layer of defence it would be very good not to call fsmcommit on that file and abort the installation with error to avoid file to be placed in real filesystem, but I guess it would be even better if the scan can happen even earlier that we don't need to abort the package installation in a such nasty way. If this can be done earlier, then I guess there is no need to pre/post hooks, but if not, they might be very much needed together with the update hook that we have already dismissed (it was used only for checksum calculation). What do you think? 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
Re: [Rpm-maint] FSM hooks for rpm plugin
Right, no surprises: there is an issue with hard links :) Oh, this is smth I should have actually remembered, but forgot :( I saw the issue a while back on our system setup, but since from security labelling hard links aren't interesting (security context should be set on a file itself, not a hard link), I simply didn't call hooks for hard links. But I agree that it might be important to know that they will be created and I guess in this case we would need to know when and where it goes. Actually not from labelling point of view, but from general security, a tightly configured system might have restrictions on where files/dirs/links are created. For example, it might not allow creating hard links in certain paths (in order for system to stay well arranged or for example in order to avoid untrusted packages creating hard links in sensitive dirs). The thing here is that such a check should be probably performed before we start to run FSM and unpacking the archive (for the same reason as conflicts). So, then if we consider that labelling and policy enforcement aren't valid use cases here, we only have informational use case left: plugin wants to know the links and their location as for any other files. Should we then create separate hook/call same hook (with modified parameters) actually at the moment when links are created (later on)? Regardless of how exactly the hardlinks are handled, perhaps we should pass a pointer to the entire stat struct instead of just st_mode to the hooks, that'd at least allow plugins to know they're dealing with hardlinks (and various other possibly useful information). I guess this is true, should be easy to change. A different issue (much easier and one that we already discussed iirc) is that I think we must check the return code from rpmpluginsCallFsmFilePost() and allow it to fail, otherwise its not possible to preserve the current behavior where eg failure to set SELinux context (or other similar security thing) causes package installation to fail. I am always very supportive for giving more power to the plugin :) I think that it is possible to setup a system that failure to set a security context for files from the package might not result in security compromise, but then it might make package unusable and the effort required to do this is much bigger (we would need to play with security context of running rpm, which isn't that great). So, yes, I guess failing in this case is the easiest way. The good news is that other than the two above issues, the plugin API seems to work quite nicely for SELinux. These are really good news! I was expecting more problems actually in beginning. 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
Re: [Rpm-maint] FSM hooks for rpm plugin
I'm actually going to be mildly surprised if there aren't any strange cases we've missed wrt hard links or such :) Anyway, the patch looks as obviously-correct as it gets within fsm. Applied, thanks for the patch! Yeah, but I guess we will hopefully find it out soon, when we will be using the hooks :) Also with this patch in place, I think all the required bits and pieces for moving the entire selinux support into a plugin are there now. Wohoo :) This is actually my next question: how should we proceed? There are still some hooks (like handling conflicts and policy enforcement on signatures) that I would like to be present in rpm (I think other systems like SELinux might benefit from them too), and also we were thinking about rearranging the plugin initialization and etc. Or should we first try to move the SELinux to plugin to verify that the basic set of hooks works for it? 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
Re: [Rpm-maint] FSM hooks for rpm plugin
Hi, Hi, sorry about the delay... the recent patch-flood on rpm-maint caught me by surprise :) Patch flood is always good, total silence is much worse :) I've cleaned it up somewhat now, for example the early return was just plain wrong as it would've leaked resources all over the place. But then it also was a case that could never be reached at all... The code still looks suspicious in many places and wants further inspection and sanitizing but achieving symmetrical behavior for the hooks might actually be possible now. At least its *closer* to that target if not there yet :) I think it looks much better now and integrating hooks to it is a pleasure. I am attaching the new version. Hope I didn't miss any strange case, but it looked very easy now after your change! Best Regards, Elena. 0001-Adding-FSM-file-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] FSM hooks for rpm plugin
Hi, I've started to wonder about the hook names though: open and close seem out of place especially here (and to lesser degree elsewhere). Perhaps these hooks should simply follow the pre/post pattern even in the names, ie FILE_PRE and FILE_POST. I think it'd be more descriptive wrt what they actually do, and leave open and close free for possible later use for the cases where a kind of open+close (files from payload stream) is actually occurring. Sure, this is an easy change and I just did it. But the one below has made me a headache :( This gets quite ugly indeed, better refactor the surrounding code (which is ugly in itself) to allow for nice clean pairing. Oh and btw, this version of the patch introduces a new asymmetry case: if the open-hook fails, close-hook wont get called. It'll need something like open-hook just flagging postpone without terminating the loop, and adjusting the rest of the code to deal with it. I'll have a look at this, how hard can it be ;) I have written and rewritten now this part and still it looks ugly or even worse :( Flagging postpone is actually easy as well as not breaking out of loop in the beginning, but the early return is a small nightmare. If you introduce a variable tracking this, smth like hardbreak and setting it at that point, then you have a number of stupid if() causes, which make the whole constructions completely unreadable in addition to handling postpone and rc status (I have to walk thought this with pen and big white sheet to track the cases). Other option is goto xyz to bypass this but I am not sure this is any better, but might be easier to read. What do you think is better? I have also fixed the other two issues you pointed in the other email. 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
Re: [Rpm-maint] FSM hooks for rpm plugin
Hi, I am attaching the next version of the patch with modifications explained inline below. The patch looks deceptively simple :) In principle things look ok to me, the issues I see have mostly to do with the symmetry part: In fsmMkdirs(), the FileOpen() hook is not called, only rpmpluginsCallFsmFileClose(). If we promise symmetry, we need to stick to it here too. Looks like just an oversight and trivial to fix. Yes, fixed now: I was looking to install/remove mostly and forgot about symmetry on this one. rpmPackageFilesRemove() looks like symmetry is indeed guaranteed, but the problem here is that the hooks will get called whether the file/directory is actually removed or not, and the hook does has no way of knowing it: passing FSM_PKGERASE (or FSM_PKGINSTALL on install) is not sufficient. Either we need to pass fsm-action (those FA_CREATE, FA_ERASE, FA_SKIP etc things) to the hooks, or alternatively only call the hooks when an actual action is taken. Or even both: a plugin might want to know if eg a backup is taken. Not sure which option is the best here: from plugin POV it'd certainly be simpler to not have to worry about files that rpm has decided to skip entirely (there shouldn't be need to label etc such things). But then there might be use-cases where plugins would want to know the fate of each file, including skipped. It's not exactly hard to do if (XFA_SKIPPED(action)) in plugins that dont care about skipped files either. I think it would make sense to call a plugin anyway but supply the action. It is indeed easy for plugin to filter actions it wants to do smth about and it is hard to see all potential use cases. So, I am passing the action now as a parameter and then it is up to plugin to react on this. And then there's the PITA also known as rpmPackageFilesInstall(). There are at least two cases where FileClose() hook could be skipped despite FileOpen() getting called (early return before expandRegular() and a break in the post-processing to rmdir/unlink failed file. At least the latter should be reasonably easy to refactor, the early return case .. guess I need to actually figure out what on earth its about in the first place :) Hm... I guess both can be fixed by manually calling fileclosed() hook before return or break (like in this version of the patcj), but it doesn't look that clean and nice. :( The verify() function before early return is hard to read for me, it seems to do million things and even called a number of times during installation. Another option would be to call fileOpen() hook later, but then one can't stop cleanly in emergency cases. And like said, the same question whether to call the hooks for skipped files is true here as well, but probably somewhat trickier as there are more details and cases to handle. Would passing the action to all hooks help to it? I think the plugins should be allowed to cause an abort here, just like rpm itself can, in case of fatal unexpected failure. Unexpected is the key really: what I complained about was the MSM plugin *planning* to fail here on unresolved file conflicts, which is something that should be handled before anything gets installed or removed. Yes, I agree and I think the only reason before was the signature check hook wasn't called early enough, but this is one of the next todo items to do it properly :) Best Regards, Elena. 0001-Adding-FSM-file-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] FSM hooks for rpm plugin
Hi, Long time again since I replied :( Unfortunately had to resolve a number of other issues and wanted to attach smth already to this mail as opposite to just reply. I have started from FSM hooks as you indicated and I am including the initial version of patch for review based on our discussion. I have two hooks: fileOpen and fileClose and call them separately for install and erase. I had to make a number of choices while writing this patch, let's see if they were good ones :) Some details: - I tried to keep the logic of other hooks: if pre_hook is called, post_hook is also called with the result of the operation. However, it is a bit trickier in fsm case. For that purpose, I moved the fileclose hook in installation out of fsmCommit() that we can nicely pass the result to the hook. I also think it looks better from symmetry point of view, but it does now perfom labelling of a file (if it happens inside of a plugin) not exactly at the same place where Selinux currently does it. - I also made it that result from fileclose hook is ignored currently for the same reason as for post_tsm and post_psm hooks: what can rpm do after file has been committed even if plugin is unhappy? -The tricky part is what to do with the result code of fileOpen hook. In principle, this can be the place to abort installation/erasure of a concrete file in case smth really terrible happened (can't even think what can happen). Normally plugins should not abort anything on this hook (as we discussed) and if they do, then smth is wrong in plugin. On the other hand, rpm itself is physically able to abort at that point and even does it in cases for example if smth wrong with the archive unpacking. So, I am not really sure what to do with the return code in this case. - I was also thinking that it is probably not worth making it initially more complicated and adding additional hooks, like for handling the temporal files, because they can't really help fully with the security part: we might succeed setting whatever label on tpm file, but fail a second after on real file, or not succeed setting a label even on tmp file. I guess these hooks can be added on demand or simply later if the strong need comes. Best Regards, Elena. 0001-Adding-FSM-file-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
[Rpm-maint] FSM hooks for rpm plugin
Hi Panu, I finally got back to file hooks and tried to look into this with a fresh head. I think given our previous discussion, I would propose to have only two symmetrical hooks inside FSM for plugins. I would also make rpm to ignore any return code from them now, since there isn't much that we can really do even if they return some failure. FSM_INIT (const char* path, mode_t mode) Called after fsm.Init() has finished, can be used by plugins to get pre-warned that this file will be now installed to filesystem. Currently in msm plugin this hook is used very wrongly in a sense that it attempts to stop the file writing. I am looking forward to change this, but first I would need to resolve the conflict hook problem (see below). However, when the need to do this ugly functionality in this hook goes away, I think plugins might still be able to benefit from the hook or at least for symmetry looks (we do have pre and post hooks for ts and te). FSM_COMMIT (const char* path, mode_t mode) - Called inside fsm.commit(), can be used by plugins to perform file labelling - in the future it would be nice to pass to this hook also fidigest and digestalgo that plugins also can access the digest of the file that got written to fs and do additional labelling (like signing the file based on digest for IMA or smth like this). In addition to these two FSM hooks, we do need a file conflict hook in order to be able to prevent packages rewriting each other files when rpm is run with --replacefiles mode. Previously you mentioned that when the hook is called, signatures of all packages have been already verified, so in principle it should not be a problem to access at this point the info about who signed this particular package that brings this file. However, I can't understand how could it already be verified, if the hook is called inside rpmtsPrepare(). You mentioned that with future changes and introduction of an object we might be able to get needed parameters passed to the hook, but I think I don't understand how it will be working while looking to the code. Could you please explain a bit on this? For FSM hooks, if there are no objections, I can send you a patch tomorrow for a review. 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
Re: [Rpm-maint] Plugin ponderings
Hi Agree, it looks nicer with 0/1 values: attaching the corrected one. Best Regards, Elena. 0001-Making-pre-post-tsm-psm-hooks-more-consistent.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] Plugin ponderings
Hi, I made a new small patch to clean up the existing pre/post hooks as we discussed this week. After this is cleaned up, I can get back to fsm hooks. After our recent discussion I think I need to reconsider places of some hooks, for example post hook, or maybe make two post hooks? One when file is placed to tpm location (time to label the file) and another one after fsm commit. What do you think? Best Regards, Elena. P.S. At least hope my indentation issues are over :) 0001-Making-pre-post-tsm-psm-hooks-more-consistent.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] Plugin ponderings
Thank you for all clarifications! Now I am attaching the corrected patch and
the most important is that I understood the indentation finally (took a
while through :()!
Hopefully this is the last iteration for the script cases :)
-Original Message-
From: Panu Matilainen [mailto:pmati...@laiskiainen.org]
Sent: Thursday, November 29, 2012 2:38 PM
To: Reshetova, Elena
Cc: rpm-maint@lists.rpm.org
Subject: Re: Plugin ponderings
On 11/29/2012 01:52 PM, Reshetova, Elena wrote:
Yes, I am sorry: I have made two mistakes now:
1) patch is wrong (sent the intermediate version before the
compilation and plenty of other changes)
Ok, thought that might be the case.
2) and yes, I had default indentation of 4, which was confusing me for
a long long while :( I was just using simple Ubuntu file editing, I
changed it now and it looks much better.
Heh. That certainly explains a lot, you must've been thinking I'm crazy
complaining about indentation on what looks like the most horrid
misindentation-mess you've ever seen :D
The rpm codebase does look like the indentation level is 4, because it
actually is. It's just that its not 4 spaces or tabs of 4, but a combination
of zero or more hardtabs of width 8, plus one softtab (4
spaces) where needed to place things between the hard tabs.
Just in case, here's a practical (if extremely stupid :) example with the
indentation levels spelled out below:
int foo(int x)
{
int rc = 0;
for (int i = 0; i x; i++) {
rc++;
if (rc 255) {
if (rc 65535) {
printf(WAY too big!);
} else {
printf(too big!);
}
}
}
return rc;
}
0 hardtabs, 1 softtab
1 hardtab, 0 softtabs
1 hardtab, 1 softtab
^ 2 hardtabs, 0 softtabs
I will send a new version in a sec with things corrected.
Sorry again for all of this!
Mistakes happen, no worries. I'm just glad we identified the source of
indentation issues - the possibility (and consequences!) of hardtab set to
width of four only occurred to me after seeing these last two patches.
For the future, if it seems like I'm talking complete nonsense, please just
say so. It's entirely possible that
a) I actually am talking complete nonsense (like with the fsm commit stuff,
still puzzling how I could remember it *that* wrong)
b) There's some local difference causing you to see something entirely
different than I do. Be it actual functionality or editor settings :)
...etc.
- Panu -
0001-Improving-scriptlet-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] Plugin ponderings
Much better late than never, the indentation is now exactly as it should be, good! There's just one extra leftover indent level for the execv() call. Oh, missed that one since it wasn't in diff: fixed now! I'm afraid we'll need one more round: you perhaps took my that's what RPMSCRIPTLET_EXEC bit is for (wrt embedded scripts) a bit too literally :) What I meant is that the RPMSCRIPTLET_EXEC bit defines whether something is embedded or external - external scripts always have it, embedded ones never have it as there is nothing to exec. The patch gets it a bit backwards in rpmScriptRun(): Oh, then I indeed misunderstood you: I was thinking that external scripts will have fork + exec and lua ones just exec, since they are executed after all (just not via exec). Now fixed, too. While not strictly necessary, I think rpmLuaScript() could be changed to take the additional 'plugins' argument while we're at it, if only to keep symmetry between the two functions. It'll be needed later on anyway, unlike the selinux argument which gets passed to runLuaScript() just for the symmetry's sake. Sure, added! I am attaching the new version now *without* saying that this is the last one :) Best Regards, Elena. 0001-Improving-scriptlet-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] Plugin ponderings
Hi,
New version is attached. I have corrected (hopefully) the indentation and
other cosmetics.
Also, changed the rpmScriptRun() as you suggested (including bits for
script execution flow indication), it indeed looks much better!
I also removed the return code assignment from POST hook, because it indeed
overwrites the actual script return code and anyway again there is no way
rpm can handle any failures from this hook.
And about changing the fsm commit to commit everything after all files are
installed to tmp and all plugins/checks/whatever has executed, I understand
that this would be a BIG change. Just maybe smth for really far future..
Just it could help improve robustness, too IMO.
Best Regards,
Elena.
-Original Message-
From: Panu Matilainen [mailto:pmati...@laiskiainen.org]
Sent: Tuesday, November 27, 2012 12:26 PM
To: Reshetova, Elena
Cc: rpm-maint@lists.rpm.org
Subject: Re: Plugin ponderings
On 11/26/2012 07:26 PM, Reshetova, Elena wrote:
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.
Some issues with the patch still, comments + further ideas below:
In rpmScriptRun(), just grab the internal/extranal bit into a variable early
so you dont need double calls differing only by the internal/external
argument to the hooks. So the whole thing then becomes something like
---
int script_type = SCRIPTLET_TYPE_EXTERNAL;
if (rstreq(args[0], lua)
script_type = SCRIPTLET_TYPE_INTERNAL;
rc = rpmpluginsCallScriptletPre(plugins, script-descr,
script_type);
if (rc != RPMRC_FAIL) {
if (script_type == SCRIPTLET_TYPE_INTERNAL)
rc = runLuaScript(...);
else
rc = runExtScript(...);
}
rc = rpmpluginsCallScriptletPost(plugins, script-descr,
script_type, rc);
---
Except that here we get to the return code issues again: the rc returned
from Post hook overrides whatever the earlier result might've been, eg if
the scriptlet execution failed then the post-hook definetely shouldn't turn
that into an ok. That flaw is present both in your patch and my sample code
above. The other question is: if the scriptlet execution succeeded, is there
ever a valid case for a plugin to turn that into a failure? For such a
seemingly simple thing, this is surprisingly tricky to get right :)
BTW perhaps it would make sense to change the internal/external scriptlet
type thing into a bitfield which tells the hooks how exactly the scriptlet
will get executed. Basically an external script would be something like
(RPMSCRIPT_FORK|RPMSCRIPT_EXEC) and internal scripts neither, until we add
the ability to fork the Lua execution too, at which point Lua scripts get
RPMSCRIPT_FORK (but no EXEC). Something like that should remove any
ambiguosity from what internal and external
script actually means.
Then there's a bunch of indentation issues, at least these:
- missing indentation level on all the new rpmpluginsCallScriptletFoo() for
loops, eg:
for (i = 0; i plugins-count; i++) {
name = plugins-names[i];
RPMPLUGINS_SET_HOOK_FUNC(PLUGINHOOK_SCRIPTLET_PRE);
if (hookFunc(s_name, type) == RPMRC_FAIL)
rc = RPMRC_FAIL;
}
..and here:
if (xx == 0) {
xx = execv(argv[0], argv);
}
...and a few cases of soft tabs where hard tabs should be used. Hard vs soft
tab mixup is mostly just a slight annoyance (due to diffs not aligning
perfectly) and not the end of the world, but the indentation levels really
need to be right.
Another minor cosmetics issue I seem to have missed on previous rounds:
the ampersand placement in KR is next to the variable name, not the type.
Eg char* s_name should be char *s_name instead (although a lot of rpm
code has whitespace on both sides, thats ok too)
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.
In a purely security context that certainly makes sense, but in our
case(s) the results tend to be mixups of potential security and miscellanous
other errors. At least to me its not at all obvious which should win in
various circumstances.
One possibility could be a bitfield type return code, where each
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] 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 above :) Best
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
Re: [Rpm-maint] Plugin ponderings
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?
[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] [PATCH 1/2] Extending rpm plugin interface, part 1
The most important use-case is to make it possible to run scripts when even /bin/sh is not available. Such as %pretrans scripts on initial install - there simply nothing to exec() in that environment. Or if the package containing /bin/sh or one of its dependencies needs scripts, there's no other choice than an embedded script during initial install. Related to that, one can avoid problematic external dependencies in the early bootstrap stages of initial install. It also allows some things that aren't really possible otherwise, such as manipulating macros and communicating with other packages through the global Lua environment (and in theory, accessing rpmdb but that's not implemented), which would not be possible in an externally executed script. It would be possible to fork() for the execution of embedded scripts without compromising the main use-case of early install, but it would break some possible uses of the second group. Whether that's a good or bad thing... is a different question. I dunno if anybody is actually using the current possibilities in that area. Thank you for explaining! Now I start to understand this a bit better. Providing better interfaces and possibilities for trusted packages to do needed things is always good, I guess the only thing would be to have the possibility for a plugin to implement restrictions if necessary. I agree that it would be better to make some decision on per script/package basis whenever this lua script is allowed to run and with what privileges instead of simply saying disable lua at compile time . I think the solution that plugin would return an error code n pre-script execution if it thinks that lua scipts aren't allowed is actually an acceptable one for now. The better one would be only to setup the security context but this would require the fork(). Good question... symmetry is usually good, so I guess the post-hook should be always called if pre-hook was called. Script path + return code as arguments seems like a reasonably starting point at least. (I'm not really even dreaming of getting every single detail 100% right the first time :) Ok, I will do it this way. I actually have these changes ready, but wanted to finish with three main filesystem hooks at the same time. I am hoping to do it this week. And like said, I think the plugin hooks should get called with embedded scripts too. It'll probably need another argument to let the plugins know whether an internal or external script is about to be executed - you could eg just deny execution of internal scripts from non-trusted sources then, without disabling lua entirely. Yes, I agree, this is a better alternative to a complete lua disabling. I will do it this way. Do you want the script changes in separate path then without any filesystem hooks? 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
Re: [Rpm-maint] [PATCH 1/2] Extending rpm plugin interface, part 1
Scripts run with all the powers that rpm itself has, and there's not a whole lot that can be done about it: scriptlets expect to, and often need to, do all sorts of crazy things. True, but we would actually like to be able to limit things that scripts can do. It is ok for a script to create a new file or to call ldconfig, but in our setup it is not ok for script to start relabeling the xattrs on filesystem (you can do this with CAP_MAC_ADMIN for example) or rewriting important system files (you can do this with CAP_MAC_OVERRIDE and CAP_DAC_OVERRIDE). The differences from regular exec()'ed scripts and Lua-ones are basically: - As Lua scripts run in the rpm process space, some extra protection is needed, like saving + restoring current working directory, umask and preventing Lua scriptlet from exec()'ing or exit()'ing the process. Just out of interest, is there are reason for them actually to run in rpm process space as opposite to doing the same as for regular scripts? - There are no context switches for Lua scripts in SELinux world currently: Lua scripts run with rpm_exec_t context, regular scripts run with rpm_script_t. But both are practically omnipotent contexts, because they have to be. Stephen, do you have any plans related to Lua-scripts? I guess the reason it wasn't touched so far is the same as for us: when script is run in the same context, very little can be done to make it with little priviledge and still be able to continue rpm operation. Oh, I meant the common pre- and post-hook pattern seems like it would make sense for scripts too in any case, regardless of whether it helps solving the context issue. It *might* in the SELinux case, but a post-hook could have some other entirely different uses, and it'd make the whole thing a bit more symmetric :) Sure, I can make the hooks to follow the same principle. Just I would need to understand how the post-hook would be used in order to figure out proper arguments and decide if we want it to be called if script fails or not. I guess I would rather call post-hook then even if script has failed, but juts indicate the failure. What do you think should be the argument of post-script hook? Return code from the script (to know if it failed or not) and maybe script path again then? 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
Re: [Rpm-maint] [PATCH 1/2] Extending rpm plugin interface, part 1
Oh and one other thing I noticed just now that'll need further thought: currently the script setup hook only runs for external scripts, but not the embedded Lua-scripts. Which are getting more and more common... They'll obviously need to be handled quite differently as they run within the rpm process itself, ie fork() + exec() does not occur. Yes, I don't currently have a very good idea how this case should be handled. The idea of script hook is that it sets the needed security context, but we obviously can't do this for lua case unless we want to drop the whole rpm security context. As a temporal and draconic measure we can compile rpm without lua support to close this hole, but it is no-go in the future since it is getting more and more usage. I guess this is one of the things that I need to think more about. In case of SELinux, AFAICS a process can change its own context back and forth, IF permitted by the policy. So at least in theory it should possible to switch to a different context while executing a scriptlet and then switch back to the original context. It isn't so easy for Smack for example (and for a good reason) and I believe SELinux won't use this functionality for this kind of purpose. A process can change its security context in Smack, if it has CAP_MAC_ADMIN, so rpm (a trusted process) can have this cap and then drop the credentials when running lua script from untrusted package. However, we also want to drop CAP_MAC_ADMIN (and all the rest of sensitive caps) for such lua script (otherwise it can setup whatever context it wants for itself), and after such drop, and after lua script execution is done, rpm can't get its own security context back (because CAP_MAC_ADMIN is lost). This actually makes sense since once you run untrusted code in the same process space you can't really guarantee that you can safely come back and run (in the same process) a trusted code with high privileges. Same logic would apply if we don't speak about LSMs, but traditional Linux DAC. Rpm usually runs as root with all POSIX caps, but suppose we want to drop the sensitive caps while running untrusted lua scripts or even simply run them under different uid/gid. How do we do this with lua scripts currently? Perhaps the script hook should just follow the common pre/post-hook pattern of the other hooks afterall: pre-hook would just replace the current setup hook, and post-hook would run after the script got executed. If we add an extra argument to notify the hooks whether it's an internal or external script (or a more generic flags argument to allow passing more such bits later), the plugin(s) should be able to figure out what to do about it. This would be good, if we can indeed manipulate the context freely, but because of above we really can't. 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
Re: [Rpm-maint] [PATCH 1/2] Extending rpm plugin interface, part 1
Okay then, done and pushed. Now that I looked closer, I spotted (and fixed) a couple of more issues: a tiny memleak from early rpmtsSetupTransactionPlugins() return and some further cosmetics (two soft-tabs instead of one hard-tab, trailing whitespace etc), but nothing dramatic. Thank you! I will seriously try to improve my style. I am not using vim for code edits, but I think I should probably reconsider it or get some kind of editor that shows all symbols explicitly. Pain to read but I get it right at the end :) Oh and one other thing I noticed just now that'll need further thought: currently the script setup hook only runs for external scripts, but not the embedded Lua-scripts. Which are getting more and more common... They'll obviously need to be handled quite differently as they run within the rpm process itself, ie fork() + exec() does not occur. Yes, I don't currently have a very good idea how this case should be handled. The idea of script hook is that it sets the needed security context, but we obviously can't do this for lua case unless we want to drop the whole rpm security context. As a temporal and draconic measure we can compile rpm without lua support to close this hole, but it is no-go in the future since it is getting more and more usage. I guess this is one of the things that I need to think more about. Cool. And thanks for all the work so far :) I hope this is only the beginning, I am really interested in security part of rpm! 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
Re: [Rpm-maint] [PATCH 1/2] Extending rpm plugin interface, part 1
Hi,
Sorry for the late reply: I was on holidays. Sure, go ahead with rpmlog and
indentation changes, if it doesn't bother you to do this!
I will then start concentrating on rest of the stuff: need to do some more
thinking on it to begin with.
Best Regards,
Elena.
-Original Message-
From: Panu Matilainen [mailto:pmati...@laiskiainen.org]
Sent: Thursday, November 01, 2012 10:22 AM
To: Reshetova, Elena
Cc: rpm-maint@lists.rpm.org
Subject: Re: [Rpm-maint] [PATCH 1/2] Extending rpm plugin interface, part 1
On 10/26/2012 12:56 PM, Reshetova, Elena wrote:
Hi,
Please find the corrected patch in the attachment.
I really spend quite some time fighting indentation :( , but let's
hope it succeeded now (I am not that sure since it so much changing
when I open it with different editors/settings).
I was trying to use only soft tabs: 4 spaces, but in places where I
had to integrated in existing code it behaves unpredictable because of
hard tabs that I think in some places aren't of same size (but maybe
this is my inability to use editors).
It's much better now, thanks. There are a couple of misindentations still
but I can just as well tweak those while applying, and its not as if the
whole codebase is perfectly indented, I just want new things to land in a
shape that doesn't need immediate whitespace fixes as they mask away real
changes from eg git blame. I dunno what editor(s) you use, vi(m) is what I
live and breath by - for sure its arcane but at least it doesn't mess with
the formatting you choose :) The only setting I have wrt C in ~/.vimrc is
autocmd BufRead,BufNewFile *.[chi] set sw=4
Apart from indentation-trivia, there's just one issue that I see:
+if (!plugins || rstreq(plugins, )) {
+rpmlog(RPMLOG_INFO, _(Failed to expand %%__transaction_plugins
macro\n));
+ return rc;
+}
This still causes superfluous/bogus messages to be emitted on what I
consider to be the default case of no plugins being configured. I'd rather
just remove the rpmlog() line from there for now, or at least change it to
RPMLOG_DEBUG so it wont show up on regular uses like 'rpm -Uvh'.
If you dont mind me doing a couple of indentation fixes and the rpmlog()
change, I'll just go ahead and commit it with those tweaks. FWIW I'm just
about to branch off for rpm-4.11 (sooner than I originally planned but...),
the plugin changes will at least initially go to master only to give us time
and freedom to fiddle with the details.
- 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] First attempt for the patch on extending the plugin interface for rpm
@Tero, do you still remember the reason why the conflict hook was done in this way? Why wasn't it better to check the conflicts before the transaction and fail the whole transaction if conflicts are seen? I'm not quite sure about the context you are talking about here, but I'll tell what I remember. File conflict hook is called early inside rpmtsPrepare when the security plugin doesn't yet know much about the package. File conflict hook just records the conflict and the decision to go ahead takes place in the FSM hook. Overwriting the conflicting file is allowed if package comes from a higher more trusted domain. If it's not allowed, then FSM hook returns an error and package processing is canceled. The problem is that while failure in FSM is always a possibility, *planning* to fail there is bad as it breaks the one promise rpm gives about transactions: transaction will not start when there are known problems. File conflicts need to be handled early, either by resolving them or aborting the entire transaction before it really even starts. Otherwise various calculations will be off, and by the time a package ends ups in the FSM, some scripts have already been run causing modifications to the system that can't be undone, plus it can cause broken dependencies to other packages. Do you happen to remember what information is missing the conflict resolution stage? I'd guess that would be something internal to the plugin as from rpm POV, pretty much everything that is known about the packages is available in rpmtsPrepare(). Thank you Tero for clarifying! This reminded me the actual reason why decision can't be made in conflict hook for us. I think I just was too tired yesterday because I miss the obvious thing. The information we are missing is the package source (we identify it based on package signature). So, suppose there is a new package X that is about to be installed. It brings file A that conflicts with the previously installed file from package Y. At this moment the verify hook for this package hasn't been called yet, so the plugin doesn't know who signed package X and can't make a decision if it is ok to overwrite a file or not. I wonder if we can somehow pass the package signature to this hook then... I will look into 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
Re: [Rpm-maint] First attempt for the patch on extending the plugin interface for rpm
Hi, After reading your responses I think we indeed need to break this patch into at least two or three parts. It would help us to concentrate on one set of the hooks at the time and integrate them separately in steps. You mentioned that pre/post tsm/psm hooks are pretty much fine, so we can start from them. I would propose to add to them a security plugin loading function and macros. Also, if there are no objections I would add here a ScriptSetup hook (it is rather simple one and outside of other hooks). This can be the first bunch that we can bring into shape to be acceptable for merge. Then after the above is hopefully merged I will start attacking the file-related hooks (I have already made a number of changes and will look into improving them further based on your comments). And at the end we can talk about how to integrate the rest. I will post the first bunch shortly if there are no objections. Further comments are inline: Actually, the ability to sanely disable these things from cli and API is in practise a hard requirement. The reason is that rpm is used in a large variety of situations beyond the basic system package management task: creation of chroot (build-)environments that are nothing like what the system might be running, install/live images, certain per-user tasks, workarounds for weird setups etc. Plus all the things we never even imagined - by now I hope to have learned (the hard way) that no matter how unlikely it seems, the unforeseen oddball (yet legal) cases always exist :) I do realize that needs on an embedded / handheld systems can be drastically different from a general purpose system. How to support full lockdown where needed but allow disabling for other uses is something to think about. Hm.. This isn't easy indeed. Can we make a security plugin a special case regarding to the cli? Meaning that if the rpm is compiled with ENFORCE_SECURITY, then it isn't possible to disable the security plugin from cli and if loading/expanding of security plugin fails, then rpm stops. Providing cli or API option to disable additional security enforced by the plugin(s) even if security have been compiled in sounds scary for me What I was thinking was more in the direction of if plugin X returned failure, should we even call the other hooks (such as falling back to something else if one of the involved filesystem doesn't support feature Y), but this is now something that neither the plugin or rpm has any way of actually knowing. The point perhaps being, the exact semantics for the plugin hooks needs to be far more clearly defined sooner or later. I think even if plugin X returns failure on hook Z, we need to call the hook Z for all other plugins, because this will make sure that all plugins have the same set of hooks that got executed during the rpm transaction. Also, it would be up to each plugin to be prepared that rpm transaction can stop and fail outside of the plugin (inside rpm itself or inside of other plugins) and then be ready that only cleanup hook is called. So, for example, plugin might have pre tsm/psm hooks called and they go fine, but then some other plugin returns a failure on prepsm hook, so whole transaction goes to failure and plugin gets cleanup hook called even if from its point of view everything went fine. FWIW, I've been considering exporting the fd digest API for quite some time. That stuff probably needs a bit of polishing/sanitizing before exporting, but I'm not at all opposed to doing it. Yes, I think this would be good and then it can be cleaner from plugin side to get digest calculated. For now, go for the absolute minimum exposure of rpm objects. The problem here is different from the other internals (such as fsm in the previous versions) exposure, the issue here is that it exposes a ad-hoc set of things that rpm currently happens to internally use to handle this stuff. Which is not entirely unlike Lucas Arts adventure games of old: you have a rope, two bananas and a fish, and you need to somehow use those to cross the ravine to get the umbrella from the other side that you need to... :) The fact that the hook breaks in rpm = 4.10 despite arguments remaining the same kinda underlines the issue. Rpm internals are going through some pretty fundamental changes at the moment, and this is one of the places that will hopefully become saner when its all done (my wishful thinking is to have at least the basics around in rpm 4.11 already). When that happens we can export more, but right now its better to keep things to minimum you can get by with. Agree, I will take a stronger attempt to reduce the arguments to the actual needed once for all the file hooks. I guess the conflict one is the most heavy one at the moment, so I will try to address it properly next time I post the patch for file hooks. The signature verification hook is another place where some weird internal bits and pieces get passed around. That might be unavoidable,
[Rpm-maint] First attempt for the patch on extending the plugin interface for rpm
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.
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.
-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?
- 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.
From 0c04ff99a00e747899137be41dca5eae72e423e4 Mon Sep 17 00:00:00 2001
From: Elena Reshetova elena.reshet...@intel.com
Date: Mon, 8 Oct 2012 14:21:53 +0300
Subject: [PATCH] Extending the plugin interface
---
lib/fsm.c | 42 ++--
lib/package.c | 14 ++-
lib/psm.c |2 +-
lib/rpmplugins.c| 228
+++
lib/rpmplugins.h| 136 +-
lib/rpmscript.c | 18 ++--
lib/rpmscript.h |2 +-
lib/rpmte.c | 14 ++-
lib/transaction.c | 52 ++
macros.in |7 ++
plugins/Makefile.am |5 +-
plugins/plugin.h| 22 +
plugins/sec-sample-plugin.c | 82
13 files changed, 599 insertions(+), 25 deletions(-)
create mode 100644 plugins/sec-sample-plugin.c
diff --git a/lib/fsm.c b/lib/fsm.c
index 5ebf28d..e1cec85 100644
--- a/lib/fsm.c
+++ b/lib/fsm.c
@@ -718,11 +718,12 @@ static int fsmMapAttrs(FSM_t fsm)
/** \ingroup payload
* Create file from payload stream.
+ * @param ts rpm transaction
* @param fsm file state machine data
* @param archive payload archive
* @return 0 on success
*/
-static int expandRegular(FSM_t fsm, rpmpsm psm, rpmcpio_t archive, int
nodigest)
+static int expandRegular(rpmts ts, FSM_t fsm, rpmpsm psm, rpmcpio_t
archive, int nodigest)
{
FD_t wfd = NULL;
const struct stat * st = fsm-sb;
@@ -753,6 +754,13 @@ static int expandRegular(FSM_t fsm, rpmpsm psm,
rpmcpio_t archive, int nodigest)
rc = CPIOERR_READ_FAILED;
goto exit;
}
+
+/* Run file updated hook for all plugins */
+rc = rpmpluginsCallFileUpdated(ts-plugins, fsm-path, fsm-buf,
len);
+if (rc) {
+ goto exit;
+}
+
if ((Fwrite(fsm-buf, sizeof(*fsm-buf), len, wfd) != len) ||
Ferror(wfd)) {
rc = CPIOERR_WRITE_FAILED;
goto exit;
@@ -1050,14 +1058,15 @@ static int fsmMakeLinks(FSM_t fsm, hardLink_t li)
return ec;
}
-static int fsmCommit(FSM_t fsm, int ix);
+static int fsmCommit(rpmts ts, FSM_t fsm, int ix);
/** \ingroup payload
* Commit hard linked file set atomically.
+ * @param ts rpm transaction
* @param fsm file state machine data
* @return 0 on success
*/
-static int fsmCommitLinks(FSM_t fsm)
+static int fsmCommitLinks(rpmts ts, FSM_t fsm)
{
char * path = fsm-path;
const char * nsuffix = fsm-nsuffix;
@@ -1078,7 +1087,7 @@ static int fsmCommitLinks(FSM_t fsm)
if (li-filex[i] 0) continue;
rc = fsmMapPath(fsm, li-filex[i]);
if (!XFA_SKIPPING(fsm-action))
- rc = fsmCommit(fsm, li-filex[i]);
+ rc =
Re: [Rpm-maint] Unified set of security hooks for rpm
Hi, Adding back SELinux guys (wonder why they got stripped in the initial posting: I blame the corporate mail client) :( One question that we have is what is the best way to define a plugin interface for rpm? Should we define just security-related hooks and embed currently existing collection plugin hooks into it or are there any plans for bigger rpm plugin interface that security hooks should be part of? Good question... The current collection hooks aren't really security at all but something else. Yet the selinux policy plugin does kinda fall into that category, and if there's an entirely different plugin type needed for actually putting the security labels (retrieved by eg collection plugin) and such into place, requiring different plugins to interact somehow, and that I suppose would get ugly real fast. There are some other places that could use plugins as well that would fall into entirely different categories. Just as an example, one such thing is translations of i18n header strings from external sources (eg specspo which exists in at least two different flavors, one upstream and one in Fedora) that could be cleanly handled (and would open up new possibilities) with plugins. Yes, I suppose many use cases can benefit of having a genetic rpm plugin interface, but I guess currently the biggest user of such interface is still security (with selinux already being in upstream rpm and us dragging patches around in order to implement our needs). Just FWIW / food for thought, back some time ago I had a brief look at isolating the security activity into a relatively abstract security manager object (instead of plugin hooks littered all around), which could actually be a stack of different bits and pieces implemented by plugins (SELinux/MSS labels, maybe file capabilities etc) but that ran into all sorts of issues right on the first steps already. Whether I just didn't pursue it hard enough or just not a feasible approach I dunno but suspect the latter to be the case. Hm... I think we can make sure that SELinux and our implementation can be pushed to the security manager. Conceptually I don't see the reason why pushing capabilities or other default security attributes (like uid, gids and etc.) to security manager wont' be possible. Could you tell what kind of issues you have run on the first steps? It isn't going to be an easy and simple exercise, but should be possible to do. As for the actual hooks, I think we just need to look at the existing and planned needs of the security labelling etc stuff one by one and try to figure whether it all needs to go into a giant transaction plugin interface or whether it's actually possible to categorize them into tidier sets. I think we have these main group of hooks: - current per-collection hooks - per transaction hooks (pre-tsm, post-tsm) - per package hooks (called in psm): pre-psm, post-psm, verify - per file hooks (called in fsm) + hook for newly created dirs - script hook (called for any invocation of any script (per-transaction/per package)) Would you prefer to break them per group or? I am not sure what you meant with whether it's actually possible to categorize them into tidier sets.? Some other plugin possibility examples: the different rpmio (de)compression types would fit the plugin use-case spot-on. Whether having them as plugins actually makes sense is another question, but it presents some interesting issues: the rpm file io subsystem lives in librpmio, suggesting the core plugin infrastructure should be there as well, instead of librpm itself. However they would need to be able to register themselves into rpmlib() provides which live in librpm, in the transaction layer... The ability for plugins to register custom rpmlib() provides seems generally useful, eg the SELinux/MSSF stuff might want to do it too and I can imagine other cases as well. And the needs of plugins wanting to advertise themselves through rpmlib() provides differ vastly - from very low-level file io to the rather high-level collections and whatnot. The only thing I know for sure is that there are needs (or at least opportunities) for more than one plugin type, and the different types should be able to share the common basic plugin manipulation (loading etc) infrastructure. Yes, that's why I was asking what would be the right way to go currently about the security hooks. I guess it is also can be made step by step: first unify the security hooks and make sure that all security functionality from LSMs are isolated to the plugin. And then integrate security plugin interface to the generic rpm interface. But of course if we can make it right (to the extend it is even possible) from beginning, it is always better. 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] Unified set of security hooks for rpm
Hi, I have been writing to the list a while back proposing a set of security hooks that we need for rpm in Tizen. I have discussed with SELinux guys (in CC) and we agreed that can should try to define a unified interface that they can also use. This would allow to move currently partly integrated SELinux code outside of rpm to the security plugin code. One question that we have is what is the best way to define a plugin interface for rpm? Should we define just security-related hooks and embed currently existing collection plugin hooks into it or are there any plans for bigger rpm plugin interface that security hooks should be part of? I can start working on the patch to define the combined set of hooks as soon as I am sure I am not going to do smth that is fully unaligned with rpm future plans. 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
Re: [Rpm-maint] Tizen rpm security plug-in interface
Hi, I would like to give an update on the status of the rpm security plugin interface. I have moved the patch to the latest rpm code and will be upgrading it weekly form now on. I have fixed the issue that you commented last time with FSM hooks that were passing the fsm structure in full to the plug-in and now it passes only the needed information. Also, I have changed some parts related to the signature handling in the plug-in itself. The code is here in one commit (together with the plug-in code to illustrate the use of hooks): https://github.com/ereshetova/rpm/commit/5f4bb06a617fefceeb2656de3f11d9dd657 e352c I would really like to bring this code to shape now (since I have time right now) and would be very thankful for all the comments! Please don't pay attention to the cosmetics yet, only to the functionality since this is far from being a ready patch :) Could you also suggest who is currently maintaining (if any) the SElinux part in rpm? I would like to discuss if the current set of hooks is enough or smth needs modification. I also updated the wiki (https://github.com/ereshetova/rpm/wiki ) and made a separate page for the hooks description: https://github.com/ereshetova/rpm/wiki/Security-Hooks-for-rpm Best Regards, Elena. -Original Message- From: rpm-maint-boun...@lists.rpm.org [mailto:rpm-maint-boun...@lists.rpm.org] On Behalf Of Reshetova, Elena Sent: Tuesday, November 01, 2011 12:52 PM To: Panu Matilainen Cc: rpm-maint@lists.rpm.org; Ware, Ryan R Subject: Re: [Rpm-maint] Tizen rpm security plug-in interface Hi Panu, Thank you for the reply! Unfortunately from time to time I have to start my mails to some people with exactly same words, so I understand very well that you mean. I personally think that selinux needs aren't very much different than our needs and we can come up with a set of security hooks that should be enough for both. Do you know if selinux people or people who made selinux implementation for rpm will be willing to collaborate with us to help defining a sound set of hooks or to verify that set we will be defining is suitable for them? Now about answering your questions about the hooks. I have written a short summary of how hooks are used or reasoning behind them. I am also attaching the whole patch, where the hooks are implemented that you can look to the implementation of hooks themselves. __ +RPMSECURITY_GET_HOOK_FUNC(SECURITYHOOK_INIT_FUNC); This is basic initialization function for the plug-in. +RPMSECURITY_GET_HOOK_FUNC(SECURITYHOOK_FILE_CONFLICT_FUNC); The reason why this hook is needed is rather simple. Yes, rpm doesn't allow this by default, but since we enforce mandatory access control, then even if the user uses force option, we want to make sure that for example some important system executables or files can't be overwritten. The example of the attack it is guarding against is that user can try to install a package that overwrites sensitive system settings file that can allow him to circumvent security. This attack can be very typical for the attacker models, where user of the device isn't trusted enough to modify system settings or security policies. In msm this hook is used to make populate a hash list of file path conflicts with a new conflict information (like a path and sw source of the package that conflicts). Later, when package signature is verified and we are sure where the package comes from, the plug-in can make a decision whatever this package introduces a security conflict and installation must be aborted. +RPMSECURITY_GET_HOOK_FUNC(SECURITYHOOK_PRE_TSM_FUNC); This hook isn't really used by msm apart that basic check is done on internal structure existence. However, I think the hook may be useful since other security plug-ins may want to include some checks before bunch of packages are installed. +RPMSECURITY_GET_HOOK_FUNC(SECURITYHOOK_VERIFY_FUNC); The reason for this hook comes from the fact that plugin maintains what we call a device security policy that arranges known software sources in a form of a tree with different trust levels assigned. This is needed because gpg can't give us any order and we want to build a hierarchical tree of software sources and define the rules, if package from one source can overwrite a package from another. Based on this information, a lot of decisions are done, if package should be installed or not and if applications from this package are allowed to request certain types of accesses on the platform. So, in this hook, msm plug-in simply maps the package to one of its internal software sources and does an additional verification of the key fingerprint. It is currently stored in device security policy too, but I think this part can be removed from plug-in later when proper certificate manager is used to secure code signing certificates. Does this explain a bit better the need of this hook? I also have things written that explain what
Re: [Rpm-maint] Tizen rpm security plug-in interface
Hi Panu, I have finally managed to bring the code repository to the place where everyone should be able to access. Also in the wiki of the project, there is some documentation explaining the design. I will be updating it still today (one more different wiki format to convert :( ). The master branch in the repo corresponds to the upstream rpm repository, the branch security-changes contains the security changes (including the plug-in implementation) put on top of the upstream branch. Let me know if you have any questions! https://github.com/ereshetova/rpm/ Best Regards, Elena. -Original Message- From: rpm-maint-boun...@lists.rpm.org [mailto:rpm-maint-boun...@lists.rpm.org] On Behalf Of Reshetova, Elena Sent: Tuesday, November 01, 2011 12:52 PM To: Panu Matilainen Cc: rpm-maint@lists.rpm.org; Ware, Ryan R Subject: Re: [Rpm-maint] Tizen rpm security plug-in interface Hi Panu, Thank you for the reply! Unfortunately from time to time I have to start my mails to some people with exactly same words, so I understand very well that you mean. I personally think that selinux needs aren't very much different than our needs and we can come up with a set of security hooks that should be enough for both. Do you know if selinux people or people who made selinux implementation for rpm will be willing to collaborate with us to help defining a sound set of hooks or to verify that set we will be defining is suitable for them? Now about answering your questions about the hooks. I have written a short summary of how hooks are used or reasoning behind them. I am also attaching the whole patch, where the hooks are implemented that you can look to the implementation of hooks themselves. __ +RPMSECURITY_GET_HOOK_FUNC(SECURITYHOOK_INIT_FUNC); This is basic initialization function for the plug-in. +RPMSECURITY_GET_HOOK_FUNC(SECURITYHOOK_FILE_CONFLICT_FUNC); The reason why this hook is needed is rather simple. Yes, rpm doesn't allow this by default, but since we enforce mandatory access control, then even if the user uses force option, we want to make sure that for example some important system executables or files can't be overwritten. The example of the attack it is guarding against is that user can try to install a package that overwrites sensitive system settings file that can allow him to circumvent security. This attack can be very typical for the attacker models, where user of the device isn't trusted enough to modify system settings or security policies. In msm this hook is used to make populate a hash list of file path conflicts with a new conflict information (like a path and sw source of the package that conflicts). Later, when package signature is verified and we are sure where the package comes from, the plug-in can make a decision whatever this package introduces a security conflict and installation must be aborted. +RPMSECURITY_GET_HOOK_FUNC(SECURITYHOOK_PRE_TSM_FUNC); This hook isn't really used by msm apart that basic check is done on internal structure existence. However, I think the hook may be useful since other security plug-ins may want to include some checks before bunch of packages are installed. +RPMSECURITY_GET_HOOK_FUNC(SECURITYHOOK_VERIFY_FUNC); The reason for this hook comes from the fact that plugin maintains what we call a device security policy that arranges known software sources in a form of a tree with different trust levels assigned. This is needed because gpg can't give us any order and we want to build a hierarchical tree of software sources and define the rules, if package from one source can overwrite a package from another. Based on this information, a lot of decisions are done, if package should be installed or not and if applications from this package are allowed to request certain types of accesses on the platform. So, in this hook, msm plug-in simply maps the package to one of its internal software sources and does an additional verification of the key fingerprint. It is currently stored in device security policy too, but I think this part can be removed from plug-in later when proper certificate manager is used to secure code signing certificates. Does this explain a bit better the need of this hook? I also have things written that explain what plug-in does internally with pictures and etc. I hope I will be able to share this with you very soon. I am not sure what you meant with this: One thing I would've kinda expected but dont see here is a hook into package verification (ie rpm --verify), where a security plugin could check that whats on the system matches expectations for the extra security data. +RPMSECURITY_GET_HOOK_FUNC(SECURITYHOOK_PRE_PSM_FUNC); This is one of the most important hooks (before actual package installation), where the plug-in is able to analyse the embeeded security information in the package and make a decision whatever the package should be installed
