- fix netmask matching for AF_INET, it was broken in many ways, problem

  was reported by nicm@ which spent a couple hours with me trying to
  understand what was causing the bug, and helping me write and test fix.

2 files changed, 62 insertions, 43 deletions

diff --git a/usr.sbin/smtpd/parse.y b/usr.sbin/smtpd/parse.y
index fe640223817..4793086720e 100644
--- a/usr.sbin/smtpd/parse.y
+++ b/usr.sbin/smtpd/parse.y
@@ -1,4 +1,4 @@
-/*	$OpenBSD: parse.y,v 1.50 2009/12/10 14:57:51 jacekm Exp $	*/
+/*	$OpenBSD: parse.y,v 1.51 2010/02/26 15:06:39 gilles Exp $	*/
 
 /*
  * Copyright (c) 2008 Gilles Chehade <gilles@openbsd.org>
@@ -454,13 +454,13 @@ stringel	: STRING			{
 				/* IP address ? */
 				if (inet_pton(AF_INET, $1, &ssin.sin_addr) == 1) {
 					ssin.sin_family = AF_INET;
-					me->me_key.med_addr.bits = 0;
+					me->me_key.med_addr.bits = 32;
 					memcpy(&me->me_key.med_addr.ss, &ssin, sizeof(ssin));
 					me->me_key.med_addr.ss.ss_len = sizeof(struct sockaddr_in);
 				}
 				else if (inet_pton(AF_INET6, $1, &ssin6.sin6_addr) == 1) {
 					ssin6.sin6_family = AF_INET6;
-					me->me_key.med_addr.bits = 0;
+					me->me_key.med_addr.bits = 128;
 					memcpy(&me->me_key.med_addr.ss, &ssin6, sizeof(ssin6));
 					me->me_key.med_addr.ss.ss_len = sizeof(struct sockaddr_in6);
 				}
@@ -538,13 +538,13 @@ mapref		: STRING			{
 				/* IP address ? */
 				if (inet_pton(AF_INET, $1, &ssin.sin_addr) == 1) {
 					ssin.sin_family = AF_INET;
-					me->me_key.med_addr.bits = 0;
+					me->me_key.med_addr.bits = 32;
 					memcpy(&me->me_key.med_addr.ss, &ssin, sizeof(ssin));
 					me->me_key.med_addr.ss.ss_len = sizeof(struct sockaddr_in);
 				}
 				else if (inet_pton(AF_INET6, $1, &ssin6.sin6_addr) == 1) {
 					ssin6.sin6_family = AF_INET6;
-					me->me_key.med_addr.bits = 0;
+					me->me_key.med_addr.bits = 128;
 					memcpy(&me->me_key.med_addr.ss, &ssin6, sizeof(ssin6));
 					me->me_key.med_addr.ss.ss_len = sizeof(struct sockaddr_in6);
 				}
@@ -886,7 +886,7 @@ from		: FROM mapref			{
 
 			if ((me = calloc(1, sizeof(*me))) == NULL)
 				fatal("out of memory");
-			me->me_key.med_addr.bits = 32;
+			me->me_key.med_addr.bits = 0;
 			me->me_key.med_addr.ss.ss_len = sizeof(struct sockaddr_in);
 			ssin = (struct sockaddr_in *)&me->me_key.med_addr.ss;
 			ssin->sin_family = AF_INET;
@@ -899,7 +899,7 @@ from		: FROM mapref			{
 
 			if ((me = calloc(1, sizeof(*me))) == NULL)
 				fatal("out of memory");
-			me->me_key.med_addr.bits = 128;
+			me->me_key.med_addr.bits = 0;
 			me->me_key.med_addr.ss.ss_len = sizeof(struct sockaddr_in6);
 			ssin6 = (struct sockaddr_in6 *)&me->me_key.med_addr.ss;
 			ssin6->sin6_family = AF_INET6;
@@ -1761,7 +1761,7 @@ set_localaddrs(void)
 
 			if ((me = calloc(1, sizeof(*me))) == NULL)
 				fatal("out of memory");
-			me->me_key.med_addr.bits = 0;
+			me->me_key.med_addr.bits = 32;
 			me->me_key.med_addr.ss = *(struct sockaddr_storage *)sain;
 			TAILQ_INSERT_TAIL(&m->m_contents, me, me_entry);
 
@@ -1774,7 +1774,7 @@ set_localaddrs(void)
 
 			if ((me = calloc(1, sizeof(*me))) == NULL)
 				fatal("out of memory");
-			me->me_key.med_addr.bits = 0;
+			me->me_key.med_addr.bits = 128;
 			me->me_key.med_addr.ss = *(struct sockaddr_storage *)sin6;
 			TAILQ_INSERT_TAIL(&m->m_contents, me, me_entry);
 
diff --git a/usr.sbin/smtpd/ruleset.c b/usr.sbin/smtpd/ruleset.c
index 47df11070cd..3c19ba917b7 100644
--- a/usr.sbin/smtpd/ruleset.c
+++ b/usr.sbin/smtpd/ruleset.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ruleset.c,v 1.9 2010/02/17 13:47:31 gilles Exp $ */
+/*	$OpenBSD: ruleset.c,v 1.10 2010/02/26 15:06:40 gilles Exp $ */
 
 /*
  * Copyright (c) 2009 Gilles Chehade <gilles@openbsd.org>
@@ -38,7 +38,8 @@
 struct rule    *ruleset_match(struct smtpd *, char *tag, struct path *, struct sockaddr_storage *);
 int		ruleset_check_source(struct map *, struct sockaddr_storage *);
 int		ruleset_match_mask(struct sockaddr_storage *, struct netaddr *);
-
+int		ruleset_inet4_match(struct sockaddr_in *, struct netaddr *);
+int		ruleset_inet6_match(struct sockaddr_in6 *, struct netaddr *);
 
 struct rule *
 ruleset_match(struct smtpd *env, char *tag, struct path *path, struct sockaddr_storage *ss)
@@ -132,39 +133,57 @@ ruleset_check_source(struct map *map, struct sockaddr_storage *ss)
 int
 ruleset_match_mask(struct sockaddr_storage *ss, struct netaddr *ssmask)
 {
-	if (ss->ss_family == AF_INET) {
-		struct sockaddr_in *ssin = (struct sockaddr_in *)ss;
-		struct sockaddr_in *ssinmask = (struct sockaddr_in *)&ssmask->ss;
-
-		if ((ssin->sin_addr.s_addr & ssinmask->sin_addr.s_addr) ==
-		    ssinmask->sin_addr.s_addr)
-			return (1);
-		return (0);
-	}
+	if (ss->ss_family == AF_INET)
+		return ruleset_inet4_match((struct sockaddr_in *)ss, ssmask);
 
-	if (ss->ss_family == AF_INET6) {
-		struct in6_addr	*in;
-		struct in6_addr	*inmask;
-		struct in6_addr	 mask;
-		int		 i;
-
-		bzero(&mask, sizeof(mask));
-		for (i = 0; i < (128 - ssmask->bits) / 8; i++)
-			mask.s6_addr[i] = 0xff;
-		i = ssmask->bits % 8;
-		if (i)
-			mask.s6_addr[ssmask->bits / 8] = 0xff00 >> i;
-
-		in = &((struct sockaddr_in6 *)ss)->sin6_addr;
-		inmask = &((struct sockaddr_in6 *)&ssmask->ss)->sin6_addr;
-
-		for (i = 0; i < 16; i++) {
-			if ((in->s6_addr[i] & mask.s6_addr[i]) !=
-			    inmask->s6_addr[i])
-				return (0);
-		}
-		return (1);
-	}
+	if (ss->ss_family == AF_INET6)
+		return ruleset_inet6_match((struct sockaddr_in6 *)ss, ssmask);
 
 	return (0);
 }
+
+int
+ruleset_inet4_match(struct sockaddr_in *ss, struct netaddr *ssmask)
+{
+	in_addr_t mask;
+	int i;
+
+	/* a.b.c.d/8 -> htonl(0xff000000) */
+	mask = 0;
+	for (i = 0; i < ssmask->bits; ++i)
+		mask = (mask >> 1) | 0x80000000;
+	mask = htonl(mask);
+
+	/* (addr & mask) == (net & mask) */
+ 	if ((ss->sin_addr.s_addr & mask) ==
+	    (((struct sockaddr_in *)ssmask)->sin_addr.s_addr & mask))
+		return 1;
+	
+	return 0;
+}
+
+int
+ruleset_inet6_match(struct sockaddr_in6 *ss, struct netaddr *ssmask)
+{
+	struct in6_addr	*in;
+	struct in6_addr	*inmask;
+	struct in6_addr	 mask;
+	int		 i;
+	
+	bzero(&mask, sizeof(mask));
+	for (i = 0; i < (128 - ssmask->bits) / 8; i++)
+		mask.s6_addr[i] = 0xff;
+	i = ssmask->bits % 8;
+	if (i)
+		mask.s6_addr[ssmask->bits / 8] = 0xff00 >> i;
+	
+	in = &ss->sin6_addr;
+	inmask = &((struct sockaddr_in6 *)&ssmask->ss)->sin6_addr;
+	
+	for (i = 0; i < 16; i++) {
+		if ((in->s6_addr[i] & mask.s6_addr[i]) !=
+		    inmask->s6_addr[i])
+			return (0);
+	}
+	return (1);
+}