accel-ppp-xebd.git/accel-pppd/ppp, branch 1.12

ppp: use random LCP (and NCP) identifiers

2018-11-27T06:57:15+00:00

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

auth: remove .recv_conf_req from struct ppp_auth_handler_t

2018-11-27T06:56:56+00:00

This callback isn't used anymore. Let's remove it from all authentication backends. Signed-off-by: Guillaume Nault

lcp: reject Authentication-Protocol option in Configure-Request packets

2018-11-27T06:56:54+00:00

If we receive a Configure-Request packet, that means the peer wants us to authenticate to him. However, none of our authentication backends (PAP, CHAP and MSCHAP v1/v2) supports authenticating ourself to the peer. Therefore, the LCP negotiation completes, but we hang in the authentication phase because accel-ppp never sends any credential. We should reject the Authentication-Protocol option found in Configure-Request packets sent by the peer. This way, the peer knows that we won't authenticate to him. Then it's up to him to keep connecting without authentication from our side or to drop the connection. This doesn't change the way we request the peer to authenticate to us. That part of the negotiation is handled by Configure-Request packets that are sent by us (not those sent by the peer). In practice some PPP clients wouldn't connect with the previous behaviour, but are perfectly happy with their Authentication-Protocol option being rejected. They just resend their Configure-Request without requesting authentication from our side. Also, since the peer_auth field of struct auth_option_t is never set anymore, we can remove the conditionals in auth_recv_conf_nak() and auth_recv_conf_rej(). Signed-off-by: Guillaume Nault

ipcp: reject *-NBNS-Address if we have no value to propose

2018-11-12T14:00:40+00:00

If wins_opt->addr is NULL, then we have no address to propose (none defined in accel-ppp.conf and none provided by RADIUS). Currently, in that case, accel-ppp accepts and acks the address found in the peer's configuration request. But the peer would normally use the undefined IP address, so if we ack it, we explicitely tell the peer that 0.0.0.0 is the primary/secondary NBNS server. If the peer already knows a NBNS server IP address, it doesn't have to negociate it with accel-ppp. It can just use it directly, after it retrieved its own IP address. Therefore there is no need for accel-ppp to blindly accept addresses proposed by the peer. This patch rejects *-NBNS-Address if accel-ppp has no NBNS server to propose, making it explicit to the peer that its request can't be satisfied. Signed-off-by: Guillaume Nault

ipcp: fix uninitialised memory access when negociating *-NBNS-Address

2018-11-12T14:00:38+00:00

When handling the EV_WINS event, IPCP assumes that the ->wins1 and ->wins2 fields of the event structure are properly set. But that may not be the case. If only one of the MS-Primary-NBNS-Server or MS-Secondary-NBNS-Server RADIUS attributes was received, then only ->wins1 or ->wins2 is set, while the other keeps a non initialised value. This uninitialised value is then copied by ev_wins() and proposed to the peer when negociating the Primary-NBNS-Address or Secondary-NBNS-Address IPCP options. That leaks four bytes of the stack to the network and prevents using the values found in the [wins] section of accel-ppp.conf as fallback. Fix this by initialising the whole event structure in rad_proc_attrs(). Then, in ev_wins(), we can check if ->wins1 or ->wins2 is properly set before copying them. That allows to propery fallback to accel-ppp.conf values when one of the values was not provided by RADIUS. Signed-off-by: Guillaume Nault

ipcp: reject *-DNS-Address if we have no value to propose

2018-11-12T14:00:37+00:00

If dns_opt->addr is NULL, then we have no address to propose (none defined in accel-ppp.conf and none provided by RADIUS). Currently, in that case, accel-ppp accepts and acks the address found in the peer's configuration request. But the peer would normally use the undefined IP address, so if we ack it, we explicitely tell the peer that 0.0.0.0 is the primary/secondary DNS server. If the peer already knows a DNS server IP address, it doesn't have to negociate it with accel-ppp. It can just use it directly, after it retrieved its own IP address. Therefore there is no need for accel-ppp to blindly accept addresses proposed by the peer. This patch rejects *-DNS-Address if accel-ppp has no DNS server to propose, making it explicit to the peer that its request can't be satisfied. Signed-off-by: Guillaume Nault

ipcp: fix uninitialised memory access when negociating *-DNS-Address

2018-11-12T14:00:34+00:00

When handling the EV_DNS event, IPCP assumes that the ->dns1 and ->dns2 fields of the event structure are properly set. But that may not be the case. If only one of the MS-Primary-DNS-Server or MS-Secondary-DNS-Server RADIUS attributes was received, then only ->dns1 or ->dns2 is set, while the other keeps a non initialised value. This uninitialised value is then copied by ev_dns() and proposed to the peer when negociating the Primary-DNS-Address or Secondary-DNS-Address IPCP options. That leaks four bytes of the stack to the network and prevents using the values found in the [dns] section of accel-ppp.conf as fallback. Fix this by initialising the whole event structure in rad_proc_attrs(). Then, in ev_dns(), we can check if ->dns1 or ->dns2 is properly set before copying them. That allows to propery fallback to accel-ppp.conf values when one of the values was not provided by RADIUS. Signed-off-by: Guillaume Nault

ppp: make ppp_fsm.h self-contained

2018-10-26T03:34:34+00:00

We need to include to define 'uint16_t' and "triton.h" for 'struct triton_timer_t'. Also, let's include "ppp.h" so that we don't need a forward declaration for 'struct ppp_t'. Signed-off-by: Guillaume Nault

ppp: make ppp_auth.h self-contained

2018-10-26T03:34:34+00:00

We need to include to define 'uint8_t' and "ppp.h" for 'struct ppp_t'. Signed-off-by: Guillaume Nault

ppp: make ppp_lcp.h self-contained

2018-10-26T03:34:34+00:00

We need to include "ppp.h" to define 'struct ppp_layer_data_t' and 'struct ppp_handler_t'. Signed-off-by: Guillaume Nault