Re: svn commit: r1809192 - /httpd/site/trunk/content/security/vulnerabilities-httpd.xml
What more would we want to say here? Mention that the Allow: header may respond with corrupted output? It seems other side effects can be present, which is why I kept this simple. On Thu, Sep 21, 2017 at 1:33 PM, wrote: > Author: wrowe > Date: Thu Sep 21 18:33:47 2017 > New Revision: 1809192 > > URL: http://svn.apache.org/viewvc?rev=1809192&view=rev > Log: > Record CVE-2017-9798 > > Modified: > httpd/site/trunk/content/security/vulnerabilities-httpd.xml > > Modified: httpd/site/trunk/content/security/vulnerabilities-httpd.xml > URL: > http://svn.apache.org/viewvc/httpd/site/trunk/content/security/vulnerabilities-httpd.xml?rev=1809192&r1=1809191&r2=1809192&view=diff > == > --- httpd/site/trunk/content/security/vulnerabilities-httpd.xml (original) > +++ httpd/site/trunk/content/security/vulnerabilities-httpd.xml Thu Sep 21 > 18:33:47 2017 > @@ -1,4 +1,99 @@ > - > + > + > + > + > +low > +Use-after-free when usingwith an unrecognized method > in .htaccess ("OptionsBleed") > + > +When an unrecognized HTTP Method is given in an > +directive in an .htaccess file, and that .htaccess file is processed by the > +corresponding request, the global methods table is corrupted in the current > +worker process, resulting in erratic behaviour. > +This behavior may be avoided by listing all unusual HTTP Methods in a > global > +httpd.conf RegisterHttpMethod directive in httpd release 2.4.25 and > later. > +To permit other .htaccess directives while denying the > directive, see the AllowOverrideList directive. > +Source code patch is at; > + > + href="http://www.apache.org/dist/httpd/patches/apply_to_2.4.27/CVE-2017-9798-patch-2.4.patch"; > +>http://www.apache.org/dist/httpd/patches/apply_to_2.4.27/CVE-2017-9798-patch-2.4.patch > + > + > + > +We would like to thank Hanno Böck for reporting this issue. > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > +low > +Use-after-free when using with an unrecognized method > in .htaccess ("OptionsBleed") > + > +When an unrecognized HTTP Method is given in an > +directive in an .htaccess file, and that .htaccess file is processed by the > +corresponding request, the global methods table is corrupted in the current > +worker process, resulting in erratic behaviour. > +This behavior may be avoided by listing all unusual HTTP Methods in a > global > +httpd.conf RegisterHttpMethod directive in httpd release 2.2.32 and > later. > +To permit other .htaccess directives while denying the > directive, see the AllowOverrideList directive. > +Source code patch is at; > + > + href="http://www.apache.org/dist/httpd/patches/apply_to_2.2.34/CVE-2017-9798-patch-2.4.patch"; > +>http://www.apache.org/dist/httpd/patches/apply_to_2.2.34/CVE-2017-9798-patch-2.2.patch > + > +Note 2.2 is end-of-life, no further release with this fix is planned. > Users > +are encouraged to migrate to 2.4.28 or later for this and other fixes. > + > + > +We would like to thank Hanno Böck for reporting this issue. > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > + > > released="20170711"> > > >
Re: configure option --enable-option-checking warns about things it does know (httpd-2.2.X)
Thanks for the report Michael. The 2.2.x series is now retired and end-of-life. The warnings are no-ops... they are inherited to child ./configure bits so the basic httpd-2.x/configure may holler about options only applicable to the bundled packages, and the bundled packages may holler about options only applicable to httpd. We have unbundled the dependencies from httpd 2.4, apr and apr-util 1.6, so expat -> pcre -> zlib -> apr -> apr-util -> httpd must be build independently, and this side-effect of passing around unrecognized configure flags is moot. Bill On Thu, Sep 21, 2017 at 6:55 AM, Michael wrote: > Just thought I would mention option-checking in httpd-2.2.X is borked. > > Fortunately, it just warns :) > > A small subset of the warnings: > > configure: WARNING: unrecognized options: --with-z, --enable-layout, > --with-apr, --with-apr-util, --with-mpm, --enable-ssl, --enable-proxy, > --enable-mods-shared > > Regards, > > Michael > > p.s. I have not been following the maillist for quite a while - so if this > is already known, please ignore. > >
Re: Time for 2.4.28 ?
Looks like we need 1 more vote on it for it to be folded in on time for 2.4.28. > On Sep 19, 2017, at 1:48 PM, Jim Jagielski wrote: > > There have been no issues w/ that on trunk... will > fold into 2.4 and do some stress testing over the next > 2 days. > >> On Sep 19, 2017, at 12:58 PM, Yann Ylavic wrote: >> >> On Tue, Sep 19, 2017 at 6:41 PM, Stefan Priebe - Profihost AG >> wrote: >>> >>> Do we get this one into 2.4.28? >>> >>> https://bz.apache.org/bugzilla/show_bug.cgi?id=60956 >> >> Proposed for backport now, will see... >> >> Regards, >> Yann. >
Re: SSLSrvConfigRec shared
On Thu, Sep 21, 2017 at 2:51 PM, Eric Covener wrote: > On Thu, Sep 21, 2017 at 8:44 AM, Yann Ylavic wrote: >> On Thu, Sep 21, 2017 at 2:11 PM, Eric Covener wrote: >>> >>> IIUC it should be safe to extend module_struct with a minor bump to >>> add 'int flags' to the bottom, but when you check the value you'd need >>> to check the MMN first. In the module you'd then just have some flags >>> or'ed together after register_hooks. >> >> Something like the attached patch might do it (untested, no MMN minor bump). >> >>> >>> (hopefully someone will check my work) >> >> Since modules (module_struct) are déclared globally, unspecified >> fields at the end of the struct should be initialized to zero, so it >> should be safe. > > I was thinking about modules compiled against the previous definition > / out of tree. Right, it's missing some #ifdefs MMN too :)
Re: SSLSrvConfigRec shared
On Thu, Sep 21, 2017 at 8:44 AM, Yann Ylavic wrote: > On Thu, Sep 21, 2017 at 2:11 PM, Eric Covener wrote: >> >> IIUC it should be safe to extend module_struct with a minor bump to >> add 'int flags' to the bottom, but when you check the value you'd need >> to check the MMN first. In the module you'd then just have some flags >> or'ed together after register_hooks. > > Something like the attached patch might do it (untested, no MMN minor bump). > >> >> (hopefully someone will check my work) > > Since modules (module_struct) are déclared globally, unspecified > fields at the end of the struct should be initialized to zero, so it > should be safe. I was thinking about modules compiled against the previous definition / out of tree. -- Eric Covener cove...@gmail.com
Re: SSLSrvConfigRec shared
On Thu, Sep 21, 2017 at 2:11 PM, Eric Covener wrote:
>
> IIUC it should be safe to extend module_struct with a minor bump to
> add 'int flags' to the bottom, but when you check the value you'd need
> to check the MMN first. In the module you'd then just have some flags
> or'ed together after register_hooks.
Something like the attached patch might do it (untested, no MMN minor bump).
>
> (hopefully someone will check my work)
Since modules (module_struct) are déclared globally, unspecified
fields at the end of the struct should be initialized to zero, so it
should be safe.
Index: include/http_config.h
===
--- include/http_config.h (revision 1809129)
+++ include/http_config.h (working copy)
@@ -329,6 +329,12 @@ struct cmd_parms_struct {
};
/**
+ * Flags associated with a module.
+ */
+#define AP_MODULE_FLAG_NONE (0)
+#define AP_MODULE_FLAG_ALWAYS_MERGE (1 << 0)
+
+/**
* Module structures. Just about everything is dispatched through
* these, directly or indirectly (through the command and handler
* tables).
@@ -407,6 +413,9 @@ struct module_struct {
* @param p the pool to use for all allocations
*/
void (*register_hooks) (apr_pool_t *p);
+
+/** A bitmask of AP_MODULE_FLAG_* */
+int flags;
};
/**
Index: server/config.c
===
--- server/config.c (revision 1809129)
+++ server/config.c (working copy)
@@ -323,7 +323,7 @@ static ap_conf_vector_t *create_server_config(apr_
}
static void merge_server_configs(apr_pool_t *p, ap_conf_vector_t *base,
- ap_conf_vector_t *virt)
+ server_rec *virt)
{
/* Can reuse the 'virt' vector for the spine of it, since we don't
* have to deal with the moral equivalent of .htaccess files here...
@@ -330,7 +330,7 @@ static void merge_server_configs(apr_pool_t *p, ap
*/
void **base_vector = (void **)base;
-void **virt_vector = (void **)virt;
+void **virt_vector = (void **)virt->module_config;
module *modp;
for (modp = ap_top_module; modp; modp = modp->next) {
@@ -337,10 +337,19 @@ static void merge_server_configs(apr_pool_t *p, ap
merger_func df = modp->merge_server_config;
int i = modp->module_index;
-if (!virt_vector[i])
-virt_vector[i] = base_vector[i];
-else if (df)
+if (!virt_vector[i]) {
+if (df && modp->create_server_config
+ && modp->flags & AP_MODULE_FLAG_ALWAYS_MERGE) {
+virt_vector[i] = (*modp->create_server_config)(p, virt);
+}
+else {
+virt_vector[i] = base_vector[i];
+df = NULL;
+}
+}
+if (df) {
virt_vector[i] = (*df)(p, base_vector[i], virt_vector[i]);
+}
}
}
@@ -2322,8 +2331,7 @@ AP_DECLARE(void) ap_fixup_virtual_hosts(apr_pool_t
dconf->log = &main_server->log;
for (virt = main_server->next; virt; virt = virt->next) {
-merge_server_configs(p, main_server->module_config,
- virt->module_config);
+merge_server_configs(p, main_server->module_config, virt);
virt->lookup_defaults =
ap_merge_per_dir_configs(p, main_server->lookup_defaults,
Index: modules/ssl/mod_ssl.c
===
--- modules/ssl/mod_ssl.c (revision 1809129)
+++ modules/ssl/mod_ssl.c (working copy)
@@ -712,5 +712,6 @@ module AP_MODULE_DECLARE_DATA ssl_module = {
ssl_config_server_create, /* create per-server config structures */
ssl_config_server_merge,/* merge per-server config structures */
ssl_config_cmds,/* table of configuration directives */
-ssl_register_hooks /* register hooks */
+ssl_register_hooks, /* register hooks */
+AP_MODULE_FLAG_ALWAYS_MERGE /* flags */
};
Re: SSLSrvConfigRec shared
On Thu, Sep 21, 2017 at 7:42 AM, Stefan Eissing wrote: > >> Am 21.09.2017 um 13:35 schrieb Eric Covener : >> >> On Thu, Sep 21, 2017 at 7:00 AM, Yann Ylavic wrote: >>> On Thu, Sep 21, 2017 at 11:48 AM, Stefan Eissing >>> wrote: > Am 21.09.2017 um 11:37 schrieb Yann Ylavic : > > If the module defines its own server_config_create() which allocates > one, each vhost will have its own, and the module's > server_config_merge() can do whatever needs to for the members of the > config (pointer copy, shallow/deep copy, ...). Yes, but only *iff* there is every a directive of that module used in a VirtualHost. >>> >>> OK, I see know, thanks. >>> >>> I'd call that a premature optimization though, even if it matured for >>> decades :) >>> Only the module knows what to do when merging, so I think the core >>> config should still call config_create() and config_merge() for those, >>> precisely because post_config() is always called. >>> Modules also know how to merge configs that do nothing (usually), with >>> all those _set members all over the place, so it should work (untested >>> ;) even if it may consume more (not that much I think) initial memory >>> for large confs. >> >> Maybe less intrusive for a module to make a copy of its config if it >> detects base/vh are the same and needs them to differ? > > That is why I "fixed" this for mod_ssl only. But mod_md has similar code now. > > Do we have some sort of bitfield of flags for each module? That would be great > as we could make an opt-in change that way. > IIUC it should be safe to extend module_struct with a minor bump to add 'int flags' to the bottom, but when you check the value you'd need to check the MMN first. In the module you'd then just have some flags or'ed together after register_hooks. (hopefully someone will check my work) -- Eric Covener cove...@gmail.com
configure option --enable-option-checking warns about things it does know (httpd-2.2.X)
Just thought I would mention option-checking in httpd-2.2.X is borked. Fortunately, it just warns :) A small subset of the warnings: configure: WARNING: unrecognized options: --with-z, --enable-layout, --with-apr, --with-apr-util, --with-mpm, --enable-ssl, --enable-proxy, --enable-mods-shared Regards, Michael p.s. I have not been following the maillist for quite a while - so if this is already known, please ignore.
Re: SSLSrvConfigRec shared
> Am 21.09.2017 um 13:35 schrieb Eric Covener : > > On Thu, Sep 21, 2017 at 7:00 AM, Yann Ylavic wrote: >> On Thu, Sep 21, 2017 at 11:48 AM, Stefan Eissing >> wrote: >>> Am 21.09.2017 um 11:37 schrieb Yann Ylavic : If the module defines its own server_config_create() which allocates one, each vhost will have its own, and the module's server_config_merge() can do whatever needs to for the members of the config (pointer copy, shallow/deep copy, ...). >>> >>> Yes, but only *iff* there is every a directive of that module used in >>> a VirtualHost. >> >> OK, I see know, thanks. >> >> I'd call that a premature optimization though, even if it matured for >> decades :) >> Only the module knows what to do when merging, so I think the core >> config should still call config_create() and config_merge() for those, >> precisely because post_config() is always called. >> Modules also know how to merge configs that do nothing (usually), with >> all those _set members all over the place, so it should work (untested >> ;) even if it may consume more (not that much I think) initial memory >> for large confs. > > Maybe less intrusive for a module to make a copy of its config if it > detects base/vh are the same and needs them to differ? That is why I "fixed" this for mod_ssl only. But mod_md has similar code now. Do we have some sort of bitfield of flags for each module? That would be great as we could make an opt-in change that way. -Stefan
Re: SSLSrvConfigRec shared
On Thu, Sep 21, 2017 at 7:00 AM, Yann Ylavic wrote: > On Thu, Sep 21, 2017 at 11:48 AM, Stefan Eissing > wrote: >> >>> Am 21.09.2017 um 11:37 schrieb Yann Ylavic : >>> >>> If the module defines its own server_config_create() which allocates >>> one, each vhost will have its own, and the module's >>> server_config_merge() can do whatever needs to for the members of the >>> config (pointer copy, shallow/deep copy, ...). >> >> Yes, but only *iff* there is every a directive of that module used in >> a VirtualHost. > > OK, I see know, thanks. > > I'd call that a premature optimization though, even if it matured for decades > :) > Only the module knows what to do when merging, so I think the core > config should still call config_create() and config_merge() for those, > precisely because post_config() is always called. > Modules also know how to merge configs that do nothing (usually), with > all those _set members all over the place, so it should work (untested > ;) even if it may consume more (not that much I think) initial memory > for large confs. Maybe less intrusive for a module to make a copy of its config if it detects base/vh are the same and needs them to differ?
Re: SSLSrvConfigRec shared
On Thu, Sep 21, 2017 at 11:48 AM, Stefan Eissing wrote: > >> Am 21.09.2017 um 11:37 schrieb Yann Ylavic : >> >> If the module defines its own server_config_create() which allocates >> one, each vhost will have its own, and the module's >> server_config_merge() can do whatever needs to for the members of the >> config (pointer copy, shallow/deep copy, ...). > > Yes, but only *iff* there is every a directive of that module used in > a VirtualHost. OK, I see know, thanks. I'd call that a premature optimization though, even if it matured for decades :) Only the module knows what to do when merging, so I think the core config should still call config_create() and config_merge() for those, precisely because post_config() is always called. Modules also know how to merge configs that do nothing (usually), with all those _set members all over the place, so it should work (untested ;) even if it may consume more (not that much I think) initial memory for large confs. > > The assumption here is that module configs at that point-in-time are > read-only. Which is not really true since several modules make changes > in the post_config phase that happens afterwards. Agreed. > > My fix for this in mod_ssl is part of > http://svn.apache.org/viewvc?view=revision&revision=1809037 > which calls ssl_config_server_uniq() once at the post_init phase > for each server. Or maybe do the right thing (IMHO) in core. >>> >>> Now, are you speaking of changing that for all modules? Add a flag to >>> "struct module" >>> or solve it in mod_ssl post_config? >> >> Of course not, create/merge process per module can already do that, up >> to the module to do what it needs (correctly). Finally, maybe :) It could be "suprising" for any module. Let's see what others think...
Re: SSLSrvConfigRec shared
> Am 21.09.2017 um 11:37 schrieb Yann Ylavic : > > Hi Stefan, > > On Wed, Sep 20, 2017 at 2:06 PM, Stefan Eissing > wrote: >> >>> Am 20.09.2017 um 12:33 schrieb Yann Ylavic : >>> >>> On Wed, Sep 20, 2017 at 12:09 PM, Stefan Eissing >>> wrote: Is there some better way? >>> >>> I would go with the usual/unconditional per server config (and hence >>> merging), trade simplicity vs a few memory space... >> >> Not sure I get your dift here. > > I think you lost me too :) > > Let's try to get in sync... Sure! :D >> >> server/config.c calls merge_server_configs() for each non-base server_rec >> and that one copies the config pointer from base if the vhost has none. >> if there is one, it merges. > > If the module defines its own server_config_create() which allocates > one, each vhost will have its own, and the module's > server_config_merge() can do whatever needs to for the members of the > config (pointer copy, shallow/deep copy, ...). Yes, but only *iff* there is every a directive of that module used in a VirtualHost. > I didn't re-looked at the mod_ssl config code lately, I remember some > special treatment for main server config (re process->pool), but it > seems to me that each vhost has its own config, and that the merge > process issues copies (shallow?). mod_ssl does nothing special here, just normal module stuff. The special thing happens in server/config.c: merge_server_configs() that goes over all modules and all virtual servers and either merges the modules config *or* just assigns the pointer from the base_server's one. The assumption here is that module configs at that point-in-time are read-only. Which is not really true since several modules make changes in the post_config phase that happens afterwards. My fix for this in mod_ssl is part of http://svn.apache.org/viewvc?view=revision&revision=1809037 which calls ssl_config_server_uniq() once at the post_init phase for each server. Cheers, Stefan > What I don't remember is some modifications of the server configs in > place in post_config, if that's the case it should indeed be preceded > by a deep copy sowehow... > >> >> Now, are you speaking of changing that for all modules? Add a flag to >> "struct module" >> or solve it in mod_ssl post_config? > > Of course not, create/merge process per module can already do that, up > to the module to do what it needs (correctly). > > > Regards, > Yann.
Re: SSLSrvConfigRec shared
Hi Stefan, On Wed, Sep 20, 2017 at 2:06 PM, Stefan Eissing wrote: > >> Am 20.09.2017 um 12:33 schrieb Yann Ylavic : >> >> On Wed, Sep 20, 2017 at 12:09 PM, Stefan Eissing >> wrote: >>> >>> Is there some better way? >> >> I would go with the usual/unconditional per server config (and hence >> merging), trade simplicity vs a few memory space... > > Not sure I get your dift here. I think you lost me too :) Let's try to get in sync... > > server/config.c calls merge_server_configs() for each non-base server_rec > and that one copies the config pointer from base if the vhost has none. > if there is one, it merges. If the module defines its own server_config_create() which allocates one, each vhost will have its own, and the module's server_config_merge() can do whatever needs to for the members of the config (pointer copy, shallow/deep copy, ...). I didn't re-looked at the mod_ssl config code lately, I remember some special treatment for main server config (re process->pool), but it seems to me that each vhost has its own config, and that the merge process issues copies (shallow?). What I don't remember is some modifications of the server configs in place in post_config, if that's the case it should indeed be preceded by a deep copy sowehow... > > Now, are you speaking of changing that for all modules? Add a flag to "struct > module" > or solve it in mod_ssl post_config? Of course not, create/merge process per module can already do that, up to the module to do what it needs (correctly). Regards, Yann.
Re: Understanding OptionsBleed
On Thu, Sep 21, 2017 at 10:54 AM, Yann Ylavic wrote: > On Wed, Sep 20, 2017 at 6:36 PM, William A Rowe Jr > wrote: >> >> Provided AllowOverride is None, and AllowOverrideList does not include >> "> this theory; >> https://httpd.apache.org/docs/2.4/mod/core.html#allowoverridelist > > I tested this and indeed the server is protected. > This is IMHO the rigth way to control the content of .htaccess files > from httpd.conf (i.e. a white-list). Also note that AllowOverride containing "AuthConfig" implicitely allows in .htaccess, I think we should change this since "Limit" can be specified explicitely in AllowOverride.
Re: Understanding OptionsBleed
On Wed, Sep 20, 2017 at 6:36 PM, William A Rowe Jr wrote: > > Provided AllowOverride is None, and AllowOverrideList does not include > " this theory; https://httpd.apache.org/docs/2.4/mod/core.html#allowoverridelist I tested this and indeed the server is protected. This is IMHO the rigth way to control the content of .htaccess files from httpd.conf (i.e. a white-list).
Re: Understanding OptionsBleed
Hi Bill, nice summary, totally agreed. Thanks! On Wed, Sep 20, 2017 at 6:36 PM, William A Rowe Jr wrote: > So as most people have correctly identified, this defect has existed > for an incredibly long time. > > But how it is triggered and avoided would help us to correctly study > unexpected behaviors. > > OPTIONS * - won't trigger the defect, .htaccess should not be examined. > > OPTIONS / - may trigger the defect, because the path is traversed and > one or more .htaccess files may be processed. > > In all versions, of the standard methods do not trigger the > defect. Only of any unregistered methods in an allowed > .htaccess file will trigger the defect. > > In 2.4.23 and prior including all 2.2/2.0, "HEAD" was not registered, > but would not be registered by number (which returned the GET method ID.) In 2.4.25 and 2.4.26, HEAD > was registered but the Allow header constructor duplicated GET -> HEAD > and HEAD --> HEAD resulting in four methods listed, fixed already in > 2.4.27. > > In order to avoid the defect with trusted .htaccess authors; > > In 2.2.31 and prior (all 2.0) or 2.4.23 and prior we can use an > otherwise no-op methods that are expected to occur in .htaccess - this avoids > registering them at request time. > > In 2.2.32+ / 2.4.25+ we can use RegisterHttpMethod in the global > httpd.conf for non-standard methods in .htaccess - this avoids > registering them at request time. > > It is not possible to avoid this defect with untrusted/malicious > .htaccess authors without disabling .htaccess files, patching or > upgrading to version 2.4.28. > > Provided AllowOverride is None, and AllowOverrideList does not include > " this theory; https://httpd.apache.org/docs/2.4/mod/core.html#allowoverridelist > > The httpd server cannot ever address every possible aspect of > malicious .htaccess authoring, and the project has reiterated this > message multiple times, leading to a vulnerability assessment of 'low' > severity in all cases that weren't disputed as not vulnerabilities > whatsoever; > > https://www.cvedetails.com/cve/CVE-2011-3607/ > https://www.cvedetails.com/cve/CVE-2011-4415/ > https://www.cvedetails.com/cve/CVE-2009-1195/ > https://www.cvedetails.com/cve/CVE-2004-2343/ > https://www.cvedetails.com/cve/CVE-2004-0747/
