httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sander Hoentjen <san...@hoentjen.eu>
Subject Re: mod_remoteip and mod_http2 combined
Date Fri, 24 Feb 2017 17:05:20 GMT


On 02/20/2017 07:48 PM, William A Rowe Jr wrote:
> On Sat, Feb 18, 2017 at 4:25 PM, Daniel Ruggeri <druggeri@apache.org> wrote:
>> On 2017-02-15 09:07 (-0600), William A Rowe Jr <wrowe@rowe-clan.net> wrote:
>>> On Wed, Feb 15, 2017 at 9:02 AM, Sander Hoentjen <sander@hoentjen.eu> wrote:
>>>> mod_remote ip has:
>>>>     /* mod_proxy creates outgoing connections - we don't want those */
>>>>     if (!remoteip_is_server_port(c->local_addr->port)) {
>>>>         return DECLINED;
>>>>     }
>>>> I am guessing something similar is needed for h2 connections?
>>> I suspect that the mod_remoteip logic is wrong, that it should be guarding
>>> against any subordinate connections and examining only explicitly configured
>>> ports / origin IPs. the PROXY protocol is not part of the HTTP protocol and
>>> incompatible with it, so the trust list logic isn't directly compatible (this
is
>>> clearly explained in the PROXY pseudo-RFC.)
>>>
>> Hi, Bill. That is what the module is doing. The original authors wrote it to have
a list of virtual hosts it is explicitly enabled for and explicitly disabled for. I added
a third list for optional vhosts. In the pre_connection hook, it checks to see if the connection's
local_addr (which should normally be the server's IP) is explicitly configured to enable PROXY
handling. It then checks to see if the local port is a server port.
>>
>> Looking at the logs shared, 192.168.122.249:84 is the server IP:Port combo and is
also the local IP:Port from mod_h2. If h2 sets the master of this connection, then we could
skip the whole ordeal with this patch:
>>
>> Index: modules/metadata/mod_remoteip.c
>> ===================================================================
>> --- modules/metadata/mod_remoteip.c     (revision 1781701)
>> +++ modules/metadata/mod_remoteip.c     (working copy)
>> @@ -862,6 +862,10 @@
>>      remoteip_conn_config_t *conn_conf;
>>      int optional;
>>
>> +    if (c->master != NULL) {
>> +        return DECLINED;
>> +    }
>> +
>>      conf = ap_get_module_config(ap_server_conf->module_config,
>>                                  &remoteip_module);
>>
>> .. but I don't know if that potentially means we are looking at the wrong connection.
First I'll say that with the "Optional" mode it worked, just not with On
I just tried this patch and as far as I have tested this seems to work
fine in On mode, as well as in Optional. I do see some other issue, but
that is probably in my own code, I'll try to track that down later.
> That should be close, but need to ensure c->master is initialized for
> http as well
> where there is no master/subordinate.
I am not sure what this means, how should I test this?
>
> If the 'optional' (unwise) feature were removed, the decision to
> inject the filter before
> the http or h2 filter would be trivial, it would step out of the way
> after the first-pass
> (and perhaps not need to live on the filter stack at all - if we do a
> fixed read against
> the core filter - we hopefully know the number of bytes affected early
> and can then
> do a second read to complete the v1 vs v2 read?) --- all before we are
> in a multiple
> pipeline state.
>
> If we move to a conn_rec oriented one-shot nothing happens during the request
> phase at all.
>
> By looking at the protocol filter stack, we should be able to glean
> whether we are
> talking to the core filter, or an 'unexpected' non-network filter, right?
>
I myself have no use-case at the moment for the "Optional" mode, maybe
others do.

Mime
View raw message