httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dean Gaudet <dgau...@arctic.org>
Subject [PATCH] mod_access overhaul
Date Sun, 27 Jul 1997 06:04:20 GMT
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 <hanche@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 <hanche@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;


Mime
View raw message