Crash the network or desktop process when an unknown IPC message is received.

remoting/host/daemon_process.cc

Index: remoting/host/daemon_process.cc
diff --git a/remoting/host/daemon_process.cc b/remoting/host/daemon_process.cc
index f76838dda20beed857b86d025086b66ef5ee9a46..9ea4c6affe6ac9b3c913ad39ec9b30af511ff0c9 100644
--- a/remoting/host/daemon_process.cc
+++ b/remoting/host/daemon_process.cc
@@ -65,6 +65,12 @@ bool DaemonProcess::OnMessageReceived(const IPC::Message& message) {
                         CloseDesktopSession)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
+
+  if (!handled) {
+    LOG(ERROR) << "An unexpected IPC message received: type=" << message.type();
+    CrashNetworkProcess(FROM_HERE);
+  }
+
   return handled;

[Wez, 2013/03/04 23:17:09]

Do you know what IpcChannel itself does with the |handled| result?  e.g. does it
call OnChannelError(), and/or tear the channel down?

[alexeypa (please no reviews), 2013/03/05 00:01:44]

It ignores the returned value.

 }
 
@@ -82,7 +88,6 @@ void DaemonProcess::CloseDesktopSession(int terminal_id) {
   if (!IsTerminalIdKnown(terminal_id)) {
     LOG(ERROR) << "An invalid terminal ID. terminal_id=" << terminal_id;
     CrashNetworkProcess(FROM_HERE);
-    DeleteAllDesktopSessions();
     return;
   }
 
@@ -131,7 +136,6 @@ void DaemonProcess::CreateDesktopSession(int terminal_id) {
   if (IsTerminalIdKnown(terminal_id)) {
     LOG(ERROR) << "An invalid terminal ID. terminal_id=" << terminal_id;
     CrashNetworkProcess(FROM_HERE);
-    DeleteAllDesktopSessions();
     return;
   }
 
@@ -145,6 +149,8 @@ void DaemonProcess::CrashNetworkProcess(
     const tracked_objects::Location& location) {
   SendToNetwork(new ChromotingDaemonNetworkMsg_Crash(
       location.function_name(), location.file_name(), location.line_number()));

[Wez, 2013/03/04 23:17:09]

nit: You're not closing the IPC channel here, so DeleteAllDesktopSessions may
result in IPCs being dispatched to the network process, which you no longer
trust, which doesn't seem right.

[alexeypa (please no reviews), 2013/03/05 00:01:44]

Agree. The current implementation does not enforce anything. It is more of a
diagnostic tool.

+
+  DeleteAllDesktopSessions();

[Wez, 2013/02/28 23:53:14]

nit: We should really tear-down the IPC channel to the process and, after some
timeout, forcibly terminate the process.

[alexeypa (please no reviews), 2013/03/05 00:01:44]

Agree, but let's do it as a separate CL. I imagine it should be a part of the
worker process launcher.

[Wez, 2013/03/05 02:12:31]

SGTM.

 }
 
 void DaemonProcess::Initialize() {