Skip to content

Commit

Permalink
Merge pull request #221 from Castaglia/proxy-ftp-data-addr-mismatch-i…
Browse files Browse the repository at this point in the history
…ssue158

Issue #158: Adding more regression test coverage for proxying FTP dat…
  • Loading branch information
Castaglia committed Jun 19, 2022
2 parents 1cbf479 + b0e3378 commit e658fc9
Show file tree
Hide file tree
Showing 5 changed files with 2,052 additions and 85 deletions.
1 change: 1 addition & 0 deletions .github/workflows/regressions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ jobs:
libfile-spec-native-perl \
libmime-base32-perl \
libnet-address-ip-local-perl \
libnet-inet6glue-perl \
libnet-ssh2-perl \
libnet-ssleay-perl \
libnet-telnet-perl \
Expand Down
50 changes: 34 additions & 16 deletions lib/proxy/conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -717,6 +717,21 @@ conn_t *proxy_conn_get_server_conn(pool *p, struct proxy_session *proxy_sess,
pr_trace_msg(trace_channel, 4,
"error converting IPv6 local address %s to IPv4 address: %s",
pr_netaddr_get_ipstr(session.c->local_addr), strerror(errno));

if (proxy_sess->src_addr == NULL) {
(void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
"local address '%s' is an IPv6 address, and remote address "
"'%s' is an IPv4 address; consider using ProxySourceAddress "
"directive to configure an IPv4 address",
pr_netaddr_get_ipstr(session.c->local_addr),
pr_netaddr_get_ipstr(remote_addr));
}

} else {
pr_trace_msg(trace_channel, 9,
"converted IPv6 local address %s to IPv4 address %s",
pr_netaddr_get_ipstr(session.c->local_addr),
pr_netaddr_get_ipstr(local_addr));
}
}

Expand Down Expand Up @@ -755,7 +770,7 @@ conn_t *proxy_conn_get_server_conn(pool *p, struct proxy_session *proxy_sess,
if (local_family != remote_family) {
pr_netaddr_t *new_addr = NULL;

#ifdef PR_USE_IPV6
#if defined(PR_USE_IPV6)
if (local_family == AF_INET) {
new_addr = pr_netaddr_v4tov6(p, new_local_addr);

Expand Down Expand Up @@ -808,22 +823,25 @@ conn_t *proxy_conn_get_server_conn(pool *p, struct proxy_session *proxy_sess,
* for those particular errors, and if so, log a suggestion to explicitly
* configure an appropriate ProxySourceAddress (Issue #213).
*/
if (netaddr_is_private(bind_addr) == TRUE) {
if (netaddr_is_private(remote_addr) != TRUE) {
(void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
"local address '%s' is a private network address, and remote address "
"'%s' is a public address; consider using ProxySourceAddress "
"directive to configure a public local address",
pr_netaddr_get_ipstr(bind_addr), remote_ipstr);
}

} else {
if (netaddr_is_private(remote_addr) == TRUE) {
(void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
"local address '%s' is a public address, and remote address '%s' is "
"a private network address; consider using ProxySourceAddress "
"directive to configure a private local address",
pr_netaddr_get_ipstr(bind_addr), remote_ipstr);
if (pr_netaddr_get_family(bind_addr) == pr_netaddr_get_family(remote_addr)) {
if (netaddr_is_private(bind_addr) == TRUE) {
if (netaddr_is_private(remote_addr) != TRUE) {
(void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
"local address '%s' is a private network address, and remote "
"address '%s' is a public address; consider using "
"ProxySourceAddress directive to configure a public local address",
pr_netaddr_get_ipstr(bind_addr), remote_ipstr);
}

} else {
if (netaddr_is_private(remote_addr) == TRUE) {
(void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
"local address '%s' is a public address, and remote address '%s' "
"is a private network address; consider using ProxySourceAddress "
"directive to configure a private local address",
pr_netaddr_get_ipstr(bind_addr), remote_ipstr);
}
}
}

Expand Down
12 changes: 6 additions & 6 deletions lib/proxy/ftp/conn.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* ProFTPD - mod_proxy FTP connection routines
* Copyright (c) 2013-2021 TJ Saunders
* Copyright (c) 2013-2022 TJ Saunders
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -166,7 +166,7 @@ conn_t *proxy_ftp_conn_connect(pool *p, const pr_netaddr_t *bind_addr,
pr_netaddr_get_ipstr(remote_addr), ntohs(pr_netaddr_get_port(remote_addr)),
pr_netaddr_get_ipstr(bind_addr), ntohs(pr_netaddr_get_port(bind_addr)));

if (frontend_data) {
if (frontend_data == TRUE) {
res = pr_inet_connect(p, conn, remote_addr,
ntohs(pr_netaddr_get_port(remote_addr)));

Expand All @@ -182,7 +182,7 @@ conn_t *proxy_ftp_conn_connect(pool *p, const pr_netaddr_t *bind_addr,
"unable to connect to %s#%u: %s\n", pr_netaddr_get_ipstr(remote_addr),
ntohs(pr_netaddr_get_port(remote_addr)), strerror(xerrno));

if (!frontend_data) {
if (frontend_data == FALSE) {
proxy_inet_close(session.pool, conn);
}
pr_inet_close(session.pool, conn);
Expand All @@ -193,7 +193,7 @@ conn_t *proxy_ftp_conn_connect(pool *p, const pr_netaddr_t *bind_addr,

/* XXX Will it always be STRM_DATA? */

if (frontend_data) {
if (frontend_data == TRUE) {
opened = pr_inet_openrw(session.pool, conn, NULL, PR_NETIO_STRM_DATA,
conn->listen_fd, -1, -1, TRUE);

Expand All @@ -207,7 +207,7 @@ conn_t *proxy_ftp_conn_connect(pool *p, const pr_netaddr_t *bind_addr,
if (opened == NULL) {
int xerrno = errno;

if (!frontend_data) {
if (frontend_data == FALSE) {
proxy_inet_close(session.pool, conn);
}
pr_inet_close(session.pool, conn);
Expand All @@ -219,7 +219,7 @@ conn_t *proxy_ftp_conn_connect(pool *p, const pr_netaddr_t *bind_addr,
/* The conn returned by pr_inet_openrw() is a copy of the input conn;
* we no longer need the input conn at this point.
*/
if (frontend_data) {
if (frontend_data == TRUE) {
pr_inet_close(session.pool, conn);
pr_pool_tag(opened->pool, "proxy frontend data connect conn pool");

Expand Down
54 changes: 50 additions & 4 deletions mod_proxy.c
Original file line number Diff line number Diff line change
Expand Up @@ -2389,24 +2389,70 @@ static int proxy_data_prepare_conns(struct proxy_session *proxy_sess,

/* XXX Should handle EPSV_ALL here, too. */
if (proxy_sess->backend_sess_flags & SF_PASSIVE) {
const pr_netaddr_t *bind_addr = NULL;
const pr_netaddr_t *bind_addr = NULL, *local_addr = NULL;

/* Connect to the backend server now. We won't receive the initial
* response until we connect to the backend data address/port.
*/

/* Check the family of the remote address vs what we'll be using to connect.
* If there's a mismatch, we need to get an addr with the matching family.
*/
if (pr_netaddr_get_family(bind_addr) != pr_netaddr_get_family(proxy_sess->backend_data_addr)) {
/* In this scenario, the proxy has an IPv6 socket, but the remote/backend
* server has an IPv4 (or IPv4-mapped IPv6) address. OR it's the proxy
* which has an IPv4 socket, and the remote/backend server has an IPv6
* address.
*/
if (pr_netaddr_get_family(session.c->local_addr) == AF_INET) {
char *ip_str;

/* Convert the local address from an IPv4 to an IPv6 addr. */
ip_str = pcalloc(cmd->tmp_pool, INET6_ADDRSTRLEN + 1);
snprintf(ip_str, INET6_ADDRSTRLEN, "::ffff:%s",
pr_netaddr_get_ipstr(session.c->local_addr));
local_addr = pr_netaddr_get_addr(cmd->tmp_pool, ip_str, NULL);

} else {
local_addr = pr_netaddr_v6tov4(cmd->tmp_pool, session.c->local_addr);
if (local_addr == NULL) {
pr_trace_msg(trace_channel, 4,
"error converting IPv6 local address %s to IPv4 address: %s",
pr_netaddr_get_ipstr(session.c->local_addr), strerror(errno));

if (proxy_sess->src_addr == NULL) {
(void) pr_log_writefile(proxy_logfd, MOD_PROXY_VERSION,
"local address '%s' is an IPv6 address, and remote address "
"'%s' is an IPv4 address; consider using ProxySourceAddress "
"directive to configure an IPv4 address",
pr_netaddr_get_ipstr(session.c->local_addr),
pr_netaddr_get_ipstr(proxy_sess->backend_data_addr));
}
}
}

if (local_addr != NULL) {
pr_trace_msg(trace_channel, 7,
"converted local address %s to %s for passive backend transfer",
pr_netaddr_get_ipstr(session.c->local_addr),
pr_netaddr_get_ipstr(local_addr));

} else {
local_addr = session.c->local_addr;
}
}

/* Specify the specific address/interface to use as the source address for
* connections to the destination server.
*/
bind_addr = proxy_sess->src_addr;
if (bind_addr == NULL) {
bind_addr = session.c->local_addr;
bind_addr = local_addr;
}

if (pr_netaddr_is_loopback(bind_addr) == TRUE &&
pr_netaddr_is_loopback(proxy_sess->backend_ctrl_conn->remote_addr) != TRUE) {
const char *local_name;
const pr_netaddr_t *local_addr;

local_name = pr_netaddr_get_localaddr_str(cmd->pool);
local_addr = pr_netaddr_get_addr(cmd->pool, local_name, NULL);
Expand All @@ -2423,7 +2469,7 @@ static int proxy_data_prepare_conns(struct proxy_session *proxy_sess,
if (local_family != remote_family) {
pr_netaddr_t *new_addr = NULL;

#ifdef PR_USE_IPV6
#if defined(PR_USE_IPV6)
if (local_family == AF_INET) {
new_addr = pr_netaddr_v4tov6(cmd->pool, local_addr);

Expand Down

0 comments on commit e658fc9

Please sign in to comment.