netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/7] sctp: enhancements for the verification tag
@ 2021-10-20 11:42 Xin Long
  2021-10-20 11:42 ` [PATCH net 1/7] sctp: use init_tag from inithdr for ABORT chunk Xin Long
                   ` (8 more replies)
  0 siblings, 9 replies; 11+ messages in thread
From: Xin Long @ 2021-10-20 11:42 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, michael.tuexen

This patchset is to address CVE-2021-3772:

  A flaw was found in the Linux SCTP stack. A blind attacker may be able to
  kill an existing SCTP association through invalid chunks if the attacker
  knows the IP-addresses and port numbers being used and the attacker can
  send packets with spoofed IP addresses.

This is caused by the missing VTAG verification for the received chunks
and the incorrect vtag for the ABORT used to reply to these invalid
chunks.

This patchset is to go over all processing functions for the received
chunks and do:

1. Make sure sctp_vtag_verify() is called firstly to verify the vtag from
   the received chunk and discard this chunk if it fails. With some
   exceptions:

   a. sctp_sf_do_5_1B_init()/5_2_2_dupinit()/9_2_reshutack(), processing
      INIT chunk, as sctphdr vtag is always 0 in INIT chunk.

   b. sctp_sf_do_5_2_4_dupcook(), processing dupicate COOKIE_ECHO chunk,
      as the vtag verification will be done by sctp_tietags_compare() and
      then it takes right actions according to the return.

   c. sctp_sf_shut_8_4_5(), processing SHUTDOWN_ACK chunk for cookie_wait
      and cookie_echoed state, as RFC demand sending a SHUTDOWN_COMPLETE
      even if the vtag verification failed.

   d. sctp_sf_ootb(), called in many types of chunks for closed state or
      no asoc, as the same reason to c.

2. Always use the vtag from the received INIT chunk to make the response
   ABORT in sctp_ootb_pkt_new().

3. Fix the order for some checks and add some missing checks for the
   received chunk.

This patch series has been tested with SCTP TAHI testing to make sure no
regression caused on protocol conformance.

Xin Long (7):
  sctp: use init_tag from inithdr for ABORT chunk
  sctp: fix the processing for INIT chunk
  sctp: fix the processing for INIT_ACK chunk
  sctp: fix the processing for COOKIE_ECHO chunk
  sctp: add vtag check in sctp_sf_violation
  sctp: add vtag check in sctp_sf_do_8_5_1_E_sa
  sctp: add vtag check in sctp_sf_ootb

 net/sctp/sm_statefuns.c | 139 ++++++++++++++++++++++++----------------
 1 file changed, 85 insertions(+), 54 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net 1/7] sctp: use init_tag from inithdr for ABORT chunk
  2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
@ 2021-10-20 11:42 ` Xin Long
  2021-10-20 11:42 ` [PATCH net 2/7] sctp: fix the processing for INIT chunk Xin Long
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Xin Long @ 2021-10-20 11:42 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, michael.tuexen

Currently Linux SCTP uses the verification tag of the existing SCTP
asoc when failing to process and sending the packet with the ABORT
chunk. This will result in the peer accepting the ABORT chunk and
removing the SCTP asoc. One could exploit this to terminate a SCTP
asoc.

This patch is to fix it by always using the initiate tag of the
received INIT chunk for the ABORT chunk to be sent.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/sm_statefuns.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 32df65f68c12..7f8306968c39 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -6348,6 +6348,7 @@ static struct sctp_packet *sctp_ootb_pkt_new(
 		 * yet.
 		 */
 		switch (chunk->chunk_hdr->type) {
+		case SCTP_CID_INIT:
 		case SCTP_CID_INIT_ACK:
 		{
 			struct sctp_initack_chunk *initack;
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net 2/7] sctp: fix the processing for INIT chunk
  2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
  2021-10-20 11:42 ` [PATCH net 1/7] sctp: use init_tag from inithdr for ABORT chunk Xin Long
@ 2021-10-20 11:42 ` Xin Long
  2021-10-20 11:42 ` [PATCH net 3/7] sctp: fix the processing for INIT_ACK chunk Xin Long
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Xin Long @ 2021-10-20 11:42 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, michael.tuexen

This patch fixes the problems below:

1. In non-shutdown_ack_sent states: in sctp_sf_do_5_1B_init() and
   sctp_sf_do_5_2_2_dupinit():

  chunk length check should be done before any checks that may cause
  to send abort, as making packet for abort will access the init_tag
  from init_hdr in sctp_ootb_pkt_new().

2. In shutdown_ack_sent state: in sctp_sf_do_9_2_reshutack():

  The same checks as does in sctp_sf_do_5_2_2_dupinit() is needed
  for sctp_sf_do_9_2_reshutack().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/sm_statefuns.c | 72 ++++++++++++++++++++++++++---------------
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 7f8306968c39..9bfa8cca9974 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -156,6 +156,12 @@ static enum sctp_disposition __sctp_sf_do_9_1_abort(
 					void *arg,
 					struct sctp_cmd_seq *commands);
 
+static enum sctp_disposition
+__sctp_sf_do_9_2_reshutack(struct net *net, const struct sctp_endpoint *ep,
+			   const struct sctp_association *asoc,
+			   const union sctp_subtype type, void *arg,
+			   struct sctp_cmd_seq *commands);
+
 /* Small helper function that checks if the chunk length
  * is of the appropriate length.  The 'required_length' argument
  * is set to be the size of a specific chunk we are testing.
@@ -337,6 +343,14 @@ enum sctp_disposition sctp_sf_do_5_1B_init(struct net *net,
 	if (!chunk->singleton)
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 
+	/* Make sure that the INIT chunk has a valid length.
+	 * Normally, this would cause an ABORT with a Protocol Violation
+	 * error, but since we don't have an association, we'll
+	 * just discard the packet.
+	 */
+	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_init_chunk)))
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+
 	/* If the packet is an OOTB packet which is temporarily on the
 	 * control endpoint, respond with an ABORT.
 	 */
@@ -351,14 +365,6 @@ enum sctp_disposition sctp_sf_do_5_1B_init(struct net *net,
 	if (chunk->sctp_hdr->vtag != 0)
 		return sctp_sf_tabort_8_4_8(net, ep, asoc, type, arg, commands);
 
-	/* Make sure that the INIT chunk has a valid length.
-	 * Normally, this would cause an ABORT with a Protocol Violation
-	 * error, but since we don't have an association, we'll
-	 * just discard the packet.
-	 */
-	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_init_chunk)))
-		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
-
 	/* If the INIT is coming toward a closing socket, we'll send back
 	 * and ABORT.  Essentially, this catches the race of INIT being
 	 * backloged to the socket at the same time as the user issues close().
@@ -1524,20 +1530,16 @@ static enum sctp_disposition sctp_sf_do_unexpected_init(
 	if (!chunk->singleton)
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 
+	/* Make sure that the INIT chunk has a valid length. */
+	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_init_chunk)))
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+
 	/* 3.1 A packet containing an INIT chunk MUST have a zero Verification
 	 * Tag.
 	 */
 	if (chunk->sctp_hdr->vtag != 0)
 		return sctp_sf_tabort_8_4_8(net, ep, asoc, type, arg, commands);
 
-	/* Make sure that the INIT chunk has a valid length.
-	 * In this case, we generate a protocol violation since we have
-	 * an association established.
-	 */
-	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_init_chunk)))
-		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
-						  commands);
-
 	if (SCTP_INPUT_CB(chunk->skb)->encap_port != chunk->transport->encap_port)
 		return sctp_sf_new_encap_port(net, ep, asoc, type, arg, commands);
 
@@ -1882,9 +1884,9 @@ static enum sctp_disposition sctp_sf_do_dupcook_a(
 	 * its peer.
 	*/
 	if (sctp_state(asoc, SHUTDOWN_ACK_SENT)) {
-		disposition = sctp_sf_do_9_2_reshutack(net, ep, asoc,
-				SCTP_ST_CHUNK(chunk->chunk_hdr->type),
-				chunk, commands);
+		disposition = __sctp_sf_do_9_2_reshutack(net, ep, asoc,
+							 SCTP_ST_CHUNK(chunk->chunk_hdr->type),
+							 chunk, commands);
 		if (SCTP_DISPOSITION_NOMEM == disposition)
 			goto nomem;
 
@@ -2970,13 +2972,11 @@ enum sctp_disposition sctp_sf_do_9_2_shut_ctsn(
  * that belong to this association, it should discard the INIT chunk and
  * retransmit the SHUTDOWN ACK chunk.
  */
-enum sctp_disposition sctp_sf_do_9_2_reshutack(
-					struct net *net,
-					const struct sctp_endpoint *ep,
-					const struct sctp_association *asoc,
-					const union sctp_subtype type,
-					void *arg,
-					struct sctp_cmd_seq *commands)
+static enum sctp_disposition
+__sctp_sf_do_9_2_reshutack(struct net *net, const struct sctp_endpoint *ep,
+			   const struct sctp_association *asoc,
+			   const union sctp_subtype type, void *arg,
+			   struct sctp_cmd_seq *commands)
 {
 	struct sctp_chunk *chunk = arg;
 	struct sctp_chunk *reply;
@@ -3010,6 +3010,26 @@ enum sctp_disposition sctp_sf_do_9_2_reshutack(
 	return SCTP_DISPOSITION_NOMEM;
 }
 
+enum sctp_disposition
+sctp_sf_do_9_2_reshutack(struct net *net, const struct sctp_endpoint *ep,
+			 const struct sctp_association *asoc,
+			 const union sctp_subtype type, void *arg,
+			 struct sctp_cmd_seq *commands)
+{
+	struct sctp_chunk *chunk = arg;
+
+	if (!chunk->singleton)
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+
+	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_init_chunk)))
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+
+	if (chunk->sctp_hdr->vtag != 0)
+		return sctp_sf_tabort_8_4_8(net, ep, asoc, type, arg, commands);
+
+	return __sctp_sf_do_9_2_reshutack(net, ep, asoc, type, arg, commands);
+}
+
 /*
  * sctp_sf_do_ecn_cwr
  *
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net 3/7] sctp: fix the processing for INIT_ACK chunk
  2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
  2021-10-20 11:42 ` [PATCH net 1/7] sctp: use init_tag from inithdr for ABORT chunk Xin Long
  2021-10-20 11:42 ` [PATCH net 2/7] sctp: fix the processing for INIT chunk Xin Long
@ 2021-10-20 11:42 ` Xin Long
  2021-10-20 11:42 ` [PATCH net 4/7] sctp: fix the processing for COOKIE_ECHO chunk Xin Long
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Xin Long @ 2021-10-20 11:42 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, michael.tuexen

Currently INIT_ACK chunk in non-cookie_echoed state is processed in
sctp_sf_discard_chunk() to send an abort with the existent asoc's
vtag if the chunk length is not valid. But the vtag in the chunk's
sctphdr is not verified, which may be exploited by one to cook a
malicious chunk to terminal a SCTP asoc.

sctp_sf_discard_chunk() also is called in many other places to send
an abort, and most of those have this problem. This patch is to fix
it by sending abort with the existent asoc's vtag only if the vtag
from the chunk's sctphdr is verified in sctp_sf_discard_chunk().

Note on sctp_sf_do_9_1_abort() and sctp_sf_shutdown_pending_abort(),
the chunk length has been verified before sctp_sf_discard_chunk(),
so replace it with sctp_sf_discard(). On sctp_sf_do_asconf_ack() and
sctp_sf_do_asconf(), move the sctp_chunk_length_valid check ahead of
sctp_sf_discard_chunk(), then replace it with sctp_sf_discard().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/sm_statefuns.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 9bfa8cca9974..672e5308839b 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -2343,7 +2343,7 @@ enum sctp_disposition sctp_sf_shutdown_pending_abort(
 	 */
 	if (SCTP_ADDR_DEL ==
 		    sctp_bind_addr_state(&asoc->base.bind_addr, &chunk->dest))
-		return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands);
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 
 	if (!sctp_err_chunk_valid(chunk))
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
@@ -2389,7 +2389,7 @@ enum sctp_disposition sctp_sf_shutdown_sent_abort(
 	 */
 	if (SCTP_ADDR_DEL ==
 		    sctp_bind_addr_state(&asoc->base.bind_addr, &chunk->dest))
-		return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands);
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 
 	if (!sctp_err_chunk_valid(chunk))
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
@@ -2659,7 +2659,7 @@ enum sctp_disposition sctp_sf_do_9_1_abort(
 	 */
 	if (SCTP_ADDR_DEL ==
 		    sctp_bind_addr_state(&asoc->base.bind_addr, &chunk->dest))
-		return sctp_sf_discard_chunk(net, ep, asoc, type, arg, commands);
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 
 	if (!sctp_err_chunk_valid(chunk))
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
@@ -3865,6 +3865,11 @@ enum sctp_disposition sctp_sf_do_asconf(struct net *net,
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 	}
 
+	/* Make sure that the ASCONF ADDIP chunk has a valid length.  */
+	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_addip_chunk)))
+		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
+						  commands);
+
 	/* ADD-IP: Section 4.1.1
 	 * This chunk MUST be sent in an authenticated way by using
 	 * the mechanism defined in [I-D.ietf-tsvwg-sctp-auth]. If this chunk
@@ -3873,13 +3878,7 @@ enum sctp_disposition sctp_sf_do_asconf(struct net *net,
 	 */
 	if (!asoc->peer.asconf_capable ||
 	    (!net->sctp.addip_noauth && !chunk->auth))
-		return sctp_sf_discard_chunk(net, ep, asoc, type, arg,
-					     commands);
-
-	/* Make sure that the ASCONF ADDIP chunk has a valid length.  */
-	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_addip_chunk)))
-		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
-						  commands);
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 
 	hdr = (struct sctp_addiphdr *)chunk->skb->data;
 	serial = ntohl(hdr->serial);
@@ -4008,6 +4007,12 @@ enum sctp_disposition sctp_sf_do_asconf_ack(struct net *net,
 		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 	}
 
+	/* Make sure that the ADDIP chunk has a valid length.  */
+	if (!sctp_chunk_length_valid(asconf_ack,
+				     sizeof(struct sctp_addip_chunk)))
+		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
+						  commands);
+
 	/* ADD-IP, Section 4.1.2:
 	 * This chunk MUST be sent in an authenticated way by using
 	 * the mechanism defined in [I-D.ietf-tsvwg-sctp-auth]. If this chunk
@@ -4016,14 +4021,7 @@ enum sctp_disposition sctp_sf_do_asconf_ack(struct net *net,
 	 */
 	if (!asoc->peer.asconf_capable ||
 	    (!net->sctp.addip_noauth && !asconf_ack->auth))
-		return sctp_sf_discard_chunk(net, ep, asoc, type, arg,
-					     commands);
-
-	/* Make sure that the ADDIP chunk has a valid length.  */
-	if (!sctp_chunk_length_valid(asconf_ack,
-				     sizeof(struct sctp_addip_chunk)))
-		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
-						  commands);
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
 
 	addip_hdr = (struct sctp_addiphdr *)asconf_ack->skb->data;
 	rcvd_serial = ntohl(addip_hdr->serial);
@@ -4595,6 +4593,9 @@ enum sctp_disposition sctp_sf_discard_chunk(struct net *net,
 {
 	struct sctp_chunk *chunk = arg;
 
+	if (asoc && !sctp_vtag_verify(chunk, asoc))
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+
 	/* Make sure that the chunk has a valid length.
 	 * Since we don't know the chunk type, we use a general
 	 * chunkhdr structure to make a comparison.
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net 4/7] sctp: fix the processing for COOKIE_ECHO chunk
  2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
                   ` (2 preceding siblings ...)
  2021-10-20 11:42 ` [PATCH net 3/7] sctp: fix the processing for INIT_ACK chunk Xin Long
@ 2021-10-20 11:42 ` Xin Long
  2021-10-20 11:42 ` [PATCH net 5/7] sctp: add vtag check in sctp_sf_violation Xin Long
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Xin Long @ 2021-10-20 11:42 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, michael.tuexen

1. In closed state: in sctp_sf_do_5_1D_ce():

  When asoc is NULL, making packet for abort will use chunk's vtag
  in sctp_ootb_pkt_new(). But when asoc exists, vtag from the chunk
  should be verified before using peer.i.init_tag to make packet
  for abort in sctp_ootb_pkt_new(), and just discard it if vtag is
  not correct.

2. In the other states: in sctp_sf_do_5_2_4_dupcook():

  asoc always exists, but duplicate cookie_echo's vtag will be
  handled by sctp_tietags_compare() and then take actions, so before
  that we only verify the vtag for the abort sent for invalid chunk
  length.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/sm_statefuns.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 672e5308839b..96a069d725e9 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -710,6 +710,9 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
 	struct sock *sk;
 	int error = 0;
 
+	if (asoc && !sctp_vtag_verify(chunk, asoc))
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+
 	/* If the packet is an OOTB packet which is temporarily on the
 	 * control endpoint, respond with an ABORT.
 	 */
@@ -724,7 +727,8 @@ enum sctp_disposition sctp_sf_do_5_1D_ce(struct net *net,
 	 * in sctp_unpack_cookie().
 	 */
 	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_chunkhdr)))
-		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
+						  commands);
 
 	/* If the endpoint is not listening or if the number of associations
 	 * on the TCP-style socket exceed the max backlog, respond with an
@@ -2204,9 +2208,11 @@ enum sctp_disposition sctp_sf_do_5_2_4_dupcook(
 	 * enough for the chunk header.  Cookie length verification is
 	 * done later.
 	 */
-	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_chunkhdr)))
-		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
-						  commands);
+	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_chunkhdr))) {
+		if (!sctp_vtag_verify(chunk, asoc))
+			asoc = NULL;
+		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg, commands);
+	}
 
 	/* "Decode" the chunk.  We have no optional parameters so we
 	 * are in good shape.
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net 5/7] sctp: add vtag check in sctp_sf_violation
  2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
                   ` (3 preceding siblings ...)
  2021-10-20 11:42 ` [PATCH net 4/7] sctp: fix the processing for COOKIE_ECHO chunk Xin Long
@ 2021-10-20 11:42 ` Xin Long
  2021-10-20 11:42 ` [PATCH net 6/7] sctp: add vtag check in sctp_sf_do_8_5_1_E_sa Xin Long
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Xin Long @ 2021-10-20 11:42 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, michael.tuexen

sctp_sf_violation() is called when processing HEARTBEAT_ACK chunk
in cookie_wait state, and some other places are also using it.

The vtag in the chunk's sctphdr should be verified, otherwise, as
later in chunk length check, it may send abort with the existent
asoc's vtag, which can be exploited by one to cook a malicious
chunk to terminate a SCTP asoc.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/sm_statefuns.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 96a069d725e9..36328ab88bdd 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4669,6 +4669,9 @@ enum sctp_disposition sctp_sf_violation(struct net *net,
 {
 	struct sctp_chunk *chunk = arg;
 
+	if (!sctp_vtag_verify(chunk, asoc))
+		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+
 	/* Make sure that the chunk has a valid length. */
 	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_chunkhdr)))
 		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net 6/7] sctp: add vtag check in sctp_sf_do_8_5_1_E_sa
  2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
                   ` (4 preceding siblings ...)
  2021-10-20 11:42 ` [PATCH net 5/7] sctp: add vtag check in sctp_sf_violation Xin Long
@ 2021-10-20 11:42 ` Xin Long
  2021-10-20 11:42 ` [PATCH net 7/7] sctp: add vtag check in sctp_sf_ootb Xin Long
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 11+ messages in thread
From: Xin Long @ 2021-10-20 11:42 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, michael.tuexen

sctp_sf_do_8_5_1_E_sa() is called when processing SHUTDOWN_ACK chunk
in cookie_wait and cookie_echoed state.

The vtag in the chunk's sctphdr should be verified, otherwise, as
later in chunk length check, it may send abort with the existent
asoc's vtag, which can be exploited by one to cook a malicious
chunk to terminate a SCTP asoc.

Note that when fails to verify the vtag from SHUTDOWN-ACK chunk,
SHUTDOWN COMPLETE message will still be sent back to peer, but
with the vtag from SHUTDOWN-ACK chunk, as said in 5) of
rfc4960#section-8.4.

While at it, also remove the unnecessary chunk length check from
sctp_sf_shut_8_4_5(), as it's already done in both places where
it calls sctp_sf_shut_8_4_5().

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/sm_statefuns.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 36328ab88bdd..a3545498a038 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -3803,12 +3803,6 @@ static enum sctp_disposition sctp_sf_shut_8_4_5(
 
 	SCTP_INC_STATS(net, SCTP_MIB_OUTCTRLCHUNKS);
 
-	/* If the chunk length is invalid, we don't want to process
-	 * the reset of the packet.
-	 */
-	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_chunkhdr)))
-		return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
-
 	/* We need to discard the rest of the packet to prevent
 	 * potential boomming attacks from additional bundled chunks.
 	 * This is documented in SCTP Threats ID.
@@ -3836,6 +3830,9 @@ enum sctp_disposition sctp_sf_do_8_5_1_E_sa(struct net *net,
 {
 	struct sctp_chunk *chunk = arg;
 
+	if (!sctp_vtag_verify(chunk, asoc))
+		asoc = NULL;
+
 	/* Make sure that the SHUTDOWN_ACK chunk has a valid length. */
 	if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_chunkhdr)))
 		return sctp_sf_violation_chunklen(net, ep, asoc, type, arg,
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH net 7/7] sctp: add vtag check in sctp_sf_ootb
  2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
                   ` (5 preceding siblings ...)
  2021-10-20 11:42 ` [PATCH net 6/7] sctp: add vtag check in sctp_sf_do_8_5_1_E_sa Xin Long
@ 2021-10-20 11:42 ` Xin Long
  2021-10-20 23:23 ` [PATCH net 0/7] sctp: enhancements for the verification tag Marcelo Ricardo Leitner
  2021-10-22  1:45 ` Marcelo Ricardo Leitner
  8 siblings, 0 replies; 11+ messages in thread
From: Xin Long @ 2021-10-20 11:42 UTC (permalink / raw)
  To: network dev, davem, kuba, linux-sctp
  Cc: Marcelo Ricardo Leitner, michael.tuexen

sctp_sf_ootb() is called when processing DATA chunk in closed state,
and many other places are also using it.

The vtag in the chunk's sctphdr should be verified, otherwise, as
later in chunk length check, it may send abort with the existent
asoc's vtag, which can be exploited by one to cook a malicious
chunk to terminate a SCTP asoc.

When fails to verify the vtag from the chunk, this patch sets asoc
to NULL, so that the abort will be made with the vtag from the
received chunk later.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/sm_statefuns.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index a3545498a038..fb3da4d8f4a3 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -3688,6 +3688,9 @@ enum sctp_disposition sctp_sf_ootb(struct net *net,
 
 	SCTP_INC_STATS(net, SCTP_MIB_OUTOFBLUES);
 
+	if (asoc && !sctp_vtag_verify(chunk, asoc))
+		asoc = NULL;
+
 	ch = (struct sctp_chunkhdr *)chunk->chunk_hdr;
 	do {
 		/* Report violation if the chunk is less then minimal */
-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 0/7] sctp: enhancements for the verification tag
  2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
                   ` (6 preceding siblings ...)
  2021-10-20 11:42 ` [PATCH net 7/7] sctp: add vtag check in sctp_sf_ootb Xin Long
@ 2021-10-20 23:23 ` Marcelo Ricardo Leitner
  2021-10-22  1:45 ` Marcelo Ricardo Leitner
  8 siblings, 0 replies; 11+ messages in thread
From: Marcelo Ricardo Leitner @ 2021-10-20 23:23 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, davem, kuba, linux-sctp, michael.tuexen

On Wed, Oct 20, 2021 at 07:42:40AM -0400, Xin Long wrote:
> This patchset is to address CVE-2021-3772:
> 
>   A flaw was found in the Linux SCTP stack. A blind attacker may be able to
>   kill an existing SCTP association through invalid chunks if the attacker
>   knows the IP-addresses and port numbers being used and the attacker can
>   send packets with spoofed IP addresses.

Please give me tomorrow to review it.

Thanks,
Marcelo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 0/7] sctp: enhancements for the verification tag
  2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
                   ` (7 preceding siblings ...)
  2021-10-20 23:23 ` [PATCH net 0/7] sctp: enhancements for the verification tag Marcelo Ricardo Leitner
@ 2021-10-22  1:45 ` Marcelo Ricardo Leitner
  2021-10-22 23:38   ` Jakub Kicinski
  8 siblings, 1 reply; 11+ messages in thread
From: Marcelo Ricardo Leitner @ 2021-10-22  1:45 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, davem, kuba, linux-sctp, michael.tuexen

On Wed, Oct 20, 2021 at 07:42:40AM -0400, Xin Long wrote:
> This patchset is to address CVE-2021-3772:
> 
>   A flaw was found in the Linux SCTP stack. A blind attacker may be able to
>   kill an existing SCTP association through invalid chunks if the attacker
>   knows the IP-addresses and port numbers being used and the attacker can
>   send packets with spoofed IP addresses.
> 
> This is caused by the missing VTAG verification for the received chunks
> and the incorrect vtag for the ABORT used to reply to these invalid
> chunks.
> 
> This patchset is to go over all processing functions for the received
> chunks and do:
> 
...
> 
> This patch series has been tested with SCTP TAHI testing to make sure no
> regression caused on protocol conformance.

Nice!

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH net 0/7] sctp: enhancements for the verification tag
  2021-10-22  1:45 ` Marcelo Ricardo Leitner
@ 2021-10-22 23:38   ` Jakub Kicinski
  0 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2021-10-22 23:38 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner, Xin Long
  Cc: network dev, davem, linux-sctp, michael.tuexen

On Thu, 21 Oct 2021 22:45:37 -0300 Marcelo Ricardo Leitner wrote:
> On Wed, Oct 20, 2021 at 07:42:40AM -0400, Xin Long wrote:
> > This patchset is to address CVE-2021-3772:
> > 
> >   A flaw was found in the Linux SCTP stack. A blind attacker may be able to
> >   kill an existing SCTP association through invalid chunks if the attacker
> >   knows the IP-addresses and port numbers being used and the attacker can
> >   send packets with spoofed IP addresses.
> > 
> > This is caused by the missing VTAG verification for the received chunks
> > and the incorrect vtag for the ABORT used to reply to these invalid
> > chunks.
> > 
> > This patchset is to go over all processing functions for the received
> > chunks and do:
> >   
> ...
> > 
> > This patch series has been tested with SCTP TAHI testing to make sure no
> > regression caused on protocol conformance.  
>  
> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

Applied, thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-10-22 23:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-20 11:42 [PATCH net 0/7] sctp: enhancements for the verification tag Xin Long
2021-10-20 11:42 ` [PATCH net 1/7] sctp: use init_tag from inithdr for ABORT chunk Xin Long
2021-10-20 11:42 ` [PATCH net 2/7] sctp: fix the processing for INIT chunk Xin Long
2021-10-20 11:42 ` [PATCH net 3/7] sctp: fix the processing for INIT_ACK chunk Xin Long
2021-10-20 11:42 ` [PATCH net 4/7] sctp: fix the processing for COOKIE_ECHO chunk Xin Long
2021-10-20 11:42 ` [PATCH net 5/7] sctp: add vtag check in sctp_sf_violation Xin Long
2021-10-20 11:42 ` [PATCH net 6/7] sctp: add vtag check in sctp_sf_do_8_5_1_E_sa Xin Long
2021-10-20 11:42 ` [PATCH net 7/7] sctp: add vtag check in sctp_sf_ootb Xin Long
2021-10-20 23:23 ` [PATCH net 0/7] sctp: enhancements for the verification tag Marcelo Ricardo Leitner
2021-10-22  1:45 ` Marcelo Ricardo Leitner
2021-10-22 23:38   ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).