From 8797168fe2f4fd32d241126181ad0d06c62c4eb4 Mon Sep 17 00:00:00 2001 From: Marco Varlese Date: Thu, 4 Oct 2018 15:46:05 +0200 Subject: [PATCH] SCTP: DATA chunk padding fix and hardening According to the RFC 4096 (section 3.3.1) the DATA chunk needs to be padded to a boundary of 4 bytes with zeros. This patch addresses that requirement. At the same time, this patch takes care of adding some hardening for corner-cases where the transmitted tag could be wrong. Change-Id: I3b653926e9933d0d3d46bc5f37eaceefd932e874 Signed-off-by: Marco Varlese --- src/vnet/sctp/sctp.c | 1 - src/vnet/sctp/sctp_input.c | 35 +++++++++++++++++++++++++++++------ src/vnet/sctp/sctp_output.c | 8 ++++++++ test/test_sctp.py | 1 + 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/vnet/sctp/sctp.c b/src/vnet/sctp/sctp.c index 53b44410f07..f2de34dccee 100644 --- a/src/vnet/sctp/sctp.c +++ b/src/vnet/sctp/sctp.c @@ -624,7 +624,6 @@ sctp_session_send_mss (transport_connection_t * trans_conn) update_smallest_pmtu_idx (sctp_conn); u8 idx = sctp_data_subconn_select (sctp_conn); - return sctp_conn->sub_conn[idx].cwnd; } diff --git a/src/vnet/sctp/sctp_input.c b/src/vnet/sctp/sctp_input.c index 4f6214501e2..e548707d32a 100644 --- a/src/vnet/sctp/sctp_input.c +++ b/src/vnet/sctp/sctp_input.c @@ -777,7 +777,9 @@ sctp_handle_data (sctp_payload_data_chunk_t * sctp_data_chunk, /* Check that the LOCALLY generated tag is being used by the REMOTE peer as the verification tag */ if (sctp_conn->local_tag != sctp_data_chunk->sctp_hdr.verification_tag) { - return SCTP_ERROR_INVALID_TAG; + *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4); + sctp_conn->sub_conn[idx].enqueue_state = SCTP_ERROR_INVALID_TAG; + return sctp_conn->sub_conn[idx].enqueue_state; } vnet_buffer (b)->sctp.sid = sctp_data_chunk->stream_id; @@ -786,8 +788,27 @@ sctp_handle_data (sctp_payload_data_chunk_t * sctp_data_chunk, u32 tsn = clib_net_to_host_u32 (sctp_data_chunk->tsn); vlib_buffer_advance (b, vnet_buffer (b)->sctp.data_offset); + u32 chunk_len = vnet_sctp_get_chunk_length (&sctp_data_chunk->chunk_hdr) - + (sizeof (sctp_payload_data_chunk_t) - sizeof (sctp_header_t)); + + ASSERT (vnet_buffer (b)->sctp.data_len); + ASSERT (chunk_len); + + /* Padding was added: see RFC 4096 section 3.3.1 */ + if (vnet_buffer (b)->sctp.data_len > chunk_len) + { + /* Let's change the data_len to the right amount calculated here now. + * We cannot do that in the generic sctp46_input_dispatcher node since + * that is common to all CHUNKS handling. + */ + vnet_buffer (b)->sctp.data_len = chunk_len; + /* We need to change b->current_length so that downstream calls to + * session_enqueue_stream_connection (called by sctp_session_enqueue_data) + * push the correct amount of data to be enqueued. + */ + b->current_length = chunk_len; + } n_data_bytes = vnet_buffer (b)->sctp.data_len; - ASSERT (n_data_bytes); sctp_is_connection_gapping (sctp_conn, tsn, &is_gapping); @@ -859,6 +880,7 @@ sctp_handle_cookie_echo (sctp_header_t * sctp_hdr, /* Check that the LOCALLY generated tag is being used by the REMOTE peer as the verification tag */ if (sctp_conn->local_tag != sctp_hdr->verification_tag) { + *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4); return SCTP_ERROR_INVALID_TAG; } @@ -901,6 +923,7 @@ sctp_handle_cookie_ack (sctp_header_t * sctp_hdr, /* Check that the LOCALLY generated tag is being used by the REMOTE peer as the verification tag */ if (sctp_conn->local_tag != sctp_hdr->verification_tag) { + *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4); return SCTP_ERROR_INVALID_TAG; } @@ -1181,6 +1204,7 @@ sctp_handle_shutdown (sctp_header_t * sctp_hdr, /* Check that the LOCALLY generated tag is being used by the REMOTE peer as the verification tag */ if (sctp_conn->local_tag != sctp_hdr->verification_tag) { + *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4); return SCTP_ERROR_INVALID_TAG; } @@ -1221,6 +1245,7 @@ sctp_handle_shutdown_ack (sctp_header_t * sctp_hdr, /* Check that the LOCALLY generated tag is being used by the REMOTE peer as the verification tag */ if (sctp_conn->local_tag != sctp_hdr->verification_tag) { + *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4); return SCTP_ERROR_INVALID_TAG; } @@ -1257,6 +1282,7 @@ sctp_handle_shutdown_complete (sctp_header_t * sctp_hdr, /* Check that the LOCALLY generated tag is being used by the REMOTE peer as the verification tag */ if (sctp_conn->local_tag != sctp_hdr->verification_tag) { + *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4); return SCTP_ERROR_INVALID_TAG; } @@ -1517,11 +1543,7 @@ sctp_handle_sack (sctp_selective_ack_chunk_t * sack_chunk, /* Check that the LOCALLY generated tag is being used by the REMOTE peer as the verification tag */ if (sctp_conn->local_tag != sack_chunk->sctp_hdr.verification_tag) { - SCTP_ADV_DBG - ("sctp_conn->local_tag != sack_chunk->sctp_hdr.verification_tag"); - *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4); - return SCTP_ERROR_INVALID_TAG; } @@ -1560,6 +1582,7 @@ sctp_handle_heartbeat (sctp_hb_req_chunk_t * sctp_hb_chunk, /* Check that the LOCALLY generated tag is being used by the REMOTE peer as the verification tag */ if (sctp_conn->local_tag != sctp_hb_chunk->sctp_hdr.verification_tag) { + *next0 = sctp_next_drop (sctp_conn->sub_conn[idx].c_is_ip4); return SCTP_ERROR_INVALID_TAG; } diff --git a/src/vnet/sctp/sctp_output.c b/src/vnet/sctp/sctp_output.c index 2b65d97e093..aa0bb41e504 100644 --- a/src/vnet/sctp/sctp_output.c +++ b/src/vnet/sctp/sctp_output.c @@ -1367,6 +1367,7 @@ sctp_push_hdr_i (sctp_connection_t * sctp_conn, vlib_buffer_t * b, { u16 data_len = b->current_length + b->total_length_not_including_first_buffer; + ASSERT (!b->total_length_not_including_first_buffer || (b->flags & VLIB_BUFFER_NEXT_PRESENT)); @@ -1375,6 +1376,13 @@ sctp_push_hdr_i (sctp_connection_t * sctp_conn, vlib_buffer_t * b, "data_len = %u", b->current_length, b->current_data, data_len); + u16 data_padding = vnet_sctp_calculate_padding (b->current_length); + if (data_padding > 0) + { + u8 *p_tail = vlib_buffer_put_uninit (b, data_padding); + clib_memset_u8 (p_tail, 0, data_padding); + } + u16 bytes_to_add = sizeof (sctp_payload_data_chunk_t); u16 chunk_length = data_len + bytes_to_add - sizeof (sctp_header_t); diff --git a/test/test_sctp.py b/test/test_sctp.py index 4d04aa06828..e4f0bd97416 100644 --- a/test/test_sctp.py +++ b/test/test_sctp.py @@ -72,6 +72,7 @@ class TestSCTP(VppTestCase): " appns 1" + " fifo-size 4" + " no-output test-bytes syn-timeout 3" + + " test-timeout 30" + " uri " + uri) if error: self.logger.critical(error) -- 2.16.6