From cc1783c1cd7f6fa1e31bfb7465306fafd8a79077 Mon Sep 17 00:00:00 2001
From: Uwe Seimet <48174652+uweseimet@users.noreply.github.com>
Date: Tue, 26 Oct 2021 01:04:10 +0200
Subject: [PATCH] Improved remote connection error handling (#376)

* Improved remote connection error handling

* Improved error handling by adding a magic string to each message

* Interface comment update

* Interface comment update

* Improve error messages

* Clients send the magic word to authenticate

Co-authored-by: Daniel Markstedt <markstedt@gmail.com>
---
 src/oled_monitor/rascsi_oled_monitor.py | 13 ++++++++-----
 src/raspberrypi/protobuf_util.cpp       | 13 ++++++++-----
 src/raspberrypi/rascsi.cpp              | 10 ++++++++++
 src/raspberrypi/rascsi_interface.proto  |  4 ++--
 src/raspberrypi/rasctl_commands.cpp     |  6 +++++-
 src/web/ractl_cmds.py                   |  2 ++
 6 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/src/oled_monitor/rascsi_oled_monitor.py b/src/oled_monitor/rascsi_oled_monitor.py
index e87317d4..e974ba5e 100755
--- a/src/oled_monitor/rascsi_oled_monitor.py
+++ b/src/oled_monitor/rascsi_oled_monitor.py
@@ -235,6 +235,8 @@ def send_over_socket(s, payload):
     Reads data from socket in 2048 bytes chunks until all data is received.
     """
 
+    # Sending the magic word "RASCSI" to authenticate with the server
+    s.send(b"RASCSI")
     # Prepending a little endian 32bit header with the message size
     s.send(pack("<i", len(payload)))
     s.send(payload)
@@ -250,16 +252,17 @@ def send_over_socket(s, payload):
         while bytes_recvd < response_length:
             chunk = s.recv(min(response_length - bytes_recvd, 2048))
             if chunk == b'':
-                exit("Read an empty chunk from the socket. \
-                        Socket connection has dropped unexpectedly. \
-                        RaSCSI may have has crashed.")
+                exit("Socket connection has dropped unexpectedly. "
+                     "RaSCSI may have crashed."
+                    )
             chunks.append(chunk)
             bytes_recvd = bytes_recvd + len(chunk)
         response_message = b''.join(chunks)
         return response_message
     else:
-        exit("The response from RaSCSI did not contain a protobuf header. \
-                RaSCSI may have crashed.")
+        exit("The response from RaSCSI did not contain a protobuf header. "
+             "RaSCSI may have crashed."
+            )
 
 
 def formatted_output():
diff --git a/src/raspberrypi/protobuf_util.cpp b/src/raspberrypi/protobuf_util.cpp
index 476b0038..6c5554c6 100644
--- a/src/raspberrypi/protobuf_util.cpp
+++ b/src/raspberrypi/protobuf_util.cpp
@@ -70,12 +70,12 @@ void protobuf_util::SerializeMessage(int fd, const google::protobuf::Message& me
 	// Write the size of the protobuf data as a header
     int32_t size = data.length();
     if (write(fd, &size, sizeof(size)) != sizeof(size)) {
-    	throw io_exception("Can't write protobuf header");
+    	throw io_exception("Can't write protobuf message header");
     }
 
     // Write the actual protobuf data
     if (write(fd, data.data(), size) != size) {
-    	throw io_exception("Can't write protobuf data");
+    	throw io_exception("Can't write protobuf message data");
     }
 }
 
@@ -83,17 +83,20 @@ void protobuf_util::DeserializeMessage(int fd, google::protobuf::Message& messag
 {
 	// Read the header with the size of the protobuf data
 	uint8_t header_buf[4];
-	int bytes_read = ReadNBytes(fd, header_buf, 4);
-	if (bytes_read < 4) {
+	int bytes_read = ReadNBytes(fd, header_buf, sizeof(header_buf));
+	if (bytes_read < (int)sizeof(header_buf)) {
 		return;
 	}
 	int32_t size = (header_buf[3] << 24) + (header_buf[2] << 16) + (header_buf[1] << 8) + header_buf[0];
+	if (size <= 0) {
+		throw io_exception("Broken protobuf message header");
+	}
 
 	// Read the binary protobuf data
 	uint8_t data_buf[size];
 	bytes_read = ReadNBytes(fd, data_buf, size);
 	if (bytes_read < size) {
-		throw io_exception("Missing protobuf data");
+		throw io_exception("Missing protobuf message data");
 	}
 
 	// Create protobuf message
diff --git a/src/raspberrypi/rascsi.cpp b/src/raspberrypi/rascsi.cpp
index 29a10757..b3d0868a 100644
--- a/src/raspberrypi/rascsi.cpp
+++ b/src/raspberrypi/rascsi.cpp
@@ -1291,6 +1291,16 @@ static void *MonThread(void *param)
 				throw io_exception("accept() failed");
 			}
 
+			// Read magic string
+			char magic[6];
+			int bytes_read = ReadNBytes(fd, (uint8_t *)magic, sizeof(magic));
+			if (!bytes_read) {
+				continue;
+			}
+			if (bytes_read != sizeof(magic) || strncmp(magic, "RASCSI", sizeof(magic))) {
+				throw io_exception("Invalid magic");
+			}
+
 			// Fetch the command
 			PbCommand command;
 			DeserializeMessage(fd, command);
diff --git a/src/raspberrypi/rascsi_interface.proto b/src/raspberrypi/rascsi_interface.proto
index 54bb7f8c..4fc6f53a 100644
--- a/src/raspberrypi/rascsi_interface.proto
+++ b/src/raspberrypi/rascsi_interface.proto
@@ -1,6 +1,6 @@
 //
-// Each rascsi remote interface message is preceded by a little endian 32 bit header,
-// which contains the protobuf message size.
+// Each rascsi message sent to the rascsi server is preceded by the magic string "RASCSI".
+// A message starts with a little endian 32 bit header which contains the protobuf message size.
 // Unless explicitly specified the order of repeated data returned is undefined.
 //
 
diff --git a/src/raspberrypi/rasctl_commands.cpp b/src/raspberrypi/rasctl_commands.cpp
index 43440e03..0302520b 100644
--- a/src/raspberrypi/rasctl_commands.cpp
+++ b/src/raspberrypi/rasctl_commands.cpp
@@ -60,7 +60,11 @@ void RasctlCommands::SendCommand()
     		throw io_exception(error.str());
     	}
 
-        SerializeMessage(fd, command);
+    	if (write(fd, "RASCSI", 6) != 6) {
+    		throw io_exception("Can't write magic");
+    	}
+
+    	SerializeMessage(fd, command);
     }
     catch(const io_exception& e) {
     	cerr << "Error: " << e.getmsg() << endl;
diff --git a/src/web/ractl_cmds.py b/src/web/ractl_cmds.py
index ac07b8e4..a2f6525c 100644
--- a/src/web/ractl_cmds.py
+++ b/src/web/ractl_cmds.py
@@ -409,6 +409,8 @@ def send_over_socket(s, payload):
     """
     from struct import pack, unpack
 
+    # Sending the magic word "RASCSI" to authenticate with the server
+    s.send(b"RASCSI")
     # Prepending a little endian 32bit header with the message size
     s.send(pack("<i", len(payload)))
     s.send(payload)