From 70b5d8d81aa3059623570972a58b7c595c1e1f26 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Mon, 15 Nov 2021 21:49:31 +0100 Subject: [PATCH] set proper peer dtls setup on ice restart received --- .../xmpp/jingle/JingleRtpConnection.java | 31 +++++++++----- .../xmpp/jingle/RtpContentMap.java | 16 ++++++-- .../xmpp/jingle/SessionDescription.java | 5 ++- .../jingle/stanzas/IceUdpTransportInfo.java | 40 +++++++++++++++++-- 4 files changed, 74 insertions(+), 18 deletions(-) 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 04c43ec1c..c0ddfd96a 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -303,16 +303,18 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web return false; } final boolean isOffer = rtpContentMap.emptyCandidates(); - if (isOffer) { - Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials); - } else { - Log.d(Config.LOGTAG, "received confirmation of ICE restart" + newCredentials); - } - //TODO rewrite setup attribute - //https://groups.google.com/g/discuss-webrtc/c/DfpIMwvUfeM - //https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-dtls-sdp-15#section-5.5 - final RtpContentMap restartContentMap = existing.modifiedCredentials(newCredentials); + final RtpContentMap restartContentMap; try { + if (isOffer) { + Log.d(Config.LOGTAG, "received offer to restart ICE " + newCredentials.values()); + 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); + // 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); + } if (applyIceRestart(jinglePacket, restartContentMap, isOffer)) { return isOffer; } else { @@ -329,6 +331,14 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web } } + private IceUdpTransportInfo.Setup getPeerDtlsSetup() { + final IceUdpTransportInfo.Setup responderSetup = this.responderRtpContentMap.getDtlsSetup(); + if (responderSetup == null || responderSetup == IceUdpTransportInfo.Setup.ACTPASS) { + throw new IllegalStateException("Invalid DTLS setup value in responder content map"); + } + return isInitiator() ? responderSetup : responderSetup.flip(); + } + private boolean applyIceRestart(final JinglePacket jinglePacket, final RtpContentMap restartContentMap, final boolean isOffer) throws ExecutionException, InterruptedException { final SessionDescription sessionDescription = SessionDescription.of(restartContentMap); final org.webrtc.SessionDescription.Type type = isOffer ? org.webrtc.SessionDescription.Type.OFFER : org.webrtc.SessionDescription.Type.ANSWER; @@ -1496,7 +1506,8 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web webRTCWrapper.setIsReadyToReceiveIceCandidates(true); } else { Log.d(Config.LOGTAG, "received failure to our ice restart"); - //TODO handle tie-break. Rollback? + //TODO ignore tie break (maybe rollback?) + //TODO handle other errors } }); } 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 99db8bd34..5366460ec 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java @@ -6,6 +6,7 @@ import com.google.common.base.Strings; import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -104,7 +105,8 @@ public class RtpContentMap { if (fingerprint == null || Strings.isNullOrEmpty(fingerprint.getContent()) || Strings.isNullOrEmpty(fingerprint.getHash())) { throw new SecurityException(String.format("Use of DTLS-SRTP (XEP-0320) is required for content %s", entry.getKey())); } - if (Strings.isNullOrEmpty(fingerprint.getSetup())) { + final IceUdpTransportInfo.Setup setup = fingerprint.getSetup(); + if (setup == null) { throw new SecurityException(String.format("Use of DTLS-SRTP (XEP-0320) is required for content %s but missing setup attribute", entry.getKey())); } } @@ -148,6 +150,14 @@ public class RtpContentMap { return Maps.transformValues(contents, dt -> dt.transport.getCredentials()); } + public IceUdpTransportInfo.Setup getDtlsSetup() { + final Set setups = ImmutableSet.copyOf(Collections2.transform( + contents.values(), + dt->dt.transport.getFingerprint().getSetup() + )); + return setups.size() == 1 ? Iterables.getFirst(setups, null) : null; + } + public boolean emptyCandidates() { int count = 0; for (DescriptionTransport descriptionTransport : contents.values()) { @@ -156,13 +166,13 @@ public class RtpContentMap { return count == 0; } - public RtpContentMap modifiedCredentials(Map credentialsMap) { + public RtpContentMap modifiedCredentials(Map credentialsMap, 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); + final IceUdpTransportInfo modifiedTransportInfo = transportInfo.modifyCredentials(credentials, setup); contentMapBuilder.put(content.getKey(), new DescriptionTransport(rtpDescription, modifiedTransportInfo)); } return new RtpContentMap(this.group, contentMapBuilder.build()); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java index 39031c4a9..e113146b1 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java @@ -156,7 +156,10 @@ public class SessionDescription { final IceUdpTransportInfo.Fingerprint fingerprint = transport.getFingerprint(); if (fingerprint != null) { mediaAttributes.put("fingerprint", fingerprint.getHash() + " " + fingerprint.getContent()); - mediaAttributes.put("setup", fingerprint.getSetup()); + final IceUdpTransportInfo.Setup setup = fingerprint.getSetup(); + if (setup != null) { + mediaAttributes.put("setup", setup.toString().toLowerCase(Locale.ROOT)); + } } final ImmutableList.Builder formatBuilder = new ImmutableList.Builder<>(); for (RtpDescription.PayloadType payloadType : description.getPayloadTypes()) { 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 1586557b7..9af1186fc 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 @@ -10,6 +10,7 @@ import com.google.common.collect.Collections2; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; +import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.Hashtable; @@ -83,11 +84,19 @@ public class IceUdpTransportInfo extends GenericTransportInfo { return transportInfo; } - public IceUdpTransportInfo modifyCredentials(Credentials credentials) { + public IceUdpTransportInfo modifyCredentials(final Credentials credentials, final Setup setup) { final IceUdpTransportInfo transportInfo = new IceUdpTransportInfo(); transportInfo.setAttribute("ufrag", credentials.ufrag); transportInfo.setAttribute("pwd", credentials.password); - transportInfo.setChildren(getChildren()); + for (final Element child : getChildren()) { + if (child.getName().equals("fingerprint") && Namespace.JINGLE_APPS_DTLS.equals(child.getNamespace())) { + final Fingerprint fingerprint = new Fingerprint(); + fingerprint.setAttributes(new Hashtable<>(child.getAttributes())); + fingerprint.setContent(child.getContent()); + fingerprint.setAttribute("setup", setup.toString().toLowerCase(Locale.ROOT)); + transportInfo.addChild(fingerprint); + } + } return transportInfo; } @@ -337,8 +346,31 @@ public class IceUdpTransportInfo extends GenericTransportInfo { return this.getAttribute("hash"); } - public String getSetup() { - return this.getAttribute("setup"); + public Setup getSetup() { + final String setup = this.getAttribute("setup"); + return setup == null ? null : Setup.of(setup); + } + } + + public enum Setup { + ACTPASS, PASSIVE, ACTIVE; + + public static Setup of(String setup) { + try { + return valueOf(setup.toUpperCase(Locale.ROOT)); + } catch (IllegalArgumentException e) { + return null; + } + } + + public Setup flip() { + if (this == PASSIVE) { + return ACTIVE; + } + if (this == ACTIVE) { + return PASSIVE; + } + throw new IllegalStateException(this.name()+" can not be flipped"); } } }