You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe Jr." <wr...@rowe-clan.net> on 2012/08/18 21:51:09 UTC
Re: svn commit: r1374640 - /httpd/httpd/branches/2.2.x/STATUS
On 8/18/2012 2:32 PM, wrowe@apache.org wrote:
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Sat Aug 18 19:32:38 2012
> @@ -145,7 +145,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
> http://svn.apache.org/viewvc?view=revision&revision=1225476
> http://svn.apache.org/viewvc?view=revision&revision=1225792
> Backport version for 2.2.x of the patches above:
> - http://people.apache.org/~wrowe/tls11-12-patch-2.2-kbrand-wrowe.1.patch
> + http://people.apache.org/~wrowe/tls11-12-patch-2.2-kbrand-wrowe.2.patch
> +1: wrowe,
> kbrand: The #define HAVE_TLSV1_X stuff should go to ssl_toolkit_compat.h,
> [wrowe] disagree, since that API was deprecated
> @@ -160,6 +160,15 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
> to drop the #ifndef around SSL_PROTOCOL_SSLV2 in ssl_private.h,
> this should also make some of the other "#if[n]def OPENSSL_NO_SSL2"
> encapsulations unnecessary.
> + [wrowe] agreed the patch was wrong, the #ifdef needed to be moved
> + up four lines. Behavior is now correct in patch .2
> + Disagree about retaining SSL_PROTOCOL_SSLV2; this is one
> + of the most basic design patterns which exists to ensure
> + that we don't have some lingering code which is still
> + attempting to pursue SSLV2 games, not to mention that
> + the various macros and functions in those blocks may
> + simply disappear disappear in an OPENSSL_NO_SSL2 build.
> + Bad idea, it helps us catch current and future problems.
Thanks for the feedback, Kaspar. Other than shifting the one #ifdef case to the
correct line of code, I believe the patch is complete, I've tried to state my case
for no further changes. If you would like to propose further changes, I'd really
prefer we defer these to 2.2.24-dev so we can T&R already. I'm pretty sure you
already agree with me that the flexibility to disable a particular cipher in light
of exploit research in the very fresh openssl code base makes this patch pretty
critical to release for legacy, as well as stable.
Even if you can give the patch your +1, we still need one more individual to chime
in before we can finish this backport, and as I mention, OpenSSL's CVE-2012-2333
suggests that this patch is a showstopper. Can one more person take a pass through
the code, since Stefan didn't have a chance to re-review this week?
Re: svn commit: r1374640 - /httpd/httpd/branches/2.2.x/STATUS
Posted by Kaspar Brand <ht...@velox.ch>.
On 19.8.12 19:59, Kaspar Brand wrote:
> Ok, thanks for clarifying, I stand corrected. opensslconf.h is included
> in ssl_private.h via #include <openssl/x509_vfy.h>, that's why the
> current 2.2.x code is working as expected.
Um, for 2.2.x that should read: in ssl_toolkit_compat.h, and it's
actually included through openssl/evp.h (x509_vfy.h is not used in 2.2.x).
Kaspar
Re: svn commit: r1374640 - /httpd/httpd/branches/2.2.x/STATUS
Posted by Kaspar Brand <ht...@velox.ch>.
On 19.8.12 19:37, Dr Stephen Henson wrote:
> On 19/08/2012 18:22, Kaspar Brand wrote:
>> (ssl.h is not customized by OpenSSL's Configure script, AFAICT you would
>> have to call "openssl version -f" and look for any flags set at compile
>> time.)
>>
>> I.e., unless mod_ssl is explicitly compiled with -DOPENSSL_NO_SSL2 (set
>> through CPPFLAGS/CFLAGS), an #ifdef OPENSSL_NO_SSL2 has no effect, and
>> the blocks enclosed with #ifndef OPENSSL_NO_SSL2 will get included,
>> irrespective of how OpenSSL has been compiled.
>>
>
> The usual way is to use no-ssl2 as an argument to Configure or config which then
> adds OPENSSL_NO_SSL2 into crypto/opensslconf.h
Ok, thanks for clarifying, I stand corrected. opensslconf.h is included
in ssl_private.h via #include <openssl/x509_vfy.h>, that's why the
current 2.2.x code is working as expected.
Kaspar
Re: svn commit: r1374640 - /httpd/httpd/branches/2.2.x/STATUS
Posted by Dr Stephen Henson <sh...@opensslfoundation.com>.
On 19/08/2012 18:22, Kaspar Brand wrote:
> On 18.8.12 21:51, William A. Rowe Jr. wrote:
>>> to drop the #ifndef around SSL_PROTOCOL_SSLV2 in ssl_private.h,
>>> this should also make some of the other "#if[n]def OPENSSL_NO_SSL2"
>>> encapsulations unnecessary.
>>> + [wrowe] agreed the patch was wrong, the #ifdef needed to be moved
>>> + up four lines. Behavior is now correct in patch .2
>>> + Disagree about retaining SSL_PROTOCOL_SSLV2; this is one
>>> + of the most basic design patterns which exists to ensure
>>> + that we don't have some lingering code which is still
>>> + attempting to pursue SSLV2 games, not to mention that
>>> + the various macros and functions in those blocks may
>>> + simply disappear disappear in an OPENSSL_NO_SSL2 build.
>>> + Bad idea, it helps us catch current and future problems.
>
> After a closer look at the mechanics of OPENSSL_NO_SSL2 in OpenSSL, I
> think there's a misunderstanding of how OpenSSL exposes this
> compile-time option to applications linking against libssl. OpenSSL
> itself only defines OPENSSL_NO_SSL2 in the following case (openssl/ssl.h):
>
>> #if (defined(OPENSSL_NO_RSA) || defined(OPENSSL_NO_MD5)) && !defined(OPENSSL_NO_SSL2)
>> #define OPENSSL_NO_SSL2
>> #endif
>
> (ssl.h is not customized by OpenSSL's Configure script, AFAICT you would
> have to call "openssl version -f" and look for any flags set at compile
> time.)
>
> I.e., unless mod_ssl is explicitly compiled with -DOPENSSL_NO_SSL2 (set
> through CPPFLAGS/CFLAGS), an #ifdef OPENSSL_NO_SSL2 has no effect, and
> the blocks enclosed with #ifndef OPENSSL_NO_SSL2 will get included,
> irrespective of how OpenSSL has been compiled.
>
The usual way is to use no-ssl2 as an argument to Configure or config which then
adds OPENSSL_NO_SSL2 into crypto/opensslconf.h
Steve.
--
Dr Stephen Henson. OpenSSL Software Foundation, Inc.
1829 Mount Ephraim Road
Adamstown, MD 21710
+1 877-673-6775
shenson@opensslfoundation.com
Re: svn commit: r1374640 - /httpd/httpd/branches/2.2.x/STATUS
Posted by Kaspar Brand <ht...@velox.ch>.
On 18.8.12 21:51, William A. Rowe Jr. wrote:
>> to drop the #ifndef around SSL_PROTOCOL_SSLV2 in ssl_private.h,
>> this should also make some of the other "#if[n]def OPENSSL_NO_SSL2"
>> encapsulations unnecessary.
>> + [wrowe] agreed the patch was wrong, the #ifdef needed to be moved
>> + up four lines. Behavior is now correct in patch .2
>> + Disagree about retaining SSL_PROTOCOL_SSLV2; this is one
>> + of the most basic design patterns which exists to ensure
>> + that we don't have some lingering code which is still
>> + attempting to pursue SSLV2 games, not to mention that
>> + the various macros and functions in those blocks may
>> + simply disappear disappear in an OPENSSL_NO_SSL2 build.
>> + Bad idea, it helps us catch current and future problems.
After a closer look at the mechanics of OPENSSL_NO_SSL2 in OpenSSL, I
think there's a misunderstanding of how OpenSSL exposes this
compile-time option to applications linking against libssl. OpenSSL
itself only defines OPENSSL_NO_SSL2 in the following case (openssl/ssl.h):
> #if (defined(OPENSSL_NO_RSA) || defined(OPENSSL_NO_MD5)) && !defined(OPENSSL_NO_SSL2)
> #define OPENSSL_NO_SSL2
> #endif
(ssl.h is not customized by OpenSSL's Configure script, AFAICT you would
have to call "openssl version -f" and look for any flags set at compile
time.)
I.e., unless mod_ssl is explicitly compiled with -DOPENSSL_NO_SSL2 (set
through CPPFLAGS/CFLAGS), an #ifdef OPENSSL_NO_SSL2 has no effect, and
the blocks enclosed with #ifndef OPENSSL_NO_SSL2 will get included,
irrespective of how OpenSSL has been compiled.
> the flexibility to disable a particular cipher in light of exploit
> research in the very fresh openssl code base makes this patch pretty
> critical to release for legacy, as well as stable.
I'm not sure what exactly you're referring to - is there new research
showing that it's more than a DoS issue (which is what
http://www.openssl.org/news/secadv_20120510.txt currently states)?
Generally speaking, users of OpenSSL 1.0.1 (and 1.0.1a/1.0.1b) should
upgrade their OpenSSL libraries... I would consider it rather awkward to
recommend them an upgrade to httpd 2.2.23, only for being able to
disable TLS 1.1/1.2.
Kaspar