[Tarantool-patches] [PATCH 6/8] raft: fix ignorance of bad state receipt

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

raft_process_msg() only validated that the state is specified. But
it didn't check if the state is inside of the allowed value range.

Such messages were considered valid, and even their other fields
were accepted. For instance, an invalid message could bump term.

It is safer to reject such messages.

Part of #5303
---
 src/lib/raft/raft.c   |  4 ++--
 src/lib/raft/raft.h   |  1 +
 test/unit/raft.c      | 10 +++++++++-
 test/unit/raft.result |  4 +++-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c
index df34d81dc..ab007a462 100644
--- a/src/lib/raft/raft.c
+++ b/src/lib/raft/raft.c
@@ -309,8 +309,8 @@ raft_process_msg(struct raft *raft, const struct raft_msg *req, uint32_t source)
 	say_info("RAFT: message %s from %u", raft_msg_to_string(req), source);
 	assert(source > 0);
 	assert(source != raft->self);
-	if (req->term == 0 || req->state == 0) {
-		diag_set(RaftError, "Raft term and state can't be zero");
+	if (req->term == 0 || req->state == 0 || req->state >= raft_state_MAX) {
+		diag_set(RaftError, "Invalid term or state");
 		return -1;
 	}
 	if (req->state == RAFT_STATE_CANDIDATE &&
diff --git a/src/lib/raft/raft.h b/src/lib/raft/raft.h
index 7a3db79c1..e447f6634 100644
--- a/src/lib/raft/raft.h
+++ b/src/lib/raft/raft.h
@@ -85,6 +85,7 @@ enum raft_state {
 	RAFT_STATE_CANDIDATE = 2,
 	/** Election was successful. The node accepts write requests. */
 	RAFT_STATE_LEADER = 3,
+	raft_state_MAX,
 };
 
 /**
diff --git a/test/unit/raft.c b/test/unit/raft.c
index 1ed8b7af7..b97d9d0aa 100644
--- a/test/unit/raft.c
+++ b/test/unit/raft.c
@@ -247,7 +247,7 @@ raft_test_recovery(void)
 static void
 raft_test_bad_msg(void)
 {
-	raft_start_test(7);
+	raft_start_test(9);
 	struct raft_msg msg;
 	struct raft_node node;
 	struct vclock vclock;
@@ -286,6 +286,14 @@ raft_test_bad_msg(void)
 	};
 	is(raft_node_process_msg(&node, &msg, 2), -1, "term can't be 0");
 
+	msg = (struct raft_msg){
+		.state = 10000,
+		.term = 10,
+		.vote = 2,
+	};
+	is(raft_node_process_msg(&node, &msg, 2), -1, "bad state");
+	is(node.raft.term, 1, "term from the bad message wasn't used");
+
 	raft_node_destroy(&node);
 	raft_finish_test();
 }
diff --git a/test/unit/raft.result b/test/unit/raft.result
index 2398bd71c..3fa2682c8 100644
--- a/test/unit/raft.result
+++ b/test/unit/raft.result
@@ -45,7 +45,7 @@ ok 1 - subtests
 ok 2 - subtests
 	*** raft_test_recovery: done ***
 	*** raft_test_bad_msg ***
-    1..7
+    1..9
     ok 1 - state can't be 0
     ok 2 - term from the bad message wasn't used
     ok 3 - node can't be a candidate but vote for another node
@@ -53,6 +53,8 @@ ok 2 - subtests
     ok 5 - node can't be a candidate without vclock
     ok 6 - term from the bad message wasn't used
     ok 7 - term can't be 0
+    ok 8 - bad state
+    ok 9 - term from the bad message wasn't used
 ok 3 - subtests
 	*** raft_test_bad_msg: done ***
 	*** raft_test_vote ***
-- 
2.24.3 (Apple Git-128)