forwarded message from mouring at etoh.eviladmin.org

[mouring at etoh.eviladmin.org]

Markus,

I still don't get why this is even an issue..

if gethostname() returns failures if the hostname is greater then
MAXHOSTNAMELEN then this whole issue is moot, and this patch is wrong
since it creates a case where you have inconsistance behavior between
platforms.  And if that is the case, I will refuse to apply it.

If OpenBSD or other platforms hack the hostname off to match the current
variable size, and they do not return some type of failure then the OS is
broken.  And it should be fixed in their code not our application since we
have no way of detecting such evilness (Also, consider a hostname getting
hacked to: "somehost.open"  now.. How in the world do you use that
correctly for key verification or X11 DISPLAY setting?!  You end up having
to make educated guess based on limited knowledge which can bite us in the
ass later on).

I still have to ask the same question I did the last time this was brought
up.. Who in their right mind would have a hostname/FQDN that is over 256
characters (using OpenBSD's documented limit)?!  That is over 3 lines
worth of information to type!!!  There are days I wish I would have bought
a shorter name then 'eviladmin.org' =)

I have no problems if we handle ENAMETOOLONG using xgethostname()
(mirroring naming already used for replacement functions [xstrdup,
xmalloc, etc]).  However, since OpenBSD currently does not suppot
ENAMETOOLONG it really should not be in the OpenBSD tree which leaves us
with 3 unique changes for portable.  Which is tolerable.

Until such time we can provide configure.in checks and define such
limitations in our code if the first assumption is true.  The change would
require a #define to be set in the configure.in since I know Damien does
not like blindly setting such variables.

Mr Brinkmann,

The less core code that can be changed between the portable and the
OpenBSD tree the happier I become.  It is an extreme bear to get a patch
set from the OpenBSD tree and find it fails to apply due to cross-platform
differences.  Then having to tip toe around code for platforms that I
can't test.  Praying I don't break the platform and it gets released in
such a broken state in the next release (luckly, we have been doing pretty
good on testing before a release thanks to the many people who help out!).

Keep this in mind.  =)  We have already three OSes (NeXTStep, Cray, and
Cygwin) with openbsd-compat/bsd-OSNAME.[ch]  which I advocate for code
that is not logical to share between platforms.

So, you now understand where I come from. =)  BTW, I do follow Hurd
weekly kernel cousin report.  So I have seen this issue before on
your own mailinglists for other software packages.

It's already horrible to do merges on a bad day when Markus starts
playing with code that has large amount of #ifdef near by. =)

- Ben

On Mon, 23 Jul 2001, Markus Friedl wrote:

> this patch looks fine,
>
> however, i don't want to see a magic numer 401 in openssh's code.
>
> why not just define MAXHOSTNAMELEN for HURD ?
>
> -m
>
> On Mon, Jul 23, 2001 at 03:41:04AM +0200, Marcus Brinkmann wrote:
> > Hi,
> >
> > I just looked in the mail archive to see what patch was posted
> > (http://marc.theaimsgroup.com/?l=openssh-unix-dev&m=99580473611227&w=2)
> >
> > This piece of the patch is not good, because it is not defined if the
> > truncated name will be null terminated.  This can be fixed by null
> > terminating the reult no matter what, I think.
> >
> > However, I really suggest to just put a get_host_name implementation in ssh,
> > that works on all systems and returns a malloced buffer.  This can then be
> > used everywhere were gethostname is used.
> >
> > A good implementation is for example in GNU inetutils
> > (libinetutils/localhost.c)
> >
> > Thanks,
> > Marcus
> >
> > diff -ur openssh-2.9p2-/channels.c openssh-2.9p2/channels.c
> > --- openssh-2.9p2-/channels.c   Sat Mar 17 01:47:55 2001
> > +++ openssh-2.9p2/channels.c    Thu Jun 28 20:11:02 2001
> > @@ -1965,7 +1965,7 @@
> >         char strport[NI_MAXSERV];
> >         int gaierr, n, num_socks = 0, socks[NUM_SOCKS];
> >         char display[512];
> > -       char hostname[MAXHOSTNAMELEN];
> > +       char hostname[401];     /* we only use the first 400 bytes anyway*/
> >
> >         for (display_number = x11_display_offset;
> >              display_number < MAX_DISPLAYS;
> > @@ -2037,7 +2037,8 @@
> >         }
> >
> >         /* Set up a suitable value for the DISPLAY variable. */
> > -       if (gethostname(hostname, sizeof(hostname)) < 0)
> > +       if (gethostname(hostname, sizeof(hostname)) < 0
> > +           && errno != ENAMETOOLONG)
> >                 fatal("gethostname: %.100s", strerror(errno));
> >
> >  #ifdef IPADDR_IN_DISPLAY
> >
> >
> > --
> > `Rhubarb is no Egyptian god.' Debian http://www.debian.org brinkmd at debian.org
> > Marcus Brinkmann              GNU    http://www.gnu.org    marcus at gnu.org
> > Marcus.Brinkmann at ruhr-uni-bochum.de
> > http://www.marcus-brinkmann.de
>