[Openvpn-devel] [PATCH 3/3] Fix too-deep indentation.

[Wayne Davison]

Fix the indentation on the code block that got moved out of an unneeded
"if".
---
 src/openvpn/misc.c | 116 ++---
 1 file changed, 58 insertions(+), 58 deletions(-)

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index e0aa5f9..517a2eb 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -1142,69 +1142,69 @@ get_user_pass_cr (struct user_pass *up,
* Get username/password from standard input?
*/
 #ifdef ENABLE_CLIENT_CR
- if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE))
-   {
- struct auth_challenge_info *ac = get_auth_challenge 
(auth_challenge, );
- if (ac)
-   {
- char *response = (char *) gc_malloc (USER_PASS_LEN, false, 
);
- struct buffer packed_resp;
-
- buf_set_write (_resp, (uint8_t*)up->password, 
USER_PASS_LEN);
- msg (M_INFO|M_NOPREFIX, "CHALLENGE: %s", ac->challenge_text);
- if (!get_console_input ("Response:", 
BOOL_CAST(ac->flags_ECHO), response, USER_PASS_LEN))
-   msg (M_FATAL, "ERROR: could not read challenge response 
from stdin");
- strncpynt (up->username, ac->user, USER_PASS_LEN);
- buf_printf (_resp, "CRV1::%s::%s", ac->state_id, 
response);
-   }
- else
-   {
- msg (M_FATAL, "ERROR: received malformed challenge request 
from server");
-   }
-   }
- else
+  if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE))
+{
+  struct auth_challenge_info *ac = get_auth_challenge (auth_challenge, 
);
+  if (ac)
+{
+  char *response = (char *) gc_malloc (USER_PASS_LEN, false, );
+  struct buffer packed_resp;
+
+  buf_set_write (_resp, (uint8_t*)up->password, 
USER_PASS_LEN);
+  msg (M_INFO|M_NOPREFIX, "CHALLENGE: %s", ac->challenge_text);
+  if (!get_console_input ("Response:", 
BOOL_CAST(ac->flags_ECHO), response, USER_PASS_LEN))
+msg (M_FATAL, "ERROR: could not read challenge response from 
stdin");
+  strncpynt (up->username, ac->user, USER_PASS_LEN);
+  buf_printf (_resp, "CRV1::%s::%s", ac->state_id, 
response);
+}
+  else
+{
+  msg (M_FATAL, "ERROR: received malformed challenge request from 
server");
+}
+}
+  else
 #endif
-   {
- if (username_from_stdin && !(flags & GET_USER_PASS_PASSWORD_ONLY))
-   {
- struct buffer user_prompt = alloc_buf_gc (128, );
- buf_printf (_prompt, "Enter %s Username:", prefix);
- if (!get_console_input (BSTR (_prompt), true, 
up->username, USER_PASS_LEN))
-   msg (M_FATAL, "ERROR: could not read %s username from 
stdin", prefix);
- if (strlen (up->username) == 0)
-   msg (M_FATAL, "ERROR: %s username is empty", prefix);
-   }
+{
+  if (username_from_stdin && !(flags & GET_USER_PASS_PASSWORD_ONLY))
+{
+  struct buffer user_prompt = alloc_buf_gc (128, );
+  buf_printf (_prompt, "Enter %s Username:", prefix);
+  if (!get_console_input (BSTR (_prompt), true, up->username, 
USER_PASS_LEN))
+msg (M_FATAL, "ERROR: could not read %s username from stdin", 
prefix);
+  if (strlen (up->username) == 0)
+msg (M_FATAL, "ERROR: %s username is empty", prefix);
+}

-  if (password_from_stdin)
-{
- struct buffer pass_prompt = alloc_buf_gc (128, );
- buf_printf (_prompt, "Enter %s Password:", prefix);
- if (!get_console_input (BSTR (_prompt), false, 
up->password, USER_PASS_LEN))
-msg (M_FATAL, "ERROR: could not not read %s password from 
stdin", prefix);
-}
+  if (password_from_stdin)
+{
+  struct buffer pass_prompt = alloc_buf_gc (128, );
+  buf_printf (_prompt, "Enter %s Password:", prefix);
+  if (!get_console_input (BSTR (_prompt), false, 
up->password, USER_PASS_LEN))
+msg (M_FATAL, "ERROR: could not not read %s password from 
stdin", prefix);
+}

 #ifdef ENABLE_CLIENT_CR
- if (auth_challenge && (flags & GET_USER_PASS_STATIC_CHALLENGE))
-   {
- char *response = (char *) gc_malloc (USER_PASS_LEN, false, 
);
- struct buffer packed_resp;
- char *pw64=NULL, *resp64=NULL;
-
- msg (M_INFO|M_NOPREFIX, "CHALLENGE: %s", auth_challenge);
- if (!get_console_input ("Response:", BOOL_CAST(flags & 
GET_USER_PASS_STATIC_CHALLENGE_ECHO), response, USER_PASS_LEN))
-  

[Openvpn-devel] [PATCH 2/3] Move 2 prompt buffers into deeper blocks.

[Wayne Davison]

Two buffers used for username/password prompting can be moved into a
deeper block so that they don't get set if they are not going to be
used.
---
 src/openvpn/misc.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index a9259f0..e0aa5f9 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -1165,22 +1165,23 @@ get_user_pass_cr (struct user_pass *up,
  else
 #endif
{
- struct buffer user_prompt = alloc_buf_gc (128, );
- struct buffer pass_prompt = alloc_buf_gc (128, );
-
- buf_printf (_prompt, "Enter %s Username:", prefix);
- buf_printf (_prompt, "Enter %s Password:", prefix);
-
  if (username_from_stdin && !(flags & GET_USER_PASS_PASSWORD_ONLY))
{
+ struct buffer user_prompt = alloc_buf_gc (128, );
+ buf_printf (_prompt, "Enter %s Username:", prefix);
  if (!get_console_input (BSTR (_prompt), true, 
up->username, USER_PASS_LEN))
msg (M_FATAL, "ERROR: could not read %s username from 
stdin", prefix);
  if (strlen (up->username) == 0)
msg (M_FATAL, "ERROR: %s username is empty", prefix);
}

- if (password_from_stdin && !get_console_input (BSTR 
(_prompt), false, up->password, USER_PASS_LEN))
-   msg (M_FATAL, "ERROR: could not not read %s password from 
stdin", prefix);
+  if (password_from_stdin)
+{
+ struct buffer pass_prompt = alloc_buf_gc (128, );
+ buf_printf (_prompt, "Enter %s Password:", prefix);
+ if (!get_console_input (BSTR (_prompt), false, 
up->password, USER_PASS_LEN))
+msg (M_FATAL, "ERROR: could not not read %s password from 
stdin", prefix);
+}

 #ifdef ENABLE_CLIENT_CR
  if (auth_challenge && (flags & GET_USER_PASS_STATIC_CHALLENGE))
-- 
1.9.1

[Openvpn-devel] [PATCH 1/3] Fix CR prompting when user & pass are read from a file.

[Wayne Davison]

The code that reads the challenge response (both dynamic & static) will
not prompt the user if the username and password information was read
from a file.  In the latest code this can be fixed by simply removing
the "if (username_from_stdin || password_from_stdin)" condition because
all the deeper code is already conditionalized adequately to only prompt
for the missing information.

NOTE: indentation left too deep so that changes are clearly seen.
---
 src/openvpn/misc.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
index 05ed073..a9259f0 100644
--- a/src/openvpn/misc.c
+++ b/src/openvpn/misc.c
@@ -1141,8 +1141,6 @@ get_user_pass_cr (struct user_pass *up,
   /*
* Get username/password from standard input?
*/
-  if (username_from_stdin || password_from_stdin)
-   {
 #ifdef ENABLE_CLIENT_CR
  if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE))
{
@@ -1206,7 +1204,6 @@ get_user_pass_cr (struct user_pass *up,
}
 #endif
}
-   }

   string_mod (up->username, CC_PRINT, CC_CRLF, 0);
   string_mod (up->password, CC_PRINT, CC_CRLF, 0);
-- 
1.9.1

Re: [Openvpn-devel] [PATCH] Warn user if their certificate has expired

[Steffan Karger]

On Tue, Dec 15, 2015 at 10:41 PM, Jan Just Keijser  wrote:
> On 15/12/15 08:53, Gert Doering wrote:
>> On Tue, Dec 15, 2015 at 01:12:49AM +0100, David Sommerseth wrote:
>>> Just tried to build openvpn on one of my laptops (Scientific Linux 7.1,
>>> openssl-1.0.1e-42.el7).  And it explodes when reaching the
>>> SSL_CTX_get0_certificate(), it seems that support arrived in OpenSSL 1.0.2?
>>> Could that be right?  Haven't had brainpower yet to dig deeper.
>> Yeah, we noticed.  Sorry for that.  Never seen so much red on the buildbot
>> list before... :-)
>>
>> Committed and pushed the "not before 1.0.2" workaround.
>>
>>
> h I think this feature is actually worth adding to systems based on
> OpenSSL 1.0.1 as well (think RHEL 7 etc). The problem is, the current
> fix is very 1.0.2 specific and there seems to be no easy work-around.
> Another approach is to check the certificate expiry time when the cert
> is loaded, but - as Steffan pointed out - this would mean that multiple
> places need a function call to check this:
> - when loading an x509 file
> - when loading a pkcs12 file
> - when loading an inline blob
> - when loading something from pkcs11
> - when loading something via cryptostore
> - when loading something using the management interface
>
> the problem lies in the 'reachability' of the actual X509 certificate
> structure in OpenSSL
> - the SSL_CTX context contains the X509 cert, but you cannot get at it
> until 1.0.2
> - at connection time the SSL struct also contains it, but this is too
> late in the process.
>
> I'm inclined to alter the patch to check it at all items listed above -
> what do you guys think?
> Having to wait for openssl 1.0.2 (which even my fedora 22 box does not
> yet have) seems a bit like "so close, yet so far ..."

Just use mbedtls ;-)

OpenSSL 1.0.2 has been released almost a year ago, so upcoming distro
releases will probably contain 1.0.2+ (e.g. Ubuntu 15.10 already has
it, 16.04 LTS will have it too).  Should not take too long, right?

As you've probably noticed in the other thread, I don't particularly
like the idea of adding that extra code.  But I won't actively oppose
such a patch either.

-Steffan

Re: [Openvpn-devel] [PATCH] Updates to Changes.rst

[Selva Nair]

Hi,


> > My original comment about this to an early version of valdikSS' patch
> > was on a different level -- if the user want DNS blocked, failure to
> block
> > should be FATAL. But not respecting ignore-unknown-option only on
> > some platforms doesn't look right.
>
> Well, it is not an "unknown option" on XP...  and the code doesn't really
> lend itself to "just claim it's an unknown option if it cannot be enabled"
> without being truly ugly...
>

Well,
else if (streq (p[0], "block-outside-dns") && !p[1] && win_wfp_init_funcs())
{..
}
will do it, but then one can't print a helpful error for the XP folks. The
current
"die with a nice mssage" is probbaly better ..

Sorry, I know I'm supposed to stop worrying about it :)

Selva

Re: [Openvpn-devel] [PATCH] Warn user if their certificate has expired

[Jan Just Keijser]

Hi,

On 15/12/15 08:53, Gert Doering wrote:

Hi,

On Tue, Dec 15, 2015 at 01:12:49AM +0100, David Sommerseth wrote:

Just tried to build openvpn on one of my laptops (Scientific Linux 7.1,
openssl-1.0.1e-42.el7).  And it explodes when reaching the
SSL_CTX_get0_certificate(), it seems that support arrived in OpenSSL 1.0.2?
Could that be right?  Haven't had brainpower yet to dig deeper.

Yeah, we noticed.  Sorry for that.  Never seen so much red on the buildbot
list before... :-)

Committed and pushed the "not before 1.0.2" workaround.


h I think this feature is actually worth adding to systems based on 
OpenSSL 1.0.1 as well (think RHEL 7 etc). The problem is, the current 
fix is very 1.0.2 specific and there seems to be no easy work-around.
Another approach is to check the certificate expiry time when the cert 
is loaded, but - as Steffan pointed out - this would mean that multiple 
places need a function call to check this:

- when loading an x509 file
- when loading a pkcs12 file
- when loading an inline blob
- when loading something from pkcs11
- when loading something via cryptostore
- when loading something using the management interface

the problem lies in the 'reachability' of the actual X509 certificate 
structure in OpenSSL
- the SSL_CTX context contains the X509 cert, but you cannot get at it 
until 1.0.2
- at connection time the SSL struct also contains it, but this is too 
late in the process.


I'm inclined to alter the patch to check it at all items listed above - 
what do you guys think?
Having to wait for openssl 1.0.2 (which even my fedora 22 box does not 
yet have) seems a bit like "so close, yet so far ..."


JJK

Re: [Openvpn-devel] [PATCH] Updates to Changes.rst

[Gert Doering]

Hi,

On Tue, Dec 15, 2015 at 04:12:25PM -0500, Selva Nair wrote:
> > Make that "setenv opt block-outside-dns".  ignore-unknown-option will
> > *not* help here, as it is not "unknown", but just "not working".
> 
> You are right, ignore-unknown .. would have worked with valdikSS patch,
> but not any more as its handled right away at msglevel_fc. Which is a
> bit disconcerting as on non-windows platforms ignore-unknown-option will
> make it non-FATAL but not on XP.

Right.  So, just use "setenv opt" always here, and don't worry any longer :)

> My original comment about this to an early version of valdikSS' patch
> was on a different level -- if the user want DNS blocked, failure to block
> should be FATAL. But not respecting ignore-unknown-option only on
> some platforms doesn't look right.

Well, it is not an "unknown option" on XP...  and the code doesn't really
lend itself to "just claim it's an unknown option if it cannot be enabled"
without being truly ugly...

(setenv opt wouldn't make it non-fatal if the functionality is *there* 
but cannot be enabled for whatever reason - so on Vista+, you still have
the "failure to block is FATAL")

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] Updates to Changes.rst

[Selva Nair]

Hi,

On Tue, Dec 15, 2015 at 3:54 PM, Gert Doering  wrote:

> Hi,
>
> On Tue, Dec 15, 2015 at 12:25:23PM -0500, Selva Nair wrote:
>
> > 2. release/2.3 (and upcoming 2.3.9)
> > - May be built for target = winxp
> >   (this is the default target in openvpn-build if release/2.3 source is
> > used)
> > - Support for --block-outside-dns determined automatically at run-time:
> >  when run on vista and above the option will be supported
> >  when run on winxp the option will be ignored or cause
> >  a fatal error depending on how it is specified:
> > if pushed from a server: just a warning in the logs
> > specified in the config: fatal error unless protected by
> > ignore-unknown-option or setenv-opt or forward-compatible...
>
> Make that "setenv opt block-outside-dns".  ignore-unknown-option will
> *not* help here, as it is not "unknown", but just "not working".
>

You are right, ignore-unknown .. would have worked with valdikSS patch,
but not any more as its handled right away at msglevel_fc. Which is a
bit disconcerting as on non-windows platforms ignore-unknown-option will
make it non-FATAL but not on XP.

My original comment about this to an early version of valdikSS' patch
was on a different level -- if the user want DNS blocked, failure to block
should be FATAL. But not respecting ignore-unknown-option only on
some platforms doesn't look right.

Selva

Re: [Openvpn-devel] [PATCH] Updates to Changes.rst

[Gert Doering]

Hi,

On Tue, Dec 15, 2015 at 12:25:23PM -0500, Selva Nair wrote:
> 1. git_master:
>  -  May be built with target = vista and will run correctly only on vista
> and above
> (this is the default target in openvpn-build if git-master source is
> used)
>  -  Will support --block-outside-dns when run on vista and above --
> undefined behaviour if attempted to run on winxp

I'm fairly sure the binary will not start at all.  It's calling symbols
that are not *there*, so the dll linker will just refuse to run it.

> 2. release/2.3 (and upcoming 2.3.9)
> - May be built for target = winxp
>   (this is the default target in openvpn-build if release/2.3 source is
> used)
> - Support for --block-outside-dns determined automatically at run-time:
>  when run on vista and above the option will be supported
>  when run on winxp the option will be ignored or cause
>  a fatal error depending on how it is specified:
> if pushed from a server: just a warning in the logs
> specified in the config: fatal error unless protected by
> ignore-unknown-option or setenv-opt or forward-compatible...

Make that "setenv opt block-outside-dns".  ignore-unknown-option will
*not* help here, as it is not "unknown", but just "not working".

>  3. Both should build correctly for 64-bit and 32-bit platforms

Yep.

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] Improve stdin prompting section, fixing CR prompting.

[Selva Nair]

Hi,

On Mon, Dec 14, 2015 at 4:10 PM, Selva Nair  wrote:
>
>> I took a quick look and it seems a simplified patch that addresses the
>> most critical-sounding issue (challenge/reponse not prompted for
>> from stdin) may be more useful.
>>
>
>
> That's exactly what that patch is.
>

Ok, now looking at the newer, version:

On Thu, Dec 10, 2015 at 11:57 AM, Wayne Davison  wrote:

> ---
>  src/openvpn/misc.c | 119
> +
>  1 file changed, 57 insertions(+), 62 deletions(-)
>

A commit message that explains the "why" and "how" will be helpful.


>
> diff --git a/src/openvpn/misc.c b/src/openvpn/misc.c
> index bc411bf..83e10f7 100644
> --- a/src/openvpn/misc.c
> +++ b/src/openvpn/misc.c
> @@ -1132,74 +1132,69 @@ get_user_pass_cr (struct user_pass *up,
>password_from_stdin = true;
>  }
>
> -  /*
> -   * Get username/password from standard input?
> -   */
> -  if (username_from_stdin || password_from_stdin)
> -   {
>  #ifdef ENABLE_CLIENT_CR
> - if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE))
> -   {
> - struct auth_challenge_info *ac = get_auth_challenge
> (auth_challenge, );
> - if (ac)
> -   {
> - char *response = (char *) gc_malloc (USER_PASS_LEN,
> false, );
> - struct buffer packed_resp;
> -
> - buf_set_write (_resp, (uint8_t*)up->password,
> USER_PASS_LEN);
> - msg (M_INFO|M_NOPREFIX, "CHALLENGE: %s",
> ac->challenge_text);
> - if (!get_console_input ("Response:",
> BOOL_CAST(ac->flags_ECHO), response, USER_PASS_LEN))
> -   msg (M_FATAL, "ERROR: could not read challenge
> response from stdin");
> - strncpynt (up->username, ac->user, USER_PASS_LEN);
> - buf_printf (_resp, "CRV1::%s::%s", ac->state_id,
> response);
> -   }
> - else
> -   {
> - msg (M_FATAL, "ERROR: received malformed challenge
> request from server");
> -   }
> -   }
> - else
> +  if (auth_challenge && (flags & GET_USER_PASS_DYNAMIC_CHALLENGE))
> +{
> +  struct auth_challenge_info *ac = get_auth_challenge
> (auth_challenge, );
> +  if (ac)
> +{
> +  char *response = (char *) gc_malloc (USER_PASS_LEN, false,
> );
> +  struct buffer packed_resp;
> +
> +  buf_set_write (_resp, (uint8_t*)up->password,
> USER_PASS_LEN);
> +  msg (M_INFO|M_NOPREFIX, "CHALLENGE: %s",
> ac->challenge_text);
> +  if (!get_console_input ("Response:",
> BOOL_CAST(ac->flags_ECHO), response, USER_PASS_LEN))
> +msg (M_FATAL, "ERROR: could not read challenge response
> from stdin");
> +  strncpynt (up->username, ac->user, USER_PASS_LEN);
> +  buf_printf (_resp, "CRV1::%s::%s", ac->state_id,
> response);
> +}
> +  else
> +{
> +  msg (M_FATAL, "ERROR: received malformed challenge request
> from server");
> +}
> +}
> +  else
>

I would either avoid excessive indentation changes or leave it for a
white-space-only patch (which will likely get ignored :). Same goes
for rest of the patch, which only moves around variable declarations,
or change whitespace.

This could be a small ~2 line patch -- easier to review and test.

Selva

Re: [Openvpn-devel] [PATCHv2] Implement the compression V2 data format for stub and lz4.

[Steffan Karger]

Hi Arne,

Some comments after a first review:

On Thu, Dec 10, 2015 at 1:39 PM, Arne Schwabe  wrote:
> V2: Fix an unintended change in the old lz4 decompress code.
>
> [..snip...]
>
> +static void
> +lz4_compress (struct buffer *buf, struct buffer work,
> + struct compress_context *compctx,
> + const struct frame* frame)
> +{
> +  if (buf->len <= 0)
> +return;
> +  bool compressed = do_lz4_compress(buf, , compctx, frame);

This looks like it will blow up on compilers that still not properly
support C99 (ie MSVC); declaration-after-statement.

> @@ -128,13 +141,73 @@ lz4_compress (struct buffer *buf, struct buffer work,
>}
>  }
>
> +
> +static void
> +lz4v2_compress (struct buffer *buf, struct buffer work,
> +   struct compress_context *compctx,
> +   const struct frame* frame)
> +{
> +  bool compressed;
> +  if (buf->len <= 0)
> +return;
> +
> +   compressed = do_lz4_compress(buf, , compctx, frame);
> +  if (buf->len <= 0)
> +return;
> +
> +  /* On Error just return */
> +  if (buf->len == 0)
> +return;

A space too many before compressed.  And the second check on buf->len
seems pointless.

> +  /* did compression save us anything? */
> +  {
> +if (compressed && work.len < buf->len)

We could also check against work.len + 2, since we will be adding two
bytes of we are going to use compression. Should save some more bytes.

> @@ -176,7 +233,57 @@ lz4_decompress (struct buffer *buf, struct buffer work,
>  }
>else
>  {
> +struct gc_arena gc = gc_new ();
>dmsg (D_COMP_ERRORS, "Bad LZ4 decompression header byte: %d", c);
> +  msg (M_INFO, "PKT_DUMP: %s", format_hex (BPTR (buf), buf->len, 0, 
> ));
> +  buf->len = 0;
> +}
> +}

Indenting looks off.  And don't forget to call gc_free().

> diff --git a/src/openvpn/compstub.c b/src/openvpn/compstub.c
> index 2ab7163..0f21a6f 100644
> --- a/src/openvpn/compstub.c
> +++ b/src/openvpn/compstub.c
> @@ -73,6 +73,8 @@ stub_compress (struct buffer *buf, struct buffer work,
>  }
>  }
>
> +
> +

Pure whitespace change?

-Steffan

Re: [Openvpn-devel] [PATCH] Use example.com to improve clarity of documentation

[Gert Doering]

Hi,

On Tue, Dec 15, 2015 at 08:54:23PM +0100, Steffan Karger wrote:
> Ooh, I like using alice and bob!  

+1

I'm fine with using example.com, but "host1" and "host2" just didn't
ring true (but I was too busy to spell this out).

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] Use example.com to improve clarity of documentation

[Selva Nair]

On Tue, Dec 15, 2015 at 2:54 PM, Steffan Karger  wrote:

> On Tue, Dec 15, 2015 at 8:46 PM, David Sommerseth
>  wrote:
> > On 30/11/15 04:03, Phillip Smith wrote:
> >> This patch uses the generic "host1.example.com" and "host2.example.com"
> to
> >> replace the current "may" and "june" hostname examples. Generic names
> chosen
> >> rather than other names like "server"/"client" or
> "head-office"/"remote-office"
> >> etc which may create other unintended or implicit meanings to the
> reader.
>
> [..]

> >
> > It doesn't need to be may or june.  Could even be
> > "alice-laptop.example.org", "bob-workstaion.example.org" or something
> > more even more unique.  Just have a broader variety.
>
> Ooh, I like using alice and bob!  Typical crypto example names, and
> easy to differentiate.  But I would prefer to ditch the -laptop and
> -workstation, to prevent unintended or implicit meanings as Philip
> suggested.
>


Yes, alice and bob please.

Alas, we have no mention of middlemen in the manpage -- having
eve.example.com as well would made it complete and more fun to read.

Cheers,

Selva

Re: [Openvpn-devel] [PATCH] Use example.com to improve clarity of documentation

[David Sommerseth]

On 30/11/15 04:03, Phillip Smith wrote:
> This patch uses the generic "host1.example.com" and "host2.example.com" to
> replace the current "may" and "june" hostname examples. Generic names chosen
> rather than other names like "server"/"client" or 
> "head-office"/"remote-office"
> etc which may create other unintended or implicit meanings to the reader.
> The example.com domain is set aside defined by IANA for use as documentation
> examples. Refer to: http://www.iana.org/domains/reserved
> Using this well-known domain makes comprehension of documentation easier.
> 
> Signed-off-by: Phillip Smith 
> ---
>  doc/openvpn.8 | 84 
> +--
>  1 file changed, 42 insertions(+), 42 deletions(-)

Hi,

As this popped up in my mailbox now as Steffan replied ... just a
thought.  I also share the general Feature-ACK to use
example.{org,com,net} domains.  Which ever variant isn't important for
my part.

[...snip...]

> -.B may.kg
> +.B host1.example.com
>  and
> -.B june.kg.
> +.B host2.example.com.

Any reason changing from {may, june} to {host1, host2}?  I find
documentation to be easier to read when there are more than just one or
two letters changing.  Especially when it is related to two different
systems you want to configure to talk to each other.  For people
struggling with dyslexia, more diversity also helps a lot too.

It doesn't need to be may or june.  Could even be
"alice-laptop.example.org", "bob-workstaion.example.org" or something
more even more unique.  Just have a broader variety.

Just my 2 cents ... not trying to paint any bike sheds.


-- 
kind regards,

David Sommerseth



signature.asc
Description: OpenPGP digital signature

Re: [Openvpn-devel] [PATCH] Use example.com to improve clarity of documentation

[Steffan Karger]

Hi Philip,

On Mon, Nov 30, 2015 at 4:03 AM, Phillip Smith  wrote:
> This patch uses the generic "host1.example.com" and "host2.example.com" to
> replace the current "may" and "june" hostname examples. Generic names chosen
> rather than other names like "server"/"client" or 
> "head-office"/"remote-office"
> etc which may create other unintended or implicit meanings to the reader.
> The example.com domain is set aside defined by IANA for use as documentation
> examples. Refer to: http://www.iana.org/domains/reserved
> Using this well-known domain makes comprehension of documentation easier.

Feature-ACK to using example.com (or .net, or .org, I don't care).

> @@ -6741,12 +6742,11 @@ option to use OpenVPN's default key renegotiation 
> interval of one hour.
>  .SS Routing:
>  Assuming you can ping across the tunnel,
>  the next step is to route a real subnet over
> -the secure tunnel.  Suppose that may and june have two network
> -interfaces each, one connected
> -to the internet, and the other to a private
> -network.  Our goal is to securely connect
> -both private networks.  We will assume that may's private subnet
> -is 10.0.0.0/24 and june's is 10.0.1.0/24.
> +the secure tunnel.  Suppose that On host1.example.com and host2.example.com
> +have two network interfaces each, one connected to the internet, and the 
> other
> +to a private network.  Our goal is to securely connect both private networks.
> +We will assume that host1.example.com's private subnet is 10.0.0.0/24 and
> +host2.example.com's is 10.0.1.0/24.

An extra 'On' seems to have slipped in there.

Finally, we should probably do the same for the sample keys, and the
gen-sample-key.sh script.  You could either do that in a new version
of your patch, or I will send a follow-up patch to do so.  Both are
fine with me.

-Steffan

Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Steffan Karger]

Hi,

On Tue, Dec 15, 2015 at 6:24 PM, Jan Just Keijser  wrote:
> ah well, in that case I would simply write out get0_certificate again: the
> code for that function actually is:
>
> 3011 X509 *SSL_CTX_get0_certificate(const SSL_CTX *ctx)
> 3012 {
> 3013 if (ctx->cert != NULL)
> 3014 return ctx->cert->key->x509;
> 3015 else
> 3016 return NULL;
> 3017 }
>
>
> and as far as I can tell that "struct-path" is also valid for openssl <
> 1.0.2

Yes that was my first attempt to work around it, but ctx->cert is an
opaque struct cert_st (declared in ssl.h, but defined in ssl_locl.h,
which is not in include/openssl/).

-Steffan

Re: [Openvpn-devel] [PATCH] Updates to Changes.rst

[debbie10t]

- Original Message - 
From: "Gert Doering" 

To: 
Cc: 
Sent: Tuesday, December 15, 2015 5:10 PM
Subject: Re: [Openvpn-devel] [PATCH] Updates to Changes.rst


This is a bug in the mingw header files.  You need a patch for it
(or upgrade to a newer version than contained in ubuntu).  Annoying
indeed.


<..>


Same thing: buggy header files that do not properly protect against
multi-inclusion.


Ahh .. OK I had a feeling it was going to be something unexpected :-)

Re: 2.3branch vs git-master .. thanks still have a lot to learn ...

Thanks very much Gert and Selva.

Re: [Openvpn-devel] [PATCH] Updates to Changes.rst

[Selva Nair]

On Tue, Dec 15, 2015 at 11:29 AM,  wrote:


> Any clarity on what to expect of the --block-outside-dns option
> and what windows version it is/will be support would help.
>

1. git_master:
 -  May be built with target = vista and will run correctly only on vista
and above
(this is the default target in openvpn-build if git-master source is
used)
 -  Will support --block-outside-dns when run on vista and above --
undefined behaviour if attempted to run on winxp

2. release/2.3 (and upcoming 2.3.9)
- May be built for target = winxp
  (this is the default target in openvpn-build if release/2.3 source is
used)
- Support for --block-outside-dns determined automatically at run-time:
 when run on vista and above the option will be supported
 when run on winxp the option will be ignored or cause
 a fatal error depending on how it is specified:
if pushed from a server: just a warning in the logs
specified in the config: fatal error unless protected by
ignore-unknown-option or setenv-opt or forward-compatible...

 3. Both should build correctly for 64-bit and 32-bit platforms

Selva

Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Jan Just Keijser]

Hi,

On 15/12/15 13:21, Steffan Karger wrote:

The SSL_CTX_get0_certificate() function I used in 091edd8e is available
in
OpenSSL 1.0.2+ only.  Older versions seem to not have a useful
alternative.
The remaining option would then be to create a cache for our parsed
certificate, but that would mean adding more struct members and code for
the select group of people that do use an up-to-date openvpn, but do not
update their openssl.  I don't think that's worth it.  So just disable
the
code for older openssl versions.

I have code lying around for checking certificate dates for openssl
v0.9.7+
; you can find it here:
https://www.nikhef.nl/~janjust/proxy-verify/

the function of interest is grid_asn1TimeToTimeT ; it was/is on my TODO
list
to convert this code into a similar patch - perhaps we can integrate the
two?

But before we extract the time from the certificate, we need to either
cache our own x509 certificate (in the certificate file reading code,
the pkcs11 code, the management-external-key code, the ms crapi code,
etc...) or find a way to extract our own x509 cert from an SSL_CTX
(which SSL_CTX_get0_certificate() does, since from that part of the
code it *can* peek into the opaque 'struct cert_st').

Looking at the mess, I still think it is just not worth the extra
code.  But if you (or someone else) manage to find a clean and simple
way to perform the check pre-1.0.2, I will gladly review a patch :)


err, isn't it much easier to check the certificate expiry date when loading
the cert, e.g. in "tls_ctx_load_cert_file_and_copy"  in ssl_openssl.c ? or
am I missing something here?

Yes, we could do that.  But we would have to do that for both crypto
libraries separately, and at each place where we load certificates
(same list as above; file, pkcs#11, ms crapi,
management-external-cert, maybe more?).  Not as nice as a single check
after all certificate processing.


ah well, in that case I would simply write out get0_certificate again: 
the code for that function actually is:


3011 X509 *SSL_CTX_get0_certificate(const SSL_CTX *ctx)
3012 {
3013 if (ctx->cert != NULL)
3014 return ctx->cert->key->x509;
3015 else
3016 return NULL;
3017 }


and as far as I can tell that "struct-path" is also valid for openssl < 
1.0.2


cheers,

JJK

Re: [Openvpn-devel] [PATCH] Updates to Changes.rst

[Gert Doering]

Hi,

On Tue, Dec 15, 2015 at 04:29:21PM -, debbie...@gmail.com wrote:
> > From: Samuli Seppänen 
> >
> > This patch is for the release/2.3 branch
[..]
> > -Peer ID support
> > +Windows DNS leak fix
> > +This feature allows blocking all out-of-tunnel communication on 
> > TCP/UDP port
> > +53 (except for OpenVPN itself), preventing DNS Leaks on Windows 8.1 
> > and 10.
> > +
> 
> is WXP no longer supported by git-master ?

"this patch is for release/2.3 branch" - nobody is talking about master here.

[..]
> I thought I would give this a shot for WXP but I cannot get it to compile 
> for win32

Just use 2.3.9 as will be released in binary tomorrow :-) - but more
comments below.

> Any clarity on what to expect of the --block-outside-dns option
> and what windows version it is/will be support would help.

It will work on Vista+, and give an error on XP.

[..]
>  from /usr/share/mingw-w64/include/fwpmu.h:10,
>  from win32.c:52:
> /usr/share/mingw-w64/include/iketypes.h:747:2: error: #endif without #if
>  #endif /* __iketypes_h__ */

This is a bug in the mingw header files.  You need a patch for it
(or upgrade to a newer version than contained in ubuntu).  Annoying
indeed.

> In file included from /usr/share/mingw-w64/include/fwpmu.h:10:0,
>  from win32.c:52:
> /usr/share/mingw-w64/include/fwpmtypes.h:54:3: error: conflicting types for 
> ???FWPM_DISPLAY_DATA0???
>  } FWPM_DISPLAY_DATA0;
>^

Same thing: buggy header files that do not properly protect against
multi-inclusion.

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] Updates to Changes.rst

[Gert Doering]

Hi,

On Tue, Dec 15, 2015 at 04:40:32PM +0200, sam...@openvpn.net wrote:
> From: Samuli Seppänen 
> 
> This patch is for the "master" branch

This one does not apply to my "master" branch...

Applying: Updates to Changes.rst
error: patch failed: Changes.rst:34
error: Changes.rst: patch does not apply
Patch failed at 0001 Updates to Changes.rst

... but I'll apply the changes manually (and add Arne's important
remark "drop XP support").

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 applied] Re: Updates to Changes.rst

[Gert Doering]

ACK, and thanks for getting this started.

Your patch has been applied to the release/2.3 branch.

(I have added a bit more to it, some of the user-visible changes were
missing)

commit 3b1fa7f6ebe5d4bedfe66aac33222e7e1e3e420a

Author: Samuli Seppänen
List-Post: openvpn-devel@lists.sourceforge.net
Date:   Tue Dec 15 17:00:26 2015 +0200

 Updates to Changes.rst

 Signed-off-by: Samuli Seppänen 
 Acked-by: Gert Doering 
 Message-Id: <1450191626-24633-1-git-send-email-sam...@openvpn.net>
 URL: http://article.gmane.org/gmane.network.openvpn.devel/10816
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering

Re: [Openvpn-devel] [PATCH] Updates to Changes.rst

[debbie10t]

Hi
- Original Message - 
From: 

To: 
Sent: Tuesday, December 15, 2015 3:00 PM
Subject: [Openvpn-devel] [PATCH] Updates to Changes.rst



From: Samuli Seppänen 

This patch is for the release/2.3 branch

+Overview of changes in 2.3
+==

New features


-Peer ID support
+Windows DNS leak fix
+This feature allows blocking all out-of-tunnel communication on 
TCP/UDP port
+53 (except for OpenVPN itself), preventing DNS Leaks on Windows 8.1 
and 10.

+


is WXP no longer supported by git-master ?

Let's call this "idiot proofing" :-)


- Another Message - 
From: "Fish" 

To: 
Sent: Monday, December 14, 2015 8:41 PM
Subject: [Openvpn-devel] [PATCH v4] Make "block-outside-dns" option
platformagnostic


Make the "block-outside-dns" option agnostic of Windows versions by
dynamically
loading WFP-related functions. Cross-compiled on Linux and tested on
Windows
XP/10.


I thought I would give this a shot for WXP but I cannot get it to compile 
for win32




My system: uname -a
Linux [mint-172-64b] 3.16.0-38-generic #52~14.04.1-Ubuntu SMP Fri May 8
09:43:57 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux


I have watched all the openvpn-dev mailing list and _tried_ to follow all
instructions ..
I hope I have just done something stupid ..
like "this does not work for 32bit"
or "you need version X of Y"
or "generic-build system cannot build this version for win32 (yet)"
or something like that .. Any help or pointers welcome.

Any clarity on what to expect of the --block-outside-dns option
and what windows version it is/will be support would help.

Thanks




What I did:
===
sudo apt-get update & upgrade
Using generic build system (which successfully built openvpn-build.git today
using: IMAGEROOT=`pwd`/image-win32 CHOST=i686-w64-mingw32 \
   CBUILD=x86_64-pc-linux-gnu ./build)

Log:
---
Tue Dec 15 14:18:35 2015 us=45270 OpenVPN 2.3.6 i686-w64-mingw32 [SSL
(OpenSSL)] [LZO] [PKCS11] [IPv6] built on Dec 15 2015
Tue Dec 15 14:18:35 2015 us=45270 library versions: OpenSSL 1.0.1j 15 Oct
2014, LZO 2.08
---


Then I used the build system like so:
===
Cloned:
github.com/OpenVPN/openvpn --to-- generic/sources/openvpn-2.3_git
execed:
autoreconf -ivf + ./configure (on that clone)
execed:
tar vzcf openvpn-2.3_git.tar.gz openvpn-2.3_git/*
(rm -rf openvpn-2.3_git)
Removed:
5 other tarballs of normal build,
left openvpn-2.3_git.tar.gz in place.
Changed build.vars:
OPENVPN_VERSION="${OPENVPN_VERSION:-2.3_git}"
execed:
IMAGEROOT=`pwd`/image-win32 CHOST=i686-w64-mingw32 \
CBUILD=x86_64-pc-linux-gnu ./build)
===


This is log where build broke:
===

i686-w64-mingw32-windres -DHAVE_CONFIG_H -I. -I../.. -I../../include  -I../../include 
-I../../src/compat -DWIN32_LEAN_AND_MEAN -DNTDDI_VERSION=NTDDI_VISTA -D_WIN32_WINNT=_WIN32_WINNT_VISTA 
-i "openvpn_win32_resources.rc" -o "openvpn_win32_resources.o"

mv -f .deps/cryptoapi.Tpo .deps/cryptoapi.Po
In file included from /usr/share/mingw-w64/include/ipsectypes.h:8:0,
from /usr/share/mingw-w64/include/fwpmtypes.h:9,
from /usr/share/mingw-w64/include/fwpmu.h:10,
from win32.c:52:
/usr/share/mingw-w64/include/iketypes.h:747:2: error: #endif without #if
#endif /* __iketypes_h__ */
 ^
In file included from /usr/share/mingw-w64/include/fwpmtypes.h:10:0,
from /usr/share/mingw-w64/include/fwpmu.h:10,
from win32.c:52:
/usr/share/mingw-w64/include/iketypes.h:747:2: error: #endif without #if
#endif /* __iketypes_h__ */
 ^
In file included from /usr/share/mingw-w64/include/fwpmu.h:10:0,
from win32.c:52:
/usr/share/mingw-w64/include/fwpmtypes.h:51:16: error: redefinition of 
‘struct FWPM_DISPLAY_DATA0_’

typedef struct FWPM_DISPLAY_DATA0_ {
   ^
In file included from /usr/share/mingw-w64/include/fwpmu.h:9:0,
from win32.c:52:
/usr/share/mingw-w64/include/fwptypes.h:336:16: note: originally defined 
here

typedef struct FWPM_DISPLAY_DATA0_ {
   ^
In file included from /usr/share/mingw-w64/include/fwpmu.h:10:0,
from win32.c:52:
/usr/share/mingw-w64/include/fwpmtypes.h:54:3: error: conflicting types for 
‘FWPM_DISPLAY_DATA0’

} FWPM_DISPLAY_DATA0;
  ^
In file included from /usr/share/mingw-w64/include/fwpmu.h:9:0,
from win32.c:52:
/usr/share/mingw-w64/include/fwptypes.h:339:3: note: previous declaration of 
‘FWPM_DISPLAY_DATA0’ was here

} FWPM_DISPLAY_DATA0;
  ^
In file included from /usr/share/mingw-w64/include/fwpmu.h:11:0,
from win32.c:52:
/usr/share/mingw-w64/include/iketypes.h:747:2: error: #endif without #if
#endif /* __iketypes_h__ */
 ^
make[4]: *** [win32.o] Error 1
make[4]: Leaving directory 
`/home/arby/openvpn-build/20151214-after/generic/tmp/openvpn-2.3_git/src/openvpn'

make[3]: *** [install-recursive] Error 1
make[3]: Leaving 

[Openvpn-devel] [PATCH] Updates to Changes.rst

[samuli]

From: Samuli Seppänen 

This patch is for the release/2.3 branch

Signed-off-by: Samuli Seppänen 
---
 Changes.rst | 390 ++--
 1 file changed, 383 insertions(+), 7 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index 0188323..38f42b6 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -1,24 +1,400 @@
-Version 2.3.9
-=
-
+Overview of changes in 2.3
+==
 
 New features
 
 
-Peer ID support
+Windows DNS leak fix
+This feature allows blocking all out-of-tunnel communication on TCP/UDP 
port
+53 (except for OpenVPN itself), preventing DNS Leaks on Windows 8.1 and 10.
+
+Client-only support for Peer ID
 Added new packet format P_DATA_V2, which includes peer-id. If
 server and client  support it, client sends all data packets in
 the new format. When data packet arrives, server identifies peer
 by peer-id. If peer's ip/port has changed, server assumes that
 client has floated, verifies HMAC and updates ip/port in internal structs.
+OpenvPN 2.3.x has client-side functionality only, server needs 2.4.
+
+TLS version negotiation
+Updated the TLS negotiation logic to adaptively try to connect using
+the highest TLS version supported by both client and server. The behavior
+of this feature can be adjusted as necessary.
+
+Push peer info
+Always push basic set of peer info values to server. This allows the
+server to make informed choices based on the capabilities of the client.
+The capabilities include things like supported compression algorithms,
+SSL library version and GUI version. The amount of data transmitted in peer
+information can be adjusted.
+
+PolarSSL support
+Allow use of PolarSSL in OpenVPN as the crypto library, the SSL library and
+for providing PKCS#11 support.
+
+Plug-in API v3
+This is a new, more flexible plug-in API.
+
+IPv6 payload and transport support
+Allow tunneling IPv6 traffic inside an IPv6 tunnel, as well as using IPv6
+as the transport for OpenVPN traffic.
+
+Client-side one-to-one NAT support
+This feature allows using SNAT or DNAT internally in OpenVPN to work around
+IP numbering conflicts with pushed routes.
+
+Support for a challenge/response protocol
+Allows an OpenVPN server to generate challenge questions for the user. This
+can be used to implement multi-factor authentication. Both dynamic and
+static challenges are supported.
+
+Improved UTF-8 support
+OpenVPN can now manage UTF-8 characters, for example in usernames,
+passwords, X.509 DNs and Windows paths.
+
+Behavioral changes
+--
+
+- Remove --enable-password-save option, which was seen as universally useful
+
+- Disallow usage of --server-poll-timeout in --secret key mode
+
+- The second parameter of --ifconfig is no longer a "remote address" but a
+  "netmask" when using --dev tun and --topology subnet
+
+- Automatic TLS version negotiation may cause issues in certain cases.
+
+- Don't exit daemon if opening or parsing the CRL fails
+
+- Do not upcase x509-username-field for mixed-case arguments
+
+- Allow use of connection block variables after connection blocks: this may
+  cause issues in some cases
+
+- Always load intermediate certificates from a PKCS#12 file, instead of 
ignoring
+  them
+
+- Remove the --disable-eurephia configure option
+
+- Remove the support for using system() when executing external programs or
+  scripts
+
+- Inline files are now always enabled
+
+- Remove the --auto-proxy option
+
+- Directory layout restructuring
+
+- A Windows buildsystem is no longer bundled with OpenVPN 
+
+- Easy-rsa is no longer bundled with OpenVPN
+
+- Tap-windows driver sources are no longer bundled with OpenVPN
+
+- Made some options connection-entry specific
+
+- Make '--win-sys env' default
+
+-  Do not randomize resolving of IP addresses in getaddr()
+
+Version 2.3.9
+=
+
+New features
+
 
-(2.3.x has client-side functionality only, server needs 2.4)
+- Windows DNS leak fix
 
+- Client-only Peer ID support
 
-User-visible Changes
-
+Behavioral changes
+--
 
 - sndbuf and recvbuf default now to OS default instead of 64k
 
 - Removed --enable-password-save from configure. This option is now
   always enabled.
+
+
+Version 2.3.8
+=
+
+Bug fixes
+-
+
+- Lots of bug fixes and documentation improvements
+
+Version 2.3.7
+=
+
+Bug fixes
+-
+
+- Lots of bug fixes and documentation improvements
+
+New features
+
+
+- include ifconfig\_ environment variables in --up-restart env set
+- Re-read auth-user-pass file on (re)connect if required
+
+Behavioral changes
+--
+
+- Disallow usage of --server-poll-timeout in --secret key mode
+- Re-enable TLS version negotiation by default
+
+Version 2.3.6
+=
+
+Bug fixes

[Openvpn-devel] [PATCH] Updates to Changes.rst

[samuli]

From: Samuli Seppänen 

This patch is for the "master" branch

Signed-off-by: Samuli Seppänen 
---
 Changes.rst | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index 41629bd..61e1e59 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -1,10 +1,13 @@
-Version 2.4.0
-=
-
+Overview of changes in 2.4
+==
 
 New features
 
 
+Windows DNS leak fix
+This feature allows blocking all out-of-tunnel communication on TCP/UDP 
port
+53 (except for OpenVPN itself), preventing DNS Leaks on Windows 8.1 and 10.
+
 keying-material-exporter
 Keying Material Exporter [RFC-5705] allow additional keying material to be
 derived from existing TLS channel.
@@ -34,18 +37,16 @@ LZ4 Compression
 compression.
 
 
-User-visible Changes
-
+Behavioral changes
+--
 - proto udp and proto tcp specify to use IPv4 and IPv6. The new
   options proto udp4 and tcp4 specify to use IPv4 only.
 
 - connect-timeout specifies now the timeout until the first TLS packet
   is received (identical to server-poll-timeout) and this timeout now
-  includes the removed socks proxy timeout and http proxy timeout.
-
-  In --static mode connect-timeout specifies the timeout for TCP and
-  proxy connection establishment
-
+  includes the removed socks proxy timeout and http proxy timeout. In --static
+  mode connect-timeout specifies the timeout for TCP and proxy connection
+  establishment
 
 - connect-retry now specifies the maximum number of unsucessfully
   trying all remote/connection entries before exiting.
-- 
2.1.0

Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Jan Just Keijser]

Hi,

On 15/12/15 10:12, Steffan Karger wrote:

Hi,

On Tue, Dec 15, 2015 at 9:42 AM, Jan Just Keijser  wrote:

On 14/12/15 23:14, Steffan Karger wrote:

The SSL_CTX_get0_certificate() function I used in 091edd8e is available in
OpenSSL 1.0.2+ only.  Older versions seem to not have a useful
alternative.
The remaining option would then be to create a cache for our parsed
certificate, but that would mean adding more struct members and code for
the select group of people that do use an up-to-date openvpn, but do not
update their openssl.  I don't think that's worth it.  So just disable the
code for older openssl versions.

I have code lying around for checking certificate dates for openssl v0.9.7+
; you can find it here:
   https://www.nikhef.nl/~janjust/proxy-verify/

the function of interest is grid_asn1TimeToTimeT ; it was/is on my TODO list
to convert this code into a similar patch - perhaps we can integrate the
two?

But before we extract the time from the certificate, we need to either
cache our own x509 certificate (in the certificate file reading code,
the pkcs11 code, the management-external-key code, the ms crapi code,
etc...) or find a way to extract our own x509 cert from an SSL_CTX
(which SSL_CTX_get0_certificate() does, since from that part of the
code it *can* peek into the opaque 'struct cert_st').

Looking at the mess, I still think it is just not worth the extra
code.  But if you (or someone else) manage to find a clean and simple
way to perform the check pre-1.0.2, I will gladly review a patch :)

err, isn't it much easier to check the certificate expiry date when 
loading the cert, e.g. in "tls_ctx_load_cert_file_and_copy"  in 
ssl_openssl.c ? or am I missing something here?


JJK

Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Steffan Karger]

Hi,

On Tue, Dec 15, 2015 at 9:42 AM, Jan Just Keijser  wrote:
> On 14/12/15 23:14, Steffan Karger wrote:
>> The SSL_CTX_get0_certificate() function I used in 091edd8e is available in
>> OpenSSL 1.0.2+ only.  Older versions seem to not have a useful
>> alternative.
>> The remaining option would then be to create a cache for our parsed
>> certificate, but that would mean adding more struct members and code for
>> the select group of people that do use an up-to-date openvpn, but do not
>> update their openssl.  I don't think that's worth it.  So just disable the
>> code for older openssl versions.
>
> I have code lying around for checking certificate dates for openssl v0.9.7+
> ; you can find it here:
>   https://www.nikhef.nl/~janjust/proxy-verify/
>
> the function of interest is grid_asn1TimeToTimeT ; it was/is on my TODO list
> to convert this code into a similar patch - perhaps we can integrate the
> two?

But before we extract the time from the certificate, we need to either
cache our own x509 certificate (in the certificate file reading code,
the pkcs11 code, the management-external-key code, the ms crapi code,
etc...) or find a way to extract our own x509 cert from an SSL_CTX
(which SSL_CTX_get0_certificate() does, since from that part of the
code it *can* peek into the opaque 'struct cert_st').

Looking at the mess, I still think it is just not worth the extra
code.  But if you (or someone else) manage to find a clean and simple
way to perform the check pre-1.0.2, I will gladly review a patch :)

-Steffan

Re: [Openvpn-devel] [PATCH] Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Jan Just Keijser]

Hi,

On 14/12/15 23:14, Steffan Karger wrote:

The SSL_CTX_get0_certificate() function I used in 091edd8e is available in
OpenSSL 1.0.2+ only.  Older versions seem to not have a useful alternative.
The remaining option would then be to create a cache for our parsed
certificate, but that would mean adding more struct members and code for
the select group of people that do use an up-to-date openvpn, but do not
update their openssl.  I don't think that's worth it.  So just disable the
code for older openssl versions.
I have code lying around for checking certificate dates for openssl 
v0.9.7+  ; you can find it here:

  https://www.nikhef.nl/~janjust/proxy-verify/

the function of interest is grid_asn1TimeToTimeT ; it was/is on my TODO 
list to convert this code into a similar patch - perhaps we can 
integrate the two?


cheers,

JJK



Signed-off-by: Steffan Karger 
---
  src/openvpn/ssl_openssl.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 2b74818..4792b08 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -353,6 +353,7 @@ tls_ctx_restrict_ciphers(struct tls_root_ctx *ctx, const 
char *ciphers)
  void
  tls_ctx_check_cert_time (const struct tls_root_ctx *ctx)
  {
+#if OPENSSL_VERSION_NUMBER >= 0x10002000L
int ret;
const X509 *cert = SSL_CTX_get0_certificate(ctx->ctx);
  
@@ -375,6 +376,7 @@ tls_ctx_check_cert_time (const struct tls_root_ctx *ctx)

  {
msg (M_WARN, "WARNING: Your certificate has expired!");
  }
+#endif
  }
  
  void

[Openvpn-devel] [PATCH applied] Re: Fix VS2013 compilation

[Gert Doering]

ACK, as this is basically the "2.3" parts of commit 123092a7a95.  Thanks.

Your patch has been applied to the release/2.3 branch.

commit 723c7c3d3a95f04a233449efd3ccd647eb0e1bf6 (release/2.3)
Author: Lev Stipakov
List-Post: openvpn-devel@lists.sourceforge.net
Date:   Tue Dec 15 10:18:22 2015 +0200

 Fix VS2013 compilation

 Signed-off-by: Lev Stipakov 
 Acked-by: Gert Doering 
 Message-Id: <1450167502-13471-1-git-send-email-lstipa...@gmail.com>
 URL: http://article.gmane.org/gmane.network.openvpn.devel/10809
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH 2.3] Fix VS2013 compilation

[Lev Stipakov]

Update toolset, define __attribute__.

Signed-off-by: Lev Stipakov 
---
 src/compat/compat.vcxproj   | 4 +++-
 src/openvpn/openvpn.vcxproj | 6 --
 src/openvpn/syshead.h   | 1 +
 src/openvpnserv/openvpnserv.vcxproj | 4 +++-
 4 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/compat/compat.vcxproj b/src/compat/compat.vcxproj
index 42979c1..1dedb33 100644
--- a/src/compat/compat.vcxproj
+++ b/src/compat/compat.vcxproj
@@ -1,5 +1,5 @@
 
-http://schemas.microsoft.com/developer/msbuild/2003;>
+http://schemas.microsoft.com/developer/msbuild/2003;>
   
 
   Debug
@@ -20,10 +20,12 @@
 StaticLibrary
 MultiByte
 true
+v120
   
   
 StaticLibrary
 MultiByte
+v120
   
   
   
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index 0b1b3fd..d691500 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -1,5 +1,5 @@
 
-http://schemas.microsoft.com/developer/msbuild/2003;>
+http://schemas.microsoft.com/developer/msbuild/2003;>
   
 
   Debug
@@ -20,10 +20,12 @@
 Application
 true
 Unicode
+v120
   
   
 Application
 Unicode
+v120
   
   
   
@@ -260,4 +262,4 @@
   
   
   
-
+
\ No newline at end of file
diff --git a/src/openvpn/syshead.h b/src/openvpn/syshead.h
index ffba4e8..24926ae 100644
--- a/src/openvpn/syshead.h
+++ b/src/openvpn/syshead.h
@@ -47,6 +47,7 @@

 #ifdef _MSC_VER // Visual Studio
 #define __func__ __FUNCTION__
+#define __attribute__(x)
 #endif

 #if defined(__APPLE__)
diff --git a/src/openvpnserv/openvpnserv.vcxproj 
b/src/openvpnserv/openvpnserv.vcxproj
index 0b75ed0..2a8943d 100644
--- a/src/openvpnserv/openvpnserv.vcxproj
+++ b/src/openvpnserv/openvpnserv.vcxproj
@@ -1,5 +1,5 @@
 
-http://schemas.microsoft.com/developer/msbuild/2003;>
+http://schemas.microsoft.com/developer/msbuild/2003;>
   
 
   Debug
@@ -20,10 +20,12 @@
 Application
 MultiByte
 true
+v120
   
   
 Application
 MultiByte
+v120
   
   
   
-- 
1.9.1

Re: [Openvpn-devel] [PATCH] Warn user if their certificate has expired

[Gert Doering]

Hi,

On Tue, Dec 15, 2015 at 01:12:49AM +0100, David Sommerseth wrote:
> Just tried to build openvpn on one of my laptops (Scientific Linux 7.1,
> openssl-1.0.1e-42.el7).  And it explodes when reaching the
> SSL_CTX_get0_certificate(), it seems that support arrived in OpenSSL 1.0.2?
> Could that be right?  Haven't had brainpower yet to dig deeper.

Yeah, we noticed.  Sorry for that.  Never seen so much red on the buildbot
list before... :-)

Committed and pushed the "not before 1.0.2" workaround.

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 applied] Re: Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

[Gert Doering]

ACK (grumble...)

Your patch has been applied to the master branch.

commit 644f2cdd13f49cd374aebc1fc506474104aac372
Author: Steffan Karger
List-Post: openvpn-devel@lists.sourceforge.net
Date:   Mon Dec 14 23:14:45 2015 +0100

 Disable certificate notBefore/notAfter sanity check on OpenSSL < 1.0.2

 Signed-off-by: Steffan Karger 
 Acked-by: Gert Doering 
 Message-Id: <1450131285-30182-1-git-send-email-stef...@karger.me>
 URL: http://article.gmane.org/gmane.network.openvpn.devel/10802
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering

Re: [Openvpn-devel] [PATCH] Improve stdin prompting section, fixing CR prompting.

[Wayne Davison]

On Mon, Dec 14, 2015 at 4:10 PM, Selva Nair  wrote:

> I took a quick look and it seems a simplified patch that addresses the
> most critical-sounding issue (challenge/reponse not prompted for
> from stdin) may be more useful.
>

That's exactly what that patch is.

>From the cover-letter to the patches:
>

That's the older patch set.  The newer one (based on newer master changes
in misc.c) separates the fix from the feature.

If the response is unchanging, its a misuse of challenge/response.
>

It's only mostly unchanging.  The user can respond "push", "sms", "phone",
or enter a number from the generator app as they see fit. For me, I use
"push" most of the time.  I figure if the app is going to support reading
the prompts from a file, it should support all the prompts the same way for
consistency.

..wayne..

Re: [Openvpn-devel] [PATCH] Warn user if their certificate has expired

[David Sommerseth]

On 14/12/15 21:09, Steffan Karger wrote:
> Previously, client certificate expiry warnings would only visible in the
> server log, and server certificate expiry warnings in the client log.
> Both after a (failed) connection attempt.  This patch adds a warning to
> log when a users own certificate has expired (or is not yet valid) to ease
> problem diagnosis / error reporting.
> 
> Note that this is just a warning, since on some systems (notably embedded
> devices) there might be no correct time available.
> 
> Signed-off-by: Steffan Karger 
> ---
>  src/openvpn/ssl.c  |  3 +++
>  src/openvpn/ssl_backend.h  |  9 +
>  src/openvpn/ssl_openssl.c  | 27 +++
>  src/openvpn/ssl_polarssl.c | 14 ++
>  4 files changed, 53 insertions(+)
> 
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index 887bd75..665fdd7 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
[...snip...]
>  
>  void
> +tls_ctx_check_cert_time (const struct tls_root_ctx *ctx)
> +{
> +  int ret;
> +  const X509 *cert = SSL_CTX_get0_certificate(ctx->ctx);

Hi,

Just tried to build openvpn on one of my laptops (Scientific Linux 7.1,
openssl-1.0.1e-42.el7).  And it explodes when reaching the
SSL_CTX_get0_certificate(), it seems that support arrived in OpenSSL 1.0.2?
Could that be right?  Haven't had brainpower yet to dig deeper.

I will retry a build on my work laptop too (RHEL 7.2) tomorrow, to see if this
is an issue there too.


-- 
kind regards,

David Sommerseth

-- 
kind regards,

David Sommerseth

Re: [Openvpn-devel] Topics for today's (Monday, 14th Dec 2015) community meeting

[Selva Nair]

Hi,

On Mon, Dec 14, 2015 at 4:45 PM, Samuli Seppänen  wrote:

Discussed OpenVPN 2.3.9 release. Here is the release plan:
>
 [..]

> In addition:
>
>
> - - the initial windows installers will not have the openvpn-gui changes
> - - mattock will provide test installers with the changes and send a link
> to the list
> - - if the test installers work fine for people, new official installers
> will released
>

Probably this is a good place to post this reminder, mainly for Samuli:

The next windows installer needs to remove the "disconnect_on_suspend = 1"
registry entry for the GUI (remove or re-write to = 0). Otherwise  GUI
users will experience weired disconnections after suspend/resume.

This is because of the way openvpn.exe now handles suspend/resume and the
way GUI treats delayed response from the app.

I thought I had submitted a patch for this to openvpn-build, but can't find
it... Anyway, its a one liner.

Thanks,

Selva

Re: [Openvpn-devel] [PATCH] Improve stdin prompting section, fixing CR prompting.

[Selva Nair]

Hi,

On Mon, Dec 14, 2015 at 4:56 PM, Wayne Davison  wrote:

>
> On Thu, Dec 10, 2015 at 8:57 AM, Wayne Davison 
> wrote:
>
>>  src/openvpn/misc.c | 119
>> +
>>  1 file changed, 57 insertions(+), 62 deletions(-)
>>
>
> Any questions I can answer about this patch?  This is such a
> straight-forward bug with a simple fix that I'd hope that it makes it into
> the upcoming release. (The patch is mainly re-indenting, which bloats it a
> good bit.)
>

I took a quick look and it seems a simplified patch that addresses
the most critical-sounding issue (challenge/reponse not prompted for
from stdin) may be more useful.

>From the cover-letter to the patches:

There is a bug in the challenge/response code when the username & password
> is read from a file -- the response is never prompted for.  This bug
> affects
> older versions, including 2.3.8.
>

A patch that fixes that and only that is easier to review.

I also added code to read the response out of the --auth-user-pass file if
> there is a 3rd line present.  This is particularly useful for an 2FA setup
> where the response is unchanging (in my case it is always "push" for a Duo
> Push).
>

I see no compelling reason to read response to a challenge from a file.
If the response is unchanging, its a misuse of challenge/response.
Implemet the check on the server such that it doesn't have to hear
"push" every time from the client. In case of, duo it could be an
out-of-band way to add user preferences on the sever saying user1
prefers push, user2 phone,  user3 sms etc. Or send it attached to
the username or password and parse it in the server-side user-pass-verify
script or in a custom pam_module, for example. This is a custom use
case trying to exploit CR for something else. Nothing wrong with that, but
may be done without adding a new "feature" into OpenVPN.

Selva