From d164cb3a82aa61039f455356c6a4f197b0d228e0 Mon Sep 17 00:00:00 2001 From: Antanas Gadeikis Date: Thu, 20 May 2021 14:15:30 +0300 Subject: [PATCH 1/2] An attemt to fix handshakeAttempts atomic counter: 0 means no HandshakeInitiation (HI) is in progress. Positive number - how many HIs have been already send. This patch also fixes race condition between HI retries and data sent watchdog timer(timersDataSent vs expiredRetransmitHandshake) Signed-off-by: Antanas Gadeikis --- device/send.go | 2 +- device/timers.go | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/device/send.go b/device/send.go index e07df1bae..88fdf1d4c 100644 --- a/device/send.go +++ b/device/send.go @@ -89,7 +89,7 @@ func (peer *Peer) SendKeepalive() { func (peer *Peer) SendHandshakeInitiation(isRetry bool) error { if !isRetry { - atomic.StoreUint32(&peer.timers.handshakeAttempts, 0) + atomic.StoreUint32(&peer.timers.handshakeAttempts, 1) } peer.handshake.mutex.RLock() diff --git a/device/timers.go b/device/timers.go index ee191e55b..4fe3edddf 100644 --- a/device/timers.go +++ b/device/timers.go @@ -78,6 +78,9 @@ func expiredRetransmitHandshake(peer *Peer) { if atomic.LoadUint32(&peer.timers.handshakeAttempts) > MaxTimerHandshakes { peer.device.log.Verbosef("%s - Handshake did not complete after %d attempts, giving up", peer, MaxTimerHandshakes+2) + // Reset attempts count + atomic.StoreUint32(&peer.timers.handshakeAttempts, 0) + if peer.timersActive() { peer.timers.sendKeepalive.Del() } @@ -95,7 +98,7 @@ func expiredRetransmitHandshake(peer *Peer) { } } else { atomic.AddUint32(&peer.timers.handshakeAttempts, 1) - peer.device.log.Verbosef("%s - Handshake did not complete after %d seconds, retrying (try %d)", peer, int(RekeyTimeout.Seconds()), atomic.LoadUint32(&peer.timers.handshakeAttempts)+1) + peer.device.log.Verbosef("%s - Handshake did not complete after %d seconds, retrying (try %d)", peer, int(RekeyTimeout.Seconds()), atomic.LoadUint32(&peer.timers.handshakeAttempts)) /* We clear the endpoint address src address, in case this is the cause of trouble. */ peer.Lock() @@ -141,10 +144,18 @@ func expiredPersistentKeepalive(peer *Peer) { } } +/* Returns true if HandshakeInitiation is already started (and not yet finished) */ +func (peer *Peer) handshakeInProgress() bool { + return atomic.LoadUint32(&peer.timers.handshakeAttempts) > 0 +} + /* Should be called after an authenticated data packet is sent. */ func (peer *Peer) timersDataSent() { if peer.timersActive() && !peer.timers.newHandshake.IsPending() { - peer.timers.newHandshake.Mod(KeepaliveTimeout + RekeyTimeout + time.Millisecond*time.Duration(rand.Int31n(RekeyTimeoutJitterMaxMs))) + // Do not restart handshake because of staled data traffic, if it is already in progress + if !peer.handshakeInProgress() { + peer.timers.newHandshake.Mod(KeepaliveTimeout + RekeyTimeout + time.Millisecond*time.Duration(rand.Int31n(RekeyTimeoutJitterMaxMs))) + } } } From 425aa976fc4e1f918e069b9fc04c67401eaba4f2 Mon Sep 17 00:00:00 2001 From: Antanas Gadeikis Date: Thu, 20 May 2021 15:31:39 +0300 Subject: [PATCH 2/2] Do not flood new HandshakeInitiations, if some is already in progress Signed-off-by: Antanas Gadeikis --- device/send.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/device/send.go b/device/send.go index 88fdf1d4c..9f350a03c 100644 --- a/device/send.go +++ b/device/send.go @@ -193,7 +193,10 @@ func (peer *Peer) keepKeyFreshSending() { } nonce := atomic.LoadUint64(&keypair.sendNonce) if nonce > RekeyAfterMessages || (keypair.isInitiator && time.Since(keypair.created) > RekeyAfterTime) { - peer.SendHandshakeInitiation(false) + // Do not send new HandshakeInitiation, if handshake retry is already in progress + if !peer.handshakeInProgress() { + peer.SendHandshakeInitiation(false) + } } } @@ -298,7 +301,10 @@ top: keypair := peer.keypairs.Current() if keypair == nil || atomic.LoadUint64(&keypair.sendNonce) >= RejectAfterMessages || time.Since(keypair.created) >= RejectAfterTime { - peer.SendHandshakeInitiation(false) + // Do not send new HandshakeInitiation, if handshake retry is already in progress + if !peer.handshakeInProgress() { + peer.SendHandshakeInitiation(false) + } return }