Re: [Openvpn-devel] [PATCH] openvpnserv: enable interactive service to open tun

[Selva Nair]

Hi,

What I have in mind would also require editing all calls
to send_msg_iservice() which is essentially what Gert is objecting to.
So ignore me -- a separate send_msg_iservice_ex may be the
best option.

Selva

On Tue, Jun 25, 2019 at 5:00 PM Selva Nair  wrote:
>
> Hi,
>
> On Tue, Jun 25, 2019 at 4:38 PM Lev Stipakov  wrote:
> >
> > Hi,
> >
> >>
> >> The way interactive service structures are coded should not require
> >> this at all, does it? The size and message type are already in the
> >> header, so why do we need to pass it?
> >
> >
> > But we need to know the response size in send_msg_iservice() since
> > we pass it to ReadFile(). So far we assumed that response is always 
> > ask_message_t
> > and we could do sizeof(*ack). With new response type this assumption 
> > doesn't hold so
> > as Gert suggested I added another version which accepts arbitrary response 
> > type and size.
>
> My point is that, this is not in the spirit of the rest of iservice code. See
> HandleMessage in interactive.c where the data is and then interpreted
> using the header type and size.
>
> For what max-size to pass to ReadFile, we know it from the size in the header
> element of the struct.
>
> Selva
>
>
> >
> > bool
> > send_msg_iservice(HANDLE pipe, const void *data, size_t size,
> >   ack_message_t *ack, const char *context)
> > {
> > return send_msg_iservice_ex(pipe, data, size, ack, sizeof(*ack), 
> > context);
> > }
> >
> > bool
> > send_msg_iservice_ex(HANDLE pipe, const void *data, size_t size,
> >  void *response, size_t response_size, const char 
> > *context)
> > {
> >
> > Will send V2 tomorrow with this and CreateFileW changes.
> >
> >
> > --
> > -Lev


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] openvpnserv: enable interactive service to open tun

[Selva Nair]

Hi,

On Tue, Jun 25, 2019 at 4:38 PM Lev Stipakov  wrote:
>
> Hi,
>
>>
>> The way interactive service structures are coded should not require
>> this at all, does it? The size and message type are already in the
>> header, so why do we need to pass it?
>
>
> But we need to know the response size in send_msg_iservice() since
> we pass it to ReadFile(). So far we assumed that response is always 
> ask_message_t
> and we could do sizeof(*ack). With new response type this assumption doesn't 
> hold so
> as Gert suggested I added another version which accepts arbitrary response 
> type and size.

My point is that, this is not in the spirit of the rest of iservice code. See
HandleMessage in interactive.c where the data is and then interpreted
using the header type and size.

For what max-size to pass to ReadFile, we know it from the size in the header
element of the struct.

Selva


>
> bool
> send_msg_iservice(HANDLE pipe, const void *data, size_t size,
>   ack_message_t *ack, const char *context)
> {
> return send_msg_iservice_ex(pipe, data, size, ack, sizeof(*ack), context);
> }
>
> bool
> send_msg_iservice_ex(HANDLE pipe, const void *data, size_t size,
>  void *response, size_t response_size, const char 
> *context)
> {
>
> Will send V2 tomorrow with this and CreateFileW changes.
>
>
> --
> -Lev


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v4 6/7] Sent indication that a session is expired to clients

[David Sommerseth]

On 13/06/2019 15:48, Arne Schwabe wrote:
> From: Arne Schwabe 
> 
> This allows OpenVPN 3 core to fall back to the original authentication
> method.
> 
> This commit changes man_def_auth_set_client_reason to
> auth_set_client_reason since it now used in more contexts.
> 
> Also remove a FIXME about client_reason not being freed, as it is freed
> in tls_multi_free with auth_set_client_reason(multi, NULL);
> 
> Patch V4: Rebase on master
> ---
>  src/openvpn/auth_token.c |  3 +++
>  src/openvpn/ssl.c|  6 ++
>  src/openvpn/ssl_common.h | 10 +-
>  src/openvpn/ssl_verify.c |  8 
>  src/openvpn/ssl_verify.h | 15 ++-
>  5 files changed, 24 insertions(+), 18 deletions(-)
> 

There's a typo in the subject line (Sent -> Send), otherwise this is fine and
works as expected.

Acked-by: David Sommerseth 


-- 
kind regards,

David Sommerseth
OpenVPN Inc



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] openvpnserv: enable interactive service to open tun

[Selva Nair]

Hi

On Tue, Jun 25, 2019 at 4:34 PM Gert Doering  wrote:
>
> Hi,
>
> On Tue, Jun 25, 2019 at 03:57:18PM -0400, Selva Nair wrote:
> > The way interactive service structures are coded should not require
> > this at all, does it? The size and message type are already in the
> > header, so why do we need to pass it? The result here is a new kind of
> > ack message with a different size and type and that could be checked
> > by accessing the header element. Unless I'm missing something.
>
> You could, indeed, do this inside send_msg_iservice() by looking
> at "what request did we sent?  so what should be coming back?" but
> I'm not sure I find this safe enough (caller allocates memory, and
> maybe we shouldn't rely on "it being big enough" unless explicitly
> told).  It would work, yes, but leaves me with a bit uneasy feeling.

I was thinking of dereferening the response pointer as a union and check the
header size which the caller is supposed to have set. So change ack * to
void * in send_msg_iservice()  as in the patch, and treat it as
ack or and extended ack depending on the specified size. Further, when
reading from the pipe one should also check the size of data received
matches what is expected.

It may be also useful to make all reply messages made up of an ack message
plus optional additional data -- ie., all start with a header and an error code.

Selva




>
> gert
> --
> "If was one thing all people took for granted, was conviction that if you
>  feed honest figures into a computer, honest figures come out. Never doubted
>  it myself till I met a computer with a sense of humor."
>  Robert A. Heinlein, The Moon is a Harsh Mistress
>
> Gert Doering - Munich, Germany g...@greenie.muc.de


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] how to migrate users to "no compression" config

[Илья Шипицин]

Hello,

for example, let us imagine we provisioned a lot of users with config files
containing "comp-lzo"
and we want to migrate them to server without compression.

I see two options

1) set up new server (actually, new udp/tcp ports on the same server) and
send new config to users

2) use push "compress empty" (if there's such an option) ?

Thanks,
Ilya Shipitsin
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] openvpnserv: enable interactive service to open tun

[Lev Stipakov]

Hi,


> The way interactive service structures are coded should not require
> this at all, does it? The size and message type are already in the
> header, so why do we need to pass it?


But we need to know the response size in send_msg_iservice() since
we pass it to ReadFile(). So far we assumed that response is always
ask_message_t
and we could do sizeof(*ack). With new response type this assumption
doesn't hold so
as Gert suggested I added another version which accepts arbitrary response
type and size.

bool
send_msg_iservice(HANDLE pipe, const void *data, size_t size,
  ack_message_t *ack, const char *context)
{
return send_msg_iservice_ex(pipe, data, size, ack, sizeof(*ack),
context);
}

bool
send_msg_iservice_ex(HANDLE pipe, const void *data, size_t size,
 void *response, size_t response_size, const char
*context)
{

Will send V2 tomorrow with this and CreateFileW changes.


-- 
-Lev
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] openvpnserv: enable interactive service to open tun

[Gert Doering]

Hi,

On Tue, Jun 25, 2019 at 03:57:18PM -0400, Selva Nair wrote:
> The way interactive service structures are coded should not require
> this at all, does it? The size and message type are already in the
> header, so why do we need to pass it? The result here is a new kind of
> ack message with a different size and type and that could be checked
> by accessing the header element. Unless I'm missing something.

You could, indeed, do this inside send_msg_iservice() by looking
at "what request did we sent?  so what should be coming back?" but
I'm not sure I find this safe enough (caller allocates memory, and
maybe we shouldn't rely on "it being big enough" unless explicitly 
told).  It would work, yes, but leaves me with a bit uneasy feeling.

gert
-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v4 7/7] Implement unit tests for auth-gen-token

[David Sommerseth]

On 13/06/2019 15:48, Arne Schwabe wrote:
> From: Arne Schwabe 
> 
> Patch V2: adapt unit tests to other V2 patches
> Patch V4: Resolve rebase conflicts
> ---
>  tests/unit_tests/openvpn/Makefile.am   |  20 +-
>  tests/unit_tests/openvpn/test_auth_token.c | 375 +
>  2 files changed, 394 insertions(+), 1 deletion(-)
>  create mode 100644 tests/unit_tests/openvpn/test_auth_token.c
> 

Looks reasonable.  But needs to be rebased on top of latest git master; I 
suspect 
this is due to sitnl patches arriving in master.  I had to "force" this into
submission, so I might have missed a few things - as I do get these errors:

-
.../openvpn/tests/unit_tests/openvpn/../../../src/openvpn/auth_token.c:201: 
undefined reference to `openvpn_base64_decode'
.../openvpn/tests/unit_tests/openvpn/../../../src/openvpn/auth_token.c:205: 
undefined reference to `openvpn_base64_decode'
.../openvpn/tests/unit_tests/openvpn/../../../src/openvpn/auth_token.c:254: 
undefined reference to `openvpn_base64_encode'
auth_token_testdriver-test_auth_token.o: In function `verify_auth_token':
.../OpenVPN/openvpn/tests/unit_tests/openvpn/../../../src/openvpn/auth_token.c:299:
 undefined reference to `openvpn_base64_decode'
-

In addition comes the warnings I've already reported in a prior patch.


-- 
kind regards,

David Sommerseth
OpenVPN Inc



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v4 4/7] Rewrite auth-token-gen to be based on HMAC based tokens

[David Sommerseth]

On 13/06/2019 15:48, Arne Schwabe wrote:
> The previous auth-token implementation had a serious problem, especially when
> paired with an unpatched OpenVPN client that keeps trying the auth-token
> (commit e61b401a).
> 
> The auth-token-gen implementation forgot the auth-token on reconnect, this
> lead to reconnect with auth-token never working.
> 
> This new implementation implements the auth-token in a stateles variant. By
> using HMAC to sign the auth-token the server can verify if a token has been
> authenticated and by checking the embedded timestamp in the token it can
> also verify that the auth-token is still valid.
> 
> Using the new config directive auth-gen-token-secret instead of
> extending auth-gen-token (--auth-gen-token [lifetime] [secret-key]) was
> chosen to allow inlinening the secret key.
> 
> Patch V2: cleaned up code, use refactored read_pem_key_file function
> Patch V3: clarify some design decision in the commit message
> Patch V4: Use ephermal_generate_key
> ---
>  doc/openvpn.8|  25 
>  src/openvpn/Makefile.am  |   1 +
>  src/openvpn/auth_token.c | 272 +++
>  src/openvpn/auth_token.h | 116 +
>  src/openvpn/init.c   |  30 -
>  src/openvpn/openvpn.h|   1 +
>  src/openvpn/options.c|  22 +++-
>  src/openvpn/options.h|   4 +
>  src/openvpn/push.c   |  70 --
>  src/openvpn/push.h   |   8 ++
>  src/openvpn/ssl.c|   7 +-
>  src/openvpn/ssl_common.h |  36 --
>  src/openvpn/ssl_verify.c | 182 --
>  13 files changed, 639 insertions(+), 135 deletions(-)
>  create mode 100644 src/openvpn/auth_token.c
>  create mode 100644 src/openvpn/auth_token.h
> 

Not as heavy tested as in last round, but changes has been applied as 
indicated.  Thank you!  Unfortunately, some compiler warnings are appearing:


auth_token.c: In function ‘generate_auth_token’:
auth_token.c:115:9: warning: dereferencing type-punned pointer will break 
strict-aliasing rules [-Wstrict-aliasing]
 initial_timestamp = *((uint64_t *)(old_tstamp_decode));
 ^
auth_token.c: In function ‘verify_auth_token’:
auth_token.c:233:9: warning: format ‘%lld’ expects argument of type ‘long long 
int’, but argument 3 has type ‘uint64_t’ [-Wformat=]
 msg(M_WARN, "Initial timestamp (%lld) in token from client earlier 
than "
 ^
auth_token.c:233:9: warning: format ‘%lld’ expects argument of type ‘long long 
int’, but argument 4 has type ‘uint64_t’ [-Wformat=]



The first one (dereferencing type-punned pointer) seems to only show up with
gcc-4.8.5 and 6.3.1.  Below are warnings reported by both gcc-7.3.1 and 8.3.1,
which are also a bit more verbose, with a few additional twists.


auth_token.c: In function ‘verify_auth_token’:
auth_token.c:233:21: warning: format ‘%lld’ expects argument of type ‘long long 
int’, but argument 3 has type ‘uint64_t’ {aka ‘long unsigned int’} [-Wformat=]
 msg(M_WARN, "Initial timestamp (%lld) in token from client earlier 
than "
 
^
auth_token.c:235:13:
 timestamp_initial, timestamp);
 ~
error.h:151:67: note: in definition of macro ‘msg’
 #define msg(flags, ...) do { if (msg_test(flags)) {x_msg((flags), 
__VA_ARGS__);} EXIT_FATAL(flags); } while (false)
   ^~~
auth_token.c:233:44: note: format string is defined here
 msg(M_WARN, "Initial timestamp (%lld) in token from client earlier 
than "
 ~~~^
 %ld
In file included from buffer.h:28,
 from auth_token.c:10:
auth_token.c:233:21: warning: format ‘%lld’ expects argument of type ‘long long 
int’, but argument 4 has type ‘uint64_t’ {aka ‘long unsigned int’} [-Wformat=]
 msg(M_WARN, "Initial timestamp (%lld) in token from client earlier 
than "
 
^
auth_token.c:235:32:
 timestamp_initial, timestamp);
~
error.h:151:67: note: in definition of macro ‘msg’
 #define msg(flags, ...) do { if (msg_test(flags)) {x_msg((flags), 
__VA_ARGS__);} EXIT_FATAL(flags); } while (false)
   ^~~
auth_token.c:234:36: note: format string is defined here
 "current timestamp (%lld). Broken/unsynchronised clock?",
 ~~~^
 %ld


Other than these issues, things looks good and would normally get an ACK.


-- 
kind 

Re: [Openvpn-devel] [PATCH v4 2/7] Implement --genkey type keyfile syntax and migrate tls-crypt-v2

[David Sommerseth]

On 13/06/2019 15:48, Arne Schwabe wrote:
> This unifies our key generation and also migrates the generation
> of the tls-crypt-v2 keys. Since tls-crypt-v2 is not included in any
> released version, we remove the the old syntax without compatibility.
> 
> PATCH V4: Introduce warning/error when using --secret with --genkey
>   Update non code usages to use new --genkey syntax
> ---
>  INSTALL|  2 +-
>  doc/openvpn.8  | 81 ++
>  sample/sample-config-files/server.conf |  2 +-
>  sample/sample-keys/gen-sample-keys.sh  |  2 +-
>  sample/sample-windows/sample.ovpn  |  2 +-
>  src/openvpn/crypto.c   |  2 +-
>  src/openvpn/init.c | 71 ++
>  src/openvpn/options.c  | 67 +
>  src/openvpn/options.h  | 11 +++-
>  tests/t_lpback.sh  |  8 +--
>  10 files changed, 151 insertions(+), 97 deletions(-)
> 

Acked-by: David Sommerseth 

Looks good now, 'make check' passes and behaviour is more consistent.



-- 
kind regards,

David Sommerseth
OpenVPN Inc



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v4 3/7] Add generate_ephemeral_key that allows a random ephermal key

[David Sommerseth]

On 13/06/2019 15:48, Arne Schwabe wrote:
> From: Arne Schwabe 
> 
> This is useful for features that can use enither a persistent
> or an ephemeral key.
> 
> Patch V2: Move the functionality of generating a random key into a
>   separate function that acts as wrapper for pem_read_key_file
> Patch V4: Move wrapper functionality to caller and leave only generate
>   epehermal key functionality in the new function
> ---
>  src/openvpn/crypto.c | 14 ++
>  src/openvpn/crypto.h | 12 +++-
>  2 files changed, 25 insertions(+), 1 deletion(-)

This looks good ... but one nit-pick, but can be fixed during commit:

> diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
> index c5947483..72244997 100644
> --- a/src/openvpn/crypto.h
> +++ b/src/openvpn/crypto.h
> @@ -428,7 +428,17 @@ unsigned int crypto_max_overhead(void);
[]> +/**
> + * Generate ephermal key material  into the key structure or if
 ^
This comment don't need those two last words, but that can be fixed during
commit time.

Acked-By: David Sommerseth 


-- 
kind regards,

David Sommerseth
OpenVPN Inc



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] openvpnserv: enable interactive service to open tun

[Selva Nair]

Hi

On Tue, Jun 25, 2019 at 3:49 PM Gert Doering  wrote:
>
> Hi,
>
> On Tue, Jun 25, 2019 at 10:34:01PM +0300, Lev Stipakov wrote:
> >  ack_message_t ack;
> >  struct gc_arena gc = gc_new();
> >
> > -if (!send_msg_iservice(pipe, rt, size, , "ROUTE"))
> > +if (!send_msg_iservice(pipe, rt, size, , sizeof(ack), "ROUTE"))
>
> I do not like this.  Please find another way to send the request message
> "with length" than to add an extra parameter to every single caller of
> send_msg_iservice().

Gert beat me to it :) Anyway, me too!

The way interactive service structures are coded should not require
this at all, does it? The size and message type are already in the
header, so why do we need to pass it? The result here is a new kind of
ack message with a different size and type and that could be checked
by accessing the header element. Unless I'm missing something.

>
> +HANDLE local_handle = CreateFileA(open_tun->device_path, GENERIC_READ | 
> GENERIC_WRITE, 0, 0,
> +  OPEN_EXISTING, FILE_ATTRIBUTE_SYSTEM | 
> FILE_FLAG_OVERLAPPED, 0);
> +
> +if (local_handle == INVALID_HANDLE_VALUE)
> +{
> +WCHAR *device_path_wchar = NULL;
> +int size = sizeof(open_tun->device_path);
> +err = GetLastError();
> +
> +device_path_wchar = malloc(size * sizeof(WCHAR));
> +if (device_path_wchar)
> +{
> +MultiByteToWideChar(CP_UTF8, 0, open_tun->device_path, size, 
> device_path_wchar, size);
> +device_path_wchar[size - 1] = 0;
> +}
> +MsgToEventLog(M_SYSERR, TEXT("Error opening tun device (%s)"), 
> device_path_wchar);
> +free(device_path_wchar);
> +return err;
> +}
>

Also this one -- I think we should just use the wide version of
CreateFile  -- all strings in OpenVPN.exe are supposed to be in utf8,
so convert to widechar and call CreateFileW.

Selva


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] openvpnserv: enable interactive service to open tun

[Gert Doering]

Hi,

On Tue, Jun 25, 2019 at 10:34:01PM +0300, Lev Stipakov wrote:
>  ack_message_t ack;
>  struct gc_arena gc = gc_new();
>  
> -if (!send_msg_iservice(pipe, rt, size, , "ROUTE"))
> +if (!send_msg_iservice(pipe, rt, size, , sizeof(ack), "ROUTE"))

I do not like this.  Please find another way to send the request message
"with length" than to add an extra parameter to every single caller of 
send_msg_iservice().

Possibly introduce a wrapper for the "standard" case which calls a new
function send_msg_iservice_ex() that takes a length field for the return 
data type.  And the open wintun / return handle would then use _ex().

gert

-- 
"If was one thing all people took for granted, was conviction that if you 
 feed honest figures into a computer, honest figures come out. Never doubted 
 it myself till I met a computer with a sense of humor."
 Robert A. Heinlein, The Moon is a Harsh Mistress

Gert Doering - Munich, Germany g...@greenie.muc.de


signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] openvpnserv: enable interactive service to open tun

[Lev Stipakov]

From: Lev Stipakov 

This patch enables interactive service to open tun device.
This is mostly needed by Wintun, which could be opened
only by privileged process.

When interactive service is used, instead of calling
CreateFile() directly by openvpn process we pass tun device path
into service process. There we open device, duplicate handle
and pass it back to openvpn process.

Signed-off-by: Lev Stipakov 
---
 include/openvpn-msg.h | 12 
 src/openvpn/route.c   |  2 +-
 src/openvpn/tun.c | 70 +++
 src/openvpn/win32.c   |  6 ++--
 src/openvpn/win32.h   |  6 ++--
 src/openvpnserv/interactive.c | 59 +++-
 6 files changed, 134 insertions(+), 21 deletions(-)

diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index 66177a2..273d9a6 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -39,6 +39,8 @@ typedef enum {
 msg_del_block_dns,
 msg_register_dns,
 msg_enable_dhcp,
+msg_open_tun_device,
+msg_open_tun_device_result,
 } message_type_t;
 
 typedef struct {
@@ -117,4 +119,14 @@ typedef struct {
 interface_t iface;
 } enable_dhcp_message_t;
 
+typedef struct {
+message_header_t header;
+char device_path[512];
+} open_tun_device_message_t;
+
+typedef struct {
+message_header_t header;
+HANDLE handle;
+int error_number;
+} open_tun_device_result_message_t;
 #endif /* ifndef OPENVPN_MSG_H_ */
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 4cdc4a9..27fa18c 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -2992,7 +2992,7 @@ do_route_service(const bool add, const route_message_t 
*rt, const size_t size, H
 ack_message_t ack;
 struct gc_arena gc = gc_new();
 
-if (!send_msg_iservice(pipe, rt, size, , "ROUTE"))
+if (!send_msg_iservice(pipe, rt, size, , sizeof(ack), "ROUTE"))
 {
 goto out;
 }
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 8f8f7c6..2d7bd0d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -115,7 +115,7 @@ do_address_service(const bool add, const short family, 
const struct tuntap *tt)
 addr.prefix_len = tt->netbits_ipv6;
 }
 
-if (!send_msg_iservice(pipe, , sizeof(addr), , "TUN"))
+if (!send_msg_iservice(pipe, , sizeof(addr), , sizeof(ack), 
"TUN"))
 {
 goto out;
 }
@@ -181,7 +181,7 @@ do_dns6_service(bool add, const struct tuntap *tt)
 msg(D_LOW, "%s IPv6 dns servers on '%s' (if_index = %d) using service",
 (add ? "Setting" : "Deleting"), dns.iface.name, dns.iface.index);
 
-if (!send_msg_iservice(pipe, , sizeof(dns), , "TUN"))
+if (!send_msg_iservice(pipe, , sizeof(dns), , sizeof(ack), "TUN"))
 {
 goto out;
 }
@@ -5227,7 +5227,7 @@ service_enable_dhcp(const struct tuntap *tt)
 .iface = { .index = tt->adapter_index, .name = "" }
 };
 
-if (!send_msg_iservice(pipe, , sizeof(dhcp), , "Enable_dhcp"))
+if (!send_msg_iservice(pipe, , sizeof(dhcp), , sizeof(ack), 
"Enable_dhcp"))
 {
 goto out;
 }
@@ -5248,6 +5248,43 @@ out:
 return ret;
 }
 
+static HANDLE
+service_open_tun_device(const HANDLE pipe, const char* device_path)
+{
+open_tun_device_result_message_t result_msg;
+struct gc_arena gc = gc_new();
+open_tun_device_message_t open_tun_device = {
+.header = {
+msg_open_tun_device,
+sizeof(open_tun_device_message_t),
+0
+}
+};
+result_msg.handle = INVALID_HANDLE_VALUE;
+
+strncpynt(open_tun_device.device_path, device_path, 
sizeof(open_tun_device.device_path));
+
+if (!send_msg_iservice(pipe, _tun_device, sizeof(open_tun_device),
+_msg, sizeof(result_msg), "Open_tun_device"))
+{
+goto out;
+}
+
+if (result_msg.error_number != NO_ERROR)
+{
+msg(D_TUNTAP_INFO, "TUN: opening tun handle using service failed: %s 
[status=%u device_path=%s]",
+strerror_win32(result_msg.error_number, ), 
result_msg.error_number, device_path);
+}
+else
+{
+msg(M_INFO, "Opened tun device %s using service", device_path);
+}
+
+out:
+gc_free();
+return result_msg.handle;
+}
+
 /*
  * Return a TAP name for netsh commands.
  */
@@ -5469,7 +5506,7 @@ register_dns_service(const struct tuntap *tt)
 
 message_header_t rdns = { msg_register_dns, sizeof(message_header_t), 0 };
 
-if (!send_msg_iservice(msg_channel, , sizeof(rdns), , 
"Register_dns"))
+if (!send_msg_iservice(msg_channel, , sizeof(rdns), , 
sizeof(ack), "Register_dns"))
 {
 gc_free();
 return;
@@ -5631,15 +5668,22 @@ open_tun(const char *dev, const char *dev_type, const 
char *dev_node, struct tun
  device_guid,
  TAP_WIN_SUFFIX);
 
-tt->hand = CreateFile(
-device_path,
-GENERIC_READ | 

Re: [Openvpn-devel] Summary of the community meeting (20th June 2019)

[Samuli Seppänen]

Hi,

I got the Static Driver Verifier to run with help from Stephen. The
correct (command-line) procedure is now documented here:



So it was bad usability after all. The tests are now running and
hopefully I can create the merged test result package from SDV and Code
Analysis today.

Meanwhile I'll make the HLK client work for its money by running the
other tests.

Samuli

Il 24/06/19 21:13, Samuli Seppänen ha scritto:
> Hi,
> 
> Il 24/06/19 14:33, Samuli Seppänen ha scritto:
>> Hi Simon,
>>
>> Thanks for the info again!
>>
>> Il 21/06/19 18:59, Simon Rozman ha scritto:
>>> (21:04:58) mattock: assuming Microsoft's systems are happy with the test 
>>> submission package, that is
>>> (21:05:12) mattock: they _should_ be, but we have not tested submitting 
>>> anything yes
>>>
>>> 1. Do the SDV and DVL to get tap901.DVL.xml.
>>
>> I'm working on this right now. Based on brief practical testing EWDK may
>> not be able to do this, even though it is apparently possible to produce
>> SDV logs from the command-line:>
>> https://docs.microsoft.com/en-us/windows-hardware/drivers/develop/creating-a-log-file-for-static-driver-verifier
>>
>> It _seems_ that EWDK might not have all the necessary pieces for
>> producing SDV log files. MS does briefly mention that EWDK is not
>> suitable for doing driver testing, whatever that means. So, to play it
> 
> Actually it is probably possible to use the EWDK for running Static
> Driver Verifier:
> 
> 
> 
> 
>> safe, I'm installing Visual Studio 2019 plus the "normal" Windows Driver
>> Kit on my tapbuilder VM.
> 
> So, I took a stab at producing the Static Driver Verifier logs in Visual
> Studio 2019 ("Extensions" -> "Static Driver Verifier"), but it just
> complains about
> 
> "Please select a driver project for sdv to verify"
> 
> I wonder if I just hit a usability problem, or if something is actually
> missing from tap-windows6?
> 
> Attempting to run SDV from the command-line (adapted from first link
> above) fails as well. The /clean part works fine, but running SDV fails:
> 
> PS> msbuild.exe src\tap-windows6.vcxproj /p:Configuration="Release"
> /p:Platform=x64 /target:sdv /p:inputs="/clean"
> 
> --- snip ---
> 
> PS> msbuild.exe src\tap-windows6.vcxproj /p:Configuration="Release"
> /p:Platform=x64 /target:sdv /p:inputs="/check:default.sdv"
> 
>   [INFO] Validating XML against schema: C:\Program Files (x86)\Windows
> Kits\10\TOOLS\SDV\smv\bin\Config.xsd
>   [INFO] Running local scheduler with 2 threads
>   [FATAL ERROR] Unrecoverable error in NormalBuild stage.
> C:\Program Files (x86)\Windows
> Kits\10\build\windowsdriver.Sdv.targets(136,9): error MSB3073: The
> command "staticdv /ch
> eck:default.sdv" exited with code -1.
> [C:\Users\samuli\opt\tap-windows6\src\tap-windows6.vcxproj]
> Done Building Project
> "C:\Users\samuli\opt\tap-windows6\src\tap-windows6.vcxproj" (sdv
> target(s)) -- FAILED.
> 
> 
> Build FAILED.
> 
> "C:\Users\samuli\opt\tap-windows6\src\tap-windows6.vcxproj" (sdv target)
> (1) ->
> (sdv target) ->
>   C:\Program Files (x86)\Windows
> Kits\10\build\windowsdriver.Sdv.targets(136,9): error MSB3073: The
> command "staticdv /
> check:default.sdv" exited with code -1.
> [C:\Users\samuli\opt\tap-windows6\src\tap-windows6.vcxproj]
> 
> 0 Warning(s)
> 1 Error(s)
> 
> Time Elapsed 00:00:02.65
> 
> ---
> 
> Am I missing something [that is obvious to a Windows developer]?
> 
> Samuli
> 
> [*] Adapting the instructions of the first link, above
> 
>>
>>> 2. Compile the driver and EV sign it. Save PDBs too.
>>
>> I assume this is not really necessary if I'm using WDKTest certificates
>> and have enabled test-signing?
>>
>>> 3. Deploy the driver on test computers (including tap901.DVL.xml, 
>>> remember?).
>>> 4. Do the WHLK.
>>> 5. When creating submission package, add the driver binaries and PDBs (on 
>>> HLK Studio submission page).
>>> 6. Submit the driver to Microsoft WHQL.
>>> 7. Miscrosoft should return you a WHQL signed driver in about 10 minutes.
>>>
>>> (21:07:09) mattock: worst case scenario is that I have to reinstall the HLK 
>>> client as Windows Server 2019 core _if_ Microsoft is not happy with the 
>>> "Operate in Server Core" having been run on a virtual machine, or on some 
>>> old i5 laptop which does not have the required 4 physical processor cores
>>>
>>> Microsoft is fine with that test being run on a virtual Windows Server 2019 
>>> Core in Wintun case. And this test is pretty straight forward - just checks 
>>> that driver loads and adapter responds, it doesn't need to be connected and 
>>> have traffic. Use devcon to make a single TAP adapter on the Server Core. 
>>> No need to have a running OpenVPN connection for this test to pass.