* [PATCH] dhcp: Don't re-use request message for reply
@ 2025-01-31 14:53 Enrique Llorente
0 siblings, 0 replies; only message in thread
From: Enrique Llorente @ 2025-01-31 14:53 UTC (permalink / raw)
To: passt-dev; +Cc: Enrique Llorente
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@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;
}
+
--
@@ -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
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2025-01-31 14:53 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-01-31 14:53 [PATCH] dhcp: Don't re-use request message for reply Enrique Llorente
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).