You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by fu...@apache.org on 2009/07/28 04:08:33 UTC
svn commit: r798359 - in /httpd/httpd/branches/2.2.x:
modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c
modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c support/ab.c
Author: fuankg
Date: Tue Jul 28 02:08:32 2009
New Revision: 798359
URL: http://svn.apache.org/viewvc?rev=798359&view=rev
Log:
backport support for OpenSSL 1.0.0 from HEAD. Based on:
http://svn.apache.org/viewvc?view=rev&revision=748396
http://svn.apache.org/viewvc?view=rev&revision=749466
http://svn.apache.org/viewvc?view=rev&revision=798274
Modified:
httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c
httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c
httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c
httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c
httpd/httpd/branches/2.2.x/support/ab.c
Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c?rev=798359&r1=798358&r2=798359&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c (original)
+++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c Tue Jul 28 02:08:32 2009
@@ -573,7 +573,7 @@
ssl_die();
}
- SSL_CTX_set_client_CA_list(ctx, (STACK *)ca_list);
+ SSL_CTX_set_client_CA_list(ctx, ca_list);
}
/*
Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c?rev=798359&r1=798358&r2=798359&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c (original)
+++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c Tue Jul 28 02:08:32 2009
@@ -222,7 +222,7 @@
X509_STORE *cert_store = NULL;
X509_STORE_CTX cert_store_ctx;
STACK_OF(SSL_CIPHER) *cipher_list_old = NULL, *cipher_list = NULL;
- SSL_CIPHER *cipher = NULL;
+ const SSL_CIPHER *cipher = NULL;
int depth, verify_old, verify, n;
if (ssl) {
@@ -668,7 +668,7 @@
* sk_X509_shift-ed the peer cert out of the chain.
* we put it back here for the purpose of quick_renegotiation.
*/
- cert_stack = sk_new_null();
+ cert_stack = sk_X509_new_null();
sk_X509_push(cert_stack, MODSSL_PCHAR_CAST cert);
}
Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c?rev=798359&r1=798358&r2=798359&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c (original)
+++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c Tue Jul 28 02:08:32 2009
@@ -628,7 +628,7 @@
ssl_var_lookup_ssl_cipher_bits(ssl, &usekeysize, &algkeysize);
if (ssl && strEQ(var, "")) {
- SSL_CIPHER *cipher = SSL_get_current_cipher(ssl);
+ const SSL_CIPHER *cipher = SSL_get_current_cipher(ssl);
result = (cipher != NULL ? (char *)SSL_CIPHER_get_name(cipher) : NULL);
}
else if (strcEQ(var, "_EXPORT"))
@@ -649,7 +649,7 @@
static void ssl_var_lookup_ssl_cipher_bits(SSL *ssl, int *usekeysize, int *algkeysize)
{
- SSL_CIPHER *cipher;
+ const SSL_CIPHER *cipher;
*usekeysize = 0;
*algkeysize = 0;
Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c?rev=798359&r1=798358&r2=798359&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c (original)
+++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c Tue Jul 28 02:08:32 2009
@@ -294,7 +294,7 @@
#ifdef HAVE_SSL_X509V3_EXT_d2i
X509_EXTENSION *ext;
int ext_nid;
- STACK *sk;
+ EXTENDED_KEY_USAGE *sk;
BOOL is_sgc;
int idx;
int i;
@@ -303,9 +303,9 @@
idx = X509_get_ext_by_NID(cert, NID_ext_key_usage, -1);
if (idx >= 0) {
ext = X509_get_ext(cert, idx);
- if ((sk = (STACK *)X509V3_EXT_d2i(ext)) != NULL) {
- for (i = 0; i < sk_num(sk); i++) {
- ext_nid = OBJ_obj2nid((ASN1_OBJECT *)sk_value(sk, i));
+ if ((sk = (EXTENDED_KEY_USAGE *)X509V3_EXT_d2i(ext)) != NULL) {
+ for (i = 0; i < sk_ASN1_OBJECT_num(sk); i++) {
+ ext_nid = OBJ_obj2nid((ASN1_OBJECT *)sk_ASN1_OBJECT_value(sk, i));
if (ext_nid == NID_ms_sgc || ext_nid == NID_ns_sgc) {
is_sgc = TRUE;
break;
@@ -467,7 +467,7 @@
X509 *x509;
unsigned long err;
int n;
- STACK *extra_certs;
+ STACK_OF(X509) *extra_certs;
if ((bio = BIO_new(BIO_s_file_internal())) == NULL)
return -1;
Modified: httpd/httpd/branches/2.2.x/support/ab.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/support/ab.c?rev=798359&r1=798358&r2=798359&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/support/ab.c (original)
+++ httpd/httpd/branches/2.2.x/support/ab.c Tue Jul 28 02:08:32 2009
@@ -189,6 +189,12 @@
#endif
+#if defined(USE_SSL) && (OPENSSL_VERSION_NUMBER >= 0x00909000)
+#define AB_SSL_METHOD_CONST const
+#else
+#define AB_SSL_METHOD_CONST
+#endif
+
#include <math.h>
#if APR_HAVE_CTYPE_H
#include <ctype.h>
@@ -480,7 +486,7 @@
static int ssl_print_connection_info(BIO *bio, SSL *ssl)
{
- SSL_CIPHER *c;
+ const SSL_CIPHER *c;
int alg_bits,bits;
c = SSL_get_current_cipher(ssl);
@@ -566,7 +572,7 @@
if (verbosity >= 2)
ssl_print_info(c);
if (ssl_info == NULL) {
- SSL_CIPHER *ci;
+ const SSL_CIPHER *ci;
X509 *cert;
int sk_bits, pk_bits, swork;
@@ -1981,7 +1987,7 @@
const char *optarg;
char c;
#ifdef USE_SSL
- SSL_METHOD *meth = SSLv23_client_method();
+ AB_SSL_METHOD_CONST SSL_METHOD *meth = SSLv23_client_method();
#endif
/* table defaults */
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Nick Kew <ni...@webthing.com>.
Guenter Knauf wrote:
> and even more clear was this:
> "So +1 for committing and I'll commit to helping review-after-commit."
> review-after-commit means for me in this case CTR instead of RTC.
That sounds like /trunk/ rules. 2.2 is firmly RTC - hence the
problem. +1 to Paul's veto in 2.2.
Since I'm posting on the subject before rather than after TIAS,
perhaps someone can clarify whether this patch has any effect
on users with the older OpenSSL version?
--
Nick Kew
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Guenter Knauf <fu...@apache.org>.
Hi,
Paul Querna schrieb:
> -1 veto, please revert this commit.
done:
http://svn.apache.org/viewvc?view=rev&revision=798508
> Unless I missed something, these changes were not voted on in the
> STATUS file.
no, you missed nothing, I was also very suprised about Bill's reply. My
own intention was very clear since I even didnt directly propose the
backport, but first only posted the 2.2.x patch to the list in order to
hear other's oppinions.
> I think wrowe's endorsement was... badly worded.
sorry, English is not my maiden's language, but from my understanding
Bill's post was cristal-clear:
"Since 2.2.12 just shipped, I'd say apply it. It will be easier for
everyone to validate from svn, no?"
and even more clear was this:
"So +1 for committing and I'll commit to helping review-after-commit."
review-after-commit means for me in this case CTR instead of RTC.
Gün.
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c support/ab.c
Posted by Sander Temme <sc...@apache.org>.
On Jul 27, 2009, at 7:13 PM, Paul Querna wrote:
> -1 veto, please revert this commit.
>
> Unless I missed something, these changes were not voted on in the
> STATUS file. I think wrowe's endorsement was... badly worded.
Yeah, let's do this the right way and I will review and vote swiftly.
S.
> Thanks,
>
> Paul
>
>
> On Mon, Jul 27, 2009 at 7:08 PM, <fu...@apache.org> wrote:
>> Author: fuankg
>> Date: Tue Jul 28 02:08:32 2009
>> New Revision: 798359
>>
>> URL: http://svn.apache.org/viewvc?rev=798359&view=rev
>> Log:
>> backport support for OpenSSL 1.0.0 from HEAD. Based on:
>> http://svn.apache.org/viewvc?view=rev&revision=748396
>> http://svn.apache.org/viewvc?view=rev&revision=749466
>> http://svn.apache.org/viewvc?view=rev&revision=798274
>>
>> Modified:
>> httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c
>> httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c
>> httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c
>> httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c
>> httpd/httpd/branches/2.2.x/support/ab.c
>>
>> Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c?rev=798359&r1=798358&r2=798359&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c
>> (original)
>> +++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c Tue
>> Jul 28 02:08:32 2009
>> @@ -573,7 +573,7 @@
>> ssl_die();
>> }
>>
>> - SSL_CTX_set_client_CA_list(ctx, (STACK *)ca_list);
>> + SSL_CTX_set_client_CA_list(ctx, ca_list);
>> }
>>
>> /*
>>
>> Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c?rev=798359&r1=798358&r2=798359&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c
>> (original)
>> +++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c Tue
>> Jul 28 02:08:32 2009
>> @@ -222,7 +222,7 @@
>> X509_STORE *cert_store = NULL;
>> X509_STORE_CTX cert_store_ctx;
>> STACK_OF(SSL_CIPHER) *cipher_list_old = NULL, *cipher_list =
>> NULL;
>> - SSL_CIPHER *cipher = NULL;
>> + const SSL_CIPHER *cipher = NULL;
>> int depth, verify_old, verify, n;
>>
>> if (ssl) {
>> @@ -668,7 +668,7 @@
>> * sk_X509_shift-ed the peer cert out of the chain.
>> * we put it back here for the purpose of
>> quick_renegotiation.
>> */
>> - cert_stack = sk_new_null();
>> + cert_stack = sk_X509_new_null();
>> sk_X509_push(cert_stack, MODSSL_PCHAR_CAST cert);
>> }
>>
>>
>> Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c?rev=798359&r1=798358&r2=798359&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c
>> (original)
>> +++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c Tue
>> Jul 28 02:08:32 2009
>> @@ -628,7 +628,7 @@
>> ssl_var_lookup_ssl_cipher_bits(ssl, &usekeysize, &algkeysize);
>>
>> if (ssl && strEQ(var, "")) {
>> - SSL_CIPHER *cipher = SSL_get_current_cipher(ssl);
>> + const SSL_CIPHER *cipher = SSL_get_current_cipher(ssl);
>> result = (cipher != NULL ? (char
>> *)SSL_CIPHER_get_name(cipher) : NULL);
>> }
>> else if (strcEQ(var, "_EXPORT"))
>> @@ -649,7 +649,7 @@
>>
>> static void ssl_var_lookup_ssl_cipher_bits(SSL *ssl, int
>> *usekeysize, int *algkeysize)
>> {
>> - SSL_CIPHER *cipher;
>> + const SSL_CIPHER *cipher;
>>
>> *usekeysize = 0;
>> *algkeysize = 0;
>>
>> Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c?rev=798359&r1=798358&r2=798359&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c (original)
>> +++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c Tue Jul
>> 28 02:08:32 2009
>> @@ -294,7 +294,7 @@
>> #ifdef HAVE_SSL_X509V3_EXT_d2i
>> X509_EXTENSION *ext;
>> int ext_nid;
>> - STACK *sk;
>> + EXTENDED_KEY_USAGE *sk;
>> BOOL is_sgc;
>> int idx;
>> int i;
>> @@ -303,9 +303,9 @@
>> idx = X509_get_ext_by_NID(cert, NID_ext_key_usage, -1);
>> if (idx >= 0) {
>> ext = X509_get_ext(cert, idx);
>> - if ((sk = (STACK *)X509V3_EXT_d2i(ext)) != NULL) {
>> - for (i = 0; i < sk_num(sk); i++) {
>> - ext_nid = OBJ_obj2nid((ASN1_OBJECT *)sk_value(sk,
>> i));
>> + if ((sk = (EXTENDED_KEY_USAGE *)X509V3_EXT_d2i(ext)) !=
>> NULL) {
>> + for (i = 0; i < sk_ASN1_OBJECT_num(sk); i++) {
>> + ext_nid = OBJ_obj2nid((ASN1_OBJECT
>> *)sk_ASN1_OBJECT_value(sk, i));
>> if (ext_nid == NID_ms_sgc || ext_nid == NID_ns_sgc) {
>> is_sgc = TRUE;
>> break;
>> @@ -467,7 +467,7 @@
>> X509 *x509;
>> unsigned long err;
>> int n;
>> - STACK *extra_certs;
>> + STACK_OF(X509) *extra_certs;
>>
>> if ((bio = BIO_new(BIO_s_file_internal())) == NULL)
>> return -1;
>>
>> Modified: httpd/httpd/branches/2.2.x/support/ab.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/support/ab.c?rev=798359&r1=798358&r2=798359&view=diff
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- httpd/httpd/branches/2.2.x/support/ab.c (original)
>> +++ httpd/httpd/branches/2.2.x/support/ab.c Tue Jul 28 02:08:32 2009
>> @@ -189,6 +189,12 @@
>>
>> #endif
>>
>> +#if defined(USE_SSL) && (OPENSSL_VERSION_NUMBER >= 0x00909000)
>> +#define AB_SSL_METHOD_CONST const
>> +#else
>> +#define AB_SSL_METHOD_CONST
>> +#endif
>> +
>> #include <math.h>
>> #if APR_HAVE_CTYPE_H
>> #include <ctype.h>
>> @@ -480,7 +486,7 @@
>>
>> static int ssl_print_connection_info(BIO *bio, SSL *ssl)
>> {
>> - SSL_CIPHER *c;
>> + const SSL_CIPHER *c;
>> int alg_bits,bits;
>>
>> c = SSL_get_current_cipher(ssl);
>> @@ -566,7 +572,7 @@
>> if (verbosity >= 2)
>> ssl_print_info(c);
>> if (ssl_info == NULL) {
>> - SSL_CIPHER *ci;
>> + const SSL_CIPHER *ci;
>> X509 *cert;
>> int sk_bits, pk_bits, swork;
>>
>> @@ -1981,7 +1987,7 @@
>> const char *optarg;
>> char c;
>> #ifdef USE_SSL
>> - SSL_METHOD *meth = SSLv23_client_method();
>> + AB_SSL_METHOD_CONST SSL_METHOD *meth = SSLv23_client_method();
>> #endif
>>
>> /* table defaults */
>>
>>
>>
>
>
--
Sander Temme
sctemme@apache.org
PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Nick Kew wrote:
>
> What do you mean by "override STATUS flow"?
The convention is "propose in status, collect votes, commit when approved".
But the policy is not "propose in STATUS", the *policy* is "review, then
commit". The devs can work these rules in whatever manner best accomplishes
forward progress, so this list is free to suspend these rules in specific
cases. An example are platform patches; nobody complains if Guenter just
fixes a Netware build flaw, if Bill patches the win32 build files, or if
Joe went ahead and fixed the rpm build files.
Common sources generally require review-then-commit. STATUS does NOT
override the dev@ list for decision making. It's simply a vehicle of
convenience. Any review that occurs on dev@ is just as binding, if not
more so, than STATUS. Only *this list* gives STATUS binding authority.
If some minority vote against patches based on their absence from STATUS,
so be it. Seems like bureaucracy to me in certain cases, and entirely
the most efficient approach in other cases.
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c support/ab.c
Posted by Nick Kew <ni...@webthing.com>.
On 28 Jul 2009, at 21:29, William A. Rowe, Jr. wrote:
> Paul Querna wrote:
>> -1 veto, please revert this commit.
>>
>> Unless I missed something, these changes were not voted on in the
>> STATUS file. I think wrowe's endorsement was... badly worded.
>
> wrowe's endorsement was fine, and one of three votes required to
> override
> STATUS flow, so you are right - it's premature. We would need at
> least
> a third committer agreeing to apply then test then flush out due to
> the
> discrepancies between httpd 2.4 and 2.2 mod_ssl code bases.
What do you mean by "override STATUS flow"? STATUS serves as a
focal point for eyes, and a backport proposal that hasn't appeared in
STATUS has denied folks the proper forum for technical review and
platform for a veto!
> But I'd really rather we didn't kick around patch files due to all
> of the
> mismatches between trunk and 2.2.
A patch file is something you apply locally, as and when you get
around to reviewing the backport proposal in STATUS.
How is that a problem?
--
Nick Kew
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Peter Sylvester <pe...@edelweb.fr>.
> Instead it is another cleanup which should go the usual way = apply in
> HEAD, propose for backport. Please lets separate these things - the
> bigger we make the one 2.2.x backport patch the lesser the other
> developers are in the mood to review it.
>
I agree.
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Dr Stephen Henson wrote:
> William A. Rowe, Jr. wrote:
>> Pity that ml.exe support was stripped (ms\do_masm), it would be a shame
>> for most VC users to drop asm optimizations.
>
> This was mainly due to problems in using some of the more advanced features
> needed by the latest optimisations in all versions of MASM. Different versions
> needed different syntax and some things didn't work at all.
Which was the point of having a generator/translator in the first place :)
I'm happy to chime in on the openssl dev list in active support of this
compiler again. Obviously with 1.0.0 it's no longer necessary to support
an 11 year old assembler, more recent versions can be assumed, or those
users pointed at the no-asm or nasm build. Let's take it to that list.
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Dr Stephen Henson <sh...@oss-institute.org>.
William A. Rowe, Jr. wrote:
>
> Pity that ml.exe support was stripped (ms\do_masm), it would be a shame
> for most VC users to drop asm optimizations.
>
>
This was mainly due to problems in using some of the more advanced features
needed by the latest optimisations in all versions of MASM. Different versions
needed different syntax and some things didn't work at all.
VC users can use the free nasm instead.
Steve.
--
Dr Stephen N. Henson. Senior Technical/Cryptography Advisor,
Open Source Software Institute: www.oss-institute.org
OpenSSL Core team: www.openssl.org
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Guenter Knauf <fu...@apache.org>.
Bill,
William A. Rowe, Jr. schrieb:
> For trunk, it should be committed. Casts are generally signs of a dumb
> compiler or poor coding design and must be avoided.
done:
http://svn.apache.org/viewvc?view=rev&revision=798989
> I'm generally -1 for stylistic cleanup backports. Although it might bring
> trunk and the 2.2 branch into closer alignment, the fact is that they
> diverge significantly enough not to sweat that. If an actual compilation or
> cast/arg flaw is discovered on trunk, of course that should be backported.
I've added to the new 2.2.x diff.
> Almost compiles clean in all three on Win32 32 bit x86 architectures.
>
> For 1.0.0beta3;
>
> C:\local0\asf\build\httpd-2.2\modules\ssl\ssl_engine_init.c(1175) : warning
> C4028: formal parameter 1 different from declaration
> C:\local0\asf\build\httpd-2.2\modules\ssl\ssl_engine_init.c(1175) : warning
> C4028: formal parameter 2 different from declaration
>
> Apparently this is bogus;
>
> static int ssl_init_FindCAList_X509NameCmp(X509_NAME **a, X509_NAME **b)
>
> It seems this solves the emits...
>
> static int ssl_init_FindCAList_X509NameCmp(const X509_NAME * const*a,
> const X509_NAME * const*b)
argh! I missed that, thanks! Its part of r749466 which I refered to,
Now added to the 2.2.x diff, please review again:
http://people.apache.org/~fuankg/diffs/openssl-1.x-2.2.x.diff
> Well done :)
I disagree since I missed the part above from r749466 :(
Gün.
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Guenter Knauf wrote:
> Hi Peter,
> Peter Sylvester schrieb:
>> A little nit in ssl_engine_init.c:
>> instead of
>>
>> - SSL_CTX_set_client_CA_list(ctx, (STACK *)ca_list);
>> + SSL_CTX_set_client_CA_list(ctx, (STACK_OF(X509_NAME) *)ca_list);
>>
>> I think I'd prefer
>> + SSL_CTX_set_client_CA_list(ctx, ca_list);
> but this is not part of the proposed patch?
> http://people.apache.org/~fuankg/diffs/openssl-1.x-2.2.x.diff
> and its already fixed in HEAD:
> http://svn.apache.org/viewvc?view=rev&revision=798274
>
>> and a few lines later instead of
>>
>> ca_list = (STACK_OF(X509_NAME) *)SSL_CTX_get_client_CA_list(ctx);
>>
>> it should be
>> ca_list = SSL_CTX_get_client_CA_list(ctx);
> this is also not part of the patch in question nor is it required.
For trunk, it should be committed. Casts are generally signs of a dumb
compiler or poor coding design and must be avoided.
> Instead it is another cleanup which should go the usual way = apply in
> HEAD, propose for backport. Please lets separate these things - the
> bigger we make the one 2.2.x backport patch the lesser the other
> developers are in the mood to review it.
I'm generally -1 for stylistic cleanup backports. Although it might bring
trunk and the 2.2 branch into closer alignment, the fact is that they
diverge significantly enough not to sweat that. If an actual compilation or
cast/arg flaw is discovered on trunk, of course that should be backported.
> Please lets focus on the proposed 2.2.x patch only at the moment, and
> make sure that it compiles warning-free with 0.9.7, 0.9.8 and 1.0.0.
+1
> now that the patch was already in 2.2.x trunk it should be very easy to
> apply it, just do a:
> svn merge -r798358:798359
> https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x
> and you should get all hunks in ... :)
Almost compiles clean in all three on Win32 32 bit x86 architectures.
For 1.0.0beta3;
C:\local0\asf\build\httpd-2.2\modules\ssl\ssl_engine_init.c(1175) : warning
C4028: formal parameter 1 different from declaration
C:\local0\asf\build\httpd-2.2\modules\ssl\ssl_engine_init.c(1175) : warning
C4028: formal parameter 2 different from declaration
Apparently this is bogus;
static int ssl_init_FindCAList_X509NameCmp(X509_NAME **a, X509_NAME **b)
It seems this solves the emits...
static int ssl_init_FindCAList_X509NameCmp(const X509_NAME * const*a,
const X509_NAME * const*b)
So I'm +1'ing your STATUS entry, and recommending you also fix the constness
flaw above. Well done :)
Pity that ml.exe support was stripped (ms\do_masm), it would be a shame
for most VC users to drop asm optimizations.
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Guenter Knauf <fu...@apache.org>.
Hi Peter,
Peter Sylvester schrieb:
> A little nit in ssl_engine_init.c:
> instead of
>
> - SSL_CTX_set_client_CA_list(ctx, (STACK *)ca_list);
> + SSL_CTX_set_client_CA_list(ctx, (STACK_OF(X509_NAME) *)ca_list);
>
> I think I'd prefer
> + SSL_CTX_set_client_CA_list(ctx, ca_list);
but this is not part of the proposed patch?
http://people.apache.org/~fuankg/diffs/openssl-1.x-2.2.x.diff
and its already fixed in HEAD:
http://svn.apache.org/viewvc?view=rev&revision=798274
> and a few lines later instead of
>
> ca_list = (STACK_OF(X509_NAME) *)SSL_CTX_get_client_CA_list(ctx);
>
> it should be
> ca_list = SSL_CTX_get_client_CA_list(ctx);
this is also not part of the patch in question nor is it required.
Instead it is another cleanup which should go the usual way = apply in
HEAD, propose for backport. Please lets separate these things - the
bigger we make the one 2.2.x backport patch the lesser the other
developers are in the mood to review it.
Please lets focus on the proposed 2.2.x patch only at the moment, and
make sure that it compiles warning-free with 0.9.7, 0.9.8 and 1.0.0.
now that the patch was already in 2.2.x trunk it should be very easy to
apply it, just do a:
svn merge -r798358:798359
https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x
and you should get all hunks in ... :)
Gün.
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Peter Sylvester <pe...@edelweb.fr>.
Dr Stephen Henson wrote:
> Peter Sylvester wrote:
>
>>
>>> There is some non-portable code round there that accesses extensions
>>> in a most
>>> convoluted fashion for some unknown reason.
>>>
>>>
>> the stuff in ..vars.c ssl_ext_list?
>>
>
> Well that too but was mainly thinking of the extension handling code in
> ssl_util_ssl.c the loops in SSL_X509_getBC et al can be replaced by a single
> call to X509_get_ext_d2i which has been in existence as long as X509_EXT_d2i.
>
> SSL_X509_getCN is rather suspect too: it ignores the string type of commonName
> entries.
>
right, this is all called only to log the value in ssl_check_public_cert
as far as I see. for the bc stuff, well X509_EXT_print may be worth to
be considered.
> Steve.
>
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Dr Stephen Henson <sh...@oss-institute.org>.
Peter Sylvester wrote:
>
>
>> There is some non-portable code round there that accesses extensions
>> in a most
>> convoluted fashion for some unknown reason.
>>
> the stuff in ..vars.c ssl_ext_list?
Well that too but was mainly thinking of the extension handling code in
ssl_util_ssl.c the loops in SSL_X509_getBC et al can be replaced by a single
call to X509_get_ext_d2i which has been in existence as long as X509_EXT_d2i.
SSL_X509_getCN is rather suspect too: it ignores the string type of commonName
entries.
Steve.
--
Dr Stephen N. Henson. Senior Technical/Cryptography Advisor,
Open Source Software Institute: www.oss-institute.org
OpenSSL Core team: www.openssl.org
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Peter Sylvester <pe...@edelweb.fr>.
> I looked at the patch in question and it seems reasonable to me. That should
> work fine on much older versions of OpenSSL it's just that now some things are
> enforced that weren't before.
>
A little nit in ssl_engine_init.c:
instead of
- SSL_CTX_set_client_CA_list(ctx, (STACK *)ca_list);
+ SSL_CTX_set_client_CA_list(ctx, (STACK_OF(X509_NAME) *)ca_list);
I think I'd prefer
+ SSL_CTX_set_client_CA_list(ctx, ca_list);
and a few lines later instead of
ca_list = (STACK_OF(X509_NAME) *)SSL_CTX_get_client_CA_list(ctx);
it should be
ca_list = SSL_CTX_get_client_CA_list(ctx);
> There is some non-portable code round there that accesses extensions in a most
> convoluted fashion for some unknown reason.
>
the stuff in ..vars.c ssl_ext_list?
> Steve.
>
/p
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Dr Stephen Henson <sh...@oss-institute.org>.
Peter Sylvester wrote:
> William A. Rowe, Jr. wrote:
>> Paul Querna wrote:
>>
>>> -1 veto, please revert this commit.
>>>
>>> Unless I missed something, these changes were not voted on in the
>>> STATUS file. I think wrowe's endorsement was... badly worded.
>>>
>>
>> wrowe's endorsement was fine, and one of three votes required to override
>> STATUS flow, so you are right - it's premature. We would need at least
>> a third committer agreeing to apply then test then flush out due to the
>> discrepancies between httpd 2.4 and 2.2 mod_ssl code bases.
>>
>> But I'd really rather we didn't kick around patch files due to all of the
>> mismatches between trunk and 2.2.
>>
> /I think it would be a good idea to plan a release together with
> openssl 1.0.0, at least //when openssl 1.0.0 comes out, mod_ssl
> should be compilable./
> /
> Concerning the differences, I can see several categories:
>
> - changes due to stricter rules in openssl, this concerns the
> mentioned patches, in fact the 'const' and 'stack' stuff. This
> doesn't change any functionality. Not a big deal that
> this backport was forgotten in 2.2.12.
>
> - "simple" corrections and functional additions in the trunk.
> some may be worth to be backported.
>
I looked at the patch in question and it seems reasonable to me. That should
work fine on much older versions of OpenSSL it's just that now some things are
enforced that weren't before.
There is some non-portable code round there that accesses extensions in a most
convoluted fashion for some unknown reason.
Steve.
--
Dr Stephen N. Henson. Senior Technical/Cryptography Advisor,
Open Source Software Institute: www.oss-institute.org
OpenSSL Core team: www.openssl.org
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by Peter Sylvester <pe...@edelweb.fr>.
William A. Rowe, Jr. wrote:
> Paul Querna wrote:
>
>> -1 veto, please revert this commit.
>>
>> Unless I missed something, these changes were not voted on in the
>> STATUS file. I think wrowe's endorsement was... badly worded.
>>
>
> wrowe's endorsement was fine, and one of three votes required to override
> STATUS flow, so you are right - it's premature. We would need at least
> a third committer agreeing to apply then test then flush out due to the
> discrepancies between httpd 2.4 and 2.2 mod_ssl code bases.
>
> But I'd really rather we didn't kick around patch files due to all of the
> mismatches between trunk and 2.2.
>
/I think it would be a good idea to plan a release together with
openssl 1.0.0, at least //when openssl 1.0.0 comes out, mod_ssl
should be compilable./
/
Concerning the differences, I can see several categories:
- changes due to stricter rules in openssl, this concerns the
mentioned patches, in fact the 'const' and 'stack' stuff. This
doesn't change any functionality. Not a big deal that
this backport was forgotten in 2.2.12.
- "simple" corrections and functional additions in the trunk.
some may be worth to be backported.
- complex additions like sni, ocsp, locks, ... may be a
problem IF the underlying 0.9.8x and 1.0.0 code behave
differently, but so far I don't really see this, it is more likely
that some features in the trunk is not considered
sufficiently mature.
Thanks for reading
have fun.
Peter
/
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x: modules/ssl/ssl_engine_init.c
modules/ssl/ssl_engine_kernel.c modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c
support/ab.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Paul Querna wrote:
> -1 veto, please revert this commit.
>
> Unless I missed something, these changes were not voted on in the
> STATUS file. I think wrowe's endorsement was... badly worded.
wrowe's endorsement was fine, and one of three votes required to override
STATUS flow, so you are right - it's premature. We would need at least
a third committer agreeing to apply then test then flush out due to the
discrepancies between httpd 2.4 and 2.2 mod_ssl code bases.
But I'd really rather we didn't kick around patch files due to all of the
mismatches between trunk and 2.2.
Re: svn commit: r798359 - in /httpd/httpd/branches/2.2.x:
modules/ssl/ssl_engine_init.c modules/ssl/ssl_engine_kernel.c
modules/ssl/ssl_engine_vars.c modules/ssl/ssl_util_ssl.c support/ab.c
Posted by Paul Querna <pa...@querna.org>.
-1 veto, please revert this commit.
Unless I missed something, these changes were not voted on in the
STATUS file. I think wrowe's endorsement was... badly worded.
Thanks,
Paul
On Mon, Jul 27, 2009 at 7:08 PM, <fu...@apache.org> wrote:
> Author: fuankg
> Date: Tue Jul 28 02:08:32 2009
> New Revision: 798359
>
> URL: http://svn.apache.org/viewvc?rev=798359&view=rev
> Log:
> backport support for OpenSSL 1.0.0 from HEAD. Based on:
> http://svn.apache.org/viewvc?view=rev&revision=748396
> http://svn.apache.org/viewvc?view=rev&revision=749466
> http://svn.apache.org/viewvc?view=rev&revision=798274
>
> Modified:
> httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c
> httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c
> httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c
> httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c
> httpd/httpd/branches/2.2.x/support/ab.c
>
> Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c?rev=798359&r1=798358&r2=798359&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_init.c Tue Jul 28 02:08:32 2009
> @@ -573,7 +573,7 @@
> ssl_die();
> }
>
> - SSL_CTX_set_client_CA_list(ctx, (STACK *)ca_list);
> + SSL_CTX_set_client_CA_list(ctx, ca_list);
> }
>
> /*
>
> Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c?rev=798359&r1=798358&r2=798359&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_kernel.c Tue Jul 28 02:08:32 2009
> @@ -222,7 +222,7 @@
> X509_STORE *cert_store = NULL;
> X509_STORE_CTX cert_store_ctx;
> STACK_OF(SSL_CIPHER) *cipher_list_old = NULL, *cipher_list = NULL;
> - SSL_CIPHER *cipher = NULL;
> + const SSL_CIPHER *cipher = NULL;
> int depth, verify_old, verify, n;
>
> if (ssl) {
> @@ -668,7 +668,7 @@
> * sk_X509_shift-ed the peer cert out of the chain.
> * we put it back here for the purpose of quick_renegotiation.
> */
> - cert_stack = sk_new_null();
> + cert_stack = sk_X509_new_null();
> sk_X509_push(cert_stack, MODSSL_PCHAR_CAST cert);
> }
>
>
> Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c?rev=798359&r1=798358&r2=798359&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_engine_vars.c Tue Jul 28 02:08:32 2009
> @@ -628,7 +628,7 @@
> ssl_var_lookup_ssl_cipher_bits(ssl, &usekeysize, &algkeysize);
>
> if (ssl && strEQ(var, "")) {
> - SSL_CIPHER *cipher = SSL_get_current_cipher(ssl);
> + const SSL_CIPHER *cipher = SSL_get_current_cipher(ssl);
> result = (cipher != NULL ? (char *)SSL_CIPHER_get_name(cipher) : NULL);
> }
> else if (strcEQ(var, "_EXPORT"))
> @@ -649,7 +649,7 @@
>
> static void ssl_var_lookup_ssl_cipher_bits(SSL *ssl, int *usekeysize, int *algkeysize)
> {
> - SSL_CIPHER *cipher;
> + const SSL_CIPHER *cipher;
>
> *usekeysize = 0;
> *algkeysize = 0;
>
> Modified: httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c?rev=798359&r1=798358&r2=798359&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/ssl/ssl_util_ssl.c Tue Jul 28 02:08:32 2009
> @@ -294,7 +294,7 @@
> #ifdef HAVE_SSL_X509V3_EXT_d2i
> X509_EXTENSION *ext;
> int ext_nid;
> - STACK *sk;
> + EXTENDED_KEY_USAGE *sk;
> BOOL is_sgc;
> int idx;
> int i;
> @@ -303,9 +303,9 @@
> idx = X509_get_ext_by_NID(cert, NID_ext_key_usage, -1);
> if (idx >= 0) {
> ext = X509_get_ext(cert, idx);
> - if ((sk = (STACK *)X509V3_EXT_d2i(ext)) != NULL) {
> - for (i = 0; i < sk_num(sk); i++) {
> - ext_nid = OBJ_obj2nid((ASN1_OBJECT *)sk_value(sk, i));
> + if ((sk = (EXTENDED_KEY_USAGE *)X509V3_EXT_d2i(ext)) != NULL) {
> + for (i = 0; i < sk_ASN1_OBJECT_num(sk); i++) {
> + ext_nid = OBJ_obj2nid((ASN1_OBJECT *)sk_ASN1_OBJECT_value(sk, i));
> if (ext_nid == NID_ms_sgc || ext_nid == NID_ns_sgc) {
> is_sgc = TRUE;
> break;
> @@ -467,7 +467,7 @@
> X509 *x509;
> unsigned long err;
> int n;
> - STACK *extra_certs;
> + STACK_OF(X509) *extra_certs;
>
> if ((bio = BIO_new(BIO_s_file_internal())) == NULL)
> return -1;
>
> Modified: httpd/httpd/branches/2.2.x/support/ab.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/support/ab.c?rev=798359&r1=798358&r2=798359&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/support/ab.c (original)
> +++ httpd/httpd/branches/2.2.x/support/ab.c Tue Jul 28 02:08:32 2009
> @@ -189,6 +189,12 @@
>
> #endif
>
> +#if defined(USE_SSL) && (OPENSSL_VERSION_NUMBER >= 0x00909000)
> +#define AB_SSL_METHOD_CONST const
> +#else
> +#define AB_SSL_METHOD_CONST
> +#endif
> +
> #include <math.h>
> #if APR_HAVE_CTYPE_H
> #include <ctype.h>
> @@ -480,7 +486,7 @@
>
> static int ssl_print_connection_info(BIO *bio, SSL *ssl)
> {
> - SSL_CIPHER *c;
> + const SSL_CIPHER *c;
> int alg_bits,bits;
>
> c = SSL_get_current_cipher(ssl);
> @@ -566,7 +572,7 @@
> if (verbosity >= 2)
> ssl_print_info(c);
> if (ssl_info == NULL) {
> - SSL_CIPHER *ci;
> + const SSL_CIPHER *ci;
> X509 *cert;
> int sk_bits, pk_bits, swork;
>
> @@ -1981,7 +1987,7 @@
> const char *optarg;
> char c;
> #ifdef USE_SSL
> - SSL_METHOD *meth = SSLv23_client_method();
> + AB_SSL_METHOD_CONST SSL_METHOD *meth = SSLv23_client_method();
> #endif
>
> /* table defaults */
>
>
>