Re: Scope of RemoteIPProxyProtocol* (was: svn commit: r1824211 - /httpd/httpd/branches/2.4.x/STATUS)
Not at all. This is physical host vs named based vhost. We got clever and eliminated the distinction after 2.0... which is catching up with us. Should we reintroduce a physical vhost? I don't have a simple answer, but we can at least keep repeating that the first vhost is the physical vhost and only certain directives have any effect there. On Feb 14, 2018 08:49, "Stefan Eissing" wrote: > I had a patch posted here on Monday that introduces > > >... > > > > > with a server_rec and configs between base server and a a set of > vhosts. Would that satisfy your needs for this? > > -Stefan > > > Am 14.02.2018 um 14:39 schrieb Graham Leggett : > > > > On 14 Feb 2018, at 3:05 PM, Yann Ylavic wrote: > > > >> It makes sense, and actually I missed some logic w.r.t. > >> enabled/disabled being a list of sockaddrs (based on servers' > >> server_addr_rec, and not a global boolean as I first thought...). This > >> is later compared to incoming connections local addr. > >> Let me grok that... but at first glance it looks quite cumbersome, why > >> not the usual server merging and a simple "enabled" boolean? > >> If we only care about the config of the base server, > >> "ap_get_module_config(c->base_server->module_config, > >> &remoteip_module)" could do it no? > >> For now it seems that all the vhost selection logic is duplicated, but > >> indeed it's not global (nor really per vhost, but yes this is thee > >> scope which comes closest). > > > > I think the problem distills down to our config lacking an explicit > configuration container for the IP address and port, something like this: > > > > # global > > > > # per port > > RemoteIPProxyProtocol on > > > > # per virtual host > > > > > > > > In the absence of a Bind directive (or equivalent) we’re left with this > undesirable config: > > > > # global, yuck > > RemoteIPProxyProtocol on > > > > # per virtual host > > > > > > So we’ve compromised and said we’ll accept this config: > > > > > > # per port > > RemoteIPProxyProtocol on > > # per virtual host > > > > > > To set RemoteIPProxyProtocol, we have to jump up one level to get > server_addr_rec, walk the list of matching IP addresses on virtual hosts > and setting (or not) as appropriate. > > > > Regards, > > Graham > > — > > > >
Re: Scope of RemoteIPProxyProtocol* (was: svn commit: r1824211 - /httpd/httpd/branches/2.4.x/STATUS)
That explanation is noxious. If enabled in the *default* initial matching physical ip:port host it applies to all related hosts If enabled in any secondary-non-default named vhost it is ignored. On Feb 14, 2018 06:28, "Graham Leggett" wrote: > On 14 Feb 2018, at 1:03 PM, Yann Ylavic wrote: > > The docs talk about connection based config, while ap_server_conf is > really the main server config. > The code should be improved to be based on c->baser_server config > (with merging of RemoteIPProxyProtocol*), unless I'm missing something > it seems (as of now) that the directives overwrite each other when > used in vhost context (not only for name-based vhosts). > So now (or post-backport) I think we should at least document the > scope as being "server config" only, and follow up with > "c->baser_server config" when possible (not a blocker for the first > version). > > > The docs explain the above here, which makes sense to me: > > While this directive may be specified in any virtual host, it is > important to understand that because the proxy protocol is connection > based and protocol agnostic, the enabling and disabling is actually > based > on ip-address and port. This means that if you have multiple name-based > virtual hosts for the same host and port, and you enable it any one of > them, then it is enabled for all them (with that host and port). It > also > means that if you attempt to enable the proxy protocol in one and > disable > in the other, that won't work; in such a case the last one wins and a > notice will be logged indicating which setting was being > overridden. > > Regards, > Graham > — > >
Re: Scope of RemoteIPProxyProtocol* (was: svn commit: r1824211 - /httpd/httpd/branches/2.4.x/STATUS)
I had a patch posted here on Monday that introduces ... with a server_rec and configs between base server and a a set of vhosts. Would that satisfy your needs for this? -Stefan > Am 14.02.2018 um 14:39 schrieb Graham Leggett : > > On 14 Feb 2018, at 3:05 PM, Yann Ylavic wrote: > >> It makes sense, and actually I missed some logic w.r.t. >> enabled/disabled being a list of sockaddrs (based on servers' >> server_addr_rec, and not a global boolean as I first thought...). This >> is later compared to incoming connections local addr. >> Let me grok that... but at first glance it looks quite cumbersome, why >> not the usual server merging and a simple "enabled" boolean? >> If we only care about the config of the base server, >> "ap_get_module_config(c->base_server->module_config, >> &remoteip_module)" could do it no? >> For now it seems that all the vhost selection logic is duplicated, but >> indeed it's not global (nor really per vhost, but yes this is thee >> scope which comes closest). > > I think the problem distills down to our config lacking an explicit > configuration container for the IP address and port, something like this: > > # global > > # per port > RemoteIPProxyProtocol on > > # per virtual host > > > > In the absence of a Bind directive (or equivalent) we’re left with this > undesirable config: > > # global, yuck > RemoteIPProxyProtocol on > > # per virtual host > > > So we’ve compromised and said we’ll accept this config: > > > # per port > RemoteIPProxyProtocol on > # per virtual host > > > To set RemoteIPProxyProtocol, we have to jump up one level to get > server_addr_rec, walk the list of matching IP addresses on virtual hosts and > setting (or not) as appropriate. > > Regards, > Graham > — >
Re: Scope of RemoteIPProxyProtocol* (was: svn commit: r1824211 - /httpd/httpd/branches/2.4.x/STATUS)
On 14 Feb 2018, at 3:05 PM, Yann Ylavic wrote: > It makes sense, and actually I missed some logic w.r.t. > enabled/disabled being a list of sockaddrs (based on servers' > server_addr_rec, and not a global boolean as I first thought...). This > is later compared to incoming connections local addr. > Let me grok that... but at first glance it looks quite cumbersome, why > not the usual server merging and a simple "enabled" boolean? > If we only care about the config of the base server, > "ap_get_module_config(c->base_server->module_config, > &remoteip_module)" could do it no? > For now it seems that all the vhost selection logic is duplicated, but > indeed it's not global (nor really per vhost, but yes this is thee > scope which comes closest). I think the problem distills down to our config lacking an explicit configuration container for the IP address and port, something like this: # global # per port RemoteIPProxyProtocol on # per virtual host In the absence of a Bind directive (or equivalent) we’re left with this undesirable config: # global, yuck RemoteIPProxyProtocol on # per virtual host So we’ve compromised and said we’ll accept this config: # per port RemoteIPProxyProtocol on # per virtual host To set RemoteIPProxyProtocol, we have to jump up one level to get server_addr_rec, walk the list of matching IP addresses on virtual hosts and setting (or not) as appropriate. Regards, Graham — smime.p7s Description: S/MIME cryptographic signature
Re: Scope of RemoteIPProxyProtocol* (was: svn commit: r1824211 - /httpd/httpd/branches/2.4.x/STATUS)
On Wed, Feb 14, 2018 at 1:28 PM, Graham Leggett wrote: > On 14 Feb 2018, at 1:03 PM, Yann Ylavic wrote: >> >> The docs talk about connection based config, while ap_server_conf is >> really the main server config. >> The code should be improved to be based on c->baser_server config >> (with merging of RemoteIPProxyProtocol*), unless I'm missing something >> it seems (as of now) that the directives overwrite each other when >> used in vhost context (not only for name-based vhosts). >> So now (or post-backport) I think we should at least document the >> scope as being "server config" only, and follow up with >> "c->baser_server config" when possible (not a blocker for the first >> version). > > The docs explain the above here, which makes sense to me: > > While this directive may be specified in any virtual host, it is > important to understand that because the proxy protocol is connection > based and protocol agnostic, the enabling and disabling is actually > based > on ip-address and port. This means that if you have multiple name-based > virtual hosts for the same host and port, and you enable it any one of > them, then it is enabled for all them (with that host and port). It also > means that if you attempt to enable the proxy protocol in one and > disable > in the other, that won't work; in such a case the last one wins and a > notice will be logged indicating which setting was being overridden. It makes sense, and actually I missed some logic w.r.t. enabled/disabled being a list of sockaddrs (based on servers' server_addr_rec, and not a global boolean as I first thought...). This is later compared to incoming connections local addr. Let me grok that... but at first glance it looks quite cumbersome, why not the usual server merging and a simple "enabled" boolean? If we only care about the config of the base server, "ap_get_module_config(c->base_server->module_config, &remoteip_module)" could do it no? For now it seems that all the vhost selection logic is duplicated, but indeed it's not global (nor really per vhost, but yes this is thee scope which comes closest). Regards, Yann.
Re: Scope of RemoteIPProxyProtocol* (was: svn commit: r1824211 - /httpd/httpd/branches/2.4.x/STATUS)
On 14 Feb 2018, at 1:03 PM, Yann Ylavic wrote: > The docs talk about connection based config, while ap_server_conf is > really the main server config. > The code should be improved to be based on c->baser_server config > (with merging of RemoteIPProxyProtocol*), unless I'm missing something > it seems (as of now) that the directives overwrite each other when > used in vhost context (not only for name-based vhosts). > So now (or post-backport) I think we should at least document the > scope as being "server config" only, and follow up with > "c->baser_server config" when possible (not a blocker for the first > version). The docs explain the above here, which makes sense to me: While this directive may be specified in any virtual host, it is important to understand that because the proxy protocol is connection based and protocol agnostic, the enabling and disabling is actually based on ip-address and port. This means that if you have multiple name-based virtual hosts for the same host and port, and you enable it any one of them, then it is enabled for all them (with that host and port). It also means that if you attempt to enable the proxy protocol in one and disable in the other, that won't work; in such a case the last one wins and a notice will be logged indicating which setting was being overridden. Regards, Graham — smime.p7s Description: S/MIME cryptographic signature
Scope of RemoteIPProxyProtocol* (was: svn commit: r1824211 - /httpd/httpd/branches/2.4.x/STATUS)
On Wed, Feb 14, 2018 at 11:21 AM, wrote: > >*) mod_remoteip: Add PROXY protocol support [] > ylavic: RemoteIPProxyProtocol* are documented as scoped to server config > and virtual host, though using ap_server_conf makes them global > only (thus less useful too...). > jim: Can docco patch be post-backport? > minfrin: The docs seem correct, and there is a long explanation in the > docs of >why the scoping is as it is. The docs talk about connection based config, while ap_server_conf is really the main server config. The code should be improved to be based on c->baser_server config (with merging of RemoteIPProxyProtocol*), unless I'm missing something it seems (as of now) that the directives overwrite each other when used in vhost context (not only for name-based vhosts). So now (or post-backport) I think we should at least document the scope as being "server config" only, and follow up with "c->baser_server config" when possible (not a blocker for the first version). Regards, Yann.
