Re: Patch for 2.0.54 + OpenSSL 0.9.8
Hi, thank you for the patch. It looks appropriate, to me, so I'll commit to 2.1.x and (if I can get two more +1's, folks???) I'll also apply to 2.0.55 before we roll in the next day. Even though I'm not a commiter, I've take a look at the patch and it seems good to me too. Regards, - Sam -- Maxime Petazzoni (http://www.bulix.org) -- gone crazy, back soon. leave message. signature.asc Description: Digital signature
Re: Patch for 2.0.54 + OpenSSL 0.9.8
At 04:01 AM 7/6/2005, Maxime Petazzoni wrote: thank you for the patch. It looks appropriate, to me, so I'll commit to 2.1.x and (if I can get two more +1's, folks???) I'll also apply to 2.0.55 before we roll in the next day. Even though I'm not a commiter, I've take a look at the patch and it seems good to me too. Thanks Sam - We do get slammed, and always appreciate all the available eyes to review patches! Bill
Re: Patch for 2.0.54 + OpenSSL 0.9.8
At 07:32 AM 7/5/2005, Georg v. Zezschwitz wrote:
However, currently 2.0.54 cannot be built with 0.9.8beta6, as
a pem.h-definition has changed. The OpenSSL-team considers this
renaming as a bug correction, so compilation of mod_ssl will
go on to fail.
I've committed that fix to both 2.0 and 2.1 trees, thank you!
In the future please separate each functional change into its
own patch.
I have attached a patch to make mod_ssl compile with OpenSSL 0.9.8,
and also added an SSL variable SSL_COMP_METHOD to allow logging
(and other usages) of the negotiated compression method.
My concern is that older libraries would break under this patch.
Do we care? I dunno - but just in case this is only committed to
the 2.1 dev branch for now.
I changed the variable name to SSL_COMPRESS_METHOD, to kill the
ambiguity of 'COMP'. I've also added this to the StdEnvVars, since
its not unreasonable for a CGI to behave differently, knowing that
compression is already applied to the stream.
For that matter, mod_deflate aught to peek at this variable and
just get out of the way :) Patches welcome.
Finally you had one small typo, a simple string compare was more
appropriate than a variable length compare (I was confused what
you ment, did you want to accept SSL_COMP_ME)?
The revised patch to 2.0.x is attached. Thanks again!
Bill #
# Add SSL_COMPRESS_METHOD variable (included in +StdEnvVars) to note
# the negotiated compression. [Georg v. Zezschwitz gvz 2scale.de]
#
Index: docs/manual/mod/mod_ssl.xml
===
--- docs/manual/mod/mod_ssl.xml (revision 209415)
+++ docs/manual/mod/mod_ssl.xml (working copy)
@@ -65,6 +65,7 @@
trtdcodeSSL_CIPHER_EXPORT/code/td tdstring/td
tdcodetrue/code if cipher is an export cipher/td/tr
trtdcodeSSL_CIPHER_USEKEYSIZE/code/td tdnumber/td
tdNumber of cipher bits (actually used)/td/tr
trtdcodeSSL_CIPHER_ALGKEYSIZE/code/td tdnumber/td
tdNumber of cipher bits (possible)/td/tr
+trtdcodeSSL_COMPRESS_METHOD/code/td tdstring/td
tdSSL compression method negotiated/td/tr
trtdcodeSSL_VERSION_INTERFACE/code/td tdstring/td
tdThe mod_ssl program version/td/tr
trtdcodeSSL_VERSION_LIBRARY/code/td tdstring/td
tdThe OpenSSL program version/td/tr
trtdcodeSSL_CLIENT_M_VERSION/code/td tdstring/td
tdThe version of the client certificate/td/tr
Index: docs/manual/ssl/ssl_faq.xml
===
--- docs/manual/ssl/ssl_faq.xml (revision 209415)
+++ docs/manual/ssl/ssl_faq.xml (working copy)
@@ -680,6 +680,7 @@
lia href=#vhostsHTTPS and name-based vhosts/a/li
lia href=#vhosts2Why is it not possible to use Name-Based Virtual
Hosting to identify different SSL virtual hosts?/a/li
+lia href=#compHow do I get SSL compression working?/a/li
lia href=#lockiconThe lock icon in Netscape locks very late/a/li
lia href=#msieWhy do I get I/O errors with MSIE clients?/a/li
lia href=#nnWhy do I get I/O errors with NS clients?/a/li
@@ -804,6 +805,23 @@
Use different port numbers for different SSL hosts./p
/section
+section id=comptitleHow do I get SSL compression working?/title
+pAlthough SSL compression negotiation was already defined in the
specification
+of SSLv2 and TLS, it took until May 2004 when RFC 3749 defined DEFLATE as
+a negotiable standard compression method.
+/p
+pOpenSSL 0.9.8 started to support this by default when compiled with the
+codezlib/code option. If both the client and the server support
compression,
+it will be used. However, most clients still try to initially connect with an
+SSLv2 Hello. As SSLv2 did not include an array of prefered compression
algorithms
+in its handshake, compression can not be negotiated with these clients.
+If the client disables support for SSLv2, based on the used SSL library
+a SSLv3 or TLS Hello might be sent and compression might be set up.
+You can verify if clients make use of SSL compression by logging the
+code%{SSL_COMPRESS_METHOD}x/code variable.
+/p
+/section
+
section id=lockicontitleWhen I use Basic Authentication over HTTPS the
lock icon in Netscape browsers
still shows the unlocked state when the dialog pops up. Does this mean the
username/password is still transmitted unencrypted?/title
Index: modules/ssl/ssl_engine_vars.c
===
--- modules/ssl/ssl_engine_vars.c (revision 209415)
+++ modules/ssl/ssl_engine_vars.c (working copy)
@@ -47,6 +47,7 @@
static char *ssl_var_lookup_ssl_cipher(apr_pool_t *p, conn_rec *c, char *var);
static void ssl_var_lookup_ssl_cipher_bits(SSL *ssl, int *usekeysize, int
*algkeysize);
static char *ssl_var_lookup_ssl_version(apr_pool_t *p, char *var);
+static char *ssl_var_lookup_ssl_compress_meth(SSL *ssl);
static int ssl_is_https(conn_rec *c)
{
@@ -282,6 +283,9 @@
if ((xs = SSL_get_certificate(ssl)) != NULL)
Re: Patch for 2.0.54 + OpenSSL 0.9.8
William A. Rowe, Jr. wrote: At 07:32 AM 7/5/2005, Georg v. Zezschwitz wrote: However, currently 2.0.54 cannot be built with 0.9.8beta6, as a pem.h-definition has changed. The OpenSSL-team considers this renaming as a bug correction, so compilation of mod_ssl will go on to fail. I've committed that fix to both 2.0 and 2.1 trees, thank you! In the future please separate each functional change into its own patch. Whoa there. This should not of been committed to the 2.0 tree directly. There was no vote. I didn't see anyone else even give it a +1 on the mailing list. That said, it is a simple change, and I will give it a +1, but please don't do that, 2.0.x is under RTC. -Paul
Re: Patch for 2.0.54 + OpenSSL 0.9.8
At 11:19 AM 7/6/2005, Paul Querna wrote: William A. Rowe, Jr. wrote: At 07:32 AM 7/5/2005, Georg v. Zezschwitz wrote: However, currently 2.0.54 cannot be built with 0.9.8beta6, as a pem.h-definition has changed. The OpenSSL-team considers this renaming as a bug correction, so compilation of mod_ssl will go on to fail. I've committed that fix to both 2.0 and 2.1 trees, thank you! In the future please separate each functional change into its own patch. Whoa there. This should not of been committed to the 2.0 tree directly. There was no vote. I didn't see anyone else even give it a +1 on the mailing list. That said, it is a simple change, and I will give it a +1, but please don't do that, 2.0.x is under RTC. Fair enough :) But I would not proceed to the 2.0 GA without this passing muster. That's two, so can Paul and I get another +1? Patch follows. Votes/Comments? Bill --- httpd/httpd/branches/2.0.x/modules/ssl/ssl_toolkit_compat.h (original) +++ httpd/httpd/branches/2.0.x/modules/ssl/ssl_toolkit_compat.h Wed Jul 6 08:10:36 2005 @@ -99,6 +99,13 @@ #define HAVE_SSL_X509V3_EXT_d2i #endif +#ifndef PEM_F_DEF_CALLBACK +#ifdef PEM_F_PEM_DEF_CALLBACK +/* In OpenSSL 0.9.8 PEM_F_DEF_CALLBACK was renamed */ +#define PEM_F_DEF_CALLBACK PEM_F_PEM_DEF_CALLBACK +#endif +#endif + #elif defined (SSLC_VERSION_NUMBER) /* RSA */ /* sslc does not support this function, OpenSSL has since 9.5.1 */
Re: Patch for 2.0.54 + OpenSSL 0.9.8
I think that there are a few more changes that need to be made. At least on NetWare it won't compile without the following additional patch. Brad Index: ssl_scache_shmcb.c === --- ssl_scache_shmcb.c (revision 201624) +++ ssl_scache_shmcb.c (working copy) @@ -1184,7 +1184,7 @@ SHMCBHeader *header; SSL_SESSION *pSession = NULL; unsigned int curr_pos, loop, count; -unsigned char *ptr; +const unsigned char *ptr; time_t now; ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, @@ -1262,7 +1262,7 @@ SHMCBIndex *idx; SHMCBHeader *header; unsigned int curr_pos, loop, count; -unsigned char *ptr; +const unsigned char *ptr; BOOL to_return = FALSE; ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, Index: ssl_scache_dbm.c === --- ssl_scache_dbm.c(revision 201624) +++ ssl_scache_dbm.c(working copy) @@ -257,7 +257,7 @@ } /* unstreamed SSL_SESSION */ -sess = d2i_SSL_SESSION(NULL, ucpData, nData); +sess = d2i_SSL_SESSION(NULL, (const UCHAR**)ucpData, nData); return sess; } [EMAIL PROTECTED] Wednesday, July 06, 2005 10:49:46 AM At 11:19 AM 7/6/2005, Paul Querna wrote: William A. Rowe, Jr. wrote: At 07:32 AM 7/5/2005, Georg v. Zezschwitz wrote: However, currently 2.0.54 cannot be built with 0.9.8beta6, as a pem.h-definition has changed. The OpenSSL-team considers this renaming as a bug correction, so compilation of mod_ssl will go on to fail. I've committed that fix to both 2.0 and 2.1 trees, thank you! In the future please separate each functional change into its own patch. Whoa there. This should not of been committed to the 2.0 tree directly. There was no vote. I didn't see anyone else even give it a +1 on the mailing list. That said, it is a simple change, and I will give it a +1, but please don't do that, 2.0.x is under RTC. Fair enough :) But I would not proceed to the 2.0 GA without this passing muster. That's two, so can Paul and I get another +1? Patch follows. Votes/Comments? Bill --- httpd/httpd/branches/2.0.x/modules/ssl/ssl_toolkit_compat.h (original) +++ httpd/httpd/branches/2.0.x/modules/ssl/ssl_toolkit_compat.h Wed Jul 6 08:10:36 2005 @@ -99,6 +99,13 @@ #define HAVE_SSL_X509V3_EXT_d2i #endif +#ifndef PEM_F_DEF_CALLBACK +#ifdef PEM_F_PEM_DEF_CALLBACK +/* In OpenSSL 0.9.8 PEM_F_DEF_CALLBACK was renamed */ +#define PEM_F_DEF_CALLBACK PEM_F_PEM_DEF_CALLBACK +#endif +#endif + #elif defined (SSLC_VERSION_NUMBER) /* RSA */ /* sslc does not support this function, OpenSSL has since 9.5.1 */
Re: Patch for 2.0.54 + OpenSSL 0.9.8
At 12:10 PM 7/6/2005, Brad Nicholes wrote: I think that there are a few more changes that need to be made. At least on NetWare it won't compile without the following additional patch. --- ssl_scache_shmcb.c (revision 201624) +++ ssl_scache_shmcb.c (working copy) This shmcb.c patch made sense... Index: ssl_scache_dbm.c === --- ssl_scache_dbm.c(revision 201624) +++ ssl_scache_dbm.c(working copy) @@ -257,7 +257,7 @@ } /* unstreamed SSL_SESSION */ -sess = d2i_SSL_SESSION(NULL, ucpData, nData); +sess = d2i_SSL_SESSION(NULL, (const UCHAR**)ucpData, nData); UCHAR? Sure that isn't a Netware-ism? Otherwise, +1 to this additional patch. Testing now. Bill
Re: Patch for 2.0.54 + OpenSSL 0.9.8
On Wed, Jul 06, 2005 at 11:10:33AM -0600, Brad Nicholes wrote: I think that there are a few more changes that need to be made. At least on NetWare it won't compile without the following additional patch. This one needs a typedef which is defined correctly for the version of OpenSSL in use, see http://issues.apache.org/bugzilla/show_bug.cgi?id=34520 - please don't commit that as-is. Index: ssl_scache_shmcb.c === --- ssl_scache_shmcb.c(revision 201624) +++ ssl_scache_shmcb.c(working copy) @@ -1184,7 +1184,7 @@ SHMCBHeader *header; SSL_SESSION *pSession = NULL; unsigned int curr_pos, loop, count; -unsigned char *ptr; +const unsigned char *ptr; time_t now; ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, ...
Re: Patch for 2.0.54 + OpenSSL 0.9.8
At 12:44 PM 7/6/2005, William A. Rowe, Jr. wrote: At 12:10 PM 7/6/2005, Brad Nicholes wrote: -sess = d2i_SSL_SESSION(NULL, ucpData, nData); +sess = d2i_SSL_SESSION(NULL, (const UCHAR**)ucpData, nData); UCHAR? Sure that isn't a Netware-ism? My bad, I'm seeing it. Otherwise, +1 to this additional patch. Testing now. Issue; you toggled from one to the other - so now we have failures in 0.9.7 and not 0.9.8. Working out a better patch now. Bill
Re: Patch for 2.0.54 + OpenSSL 0.9.8
William A. Rowe, Jr. wrote: I have attached a patch to make mod_ssl compile with OpenSSL 0.9.8, and also added an SSL variable SSL_COMP_METHOD to allow logging (and other usages) of the negotiated compression method. My concern is that older libraries would break under this patch. Do we care? Why? There is a verification that a) there is an OpenSSL version number b) it is = 0.9.8 Otherwise NULL is returned (which should be right for older versions). I guess compression might get attractive as soon as the first browsers support it, and then people might get quite curious how many clients run compression. So, I think the variable could be quite interesting. I changed the variable name to SSL_COMPRESS_METHOD, to kill the ambiguity of 'COMP'. I've also added this to the StdEnvVars, since its not unreasonable for a CGI to behave differently, knowing that compression is already applied to the stream. Absolutely, sorry! For that matter, mod_deflate aught to peek at this variable and just get out of the way :) Patches welcome. I'd like to suggest another patch, too: Some people might prefer to disable compression for CPU load reasons. Instead of compiling an OpenSSL version for these needs, a Apache directive would be helpful. Finally you had one small typo, a simple string compare was more appropriate than a variable length compare (I was confused what you ment, did you want to accept SSL_COMP_ME)? No, I made a CopyPaste-flaw... Sorry, a 2nd time. Cheers, Georg
Re: Patch for 2.0.54 + OpenSSL 0.9.8
I'm running into the same const problem here as well on the calls to d2i_X509()
and d2i_PrivateKey(). Add these to your patch reworking.
Index: ssl_engine_init.c
===
--- ssl_engine_init.c (revision 209481)
+++ ssl_engine_init.c (working copy)
@@ -719,7 +719,7 @@
{
SSLModConfigRec *mc = myModConfig(s);
ssl_asn1_t *asn1;
-unsigned char *ptr;
+const unsigned char *ptr;
const char *type = ssl_asn1_keystr(idx);
X509 *cert;
@@ -757,7 +757,7 @@
{
SSLModConfigRec *mc = myModConfig(s);
ssl_asn1_t *asn1;
-unsigned char *ptr;
+const unsigned char *ptr;
const char *type = ssl_asn1_keystr(idx);
int pkey_type = (idx == SSL_AIDX_RSA) ? EVP_PKEY_RSA : EVP_PKEY_DSA;
EVP_PKEY *pkey;
William A. Rowe, Jr. [EMAIL PROTECTED] Wednesday, July 06, 2005
12:06:45 PM
At 12:44 PM 7/6/2005, William A. Rowe, Jr. wrote:
At 12:10 PM 7/6/2005, Brad Nicholes wrote:
-sess = d2i_SSL_SESSION(NULL, ucpData, nData);
+sess = d2i_SSL_SESSION(NULL, (const UCHAR**)ucpData, nData);
UCHAR? Sure that isn't a Netware-ism?
My bad, I'm seeing it.
Otherwise, +1 to this additional patch. Testing now.
Issue; you toggled from one to the other - so now we have failures
in 0.9.7 and not 0.9.8. Working out a better patch now.
Bill
Re: Patch for 2.0.54 + OpenSSL 0.9.8
At 03:04 PM 7/6/2005, Brad Nicholes wrote: I'm running into the same const problem here as well on the calls to d2i_X509() and d2i_PrivateKey(). Add these to your patch reworking. Sorry; I'm not seeing that. Are you looking at 2.1-dev or 2.0.x branch? Which version of OpenSSL? Please advise. Committing the corrected patch now for the single case we were considering. Bill
Re: Patch for 2.0.54 + OpenSSL 0.9.8
Compiling 2.1-dev on NetWare using OpenSSL 0.9.8-beta4 Brad [EMAIL PROTECTED] Wednesday, July 06, 2005 2:39:18 PM At 03:04 PM 7/6/2005, Brad Nicholes wrote: I'm running into the same const problem here as well on the calls to d2i_X509() and d2i_PrivateKey(). Add these to your patch reworking. Sorry; I'm not seeing that. Are you looking at 2.1-dev or 2.0.x branch? Which version of OpenSSL? Please advise. Committing the corrected patch now for the single case we were considering. Bill
Re: Patch for 2.0.54 + OpenSSL 0.9.8
Good morning,
Getting the same result on OpenSSL-0.9.8 (final);
Compiling ssl_engine_init.c
### mwccnlm Compiler:
#File: ssl_engine_init.c
# --
# 734: if (!(cert = d2i_X509(NULL, ptr, asn1-nData))) {
# Error: ^
# illegal implicit conversion from 'unsigned char **' to
# 'const unsigned char **'
### mwccnlm Compiler:
# 773: if (!(pkey = d2i_PrivateKey(pkey_type, NULL, ptr,
asn1-nData)))
# Error:^
# illegal implicit conversion from 'unsigned char **' to
# 'const unsigned char **'
Errors caused tool to abort.
make[2]: *** [Release.o/ssl_engine_init.o] Error 1
make[1]: *** [ssl] Error 2
make: *** [modules] Error 2
Norm
Brad Nicholes wrote:
Compiling 2.1-dev on NetWare using OpenSSL 0.9.8-beta4
Brad
[EMAIL PROTECTED] Wednesday, July 06, 2005 2:39:18 PM
At 03:04 PM 7/6/2005, Brad Nicholes wrote:
I'm running into the same const problem here as well on the calls to d2i_X509()
and d2i_PrivateKey(). Add these to your patch reworking.
Sorry; I'm not seeing that. Are you looking at 2.1-dev or 2.0.x
branch? Which version of OpenSSL?
Please advise. Committing the corrected patch now for the single
case we were considering.
Bill
.
Re: Patch for 2.0.54 + OpenSSL 0.9.8
Georg, thank you for the patch. It looks appropriate, to me, so I'll commit to 2.1.x and (if I can get two more +1's, folks???) I'll also apply to 2.0.55 before we roll in the next day. Bill At 07:32 AM 7/5/2005, Georg v. Zezschwitz wrote: Hi, the OpenSSL team will pretty soon release 0.9.8 as stable release. However, currently 2.0.54 cannot be built with 0.9.8beta6, as a pem.h-definition has changed. The OpenSSL-team considers this renaming as a bug correction, so compilation of mod_ssl will go on to fail. OpenSSL 0.9.8 will introduce SSL compression (which has in principle been defined since SSLv3, however, concrete compression methods like DEFLATE were RFCed in May 2004). I have attached a patch to make mod_ssl compile with OpenSSL 0.9.8, and also added an SSL variable SSL_COMP_METHOD to allow logging (and other usages) of the negotiated compression method. Moreover, I have - to my best knowledge - extended the documentation. Could anybody check this and decide if to include? I guess quite some confusion will happen if OpenSSL 0.9.8 is finally out and using it with Apache 2.0.54 fails. Also, please don't kill the messenger :-) - it is not my decision to change the OpenSSL include file definition... Regards, Georg v.Zezschwitz
