openssh unix dev - Oct 2001 - Recent openssl is required for OPENSSL_free [Re: Please test snapshots for 3.0 release] (fwd)

No response yet, so resending.

-- 
Pekka Savola                 "Tell me of difficulties surmounted,
Netcore Oy                   not those you stumble over and fall"
Systems. Networks. Security.  -- Robert Jordan: A Crown of Swords

---------- Forwarded message ----------
Date: Fri, 12 Oct 2001 09:44:54 +0300 (EEST)
From: Pekka Savola <pekkas at netcore.fi>
To: Damien Miller <djm at mindrot.org>
Cc: openssh-unix-dev at mindrot.org
Subject: Recent openssl is required for OPENSSL_free [Re: Please test
    snapshots for 3.0 release]

On Fri, 12 Oct 2001, Damien Miller wrote:
> Could everyone please test the latest snapshots as we will be making a
> new release soon.
>
> If you have any patches you would like us to consider, please resend
> them to the list ASAP.
1) As sshd -t is used when restarting sshd with RH scripts now, I think
sshd_config is better marked with noreplace as config files should.

2) I'd probably remove '--with-ipv4-default'; it's a major
release after
all.  I haven't noticed problems with this, and if you'd have to run
'sshd
-6', IPv4 port forwarding through mapped addresses won't work.

3) Building appears to rely on the existance of rather recent openssl.
This is good from security perspective, but will make building with e.g.
0.9.5a impossible.  If this is intended to be requirement (there _have_
been security fixes), at least Requires:  openssl >= 0.9.6 or whatever
should be added and the requirement noted in the docs.

The build failed on my RHL62 with:

./libssh.a(key.o): In function `write_bignum':
key.o(.text+0x7f7): undefined reference to `OPENSSL_free'

I bet this is an issue that people might complain about.

Build works ok on RHL72 beta w/ openssh 0.9.6b.

-- 
Pekka Savola                 "Tell me of difficulties surmounted,
Netcore Oy                   not those you stumble over and fall"
Systems. Networks. Security.  -- Robert Jordan: A Crown of Swords
-------------- next part --------------
Index: openssh.spec
==================================================================RCS file:
/cvs/openssh_cvs/contrib/redhat/openssh.spec,v
retrieving revision 1.86
diff -u -r1.86 openssh.spec
--- openssh.spec	2001/09/26 14:24:21	1.86
+++ openssh.spec	2001/09/27 15:51:33
@@ -264,8 +264,7 @@
 %attr(0755,root,root) %{_libexecdir}/openssh/sftp-server
 %attr(0644,root,root) %{_mandir}/man8/sshd.8*
 %attr(0644,root,root) %{_mandir}/man8/sftp-server.8*
-#%attr(0600,root,root) %config(noreplace) %{_sysconfdir}/sshd_config
-%attr(0600,root,root) %config %{_sysconfdir}/sshd_config
+%attr(0600,root,root) %config(noreplace) %{_sysconfdir}/sshd_config
 %attr(0600,root,root) %config(noreplace) /etc/pam.d/sshd
 %attr(0755,root,root) %config /etc/rc.d/init.d/sshd

On Sat, Oct 20, 2001 at 11:41:24PM +0300, Pekka Savola
wrote:
> 3) Building appears to rely on the existance of rather recent openssl.
> This is good from security perspective, but will make building with e.g.
> 0.9.5a impossible.  If this is intended to be requirement (there _have_
> been security fixes), at least Requires:  openssl >= 0.9.6 or whatever
> should be added and the requirement noted in the docs.
> 
> The build failed on my RHL62 with:
> 
> ./libssh.a(key.o): In function `write_bignum':
> key.o(.text+0x7f7): undefined reference to `OPENSSL_free'
I just had a look into the source. Since BN_bn2dec() really allocates
the buffer itself (using OPENSSL_malloc() in recent versions), there is
nothing an application writer can do with respect to this inconsistency.
(For all OpenSSL special data types, TYPE_new() and TYPE_free() exist.)
The only thing that could be done is to query the version defined in
opensslv.h and based on that make a #if OPENSSL_VERSION_NUMBER construct.
(The comment on security fixes with respect to OpenSSL 0.9.6 applies,
but the only thing touching OpenSSH would be the PRNG fix, and this one
has been backported by some distributors to older OpenSSL versions in
order to maintain compatibility. And, in fact, OpenSSH was immune to
the PRNG problem anyway.)

Best regards,
	Lutz
-- 
Lutz Jaenicke                             Lutz.Jaenicke at aet.TU-Cottbus.DE
BTU Cottbus               http://www.aet.TU-Cottbus.DE/personen/jaenicke/
Lehrstuhl Allgemeine Elektrotechnik                  Tel. +49 355 69-4129
Universitaetsplatz 3-4, D-03044 Cottbus              Fax. +49 355 69-4153

On Tue, 30 Oct 2001, Lutz Jaenicke wrote:
> On Tue, Oct 30, 2001 at 09:17:54AM -0600, mouring at etoh.eviladmin.org
wrote:
> >
> > I thought the reason we moved to OPENSSL_free() is because free() does
not
> > do the right thing on OpenSSL data structures.  Why are we
reintroducing
> > this again?  Or did I miss something.
>
> OPENSSL_free() was introduced with OpenSSL 0.9.6, for OpenSSL 0.9.5, free()
> is the correct way to go. In order to allow the use of 0.9.5,
OPENSSL_free()
> must be replaced (for <0.9.6 only), because we get an unresolved symbol
error
> otherwise.
>
Lutz,

Still does not answer the underlying question.  if OPENSSL_free() =free() why
have OPENSSL_free()?  Feels like an API for the sake of an API
unless there is plans to make OPENSSL_free() do more.  At which point this
change will be a bad thing.

Not arguing this is not need.  I just would like to get a sense of why.

- Ben

On Tue, Oct 30, 2001 at 10:12:02AM -0600, mouring at etoh.eviladmin.org
wrote:
> On Tue, 30 Oct 2001, Lutz Jaenicke wrote:
> > OPENSSL_free() was introduced with OpenSSL 0.9.6, for OpenSSL 0.9.5,
free()
> > is the correct way to go. In order to allow the use of 0.9.5,
OPENSSL_free()
> > must be replaced (for <0.9.6 only), because we get an unresolved
symbol error
> > otherwise.
> >
> 
> Still does not answer the underlying question.  if OPENSSL_free() =>
free() why have OPENSSL_free()?  Feels like an API for the sake of an API
> unless there is plans to make OPENSSL_free() do more.  At which point this
> change will be a bad thing.
The following excerpts from the CHANGES file should explain the fundamental
idea behind using specific functions for the memory handling. Practically,
as long as no debugging routines are enabled, it comes done to malloc()
and friends...

...
 Changes between 0.9.5a and 0.9.6  [24 Sep 2000]
...
  *) Rename memory handling macros to avoid conflicts with other
     software:
          Malloc         =>  OPENSSL_malloc
          Malloc_locked  =>  OPENSSL_malloc_locked
          Realloc        =>  OPENSSL_realloc
          Free           =>  OPENSSL_free
...
 Changes between 0.9.4 and 0.9.5  [28 Feb 2000]
...
  *) Rebuild of the memory allocation routines used by OpenSSL code and
     possibly others as well.  The purpose is to make an interface that
     provide hooks so anyone can build a separate set of allocation and
     deallocation routines to be used by OpenSSL, for example memory
     pool implementations, or something else, which was previously hard
     since Malloc(), Realloc() and Free() were defined as macros having
     the values malloc, realloc and free, respectively (except for Win32
     compilations).  The same is provided for memory debugging code.
     OpenSSL already comes with functionality to find memory leaks, but
     this gives people a chance to debug other memory problems.
...
> Not arguing this is not need.  I just would like to get a sense of why.
:-)
	Lutz
-- 
Lutz Jaenicke                             Lutz.Jaenicke at aet.TU-Cottbus.DE
BTU Cottbus               http://www.aet.TU-Cottbus.DE/personen/jaenicke/
Lehrstuhl Allgemeine Elektrotechnik                  Tel. +49 355 69-4129
Universitaetsplatz 3-4, D-03044 Cottbus              Fax. +49 355 69-4153

Thanks.

On Tue, 30 Oct 2001, Lutz Jaenicke wrote:
> On Tue, Oct 30, 2001 at 10:12:02AM -0600, mouring at etoh.eviladmin.org
wrote:
> > On Tue, 30 Oct 2001, Lutz Jaenicke wrote:
> > > OPENSSL_free() was introduced with OpenSSL 0.9.6, for OpenSSL
0.9.5, free()
> > > is the correct way to go. In order to allow the use of 0.9.5,
OPENSSL_free()
> > > must be replaced (for <0.9.6 only), because we get an
unresolved symbol error
> > > otherwise.
> > >
> >
>
> > Still does not answer the underlying question.  if OPENSSL_free()
=> > free() why have OPENSSL_free()?  Feels like an API for the sake of an
API
> > unless there is plans to make OPENSSL_free() do more.  At which point
this
> > change will be a bad thing.
>
> The following excerpts from the CHANGES file should explain the fundamental
> idea behind using specific functions for the memory handling. Practically,
> as long as no debugging routines are enabled, it comes done to malloc()
> and friends...
>
> ...
>  Changes between 0.9.5a and 0.9.6  [24 Sep 2000]
> ...
>   *) Rename memory handling macros to avoid conflicts with other
>      software:
>           Malloc         =>  OPENSSL_malloc
>           Malloc_locked  =>  OPENSSL_malloc_locked
>           Realloc        =>  OPENSSL_realloc
>           Free           =>  OPENSSL_free
> ...
>  Changes between 0.9.4 and 0.9.5  [28 Feb 2000]
> ...
>   *) Rebuild of the memory allocation routines used by OpenSSL code and
>      possibly others as well.  The purpose is to make an interface that
>      provide hooks so anyone can build a separate set of allocation and
>      deallocation routines to be used by OpenSSL, for example memory
>      pool implementations, or something else, which was previously hard
>      since Malloc(), Realloc() and Free() were defined as macros having
>      the values malloc, realloc and free, respectively (except for Win32
>      compilations).  The same is provided for memory debugging code.
>      OpenSSL already comes with functionality to find memory leaks, but
>      this gives people a chance to debug other memory problems.
> ...
>
> > Not arguing this is not need.  I just would like to get a sense of
why.
>
> :-)
> 	Lutz
> --
> Lutz Jaenicke                             Lutz.Jaenicke at
aet.TU-Cottbus.DE
> BTU Cottbus               http://www.aet.TU-Cottbus.DE/personen/jaenicke/
> Lehrstuhl Allgemeine Elektrotechnik                  Tel. +49 355 69-4129
> Universitaetsplatz 3-4, D-03044 Cottbus              Fax. +49 355 69-4153
>

On Tue, Oct 30, 2001 at 06:49:55PM +0100, Lutz Jaenicke
wrote:
> ...
>  Changes between 0.9.5a and 0.9.6  [24 Sep 2000]
> ...
>   *) Rename memory handling macros to avoid conflicts with other
>      software:
>           Malloc         =>  OPENSSL_malloc
>           Malloc_locked  =>  OPENSSL_malloc_locked
>           Realloc        =>  OPENSSL_realloc
>           Free           =>  OPENSSL_free
> ...
Hmm, while thinking about it: the correct macro substution should therefore
be "Free()" instead of "free()", as we must make sure that
the correct
memory handling function (CRYPTO_free()) is being called:

/* OPENSSL_free() is only available in OpenSSL 0.9.6 onwards */
#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER < 0x0090600f)
# define OPENSSL_free(x) Free(x)
#endif

-- 
Lutz Jaenicke                             Lutz.Jaenicke at aet.TU-Cottbus.DE
BTU Cottbus               http://www.aet.TU-Cottbus.DE/personen/jaenicke/
Lehrstuhl Allgemeine Elektrotechnik                  Tel. +49 355 69-4129
Universitaetsplatz 3-4, D-03044 Cottbus              Fax. +49 355 69-4153

On Tue, 30 Oct 2001, Lutz Jaenicke wrote:
> On Tue, Oct 30, 2001 at 06:49:55PM +0100, Lutz Jaenicke wrote:
> > ...
> >  Changes between 0.9.5a and 0.9.6  [24 Sep 2000]
> > ...
> >   *) Rename memory handling macros to avoid conflicts with other
> >      software:
> >           Malloc         =>  OPENSSL_malloc
> >           Malloc_locked  =>  OPENSSL_malloc_locked
> >           Realloc        =>  OPENSSL_realloc
> >           Free           =>  OPENSSL_free
> > ...
>
> Hmm, while thinking about it: the correct macro substution should therefore
> be "Free()" instead of "free()", as we must make sure
that the correct
> memory handling function (CRYPTO_free()) is being called:
>
> /* OPENSSL_free() is only available in OpenSSL 0.9.6 onwards */
> #if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER <
0x0090600f)
> # define OPENSSL_free(x) Free(x)
> #endif
>
This builds fine with 0.9.5a also.
But I have not tested it. (I use 0.9.6b on my production versions)
>
-- 
Tim Rice				Multitalents	(707) 887-1469
tim at multitalents.net

On Tue, 30 Oct 2001, Lutz Jaenicke wrote:
> Hmm, while thinking about it: the correct macro substution should therefore
> be "Free()" instead of "free()", as we must make sure
that the correct
> memory handling function (CRYPTO_free()) is being called:
That is (as the OpenSSL developers discovered) a namespace collision 
waiting to happen. Can people try this patch?

Index: defines.h
==================================================================RCS file:
/var/cvs/openssh/defines.h,v
retrieving revision 1.74
diff -u -r1.74 defines.h
--- defines.h	2001/10/30 02:50:40	1.74
+++ defines.h	2001/10/30 23:35:22
@@ -45,6 +45,7 @@
 #include <unistd.h> /* For STDIN_FILENO, etc */
 #include <termios.h> /* Struct winsize */
 #include <fcntl.h> /* For O_NONBLOCK */
+#include <openssl/opensslv.h> /* For OPENSSL_VERSION_NUMBER */
 
 /* *-*-nto-qnx needs these headers for strcasecmp and LASTLOG_FILE respectively
*/
 #ifdef HAVE_STRINGS_H
@@ -448,6 +449,11 @@
 
 #ifndef GETPGRP_VOID
 # define getpgrp() getpgrp(0)
+#endif
+
+/* OPENSSL_free() is only available in OpenSSL 0.9.6 onwards */
+#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER <
0x0090600f)
+# define OPENSSL_free(x) Free(x)
 #endif
 
 /*


-- 
| By convention there is color,       \\ Damien Miller <djm at
mindrot.org>
| By convention sweetness, By convention bitterness, \\ www.mindrot.org
| But in reality there are atoms and space - Democritus (c. 400 BCE)

On Wed, 31 Oct 2001, Damien Miller wrote:
> On Tue, 30 Oct 2001, Lutz Jaenicke wrote:
> 
> > Hmm, while thinking about it: the correct macro substution should
therefore
> > be "Free()" instead of "free()", as we must make
sure that the correct
> > memory handling function (CRYPTO_free()) is being called:
> 
> That is (as the OpenSSL developers discovered) a namespace collision 
> waiting to happen. Can people try this patch?
I put this on latest CVS, rebuilt the new RPM with
contrib/redhat/openssh.spec and installed it on my RHL62 system w/ OpenSSL
0.9.5a; seems to work fine.

> Index: defines.h
> ==================================================================> RCS
file: /var/cvs/openssh/defines.h,v
> retrieving revision 1.74
> diff -u -r1.74 defines.h
> --- defines.h	2001/10/30 02:50:40	1.74
> +++ defines.h	2001/10/30 23:35:22
> @@ -45,6 +45,7 @@
>  #include <unistd.h> /* For STDIN_FILENO, etc */
>  #include <termios.h> /* Struct winsize */
>  #include <fcntl.h> /* For O_NONBLOCK */
> +#include <openssl/opensslv.h> /* For OPENSSL_VERSION_NUMBER */
>  
>  /* *-*-nto-qnx needs these headers for strcasecmp and LASTLOG_FILE
respectively */
>  #ifdef HAVE_STRINGS_H
> @@ -448,6 +449,11 @@
>  
>  #ifndef GETPGRP_VOID
>  # define getpgrp() getpgrp(0)
> +#endif
> +
> +/* OPENSSL_free() is only available in OpenSSL 0.9.6 onwards */
> +#if !defined(OPENSSL_VERSION_NUMBER) || (OPENSSL_VERSION_NUMBER <
0x0090600f)
> +# define OPENSSL_free(x) Free(x)
>  #endif
>  
>  /*
> 
> 
> 
-- 
Pekka Savola                 "Tell me of difficulties surmounted,
Netcore Oy                   not those you stumble over and fall"
Systems. Networks. Security.  -- Robert Jordan: A Crown of Swords