Re: [RFC] CGIPassHeader Authorization|Proxy-Authorization|...

2014-11-24 Thread Eric Covener
On Thu, Aug 21, 2014 at 8:42 AM, Jeff Trawick  wrote:
> CGIPassHeader could be allowed in htaccess if the httpd admin has specified
> "AllowOverride ... AuthConfig ..."* or "AllowOverrideList CGIPassHeader" in
> the main config.
>
> Make sense?
>
> *Only auth headers are currently suppressed, so this directive could be
> named CGIPassAuthHeader and have a more visual tie to "AuthConfig".

+1

(a convo on IRC had me searching for this)


Fwd: [Bug 57204] New: LuaAuthzProvider mixes up parsed require arguments when used multiple times

2014-11-24 Thread Eric Covener
Mark, can you allocate a CVE for this? It is already public.


-- Forwarded message --
From: Eric Covener 
Date: Wed, Nov 19, 2014 at 7:16 PM
Subject: Fwd: [Bug 57204] New: LuaAuthzProvider mixes up parsed
require arguments when used multiple times
To: Apache HTTP Server Development List 


CVE worthy?

(sent to dev@ since it's mild and already discussed publically)


-- Forwarded message --
From:  
Date: Wed, Nov 12, 2014 at 9:52 AM
Subject: [Bug 57204] New: LuaAuthzProvider mixes up parsed require
arguments when used multiple times
To: b...@httpd.apache.org


https://issues.apache.org/bugzilla/show_bug.cgi?id=57204

Bug ID: 57204
   Summary: LuaAuthzProvider mixes up parsed require arguments
when used multiple times
   Product: Apache httpd-2
   Version: 2.4.10
  Hardware: PC
OS: Linux
Status: NEW
  Severity: normal
  Priority: P2
 Component: mod_lua
  Assignee: b...@httpd.apache.org
  Reporter: cove...@gmail.com

as reported in comments section of the manual anonymously, it looks like the
lua-specific hash used to store the parameters gets mixed up if you define 1
provider but use it with multiple require arguments.


original:

http://httpd.apache.org/docs/trunk/mod/mod_lua.html#comment_3245

--
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: bugs-unsubscr...@httpd.apache.org
For additional commands, e-mail: bugs-h...@httpd.apache.org



--
Eric Covener
cove...@gmail.com


-- 
Eric Covener
cove...@gmail.com


Re: svn commit: r1540052 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.c proxy_util.c

2014-11-24 Thread Eric Covener
On Mon, Nov 24, 2014 at 7:54 AM, Jim Jagielski  wrote:
> Hmmm let me try to recreate.

I am really confused about how it required both, but focused on the
ematch thing and have a fix in for that.

-- 
Eric Covener
cove...@gmail.com


Re: svn commit: r1540052 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.c proxy_util.c

2014-11-24 Thread Jim Jagielski
Hmmm let me try to recreate.

> On Nov 23, 2014, at 7:47 PM, Eric Covener  wrote:
> 
> On Fri, Nov 8, 2013 at 9:30 AM,   wrote:
>> 
>> URL: http://svn.apache.org/r1540052
>> Log:
>> UDS urls need to be desockified when configuring...
>> 
>> Modified:
>>httpd/httpd/trunk/modules/proxy/mod_proxy.c
>>httpd/httpd/trunk/modules/proxy/proxy_util.c
> 
> 
> Caution: I don't know anything about UDS in mod_prox as user much less
> as a developer, but in my sandboxes with:
> 
>  ProxyPassMatch ^/test.php$
> "unix:/var/run/php5-fpm.sock|fcgi://localhost/home/covener/SRC/httpd-trunk/built/htdocs/info.php"
> 
> * 2.4 works
> * trunk w/o this patch works
> * trunk fails because it tries to actually connect to localhost:8000
> (fcgi:// default port)
> 
> Is this commit broken or did it change the syntax?
> 
> -- 
> Eric Covener
> cove...@gmail.com



Re: svn commit: r1609680 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.c mod_proxy.h proxy_util.c

2014-11-24 Thread Jan Kaluža

On 11/24/2014 03:59 AM, Eric Covener wrote:

On Sun, Nov 23, 2014 at 9:57 PM, Eric Covener  wrote:

On Fri, Jul 11, 2014 at 6:36 AM,   wrote:

static int ap_proxy_strcmp_ematch(const char *str, const char *expected)
+{
+apr_size_t x, y;
+
+for (x = 0, y = 0; expected[y]; ++y, ++x) {
+if ((!str[x]) && (expected[y] != '
|| !apr_isdigit(expected[y + 1])))
+return -1;
+if (expected[y] == ' && apr_isdigit(expected[y + 1])) {
+while (expected[y] == ' && apr_isdigit(expected[y + 1]))
+y += 2;
+if (!expected[y])
+return 0;
+while (str[x]) {
+int ret;
+if ((ret = ap_proxy_strcmp_ematch(&str[x++], &expected[y])) != 
1)
+return ret;
+}
+return -1;
+}
+else if (expected[y] == '\\') {
+/* NUL is an invalid char! */
+if (!expected[++y])
+return -2;
+}
+if (str[x] != expected[y])
+return 1;
+}
+return (str[x] != '\0');
+}




Sorry, stray keystroke (tab?) made gmail send early.

This is breaking the common PHP-FPM recipes using unix domain sockets
in trunk e.g.

 ProxyPassMatch ^/info.php$
"unix:/var/run/php5-fpm.sock|fcgi://localhost/home/covener/SRC/httpd-trunk/built/htdocs/"

The old test accepted the worker URL being a prefix of the worker:

 strncmp(url_copy, worker->s->name, worker_name_length) == 0)

but now that doesn't happen for ProxyPassMatch.  This seems to be
due to the last return expecting str[x] to have been totally consumed
by the expected (worker) string.




Conflict discovered in file 'proxy/proxy_util.c'.

^ You were faster, thanks for fixing this issue :).

Regards,
Jan Kaluza



Re: PR56729: reqtimeout bug with fast response and slow POST

2014-11-24 Thread Yann Ylavic
On Sun, Nov 23, 2014 at 12:11 AM, Eric Covener  wrote:
> On Thu, Nov 20, 2014 at 9:57 AM, Yann Ylavic  wrote:
>> On Wed, Nov 19, 2014 at 1:13 PM, Eric Covener  wrote:
>>> On Wed, Nov 19, 2014 at 4:47 AM, Yann Ylavic  wrote:
 Errr, this is in 2.2.x/STATUS only (not 2.4.x).
 Is it already proposed/backported to 2.4.x (I can't find the commit)?
>>>
>>> I diff'ed trunk and 2.4 and It seems to be absent.
>>>
>>> I don't have the best handle on this, but if we're about to go down
>>> into a blocking read, wouldn't we want to check the time left and
>>> reduce the timeout?
>>
>> Yes, good point.
>>
>> Maybe this way then?
>>
>> Index: modules/filters/mod_reqtimeout.c
>> ===
>> --- modules/filters/mod_reqtimeout.c(revision 1640032)
>> +++ modules/filters/mod_reqtimeout.c(working copy)
>> @@ -311,7 +311,12 @@ static apr_status_t reqtimeout_filter(ap_filter_t
>>  else {
>>  /* mode != AP_MODE_GETLINE */
>>  rv = ap_get_brigade(f->next, bb, mode, block, readbytes);
>> -if (ccfg->min_rate > 0 && rv == APR_SUCCESS) {
>> +/* Don't extend the timeout in speculative mode, wait for
>> + * the real (relevant) bytes to be asked later, within the
>> + * currently alloted time.
>> + */
>> +if (ccfg->min_rate > 0 && rv == APR_SUCCESS
>> +&& mode != AP_MODE_SPECULATIVE) {
>>  extend_timeout(ccfg, bb);
>>  }
>>  }
>
> Looks good

Commited in r1641376.

However, I now think you were right with your original proposal to
bypass the filter based on EOS :p
I don't see any reason why we should not do the same for blocking and
nonblocking reads.
The call from check_pipeline() is an exception that shouldn't
interfere with the real calls from modules.

So the current code (including r1641376) is probably "good enough" for
2.2.x, but maybe we could make it better for trunk and 2.4.x.

A first proposal is straight forward from you original patch :
- bypass the filter when the request is over (base on EOS seen),
- check the timeout (but don't extend it) in speculative mode for both
blocking and nonblocking reads.
See httpd-trunk-reqtimeout_filter_eos.patch attached.

A second proposal would be to have an optional function from
mod_reqtimeout to (des)activate itself on demand.
Thus check_pipeline() can use it (if not NULL, ie. mod_reqtimeout
loaded) before/after the read to desactivate/reactivate the checks.
This is maybe more intrusive (requires a new
ap_init_request_processing() function is http_request.h) but is less
dependent on mod_reqtimeout seeing the EOS (and it could also be used
where currently mod_reqtimeout is forcibly removed from the chain).
See httpd-trunk-reqtimeout_set_inactive.patch attached.

WDYT?
Index: modules/filters/mod_reqtimeout.c
===
--- modules/filters/mod_reqtimeout.c	(revision 1641376)
+++ modules/filters/mod_reqtimeout.c	(working copy)
@@ -64,6 +64,7 @@ typedef struct
 } reqtimeout_con_cfg;
 
 static const char *const reqtimeout_filter_name = "reqtimeout";
+static const char *const reqtimeout_filter_eos_name = "reqtimeout_eos";
 static int default_header_rate_factor;
 static int default_body_rate_factor;
 
@@ -176,23 +177,13 @@ static apr_status_t reqtimeout_filter(ap_filter_t
 apr_status_t rv;
 apr_interval_time_t saved_sock_timeout = UNSET;
 reqtimeout_con_cfg *ccfg = f->ctx;
+int extendable;
 
 if (ccfg->in_keep_alive) {
 /* For this read, the normal keep-alive timeout must be used */
-ccfg->in_keep_alive = 0;
 return ap_get_brigade(f->next, bb, mode, block, readbytes);
 }
 
-if (block == APR_NONBLOCK_READ && mode == AP_MODE_SPECULATIVE) { 
-/*  The source of these above us in the core is check_pipeline(), which
- *  is between requests but before this filter knows to reset timeouts 
- *  during log_transaction().  If they appear elsewhere, just don't 
- *  check or extend the time since they won't block and we'll see the
- *  bytes again later
- */
-return ap_get_brigade(f->next, bb, mode, block, readbytes);
-}
-
 if (ccfg->new_timeout > 0) {
 /* set new timeout */
 now = apr_time_now();
@@ -212,6 +203,12 @@ static apr_status_t reqtimeout_filter(ap_filter_t
 ccfg->socket = ap_get_conn_socket(f->c);
 }
 
+/* Don't extend the timeout in speculative mode, wait for
+ * the real (relevant) bytes to be asked later, within the
+ * currently alloted time.
+ */
+extendable = (ccfg->min_rate > 0 && mode != AP_MODE_SPECULATIVE);
+
 rv = check_time_left(ccfg, &time_left, now);
 if (rv != APR_SUCCESS)
 goto out;
@@ -219,7 +216,7 @@ static apr_status_t reqtimeout_filter(ap_filter_t
 if (block == APR_NONBLOCK_READ || mode == AP_MODE_INIT
 || mode == AP_MODE_EATCRLF) {
 rv = ap_ge

Re: svn commit: r1609680 - in /httpd/httpd/trunk/modules/proxy: mod_proxy.c mod_proxy.h proxy_util.c

2014-11-24 Thread Eric Covener
please check r1641381

On Sun, Nov 23, 2014 at 9:59 PM, Eric Covener  wrote:
> On Sun, Nov 23, 2014 at 9:57 PM, Eric Covener  wrote:
>> On Fri, Jul 11, 2014 at 6:36 AM,   wrote:
>>> static int ap_proxy_strcmp_ematch(const char *str, const char *expected)
>>> +{
>>> +apr_size_t x, y;
>>> +
>>> +for (x = 0, y = 0; expected[y]; ++y, ++x) {
>>> +if ((!str[x]) && (expected[y] != '
>>> || !apr_isdigit(expected[y + 1])))
>>> +return -1;
>>> +if (expected[y] == ' && apr_isdigit(expected[y + 1])) {
>>> +while (expected[y] == ' && apr_isdigit(expected[y + 1]))
>>> +y += 2;
>>> +if (!expected[y])
>>> +return 0;
>>> +while (str[x]) {
>>> +int ret;
>>> +if ((ret = ap_proxy_strcmp_ematch(&str[x++], 
>>> &expected[y])) != 1)
>>> +return ret;
>>> +}
>>> +return -1;
>>> +}
>>> +else if (expected[y] == '\\') {
>>> +/* NUL is an invalid char! */
>>> +if (!expected[++y])
>>> +return -2;
>>> +}
>>> +if (str[x] != expected[y])
>>> +return 1;
>>> +}
>>> +return (str[x] != '\0');
>>> +}
>>
>
> Sorry, stray keystroke (tab?) made gmail send early.
>
> This is breaking the common PHP-FPM recipes using unix domain sockets
> in trunk e.g.
>
> ProxyPassMatch ^/info.php$
> "unix:/var/run/php5-fpm.sock|fcgi://localhost/home/covener/SRC/httpd-trunk/built/htdocs/"
>
> The old test accepted the worker URL being a prefix of the worker:
>
> strncmp(url_copy, worker->s->name, worker_name_length) == 0)
>
> but now that doesn't happen for ProxyPassMatch.  This seems to be
> due to the last return expecting str[x] to have been totally consumed
> by the expected (worker) string.
>
>
> --
> Eric Covener
> cove...@gmail.com



-- 
Eric Covener
cove...@gmail.com


Re: mod_ssl FakeBasicAuth, the colon problem (PR 52644)

2014-11-24 Thread Jan Kaluža

On 06/26/2014 09:22 AM, Ruediger Pluem wrote:



Joe Orton wrote:

I've had a user hit this: with FakeBasicAuth the client DN gets
translated into a Basic auth blob of base64("username:password"), which
then fails when the username part contains a ":" colon character.

At minimum mod_ssl could/should catch and fail auth under FakeBasicAuth
when DN is seen with a ":", that's easy enough.  We *could* also try
escaping the colon, but that introduces an inevitable ambiguity since
there is no escaping standard.

One approach would be to escape any colon in the DN by replacing with
some unusual character sequence ("" or whatever) and then only fail
for unescaped DNs which contain that sequence to avoid ambiguity
problems.

Any opinions before I hack something up?

Probably the "correct" way to approach this problem is using Graham's
nice hacks in the trunk to allow users to construct an appropriate
username:password blog based on expressions:

   http://svn.apache.org/viewvc?view=revision&revision=r1457471


+1 as this being the real fix at least where the expression parser is available.
Maybe just document the colon problem with FakeBasicAuth and point the user to 
AuthBasicFake
to do its own escaping of the colon with the expression parser (at best with an 
example).
But I just realize that a simple search and replace function is missing in the 
expression parser.


Attached patch implements that. You can test the patch for example like 
that:


Require expr replace(%{REQUEST_METHOD},  "E", "O") == "GOT"

If there won't be any -1, I will commit it (+ docs) to trunk later this 
week.



So maybe hack that up an then go the way above?

Regards

Rüdiger



Regards,
Jan Kaluza



httpd-trunk-ap_expr-replace.patch
Description: application/download