Fixes and refactoring for SOCKS5 UDP.

Author: ambrop72

compile-tun2socks.sh

@@ -61,6 +61,7 @@ system/BConnection_common.c
 system/BTime.c
 system/BUnixSignal.c
 system/BNetwork.c
+system/BDatagram_common.c
 system/BDatagram_unix.c
 flow/StreamRecvInterface.c
 flow/PacketRecvInterface.c
@@ -109,6 +110,7 @@ base/BPending.c
 flowextra/PacketPassInactivityMonitor.c
 tun2socks/SocksUdpGwClient.c
 udpgw_client/UdpGwClient.c
+socks_udp_client/SocksUdpClient.c
 "
 
 set -e

socks_udp_client/CMakeLists.txt

@@ -1 +1 @@
-badvpn_add_library(socks_udp_client "system;flow;flowextra" "" SocksUdpClient.c)
+badvpn_add_library(socks_udp_client "base;system;flow;flowextra;socksclient" "" SocksUdpClient.c)

socks_udp_client/SocksUdpClient.c

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2018 Jigsaw Operations LLC
+ * Copyright (C) 2019 Ambroz Bizjak (modifications)
  * 
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -27,37 +28,35 @@
 #ifndef BADVPN_SOCKS_UDP_CLIENT_SOCKSUDPCLIENT_H
 #define BADVPN_SOCKS_UDP_CLIENT_SOCKSUDPCLIENT_H
 
+#include <stddef.h>
 #include <stdint.h>
 
 #include <base/BPending.h>
 #include <base/DebugObject.h>
 #include <flow/BufferWriter.h>
 #include <flow/PacketBuffer.h>
 #include <flow/SinglePacketBuffer.h>
+#include <flow/PacketPassInterface.h>
 #include <flowextra/PacketPassInactivityMonitor.h>
-#include <misc/debug.h>
-#include <misc/socks_proto.h>
 #include <socksclient/BSocksClient.h>
 #include <structure/BAVL.h>
 #include <system/BAddr.h>
 #include <system/BDatagram.h>
 #include <system/BReactor.h>
 #include <system/BTime.h>
 
-// This sets the number of packets to accept while waiting for SOCKS server to authenticate and
-// connect.  A slow or far-away SOCKS server could require 300 ms to connect, and a chatty
-// client (e.g. STUN) could send a packet every 20 ms, so a limit of 16 seems reasonable.
-#define SOCKS_UDP_SEND_BUFFER_PACKETS 16
-
-typedef void (*SocksUdpClient_handler_received) (void *user, BAddr local_addr, BAddr remote_addr, const uint8_t *data, int data_len);
+typedef void (*SocksUdpClient_handler_received) (
+    void *user, BAddr local_addr, BAddr remote_addr, const uint8_t *data, int data_len);
 
 typedef struct {
     BAddr server_addr;
     const struct BSocksClient_auth_info *auth_info;
     size_t num_auth_info;
     int num_connections;
     int max_connections;
+    int send_buf_size;
     int udp_mtu;
+    int socks_mtu;
     btime_t keepalive_time;
     BReactor *reactor;
     void *user;
@@ -77,8 +76,8 @@ struct SocksUdpClient_connection {
     BDatagram socket;
     PacketPassInterface recv_if;
     SinglePacketBuffer recv_buffer;
-    // The first_* members represent the initial packet, which has to be stored so it can wait for
-    // send_writer to become ready.
+    // The first_* members represent the initial packet, which has to be stored so it can
+    // wait for send_writer to become ready.
     uint8_t *first_data;
     int first_data_len;
     BAddr first_remote_addr;
@@ -92,43 +91,59 @@ struct SocksUdpClient_connection {
 
 /**
  * Initializes the SOCKS5-UDP client object.
- * This function does not perform network access, so it will always succeed if the arguments
- * are valid.
+ * 
+ * This function only initialzies the object and does not perform network access.
  * 
  * Currently, this function only supports connection to a SOCKS5 server that is routable from
- * localhost (i.e. running on the local machine).  It may be possible to add support for remote
- * servers, but SOCKS5 does not support UDP if there is a NAT or firewall between the client
- * and the proxy.
+ * localhost (i.e. running on the local machine). It may be possible to add support for
+ * remote servers, but SOCKS5 does not support UDP if there is a NAT or firewall between the
+ * client and the proxy.
  * 
  * @param o the object
  * @param udp_mtu the maximum size of packets that will be sent through the tunnel
  * @param max_connections how many local ports to track before dropping packets
+ * @param send_buf_size maximum number of buffered outgoing packets per connection
  * @param keepalive_time how long to track an idle local port before forgetting it
  * @param server_addr SOCKS5 server address.  MUST BE ON LOCALHOST.
+ * @param auth_info List of authentication info for BSocksClient. The pointer must remain
+ *        valid while this object exists, the data is not copied.
+ * @param num_auth_info Number of the above.
  * @param reactor reactor we live in
  * @param user value passed to handler
  * @param handler_received handler for incoming UDP packets
+ * @return 1 on success, 0 on failure
+ */
+int SocksUdpClient_Init (SocksUdpClient *o, int udp_mtu, int max_connections,
+    int send_buf_size, btime_t keepalive_time, BAddr server_addr,
+    const struct BSocksClient_auth_info *auth_info, size_t num_auth_info,
+    BReactor *reactor, void *user, SocksUdpClient_handler_received handler_received);
+
+/**
+ * Frees the SOCKS5-UDP client object.
+ *
+ * @param o the object
  */
-void SocksUdpClient_Init (SocksUdpClient *o, int udp_mtu, int max_connections, btime_t keepalive_time,
-                          BAddr server_addr, const struct BSocksClient_auth_info *auth_info, size_t num_auth_info,
-                          BReactor *reactor, void *user, SocksUdpClient_handler_received handler_received);
 void SocksUdpClient_Free (SocksUdpClient *o);
 
 /**
  * Submit a packet to be sent through the proxy.
  *
  * This will reuse an existing connection for packets from local_addr, or create one if
- * there is none.  If the number of live connections exceeds max_connections, or if the number of
- * buffered packets from this port exceeds a limit, packets will be dropped silently.
+ * there is none. If the number of live connections exceeds max_connections, or if the
+ * number of buffered packets from this port exceeds a limit, packets will be dropped
+ * silently.
  * 
- * As a resource optimization, if a connection has only been used to send one DNS query, then
- * the connection will be closed and freed once the reply is received.
+ * As a resource optimization, if a connection has only been used to send one DNS query,
+ * then the connection will be closed and freed once the reply is received.
  * 
  * @param o the object
- * @param local_addr the UDP packet's source address, and the expected destination for replies
+ * @param local_addr the UDP packet's source address, and the expected destination for
+ *        replies
  * @param remote_addr the destination of the packet after it exits the proxy
- * @param data the packet contents.  Caller retains ownership.
+ * @param data the packet contents. Caller retains ownership.
+ * @param data_len number of bytes in the data
  */
-void SocksUdpClient_SubmitPacket (SocksUdpClient *o, BAddr local_addr, BAddr remote_addr, const uint8_t *data, int data_len);
+void SocksUdpClient_SubmitPacket (SocksUdpClient *o,
+    BAddr local_addr, BAddr remote_addr, const uint8_t *data, int data_len);
 
 #endif

socks_udp_client/SocksUdpClient.h

@@ -375,14 +375,16 @@ void recv_handler_done (BSocksClient *o, int data_len)
             void *addr_buffer = o->buffer + sizeof(struct socks_reply_header);
             switch (o->bind_addr.type) {
                 case BADDR_TYPE_IPV4: {
-                    struct socks_addr_ipv4 *ip4 = addr_buffer;
-                    o->bind_addr.ipv4.ip = ip4->addr;
-                    o->bind_addr.ipv4.port = ip4->port;
+                    struct socks_addr_ipv4 ip4;
+                    memcpy(&ip4, addr_buffer, sizeof(ip4));
+                    o->bind_addr.ipv4.ip = ip4.addr;
+                    o->bind_addr.ipv4.port = ip4.port;
                 } break;
                 case BADDR_TYPE_IPV6: {
-                    struct socks_addr_ipv6 *ip6 = addr_buffer;
-                    memcpy(o->bind_addr.ipv6.ip, ip6->addr, sizeof(ip6->addr));
-                    o->bind_addr.ipv6.port = ip6->port;
+                    struct socks_addr_ipv6 ip6;
+                    memcpy(&ip6, addr_buffer, sizeof(ip6));
+                    memcpy(o->bind_addr.ipv6.ip, ip6.addr, sizeof(ip6.addr));
+                    o->bind_addr.ipv6.port = ip6.port;
                 } break;
                 default: ASSERT(0);
             }
@@ -395,6 +397,8 @@ void recv_handler_done (BSocksClient *o, int data_len)
             free_control_io(o);
 
             // init up I/O
+            // Initializing this is not needed for UDP ASSOCIATE but it doesn't hurt.
+            // We anyway don't allow the user to use these interfaces in that case.
             init_up_io(o);
 
             // set state
@@ -614,9 +618,18 @@ void BSocksClient_Free (BSocksClient *o)
     }
 }
 
+BAddr BSocksClient_GetBindAddr (BSocksClient *o)
+{
+    ASSERT(o->state == STATE_UP)
+    DebugObject_Access(&o->d_obj);
+
+    return o->bind_addr;
+}
+
 StreamPassInterface * BSocksClient_GetSendInterface (BSocksClient *o)
 {
     ASSERT(o->state == STATE_UP)
+    ASSERT(!o->udp)
     DebugObject_Access(&o->d_obj);
 
     return BConnection_SendAsync_GetIf(&o->con);
@@ -625,6 +638,7 @@ StreamPassInterface * BSocksClient_GetSendInterface (BSocksClient *o)
 StreamRecvInterface * BSocksClient_GetRecvInterface (BSocksClient *o)
 {
     ASSERT(o->state == STATE_UP)
+    ASSERT(!o->udp)
     DebugObject_Access(&o->d_obj);
 
     return BConnection_RecvAsync_GetIf(&o->con);

socksclient/BSocksClient.c

@@ -34,6 +34,7 @@
 #ifndef BADVPN_SOCKS_BSOCKSCLIENT_H
 #define BADVPN_SOCKS_BSOCKSCLIENT_H
 
+#include <stddef.h>
 #include <stdint.h>
 #include <stdbool.h>
 
@@ -112,7 +113,14 @@ struct BSocksClient_auth_info BSocksClient_auth_password (const char *username,
  * 
  * @param o the object
  * @param server_addr SOCKS5 server address
+ * @param auth_info List of supported authentication methods and associated parameters.
+ *        Initialize these using functions such as BSocksClient_auth_none() and
+ *        BSocksClient_auth_password(). The pointer must remain valid while this object
+ *        exists, the data is not copied.
+ * @param num_auth_info Number of the above. There should be at least one, otherwise it
+ *        certainly won't work.
  * @param dest_addr remote address
+ * @param udp whether to do UDP ASSOCIATE instead of CONNECT
  * @param handler handler for up and error events
  * @param user value passed to handler
  * @param reactor reactor we live in
@@ -129,9 +137,20 @@ int BSocksClient_Init (BSocksClient *o,
  */
 void BSocksClient_Free (BSocksClient *o);
 
+/**
+ * Return the bind address that the SOCKS server reported.
+ * The object must be in up state. The bind address is needed for UDP ASSOCIATE
+ * because it is the address that the client should send UDP packets to.
+ *
+ * @param o the object
+ * @return The bind address, of type BADDR_TYPE_IPV4 or BADDR_TYPE_IPV6.
+ */
+BAddr BSocksClient_GetBindAddr (BSocksClient *o);
+
 /**
  * Returns the send interface.
- * The object must be in up state.
+ * The object must be in up state. Additionally this must not be called if the
+ * object was initialized in UDP ASSOCIATE mode.
  * 
  * @param o the object
  * @return send interface
@@ -140,7 +159,8 @@ StreamPassInterface * BSocksClient_GetSendInterface (BSocksClient *o);
 
 /**
  * Returns the receive interface.
- * The object must be in up state.
+ * The object must be in up state. Additionally this must not be called if the
+ * object was initialized in UDP ASSOCIATE mode.
  * 
  * @param o the object
  * @return receive interface

socksclient/BSocksClient.h

@@ -97,6 +97,8 @@ static int BIPAddr_Resolve (BIPAddr *addr, char *str, int noresolve) WARN_UNUSED
 
 static int BIPAddr_Compare (BIPAddr *addr1, BIPAddr *addr2);
 
+static void BIPAddr_InitLocalhost (BIPAddr *addr, int addr_type);
+
 /**
  * Converts an IP address to human readable form.
  *
@@ -805,4 +807,20 @@ int BAddr_CompareOrder (BAddr *addr1, BAddr *addr2)
     }
 }
 
+void BIPAddr_InitLocalhost (BIPAddr *addr, int addr_type)
+{
+    if (addr_type == BADDR_TYPE_IPV4) {
+        addr->type = addr_type;
+        addr->ipv4 = hton32(0x7f000001);
+    }
+    else if (addr_type == BADDR_TYPE_IPV6) {
+        addr->type = addr_type;
+        memset(addr->ipv6, 0, 16);
+        addr->ipv6[15] = 1;
+    }
+    else {
+        addr->type = BADDR_TYPE_NONE;
+    }
+}
+
 #endif

system/BAddr.h

@@ -124,8 +124,22 @@ void BDatagram_SetSendAddrs (BDatagram *o, BAddr remote_addr, BIPAddr local_addr
 int BDatagram_GetLastReceiveAddrs (BDatagram *o, BAddr *remote_addr, BIPAddr *local_addr);
 
 /**
- * Returns the bound port.
- * Fails if and only if a port is not yet bound.
+ * Determines the local address.
+ * 
+ * This calls getsockname() to determine the local address and returns the result as
+ * BAddr. This function fails if the address cannot be determined or translated to
+ * BAddr (it never succeeds but returns a BADDR_TYPE_NONE address).
+ *
+ * @param o the object
+ * @param local_addr returns the local bound address.
+ * @return 1 on success, 0 on failure
+ */
+int BDatagram_GetLocalAddr (BDatagram *o, BAddr *local_addr);
+
+/**
+ * Returns the local port.
+ *
+ * This is a convenience function implemented based on BDatagram_GetLocalAddr.
  * 
  * @param o the object
  * @param local_port returns the local bound port.

system/BDatagram.h

@@ -0,0 +1,52 @@
+/**
+ * @file BDatagram_unix.c
+ * @author Ambroz Bizjak <ambrop7@gmail.com>
+ * 
+ * @section LICENSE
+ * 
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the author nor the
+ *    names of its contributors may be used to endorse or promote products
+ *    derived from this software without specific prior written permission.
+ * 
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <base/BLog.h>
+#include <system/BAddr.h>
+
+#include "BDatagram.h"
+
+#include <generated/blog_channel_BDatagram.h>
+
+int BDatagram_GetLocalPort (BDatagram *o, uint16_t *local_port)
+{
+    BAddr addr;
+    if (!BDatagram_GetLocalAddr(o, &addr)) {
+        return 0;
+    }
+
+    if (addr.type != BADDR_TYPE_IPV4 && addr.type != BADDR_TYPE_IPV6) {
+        BLog(BLOG_ERROR,
+            "BDatagram_GetLocalPort: Port not defined for this address type.");
+        return 0;
+    }
+
+    *local_port = BAddr_GetPort(&addr);
+    return 1;
+}