Commit dfb7e3b9 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'sctp-fix-a-vtag-verification-failure-caused-by-stale-inits' 

Xin Long says:

====================
sctp: fix a vtag verification failure caused by stale INITs

Similar to Scenario B in commit 8e56b063 ( netfilter: handle the
connecting collision properly in nf_conntrack_proto_sctp"):

Scenario B: INIT_ACK is delayed until the peer completes its own handshake

  192.168.1.2 > 192.168.1.1: sctp (1) [INIT] [init tag: 3922216408]
    192.168.1.1 > 192.168.1.2: sctp (1) [INIT] [init tag: 144230885]
    192.168.1.2 > 192.168.1.1: sctp (1) [INIT ACK] [init tag: 3922216408]
    192.168.1.1 > 192.168.1.2: sctp (1) [COOKIE ECHO]
    192.168.1.2 > 192.168.1.1: sctp (1) [COOKIE ACK]
  192.168.1.1 > 192.168.1.2: sctp (1) [INIT ACK] [init tag: 3914796021] *

There is another case:

Scenario F: INIT is delayed until the peer completes its own handshake

  192.168.1.2 > 192.168.1.1: sctp (1) [INIT] [init tag: 3922216408]
  (OVS upcall)
    192.168.1.1 > 192.168.1.2: sctp (1) [INIT] [init tag: 144230885]
    192.168.1.2 > 192.168.1.1: sctp (1) [INIT ACK] [init tag: 3922216408]
    192.168.1.1 > 192.168.1.2: sctp (1) [COOKIE ECHO]
    192.168.1.2 > 192.168.1.1: sctp (1) [COOKIE ACK]
  192.168.1.2 > 192.168.1.1: sctp (1) [INIT] [init tag: 3922216408]
  (delayed)
  192.168.1.1 > 192.168.1.2: sctp (1) [INIT ACK] [init tag: 3914796021] *

In this case, the delayed INIT (e.g. due to OVS upcall) is recorded by
conntrack, which prevents vtag verification from dropping the unexpected
INIT-ACK in nf_conntrack_sctp_packet():

  vtag = ct->proto.sctp.vtag[!dir];
  if (!ct->proto.sctp.init[!dir] && vtag && vtag != ih->init_tag)
          goto out_unlock;

This happens because ct->proto.sctp.init[!dir] is set by the delayed INIT,
even though it is stale.

Fix this in two parts:

- In netfilter: Do not record INITs whose init_tag matches the peer vtag,
  as they carry no new handshake state in the 1st patch.

- In SCTP: Prevent endpoints from responding to such INITs with INIT-ACK,
  ensuring correctness even when middleboxes lack the netfilter fix in
  the 2nd patch.

A follow-up selftest for this scenario will be posted in a separate patch
by Yi Chen.
====================

Link: https://patch.msgid.link/cover.1777214801.git.lucien.xin@gmail.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents b718342a 8a92cb47

net/netfilter/nf_conntrack_proto_sctp.c

+7 −3
Original line number Diff line number Diff line
@@ -466,9 +466,13 @@ int nf_conntrack_sctp_packet(struct nf_conn *ct, 
			if (!ih)

				goto out_unlock;



			/* Do not record INIT matching peer vtag (stale or retransmitted INIT). */

			if (old_state == SCTP_CONNTRACK_NONE ||

			    ct->proto.sctp.vtag[!dir] != ih->init_tag) {

				if (ct->proto.sctp.init[dir] && ct->proto.sctp.init[!dir])

					ct->proto.sctp.init[!dir] = 0;

				ct->proto.sctp.init[dir] = 1;

			}



			pr_debug("Setting vtag %x for dir %d\n", ih->init_tag, !dir);

			ct->proto.sctp.vtag[!dir] = ih->init_tag;

net/sctp/sm_statefuns.c

+6 −0
Original line number Diff line number Diff line
@@ -1556,6 +1556,12 @@ static enum sctp_disposition sctp_sf_do_unexpected_init( 
	/* Tag the variable length parameters.  */

	chunk->param_hdr.v = skb_pull(chunk->skb, sizeof(struct sctp_inithdr));



	if (asoc->state >= SCTP_STATE_ESTABLISHED) {

		/* Discard INIT matching peer vtag after handshake completion (stale INIT). */

		if (ntohl(chunk->subh.init_hdr->init_tag) == asoc->peer.i.init_tag)

			return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);

	}



	/* Verify the INIT chunk before processing it. */

	err_chunk = NULL;

	if (!sctp_verify_init(net, ep, asoc, chunk->chunk_hdr->type,