Bellsouth/AT&T PPPoE broken
-
Warning: Object directory not changed from original /usr/ports/net/mpd/work/mpd-3.18/src
cc -O2 -fno-strict-aliasing -pipe -DPATH_CONF_DIR="/usr/local/etc/mpd" -DSYSLOG_FACILITY=LOG_DAEMON -DMPD_VERSION='"3.18 (root@freebsd6.geekgod.com 01:13 31-Dec-2007)"' -g -Wall -Wcast-align -Wchar-subscripts -Wformat -Winline -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wwrite-strings -DPHYSTYPE_MODEM -DPHYSTYPE_UDP -DPHYSTYPE_NG_SOCKET -DPHYSTYPE_PPTP '-DPPTP_VENDOR_NAME="FreeBSD mpd-3.18"' -DPHYSTYPE_PPPOE -DENCRYPTION_DES -DENCRYPTION_MPPE -DCOMPRESSION_MPPC -c ipcp.c
ipcp.c: In functionIpcpBuildConfigReq': ipcp.c:352: error:FSM_MAXNAK' undeclared (first use in this function)
ipcp.c:352: error: (Each undeclared identifier is reported only once
ipcp.c:352: error: for each function it appears in.)
*** Error code 1Stop in /usr/ports/net/mpd/work/mpd-3.18/src.
-
I must have been tired last night. In ipcp.c, it includes fsm.h, but that constant is defined on line 23 of fsm.c.
We could move the Definition of that constant from fsm.c to fsm.h. If (for some odd reason) that didn't work, we could always hard-wire the if statement that I suggested to 9 instead of (FSM_MAXNAK - 1).. (Yea, ugly, I know)
Or just comment out the first "if" in that function (line 344 on my copy):
if (!IPCP_REJECTED(ipcp, TY_IPADDR) || ipcp->want_addr.s_addr == 0)so that the next line is always executed:
cp = FsmConfValue(cp, TY_IPADDR, 4, &ipcp->want_addr.s_addr); -
I spent some time looking at the code last night, and I think I know why it's failing. The problem section is definitely this part in ipcp.c:
/* Put in my desired IP address */ if (!IPCP_REJECTED(ipcp, TY_IPADDR) || ipcp->want_addr.s_addr == 0) cp = FsmConfValue(cp, TY_IPADDR, 4, &ipcp->want_addr.s_addr);You can hack it to make it work by removing the if statement so that the FsmConfValue line is always executed. The if statement is essentially checking to see if the IPADDR option has been rejected by the server. If not, it should add its desired ipaddr to the request.
The reason this is failing is because Bellsouth is rejecting the SECONDARYDNS option, which has a value of 131. MPD stores the rejected options in a bit mask. Look at the IPCP_REJECTED and IPCP_PEER_REJ macros towards the top of ipcp.c.
The problem is that when you left shift 131 places (the value of TY_SECONDARYDNS), it's going to wrap the peer_reject variable a bunch of times, with the end result the same as if you had shifted to the left by 3, which just so happens to be the value of TY_IPADDR.
So, to make a long story short, if the server rejects the SECONDARYDNS option, the MPD code thinks the IPADDR option has been rejected. The way it keeps track of rejected options is fundamentally broken. I could probably come up with a patch to redo the IPCP_REJECTED/IPCP_PEER_REJ macros within a couple of days (unless there is an MPD dev reading this, who could probably do it a lot faster, and integrate it into the main MPD codebase)
The question I have is: is it really so bad to take out that if statement so that the desired ipaddr is always added to the request? I can't think of a reason why the server would ever reject the IPADDR option. I don't really know the ins and outs of IPCP though.
-
If I've read the code correctly, that should wait until the last attempt before it would normally fail, at which time it would insert the IP Address field in the Configuration Request, regardless of if it has been previously rejected. This would be less likely to break MPD, if this code is used for other purposes. (I believe FSM_MAXNAK is a valid variable here, but as I indicated above, my C is rusty.)
I'm not sure this is the right approach… it should actually be putting the IP in the first request after the first NAK. This is what XP's PPPoE client does:
http://attenuated.org/~greg/mpd/xp_working.txt
-
I'm not sure this is the right approach… it should actually be putting the IP in the first request after the first NAK. This is what XP's PPPoE client does:
http://attenuated.org/~greg/mpd/xp_working.txt
I'm not sure it is the right approach either, but thought that it would be less likely to cause issues if this code is used for some purpose other than what we are doing with it here. I believe that MPD can run in a server mode as well, and perhaps for other purposes. I do not know if Monowall or pfSense are taking advantage of any of those features, though.
Your previous post indicates the issue is with the way MPD determines what is rejected. I think you are spot-on here, as that code segment you posted is the what I homed in on as well. Fixing the underlying cause would be the best option, but that would be best handled by someone intimately familiar with the MPD code, as I don't think it would be trivial. If someone can verify that MPD's only use in pfSense is as a PPPoE client, then the suggestion to simply take the "if" statement out would probably take care of everything in short order.
-
Try editing config.xml and in the pppoe section add:
<dnsnosec>IE: from /etc/inc/interfaces.inc:
if (!isset($config['pppoe']['dnsnosec'])) {
$mpdconf .= << <eod<br>set ipcp enable req-sec-dnsEOD;</eod<br></dnsnosec>
-
Scott, you rock! This minor config change has me up and running!
Perhaps this can be put in as a webGui option since this is becoming a common issue.
-
Excellent! That got me up and running as well. Thanks for finding that.
I went ahead and submitted a bug ticket to the MPD project so that they are at least aware of the problem:
http://sourceforge.net/tracker/index.php?func=detail&aid=1861740&group_id=14145&atid=114145I also whipped up a patch to add an option to the webgui. This patch is against m0n0wall 1.3b7. It should be fairly easy to adapt to pfsense. I'm going to post this to the monowall development mailing list. Hopefully it'll get included in the next release:
--- interfaces_wan.php.orig 2008-01-01 13:04:15.000000000 -0500 +++ interfaces_wan.php 2008-01-01 13:35:52.000000000 -0500 @@ -38,6 +38,7 @@ $pconfig['username'] = $config['pppoe']['username']; $pconfig['password'] = $config['pppoe']['password']; $pconfig['provider'] = $config['pppoe']['provider']; +$pconfig['pppoe_dnsnosec'] = isset($config['pppoe']['dnsnosec']); $pconfig['pppoe_dialondemand'] = isset($config['pppoe']['ondemand']); $pconfig['pppoe_idletimeout'] = $config['pppoe']['timeout']; @@ -179,6 +180,7 @@ unset($config['pppoe']['username']); unset($config['pppoe']['password']); unset($config['pppoe']['provider']); + unset($config['pppoe']['dnsnosec']); unset($config['pppoe']['ondemand']); unset($config['pppoe']['timeout']); unset($config['pptp']['username']); @@ -208,6 +210,7 @@ $config['pppoe']['username'] = $_POST['username']; $config['pppoe']['password'] = $_POST['password']; $config['pppoe']['provider'] = $_POST['provider']; + $config['pppoe']['dnsnosec'] = $_POST['pppoe_dnsnosec'] ? true : false; $config['pppoe']['ondemand'] = $_POST['pppoe_dialondemand'] ? true : false; $config['pppoe']['timeout'] = $_POST['pppoe_idletimeout']; } else if ($_POST['type'] == "PPTP") { @@ -274,6 +277,7 @@ document.iform.username.disabled = 1; document.iform.password.disabled = 1; document.iform.provider.disabled = 1; + document.iform.pppoe_dnsnosec.disabled = 1; document.iform.pppoe_dialondemand.disabled = 1; document.iform.pppoe_idletimeout.disabled = 1; document.iform.ipaddr.disabled = 0; @@ -297,6 +301,7 @@ document.iform.username.disabled = 1; document.iform.password.disabled = 1; document.iform.provider.disabled = 1; + document.iform.pppoe_dnsnosec.disabled = 1; document.iform.pppoe_dialondemand.disabled = 1; document.iform.pppoe_idletimeout.disabled = 1; document.iform.ipaddr.disabled = 1; @@ -320,6 +325,7 @@ document.iform.username.disabled = 0; document.iform.password.disabled = 0; document.iform.provider.disabled = 0; + document.iform.pppoe_dnsnosec.disabled = 0; document.iform.pppoe_dialondemand.disabled = 0; if (document.iform.pppoe_dialondemand.checked || enable_change) { document.iform.pppoe_idletimeout.disabled = 0; @@ -347,6 +353,7 @@ document.iform.username.disabled = 1; document.iform.password.disabled = 1; document.iform.provider.disabled = 1; + document.iform.pppoe_dnsnosec.disabled = 1; document.iform.pppoe_dialondemand.disabled = 1; document.iform.pppoe_idletimeout.disabled = 1; document.iform.ipaddr.disabled = 1; @@ -374,6 +381,7 @@ document.iform.username.disabled = 1; document.iform.password.disabled = 1; document.iform.provider.disabled = 1; + document.iform.pppoe_dnsnosec.disabled = 1; document.iform.pppoe_dialondemand.disabled = 1; document.iform.pppoe_idletimeout.disabled = 1; document.iform.ipaddr.disabled = 1; @@ -501,6 +509,13 @@ Hint: this field can usually be left empty + + Secondary DNS + > + **Disable secondary DNS request** + This option disables the request for a secondary DNS server in the PPPoE connection. Workaround for some BellSouth/AT&T DSL users. + + Dial on demand onClick="enable_change(false)" > -
Scott Rocks. Thanks a bunch. I am honored to be in a forum with such genious. You are da bomb. Oh and if you can't tell the fix works great for me.
-
Great, glad it worked out. We are now exploring what to do about this. I almost half inclined to ship with this option on by default and if someone needs multiple dns servers they can uncomment this.
Thoughts?
-
For now, I think having this option enabled by default would probably be fine.
In my mind, though, the best long term fix would be for the MPD dev's fix this little bug.
I've never heard of an ISP not giving you more than one DNS server before, which is probably why this problem has remained hidden until now.
As a side note, my employer uses BellSouth for their Internet service, and since I work in the network department there, I've been involved with issues courtesy of their DNS servers on numerous occasions. Perhaps they felt that only handing out a single DNS server (splitting up who gets handed which server) would half the traffic to their servers, possibly cutting down on the problems. This seems like a short-sited fix though, since that introduces more single points of failure for their customers.
If fixing their DNS issues was the goal of this change, I'd think a better answer would be to get a couple of F5 load balancers and a few more DNS servers. Those F5's aren't cheap, though… Hey - They could use pfSense instead! :)
-
Thanks so much for the help. This one hit me right after new years. Damn Bell South.
-
This issue has been fixed in MPD 5 and Manuel has back-ported the fix to MPD 3.18. He has a pre-release version that has been verified to work (without the dnsnosec option).
So, this issue sounds like it will be all taken care of very soon.
Paul
-
I've just commited a new mpd which contains his changes. Please check the snapshot server in about an hour or two from now.