ref: 52b773d635aa7ffefac5f1e64ed97c3d67d34e5f
parent: 5c326d9f3566be32af7b18d2638fd8d26b169e31
author: cinap_lenrek <cinap_lenrek@felloff.net>
date: Sun Aug 26 14:36:35 EDT 2018
ip/dhcpd: reject bogus requests, handle multiple ether= attributes in ndb, cleanup unless relay agent (gaddr) is specified, dhcp requests need to taget a local ip address on the incoming interface or broadcast. clients might have multiple ethernet interfaces, so we need to check if any of the ether= attributes in ndb matches. this is done by passing lookupip() the attribute name and a expected value and if a match is found, set Info.indb = 1. remove tohex(), use encodefmt instead. avoid dynamcic allocation. include interface device in log messages.
--- a/sys/src/cmd/ip/dhcpd/dat.h
+++ b/sys/src/cmd/ip/dhcpd/dat.h
@@ -27,8 +27,7 @@
typedef struct Info Info;
struct Info
{
- int indb; /* true if found in database */
-
+ int indb; /* true when found in ndb */
Ipifc *ifc; /* ifc when directly connected */
uchar ipaddr[NDB_IPlen]; /* ip address of system */
@@ -35,18 +34,17 @@
uchar ipmask[NDB_IPlen]; /* ip network mask */
uchar ipnet[NDB_IPlen]; /* ip network address (ipaddr & ipmask) */
- char domain[Maxstr]; /* system domain name */
+ char domain[Maxstr]; /* system domain name */
char bootf[Maxstr]; /* boot file */
- char bootf2[Maxstr]; /* alternative boot file */
+ char bootf2[Maxstr]; /* alternative boot file */
uchar tftp[NDB_IPlen]; /* ip addr of tftp server */
uchar tftp2[NDB_IPlen]; /* ip addr of alternate server */
- uchar etheraddr[6]; /* ethernet address */
uchar gwip[NDB_IPlen]; /* gateway ip address */
uchar fsip[NDB_IPlen]; /* file system ip address */
uchar auip[NDB_IPlen]; /* authentication server ip address */
char rootpath[Maxstr]; /* rootfs for diskless nfs clients */
char dhcpgroup[Maxstr];
- char vendor[Maxstr]; /* vendor info */
+ char vendor[Maxstr]; /* vendor info */
};
@@ -56,7 +54,6 @@
extern int minlease;
/* from db.c */
-extern char* tohex(char*, uchar*, int);
extern char* toid(uchar*, int);
extern void initbinding(uchar*, int);
extern Binding* iptobinding(uchar*, int);
@@ -70,10 +67,10 @@
/* from ndb.c */
extern int lookup(Bootp*, Info*, Info*);
-extern int lookupip(uchar*, Info*, int);
+extern int lookupip(uchar*, char*, char*, Info*, int);
extern void lookupname(char*, int, Ndbtuple*);
extern Ipifc* findifc(uchar*);
-extern Iplifc* findlifc(uchar*, Ipifc*);
+extern Iplifc* localonifc(uchar*, Ipifc*);
extern void localip(uchar*, uchar*, Ipifc*);
extern int lookupserver(char*, uchar**, int, Ndbtuple *t);
extern Ndbtuple* lookupinfo(uchar *ipaddr, char **attr, int n);
--- a/sys/src/cmd/ip/dhcpd/db.c
+++ b/sys/src/cmd/ip/dhcpd/db.c
@@ -18,36 +18,6 @@
char *binddir = "/lib/ndb/dhcp";
/*
- * convert a byte array to hex
- */
-static char
-hex(int x)
-{
- if(x < 10)
- return x + '0';
- return x - 10 + 'a';
-}
-extern char*
-tohex(char *hdr, uchar *p, int len)
-{
- char *s, *sp;
- int hlen;
-
- hlen = strlen(hdr);
- s = malloc(hlen + 2*len + 1);
- sp = s;
- strcpy(sp, hdr);
- sp += hlen;
- for(; len > 0; len--){
- *sp++ = hex(*p>>4);
- *sp++ = hex(*p & 0xf);
- p++;
- }
- *sp = 0;
- return s;
-}
-
-/*
* convert a client id to a string. If it's already
* ascii, leave it be. Otherwise, convert it to hex.
*/
@@ -54,16 +24,16 @@
extern char*
toid(uchar *p, int n)
{
+ static char id[Maxstr];
int i;
- char *s;
- for(i = 0; i < n; i++)
- if(!isprint(p[i]))
- return tohex("id", p, n);
- s = malloc(n + 1);
- memmove(s, p, n);
- s[n] = 0;
- return s;
+ for(i = 0; i < n && isprint(p[i]); i++)
+ ;
+ if(i == n)
+ snprint(id, sizeof(id), "%.*s", n, (char*)p);
+ else
+ snprint(id, sizeof(id), "id%.*lH", n, p);
+ return id;
}
/*
--- a/sys/src/cmd/ip/dhcpd/dhcpd.c
+++ b/sys/src/cmd/ip/dhcpd/dhcpd.c
@@ -227,6 +227,8 @@
fmtinstall('I', eipfmt);
fmtinstall('V', eipfmt);
fmtinstall('M', eipfmt);
+ fmtinstall('H', encodefmt);
+
ARGBEGIN {
case '6':
v6opts = 1;
@@ -317,8 +319,6 @@
op = optbuf;
*op = 0;
proto(&r, n);
- if(r.id != nil)
- free(r.id);
}
}
@@ -325,8 +325,6 @@
void
proto(Req *rp, int n)
{
- char buf[64];
-
now = time(0);
rp->e = rp->buf + n;
@@ -345,6 +343,32 @@
return;
}
+ if(!isv4(rp->up->laddr))
+ return;
+
+ ipifcs = readipifc(net, ipifcs, -1);
+ if((rp->ifc = findifc(rp->up->ifcaddr)) == nil){
+ warning(0, "no interface");
+ return;
+ }
+ if(validip(rp->giaddr)){
+ /* info about gateway */
+ if(lookupip(rp->giaddr, nil, nil, &rp->gii, 1) < 0){
+ warning(0, "unknown gateway %I", rp->giaddr);
+ return;
+ }
+ rp->gii.ifc = nil;
+ } else {
+ /* no gateway, directly connected */
+ if(ipcmp(rp->up->laddr, IPv4bcast) != 0 && localonifc(rp->up->laddr, rp->ifc) == nil){
+ warning(0, "wrong network %I->%I on %s",
+ rp->up->raddr, rp->up->laddr, rp->ifc->dev);
+ return;
+ }
+ memset(&rp->gii, 0, sizeof(rp->gii));
+ rp->gii.ifc = rp->ifc;
+ }
+
if(rp->e < (uchar*)rp->bp->sname){
warning(0, "packet too short");
return;
@@ -364,38 +388,16 @@
* which could be a mistake.
*/
if(rp->id == nil){
- if(rp->bp->hlen > Maxhwlen){
- warning(0, "hlen %d", rp->bp->hlen);
- return;
- }
- if(memcmp(zeros, rp->bp->chaddr, rp->bp->hlen) == 0){
+ static char hwaid[Maxstr];
+
+ if(rp->bp->hlen > Maxhwlen || memcmp(zeros, rp->bp->chaddr, rp->bp->hlen) == 0){
warning(0, "no chaddr");
return;
}
- sprint(buf, "hwa%2.2ux_", rp->bp->htype);
- rp->id = tohex(buf, rp->bp->chaddr, rp->bp->hlen);
+ snprint(hwaid, sizeof(hwaid), "hwa%2.2ux_%.*lH", rp->bp->htype, rp->bp->hlen, rp->bp->chaddr);
+ rp->id = hwaid;
}
- ipifcs = readipifc(net, ipifcs, -1);
- rp->ifc = findifc(rp->up->ifcaddr);
- if(rp->ifc == nil){
- warning(0, "no interface");
- return;
- }
-
- if(validip(rp->giaddr)){
- /* info about gateway */
- if(lookupip(rp->giaddr, &rp->gii, 1) < 0){
- warning(0, "lookupip failed");
- return;
- }
- rp->gii.ifc = nil;
- } else {
- /* no gateway, directly connected */
- memset(&rp->gii, 0, sizeof(rp->gii));
- rp->gii.ifc = rp->ifc;
- }
-
/* info about target system */
if(lookup(rp->bp, &rp->ii, &rp->gii) == 0)
if(rp->ii.indb && rp->ii.dhcpgroup[0] == 0)
@@ -485,8 +487,8 @@
}
}
if(b == nil){
- warning(0, "!Discover(%s via %I): no binding %I",
- rp->id, rp->gii.ipaddr, rp->ip);
+ warning(0, "!Discover(%s via %I on %s): no binding %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ip);
return;
}
mkoffer(b, rp->id, rp->leasetime);
@@ -504,8 +506,8 @@
/* check for hard assignment */
if(rp->staticbinding){
if(findifc(rp->server) != rp->ifc) {
- warning(0, "!Request(%s via %I): for server %I not me",
- rp->id, rp->gii.ipaddr, rp->server);
+ warning(0, "!Request(%s via %I on %s): for server %I not me",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->server);
} else
sendack(rp, rp->ii.ipaddr,
(staticlease > minlease? staticlease:
@@ -517,8 +519,8 @@
/* if we don't have an offer, nak */
if(b == nil){
- warning(0, "!Request(%s via %I): no offer",
- rp->id, rp->gii.ipaddr);
+ warning(0, "!Request(%s via %I on %s): no offer",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev);
if(findifc(rp->server) == rp->ifc)
sendnak(rp, rp->server, "no offer for you");
return;
@@ -527,8 +529,8 @@
/* if not for me, retract offer */
if(findifc(rp->server) != rp->ifc){
b->expoffer = 0;
- warning(0, "!Request(%s via %I): for server %I not me",
- rp->id, rp->gii.ipaddr, rp->server);
+ warning(0, "!Request(%s via %I on %s): for server %I not me",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->server);
return;
}
@@ -537,14 +539,14 @@
* client really shouldn't be specifying this when selecting
*/
if(validip(rp->ip) && ipcmp(rp->ip, b->ip) != 0){
- warning(0, "!Request(%s via %I): requests %I, not %I",
- rp->id, rp->gii.ipaddr, rp->ip, b->ip);
+ warning(0, "!Request(%s via %I on %s): requests %I, not %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ip, b->ip);
sendnak(rp, rp->ip, "bad ip address option");
return;
}
if(commitbinding(b) < 0){
- warning(0, "!Request(%s via %I): can't commit %I",
- rp->id, rp->gii.ipaddr, b->ip);
+ warning(0, "!Request(%s via %I on %s): can't commit %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, b->ip);
sendnak(rp, b->ip, "can't commit binding");
return;
}
@@ -559,8 +561,8 @@
/* check for hard assignment */
if(rp->staticbinding){
if(ipcmp(rp->ip, rp->ii.ipaddr) != 0){
- warning(0, "!Request(%s via %I): %I not valid for %E",
- rp->id, rp->gii.ipaddr, rp->ip, rp->bp->chaddr);
+ warning(0, "!Request(%s via %I on %s): %I not valid for %E",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ip, rp->bp->chaddr);
sendnak(rp, rp->ip, "not valid");
} else
sendack(rp, rp->ip, (staticlease > minlease?
@@ -570,19 +572,19 @@
/* make sure the network makes sense */
if(!samenet(rp->ip, &rp->gii)){
- warning(0, "!Request(%s via %I): bad forward of %I",
- rp->id, rp->gii.ipaddr, rp->ip);
+ warning(0, "!Request(%s via %I on %s): bad forward of %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ip);
return;
}
b = iptobinding(rp->ip, 0);
if(b == nil){
- warning(0, "!Request(%s via %I): no binding for %I",
- rp->id, rp->gii.ipaddr, rp->ip);
+ warning(0, "!Request(%s via %I on %s): no binding for %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ip);
return;
}
if(ipcmp(rp->ip, b->ip) != 0 || now > b->lease){
- warning(0, "!Request(%s via %I): %I not valid",
- rp->id, rp->gii.ipaddr, rp->ip);
+ warning(0, "!Request(%s via %I on %s): %I not valid",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ip);
sendnak(rp, rp->ip, "not valid");
return;
}
@@ -601,8 +603,8 @@
/* check for hard assignment */
if(rp->staticbinding){
if(ipcmp(rp->ciaddr, rp->ii.ipaddr) != 0){
- warning(0, "!Request(%s via %I): %I not valid",
- rp->id, rp->gii.ipaddr, rp->ciaddr);
+ warning(0, "!Request(%s via %I on %s): %I not valid",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ciaddr);
sendnak(rp, rp->ciaddr, "not valid");
} else
sendack(rp, rp->ciaddr, (staticlease > minlease?
@@ -612,26 +614,26 @@
/* make sure the network makes sense */
if(!samenet(rp->ciaddr, &rp->gii)){
- warning(0, "!Request(%s via %I): bad forward of %I",
- rp->id, rp->gii.ipaddr, rp->ip);
+ warning(0, "!Request(%s via %I on %s): bad forward of %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ciaddr);
return;
}
b = iptobinding(rp->ciaddr, 0);
if(b == nil){
- warning(0, "!Request(%s via %I): no binding for %I",
- rp->id, rp->ciaddr, rp->ciaddr);
+ warning(0, "!Request(%s via %I on %s): no binding for %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ciaddr);
return;
}
if(ipcmp(rp->ciaddr, b->ip) != 0){
- warning(0, "!Request(%s via %I): %I not valid",
- rp->id, rp->gii.ipaddr, rp->ciaddr);
+ warning(0, "!Request(%s via %I on %s): %I not valid",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ciaddr);
sendnak(rp, rp->ciaddr, "invalid ip address");
return;
}
mkoffer(b, rp->id, rp->leasetime);
if(commitbinding(b) < 0){
- warning(0, "!Request(%s via %I): can't commit %I",
- rp->id, rp->gii.ipaddr, b->ip);
+ warning(0, "!Request(%s via %I on %s): can't commit %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, b->ip);
sendnak(rp, b->ip, "can't commit binding");
return;
}
@@ -650,8 +652,8 @@
b = idtooffer(rp->id, &rp->gii);
if(b == nil){
- warning(0, "!Decline(%s via %I): no binding",
- rp->id, rp->gii.ipaddr);
+ warning(0, "!Decline(%s via %I on %s): no binding",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev);
return;
}
@@ -671,16 +673,17 @@
b = idtobinding(rp->id, &rp->gii, 0);
if(b == nil){
- warning(0, "!Release(%s via %I): no binding",
- rp->id, rp->gii.ipaddr);
+ warning(0, "!Release(%s via %I on %s): no binding",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev);
return;
}
if(strcmp(rp->id, b->boundto) != 0){
- warning(0, "!Release(%s via %I): invalid release of %I",
- rp->id, rp->gii.ipaddr, rp->ip);
+ warning(0, "!Release(%s via %I on %s): invalid release of %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ip);
return;
}
- warning(0, "Release(%s via %I): releasing %I", b->boundto, rp->gii.ipaddr, b->ip);
+ warning(0, "Release(%s via %I on %s): releasing %I",
+ b->boundto, rp->gii.ipaddr, rp->ifc->dev, b->ip);
if(releasebinding(b, rp->id) < 0)
warning(0, "release: couldn't release");
}
@@ -697,8 +700,8 @@
b = iptobinding(rp->ciaddr, 0);
if(b == nil){
- warning(0, "!Inform(%s via %I): no binding for %I",
- rp->id, rp->gii.ipaddr, rp->ip);
+ warning(0, "!Inform(%s via %I on %s): no binding for %I",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev, rp->ip);
return;
}
sendack(rp, b->ip, 0, 0);
@@ -905,10 +908,10 @@
ushort flags;
Info *iip;
- warning(0, "bootp %s %I->%I from %s via %I, file %s",
+ warning(0, "bootp %s %I->%I from %s via %I on %s, file %s",
rp->genrequest? "generic": (rp->p9request? "p9": ""),
rp->up->raddr, rp->up->laddr,
- rp->id, rp->gii.ipaddr,
+ rp->id, rp->gii.ipaddr, rp->ifc->dev,
rp->bp->file);
if(nobootp)
@@ -919,7 +922,8 @@
iip = &rp->ii;
if(rp->staticbinding == 0){
- warning(0, "bootp from unknown %s via %I", rp->id, rp->gii.ipaddr);
+ warning(0, "bootp from unknown %s via %I on %s",
+ rp->id, rp->gii.ipaddr, rp->ifc->dev);
return;
}
@@ -1087,7 +1091,7 @@
case ODclientid:
if(n <= 1)
break;
- rp->id = toid( o, n);
+ rp->id = toid(o, n);
break;
case ODparams:
if(n > sizeof(rp->requested))
@@ -1138,7 +1142,7 @@
maskopt(rp, OBmask, rp->ii.ipmask);
else if(validip(rp->gii.ipmask))
maskopt(rp, OBmask, rp->gii.ipmask);
- else if((lifc = findlifc(ip, rp->ifc)) != nil)
+ else if((lifc = localonifc(ip, rp->ifc)) != nil)
maskopt(rp, OBmask, lifc->mask);
if(validip(rp->ii.gwip)){
--- a/sys/src/cmd/ip/dhcpd/ndb.c
+++ b/sys/src/cmd/ip/dhcpd/ndb.c
@@ -48,7 +48,7 @@
}
Iplifc*
-findlifc(uchar *ip, Ipifc *ifc)
+localonifc(uchar *ip, Ipifc *ifc)
{
uchar x[IPaddrlen];
Iplifc *lifc;
@@ -69,7 +69,7 @@
{
Iplifc *lifc;
- if((lifc = findlifc(raddr, ifc)) != nil)
+ if((lifc = localonifc(raddr, ifc)) != nil)
ipmove(laddr, lifc->ip);
else if(ipcmp(laddr, IPv4bcast) == 0)
ipmove(laddr, IPnoaddr);
@@ -76,8 +76,6 @@
}
-uchar noetheraddr[6];
-
static void
setipaddr(uchar *addr, char *ip)
{
@@ -96,7 +94,7 @@
* do an ipinfo with defaults
*/
int
-lookupip(uchar *ipaddr, Info *iip, int gate)
+lookupip(uchar *ipaddr, char *hwattr, char *hwval, Info *iip, int gate)
{
char ip[32];
Ndbtuple *t, *nt;
@@ -119,11 +117,12 @@
*p++ = "rootpath";
*p++ = "dhcp";
*p++ = "vendorclass";
- *p++ = "ether";
*p++ = "dom";
*p++ = "@fs";
*p++ = "@auth";
}
+ if(hwattr != nil)
+ *p++ = hwattr;
*p = 0;
memset(iip, 0, sizeof(*iip));
@@ -154,17 +153,6 @@
if(strcmp(nt->attr, "ipgw") == 0)
setipaddr(iip->gwip, nt->val);
else
- if(strcmp(nt->attr, "ether") == 0){
- /*
- * this is probably wrong for machines with multiple
- * ethers. bootp or dhcp requests could come from any
- * of the ethers listed in the ndb entry.
- */
- if(memcmp(iip->etheraddr, noetheraddr, 6) == 0)
- parseether(iip->etheraddr, nt->val);
- iip->indb = 1;
- }
- else
if(strcmp(nt->attr, "dhcp") == 0){
if(iip->dhcpgroup[0] == 0)
strncpy(iip->dhcpgroup, nt->val, sizeof(iip->dhcpgroup)-1);
@@ -194,6 +182,9 @@
if(iip->rootpath[0] == 0)
strncpy(iip->rootpath, nt->val, sizeof(iip->rootpath)-1);
}
+ if(hwattr != nil && strcmp(nt->attr, hwattr) == 0)
+ if(strcmp(hwval, nt->val) == 0)
+ iip->indb = 1;
}
ndbfree(t);
maskip(iip->ipaddr, iip->ipmask, iip->ipnet);
@@ -207,57 +198,47 @@
int
lookup(Bootp *bp, Info *iip, Info *riip)
{
+ char *hwattr, hwval[Maxhwlen*2+1];
+ uchar ciaddr[IPaddrlen];
Ndbtuple *t, *nt;
Ndbs s;
- char *hwattr;
- char *hwval, hwbuf[33];
- uchar ciaddr[IPaddrlen];
+ memset(iip, 0, sizeof(*iip));
+
if(opendb() == nil){
warning(1, "can't open db");
return -1;
}
- memset(iip, 0, sizeof(*iip));
+ switch(bp->htype){
+ case 1:
+ hwattr = "ether";
+ snprint(hwval, sizeof(hwval), "%E", bp->chaddr);
+ break;
+ default:
+ hwattr = nil;
+ }
/* client knows its address? */
v4tov6(ciaddr, bp->ciaddr);
if(validip(ciaddr)){
if(!samenet(ciaddr, riip)){
- warning(0, "%I not on %I", ciaddr, riip->ipnet);
+ if(riip->ifc != nil)
+ warning(0, "%I not on %s", ciaddr, riip->ifc->dev);
+ else
+ warning(0, "%I not on %I", ciaddr, riip->ipnet);
return -1;
}
- if(lookupip(ciaddr, iip, 0) < 0) {
+ if(lookupip(ciaddr, hwattr, hwval, iip, 0) < 0) {
if (debug)
warning(0, "don't know %I", ciaddr);
return -1; /* don't know anything about it */
}
-
- /*
- * see if this is a masquerade, i.e., if the ether
- * address doesn't match what we expected it to be.
- */
- if(memcmp(iip->etheraddr, zeros, 6) != 0)
- if(memcmp(bp->chaddr, iip->etheraddr, 6) != 0)
- warning(0, "ciaddr %I rcvd from %E instead of %E",
- ciaddr, bp->chaddr, iip->etheraddr);
-
return 0;
}
- if(bp->hlen > Maxhwlen)
+ if(hwattr == nil)
return -1;
- switch(bp->htype){
- case 1:
- hwattr = "ether";
- hwval = hwbuf;
- snprint(hwbuf, sizeof(hwbuf), "%E", bp->chaddr);
- break;
- default:
- syslog(0, blog, "not ethernet %E, htype %d, hlen %d",
- bp->chaddr, bp->htype, bp->hlen);
- return -1;
- }
/*
* use hardware address to find an ip address on
@@ -272,7 +253,7 @@
continue;
if(!validip(ciaddr) || !samenet(ciaddr, riip))
continue;
- if(lookupip(ciaddr, iip, 0) < 0)
+ if(lookupip(ciaddr, hwattr, hwval, iip, 0) < 0)
continue;
ndbfree(t);
return 0;