Re: mod_remoteip and mod_http2 combined

[Daniel Ruggeri <DR...@primary.net>]

Agreed - as many times as I read the spec, I have no idea how I did not
see that security advisory.  It's flat-out damning to the idea of an
"optional" mode. I'll go ahead and rip out the optional processing and
will add your suggested idea of a list of subnets to disable parsing. I
hope to have a patch later this morning to share. As awful as the name
is, I'm thinking RemoteIPProxyProtocolDisableNetworks ARG1 ARG2 ARG3.

-- 
Daniel Ruggeri

On 3/29/2017 4:43 PM, William A Rowe Jr wrote:
> This is the gist of my remaining objections.
>
> It would be nice if the mod_remoteip patch to PROXY protocol followed the
> security advisories of the PROXY draft security comments, and we rip out the
> 'optional' mode. The remaining objection is around the ambiguity of 'optional'
> (which can't exist) and the objection that how PROXY works as an implicit
> trust model using mod_remoteip is laughable, since the connection cannot
> be established without some PROXY protocol line interceptor yanking the
> garbage out of otherwise well-formed HTTP/1.1 - HTTP-TLS - h2c - h2 input.
>
> There is no 'untrusted PROXY header input' because that isn't part of the
> HTTP protocol and that garbage generates a 400 without an interceptor.
> No problem declaring that if we are willing to decode it, we will accept the
> input as gospel.

Re: mod_remoteip and mod_http2 combined

[Stefan Eissing <st...@greenbytes.de>]

I can see that a flat directive namespace has its drawbacks... ;-)

> Am 01.04.2017 um 19:12 schrieb Daniel Ruggeri <DR...@primary.net>:
> 
> 
> On 4/1/2017 11:18 AM, Yann Ylavic wrote:
>> Hi Daniel,
>> 
>> On Sat, Apr 1, 2017 at 3:56 PM, Daniel Ruggeri <DR...@primary.net> wrote:
>>> I went with the directive name
>>> RemoteIPProxyProtocolDisableHosts to align more with the fact that a
>>> single host or range can be disabled.
>> How about RemoteIPProxyProtocolExceptions since one can configure
>> either an IP or a network?
>> Host looks a bit ambiguous to me, especially with HTTP...
>> 
>> 
>> Regards,
>> Yann.
> 
> Oh, yeah, I like that even better. I'll stage this up and will commit in
> a day or two unless we have some better suggestions.
> 
> -- 
> Daniel Ruggeri
> 

Re: mod_remoteip and mod_http2 combined

[Daniel Ruggeri <DR...@primary.net>]

On 4/1/2017 11:18 AM, Yann Ylavic wrote:
> Hi Daniel,
>
> On Sat, Apr 1, 2017 at 3:56 PM, Daniel Ruggeri <DR...@primary.net> wrote:
>> I went with the directive name
>> RemoteIPProxyProtocolDisableHosts to align more with the fact that a
>> single host or range can be disabled.
> How about RemoteIPProxyProtocolExceptions since one can configure
> either an IP or a network?
> Host looks a bit ambiguous to me, especially with HTTP...
>
>
> Regards,
> Yann.

Oh, yeah, I like that even better. I'll stage this up and will commit in
a day or two unless we have some better suggestions.

-- 
Daniel Ruggeri

Re: mod_remoteip and mod_http2 combined

[Yann Ylavic <yl...@gmail.com>]

Hi Daniel,

On Sat, Apr 1, 2017 at 3:56 PM, Daniel Ruggeri <DR...@primary.net> wrote:
> I went with the directive name
> RemoteIPProxyProtocolDisableHosts to align more with the fact that a
> single host or range can be disabled.

How about RemoteIPProxyProtocolExceptions since one can configure
either an IP or a network?
Host looks a bit ambiguous to me, especially with HTTP...


Regards,
Yann.

Re: mod_remoteip and mod_http2 combined

[Daniel Ruggeri <DR...@primary.net>]

Sorry for the top post. I've committed r1789800 which pulls out Optional
handling and adds the ability to disable based on source network. This
is more or less the code as it was donated, plus some cleanup and the
small addition to disable based on networks (overall a cleaner approach
anyway). I went with the directive name
RemoteIPProxyProtocolDisableHosts to align more with the fact that a
single host or range can be disabled. I've verified this works via
haproxy, rejects when hit directly and disables processing when coming
from a permitted network.

I'm in the process of updating the backport proposal. To be safe, I'm
removing @jim's vote given how many times the code has changed since he
reviewed and will put it back in the "active" section of STATUS.

-- 
Daniel Ruggeri

On 4/1/2017 8:17 AM, Daniel Ruggeri wrote:
> Agreed - as many times as I read the spec, I have no idea how I did not
> see that security advisory.  It's flat-out damning to the idea of an
> "optional" mode. I'll go ahead and rip out the optional processing and
> will add your suggested idea of a list of subnets to disable parsing. I
> hope to have a patch later this morning to share. As awful as the name
> is, I'm thinking RemoteIPProxyProtocolDisableNetworks ARG1 ARG2 ARG3.
>