net_ftp: Advertise correct IP for PASV depending on source.

Originally, PASV would autodetermine the IP to use by using the first
interface's address, which doesn't work if there are multiple
addresses, or if the client is behind NAT.

To fix this, we now iterate all the interfaces and pick the best one
based on if the netmask matches the client's address. Additionally,
an IP address for PASV can be explicitly specified, one for both
private and public IP ranges.

bbs/socket.c

@@ -1014,39 +1014,83 @@ void bbs_tcp_listener(int socket, const char *name, void *(*handler)(void *varg)
 	return __bbs_tcp_listener(socket, name, NULL, handler, module);
 }
 
-int bbs_get_local_ip(char *buf, size_t len)
+int bbs_get_local_ip(struct bbs_node *node, char *buf, size_t len)
 {
 	int res = -1;
 	struct sockaddr_in *sinaddr;
 	struct ifaddrs *iflist, *iface;
+	int loops = 0;
 	if (getifaddrs(&iflist)) {
 		bbs_error("getifaddrs failed: %s\n", strerror(errno));
 		return -1;
 	}
 
-	for (iface = iflist; res && iface; iface = iface->ifa_next) {
+loop:
+	for (iface = iflist; iface; iface = iface->ifa_next) {
 		int af;
+		char netmaskbuf[32];
+		struct sockaddr_in *netmask;
 		if (!iface->ifa_addr) {
 			/* This can be NULL for interfaces without an IP address assigned. */
 			continue;
 		}
 		af = iface->ifa_addr->sa_family;
 		switch (af) {
 			case AF_INET:
-				sinaddr = ((struct sockaddr_in *) iface->ifa_addr);
+				sinaddr = (struct sockaddr_in *) iface->ifa_addr;
+				netmask = (struct sockaddr_in *) iface->ifa_netmask;
 				bbs_get_remote_ip(sinaddr, buf, len);
 				if (bbs_is_loopback_ipv4(buf)) {
 					break; /* Skip the loopback interface, we want the (a) real one */
 				}
-				bbs_debug(5, "Local IP: %s\n", buf);
-				res = 0; /* for loop condition will now be false */
-				break;
+				/* Yeah, not very efficient, but since we store the node's IP as
+				 * a string, rather than binary, we have to convert the netmask, too,
+				 * for comparison (but we're also printing it, so, whatever). */
+				inet_ntop(af, &netmask->sin_addr, netmaskbuf, sizeof(netmaskbuf));
+				bbs_debug(5, "%s: %s/%s\n", iface->ifa_name, buf, netmaskbuf);
+				res = 0;
+				if (node && !loops) {
+					struct in_addr addr;
+					uint32_t netmask_mask;
+					int netmasklen = 0;
+					char cidr_range[96];
+
+					/* Convert subnet mask to prefix length */
+					inet_pton(af, netmaskbuf, &addr);
+					netmask_mask = ntohl(addr.s_addr);
+					while (netmask_mask & 0x80000000) {
+						netmask_mask <<= 1;
+						netmasklen++;
+					}
+
+					snprintf(cidr_range, sizeof(cidr_range), "%s/%d", buf, netmasklen);
+					/* If we have a node, that means we have an IP address against which to compare,
+					 * and we want the interface on which this connection arrived.
+					 * For example, if this interface has a public IP but the node has a private one,
+					 * we should look for an interface with a private address.
+					 *
+					 * Of course, it's impossible to reliably determine the IP address
+					 * that a client behind certain kinds of NAT used to get to us,
+					 * e.g. a NAT'ed VPN tunnel. */
+					if (bbs_cidr_match_ipv4(node->ip, cidr_range)) {
+						/* IP matches netmask */
+						bbs_debug(5, "%s matches CIDR range %s\n", node->ip, cidr_range);
+						goto done;
+					} /* else, check next interface and see if it matches */
+				} else {
+					goto done; /* Break out of for loop */
+				}
 			case AF_INET6:
 			default:
 				break;
 		}
 	}
 
+	bbs_debug(4, "Unable to determine matching interface, using default\n");
+	loops++;
+	goto loop;
+
+done:
 	if (res) {
 		bbs_error("Failed to determine local IP address\n");
 	}
@@ -1185,6 +1229,7 @@ int bbs_cidr_match_ipv4(const char *ip, const char *cidr)
 	int netbits;
 	struct in_addr addr, netmask;
 	uint32_t a, b;
+	int match;
 
 	safe_strncpy(cidr_dup, cidr, sizeof(cidr_dup));
 	tmp = strchr(cidr_dup, '/');
@@ -1224,8 +1269,9 @@ int bbs_cidr_match_ipv4(const char *ip, const char *cidr)
 	a = a >> (32 - netbits);
 	b = b >> (32 - netbits);
 
-	bbs_debug(7, "IP comparison (%d): %08x/%08x\n", netbits, a, b);
-	return a == b;
+	match = a == b;
+	bbs_debug(7, "IP comparison (%d): %08x/%08x => match: %s\n", netbits, a, b, match ? "yes" : "no");
+	return match;
 }
 
 int bbs_ip_match_ipv4(const char *ip, const char *s)
@@ -1235,7 +1281,7 @@ int bbs_ip_match_ipv4(const char *ip, const char *s)
 	if (strchr(s, '/')) {
 		/* It's a CIDR range. Do a direct comparison. */
 		if (bbs_cidr_match_ipv4(ip, s)) {
-			bbs_debug(5, "CIDR match: %s\n", s);
+			bbs_debug(5, "%s matches CIDR range %s\n", ip, s);
 			return 1;
 		}
 		return 0;

bbs/transfer.c

@@ -662,7 +662,7 @@ int bbs_transfer_config_load(void)
 	upload_priv = 1;
 	delete_priv = 2;
 	newdir_priv = 2;
-	
+
 	bbs_config_val_set_int(cfg, "privs", "access", &privs[TRANSFER_ACCESS]);
 	bbs_config_val_set_int(cfg, "privs", "download", &privs[TRANSFER_DOWNLOAD]);
 	bbs_config_val_set_int(cfg, "privs", "upload", &privs[TRANSFER_UPLOAD]);

configs/net_ftp.conf

@@ -21,3 +21,10 @@ requirereuse=no ; Whether to require TLS session reuse from the control connecti
 ; These settings have no default. They must be specified explicitly.
 minport=10000
 maxport=20000
+; If your machine has multiple interfaces, or is behind NAT, you will want to specify the IP addresses
+; to be used for PASV connections so clients establish a connection to the same interface as the original connection.
+; If not specified, the FTP server will attempt to determine the address of the interface
+; that matches the client's connection and use that, which will likely not work behind NAT.
+;
+;public_ip = 192.0.2.21 ; Public IP address to advertise in PASV, for clients connecting with public IP addresses
+;private_ip = 10.1.1.21 ; Private IP address to advertise in PASV, for clients connecting with private IP addresses

include/socket.h

@@ -187,12 +187,13 @@ void bbs_tcp_listener2(int socket, int socket2, const char *name, const char *na
 void bbs_tcp_listener3(int socket, int socket2, int socket3, const char *name, const char *name2, const char *name3, void *(*handler)(void *varg), void *module);
 
 /*!
- * \brief Get local IP address
- * \param buf
+ * \brief Get local IP address of the BBS itself, i.e. the IP address to which a connection was established
+ * \param node Node to fetch the IP address associated with the interface being used by this node, NULL to get the default one.
+ * \param[out] buf
  * \param len
  * \retval 0 on success, -1 on failure
  */
-int bbs_get_local_ip(char *buf, size_t len);
+int bbs_get_local_ip(struct bbs_node *node, char *buf, size_t len);
 
 /*!
  * \brief Get the hostname of an IP address

nets/net_ftp.c

@@ -47,6 +47,8 @@ static int minport, maxport;
 
 static int ftp_port = DEFAULT_FTP_PORT;
 static int ftps_port = DEFAULT_FTPS_PORT;
+static char pasv_address_public[64] = "";
+static char pasv_address_private[64] = "";
 
 static int ftps_enabled = 0;
 
@@ -250,6 +252,27 @@ static ssize_t ftp_put(struct ftp_session *ftp, int *pasvfdptr, const char *full
 	return res;
 }
 
+static int get_local_ip(struct bbs_node *node, char *buf, size_t len)
+{
+	if (bbs_ip_is_private_ipv4(node->ip)) {
+		if (!s_strlen_zero(pasv_address_private)) {
+			safe_strncpy(buf, pasv_address_private, len);
+			return 0;
+		}
+	} else {
+		if (!s_strlen_zero(pasv_address_public)) {
+			safe_strncpy(buf, pasv_address_public, len);
+			return 0;
+		}
+	}
+	/* As default, try to determine the address based on the interface
+	 * through which the client connected to us. */
+	if (!bbs_get_local_ip(node, buf, len)) { /* Determine it just once, now */
+		return 0;
+	}
+	return -1;
+}
+
 static void *ftp_handler(void *varg)
 {
 	char buf[512];
@@ -270,7 +293,7 @@ static void *ftp_handler(void *varg)
 
 	bbs_node_net_begin(node);
 
-	if (bbs_get_local_ip(our_ip, sizeof(our_ip))) { /* Determine it just once, now */
+	if (get_local_ip(node, our_ip, sizeof(our_ip))) { /* Determine it just once, now */
 		goto cleanup;
 	}
 
@@ -869,6 +892,8 @@ static int load_config(void)
 	maxport = 20000;
 	bbs_config_val_set_port(cfg, "pasv", "minport", &minport);
 	bbs_config_val_set_port(cfg, "pasv", "maxport", &maxport);
+	bbs_config_val_set_str(cfg, "pasv", "public_ip", pasv_address_public, sizeof(pasv_address_public));
+	bbs_config_val_set_str(cfg, "pasv", "private_ip", pasv_address_private, sizeof(pasv_address_private));
 
 	if (ftps_enabled && !ssl_available()) {
 		bbs_error("TLS is not available, FTPS may not be used\n");

nets/net_irc.c

@@ -3902,7 +3902,7 @@ static int load_config(void)
 	if (s_strlen_zero(irc_hostname)) {
 		safe_strncpy(irc_hostname, bbs_hostname(), sizeof(irc_hostname)); /* Default to BBS hostname */
 		if (s_strlen_zero(irc_hostname)) {
-			if (bbs_get_local_ip(irc_hostname, sizeof(irc_hostname))) {
+			if (bbs_get_local_ip(NULL, irc_hostname, sizeof(irc_hostname))) {
 				bbs_error("No IRC or BBS hostname specified, and unable to determine local IP address. Aborting.\n");
 				return -1;
 			}