Set source address for UDP packets from multihomed server

Hi.
This is the first of 4 patches that I would like to offer for openvpn.
They are against 2.0_beta7.

This first one is a refinement of a patch I posted earlier which, at that time, didn't work.
The purpose of the patch is to use the IP_PKTINFO socket option to get and set the local address for UDP packets being sent or received. This means that if you have a multihomed server, then packets sent to it will always come from the address that they corresponding packets were sent to. This is needed to pass cleanly through a NAT.

In replaces "struct sockaddr_in from" in "struct context_2" with
struct sockaddr_in incoming_from, incoming_to
and records the address provided by IP_PKTINFO in incoming_to.

It adds "struct sockaddr_in actuallocal" alongside "struct sockaddr_in actual" in "struct lock_socket_addr", for storing the local enpoint if a udp "connection" in this structure.

It adds "struct sockaddr_in local_addr" alongside "remote_addr" in "struct key_state" for a similar purpose.

And it makes sure these fields are set and used appropriately.

The main reason that it didn't work before was that in multi_process_incoming_link, when the packet and address are transferred from the "mt" structure into the per-session context structure, I wasn't copying the incoming_to address as well. I do now.

One part of the change that I am not completely happy with is

socket.h renamed from socket.h~current~ +2 -0

+      /* alway set localaddr address */
+      info->lsa->actuallocal = *localaddr;

in link_socket_set_outgoing_addr.
The current code only changes the outgoing address if it has changed, and it does that inside link_socket_connection_initiated. However it is possible for the localaddress to change without the remote address changing.
e.g. If I stop and restart my openvpn instance on my notebook, and it gets a different IP address via DNS lookup. In this case it will appear to be a continuation of the same session, but will need to use a different local address. I think it is OK to change the localaddress on every packet, but I am open to be convinced otherwise, or that the code could be cleaner.

Anyway, here is the patch. I hope it can be included in an upcoming beta of openvpn.

Thanks,
NeilBrown

============================================

### Diffstat output

 ./forward.c |   23 ++++++++++++--------
 ./multi.c   |   10 +++++---
 ./openvpn.h |    5 +++-
 ./socket.c  |   46 ++++++++++++++++++++++++++++++++++++++++-
 ./socket.h  |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++----------
 ./ssl.c     |   25 +++++++++++++++++-----
 ./ssl.h     |    4 +++
 7 files changed, 148 insertions(+), 32 deletions(-)

diff ./forward.c~current~ ./forward.c

forward.c renamed from forward.c~current~ +2 -1

@@ -87,6 +87,7 @@ check_tls_dowork (struct context *c)
     {
       if (tls_multi_process
 	  (c->c2.tls_multi, &c->c2.to_link, &c->c2.to_link_addr,
+	   &c->c2.from_link_addr,
 	   get_link_socket_info (c), &wakeup))
 	{
 	  update_time ();

@@ -342,7 +343,7 @@ encrypt_sign (struct context *c, bool co
    * Get the address we will be sending the packet to.
    */
   link_socket_get_outgoing_addr (&c->c2.buf, get_link_socket_info (c),
-				 &c->c2.to_link_addr);
+				 &c->c2.to_link_addr, &c->c2.from_link_addr);
 #ifdef USE_CRYPTO
 #ifdef USE_SSL
   /*

forward.c renamed from forward.c~current~ +2 -1

+			     &c->c2.incoming_from, &c->c2.incoming_to);
 
   if (socket_connection_reset (c->c2.link_socket, status))
     {

@@ -561,7 +563,7 @@ process_incoming_link (struct context *c
   msg (D_LINK_RW, "%s READ [%d] from %s: %s",
        proto2ascii (lsi->proto, true),
        BLEN (&c->c2.buf),
-       print_sockaddr (&c->c2.from, &gc),
+       print_sockaddr (&c->c2.incoming_from, &gc),
        PROTO_DUMP (&c->c2.buf, &gc));
 
   /*

forward.c renamed from forward.c~current~ +2 -1

       /* reset packet received timer */
       if (c->options.ping_rec_timeout && c->c2.buf.len > 0)

@@ -826,7 +830,8 @@ process_outgoing_link (struct context *c
 	    socks_preprocess_outgoing_link (c, &to_addr, &size_delta);
 
 	    /* Send packet */
-	    size = link_socket_write (c->c2.link_socket, &c->c2.to_link, to_addr);
+	    size = link_socket_write (c->c2.link_socket, &c->c2.to_link, to_addr,
+				      &c->c2.from_link_addr);
 
 	    /* Undo effect of prepend */
 	    link_socket_write_post_size_adjust (&size, size_delta, &c->c2.to_link);

diff ./multi.c~current~ ./multi.c

multi.c renamed from multi.c~current~ +1 -1

@@ -544,7 +544,7 @@ multi_get_create_instance (struct multi_
   struct multi_instance *mi = NULL;
   struct hash *hash = mt->multi->hash;
 
-  if (mroute_extract_sockaddr_in (&real, &mt->top.c2.from, true))
+  if (mroute_extract_sockaddr_in (&real, &mt->top.c2.incoming_from, true))
     {
       struct hash_element *he;
       const uint32_t hv = hash_value (hash, &real);

openvpn.h renamed from openvpn.h~current~ +4 -1

   struct sockaddr_in to_link_addr;	 /* IP address of remote */
-  struct sockaddr_in from;               /* address of incoming datagram */
+  struct sockaddr_in from_link_addr;	 /* Address to send messages from */
+
+  struct sockaddr_in incoming_from;      /* address of incoming datagram */
+  struct sockaddr_in incoming_to;	 /* address incoming datagram was to */
 
   /* MTU frame parameters */
   struct frame frame;
 

diff ./socket.c~current~ ./socket.c

socket.c renamed from socket.c~current~ +45 -1

@@ -392,6 +392,12 @@ create_socket_udp (void)
 
   if ((sd = socket (PF_INET, SOCK_DGRAM, IPPROTO_UDP)) < 0)
     msg (M_SOCKERR, "UDP: Cannot create UDP socket");
+#ifdef IP_PKTINFO
+  else {
+	  int pad = 1;
+	  setsockopt(sd, SOL_IP, IP_PKTINFO, (void*)&pad, sizeof(pad));
+  }
+#endif
   return sd;
 }
 

@@ -1135,6 +1141,7 @@ void
 link_socket_connection_initiated (const struct buffer *buf,
 				  struct link_socket_info *info,
 				  const struct sockaddr_in *addr,
+				  const struct sockaddr_in *localaddr,
 				  const char *common_name)
 {
   struct gc_arena gc = gc_new ();

@@ -1143,6 +1150,7 @@ link_socket_connection_initiated (const 
   mutex_lock_static (L_SCRIPT);
 
   info->lsa->actual = *addr; /* Note: skip this line for --force-dest */
+  info->lsa->actuallocal = *localaddr;
   setenv_trusted (info);
   info->connection_established = true;
 

@@ -1582,13 +1590,49 @@ int
 link_socket_read_udp_posix (struct link_socket *sock,
 			    struct buffer *buf,
 			    int maxsize,
-			    struct sockaddr_in *from)
+			    struct sockaddr_in *from,
+			    struct sockaddr_in *to)
 {
   socklen_t fromlen = sizeof (*from);
   CLEAR (*from);
+  CLEAR (*to);
   ASSERT (buf_safe (buf, maxsize));
+#ifdef IP_PKTINFO
+  {
+    struct iovec iov;
+    char b[sizeof(struct cmsghdr)+sizeof(struct in_pktinfo)];
+    struct msghdr mesg;
+    iov.iov_base = BPTR (buf);
+    iov.iov_len = maxsize;
+    mesg.msg_iov = &iov;
+    mesg.msg_iovlen = 1;
+    mesg.msg_name = from;
+    mesg.msg_namelen = fromlen;
+    mesg.msg_control = b;
+    mesg.msg_controllen = sizeof(b);
+    CLEAR(*to);
+    buf->len = recvmsg (sock->sd, &mesg, 0);
+    if (buf->len >= 0) 
+     {
+       struct cmsghdr *cmsg;
+       fromlen = mesg.msg_namelen;
+       cmsg = CMSG_FIRSTHDR (&mesg);
+       if (cmsg != NULL
+	   && CMSG_NXTHDR (&mesg, cmsg) == NULL
+	   && cmsg->cmsg_level == SOL_IP 
+	   && cmsg->cmsg_type == IP_PKTINFO
+	   && cmsg->cmsg_len >= (sizeof (struct cmsghdr)
+				 + sizeof (struct in_pktinfo))) 
+        {
+	       struct in_pktinfo *pkti = (struct in_pktinfo *) CMSG_DATA (cmsg);
+	       to->sin_addr = pkti->ipi_addr;
+        }
+      }
+  }
+#else
   buf->len = recvfrom (sock->sd, BPTR (buf), maxsize, 0,
 		       (struct sockaddr *) from, &fromlen);
+#endif
   if (fromlen != sizeof (*from))
     bad_address_length (fromlen, sizeof (*from));
   return buf->len;

diff ./socket.h~current~ ./socket.h

socket.h renamed from socket.h~current~ +53 -9

@@ -70,6 +70,7 @@ struct link_socket_addr
   struct sockaddr_in local;
   struct sockaddr_in remote; /* initial remote */
   struct sockaddr_in actual; /* remote may change due to --float */
+  struct sockaddr_in actuallocal; /* make sure we reply from  right address */
 };
 
 struct link_socket_info

@@ -273,6 +274,7 @@ in_addr_t link_socket_current_remote (co
 void link_socket_connection_initiated (const struct buffer *buf,
 				       struct link_socket_info *info,
 				       const struct sockaddr_in *addr,
+				       const struct sockaddr_in *localaddr,
 				       const char *common_name);
 
 void link_socket_bad_incoming_addr (struct buffer *buf,

@@ -443,7 +445,8 @@ link_socket_verify_incoming_addr (struct
 static inline void
 link_socket_get_outgoing_addr (struct buffer *buf,
 			      const struct link_socket_info *info,
-			      struct sockaddr_in *addr)
+			      struct sockaddr_in *addr,
+			      struct sockaddr_in *fromaddr)
 {
   if (buf->len > 0)
     {

@@ -453,6 +456,10 @@ link_socket_get_outgoing_addr (struct bu
 	  addr->sin_family = lsa->actual.sin_family;
 	  addr->sin_addr.s_addr = lsa->actual.sin_addr.s_addr;
 	  addr->sin_port = lsa->actual.sin_port;
+
+	  fromaddr->sin_family = lsa->actuallocal.sin_family;
+	  fromaddr->sin_addr.s_addr = lsa->actuallocal.sin_addr.s_addr;
+	  fromaddr->sin_port = lsa->actuallocal.sin_port;
 	}
       else
 	{

@@ -466,6 +473,7 @@ static inline void
 link_socket_set_outgoing_addr (const struct buffer *buf,
 			       struct link_socket_info *info,
 			       const struct sockaddr_in *addr,
+			       const struct sockaddr_in *localaddr,
 			       const char *common_name)
 {
   if (!buf || buf->len > 0)

@@ -481,8 +489,10 @@ link_socket_set_outgoing_addr (const str
 	      || addr_match_proto (addr, &lsa->remote, info->proto))
 	  )
 	{
-	  link_socket_connection_initiated (buf, info, addr, common_name);
+	  link_socket_connection_initiated (buf, info, addr, localaddr, common_name);
 	}
+      /* alway set localaddr address */
+      info->lsa->actuallocal = *localaddr;
     }
 }
 

@@ -528,7 +538,8 @@ link_socket_read_udp_win32 (struct link_
 int link_socket_read_udp_posix (struct link_socket *sock,
 				struct buffer *buf,
 				int maxsize,
-				struct sockaddr_in *from);
+				struct sockaddr_in *from,
+				struct sockaddr_in *to);
 
 #endif
 

@@ -537,16 +548,17 @@ static inline int
 link_socket_read (struct link_socket *sock,
 		  struct buffer *buf,
 		  int maxsize,
-		  struct sockaddr_in *from)
+		  struct sockaddr_in *from,
+		  struct sockaddr_in *to)
 {
   if (sock->info.proto == PROTO_UDPv4)
     {
       int res;
-
+      CLEAR (*to);
 #ifdef WIN32
       res = link_socket_read_udp_win32 (sock, buf, from);
 #else
-      res = link_socket_read_udp_posix (sock, buf, maxsize, from);
+      res = link_socket_read_udp_posix (sock, buf, maxsize, from, to);
 #endif
       return res;
     }

@@ -601,8 +613,39 @@ link_socket_write_win32 (struct link_soc
 static inline int
 link_socket_write_udp_posix (struct link_socket *sock,
 			     struct buffer *buf,
-			     struct sockaddr_in *to)
+			     struct sockaddr_in *to,
+			     struct sockaddr_in *from)
 {
+#ifdef IP_PKTINFO
+ if (addr_defined(from)) 
+   {
+     struct iovec iov;
+     struct msghdr mesg;
+     struct cmsghdr *cmsg;
+     struct in_pktinfo *pkti;
+
+     char b[sizeof(struct cmsghdr)+sizeof(struct in_pktinfo)];
+     iov.iov_base = BPTR (buf);
+     iov.iov_len = BLEN (buf);
+     mesg.msg_iov = &iov;
+     mesg.msg_iovlen = 1;
+     mesg.msg_name = to;
+     mesg.msg_namelen = sizeof(*to);
+     mesg.msg_control = b;
+     mesg.msg_controllen = sizeof(b);
+     mesg.msg_flags = 0;
+     cmsg = CMSG_FIRSTHDR(&mesg);
+     cmsg->cmsg_len = sizeof(b);
+     cmsg->cmsg_level = SOL_IP;
+     cmsg->cmsg_type = IP_PKTINFO;
+     pkti = (struct in_pktinfo *)CMSG_DATA(cmsg);
+     pkti->ipi_ifindex = 0;
+     pkti->ipi_spec_dst = from->sin_addr;
+     pkti->ipi_addr = from->sin_addr;
+
+     return sendmsg (sock->sd, &mesg, 0);
+   }
+#endif
   return sendto (sock->sd, BPTR (buf), BLEN (buf), 0,
 		 (struct sockaddr *) to,
 		 (socklen_t) sizeof (*to));

@@ -621,12 +664,13 @@ link_socket_write_tcp_posix (struct link
 static inline int
 link_socket_write_udp (struct link_socket *sock,
 		       struct buffer *buf,
-		       struct sockaddr_in *to)
+		       struct sockaddr_in *to,
+		       struct sockaddr_in *from)
 {
 #ifdef WIN32
   return link_socket_write_win32 (sock, buf, to);
 #else
-  return link_socket_write_udp_posix (sock, buf, to);
+  return link_socket_write_udp_posix (sock, buf, to, from);
 #endif
 }
 

@@ -634,11 +678,12 @@ link_socket_write_udp (struct link_socke
 static inline int
 link_socket_write (struct link_socket *sock,
 		   struct buffer *buf,

socket.h renamed from socket.h~current~ +0 -0

     {

diff ./ssl.c~current~ ./ssl.c

ssl.c renamed from ssl.c~current~ +4 -0

@@ -1494,6 +1494,7 @@ write_control_auth (struct tls_session *
 		    struct key_state *ks,
 		    struct buffer *buf,
 		    struct sockaddr_in *to_link_addr,
+		    struct sockaddr_in *from_link_addr,
 		    int opcode,
 		    int max_ack,
 		    bool prepend_ack)

@@ -1514,6 +1515,7 @@ write_control_auth (struct tls_session *
       ASSERT (swap_hmac (buf, &session->tls_auth, false));
     }
   *to_link_addr = ks->remote_addr;
+  *from_link_addr = ks->local_addr;
 }
 
 /*

@@ -1916,6 +1918,7 @@ key_state_soft_reset (struct tls_session
   key_state_init (session, ks);
   ks->session_id_remote = ks_lame->session_id_remote;
   ks->remote_addr = ks_lame->remote_addr;
+  ks->local_addr = ks_lame->local_addr;
 }
 
 /*

@@ -1932,6 +1935,7 @@ tls_process (struct tls_multi *multi,
 	     struct tls_session *session,
 	     struct buffer *to_link,
 	     struct sockaddr_in *to_link_addr,
+	     struct sockaddr_in *from_link_addr,
 	     struct link_socket_info *to_link_socket_info,
 	     interval_t *wakeup)
 {

ssl.c renamed from ssl.c~current~ +6 -2

+		  link_socket_set_outgoing_addr (NULL, to_link_socket_info, &ks->remote_addr,
+						 &ks->local_addr, session->common_name);
 
 #ifdef MEASURE_TLS_HANDSHAKE_STATS
 		  show_tls_performance_stats();

@@ -2072,7 +2077,8 @@ tls_process (struct tls_multi *multi,
 	      b = *buf;
 	      INCR_SENT;
 
-	      write_control_auth (session, ks, &b, to_link_addr, opcode,
+	      write_control_auth (session, ks, &b, to_link_addr, 
+				  from_link_addr, opcode,
 				  CONTROL_SEND_ACK_MAX, true);
 	      *to_link = b;
 	      active = true;

@@ -2087,7 +2093,8 @@ tls_process (struct tls_multi *multi,
 	    {
 	      buf = &ks->ack_write_buf;
 	      ASSERT (buf_init (buf, FRAME_HEADROOM (&multi->opt.frame)));
-	      write_control_auth (session, ks, buf, to_link_addr, P_ACK_V1,
+	      write_control_auth (session, ks, buf, to_link_addr, from_link_addr,
+				  P_ACK_V1,
 				  RELIABLE_ACK_SIZE, false);
 	      *to_link = *buf;
 	      active = true;

ssl.c renamed from ssl.c~current~ +1 -0

 			  RELIABLE_ACK_SIZE, false);
       *to_link = *buf;
       active = true;

@@ -2453,6 +2460,7 @@ bool
 tls_multi_process (struct tls_multi *multi,
 		   struct buffer *to_link,
 		   struct sockaddr_in *to_link_addr,
+		   struct sockaddr_in *from_link_addr,
 		   struct link_socket_info *to_link_socket_info,
 		   interval_t *wakeup)
 {

@@ -2473,8 +2481,10 @@ tls_multi_process (struct tls_multi *mul
 

ssl.c renamed from ssl.c~current~ +4 -1

@@ -2488,7 +2498,7 @@ tls_multi_process (struct tls_multi *mul
 	{
 	  update_time ();
 
-	  if (tls_process (multi, session, to_link, to_link_addr,
+	  if (tls_process (multi, session, to_link, to_link_addr, from_link_addr,
 			   to_link_socket_info, wakeup))
 	    active = true;
 

@@ -2617,6 +2627,7 @@ tls_rec_payload (struct tls_multi *multi
 bool
 tls_pre_decrypt (struct tls_multi *multi,
 		 struct sockaddr_in *from,
+		 struct sockaddr_in *to,
 		 struct buffer *buf,
 		 struct crypto_options *opt)
 {

@@ -2919,6 +2930,7 @@ tls_pre_decrypt (struct tls_multi *multi
 	      {
 		ks->session_id_remote = sid;
 		ks->remote_addr = *from;
+		ks->local_addr = *to;
 		++multi->n_sessions;
 	      }
 	    else if (!addr_port_match (&ks->remote_addr, from))

@@ -3023,6 +3035,7 @@ tls_pre_decrypt (struct tls_multi *multi
 bool
 tls_pre_decrypt_lite (const struct tls_auth_standalone *tas,
 		      const struct sockaddr_in *from,
+		      const struct sockaddr_in *to,
 		      const struct buffer *buf)
 {
   struct gc_arena gc = gc_new ();

diff ./ssl.h~current~ ./ssl.h

ssl.h renamed from ssl.h~current~ +3 -0

@@ -456,6 +457,7 @@ void tls_multi_init_set_options(struct t
 bool tls_multi_process (struct tls_multi *multi,
 			struct buffer *to_link,
 			struct sockaddr_in *to_link_addr,
+			struct sockaddr_in *from_link_addr,
 			struct link_socket_info *to_link_socket_info,
 			interval_t *wakeup);
 

@@ -463,11 +465,13 @@ void tls_multi_free (struct tls_multi *m
 
 bool tls_pre_decrypt (struct tls_multi *multi,
 		      struct sockaddr_in *from,
+		      struct sockaddr_in *to,
 		      struct buffer *buf,
 		      struct crypto_options *opt);
 
 bool tls_pre_decrypt_lite (const struct tls_auth_standalone *tas,
 			   const struct sockaddr_in *from,
+			   const struct sockaddr_in *to,
 			   const struct buffer *buf);
 
 void tls_pre_encrypt (struct tls_multi *multi,

Neil,

Thanks for the patches... here are some comments:

* You've obviously put a lot of effort into understanding the code to write this patch. But did you consider any alternative approaches, such as starting multiple OpenVPN instances and binding each to a separate local interface?