SOLVED: kernel-mode PPPoE does not seem able to work with MPPE.

SOLVED: kernel-mode PPPoE does not seem able to work with MPPE.

[David Fernandez]

On 25/10/17 18:45, David Fernandez wrote:
> On 24/10/17 13:09, David Fernandez wrote:
>> On 24/10/17 09:52, David Fernandez wrote:
>>> Hi there,
>>>
>>> I've tried to run MPPE in a PPPoE connection to my LEDE linux 
>>> server. The log is below.
>>>
>>> Looking at wireshark traces, it seems to negotiate mschap-v2 and 
>>> mppe fine, but then ppp seems not to accept encrypted payloads.
>>>
>>
>>> Although it should not be needed, if I use the option require-mppe, 
>>> pppd complains of unrecognized option.
>>>
>>> If I grep for mppe in the 2.4.7 sources downloaded by the LEDE build 
>>> system, I see that it appears only in the pptp plugin, which is 
>>> strange, as the mppe options are in the pppd manual page as 
>>> generally available ones.
>>>
>>> I've tried to load both plugins (rp-pppoe.so and pptp.so) in an 
>>> attempt to have the mppe working with require-mppe, but the result 
>>> seems the same (unrecognized option).
>>>
>> On this bit, looking at the sources I found that the way it works is 
>> by making the option like:
>> mppe require
>> I guess that this should be updated in the manual...
>> With it I get this logging line:
>> Feb  3 09:09:16 LEDE pppd[3307]: mppe xxx # [don't know how to print 
>> value]#011#011# (from /etc/ppp/pppoe-server-options)
>> Everything else is the same.
>> So I guess this is some kind of bug in pppd/ccp.c?
>>
>>> Anybody knows why it does not work as expected?
>>>
>>> (started with pppoe-server -k -C myserver -S myservice -I eth1)
>>>
>>>
>>> Feb  2 12:05:20 LEDE pppoe-server[1580]: Session 1 created for 
>>> client 7c:d3:0a:15:22:49 (10.67.15.1) on eth1 using Service-Name 
>>> 'myservice'
>>> Feb  2 12:05:20 LEDE pppd[1580]: Plugin /etc/ppp/plugins/rp-pppoe.so 
>>> loaded.
>>> Feb  2 12:05:20 LEDE pppd[1580]: RP-PPPoE plugin version 3.8p 
>>> compiled against pppd 2.4.7
>>> Feb  2 12:05:20 LEDE modprobe: failed to find a module named 
>>> netdev-10.0.0.1
>>> Feb  2 12:05:20 LEDE modprobe: failed to find a module named 
>>> netdev-10.0.0.1
>>> Feb  2 12:05:20 LEDE pppd[1580]: pppd options in effect:
>>> Feb  2 12:05:20 LEDE pppd[1580]: debug#011#011# (from 
>>> /etc/ppp/pppoe-server-options)
>>> Feb  2 12:05:20 LEDE pppd[1580]: nodetach#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: dump#011#011# (from 
>>> /etc/ppp/pppoe-server-options)
>>> Feb  2 12:05:20 LEDE pppd[1580]: plugin 
>>> /etc/ppp/plugins/rp-pppoe.so#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: require-mschap-v2#011#011# (from 
>>> /etc/ppp/pppoe-server-options)
>>> Feb  2 12:05:20 LEDE pppd[1580]: name myserver#011#011# (from 
>>> /etc/ppp/pppoe-server-options)
>>> Feb  2 12:05:20 LEDE pppd[1580]: eth1#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: rp_pppoe_service myservice#011#011# 
>>> (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: rp_pppoe_sess 
>>> 1:7c:d3:0a:15:22:49#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: eth1#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: rp_pppoe_service myservice#011#011# 
>>> (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: rp_pppoe_sess 
>>> 1:7c:d3:0a:15:22:49#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: noaccomp#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: default-asyncmap#011#011# (from 
>>> command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: mru 1492#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: mtu 1492#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: nopcomp#011#011# (from command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: lcp-echo-failure 2#011#011# (from 
>>> /etc/ppp/pppoe-server-options)
>>> Feb  2 12:05:20 LEDE pppd[1580]: lcp-echo-interval 10#011#011# (from 
>>> /etc/ppp/pppoe-server-options)
>>> Feb  2 12:05:20 LEDE pppd[1580]: noipdefault#011#011# (from 
>>> /etc/ppp/pppoe-server-options)
>>> Feb  2 12:05:20 LEDE pppd[1580]: nodefaultroute#011#011# (from 
>>> /etc/ppp/pppoe-server-options)
>>> Feb  2 12:05:20 LEDE pppd[1580]: netmask 255.0.0.0#011#011# (from 
>>> /etc/ppp/pppoe-server-options)
>>> Feb  2 12:05:20 LEDE pppd[1580]: 10.0.0.1:10.67.15.1#011#011# (from 
>>> command line)
>>> Feb  2 12:05:20 LEDE pppd[1580]: pppd 2.4.7 started by root, uid 0
>>> Feb  2 12:05:20 LEDE pppd[1580]: Connected to 7c:d3:0a:15:22:49 via 
>>> interface eth1
>>> Feb  2 12:05:20 LEDE pppd[1580]: Using interface ppp0
>>> Feb  2 12:05:20 LEDE pppd[1580]: Connect: ppp0 <--> eth1
>>> Feb  2 12:05:22 LEDE pppd[1580]: peer from calling number 
>>> 7C:D3:0A:15:22:49 authorized
>>> Feb  2 12:05:22 LEDE pppd[1580]: MPPE 128-bit stateful compression 
>>> enabled
>>> Feb  2 12:05:22 LEDE pppd[1580]: local  IP address 10.0.0.1
>>> Feb  2 12:05:22 LEDE pppd[1580]: remote IP address 10.67.15.1
>>> Feb  2 12:05:22 LEDE pppd[1580]: Unsupported protocol 0xc8c8 received
>>> Feb  2 12:05:22 LEDE pppd[1580]: Unsupported protocol 0x3d received
>>> Feb  2 12:05:22 LEDE pppd[1580]: Unsupported protocol 0x79 received
>>> Feb  2 12:05:22 LEDE pppd[1580]: Unsupported protocol 'PPP Muxing' 
>>> (0x59) received
>>> Feb  2 12:05:22 LEDE pppd[1580]: Unsupported protocol 0x2805 received
>>> Feb  2 12:05:26 LEDE pppd[1580]: Unsupported protocol 0xf6a9 received
>>> Feb  2 12:05:29 LEDE pppd[1580]: Unsupported protocol 0x2e59 received
>>> ...
>>
> Seems that this problem was kind of reported here as this:
>
> I Found it originally here: 
> https://www.spinics.net/lists/linux-ppp/msg01106.html
>
> It is indeed in the list here: 
> https://marc.info/?l=linux-ppp&m\x129753728204109&w=2
>
> Seems that it does solve two problems, but not all of them... Anyway, 
> it seems that it is an olde kernel version problem, as I'm using 
> kernel 4.4 and this might be fixed entirely in modern kernels...
>
> I'll check what the latest kernel ppp_mppe.c looks like.

Right, seems that the latest kernel has not bother with this at all (at 
least in kernel.org).

The two patches proposed in the links above are basically all that is 
needed AFAICS, only that the first one seems wrong in using only ccount 
to avoid the first re-rekeying, as ccount will wrap around to 0 every 
now and then, so this is the patch that works for me (applied to LEDE 
kernel 4.4.45, I guess it will apply fine to later kernels, only the 
line numbers might be different).

--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -521,11 +521,12 @@ mppe_decompress(void *arg, unsigned char
                 state->sanity_errors += 100;
                 goto sanity_error;
         }
-       if (state->stateful && ((ccount & 0xff) = 0xff) && !flushed) {
+       if (state->stateful && ((ccount & 0xff) = 0xff) && !flushed) {/*
                 printk(KERN_DEBUG "mppe_decompress[%d]: FLUSHED bit not 
set on "
                        "flag packet!\n", state->unit);
                 state->sanity_errors += 100;
-               goto sanity_error;
+               goto sanity_error;*/
+                flushed = 1;
         }

         /*
@@ -586,8 +587,11 @@ mppe_decompress(void *arg, unsigned char
                                  */
                         }
                 }
-               if (flushed)
+               if (flushed && (state->bits & 1) != 0)
                         mppe_rekey(state, 0);
+                else
+                if ((state->bits & 1) = 0 && ccount = 0 && flushed)
+                  state->bits |= 1;
         }

         /*

Basically use the state->bits & 1 as a start flag, given that they are 
not used at all in the decompressor, is a way of quickly doing it with 
minimal changes... Feel free to add a proper boolean to the state 
structure and make it more obvious, but with thos two things I get it 
working just fine for a long while now.

Regards

Re: SOLVED: kernel-mode PPPoE does not seem able to work with MPPE.

[David Fernandez]

On 26/10/17 11:39, David Fernandez wrote:
> Right, seems that the latest kernel has not bother with this at all 
> (at least in kernel.org).
>
> The two patches proposed in the links above are basically all that is 
> needed AFAICS, only that the first one seems wrong in using only 
> ccount to avoid the first re-rekeying, as ccount will wrap around to 0 
> every now and then, so this is the patch that works for me (applied to 
> LEDE kernel 4.4.45, I guess it will apply fine to later kernels, only 
> the line numbers might be different).
>
> --- a/drivers/net/ppp/ppp_mppe.c
> +++ b/drivers/net/ppp/ppp_mppe.c
> @@ -521,11 +521,12 @@ mppe_decompress(void *arg, unsigned char
>                 state->sanity_errors += 100;
>                 goto sanity_error;
>         }
> -       if (state->stateful && ((ccount & 0xff) = 0xff) && !flushed) {
> +       if (state->stateful && ((ccount & 0xff) = 0xff) && !flushed) {/*
>                 printk(KERN_DEBUG "mppe_decompress[%d]: FLUSHED bit 
> not set on "
>                        "flag packet!\n", state->unit);
>                 state->sanity_errors += 100;
> -               goto sanity_error;
> +               goto sanity_error;*/
> +                flushed = 1;
>         }
>
>         /*
> @@ -586,8 +587,9 @@ mppe_decompress(void *arg, unsigned char
>                                  */
>                         }
>                 }
> -               if (flushed)
> +               if (flushed && (state->bits & 1) != 0)
>                         mppe_rekey(state, 0);
> +               state->bits |= 1;
>         }
>
>         /*
>
Fixed to make it work for both linux and windows.
> Basically use the state->bits & 1 as a start flag, given that they are 
> not used at all in the decompressor, is a way of quickly doing it with 
> minimal changes... Feel free to add a proper boolean to the state 
> structure and make it more obvious, but with thos two things I get it 
> working just fine for a long while now.
>
> Regards

Re: SOLVED: kernel-mode PPPoE does not seem able to work with MPPE.

[Charlie Brady]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 919 bytes --]


Why would you do this, rather than just delete the code you are removing?

> --- a/drivers/net/ppp/ppp_mppe.c
> +++ b/drivers/net/ppp/ppp_mppe.c
> @@ -521,11 +521,12 @@ mppe_decompress(void *arg, unsigned char
>                 state->sanity_errors += 100;
>                 goto sanity_error;
>         }
> -       if (state->stateful && ((ccount & 0xff) == 0xff) && !flushed) {
> +       if (state->stateful && ((ccount & 0xff) == 0xff) && !flushed)
> {/*
>                 printk(KERN_DEBUG "mppe_decompress[%d]: FLUSHED
> bit not set on "
>                        "flag packet!\n", state->unit);
>                 state->sanity_errors += 100;
> -               goto sanity_error;
> +               goto sanity_error;*/
> +                flushed = 1;
>         }

Re: SOLVED: kernel-mode PPPoE does not seem able to work with MPPE.

[David Fernandez]

On 06/11/17 14:19, Charlie Brady wrote:
> Why would you do this, rather than just delete the code you are removing?
>
>> --- a/drivers/net/ppp/ppp_mppe.c
>> +++ b/drivers/net/ppp/ppp_mppe.c
>> @@ -521,11 +521,12 @@ mppe_decompress(void *arg, unsigned char
>>                  state->sanity_errors += 100;
>>                  goto sanity_error;
>>          }
>> -       if (state->stateful && ((ccount & 0xff) = 0xff) && !flushed) {
>> +       if (state->stateful && ((ccount & 0xff) = 0xff) && !flushed)
>> {/*
>>                  printk(KERN_DEBUG "mppe_decompress[%d]: FLUSHED
>> bit not set on "
>>                         "flag packet!\n", state->unit);
>>                  state->sanity_errors += 100;
>> -               goto sanity_error;
>> +               goto sanity_error;*/
>> +                flushed = 1;
>>          }

Hi Charlie,

Yes, the code could be deleted, I just wanted somebody to check and say 
that there is no point in yet putting a warning in the log for this...

Definitely you could do:

--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -521,11 +521,7 @@ mppe_decompress(void *arg, unsigned char
                 state->sanity_errors += 100;
                 goto sanity_error;
         }
-       if (state->stateful && ((ccount & 0xff) = 0xff) && !flushed) {
+       if (state->stateful && ((ccount & 0xff) = 0xff) && !flushed)
-               printk(KERN_DEBUG "mppe_decompress[%d]: FLUSHED bit not 
set on "
-                      "flag packet!\n", state->unit);
-               state->sanity_errors += 100;
-               goto sanity_error;
+               flushed = 1;
-       }

         /*
@@ -586,8 +587,9 @@ mppe_decompress(void *arg, unsigned char
                                  */
                         }
                 }
-               if (flushed)
+               if (flushed && (state->bits & 1) != 0)
                         mppe_rekey(state, 0);
+               state->bits |= 1;
         }

         /*

Would be good to have this patch in the current and previous kernel 
versions, as it seems to apply cleanly enough.

If there is anything I can do to prepare a more formal patch, let me 
know. Specially some formating and changelog things that I am not aware of.

Regards

Re: SOLVED: kernel-mode PPPoE does not seem able to work with MPPE.

[James Carlson]

On 11/06/17 09:59, David Fernandez wrote:
> Yes, the code could be deleted, I just wanted somebody to check and say
> that there is no point in yet putting a warning in the log for this...

RFC3078 7.2 says that the receiver changes its key on receiving a "flag"
packet (where (count&0xff) = 0xff).  It says nothing about the
transmitter setting the FLUSHED flag in this case.  That flag is used
*only* when a sequence error is detected.

So I think the original code was wrong (it should not have insisted on
the flag being set), and the new code without the message and without
the error handling is correct.

It _might_ be a good idea to check and at least comment the sending
code.  It should not be setting the FLUSHED flag merely because it is
sending a flag packet ... but, for compatibility with broken Linux MPPE
implementations already deployed, it might have to continue doing that.

It's a shame documents like this don't go through WG review with
multiple independent implementations.  This is the sort of spec
misinterpretation that a good open review is designed to catch.

-- 
James Carlson         42.703N 71.076W         <carlsonj@workingcode.com>

Re: SOLVED: kernel-mode PPPoE does not seem able to work with MPPE.

[David Fernandez]

On 06/11/17 18:31, James Carlson wrote:
> On 11/06/17 09:59, David Fernandez wrote:
>> Yes, the code could be deleted, I just wanted somebody to check and say
>> that there is no point in yet putting a warning in the log for this...
> RFC3078 7.2 says that the receiver changes its key on receiving a "flag"
> packet (where (count&0xff) = 0xff).  It says nothing about the
> transmitter setting the FLUSHED flag in this case.  That flag is used
> *only* when a sequence error is detected.
>
> So I think the original code was wrong (it should not have insisted on
> the flag being set), and the new code without the message and without
> the error handling is correct.
>
> It _might_ be a good idea to check and at least comment the sending
> code.  It should not be setting the FLUSHED flag merely because it is
> sending a flag packet ... but, for compatibility with broken Linux MPPE
> implementations already deployed, it might have to continue doing that.
>
> It's a shame documents like this don't go through WG review with
> multiple independent implementations.  This is the sort of spec
> misinterpretation that a good open review is designed to catch.
>
Hi James,

I think you are right, the flushed bit is only used for sequence errors 
in stateful mode, as that is the only way to change keys in a 
deterministic manner.

I'm runing now into the problem of packet loss, and I guess I'll have to 
modify the implementation in order to use the flushed bit instead of a 
reset-ack packet to decide if we have to send another reset-req after a 
timeout...

I think this will become a formal patch, so let me know what ettiquete 
bits are required and I'll prepare it for review and submission to 
whomever can approve and commit.

Regards