Add signatures for session-description messages.

Now the session-description messages are signed with the key generated
by the authenticators and the signature is verified by the receiver.

Verification can be disabled using --disable-authentication flag.

BUG=547158

Review URL: https://codereview.chromium.org/1815043002

Cr-Commit-Position: refs/heads/master@{#382120}

6 files changed, 91 insertions, 38 deletions

diff --git a/remoting/protocol/fake_authenticator.cc b/remoting/protocol/fake_authenticator.cc
index 49813ab..93f443c 100644
--- a/remoting/protocol/fake_authenticator.cc
+++ b/remoting/protocol/fake_authenticator.cc
@@ -24,13 +24,9 @@ namespace protocol {
 FakeChannelAuthenticator::FakeChannelAuthenticator(bool accept, bool async)
     : result_(accept ? net::OK : net::ERR_FAILED),
       async_(async),
-      did_read_bytes_(false),
-      did_write_bytes_(false),
-      weak_factory_(this) {
-}
+      weak_factory_(this) {}
 
-FakeChannelAuthenticator::~FakeChannelAuthenticator() {
-}
+FakeChannelAuthenticator::~FakeChannelAuthenticator() {}
 
 void FakeChannelAuthenticator::SecureAndAuthenticate(
     scoped_ptr<P2PStreamSocket> socket,
@@ -96,16 +92,9 @@ FakeAuthenticator::FakeAuthenticator(Type type,
                                      int round_trips,
                                      Action action,
                                      bool async)
-    : type_(type),
-      round_trips_(round_trips),
-      action_(action),
-      async_(async),
-      messages_(0),
-      messages_till_started_(0) {
-}
+    : type_(type), round_trips_(round_trips), action_(action), async_(async) {}
 
-FakeAuthenticator::~FakeAuthenticator() {
-}
+FakeAuthenticator::~FakeAuthenticator() {}
 
 void FakeAuthenticator::set_messages_till_started(int messages) {
   messages_till_started_ = messages;
@@ -192,6 +181,7 @@ scoped_ptr<buzz::XmlElement> FakeAuthenticator::GetNextMessage() {
 
 const std::string& FakeAuthenticator::GetAuthKey() const {
   EXPECT_EQ(ACCEPTED, state());
+  DCHECK(!auth_key_.empty());
   return auth_key_;
 }
 
@@ -203,13 +193,14 @@ FakeAuthenticator::CreateChannelAuthenticator() const {
 }
 
 FakeHostAuthenticatorFactory::FakeHostAuthenticatorFactory(
-    int round_trips, int messages_till_started,
-    FakeAuthenticator::Action action, bool async)
+    int round_trips,
+    int messages_till_started,
+    FakeAuthenticator::Action action,
+    bool async)
     : round_trips_(round_trips),
       messages_till_started_(messages_till_started),
-      action_(action), async_(async) {
-}
-
+      action_(action),
+      async_(async) {}
 FakeHostAuthenticatorFactory::~FakeHostAuthenticatorFactory() {}
 
 scoped_ptr<Authenticator> FakeHostAuthenticatorFactory::CreateAuthenticator(
diff --git a/remoting/protocol/fake_authenticator.h b/remoting/protocol/fake_authenticator.h
index b6c40ce..4928d91 100644
--- a/remoting/protocol/fake_authenticator.h
+++ b/remoting/protocol/fake_authenticator.h
@@ -29,14 +29,14 @@ class FakeChannelAuthenticator : public ChannelAuthenticator {
 
   void CallDoneCallback();
 
-  int result_;
-  bool async_;
+  const int result_;
+  const bool async_;
 
   scoped_ptr<P2PStreamSocket> socket_;
   DoneCallback done_callback_;
 
-  bool did_read_bytes_;
-  bool did_write_bytes_;
+  bool did_read_bytes_ = false;
+  bool did_write_bytes_ = false;
 
   base::WeakPtrFactory<FakeChannelAuthenticator> weak_factory_;
 
@@ -64,6 +64,10 @@ class FakeAuthenticator : public Authenticator {
   // started() returns true.  Default to 0.
   void set_messages_till_started(int messages);
 
+  // Sets auth key to be returned by GetAuthKey(). Must be called when
+  // |round_trips| is set to 0.
+  void set_auth_key(const std::string& auth_key) { auth_key_ = auth_key; }
+
   // Authenticator interface.
   State state() const override;
   bool started() const override;
@@ -75,16 +79,16 @@ class FakeAuthenticator : public Authenticator {
   scoped_ptr<ChannelAuthenticator> CreateChannelAuthenticator() const override;
 
  protected:
-  Type type_;
-  int round_trips_;
-  Action action_;
-  bool async_;
+  const Type type_;
+  const int round_trips_;
+  const Action action_;
+  const bool async_;
 
   // Total number of messages that have been processed.
-  int messages_;
+  int messages_ = 0;
   // Number of messages that the authenticator needs to process before started()
   // returns true.  Default to 0.
-  int messages_till_started_;
+  int messages_till_started_ = 0;
 
   std::string auth_key_;
 
@@ -104,10 +108,10 @@ class FakeHostAuthenticatorFactory : public AuthenticatorFactory {
       const std::string& remote_jid) override;
 
  private:
-  int round_trips_;
-  int messages_till_started_;
-  FakeAuthenticator::Action action_;
-  bool async_;
+  const int round_trips_;
+  const int messages_till_started_;
+  const FakeAuthenticator::Action action_;
+  const bool async_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeHostAuthenticatorFactory);
 };
diff --git a/remoting/protocol/fake_session.cc b/remoting/protocol/fake_session.cc
index 616d12d..f1e974e 100644
--- a/remoting/protocol/fake_session.cc
+++ b/remoting/protocol/fake_session.cc
@@ -13,6 +13,7 @@ namespace remoting {
 namespace protocol {
 
 const char kTestJid[] = "host1@gmail.com/chromoting123";
+const char kTestAuthKey[] = "test_auth_key";
 
 FakeSession::FakeSession()
     : config_(SessionConfig::ForTest()), jid_(kTestJid), weak_factory_(this) {}
@@ -32,6 +33,7 @@ void FakeSession::SimulateConnection(FakeSession* peer) {
   // Initialize transport and authenticator on the client.
   authenticator_.reset(new FakeAuthenticator(FakeAuthenticator::CLIENT, 0,
                                              FakeAuthenticator::ACCEPT, false));
+  authenticator_->set_auth_key(kTestAuthKey);
   transport_->Start(authenticator_.get(),
                     base::Bind(&FakeSession::SendTransportInfo,
                                weak_factory_.GetWeakPtr()));
@@ -39,6 +41,7 @@ void FakeSession::SimulateConnection(FakeSession* peer) {
   // Initialize transport and authenticator on the host.
   peer->authenticator_.reset(new FakeAuthenticator(
       FakeAuthenticator::HOST, 0, FakeAuthenticator::ACCEPT, false));
+  peer->authenticator_->set_auth_key(kTestAuthKey);
   peer->transport_->Start(peer->authenticator_.get(),
                           base::Bind(&FakeSession::SendTransportInfo, peer_));
 
diff --git a/remoting/protocol/webrtc_transport.cc b/remoting/protocol/webrtc_transport.cc
index c1b7035..9a6d243 100644
--- a/remoting/protocol/webrtc_transport.cc
+++ b/remoting/protocol/webrtc_transport.cc
@@ -6,13 +6,16 @@
 
 #include <utility>
 
+#include "base/base64.h"
 #include "base/callback_helpers.h"
+#include "base/command_line.h"
 #include "base/macros.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/task_runner_util.h"
 #include "base/thread_task_runner_handle.h"
 #include "jingle/glue/thread_wrapper.h"
+#include "remoting/protocol/authenticator.h"
 #include "remoting/protocol/port_allocator_factory.h"
 #include "remoting/protocol/stream_message_pipe_adapter.h"
 #include "remoting/protocol/transport_context.h"
@@ -36,6 +39,12 @@ const int kTransportInfoSendDelayMs = 20;
 // XML namespace for the transport elements.
 const char kTransportNamespace[] = "google:remoting:webrtc";
 
+#if !defined(NDEBUG)
+// Command line switch used to disable signature verification.
+// TODO(sergeyu): Remove this flag.
+const char kDisableAuthenticationSwitchName[] = "disable-authentication";
+#endif
+
 // A webrtc::CreateSessionDescriptionObserver implementation used to receive the
 // results of creating descriptions for this end of the PeerConnection.
 class CreateSessionDescriptionObserver
@@ -113,6 +122,7 @@ WebrtcTransport::WebrtcTransport(
     : worker_thread_(worker_thread),
       transport_context_(transport_context),
       event_handler_(event_handler),
+      handshake_hmac_(crypto::HMAC::SHA256),
       outgoing_data_stream_adapter_(
           true,
           base::Bind(&WebrtcTransport::Close, base::Unretained(this))),
@@ -138,7 +148,9 @@ void WebrtcTransport::Start(
 
   send_transport_info_callback_ = std::move(send_transport_info_callback);
 
-  // TODO(sergeyu): Use the |authenticator| to authenticate PeerConnection.
+  if (!handshake_hmac_.Init(authenticator->GetAuthKey())) {
+    LOG(FATAL) << "HMAC::Init() failed.";
+  }
 
   fake_audio_device_module_.reset(new webrtc::FakeAudioDeviceModule());
 
@@ -189,7 +201,7 @@ bool WebrtcTransport::ProcessTransportInfo(XmlElement* transport_info) {
             ? webrtc::PeerConnectionInterface::kStable
             : webrtc::PeerConnectionInterface::kHaveLocalOffer;
     if (peer_connection_->signaling_state() != expected_state) {
-      LOG(ERROR) << "Received unexpected WebRTC session_description. ";
+      LOG(ERROR) << "Received unexpected WebRTC session_description.";
       return false;
     }
 
@@ -200,6 +212,23 @@ bool WebrtcTransport::ProcessTransportInfo(XmlElement* transport_info) {
       return false;
     }
 
+    std::string signature_base64 =
+        session_description->Attr(QName(std::string(), "signature"));
+    std::string signature;
+    if (!base::Base64Decode(signature_base64, &signature) ||
+        !handshake_hmac_.Verify(sdp, signature)) {
+      LOG(WARNING) << "Received session-description with invalid signature.";
+      bool ignore_error = false;
+#if !defined(NDEBUG)
+      ignore_error = base::CommandLine::ForCurrentProcess()->HasSwitch(
+          kDisableAuthenticationSwitchName);
+#endif
+      if (!ignore_error) {
+        Close(AUTHENTICATION_FAILED);
+        return true;
+      }
+    }
+
     webrtc::SdpParseError error;
     scoped_ptr<webrtc::SessionDescriptionInterface> session_description(
         webrtc::CreateSessionDescription(type, sdp, &error));
@@ -288,6 +317,15 @@ void WebrtcTransport::OnLocalSessionDescriptionCreated(
   offer_tag->SetAttr(QName(std::string(), "type"), description->type());
   offer_tag->SetBodyText(description_sdp);
 
+  std::string digest;
+  digest.resize(handshake_hmac_.DigestLength());
+  CHECK(handshake_hmac_.Sign(description_sdp,
+                              reinterpret_cast<uint8_t*>(&(digest[0])),
+                              digest.size()));
+  std::string digest_base64;
+  base::Base64Encode(digest, &digest_base64);
+  offer_tag->SetAttr(QName(std::string(), "signature"), digest_base64);
+
   send_transport_info_callback_.Run(std::move(transport_info));
 
   peer_connection_->SetLocalDescription(
diff --git a/remoting/protocol/webrtc_transport.h b/remoting/protocol/webrtc_transport.h
index 0001377..f3d2cef 100644
--- a/remoting/protocol/webrtc_transport.h
+++ b/remoting/protocol/webrtc_transport.h
@@ -12,6 +12,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "base/timer/timer.h"
+#include "crypto/hmac.h"
 #include "remoting/protocol/transport.h"
 #include "remoting/protocol/webrtc_data_stream_adapter.h"
 #include "remoting/signaling/signal_strategy.h"
@@ -116,6 +117,8 @@ class WebrtcTransport : public Transport,
   EventHandler* event_handler_ = nullptr;
   SendTransportInfoCallback send_transport_info_callback_;
 
+  crypto::HMAC handshake_hmac_;
+
   scoped_ptr<webrtc::FakeAudioDeviceModule> fake_audio_device_module_;
 
   rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface>
diff --git a/remoting/protocol/webrtc_transport_unittest.cc b/remoting/protocol/webrtc_transport_unittest.cc
index 40f4613..431cc0f 100644
--- a/remoting/protocol/webrtc_transport_unittest.cc
+++ b/remoting/protocol/webrtc_transport_unittest.cc
@@ -28,6 +28,7 @@ namespace protocol {
 namespace {
 
 const char kChannelName[] = "test_channel";
+const char kAuthKey[] = "test_auth_key";
 
 class TestTransportEventHandler : public WebrtcTransport::EventHandler {
  public:
@@ -95,8 +96,8 @@ class WebrtcTransportTest : public testing::Test {
   void ProcessTransportInfo(scoped_ptr<WebrtcTransport>* target_transport,
                             scoped_ptr<buzz::XmlElement> transport_info) {
     ASSERT_TRUE(target_transport);
-    EXPECT_TRUE((*target_transport)
-                    ->ProcessTransportInfo(transport_info.get()));
+    EXPECT_TRUE(
+        (*target_transport)->ProcessTransportInfo(transport_info.get()));
   }
 
   void InitializeConnection() {
@@ -106,6 +107,7 @@ class WebrtcTransportTest : public testing::Test {
                             &host_event_handler_));
     host_authenticator_.reset(new FakeAuthenticator(
         FakeAuthenticator::HOST, 0, FakeAuthenticator::ACCEPT, false));
+    host_authenticator_->set_auth_key(kAuthKey);
 
     client_transport_.reset(
         new WebrtcTransport(jingle_glue::JingleThreadWrapper::current(),
@@ -113,6 +115,7 @@ class WebrtcTransportTest : public testing::Test {
                             &client_event_handler_));
     client_authenticator_.reset(new FakeAuthenticator(
         FakeAuthenticator::CLIENT, 0, FakeAuthenticator::ACCEPT, false));
+    client_authenticator_->set_auth_key(kAuthKey);
   }
 
   void StartConnection() {
@@ -233,6 +236,17 @@ TEST_F(WebrtcTransportTest, Connects) {
   WaitUntilConnected();
 }
 
+TEST_F(WebrtcTransportTest, InvalidAuthKey) {
+  InitializeConnection();
+  client_authenticator_->set_auth_key("Incorrect Key");
+  StartConnection();
+
+  run_loop_.reset(new base::RunLoop());
+  run_loop_->Run();
+
+  EXPECT_EQ(AUTHENTICATION_FAILED, client_error_);
+}
+
 TEST_F(WebrtcTransportTest, DataStream) {
   client_event_handler_.set_connecting_callback(base::Bind(
       &WebrtcTransportTest::CreateClientDataStream, base::Unretained(this)));