Thanks for working on this.
1/
What is the use case for multiple adjustments?
This code has been tested externally to Vmware as well.
Also multiple adjustments may be indicative of an exploit attempt or other
problem, so lets delineate
the use case first; please add a 'real' test case for this.
2/
IF we end up supporting multiple adjustments, as it stands now the patch
fails these tests
conntrack - NAT
96: conntrack - FTP NAT postrecirc seqadj FAILED (
system-traffic.at:4391)
98: conntrack - FTP NAT orig tuple seqadj FAILED (
system-traffic.at:4515)
The following alternative patch would correctly and succinctly implement
multiple adjustments.
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 6f6021a..f283db2 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -3225,7 +3225,8 @@ handle_ftp_ctl(struct conntrack *ct, const struct
conn_lookup_ctx *ctx,
ip_len += seq_skew;
nh6->ip6_ctlun.ip6_un1.ip6_un1_plen = htons(ip_len);
conn_seq_skew_set(ct, &conn_for_expectation->key, now,
- seq_skew, ctx->reply);
+ seq_skew +
+ conn_for_expectation->seq_skew,
ctx->reply);
}
} else {
seq_skew = repl_ftp_v4_addr(pkt, v4_addr_rep,
ftp_data_start,
@@ -3237,7 +3238,8 @@ handle_ftp_ctl(struct conntrack *ct, const struct
conn_lookup_ctx *ctx,
l3_hdr->ip_tot_len,
htons(ip_len));
l3_hdr->ip_tot_len = htons(ip_len);
conn_seq_skew_set(ct, &conn_for_expectation->key, now,
- seq_skew, ctx->reply);
+ seq_skew +
+ conn_for_expectation->seq_skew,
ctx->reply);
}
}
} else {
On Sat, Dec 15, 2018 at 9:38 AM David Marchand <david.march...@redhat.com>
wrote:
> The ftp alg deals with packets in two ways for the command connection:
> either they are inspected (CT_FTP_CTL_INTEREST) or they just go through
> without being modified (CT_FTP_CTL_OTHER).
>
> In both cases, the tcp seq/ack must be adjusted by the current offset
> that has been introduced in previous mangle operations and prepare for
> the next packets by setting an accumulated offset.
>
> Signed-off-by: David Marchand <david.march...@redhat.com>
> ---
> lib/conntrack.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/lib/conntrack.c b/lib/conntrack.c
> index 974f985..d08d0ea 100644
> --- a/lib/conntrack.c
> +++ b/lib/conntrack.c
> @@ -3185,11 +3185,9 @@ handle_ftp_ctl(struct conntrack *ct, const struct
> conn_lookup_ctx *ctx,
> }
>
> struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt);
> - int64_t seq_skew = 0;
> + int64_t seq_skew = conn_for_expectation->seq_skew;
>
> - if (ftp_ctl == CT_FTP_CTL_OTHER) {
> - seq_skew = conn_for_expectation->seq_skew;
> - } else if (ftp_ctl == CT_FTP_CTL_INTEREST) {
> + if (ftp_ctl == CT_FTP_CTL_INTEREST) {
> enum ftp_ctl_pkt rc;
> if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
> rc = process_ftp_ctl_v6(ct, pkt, conn_for_expectation,
> @@ -3208,35 +3206,36 @@ handle_ftp_ctl(struct conntrack *ct, const struct
> conn_lookup_ctx *ctx,
> return;
> } else if (rc == CT_FTP_CTL_INTEREST) {
> uint16_t ip_len;
> + int64_t new_skew;
>
> if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
> - seq_skew = repl_ftp_v6_addr(pkt, v6_addr_rep,
> ftp_data_start,
> + new_skew = repl_ftp_v6_addr(pkt, v6_addr_rep,
> ftp_data_start,
>
> addr_offset_from_ftp_data_start,
> addr_size, mode);
> - if (seq_skew) {
> + if (new_skew) {
> ip_len = ntohs(nh6->ip6_ctlun.ip6_un1.ip6_un1_plen);
> - ip_len += seq_skew;
> + ip_len += new_skew;
> nh6->ip6_ctlun.ip6_un1.ip6_un1_plen = htons(ip_len);
> conn_seq_skew_set(ct, &conn_for_expectation->key, now,
> - seq_skew, ctx->reply);
> + new_skew + seq_skew, ctx->reply);
> }
> } else {
> - seq_skew = repl_ftp_v4_addr(pkt, v4_addr_rep,
> ftp_data_start,
> + new_skew = repl_ftp_v4_addr(pkt, v4_addr_rep,
> ftp_data_start,
>
> addr_offset_from_ftp_data_start);
> ip_len = ntohs(l3_hdr->ip_tot_len);
> - if (seq_skew) {
> - ip_len += seq_skew;
> + if (new_skew) {
> + ip_len += new_skew;
> l3_hdr->ip_csum = recalc_csum16(l3_hdr->ip_csum,
> l3_hdr->ip_tot_len,
> htons(ip_len));
> l3_hdr->ip_tot_len = htons(ip_len);
> conn_seq_skew_set(ct, &conn_for_expectation->key, now,
> - seq_skew, ctx->reply);
> + new_skew + seq_skew, ctx->reply);
> }
> }
> } else {
> OVS_NOT_REACHED();
> }
> - } else {
> + } else if (ftp_ctl == CT_FTP_CTL_INVALID) {
> OVS_NOT_REACHED();
> }
>
> --
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
