[LEDE-DEV] [PATCH odpcpd] dhcpv6: allow creating new bindings on REBIND

[Hans Dedecker]

On Mon, Apr 10, 2017 at 11:08 AM, Lubomir Rintel <lkundrak at v3.sk> wrote:
> There might be unassigned IAs among the options the client sends when it
> issues a REBIND. In fact, that's how the ISC dhclient reqests new IA_PDs
> to be assigned. However, odhcpd would just return the IA_PDs back
> without assigning a prefix:
>
>   $ dhclient -6 -d -lf $PWD/leases eth0 -N
>   XMT: Forming Solicit, 0 ms elapsed.
>   XMT:  X-- IA_NA 00:3b:dd:9d
>   RCV: Advertise message on eth2 from fe80::5054:ff:feba:444e.
>   RCV:  X-- IA_NA 00:3b:dd:9d
>   ^C
>   $ dhclient -6 -d -lf $PWD/leases eth0 -N -P
>   XMT: Forming Rebind, 0 ms elapsed.
>   XMT:  X-- IA_NA 00:3b:dd:9d
>   XMT:  X-- IA_PD 00:3b:dd:9d
>   RCV: Reply message on eth2 from fe80::5054:ff:feba:444e.
>   RCV:  X-- IA_NA 00:3b:dd:9d
>   RCV:  X-- IA_PD 00:3b:dd:9d
>   RCV:  | !-- Status code of no prefix, IA_PD discarded.
>
> That is neither too useful, nor consistent with what the ISC dhcpcd does,
> nor does it seem correct. Notably, it breaks NetworkManager's IPv6
> connection sharing that utilizes the ISC dhclient and needs to request the
> prefixes as-needed on networks with OpenWRT.
>
> The attached patch moves the REBIND handling to the branch that is able
> to add new bindings. It then, perhaps unnecessarily, also allows
> obtaining new IA_NAs this way.
>
> (While the patch fixes the issue for me, I'm not too familiar with the
> odhcpd code base, so a careful review is needed.)
>
> Signed-off-by: Lubomir Rintel <lkundrak at v3.sk>
> ---
>  src/dhcpv6-ia.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/dhcpv6-ia.c b/src/dhcpv6-ia.c
> index 09022a4..c3eecfe 100644
> --- a/src/dhcpv6-ia.c
> +++ b/src/dhcpv6-ia.c
> @@ -1093,8 +1093,9 @@ ssize_t dhcpv6_handle_ia(uint8_t *buf, size_t buflen, struct interface *iface,
>                 uint16_t status = DHCPV6_STATUS_OK;
>                 if (a && a->managed_size < 0)
>                         return -1;
> -
> -               if (hdr->msg_type == DHCPV6_MSG_SOLICIT || hdr->msg_type == DHCPV6_MSG_REQUEST) {
> +               if (hdr->msg_type == DHCPV6_MSG_SOLICIT ||
> +                               hdr->msg_type == DHCPV6_MSG_REQUEST ||
> +                               hdr->msg_type == DHCPV6_MSG_REBIND) {
Creation of an assignment when a rebind is received is allowed for
IA_PD according to RFC6704 but this change will also create an
assignment for IA_NA which is not allowed to RFC3315. On top the IA_PD
bindings present in the rebind messages need to be checked if they're
valid which is already done in append_reply
>                         bool assigned = !!a;
>
>                         if (!a && !iface->no_dynamic_dhcp) {
> @@ -1189,8 +1190,7 @@ ssize_t dhcpv6_handle_ia(uint8_t *buf, size_t buflen, struct interface *iface,
>                         if (!a && hdr->msg_type != DHCPV6_MSG_REBIND) {
>                                 status = DHCPV6_STATUS_NOBINDING;
>                                 ia_response_len = append_reply(buf, buflen, status, ia, a, iface, false);
> -                       } else if (hdr->msg_type == DHCPV6_MSG_RENEW ||
> -                                       hdr->msg_type == DHCPV6_MSG_REBIND) {
> +                       } else if (hdr->msg_type == DHCPV6_MSG_RENEW) {
Correct patch would be to create here an assignment when receiving a
rebind with an IA_PD

Hans
>                                 ia_response_len = append_reply(buf, buflen, status, ia, a, iface, false);
>                                 if (a) {
>                                         a->flags |= OAF_BOUND;
> --
> 2.9.3
>