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 <echau...@redhat.com>
Signed-off-by: Mike Pattrick <m...@redhat.com>

---
v2:
- Corrected news item and comments
- Active/passive parsing now uses repl_bytes instead of spaghetti loop

Signed-off-by: Mike Pattrick <m...@redhat.com>
---
 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
-- 
2.50.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to