diff --git a/connection_manager.go b/connection_manager.go index 7242c72..08937e6 100644 --- a/connection_manager.go +++ b/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,18 +488,38 @@ 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 } - hostinfo.logger(cm.l).WithError(err). - WithField("fingerprint", remoteCert.Fingerprint). - Info("Remote certificate is no longer valid, tearing down the tunnel") + //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 + } - return 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) { diff --git a/pki.go b/pki.go index e71d326..0371b26 100644 --- a/pki.go +++ b/pki.go @@ -100,55 +100,62 @@ 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( + "Networks in new cert was different from old", + m{"new_networks": newState.v1Cert.Networks(), "old_networks": currentState.v1Cert.Networks()}, + nil, + ) + } - // did IP in cert change? if so, don't set - if !slices.Equal(currentState.v1Cert.Networks(), newState.v1Cert.Networks()) { - return util.NewContextualError( - "Networks in new cert was different from old", - m{"new_networks": newState.v1Cert.Networks(), "old_networks": currentState.v1Cert.Networks()}, - nil, - ) + if currentState.v1Cert.Curve() != newState.v1Cert.Curve() { + return util.NewContextualError( + "Curve in new cert was different from old", + m{"new_curve": newState.v1Cert.Curve(), "old_curve": currentState.v1Cert.Curve()}, + nil, + ) + } } - - if currentState.v1Cert.Curve() != newState.v1Cert.Curve() { - return util.NewContextualError( - "Curve in new cert was different from old", - m{"new_curve": newState.v1Cert.Curve(), "old_curve": currentState.v1Cert.Curve()}, - 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( + "Networks in new cert was different from old", + m{"new_networks": newState.v2Cert.Networks(), "old_networks": currentState.v2Cert.Networks()}, + nil, + ) + } - // did IP in cert change? if so, don't set - if !slices.Equal(currentState.v2Cert.Networks(), newState.v2Cert.Networks()) { - return util.NewContextualError( - "Networks in new cert was different from old", - m{"new_networks": newState.v2Cert.Networks(), "old_networks": currentState.v2Cert.Networks()}, - nil, - ) - } - - if currentState.v2Cert.Curve() != newState.v2Cert.Curve() { - return util.NewContextualError( - "Curve in new cert was different from old", - m{"new_curve": newState.v2Cert.Curve(), "old_curve": currentState.v2Cert.Curve()}, - nil, - ) + if currentState.v2Cert.Curve() != newState.v2Cert.Curve() { + return util.NewContextualError( + "Curve in new cert was different from old", + m{"new_curve": newState.v2Cert.Curve(), "old_curve": currentState.v2Cert.Curve()}, + 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