| author | Michael Froman <mfroman@mozilla.com> |
| Wed, 01 Nov 2017 11:56:35 -0500 | |
| changeset 389913 | 6f3b7e0969c97c5c085b52272cccfc9bc3e9c37d |
| parent 389912 | 399dbcf3b66e3c4c71127635b5fe6c46dc340018 |
| child 389914 | 43e56a29736611ceadaa613d2be8168b12f7c45f |
| push id | 54648 |
| push user | mfroman@nostrum.com |
| push date | Thu, 02 Nov 2017 22:08:11 +0000 |
| treeherder | autoland@6f3b7e0969c9 [default view] [failures only] |
| perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
| reviewers | bwc |
| bugs | 1413709, 1405940 |
| milestone | 58.0a1 |
| first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
| last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/dom/media/tests/mochitest/mochitest.ini +++ b/dom/media/tests/mochitest/mochitest.ini @@ -192,16 +192,18 @@ skip-if = toolkit == 'android' [test_peerConnection_restartIceNoBundleNoRtcpMux.html] skip-if = toolkit == 'android' [test_peerConnection_restartIceNoRtcpMux.html] skip-if = toolkit == 'android' [test_peerConnection_restartIceLocalRollback.html] skip-if = toolkit == 'android' [test_peerConnection_restartIceLocalAndRemoteRollback.html] skip-if = toolkit == 'android' +[test_peerConnection_restartIceBadAnswer.html] +skip-if = toolkit == 'android' [test_peerConnection_scaleResolution.html] skip-if = (android_version == '18') # android(Bug 1189784, timeouts on 4.3 emulator) [test_peerConnection_simulcastOffer.html] skip-if = android_version # no simulcast support on android [test_peerConnection_simulcastAnswer.html] skip-if = android_version # no simulcast support on android [test_peerConnection_relayOnly.html] disabled=
new file mode 100644 --- /dev/null +++ b/dom/media/tests/mochitest/test_peerConnection_restartIceBadAnswer.html @@ -0,0 +1,58 @@ +<!DOCTYPE HTML> +<html> +<head> + <script type="application/javascript" src="pc.js"></script> +</head> +<body> +<pre id="test"> +<script type="application/javascript"> + createHTML({ + bug: "1413709", + title: "Renegotiation: bad answer ICE credentials" + }); + + var test; + runNetworkTest(function (options) { + test = new PeerConnectionTest(options); + + addRenegotiation(test.chain, + [ + function PC_LOCAL_ADD_SECOND_STREAM(test) { + test.setMediaConstraints([{audio: true}], + []); + return test.pcLocal.getAllUserMedia([{audio: true}]); + }, + ] + ); + + // If the offerer hasn't indicated ICE restart, then an answer + // arriving during renegotiation that has modified ICE credentials + // should cause an error + test.chain.replaceAfter("PC_LOCAL_GET_ANSWER", + [ + function PC_LOCAL_REWRITE_REMOTE_SDP_ICE_CREDS(test) { + test._remote_answer.sdp = + test._remote_answer.sdp.replace(/a=ice-pwd:.*\r\n/g, + "a=ice-pwd:bad-pwd\r\n") + .replace(/a=ice-ufrag:.*\r\n/g, + "a=ice-ufrag:bad-ufrag\r\n"); + }, + + function PC_LOCAL_EXPECT_SET_REMOTE_DESCRIPTION_FAIL(test) { + return test.setRemoteDescription(test.pcLocal, + test._remote_answer, + STABLE) + .then(() => ok(false, "setRemoteDescription must fail"), + e => is(e.name, "InvalidSessionDescriptionError", + "setRemoteDescription must fail and did")); + } + ], 1 // replace after the second PC_LOCAL_GET_ANSWER + ); + + test.setMediaConstraints([{audio: true}], []); + test.run(); + }); +</script> +</pre> +</body> +</html>
--- a/media/webrtc/signaling/gtest/jsep_session_unittest.cpp +++ b/media/webrtc/signaling/gtest/jsep_session_unittest.cpp @@ -78,36 +78,56 @@ public: mOffCandidates = MakeUnique<CandidateSet>(); mAnsCandidates = MakeUnique<CandidateSet>(); } protected: struct TransportData { std::string mIceUfrag; std::string mIcePwd; + int iceCredentialSerial; std::map<std::string, std::vector<uint8_t> > mFingerprints; }; void + GenerateNewIceCredentials(const JsepSessionImpl& session, + TransportData& tdata) + { + std::ostringstream ostr; + ostr << session.GetName() << "-" << ++tdata.iceCredentialSerial; + + // Values here semi-borrowed from JSEP draft. + tdata.mIceUfrag = ostr.str() + "-ufrag"; + tdata.mIcePwd = ostr.str() + "-1234567890"; + } + + void + ModifyOffererIceCredentials() + { + GenerateNewIceCredentials(*mSessionOff, *mOffererTransport); + mSessionOff->SetIceCredentials(mOffererTransport->mIceUfrag, + mOffererTransport->mIcePwd); + } + + void AddDtlsFingerprint(const std::string& alg, JsepSessionImpl& session, TransportData& tdata) { std::vector<uint8_t> fp; fp.assign((alg == "sha-1") ? 20 : 32, (session.GetName() == "Offerer") ? 0x4f : 0x41); session.AddDtlsFingerprint(alg, fp); tdata.mFingerprints[alg] = fp; } void AddTransportData(JsepSessionImpl& session, TransportData& tdata) { - // Values here semi-borrowed from JSEP draft. - tdata.mIceUfrag = session.GetName() + "-ufrag"; - tdata.mIcePwd = session.GetName() + "-1234567890"; + tdata.iceCredentialSerial = 0; + GenerateNewIceCredentials(session, tdata); session.SetIceCredentials(tdata.mIceUfrag, tdata.mIcePwd); AddDtlsFingerprint("sha-1", session, tdata); AddDtlsFingerprint("sha-256", session, tdata); } std::string CreateOffer(const Maybe<JsepOfferOptions>& options = Nothing()) { @@ -519,16 +539,24 @@ protected: } } std::string CreateAnswer() { JsepAnswerOptions options; std::string answer; + + // detect ice restart and generate new ice credentials (like + // PeerConnectionImpl does). + if (mSessionAns->RemoteIceIsRestarting()) { + GenerateNewIceCredentials(*mSessionAns, *mAnswererTransport); + mSessionAns->SetIceCredentials(mAnswererTransport->mIceUfrag, + mAnswererTransport->mIcePwd); + } nsresult rv = mSessionAns->CreateAnswer(options, &answer); EXPECT_EQ(NS_OK, rv); std::cerr << "ANSWER: " << answer << std::endl; ValidateTransport(*mAnswererTransport, answer); return answer; @@ -4098,16 +4126,103 @@ TEST_F(JsepSessionTest, TestIceOptions) ASSERT_EQ(1U, mSessionOff->GetIceOptions().size()); ASSERT_EQ("trickle", mSessionOff->GetIceOptions()[0]); ASSERT_EQ(1U, mSessionAns->GetIceOptions().size()); ASSERT_EQ("trickle", mSessionAns->GetIceOptions()[0]); } +TEST_F(JsepSessionTest, TestIceRestart) +{ + AddTracks(*mSessionOff, "audio"); + AddTracks(*mSessionAns, "audio"); + std::string offer = CreateOffer(); + SetLocalOffer(offer, CHECK_SUCCESS); + SetRemoteOffer(offer, CHECK_SUCCESS); + std::string answer = CreateAnswer(); + SetLocalAnswer(answer, CHECK_SUCCESS); + SetRemoteAnswer(answer, CHECK_SUCCESS); + + JsepOfferOptions options; + options.mIceRestart = Some(true); + ModifyOffererIceCredentials(); + + std::string reoffer = CreateOffer(Some(options)); + SetLocalOffer(reoffer, CHECK_SUCCESS); + SetRemoteOffer(reoffer, CHECK_SUCCESS); + std::string reanswer = CreateAnswer(); + SetLocalAnswer(reanswer, CHECK_SUCCESS); + SetRemoteAnswer(reanswer, CHECK_SUCCESS); + + UniquePtr<Sdp> parsedOffer(Parse(offer)); + ASSERT_EQ(1U, parsedOffer->GetMediaSectionCount()); + UniquePtr<Sdp> parsedReoffer(Parse(reoffer)); + ASSERT_EQ(1U, parsedReoffer->GetMediaSectionCount()); + + UniquePtr<Sdp> parsedAnswer(Parse(answer)); + ASSERT_EQ(1U, parsedAnswer->GetMediaSectionCount()); + UniquePtr<Sdp> parsedReanswer(Parse(reanswer)); + ASSERT_EQ(1U, parsedReanswer->GetMediaSectionCount()); + + // verify ice pwd/ufrag are present in offer/answer and reoffer/reanswer + auto& offerMediaAttrs = parsedOffer->GetMediaSection(0).GetAttributeList(); + ASSERT_TRUE(offerMediaAttrs.HasAttribute(SdpAttribute::kIcePwdAttribute)); + ASSERT_TRUE(offerMediaAttrs.HasAttribute(SdpAttribute::kIceUfragAttribute)); + + auto& reofferMediaAttrs = parsedReoffer->GetMediaSection(0).GetAttributeList(); + ASSERT_TRUE(reofferMediaAttrs.HasAttribute(SdpAttribute::kIcePwdAttribute)); + ASSERT_TRUE(reofferMediaAttrs.HasAttribute(SdpAttribute::kIceUfragAttribute)); + + auto& answerMediaAttrs = parsedAnswer->GetMediaSection(0).GetAttributeList(); + ASSERT_TRUE(answerMediaAttrs.HasAttribute(SdpAttribute::kIcePwdAttribute)); + ASSERT_TRUE(answerMediaAttrs.HasAttribute(SdpAttribute::kIceUfragAttribute)); + + auto& reanswerMediaAttrs = parsedReanswer->GetMediaSection(0).GetAttributeList(); + ASSERT_TRUE(reanswerMediaAttrs.HasAttribute(SdpAttribute::kIcePwdAttribute)); + ASSERT_TRUE(reanswerMediaAttrs.HasAttribute(SdpAttribute::kIceUfragAttribute)); + + // make sure offer/reoffer ice pwd/ufrag changed on ice restart + ASSERT_NE(offerMediaAttrs.GetIcePwd().c_str(), + reofferMediaAttrs.GetIcePwd().c_str()); + ASSERT_NE(offerMediaAttrs.GetIceUfrag().c_str(), + reofferMediaAttrs.GetIceUfrag().c_str()); + + // make sure answer/reanswer ice pwd/ufrag changed on ice restart + ASSERT_NE(answerMediaAttrs.GetIcePwd().c_str(), + reanswerMediaAttrs.GetIcePwd().c_str()); + ASSERT_NE(answerMediaAttrs.GetIceUfrag().c_str(), + reanswerMediaAttrs.GetIceUfrag().c_str()); +} + +TEST_F(JsepSessionTest, TestAnswererIndicatingIceRestart) +{ + AddTracks(*mSessionOff, "audio"); + AddTracks(*mSessionAns, "audio"); + std::string offer = CreateOffer(); + SetLocalOffer(offer, CHECK_SUCCESS); + SetRemoteOffer(offer, CHECK_SUCCESS); + std::string answer = CreateAnswer(); + SetLocalAnswer(answer, CHECK_SUCCESS); + SetRemoteAnswer(answer, CHECK_SUCCESS); + + // reoffer, but we'll improperly indicate an ice restart in the answer by + // modifying the ice pwd and ufrag + std::string reoffer = CreateOffer(); + SetLocalOffer(reoffer, CHECK_SUCCESS); + SetRemoteOffer(reoffer, CHECK_SUCCESS); + std::string reanswer = CreateAnswer(); + + // change the ice pwd and ufrag + ReplaceInSdp(&reanswer, "Answerer-1-", "bad-2-"); + SetLocalAnswer(reanswer, CHECK_SUCCESS); + nsresult rv = mSessionOff->SetRemoteDescription(kJsepSdpAnswer, reanswer); + ASSERT_NE(NS_OK, rv); // NS_ERROR_INVALID_ARG +} + TEST_F(JsepSessionTest, TestExtmap) { AddTracks(*mSessionOff, "audio"); AddTracks(*mSessionAns, "audio"); // ssrc-audio-level will be extmap 1 for both mSessionOff->AddAudioRtpExtension("foo"); // Default mapping of 3 mSessionOff->AddAudioRtpExtension("bar"); // Default mapping of 4 mSessionAns->AddAudioRtpExtension("bar"); // Default mapping of 4