[Rpm-maint] FSM hooks for rpm plugin

[Panu Matilainen]

On 01/08/2013 02:26 PM, Reshetova, Elena wrote:
> Hi Panu,

Hi!

>
> I finally got back to file hooks and tried to look into this with a fresh
> head.

Fresh head often helps. I'm afraid my head is a bit too fresh now, as in 
"details blissfully forgotten" :) I'll need to go back and re-read our 
previous discussions on the topic, but some preliminary comments...

> 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

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?

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).

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.

>
> - 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).

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.

> 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().

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.

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.

> 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?

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.

>
>
> For FSM hooks, if there are no objections, I can send you a patch tomorrow
> for a review.

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 :)

	- Panu -