Bug 1323723: enforce a=setup in SDP offers. r?bwc draft
authorNils Ohlmeier [:drno] <drno@ohlmeier.org>
Tue, 14 Mar 2017 14:39:54 -0700
changeset 498507 b66168dc2e4c739b34b38f5e518ddec7ea330f6c
parent 497813 727412152afc7cf3530ce0fd9f2199aa84aa33a0
child 498508 468af317341243087dfb4afeca59e71d2c39e0d0
push id49219
push userdrno@ohlmeier.org
push dateTue, 14 Mar 2017 22:01:56 +0000
reviewerssetup, bwc
bugs1323723
milestone55.0a1
Bug 1323723: enforce a=setup in SDP offers. r?bwc MozReview-Commit-ID: 88y11CrXdhf
media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
media/webrtc/signaling/src/jsep/JsepSessionImpl.h
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -1696,20 +1696,20 @@ JsepSessionImpl::ParseSdp(const std::str
     const SdpFingerprintAttributeList& fingerprints(
         mediaAttrs.GetFingerprint());
     if (fingerprints.mFingerprints.empty()) {
       JSEP_SET_ERROR("Invalid description, no supported fingerprint algorithms "
                      "present");
       return NS_ERROR_INVALID_ARG;
     }
 
-    if (mediaAttrs.HasAttribute(SdpAttribute::kSetupAttribute) &&
+    if (mediaAttrs.HasAttribute(SdpAttribute::kSetupAttribute, true) &&
         mediaAttrs.GetSetup().mRole == SdpSetupAttribute::kHoldconn) {
       JSEP_SET_ERROR("Description has illegal setup attribute "
-                     "\"holdconn\" at level "
+                     "\"holdconn\" in m-section at level "
                      << i);
       return NS_ERROR_INVALID_ARG;
     }
 
     std::string streamId;
     std::string trackId;
     nsresult rv = mSdpHelper.GetIdsFromMsid(*parsed,
                                             parsed->GetMediaSection(i),
@@ -1758,19 +1758,22 @@ JsepSessionImpl::ParseSdp(const std::str
   return NS_OK;
 }
 
 nsresult
 JsepSessionImpl::SetRemoteDescriptionOffer(UniquePtr<Sdp> offer)
 {
   MOZ_ASSERT(mState == kJsepStateStable);
 
+  nsresult rv = ValidateOffer(*offer);
+  NS_ENSURE_SUCCESS(rv, rv);
+
   // TODO(bug 1095780): Note that we create remote tracks even when
   // They contain only codecs we can't negotiate or other craziness.
-  nsresult rv = SetRemoteTracksFromDescription(offer.get());
+  rv = SetRemoteTracksFromDescription(offer.get());
   NS_ENSURE_SUCCESS(rv, rv);
 
   mPendingRemoteDescription = Move(offer);
 
   SetState(kJsepStateHaveRemoteOffer);
   return NS_OK;
 }
 
@@ -1984,16 +1987,36 @@ JsepSessionImpl::ValidateRemoteDescripti
       return NS_ERROR_INVALID_ARG;
     }
   }
 
   return NS_OK;
 }
 
 nsresult
+JsepSessionImpl::ValidateOffer(const Sdp& offer)
+{
+  for (size_t i = 0; i < offer.GetMediaSectionCount(); ++i) {
+    const SdpMediaSection& offerMsection = offer.GetMediaSection(i);
+    if (mSdpHelper.MsectionIsDisabled(offerMsection)) {
+      continue;
+    }
+
+    const SdpAttributeList& offerAttrs(offerMsection.GetAttributeList());
+    if (!offerAttrs.HasAttribute(SdpAttribute::kSetupAttribute, true)) {
+      JSEP_SET_ERROR("Offer is missing required setup attribute "
+                     " at level " << i);
+      return NS_ERROR_INVALID_ARG;
+    }
+  }
+
+  return NS_OK;
+}
+
+nsresult
 JsepSessionImpl::ValidateAnswer(const Sdp& offer, const Sdp& answer)
 {
   if (offer.GetMediaSectionCount() != answer.GetMediaSectionCount()) {
     JSEP_SET_ERROR("Offer and answer have different number of m-lines "
                    << "(" << offer.GetMediaSectionCount() << " vs "
                    << answer.GetMediaSectionCount() << ")");
     return NS_ERROR_INVALID_ARG;
   }
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
@@ -221,16 +221,17 @@ private:
   // Non-const so it can set mLastError
   nsresult ParseSdp(const std::string& sdp, UniquePtr<Sdp>* parsedp);
   nsresult SetLocalDescriptionOffer(UniquePtr<Sdp> offer);
   nsresult SetLocalDescriptionAnswer(JsepSdpType type, UniquePtr<Sdp> answer);
   nsresult SetRemoteDescriptionOffer(UniquePtr<Sdp> offer);
   nsresult SetRemoteDescriptionAnswer(JsepSdpType type, UniquePtr<Sdp> answer);
   nsresult ValidateLocalDescription(const Sdp& description);
   nsresult ValidateRemoteDescription(const Sdp& description);
+  nsresult ValidateOffer(const Sdp& offer);
   nsresult ValidateAnswer(const Sdp& offer, const Sdp& answer);
   nsresult SetRemoteTracksFromDescription(const Sdp* remoteDescription);
   // Non-const because we use our Uuid generator
   nsresult CreateReceivingTrack(size_t mline,
                                 const Sdp& sdp,
                                 const SdpMediaSection& msection,
                                 RefPtr<JsepTrack>* track);
   nsresult HandleNegotiatedSession(const UniquePtr<Sdp>& local,