date:20210321

Re: [Openvpn-devel] [PATCH 2/2] Implement '--compress migrate' to migrate to non-compression setup

2021-03-21 Thread David Sommerseth


On 21/03/2021 13:56, Arne Schwabe wrote:

Am 20.03.21 um 14:20 schrieb David Sommerseth:

On 19/03/2021 16:31, Arne Schwabe wrote:

This option allow migration to a non compression server config while
still retraining compatibility with client that have a compression
setting in their config.

For existing setups that used to have comp-lzo no or another
compression setting in their configs it is a difficult to migrate to
a setup without compression without replacing all client configs at
once especially if OpenVPN 2.3 or earlier clients are in the mix that
do not support pushing stub-v2. Even with OpenVPN 2.4 and later clients
that support pushing this is not a satisfying solution as the clients
log occ mismatches and the "push stub-v2" needs to be in the server
config "forever".

Signed-off-by: Arne Schwabe 
---
   doc/man-sections/protocol-options.rst | 12 +++-
   src/openvpn/comp.h    |  1 +
   src/openvpn/multi.c   | 41 +
   src/openvpn/options.c |  6 ++
   src/openvpn/ssl.c | 34 ++-
   src/openvpn/ssl_common.h  |  1 +
   src/openvpn/ssl_util.c    | 43 ++
   src/openvpn/ssl_util.h    | 15 +
   tests/unit_tests/openvpn/Makefile.am  | 14 -
   tests/unit_tests/openvpn/test_misc.c  | 83 +++
   10 files changed, 245 insertions(+), 5 deletions(-)
   create mode 100644 tests/unit_tests/openvpn/test_misc.c



This fails compiling:

../../../src/openvpn/ssl.c: In function ‘key_method_2_write’:
../../../src/openvpn/ssl.c:2280:13: error: ‘multi’ undeclared (first use
in this function)
  if (multi->remote_usescomp && session->opt->mode == MODE_SERVER
  ^
../../../src/openvpn/ssl.c:2280:13: note: each undeclared identifier is
reported only once for each function it appears in
make[3]: *** [Makefile:742: ssl.o] Error 1


Oh the fun of rebasing. The multi argument was added by another patch.
Can you just the small patch to review the rest of the patch?:

  static bool
-key_method_2_write(struct buffer *buf, struct tls_session *session)
+key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct
tls_session *session)
  {
  struct key_state *ks = >key[KS_PRIMARY];  /* primary
key */

@@ -2856,7 +2856,7 @@ tls_process(struct tls_multi *multi,
  if (!buf->len && ((ks->state == S_START && !session->opt->server)
|| (ks->state == S_GOT_KEY &&
session->opt->server)))
  {
-if (!key_method_2_write(buf, session))
+if (!key_method_2_write(buf, multi, session))
  {
  goto error;
  }



I'll give this a spin and continue the review.  Thx!


--
kind regards,

David Sommerseth
OpenVPN Inc




OpenPGP_signature
Description: OpenPGP digital signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2 5/5] Deprecate the --verify-hash option

2021-03-21 Thread Antonio Quartulli

Hi,

On 21/03/2021 18:38, Arne Schwabe wrote:
> This patch conflicts since the grammar in the previous patch was fixed.
> If there is nothing else wrong with it I can resend a rebased v3.

That was it. Feel free to send v3.

Cheers,

-- 
Antonio Quartulli


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

[Openvpn-devel] [PATCH applied] Re: Implement peer-fingerprint to check fingerprint of peer certificate

2021-03-21 Thread Gert Doering

Your patch has been applied to the master branch.

I have moved the Changes.rst hunk to the "new in 2.6" section, as my
time machine is broken and this won't make 2.5.1 anymore :-)

I have not tested this feature itself, just stared at the code + docs
(seems to make sense) and ran the client side tests (fine).  A real
test setup will have to be built.

commit c3a7065d5bec0ca4ad479e27c124e74fbd7c2234
Author: Arne Schwabe
Date:   Sun Mar 21 15:33:53 2021 +0100

 Implement peer-fingerprint to check fingerprint of peer certificate

 Signed-off-by: Arne Schwabe 
 Acked-by: Antonio Quartulli 
 Message-Id: <20210321143353.2677-1-a...@rfc2549.org>
 URL: 
https://www.mail-archive.com/search?l=mid=20210321143353.2677-1-a...@rfc2549.org
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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

[Openvpn-devel] [PATCH applied] Re: Extend verify-hash to allow multiple hashes

2021-03-21 Thread Gert Doering

Your patch has been applied to the master branch.

I have not really tested it, but stared a bit at code and documentation,
and done a basic t_client test.  Haven't set up anything that could
thoroughly test this (yet!).

As discussed on IRC, I have removed the "return NULL" as suggested,
and there will be a cleanup patch to fix this function for good.

commit d1fe6d52ca066ec2d49712081d5056825c8973b2
Author: Arne Schwabe
Date:   Sun Mar 21 15:25:38 2021 +0100

 Extend verify-hash to allow multiple hashes

 Signed-off-by: Arne Schwabe 
 Acked-by: Antonio Quartulli 
 Message-Id: <20210321142538.1656-1-a...@rfc2549.org>
 URL: 
https://www.mail-archive.com/search?l=mid=20210321142538.1656-1-a...@rfc2549.org
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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

Re: [Openvpn-devel] [PATCH v2 5/5] Deprecate the --verify-hash option

2021-03-21 Thread Arne Schwabe

Am 21.03.21 um 18:22 schrieb Antonio Quartulli:
> Hi,
> 
> On 19/03/2021 15:20, Arne Schwabe wrote:
>> Despite trying to figure out with multiple people what the use case for
>> this option is, we could not come up with a good one. Checking that only
>> a specific CA is used can be also done by only using that CA in the --ca
>> directive.
>>
>> Although it feels a bit strange to deprecate the option after improving
>> it with peer-fingerprint patches, all the improvements are needed for
>> --peer-fingerprint and making them specify to --peer-fingerprint would
>> have added more (unecessary) changes.
>>
>> Signed-off-by: Arne Schwabe 
> 
> This patch looks good to me, but it does not apply on top of the
> previous 4. Maybe it was committed without considering the old 4/4?
> 

This patch conflicts since the grammar in the previous patch was fixed.
If there is nothing else wrong with it I can resend a rebased v3.

Arne


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

Re: [Openvpn-devel] [PATCH v2 5/5] Deprecate the --verify-hash option

2021-03-21 Thread Antonio Quartulli

Hi,

On 19/03/2021 15:20, Arne Schwabe wrote:
> Despite trying to figure out with multiple people what the use case for
> this option is, we could not come up with a good one. Checking that only
> a specific CA is used can be also done by only using that CA in the --ca
> directive.
> 
> Although it feels a bit strange to deprecate the option after improving
> it with peer-fingerprint patches, all the improvements are needed for
> --peer-fingerprint and making them specify to --peer-fingerprint would
> have added more (unecessary) changes.
> 
> Signed-off-by: Arne Schwabe 

This patch looks good to me, but it does not apply on top of the
previous 4. Maybe it was committed without considering the old 4/4?

Regards,


-- 
Antonio Quartulli


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

[Openvpn-devel] [PATCH applied] Re: iservice: Resolve MSVC C4996 warnings

2021-03-21 Thread Gert Doering

Tested on Ubuntu 18 / MinGW, compiles.

Checked with the MSVC documentation, seems to make sense :-) - I do notice
that we use wcscat_s() in one of these hunks, and _tcscat_s() in another,
which seems to be the same thing if _UNICODE is defined (which, I think
we do).  Maybe an opportunity for another round of cleanup - unify the
windows string functions used to "just one variant".

From: line fixed, so Author: is right for this commit.

Your patch has been applied to the master branch.

commit df471f4de8af0cbcf23a4e36910554bea7bd9058
Author: Simon Rozman
Date:   Sun Mar 21 15:46:27 2021 +0100

 iservice: Resolve MSVC C4996 warnings

 Signed-off-by: Simon Rozman 
 Acked-by: Arne Schwabe 
 Message-Id: <20210321144627.1621-5-si...@rozman.si>
 URL: 
https://www.mail-archive.com/search?l=mid=20210321144627.1621-5-si...@rozman.si
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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

Re: [Openvpn-devel] [PATCH v2 3/5] Support fingerprint authentication without CA certificate

2021-03-21 Thread Antonio Quartulli

Hi,

On 19/03/2021 15:20, Arne Schwabe wrote:
> From: "Jason A. Donenfeld" 
> 
> OpenVPN traditionally works around CAs. However many TLS-based protocols also
> allow an alternative simpler mode in which rather than verify certificates
> against CAs, the certificate itself is hashed and compared against a
> pre-known set of acceptable hashes. This is usually referred to as
> "fingerprint verification". It's popular across SMTP servers, IRC servers,
> XMPP servers, and even in the context of HTTP with pinning.
> 
>* Allow not specifying the --ca parameter, to specify that
>  certificates should not be checked against a CA.
> 
> I've included some instructions on how to use all of this.
> 
> Server side:
> 
> 
> Make self-signed cert:
> $ openssl req -x509 -newkey ec:<(openssl ecparam -name secp384r1) -keyout 
> serverkey.pem -out servercert.pem -nodes -sha256 -days 3650 -subj '/CN=server'
> 
> Record our fingerprint in an environment variable for the client to use later:
> $ server_fingerprint="$(openssl x509 -in servercert.pem -noout -sha256 
> -fingerprint | sed 's/.*=//;s/\(.*\)/\1/')"
> 
> Client side:
> 
> Make self-signed cert:
> $ openssl req -x509 -newkey ec:<(openssl ecparam -name secp384r1) -keyout 
> clientkey.pem -out clientcert.pem -nodes -sha256 -days 3650 -subj '/CN=client'
> 
> Record our fingerprint in an environment variable for the server to use later:
> $ client_fingerprint="$(openssl x509 -in clientcert.pem -noout -sha256 
> -fingerprint | sed 's/.*=//;s/\(.*\)/\1/')"
> 
> Start server/client
> ===
> 
> Start openvpn with peer fingerprint verification:
> 
> $ sudo openvpn --server 10.66.0.0 255.255.255.0 --dev tun --dh none --cert 
> servercert.pem --key serverkey.pem --peer-fingerprint "$client_fingerprint"
> 
> $ sudo openvpn --client --remote 127.0.0.1 --dev tun --cert clientcert.pem 
> --key clientkey.pem --peer-fingerprint "$server_fingerprint" --nobind
> 
> Signed-off-by: Jason A. Donenfeld 
> 
> Patch V2: Changes in V2 (by Arne Schwabe):
>   - Only check peer certificates, not all cert levels, if you need
> multiple levels of certificate you should use a real CA
>   - Use peer-fingerprint instead tls-verify on server side in example.
>   - rename variable ca_file_none to verify_hash_no_ca
>   - do no require --ca none but allow --ca simply
> to be absent when --peer-fingprint is present
>   - adjust warnings/errors messages to also point to
> peer-fingerprint as valid verification method.
>   - Fix mbed TLS version of not requiring CA
> not working
> 
> Signed-off-by: Arne Schwabe 
> ---
>  src/openvpn/init.c   |  2 ++
>  src/openvpn/options.c| 30 +++---
>  src/openvpn/options.h|  1 +
>  src/openvpn/ssl.c|  2 +-
>  src/openvpn/ssl_common.h |  1 +
>  src/openvpn/ssl_verify_mbedtls.c | 17 +
>  src/openvpn/ssl_verify_openssl.c |  2 +-
>  7 files changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 731b0cf2..835621cb 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -2928,6 +2928,8 @@ do_init_crypto_tls(struct context *c, const unsigned 
> int flags)
>  to.verify_hash = options->verify_hash;
>  to.verify_hash_algo = options->verify_hash_algo;
>  to.verify_hash_depth = options->verify_hash_depth;
> +to.verify_hash_no_ca = options->verify_hash_no_ca;
> +
>  #ifdef ENABLE_X509ALTUSERNAME
>  memcpy(to.x509_username_field, options->x509_username_field, 
> sizeof(to.x509_username_field));
>  #else
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 6b4a2c11..27ed813d 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -2712,18 +2712,23 @@ options_postprocess_verify_ce(const struct options 
> *options,
>  else
>  {
>  #ifdef ENABLE_CRYPTO_MBEDTLS
> -if (!(options->ca_file))
> +if (!(options->ca_file || options->verify_hash_no_ca))
>  {
> -msg(M_USAGE, "You must define CA file (--ca)");
> +msg(M_USAGE, "You must define CA file (--ca) and/or "
> +"peer fingeprint verification "
> +"(--peer-fingerprint)");
>  }
>  if (options->ca_path)
>  {
>  msg(M_USAGE, "Parameter --capath cannot be used with the 
> mbed TLS version version of OpenVPN.");
>  }
>  #else  /* ifdef ENABLE_CRYPTO_MBEDTLS */
> -if ((!(options->ca_file)) && (!(options->ca_path)))
> +if ((!(options->ca_file)) && (!(options->ca_path))
> +&& (!(options->verify_hash_no_ca)))
>  {
> -msg(M_USAGE, "You must define CA file (--ca) or CA path 
> (--capath)");
> +msg(M_USAGE, "You must define CA file (--ca)

Re: [Openvpn-devel] [PATCH applied] Re: interactive.c: Resolve MSVC C4996 warning

2021-03-21 Thread Gert Doering

Hi,

On Sun, Mar 21, 2021 at 06:05:19PM +0100, Gert Doering wrote:
> commit 709c3810a1d67e2c4049e852529a0a0d1338c797
> Author: Simon Rozman via Openvpn-devel

Yeah.  Right.

So, apologies for not catching this in time and fixing the Author: line,
but this is exactly why I brought up the DMARC issue with Max Fillinger
yesterday.

And it royally messes up patchwork as well, because patchwork only cares
about mail addresses internally, and shows the name "last seen with that
address" - so the Patch from Steffan / Max is now listed as

"Simon Rozman via Openvpn-devel"

as well... https://patchwork.openvpn.net/patch/1628/

*sigh*

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 applied] Re: interactive.c: Resolve MSVC C4996 warning

2021-03-21 Thread Gert Doering

I seem to remember we had that discussion in the context of openpvpn-gui
already - Problems with the "POSIX compatible" function names, which end
up acting on narrow or wide strings depending on compiler settings, compiler
version, phase of the moon...  can't find that commit, but if MS docs says
"this is how windows code should look like", so be it...

I have test compiled stuff on Ubuntu 18 / MinGW, and it compiles.  Not tested.

Your patch has been applied to the master branch.

commit 709c3810a1d67e2c4049e852529a0a0d1338c797
Author: Simon Rozman via Openvpn-devel
Date:   Sun Mar 21 15:46:25 2021 +0100

 interactive.c: Resolve MSVC C4996 warning

 Signed-off-by: Simon Rozman 
 Acked-by: Arne Schwabe 
 Message-Id: <20210321144627.1621-3-si...@rozman.si>
 URL: 
https://www.mail-archive.com/search?l=mid=20210321144627.1621-3-si...@rozman.si
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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

[Openvpn-devel] [PATCH applied] Re: tun.c: Remove dead code

2021-03-21 Thread Gert Doering

Your patch has been applied to the master branch.

"Trivially correct" :-) - but since Windows keeps biting me these
days, test built with Ubuntu/MinGW.  Just to be sure.

commit 26540310efa8c8955f38974969b317460c075dd4
Author: Simon Rozman via Openvpn-devel
Date:   Sun Mar 21 15:46:24 2021 +0100

 tun.c: Remove dead code

 Signed-off-by: Simon Rozman 
 Acked-by: Arne Schwabe 
 Message-Id: <20210321144627.1621-2-si...@rozman.si>
 URL: 
https://www.mail-archive.com/search?l=mid=20210321144627.1621-2-si...@rozman.si
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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

Re: [Openvpn-devel] [PATCH 1/5] MSVC: Disable LZ4

2021-03-21 Thread Gert Doering

Hi,

On Sun, Mar 21, 2021 at 03:46:23PM +0100, Simon Rozman via Openvpn-devel wrote:
> Commit 24596b25 ("build: Remove compat-lz4") removed lz4 compat layer,
> but openvpn-build\msvc doesn't provide LZ4 library either.

What would be needed to actually *build* with LZ4 on MSVC?  That is,
build it as prerequisite as LZO is built?

The idea wasn't to remove LZ4 from builds, just to remove the bundled
LZ4 "because all platforms have it now, so we do not need to maintain 
our own copy".  But it seems that was a bit shortsighted wrt windows
building...

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 4/5] tapctl: Resolve MSVC C4996 warnings

2021-03-21 Thread Arne Schwabe

Am 21.03.21 um 17:37 schrieb Simon Rozman:
> Hi,
> 
>>> -73,14 +73,13 @@ find_function(const WCHAR *libname, const char
>> *funcname, HMODULE *m)
>>> return NULL;
>>>  }
>>>
>>> -size_t len = _countof(libpath) - wcslen(libpath) - 1;
>>> -if (len < wcslen(libname) + 1)
>>> +if (wcslen(libpath) + 1 /*\*/ + wcslen(libname) >=
>>> + _countof(libpath))
>>
>> This random inline comment feels extremely weird.
> 
> It's trying to describe the "+ 1" amounts for a backslash \ being strcat-ed 
> in the process below.
> 
> Regards, Simon
> 

I totally didn't get that. I was actually wondering why that is valid
code and why the \* doesn't escape the end of comment. I would prefer it
be either a longer inline comment or even better use a variable with a
normal comment like

/* +1 for the path seperator '\' */
size_t pathlenght = wcslen(libpath) + 1 +  wcslen(libname);

Arne


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

Re: [Openvpn-devel] [PATCH v3] Implement peer-fingerprint to check fingerprint of peer certificate

2021-03-21 Thread Antonio Quartulli

Hi,

On 21/03/2021 15:33, Arne Schwabe wrote:
> This option allows to pin one or more more peer certificates. It also
> prepares for doing TLS authentication without a CA and just
> self-signed certificates.
> 
> Patch V2: Allow peer-fingerprint to be specified multiple times
>   to allow multiple peers without needing to use inline
>   syntax. (e.g. on command line).
> 
> Patch V3: rebase on v3 of 1/4, reword message of verify-hash and
>   peer-fingerpring incompatibility
> 
> Signed-off-by: Arne Schwabe 
> ---
>  Changes.rst   |  9 ++-
>  doc/man-sections/inline-files.rst |  4 +--
>  doc/man-sections/tls-options.rst  | 22 +++-
>  src/openvpn/init.c|  1 +
>  src/openvpn/options.c | 42 +++
>  src/openvpn/options.h |  1 +
>  src/openvpn/ssl_common.h  |  1 +
>  src/openvpn/ssl_verify.c  | 19 --
>  8 files changed, 76 insertions(+), 23 deletions(-)
> 
> diff --git a/Changes.rst b/Changes.rst
> index d6be1050..ac32de26 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -22,13 +22,20 @@ Compatibility with OpenSSL in FIPS mode
>  
>  Deprecated features
>  ---
> -``inetd`` has been removed
> +``inetd`` has been removed 

I guess this is just a mistake.


>  This was a very limited and not-well-tested way to run OpenVPN, on TCP
>  and TAP mode only.
>  
>  
>  Overview of changes in 2.5
>  ==
> +New features in 2.5.1

I believe this feature was meant to target master/2.6, therefore putting
it under 2.5.1 was probably a mistake.

> +-
> +Certificate pinning/verify peer fingerprint
> +The ``--peer-fingerprint`` option has been introduced to give users an
> +easy to use alternative to the ``tls-verify`` for matching the
> +fingerprint of the peer. The option takes use a number of allowed
> +SHA256 certificate fingerprints.
>  
>  New features
>  
> diff --git a/doc/man-sections/inline-files.rst 
> b/doc/man-sections/inline-files.rst
> index 303bb3c8..01e4a840 100644
> --- a/doc/man-sections/inline-files.rst
> +++ b/doc/man-sections/inline-files.rst
> @@ -4,8 +4,8 @@ INLINE FILE SUPPORT
>  OpenVPN allows including files in the main configuration for the ``--ca``,
>  ``--cert``, ``--dh``, ``--extra-certs``, ``--key``, ``--pkcs12``,
>  ``--secret``, ``--crl-verify``, ``--http-proxy-user-pass``, ``--tls-auth``,
> -``--auth-gen-token-secret``, ``--tls-crypt``, ``--tls-crypt-v2`` and
> -``--verify-hash`` options.
> +``--auth-gen-token-secret``, ``--peer-fingerprint``, ``--tls-crypt``,
> +``--tls-crypt-v2`` and ``--verify-hash`` options.
>  
>  Each inline file started by the line  and ended by the line
>  
> diff --git a/doc/man-sections/tls-options.rst 
> b/doc/man-sections/tls-options.rst
> index d8f9800e..cfe1ec98 100644
> --- a/doc/man-sections/tls-options.rst
> +++ b/doc/man-sections/tls-options.rst
> @@ -271,7 +271,8 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
>man-in-the-middle attack where an authorized client attempts to connect
>to another client by impersonating the server. The attack is easily
>prevented by having clients verify the server certificate using any one
> -  of ``--remote-cert-tls``, ``--verify-x509-name``, or ``--tls-verify``.
> +  of ``--remote-cert-tls``, ``--verify-x509-name``, ``--peer-fingerprint``
> +  or ``--tls-verify``.
>  
>  --tls-auth args
>Add an additional layer of HMAC authentication on top of the TLS control
> @@ -592,6 +593,25 @@ certificates and keys: 
> https://github.com/OpenVPN/easy-rsa
>  
>  If the option is inlined, ``algo`` is always :code:`SHA256`.
>  
> +--peer-fingerprint args
> +   Specify a SHA256 fingerprint or list of SHA256 fingerprints to verify
> +   the peer certificate against. The peer certificate must match one of the
> +   fingerprint or certificate verification will fail. The option can also
> +   be inlined
> +
> +  Valid syntax:
> +  ::
> +
> +peer-fingerprint AD:B0:95:D8:09:...
> +
> +  or inline:
> +  ::
> +
> +
> +
> 00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff
> +
> 11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00
> +
> +
>  --verify-x509-name args
>Accept connections only if a host's X.509 name is equal to **name.** The
>remote host must also pass all other tests of verification.
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index d234729c..731b0cf2 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -2927,6 +2927,7 @@ do_init_crypto_tls(struct context *c, const unsigned 
> int flags)
>  to.remote_cert_eku = options->remote_cert_eku;
>  to.verify_hash = options->verify_hash;
>  to.verify_hash_algo = options->verify_hash_algo;
> +to.verify_hash_depth = options->verify_hash_depth;
>  #ifdef

Re: [Openvpn-devel] [PATCH 3/5] interactive.c: Resolve MSVC C4996 warning

2021-03-21 Thread Arne Schwabe

Am 21.03.21 um 15:46 schrieb Simon Rozman via Openvpn-devel:
> It's about using a standard recommended alias for the wcsdup():
> 
>> warning C4996: 'wcsdup': The POSIX name for this item is deprecated.
>> Instead, use the ISO C and C++ conformant name: _wcsdup. See online
>> help for details.
> 
> And the documentation says:
> 
>> The Microsoft-implemented POSIX function names strdup and wcsdup are
>> deprecated aliases for the _strdup and _wcsdup functions. By default,
>> they generate Compiler warning (level 3) C4996. The names are
>> deprecated because they don't follow the Standard C rules for
>> implementation-specific names. However, the functions are still
>> supported.
>>
>> We recommend you use _strdup and _wcsdup instead. Or, you can continue
>> to use these function names, and disable the warning. For more
>> information, see Turn off the warning and POSIX function names.
> 
> Reference: 
> https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/strdup-wcsdup
> Signed-off-by: Simon Rozman 
> ---
>  src/openvpnserv/interactive.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
> index 5d5cbfe6..b073a0d5 100644
> --- a/src/openvpnserv/interactive.c
> +++ b/src/openvpnserv/interactive.c
> @@ -899,7 +899,7 @@ ExecCommand(const WCHAR *argv0, const WCHAR *cmdline, 
> DWORD timeout)
>  si.cb = sizeof(si);
>  
>  /* CreateProcess needs a modifiable cmdline: make a copy */
> -cmdline_dup = wcsdup(cmdline);
> +cmdline_dup = _wcsdup(cmdline);
>  if (cmdline_dup && CreateProcessW(argv0, cmdline_dup, NULL, NULL, FALSE,
>proc_flags, NULL, NULL, , ) )
>  {
> @@ -1181,7 +1181,7 @@ SetDNSDomain(const wchar_t *if_name, const char 
> *domain, undo_lists_t *lists)
> /* Add to undo list if domain is non-empty */
> if (err == 0 && wdomain[0] && lists)
> {
> -wchar_t *tmp_name = wcsdup(if_name);
> +wchar_t *tmp_name = _wcsdup(if_name);
>  if (!tmp_name || AddListItem(&(*lists)[undo_domain], tmp_name))
>  {
>  free(tmp_name);
> @@ -1272,7 +1272,7 @@ HandleDNSConfigMessage(const dns_cfg_message_t *msg, 
> undo_lists_t *lists)
>  
>  if (msg->addr_len > 0)
>  {
> -wchar_t *tmp_name = wcsdup(wide_name);
> +wchar_t *tmp_name = _wcsdup(wide_name);
>  if (!tmp_name || AddListItem(&(*lists)[undo_type], tmp_name))
>  {
>  free(tmp_name);
> 

Acked-By: Arne Schwabe 

I can accept the change in this function since it is windows specific
but overall I don't think we can reasonable move away from POSIX
function names and instead use the names with _ in front of it because
that is currently Windows specific. I am not sure what the idea for a
portable code is other than to set the _CRT_NONSTDC_NO_DEPRECATE option
on Windows.

Arne


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

Re: [Openvpn-devel] [PATCH 4/5] tapctl: Resolve MSVC C4996 warnings

2021-03-21 Thread Simon Rozman via Openvpn-devel

Hi,

> > -73,14 +73,13 @@ find_function(const WCHAR *libname, const char
> *funcname, HMODULE *m)
> > return NULL;
> >  }
> >
> > -size_t len = _countof(libpath) - wcslen(libpath) - 1;
> > -if (len < wcslen(libname) + 1)
> > +if (wcslen(libpath) + 1 /*\*/ + wcslen(libname) >=
> > + _countof(libpath))
> 
> This random inline comment feels extremely weird.

It's trying to describe the "+ 1" amounts for a backslash \ being strcat-ed in 
the process below.

Regards, Simon

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

Re: [Openvpn-devel] [PATCH 4/5] tapctl: Resolve MSVC C4996 warnings

2021-03-21 Thread Arne Schwabe

Am 21.03.21 um 15:46 schrieb Simon Rozman via Openvpn-devel:
> wcsncat() was declared unsafe in favour of wcsncat_s(). However, the
> string concatenation follows the string length check, making wcsncat()
> safe too. Code analysis is just not smart enough (yet) to detect this.
> 
> The code was refactored to use wcscat_s() MSVC is considering as "safe".
> 
> Signed-off-by: Simon Rozman 
> ---
>  src/tapctl/tap.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c
> index dd4a10a3..3f76c43a 100644
> --- a/src/tapctl/tap.c
> +++ b/src/tapctl/tap.c
> @@ -2,7 +2,7 @@
>   *  tapctl -- Utility to manipulate TUN/TAP adapters on Windows
>   *https://community.openvpn.net/openvpn/wiki/Tapctl
>   *
> - *  Copyright (C) 2018-2020 Simon Rozman 
> + *  Copyright (C) 2018-2021 Simon Rozman 
>   *
>   *  This program is free software; you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License version 2
> @@ -73,14 +73,13 @@ find_function(const WCHAR *libname, const char *funcname, 
> HMODULE *m)
> return NULL;
>  }
>  
> -size_t len = _countof(libpath) - wcslen(libpath) - 1;
> -if (len < wcslen(libname) + 1)
> +if (wcslen(libpath) + 1 /*\*/ + wcslen(libname) >= _countof(libpath))

This random inline comment feels extremely weird.

Arne


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

Re: [Openvpn-devel] [PATCH 2/5] tun.c: Remove dead code

2021-03-21 Thread Arne Schwabe

Am 21.03.21 um 15:46 schrieb Simon Rozman via Openvpn-devel:
> Signed-off-by: Simon Rozman 
> ---
>  src/openvpn/tun.c | 34 --
>  1 file changed, 34 deletions(-)
> 


Acked-By: Arne Schwabe 


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

Re: [Openvpn-devel] [PATCH 1/5] MSVC: Disable LZ4

2021-03-21 Thread Arne Schwabe

Am 21.03.21 um 15:46 schrieb Simon Rozman via Openvpn-devel:
> Commit 24596b25 ("build: Remove compat-lz4") removed lz4 compat layer,
> but openvpn-build\msvc doesn't provide LZ4 library either.

We should either add lz4 to openvpn-build or change the default of lz4
to disabled in all variant. I don't like the idea that windows has a
different default than the other variants.

Arne


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

Re: [Openvpn-devel] [PATCH v3] Extend verify-hash to allow multiple hashes

2021-03-21 Thread Antonio Quartulli

Hi,

This patch looks good to me.
There is just one minor note below:

On 21/03/2021 15:25, Arne Schwabe wrote:
> This patch introduces support for verify-hash inlining.
> When inlined, this options now allows to specify multiple fingerprints,
> one per line.
> 
> Since this is a new syntax, there is no backwards compatibility to take
> care of, therefore we can drop support for SHA1. Inlined fingerprints
> are assumed be to SHA-256 only.
> 
> Also print a warning about SHA1 hash being deprecated to verify
> certificates as it is not "industry standard" anymore.
> 
> Patch v2: fix/clarify various comments, fix a few minor problems, allow
>   the option to be specified multiple times and have that
>   added to the list.
> 
> Patch v3: Remove leftover variable, always call
>   parse_hash_fingerprint_multiline, add comments clarifying list
>   appending
> 
> Signed-off-by: Arne Schwabe 
> ---
>  doc/man-sections/inline-files.rst |   4 +-
>  doc/man-sections/tls-options.rst  |  10 +++
>  src/openvpn/options.c | 102 ++
>  src/openvpn/options.h |  10 ++-
>  src/openvpn/ssl_common.h  |   2 +-
>  src/openvpn/ssl_verify.c  |  16 -
>  6 files changed, 127 insertions(+), 17 deletions(-)
> 
> diff --git a/doc/man-sections/inline-files.rst 
> b/doc/man-sections/inline-files.rst
> index 819bd3c8..303bb3c8 100644
> --- a/doc/man-sections/inline-files.rst
> +++ b/doc/man-sections/inline-files.rst
> @@ -4,8 +4,8 @@ INLINE FILE SUPPORT
>  OpenVPN allows including files in the main configuration for the ``--ca``,
>  ``--cert``, ``--dh``, ``--extra-certs``, ``--key``, ``--pkcs12``,
>  ``--secret``, ``--crl-verify``, ``--http-proxy-user-pass``, ``--tls-auth``,
> -``--auth-gen-token-secret``, ``--tls-crypt`` and ``--tls-crypt-v2``
> -options.
> +``--auth-gen-token-secret``, ``--tls-crypt``, ``--tls-crypt-v2`` and
> +``--verify-hash`` options.
>  
>  Each inline file started by the line  and ended by the line
>  
> diff --git a/doc/man-sections/tls-options.rst 
> b/doc/man-sections/tls-options.rst
> index e13fb3c8..d8f9800e 100644
> --- a/doc/man-sections/tls-options.rst
> +++ b/doc/man-sections/tls-options.rst
> @@ -582,6 +582,16 @@ certificates and keys: 
> https://github.com/OpenVPN/easy-rsa
>The ``algo`` flag can be either :code:`SHA1` or :code:`SHA256`. If not
>provided, it defaults to :code:`SHA1`.
>  
> +  This option can also be inlined
> +  ::
> +
> +
> +
> 00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff
> +
> 11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00
> +
> +
> +If the option is inlined, ``algo`` is always :code:`SHA256`.
> +
>  --verify-x509-name args
>Accept connections only if a host's X.509 name is equal to **name.** The
>remote host must also pass all other tests of verification.
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 86e78b05..3b1c69ba 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -1078,12 +1078,24 @@ string_substitute(const char *src, int from, int to, 
> struct gc_arena *gc)
>  return ret;
>  }
>  
> -static uint8_t *
> +/**
> + * Parses a hexstring and checks if the string has the correct length. Return
> + * a verify_hash_list containing the parsed hash string.
> + *
> + * @param   str String to check/parse
> + * @param   nbytes  Number of bytes expected in the hexstr (e.g. 20 for 
> SHA1)
> + * @param   msglevelmessage level to use when printing warnings/errors
> + * @param   gc  The returned object will be allocated in this gc
> + */
> +static struct verify_hash_list *
>  parse_hash_fingerprint(const char *str, int nbytes, int msglevel, struct 
> gc_arena *gc)
>  {
>  int i;
>  const char *cp = str;
> -uint8_t *ret = (uint8_t *) gc_malloc(nbytes, true, gc);
> +
> +struct verify_hash_list *ret;
> +ALLOC_OBJ_CLEAR_GC(ret, struct verify_hash_list, gc);
> +
>  char term = 1;
>  int byte;
>  char bs[3];
> @@ -1102,7 +1114,7 @@ parse_hash_fingerprint(const char *str, int nbytes, int 
> msglevel, struct gc_aren
>  {
>  msg(msglevel, "format error in hash fingerprint hex byte: %s", 
> str);
>  }
> -ret[i] = (uint8_t)byte;
> +ret->hash[i] = (uint8_t)byte;
>  term = *cp++;
>  if (term != ':' && term != 0)
>  {
> @@ -1116,10 +1128,55 @@ parse_hash_fingerprint(const char *str, int nbytes, 
> int msglevel, struct gc_aren
>  if (term != 0 || i != nbytes-1)
>  {
>  msg(msglevel, "hash fingerprint is different length than expected 
> (%d bytes): %s", nbytes, str);
> +return NULL;

The rest of this function assumes that msglevel is set to a FATAL level.

As you can also see in the hunk above, if blocks handling other errors
have no return statement, because they assume the

Re: [Openvpn-devel] [PATCH 5/5] iservice: Resolve MSVC C4996 warnings

2021-03-21 Thread Arne Schwabe

Am 21.03.21 um 15:46 schrieb Simon Rozman via Openvpn-devel:
> Lots of string functions were declared unsafe in favor of ..._s()
> counterparts. However, the code already is careful about the buffer
> size. Code analysis is just not smart enough (yet) to detect this.
> 
> The code was refactored to use ..._s() variants MSVC is considering as
> "safe".

Acked-By: Arne Schwabe 

This change changes Windows specifc functions to other windows specific
functions that do not emit errors. Test compiled with msvc/clang-cl

Arne


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

[Openvpn-devel] [PATCH 3/5] interactive.c: Resolve MSVC C4996 warning

2021-03-21 Thread Simon Rozman via Openvpn-devel

It's about using a standard recommended alias for the wcsdup():

> warning C4996: 'wcsdup': The POSIX name for this item is deprecated.
> Instead, use the ISO C and C++ conformant name: _wcsdup. See online
> help for details.

And the documentation says:

> The Microsoft-implemented POSIX function names strdup and wcsdup are
> deprecated aliases for the _strdup and _wcsdup functions. By default,
> they generate Compiler warning (level 3) C4996. The names are
> deprecated because they don't follow the Standard C rules for
> implementation-specific names. However, the functions are still
> supported.
>
> We recommend you use _strdup and _wcsdup instead. Or, you can continue
> to use these function names, and disable the warning. For more
> information, see Turn off the warning and POSIX function names.

Reference: 
https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/strdup-wcsdup
Signed-off-by: Simon Rozman 
---
 src/openvpnserv/interactive.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 5d5cbfe6..b073a0d5 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -899,7 +899,7 @@ ExecCommand(const WCHAR *argv0, const WCHAR *cmdline, DWORD 
timeout)
 si.cb = sizeof(si);
 
 /* CreateProcess needs a modifiable cmdline: make a copy */
-cmdline_dup = wcsdup(cmdline);
+cmdline_dup = _wcsdup(cmdline);
 if (cmdline_dup && CreateProcessW(argv0, cmdline_dup, NULL, NULL, FALSE,
   proc_flags, NULL, NULL, , ) )
 {
@@ -1181,7 +1181,7 @@ SetDNSDomain(const wchar_t *if_name, const char *domain, 
undo_lists_t *lists)
/* Add to undo list if domain is non-empty */
if (err == 0 && wdomain[0] && lists)
{
-wchar_t *tmp_name = wcsdup(if_name);
+wchar_t *tmp_name = _wcsdup(if_name);
 if (!tmp_name || AddListItem(&(*lists)[undo_domain], tmp_name))
 {
 free(tmp_name);
@@ -1272,7 +1272,7 @@ HandleDNSConfigMessage(const dns_cfg_message_t *msg, 
undo_lists_t *lists)
 
 if (msg->addr_len > 0)
 {
-wchar_t *tmp_name = wcsdup(wide_name);
+wchar_t *tmp_name = _wcsdup(wide_name);
 if (!tmp_name || AddListItem(&(*lists)[undo_type], tmp_name))
 {
 free(tmp_name);
-- 
2.30.0.windows.2



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

[Openvpn-devel] [PATCH 1/5] MSVC: Disable LZ4

2021-03-21 Thread Simon Rozman via Openvpn-devel

Commit 24596b25 ("build: Remove compat-lz4") removed lz4 compat layer,
but openvpn-build\msvc doesn't provide LZ4 library either.

Signed-off-by: Simon Rozman 
---
 config-msvc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config-msvc.h b/config-msvc.h
index e430ca96..53d97902 100644
--- a/config-msvc.h
+++ b/config-msvc.h
@@ -9,7 +9,6 @@
 #define ENABLE_FRAGMENT 1
 #define ENABLE_HTTP_PROXY 1
 #define ENABLE_LZO 1
-#define ENABLE_LZ4 1
 #define ENABLE_MANAGEMENT 1
 #define ENABLE_MULTIHOME 1
 #define ENABLE_PKCS11 1
-- 
2.30.0.windows.2



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

[Openvpn-devel] [PATCH 4/5] tapctl: Resolve MSVC C4996 warnings

2021-03-21 Thread Simon Rozman via Openvpn-devel

wcsncat() was declared unsafe in favour of wcsncat_s(). However, the
string concatenation follows the string length check, making wcsncat()
safe too. Code analysis is just not smart enough (yet) to detect this.

The code was refactored to use wcscat_s() MSVC is considering as "safe".

Signed-off-by: Simon Rozman 
---
 src/tapctl/tap.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/tapctl/tap.c b/src/tapctl/tap.c
index dd4a10a3..3f76c43a 100644
--- a/src/tapctl/tap.c
+++ b/src/tapctl/tap.c
@@ -2,7 +2,7 @@
  *  tapctl -- Utility to manipulate TUN/TAP adapters on Windows
  *https://community.openvpn.net/openvpn/wiki/Tapctl
  *
- *  Copyright (C) 2018-2020 Simon Rozman 
+ *  Copyright (C) 2018-2021 Simon Rozman 
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2
@@ -73,14 +73,13 @@ find_function(const WCHAR *libname, const char *funcname, 
HMODULE *m)
return NULL;
 }
 
-size_t len = _countof(libpath) - wcslen(libpath) - 1;
-if (len < wcslen(libname) + 1)
+if (wcslen(libpath) + 1 /*\*/ + wcslen(libname) >= _countof(libpath))
 {
SetLastError(ERROR_INSUFFICIENT_BUFFER);
return NULL;
 }
-wcsncat(libpath, L"\\", len);
-wcsncat(libpath, libname, len-1);
+wcscat_s(libpath, _countof(libpath), L"\\");
+wcscat_s(libpath, _countof(libpath), libname);
 
 *m = LoadLibraryW(libpath);
 if (*m == NULL)
-- 
2.30.0.windows.2



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

[Openvpn-devel] [PATCH 2/5] tun.c: Remove dead code

2021-03-21 Thread Simon Rozman via Openvpn-devel

Signed-off-by: Simon Rozman 
---
 src/openvpn/tun.c | 34 --
 1 file changed, 34 deletions(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 6c51a52d..6b7c8ef1 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -85,8 +85,6 @@ static void netsh_command(const struct argv *a, int n, int 
msglevel);
 
 static const char *netsh_get_id(const char *dev_node, struct gc_arena *gc);
 
-static DWORD get_adapter_index_flexible(const char *name);
-
 static bool
 do_address_service(const bool add, const short family, const struct tuntap *tt)
 {
@@ -4877,38 +4875,6 @@ get_adapter_index(const char *guid)
 return index;
 }
 
-static DWORD
-get_adapter_index_flexible(const char *name)  /* actual name or GUID */
-{
-struct gc_arena gc = gc_new();
-DWORD index;
-index = get_adapter_index_method_1(name);
-if (index == TUN_ADAPTER_INDEX_INVALID)
-{
-index = get_adapter_index_method_2(name);
-}
-if (index == TUN_ADAPTER_INDEX_INVALID)
-{
-const struct tap_reg *tap_reg = get_tap_reg();
-const struct panel_reg *panel_reg = get_panel_reg();
-const struct tap_reg *tr = get_adapter_by_name(name, tap_reg, 
panel_reg);
-if (tr)
-{
-index = get_adapter_index_method_1(tr->guid);
-if (index == TUN_ADAPTER_INDEX_INVALID)
-{
-index = get_adapter_index_method_2(tr->guid);
-}
-}
-}
-if (index == TUN_ADAPTER_INDEX_INVALID)
-{
-msg(M_INFO, "NOTE: could not get adapter index for name/GUID '%s'", 
name);
-}
-gc_free();
-return index;
-}
-
 /*
  * Return a string representing a PIP_ADDR_STRING
  */
-- 
2.30.0.windows.2



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

[Openvpn-devel] [PATCH 5/5] iservice: Resolve MSVC C4996 warnings

2021-03-21 Thread Simon Rozman via Openvpn-devel

Lots of string functions were declared unsafe in favor of ..._s()
counterparts. However, the code already is careful about the buffer
size. Code analysis is just not smart enough (yet) to detect this.

The code was refactored to use ..._s() variants MSVC is considering as
"safe".

Signed-off-by: Simon Rozman 
---
 src/openvpnserv/automatic.c   | 8 
 src/openvpnserv/common.c  | 4 ++--
 src/openvpnserv/interactive.c | 2 +-
 src/openvpnserv/service.c | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 3f2ca345..0ba222a0 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -137,7 +137,7 @@ modext(LPTSTR dest, size_t size, LPCTSTR src, LPCTSTR 
newext)
 
 if (size > 0 && (_tcslen(src) + 1) <= size)
 {
-_tcscpy(dest, src);
+_tcscpy_s(dest, size, src);
 dest [size - 1] = TEXT('\0');
 i = _tcslen(dest);
 while (i-- > 0)
@@ -154,8 +154,8 @@ modext(LPTSTR dest, size_t size, LPCTSTR src, LPCTSTR 
newext)
 }
 if (_tcslen(dest) + _tcslen(newext) + 2 <= size)
 {
-_tcscat(dest, TEXT("."));
-_tcscat(dest, newext);
+_tcscat_s(dest, size, TEXT("."));
+_tcscat_s(dest, size, newext);
 return true;
 }
 dest[0] = TEXT('\0');
@@ -271,7 +271,7 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 BOOL more_files;
 TCHAR find_string[MAX_PATH];
 
-openvpn_sntprintf(find_string, MAX_PATH, TEXT("%s\\*"), 
settings.config_dir);
+openvpn_sntprintf(find_string, _countof(find_string), TEXT("%s\\*"), 
settings.config_dir);
 
 find_handle = FindFirstFile(find_string, _obj);
 if (find_handle == INVALID_HANDLE_VALUE)
diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index 958643df..48769be4 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -37,7 +37,7 @@ openvpn_vsntprintf(LPTSTR str, size_t size, LPCTSTR format, 
va_list arglist)
 int len = -1;
 if (size > 0)
 {
-len = _vsntprintf(str, size, format, arglist);
+len = _vsntprintf_s(str, size, _TRUNCATE, format, arglist);
 str[size - 1] = 0;
 }
 return (len >= 0 && (size_t)len < size);
@@ -311,7 +311,7 @@ get_win_sys_path(void)
 
 if (!GetSystemDirectoryW(win_sys_path, _countof(win_sys_path)))
 {
-wcsncpy(win_sys_path, default_sys_path, _countof(win_sys_path));
+wcscpy_s(win_sys_path, _countof(win_sys_path), default_sys_path);
 win_sys_path[_countof(win_sys_path) - 1] = L'\0';
 }
 
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index b073a0d5..ed83d2a3 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1067,7 +1067,7 @@ netsh_dns_cmd(const wchar_t *action, const wchar_t 
*proto, const wchar_t *if_nam
 
 if (IsWindows7OrGreater())
 {
-wcsncat(cmdline, L" validate=no", ncmdline - wcslen(cmdline) - 1);
+wcscat_s(cmdline, ncmdline, L" validate=no");
 }
 err = ExecCommand(argv0, cmdline, timeout);
 
diff --git a/src/openvpnserv/service.c b/src/openvpnserv/service.c
index 8efe25f9..8101f83d 100644
--- a/src/openvpnserv/service.c
+++ b/src/openvpnserv/service.c
@@ -61,14 +61,14 @@ CmdInstallServices()
 TCHAR path[512];
 int i, ret = _service_max;
 
-if (GetModuleFileName(NULL, path + 1, 510) == 0)
+if (GetModuleFileName(NULL, path + 1, _countof(path) - 2) == 0)
 {
 _tprintf(TEXT("Unable to install service - %s\n"), GetLastErrorText());
 return 1;
 }
 
 path[0] = TEXT('\"');
-_tcscat(path, TEXT("\""));
+_tcscat_s(path, _countof(path), TEXT("\""));
 
 svc_ctl_mgr = OpenSCManager(NULL, NULL, SC_MANAGER_CONNECT | 
SC_MANAGER_CREATE_SERVICE);
 if (svc_ctl_mgr == NULL)
-- 
2.30.0.windows.2



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

[Openvpn-devel] [PATCH v3] Implement peer-fingerprint to check fingerprint of peer certificate

2021-03-21 Thread Arne Schwabe

This option allows to pin one or more more peer certificates. It also
prepares for doing TLS authentication without a CA and just
self-signed certificates.

Patch V2: Allow peer-fingerprint to be specified multiple times
  to allow multiple peers without needing to use inline
  syntax. (e.g. on command line).

Patch V3: rebase on v3 of 1/4, reword message of verify-hash and
  peer-fingerpring incompatibility

Signed-off-by: Arne Schwabe 
---
 Changes.rst   |  9 ++-
 doc/man-sections/inline-files.rst |  4 +--
 doc/man-sections/tls-options.rst  | 22 +++-
 src/openvpn/init.c|  1 +
 src/openvpn/options.c | 42 +++
 src/openvpn/options.h |  1 +
 src/openvpn/ssl_common.h  |  1 +
 src/openvpn/ssl_verify.c  | 19 --
 8 files changed, 76 insertions(+), 23 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index d6be1050..ac32de26 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -22,13 +22,20 @@ Compatibility with OpenSSL in FIPS mode
 
 Deprecated features
 ---
-``inetd`` has been removed
+``inetd`` has been removed 
 This was a very limited and not-well-tested way to run OpenVPN, on TCP
 and TAP mode only.
 
 
 Overview of changes in 2.5
 ==
+New features in 2.5.1
+-
+Certificate pinning/verify peer fingerprint
+The ``--peer-fingerprint`` option has been introduced to give users an
+easy to use alternative to the ``tls-verify`` for matching the
+fingerprint of the peer. The option takes use a number of allowed
+SHA256 certificate fingerprints.
 
 New features
 
diff --git a/doc/man-sections/inline-files.rst 
b/doc/man-sections/inline-files.rst
index 303bb3c8..01e4a840 100644
--- a/doc/man-sections/inline-files.rst
+++ b/doc/man-sections/inline-files.rst
@@ -4,8 +4,8 @@ INLINE FILE SUPPORT
 OpenVPN allows including files in the main configuration for the ``--ca``,
 ``--cert``, ``--dh``, ``--extra-certs``, ``--key``, ``--pkcs12``,
 ``--secret``, ``--crl-verify``, ``--http-proxy-user-pass``, ``--tls-auth``,
-``--auth-gen-token-secret``, ``--tls-crypt``, ``--tls-crypt-v2`` and
-``--verify-hash`` options.
+``--auth-gen-token-secret``, ``--peer-fingerprint``, ``--tls-crypt``,
+``--tls-crypt-v2`` and ``--verify-hash`` options.
 
 Each inline file started by the line  and ended by the line
 
diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst
index d8f9800e..cfe1ec98 100644
--- a/doc/man-sections/tls-options.rst
+++ b/doc/man-sections/tls-options.rst
@@ -271,7 +271,8 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
   man-in-the-middle attack where an authorized client attempts to connect
   to another client by impersonating the server. The attack is easily
   prevented by having clients verify the server certificate using any one
-  of ``--remote-cert-tls``, ``--verify-x509-name``, or ``--tls-verify``.
+  of ``--remote-cert-tls``, ``--verify-x509-name``, ``--peer-fingerprint``
+  or ``--tls-verify``.
 
 --tls-auth args
   Add an additional layer of HMAC authentication on top of the TLS control
@@ -592,6 +593,25 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
 
 If the option is inlined, ``algo`` is always :code:`SHA256`.
 
+--peer-fingerprint args
+   Specify a SHA256 fingerprint or list of SHA256 fingerprints to verify
+   the peer certificate against. The peer certificate must match one of the
+   fingerprint or certificate verification will fail. The option can also
+   be inlined
+
+  Valid syntax:
+  ::
+
+peer-fingerprint AD:B0:95:D8:09:...
+
+  or inline:
+  ::
+
+
+
00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff
+
11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00
+
+
 --verify-x509-name args
   Accept connections only if a host's X.509 name is equal to **name.** The
   remote host must also pass all other tests of verification.
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index d234729c..731b0cf2 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2927,6 +2927,7 @@ do_init_crypto_tls(struct context *c, const unsigned int 
flags)
 to.remote_cert_eku = options->remote_cert_eku;
 to.verify_hash = options->verify_hash;
 to.verify_hash_algo = options->verify_hash_algo;
+to.verify_hash_depth = options->verify_hash_depth;
 #ifdef ENABLE_X509ALTUSERNAME
 memcpy(to.x509_username_field, options->x509_username_field, 
sizeof(to.x509_username_field));
 #else
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3b1c69ba..871f6f5c 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -8118,26 +8118,45 @@ add_option(struct options *options,
 options->extra_certs_file = p[1];
 options->extra_certs_file_inline = is_inline;
 }
-

[Openvpn-devel] [PATCH v3] Extend verify-hash to allow multiple hashes

2021-03-21 Thread Arne Schwabe

This patch introduces support for verify-hash inlining.
When inlined, this options now allows to specify multiple fingerprints,
one per line.

Since this is a new syntax, there is no backwards compatibility to take
care of, therefore we can drop support for SHA1. Inlined fingerprints
are assumed be to SHA-256 only.

Also print a warning about SHA1 hash being deprecated to verify
certificates as it is not "industry standard" anymore.

Patch v2: fix/clarify various comments, fix a few minor problems, allow
  the option to be specified multiple times and have that
  added to the list.

Patch v3: Remove leftover variable, always call
  parse_hash_fingerprint_multiline, add comments clarifying list
  appending

Signed-off-by: Arne Schwabe 
---
 doc/man-sections/inline-files.rst |   4 +-
 doc/man-sections/tls-options.rst  |  10 +++
 src/openvpn/options.c | 102 ++
 src/openvpn/options.h |  10 ++-
 src/openvpn/ssl_common.h  |   2 +-
 src/openvpn/ssl_verify.c  |  16 -
 6 files changed, 127 insertions(+), 17 deletions(-)

diff --git a/doc/man-sections/inline-files.rst 
b/doc/man-sections/inline-files.rst
index 819bd3c8..303bb3c8 100644
--- a/doc/man-sections/inline-files.rst
+++ b/doc/man-sections/inline-files.rst
@@ -4,8 +4,8 @@ INLINE FILE SUPPORT
 OpenVPN allows including files in the main configuration for the ``--ca``,
 ``--cert``, ``--dh``, ``--extra-certs``, ``--key``, ``--pkcs12``,
 ``--secret``, ``--crl-verify``, ``--http-proxy-user-pass``, ``--tls-auth``,
-``--auth-gen-token-secret``, ``--tls-crypt`` and ``--tls-crypt-v2``
-options.
+``--auth-gen-token-secret``, ``--tls-crypt``, ``--tls-crypt-v2`` and
+``--verify-hash`` options.
 
 Each inline file started by the line  and ended by the line
 
diff --git a/doc/man-sections/tls-options.rst b/doc/man-sections/tls-options.rst
index e13fb3c8..d8f9800e 100644
--- a/doc/man-sections/tls-options.rst
+++ b/doc/man-sections/tls-options.rst
@@ -582,6 +582,16 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
   The ``algo`` flag can be either :code:`SHA1` or :code:`SHA256`. If not
   provided, it defaults to :code:`SHA1`.
 
+  This option can also be inlined
+  ::
+
+
+
00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff
+
11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00:11:22:33:44:55:66:77:88:99:aa:bb:cc:dd:ee:ff:00
+
+
+If the option is inlined, ``algo`` is always :code:`SHA256`.
+
 --verify-x509-name args
   Accept connections only if a host's X.509 name is equal to **name.** The
   remote host must also pass all other tests of verification.
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 86e78b05..3b1c69ba 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1078,12 +1078,24 @@ string_substitute(const char *src, int from, int to, 
struct gc_arena *gc)
 return ret;
 }
 
-static uint8_t *
+/**
+ * Parses a hexstring and checks if the string has the correct length. Return
+ * a verify_hash_list containing the parsed hash string.
+ *
+ * @param   str String to check/parse
+ * @param   nbytes  Number of bytes expected in the hexstr (e.g. 20 for 
SHA1)
+ * @param   msglevelmessage level to use when printing warnings/errors
+ * @param   gc  The returned object will be allocated in this gc
+ */
+static struct verify_hash_list *
 parse_hash_fingerprint(const char *str, int nbytes, int msglevel, struct 
gc_arena *gc)
 {
 int i;
 const char *cp = str;
-uint8_t *ret = (uint8_t *) gc_malloc(nbytes, true, gc);
+
+struct verify_hash_list *ret;
+ALLOC_OBJ_CLEAR_GC(ret, struct verify_hash_list, gc);
+
 char term = 1;
 int byte;
 char bs[3];
@@ -1102,7 +1114,7 @@ parse_hash_fingerprint(const char *str, int nbytes, int 
msglevel, struct gc_aren
 {
 msg(msglevel, "format error in hash fingerprint hex byte: %s", 
str);
 }
-ret[i] = (uint8_t)byte;
+ret->hash[i] = (uint8_t)byte;
 term = *cp++;
 if (term != ':' && term != 0)
 {
@@ -1116,10 +1128,55 @@ parse_hash_fingerprint(const char *str, int nbytes, int 
msglevel, struct gc_aren
 if (term != 0 || i != nbytes-1)
 {
 msg(msglevel, "hash fingerprint is different length than expected (%d 
bytes): %s", nbytes, str);
+return NULL;
 }
 return ret;
 }
 
+/**
+ * Parses a string consisting of multiple lines of hexstrings and checks if 
each
+ * string has the correct length. Empty lines are ignored. Returns
+ * a linked list of (possibly) multiple verify_hash_list objects.
+ *
+ * @param   str String to check/parse
+ * @param   nbytes  Number of bytes expected in the hexstring (e.g. 20 for 
SHA1)
+ * @param   msglevelmessage level to use when printing warnings/errors
+ * @param   gc  The returned list items will be allocated in this gc
+ */
+static

Re: [Openvpn-devel] [PATCH 2/2] Implement '--compress migrate' to migrate to non-compression setup

2021-03-21 Thread Arne Schwabe

Am 20.03.21 um 14:20 schrieb David Sommerseth:
> On 19/03/2021 16:31, Arne Schwabe wrote:
>> This option allow migration to a non compression server config while
>> still retraining compatibility with client that have a compression
>> setting in their config.
>>
>> For existing setups that used to have comp-lzo no or another
>> compression setting in their configs it is a difficult to migrate to
>> a setup without compression without replacing all client configs at
>> once especially if OpenVPN 2.3 or earlier clients are in the mix that
>> do not support pushing stub-v2. Even with OpenVPN 2.4 and later clients
>> that support pushing this is not a satisfying solution as the clients
>> log occ mismatches and the "push stub-v2" needs to be in the server
>> config "forever".
>>
>> Signed-off-by: Arne Schwabe 
>> ---
>>   doc/man-sections/protocol-options.rst | 12 +++-
>>   src/openvpn/comp.h    |  1 +
>>   src/openvpn/multi.c   | 41 +
>>   src/openvpn/options.c |  6 ++
>>   src/openvpn/ssl.c | 34 ++-
>>   src/openvpn/ssl_common.h  |  1 +
>>   src/openvpn/ssl_util.c    | 43 ++
>>   src/openvpn/ssl_util.h    | 15 +
>>   tests/unit_tests/openvpn/Makefile.am  | 14 -
>>   tests/unit_tests/openvpn/test_misc.c  | 83 +++
>>   10 files changed, 245 insertions(+), 5 deletions(-)
>>   create mode 100644 tests/unit_tests/openvpn/test_misc.c
>>
> 
> This fails compiling:
> 
> ../../../src/openvpn/ssl.c: In function ‘key_method_2_write’:
> ../../../src/openvpn/ssl.c:2280:13: error: ‘multi’ undeclared (first use
> in this function)
>  if (multi->remote_usescomp && session->opt->mode == MODE_SERVER
>  ^
> ../../../src/openvpn/ssl.c:2280:13: note: each undeclared identifier is
> reported only once for each function it appears in
> make[3]: *** [Makefile:742: ssl.o] Error 1

Oh the fun of rebasing. The multi argument was added by another patch.
Can you just the small patch to review the rest of the patch?:

 static bool
-key_method_2_write(struct buffer *buf, struct tls_session *session)
+key_method_2_write(struct buffer *buf, struct tls_multi *multi, struct
tls_session *session)
 {
 struct key_state *ks = >key[KS_PRIMARY];  /* primary
key */

@@ -2856,7 +2856,7 @@ tls_process(struct tls_multi *multi,
 if (!buf->len && ((ks->state == S_START && !session->opt->server)
   || (ks->state == S_GOT_KEY &&
session->opt->server)))
 {
-if (!key_method_2_write(buf, session))
+if (!key_method_2_write(buf, multi, session))
 {
 goto error;
 }


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

Re: [Openvpn-devel] [PATCH 2/2] Implement '--compress migrate' to migrate to non-compression setup

Re: [Openvpn-devel] [PATCH v2 5/5] Deprecate the --verify-hash option

[Openvpn-devel] [PATCH applied] Re: Implement peer-fingerprint to check fingerprint of peer certificate

[Openvpn-devel] [PATCH applied] Re: Extend verify-hash to allow multiple hashes

Re: [Openvpn-devel] [PATCH v2 5/5] Deprecate the --verify-hash option

Re: [Openvpn-devel] [PATCH v2 5/5] Deprecate the --verify-hash option

[Openvpn-devel] [PATCH applied] Re: iservice: Resolve MSVC C4996 warnings

Re: [Openvpn-devel] [PATCH v2 3/5] Support fingerprint authentication without CA certificate

Re: [Openvpn-devel] [PATCH applied] Re: interactive.c: Resolve MSVC C4996 warning

[Openvpn-devel] [PATCH applied] Re: interactive.c: Resolve MSVC C4996 warning

[Openvpn-devel] [PATCH applied] Re: tun.c: Remove dead code

Re: [Openvpn-devel] [PATCH 1/5] MSVC: Disable LZ4

Re: [Openvpn-devel] [PATCH 4/5] tapctl: Resolve MSVC C4996 warnings

Re: [Openvpn-devel] [PATCH v3] Implement peer-fingerprint to check fingerprint of peer certificate

Re: [Openvpn-devel] [PATCH 3/5] interactive.c: Resolve MSVC C4996 warning

Re: [Openvpn-devel] [PATCH 4/5] tapctl: Resolve MSVC C4996 warnings

Re: [Openvpn-devel] [PATCH 4/5] tapctl: Resolve MSVC C4996 warnings

Re: [Openvpn-devel] [PATCH 2/5] tun.c: Remove dead code

Re: [Openvpn-devel] [PATCH 1/5] MSVC: Disable LZ4

Re: [Openvpn-devel] [PATCH v3] Extend verify-hash to allow multiple hashes

Re: [Openvpn-devel] [PATCH 5/5] iservice: Resolve MSVC C4996 warnings

[Openvpn-devel] [PATCH 3/5] interactive.c: Resolve MSVC C4996 warning

[Openvpn-devel] [PATCH 1/5] MSVC: Disable LZ4

[Openvpn-devel] [PATCH 4/5] tapctl: Resolve MSVC C4996 warnings

[Openvpn-devel] [PATCH 2/5] tun.c: Remove dead code

[Openvpn-devel] [PATCH 5/5] iservice: Resolve MSVC C4996 warnings

[Openvpn-devel] [PATCH v3] Implement peer-fingerprint to check fingerprint of peer certificate

[Openvpn-devel] [PATCH v3] Extend verify-hash to allow multiple hashes

Re: [Openvpn-devel] [PATCH 2/2] Implement '--compress migrate' to migrate to non-compression setup

29 matches

Site Navigation

Mail list logo

Footer information