[PATCH nf] netfilter: conntrack: sctp: use distinct states for new SCTP connections

[PATCH nf] netfilter: conntrack: sctp: use distinct states for new SCTP connections

[Jiri Wiesner]

The netlink notifications triggered by the INIT and INIT_ACK chunks
for a tracked SCTP association do not include protocol information
for the corresponding connection - SCTP state and verification tags
for the original and reply direction are missing. Since the connection
tracking implementation allows user space programs to receive
notifications about a connection and then create a new connection
based on the values received in a notification, it makes sense that
INIT and INIT_ACK notifications should contain the SCTP state
and verification tags available at the time when a notification
is sent. The missing verification tags cause a newly created
netfilter connection to fail to verify the tags of SCTP packets
when this connection has been created from the values previously
received in an INIT or INIT_ACK notification.

A PROTOINFO event is cached in sctp_packet() when the state
of a connection changes. The CLOSED and COOKIE_WAIT state will
be used for connections that have seen an INIT and INIT_ACK chunk,
respectively. The distinct states will cause a connection state
change in sctp_packet().

Signed-off-by: Jiri Wiesner <jwiesner@suse.com>
---
 net/netfilter/nf_conntrack_proto_sctp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 0399ae8f1188..4f897b14b606 100644
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -114,7 +114,7 @@ static const u8 sctp_conntracks[2][11][SCTP_CONNTRACK_MAX] = {
 	{
 /*	ORIGINAL	*/
 /*                  sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA */
-/* init         */ {sCW, sCW, sCW, sCE, sES, sSS, sSR, sSA, sCW, sHA},
+/* init         */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCW, sHA},
 /* init_ack     */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL, sHA},
 /* abort        */ {sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL},
 /* shutdown     */ {sCL, sCL, sCW, sCE, sSS, sSS, sSR, sSA, sCL, sSS},
@@ -130,7 +130,7 @@ static const u8 sctp_conntracks[2][11][SCTP_CONNTRACK_MAX] = {
 /*	REPLY	*/
 /*                  sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA */
 /* init         */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},/* INIT in sCL Big TODO */
-/* init_ack     */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},
+/* init_ack     */ {sIV, sCW, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},
 /* abort        */ {sIV, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sIV, sCL},
 /* shutdown     */ {sIV, sCL, sCW, sCE, sSR, sSS, sSR, sSA, sIV, sSR},
 /* shutdown_ack */ {sIV, sCL, sCW, sCE, sES, sSA, sSA, sSA, sIV, sHA},
@@ -316,7 +316,7 @@ sctp_new(struct nf_conn *ct, const struct sk_buff *skb,
 			ct->proto.sctp.vtag[IP_CT_DIR_REPLY] = sh->vtag;
 		}
 
-		ct->proto.sctp.state = new_state;
+		ct->proto.sctp.state = SCTP_CONNTRACK_NONE;
 	}
 
 	return true;
-- 
2.16.4

Re: [PATCH nf] netfilter: conntrack: sctp: use distinct states for new SCTP connections

[Pablo Neira Ayuso]

On Sat, Jan 18, 2020 at 01:10:50PM +0100, Jiri Wiesner wrote:
> The netlink notifications triggered by the INIT and INIT_ACK chunks
> for a tracked SCTP association do not include protocol information
> for the corresponding connection - SCTP state and verification tags
> for the original and reply direction are missing. Since the connection
> tracking implementation allows user space programs to receive
> notifications about a connection and then create a new connection
> based on the values received in a notification, it makes sense that
> INIT and INIT_ACK notifications should contain the SCTP state
> and verification tags available at the time when a notification
> is sent. The missing verification tags cause a newly created
> netfilter connection to fail to verify the tags of SCTP packets
> when this connection has been created from the values previously
> received in an INIT or INIT_ACK notification.
> 
> A PROTOINFO event is cached in sctp_packet() when the state
> of a connection changes. The CLOSED and COOKIE_WAIT state will
> be used for connections that have seen an INIT and INIT_ACK chunk,
> respectively. The distinct states will cause a connection state
> change in sctp_packet().

This problem shows through conntrack -E, correct?

Thanks.

Re: [PATCH nf] netfilter: conntrack: sctp: use distinct states for new SCTP connections

[Jiri Wiesner]

On 18/01/2020 21:39, Pablo Neira Ayuso wrote:
> On Sat, Jan 18, 2020 at 01:10:50PM +0100, Jiri Wiesner wrote:
>> The netlink notifications triggered by the INIT and INIT_ACK chunks
>> for a tracked SCTP association do not include protocol information
>> for the corresponding connection - SCTP state and verification tags
>> for the original and reply direction are missing. Since the connection
>> tracking implementation allows user space programs to receive
>> notifications about a connection and then create a new connection
>> based on the values received in a notification, it makes sense that
>> INIT and INIT_ACK notifications should contain the SCTP state
>> and verification tags available at the time when a notification
>> is sent. The missing verification tags cause a newly created
>> netfilter connection to fail to verify the tags of SCTP packets
>> when this connection has been created from the values previously
>> received in an INIT or INIT_ACK notification.
>>
>> A PROTOINFO event is cached in sctp_packet() when the state
>> of a connection changes. The CLOSED and COOKIE_WAIT state will
>> be used for connections that have seen an INIT and INIT_ACK chunk,
>> respectively. The distinct states will cause a connection state
>> change in sctp_packet().
> This problem shows through conntrack -E, correct?
>
> Thanks.
Yes, although "conntrack -E" does not display verification tags. These 
are the first 3 notifications of an association as printed by "conntrack 
-E" (output truncated after src=):
     [NEW] ipv4     2 sctp     132 3 src=
  [UPDATE] ipv4     2 sctp     132 3 src=
  [UPDATE] ipv4     2 sctp     132 3 COOKIE_ECHOED src=
As you see, there is no connection state printed in the first two 
notifications.

I used a custom tool which can print verification tags and formats its 
output similarly to "conntrack -E":
     [NEW] ipv4     2 sctp     132 3 0 0 src=
  [UPDATE] ipv4     2 sctp     132 3 0 0 src=
  [UPDATE] ipv4     2 sctp     132 3 COOKIE_ECHOED 50ced389 e967350e src=
The tags are printed as zero in the first two notifications, but that 
rather means the tags have not been received in the notification. The 
above test was done under Linux 5.5-rc4.

Re: [PATCH nf] netfilter: conntrack: sctp: use distinct states for new SCTP connections

[Pablo Neira Ayuso]

On Sat, Jan 18, 2020 at 01:10:50PM +0100, Jiri Wiesner wrote:
> The netlink notifications triggered by the INIT and INIT_ACK chunks
> for a tracked SCTP association do not include protocol information
> for the corresponding connection - SCTP state and verification tags
> for the original and reply direction are missing. Since the connection
> tracking implementation allows user space programs to receive
> notifications about a connection and then create a new connection
> based on the values received in a notification, it makes sense that
> INIT and INIT_ACK notifications should contain the SCTP state
> and verification tags available at the time when a notification
> is sent. The missing verification tags cause a newly created
> netfilter connection to fail to verify the tags of SCTP packets
> when this connection has been created from the values previously
> received in an INIT or INIT_ACK notification.
> 
> A PROTOINFO event is cached in sctp_packet() when the state
> of a connection changes. The CLOSED and COOKIE_WAIT state will
> be used for connections that have seen an INIT and INIT_ACK chunk,
> respectively. The distinct states will cause a connection state
> change in sctp_packet().

Applied, thanks for explaining.