Fwd: nonportable-atomics configure.in setting
Sorry, I had no intention to send this offlist. -- Forwarded message -- From: Yann Ylavic ylavic@gmail.com Date: Wed, Dec 4, 2013 at 10:37 AM Subject: Re: nonportable-atomics configure.in setting To: Daniel Lescohier daniel.lescoh...@cbsi.com On Wed, Dec 4, 2013 at 1:22 AM, Daniel Lescohier daniel.lescoh...@cbsi.comwrote: I see this in configure.in: AC_ARG_ENABLE(nonportable-atomics, [ --enable-nonportable-atomics Use optimized atomic code which may produce nonportable binaries], [if test $enableval = yes; then force_generic_atomics=no else force_generic_atomics=yes fi ], [case $host_cpu in i[[456]]86) force_generic_atomics=yes ;; *) force_generic_atomics=no ;; esac ]) I was wondering why the three host_cpus i486, i586, and i686 have special treatment for the default setting as compared to all other cpu architectures? I don't see any reason since the code in srclib/apr/atomic/unix/ia32.c seems compatible with i386 (cmpxchg starts with i486), and atomic builtins work with gcc-v2+ (at worst USE_ATOMICS_IA32 could be defined for gcc-v1). The issue could have been in apr_atomic_casptr() and apr_atomic_xchgptr(), but APR_SIZEOF_VOIDP is checked to do the right thing with 32bits cpus...
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: nonportable-atomics configure.in setting
On Wed, Dec 4, 2013 at 10:40 AM, Yann Ylavic ylavic@gmail.com wrote: On Wed, Dec 4, 2013 at 1:22 AM, Daniel Lescohier daniel.lescoh...@cbsi.com wrote: I see this in configure.in: AC_ARG_ENABLE(nonportable-atomics, [ --enable-nonportable-atomics Use optimized atomic code which may produce nonportable binaries], [if test $enableval = yes; then force_generic_atomics=no else force_generic_atomics=yes fi ], [case $host_cpu in i[[456]]86) force_generic_atomics=yes ;; *) force_generic_atomics=no ;; esac ]) I was wondering why the three host_cpus i486, i586, and i686 have special treatment for the default setting as compared to all other cpu architectures? I don't see any reason since the code in srclib/apr/atomic/unix/ia32.c seems compatible with i386 (cmpxchg starts with i486), and atomic builtins work with gcc-v2+ (at worst USE_ATOMICS_IA32 could be defined for gcc-v1). The issue could have been in apr_atomic_casptr() and apr_atomic_xchgptr(), but APR_SIZEOF_VOIDP is checked to do the right thing with 32bits cpus... The changes in configure.in last http://svn.apache.org/viewvc?view=revisionrevision=r64861 , where force_generic_atomics went from : case $host in *linux*) apr_force_atomic_generic=1 case $host_cpu in i486|i586|i686|powerpc64) if test $nonportable_atomics_enabled = 1; then apr_force_atomic_generic=0 fi ;; esac To : AC_ARG_ENABLE(nonportable-atomics, [ --enable-nonportable-atomics Use optimized atomic code which may produce nonportable binaries], [if test $enableval = yes; then force_generic_atomics=no else force_generic_atomics=yes fi ], [case $host_cpu in i[[456]]86) force_generic_atomics=yes ;; *) force_generic_atomics=no ;; esac ]) So prior to r64861: native atomics were enabled on i[456]86 when --enable-nonportable-atomics=yes. After r64861: native atomics are disabled (forcibly) on those CPUs, whatever --enable-nonportable-atomics is. Was this intented?
Re: nonportable-atomics configure.in setting
On Wed, Dec 4, 2013 at 11:02 AM, Yann Ylavic ylavic@gmail.com wrote: After r64861: native atomics are disabled (forcibly) on those CPUs, whatever --enable-nonportable-atomics is. Sorry, --enable-nonportable-atomics is still honored, I misread the code.
Re: nonportable-atomics configure.in setting
On Wed, Dec 4, 2013 at 11:11 AM, Yann Ylavic ylavic@gmail.com wrote: On Wed, Dec 4, 2013 at 11:02 AM, Yann Ylavic ylavic@gmail.com wrote: After r64861: native atomics are disabled (forcibly) on those CPUs, whatever --enable-nonportable-atomics is. Sorry, --enable-nonportable-atomics is still honored, I misread the code. What I meant is that configure.in seem to have changed from use native atomics when explicitely specified AND the cpu is compatible to use native atomics when explicitely specified OR the CPU is compatible but not i[456]86, probably due to new intel capable cpus that need not be added to list whenever they appear.
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: nonportable-atomics configure.in setting
On Wed, Dec 4, 2013 at 10:40 AM, Yann Ylavic ylavic@gmail.com wrote: Sorry, I had no intention to send this offlist. Well, I really strayed here, I just realize now this is the wrong list. Sorry again Daniel and httpd folks, I'll now restate all that to the apr list :)
Re: time caching in util_time.c and mod_log_config.c
Adding APR dev list: IMO, httpd should expect APR to do the right thing. If APR isn't doing that, then it's an APR bug and needs to be fixed/ addressed within APR. All this implies that the atomics code in APR needs a serious review and update. We should also look into leveraging what we can from stdcxx (https://svn.apache.org/repos/asf/stdcxx/trunk/src) as well as OPA (https://trac.mcs.anl.gov/projects/openpa/wiki/FAQ). Also, IMO, the default should be non-portable atomics. On Dec 3, 2013, at 7:41 PM, Daniel Lescohier daniel.lescoh...@cbsi.com wrote: So I think we should reach a consensus on what approach to take. My goal was to implement an algorithm that is correct, with code that is easy to maintain. I think using the apr_atomics functions meets those goals the best. The downside are for those systems that are running 32-bit i486, i586, i686 systems where the default APR configure setting was not overridden for atomics. There may be i686 servers still out there using 32-bit web server, probably memory-constrained systems like VPS hosts; the question is have they overridden the APR default configuration or not. Should we hold back on fixing this because of these systems? If we go forward, should there be something in the release notes warning of this APR configuration issue? On Tue, Dec 3, 2013 at 7:15 PM, Daniel Lescohier daniel.lescoh...@cbsi.com wrote: (continued, hit send too early) %ix86 i386 i486 i586 i686 pentium3 pentium4 athlon geode However, I looked at the CentOS 6 apr.spec, and it's not overriding the default.
Re: time caching in util_time.c and mod_log_config.c
On Wed, Dec 4, 2013 at 7:47 AM, Jim Jagielski j...@jagunet.com wrote: Also, IMO, the default should be non-portable atomics. Yes: that is the purpose of APR: having a portable API on top of non-portable implementations for each platform.
Re: time caching in util_time.c and mod_log_config.c
So it sounds like I should go ahead and work on an implementation of the time caching using apr_atomic_cas32 and apr_atomic_dec32. This won't be an issue for RHEL/CentOS/etc., because they're using old versions of httpd. We can put something in the release notes saying that for 32-bit i486, i586, i686 builds, you should build APR with --enable-nonportable-atomics, for distro maintainers info when they package the new version. On Wed, Dec 4, 2013 at 7:47 AM, Jim Jagielski j...@jagunet.com wrote: Adding APR dev list: IMO, httpd should expect APR to do the right thing. If APR isn't doing that, then it's an APR bug and needs to be fixed/ addressed within APR. All this implies that the atomics code in APR needs a serious review and update. We should also look into leveraging what we can from stdcxx (https://svn.apache.org/repos/asf/stdcxx/trunk/src) as well as OPA (https://trac.mcs.anl.gov/projects/openpa/wiki/FAQ). Also, IMO, the default should be non-portable atomics. On Dec 3, 2013, at 7:41 PM, Daniel Lescohier daniel.lescoh...@cbsi.com wrote: So I think we should reach a consensus on what approach to take. My goal was to implement an algorithm that is correct, with code that is easy to maintain. I think using the apr_atomics functions meets those goals the best. The downside are for those systems that are running 32-bit i486, i586, i686 systems where the default APR configure setting was not overridden for atomics. There may be i686 servers still out there using 32-bit web server, probably memory-constrained systems like VPS hosts; the question is have they overridden the APR default configuration or not. Should we hold back on fixing this because of these systems? If we go forward, should there be something in the release notes warning of this APR configuration issue? On Tue, Dec 3, 2013 at 7:15 PM, Daniel Lescohier daniel.lescoh...@cbsi.com wrote: (continued, hit send too early) %ix86 i386 i486 i586 i686 pentium3 pentium4 athlon geode However, I looked at the CentOS 6 apr.spec, and it's not overriding the default.
Re: time caching in util_time.c and mod_log_config.c
+1 On Dec 4, 2013, at 11:19 AM, Daniel Lescohier daniel.lescoh...@cbsi.com wrote: So it sounds like I should go ahead and work on an implementation of the time caching using apr_atomic_cas32 and apr_atomic_dec32. This won't be an issue for RHEL/CentOS/etc., because they're using old versions of httpd. We can put something in the release notes saying that for 32-bit i486, i586, i686 builds, you should build APR with --enable-nonportable-atomics, for distro maintainers info when they package the new version. On Wed, Dec 4, 2013 at 7:47 AM, Jim Jagielski j...@jagunet.com wrote: Adding APR dev list: IMO, httpd should expect APR to do the right thing. If APR isn't doing that, then it's an APR bug and needs to be fixed/ addressed within APR. All this implies that the atomics code in APR needs a serious review and update. We should also look into leveraging what we can from stdcxx (https://svn.apache.org/repos/asf/stdcxx/trunk/src) as well as OPA (https://trac.mcs.anl.gov/projects/openpa/wiki/FAQ). Also, IMO, the default should be non-portable atomics. On Dec 3, 2013, at 7:41 PM, Daniel Lescohier daniel.lescoh...@cbsi.com wrote: So I think we should reach a consensus on what approach to take. My goal was to implement an algorithm that is correct, with code that is easy to maintain. I think using the apr_atomics functions meets those goals the best. The downside are for those systems that are running 32-bit i486, i586, i686 systems where the default APR configure setting was not overridden for atomics. There may be i686 servers still out there using 32-bit web server, probably memory-constrained systems like VPS hosts; the question is have they overridden the APR default configuration or not. Should we hold back on fixing this because of these systems? If we go forward, should there be something in the release notes warning of this APR configuration issue? On Tue, Dec 3, 2013 at 7:15 PM, Daniel Lescohier daniel.lescoh...@cbsi.com wrote: (continued, hit send too early) %ix86 i386 i486 i586 i686 pentium3 pentium4 athlon geode However, I looked at the CentOS 6 apr.spec, and it's not overriding the default.
Some redundant code and comment typos in mod_remoteip
While researching mod_remoteip to work on httpd bugs 55635 and 55637, I noticed a few unrelated blemishes in mod_remoteip.c. These include some redundant code and comment typos. The attached patch against httpd trunk should address these. Thanks, Mike Rumph Index: modules/metadata/mod_remoteip.c === --- modules/metadata/mod_remoteip.c (revision 1547874) +++ modules/metadata/mod_remoteip.c (working copy) @@ -37,11 +37,11 @@ } remoteip_proxymatch_t; typedef struct { -/** The header to retrieve a proxy-via ip list */ +/** The header to retrieve a proxy-via IP list */ const char *header_name; /** A header to record the proxied IP's * (removed as the physical connection and - * from the proxy-via ip header value list) + * from the proxy-via IP header value list) */ const char *proxies_header_name; /** A list of trusted proxies, ideally configured @@ -53,9 +53,9 @@ typedef struct { apr_sockaddr_t *useragent_addr; char *useragent_ip; -/** The list of proxy ip's ignored as remote ip's */ +/** The list of proxy IP's ignored as remote IP's */ const char *proxy_ips; -/** The remaining list of untrusted proxied remote ip's */ +/** The remaining list of untrusted proxied remote IP's */ const char *proxied_remote; } remoteip_req_t; @@ -111,7 +111,7 @@ static int looks_like_ip(const char *ipstr) { if (ap_strchr_c(ipstr, ':')) { -/* definitely not a hostname; assume it is intended to be an IPv6 address */ +/* definitely not a host name; assume it is intended to be an IPv6 address */ return 1; } @@ -198,7 +198,7 @@ while (!(ap_cfg_getline(lbuf, MAX_STRING_LEN, cfp))) { args = lbuf; while (*(arg = ap_getword_conf(cmd-temp_pool, args)) != '\0') { -if (*arg == '#' || *arg == '\0') { +if (*arg == '#') { break; } errmsg = proxies_set(cmd, cfg, arg); @@ -290,7 +290,7 @@ break; } -/* We map as IPv4 rather than IPv6 for equivilant host names +/* We map as IPv4 rather than IPv6 for equivalent host names * or IPV4OVERIPV6 */ rv = apr_sockaddr_info_get(temp_sa, parse_remote, @@ -309,7 +309,6 @@ remote = parse_remote; } break; - } addrbyte = (unsigned char *) temp_sa-sa.sin.sin_addr; @@ -422,11 +421,11 @@ which are trusted to present IP headers), AP_INIT_TAKE1(RemoteIPTrustedProxyList, proxylist_read, 0, RSRC_CONF | EXEC_ON_READ, - The filename to read the list of trusted proxies, + The file name to read the list of trusted proxies, see the RemoteIPTrustedProxy directive), AP_INIT_TAKE1(RemoteIPInternalProxyList, proxylist_read, (void*)1, RSRC_CONF | EXEC_ON_READ, - The filename to read the list of internal proxies, + The file name to read the list of internal proxies, see the RemoteIPInternalProxy directive), { NULL } };