Resolve some todos (#1274)

connection_manager.go

@@ -426,17 +426,17 @@ func (n *connectionManager) shouldSwapPrimary(current, primary *HostInfo) bool {
 	// If we are here then we have multiple tunnels for a host pair and neither side believes the same tunnel is primary.
 	// Let's sort this out.
 
-	//TODO: current.vpnIp should become an array of vpnIps
+	// Only one side should swap because if both swap then we may never resolve to a single tunnel.
+	// vpn addr is static across all tunnels for this host pair so lets
+	// use that to determine if we should consider swapping.
 	if current.vpnAddrs[0].Compare(n.intf.myVpnAddrs[0]) < 0 {
-		// Only one side should flip primary because if both flip then we may never resolve to a single tunnel.
-		// vpn ip is static across all tunnels for this host pair so lets use that to determine who is flipping.
-		// The remotes vpn ip is lower than mine. I will not flip.
+		// Their primary vpn addr is less than mine. Do not swap.
 		return false
 	}
 
-	//TODO: we should favor v2 over v1 certificates if configured to send them
-
-	crt := n.intf.pki.getCertificate(current.ConnectionState.myCert.Version())
+	crt := n.intf.pki.getCertState().getCertificate(current.ConnectionState.myCert.Version())
+	// If this tunnel is using the latest certificate then we should swap it to primary for a bit and see if things
+	// settle down.
 	return bytes.Equal(current.ConnectionState.myCert.Signature(), crt.Signature())
 }
 
@@ -495,13 +495,14 @@ func (n *connectionManager) sendPunch(hostinfo *HostInfo) {
 }
 
 func (n *connectionManager) tryRehandshake(hostinfo *HostInfo) {
-	crt := n.intf.pki.getCertificate(hostinfo.ConnectionState.myCert.Version())
-	if bytes.Equal(hostinfo.ConnectionState.myCert.Signature(), crt.Signature()) {
+	cs := n.intf.pki.getCertState()
+	curCrt := hostinfo.ConnectionState.myCert
+	myCrt := cs.getCertificate(curCrt.Version())
+	if curCrt.Version() >= cs.defaultVersion && bytes.Equal(curCrt.Signature(), myCrt.Signature()) == true {
+		// The current tunnel is using the latest certificate and version, no need to rehandshake.
 		return
 	}
 
-	//TODO: we should favor v2 over v1 certificates if configured to send them
-
 	n.l.WithField("vpnAddrs", hostinfo.vpnAddrs).
 		WithField("reason", "local certificate is not current").
 		Info("Re-handshaking with remote")

control.go

@@ -133,9 +133,9 @@ func (c *Control) ListHostmapIndexes(pendingMap bool) []ControlHostInfo {
 func (c *Control) GetCertByVpnIp(vpnIp netip.Addr) cert.Certificate {
 	_, found := c.f.myVpnAddrsTable.Lookup(vpnIp)
 	if found {
-		//TODO: we might have 2 certs....
-		//TODO: this should return our latest version cert
-		return c.f.pki.getDefaultCertificate().Copy()
+		// Only returning the default certificate since its impossible
+		// for any other host but ourselves to have more than 1
+		return c.f.pki.getCertState().GetDefaultCertificate().Copy()
 	}
 	hi := c.f.hostMap.QueryVpnAddr(vpnIp)
 	if hi == nil {
@@ -228,13 +228,9 @@ func (c *Control) CloseTunnel(vpnIp netip.Addr, localOnly bool) bool {
 // the int returned is a count of tunnels closed
 func (c *Control) CloseAllTunnels(excludeLighthouses bool) (closed int) {
 	//TODO: this is probably better as a function in ConnectionManager or HostMap directly
-	lighthouses := c.f.lightHouse.GetLighthouses()
-
 	shutdown := func(h *HostInfo) {
-		if excludeLighthouses {
-			if _, ok := lighthouses[h.vpnAddrs[0]]; ok {
-				return
-			}
+		if excludeLighthouses && c.f.lightHouse.IsAnyLighthouseAddr(h.vpnAddrs) {
+			return
 		}
 		c.f.send(header.CloseTunnel, 0, h.ConnectionState, h, []byte{}, make([]byte, 12, 12), make([]byte, mtu))
 		c.f.closeTunnel(h)

firewall.go

@@ -23,7 +23,6 @@ import (
 )
 
 type FirewallInterface interface {
-	//TODO: name these better addr, localAddr. Are they vpnAddrs?
 	AddRule(incoming bool, proto uint8, startPort int32, endPort int32, groups []string, host string, addr, localAddr netip.Prefix, caName string, caSha string) error
 }
 

interface.go

@@ -419,7 +419,7 @@ func (f *Interface) emitStats(ctx context.Context, i time.Duration) {
 			f.firewall.EmitStats()
 			f.handshakeManager.EmitStats()
 			udpStats()
-			certExpirationGauge.Update(int64(f.pki.getDefaultCertificate().NotAfter().Sub(time.Now()) / time.Second))
+			certExpirationGauge.Update(int64(f.pki.getCertState().GetDefaultCertificate().NotAfter().Sub(time.Now()) / time.Second))
 			//TODO: we should also report the default certificate version
 		}
 	}

overlay/tun_windows.go

@@ -239,11 +239,12 @@ func (t *winTun) Close() error {
 	luid := winipcfg.LUID(t.tun.LUID())
 	_ = luid.FlushRoutes(windows.AF_INET)
 	_ = luid.FlushIPAddresses(windows.AF_INET)
-	/* We don't support IPV6 yet
+
 	_ = luid.FlushRoutes(windows.AF_INET6)
 	_ = luid.FlushIPAddresses(windows.AF_INET6)
-	*/
+
 	_ = luid.FlushDNS(windows.AF_INET)
+	_ = luid.FlushDNS(windows.AF_INET6)
 
 	return t.tun.Close()
 }

pki.go

@@ -70,16 +70,6 @@ func (p *PKI) getCertState() *CertState {
 	return p.cs.Load()
 }
 
-// TODO: We should remove this
-func (p *PKI) getDefaultCertificate() cert.Certificate {
-	return p.cs.Load().GetDefaultCertificate()
-}
-
-// TODO: We should remove this
-func (p *PKI) getCertificate(v cert.Version) cert.Certificate {
-	return p.cs.Load().getCertificate(v)
-}
-
 func (p *PKI) reload(c *config.C, initial bool) error {
 	err := p.reloadCerts(c, initial)
 	if err != nil {
@@ -300,7 +290,6 @@ func newCertStateFromConfig(c *config.C) (*CertState, error) {
 		// Load the certificate
 		crt, rawCert, err = loadCertificate(rawCert)
 		if err != nil {
-			//TODO: check error
 			return nil, err
 		}