openssh bugs - Sep 2003 - [Bug 715] usage of BROKEN_SETREUID/BROKEN_SETREGID considered harmful

http://bugzilla.mindrot.org/show_bug.cgi?id=715

           Summary: usage of BROKEN_SETREUID/BROKEN_SETREGID considered
                    harmful
           Product: Portable OpenSSH
           Version: -current
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: major
          Priority: P2
         Component: Build system
        AssignedTo: openssh-bugs at mindrot.org
        ReportedBy: Robert.Dahlem at siemens.com


3.7.1p1 introduced BROKEN_SETREUID/BROKEN_SETREGID and requires each and every
OS to have AC_DEFINE(BROKEN_SETREUID)/AC_DEFINE(BROKEN_SETREGID) in
configure.ac, otherwise - if setreuid() is indeed broken - this will break sshd
for non-root users (disconnect, "fatal: : was able to restore old
[e]uid"), see
bug #649 (IRIX), #653 (Tru64), #665 (Mac OS X) and others.

OpenSSH should not depend on untested decisions ("OS x needs
BROKEN_SETREUID/BROKEN_SETREGID, OS y does not need") but instead check if
the
functions are broken and consider them broken until the opposite is proven,
i.e.:

if(geteuid() != 0) /* make sure we're root */
        exit(1); /* otherwise declare setreuid() broken */
setreuid(1,1); /* try to lose UID 0 */
setuid(0); /* try to regain UID 0, must fail */
if(geteuid() != 0) /* if we're root again */
        exit(1); /* setreuid() is broken */                       
exit(0); /* setreuid() is ok */



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=715





------- Additional Comments From djm at mindrot.org  2003-09-26 00:21 -------
So ./configure should require root privileges? I think not...

These tests expose operating system bugs, not OpenSSH bugs. We are willing to
work around them, but you should complain to your OS vendor first.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=715





------- Additional Comments From Robert.Dahlem at siemens.com  2003-09-26 01:06
-------
The proposed check would not require root privileges, it would just sacrifice
setreuid() when lacking root privileges. I didn't see the advantage of
setreuid() over setuid() anyway, but that's just me lacking the knowledge.
May
be someone can explain this to me off-bugzilla.

I agree upon your statement about who should fix OS bugs. But: we live in real
world with real buggy OSs and real constraints concerning budgets for new
hardware. New OS versions tend to require more/bigger hardware, old OS versions
tend to be no longer under vendor maintenance. Complaining to the vendor is not
always an option.

What OpenSSH does at the moment is knowingly running into a bug multiple OSs
have (in this case we know at least about IRIX, Tru64, Mac OS X and ReliantUnix)
and leaving non-C-capable admins only the choice between vulnerable OpenSSH
(3.6.1) or mal-/non-functioning OpenSSH (sshd simply disconnecting non-root
users).

I think the principle of least surprise should apply here, which means: if you
know it might be broken, then don't use it unless you have prove it is not
broken.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=715





------- Additional Comments From mouring at eviladmin.org  2003-09-26 01:52
-------
We original saw two platforms out of our collection with this issue. After the 
release we learned 50% don't behave the way we expect it.  Plus it has
always
been our policy to track "broken" interfaces and not
"correct" ones.  It is
better logic.

The issue is people not testing when we call for it.  We had a good few weeks 
before we had to release (Shorter than I like, but life sucks).  We recieved 
limited response (some good, some with bug fixes attached.. And we are thankful 
for those who help during release).  However, I know all the platforms we 
tagged with this issue are all platforms with people (admins to C programmers) 
*ON THE MAILINGLIST*.

I'm sorry if people expect us to have every hardware/OS under the sun, but
it
is impossible.  <shrug> Since it is an impossiblity and since neither
those
companies or OS communities are unwilling to support us.  Then maybe we should 
stop breaking our backs trying to support them.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=715





------- Additional Comments From Robert.Dahlem at siemens.com  2003-09-26 02:43
-------
First of all: I didn't want to sound offensive. If I sounded that way then
please excuse, it's probably due to English not being my mother tongue.

I'm sorry I wasn't available for testing before the release although
I'm
subscribed to the mailinglist too. But please: My job is to care for bind,
sendmail, httpd, squid, OpenSSH and some dozens of other packages while caring
for some dozens of systems and applications in production. According to the rule
"If it ain't broke: don't fix it" my policy is to upgrade only
when new
functions are desired or vulnerabilities get known. "I'm sorry if
people expect
me to test every software package pre-releases under the sun, but it is
impossible". :-)

I contribute ReliantUnix patches as soon as I have them. And I try to contribute
opinion. In this case my opinion is that setreuid() has a long, sad story of OSs
implementing it in a way OpenSSH considers broken. Well, then it seems not to be
a good idea to consider it working unless someone proves it's broken.

Now for something different: According to my - probably limited - knowledge
setreuid(x,y) is only different from setuid(x) when x!=y. Well, then why does
OpenSSH use only setreuid(x,x)? This looks like asking for trouble to me.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=715





------- Additional Comments From Robert.Dahlem at siemens.com  2003-09-26 02:45
-------
Created an attachment (id=468)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=468&action=view)
Patch for ReliantUnix.

Trying to be a little more constructive. :-)



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=715

dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Uwe.Veiel at siemens.com



------- Additional Comments From dtucker at zip.com.au  2003-10-07 18:38 -------
*** Bug 731 has been marked as a duplicate of this bug. ***



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=715

dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED



------- Additional Comments From dtucker at zip.com.au  2003-10-07 18:40 -------
Patch applied to -current and 3.7 branch and will be in tomorrow's snapsnot.
Thanks.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.