Re: [Openvpn-devel] [PATCH v2] Support for disabled peer-id

[Steffan Karger]

Hi,

The patch looks functionally ok.  Just one minor remark:

On 09-10-15 16:13, Lev Stipakov wrote:

@@ -103,10 +107,14 @@ multi_get_create_instance_udp (struct multi_context *m, 
bool *floated)
  hash_add_fast (hash, bucket, >real, hv, mi);
  mi->did_real_hash = true;

+ /* In future we might want to use P_DATA_V2 but not need 
peer-id/float functionality */
  for (i = 0; i < m->max_clients; ++i)
{
  if (!m->instances[i])
{
+ /* issued peer-id should fit into 3 bytes to 
avoid wrap and cannot have reserved value 0xFF */
+ ASSERT(i < 0xFF);
+
  mi->context.c2.tls_multi->peer_id = i;
  m->instances[i] = mi;
  break;


Checking against the max peer-id indeed makes sense, but this assert can 
be made simpler by just putting


   ASSERT (m->max-clients < 0xFF);

before the loop.  Additionally, I think we should add a user-friendly 
check in options.c, to give a proper error message on startup when a 
user tries to set --max-clients >= 0xFF.


Finally, why did you put this comment at this location?  Shouldn't it be 
in the place where we *send* the peer-id?  (That is the part that is not 
implemented now, right?)


-Steffan

[Openvpn-devel] [PATCH] Handle ctrl-C and ctrl-break events on Windows

[Selva Nair]

On Mon, Nov 9, 2015 at 2:03 PM, Selva Nair  wrote:
>> It's probably okay to just make CTRL-c generate a SIGTERM as F4 is
>> already doing.
>>
>> James
>
> Thanks for the comment.
>
> In the interactive mode, the console is opened with no
> ENABLE_PROCESSED_INPUT so ctrl-C will be delivered as key-board input
> and could be handled just like F4.
>
> With nssm, the console is shared with nssm, so ctrl-C is delivered as
> a signal. I'll send a patch handling both cases.

The patch is in the next email. 

Handling the key-board input is easy, but making the callback interrupt the 
event loop
looks tricky. I am not well versed with the event handling in the code, so 
opted to generate a key-pressed event. If there is a better way please suggest.

Simply calling throw_signal in the callback will work, but could take several 
seconds
before its noticed.

Tested on windows 7 with cmd-line use and start/stop with nssm. For nssm, the 
default 
delay after ctrl-C is 1500 msec which is not enough for the process to exit if 
exit-notify
is being used. About 1 second per extit notify trials + an extra 1 second is 
suggested. 

 src/openvpn/win32.c | 54 +
 1 file changed, 54 insertions(+)

-- 
2.6.2

Re: [Openvpn-devel] [PATCH v3] Use adapter index instead of name

[Gert Doering]

Hi,

On Wed, Nov 11, 2015 at 01:48:07PM +0200, Lev Stipakov wrote:
> @@ -1301,18 +1303,20 @@ do_ifconfig (struct tuntap *tt,
>  if ( do_ipv6 )
>{
>   char * saved_actual;
> + char iface[64];
>  
>   if (!strcmp (actual, "NULL"))
> msg (M_FATAL, "Error: When using --tun-ipv6, if you have more than 
> one TAP-Windows adapter, you must also specify --dev-node");
>  
> - /* example: netsh interface ipv6 set address MyTap 2001:608:8003::d 
> store=active */
> + openvpn_snprintf(iface, sizeof(iface), "interface=%lu", 
> get_adapter_index_flexible(actual));

Uh.  I'm afraid if I speak up once more, you'll stop talking to me...
(or not give me any beer next time we meet :) )

> + /* example: netsh interface ipv6 set address interface=42 
> 2001:608:8003::d store=active */
>   argv_printf (,
> - "%s%sc interface ipv6 set address %s %s store=active",
> +  "%s%sc interface ipv6 set address %s %s store=active",
>get_win_sys_path(),
>NETSH_PATH_SUFFIX,
> -  actual,
> -  ifconfig_ipv6_local );
> -

... but wouldn't

argv_printf (,
"%s%sc interface ipv6 set address interface=%lu %s 
store=active",
 get_win_sys_path(),
 NETSH_PATH_SUFFIX,
 get_adapter_index_flexible(actual),
 ifconfig_ipv6_local);

(without storing into a temp variable) work "just fine"?

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

[Openvpn-devel] [PATCH] extend management interface command "state"

[Heiko Hund]

Currently the state command shows only the tun/tap IPv4 address. The
IPv4 address of the remote peer is also displayed. In case you connect
via IPv6 it just shows the first 4 bytes of the address in IPv4 notation.

This patch extends the state command, so it handles IPv6 addresses.
In addition it also displays the local address and the both port numbers
of the connection, e.g.

1447250958,CONNECTED,SUCCESS,10.0.0.2,fd00::1,1193,fd00::2,6492,fdff::1002

Signed-off-by: Heiko Hund 
---
 doc/management-notes.txt |   17 ++
 src/openvpn/forward.c|6 +++--
 src/openvpn/init.c   |   55 +-
 src/openvpn/manage.c |   27 ++-
 src/openvpn/manage.h |   10 ++---
 src/openvpn/route.c  |6 +++--
 src/openvpn/sig.c|6 +++--
 src/openvpn/socket.c |   23 +--
 src/openvpn/socket.h |1 +
 src/openvpn/ssl.c|   12 ++
 src/openvpn/tun.c|6 +++--
 11 files changed, 122 insertions(+), 47 deletions(-)

diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 0265d55..f68f3db 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -366,14 +366,23 @@ Command examples:
   same time enable real-time state notification
  of future state transitions.

-The output format consists of 4 comma-separated parameters:
+The output format consists of up to 9 comma-separated parameters:
   (a) the integer unix date/time,
   (b) the state name,
   (c) optional descriptive string (used mostly on RECONNECTING
   and EXITING to show the reason for the disconnect),
-  (d) optional TUN/TAP local IP address (shown for ASSIGN_IP
-  and CONNECTED), and
-  (e) optional address of remote server (OpenVPN 2.1 or higher).
+  (d) optional TUN/TAP local IPv4 address
+  (e) optional address of remote server,
+  (f) optional port of remote server,
+  (g) optional local address,
+  (h) optional local port, and
+  (i) optional TUN/TAP local IPv6 address.
+
+Fields (e)-(h) are shown for CONNECTED state,
+(d) and (i) are shown for ASSIGN_IP and CONNECTED states.
+
+(e) is available starting from OpenVPN 2.1
+(f)-(i) are available starting from OpenVPN 2.4

 Real-time state notifications will have a ">STATE:" prefix
 prepended to them.
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 62eb6fc..fab5e3f 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -208,8 +208,10 @@ check_connection_established_dowork (struct context *c)
  management_set_state (management,
OPENVPN_STATE_GET_CONFIG,
NULL,
-   0,
-   0);
+NULL,
+NULL,
+NULL,
+NULL);
}
 #endif
  /* fire up push request right away (already 1s delayed) */
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index c5c0ab6..5c17087 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -44,6 +44,7 @@
 #include "ping.h"
 #include "mstats.h"
 #include "ssl_verify.h"
+#include "forward-inline.h"

 #include "memdbg.h"

@@ -1273,26 +1274,48 @@ initialization_sequence_completed (struct context *c, 
const unsigned int flags)
   /* Tell management interface that we initialized */
   if (management)
 {
-  in_addr_t tun_local = 0;
-  in_addr_t tun_remote = 0; /* FKS */
+  in_addr_t *tun_local = NULL;
+  struct in6_addr *tun_local6 = NULL;
+  struct openvpn_sockaddr local, remote;
+  struct link_socket_actual *actual;
+  socklen_t sa_len = sizeof(local);
   const char *detail = "SUCCESS";
-  if (c->c1.tuntap)
-   tun_local = c->c1.tuntap->local;
-  /* TODO(jjo): for ipv6 this will convert some 32bits in the ipv6 addr
-   *to a meaningless ipv4 address.
-   *In any case, is somewhat inconsistent to send local tunnel
-   *addr with remote _endpoint_ addr (?)
-   */
-  tun_remote = htonl 
(c->c1.link_socket_addr.actual.dest.addr.in4.sin_addr.s_addr);
   if (flags & ISC_ERRORS)
-   detail = "ERROR";
+detail = "ERROR";
+
+  CLEAR (local);
+  actual = _link_socket_info(c)->lsa->actual;
+  remote = actual->dest;
+  getsockname(c->c2.link_socket->sd, , _len);
+#if ENABLE_IP_PKTINFO
+  if (!addr_defined())
+{
+  switch (local.addr.sa.sa_family)
+{
+case AF_INET:
+  local.addr.in4.sin_addr = actual->pi.in4.ipi_spec_dst;
+  break;
+case AF_INET6:
+  local.addr.in6.sin6_addr = actual->pi.in6.ipi6_addr;
+  break;
+}
+}
+#endif
+
+  if 

[Openvpn-devel] [PATCH] Avoid partial authentication state when using --disabled in CCD configs

[David Sommerseth]

From: David Sommerseth 

If an openvpn server is configured with --client-config-dir and a client
configuration file contains 'disabled', it is supposed to tell the client
it is not authorized to use the service.

This patch will ensure that the internal state in this scenario is a
complete CAS_FAILED state, and not CAS_PARTIAL if other authorization
steps passed.

Trac: #521
Tested-by: Eric Crist 
Signed-off-by: David Sommerseth 
---
 src/openvpn/multi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 7c3aaac..e999450 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1858,6 +1858,7 @@ multi_connection_established (struct multi_context *m, 
struct multi_instance *mi
{
  msg (D_MULTI_ERRORS, "MULTI: client has been rejected due to 
'disable' directive");
  cc_succeeded = false;
+ cc_succeeded_count = 0;
}

   if (cc_succeeded)
-- 
1.8.3.1

[Openvpn-devel] [PATCH] Fix "implicit declaration" compiler warning

[Lev Stipakov]

Add missing "include" directive.

Signed-off-by: Lev Stipakov 
---
 src/openvpn/mtcp.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/openvpn/mtcp.c b/src/openvpn/mtcp.c
index b27c5eb..9926d47 100644
--- a/src/openvpn/mtcp.c
+++ b/src/openvpn/mtcp.c
@@ -37,6 +37,10 @@

 #include "memdbg.h"

+#ifdef HAVE_SYS_INOTIFY_H
+#include 
+#endif
+
 /*
  * TCP States
  */
-- 
1.9.1

[Openvpn-devel] [PATCH v3] Use adapter index instead of name

[Lev Stipakov]

v3:
 * Use interface= syntax.
 * Add forward declaration of get_adapter_index_flexible to get rid of warning.

v2:
 * Remove netsh call which uses adapter name. After thoughtful testing
turns out that "adapter name" code branch is never used.

Some windows machines get weird issues with netsh when using
adapter name on "netsh.exe interface ipv6 set address" command.

Changed logic to get adapter index and use it instead of adapter
name for netsh set address command. if unable to get adapter index,
try with adapter name.

Signed-off-by: Olli Mannisto 
Signed-off-by: Lev Stipakov 
---
 src/openvpn/tun.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 24a61ec..070fd18 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -67,6 +67,8 @@ static void netsh_command (const struct argv *a, int n);

 static const char *netsh_get_id (const char *dev_node, struct gc_arena *gc);

+static DWORD get_adapter_index_flexible (const char *name);
+
 #endif

 #ifdef TARGET_SOLARIS
@@ -1301,18 +1303,20 @@ do_ifconfig (struct tuntap *tt,
 if ( do_ipv6 )
   {
char * saved_actual;
+   char iface[64];

if (!strcmp (actual, "NULL"))
  msg (M_FATAL, "Error: When using --tun-ipv6, if you have more than 
one TAP-Windows adapter, you must also specify --dev-node");

-   /* example: netsh interface ipv6 set address MyTap 2001:608:8003::d 
store=active */
+   openvpn_snprintf(iface, sizeof(iface), "interface=%lu", 
get_adapter_index_flexible(actual));
+
+   /* example: netsh interface ipv6 set address interface=42 
2001:608:8003::d store=active */
argv_printf (,
-   "%s%sc interface ipv6 set address %s %s store=active",
+"%s%sc interface ipv6 set address %s %s store=active",
 get_win_sys_path(),
 NETSH_PATH_SUFFIX,
-actual,
-ifconfig_ipv6_local );
-
+iface,
+ifconfig_ipv6_local);
netsh_command (, 4);

/* explicit route needed */
-- 
1.9.1

Re: [Openvpn-devel] [PATCH v2] Use adapter index instead of name

[Gert Doering]

Hi,

On Wed, Nov 11, 2015 at 10:05:11AM +0200, Lev Stipakov wrote:
> >
> > It should actually be not very hard - we should be able to set "tt->actual"
> > to read "interface=nnn", and then it should work automagically without even
> > touching route.c at all
> 
> Setting "interface=" to "tt->actual_name" will affect all code 
> branches which use that value, for example "netsh_enable_dhcp". In this 
> particular case we cannot use index and must use interface name, 
> according to MS 
> (https://technet.microsoft.com/en-us/library/cc731521(v=ws.10).aspx#BKMK_setaddress)

Uh.  Thanks for investigating.  Why, oh why... it was naive of me to expect
consistency here, I should have known better :-(

[..]
> So, if we want to use index also for "add/del route", I'd gently modify 
> add/del_route_ipv6 and make it use "interface=" (without breaking 
> "vpn server special route" case).

For consistency, I think we should do that.  What I'd avoid is to do
the adapter_index lookup for every single route - which would imply adding
the index to struct tuntap_options or something like that which is 
available in add/del_route[_ipv6]() already.  But this is less elegant
than I hoped for, and might have to look different for 2.3 and master.

So maybe make this a separate patch?

> > What does surprise me, though, is that it works for you with just specifying
> > the interface index, without "IF" or "interface=" before it.
> >
> 
> MS says "[[ interface=] String] Specifies an interface name or index" 
> for "set address" and "add route". If I read it right, "interface" 
> prefix can be omitted. But let's use it for consistency with existing code.

... and make this ("interface=") a v3 patch, only for "set address"
for now, to move forward.

thanks,

gert

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

Re: [Openvpn-devel] [PATCH v2] Use adapter index instead of name

[Lev Stipakov]

Hi,



It should actually be not very hard - we should be able to set "tt->actual"
to read "interface=nnn", and then it should work automagically without even
touching route.c at all


Setting "interface=" to "tt->actual_name" will affect all code 
branches which use that value, for example "netsh_enable_dhcp". In this 
particular case we cannot use index and must use interface name, 
according to MS 
(https://technet.microsoft.com/en-us/library/cc731521(v=ws.10).aspx#BKMK_setaddress)


Same documentation claims that "set/delete address" accepts interface 
index only for ipv6. That said, "add/delete route" accepts index for 
both ipv4/ipv6. Besides, ipv4 syntax for "set address" is "[ name =] 
InterfaceName" and ipv6 is "[[ interface=] String]".


So, if we want to use index also for "add/del route", I'd gently modify 
add/del_route_ipv6 and make it use "interface=" (without breaking 
"vpn server special route" case).



What does surprise me, though, is that it works for you with just specifying
the interface index, without "IF" or "interface=" before it.



MS says "[[ interface=] String] Specifies an interface name or index" 
for "set address" and "add route". If I read it right, "interface" 
prefix can be omitted. But let's use it for consistency with existing code.


-Lev