Closed choopm closed 6 years ago
Thanks for taking the time to contribute to netctl
!
The addition looks okay. Any chance this will break someones setup?
The addition looks okay. Any chance this will break someones setup?
I don't see any reason to leak private addresses to mobile providers and someone depending on that, I think it's safe to merge.
If this breaks someones setup though, they would search for the message IPCP: timeout sending Config-Requests
and find this PR / commit message.
That is not good enough. We don't want someone to end up without a connection just because the defaults of netctl
changed. If there is a reasonable scenario where you would not want noipdefault
in your configuration, then the current PR should be changed into an extra flag. However, if there is no such scenario, I would rather not add such a flag.
This is now a profile option, defaulting to 'no' and shipped with the examples as 'yes'. Should be good now, won't break existing profiles/behaviour.
@zoqaeski This PR suggests to add noipdefault
to the pppd options of the mobile_ppp
connection type. I looked into the manual and believe that not having this option is really only meaningful if your options include a local IP address. Is this correct? If so, the addition can be done unconditionally. Otherwise, it has to be added as a configuration flag.
I can imagine someone using pppd to establish a connection using fixed known endpoints. In this case you'd have to configure the addresses or depend on the default behaviour. In our case (mobile_ppp), we're establishing the link without knowing any IP configurations. In my opinion there's no reason to request any specific address when establishing a mobile_ppp connection and we should have it fixed to request 0.0.0.0
, since some providers don't reply with a ConfNak when requesting private addresses.
@joukewitteveen Did you try it yourself? What does your IPCP negotiation look like? Are you receiving ConfNak for your private addresses too?
edit: @zoqaeski uses it too: netcfg-ppp-mobile
Any updates on this?
Yes, I pulled one of your commits locally (and changed it slightly). The next version of netctl
will set noipdefault
by default. As far as I understand, not setting this option only makes sense in situations where you can be explicit about the local IP. If this new default causes trouble, we should implement more elaborate support for setting the local IP through e.g. IP=
. The noipdefault
option would then only be included when IP=
is unset.
In the future, try to squash typo commits.
Anyways, thank you for making netctl better!
f13c69f1136a9b03138744b53b368807029cbb2c
According to the docs, pppd will try to determine the local IP address and include it in the IPCP ConfReq. This leads to any local IP being sent in the request as seen in journalctl (added
debug dump logfd 2
too):As you can see, some providers won't reply with a IPCP ConfNak, so this times out and no IP is assigned. I'm on Vodafone.de.
This PR disables the default behaviour and will include
0.0.0.0
in the IPCP ConfReq:Signed-off-by: Christoph Hoopmann christophhoopmann@gmail.com