On Mon, Aug 07, 2000 at 02:14:24PM -0400, Greg A. Woods wrote:
> [ On Sunday, August 6, 2000 at 22:35:33 (-0400), Justin Wells wrote: ]
> > Subject: Re: cvs-nserver and latest CVS advisory (Was: patch to make CVS chroot)
> >
> > It's also not coincidental that pserver performs the authentication
> > separately and then hands control down to the lower level just as ssh
> > would have done.
> >
> > No, pserver isn't sensibly implemented like ssh is. But it does the
> > same thing. You don't do yourself any service by pretending that pserver
> > has a different design flaw than the one it really does have.
>
> You should read up on the set-user-id feature. There's an entire world
> of difference between how SSH authorises a user and how CVS would have
> to do it under your scheme.
>
> There *MUST* be an intervening exec() in order to protect the privileged
> process from being exploited.
If that's all you want to accomplish it wouldn't be much work to move the
pserver code out of CVS into a binary called pserver which performs an
exec() after reading the authorization block. Isn't this what nserver does?
I think that's a good idea. Less code to audit.
By the way, I looked over the pserver code and found potential stack
overflows only in the getline() code, and then only if there is some
error reading from the stream. I patched my copy of pserver so that if
there is an error reading the stream during the authentication process
it immediately calls exit(0), removing the potential overflow problem.
There's not THAT much code to audit once my patch is applied:
-- I didn't bother auditing the code which checks the environment
variables and command lines because I figure the person who sets
up inetd is trustworthy
-- I didn't bother auditing any of the code that is guaranteed to run
as a non-root user inside a chroot jail
That leaves just the pserver_authenticate_connection() method to audit.
I should probably stop auditing it since this sounds like something the
nserver people have probably done.
Justin
