You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ni...@apache.org on 2009/07/01 17:01:55 UTC
svn commit: r790205 - /httpd/httpd/trunk/modules/experimental/mod_noloris.c
Author: niq
Date: Wed Jul 1 15:01:55 2009
New Revision: 790205
URL: http://svn.apache.org/viewvc?rev=790205&view=rev
Log:
mod_noloris just moved from discussion to attracting its first patch
on dev@. That means it wants to be in svn. Adding to modules/experimental
pending anything more definite.
Added:
httpd/httpd/trunk/modules/experimental/mod_noloris.c
Added: httpd/httpd/trunk/modules/experimental/mod_noloris.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/experimental/mod_noloris.c?rev=790205&view=auto
==============================================================================
--- httpd/httpd/trunk/modules/experimental/mod_noloris.c (added)
+++ httpd/httpd/trunk/modules/experimental/mod_noloris.c Wed Jul 1 15:01:55 2009
@@ -0,0 +1,245 @@
+/* Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+/* The use of the scoreboard in this module is based on a similar
+ * but simpler module, mod_antiloris by Kees Monshouwer, from
+ * ftp://ftp.monshouwer.eu/pub/linux/mod_antiloris/
+ * Note the FIXME that affects both modules.
+ *
+ * The major difference is that mod_antiloris checks the scoreboard
+ * on every request. This implies a per-request overhead that grows
+ * with the scoreboard, and gets very expensive on a big server.
+ * On the other hand, this module (mod_noloris) may be slower to
+ * react to a DoS attack, and in the case of a very small server
+ * it might be too late.
+ *
+ * Author's untested instinct: mod_antiloris will suit servers with
+ * Prefork MPM and low traffic. A server with a threaded MPM
+ * (or possibly a big prefork server with lots of memory) should
+ * raise MaxClients and use mod_noloris.
+ */
+
+#include "httpd.h"
+#include "http_config.h"
+#include "http_connection.h"
+#include "http_log.h"
+#include "mpm_common.h"
+#include "ap_mpm.h"
+#include "apr_hash.h"
+
+module AP_MODULE_DECLARE_DATA noloris_module;
+module AP_MODULE_DECLARE_DATA core_module;
+
+static unsigned int default_max_connections;
+static apr_hash_t *trusted;
+static apr_interval_time_t recheck_time;
+static apr_shm_t *shm;
+static apr_size_t shm_size;
+static int server_limit;
+static int thread_limit;
+
+static int noloris_conn(conn_rec *conn)
+{
+ /*** FIXME
+ * This is evil: we're assuming info that's private to the scoreboard
+ * We need to do that because there's no API to update the scoreboard
+ * on a connection, only with a request (or NULL to say not processing
+ * any request). We need a version of ap_update_child_status that
+ * accepts a conn_rec.
+ */
+ struct { int child_num; int thread_num; } *sbh = conn->sbh;
+
+ char *shm_rec;
+ worker_score *ws;
+ if (shm == NULL) {
+ return DECLINED; /* we're disabled */
+ }
+
+ /* check the IP is not banned */
+ shm_rec = apr_shm_baseaddr_get(shm);
+ if (strstr(shm_rec, conn->remote_ip)) {
+ apr_socket_t *csd = ap_get_module_config(conn->conn_config, &core_module);
+ ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, conn,
+ "Dropping connection from banned IP %s", conn->remote_ip);
+ //ap_flush_conn(conn); /* just close it */
+ apr_socket_close(csd);
+
+ return DONE;
+ }
+
+ /* store this client IP for the monitor to pick up */
+ /* under traditional scoreboard, none of this happens until
+ * there's a request_rec. This is where we use the illegally-
+ * obtained private info from the scoreboard.
+ */
+
+ ws = &ap_scoreboard_image->servers[sbh->child_num][sbh->thread_num];
+ strcpy(ws->client, conn->remote_ip);
+
+ return DECLINED;
+}
+static int noloris_monitor(apr_pool_t *pool)
+{
+ static apr_hash_t *connections = NULL;
+ static apr_time_t last_check = 0;
+ static int *totals;
+
+ int i, j;
+ int *n;
+ int index = 0;
+ apr_hash_index_t *hi;
+ char *ip;
+ apr_time_t time_now;
+ char *shm_rec;
+ worker_score *ws;
+
+ /* do nothing if disabled */
+ if (shm == NULL) {
+ return 0;
+ }
+
+ /* skip check if it's not due yet */
+ time_now = apr_time_now();
+ if (time_now - last_check < recheck_time) {
+ return 0;
+ }
+ last_check = time_now;
+
+ /* alloc lots of stuff at start, so we don't leak memory per-call */
+ if (connections == NULL) {
+ connections = apr_hash_make(pool);
+ totals = apr_palloc(pool, server_limit*thread_limit);
+ ip = apr_palloc(pool, 18);
+ }
+
+ /* Get a per-client count of connections in READ state */
+ for (i = 0; i < server_limit; ++i) {
+ for (j = 0; j < thread_limit; ++j) {
+ ws = ap_get_scoreboard_worker(i, j);
+ if (ws->status == SERVER_BUSY_READ) {
+ n = apr_hash_get(connections, ws->client, APR_HASH_KEY_STRING);
+ if (n == NULL) {
+ n = totals + index++ ;
+ *n = 0;
+ }
+ ++*n;
+ apr_hash_set(connections, ws->client, APR_HASH_KEY_STRING, n);
+ }
+ }
+ }
+
+ /* reset shm before writing to it.
+ * We're only dealing with approx. counts, so we ignore the race condition
+ * with our prospective readers
+ */
+ shm_rec = apr_shm_baseaddr_get(shm);
+ memset(shm_rec, NULL, shm_size);
+
+ /* Now check the hash for clients with too many connections in READ state */
+ for (hi = apr_hash_first(NULL, connections); hi; hi = apr_hash_next(hi)) {
+ apr_hash_this(hi, (const void**) &ip, NULL, (void**)&n);
+ if (*n >= default_max_connections) {
+ /* if this isn't a trusted proxy, we mark it as bad */
+ if (!apr_hash_get(trusted, ip, APR_HASH_KEY_STRING)) {
+ ap_log_error(APLOG_MARK, APLOG_WARNING, 0, 0,
+ "noloris: banning %s with %d connections in READ state",
+ ip, *n);
+ strcpy(shm_rec++, " "); /* space == separator */
+ strcpy(shm_rec, ip);
+ shm_rec += strlen(ip);
+ }
+ }
+ }
+ apr_hash_clear(connections);
+ return 0;
+}
+static int noloris_post(apr_pool_t *pconf, apr_pool_t *ptmp, apr_pool_t *plog,
+ server_rec *s)
+{
+ apr_status_t rv;
+ int max_bans = thread_limit * server_limit / default_max_connections;
+ shm_size = 18 * max_bans;
+
+ rv = apr_shm_create(&shm, shm_size, NULL, pconf);
+ if (rv != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
+ "Failed to create shm segment; mod_noloris disabled");
+ apr_hash_clear(trusted);
+ shm = NULL;
+ }
+ return 0;
+}
+static int noloris_pre(apr_pool_t *pconf, apr_pool_t *ptmp, apr_pool_t *plog)
+{
+ ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);
+ ap_mpm_query(AP_MPMQ_HARD_LIMIT_DAEMONS, &server_limit);
+
+ /* set up default config stuff here */
+ trusted = apr_hash_make(pconf);
+ default_max_connections = 50;
+ recheck_time = apr_time_from_sec(10);
+ return 0;
+}
+static void noloris_hooks(apr_pool_t *p)
+{
+ ap_hook_process_connection(noloris_conn, NULL, NULL, APR_HOOK_FIRST);
+ ap_hook_pre_config(noloris_pre, NULL, NULL, APR_HOOK_MIDDLE);
+ ap_hook_post_config(noloris_post, NULL, NULL, APR_HOOK_MIDDLE);
+ ap_hook_monitor(noloris_monitor, NULL, NULL, APR_HOOK_MIDDLE);
+}
+static const char *noloris_trusted(cmd_parms *cmd, void *cfg, const char *val)
+{
+ const char* err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+ if (!err) {
+ apr_hash_set(trusted, val, APR_HASH_KEY_STRING, &noloris_module);
+ }
+ return err;
+}
+static const char *noloris_recheck(cmd_parms *cmd, void *cfg, const char *val)
+{
+ const char* err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+ if (!err) {
+ recheck_time = apr_time_from_sec(atoi(val));
+ }
+ return err;
+}
+static const char *noloris_max_conn(cmd_parms *cmd, void *cfg, const char *val)
+{
+ const char* err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+ if (!err) {
+ default_max_connections = atoi(val);
+ }
+ return err;
+}
+static const command_rec noloris_cmds[] = {
+ AP_INIT_ITERATE("TrustedProxy", noloris_trusted, NULL, RSRC_CONF,
+ "IP addresses from which to allow unlimited connections"),
+ AP_INIT_TAKE1("ClientRecheckTime", noloris_recheck, NULL, RSRC_CONF,
+ "Time interval for rechecking client connection tables"),
+ AP_INIT_TAKE1("MaxClientConnections", noloris_max_conn, NULL, RSRC_CONF,
+ "Max connections in READ state to permit from an untrusted client"),
+ {NULL}
+};
+module AP_MODULE_DECLARE_DATA noloris_module = {
+ STANDARD20_MODULE_STUFF,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ noloris_cmds,
+ noloris_hooks
+};
Re: svn commit: r790205 - /httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Jul 2, 2009, at 5:58 AM, Joe Orton wrote:
> On Wed, Jul 01, 2009 at 03:01:55PM -0000, niq@apache.org wrote:
>> Author: niq
>> Date: Wed Jul 1 15:01:55 2009
>> New Revision: 790205
>>
>> URL: http://svn.apache.org/viewvc?rev=790205&view=rev
>> Log:
>> mod_noloris just moved from discussion to attracting its first patch
>> on dev@. That means it wants to be in svn. Adding to modules/
>> experimental
>> pending anything more definite.
>
> 1) A *linear-time* search on a shm segment, using strstr.
> 2) ... for each new connection.
> 3) On a shm segment which will get modified in-place by another
> process
> 4) ... without locking
>
> Awesome! What could possibly go wrong? Ship it!
>
lol
> Regards, Joe
>
> p.s. iptables -A INPUT -p tcp --syn --dport 80 \
> -m connlimit --connlimit-above 50 -j REJECT
>
Re: svn commit: r790205 - /httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
> On Thu, Jul 02, 2009 at 01:37:22PM +0100, Nick Kew wrote:
>> Joe Orton wrote:
>>
>>> 1) A *linear-time* search on a shm segment, using strstr.
>>> 2) ... for each new connection.
>> With the expectation that the shm segment normally has strlen
>> of zero, and even under attack is just a few bytes.
>
> As far as I can tell, the worst case is when the size of the string in
> the shm segment approaches the maximum in a distributed DoS. The
> maximum will need to be:
>
> (MaxClients / MaxClientConnections) * 47 + 1
>
> (46 is the max length of an IPv6 address, not 18, IIRC, and you need
> +1's for both the space and the NUL terminator which strcpy will append)
>
> That could easily be tens or hundreds of kilobytes, depending on
> configuration. Presuming that strstr() on that could be non-trivial,
> the CPU cost of handling a DDoS attack becomes O(N^2) in an effort to
> mitigate a single-client-DoS. That sounds like very poor trade-off.
A fixed memcmp of the fixed strlen(match)+1 is sufficient, as you are
observing the trailing NULL, which should correspond to the individual
client IP strings' trailing NULLs.
strstr is certainly suboptimal.
Re: svn commit: r790205 -
/httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by Joe Orton <jo...@redhat.com>.
On Thu, Jul 02, 2009 at 01:37:22PM +0100, Nick Kew wrote:
> Joe Orton wrote:
>
>> 1) A *linear-time* search on a shm segment, using strstr.
> > 2) ... for each new connection.
>
> With the expectation that the shm segment normally has strlen
> of zero, and even under attack is just a few bytes.
As far as I can tell, the worst case is when the size of the string in
the shm segment approaches the maximum in a distributed DoS. The
maximum will need to be:
(MaxClients / MaxClientConnections) * 47 + 1
(46 is the max length of an IPv6 address, not 18, IIRC, and you need
+1's for both the space and the NUL terminator which strcpy will append)
That could easily be tens or hundreds of kilobytes, depending on
configuration. Presuming that strstr() on that could be non-trivial,
the CPU cost of handling a DDoS attack becomes O(N^2) in an effort to
mitigate a single-client-DoS. That sounds like very poor trade-off.
Regards, Joe
Re: svn commit: r790205 - /httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by Guenter Knauf <fu...@apache.org>.
Hi Nick,
in mod_noloris line 137:
ws = ap_get_scoreboard_worker(i, j);
looks wrong to me; in scoreboard.h I see:
AP_DECLARE(worker_score *) ap_get_scoreboard_worker(ap_sb_handle_t *sbh);
AP_DECLARE(worker_score *) ap_get_scoreboard_worker_from_indexes(int
child_num,
int
thread_num);
any chance you meant ap_get_scoreboard_worker_from_indexes() here?
Gün.
Re: svn commit: r790205 -
/httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by "Akins, Brian" <Br...@turner.com>.
On 7/2/09 8:37 AM, "Nick Kew" <ni...@webthing.com> wrote:
> Not everyone who's concerned right now about slowloris has
> iptables at their disposal.
Also, not everyone has access to the "real" IP very early in the connection
phase. Some load balancers add the IP as a header. Generally speaking,
most load balancers can handle many times more connections than the actual
webservers (even when there are many webservers). Some load balancers per
IP blocking schemes are either nonexistent or just don't work. Perhaps this
is beyond the scope of this discussion, but it is an unfortunate reality for
some folks.
--
Brian Akins
Re: svn commit: r790205 - /httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by Nick Kew <ni...@webthing.com>.
Joe Orton wrote:
> 1) A *linear-time* search on a shm segment, using strstr.
> 2) ... for each new connection.
With the expectation that the shm segment normally has strlen
of zero, and even under attack is just a few bytes.
> 3) On a shm segment which will get modified in-place by another process
> 4) ... without locking
with a comment about the race condition. When the worst outcome is
that a connection is accepted from a should-be-banned client ...
> p.s. iptables -A INPUT -p tcp --syn --dport 80 \
> -m connlimit --connlimit-above 50 -j REJECT
Not everyone who's concerned right now about slowloris has
iptables at their disposal.
--
Nick Kew
Re: svn commit: r790205 -
/httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Jul 01, 2009 at 03:01:55PM -0000, niq@apache.org wrote:
> Author: niq
> Date: Wed Jul 1 15:01:55 2009
> New Revision: 790205
>
> URL: http://svn.apache.org/viewvc?rev=790205&view=rev
> Log:
> mod_noloris just moved from discussion to attracting its first patch
> on dev@. That means it wants to be in svn. Adding to modules/experimental
> pending anything more definite.
1) A *linear-time* search on a shm segment, using strstr.
2) ... for each new connection.
3) On a shm segment which will get modified in-place by another process
4) ... without locking
Awesome! What could possibly go wrong? Ship it!
Regards, Joe
p.s. iptables -A INPUT -p tcp --syn --dport 80 \
-m connlimit --connlimit-above 50 -j REJECT
Re: svn commit: r790205 - /httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by Nick Kew <ni...@webthing.com>.
Ruediger Pluem wrote:
>> + * The major difference is that mod_antiloris checks the scoreboard
>> + * on every request. This implies a per-request overhead that grows
>> + * with the scoreboard, and gets very expensive on a big server.
>> + * On the other hand, this module (mod_noloris) may be slower to
>> + * react to a DoS attack, and in the case of a very small server
>> + * it might be too late.
>
> I am not sure if doing this for each connection (not each *request*)
> is really that much of a performace hit. I used a modified version of
> mod_limitipcon and do just that. So far I haven't noticed any performance
> issues with this approach. But maybe with a maximum of 1040 clients
> I am just having a small server :-).
I'm also thinking of simpler versions that set per-process limits
(no use of scoreboard/shm). Obviously for servers with high numbers
of threads-per-server. As and when round tuits permit.
>> + apr_socket_close(csd);
>
> This looks like an ugly thing to do for an module. Why not running the whole
> thing in the pre_connection hook and return something different then OK or DONE.
> This lets the core perform this dirty work in the predefined way.
I did have a good reason for process_connection over pre_connection,
but I can't bring it to mind right now.
>> + /* store this client IP for the monitor to pick up */
>> + /* under traditional scoreboard, none of this happens until
>> + * there's a request_rec. This is where we use the illegally-
>> + * obtained private info from the scoreboard.
>> + */
>> +
>> + ws = &ap_scoreboard_image->servers[sbh->child_num][sbh->thread_num];
>
> Shouldn't we use ap_get_scoreboard_worker here instead?
Probably, but it does nothing about the badness of how we got
child_num and thread_num.
>> + ip = apr_palloc(pool, 18);
>
> Harcoded values are IMHO ugly. Why 18 at all?
Erm .. 'cos I can't count. Needs to hold the max size of
an (IPv6) address string.
>> + memset(shm_rec, NULL, shm_size);
>
> Why NULL and not 0?
Erm ... 'cos we have a heatwave, and I'm in an office in an
attic apartment with a south-facing window and low ceiling.
> Storing the configuration in global variables looks kind of ugly.
> But I get the point that it is hard to get them in the monitor hook.
> But at least a struct for them and a global pointer to the struct would
> make them look a little nicer :-).
Feel free to change it. I don't really have any strong preference
there, and with GLOBAL_ONLY there's no context or hierarchy to
worry about.
Since it's now in svn, it's open to hack on
Re: svn commit: r790205 - /httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ruediger Pluem wrote:
>
> I am not sure if doing this for each connection (not each *request*)
> is really that much of a performace hit. I used a modified version of
> mod_limitipcon and do just that. So far I haven't noticed any performance
> issues with this approach. But maybe with a maximum of 1040 clients
> I am just having a small server :-).
The problem is which pain do you want; single-connection contention over
a lock to a IP resource counter which may bottleneck, or multiple thread
or cpu examination of the scoreboard. Presuming the score never swaps,
and is frequently queried, I'm speculating that this approach is more
robust. As far as niq's "already too late" comment, if N+x requests will
sink the server, where x is the number of CPU's which may be constructing
new scoreboard slots, then N was already set too high.
Re: svn commit: r790205 - /httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by Ruediger Pluem <rp...@apache.org>.
tOn 01.07.2009 17:01, niq@apache.org wrote:
> Author: niq
> Date: Wed Jul 1 15:01:55 2009
> New Revision: 790205
>
> URL: http://svn.apache.org/viewvc?rev=790205&view=rev
> Log:
> mod_noloris just moved from discussion to attracting its first patch
> on dev@. That means it wants to be in svn. Adding to modules/experimental
> pending anything more definite.
>
> Added:
> httpd/httpd/trunk/modules/experimental/mod_noloris.c
>
> Added: httpd/httpd/trunk/modules/experimental/mod_noloris.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/experimental/mod_noloris.c?rev=790205&view=auto
> ==============================================================================
> --- httpd/httpd/trunk/modules/experimental/mod_noloris.c (added)
> +++ httpd/httpd/trunk/modules/experimental/mod_noloris.c Wed Jul 1 15:01:55 2009
> @@ -0,0 +1,245 @@
> +/* Licensed to the Apache Software Foundation (ASF) under one or more
> + * contributor license agreements. See the NOTICE file distributed with
> + * this work for additional information regarding copyright ownership.
> + * The ASF licenses this file to You under the Apache License, Version 2.0
> + * (the "License"); you may not use this file except in compliance with
> + * the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +
> +/* The use of the scoreboard in this module is based on a similar
> + * but simpler module, mod_antiloris by Kees Monshouwer, from
> + * ftp://ftp.monshouwer.eu/pub/linux/mod_antiloris/
> + * Note the FIXME that affects both modules.
> + *
> + * The major difference is that mod_antiloris checks the scoreboard
> + * on every request. This implies a per-request overhead that grows
> + * with the scoreboard, and gets very expensive on a big server.
> + * On the other hand, this module (mod_noloris) may be slower to
> + * react to a DoS attack, and in the case of a very small server
> + * it might be too late.
I am not sure if doing this for each connection (not each *request*)
is really that much of a performace hit. I used a modified version of
mod_limitipcon and do just that. So far I haven't noticed any performance
issues with this approach. But maybe with a maximum of 1040 clients
I am just having a small server :-).
> + *
> + * Author's untested instinct: mod_antiloris will suit servers with
> + * Prefork MPM and low traffic. A server with a threaded MPM
> + * (or possibly a big prefork server with lots of memory) should
> + * raise MaxClients and use mod_noloris.
> + */
> +
> +#include "httpd.h"
> +#include "http_config.h"
> +#include "http_connection.h"
> +#include "http_log.h"
> +#include "mpm_common.h"
> +#include "ap_mpm.h"
> +#include "apr_hash.h"
> +
> +module AP_MODULE_DECLARE_DATA noloris_module;
> +module AP_MODULE_DECLARE_DATA core_module;
> +
> +static unsigned int default_max_connections;
> +static apr_hash_t *trusted;
> +static apr_interval_time_t recheck_time;
> +static apr_shm_t *shm;
> +static apr_size_t shm_size;
> +static int server_limit;
> +static int thread_limit;
> +
> +static int noloris_conn(conn_rec *conn)
> +{
> + /*** FIXME
> + * This is evil: we're assuming info that's private to the scoreboard
> + * We need to do that because there's no API to update the scoreboard
> + * on a connection, only with a request (or NULL to say not processing
> + * any request). We need a version of ap_update_child_status that
> + * accepts a conn_rec.
> + */
> + struct { int child_num; int thread_num; } *sbh = conn->sbh;
> +
> + char *shm_rec;
> + worker_score *ws;
> + if (shm == NULL) {
> + return DECLINED; /* we're disabled */
> + }
> +
> + /* check the IP is not banned */
> + shm_rec = apr_shm_baseaddr_get(shm);
> + if (strstr(shm_rec, conn->remote_ip)) {
> + apr_socket_t *csd = ap_get_module_config(conn->conn_config, &core_module);
> + ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, conn,
> + "Dropping connection from banned IP %s", conn->remote_ip);
> + //ap_flush_conn(conn); /* just close it */
> + apr_socket_close(csd);
This looks like an ugly thing to do for an module. Why not running the whole
thing in the pre_connection hook and return something different then OK or DONE.
This lets the core perform this dirty work in the predefined way.
> +
> + return DONE;
> + }
> +
> + /* store this client IP for the monitor to pick up */
> + /* under traditional scoreboard, none of this happens until
> + * there's a request_rec. This is where we use the illegally-
> + * obtained private info from the scoreboard.
> + */
> +
> + ws = &ap_scoreboard_image->servers[sbh->child_num][sbh->thread_num];
Shouldn't we use ap_get_scoreboard_worker here instead?
> + strcpy(ws->client, conn->remote_ip);
> +
> + return DECLINED;
> +}
> +static int noloris_monitor(apr_pool_t *pool)
> +{
> + static apr_hash_t *connections = NULL;
> + static apr_time_t last_check = 0;
> + static int *totals;
> +
> + int i, j;
> + int *n;
> + int index = 0;
> + apr_hash_index_t *hi;
> + char *ip;
> + apr_time_t time_now;
> + char *shm_rec;
> + worker_score *ws;
> +
> + /* do nothing if disabled */
> + if (shm == NULL) {
> + return 0;
> + }
> +
> + /* skip check if it's not due yet */
> + time_now = apr_time_now();
> + if (time_now - last_check < recheck_time) {
> + return 0;
> + }
> + last_check = time_now;
> +
> + /* alloc lots of stuff at start, so we don't leak memory per-call */
> + if (connections == NULL) {
> + connections = apr_hash_make(pool);
> + totals = apr_palloc(pool, server_limit*thread_limit);
> + ip = apr_palloc(pool, 18);
Harcoded values are IMHO ugly. Why 18 at all?
> + }
> +
> + /* Get a per-client count of connections in READ state */
> + for (i = 0; i < server_limit; ++i) {
> + for (j = 0; j < thread_limit; ++j) {
> + ws = ap_get_scoreboard_worker(i, j);
> + if (ws->status == SERVER_BUSY_READ) {
> + n = apr_hash_get(connections, ws->client, APR_HASH_KEY_STRING);
> + if (n == NULL) {
> + n = totals + index++ ;
> + *n = 0;
> + }
> + ++*n;
> + apr_hash_set(connections, ws->client, APR_HASH_KEY_STRING, n);
> + }
> + }
> + }
> +
> + /* reset shm before writing to it.
> + * We're only dealing with approx. counts, so we ignore the race condition
> + * with our prospective readers
> + */
> + shm_rec = apr_shm_baseaddr_get(shm);
> + memset(shm_rec, NULL, shm_size);
Why NULL and not 0?
> +
> + /* Now check the hash for clients with too many connections in READ state */
> + for (hi = apr_hash_first(NULL, connections); hi; hi = apr_hash_next(hi)) {
> + apr_hash_this(hi, (const void**) &ip, NULL, (void**)&n);
> + if (*n >= default_max_connections) {
> + /* if this isn't a trusted proxy, we mark it as bad */
> + if (!apr_hash_get(trusted, ip, APR_HASH_KEY_STRING)) {
> + ap_log_error(APLOG_MARK, APLOG_WARNING, 0, 0,
> + "noloris: banning %s with %d connections in READ state",
> + ip, *n);
> + strcpy(shm_rec++, " "); /* space == separator */
> + strcpy(shm_rec, ip);
> + shm_rec += strlen(ip);
> + }
> + }
> + }
> + apr_hash_clear(connections);
> + return 0;
> +}
> +static int noloris_post(apr_pool_t *pconf, apr_pool_t *ptmp, apr_pool_t *plog,
> + server_rec *s)
> +{
> + apr_status_t rv;
> + int max_bans = thread_limit * server_limit / default_max_connections;
> + shm_size = 18 * max_bans;
Hardcoded values always look ugly. Why 18 at all?
> +
> + rv = apr_shm_create(&shm, shm_size, NULL, pconf);
> + if (rv != APR_SUCCESS) {
> + ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
> + "Failed to create shm segment; mod_noloris disabled");
> + apr_hash_clear(trusted);
> + shm = NULL;
> + }
> + return 0;
> +}
> +static int noloris_pre(apr_pool_t *pconf, apr_pool_t *ptmp, apr_pool_t *plog)
> +{
> + ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);
> + ap_mpm_query(AP_MPMQ_HARD_LIMIT_DAEMONS, &server_limit);
> +
> + /* set up default config stuff here */
> + trusted = apr_hash_make(pconf);
> + default_max_connections = 50;
> + recheck_time = apr_time_from_sec(10);
> + return 0;
> +}
> +static void noloris_hooks(apr_pool_t *p)
> +{
> + ap_hook_process_connection(noloris_conn, NULL, NULL, APR_HOOK_FIRST);
> + ap_hook_pre_config(noloris_pre, NULL, NULL, APR_HOOK_MIDDLE);
> + ap_hook_post_config(noloris_post, NULL, NULL, APR_HOOK_MIDDLE);
> + ap_hook_monitor(noloris_monitor, NULL, NULL, APR_HOOK_MIDDLE);
> +}
> +static const char *noloris_trusted(cmd_parms *cmd, void *cfg, const char *val)
> +{
> + const char* err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
> + if (!err) {
> + apr_hash_set(trusted, val, APR_HASH_KEY_STRING, &noloris_module);
> + }
> + return err;
> +}
> +static const char *noloris_recheck(cmd_parms *cmd, void *cfg, const char *val)
> +{
> + const char* err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
> + if (!err) {
> + recheck_time = apr_time_from_sec(atoi(val));
> + }
> + return err;
> +}
> +static const char *noloris_max_conn(cmd_parms *cmd, void *cfg, const char *val)
> +{
> + const char* err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
> + if (!err) {
> + default_max_connections = atoi(val);
> + }
> + return err;
> +}
Storing the configuration in global variables looks kind of ugly.
But I get the point that it is hard to get them in the monitor hook.
But at least a struct for them and a global pointer to the struct would
make them look a little nicer :-).
> +static const command_rec noloris_cmds[] = {
> + AP_INIT_ITERATE("TrustedProxy", noloris_trusted, NULL, RSRC_CONF,
> + "IP addresses from which to allow unlimited connections"),
> + AP_INIT_TAKE1("ClientRecheckTime", noloris_recheck, NULL, RSRC_CONF,
> + "Time interval for rechecking client connection tables"),
> + AP_INIT_TAKE1("MaxClientConnections", noloris_max_conn, NULL, RSRC_CONF,
> + "Max connections in READ state to permit from an untrusted client"),
> + {NULL}
> +};
> +module AP_MODULE_DECLARE_DATA noloris_module = {
> + STANDARD20_MODULE_STUFF,
> + NULL,
> + NULL,
> + NULL,
> + NULL,
> + noloris_cmds,
> + noloris_hooks
> +};
>
Regards
Rüdiger
Re: svn commit: r790205 - /httpd/httpd/trunk/modules/experimental/mod_noloris.c
Posted by Paul Querna <pa...@querna.org>.
On Wed, Jul 1, 2009 at 8:01 AM, <ni...@apache.org> wrote:
> Author: niq
> Date: Wed Jul 1 15:01:55 2009
> New Revision: 790205
>
> URL: http://svn.apache.org/viewvc?rev=790205&view=rev
> Log:
> mod_noloris just moved from discussion to attracting its first patch
> on dev@. That means it wants to be in svn. Adding to modules/experimental
> pending anything more definite.
>
I would also suggest renaming the module.
Name it something that makes sense, 5 years from now, no one will know
what 'noloris' was supposed to mean.
Thanks
Paul