make jingle->sdp parsing fail on some obvious errors

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java    | 18 
src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java     | 73 
src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java | 43 
3 files changed, 125 insertions(+), 9 deletions(-)

Detailed changes

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

@@ -214,14 +214,26 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web
         if (rtpContentMap == null) {
             throw new IllegalStateException("initiator RTP Content Map has not been set");
         }
+        final SessionDescription offer;
+        try {
+            offer = SessionDescription.of(rtpContentMap);
+        } catch (final IllegalArgumentException e) {
+            Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": unable to process offer", e);
+            //TODO terminate session with application error
+            return;
+        }
+        sendSessionAccept(offer);
+    }
+
+    private void sendSessionAccept(SessionDescription offer) {
         discoverIceServers(iceServers -> {
             setupWebRTC(iceServers);
-            final org.webrtc.SessionDescription offer = new org.webrtc.SessionDescription(
+            final org.webrtc.SessionDescription sdp = new org.webrtc.SessionDescription(
                     org.webrtc.SessionDescription.Type.OFFER,
-                    SessionDescription.of(rtpContentMap).toString()
+                    offer.toString()
             );
             try {
-                this.webRTCWrapper.setRemoteDescription(offer).get();
+                this.webRTCWrapper.setRemoteDescription(sdp).get();
                 org.webrtc.SessionDescription webRTCSessionDescription = this.webRTCWrapper.createAnswer().get();
                 final SessionDescription sessionDescription = SessionDescription.parse(webRTCSessionDescription.description);
                 final RtpContentMap respondingRtpContentMap = RtpContentMap.of(sessionDescription);

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

@@ -158,32 +158,80 @@ public class SessionDescription {
             }
             final ImmutableList.Builder<Integer> formatBuilder = new ImmutableList.Builder<>();
             for (RtpDescription.PayloadType payloadType : description.getPayloadTypes()) {
+                final String id = payloadType.getId();
+                if (Strings.isNullOrEmpty(id)) {
+                    throw new IllegalArgumentException("Payload type is missing id");
+                }
+                if (!isInt(id)) {
+                    throw new IllegalArgumentException("Payload id is not numeric");
+                }
                 formatBuilder.add(payloadType.getIntId());
                 mediaAttributes.put("rtpmap", payloadType.toSdpAttribute());
                 List<RtpDescription.Parameter> parameters = payloadType.getParameters();
                 if (parameters.size() > 0) {
-                    mediaAttributes.put("fmtp", RtpDescription.Parameter.toSdpString(payloadType.getId(), parameters));
+                    mediaAttributes.put("fmtp", RtpDescription.Parameter.toSdpString(id, parameters));
                 }
                 for (RtpDescription.FeedbackNegotiation feedbackNegotiation : payloadType.getFeedbackNegotiations()) {
-                    mediaAttributes.put("rtcp-fb", payloadType.getId() + " " + feedbackNegotiation.getType() + (Strings.isNullOrEmpty(feedbackNegotiation.getSubType()) ? "" : " " + feedbackNegotiation.getSubType()));
+                    final String type = feedbackNegotiation.getType();
+                    final String subtype = feedbackNegotiation.getSubType();
+                    if (Strings.isNullOrEmpty(type)) {
+                        throw new IllegalArgumentException("a feedback for payload-type " + id + " negotiation is missing type");
+                    }
+                    mediaAttributes.put("rtcp-fb", id + " " + type + (Strings.isNullOrEmpty(subtype) ? "" : " " + subtype));
                 }
                 for (RtpDescription.FeedbackNegotiationTrrInt feedbackNegotiationTrrInt : payloadType.feedbackNegotiationTrrInts()) {
-                    mediaAttributes.put("rtcp-fb", payloadType.getId() + " trr-int " + feedbackNegotiationTrrInt.getValue());
+                    mediaAttributes.put("rtcp-fb", id + " trr-int " + feedbackNegotiationTrrInt.getValue());
                 }
             }
 
             for (RtpDescription.FeedbackNegotiation feedbackNegotiation : description.getFeedbackNegotiations()) {
-                mediaAttributes.put("rtcp-fb", "* " + feedbackNegotiation.getType() + (Strings.isNullOrEmpty(feedbackNegotiation.getSubType()) ? "" : " " + feedbackNegotiation.getSubType()));
+                final String type = feedbackNegotiation.getType();
+                final String subtype = feedbackNegotiation.getSubType();
+                if (Strings.isNullOrEmpty(type)) {
+                    throw new IllegalArgumentException("a feedback negotiation is missing type");
+                }
+                mediaAttributes.put("rtcp-fb", "* " + type + (Strings.isNullOrEmpty(subtype) ? "" : " " + subtype));
             }
             for (RtpDescription.FeedbackNegotiationTrrInt feedbackNegotiationTrrInt : description.feedbackNegotiationTrrInts()) {
                 mediaAttributes.put("rtcp-fb", "* trr-int " + feedbackNegotiationTrrInt.getValue());
             }
             for (RtpDescription.RtpHeaderExtension extension : description.getHeaderExtensions()) {
-                mediaAttributes.put("extmap", extension.getId() + " " + extension.getUri());
+                final String id = extension.getId();
+                final String uri = extension.getUri();
+                if (Strings.isNullOrEmpty(id)) {
+                    throw new IllegalArgumentException("A header extension is missing id");
+                }
+                if (Strings.isNullOrEmpty(uri)) {
+                    throw new IllegalArgumentException("A header extension is missing uri");
+                }
+                mediaAttributes.put("extmap", id + " " + uri);
+            }
+            for (RtpDescription.SourceGroup sourceGroup : description.getSourceGroups()) {
+                final String semantics = sourceGroup.getSemantics();
+                final List<String> groups = sourceGroup.getSsrcs();
+                if (Strings.isNullOrEmpty(semantics)) {
+                    throw new IllegalArgumentException("A SSRC group is missing semantics attribute");
+                }
+                if (groups.size() == 0) {
+                    throw new IllegalArgumentException("A SSRC group is missing SSRC ids");
+                }
+                mediaAttributes.put("ssrc-group", String.format("%s %s", semantics, Joiner.on(' ').join(groups)));
             }
             for (RtpDescription.Source source : description.getSources()) {
                 for (RtpDescription.Source.Parameter parameter : source.getParameters()) {
-                    mediaAttributes.put("ssrc", source.getSsrcId() + " " + parameter.getParameterName() + ":" + parameter.getParameterValue());
+                    final String id = source.getSsrcId();
+                    final String parameterName = parameter.getParameterName();
+                    final String parameterValue = parameter.getParameterValue();
+                    if (Strings.isNullOrEmpty(id)) {
+                        throw new IllegalArgumentException("A source specific media attribute is missing the id");
+                    }
+                    if (Strings.isNullOrEmpty(parameterName)) {
+                        throw new IllegalArgumentException("A source specific media attribute is missing its name");
+                    }
+                    if (Strings.isNullOrEmpty(parameterValue)) {
+                        throw new IllegalArgumentException("A source specific media attribute is missing its value");
+                    }
+                    mediaAttributes.put("ssrc", id + " " + parameter.getParameterName() + ":" + parameter.getParameterValue());
                 }
             }
 
@@ -220,6 +268,18 @@ public class SessionDescription {
         }
     }
 
+    public static boolean isInt(final String input) {
+        if (input == null) {
+            return false;
+        }
+        try {
+            Integer.parseInt(input);
+            return true;
+        } catch (NumberFormatException e) {
+            return false;
+        }
+    }
+
     public static Pair<String, String> parseAttribute(final String input) {
         final String[] pair = input.split(":", 2);
         if (pair.length == 2) {
@@ -233,6 +293,7 @@ public class SessionDescription {
     public String toString() {
         final StringBuilder s = new StringBuilder()
                 .append("v=").append(version).append(LINE_DIVIDER)
+                //TODO randomize or static
                 .append("o=- 8770656990916039506 2 IN IP4 127.0.0.1").append(LINE_DIVIDER) //what ever that means
                 .append("s=").append(name).append(LINE_DIVIDER)
                 .append("t=0 0").append(LINE_DIVIDER);

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

@@ -70,6 +70,16 @@ public class RtpDescription extends GenericDescription {
         return builder.build();
     }
 
+    public List<SourceGroup> getSourceGroups() {
+        final ImmutableList.Builder<SourceGroup> builder = new ImmutableList.Builder<>();
+        for (final Element child : this.children) {
+            if ("ssrc-group".equals(child.getName()) && Namespace.JINGLE_RTP_SOURCE_SPECIFIC_MEDIA_ATTRIBUTES.equals(child.getNamespace())) {
+                builder.add(SourceGroup.upgrade(child));
+            }
+        }
+        return builder.build();
+    }
+
     public static RtpDescription upgrade(final Element element) {
         Preconditions.checkArgument("description".equals(element.getName()), "Name of provided element is not description");
         Preconditions.checkArgument(Namespace.JINGLE_APPS_RTP.equals(element.getNamespace()), "Element does not match the jingle rtp namespace");
@@ -458,6 +468,39 @@ public class RtpDescription extends GenericDescription {
 
     }
 
+    public static class SourceGroup extends Element {
+
+        private SourceGroup() {
+            super("ssrc-group", Namespace.JINGLE_RTP_SOURCE_SPECIFIC_MEDIA_ATTRIBUTES);
+        }
+
+        public String getSemantics() {
+            return this.getAttribute("semantics");
+        }
+
+        public List<String> getSsrcs() {
+            ImmutableList.Builder<String> builder = new ImmutableList.Builder<>();
+            for(Element child : this.children) {
+                if ("source".equals(child.getName())) {
+                    final String ssrc = child.getAttribute("ssrc");
+                    if (ssrc != null) {
+                        builder.add(ssrc);
+                    }
+                }
+            }
+            return builder.build();
+        }
+
+        public static SourceGroup upgrade(final Element element) {
+            Preconditions.checkArgument("ssrc-group".equals(element.getName()));
+            Preconditions.checkArgument(Namespace.JINGLE_RTP_SOURCE_SPECIFIC_MEDIA_ATTRIBUTES.equals(element.getNamespace()));
+            final SourceGroup group = new SourceGroup();
+            group.setChildren(element.getChildren());
+            group.setAttributes(element.getAttributes());
+            return group;
+        }
+    }
+
     public enum Media {
         VIDEO, AUDIO, UNKNOWN;