From 7aa1a4767f14b58924822a0b7b30b265870fa806 Mon Sep 17 00:00:00 2001 From: "buildbot@webrtc.org" Date: Fri, 23 May 2014 17:33:05 +0000 Subject: [PATCH] (Auto)update libjingle 67848628-> 67848776 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6237 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/webrtcsession_unittest.cc | 83 +++++++++++++++++++++++ talk/p2p/base/constants.cc | 4 ++ talk/p2p/base/constants.h | 4 ++ talk/p2p/base/transport.cc | 30 +++++++- 4 files changed, 119 insertions(+), 2 deletions(-) diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index b7464935ba..b6f726b345 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -124,6 +124,12 @@ static const char kFakeDtlsFingerprint[] = "BB:CD:72:F7:2F:D0:BA:43:F3:68:B1:0C:23:72:B6:4A:" "0F:DE:34:06:BC:E0:FE:01:BC:73:C8:6D:F4:65:D5:24"; +static const char kTooLongIceUfragPwd[] = + "IceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfrag" + "IceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfrag" + "IceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfrag" + "IceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfragIceUfrag"; + // Add some extra |newlines| to the |message| after |line|. static void InjectAfter(const std::string& line, const std::string& newlines, @@ -572,6 +578,35 @@ class WebRtcSessionTest : public testing::Test { } } + void ModifyIceUfragPwdLines(const SessionDescriptionInterface* current_desc, + const std::string& modified_ice_ufrag, + const std::string& modified_ice_pwd, + std::string* sdp) { + const cricket::SessionDescription* desc = current_desc->description(); + EXPECT_TRUE(current_desc->ToString(sdp)); + + const cricket::ContentInfos& contents = desc->contents(); + cricket::ContentInfos::const_iterator it = contents.begin(); + // Replace ufrag and pwd lines with |modified_ice_ufrag| and + // |modified_ice_pwd| strings. + for (; it != contents.end(); ++it) { + const cricket::TransportDescription* transport_desc = + desc->GetTransportDescriptionByName(it->name); + std::string ufrag_line = "a=ice-ufrag:" + transport_desc->ice_ufrag + + "\r\n"; + std::string pwd_line = "a=ice-pwd:" + transport_desc->ice_pwd + + "\r\n"; + std::string mod_ufrag = "a=ice-ufrag:" + modified_ice_ufrag + "\r\n"; + std::string mod_pwd = "a=ice-pwd:" + modified_ice_pwd + "\r\n"; + talk_base::replace_substrs(ufrag_line.c_str(), ufrag_line.length(), + mod_ufrag.c_str(), mod_ufrag.length(), + sdp); + talk_base::replace_substrs(pwd_line.c_str(), pwd_line.length(), + mod_pwd.c_str(), mod_pwd.length(), + sdp); + } + } + // Creates a remote offer and and applies it as a remote description, // creates a local answer and applies is as a local description. // Call mediastream_signaling_.UseOptionsWithStreamX() before this function @@ -2248,6 +2283,54 @@ TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionWithoutIce) { SetRemoteDescriptionOfferExpectError(kSdpWithoutIceUfragPwd, modified_offer); } +// This test verifies that setLocalDescription fails if local offer has +// too short ice ufrag and pwd strings. +TEST_F(WebRtcSessionTest, TestSetLocalDescriptionInvalidIceCredentials) { + Init(NULL); + tdesc_factory_->set_protocol(cricket::ICEPROTO_RFC5245); + mediastream_signaling_.SendAudioVideoStream1(); + talk_base::scoped_ptr offer(CreateOffer(NULL)); + std::string sdp; + // Modifying ice ufrag and pwd in local offer with strings smaller than the + // recommended values of 4 and 22 bytes respectively. + ModifyIceUfragPwdLines(offer.get(), "ice", "icepwd", &sdp); + SessionDescriptionInterface* modified_offer = + CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); + std::string error; + EXPECT_FALSE(session_->SetLocalDescription(modified_offer, &error)); + + // Test with string greater than 256. + sdp.clear(); + ModifyIceUfragPwdLines(offer.get(), kTooLongIceUfragPwd, kTooLongIceUfragPwd, + &sdp); + modified_offer = CreateSessionDescription(JsepSessionDescription::kOffer, sdp, + NULL); + EXPECT_FALSE(session_->SetLocalDescription(modified_offer, &error)); +} + +// This test verifies that setRemoteDescription fails if remote offer has +// too short ice ufrag and pwd strings. +TEST_F(WebRtcSessionTest, TestSetRemoteDescriptionInvalidIceCredentials) { + Init(NULL); + tdesc_factory_->set_protocol(cricket::ICEPROTO_RFC5245); + talk_base::scoped_ptr offer(CreateRemoteOffer()); + std::string sdp; + // Modifying ice ufrag and pwd in remote offer with strings smaller than the + // recommended values of 4 and 22 bytes respectively. + ModifyIceUfragPwdLines(offer.get(), "ice", "icepwd", &sdp); + SessionDescriptionInterface* modified_offer = + CreateSessionDescription(JsepSessionDescription::kOffer, sdp, NULL); + std::string error; + EXPECT_FALSE(session_->SetRemoteDescription(modified_offer, &error)); + + sdp.clear(); + ModifyIceUfragPwdLines(offer.get(), kTooLongIceUfragPwd, kTooLongIceUfragPwd, + &sdp); + modified_offer = CreateSessionDescription(JsepSessionDescription::kOffer, sdp, + NULL); + EXPECT_FALSE(session_->SetRemoteDescription(modified_offer, &error)); +} + TEST_F(WebRtcSessionTest, VerifyBundleFlagInPA) { // This test verifies BUNDLE flag in PortAllocator, if BUNDLE information in // local description is removed by the application, BUNDLE flag should be diff --git a/talk/p2p/base/constants.cc b/talk/p2p/base/constants.cc index 14cac88024..2e57d9d18d 100644 --- a/talk/p2p/base/constants.cc +++ b/talk/p2p/base/constants.cc @@ -176,6 +176,10 @@ const int ICE_UFRAG_LENGTH = 16; // Minimum password length of 22 characters as per RFC5245. We chose 24 because // some internal systems expect password to be multiple of 4. const int ICE_PWD_LENGTH = 24; +const size_t ICE_UFRAG_MIN_LENGTH = 4; +const size_t ICE_PWD_MIN_LENGTH = 22; +const size_t ICE_UFRAG_MAX_LENGTH = 255; +const size_t ICE_PWD_MAX_LENGTH = 256; // TODO: This is media-specific, so might belong // somewhere like media/base/constants.h const int ICE_CANDIDATE_COMPONENT_RTP = 1; diff --git a/talk/p2p/base/constants.h b/talk/p2p/base/constants.h index 99e006a018..61dd815b38 100644 --- a/talk/p2p/base/constants.h +++ b/talk/p2p/base/constants.h @@ -178,6 +178,10 @@ extern const char ICE_CANDIDATE_TYPE_PEER_STUN[]; extern const char ICE_CANDIDATE_TYPE_SERVER_STUN[]; extern const int ICE_UFRAG_LENGTH; extern const int ICE_PWD_LENGTH; +extern const size_t ICE_UFRAG_MIN_LENGTH; +extern const size_t ICE_PWD_MIN_LENGTH; +extern const size_t ICE_UFRAG_MAX_LENGTH; +extern const size_t ICE_PWD_MAX_LENGTH; extern const int ICE_CANDIDATE_COMPONENT_RTP; extern const int ICE_CANDIDATE_COMPONENT_RTCP; extern const int ICE_CANDIDATE_COMPONENT_DEFAULT; diff --git a/talk/p2p/base/transport.cc b/talk/p2p/base/transport.cc index 8261f723eb..16087e3f06 100644 --- a/talk/p2p/base/transport.cc +++ b/talk/p2p/base/transport.cc @@ -94,6 +94,22 @@ static std::string IceProtoToString(TransportProtocol proto) { return proto_str; } +static bool VerifyIceParams(const TransportDescription& desc) { + // For legacy protocols. + if (desc.ice_ufrag.empty() && desc.ice_pwd.empty()) + return true; + + if (desc.ice_ufrag.length() < ICE_UFRAG_MIN_LENGTH || + desc.ice_ufrag.length() > ICE_UFRAG_MAX_LENGTH) { + return false; + } + if (desc.ice_pwd.length() < ICE_PWD_MIN_LENGTH || + desc.ice_pwd.length() > ICE_PWD_MAX_LENGTH) { + return false; + } + return true; +} + bool BadTransportDescription(const std::string& desc, std::string* err_desc) { if (err_desc) { *err_desc = desc; @@ -704,8 +720,13 @@ bool Transport::SetLocalTransportDescription_w( std::string* error_desc) { bool ret = true; talk_base::CritScope cs(&crit_); - local_description_.reset(new TransportDescription(desc)); + if (!VerifyIceParams(desc)) { + return BadTransportDescription("Invalid ice-ufrag or ice-pwd length", + error_desc); + } + + local_description_.reset(new TransportDescription(desc)); for (ChannelMap::iterator iter = channels_.begin(); iter != channels_.end(); ++iter) { ret &= ApplyLocalTransportDescription_w(iter->second.get(), error_desc); @@ -726,8 +747,13 @@ bool Transport::SetRemoteTransportDescription_w( std::string* error_desc) { bool ret = true; talk_base::CritScope cs(&crit_); - remote_description_.reset(new TransportDescription(desc)); + if (!VerifyIceParams(desc)) { + return BadTransportDescription("Invalid ice-ufrag or ice-pwd length", + error_desc); + } + + remote_description_.reset(new TransportDescription(desc)); for (ChannelMap::iterator iter = channels_.begin(); iter != channels_.end(); ++iter) { ret &= ApplyRemoteTransportDescription_w(iter->second.get(), error_desc);