commit ba404a60ffd9e9a6143a3f6f7e57e400607a5cae
author Jan Schär <jan@monogon.tech> Thu Jul 11 10:46:27 2024 +0200
committer Jan Schär <jan@monogon.tech> Mon Jul 15 11:59:30 2024 +0000
tree 9fe27c2e435dab9ab0927b94ba9ee48d2e2e31b5
parent f69d84b0f2b8b363f42ed6a3c09523b75d002f94

m/n/c/network: fix panic when DHCP lease expires

The statusCallback of the network service previously accessed
new.AssignedIP without checking if new is nil, which caused a panic when
the DHCP lease expired. When subsequently the DHCP service was restarted
and a new lease obtained, CoreDNS was left without upstream servers
configured. The reason for this is that just before the panic, CoreDNS
was configured with an empty list of upstreams, but the lease field of
the DHCP service was not updated. When the lease callback was called
again with the new lease, old and new lease had the same DNS servers, so
CoreDNS was not configured to use the upstreams.

To fix the panic, this adds a check for a nil lease before accessing
AssignedIP. I looked if all consumers of the ExternalAddress Status can
handle nil, and added a nil check in the statuspush worker. The
apiserver stops when the lease is lost, and starts again once it is
reacquired; I'm not sure if this is the intended behavior.

The DNS problem occured because the old lease passed to the callback was
not the last lease that the callback had seen, and it then mistakenly
suppressed the update. In general, a callback cannot rely on the old
lease being the last lease that the callback has been called with. For
example, when a callback earlier in the Compose chain returns an error,
later callbacks are not called, so a callback may not see all lease
changes. Because the old lease parameter cannot be trusted, I removed
it. Callbacks which need the previous lease should keep track of it
themselves.

For manually testing lease expiry, I modified
metropolis/test/nanoswitch/nanoswitch.go like this:

+		start := time.Now()
 		server, err := server4.NewServer(link.Attrs().Name, &laddr, func(conn net.PacketConn, peer net.Addr, m *dhcpv4.DHCPv4) {
 			if m == nil {
 				return
 			}
+			if start.Add(50*time.Second).Before(time.Now()) && start.Add(90*time.Second).After(time.Now()) {
+				supervisor.Logger(ctx).Infof("Dropping DHCP packet")
+				return
+			}
 			reply, err := dhcpv4.NewReplyFromRequest(m)

Change-Id: Ifa0c039769c37ee53033ce013eed4f1af6f02142
Reviewed-on: https://review.monogon.dev/c/monogon/+/3214
Tested-by: Jenkins CI
Reviewed-by: Lorenz Brun <lorenz@monogon.tech>