kern/182212: [patch] [ng_mppc] ng_mppc(4) blocks on network errors unconditionaly

[Eugene Grosbein]

>Number:         182212
>Category:       kern
>Synopsis:       [patch] [ng_mppc] ng_mppc(4) blocks on network errors unconditionaly
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          change-request
>Submitter-Id:   current-users
>Arrival-Date:   Wed Sep 18 19:30:00 UTC 2013
>Closed-Date:
>Last-Modified:
>Originator:     Eugene Grosbein
>Release:        FreeBSD 8.4-STABLE amd64
>Organization:
RDTC JSC
>Environment:
System: FreeBSD eg.sd.rdtc.ru 8.4-STABLE FreeBSD 8.4-STABLE #2 r251830M: Mon Jun 17 18:05:24 NOVT 2013 root at eg.sd.rdtc.ru:/usr/local/obj/usr/local/src/sys/EG amd64

>Description:
	Netgraph node ng_mppc(4) is used by mpd daemon to implement
	MPPC/MPPE for Compression Control Protocol (CCP) used with various
	PPP tunnels (pptp/l2tp/etc.)

	This node blocks itself when "too many" packets got dropped in between
	PPP peers and MPPE should do "re-keying". The threshold is hardcoded
	in its code. The code considers "re-keying" as too CPU intensive task.
	Thus, it tries to protect the box from DoS.
	This code dates back to year 2000.

	When such event occcurs, PPP tunnel hangs: CCP does not get reset,
	IP packets cannot pass tunnel anymore but system interface "looks fine".

	These days mpd runs on multi-MHZ and GHZ boxes and may be used
	not for BRAS'es with lots of tunnels but to form VPN between a pair
	of routers. In the latter case, manual reset of mpd link is required
	to revive VPN tunnel.


>How-To-Repeat:
	
	Run PPtP or L2TP tunnel between two mpd servers over WAN link
	with non-zero amount of packet drops and/or packet rearrangements
	having MPPC/MPPE enabled. Soon, you will get a message in the dmesg
	buffer similar to:

ng_mppc_decompress: too many (4094) packets dropped, disabling node 0xc7020900!

	Then tunnel just hangs until manually restarted as LCP echos
	cannot detect this problem.

>Fix:

	Let's system administrator decides if such behavour is needed.
	The following patch introduces new sysctl subtree "net.graph.mppe"
	with three read/write sysctls (each one is loader tunnable too):

net.graph.mppe.block_on_max_rekey - non-zero value means current behavour,
zero disables node block;

net.graph.mppe.mppe_log_max_rekey - non-zero value permits to write messages
to the log to notify of described event;

net.graph.mppe.max_rekey - allows to change the threshold.

	By default, node block is prohibited and mpd just resets CCP
	and tunnel continues to work. Mpd writes a line like this to its log:

CCP: SendResetReq #3 link 0 (Opened)
	
	At the another side, next line is written to mpd log:

CCP: rec'd Reset Request #3 (Opened)

--- sys/netgraph/ng_mppc.c.orig	2013-04-07 01:07:27.000000000 +0700
+++ sys/netgraph/ng_mppc.c	2013-09-19 01:54:54.000000000 +0700
@@ -55,6 +55,7 @@
 #include <sys/malloc.h>
 #include <sys/endian.h>
 #include <sys/errno.h>
+#include <sys/sysctl.h>
 #include <sys/syslog.h>
 
 #include <netgraph/ng_message.h>
@@ -107,6 +108,23 @@
  */
 #define MPPE_MAX_REKEY		1000
 
+SYSCTL_NODE(_net_graph, OID_AUTO, mppe, CTLFLAG_RW, 0, "MPPE");
+
+static int mppe_block_on_max_rekey = 0;
+TUNABLE_INT("net.graph.mppe.block_on_max_rekey", &mppe_block_on_max_rekey);
+SYSCTL_INT(_net_graph_mppe, OID_AUTO, block_on_max_rekey, CTLFLAG_RW,
+    &mppe_block_on_max_rekey, 0, "Block node on max MPPE key re-calculations");
+
+static int mppe_log_max_rekey = 1;
+TUNABLE_INT("net.graph.mppe.log_max_rekey", &mppe_log_max_rekey);
+SYSCTL_INT(_net_graph_mppe, OID_AUTO, log_max_rekey, CTLFLAG_RW,
+    &mppe_log_max_rekey, 0, "Log max MPPE key re-calculations event");
+
+static int mppe_max_rekey = MPPE_MAX_REKEY;
+TUNABLE_INT("net.graph.mppe.max_rekey", &mppe_max_rekey);
+SYSCTL_INT(_net_graph_mppe, OID_AUTO, max_rekey, CTLFLAG_RW,
+    &mppe_max_rekey, 0, "Maximum number of MPPE key re-calculations");
+
 /* MPPC packet header bits */
 #define MPPC_FLAG_FLUSHED	0x8000		/* xmitter reset state */
 #define MPPC_FLAG_RESTART	0x4000		/* compress history restart */
@@ -651,12 +669,23 @@
 			/* How many times are we going to have to re-key? */
 			rekey = ((d->cfg.bits & MPPE_STATELESS) != 0) ?
 			    numLost : (numLost / (MPPE_UPDATE_MASK + 1));
-			if (rekey > MPPE_MAX_REKEY) {
-				log(LOG_ERR, "%s: too many (%d) packets"
-				    " dropped, disabling node %p!",
-				    __func__, numLost, node);
+			if (rekey > mppe_max_rekey) {
+			    if (mppe_block_on_max_rekey) {
+				if (mppe_log_max_rekey) {
+				    log(LOG_ERR, "%s: too many (%d) packets"
+					" dropped, disabling node %p!\n",
+					__func__, numLost, node);
+				}
 				priv->recv.cfg.enable = 0;
 				goto failed;
+			    } else {
+				if (mppe_log_max_rekey) {
+				    log(LOG_ERR, "%s: %d packets"
+					" dropped, node %p\n",
+					__func__, numLost, node);
+				}
+				goto failed;
+			    }
 			}
 
 			/* Re-key as necessary to catch up to peer */

>Release-Note:
>Audit-Trail:
>Unformatted: