From 96a3cc1e1f5c0c4e73ad1036a7e0add19d9a197e Mon Sep 17 00:00:00 2001
From: Alex Auvolat <alex@adnab.me>
Date: Mon, 21 Feb 2022 13:45:41 +0100
Subject: [PATCH] Implement version check & transmit more error info

---
 src/client.rs |  3 ++-
 src/error.rs  | 10 +++++++---
 src/proto.rs  | 22 +++++++++++++++++++++-
 src/server.rs |  4 +++-
 4 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/src/client.rs b/src/client.rs
index 62f876b..e84c85e 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -206,7 +206,8 @@ impl ClientConn {
 				<T as Message>::Response,
 			>(&resp[1..])?)
 		} else {
-			Err(Error::Remote(format!("Remote error code {}", code)))
+			let msg = String::from_utf8(resp[1..].to_vec()).unwrap_or_default();
+			Err(Error::Remote(code, msg))
 		}
 	}
 }
diff --git a/src/error.rs b/src/error.rs
index 0ed30a5..99acdd1 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -34,8 +34,11 @@ pub enum Error {
 	#[error(display = "Connection closed")]
 	ConnectionClosed,
 
-	#[error(display = "Remote error: {}", _0)]
-	Remote(String),
+	#[error(display = "Version mismatch: {}", _0)]
+	VersionMismatch(String),
+
+	#[error(display = "Remote error {}: {}", _0, _1)]
+	Remote(u8, String),
 }
 
 impl Error {
@@ -50,7 +53,8 @@ impl Error {
 			Self::NoHandler => 20,
 			Self::ConnectionClosed => 21,
 			Self::Handshake(_) => 30,
-			Self::Remote(_) => 40,
+			Self::VersionMismatch(_) => 31,
+			Self::Remote(c, _) => *c,
 			Self::Message(_) => 99,
 		}
 	}
diff --git a/src/proto.rs b/src/proto.rs
index 2db3f83..146211b 100644
--- a/src/proto.rs
+++ b/src/proto.rs
@@ -1,7 +1,7 @@
 use std::collections::{HashMap, VecDeque};
 use std::sync::Arc;
 
-use log::trace;
+use log::{error, trace};
 
 use futures::{AsyncReadExt, AsyncWriteExt};
 use kuska_handshake::async_std::BoxStreamWrite;
@@ -12,6 +12,10 @@ use async_trait::async_trait;
 
 use crate::error::*;
 
+/// Tag which is exchanged between client and server upon connection establishment
+/// to check that they are running compatible versions of Netapp
+pub const VERSION_TAG: [u8; 8] = [b'n', b'e', b't', b'a', b'p', b'p', 0x00, 0x04];
+
 /// Priority of a request (click to read more about priorities).
 ///
 /// This priority value is used to priorize messages
@@ -114,6 +118,10 @@ pub(crate) trait SendLoop: Sync {
 	where
 		W: AsyncWriteExt + Unpin + Send + Sync,
 	{
+		// Before anything, send version tag, which is checked in recv_loop
+		write.write_all(&VERSION_TAG[..]).await?;
+		write.flush().await?;
+
 		let mut sending = SendQueue::new();
 		let mut should_exit = false;
 		while !should_exit || !sending.is_empty() {
@@ -169,6 +177,7 @@ pub(crate) trait SendLoop: Sync {
 				}
 			}
 		}
+
 		let _ = write.goodbye().await;
 		Ok(())
 	}
@@ -189,6 +198,17 @@ pub(crate) trait RecvLoop: Sync + 'static {
 	where
 		R: AsyncReadExt + Unpin + Send + Sync,
 	{
+		let mut their_version_tag = [0u8; 8];
+		read.read_exact(&mut their_version_tag[..]).await?;
+		if their_version_tag != VERSION_TAG {
+			let msg = format!(
+				"Different netapp versions: {:?} (theirs) vs. {:?} (ours)",
+				their_version_tag, VERSION_TAG
+			);
+			error!("{}", msg);
+			return Err(Error::VersionMismatch(msg));
+		}
+
 		let mut receiving = HashMap::new();
 		loop {
 			trace!("recv_loop: reading packet");
diff --git a/src/server.rs b/src/server.rs
index eb70057..31f6ad6 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -184,7 +184,9 @@ impl RecvLoop for ServerConn {
 					resp_bytes
 				}
 				Err(e) => {
-					vec![e.code()]
+					let mut resp_bytes = vec![e.code()];
+					resp_bytes.extend(e.to_string().into_bytes());
+					resp_bytes
 				}
 			};