Allow remote fingerprint update during a call

talk/app/webrtc/peerconnection_unittest.cc

 /*
  * libjingle
  * Copyright 2012 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
@@ skipping 126 matching lines @@
 
  protected:
   SignalingMessageReceiver() {}
   virtual ~SignalingMessageReceiver() {}
 };
 
 class PeerConnectionTestClient : public webrtc::PeerConnectionObserver,
                                  public SignalingMessageReceiver,
                                  public ObserverInterface {
  public:
-  static PeerConnectionTestClient* CreateClient(
+  static PeerConnectionTestClient* CreateClientWithKeySpecified(
       const std::string& id,
       const MediaConstraintsInterface* constraints,
-      const PeerConnectionFactory::Options* options) {
+      const PeerConnectionFactory::Options* options,
+      bool use_alternate_key) {

[pthatcher1, 2015/11/30 20:23:10]

Why not just CreateClientWithAlternateKey and then leave off the
"use_alternate_key" variable and just assume it's true?  

[guoweis_webrtc, 2015/12/01 22:05:12]

I think this will duplicate the code in this function, which I'm trying to
avoid. Let me know if I miss something.

     PeerConnectionTestClient* client(new PeerConnectionTestClient(id));
-    if (!client->Init(constraints, options)) {
+    if (!client->Init(constraints, options, use_alternate_key)) {
       delete client;
       return nullptr;
     }
     return client;
   }
 
+  static PeerConnectionTestClient* CreateClient(
+      const std::string& id,
+      const MediaConstraintsInterface* constraints,
+      const PeerConnectionFactory::Options* options) {
+    return CreateClientWithKeySpecified(id, constraints, options, false);
+  }
+
   ~PeerConnectionTestClient() {
     while (!fake_video_renderers_.empty()) {
       RenderMap::iterator it = fake_video_renderers_.begin();
       delete it->second;
       fake_video_renderers_.erase(it);
     }
   }
 
   void Negotiate() { Negotiate(true, true); }
 
@@ skipping 510 matching lines @@
     bool completed() const { return completed_; }
 
    private:
     bool completed_;
     std::vector<std::string> tones_;
   };
 
   explicit PeerConnectionTestClient(const std::string& id) : id_(id) {}
 
   bool Init(const MediaConstraintsInterface* constraints,
-            const PeerConnectionFactory::Options* options) {
+            const PeerConnectionFactory::Options* options,
+            bool use_alternate_key) {
     EXPECT_TRUE(!peer_connection_);
     EXPECT_TRUE(!peer_connection_factory_);
     allocator_factory_ = webrtc::FakePortAllocatorFactory::Create();
     if (!allocator_factory_) {
       return false;
     }
     fake_audio_capture_module_ = FakeAudioCaptureModule::Create();
 
     if (fake_audio_capture_module_ == nullptr) {
       return false;
     }
     fake_video_decoder_factory_ = new FakeWebRtcVideoDecoderFactory();
     fake_video_encoder_factory_ = new FakeWebRtcVideoEncoderFactory();
     peer_connection_factory_ = webrtc::CreatePeerConnectionFactory(
         rtc::Thread::Current(), rtc::Thread::Current(),
         fake_audio_capture_module_, fake_video_encoder_factory_,
         fake_video_decoder_factory_);
     if (!peer_connection_factory_) {
       return false;
     }
     if (options) {
       peer_connection_factory_->SetOptions(*options);
     }
     peer_connection_ = CreatePeerConnection(allocator_factory_.get(),
-                                            constraints);
+                                            constraints, use_alternate_key);
     return peer_connection_.get() != nullptr;
   }
 
   rtc::scoped_refptr<webrtc::VideoTrackInterface>
   CreateLocalVideoTrack(const std::string stream_label) {
     // Set max frame rate to 10fps to reduce the risk of the tests to be flaky.
     FakeConstraints source_constraints = video_constraints_;
     source_constraints.SetMandatoryMaxFrameRate(10);
 
     cricket::FakeVideoCapturer* fake_capturer =
         new webrtc::FakePeriodicVideoCapturer();
     video_capturers_.push_back(fake_capturer);
     rtc::scoped_refptr<webrtc::VideoSourceInterface> source =
         peer_connection_factory_->CreateVideoSource(
             fake_capturer, &source_constraints);
     std::string label = stream_label + kVideoTrackLabelBase;
     return peer_connection_factory_->CreateVideoTrack(label, source);
   }
 
   rtc::scoped_refptr<webrtc::PeerConnectionInterface> CreatePeerConnection(
       webrtc::PortAllocatorFactoryInterface* factory,
-      const MediaConstraintsInterface* constraints) {
+      const MediaConstraintsInterface* constraints,
+      bool use_alternate_key) {
     // CreatePeerConnection with IceServers.
     webrtc::PeerConnectionInterface::IceServers ice_servers;
     webrtc::PeerConnectionInterface::IceServer ice_server;
     ice_server.uri = "stun:stun.l.google.com:19302";
     ice_servers.push_back(ice_server);
 
-    rtc::scoped_ptr<webrtc::DtlsIdentityStoreInterface> dtls_identity_store(
+    rtc::scoped_ptr<FakeDtlsIdentityStore> dtls_identity_store(
         rtc::SSLStreamAdapter::HaveDtlsSrtp() ? new FakeDtlsIdentityStore()
                                               : nullptr);
+
+    if (use_alternate_key) {
+      dtls_identity_store->use_alternate_key();
+    } else {
+      dtls_identity_store->use_original_key();
+    }
     return peer_connection_factory_->CreatePeerConnection(
         ice_servers, constraints, factory, dtls_identity_store.Pass(), this);
   }
 
   void HandleIncomingOffer(const std::string& msg) {
     LOG(INFO) << id_ << "HandleIncomingOffer ";
     if (NumberOfLocalMediaStreams() == 0 && auto_add_stream_) {
       // If we are not sending any streams ourselves it is time to add some.
       AddMediaStream(true, true);
     }
@@ skipping 203 matching lines @@
     receiving_client_->VerifyLocalIceUfragAndPassword();
   }
 
   ~MAYBE_JsepPeerConnectionP2PTestClient() {
     if (initiating_client_) {
       initiating_client_->set_signaling_message_receiver(nullptr);
     }
     if (receiving_client_) {
       receiving_client_->set_signaling_message_receiver(nullptr);
     }
+    // The clients have to be explicitly closed. Otherwise, they will be
+    // disposed after VirtualSocketServer which they still have dependency
+    // during the destructing time.

[pthatcher1, 2015/11/30 20:23:11]

which => on which
dependency => a dependency
during the destructing time => when destroyed

[guoweis_webrtc, 2015/12/01 22:05:12]

Done.

+    initiating_client_->pc()->Close();
+    receiving_client_->pc()->Close();
   }
 
   bool CreateTestClients() { return CreateTestClients(nullptr, nullptr); }
 
   bool CreateTestClients(MediaConstraintsInterface* init_constraints,
                          MediaConstraintsInterface* recv_constraints) {
     return CreateTestClients(init_constraints, nullptr, recv_constraints,
                              nullptr);
   }
 
+  void SetSignalingReceivers() {
+    initiating_client_->set_signaling_message_receiver(receiving_client_.get());
+    receiving_client_->set_signaling_message_receiver(initiating_client_.get());
+  }
+
   bool CreateTestClients(MediaConstraintsInterface* init_constraints,
                          PeerConnectionFactory::Options* init_options,
                          MediaConstraintsInterface* recv_constraints,
                          PeerConnectionFactory::Options* recv_options) {
     initiating_client_.reset(PeerConnectionTestClient::CreateClient(
         "Caller: ", init_constraints, init_options));
     receiving_client_.reset(PeerConnectionTestClient::CreateClient(
         "Callee: ", recv_constraints, recv_options));
     if (!initiating_client_ || !receiving_client_) {
       return false;
     }
-    initiating_client_->set_signaling_message_receiver(receiving_client_.get());
-    receiving_client_->set_signaling_message_receiver(initiating_client_.get());
+    SetSignalingReceivers();
     return true;
   }
 
   void SetVideoConstraints(const webrtc::FakeConstraints& init_constraints,
                            const webrtc::FakeConstraints& recv_constraints) {
     initiating_client_->SetVideoConstraints(init_constraints);
     receiving_client_->SetVideoConstraints(recv_constraints);
   }
 
   void EnableVideoDecoderFactory() {
@@ skipping 57 matching lines @@
         receiving_client_->can_receive_video()) {
       EXPECT_EQ_WAIT(webrtc::PeerConnectionInterface::kIceGatheringComplete,
                      receiving_client_->ice_gathering_state(),
                      kMaxWaitForFramesMs);
     }
 
     EXPECT_TRUE_WAIT(FramesNotPending(audio_frame_count, video_frame_count),
                      kMaxWaitForFramesMs);
   }
 
+  void SetupAndVerifyDtlsCall() {
+    MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
+    FakeConstraints setup_constraints;
+    setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp,
+                                   true);
+    ASSERT_TRUE(CreateTestClients(&setup_constraints, &setup_constraints));
+    LocalP2PTest();
+    VerifyRenderedSize(640, 480);
+  }
+
+  PeerConnectionTestClient* CreateDtlsClientWithAlternateKey() {
+    FakeConstraints setup_constraints;
+    setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp,
+                                   true);
+
+    // Make sure the new client is using a different certificate.
+    return PeerConnectionTestClient::CreateClientWithKeySpecified(
+        "New Peer: ", &setup_constraints, nullptr, true);
+  }
+
   void SendRtpData(webrtc::DataChannelInterface* dc, const std::string& data) {
     // Messages may get lost on the unreliable DataChannel, so we send multiple
     // times to avoid test flakiness.
     static const size_t kSendAttempts = 5;
 
     for (size_t i = 0; i < kSendAttempts; ++i) {
       dc->Send(DataBuffer(data));
     }
   }
 
   PeerConnectionTestClient* initializing_client() {
     return initiating_client_.get();
   }
+
+  // Reset the |initiating_client_| to the |client| passed in and return the
+  // original |initiating_client_|.
+  PeerConnectionTestClient* swap_initializing_client(
+      PeerConnectionTestClient* client) {
+    PeerConnectionTestClient* old = initiating_client_.release();
+    initiating_client_.reset(client);
+    return old;
+  }

[pthatcher1, 2015/11/30 20:23:11]

These two methods aren't used any more.

[guoweis_webrtc, 2015/12/01 22:05:12]

Done.

+
   PeerConnectionTestClient* receiving_client() {
     return receiving_client_.get();
   }
 
+  // Reset the |receiving_client_| to the |client| passed in and return the
+  // original |receiving_client_|.
+  PeerConnectionTestClient* swap_receiving_client(
+      PeerConnectionTestClient* client) {
+    PeerConnectionTestClient* old = receiving_client_.release();
+    receiving_client_.reset(client);
+    return old;
+  }
+
+ protected:
+  rtc::scoped_ptr<PeerConnectionTestClient> initiating_client_;
+  rtc::scoped_ptr<PeerConnectionTestClient> receiving_client_;

[pthatcher1, 2015/11/30 20:23:10]

Usually, we leave "foo_" members private and then have a protected "Foo& foo()".

[guoweis_webrtc, 2015/12/01 22:05:12]

but we'll be doing this

foo() = new client():

Unless we have the swaping functions again? Since I want to keep the old client?

[pthatcher1, 2015/12/01 23:29:34]

Then add set_foo and use it: set_foo(new_client()).

+
  private:
   rtc::scoped_ptr<rtc::PhysicalSocketServer> pss_;
   rtc::scoped_ptr<rtc::VirtualSocketServer> ss_;
   rtc::SocketServerScope ss_scope_;
-  rtc::scoped_ptr<PeerConnectionTestClient> initiating_client_;
-  rtc::scoped_ptr<PeerConnectionTestClient> receiving_client_;
 };
 
 // Disable for TSan v2, see
 // https://code.google.com/p/webrtc/issues/detail?id=1205 for details.
 #if !defined(THREAD_SANITIZER)
 
 // This test sets up a Jsep call between two parties and test Dtmf.
 // TODO(holmer): Disabled due to sometimes crashing on buildbots.
 // See issue webrtc/2378.
 TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, DISABLED_LocalP2PTestDtmf) {
@@ skipping 35 matching lines @@
   constraint.SetMandatoryMinWidth(1280);
   constraint.SetMandatoryMinHeight(720);
   SetVideoConstraints(constraint, constraint);
   LocalP2PTest();
   VerifyRenderedSize(1280, 720);
 }
 
 // This test sets up a call between two endpoints that are configured to use
 // DTLS key agreement. As a result, DTLS is negotiated and used for transport.
 TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDtls) {
-  MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
-  FakeConstraints setup_constraints;
-  setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp,
-                                 true);
-  ASSERT_TRUE(CreateTestClients(&setup_constraints, &setup_constraints));
-  LocalP2PTest();
-  VerifyRenderedSize(640, 480);
+  SetupAndVerifyDtlsCall();
 }
 
 // This test sets up a audio call initially and then upgrades to audio/video,
 // using DTLS.
 TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDtlsRenegotiate) {
   MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
   FakeConstraints setup_constraints;
   setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp,
                                  true);
   ASSERT_TRUE(CreateTestClients(&setup_constraints, &setup_constraints));
   receiving_client()->SetReceiveAudioVideo(true, false);
   LocalP2PTest();
   receiving_client()->SetReceiveAudioVideo(true, true);
   receiving_client()->Negotiate();
 }
 
+// This test sets up a call transfer to a new caller with a different DTLS
+// fingerprint.
+TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDtlsTransferCallee) {
+  MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
+  SetupAndVerifyDtlsCall();
+
+  // Keeping the original peer around which will still send packets to the
+  // receiving client. These SRTP packets will be dropped.
+  rtc::scoped_ptr<PeerConnectionTestClient> original_peer(
+      initiating_client_.release());
+  initiating_client_.reset(CreateDtlsClientWithAlternateKey());
+
+  SetSignalingReceivers();
+  receiving_client()->SetExpectIceRestart(true);
+  LocalP2PTest();
+  VerifyRenderedSize(640, 480);
+}
+
+// This test sets up a call transfer to a new callee with a different DTLS
+// fingerprint.
+TEST_F(MAYBE_JsepPeerConnectionP2PTestClient, LocalP2PTestDtlsTransferCaller) {
+  MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
+  SetupAndVerifyDtlsCall();
+
+  // Keeping the original peer around which will still send packets to the
+  // receiving client. These SRTP packets will be dropped.
+  rtc::scoped_ptr<PeerConnectionTestClient> original_peer(
+      receiving_client_.release());
+  receiving_client_.reset(CreateDtlsClientWithAlternateKey());
+
+  SetSignalingReceivers();
+  initializing_client()->IceRestart();

[pthatcher1, 2015/11/30 20:23:10]

It looks like we are testing the scenario where ICE + DTLS are both restarted. 
Should we also test the case where DTLS is restarted but not ICE?

[guoweis_webrtc, 2015/12/01 22:05:12]

Can we do that? Since we don't do DTLS rekeying, I'm not sure how we could only
restart DTLS.

[pthatcher1, 2015/12/01 23:29:34]

Yeah, good point.  In ORTC land, we disallow there being more than one
DtlsTransport per IceTransport and also restarting DTLS.  So, really, we should
have implemented this as creating a new DtlsTransportChannel, not as restarting
a DtlsTransportChannel.  At least, that's how we'll have to refactor it some
day. 

+  LocalP2PTest();
+  VerifyRenderedSize(640, 480);
+}
+
 // This test sets up a call between two endpoints that are configured to use
 // DTLS key agreement. The offerer don't support SDES. As a result, DTLS is
 // negotiated and used for transport.
 TEST_F(MAYBE_JsepPeerConnectionP2PTestClient,
        MAYBE_LocalP2PTestOfferDtlsButNotSdes) {
   MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp);
   FakeConstraints setup_constraints;
   setup_constraints.AddMandatory(MediaConstraintsInterface::kEnableDtlsSrtp,
                                  true);
   ASSERT_TRUE(CreateTestClients(&setup_constraints, &setup_constraints));
@@ skipping 673 matching lines @@
   server.urls.push_back("stun:hostname");
   server.urls.push_back("turn:hostname");
   servers.push_back(server);
   EXPECT_TRUE(webrtc::ParseIceServers(servers, &stun_configurations_,
                                       &turn_configurations_));
   EXPECT_EQ(1U, stun_configurations_.size());
   EXPECT_EQ(1U, turn_configurations_.size());
 }
 
 #endif // if !defined(THREAD_SANITIZER)