Updating SecurityKeyAuthHandlerPosix socket lifetime management

remoting/host/security_key/security_key_auth_handler_posix.cc

Index: remoting/host/security_key/security_key_auth_handler_posix.cc
diff --git a/remoting/host/security_key/security_key_auth_handler_posix.cc b/remoting/host/security_key/security_key_auth_handler_posix.cc
index 99f72b5db1dbb57cab7a64aac33f5fee72a938e6..2df90b2f3280cbdf972428326e2aae2bf2a68c0b 100644
--- a/remoting/host/security_key/security_key_auth_handler_posix.cc
+++ b/remoting/host/security_key/security_key_auth_handler_posix.cc
@@ -43,8 +43,9 @@ base::LazyInstance<base::FilePath>::Leaky g_security_key_socket_name =
 // the current uid.
 bool MatchUid(const net::UnixDomainServerSocket::Credentials& credentials) {
   bool allowed = credentials.user_id == getuid();
-  if (!allowed)
+  if (!allowed) {
     HOST_LOG << "Refused socket connection from uid " << credentials.user_id;
+  }
   return allowed;
 }
 
@@ -201,7 +202,11 @@ void SecurityKeyAuthHandlerPosix::SendClientResponse(
     const std::string& response) {
   ActiveSockets::const_iterator iter = GetSocketForConnectionId(connection_id);
   if (iter != active_sockets_.end()) {
+    HOST_LOG << "Sending client response to socket: " << connection_id;

[Sergey Ulanov, 2016/12/19 23:19:18]

Do we really need to log for every Gnubby connection?

[joedow, 2016/12/20 00:28:00]

There shouldn't be too much debug spew but I'll update it to a DLOG.

     iter->second->SendResponse(response);
+    iter->second->StartReadingRequest(
+        base::Bind(&SecurityKeyAuthHandlerPosix::OnReadComplete,
+                   base::Unretained(this), connection_id));
   } else {
     LOG(WARNING) << "Unknown gnubby-auth connection id: " << connection_id;
   }
@@ -210,7 +215,7 @@ void SecurityKeyAuthHandlerPosix::SendClientResponse(
 void SecurityKeyAuthHandlerPosix::SendErrorAndCloseConnection(int id) {
   ActiveSockets::const_iterator iter = GetSocketForConnectionId(id);
   if (iter != active_sockets_.end()) {
-    HOST_LOG << "Sending security key error";
+    HOST_LOG << "Sending error and closing socket: " << id;
     SendErrorAndCloseActiveSocket(iter);
   } else {
     LOG(WARNING) << "Unknown gnubby-auth connection id: " << id;
@@ -246,11 +251,12 @@ void SecurityKeyAuthHandlerPosix::OnAccepted(int result) {
   DCHECK_NE(net::ERR_IO_PENDING, result);
 
   if (result < 0) {
-    LOG(ERROR) << "Error in accepting a new connection";
+    LOG(ERROR) << "Error accepting new socket connection: " << result;
     return;
   }
 
   int security_key_connection_id = ++last_connection_id_;
+  HOST_LOG << "Creating new socket: " << security_key_connection_id;
   SecurityKeySocket* socket = new SecurityKeySocket(
       std::move(accept_socket_), request_timeout_,
       base::Bind(&SecurityKeyAuthHandlerPosix::RequestTimedOut,
@@ -271,16 +277,17 @@ void SecurityKeyAuthHandlerPosix::OnReadComplete(int connection_id) {
   DCHECK(iter != active_sockets_.end());
   std::string request_data;
   if (!iter->second->GetAndClearRequestData(&request_data)) {
-    SendErrorAndCloseActiveSocket(iter);
+    HOST_LOG << "Closing socket: " << connection_id;

[Sergey Ulanov, 2016/12/19 23:19:18]

Log only in case of an error?

[joedow, 2016/12/20 00:28:00]

Updating to DLOG

+    if (iter->second->socket_read_error()) {
+      iter->second->SendSshError();
+    }
+    active_sockets_.erase(iter);
     return;
   }
 
-  HOST_LOG << "Received security key request: " << GetCommandCode(request_data);
+  HOST_LOG << "Received request from socket: " << connection_id
+           << ", code: " << GetCommandCode(request_data);
   send_message_callback_.Run(connection_id, request_data);
-
-  iter->second->StartReadingRequest(
-      base::Bind(&SecurityKeyAuthHandlerPosix::OnReadComplete,
-                 base::Unretained(this), connection_id));
 }
 
 SecurityKeyAuthHandlerPosix::ActiveSockets::const_iterator
@@ -295,7 +302,7 @@ void SecurityKeyAuthHandlerPosix::SendErrorAndCloseActiveSocket(
 }
 
 void SecurityKeyAuthHandlerPosix::RequestTimedOut(int connection_id) {
-  HOST_LOG << "SecurityKey request timed out: " << connection_id;
+  HOST_LOG << "SecurityKey request timed out for socket: " << connection_id;
   ActiveSockets::const_iterator iter = active_sockets_.find(connection_id);
   if (iter != active_sockets_.end()) {
     SendErrorAndCloseActiveSocket(iter);