Re: Regression with range fix
On Tue, Aug 30, 2011 at 08:51:55PM +0200, Stefan Fritsch wrote: The first regression report, though slightly too late for the vote: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825 The byterange_filter.c in the Debian update is exactly the one from 2.2.20. I will keep you updated. Hi; I'm just back from holiday and catching up. The behaviour changes in the patch which could feasibly break non-compliant clients are: a) using 200 in some cases where a 206 response would end up being larger b) using a chunked response where previously C-L was always used, in cases where =32 ranges are being returned Anything else to watch out for? Looking at the patch in 2.2.x; there is a lot of effort expended deadling with apr_bucket_split() returning ENOTIMPL - that looks unnecessary; the filter will only handle brigades containing buckets with known length, and all such buckets must be _split-able. Regards, Joe
Re: [VOTE] httpd-2.2.20 tarballs
On Aug 31, 2011, at 4:38 AM, Plüm, Rüdiger, VF-Group wrote: Let's see where we head with the regression report Stefan mentioned. If it is really something that needs fixing we should go for 2.2.21 and fix the above issues as well. Agreed… also, if this is such a concern, then Bill should update the release tools to req what he demands as the canon list of build tools that must be used and recall such concepts as one cannot veto a release (and other times when we pushed a security release out aggressively)
[ANNOUNCEMENT] Apache HTTP Server 2.2.20 Released
Apache HTTP Server 2.2.20 Released The Apache Software Foundation and the Apache HTTP Server Project are pleased to announce the release of version 2.2.20 of the Apache HTTP Server (Apache). This version of Apache is principally a security and bug fix release: * SECURITY: CVE-2011-3192 (cve.mitre.org) core: Fix handling of byte-range requests to use less memory, to avoid denial of service. If the sum of all ranges in a request is larger than the original file, ignore the ranges and send the complete file. PR 51714. We consider this release to be the best version of Apache available, and encourage users of all prior versions to upgrade. Apache HTTP Server 2.2.20 is available for download from: http://httpd.apache.org/download.cgi Please see the CHANGES_2.2 file, linked from the download page, for a full list of changes. A condensed list, CHANGES_2.2.20 provides the complete list of changes since 2.2.19. A summary of all of the security vulnerabilities addressed in this and earlier releases is available: http://httpd.apache.org/security/vulnerabilities_22.html This release includes the Apache Portable Runtime (APR) version 1.4.5 and APR Utility Library (APR-util) version 1.3.12, bundled with the tar and zip distributions. The APR libraries libapr and libaprutil (and on Win32, libapriconv version 1.2.1) must all be updated to ensure binary compatibility and address many known security and platform bugs. Apache 2.2 offers numerous enhancements, improvements, and performance boosts over the 2.0 codebase. For an overview of new features introduced since 2.0 please see: http://httpd.apache.org/docs/2.2/new_features_2_2.html This release builds on and extends the Apache 2.0 API. Modules written for Apache 2.0 will need to be recompiled in order to run with Apache 2.2, and require minimal or no source code changes. http://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x/VERSIONING When upgrading or installing this version of Apache, please bear in mind that if you intend to use Apache with one of the threaded MPMs (other than the Prefork MPM), you must ensure that any modules you will be using (and the libraries they depend on) are thread-safe.
RE: Regression with range fix
-Original Message- From: Joe Orton [mailto:jor...@redhat.com] Sent: Mittwoch, 31. August 2011 11:13 To: dev@httpd.apache.org Subject: Re: Regression with range fix On Tue, Aug 30, 2011 at 08:51:55PM +0200, Stefan Fritsch wrote: The first regression report, though slightly too late for the vote: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825 The byterange_filter.c in the Debian update is exactly the one from 2.2.20. I will keep you updated. Hi; I'm just back from holiday and catching up. The behaviour changes in the patch which could feasibly break non-compliant clients are: a) using 200 in some cases where a 206 response would end up being larger b) using a chunked response where previously C-L was always used, in cases where =32 ranges are being returned Anything else to watch out for? Looking at the patch in 2.2.x; there is a lot of effort expended deadling with apr_bucket_split() returning ENOTIMPL - that looks unnecessary; the filter will only handle brigades containing buckets with known length, and all such buckets must be _split-able. So you think we can rip out the whole if (rv == APR_ENOTIMPL) blocks? Regards Rüdiger
Re: Regression with range fix
On Aug 31, 2011, at 8:21 AM, Plüm, Rüdiger, VF-Group wrote: -Original Message- From: Joe Orton [mailto:jor...@redhat.com] Sent: Mittwoch, 31. August 2011 11:13 To: dev@httpd.apache.org Subject: Re: Regression with range fix On Tue, Aug 30, 2011 at 08:51:55PM +0200, Stefan Fritsch wrote: The first regression report, though slightly too late for the vote: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825 The byterange_filter.c in the Debian update is exactly the one from 2.2.20. I will keep you updated. Hi; I'm just back from holiday and catching up. The behaviour changes in the patch which could feasibly break non-compliant clients are: a) using 200 in some cases where a 206 response would end up being larger b) using a chunked response where previously C-L was always used, in cases where =32 ranges are being returned Anything else to watch out for? Looking at the patch in 2.2.x; there is a lot of effort expended deadling with apr_bucket_split() returning ENOTIMPL - that looks unnecessary; the filter will only handle brigades containing buckets with known length, and all such buckets must be _split-able. So you think we can rip out the whole if (rv == APR_ENOTIMPL) blocks? Belt and suspenders?
Re: [VOTE] httpd-2.2.20 tarballs
On 8/31/2011 3:41 AM, Plüm, Rüdiger, VF-Group wrote: It will not fail, as we know that the parameter we pass to apr_bucket_split is within the limits of apr_size_t due to earlier checks in apr_uint64_t arithmetic. It is really just silencing a compiler warning. Exactly... because this has a 'size' that size has to already exist in memory, ergo max(apr_size_t). Comments on such casts might be helpful.
Re: [VOTE] httpd-2.2.20 tarballs
On 8/31/2011 5:31 AM, Jim Jagielski wrote: On Aug 31, 2011, at 4:38 AM, Plüm, Rüdiger, VF-Group wrote: Let's see where we head with the regression report Stefan mentioned. If it is really something that needs fixing we should go for 2.2.21 and fix the above issues as well. Agreed… also, if this is such a concern, then Bill should update the release tools to req what he demands as the canon list of build tools that must be used and recall such concepts as one cannot veto a release (and other times when we pushed a security release out aggressively) No veto - just expressed a preference that we don't wildly change the build tools for a 'minimal change'. If this were simply 2.2.20, the latest and greatest release, I couldn't care less :) Let folks who encounter issues stay with 2.2.19. It isn't my demand, just a simple observation of what is in httpd-2.2.19.tar vs httpd-2.2.20.tar. Somehow it's Wednesday, and none of the patch authors have actually placed this critical security patch in either https://dist.apache.org/repos/dist/release/httpd/patches/apply_to_2.2.20/ https://dist.apache.org/repos/dist/release/httpd/patches/apply_to_2.0.64/ If these are not patch-worthy, I highly doubt them to be release worthy, so I'm afraid I don't have a lot of respect for this whole 24 hour release process. You are almost right, Jim, we've been aggressive with our security releases, but _never_ with less than 24 hours of careful consideration.
Re: Next update
Note some additional improvements for a 'final' update 3 advisory... We aught to mention that mod_header or mod_rewrite and mod_setenvif are required for their respective workarounds, this apparently confuses some beginning users. We aught to mention that backend/application servers are not protected from odd Range: constructs passed through mod_proxy. We aught to add the release 2.2.20 as solution #1. We aught to add reference to patches published at; http://www.apache.org/dist/httpd/patches/apply_to_2.2.19/ http://www.apache.org/dist/httpd/patches/apply_to_2.0.64/ We must advise that 1.3 is not affected, per our further research, although we can note that the default configuration (MaxClients etc) may already be inappropriate in any number of distributions, and remind administrators to tune their configuration to gracefully handle the maximum volume of requests.
Re: Next update
On Aug 31, 2011, at 7:20 AM, William A. Rowe Jr. wrote: We must advise that 1.3 is not affected, per our further research, although we can note that the default configuration (MaxClients etc) may already be inappropriate in any number of distributions, and remind administrators to tune their configuration to gracefully handle the maximum volume of requests. Or upgrade already. (: S. -- scte...@apache.orghttp://www.temme.net/sander/ PGP FP: FC5A 6FC6 2E25 2DFD 8007 EE23 9BB8 63B0 F51B B88A View my availability: http://tungle.me/sctemme
Re: Next update
On 31 Aug 2011, at 18:20, William A. Rowe Jr. wrote: Note some additional improvements for a 'final' update 3 advisory… Ack! Draft coming in half an hour or so, Dw.
Re: Next update
On 26 Aug 2011, at 18:05, William A. Rowe Jr. wrote: On 8/26/2011 11:41 AM, Eric Covener wrote: Should we bump the 5's in the draft advisory and/or code to a more liberal #? At the very least for the 2.0 rewrite solution that will return forbidden instead of full content? Can we please avoid sending more advisories without a canonical link to a wiki recommendations/discussion/observations page? As admins have already adopted the 5's, they are in the best position to tell us what broke, based on feedback from their audience. I'll start one shortly. URL ? Dw.
Re: Next update
Suggestion for http://people.apache.org/~dirkx/CVE-2011-3192.txt to be sent to announce and the usual security places. - Comments on weaken/strenghten 1.3 text Happy to completely recant that it was vulnerable. Or happy to keep a bit of a warning in there. - Lots of small tweaks. - Do we leave the 200/206 chunked/full range caveats in - or is that no longer the case ? Thanks, Dw.
Re: Next update
On 31 Aug 2011, at 21:03, Dirk-WIllem van Gulik wrote: Suggestion for http://people.apache.org/~dirkx/CVE-2011-3192.txt to be sent to announce and the usual security places. -Comments on weaken/strenghten 1.3 text Happy to completely recant that it was vulnerable. Or happy to keep a bit of a warning in there. -Lots of small tweaks. -Do we leave the 200/206 chunked/full range caveats in - or is that no longer the case ? Thanks, Ah - before I forget - also fine to not do it this heavy handed - but to sent Jim his message to users/devs@ to these security places as well. But am slightly biased to towards an advisory of this size - as it helps admins in large organizations negotiate priorities with their ops teams, bosses and others. Dw.
Re: Regression with range fix
On Wednesday 31 August 2011, Joe Orton wrote: On Tue, Aug 30, 2011 at 08:51:55PM +0200, Stefan Fritsch wrote: The first regression report, though slightly too late for the vote: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825 The byterange_filter.c in the Debian update is exactly the one from 2.2.20. I will keep you updated. Hi; I'm just back from holiday and catching up. The behaviour changes in the patch which could feasibly break non-compliant clients are: a) using 200 in some cases where a 206 response would end up being larger For the first user who complained, I think this is the problem. His client does a Range: bytes=0- request. I suspect the 200 (instead of 206) response to this request lets the client assume that the server does not support ranges at all. I will try to get this verified. b) using a chunked response where previously C-L was always used, in cases where =32 ranges are being returned Anything else to watch out for? c) a request with a byterange beyond the end of the file used to return 416 but now returns 200. This is a violation of a RFC2616 SHOULD. We didn't catch this when testing. This is how mplayer seems to determine that it has reached the end of file. This seems a rather stupid thing to do unless mplayer assumes that the file may grow. But as it's a SHOULD, we should fix it.
non-splittable buckets (was: Regression with range fix)
On Wednesday 31 August 2011, Jim Jagielski wrote: Looking at the patch in 2.2.x; there is a lot of effort expended deadling with apr_bucket_split() returning ENOTIMPL - that looks unnecessary; the filter will only handle brigades containing buckets with known length, and all such buckets must be _split-able. So you think we can rip out the whole if (rv == APR_ENOTIMPL) blocks? Belt and suspenders? If we rip it out, we should replace it with ap_assert()s. And maybe only do it in 2.3? A different idea I had: If apr_bucket_split() returns ENOTIMPL, we could call apr_brigade_partition on the copied brigade. apr_brigade_partition() would then do all the complex handling of apr_bucket_read(), etc. It is only slightly less efficient because it has to traverse the brigade again. But the memory usage would still stay low because the original brigade would remain untouched. And we would get rid of much of the complexity in copy_brigade_range().
Re: Regression with range fix
On 8/31/2011 4:00 PM, Stefan Fritsch wrote: On Wednesday 31 August 2011, Joe Orton wrote: Anything else to watch out for? c) a request with a byterange beyond the end of the file used to return 416 but now returns 200. This is a violation of a RFC2616 SHOULD. We didn't catch this when testing. This is how mplayer seems to determine that it has reached the end of file. This seems a rather stupid thing to do unless mplayer assumes that the file may grow. But as it's a SHOULD, we should fix it. Yup, that's a major flaw. If one range may be satisfied, we may not return 416, but must return those ranges which we are able. If no ranges are satisfiable, the answer needs to be restored to 416.
Re: svn commit: r1163833 - /httpd/httpd/trunk/modules/http/byterange_filter.c
On Aug 31, 2011, at 2:37 PM, s...@apache.org wrote: Author: sf Date: Wed Aug 31 21:37:38 2011 New Revision: 1163833 URL: http://svn.apache.org/viewvc?rev=1163833view=rev Log: Send a 206 response for a Range: bytes=0- request, even if 200 would be more efficient. 200 is a better response for caches as well. Why do we want to support this? Roy
vote for FINAL announce
On 31 Aug 2011, at 21:07, Dirk-WIllem van Gulik wrote: http://people.apache.org/~dirkx/CVE-2011-3192.txt Off to bed now. If there is consensus we are 'done' then a vote on either of these options would be of use 1 as above 2 as above but with complete reversal of the 1.3 situation - state it is totally not affected (i.e. when configure right). 3 as above - but with a large caveat that we're not yet happy with this - follow this wiki page and keep an eye out for the release notes of the next releases. Aiming at 1000Z tomorrow - as that gives folks in big companies a lot of planning room - and is not yet hitting the friday-thou-shall-not-break-things-before-the-weekend. Thanks, Dw.
Re: Advisory: Range header DoS vulnerability Apache HTTPD 1.3/2.x (CVE-2011-3192)
Folks, See below - for the 1.3 discussion - that suggest we should take it a notch down: On 31 Aug 2011, at 22:35, Munechika Sumikawa wrote: We're currently discussing this - and will propably adjust the announcement a bit. It is vulnerable in that it can suddenly take a lot more CPU, memory and resources when 'attacked'. And the response is worse than pure linear. But unlike 2.0 and 2.2 it does not exploded as exponential. So at this point I am expecting us to conclude that 1.3 is 'as affected' as most other servers implementing this protocol; not due to a fault in the code - but more to a fault in the protocol desgin. Does that make sense ? Let me confirm the code. Apache 1.3 allocates only several bytes per each byte-range to record first-pos and last-pos. And the memory is released immediately after the HTTP session is disconnected. Thus, it's impossible a cracker succeed to DoS 1.3x server with the paranoia range header. Am I correct? If so, IMO Apache 1.3's behavior should be normal case. More complicated pattern based on the designed protocol eat up more resources than simpler pattern. That always happens in any protocols. (e.g. IP fragmentation) I think it's stll in scope of linear even though it's not pure linear. Which makes good sense. And looking at the default 1.3 configs on the standard platforms of that time - it is indeed not really apache which is at fault. Dw.
Re: svn commit: r1163833 - /httpd/httpd/trunk/modules/http/byterange_filter.c
On Wednesday 31 August 2011, Roy T. Fielding wrote: Author: sf Date: Wed Aug 31 21:37:38 2011 New Revision: 1163833 URL: http://svn.apache.org/viewvc?rev=1163833view=rev Log: Send a 206 response for a Range: bytes=0- request, even if 200 would be more efficient. 200 is a better response for caches as well. Why do we want to support this? There seems to be at least one streaming client that gets confused by this behaviour. But maybe my commit was premature. If the change does not actually fix the problem, I will revert.
Re: svn commit: r1163833 - /httpd/httpd/trunk/modules/http/byterange_filter.c
On 8/31/2011 6:06 PM, Stefan Fritsch wrote: On Wednesday 31 August 2011, Roy T. Fielding wrote: Author: sf Date: Wed Aug 31 21:37:38 2011 New Revision: 1163833 URL: http://svn.apache.org/viewvc?rev=1163833view=rev Log: Send a 206 response for a Range: bytes=0- request, even if 200 would be more efficient. 200 is a better response for caches as well. Why do we want to support this? As explained to me by Stefan on irc... There seems to be at least one streaming client that gets confused by this behaviour. But maybe my commit was premature. If the change does not actually fix the problem, I will revert. The presumption here is that the client requests bytes=0- to begin the transmission, and provided it sees a 206, restarting somewhere in the stream results in aborting the connection and streaming bytes=n- from the restart point. Further testing should determine if this was the broken assumption.
Re: svn commit: r1163833 - /httpd/httpd/trunk/modules/http/byterange_filter.c
On Aug 31, 2011, at 6:10 PM, William A. Rowe Jr. wrote: On 8/31/2011 6:06 PM, Stefan Fritsch wrote: On Wednesday 31 August 2011, Roy T. Fielding wrote: Author: sf Date: Wed Aug 31 21:37:38 2011 New Revision: 1163833 URL: http://svn.apache.org/viewvc?rev=1163833view=rev Log: Send a 206 response for a Range: bytes=0- request, even if 200 would be more efficient. 200 is a better response for caches as well. Why do we want to support this? As explained to me by Stefan on irc... There seems to be at least one streaming client that gets confused by this behaviour. But maybe my commit was premature. If the change does not actually fix the problem, I will revert. The presumption here is that the client requests bytes=0- to begin the transmission, and provided it sees a 206, restarting somewhere in the stream results in aborting the connection and streaming bytes=n- from the restart point. Further testing should determine if this was the broken assumption. Do we send the Accept-Ranges header field? http://tools.ietf.org/html/rfc2616#page-105 Roy
Re: Appropriate patches for 2.2.19 and 2.0.64?
On 8/31/2011 4:16 PM, William A. Rowe Jr. wrote:
I've attempted to simply substitute the 2.2.19 filter code into the
2.0.64 http_protocol.c sources, and am unsure how far off these patches
are from what they need to be; there's been a significant amount of drift
and refactoring in the interim.
Still looking for feedback, but the attached applies and corresponds to
2.2.20 with the exception of atoi rather than strtoi semantics, and without
the no DefaultType exception..
Index: modules/http/http_protocol.c
===
--- modules/http/http_protocol.c(revision 1163022)
+++ modules/http/http_protocol.c(working copy)
@@ -2893,56 +2893,9 @@
apr_table_setn(r-headers_out, ETag, etag);
}
-static int parse_byterange(char *range, apr_off_t clength,
- apr_off_t *start, apr_off_t *end)
-{
-char *dash = strchr(range, '-');
+static int ap_set_byterange(request_rec *r, apr_off_t clength,
+apr_array_header_t **indexes);
-if (!dash) {
-return 0;
-}
-
-if ((dash == range)) {
-/* In the form -5 */
-*start = clength - apr_atoi64(dash + 1);
-*end = clength - 1;
-}
-else {
-*dash = '\0';
-dash++;
-*start = apr_atoi64(range);
-if (*dash) {
-*end = apr_atoi64(dash);
-}
-else { /* 5- */
-*end = clength - 1;
-}
-}
-
-if (*start 0) {
-*start = 0;
-}
-
-if (*end = clength) {
-*end = clength - 1;
-}
-
-if (*start *end) {
-return -1;
-}
-
-return (*start 0 || *end clength);
-}
-
-static int ap_set_byterange(request_rec *r);
-
-typedef struct byterange_ctx {
-apr_bucket_brigade *bb;
-int num_ranges;
-char *boundary;
-char *bound_head;
-} byterange_ctx;
-
/*
* Here we try to be compatible with clients that want multipart/x-byteranges
* instead of multipart/byteranges (also see above), as per HTTP/1.1. We
@@ -2959,27 +2912,200 @@
}
#define BYTERANGE_FMT % APR_OFF_T_FMT -% APR_OFF_T_FMT /% APR_OFF_T_FMT
-#define PARTITION_ERR_FMT apr_brigade_partition() failed \
- [% APR_OFF_T_FMT ,% APR_OFF_T_FMT ]
+static apr_status_t copy_brigade_range(apr_bucket_brigade *bb,
+ apr_bucket_brigade *bbout,
+ apr_off_t start,
+ apr_off_t end)
+{
+apr_bucket *first = NULL, *last = NULL, *out_first = NULL, *e;
+apr_uint64_t pos = 0, off_first = 0, off_last = 0;
+apr_status_t rv;
+const char *s;
+apr_size_t len;
+apr_uint64_t start64, end64;
+apr_off_t pofft = 0;
+
+/*
+ * Once we know that start and end are = 0 convert everything to
apr_uint64_t.
+ * See the comments in apr_brigade_partition why.
+ * In short apr_off_t (for values = 0)and apr_size_t fit into
apr_uint64_t.
+ */
+start64 = (apr_uint64_t)start;
+end64 = (apr_uint64_t)end;
+
+if (start 0 || end 0 || start64 end64)
+return APR_EINVAL;
+
+for (e = APR_BRIGADE_FIRST(bb);
+ e != APR_BRIGADE_SENTINEL(bb);
+ e = APR_BUCKET_NEXT(e))
+{
+apr_uint64_t elen64;
+/* we know that no bucket has undefined length (-1) */
+AP_DEBUG_ASSERT(e-length != (apr_size_t)(-1));
+elen64 = (apr_uint64_t)e-length;
+if (!first (elen64 + pos start64)) {
+first = e;
+off_first = pos;
+}
+if (elen64 + pos end64) {
+last = e;
+off_last = pos;
+break;
+}
+pos += elen64;
+}
+if (!first || !last)
+return APR_EINVAL;
+
+e = first;
+while (1)
+{
+apr_bucket *copy;
+AP_DEBUG_ASSERT(e != APR_BRIGADE_SENTINEL(bb));
+rv = apr_bucket_copy(e, copy);
+if (rv != APR_SUCCESS) {
+apr_brigade_cleanup(bbout);
+return rv;
+}
+
+APR_BRIGADE_INSERT_TAIL(bbout, copy);
+if (e == first) {
+if (off_first != start64) {
+rv = apr_bucket_split(copy, (apr_size_t)(start64 - off_first));
+if (rv == APR_ENOTIMPL) {
+rv = apr_bucket_read(copy, s, len, APR_BLOCK_READ);
+if (rv != APR_SUCCESS) {
+apr_brigade_cleanup(bbout);
+return rv;
+}
+/*
+ * The read above might have morphed copy in a bucket
+ * of shorter length. So read and delete until we reached
+ * the correct bucket for splitting.
+ */
+while (start64 - off_first (apr_uint64_t)copy-length) {
+apr_bucket *tmp = APR_BUCKET_NEXT(copy);
+
