No subject


Sun Jul 8 18:14:56 CDT 2012


      The MPPE scheme does not require a reliable link.  Instead, it
      relies on a 12-bit coherency count in each packet to keep the
      encryption tables synchronized.  If stateless mode has not been
      negotiated and the coherency count in the received packet does not
      match the expected count, the receiver MUST send a CCP Reset-
      Request packet to cause the resynchronization of the RC4 tables.

This requirement was not being met. The mppe kernel module was correctly
returning DECOMP_ERROR, after (unnecessarily) updating the in-kernel
encryption state, but the CCP code in the PPP daemon was not sending the
CCP Reset-Request packet:

          /* MPPE/MPPC does not requires CCP_RESETREQ */
          if (ccp_gotoptions[f->unit].method == CI_MPPE)
              return;

Other aspects of packet handling in stateful mode are not correct:

 If packet loss is detected while using stateful encryption, the
 receiver MUST drop the packet and send a CCP Reset-Request packet without data.
 After transmitting the CCP Reset-Request packet, the receiver SHOULD
 silently discard all packets until a packet is received with the FLUSHED
 bit set.

The current mppe code does not comply with this requirement. If packet
loss is detected (by the coherency count not matching the expected value),
then the current packet is dropped, but the decomp_error state is not set,
and following packets are not dropped (until a packet is received with the
FLUSHED bit set).

The following patch corrects these protocol violations, as well as a few
miscellaneous changes:

Line 334 of ppp_mppe.c:
  add an indication of what code is printing the info.

Line 415 of ppp_mppe.c:
 Correct order of parameters to DEBUG printk, and

Line 512 of ppp_mppe.c:
 Add heuristics to determine the arrival of delayed out-of-order packets.

 As draft-ietf-pppext-mppe-05.txt states, MPPE expects packets to be
 delivered in sequence. As we know, this isn't the case in general over
 the Internet. The MPPE handler module currently treats arrival of a
 delayed out of order packet as though it was the first packet after a
 long sequence of dropped packets - it advances the encryption state to
 the expected state that many more packets. The added heuristic ignores
 packets with a coherency count more than 50 more than expected. These
 packets will almost certainly be packets which have already been treated
 as lost.

Feedback welcome.

  Charlie Brady                         charlieb at e-smith.com
  http://www.e-smith.org (development)  http://www.e-smith.com (corporate)
  Phone: +1 (613) 368 4376 or 564 8000  Fax: +1 (613) 564 7739
  e-smith, inc. 1500-150 Metcalfe St, Ottawa, ON K2P 1P1 Canada

diff -ruN ppp-2.4.0-4.orig/linux/ppp_mppe.c ppp-2.4.0-4/linux/ppp_mppe.c
--- ppp-2.4.0-4.orig/linux/ppp_mppe.c	Wed Jan  3 21:57:42 2001
+++ ppp-2.4.0-4/linux/ppp_mppe.c	Thu Jan  4 11:06:14 2001
@@ -331,7 +331,8 @@
     struct ppp_mppe_state *state = (struct ppp_mppe_state *)arg;

     if (options[0] != CI_MPPE || options[1] != CILEN_MPPE) {
-	printk(KERN_DEBUG"options are bad: %x %x\n",options[0],options[1]);
+	printk(KERN_DEBUG "mppe_decomp_init: unexpected options: %x %x\n",
+	    options[0],options[1]);
 	return 0;
     }

@@ -412,7 +413,7 @@

     if(osize < isize+MPPE_OVHD) {
 	printk(KERN_DEBUG "Not enough space to encrypt packet: %d<%d+%d!\n",
-		isize, osize, MPPE_OVHD);
+		osize, isize, MPPE_OVHD);
 	return 0;
     }

@@ -460,13 +461,7 @@
     (state->stats).in_count = (state->stats).unc_bytes;
     (state->stats).bytes_out = (state->stats).comp_bytes;

-#if 0 /* FP not allowed in the kernel */
-    /* this _SHOULD_ always be 1 */
-    (state->stats).ratio = (state->stats).in_count/(state->stats).bytes_out;
-#endif
-
     *stats = state->stats;
-
 }


@@ -477,6 +472,7 @@
 {
     struct ppp_mppe_state *state = (struct ppp_mppe_state *)arg;
     int seq;
+    int missing;

     if (isize <= PPP_HDRLEN + MPPE_OVHD) {
 	if (state->debug) {
@@ -490,28 +486,51 @@
     /* Check the sequence number. */
     seq = MPPE_CCOUNT_FROM_PACKET(ibuf);

-    if(!state->stateless && (MPPE_BITS(ibuf) & MPPE_BIT_FLUSHED)) {
+    if (!state->stateless && (MPPE_BITS(ibuf) & MPPE_BIT_FLUSHED)) {
         state->decomp_error = 0;
         state->ccount = seq;
     }

-    if(state->decomp_error) {
+    if (state->decomp_error) {
+	// ignore incoming packets until a packet arrives with FLUSHED bit set
         return DECOMP_ERROR;
     }

-    if (seq != state->ccount) {
+    if (seq != state->ccount)
+    {
 	if (state->debug) {
 	    printk(KERN_DEBUG "mppe_decompress%d: bad seq # %d, expected %d\n",
 		   state->unit, seq, state->ccount);
 	}
-
-        while(state->ccount != seq) {
-            mppe_update_count(state);
+	if (!state->stateless)
+	{
+	    // In stateful mode, we drop this packet, and then all packets
+	    // until the encryption is resynchronised
+	    state->decomp_error = 1;
+	    return DECOMP_ERROR;
+	}
+	// In stateless mode, we have no alternative but to ignore out of
+	// order packets which arrive late. If a packet arrives before
+	// its predecessor, we need to assume that the predecessor packet
+	// is lost. We have to use a heuristic to determine whether a packet
+	// is very late or very early
+	missing = seq - state->ccount;
+	if (missing < 0)
+	{
+	    missing += 0xfff;
+	}
+	if (missing > 50)
+	{
+	    // Assume that this is actually a late packet, which we can
+	    // no longer process
+	    return DECOMP_ERROR;
+	}
+	// We have missed some small number of packets - move our encryption
+	// state along so that we can decrypt it
+	while (state->ccount != seq)
+	{
+	    mppe_update_count(state);
 	}
-
-        mppe_update_count(state);
-
-	return DECOMP_ERROR;
     }

     /*
@@ -522,11 +541,14 @@
     obuf[1] = PPP_CONTROL(ibuf);
     obuf += 2;

-    if(!(MPPE_BITS(ibuf) & MPPE_BIT_ENCRYPTED)) {
+    if (!(MPPE_BITS(ibuf) & MPPE_BIT_ENCRYPTED))
+    {
         printk(KERN_DEBUG"ERROR: not an encrypted packet");
         mppe_synchronize_key(state);
 	return DECOMP_ERROR;
-    } else {
+    }
+    else
+    {
 	if(!state->stateless && (MPPE_BITS(ibuf) & MPPE_BIT_FLUSHED))
 	    mppe_synchronize_key(state);
 	mppe_update_count(state);
@@ -540,7 +562,6 @@
 	return isize-MPPE_OVHD;
     }
 }
-

 /* Incompressible data has arrived - add it to the history.  */
 static void
diff -ruN ppp-2.4.0-4.orig/pppd/ccp.c ppp-2.4.0-4/pppd/ccp.c
--- ppp-2.4.0-4.orig/pppd/ccp.c	Wed Jan  3 21:57:41 2001
+++ ppp-2.4.0-4/pppd/ccp.c	Wed Jan  3 22:42:44 2001
@@ -1454,11 +1454,13 @@
 	     */
 	    error("Lost compression sync: disabling compression");
 	    ccp_close(unit, "Lost compression sync");
-    	    if ( ccp_wantoptions[unit].require_mppe || ccp_wantoptions[unit].require_mppe_stateless )
+    	    if (ccp_wantoptions[unit].require_mppe
+		    || ccp_wantoptions[unit].require_mppe_stateless )
 		lcp_close(unit,"Encryption got out of order");
 	} else {
-	    /* MPPE/MPPC does not requires CCP_RESETREQ */
-	    if (ccp_gotoptions[f->unit].method == CI_MPPE)
+	    /* stateless MPPE/MPPC does not requires CCP_RESETREQ */
+	    if (ccp_gotoptions[f->unit].method == CI_MPPE
+		&& ccp_gotoptions[f->unit].require_mppe_stateless)
 		return;
 	    /*
 	     * Send a reset-request to reset the peer's compressor.








More information about the pptp-server mailing list