RE: Mergine of Multiple Cookie Headers

2016-06-29 Thread Plüm , Rüdiger , Vodafone Group


> -Original Message-
> From: Rainer Canavan [mailto:rainer.cana...@sevenval.com]
> Sent: Dienstag, 28. Juni 2016 16:30
> To: dev@httpd.apache.org
> Subject: Mergine of Multiple Cookie Headers
> 
> Hi,
> 
> We've observed multiple gateways, operated by e.g. AT&T, COLT and
> Vodafone, that inject additional Cookie: headers into client requests,
> such as
> 
> Cookie: actually=from_the_client
> Cookie: Bearer-Type=w-TCP
> Cookie: network-access-type=UMTS
> 
> Apache httpd merges those headers into a single, comma separated list,
> and also appends the names and values of all Cookies set in the
> additional Cookie Headers to the value of the last Cookie of the first
> header. This can be seeen by logging  %{actually}C for the example
> above, which would contain
> 
> actually=from_the_client, Bearer-Type=w-TCP, network-access-type=UMTS
> 
> While RFC 6265 clearly requires that User-Agents send only a single
> Cookie: request header, I would argue that the Cookie header should be
> treated as an exception, similar to the Set-Cookie:-response header,
> and not be merged into a single header field. An alternative would be
> to use "; " as a separator.
> 
> Any thoughts?

How about

RequestHeader edit* Cookie ", " "; "

Regards

Rüdiger


Last-Modified header value returned by FCGI scripts

2016-06-29 Thread Luca Toscano
Hi Apache devs!

I have been working on an email thread [1] in the users@ mailing list in
which it was asked some questions about how httpd (using mod-proxy-fcgi)
manages Last-Modified headers returned by FCGI/CGI scripts. Two strange
behaviors were brought up:

1) Last-Modified: foo returned by a simple PHP script forces httpd to
replace it with Thu, 01 Jan 1970 00:00:00 GMT. Patch proposed to backport:
http://svn.apache.org/r1748379
2) Last-Modified header value with a date not in GMT are replaced with
(now() + time taken to serve the request) without any trace in the logs.
This seems to be due to httpd recognizing the date as "in the future" and
replacing it with its response origination time (following
https://tools.ietf.org/html/rfc2616#section-14.29).

To demonstrate 2), Manuel in users@ suggested a simple PHP script returning
the current datetime in the Europe/Paris timezone (GMT +2). I tried to
check the code and I came up with two possible solutions:

1)  mod-proxy-fcgi eventually triggers a call
to ap_scan_script_header_err_core_ex in util_script.c that
uses apr_date_parse_http to transform a datestring into a unix timestamp.
This one seems not to check the timezone, assuming GMT all the times. It
might be an option to add a check in the code to return APR_DATE_BAD in
case the datetime is not GMT, and then r1748379 will take care of the rest.
This could potentially break existing code that relies on this behavior to
work correctly.

2) Simply log what httpd does, for example with http://apaste.info/8pa or
http://apaste.info/JlZ (wording might need to be changed).

Any suggestion?

Regards,

Luca



[1]:
https://lists.apache.org/thread.html/63e70b8f9e1ebfec8d169d6cdd51875db77ff8161a7936419be4@1464608714@%3Cusers.httpd.apache.org%3E


Re: Mergine of Multiple Cookie Headers

2016-06-29 Thread Yann Ylavic
On Wed, Jun 29, 2016 at 9:33 AM, Plüm, Rüdiger, Vodafone Group
 wrote:
>
>
>> -Original Message-
>> From: Rainer Canavan [mailto:rainer.cana...@sevenval.com]
>> Sent: Dienstag, 28. Juni 2016 16:30
>> To: dev@httpd.apache.org
>> Subject: Mergine of Multiple Cookie Headers
>>
>> Hi,
>>
>> We've observed multiple gateways, operated by e.g. AT&T, COLT and
>> Vodafone, that inject additional Cookie: headers into client requests,
>> such as
>>
>> Cookie: actually=from_the_client
>> Cookie: Bearer-Type=w-TCP
>> Cookie: network-access-type=UMTS
>>
>> Apache httpd merges those headers into a single, comma separated list,
>> and also appends the names and values of all Cookies set in the
>> additional Cookie Headers to the value of the last Cookie of the first
>> header. This can be seeen by logging  %{actually}C for the example
>> above, which would contain
>>
>> actually=from_the_client, Bearer-Type=w-TCP, network-access-type=UMTS
>>
>> While RFC 6265 clearly requires that User-Agents send only a single
>> Cookie: request header, I would argue that the Cookie header should be
>> treated as an exception, similar to the Set-Cookie:-response header,
>> and not be merged into a single header field. An alternative would be
>> to use "; " as a separator.
>>
>> Any thoughts?
>
> How about
>
> RequestHeader edit* Cookie ", " "; "

Or possibly something more generic (quoting, escaping...), but less readable :p

RequestHeader edit* Cookie
([^=;,]++)(="(?:[^"].)*+[^"]*+"|[^;,]*)?+[;,] $1$2; early

(with or without the "early" flag)

Regards,
Yann.


[PATCH] on TRACE & RFC compliance

2016-06-29 Thread Joe Orton
We had a couple of people complaining about the language around TRACE in 
the docs, which say disabling TRACE "makes your server noncompliant", a 
claim I found hard to support.
  
All methods but HEAD and GET are defined as OPTIONAL, so I'm not sure 
how this is true, am I missing something?

https://tools.ietf.org/html/rfc2616#section-5.1.1
https://tools.ietf.org/html/rfc7231#section-4.1

Is that choice of words just to scare people off?  IIRC there are some 
pretty strong opinions on this; attached is how I'd change it.
Index: docs/manual/mod/core.xml
===
--- docs/manual/mod/core.xml(revision 1750619)
+++ docs/manual/mod/core.xml(working copy)
@@ -4532,16 +4532,20 @@
 Finally, for testing and diagnostic purposes only, request
 bodies may be allowed using the non-compliant TraceEnable
 extended directive.  The core (as an origin server) will
-restrict the request body to 64k (plus 8k for chunk headers if
+restrict the request body to 64Kb (plus 8Kb for chunk headers if
 Transfer-Encoding: chunked is used).  The core will
 reflect the full headers and all chunk headers with the response
 body.  As a proxy server, the request body is not restricted to 64k.
 
 Note
-Despite claims to the contrary, TRACE is not
-a security vulnerability, and there is no viable reason for
-it to be disabled. Doing so necessarily makes your server
-noncompliant.
+
+Despite claims to the contrary, enabling the TRACE
+method does not expose any security vulnerability in Apache httpd.
+The TRACE method is defined by the HTTP/1.1
+specification and implementations are expected to support it.
+Leaving TRACE enabled is strongly recommended for all
+deployments of Apache httpd.
+
 
 
 


Re: Last-Modified header value returned by FCGI scripts

2016-06-29 Thread William A Rowe Jr
On Wed, Jun 29, 2016 at 3:12 AM, Luca Toscano 
wrote:

> Hi Apache devs!
>
> I have been working on an email thread [1] in the users@ mailing list in
> which it was asked some questions about how httpd (using mod-proxy-fcgi)
> manages Last-Modified headers returned by FCGI/CGI scripts. Two strange
> behaviors were brought up:
>
> 1) Last-Modified: foo returned by a simple PHP script forces httpd to
> replace it with Thu, 01 Jan 1970 00:00:00 GMT. Patch proposed to backport:
> http://svn.apache.org/r1748379
>

Sensible, but we should be filling this in with now(), based on comments in
14.29?


> 2) Last-Modified header value with a date not in GMT are replaced with
> (now() + time taken to serve the request) without any trace in the logs.
> This seems to be due to httpd recognizing the date as "in the future" and
> replacing it with its response origination time (following
> https://tools.ietf.org/html/rfc2616#section-14.29).
>

https://tools.ietf.org/html/rfc2616#section-3.3.1 declares these are
meaningless, and we follow the appropriate recommendations.


> To demonstrate 2), Manuel in users@ suggested a simple PHP script
> returning the current datetime in the Europe/Paris timezone (GMT +2). I
> tried to check the code and I came up with two possible solutions:
>
> 1)  mod-proxy-fcgi eventually triggers a call
> to ap_scan_script_header_err_core_ex in util_script.c that
> uses apr_date_parse_http to transform a datestring into a unix timestamp.
> This one seems not to check the timezone, assuming GMT all the times. It
> might be an option to add a check in the code to return APR_DATE_BAD in
> case the datetime is not GMT, and then r1748379 will take care of the rest.
> This could potentially break existing code that relies on this behavior to
> work correctly.
>

-0 on recognizing non-GMT, per section 3.3.1 of spec.


> 2) Simply log what httpd does, for example with http://apaste.info/8pa or
> http://apaste.info/JlZ (wording might need to be changed).
>

+1 in all cases to adding trace messages for sysadmins debugging bad cgi.

Cheers,

Bill


Re: [PATCH] on TRACE & RFC compliance

2016-06-29 Thread William A Rowe Jr
The wording change seems fine to me, I'd actually be fine with simply
dropping
your last sentence entirely. Our config defaults speak for themselves.

On Wed, Jun 29, 2016 at 6:55 AM, Joe Orton  wrote:

> We had a couple of people complaining about the language around TRACE in
> the docs, which say disabling TRACE "makes your server noncompliant", a
> claim I found hard to support.
>
> All methods but HEAD and GET are defined as OPTIONAL, so I'm not sure
> how this is true, am I missing something?
>
> https://tools.ietf.org/html/rfc2616#section-5.1.1
> https://tools.ietf.org/html/rfc7231#section-4.1
>
> Is that choice of words just to scare people off?  IIRC there are some
> pretty strong opinions on this; attached is how I'd change it.
>


Re: [PATCH] on TRACE & RFC compliance

2016-06-29 Thread Eric Covener
On Wed, Jun 29, 2016 at 8:08 AM, William A Rowe Jr  wrote:
> The wording change seems fine to me, I'd actually be fine with simply
> dropping
> your last sentence entirely. Our config defaults speak for themselves.


I think the last sentence is a little strong too.  People are just
going to disable it based on commercial scanners anyway.

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


Re: [PATCH] on TRACE & RFC compliance

2016-06-29 Thread Ruediger Pluem


On 06/29/2016 02:44 PM, Eric Covener wrote:
> On Wed, Jun 29, 2016 at 8:08 AM, William A Rowe Jr  
> wrote:
>> The wording change seems fine to me, I'd actually be fine with simply
>> dropping
>> your last sentence entirely. Our config defaults speak for themselves.
> 
> 
> I think the last sentence is a little strong too.  People are just
> going to disable it based on commercial scanners anyway.
> 

+1

Regards

Rüdiger


Re: Merging of Multiple Cookie Headers

2016-06-29 Thread Rainer Canavan
On Wed, Jun 29, 2016 at 2:02 AM, Joseph Schaefer  wrote:
> Php's cookie parser can be more lax in treating ", " similar to "; ", that 
> would be a better avenue of redress.  Otherwise they can adopt libapreq2's 
> cookie parsing code which has much richer support for merging cookie headers 
> written to different cookie specs.

That's basically what it'll have to be, since even if this was "fixed"
in httpd, it would probably take years for this to appear in the
apache packages of any of the "enterprise" Linux distributions.
Anyway, in my opinion, the same argument regarding treating ", " like
"; " also applies to httpd (regarding %{...}c), especially since ,
isn't allowed anywhere in a cookie, but there's no point in continuing
this discussion.

rainer


Re: Last-Modified header value returned by FCGI scripts

2016-06-29 Thread Luca Toscano
2016-06-29 14:06 GMT+02:00 William A Rowe Jr :

> On Wed, Jun 29, 2016 at 3:12 AM, Luca Toscano 
> wrote:
>
>> Hi Apache devs!
>>
>> I have been working on an email thread [1] in the users@ mailing list in
>> which it was asked some questions about how httpd (using mod-proxy-fcgi)
>> manages Last-Modified headers returned by FCGI/CGI scripts. Two strange
>> behaviors were brought up:
>>
>> 1) Last-Modified: foo returned by a simple PHP script forces httpd to
>> replace it with Thu, 01 Jan 1970 00:00:00 GMT. Patch proposed to backport:
>> http://svn.apache.org/r1748379
>>
>
> Sensible, but we should be filling this in with now(), based on comments
> in 14.29?
>

Very open to discuss this one, I have some doubts too. I chose to drop the
header since I applied 14.29 only to dates (i.e. not triggering any
APR_DATE_BAD after parsing) and not to completely wrong values like "foo".
My aim was to drop any non parsable date (logging the action) and to
correct the rest if considered in the future from httpd's point of view.


>
>
>> 2) Last-Modified header value with a date not in GMT are replaced with
>> (now() + time taken to serve the request) without any trace in the logs.
>> This seems to be due to httpd recognizing the date as "in the future" and
>> replacing it with its response origination time (following
>> https://tools.ietf.org/html/rfc2616#section-14.29).
>>
>
> https://tools.ietf.org/html/rfc2616#section-3.3.1 declares these are
> meaningless, and we follow the appropriate recommendations.
>

+1, completely missed this part: "This is indicated in the first two
formats by the inclusion of "GMT" as the three-letter abbreviation for time
zone, and MUST be assumed when reading the asctime format."


>
>
>> To demonstrate 2), Manuel in users@ suggested a simple PHP script
>> returning the current datetime in the Europe/Paris timezone (GMT +2). I
>> tried to check the code and I came up with two possible solutions:
>>
>> 1)  mod-proxy-fcgi eventually triggers a call
>> to ap_scan_script_header_err_core_ex in util_script.c that
>> uses apr_date_parse_http to transform a datestring into a unix timestamp.
>> This one seems not to check the timezone, assuming GMT all the times. It
>> might be an option to add a check in the code to return APR_DATE_BAD in
>> case the datetime is not GMT, and then r1748379 will take care of the rest.
>> This could potentially break existing code that relies on this behavior to
>> work correctly.
>>
>
> -0 on recognizing non-GMT, per section 3.3.1 of spec.
>

Yes completely agree.


>
>
>> 2) Simply log what httpd does, for example with http://apaste.info/8pa
>> or http://apaste.info/JlZ (wording might need to be changed).
>>
>
> +1 in all cases to adding trace messages for sysadmins debugging bad cgi.
>
>
Thank you for the feedback!

Regards,

Luca


2.4.23 T&R Status

2016-06-29 Thread Jim Jagielski
With this coming weekend a US holiday weekend, I am hesitant to do a
release this actual week. Instead, I will T&R either tomorrow or Fri
in hopes of announcing on the 5th (Tues).


ScriptAlias with a specific handler

2016-06-29 Thread Blaise

ScriptAlias seems to have an unfair preference for mod_cgi, since the handler 
"cgi-script" is hard-coded in mod_alias.
Would it not be nicer (towards other scripting modules and the user) if 
ScriptAlias accepted an optional third parameter -- handler's name, so that one 
could write
ScriptAlias /webapp "/path/to/webapp" custom-script-processor
instead of
Alias /webapp "/path/to/webapp"

Options +ExecCGI
SetHandler custom-script-processor

?

--
βþ


Re: svn commit: r1750681 - in /httpd/httpd/branches/2.4.x: STATUS docs/manual/mod/mod_proxy_http2.html docs/manual/mod/mod_proxy_http2.html.en docs/manual/mod/mod_proxy_http2.xml.meta

2016-06-29 Thread André Malo
* j...@apache.org wrote:

> Author: jim
> Date: Wed Jun 29 17:28:53 2016
> New Revision: 1750681
>
> URL: http://svn.apache.org/viewvc?rev=1750681&view=rev
> Log:
> vote.
> Required since w/o this patch, previous build fail (no lbmethods)
>
> Removed:
> httpd/httpd/branches/2.4.x/docs/manual/mod/mod_proxy_http2.html
> httpd/httpd/branches/2.4.x/docs/manual/mod/mod_proxy_http2.html.en
> httpd/httpd/branches/2.4.x/docs/manual/mod/mod_proxy_http2.xml.meta

Huh?

nd
-- 
Das einzige, das einen Gebäudekollaps (oder auch einen
thermonuklearen Krieg) unbeschadet übersteht, sind Kakerlaken
und AOL-CDs.
  -- Bastian Lipp in dcsm


Re: svn commit: r1750730 - /httpd/httpd/branches/2.2.x/STATUS

2016-06-29 Thread William A Rowe Jr
I'd prefer if you would not invalidate a vote that others present.

I support the original patch.  I reviewed and accept the amended patch
also, but it hasn't seen nearly the same scrutiny as the widely adopted
patch presented in the PR.

You are free to vote for only the enhanced patch, of course, but whichever
attains three reviews, be it only the tested patch or the improved variant,
should be adopted just as soon as they are accepted.  That's why I made the
status vote cumulative, so people could accept one or all of these
proposals.

Cheers,

Bill
On Jun 29, 2016 4:38 PM,  wrote:

> Author: ylavic
> Date: Wed Jun 29 21:38:02 2016
> New Revision: 1750730
>
> URL: http://svn.apache.org/viewvc?rev=1750730&view=rev
> Log:
> Add CHANGES entry in mod_mem_cache patch relating to PR 45049.
> While at it, propose another fix for mod_mem_cache relating to PR 43724.
>
> Modified:
> httpd/httpd/branches/2.2.x/STATUS
>
> Modified: httpd/httpd/branches/2.2.x/STATUS
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=1750730&r1=1750729&r2=1750730&view=diff
>
> ==
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Wed Jun 29 21:38:02 2016
> @@ -170,9 +170,10 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
>  PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>[ New proposals should be added at the end of the list ]
>
> -  *) mod_mem_cache: Don't cache incomplete responses when the client
> aborts the connection.
> +  *) mod_mem_cache: Don't cache incomplete responses when the client
> aborts
> +the connection, unless they are complete.  PR 45049.
>   Not applicable to trunk, mod_mem_cache doesn't exist there.
> - 2.2.x patch:
> http://home.apache.org/~ylavic/patches/httpd-2.2.x-mem_cache_client_abort-v2.patch
> + 2.2.x patch:
> http://home.apache.org/~ylavic/patches/httpd-2.2.x-mod_mem_cache-pr45049.patch
>   +1: ylavic, wrowe
>   ylavic: don't we know from the very beginning of store_body() if
>   r->connection->aborted, so to fail there?
> @@ -185,6 +186,10 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>   client won't receive it completely.
>   wrowe:  Patch looks like an improvement, would be good if you both
> assert
>   that this is what we will ship.
> + ylavic: This is a bugfix (added CHANGES entry), the improvement is
> upon
> + Ed's original patch (don't cache the response if the client
> + aborted the connection) where we now still cache it in any
> case
> + if it is complete (for immediate availability for next
> clients).
>
>*) mod_ssl: Free dhparams and ecparams reading certificates at startup.
>   This fixes issue when SSLCryptoDevice does not get unregistered
> because
> @@ -205,7 +210,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>   Trunk Patch: https://svn.apache.org/r813178
>   2.2.x Patch: https://bz.apache.org/bugzilla/attachment.cgi?id=30144
>plus CHANGES above.
> - +1: wrowe
>   ylavic: this code has evolved in trunk/2.4.x (including fixes) since
> this
>   original commit (2009), we should probably include these
> changes.
>   I'm thinking of r1642857 and r1670324 from 2.4.x, with this
> delta
> @@ -215,7 +219,13 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
>https://svn.apache.org/r1670324
>[plus patch and changes noted above]
>   2.2.x Patch:
> http://home.apache.org/~ylavic/patches/httpd-2.2.x-mod_proxy_connect-transfer.patch
> - +1: wrowe
> + +1: wrowe, ylavic
> +
> +  *) mod_mem_cache: Fix concurrent removal of stale entries which could
> lead
> +to a crash. PR 43724.
> + trunk patch: not applicable (2.2.x only)
> + 2.2.x patch:
> http://home.apache.org/~ylavic/patches/httpd-2.2.x-mod_mem_cache-pr43724.patch
> + +1: ylavic
>
>  PATCHES/ISSUES THAT ARE STALLED
>
>
>
>


Re: svn commit: r1750730 - /httpd/httpd/branches/2.2.x/STATUS

2016-06-29 Thread Yann Ylavic
On Thu, Jun 30, 2016 at 12:40 AM, William A Rowe Jr  wrote:
> I'd prefer if you would not invalidate a vote that others present.

Sorry about that, I thought it was an oversight.

>
> I support the original patch.  I reviewed and accept the amended patch also,
> but it hasn't seen nearly the same scrutiny as the widely adopted patch
> presented in the PR.
>
> You are free to vote for only the enhanced patch, of course, but whichever
> attains three reviews, be it only the tested patch or the improved variant,
> should be adopted just as soon as they are accepted.  That's why I made the
> status vote cumulative, so people could accept one or all of these
> proposals.

The issue is that the patch from PR 30144 is not really satisfactory
(there were some PR fixed in 2.4.x since then, as I noted), so I'm -1
for it alone...

Regards,
Yann.


Re: svn commit: r1750730 - /httpd/httpd/branches/2.2.x/STATUS

2016-06-29 Thread William A Rowe Jr
On Jun 29, 2016 5:57 PM, "Yann Ylavic"  wrote:
>
> On Thu, Jun 30, 2016 at 12:40 AM, William A Rowe Jr 
wrote:
> > I'd prefer if you would not invalidate a vote that others present.
>
> Sorry about that, I thought it was an oversight.
>
> >
> > I support the original patch.  I reviewed and accept the amended patch
also,
> > but it hasn't seen nearly the same scrutiny as the widely adopted patch
> > presented in the PR.
> >
> > You are free to vote for only the enhanced patch, of course, but
whichever
> > attains three reviews, be it only the tested patch or the improved
variant,
> > should be adopted just as soon as they are accepted.  That's why I made
the
> > status vote cumulative, so people could accept one or all of these
> > proposals.
>
> The issue is that the patch from PR 30144 is not really satisfactory
> (there were some PR fixed in 2.4.x since then, as I noted), so I'm -1
> for it alone...

I hope the entire patch as reviewed is acceptable and gets a third vote...

...that said, did the patch introduce a regression? If not, it still
represents an improvement, even if less than a full rewrite/perfect
implementation.

Cheers

Bill