diff --git a/webrtc/p2p/base/transportdescriptionfactory.cc b/webrtc/p2p/base/transportdescriptionfactory.cc index 238e1b66a1..b431eaaa1b 100644 --- a/webrtc/p2p/base/transportdescriptionfactory.cc +++ b/webrtc/p2p/base/transportdescriptionfactory.cc @@ -56,6 +56,7 @@ TransportDescription* TransportDescriptionFactory::CreateOffer( 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 @@ TransportDescription* TransportDescriptionFactory::CreateAnswer( 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 2dde5b1258..ac04f048c6 100644 --- a/webrtc/p2p/base/transportdescriptionfactory.h +++ b/webrtc/p2p/base/transportdescriptionfactory.h @@ -53,9 +53,16 @@ class TransportDescriptionFactory { 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 14be338a0b..195ccd239b 100644 --- a/webrtc/p2p/base/transportdescriptionfactory_unittest.cc +++ b/webrtc/p2p/base/transportdescriptionfactory_unittest.cc @@ -64,7 +64,7 @@ class TransportDescriptionFactoryTest : public testing::Test { // The initial offer / answer exchange. std::unique_ptr offer(f1_.CreateOffer(options, NULL)); std::unique_ptr 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 @@ class TransportDescriptionFactoryTest : public testing::Test { // Create a new answer. The transport ufrag and password is changed since // |options.ice_restart == true| std::unique_ptr 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 @@ class TransportDescriptionFactoryTest : public testing::Test { std::unique_ptr offer( f1_.CreateOffer(options, nullptr)); std::unique_ptr 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 @@ class TransportDescriptionFactoryTest : public testing::Test { f1_.CreateOffer(options, offer.get())); VerifyRenomination(renomination_offer.get(), true); - std::unique_ptr renomination_answer( - f2_.CreateAnswer(renomination_offer.get(), options, answer.get())); + std::unique_ptr renomination_answer(f2_.CreateAnswer( + renomination_offer.get(), options, true, answer.get())); VerifyRenomination(renomination_answer.get(), true); } @@ -202,10 +202,9 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerDefault) { f1_.CreateOffer(TransportOptions(), NULL)); ASSERT_TRUE(offer.get() != NULL); std::unique_ptr 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 @@ TEST_F(TransportDescriptionFactoryTest, TestReanswer) { f1_.CreateOffer(TransportOptions(), NULL)); ASSERT_TRUE(offer.get() != NULL); std::unique_ptr old_desc( - f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); + f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL)); ASSERT_TRUE(old_desc.get() != NULL); std::unique_ptr 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 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToNoDtls) { f1_.CreateOffer(TransportOptions(), NULL)); ASSERT_TRUE(offer.get() != NULL); std::unique_ptr desc( - f2_.CreateAnswer(offer.get(), TransportOptions(), NULL)); + f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL)); CheckDesc(desc.get(), "", "", "", ""); } @@ -245,11 +244,10 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerNoDtlsToDtls) { f1_.CreateOffer(TransportOptions(), NULL)); ASSERT_TRUE(offer.get() != NULL); std::unique_ptr 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 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToDtls) { f1_.CreateOffer(TransportOptions(), NULL)); ASSERT_TRUE(offer.get() != NULL); std::unique_ptr 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 000c289c18..5e38088389 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::CreateOffer( 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 @@ SessionDescription* MediaSessionDescriptionFactory::CreateAnswer( 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 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 @@ TransportDescription* MediaSessionDescriptionFactory::CreateTransportAnswer( 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 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( 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(audio_content->description); - std::unique_ptr audio_transport(CreateTransportAnswer( - audio_content->name, offer, - GetTransportOptions(options, audio_content->name), current_description)); + std::unique_ptr 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 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer( 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 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( 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 video_transport(CreateTransportAnswer( - video_content->name, offer, - GetTransportOptions(options, video_content->name), current_description)); + std::unique_ptr 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 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer( 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 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer( 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 data_transport(CreateTransportAnswer( - data_content->name, offer, - GetTransportOptions(options, data_content->name), current_description)); + std::unique_ptr 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 MediaSessionDescriptionFactory::AddDataContentForAnswer( 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 68f4c37099..f864fe99b7 100644 --- a/webrtc/pc/mediasession.h +++ b/webrtc/pc/mediasession.h @@ -494,7 +494,8 @@ class MediaSessionDescriptionFactory { 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 @@ class MediaSessionDescriptionFactory { 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 df871356db..f62d30baf4 100644 --- a/webrtc/pc/peerconnection_unittest.cc +++ b/webrtc/pc/peerconnection_unittest.cc @@ -517,6 +517,10 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, 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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, } std::unique_ptr 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 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, // |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 data_channel_; std::unique_ptr data_observer_; @@ -1849,6 +1882,19 @@ TEST_F(P2PTestConductor, LocalP2PTestTwoStreams) { 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 763c19cc16..22cb611740 100644 --- a/webrtc/pc/webrtcsession.cc +++ b/webrtc/pc/webrtcsession.cc @@ -163,20 +163,32 @@ static bool VerifyMediaDescriptions( } // 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(cinfo->description); const TransportInfo* tinfo = desc->GetTransportInfoByName(cinfo->name); @@ -206,16 +218,28 @@ static bool VerifyCrypto(const SessionDescription* desc, 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.