ProxyPassReverseCookieDomain and non constant arguments
I'm trying to make ProxyPassReverseCookieDomain understand two things it
apparently does not understand at the moment: accept 1) balancer names and
2) variables as arguments. For the first problem I was able to come up with
a patch based on the code for the ProxyPassReverse directive (see below).
The second problem gives me headaches though. Suppose I would like to do
ProxyPassReverseCookieDomain balancer://foobar/ %{HTTP_HOST}
so the %{HTTP_HOST} expression is translated to the http host field of the
original client request. This is required in case the reverse proxy is
reachable through multiple domains because the cookie must be set to the
correct domain - else it will not be transmitted by the client's agent in
the future.To achieve this I would have to carry this information from the
client-proxy request to the proxy-server request and their responses.
Obviously, this 'linking' of requests is already done be mod_proxy since it
could not function as a reveres proxy otherwise.
I would be grateful for some hints on this.
My preliminary patch for the balancer issue is located below
diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 5ab5d91..5345246 100644
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -933,6 +933,29 @@ PROXY_DECLARE(const char *)
ap_proxy_location_reverse_map(
request_rec *r,
return url;
}
+/* Attempts to translate 'balancer_name' to a valid balancer, using the
'domain_name'
+ * to find the worker which matches to the domain. */
+PROXY_DECLARE(int) ap_proxy_balancer_matches_domain(request_rec *r, const
char* domain_name, const char *balancer_name)
+{
+ proxy_server_conf *sconf = (proxy_server_conf *)
ap_get_module_config(r-server-module_config, proxy_module);
+
+ if (ap_proxy_valid_balancer_name((char *)balancer_name, 0) == 0)
+return 0;
+
+ proxy_balancer *balancer = ap_proxy_get_balancer(r-pool, sconf,
balancer_name, 1);
+ if (balancer == NULL)
+return 0;
+
+ int n;
+ proxy_worker **worker = (proxy_worker **)balancer-workers-elts;
+ for (n = 0; n balancer-workers-nelts; n++) {
+if (strcasecmp((*worker)-s-name, domain_name) == 0)
+ return 1;
+ }
+
+ return 0;
+}
+
/*
* Cookies are a bit trickier to match: we've got two substrings to worry
* about, and we can't just find them with strstr 'cos of case. Regexp
@@ -1003,7 +1026,8 @@ PROXY_DECLARE(const char *)
ap_proxy_cookie_reverse_map(request_rec *r,
}
for (i = 0; i conf-cookie_domains-nelts; i++) {
l2 = strlen(ent[i].fake);
-if (l1 = l2 strncasecmp(ent[i].fake, domainp, l2) == 0) {
+if ((ap_proxy_balancer_matches_domain(r, domainp, ent[i].fake)
== 1) ||
+(l1 = l2 strncasecmp(ent[i].fake, domainp, l2) == 0)) {
newdomain = ent[i].real;
ddiff = strlen(newdomain) - l1;
break;
Re: ProxyPassReverseCookieDomain and non constant arguments
To make the directive more useful I suggest the attached patches. The first one will make the directive use balancer ids to look up matching worker names instead of just dumping the balancer name into the cookie - which is obviously never useful at all. The second one will default an empty second argument to the hostname of the client request so the cookies get reverse mapped to the correct domain, in case the client can reach the reverse proxy on multiple domains. 1_balancer_argument_for_reverse_cookie_domain.patch Description: Binary data 2_default_argument_cookie_domain.patch Description: Binary data
Re: ProxyPassReverseCookieDomain and non constant arguments
Suppose you have several balancers defined, each with multiple workers. Now
you want to rewrite all cookies from all servers, no matter the hostname
the client requested the content under.
This means that - at time of configuration - you don't know the domain of
the content server you want to specify the directive for (1st argument to
ProxyPassReverseCookieDomain). This *might* be solved by iterating over all
domains and writing the directive for every single entry but this is surely
not considered 'good' style and absolutely not something users with large
setups want to do.
The second problem is that - at time of configuration - you don't know the
hostname you have to rewrite the cookie to. It has to be the hostname the
client requested the content for, since only then will (or at least should)
it be transmitted by the client's agent in subsequent requests. This would
be solved by using %{HTTP_HOST} *if* that worked here. I expected it to but
after checking the code for rewriting the cookies I got the impression the
second argument to ProxyPassReverseCookieDomain would simply be copied
unchanged into the cookie without any previous
parsing/translation/substitution. I tried using %{HTTP_HOST} as second
argument - inside a VirtualHost - but it was not replaced by the
VirtualHost's ServerName or one of it's aliases and instead got printed
into the cookie as Domain=%{HTTP_HOST} (2.4.4 of course)
The idea behind the patches is to be able to specify something like
Proxy balancer://cd107d9706d71153bafd4ab15f1c6b5d
BalancerMember http://server1.local
BalancerMember http://server2.local
/Proxy
VirtualHost 10.10.10.10:80
ServerName reverseproxy.local
Location /
ProxyPass balancer://cd107d9706d71153bafd4ab15f1c6b5d/
lbmethod=bybusyness
ProxyPassReverse
balancer://cd107d9706d71153bafd4ab15f1c6b5d/
ProxyPassReverseCookieDomain
balancer://cd107d9706d71153bafd4ab15f1c6b5d/
/Location
/VirtualHost
With the suggested patches this is all it takes to rewrite all the cookies
to the correct domain names, independant of the number workers for each
balancer and the hostname of the client requests. I found no way to do that
with the existing code but I also feel like having missed something in
regards to using %{HTTP_HOST} - maybe some behind-the-scenes-magic I am
unaware of?
On Thu, Apr 4, 2013 at 10:43 AM, Nick Kew n...@webthing.com wrote:
On 3 Apr 2013, at 08:52, Thomas Eckert wrote:
I'm trying to make ProxyPassReverseCookieDomain understand two things it
apparently does not understand at the moment: accept 1) balancer names and
2) variables as arguments. For the first problem I was able to come up with
a patch based on the code for the ProxyPassReverse directive (see below).
The second problem gives me headaches though. Suppose I would like to do
ProxyPassReverseCookieDomain balancer://foobar/ %{HTTP_HOST}
Why would you do that? What backend application is setting cookies
with such a broken domain parameter as that?
so the %{HTTP_HOST} expression is translated to the http host field of
the original client request. This is
The HTTP_HOST will always match the VirtualHost in which the reverse
proxy
is configured. You can of course have as many virtualhosts as you want,
but I'm not convinced you need even that.
At a glance, your patch is unnecessary: you should get the same outcome
by just adding a ProxyPassReverseCookieDomain directive for each Host:
value you answer to. Am I missing something?
--
Nick Kew
Re: ProxyPassReverseCookieDomain and non constant arguments
Any news here ? I would like this to get included since it fixes what I
think is a real lack of usability - see my previous example as to the 'why'
and 'how'.
On Thu, Apr 4, 2013 at 11:34 AM, Thomas Eckert
thomas.r.w.eck...@gmail.comwrote:
Suppose you have several balancers defined, each with multiple workers.
Now you want to rewrite all cookies from all servers, no matter the
hostname the client requested the content under.
This means that - at time of configuration - you don't know the domain of
the content server you want to specify the directive for (1st argument to
ProxyPassReverseCookieDomain). This *might* be solved by iterating over all
domains and writing the directive for every single entry but this is surely
not considered 'good' style and absolutely not something users with large
setups want to do.
The second problem is that - at time of configuration - you don't know the
hostname you have to rewrite the cookie to. It has to be the hostname the
client requested the content for, since only then will (or at least should)
it be transmitted by the client's agent in subsequent requests. This would
be solved by using %{HTTP_HOST} *if* that worked here. I expected it to but
after checking the code for rewriting the cookies I got the impression the
second argument to ProxyPassReverseCookieDomain would simply be copied
unchanged into the cookie without any previous
parsing/translation/substitution. I tried using %{HTTP_HOST} as second
argument - inside a VirtualHost - but it was not replaced by the
VirtualHost's ServerName or one of it's aliases and instead got printed
into the cookie as Domain=%{HTTP_HOST} (2.4.4 of course)
The idea behind the patches is to be able to specify something like
Proxy balancer://cd107d9706d71153bafd4ab15f1c6b5d
BalancerMember http://server1.local
BalancerMember http://server2.local
/Proxy
VirtualHost 10.10.10.10:80
ServerName reverseproxy.local
Location /
ProxyPass balancer://cd107d9706d71153bafd4ab15f1c6b5d/
lbmethod=bybusyness
ProxyPassReverse
balancer://cd107d9706d71153bafd4ab15f1c6b5d/
ProxyPassReverseCookieDomain
balancer://cd107d9706d71153bafd4ab15f1c6b5d/
/Location
/VirtualHost
With the suggested patches this is all it takes to rewrite all the cookies
to the correct domain names, independant of the number workers for each
balancer and the hostname of the client requests. I found no way to do that
with the existing code but I also feel like having missed something in
regards to using %{HTTP_HOST} - maybe some behind-the-scenes-magic I am
unaware of?
On Thu, Apr 4, 2013 at 10:43 AM, Nick Kew n...@webthing.com wrote:
On 3 Apr 2013, at 08:52, Thomas Eckert wrote:
I'm trying to make ProxyPassReverseCookieDomain understand two things
it apparently does not understand at the moment: accept 1) balancer names
and 2) variables as arguments. For the first problem I was able to come up
with a patch based on the code for the ProxyPassReverse directive (see
below).
The second problem gives me headaches though. Suppose I would like to do
ProxyPassReverseCookieDomain balancer://foobar/ %{HTTP_HOST}
Why would you do that? What backend application is setting cookies
with such a broken domain parameter as that?
so the %{HTTP_HOST} expression is translated to the http host field of
the original client request. This is
The HTTP_HOST will always match the VirtualHost in which the reverse
proxy
is configured. You can of course have as many virtualhosts as you want,
but I'm not convinced you need even that.
At a glance, your patch is unnecessary: you should get the same outcome
by just adding a ProxyPassReverseCookieDomain directive for each Host:
value you answer to. Am I missing something?
--
Nick Kew
mod_proxy seg faulting ?
Lately, I've been seeing httpd/mod_proxy seg faulting in reverse proxy setups, frequency increasing. #0 apr_palloc (pool=0x8b52518, in_size=16) at memory/unix/apr_pools.c:684 #1 0xf756fc10 in apr_pool_cleanup_register (p=0x8b52518, data=0x8b52528, plain_cleanup_fn=0xf756edb0 thread_cond_cleanup, child_cleanup_fn=0x8069e20 apr_pool_cleanup_null@plt) at memory/unix/apr_pools.c:2218 #2 0xf756ef9c in apr_thread_cond_create (cond=0x8b524e0, pool=0x8b52390) at locks/unix/thread_cond.c:55 #3 0xf76fac64 in apr_reslist_create (reslist=0x8a6c488, min=0, smax=50, hmax=50, ttl=0, con=0xf72cb8d0 connection_constructor, de=0xf72cb890 connection_destructor, params=0x89a8268, pool=0x8b52390) at misc/apr_reslist.c:299 #4 0xf72cc0d8 in ap_proxy_initialize_worker (worker=0x89a8268, s=0x89b3a50, p=0x890b0a8) at proxy_util.c:1751 #5 0xf72c7d8f in proxy_handler (r=0x8b3e1f0) at mod_proxy.c:1011 #6 0x0808ba41 in ap_run_handler (r=0x8b3e1f0) at config.c:168 #7 0x0808f8e6 in ap_invoke_handler (r=0x8b3e1f0) at config.c:432 #8 0x080a200a in ap_process_async_request (r=0x8b3e1f0) at http_request.c:317 #9 0x080a213d in ap_process_request (r=0x8b3e1f0) at http_request.c:363 #10 0x0809ea60 in ap_process_http_sync_connection (c=optimized out) at http_core.c:190 #11 ap_process_http_connection (c=0x8b3a390) at http_core.c:231 #12 0x08095e51 in ap_run_process_connection (c=0x8b3a390) at connection.c:41 #13 0x080a9470 in process_socket (bucket_alloc=optimized out, my_thread_num=optimized out, my_child_num=optimized out, sock=optimized out, p=optimized out, thd=optimized out) at worker.c:620 #14 worker_thread (thd=0x8a6ddf8, dummy=0x8ac73f8) at worker.c:979 #15 0xf757cf96 in dummy_worker (opaque=0x8a6ddf8) at threadproc/unix/thread.c:142 #16 0xf7503809 in start_thread () from /lib/libpthread.so.0 #17 0xf746430e in clone () from /lib/libc.so.6 Backtrace stopped: Not enough registers or memory available to unwind further (gdb) frame 0 #0 apr_palloc (pool=0x8b52518, in_size=16) at memory/unix/apr_pools.c:684 684in memory/unix/apr_pools.c (gdb) info locals active = 0x0 node = optimized out mem = optimized out size = 16 free_index = optimized out This is awkward. Shall I assume httpd ran out of memory ? That's about the only plausible reason I could see why the apr would run into active=0x0. #0 0xf74aa2ed in pthread_mutex_lock () from /lib/libpthread.so.0 #1 0xf75140c0 in apr_thread_mutex_lock (mutex=0x0) at locks/unix/thread_mutex.c:92 #2 0xf769f702 in apr_reslist_maintain (reslist=0x9dc2da8) at misc/apr_reslist.c:184 #3 0xf769fc79 in apr_reslist_create (reslist=0x9dbe448, min=0, smax=50, hmax=50, ttl=0, con=0xf72708d0 connection_constructor, de=0xf7270890 connection_destructor, params=0x9cfa268, pool=0x9dc2cb0) at misc/apr_reslist.c:305 #4 0xf72710d8 in ap_proxy_initialize_worker (worker=0x9cfa268, s=0x9d05a50, p=0x9c5d0a8) at proxy_util.c:1751 #5 0xf726cd8f in proxy_handler (r=0xdc9004c0) at mod_proxy.c:1011 #6 0x0808ba41 in ap_run_handler (r=0xdc9004c0) at config.c:168 #7 0x0808f8e6 in ap_invoke_handler (r=0xdc9004c0) at config.c:432 #8 0x080a200a in ap_process_async_request (r=0xdc9004c0) at http_request.c:317 #9 0x080a213d in ap_process_request (r=0xdc9004c0) at http_request.c:363 #10 0x0809ea60 in ap_process_http_sync_connection (c=optimized out) at http_core.c:190 #11 ap_process_http_connection (c=0x9e5f940) at http_core.c:231 #12 0x08095e51 in ap_run_process_connection (c=0x9e5f940) at connection.c:41 #13 0x080a9470 in process_socket (bucket_alloc=optimized out, my_thread_num=optimized out, my_child_num=optimized out, sock=optimized out, p=optimized out, thd=optimized out) at worker.c:620 #14 worker_thread (thd=0x9dbfd98, dummy=0x9e19318) at worker.c:979 #15 0xf7521f96 in dummy_worker (opaque=0x9dbfd98) at threadproc/unix/thread.c:142 #16 0xf74a8809 in start_thread () from /lib/libpthread.so.0 #17 0xf740930e in clone () from /lib/libc.so.6 This is even stranger: it's trying to lock on a null pointer. I'm just starting to dig into this myself. Has anyone seen stuff like this (recently) ? I checked the bug reports but didn't see anything obvious.
Re: ProxyPassReverseCookieDomain and non constant arguments
I'm getting the impression I did not state why these patches improve the existing feature clearly enough, so I'll give it another shot. 1) The first patch addresses a scalability issue in mod_proxy. At present an admin has to configure one ProxyPassReverseCookieDomain for every single server he wants to reverse proxy. This does not scale well at all. Instead of forcing admins to pollute their configuration files with repetitive entries we can simply make use of the existing balancer names, which themselves contain the server addresses. The resulting configuration files look a lot better then without the patch and maintenance is not nearly as inconvenient. 2) The second patch adds a currently missing feature. That is to allow the configuration to reference a (specific) request-time value, just as many configuration directives for other (or even the same!) modules already do. Currently there is now way to have the directive adapt to each request based on the request's (destination) hostname, which is necessary if you want to rewrite the cookie to the _correct_ domain name. Remember, for the client, the reverse proxy might be reachable via different hostnames so the admin cannot know the value she has to put as the second argument. I added this little feature so the directive can be used in this situation as well. Otherwise an admin who wants to do this is out of luck. The second patch might become obsolete with the implementation of a global interpolation mechanism, as is discussed on the mailing list right now. But since I doubt this is going to happen any time soon, we should see this added until then. Combined, these two patches make the directive a lot easier to use and add a degree of usability which I find sorely lacking right now. Consider this configuration (which includes the suggested patches) : Proxy balancer://id1 BalancerMember http://server1.local BalancerMember http://server2.local BalancerMember http://server3.local http://server1.local BalancerMember http://server4.local http://server2.local /Proxy VirtualHost 10.10.10.10:80 ServerName reverseproxy.local Location / ProxyPass balancer://id1/ lbmethod=bybusyness ProxyPassReverse balancer://id1/ ProxyPassReverseCookieDomain balancer://id1/ /Location /VirtualHost The above configuration is clearly easier to read and maintain then the one below. And this is for 4 servers only. Just consider how this would look like for 10+ servers, probably even spread over several VirtualHost entries. Proxy balancer://id1 BalancerMember http://server1.local BalancerMember http://server2.local BalancerMember http://server3.local http://server1.local BalancerMember http://server4.local http://server2.local /Proxy VirtualHost 10.10.10.10:80 ServerName reverseproxy.local Location / ProxyPass balancer://id1/ lbmethod=bybusyness ProxyPassReverse balancer://id1/ ProxyPassReverseCookieDomain http://server1.localreverseproxy.local ProxyPassReverseCookieDomain http://server2.local http://server1.localreverseproxy.local ProxyPassReverseCookieDomain http://server3.local http://server1.localreverseproxy.local ProxyPassReverseCookieDomain http://server4.local http://server1.localreverseproxy.local /Location /VirtualHost Now, if you also want to give your reverse proxy a ServerAlias, one would do something like Proxy balancer://id1 BalancerMember http://server1.local BalancerMember http://server2.local BalancerMember http://server3.local http://server1.local BalancerMember http://server4.local http://server2.local /Proxy VirtualHost 10.10.10.10:80 ServerName reverseproxy.local ServerAlias other_name.local Location / ProxyPass balancer://id1/ lbmethod=bybusyness ProxyPassReverse balancer://id1/ ProxyPassReverseCookieDomain http://server1.localreverseproxy.local --- problem ! ProxyPassReverseCookieDomain http://server2.local http://server1.localreverseproxy.local --- problem ! ProxyPassReverseCookieDomain http://server3.local http://server1.localreverseproxy.local --- problem ! ProxyPassReverseCookieDomain http://server4.local http://server1.localreverseproxy.local --- problem ! /Location /VirtualHost The above marked lines will rewrite the cookies delivered to the client _incorrectly_ if the client reached the reverse proxy through other_name.local. With the second patch we can just omit the last argument to the directive (which really is redundant anyway) and have mod_proxy insert the correct value by itself. On Mon, Apr 22, 2013 at 10:33 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: Any news here ? I would like this to get
Re: mod_proxy seg faulting ?
Based on Stefan's reply I replaced mod_proxy's config pool with a sub-pool and wrapped a mutex around the pool usage. Basic testing went well but I have to do some more thorough parallel testing. One thing which had me confused was the balancers. In ap_proxy_define_balancer() they are handed a pointer to mod_proxy's config (via balancer-sconf). So I also had to check whether the pool is ever accessed via a balancer but to my surprise that pointer is never used at all. I suggest removing the balancers' reference to mod_proxy's configuration pool, since that makes managing that pool a lot eaiser in the future. It's really only 2 lines that have to be edited. On Sat, May 4, 2013 at 8:20 PM, Micha Lenk mi...@lenk.info wrote: Hi Stefan, Am 03.05.2013 14:09, schrieb Stefan Fritsch: On Thursday 02 May 2013, Thomas Eckert wrote: Lately, I've been seeing httpd/mod_proxy seg faulting in reverse proxy setups, frequency increasing. I am pretty sure that this is a thread-unsafe pool usage. create_proxy_config() puts the global config pool into (proxy_server_conf)-pool. It is later (during request processing) used all over the place without further locking. This must be a sub- pool instead, and it must be protected with a mutex. Or new sub-pools must be created wherever conf-pool is used. Can you please elaborate the problem of thread-unsafe pool usage a bit more so that we can better understand why this is causing a segmentation fault? Or alternatively, do you have any pointer to documentation, mailinglists, what ever, where to read more about it? Thanks in advance, Micha fix_mod_proxy_thread_unsafety.patch Description: Binary data remove_obselete_balancer_pointer.patch Description: Binary data
Re: mod_proxy seg faulting ?
However, looking at your patch, having to lock the mutex for ap_proxy_get_worker() looks wrong. I think it should be passed r-pool instead of conf-pool. I checked how ap_proxy_get_worker() is used in other places and also what is done with the pool inside and you are right. It really shouldn't be in a persistent pool. Corrected patch is attached. Hmm. I thought there was some introductory documentation, but I didn't find much. There is a short note at the end of http://apr.apache.org/docs/apr/1.4/apr__pools_8h.html . Basically, only the functions in http://apr.apache.org/docs/apr/1.4/group__apr__pools.html that are explicitly marked as thread safe may safely operate on a pool from different threads without locking. Which is basically saying only the create-sub-pool functions ;-) fix_mod_proxy_thread_unsafety.patch Description: Binary data
Improve mod_proxy's error marking of workers
Attached patch contains a directive to improve the error marking of workers. Basically, some errors will cause a worker to be marked as in error while others don't. I can't see a reason for this so I added a directive to have all errors mark the error correctly - especially useful for automated systems using mod_proxy to check if the system is healthy. improve_worker_error_marking.patch Description: Binary data
Re: Improve mod_proxy's error marking of workers
I think you need to screen out 4xx at least to prevent client errors from marking down your backends. Seems like those responses never reach that code. Neither 400 nor 401 or 403 had an effect on the worker's error state, it always remained in state 'OK'. I'd like to say it's not an issue but until I know why it behaves like this I would agree with you. Need to screen them out unless we can be 100% sure not to encounter those status codes at that point. Agreed... An all or nothing setting will likely create more trouble than not. In my opinion this should not even be configurable, at least not when other error related response codes lead to having workers set to state 'ERROR'. I only added this so it could be applied to existing setups without causing havoc. Ideally this should be part of the code just like the other worker state changes (some lines above my suggested change). I really don't see why some errors should cause the worker to display as such while others don't. This of course includes Eric's reply regarding 4xx codes which should not have the worker's state be changed. This patch addresses corner cases (e.g. DNS resolution failure) but in those cases mod_proxy should not just be silent about it. If such errors occur let the system know about so it can be fixed / worked around. It really does not matter whether this is done as directive via the suggested patch, via failonstatus or some other way as long as it's there. On Wed, May 8, 2013 at 1:18 AM, Daniel Ruggeri drugg...@primary.net wrote: On 5/7/2013 2:00 PM, Jim Jagielski wrote: Agreed... An all or nothing setting will likely create more trouble than not. On May 7, 2013, at 8:08 AM, Eric Covener cove...@gmail.com wrote: On Tue, May 7, 2013 at 6:24 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: Attached patch contains a directive to improve the error marking of workers. Basically, some errors will cause a worker to be marked as in error while others don't. I can't see a reason for this so I added a directive to have all errors mark the error correctly - especially useful for automated systems using mod_proxy to check if the system is healthy. I think you need to screen out 4xx at least to prevent client errors from marking down your backends. There is also failonstatus which, granted, could probably be enhanced to accept ranges instead of just a list to accommodate this need. -- Daniel Ruggeri
Re: proxy segfault in httpd-trunk
BTW: I ask myself why we need a global mutex to protect a pool. Wouldn't
a thread mutex be sufficient?
I figured accessing a pool inside a module config is something that needs
to be protected across process boundaries, hence the global lock. Are
module configs not globally unique in the sense that they are not copied
into every process ?
I guess a call to apr_global_mutex_child_init is missing in the
child_init hook of mod_proxy.
Can't we fix the code in merge_proxy_config with something like mutex =
overrides-mutex ? base-mutex ? create_mutex() for which we don't need to
make use of another hook.
On Tue, May 14, 2013 at 9:36 PM, Ruediger Pluem rpl...@apache.org wrote:
I guess a call to apr_global_mutex_child_init is missing in the
child_init hook of mod_proxy.
BTW: I ask myself why we need a global mutex to protect a pool. Wouldn't a
thread mutex be sufficient?
Plus why are we not using the httpd mutex API to make the mutex method
configurable?
Regards
Rüdiger
Graham Leggett wrote:
Hi all,
I am currently getting a segfault in the proxy during httpd-test, it
looks like conf-mutex is being used before being initialised:
Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0010
0x0001007cc8c0 in apr_global_mutex_lock (mutex=0x0) at
global_mutex.c:97
97if (mutex-thread_mutex) {
(gdb) bt
#0 0x0001007cc8c0 in apr_global_mutex_lock (mutex=0x0) at
global_mutex.c:97
#1 0x000100159709 in ap_proxy_initialize_worker
(worker=0x10108f460, s=0x10104cae0, p=0x101051228) at proxy_util.c:1734
Not sure if this rings a bell for anyone?
Regards,
Graham
--
Re: proxy segfault in httpd-trunk
Just wondering if we also have a problem with the pool as well... if base doesn't have a proxy, we don't have the subpool. Looks like it. At least I don't see a reason why Nick's reasoning would apply to the mutex but not to the pool. BTW, wondering if instead of leaking proxy_mutex we could set ps-mutex = proxy_mutex in mod_proxy.c when we merge. We could then make proxy_mutex static...? I must admit I'm not familiar enough with the httpd modules at large to be an expert here but to me it *feels* weird to put such a central piece of module management outside the module's config structure. It would solve the problem with create_config/merge_config though and it's also a bit better performance wise (see Graham's commit). Hm, maybe it doesn't feel so weird after all ... Hmmm... The other idea is to keep it as it was, stick pconf back in conf-pool but just always create a sub-pool before conf-pool is used. This *looks* like it removes the need for a mutex... I thought about this as well but decided against it because I figured creating those sub pools would be a much larger performance hit then just having a locking mechanism in place. It'd be a different matter with sub pools being pre-allocated, 'swapped' into place when needed, etc. This feels like going way overboard though. On Thu, May 16, 2013 at 5:20 AM, Jim Jagielski j...@jagunet.com wrote: Hmmm... The other idea is to keep it as it was, stick pconf back in conf-pool but just always create a sub-pool before conf-pool is used. This *looks* like it removes the need for a mutex... On May 15, 2013, at 7:37 PM, Jim Jagielski j...@jagunet.com wrote: Just wondering if we also have a problem with the pool as well... if base doesn't have a proxy, we don't have the subpool. BTW, wondering if instead of leaking proxy_mutex we could set ps-mutex = proxy_mutex in mod_proxy.c when we merge. We could then make proxy_mutex static...? On May 15, 2013, at 7:27 PM, Graham Leggett minf...@sharp.fm wrote: On 16 May 2013, at 1:25 AM, Jim Jagielski j...@jagunet.com wrote: Ugg. You're 100% right. We need to create a global. Here is one I made earlier: http://svn.apache.org/r1482859 Regards, Graham --
mod_security core dumps and r-per_dir_config
I'm trying to investigate some core dumps in mod_security and currently
face this
(gdb) bt
#0 0xf6efc232 in create_tx_context (r=0x1eac8ed0) at mod_security2.c:325
#1 0xf6efc606 in hook_error_log (file=0x80a51bd http_filters.c,
line=493, level=3, status=104, s=0x18144178, r=0x1eac8ed0, mp=0x0,
fmt=0xeb2f6dff Error reading chunk \n)
at mod_security2.c:771
#2 0x08080c2d in ap_run_error_log (file=0x80a51bd http_filters.c,
line=493, level=3, status=104, s=0x18144178, r=0x1eac8ed0, pool=0x0,
errstr=0xeb2f6dff Error reading chunk \n) at log.c:1116
#3 0x0808109b in log_error_core (file=0x80a51bd http_filters.c,
line=493, level=optimized out, status=104, s=0x18144178, c=0x0,
r=0x1eac8ed0, pool=0x0,
fmt=0x80a52a1 Error reading chunk %s , args=0xeb2f8e08
hT\n\b\230L\255\036\177\017\275\036\376\001) at log.c:705
#4 0x08081ec1 in ap_log_rerror (file=0x80a51bd http_filters.c, line=493,
level=3, status=104, r=0x1eac8ed0, fmt=0x80a52a1 Error reading chunk %s )
at log.c:737
#5 0x0808d400 in ap_http_filter (f=0x1eaca3b0, b=0x1eac8eb0,
mode=AP_MODE_READBYTES, block=APR_NONBLOCK_READ, readbytes=8192) at
http_filters.c:493
#6 0xf716bfe6 in ap_proxy_http_process_response (p=0x1eab4cd8,
r=0x1eab4d18, backend=0x9241848, origin=0x929ab40, conf=0x1e8db5e0,
server_portstr=0xeb2fd107 )
at mod_proxy_http.c:1771
#7 0xf716d609 in proxy_http_handler (r=0x1eab4d18, worker=0x13db7be8,
conf=0x1e8db5e0, url=0x1eac8950 /a/b/WebService, proxyname=0x0,
proxyport=optimized out) at mod_proxy_http.c:2037
#8 0xf72daec0 in proxy_run_scheme_handler (r=0x1eab4d18,
worker=0x13db7be8, conf=0x1e8db5e0, url=0x1eac8878
http://10.10.10.10:8080/a/b/WebServicehttp://10.21.9.205:8080/tourconex/services/TourConexWebService,
proxyhost=0x0, proxyport=0) at mod_proxy.c:2455
#9 0xf72dfdbb in proxy_handler (r=0x1eab4d18) at mod_proxy.c:1023
#10 0x0807cdc1 in ap_run_handler (r=0x1eab4d18) at config.c:157
#11 0x080802c6 in ap_invoke_handler (r=0x1eab4d18) at config.c:376
#12 0x0808b6d0 in ap_process_request (r=0x1eab4d18) at http_request.c:282
#13 0x080886e8 in ap_process_http_connection (c=0x1eaaaea8) at
http_core.c:190
#14 0x08084571 in ap_run_process_connection (c=0x1eaaaea8) at
connection.c:43
#15 0x08091aac in process_socket (bucket_alloc=optimized out,
my_thread_num=optimized out, my_child_num=optimized out,
sock=optimized out, p=optimized out)
at worker.c:544
#16 worker_thread (thd=0x1e96ac40, dummy=0x1ea48678) at worker.c:894
#17 0xf7575ac6 in dummy_worker (opaque=0x1e96ac40) at
threadproc/unix/thread.c:142
#18 0xf74fc809 in start_thread () from /lib/libpthread.so.0
#19 0xf745d30e in clone () from /lib/libc.so.6
Backtrace stopped: Not enough registers or memory available to unwind
further
(gdb) print r-per_dir_config
$1 = (struct ap_conf_vector_t *) 0x0
where the offending line of code is
msr-dcfg1 = (directory_config *)ap_get_module_config(r-per_dir_config,
security2_module);
Why would the per_dir_config be NULL here ? I don't think that should ever
be encountered during the request's lifetime, right ?
My confusion continued when I saw
(gdb) print *r
$2 = {pool = 0x1eab4cd8, connection = 0x929ab40, server = 0x18144178, next
= 0x0, prev = 0x0, main = 0x0, the_request = 0x0, assbackwards = 0,
proxyreq = 3, header_only = 0,
protocol = 0x0, proto_num = 0, hostname = 0x0, request_time =
1368183918343107, status_line = 0x0, status = 200, method = 0x0,
method_number = 0, allowed = 0,
allowed_xmethods = 0x0, allowed_methods = 0x0, sent_bodyct = 0,
bytes_sent = 0, mtime = 0, chunked = 0, range = 0x0, clength = 0, remaining
= 0, read_length = 0,
read_body = 0, read_chunked = 0, expecting_100 = 0, headers_in =
0x1ebcee40, headers_out = 0x1eac9750, err_headers_out = 0x1eac98f8,
subprocess_env = 0x1eac93e0,
notes = 0x1eac9a50, content_type = 0x0, handler = 0x0, content_encoding =
0x0, content_languages = 0x0, vlist_validator = 0x0, user = 0x0,
ap_auth_type = 0x0, no_cache = 0,
no_local_copy = 0, unparsed_uri = 0x0, uri = 0x0, filename = 0x0,
canonical_filename = 0x0, path_info = 0x0, args = 0x0, finfo = {pool = 0x0,
valid = 0, protection = 0,
filetype = APR_NOFILE, user = 0, group = 0, inode = 0, device = 0,
nlink = 0, size = 0, csize = 0, atime = 0, mtime = 0, ctime = 0, fname =
0x0, name = 0x0,
filehand = 0x0}, parsed_uri = {scheme = 0x0, hostinfo = 0x0, user =
0x0, password = 0x0, hostname = 0x0, port_str = 0x0, path = 0x0, query =
0x0, fragment = 0x0,
hostent = 0x0, port = 0, is_initialized = 0, dns_looked_up = 0,
dns_resolved = 0}, used_path_info = 0, per_dir_config = 0x0, request_config
= 0x1eac9ba8, htaccess = 0x0,
output_filters = 0x929aff8, input_filters = 0x1eaca3b0,
proto_output_filters = 0x929aff8, proto_input_filters = 0x1eaca3b0,
eos_sent = 0}
(gdb) dump_table(r-headers_in)
[0] 'Server'='Apache-Coyote/1.1' [0x1eaca1e0]
[1] 'X-Powered-By'='Servlet 2.4; JBoss-4.2.2.GA (build:
SVNTag=JBoss_4_2_2_GA date=200710221139)/Tomcat-5.5' [0x1eaca218]
[2] 'Content-Type'='text/xml;charset=utf-8'
Re: mod_security core dumps and r-per_dir_config
How did you investigate into this ? I'll assume with rebuild you mean you rebuilt apache2 from source and reinstalled it. What made you rebuild it in this scenario ? Also, why do you think rebuilding solved it ? I'm being so specific about investigation options since rebuilding and reinstalling it is out of question in my case. I also saw some other issues with the rev-proxy I posted the core dumps for, like core dumps from one of my modules and some really REALLY weird looking core dumps which claimed to originate from system calls. Looking at the time stamps it's clear they are somehow related as they do not even differ in seconds. One of those incidents I relate to an attack on the rev-proxy but the others keep me confused. My best guess is that one process core dumped and took with it at least one other - but I thought this was cared for. I'm talking apache 2.2.22 here and updating it is - unfortunately - also out of question :-/ On Fri, May 24, 2013 at 10:46 AM, Graham Leggett minf...@sharp.fm wrote: On 24 May 2013, at 10:38 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: Why would the per_dir_config be NULL here ? I don't think that should ever be encountered during the request's lifetime, right ? I had this recently, and a completely clean rebuild sorted it out. Regards, Graham --
Bug with mod_xml2enc and docx files
Downloading a .docx file through a HTML rewriting reverse-proxy suddenly
increased file size by 5 kB. So looking at
modules/filters/mod_xml2enc :: static apr_status_t
xml2enc_ffunc(ap_filter_t* f, apr_bucket_brigade* bb)
I saw the following
/* only act if starts-with text/ or contains xml */
if (strncmp(ctype, text/, 5) !strstr(ctype, xml)) {
ap_remove_output_filter(f);
return ap_pass_brigade(f-next, bb) ;
}
(...)
What's this supposed to do with .docx files ? They are (usually) registered
as
application/vnd.openxmlformats-officedocument.wordprocessingml.document
docx
which I would think not suitable for HTML rewriting. Hence I suggest
diff --git a/modules/filters/mod_xml2enc.c b/modules/filters/mod_xml2enc.c
index 13608ed..0ad35aa 100644
--- a/modules/filters/mod_xml2enc.c
+++ b/modules/filters/mod_xml2enc.c
@@ -336,6 +336,15 @@ static apr_status_t xml2enc_ffunc(ap_filter_t* f,
apr_bucket_brigade* bb)
return ap_pass_brigade(f-next, bb) ;
}
+/* 'docx-patch': do not touch application ctypes, even if they contain
xml */
+if (!strncmp(ctype, application, 11)) {
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, f-r, application ctype
detected, removing filter);
+ ap_remove_output_filter(f);
+ return ap_pass_brigade(f-next, bb);
+}
+
if (ctx-bbsave == NULL) {
ctx-bbsave = apr_brigade_create(f-r-pool,
f-r-connection-bucket_alloc);
This of course goes for all such x-typed document formats, since all of
their corresponding mime types start with application.
Cheers,
Thomas
docx.patch
Description: Binary data
Re: apr_palloc is not thread safe
You do not need to expose pools to users through your API to make their usage thread safe. Identify the spots which can trigger pool access and wrap some thread safety mechanism around them, e.g. mutexes. APR does supply you with good means to get your code thread safe - just use it ;-) Look up http://apr.apache.org/docs/apr/1.4/group__apr__proc__mutex.html or http://apr.apache.org/docs/apr/1.4/group___a_p_r___global_mutex.html depending on what kind of thread safety you are interested in - within one process or across process borders. Cheers, Thomas On Fri, May 31, 2013 at 6:15 AM, TROY.LIU 劉春偉 troy@deltaww.com.cnwrote: Thanks for reply, I got it. Now the problem is my APIs does not expose apr_pool_t to user like apr, so my APIs are not thread safe. Best Regards Chunwei Liu -Original Message- From: Philip Martin [mailto:codematt...@ntlworld.com] On Behalf Of Philip Martin Sent: Thursday, May 30, 2013 17:22 troy To: TROY.LIU 劉春偉 Cc: d...@apr.apache.org; modules-...@httpd.apache.org Subject: Re: apr_palloc is not thread safe TROY.LIU 劉春偉 troy@deltaww.com.cn writes: In our practice, we found two threads get same address returned by apr_palloc. It will happen about one hour later after our server starts. We are using apr 1.4.5, the issue still exists in the latest subversion. From apr_pools.h: * Note that most operations on pools are not thread-safe: a single pool * should only be accessed by a single thread at any given time. The one * exception to this rule is creating a subpool of a given pool: one or more * threads can safely create subpools at the same time that another thread * accesses the parent pool. The pool system allows multiple threads to use multiple pools. There is no point trying to make apr_palloc thread-safe as the API is not designed to work that way. Even if apr_palloc was thread-safe how would apr_pool_clear work? A related discussion: http://mail-archives.apache.org/mod_mbox/apr-dev/201304.mbox/%3c792240597.462741.1366211671567.javamail.r...@brainsware.org%3E -- Philip * This email message, including any attachments, is for the sole use of the intended recipient(s) and may contain confidential and privileged information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message. [Delta Electronics, INC. China] *
Re: Bug with mod_xml2enc and docx files
I attached what I have in mind on how to solve this. It's basically what
you suggested.
There's one thing I didn't know how to address and that is mod_proxy_html's
ProxyHTMLExtended directive. It does suggest not to be turned on by default
(
http://httpd.apache.org/docs/current/mod/mod_proxy_html.html#proxyhtmlextended)
so I wouldn't put it into mod_xml2enc config creation. But then
mod_proxy_html would have to cause mod_xml2enc to include CSS/JS as content
type or we risk the two modules behaving inconsistently for non UTF-8
encoded documents.
Personally, I wouldn't want to rely on the user to remember configuring
ProxyHTMLExtended should always be paired with xml2AcceptContentType
text/css and xml2AcceptContentType application/javascript. That's bound to
cause problems.
I don't know about the caveats of one module telling another module how to
configure itself.
Cheers,
Thomas
On Wed, May 29, 2013 at 8:20 PM, Nick Kew n...@webthing.com wrote:
On 29 May 2013, at 14:28, Thomas Eckert wrote:
I saw the following
/* only act if starts-with text/ or contains xml */
if (strncmp(ctype, text/, 5) !strstr(ctype, xml)) {
ap_remove_output_filter(f);
return ap_pass_brigade(f-next, bb) ;
}
You're right, that's a bug.
I don't think your patch really works either: any hardwired rules are
going to be susceptible to similar issues. I guess what it needs is to
be user-configurable, perhaps with HTML and XHTML as defaults.
--
Nick Kew
mod_xml2enc_ctype.patch
Description: Binary data
Re: Bug with mod_xml2enc and docx files
Would you please let me know once you have completed/commited this ? I much rather copy your fix locally and apply it until the next upgrade where it would be contained anyway, instead of live with another custom patch. On Tue, Jun 4, 2013 at 1:13 AM, Nick Kew n...@webthing.com wrote: On 3 Jun 2013, at 14:31, Thomas Eckert wrote: mod_xml2enc_ctype.patch Thanks. Looks a lot like the patch I hacked up but have yet to test or commit :) -- Nick Kew
Re: mod_security core dumps and r-per_dir_config
I finally came around to reinstalling but sadly it didn't help. I still get the exact same core dumps :-/ If anyone else has experienced similiar issues I would be grateful for some input. Caveat: I am one data point, I recently had the problem, and a rebuild solved it for me. That doesn't mean the problem isn't the same for you. True but one is still better then none ;-) Cheers, Thomas On Fri, May 24, 2013 at 11:12 AM, Graham Leggett minf...@sharp.fm wrote: On 24 May 2013, at 11:03 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: How did you investigate into this ? I'll assume with rebuild you mean you rebuilt apache2 from source and reinstalled it. What made you rebuild it in this scenario ? Yes, in my case the module that was returning NULL for per_dir_config was part of httpd, and a make distclean followed by make all sorted it out. Changes to header files had been made, and I suspect this was part of my specific problem. Also, why do you think rebuilding solved it ? I'm being so specific about investigation options since rebuilding and reinstalling it is out of question in my case. In that case are you 200% sure that the headers you are using to build against match the binaries you are building against? In the Redhat world, headers and binaries are packaged separately, and other OSes are probably similar. Caveat: I am one data point, I recently had the problem, and a rebuild solved it for me. That doesn't mean the problem isn't the same for you. Regards, Graham --
sub requests and accessing paths outside the original request's path
Looking at the code for creating sub requests (ap_sub_req_lookup_* and ap_sub_req_method_uri) I get the impression sub requests always have to remain inside the path of their original request's URI. That is, the path specified as the new URI for the before mentioned function calls is appended to the original request's URI. Given an arbitrary request, what would I need to do to create a sub request based off the first request which I can use to inspect the server pointed at by the original request ? For example, think of using some sort of authentication detection in a reverse proxy. Upon receiving a request the reverse proxy should inspect the target server by using a configured authentication URL to check if the target server requires authentication and then display some nice form/query basic/do whatever. In this scenario the configured authentication URL might be outside the path queried by the original request, e.g. original request : uri == /foo/bar/show_pretty.html inspection request : uri == /foo/auth.html which does not seem possible with the ap_sub_req_* functions, as they will construct something like /foo/bar/foo/auth.html or /foo/bar/auth.html. Is there a way to do this ? Cheers, Thomas
reverse proxy bind backend connection to request
In a reverse proxy scenario, I want to do the following 1) read incoming request A and keep it on hold 2) set up connection to backend for (new) request B 3) send request B and read response over that backend connection 4) bind that backend connection to request A so that mod_proxy will use that connection when sending request A. No other request is to use that backend connection, so prevent mod_proxy from reusing that backend connection for any request other then request A (and it's sub requests) I'm done with steps 1 to 3 but the last one is somewhat tricky. My current plan is to patch mod_proxy_http.c so it will only set up a new backend connection for a request if it cannot find an existing one in some table (probably use some module config to get that data across). But this plan lacks a part of step 4) which is to prevent any other request from using that backend connection. Is there a 'built-in' way to do this ? If there is none, what would be the pitfalls I should watch out for when writing a patch for that specific part of mod_proxy ? Cheers, Thomas
mod_proxy, oooled backend connections and the keep-alive race condition
So I've been seeing lots of proxy: error reading status line from remote server by mod_proxy lately. Usually this is caused by the race condition between checking the connection state and the backend closing the connection due to the keep-alive timeout. As Covener pointed out to me in IRC, using mod_proxy_http's env variable proxy-initial-not-pooled does offer a solution to the problem albeit at the cost of performance. The call to ap_proxy_http_process_response() in mod_proxy_http.c eventually boils down to ap_rgetline_core() which calls ap_get_brigade() on r-input_filters. This looks to me like a simple input filter might do the trick if it only checked for a possibility to read on the socket and reopens the connection upon failure type reset by peer. I took a short look at core_create_proxy_req() in server/core.c to see how connections are set up and I wonder if it's possible to recreate/reuse that logic in an input filter. If so, this input filter would offer a nice alternative if hard coding this behavior into mod_proxy/core is frowned upon. Simply make the filter dependant on an env variable, just like proxy-initial-not-pooled.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Yes, the theory thing ... I wish I could have added an experimental patch for such an input filter but I'm afraid that might take a long time for me to finish. I'll try though I hope someone more knowledgeable will pick this up. On Fri, Aug 2, 2013 at 2:28 PM, Jim Jagielski j...@jagunet.com wrote: +1 for the theory, but I'm not sure if it's feasible or not. On Aug 2, 2013, at 5:28 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: So I've been seeing lots of proxy: error reading status line from remote server by mod_proxy lately. Usually this is caused by the race condition between checking the connection state and the backend closing the connection due to the keep-alive timeout. As Covener pointed out to me in IRC, using mod_proxy_http's env variable proxy-initial-not-pooled does offer a solution to the problem albeit at the cost of performance. The call to ap_proxy_http_process_response() in mod_proxy_http.c eventually boils down to ap_rgetline_core() which calls ap_get_brigade() on r-input_filters. This looks to me like a simple input filter might do the trick if it only checked for a possibility to read on the socket and reopens the connection upon failure type reset by peer. I took a short look at core_create_proxy_req() in server/core.c to see how connections are set up and I wonder if it's possible to recreate/reuse that logic in an input filter. If so, this input filter would offer a nice alternative if hard coding this behavior into mod_proxy/core is frowned upon. Simply make the filter dependant on an env variable, just like proxy-initial-not-pooled.
Re: mod_proxy, oooled backend connections and the keep-alive race condition
One could do an 'OPTIONS *' request. But I am not sure if that is any better than proxy-initial-not-pooled in terms of performance. I don't see why an OPTIONS request should not encounter problems where a GET request will. After all, the problem is on the transport layer, not on the application layer. Am I missing something ? Or a filter could just send the first bytes of the request (less than the request line) and then do a filter flush. If that fails, repeating the request on a different connection would be ok even for non- idempotent requests, because we would know that the backend has not received the full request yet. I don't know how many errors this would catch in practice, though. This is pretty much what I stated earlier. So I assume (re)opening a connection and having the whole process of sending the request transition to that (re)new(ed) connection is possible for an input filter. I was not sure about it. Going to look into it once I have time.. On Sat, Aug 3, 2013 at 7:26 PM, Stefan Fritsch s...@sfritsch.de wrote: Am Freitag, 2. August 2013, 11:21:56 schrieb Eric Covener: I think this does not work for GET requests or request without a request body. Just re-read spec, you are right -- we are abusing this in a module as a sort of extended handshake even w/ no body, but not against heterogenous backends. One could do an 'OPTIONS *' request. But I am not sure if that is any better than proxy-initial-not-pooled in terms of performance. Or a filter could just send the first bytes of the request (less than the request line) and then do a filter flush. If that fails, repeating the request on a different connection would be ok even for non- idempotent requests, because we would know that the backend has not received the full request yet. I don't know how many errors this would catch in practice, though.
ProxyPassReverse and regex
I'm facing the problem that I have to use ProxyPassReverse inside a LocationMatch container, which is not really supported as documented in the last paragrpah at http://httpd.apache.org/docs/current/mod/mod_proxy.html#proxypassreverse I find the 'workaround' mentioned in the docs quite useless: The same occurs inside a LocationMatch section, but will probably not work as intended, as ProxyPassReverse will interpret the regexp literally as a path; if needed in this situation, specify the ProxyPassReverse outside the section, or in a separate Location section. How is this supposed to help me when facing such a situation ? If I need to have ProxyPassReverse understand a regex then it will not do so just because I placed it outside of the LocationMatch container since it *always* understands the path argument as literal string - or did I miss anyhing when looking at the ProxyPassReverse code ? In my concrete situation I have a LocationMatch container with a negative lookahead which I need to have ProxyPassReverse understand somehow. I'm thinking of patching ProxyPassReverse using the ProxyPassMatch code so it understands regexps correctly. However, this has surely been considered before and I'm wondering why it was not put in - after all similar code exists for ProxyPassMatch. Are there pitfalls which I haven't seen yet ? Some time ago I dig into some issues I had with using directives inside a LocationMatch container instead of a Location container. I remember being told in IRC LocationMatch behaves less like a Location and more like a Directory internally. Might this be connected to 'ProxyPassReverseMatch' not existing ? Cheers, Thomas
Re: ProxyPassReverse and regex
Given something like this
LocationMatch ^/(foo|bar)
ProxyPass balancer://abc123/
ProxyPassReverse balancer://abc123/
...
LocationMatch
it is obvious the regexp ^/(foo|bar) is used to determine the correct
location container to use for a given request. But after this, what is it's
value for ProxyPassReverse ? The path usually given in Location and
passed on to ProxyPassReverse by putting it inside the location container
is no real path - it is only an evaluation statement. If a request was
matched into the location above we know that the request's path is now
equivalent to the path in a normal location container. For example, compare
the above LocationMatch with this
Location /other
ProxyPass balancer://abc123/
ProxyPassReverse balancer://abc123/
...
/Location
both can be used to catch request with paths along the line of /other.
The second example will pass on the path information to ProxyPassReverse
directly while the first will not. However, for the mod_proxy logic we
still have that information in the request structure. So as long as we can
translate an origin server's name to the one used by the client to query
the reverse proxy and have access to the original request's path we are
fine.
'proof of concept' below works for me:
diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 4fa53dc..febb581 100644
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -895,7 +895,8 @@ PROXY_DECLARE(const char *)
ap_proxy_location_reverse_map(request_rec *r,
}
else if (l1 = l2 strncasecmp((*worker)-s-name, url,
l2) == 0) {
/* edge case where fake is just /... avoid double
slash */
-if ((ent[i].fake[0] == '/') (ent[i].fake[1] == 0)
(url[l2] == '/')) {
+if (((ent[i].fake[0] == '/') (ent[i].fake[1] == 0)
(url[l2] == '/')) ||
+apr_fnmatch_test(ent[i].fake) {
u = apr_pstrdup(r-pool, url[l2]);
} else {
u = apr_pstrcat(r-pool, ent[i].fake, url[l2],
NULL);
I'm using ProxyPassReverse in a rather limited fashion. Do you see
situations where the above fails ?
On Wed, Sep 25, 2013 at 12:31 PM, Nick Kew n...@webthing.com wrote:
On 25 Sep 2013, at 10:06, Thomas Eckert wrote:
I'm facing the problem that I have to use ProxyPassReverse inside a
LocationMatch
Just a thought: could you hack a workaround with Header Edit?
In my concrete situation I have a LocationMatch container with a
negative lookahead which I need to have ProxyPassReverse understand
somehow. I'm thinking of patching ProxyPassReverse using the ProxyPassMatch
code so it understands regexps correctly. However, this has surely been
considered before and I'm wondering why it was not put in - after all
similar code exists for ProxyPassMatch. Are there pitfalls which I haven't
seen yet ?
ProxyPass(Match) applies to the Request, ProxyPassReverse to the Response.
From memory and without looking in the code, the missing link is
per-request
memory of how a regexp was expanded in the ProxyPass so that
ProxyPassReverse
can apply an equivalent rule. It just requires someone to do the work.
If you hack it, you might give some consideration to making an API for the
ProxyPassReverse regexp expansion, so output filters like mod_proxy_html
can use it.
--
Nick Kew
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Yann, although I do expect it to solve the issue discussed here, I don't
think simply flushing everything instantly is the right way to go. For
example, how do the proposed changes work with modules which scan the
request body like mod_security ? A lot of scanning/parsing can only be done
in a senseful way if the entity to be scanned/parsed is available in full.
Passing through everything immediately will probably cause the SlowHTTP to
bybass the reverse proxy and harm the web server direclty. (I haven't had
time to appl the changes to a test environment)
On Tue, Oct 1, 2013 at 3:39 PM, Yann Ylavic ylavic@gmail.com wrote:
Hi devs,
I was about to propose the following patch to allow mod_proxy_http to
flush all data received from the client to the backend immediatly (as it
does already for response data), based on the env proxy-flushall.
It should address this issue, and in the same time allow protocols like MS
RPC-over-HTTP to work with :
SetEnvIf Request_Method ^RPC_IN_DATA proxy-flushall
The patch does preserve the CL when the client gives one, unless some
input filters have been inserted (DEFLATE...).
In no case, when the env is there, the patch will spool the data, using
chunked encoding when CL was provided but might have change (input_filters
!= proto_input_filters).
Maybe you can give it a chance here...
[patch]
Index: modules/proxy/mod_proxy_http.c
===
--- modules/proxy/mod_proxy_http.c(revision 1528080)
+++ modules/proxy/mod_proxy_http.c(working copy)
@@ -244,10 +244,15 @@ static int stream_reqbody_chunked(apr_pool_t *p,
apr_bucket_alloc_t *bucket_alloc = r-connection-bucket_alloc;
apr_bucket_brigade *bb;
apr_bucket *e;
+int flushall = 0;
add_te_chunked(p, bucket_alloc, header_brigade);
terminate_headers(bucket_alloc, header_brigade);
+if (apr_table_get(r-subprocess_env, proxy-flushall)) {
+flushall = 1;
+}
+
while (!APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade)))
{
char chunk_hdr[20]; /* must be here due to transient bucket. */
@@ -305,7 +310,8 @@ static int stream_reqbody_chunked(apr_pool_t *p,
}
/* The request is flushed below this loop with chunk EOS header */
-rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb,
0);
+rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb,
+ flushall);
if (rv != OK) {
return rv;
}
@@ -371,6 +377,7 @@ static int stream_reqbody_cl(apr_pool_t *p,
apr_off_t cl_val = 0;
apr_off_t bytes;
apr_off_t bytes_streamed = 0;
+int flushall = 0;
if (old_cl_val) {
char *endstr;
@@ -387,6 +394,10 @@ static int stream_reqbody_cl(apr_pool_t *p,
}
terminate_headers(bucket_alloc, header_brigade);
+if (apr_table_get(r-subprocess_env, proxy-flushall)) {
+flushall = 1;
+}
+
while (!APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade)))
{
apr_brigade_length(input_brigade, 1, bytes);
@@ -450,7 +461,8 @@ static int stream_reqbody_cl(apr_pool_t *p,
}
/* Once we hit EOS, we are ready to flush. */
-rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb,
seen_eos);
+rv = ap_proxy_pass_brigade(bucket_alloc, r, p_conn, origin, bb,
+ (seen_eos || flushall));
if (rv != OK) {
return rv ;
}
@@ -700,6 +712,7 @@ int ap_proxy_http_request(apr_pool_t *p, request_r
apr_off_t bytes;
int force10, rv;
conn_rec *origin = p_conn-connection;
+int flushall = 0;
if (apr_table_get(r-subprocess_env, force-proxy-request-1.0)) {
if (r-expecting_100) {
@@ -710,6 +723,10 @@ int ap_proxy_http_request(apr_pool_t *p, request_r
force10 = 0;
}
+if (apr_table_get(r-subprocess_env, proxy-flushall)) {
+flushall = 1;
+}
+
header_brigade = apr_brigade_create(p, origin-bucket_alloc);
rv = ap_proxy_create_hdrbrgd(p, header_brigade, r, p_conn,
worker, conf, uri, url, server_portstr,
@@ -822,7 +839,8 @@ int ap_proxy_http_request(apr_pool_t *p, request_r
* (an arbitrary value.)
*/
} while ((bytes_read MAX_MEM_SPOOL - 80)
- !APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade)));
+ !APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))
+ !flushall);
/* Use chunked request body encoding or send a content-length body?
*
@@ -876,7 +894,8 @@ int ap_proxy_http_request(apr_pool_t *p, request_r
if (force10
|| (apr_table_get(r-subprocess_env, proxy-sendcl)
!apr_table_get(r-subprocess_env, proxy-sendchunks)
- !apr_table_get(r-subprocess_env,
Re: mod_proxy, oooled backend connections and the keep-alive race condition
Sorry for the delayed reply. At the moment I don't have time to look at the
patch proposal in detail, sorry about that too. I'll get back to it soon, I
hope.
Pre-fetching 16K (or waiting for input filters to provide these) is not
always a fast operation, and the case where the backend closes its
connection in the meantime is easy to
reproduce (and even likely to happen with some applications).
I absolutely agree. That's why I proposed to solve this more systematically
then trial-and-error-like in the first place.
Hence, why cannot mod_proxy_http prefetch the client's body *before*
trying anything with the backend connection, and only if it's all good,
connect (or reuse a connection to) the
backend and then start forwarding the data immediatly (flushing the
prefetched ones) ?
Now I'm confused. Are we talking about flushing everything immediately from
client to backend (that's what I understood from your first patch proposal
in this thread) or are we talking about pre fetching the whole request body
from the client and then passing it on to the backend as a whole ?
The problem I mentioned in the first message is about treating the backend
connection in an error prone way. By not ensuring the connection is still
valid - e.g. as seen by the peer - we risk running into this problem no
matter what we do elsewhere. So I really think the proper way to handle
this is to reopen the connection if necessary - be this with a filter or
integrated into the connection handling itself - and only fail after we
tried at least once.
Re-thinking the proposed flush mechanism, I don't think using the flushing
will really solve the problem, albeit it probably narrowing down the window
of opportunity for the problem significantly in most scenarios. I mention
this because I'm getting the feeling two different discussions are being
mixed up: Solving the keep-alive time out problem and introducing the
flush-all-immediately option. While the latter might improve the
situation with the first, it is no complete solution and there are a lot of
scenarios where it does not apply due to other factors like filters (as
discussed previously).
The flush option itself sounds like a good idea, so I see no reason why not
to put it in. I just don't want to loose focus on the actual problem ;-)
Cheers,
Thomas
On Mon, Oct 7, 2013 at 6:49 PM, Yann Ylavic ylavic@gmail.com wrote:
Sorry to insist about this issue but I don't see how the current
mod_proxy_http's behaviour of connecting the backend (or checking
established connection reusability) before prefetching the client's body is
not a problem.
Pre-fetching 16K (or waiting for input filters to provide these) is not
always a fast operation, and the case where the backend closes its
connection in the meantime is easy to reproduce (and even likely to happen
with some applications).
Hence, why cannot mod_proxy_http prefetch the client's body *before*
trying anything with the backend connection, and only if it's all good,
connect (or reuse a connection to) the backend and then start forwarding
the data immediatly (flushing the prefetched ones) ?
By doing this, the time between the connect (or check) and the first bytes
sent to the backend is minimal.
It does not require to disable the prefetch since this one can really help
the decision about forwarding Content-Length vs chunked (which may not be
handled by the backend, IFAIK this is not a HTTP/1.1 requirement to handle
it for requests).
That was the purpose of the first patch I proposed ealier, but had no
feedback...
Maybe this message and the following patch could have more chance.
This patch is quite the same as the previous one (ap_proxy_http_request
split in 2, ap_proxy_http_prefetch to prefetch before connect and
ap_proxy_http_request to forward using the relevent
stream_reqbody_cl/chunked once prefetched and connected). It just fixes the
immediate flush of the prefetched bytes when it's time to forward, and the
handling of backend-close set by ap_proxy_create_hdrbrgd (or
ap_proxy_http_prefetch) before the connection even exist (or the previous
one to be reused).
Regards,
Yann.
Index: modules/proxy/mod_proxy_http.c
===
--- modules/proxy/mod_proxy_http.c(revision 1529127)
+++ modules/proxy/mod_proxy_http.c(working copy)
@@ -251,6 +251,7 @@ static int stream_reqbody_chunked(apr_pool_t *p,
while (!APR_BUCKET_IS_EOS(APR_BRIGADE_FIRST(input_brigade)))
{
char chunk_hdr[20]; /* must be here due to transient bucket. */
+int flush = 0;
/* If this brigade contains EOS, either stop or remove it. */
if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) {
@@ -299,13 +300,14 @@ static int stream_reqbody_chunked(apr_pool_t *p,
}
header_brigade = NULL;
+flush = 1;
}
else {
bb = input_brigade;
Erorr scoreboard is full, not at MaxRequestWorkers without traffic
Hey folks, there's been quite a few issues with the scoreboard is full, not at MaxRequestWorkers error log message. From what I've found all of them involved some sort of heavy traffic. I've been wondering about the root cause for this but now I have a reverse proxy showing this message right after starting up - no traffic involved at all. I'm getting the message for a simple reverse proxy with about a hundred virtual hosts which is strange because StartServers 3 ServerLimit 30 MaxClients 1500 MinSpareThreads 25 MaxSpareThreads 75 ThreadsPerChild 50 MaxRequestsPerChild 0 doesn't look like it cannot handle 100 hosts without traffic. The vhosts themselves look like VirtualHost 10.8.17.133:80 ServerName my_domain_3 SSLProxyEngine On RequestHeader set X-Forwarded-Proto http Location / ProxyPass balancer://58be3a18b1c6590f106e3536d31d0447/ lbmethod=bybusyness ProxyPassReverse balancer://58be3a18b1c6590f106e3536d31d0447/ Require all granted /Location /VirtualHost which is really basic. There is only one virtual host with a real configuration (setup is part of a scalability test). Again, there is no traffic involved, it's happening right after start up. I checked the processes and the start up is clean, meaning there are no old left-overs that might interfere somehow. Any hints on how to debug this ?
Re: Erorr scoreboard is full, not at MaxRequestWorkers without traffic
Regarding normal traffic handling, where the thread count might change over time, I can see your point. But with a 'static' state, such as right after start up with no traffic, how would (idle_thread_count min_spare_threads) be triggered ? I would expect the thread count to be unchanging as there are no reasons to shut down active threads - both since (MinSpareThreads MaxSpareThreads) and (idle_thread_count == min_spare_threads). On Fri, Oct 18, 2013 at 5:19 PM, Jim Jagielski j...@jagunet.com wrote: The mojo for this, in both worker and event is: (active_thread_count = ap_daemons_limit * threads_per_child) which is only worried about if: (idle_thread_count min_spare_threads) On Oct 18, 2013, at 10:22 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: Hey folks, there's been quite a few issues with the scoreboard is full, not at MaxRequestWorkers error log message. From what I've found all of them involved some sort of heavy traffic. I've been wondering about the root cause for this but now I have a reverse proxy showing this message right after starting up - no traffic involved at all. I'm getting the message for a simple reverse proxy with about a hundred virtual hosts which is strange because StartServers 3 ServerLimit 30 MaxClients 1500 MinSpareThreads 25 MaxSpareThreads 75 ThreadsPerChild 50 MaxRequestsPerChild 0 doesn't look like it cannot handle 100 hosts without traffic. The vhosts themselves look like VirtualHost 10.8.17.133:80 ServerName my_domain_3 SSLProxyEngine On RequestHeader set X-Forwarded-Proto http Location / ProxyPass balancer://58be3a18b1c6590f106e3536d31d0447/ lbmethod=bybusyness ProxyPassReverse balancer://58be3a18b1c6590f106e3536d31d0447/ Require all granted /Location /VirtualHost which is really basic. There is only one virtual host with a real configuration (setup is part of a scalability test). Again, there is no traffic involved, it's happening right after start up. I checked the processes and the start up is clean, meaning there are no old left-overs that might interfere somehow. Any hints on how to debug this ?
mod_proxy reverse proxying and AH01179: balancer slotmem_create failed
As of late I'm seeing a lot of AH01179: balancer slotmem_create failed error messages which prevent apache2 from starting. I do have DefaultRuntimeDir /var/run/apache2 set. For some reasons there appear to be left-over .shm files in the DefaultRunTimeDir between stop-starts/restarts which then get picked up by mod_slotmem when mod_proxy asks it for shared memory. Unfortunately, those .shm files sometimes have the wrong size and get rejected by the mod_slotmem code due to size mismatch. To me it looks like this situation suffers from the same issue as reported in https://issues.apache.org/bugzilla/show_bug.cgi?id=55449 though I only have one user (nobody) for all the worker processes. I get the systems back up and running by deleting all .shm files in the DefaultRunTimeDir between stop-starts/restarts but this seems fragile to me. Another workaround, as crude as it is, seems to be disabling the reusing of existing shared memory segments by setting fbased = 0; in modules/slotmem/mod_slotmem_shm.c in slotmem_create(). Neither of those two appraoches seems right to me. I do need at least a temporary workaround since this is causing major havoc.
Re: mod_proxy reverse proxying and AH01179: balancer slotmem_create failed
It is not mentioned specifically in the configuration files, so it defaults to Off according to http://httpd.apache.org/docs/current/mod/mod_proxy.html#balancerpersist On Wed, Nov 6, 2013 at 3:44 PM, Jim Jagielski j...@jagunet.com wrote: Is BalancerPersist Off or On? Those .shm files should only stick around if we want to persist state across restarts. On Nov 6, 2013, at 8:39 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: As of late I'm seeing a lot of AH01179: balancer slotmem_create failed error messages which prevent apache2 from starting. I do have DefaultRuntimeDir /var/run/apache2 set. For some reasons there appear to be left-over .shm files in the DefaultRunTimeDir between stop-starts/restarts which then get picked up by mod_slotmem when mod_proxy asks it for shared memory. Unfortunately, those .shm files sometimes have the wrong size and get rejected by the mod_slotmem code due to size mismatch. To me it looks like this situation suffers from the same issue as reported in https://issues.apache.org/bugzilla/show_bug.cgi?id=55449 though I only have one user (nobody) for all the worker processes. I get the systems back up and running by deleting all .shm files in the DefaultRunTimeDir between stop-starts/restarts but this seems fragile to me. Another workaround, as crude as it is, seems to be disabling the reusing of existing shared memory segments by setting fbased = 0; in modules/slotmem/mod_slotmem_shm.c in slotmem_create(). Neither of those two appraoches seems right to me. I do need at least a temporary workaround since this is causing major havoc.
Re: mod_proxy reverse proxying and AH01179: balancer slotmem_create failed
Thanks for the patch, so far it's looking good. I'll get back to you once I have more information. On Wed, Nov 6, 2013 at 4:09 PM, Jim Jagielski j...@jagunet.com wrote: try this: On Nov 6, 2013, at 9:59 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: It is not mentioned specifically in the configuration files, so it defaults to Off according to http://httpd.apache.org/docs/current/mod/mod_proxy.html#balancerpersist On Wed, Nov 6, 2013 at 3:44 PM, Jim Jagielski j...@jagunet.com wrote: Is BalancerPersist Off or On? Those .shm files should only stick around if we want to persist state across restarts. On Nov 6, 2013, at 8:39 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: As of late I'm seeing a lot of AH01179: balancer slotmem_create failed error messages which prevent apache2 from starting. I do have DefaultRuntimeDir /var/run/apache2 set. For some reasons there appear to be left-over .shm files in the DefaultRunTimeDir between stop-starts/restarts which then get picked up by mod_slotmem when mod_proxy asks it for shared memory. Unfortunately, those .shm files sometimes have the wrong size and get rejected by the mod_slotmem code due to size mismatch. To me it looks like this situation suffers from the same issue as reported in https://issues.apache.org/bugzilla/show_bug.cgi?id=55449 though I only have one user (nobody) for all the worker processes. I get the systems back up and running by deleting all .shm files in the DefaultRunTimeDir between stop-starts/restarts but this seems fragile to me. Another workaround, as crude as it is, seems to be disabling the reusing of existing shared memory segments by setting fbased = 0; in modules/slotmem/mod_slotmem_shm.c in slotmem_create(). Neither of those two appraoches seems right to me. I do need at least a temporary workaround since this is causing major havoc.
mod_proxy: maximum hostname length for workers
I'm looking at an issue with this log message AH00526: Syntax error on line 6 of myconfig.conf: BalancerMember worker hostname (---dd-eee-ff.us-east-1.elb.amazonaws.com) too long with the root cause being (modules/proxy/mod_proxy.h) #define PROXY_WORKER_MAX_HOSTNAME_SIZE 64 and I'm wondering why this is. I did not see any comments on why there is this limit, so I assume it's for performance reasons. Also, I'm wondering what the implications (if any) of altering this limit are.
Re: mod_proxy: maximum hostname length for workers
I'd be +1 in adjusting all of those fields bigger, but I'm guessing that constitutes an API change for proxy... API change, why is that ? At least the shm size stuff doesn't look like having a lot of implications other then memory consumption - to make sure of this assumption is why I asked in the first place. I can well imagine setting those defaults at build time, possibly with a directive for performance fine tuning at post-build time. On Fri, Nov 8, 2013 at 3:35 PM, Jim Jagielski j...@jagunet.com wrote: Yeah, it's basically for performance and storage reasons (since those strings are stored in shm)... Nowadays I don't think shm is such an expensive commodity, though I can imagine some setups where the default sizes allowed by the kernel could be kinda small. I'd be +1 in adjusting all of those fields bigger, but I'm guessing that constitutes an API change for proxy... On Nov 8, 2013, at 5:17 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: I'm looking at an issue with this log message AH00526: Syntax error on line 6 of myconfig.conf: BalancerMember worker hostname ( ---dd-eee-ff.us-east-1.elb.amazonaws.com) too long with the root cause being (modules/proxy/mod_proxy.h) #define PROXY_WORKER_MAX_HOSTNAME_SIZE 64 and I'm wondering why this is. I did not see any comments on why there is this limit, so I assume it's for performance reasons. Also, I'm wondering what the implications (if any) of altering this limit are.
Fwd: unsetting encrypted cookies when encryption key changes
Switching mailing list from users to dev becazse to me this does not appear to be a configuration problem. Anyone care to give a hint ? -- Forwarded message -- From: Thomas Eckert thomas.r.w.eck...@gmail.com Date: Mon, Nov 18, 2013 at 9:36 AM Subject: Re: unsetting encrypted cookies when encryption key changes To: us...@httpd.apache.org Ideas, anyone ? On Mon, Nov 11, 2013 at 5:26 PM, Thomas Eckert thomas.r.w.eck...@gmail.comwrote: Trying to figure out how to unset encrypted cookies for which the encryption key was changed. Docs at http://httpd.apache.org/docs/current/mod/mod_session_crypto.html say If the encryption key is changed, sessions will be invalidated automatically. but using a config like Location / AuthName my_auth AuthFormProvider custom_provider AuthType form AuthFormLoginRequiredLocation /form_login Session On SessionCookieName example_cookie path=/;httponly SessionCryptoPassphrase aaadGJ0c3BwWWRqTktzQmZQcERGYk0= Require valid-user /Location Location /form_login SetHandler form-login-handler AuthFormLoginRequiredLocation /form_login AuthFormLoginSuccessLocation / AuthFormProvider custom_provider AuthType form AuthName my_auth Session On SessionCookieName example_cookie path=/;httponly SessionCryptoPassphrase aaadGJ0c3BwWWRqTktzQmZQcERGYk0= Require valid-user /Location and changing the encryption secret after a user has logged on succesfully will give me [session_crypto:error] [pid 22437:tid 3024407408] (16)Error string not specified yet: [client 10.10.10.10:57469] AH01842: decrypt session failed, wrong passphrase? [session:error] [pid 22437:tid 3024407408] (16)Error string not specified yet: [client 10.10.10.10:57469] AH01817: error while decoding the session, session not loaded: /form_login [session_crypto:error] [pid 22437:tid 3024407408] (16)Error string not specified yet: [client 10.10.10.10:57469] AH01842: decrypt session failed, wrong passphrase? [session:error] [pid 22437:tid 3024407408] (16)Error string not specified yet: [client 10.10.10.10:57469] AH01817: error while decoding the session, session not loaded: /form_login and redirecting the user back to the form page again and again. I don't see a directive to deal with this in mod_cookie, mod_session or mod_session_crypto so I guess this is meant to work out of the box. What am I missing here ?
Re: unsetting encrypted cookies when encryption key changes
Thanks but I'm no sure if that's what I am looking for. I want to get rid of the old sessions (with the old key) and replace them with new ones (with the new key). For me, that's pretty much invalidating them but I think the docs mean something different with ( http://httpd.apache.org/docs/current/mod/mod_session_crypto.html#sessioncryptopassphrase ) Changing the key on a server has the effect of invalidating all existing sessions. Does your reply mean this is not possible without listing every single key that has ever been used on this vhost ? On Mon, Nov 25, 2013 at 1:48 PM, Graham Leggett minf...@sharp.fm wrote: On 25 Nov 2013, at 2:43 PM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: Switching mailing list from users to dev becazse to me this does not appear to be a configuration problem. Anyone care to give a hint ? and redirecting the user back to the form page again and again. I don't see a directive to deal with this in mod_cookie, mod_session or mod_session_crypto so I guess this is meant to work out of the box. What am I missing here ? Specify multiple keys, with the current one you want to use on top of the list. The very first key will be used for encryption, but all subsequent keys will be used for decryption in turn until one works. Regards, Graham --
Re: unsetting encrypted cookies when encryption key changes
If I have misunderstood, and you simply want all the old cookies ignored and/or removed, then just list the new key by itself, the old cookies will not be considered at all - I'm not sure if the invalid cookie is deleted or not.. That's *exactly* what I want: get rid of the old cookies, encrypted with the old key. And that's also exactly what's not working, see my first message in this thread. There appears an endless loop from the authentication form to the authentication form on cookie decryption failure. On Mon, Nov 25, 2013 at 5:53 PM, Tom Evans tevans...@googlemail.com wrote: On Mon, Nov 25, 2013 at 1:34 PM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: Thanks but I'm no sure if that's what I am looking for. I want to get rid of the old sessions (with the old key) and replace them with new ones (with the new key). Firstly, (ISTM) you want to preserve the contents of the cookies, but encrypted with a new key. In order to do this, you must wait for people to present the old cookies to your site. Since you want to preserve the contents, you must be able to decrypt the old cookie first, thus you require all the old keys that you want to convert. Once all/enough cookies have been converted, you can remove any old keys from your config. So yes, you would need to list all keys used, as long as you expect sessions encrypted with those keys to still be valid as far as httpd is concerned. If I have misunderstood, and you simply want all the old cookies ignored and/or removed, then just list the new key by itself, the old cookies will not be considered at all - I'm not sure if the invalid cookie is deleted or not.. Cheers Tom
ap_proxy_location_reverse_map()
I've been debugging some problems with incorrectly reverse mapped Location
headers and found some backend servers (e.g. OWA for Exchange 2013) to give
headers like
Location: https://myserver:443/path/file?query
which I think are perfectly fine. mod proxy fails to do the trick because
else {
const char *part = url;
l2 = strlen(real);
if (real[0] == '/') {
part = ap_strstr_c(url, ://);
if (part) {
part = ap_strchr_c(part+3, '/');
if (part) {
l1 = strlen(part);
}
else {
part = url;
}
}
else {
part = url;
}
}
if (l1 = l2 strncasecmp(real, part, l2) == 0) {
u = apr_pstrcat(r-pool, ent[i].fake, part[l2], NULL);
return ap_is_url(u) ? u : ap_construct_url(r-pool, u, r);
}
}
which does not take the port behind the domain name into consideration
(note: simple example setup, fake path is just '/' obviously). I looked
over the code and got the feeling the same problem applies to the whole
section, not just that one strncasecmp() call. Since the port given by the
backend server is not much use to the reverse proxy at that point, we can
just drop it on the floor and continue, e.g. like this
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -894,11 +894,17 @@ PROXY_DECLARE(const char *)
ap_proxy_location_reverse_map(request_rec *r,
}
}
else if (l1 = l2 strncasecmp((*worker)-s-name, url,
l2) == 0) {
+const char* tmp_pchar = url + l2;
+if (url[l2] == ':') {
+tmp_pchar = ap_strchr_c(tmp_pchar, '/');
+}
+
/* edge case where fake is just /... avoid double
slash */
-if ((ent[i].fake[0] == '/') (ent[i].fake[1] == 0)
(url[l2] == '/')) {
-u = apr_pstrdup(r-pool, url[l2]);
+if ((ent[i].fake[0] == '/') (ent[i].fake[1] == 0)
+(tmp_pchar != NULL) (tmp_pchar[0] == '/')) {
+u = apr_pstrdup(r-pool, tmp_pchar);
} else {
-u = apr_pstrcat(r-pool, ent[i].fake, url[l2],
NULL);
+u = apr_pstrcat(r-pool, ent[i].fake, tmp_pchar +
1, NULL);
}
return ap_is_url(u) ? u : ap_construct_url(r-pool, u,
r);
As said above this most likely needs to be spread to the other cases in
that section as well. Anyone see problems with this ?
Re: ap_proxy_location_reverse_map()
Given a config extract like
Proxyy balancer://abcd
BalancerMember https://mybackend.local status=-SE
/Proxy
...
Location /
ProxyPass balancer://abcd/
ProxyPassReverse balancer://abcd/
/Location
what exactly is your suggestion ? Also, can you give an example for a
situation where the port matters ?
On Tue, Nov 26, 2013 at 5:14 PM, Plüm, Rüdiger, Vodafone Group
ruediger.pl...@vodafone.com wrote:
IMHO this should be fixed in the configuration with an additional
mapping that has the port in. In many cases the port matters.
Regards
Rüdiger
*From:* Thomas Eckert [mailto:thomas.r.w.eck...@gmail.com]
*Sent:* Dienstag, 26. November 2013 17:11
*To:* dev@httpd.apache.org
*Subject:* ap_proxy_location_reverse_map()
I've been debugging some problems with incorrectly reverse mapped Location
headers and found some backend servers (e.g. OWA for Exchange 2013) to give
headers like
Location: https://myserver:443/path/file?query
which I think are perfectly fine. mod proxy fails to do the trick because
else {
const char *part = url;
l2 = strlen(real);
if (real[0] == '/') {
part = ap_strstr_c(url, ://);
if (part) {
part = ap_strchr_c(part+3, '/');
if (part) {
l1 = strlen(part);
}
else {
part = url;
}
}
else {
part = url;
}
}
if (l1 = l2 strncasecmp(real, part, l2) == 0) {
u = apr_pstrcat(r-pool, ent[i].fake, part[l2], NULL);
return ap_is_url(u) ? u : ap_construct_url(r-pool, u, r);
}
}
which does not take the port behind the domain name into consideration
(note: simple example setup, fake path is just '/' obviously). I looked
over the code and got the feeling the same problem applies to the whole
section, not just that one strncasecmp() call. Since the port given by the
backend server is not much use to the reverse proxy at that point, we can
just drop it on the floor and continue, e.g. like this
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -894,11 +894,17 @@ PROXY_DECLARE(const char *)
ap_proxy_location_reverse_map(request_rec *r,
}
}
else if (l1 = l2 strncasecmp((*worker)-s-name, url,
l2) == 0) {
+const char* tmp_pchar = url + l2;
+if (url[l2] == ':') {
+tmp_pchar = ap_strchr_c(tmp_pchar, '/');
+}
+
/* edge case where fake is just /... avoid double
slash */
-if ((ent[i].fake[0] == '/') (ent[i].fake[1] == 0)
(url[l2] == '/')) {
-u = apr_pstrdup(r-pool, url[l2]);
+if ((ent[i].fake[0] == '/') (ent[i].fake[1] == 0)
+(tmp_pchar != NULL) (tmp_pchar[0] == '/')) {
+u = apr_pstrdup(r-pool, tmp_pchar);
} else {
-u = apr_pstrcat(r-pool, ent[i].fake, url[l2],
NULL);
+u = apr_pstrcat(r-pool, ent[i].fake, tmp_pchar +
1, NULL);
}
return ap_is_url(u) ? u : ap_construct_url(r-pool,
u, r);
As said above this most likely needs to be spread to the other cases
in that section as well. Anyone see problems with this ?
Re: ap_proxy_location_reverse_map()
Thanks but you ignored the config extract I mentioned.
ProxyPassReverse / https://mybackend.local
ProxyPassReverse / https://mybackend.local:443
does this not translate to
Proxyy balancer://abcd
BalancerMember https://mybackend.local status=-SE
BalancerMember https://mybackend.local:443 status=-SE
/Proxy
? I'm not even sure whether this is correct in terms of configuration - the
docs speak of 'url' as argument to BalancerMember so I guess giving the
port is ok. However, when accessing /path this does not do anything
different then without adding the ':443' line.
https://mybackend.local:443
The original problem was that Location headers like
Location:
https://mybackend.local:443/path/file?queryhttps://myserver:443/path/file?query
are being rewritten to
Location: https://myfrontend.local/:443/path/file?query
which is nonsense. Based on your example I replaced the usage of the
balancer argument with
ProxyPass /path https://mybackend.local
ProxyPassReverse /path https://mybackend.local
ProxyPassReverse /path https://mybackend.local:443
and it will rewrite the above mentioned Location header to
https://myfrontend.local/path:443/path/file?query
which is just as wrong.
Did I misunderstand you somewhere ?
https://mybackend.local:443
On Wed, Nov 27, 2013 at 11:25 AM, Plüm, Rüdiger, Vodafone Group
ruediger.pl...@vodafone.com wrote:
ProxyPass / http://backend:8080/
ProxyPassReverse / http://backend:8080/
There the port matters.
Fix for your issue:
ProxyPassReverse / https://mybackend.local
ProxyPassReverse / https://mybackend.local:443
Regards
Rüdiger
*Von:* Thomas Eckert [mailto:thomas.r.w.eck...@gmail.com]
*Gesendet:* Mittwoch, 27. November 2013 11:20
*An:* dev@httpd.apache.org
*Betreff:* Re: ap_proxy_location_reverse_map()
Given a config extract like
Proxyy balancer://abcd
BalancerMember https://mybackend.local status=-SE
/Proxy
...
Location /
ProxyPass balancer://abcd/
ProxyPassReverse balancer://abcd/
/Location
what exactly is your suggestion ? Also, can you give an example for a
situation where the port matters ?
On Tue, Nov 26, 2013 at 5:14 PM, Plüm, Rüdiger, Vodafone Group
ruediger.pl...@vodafone.com wrote:
IMHO this should be fixed in the configuration with an additional mapping
that has the port in. In many cases the port matters.
Regards
Rüdiger
*From:* Thomas Eckert [mailto:thomas.r.w.eck...@gmail.com]
*Sent:* Dienstag, 26. November 2013 17:11
*To:* dev@httpd.apache.org
*Subject:* ap_proxy_location_reverse_map()
I've been debugging some problems with incorrectly reverse mapped Location
headers and found some backend servers (e.g. OWA for Exchange 2013) to give
headers like
Location: https://myserver:443/path/file?query
which I think are perfectly fine. mod proxy fails to do the trick because
else {
const char *part = url;
l2 = strlen(real);
if (real[0] == '/') {
part = ap_strstr_c(url, ://);
if (part) {
part = ap_strchr_c(part+3, '/');
if (part) {
l1 = strlen(part);
}
else {
part = url;
}
}
else {
part = url;
}
}
if (l1 = l2 strncasecmp(real, part, l2) == 0) {
u = apr_pstrcat(r-pool, ent[i].fake, part[l2], NULL);
return ap_is_url(u) ? u : ap_construct_url(r-pool, u, r);
}
}
which does not take the port behind the domain name into consideration
(note: simple example setup, fake path is just '/' obviously). I looked
over the code and got the feeling the same problem applies to the whole
section, not just that one strncasecmp() call. Since the port given by the
backend server is not much use to the reverse proxy at that point, we can
just drop it on the floor and continue, e.g. like this
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -894,11 +894,17 @@ PROXY_DECLARE(const char *)
ap_proxy_location_reverse_map(request_rec *r,
}
}
else if (l1 = l2 strncasecmp((*worker)-s-name, url,
l2) == 0) {
+const char* tmp_pchar = url + l2;
+if (url[l2] == ':') {
+tmp_pchar = ap_strchr_c(tmp_pchar, '/');
+}
+
/* edge case where fake is just /... avoid double
slash */
-if ((ent[i].fake[0] == '/') (ent[i].fake[1] == 0)
(url[l2] == '/')) {
-u = apr_pstrdup(r-pool, url[l2]);
+if ((ent[i].fake[0] == '/') (ent[i].fake[1] == 0)
+(tmp_pchar != NULL) (tmp_pchar[0
Re: ap_proxy_location_reverse_map()
Reversing the order results in
Location https://myfrontend.local/path/path/file?query
My expectation is that
Location https://mybackend.local/path/file?query
Location https://mybackend.local:433/path/file?query
both be rewritten to
Location https://myfrontend.local/path/file?query
because I think this is the only plausible 'address' the reverse proxy can
deliver to the client. If stripping the port results in breaking some logic
with the backend then that's an internal issue most likely not fixable by
simple rewriting. I don't think we can assume the reverse proxy uses the
same ports facing the clients as does the backend facing the reverse proxy.
Maybe a mapping option like 'backend_port[n] = frontend_port[k] for known
n and k is possible but I even doubt that.
In my test case, there is no 'fishy' stuff going on. The Location headers
look good, simply with an added port that - strictly speaking - is not
necessary. I can imagine the MS folks added the port there to get their URL
parsing a little less complex by standardizing their URL formats (in as
out). That's just me speculating however.
On Wed, Nov 27, 2013 at 12:27 PM, Plüm, Rüdiger, Vodafone Group
ruediger.pl...@vodafone.com wrote:
Can you please reverse the order of your ProxyPassReverse directives in
the test (such that the one with the port comes first in the configuration).
Regards
Rüdiger
*Von:* Plüm, Rüdiger, Vodafone Group
*Gesendet:* Mittwoch, 27. November 2013 12:19
*An:* dev@httpd.apache.org
*Betreff:* AW: ap_proxy_location_reverse_map()
What location would you expect? I agree that the result you see is not
correct.
BTW: ProxyPassReverse does not change anything to your balancer setup.
Regards
Rüdiger
*Von:* Thomas Eckert
[mailto:thomas.r.w.eck...@gmail.comthomas.r.w.eck...@gmail.com]
*Gesendet:* Mittwoch, 27. November 2013 11:54
*An:* dev@httpd.apache.org
*Betreff:* Re: ap_proxy_location_reverse_map()
Thanks but you ignored the config extract I mentioned.
ProxyPassReverse / https://mybackend.local
ProxyPassReverse / https://mybackend.local:443
does this not translate to
Proxyy balancer://abcd
BalancerMember https://mybackend.local status=-SE
BalancerMember https://mybackend.local:443 status=-SE
/Proxy
? I'm not even sure whether this is correct in terms of configuration -
the docs speak of 'url' as argument to BalancerMember so I guess giving the
port is ok. However, when accessing /path this does not do anything
different then without adding the ':443' line.
The original problem was that Location headers like
Location:
https://mybackend.local:443/path/file?queryhttps://myserver:443/path/file?query
are being rewritten to
Location: https://myfrontend.local/:443/path/file?query
which is nonsense. Based on your example I replaced the usage of the
balancer argument with
ProxyPass /path https://mybackend.local
ProxyPassReverse /path https://mybackend.local
ProxyPassReverse /path https://mybackend.local:443
and it will rewrite the above mentioned Location header to
https://myfrontend.local/path:443/path/file?query
which is just as wrong.
Did I misunderstand you somewhere ?
https://mybackend.local:443
On Wed, Nov 27, 2013 at 11:25 AM, Plüm, Rüdiger, Vodafone Group
ruediger.pl...@vodafone.com wrote:
ProxyPass / http://backend:8080/
ProxyPassReverse / http://backend:8080/
There the port matters.
Fix for your issue:
ProxyPassReverse / https://mybackend.local
ProxyPassReverse / https://mybackend.local:443
Regards
Rüdiger
*Von:* Thomas Eckert [mailto:thomas.r.w.eck...@gmail.com]
*Gesendet:* Mittwoch, 27. November 2013 11:20
*An:* dev@httpd.apache.org
*Betreff:* Re: ap_proxy_location_reverse_map()
Given a config extract like
Proxyy balancer://abcd
BalancerMember https://mybackend.local status=-SE
/Proxy
...
Location /
ProxyPass balancer://abcd/
ProxyPassReverse balancer://abcd/
/Location
what exactly is your suggestion ? Also, can you give an example for a
situation where the port matters ?
On Tue, Nov 26, 2013 at 5:14 PM, Plüm, Rüdiger, Vodafone Group
ruediger.pl...@vodafone.com wrote:
IMHO this should be fixed in the configuration with an additional mapping
that has the port in. In many cases the port matters.
Regards
Rüdiger
*From:* Thomas Eckert [mailto:thomas.r.w.eck...@gmail.com]
*Sent:* Dienstag, 26. November 2013 17:11
*To:* dev@httpd.apache.org
*Subject:* ap_proxy_location_reverse_map()
I've been debugging some problems with incorrectly reverse mapped Location
headers and found some backend servers (e.g. OWA for Exchange 2013) to give
headers like
Location: https://myserver:443/path/file?query
which I think are perfectly fine. mod proxy fails to do the trick because
else {
const char *part = url;
l2 = strlen(real);
if (real[0
Re: ap_proxy_location_reverse_map()
Indeed, with that example it works. Now how do I translate this into a configuration like the extract I posted earlier Proxy balancer://abcd BalancerMember https://mybackend.local /Proxy ... Location /path ProxyPass balancer://abcd/path ProxyPassReverse balancer://abcd/path /Location As I said, according to the docs BalancerMember accepts an url as argument just like ProxyPass and ProxyPassReverse do but the port seems to be ignored with BalancerMember. This solution requires the configuration to be repeated, does it not ? Instead of doing ProxyPassReverse /path server you need to do ProxyPassReverse /path https://backend.local:433/path ProxyPassReverse /path https://backend.local/path and also (if applicable) ProxyPassReverse /path http://backend.local:80/path ProxyPassReverse /path http://backend.local/path so it effectively doubles the amount of required ProxyPassReverse to be on the safe side. This is even though the ports are standard ports for the schemes and could just as well be skipped. On Wed, Nov 27, 2013 at 2:08 PM, Plüm, Rüdiger, Vodafone Group ruediger.pl...@vodafone.com wrote: In this case your configuration is wrong. Try either ProxyPassReverse /path https://mybackend.local:443/path ProxyPassReverse /path https://mybackend.local/path Or ProxyPassReverse / https://mybackend.local:443/ ProxyPassReverse / https://mybackend.local/ Regards Rüdiger *Von:* Thomas Eckert [mailto:thomas.r.w.eck...@gmail.com] *Gesendet:* Mittwoch, 27. November 2013 13:44 *An:* dev@httpd.apache.org *Betreff:* Re: ap_proxy_location_reverse_map() Reversing the order results in Location https://myfrontend.local/path/path/file?query My expectation is that Location https://mybackend.local/path/file?query Location https://mybackend.local:433/path/file?query both be rewritten to Location https://myfrontend.local/path/file?query because I think this is the only plausible 'address' the reverse proxy can deliver to the client. If stripping the port results in breaking some logic with the backend then that's an internal issue most likely not fixable by simple rewriting. I don't think we can assume the reverse proxy uses the same ports facing the clients as does the backend facing the reverse proxy. Maybe a mapping option like 'backend_port[n] = frontend_port[k] for known n and k is possible but I even doubt that. In my test case, there is no 'fishy' stuff going on. The Location headers look good, simply with an added port that - strictly speaking - is not necessary. I can imagine the MS folks added the port there to get their URL parsing a little less complex by standardizing their URL formats (in as out). That's just me speculating however. On Wed, Nov 27, 2013 at 12:27 PM, Plüm, Rüdiger, Vodafone Group ruediger.pl...@vodafone.com wrote: Can you please reverse the order of your ProxyPassReverse directives in the test (such that the one with the port comes first in the configuration). Regards Rüdiger *Von:* Plüm, Rüdiger, Vodafone Group *Gesendet:* Mittwoch, 27. November 2013 12:19 *An:* dev@httpd.apache.org *Betreff:* AW: ap_proxy_location_reverse_map() What location would you expect? I agree that the result you see is not correct. BTW: ProxyPassReverse does not change anything to your balancer setup. Regards Rüdiger *Von:* Thomas Eckert [mailto:thomas.r.w.eck...@gmail.comthomas.r.w.eck...@gmail.com] *Gesendet:* Mittwoch, 27. November 2013 11:54 *An:* dev@httpd.apache.org *Betreff:* Re: ap_proxy_location_reverse_map() Thanks but you ignored the config extract I mentioned. ProxyPassReverse / https://mybackend.local ProxyPassReverse / https://mybackend.local:443 does this not translate to Proxyy balancer://abcd BalancerMember https://mybackend.local status=-SE BalancerMember https://mybackend.local:443 status=-SE /Proxy ? I'm not even sure whether this is correct in terms of configuration - the docs speak of 'url' as argument to BalancerMember so I guess giving the port is ok. However, when accessing /path this does not do anything different then without adding the ':443' line. The original problem was that Location headers like Location: https://mybackend.local:443/path/file?queryhttps://myserver:443/path/file?query are being rewritten to Location: https://myfrontend.local/:443/path/file?query which is nonsense. Based on your example I replaced the usage of the balancer argument with ProxyPass /path https://mybackend.local ProxyPassReverse /path https://mybackend.local ProxyPassReverse /path https://mybackend.local:443 and it will rewrite the above mentioned Location header to https://myfrontend.local/path:443/path/file?query which is just as wrong. Did I misunderstand you somewhere ? https://mybackend.local:443 On Wed, Nov 27, 2013 at 11:25 AM, Plüm, Rüdiger
adding hook into mod_auth_form
Trying to add a hook to mod_auth_form via
diff --git a/include/mod_auth.h b/include/mod_auth.h
index 9b9561e..74e2dc6 100644
--- a/include/mod_auth.h
+++ b/include/mod_auth.h
@@ -134,6 +134,8 @@ APR_DECLARE_OPTIONAL_FN(void, ap_authn_cache_store,
(request_rec*, const char*, const char*,
const char*, const char*));
+AP_DECLARE_HOOK(int, form_logout_handler, (request_rec* r));
+
#ifdef __cplusplus
}
#endif
diff --git a/modules/aaa/mod_auth_form.c b/modules/aaa/mod_auth_form.c
index 28045b5..1662eeb 100644
--- a/modules/aaa/mod_auth_form.c
+++ b/modules/aaa/mod_auth_form.c
@@ -82,6 +82,12 @@ typedef struct {
int logout_set;
} auth_form_config_rec;
+APR_HOOK_STRUCT(
+APR_HOOK_LINK(form_logout_handler)
+)
+
+AP_IMPLEMENT_HOOK_RUN_ALL(int, form_logout_handler, (request_rec* r), (r),
OK, DECLINED)
+
static void *create_auth_form_dir_config(apr_pool_t * p, char *d)
{
auth_form_config_rec *conf = apr_pcalloc(p, sizeof(*conf));
@@ -1200,6 +1208,9 @@ static int
authenticate_form_logout_handler(request_rec * r)
conf = ap_get_module_config(r-per_dir_config, auth_form_module);
+/* run the form logout hook as long as the apache session still
contains useful data */
+ap_run_form_logout_handler(r);
+
/* remove the username and password, effectively logging the user out
*/
set_session_auth(r, NULL, NULL, NULL);
but keep getting
server/.libs/libmain.a(exports.o):(.data+0x11dc): undefined reference to
`ap_hook_form_logout_handler'
server/.libs/libmain.a(exports.o):(.data+0x11e0): undefined reference to
`ap_hook_get_form_logout_handler'
server/.libs/libmain.a(exports.o):(.data+0x11e4): undefined reference to
`ap_run_form_logout_handler'
I read the comment in AP_IMPLEMENT_HOOK_RUN_ALL regarding the link prefix
but I figured since I was adding this to
modules/aaa/mod_auth_form.c and include/mod_auth.h it was not relevant. I
tried using a different link prefix and namespace
anyway. Using
diff --git a/include/mod_auth.h b/include/mod_auth.h
index 9b9561e..6535770 100644
--- a/include/mod_auth.h
+++ b/include/mod_auth.h
@@ -134,6 +134,8 @@ APR_DECLARE_OPTIONAL_FN(void, ap_authn_cache_store,
(request_rec*, const char*, const char*,
const char*, const char*));
+APR_DECLARE_EXTERNAL_HOOK(auth, AUTH, int, form_logout_handler,
(request_rec* r));
+
#ifdef __cplusplus
}
#endif
diff --git a/modules/aaa/mod_auth_form.c b/modules/aaa/mod_auth_form.c
index 28045b5..7fd5edd 100644
--- a/modules/aaa/mod_auth_form.c
+++ b/modules/aaa/mod_auth_form.c
@@ -82,6 +82,12 @@ typedef struct {
int logout_set;
} auth_form_config_rec;
+APR_HOOK_STRUCT(
+APR_HOOK_LINK(form_logout_handler)
+)
+
+APR_IMPLEMENT_EXTERNAL_HOOK_RUN_ALL(auth, AUTH, int, form_logout_handler,
(request_rec* r), (r), OK, DECLINED)
+
static void *create_auth_form_dir_config(apr_pool_t * p, char *d)
{
auth_form_config_rec *conf = apr_pcalloc(p, sizeof(*conf));
@@ -1200,6 +1208,9 @@ static int
authenticate_form_logout_handler(request_rec * r)
conf = ap_get_module_config(r-per_dir_config, auth_form_module);
+/* run the form logout hook as long as the apache session still
contains useful data */
+auth_run_form_logout_handler(r);
+
/* remove the username and password, effectively logging the user out
*/
set_session_auth(r, NULL, NULL, NULL);
got me literally flooded with error messages starting with
In file included from exports.c:105:
/usr/src/packages/BUILD/httpd-2.4.4/include/mod_auth.h:137: warning:
return type defaults to 'int'
/usr/src/packages/BUILD/httpd-2.4.4/include/mod_auth.h: In function
'AUTH_DECLARE':
/usr/src/packages/BUILD/httpd-2.4.4/include/mod_auth.h:137: error:
expected declaration specifiers before 'auth_hook_form_logout_handler'
so I guess there's a bit more work required for setting up a new
namespace/link prefix.
Back with the first appraoch (using AP_DECLARE_HOOK and friends) I compared
this with other hooks, e.g. with
http_protocol, http_connection and mod_proxy in general but could not find
the missing link (no pun intended..)
I did read http://httpd.apache.org/docs/2.4/developer/hooks.html but didn't
see discrepancies with my AP_DECLARE* patch.
Anyone care to shed some light on this for me ? Is it preferrable to get it
done via AP_DECLARE_*and friends or should I use APR_DECLARE_* and if so,
then how would I take care of the above mentioned error messages ?
(If anyone is interested in the why: I need to clean up data in my custom
form auth provider on logout. I figured the proper way was to do this via a
hook.)
make mod_auth_form tell you where the credentials came from
I have been having problems with mod_auth_form on returning DENIED from my
custom auth provider. This provider has it's own module-local session
cache, where stuff like accessible paths, credentials and the like are
stored to avoid having to query an external (and expensive) authentication
daemon. Once such a session is accessed by the user browsing (e.g. with the
corresponding session cookie) I might need to invalidate the session (e.g.
time out). After failing the appropriate checks I would return DENIED but
this had an unpleasant drawback: If a user accessed the session by sending
the filled-in form (e.g. on a new device with no cookie) the code would
still return DENIED if the session was invalid for whatever reason. This
resulted in the user being shown the form again, even though the user just
filled in the form correctly.
This is how I solved the problem for me:
diff --git a/modules/aaa/mod_auth_form.c b/modules/aaa/mod_auth_form.c
index 28045b5..91df0c9 100644
--- a/modules/aaa/mod_auth_form.c
+++ b/modules/aaa/mod_auth_form.c
@@ -687,6 +687,18 @@ static int get_form_auth(request_rec * r,
}
}
+/* We sometimes want to know whether the user credentials came from
the HTTP body (on form submit) or from the headers (e.g. cookie).
+ At this point we know the user credentials have not been fetched
from the headers but from the body. */
+if (*sent_user *sent_pw) {
+ /* always attach this note to the main request, so we can find it
again later */
+ request_rec* r_main = r;
+ while (r_main-main) {
+r_main = r_main-main;
+ }
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, got user credentials
from HTTP body);
+ apr_table_set(r_main-notes, auth_form_credentials_source,
apr_pstrdup(r-pool, body));
+}
+
/* set the user, even though the user is unauthenticated at this point
*/
if (*sent_user) {
r-user = (char *) *sent_user;
Is there a better solution with existing means ? If not I propose adding
the above in some way so that custom providers can work around the
described problem.
Re: make mod_auth_form tell you where the credentials came from
I will assume a forms based login and cookie managed sessions but it is not limited to this setup. A user connects, is queried for authentication, submits credentials and is subsequently allowed access. The session is established via a cookie. If the user credentials were accepted by a custom provider, this custom provider does not know whether the client sent the session cookie or whether the user just filled in the authentication form but this might be of relevance to how the custom provider has to react. Suppose, due to internal logic (see below for an example) the custom provider needs to confirm the user credentials by having the user log in again. Obviously, this should not happen if the user just filled in and submitted the form but it should happen if the user's client sent the authentication details via the session cookie. To decide correctly the custom provider needs to be able to discern between the credential sources. In my case the custom provider has to maintain a module local session cache, where each user is mapped to a cache entry in order to prevent unnecessary calls to the underlying authentication daemon. Calls to the daemon are costly compared to accessing the module local session cache. Entries in the session cache can become invalid in between user requests, e.g. by a time out. When such a session cache entry is accessed (due to a request by the corresponding user) the time out is realized by the custom provider and the session entry is invalidated. Now the custom provider needs to decide whether to use the existing user credentials which were handed down into the provider by apache or whether to force the user to confirm the credentials by returning DECLINED - which causes the form to be displayed again. This whole process is important for supporting two factor authentication - in my example with OTP - but I doubt this is the only use case. In general it's a good idea to let the auth providers know where the user credentials came from (eg. headers vs. body). On Tue, Dec 3, 2013 at 1:15 PM, Graham Leggett minf...@sharp.fm wrote: On 03 Dec 2013, at 1:27 PM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: I have been having problems with mod_auth_form on returning DENIED from my custom auth provider. This provider has it's own module-local session cache, where stuff like accessible paths, credentials and the like are stored to avoid having to query an external (and expensive) authentication daemon. Once such a session is accessed by the user browsing (e.g. with the corresponding session cookie) I might need to invalidate the session (e.g. time out). After failing the appropriate checks I would return DENIED but this had an unpleasant drawback: If a user accessed the session by sending the filled-in form (e.g. on a new device with no cookie) the code would still return DENIED if the session was invalid for whatever reason. This resulted in the user being shown the form again, even though the user just filled in the form correctly. I'm not fully understanding the flow you're describing. Are you sure you're not accidentally password protecting / session protecting the login form? The login form needs to be accessible without any restrictions on authn/authz or session, otherwise httpd will deny access to the form too. Can you clarify the flow of requests during login that you are expecting? Regards, Graham --
Re: make mod_auth_form tell you where the credentials came from
There are two type of sessions: * sessions by mod_session which are used to maintain a mapping between user requests and apache's session * sessions in my custom provider, which are used to prevent accessing the underlying auth daemon if not necessary The custom provider itself is fairly simple, as it just registers in the AUTHN_PROVIDER_GROUP with a simple check_password(r, user, pass) function. The module's configuration (server config) contains a hash which I referred to as the module's local session cache. This module local session cache is not visible to anything else then the custom provider module and there is no interaction between that hash and any other part of apache - aside from mutex locks/unlocks around it for accessing the module internal session cache securely (worker mpm). mod_auth_socache is not used. The end user has never logged in, so they get a form, they enter credentials, they are logged in. Time passes, a session of some kind expires (a session provided by mod_session, or an internal unrelated session?), and the user… has to log in again? Basically, yes. 1 user tries to browse protected resource 2 user is redirected to form 3 user fills in and submits form 4 custom auth provider receives the user credentials 5 custom auth provider sets up internal session 6 custom auth provider returns OK 7 mod_session/mod_session_cookie set up their session and cookie (a part of this probably happens before step 4 already but that doesn't matter) 8 user is directed to whatever was given by AuthFormLoginsuccessLocation 9 time passes and custom provider internal session expires 10 user tries to browse protected resource, user's client submitting the cookie 11 based on the session cookie the user name/pass is extracted 12 custom auth provider receives the user credentials 13 custom auth provider looks up it's own session in it's module internal session cache 14 custom auth provider realizes the provider internal session expired 15 custom auth provider returns DECLINED to force the user to log in again 16 continue at step 2 Now suppose the following http://httpd.apache.org/docs/current/mod/mod_auth_form.html#authformloginsuccesslocation 21 user tries to browse protected resource 22 user is redirected to form 23 user fills in and submits form 24 custom auth provider receives the user credentials 25 custom auth provider sets up internal session 26 custom auth provider returns OK 27 mod_session/mod_session_cookie set up their session and cookie 28 user is directed to whatever was given by AuthFormLoginsuccessLocation 29 time passes and custom provider internal session expires 30 user tries to browse protected resource BUT user's client DOES NOT submit the cookie (browser private mode, different browser, completely different device, etc.) 31 user is redirected to form 32 user fills in and submits form 32 custom auth provider receives the user credentials 33 custom auth provider looks up it's own session in it's module internal session cache 34 custom auth provider realizes the provider internal session expired After step 34, the custom provider should *not* return DECLINED because it would have the user be presented with the login form even though the user just submitted the correclty filled-in form. Instead, the custom provider should go on using the credentials it was given. In the custom provider, is there a way to know about the difference with currently existing means ? On Tue, Dec 3, 2013 at 4:45 PM, Graham Leggett minf...@sharp.fm wrote: On 03 Dec 2013, at 5:29 PM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: This whole process is important for supporting two factor authentication - in my example with OTP - but I doubt this is the only use case. In general it's a good idea to let the auth providers know where the user credentials came from (eg. headers vs. body). I see a possible technical solution to something, but I don't yet understand the problem that technical solution is trying to solve. The end user has never logged in, so they get a form, they enter credentials, they are logged in. Time passes, a session of some kind expires (a session provided by mod_session, or an internal unrelated session?), and the user… has to log in again? I get the sense you're fighting against httpd's AAA modules instead of using them. Are you using mod_auth_socache to cache the credentials or something else? Are you using mod_session to implement your session or something else? Regards, Graham --
Re: unsetting encrypted cookies when encryption key changes
1 user tries to browse protected resource 2 user is redirected to form 3 user fills in and submits form 4 user is redirected to AuthFormLoginSuccessLocation and receives encrypted session cookie (encrypted with key A) 5 encryption key changes from key A to key B 6 user tries to browse protected resource 7 apache fails to load the session 8 user is redirected to form 9 user fills in and submits form 10 user is redirected to AuthFormLoginSuccessLocation 11 apache logs the 'failed to decrypt' and 'failed to load session' again 12 user is redirected to form continue at step 9 At this point the only way I found to have the user regain access is to delete the encrypted session cookie in the user's client. This is exactly where my original question sets in because I want to configure mod_session and friends in such a way that any cookie which failed decryption is simply dropped and replaced with a new one. All redirets are 302s. I did not see any 401s. The encrypted session cookie, sent out in step 4, is never changed. I can not see any Set-Cookie headers coming from apache, not even in step 10. On step 7 the log shows [authz_core:debug] mod_authz_core.c(802): [client 10.128.128.51:57290] AH01626: authorization result of Require valid-user : denied (no authenticated user yet) [authz_core:debug] mod_authz_core.c(802): [client 10.128.128.51:57290] AH01626: authorization result of RequireAny: denied (no authenticated user yet) [session_crypto:debug] mod_session_crypto.c(318): (16)Error string not specified yet: [client 10.128.128.51:57290] AH01839: apr_crypto_block_decrypt_finish failed [session_crypto:info] (16)Error string not specified yet: [client 10.128.128.51:57290] AH01840: decryption failed [session_crypto:error] (16)Error string not specified yet: [client 10.128.128.51:57290] AH01842: decrypt session failed, wrong passphrase? [session:error] (16)Error string not specified yet: [client 10.128.128.51:57290] AH01817: error while decoding the session, session not loaded: /form_to_none_login [session_crypto:debug] mod_session_crypto.c(318): (16)Error string not specified yet: [client 10.128.128.51:57290] AH01839: apr_crypto_block_decrypt_finish failed [session_crypto:info] (16)Error string not specified yet: [client 10.128.128.51:57290] AH01840: decryption failed [session_crypto:error] (16)Error string not specified yet: [client 10.128.128.51:57290] AH01842: decrypt session failed, wrong passphrase? [session:error] (16)Error string not specified yet: [client 10.128.128.51:57290] AH01817: error while decoding the session, session not loaded: /form_to_none_login [session_crypto:debug] mod_session_crypto.c(318): (16)Error string not specified yet: [client 10.128.128.51:57290] AH01839: apr_crypto_block_decrypt_finish failed [session_crypto:info] (16)Error string not specified yet: [client 10.128.128.51:57290] AH01840: decryption failed [session_crypto:error] (16)Error string not specified yet: [client 10.128.128.51:57290] AH01842: decrypt session failed, wrong passphrase? [session:error] (16)Error string not specified yet: [client 10.128.128.51:57290] AH01817: error while decoding the session, session not loaded: /form_to_none_login [session_crypto:debug] mod_session_crypto.c(318): (16)Error string not specified yet: [client 10.128.128.51:57290] AH01839: apr_crypto_block_decrypt_finish failed and keeps going on like that for a bit longer. This is repeated for every step following 7. The path '/form_to_none_login' the login form's action. On Mon, Nov 25, 2013 at 6:55 PM, Graham Leggett minf...@sharp.fm wrote: On 25 Nov 2013, at 7:30 PM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: If I have misunderstood, and you simply want all the old cookies ignored and/or removed, then just list the new key by itself, the old cookies will not be considered at all - I'm not sure if the invalid cookie is deleted or not.. That's *exactly* what I want: get rid of the old cookies, encrypted with the old key. And that's also exactly what's not working, see my first message in this thread. There appears an endless loop from the authentication form to the authentication form on cookie decryption failure. Can you be more specific about what is flowing in and out of the server? I take it an encrypted cookie comes in that the server cannot decrypt, the response is… what? 401 Unauthorised? 302 Temporary Redirect? And on that response, what is the value of the cookie being set (assuming the cookie is being set at all?). Regards, Graham --
Re: adding hook into mod_auth_form
I saw those but figured since I was not using any AUTH_DECLARE I would be fine. I'm now using diff --git a/include/mod_auth.h b/include/mod_auth.h index 9b9561e..8807a5c 100644 --- a/include/mod_auth.h +++ b/include/mod_auth.h @@ -134,6 +134,30 @@ APR_DECLARE_OPTIONAL_FN(void, ap_authn_cache_store, (request_rec*, const char*, const char*, const char*, const char*)); +/* Create a set of AUTH_DECLARE(type), AUTH_DECLARE_NONSTD(type) and + * AUTH_DECLARE_DATA with appropriate export and import tags for the platform + */ + +#if !defined(WIN32) + #define AUTH_DECLARE(type) type + #define AUTH_DECLARE_NONSTD(type) type + #define AUTH_DECLARE_DATA +#elif defined(AUTH_DECLARE_STATIC) + #define AUTH_DECLARE(type) type __stdcall + #define AUTH_DECLARE_NONSTD(type) type + #define AUTH_DECLARE_DATA +#elif defined(AUTH_DECLARE_EXPORT) + #define AUTH_DECLARE(type) __declspec(dllexport) type __stdcall + #define AUTH_DECLARE_NONSTD(type) __declspec(dllexport) type + #define AUTH_DECLARE_DATA __declspec(dllexport) +#else + #define AUTH_DECLARE(type) __declspec(dllimport) type __stdcall + #define AUTH_DECLARE_NONSTD(type) __declspec(dllimport) type + #define AUTH_DECLARE_DATA __declspec(dllimport) +#endif + +APR_DECLARE_EXTERNAL_HOOK(auth, AUTH, int, form_logout_handler, (request_rec* r)); + #ifdef __cplusplus } #endif diff --git a/modules/aaa/mod_auth_form.c b/modules/aaa/mod_auth_form.c index 28045b5..317db2f 100644 --- a/modules/aaa/mod_auth_form.c +++ b/modules/aaa/mod_auth_form.c @@ -1200,6 +1202,9 @@ static int authenticate_form_logout_handler(request_rec * r) conf = ap_get_module_config(r-per_dir_config, auth_form_module); +/* run the form logout hook as long as the apache session still contains useful data */ +auth_run_form_logout_handler(r); + /* remove the username and password, effectively logging the user out */ set_session_auth(r, NULL, NULL, NULL); @@ -1298,3 +1303,9 @@ AP_DECLARE_MODULE(auth_form) = auth_form_cmds, /* command apr_table_t */ register_hooks /* register hooks */ }; + +APR_HOOK_STRUCT( +APR_HOOK_LINK(form_logout_handler) +) + +APR_IMPLEMENT_EXTERNAL_HOOK_RUN_ALL(auth, AUTH, int, form_logout_handler, (request_rec* r), (r), OK, DECLINED) and it compiles/links fine but loading my module I get undefined symbol: auth_hook_form_logout_handler. Looking into mod_auth_form.so (readelf -a) I don't see any symbol auth_hook_form_logout_handler nor anything close to it. The only hook related symbols in mod_auth_form.so are ap_hook_ stuff. The reason why I didn't include those macros in the first place is that I'm running on Linux (SLES) and figured they would do exactly nothing for me because #if !defined(WIN32) #define AUTH_DECLARE(type) type #define AUTH_DECLARE_NONSTD(type) type #define AUTH_DECLARE_DATA #elif (...) Feels like I'm missing something obvious here.. On Fri, Nov 29, 2013 at 8:52 PM, Plüm, Rüdiger, Vodafone Group ruediger.pl...@vodafone.com wrote: I guess your second example is correct (with EXTERNAL), but you miss the following before in the header file (example from mod_proxy): /* Create a set of PROXY_DECLARE(type), PROXY_DECLARE_NONSTD(type) and * PROXY_DECLARE_DATA with appropriate export and import tags for the platform */ #if !defined(WIN32) #define PROXY_DECLARE(type)type #define PROXY_DECLARE_NONSTD(type) type #define PROXY_DECLARE_DATA #elif defined(PROXY_DECLARE_STATIC) #define PROXY_DECLARE(type)type __stdcall #define PROXY_DECLARE_NONSTD(type) type #define PROXY_DECLARE_DATA #elif defined(PROXY_DECLARE_EXPORT) #define PROXY_DECLARE(type)__declspec(dllexport) type __stdcall #define PROXY_DECLARE_NONSTD(type) __declspec(dllexport) type #define PROXY_DECLARE_DATA __declspec(dllexport) #else #define PROXY_DECLARE(type)__declspec(dllimport) type __stdcall #define PROXY_DECLARE_NONSTD(type) __declspec(dllimport) type #define PROXY_DECLARE_DATA __declspec(dllimport) #endif Regards Rüdiger *Von:* Thomas Eckert [mailto:thomas.r.w.eck...@gmail.com] *Gesendet:* Freitag, 29. November 2013 18:36 *An:* dev@httpd.apache.org *Betreff:* adding hook into mod_auth_form Trying to add a hook to mod_auth_form via diff --git a/include/mod_auth.h b/include/mod_auth.h index 9b9561e..74e2dc6 100644 --- a/include/mod_auth.h +++ b/include/mod_auth.h @@ -134,6 +134,8 @@ APR_DECLARE_OPTIONAL_FN(void, ap_authn_cache_store, (request_rec*, const char*, const char*, const char*, const char*)); +AP_DECLARE_HOOK(int, form_logout_handler, (request_rec* r)); + #ifdef __cplusplus } #endif diff --git a/modules/aaa/mod_auth_form.c b/modules/aaa/mod_auth_form.c index
Re: mod_proxy, oooled backend connections and the keep-alive race condition
It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. While it's true that the race condition itself cannot be avoided we can definitely work around the resulting problem situation, e.g. by trying to open the connection again once and then either send the data or fail back to the client as we do right now. The approach itself is not theoretical: I know that's exactly how certain forward proxies solve the problem. Plus, if we do this here, shouldn't we do it for all proxy protocol submodules (fcgi, etc...)? I go with my original suggestion: Have the above mentioned retry-once mechanism apply on connection level so that request modules don't even notice. Otherwise I would agree in having it added to the appropriate modules. I would like to come up with the appropriate patch but I doubt I'll have the time for it anytime soon :-( As it stands I think Yann's approach in flushing through the reverse proxy is better then doing nothing. Though I expect that to be useless for a lot of deployments where stuff like AV scanning or mod_security will need to buffer the request bodies - then you will have the original problem again. On Thu, Dec 5, 2013 at 4:05 PM, Jim Jagielski j...@jagunet.com wrote: There hardly seemed any consensus on the patch... It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. IMO, a reverse proxy should get out of the way as quickly as possible. Plus, if we do this here, shouldn't we do it for all proxy protocol submodules (fcgi, etc...)? Plus, as mentioned, this single patch includes 2 fixes which can be/should be independent. On Dec 5, 2013, at 8:06 AM, Yann Ylavic ylavic@gmail.com wrote: Hi Jan, I don't think it is fixed in trunk, but I may have missed the commits. Which ones are you talking about? Regards, Yann. On Thu, Dec 5, 2013 at 1:51 PM, Jan Kaluža jkal...@redhat.com wrote: Hi, should this be fixed in trunk already? I see some commits in proxy code based on your ideas Yann, but I'm not sure if they address this particular problem too. Jan Kaluza On 10/17/2013 04:52 PM, Yann Ylavic wrote: On Thu, Oct 17, 2013 at 11:36 AM, Thomas Eckert thomas.r.w.eck...@gmail.com mailto:thomas.r.w.eck...@gmail.com wrote: Hence, why cannot mod_proxy_http prefetch the client's body *before* trying anything with the backend connection, and only if it's all good, connect (or reuse a connection to) the backend and then start forwarding the data immediatly (flushing the prefetched ones) ? Now I'm confused. Are we talking about flushing everything immediately from client to backend (that's what I understood from your first patch proposal in this thread) or are we talking about pre fetching the whole request body from the client and then passing it on to the backend as a whole ? Sorry, I really should not have proposed 2 patches in the same thread without being clear about their different purposes. I first thought flushing everything received from the client to the backend immediatly, and disabling *retries* in the 16K-prefetch, would address the issue (my first patch, httpd-trunk-mod_proxy_http_flushall.patch). But as you said in a previous message, that did not address the case where the client and/or an input filter retain the very first body bytes, and mod_proxy_http blocks, and the backend timeouts... Hence the second patch (httpd-trunk-mod_proxy_http_prefetch.patch), which does prefetch *before* establishing/checking the backend's connection, so to minimize (to the minimum) the future delay between that and the first bytes sent to the backend (hence the chances for it to timeout). The prefetch is still up to 16K (same as existing), and not the whole body of course. Then, when it's time to forward (what is available from) the client's request, the patched mod_proxy_http will establish or check+reuse a backend connection and always flush immediatly what it has already (headers + prefetch, see stream_reqbody_* functions). For the remaining request's body chunks (if any), I intentionally did not change the current behaviour of letting the output filters choose when to flush, this is not the keep-alive timeout anymore, so it becomes out of the issue's scope. But I'm definitively +1 to flush-all here too (optionaly), since mod_proxy may retain small chunks and can then face another backend timeout later! That's where patch1 meets patch2, and comes patch3 (httpd-trunk-mod_proxy_http_prefetch+flushall.patch, attached here). There is still the potential (minimal) race condition, the backend could have closed in the (short) meantime, that's indeed *not* addressed by my patch(es). The problem I mentioned in the first message is about treating
Re: mod_proxy, oooled backend connections and the keep-alive race condition
You can't retry all the requests (particularly non idempotent ones), not even once. Suppose it is a charged $100 order, you wouldn't like any proxy to double that because of network problems... I'm not talking about retrying requests but retrying writing on the socket after trying to re-open a connection. When mod_proxy tries to forward the client request to the backends and encounters a closed connection due to the described race condition it will fail back to the client. Instead, I suggest trying to re-open the connection once and then either send the client request over that connection or go back to the client with an error. No double-sending requests anywhere. Same as above, this can't be done (even less at the connection level). There is really no overall solution, IMHO. I doubt that statement is correct. As I said before, I've seen it work with another proxy so the basic approach is no problem. You might be correct in that it's not possible with apache but I cannot confirm or disconfirm this until I have tried adding it. Anyway, this is getting us nowhere until I (or anyone) come(s) up with a corresponding patch. I hope I'll get around for it sometime until January. Until then I'll just see this as declined. On Thu, Dec 5, 2013 at 5:26 PM, Yann Ylavic ylavic@gmail.com wrote: On Thu, Dec 5, 2013 at 5:04 PM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: It also seems that it adds more cycles to Apache on the front to reduce a race condition that can't really be removed. While it's true that the race condition itself cannot be avoided we can definitely work around the resulting problem situation, e.g. by trying to open the connection again once and then either send the data or fail back to the client as we do right now. The approach itself is not theoretical: I know that's exactly how certain forward proxies solve the problem. You can't retry all the requests (particularly non idempotent ones), not even once. Suppose it is a charged $100 order, you wouldn't like any proxy to double that because of network problems... Plus, if we do this here, shouldn't we do it for all proxy protocol submodules (fcgi, etc...)? I go with my original suggestion: Have the above mentioned retry-once mechanism apply on connection level so that request modules don't even notice. Otherwise I would agree in having it added to the appropriate modules. I would like to come up with the appropriate patch but I doubt I'll have the time for it anytime soon :-( Same as above, this can't be done (even less at the connection level). There is really no overall solution, IMHO. As it stands I think Yann's approach in flushing through the reverse proxy is better then doing nothing. Though I expect that to be useless for a lot of deployments where stuff like AV scanning or mod_security will need to buffer the request bodies - then you will have the original problem again. My first approach was to flush only, but the latest patch take also care about buffered request, the backend won't be connected/reused until the bytes are in mod_proxy.
Re: unsetting encrypted cookies when encryption key changes
So it should work out of the box. I figured as much but was unsure whether I hit a bug or forgot a configuration directive. Will look into it once I have the time :-/ On Sun, Dec 8, 2013 at 2:42 PM, Graham Leggett minf...@sharp.fm wrote: On 04 Dec 2013, at 11:53 AM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: The encrypted session cookie, sent out in step 4, is never changed. I can not see any Set-Cookie headers coming from apache, not even in step 10. That is definitely a bug - if the session is decrypted with any key other than the key that will be used for encryption, the session must be marked as dirty so the session gets rewritten. Regards, Graham --
Re: make mod_auth_form tell you where the credentials came from
Or why is it in 34 that the provider's internal session is expired? I don't think it matters why the session is not valid anymore, as we cannot make assumptions to the logic someone wants to implement in their custom auth provider. In the example, however, you can just assume a time out. I think what is missing here is something between 32 and 33 that re-validates the custom auth provider's internal session, whenever valid user credentials where received from a filled form. That's what I aimed for with the patch. Because the auth provider should behave differently depending on whether the user just submitted a filled-in form or the user's client simply sent the session cookie, that information has to be there. I figured the r-notes table to be the 'easiest' place to pull it from. If there exists a way to pull/derive that information from without the patch, I would like to know. On Sun, Dec 8, 2013 at 12:33 PM, Micha Lenk mi...@lenk.info wrote: Hi Thomas, Am 03.12.2013 18:04, schrieb Thomas Eckert: Now suppose the following [...] 32 user fills in and submits form 32 custom auth provider receives the user credentials 33 custom auth provider looks up it's own session in it's module internal session cache 34 custom auth provider realizes the provider internal session expired I think what is missing here is something between 32 and 33 that re-validates the custom auth provider's internal session, whenever valid user credentials where received from a filled form. Or why is it in 34 that the provider's internal session is expired? Regards, Micha
Re: unsetting encrypted cookies when encryption key changes
The patch does not help but I think it got me on the right track though I'm
a bit confused about the 'dirty' flag. Where is that flag supposed to be
used ? In both trunk and 2.4.7 I only found one place
(./modules/session/mod_session.c:200) where that flag is used but none that
remotely looked like triggering a session/cookie replacing.
I assume the real problem lies in mod_session's ap_session_load(). There
the comment says If the session doesn't exist, a blank one will be
created. but that's simply not true if the session decryption failed.
Watching the session_rec pointer it will be NULL even after calling
ap_session_load(). This holds true for the subsequent calls to
ap_session_get() in get_session_auth() as well.If you look at mod_session's
ap_session_load() it's obvious why: The session_rec pointer z will never be
set if decoding the session failed.
I would expect a failure to decode the session to result in some form of
reset on the session so the surrounding code can go ahead and set
everything as required, including encrypting the session with a new key.
Looking at mod_auth_form's get_session_auth() this seems to be the
assumption there.
I thought of something as simple as
diff --git a/modules/session/mod_session.c b/modules/session/mod_session.c
index a3354a5..c3c2f27 100644
--- a/modules/session/mod_session.c
+++ b/modules/session/mod_session.c
@@ -142,6 +142,15 @@ static apr_status_t ap_session_load(request_rec * r,
session_rec ** z)
ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01817)
error while decoding the session,
session not loaded: %s, r-uri);
+
+/* no luck, create a blank session */
+zz = (session_rec *) apr_pcalloc(r-pool, sizeof(session_rec));
+zz-pool = r-pool;
+zz-entries = apr_table_make(zz-pool, 10);
+zz-uuid = (apr_uuid_t *) apr_pcalloc(zz-pool,
sizeof(apr_uuid_t));
+apr_uuid_get(zz-uuid);
+*z = zz;
+
return rv;
}
}
but that didn't do the trick. Going to take another look at it tomorrow.
On Thu, Dec 12, 2013 at 12:25 AM, Graham Leggett minf...@sharp.fm wrote:
On 09 Dec 2013, at 10:50 AM, Thomas Eckert thomas.r.w.eck...@gmail.com
wrote:
So it should work out of the box. I figured as much but was unsure
whether I hit a bug or forgot a configuration directive. Will look into it
once I have the time :-/
Here is an untested patch, can you give it a try and confirm?
Index: modules/session/mod_session_crypto.c
===
--- modules/session/mod_session_crypto.c(revision 1550312)
+++ modules/session/mod_session_crypto.c(working copy)
@@ -222,7 +222,7 @@
* Returns APR_SUCCESS if successful.
*/
static apr_status_t decrypt_string(request_rec * r, const apr_crypto_t *f,
-session_crypto_dir_conf *dconf, const char *in, char **out)
+session_crypto_dir_conf *dconf, const char *in, char **out, int
*dirty)
{
apr_status_t res;
apr_crypto_key_t *key = NULL;
@@ -252,6 +252,9 @@
apr_size_t len = decodedlen;
char *slider = decoded;
+/* if not first passphrase, mark the session as dirty */
+*dirty = *dirty (i 0);
+
/* encrypt using the first passphrase in the list */
res = apr_crypto_passphrase(key, ivSize, passphrase,
strlen(passphrase),
@@ -382,7 +385,7 @@
if ((dconf-passphrases_set) z-encoded *z-encoded) {
apr_pool_userdata_get((void **)f, CRYPTO_KEY,
r-server-process-pconf);
-res = decrypt_string(r, f, dconf, z-encoded, encoded);
+res = decrypt_string(r, f, dconf, z-encoded, encoded,
z-dirty);
if (res != APR_SUCCESS) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, res, r, APLOGNO(01842)
decrypt session failed, wrong passphrase?);
Regards,
Graham
--
Re: unsetting encrypted cookies when encryption key changes
Must have made some mistake when testing it yesterday because it works like
a charm. Suggesting this patch (against trunk)
diff --git a/modules/session/mod_session.c b/modules/session/mod_session.c
index 89c8074..476e021 100644
--- a/modules/session/mod_session.c
+++ b/modules/session/mod_session.c
@@ -126,22 +126,28 @@ static apr_status_t ap_session_load(request_rec * r,
session_rec ** z)
/* found a session that hasn't expired? */
now = apr_time_now();
-if (!zz || (zz-expiry zz-expiry now)) {
+if (zz) {
+if (zz-expiry zz-expiry now) {
+zz = NULL;
+}
+else {
+/* having a session we cannot decode is just as good as having
+ none at all */
+rv = ap_run_session_decode(r, zz);
+if (OK != rv) {
+ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01817)
+ error while decoding the session,
+ session not loaded: %s, r-uri);
+zz = NULL;
+}
+}
+}
-/* no luck, create a blank session */
+/* no luck, create a blank session */
+if (!zz) {
zz = (session_rec *) apr_pcalloc(r-pool, sizeof(session_rec));
zz-pool = r-pool;
zz-entries = apr_table_make(zz-pool, 10);
-
-}
-else {
-rv = ap_run_session_decode(r, zz);
-if (OK != rv) {
-ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01817)
- error while decoding the session,
- session not loaded: %s, r-uri);
-return rv;
-}
}
/* make sure the expiry and maxage are set, if present */
On Thu, Dec 12, 2013 at 10:11 PM, Tom Evans tevans...@googlemail.comwrote:
On Thu, Dec 12, 2013 at 7:30 PM, Graham Leggett minf...@sharp.fm wrote:
On 12 Dec 2013, at 16:57, Thomas Eckert thomas.r.w.eck...@gmail.com
wrote:
The patch does not help but I think it got me on the right track though
I'm a bit confused about the 'dirty' flag. Where is that flag supposed to
be used ? In both trunk and 2.4.7 I only found one place
(./modules/session/mod_session.c:200) where that flag is used but none that
remotely looked like triggering a session/cookie replacing.
I assume the real problem lies in mod_session's ap_session_load().
There the comment says If the session doesn't exist, a blank one will be
created. but that's simply not true if the session decryption failed.
Can you clarify what you mean by session decryption failed?
When the request has a session cookie present, but the contents are
corrupted or in any way incorrect, then decoding the cookie fails.
When this occurs, no new session is created.
Since no new session is created, no new cookie is set.
(I think!)
Cheers
Tom
Re: Question about Proxy ~ something syntax
Why offer an option to a directive which makes it behave differently AND
have an explicit directive for that exact behaviour ? I see no gain from
this but a lot of potential harm in terms of user confusion. Better be
explicit and unambiguous - 2)
On Mon, Dec 16, 2013 at 6:37 AM, Christophe JAILLET
christophe.jail...@wanadoo.fr wrote:
Hi,
I was about to commit a patch for mod_proxy.c, but around line 2230
else if (!strcmp(cmd-path, ~)) {
cmd-path = ap_getword_conf(cmd-pool, arg);
if (!cmd-path)
return Proxy ~ block must specify a path;
!cmd-path should be !cmd-path[0] because 'ap_getword_conf' does not
return NULL but an empty string, so the test can never be true.
However, I've searched in the doc to get explanation of this '~' in Proxy
block.
There is nothing about it.
Apparently Proxy ~ xx is equivalent to ProxyMatch xx
So, should we:
1) just leave it as it is.
2) remove this behavior, which has never been documented.
3) update doc.
My choice is 2) because
- it has never been documented, so it is likely that no one use it
- apparently it is equivalent to ProxyMatch
Your opinion ?
Best regards,
CJ
Revisiting: xml2enc, mod_proxy_html and content compression
I've been over this with Nick before: mod_proxy_html uses mod_xml2enc to do
the detection magic but mod_xml2enc fails to detect compressed content
correctly. Hence a simple ProxyHTMLEnable fails when content compression
is in place.
To work around this without dropping support for content compression you
can do
SetOutputfilter INFLATE;xml2enc;proxy-html;DEFLATE
or at least that was the kind-of-result of the half-finished discussion
last time.
Aside from being plain ugly and troublesome to use (e.g. if you want to use
AddOutputfilter somewhere else) the above also has a major shortcoming,
which lies with already-compressed content.
Suppose the client does
GET /something.tar.gz HTTP/1.1
...
Accept-Encoding: gzip, deflate
to which the backend will respond with 200 but *not* send an
Content-Encoding header since the content is already encoded. Using the
above filter chain corrupts the content because it will be inflated and
then deflated, double compressing it in the end.
Imho this whole issue lies with proxy_html using xml2enc to do the content
type detection and xml2enc failing to detect the content encoding. I guess
all it really takes is to have xml2enc inspect the headers_in to see if
there is a Content-Encoding header and then add the inflate/deflate
filters (unless there is a general reason not to rely on the input headers,
see below).
Adding the inflate/deflate filters inside xml2enc is where I need some
advice. For the deflate part I can probably do something like
const char *compression_method = apr_table_get(f-r-headers_in,
Content-Encoding);
if (compression_method != NULL
strncasecmp(compression_method, gzip, 4) == 0) {
ap_add_output_filter(deflate, NULL, r, NULL);
}
but what about the inflate part ? I can't simply add the inflate input
filter because at that point (in mod_xml2enc's xml2enc_ffunc() ) I would
then need to go back in the input filter chain which is afaik not
possible. So I would have to run the inflate input filter in place.
Of course, this whole issue would disappear if inflate/deflate would be run
automagically (upon seeing a Content-Encoding header) in general. Anyway,
what's the reasoning behind not having them run always and give them the
knowledge (e.g. about the input headers) to get out of the way if necessary
?
Re: Revisiting: xml2enc, mod_proxy_html and content compression
Aha! Revisiting that, I see I still have an uncommitted patch to make
content types to process configurable. I think that was an issue you
originally raised? But compression is another issue.
Yep.
Hmmm?
If the backend sends compressed contents with no content-encoding,
doesn't that imply:
1. INFLATE doesn't see encoding, so steps away.
2. xml2enc and proxy-html can't parse compressed content, so step away
(log an error?)
3. DEFLATE … aha, that's what you meant about double-compression.
In effect the whole chain was reduced to just DEFLATE. That's a bit
nonsensical
but not incorrect, and the user-agent will reverse the DEFLATE and
restore the
original from the backend, yesno?
I think you are right. Yet, when using FF or Chrome (both in the latest
versions) the final result is 'double compressed' nonetheless. Repeating
the steps 'manually' (curl + gzip) it's all good, meaning the original file
from the server is restored as it should be. I'm reluctant to blame the
clients however.
But is the real issue anything more than an inability to use
ProxyHTMLEnable
with compressed contents? In which case, wouldn't mod_proxy_html be the
place to patch? Have it test/insert deflate at the same point as it
inserts xml2enc?
No, yes and I tried but couldn't get it to work. Following your advice I
went along the lines of
diff --git a/modules/filters/mod_proxy_html.c
b/modules/filters/mod_proxy_html.c
index b964fec..9760115 100644
--- a/modules/filters/mod_proxy_html.c
+++ b/modules/filters/mod_proxy_html.c
@@ -1569,10 +1569,19 @@ static void proxy_html_insert(request_rec *r)
proxy_html_conf *cfg;
cfg = ap_get_module_config(r-per_dir_config, proxy_html_module);
if (cfg-enabled) {
-if (xml2enc_filter)
+int add_deflate_output_filter = 0;
+if (apr_table_get(r-headers_in, Content-Encoding:) != NULL) {
+ap_add_input_filter(inflate, NULL, r, r-connection);
+add_deflate_output_filter = 1;
+}
+if (xml2enc_filter) {
xml2enc_filter(r, NULL, ENCIO_INPUT_CHECKS);
+}
ap_add_output_filter(proxy-html, NULL, r, r-connection);
ap_add_output_filter(proxy-css, NULL, r, r-connection);
+if (add_deflate_output_filter) {
+ap_add_output_filter(deflate, NULL, r, r-connection);
+}
}
}
static void proxy_html_hooks(apr_pool_t *p)
but it appears to be way off because it does exactly nothing. When logging
the headers at this point, I found r-headers_in to contain the client
request whereas r-headers_out was empty. Doesn't this tell me I'm doing
all of this too early ?
On Tue, Dec 17, 2013 at 12:47 PM, Nick Kew n...@webthing.com wrote:
On 17 Dec 2013, at 10:32, Thomas Eckert wrote:
I've been over this with Nick before: mod_proxy_html uses mod_xml2enc to
do the detection magic but mod_xml2enc fails to detect compressed content
correctly. Hence a simple ProxyHTMLEnable fails when content compression
is in place.
Aha! Revisiting that, I see I still have an uncommitted patch to make
content types to process configurable. I think that was an issue you
originally raised? But compression is another issue.
To work around this without dropping support for content compression you
can do
SetOutputfilter INFLATE;xml2enc;proxy-html;DEFLATE
or at least that was the kind-of-result of the half-finished discussion
last time.
I didn't find that discussion. But I suspect my reaction would have
included
a certain aversion to that level of processing overhead in the proxy in
these
days of fatter pipes and hardware compression.
Suppose the client does
GET /something.tar.gz HTTP/1.1
...
Accept-Encoding: gzip, deflate
to which the backend will respond with 200 but *not* send an
Content-Encoding header since the content is already encoded. Using the
above filter chain corrupts the content because it will be inflated and
then deflated, double compressing it in the end.
Hmmm?
If the backend sends compressed contents with no content-encoding, doesn't
that imply:
1. INFLATE doesn't see encoding, so steps away.
2. xml2enc and proxy-html can't parse compressed content, so step away
(log an error?)
3. DEFLATE … aha, that's what you meant about double-compression.
In effect the whole chain was reduced to just DEFLATE. That's a bit
nonsensical
but not incorrect, and the user-agent will reverse the DEFLATE and restore
the
original from the backend, yesno?
Imho this whole issue lies with proxy_html using xml2enc to do the
content type detection and xml2enc failing to detect the content encoding.
I guess all it really takes is to have xml2enc inspect the headers_in to
see if there is a Content-Encoding header and then add the
inflate/deflate filters (unless there is a general reason not to rely on
the input headers, see below).
Well in this particular case, surely it lies with the backend?
But is the real issue anything
Re: Revisiting: xml2enc, mod_proxy_html and content compression
After applying
@@ -1569,10 +1579,13 @@ static void proxy_html_insert(request_rec *r)
proxy_html_conf *cfg;
cfg = ap_get_module_config(r-per_dir_config, proxy_html_module);
if (cfg-enabled) {
-if (xml2enc_filter)
+ap_add_output_filter(INFLATE, NULL, r, r-connection);
+if (xml2enc_filter) {
xml2enc_filter(r, NULL, ENCIO_INPUT_CHECKS);
+}
ap_add_output_filter(proxy-html, NULL, r, r-connection);
ap_add_output_filter(proxy-css, NULL, r, r-connection);
+ap_add_output_filter(DEFLATE, NULL, r, r-connection);
}
}
a simple
ProxyHTMLEnable On
will do the trick for simple text/html but I did have to remove the
mod_deflate config (see further down). This does not solve the problem
regarding .gz files however. They still suffer from a double-compression.
Using the above patch/configuration we could either
1) patch mod_deflate to bail out when it sees a .gz file
or
2) patch mod_proxy_html (in the above mentioned section) to bail out if
it sees a .gz file.
I cannot think of a situation where we would actually want to HTTP
compress a .gz file. There might also be other formats then gzip invovled
- at least the RFC allows for them, though I've only seen gzip in the wild.
For there two reasons I would to with 1).
In order to get the above patch working I also had to remove
AddOutputFilterByType DEFLATE text/html text/plain text/xml
AddOutputFilterByType DEFLATE text/css
AddOutputFilterByType DEFLATE application/x-javascript
application/javascript application/ecmascript
AddOutputFilterByType DEFLATE application/rss+xml
from the (global) configuration because the compression would kick in
*before* mod_xml2enc was called for the second time in the output filter
chain. This makes mod_xml2enc see compressed content and fail. Here's how
the output filter chain looks like at different points in time:
called: inflate_out_filter()
output filters:
inflate
xml2enc
proxy-html
proxy-css
BYTYPE:DEFLATE
deflate
mod_session_out
byterange
content_length
http_header
http_outerror
core
called: proxy_html_filter()
output filters:
xml2enc
proxy-html
proxy-css
BYTYPE:DEFLATE
deflate
mod_session_out
byterange
content_length
http_header
http_outerror
core
called: proxy_css_filter()
output filters:
xml2enc
proxy-html
proxy-css
BYTYPE:DEFLATE
xml2enc
deflate
mod_session_out
byterange
content_length
http_header
http_outerror
core
How do I move the second pass to xml2enc before BYTYPE:DEFLATE ? I'm not
aware of a variant of ap_add_output_filter() which lets one adjust the
position of the to-insert filter.
Solving this problem would allow to remove the call to
ap_add_output_filter() in the above patch, which in turn allows for nice
and clean configurations (e.g. by using the example config of mod_deflate)
as well as allowing the reverseproxy to do HTTP compression even if the
backend did not choose to do so.
On Thu, Dec 19, 2013 at 4:01 AM, Nick Kew n...@webthing.com wrote:
On 18 Dec 2013, at 14:47, Thomas Eckert wrote:
No, yes and I tried but couldn't get it to work. Following your advice I
went along the lines of
Yes, I'd be trying something like that. You can insert inflate (and
deflate)
unconditionally, as they will check the headers themselves and remove
themselves if appropriate.
But I'd make at least the deflate component of that configurable:
many sysops may prefer to sacrifice compression to avoid that
unnecessary overhead.
--
Nick Kew
Re: Revisiting: xml2enc, mod_proxy_html and content compression
IIRC the OP wants to decompress such contents and run them through mod_proxy_html. I don't think that works with any sane setup: running non-HTML content-types through proxy_html will always be an at-your-own-risk hack. What I want is a (preferrably as simple as possible) method of configuring mod_proxy_html in such a way that it will attempt to rewrite html(/css/js) content even if the content was delivered in a compressed format by the backend server. In my opinion the part about compression should actually be done transparently (to the user) by mod_proxy_html/mod_deflate. The reason I brought the .gz files up as example is because they were handled sligthly incorrect (unnecessary overhead + unpleasant side effect on client side). Gzip compressed content sometimes gets served with no declared encoding and a media type of, e.g., “application/x-gzip”. I reckon that's more common than serving it as application/octet-stream or with no Content-Type: declared. mod_deflate could use this information to avoid compressing the response, and without sniffing the content. Exactly what I'm aiming for. I think that's the way to go here, see '1)' in my previous reply. In this case we should also make mod_xml2enc bail out with corresponding log message when it gets to see compressed content, e.g. either via env variable set by inflate filter or read Content-Type header, so all of the involved modules act consistently and their log output will not be misunderstood as errors. This more limited approach is already available through configuration, so maybe the way to handle this is via a change to documentation / default configuration, rather than code. In order to make mod_proxy_html work with possibly compressed contents you cannot simply do a ProxyHTMLEnable On and what I have been using since the last discussion which I mentioned before is SetOutputFilter inflate;xml2enc;proxy-html;deflate with no other explicit configuration of mod_deflate. I'm aware of AddOutputFilterByType DEFLATE text/html text/plain text/xml AddOutputFilterByType DEFLATE text/css AddOutputFilterByType DEFLATE application/x-javascript application/javascript application/ecmascript AddOutputFilterByType DEFLATE application/rss+xml but this is not compatible with the above output filter chain (see my previous reply). Maybe one is able to disable output compression on already-compressed content with a smart If like block but do we really want this as default configuration ? Is there ever a case where someone does *NOT* want mod_proxy_html and friends to handle compression transparently ? On Sun, Jan 5, 2014 at 2:57 PM, Tim Bannister is...@jellybaby.net wrote: On 5 Jan 2014, at 02:21, Nick Kew wrote: IIRC the OP wants to decompress such contents and run them through mod_proxy_html. I don't think that works with any sane setup: running non-HTML content-types through proxy_html will always be an at-your-own-risk hack. I've believed for a while that the right way to address this is for httpd to support gzip Transfer-Encoding which is always hop-by-hop and applies to the transfer rather than the entity being transferred. For this scenario, it could look like this: [Client] ⇦ gzip content-encoding ⇦ [transforming reverse proxy] ⇦ gzip,chunked transfer-encodings ⇦ [origin server] (I'm assuming that the client doesn't negotiate gzip transfer encoding) Of course, this still won't help with a badly-configured origin server. -- Tim Bannister – is...@jellybaby.net
Re: unsetting encrypted cookies when encryption key changes
I've had this deployed for some time now and it works just fine. Did this
just fall asleep or is further explanation desired ?
On Fri, Dec 13, 2013 at 9:10 AM, Thomas Eckert
thomas.r.w.eck...@gmail.comwrote:
Must have made some mistake when testing it yesterday because it works
like a charm. Suggesting this patch (against trunk)
diff --git a/modules/session/mod_session.c b/modules/session/mod_session.c
index 89c8074..476e021 100644
--- a/modules/session/mod_session.c
+++ b/modules/session/mod_session.c
@@ -126,22 +126,28 @@ static apr_status_t ap_session_load(request_rec * r,
session_rec ** z)
/* found a session that hasn't expired? */
now = apr_time_now();
-if (!zz || (zz-expiry zz-expiry now)) {
+if (zz) {
+if (zz-expiry zz-expiry now) {
+zz = NULL;
+}
+else {
+/* having a session we cannot decode is just as good as having
+ none at all */
+rv = ap_run_session_decode(r, zz);
+if (OK != rv) {
+ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01817)
+ error while decoding the session,
+ session not loaded: %s, r-uri);
+zz = NULL;
+}
+}
+}
-/* no luck, create a blank session */
+/* no luck, create a blank session */
+if (!zz) {
zz = (session_rec *) apr_pcalloc(r-pool, sizeof(session_rec));
zz-pool = r-pool;
zz-entries = apr_table_make(zz-pool, 10);
-
-}
-else {
-rv = ap_run_session_decode(r, zz);
-if (OK != rv) {
-ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01817)
- error while decoding the session,
- session not loaded: %s, r-uri);
-return rv;
-}
}
/* make sure the expiry and maxage are set, if present */
On Thu, Dec 12, 2013 at 10:11 PM, Tom Evans tevans...@googlemail.comwrote:
On Thu, Dec 12, 2013 at 7:30 PM, Graham Leggett minf...@sharp.fm wrote:
On 12 Dec 2013, at 16:57, Thomas Eckert thomas.r.w.eck...@gmail.com
wrote:
The patch does not help but I think it got me on the right track
though I'm a bit confused about the 'dirty' flag. Where is that flag
supposed to be used ? In both trunk and 2.4.7 I only found one place
(./modules/session/mod_session.c:200) where that flag is used but none that
remotely looked like triggering a session/cookie replacing.
I assume the real problem lies in mod_session's ap_session_load().
There the comment says If the session doesn't exist, a blank one will be
created. but that's simply not true if the session decryption failed.
Can you clarify what you mean by session decryption failed?
When the request has a session cookie present, but the contents are
corrupted or in any way incorrect, then decoding the cookie fails.
When this occurs, no new session is created.
Since no new session is created, no new cookie is set.
(I think!)
Cheers
Tom
Re: Revisiting: xml2enc, mod_proxy_html and content compression
Here is what I ended up with.
diff --git a/modules/filters/mod_deflate.c b/modules/filters/mod_deflate.c
index 605c158..fd3662a 100644
--- a/modules/filters/mod_deflate.c
+++ b/modules/filters/mod_deflate.c
@@ -450,6 +450,12 @@ static apr_status_t deflate_out_filter(ap_filter_t *f,
return APR_SUCCESS;
}
+if (!strncasecmp(f-r-content_type, application/x-gzip, 18)) {
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f-r, not going to
compress application/x-gzip content);
+ ap_remove_output_filter(f);
+ return ap_pass_brigade(f-next, bb);
+}
+
c = ap_get_module_config(r-server-module_config,
deflate_module);
@@ -1162,7 +1168,6 @@ static apr_status_t deflate_in_filter(ap_filter_t *f,
return APR_SUCCESS;
}
-
/* Filter to inflate for a content-transforming proxy. */
static apr_status_t inflate_out_filter(ap_filter_t *f,
apr_bucket_brigade *bb)
@@ -1181,6 +1186,12 @@ static apr_status_t inflate_out_filter(ap_filter_t
*f,
return APR_SUCCESS;
}
+if (!strncasecmp(f-r-content_type, application/x-gzip, 18)) {
+ ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, f-r, not going to
decompress application/x-gzip content);
+ ap_remove_output_filter(f);
+ return ap_pass_brigade(f-next, bb);
+}
+
c = ap_get_module_config(r-server-module_config, deflate_module);
if (!ctx) {
diff --git a/modules/filters/mod_proxy_html.c
b/modules/filters/mod_proxy_html.c
index b964fec..61834ff 100644
--- a/modules/filters/mod_proxy_html.c
+++ b/modules/filters/mod_proxy_html.c
@@ -107,6 +107,8 @@ typedef struct {
int strip_comments;
int interp;
int enabled;
+int inflate;
+int deflate;
} proxy_html_conf;
typedef struct {
ap_filter_t *f;
@@ -1322,6 +1324,8 @@ static void *proxy_html_merge(apr_pool_t *pool, void
*BASE, void *ADD)
conf-interp = add-interp;
conf-strip_comments = add-strip_comments;
conf-enabled = add-enabled;
+conf-inflate = add-inflate;
+conf-deflate = add-deflate;
}
else {
conf-flags = base-flags | add-flags;
@@ -1330,6 +1334,8 @@ static void *proxy_html_merge(apr_pool_t *pool, void
*BASE, void *ADD)
conf-interp = base-interp | add-interp;
conf-strip_comments = base-strip_comments | add-strip_comments;
conf-enabled = add-enabled | base-enabled;
+conf-inflate = add-inflate | base-inflate;
+conf-deflate = add-deflate | base-deflate;
}
return conf;
}
@@ -1537,6 +1543,14 @@ static const command_rec proxy_html_cmds[] = {
(void*)APR_OFFSETOF(proxy_html_conf, enabled),
RSRC_CONF|ACCESS_CONF,
Enable proxy-html and xml2enc filters),
+AP_INIT_FLAG(ProxyHTMLInflate, ap_set_flag_slot,
+(void*)APR_OFFSETOF(proxy_html_conf, inflate),
+RSRC_CONF|ACCESS_CONF,
+Will inflate compressed content before rewriting),
+AP_INIT_FLAG(ProxyHTMLDeflate, ap_set_flag_slot,
+(void*)APR_OFFSETOF(proxy_html_conf, deflate),
+RSRC_CONF|ACCESS_CONF,
+Will deflate content after rewriting),
{ NULL }
};
static int mod_proxy_html(apr_pool_t *p, apr_pool_t *p1, apr_pool_t *p2)
@@ -1569,10 +1583,16 @@ static void proxy_html_insert(request_rec *r)
proxy_html_conf *cfg;
cfg = ap_get_module_config(r-per_dir_config, proxy_html_module);
if (cfg-enabled) {
+if (cfg-inflate) {
+ ap_add_output_filter(inflate, NULL, r, r-connection);
+}
if (xml2enc_filter)
xml2enc_filter(r, NULL, ENCIO_INPUT_CHECKS);
ap_add_output_filter(proxy-html, NULL, r, r-connection);
ap_add_output_filter(proxy-css, NULL, r, r-connection);
+if (cfg-deflate) {
+ ap_add_output_filter(deflate, NULL, r, r-connection);
+}
}
}
static void proxy_html_hooks(apr_pool_t *p)
The diffs are obviously not against trunk/2.4.x since they are just meant
to show what I have in mind. I'm still worried about the mod_xml2enc
though. Seeing how it inserts itself into the output filter chain, above
mod_proxy_html patch might actually result in xml2enc attaching itself
*behind* deflate - which is bad. I haven't figured out how to work around
this yet. Any suggestions on how to do this ?
In general, is this a sensible way to approach the proxy-html/compression
issue in your opinion ?
On Tue, Jan 14, 2014 at 2:08 PM, Thomas Eckert
thomas.r.w.eck...@gmail.comwrote:
IIRC the OP wants to decompress such contents and run them
through mod_proxy_html. I don't think that works with any sane
setup: running non-HTML content-types through proxy_html
will always be an at-your-own-risk hack.
What I want is a (preferrably as simple as possible) method of configuring
mod_proxy_html in such a way that it will attempt to rewrite html(/css/js)
content even
mod_alias' Redirect with dynamic host
Some time ago I put up HTTP to HTTPS redirects in place which now needed an
update so they would not only work for constant host names but use the
'Host' header information as target host.
So a simple
Redirect permanent / https://example.org/
wasn't enough. I wanted to avoid using mod_rewrite (not included in my
configs so far anyway) and stick with the much simpler mod_alias so I read
through mod_alias.c. From what I could see there wasn't any means to do get
this working so I came up with
diff --git a/modules/mappers/mod_alias.c b/modules/mappers/mod_alias.c
index 0740cef..b73d262 100644
--- a/modules/mappers/mod_alias.c
+++ b/modules/mappers/mod_alias.c
@@ -42,6 +42,7 @@ typedef struct {
char *handler;
ap_regex_t *regexp;
int redir_status;/* 301, 302, 303, 410, etc */
+int ssl_redir;
} alias_entry;
typedef struct {
@@ -169,7 +170,8 @@ static const char *add_alias_regex(cmd_parms *cmd, void
*dummy,
static const char *add_redirect_internal(cmd_parms *cmd,
alias_dir_conf *dirconf,
const char *arg1, const char
*arg2,
- const char *arg3, int use_regex)
+ const char *arg3, int use_regex,
+ int ssl_redir)
{
alias_entry *new;
server_rec *s = cmd-server;
@@ -222,13 +224,17 @@ static const char *add_redirect_internal(cmd_parms
*cmd,
}
if (ap_is_HTTP_REDIRECT(status)) {
-if (!url)
-return URL to redirect to is missing;
-/* PR#35314: we can allow path components here;
- * they get correctly resolved to full URLs.
- */
-if (!use_regex !ap_is_url(url) (url[0] != '/'))
-return Redirect to non-URL;
+if (ssl_redir) {
+url = apr_pstrdup(cmd-pool, debugging place holder);
+} else {
+if (!url)
+return URL to redirect to is missing;
+/* PR#35314: we can allow path components here;
+ * they get correctly resolved to full URLs.
+ */
+if (!use_regex !ap_is_url(url) (url[0] != '/'))
+return Redirect to non-URL;
+}
}
else {
if (url)
@@ -244,6 +250,7 @@ static const char *add_redirect_internal(cmd_parms *cmd,
new-real = url;
new-regexp = regex;
new-redir_status = status;
+new-ssl_redir = ssl_redir;
return NULL;
}
@@ -251,20 +258,27 @@ static const char *add_redirect(cmd_parms *cmd, void
*dirconf,
const char *arg1, const char *arg2,
const char *arg3)
{
-return add_redirect_internal(cmd, dirconf, arg1, arg2, arg3, 0);
+return add_redirect_internal(cmd, dirconf, arg1, arg2, arg3, 0, 0);
+}
+
+static const char *add_redirect_ssl(cmd_parms *cmd, void *dirconf,
+const char *arg1, const char *arg2)
+{
+return add_redirect_internal(cmd, dirconf, arg1, arg2, NULL, 0, 1);
}
+
static const char *add_redirect2(cmd_parms *cmd, void *dirconf,
const char *arg1, const char *arg2)
{
-return add_redirect_internal(cmd, dirconf, arg1, arg2, NULL, 0);
+return add_redirect_internal(cmd, dirconf, arg1, arg2, NULL, 0, 0);
}
static const char *add_redirect_regex(cmd_parms *cmd, void *dirconf,
const char *arg1, const char *arg2,
const char *arg3)
{
-return add_redirect_internal(cmd, dirconf, arg1, arg2, arg3, 1);
+return add_redirect_internal(cmd, dirconf, arg1, arg2, arg3, 1, 0);
}
static const command_rec alias_cmds[] =
@@ -277,6 +291,8 @@ static const command_rec alias_cmds[] =
OR_FILEINFO,
an optional status, then document to be redirected and
destination URL),
+AP_INIT_TAKE2(RedirectSSL, add_redirect_ssl, NULL, OR_FILEINFO,
+ Add Host Header based redirect using HTTPS),
AP_INIT_TAKE2(AliasMatch, add_alias_regex, NULL, RSRC_CONF,
a regular expression and a filename),
AP_INIT_TAKE2(ScriptAliasMatch, add_alias_regex, cgi-script,
RSRC_CONF,
@@ -403,11 +419,14 @@ static char *try_alias_list(request_rec *r,
apr_array_header_t *aliases,
if (is_redir) {
char *escurl;
escurl = ap_os_escape_path(r-pool, r-uri + l, 1);
-
-found = apr_pstrcat(r-pool, alias-real, escurl,
NULL);
-}
-else
+if (alias-ssl_redir) {
+found = apr_pstrcat(r-pool, https://;,
apr_table_get(r-headers_in, Host), alias-fake, escurl, NULL);
+} else {
+found = apr_pstrcat(r-pool, alias-real, escurl,
NULL);
+}
+
Re: mod_alias' Redirect with dynamic host
I remember a discussion about general support for this kind of expression
parsing after I asked for it via IRC/list but cannot remember what became
of it. It would definitely be neat to have that kind of thing in there -
much less copy-n-paste like config sections ! Glad to hear there's progress
on this front. Thanks !
On Wed, Jan 22, 2014 at 4:42 PM, Graham Leggett minf...@sharp.fm wrote:
On 22 Jan 2014, at 5:36 PM, Thomas Eckert thomas.r.w.eck...@gmail.com
wrote:
Some time ago I put up HTTP to HTTPS redirects in place which now needed
an update so they would not only work for constant host names but use the
'Host' header information as target host.
So a simple
Redirect permanent / https://example.org/
wasn't enough. I wanted to avoid using mod_rewrite (not included in my
configs so far anyway) and stick with the much simpler mod_alias so I read
through mod_alias.c. From what I could see there wasn't any means to do get
this working so I came up with
This looks like a job for the expression parser, ie this:
Redirect permanent / https://%{HOST}/
(Syntax off top of head, probably wrong).
Having done expression parser for the require directive, my next one on
the wishlist was DocumentRoot (to replace the mass virtual hosting module)
followed by mod_alias.
Regards,
Graham
--
Re: Simplifying mod_alias
When doing this please keep in mind there is a huge amount of users out
there who are not developers and who will struggle with something like
LocationMatch ^/foo/(?bar[^/]+)
Alias /var/lib/%{env:MATCH_BAR}/baz
…stuff...
/LocationMatch
As long as they are reusing the same code under the hood, I don't think
there is anything wrong with having redundant directives whose only purpose
is to have easier-to-read configurations.
On Sun, Jan 26, 2014 at 11:19 PM, Reindl Harald h.rei...@thelounge.netwrote:
Am 26.01.2014 23:11, schrieb Graham Leggett:
A look at mod_alias shows it has 7 directives:
• Alias
• AliasMatch
• Redirect
• RedirectMatch
• RedirectPermanent
• RedirectTemp
• ScriptAlias
• ScriptAliasMatch
In theory we only need these three:
• Alias
• Redirect
• ScriptAlias
in real world you would break existing configs like
RedirectMatch 404 ^/.*setup/(.*)$
without handle the non-Match directives also like expressions
which changes behavior you can't remove Match
however, admins will not appreciate such incompatible changes for no
*real* good reason, even not in case of major upgrades
Re: Simplifying mod_alias
Why would they struggle any more than this, which is what they would need
to do for the same config today?
AliasMatch ^/foo/(?bar[^/]+) /var/lib/${1}/baz
LocationMatch ^/foo/(?bar[^/]+)
…stuff...
/LocationMatch
I can't put my finger on it so I guess it's not much of an argument and
more of a gut feeling.
One is a performance problem, which is a nice but not critical. The
second more important problem is that I am hearing from more and more
people that httpd has too many
directives - they look at httpd and they don't know where to start.
I'm always +1 on the speed stuff and you also got a point there about the
number of directives. Overall there really are quite a lot. So cutting down
on them would probably make for a less scary environment for new users and
established ones as well.
I was going to write a novel worth of text but actually I think you are on
to something good :-)
On Mon, Jan 27, 2014 at 9:29 AM, Graham Leggett minf...@sharp.fm wrote:
On 27 Jan 2014, at 9:58 AM, Thomas Eckert thomas.r.w.eck...@gmail.com
wrote:
When doing this please keep in mind there is a huge amount of users out
there who are not developers and who will struggle with something like
LocationMatch ^/foo/(?bar[^/]+)
Alias /var/lib/%{env:MATCH_BAR}/baz
…stuff...
/LocationMatch
Why would they struggle any more than this, which is what they would need
to do for the same config today?
AliasMatch ^/foo/(?bar[^/]+) /var/lib/${1}/baz
LocationMatch ^/foo/(?bar[^/]+)
…stuff...
/LocationMatch
As long as they are reusing the same code under the hood, I don't think
there is anything wrong with having redundant directives whose only purpose
is to have easier-to-read configurations.
They're not reusing the same code under the hood, the code that performs
the Location handling and the code that matches the Aliases are different
code, and in today's code, the Alias is almost always followed by a
Location directive matching the same URL space.
That said there are two problems being solved here.
One is a performance problem, which is a nice but not critical. The second
more important problem is that I am hearing from more and more people that
httpd has too many directives - they look at httpd and they don't know
where to start.
Existing configurations as I said will still work, they would just be
deprecated. But the long term goal would be to remove the duplicated
functionality and slim down the server, with a trimmer, cleaner server in a
v3.x timeframe.
Regards,
Graham
--
Re: unsetting encrypted cookies when encryption key changes
It just woke up - committed in r1560977 and proposed for backport to v2.4.x. Nice, thank you ! Isn't it curious how the expiry is inspected before the session is decoded? Why ? On Fri, Jan 24, 2014 at 9:16 PM, Erik Pearson e...@adaptations.com wrote: Isn't it curious how the expiry is inspected before the session is decoded? On Fri, Jan 24, 2014 at 5:11 AM, Graham Leggett minf...@sharp.fm wrote: On 16 Jan 2014, at 5:15 PM, Thomas Eckert thomas.r.w.eck...@gmail.com wrote: I've had this deployed for some time now and it works just fine. Did this just fall asleep or is further explanation desired ? It just woke up - committed in r1560977 and proposed for backport to v2.4.x. Regards, Graham -- -- Erik Pearson Adaptations ;; web form and function
Re: 2.4.8 This Month
And it's also important to keep Apache httpd-x64 code current with the new Windows Server + Visual Studio versions, since that's the most cases I know of around. Let's not hijack the 2.4.8 TR thread for yet another round of this topic. On Fri, Feb 7, 2014 at 1:18 PM, Eric Covener cove...@gmail.com wrote: And it's also important to keep Apache httpd-x64 code current with the new Windows Server + Visual Studio versions, since that's the most cases I know of around. Is there a particular problem on that front? Contributions are welcome.
Re: Revisiting: xml2enc, mod_proxy_html and content compression
Doesn't
+else {
+/* default - only act if starts-with text/ or contains xml */
+wanted = !strncmp(ctype, text/, 5) || strstr(ctype, xml);
+}
suffer from the same problem as the original code ? So if the user did not
give any xml2Types the default behaviour will hit the same problem as
with http://www.mail-archive.com/dev@httpd.apache.org/msg57029.html (the
mentioned earlier discussion regarding ctypes). IMO, this is especially
important seeing how using the new code requires configuration entries to
be added which a lot of admins will either not be aware of or simply not be
doing. In regards to the cited patch section above, I suggest something
along the lines of my patch suggestion in the first message of the
mentioned thread, e.g. include
!strncmp(ctype, application, 11)
Otherweise it's fine I think. Will test it sometime this week.
On Thu, Feb 6, 2014 at 10:40 AM, Ewald Dieterich
ewald_dieter...@t-online.de wrote:
Thanks for the patch!
On 02/05/2014 02:57 PM, Nick Kew wrote:
The hesitation is because I've been wanting to review the
patch before committing, and round tuits are in woefully
short supply. So I'm attaching it here. I'll take any feedback
from you or other users as a substitute for my own review,
and commit if it works for you without glitches.
Minor glitch: the patch doesn't compile because it uses the unknown
variable cfg in xml2enc_ffunc(). Otherwise it works as advertised.
My wishlist:
* Make the configuration option as powerful as the compiled in fallback so
that you can configure eg. contains xml. But how would you do that?
Support regular expressions?
* Provide a configuration option to blacklist content types so that you
can use the defaults that are compiled in but exclude specific types from
processing (this is how I work around the Sharepoint problem, I simply
exclude content type multipart/related).
