Accept SDP with TRANSPORT attributes missing from bundled m= sections.
Where "TRANSPORT attributes" refers to:
https://tools.ietf.org/html/draft-ietf-mmusic-sdp-mux-attributes-16
The BUNDLE draft now says that these attributes can
(in fact, MUST) be omitted when m= sections are bundled
(they only need to go in one of the bundled m= sections),
so we should start accepting that SDP.
This CL doesn't fix "a=rtcp-mux", unfortunately. That will be easier
to fix once we've split apart an "RtpTransport" object from
BaseChannel.
BUG=webrtc:6351
Review-Url: https://codereview.webrtc.org/2647593003
Cr-Commit-Position: refs/heads/master@{#16782}
diff --git a/webrtc/p2p/base/transportdescriptionfactory.cc b/webrtc/p2p/base/transportdescriptionfactory.cc
index 238e1b6..b431eaa 100644
--- a/webrtc/p2p/base/transportdescriptionfactory.cc
+++ b/webrtc/p2p/base/transportdescriptionfactory.cc
@@ -56,6 +56,7 @@
TransportDescription* TransportDescriptionFactory::CreateAnswer(
const TransportDescription* offer,
const TransportOptions& options,
+ bool require_transport_attributes,
const TransportDescription* current_description) const {
// TODO(juberti): Figure out why we get NULL offers, and fix this upstream.
if (!offer) {
@@ -91,7 +92,7 @@
return NULL;
}
}
- } else if (secure_ == SEC_REQUIRED) {
+ } else if (require_transport_attributes && secure_ == SEC_REQUIRED) {
// We require DTLS, but the other side didn't offer it. Fail.
LOG(LS_WARNING) << "Failed to create TransportDescription answer "
"because of incompatible security settings";
diff --git a/webrtc/p2p/base/transportdescriptionfactory.h b/webrtc/p2p/base/transportdescriptionfactory.h
index 2dde5b1..ac04f04 100644
--- a/webrtc/p2p/base/transportdescriptionfactory.h
+++ b/webrtc/p2p/base/transportdescriptionfactory.h
@@ -53,9 +53,16 @@
TransportDescription* CreateOffer(const TransportOptions& options,
const TransportDescription* current_description) const;
// Create a transport description that is a response to an offer.
+ //
+ // If |require_transport_attributes| is true, then TRANSPORT category
+ // attributes are expected to be present in |offer|, as defined by
+ // sdp-mux-attributes, and null will be returned otherwise. It's expected
+ // that this will be set to false for an m= section that's in a BUNDLE group
+ // but isn't the first m= section in the group.
TransportDescription* CreateAnswer(
const TransportDescription* offer,
const TransportOptions& options,
+ bool require_transport_attributes,
const TransportDescription* current_description) const;
private:
diff --git a/webrtc/p2p/base/transportdescriptionfactory_unittest.cc b/webrtc/p2p/base/transportdescriptionfactory_unittest.cc
index 14be338..195ccd2 100644
--- a/webrtc/p2p/base/transportdescriptionfactory_unittest.cc
+++ b/webrtc/p2p/base/transportdescriptionfactory_unittest.cc
@@ -64,7 +64,7 @@
// The initial offer / answer exchange.
std::unique_ptr<TransportDescription> offer(f1_.CreateOffer(options, NULL));
std::unique_ptr<TransportDescription> answer(
- f2_.CreateAnswer(offer.get(), options, NULL));
+ f2_.CreateAnswer(offer.get(), options, true, NULL));
// Create an updated offer where we restart ice.
options.ice_restart = true;
@@ -76,7 +76,7 @@
// Create a new answer. The transport ufrag and password is changed since
// |options.ice_restart == true|
std::unique_ptr<TransportDescription> restart_answer(
- f2_.CreateAnswer(restart_offer.get(), options, answer.get()));
+ f2_.CreateAnswer(restart_offer.get(), options, true, answer.get()));
ASSERT_TRUE(restart_answer.get() != NULL);
VerifyUfragAndPasswordChanged(dtls, answer.get(), restart_answer.get());
@@ -108,7 +108,7 @@
std::unique_ptr<TransportDescription> offer(
f1_.CreateOffer(options, nullptr));
std::unique_ptr<TransportDescription> answer(
- f2_.CreateAnswer(offer.get(), options, nullptr));
+ f2_.CreateAnswer(offer.get(), options, true, nullptr));
VerifyRenomination(offer.get(), false);
VerifyRenomination(answer.get(), false);
@@ -117,8 +117,8 @@
f1_.CreateOffer(options, offer.get()));
VerifyRenomination(renomination_offer.get(), true);
- std::unique_ptr<TransportDescription> renomination_answer(
- f2_.CreateAnswer(renomination_offer.get(), options, answer.get()));
+ std::unique_ptr<TransportDescription> renomination_answer(f2_.CreateAnswer(
+ renomination_offer.get(), options, true, answer.get()));
VerifyRenomination(renomination_answer.get(), true);
}
@@ -202,10 +202,9 @@
f1_.CreateOffer(TransportOptions(), NULL));
ASSERT_TRUE(offer.get() != NULL);
std::unique_ptr<TransportDescription> desc(
- f2_.CreateAnswer(offer.get(), TransportOptions(), NULL));
+ f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL));
CheckDesc(desc.get(), "", "", "", "");
- desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(),
- NULL));
+ desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL));
CheckDesc(desc.get(), "", "", "", "");
}
@@ -215,10 +214,10 @@
f1_.CreateOffer(TransportOptions(), NULL));
ASSERT_TRUE(offer.get() != NULL);
std::unique_ptr<TransportDescription> old_desc(
- f2_.CreateAnswer(offer.get(), TransportOptions(), NULL));
+ f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL));
ASSERT_TRUE(old_desc.get() != NULL);
std::unique_ptr<TransportDescription> desc(
- f2_.CreateAnswer(offer.get(), TransportOptions(), old_desc.get()));
+ f2_.CreateAnswer(offer.get(), TransportOptions(), true, old_desc.get()));
ASSERT_TRUE(desc.get() != NULL);
CheckDesc(desc.get(), "",
old_desc->ice_ufrag, old_desc->ice_pwd, "");
@@ -232,7 +231,7 @@
f1_.CreateOffer(TransportOptions(), NULL));
ASSERT_TRUE(offer.get() != NULL);
std::unique_ptr<TransportDescription> desc(
- f2_.CreateAnswer(offer.get(), TransportOptions(), NULL));
+ f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL));
CheckDesc(desc.get(), "", "", "", "");
}
@@ -245,11 +244,10 @@
f1_.CreateOffer(TransportOptions(), NULL));
ASSERT_TRUE(offer.get() != NULL);
std::unique_ptr<TransportDescription> desc(
- f2_.CreateAnswer(offer.get(), TransportOptions(), NULL));
+ f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL));
CheckDesc(desc.get(), "", "", "", "");
f2_.set_secure(cricket::SEC_REQUIRED);
- desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(),
- NULL));
+ desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL));
ASSERT_TRUE(desc.get() == NULL);
}
@@ -271,11 +269,10 @@
f1_.CreateOffer(TransportOptions(), NULL));
ASSERT_TRUE(offer.get() != NULL);
std::unique_ptr<TransportDescription> desc(
- f2_.CreateAnswer(offer.get(), TransportOptions(), NULL));
+ f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL));
CheckDesc(desc.get(), "", "", "", digest_alg2);
f2_.set_secure(cricket::SEC_REQUIRED);
- desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(),
- NULL));
+ desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL));
CheckDesc(desc.get(), "", "", "", digest_alg2);
}
diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc
index 000c289..5e38088 100644
--- a/webrtc/pc/mediasession.cc
+++ b/webrtc/pc/mediasession.cc
@@ -22,6 +22,7 @@
#include "webrtc/base/checks.h"
#include "webrtc/base/helpers.h"
#include "webrtc/base/logging.h"
+#include "webrtc/base/optional.h"
#include "webrtc/base/stringutils.h"
#include "webrtc/common_types.h"
#include "webrtc/common_video/h264/profile_level_id.h"
@@ -1430,6 +1431,9 @@
SessionDescription* MediaSessionDescriptionFactory::CreateAnswer(
const SessionDescription* offer, const MediaSessionOptions& options,
const SessionDescription* current_description) const {
+ if (!offer) {
+ return nullptr;
+ }
// The answer contains the intersection of the codecs in the offer with the
// codecs we support. As indicated by XEP-0167, we retain the same payload ids
// from the offer in the answer.
@@ -1438,54 +1442,60 @@
StreamParamsVec current_streams;
GetCurrentStreamParams(current_description, ¤t_streams);
- if (offer) {
- ContentInfos::const_iterator it = offer->contents().begin();
- for (; it != offer->contents().end(); ++it) {
- if (IsMediaContentOfType(&*it, MEDIA_TYPE_AUDIO)) {
- if (!AddAudioContentForAnswer(offer, options, current_description,
- ¤t_streams, answer.get())) {
- return NULL;
- }
- } else if (IsMediaContentOfType(&*it, MEDIA_TYPE_VIDEO)) {
- if (!AddVideoContentForAnswer(offer, options, current_description,
- ¤t_streams, answer.get())) {
- return NULL;
- }
- } else {
- RTC_DCHECK(IsMediaContentOfType(&*it, MEDIA_TYPE_DATA));
- if (!AddDataContentForAnswer(offer, options, current_description,
- ¤t_streams, answer.get())) {
- return NULL;
- }
+ // If the offer supports BUNDLE, and we want to use it too, create a BUNDLE
+ // group in the answer with the appropriate content names.
+ const ContentGroup* offer_bundle = offer->GetGroupByName(GROUP_TYPE_BUNDLE);
+ ContentGroup answer_bundle(GROUP_TYPE_BUNDLE);
+ // Transport info shared by the bundle group.
+ std::unique_ptr<TransportInfo> bundle_transport;
+
+ ContentInfos::const_iterator it = offer->contents().begin();
+ for (; it != offer->contents().end(); ++it) {
+ if (IsMediaContentOfType(&*it, MEDIA_TYPE_AUDIO)) {
+ if (!AddAudioContentForAnswer(offer, options, current_description,
+ bundle_transport.get(), ¤t_streams,
+ answer.get())) {
+ return NULL;
}
+ } else if (IsMediaContentOfType(&*it, MEDIA_TYPE_VIDEO)) {
+ if (!AddVideoContentForAnswer(offer, options, current_description,
+ bundle_transport.get(), ¤t_streams,
+ answer.get())) {
+ return NULL;
+ }
+ } else {
+ RTC_DCHECK(IsMediaContentOfType(&*it, MEDIA_TYPE_DATA));
+ if (!AddDataContentForAnswer(offer, options, current_description,
+ bundle_transport.get(), ¤t_streams,
+ answer.get())) {
+ return NULL;
+ }
+ }
+ // See if we can add the newly generated m= section to the BUNDLE group in
+ // the answer.
+ ContentInfo& added = answer->contents().back();
+ if (!added.rejected && options.bundle_enabled && offer_bundle &&
+ offer_bundle->HasContentName(added.name)) {
+ answer_bundle.AddContentName(added.name);
+ bundle_transport.reset(
+ new TransportInfo(*answer->GetTransportInfoByName(added.name)));
}
}
- // If the offer supports BUNDLE, and we want to use it too, create a BUNDLE
- // group in the answer with the appropriate content names.
- if (offer->HasGroup(GROUP_TYPE_BUNDLE) && options.bundle_enabled) {
- const ContentGroup* offer_bundle = offer->GetGroupByName(GROUP_TYPE_BUNDLE);
- ContentGroup answer_bundle(GROUP_TYPE_BUNDLE);
- for (ContentInfos::const_iterator content = answer->contents().begin();
- content != answer->contents().end(); ++content) {
- if (!content->rejected && offer_bundle->HasContentName(content->name)) {
- answer_bundle.AddContentName(content->name);
- }
+ // Only put BUNDLE group in answer if nonempty.
+ if (answer_bundle.FirstContentName()) {
+ answer->AddGroup(answer_bundle);
+
+ // Share the same ICE credentials and crypto params across all contents,
+ // as BUNDLE requires.
+ if (!UpdateTransportInfoForBundle(answer_bundle, answer.get())) {
+ LOG(LS_ERROR) << "CreateAnswer failed to UpdateTransportInfoForBundle.";
+ return NULL;
}
- if (answer_bundle.FirstContentName()) {
- answer->AddGroup(answer_bundle);
- // Share the same ICE credentials and crypto params across all contents,
- // as BUNDLE requires.
- if (!UpdateTransportInfoForBundle(answer_bundle, answer.get())) {
- LOG(LS_ERROR) << "CreateAnswer failed to UpdateTransportInfoForBundle.";
- return NULL;
- }
-
- if (!UpdateCryptoParamsForBundle(answer_bundle, answer.get())) {
- LOG(LS_ERROR) << "CreateAnswer failed to UpdateCryptoParamsForBundle.";
- return NULL;
- }
+ if (!UpdateCryptoParamsForBundle(answer_bundle, answer.get())) {
+ LOG(LS_ERROR) << "CreateAnswer failed to UpdateCryptoParamsForBundle.";
+ return NULL;
}
}
@@ -1634,16 +1644,17 @@
const std::string& content_name,
const SessionDescription* offer_desc,
const TransportOptions& transport_options,
- const SessionDescription* current_desc) const {
+ const SessionDescription* current_desc,
+ bool require_transport_attributes) const {
if (!transport_desc_factory_)
return NULL;
const TransportDescription* offer_tdesc =
GetTransportDescription(content_name, offer_desc);
const TransportDescription* current_tdesc =
GetTransportDescription(content_name, current_desc);
- return
- transport_desc_factory_->CreateAnswer(offer_tdesc, transport_options,
- current_tdesc);
+ return transport_desc_factory_->CreateAnswer(offer_tdesc, transport_options,
+ require_transport_attributes,
+ current_tdesc);
}
bool MediaSessionDescriptionFactory::AddTransportAnswer(
@@ -1838,15 +1849,17 @@
const SessionDescription* offer,
const MediaSessionOptions& options,
const SessionDescription* current_description,
+ const TransportInfo* bundle_transport,
StreamParamsVec* current_streams,
SessionDescription* answer) const {
const ContentInfo* audio_content = GetFirstAudioContent(offer);
const AudioContentDescription* offer_audio =
static_cast<const AudioContentDescription*>(audio_content->description);
- std::unique_ptr<TransportDescription> audio_transport(CreateTransportAnswer(
- audio_content->name, offer,
- GetTransportOptions(options, audio_content->name), current_description));
+ std::unique_ptr<TransportDescription> audio_transport(
+ CreateTransportAnswer(audio_content->name, offer,
+ GetTransportOptions(options, audio_content->name),
+ current_description, bundle_transport != nullptr));
if (!audio_transport) {
return false;
}
@@ -1885,10 +1898,11 @@
return false; // Fails the session setup.
}
+ bool secure = bundle_transport ? bundle_transport->description.secure()
+ : audio_transport->secure();
bool rejected = !options.has_audio() || audio_content->rejected ||
- !IsMediaProtocolSupported(MEDIA_TYPE_AUDIO,
- audio_answer->protocol(),
- audio_transport->secure());
+ !IsMediaProtocolSupported(MEDIA_TYPE_AUDIO,
+ audio_answer->protocol(), secure);
if (!rejected) {
AddTransportAnswer(audio_content->name, *(audio_transport.get()), answer);
} else {
@@ -1906,12 +1920,14 @@
const SessionDescription* offer,
const MediaSessionOptions& options,
const SessionDescription* current_description,
+ const TransportInfo* bundle_transport,
StreamParamsVec* current_streams,
SessionDescription* answer) const {
const ContentInfo* video_content = GetFirstVideoContent(offer);
- std::unique_ptr<TransportDescription> video_transport(CreateTransportAnswer(
- video_content->name, offer,
- GetTransportOptions(options, video_content->name), current_description));
+ std::unique_ptr<TransportDescription> video_transport(
+ CreateTransportAnswer(video_content->name, offer,
+ GetTransportOptions(options, video_content->name),
+ current_description, bundle_transport != nullptr));
if (!video_transport) {
return false;
}
@@ -1937,10 +1953,11 @@
video_answer.get())) {
return false;
}
+ bool secure = bundle_transport ? bundle_transport->description.secure()
+ : video_transport->secure();
bool rejected = !options.has_video() || video_content->rejected ||
- !IsMediaProtocolSupported(MEDIA_TYPE_VIDEO,
- video_answer->protocol(),
- video_transport->secure());
+ !IsMediaProtocolSupported(MEDIA_TYPE_VIDEO,
+ video_answer->protocol(), secure);
if (!rejected) {
if (!AddTransportAnswer(video_content->name, *(video_transport.get()),
answer)) {
@@ -1961,12 +1978,14 @@
const SessionDescription* offer,
const MediaSessionOptions& options,
const SessionDescription* current_description,
+ const TransportInfo* bundle_transport,
StreamParamsVec* current_streams,
SessionDescription* answer) const {
const ContentInfo* data_content = GetFirstDataContent(offer);
- std::unique_ptr<TransportDescription> data_transport(CreateTransportAnswer(
- data_content->name, offer,
- GetTransportOptions(options, data_content->name), current_description));
+ std::unique_ptr<TransportDescription> data_transport(
+ CreateTransportAnswer(data_content->name, offer,
+ GetTransportOptions(options, data_content->name),
+ current_description, bundle_transport != nullptr));
if (!data_transport) {
return false;
}
@@ -2002,10 +2021,12 @@
bool offer_uses_sctpmap = offer_data_description->use_sctpmap();
data_answer->set_use_sctpmap(offer_uses_sctpmap);
+ bool secure = bundle_transport ? bundle_transport->description.secure()
+ : data_transport->secure();
+
bool rejected = !options.has_data() || data_content->rejected ||
- !IsMediaProtocolSupported(MEDIA_TYPE_DATA,
- data_answer->protocol(),
- data_transport->secure());
+ !IsMediaProtocolSupported(MEDIA_TYPE_DATA,
+ data_answer->protocol(), secure);
if (!rejected) {
data_answer->set_bandwidth(options.data_bandwidth);
if (!AddTransportAnswer(data_content->name, *(data_transport.get()),
diff --git a/webrtc/pc/mediasession.h b/webrtc/pc/mediasession.h
index 68f4c37..f864fe9 100644
--- a/webrtc/pc/mediasession.h
+++ b/webrtc/pc/mediasession.h
@@ -494,7 +494,8 @@
const std::string& content_name,
const SessionDescription* offer_desc,
const TransportOptions& transport_options,
- const SessionDescription* current_desc) const;
+ const SessionDescription* current_desc,
+ bool require_transport_attributes) const;
bool AddTransportAnswer(
const std::string& content_name,
@@ -528,26 +529,26 @@
StreamParamsVec* current_streams,
SessionDescription* desc) const;
- bool AddAudioContentForAnswer(
- const SessionDescription* offer,
- const MediaSessionOptions& options,
- const SessionDescription* current_description,
- StreamParamsVec* current_streams,
- SessionDescription* answer) const;
+ bool AddAudioContentForAnswer(const SessionDescription* offer,
+ const MediaSessionOptions& options,
+ const SessionDescription* current_description,
+ const TransportInfo* bundle_transport,
+ StreamParamsVec* current_streams,
+ SessionDescription* answer) const;
- bool AddVideoContentForAnswer(
- const SessionDescription* offer,
- const MediaSessionOptions& options,
- const SessionDescription* current_description,
- StreamParamsVec* current_streams,
- SessionDescription* answer) const;
+ bool AddVideoContentForAnswer(const SessionDescription* offer,
+ const MediaSessionOptions& options,
+ const SessionDescription* current_description,
+ const TransportInfo* bundle_transport,
+ StreamParamsVec* current_streams,
+ SessionDescription* answer) const;
- bool AddDataContentForAnswer(
- const SessionDescription* offer,
- const MediaSessionOptions& options,
- const SessionDescription* current_description,
- StreamParamsVec* current_streams,
- SessionDescription* answer) const;
+ bool AddDataContentForAnswer(const SessionDescription* offer,
+ const MediaSessionOptions& options,
+ const SessionDescription* current_description,
+ const TransportInfo* bundle_transport,
+ StreamParamsVec* current_streams,
+ SessionDescription* answer) const;
AudioCodecs audio_send_codecs_;
AudioCodecs audio_recv_codecs_;
diff --git a/webrtc/pc/peerconnection_unittest.cc b/webrtc/pc/peerconnection_unittest.cc
index df87135..f62d30b 100644
--- a/webrtc/pc/peerconnection_unittest.cc
+++ b/webrtc/pc/peerconnection_unittest.cc
@@ -517,6 +517,10 @@
void RemoveCvoFromReceivedSdp(bool remove) { remove_cvo_ = remove; }
+ void MakeSpecCompliantMaxBundleOfferFromReceivedSdp(bool real) {
+ make_spec_compliant_max_bundle_offer_ = real;
+ }
+
bool can_receive_audio() {
bool value;
if (prefer_constraint_apis_) {
@@ -1049,6 +1053,33 @@
}
std::unique_ptr<SessionDescriptionInterface> desc(
webrtc::CreateSessionDescription("offer", msg, nullptr));
+
+ // Do the equivalent of setting the port to 0, adding a=bundle-only, and
+ // removing a=ice-ufrag, a=ice-pwd, a=fingerprint and a=setup from all but
+ // the first m= section.
+ if (make_spec_compliant_max_bundle_offer_) {
+ bool first = true;
+ for (cricket::ContentInfo& content : desc->description()->contents()) {
+ if (first) {
+ first = false;
+ continue;
+ }
+ content.bundle_only = true;
+ }
+ first = true;
+ for (cricket::TransportInfo& transport :
+ desc->description()->transport_infos()) {
+ if (first) {
+ first = false;
+ continue;
+ }
+ transport.description.ice_ufrag.clear();
+ transport.description.ice_pwd.clear();
+ transport.description.connection_role = cricket::CONNECTIONROLE_NONE;
+ transport.description.identity_fingerprint.reset(nullptr);
+ }
+ }
+
EXPECT_TRUE(DoSetRemoteDescription(desc.release()));
// Set the RtpReceiverObserver after receivers are created.
SetRtpReceiverObservers();
@@ -1209,6 +1240,8 @@
// |remove_cvo_| is true if extension urn:3gpp:video-orientation should be
// removed in the received SDP.
bool remove_cvo_ = false;
+ // See LocalP2PTestWithSpecCompliantMaxBundleOffer.
+ bool make_spec_compliant_max_bundle_offer_ = false;
rtc::scoped_refptr<DataChannelInterface> data_channel_;
std::unique_ptr<MockDataChannelObserver> data_observer_;
@@ -1849,6 +1882,19 @@
EXPECT_EQ(2u, receiving_client()->number_of_remote_streams());
}
+// Test that if applying a true "max bundle" offer, which uses ports of 0,
+// "a=bundle-only", omitting "a=fingerprint", "a=setup", "a=ice-ufrag" and
+// "a=ice-pwd" for all but the audio "m=" section, negotiation still completes
+// successfully and media flows.
+// TODO(deadbeef): Update this test to also omit "a=rtcp-mux", once that works.
+// TODO(deadbeef): Won't need this test once we start generating actual
+// standards-compliant SDP.
+TEST_F(P2PTestConductor, LocalP2PTestWithSpecCompliantMaxBundleOffer) {
+ ASSERT_TRUE(CreateTestClients());
+ receiving_client()->MakeSpecCompliantMaxBundleOfferFromReceivedSdp(true);
+ LocalP2PTest();
+}
+
// Test that we can receive the audio output level from a remote audio track.
TEST_F(P2PTestConductor, GetAudioOutputLevelStats) {
ASSERT_TRUE(CreateTestClients());
diff --git a/webrtc/pc/webrtcsession.cc b/webrtc/pc/webrtcsession.cc
index 763c19c..22cb611 100644
--- a/webrtc/pc/webrtcsession.cc
+++ b/webrtc/pc/webrtcsession.cc
@@ -163,20 +163,32 @@
}
// Checks that each non-rejected content has SDES crypto keys or a DTLS
-// fingerprint. Mismatches, such as replying with a DTLS fingerprint to SDES
-// keys, will be caught in Transport negotiation, and backstopped by Channel's
-// |srtp_required| check.
+// fingerprint, unless it's in a BUNDLE group, in which case only the
+// BUNDLE-tag section (first media section/description in the BUNDLE group)
+// needs a ufrag and pwd. Mismatches, such as replying with a DTLS fingerprint
+// to SDES keys, will be caught in JsepTransport negotiation, and backstopped
+// by Channel's |srtp_required| check.
static bool VerifyCrypto(const SessionDescription* desc,
bool dtls_enabled,
std::string* error) {
+ const cricket::ContentGroup* bundle =
+ desc->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
const ContentInfos& contents = desc->contents();
for (size_t index = 0; index < contents.size(); ++index) {
const ContentInfo* cinfo = &contents[index];
if (cinfo->rejected) {
continue;
}
+ if (bundle && bundle->HasContentName(cinfo->name) &&
+ cinfo->name != *(bundle->FirstContentName())) {
+ // This isn't the first media section in the BUNDLE group, so it's not
+ // required to have crypto attributes, since only the crypto attributes
+ // from the first section actually get used.
+ continue;
+ }
- // If the content isn't rejected, crypto must be present.
+ // If the content isn't rejected or bundled into another m= section, crypto
+ // must be present.
const MediaContentDescription* media =
static_cast<const MediaContentDescription*>(cinfo->description);
const TransportInfo* tinfo = desc->GetTransportInfoByName(cinfo->name);
@@ -206,16 +218,28 @@
return true;
}
-// Checks that each non-rejected content has ice-ufrag and ice-pwd set.
+// Checks that each non-rejected content has ice-ufrag and ice-pwd set, unless
+// it's in a BUNDLE group, in which case only the BUNDLE-tag section (first
+// media section/description in the BUNDLE group) needs a ufrag and pwd.
static bool VerifyIceUfragPwdPresent(const SessionDescription* desc) {
+ const cricket::ContentGroup* bundle =
+ desc->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
const ContentInfos& contents = desc->contents();
for (size_t index = 0; index < contents.size(); ++index) {
const ContentInfo* cinfo = &contents[index];
if (cinfo->rejected) {
continue;
}
+ if (bundle && bundle->HasContentName(cinfo->name) &&
+ cinfo->name != *(bundle->FirstContentName())) {
+ // This isn't the first media section in the BUNDLE group, so it's not
+ // required to have ufrag/password, since only the ufrag/password from
+ // the first section actually get used.
+ continue;
+ }
- // If the content isn't rejected, ice-ufrag and ice-pwd must be present.
+ // If the content isn't rejected or bundled into another m= section,
+ // ice-ufrag and ice-pwd must be present.
const TransportInfo* tinfo = desc->GetTransportInfoByName(cinfo->name);
if (!tinfo) {
// Something is not right.
