Mike Pattrick via dev <[email protected]> writes: > Recent versions of Curl combine the EPSV and EPRT commands with IPv4 > connections instead of the PASV and PORT commands. EPSV and EPRT were > added to FTP for IPv6 support but these commands also support IPv4. Most > software still uses the old PASV and PORT commands in IPv4 connections > but recent versions of curl will default to EPSV and EPRT for all > connections. This patch adds support for these extended commands, and > added tests for both with and without them. > > Reported-at: https://issues.redhat.com/browse/FDP-907 > Reported-by: Eelco Chaudron <[email protected]> > Signed-off-by: Mike Pattrick <[email protected]> > > ---
Looks like the version number got messed up, but that was the only real nit, so I've applied it after working around the NEWS hung. Thanks. Thanks Mike. > v2: > - Corrected news item and comments > - Active/passive parsing now uses repl_bytes instead of spaghetti loop > > Signed-off-by: Mike Pattrick <[email protected]> > --- > NEWS | 3 + > lib/conntrack.c | 180 ++++++++++++++++++++++------------ > tests/system-common-macros.at | 4 +- > tests/system-traffic.at | 10 +- > 4 files changed, 131 insertions(+), 66 deletions(-) > > diff --git a/NEWS b/NEWS > index b5d565ecc..5f83cda6d 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,8 @@ > Post-v3.6.0 > -------------------- > + - Userspace datapath: > + * Conntrack now supports the FTP commands EPSV and EPRT with IPv4 > + connections, instead of limiting these commands to IPv6 only. > > > v3.6.0 - xx xxx xxxx > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 00346a0be..00262a0c6 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -192,6 +192,8 @@ static alg_helper alg_helpers[] = { > #define FTP_PORT_CMD "PORT" > /* FTP pasv string used in passive mode. */ > #define FTP_PASV_REPLY_CODE "227" > +/* FTP epsv string used in passive mode. */ > +#define FTP_EPSV_REPLY_CODE "229" > /* Maximum decimal digits for port in FTP command. > * The port is represented as two 3 digit numbers with the > * high part a multiple of 256. */ > @@ -203,6 +205,8 @@ static alg_helper alg_helpers[] = { > #define FTP_EPSV_REPLY "EXTENDED PASSIVE" > /* Maximum decimal digits for port in FTP extended command. */ > #define MAX_EXT_FTP_PORT_DGTS 5 > +/* FTP extended command code for IPv4. */ > +#define FTP_AF_V4 '1' > /* FTP extended command code for IPv6. */ > #define FTP_AF_V6 '2' > /* Used to indicate a wildcard L4 source port number for ALGs. > @@ -3243,11 +3247,15 @@ replace_substring(char *substr, uint8_t substr_size, > } > > static void > -repl_bytes(char *str, char c1, char c2) > +repl_bytes(char *str, char c1, char c2, int max) > { > while (*str) { > if (*str == c1) { > *str = c2; > + > + if (--max == 0) { > + break; > + } > } > str++; > } > @@ -3269,10 +3277,16 @@ static int > repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 v4_addr_rep, > char *ftp_data_start, > size_t addr_offset_from_ftp_data_start, > - size_t addr_size OVS_UNUSED) > + size_t addr_size) > { > enum { MAX_FTP_V4_NAT_DELTA = 8 }; > > + /* EPSV mode. */ > + if (addr_offset_from_ftp_data_start == 0 && > + addr_size == 0) { > + return 0; > + } > + > /* Do conservative check for pathological MTU usage. */ > uint32_t orig_used_size = dp_packet_size(pkt); > if (orig_used_size + MAX_FTP_V4_NAT_DELTA > > @@ -3287,7 +3301,7 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 > v4_addr_rep, > char v4_addr_str[INET_ADDRSTRLEN] = {0}; > ovs_assert(inet_ntop(AF_INET, &v4_addr_rep, v4_addr_str, > sizeof v4_addr_str)); > - repl_bytes(v4_addr_str, '.', ','); > + repl_bytes(v4_addr_str, '.', ',', 0); > modify_packet(pkt, ftp_data_start + addr_offset_from_ftp_data_start, > addr_size, v4_addr_str, strlen(v4_addr_str), > orig_used_size); > @@ -3345,8 +3359,11 @@ detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx, > } > } else { > if (strncasecmp(ftp_msg, FTP_PORT_CMD, strlen(FTP_PORT_CMD)) && > + strncasecmp(ftp_msg, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD)) && > strncasecmp(ftp_msg, FTP_PASV_REPLY_CODE, > - strlen(FTP_PASV_REPLY_CODE))) { > + strlen(FTP_PASV_REPLY_CODE)) && > + strncasecmp(ftp_msg, FTP_EPSV_REPLY_CODE, > + strlen(FTP_EPSV_REPLY_CODE))) { > return CT_FTP_CTL_OTHER; > } > } > @@ -3370,11 +3387,22 @@ process_ftp_ctl_v4(struct conntrack *ct, > char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0}; > get_ftp_ctl_msg(pkt, ftp_msg); > char *ftp = ftp_msg; > + struct in_addr ip_addr; > enum ct_alg_mode mode; > + bool extended = false; > > if (!strncasecmp(ftp, FTP_PORT_CMD, strlen(FTP_PORT_CMD))) { > ftp = ftp_msg + strlen(FTP_PORT_CMD); > mode = CT_FTP_MODE_ACTIVE; > + } else if (!strncasecmp(ftp, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD))) { > + ftp = ftp_msg + strlen(FTP_EPRT_CMD); > + mode = CT_FTP_MODE_ACTIVE; > + extended = true; > + } else if (!strncasecmp(ftp, FTP_EPSV_REPLY_CODE, > + strlen(FTP_EPSV_REPLY_CODE))) { > + ftp = ftp_msg + strlen(FTP_EPSV_REPLY_CODE); > + mode = CT_FTP_MODE_PASSIVE; > + extended = true; > } else { > ftp = ftp_msg + strlen(FTP_PASV_REPLY_CODE); > mode = CT_FTP_MODE_PASSIVE; > @@ -3392,71 +3420,101 @@ process_ftp_ctl_v4(struct conntrack *ct, > return CT_FTP_CTL_INVALID; > } > > - char *ip_addr_start = ftp; > - *addr_offset_from_ftp_data_start = ip_addr_start - ftp_msg; > + /* EPRT, verify address family. */ > + if (extended && mode == CT_FTP_MODE_ACTIVE) { > + if (ftp[0] != FTP_AF_V4 || isdigit(ftp[1])) { > + return CT_FTP_CTL_INVALID; > + } > + > + ftp = skip_non_digits(ftp + 1); > + if (*ftp == 0) { > + return CT_FTP_CTL_INVALID; > + } > + } > + > + if (!extended || mode == CT_FTP_MODE_ACTIVE) { > + char *ip_addr_start = ftp; > + *addr_offset_from_ftp_data_start = ip_addr_start - ftp_msg; > + repl_bytes(ftp, ',', '.', 3); > > - uint8_t comma_count = 0; > - while (comma_count < 4 && *ftp) { > - if (*ftp == ',') { > - comma_count++; > - if (comma_count == 4) { > - *ftp = 0; > - } else { > - *ftp = '.'; > + /* Advance to end of IP address, to terminate it. */ > + while (*ftp) { > + if (!isdigit(*ftp) && *ftp != '.') { > + break; > } > + ftp++; > } > + *ftp = 0; > ftp++; > - } > - if (comma_count != 4) { > - return CT_FTP_CTL_INVALID; > - } > > - struct in_addr ip_addr; > - int rc2 = inet_pton(AF_INET, ip_addr_start, &ip_addr); > - if (rc2 != 1) { > - return CT_FTP_CTL_INVALID; > + int rc2 = inet_pton(AF_INET, ip_addr_start, &ip_addr); > + if (rc2 != 1) { > + return CT_FTP_CTL_INVALID; > + } > + > + *addr_size = ftp - ip_addr_start - 1; > + } else { > + *addr_size = 0; > + *addr_offset_from_ftp_data_start = 0; > } > > - *addr_size = ftp - ip_addr_start - 1; > char *save_ftp = ftp; > - ftp = terminate_number_str(ftp, MAX_FTP_PORT_DGTS); > - if (!ftp) { > - return CT_FTP_CTL_INVALID; > - } > - int value; > - if (!str_to_int(save_ftp, 10, &value)) { > - return CT_FTP_CTL_INVALID; > - } > + uint16_t port_hs; > > - /* This is derived from the L4 port maximum is 65535. */ > - if (value > 255) { > - return CT_FTP_CTL_INVALID; > - } > + if (!extended) { > + ftp = terminate_number_str(ftp, MAX_FTP_PORT_DGTS); > + if (!ftp) { > + return CT_FTP_CTL_INVALID; > + } > + int value; > + if (!str_to_int(save_ftp, 10, &value)) { > + return CT_FTP_CTL_INVALID; > + } > > - uint16_t port_hs = value; > - port_hs <<= 8; > + /* This is derived from the L4 port maximum is 65535. */ > + if (value > 255) { > + return CT_FTP_CTL_INVALID; > + } > + > + port_hs = value; > + port_hs <<= 8; > > - /* Skip over comma. */ > - ftp++; > - save_ftp = ftp; > - bool digit_found = false; > - while (isdigit(*ftp)) { > + /* Skip over comma. */ > ftp++; > - digit_found = true; > - } > - if (!digit_found) { > - return CT_FTP_CTL_INVALID; > - } > - *ftp = 0; > - if (!str_to_int(save_ftp, 10, &value)) { > - return CT_FTP_CTL_INVALID; > - } > + save_ftp = ftp; > + bool digit_found = false; > + while (isdigit(*ftp)) { > + ftp++; > + digit_found = true; > + } > + if (!digit_found) { > + return CT_FTP_CTL_INVALID; > + } > + *ftp = 0; > + if (!str_to_int(save_ftp, 10, &value)) { > + return CT_FTP_CTL_INVALID; > + } > > - if (value > 255) { > - return CT_FTP_CTL_INVALID; > + if (value > 255) { > + return CT_FTP_CTL_INVALID; > + } > + > + port_hs |= value; > + } else { > + ftp = terminate_number_str(ftp, MAX_EXT_FTP_PORT_DGTS); > + if (!ftp) { > + return CT_FTP_CTL_INVALID; > + } > + int value; > + if (!str_to_int(save_ftp, 10, &value)) { > + return CT_FTP_CTL_INVALID; > + } > + if (value > UINT16_MAX) { > + return CT_FTP_CTL_INVALID; > + } > + port_hs = (uint16_t) value; > } > > - port_hs |= value; > ovs_be16 port = htons(port_hs); > ovs_be32 conn_ipv4_addr; > > @@ -3478,12 +3536,14 @@ process_ftp_ctl_v4(struct conntrack *ct, > OVS_NOT_REACHED(); > } > > - ovs_be32 ftp_ipv4_addr; > - ftp_ipv4_addr = ip_addr.s_addr; > - /* Although most servers will block this exploit, there may be some > - * less well managed. */ > - if (ftp_ipv4_addr != conn_ipv4_addr && ftp_ipv4_addr != *v4_addr_rep) { > - return CT_FTP_CTL_INVALID; > + if (!extended || mode == CT_FTP_MODE_ACTIVE) { > + ovs_be32 ftp_ipv4_addr; > + ftp_ipv4_addr = ip_addr.s_addr; > + /* Although most servers will block this exploit, there may be some > + * less well managed. */ > + if (ftp_ipv4_addr != conn_ipv4_addr && ftp_ipv4_addr != > *v4_addr_rep) { > + return CT_FTP_CTL_INVALID; > + } > } > > expectation_create(ct, port, conn_for_expectation, > diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at > index 7f029dbb0..e4862231a 100644 > --- a/tests/system-common-macros.at > +++ b/tests/system-common-macros.at > @@ -280,7 +280,7 @@ m4_define([OVS_GET_HTTP], > # > m4_define([OVS_GET_FTP], > [curl ftp://$1 --retry 3 --max-time 1 --retry-connrefused \ > - --disable-epsv -v $2] > + -v $2] > ) > > # OVS_GET_FTP_ACTIVE([url], [optional_curl_arguments]) > @@ -289,7 +289,7 @@ m4_define([OVS_GET_FTP], > # > m4_define([OVS_GET_FTP_ACTIVE], > [curl ftp://$1 --retry 3 --max-time 1 --retry-connrefused -v \ > - --ftp-port - --disable-eprt $2] > + --ftp-port - $2] > ) > > # OVS_CHECK_FIREWALL() > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index 0f3acef3f..e5a1131e6 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -8204,8 +8204,9 @@ OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 10.1.1.2 > >/dev/null]) > > OVS_START_L7([at_ns1], [ftp]) > > -dnl FTP requests from p0->p1 should work fine. > -NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([10.1.1.2]), [0], [ignore], [ignore]) > +dnl FTP requests from p0->p1 should work fine, with and without EPSV. > +NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([10.1.1.2], [--epsv]), [0], [ignore], > [ignore]) > +NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([10.1.1.2], [--disable-epsv]), [0], > [ignore], [ignore]) > > dnl Discards CLOSE_WAIT and CLOSING > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl > @@ -8384,8 +8385,9 @@ OVS_WAIT_UNTIL([ip netns exec at_ns0 ping -c 1 > 10.1.1.240 >/dev/null]) > > OVS_START_L7([at_ns1], [ftp]) > > -dnl FTP requests from p0->p1 should work fine. > -NS_CHECK_EXEC([at_ns0], OVS_GET_FTP_ACTIVE([10.1.1.2]), [0], [ignore], > [ignore]) > +dnl FTP requests from p0->p1 should work fine with and without EPRT. > +NS_CHECK_EXEC([at_ns0], OVS_GET_FTP_ACTIVE([10.1.1.2], [--eprt]), [0], > [ignore], [ignore]) > +NS_CHECK_EXEC([at_ns0], OVS_GET_FTP_ACTIVE([10.1.1.2], [--disable-eprt]), > [0], [ignore], [ignore]) > > dnl Discards CLOSE_WAIT and CLOSING > AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
