You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2016/02/11 15:10:10 UTC
Re: svn commit: r1729826 - in /httpd/httpd/trunk: CHANGES
modules/proxy/proxy_util.c
On 02/11/2016 02:46 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Thu Feb 11 13:46:39 2016
> New Revision: 1729826
>
> URL: http://svn.apache.org/viewvc?rev=1729826&view=rev
> Log:
> mod_proxy: Play/restore the TLS-SNI on new backend connections which
> had to be issued because the remote closed the previous/reusable one
> during idle (keep-alive) time.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1729826&r1=1729825&r2=1729826&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Thu Feb 11 13:46:39 2016
> @@ -1,6 +1,10 @@
> -*- coding: utf-8 -*-
> Changes with Apache 2.5.0
>
> + *) mod_proxy: Play/restore the TLS-SNI on new backend connections which
> + had to be issued because the remote closed the previous/reusable one
> + during idle (keep-alive) time. [Yann Ylavic]
> +
> *) mod_proxy_http2: new experimental http2 proxy module for h2: and h2c: proxy
> urls. Uses, so far, one connection per request, reuses connections.
>
>
> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1729826&r1=1729825&r2=1729826&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Feb 11 13:46:39 2016
> @@ -2717,12 +2717,18 @@ PROXY_DECLARE(int) ap_proxy_connect_back
>
> if (conn->sock) {
> if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
> - /* FIXME: this loses conn->ssl_hostname and it will not be
> - * restablished before the SSL connection is made -> no SNI! */
> + /* This clears conn->scpool (and associated data), so backup and
> + * restore any ssl_hostname for this connection set earlier by
> + * ap_proxy_determine_connection().
> + */
> + const char *ssl_hostname = conn->ssl_hostname;
conn->ssl_hostname might be allocated from conn->scpool.
So the pointer might be invalid after socket_cleanup(conn). So you need to apr_pstrdup it first.
No real good idea which pool to use here. Probably create a temp subpool of conn->pool,
strdup, after socket_cleanup dup with conn->scpool and destroy tmp pool.
> +
> socket_cleanup(conn);
> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
> "%s: backend socket is disconnected.",
> proxy_function);
> +
> + conn->ssl_hostname = apr_pstrdup(conn->scpool, ssl_hostname);
> }
> }
> while ((backend_addr || conn->uds_path) && !connected) {
>
>
>
Regards
Rüdiger
Re: svn commit: r1729826 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Feb 11, 2016 at 3:12 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
>
>> Am 11.02.2016 um 15:10 schrieb Ruediger Pluem <rp...@apache.org>:
>>
>>
>> conn->ssl_hostname might be allocated from conn->scpool.
>> So the pointer might be invalid after socket_cleanup(conn). So you need to apr_pstrdup it first.
>> No real good idea which pool to use here. Probably create a temp subpool of conn->pool,
>> strdup, after socket_cleanup dup with conn->scpool and destroy tmp pool.
>
> I only dup'ed it once into a stack pointer. So, I used the r->pool for that.
But we don't have an 'r' in ap_proxy_connect_backend(), maybe a new
_ex() function is needed.
Re: svn commit: r1729826 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 11.02.2016 um 15:10 schrieb Ruediger Pluem <rp...@apache.org>:
>
>
>
> On 02/11/2016 02:46 PM, ylavic@apache.org wrote:
>> Author: ylavic
>> Date: Thu Feb 11 13:46:39 2016
>> New Revision: 1729826
>>
>> URL: http://svn.apache.org/viewvc?rev=1729826&view=rev
>> Log:
>> mod_proxy: Play/restore the TLS-SNI on new backend connections which
>> had to be issued because the remote closed the previous/reusable one
>> during idle (keep-alive) time.
>>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/modules/proxy/proxy_util.c
>>
>> Modified: httpd/httpd/trunk/CHANGES
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1729826&r1=1729825&r2=1729826&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>> +++ httpd/httpd/trunk/CHANGES [utf-8] Thu Feb 11 13:46:39 2016
>> @@ -1,6 +1,10 @@
>> -*- coding: utf-8 -*-
>> Changes with Apache 2.5.0
>>
>> + *) mod_proxy: Play/restore the TLS-SNI on new backend connections which
>> + had to be issued because the remote closed the previous/reusable one
>> + during idle (keep-alive) time. [Yann Ylavic]
>> +
>> *) mod_proxy_http2: new experimental http2 proxy module for h2: and h2c: proxy
>> urls. Uses, so far, one connection per request, reuses connections.
>>
>>
>> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1729826&r1=1729825&r2=1729826&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Thu Feb 11 13:46:39 2016
>> @@ -2717,12 +2717,18 @@ PROXY_DECLARE(int) ap_proxy_connect_back
>>
>> if (conn->sock) {
>> if (!(connected = ap_proxy_is_socket_connected(conn->sock))) {
>> - /* FIXME: this loses conn->ssl_hostname and it will not be
>> - * restablished before the SSL connection is made -> no SNI! */
>> + /* This clears conn->scpool (and associated data), so backup and
>> + * restore any ssl_hostname for this connection set earlier by
>> + * ap_proxy_determine_connection().
>> + */
>> + const char *ssl_hostname = conn->ssl_hostname;
>
> conn->ssl_hostname might be allocated from conn->scpool.
> So the pointer might be invalid after socket_cleanup(conn). So you need to apr_pstrdup it first.
> No real good idea which pool to use here. Probably create a temp subpool of conn->pool,
> strdup, after socket_cleanup dup with conn->scpool and destroy tmp pool.
I only dup'ed it once into a stack pointer. So, I used the r->pool for that.
>
>> +
>> socket_cleanup(conn);
>> ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(00951)
>> "%s: backend socket is disconnected.",
>> proxy_function);
>> +
>> + conn->ssl_hostname = apr_pstrdup(conn->scpool, ssl_hostname);
>> }
>> }
>> while ((backend_addr || conn->uds_path) && !connected) {
>>
>>
>>
>
> Regards
>
> Rüdiger
Re: svn commit: r1729826 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by Stefan Eissing <st...@greenbytes.de>.
Works in my cases.
> Am 11.02.2016 um 16:00 schrieb Yann Ylavic <yl...@gmail.com>:
>
> On Thu, Feb 11, 2016 at 3:14 PM, Yann Ylavic <yl...@gmail.com> wrote:
>> On Thu, Feb 11, 2016 at 3:10 PM, Ruediger Pluem <rp...@apache.org> wrote:
>>>
>>> conn->ssl_hostname might be allocated from conn->scpool.
>>> So the pointer might be invalid after socket_cleanup(conn). So you need to apr_pstrdup it first.
>>> No real good idea which pool to use here.
>>
>> Oh, correct!
>>
>>> Probably create a temp subpool of conn->pool,
>>> strdup, after socket_cleanup dup with conn->scpool and destroy tmp pool.
>>
>> Yes, possibly overkill, I have to find something else...
>
> Maybe r1729847 is fine?
>
>>
>> Regards,
>> Yann.
Re: svn commit: r1729826 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Feb 11, 2016 at 3:14 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Thu, Feb 11, 2016 at 3:10 PM, Ruediger Pluem <rp...@apache.org> wrote:
>>
>> conn->ssl_hostname might be allocated from conn->scpool.
>> So the pointer might be invalid after socket_cleanup(conn). So you need to apr_pstrdup it first.
>> No real good idea which pool to use here.
>
> Oh, correct!
>
>> Probably create a temp subpool of conn->pool,
>> strdup, after socket_cleanup dup with conn->scpool and destroy tmp pool.
>
> Yes, possibly overkill, I have to find something else...
Maybe r1729847 is fine?
>
> Regards,
> Yann.
Re: svn commit: r1729826 - in /httpd/httpd/trunk: CHANGES modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Feb 11, 2016 at 3:10 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
> conn->ssl_hostname might be allocated from conn->scpool.
> So the pointer might be invalid after socket_cleanup(conn). So you need to apr_pstrdup it first.
> No real good idea which pool to use here.
Oh, correct!
> Probably create a temp subpool of conn->pool,
> strdup, after socket_cleanup dup with conn->scpool and destroy tmp pool.
Yes, possibly overkill, I have to find something else...
Regards,
Yann.