[PATCH][saslauthd] cyrus-sasl-2.1.10/saslauthd credential caching

[Rob Siemborski]

On Wed, 15 Jan 2003, Jeremy Rumpf wrote:

> Ok, I've set things up on a Solaris 8 machine for testing. After a few tweaks,
> things appear to be working ok. One thing though, it seems that the maximum
> shared memory segment size on Solaris is 1M by default. Solaris users will
> have to tweak their system settings if a larger cache is desired. Due to
> this, I've set the default cache size to under 1M (down from ~4M). This way
> saslauthd will always start with the cache enabled under default
> circumstances.

I've taken a look at this new code, and I have a number concerns right off
the bat.  (The idea is sound, I think, but it looks like the current
implementation is a security nightmare).

The biggie:

cache_lookup() writes the user-provided password hash into the hash table
BEFORE it is verfied.  There's now a race where an attacker can submit an
arbitrary string as a password, (if there's currently an entry in the
cache, it will be evicted), then while the real lookup is happening he'll
try a second authentication, which will succeed if cache_purge has not yet
been called.

I think I have a general problem with cache_lookup writing to the hash
table, I'd expect "lookup" to be a read-only operation.  If it fails, we
do a lookup to the backend anyway, and then only if we SUCCEED in that
lookup do we write the good password to the table.

Another important one:

No locking of any kind is used on the shared memory segment.  I'm not sure
this is directly exploitable, but it could give odd behavior.

Let's assume that we've fixed the above problem.  Now we have a
cache_write() (that looks similar to the writing portion of the
current cache_lookup()) instead of a cache_purge().

If two processes are in cache_write at the same time, there's a race where
they could both pick the same block to evict, and the result could be some
mangled combination of both (or one or the other, but not both).

Take this for instance (we assume that we had two cache misses, and that
we only have one processor):

	processes
time	(a)		(b)
1	find old bucket
2			find old bucket

	-they now have the same bucket-

3	compute pw hash
4	write pw hash
5			compute pw hash
6			write pw hash
7			write offsets
8	write offsets
9	write creds
10			write creds

	-uh-oh, now we have a bucket with offesets for one entry but
	-credentials and password for another

11	update timestamp
12			update timestamp

There clearly needs to be some sort of semaphore on the table (or on
entries, or something).

Wasn't this originally going to use libmm?

Finally:

I think shared memory is way overkill for the doors version, since it all
takes place within one process.  Though, we'd still need to use some sort
of councurrancy control.

-Rob

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-
Rob Siemborski * Andrew Systems Group * Cyert Hall 207 * 412-268-7456
Research Systems Programmer * /usr/contributed Gatekeeper