From 82cfcda452569cc4d9cf1ae9cac4285ed5527146 Mon Sep 17 00:00:00 2001
From: Jonathan Lennox <jonathan.lennox@8x8.com>
Date: Wed, 25 Sep 2024 16:53:14 -0400
Subject: [PATCH] Fail a connectivity check request received after checks are
 stopped.

This should cause clients who've had NAT rebinding events to start failing their consent checks, so they reconnect.
---
 src/main/java/org/ice4j/ice/Agent.java        | 20 ++++++++++++++-----
 .../ice4j/ice/ConnectivityCheckClient.java    |  6 ++++++
 .../ice4j/ice/ConnectivityCheckServer.java    | 14 ++++++++++---
 3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/src/main/java/org/ice4j/ice/Agent.java b/src/main/java/org/ice4j/ice/Agent.java
index fb007042..3218a58c 100644
--- a/src/main/java/org/ice4j/ice/Agent.java
+++ b/src/main/java/org/ice4j/ice/Agent.java
@@ -1578,8 +1578,9 @@ public CandidatePair findCandidatePair(String localUFrag,
      * @param useCandidate indicates whether the incoming check
      * {@link org.ice4j.message.Request} contained the USE-CANDIDATE ICE
      * attribute.
+     * @return Whether the response to the check should get a success response
      */
-    protected void incomingCheckReceived(TransportAddress remoteAddress,
+    protected boolean incomingCheckReceived(TransportAddress remoteAddress,
                                          TransportAddress localAddress,
                                          long             priority,
                                          String           remoteUFrag,
@@ -1593,7 +1594,7 @@ protected void incomingCheckReceived(TransportAddress remoteAddress,
         {
             logger.info("No localAddress for this incoming checks: " +
                     localAddress);
-            return;
+            return false;
         }
 
         Component parentComponent = localCandidate.getParentComponent();
@@ -1639,9 +1640,10 @@ else if (state == IceProcessingState.FAILED)
 
                 // We have been started, and have not failed (yet). If this is
                 // a new pair, handle it (even if we have already completed).
-                triggerCheck(triggeredPair);
+                return triggerCheck(triggeredPair);
             }
         }
+        return true;
     }
 
     /**
@@ -1651,8 +1653,9 @@ else if (state == IceProcessingState.FAILED)
      *
      * @param triggerPair the pair containing the local and remote candidate
      * that we'd need to trigger a check for.
+     * @return Whether a triggered check was started
      */
-    private void triggerCheck(CandidatePair triggerPair)
+    private boolean triggerCheck(CandidatePair triggerPair)
     {
         //first check whether we already know about the remote address in case
         //we've just discovered a peer-reflexive candidate.
@@ -1702,7 +1705,7 @@ private void triggerCheck(CandidatePair triggerPair)
                     checkListStatesUpdated();
                 }
 
-                return;
+                return true;
             }
 
             // RFC 5245: If the state of that pair is In-Progress, the agent
@@ -1723,6 +1726,11 @@ private void triggerCheck(CandidatePair triggerPair)
             // Its state is set to Waiting [and it] is enqueued into the
             // triggered check queue.
             //
+            // Local addition: if we're already stopped, we're never going to send the
+            // check for the triggered pair, so don't enqueue it.
+            if (connCheckClient.isStopped()) {
+                return false;
+            }
             if (triggerPair.getParentComponent().getSelectedPair() == null)
                 logger.info("Add peer CandidatePair with new reflexive " +
                         "address to checkList: " + triggerPair.toRedactedString());
@@ -1749,6 +1757,8 @@ private void triggerCheck(CandidatePair triggerPair)
         checkList.scheduleTriggeredCheck(triggerPair);
         if (wasFrozen && !checkList.isFrozen())
             connCheckClient.startChecks(checkList);
+
+        return true;
     }
 
     /**
diff --git a/src/main/java/org/ice4j/ice/ConnectivityCheckClient.java b/src/main/java/org/ice4j/ice/ConnectivityCheckClient.java
index 6892c144..5541a845 100644
--- a/src/main/java/org/ice4j/ice/ConnectivityCheckClient.java
+++ b/src/main/java/org/ice4j/ice/ConnectivityCheckClient.java
@@ -1004,4 +1004,10 @@ public void stop()
             }
         }
     }
+
+    public boolean isStopped() {
+        synchronized (paceMakers) {
+            return stopped;
+        }
+    }
 }
diff --git a/src/main/java/org/ice4j/ice/ConnectivityCheckServer.java b/src/main/java/org/ice4j/ice/ConnectivityCheckServer.java
index c951ac21..751103f2 100644
--- a/src/main/java/org/ice4j/ice/ConnectivityCheckServer.java
+++ b/src/main/java/org/ice4j/ice/ConnectivityCheckServer.java
@@ -153,12 +153,20 @@ public void processRequest(StunMessageEvent evt)
         remoteUfrag = username.substring(0, colon);
 
         //tell our address handler we saw a new remote address;
-        parentAgent.incomingCheckReceived(evt.getRemoteAddress(),
+        boolean respond = parentAgent.incomingCheckReceived(evt.getRemoteAddress(),
                 evt.getLocalAddress(), priority, remoteUfrag, localUFrag,
                 useCandidate);
 
-        Response response = MessageFactory.createBindingResponse(
-                        request, evt.getRemoteAddress());
+        Response response;
+
+        if (respond) {
+            response = MessageFactory.createBindingResponse(
+                    request, evt.getRemoteAddress());
+        } else {
+            response = MessageFactory.createBindingErrorResponse(
+                    ErrorCodeAttribute.FORBIDDEN,
+                    "Cannot add new remote candidates in current ICE state");
+        }
 
         /* add USERNAME and MESSAGE-INTEGRITY attribute in the response */