ppp: do not answer IPCP/IPV6CP ConfReq with TermAck while CCP negotiates

When the peer's IPCP (or IPV6CP) ConfReq arrives before CCP negotiation
has finished, delay_ack is set and send_conf_ack() answers with a TermAck
instead of the ConfAck. A TermAck is only a valid response to a TermReq,
and the trick relies on the peer retransmitting its ConfReq: conformant
peers recover only after their restart timer (3 seconds added to session
setup), while some clients (MikroTik RouterOS over L2TP, see issue #353)
treat it as a failure and drop the session.

Withhold the ConfAck instead and send it as soon as CCP settles. CCP now
notifies IPCP/IPV6CP when it comes up or gives up (passive); that clears
delay_ack, brings the layer up if the FSM is already Opened, and flushes
the withheld ConfAck. Nothing is sent to the peer while CCP is still in
progress, so IP data cannot flow before MPPE is set up.

Verified with pppoe + mschap-v2: with mppe=prefer and a client that does
not require MPPE, the peer's ConfReq used to be answered with a TermAck
and the session came up 3 seconds later; now the ConfAck is emitted right
after ccp_layer_started and the session comes up immediately. mppe=require
and mppe=deny sessions are unaffected.

5 files changed, 102 insertions, 6 deletions

diff --git a/accel-pppd/ppp/ppp_ccp.c b/accel-pppd/ppp/ppp_ccp.c
index f9e05e89..5eadb718 100644
--- a/accel-pppd/ppp/ppp_ccp.c
+++ b/accel-pppd/ppp/ppp_ccp.c
@@ -13,6 +13,8 @@
 
 #include "ppp.h"
 #include "ppp_ccp.h"
+#include "ppp_ipcp.h"
+#include "ppp_ipv6cp.h"
 
 #include "memdebug.h"
 
@@ -202,6 +204,9 @@ static void ccp_layer_up(struct ppp_fsm_t *fsm)
 			return;
 		}
 		ppp_layer_started(ccp->ppp, &ccp->ld);
+
+		ipcp_ccp_started(ccp->ppp);
+		ipv6cp_ccp_started(ccp->ppp);
 	}
 }
 
@@ -211,9 +216,12 @@ static void ccp_layer_finished(struct ppp_fsm_t *fsm)
 
 	log_ppp_debug("ccp_layer_finished\n");
 
-	if (!ccp->started)
+	if (!ccp->started) {
 		ppp_layer_passive(ccp->ppp, &ccp->ld);
-	else if (!ccp->ppp->ses.terminating)
+
+		ipcp_ccp_started(ccp->ppp);
+		ipv6cp_ccp_started(ccp->ppp);
+	} else if (!ccp->ppp->ses.terminating)
 		ap_session_terminate(&ccp->ppp->ses, TERM_USER_ERROR, 0);
 
 	fsm->fsm_state = FSM_Closed;
diff --git a/accel-pppd/ppp/ppp_ipcp.c b/accel-pppd/ppp/ppp_ipcp.c
index 416fba93..06766951 100644
--- a/accel-pppd/ppp/ppp_ipcp.c
+++ b/accel-pppd/ppp/ppp_ipcp.c
@@ -173,6 +173,9 @@ void ipcp_layer_free(struct ppp_layer_data_t *ld)
 	if (ipcp->timeout.tpd)
 		triton_timer_del(&ipcp->timeout);
 
+	if (ipcp->delay_ack_buf)
+		_free(ipcp->delay_ack_buf);
+
 	_free(ipcp);
 }
 
@@ -292,7 +295,11 @@ static void send_conf_ack(struct ppp_fsm_t *fsm)
 	struct ipcp_hdr_t *hdr = (struct ipcp_hdr_t*)ipcp->ppp->buf;
 
 	if (ipcp->delay_ack) {
-		send_term_ack(fsm);
+		/* CCP is still negotiating, withhold the ack until it settles */
+		if (ipcp->delay_ack_buf)
+			_free(ipcp->delay_ack_buf);
+		ipcp->delay_ack_buf = _malloc(ntohs(hdr->len) + 2);
+		memcpy(ipcp->delay_ack_buf, hdr, ntohs(hdr->len) + 2);
 		return;
 	}
 
@@ -671,7 +678,7 @@ static void ipcp_recv(struct ppp_handler_t*h)
 	}
 
 	hdr = (struct ipcp_hdr_t *)ipcp->ppp->buf;
-	if (ntohs(hdr->len) < PPP_HEADERLEN) {
+	if (ntohs(hdr->len) < PPP_HEADERLEN || ntohs(hdr->len) > ipcp->ppp->buf_size - 2) {
 		log_ppp_warn("IPCP: short packet received\n");
 		return;
 	}
@@ -781,6 +788,40 @@ int ipcp_option_register(struct ipcp_option_handler_t *h)
 	return 0;
 }
 
+void ipcp_ccp_started(struct ppp_t *ppp)
+{
+	struct ppp_layer_data_t *ld = ppp_find_layer_data(ppp, &ipcp_layer);
+	struct ppp_ipcp_t *ipcp;
+	struct ipcp_hdr_t *hdr;
+
+	if (!ld)
+		return;
+
+	ipcp = container_of(ld, typeof(*ipcp), ld);
+
+	if (!ipcp->delay_ack)
+		return;
+
+	ipcp->delay_ack = 0;
+
+	if (ipcp->fsm.fsm_state == FSM_Opened)
+		__ipcp_layer_up(ipcp);
+
+	if (!ipcp->delay_ack_buf)
+		return;
+
+	hdr = (struct ipcp_hdr_t *)ipcp->delay_ack_buf;
+	hdr->code = CONFACK;
+
+	if (conf_ppp_verbose)
+		log_ppp_info2("send [IPCP ConfAck id=%x]\n", hdr->id);
+
+	ppp_unit_send(ipcp->ppp, hdr, ntohs(hdr->len) + 2);
+
+	_free(ipcp->delay_ack_buf);
+	ipcp->delay_ack_buf = NULL;
+}
+
 struct ipcp_option_t *ipcp_find_option(struct ppp_t *ppp, struct ipcp_option_handler_t *h)
 {
 	struct ppp_ipcp_t *ipcp = container_of(ppp_find_layer_data(ppp, &ipcp_layer), typeof(*ipcp), ld);
diff --git a/accel-pppd/ppp/ppp_ipcp.h b/accel-pppd/ppp/ppp_ipcp.h
index 036f0a4d..8cd7790f 100644
--- a/accel-pppd/ppp/ppp_ipcp.h
+++ b/accel-pppd/ppp/ppp_ipcp.h
@@ -92,12 +92,15 @@ struct ppp_ipcp_t
 	struct list_head ropt_list; // last received ConfReq
 	int ropt_len;
 
+	void *delay_ack_buf; // ConfAck withheld until CCP finishes
+
 	int conf_req_len;
 	unsigned int starting:1;
 	unsigned int started:1;
 	unsigned int delay_ack:1;
 };
 
+void ipcp_ccp_started(struct ppp_t *ppp);
 int ipcp_option_register(struct ipcp_option_handler_t *h);
 struct ipcp_option_t *ipcp_find_option(struct ppp_t *ppp, struct ipcp_option_handler_t *h);
 
diff --git a/accel-pppd/ppp/ppp_ipv6cp.c b/accel-pppd/ppp/ppp_ipv6cp.c
index 7f278daa..034b2589 100644
--- a/accel-pppd/ppp/ppp_ipv6cp.c
+++ b/accel-pppd/ppp/ppp_ipv6cp.c
@@ -173,6 +173,9 @@ void ipv6cp_layer_free(struct ppp_layer_data_t *ld)
 	if (ipv6cp->timeout.tpd)
 		triton_timer_del(&ipv6cp->timeout);
 
+	if (ipv6cp->delay_ack_buf)
+		_free(ipv6cp->delay_ack_buf);
+
 	_free(ipv6cp);
 }
 
@@ -296,7 +299,11 @@ static void send_conf_ack(struct ppp_fsm_t *fsm)
 	struct ipv6cp_hdr_t *hdr = (struct ipv6cp_hdr_t*)ipv6cp->ppp->buf;
 
 	if (ipv6cp->delay_ack) {
-		send_term_ack(fsm);
+		/* CCP is still negotiating, withhold the ack until it settles */
+		if (ipv6cp->delay_ack_buf)
+			_free(ipv6cp->delay_ack_buf);
+		ipv6cp->delay_ack_buf = _malloc(ntohs(hdr->len) + 2);
+		memcpy(ipv6cp->delay_ack_buf, hdr, ntohs(hdr->len) + 2);
 		return;
 	}
 
@@ -675,7 +682,7 @@ static void ipv6cp_recv(struct ppp_handler_t*h)
 	}
 
 	hdr = (struct ipv6cp_hdr_t *)ipv6cp->ppp->buf;
-	if (ntohs(hdr->len) < PPP_HEADERLEN) {
+	if (ntohs(hdr->len) < PPP_HEADERLEN || ntohs(hdr->len) > ipv6cp->ppp->buf_size - 2) {
 		log_ppp_warn("IPV6CP: short packet received\n");
 		return;
 	}
@@ -790,6 +797,40 @@ int ipv6cp_option_register(struct ipv6cp_option_handler_t *h)
 	return 0;
 }
 
+void ipv6cp_ccp_started(struct ppp_t *ppp)
+{
+	struct ppp_layer_data_t *ld = ppp_find_layer_data(ppp, &ipv6cp_layer);
+	struct ppp_ipv6cp_t *ipv6cp;
+	struct ipv6cp_hdr_t *hdr;
+
+	if (!ld)
+		return;
+
+	ipv6cp = container_of(ld, typeof(*ipv6cp), ld);
+
+	if (!ipv6cp->delay_ack)
+		return;
+
+	ipv6cp->delay_ack = 0;
+
+	if (ipv6cp->fsm.fsm_state == FSM_Opened)
+		__ipv6cp_layer_up(ipv6cp);
+
+	if (!ipv6cp->delay_ack_buf)
+		return;
+
+	hdr = (struct ipv6cp_hdr_t *)ipv6cp->delay_ack_buf;
+	hdr->code = CONFACK;
+
+	if (conf_ppp_verbose)
+		log_ppp_info2("send [IPV6CP ConfAck id=%x]\n", hdr->id);
+
+	ppp_unit_send(ipv6cp->ppp, hdr, ntohs(hdr->len) + 2);
+
+	_free(ipv6cp->delay_ack_buf);
+	ipv6cp->delay_ack_buf = NULL;
+}
+
 struct ipv6cp_option_t *ipv6cp_find_option(struct ppp_t *ppp, struct ipv6cp_option_handler_t *h)
 {
 	struct ppp_ipv6cp_t *ipv6cp = container_of(ppp_find_layer_data(ppp, &ipv6cp_layer), typeof(*ipv6cp), ld);
diff --git a/accel-pppd/ppp/ppp_ipv6cp.h b/accel-pppd/ppp/ppp_ipv6cp.h
index 6f1789ec..31bdccf6 100644
--- a/accel-pppd/ppp/ppp_ipv6cp.h
+++ b/accel-pppd/ppp/ppp_ipv6cp.h
@@ -98,12 +98,15 @@ struct ppp_ipv6cp_t
 	struct list_head ropt_list; // last received ConfReq
 	int ropt_len;
 
+	void *delay_ack_buf; // ConfAck withheld until CCP finishes
+
 	int conf_req_len;
 	unsigned int starting:1;
 	unsigned int started:1;
 	unsigned int delay_ack:1;
 };
 
+void ipv6cp_ccp_started(struct ppp_t *ppp);
 int ipv6cp_option_register(struct ipv6cp_option_handler_t *h);
 
 #endif