On 2021-11-16 09:56 -07, Joel Knight <[email protected]> wrote:
> Hi. On a firewall recently upgraded to 7.0, I noticed that dhcpleased
> was not getting a reply from my ISP's DHCP server during renewal at
> T1. At T2, dhcpleased would broadcast the REQUEST which would be
> answered. Testing with dhclient showed successful renewals at T1.
>
> Inspecting the REQUEST packets showed that dhclient was setting the
> ciaddr to the existing leased IP address while dhcpleased was not
> setting this field. RFC 2131 4.3.2 (with a nice summary at 4.3.6) is
> pretty strict about when ciaddr and the 'requested ip' option should
> be used. The diff below modifies dhcpleased to set ciaddr and
> 'requested ip' at the appropriate times.
Nice catch. I'd like to solve it a bit differently.
The frontend process should not need to know about protocol state
specifics. The idea is that the engine process tells it to send a packet
and that's it. We are doing the same thing with server_identifier when
we transition to RENEWING, that is, we are setting it to INADDR_ANY so
that the frontend doesn't send it out.
(Unfortunately send_discover() in the frontend needs to know to clear
ciaddr. Maybe a bit of refactoring is in order. Also build_packet's
signature gets a bit out of hand. We should probably just pass in an
interface pointer and be done with it.)
>
> While here, I also noticed that if you configure an interface in
> dhcpleased.conf but do not use "set client id", dhcpleased will not
> send the default client id as is stated in dhcpleased.conf(5).
One problem at a time, I'll extract this out in a bit.
>
> Since gmail will undoubtedly muck up this diff, a clean copy is here:
> www.packetmischief.ca/files/patches/dhcpleased.ciaddr.diff
>
>
> .joel
>
Does this work for you?
diff --git dhcpleased.h dhcpleased.h
index b3b4938ad3c..d209d50a246 100644
--- dhcpleased.h
+++ dhcpleased.h
@@ -296,6 +296,7 @@ struct imsg_req_discover {
struct imsg_req_request {
uint32_t if_index;
uint32_t xid;
+ struct in_addr ciaddr;
struct in_addr requested_ip;
struct in_addr server_identifier;
struct in_addr dhcp_server;
diff --git engine.c engine.c
index 17e65fbe789..3abae44afa4 100644
--- engine.c
+++ engine.c
@@ -1486,6 +1486,10 @@ request_dhcp_request(struct dhcpleased_iface *iface)
iface->xid = arc4random();
imsg_req_request.if_index = iface->if_index;
imsg_req_request.xid = iface->xid;
+ if (iface->state == IF_RENEWING || iface->state == IF_REBINDING)
+ imsg_req_request.ciaddr.s_addr = iface->requested_ip.s_addr;
+ else
+ imsg_req_request.ciaddr.s_addr = 0;
imsg_req_request.server_identifier.s_addr =
iface->server_identifier.s_addr;
imsg_req_request.requested_ip.s_addr = iface->requested_ip.s_addr;
diff --git frontend.c frontend.c
index 5131dce1471..e4b2419c96a 100644
--- frontend.c
+++ frontend.c
@@ -70,6 +70,7 @@ struct iface {
struct imsg_ifinfo ifinfo;
int send_discover;
uint32_t xid;
+ struct in_addr ciaddr;
struct in_addr requested_ip;
struct in_addr server_identifier;
struct in_addr dhcp_server;
@@ -91,7 +92,7 @@ struct iface *get_iface_by_id(uint32_t);
void remove_iface(uint32_t);
void set_bpfsock(int, uint32_t);
ssize_t build_packet(uint8_t, char *, uint32_t, struct
ether_addr *,
- struct in_addr *, struct in_addr *);
+ struct in_addr *, struct in_addr *, struct in_addr *);
void send_discover(struct iface *);
void send_request(struct iface *);
void bpf_send_packet(struct iface *, uint8_t *, ssize_t);
@@ -505,6 +506,7 @@ frontend_dispatch_engine(int fd, short event, void *bula)
iface = get_iface_by_id(imsg_req_request.if_index);
if (iface != NULL) {
iface->xid = imsg_req_request.xid;
+ iface->ciaddr.s_addr =
imsg_req_request.ciaddr.s_addr;
iface->requested_ip.s_addr =
imsg_req_request.requested_ip.s_addr;
iface->server_identifier.s_addr =
@@ -887,7 +889,7 @@ bpf_receive(int fd, short events, void *arg)
ssize_t
build_packet(uint8_t message_type, char *if_name, uint32_t xid,
- struct ether_addr *hw_address, struct in_addr *requested_ip,
+ struct ether_addr *hw_address, struct in_addr *ciaddr, struct in_addr
*requested_ip,
struct in_addr *server_identifier)
{
static uint8_t dhcp_cookie[] = DHCP_COOKIE;
@@ -926,6 +928,7 @@ build_packet(uint8_t message_type, char *if_name, uint32_t
xid,
hdr->hops = 0;
hdr->xid = xid;
hdr->secs = 0;
+ hdr->ciaddr.s_addr = ciaddr->s_addr;
memcpy(hdr->chaddr, hw_address, sizeof(*hw_address));
p += sizeof(struct dhcp_hdr);
memcpy(p, dhcp_cookie, sizeof(dhcp_cookie));
@@ -1001,12 +1004,13 @@ send_discover(struct iface *iface)
return;
}
iface->send_discover = 0;
+ iface->ciaddr.s_addr = 0;
if_name = if_indextoname(iface->ifinfo.if_index, ifnamebuf);
log_debug("DHCPDISCOVER on %s", if_name == NULL ? "?" : if_name);
pkt_len = build_packet(DHCPDISCOVER, if_name, iface->xid,
- &iface->ifinfo.hw_address, &iface->requested_ip, NULL);
+ &iface->ifinfo.hw_address, &iface->ciaddr, &iface->requested_ip,
NULL);
bpf_send_packet(iface, dhcp_packet, pkt_len);
}
@@ -1020,7 +1024,7 @@ send_request(struct iface *iface)
log_debug("DHCPREQUEST on %s", if_name == NULL ? "?" : if_name);
pkt_len = build_packet(DHCPREQUEST, if_name, iface->xid,
- &iface->ifinfo.hw_address, &iface->requested_ip,
+ &iface->ifinfo.hw_address, &iface->ciaddr, &iface->requested_ip,
&iface->server_identifier);
if (iface->dhcp_server.s_addr != INADDR_ANY) {
if (udp_send_packet(iface, dhcp_packet, pkt_len) == -1)
--
I'm not entirely sure you are real.
