You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dean Gaudet <dg...@arctic.org> on 1997/07/27 08:04:20 UTC
[PATCH] mod_access overhaul
This is an overhaul of mod_access.c's matching and syntax. I was originally
just going to implement the CIDR syntax like PR#762 wants. But I went a
bit further. From my CHANGES note:
- Now understands network/netmask syntax (i.e. 10.1.0.0/255.255.0.0)
and cidr syntax (i.e. 10.1.0.0/16).
- Critical path was sped up by pre-computing a few things at config time.
- The undocumented syntax "allow user-agents" was removed,
the replacement is "allow from env=foobar" combined with mod_browser.
- When used with hostnames it now forces a double-reverse lookup
no matter what the directory settings are. This double-reverse
doesn't affect any of the other routines that use the remote
hostname. In particular it's still passed to CGIs and the log
without the double-reverse check.
I expect a little resistance to the last point ... but my argument is
that it's a proactive attempt to avoid a CERT advisory. As of 1.2 we
no longer document MAXIMUM_DNS except in the FAQ... it used to be right
in Configuration in front of your face.
Note that I still maintain MAXIMUM_DNS, if it's defined.
I've given this a fairly good test. But I'd appreciate someone else
trying to break it. So I won't even say what I tested ;)
Dean
P.S. I expect a little resistance at the use of bitfields in conn_rec too!
Index: htdocs/manual/mod/mod_access.html
===================================================================
RCS file: /export/home/cvs/apache/htdocs/manual/mod/mod_access.html,v
retrieving revision 1.9
diff -u -r1.9 mod_access.html
--- mod_access.html 1997/07/06 17:19:14 1.9
+++ mod_access.html 1997/07/27 05:54:17
@@ -53,6 +53,12 @@
<dd>An IP address of a host allowed access
<dt>A partial IP address
<dd>The first 1 to 3 bytes of an IP address, for subnet restriction.
+<dt>A network/netmask pair
+<dd>A network a.b.c.d, and a netmask w.x.y.z. For more fine-grained subnet
+ restriction. (i.e. 10.1.0.0/255.255.0.0)
+<dt>A network/nnn CIDR specification
+<dd>Similar to the previous case, except the netmask consists of nnn
+ high-order 1 bits. (i.e. 10.1.0.0/16 is the same as 10.1.0.0/255.255.0.0)
</dl>
<P>
Example:
@@ -121,6 +127,12 @@
<dd>An IP address of a host denied access
<dt>A partial IP address
<dd>The first 1 to 3 bytes of an IP address, for subnet restriction.
+<dt>A network/netmask pair
+<dd>A network a.b.c.d, and a netmask w.x.y.z. For more fine-grained subnet
+ restriction. (i.e. 10.1.0.0/255.255.0.0)
+<dt>A network/nnn CIDR specification
+<dd>Similar to the previous case, except the netmask consists of nnn
+ high-order 1 bits. (i.e. 10.1.0.0/16 is the same as 10.1.0.0/255.255.0.0)
</dl>
<P>
Example:
Index: src/CHANGES
===================================================================
RCS file: /export/home/cvs/apache/src/CHANGES,v
retrieving revision 1.368
diff -u -r1.368 CHANGES
--- CHANGES 1997/07/27 02:38:03 1.368
+++ CHANGES 1997/07/27 05:54:25
@@ -1,5 +1,19 @@
Changes with Apache 1.3a2
+ *) mod_access overhaul:
+ - Now understands network/netmask syntax (i.e. 10.1.0.0/255.255.0.0)
+ and cidr syntax (i.e. 10.1.0.0/16).
+ - Critical path was sped up by pre-computing a few things at config
+ time.
+ - The undocumented syntax "allow user-agents" was removed,
+ the replacement is "allow from env=foobar" combined with mod_browser.
+ - When used with hostnames it now forces a double-reverse lookup
+ no matter what the directory settings are. This double-reverse
+ doesn't affect any of the other routines that use the remote
+ hostname. In particular it's still passed to CGIs and the log
+ without the double-reverse check.
+ [Dean Gaudet]
+
*) mod_cern_meta would attempt to find meta files for the directory itself
in some cases, but not in others. It now avoids it in all cases.
[Dean Gaudet]
Index: src/http_core.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_core.c,v
retrieving revision 1.100
diff -u -r1.100 http_core.c
--- http_core.c 1997/07/24 04:38:09 1.100
+++ http_core.c 1997/07/27 05:54:29
@@ -324,20 +324,45 @@
return conf->response_code_strings[error_index];
}
+
+/* Code from Harald Hanche-Olsen <ha...@imf.unit.no> */
+static void inline do_double_reverse (conn_rec *conn)
+{
+ struct hostent *hptr;
+ char **haddr;
+
+ if (conn->double_reverse) {
+ /* already done */
+ return;
+ }
+ hptr = gethostbyname(conn->remote_host);
+ if (hptr) {
+ for (haddr = hptr->h_addr_list; *haddr; haddr++) {
+ if (((struct in_addr *)(*haddr))->s_addr
+ == conn->remote_addr.sin_addr.s_addr) {
+ break;
+ }
+ }
+ }
+ if (!hptr || !*haddr) {
+ conn->double_reverse = -1;
+ } else {
+ conn->double_reverse = 1;
+ }
+}
+
API_EXPORT(const char *) get_remote_host(conn_rec *conn, void *dir_config, int type)
{
struct in_addr *iaddr;
struct hostent *hptr;
-#ifdef MAXIMUM_DNS
- char **haddr;
-#endif
core_dir_config *dir_conf = NULL;
/* If we haven't checked the host name, and we want to */
if (dir_config)
dir_conf = (core_dir_config *)get_module_config(dir_config, &core_module);
- if ((!dir_conf) || (type != REMOTE_NOLOOKUP && conn->remote_host == NULL && dir_conf->hostname_lookups))
+ if ((!dir_conf) || (type != REMOTE_NOLOOKUP && conn->remote_host == NULL
+ && (type == REMOTE_DOUBLE_REV || dir_conf->hostname_lookups)))
{
#ifdef STATUS
int old_stat = update_child_status(conn->child_num,
@@ -346,24 +371,15 @@
#endif /* STATUS */
iaddr = &(conn->remote_addr.sin_addr);
hptr = gethostbyaddr((char *)iaddr, sizeof(struct in_addr), AF_INET);
- if (hptr != NULL)
- {
+ if (hptr != NULL) {
conn->remote_host = pstrdup(conn->pool, (void *)hptr->h_name);
str_tolower (conn->remote_host);
#ifdef MAXIMUM_DNS
- /* Grrr. Check THAT name to make sure it's really the name of the addr. */
- /* Code from Harald Hanche-Olsen <ha...@imf.unit.no> */
-
- hptr = gethostbyname(conn->remote_host);
- if (hptr)
- {
- for(haddr=hptr->h_addr_list; *haddr; haddr++)
- if(((struct in_addr *)(*haddr))->s_addr == iaddr->s_addr)
- break;
- }
- if((!hptr) || (!(*haddr)))
+ do_double_reverse (conn);
+ if (conn->double_reverse != 1) {
conn->remote_host = NULL;
+ }
#endif
}
/* if failed, set it to the NULL string to indicate error */
@@ -372,6 +388,14 @@
(void)update_child_status(conn->child_num,old_stat,(request_rec*)NULL);
#endif /* STATUS */
}
+ if (type == REMOTE_DOUBLE_REV) {
+#ifndef MAXIMUM_DNS
+ do_double_reverse (conn);
+#endif
+ if (conn->double_reverse == -1) {
+ return NULL;
+ }
+ }
/*
* Return the desired information; either the remote DNS name, if found,
@@ -382,7 +406,7 @@
return conn->remote_host;
else
{
- if (type == REMOTE_HOST) return NULL;
+ if (type == REMOTE_HOST || type == REMOTE_DOUBLE_REV) return NULL;
else return conn->remote_ip;
}
}
Index: src/http_core.h
===================================================================
RCS file: /export/home/cvs/apache/src/http_core.h,v
retrieving revision 1.24
diff -u -r1.24 http_core.h
--- http_core.h 1997/07/16 00:41:21 1.24
+++ http_core.h 1997/07/27 05:54:29
@@ -77,6 +77,7 @@
#define REMOTE_HOST (0)
#define REMOTE_NAME (1)
#define REMOTE_NOLOOKUP (2)
+#define REMOTE_DOUBLE_REV (3)
#define SATISFY_ALL 0
#define SATISFY_ANY 1
Index: src/http_protocol.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_protocol.c,v
retrieving revision 1.145
diff -u -r1.145 http_protocol.c
--- http_protocol.c 1997/07/24 04:23:58 1.145
+++ http_protocol.c 1997/07/27 05:54:35
@@ -772,7 +772,7 @@
r->server = conn->server;
r->pool = make_sub_pool(conn->pool);
- conn->keptalive = conn->keepalive;
+ conn->keptalive = conn->keepalive == 1;
conn->keepalive = 0;
conn->user = NULL;
Index: src/httpd.h
===================================================================
RCS file: /export/home/cvs/apache/src/httpd.h,v
retrieving revision 1.133
diff -u -r1.133 httpd.h
--- httpd.h 1997/07/27 02:15:46 1.133
+++ httpd.h 1997/07/27 05:54:37
@@ -581,7 +581,6 @@
int child_num; /* The number of the child handling conn_rec */
BUFF *client; /* Connection to the guy */
- int aborted; /* Are we still talking? */
/* Who is the client? */
@@ -602,8 +601,12 @@
*/
char *auth_type; /* Ditto. */
- int keepalive; /* Are we using HTTP Keep-Alive? */
- int keptalive; /* Did we use HTTP Keep-Alive? */
+ int aborted : 1; /* Are we still talking? */
+ int keepalive : 2; /* Are we using HTTP Keep-Alive?
+ * -1 fatal error, 0 undecided, 1 yes */
+ int keptalive : 1; /* Did we use HTTP Keep-Alive? */
+ int double_reverse : 2; /* have we done double-reverse DNS?
+ * -1 yes/failure, 0 not yet, 1 yes/success */
int keepalives; /* How many times have we used it? */
};
Index: src/mod_access.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_access.c,v
retrieving revision 1.20
diff -u -r1.20 mod_access.c
--- mod_access.c 1997/07/27 01:43:21 1.20
+++ mod_access.c 1997/07/27 05:54:38
@@ -63,9 +63,24 @@
#include "http_log.h"
#include "http_request.h"
+enum allowdeny_type {
+ T_ENV,
+ T_ALL,
+ T_IP,
+ T_HOST,
+ T_FAIL
+};
+
typedef struct {
- char *from;
int limited;
+ union {
+ char *from;
+ struct {
+ unsigned long net;
+ unsigned long mask;
+ } ip;
+ } x;
+ enum allowdeny_type type;
} allowdeny;
/* things in the 'order' array */
@@ -111,17 +126,104 @@
return NULL;
}
+static int is_ip(const char *host)
+{
+ while ((*host == '.') || isdigit(*host))
+ host++;
+ return (*host == '\0');
+}
+
static const char *allow_cmd (cmd_parms *cmd, void *dv, char *from, char *where)
{
access_dir_conf *d = (access_dir_conf *)dv;
allowdeny *a;
+ char *s;
if (strcasecmp (from, "from"))
return "allow and deny must be followed by 'from'";
a = (allowdeny *)push_array (cmd->info ? d->allows : d->denys);
- a->from = pstrdup (cmd->pool, where);
+ a->x.from = where = pstrdup (cmd->pool, where);
a->limited = cmd->limited;
+
+ if (!strncmp (where, "env=", 4)) {
+ a->type = T_ENV;
+ a->x.from += 4;
+
+ } else if (!strcmp (where, "all")) {
+ a->type = T_ALL;
+
+ } else if ((s = strchr (where, '/'))) {
+ unsigned long mask;
+
+ a->type = T_IP;
+ /* trample on where, we won't be using it any more */
+ *s++ = '\0';
+
+ if (!is_ip (where)
+ || (a->x.ip.net = ap_inet_addr (where)) == INADDR_NONE) {
+ a->type = T_FAIL;
+ return "syntax error in network portion of network/netmask";
+ }
+
+ /* is_ip just tests if it matches [\d.]+ */
+ if (!is_ip (s)) {
+ a->type = T_FAIL;
+ return "syntax error in mask portion of network/netmask";
+ }
+ /* is it in /a.b.c.d form? */
+ if (strchr (s, '.')) {
+ mask = ap_inet_addr (s);
+ if (mask == INADDR_NONE) {
+ a->type = T_FAIL;
+ return "syntax error in mask portion of network/netmask";
+ }
+ } else {
+ /* assume it's in /nnn form */
+ mask = atoi (s);
+ if (mask > 32 || mask <= 0) {
+ a->type = T_FAIL;
+ return "invalid mask in network/netmask";
+ }
+ mask = 0xFFFFFFFFUL << (32 - mask);
+ mask = htonl (mask);
+ }
+ a->x.ip.mask = mask;
+
+ } else if (isdigit (*where) && is_ip (where)) {
+ /* legacy syntax for ip addrs: a.b.c. ==> a.b.c.0/24 for example */
+ int shift;
+ char *t;
+
+ a->type = T_IP;
+ /* parse components */
+ s = where;
+ a->x.ip.net = 0;
+ shift = 0;
+ while (*s) {
+ t = s;
+ if (!isdigit (*t)) {
+ a->type = T_FAIL;
+ return "invalid ip address";
+ }
+ while (isdigit (*t)) {
+ ++t;
+ }
+ if (*t == '.') {
+ *t++ = 0;
+ } else if (*t) {
+ a->type = T_FAIL;
+ return "invalid ip address";
+ }
+ a->x.ip.net |= atoi(s) << shift;
+ a->x.ip.mask |= 0xFFUL << shift;
+ shift += 8;
+ s = t;
+ }
+ } else {
+ a->type = T_HOST;
+ }
+
return NULL;
}
@@ -155,25 +257,6 @@
return 0;
}
-static int in_ip(char *domain, char *what) {
-
- /* Check a similar screw case to the one checked above ---
- * "allow from 204.26.2" shouldn't let in people from 204.26.23
- */
-
- int l = strlen(domain);
- if (strncmp(domain,what,l) != 0) return 0;
- if (domain[l - 1] == '.') return 1;
- return (what[l] == '\0' || what[l] == '.');
-}
-
-static int is_ip(const char *host)
-{
- while ((*host == '.') || isdigit(*host))
- host++;
- return (*host == '\0');
-}
-
static int find_allowdeny (request_rec *r, array_header *a, int method)
{
allowdeny *ap = (allowdeny *)a->elts;
@@ -186,39 +269,43 @@
if (!(mmask & ap[i].limited))
continue;
- if (!strncmp(ap[i].from,"env=",4) && table_get(r->subprocess_env,ap[i].from+4))
+ switch (ap[i].type) {
+ case T_ENV:
+ if (table_get (r->subprocess_env, ap[i].x.from)) {
+ return 1;
+ }
+ break;
+
+ case T_ALL:
return 1;
-
- if (ap[i].from && !strcmp(ap[i].from, "user-agents")) {
- char * this_agent = table_get(r->headers_in, "User-Agent");
- int j;
-
- if (!this_agent) return 0;
-
- for (j = i+1; j < a->nelts; ++j) {
- if (strstr(this_agent, ap[j].from)) return 1;
+
+ case T_IP:
+ if (ap[i].x.ip.net != INADDR_NONE
+ && (r->connection->remote_addr.sin_addr.s_addr
+ & ap[i].x.ip.mask) == ap[i].x.ip.net) {
+ return 1;
}
- return 0;
- }
+ break;
- if (!strcmp (ap[i].from, "all"))
- return 1;
+ case T_HOST:
+ if (!gothost) {
+ remotehost = get_remote_host(r->connection, r->per_dir_config,
+ REMOTE_DOUBLE_REV);
+
+ if ((remotehost == NULL) || is_ip(remotehost))
+ gothost = 1;
+ else
+ gothost = 2;
+ }
- if (!gothost) {
- remotehost = get_remote_host(r->connection, r->per_dir_config,
- REMOTE_HOST);
-
- if ((remotehost == NULL) || is_ip(remotehost))
- gothost = 1;
- else
- gothost = 2;
+ if ((gothost == 2) && in_domain(ap[i].x.from, remotehost))
+ return 1;
+ break;
+
+ case T_FAIL:
+ /* do nothing? */
+ break;
}
-
- if ((gothost == 2) && in_domain(ap[i].from, remotehost))
- return 1;
-
- if (in_ip (ap[i].from, r->connection->remote_ip))
- return 1;
}
return 0;
Re: [PATCH] mod_access overhaul
Posted by Dean Gaudet <dg...@arctic.org>.
On Sun, 27 Jul 1997, Marc Slemko wrote:
> But what about octal? <g>
Pfft!
> Does it work with 10.0.1.0/23? 10.0.254.0/22? (yes, those are
> pathological, but not as bad as non-contiguous subnets; hey, does it
> support those?)
It should support those just fine. It supports non-contiguous if you use
the /a.b.c.d netmask format. Like uh, 10.1.0.0/255.254.0.255 should you
be so deranged.
> But if it had to be looked up, then it will still be passed to things
> even if hostnamelookups are off, right?
Yep it does the normal remote_host thing, and then keeps track of whether
it has also done a double_reverse. It knows if the double_reverse failed
or succeeded (tri-state).
> Hmm. That could break some things. Picture a moron. Picture him
> writing a CGI script and not having a clue about anything, so
> using REMOTE_HOST and turning off DNS lookups and expecting
> a numeric IP. I'm not sure if I am woried about this or not.
I'm willing to accept this ... they've got REMOTE_ADDR to play with too.
> How about expanding Hostnamelookups to allow for maximum_dns?
Easy to do. Patch coming up.
Dean
Re: [PATCH] mod_access overhaul
Posted by Marc Slemko <ma...@worldgate.com>.
On Sat, 26 Jul 1997, Dean Gaudet wrote:
> This is an overhaul of mod_access.c's matching and syntax. I was originally
> just going to implement the CIDR syntax like PR#762 wants. But I went a
> bit further. From my CHANGES note:
>
> - Now understands network/netmask syntax (i.e. 10.1.0.0/255.255.0.0)
> and cidr syntax (i.e. 10.1.0.0/16).
But what about octal? <g>
Does it work with 10.0.1.0/23? 10.0.254.0/22? (yes, those are
pathological, but not as bad as non-contiguous subnets; hey, does it
support those?)
> - When used with hostnames it now forces a double-reverse lookup
> no matter what the directory settings are. This double-reverse
> doesn't affect any of the other routines that use the remote
> hostname. In particular it's still passed to CGIs and the log
> without the double-reverse check.
But if it had to be looked up, then it will still be passed to things
even if hostnamelookups are off, right?
Hmm. That could break some things. Picture a moron. Picture him
writing a CGI script and not having a clue about anything, so
using REMOTE_HOST and turning off DNS lookups and expecting
a numeric IP. I'm not sure if I am woried about this or not.
>
> I expect a little resistance to the last point ... but my argument is
> that it's a proactive attempt to avoid a CERT advisory. As of 1.2 we
> no longer document MAXIMUM_DNS except in the FAQ... it used to be right
> in Configuration in front of your face.
No resistance. I was going to do it myself when I got time.
>
> Note that I still maintain MAXIMUM_DNS, if it's defined.
>
How about expanding Hostnamelookups to allow for maximum_dns?
Re: [PATCH] mod_access overhaul
Posted by Dean Gaudet <dg...@arctic.org>.
On Mon, 28 Jul 1997, Paul Sutton wrote:
> Incidently I still find this non-obvious:
>
> BrowserMatch nice-robots banned=0
> deny from env=banned
>
> bans the nice-robots.
You would prefer:
deny env banned ?
I'm not sure which part you find non-obvious. Or you'd prefer it to test
the value of the env var? 'cause I usually just do things like this:
#deny mozilla but not MSIE
BrowserMatch Mozilla/2 banned
BrowserMatch MSIE !banned
(well I never ban based on user-agents... I just use the ! notation rather
than =1, =0.)
Dean
Re: [PATCH] mod_access overhaul
Posted by Paul Sutton <pa...@ukweb.com>.
On Sat, 26 Jul 1997, Dean Gaudet wrote:
> This is an overhaul of mod_access.c's matching and syntax. I was originally
> just going to implement the CIDR syntax like PR#762 wants. But I went a
> bit further. From my CHANGES note:
>
> - Now understands network/netmask syntax (i.e. 10.1.0.0/255.255.0.0)
> and cidr syntax (i.e. 10.1.0.0/16).
>
> - Critical path was sped up by pre-computing a few things at config time.
>
> - The undocumented syntax "allow user-agents" was removed,
> the replacement is "allow from env=foobar" combined with mod_browser.
>
> - When used with hostnames it now forces a double-reverse lookup
> no matter what the directory settings are. This double-reverse
> doesn't affect any of the other routines that use the remote
> hostname. In particular it's still passed to CGIs and the log
> without the double-reverse check.
Yeah, this is neat. It seems to work in some basic testing and looks
good. +1 for 1.3.
Incidently I still find this non-obvious:
BrowserMatch nice-robots banned=0
deny from env=banned
bans the nice-robots.
//pcs