Running dropbear as unprivileged user to a single user SSH Server

[Antoine Catton]

On Wed, 2011-08-31 at 06:46 -0500, Rob Landley wrote:
> Why does ENABLE_SINGLEUSER_ROOT exist?  If somebody can set environment
> variables for the root user, there's plenty of other stuff they can do,
> is there any point in switching this _off_ for root?
> 

The point of my patch is to run dropbear as single user. So, when you
log in “singleuser dropbear”, you are logged as the user running
dropbear.

But if the user running dropbear is root, I thought it could be a
security issue, and it could lead to my patch refusal. But if it's not,
I can remove this part.

> What's the point of the log messages?  (Isn't the point of dropbear that
> it's small and simple?)

Actually, I got to admit it was for my personal need to have a feedback
(not as developer, as user). But if it's not the point of dropbear,
you're right, I should remove these log messages.

> 
> In generally you seem to be m_free()-ing a lot right before assigning
> it, but the context of the hunks you're inserting stuff in aren't doing
> m_free() before their assignments.  Is there a reason for this?

Yes, most of my hunks are some value overriding. And this values are
read from a buffer or something like that. Moreover, by reading these
values, it goes forward on the buffer. So I kept the original code and
overridden the values after it. And because this variables are already
allocated, I did some m_free().
Maybe there's a nicer way to do it, I don't know.

> 
> Just to look closer at one hunk:
> 
> > 	username = buf_getstring(ses.payload, &userlen)
> > +#ifdef ENABLE_SINGLEUSER
> > +	/* If userspace enabled, ignore username */
> > +	if (svr_opts.singleuser) {
> > +		m_free(username);
> > +		/* Get the current login of the user running dropbear */
> > +		username = m_strdup(getlogin());
> > +	}
> > +#endif /* ifdef ENABLE_SINGLEUSER */
> [...]
> 2) The first line of that hunk creates a copy of username, then you
> check if you need to free that copy and make a different copy.  Seems
> like a waste of work to me?

As I wrote above, I’m overriding “username” value. But this value is
read from a buffer, so it goes forward on this buffer (in this case
ses.payload).

I got to admit I didn't read the whole code of Dropbear. But, I took a
closer look, and I think instead of reading it and overriding it, I
could go forward by using “buf_incrpos(buffer *, int)”.

> [...]

Okay, I will modify my patch the way you want, and will submit it again.

Regards,

--
Antoine Catton
Nexedi Intern