Re: dhcpleased: default route with classless static routes option
On Sun 18/07/2021 10:38, Florian Obser wrote: > On 2021-07-18 01:02 +02, Bjorn Ketelaars wrote: > > On Sat 17/07/2021 17:12, Florian Obser wrote: > >> > >> > >> On 17 July 2021 13:16:59 CEST, Bjorn Ketelaars wrote: > >> >An inconsistency exists between dhclient(8) and dhcpleased(8) when > >> >receiving the Classless Static Routes option: dhcpleased creates a > >> >default route, while dhclient does not. > >> > > >> >If I'm not mistaken, the behaviour of dhclient is correct. From > >> >rfc3442: > >> >"If the DHCP server returns both a Classless Static Routes option and a > >> >Router option, the DHCP client MUST ignore the Router option." > >> > > >> > >> Correct. But you are fixing it in the wrong place. It doesn't say to > >> ignore a default route, which can be transmitted via classless static > >> routes option (it's 0/0). > > > > That makes sense. New diff... > > Not quite. This assumes that the router option comes first, a DHCP > server could send us something like this: > > DHO_ROUTERS > DHO_CLASSLESS_STATIC_ROUTES > DHO_ROUTERS > DHO_CLASSLESS_STATIC_ROUTES > > Which would be quite broken, but hey... > > How about this? Do you actually have a test case for this? I have a simple test case: DHCP server from ISP (more specific IPTV platform from KPN), which sends a single router option followed by a classless static router option. With your diff the router option is ignored. This fixes my issue, thank you!
Re: dhcpleased: default route with classless static routes option
On 2021-07-18 01:02 +02, Bjorn Ketelaars wrote:
> On Sat 17/07/2021 17:12, Florian Obser wrote:
>>
>>
>> On 17 July 2021 13:16:59 CEST, Bjorn Ketelaars wrote:
>> >An inconsistency exists between dhclient(8) and dhcpleased(8) when
>> >receiving the Classless Static Routes option: dhcpleased creates a
>> >default route, while dhclient does not.
>> >
>> >If I'm not mistaken, the behaviour of dhclient is correct. From
>> >rfc3442:
>> >"If the DHCP server returns both a Classless Static Routes option and a
>> >Router option, the DHCP client MUST ignore the Router option."
>> >
>>
>> Correct. But you are fixing it in the wrong place. It doesn't say to ignore
>> a default route, which can be transmitted via classless static routes option
>> (it's 0/0).
>
> That makes sense. New diff...
Not quite. This assumes that the router option comes first, a DHCP
server could send us something like this:
DHO_ROUTERS
DHO_CLASSLESS_STATIC_ROUTES
DHO_ROUTERS
DHO_CLASSLESS_STATIC_ROUTES
Which would be quite broken, but hey...
How about this? Do you actually have a test case for this?
dhcpd(8) correctly only hands me a routers option or classless static
routes option, never both.
diff --git engine.c engine.c
index ad8202c9a33..700d7c5ae80 100644
--- engine.c
+++ engine.c
@@ -613,7 +613,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct imsg_dhcp
*dhcp)
uint32_t rebinding_time = 0;
uint8_t *p, dho = DHO_PAD, dho_len;
uint8_t dhcp_message_type = 0;
- int routes_len = 0;
+ int routes_len = 0, routers = 0, csr = 0;
char from[sizeof("xx:xx:xx:xx:xx:xx")];
char to[sizeof("xx:xx:xx:xx:xx:xx")];
char hbuf_src[INET_ADDRSTRLEN];
@@ -836,19 +836,28 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
imsg_dhcp *dhcp)
if (dho_len % sizeof(routes[routes_len].gw) != 0)
goto wrong_length;
- while (routes_len < MAX_DHCP_ROUTES && dho_len > 0) {
- memcpy(&routes[routes_len].gw, p,
- sizeof(routes[routes_len].gw));
- if (log_getverbose() > 1) {
- log_debug("DHO_ROUTER: %s",
- inet_ntop(AF_INET,
- &routes[routes_len].gw, hbuf,
- sizeof(hbuf)));
+ /*
+* Ignore routers option if classless static routes
+* are present (RFC3442).
+*/
+ if (!csr) {
+ routers = 1;
+ while (routes_len < MAX_DHCP_ROUTES &&
+ dho_len > 0) {
+ memcpy(&routes[routes_len].gw, p,
+ sizeof(routes[routes_len].gw));
+ if (log_getverbose() > 1) {
+ log_debug("DHO_ROUTER: %s",
+ inet_ntop(AF_INET,
+ &routes[routes_len].gw,
+ hbuf, sizeof(hbuf)));
+ }
+ p += sizeof(routes[routes_len].gw);
+ rem -= sizeof(routes[routes_len].gw);
+ dho_len -=
+ sizeof(routes[routes_len].gw);
+ routes_len++;
}
- p += sizeof(routes[routes_len].gw);
- rem -= sizeof(routes[routes_len].gw);
- dho_len -= sizeof(routes[routes_len].gw);
- routes_len++;
}
if (dho_len != 0) {
/* ignore > MAX_DHCP_ROUTES routes */
@@ -939,6 +948,15 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
imsg_dhcp *dhcp)
case DHO_CLASSLESS_STATIC_ROUTES: {
int prefixlen, compressed_prefixlen;
+ csr = 1;
+ if (routers) {
+ /*
+* Ignore routers option if classless static
+* routes are present (RFC3442).
+*/
+ routers = 0;
+ routes_len = 0;
+ }
while (routes_len < MAX_DHCP_ROUTES && dho_len > 0
Re: dhcpleased: default route with classless static routes option
On Sat 17/07/2021 17:12, Florian Obser wrote:
>
>
> On 17 July 2021 13:16:59 CEST, Bjorn Ketelaars wrote:
> >An inconsistency exists between dhclient(8) and dhcpleased(8) when
> >receiving the Classless Static Routes option: dhcpleased creates a
> >default route, while dhclient does not.
> >
> >If I'm not mistaken, the behaviour of dhclient is correct. From
> >rfc3442:
> >"If the DHCP server returns both a Classless Static Routes option and a
> >Router option, the DHCP client MUST ignore the Router option."
> >
>
> Correct. But you are fixing it in the wrong place. It doesn't say to ignore a
> default route, which can be transmitted via classless static routes option
> (it's 0/0).
That makes sense. New diff...
diff --git sbin/dhcpleased/engine.c sbin/dhcpleased/engine.c
index ad8202c9a33..b3a86e68398 100644
--- sbin/dhcpleased/engine.c
+++ sbin/dhcpleased/engine.c
@@ -613,7 +613,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct imsg_dhcp
*dhcp)
uint32_t rebinding_time = 0;
uint8_t *p, dho = DHO_PAD, dho_len;
uint8_t dhcp_message_type = 0;
- int routes_len = 0;
+ int routes_len = 0, csr_len = 0, n;
char from[sizeof("xx:xx:xx:xx:xx:xx")];
char to[sizeof("xx:xx:xx:xx:xx:xx")];
char hbuf_src[INET_ADDRSTRLEN];
@@ -979,6 +979,7 @@ parse_dhcp(struct dhcpleased_iface *iface, struct imsg_dhcp
*dhcp)
dho_len -= sizeof(routes[routes_len].gw);
routes_len++;
+ csr_len++;
}
if (dho_len != 0) {
@@ -996,6 +997,20 @@ parse_dhcp(struct dhcpleased_iface *iface, struct
imsg_dhcp *dhcp)
}
}
+
+ /*
+ * RFC3442 states that if the DHCP server returns both a Classless Static
+ * Routes option and a Router option, the DHCP client MUST ignore the
+ * Router option.
+ */
+ if (csr_len > 0) {
+ log_warnx("%s: ignoring router option", __func__);
+ for (n = 0; n < routes_len - csr_len; n++) {
+ routes[n] = routes[n + 1];
+ routes_len--;
+ }
+ }
+
while (rem != 0) {
if (*p != DHO_PAD)
break;
Re: dhcpleased: default route with classless static routes option
On 17 July 2021 13:16:59 CEST, Bjorn Ketelaars wrote:
>An inconsistency exists between dhclient(8) and dhcpleased(8) when
>receiving the Classless Static Routes option: dhcpleased creates a
>default route, while dhclient does not.
>
>If I'm not mistaken, the behaviour of dhclient is correct. From
>rfc3442:
>"If the DHCP server returns both a Classless Static Routes option and a
>Router option, the DHCP client MUST ignore the Router option."
>
Correct. But you are fixing it in the wrong place. It doesn't say to ignore a
default route, which can be transmitted via classless static routes option
(it's 0/0).
You need to fix this in parse_dhcp in engine.c
>Comments?
>
>
>diff --git sbin/dhcpleased/dhcpleased.c sbin/dhcpleased/dhcpleased.c
>index 2157406e71a..93df9f55e7f 100644
>--- sbin/dhcpleased/dhcpleased.c
>+++ sbin/dhcpleased/dhcpleased.c
>@@ -819,7 +819,7 @@ configure_routes(uint8_t rtm_type, struct
>imsg_configure_interface *imsg)
> {
> struct sockaddr_in dst, mask, gw, ifa;
> in_addr_taddrnet, gwnet;
>- int i;
>+ int csr = 0, i;
>
> memset(&ifa, 0, sizeof(ifa));
> ifa.sin_family = AF_INET;
>@@ -840,6 +840,13 @@ configure_routes(uint8_t rtm_type, struct
>imsg_configure_interface *imsg)
>
> addrnet = imsg->addr.s_addr & imsg->mask.s_addr;
>
>+ for (i = 0; i < imsg->routes_len; i++) {
>+ if (imsg->routes[i].dst.s_addr != INADDR_ANY) {
>+ csr = 1;
>+ break;
>+ }
>+ }
>+
> for (i = 0; i < imsg->routes_len; i++) {
> dst.sin_addr.s_addr = imsg->routes[i].dst.s_addr;
> mask.sin_addr.s_addr = imsg->routes[i].mask.s_addr;
>@@ -872,6 +879,15 @@ configure_routes(uint8_t rtm_type, struct
>imsg_configure_interface *imsg)
> /* directly connected default */
> configure_route(rtm_type, imsg->if_index,
> imsg->rdomain, &dst, &mask, &gw, NULL, 0);
>+ } else if (csr) {
>+ /*
>+ * RFC3442 states that if the DHCP server returns
>+ * both a Classless Static Routes option and a
>+ * Router option, the DHCP client MUST ignore the
>+ * Router option.
>+ */
>+ log_warnx("%s: ignoring router option",
>+ __func__);
> } else {
> /* default route via gateway */
> configure_route(rtm_type, imsg->if_index,
--
Sent from a mobile device. Please excuse poor formatting.
Re: dhcpleased: default route with classless static routes option
On 2021/07/17 13:16, Bjorn Ketelaars wrote:
> An inconsistency exists between dhclient(8) and dhcpleased(8) when
> receiving the Classless Static Routes option: dhcpleased creates a
> default route, while dhclient does not.
>
> If I'm not mistaken, the behaviour of dhclient is correct. From rfc3442:
> "If the DHCP server returns both a Classless Static Routes option and a
> Router option, the DHCP client MUST ignore the Router option."
That is correct.
> Comments?
>
>
> diff --git sbin/dhcpleased/dhcpleased.c sbin/dhcpleased/dhcpleased.c
> index 2157406e71a..93df9f55e7f 100644
> --- sbin/dhcpleased/dhcpleased.c
> +++ sbin/dhcpleased/dhcpleased.c
> @@ -819,7 +819,7 @@ configure_routes(uint8_t rtm_type, struct
> imsg_configure_interface *imsg)
> {
> struct sockaddr_in dst, mask, gw, ifa;
> in_addr_taddrnet, gwnet;
> - int i;
> + int csr = 0, i;
>
> memset(&ifa, 0, sizeof(ifa));
> ifa.sin_family = AF_INET;
> @@ -840,6 +840,13 @@ configure_routes(uint8_t rtm_type, struct
> imsg_configure_interface *imsg)
>
> addrnet = imsg->addr.s_addr & imsg->mask.s_addr;
>
> + for (i = 0; i < imsg->routes_len; i++) {
> + if (imsg->routes[i].dst.s_addr != INADDR_ANY) {
> + csr = 1;
> + break;
> + }
> + }
> +
> for (i = 0; i < imsg->routes_len; i++) {
> dst.sin_addr.s_addr = imsg->routes[i].dst.s_addr;
> mask.sin_addr.s_addr = imsg->routes[i].mask.s_addr;
> @@ -872,6 +879,15 @@ configure_routes(uint8_t rtm_type, struct
> imsg_configure_interface *imsg)
> /* directly connected default */
> configure_route(rtm_type, imsg->if_index,
> imsg->rdomain, &dst, &mask, &gw, NULL, 0);
> + } else if (csr) {
> + /*
> + * RFC3442 states that if the DHCP server returns
> + * both a Classless Static Routes option and a
> + * Router option, the DHCP client MUST ignore the
> + * Router option.
> + */
> + log_warnx("%s: ignoring router option",
> + __func__);
> } else {
> /* default route via gateway */
> configure_route(rtm_type, imsg->if_index,
>
dhcpleased: default route with classless static routes option
An inconsistency exists between dhclient(8) and dhcpleased(8) when
receiving the Classless Static Routes option: dhcpleased creates a
default route, while dhclient does not.
If I'm not mistaken, the behaviour of dhclient is correct. From rfc3442:
"If the DHCP server returns both a Classless Static Routes option and a
Router option, the DHCP client MUST ignore the Router option."
Comments?
diff --git sbin/dhcpleased/dhcpleased.c sbin/dhcpleased/dhcpleased.c
index 2157406e71a..93df9f55e7f 100644
--- sbin/dhcpleased/dhcpleased.c
+++ sbin/dhcpleased/dhcpleased.c
@@ -819,7 +819,7 @@ configure_routes(uint8_t rtm_type, struct
imsg_configure_interface *imsg)
{
struct sockaddr_in dst, mask, gw, ifa;
in_addr_taddrnet, gwnet;
- int i;
+ int csr = 0, i;
memset(&ifa, 0, sizeof(ifa));
ifa.sin_family = AF_INET;
@@ -840,6 +840,13 @@ configure_routes(uint8_t rtm_type, struct
imsg_configure_interface *imsg)
addrnet = imsg->addr.s_addr & imsg->mask.s_addr;
+ for (i = 0; i < imsg->routes_len; i++) {
+ if (imsg->routes[i].dst.s_addr != INADDR_ANY) {
+ csr = 1;
+ break;
+ }
+ }
+
for (i = 0; i < imsg->routes_len; i++) {
dst.sin_addr.s_addr = imsg->routes[i].dst.s_addr;
mask.sin_addr.s_addr = imsg->routes[i].mask.s_addr;
@@ -872,6 +879,15 @@ configure_routes(uint8_t rtm_type, struct
imsg_configure_interface *imsg)
/* directly connected default */
configure_route(rtm_type, imsg->if_index,
imsg->rdomain, &dst, &mask, &gw, NULL, 0);
+ } else if (csr) {
+ /*
+ * RFC3442 states that if the DHCP server returns
+ * both a Classless Static Routes option and a
+ * Router option, the DHCP client MUST ignore the
+ * Router option.
+ */
+ log_warnx("%s: ignoring router option",
+ __func__);
} else {
/* default route via gateway */
configure_route(rtm_type, imsg->if_index,
