[Tarantool-patches] [PATCH 6/8] raft: fix ignorance of bad state receipt
Serge Petrenko
sergepetrenko at tarantool.org
Wed Dec 16 16:06:59 MSK 2020
13.12.2020 20:15, Vladislav Shpilevoy пишет:
> 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
> ---
LGTM.
> 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 ***
--
Serge Petrenko
More information about the Tarantool-patches
mailing list