On 01/09/2013 08:19 PM, Reshetova, Elena wrote:
Hmm, do you mean FSM_INIT hook is the "pre" hook and FSM_COMMIT the "post"
hook, or that both INIT and COMMIT have their own separate pre- and
post-hooks?

Sorry for being unclear, I just meant that for FSM it is better to have two
hooks before file is put to the filesystem and after as opposite to one
hook.

Ah, yup. Hooks both before and after create/remove is not only better but necessary I think.


If the former, its not symmetric as fsmCommit() does not get called on
removed files, and there are almost certainly other (at least error) paths
where fsmCommit() wont get called. Of course we can make the removal-path
call FSM_COMMIT hook despite not being in fsmCommit(), but ... perhaps the
hook names should follow the pre/post convention then, eg FSM_FILE_PRE and
FSM_FILE_POST.
In either case, I think file removals should be covered by the hooks
too: you might want to strip security-related labels or such before
actually removing, eg to prevent "leaks" through hardlinks (see
removeSBITS() in fsm.c).

I didn't think of removal cases originally, this is really a good point that
we should take them into account, too. The safe choice won't be to strip the
security-related label in this case, because in hardlink case this might
open a hole in a system. For example, if my package has file
"sensitive_data" and plugin labelled it "Sensitive" upon installation.
During run-time only processes that have access to "Sensitive" can
read/write this file. But suppose attacker would make a hard link to the
file and then somehow initiate a package uninstall. Plugin will remove the
file (the first path to it), strip the label and then attacker can access
the file via hardlink because file isn't protected with the correct label
anymore. Moreover in MAC systems you can't just remove a label, since
everything has to have a label, you can only change label. So, I guess the
correct way in this case would be to change the label to some special
"Isolated" label that no one has any rights to read/write/execute. Then even
if hardlink is left, it is unusable after package uninstallation.

For MAC labels and the like, yes. For some other purposes (eg if we were to move the posix file capabilities handling into a plugin) you'd want to strip. While the immediate prime motivations for this stuff revolve around MAC systems, lets try to keep other possibilities in mind :)

Perhaps the hooks should take an additional argument to indicate the
operation - create/remove at least, but depending on other things skip might
be needed as well.

OK, so then I will make two hooks FSM_FILE_PRE and FSM_FILE_POST that would
be always called for any file (both install and remove). I will pass the
info about the operation type to the hook together with path and mode.
I guess FSM_FILE_PRE can be called just after FsmInit(), but what would be
the correct place for FSM_FILE_POST? Seems like I would have to call it
twice based on operation performed either in commit() or unlink().

Hmm. It might actually be better to have both install and erase case individually call the hooks where appropriate for better control over symmetry and such: for example in erase case, fsmInit() can and often will get called without it ever unlinking anything. The install case is likely to be quite tricky to get just right because there are all sorts of failure paths and whatnot, but fsmCommit() is probably where the post hook needs to be. Actually makes me wonder whether it should be the place for pre-hook as well.

There are all sorts of questions and issues here, depending what exactly we want. For the current in-rpm selinux implementation, having both PRE and POST hooks called from fsmCommit() would be sufficient I think. But if you want to use temporary labels while the files are getting unpacked... you might actually need yet another set of similar hooks, eg FSM_UNPACK_PRE|POST that get called when the temporary unpacking suffix is in use, and additional FSM_FILE_PRE|POST calls when the file is getting moved into its final place in fsmCommit().


Yup, path and mode is minimalism to the extreme :) Being able to sanely
pass rpmfi objects would be nice but it requires bunch of other changes
to happen first.

Yes, I guess other arguments would have to wait for changes to come.

BTW it should be possible to pass rpmfi objects if we want even now. It just needs a bit of wrapping to deal with the iterator index madness to ensure the hooks get called with correct indexes and save + restore the former values on entry/exit to the hooks. IIRC there are some funny little quirks related to how rpmfiSetFX() and friends "work", so perhaps its better to concentrate on the "simple arguments only" version first.


Rpm checks signatures when reading packages/headers, so unless signature
checking is disabled, the headers that were fed to
rpmtsAddInstall/EraseElement() are already signature checked. With
caveats. Eg. while rpm generally assumes the re-opened package within
transaction is the same thing as initially added to the transaction,
technially the callback can hand us something else. In which case things
are likely to go not very well :) And then there's the fact that rpm
doesn't have a mechanism to actually enforce signed packages only, etc.

This is smth I didn't know about rpm: never looked into that part before:
was thinking that signature is checked per package one by one.

That's what many/most depsolvers (yum etc) do: they turn off the sigcheck-on-read functionality and then perform the signature check as a separate step. The reasons vary from historical (ancient rpm versions did not have sigcheck-on-read, so it had to be a separate step) to hysterical (misdesigned and/or missing API's to deal with stuff, especially bad in the python bindings) and anything in between...


But this reminds me...

The root issue with MSM delaying conflict checking until its basically
too late was insufficient information available during rpmtsPrepare():
at that point rpm has long since ditched the header object, which you'd
need to get the MSM-specific data to decide if something should be
allowed or not. And the next time the header is available is indeed only
inside the psm/fsm stage deep inside already running transaction.

I think we need to have additional hook(s) in the
rpmtsAddInstall/EraseElement() area where the full header is available
for the cases where additional data is needed by plugins. There's just a
slight problem: these hooks would be called long before the plugins are
currently even initialized, so the plugin initialization would have to
change quite significantly.

Yes, I guess if we go this path, then plugin initialization would have to
happen in rpmtsCreate(). Currently there is only memory for pointer
allocated, but I guess nothing prevents us from doing the loading of plugins
here too. What do you think?

The caveat with initializing from rpmtsCreate() is that transaction sets are used for all sorts of things that have absolutely nothing to do with installing or removing packages, but we have no clue whatsoever about the inteded use in rpmtsCreate(). In fact we dont really know anything at all at that point, as all the transaction parameters are set with other setter functions one by one later on. So it's not a very friendly place to do initialization from, but OTOH it would be rather obvious and central place for doing it. Needs more head-scratching I'm afraid.

Another possibility might be doing a lazy initialization on the first rpmtsAddInstall/EraseElement() call, at which point we know at least a little bit more about our surroundings.

I can move the loading here from rpmtsRun() and also then add hooks in
addinstall and EraseElement. So, these hooks will be called per each package
in transaction and before even transaction starts, and then from header h I
can pass the needed info to the hook, which would make the plugin to be
aware of sw source earlier than now. So, then the conflict hook is called
later, it has all the needed info and if there are some conflicts it can't
allow, it can stop the transaction in advance.

This sounds all good, but there is still small problem left. I still would
like to enforce my own policy on what signatures are ok and not and what are
the action if signature verification fails. Do you think this should be the
part of new hook in addInstallElement? This would be logical since it again
allows us to fail transaction way before it started (if needed) without
installing some packages from it first. In this case verify hook isn't
needed at all.

A good question (and a complex one too), unfortunately I dont have a clear answer to it. The thing is, something being added as a transaction element doesn't necessarily mean an install/erase will be attempted at all, it can be used eg just to test dependency closure of a package set that might be completely unrelated to the system itself. So it shouldn't be overly strict, and probably is not the place to try to enforce any policies.

It depends on what exactly you want to be able to enforce: the signature checking done in rpmReadPackageFile() is the most central place of them all, but that includes things like queries of non-installed packages too, for which you might want a rather different policy than what's allowed to be installed. Another possibility might be checkProblems() in the early rpmtsRun() stage.

If you're referring to the unified package object absraction I think I
mentioned at some point (related to the mess of stuff currently needed
for file conflict calculations), that's basically just a fuzzy cloud
dream at the moment. It'd require massive changes to rpm internals (and
API as well), and I dont have much of a clue when that might happen in
reality so we'll just have to get by what we got now, more or less.

OK, thank you for explaining. It was hard to understand the transition to
this object, but I guess it is normal if it still in "fuzzy cloud dream
state" :)

Sure, just send what you have, even if only as a basis of further
discussion. Looking at an actual patch helps highlighting the bizarre
dark corners of rpm that need dealing with :)

It seems that talking to you first is better than writing patches :)

To get a higher level idea of what might be needed etc before committing to a lot of work, sure. My point was perhaps just more generally that in cases where you have a rough-cut/work-in-progress patch(es) at hand already, there's no harm done sending those as a basis of discussion.

I guess now we might have 3 different patches in a queue: FSM hooks,
changing the plugin initialization, and new hooks for
add/eraseElement. Do you have order preferences for this?

I'd prefer getting the FSM hooks done first, and for two others the order is dictated by practicalities: add/eraseElement hooks cant happen unless the plugin initialization is moved first.

Btw, not related to hooks. I am using rpm from master on my host machine for
testing hooks and etc. Also, I have been porting plugin to your latest dev.
release since we are finally moving to a new rpm (happiness!).
But then it comes to using rpm from master it seems that rpm2cpio gets
broken on my machine: I get plenty of cpio: Malformed number <garbage>. I
discover it when running gbs build for our Tizen environment with rpm from
master. I think this is smth related to lzma, but I don't really understand
what happens. Could it be that it gets disabled for some reason or I didn't
use correct configure options when compiling rpm?

My first guess would be missing XZ development libraries+includes during compilation of rpm. If that's the case, when installing package with 'rpm -U...' you'd get dependency errors on rpmlib(PayloadIsXz) or rpmlib(PayloadIsLzma), but rpm2cpio performs no such sanity checks.

        - Panu -


Best Regards,
Elena.



_______________________________________________
Rpm-maint mailing list
Rpm-maint@lists.rpm.org
http://lists.rpm.org/mailman/listinfo/rpm-maint

Reply via email to