Skip to contentSkip to conversation

Cleanup patches

PATCH Applied Arne Schwabe 4 participants 30 messages Aug 1, 2012 – Sep 3, 2012

lore
Share & tools
lore

Arne Schwabe

author
#0 lore raw

cover Cleanup patches

The first patch is a corrected (read working) version of the patch I posted a while ago.

For the last three patches. I am in the progress of looking through the socket.c code. The ultimate goal is to simplify the code and have good way of dealing with ipv4/ipv6 dual stack. These first patches don't change any functionality but clean up the code.

Arne Schwabe (4):
Fixes error: --key fails with EXTERNAL_PRIVATE_KEY: No such file or

    directory if  --management-external-key is used

Merge getaddr_multi and getaddr6 into one function
Merge almost identical create_socket_tcp and create_socket_tcp6 Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

 src/openvpn/manage.c  |    4 +-
 src/openvpn/options.c |   18 +-
 src/openvpn/route.c   |   50 ++--
 src/openvpn/socket.c  |  724 ++++++++++++++++---------------------------------
 src/openvpn/socket.h  |   19 +-
 5 files changed, 280 insertions(+), 535 deletions(-)
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
author
#1 lore raw

PATCH 1/4 Fixes error: --key fails with EXTERNAL_PRIVATE_KEY: No such file or directory if --management-external-key is used

Warn if both options are used at the same time.

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/options.c |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
src/openvpn/options.c +16 -2
index 9f4ddbb..03dc82f 100644
@@ -62,6 +62,10 @@
 
 #include "memdbg.h"
 
+#ifdef MANAGMENT_EXTERNAL_KEY
+#define EXTERNAL_KEY_STRING "EXTERNAL_PRIVATE_KEY"
+#endif
+
 const char title_string[] =
   PACKAGE_STRING
   " " TARGET_ALIAS

@@ -2181,6 +2185,11 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
       else
 #endif
 #ifdef ENABLE_CRYPTOAPI
+#ifdef MANAGMENT_EXTERNAL_KEY
+    if((options->management_flags & MF_EXTERNAL_KEY) && !strcmp(options->priv_key_file,EXTERNAL_KEY_STRING)==0)
+		msg (M_USAGE, "--key and --management-external-key are mutually exclusive");
+#endif
+
       if (options->cryptoapi_cert)
 	{
 	  if ((!(options->ca_file)) && (!(options->ca_path)))

@@ -2627,7 +2636,10 @@ options_postprocess_filechecks (struct options *options)
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->cert_file, R_OK, "--cert");
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->extra_certs_file, R_OK,
                              "--extra-certs");
-  errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->priv_key_file, R_OK,
+#ifdef MANAGMENT_EXTERNAL_KEY
+  if(!options->management_flags & MF_EXTERNAL_KEY)
+#endif
+     errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->priv_key_file, R_OK,
                              "--key");
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->pkcs12_file, R_OK,
                              "--pkcs12");

@@ -4141,7 +4153,9 @@ add_option (struct options *options,
     {
       VERIFY_PERMISSION (OPT_P_GENERAL);
       options->management_flags |= MF_EXTERNAL_KEY;
-      options->priv_key_file = "EXTERNAL_PRIVATE_KEY";
+	  /* Set priv key file name only if not defined, so --key and this option can be checked later */
+	  if(!options->priv_key_file)
+		  options->priv_key_file = EXTERNAL_KEY_STRING;
     }
 #endif
 #ifdef MANAGEMENT_DEF_AUTH
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
author
#2 lore raw

PATCH 1/4 Fixes error: --key fails with EXTERNAL_PRIVATE_KEY: No such file or directory if --management-external-key is used

Warn if both options are used at the same time.

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/options.c |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
src/openvpn/options.c +15 -2
index 9f4ddbb..39dceb8 100644
@@ -1581,6 +1581,12 @@ show_settings (const struct options *o)
   SHOW_STR (ca_path);
   SHOW_STR (dh_file);
   SHOW_STR (cert_file);
+
+#ifdef MANAGMENT_EXTERNAL_KEY
+  if((options->management_flags & MF_EXTERNAL_KEY))
+	SHOW_PARM ("priv_key_file","EXTERNAL_PRIVATE_KEY","%s");
+  else 
+#endif
   SHOW_STR (priv_key_file);
 #ifndef ENABLE_CRYPTO_POLARSSL
   SHOW_STR (pkcs12_file);

@@ -2181,6 +2187,11 @@ options_postprocess_verify_ce (const struct options *options, const struct conne
       else
 #endif
 #ifdef ENABLE_CRYPTOAPI
+#ifdef MANAGMENT_EXTERNAL_KEY
+	 if((options->management_flags & MF_EXTERNAL_KEY) && options->priv_key_file)
+		msg (M_USAGE, "--key and --management-external-key are mutually exclusive");
+#endif
+
       if (options->cryptoapi_cert)
 	{
 	  if ((!(options->ca_file)) && (!(options->ca_path)))

@@ -2627,7 +2638,10 @@ options_postprocess_filechecks (struct options *options)
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->cert_file, R_OK, "--cert");
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->extra_certs_file, R_OK,
                              "--extra-certs");
-  errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->priv_key_file, R_OK,
+#ifdef MANAGMENT_EXTERNAL_KEY
+  if(!options->management_flags & MF_EXTERNAL_KEY)
+#endif
+     errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->priv_key_file, R_OK,
                              "--key");
   errs |= check_file_access (CHKACC_FILE|CHKACC_INLINE, options->pkcs12_file, R_OK,
                              "--pkcs12");

@@ -4141,7 +4155,6 @@ add_option (struct options *options,
     {
       VERIFY_PERMISSION (OPT_P_GENERAL);
       options->management_flags |= MF_EXTERNAL_KEY;
-      options->priv_key_file = "EXTERNAL_PRIVATE_KEY";
     }
 #endif
 #ifdef MANAGEMENT_DEF_AUTH
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
#3 lore raw

Re: [Openvpn-devel] [PATCH 1/4] Fixes error: --key fails with EXTERNAL_PRIVATE_KEY: No such file or directory if --management-external-key is used

Hi,

Quoted 3 lines · Arne Schwabe

Warn if both options are used at the same time.

Signed-off-by Arne Schwabe <[email protected]...>

ACK on v2 of 1/4. Same effect, less complications.

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
#4 lore raw

Re: [Openvpn-devel] [PATCH 1/4] Fixes error: --key fails with EXTERNAL_PRIVATE_KEY: No such file or directory if --management-external-key is used

In reply to Arne Schwabe
Quoted 12 lines · Arne Schwabe

Warn if both options are used at the same time.

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/options.c |   17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)
src/openvpn/options.c +0 -0
index 9f4ddbb..39dceb8 100644

src/openvpn/options.c show_settings (const struct options *o) · line 1589

Quoted 9 lines
src/openvpn/options.c +6 -0
@@ -1581,6 +1581,12 @@ show_settings (const struct options *o)
   SHOW_STR (ca_path);
   SHOW_STR (dh_file);
   SHOW_STR (cert_file);
+
+#ifdef MANAGMENT_EXTERNAL_KEY
+  if((options->management_flags & MF_EXTERNAL_KEY))
+	SHOW_PARM ("priv_key_file","EXTERNAL_PRIVATE_KEY","%s");
+  else 
+#endif

Please test compile without any special ./configure settings too before submitting patches ;-) The piece of code above won't compile. Look at the function declaration, it's not 'options' but 'o' which needs to be used. This time I fixed it before pushing out the patch, just because it was too simple fix to insist on another patch.

So, applied to master - with a minor fix :)

commit 4806cc102655f1a829d656f6deb83e5953c5eab2

Author: Arne Schwabe <[email protected]...>
Date:   Thu Aug 2 15:37:48 2012 +0200
    Fixes error: --key fails with EXTERNAL_PRIVATE_KEY: No such file or directory if --management-external-key is used
        
    Signed-off-by: Arne Schwabe <[email protected]...>
    Acked-by: Gert Doering <[email protected]...>
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/6943

kind regards,

David Sommerseth

author
#5 lore raw

PATCH 2/4 Merge getaddr_multi and getaddr6 into one function

In reply to Arne Schwabe

the getaddr6 and getaddr_mutli functions are duplicates of each other. Since we always require getaddrinfo to be present both function are merge into one openvpn_getaddrinfo.

This functions also returns a standard struct addrinfo* so our resolve interface is closer to the standard unix interface. The getaddr function is a wrapper which provides backward compatibility for IPv4 addresses. Ipv6 calls and calls to getaddr_multi are replaced with the new interface.

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/route.c  |   50 ++---
 src/openvpn/socket.c |  582 ++++++++++++++++----------------------------------
 src/openvpn/socket.h |   17 +-
 3 files changed, 215 insertions(+), 434 deletions(-)
src/openvpn/route.c +25 -25
index d967d4f..a1064fe 100644
@@ -268,12 +268,14 @@ is_special_addr (const char *addr_str)
 
 static bool
 init_route (struct route *r,
-	    struct resolve_list *network_list,
+	    struct addrinfo **network_list,
 	    const struct route_option *ro,
 	    const struct route_list *rl)
 {
   const in_addr_t default_netmask = IPV4_NETMASK_HOST;
   bool status;
+  int ret;
+  struct in_addr special;
 
   CLEAR (*r);
   r->option = ro;

@@ -284,19 +286,23 @@ init_route (struct route *r,
     {
       goto fail;
     }
-  
-  if (!get_special_addr (rl, ro->network, &r->network, &status))
-    {
-      r->network = getaddr_multi (
-				  GETADDR_RESOLVE
-				  | GETADDR_HOST_ORDER
-				  | GETADDR_WARN_ON_SIGNAL,
-				  ro->network,
-				  0,
-				  &status,
-				  NULL,
-				  network_list);
-    }
+
+	
+	/* get_special_addr replaces specialaddr with a special ip addr
+	   like gw. getaddrinfo is called to convert a a addrinfo struct */
+
+	if(get_special_addr (rl, ro->network, &special.s_addr, &status)) 
+	  {
+		special.s_addr = htonl(special.s_addr);
+		ret = openvpn_getaddrinfo(0, inet_ntoa(special), 0, NULL,
+								  AF_INET, network_list);
+	  }
+	else 
+		ret = openvpn_getaddrinfo(GETADDR_RESOLVE | GETADDR_WARN_ON_SIGNAL,
+								  ro->network, 0, NULL, AF_INET, network_list);
+
+	status = (ret == 0);
+
 
   if (!status)
     goto fail;

@@ -642,11 +648,8 @@ init_route_list (struct route_list *rl,
     bool warned = false;
     for (i = 0; i < opt->n; ++i)
       {
-	struct resolve_list netlist;
+	struct addrinfo* netlist;
 	struct route r;
-	int k;
-
-        CLEAR(netlist);		/* init_route() will not always init this */
 
 	if (!init_route (&r,
 			 &netlist,

@@ -655,16 +658,12 @@ init_route_list (struct route_list *rl,
 	  ret = false;
 	else
 	  {
-	    if (!netlist.len)
-	      {
-		netlist.data[0] = r.network;
-		netlist.len = 1;
-	      }
-	    for (k = 0; k < netlist.len; ++k)
+		struct addrinfo* curele;
+		for (curele	= netlist; curele; curele = curele->ai_next) 
 	      {
 		if (j < rl->capacity)
 		  {
-		    r.network = netlist.data[k];
+			r.network = ntohl(((struct sockaddr_in*)(curele)->ai_addr)->sin_addr.s_addr);
 		    rl->routes[j++] = r;
 		  }
 		else

@@ -676,6 +675,7 @@ init_route_list (struct route_list *rl,
 		      }
 		  }
 	      }
+   	  freeaddrinfo(netlist);
 	  }
       }
     rl->n = j;
src/openvpn/socket.c +184 -398

663 lines not on this page · raw message

src/openvpn/socket.h +6 -11
index 47c6e8e..76d9a79 100644
@@ -467,11 +467,6 @@ bool unix_socket_get_peer_uid_gid (const socket_descriptor_t sd, int *uid, int *
  * DNS resolution
  */
 
-struct resolve_list {
-  int len;
-  in_addr_t data[16];
-};
-
 #define GETADDR_RESOLVE               (1<<0)
 #define GETADDR_FATAL                 (1<<1)
 #define GETADDR_HOST_ORDER            (1<<2)

@@ -494,12 +489,12 @@ in_addr_t getaddr (unsigned int flags,
 		   bool *succeeded,
 		   volatile int *signal_received);
 
-in_addr_t getaddr_multi (unsigned int flags,
-			 const char *hostname,
-			 int resolve_retry_seconds,
-			 bool *succeeded,
-			 volatile int *signal_received,
-			 struct resolve_list *reslist);
+int openvpn_getaddrinfo (unsigned int flags,
+					 const char *hostname,
+					 int resolve_retry_seconds,
+					 volatile int *signal_received,
+					 int ai_family, 
+					 struct addrinfo **res);
 
 /*
  * Transport protocol naming and other details.
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
#6 lore raw

Re: [Openvpn-devel] [PATCH 2/4] Merge getaddr_multi and getaddr6 into one function

Hi,

Quoted 4 lines · Arne Schwabe

the getaddr6 and getaddr_mutli functions are duplicates of each other. Since we always require getaddrinfo to be present both function are merge into one openvpn_getaddrinfo.

This functions also returns a standard struct addrinfo* so our resolve interface is closer to the standard unix interface. The getaddr function is a wrapper which provides backward compatibility for IPv4 addresses. Ipv6 calls and calls to getaddr_multi are replaced with the new interface.

ACK on the feature, but there's something weird in the implementation (which I've not had time to track down yet).

When connecting to a TCP ipv6 server (--client --proto tcp6-client), things fail in interesting ways:

$ openvpn --client --nobind --comp-lzo --verb 3 --dev tun --proto tcp6-client --remote conn-test-server.openvpn.org --port 51194
...
Mon Aug  6 19:49:06 2012 Attempting to establish TCP connection with [AF_INET6]2607:fc50:1001:5200:::51194 [nonblock]

This is buggy already, because "conn-test-server" is "2607:fc50:1001:5200::4", and the ":4" got lost...

Mon Aug 6 19:49:16 2012 TCP: connect to [AF_INET6]2607:fc50:1001:5200:::51194 failed, will try again in 5 seconds: Operation timed out Mon Aug 6 19:49:22 2012 TCP connection established with [AF_INET6]2607:fc50:1001:5200::4:51194

Nevertheless, it eventually it *does* connect - and that looks really weird in tcpdump:

20:29:53.410339 IP6 2001:608:0:814::f000:3.51581 > 2607:fc50:1001:5200::.51194: S 3439435758:3439435758(0) win 65535 <mss 1440,nop,wscale 3,sackOK,timestamp 1696266663 0> 20:29:56.609888 IP6 2001:608:0:814::f000:3.51581 > 2607:fc50:1001:5200::.51194: S 3439435758:3439435758(0) win 65535 <mss 1440,nop,wscale 3,sackOK,timestamp 1696266983 0> 20:29:59.809652 IP6 2001:608:0:814::f000:3.51581 > 2607:fc50:1001:5200::.51194: S 3439435758:3439435758(0) win 65535 <mss 1440,sackOK,eol>

3 tries with "5200::"...

20:30:05.520033 IP6 2001:608:0:814::f000:3.52113 > 2607:fc50:1001:5200::4.51194: S 1364097067:1364097067(0) win 65535 <mss 1440,nop,wscale 3,sackOK,timestamp 1696267874 0> 20:30:05.658230 IP6 2607:fc50:1001:5200::4.51194 > 2001:608:0:814::f000:3.52113: S 278837698:278837698(0) ack 1364097068 win 65535 <mss 1440,nop,wscale 6,sackOK,timestamp 1030394651 1696267874> 20:30:05.658368 IP6 2001:608:0:814::f000:3.52113 > 2607:fc50:1001:5200::4.51194: . ack 1 win 8211 <nop,nop,timestamp 1696267887 1030394651>

... and then it tries the proper host, which answers...

After connecting, it then fails on verifying the remote (huh, for TCP?!):

Mon Aug  6 19:49:22 2012 TCPv6_CLIENT link local: [undef]
Mon Aug  6 19:49:22 2012 TCPv6_CLIENT link remote: [AF_INET6]2607:fc50:1001:5200::4:51194
Mon Aug  6 19:49:22 2012 TCP/UDP: Incoming packet rejected from [AF_INET6]2607:fc50:1001:5200::4:51194[28], expected peer address: [AF_INET6]2607:fc50:1001:5200:::51194 (allow this incoming source address/port by removing --remote or adding --float)
Mon Aug  6 19:49:22 2012 TCP/UDP: Incoming packet rejected from [AF_INET6]2607:fc50:1001:5200::4:51194[28], expected peer address: [AF_INET6]2607:fc50:1001:5200:::51194 (allow this incoming source address/port by removing --remote or adding --float)

(and of course, from there on it will not go anywhere). This happens on FreeBSD 7.4 with yesterday's "master" and patch2/4 applied - without the patch, it works.

#7 lore raw

Re: [Openvpn-devel] [PATCH 2/4] Merge getaddr_multi and getaddr6 into one function

Hi,

Quoted 4 lines · Gert Doering

Mon Aug 6 19:49:06 2012 Attempting to establish TCP connection with [AF_INET6]2607:fc50:1001:5200:::51194 [nonblock]

This is buggy already, because "conn-test-server" is "2607:fc50:1001:5200::4", and the ":4" got lost...

Looking more closely, the last 64 bits seem to be zeroed - connecting to something that has lots of f's...

src/openvpn/openvpn --comp-lzo --verb 3 --dev tun --proto tcp6-client \
	--remote 2001:608:5:ffff:ffff:ffff:ffff:ffff --port 51194

...
23:27:32.948690 IP6 2001:608:0:814::f000:3.54055 > 2001:608:5:ffff::.51194: S 2476353562:2476353562(0) win 65535 <mss 1440,nop,wscale 3,sackOK,timestamp 1697332759 0>

And it's not related to DNS itself, just to getaddrinfo() (so you don't need to muck with DNS or setup a tcp6 test server to see this)

If family permits, I'll try to dig into this more tomorrow, it certainly raised my curiousity :-)

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#8 lore raw

[Openvpn-devel] [Version 2:] Merge getaddr_multi and getaddr6 into one function

the getaddr6 and getaddr_mutli functions are duplicates of each other. Since we always require getaddrinfo to be present both function are merge into one openvpn_getaddrinfo.

This functions also returns a standard struct addrinfo* so our resolve interface is closer to the standard unix interface. The getaddr function is a wrapper which provides backward compatibility for IPv4 addresses. Ipv6 calls and calls to getaddr_multi are replaced with the new interface.

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/route.c  |   50 ++---
 src/openvpn/socket.c |  579 ++++++++++++++++----------------------------------
 src/openvpn/socket.h |   17 +-
 3 files changed, 212 insertions(+), 434 deletions(-)
src/openvpn/route.c +25 -25
index d967d4f..a1064fe 100644
@@ -268,12 +268,14 @@ is_special_addr (const char *addr_str)
 
 static bool
 init_route (struct route *r,
-	    struct resolve_list *network_list,
+	    struct addrinfo **network_list,
 	    const struct route_option *ro,
 	    const struct route_list *rl)
 {
   const in_addr_t default_netmask = IPV4_NETMASK_HOST;
   bool status;
+  int ret;
+  struct in_addr special;
 
   CLEAR (*r);
   r->option = ro;

@@ -284,19 +286,23 @@ init_route (struct route *r,
     {
       goto fail;
     }
-  
-  if (!get_special_addr (rl, ro->network, &r->network, &status))
-    {
-      r->network = getaddr_multi (
-				  GETADDR_RESOLVE
-				  | GETADDR_HOST_ORDER
-				  | GETADDR_WARN_ON_SIGNAL,
-				  ro->network,
-				  0,
-				  &status,
-				  NULL,
-				  network_list);
-    }
+
+	
+	/* get_special_addr replaces specialaddr with a special ip addr
+	   like gw. getaddrinfo is called to convert a a addrinfo struct */
+
+	if(get_special_addr (rl, ro->network, &special.s_addr, &status)) 
+	  {
+		special.s_addr = htonl(special.s_addr);
+		ret = openvpn_getaddrinfo(0, inet_ntoa(special), 0, NULL,
+								  AF_INET, network_list);
+	  }
+	else 
+		ret = openvpn_getaddrinfo(GETADDR_RESOLVE | GETADDR_WARN_ON_SIGNAL,
+								  ro->network, 0, NULL, AF_INET, network_list);
+
+	status = (ret == 0);
+
 
   if (!status)
     goto fail;

@@ -642,11 +648,8 @@ init_route_list (struct route_list *rl,
     bool warned = false;
     for (i = 0; i < opt->n; ++i)
       {
-	struct resolve_list netlist;
+	struct addrinfo* netlist;
 	struct route r;
-	int k;
-
-        CLEAR(netlist);		/* init_route() will not always init this */
 
 	if (!init_route (&r,
 			 &netlist,

@@ -655,16 +658,12 @@ init_route_list (struct route_list *rl,
 	  ret = false;
 	else
 	  {
-	    if (!netlist.len)
-	      {
-		netlist.data[0] = r.network;
-		netlist.len = 1;
-	      }
-	    for (k = 0; k < netlist.len; ++k)
+		struct addrinfo* curele;
+		for (curele	= netlist; curele; curele = curele->ai_next) 
 	      {
 		if (j < rl->capacity)
 		  {
-		    r.network = netlist.data[k];
+			r.network = ntohl(((struct sockaddr_in*)(curele)->ai_addr)->sin_addr.s_addr);
 		    rl->routes[j++] = r;
 		  }
 		else

@@ -676,6 +675,7 @@ init_route_list (struct route_list *rl,
 		      }
 		  }
 	      }
+   	  freeaddrinfo(netlist);
 	  }
       }
     rl->n = j;
src/openvpn/socket.c +181 -398

660 lines not on this page · raw message

src/openvpn/socket.h +6 -11
index 43f72dc..2060bc8 100644
@@ -467,11 +467,6 @@ bool unix_socket_get_peer_uid_gid (const socket_descriptor_t sd, int *uid, int *
  * DNS resolution
  */
 
-struct resolve_list {
-  int len;
-  in_addr_t data[16];
-};
-
 #define GETADDR_RESOLVE               (1<<0)
 #define GETADDR_FATAL                 (1<<1)
 #define GETADDR_HOST_ORDER            (1<<2)

@@ -494,12 +489,12 @@ in_addr_t getaddr (unsigned int flags,
 		   bool *succeeded,
 		   volatile int *signal_received);
 
-in_addr_t getaddr_multi (unsigned int flags,
-			 const char *hostname,
-			 int resolve_retry_seconds,
-			 bool *succeeded,
-			 volatile int *signal_received,
-			 struct resolve_list *reslist);
+int openvpn_getaddrinfo (unsigned int flags,
+					 const char *hostname,
+					 int resolve_retry_seconds,
+					 volatile int *signal_received,
+					 int ai_family, 
+					 struct addrinfo **res);
 
 /*
  * Transport protocol naming and other details.
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
author
#9 lore raw

[Openvpn-devel] [Version 3] Merge getaddr_multi and getaddr6 into one function

In reply to Gert Doering

the getaddr6 and getaddr_mutli functions are duplicates of each other. Since we always require getaddrinfo to be present both function are merge into one openvpn_getaddrinfo.

This functions also returns a standard struct addrinfo* so our resolve interface is closer to the standard unix interface. The getaddr function is a wrapper which provides backward compatibility for IPv4 addresses. Ipv6 calls and calls to getaddr_multi are replaced with the new interface.

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/route.c  |   50 ++---
 src/openvpn/socket.c |  596 ++++++++++++++++----------------------------------
 src/openvpn/socket.h |   17 +-
 3 files changed, 221 insertions(+), 442 deletions(-)
src/openvpn/route.c +25 -25
index d967d4f..a1064fe 100644
@@ -268,12 +268,14 @@ is_special_addr (const char *addr_str)
 
 static bool
 init_route (struct route *r,
-	    struct resolve_list *network_list,
+	    struct addrinfo **network_list,
 	    const struct route_option *ro,
 	    const struct route_list *rl)
 {
   const in_addr_t default_netmask = IPV4_NETMASK_HOST;
   bool status;
+  int ret;
+  struct in_addr special;
 
   CLEAR (*r);
   r->option = ro;

@@ -284,19 +286,23 @@ init_route (struct route *r,
     {
       goto fail;
     }
-  
-  if (!get_special_addr (rl, ro->network, &r->network, &status))
-    {
-      r->network = getaddr_multi (
-				  GETADDR_RESOLVE
-				  | GETADDR_HOST_ORDER
-				  | GETADDR_WARN_ON_SIGNAL,
-				  ro->network,
-				  0,
-				  &status,
-				  NULL,
-				  network_list);
-    }
+
+	
+	/* get_special_addr replaces specialaddr with a special ip addr
+	   like gw. getaddrinfo is called to convert a a addrinfo struct */
+
+	if(get_special_addr (rl, ro->network, &special.s_addr, &status)) 
+	  {
+		special.s_addr = htonl(special.s_addr);
+		ret = openvpn_getaddrinfo(0, inet_ntoa(special), 0, NULL,
+								  AF_INET, network_list);
+	  }
+	else 
+		ret = openvpn_getaddrinfo(GETADDR_RESOLVE | GETADDR_WARN_ON_SIGNAL,
+								  ro->network, 0, NULL, AF_INET, network_list);
+
+	status = (ret == 0);
+
 
   if (!status)
     goto fail;

@@ -642,11 +648,8 @@ init_route_list (struct route_list *rl,
     bool warned = false;
     for (i = 0; i < opt->n; ++i)
       {
-	struct resolve_list netlist;
+	struct addrinfo* netlist;
 	struct route r;
-	int k;
-
-        CLEAR(netlist);		/* init_route() will not always init this */
 
 	if (!init_route (&r,
 			 &netlist,

@@ -655,16 +658,12 @@ init_route_list (struct route_list *rl,
 	  ret = false;
 	else
 	  {
-	    if (!netlist.len)
-	      {
-		netlist.data[0] = r.network;
-		netlist.len = 1;
-	      }
-	    for (k = 0; k < netlist.len; ++k)
+		struct addrinfo* curele;
+		for (curele	= netlist; curele; curele = curele->ai_next) 
 	      {
 		if (j < rl->capacity)
 		  {
-		    r.network = netlist.data[k];
+			r.network = ntohl(((struct sockaddr_in*)(curele)->ai_addr)->sin_addr.s_addr);
 		    rl->routes[j++] = r;
 		  }
 		else

@@ -676,6 +675,7 @@ init_route_list (struct route_list *rl,
 		      }
 		  }
 	      }
+   	  freeaddrinfo(netlist);
 	  }
       }
     rl->n = j;
src/openvpn/socket.c +190 -406

676 lines not on this page · raw message

src/openvpn/socket.h +6 -11
index 43f72dc..2060bc8 100644
@@ -467,11 +467,6 @@ bool unix_socket_get_peer_uid_gid (const socket_descriptor_t sd, int *uid, int *
  * DNS resolution
  */
 
-struct resolve_list {
-  int len;
-  in_addr_t data[16];
-};
-
 #define GETADDR_RESOLVE               (1<<0)
 #define GETADDR_FATAL                 (1<<1)
 #define GETADDR_HOST_ORDER            (1<<2)

@@ -494,12 +489,12 @@ in_addr_t getaddr (unsigned int flags,
 		   bool *succeeded,
 		   volatile int *signal_received);
 
-in_addr_t getaddr_multi (unsigned int flags,
-			 const char *hostname,
-			 int resolve_retry_seconds,
-			 bool *succeeded,
-			 volatile int *signal_received,
-			 struct resolve_list *reslist);
+int openvpn_getaddrinfo (unsigned int flags,
+					 const char *hostname,
+					 int resolve_retry_seconds,
+					 volatile int *signal_received,
+					 int ai_family, 
+					 struct addrinfo **res);
 
 /*
  * Transport protocol naming and other details.
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
#10 lore raw

Re: [Openvpn-devel] [Version 3] Merge getaddr_multi and getaddr6 into one function

Hi,

Quoted 4 lines · Arne Schwabe

the getaddr6 and getaddr_mutli functions are duplicates of each other. Since we always require getaddrinfo to be present both function are merge into one openvpn_getaddrinfo.

This functions also returns a standard struct addrinfo* so our resolve interface is closer to the standard unix interface. The getaddr function is a wrapper which provides backward compatibility for IPv4 addresses. Ipv6 calls and calls to getaddr_multi are replaced with the new interface.

Getting closer, but something is still funny.

Connecting to IPv6 tcp or udp server works now, but the resolving of route destinations seems to result in triplication of all IPv4 routes.

Unchanged "master" code connecting to a server that pushes 2 IPv4 routes:

Tue Aug  7 12:39:05 2012 /sbin/ifconfig tun1 10.194.1.6 10.194.1.5 mtu 1500 netmask 255.255.255.255 up
Tue Aug  7 12:39:05 2012 /sbin/ifconfig tun1 inet6 fd00:abcd:194:1::1000/64
Tue Aug  7 12:39:05 2012 /sbin/route add -net 10.194.0.0 10.194.1.5 255.255.0.0

add net 10.194.0.0: gateway 10.194.1.5
Tue Aug 7 12:39:05 2012 /sbin/route add -net 10.194.1.1 10.194.1.5 255.255.255.255 add net 10.194.1.1: gateway 10.194.1.5
Tue Aug 7 12:39:05 2012 add_route_ipv6(fd00:abcd:194::/48 -> fd00:abcd:194:1::1 metric -1) dev tun1 Tue Aug 7 12:39:05 2012 /sbin/route add -inet6 fd00:abcd:194::/48 -iface tun1
add net fd00:abcd:194::/48: gateway tun1
Tue Aug 7 12:39:05 2012 Initialization Sequence Completed

With your v3 patch:

Tue Aug  7 12:38:16 2012 /sbin/ifconfig tun1 10.194.1.6 10.194.1.5 mtu 1500 netmask 255.255.255.255 up
Tue Aug  7 12:38:16 2012 /sbin/ifconfig tun1 inet6 fd00:abcd:194:1::1000/64
Tue Aug  7 12:38:16 2012 /sbin/route add -net 10.194.0.0 10.194.1.5 255.255.0.0

add net 10.194.0.0: gateway 10.194.1.5
Tue Aug 7 12:38:16 2012 /sbin/route add -net 10.194.0.0 10.194.1.5 255.255.0.0
route: writing to routing socket: File exists
add net 10.194.0.0: gateway 10.194.1.5: route already in table
Tue Aug 7 12:38:16 2012 ERROR: FreeBSD route add command failed: external program exited with error status: 1 Tue Aug 7 12:38:16 2012 /sbin/route add -net 10.194.0.0 10.194.1.5 255.255.0.0
route: writing to routing socket: File exists
add net 10.194.0.0: gateway 10.194.1.5: route already in table
Tue Aug 7 12:38:16 2012 ERROR: FreeBSD route add command failed: external program exited with error status: 1 Tue Aug 7 12:38:16 2012 /sbin/route add -net 10.194.1.1 10.194.1.5 255.255.255.255
add net 10.194.1.1: gateway 10.194.1.5
Tue Aug 7 12:38:16 2012 /sbin/route add -net 10.194.1.1 10.194.1.5 255.255.255.255
route: writing to routing socket: File exists
add net 10.194.1.1: gateway 10.194.1.5: route already in table
Tue Aug 7 12:38:16 2012 ERROR: FreeBSD route add command failed: external program exited with error status: 1 Tue Aug 7 12:38:16 2012 /sbin/route add -net 10.194.1.1 10.194.1.5 255.255.255.255
route: writing to routing socket: File exists
add net 10.194.1.1: gateway 10.194.1.5: route already in table

Tue Aug  7 12:38:16 2012 ERROR: FreeBSD route add command failed: external program exited with error status: 1
Tue Aug  7 12:38:16 2012 add_route_ipv6(fd00:abcd:194::/48 -> fd00:abcd:194:1::1 metric -1) dev tun1
Tue Aug  7 12:38:16 2012 /sbin/route add -inet6 fd00:abcd:194::/48 -iface tun1

add net fd00:abcd:194::/48: gateway tun1
Tue Aug 7 12:38:16 2012 Initialization Sequence Completed

(again, just reporting symptoms, not having stared at the code)

Even though it might be frustrating - thanks for working on this!

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#11 lore raw

[Openvpn-devel] [Version 4] Merge getaddr_multi and getaddr6 into one function

the getaddr6 and getaddr_mutli functions are duplicates of each other. Since we always require getaddrinfo to be present both function are merge into one openvpn_getaddrinfo.

This functions also returns a standard struct addrinfo* so our resolve interface is closer to the standard unix interface. The getaddr function is a wrapper which provides backward compatibility for IPv4 addresses. Ipv6 calls and calls to getaddr_multi are replaced with the new interface.

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/route.c  |   50 ++---
 src/openvpn/socket.c |  594 ++++++++++++++++----------------------------------
 src/openvpn/socket.h |   17 +-
 3 files changed, 220 insertions(+), 441 deletions(-)
src/openvpn/route.c +25 -25
index d967d4f..a1064fe 100644
@@ -268,12 +268,14 @@ is_special_addr (const char *addr_str)
 
 static bool
 init_route (struct route *r,
-	    struct resolve_list *network_list,
+	    struct addrinfo **network_list,
 	    const struct route_option *ro,
 	    const struct route_list *rl)
 {
   const in_addr_t default_netmask = IPV4_NETMASK_HOST;
   bool status;
+  int ret;
+  struct in_addr special;
 
   CLEAR (*r);
   r->option = ro;

@@ -284,19 +286,23 @@ init_route (struct route *r,
     {
       goto fail;
     }
-  
-  if (!get_special_addr (rl, ro->network, &r->network, &status))
-    {
-      r->network = getaddr_multi (
-				  GETADDR_RESOLVE
-				  | GETADDR_HOST_ORDER
-				  | GETADDR_WARN_ON_SIGNAL,
-				  ro->network,
-				  0,
-				  &status,
-				  NULL,
-				  network_list);
-    }
+
+	
+	/* get_special_addr replaces specialaddr with a special ip addr
+	   like gw. getaddrinfo is called to convert a a addrinfo struct */
+
+	if(get_special_addr (rl, ro->network, &special.s_addr, &status)) 
+	  {
+		special.s_addr = htonl(special.s_addr);
+		ret = openvpn_getaddrinfo(0, inet_ntoa(special), 0, NULL,
+								  AF_INET, network_list);
+	  }
+	else 
+		ret = openvpn_getaddrinfo(GETADDR_RESOLVE | GETADDR_WARN_ON_SIGNAL,
+								  ro->network, 0, NULL, AF_INET, network_list);
+
+	status = (ret == 0);
+
 
   if (!status)
     goto fail;

@@ -642,11 +648,8 @@ init_route_list (struct route_list *rl,
     bool warned = false;
     for (i = 0; i < opt->n; ++i)
       {
-	struct resolve_list netlist;
+	struct addrinfo* netlist;
 	struct route r;
-	int k;
-
-        CLEAR(netlist);		/* init_route() will not always init this */
 
 	if (!init_route (&r,
 			 &netlist,

@@ -655,16 +658,12 @@ init_route_list (struct route_list *rl,
 	  ret = false;
 	else
 	  {
-	    if (!netlist.len)
-	      {
-		netlist.data[0] = r.network;
-		netlist.len = 1;
-	      }
-	    for (k = 0; k < netlist.len; ++k)
+		struct addrinfo* curele;
+		for (curele	= netlist; curele; curele = curele->ai_next) 
 	      {
 		if (j < rl->capacity)
 		  {
-		    r.network = netlist.data[k];
+			r.network = ntohl(((struct sockaddr_in*)(curele)->ai_addr)->sin_addr.s_addr);
 		    rl->routes[j++] = r;
 		  }
 		else

@@ -676,6 +675,7 @@ init_route_list (struct route_list *rl,
 		      }
 		  }
 	      }
+   	  freeaddrinfo(netlist);
 	  }
       }
     rl->n = j;
src/openvpn/socket.c +189 -405

675 lines not on this page · raw message

src/openvpn/socket.h +6 -11
index 43f72dc..2060bc8 100644
@@ -467,11 +467,6 @@ bool unix_socket_get_peer_uid_gid (const socket_descriptor_t sd, int *uid, int *
  * DNS resolution
  */
 
-struct resolve_list {
-  int len;
-  in_addr_t data[16];
-};
-
 #define GETADDR_RESOLVE               (1<<0)
 #define GETADDR_FATAL                 (1<<1)
 #define GETADDR_HOST_ORDER            (1<<2)

@@ -494,12 +489,12 @@ in_addr_t getaddr (unsigned int flags,
 		   bool *succeeded,
 		   volatile int *signal_received);
 
-in_addr_t getaddr_multi (unsigned int flags,
-			 const char *hostname,
-			 int resolve_retry_seconds,
-			 bool *succeeded,
-			 volatile int *signal_received,
-			 struct resolve_list *reslist);
+int openvpn_getaddrinfo (unsigned int flags,
+					 const char *hostname,
+					 int resolve_retry_seconds,
+					 volatile int *signal_received,
+					 int ai_family, 
+					 struct addrinfo **res);
 
 /*
  * Transport protocol naming and other details.
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
#12 lore raw

Re: [Openvpn-devel] [Version 4] Merge getaddr_multi and getaddr6 into one function

Hi,

Quoted 3 lines · Arne Schwabe

the getaddr6 and getaddr_mutli functions are duplicates of each other. Since we always require getaddrinfo to be present both function are merge into one openvpn_getaddrinfo.

This functions also returns a standard struct addrinfo* so our resolve interface is closer to the standard unix interface. The getaddr function is a wrapper which provides backward compatibility for IPv4 addresses. Ipv6 calls and calls to getaddr_multi are replaced with the new interface.

So. I tested this already when you posted it (and it passes all my t_client tests, connecting to IPv4 and IPv6 hosts, and inserting lots of IPv4 routes - none of them to-be-resolved, though, but given the problems with route duplication in v3, I assume it will do).

Today I went there and reviewed all of the code, and tried in earnest to understand what happens where :-)

I ACK this, but please remember to tell your editor to use 8-character tabs, as that is "what the rest of the code uses" - some of the new code looks quite weird with ":set ts=8", and there's more diffs in there due to whitespace changes (4 spaces getting converted into a single tab, like before 'hostname = "::"'). We do not need a v5 re-spin, as David said he'll take care of these.

(Samuli: we need a style guide in the wiki...)

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
#13 lore raw

Re: [Openvpn-devel] [Version 4] Merge getaddr_multi and getaddr6 into one function

Quoted 11 lines

I ACK this, but please remember to tell your editor to use 8-character tabs, as that is "what the rest of the code uses" - some of the new code looks quite weird with ":set ts=8", and there's more diffs in there due to whitespace changes (4 spaces getting converted into a single tab, like before 'hostname = "::"'). We do not need a v5 re-spin, as David said he'll take care of these.

(Samuli: we need a style guide in the wiki...)

gert

+1.

Anyone who knows what the current "standard" formatting please add instructions here:

<https://community.openvpn.net/openvpn/wiki/DeveloperDocumentation#Formatting>

Signature · 5 lines
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock
Applied
#14 lore raw

Re: [Openvpn-devel] [Version 4] Merge getaddr_multi and getaddr6 into one function

In reply to Arne Schwabe
Quoted 10 lines · Arne Schwabe

the getaddr6 and getaddr_mutli functions are duplicates of each other. Since we always require getaddrinfo to be present both function are merge into one openvpn_getaddrinfo.

This functions also returns a standard struct addrinfo* so our resolve interface is closer to the standard unix interface. The getaddr function is a wrapper which provides backward compatibility for IPv4 addresses. Ipv6 calls and calls to getaddr_multi are replaced with the new interface.

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/route.c  |   50 ++---
 src/openvpn/socket.c |  594 ++++++++++++++++----------------------------------
 src/openvpn/socket.h |   17 +-
 3 files changed, 220 insertions(+), 441 deletions(-)

Applied to master.

commit f2d6f3bc06db4f9e0815fc25e35e393f794c193a

Author: Arne Schwabe <[email protected]...>
Date:   Tue Aug 7 12:03:57 2012 +0200
    Merge getaddr_multi and getaddr6 into one function

    Signed-off-by: Arne Schwabe <[email protected]...>
    Acked-by: Gert Doering <[email protected]...>
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/6959
    Signed-off-by: David Sommerseth <[email protected]...>

kind regards,

David Sommerseth

author
#15 lore raw

PATCH 3/4 Merge almost identical create_socket_tcp and create_socket_tcp6

In reply to Arne Schwabe
---
 src/openvpn/manage.c |    4 ++--
 src/openvpn/socket.c |   53 +++++++-------------------------------------------
 src/openvpn/socket.h |    2 +-
 3 files changed, 10 insertions(+), 49 deletions(-)
src/openvpn/manage.c +2 -2
index 3ef14e5..d0bb416 100644
@@ -1567,7 +1567,7 @@ man_listen (struct management *man)
       else
 #endif
 	{
-	  man->connection.sd_top = create_socket_tcp ();
+	  man->connection.sd_top = create_socket_tcp (AF_INET);
 	  socket_bind (man->connection.sd_top, &man->settings.local, "MANAGEMENT");
 	}
 

@@ -1633,7 +1633,7 @@ man_connect (struct management *man)
   else
 #endif
     {
-      man->connection.sd_cli = create_socket_tcp ();
+      man->connection.sd_cli = create_socket_tcp (AF_INET);
       status = openvpn_connect (man->connection.sd_cli,
 				&man->settings.local,
 				5,
src/openvpn/socket.c +7 -46

101 lines not on this page · raw message

src/openvpn/socket.h +1 -1
index 76d9a79..2060bc8 100644
@@ -425,7 +425,7 @@ bool ip_or_dns_addr_safe (const char *addr, const bool allow_fqdn);
 bool mac_addr_safe (const char *mac_addr);
 bool ipv6_addr_safe (const char *ipv6_text_addr);
 
-socket_descriptor_t create_socket_tcp (void);
+socket_descriptor_t create_socket_tcp (int af);
 
 socket_descriptor_t socket_do_accept (socket_descriptor_t sd,
 				      struct link_socket_actual *act,
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
#16 lore raw

Re: [Openvpn-devel] [PATCH 3/4] Merge almost identical create_socket_tcp and create_socket_tcp6

Hi,

Quoted 4 lines · Arne Schwabe
src/openvpn/socket.c +0 -0
@@ -1099,15 +1068,7 @@ socket_connect (socket_descriptor_t *sd,
       if (*signal_received)
 	goto done;

src/openvpn/socket.c socket_connect (socket_descriptor_t *sd, · line 1071

Quoted 10 lines
src/openvpn/socket.c +1 -9
-      switch(local->addr.sa.sa_family)
-	{
-	case PF_INET6:
-	  *sd = create_socket_tcp6 ();
-	  break;
-	case PF_INET:
-	  *sd = create_socket_tcp ();
-	  break;
-	}
+	create_socket_tcp(local->addr.sa.sa_family);

I might be overlooking something, but shouldn't that be

+ *sd = create_socket_tcp(local->addr.sa.sa_family);

?

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#17 lore raw

Re: [Openvpn-devel] [PATCH 3/4] Merge almost identical create_socket_tcp and create_socket_tcp6

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 01.08.12 22:38, schrieb Gert Doering:

Quoted 20 lines

Hi,

Quoted 4 lines · Arne Schwabe
src/openvpn/socket.c +0 -0
@@ -1099,15 +1068,7 @@ socket_connect (socket_descriptor_t *sd,
 if (*signal_received)
 goto done;

src/openvpn/socket.c socket_connect (socket_descriptor_t *sd, · line 1071

Quoted 10 lines
src/openvpn/socket.c +1 -9
- switch(local->addr.sa.sa_family)
- {
- case PF_INET6:
- *sd = create_socket_tcp6 ();
- break;
- case PF_INET:
- *sd = create_socket_tcp ();
- break;
- }
+ create_socket_tcp(local->addr.sa.sa_family);

I might be overlooking something, but shouldn't that be

Quoted 3 lines

+ *sd = create_socket_tcp(local->addr.sa.sa_family);

Yes you are right.

Arne
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlAZmXUACgkQe8+cMNS4zRdDrQCfdTfEEbZAF+F1UlcoW22lk2QG UpkAmwWhOrJqOHsuaYlcD2JLcifH9VfY
=nrKu
-----END PGP SIGNATURE-----

author
#18 lore raw

PATCH 3/4 Merge almost identical create_socket_tcp and create_socket_tcp6

In reply to Gert Doering
Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/manage.c |    4 ++--
 src/openvpn/socket.c |   53 +++++++-------------------------------------------
 src/openvpn/socket.h |    2 +-
 3 files changed, 10 insertions(+), 49 deletions(-)
src/openvpn/manage.c +2 -2
index 3ef14e5..d0bb416 100644
@@ -1567,7 +1567,7 @@ man_listen (struct management *man)
       else
 #endif
 	{
-	  man->connection.sd_top = create_socket_tcp ();
+	  man->connection.sd_top = create_socket_tcp (AF_INET);
 	  socket_bind (man->connection.sd_top, &man->settings.local, "MANAGEMENT");
 	}
 

@@ -1633,7 +1633,7 @@ man_connect (struct management *man)
   else
 #endif
     {
-      man->connection.sd_cli = create_socket_tcp ();
+      man->connection.sd_cli = create_socket_tcp (AF_INET);
       status = openvpn_connect (man->connection.sd_cli,
 				&man->settings.local,
 				5,
src/openvpn/socket.c +7 -46

101 lines not on this page · raw message

src/openvpn/socket.h +1 -1
index 76d9a79..2060bc8 100644
@@ -425,7 +425,7 @@ bool ip_or_dns_addr_safe (const char *addr, const bool allow_fqdn);
 bool mac_addr_safe (const char *mac_addr);
 bool ipv6_addr_safe (const char *ipv6_text_addr);
 
-socket_descriptor_t create_socket_tcp (void);
+socket_descriptor_t create_socket_tcp (int af);
 
 socket_descriptor_t socket_do_accept (socket_descriptor_t sd,
 				      struct link_socket_actual *act,
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
#19 lore raw

Re: [Openvpn-devel] [PATCH 3/4] Merge almost identical create_socket_tcp and create_socket_tcp6

Hi,

Quoted 6 lines · Arne Schwabe
Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/manage.c |    4 ++--
 src/openvpn/socket.c |   53 +++++++-------------------------------------------
 src/openvpn/socket.h |    2 +-
 3 files changed, 10 insertions(+), 49 deletions(-)

ACK on v2 of 3/4.

I think this can go into 2.3 as well, it's basic cleanup (and in addition, it's cleanup of new 2.3 functionality, so doubly welcome).

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
Applied
#20 lore raw

Re: [Openvpn-devel] [PATCH 3/4] Merge almost identical create_socket_tcp and create_socket_tcp6

In reply to Arne Schwabe
Quoted 7 lines · Arne Schwabe
Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/manage.c |    4 ++--
 src/openvpn/socket.c |   53 +++++++-------------------------------------------
 src/openvpn/socket.h |    2 +-
 3 files changed, 10 insertions(+), 49 deletions(-)

Applied to master.

commit e1f6121d6c189c59b367890e82efe369e08861b4

Author: Arne Schwabe <[email protected]...>
Date:   Thu Aug 2 15:36:59 2012 +0200
    Merge almost identical create_socket_tcp and create_socket_tcp6
    
    Signed-off-by: Arne Schwabe <[email protected]...>
    Acked-by: Gert Doering <[email protected]...>
    Message-Id: [email protected]...
    URL: http://article.gmane.org/gmane.network.openvpn.devel/6942
    Signed-off-by: David Sommerseth <[email protected]...>

kind regards,

David Sommerseth

author
#21 lore raw

PATCH 4/4 Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

In reply to Arne Schwabe
---
 src/openvpn/socket.c |   89 ++++++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 50 deletions(-)
src/openvpn/socket.c +39 -50
index acc4f2b..8ab5872 100644
@@ -2173,57 +2173,46 @@ print_sockaddr_ex (const struct openvpn_sockaddr *addr,
 		   const unsigned int flags,
 		   struct gc_arena *gc)
 {
-  struct buffer out = alloc_buf_gc (128, gc);
-  bool addr_is_defined;
-  addr_is_defined = addr_defined (addr);
-  if (!addr_is_defined) {
-    return "[undef]";
-  }
-  switch(addr->addr.sa.sa_family)
-    {
-    case AF_INET:
-	{
-	  const int port= ntohs (addr->addr.in4.sin_port);
-	  buf_puts (&out, "[AF_INET]");
-
-	  if (!(flags & PS_DONT_SHOW_ADDR))
-	    buf_printf (&out, "%s", (addr_defined (addr) ? inet_ntoa (addr->addr.in4.sin_addr) : "[undef]"));
-
-	  if (((flags & PS_SHOW_PORT) || (addr_defined (addr) && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_printf (&out, "%s", separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
-	}
-      break;
-    case AF_INET6:
-	{
-	  const int port= ntohs (addr->addr.in6.sin6_port);
-	  char buf[INET6_ADDRSTRLEN] = "";
-	  buf_puts (&out, "[AF_INET6]");
-	  if (addr_is_defined)
-	    {
-	      getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
-			  buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
-	      buf_puts (&out, buf);
-	    }
-	  if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_puts (&out, separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
+	struct buffer out = alloc_buf_gc (128, gc);
+	bool addr_is_defined;
+	char buf[NI_MAXHOST] = "";
+	
+	addr_is_defined = addr_defined (addr);
+	if (!addr_is_defined) {
+		return "[undef]";
 	}
-      break;
-    default:
-      ASSERT(0);
-    }
-  return BSTR (&out);
+	
+	int port;
+	switch(addr->addr.sa.sa_family)
+  {
+	  case AF_INET:
+		port= ntohs (addr->addr.in4.sin_port);
+		buf_puts (&out, "[AF_INET]");
+		break;
+	  case AF_INET6:
+		port= ntohs (addr->addr.in6.sin6_port);
+		buf_puts (&out, "[AF_INET6]");
+		break;
+	  default:
+		ASSERT(0);
+  }
+	
+	if (!(flags & PS_DONT_SHOW_ADDR))
+	  {
+		getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
+					buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
+		buf_puts (&out, buf);
+	  }
+	if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
+		&& port)
+	  {
+		if (separator)
+			buf_puts (&out, separator);
+		
+		buf_printf (&out, "%d", port);
+	  }
+	
+	return BSTR (&out);
 }
 
 const char *
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
author
#22 lore raw

PATCH 1/1 Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/socket.c |   89 ++++++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 50 deletions(-)
src/openvpn/socket.c +39 -50
index a34e78c..5b2bb9d 100644
@@ -2387,57 +2387,46 @@ print_sockaddr_ex (const struct openvpn_sockaddr *addr,
 		   const unsigned int flags,
 		   struct gc_arena *gc)
 {
-  struct buffer out = alloc_buf_gc (128, gc);
-  bool addr_is_defined;
-  addr_is_defined = addr_defined (addr);
-  if (!addr_is_defined) {
-    return "[undef]";
-  }
-  switch(addr->addr.sa.sa_family)
-    {
-    case AF_INET:
-	{
-	  const int port= ntohs (addr->addr.in4.sin_port);
-	  buf_puts (&out, "[AF_INET]");
-
-	  if (!(flags & PS_DONT_SHOW_ADDR))
-	    buf_printf (&out, "%s", (addr_defined (addr) ? inet_ntoa (addr->addr.in4.sin_addr) : "[undef]"));
-
-	  if (((flags & PS_SHOW_PORT) || (addr_defined (addr) && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_printf (&out, "%s", separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
+	struct buffer out = alloc_buf_gc (128, gc);
+	bool addr_is_defined;
+	char buf[NI_MAXHOST] = "";
+	
+	addr_is_defined = addr_defined (addr);
+	if (!addr_is_defined) {
+		return "[undef]";
 	}
-      break;
-    case AF_INET6:
-	{
-	  const int port= ntohs (addr->addr.in6.sin6_port);
-	  char buf[INET6_ADDRSTRLEN] = "";
-	  buf_puts (&out, "[AF_INET6]");
-	  if (addr_is_defined)
-	    {
-	      getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
-			  buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
-	      buf_puts (&out, buf);
-	    }
-	  if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_puts (&out, separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
-	}
-      break;
-    default:
-      ASSERT(0);
-    }
-  return BSTR (&out);
+	
+	int port;
+	switch(addr->addr.sa.sa_family)
+  {
+	  case AF_INET:
+		port= ntohs (addr->addr.in4.sin_port);
+		buf_puts (&out, "[AF_INET]");
+		break;
+	  case AF_INET6:
+		port= ntohs (addr->addr.in6.sin6_port);
+		buf_puts (&out, "[AF_INET6]");
+		break;
+	  default:
+		ASSERT(0);
+  }
+	
+	if (!(flags & PS_DONT_SHOW_ADDR))
+	  {
+		getnameinfo(&addr->addr.sa,  addr->addr.sa.sa_len,
+					buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
+		buf_puts (&out, buf);
+	  }
+	if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
+		&& port)
+	  {
+		if (separator)
+			buf_puts (&out, separator);
+		
+		buf_printf (&out, "%d", port);
+	  }
+	
+	return BSTR (&out);
 }
 
 const char *
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
#23 lore raw

Re: [Openvpn-devel] [PATCH] Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

Hi,

(note that this is v2 of the patch with a change to getnameinfo()'s 2nd field, passing ->addr.sa.sa_len now)

ACK, tested and reviewed.

The tabbing is left for David to repair :-)

gert

Quoted 121 lines · Arne Schwabe
Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/socket.c |   89 ++++++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 50 deletions(-)
src/openvpn/socket.c +39 -50
index a34e78c..5b2bb9d 100644
@@ -2387,57 +2387,46 @@ print_sockaddr_ex (const struct openvpn_sockaddr *addr,
 		   const unsigned int flags,
 		   struct gc_arena *gc)
 {
-  struct buffer out = alloc_buf_gc (128, gc);
-  bool addr_is_defined;
-  addr_is_defined = addr_defined (addr);
-  if (!addr_is_defined) {
-    return "[undef]";
-  }
-  switch(addr->addr.sa.sa_family)
-    {
-    case AF_INET:
-	{
-	  const int port= ntohs (addr->addr.in4.sin_port);
-	  buf_puts (&out, "[AF_INET]");
-
-	  if (!(flags & PS_DONT_SHOW_ADDR))
-	    buf_printf (&out, "%s", (addr_defined (addr) ? inet_ntoa (addr->addr.in4.sin_addr) : "[undef]"));
-
-	  if (((flags & PS_SHOW_PORT) || (addr_defined (addr) && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_printf (&out, "%s", separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
+	struct buffer out = alloc_buf_gc (128, gc);
+	bool addr_is_defined;
+	char buf[NI_MAXHOST] = "";
+	
+	addr_is_defined = addr_defined (addr);
+	if (!addr_is_defined) {
+		return "[undef]";
 	}
-      break;
-    case AF_INET6:
-	{
-	  const int port= ntohs (addr->addr.in6.sin6_port);
-	  char buf[INET6_ADDRSTRLEN] = "";
-	  buf_puts (&out, "[AF_INET6]");
-	  if (addr_is_defined)
-	    {
-	      getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
-			  buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
-	      buf_puts (&out, buf);
-	    }
-	  if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_puts (&out, separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
-	}
-      break;
-    default:
-      ASSERT(0);
-    }
-  return BSTR (&out);
+	
+	int port;
+	switch(addr->addr.sa.sa_family)
+  {
+	  case AF_INET:
+		port= ntohs (addr->addr.in4.sin_port);
+		buf_puts (&out, "[AF_INET]");
+		break;
+	  case AF_INET6:
+		port= ntohs (addr->addr.in6.sin6_port);
+		buf_puts (&out, "[AF_INET6]");
+		break;
+	  default:
+		ASSERT(0);
+  }
+	
+	if (!(flags & PS_DONT_SHOW_ADDR))
+	  {
+		getnameinfo(&addr->addr.sa,  addr->addr.sa.sa_len,
+					buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
+		buf_puts (&out, buf);
+	  }
+	if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
+		&& port)
+	  {
+		if (separator)
+			buf_puts (&out, separator);
+		
+		buf_printf (&out, "%d", port);
+	  }
+	
+	return BSTR (&out);
 }
 
 const char *
Signature · 13 lines
1.7.9.6 (Apple Git-31.1)


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#24 lore raw

PATCH 1/1 Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

In reply to Arne Schwabe

Some BSD need the right salen, max(sizeof(v4),sizeof(v6)) does not work. Since sa_len is not a member in sockaddr for Linux and Windows and SA_LEN is no available on Darwin, NetBSD explicitly set salem

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/socket.c |   89 ++++++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 50 deletions(-)
src/openvpn/socket.c +39 -50
index 505cf3b..a2f90ca 100644
@@ -2168,57 +2168,46 @@ print_sockaddr_ex (const struct openvpn_sockaddr *addr,
 		   const unsigned int flags,
 		   struct gc_arena *gc)
 {
-  struct buffer out = alloc_buf_gc (128, gc);
-  bool addr_is_defined;
-  addr_is_defined = addr_defined (addr);
-  if (!addr_is_defined) {
-    return "[undef]";
-  }
-  switch(addr->addr.sa.sa_family)
-    {
-    case AF_INET:
-	{
-	  const int port= ntohs (addr->addr.in4.sin_port);
-	  buf_puts (&out, "[AF_INET]");
-
-	  if (!(flags & PS_DONT_SHOW_ADDR))
-	    buf_printf (&out, "%s", (addr_defined (addr) ? inet_ntoa (addr->addr.in4.sin_addr) : "[undef]"));
-
-	  if (((flags & PS_SHOW_PORT) || (addr_defined (addr) && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_printf (&out, "%s", separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
+	struct buffer out = alloc_buf_gc (128, gc);
+	bool addr_is_defined;
+	char buf[NI_MAXHOST] = "";
+	
+	addr_is_defined = addr_defined (addr);
+	if (!addr_is_defined) {
+		return "[undef]";
 	}
-      break;
-    case AF_INET6:
-	{
-	  const int port= ntohs (addr->addr.in6.sin6_port);
-	  char buf[INET6_ADDRSTRLEN] = "";
-	  buf_puts (&out, "[AF_INET6]");
-	  if (addr_is_defined)
-	    {
-	      getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
-			  buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
-	      buf_puts (&out, buf);
-	    }
-	  if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_puts (&out, separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
-	}
-      break;
-    default:
-      ASSERT(0);
-    }
-  return BSTR (&out);
+	
+	int port;
+	switch(addr->addr.sa.sa_family)
+  {
+	  case AF_INET:
+		port= ntohs (addr->addr.in4.sin_port);
+		buf_puts (&out, "[AF_INET]");
+		break;
+	  case AF_INET6:
+		port= ntohs (addr->addr.in6.sin6_port);
+		buf_puts (&out, "[AF_INET6]");
+		break;
+	  default:
+		ASSERT(0);
+  }
+	
+	if (!(flags & PS_DONT_SHOW_ADDR))
+	  {
+		getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
+					buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
+		buf_puts (&out, buf);
+	  }
+	if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
+		&& port)
+	  {
+		if (separator)
+			buf_puts (&out, separator);
+		
+		buf_printf (&out, "%d", port);
+	  }
+	
+	return BSTR (&out);
 }
 
 const char *
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
author
#25 lore raw

PATCH 1/1 Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

In reply to Arne Schwabe

Some BSD need the right salen, max(sizeof(v4),sizeof(v6)) does not work. Since sa_len is not a member in sockaddr for Linux and Windows and SA_LEN is no available on Darwin, NetBSD explicitly set salem

Signed-off-by Arne Schwabe <[email protected]...>
---
 src/openvpn/socket.c |   89 ++++++++++++++++++++++----------------------------
 1 file changed, 39 insertions(+), 50 deletions(-)
src/openvpn/socket.c +39 -50
index 505cf3b..a2f90ca 100644
@@ -2168,57 +2168,46 @@ print_sockaddr_ex (const struct openvpn_sockaddr *addr,
 		   const unsigned int flags,
 		   struct gc_arena *gc)
 {
-  struct buffer out = alloc_buf_gc (128, gc);
-  bool addr_is_defined;
-  addr_is_defined = addr_defined (addr);
-  if (!addr_is_defined) {
-    return "[undef]";
-  }
-  switch(addr->addr.sa.sa_family)
-    {
-    case AF_INET:
-	{
-	  const int port= ntohs (addr->addr.in4.sin_port);
-	  buf_puts (&out, "[AF_INET]");
-
-	  if (!(flags & PS_DONT_SHOW_ADDR))
-	    buf_printf (&out, "%s", (addr_defined (addr) ? inet_ntoa (addr->addr.in4.sin_addr) : "[undef]"));
-
-	  if (((flags & PS_SHOW_PORT) || (addr_defined (addr) && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_printf (&out, "%s", separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
+	struct buffer out = alloc_buf_gc (128, gc);
+	bool addr_is_defined;
+	char buf[NI_MAXHOST] = "";
+	
+	addr_is_defined = addr_defined (addr);
+	if (!addr_is_defined) {
+		return "[undef]";
 	}
-      break;
-    case AF_INET6:
-	{
-	  const int port= ntohs (addr->addr.in6.sin6_port);
-	  char buf[INET6_ADDRSTRLEN] = "";
-	  buf_puts (&out, "[AF_INET6]");
-	  if (addr_is_defined)
-	    {
-	      getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
-			  buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
-	      buf_puts (&out, buf);
-	    }
-	  if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_puts (&out, separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
-	}
-      break;
-    default:
-      ASSERT(0);
-    }
-  return BSTR (&out);
+	
+	int port;
+	switch(addr->addr.sa.sa_family)
+  {
+	  case AF_INET:
+		port= ntohs (addr->addr.in4.sin_port);
+		buf_puts (&out, "[AF_INET]");
+		break;
+	  case AF_INET6:
+		port= ntohs (addr->addr.in6.sin6_port);
+		buf_puts (&out, "[AF_INET6]");
+		break;
+	  default:
+		ASSERT(0);
+  }
+	
+	if (!(flags & PS_DONT_SHOW_ADDR))
+	  {
+		getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
+					buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
+		buf_puts (&out, buf);
+	  }
+	if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
+		&& port)
+	  {
+		if (separator)
+			buf_puts (&out, separator);
+		
+		buf_printf (&out, "%d", port);
+	  }
+	
+	return BSTR (&out);
 }
 
 const char *
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
#26 lore raw

Re: [Openvpn-devel] [PATCH] Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

Hi,

Quoted 1 line · Arne Schwabe

Some BSD need the right salen, max(sizeof(v4),sizeof(v6)) does not work. Since sa_len is not a member in sockaddr for Linux and Windows and SA_LEN is no available on Darwin, NetBSD explicitly set salem

"Something sneaked into your keyboard"...

src/openvpn/socket.c print_sockaddr_ex (const struct openvpn_sockaddr *addr, · line 2198

Quoted 2 lines
src/openvpn/socket.c +2 -0
+		getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
+					buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);

... this very much looks like v1 of the patch...? :-)

Anyway, what about defining an openvpn_sa_len() macro that will either use SA_LEN() if it exists, or fall back to

  #define openvpn_sa_len(addr) ( addr->sa.sa_family == AF_INET? sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6) )

this will, of course, bite us when we implement IPv8, but should be fine for 2.3...

(Is there something magic in Autoconf to solve this? Must be a standard problem...)

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#27 lore raw

Re: [Openvpn-devel] [PATCH] Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 03.09.12 09:09, schrieb Gert Doering:

Quoted 4 lines

Hi,

Quoted 1 line · Arne Schwabe

Some BSD need the right salen, max(sizeof(v4),sizeof(v6)) does not

work. Since sa_len is not a member in sockaddr for Linux and Windows and SA_LEN is no available on Darwin, NetBSD explicitly set salem

Quoted 6 lines

"Something sneaked into your keyboard"...

Quoted 2 lines

+ getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
+ buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);

Quoted 6 lines

... this very much looks like v1 of the patch...? :-)

Anyway, what about defining an openvpn_sa_len() macro that will either use SA_LEN() if it exists, or fall back to

#define openvpn_sa_len(addr) ( addr->sa.sa_family == AF_INET?

sizeof(struct sockaddr_in) : sizeof(struct sockaddr_in6) )

Quoted 3 lines

this will, of course, bite us when we implement IPv8, but should be fine for 2.3...

I will try to avoid this for now... I hope to convert openvpn to fully using getnameinfo/getaddrinfo
one day so that we do not need these deep look into the sockaddr anymore.

Quoted 4 lines

(Is there something magic in Autoconf to solve this? Must be a standard problem...)

I think most people don't deal with raw sockaddr structs. getaddrinfo returns ai_addrlen.

Arne
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (Darwin)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iEYEARECAAYFAlBEWZUACgkQe8+cMNS4zRf5/QCeKPwCJKva6l05i18OWv4tZJFB BuQAn36NT4DZoi5e9RTqm3XAfBQqiWup
=e27Z
-----END PGP SIGNATURE-----

author
#28 lore raw

PATCH 1/1 Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

In reply to Arne Schwabe
---
 src/openvpn/socket.c |   68 ++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 38 deletions(-)
src/openvpn/socket.c +30 -38
index 505cf3b..a9adf3f 100644
@@ -2164,60 +2164,52 @@ print_sockaddr (const struct openvpn_sockaddr *addr, struct gc_arena *gc)
 
 const char *
 print_sockaddr_ex (const struct openvpn_sockaddr *addr,
-		   const char* separator,
-		   const unsigned int flags,
-		   struct gc_arena *gc)
+				   const char* separator,
+				   const unsigned int flags,
+				   struct gc_arena *gc)
 {
   struct buffer out = alloc_buf_gc (128, gc);
   bool addr_is_defined;
+  char buf[NI_MAXHOST] = "";
+
   addr_is_defined = addr_defined (addr);
   if (!addr_is_defined) {
     return "[undef]";
   }
+
+  int port;
+  int salen;
   switch(addr->addr.sa.sa_family)
     {
     case AF_INET:
-	{
-	  const int port= ntohs (addr->addr.in4.sin_port);
-	  buf_puts (&out, "[AF_INET]");
-
-	  if (!(flags & PS_DONT_SHOW_ADDR))
-	    buf_printf (&out, "%s", (addr_defined (addr) ? inet_ntoa (addr->addr.in4.sin_addr) : "[undef]"));
-
-	  if (((flags & PS_SHOW_PORT) || (addr_defined (addr) && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_printf (&out, "%s", separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
-	}
+      port= ntohs (addr->addr.in4.sin_port);
+      buf_puts (&out, "[AF_INET]");
+      salen = sizeof (struct sockaddr_in);
       break;
     case AF_INET6:
-	{
-	  const int port= ntohs (addr->addr.in6.sin6_port);
-	  char buf[INET6_ADDRSTRLEN] = "";
-	  buf_puts (&out, "[AF_INET6]");
-	  if (addr_is_defined)
-	    {
-	      getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
-			  buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
-	      buf_puts (&out, buf);
-	    }
-	  if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_puts (&out, separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
-	}
+      port= ntohs (addr->addr.in6.sin6_port);
+      buf_puts (&out, "[AF_INET6]");
+      salen = sizeof (struct sockaddr_in6);
       break;
     default:
       ASSERT(0);
     }
+	
+  if (!(flags & PS_DONT_SHOW_ADDR))
+    {
+      getnameinfo(&addr->addr.sa, salen,
+                  buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
+      buf_puts (&out, buf);
+    }
+  if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
+      && port)
+    {
+      if (separator)
+        buf_puts (&out, separator);
+		
+      buf_printf (&out, "%d", port);
+    }
+	
   return BSTR (&out);
 }
Signature · 1 line
1.7.9.6 (Apple Git-31.1)
#29 lore raw

Re: [Openvpn-devel] [PATCH] Simplify print_sockaddr_ex function, merge duplicate ipv4/ipv6 logic

Hi,

this looks like it should work both on *BSD and on Linux - explicit salen parameter, based on sa_family, no special macros (since we have the switch/case part anyway).

And the tabs are right \o/

ACK ;-)

gert

Quoted 114 lines · Arne Schwabe
---
 src/openvpn/socket.c |   68 ++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 38 deletions(-)
src/openvpn/socket.c +30 -38
index 505cf3b..a9adf3f 100644
@@ -2164,60 +2164,52 @@ print_sockaddr (const struct openvpn_sockaddr *addr, struct gc_arena *gc)
 
 const char *
 print_sockaddr_ex (const struct openvpn_sockaddr *addr,
-		   const char* separator,
-		   const unsigned int flags,
-		   struct gc_arena *gc)
+				   const char* separator,
+				   const unsigned int flags,
+				   struct gc_arena *gc)
 {
   struct buffer out = alloc_buf_gc (128, gc);
   bool addr_is_defined;
+  char buf[NI_MAXHOST] = "";
+
   addr_is_defined = addr_defined (addr);
   if (!addr_is_defined) {
     return "[undef]";
   }
+
+  int port;
+  int salen;
   switch(addr->addr.sa.sa_family)
     {
     case AF_INET:
-	{
-	  const int port= ntohs (addr->addr.in4.sin_port);
-	  buf_puts (&out, "[AF_INET]");
-
-	  if (!(flags & PS_DONT_SHOW_ADDR))
-	    buf_printf (&out, "%s", (addr_defined (addr) ? inet_ntoa (addr->addr.in4.sin_addr) : "[undef]"));
-
-	  if (((flags & PS_SHOW_PORT) || (addr_defined (addr) && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_printf (&out, "%s", separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
-	}
+      port= ntohs (addr->addr.in4.sin_port);
+      buf_puts (&out, "[AF_INET]");
+      salen = sizeof (struct sockaddr_in);
       break;
     case AF_INET6:
-	{
-	  const int port= ntohs (addr->addr.in6.sin6_port);
-	  char buf[INET6_ADDRSTRLEN] = "";
-	  buf_puts (&out, "[AF_INET6]");
-	  if (addr_is_defined)
-	    {
-	      getnameinfo(&addr->addr.sa, sizeof (struct sockaddr_in6),
-			  buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
-	      buf_puts (&out, buf);
-	    }
-	  if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
-	      && port)
-	    {
-	      if (separator)
-		buf_puts (&out, separator);
-
-	      buf_printf (&out, "%d", port);
-	    }
-	}
+      port= ntohs (addr->addr.in6.sin6_port);
+      buf_puts (&out, "[AF_INET6]");
+      salen = sizeof (struct sockaddr_in6);
       break;
     default:
       ASSERT(0);
     }
+	
+  if (!(flags & PS_DONT_SHOW_ADDR))
+    {
+      getnameinfo(&addr->addr.sa, salen,
+                  buf, sizeof (buf), NULL, 0, NI_NUMERICHOST);
+      buf_puts (&out, buf);
+    }
+  if (((flags & PS_SHOW_PORT) || (addr_is_defined && (flags & PS_SHOW_PORT_IF_DEFINED)))
+      && port)
+    {
+      if (separator)
+        buf_puts (&out, separator);
+		
+      buf_printf (&out, "%d", port);
+    }
+	
   return BSTR (&out);
 }
Signature · 13 lines
1.7.9.6 (Apple Git-31.1)


------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...