try to make certificate addition/removal reloadable in some cases

connection_manager.go

@@ -476,8 +476,8 @@ func (cm *connectionManager) swapPrimary(current, primary *HostInfo) {
 }
 
 // isInvalidCertificate will check if we should destroy a tunnel if pki.disconnect_invalid is true and
-// the certificate is no longer valid. Block listed certificates will skip the pki.disconnect_invalid
-// check and return true.
+// the certificate is no longer valid, or if we no longer have a certificate of the same version as the remote.
+// Blocklisted certificates will skip the pki.disconnect_invalid check and return true.
 func (cm *connectionManager) isInvalidCertificate(now time.Time, hostinfo *HostInfo) bool {
 	remoteCert := hostinfo.GetCert()
 	if remoteCert == nil {
@@ -488,20 +488,40 @@ func (cm *connectionManager) isInvalidCertificate(now time.Time, hostinfo *HostI
 	err := caPool.VerifyCachedCertificate(now, remoteCert)
 	if err == nil {
 		return false
-	}
-
-	if !cm.intf.disconnectInvalid.Load() && err != cert.ErrBlockListed {
+	} else if err == cert.ErrBlockListed { //avoiding errors.Is for speed
 		// Block listed certificates should always be disconnected
-		return false
-	}
-
+		hostinfo.logger(cm.l).WithError(err).
+			WithField("fingerprint", remoteCert.Fingerprint).
+			Info("Remote certificate is blocked, tearing down the tunnel")
+		return true
+	} else if cm.intf.disconnectInvalid.Load() {
 		hostinfo.logger(cm.l).WithError(err).
 			WithField("fingerprint", remoteCert.Fingerprint).
 			Info("Remote certificate is no longer valid, tearing down the tunnel")
-
 		return true
 	}
 
+	//check that we still have a cert version in common with this connection. If we do not, disconnect.
+	remoteVersion := remoteCert.Certificate.Version()
+	cs := cm.intf.pki.getCertState()
+	out := false
+	switch remoteVersion {
+	case cert.Version1:
+		out = cs.v1Cert == nil
+	case cert.Version2:
+		out = cs.v2Cert == nil
+	default:
+		out = true
+	}
+
+	if out {
+		hostinfo.logger(cm.l).WithField("fingerprint", remoteCert.Fingerprint).
+			WithField("version", remoteVersion).
+			Info("We no longer have a certificate in common with remote, tearing down the tunnel")
+	}
+	return out
+}
+
 func (cm *connectionManager) sendPunch(hostinfo *HostInfo) {
 	if !cm.punchy.GetPunch() {
 		// Punching is disabled

pki.go

@@ -100,9 +100,8 @@ func (p *PKI) reloadCerts(c *config.C, initial bool) *util.ContextualError {
 		currentState := p.cs.Load()
 		if newState.v1Cert != nil {
 			if currentState.v1Cert == nil {
-				return util.NewContextualError("v1 certificate was added, restart required", nil, err)
-			}
-
+				//adding certs is fine, actually. Networks-in-common confirmed in newCertState().
+			} else {
 				// did IP in cert change? if so, don't set
 				if !slices.Equal(currentState.v1Cert.Networks(), newState.v1Cert.Networks()) {
 					return util.NewContextualError(
@@ -119,17 +118,13 @@ func (p *PKI) reloadCerts(c *config.C, initial bool) *util.ContextualError {
 						nil,
 					)
 				}
-
-		} else if currentState.v1Cert != nil {
-			//TODO: CERT-V2 we should be able to tear this down
-			return util.NewContextualError("v1 certificate was removed, restart required", nil, err)
+			}
 		}
 
 		if newState.v2Cert != nil {
 			if currentState.v2Cert == nil {
-				return util.NewContextualError("v2 certificate was added, restart required", nil, err)
-			}
-
+				//adding certs is fine, actually
+			} else {
 				// did IP in cert change? if so, don't set
 				if !slices.Equal(currentState.v2Cert.Networks(), newState.v2Cert.Networks()) {
 					return util.NewContextualError(
@@ -146,9 +141,21 @@ func (p *PKI) reloadCerts(c *config.C, initial bool) *util.ContextualError {
 						nil,
 					)
 				}
+			}
 
 		} else if currentState.v2Cert != nil {
-			return util.NewContextualError("v2 certificate was removed, restart required", nil, err)
+			//newState.v1Cert is non-nil bc empty certstates aren't permitted
+			if newState.v1Cert == nil {
+				return util.NewContextualError("v1 and v2 certs are nil, this should be impossible", nil, err)
+			}
+			//if we're going to v1-only, we need to make sure we didn't orphan any v2-cert vpnaddrs
+			if !slices.Equal(currentState.v2Cert.Networks(), newState.v1Cert.Networks()) {
+				return util.NewContextualError(
+					"Removing a V2 cert is not permitted unless it has identical networks to the new V1 cert",
+					m{"new_networks": newState.v1Cert.Networks(), "old_networks": currentState.v2Cert.Networks()},
+					nil,
+				)
+			}
 		}
 
 		// Cipher cant be hot swapped so just leave it at what it was before