[Rpm-maint] [PATCH] selinux: reopen label between transactions if necessary (RhBug: 746073)

[Panu Matilainen]

On 12/20/2011 04:14 PM, Ales Kozumplik wrote:
> ---
>   lib/rpmts.c          |    9 +++++++++
>   lib/rpmts_internal.h |    8 ++++++++
>   lib/transaction.c    |    9 ++++++++-
>   system.h             |    6 +++++-
>   4 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/lib/rpmts.c b/lib/rpmts.c
> index 25ce83d..09e263a 100644
> --- a/lib/rpmts.c
> +++ b/lib/rpmts.c
> @@ -771,6 +771,15 @@ void rpmtsSELabelFini(rpmts ts)
>   #endif
>   }
>
> +rpmRC rpmtsSELabelReinit(rpmts ts, const char *path)
> +{
> +    if (rpmtsFlags(ts)&  RPMTRANS_FLAG_NOCONTEXTS)
> +	return RPMRC_OK;
> +
> +    rpmtsSELabelFini(ts);
> +    return rpmtsSELabelInit(ts, path);
> +}
> +
>   rpm_tid_t rpmtsGetTid(rpmts ts)
>   {
>       rpm_tid_t tid = (rpm_tid_t)-1;  /* XXX -1 is time(2) error return. */
> diff --git a/lib/rpmts_internal.h b/lib/rpmts_internal.h
> index 438fd46..3ff1af8 100644
> --- a/lib/rpmts_internal.h
> +++ b/lib/rpmts_internal.h
> @@ -102,6 +102,14 @@ rpmRC rpmtsSELabelInit(rpmts ts, const char * path);
>    */
>   void rpmtsSELabelFini(rpmts ts);
>
> +/** \ingroup rpmts
> + * Reinitialize selabel
> + * @param ts		transaction set
> + * @param path		path to contexts file
> + * @return		RPMRC_OK on success, RPMRC_FAIL otherwise
> + */
> +rpmRC rpmtsSELabelReinit(rpmts ts, const char * path);
> +
>   #ifdef __cplusplus
>   }
>   #endif
> diff --git a/lib/transaction.c b/lib/transaction.c
> index 88219b7..a71a286 100644
> --- a/lib/transaction.c
> +++ b/lib/transaction.c
> @@ -1254,8 +1254,10 @@ static int rpmtsSetup(rpmts ts, rpmprobFilterFlags ignoreSet)
>   	(void) rpmtsSetFlags(ts, (rpmtsFlags(ts) | _noTransScripts | _noTransTriggers | RPMTRANS_FLAG_NOCOLLECTIONS));
>
>       /* if SELinux isn't enabled, init fails or test run, don't bother... */
> -    if (!is_selinux_enabled() || (rpmtsFlags(ts)&  RPMTRANS_FLAG_TEST)) {
> +    if (!is_selinux_enabled() || (rpmtsFlags(ts)&  RPMTRANS_FLAG_TEST) ||
> +	(selinux_status_open(0)<  0)) {
>           rpmtsSetFlags(ts, (rpmtsFlags(ts) | RPMTRANS_FLAG_NOCONTEXTS));
> +	rpmlog(RPMLOG_DEBUG, "Selinux disabled.\n");
>       }
>
>       if (!(rpmtsFlags(ts)&  RPMTRANS_FLAG_NOCONTEXTS)) {
> @@ -1284,6 +1286,7 @@ static int rpmtsFinish(rpmts ts)
>   {
>       if (!(rpmtsFlags(ts)&  RPMTRANS_FLAG_NOCONTEXTS)) {
>   	rpmtsSELabelFini(ts);
> +	selinux_status_close();
>       }
>       return rpmChrootSet(NULL);
>   }
> @@ -1384,6 +1387,10 @@ static int rpmtsProcess(rpmts ts)
>   	rpmlog(RPMLOG_DEBUG, "========== +++ %s %s-%s 0x%x\n",
>   		rpmteNEVR(p), rpmteA(p), rpmteO(p), rpmteColor(p));
>
> +	if (selinux_status_updated()>  0) {
> +	    rpmtsSELabelReinit(ts, selinux_file_context_path());
> +	}
> +
>   	failed = rpmteProcess(p, rpmteType(p));
>   	if (failed) {
>   	    rpmlog(RPMLOG_ERR, "%s: %s %s\n", rpmteNEVRA(p),
> diff --git a/system.h b/system.h
> index 9b23e45..228ad95 100644
> --- a/system.h
> +++ b/system.h
> @@ -79,6 +79,7 @@ char * stpncpy(char * dest, const char * src, size_t n);
>   #if WITH_SELINUX
>   #include<selinux/selinux.h>
>   #include<selinux/label.h>
> +#include<selinux/avc.h>
>   #else
>   typedef	char * security_context_t;
>
> @@ -95,7 +96,10 @@ typedef	char * security_context_t;
>
>   #define selabel_lookup_raw(_hnd, _scon, _key,_type)	(-1)
>
> -#define selinux_file_context_path() (0)
> +#define selinux_file_context_path()    (0)
> +#define selinux_status_open(_fallback) (-1)
> +#define selinux_status_close()
> +#define selinux_status_updated()       (-1)
>
>   #define rpm_execcon(_v, _fn, _av, _envp)	(0)
>   #endif

No objections to what this does, just a couple of comments:

rpmtsSELabelInit() closes the handle if it exists, so I think it would 
be sufficient to just call that instead of introducing the new reinit 
function.

Also unless I'm missing something the selinux_status_open() and _close() 
calls could just as well be put into rpmtsSELabelInit() and 
rpmtsSELabelFini() - no other reason than keeping the selinux stuff as 
local as possible. On a related note, we're not checking 
rpmtsSELabelInit() return code in rpmtsSetup() but we should (disable 
selinux for that transaction if it fails)

	- Panu -