Re: RFV: Drop support for OpenSSL 0.9.6 and before

[Wes Hardaker]

>>>>> On Fri, 2 Sep 2011 19:03:05 +0200, Bart Van Assche <bvana...@acm.org> said:

BVA> An issue with how Net-SNMP currently interfaces with OpenSSL,
BVA> especially on Windows, is that replacing the OpenSSL binaries may
BVA> cause Net-SNMP to crash.

I think I'm fine with this post 5.7 time. The question is, is it a bug
or a feature to make the change and which branch(es) should it go into?
--
Wes Hardaker
Please mail all replies to net-snm...@lists.sourceforge.net

------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2dcopy1
_______________________________________________
Net-snmp-coders mailing list
Net-snm...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders

[Wes Hardaker]

>>>>> On Wed, 21 Sep 2011 20:14:07 +0200, Bart Van Assche <bvana...@acm.org> said:

BVA> Passing a buffer that is smaller than what OpenSSL writes into is a bug, a
BVA> bug I've already seen causing a crash far too often on Windows. I'd like to
BVA> apply this patch to branches 5.4 and above.

Well, it's only a bug when using it with libraries that weren't used to
build it, right? I'm not sure that is technically a bug in our
software. It's a bug in the fact that it's succeeding in dynamic
linking with another library that doesn't match against the one it was
compiled with.

That being said, the point is: what's best for the end-community. And
to answer that I'd want to know about older OSes that were still
using 0.9.6 out of the box. Sadly, I doubt we have that list.

[Dave Shield]

On 21 September 2011 21:03, Wes Hardaker <hard...@users.sourceforge.net> wrote:
> That being said, the point is: what's best for the end-community.  And
> to answer that I'd want to know about older OSes that were still
> using 0.9.6 out of the box.  Sadly, I doubt we have that list.

How about checking for the presence of EVP_MD_CTX_create as part of the
configure script, and defining a local version if it's not found?

Dave

[Wes Hardaker]

>>>>> On Thu, 22 Sep 2011 12:11:36 +0200, Bart Van Assche <bvana...@acm.org> said:

BVA> A crash would still be triggered if the OpenSSL library would be
BVA> upgraded to a version with larger EVP_MD_CTX.

But it would only crash if using an older version of OpenSSL, but would
we'd still support it.

--
Wes Hardaker
Please mail all replies to net-snm...@lists.sourceforge.net

[Wes Hardaker]

>>>>> On Thu, 22 Sep 2011 17:05:46 +0200, Bart Van Assche <bvana...@acm.org> said:

BVA> If EVP_MD_CTX would grow larger in the future, a crash could be triggered by
BVA> upgrading too. That's actually what several (Windows) users have reported
BVA> when they tried to run the Net-SNMP 5.4 binaries (built against OpenSSL
BVA> 0.9.something) on a system where OpenSSL 1.0 was installed.

We have two situations:

1) people compiling the source themselves against an unknown OpenSSL.
If we want to support < 0.9.6, then Dave's solution allows for that.
The only time something would break would be if the user upgraded
from 0.9.6 to something else without recompiling.

2) people using our distributed binaries (eg, on windows). I think it's
likely safe to say we'll only be producing binaries with 0.9.7 or
later and thus the original function would be used from OpenSSL,
after applying your proposed patch, so that users upgrading or using
a different OpenSSL won't be affected by that particular malloc size
change.

In actuality, though, I actually think that this is not necessarily the
only problem we'll run into. OpenSSL is not known for being backwards
compatible and I suspect if you asked them if it was safe to upgrade
their library to a new version without recompiling all the software
linked against it they'd say "no way; please recompile".

[Bart Van Assche]

On Thu, Sep 22, 2011 at 5:49 PM, Wes Hardaker
<hard...@users.sourceforge.net> wrote:
> >>>>> On Thu, 22 Sep 2011 17:05:46 +0200, Bart Van Assche <bvana...@acm.org> said:
> BVA> If EVP_MD_CTX would grow larger in the future, a crash could be triggered by
> BVA> upgrading too. That's actually what several (Windows) users have reported
> BVA> when they tried to run the Net-SNMP 5.4 binaries (built against OpenSSL
> BVA> 0.9.something) on a system where OpenSSL 1.0 was installed.
>
> We have two situations:
>
> 1) people compiling the source themselves against an unknown OpenSSL.
> If we want to support < 0.9.6, then Dave's solution allows for that.
> The only time something would break would be if the user upgraded
> from 0.9.6 to something else without recompiling.
>
> 2) people using our distributed binaries (eg, on windows). I think it's
> likely safe to say we'll only be producing binaries with 0.9.7 or
> later and thus the original function would be used from OpenSSL,
> after applying your proposed patch, so that users upgrading or using
> a different OpenSSL won't be affected by that particular malloc size
> change.
>
> In actuality, though, I actually think that this is not necessarily the
> only problem we'll run into. OpenSSL is not known for being backwards
> compatible and I suspect if you asked them if it was safe to upgrade
> their library to a new version without recompiling all the software
> linked against it they'd say "no way; please recompile".

This follow-up patch should restore OpenSSL 0.9.6 compatibility:

---
configure | 90 +++++++++++++++++++++++++++++++++
configure.d/config_os_libs2 | 8 +++
include/net-snmp/net-snmp-config.h.in | 6 ++
snmplib/keytools.c | 17 ++++++-
snmplib/scapi.c | 10 ++++
win32/net-snmp/net-snmp-config.h | 6 ++
win32/net-snmp/net-snmp-config.h.in | 6 ++
7 files changed, 141 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 9fa98e6..77c6dff 100755
--- a/configure
+++ b/configure
@@ -22534,6 +22534,96 @@ $as_echo "#define HAVE_AES_CFB128_ENCRYPT 1"
>>confdefs.h

fi

+
+ as_ac_Lib=`$as_echo
"ac_cv_lib_${CRYPTO}''_EVP_MD_CTX_create" | $as_tr_sh`
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for
EVP_MD_CTX_create in -l${CRYPTO}" >&5
+$as_echo_n "checking for EVP_MD_CTX_create in -l${CRYPTO}... " >&6; }
+if eval \${$as_ac_Lib+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_check_lib_save_LIBS=$LIBS
+LIBS="-l${CRYPTO} $LIBS"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+/* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+#ifdef __cplusplus
+extern "C"
+#endif
+char EVP_MD_CTX_create ();
+int
+main ()
+{
+return EVP_MD_CTX_create ();
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+ eval "$as_ac_Lib=yes"
+else
+ eval "$as_ac_Lib=no"
+fi
+rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext conftest.$ac_ext
+LIBS=$ac_check_lib_save_LIBS
+fi
+eval ac_res=\$$as_ac_Lib
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+if eval test \"x\$"$as_ac_Lib"\" = x"yes"; then :
+
+$as_echo "#define HAVE_EVP_MD_CTX_CREATE /**/" >>confdefs.h
+
+fi
+
+
+ as_ac_Lib=`$as_echo
"ac_cv_lib_${CRYPTO}''_EVP_MD_CTX_destroy" | $as_tr_sh`
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for
EVP_MD_CTX_destroy in -l${CRYPTO}" >&5
+$as_echo_n "checking for EVP_MD_CTX_destroy in -l${CRYPTO}... " >&6; }
+if eval \${$as_ac_Lib+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_check_lib_save_LIBS=$LIBS
+LIBS="-l${CRYPTO} $LIBS"
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+/* Override any GCC internal prototype to avoid an error.
+ Use char because int might match the return type of a GCC
+ builtin and then its argument prototype would still apply. */
+#ifdef __cplusplus
+extern "C"
+#endif
+char EVP_MD_CTX_destroy ();
+int
+main ()
+{
+return EVP_MD_CTX_destroy ();
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_link "$LINENO"; then :
+ eval "$as_ac_Lib=yes"
+else
+ eval "$as_ac_Lib=no"
+fi
+rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext conftest.$ac_ext
+LIBS=$ac_check_lib_save_LIBS
+fi
+eval ac_res=\$$as_ac_Lib
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_res" >&5
+$as_echo "$ac_res" >&6; }
+if eval test \"x\$"$as_ac_Lib"\" = x"yes"; then :
+
+$as_echo "#define HAVE_EVP_MD_CTX_DESTROY /**/" >>confdefs.h
+
+fi
+
fi
if echo " $transport_result_list " | $GREP "DTLS" > /dev/null; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for
DTLSv1_method in -lssl" >&5
diff --git a/configure.d/config_os_libs2 b/configure.d/config_os_libs2
index 22ab764..74aba29 100644
--- a/configure.d/config_os_libs2
+++ b/configure.d/config_os_libs2
@@ -277,6 +277,14 @@ if test "x$tryopenssl" != "xno" -a "x$tryopenssl"
!= "xinternal"; then
AC_CHECK_LIB(${CRYPTO}, AES_cfb128_encrypt,
AC_DEFINE(HAVE_AES_CFB128_ENCRYPT, 1,
[Define to 1 if you have the `AES_cfb128_encrypt'
function.]))
+
+ AC_CHECK_LIB(${CRYPTO}, EVP_MD_CTX_create,
+ AC_DEFINE([HAVE_EVP_MD_CTX_CREATE], [],
+ [Define to 1 if you have the `EVP_MD_CTX_create' function.]))
+
+ AC_CHECK_LIB(${CRYPTO}, EVP_MD_CTX_destroy,
+ AC_DEFINE([HAVE_EVP_MD_CTX_DESTROY], [],
+ [Define to 1 if you have the `EVP_MD_CTX_destroy' function.]))
fi
if echo " $transport_result_list " | $GREP "DTLS" > /dev/null; then
AC_CHECK_LIB(ssl, DTLSv1_method,
diff --git a/include/net-snmp/net-snmp-config.h.in
b/include/net-snmp/net-snmp-config.h.in
index 7f5d3c6..e13342d 100644
--- a/include/net-snmp/net-snmp-config.h.in
+++ b/include/net-snmp/net-snmp-config.h.in
@@ -137,6 +137,12 @@
/* Define to 1 if you have the `eval_pv' function. */
#undef HAVE_EVAL_PV

+/* Define to 1 if you have the `EVP_MD_CTX_create' function. */
+#undef HAVE_EVP_MD_CTX_CREATE
+
+/* Define to 1 if you have the `EVP_MD_CTX_destroy' function. */
+#undef HAVE_EVP_MD_CTX_DESTROY
+
/* Define if you have EVP_sha224/256 in openssl */
#undef HAVE_EVP_SHA224

diff --git a/snmplib/keytools.c b/snmplib/keytools.c
index 72f39b9..cd530e8 100644
--- a/snmplib/keytools.c
+++ b/snmplib/keytools.c
@@ -118,7 +118,7 @@ generate_Ku(const oid * hashtype, u_int hashtype_len,
u_char buf[USM_LENGTH_KU_HASHBLOCK], *bufp;

#ifdef NETSNMP_USE_OPENSSL
- EVP_MD_CTX *ctx = EVP_MD_CTX_create();
+ EVP_MD_CTX *ctx = NULL;
unsigned int tmp_len;
#elif NETSNMP_USE_INTERNAL_CRYPTO
SHA_CTX csha1;
@@ -151,6 +151,12 @@ generate_Ku(const oid * hashtype, u_int hashtype_len,
*/
#ifdef NETSNMP_USE_OPENSSL

+#ifdef HAVE_EVP_MD_CTX_CREATE
+ ctx = EVP_MD_CTX_create();
+#else
+ ctx = malloc(sizeof(*ctx));
+ EVP_MD_CTX_init(ctx);
+#endif
#ifndef NETSNMP_DISABLE_MD5
if (ISTRANSFORM(hashtype, HMACMD5Auth))
EVP_DigestInit(ctx, EVP_md5());
@@ -246,7 +252,14 @@ generate_Ku(const oid * hashtype, u_int hashtype_len,
generate_Ku_quit:
memset(buf, 0, sizeof(buf));
#ifdef NETSNMP_USE_OPENSSL
- EVP_MD_CTX_destroy(ctx);
+ if (ctx) {
+#ifdef HAVE_EVP_MD_CTX_DESTROY
+ EVP_MD_CTX_destroy(ctx);
+#else
+ EVP_MD_CTX_cleanup(ctx);
+ free(ctx);
+#endif
+ }
#endif
return rval;

diff --git a/snmplib/scapi.c b/snmplib/scapi.c
index 2b2eeb1..5b4673c 100644
--- a/snmplib/scapi.c
+++ b/snmplib/scapi.c
@@ -485,7 +485,12 @@ sc_hash(const oid * hashtype, size_t hashtypelen,
const u_char * buf,
}

/** initialize the pointer */
+#ifdef HAVE_EVP_MD_CTX_CREATE
cptr = EVP_MD_CTX_create();
+#else
+ cptr = malloc(sizeof(*cptr));
+ EVP_MD_CTX_init(cptr);
+#endif
EVP_DigestInit(cptr, hashfn);

/** pass the data */
@@ -494,7 +499,12 @@ sc_hash(const oid * hashtype, size_t hashtypelen,
const u_char * buf,
/** do the final pass */
EVP_DigestFinal(cptr, MAC, &tmp_len);
*MAC_len = tmp_len;
+#ifdef HAVE_EVP_MD_CTX_DESTROY
EVP_MD_CTX_destroy(cptr);
+#else
+ EVP_MD_CTX_cleanup(cptr);
+ free(cptr);
+#endif
return (rval);

#elif NETSNMP_USE_INTERNAL_CRYPTO
diff --git a/win32/net-snmp/net-snmp-config.h b/win32/net-snmp/net-snmp-config.h
index 85ccd61..8d8c36f 100644
--- a/win32/net-snmp/net-snmp-config.h
+++ b/win32/net-snmp/net-snmp-config.h
@@ -1358,6 +1358,12 @@
/* Define to 1 if you have the <openssl/aes.h> header file. */
#define HAVE_OPENSSL_AES_H 1

+/* Define to 1 if you have the `EVP_MD_CTX_create' function. */
+#define HAVE_EVP_MD_CTX_CREATE 1
+
+/* Define to 1 if you have the `EVP_MD_CTX_destroy' function. */
+#define HAVE_EVP_MD_CTX_DESTROY 1
+
/* Define to 1 if you have the `AES_cfb128_encrypt' function. */
#define HAVE_AES_CFB128_ENCRYPT 1

diff --git a/win32/net-snmp/net-snmp-config.h.in
b/win32/net-snmp/net-snmp-config.h.in
index b16931b..8004eaa 100644
--- a/win32/net-snmp/net-snmp-config.h.in
+++ b/win32/net-snmp/net-snmp-config.h.in
@@ -1358,6 +1358,12 @@
/* Define to 1 if you have the <openssl/aes.h> header file. */
#define HAVE_OPENSSL_AES_H 1

+/* Define to 1 if you have the `EVP_MD_CTX_create' function. */
+#define HAVE_EVP_MD_CTX_CREATE 1
+
+/* Define to 1 if you have the `EVP_MD_CTX_destroy' function. */
+#define HAVE_EVP_MD_CTX_DESTROY 1
+
/* Define to 1 if you have the `AES_cfb128_encrypt' function. */
#define HAVE_AES_CFB128_ENCRYPT 1

--
1.7.3.4

------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2dcopy2