Explicitly compare return value of cmpsaddr() against a return value

define to make it more obvious what is the intended action. One more
return value is also added, to fix comparison of security policy
descriptors. Namely, getsp() should not allow wildcard matching (as the
comment says, it does exact matching) - otherwise we get problems when
kernel has generic policy with no ports, and a second similar policy with
ports.
This commit is contained in:
tteras 2011-03-14 17:18:12 +00:00
parent 3bc8d7931c
commit 4e499ee605
11 changed files with 72 additions and 69 deletions

View File

@ -1,4 +1,4 @@
/* $NetBSD: grabmyaddr.c,v 1.27 2010/12/03 09:46:24 tteras Exp $ */
/* $NetBSD: grabmyaddr.c,v 1.28 2011/03/14 17:18:12 tteras Exp $ */
/*
* Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
* Copyright (C) 2008 Timo Teras <timo.teras@iki.fi>.
@ -100,7 +100,7 @@ myaddr_configured(addr)
return TRUE;
LIST_FOREACH(cfg, &configured, chain) {
if (cmpsaddr(addr, (struct sockaddr *) &cfg->addr) == 0)
if (cmpsaddr(addr, (struct sockaddr *) &cfg->addr) <= CMPSADDR_WILDPORT_MATCH)
return TRUE;
}
@ -116,7 +116,7 @@ myaddr_open(addr, udp_encap)
/* Already open? */
LIST_FOREACH(my, &opened, chain) {
if (cmpsaddr(addr, (struct sockaddr *) &my->addr) == 0)
if (cmpsaddr(addr, (struct sockaddr *) &my->addr) <= CMPSADDR_WILDPORT_MATCH)
return TRUE;
}
@ -156,7 +156,7 @@ myaddr_open_all_configured(addr)
LIST_FOREACH(cfg, &configured, chain) {
if (addr != NULL &&
cmpsaddr(addr, (struct sockaddr *) &cfg->addr) != 0)
cmpsaddr(addr, (struct sockaddr *) &cfg->addr) > CMPSADDR_WILDPORT_MATCH)
continue;
if (!myaddr_open((struct sockaddr *) &cfg->addr, cfg->udp_encap))
return FALSE;
@ -262,7 +262,7 @@ myaddr_getfd(addr)
struct myaddr *my;
LIST_FOREACH(my, &opened, chain) {
if (cmpsaddr((struct sockaddr *) &my->addr, addr) == 0)
if (cmpsaddr((struct sockaddr *) &my->addr, addr) <= CMPSADDR_WILDPORT_MATCH)
return my->fd;
}
@ -276,7 +276,7 @@ myaddr_getsport(addr)
struct myaddr *my;
LIST_FOREACH(my, &opened, chain) {
if (cmpsaddr((struct sockaddr *) &my->addr, addr) == 0)
if (cmpsaddr((struct sockaddr *) &my->addr, addr) <= CMPSADDR_WILDPORT_MATCH)
return extract_port((struct sockaddr *) &my->addr);
}

View File

@ -1,4 +1,4 @@
/* $NetBSD: handler.c,v 1.38 2011/03/14 14:54:07 vanhu Exp $ */
/* $NetBSD: handler.c,v 1.39 2011/03/14 17:18:12 tteras Exp $ */
/* Id: handler.c,v 1.28 2006/05/26 12:17:29 manubsd Exp */
@ -120,11 +120,11 @@ enumph1(sel, enum_func, enum_arg)
LIST_FOREACH(p, &ph1tree, chain) {
if (sel != NULL) {
if (sel->local != NULL &&
cmpsaddr(sel->local, p->local) != 0)
cmpsaddr(sel->local, p->local) > CMPSADDR_WILDPORT_MATCH)
continue;
if (sel->remote != NULL &&
cmpsaddr(sel->remote, p->remote) != 0)
cmpsaddr(sel->remote, p->remote) > CMPSADDR_WILDPORT_MATCH)
continue;
}
@ -300,8 +300,8 @@ void migrate_dying_ph12(iph1)
if (p->status < PHASE1ST_DYING)
continue;
if (cmpsaddr(iph1->local, p->local) == 0
&& cmpsaddr(iph1->remote, p->remote) == 0)
if (cmpsaddr(iph1->local, p->local) == CMPSADDR_MATCH
&& cmpsaddr(iph1->remote, p->remote) == CMPSADDR_MATCH)
migrate_ph12(p, iph1);
}
}
@ -547,11 +547,11 @@ enumph2(sel, enum_func, enum_arg)
continue;
if (sel->src != NULL &&
cmpsaddr(sel->src, p->src) != 0)
cmpsaddr(sel->src, p->src) != CMPSADDR_MATCH)
continue;
if (sel->dst != NULL &&
cmpsaddr(sel->dst, p->dst) != 0)
cmpsaddr(sel->dst, p->dst) != CMPSADDR_MATCH)
continue;
}
@ -615,8 +615,8 @@ getph2byid(src, dst, spid)
LIST_FOREACH(p, &ph2tree, chain) {
if (spid == p->spid &&
cmpsaddr(src, p->src) == 0 &&
cmpsaddr(dst, p->dst) == 0){
cmpsaddr(src, p->src) <= CMPSADDR_WILDPORT_MATCH &&
cmpsaddr(dst, p->dst) <= CMPSADDR_WILDPORT_MATCH){
/* Sanity check to detect zombie handlers
* XXX Sould be done "somewhere" more interesting,
* because we have lots of getph2byxxxx(), but this one
@ -643,8 +643,8 @@ getph2bysaddr(src, dst)
struct ph2handle *p;
LIST_FOREACH(p, &ph2tree, chain) {
if (cmpsaddr(src, p->src) == 0 &&
cmpsaddr(dst, p->dst) == 0)
if (cmpsaddr(src, p->src) <= CMPSADDR_WILDPORT_MATCH &&
cmpsaddr(dst, p->dst) <= CMPSADDR_WILDPORT_MATCH)
return p;
}
@ -947,7 +947,7 @@ getcontacted(remote)
struct contacted *p;
LIST_FOREACH(p, &ctdtree, chain) {
if (cmpsaddr(remote, p->remote) == 0)
if (cmpsaddr(remote, p->remote) <= CMPSADDR_WILDPORT_MATCH)
return p;
}
@ -988,7 +988,7 @@ remcontacted(remote)
struct contacted *p;
LIST_FOREACH(p, &ctdtree, chain) {
if (cmpsaddr(remote, p->remote) == 0) {
if (cmpsaddr(remote, p->remote) <= CMPSADDR_WILDPORT_MATCH) {
LIST_REMOVE(p, chain);
racoon_free(p->remote);
racoon_free(p);
@ -1042,7 +1042,7 @@ check_recvdpkt(remote, local, rbuf)
/*
* the packet was processed before, but the remote address mismatches.
*/
if (cmpsaddr(remote, r->remote) != 0)
if (cmpsaddr(remote, r->remote) != CMPSADDR_MATCH)
return 2;
/*

View File

@ -1,4 +1,4 @@
/* $NetBSD: isakmp.c,v 1.69 2011/03/11 14:30:07 vanhu Exp $ */
/* $NetBSD: isakmp.c,v 1.70 2011/03/14 17:18:12 tteras Exp $ */
/* Id: isakmp.c,v 1.74 2006/05/07 21:32:59 manubsd Exp */
@ -468,8 +468,8 @@ isakmp_main(msg, remote, local)
/* Floating ports for NAT-T */
if (NATT_AVAILABLE(iph1) &&
! (iph1->natt_flags & NAT_PORTS_CHANGED) &&
((cmpsaddr(iph1->remote, remote) != 0) ||
(cmpsaddr(iph1->local, local) != 0)))
((cmpsaddr(iph1->remote, remote) != CMPSADDR_MATCH) ||
(cmpsaddr(iph1->local, local) != CMPSADDR_MATCH)))
{
/* prevent memory leak */
racoon_free(iph1->remote);
@ -510,7 +510,7 @@ isakmp_main(msg, remote, local)
#endif
/* must be same addresses in one stream of a phase at least. */
if (cmpsaddr(iph1->remote, remote) != 0) {
if (cmpsaddr(iph1->remote, remote) != CMPSADDR_MATCH) {
char *saddr_db, *saddr_act;
saddr_db = racoon_strdup(saddr2str(iph1->remote));
@ -636,7 +636,7 @@ isakmp_main(msg, remote, local)
"exchange received.\n");
return -1;
}
if (cmpsaddr(iph1->remote, remote) != 0) {
if (cmpsaddr(iph1->remote, remote) != CMPSADDR_MATCH) {
plog(LLV_WARNING, LOCATION, remote,
"remote address mismatched. "
"db=%s\n",
@ -3325,10 +3325,10 @@ purge_remote(iph1)
* Select only SAs where src == local and dst == remote (outgoing)
* or src == remote and dst == local (incoming).
*/
if ((cmpsaddr(iph1->local, src) ||
cmpsaddr(iph1->remote, dst)) &&
(cmpsaddr(iph1->local, dst) ||
cmpsaddr(iph1->remote, src))) {
if ((cmpsaddr(iph1->local, src) != CMPSADDR_MATCH ||
cmpsaddr(iph1->remote, dst) != CMPSADDR_MATCH) &&
(cmpsaddr(iph1->local, dst) != CMPSADDR_MATCH ||
cmpsaddr(iph1->remote, src) != CMPSADDR_MATCH)) {
msg = next;
continue;
}

View File

@ -1,4 +1,4 @@
/* $NetBSD: isakmp_inf.c,v 1.45 2011/01/22 07:38:51 tteras Exp $ */
/* $NetBSD: isakmp_inf.c,v 1.46 2011/03/14 17:18:13 tteras Exp $ */
/* Id: isakmp_inf.c,v 1.44 2006/05/06 20:45:52 manubsd Exp */
@ -1177,7 +1177,7 @@ purge_ipsec_spi(dst0, proto, spi, n)
/* don't delete inbound SAs at the moment */
/* XXX should we remove SAs with opposite direction as well? */
if (cmpsaddr(dst0, dst)) {
if (cmpsaddr(dst0, dst) != CMPSADDR_MATCH) {
msg = next;
continue;
}
@ -1355,10 +1355,10 @@ isakmp_info_recv_initialcontact(iph1, protectedph2)
* ports. Correct thing to do is delete all entries with
* same identity. -TT
*/
if ((cmpsaddr(iph1->local, src) != 0 ||
cmpsaddr(iph1->remote, dst) != 0) &&
(cmpsaddr(iph1->local, dst) != 0 ||
cmpsaddr(iph1->remote, src) != 0))
if ((cmpsaddr(iph1->local, src) != CMPSADDR_MATCH ||
cmpsaddr(iph1->remote, dst) != CMPSADDR_MATCH) &&
(cmpsaddr(iph1->local, dst) != CMPSADDR_MATCH ||
cmpsaddr(iph1->remote, src) != CMPSADDR_MATCH))
continue;
/*

View File

@ -1,4 +1,4 @@
/* $NetBSD: isakmp_quick.c,v 1.28 2010/12/07 14:28:12 tteras Exp $ */
/* $NetBSD: isakmp_quick.c,v 1.29 2011/03/14 17:18:13 tteras Exp $ */
/* Id: isakmp_quick.c,v 1.29 2006/08/22 18:17:17 manubsd Exp */
@ -629,7 +629,7 @@ quick_i2recv(iph2, msg0)
#endif
if (cmpsaddr((struct sockaddr *) &proposed_addr,
(struct sockaddr *) &got_addr) == 0) {
(struct sockaddr *) &got_addr) == CMPSADDR_MATCH) {
plog(LLV_DEBUG, LOCATION, NULL,
"IDci matches proposal.\n");
#ifdef ENABLE_NATT
@ -677,13 +677,13 @@ quick_i2recv(iph2, msg0)
#endif
if (cmpsaddr((struct sockaddr *) &proposed_addr,
(struct sockaddr *) &got_addr) == 0) {
(struct sockaddr *) &got_addr) == CMPSADDR_MATCH) {
plog(LLV_DEBUG, LOCATION, NULL,
"IDcr matches proposal.\n");
#ifdef ENABLE_NATT
} else if (iph2->natoa_dst != NULL
&& cmpsaddr(iph2->natoa_dst,
(struct sockaddr *) &got_addr) == 0) {
(struct sockaddr *) &got_addr) == CMPSADDR_MATCH) {
plog(LLV_DEBUG, LOCATION, NULL,
"IDcr matches NAT-OAr.\n");
#endif

View File

@ -1,4 +1,4 @@
/* $NetBSD: nattraversal.c,v 1.13 2009/09/01 12:22:09 tteras Exp $ */
/* $NetBSD: nattraversal.c,v 1.14 2011/03/14 17:18:13 tteras Exp $ */
/*
* Copyright (C) 2004 SuSE Linux AG, Nuernberg, Germany.
@ -398,8 +398,8 @@ natt_keepalive_add (struct sockaddr *src, struct sockaddr *dst)
struct natt_ka_addrs *ka = NULL, *new_addr;
TAILQ_FOREACH (ka, &ka_tree, chain) {
if (cmpsaddr(ka->src, src) == 0 &&
cmpsaddr(ka->dst, dst) == 0) {
if (cmpsaddr(ka->src, src) == CMPSADDR_MATCH &&
cmpsaddr(ka->dst, dst) == CMPSADDR_MATCH) {
ka->in_use++;
plog (LLV_INFO, LOCATION, NULL, "KA found: %s (in_use=%u)\n",
saddr2str_fromto("%s->%s", src, dst), ka->in_use);
@ -462,8 +462,8 @@ natt_keepalive_remove (struct sockaddr *src, struct sockaddr *dst)
plog (LLV_DEBUG, LOCATION, NULL, "KA tree dump: %s (in_use=%u)\n",
saddr2str_fromto("%s->%s", src, dst), ka->in_use);
if (cmpsaddr(ka->src, src) == 0 &&
cmpsaddr(ka->dst, dst) == 0 &&
if (cmpsaddr(ka->src, src) == CMPSADDR_MATCH &&
cmpsaddr(ka->dst, dst) == CMPSADDR_MATCH &&
-- ka->in_use <= 0) {
plog (LLV_DEBUG, LOCATION, NULL, "KA removing this one...\n");

View File

@ -1,6 +1,6 @@
/* $NetBSD: pfkey.c,v 1.55 2011/03/01 14:33:58 vanhu Exp $ */
/* $NetBSD: pfkey.c,v 1.56 2011/03/14 17:18:13 tteras Exp $ */
/* $Id: pfkey.c,v 1.55 2011/03/01 14:33:58 vanhu Exp $ */
/* $Id: pfkey.c,v 1.56 2011/03/14 17:18:13 tteras Exp $ */
/*
* Copyright (C) 1995, 1996, 1997, and 1998 WIDE Project.
@ -2882,8 +2882,8 @@ migrate_ph1_ike_addresses(iph1, arg)
u_int16_t port;
/* Already up-to-date? */
if (cmpsaddr(iph1->local, ma->local) == 0 &&
cmpsaddr(iph1->remote, ma->remote) == 0)
if (cmpsaddr(iph1->local, ma->local) == CMPSADDR_MATCH &&
cmpsaddr(iph1->remote, ma->remote) == CMPSADDR_MATCH)
return 0;
if (iph1->status < PHASE1ST_ESTABLISHED) {
@ -2983,8 +2983,8 @@ migrate_ph2_ike_addresses(iph2, arg)
migrate_ph1_ike_addresses(iph2->ph1, arg);
/* Already up-to-date? */
if (cmpsaddr(iph2->src, ma->local) == 0 &&
cmpsaddr(iph2->dst, ma->remote) == 0)
if (cmpsaddr(iph2->src, ma->local) == CMPSADDR_MATCH &&
cmpsaddr(iph2->dst, ma->remote) == CMPSADDR_MATCH)
return 0;
/* save src/dst as sa_src/sa_dst before rewriting */
@ -3207,8 +3207,8 @@ migrate_ph2_one_isr(spid, isr_cur, xisr_old, xisr_new)
"changing address families (%d to %d) for endpoints.\n",
osaddr->sa_family, nsaddr->sa_family);
if (cmpsaddr(osaddr, (struct sockaddr *) &saidx->src) ||
cmpsaddr(odaddr, (struct sockaddr *) &saidx->dst)) {
if (cmpsaddr(osaddr, (struct sockaddr *) &saidx->src) != CMPSADDR_MATCH ||
cmpsaddr(odaddr, (struct sockaddr *) &saidx->dst) != CMPSADDR_MATCH) {
plog(LLV_DEBUG, LOCATION, NULL, "SADB_X_MIGRATE: "
"mismatch of addresses in saidx and xisr.\n");
return -1;

View File

@ -1,4 +1,4 @@
/* $NetBSD: policy.c,v 1.11 2009/07/03 06:41:46 tteras Exp $ */
/* $NetBSD: policy.c,v 1.12 2011/03/14 17:18:13 tteras Exp $ */
/* $KAME: policy.c,v 1.46 2001/11/16 04:08:10 sakane Exp $ */
@ -142,7 +142,7 @@ getsp_r(spidx, iph2)
plog(LLV_DEBUG, LOCATION, NULL, "src2: %s\n",
saddr2str((struct sockaddr *)&spidx->src));
if (cmpsaddr(iph2->src, (struct sockaddr *) &spidx->src) ||
if (cmpsaddr(iph2->src, (struct sockaddr *) &spidx->src) != CMPSADDR_MATCH ||
spidx->prefs != prefixlen)
return NULL;
@ -151,7 +151,7 @@ getsp_r(spidx, iph2)
plog(LLV_DEBUG, LOCATION, NULL, "dst2: %s\n",
saddr2str((struct sockaddr *)&spidx->dst));
if (cmpsaddr(iph2->dst, (struct sockaddr *) &spidx->dst) ||
if (cmpsaddr(iph2->dst, (struct sockaddr *) &spidx->dst) != CMPSADDR_MATCH ||
spidx->prefd != prefixlen)
return NULL;
@ -201,10 +201,10 @@ cmpspidxstrict(a, b)
return 1;
if (cmpsaddr((struct sockaddr *) &a->src,
(struct sockaddr *) &b->src))
(struct sockaddr *) &b->src) != CMPSADDR_MATCH)
return 1;
if (cmpsaddr((struct sockaddr *) &a->dst,
(struct sockaddr *) &b->dst))
(struct sockaddr *) &b->dst) != CMPSADDR_MATCH)
return 1;
#ifdef HAVE_SECCTX
@ -261,7 +261,7 @@ cmpspidxwild(a, b)
a, b->prefs, saddr2str((struct sockaddr *)&sa1));
plog(LLV_DEBUG, LOCATION, NULL, "%p masked with /%d: %s\n",
b, b->prefs, saddr2str((struct sockaddr *)&sa2));
if (cmpsaddr((struct sockaddr *)&sa1, (struct sockaddr *)&sa2))
if (cmpsaddr((struct sockaddr *)&sa1, (struct sockaddr *)&sa2) > CMPSADDR_WILDPORT_MATCH)
return 1;
#ifndef __linux__
@ -279,7 +279,7 @@ cmpspidxwild(a, b)
a, b->prefd, saddr2str((struct sockaddr *)&sa1));
plog(LLV_DEBUG, LOCATION, NULL, "%p masked with /%d: %s\n",
b, b->prefd, saddr2str((struct sockaddr *)&sa2));
if (cmpsaddr((struct sockaddr *)&sa1, (struct sockaddr *)&sa2))
if (cmpsaddr((struct sockaddr *)&sa1, (struct sockaddr *)&sa2) > CMPSADDR_WILDPORT_MATCH)
return 1;
#ifdef HAVE_SECCTX

View File

@ -1,4 +1,4 @@
/* $NetBSD: sockmisc.c,v 1.18 2010/02/28 15:52:16 snj Exp $ */
/* $NetBSD: sockmisc.c,v 1.19 2011/03/14 17:18:13 tteras Exp $ */
/* Id: sockmisc.c,v 1.24 2006/05/07 21:32:59 manubsd Exp */
@ -132,11 +132,13 @@ cmpsaddr(addr1, addr2)
return CMPSADDR_MISMATCH;
}
if (port1 == port2 ||
port1 == IPSEC_PORT_ANY ||
port2 == IPSEC_PORT_ANY)
if (port1 == port2)
return CMPSADDR_MATCH;
if (port1 == IPSEC_PORT_ANY ||
port2 == IPSEC_PORT_ANY)
return CMPSADDR_WILDPORT_MATCH;
return CMPSADDR_WOP_MATCH;
}
@ -934,7 +936,7 @@ naddr_score(const struct netaddr *naddr, const struct sockaddr *saddr)
free(a2);
free(a3);
}
if (cmpsaddr(&sa, &naddr->sa.sa) == 0)
if (cmpsaddr(&sa, &naddr->sa.sa) <= CMPSADDR_WOP_MATCH)
return naddr->prefix + port_score;
return -1;

View File

@ -1,4 +1,4 @@
/* $NetBSD: sockmisc.h,v 1.12 2010/02/28 15:52:16 snj Exp $ */
/* $NetBSD: sockmisc.h,v 1.13 2011/03/14 17:18:13 tteras Exp $ */
/* Id: sockmisc.h,v 1.9 2005/10/05 16:55:41 manubsd Exp */
@ -57,8 +57,9 @@ struct netaddr {
extern const int niflags;
#define CMPSADDR_MATCH 0
#define CMPSADDR_WOP_MATCH 1
#define CMPSADDR_MISMATCH 2
#define CMPSADDR_WILDPORT_MATCH 1
#define CMPSADDR_WOP_MATCH 2
#define CMPSADDR_MISMATCH 3
extern int cmpsaddr __P((const struct sockaddr *, const struct sockaddr *));

View File

@ -1,4 +1,4 @@
/* $NetBSD: throttle.c,v 1.6 2009/07/03 06:41:47 tteras Exp $ */
/* $NetBSD: throttle.c,v 1.7 2011/03/14 17:18:13 tteras Exp $ */
/* Id: throttle.c,v 1.5 2006/04/05 20:54:50 manubsd Exp */
@ -104,7 +104,7 @@ restart:
goto restart;
}
if (cmpsaddr(addr, (struct sockaddr *) &te->host) == 0) {
if (cmpsaddr(addr, (struct sockaddr *) &te->host) <= CMPSADDR_WOP_MATCH) {
found = 1;
break;
}