From 82cfcda452569cc4d9cf1ae9cac4285ed5527146 Mon Sep 17 00:00:00 2001 From: Jonathan Lennox Date: Wed, 25 Sep 2024 16:53:14 -0400 Subject: [PATCH 1/2] 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 */ From 7fdcc680cb812c88dbd3cfe31957e4b0179a7214 Mon Sep 17 00:00:00 2001 From: Jonathan Lennox Date: Thu, 26 Sep 2024 09:21:45 -0400 Subject: [PATCH 2/2] Checkstyle fixes --- src/main/java/org/ice4j/ice/Agent.java | 3 ++- src/main/java/org/ice4j/ice/ConnectivityCheckServer.java | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/ice4j/ice/Agent.java b/src/main/java/org/ice4j/ice/Agent.java index 3218a58c..0cc57467 100644 --- a/src/main/java/org/ice4j/ice/Agent.java +++ b/src/main/java/org/ice4j/ice/Agent.java @@ -1728,7 +1728,8 @@ private boolean triggerCheck(CandidatePair triggerPair) // // 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()) { + if (connCheckClient.isStopped()) + { return false; } if (triggerPair.getParentComponent().getSelectedPair() == null) diff --git a/src/main/java/org/ice4j/ice/ConnectivityCheckServer.java b/src/main/java/org/ice4j/ice/ConnectivityCheckServer.java index 751103f2..cdbd2977 100644 --- a/src/main/java/org/ice4j/ice/ConnectivityCheckServer.java +++ b/src/main/java/org/ice4j/ice/ConnectivityCheckServer.java @@ -159,10 +159,13 @@ public void processRequest(StunMessageEvent evt) Response response; - if (respond) { + if (respond) + { response = MessageFactory.createBindingResponse( request, evt.getRemoteAddress()); - } else { + } + else + { response = MessageFactory.createBindingErrorResponse( ErrorCodeAttribute.FORBIDDEN, "Cannot add new remote candidates in current ICE state");