On Sat, 2010-02-06 at 22:22:56 +1100, Russell Coker wrote:
> On Sat, 6 Feb 2010, Guillem Jover <[email protected]> wrote:
> > Are the SE Linux file contexts handled like normal file attributes,
> > that get propagated with rename(2) and only get reset on unlink(2) or
> > with an explicit SE Linux call? Or are they handled differently and
> > do get reset on rename(2) (I thought SE Linux was not path based)?
>
> rename(2) and link(2) make no difference to the label. unlink(2) doesn't
> change it either. Access checks are performed on open handles to unlinked
> files - when a process calls exec() file handles may be closed by SE Linux
> because the process transitioned to a new domain and is no longer granted
> access to the file (or isn't permitted to inherit the file handle).
> fsetfilecon(3) may be called on an unlinked file, as access controls in SE
> Linux are performed on all accesses if an unlinked file has it's context
> changed then processes that are accessing it may suddenly get their access
> denied with EPERM.
Yeah, sorry for the confusing question, there was an implied question
as if the contexts would be remembered based on their paths and applied
whenever seen (clarified now anyway). It's just that the current code
and comments in there made it more difficult to understand how this
actually works.
> > (Or I guess in other words are they associated to the inode or the
> > dentry?)
>
> It's always been the Inode. There is no other sane way to do it.
Thanks, that clarifies it perfectly now.
> > If it's the former then yes the extra setfscreatecon() at the end of
> > tarobject() does not seem to make sense, if it's the latter we have
> > an additional problem, as if tarobject gets interrupted and then it
> > tries to restore the .dpkg-tmp backup then the context will be wrong
> > for that one as rename(2) would reset the context.
>
> rename(2) does not affect the context.
Ok, given my current understanding I've just reworked the current code
to use lsetfilecon() instead (patch attached), there's no need to
restore anything anymore on error. Completely untested as I don't have
or use SE Linux anywhere, but if it seems sane and works fine I'll just
apply it.
Also should a failure from lsetfilecon() ignoring ENOTSUP, be considered
fatal then? I'd say yes but maybe there's some reason not to?
thanks,
guillem
diff --git a/src/archives.c b/src/archives.c
index 0d1d9d4..7a3d43c 100644
--- a/src/archives.c
+++ b/src/archives.c
@@ -51,8 +51,6 @@
#ifdef WITH_SELINUX
#include <selinux/selinux.h>
-static int selinux_enabled=-1;
-static security_context_t scontext = NULL;
#endif
#include "filesdb.h"
@@ -256,6 +254,40 @@ static void newtarobject_allmodes(const char *path, struct TarInfo *ti, struct f
newtarobject_utime(path,ti);
}
+static void
+set_path_selinux_context(const char *matchpath, const char *path, mode_t mode)
+{
+#ifdef WITH_SELINUX
+ static int selinux_enabled = -1;
+ security_context_t scontext = NULL;
+ int ret;
+
+ /* Set selinux_enabled if it is not already set (singleton). */
+ if (selinux_enabled < 0)
+ selinux_enabled = (is_selinux_enabled() > 0);
+
+ /* If SE Linux is not enabled just do nothing. */
+ if (!selinux_enabled)
+ return;
+
+ /* XXX: Well, we could use set_matchpathcon_printf to redirect the
+ * errors from the following bit, but that seems too much effort. */
+
+ /* Do nothing if we can't figure out what the context is, or if it has
+ * no context; in which case the default context shall be applied. */
+ ret = matchpathcon(matchpath, mode & ~S_IFMT, &scontext);
+ if (ret == -1 || (ret == 0 && scontext == NULL))
+ return;
+
+ if (strcmp(scontext, "<<none>>") != 0) {
+ if (lsetfilecon(path, scontext) < 0)
+ perror("Error setting security context for next file object:");
+ }
+
+ freecon(scontext);
+#endif /* WITH_SELINUX */
+}
+
void setupfnamevbs(const char *filename) {
fnamevb.used= fnameidlu;
varbufaddstr(&fnamevb,filename);
@@ -593,38 +625,6 @@ int tarobject(struct TarInfo *ti) {
* its original filename.
*/
-#ifdef WITH_SELINUX
- /* Set selinux_enabled if it is not already set (singleton) */
- if (selinux_enabled < 0)
- selinux_enabled = (is_selinux_enabled() > 0);
-
- /* Since selinux is enabled, try and set the context */
- if (selinux_enabled > 0) {
- /*
- * well, we could use
- * void set_matchpathcon_printf(void (*f)(const char *fmt, ...));
- * to redirect the errors from the following bit, but that
- * seems too much effort.
- */
-
- /*
- * Do nothing if we can't figure out what the context is,
- * or if it has no context; in which case the default
- * context shall be applied.
- */
- if( ! ((matchpathcon(fnamevb.buf,
- (nifd->namenode->statoverride ?
- nifd->namenode->statoverride->mode : ti->Mode)
- & ~S_IFMT, &scontext) != 0) ||
- (strcmp(scontext, "<<none>>") == 0)))
- {
- if(setfscreatecon(scontext) < 0)
- perror("Error setting security context for file object:");
- }
- }
-#endif /* WITH_SELINUX */
-
-
/* Extract whatever it is as .dpkg-new ... */
switch (ti->Type) {
case NormalFile0: case NormalFile1:
@@ -710,6 +710,11 @@ int tarobject(struct TarInfo *ti) {
default:
internerr("unknown tar type '%d', but already checked", ti->Type);
}
+
+ set_path_selinux_context(fnamevb.buf, fnamenewvb.buf,
+ nifd->namenode->statoverride ?
+ nifd->namenode->statoverride->mode : ti->Mode);
+
/* CLEANUP: Now we have extracted the new object in .dpkg-new (or,
* if the file already exists as a directory and we were trying to extract
* a directory or symlink, we returned earlier, so we don't need
@@ -754,6 +759,7 @@ int tarobject(struct TarInfo *ti) {
ohshite(_("unable to make backup symlink for `%.255s'"),ti->Name);
if (lchown(fnametmpvb.buf,stab.st_uid,stab.st_gid))
ohshite(_("unable to chown backup symlink for `%.255s'"),ti->Name);
+ set_path_selinux_context(fnamevb.buf, fnametmpvb.buf, stab.st_mode);
} else {
debug(dbg_eachfiledetail,"tarobject nondirectory, `link' backup");
if (link(fnamevb.buf,fnametmpvb.buf))
@@ -766,21 +772,6 @@ int tarobject(struct TarInfo *ti) {
* in dpkg-new.
*/
-#ifdef WITH_SELINUX
- /*
- * if selinux is enabled, try and set the default security context
- * for the renamed file
- */
- if (selinux_enabled > 0)
- if(scontext) {
- if(setfscreatecon(scontext) < 0)
- perror("Error setting security context for next file object:");
- freecon(scontext);
- scontext = NULL;
- }
-
-#endif /* WITH_SELINUX */
-
if (rename(fnamenewvb.buf,fnamevb.buf))
ohshite(_("unable to install new version of `%.255s'"),ti->Name);
@@ -791,17 +782,6 @@ int tarobject(struct TarInfo *ti) {
*/
nifd->namenode->flags |= fnnf_placed_on_disk;
-
-#ifdef WITH_SELINUX
- /*
- * if selinux is enabled, restore the default security context
- */
- if (selinux_enabled > 0)
- if(setfscreatecon(NULL) < 0)
- perror("Error restoring default security context:");
-#endif /* WITH_SELINUX */
-
-
nifd->namenode->flags |= fnnf_elide_other_lists;
debug(dbg_eachfiledetail,"tarobject done and installed");
