set proper peer dtls setup on ice restart received

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java         | 31 
src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java               | 16 
src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java          |  5 
src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java | 40 
4 files changed, 74 insertions(+), 18 deletions(-)

Detailed changes

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
             }
         });
     }

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<IceUdpTransportInfo.Setup> 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<String, IceUdpTransportInfo.Credentials> credentialsMap) {
+    public RtpContentMap modifiedCredentials(Map<String, IceUdpTransportInfo.Credentials> credentialsMap, final IceUdpTransportInfo.Setup setup) {
         final ImmutableMap.Builder<String, DescriptionTransport> contentMapBuilder = new ImmutableMap.Builder<>();
         for (final Map.Entry<String, DescriptionTransport> 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());

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<Integer> formatBuilder = new ImmutableList.Builder<>();
             for (RtpDescription.PayloadType payloadType : description.getPayloadTypes()) {

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");
         }
     }
 }