[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Possible issues with dpkg SELinux support

On 11/09/2012 10:19 PM, Guillem Jover wrote:
Is the "<<none>>" check needed at all? This has bothered me for a
while, and it's not clear if calling lsetfilecon_raw() with that would
DTRT, or if selabel_lookup_raw() would never return that to start with.

As you already saw, this check is no longer necessary.

Should the code be handling selinux_status_updated(), and reopening
the selabel_handle in such case? If so, how often, per extracted file,
per package processed (probably this usually happens only after
upgrading the refpolicy package?), some other time(s)?

I believe rpm does selinux_status_open() at the beginning of the transaction and selinux_status_close() at the end of the transaction, and during the transaction, checks selinux_status_updated() once per package processed to see whether it needs to re-open the selabel handle. This ensures that rpm gets a fresh view of file_contexts if it is modified during the transaction by a semodule or semanage command.

Should the code be ignoring some other SELinux errors or considering
them warnings when running in non-enforcing mode, or is that already
handled internally by the SELinux interfaces?

This should generally be done within libselinux rather than in the caller.

Is ignoring errors from lsetfilecon_raw() enough of a grave issue as
to warrant a stable dpkg update (can it create security issues, or just
wrongful or too restrictive unpacks)? (I'd be preparing updates for the
current Debian stable and the just-being-prepared-stable releases in
such case.)

It could be a security issue, although obviously no current Debian SELinux users were relying on it.

Also Elena's proposed plugin system did not seem to be directly related
to SELinux, so I've ended adapting rpm_execcon() almost verbatim to the
dpkg case. But something that came to mind is if you think it would make
sense to refactor that function (I've read it's supposed to disappear
anyway) into something slightly more generic so that it could be used by
at least both rpm and dpkg (and possibly other package managers or even
programs invoking helper programs). Something ressembling the adaptation
that I've made in the attached patch, but in addition passing to it (at
least) the fallback context type, it could perhaps have a signature
similar to something like:

   int setexecfilecon(const char *filename, const char *fallback_type);

and be called like:

   rc = setexecfilecon(path, "dpkg_script_t");
   rc = execve(path, argv, envp);


That seems reasonable, except that we'd ideally like to get rid of the hardcoded type altogether and get it from a configuration, preferably supplied by the policy. But the problem of course is what to do before the policy package is installed.

If the attached patch looks fine in principle, I'd ask the Debian
SELinux folks for some testing (as I've only build tested it), and if
they need to somehow adapt the Debian SELinux refpolicy.

Looks mostly correct to me, but the error check for lsetfilecon_raw() should be:
	if (ret < 0 && errno != EOPNOTSUPP)

Reply to: