There was no way to disable writing pidfiles before.
pidfiles are an unreliable method of tracking processes, anyway; process
supervisors are strongly recommended. If a pidfile is really needed, it
can be explicitly specified.
Before the exit code would be success and no error message would print,
and it required a bit of control flow tracing to determine what would
actually happen.
No direct functional change (unless the supervising process cares about
the return code of ndhc on exit).
If changing interface properties fails after getting a lease, it is
possible under some strange conditions for the failure to be
persistent. This seems to happen if the carrier cycles off and on
several times during ndhc initialization.
Since this issue is very hard to replicate, the most conservative
thing to do here is to simply have ndhc suicide itself so it can
be respawned by a process supervisor.
Logs of the issue in practice:
(carrier is down while the daemon is started here, it seems)
16:57:09.638979845 ndhc-ifch seccomp filter installed. Please disable seccomp if you
16:57:09.638989136 Discovering DHCP servers...
16:57:09.638991371 (send_dhcp_raw) carrier down; sendto would fail
16:57:09.638993318 Failed to send a discover request packet.
...
16:57:13.636519925 Discovering DHCP servers...
16:57:13.651462476 Received IP offer: X from server Y via Z
...
16:57:13.912592571 wan0: Gateway router set to: A
16:57:13.912607463 wan0: arp: Searching for dhcp server and gw addresses...
16:57:14.635532676 wan0: Carrier down.
17:04:32.983897760 wan0: arp: Still looking for gateway hardware address...
17:04:32.984158226 wan0: arp: Still looking for DHCP agent hardware address...
17:04:32.984781255 wan0: Interface is back. Revalidating lease...
17:04:32.985585501 wan0: arp: Gateway hardware address B
17:04:32.985590436 wan0: arp: DHCP agent hardware address C
17:04:38.234857403 wan0: arp: Still waiting for gateway to reply to arp ping...
17:04:38.235109016 wan0: arp: Still waiting for DHCP agent to reply to arp ping...
16:57:24.165620224 wan0: arp: Still waiting for gateway to reply to arp ping...
16:57:29.169621070 wan0: arp: DHCP agent and gateway didn't reply. Getting new lease.
16:57:29.217710616 wan0: Discovering DHCP servers...
16:57:29.249645130 wan0: Received IP offer: X from server Y via Z
16:57:29.249657203 wan0: Sending a selection request for X...
16:57:29.285632973 wan0: Received ACK: X from server Y via Z
16:57:29.297717159 wan0: arp: Probing for hosts that may conflict with our lease...
16:57:29.360249458 wan0: arp: Probing for hosts that may conflict with our lease...
16:57:29.435114526 wan0: arp: Probing for hosts that may conflict with our lease...
16:57:29.500473345 wan0: Lease of X obtained. Lease time is D seconds.
16:57:29.500485894 wan0: Failed to set the interface IP address and properties!
...
And the final two errors repeat. Restarting ndhc by hand instantly
fixes the issue.
So there's a lot going on -- bizzare clock skew, and carrier flickering
on and off.
This requires execute_buffer() and its callers to distinguish between
fatal and non-fatal errors. The -99 return value was already used for
non-recoverable errors that should force the daemon to restart, but the
execute_buffer() callers treated any non-success return as a fatal
error.
There a judgement call here on how to handle various error types. I
choose to assume that failures to set the IP address, netmask, broadcast
address, or default router are fatal errors. ndhc should be run from
process supervision, and this will trigger a daemon restart, which will
allow the machine to recover as soon as the problem (probably on the
dhcp server or local kernel state outside of ndhc's control) is
corrected.
This change corrects errors such as:
Discovering DHCP servers...
(process_client_socket) received invalid commands: 'carrier:;'
(send_dhcp_raw) carrier down; sendto would fail
Failed to send a discover request packet.
which happened if ndhc is started on a machine where the network
interface is down. After this change, ndhc should function as intended
by going to sleep until the carrier returns rather than terminating
itself to be restarted by the process supervisor until carrier returns.
Thus signed/unsigned conversion issues can't be a problem.
The extra range provided by ull isn't useful, either, since
we're dealing with uint32_t time_t seconds converted to ns,
which doesn't come close to exhausting the amount of post-epoch
ns that will be consumed until after 2100AD.
Lease times and arp timeouts are all calculated using long long,
but epoll takes its timeout argument as an int. Guard against
timeouts > INT_MAX but < UINT_MAX wrapping and causing spuriously
short timeouts when converted to a signed int.
This problem has been observed in the wild. Thanks to thypon
for a detailed strace that pointed me towards this issue.
Somewhere along the line it quit being set at the start of discovery
and was always 0. This is clearly not desired behavior.
Found by manual examination of packets while fuzzing the options
parser.
This corrects a bug where stale dhcp packets would get reprocessed,
causing very bad behavior; an issue that was introduced in the
coroutine conversion.
Some networks have multiple DHCP servers that don't respect the
serverid that is specified in DHCPREQUESTs.
Instead we simply check to see that the yiaddr matches.
While we're at it, ignore DHCP NAK during REQUESTING state. It
doesn't really make sense. Instead we should just wait for
timeout.
This change actually has no effect because incoming dhcp packets
that differ from our xid are dropped, so the xid is always changed
to the same value that it already has.
If ndhc were a high-performance program that handled lots of events,
this change would harm performance. But it is not, and it implicitly
believes that events come in one at a time. Processing batches
would make it harder to assure correctness while also never allocating
memory at runtime.
The previous structure was fine when everything was handled
immediately by callbacks, but it isn't now.
This change makes it much easier to reason about ndhc's behavior
and properly handle errors.
It is a very large changeset, but there is no way to make this
sort of change incrementally. Lease acquisition is tested to
work.
It is highly likely that some bugs were both introduced and
squashed here. Some obvious code cleanups will quickly follow.
This change paves the way for allowing ifch to notify the core ndhc
about failures. It would be far too difficult to reason about the
state machine if the requests to ifch were asynchronous.
Currently ndhc assumes that ifch requests never fail, but this
is not always true because of eg, rfkill.
If a packet send failed because the carrier went down without a
netlink notification, then assume the hardware carrier was lost while
the machine was suspended (eg, ethernet cable pulled during suspend).
Simulate a netlink carrier down event and freeze the dhcp state
machine until a netlink carrier up event is received.
The ARP code is not yet handling this issue everywhere, but the
window of opportunity for it to happen there is much shorter.
Linux will quietly proceed as if the data were sent even if the carrier
is down and nothing actually happened. There is still a tiny race
condition where the carrier could drop between the check and the actual
write, but we really can't do anything about that and it is a very
small race.
In order for this to work, the correct rfkill index must be specified
with the rfkill-idx option.
It might be possible to auto-detect the corresponding rfkill-idx option,
but I'm not sure if there's a guaranteed mapping between rfkill name and
interface name, as it seems that rfkills should represent phy devices
and not wlan devices.
The rfkill indexes can be found by checking
/sys/class/rfkill/rfkill<IDX>.
Mostly reverts the previous commit and instead teaches ndhc to properly
handle the case when it is communicating with a DHCP relay agent on
its local segment rather than directly with a DHCP server.
different segment.
The network fingerprinting would never complete if the DHCP server was
on a different segment before this change, since it would be impossible
for the ARP messages sent by ndhc to ever reach the DHCP server
(and vice-versa).
Now just give up trying to find the hardware address after two tries
and assume that the DHCP server cannot be reached by ARP.
An alternative would be to fingerprint the relay agent instead, but
to do so would require a lot more work as the giaddr field is only
meaningful in the client->server message path, not in the
server->client path. Thus it would require gathering the source IP
for DHCP replies sent by unicast or broadcast and ferrying along
this information to the ARP checking code where it would be used
in place of the DHCP server address.
This is entirely possible to do, but is quite a bit more work.
Without this change, it is possible for malicious UDP packets to
make the function read past the end of a buffer.
If this was ever a possibility in ndhc, the previous commit fixed
that issue, but there is no reason for udp_checksum() to have
such a subtle precondition to proper use. This change also makes
it easier to audit correctness.
This facility was added to Linux in early 2013. If it is not available,
the BPF will still be installed, but redundant checks will be performed
to guard against the BPF possibly being removed by an attacker.
Since the transfer is conceptually a move, this is the correct thing to
do and prevents sockets from spuriously hanging around forever and
eventually exhausting the per process limit on fds.
AF_UNIX SOCK_STREAM sockets between the master processes and each subprocess,
and poll for the HUP event.
At the same time, be specific about the events that are checked in epoll
when dispatching on an event.
The pipes wouldn't do this job anymore because they were unused and thus
never performed writes that would generate SIGPIPEs, so the pipes are
removed, too.
Nicholas J. Kain