OPNsense Forum

English Forums => Development and Code Review => Topic started by: mleone87 on June 08, 2017, 09:50:35 am

Title: [SOLVED] Integrate an important PPPoE patch
Post by: mleone87 on June 08, 2017, 09:50:35 am
Hi friends,
my and many others ISPs tend to modify their implementation of PPPoE protocol to avoid the usage of custom routers, forcing PADI request to contains a fixed host-unique tag.

There are already some patch for pppd daemon on linux and on openwrt.
Ubiquity has patched their firmware starting version 1.9 and they work too.

The situation is not so good on freebsd.

I've found this working patch on freebsd https://reviews.freebsd.org/D9270

I've submitted a feature request to pfsense but they do not seem to be interested in integrating it(cost free, almost). Also, download and compiling their sources is difficult due their build chain restrictions.


Can you help me integrating this patch in opnsense and ad the host-uniq tag in web frontent page?
I've seen that build the system from scratch is possible and I have a very good technical background on linux but no experience on freebsd. With some tips I could do it myself(except frontend part, no experience on it)-

Thanks for any help

Thanks
Title: Re: Integrate an important PPPoE patch
Post by: franco on June 08, 2017, 01:51:30 pm
Hi there,

Patch added here: https://github.com/opnsense/src/commit/ae4a6741b

Patched test kernel uploaded for OPNsense 17.1 amd64:

# opnsense-update -kr 17.1.7-pppoe

Let me know if this is what you expected.

Disclaimer: I have not booted this kernel. I don't have a PPPoE setup to verify its operational integrity.


Cheers,
Franco
Title: Re: Integrate an important PPPoE patch
Post by: mleone87 on June 08, 2017, 04:35:25 pm
Thanks!

The kernel boots.
A normal pppoe session with new kernel(and random host-uniq), partialy works as usual, so the ng_pppoe is not broken.

The problem is that now I need to specify the host-uniq tag. If I manually edit /var/etc/mpd_opt2.conf (in my setup) trying to add the new tag with "set host-uniq XXXX..." the file gets overwrited so we need to pass it via web interface or prevent overwriting

Dunno if the sintax of mpd.conf is correct, tough. It would be easier to specify a new textbox in the GUI
thanks

edit: syntax should be "set service "string|""  so a little tweak of XML config works
Title: Re: Integrate an important PPPoE patch
Post by: mleone87 on June 19, 2017, 01:08:34 pm
After some days of testing i can confirm that this kernel works
Title: Re: Integrate an important PPPoE patch
Post by: franco on July 04, 2017, 03:57:14 pm
We will consider this for inclusion in 17.7 by adding it to the upcoming release candidate. Thank you!
Title: Re: Integrate an important PPPoE patch
Post by: pvelati on July 17, 2017, 01:34:11 pm
hi franco, the 17.7.r1 includes this pppoe patch?
If yes, it's in the standard kernel or I have to install the 17.1.7-pppoe?

@mleone87
How did you test connectivity without the custom field in the webgui?

thanks
Title: Re: Integrate an important PPPoE patch
Post by: franco on July 17, 2017, 04:21:58 pm
Yes: https://github.com/opnsense/changelog/blob/master/doc/17.7/17.7.r1#L53
Title: Re: Integrate an important PPPoE patch
Post by: pvelati on July 18, 2017, 06:08:12 pm
I've done some tweaks on "/usr/local/www/interfaces_ppps_edit.php" in order to add host-uniq directly from webgui.
You can see in the image posted on the left the webgui, on the right the config.xml
(https://s3.postimg.org/8wmx3kosv/host-uniq1.png) (https://postimg.org/image/8wmx3kosv/)

The problem now is to make this configuration be parsed and apply the "host-uniq" value to mpd_XXX.conf
Title: Re: Integrate an important PPPoE patch
Post by: franco on July 18, 2017, 06:29:55 pm
Nice work! The underlying code that writes the configuration is here:

https://github.com/opnsense/core/blob/master/src/etc/inc/interfaces.inc#L1539-L1543

Should be even easier than editing the GUI page. :)


Cheers,
Franco
Title: Re: Integrate an important PPPoE patch
Post by: pvelati on July 19, 2017, 11:39:21 am
ok, I've done it.

/usr/local/etc/inc/interfaces.inc

line 1575:

original:
        } elseif ($ppp['type'] == "pppoe") {
            $provider = isset($ppp['provider']) ? $ppp['provider'] : "";
            $mpdconf_arr[] = "set pppoe service \"{$provider}\"";
            $mpdconf_arr[] = "set pppoe iface {$port}";

modified:

        } elseif ($ppp['type'] == "pppoe") {
            $provider = isset($ppp['provider']) ? $ppp['provider'] : "";
            $mpdconf_arr[] = "set pppoe service \"{$provider}\"";
            $mpdconf_arr[] = "set pppoe iface {$port}";
         if (!empty($ppp['hostuniq'])) {
            $mpdconf_arr[] = "set host-uniq {$ppp["hostuniq"]}";
         }
-------------------------------------------------------------------------------

/usr/local/www/interfaces_ppps_edit.php

line 52:
original:
                        'apn', 'apnum', 'phone', 'connect-timeout', 'provider');
modified:
                        'apn', 'apnum', 'phone', 'connect-timeout', 'provider', 'hostuniq');

line 178:
original:
        $port_fields = array("localip", "gateway", "subnet", "bandwidth", "mtu", "mru", "mrru");   
modified:
        $port_fields = array("localip", "gateway", "subnet", "bandwidth", "mtu", "mru", "mrru", "hostuniq");

line 236:
added:
      $ppp['hostuniq'] = $pconfig['hostuniq'];

line 784:
added:
   <tr>
            <td><?=gettext("Host-Uniq"); ?></td>
       <td>
           <input name="hostuniq" class="intf_select_<?=$intf_idx;?>" type="text" value="<?=$pconfig['hostuniq'];?>" />
            </td>
     </div>
   </tr>

line 791:
added:
                              <li><?=gettext("Host-Uniq: Set ONLY if needed.");?></li>
-------------------------------------------------------------------------------


this should work, I don't know if someone wants to review this code and push to upstream....
Title: Re: Integrate an important PPPoE patch
Post by: pvelati on July 19, 2017, 04:25:47 pm
I've created a pull request

https://github.com/opnsense/core/pull/1730
Title: Re: Integrate an important PPPoE patch
Post by: franco on July 24, 2017, 07:33:16 pm
This was completed with the help of Paolo in time for 17.7.

Thanks!