On Fri, Apr 09, 2021 at 09:23:50AM +0200, Martijn van Duren wrote:
> Hello tech@,
>
> I´m currently faced with a Comtrend VI-3223u router, which sends out
> dhcp leases with:
> DHO_DHCP_LEASE_TIME 86400s
> DHO_DHCP_RENEWAL_TIME 43200s
> DHO_DHCP_REBINDING_TIME 86400s
>
> This trips up dhcpleased and I´m stuck without an IP-address
> (unless I move back to dhclient). I know that RFC2131 states that
> T2 MUST be smaller than lease expire, but chances this device gets
> fixed is small.
>
> The diff below fixes the issue for me by rescaling both T1 and T2
> if they are invalid. This would also help if there is some dhcp
> server which sets T1 to 0 and T2 to < lease_time / 2.
>
> Is something like this acceptable or should I scream at the
> wall^Wmanufacturer?
Yeah, no. dhcpleased has to be increadibly lenient in what it's
willing to accept. This stuff is around for nearly 3 decades.
Shit is not going to get fixed server side.
I think it would be better (and less ugly) to treat invalid values as
if they had not been set. Could you pull the two checks up before
if(renewal_time == 0) and do something like this:
if (renewal_time >= rebinding_time) {
log_warn(...)
renewal_time = 0;
}
if (rebinding_time >= lease_time) {
log_warn(...)
rebinding_time = 0;
}
>
> martijn@
>
> Index: engine.c
> ===================================================================
> RCS file: /cvs/src/sbin/dhcpleased/engine.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 engine.c
> --- engine.c 22 Mar 2021 15:34:07 -0000 1.11
> +++ engine.c 9 Apr 2021 07:20:29 -0000
> @@ -977,17 +977,19 @@ parse_dhcp(struct dhcpleased_iface *ifac
> if (rebinding_time == 0)
> rebinding_time = lease_time - (lease_time / 8);
>
> - if (renewal_time >= rebinding_time) {
> - log_warnx("%s: renewal_time >= rebinding_time "
> - "(%u >= %u) from %s", __func__, renewal_time,
> - rebinding_time, from);
> - return;
> - }
> if (rebinding_time >= lease_time) {
> log_warnx("%s: rebinding_time >= lease_time"
> - "(%u >= %u) from %s", __func__, rebinding_time,
> - lease_time, from);
> - return;
> + "(%u >= %u) from %s: adjusted to %u", __func__,
> + rebinding_time, lease_time, from,
> + lease_time - (lease_time / 8));
> + rebinding_time = lease_time - (lease_time / 8);
> + }
> + if (renewal_time >= rebinding_time) {
> + log_warnx("%s: renewal_time >= rebinding_time "
> + "(%u >= %u) from %s: adjusted to %u", __func__,
> + renewal_time, rebinding_time, from,
> + rebinding_time / 7 * 8 / 2);
> + renewal_time = rebinding_time / 7 * 8 / 2;
> }
> clock_gettime(CLOCK_MONOTONIC, &iface->request_time);
> iface->server_identifier.s_addr = server_identifier.s_addr;
>
>
--
I'm not entirely sure you are real.
