On Sat, Jun 02, 2018 at 09:50:01PM +0300, Julian Anastasov wrote:
> cp->state was not used for templates. Add support for state bits
> and for the first "assured" bit which indicates that some
> connection controlled by this template was established or assured
> by the real server. In a followup patch we will use it to drop
> templates under SYN attack.
>
> Signed-off-by: Julian Anastasov <ja@xxxxxx>
> ---
> include/net/ip_vs.h | 7 ++++++-
> net/netfilter/ipvs/ip_vs_conn.c | 8 ++++----
> net/netfilter/ipvs/ip_vs_proto.c | 19 ++++++++++++++++---
> net/netfilter/ipvs/ip_vs_proto_sctp.c | 7 +++++++
> net/netfilter/ipvs/ip_vs_proto_tcp.c | 7 +++++++
> net/netfilter/ipvs/ip_vs_proto_udp.c | 7 +++++++
> net/netfilter/ipvs/ip_vs_sync.c | 18 ++++++++----------
> 7 files changed, 55 insertions(+), 18 deletions(-)
>
> diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
> index 824d7ef..d786649 100644
> --- a/include/net/ip_vs.h
> +++ b/include/net/ip_vs.h
> @@ -347,6 +347,11 @@ enum ip_vs_sctp_states {
> IP_VS_SCTP_S_LAST
> };
>
> +/* Connection templates use bits from state */
> +#define IP_VS_CTPL_S_NONE 0x0000
> +#define IP_VS_CTPL_S_ASSURED 0x0001
> +#define IP_VS_CTPL_S_LAST 0x0002
> +
> /* Delta sequence info structure
> * Each ip_vs_conn has 2 (output AND input seq. changes).
> * Only used in the VS/NAT.
> @@ -1232,7 +1237,7 @@ struct ip_vs_conn *ip_vs_conn_new(const struct ip_vs_conn_param *p, int dest_af,
> struct ip_vs_dest *dest, __u32 fwmark);
> void ip_vs_conn_expire_now(struct ip_vs_conn *cp);
>
> -const char *ip_vs_state_name(__u16 proto, int state);
> +const char *ip_vs_state_name(const struct ip_vs_conn *cp);
>
> void ip_vs_tcp_conn_listen(struct ip_vs_conn *cp);
> int ip_vs_check_template(struct ip_vs_conn *ct, struct ip_vs_dest *cdest);
> diff --git a/net/netfilter/ipvs/ip_vs_conn.c b/net/netfilter/ipvs/ip_vs_conn.c
> index 75de465..8f76644 100644
> --- a/net/netfilter/ipvs/ip_vs_conn.c
> +++ b/net/netfilter/ipvs/ip_vs_conn.c
> @@ -1107,7 +1107,7 @@ static int ip_vs_conn_seq_show(struct seq_file *seq, void *v)
> &cp->caddr.in6, ntohs(cp->cport),
> &cp->vaddr.in6, ntohs(cp->vport),
> dbuf, ntohs(cp->dport),
> - ip_vs_state_name(cp->protocol, cp->state),
> + ip_vs_state_name(cp),
> (cp->timer.expires-jiffies)/HZ, pe_data);
> else
> #endif
> @@ -1118,7 +1118,7 @@ static int ip_vs_conn_seq_show(struct seq_file *seq, void *v)
> ntohl(cp->caddr.ip), ntohs(cp->cport),
> ntohl(cp->vaddr.ip), ntohs(cp->vport),
> dbuf, ntohs(cp->dport),
> - ip_vs_state_name(cp->protocol, cp->state),
> + ip_vs_state_name(cp),
> (cp->timer.expires-jiffies)/HZ, pe_data);
> }
> return 0;
> @@ -1182,7 +1182,7 @@ static int ip_vs_conn_sync_seq_show(struct seq_file *seq, void *v)
> &cp->caddr.in6, ntohs(cp->cport),
> &cp->vaddr.in6, ntohs(cp->vport),
> dbuf, ntohs(cp->dport),
> - ip_vs_state_name(cp->protocol, cp->state),
> + ip_vs_state_name(cp),
> ip_vs_origin_name(cp->flags),
> (cp->timer.expires-jiffies)/HZ);
> else
> @@ -1194,7 +1194,7 @@ static int ip_vs_conn_sync_seq_show(struct seq_file *seq, void *v)
> ntohl(cp->caddr.ip), ntohs(cp->cport),
> ntohl(cp->vaddr.ip), ntohs(cp->vport),
> dbuf, ntohs(cp->dport),
> - ip_vs_state_name(cp->protocol, cp->state),
> + ip_vs_state_name(cp),
> ip_vs_origin_name(cp->flags),
> (cp->timer.expires-jiffies)/HZ);
> }
> diff --git a/net/netfilter/ipvs/ip_vs_proto.c b/net/netfilter/ipvs/ip_vs_proto.c
> index ca880a3..ae7d786 100644
> --- a/net/netfilter/ipvs/ip_vs_proto.c
> +++ b/net/netfilter/ipvs/ip_vs_proto.c
> @@ -42,6 +42,11 @@
>
> static struct ip_vs_protocol *ip_vs_proto_table[IP_VS_PROTO_TAB_SIZE];
>
> +/* States for conn templates: NONE or letters separated with "," */
> +static const char *ip_vs_ctpl_state_name_table[IP_VS_CTPL_S_LAST] = {
> + [IP_VS_CTPL_S_NONE] = "NONE",
> + [IP_VS_CTPL_S_ASSURED] = "A",
"A" seems a bit cryptic, why not "ASSURED" ?
> +};
>
> /*
> * register an ipvs protocol
> @@ -193,12 +198,20 @@ ip_vs_create_timeout_table(int *table, int size)
> }
>
>
> -const char * ip_vs_state_name(__u16 proto, int state)
> +const char *ip_vs_state_name(const struct ip_vs_conn *cp)
> {
> - struct ip_vs_protocol *pp = ip_vs_proto_get(proto);
> + unsigned int state = cp->state;
> + struct ip_vs_protocol *pp;
> +
> + if (cp->flags & IP_VS_CONN_F_TEMPLATE) {
>
> + if (state >= IP_VS_CTPL_S_LAST)
> + return "ERR!";
> + return ip_vs_ctpl_state_name_table[state] ? : "?";
> + }
> + pp = ip_vs_proto_get(cp->protocol);
> if (pp == NULL || pp->state_name == NULL)
> - return (IPPROTO_IP == proto) ? "NONE" : "ERR!";
> + return (cp->protocol == IPPROTO_IP) ? "NONE" : "ERR!";
> return pp->state_name(state);
> }
I'd slightly prefer if refactoring ip_vs_state_name to not take a state
parameter was a separate patch. Its a nice cleanup. But it doesn't seem
related to the rest of what is going on here.
>
> diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> index 3250c4a1..13826ee 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> @@ -461,6 +461,13 @@ set_sctp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
> cp->flags &= ~IP_VS_CONN_F_INACTIVE;
> }
> }
> + if (next_state == IP_VS_SCTP_S_ESTABLISHED) {
> + struct ip_vs_conn *ct = cp->control;
> +
> + if (ct && (ct->flags & IP_VS_CONN_F_TEMPLATE) &&
> + !(ct->state & IP_VS_CTPL_S_ASSURED))
> + ct->state |= IP_VS_CTPL_S_ASSURED;
> + }
The logic above seems to be replicated several times below.
Could we have a helper function?
> }
> if (likely(pd))
> cp->timeout = pd->timeout_table[cp->state = next_state];
> diff --git a/net/netfilter/ipvs/ip_vs_proto_tcp.c b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> index b73fe6e..467a69e 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_tcp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_tcp.c
> @@ -569,6 +569,13 @@ set_tcp_state(struct ip_vs_proto_data *pd, struct ip_vs_conn *cp,
> cp->flags &= ~IP_VS_CONN_F_INACTIVE;
> }
> }
> + if (new_state == IP_VS_TCP_S_ESTABLISHED) {
> + struct ip_vs_conn *ct = cp->control;
> +
> + if (ct && (ct->flags & IP_VS_CONN_F_TEMPLATE) &&
> + !(ct->state & IP_VS_CTPL_S_ASSURED))
> + ct->state |= IP_VS_CTPL_S_ASSURED;
> + }
> }
>
> if (likely(pd))
> diff --git a/net/netfilter/ipvs/ip_vs_proto_udp.c b/net/netfilter/ipvs/ip_vs_proto_udp.c
> index e0ef11c..c733ed6 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_udp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_udp.c
> @@ -460,6 +460,13 @@ udp_state_transition(struct ip_vs_conn *cp, int direction,
> }
>
> cp->timeout = pd->timeout_table[IP_VS_UDP_S_NORMAL];
> + if (direction == IP_VS_DIR_OUTPUT) {
> + struct ip_vs_conn *ct = cp->control;
> +
> + if (ct && (ct->flags & IP_VS_CONN_F_TEMPLATE) &&
> + !(ct->state & IP_VS_CTPL_S_ASSURED))
> + ct->state |= IP_VS_CTPL_S_ASSURED;
> + }
> }
>
> static int __udp_init(struct netns_ipvs *ipvs, struct ip_vs_proto_data *pd)
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 001501e..24891bd 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -1003,11 +1003,10 @@ static void ip_vs_process_message_v0(struct netns_ipvs *ipvs, const char *buffer
> continue;
> }
> } else {
> - /* protocol in templates is not used for state/timeout */
> - if (state > 0) {
> - IP_VS_DBG(2, "BACKUP v0, Invalid template state %u\n",
> - state);
> - state = 0;
> + if (state >= IP_VS_CTPL_S_LAST) {
> + IP_VS_DBG(7, "BACKUP v0, Invalid tpl state %u\n",
> + state);
Not strictly related to this patch, but should these debug messages
be rate limited in some way?
> + state &= IP_VS_CTPL_S_LAST - 1;
> }
> }
>
> @@ -1166,11 +1165,10 @@ static inline int ip_vs_proc_sync_conn(struct netns_ipvs *ipvs, __u8 *p, __u8 *m
> goto out;
> }
> } else {
> - /* protocol in templates is not used for state/timeout */
> - if (state > 0) {
> - IP_VS_DBG(3, "BACKUP, Invalid template state %u\n",
> - state);
> - state = 0;
> + if (state >= IP_VS_CTPL_S_LAST) {
> + IP_VS_DBG(7, "BACKUP, Invalid tpl state %u\n",
> + state);
> + state &= IP_VS_CTPL_S_LAST - 1;
> }
> }
> if (ip_vs_conn_fill_param_sync(ipvs, af, s, ¶m, pe_data,
> --
> 2.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe lvs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
