Diff for flood_net_ssl.c
To Apache Flood development team:
As part of our work on the Flood code (in our usage of it as par of an
enterprise product), I have been instructed to contribute back our changes
on a daily basis so we can both contribute them back to the product and
get back inportant feedback on these changes.
Other than some small touch-ups, these changes to flood_net_ssl.c involve
1) taking recursive function code out of socket read/write functions,
(replacing with do .. while loop) resulting in
a) more robust code, with less possibility of stack-related issues.
b) errors returned in recursively-called code were not being propagated.
c) with iterative code it is easier to set limits on the amount of
iteration, if necessary. I have not changed the logic here (yet).
and
2) the addition of certain cases which should cause a continuation of
reading.
The -u3 diff is taken from the current directory holding the modified file
and the \flood-1.1 directory holding the original from Apache project, and
is attached here as fns.diff since as inline the mailer would modify the
text.
-Norman Tuttle, OpenDemand Systems Developer [EMAIL PROTECTED]
--- flood_net_ssl.c 2003-10-13 09:30:48.0 -0400
+++ \flood-1.1\flood_net_ssl.c 2003-10-08 19:25:02.0 -0400
@@ -84,8 +84,8 @@
#if APR_HAS_THREADS
apr_thread_mutex_t **ssl_locks;
-typedef struct CRYPTO_dynlock_value {
-apr_thread_mutex_t *lock;
+typedef struct CRYPTO_dynlock_value {
+apr_thread_mutex_t *lock;
} CRYPTO_dynlock_value;
static CRYPTO_dynlock_value *ssl_dyn_create(const char* file, int line)
@@ -101,7 +101,7 @@
return l;
}
-static void ssl_dyn_lock(int mode, CRYPTO_dynlock_value *l, const char *file,
+static void ssl_dyn_lock(int mode, CRYPTO_dynlock_value *l, const char *file,
int line)
{
if (mode CRYPTO_LOCK) {
@@ -131,7 +131,7 @@
static unsigned long ssl_id(void)
{
/* FIXME: This is lame and not portable. -aaron */
-return (unsigned long) apr_os_thread_current();
+return (unsigned long) apr_os_thread_current();
}
#endif
@@ -213,7 +213,7 @@
void ssl_read_socket_handshake(ssl_socket_t *s);
ssl_socket_t* ssl_open_socket(apr_pool_t *pool, request_t *r,
- apr_status_t *status)
+ apr_status_t *status)
{
apr_os_sock_t ossock;
int e, sslError;
@@ -222,7 +222,7 @@
/* Open our TCP-based connection */
ssl_socket-socket = open_socket(pool, r, status);
-
+
if (!ssl_socket-socket)
return NULL;
@@ -260,7 +260,7 @@
break;
default:
ERR_print_errors_fp(stderr);
-return NULL;
+return NULL;
}
}
@@ -281,52 +281,45 @@
int sslError;
apr_int32_t socketsRead;
-do
-{
-
- /* Wait until there is something to read. */
- if (SSL_pending(s-ssl_connection) *buflen) {
+/* Wait until there is something to read. */
+if (SSL_pending(s-ssl_connection) *buflen) {
e = apr_poll(s-socket-read_pollset, 1, socketsRead,
LOCAL_SOCKET_TIMEOUT);
if (socketsRead != 1)
return APR_TIMEUP;
- }
+}
- e = SSL_read(s-ssl_connection, buf, *buflen);
- sslError = SSL_get_error(s-ssl_connection, e);
- if (sslError!=SSL_ERROR_WANT_READ) *buflen = 0;
+e = SSL_read(s-ssl_connection, buf, *buflen);
+sslError = SSL_get_error(s-ssl_connection, e);
- switch (sslError)
- {
-case SSL_ERROR_NONE:
- *buflen = e;
- break;
-case SSL_ERROR_WANT_READ:
-case SSL_ERROR_WANT_WRITE:
-case SSL_ERROR_WANT_X509_LOOKUP:
-// ssl_read_socket(s, buf, buflen); // Now uses do-while loop to do
this properly!
- break;
-case SSL_ERROR_ZERO_RETURN: /* Peer closed connection. */
- return APR_EOF;
-case SSL_ERROR_SYSCALL: /* Look at errno. */
- if (errno == 0)
+switch (sslError)
+{
+case SSL_ERROR_NONE:
+*buflen = e;
+break;
+case SSL_ERROR_WANT_READ:
+ssl_read_socket(s, buf, buflen);
+break;
+case SSL_ERROR_ZERO_RETURN: /* Peer closed connection. */
+return APR_EOF;
+case SSL_ERROR_SYSCALL: /* Look at errno. */
+if (errno == 0)
return APR_EOF;
- /* Continue through with the error case. */
-//case SSL_ERROR_WANT_WRITE: /* Technically, not an error. */ // (this is
also in default) // Moved up now!
-default:
- ERR_print_errors_fp(stderr);
- return APR_EGENERAL;
- }
+/* Continue through with the error case. */
+case SSL_ERROR_WANT_WRITE: /* Technically, not an error. */
+default:
+ERR_print_errors_fp(stderr);
+return APR_EGENERAL;
}
-while (sslError != SSL_ERROR_NONE); // to get out of this loop, need no
errors!
+
return APR_SUCCESS;
}
void
[1.3 PATCHLET] fix wording of warning for ErrorDocument 401 full-URL
2.0 already has this fixed
Index: http_core.c
===
RCS file: /home/cvs/apache-1.3/src/main/http_core.c,v
retrieving revision 1.325
diff -u -r1.325 http_core.c
--- http_core.c 7 Jul 2003 13:02:28 - 1.325
+++ http_core.c 14 Oct 2003 12:07:53 -
@@ -1276,7 +1276,7 @@
if (error_number == 401
line[0] != '/' line[0] != '') { /* Ignore it... */
ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, cmd-server,
-cannot use a full or relative URL in a 401 ErrorDocument
+cannot use a full URL in a 401 ErrorDocument
directive --- ignoring!);
}
else { /* Store it... */
Re: the scoreboard doesn't maintain start/stop times
Stas Bekman wrote: Also we need to have the vhost info in the score, otherwise request URI info is not quite useful on the system with several vhosts, since you can't tell which vhost it was invoked from. hmmm? AFAIK vhost is working fine - see http://apache.org/server-status Greg
malformed header causes segfault
[Tue Oct 14 08:24:57 2003] [error] [client 213.243.186.233] Client sent malformed Host header [Tue Oct 14 08:24:57 2003] [notice] child pid 29013 exit signal Segmentation fault (11) Every time I get a malformed header, I get a segfault. Any ideas? I have UseCannonicalName On, and I alwasy use ap_get_server_name and ap_get_server_port in my mods. TIA. -- Brian Akins [EMAIL PROTECTED] CNN Internet Technologies
Re: malformed header causes segfault
Brian Akins wrote: [Tue Oct 14 08:24:57 2003] [error] [client 213.243.186.233] Client sent malformed Host header [Tue Oct 14 08:24:57 2003] [notice] child pid 29013 exit signal Segmentation fault (11) Every time I get a malformed header, I get a segfault. Any ideas? next step is to get backtrace from coredump... do you know how to enable core dumps and get backtraces? (if not, what OS is this and what version of the web server?)
Re: malformed header causes segfault
On Tue, 2003-10-14 at 09:56, Jeff Trawick wrote: next step is to get backtrace from coredump... do you know how to enable core dumps and get backtraces? (if not, what OS is this and what version of the web server?) Yes. But I can never get them when I let httpd change id (ie, from root to httpd). I set CoreDumpDirectory to /tmp, but no dumps. If I run everything as myself (without port 80), I can get dumps. This is httpd 2.0.47, RedHat AS 2.1. -- Brian Akins [EMAIL PROTECTED] CNN Internet Technologies
Re: malformed header causes segfault
Brian Akins wrote: On Tue, 2003-10-14 at 09:56, Jeff Trawick wrote: next step is to get backtrace from coredump... do you know how to enable core dumps and get backtraces? (if not, what OS is this and what version of the web server?) Yes. But I can never get them when I let httpd change id (ie, from root to httpd). I set CoreDumpDirectory to /tmp, but no dumps. If I run everything as myself (without port 80), I can get dumps. This is httpd 2.0.47, RedHat AS 2.1. Check avail disk space in tmp, check ulimits in the shell used to start the server. Linux = 2.4 requires a special prctl() invocation in order to get core dumps from programs that switch identity, but Apache2.0.47 should be taking care of that for you as long as it is built on RHAS 2.1. To verify, find the call to prctl() in os/unix/unixd.c, stick #error gobble on the line right before it, type make, and verify that compilation broke. (i.e., verify that Apache recognized that prctl() was possible and necessary on your system) for ulimits, the core dump size is the normal problem # ulimit -c unlimited # apachectl start # ulimit -c (whatever you had before)
Re: malformed header causes segfault
Thanks. How can I simulated a malformed host header? I set all kinds of garbage, but it never complains. -- Brian Akins [EMAIL PROTECTED] CNN Internet Technologies
Re: malformed header causes segfault
On Tue, 2003-10-14 at 10:28, Brian Akins wrote: Thanks. How can I simulate a malformed host header? I set all kinds of garbage, but it never complains. I figure out if I send a / in the Host header this will be triggered. It seems to be in mod_include can anyone else reproduce this? -- Brian Akins [EMAIL PROTECTED] CNN Internet Technologies
Re: malformed header causes segfault
On Tue, 2003-10-14 at 12:16, Brian Akins wrote: On Tue, 2003-10-14 at 10:28, Brian Akins wrote: Thanks. How can I simulate a malformed host header? I set all kinds of garbage, but it never complains. I figure out if I send a / in the Host header this will be triggered. It seems to be in mod_include can anyone else reproduce this? Someone else confirmed on a couple of installs. This seems to only happen when AddOutputFilterByType INCLUDES text/html ... is used. -- Brian Akins [EMAIL PROTECTED] CNN Internet Technologies
Re: malformed header causes segfault
On Tue, 2003-10-14 at 13:39, Brian Akins wrote:
I figure out if I send a / in the Host header this will be triggered.
It seems to be in mod_include can anyone else reproduce this?
Someone else confirmed on a couple of installs. This seems to only
happen when AddOutputFilterByType INCLUDES text/html ... is used.
Quick fix:
Change
if (!(ap_allow_options(r)) {
to
if (!(ap_allow_options(r) OPT_INCLUDES) || (r-status ==
HTTP_BAD_REQUEST)) {
in includes_filter
See attached diff.
--
Brian Akins [EMAIL PROTECTED]
CNN Internet Technologies
--- mod_include.c.orig Tue Oct 14 13:54:43 2003
+++ mod_include.c Tue Oct 14 13:44:23 2003
@@ -3353,7 +3353,7 @@
include_server_config *sconf= ap_get_module_config(r-server-module_config,
include_module);
-if (!(ap_allow_options(r) OPT_INCLUDES)) {
+if (!(ap_allow_options(r) OPT_INCLUDES) || (r-status == HTTP_BAD_REQUEST)) {
return ap_pass_brigade(f-next, b);
}
Apache 1.3.28 crash when use proxy config together with SSL
WinXP, Apache 1.3.28 crashed when browse I configure the proxy inside the SSL VirtualHost:443 config directive. Similar crash happen when I include proxy+mod_gzip or mod_php inside SSL config too. The crash seems to happen regularly with framed pages. Is this know issue? Am I required to recompile all the module with -EAPI define? I saw some post mention that -EAPI is not needed. -- Browse, Analyze and document complex project source code such as PHP, Apache, MySQL on http://www.slink-software.com
isn't ap_die() broken with recognizing recursive errors and/or should my module leave r-status alone?
1.3 ap_die() is simpler, so use that as an example when referring to code.
case 1:
ErrorDocument 413 /index.html
module returns 413 from handler and doesn't set r-status to 413
GOOD: /index.html is used for the error document
case 2:
ErrorDocument 413 /index.html
module returns 413 from handler and does set r-status to 413
BAD: this is what client sees:
!DOCTYPE HTML PUBLIC -//IETF//DTD HTML 2.0//EN
HTMLHEAD
TITLE413 Request Entity Too Large/TITLE
/HEADBODY
H1Request Entity Too Large/H1
The requested resourceBR/sillyBR
does not allow request data with GET requests, or the amount of data
provided in
the request exceeds the capacity limit.
PAdditionally, a 413 Request Entity Too Large
error was encountered while trying to use an ErrorDocument to handle the
request.
HR
ADDRESSApache/1.3.29-dev Server at cs390-1.raleigh.ibm.com Port
80/ADDRESS
/BODY/HTML
ap_die() is definitely confused about whether or not it is a recursive
error. Any time r-status is set, it thinks it is a recursive error.
Attached is a patch to 1.3's ap_die() to make it smarter about
recognizing recursive errors. I suspect that some funny redirect
optimizations in 2.0 would make the same fix impossible, but I haven't
checked.
On a side note, is it bad for modules to set r-status? Is that why
this recursion check, present since 1.2, is not causing grief for others?
Index: src/main/http_request.c
===
RCS file: /home/cvs/apache-1.3/src/main/http_request.c,v
retrieving revision 1.170
diff -u -r1.170 http_request.c
--- src/main/http_request.c 7 Jul 2003 00:34:10 - 1.170
+++ src/main/http_request.c 14 Oct 2003 20:01:42 -
@@ -1073,13 +1073,16 @@
*/
if (r-status != HTTP_OK) {
-recursive_error = type;
-while (r-prev (r-prev-status != HTTP_OK))
+while (r-prev (r-prev-status != HTTP_OK)) {
+recursive_error = type;
r = r-prev;/* Get back to original error */
+}
-type = r-status;
-custom_response = NULL; /* Do NOT retry the custom thing! */
+if (recursive_error) {
+type = r-status;
+custom_response = NULL; /* Do NOT retry the custom thing! */
+}
}
r-status = type;
Re: the scoreboard doesn't maintain start/stop times
[EMAIL PROTECTED] wrote: Stas Bekman wrote: Also we need to have the vhost info in the score, otherwise request URI info is not quite useful on the system with several vhosts, since you can't tell which vhost it was invoked from. hmmm? AFAIK vhost is working fine - see http://apache.org/server-status hmmm, it's indeed there, for some reason I thought that I still had an outstanding patch to add this feature, posted some 2 years ago. Great news, Greg. __ Stas BekmanJAm_pH -- Just Another mod_perl Hacker http://stason.org/ mod_perl Guide --- http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
Re: isn't ap_die() broken with recognizing recursive errors and/or should my module leave r-status alone?
Geoffrey Young wrote: ap_die() is definitely confused about whether or not it is a recursive error. Any time r-status is set, it thinks it is a recursive error. On a side note, is it bad for modules to set r-status? it was because of the former that I thought it was bad to do the latter. and I was wondering if more people didn't run into this because there were other reasons why modules shouldn't set r-status :) over in mod_perl-land there is the propensity for people to play with $r-status in handlers (due to Apache::Registry using it as a hack). I always tell them that handlers (Perl or C) communicate their status via their return code, not $r-status - doing so will muck with the error document cycle, and no core module does it, so don't do it. the confusion about whether the error is recursive can definitely be fixed... it turns out that my 413 example may not be the best one, as there are additional issues when core sets 413 based on LimitRequestBody... see http://nagoya.apache.org/bugzilla/show_bug.cgi?id=21035 it would be nice to hear somebody verify that here :) definitely!
