PATCH Prevent segfaults in connection state

Maintainers,

This patch addresses a still-outstanding flaw in mod_ssl, on *all* platforms.
However it's rarely evident on any platform other than Win32, because only Win32
recycles memory -so quickly- on other threads, that the cleanup cannot be
invoked.

Instead, in the LogRequest (request-is-done) hook is used to clean everything
up before r->pool goes poof.

Please, again consider this patch... I'm finished submitting through private
channels and would like the end-users to be able to take advantage of it already.

Thanks.

Bill


--- mod_ssl.h 25 Oct 2005 04:32:42 -0000 1.1
+++ mod_ssl.h 25 Oct 2005 05:54:19 -0000 1.2
@@ -711,6 +711,7 @@
int ssl_hook_Fixup(request_rec *);
int ssl_hook_ReadReq(request_rec *);
int ssl_hook_Handler(request_rec *);
+int ssl_hook_LogRequest(request_rec *r);

/* OpenSSL callbacks */
RSA *ssl_callback_TmpRSA(SSL *, int, int);
--- mod_ssl.c 25 Oct 2005 04:32:46 -0000 1.1
+++ mod_ssl.c 25 Oct 2005 05:52:20 -0000 1.2
@@ -231,7 +231,7 @@
ssl_hook_Access, /* [#3] check access by host address */
NULL, /* [#6] determine MIME type */
ssl_hook_Fixup, /* [#7] pre-run fixups */
- NULL, /* [#9] log a transaction */
+ ssl_hook_LogRequest, /* [#9] log a transaction */
NULL, /* [#2] header parser */
ssl_init_Child, /* child_init */
NULL, /* child_exit */
--- ssl_engine_io.c 25 Oct 2005 04:32:28 -0000 1.1
+++ ssl_engine_io.c 25 Oct 2005 05:52:20 -0000 1.2
@@ -263,7 +263,7 @@
r = (request_rec *)ap_ctx_get(actx, "ssl::request_rec");

rv = -1;
- if (r != NULL) {
+ if (r != NULL && r->ctx != NULL) {
ss = ap_ctx_get(r->ctx, "ssl::io::suck");
if (ss != NULL) {
if (ss->active && ss->pendlen > 0) {
--- ssl_engine_kernel.c 25 Oct 2005 04:32:41 -0000 1.1
+++ ssl_engine_kernel.c 25 Oct 2005 05:52:20 -0000 1.2
@@ -542,6 +542,28 @@
}

/*
+ * Logging Handler, last chance at request_rec
+ */
+int ssl_hook_LogRequest(request_rec *r)
+{
+ SSL *ssl;
+ ap_ctx *apctx;
+
+ /* Mitigate potential damage of any invalid ssl::request_rec
+ * by clearing this datum prior to child_sub_main destroying
+ * our r->pool (and within in, our request_rec!!!)
+ */
+ ssl = ap_ctx_get(r->connection->client->ctx, "ssl");
+ if (ssl != NULL) {
+ apctx = SSL_get_app_data2(ssl);
+ if (apctx && ap_ctx_get(apctx, "ssl::request_rec")) {
+ ap_ctx_set(apctx, "ssl::request_rec", NULL);
+ }
+ }
+ return OK;
+}
+
+/*
* Post Read Request Handler
*/
int ssl_hook_ReadReq(request_rec *r)



____________________________________________________________ __________
Apache Interface to OpenSSL (mod_ssl) www.modssl.org
User Support Mailing List modssl-users@modssl.org
Automated List Manager majordomo@modssl.org

Re: PATCH Prevent segfaults in connection state

Ping, in anticipation of a 2.8.26 for 1.3.35 (are we expecting one?)

I didn't see any feedback, do any of the Apache guru's have comments? Any
clarifications? (I hope I made the specifics clear enough, below.)

Yours,

Bill


William A. Rowe, Jr. wrote:
> Maintainers,
>
> This patch addresses a still-outstanding flaw in mod_ssl, on *all* platforms.
> However it's rarely evident on any platform other than Win32, because only Win32
> recycles memory -so quickly- on other threads, that the cleanup cannot be
> invoked.
>
> Instead, in the LogRequest (request-is-done) hook is used to clean everything
> up before r->pool goes poof.
>
> Please, again consider this patch... I'm finished submitting through private
> channels and would like the end-users to be able to take advantage of it
> already.
>
> Thanks.
>
> Bill
>
>
> --- mod_ssl.h 25 Oct 2005 04:32:42 -0000 1.1
> +++ mod_ssl.h 25 Oct 2005 05:54:19 -0000 1.2
> @@ -711,6 +711,7 @@
> int ssl_hook_Fixup(request_rec *);
> int ssl_hook_ReadReq(request_rec *);
> int ssl_hook_Handler(request_rec *);
> +int ssl_hook_LogRequest(request_rec *r);
>
> /* OpenSSL callbacks */
> RSA *ssl_callback_TmpRSA(SSL *, int, int);
> --- mod_ssl.c 25 Oct 2005 04:32:46 -0000 1.1
> +++ mod_ssl.c 25 Oct 2005 05:52:20 -0000 1.2
> @@ -231,7 +231,7 @@
> ssl_hook_Access, /* [#3] check access by host address */
> NULL, /* [#6] determine MIME type */
> ssl_hook_Fixup, /* [#7] pre-run fixups */
> - NULL, /* [#9] log a transaction */
> + ssl_hook_LogRequest, /* [#9] log a transaction */
> NULL, /* [#2] header parser */
> ssl_init_Child, /* child_init */
> NULL, /* child_exit */
> --- ssl_engine_io.c 25 Oct 2005 04:32:28 -0000 1.1
> +++ ssl_engine_io.c 25 Oct 2005 05:52:20 -0000 1.2
> @@ -263,7 +263,7 @@
> r = (request_rec *)ap_ctx_get(actx, "ssl::request_rec");
>
> rv = -1;
> - if (r != NULL) {
> + if (r != NULL && r->ctx != NULL) {
> ss = ap_ctx_get(r->ctx, "ssl::io::suck");
> if (ss != NULL) {
> if (ss->active && ss->pendlen > 0) {
> --- ssl_engine_kernel.c 25 Oct 2005 04:32:41 -0000 1.1
> +++ ssl_engine_kernel.c 25 Oct 2005 05:52:20 -0000 1.2
> @@ -542,6 +542,28 @@
> }
>
> /*
> + * Logging Handler, last chance at request_rec
> + */
> +int ssl_hook_LogRequest(request_rec *r)
> +{
> + SSL *ssl;
> + ap_ctx *apctx;
> +
> + /* Mitigate potential damage of any invalid ssl::request_rec
> + * by clearing this datum prior to child_sub_main destroying
> + * our r->pool (and within in, our request_rec!!!)
> + */
> + ssl = ap_ctx_get(r->connection->client->ctx, "ssl");
> + if (ssl != NULL) {
> + apctx = SSL_get_app_data2(ssl);
> + if (apctx && ap_ctx_get(apctx, "ssl::request_rec")) {
> + ap_ctx_set(apctx, "ssl::request_rec", NULL);
> + }
> + }
> + return OK;
> +}
> +
> +/*
> * Post Read Request Handler
> */
> int ssl_hook_ReadReq(request_rec *r)
>
>
>
> ____________________________________________________________ __________
> Apache Interface to OpenSSL (mod_ssl) www.modssl.org
> User Support Mailing List modssl-users@modssl.org
> Automated List Manager majordomo@modssl.org
>
>
____________________________________________________________ __________
Apache Interface to OpenSSL (mod_ssl) www.modssl.org
User Support Mailing List modssl-users@modssl.org
Automated List Manager majordomo@modssl.org

Re: PATCH Prevent segfaults in connection state

On Fri, May 05, 2006, William A. Rowe, Jr. wrote:

> Ping, in anticipation of a 2.8.26 for 1.3.35 (are we expecting one?)
>
> I didn't see any feedback, do any of the Apache guru's have comments? Any
> clarifications? (I hope I made the specifics clear enough, below.)

Hmmm... the cleanup is done in the ssl_hook_CloseConnection()
function which comes after your ssl_hook_LogRequest() anyway. Do I
understand correctly: under Win32 the r->pool is cleaned up _before_
ssl_hook_CloseConnection() is called?

Ralf S. Engelschall
rse@engelschall.com
www.engelschall.com

____________________________________________________________ __________
Apache Interface to OpenSSL (mod_ssl) www.modssl.org
User Support Mailing List modssl-users@modssl.org
Automated List Manager majordomo@modssl.org

Re: PATCH Prevent segfaults in connection state

Ralf S. Engelschall wrote:
> On Fri, May 05, 2006, William A. Rowe, Jr. wrote:
>
>>Ping, in anticipation of a 2.8.26 for 1.3.35 (are we expecting one?)
>>
>>I didn't see any feedback, do any of the Apache guru's have comments? Any
>>clarifications? (I hope I made the specifics clear enough, below.)
>
> Hmmm... the cleanup is done in the ssl_hook_CloseConnection()
> function which comes after your ssl_hook_LogRequest() anyway. Do I
> understand correctly: under Win32 the r->pool is cleaned up _before_
> ssl_hook_CloseConnection() is called?

Yes, especially, with keep alives. Think about it, requests disappear while
the connection remains.

All in all the old code was evil, and equally faulty on unix. Only on unix,
we never free the pool mem (just hold it for recycling) and there's no second
thread to come along and appropriate it.

Bill
____________________________________________________________ __________
Apache Interface to OpenSSL (mod_ssl) www.modssl.org
User Support Mailing List modssl-users@modssl.org
Automated List Manager majordomo@modssl.org

Re: PATCH Prevent segfaults in connection state

--0-1123420114-1147090076=:12545
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: quoted-printable

D i have to report you as spam or what please remove! THANKYOU

"William A. Rowe, Jr." wrote: Ralf S. Engelschall =
wrote:
> On Fri, May 05, 2006, William A. Rowe, Jr. wrote:
>=20
>>Ping, in anticipation of a 2.8.26 for 1.3.35 (are we expecting one?)
>>
>>I didn't see any feedback, do any of the Apache guru's have comments? A=
ny
>>clarifications? (I hope I made the specifics clear enough, below.)
>=20
> Hmmm... the cleanup is done in the ssl_hook_CloseConnection()
> function which comes after your ssl_hook_LogRequest() anyway. Do I
> understand correctly: under Win32 the r->pool is cleaned up _before_
> ssl_hook_CloseConnection() is called?

Yes, especially, with keep alives. Think about it, requests disappear whi=
le
the connection remains.

All in all the old code was evil, and equally faulty on unix. Only on uni=
x,
we never free the pool mem (just hold it for recycling) and there's no se=
cond
thread to come along and appropriate it.

Bill
____________________________________________________________ __________
Apache Interface to OpenSSL (mod_ssl) www.modssl.org
User Support Mailing List modssl-users@modssl.org
Automated List Manager majordomo@modssl.org


=09
---------------------------------
How low will we go? Check out Yahoo! Messenger=92s low PC-to-Phone call =
rates.
--0-1123420114-1147090076=:12545
Content-Type: text/html; charset=iso-8859-1
Content-Transfer-Encoding: quoted-printable

D i have to report you as spam or what please remove! THANKYOU

=
"William A. Rowe, Jr." <wrowe@rowe-clan.net> wrote: OCKQUOTE class=3Dreplbq style=3D"PADDING-LEFT: 5px; MARGIN-LEFT: 5px; BOR=
DER-LEFT: #1010ff 2px solid">Ralf S. Engelschall wrote:
> On Fri, M=
ay 05, 2006, William A. Rowe, Jr. wrote:
>
>>Ping, in ant=
icipation of a 2.8.26 for 1.3.35 (are we expecting one?)
>>
&=
gt;>I didn't see any feedback, do any of the Apache guru's have commen=
ts? Any
>>clarifications? (I hope I made the specifics clear eno=
ugh, below.)
>
> Hmmm... the cleanup is done in the ssl_hook=
_CloseConnection()
> function which comes after your ssl_hook_LogRe=
quest() anyway. Do I
> understand correctly: under Win32 the r->=
pool is cleaned up _before_
> ssl_hook_CloseConnection() is called?=


Yes, especially, with keep alives. Think about it, requests disap=
pear while
the connection
remains.

All in all the old code was evil, and equally faulty on =
unix. Only on unix,
we never free the pool mem (just hold it for recyc=
ling) and there's no second
thread to come along and appropriate it. R>
Bill
___________________________________________________________=
___________
Apache Interface to OpenSSL (mod_ssl) www.modssl.org
Us=
er Support Mailing List modssl-users@modssl.org
Automated List Manager=
majordomo@modssl.org