dhcpcd-discuss

Hi Mohan

Does the attached patch fix it for you?
I won't be able to test it for a while myself.

Roy

On 27/07/2019 03:58, Kannekanti, Mohan wrote:
> Hi Roy,
>
>
> Thanks for the quick reply.
>
>
> I read the following from RFC8415 section 18.2.10.1 
> <https://tools.ietf.org/html/rfc8415#section-18.2.10.1>
>
>
> When the client receives a Reply message in response to a Renew or Rebind message, the client:
>
>     -  Sends a Request message to the server that responded if any of the
>        IAs in the Reply message contain the NoBinding status code.  The
>        client places IA options in this message for all IAs.  The client
>        continues to use other bindings for which the server did not
>        return an error.
>
>
> This suggests that the dhcp client to send a Request when any of the IAs 
> in the Reply contain the NoBinding status code.
>
> I believe the expectation of CDRouter is still valid as per above text. 
> Isn't it?
>
>
> Thanks,
>
> Mohan.
>
> ------------------------------------------------------------------------
> *From:* Roy Marples <roy@xxxxxxxxxxxx>
> *Sent:* Friday, July 26, 2019 6:36:12 PM
> *To:* Kannekanti, Mohan <mkannekanti@xxxxxxxx>; 
> dhcpcd-discuss@xxxxxxxxxxxx <dhcpcd-discuss@xxxxxxxxxxxx>
> *Subject:* Re: dhcpcd is not sending REQUEST when NoBinding was sent 
> during IA_PD RENEW
> ------------------------------------------------------------------------
> NOTICE: This email was received from an EXTERNAL sender
> ------------------------------------------------------------------------
>
> Hi Kannekanti
>
> On 26/07/2019 22:49, Kannekanti, Mohan wrote:
>  > We are using version 7.2.2.
>  >
>  > The CDRouter test suite reported a NoBinding testcase failure for IA_PD.
>  > Below are the steps that are followed.
>  >
>  > /    step 1. Wait for DHCPv6 client's current prefix binding T1 timer to
>  > expire
>  >     step 2. Verify DHCPv6 client sends DHCPv6 Renew
>  >     step 3. Verify Renew contains IA_PD option (26) for same address 
> prefix
>  >     step 4. Send valid DHCPv6 Reply with NoBinding status code (3)
>  >     step 5. Verify DHCPv6 client sends Request message
>  >     step 6. Verify Request contains IA_PD option (26) for same address
>  > prefix
>  >
>  >     Reference: IETF RFC 3633 Section 12.1 "Requesting router behavior"/
>  >
>  > The test failing at "step 6". It is expecting that dhcp client should
>  > send a REQUEST (as per section 12.1 and 18.1.8 in RFC3315) but the
>  > client continue to send RENEW.
>  > In the attached packet capture, you can find the Reply (Packet #2) from
>  > the server with NoBinding status for IA_PD followed by Renew packets at
>  > T1 interval.
>
> RFC 3633
> 12.1. Requesting router behavior
>
> The requesting router uses a Request message to populate IA_PDs with
> prefixes. The requesting router includes one or more IA_PD options
> in the Request message. The delegating router then returns the
> prefixes for the IA_PDs to the requesting router in IA_PD options in
> a Reply message.
>
> RFC 3633 has been obsoleted by RFC 8415:
> 18.2.4. Creation and Transmission of Renew Messages
>
> To extend the preferred and valid lifetimes for the leases assigned
> to the IAs and obtain new addresses or delegated prefixes for IAs,
> the client sends a Renew message to the server from which the leases
> were obtained; the Renew message includes IA options for the IAs
> whose lease lifetimes are to be extended. The client includes IA
> Address options (see Section 21.6) within IA_NA (see Section 21.4)
> and IA_TA (see Section 21.5) options for the addresses assigned to
> the IAs. The client includes IA Prefix options (see Section 21.22)
> within IA_PD options (see Section 21.21) for the delegated prefixes
> assigned to the IAs.
>
>
>  > When I checked the code, looks like dhcp6_validatelease() is expecting
>  > dhcp6_findia() to return "-1" to invalidate the lease and fallback to
>  > discover state.
>  > But, in our case dhcp6_findia() is not returning "-1" thus the client
>  > continued to send RENEW.
>  >
>  > dhcp6_findia(...) {
>  >     ...
>  >     while (l > sizeof(o)) {
>  >         ....
>  >         l -= sizeof(o) + o.len;
>  >         ....
>  >         if (dhcp6_checkstatusok(ifp, NULL, p, o.len) == -1) {
>  >             e = 1;
>  >             continue;
>  >         }
>  >         ....
>  >         i++;
>  >     }
>  >     ...
>  > *    if (i == 0 && e)
>  >         return -1;*
>  >     return i;
>  > }
>  >
>  > When we receive a valid IA_NA and invalid IA_PD (NoBinding), 'i' value
>  > is 1 and 'e' value is '1'.
>  >
>  > Are we missing anything here? is it bug in dhcpcd?
>
> You are missing the the IA_NA which is still valid, so while e is 1 due
> the the lack of binding for the PD, i is 1 because the NA is valid, thus
> no error is returned by this function. So dhcpcd RENEWS.
>
> You can see that it's renewing at 45 second intervals which matches the
> T1 timer of the NA.
> Thanks to the text of RFC 8415 you can request new addresses for
> assignment within a RENEW.
>
> One of the goals of RFC 8415 was to make the client simpler. If we take
> RFC 3315 and 3633 at their literals, we would need to have seperate
> state machines for IA_PD and IA_NA/TA, but this is no longer the case.
>
> I don't believe this is a bug in dhcpcd.
>
> Roy
>
>
> ------------------------------------------------------------------------
> Notice: This e-mail together with any attachments may contain 
> information of Ribbon Communications Inc. that is confidential and/or 
> proprietary for the sole use of the intended recipient. Any review, 
> disclosure, reliance or distribution by others or forwarding without 
> express permission is strictly prohibited. If you are not the intended 
> recipient, please notify the sender immediately and then delete all 
> copies, including any attachments.
> ------------------------------------------------------------------------
diff --git a/src/dhcp6.c b/src/dhcp6.c
index 337a8367..c6fd7af1 100644
--- a/src/dhcp6.c
+++ b/src/dhcp6.c
@@ -1938,6 +1938,7 @@ dhcp6_checkstatusok(const struct interface *ifp,
 	if ((opt = f(farg, len, D6_OPTION_STATUS_CODE, &opt_len)) == NULL) {
 		//logdebugx("%s: no status", ifp->name);
 		state->lerror = 0;
+		errno = ESRCH;
 		return 0;
 	}
 
@@ -1949,7 +1950,8 @@ dhcp6_checkstatusok(const struct interface *ifp,
 	code = ntohs(code);
 	if (code == D6_STATUS_OK) {
 		state->lerror = 0;
-		return 1;
+		errno = 0;
+		return 0;
 	}
 
 	/* Anything after the code is a message. */
@@ -1980,7 +1982,8 @@ dhcp6_checkstatusok(const struct interface *ifp,
 	logfunc("%s: DHCPv6 REPLY: %s", ifp->name, status);
 	free(sbuf);
 	state->lerror = code;
-	return -1;
+	errno = 0;
+	return (int)code;
 }
 
 const struct ipv6_addr *
@@ -2227,7 +2230,7 @@ dhcp6_findia(struct interface *ifp, struct dhcp6_message *m, size_t l,
 	struct dhcp6_option o;
 	uint8_t *d, *p;
 	struct dhcp6_ia_na ia;
-	int i, e;
+	int i, e, error;
 	size_t j;
 	uint16_t nl;
 	uint8_t iaid[4];
@@ -2316,7 +2319,9 @@ dhcp6_findia(struct interface *ifp, struct dhcp6_message *m, size_t l,
 			}
 		} else
 			ia.t1 = ia.t2 = 0; /* appease gcc */
-		if (dhcp6_checkstatusok(ifp, NULL, p, o.len) == -1) {
+		if ((error = dhcp6_checkstatusok(ifp, NULL, p, o.len)) != 0) {
+			if (error == D6_STATUS_NOBINDING)
+				state->has_no_binding = true;
 			e = 1;
 			continue;
 		}
@@ -2417,7 +2422,7 @@ dhcp6_validatelease(struct interface *ifp,
     const char *sfrom, const struct timespec *acquired)
 {
 	struct dhcp6_state *state;
-	int ok, nia;
+	int nia, ok_errno;
 	struct timespec aq;
 
 	if (len <= sizeof(*m)) {
@@ -2426,8 +2431,10 @@ dhcp6_validatelease(struct interface *ifp,
 	}
 
 	state = D6_STATE(ifp);
-	if ((ok = dhcp6_checkstatusok(ifp, m, NULL, len) == -1))
+	errno = 0;
+	if (dhcp6_checkstatusok(ifp, m, NULL, len) != 0)
 		return -1;
+	ok_errno = errno;
 
 	state->renew = state->rebind = state->expire = 0;
 	state->lowpl = ND6_INFINITE_LIFETIME;
@@ -2435,9 +2442,10 @@ dhcp6_validatelease(struct interface *ifp,
 		clock_gettime(CLOCK_MONOTONIC, &aq);
 		acquired = &aq;
 	}
+	state->has_no_binding = false;
 	nia = dhcp6_findia(ifp, m, len, sfrom, acquired);
 	if (nia == 0) {
-		if (state->state != DH6S_CONFIRM && ok != 1) {
+		if (state->state != DH6S_CONFIRM && ok_errno != 0) {
 			logerrx("%s: no useable IA found in lease", ifp->name);
 			return -1;
 		}
@@ -2447,6 +2455,7 @@ dhcp6_validatelease(struct interface *ifp,
 		 * IA's must have existed here otherwise we would
 		 * have rejected it earlier. */
 		assert(state->new != NULL && state->new_len != 0);
+		state->has_no_binding = false;
 		nia = dhcp6_findia(ifp, state->new, state->new_len,
 		    sfrom, acquired);
 	}
@@ -3269,7 +3278,7 @@ dhcp6_recvif(struct interface *ifp, const char *sfrom,
 	case DHCP6_REPLY:
 		switch(state->state) {
 		case DH6S_INFORM:
-			if (dhcp6_checkstatusok(ifp, r, NULL, len) == -1)
+			if (dhcp6_checkstatusok(ifp, r, NULL, len) != 0)
 				return;
 			break;
 		case DH6S_CONFIRM:
@@ -3317,6 +3326,14 @@ dhcp6_recvif(struct interface *ifp, const char *sfrom,
 					dhcp6_startdiscover(ifp);
 				return;
 			}
+			/* RFC8415 18.2.10.1 */
+			if ((state->state == DH6S_RENEW ||
+			    state->state == DH6S_REBIND) &&
+			    state->has_no_binding)
+			{
+				dhcp6_startrequest(ifp);
+				return;
+			}
 			if (state->state == DH6S_DISCOVER)
 				state->state = DH6S_REQUEST;
 			break;
diff --git a/src/dhcp6.h b/src/dhcp6.h
index 42b89b06..ce6ad8d5 100644
--- a/src/dhcp6.h
+++ b/src/dhcp6.h
@@ -208,6 +208,7 @@ struct dhcp6_state {
 	char leasefile[sizeof(LEASEFILE6) + IF_NAMESIZE + (IF_SSIDLEN * 4) +3];
 	const char *reason;
 	uint16_t lerror; /* Last error received from DHCPv6 reply. */
+	bool has_no_binding;
 	struct authstate auth;
 };