On Sat, Feb 18, 2017 at 4:25 PM, Daniel Ruggeri <drugg...@apache.org> wrote:
> On 2017-02-15 09:07 (-0600), William A Rowe Jr <wr...@rowe-clan.net> wrote:
>> On Wed, Feb 15, 2017 at 9:02 AM, Sander Hoentjen <san...@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.
That should be close, but need to ensure c->master is initialized for
http as well
where there is no master/subordinate.
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?
