Skip to content

Commit

Permalink
conntrack: fix tcp seq adjustments when mangling commands.
Browse files Browse the repository at this point in the history 
The ftp alg deals with packets in two ways for the command connection:
either they are inspected and can be mangled when nat is enabled
(CT_FTP_CTL_INTEREST) or they just go through without being modified
(CT_FTP_CTL_OTHER).

For CT_FTP_CTL_INTEREST packets, we must both adjust the packet tcp seq
number by the connection current offset, then prepare for the next
packets by setting an accumulated offset in the ct object.  However,
this was not done for multiple CT_FTP_CTL_INTEREST packets for the same
connection.
This is relevant for handling multiple child data connections that also
need natting.

The tests are updated so that some ftp+NAT tests send multiple port
commands or other similar commands for a single control connection.
Wget is not able to do this, so switch to lftp.

Fixes: bd5e81a ("Userspace Datapath: Add ALG infra and FTP.")
Co-authored-by: Darrell Ball <[email protected]>
Signed-off-by: Darrell Ball <[email protected]>
Signed-off-by: David Marchand <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
@david-marchand @darball1 @blp
2 people authored and blp committed Jan 19, 2019
1 parent 3c61cc7 commit 253e4dc
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 43 deletions.
9 changes: 6 additions & 3 deletions Vagrantfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ dnf -y install autoconf automake openssl-devel libtool \
python-twisted python-zope-interface \
desktop-file-utils groff graphviz rpmdevtools nc curl \
wget python-six pyftpdlib checkpolicy selinux-policy-devel \
libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy
libcap-ng-devel kernel-devel-`uname -r` ethtool python-tftpy \
lftp
echo "search extra update built-in" >/etc/depmod.d/search_path.conf
SCRIPT

Expand All @@ -28,7 +29,8 @@ aptitude -y install -R \
wget python-six ethtool \
libcap-ng-dev libssl-dev python-dev openssl \
python-pyftpdlib python-flake8 python-tftpy \
linux-headers-`uname -r`
linux-headers-`uname -r` \
lftp
SCRIPT

$bootstrap_centos = <<SCRIPT
Expand All @@ -37,7 +39,8 @@ yum -y install autoconf automake openssl-devel libtool \
python-twisted-core python-zope-interface \
desktop-file-utils groff graphviz rpmdevtools nc curl \
wget python-six pyftpdlib checkpolicy selinux-policy-devel \
libcap-ng-devel kernel-devel-`uname -r` ethtool net-tools
libcap-ng-devel kernel-devel-`uname -r` ethtool net-tools \
lftp
SCRIPT

$configure_ovs = <<SCRIPT
Expand Down
2 changes: 1 addition & 1 deletion Vagrantfile-FreeBSD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ Vagrant.require_version ">=1.7.0"
$bootstrap_freebsd = <<SCRIPT
sed -e 's/\#DEFAULT_ALWAYS_YES = false/DEFAULT_ALWAYS_YES = true/g' -e 's/\#ASSUME_ALWAYS_YES = false/ASSUME_ALWAYS_YES = true/g' /usr/local/etc/pkg.conf > /tmp/pkg.conf
mv -f /tmp/pkg.conf /usr/local/etc/pkg.conf
pkg install automake libtool wget python py27-six gmake
pkg install automake libtool wget python py27-six gmake lftp
SCRIPT

$configure_ovs = <<SCRIPT
Expand Down
74 changes: 36 additions & 38 deletions lib/conntrack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3170,41 +3170,33 @@ repl_ftp_v6_addr(struct dp_packet *pkt, struct ct_addr v6_addr_rep,

static void
handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
struct dp_packet *pkt,
const struct conn *conn_for_expectation,
long long now, enum ftp_ctl_pkt ftp_ctl, bool nat)
struct dp_packet *pkt, const struct conn *ec, long long now,
enum ftp_ctl_pkt ftp_ctl, bool nat)
{
struct ip_header *l3_hdr = dp_packet_l3(pkt);
ovs_be32 v4_addr_rep = 0;
struct ct_addr v6_addr_rep;
size_t addr_offset_from_ftp_data_start = 0;
size_t addr_size = 0;
char *ftp_data_start;
bool do_seq_skew_adj = true;
enum ct_alg_mode mode = CT_FTP_MODE_ACTIVE;

if (detect_ftp_ctl_type(ctx, pkt) != ftp_ctl) {
return;
}

if (!nat || !conn_for_expectation->seq_skew) {
do_seq_skew_adj = false;
}

struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt);
int64_t seq_skew = 0;

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,
rc = process_ftp_ctl_v6(ct, pkt, ec,
&v6_addr_rep, &ftp_data_start,
&addr_offset_from_ftp_data_start,
&addr_size, &mode);
} else {
rc = process_ftp_ctl_v4(ct, pkt, conn_for_expectation,
rc = process_ftp_ctl_v4(ct, pkt, ec,
&v4_addr_rep, &ftp_data_start,
&addr_offset_from_ftp_data_start);
}
Expand All @@ -3217,62 +3209,63 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
uint16_t ip_len;

if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) {
seq_skew = repl_ftp_v6_addr(pkt, v6_addr_rep, ftp_data_start,
addr_offset_from_ftp_data_start,
addr_size, mode);
if (nat) {
seq_skew = repl_ftp_v6_addr(pkt, v6_addr_rep,
ftp_data_start,
addr_offset_from_ftp_data_start,
addr_size, mode);
}

if (seq_skew) {
ip_len = ntohs(nh6->ip6_ctlun.ip6_un1.ip6_un1_plen);
ip_len += seq_skew;
ip_len = ntohs(nh6->ip6_ctlun.ip6_un1.ip6_un1_plen) +
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);
}
} else {
seq_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 (nat) {
seq_skew = repl_ftp_v4_addr(pkt, v4_addr_rep,
ftp_data_start,
addr_offset_from_ftp_data_start);
}
if (seq_skew) {
ip_len += seq_skew;
ip_len = ntohs(l3_hdr->ip_tot_len) + seq_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);
}
}
} else {
OVS_NOT_REACHED();
}
} else {
OVS_NOT_REACHED();
}

struct tcp_header *th = dp_packet_l4(pkt);

if (do_seq_skew_adj && seq_skew != 0) {
if (ctx->reply != conn_for_expectation->seq_skew_dir) {
if (nat && ec->seq_skew != 0) {
if (ctx->reply != ec->seq_skew_dir) {

uint32_t tcp_ack = ntohl(get_16aligned_be32(&th->tcp_ack));

if ((seq_skew > 0) && (tcp_ack < seq_skew)) {
if ((ec->seq_skew > 0) && (tcp_ack < ec->seq_skew)) {
/* Should not be possible; will be marked invalid. */
tcp_ack = 0;
} else if ((seq_skew < 0) && (UINT32_MAX - tcp_ack < -seq_skew)) {
tcp_ack = (-seq_skew) - (UINT32_MAX - tcp_ack);
} else if ((ec->seq_skew < 0) &&
(UINT32_MAX - tcp_ack < -ec->seq_skew)) {
tcp_ack = (-ec->seq_skew) - (UINT32_MAX - tcp_ack);
} else {
tcp_ack -= seq_skew;
tcp_ack -= ec->seq_skew;
}
ovs_be32 new_tcp_ack = htonl(tcp_ack);
put_16aligned_be32(&th->tcp_ack, new_tcp_ack);
} else {
uint32_t tcp_seq = ntohl(get_16aligned_be32(&th->tcp_seq));
if ((seq_skew > 0) && (UINT32_MAX - tcp_seq < seq_skew)) {
tcp_seq = seq_skew - (UINT32_MAX - tcp_seq);
} else if ((seq_skew < 0) && (tcp_seq < -seq_skew)) {
if ((ec->seq_skew > 0) && (UINT32_MAX - tcp_seq < ec->seq_skew)) {
tcp_seq = ec->seq_skew - (UINT32_MAX - tcp_seq);
} else if ((ec->seq_skew < 0) && (tcp_seq < -ec->seq_skew)) {
/* Should not be possible; will be marked invalid. */
tcp_seq = 0;
} else {
tcp_seq += seq_skew;
tcp_seq += ec->seq_skew;
}
ovs_be32 new_tcp_seq = htonl(tcp_seq);
put_16aligned_be32(&th->tcp_seq, new_tcp_seq);
Expand All @@ -3290,6 +3283,11 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx,
uint8_t pad = dp_packet_l2_pad_size(pkt);
th->tcp_csum = csum_finish(
csum_continue(tcp_csum, th, tail - (char *) th - pad));

if (seq_skew) {
conn_seq_skew_set(ct, &ec->key, now, seq_skew + ec->seq_skew,
ctx->reply);
}
}

static void
Expand Down
3 changes: 3 additions & 0 deletions tests/atlocal.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ fi
# Set HAVE_TCPDUMP
find_command tcpdump

# Set HAVE_LFTP
find_command lftp

CURL_OPT="-g -v --max-time 1 --retry 2 --retry-delay 1 --connect-timeout 1"

# Determine whether "diff" supports "normal" diffs. (busybox diff does not.)
Expand Down
14 changes: 13 additions & 1 deletion tests/system-traffic.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4226,6 +4226,7 @@ dnl NAT, using the provided flow table.
m4_define([CHECK_FTP_NAT],
[AT_SETUP([conntrack - FTP NAT $1])
AT_SKIP_IF([test $HAVE_FTP = no])
AT_SKIP_IF([test $HAVE_LFTP = no])
CHECK_CONNTRACK()
CHECK_CONNTRACK_NAT()
CHECK_CONNTRACK_ALG()
Expand All @@ -4246,7 +4247,18 @@ m4_define([CHECK_FTP_NAT],
OVS_START_L7([at_ns1], [ftp])

dnl FTP requests from p0->p1 should work fine.
NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2 -4 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v --server-response --no-remove-listing -o wget0.log -d])
AT_DATA([ftp.cmd], [dnl
set net:max-retries 1
set net:timeout 1
set ftp:passive-mode off
cache off
connect ftp://anonymous:@10.1.1.2
ls
ls
ls
ls
])
NS_CHECK_EXEC([at_ns0], [lftp -f ftp.cmd > lftp.log])

dnl Discards CLOSE_WAIT and CLOSING
AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [$4])
Expand Down

0 comments on commit 253e4dc

Please sign in to comment.