Re: An ask for eyes on proposal

[Sander Hoentjen]

On 06/09/2017 03:29 PM, William A Rowe Jr wrote:
> On Fri, Jun 9, 2017 at 4:17 AM, Sander Hoentjen <san...@hoentjen.eu> wrote:
>> On 06/08/2017 07:30 PM, Daniel Ruggeri wrote:
>>> Hi, all;
>>> With the proposal to T set for Monday, I wanted to draw attention to
>>> the PROXY protocol proposal in STATUS. Just hoping for a quick review.
>>> I know it appears to be a large change, but as I worked through the
>>> feedback, ten of the commits effectively got coded out. What we are
>>> left with is essentially just the donated code + safety around IPv6 +
>>> the ability to designate subnets that do not get PROXY processing.
>> [...] I still believe it would be better to specify enabling
>> Proxy Protocol on a server, not vhost level. Because well, once you
>> enable it in one vhost it gets enabled for all vhosts using that port/ip
>> combination.
>>
>> Here is what I said before about it:
>>
>> Right now the patch proposes RemoteIPProxyProtocol inside a vhost config, 
>> but wouldn't it be better (since it is connection-specific) to have 
>> something like a ProxyProtocolListen directive? Where you say instead of:
>> --
>> 
>> RemoteIPProxyProtocol On
>> 
>> --
>> Something like:
>> --
>> ProxyProtocolListen 127.0.0.1:9001
>> or
>> ProxyProtocolEnable 127.0.0.1:9001
>> --
>>
>> IMHO this is much cleaner than within a vhost (because that has side-effects 
>> on other vhosts as well)
> As this lives in mod_remoteip (for better or worse) let's look at what
> context mod_remoteip is configured in; we set up a list of those local
> or global *client* IP's which we trust to provide legit x-f-f (or remote-ip
> or otherly named) true IP address header fields.
>
> in the PROXY protocol case, we configure which *client* IP's which
> we *require* to submit a PROXY protocol line. Right now, we do that
> as a RemoteIPProxyProtocolExceptions list of those which we do *not*
> allow to submit a PROXY protocol line. I proposed we make the config
> simpler, in theory, by listing those we will trust.
>
> To your example, the *global* config line;
>
> RemoteIPProxyProtocol 127.0.0.1  [or 127.0.0.0/24]
>
> would configure all locally routed *client* requests, irrespective of
> which by-IP vhost, to require the PROXY protocol line. Requests
> from other hosts would not process the globally routed request for a PROXY 
> line.
Now I'm not sure if I understand you. I agree with you that a whitelist
is better than a blacklist, so adding

`RemoteIPProxyProtocol` as a list of *clients* that are allowed to use the 
*server/port* that is specified in `ProxyProtocolEnable` would make sense to me.

>
> I think that's sufficient. But if we wanted to implement your basic
> idea, we would still have the complication that we need to infer
> whether 9001 is a http, https, or h2 listener following the PROXY
> line processing. Your proposed syntax didn't really touch on that.
No because it doesn't need to. Because proxy protocol is prepended all
that needs to be done is strip it, and store the information for later
use. So you just configure that as normal.
>
> It is still possible to override behavior by-vhost (ip-based, we are
> unprepared to read the TLS SNI or Host header at that point)
> but I don't see any application to do so. A given client is either
> an haproxy or similar, or it is not.
>
Well yes, I guess my main point is that the behaviour is set for a
server_ip:port combination, so it doesn't work for name-based virtual
hosts. That led me to the

ProxyProtocolEnable idea. 

Re: An ask for eyes on proposal

[Sander Hoentjen]

On 06/08/2017 07:30 PM, Daniel Ruggeri wrote:
> Hi, all;
> With the proposal to T set for Monday, I wanted to draw attention to
> the PROXY protocol proposal in STATUS. Just hoping for a quick review.
> I know it appears to be a large change, but as I worked through the
> feedback, ten of the commits effectively got coded out. What we are
> left with is essentially just the donated code + safety around IPv6 +
> the ability to designate subnets that do not get PROXY processing.
>
> This code has been around a while and I think it would be nice if we
> could incorporate the donated code in the first release since being
> donated.
>
Hi,

While I know I don't have much say in this, since I never really
contributed much I still believe it would be better to specify enabling
Proxy Protocol on a server, not vhost level. Because well, once you
enable it in one vhost it gets enabled for all vhosts using that port/ip
combination.

Here is what I said before about it:

Right now the patch proposes RemoteIPProxyProtocol inside a vhost config, but 
wouldn't it be better (since it is connection-specific) to have something like 
a ProxyProtocolListen directive? Where you say instead of:
--

RemoteIPProxyProtocol On

--
Something like:
--
ProxyProtocolListen 127.0.0.1:9001
or
ProxyProtocolEnable 127.0.0.1:9001
--

IMHO this is much cleaner than within a vhost (because that has side-effects on 
other vhosts as well)

What do you guys think?

Regards,
Sander

Upgrade header

[Sander Hoentjen]

Hi Stefan (and others),

Right now when I enable h2 and/or h2c Apache will respond with headers
"Upgrade: h2,h2c" and Connection: Upgrade
As I understand it this is the wrong way around. Only the client should
send the Upgrade headers, and only then should the server respond with a
101 with the Upgrade header only specifying the proto that is being
switched to.
Is this a bug, or does Apache respond intentionally with the Upgrade
header? If intentional, then what is the reason?

Regards,
Sander

Re: mod_remoteip and mod_http2 combined

[Sander Hoentjen]

On 03/16/2017 10:34 AM, Sander Hoentjen wrote:
>
> On 03/11/2017 07:57 PM, Daniel Ruggeri wrote:
>> Thanks, all, for the patience as I finally got back to this.
>>
>> On 2/24/2017 11:05 AM, Sander Hoentjen wrote:
>>> On 02/20/2017 07:48 PM, William A Rowe Jr wrote:
>>>> 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,
>>>>>  _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.
>> This is good news and about what I was expecting to happen. I will add
>> this to the commit I've got coming that incorporates a lot of Ruediger's
>> feedback.
>>
>>>> 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?
>> Hi, Bill - also hoping for a bit more input. Since PROXY protocol is not
>> tied to any particular layer 7 protocol, I don't think we'd have to
>> verify it is initialized for HTTP - just that there is no master at all.
>> At least, that's my understanding so I appreciate any corrections.
> Here are my changes by the way:
> https://github.com/AntagonistHQ/httpd/commit/2d208793b4494e73289477c231c79be9e0030a2b
>> Sure, to clarify, the Optional use case came from a member on one of our
>> cousin projects (Tomcat) Chris Schultz as well as my own use cases. It
>> is useful for internally accessing the site from behind the
>&g

Re: mod_http2 file uploads slow/broken?

[Sander Hoentjen]

Hi Stefan,

With 1.9.3 file upload seems to complete every time just fine, so looks
like graceful restarts were indeed the issue. Thanks!
As to the speed, yes it needs to be improved but there is also some
weird thing:
For my testfile of about 14M,
uploads over http take about 20s
uploads over https take about 15s
(with http1.1 both take about 5s)
In my setup SSL is terminated by HAProxy in TCP mode, so Apache gets
non-SSL in both case, only in the SSL case prepended py proxy-protocol info.

Regards,
Sander

On 03/24/2017 08:14 PM, Stefan Eissing wrote:
> Hi Sander,
>
> from the logs, I think you are running into PR 59348 which was fixed in 
> v1.8.11.
>
> The bug was that during graceful restart, ongoing streams were not finished. 
> This restart could also happen, I think, if you have configured a max number 
> of connections or requests per process, for example with 
> MaxConnectionsPerChild. If that is the case, you can set this to 0 and see if 
> the upload still fails.
>
> More interesting would be to know if the current github version has the 
> problem still. With the fix, I hope it finishes the ongoing h2 requests 
> before the process exits.
>
> As to the speed: it is as I described earlier, the data is written in too 
> small chunks to the request processing thread. That needs to be improved for 
> sure.
>
> Thanks for testing, awaiting your results. If you need help with building the 
> github version, please let me know.
>
> Cheers,
>
> Stefan
>
>> Am 24.03.2017 um 16:56 schrieb Sander Hoentjen <san...@hoentjen.eu>:
>>
>> Hi Stefan,
>>
>> So far I can't reproduce the breaking of the upload on a testing
>> machine. I send you logs from a production machine off-list. In the
>> meanwhile I will try to build mod_http2 from
>> https://github.com/icing/mod_h2.
>>
>> Thanks for your help!
>>
>> Sander
>>
>>
>>
>> On 03/24/2017 03:38 PM, Stefan Eissing wrote:
>>> Hi Sander,
>>>
>>> the uploads sometimes break is new to me and I sure would like
>>> to find out what is going wrong in your setup. One obvious reason
>>> for the php script failing to read its input is a timeout or
>>> abort of the main connection. mod_http2 uses the general Timeout
>>> settings for its requests as well. If you can reproduce this,
>>> a log with "LogLevel http2:trace1" or even trace2 would help.
>>>
>>> I know that input streaming is not very optimal right now. Each
>>> arriving DATA frame is send directly onward to the request thread
>>> and that should be buffered and flushed at proper times. Again,
>>> a log at trace1/2 level should show exactly what is going on.
>>>
>>> If you can, I'd be interested to hear how the current version,
>>> available at https://github.com/icing/mod_h2 fares in you 2.4.25
>>> server. If you are on Windows, apachelounge has also builds with
>>> the latest version packaged in.
>>>
>>> Cheers,
>>>
>>> Stefan
>>>
>>>> Am 24.03.2017 um 14:10 schrieb Sander Hoentjen <san...@hoentjen.eu>:
>>>>
>>>> Hi,
>>>>
>>>> I am running Apache 2.4.25 with mod_http2, and I notice that sometimes
>>>> file uploads are broken.
>>>>
>>>> Receiving end is a php script, and it logs something like:
>>>> Internal error on sending request(POST /upload/upload.php HTTP/2.0);
>>>> uri(/upload/upload.php) content-length(931728): SendRequest: prepare():
>>>> user_get_body(bodyLocalBuf, 36865): read from client failed
>>>>
>>>> With HTTP/1.1 this problem does not occur. With HTTP/2 this problems
>>>> occurs sometimes, but not always. What I do notice though is that
>>>> uploads via HTTP/2 are *much* slower, about 20-30s on HTTP/2 vs 4-5s on
>>>> HTTP/1.1 for the exact same file (about 15MB)
>>>>
>>>> Is this a known issue? If not, anything I can do to help?
>>>>
>>>> Regards,
>>>> Sander
>>> Stefan Eissing
>>>
>>> bytes GmbH
>>> Hafenstrasse 16
>>> 48155 Münster
>>> www.greenbytes.de
>>>
> Stefan Eissing
>
> bytes GmbH
> Hafenstrasse 16
> 48155 Münster
> www.greenbytes.de
>

Re: mod_http2 file uploads slow/broken?

[Sander Hoentjen]

Hi Stefan,

So far I can't reproduce the breaking of the upload on a testing
machine. I send you logs from a production machine off-list. In the
meanwhile I will try to build mod_http2 from
https://github.com/icing/mod_h2.

Thanks for your help!

Sander



On 03/24/2017 03:38 PM, Stefan Eissing wrote:
> Hi Sander,
>
> the uploads sometimes break is new to me and I sure would like
> to find out what is going wrong in your setup. One obvious reason
> for the php script failing to read its input is a timeout or
> abort of the main connection. mod_http2 uses the general Timeout
> settings for its requests as well. If you can reproduce this,
> a log with "LogLevel http2:trace1" or even trace2 would help.
>
> I know that input streaming is not very optimal right now. Each
> arriving DATA frame is send directly onward to the request thread
> and that should be buffered and flushed at proper times. Again,
> a log at trace1/2 level should show exactly what is going on.
>
> If you can, I'd be interested to hear how the current version,
> available at https://github.com/icing/mod_h2 fares in you 2.4.25
> server. If you are on Windows, apachelounge has also builds with
> the latest version packaged in.
>
> Cheers,
>
> Stefan
>
>> Am 24.03.2017 um 14:10 schrieb Sander Hoentjen <san...@hoentjen.eu>:
>>
>> Hi,
>>
>> I am running Apache 2.4.25 with mod_http2, and I notice that sometimes
>> file uploads are broken.
>>
>> Receiving end is a php script, and it logs something like:
>> Internal error on sending request(POST /upload/upload.php HTTP/2.0);
>> uri(/upload/upload.php) content-length(931728): SendRequest: prepare():
>> user_get_body(bodyLocalBuf, 36865): read from client failed
>>
>> With HTTP/1.1 this problem does not occur. With HTTP/2 this problems
>> occurs sometimes, but not always. What I do notice though is that
>> uploads via HTTP/2 are *much* slower, about 20-30s on HTTP/2 vs 4-5s on
>> HTTP/1.1 for the exact same file (about 15MB)
>>
>> Is this a known issue? If not, anything I can do to help?
>>
>> Regards,
>> Sander
> Stefan Eissing
>
> bytes GmbH
> Hafenstrasse 16
> 48155 Münster
> www.greenbytes.de
>

mod_http2 file uploads slow/broken?

[Sander Hoentjen]

Hi,

I am running Apache 2.4.25 with mod_http2, and I notice that sometimes
file uploads are broken.

Receiving end is a php script, and it logs something like:
 Internal error on sending request(POST /upload/upload.php HTTP/2.0);
uri(/upload/upload.php) content-length(931728): SendRequest: prepare():
user_get_body(bodyLocalBuf, 36865): read from client failed

With HTTP/1.1 this problem does not occur. With HTTP/2 this problems
occurs sometimes, but not always. What I do notice though is that
uploads via HTTP/2 are *much* slower, about 20-30s on HTTP/2 vs 4-5s on
HTTP/1.1 for the exact same file (about 15MB)

Is this a known issue? If not, anything I can do to help?

Regards,
Sander

Re: mod_remoteip and mod_http2 combined

[Sander Hoentjen]

On 03/11/2017 07:57 PM, Daniel Ruggeri wrote:
> Thanks, all, for the patience as I finally got back to this.
>
> On 2/24/2017 11:05 AM, Sander Hoentjen wrote:
>> On 02/20/2017 07:48 PM, William A Rowe Jr wrote:
>>> 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,
>>>>  _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.
> This is good news and about what I was expecting to happen. I will add
> this to the commit I've got coming that incorporates a lot of Ruediger's
> feedback.
>
>>> 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?
> Hi, Bill - also hoping for a bit more input. Since PROXY protocol is not
> tied to any particular layer 7 protocol, I don't think we'd have to
> verify it is initialized for HTTP - just that there is no master at all.
> At least, that's my understanding so I appreciate any corrections.
Here are my changes by the way:
https://github.com/AntagonistHQ/httpd/commit/2d208793b4494e73289477c231c79be9e0030a2b
> Sure, to clarify, the Optional use case came from a member on one of our
> cousin projects (Tomcat) Chris Schultz as well as my own use cases. It
> is useful for internally accessing the site from behind the
> loadbalancer. When there is a publicly addressed upstream loadbalancer
> (Amazon ELB or just HAProxy itself) talking to RFC1918 addressed or
> non-routeable backend httpd servers, it becomes impossible to enable
> internal communication on the RFC1918 space to the backend instances.
> If the goal is to monitor or probe the site and (httpd proxy) backends
> internally for health, this *can* be done 

Re: mod_remoteip and mod_http2 combined

[Sander Hoentjen]

On 02/20/2017 07:48 PM, William A Rowe Jr wrote:
> 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,
>>  _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.

Re: mod_remoteip and mod_http2 combined

[Sander Hoentjen]

I am away on holiday until the 25th, will try when i get back.

On Sat Feb 18 23:25:51 2017 GMT+0100, Daniel Ruggeri 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,
>  _module);
> 
> .. but I don't know if that potentially means we are looking at the wrong 
> connection.
> 
> Sander, would it be possible to try this out?
> 
>

-- 
Sent from my Jolla

Re: mod_remoteip and mod_http2 combined

[Sander Hoentjen]

On 02/15/2017 12:19 PM, Jordan Gigov wrote:
> On 15 February 2017 at 12:50, Sander Hoentjen <san...@hoentjen.eu> wrote:
>> Hey guys,
>>
>> I am trying to use both mod_remoteip with ProxyProtocol and mod_http2.
>> It looks like mod_http2 gets handed the connection before mod_remoteip,
>> so things don't work as they should. ProxyProtocol should always be
>> handled first, since it is prepended to the stream. Any pointers to
>> where in the code I can look to change things around?
>>
>> --
>> Sander
> Try modules/http2/h2_h2.c line 550 add "mod_remoteip.c" after "mod_ssl.c".
>
> This reminds me since remoteip is being updated, maybe it should also
> specify some before and after mods to avoid.
It seems I was wrong, the issue is that for some reason
remoteip_hook_pre_connection is called twice for a h2 connection:

[Wed Feb 15 14:32:08.048968 2017] [remoteip:debug] [pid 8521]
mod_remoteip.c(1015): [client 192.168.122.1:35136] AH03503:
RemoteIPProxyProtocol: enabled on connection to 192.168.122.249:84
[Wed Feb 15 14:32:08.049082 2017] [remoteip:debug] [pid 8521]
mod_remoteip.c(1406): [client 192.168.122.1:35136] AH03511:
RemoteIPProxyProtocol: received valid PROXY header: 192.168.122.1:35136
[Wed Feb 15 14:32:08.049554 2017] [http2:debug] [pid 8521]
h2_session.c(994): [client 192.168.122.1:35136] AH03200: h2_session(4)
created, max_streams=100, stream_mem=32768, workers_limit=6,
workers_max=1, push_diary(type=1,N=256)
[Wed Feb 15 14:32:08.049579 2017] [http2:debug] [pid 8521]
h2_session.c(1088): [client 192.168.122.1:35136] AH03201: h2_session(4):
start, INITIAL_WINDOW_SIZE=65535, MAX_CONCURRENT_STREAMS=100
[Wed Feb 15 14:32:08.049589 2017] [http2:debug] [pid 8521]
h2_session.c(2067): [client 192.168.122.1:35136] AH03079: h2_session(4):
started on localhost.localdomain:84
[Wed Feb 15 14:32:08.049596 2017] [http2:debug] [pid 8521]
h2_session.c(1721): [client 192.168.122.1:35136] AH03078: h2_session(4):
transit [INIT] -- init --> [BUSY]
[Wed Feb 15 14:32:08.049616 2017] [http2:debug] [pid 8521]
h2_session.c(441): [client 192.168.122.1:35136] AH03066: h2_session(4):
recv FRAME[SETTINGS[length=0, stream=0]], frames=0/0 (r/s)
[Wed Feb 15 14:32:08.049628 2017] [http2:debug] [pid 8521]
h2_stream.c(189): [client 192.168.122.1:35136] AH03082: h2_stream(4-1):
opened
[Wed Feb 15 14:32:08.049655 2017] [http2:debug] [pid 8521]
h2_session.c(441): [client 192.168.122.1:35136] AH03066: h2_session(4):
recv FRAME[HEADERS[length=31, hend=1, stream=1, eos=1]], frames=1/0 (r/s)
[Wed Feb 15 14:32:08.049706 2017] [http2:debug] [pid 8521]
h2_session.c(677): [client 192.168.122.1:35136] AH03068: h2_session(4):
sent FRAME[SETTINGS[length=6, stream=0]], frames=2/0 (r/s)
[Wed Feb 15 14:32:08.049768 2017] [remoteip:debug] [pid 8521]
mod_remoteip.c(1015): [client 192.168.122.1:35136] AH03503:
RemoteIPProxyProtocol: enabled on connection to 192.168.122.249:84
[Wed Feb 15 14:32:08.049810 2017] [remoteip:warn] [pid 8521] [client
192.168.122.1:35136] AH03496: RemoteIPProxyProtocol data is missing, but
required! Aborting request.
[Wed Feb 15 14:32:08.049840 2017] [http2:debug] [pid 8521]
h2_request.c(271): [client 192.168.122.1:35136] AH03367: h2_request:
access_status=400, request_create failed


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?

-- 
Sander

mod_remoteip and mod_http2 combined

[Sander Hoentjen]

Hey guys,

I am trying to use both mod_remoteip with ProxyProtocol and mod_http2.
It looks like mod_http2 gets handed the connection before mod_remoteip,
so things don't work as they should. ProxyProtocol should always be
handled first, since it is prepended to the stream. Any pointers to
where in the code I can look to change things around?

-- 
Sander

Re: mood_remoteip ProxyProtocol addition

[Sander Hoentjen]

On 02/08/2017 01:00 AM, Reindl Harald wrote:
>
>
> Am 08.02.2017 um 00:44 schrieb Yann Ylavic:
>> On Wed, Feb 8, 2017 at 12:25 AM, Yann Ylavic 
>> wrote:
>>> On Wed, Feb 8, 2017 at 12:01 AM, Reindl Harald
>>>  wrote:

 how can you trust as a php application developer that
 "X-Forwarded-Proto" is
 trustable and not from the enduser client at all - for REMOTE_ADDR
 you don't
 consider "X-Forwarded-For" exactly for that reason
>>>
>>> I'm not proposing to use or trust "X-Forwarded-Proto" directly, but
>>> that mod_remoteip [either directly or provides the (optional)
>>> functions for ap_add_{common,cgi}_vars() to] set REMOTE_HTTPS=on
>>> and/or REMOTE_SCHEME=https accordingly.
>>> Just like REMOTE_ADDR.
>>>
>>> But not change HTTPS and/or REQUEST_SCHEME (but more importantly their
>>> sources/hooks as accessed and read by core/modules), like (IIUC)
>>> proposed by the patches.
>>> These are local informations.
>>
>> Actually, I'm not really opposed to set HTTPS=on (according to
>> mod_remoteip) in the environment *given to the script/CGI* only, if
>> that's the trigger for it to do the desired thing, this won't be used
>> by httpd internally at least.
>>
>> What's proposed so far is much more than that (if I read the patches
>> correctly)
>
> ok, so finally we agree :-)
>
> i am only interested in a centralized way to get rid of hacks like
> below in each and every application where mod_remoteip solves the
> similar problem with $_SERVER['REMOTAE_ADDR'] for cgi/mod_php
>
> $_SERVER['REQUEST_SCHEME'] because you typically build a full
> qualifiied URL for a link in emails with $_SERVER['REQUEST_SCHEME'] .
> '//' . $_SERVER['SERVER_NAME'] . $_SERVER['REQUEST_URI'] . '?param=x'
>
> in my own application the hack below was simple - in case of other
> software like Magento and so on you have to hack "index.php" while at
> the same time you should not touch the application code to keep it
> easily updateable
>
> if(!empty($config['cms_tls_offload']))
> {
>  $_SERVER['SERVER_PORT']= '443';
>  $_SERVER['REQUEST_SCHEME'] = 'https';
>  $_SERVER['HTTPS']  = 'on';
> }
>
As the OP, first let me say sorry for the typo in the subject.
Now, I must say that I agree too about anything that has been said in
this thread so far.
There should be a standard way for applications to check information.
Unfortunately, historically the way applications(1) deal with it varies
a bit. In a .htaccess the things most seen are:

RewriteCond %{HTTPS} !=on
RewriteCond %{SERVER_PORT} ^443$ [OR]
Header set Strict-Transport-Security "max-age=31536000" env=HTTPS

Basically, if those stop working I am screwed.

Then, on to PHP:
I think most of them check the HTTPS env var ($_SERVER['HTTPS']) but
some check REQUEST_SCHEME or SERVER_PORT. Probably some users are even
more creative.

Of course, there are also those who check things like
HTTP_X_FORWARDED_PROTO in addition.

Because we have lots of customers, it would be virtually impossible to
"fix" all their applications.

Onto the subject of security:
As the system administrator of those servers, I could choose to do SSL
ofloading, non-encrypted to a proxy, then that proxy uses an Apache SSL
backend. In this case the connection is as (in)secure as the case where
the SSL-offloader talks directly to Apache. But, applications would
continue to work. On the matter of PHP applications I would like to add
that PHP-FPM also does not have encryption. From an end-user security
POV there would be (IMHO) no difference between SSL-offloader -> Apache
-> PHP-FPM and Apache -> PHP-FPM.

The thing I am trying to say here is that for the most part, it is up to
the system administrator to configure things securely. It would help if
Apache allows the sysadmin to choose. Of course it is important to think
really hard about what is allowed and what not, so I am glad this
discussion is taking place.


(1) I work at a hosting provider, so I am talking mostly about PHP
"applications"

-- 
Sander

mood_remoteip ProxyProtocol addition

[Sander Hoentjen]

Hi guys,

I am trying to have haproxy added in front of our Apache servers, for
SSL termination. This is not hard to do, and especially with the recent
addition of ProxyProtocol support to mod_remoteip it works almost as we
need it.
Unfortunately we have a lot of users that use things like:
RewriteCond %{HTTPS} !on
in their .htaccess, and stuff like:
if $_SERVER['HTTPS']
in their PHP code.

Long story short, I need httpd to act like SSL is on, if the haproxy
instance says it is.
Fortunately the proxy protocol supports this, and haproxy can send it.
I have created a patch [1] that adds support for reading this in httpd,
but I have never before done something with the httpd source code. And,
as a matter of fact, have never really programmed in C before.

I am hoping you would be willing to have a look at it and tell me all
the things I am doing wrong. Eventually it would be awesome if the code
could be added to apache itself.

[1] https://github.com/AntagonistHQ/httpd/tree/remote-ssl

Kind regards,
Sander Hoentjen