You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2012/07/23 21:22:39 UTC

ProxyBlock question

Short question: should ProxyBlock apply to the hostname from the request 
URI, or the hostname of the next hop?

Long question: the way ProxyBlock is documented does not make explicit 
that it is applied to the next hop; it would be natural to expect it is 
matched against the request URI hostname.  In this configuration:

   ProxyRequests on
   ProxyBlock badstuff.com
   ProxyRemote * http://cache.mycorp.com/

cache.mycorp.com is the next hop, so it is "cache.mycorp.com" which is 
checked against the ->noproxies list, and in that case would never 
match.  I'm struggling to think how that's useful... feature or bug?

Implications:

a) if the current implementation is the desired behaviour, that needs to 
be clear in the docs, and mod_proxy_connect doesn't implement it 
correctly, but that's all simple enough to fix up.

b) if it's not the desired behaviour, that's a lot more messy.

Regards, Joe

Re: [PATCH] Re: ProxyBlock question

Posted by Joe Orton <jo...@redhat.com>.
On Tue, Jul 24, 2012 at 10:05:34AM +0000, Plüm, Rüdiger, Vodafone Group wrote:
> Looks good. Slight optimization:
> 
> If addr == NULL we can just skip the whole  while (conf_addr) {
> loop.

Thanks to all for the feedback.

main fix: http://svn.apache.org/viewvc?rev=1365001&view=rev
pool use fix: http://svn.apache.org/viewvc?rev=1365020&view=rev
& style fix: http://svn.apache.org/viewvc?rev=1365029&view=rev

...will propose backport shortly.

Regards, Joe

RE: [PATCH] Re: ProxyBlock question

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Joe Orton [mailto:jorton@redhat.com]
> Sent: Dienstag, 24. Juli 2012 11:37
> To: dev@httpd.apache.org
> Subject: [PATCH] Re: ProxyBlock question
> 
> On Tue, Jul 24, 2012 at 08:42:34AM +0000, Plüm, Rüdiger, Vodafone Group
> wrote:
> > So after this rant I come to the conclusion that your proposed
> approach is the best:
> >
> > Only compare the names and not the IP's in the proxy case.
> 
> Attached does this - any comments?  I suppose this requires a major MMN
> bump?  Can backport it in a backwards-compat way for 2.4.x, though.

Looks good. Slight optimization:

If addr == NULL we can just skip the whole  while (conf_addr) {
loop.

Regards

Rüdiger

[PATCH] Re: ProxyBlock question

Posted by Joe Orton <jo...@redhat.com>.
On Tue, Jul 24, 2012 at 08:42:34AM +0000, Plüm, Rüdiger, Vodafone Group wrote:
> So after this rant I come to the conclusion that your proposed approach is the best:
> 
> Only compare the names and not the IP's in the proxy case.

Attached does this - any comments?  I suppose this requires a major MMN 
bump?  Can backport it in a backwards-compat way for 2.4.x, though.

RE: ProxyBlock question

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Joe Orton [mailto:jorton@redhat.com]
> Sent: Dienstag, 24. Juli 2012 10:20
> To: dev@httpd.apache.org
> Subject: Re: ProxyBlock question
> 
> On Tue, Jul 24, 2012 at 07:55:27AM +0000, Plüm, Rüdiger, Vodafone Group
> wrote:
> > Thanks. The patch reminded me of a special situation where the patch
> > might not be suitable: If the forward proxy just forwards everything
> > to the next proxy e.g. because it cannot do DNS lookups of the target
> > URL's
> 
> Exactly my thought.  So in presence of a forward proxy, the "least
> worst" option is probably to omit the DNS lookup and only do the string
> comparison against the ->noproxies list?  Doing a (possibly slow to
> timeout) DNS lookup just in case could impose a horrible performance
> hit.

I think in trunk ap_proxy_checkproxyblock already does the correct thing.
It only compares the addresses if the DNS lookup for the ProxyBlock was successful
during startup / configuration. The issue with ap_proxy_checkproxyblock is that it
expects the host to compare as a apr_sockaddr_t type which would force us to do the
possible bad DNS lookup before calling ap_proxy_checkproxyblock. So apart from the question
if we can generate a fake apr_sockaddr_t variable which contains only the name but not the IP
it would be handy to know if there are any ProxyBlock members for which we had
a successful DNS lookup. OTOH this means nothing as the ProxyBlock might contain hostname that
could be resolved successfully via e.g. a hosts file like localhost or something else.

So after this rant I come to the conclusion that your proposed approach is the best:

Only compare the names and not the IP's in the proxy case.

Regards

Rüdiger


RE: ProxyBlock question

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Rainer Jung [mailto:]
> Sent: Dienstag, 24. Juli 2012 12:49
> To: dev@httpd.apache.org
> Subject: Re: ProxyBlock question
> 
> On 24.07.2012 11:22, Joe Orton wrote:
> 
> > (But reading that code again, you also lead me to another bug; the use
> > of apr_sockaddr_ip_get() against resolved addresses on the ->noproxies
> > list looks to be leaky/unsafe, it will allocate memory out of pconf
> each
> > time we check a resolved address!)
> 
> :(

I guess we should use apr_sockaddr_ip_getbuf instead and allocate the buffer
by ourselves from the correct pool / use a local char array of the maximum size needed,
which is IMHO 46. So something like this:

Index: modules/proxy/proxy_util.c        
===================================================================
--- modules/proxy/proxy_util.c  (revision 1364919)                 
+++ modules/proxy/proxy_util.c  (working copy)                     
@@ -759,6 +759,8 @@                                                
     return host != NULL && ap_strstr_c(host, This->name) != NULL; 
 }                                                                 

+#define MAX_IP_STR_LEN 46
+
 /* checks whether a host in uri_addr matches proxyblock */
 PROXY_DECLARE(int) ap_proxy_checkproxyblock(request_rec *r, proxy_server_conf *conf,
                              apr_sockaddr_t *uri_addr)
@@ -783,10 +785,12 @@
         while (conf_addr) {
             uri_addr = src_uri_addr;
             while (uri_addr) {
-                char *conf_ip;
-                char *uri_ip;
-                apr_sockaddr_ip_get(&conf_ip, conf_addr);
-                apr_sockaddr_ip_get(&uri_ip, uri_addr);
+                char conf_ip[MAX_IP_STR_LEN];
+                char uri_ip[MAX_IP_STR_LEN];
+                apr_sockaddr_ip_getbuf(conf_ip, conf_addr->addr_str_len,
+                                       conf_addr);
+                apr_sockaddr_ip_getbuf(uri_ip, uri_addr->addr_str_len,
+                                       uri_addr);
                 ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,
                               "ProxyBlock comparing %s and %s", conf_ip,
                               uri_ip);

Regards

Rüdiger

Re: ProxyBlock question

Posted by Rainer Jung <ra...@kippdata.de>.
On 24.07.2012 11:22, Joe Orton wrote:
> On Tue, Jul 24, 2012 at 10:46:12AM +0200, Rainer Jung wrote:
>> IMHO if the admin explicitely configured an IP in the ProxyBlock
>> list we should nevertheless check. For this case there's already a
>> somewhat related warning in the docs which we could enhance for this
>> new case.
>>
>> It looks like we could check whether we have an explicit IP during
>> set_proxy_exclude() by comparing new->name and apr_sockaddr_ip_get()
>> of new->addr and later do the IP lookup for the target host only for
>> those rules where we had an explicit IP.
>>
>> Not sure whether apr_sockaddr_ip_get() applied to the result of
>> apr_sockaddr_info_get() applied to an IP gives back the same IP,
>> e.g. when there's IPv4 and v6 involved.
>
> Right, with a v6 address there can be multiple representations of the
> same address so that wouldn't be reliable.
>
> This seems to pile caveat on top of caveat; is it really necessary?
> ProxyBlock is not even documented to take literal IP addresses, but
> rather "*|word|host|domain".  Adding a special case for a literal IP
> will add significant complexity here; is it useful?  If there is a
> forward proxy configured why can't that proxy block the IP address?

You are right, I got the feature form the code not really from the docs. 
We might remov the sentence "rocky.wotsamattau.edu would also be matched 
if referenced by IP address." though or explain the limitations. Now 
that we have understood it, that's easy. So I'm OK with not supporting 
checking the request IP in the case we use another proxy.

> (But reading that code again, you also lead me to another bug; the use
> of apr_sockaddr_ip_get() against resolved addresses on the ->noproxies
> list looks to be leaky/unsafe, it will allocate memory out of pconf each
> time we check a resolved address!)

:(

Thanks!

Rainer


Re: ProxyBlock question

Posted by Joe Orton <jo...@redhat.com>.
On Tue, Jul 24, 2012 at 10:46:12AM +0200, Rainer Jung wrote:
> IMHO if the admin explicitely configured an IP in the ProxyBlock
> list we should nevertheless check. For this case there's already a
> somewhat related warning in the docs which we could enhance for this
> new case.
> 
> It looks like we could check whether we have an explicit IP during
> set_proxy_exclude() by comparing new->name and apr_sockaddr_ip_get()
> of new->addr and later do the IP lookup for the target host only for
> those rules where we had an explicit IP.
> 
> Not sure whether apr_sockaddr_ip_get() applied to the result of
> apr_sockaddr_info_get() applied to an IP gives back the same IP,
> e.g. when there's IPv4 and v6 involved.

Right, with a v6 address there can be multiple representations of the 
same address so that wouldn't be reliable. 

This seems to pile caveat on top of caveat; is it really necessary? 
ProxyBlock is not even documented to take literal IP addresses, but 
rather "*|word|host|domain".  Adding a special case for a literal IP 
will add significant complexity here; is it useful?  If there is a 
forward proxy configured why can't that proxy block the IP address?

(But reading that code again, you also lead me to another bug; the use 
of apr_sockaddr_ip_get() against resolved addresses on the ->noproxies 
list looks to be leaky/unsafe, it will allocate memory out of pconf each 
time we check a resolved address!)

Regards, Joe

Re: ProxyBlock question

Posted by Rainer Jung <ra...@kippdata.de>.
On 24.07.2012 10:20, Joe Orton wrote:
> On Tue, Jul 24, 2012 at 07:55:27AM +0000, Plüm, Rüdiger, Vodafone Group wrote:
>> Thanks. The patch reminded me of a special situation where the patch
>> might not be suitable: If the forward proxy just forwards everything
>> to the next proxy e.g. because it cannot do DNS lookups of the target
>> URL's
>
> Exactly my thought.  So in presence of a forward proxy, the "least
> worst" option is probably to omit the DNS lookup and only do the string
> comparison against the ->noproxies list?  Doing a (possibly slow to
> timeout) DNS lookup just in case could impose a horrible performance
> hit.

IMHO if the admin explicitely configured an IP in the ProxyBlock list we 
should nevertheless check. For this case there's already a somewhat 
related warning in the docs which we could enhance for this new case.

It looks like we could check whether we have an explicit IP during 
set_proxy_exclude() by comparing new->name and apr_sockaddr_ip_get() of 
new->addr and later do the IP lookup for the target host only for those 
rules where we had an explicit IP.

Not sure whether apr_sockaddr_ip_get() applied to the result of 
apr_sockaddr_info_get() applied to an IP gives back the same IP, e.g. 
when there's IPv4 and v6 involved.

Regards,

Rainer


Re: ProxyBlock question

Posted by Joe Orton <jo...@redhat.com>.
On Tue, Jul 24, 2012 at 07:55:27AM +0000, Plüm, Rüdiger, Vodafone Group wrote:
> Thanks. The patch reminded me of a special situation where the patch 
> might not be suitable: If the forward proxy just forwards everything 
> to the next proxy e.g. because it cannot do DNS lookups of the target 
> URL's

Exactly my thought.  So in presence of a forward proxy, the "least 
worst" option is probably to omit the DNS lookup and only do the string 
comparison against the ->noproxies list?  Doing a (possibly slow to 
timeout) DNS lookup just in case could impose a horrible performance 
hit.

Regards, Joe


RE: ProxyBlock question

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Rainer Jung [mailto:
> Sent: Dienstag, 24. Juli 2012 09:40
> To: dev@httpd.apache.org
> Subject: Re: ProxyBlock question
> 
> On 24.07.2012 08:58, Plüm, Rüdiger, Vodafone Group wrote:
> >
> >
> >> -----Original Message-----
> >> From: Joe Orton > Sent: Montag, 23. Juli 2012 22:06
> >> To: dev@httpd.apache.org
> >> Subject: Re: ProxyBlock question
> >>
> >> On Mon, Jul 23, 2012 at 03:41:19PM -0400, Eric Covener wrote:
> >>>> b) if it's not the desired behaviour, that's a lot more messy.
> >>>
> >>> I had assumed this was a bug in the checking but apparently never
> >>> brought it here correctly.
> >>
> >> Ah ha!  I hadn't checked the list archives, sorry - you did indeed
> post
> >> on this, and there's even a patch.  Thanks Eric, I will take a
> further
> >> look now.
> >
> > Thanks Joe. IMHO the current behaviour is a bug.
> 
> I also agree. The usefulness of the current behaviour is very limited
> whereas checking the request URL would be the expected behaviour.
> 
> The old post mentioned by Joe (2005) can be found at:
> 
> http://marc.info/?t=111446232900002&r=1&w=2

Thanks. The patch reminded me of a special situation where the patch might not be suitable:
If the forward proxy just forwards everything to the next proxy e.g. because it cannot do DNS
lookups of the target URL's

Regards

Rüdiger

Re: ProxyBlock question

Posted by Rainer Jung <ra...@kippdata.de>.
On 24.07.2012 08:58, Plüm, Rüdiger, Vodafone Group wrote:
>
>
>> -----Original Message-----
>> From: Joe Orton > Sent: Montag, 23. Juli 2012 22:06
>> To: dev@httpd.apache.org
>> Subject: Re: ProxyBlock question
>>
>> On Mon, Jul 23, 2012 at 03:41:19PM -0400, Eric Covener wrote:
>>>> b) if it's not the desired behaviour, that's a lot more messy.
>>>
>>> I had assumed this was a bug in the checking but apparently never
>>> brought it here correctly.
>>
>> Ah ha!  I hadn't checked the list archives, sorry - you did indeed post
>> on this, and there's even a patch.  Thanks Eric, I will take a further
>> look now.
>
> Thanks Joe. IMHO the current behaviour is a bug.

I also agree. The usefulness of the current behaviour is very limited 
whereas checking the request URL would be the expected behaviour.

The old post mentioned by Joe (2005) can be found at:

http://marc.info/?t=111446232900002&r=1&w=2

Regards,

Rainer





RE: ProxyBlock question

Posted by Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>.

> -----Original Message-----
> From: Joe Orton > Sent: Montag, 23. Juli 2012 22:06
> To: dev@httpd.apache.org
> Subject: Re: ProxyBlock question
> 
> On Mon, Jul 23, 2012 at 03:41:19PM -0400, Eric Covener wrote:
> > > b) if it's not the desired behaviour, that's a lot more messy.
> >
> > I had assumed this was a bug in the checking but apparently never
> > brought it here correctly.
> 
> Ah ha!  I hadn't checked the list archives, sorry - you did indeed post
> on this, and there's even a patch.  Thanks Eric, I will take a further
> look now.

Thanks Joe. IMHO the current behaviour is a bug.

Regards

Rüdiger


Re: ProxyBlock question

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Jul 23, 2012 at 03:41:19PM -0400, Eric Covener wrote:
> > b) if it's not the desired behaviour, that's a lot more messy.
> 
> I had assumed this was a bug in the checking but apparently never
> brought it here correctly.

Ah ha!  I hadn't checked the list archives, sorry - you did indeed post 
on this, and there's even a patch.  Thanks Eric, I will take a further 
look now.

Regards, Joe

Re: ProxyBlock question

Posted by Eric Covener <co...@gmail.com>.
> b) if it's not the desired behaviour, that's a lot more messy.

I had assumed this was a bug in the checking but apparently never
brought it here correctly.