From 37fb5d2fc34dd7df9d09fe4d9fc2541e80a1b675 Mon Sep 17 00:00:00 2001
From: oto313 <otodusek@seznam.cz>
Date: Mon, 17 Mar 2025 19:06:54 +0100
Subject: [PATCH] Always send KeepAlives (#767)

Pion incorrectly resets the consent timer when sending any traffic.
The consent timer must only be reset on STUN traffic.

RFC 7675
> Consent expires after 30 seconds.  That is, if a valid STUN binding
> response has not been received from the remote peer's transport
> address in 30 seconds, the endpoint MUST cease transmission on that
> 5-tuple.  STUN consent responses received after consent expiry do not
> re-establish consent and may be discarded or cause an ICMP error.
---
 agent.go      |  4 +---
 agent_test.go | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/agent.go b/agent.go
index 7110c32..87949b1 100644
--- a/agent.go
+++ b/agent.go
@@ -594,9 +594,7 @@ func (a *Agent) checkKeepalive() {
 		return
 	}
 
-	if (a.keepaliveInterval != 0) &&
-		((time.Since(selectedPair.Local.LastSent()) > a.keepaliveInterval) ||
-			(time.Since(selectedPair.Remote.LastReceived()) > a.keepaliveInterval)) {
+	if a.keepaliveInterval != 0 {
 		// We use binding request instead of indication to support refresh consent schemas
 		// see https://tools.ietf.org/html/rfc7675
 		a.selector.PingCandidate(selectedPair.Local, selectedPair.Remote)
diff --git a/agent_test.go b/agent_test.go
index 8fd289a..1ec75c1 100644
--- a/agent_test.go
+++ b/agent_test.go
@@ -2121,3 +2121,42 @@ func TestSetCandidatesUfrag(t *testing.T) {
 		require.Equal(t, agent.localUfrag, ext.Value)
 	}
 }
+
+func TestAlwaysSentKeepAlive(t *testing.T) { //nolint:cyclop
+	defer test.CheckRoutines(t)()
+
+	// Avoid deadlocks?
+	defer test.TimeOut(1 * time.Second).Stop()
+
+	agent, err := NewAgent(&AgentConfig{})
+	if err != nil {
+		t.Fatalf("Failed to create agent: %s", err)
+	}
+	defer func() {
+		require.NoError(t, agent.Close())
+	}()
+
+	log := logging.NewDefaultLoggerFactory().NewLogger("agent")
+	agent.selector = &controllingSelector{agent: agent, log: log}
+	pair := makeCandidatePair(t)
+	if s, ok := pair.Local.(*CandidateHost); ok {
+		s.conn = &fakenet.MockPacketConn{}
+	} else {
+		t.Fatalf("Invalid local candidate")
+	}
+	agent.setSelectedPair(pair)
+
+	pair.Remote.seen(false)
+
+	lastSent := pair.Local.LastSent()
+	agent.checkKeepalive()
+	newLastSent := pair.Local.LastSent()
+	require.NotEqual(t, lastSent, newLastSent)
+	lastSent = newLastSent
+
+	// sleep, so there is difference in sent time of local candidate
+	time.Sleep(10 * time.Millisecond)
+	agent.checkKeepalive()
+	newLastSent = pair.Local.LastSent()
+	require.NotEqual(t, lastSent, newLastSent)
+}