On 10/09/2012 03:26 PM, Reshetova, Elena wrote:
Hi Panu,
Thank you for such fast reply given that you are so busy!
Just trying to make up a bit for all the past delays ;)
My comments are below.
Note that the existing hooks are by no means "sacred" for other than
documenting the needs of the SELinux collection. AFAIK the collections
aren't actually being used anywhere yet (its still marked experimental), and
the interfaces are internal-only so if there's something that needs/wants to
be changed, >they can be changed.
Sure, I was first planning to substitute the OpenTE hook with generic pre
transaction hook,
but then I started to think that there is not enough reasons to touch the
collections hooks because they are making sense
for collections and there is even a collection plugin defined as a special
plugin case. Also, the hooks that I try to introduce
are not conflicting with collection hooks, so I didn't touch them so far.
Yup. My point was just that it IS still ok to change (rather than
painfully work around) the existing hooks if the need rises.
Macros the means through which rpm is generally configured, and that kind
of approach should make it reasonably simple to allow selective
enabling/disabling of them from cli (annoying but inevitable necessity).
Might be more annoying for other API users - currently disabling SELinux is
a simple matter of setting RPMTRANS_FLAG_NOCONTEXTS flag for the
transaction, with plugins it would be something ... quite different.
Yes, it would be different and I think it is good not to allow to disable
the security plugin simply from command line for some transaction. This
would allow the distribution to specify and control what kind of rpm
plugins must be present.
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.
I think this was one of the prime reasons I had that look into the
"security manager" object thingie: to avoid having to pass the transaction
set to places where it really does not belong to.
OTOH it would seem to me (most of) the places could simply be passed a
handle to the plugins instead of ts, as the ts is pretty much only used for
ts->plugins.
True, I think this is that I will do: it still requires adding the plugins
as parameter to functions, but avoids adding ts_internal and ts itself. I
will change my patch.
Ok.
...and this is related to what was one of the issues the "security manager"
thing ran into. Assuming there can be more than one present:
how to meaningfully deal with errors? Returning an error if one or more of
them failed is a simple answer, but whether that's sufficient for the
callee(s) I dont know, I seem to recall some things wanting a better idea
of what failed and how (but that might well be just from staring too much at
the way things are currently done)
Currently I am just returning an error is any of the plugins have returned
an error. This is quite typical approach for security, when multi-layer
security modules are present.
Yup, it's a simple and straightforward rule. Whether it's sufficient
remains to be seen I guess.
I think in this case it would be a responsibility of the plugin to indicate
why an error condition was returned. For MSM plugin an error with
explanation will be printed when smth fails due to plugin.
It might not be the same thing as returning a concrete error, but there is
no way to generalize the errors for different types of plugins (even
security) that it actually makes much more sense.
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.
+
+ /* Run file updated hook for all plugins */
+ rc = rpmpluginsCallFileUpdated(ts->plugins, fsm->path,
+ fsm->buf,
len);
Hmm. Looking at the MSM plugin (from
git://github.com/ereshetova/rpm.git), this is primarily used for
calculating yet another digest for the file being processed. This should be
avoidable, the rpmio FD_t type supports calculating more than one on the
digest on the fly, only the interfaces to use it are currently buried in
rpmio/rpmio_internal.h >(but not actually hidden from ABI).
Yes, it can be used to calculate any digest or hash for the file to be
installed and then used for example to calculate the RSA signature over the
hash and write it to the xattrs for IMA. I will take a look if I can redo
this part with the interface you mentioned.
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.
rc = fsmSetSELabel(sehandle, dn, mode);
-
+ if (!rc)
+ rc = rpmpluginsCallDirCreated(ts->plugins,
+ dn,
mode);
This is a pretty ugly special case hack :) Part of the reason for
fsmSetSELabel() was to isolate the whole labeling thing into something
fairly generic that could be as well in a plugin. So this should be more of
a "post file/dir-creation hook" I think . Also access to such a thing (or a
way to avoid the need there completely...) would be needed in
rpmdbMoveDatabase() too.
Yes, special case hack indeed :) Actually I was thinking of simply calling
the FILE_CLOSED hook (and maybe renaming it to smth better in this case) at
this point, but then I guess I would need to add a special argument to
indicate that this was for dir that wasn't included in the package (it is
important to distinguish this for security reasons since you would likely
label such directory in a different way). I will address this in the next
patch version, too.
Right. For SELinux it hasn't mattered whether a directory was owned or
not. Perhaps it could... although unowned directories are not exactly
infrequent occurence in the real world non-perfect packaging. All it
takes is a simple dependency loop (that might be hard to solve in
packaging) to cause unowned directory to be created before the package
actually owning the directory gets installed.
if (xx == 0) {
- xx = execv(argv[0], argv);
+ /* Run script exec hook for all plugins */
+ if (rpmpluginsCallScriptExec(ts->plugins, argv) !=
RPMRC_FAIL) {
+ xx = execv(argv[0], argv);
+ }
Ah, this is one of the places where I remember pondering over with the
"security manager" experiment: my idea was to bury the execv() call itself
into the manager so it could be execv(), rpm_execcon() or whatever depending
on the manager type/what's loaded, but that didn't fly at all :) This seems
like a >much more feasible approach, but the hook name wants clarification
as the hook MUST NOT actually perform the exec, only setup any preliminaries
it wants for the exec (eg setexeccon() in case of selinux)
Yes, this is what I had in mind now when I wrote this patch. I have noticed
that actually executing the script in plugin won't be possible unless you
want your scripts to be executed n times :) The purpose of the hook now is
only to setup the security context.
I will rename it in the next version to avoid confusion.
Ok.
@@ -409,6 +411,9 @@ static void handleInstInstalledFile(const rpmts
ts, rpmte p, rpmfi fi, int fx,
if (rpmtsFilterFlags(ts) & RPMPROB_FILTER_REPLACEOLDFILES)
rConflicts = 0;
+ /* run file conflict hook for all plugins */
+ rpmpluginsCallFileConflict(ts->plugins, ts, p, fi, otherFi,
otherHeader);
+
This wont work in rpm >= 4.10 which no longer uses the rpmfi
"self-iterator" here - random access to the file info sets is needed and it
can even end up accessing the same file info set at two places
simultaneously (its possible for a package to have file conflicts within
itself via directory symlinks). So you'd >have to actually pass indexes for
both fi and otherFi, but then the indexed accessors to rpmfi aren't exported
in the API currently and...
Ok, sorry that I have missed this from the latest release. I will take a
look on this and try to think what can be done. As minimum we would need
just to know the path of conflicting file and previous package name that
have installed this file.
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.
The signature verification hook is another place where some weird
internal bits and pieces get passed around. That might be unavoidable,
dunno... but then (and I think I've said this before), signature
enforcing mechanism should really be in rpm core (and is sorely missed
currently). Plugins might further enhance / implement policy around
that, or something. Oh and I know this is hand-wavy and vague :-/
Overall I'm still not really comfortable of letting plugins mess around
with such a core functionality of rpm (I do remember the previous discussion
around this though)
In previous discussion you have mentioned that plugins should not get any
rpm internal data structs when it can be avoided (like fsm and etc.) and I
have tried to address this.
Yes, noticed and appreciated :) The current version of the patch is a
vast improvement over the previous one(s) from that POV.
What I referred to above is specifically the file conflict resolution
thing (and our previous discussion about that). Eg looking at the MSM
plugin, AFAICT it can let conflicts pass at this point, yet deny it deep
down from inside when the transaction is already in progress. There
always will be cases where a failure cannot be detected before actually
trying to install (ie, in fsm), but that should be the absolute last
resort, not something you'd ever see in "normal" operation.
I am also willing to work further on this unless
it becomes acceptable for upstream. The actual reason why a security plugin
needs all these hooks is to get enough control over the installation
process.
Like being able to control if a certain file can be substituted from another
package or if a certain package signed with a certain key should get
installed and obtain all requested resources. It is very hard to address any
of these things through rpm itself without starting to ugly strip and add
the code here and there or arrange a bunch of ifdefs. And this of course
makes the maintenance of rpm for a distribution with strict security
requirements a nightmare :(
I understand that for the PC world with root given to the user these things
aren't really that needed. But then you take a look on cases in mobile or
IVI (In-vehicle-infotainment), they are not only applicable, but required by
operators, car manufacturers and even safety requirements. How do we prevent
a user from installing a new cool release of Angry Birds (that just happens
to be malicious) that actually substitutes some of our system binaries,
makes the car to lose the reversing back video stream and makes the user to
reverse over the neighbour's dog? Maybe a bit too harsh example, but with
modern systems it might be possible soon unless we put proper security
mechanisms in place :(
I understand these concerns (I think :), and tightening rpm security I
certainly have nothing against. I'm just perhaps looking at this from
slightly different perspective:
Many of these issues actually exist on general purpose systems as well:
for example you wouldn't generally want a 3rd party package from
somewhere to be able to replace (whether through updating or obsoleting)
a "system" package or its files. Or have that 3rd party package prevent
an important update that happens to conflict with a file that the 3rd
party package has already "claimed". So the kind of package priorization
by its source / signature and related policies is a much more generic
need than specialized handheld/embedded systems, the allowability (and
need) of overriding is what differs more (BTW, how does the user of an
embedded IVI or handheld device specify an rpm conflict override to
begin with?)
That's why I'm a bit doubtful over some things I see here: plugins
should not (have to) be by design doing strange things behind rpms back
to achieve what are actually common needs.
- Panu -
_______________________________________________
Rpm-maint mailing list
[email protected]
http://lists.rpm.org/mailman/listinfo/rpm-maint
