support omemo verification in non stub transport content modifications

Daniel Gultsch created

Dino (and this is probably correct behaviour) expects a fingerprint in the
content-add message. (and not a stub transport as indicated in the examples).

however if we start to include them we also need to encrypt and verify them
properly.

Change summary

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java         | 105 
src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java               |  41 
src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java |   8 
3 files changed, 126 insertions(+), 28 deletions(-)

Detailed changes

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java 🔗

@@ -370,6 +370,7 @@ public class JingleRtpConnection extends AbstractJingleConnection
     }
 
     private void receiveContentAdd(final JinglePacket jinglePacket) {
+        // TODO check if in session accepted
         final RtpContentMap modification;
         try {
             modification = RtpContentMap.of(jinglePacket);
@@ -385,7 +386,29 @@ public class JingleRtpConnection extends AbstractJingleConnection
             return;
         }
         if (isInState(State.SESSION_ACCEPTED)) {
-            receiveContentAdd(jinglePacket, modification);
+            final boolean hasFullTransportInfo = modification.hasFullTransportInfo();
+            final ListenableFuture<RtpContentMap> future =
+                    receiveRtpContentMap(
+                            modification, this.omemoVerification.hasFingerprint() && hasFullTransportInfo);
+            Futures.addCallback(future, new FutureCallback<RtpContentMap>() {
+                @Override
+                public void onSuccess(final RtpContentMap rtpContentMap) {
+                    receiveContentAdd(jinglePacket, rtpContentMap);
+                }
+
+                @Override
+                public void onFailure(@NonNull Throwable throwable) {
+                    respondOk(jinglePacket);
+                    final Throwable rootCause = Throwables.getRootCause(throwable);
+                    Log.d(
+                            Config.LOGTAG,
+                            id.account.getJid().asBareJid()
+                                    + ": improperly formatted contents in content-add",
+                            throwable);
+                    webRTCWrapper.close();
+                    sendSessionTerminate(Reason.ofThrowable(rootCause), rootCause.getMessage());
+                }
+            }, MoreExecutors.directExecutor());
         } else {
             terminateWithOutOfOrder(jinglePacket);
         }
@@ -470,7 +493,22 @@ public class JingleRtpConnection extends AbstractJingleConnection
         if (ourSummary.equals(ContentAddition.summary(receivedContentAccept))) {
             this.outgoingContentAdd = null;
             respondOk(jinglePacket);
-            receiveContentAccept(receivedContentAccept);
+            final boolean hasFullTransportInfo = receivedContentAccept.hasFullTransportInfo();
+            final ListenableFuture<RtpContentMap> future =
+                    receiveRtpContentMap(
+                            receivedContentAccept, this.omemoVerification.hasFingerprint() && hasFullTransportInfo);
+            Futures.addCallback(future, new FutureCallback<RtpContentMap>() {
+                @Override
+                public void onSuccess(final RtpContentMap result) {
+                    receiveContentAccept(result);
+                }
+
+                @Override
+                public void onFailure(@NonNull final Throwable throwable) {
+                    webRTCWrapper.close();
+                    sendSessionTerminate(Reason.ofThrowable(throwable), throwable.getMessage());
+                }
+            }, MoreExecutors.directExecutor());
         } else {
             Log.d(Config.LOGTAG, "received content-accept did not match our outgoing content-add");
             terminateWithOutOfOrder(jinglePacket);
@@ -759,14 +797,29 @@ public class JingleRtpConnection extends AbstractJingleConnection
             final RtpContentMap contentAcceptMap =
                     rtpContentMap.toContentModification(
                             Collections2.transform(contentAddition, ca -> ca.name));
+
             Log.d(
                     Config.LOGTAG,
                     id.getAccount().getJid().asBareJid()
                             + ": sending content-accept "
                             + ContentAddition.summary(contentAcceptMap));
             modifyLocalContentMap(rtpContentMap);
-            sendContentAccept(contentAcceptMap);
-            this.webRTCWrapper.setIsReadyToReceiveIceCandidates(true);
+            final ListenableFuture<RtpContentMap> future = prepareOutgoingContentMap(contentAcceptMap);
+            Futures.addCallback(
+                    future,
+                    new FutureCallback<RtpContentMap>() {
+                        @Override
+                        public void onSuccess(final RtpContentMap rtpContentMap) {
+                            sendContentAccept(rtpContentMap);
+                            webRTCWrapper.setIsReadyToReceiveIceCandidates(true);
+                        }
+
+                        @Override
+                        public void onFailure(@NonNull final Throwable throwable) {
+                            failureToPerformAction(JinglePacket.Action.CONTENT_ACCEPT, throwable);
+                        }
+                    },
+                    MoreExecutors.directExecutor());
         } catch (final Exception e) {
             Log.d(Config.LOGTAG, "unable to accept content add", Throwables.getRootCause(e));
             webRTCWrapper.close();
@@ -979,12 +1032,20 @@ public class JingleRtpConnection extends AbstractJingleConnection
 
     private ListenableFuture<RtpContentMap> receiveRtpContentMap(
             final JinglePacket jinglePacket, final boolean expectVerification) {
-        final RtpContentMap receivedContentMap;
         try {
-            receivedContentMap = RtpContentMap.of(jinglePacket);
+            return receiveRtpContentMap(RtpContentMap.of(jinglePacket), expectVerification);
         } catch (final Exception e) {
             return Futures.immediateFailedFuture(e);
         }
+        }
+        private ListenableFuture<RtpContentMap> receiveRtpContentMap(final RtpContentMap receivedContentMap, final boolean expectVerification) {
+        Log.d(
+                Config.LOGTAG,
+                "receiveRtpContentMap("
+                        + receivedContentMap.getClass().getSimpleName()
+                        + ",expectVerification="
+                        + expectVerification
+                        + ")");
         if (receivedContentMap instanceof OmemoVerifiedRtpContentMap) {
             final ListenableFuture<AxolotlService.OmemoVerifiedPayload<RtpContentMap>> future =
                     id.account
@@ -1287,6 +1348,16 @@ public class JingleRtpConnection extends AbstractJingleConnection
         sendSessionTerminate(Reason.ofThrowable(rootCause), rootCause.getMessage());
     }
 
+    private void failureToPerformAction(final JinglePacket.Action action, final Throwable throwable) {
+        if (isTerminated()) {
+            return;
+        }
+        final Throwable rootCause = Throwables.getRootCause(throwable);
+        Log.d(Config.LOGTAG, "unable to send " + action, rootCause);
+        webRTCWrapper.close();
+        sendSessionTerminate(Reason.ofThrowable(rootCause), rootCause.getMessage());
+    }
+
     private void addIceCandidatesFromBlackLog() {
         Map.Entry<String, RtpContentMap.DescriptionTransport> foo;
         while ((foo = this.pendingIceCandidates.poll()) != null) {
@@ -2486,6 +2557,27 @@ public class JingleRtpConnection extends AbstractJingleConnection
     private void sendContentAdd(final RtpContentMap rtpContentMap, final Collection<String> added) {
         final RtpContentMap contentAdd = rtpContentMap.toContentModification(added);
         this.outgoingContentAdd = contentAdd;
+        final ListenableFuture<RtpContentMap> outgoingContentMapFuture =
+                prepareOutgoingContentMap(contentAdd);
+        Futures.addCallback(
+                outgoingContentMapFuture,
+                new FutureCallback<RtpContentMap>() {
+                    @Override
+                    public void onSuccess(final RtpContentMap outgoingContentMap) {
+                        sendContentAdd(outgoingContentMap);
+                        webRTCWrapper.setIsReadyToReceiveIceCandidates(true);
+                    }
+
+                    @Override
+                    public void onFailure(@NonNull Throwable throwable) {
+                        failureToPerformAction(JinglePacket.Action.CONTENT_ADD, throwable);
+                    }
+                },
+                MoreExecutors.directExecutor());
+    }
+
+    private void sendContentAdd(final RtpContentMap contentAdd) {
+
         final JinglePacket jinglePacket =
                 contentAdd.toJinglePacket(JinglePacket.Action.CONTENT_ADD, id.sessionId);
         jinglePacket.setTo(id.with);
@@ -2512,7 +2604,6 @@ public class JingleRtpConnection extends AbstractJingleConnection
                         handleIqTimeoutResponse(response);
                     }
                 });
-        this.webRTCWrapper.setIsReadyToReceiveIceCandidates(true);
     }
 
     private void setLocalContentMap(final RtpContentMap rtpContentMap) {

src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java 🔗

@@ -23,7 +23,6 @@ import eu.siacs.conversations.xmpp.jingle.stanzas.OmemoVerifiedIceUdpTransportIn
 import eu.siacs.conversations.xmpp.jingle.stanzas.RtpDescription;
 
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -275,6 +274,11 @@ public class RtpContentMap {
         return count == 0;
     }
 
+    public boolean hasFullTransportInfo() {
+        return Collections2.transform(this.contents.values(), dt -> dt.transport.isStub())
+                .contains(false);
+    }
+
     public RtpContentMap modifiedCredentials(
             IceUdpTransportInfo.Credentials credentials, final IceUdpTransportInfo.Setup setup) {
         final ImmutableMap.Builder<String, DescriptionTransport> contentMapBuilder =
@@ -354,12 +358,7 @@ public class RtpContentMap {
 
     public RtpContentMap toContentModification(final Collection<String> modifications) {
         return new RtpContentMap(
-                this.group,
-                Maps.transformValues(
-                        Maps.filterKeys(contents, Predicates.in(modifications)),
-                        dt ->
-                                new DescriptionTransport(
-                                        dt.senders, dt.description, IceUdpTransportInfo.STUB)));
+                this.group, Maps.filterKeys(contents, Predicates.in(modifications)));
     }
 
     public RtpContentMap toStub() {
@@ -396,37 +395,43 @@ public class RtpContentMap {
     }
 
     public RtpContentMap addContent(
-            final RtpContentMap modification, final IceUdpTransportInfo.Setup setup) {
-        final IceUdpTransportInfo.Credentials credentials = getDistinctCredentials();
-        final Collection<String> iceOptions = getCombinedIceOptions();
-        final DTLS dtls = getDistinctDtls();
+            final RtpContentMap modification, final IceUdpTransportInfo.Setup setupOverwrite) {
         final Map<String, DescriptionTransport> combined = merge(contents, modification.contents);
         final Map<String, DescriptionTransport> combinedFixedTransport =
                 Maps.transformValues(
                         combined,
                         dt -> {
                             final IceUdpTransportInfo iceUdpTransportInfo;
-                            if (dt.transport.emptyCredentials()) {
+                            if (dt.transport.isStub()) {
+                                final IceUdpTransportInfo.Credentials credentials =
+                                        getDistinctCredentials();
+                                final Collection<String> iceOptions = getCombinedIceOptions();
+                                final DTLS dtls = getDistinctDtls();
                                 iceUdpTransportInfo =
                                         IceUdpTransportInfo.of(
                                                 credentials,
                                                 iceOptions,
-                                                setup,
+                                                setupOverwrite,
                                                 dtls.hash,
                                                 dtls.fingerprint);
                             } else {
+                                final IceUdpTransportInfo.Fingerprint fp =
+                                        dt.transport.getFingerprint();
+                                final IceUdpTransportInfo.Setup setup = fp.getSetup();
                                 iceUdpTransportInfo =
                                         IceUdpTransportInfo.of(
                                                 dt.transport.getCredentials(),
-                                                iceOptions,
-                                                setup,
-                                                dtls.hash,
-                                                dtls.fingerprint);
+                                                dt.transport.getIceOptions(),
+                                                setup == IceUdpTransportInfo.Setup.ACTPASS
+                                                        ? setupOverwrite
+                                                        : setup,
+                                                fp.getHash(),
+                                                fp.getContent());
                             }
                             return new DescriptionTransport(
                                     dt.senders, dt.description, iceUdpTransportInfo);
                         });
-        return new RtpContentMap(modification.group, combinedFixedTransport);
+        return new RtpContentMap(modification.group, ImmutableMap.copyOf(combinedFixedTransport));
     }
 
     private static Map<String, DescriptionTransport> merge(

src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/IceUdpTransportInfo.java 🔗

@@ -82,7 +82,7 @@ public class IceUdpTransportInfo extends GenericTransportInfo {
         iceUdpTransportInfo.addChild(Fingerprint.of(setup, hash, fingerprint));
         iceUdpTransportInfo.setAttribute("ufrag", credentials.ufrag);
         iceUdpTransportInfo.setAttribute("pwd", credentials.password);
-        for(final String iceOption : iceOptions) {
+        for (final String iceOption : iceOptions) {
             iceUdpTransportInfo.addChild(new IceOption(iceOption));
         }
         return iceUdpTransportInfo;
@@ -110,8 +110,10 @@ public class IceUdpTransportInfo extends GenericTransportInfo {
         return new Credentials(ufrag, password);
     }
 
-    public boolean emptyCredentials() {
-        return Strings.isNullOrEmpty(this.getAttribute("ufrag")) || Strings.isNullOrEmpty(this.getAttribute("pwd"));
+    public boolean isStub() {
+        return Strings.isNullOrEmpty(this.getAttribute("ufrag"))
+                && Strings.isNullOrEmpty(this.getAttribute("pwd"))
+                && this.children.isEmpty();
     }
 
     public List<Candidate> getCandidates() {