The logic composing the DHCP reply message is reusing the request message to compose the it, this kind be problematic from a security context and may break the functionality. This change create a new reply message and fill it in with proper fields from request adding on top the generated opetions. Signed-off-by: Enrique Llorente <ellorent(a)redhat.com> --- dhcp.c | 55 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/dhcp.c b/dhcp.c index d8515aa..d8ff330 100644 --- a/dhcp.c +++ b/dhcp.c @@ -142,17 +142,36 @@ static void fill_one(struct msg *m, int o, int *offset) } /** - * fill() - Fill options in message - * @m: Message to fill + * fill() - Fill fields and options in response message + * @c: Execution context to copy from + * @req: Request message to copy from + * @resp: Response Message to write to * * Return: current size of options field */ -static int fill(struct msg *m) +static int fill(const struct ctx *c, struct msg const* req, + struct msg *resp) { int i, o, offset = 0; - m->op = BOOTREPLY; - m->secs = 0; + resp->op = BOOTREPLY; + resp->secs = 0; + resp->hops = 0; // We are not a RELAY agent + memset(&resp->sname, 0, sizeof(resp->sname)); + memset(&resp->file, 0, sizeof(resp->file)); + resp->yiaddr = c->ip4.addr; + + + /* Copy these fields from request */ + memcpy(&resp->chaddr, req->chaddr, sizeof(resp->chaddr)); + resp->htype = req->htype; + resp->hlen = req->hlen; + resp->xid = req->xid; + resp->flags = req->flags; + resp->ciaddr = req->ciaddr; + resp->siaddr = req->siaddr; /* TODO server ip ? */ + resp->giaddr = req->giaddr; + resp->magic = req->magic; for (o = 0; o < 255; o++) opts[o].sent = 0; @@ -162,24 +181,24 @@ static int fill(struct msg *m) * Put it there explicitly, unless requested via option 55. */ if (opts[55].clen > 0 && !memchr(opts[55].c, 53, opts[55].clen)) - fill_one(m, 53, &offset); + fill_one(resp, 53, &offset); for (i = 0; i < opts[55].clen; i++) { o = opts[55].c[i]; if (opts[o].slen != -1) - fill_one(m, o, &offset); + fill_one(resp, o, &offset); } for (o = 0; o < 255; o++) { if (opts[o].slen != -1 && !opts[o].sent) - fill_one(m, o, &offset); + fill_one(resp, o, &offset); } - m->o[offset++] = 255; - m->o[offset++] = 0; + resp->o[offset++] = 255; + resp->o[offset++] = 0; if (offset < OPT_MIN) { - memset(&m->o[offset], 0, OPT_MIN - offset); + memset(&resp->o[offset], 0, OPT_MIN - offset); offset = OPT_MIN; } @@ -291,8 +310,9 @@ int dhcp(const struct ctx *c, const struct pool *p) const struct ethhdr *eh; const struct iphdr *iph; const struct udphdr *uh; + struct msg const *m; + struct msg resp; unsigned int i; - struct msg *m; eh = packet_get(p, 0, offset, sizeof(*eh), NULL); offset += sizeof(*eh); @@ -321,6 +341,7 @@ int dhcp(const struct ctx *c, const struct pool *p) m->op != BOOTREQUEST) return -1; + offset += offsetof(struct msg, o); for (i = 0; i < ARRAY_SIZE(opts); i++) @@ -364,7 +385,6 @@ int dhcp(const struct ctx *c, const struct pool *p) info(" from %s", eth_ntop(m->chaddr, macstr, sizeof(macstr))); - m->yiaddr = c->ip4.addr; mask.s_addr = htonl(0xffffffff << (32 - c->ip4.prefix_len)); memcpy(opts[1].s, &mask, sizeof(mask)); memcpy(opts[3].s, &c->ip4.guest_gw, sizeof(c->ip4.guest_gw)); @@ -399,16 +419,17 @@ int dhcp(const struct ctx *c, const struct pool *p) opts[6].slen = -1; if (!c->no_dhcp_dns_search) - opt_set_dns_search(c, sizeof(m->o)); + opt_set_dns_search(c, sizeof(resp.o)); + - dlen = offsetof(struct msg, o) + fill(m); + dlen = offsetof(struct msg, o) + fill(c, m, &resp); if (m->flags & FLAG_BROADCAST) dst = in4addr_broadcast; else dst = c->ip4.addr; - - tap_udp4_send(c, c->ip4.our_tap_addr, 67, dst, 68, m, dlen); + tap_udp4_send(c, c->ip4.our_tap_addr, 67, dst, 68, &resp, dlen); return 1; } + -- 2.47.0