You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2016/12/09 14:00:51 UTC

svn commit: r1773397 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c

Author: covener
Date: Fri Dec  9 14:00:51 2016
New Revision: 1773397

URL: http://svn.apache.org/viewvc?rev=1773397&view=rev
Log:
ProxyPass ! doesn't block per-directory ProxyPass

 *) mod_proxy: Honor a server scoped ProxyPass exception when ProxyPass is
     configured in <Location>, like in 2.2. PR 60458.
     [Eric Covener]


Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/proxy/mod_proxy.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1773397&r1=1773396&r2=1773397&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Dec  9 14:00:51 2016
@@ -1,9 +1,13 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+ *) mod_proxy: Honor a server scoped ProxyPass exception when ProxyPass is
+     configured in <Location>, like in 2.2. PR 60458.
+     [Eric Covener]
+
   *) core: Drop Content-Length header and message-body from HTTP 204 responses.
      PR 51350 [Luca Toscano]
-
+ 
   *) mod_lua: Fix default value of LuaInherit directive. It should be 
      'parent-first' instead of 'none', as per documentation.  PR 60419
      [Christophe Jaillet]

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1773397&r1=1773396&r2=1773397&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Fri Dec  9 14:00:51 2016
@@ -788,18 +788,29 @@ static int proxy_trans(request_rec *r)
      */
 
     dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
-
+    conf = (proxy_server_conf *) ap_get_module_config(r->server->module_config,
+                                                      &proxy_module);
     /* short way - this location is reverse proxied? */
     if (dconf->alias) {
         int rv = ap_proxy_trans_match(r, dconf->alias, dconf);
+        if (OK == rv) { 
+            /* Got a hit. Need to make sure it's not explicitly declined */
+            if (conf->aliases->nelts) {
+                ent = (struct proxy_alias *) conf->aliases->elts;
+                for (i = 0; i < conf->aliases->nelts; i++) {
+                    int rv = ap_proxy_trans_match(r, &ent[i], dconf);
+                    if (DECLINED == rv) { 
+                        return DECLINED;
+                    }
+                }
+            }
+            return OK; 
+        }
         if (DONE != rv) {
             return rv;
         }
     }
 
-    conf = (proxy_server_conf *) ap_get_module_config(r->server->module_config,
-                                                      &proxy_module);
-
     /* long way - walk the list of aliases, find a match */
     if (conf->aliases->nelts) {
         ent = (struct proxy_alias *) conf->aliases->elts;



Re: svn commit: r1773397 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c

Posted by Eric Covener <co...@gmail.com>.
Could offer instead (in the <location> case) a "noproxy" envvar, which
is a hash lookup instead of iterating.

On Mon, Jan 30, 2017 at 7:52 AM, Eric Covener <co...@gmail.com> wrote:
> On Mon, Jan 30, 2017 at 7:21 AM, Joe Orton <jo...@redhat.com> wrote:
>>> ProxyPass ! doesn't block per-directory ProxyPass
>>>
>>>  *) mod_proxy: Honor a server scoped ProxyPass exception when ProxyPass is
>>>      configured in <Location>, like in 2.2. PR 60458.
>>>      [Eric Covener]
>>
>> Not immediately obvious to me how to fix it (sorry), but a Fedora user
>> reported that this is a regression in 2.4.25, breaking a config like:
>>
>> <VirtualHost>
>>    ProxyPass / foo
>>    <Location /bar>
>>      ProxyPass baz
>>    <Location>
>> </VirtualHost>
>>
>> The requests to /bar no longer maps to baz. I added a test case in
>> r1780905.
>
>
> I have a fix but not sure if the change should just be reverted. In
> the PR, the user changed the 2.2 config to make the ProxyPass within
> location and expected similar behavior.
>
> Should have probably just told them that exceptions just could not be
> done that way.
>
> PR config is
>
> ProxyPass /error !
> <Location />
>   ProxyPass http://foo/...
> </Location>
>
> It seemed useful at the time, but since stuffing the thing inside of
> Location is not all that more useful functionally, adding more code
> seems like a mistake.
>
> Any opinions?
>
> --
> Eric Covener
> covener@gmail.com



-- 
Eric Covener
covener@gmail.com

Re: svn commit: r1773397 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c

Posted by Luca Toscano <to...@gmail.com>.
2017-01-31 10:53 GMT+01:00 Plüm, Rüdiger, Vodafone Group <
ruediger.pluem@vodafone.com>:

>
>
> > -----Ursprüngliche Nachricht-----
> > Von: Joe Orton [mailto:jorton@redhat.com]
> > Gesendet: Dienstag, 31. Januar 2017 10:42
> > An: dev@httpd.apache.org
> > Betreff: Re: svn commit: r1773397 - in /httpd/httpd/trunk: CHANGES
> > modules/proxy/mod_proxy.c
> >
> > On Mon, Jan 30, 2017 at 07:52:03AM -0500, Eric Covener wrote:
> > > I have a fix but not sure if the change should just be reverted. In
> > > the PR, the user changed the 2.2 config to make the ProxyPass within
> > > location and expected similar behavior.
> > >
> > > Should have probably just told them that exceptions just could not be
> > > done that way.
> > >
> > > PR config is
> > >
> > > ProxyPass /error !
> > > <Location />
> > >   ProxyPass http://foo/...
> > > </Location>
> > >
> > > It seemed useful at the time, but since stuffing the thing inside of
> > > Location is not all that more useful functionally, adding more code
> > > seems like a mistake.
> >
> > Asserting/documenting that higher-level exceptions don't apply for
> > ProxyPass within <Location> seems pretty reasonable to me.
> >
>
> +1


While reviewing https://bz.apache.org/bugzilla/show_bug.cgi?id=61225 I
tried to add some clarification with http://svn.apache.org/r1828069 (trunk
only for the moment, will wait a bit before backporting if anybody wants to
add/amend).

Luca

AW: svn commit: r1773397 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c

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

> -----Ursprüngliche Nachricht-----
> Von: Joe Orton [mailto:jorton@redhat.com]
> Gesendet: Dienstag, 31. Januar 2017 10:42
> An: dev@httpd.apache.org
> Betreff: Re: svn commit: r1773397 - in /httpd/httpd/trunk: CHANGES
> modules/proxy/mod_proxy.c
> 
> On Mon, Jan 30, 2017 at 07:52:03AM -0500, Eric Covener wrote:
> > I have a fix but not sure if the change should just be reverted. In
> > the PR, the user changed the 2.2 config to make the ProxyPass within
> > location and expected similar behavior.
> >
> > Should have probably just told them that exceptions just could not be
> > done that way.
> >
> > PR config is
> >
> > ProxyPass /error !
> > <Location />
> >   ProxyPass http://foo/...
> > </Location>
> >
> > It seemed useful at the time, but since stuffing the thing inside of
> > Location is not all that more useful functionally, adding more code
> > seems like a mistake.
> 
> Asserting/documenting that higher-level exceptions don't apply for
> ProxyPass within <Location> seems pretty reasonable to me.
> 

+1

Regards

Rüdiger

Re: svn commit: r1773397 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Jan 30, 2017 at 07:52:03AM -0500, Eric Covener wrote:
> I have a fix but not sure if the change should just be reverted. In
> the PR, the user changed the 2.2 config to make the ProxyPass within
> location and expected similar behavior.
> 
> Should have probably just told them that exceptions just could not be
> done that way.
> 
> PR config is
> 
> ProxyPass /error !
> <Location />
>   ProxyPass http://foo/...
> </Location>
> 
> It seemed useful at the time, but since stuffing the thing inside of
> Location is not all that more useful functionally, adding more code
> seems like a mistake.

Asserting/documenting that higher-level exceptions don't apply for 
ProxyPass within <Location> seems pretty reasonable to me.

Regards, Joe

Re: svn commit: r1773397 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c

Posted by Eric Covener <co...@gmail.com>.
On Mon, Jan 30, 2017 at 7:21 AM, Joe Orton <jo...@redhat.com> wrote:
>> ProxyPass ! doesn't block per-directory ProxyPass
>>
>>  *) mod_proxy: Honor a server scoped ProxyPass exception when ProxyPass is
>>      configured in <Location>, like in 2.2. PR 60458.
>>      [Eric Covener]
>
> Not immediately obvious to me how to fix it (sorry), but a Fedora user
> reported that this is a regression in 2.4.25, breaking a config like:
>
> <VirtualHost>
>    ProxyPass / foo
>    <Location /bar>
>      ProxyPass baz
>    <Location>
> </VirtualHost>
>
> The requests to /bar no longer maps to baz. I added a test case in
> r1780905.


I have a fix but not sure if the change should just be reverted. In
the PR, the user changed the 2.2 config to make the ProxyPass within
location and expected similar behavior.

Should have probably just told them that exceptions just could not be
done that way.

PR config is

ProxyPass /error !
<Location />
  ProxyPass http://foo/...
</Location>

It seemed useful at the time, but since stuffing the thing inside of
Location is not all that more useful functionally, adding more code
seems like a mistake.

Any opinions?

-- 
Eric Covener
covener@gmail.com

Re: svn commit: r1776575 - in /httpd/httpd/trunk: docs/log-message-tags/next-number docs/manual/mod/mod_remoteip.xml modules/metadata/mod_remoteip.c

Posted by Daniel Ruggeri <DR...@primary.net>.
I'm not sure if my mail client mangled the message or my email provider
did, but I couldn't read this except from lists.a.o so my reply may
appear as a new thread in your email client if it's following thread IDs.


Christophe JAILLET wrote:
> First of all, http://blog.haproxy.com/haproxy/proxy-protocol/ list 
> another module implementation for Apache:
> https://github.com/ggrandes/apache24-modules/blob/master/mod_myfixip.c
>
> If anyone wants to give it a look.
>
>
>
> Anyway, a few minor comments below.
>
> CJ
>
>
> Le 30/12/2016 �  15:20, druggeri@apache.org a écrit :
>> Author: druggeri
>> Date: Fri Dec 30 14:20:48 2016
>> New Revision: 1776575
>>
>> URL:http://svn.apache.org/viewvc?rev=1776575&view=rev
>> Log:
>> Merge new PROXY protocol code into mod_remoteip
>>
>> Modified:
>>      httpd/httpd/trunk/docs/log-message-tags/next-number
>>      httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml
>>      httpd/httpd/trunk/modules/metadata/mod_remoteip.c
>>
>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>> URL:http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1776575&r1=1776574&r2=1776575&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Dec 30 14:20:48 2016
>> @@ -1 +1 @@
>> -3491
>> +3514
>>
>> Modified: httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml
>> URL:http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml?rev=1776575&r1=1776574&r2=1776575&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml (original)
>> +++ httpd/httpd/trunk/docs/manual/mod/mod_remoteip.xml Fri Dec 30 14:20:48 2016
>> @@ -42,6 +42,12 @@ via the request headers.
>>       with the useragent IP address reported in the request header configured
>>       with the <directive module="mod_remoteip">RemoteIPHeader</directive> directive.</p>
>>   
>> +    <p>Additionally, this module implements the server side of
>> +    HAProxy's
>> +    <a href="http://blog.haproxy.com/haproxy/proxy-protocol/">Proxy Protocol</a> when
>> +    using the <directive module="mod_remoteip">RemoteIPProxyProtocolEnable</directive>
>> +    directive.</p>
>> +
>>       <p>Once replaced as instructed, this overridden useragent IP address is
>>       then used for the <module>mod_authz_host</module>
>>       <directive module="mod_authz_core" name="Require">Require ip</directive>
>> @@ -59,6 +65,7 @@ via the request headers.
>>   <seealso><module>mod_authz_host</module></seealso>
>>   <seealso><module>mod_status</module></seealso>
>>   <seealso><module>mod_log_config</module></seealso>
>> +<seealso><a href="http://www.haproxy.org/download/1.5/doc/proxy-protocol.txt">Proxy Protocol Spec</a></seealso>
>>   
>>   <section id="processing"><title>Remote IP Processing</title>
>>   
>> @@ -214,6 +221,70 @@ RemoteIPProxiesHeader X-Forwarded-By
>>   </usage>
>>   </directivesynopsis>
>>   
>> +<directivesynopsis>
>> +<name>RemoteIPProxyProtocol</name>
> s/RemoteIPProxyProtocol/RemoteIPProxyProtocolEnable/

Right - this was addressed in a subsequent commit after discussing the
name. It's now RemoteIPProxyProtocol


>
>> +<description>Enable, optionally enable or disable the proxy protocol handling</description>
>> +<syntax>ProxyProtocol On|Optional|Off</syntax>
>> +<contextlist><context>server config</context><context>virtual host</context>
>> +</contextlist>
> Compatibility note missing.

Added - thanks!


>
>> ....
>>
>> Modified: httpd/httpd/trunk/modules/metadata/mod_remoteip.c
>> URL:http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/metadata/mod_remoteip.c?rev=1776575&r1=1776574&r2=1776575&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/metadata/mod_remoteip.c (original)
>> +++ httpd/httpd/trunk/modules/metadata/mod_remoteip.c Fri Dec 30 14:20:48 2016
>> @@ -12,15 +12,20 @@
>>    * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>    * See the License for the specific language governing permissions and
>>    * limitations under the License.
>> + *
>> + * The majority of the input filter code for PROXY protocol support is
>> + * Copyright 2014 Cloudzilla Inc.
>>    */
>>   
>>   #include "ap_config.h"
>>   #include "ap_mmn.h"
>> +#include "ap_listen.h"
>>   #include "httpd.h"
>>   #include "http_config.h"
>>   #include "http_connection.h"
>>   #include "http_protocol.h"
>>   #include "http_log.h"
>> +#include "http_main.h"
>>   #include "apr_strings.h"
>>   #include "apr_lib.h"
>>   #define APR_WANT_BYTEFUNC
>> @@ -36,6 +41,12 @@ typedef struct {
>>       void  *internal;
>>   } remoteip_proxymatch_t;
>>   
>> +typedef struct remoteip_addr_info {
>> +    struct remoteip_addr_info *next;
>> +    apr_sockaddr_t *addr;
>> +    server_rec *source;
>> +} remoteip_addr_info;
>> +
>>   typedef struct {
>>       /** The header to retrieve a proxy-via IP list */
>>       const char *header_name;
>> @@ -48,6 +59,17 @@ typedef struct {
>>        *  with the most commonly encountered listed first
>>        */
>>       apr_array_header_t *proxymatch_ip;
>> +
>> +    remoteip_addr_info *proxy_protocol_enabled;
>> +    remoteip_addr_info *proxy_protocol_optional;
>> +    remoteip_addr_info *proxy_protocol_disabled;
>> +
>> +    /** A flag indicating whether or not proxyprotocol
>> +     * is optoinal for this specific server
>> +     */
>> +    int pp_optional;
>> +
>> +    apr_pool_t *pool;
> Do we really need to keep the config pool here? if badly used", wouldn't 
> it generate a kind of leak?

See response below to your follow up question


>
>>   } remoteip_config_t;
>>   
>>   typedef struct {
>> @@ -59,12 +81,92 @@ typedef struct {
>>       const char *proxied_remote;
>>   } remoteip_req_t;
>>   
>> +/* For PROXY protocol processing */
>> +static const char *remoteip_filter_name = "REMOTEIP_INPUT";
>> +
>> +typedef struct {
>> +    char line[108];
>> +} proxy_v1;
>> +
>> +typedef union {
>> +    struct {        /* for TCP/UDP over IPv4, len = 12 */
>> +        uint32_t src_addr;
>> +        uint32_t dst_addr;
>> +        uint16_t src_port;
>> +        uint16_t dst_port;
>> +    } ip4;
>> +    struct {        /* for TCP/UDP over IPv6, len = 36 */
>> +         uint8_t  src_addr[16];
>> +         uint8_t  dst_addr[16];
>> +         uint16_t src_port;
>> +         uint16_t dst_port;
>> +    } ip6;
>> +    struct {        /* for AF_UNIX sockets, len = 216 */
>> +         uint8_t src_addr[108];
>> +         uint8_t dst_addr[108];
>> +    } unx;
>> +} proxy_v2_addr;
>> +
>> +typedef struct {
>> +    uint8_t  sig[12];  /* hex 0D 0A 0D 0A 00 0D 0A 51 55 49 54 0A */
>> +    uint8_t  ver_cmd;  /* protocol version and command */
>> +    uint8_t  fam;      /* protocol family and address */
>> +    uint16_t len;     /* number of following bytes part of the header */
>> +    proxy_v2_addr addr;
>> +} proxy_v2;
>> +
>> +typedef union {
>> +        proxy_v1 v1;
>> +        proxy_v2 v2;
>> +} proxy_header;
>> +
>> +static const char v2sig[12] = "\x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A";
>> +#define MIN_V1_HDR_LEN 15
>> +#define MIN_V2_HDR_LEN 16
>> +#define MIN_HDR_LEN MIN_V1_HDR_LEN
>> +
>> +/* XXX: Unsure if this is needed if v6 support is not available on
>> +   this platform */
>> +#ifndef INET6_ADDRSTRLEN
>> +#define INET6_ADDRSTRLEN 46
>> +#endif
>> +
>> +typedef struct {
>> +    char header[sizeof(proxy_header)];
>> +    apr_size_t rcvd;
>> +    apr_size_t need;
>> +    int version;
>> +    ap_input_mode_t mode;
>> +    apr_bucket_brigade *bb;
>> +    int done;
>> +} remoteip_filter_context;
>> +
>> +/** Holds the resolved proxy info for this connection and any addition
>> +  configurable parameters
>> +*/
>> +typedef struct {
>> +    /** The parsed client address in native format */
>> +    apr_sockaddr_t *client_addr;
>> +    /** Character representation of the client */
>> +    char *client_ip;
>> +    /** Flag indicating that the PROXY header may be omitted on this
>> +      connection (do not abort if it is missing). */
>> +    int proxy_protocol_optional;
>> +} remoteip_conn_config_t;
>> +
>> +typedef enum { HDR_DONE, HDR_ERROR, HDR_MISSING, HDR_NEED_MORE } remoteip_parse_status_t;
>> +
>>   static void *create_remoteip_server_config(apr_pool_t *p, server_rec *s)
>>   {
>>       remoteip_config_t *config = apr_pcalloc(p, sizeof *config);
>>       /* config->header_name = NULL;
>>        * config->proxies_header_name = NULL;
>>        */
>> +    config->proxy_protocol_enabled = NULL;
>> +    config->proxy_protocol_optional = NULL;
>> +    config->proxy_protocol_disabled = NULL;
>> +    config->pp_optional = 0;
> Useless init because of apr_pcalloc. Should be consistent with 
> header_name and proxies_header_name. All commented or all NULL/0 
> assigned or dropped completely.

NP - moved to comments in latest commit


>
>> +    config->pool = p;
>>       return config;
>>   }
>>   
>> @@ -85,6 +187,9 @@ static void *merge_remoteip_server_confi
>>       config->proxymatch_ip = server->proxymatch_ip
>>                             ? server->proxymatch_ip
>>                             : global->proxymatch_ip;
>> +    config->pp_optional = server->pp_optional
>> +                          ? server->pp_optional
>> +                          : global->pp_optional;
>>       return config;
>>   }
>>   
>> @@ -215,13 +320,184 @@ static const char *proxylist_read(cmd_pa
>>       return NULL;
>>   }
>>   
>> +/** Similar apr_sockaddr_equal, except that it compares ports too. */
>> +static int remoteip_sockaddr_equal(apr_sockaddr_t *addr1, apr_sockaddr_t *addr2)
>> +{
>> +    return (addr1->port == addr2->port && apr_sockaddr_equal(addr1, addr2));
>> +}
>> +
>> +/** Similar remoteip_sockaddr_equal, except that it handles wildcard addresses
>> + *  and ports too.
>> + */
>> +static int remoteip_sockaddr_compat(apr_sockaddr_t *addr1, apr_sockaddr_t *addr2)
>> +{
>> +    /* test exact address equality */
>> +    if (apr_sockaddr_equal(addr1, addr2) &&
>> +        (addr1->port == addr2->port || addr1->port == 0 || addr2->port == 0)) {
>> +        return 1;
>> +    }
>> +
>> +    /* test address wildcards */
>> +    if (apr_sockaddr_is_wildcard(addr1) &&
>> +        (addr1->port == 0 || addr1->port == addr2->port)) {
>> +        return 1;
>> +    }
>> +
>> +    if (apr_sockaddr_is_wildcard(addr2) &&
>> +        (addr2->port == 0 || addr2->port == addr1->port)) {
>> +        return 1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int remoteip_addr_in_list(remoteip_addr_info *list, apr_sockaddr_t *addr)
>> +{
>> +    for (; list; list = list->next) {
>> +        if (remoteip_sockaddr_compat(list->addr, addr)) {
>> +            return 1;
>> +        }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static void remoteip_warn_enable_conflict(remoteip_addr_info *prev, server_rec *new, const char* arg)
>> +{
>> +    char buf[INET6_ADDRSTRLEN];
>> +
>> +    apr_sockaddr_ip_getbuf(buf, sizeof(buf), prev->addr);
>> +
>> +    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, new, APLOGNO(03491)
>> +                 "RemoteIPProxyProtocolEnable: previous setting for %s:%hu from virtual "
>> +                 "host {%s:%hu in %s} is being overriden by virtual host "
>> +                 "{%s:%hu in %s}; new setting is '%s'",
>> +                 buf, prev->addr->port, prev->source->server_hostname,
>> +                 prev->source->addrs->host_port, prev->source->defn_name,
>> +                 new->server_hostname, new->addrs->host_port, new->defn_name,
>> +                 arg);
>> +}
>> +
>> +static const char *remoteip_enable_proxy_protocol(cmd_parms *cmd, void *config,
>> +                                                  const char *arg)
>> +{
>> +    remoteip_config_t *global_conf;
>> +    remoteip_config_t *server_conf;
>> +    server_addr_rec *addr;
>> +    remoteip_addr_info **add;
>> +    int list_len = 2;
>> +    remoteip_addr_info **rem_list[list_len];
>> +    remoteip_addr_info *list;
>> +    int i;
>> +
>> +    global_conf = ap_get_module_config(ap_server_conf->module_config,
>> +                                &remoteip_module);
>> +    server_conf = ap_get_module_config(cmd->server->module_config,
>> +                                &remoteip_module);
>> +
>> +    if (strcasecmp(arg, "On") == 0) {
>> +        add = &global_conf->proxy_protocol_enabled;
>> +        rem_list[0] = &global_conf->proxy_protocol_optional;
>> +        rem_list[1] = &global_conf->proxy_protocol_disabled;
>> +    }
>> +    else if (strcasecmp(arg, "Optional") == 0) {
>> +        add = &global_conf->proxy_protocol_optional;
>> +        rem_list[0] = &global_conf->proxy_protocol_enabled;
>> +        rem_list[1] = &global_conf->proxy_protocol_disabled;
>> +        server_conf->pp_optional = 1;
>> +    }
>> +    else if (strcasecmp(arg, "Off") == 0 ) {
>> +        add = &global_conf->proxy_protocol_disabled;
>> +        rem_list[0] = &global_conf->proxy_protocol_enabled;
>> +        rem_list[1] = &global_conf->proxy_protocol_optional;
>> +    }
>> +    else {
>> +        return apr_pstrcat(cmd->pool, "Unrecognized option for ProxyProtocolEnable `%s'", arg, NULL);
> s/ProxyProtocolEnable/RemoteIPProxyProtocolEnable/
> or cmd->cmd->name to be safe.
I like that even better - updated to the safest case

>
>> +    }
>> +
>> +    for (addr = cmd->server->addrs; addr; addr = addr->next) {
>> +        /* remove address from other lists */
>> +        for (i = 0; i < list_len ; i++) {
>> +            remoteip_addr_info **rem = rem_list[i];
>> +            if (*rem) {
>> +                if (remoteip_sockaddr_equal((*rem)->addr, addr->host_addr)) {
>> +                    remoteip_warn_enable_conflict(*rem, cmd->server, arg);
>> +                    *rem = (*rem)->next;
>> +                }
>> +                else {
>> +                    for (list = *rem; list->next; list = list->next) {
>> +                        if (remoteip_sockaddr_equal(list->next->addr, addr->host_addr)) {
>> +                            remoteip_warn_enable_conflict(list->next, cmd->server, arg);
>> +                            list->next = list->next->next;
>> +                            break;
>> +                        }
>> +                    }
>> +                }
>> +            }
>> +        }
>> +
>> +        /* add address to desired list */
>> +        if (!remoteip_addr_in_list(*add, addr->host_addr)) {
>> +            remoteip_addr_info *info = apr_palloc(global_conf->pool, sizeof(*info));
> Could cmd->pool be used here, instead?

This came from the original authors of the code, but I think it's
correct. This is the only place remoteip_config_t->pool is allocated
into. A collection of all enabled, disabled and optional
remoteip_addr_info structs is kept and examined pre-connection to
determine if the filter should be inserted for the connection. Since the
server is not known pre-connection, this must be stored in the global
server. The lifetime of cmd->pool would prevent using it here.


>
>> . . . 
>>   static const command_rec remoteip_cmds[] =
>>   {
>>       AP_INIT_TAKE1("RemoteIPHeader", header_name_set, NULL, RSRC_CONF,
>> @@ -450,11 +1211,21 @@ static const command_rec remoteip_cmds[]
>>                     RSRC_CONF | EXEC_ON_READ,
>>                     "The filename to read the list of internal proxies, "
>>                     "see the RemoteIPInternalProxy directive"),
>> +    AP_INIT_TAKE1("RemoteIPProxyProtocolEnable", remoteip_enable_proxy_protocol, NULL,
>> +                  RSRC_CONF, "Enable proxy-protocol handling (`on', `off')"),
> `optional' is missing

Fixed - thanks!


>
>>       { NULL }
>>   };
>>   
>>   static void register_hooks(apr_pool_t *p)
>>   {
>> +    /* mod_ssl is CONNECTION + 5, so we want something higher (earlier);
>> +     * mod_reqtimeout is CONNECTION + 8, so we want something lower (later) */
>> +    ap_register_input_filter(remoteip_filter_name, remoteip_input_filter, NULL,
>> +                             AP_FTYPE_CONNECTION + 7);
>> +
>> +    ap_hook_pre_config(remoteip_hook_pre_config, NULL, NULL, APR_HOOK_MIDDLE);
>> +    ap_hook_post_config(remoteip_hook_post_config, NULL, NULL, APR_HOOK_MIDDLE);
>> +    ap_hook_pre_connection(remoteip_hook_pre_connection, NULL, NULL, APR_HOOK_MIDDLE);
>>       ap_hook_post_read_request(remoteip_modify_request, NULL, NULL, APR_HOOK_FIRST);
>>   }
>>   

-- 
Daniel Ruggeri


Re: svn commit: r1773397 - in /httpd/httpd/trunk: CHANGES modules/proxy/mod_proxy.c

Posted by Joe Orton <jo...@redhat.com>.
On Fri, Dec 09, 2016 at 02:00:51PM -0000, covener@apache.org wrote:
> Author: covener
> Date: Fri Dec  9 14:00:51 2016
> New Revision: 1773397
> 
> URL: http://svn.apache.org/viewvc?rev=1773397&view=rev
> Log:
> ProxyPass ! doesn't block per-directory ProxyPass
> 
>  *) mod_proxy: Honor a server scoped ProxyPass exception when ProxyPass is
>      configured in <Location>, like in 2.2. PR 60458.
>      [Eric Covener]

Not immediately obvious to me how to fix it (sorry), but a Fedora user 
reported that this is a regression in 2.4.25, breaking a config like:

<VirtualHost>
   ProxyPass / foo
   <Location /bar>
     ProxyPass baz
   <Location>
</VirtualHost>

The requests to /bar no longer maps to baz. I added a test case in 
r1780905.

Regards, Joe