Fragmented ordered user messages with inconsistent stream sequence numbers are accepted and delivere

DISCUSSION Julian Cordes 3 participants 7 messages Jan 17, 2017 – Jan 19, 2017

Subsystems (2)

Networking SCTP protocol

lore

Share & tools

lore

Julian Cordes

author

2017-01-17 13:13 UTC #0 lore raw

When fragmented ordered user messages are sent to the linux kernel user messages with inconsistent stream sequence numbers are accepted and delivered to the application. Take for example an look at the following test case:
https://github.com/nplab/PR_SCTP_Testsuite/blob/master/forward-tsn/error-cases/packet-loss/ordered/ordered-packet-loss-2.pkt

Here I first inject two DATA-Chunks with sid/ssn 1/1, 2/1 into the kernel with packetdrill:
line 67: +0.0 < sctp: DATA[flgs=E, len=1016, tsn=2, sid=1, ssn=1, ppid=0]
line 69: +0.1 < sctp: DATA[flgs=E, len=1016, tsn=4, sid=2, ssn=1, ppid=0]

After 0.1 seconds i then inject an FORWARD-TSN with cum_tsn=2 bundled with an DATA-Chunk that looks like this:
DATA[flgs=B, len=1016, tsn=3, sid=2, ssn=0, ppid=0]

This "looks" like the first segment of the user message where already a fragment (tsn=4) was received. But the ssn-values are not consistent. The DATA-Chunk with tsn=3 has ssn=0 and the ssn=1 therefore they should be ignored by the IUT and not be delivered to the userland application when calling sctp_recvmsg. Unfortunatly the linux kernel implementation does currently deliver these inconsistent user messages.

There are many test-cases that all reproduce this
issue. Just take a look at the README at
https://github.com/nplab/PR_SCTP_Testsuite/tree/master/forward-tsn/error-cases/packet-loss/ordered.

Xin Long

maintainer

2017-01-17 18:49 UTC #1 lore raw

Re: Fragmented ordered user messages with inconsistent stream sequence numbers are accepted and deli

Quoted 16 lines · Julian Cordes

When fragmented ordered user messages are sent to the linux kernel user messages with inconsistent stream sequence numbers are accepted and delivered to the application. Take for example an look at the following test case:
https://github.com/nplab/PR_SCTP_Testsuite/blob/master/forward-tsn/error-cases/packet-loss/ordered/ordered-packet-loss-2.pkt
Here I first inject two DATA-Chunks with sid/ssn 1/1, 2/1 into the kernel with packetdrill:
line 67: +0.0 < sctp: DATA[flgs=E, len16, tsn=2, sid=1, ssn=1, ppid=0]
line 69: +0.1 < sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0]
After 0.1 seconds i then inject an FORWARD-TSN with cum_tsn=2 bundled with an DATA-Chunk that looks like this:
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]

Quoted 6 lines

This "looks" like the first segment of the user message where already a fragment (tsn=4) was received. But the ssn-values are not consistent. The DATA-Chunk with tsn=3 has ssn=0 and the ssn=1 therefore they should be ignored by the IUT and not be delivered to the userland application when calling sctp_recvmsg. Unfortunatly the linux kernel implementation does currently deliver these inconsistent user messages.

sorry, I may misunderstand you.

after FORWARD-TSN, "DATA[flgs=E, len16, tsn=2, sid=1, ssn=1" should be received, then only "sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1" is in the queue, then "DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]" is processed, these two chunks (tsn=3 and tsn=4) should be recvmsg, as their sid are the same and ssn are consistent (ssn=0 and ssn=1).
and your sctp_recvmsg is after above, so it should be right.

did I miss something?

Quoted 4 lines

There are many test-cases that all reproduce this
issue. Just take a look at the README at
https://github.com/nplab/PR_SCTP_Testsuite/tree/master/forward-tsn/error-cases/packet-loss/ordered.

Julian Cordes

author

2017-01-17 18:57 UTC #2 lore raw

Re: Fragmented ordered user messages with inconsistent stream sequence numbers are accepted and deli

In reply to Julian Cordes

2017-01-17 19:49 GMT+01:00 Xin Long <[email protected]>:

Quoted 33 lines · Julian Cordes

Quoted 16 lines
When fragmented ordered user messages are sent to the linux kernel user messages with inconsistent stream sequence numbers are accepted and delivered to the application. Take for example an look at the following test case:
https://github.com/nplab/PR_SCTP_Testsuite/blob/master/forward-tsn/error-cases/packet-loss/ordered/ordered-packet-loss-2.pkt
Here I first inject two DATA-Chunks with sid/ssn 1/1, 2/1 into the kernel with packetdrill:
line 67: +0.0 < sctp: DATA[flgs=E, len16, tsn=2, sid=1, ssn=1, ppid=0]
line 69: +0.1 < sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0]
After 0.1 seconds i then inject an FORWARD-TSN with cum_tsn=2 bundled with an DATA-Chunk that looks like this:
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]
Quoted 6 lines
This "looks" like the first segment of the user message where already a fragment (tsn=4) was received. But the ssn-values are not consistent. The DATA-Chunk with tsn=3 has ssn=0 and the ssn=1 therefore they should be ignored by the IUT and not be delivered to the userland application when calling sctp_recvmsg. Unfortunatly the linux kernel implementation does currently deliver these inconsistent user messages.
sorry, I may misunderstand you.
after FORWARD-TSN, "DATA[flgs=E, len16, tsn=2, sid=1, ssn=1" should be received, then only "sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1" is in the queue, then "DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]" is processed, these two chunks (tsn=3 and tsn=4) should be recvmsg, as their sid are the same and ssn are consistent (ssn=0 and ssn=1).
and your sctp_recvmsg is after above, so it should be right.

Quoted 3 lines

did I miss something?

Shouldnt the ssn be 0 for both DATA-Chunks with sid=2, because they consistent to the same first fragmented user message of stream with sid=2?

Quoted 4 lines

Quoted 4 lines
There are many test-cases that all reproduce this
issue. Just take a look at the README at
https://github.com/nplab/PR_SCTP_Testsuite/tree/master/forward-tsn/error-cases/packet-loss/ordered.

Xin Long

maintainer

2017-01-17 19:15 UTC #3 lore raw

Re: Fragmented ordered user messages with inconsistent stream sequence numbers are accepted and deli

In reply to Julian Cordes

Quoted 34 lines · Julian Cordes

2017-01-17 19:49 GMT+01:00 Xin Long <[email protected]>:
Quoted 33 lines · Julian Cordes
Quoted 16 lines
When fragmented ordered user messages are sent to the linux kernel user messages with inconsistent stream sequence numbers are accepted and delivered to the application. Take for example an look at the following test case:
https://github.com/nplab/PR_SCTP_Testsuite/blob/master/forward-tsn/error-cases/packet-loss/ordered/ordered-packet-loss-2.pkt
Here I first inject two DATA-Chunks with sid/ssn 1/1, 2/1 into the kernel with packetdrill:
line 67: +0.0 < sctp: DATA[flgs=E, len16, tsn=2, sid=1, ssn=1, ppid=0]
line 69: +0.1 < sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0]
After 0.1 seconds i then inject an FORWARD-TSN with cum_tsn=2 bundled with an DATA-Chunk that looks like this:
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]
Quoted 6 lines
This "looks" like the first segment of the user message where already a fragment (tsn=4) was received. But the ssn-values are not consistent. The DATA-Chunk with tsn=3 has ssn=0 and the ssn=1 therefore they should be ignored by the IUT and not be delivered to the userland application when calling sctp_recvmsg. Unfortunatly the linux kernel implementation does currently deliver these inconsistent user messages.
sorry, I may misunderstand you.
after FORWARD-TSN, "DATA[flgs=E, len16, tsn=2, sid=1, ssn=1" should be received, then only "sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1" is in the queue, then "DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]" is processed, these two chunks (tsn=3 and tsn=4) should be recvmsg, as their sid are the same and ssn are consistent (ssn=0 and ssn=1).
and your sctp_recvmsg is after above, so it should be right.

Quoted 6 lines

Quoted 3 lines
did I miss something?
Shouldnt the ssn be 0 for both DATA-Chunks with sid=2, because they consistent to the same first fragmented user message of stream with sid=2?

ahh, got you now, will look into it tomorrow, thanks.

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
There are many test-cases that all reproduce this
issue. Just take a look at the README at
https://github.com/nplab/PR_SCTP_Testsuite/tree/master/forward-tsn/error-cases/packet-loss/ordered.

Xin Long

maintainer

2017-01-18 07:08 UTC #4 lore raw

Re: Fragmented ordered user messages with inconsistent stream sequence numbers are accepted and deli

In reply to Julian Cordes

Quoted 35 lines · Xin Long

Quoted 34 lines · Julian Cordes
2017-01-17 19:49 GMT+01:00 Xin Long <[email protected]>:
Quoted 33 lines · Julian Cordes
Quoted 16 lines
When fragmented ordered user messages are sent to the linux kernel user messages with inconsistent stream sequence numbers are accepted and delivered to the application. Take for example an look at the following test case:
https://github.com/nplab/PR_SCTP_Testsuite/blob/master/forward-tsn/error-cases/packet-loss/ordered/ordered-packet-loss-2.pkt
Here I first inject two DATA-Chunks with sid/ssn 1/1, 2/1 into the kernel with packetdrill:
line 67: +0.0 < sctp: DATA[flgs=E, len16, tsn=2, sid=1, ssn=1, ppid=0]
line 69: +0.1 < sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0]
After 0.1 seconds i then inject an FORWARD-TSN with cum_tsn=2 bundled with an DATA-Chunk that looks like this:
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]
Quoted 6 lines
This "looks" like the first segment of the user message where already a fragment (tsn=4) was received. But the ssn-values are not consistent. The DATA-Chunk with tsn=3 has ssn=0 and the ssn=1 therefore they should be ignored by the IUT and not be delivered to the userland application when calling sctp_recvmsg. Unfortunatly the linux kernel implementation does currently deliver these inconsistent user messages.
sorry, I may misunderstand you.
after FORWARD-TSN, "DATA[flgs=E, len16, tsn=2, sid=1, ssn=1" should be received, then only "sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1" is in the queue, then "DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]" is processed, these two chunks (tsn=3 and tsn=4) should be recvmsg, as their sid are the same and ssn are consistent (ssn=0 and ssn=1).
and your sctp_recvmsg is after above, so it should be right.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
did I miss something?
Shouldnt the ssn be 0 for both DATA-Chunks with sid=2, because they consistent to the same first fragmented user message of stream with sid=2?
ahh, got you now, will look into it tomorrow, thanks.

linux sctp now doesn't yet support "interleave", it believes the chunks / fragmenteds from the same msg have continuous / consistent TSNs. That's why it can ignore the ssn and only care about tsn and flag when reasm fragmenteds.

we will improve it when implementing "interleave" soon, thanks for reporting.

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
There are many test-cases that all reproduce this
issue. Just take a look at the README at
https://github.com/nplab/PR_SCTP_Testsuite/tree/master/forward-tsn/error-cases/packet-loss/ordered.

Marcelo Ricardo Leitner

maintainer

2017-01-18 21:49 UTC #5 lore raw

Re: Fragmented ordered user messages with inconsistent stream sequence numbers are accepted and deli

In reply to Julian Cordes

Quoted 42 lines · Xin Long

Quoted 41 lines · Xin Long
Quoted 40 lines · Julian Cordes
2017-01-17 19:49 GMT+01:00 Xin Long <[email protected]>:
Quoted 33 lines · Julian Cordes
Quoted 16 lines
When fragmented ordered user messages are sent to the linux kernel user messages with inconsistent stream sequence numbers are accepted and delivered to the application. Take for example an look at the following test case:
https://github.com/nplab/PR_SCTP_Testsuite/blob/master/forward-tsn/error-cases/packet-loss/ordered/ordered-packet-loss-2.pkt
Here I first inject two DATA-Chunks with sid/ssn 1/1, 2/1 into the kernel with packetdrill:
line 67: +0.0 < sctp: DATA[flgs=E, len16, tsn=2, sid=1, ssn=1, ppid=0]
line 69: +0.1 < sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0]
After 0.1 seconds i then inject an FORWARD-TSN with cum_tsn=2 bundled with an DATA-Chunk that looks like this:
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]
Quoted 6 lines
This "looks" like the first segment of the user message where already a fragment (tsn=4) was received. But the ssn-values are not consistent. The DATA-Chunk with tsn=3 has ssn=0 and the ssn=1 therefore they should be ignored by the IUT and not be delivered to the userland application when calling sctp_recvmsg. Unfortunatly the linux kernel implementation does currently deliver these inconsistent user messages.
sorry, I may misunderstand you.
after FORWARD-TSN, "DATA[flgs=E, len16, tsn=2, sid=1, ssn=1" should be received, then only "sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1" is in the queue, then "DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]" is processed, these two chunks (tsn=3 and tsn=4) should be recvmsg, as their sid are the same and ssn are consistent (ssn=0 and ssn=1).
and your sctp_recvmsg is after above, so it should be right.
Quoted 3 lines
did I miss something?
Shouldnt the ssn be 0 for both DATA-Chunks with sid=2, because they consistent to the same first fragmented user message of stream with sid=2?

Quoted 6 lines

Quoted 2 lines
ahh, got you now, will look into it tomorrow, thanks.
linux sctp now doesn't yet support "interleave", it believes the chunks / fragmenteds from the same msg have continuous / consistent TSNs. That's why it can ignore the ssn and only care about tsn and flag when reasm fragmenteds.

Interleaving is a feature, but what is happening here is that we are not validating the ssn in this case, and it allows an attacker to corrupt the messages once necessary knowledge is obtained (such as knowing vtag).

line 67: +0.0 < sctp: DATA[flgs=E, len16, tsn=2, sid=1, ssn=1, ppid=0] line 69: +0.1 < sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0] After 0.1 seconds i then inject an FORWARD-TSN with cum_tsn=2 bundled with an DATA-Chunk that looks like this:
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]

Simplifying, looks like:

                                         v----------.
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0] |
DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0] |
                                         ^----------+-- doesn't match

https://tools.ietf.org/html/rfc4960#section-6.9
2) The transmitter MUST then assign, in sequence, a separate TSN to each of the DATA chunks in the series. The transmitter assigns the same SSN to each of the DATA chunks. ....

A draft, just to illustrate an initial fix for this:

net/sctp/ulpqueue.c +6 -2

index aa3624d50278..273abcf631ef 100644
@@ -419,6 +419,7 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_reassembled(struct sctp_ulpq *ul
 	size_t pd_len = 0;
 	struct sctp_association *asoc;
 	u32 pd_point;
+	u16 cssn;
 
 	/* Initialized to 0 just to avoid compiler warning message.  Will
 	 * never be used with this value. It is referenced only after it

@@ -461,10 +462,12 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_reassembled(struct sctp_ulpq *ul
 
 			first_frag = pos;
 			next_tsn = ctsn + 1;
+			cssn = cevent->ssn;
 			break;
 
 		case SCTP_DATA_MIDDLE_FRAG:
-			if ((first_frag) && (ctsn = next_tsn)) {
+			if (first_frag && ctsn = next_tsn &&
+			    cssn = cevent->ssn) {
 				next_tsn++;
 				if (pd_first) {
 				    pd_last = pos;

@@ -475,7 +478,8 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_reassembled(struct sctp_ulpq *ul
 			break;
 
 		case SCTP_DATA_LAST_FRAG:
-			if (first_frag && (ctsn = next_tsn))
+			if (first_frag && ctsn = next_tsn &&
+			    cssn = cevent->ssn)
 				goto found;
 			else
 				first_frag = NULL;

Though the real fix is more complicated than this because in such case the fragments have to be discarded and this is not going to happen with the above.

Quoted 12 lines

we will improve it when implementing "interleave" soon, thanks for reporting.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
There are many test-cases that all reproduce this
issue. Just take a look at the README at
https://github.com/nplab/PR_SCTP_Testsuite/tree/master/forward-tsn/error-cases/packet-loss/ordered.
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Xin Long

maintainer

2017-01-19 17:38 UTC #6 lore raw

Re: Fragmented ordered user messages with inconsistent stream sequence numbers are accepted and deli

In reply to Julian Cordes

Quoted 110 lines · Marcelo Ricardo Leitner

Quoted 42 lines · Xin Long
Quoted 41 lines · Xin Long
Quoted 40 lines · Julian Cordes
2017-01-17 19:49 GMT+01:00 Xin Long <[email protected]>:
Quoted 33 lines · Julian Cordes
Quoted 16 lines
When fragmented ordered user messages are sent to the linux kernel user messages with inconsistent stream sequence numbers are accepted and delivered to the application. Take for example an look at the following test case:
https://github.com/nplab/PR_SCTP_Testsuite/blob/master/forward-tsn/error-cases/packet-loss/ordered/ordered-packet-loss-2.pkt
Here I first inject two DATA-Chunks with sid/ssn 1/1, 2/1 into the kernel with packetdrill:
line 67: +0.0 < sctp: DATA[flgs=E, len16, tsn=2, sid=1, ssn=1, ppid=0]
line 69: +0.1 < sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0]
After 0.1 seconds i then inject an FORWARD-TSN with cum_tsn=2 bundled with an DATA-Chunk that looks like this:
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]
Quoted 6 lines
This "looks" like the first segment of the user message where already a fragment (tsn=4) was received. But the ssn-values are not consistent. The DATA-Chunk with tsn=3 has ssn=0 and the ssn=1 therefore they should be ignored by the IUT and not be delivered to the userland application when calling sctp_recvmsg. Unfortunatly the linux kernel implementation does currently deliver these inconsistent user messages.
sorry, I may misunderstand you.
after FORWARD-TSN, "DATA[flgs=E, len16, tsn=2, sid=1, ssn=1" should be received, then only "sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1" is in the queue, then "DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]" is processed, these two chunks (tsn=3 and tsn=4) should be recvmsg, as their sid are the same and ssn are consistent (ssn=0 and ssn=1).
and your sctp_recvmsg is after above, so it should be right.
Quoted 3 lines
did I miss something?
Shouldnt the ssn be 0 for both DATA-Chunks with sid=2, because they consistent to the same first fragmented user message of stream with sid=2?
Quoted 6 lines
Quoted 2 lines
ahh, got you now, will look into it tomorrow, thanks.
linux sctp now doesn't yet support "interleave", it believes the chunks / fragmenteds from the same msg have continuous / consistent TSNs. That's why it can ignore the ssn and only care about tsn and flag when reasm fragmenteds.
Interleaving is a feature, but what is happening here is that we are not validating the ssn in this case, and it allows an attacker to corrupt the messages once necessary knowledge is obtained (such as knowing vtag).
line 67: +0.0 < sctp: DATA[flgs=E, len16, tsn=2, sid=1, ssn=1, ppid=0] line 69: +0.1 < sctp: DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0] After 0.1 seconds i then inject an FORWARD-TSN with cum_tsn=2 bundled with an DATA-Chunk that looks like this:
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0]
Simplifying, looks like:
                                         v----------.
DATA[flgs=B, len16, tsn=3, sid=2, ssn=0, ppid=0] |
DATA[flgs=E, len16, tsn=4, sid=2, ssn=1, ppid=0] |
                                         ^----------+-- doesn't match
https://tools.ietf.org/html/rfc4960#section-6.9
2) The transmitter MUST then assign, in sequence, a separate TSN to each of the DATA chunks in the series. The transmitter assigns the same SSN to each of the DATA chunks. ....
A draft, just to illustrate an initial fix for this:
net/sctp/ulpqueue.c +6 -2
index aa3624d50278..273abcf631ef 100644
@@ -419,6 +419,7 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_reassembled(struct sctp_ulpq *ul
        size_t pd_len = 0;
        struct sctp_association *asoc;
        u32 pd_point;
+       u16 cssn;
 
        /* Initialized to 0 just to avoid compiler warning message.  Will
         * never be used with this value. It is referenced only after it
@@ -461,10 +462,12 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_reassembled(struct sctp_ulpq *ul
 
                        first_frag = pos;
                        next_tsn = ctsn + 1;
+                       cssn = cevent->ssn;
                        break;
 
                case SCTP_DATA_MIDDLE_FRAG:
-                       if ((first_frag) && (ctsn = next_tsn)) {
+                       if (first_frag && ctsn = next_tsn &&
+                           cssn = cevent->ssn) {
                                next_tsn++;
                                if (pd_first) {
                                    pd_last = pos;
@@ -475,7 +478,8 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_reassembled(struct sctp_ulpq *ul
                        break;
 
                case SCTP_DATA_LAST_FRAG:
-                       if (first_frag && (ctsn = next_tsn))
+                       if (first_frag && ctsn = next_tsn &&
+                           cssn = cevent->ssn)
                                goto found;
                        else
                                first_frag = NULL;

Quoted 3 lines

Though the real fix is more complicated than this because in such case the fragments have to be discarded and this is not going to happen with the above.

I agree with that it should be discarded.

also because only with the fix above, when the correct chunk comes, it will report DUP, instead of delivering to ULP, as it has the same tsn with the abnormal chunk.

Quoted 13 lines

Quoted 13 lines
we will improve it when implementing "interleave" soon, thanks for reporting.
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
There are many test-cases that all reproduce this
issue. Just take a look at the README at
https://github.com/nplab/PR_SCTP_Testsuite/tree/master/forward-tsn/error-cases/packet-loss/ordered.
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html