Re: [RFC PATCH 2/2] ppp_mppe: Check coherency counter for out-of-order sequencing

["Jorge Boncompte [DTI2]" <jorge@dti2.net>]

El 20/05/2013 19:32, Eric Dumazet escribió:
> On Mon, 2013-05-20 at 18:34 +0200, Jorge Boncompte [DTI2] wrote:
>> From: "Jorge Boncompte [DTI2]" <jorge@dti2.net>
>>
>> While testing a L2TP tunnel without sequencing with MPPE encryption in
>> stateless mode noticed that after a packet was reordered the encapsulated
>> traffic session was stuck but testing against a Cisco gear did work.
>>
>> From RFC3078 "MPPE expects packets to be delivered in sequence".
>>
>> The thing it's that the ppp_mppe module treats the reorder as if the
>> coherency counter did wrap and rekeys all the "missing" packets.
>>
>> The link layer protocol should deliver the packets in order but at least
>> with this patch in place the decryption process survives some packet reorder.
>>
>> Signed-off-by: Jorge Boncompte [DTI2] <jorge@dti2.net>
>> ---
>>  drivers/net/ppp/ppp_mppe.c |   21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
>> index 9a1849a..0a10a6d 100644
>> --- a/drivers/net/ppp/ppp_mppe.c
>> +++ b/drivers/net/ppp/ppp_mppe.c
>> @@ -55,6 +55,7 @@
>>  #include <linux/ppp_defs.h>
>>  #include <linux/ppp-comp.h>
>>  #include <linux/scatterlist.h>
>> +#include <linux/net.h>
>>  #include <asm/unaligned.h>
>>  
>>  #include "ppp_mppe.h"
>> @@ -469,6 +470,15 @@ static void mppe_decomp_reset(void *arg)
>>  }
>>  
>>  /*
>> + * Compares two coherency counter values.
>> + */
>> +static int
>> +mppe_cmp_ccount(unsigned int a, unsigned int b)
>> +{
>> +	return (int)((a << 20) - (b << 20));
>> +}
>> +
> 
> How was chosen this magical value ?

	The coherency count it's a 12-bit value. I'll add a define for it.

> 
>> +/*
>>   * Decompress (decrypt) an MPPE packet.
>>   */
>>  static int
>> @@ -547,6 +557,17 @@ mppe_decompress(void *arg, unsigned char *ibuf, int isize, unsigned char *obuf,
>>  	 */
>>  
>>  	if (!state->stateful) {
>> +		if (mppe_cmp_ccount(ccount, state->ccount) < 0) {
>> +			if (state->debug >= 7 && net_ratelimit())
>> +				printk(KERN_DEBUG
>> +				       "%s[%d:]: Dropping out-of-order packet, "
>> +				       "ccount %u expecting %u.\n",
>> +				       __func__, state->unit, ccount,
>> +				       state->ccount);
>> +
> 
> 
> net_dbg_ratelimited() ?

	I think it will be better if I prepare a third patch that cleanups the whole
file after.

-- 
==============================================================
Jorge Boncompte - Ingenieria y Gestion de RED
DTI2 - Desarrollo de la Tecnologia de las Comunicaciones
--------------------------------------------------------------
C/ Abogado Enriquez Barrios, 5   14004 CORDOBA (SPAIN)
Tlf: +34 957 761395 / FAX: +34 957 450380
==============================================================
- There is only so much duct tape you can put on something
  before it just becomes a giant ball of duct tape.
==============================================================