[net-next 0/2] tipc: make link protocol more resilient

[net-next 0/2] tipc: make link protocol more resilient

[Jon Maloy]

These two commits make the link ptotocol more resilient to 
infrastructures with frequent packet duplication and long delays.

Jon Maloy (2):
  tipc: add sequence number check for link STATE messages
  tipc: check session number before accepting link protocol messages

 net/tipc/link.c | 76 ++++++++++++++++++++++++++++++++++++++++++++-------------
 net/tipc/link.h |  2 ++
 net/tipc/node.c | 12 ++++++++-
 net/tipc/node.h | 14 ++++++-----
 4 files changed, 80 insertions(+), 24 deletions(-)

-- 
2.1.4

[net-next 1/2] tipc: add sequence number check for link STATE messages

[Jon Maloy]

Some switch infrastructures produce huge amounts of packet duplicates.
This becomes a problem if those messages are STATE/NACK protocol
messages, causing unnecessary retransmissions of already accepted
packets.

We now introduce a unique sequence number per STATE protocol message
so that duplicates can be identified and ignored. This will also be
useful when tracing such cases, and to avert replay attacks when TIPC
is encrypted.

For compatibility reasons we have to introduce a new capability flag
TIPC_LINK_PROTO_SEQNO to handle this new feature.

Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/link.c | 16 ++++++++++++++++
 net/tipc/link.h |  1 +
 net/tipc/node.c |  7 +++++++
 net/tipc/node.h | 14 ++++++++------
 4 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index ec4d283..065e9e6 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -130,6 +130,8 @@ struct tipc_link {
 	/* Management and link supervision data */
 	u32 peer_session;
 	u32 session;
+	u16 snd_nxt_state;
+	u16 rcv_nxt_state;
 	u32 peer_bearer_id;
 	u32 bearer_id;
 	u32 tolerance;
@@ -339,6 +341,11 @@ char tipc_link_plane(struct tipc_link *l)
 	return l->net_plane;
 }
 
+void tipc_link_update_caps(struct tipc_link *l, u16 capabilities)
+{
+	l->peer_caps = capabilities;
+}
+
 void tipc_link_add_bc_peer(struct tipc_link *snd_l,
 			   struct tipc_link *uc_l,
 			   struct sk_buff_head *xmitq)
@@ -859,6 +866,8 @@ void tipc_link_reset(struct tipc_link *l)
 	l->rcv_unacked = 0;
 	l->snd_nxt = 1;
 	l->rcv_nxt = 1;
+	l->snd_nxt_state = 1;
+	l->rcv_nxt_state = 1;
 	l->acked = 0;
 	l->silent_intv_cnt = 0;
 	l->rst_cnt = 0;
@@ -1353,6 +1362,8 @@ static void tipc_link_build_proto_msg(struct tipc_link *l, int mtyp, bool probe,
 	msg_set_seqno(hdr, l->snd_nxt + U16_MAX / 2);
 
 	if (mtyp == STATE_MSG) {
+		if (l->peer_caps & TIPC_LINK_PROTO_SEQNO)
+			msg_set_seqno(hdr, l->snd_nxt_state++);
 		msg_set_seq_gap(hdr, rcvgap);
 		msg_set_bc_gap(hdr, link_bc_rcv_gap(bcl));
 		msg_set_probe(hdr, probe);
@@ -1522,6 +1533,11 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
 
 	case STATE_MSG:
 
+		if (l->peer_caps & TIPC_LINK_PROTO_SEQNO &&
+		    less(msg_seqno(hdr), l->rcv_nxt_state))
+			break;
+		l->rcv_nxt_state = msg_seqno(hdr) + 1;
+
 		/* Update own tolerance if peer indicates a non-zero value */
 		if (in_range(peers_tol, TIPC_MIN_LINK_TOL, TIPC_MAX_LINK_TOL))
 			l->tolerance = peers_tol;
diff --git a/net/tipc/link.h b/net/tipc/link.h
index ec59348..d56f9c9 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -110,6 +110,7 @@ char *tipc_link_name(struct tipc_link *l);
 char tipc_link_plane(struct tipc_link *l);
 int tipc_link_prio(struct tipc_link *l);
 int tipc_link_window(struct tipc_link *l);
+void tipc_link_update_caps(struct tipc_link *l, u16 capabilities);
 unsigned long tipc_link_tolerance(struct tipc_link *l);
 void tipc_link_set_tolerance(struct tipc_link *l, u32 tol,
 			     struct sk_buff_head *xmitq);
diff --git a/net/tipc/node.c b/net/tipc/node.c
index cfdbaf4..1cdb176 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -363,6 +363,8 @@ static struct tipc_node *tipc_node_create(struct net *net, u32 addr,
 {
 	struct tipc_net *tn = net_generic(net, tipc_net_id);
 	struct tipc_node *n, *temp_node;
+	struct tipc_link *l;
+	int bearer_id;
 	int i;
 
 	spin_lock_bh(&tn->node_list_lock);
@@ -370,6 +372,11 @@ static struct tipc_node *tipc_node_create(struct net *net, u32 addr,
 	if (n) {
 		/* Same node may come back with new capabilities */
 		n->capabilities = capabilities;
+		for (bearer_id = 0; bearer_id < MAX_BEARERS; bearer_id++) {
+			l = n->links[bearer_id].link;
+			if (l)
+				tipc_link_update_caps(l, capabilities);
+		}
 		goto exit;
 	}
 	n = kzalloc(sizeof(*n), GFP_ATOMIC);
diff --git a/net/tipc/node.h b/net/tipc/node.h
index 846c8f2..48b3298 100644
--- a/net/tipc/node.h
+++ b/net/tipc/node.h
@@ -49,14 +49,16 @@ enum {
 	TIPC_BCAST_STATE_NACK = (1 << 2),
 	TIPC_BLOCK_FLOWCTL    = (1 << 3),
 	TIPC_BCAST_RCAST      = (1 << 4),
-	TIPC_NODE_ID128       = (1 << 5)
+	TIPC_NODE_ID128       = (1 << 5),
+	TIPC_LINK_PROTO_SEQNO = (1 << 6)
 };
 
-#define TIPC_NODE_CAPABILITIES (TIPC_BCAST_SYNCH | \
-				TIPC_BCAST_STATE_NACK | \
-				TIPC_BCAST_RCAST | \
-				TIPC_BLOCK_FLOWCTL | \
-				TIPC_NODE_ID128)
+#define TIPC_NODE_CAPABILITIES (TIPC_BCAST_SYNCH       |  \
+				TIPC_BCAST_STATE_NACK  |  \
+				TIPC_BCAST_RCAST       |  \
+				TIPC_BLOCK_FLOWCTL     |  \
+				TIPC_NODE_ID128        |  \
+				TIPC_LINK_PROTO_SEQNO)
 #define INVALID_BEARER_ID -1
 
 void tipc_node_stop(struct net *net);
-- 
2.1.4

[net-next 2/2] tipc: check session number before accepting link protocol messages

[Jon Maloy]

In some virtual environments we observe a significant higher number of
packet reordering and delays than we have been used to traditionally.

This makes it necessary with stricter checks on incoming link protocol
messages' session number, which until now only has been validated for
RESET messages.

Since the other two message types, ACTIVATE and STATE messages also
carry this number, it is easy to extend the validation check to those
messages.

We also introduce a flag indicating if a link has a valid peer session
number or not. This eliminates the mixing of 32- and 16-bit arithmethics
we are currently using to achieve this.

Acked-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/link.c | 68 +++++++++++++++++++++++++++++++++++++++------------------
 net/tipc/link.h |  1 +
 net/tipc/node.c |  5 ++++-
 3 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 065e9e6..df763be 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -128,8 +128,8 @@ struct tipc_link {
 	struct net *net;
 
 	/* Management and link supervision data */
-	u32 peer_session;
-	u32 session;
+	u16 peer_session;
+	u16 session;
 	u16 snd_nxt_state;
 	u16 rcv_nxt_state;
 	u32 peer_bearer_id;
@@ -138,6 +138,7 @@ struct tipc_link {
 	u32 abort_limit;
 	u32 state;
 	u16 peer_caps;
+	bool in_session;
 	bool active;
 	u32 silent_intv_cnt;
 	char if_name[TIPC_MAX_IF_NAME];
@@ -216,11 +217,6 @@ enum {
  */
 #define TIPC_NACK_INTV (TIPC_MIN_LINK_WIN * 2)
 
-/* Wildcard value for link session numbers. When it is known that
- * peer endpoint is down, any session number must be accepted.
- */
-#define ANY_SESSION 0x10000
-
 /* Link FSM states:
  */
 enum {
@@ -478,7 +474,7 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id,
 	l->addr = peer;
 	l->peer_caps = peer_caps;
 	l->net = net;
-	l->peer_session = ANY_SESSION;
+	l->in_session = false;
 	l->bearer_id = bearer_id;
 	l->tolerance = tolerance;
 	l->net_plane = net_plane;
@@ -847,7 +843,7 @@ void link_prepare_wakeup(struct tipc_link *l)
 
 void tipc_link_reset(struct tipc_link *l)
 {
-	l->peer_session = ANY_SESSION;
+	l->in_session = false;
 	l->session++;
 	l->mtu = l->advertised_mtu;
 	__skb_queue_purge(&l->transmq);
@@ -1455,6 +1451,44 @@ void tipc_link_tnl_prepare(struct tipc_link *l, struct tipc_link *tnl,
 	}
 }
 
+/* tipc_link_validate_msg(): validate message against current link state
+ * Returns true if message should be accepted, otherwise false
+ */
+bool tipc_link_validate_msg(struct tipc_link *l, struct tipc_msg *hdr)
+{
+	u16 curr_session = l->peer_session;
+	u16 session = msg_session(hdr);
+	int mtyp = msg_type(hdr);
+
+	if (msg_user(hdr) != LINK_PROTOCOL)
+		return true;
+
+	switch (mtyp) {
+	case RESET_MSG:
+		if (!l->in_session)
+			return true;
+		/* Accept only RESET with new session number */
+		return more(session, curr_session);
+	case ACTIVATE_MSG:
+		if (!l->in_session)
+			return true;
+		/* Accept only ACTIVATE with new or current session number */
+		return !less(session, curr_session);
+	case STATE_MSG:
+		/* Accept only STATE with current session number */
+		if (!l->in_session)
+			return false;
+		if (session != curr_session)
+			return false;
+		if (!(l->peer_caps & TIPC_LINK_PROTO_SEQNO))
+			return true;
+		/* Accept only STATE with new sequence number */
+		return !less(msg_seqno(hdr), l->rcv_nxt_state);
+	default:
+		return false;
+	}
+}
+
 /* tipc_link_proto_rcv(): receive link level protocol message :
  * Note that network plane id propagates through the network, and may
  * change at any time. The node with lowest numerical id determines
@@ -1488,17 +1522,12 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
 	hdr = buf_msg(skb);
 	data = msg_data(hdr);
 
+	if (!tipc_link_validate_msg(l, hdr))
+		goto exit;
+
 	switch (mtyp) {
 	case RESET_MSG:
-
-		/* Ignore duplicate RESET with old session number */
-		if ((less_eq(msg_session(hdr), l->peer_session)) &&
-		    (l->peer_session != ANY_SESSION))
-			break;
-		/* fall thru' */
-
 	case ACTIVATE_MSG:
-
 		/* Complete own link name with peer's interface name */
 		if_name =  strrchr(l->name, ':') + 1;
 		if (sizeof(l->name) - (if_name - l->name) <= TIPC_MAX_IF_NAME)
@@ -1526,16 +1555,13 @@ static int tipc_link_proto_rcv(struct tipc_link *l, struct sk_buff *skb,
 			rc = TIPC_LINK_UP_EVT;
 
 		l->peer_session = msg_session(hdr);
+		l->in_session = true;
 		l->peer_bearer_id = msg_bearer_id(hdr);
 		if (l->mtu > msg_max_pkt(hdr))
 			l->mtu = msg_max_pkt(hdr);
 		break;
 
 	case STATE_MSG:
-
-		if (l->peer_caps & TIPC_LINK_PROTO_SEQNO &&
-		    less(msg_seqno(hdr), l->rcv_nxt_state))
-			break;
 		l->rcv_nxt_state = msg_seqno(hdr) + 1;
 
 		/* Update own tolerance if peer indicates a non-zero value */
diff --git a/net/tipc/link.h b/net/tipc/link.h
index d56f9c9..7bc494a3 100644
--- a/net/tipc/link.h
+++ b/net/tipc/link.h
@@ -111,6 +111,7 @@ char tipc_link_plane(struct tipc_link *l);
 int tipc_link_prio(struct tipc_link *l);
 int tipc_link_window(struct tipc_link *l);
 void tipc_link_update_caps(struct tipc_link *l, u16 capabilities);
+bool tipc_link_validate_msg(struct tipc_link *l, struct tipc_msg *hdr);
 unsigned long tipc_link_tolerance(struct tipc_link *l);
 void tipc_link_set_tolerance(struct tipc_link *l, u32 tol,
 			     struct sk_buff_head *xmitq);
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 1cdb176..52fd80b 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -1540,7 +1540,7 @@ static void tipc_node_bc_rcv(struct net *net, struct sk_buff *skb, int bearer_id
  * tipc_node_check_state - check and if necessary update node state
  * @skb: TIPC packet
  * @bearer_id: identity of bearer delivering the packet
- * Returns true if state is ok, otherwise consumes buffer and returns false
+ * Returns true if state and msg are ok, otherwise false
  */
 static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb,
 				  int bearer_id, struct sk_buff_head *xmitq)
@@ -1574,6 +1574,9 @@ static bool tipc_node_check_state(struct tipc_node *n, struct sk_buff *skb,
 		}
 	}
 
+	if (!tipc_link_validate_msg(l, hdr))
+		return false;
+
 	/* Check and update node accesibility if applicable */
 	if (state == SELF_UP_PEER_COMING) {
 		if (!tipc_link_is_up(l))
-- 
2.1.4

Re: [net-next 0/2] tipc: make link protocol more resilient

[David Miller]

From: Jon Maloy <jon.maloy@ericsson.com>
Date: Tue, 10 Jul 2018 01:07:34 +0200

> These two commits make the link ptotocol more resilient to 
> infrastructures with frequent packet duplication and long delays.

Series applied, thanks Jon.