--- Begin Message ---
Package: ppp
Version: 2.4.4rel-9
Severity: normal
Tags: patch
Sorry for this long report, it got longer than intended.
When pppd is configured with the defaultroute option and there is a
default route without a gateway in the routing table, then pppd adds a
new default route to the routing table, but without replacing the old
default route. Thus, this results in there being two default routes in
the routing table, with the one created by pppd potentially overriding
the original one. Also, when the PPP connection is brought down, pppd
deletes both of these routes, so there is no default route any
more. It is my understanding that pppd should not add a new default
route when there already is one (unless the replacedefaultroute option
is specified), and this is also how it behaves when the existing
default route goes through a gateway.
The following sequence of commands indicate the problem. For the
purpose of demonstrating the problem, the PPP connection is tunneled
using netcat and a dummy default route without a gateway is created
through the loopback interface (if the usual default route does not go
through a gateway, that can be used instead).
$ sudo route add default dev lo
$ route -n
Kernel IP routing table
Destination Gateway Genmask Flags Metric Ref Use Iface
0.0.0.0 0.0.0.0 0.0.0.0 U 0 0 0 lo
$ sudo pppd 192.168.100.1:192.168.100.2 noauth passive pty "nc -l -p 2020"
$ sudo pppd noauth passive defaultroute pty "nc 127.0.0.1 2020"
$ route -n
Kernel IP routing table
Destination Gateway Genmask Flags Metric Ref Use Iface
192.168.100.1 0.0.0.0 255.255.255.255 UH 0 0 0 ppp1
192.168.100.2 0.0.0.0 255.255.255.255 UH 0 0 0 ppp0
0.0.0.0 0.0.0.0 0.0.0.0 U 0 0 0 ppp1
0.0.0.0 0.0.0.0 0.0.0.0 U 0 0 0 lo
$ sudo kill 11590 # The pid of either of the pppds.
$ route -n
Kernel IP routing table
Destination Gateway Genmask Flags Metric Ref Use Iface
This behavior happens regardless of whether the replacedefaultroute
option has been specified, but seems to be related with the patch
cifdefroute.dif, which adds the support for this option. The problem
seems to be in the function sifdefaultroute, where the upstream
version of pppd has the following code:
if (defaultroute_exists(&rt) && strcmp(rt.rt_dev, ifname) != 0) {
if (rt.rt_flags & RTF_GATEWAY)
error("not replacing existing default route via %I",
SIN_ADDR(rt.rt_gateway));
else
error("not replacing existing default route through %s",
rt.rt_dev);
return 0;
}
While the patched version has this:
} else if (defaultroute_exists(&old_def_rt) &&
strcmp(old_def_rt.rt_dev, ifname) != 0) {
/* We did not yet replace an existing default route, let's
check if we should save and replace a default route: */
if (old_def_rt.rt_flags & RTF_GATEWAY) {
if (!replace) {
error("not replacing existing default route via %I",
SIN_ADDR(old_def_rt.rt_gateway));
return 0;
} else {
/* we need to copy rt_dev because we need it permanent too: */
char *tmp_dev = malloc(strlen(old_def_rt.rt_dev) + 1);
strcpy(tmp_dev, old_def_rt.rt_dev);
old_def_rt.rt_dev = tmp_dev;
notice("replacing old default route to %s [%I]",
old_def_rt.rt_dev, SIN_ADDR(old_def_rt.rt_gateway));
default_rt_repl_rest = 1;
del_rt = &old_def_rt;
}
} else
error("not replacing existing default route through %s",
old_def_rt.rt_dev);
}
Note that when there is a default route and it doesn't have a gateway
(old_def_rt.rt_flags & RTF_GATEWAY is false), the patched version will
print an error message, but continues the execution of the function,
where another default route is added to the routing table.
I've attached a slightly modified version of the patch, which retains
the upstream behavior when the replacedefaultroute option is not in
effect (replace is false), but which also includes the code necessary
to flag and save the original default route when the replacedefaultroute
option is in effect (replace is true), so that the code becomes like
this:
} else if (defaultroute_exists(&old_def_rt) &&
strcmp(old_def_rt.rt_dev, ifname) != 0) {
/* We did not yet replace an existing default route, let's
check if we should save and replace a default route: */
if (!replace) {
if (old_def_rt.rt_flags & RTF_GATEWAY)
error("not replacing existing default route via %I",
SIN_ADDR(old_def_rt.rt_gateway));
else
error("not replacing existing default route through %s",
old_def_rt.rt_dev);
return 0;
}
/* we need to copy rt_dev because we need it permanent too: */
char *tmp_dev = malloc(strlen(old_def_rt.rt_dev) + 1);
strcpy(tmp_dev, old_def_rt.rt_dev);
old_def_rt.rt_dev = tmp_dev;
notice("replacing old default route to %s [%I]",
old_def_rt.rt_dev, SIN_ADDR(old_def_rt.rt_gateway));
default_rt_repl_rest = 1;
del_rt = &old_def_rt;
}
-- System Information:
Debian Release: lenny/sid
APT prefers unstable
APT policy: (990, 'unstable'), (500, 'testing'), (500, 'stable')
Architecture: i386 (i686)
Kernel: Linux 2.6.19.3-20070213 (SMP w/2 CPU cores; PREEMPT)
Locale: LANG=en_US, LC_CTYPE=en_US (charmap=ISO-8859-1)
Shell: /bin/sh linked to /bin/bash
Versions of packages ppp depends on:
ii libc6 2.5-4 GNU C Library: Shared libraries
ii libpam-modules 0.79-4 Pluggable Authentication Modules f
ii libpam-runtime 0.79-4 Runtime support for the PAM librar
ii libpam0g 0.79-4 Pluggable Authentication Modules l
ii libpcap0.8 0.9.5-1 System interface for user-level pa
ii netbase 4.29 Basic TCP/IP networking system
ii procps 1:3.2.7-3 /proc file system utilities
ppp recommends no packages.
-- no debconf information
diff -ruNp ppp-2.4.4.orig/pppd/ipcp.c ppp-2.4.4/pppd/ipcp.c
--- ppp-2.4.4.orig/pppd/ipcp.c 2005-08-25 23:59:34.000000000 +0000
+++ ppp-2.4.4/pppd/ipcp.c 2007-05-18 19:57:57.000000000 +0000
@@ -197,6 +197,14 @@ static option_t ipcp_option_list[] = {
"disable defaultroute option", OPT_ALIAS | OPT_A2CLR,
&ipcp_wantoptions[0].default_route },
+ { "replacedefaultroute", o_bool,
+ &ipcp_wantoptions[0].replace_default_route,
+ "Replace default route", 1
+ },
+ { "noreplacedefaultroute", o_bool,
+ &ipcp_allowoptions[0].replace_default_route,
+ "Never replace default route", OPT_A2COPY,
+ &ipcp_wantoptions[0].replace_default_route },
{ "proxyarp", o_bool, &ipcp_wantoptions[0].proxy_arp,
"Add proxy ARP entry", OPT_ENABLE|1, &ipcp_allowoptions[0].proxy_arp },
{ "noproxyarp", o_bool, &ipcp_allowoptions[0].proxy_arp,
@@ -263,7 +271,7 @@ struct protent ipcp_protent = {
ip_active_pkt
};
-static void ipcp_clear_addrs __P((int, u_int32_t, u_int32_t));
+static void ipcp_clear_addrs __P((int, u_int32_t, u_int32_t, bool));
static void ipcp_script __P((char *, int)); /* Run an up/down script */
static void ipcp_script_done __P((void *));
@@ -1660,7 +1668,8 @@ ip_demand_conf(u)
if (!sifnpmode(u, PPP_IP, NPMODE_QUEUE))
return 0;
if (wo->default_route)
- if (sifdefaultroute(u, wo->ouraddr, wo->hisaddr))
+ if (sifdefaultroute(u, wo->ouraddr, wo->hisaddr,
+ wo->replace_default_route))
default_route_set[u] = 1;
if (wo->proxy_arp)
if (sifproxyarp(u, wo->hisaddr))
@@ -1742,7 +1751,8 @@ ipcp_up(f)
*/
if (demand) {
if (go->ouraddr != wo->ouraddr || ho->hisaddr != wo->hisaddr) {
- ipcp_clear_addrs(f->unit, wo->ouraddr, wo->hisaddr);
+ ipcp_clear_addrs(f->unit, wo->ouraddr, wo->hisaddr,
+ wo->replace_default_route);
if (go->ouraddr != wo->ouraddr) {
warn("Local IP address changed to %I", go->ouraddr);
script_setenv("OLDIPLOCAL", ip_ntoa(wo->ouraddr), 0);
@@ -1767,7 +1777,8 @@ ipcp_up(f)
/* assign a default route through the interface if required */
if (ipcp_wantoptions[f->unit].default_route)
- if (sifdefaultroute(f->unit, go->ouraddr, ho->hisaddr))
+ if (sifdefaultroute(f->unit, go->ouraddr, ho->hisaddr,
+ wo->replace_default_route))
default_route_set[f->unit] = 1;
/* Make a proxy ARP entry if requested. */
@@ -1817,7 +1828,8 @@ ipcp_up(f)
/* assign a default route through the interface if required */
if (ipcp_wantoptions[f->unit].default_route)
- if (sifdefaultroute(f->unit, go->ouraddr, ho->hisaddr))
+ if (sifdefaultroute(f->unit, go->ouraddr, ho->hisaddr,
+ wo->replace_default_route))
default_route_set[f->unit] = 1;
/* Make a proxy ARP entry if requested. */
@@ -1894,7 +1906,7 @@ ipcp_down(f)
sifnpmode(f->unit, PPP_IP, NPMODE_DROP);
sifdown(f->unit);
ipcp_clear_addrs(f->unit, ipcp_gotoptions[f->unit].ouraddr,
- ipcp_hisoptions[f->unit].hisaddr);
+ ipcp_hisoptions[f->unit].hisaddr, 0);
}
/* Execute the ip-down script */
@@ -1910,16 +1922,25 @@ ipcp_down(f)
* proxy arp entries, etc.
*/
static void
-ipcp_clear_addrs(unit, ouraddr, hisaddr)
+ipcp_clear_addrs(unit, ouraddr, hisaddr, replacedefaultroute)
int unit;
u_int32_t ouraddr; /* local address */
u_int32_t hisaddr; /* remote address */
+ bool replacedefaultroute;
{
if (proxy_arp_set[unit]) {
cifproxyarp(unit, hisaddr);
proxy_arp_set[unit] = 0;
}
- if (default_route_set[unit]) {
+ /* If replacedefaultroute, sifdefaultroute will be called soon
+ * with replacedefaultroute set and that will overwrite the current
+ * default route. This is the case only when doing demand, otherwise
+ * during demand, this cifdefaultroute would restore the old default
+ * route which is not what we want in this case. In the non-demand
+ * case, we'll delete the default route and restore the old if there
+ * is one saved by an sifdefaultroute with replacedefaultroute.
+ */
+ if (!replacedefaultroute && default_route_set[unit]) {
cifdefaultroute(unit, ouraddr, hisaddr);
default_route_set[unit] = 0;
}
diff -ruNp ppp-2.4.4.orig/pppd/ipcp.h ppp-2.4.4/pppd/ipcp.h
--- ppp-2.4.4.orig/pppd/ipcp.h 2002-12-04 23:03:32.000000000 +0000
+++ ppp-2.4.4/pppd/ipcp.h 2007-05-18 19:57:57.000000000 +0000
@@ -70,6 +70,7 @@ typedef struct ipcp_options {
bool old_addrs; /* Use old (IP-Addresses) option? */
bool req_addr; /* Ask peer to send IP address? */
bool default_route; /* Assign default route through interface? */
+ bool replace_default_route; /* Replace default route through interface? */
bool proxy_arp; /* Make proxy ARP entry for peer? */
bool neg_vj; /* Van Jacobson Compression? */
bool old_vj; /* use old (short) form of VJ option? */
diff -ruNp ppp-2.4.4.orig/pppd/pppd.8 ppp-2.4.4/pppd/pppd.8
--- ppp-2.4.4.orig/pppd/pppd.8 2006-06-16 00:01:23.000000000 +0000
+++ ppp-2.4.4/pppd/pppd.8 2007-05-18 19:57:57.000000000 +0000
@@ -121,6 +121,11 @@ the gateway, when IPCP negotiation is su
This entry is removed when the PPP connection is broken. This option
is privileged if the \fInodefaultroute\fR option has been specified.
.TP
+.B replacedefaultroute
+This option is a flag to the defaultroute option. If defaultroute is
+set and this flag is also set, pppd replaces an existing default route
+with the new default route.
+.TP
.B disconnect \fIscript
Execute the command specified by \fIscript\fR, by passing it to a
shell, after
@@ -706,7 +711,12 @@ disable both forms of hardware flow cont
.TP
.B nodefaultroute
Disable the \fIdefaultroute\fR option. The system administrator who
-wishes to prevent users from creating default routes with pppd
+wishes to prevent users from adding a default route with pppd
+can do so by placing this option in the /etc/ppp/options file.
+.TP
+.B noreplacedefaultroute
+Disable the \fIreplacedefaultroute\fR option. The system administrator who
+wishes to prevent users from replacing a default route with pppd
can do so by placing this option in the /etc/ppp/options file.
.TP
.B nodeflate
diff -ruNp ppp-2.4.4.orig/pppd/pppd.h ppp-2.4.4/pppd/pppd.h
--- ppp-2.4.4.orig/pppd/pppd.h 2005-08-25 23:59:34.000000000 +0000
+++ ppp-2.4.4/pppd/pppd.h 2007-05-18 19:57:57.000000000 +0000
@@ -642,7 +642,7 @@ int sif6addr __P((int, eui64_t, eui64_t
int cif6addr __P((int, eui64_t, eui64_t));
/* Remove an IPv6 address from i/f */
#endif
-int sifdefaultroute __P((int, u_int32_t, u_int32_t));
+int sifdefaultroute __P((int, u_int32_t, u_int32_t, bool replace_default_rt));
/* Create default route through i/f */
int cifdefaultroute __P((int, u_int32_t, u_int32_t));
/* Delete default route through i/f */
diff -ruNp ppp-2.4.4.orig/pppd/sys-linux.c ppp-2.4.4/pppd/sys-linux.c
--- ppp-2.4.4.orig/pppd/sys-linux.c 2005-08-26 22:44:35.000000000 +0000
+++ ppp-2.4.4/pppd/sys-linux.c 2007-05-18 20:01:50.000000000 +0000
@@ -206,6 +206,8 @@ static unsigned char inbuf[512]; /* buff
static int if_is_up; /* Interface has been marked up */
static int have_default_route; /* Gateway for default route added */
+static struct rtentry old_def_rt; /* Old default route */
+static int default_rt_repl_rest; /* replace and restore old default rt */
static u_int32_t proxy_arp_addr; /* Addr for proxy arp entry added */
static char proxy_arp_dev[16]; /* Device for proxy arp entry */
static u_int32_t our_old_addr; /* for detecting address changes */
@@ -1513,6 +1515,9 @@ static int read_route_table(struct rtent
p = NULL;
}
+ SET_SA_FAMILY (rt->rt_dst, AF_INET);
+ SET_SA_FAMILY (rt->rt_gateway, AF_INET);
+
SIN_ADDR(rt->rt_dst) = strtoul(cols[route_dest_col], NULL, 16);
SIN_ADDR(rt->rt_gateway) = strtoul(cols[route_gw_col], NULL, 16);
SIN_ADDR(rt->rt_genmask) = strtoul(cols[route_mask_col], NULL, 16);
@@ -1582,20 +1587,52 @@ int have_route_to(u_int32_t addr)
/********************************************************************
*
* sifdefaultroute - assign a default route through the address given.
- */
-
-int sifdefaultroute (int unit, u_int32_t ouraddr, u_int32_t gateway)
-{
- struct rtentry rt;
+ *
+ * If the global default_rt_repl_rest flag is set, then this function
+ * already replaced the original system defaultroute with some other
+ * route and it should just replace the current defaultroute with
+ * another one, without saving the current route. Use: demand mode,
+ * when pppd sets first a defaultroute it it's temporary ppp0 addresses
+ * and then changes the temporary addresses to the addresses for the real
+ * ppp connection when it has come up.
+ */
+
+int sifdefaultroute (int unit, u_int32_t ouraddr, u_int32_t gateway, bool replace)
+{
+ struct rtentry rt, tmp_rt;
+ struct rtentry *del_rt = NULL;
+
+ if (default_rt_repl_rest) {
+ /* We have already reclaced the original defaultroute, if we
+ are called again, we will delete the current default route
+ and set the new default route in this function.
+ - this is normally only the case the doing demand: */
+ if (defaultroute_exists(&tmp_rt))
+ del_rt = &tmp_rt;
+ } else if (defaultroute_exists(&old_def_rt) &&
+ strcmp(old_def_rt.rt_dev, ifname) != 0) {
+ /* We did not yet replace an existing default route, let's
+ check if we should save and replace a default route: */
+
+ if (!replace) {
+ if (old_def_rt.rt_flags & RTF_GATEWAY)
+ error("not replacing existing default route via %I",
+ SIN_ADDR(old_def_rt.rt_gateway));
+ else
+ error("not replacing existing default route through %s",
+ old_def_rt.rt_dev);
+ return 0;
+ }
- if (defaultroute_exists(&rt) && strcmp(rt.rt_dev, ifname) != 0) {
- if (rt.rt_flags & RTF_GATEWAY)
- error("not replacing existing default route via %I",
- SIN_ADDR(rt.rt_gateway));
- else
- error("not replacing existing default route through %s",
- rt.rt_dev);
- return 0;
+ /* we need to copy rt_dev because we need it permanent too: */
+ char *tmp_dev = malloc(strlen(old_def_rt.rt_dev) + 1);
+ strcpy(tmp_dev, old_def_rt.rt_dev);
+ old_def_rt.rt_dev = tmp_dev;
+
+ notice("replacing old default route to %s [%I]",
+ old_def_rt.rt_dev, SIN_ADDR(old_def_rt.rt_gateway));
+ default_rt_repl_rest = 1;
+ del_rt = &old_def_rt;
}
memset (&rt, 0, sizeof (rt));
@@ -1610,10 +1647,16 @@ int sifdefaultroute (int unit, u_int32_t
rt.rt_flags = RTF_UP;
if (ioctl(sock_fd, SIOCADDRT, &rt) < 0) {
- if ( ! ok_error ( errno ))
+ if (!ok_error(errno))
error("default route ioctl(SIOCADDRT): %m");
return 0;
}
+ if (default_rt_repl_rest && del_rt)
+ if (ioctl(sock_fd, SIOCDELRT, del_rt) < 0) {
+ if (!ok_error(errno))
+ error("del old default route ioctl(SIOCDELRT): %m");
+ return 0;
+ }
have_default_route = 1;
return 1;
@@ -1642,11 +1685,21 @@ int cifdefaultroute (int unit, u_int32_t
rt.rt_flags = RTF_UP;
if (ioctl(sock_fd, SIOCDELRT, &rt) < 0 && errno != ESRCH) {
if (still_ppp()) {
- if ( ! ok_error ( errno ))
+ if (!ok_error(errno))
error("default route ioctl(SIOCDELRT): %m");
return 0;
}
}
+ if (default_rt_repl_rest) {
+ notice("restoring old default route to %s [%I]",
+ old_def_rt.rt_dev, SIN_ADDR(old_def_rt.rt_gateway));
+ if (ioctl(sock_fd, SIOCADDRT, &old_def_rt) < 0) {
+ if (!ok_error(errno))
+ error("restore default route ioctl(SIOCADDRT): %m");
+ return 0;
+ }
+ default_rt_repl_rest = 0;
+ }
return 1;
}
diff -ruNp ppp-2.4.4.orig/pppd/sys-solaris.c ppp-2.4.4/pppd/sys-solaris.c
--- ppp-2.4.4.orig/pppd/sys-solaris.c 2005-07-09 00:23:06.000000000 +0000
+++ ppp-2.4.4/pppd/sys-solaris.c 2007-05-18 19:57:57.000000000 +0000
@@ -2036,12 +2036,18 @@ cifaddr(u, o, h)
* sifdefaultroute - assign a default route through the address given.
*/
int
-sifdefaultroute(u, l, g)
+sifdefaultroute(u, l, g, replace)
int u;
u_int32_t l, g;
+ bool replace;
{
struct rtentry rt;
+ if (replace) {
+ error("replacedefaultroute not supported on this platform");
+ return 0;
+ }
+
#if defined(__USLC__)
g = l; /* use the local address as gateway */
#endif
--- End Message ---
