RE: Time for 2.4.11
Hi All, I just want to ping again to see if there is any updates on this? Thanks, Yingqi -Original Message- From: Lu, Yingqi [mailto:yingqi...@intel.com] Sent: Friday, January 09, 2015 9:57 AM To: dev@httpd.apache.org Subject: RE: Time for 2.4.11 Hi Jim, Thanks for your email. I think it should not be very hard to back port. After you trunked the original patch last June, I was working with Yann Ylavic last November to fix some minor issues. With current trunked code, there is no major API change to 2.4 version and we have tested with multiple workloads and usage cases for all 4 existing MPMs. It looks good to us. Please note, with current code, there is a new configurable flag called "ListenCoresBucketsRatio". The default value is 0 which means SO_REUSEPORT is disabled. This is different than the original patch. The reason Yann decided to choose the opt-in way because he finds it safer, especially for backports to stable. Given this said, I think it would be a good idea to add some document to introduce the feature and the flag itself. This would allow users to take advantage of this. Please let me know if you have any questions. Again, thanks very much for the help, really appreciated! The whole work can be followed in three threads with name: 1. "[PATCH ASF bugzilla# 55897]prefork_mpm patch with SO_REUSEPORT support" 2. "svn commit: r1599531 - in /httpd/httpd/trunk: CHANGES include/ap_listen.h server/listen.c server/mpm/event/event.c server/mpm/prefork/prefork.c server/mpm/worker/worker.c server/mpm_unix.c" 3. "Listeners buckets and duplication w/ and w/o SO_REUSEPORT on trunk" Thanks, Yingqi -Original Message- From: Jim Jagielski [mailto:j...@jagunet.com] Sent: Friday, January 09, 2015 5:47 AM To: dev@httpd.apache.org Subject: Re: Time for 2.4.11 Let me look... how easy is the backport? > On Jan 8, 2015, at 12:22 PM, Lu, Yingqi wrote: > > Hi All, > > Can we make the "SO_REUSEPORT" support into this new stable version? The > first version of the patch was trunked last June. After tests and > modifications, I think it is ready to go. > > Please let me know what you think. > > Thanks, > Yingqi > > -Original Message- > From: Jim Jagielski [mailto:j...@jagunet.com] > Sent: Thursday, January 08, 2015 3:12 AM > To: httpd > Subject: Time for 2.4.11 > > Let's shoot for a T&R next week. The work will keep me warm :)
Re: Re: CVE-2013-5704 fix breaks mod_wsgi
> But the damage has been done for some months on 2.2, and we are noticing this, now? All distros still shipping Apache 2.2 still are using older mod_wsgi 3.X versions which I don't at this point believe are affected by this issue. People who build stuff from source code themselves would be using latest Apache 2.4. So the big hit on mod_wsgi will come with Apache 2.4.11. Graham
RE: Re: CVE-2013-5704 fix breaks mod_wsgi
- Original Message - Subject: Re: CVE-2013-5704 fix breaks mod_wsgi From: "Joe Orton" Date: 1/12/15 11:05 am To: dev@httpd.apache.org On Mon, Jan 12, 2015 at 11:25:53AM -0500, Eric Covener wrote: > On Fri, Jan 9, 2015 at 3:23 PM, Joe Orton wrote: > > Either way, the fix for CVE-2013-5704 ends up breaking backwards > > compatibility with existing 2.4.x builds of mod_wsgi, which is kind of > > Bad. I don't have a good proposal for how to fix or avoid this. Worst > > case, we make clear the mod_wsgi case is API/ABI abuse and warn binary > > distributors they have to handle this by rebuilding. > > Is there anything we can do in 2.4.11 for packagers who haven't picked > this up yet since we're already picking up a problematic extension of > the struct? > > What if we stashed away the MMN after these fields, and validated it? > Or just a request_rec version? It would be possible to do some hack. Say, stash something in r->notes that this is a "real" request_rec, and check for that before accessing r->trailers (which only happens in one place). There may well be a cheaper way than modifying r->notes. Not really useful, though. You could extend ap_mpm_query easily enough but it doesn't win us anything in this version, or the 2.2 branch. We should be looking ahead at a more effective (although not more 'strict') ABI/API contract for 2.6/3.0. But the damage has been done for some months on 2.2, and we are noticing this, now? I don't think there's anything to be done w.r.t. this issue in 2.4.11. Once the module is compiled against the later MMN, there is no harm in initializing the extra fields, even if loaded against an earlier httpd core. But the MMN here is absolutely useless. What we would have needed had we foreseen it and module authors adopted it... was a reversioning of each struct within httpd. I don't care that my loaded module knows of an earlier compatible conn_rec if the only thing it allocates is the request_req. The raw MMN value provides no insight. Who is to say what is a 'real' request rec? No such beast if the module implements the request_rec entirely on its own. Yes, mod_ftp, and mod_wsgi will have some pain from this enhancement. We eat it and move on, IMHO, and look forward to a new 2.x/3.0 world where we can introduce the semantics of incomplete types, versioned struct declarations and alloc/initializers where they are called for.
Re: svn commit: r1651084 - in /httpd/httpd/branches/2.4.x: CHANGES STATUS include/ap_mmn.h include/http_log.h server/log.c
Hi, the commit message is wrong for this one. Cut'n paste error from the previous one. Won't have time myself to fix it in the comming days. Best regards CJ Le 12/01/2015 14:39, j...@apache.org a écrit : Author: jim Date: Mon Jan 12 13:39:07 2015 New Revision: 1651084 URL: http://svn.apache.org/r1651084 Log: Merge r1643825 from trunk: * core: Fix -D[efined] or [d] variables lifetime accross restarts. PR 57328. Submitted-by: Armin Abfalterer Reviewed/Committed-by: ylavic Submitted by: ylavic Reviewed/backported by: jim Modified: httpd/httpd/branches/2.4.x/CHANGES httpd/httpd/branches/2.4.x/STATUS httpd/httpd/branches/2.4.x/include/ap_mmn.h httpd/httpd/branches/2.4.x/include/http_log.h httpd/httpd/branches/2.4.x/server/log.c Modified: httpd/httpd/branches/2.4.x/CHANGES URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1651084&r1=1651083&r2=1651084&view=diff == --- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original) +++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Mon Jan 12 13:39:07 2015 @@ -46,6 +46,9 @@ Changes with Apache 2.4.11 *) mod_ssl: Fix recognition of OCSP stapling responses that are encoded improperly or too large. [Jeff Trawick] + *) core: Add ap_log_data(), ap_log_rdata(), etc. for logging buffers. + [Jeff Trawick] + *) mod_proxy_fcgi, mod_authnz_fcgi: stop reading the response and issue an error when parsing or forwarding the response fails. [Yann Ylavic]
RE: Re: CVE-2013-5704 fix breaks mod_wsgi
- Original Message - Subject: Re: CVE-2013-5704 fix breaks mod_wsgi From: "Joe Orton" Date: 1/12/15 5:27 am To: "Graham Dumpleton" Cc: "dev@httpd.apache.org" On Sat, Jan 10, 2015 at 09:04:12AM +1100, Graham Dumpleton wrote: > 1. Verify that recompiling mod_wsgi is actually sufficient given than my > direct use of request_rec isn't going to populate the extra fields and they > will remain NULL still. As trailers shouldn't be expected in context the > request_rec is being used directly by mod_wsgi those attributes shouldn't > be touched, but if that is the case, why would it be crashing without > recompilation happening. So need to also actually verify whether it can't > limp on as is for now if it isn't crashing. Yup, I should have mentioned that too. You are right, we had to pach mod_wsgi to fix the issue properly as well: http://pkgs.fedoraproject.org/cgit/mod_wsgi.git/plain/mod_wsgi-4.4.3-trailers.patch?h=f21 that can/should be surrounded with #if AP_MODULE_MAGIC_AT_LEAST(20120211, 37) ... #endif ...to make it conditional on an httpd with those fields. (Hadn't submitted that back upstream yet sorry - we wanted to find a "proper" solution httpd-side for this.) Exactly. These fixes are already in for httpd's mod_ftp svn. I had also missed this while building that module and hadn't put it through its paces, as we were preparing 2.2. The proper solution is to never let the module create a structure such as request_req. This would normally be accomplished by semi-opaque/incomplete data types, which weren't portably possible in '00 but might be realistic today. Whether we expose a complete or incomplete datatype - that solution is to expose the object's allocator/initializer. OO programming topic 101.
Re: CVE-2013-5704 fix breaks mod_wsgi
On Mon, Jan 12, 2015 at 11:25:53AM -0500, Eric Covener wrote: > On Fri, Jan 9, 2015 at 3:23 PM, Joe Orton wrote: > > Either way, the fix for CVE-2013-5704 ends up breaking backwards > > compatibility with existing 2.4.x builds of mod_wsgi, which is kind of > > Bad. I don't have a good proposal for how to fix or avoid this. Worst > > case, we make clear the mod_wsgi case is API/ABI abuse and warn binary > > distributors they have to handle this by rebuilding. > > Is there anything we can do in 2.4.11 for packagers who haven't picked > this up yet since we're already picking up a problematic extension of > the struct? > > What if we stashed away the MMN after these fields, and validated it? > Or just a request_rec version? It would be possible to do some hack. Say, stash something in r->notes that this is a "real" request_rec, and check for that before accessing r->trailers (which only happens in one place). There may well be a cheaper way than modifying r->notes.
Re: CVE-2013-5704 fix breaks mod_wsgi
On Mon, Jan 12, 2015 at 11:25 AM, Eric Covener wrote: > Part of that question is probably "who else has figured out how to cope ignore this trailing bit.
Re: CVE-2013-5704 fix breaks mod_wsgi
On Fri, Jan 9, 2015 at 3:23 PM, Joe Orton wrote: > Either way, the fix for CVE-2013-5704 ends up breaking backwards > compatibility with existing 2.4.x builds of mod_wsgi, which is kind of > Bad. I don't have a good proposal for how to fix or avoid this. Worst > case, we make clear the mod_wsgi case is API/ABI abuse and warn binary > distributors they have to handle this by rebuilding. Is there anything we can do in 2.4.11 for packagers who haven't picked this up yet since we're already picking up a problematic extension of the struct? What if we stashed away the MMN after these fields, and validated it? Or just a request_rec version? That way we'd be be able to read past the end and blow up intentionally w/ a message, but wouldn't be writing past the end and blowing up much confusingly. I think a big factor in whether we do something is how many modules might have already been corrected for it. I am thinking not many if we haven't heard about yet (other than mod_ftpd). Part of that question is probably "who else has figured out how to cope
Fwd: svn commit: r1651090 - /httpd/httpd/branches/2.4.x/STATUS
Would like to get this one in 2.4.11 if any eyes available. -- Forwarded message -- From: Date: Mon, Jan 12, 2015 at 8:45 AM Subject: svn commit: r1651090 - /httpd/httpd/branches/2.4.x/STATUS To: c...@httpd.apache.org Author: covener Date: Mon Jan 12 13:45:47 2015 New Revision: 1651090 URL: http://svn.apache.org/r1651090 Log: propose conn_rec.id fix Modified: httpd/httpd/branches/2.4.x/STATUS Modified: httpd/httpd/branches/2.4.x/STATUS URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1651090&r1=1651089&r2=1651090&view=diff == --- httpd/httpd/branches/2.4.x/STATUS (original) +++ httpd/httpd/branches/2.4.x/STATUS Mon Jan 12 13:45:47 2015 @@ -247,6 +247,11 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK: 2.4.x patch: 1st trunk patch works, 2nd is for CHANGES +1 jailletc36, covener + * event: Update conn_rec.id when a new thread begins working on a connection, because + the old thread may work on a new connection and assign the same ID in parallel. PR57435 + trunk patch: http://svn.apache.org/r1651088 + 2.4.x patch: trunk works + +1 covener OTHER PROPOSALS -- Eric Covener cove...@gmail.com
Re: CVE-2013-5704 fix breaks mod_wsgi
BTW. I need to go back and check, but I actually suspect that the crash will only occur in mod_wsgi where mod_wsgi 4.4.0 or later was being used. It was only in 4.4.0 that content started to be passed between the Apache child worker processes and the mod_wsgi daemon process using chunking. The WSGI specification doesn't actually permit chunked request content or the idea of Apache input filters that can change the length of the request content but not change the content length. I did have a WSGIChunkedRequests directive to allow you to side step the WSGI specification and allow your application to work with these, but it was broken for mod_wsgi daemon mode and only worked for embedded mode. The change in 4.4.0 fixed that and at the same time had to start using chunking so could detect when request content had been truncated before all being received. If this turns out to be the case, that may limit damage a big, as most distros are still on ancient mod_wsgi versions and not using mod_wsgi 4.X versions at all. They wouldn't therefore need to patch the older versions I don't think, nor even possibly recompile them. I will do some checking with older versions tomorrow, as well as work on the hack that tries to infer the request_rec size to work out if the CVE change has been back ported. Graham On 12 January 2015 at 23:20, Graham Dumpleton wrote: > On 12 January 2015 at 22:27, Joe Orton wrote: > >> On Sat, Jan 10, 2015 at 09:04:12AM +1100, Graham Dumpleton wrote: >> > 1. Verify that recompiling mod_wsgi is actually sufficient given than my >> > direct use of request_rec isn't going to populate the extra fields and >> they >> > will remain NULL still. As trailers shouldn't be expected in context the >> > request_rec is being used directly by mod_wsgi those attributes >> shouldn't >> > be touched, but if that is the case, why would it be crashing without >> > recompilation happening. So need to also actually verify whether it >> can't >> > limp on as is for now if it isn't crashing. >> >> Yup, I should have mentioned that too. You are right, we had to pach >> mod_wsgi to fix the issue properly as well: >> >> >> http://pkgs.fedoraproject.org/cgit/mod_wsgi.git/plain/mod_wsgi-4.4.3-trailers.patch?h=f21 >> >> that can/should be surrounded with >> >> #if AP_MODULE_MAGIC_AT_LEAST(20120211, 37) >> ... >> #endif >> >> ...to make it conditional on an httpd with those fields. (Hadn't >> submitted that back upstream yet sorry - we wanted to find a "proper" >> solution httpd-side for this.) >> > > The problem I have is that Linux distros who are back porting the change > aren't going to be updating MODULE_MAGIC_NUMBER. > > So if I add: > > #if AP_MODULE_MAGIC_AT_LEAST(20120211, 37) > r->trailers_in = apr_table_make(r->pool, 5); > r->trailers_out = apr_table_make(r->pool, 5); > #endif > > This only helps if someone is using Apache 2.4.11 or later where > MODULE_MAGIC_NUMBER has been updated. > > Someone who takes the latest mod_wsgi code with the above change and > compiles it against an Apache with back ported change will still find that > mod_wsgi will crash as that code will never be compiled in. > > In short, I can't see that I have any way of detecting that an Apache > instance which has back ported the change is being used at compile time. > > The only hack I can think of, is that for where > AP_MODULE_MAGIC_AT_LEAST(20120211, 37) doesn't succeed, I try and calculate > whether the 'useragent_ip' member of the structure is still likely to be > the last thing that can fit in the amount of memory allocated for the > request_rec and if it isn't and the following memory is enough to hold the > trailers_in and trailers_out pointers, that i somehow work out where they > would fall and set them. This could be fiddly because of struct packing > issues. > > I know it is asking a lot and likely too late, but what would have helped > immensely in such cases where back ports occur which change structure sizes > is that a #define was added within the struct where the new members were > added. > > char *useragent_ip; > > #define CVE_2013_5704 1 > > /** MIME trailer environment from the request */ > apr_table_t *trailers_in; > /** MIME trailer environment from the response */ > apr_table_t *trailers_out; > }; > > I know this goes against the MODULE_MAGIC_NUMBER idea, but the magic > number doesn't help with back ported changes like this. > > With such #define's then I could have had: > > #if AP_MODULE_MAGIC_AT_LEAST(20120211, 37) || defined(CVE_2013_5704) > r->trailers_in = apr_table_make(r->pool, 5); > r->trailers_out = apr_table_make(r->pool, 5); > #endif > > So right now it looks like I have to use the rather fragile approach of > trying to work out whether the size of request_rec is no larger without > actually being able to access the members, which if they didn't exist would > cause a compile time failure. > > Graham > > >
Re: CVE-2013-5704 fix breaks mod_wsgi
On 12 January 2015 at 22:27, Joe Orton wrote: > On Sat, Jan 10, 2015 at 09:04:12AM +1100, Graham Dumpleton wrote: > > 1. Verify that recompiling mod_wsgi is actually sufficient given than my > > direct use of request_rec isn't going to populate the extra fields and > they > > will remain NULL still. As trailers shouldn't be expected in context the > > request_rec is being used directly by mod_wsgi those attributes shouldn't > > be touched, but if that is the case, why would it be crashing without > > recompilation happening. So need to also actually verify whether it can't > > limp on as is for now if it isn't crashing. > > Yup, I should have mentioned that too. You are right, we had to pach > mod_wsgi to fix the issue properly as well: > > > http://pkgs.fedoraproject.org/cgit/mod_wsgi.git/plain/mod_wsgi-4.4.3-trailers.patch?h=f21 > > that can/should be surrounded with > > #if AP_MODULE_MAGIC_AT_LEAST(20120211, 37) > ... > #endif > > ...to make it conditional on an httpd with those fields. (Hadn't > submitted that back upstream yet sorry - we wanted to find a "proper" > solution httpd-side for this.) > The problem I have is that Linux distros who are back porting the change aren't going to be updating MODULE_MAGIC_NUMBER. So if I add: #if AP_MODULE_MAGIC_AT_LEAST(20120211, 37) r->trailers_in = apr_table_make(r->pool, 5); r->trailers_out = apr_table_make(r->pool, 5); #endif This only helps if someone is using Apache 2.4.11 or later where MODULE_MAGIC_NUMBER has been updated. Someone who takes the latest mod_wsgi code with the above change and compiles it against an Apache with back ported change will still find that mod_wsgi will crash as that code will never be compiled in. In short, I can't see that I have any way of detecting that an Apache instance which has back ported the change is being used at compile time. The only hack I can think of, is that for where AP_MODULE_MAGIC_AT_LEAST(20120211, 37) doesn't succeed, I try and calculate whether the 'useragent_ip' member of the structure is still likely to be the last thing that can fit in the amount of memory allocated for the request_rec and if it isn't and the following memory is enough to hold the trailers_in and trailers_out pointers, that i somehow work out where they would fall and set them. This could be fiddly because of struct packing issues. I know it is asking a lot and likely too late, but what would have helped immensely in such cases where back ports occur which change structure sizes is that a #define was added within the struct where the new members were added. char *useragent_ip; #define CVE_2013_5704 1 /** MIME trailer environment from the request */ apr_table_t *trailers_in; /** MIME trailer environment from the response */ apr_table_t *trailers_out; }; I know this goes against the MODULE_MAGIC_NUMBER idea, but the magic number doesn't help with back ported changes like this. With such #define's then I could have had: #if AP_MODULE_MAGIC_AT_LEAST(20120211, 37) || defined(CVE_2013_5704) r->trailers_in = apr_table_make(r->pool, 5); r->trailers_out = apr_table_make(r->pool, 5); #endif So right now it looks like I have to use the rather fragile approach of trying to work out whether the size of request_rec is no larger without actually being able to access the members, which if they didn't exist would cause a compile time failure. Graham
Re: CVE-2013-5704 fix breaks mod_wsgi
On Sat, Jan 10, 2015 at 07:38:03AM -0500, Jeff Trawick wrote:
> On Fri, Jan 9, 2015 at 3:48 PM, Jeff Trawick wrote:
> > * Add helper functions to allocate a request_rec, conn_rec, server_rec.
> > It doesn't solve all possible problems of course but can drastically reduce
> > the frequency of needing to recompile a module that needs to do such things.
>
> Actually, ap_{request_rec|conn_rec|server_rec}_size would be much better;
> that supports allocation, copy, as well as "Pfft! You better recompile me."
I think one of these solutions is probably necessary, but I'm not sure
which.
1) Only exporting the struct sizes means httpd cannot initialize the
structs, which is both a problem in general for extending structs and
specifically for this case (per my other mail).
2) Only exporting the struct sizes means we don't have to invent a new
API and decide what "initialize a request_rec (etc)" really means and
add new API/ABI guarantees there which we might screw up later.
3) We could have a stricter API rule which says "you can extend
request_rec (etc), but assume third-party modules might initialize the
the fields to zero." This kind of guarantee might be a tough
commitment.
It's definitely a "don't start from here" problem.
Regards, Joe
Re: CVE-2013-5704 fix breaks mod_wsgi
On Sat, Jan 10, 2015 at 09:04:12AM +1100, Graham Dumpleton wrote: > 1. Verify that recompiling mod_wsgi is actually sufficient given than my > direct use of request_rec isn't going to populate the extra fields and they > will remain NULL still. As trailers shouldn't be expected in context the > request_rec is being used directly by mod_wsgi those attributes shouldn't > be touched, but if that is the case, why would it be crashing without > recompilation happening. So need to also actually verify whether it can't > limp on as is for now if it isn't crashing. Yup, I should have mentioned that too. You are right, we had to pach mod_wsgi to fix the issue properly as well: http://pkgs.fedoraproject.org/cgit/mod_wsgi.git/plain/mod_wsgi-4.4.3-trailers.patch?h=f21 that can/should be surrounded with #if AP_MODULE_MAGIC_AT_LEAST(20120211, 37) ... #endif ...to make it conditional on an httpd with those fields. (Hadn't submitted that back upstream yet sorry - we wanted to find a "proper" solution httpd-side for this.) Regards, Joe
