netfilter: conntrack: unify established states for SCTP paths

commit a44b7651489f26271ac784b70895e8a85d0cebf4 upstream.

An SCTP endpoint can start an association through a path and tear it
down over another one. That means the initial path will not see the
shutdown sequence, and the conntrack entry will remain in ESTABLISHED
state for 5 days.

By merging the HEARTBEAT_ACKED and ESTABLISHED states into one
ESTABLISHED state, there remains no difference between a primary or
secondary path. The timeout for the merged ESTABLISHED state is set to
210 seconds (hb_interval * max_path_retrans + rto_max). So, even if a
path doesn't see the shutdown sequence, it will expire in a reasonable
amount of time.

With this change in place, there is now more than one state from which
we can transition to ESTABLISHED, COOKIE_ECHOED and HEARTBEAT_SENT, so
handle the setting of ASSURED bit whenever a state change has happened
and the new state is ESTABLISHED. Removed the check for dir==REPLY since
the transition to ESTABLISHED can happen only in the reply direction.

Fixes: b0bbff62ec62 ("[NETFILTER]: Add nf_conntrack subsystem.")
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/uapi/linux/netfilter/nf_conntrack_sctp.h b/include/uapi/linux/netfilter/nf_conntrack_sctp.h
index edc6ddab0de6afcf7d5cf7284087e959c17d2ad4..2d6f80d75ae74c4eaa55809bf96ddcd5edd6c2bf 100644 (file)
--- a/include/uapi/linux/netfilter/nf_conntrack_sctp.h
+++ b/include/uapi/linux/netfilter/nf_conntrack_sctp.h
@@ -15,7 +15,7 @@ enum sctp_conntrack {
        SCTP_CONNTRACK_SHUTDOWN_RECD,
        SCTP_CONNTRACK_SHUTDOWN_ACK_SENT,
        SCTP_CONNTRACK_HEARTBEAT_SENT,
-       SCTP_CONNTRACK_HEARTBEAT_ACKED,
+       SCTP_CONNTRACK_HEARTBEAT_ACKED, /* no longer used */
        SCTP_CONNTRACK_MAX
 };
 

diff --git a/include/uapi/linux/netfilter/nfnetlink_cttimeout.h b/include/uapi/linux/netfilter/nfnetlink_cttimeout.h
index 6b20fb22717b249bcea8c354ef997ca0e04564b4..aa805e6d4e284d63ae43330e571453cacfc18acd 100644 (file)
--- a/include/uapi/linux/netfilter/nfnetlink_cttimeout.h
+++ b/include/uapi/linux/netfilter/nfnetlink_cttimeout.h
@@ -94,7 +94,7 @@ enum ctattr_timeout_sctp {
        CTA_TIMEOUT_SCTP_SHUTDOWN_RECD,
        CTA_TIMEOUT_SCTP_SHUTDOWN_ACK_SENT,
        CTA_TIMEOUT_SCTP_HEARTBEAT_SENT,
-       CTA_TIMEOUT_SCTP_HEARTBEAT_ACKED,
+       CTA_TIMEOUT_SCTP_HEARTBEAT_ACKED, /* no longer used */
        __CTA_TIMEOUT_SCTP_MAX
 };
 #define CTA_TIMEOUT_SCTP_MAX (__CTA_TIMEOUT_SCTP_MAX - 1)

diff --git a/net/netfilter/nf_conntrack_proto_sctp.c b/net/netfilter/nf_conntrack_proto_sctp.c
index 72d0aa603cd61d44151e1a595150dc3cc681e5df..cec4b16170a0bc7503f7438e50e6259d6abfb853 100644 (file)
--- a/net/netfilter/nf_conntrack_proto_sctp.c
+++ b/net/netfilter/nf_conntrack_proto_sctp.c
@@ -27,22 +27,16 @@
 #include <net/netfilter/nf_conntrack_ecache.h>
 #include <net/netfilter/nf_conntrack_timeout.h>
 
-/* FIXME: Examine ipfilter's timeouts and conntrack transitions more
-   closely.  They're more complex. --RR
-
-   And so for me for SCTP :D -Kiran */
-
 static const char *const sctp_conntrack_names[] = {
-       "NONE",
-       "CLOSED",
-       "COOKIE_WAIT",
-       "COOKIE_ECHOED",
-       "ESTABLISHED",
-       "SHUTDOWN_SENT",
-       "SHUTDOWN_RECD",
-       "SHUTDOWN_ACK_SENT",
-       "HEARTBEAT_SENT",
-       "HEARTBEAT_ACKED",
+       [SCTP_CONNTRACK_NONE]                   = "NONE",
+       [SCTP_CONNTRACK_CLOSED]                 = "CLOSED",
+       [SCTP_CONNTRACK_COOKIE_WAIT]            = "COOKIE_WAIT",
+       [SCTP_CONNTRACK_COOKIE_ECHOED]          = "COOKIE_ECHOED",
+       [SCTP_CONNTRACK_ESTABLISHED]            = "ESTABLISHED",
+       [SCTP_CONNTRACK_SHUTDOWN_SENT]          = "SHUTDOWN_SENT",
+       [SCTP_CONNTRACK_SHUTDOWN_RECD]          = "SHUTDOWN_RECD",
+       [SCTP_CONNTRACK_SHUTDOWN_ACK_SENT]      = "SHUTDOWN_ACK_SENT",
+       [SCTP_CONNTRACK_HEARTBEAT_SENT]         = "HEARTBEAT_SENT",
 };
 
 #define SECS  * HZ
@@ -54,12 +48,11 @@ static const unsigned int sctp_timeouts[SCTP_CONNTRACK_MAX] = {
        [SCTP_CONNTRACK_CLOSED]                 = 10 SECS,
        [SCTP_CONNTRACK_COOKIE_WAIT]            = 3 SECS,
        [SCTP_CONNTRACK_COOKIE_ECHOED]          = 3 SECS,
-       [SCTP_CONNTRACK_ESTABLISHED]            = 5 DAYS,
+       [SCTP_CONNTRACK_ESTABLISHED]            = 210 SECS,
        [SCTP_CONNTRACK_SHUTDOWN_SENT]          = 300 SECS / 1000,
        [SCTP_CONNTRACK_SHUTDOWN_RECD]          = 300 SECS / 1000,
        [SCTP_CONNTRACK_SHUTDOWN_ACK_SENT]      = 3 SECS,
        [SCTP_CONNTRACK_HEARTBEAT_SENT]         = 30 SECS,
-       [SCTP_CONNTRACK_HEARTBEAT_ACKED]        = 210 SECS,
 };
 
 #define        SCTP_FLAG_HEARTBEAT_VTAG_FAILED 1
@@ -73,7 +66,6 @@ static const unsigned int sctp_timeouts[SCTP_CONNTRACK_MAX] = {
 #define        sSR SCTP_CONNTRACK_SHUTDOWN_RECD
 #define        sSA SCTP_CONNTRACK_SHUTDOWN_ACK_SENT
 #define        sHS SCTP_CONNTRACK_HEARTBEAT_SENT
-#define        sHA SCTP_CONNTRACK_HEARTBEAT_ACKED
 #define        sIV SCTP_CONNTRACK_MAX
 
 /*
@@ -96,9 +88,6 @@ SHUTDOWN_ACK_SENT - We have seen a SHUTDOWN_ACK chunk in the direction opposite
 CLOSED            - We have seen a SHUTDOWN_COMPLETE chunk in the direction of
                    the SHUTDOWN chunk. Connection is closed.
 HEARTBEAT_SENT    - We have seen a HEARTBEAT in a new flow.
-HEARTBEAT_ACKED   - We have seen a HEARTBEAT-ACK in the direction opposite to
-                   that of the HEARTBEAT chunk. Secondary connection is
-                   established.
 */
 
 /* TODO
@@ -115,33 +104,33 @@ cookie echoed to closed.
 static const u8 sctp_conntracks[2][11][SCTP_CONNTRACK_MAX] = {
        {
 /*     ORIGINAL        */
-/*                  sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, 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},
-/* shutdown_ack */ {sSA, sCL, sCW, sCE, sES, sSA, sSA, sSA, sSA, sHA},
-/* error        */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL, sHA},/* Can't have Stale cookie*/
-/* cookie_echo  */ {sCL, sCL, sCE, sCE, sES, sSS, sSR, sSA, sCL, sHA},/* 5.2.4 - Big TODO */
-/* cookie_ack   */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL, sHA},/* Can't come in orig dir */
-/* shutdown_comp*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sCL, sCL, sHA},
-/* heartbeat    */ {sHS, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA},
-/* heartbeat_ack*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA}
+/*                  sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS */
+/* init         */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCW},
+/* init_ack     */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL},
+/* abort        */ {sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sCL},
+/* shutdown     */ {sCL, sCL, sCW, sCE, sSS, sSS, sSR, sSA, sCL},
+/* shutdown_ack */ {sSA, sCL, sCW, sCE, sES, sSA, sSA, sSA, sSA},
+/* error        */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL},/* Can't have Stale cookie*/
+/* cookie_echo  */ {sCL, sCL, sCE, sCE, sES, sSS, sSR, sSA, sCL},/* 5.2.4 - Big TODO */
+/* cookie_ack   */ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sCL},/* Can't come in orig dir */
+/* shutdown_comp*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sCL, sCL},
+/* heartbeat    */ {sHS, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS},
+/* heartbeat_ack*/ {sCL, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS},
        },
        {
 /*     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, 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},
-/* error        */ {sIV, sCL, sCW, sCL, sES, sSS, sSR, sSA, sIV, sHA},
-/* cookie_echo  */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV, sHA},/* Can't come in reply dir */
-/* cookie_ack   */ {sIV, sCL, sCW, sES, sES, sSS, sSR, sSA, sIV, sHA},
-/* shutdown_comp*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sCL, sIV, sHA},
-/* heartbeat    */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS, sHA},
-/* heartbeat_ack*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHA, sHA}
+/*                  sNO, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS */
+/* init         */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV},/* INIT in sCL Big TODO */
+/* init_ack     */ {sIV, sCW, sCW, sCE, sES, sSS, sSR, sSA, sIV},
+/* abort        */ {sIV, sCL, sCL, sCL, sCL, sCL, sCL, sCL, sIV},
+/* shutdown     */ {sIV, sCL, sCW, sCE, sSR, sSS, sSR, sSA, sIV},
+/* shutdown_ack */ {sIV, sCL, sCW, sCE, sES, sSA, sSA, sSA, sIV},
+/* error        */ {sIV, sCL, sCW, sCL, sES, sSS, sSR, sSA, sIV},
+/* cookie_echo  */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sIV},/* Can't come in reply dir */
+/* cookie_ack   */ {sIV, sCL, sCW, sES, sES, sSS, sSR, sSA, sIV},
+/* shutdown_comp*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sCL, sIV},
+/* heartbeat    */ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sHS},
+/* heartbeat_ack*/ {sIV, sCL, sCW, sCE, sES, sSS, sSR, sSA, sES},
        }
 };
 
@@ -508,8 +497,12 @@ int nf_conntrack_sctp_packet(struct nf_conn *ct,
                }
 
                ct->proto.sctp.state = new_state;
-               if (old_state != new_state)
+               if (old_state != new_state) {
                        nf_conntrack_event_cache(IPCT_PROTOINFO, ct);
+                       if (new_state == SCTP_CONNTRACK_ESTABLISHED &&
+                           !test_and_set_bit(IPS_ASSURED_BIT, &ct->status))
+                               nf_conntrack_event_cache(IPCT_ASSURED, ct);
+               }
        }
        spin_unlock_bh(&ct->lock);
 
@@ -523,14 +516,6 @@ int nf_conntrack_sctp_packet(struct nf_conn *ct,
 
        nf_ct_refresh_acct(ct, ctinfo, skb, timeouts[new_state]);
 
-       if (old_state == SCTP_CONNTRACK_COOKIE_ECHOED &&
-           dir == IP_CT_DIR_REPLY &&
-           new_state == SCTP_CONNTRACK_ESTABLISHED) {
-               pr_debug("Setting assured bit\n");
-               set_bit(IPS_ASSURED_BIT, &ct->status);
-               nf_conntrack_event_cache(IPCT_ASSURED, ct);
-       }
-
        return NF_ACCEPT;
 
 out_unlock:

diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c
index a3faeacaa1cbbf9f478406c753d063f72e0d0631..43c3c3be6defc439872800c5090359c6195434f5 100644 (file)
--- a/net/netfilter/nf_conntrack_standalone.c
+++ b/net/netfilter/nf_conntrack_standalone.c
@@ -581,7 +581,6 @@ enum nf_ct_sysctl_index {
        NF_SYSCTL_CT_PROTO_TIMEOUT_SCTP_SHUTDOWN_RECD,
        NF_SYSCTL_CT_PROTO_TIMEOUT_SCTP_SHUTDOWN_ACK_SENT,
        NF_SYSCTL_CT_PROTO_TIMEOUT_SCTP_HEARTBEAT_SENT,
-       NF_SYSCTL_CT_PROTO_TIMEOUT_SCTP_HEARTBEAT_ACKED,
 #endif
 #ifdef CONFIG_NF_CT_PROTO_DCCP
        NF_SYSCTL_CT_PROTO_TIMEOUT_DCCP_REQUEST,
@@ -851,12 +850,6 @@ static struct ctl_table nf_ct_sysctl_table[] = {
                .mode           = 0644,
                .proc_handler   = proc_dointvec_jiffies,
        },
-       [NF_SYSCTL_CT_PROTO_TIMEOUT_SCTP_HEARTBEAT_ACKED] = {
-               .procname       = "nf_conntrack_sctp_timeout_heartbeat_acked",
-               .maxlen         = sizeof(unsigned int),
-               .mode           = 0644,
-               .proc_handler   = proc_dointvec_jiffies,
-       },
 #endif
 #ifdef CONFIG_NF_CT_PROTO_DCCP
        [NF_SYSCTL_CT_PROTO_TIMEOUT_DCCP_REQUEST] = {
@@ -985,7 +978,6 @@ static void nf_conntrack_standalone_init_sctp_sysctl(struct net *net,
        XASSIGN(SHUTDOWN_RECD, sn);
        XASSIGN(SHUTDOWN_ACK_SENT, sn);
        XASSIGN(HEARTBEAT_SENT, sn);
-       XASSIGN(HEARTBEAT_ACKED, sn);
 #undef XASSIGN
 #endif
 }