Niall,
thanks for the comments. That'll teach me to blindly port JohnF's code.
I will change this to the piece of code I wrote for gnome-desktop.
You are right for the case where the env var TRUSTED_SESSION is not
defined trusted_session will remain null and the getenv call will happen
multiple times. I have changed this to the following.
static gboolean
g_tsol_is_multi_label_session (void)
{
static char *trusted = -1;
if (trusted < 0) {
if (getenv ("TRUSTED_SESSION")) {
trusted = 1;
} else {
trusted = 0;
}
}
return trusted ? TRUE : FALSE;
}
This way the getenv is only ever called once. After that it has been
initialised and the function will return the appropriate value.
I don't see the need to conditionally compile this code, as you say
there are no additional deps introduced.
Since you mention pushing upstream I'd like to remind people that TJDS
is very much Solaris specific right now. Once we have everything out on
opensolaris.org I will be starting a discussion about how to push these
changes back upstream. This may involve a partial redesign to
accommodate SElinux and/or other security architectures that the GNOME
project may be interested in.
Thanks again,
Stephen.
On Wed, 2006-11-01 at 13:30, Niall Power wrote:
> Hi Stephen,
>
> I have a couple of questions about this patch.
>
> The first is a minor nit:
>
> ++static gboolean
> ++g_tsol_is_multi_label_session (void)
> ++{
> ++ static char *trusted_session = NULL;
> ++
> ++ if (!trusted_session)
> ++ trusted_session = getenv("TRUSTED_SESSION");
> ++
> ++ if (!trusted_session)
> ++ return FALSE;
> ++
> ++ return TRUE;
> ++}
> ++
>
> It's hard to figure out if this function is called multiple times
> or just once, from looking at the diff. I see it called in
> _g_module_open(). Is it called once or potentially many times?
> If multiple times, then it costs a getenv() system call each time
> _g_module_open() is invoked on a non trusted system. It seems like
> a performance hit for the most common case.
>
> The second nit is that I don't see any conditional pre-processor
> macros around this code or configure.in environment checks.
> If we want to get this patch back upstream then we might want to
> consider this. But since there are no trusted extensions specific
> library dependencies introduced, maybe it's not such a big deal.
>
> Otherwise, looks good.
>
> Thanks,
> Niall.
>
> On Wed, 2006-11-01 at 13:28 +0000, Stephen Browne wrote:
> > Hi,
> >
> > I thought I'd start you off with one of our more simple Trusted
> > Extensions patches. :)
> >
> > Please take a look at the following NEW patch for glib.
> >
> > This patch is to improve the security of the module loading aspect of
> > glib so that trusted code can not be extended by arbitrary modules and
> > left open to the case where rogue code can be inserted into a trusted
> > component.
> >
> > In a trusted multi-label session, indicated by the env var
> > TRUSTED_SESSION, glib will restrict the locations that it will accept
> > modules from by first asking the runtime linker for the information it
> > has been configured with and falling back to some default file system
> > locations.
> >
> > Thanks,
> >
> > Stephen.
>
