Skip to content

Commit ef04a4e

Browse files
paweldomaspaweldomas
authored
feat: add support for session-terminate (#562)
Implements 'session-terminate' which allows the clients to end the session. If there's the restart attribute set to true in the <bridge-session> extension, then Jicofo will start a new session after tearing down the existing one. Adds rate limiting on the restart requests to prevent abuse and/or client bugs. There must be at least 10 second gap between the requests and no more than 3 requests are allowed in the last minute.
1 parent 902880b commit ef04a4e

File tree

10 files changed

+295
-37
lines changed

10 files changed

+295
-37
lines changed

‎pom.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@
156156
<dependency>
157157
<groupId>${project.groupId}</groupId>
158158
<artifactId>jitsi-xmpp-extensions</artifactId>
159-
<version>1.0-6-g009420d</version>
159+
<version>1.0-13-ga40f9c3</version>
160160
</dependency>
161161
<dependency>
162162
<groupId>org.osgi</groupId>

‎src/main/java/org/jitsi/impl/protocol/xmpp/DefaultJingleRequestHandler.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,14 @@ public XMPPError onSessionAccept(JingleSession jingleSession,
6666
return null;
6767
}
6868

69+
@Override
70+
public XMPPError onSessionTerminate(JingleSession jingleSession, JingleIQ iq)
71+
{
72+
logger.warn("Ignored Jingle 'session-terminate'");
73+
74+
return null;
75+
}
76+
6977
@Override
7078
public XMPPError onSessionInfo(JingleSession session, JingleIQ iq)
7179
{

‎src/main/java/org/jitsi/jicofo/JitsiMeetConferenceImpl.java‎

Lines changed: 96 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,11 @@ protected void onMemberLeft(ChatRoomMember chatRoomMember)
12861286
= findParticipantForChatMember(chatRoomMember);
12871287
if (leftParticipant != null)
12881288
{
1289-
terminateParticipant(leftParticipant, Reason.GONE, null);
1289+
terminateParticipant(
1290+
leftParticipant,
1291+
Reason.GONE,
1292+
null,
1293+
/* no need to send session-terminate - gone */ false);
12901294
}
12911295
else
12921296
{
@@ -1308,31 +1312,38 @@ else if (participants.size() == 0)
13081312

13091313
private void terminateParticipant(Participant participant,
13101314
Reason reason,
1311-
String message)
1315+
String message,
1316+
boolean sendSessionTerminate)
13121317
{
1318+
logger.info(String.format(
1319+
"Terminating %s, reason: %s, send st: %s",
1320+
participant,
1321+
reason,
1322+
sendSessionTerminate));
1323+
13131324
BridgeSession bridgeSession;
13141325
synchronized (participantLock)
13151326
{
13161327
Jid contactAddress = participant.getMucJid();
13171328
if (participant.isSessionEstablished())
13181329
{
13191330
JingleSession jingleSession = participant.getJingleSession();
1320-
logger.info("Terminating: " + contactAddress);
13211331

1322-
jingle.terminateSession(jingleSession, reason, message);
1332+
jingle.terminateSession(jingleSession, reason, message, sendSessionTerminate);
13231333

13241334
removeSources(
13251335
jingleSession,
13261336
participant.getSourcesCopy(),
13271337
participant.getSourceGroupsCopy(),
13281338
false /* no JVB update - will expire */);
1339+
1340+
participant.setJingleSession(null);
13291341
}
13301342

13311343
bridgeSession = participant.terminateBridgeSession();
13321344

13331345
boolean removed = participants.remove(participant);
1334-
logger.info(
1335-
"Removed participant: " + removed + ", " + contactAddress);
1346+
logger.info("Removed participant: " + removed + ", " + contactAddress);
13361347
}
13371348

13381349
if (bridgeSession != null)
@@ -1523,7 +1534,7 @@ public XMPPError onSessionInfo(JingleSession session, JingleIQ iq)
15231534
String bridgeSessionId = bsPE != null ? bsPE.getId() : null;
15241535
BridgeSession bridgeSession = findBridgeSession(participant);
15251536

1526-
if (bridgeSession != null)
1537+
if (bridgeSession != null && bridgeSession.id.equals(bridgeSessionId))
15271538
{
15281539
logger.info(String.format(
15291540
"Received ICE failed notification from %s, session: %s",
@@ -1543,6 +1554,75 @@ public XMPPError onSessionInfo(JingleSession session, JingleIQ iq)
15431554
return null;
15441555
}
15451556

1557+
/**
1558+
* Handles 'session-terminate' received from the client.
1559+
*
1560+
* {@inheritDoc}
1561+
*/
1562+
@Override
1563+
public XMPPError onSessionTerminate(JingleSession session, JingleIQ iq)
1564+
{
1565+
Participant participant = findParticipantForJingleSession(session);
1566+
1567+
// FIXME: (duplicate) there's very similar logic in onSessionAccept/onSessionInfo
1568+
if (participant == null)
1569+
{
1570+
String errorMsg = "No participant for " + session.getAddress();
1571+
1572+
logger.warn("onSessionTerminate: " + errorMsg);
1573+
1574+
return XMPPError.from(
1575+
XMPPError.Condition.item_not_found, errorMsg).build();
1576+
}
1577+
1578+
BridgeSessionPacketExtension bsPE
1579+
= iq.getExtension(
1580+
BridgeSessionPacketExtension.ELEMENT_NAME,
1581+
BridgeSessionPacketExtension.NAMESPACE);
1582+
String bridgeSessionId = bsPE != null ? bsPE.getId() : null;
1583+
BridgeSession bridgeSession = findBridgeSession(participant);
1584+
boolean restartRequested = bsPE != null ? bsPE.isRestart() : false;
1585+
1586+
if (bridgeSession == null || !bridgeSession.id.equals(bridgeSessionId))
1587+
{
1588+
logger.info(String.format(
1589+
"Ignored session-terminate for invalid session: %s, bridge session ID: %s restart: %s",
1590+
participant,
1591+
bridgeSessionId,
1592+
restartRequested));
1593+
1594+
return XMPPError.from(XMPPError.Condition.item_not_found, "invalid bridge session ID").build();
1595+
}
1596+
1597+
logger.info(String.format(
1598+
"Received session-terminate from %s, session: %s, restart: %s",
1599+
participant,
1600+
bridgeSession,
1601+
restartRequested));
1602+
1603+
synchronized (participantLock)
1604+
{
1605+
terminateParticipant(participant, null, null, /* do not send session-terminate */ false);
1606+
1607+
if (restartRequested)
1608+
{
1609+
if (participant.incrementAndCheckRestartRequests())
1610+
{
1611+
participants.add(participant);
1612+
inviteParticipant(participant, false, hasToStartMuted(participant, false));
1613+
}
1614+
else
1615+
{
1616+
logger.warn(String.format("Rate limiting %s for restart requests", participant));
1617+
1618+
return XMPPError.from(XMPPError.Condition.resource_constraint, "rate-limited").build();
1619+
}
1620+
}
1621+
}
1622+
1623+
return null;
1624+
}
1625+
15461626
/**
15471627
* Advertises new sources across all conference participants by using
15481628
* 'source-add' Jingle notification.
@@ -2507,7 +2587,8 @@ void onInviteFailed(ParticipantChannelAllocator channelAllocator)
25072587
terminateParticipant(
25082588
channelAllocator.getParticipant(),
25092589
Reason.GENERAL_ERROR,
2510-
"jingle session failed");
2590+
"jingle session failed",
2591+
/* send session-terminate */ true);
25112592
}
25122593

25132594
/**
@@ -2712,19 +2793,20 @@ public void run()
27122793
if (participants.size() == 1)
27132794
{
27142795
Participant p = participants.get(0);
2715-
logger.info(
2716-
"Timing out single participant: " + p.getMucJid());
2796+
2797+
logger.info("Timing out single participant: " + p.getMucJid());
27172798

27182799
terminateParticipant(
2719-
p, Reason.EXPIRED, "Idle session timeout");
2800+
p,
2801+
Reason.EXPIRED,
2802+
"Idle session timeout",
2803+
/* send session-terminate */ true);
27202804

27212805
disposeConference();
27222806
}
27232807
else
27242808
{
2725-
logger.error(
2726-
"Should never execute if more than 1 participant? "
2727-
+ getRoomName());
2809+
logger.error("Should never execute if more than 1 participant? " + getRoomName());
27282810
}
27292811
singleParticipantTout = null;
27302812
}

‎src/main/java/org/jitsi/jicofo/LipSyncHack.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,9 +366,10 @@ public void sendRemoveSourceIQ(
366366
@Override
367367
public void terminateSession(JingleSession session,
368368
Reason reason,
369-
String msg)
369+
String msg,
370+
boolean sendTerminate)
370371
{
371-
jingleImpl.terminateSession(session, reason, msg);
372+
jingleImpl.terminateSession(session, reason, msg, sendTerminate);
372373
}
373374

374375
/**

‎src/main/java/org/jitsi/jicofo/Participant.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,11 @@
2626
import org.jitsi.utils.logging.*;
2727
import org.jxmpp.jid.*;
2828

29+
import java.time.*;
2930
import java.util.*;
3031

32+
import static java.time.temporal.ChronoUnit.SECONDS;
33+
3134
/**
3235
* Class represent Jitsi Meet conference participant. Stores information about
3336
* Colibri channels allocated, Jingle session and media sources.
@@ -66,6 +69,17 @@ public static String getEndpointId(XmppChatMember chatRoomMember)
6669
*/
6770
private JitsiMeetConferenceImpl.BridgeSession bridgeSession;
6871

72+
/**
73+
* The {@link Clock} used by this participant.
74+
*/
75+
private Clock clock = Clock.systemUTC();
76+
77+
/**
78+
* The list stored the timestamp when the last restart requests have been received for this participant and is used
79+
* for rate limiting. See {@link #incrementAndCheckRestartRequests()} for more details.
80+
*/
81+
private final Deque<Instant> restartRequests = new LinkedList<>();
82+
6983
/**
7084
* MUC chat member of this participant.
7185
*/
@@ -152,6 +166,15 @@ void setBridgeSession(JitsiMeetConferenceImpl.BridgeSession bridgeSession)
152166
this.bridgeSession = bridgeSession;
153167
}
154168

169+
/**
170+
* Sets the new clock instance to be used by this participant. Meant for testing.
171+
* @param newClock - the new {@link Clock}
172+
*/
173+
public void setClock(Clock newClock)
174+
{
175+
this.clock = newClock;
176+
}
177+
155178
/**
156179
* Sets {@link JingleSession} established with this peer.
157180
* @param jingleSession the new Jingle session to be assigned to this peer.
@@ -170,6 +193,14 @@ public XmppChatMember getChatMember()
170193
return roomMember;
171194
}
172195

196+
/**
197+
* @return {@link Clock} used by this participant instance.
198+
*/
199+
public Clock getClock()
200+
{
201+
return clock;
202+
}
203+
173204
/**
174205
* Returns <tt>true</tt> if this participant supports RTP bundle and RTCP
175206
* mux.
@@ -205,6 +236,44 @@ public boolean hasRtxSupport()
205236
return supportedFeatures.contains(DiscoveryUtil.FEATURE_RTX);
206237
}
207238

239+
/**
240+
* Rate limiting mechanism for session restart requests received from participants.
241+
* The rules ar as follows:
242+
* - must be at least 10 second gap between the requests
243+
* - no more than 3 requests within the last minute
244+
*
245+
* @return {@code true} if it's okay to process the request, as in it doesn't violate the current rate limiting
246+
* policy, or {@code false} if the request should be denied.
247+
*/
248+
public boolean incrementAndCheckRestartRequests()
249+
{
250+
final Instant now = Instant.now(clock);
251+
Instant previousRequest = this.restartRequests.peekLast();
252+
253+
if (previousRequest == null)
254+
{
255+
this.restartRequests.add(now);
256+
257+
return true;
258+
}
259+
260+
if (previousRequest.until(now, SECONDS) < 10)
261+
{
262+
return false;
263+
}
264+
265+
// Allow only 3 requests within the last minute
266+
this.restartRequests.removeIf(requestTime -> requestTime.until(now, SECONDS) > 60);
267+
if (this.restartRequests.size() > 2)
268+
{
269+
return false;
270+
}
271+
272+
this.restartRequests.add(now);
273+
274+
return true;
275+
}
276+
208277
/**
209278
* FIXME: we need to remove "is SIP gateway code", but there are still
210279
* situations where we need to know whether given peer is a human or not.

‎src/main/java/org/jitsi/protocol/xmpp/AbstractOperationSetJingle.java‎

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,9 @@ protected IQ processJingleIQ(JingleIQ iq)
307307
case SESSION_INFO:
308308
error = requestHandler.onSessionInfo(session, iq);
309309
break;
310+
case SESSION_TERMINATE:
311+
error = requestHandler.onSessionTerminate(session, iq);
312+
break;
310313
case TRANSPORT_ACCEPT:
311314
error = requestHandler.onTransportAccept(session, iq.getContentList());
312315
break;
@@ -558,39 +561,47 @@ public void terminateHandlersSessions(JingleRequestHandler requestHandler)
558561
{
559562
if (session.getRequestHandler() == requestHandler)
560563
{
561-
terminateSession(session, Reason.GONE, null);
564+
terminateSession(session, Reason.GONE, null, true);
562565
}
563566
}
564567
}
565568

566569
/**
567-
* Terminates given Jingle session by sending 'session-terminate' with some
568-
* {@link Reason} if provided.
570+
* Terminates given Jingle session. This method is to be called either to send 'session-terminate' or to inform
571+
* this operation set that the session has been terminated as a result of 'session-terminate' received from
572+
* the other peer in which case {@code sendTerminate} should be set to {@code false}.
569573
*
570574
* @param session the <tt>JingleSession</tt> to terminate.
571575
* @param reason one of {@link Reason} enum that indicates why the session
572576
* is being ended or <tt>null</tt> to omit.
577+
* @param sendTerminate when {@code true} it means that a 'session-terminate' is to be sent, otherwise it means
578+
* the session is being ended on the remote peer's request.
573579
* {@inheritDoc}
574580
*/
575581
@Override
576582
public void terminateSession(JingleSession session,
577583
Reason reason,
578-
String message)
584+
String message,
585+
boolean sendTerminate)
579586
{
580-
logger.info("Terminate session: " + session.getAddress());
587+
logger.info(String.format(
588+
"Terminate session: %s, reason: %s, send terminate: %s",
589+
session.getAddress(),
590+
reason,
591+
sendTerminate));
581592

582-
// we do not send session-terminate as muc addresses are invalid at this
583-
// point
584-
// FIXME: but there is also connection address available
585-
JingleIQ terminate
586-
= JinglePacketFactory.createSessionTerminate(
593+
if (sendTerminate)
594+
{
595+
JingleIQ terminate
596+
= JinglePacketFactory.createSessionTerminate(
587597
getOurJID(),
588598
session.getAddress(),
589599
session.getSessionID(),
590600
reason,
591601
message);
592602

593-
getConnection().sendStanza(terminate);
603+
getConnection().sendStanza(terminate);
604+
}
594605

595606
sessions.remove(session.getSessionID());
596607
}

0 commit comments

Comments
 (0)