[Accel-ppp-users] [PATCH] ppp: use random LCP (and NCP) identifiers

[Guillaume N. <g....@al...>]

In DSL setups, it's common to have an intermediate equipment,
potentially managed by a different operator, between the two PPP
endpoints. In such setups, the client establishes a PPPoE or L2TP
session with the intermediate equipment. They perform LCP negotiation
and eventually get to the authentication phase. Based on the client's
username, the intermediate equipment then establishes another L2TP
session with the final PPP endpoint (accel-ppp). At this point, the
intermediate equipment forwards any PPP frame received on one side to
the other side, so that it becomes transparent to PPP frames.
Then accel-ppp starts an LCP negotiation again, performs
authentication, negotiates NCPs and finally forwards IP packets to and
from the client.

+--------+                        +--------------+                    +-----------+
| Client |------------------------| Intermediate |--------------------| accel-ppp |
|        |                        | equipment    |                    |           |
+--------+                        +--------------+                    +-----------+
          <-- First hop PPPoE  -->                <--  Second hop  -->
	      or L2TP session                         L2TP session

          <----------------- End to end PPP session ----------------->

Therefore, from the client point of view, two LCP negotiations occur.
LCP re-negotiation is explicitly handled by RFC 1661 and even
non-conforming PPP clients generally cope with this situation well
enough (as long as LCP re-negotiation occurs before the authentication
phase completes).

However, accel-ppp always starts its LCP negotiation with an identifier
set to 1. If the previous LCP negotiation also used identifier 1, then
some clients (at least MikroTik products) consider that the
Configure-Request sent by accel-ppp is part of the previous LCP
negotiation and refuse to return to link establishment phase as
mandated by section 3.4 of RFC 1661.

We can easily work around this problem by using random identifiers.
This maximises the chances that accel-ppp picks a different identifier
than the intermediate equipment and avoids falling into the MikroTik
problem. In case of bad luck and the chosen identifier is the same as
the one used for the original LCP negotiation, then PPP establishment
fails and the client tries to reconnect until the intermediate
equipment and accel-ppp pick up different numbers. So the connection
eventually succeeds.

The identifier is set in ppp_fsm_init(), so it also affects NCPs.
Therefore, IPCP and IPv6CP now also use random identifiers.

We need to define 'id' and 'recv_id' in struct ppp_fsm_t as uint8_t,
otherwise they could be chosen larger than 255 and comparing their
value with the 8-bits values found in received packets would fail (this
was generally not a problem when id was initially set to 1 and wouldn't
grow much).

Also, let's seed random() at startup, so that we don't end up with the
same sequences across restarts. This also benefits other users of
random(), like LCP magic numbers.

Signed-off-by: Guillaume Nault <g....@al...>
---
 accel-pppd/main.c        | 2 ++
 accel-pppd/ppp/ppp_fsm.c | 2 +-
 accel-pppd/ppp/ppp_fsm.h | 4 ++--
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/accel-pppd/main.c b/accel-pppd/main.c
index 2992026c..ecffb7c1 100644
--- a/accel-pppd/main.c
+++ b/accel-pppd/main.c
@@ -283,6 +283,8 @@ int main(int _argc, char **_argv)
 			sleep(1);
 	}
 
+	srandom(time(NULL));
+
 	if (triton_init(conf_file))
 		_exit(EXIT_FAILURE);
 
diff --git a/accel-pppd/ppp/ppp_fsm.c b/accel-pppd/ppp/ppp_fsm.c
index bb45d5cb..c884143c 100644
--- a/accel-pppd/ppp/ppp_fsm.c
+++ b/accel-pppd/ppp/ppp_fsm.c
@@ -36,7 +36,7 @@ void ppp_fsm_init(struct ppp_fsm_t *layer)
 	layer->max_configure = conf_max_configure;
 	layer->max_failure = conf_max_failure;
 
-	layer->id = 1;
+	layer->id = random();
 }
 void ppp_fsm_free(struct ppp_fsm_t *layer)
 {
diff --git a/accel-pppd/ppp/ppp_fsm.h b/accel-pppd/ppp/ppp_fsm.h
index 6208d9ad..60b8aadb 100644
--- a/accel-pppd/ppp/ppp_fsm.h
+++ b/accel-pppd/ppp/ppp_fsm.h
@@ -36,8 +36,8 @@ struct ppp_fsm_t
 	int max_failure;
 	int conf_failure;
 
-	int id;
-	int recv_id;
+	uint8_t id;
+	uint8_t recv_id;
 
 	//fsm handling
 	void (*layer_up)(struct ppp_fsm_t*);
-- 
2.20.0.rc1