[Openvpn-devel] [PATCH v2] Use separate list for per-client push options

2016-10-09 Thread Lev Stipakov 
v2:
 - Also move ifconfig and ipv6-ifconfig to separate options list

Move client-specific push options (currently peer-id and cipher) to
separate list, which is deallocated after push_reply
has been send. This makes sure that options fit into buf,
not duplicated nor leak memory on renegotiation.

Signed-off-by: Lev Stipakov 
---
 src/openvpn/push.c | 186 ++---
 1 file changed, 104 insertions(+), 82 deletions(-)

diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index a1b999e..f7bcad1 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -40,26 +40,29 @@
 
 #if P2MP
 
+static char push_reply_cmd[] = "PUSH_REPLY";
+
 /**
- * Add an option to the push list by providing a format string.
+ * Add an option to the given push list by providing a format string.
  *
  * The string added to the push options is allocated in o->gc, so the caller
  * does not have to preserve anything.
  *
- * @param oThe current connection's options
- * @param msglevel The message level to use when printing errors
+ * @param gc   GC arena where options are allocated
+ * @param push_list Push list containing options
+ * @param msglevel  The message level to use when printing errors
  * @param fmt  Format string for the option
  * @param ...  Format string arguments
  *
  * @return true on success, false on failure.
  */
-static bool push_option_fmt(struct options *o, int msglevel,
+static bool push_option_fmt(struct gc_arena *gc, struct push_list *push_list, 
int msglevel,
 const char *fmt, ...)
 #ifdef __GNUC__
 #if __USE_MINGW_ANSI_STDIO
-__attribute__ ((format (gnu_printf, 3, 4)))
+__attribute__ ((format (gnu_printf, 4, 5)))
 #else
-__attribute__ ((format (__printf__, 3, 4)))
+__attribute__ ((format (__printf__, 4, 5)))
 #endif
 #endif
 ;
@@ -295,16 +298,39 @@ send_push_request (struct context *c)
 /**
  * Prepare push options, based on local options and available peer info.
  *
- * @param options  Connection options
- * @param tls_multiTLS state structure for the current tunnel
+ * @param context  context structure storing data for VPN tunnel
+ * @param gc   gc arena for allocating push options
+ * @param push_listpush list to where options are added
  *
  * @return true on success, false on failure.
  */
 static bool
-prepare_push_reply (struct options *o, struct tls_multi *tls_multi)
+prepare_push_reply (struct context *c, struct gc_arena *gc, struct push_list 
*push_list)
 {
   const char *optstr = NULL;
+  const struct tls_multi *tls_multi = c->c2.tls_multi;
   const char * const peer_info = tls_multi->peer_info;
+  struct options *o = >options;
+
+  /* ipv6 */
+  if (c->c2.push_ifconfig_ipv6_defined && !o->push_ifconfig_ipv6_blocked)
+{
+  push_option_fmt (gc, push_list, M_USAGE, "ifconfig-ipv6 %s/%d %s",
+  print_in6_addr (c->c2.push_ifconfig_ipv6_local, 0, gc),
+  c->c2.push_ifconfig_ipv6_netbits,
+  print_in6_addr (c->c2.push_ifconfig_ipv6_remote, 0, gc));
+}
+
+  /* ipv4 */
+  if (c->c2.push_ifconfig_defined && c->c2.push_ifconfig_local && 
c->c2.push_ifconfig_remote_netmask)
+{
+  in_addr_t ifconfig_local = c->c2.push_ifconfig_local;
+  if (c->c2.push_ifconfig_local_alias)
+   ifconfig_local = c->c2.push_ifconfig_local_alias;
+  push_option_fmt (gc, push_list, M_USAGE, "ifconfig %s %s",
+  print_in_addr_t (ifconfig_local, 0, gc),
+  print_in_addr_t (c->c2.push_ifconfig_remote_netmask, 0, 
gc));
+}
 
   /* Send peer-id if client supports it */
   optstr = peer_info ? strstr(peer_info, "IV_PROTO=") : NULL;
@@ -314,7 +340,7 @@ prepare_push_reply (struct options *o, struct tls_multi 
*tls_multi)
   int r = sscanf(optstr, "IV_PROTO=%d", );
   if ((r == 1) && (proto >= 2))
{
- push_option_fmt(o, M_USAGE, "peer-id %d", tls_multi->peer_id);
+ push_option_fmt(gc, push_list, M_USAGE, "peer-id %d", 
tls_multi->peer_id);
}
 }
 
@@ -324,7 +350,7 @@ prepare_push_reply (struct options *o, struct tls_multi 
*tls_multi)
   /* if we have already created our key, we cannot change our own
* cipher, so disable NCP and warn = explain why
*/
-  struct tls_session *session = _multi->session[TM_ACTIVE];
+  const struct tls_session *session = _multi->session[TM_ACTIVE];
   if ( session->key[KS_PRIMARY].crypto_options.key_ctx_bi.initialized )
{
   msg( M_INFO, "PUSH: client wants to negotiate cipher (NCP), but "
@@ -335,86 +361,76 @@ prepare_push_reply (struct options *o, struct tls_multi 
*tls_multi)
{
  /* Push the first cipher from --ncp-ciphers to the client.
   * TODO: actual negotiation, instead of server dictatorship. */
- char *push_cipher = string_alloc(o->ncp_ciphers, >gc);
+ char *push_cipher =

Re: [Openvpn-devel] strtok on Windows

2016-10-09 Thread Selva Nair 
On Sun, Oct 9, 2016 at 11:02 AM, Selva Nair  wrote:

> As per MSDN, strtok on Windows is thread-safe --- uses thread-local
> static for the internal state (though not reentrant). Anyone knows
> whether this is true even when mingw is used --- does it use strtok from
> Microsoft's run time or some internal implementation?
>

Well, I went ahead and tested it: indeed strtok is thread-safe on windows
(obviously not reentrant).

I guess it uses strtok in MS runtime, not a typical C library
implementation.
(Tested on Debian jessie with mingw-w64 version 4.9.1-19+14.3.)

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] strtok on Windows

2016-10-09 Thread Selva Nair 
Hi,

As per MSDN, strtok on Windows is thread-safe --- uses thread-local static
for the internal state (though not reentrant). Anyone knows whether this is
true even when mingw is used --- does it use strtok from Microsoft's run
time or some internal implementation?

I have patch for the GUI that uses strtok_r which is not natively available
on Windows. Just realized some older versions of mingw doesn't have an
implementation of strtok_r. For my use-case a thread-safe strtok will work
as reentrancy is not required.

Thanks,

Selva
--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH applied] Fix --multihome for IPv6 on 64bit BSD systems.

2016-10-09 Thread Gert Doering 
Thanks, Arne.

Patch has been applied to the following branches

commit 3fb246e38fc670c7dfff8ce4521c75c95c766c9e  (master)
commit f7aba873ca593eb2ab8a4a358cd6c06ef46a09e1  (release/2.3)
Author: Gert Doering
Date:   Sun Oct 9 12:09:29 2016 +0200

 Fix --multihome for IPv6 on 64bit BSD systems.

 Signed-off-by: Gert Doering 
 Acked-by: Arne Schwabe 
 Message-Id: <20161009100929.46472-1-g...@greenie.muc.de>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg12626.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Fix --multihome for IPv6 on 64bit BSD systems.

2016-10-09 Thread Gert Doering 
The old code only worked if "struct openvpn*pktinfo" happened to use
the same structure packing as the CMSG_SPACE() / CMSG_LEN() macros
(which are part of the official API, see RFC 2292).

Get rid of "struct openvpn_*_pktinfo" definitions, replace them by
an opaque buffer sized large enough to fit IPv4 and IPv6 packet info
messages, as defined by CMSG_SPACE(sizeof(struct ...)).

On 32 bit platforms, the net result is the same.  On 64 bit platforms,
the new buffer is bigger than openvpn_pktinfo was, fixing an overflow
with ipi6_ifindex corruption on reception, and EINVAL on sendmsg().

The IPv4 related changes are only side effects of using the new buffer.

Fixes: FreeBSD 10.3/amd64, FreeBSD 9.3/sparc64, OpenBSD 6.0/amd64,
   NetBSD 7.0.1/i386.

Note: --multihome for IPv4 on NetBSD is still broken and non-fixable(!)
   as NetBSD lacks the necessary kernel code for the sendmsg() side.

Verified that "--multihome works as well as before" on FreeBSD 7.4/amd64,
   NetBSD 5.1/amd64, OpenBSD 4.9/i386, Linux/x86_64, Linux/i386,
   OpenSolaris 10 (--multihome needs -D_XPG4_2, see trac #750)

See also: ip(4), ip6(4), recv(2)

Trac #634, #327, #28

Signed-off-by: Gert Doering 
---
 src/openvpn/socket.c | 57 +++-
 1 file changed, 25 insertions(+), 32 deletions(-)

diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index e096132..184c7ad 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -2863,27 +2863,16 @@ link_socket_read_tcp (struct link_socket *sock,
 
 #if ENABLE_IP_PKTINFO
 
-#pragma pack(1) /* needed to keep structure size consistent for 32 vs. 64-bit 
architectures */
-struct openvpn_in4_pktinfo
-{
-  struct cmsghdr cmsghdr;
+/* make the buffer large enough to handle ancilliary socket data for
+ * both IPv4 and IPv6 destination addresses, plus padding (see RFC 2292)
+ */
 #if defined(HAVE_IN_PKTINFO) && defined(HAVE_IPI_SPEC_DST)
-  struct in_pktinfo pi4;
-#elif defined(IP_RECVDSTADDR)
-  struct in_addr pi4;
+#define PKTINFO_BUF_SIZE max_int( CMSG_SPACE(sizeof (struct in6_pktinfo)), \
+ CMSG_SPACE(sizeof (struct in_pktinfo)) )
+#else
+#define PKTINFO_BUF_SIZE max_int( CMSG_SPACE(sizeof (struct in6_pktinfo)), \
+ CMSG_SPACE(sizeof (struct in_addr)) )
 #endif
-};
-struct openvpn_in6_pktinfo
-{
-  struct cmsghdr cmsghdr;
-  struct in6_pktinfo pi6;
-};
-
-union openvpn_pktinfo {
-   struct openvpn_in4_pktinfo msgpi4;
-   struct openvpn_in6_pktinfo msgpi6;
-};
-#pragma pack()
 
 static socklen_t
 link_socket_read_udp_posix_recvmsg (struct link_socket *sock,
@@ -2891,7 +2880,7 @@ link_socket_read_udp_posix_recvmsg (struct link_socket 
*sock,
struct link_socket_actual *from)
 {
   struct iovec iov;
-  union openvpn_pktinfo opi;
+  uint8_t pktinfo_buf[PKTINFO_BUF_SIZE];
   struct msghdr mesg;
   socklen_t fromlen = sizeof (from->dest.addr);
 
@@ -2901,8 +2890,8 @@ link_socket_read_udp_posix_recvmsg (struct link_socket 
*sock,
   mesg.msg_iovlen = 1;
   mesg.msg_name = >dest.addr;
   mesg.msg_namelen = fromlen;
-  mesg.msg_control = 
-  mesg.msg_controllen = sizeof opi;
+  mesg.msg_control = pktinfo_buf;
+  mesg.msg_controllen = sizeof pktinfo_buf;
   buf->len = recvmsg (sock->sd, , 0);
   if (buf->len >= 0)
 {
@@ -2914,13 +2903,14 @@ link_socket_read_udp_posix_recvmsg (struct link_socket 
*sock,
 #if defined(HAVE_IN_PKTINFO) && defined(HAVE_IPI_SPEC_DST)
  && cmsg->cmsg_level == SOL_IP 
  && cmsg->cmsg_type == IP_PKTINFO
+ && cmsg->cmsg_len >= CMSG_LEN(sizeof(struct in_pktinfo)) )
 #elif defined(IP_RECVDSTADDR)
  && cmsg->cmsg_level == IPPROTO_IP
  && cmsg->cmsg_type == IP_RECVDSTADDR
+ && cmsg->cmsg_len >= CMSG_LEN(sizeof(struct in_addr)) )
 #else
 #error ENABLE_IP_PKTINFO is set without IP_PKTINFO xor IP_RECVDSTADDR (fix 
syshead.h)
 #endif
- && cmsg->cmsg_len >= sizeof (struct openvpn_in4_pktinfo))
{
 #if defined(HAVE_IN_PKTINFO) && defined(HAVE_IPI_SPEC_DST)
  struct in_pktinfo *pkti = (struct in_pktinfo *) CMSG_DATA (cmsg);
@@ -2936,7 +2926,7 @@ link_socket_read_udp_posix_recvmsg (struct link_socket 
*sock,
  && CMSG_NXTHDR (, cmsg) == NULL
  && cmsg->cmsg_level == IPPROTO_IPV6 
  && cmsg->cmsg_type == IPV6_PKTINFO
- && cmsg->cmsg_len >= sizeof (struct openvpn_in6_pktinfo))
+ && cmsg->cmsg_len >= CMSG_LEN(sizeof(struct in6_pktinfo)) )
{
  struct in6_pktinfo *pkti6 = (struct in6_pktinfo *) CMSG_DATA (cmsg);
  from->pi.in6.ipi6_ifindex = pkti6->ipi6_ifindex;
@@ -3007,7 +2997,7 @@ link_socket_write_udp_posix_sendmsg (struct link_socket 
*sock,
   struct iovec iov;
   struct msghdr mesg;
   struct cmsghdr *cmsg;
-  union openvpn_pktinfo opi;
+  uint8_t pktinfo_buf[PKTINFO_BUF_SIZE];
 
   iov.iov_base = BPTR (buf);
   iov.iov_len = BLEN (buf);
@@ -3019,12 +3009,12 @@