Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Joshua Slive

On Thu, 17 Jul 2003, William A. Rowe, Jr. wrote:
> Anyone strongly object to moving on to .29 so we catch this fix?
> Version numbers are cheap - but I think that 1.3'ers enjoy the fact
> that releases are few and far between.

Uhhh... proxypass to ftp urls is a rather obscure feature that is
obviously unused (since it doesn't work).  I don't see why this needs to
cause us any grief.

Joshua.


Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread William A. Rowe, Jr.
I should have read the rest of the thread first.  I'm happy with a .28 release
already and will do the win32-magic tonight.

Bill

At 07:43 AM 7/17/2003, Jim Jagielski wrote:
>Thom May wrote:
>> 
>> 
>> Hi folks,
>> so it seems that ProxyPass operation has been broken since at least 1.3.9;
>> we're currently firefighting our way through the list of debian bugs and
>> found http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=57316 - ProxyPass to
>> ftp urls causes apache to segfault.
>> The attached patch borrows from proxy_http.c's logic and was written by
>> Tollef Fog Heen, with hindrance from myself.
>> If no-one has any objections I'll commit the fix shortly.
>> Cheers,
>> -Thom
>> 
>
>Hmmm... I wonder if we should hold off on 1.3.28 then. I'm
>leaning towards releasing 1.3.28 as-is, and us placing this
>in patches/ (and of course, being committed to 1.3.29-dev)...

The problem is that for the win32 world, we would also distribute a
patched replacement for this module.  That seems unclean, and the
logging issue is fairly minor.

Anyone strongly object to moving on to .29 so we catch this fix?
Version numbers are cheap - but I think that 1.3'ers enjoy the fact
that releases are few and far between.

Bill





Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread William A. Rowe, Jr.
At 07:43 AM 7/17/2003, Jim Jagielski wrote:
>Thom May wrote:
>> 
>> 
>> Hi folks,
>> so it seems that ProxyPass operation has been broken since at least 1.3.9;
>> we're currently firefighting our way through the list of debian bugs and
>> found http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=57316 - ProxyPass to
>> ftp urls causes apache to segfault.
>> The attached patch borrows from proxy_http.c's logic and was written by
>> Tollef Fog Heen, with hindrance from myself.
>> If no-one has any objections I'll commit the fix shortly.
>> Cheers,
>> -Thom
>> 
>
>Hmmm... I wonder if we should hold off on 1.3.28 then. I'm
>leaning towards releasing 1.3.28 as-is, and us placing this
>in patches/ (and of course, being committed to 1.3.29-dev)...

The problem is that for the win32 world, we would also distribute a
patched replacement for this module.  That seems unclean, and the
logging issue is fairly minor.

Anyone strongly object to moving on to .29 so we catch this fix?
Version numbers are cheap - but I think that 1.3'ers enjoy the fact
that releases are few and far between.

Bill





Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Ben Laurie
Thom May wrote:

> Hi folks,
> so it seems that ProxyPass operation has been broken since at least 1.3.9;
> we're currently firefighting our way through the list of debian bugs and
> found http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=57316 - ProxyPass to
> ftp urls causes apache to segfault.
> The attached patch borrows from proxy_http.c's logic and was written by
> Tollef Fog Heen, with hindrance from myself.
> If no-one has any objections I'll commit the fix shortly.

You can't commit EAPI changes to the base.

-- 
http://www.apache-ssl.org/ben.html   http://www.thebunker.net/

"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff




Re: cvs commit: httpd-2.0/server util_md5.c

2003-07-17 Thread Joe Orton
On Thu, Jul 17, 2003 at 10:03:59AM -0700, Greg Stein wrote:
> On Thu, Jul 17, 2003 at 04:17:05PM -, [EMAIL PROTECTED] wrote:
> >...
> >   +++ util_md5.c17 Jul 2003 16:17:04 -  1.30
> >...
> >apr_md5_init(&context);
> >nbytes = sizeof(buf);
> >while (apr_file_read(infile, buf, &nbytes) == APR_SUCCESS) {
> >   - length += nbytes;
> > apr_md5_update(&context, buf, nbytes);
> >   +nbytes = sizeof(buf);
> >}
> 
> That indentation looks funny. Did a TAB character get in there?

There are TABs in the original code, the lines I added use spaces.


Re: cvs commit: httpd-2.0/server util_md5.c

2003-07-17 Thread Greg Stein
On Thu, Jul 17, 2003 at 04:17:05PM -, [EMAIL PROTECTED] wrote:
>...
>   +++ util_md5.c  17 Jul 2003 16:17:04 -  1.30
>...
>apr_md5_init(&context);
>nbytes = sizeof(buf);
>while (apr_file_read(infile, buf, &nbytes) == APR_SUCCESS) {
>   -   length += nbytes;
>   apr_md5_update(&context, buf, nbytes);
>   +nbytes = sizeof(buf);
>}

That indentation looks funny. Did a TAB character get in there?

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/


Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Thom May
* Jeff Trawick ([EMAIL PROTECTED]) wrote :
> Thom May wrote:
> 
> >--- build-tree/apache_1.3.27/src/modules/proxy/proxy_ftp.c   2002-04-07 
> >20:57:36.0 +0200
> >+++ build-tree/apache_1.3.27/src/modules/proxy/proxy_ftp.c   2003-07-17 
> >12:12:34.0 +0200
> ...
> >+#ifdef EAPI
> >+ap_hook_use("ap::mod_proxy::ftp::handler::set_destport", 
> >+AP_HOOK_SIG2(int,ptr), 
> >+AP_HOOK_TOPMOST,
> >+&destport, r);
> >+#endif /* EAPI */
> 
> no "ifdef EAPI" stuff should be present in real Apache for various reasons
> 
> (I figure you were going to yank this before committing, but just in 
> case...)
> 
yes, i was. I should've pulled it before posting, but didn't want to go
messing around hand editting diffs :-)
it's gone from the patch I've applied to my source tree.
Cheers for the review!
-Thom


[PATCH] mod_status optional extension hook

2003-07-17 Thread Joe Orton
This adds an optional hook so that modules can put their own status
information in the server-status page when mod_status is loaded.  
(mod_ssl has code which can use this to output session cache status).
Any objections?

* mod_status.h: New file.

* mod_status.c: Implement the status hook.
(status_handler): Call the status hook.

Index: mod_status.c
===
RCS file: /home/cvs/httpd-2.0/modules/generators/mod_status.c,v
retrieving revision 1.72
diff -u -r1.72 mod_status.c
--- mod_status.c3 Feb 2003 17:53:03 -   1.72
+++ mod_status.c17 Jul 2003 14:10:18 -
@@ -107,6 +107,7 @@
 #include 
 #include "scoreboard.h"
 #include "http_log.h"
+#include "mod_status.h"
 #if APR_HAVE_UNISTD_H
 #include 
 #endif
@@ -141,6 +142,12 @@
 
 int server_limit, thread_limit;
 
+/* Implement 'ap_run_status_hook': */
+AP_IMPLEMENT_OPTIONAL_HOOK_RUN_ALL(int,status_hook,
+   (request_rec *r, int flags),
+   (r, flags),
+   OK, DECLINED);
+
 /*
  * command-related code. This is here to prevent use of ExtendedStatus
  * without status_module included.
@@ -784,6 +791,16 @@
  "information you need to use the "
  "ExtendedStatus On directive.\n", r);
 }
+}
+
+{
+/* Run extension hooks to insert extra content. */
+int flags = 
+(short_report ? AP_STATUS_SHORT : 0) | 
+(no_table_report ? AP_STATUS_NOTABLE : 0) |
+(ap_extended_status ? AP_STATUS_EXTENDED : 0);
+
+ap_run_status_hook(r, flags);
 }
 
 if (!short_report) {
Index: mod_status.h
===
RCS file: mod_status.h
diff -N mod_status.h
--- /dev/null   1 Jan 1970 00:00:00 -
+++ mod_status.h17 Jul 2003 14:10:18 -
@@ -0,0 +1,70 @@
+/* 
+ * The Apache Software License, Version 1.1
+ *
+ * Copyright (c) 2003 The Apache Software Foundation.  All rights
+ * reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in
+ *the documentation and/or other materials provided with the
+ *distribution.
+ *
+ * 3. The end-user documentation included with the redistribution,
+ *if any, must include the following acknowledgment:
+ *   "This product includes software developed by the
+ *Apache Software Foundation (http://www.apache.org/)."
+ *Alternately, this acknowledgment may appear in the software itself,
+ *if and wherever such third-party acknowledgments normally appear.
+ *
+ * 4. The names "Apache" and "Apache Software Foundation" must
+ *not be used to endorse or promote products derived from this
+ *software without prior written permission. For written
+ *permission, please contact [EMAIL PROTECTED]
+ *
+ * 5. Products derived from this software may not be called "Apache",
+ *nor may "Apache" appear in their name, without prior written
+ *permission of the Apache Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
+ * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ * 
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals on behalf of the Apache Software Foundation.  For more
+ * information on the Apache Software Foundation, please see
+ * .
+ */
+
+#ifndef MOD_STATUS_H
+#define MOD_STATUS_H
+
+#include "ap_config.h"
+#include "httpd.h"
+
+#define AP_STATUS_SHORT(0x1)  /* short, non-HTML report requested */
+#define AP_STATUS_NOTABLE  (0x2)  /* HTML report without tables */
+#define AP_STATUS_EXTENDED (0x4)  /* detailed report */
+
+/* Optional hooks which can insert extra content in

Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Jeff Trawick
Thom May wrote:

--- build-tree/apache_1.3.27/src/modules/proxy/proxy_ftp.c  2002-04-07 
20:57:36.0 +0200
+++ build-tree/apache_1.3.27/src/modules/proxy/proxy_ftp.c  2003-07-17 
12:12:34.0 +0200
...
+#ifdef EAPI
+ap_hook_use("ap::mod_proxy::ftp::handler::set_destport", 
+AP_HOOK_SIG2(int,ptr), 
+AP_HOOK_TOPMOST,
+&destport, r);
+#endif /* EAPI */
no "ifdef EAPI" stuff should be present in real Apache for various reasons

(I figure you were going to yank this before committing, but just in 
case...)




Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread David Reid
Let's release 1.3.28...

david
- Original Message -
From: "Thom May" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>; <[EMAIL PROTECTED]>
Cc: <[EMAIL PROTECTED]>
Sent: Thursday, July 17, 2003 1:49 PM
Subject: Re: [PATCH][1.3] Segfault in mod_proxy


> * Jim Jagielski ([EMAIL PROTECTED]) wrote :
> > Thom May wrote:
> > >
> > >
> > > Hi folks,
> >
> > Hmmm... I wonder if we should hold off on 1.3.28 then. I'm
> > leaning towards releasing 1.3.28 as-is, and us placing this
> > in patches/ (and of course, being committed to 1.3.29-dev)...
> >
> > Any objections?
> I don't see that we should hold off 1.3.28, but maybe bring up the
timescale
> for .29 a bit afterwards. So yes, +1.
> (Sorry for bringing this up today, but we litterally only found and fixed
it
> late last night)
> Cheers
> -Thom
>



Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Jeff Trawick
Jim Jagielski wrote:

Thom May wrote:

Hi folks,
so it seems that ProxyPass operation has been broken since at least 1.3.9;
we're currently firefighting our way through the list of debian bugs and
found http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=57316 - ProxyPass to
ftp urls causes apache to segfault.
The attached patch borrows from proxy_http.c's logic and was written by
Tollef Fog Heen, with hindrance from myself.
If no-one has any objections I'll commit the fix shortly.
Cheers,
-Thom


Hmmm... I wonder if we should hold off on 1.3.28 then. I'm
leaning towards releasing 1.3.28 as-is, and us placing this
in patches/ (and of course, being committed to 1.3.29-dev)...
Any objections?
Please release as-is.  This has gone on long enough ;)

Meanwhile, having fixes in the tree that people want in a real release 
is not the worst thing in the world but is instead an incentive for 
certain types of good behavior.




Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread André Malo
* Thom May wrote:

> so it seems that ProxyPass operation has been broken since at least 1.3.9;
> we're currently firefighting our way through the list of debian bugs and
> found http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=57316 - ProxyPass to
> ftp urls causes apache to segfault.
> The attached patch borrows from proxy_http.c's logic and was written by
> Tollef Fog Heen, with hindrance from myself.
> If no-one has any objections I'll commit the fix shortly.

looks good (though untested). +1.

nd


Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Thom May
* Jim Jagielski ([EMAIL PROTECTED]) wrote :
> Thom May wrote:
> > 
> > 
> > Hi folks,
> 
> Hmmm... I wonder if we should hold off on 1.3.28 then. I'm
> leaning towards releasing 1.3.28 as-is, and us placing this
> in patches/ (and of course, being committed to 1.3.29-dev)...
> 
> Any objections?
I don't see that we should hold off 1.3.28, but maybe bring up the timescale
for .29 a bit afterwards. So yes, +1.
(Sorry for bringing this up today, but we litterally only found and fixed it
late last night)
Cheers
-Thom


Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Jim Jagielski
=?ISO-8859-1?Q?Andr=E9?= Malo wrote:
> 
> +1. It's tagged already and therefore no chance for 1.3.28 get any patches in.
> 

Of course. That's why I asked if we should hold off on 1.3.28. I
didn't want (and don't want) to see 1.3.28 DOA. If I hadn't
tagged and rolled 1.3.28, it wouldn't even be an issue (almost :) ).

-- 
===
   Jim Jagielski   [|]   [EMAIL PROTECTED]   [|]   http://www.jaguNET.com/
  "A society that will trade a little liberty for a little order
 will lose both and deserve neither" - T.Jefferson


Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread André Malo
* Jim Jagielski wrote:

> Hmmm... I wonder if we should hold off on 1.3.28 then. I'm
> leaning towards releasing 1.3.28 as-is, and us placing this
> in patches/ (and of course, being committed to 1.3.29-dev)...

+1. It's tagged already and therefore no chance for 1.3.28 get any patches in.

nd


Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Bill Stoddard
Jim Jagielski wrote:

Thom May wrote:

Hi folks,
so it seems that ProxyPass operation has been broken since at least 1.3.9;
we're currently firefighting our way through the list of debian bugs and
found http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=57316 - ProxyPass to
ftp urls causes apache to segfault.
The attached patch borrows from proxy_http.c's logic and was written by
Tollef Fog Heen, with hindrance from myself.
If no-one has any objections I'll commit the fix shortly.
Cheers,
-Thom


Hmmm... I wonder if we should hold off on 1.3.28 then. 
Nope.

I'm
leaning towards releasing 1.3.28 as-is, and us placing this
in patches/ (and of course, being committed to 1.3.29-dev)...
Yep. Version numbers are cheap.

Bill



Re: [PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Jim Jagielski
Thom May wrote:
> 
> 
> Hi folks,
> so it seems that ProxyPass operation has been broken since at least 1.3.9;
> we're currently firefighting our way through the list of debian bugs and
> found http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=57316 - ProxyPass to
> ftp urls causes apache to segfault.
> The attached patch borrows from proxy_http.c's logic and was written by
> Tollef Fog Heen, with hindrance from myself.
> If no-one has any objections I'll commit the fix shortly.
> Cheers,
> -Thom
> 

Hmmm... I wonder if we should hold off on 1.3.28 then. I'm
leaning towards releasing 1.3.28 as-is, and us placing this
in patches/ (and of course, being committed to 1.3.29-dev)...

Any objections?
-- 
===
   Jim Jagielski   [|]   [EMAIL PROTECTED]   [|]   http://www.jaguNET.com/
  "A society that will trade a little liberty for a little order
 will lose both and deserve neither" - T.Jefferson


[PATCH][1.3] Segfault in mod_proxy

2003-07-17 Thread Thom May
Hi folks,
so it seems that ProxyPass operation has been broken since at least 1.3.9;
we're currently firefighting our way through the list of debian bugs and
found http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=57316 - ProxyPass to
ftp urls causes apache to segfault.
The attached patch borrows from proxy_http.c's logic and was written by
Tollef Fog Heen, with hindrance from myself.
If no-one has any objections I'll commit the fix shortly.
Cheers,
-Thom
--- build-tree/apache_1.3.27/src/modules/proxy/proxy_ftp.c  2002-04-07 
20:57:36.0 +0200
+++ build-tree/apache_1.3.27/src/modules/proxy/proxy_ftp.c  2003-07-17 
12:12:34.0 +0200
@@ -547,13 +547,14 @@
  */
 int ap_proxy_ftp_handler(request_rec *r, cache_req *c, char *url)
 {
-char *host, *path, *strp, *parms;
+char *desthost, *path, *strp, *parms;
+char *strp2;
 char *cwd = NULL;
 char *user = NULL;
 /*char *account = NULL; how to supply an account in a URL? */
 const char *password = NULL;
 const char *err;
-int port, i, j, len, rc, nocache = 0;
+int destport, i, j, len, rc, nocache = 0;
 int csd = 0, sock = -1, dsock = -1;
 struct sockaddr_in server;
 struct hostent server_hp;
@@ -562,6 +563,8 @@
 BUFF *ctrl = NULL;
 BUFF *data = NULL;
 pool *p = r->pool;
+char *destportstr = NULL;
+const char *urlptr = NULL;
 int one = 1;
 NET_SIZE_T clen;
 char xfer_type = 'A';   /* after ftp login, the default is ASCII */
@@ -593,17 +596,40 @@
 
 /* We break the URL into host, port, path-search */
 
-host = r->parsed_uri.hostname;
-port = (r->parsed_uri.port != 0)
-? r->parsed_uri.port
-: ap_default_port_for_request(r);
-path = ap_pstrdup(p, r->parsed_uri.path);
-if (path == NULL)
-path = "";
-else
-while (*path == '/')
-++path;
-
+urlptr = strstr(url, "://");
+if (urlptr == NULL)
+return HTTP_BAD_REQUEST;
+urlptr += 3;
+destport = 21;
+#ifdef EAPI
+ap_hook_use("ap::mod_proxy::ftp::handler::set_destport", 
+AP_HOOK_SIG2(int,ptr), 
+AP_HOOK_TOPMOST,
+&destport, r);
+#endif /* EAPI */
+strp = strchr(urlptr, '/');
+if (strp == NULL) {
+desthost = ap_pstrdup(p, urlptr);
+urlptr = "/";
+}
+else {
+char *q = ap_palloc(p, strp - urlptr + 1);
+memcpy(q, urlptr, strp - urlptr);
+q[strp - urlptr] = '\0';
+urlptr = strp;
+desthost = q;
+}
+
+strp2 = strchr(desthost, ':');
+if (strp2 != NULL) {
+*(strp2++) = '\0';
+if (ap_isdigit(*strp2)) {
+destport = atoi(strp2);
+destportstr = strp2;
+}
+}
+path = strchr(urlptr, '/')+1;
+
 /*
  * The "Authorization:" header must be checked first. We allow the user
  * to "override" the URL-coded user [ & password ] in the Browsers'
@@ -643,25 +669,25 @@
 }
 
 /* check if ProxyBlock directive on this host */
-destaddr.s_addr = ap_inet_addr(host);
+destaddr.s_addr = ap_inet_addr(desthost);
 for (i = 0; i < conf->noproxies->nelts; i++) {
 if (destaddr.s_addr == npent[i].addr.s_addr ||
 (npent[i].name != NULL &&
-  (npent[i].name[0] == '*' || strstr(host, npent[i].name) != NULL)))
+  (npent[i].name[0] == '*' || strstr(desthost, npent[i].name) != NULL)))
 return ap_proxyerror(r, HTTP_FORBIDDEN,
  "Connect to remote machine blocked");
 }
 
-ap_log_error(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, r->server, "FTP: connect to 
%s:%d", host, port);
+ap_log_error(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, r->server, "FTP: connect to 
%s:%d", desthost, destport);
 
-parms = strchr(path, ';');
+parms = strchr(url, ';');
 if (parms != NULL)
 *(parms++) = '\0';
 
 memset(&server, 0, sizeof(struct sockaddr_in));
 server.sin_family = AF_INET;
-server.sin_port = htons((unsigned short)port);
-err = ap_proxy_host2addr(host, &server_hp);
+server.sin_port = htons((unsigned short)destport);
+err = ap_proxy_host2addr(desthost, &server_hp);
 if (err != NULL)
 return ap_proxyerror(r, HTTP_INTERNAL_SERVER_ERROR, err);
 
@@ -1293,7 +1319,7 @@
 if (destaddr.s_addr == ncent[i].addr.s_addr ||
 (ncent[i].name != NULL &&
  (ncent[i].name[0] == '*' ||
-  strstr(host, ncent[i].name) != NULL))) {
+  strstr(desthost, ncent[i].name) != NULL))) {
 nocache = 1;
 break;
 }


RE: [PATCH] SSL: stop trying to talk to client after a negotiation failure?

2003-07-17 Thread Sander Striker
> From: Jeff Trawick [mailto:[EMAIL PROTECTED]
> Sent: Thursday, July 17, 2003 12:45 PM

[...]
> I will be off the net for 10 days starting this afternoon and have some 
> crucial stuff to do in the meantime (find passport, pack, etc.).  If I 
> haven't committed this stuff yet, go for it.  If I have, my apologies to 
> everyone if something breaks :)

heh heh.  Don't worry about it.  Have a good trip ;).


Sander


Re: [PATCH] SSL: stop trying to talk to client after a negotiationfailure?

2003-07-17 Thread Jeff Trawick
Joe Orton wrote:

* ssl_engine_io.c (ssl_filter_write, ssl_io_filter_output): Don't
dereference the BIOs in filter_ctx when filter_ctx->pssl is NULL.
Index: ssl_engine_io.c
===


+1 for your patch...  it is a cleaner patch than what the PR reporter 
provided in http://nagoya.apache.org/bugzilla/show_bug.cgi?id=21370

With sufficient caffeine, I see that the patch I posted to set 
c->aborted in a couple of places is different than the one the PR 
reporter had posted (I said it was his because I thought they were the 
same and I couldn't recall whether I came up with the idea before/after 
I saw his patch :) )

So my patch posted earlier in this thread is a second patch to commit...

There is still part of the PR reporter's patch that is unaccounted for:

Diff:
diff -c -r1.2 -r1.3
*** ssl_engine_io.c 2003/04/16 14:14:39 1.2
--- ssl_engine_io.c 2003/07/03 11:36:24 1.3
***
*** 1112,1117 
--- 1122,1129 
  inctx->rc = APR_EGENERAL;
  }
+   /* 2.7.2003/hk,mv: handshake failed, close the connection */
+   c->aborted=1;
  return ssl_filter_io_shutdown(filter_ctx, c, 1);
  }
***
*** 1153,1158 
--- 1165,1172 
   error ? error : "unknown");
  ssl_log_ssl_error(APLOG_MARK, APLOG_INFO, c->base_server);
+   /* 2.7.2003/hk,mv: no client cert, close the 
connection
*/
+   c->aborted=1;
  return ssl_filter_io_shutdown(filter_ctx, c, 1);
  }
  }

I didn't need this change in my testing, but it looks to me that it is 
proper for c->aborted to be set on this path, and that 
ssl_filter_io_shutdown() should do the setting instead of putting it 
here since any time ssl_filter_io_shutdown() is called it is appropriate 
for c->aborted to be set.

I will be off the net for 10 days starting this afternoon and have some 
crucial stuff to do in the meantime (find passport, pack, etc.).  If I 
haven't committed this stuff yet, go for it.  If I have, my apologies to 
everyone if something breaks :)




Re: [PATCH] SSL: stop trying to talk to client after a negotiation failure?

2003-07-17 Thread Joe Orton
On Wed, Jul 16, 2003 at 11:21:38AM -0400, Jeff Trawick wrote:
> I turned on SSLVerifyClient and my client wasn't set up properly and I 
> started getting server segfaults because we try to read on the 
> connection after some SSL-related data has been cleaned up.
> 
> #2  0x8091753 in ssl_io_filter_input (f=0x825d760, bb=0x825f440, 
> mode=AP_MODE_GETLINE, block=APR_BLOCK_READ,
> readbytes=0) at ssl_engine_io.c:1231
> 
> 1231if ((status = ssl_io_filter_connect(inctx->filter_ctx)) != 
> APR_SUCCESS) {
> 
> (gdb) p inctx->filter_ctx
> $1 = (ssl_filter_ctx_t *) 0x0
> 
> I haven't tried very hard to see how inctx->filter_ctx can get cleared, 
> but first I wonder about the big picture: Is continued I/O reasonabe 
> after the negotiation failure?  It *seems* like the clients I'm playing 
> with (wget, Mozilla) have given up at that point anyway.
>
> If it isn't reasonable, then c->aborted should be set.

I agree it's not reasonable; the SSL alert has been sent so nothing more
should be done.  That patch was necessary but not sufficient to cure the
segfaults for me in this case - I also needed the below change.  Do you
want to check these in together?

* ssl_engine_io.c (ssl_filter_write, ssl_io_filter_output): Don't
dereference the BIOs in filter_ctx when filter_ctx->pssl is NULL.

Index: ssl_engine_io.c
===
RCS file: /store/cvs/root/httpd-2.0/modules/ssl/ssl_engine_io.c,v
retrieving revision 1.109
diff -u -p -r1.109 ssl_engine_io.c
--- ssl_engine_io.c 22 May 2003 19:41:32 -  1.109
+++ ssl_engine_io.c 17 Jul 2003 09:54:03 -
@@ -780,8 +780,7 @@ static apr_status_t ssl_filter_write(ap_
  apr_size_t len)
 {
 ssl_filter_ctx_t *filter_ctx = f->ctx;
-bio_filter_out_ctx_t *outctx = 
-   (bio_filter_out_ctx_t *)(filter_ctx->pbioWrite->ptr);
+bio_filter_out_ctx_t *outctx;
 int res;
 
 /* write SSL */
@@ -789,6 +788,7 @@ static apr_status_t ssl_filter_write(ap_
 return APR_EGENERAL;
 }
 
+outctx = (bio_filter_out_ctx_t *)filter_ctx->pbioWrite->ptr;
 res = SSL_write(filter_ctx->pssl, (unsigned char *)data, len);
 
 if (res < 0) {
@@ -1362,8 +1362,7 @@ static apr_status_t ssl_io_filter_output
 {
 apr_status_t status = APR_SUCCESS;
 ssl_filter_ctx_t *filter_ctx = f->ctx;
-bio_filter_in_ctx_t *inctx = (bio_filter_in_ctx_t *)
- (filter_ctx->pbioRead->ptr);
+bio_filter_in_ctx_t *inctx;
 
 if (f->c->aborted) {
 apr_brigade_cleanup(bb);
@@ -1375,6 +1374,7 @@ static apr_status_t ssl_io_filter_output
 return ap_pass_brigade(f->next, bb);
 }
 
+inctx = (bio_filter_in_ctx_t *)filter_ctx->pbioRead->ptr;
 /* When we are the writer, we must initialize the inctx
  * mode so that we block for any required ssl input, because
  * output filtering is always nonblocking.