From 0bf991d95c48adabd4084a0aa110cb1f81996099 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 9 Apr 2020 07:38:12 +0200 Subject: [PATCH] make jingle->sdp parsing fail on some obvious errors --- .../xmpp/jingle/JingleRtpConnection.java | 18 ++++- .../xmpp/jingle/SessionDescription.java | 73 +++++++++++++++++-- .../xmpp/jingle/stanzas/RtpDescription.java | 43 +++++++++++ 3 files changed, 125 insertions(+), 9 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 da9e99908..546c35f48 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/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); 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 c205ab0d8..587fb513c 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/SessionDescription.java @@ -158,32 +158,80 @@ public class SessionDescription { } final ImmutableList.Builder 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 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 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 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); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java index 53621a6d8..59881b089 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/RtpDescription.java +++ b/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 getSourceGroups() { + final ImmutableList.Builder 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 getSsrcs() { + ImmutableList.Builder 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;