You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Daniel Ruggeri <DR...@primary.net> on 2010/11/25 06:03:33 UTC
Making mod_proxy_http more aware of SSL
All;
I opened up bug 50332 to attach/document these patches. The patch
causes mod_ssl to create a note on the conn_req which is checked by
mod_proxy_http when it attempts to pass the request. The intent is for
mod_proxy_http to realize that an SSL handshake error has occurred and
mark the worker out of service.
This is a huge step forward in that mod_proxy will not be oblivious
to the failed SSL connection and can take a worker out of service,
however... it's not all roses. I don't know what it would take (or if
it's even possible since mod_ssl and mod_proxy run in very separate
filters), but it would be really great if mod_proxy in general were
aware of handshake failures before it ever attempts to submit a request
to the backend. I would envision this enlightenment to come at "/* Step
Two: Make the Connection */" in modules/proxy/mod_proxy_http.c.
Thoughts?
If the great minds of this mail list deem these patches acceptable, here
is the proposed patch to 2.2 STATUS:
Index: httpd-2.2.x/STATUS
===================================================================
--- httpd-2.2.x/STATUS (revision 1037345)
+++ httpd-2.2.x/STATUS (working copy)
@@ -184,6 +184,14 @@
enabling/disabling the basic capability is not split out into
mod_unixd 2.2.x.
+1: trawick
+ * mod_proxy_http: Become aware of ssl handshake failures when attempting
+ to pass request. Makes it so workers are put in error state when a
+ handshake failure is encountered.
+ PR50332
+ Trunk patch:
https://issues.apache.org/bugzilla/attachment.cgi?id=26339
+ 2.2.x patch:
https://issues.apache.org/bugzilla/attachment.cgi?id=26338
+ druggeri: Need doc update?
+
PATCHES/ISSUES THAT ARE STALLED
* core: Support wildcards in both the directory and file components of
A tag in CHANGES would be appreciated:
*) Proxy: Detect SSL handshake failures during proxy pass attempts
and place backend in error state. PR 50332. [Daniel Ruggeri <DRuggeri
primary.net>]
--
--
Daniel Ruggeri
Re: Making mod_proxy_http more aware of SSL
Posted by Daniel Ruggeri <DR...@primary.net>.
> Hence you should return a 500 as this signals the mod_proxy code that the
> backend is broken and should be put in error state.
> A 502 does not put the backend in error state (as you found out).
>
> Regards
>
> Rüdiger
>
I didn't realize this was the case - marking it in error inside
ap_proxy_http_process_response would definitely be redundant! Thank you
very much for catching it (and explaining this to me). I have updated
the patches and bug report and attached the updates for reference.
--
Daniel Ruggeri
RE: Making mod_proxy_http more aware of SSL
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Daniel Ruggeri
> Sent: Donnerstag, 25. November 2010 16:20
> To: dev@httpd.apache.org
> Subject: Re: Making mod_proxy_http more aware of SSL
>
> >
> > The loggers get in error state automatically when you call
> > ap_proxyerror with HTTP_INTERNAL_SERVER_ERROR. No need to
> do it manually.
> >
> > Regards
> >
> > Rüdiger
> >
>
> The request goes into error state - that has never been the
> problem (502
> is correct here). The real issue is that the proxy worker that served
> the request does not. In a load balancing situation this
> means that the
> backend with broken SSL will stay in service and every other request
> will send a 502 back to the user (for a 2 node round-robin
> type of load
> balancing). If I'm not explaining it well enough, have a look
> at PR50332
> for a quick overview to duplicate the issue.
>
> With this patch, the backend worker is put into error state and is
> subject to normal recovery attempts. In other words, one
> request fails
> and a 502 is returned but subsequent requests are correctly
> sent to the
> other backend until the error interval has expired and the
> first backend
> is retried. IMO, SSL handshake failures should be detected during
> connection so we could attempt another backend but I am not
> sure that's
> possible.
Hence you should return a 500 as this signals the mod_proxy code that the
backend is broken and should be put in error state.
A 502 does not put the backend in error state (as you found out).
Regards
Rüdiger
Re: Making mod_proxy_http more aware of SSL
Posted by Daniel Ruggeri <DR...@primary.net>.
>
> The loggers get in error state automatically when you call
> ap_proxyerror with HTTP_INTERNAL_SERVER_ERROR. No need to do it manually.
>
> Regards
>
> Rüdiger
>
The request goes into error state - that has never been the problem (502
is correct here). The real issue is that the proxy worker that served
the request does not. In a load balancing situation this means that the
backend with broken SSL will stay in service and every other request
will send a 502 back to the user (for a 2 node round-robin type of load
balancing). If I'm not explaining it well enough, have a look at PR50332
for a quick overview to duplicate the issue.
With this patch, the backend worker is put into error state and is
subject to normal recovery attempts. In other words, one request fails
and a 502 is returned but subsequent requests are correctly sent to the
other backend until the error interval has expired and the first backend
is retried. IMO, SSL handshake failures should be detected during
connection so we could attempt another backend but I am not sure that's
possible.
--
Daniel Ruggeri
RE: Making mod_proxy_http more aware of SSL
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Daniel Ruggeri
> Sent: Donnerstag, 25. November 2010 16:01
> To: dev@httpd.apache.org
> Subject: Re: Making mod_proxy_http more aware of SSL
>
>
> On 11/25/2010 4:14 AM, "Plüm, Rüdiger, VF-Group" wrote:
> > the following seems better:
> >
> >
> > + else
> if(strcmp(apr_table_get(backend->connection->notes,
> "SSL_connect_rv"), "err") == 0) {
> > + return ap_proxyerror(r,
> HTTP_INTERNAL_SERVER_ERROR,
> > + "Error during SSL
> Handshake with remote server");
> > +
> >
> >
> > Regards
> >
> > Rüdiger
>
> I agree that the message should be logged as such since
> logging higher
> than INFO would hide the actual SSL error from mod_ssl. My
> focus though
> is on marking the backend server out of service as you can't
> communicate
> unless the SSL transport has been established (essentially a
> failure to
> connect). Nothing else in the request/response cycle in
> mod_proxy_http
> does this due to handshake errors and this spot seems to be the very
> first place we can actually check for that condition.
>
> I have updated the patches to log your suggested message
> after marking
> the workers to be in error state.
The loggers get in error state automatically when you call
ap_proxyerror with HTTP_INTERNAL_SERVER_ERROR. No need to do it manually.
Regards
Rüdiger
Re: Making mod_proxy_http more aware of SSL
Posted by Daniel Ruggeri <DR...@primary.net>.
On 11/25/2010 4:14 AM, "Plüm, Rüdiger, VF-Group" wrote:
> the following seems better:
>
>
> + else if(strcmp(apr_table_get(backend->connection->notes, "SSL_connect_rv"), "err") == 0) {
> + return ap_proxyerror(r, HTTP_INTERNAL_SERVER_ERROR,
> + "Error during SSL Handshake with remote server");
> +
>
>
> Regards
>
> Rüdiger
I agree that the message should be logged as such since logging higher
than INFO would hide the actual SSL error from mod_ssl. My focus though
is on marking the backend server out of service as you can't communicate
unless the SSL transport has been established (essentially a failure to
connect). Nothing else in the request/response cycle in mod_proxy_http
does this due to handshake errors and this spot seems to be the very
first place we can actually check for that condition.
I have updated the patches to log your suggested message after marking
the workers to be in error state.
--
Daniel Ruggeri
RE: Making mod_proxy_http more aware of SSL
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Daniel Ruggeri [mailto:DRuggeri@primary.net]
> Sent: Donnerstag, 25. November 2010 06:04
> To: dev@httpd.apache.org
> Subject: Making mod_proxy_http more aware of SSL
>
> All;
> I opened up bug 50332 to attach/document these patches. The patch
> causes mod_ssl to create a note on the conn_req which is checked by
> mod_proxy_http when it attempts to pass the request. The
> intent is for
> mod_proxy_http to realize that an SSL handshake error has
> occurred and
> mark the worker out of service.
I guess the part in mod_proxy_http.c is not the correct way to do it.
Instead of
===================================================================
--- httpd-trunk/modules/proxy/mod_proxy_http.c (revision 1037345)
+++ httpd-trunk/modules/proxy/mod_proxy_http.c (working copy)
@@ -1468,6 +1468,10 @@
return ap_proxyerror(r, HTTP_SERVICE_UNAVAILABLE, "Timeout on 100-Continue");
}
}
+ else if(strcmp(apr_table_get(backend->connection->notes, "SSL_connect_rv"), "err") == 0) {
+ backend->worker->s->status |= PROXY_WORKER_IN_ERROR;
+ backend->worker->s->error_time = apr_time_now();
+ }
/*
* If we are a reverse proxy request shutdown the connection
* WITHOUT ANY response to trigger a retry by the client
the following seems better:
+ else if(strcmp(apr_table_get(backend->connection->notes, "SSL_connect_rv"), "err") == 0) {
+ return ap_proxyerror(r, HTTP_INTERNAL_SERVER_ERROR,
+ "Error during SSL Handshake with remote server");
+
Regards
Rüdiger