On Wed, 2006-11-01 at 14:51, Stephen Browne wrote:
> 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;
whoops typo in my correction :)
static int 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.
> >
