Re: locking workaround patch

[Michael Meeks <michael ximian com>]

Hi Havoc,

On Wed, 2002-09-11 at 15:20, Havoc Pennington wrote:
> > 	The ORBit2 code scans in a race-free way for a correctly attributed
> > user-owned directory, and this is also the directory that b-a-s uses.
> > It'd be nice to unify on this, so it's localised - oh and it solves the
> > DOS problem at a stroke ;-)
> 
> I looked at that code - the ORBit way doesn't fix gconf for the
> following reason. What it does is create:

	Um; did you in fact read the code that I'm reading ? ultimately the
code in ORBit2 is there to solve precisely the same problem - so if
there is a problem in it - proposing not fixing it, and using your own
version is anti-social in the extreme ;-)

>  /tmp/orbit-username
>  /tmp/orbit-username-1
>  /tmp/orbit-username-2
> 
> until it finds one with right perms.

	This is not what it does in fact; see ORBit2/src/orb/GIOP/giop.c before
(and after) creating any directory we do:

/*
 *   In the absence of being told which directory to
 * use, we have to scan /tmp/orbit-$USER-* to work out
 * which directory to use.
 */
static char *
scan_socket_dir (const char *dir, const char *prefix)
...
		/* Check it's credentials */
...
		/* Sort into some repeatable order */

	ie. we scan that directory; for anything matching, filter by
credentials and then sort into order.

	We do this - always, before we select which to use. Hopefully we will
avoid this cost in future by propagating from b-a-s to child via a
command line switch to ORBit2.

> However, in gconf say malicious user creates /tmp/gconf-username, 
> and we start using /tmp/gconf-username-1. Then malicious user deletes
> /tmp/gconf-username. Subsequent apps to start up will use
> /tmp/gconf-username instead of /tmp/gconf-username-1

	That's not what I get out of the control flow. Indeed - the code would
not fix the DOS it was designed to fix if this was the case. Every app
when it starts scans /tmp/orbit-$USER* for valid matches in sort order.
It's not possible for a malicious user to disrupt that (I hope).

> and not find
> gconfd, and create duplicate gconfd. Thus effectively DOSing an
> existing session. Point is that all gconf-using apps need to find the
> same /tmp/whatever.

	As do all b-a-s clients, which is again what it's for in fact [ ORBit
couldn't really care less, apart from the permissions issue ].

> The only way around that I see is to glob /tmp/gconf-username* and try
> to use anything found in the glob, in ascending order of the little
> number at the end - that could work. I'm not sure though.

	Which is effectively what we do - but we use sort order instead of
'incrementing glob' - since it's very easy to make the incrementing
directories maliciously; and so we use our (fairly hard to guess ObjId
generation):

			ORBit_genuid_buffer ((guint8 *)&id, sizeof (id),
					     ORBIT_GENUID_OBJECT_ID);

			newname = g_strdup_printf (
				"%s/%s-%4x", PATH_ROOT, dirname, id.b);

	To make the next directory [ before re-scanning to avoid the race ].
Now I think about it there is perhaps a remaining race in there
if we do a double create hmm; we need to have a precise time element
instead of the random bit I suppose. Fixes for that welcome,

	HTH,

		Michael.

-- 
 mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot