From 0a18c8613f83d2a6861a4ae4fef4e534f3f122f9 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Tue, 16 Nov 2021 17:08:34 +0100 Subject: [PATCH] assume credentials are the same for all contents when restarting ICE --- .../conversations/ui/RtpSessionActivity.java | 1 + .../xmpp/jingle/JingleRtpConnection.java | 23 +++++++++++-------- .../xmpp/jingle/RtpContentMap.java | 17 ++++++++++---- .../jingle/stanzas/IceUdpTransportInfo.java | 4 ++-- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java index d0bdbb788..39ba7429f 100644 --- a/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java +++ b/src/main/java/eu/siacs/conversations/ui/RtpSessionActivity.java @@ -1003,6 +1003,7 @@ public class RtpSessionActivity extends XmppActivity implements XmppConnectionSe RendererCommon.ScalingType.SCALE_ASPECT_FILL, RendererCommon.ScalingType.SCALE_ASPECT_FIT ); + //TODO this should probably only be 'connected' if (STATES_CONSIDERED_CONNECTED.contains(state)) { binding.appBarLayout.setVisibility(View.GONE); getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java index 66cf5c23b..26c068c08 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -141,7 +141,6 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } private final WebRTCWrapper webRTCWrapper = new WebRTCWrapper(this); - //TODO convert to Queue>? private final Queue> pendingIceCandidates = new LinkedList<>(); private final OmemoVerification omemoVerification = new OmemoVerification(); private final Message message; @@ -295,9 +294,13 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web private boolean checkForIceRestart(final JinglePacket jinglePacket, final RtpContentMap rtpContentMap) { final RtpContentMap existing = getRemoteContentMap(); - final Map existingCredentials = existing.getCredentials(); - final Map newCredentials = rtpContentMap.getCredentials(); - if (!existingCredentials.keySet().equals(newCredentials.keySet())) { + final IceUdpTransportInfo.Credentials existingCredentials; + final IceUdpTransportInfo.Credentials newCredentials; + try { + existingCredentials = existing.getCredentials(); + newCredentials = rtpContentMap.getCredentials(); + } catch (final IllegalStateException e) { + Log.d(Config.LOGTAG, "unable to gather credentials for comparison", e); return false; } if (existingCredentials.equals(newCredentials)) { @@ -307,11 +310,11 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web final RtpContentMap restartContentMap; try { if (isOffer) { - Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials.values()); + Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials); restartContentMap = existing.modifiedCredentials(newCredentials, IceUdpTransportInfo.Setup.ACTPASS); } else { final IceUdpTransportInfo.Setup setup = getPeerDtlsSetup(); - Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials.values() + " peer_setup=" + setup); + Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials + " peer_setup=" + setup); // DTLS setup attribute needs to be rewritten to reflect current peer state // https://groups.google.com/g/discuss-webrtc/c/DfpIMwvUfeM restartContentMap = existing.modifiedCredentials(newCredentials, setup); @@ -319,7 +322,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (applyIceRestart(jinglePacket, restartContentMap, isOffer)) { return isOffer; } else { - Log.d(Config.LOGTAG,"ignored ice restart. offer="+isOffer); + Log.d(Config.LOGTAG, "ignoring ICE restart. sending tie-break"); respondWithTieBreak(jinglePacket); return true; } @@ -327,7 +330,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web respondOk(jinglePacket); final Throwable rootCause = Throwables.getRootCause(exception); if (rootCause instanceof WebRTCWrapper.PeerConnectionNotInitialized) { - Log.d(Config.LOGTAG,"ignoring PeerConnectionNotInitialized"); + Log.d(Config.LOGTAG, "ignoring PeerConnectionNotInitialized"); //TODO don’t respond OK but respond with out-of-order return true; } @@ -1451,8 +1454,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web @Override public void onIceCandidate(final IceCandidate iceCandidate) { final RtpContentMap rtpContentMap = isInitiator() ? this.initiatorRtpContentMap : this.responderRtpContentMap; - final Collection currentUfrags = Collections2.transform(rtpContentMap.getCredentials().values(), c -> c.ufrag); - final IceUdpTransportInfo.Candidate candidate = IceUdpTransportInfo.Candidate.fromSdpAttribute(iceCandidate.sdp, currentUfrags); + final String ufrag = rtpContentMap.getCredentials().ufrag; + final IceUdpTransportInfo.Candidate candidate = IceUdpTransportInfo.Candidate.fromSdpAttribute(iceCandidate.sdp, ufrag); if (candidate == null) { Log.d(Config.LOGTAG, "ignoring (not sending) candidate: " + iceCandidate.toString()); return; diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java index 091bf7c10..ea351cb1b 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java @@ -146,14 +146,22 @@ public class RtpContentMap { ); } - public Map getCredentials() { - return Maps.transformValues(contents, dt -> dt.transport.getCredentials()); + public IceUdpTransportInfo.Credentials getCredentials() { + final Set allCredentials = ImmutableSet.copyOf(Collections2.transform( + contents.values(), + dt -> dt.transport.getCredentials() + )); + final IceUdpTransportInfo.Credentials credentials = Iterables.getFirst(allCredentials, null); + if (allCredentials.size() == 1 && credentials != null) { + return credentials; + } + throw new IllegalStateException("Content map does not have distinct credentials"); } public IceUdpTransportInfo.Setup getDtlsSetup() { final Set setups = ImmutableSet.copyOf(Collections2.transform( contents.values(), - dt->dt.transport.getFingerprint().getSetup() + dt -> dt.transport.getFingerprint().getSetup() )); final IceUdpTransportInfo.Setup setup = Iterables.getFirst(setups, null); if (setups.size() == 1 && setup != null) { @@ -170,12 +178,11 @@ public class RtpContentMap { return count == 0; } - public RtpContentMap modifiedCredentials(Map credentialsMap, final IceUdpTransportInfo.Setup setup) { + public RtpContentMap modifiedCredentials(IceUdpTransportInfo.Credentials credentials, final IceUdpTransportInfo.Setup setup) { final ImmutableMap.Builder contentMapBuilder = new ImmutableMap.Builder<>(); for (final Map.Entry content : contents.entrySet()) { final RtpDescription rtpDescription = content.getValue().description; IceUdpTransportInfo transportInfo = content.getValue().transport; - final IceUdpTransportInfo.Credentials credentials = Objects.requireNonNull(credentialsMap.get(content.getKey())); final IceUdpTransportInfo modifiedTransportInfo = transportInfo.modifyCredentials(credentials, setup); contentMapBuilder.put(content.getKey(), new DescriptionTransport(rtpDescription, modifiedTransportInfo)); } diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java index 9af1186fc..45260cafb 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java @@ -146,7 +146,7 @@ public class IceUdpTransportInfo extends GenericTransportInfo { } // https://tools.ietf.org/html/draft-ietf-mmusic-ice-sip-sdp-39#section-5.1 - public static Candidate fromSdpAttribute(final String attribute, Collection currentUfrags) { + public static Candidate fromSdpAttribute(final String attribute, String currentUfrag) { final String[] pair = attribute.split(":", 2); if (pair.length == 2 && "candidate".equals(pair[0])) { final String[] segments = pair[1].split(" "); @@ -163,7 +163,7 @@ public class IceUdpTransportInfo extends GenericTransportInfo { additional.put(segments[i], segments[i + 1]); } final String ufrag = additional.get("ufrag"); - if (ufrag != null && !currentUfrags.contains(ufrag)) { + if (ufrag != null && !ufrag.equals(currentUfrag)) { return null; } final Candidate candidate = new Candidate();