From 9fe8923419a954fedf8b6d1a6cc07b45f165c1ab Mon Sep 17 00:00:00 2001 From: pali <7141871+pali@users.noreply.github.com> Date: Tue, 26 Jan 2021 03:53:59 +0100 Subject: [PATCH] pppd: Fix enforcing peer IP address (#235) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit If peer address is specified and ipcp-accept-remote is not set then peer address is enforced. But there is bug in pppd which allows peer to not use supplied address when it reply with empty IPCP ConfReq. In this case pppd thinks that peer accepted its idea of remote/peer address even it is not truth. This issue can be reproduced by running pppd with arguments: ./pppd debug local noauth nolock nodetach asyncmap 0 default-asyncmap novj noaccomp nopcomp nodeflate nobsdcomp nomagic noipv6 noipdefault :10.1.0.1 pty "./pppd debug local noauth nolock nodetach asyncmap 0 default-asyncmap novj noaccomp nopcomp nodeflate nobsdcomp nomagic noipv6 nosendip nodefaultroute 10.0.0.1:10.0.0.2 notty" Which means that first pppd force usage of address 10.1.0.1 for peer and second pppd (peer) wants to use only address 10.0.0.1 for itself. First pppd see this communication rcvd [IPCP ConfReq id=0x64 ] sent [IPCP ConfNak id=0x64 ] rcvd [IPCP ConfReq id=0x65] sent [IPCP ConfAck id=0x65] local IP address 10.0.0.2 remote IP address 10.1.0.1 and thinks that peer (second pppd) accepted its idea of remote/peer address. After applying this patch first pppd correctly detects that peer refused its proposed peer address and therefore close connection. rcvd [IPCP ConfReq id=0x64 ] sent [IPCP ConfNak id=0x64 ] rcvd [IPCP ConfReq id=0x65] sent [IPCP ConfAck id=0x65] Peer refused to agree to his IP address Connect time 0.0 minutes. Sent 1024 bytes, received 1018 bytes. sent [IPCP TermReq id=0x3 "Refused his IP address"] Signed-off-by: Pali Rohár --- pppd/ipcp.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pppd/ipcp.c b/pppd/ipcp.c index 24452bf..fcf17b1 100644 --- a/pppd/ipcp.c +++ b/pppd/ipcp.c @@ -1760,6 +1760,12 @@ ipcp_up(fsm *f) /* * We must have a non-zero IP address for both ends of the link. */ + + if (wo->hisaddr && !wo->accept_remote && (!(ho->neg_addr || ho->old_addrs) || ho->hisaddr != wo->hisaddr)) { + error("Peer refused to agree to his IP address"); + ipcp_close(f->unit, "Refused his IP address"); + return; + } if (!ho->neg_addr && !ho->old_addrs) ho->hisaddr = wo->hisaddr; -- 2.39.2