[Openvpn-devel] [PATCH] New man page - Simple corrections

2020-06-25 Thread Richard Bonhomme
Signed-off-by: Richard Bonhomme 
---
 doc/man-sections/connection-profiles.rst | 2 +-
 doc/man-sections/examples.rst| 2 +-
 doc/man-sections/pkcs11-options.rst  | 2 +-
 doc/man-sections/plugin-options.rst  | 4 ++--
 doc/man-sections/signals.rst | 2 +-
 doc/man-sections/unsupported-options.rst | 4 ++--
 6 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/doc/man-sections/connection-profiles.rst 
b/doc/man-sections/connection-profiles.rst
index f72db56e..fd3382b2 100644
--- a/doc/man-sections/connection-profiles.rst
+++ b/doc/man-sections/connection-profiles.rst
@@ -5,5 +5,5 @@ Client configuration files may contain multiple remote servers 
which
 it will attempt to connect against.  But there are some configuration
 options which are related to specific ``--remote`` options.  For these
-use cases, connection profiles is the solution.
+use cases, connection profiles are the solution.
 
 By enacpulating the ``--remote`` option and related options within
diff --git a/doc/man-sections/examples.rst b/doc/man-sections/examples.rst
index ecc2a29f..20cc55ef 100644
--- a/doc/man-sections/examples.rst
+++ b/doc/man-sections/examples.rst
@@ -219,5 +219,5 @@ documentation how to properly configure IP forwarding, 
which is also
 persistent through system boots.
 
-If you system is configured with a firewall.  Please see your operating
+If your system is configured with a firewall.  Please see your operating
 systems guide on how to configure the firewall.  You typically want to
 allow traffic coming from and going to the tun/tap adapter OpenVPN is
diff --git a/doc/man-sections/pkcs11-options.rst 
b/doc/man-sections/pkcs11-options.rst
index 5495c187..d40ce9ea 100644
--- a/doc/man-sections/pkcs11-options.rst
+++ b/doc/man-sections/pkcs11-options.rst
@@ -53,5 +53,5 @@ PKCS#11 / SmartCard options
 
 --pkcs11-providers provider
-  Specify a RSA Security Inc. PKCS #11 Cryptographic Token Interface
+  Specify an RSA Security Inc. PKCS #11 Cryptographic Token Interface
   (Cryptoki) providers to load. This option can be used instead of
   ``--cert``, ``--key`` and ``--pkcs12``.
diff --git a/doc/man-sections/plugin-options.rst 
b/doc/man-sections/plugin-options.rst
index 75dfd5c0..20b74edd 100644
--- a/doc/man-sections/plugin-options.rst
+++ b/doc/man-sections/plugin-options.rst
@@ -3,8 +3,8 @@ Plug-in Interface Options
 
 OpenVPN can be extended by loading external plug-in modules at runtime.  These
-plug-ins must be prebuild and adhere to the OpenVPN Plug-In API.
+plug-ins must be prebuilt and adhere to the OpenVPN Plug-In API.
 
 --plugin args
-  Loads a given plug-in module.  The ``module-name`` need to be the first
+  Loads a given plug-in module.  The ``module-name`` needs to be the first
   argument, indicating the plug-in to load.  The second argument is an
   optional init string which will be passed directly to the plug-in.
diff --git a/doc/man-sections/signals.rst b/doc/man-sections/signals.rst
index fce43d84..63611b3c 100644
--- a/doc/man-sections/signals.rst
+++ b/doc/man-sections/signals.rst
@@ -21,5 +21,5 @@ SIGNALS
 when the underlying parameters of the host's network interface change
 such as when the host is a DHCP client and is assigned a new IP address.
-See ``--ipchange`` above for more information.
+See ``--ipchange`` for more information.
 
 :code:`SIGUSR2`
diff --git a/doc/man-sections/unsupported-options.rst 
b/doc/man-sections/unsupported-options.rst
index ae7c1bcc..67d3f54e 100644
--- a/doc/man-sections/unsupported-options.rst
+++ b/doc/man-sections/unsupported-options.rst
@@ -3,5 +3,5 @@ UNSUPPORTED OPTIONS
 ===
 
-Options listed in this section has been removed from OpenVPN and is no
+Options listed in this section have been removed from OpenVPN and are no
 longer supported
 
@@ -22,5 +22,5 @@ longer supported
   VPN tunnel security.
 
---no-reply
+--no-replay
   Removed in OpenVPN 2.5.  This option should not be used as it weakens the
   VPN tunnel security.
-- 
2.17.1



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


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

2020-06-25 Thread Uipko Berghuis
> From: Samuli Seppänen [mailto:sam...@openvpn.net]
> Sent: woensdag 24 juni 2020 13:10
> Talked about starting the deprecation of "--ncp-disable". The idea is that --
> ncp-disable has been mostly a debug feature and as we move forward and
> want to be able to manage VPN security more from server side, we want to
> abandon the possibility to ignore NCP.
> 
> This is tied with deprecation of --cipher for everything except p2p:
> 
> https://www.mail-archive.com/openvpn-
> de...@lists.sourceforge.net/msg20062.html
> 
> Uip will bring these topics up with syzzer a.s.a.p.

Syzzer isn't available this week so we will (most likely) not responded on this 
issue this week. I'll try to discuss it with him asap next week.



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


[Openvpn-devel] [PATCH v3] msvc: fix various level2 warnings

2020-06-25 Thread Lev Stipakov
From: Lev Stipakov 

Also set warnings level to level2 and
enable "treat warnings as errors" flag.

Signed-off-by: Lev Stipakov 
---

 v3:
  - use proper format specifier for unsigned long long
  - add comment about cast for qsort()
  - remove unused parameter in delete_route_connected_v6_net() 

 v2:
  - rebase on top of master

 src/compat/Debug.props   |  1 -
 src/openvpn/block_dns.c  |  3 ---
 src/openvpn/crypto_openssl.c |  3 ++-
 src/openvpn/init.c   |  2 +-
 src/openvpn/mtu.c|  4 ++--
 src/openvpn/openvpn.vcxproj  | 26 +-
 src/openvpn/options.c|  2 +-
 src/openvpn/route.c  |  2 +-
 src/openvpn/ssl_openssl.c|  2 +-
 src/openvpn/tun.c| 12 ++--
 10 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/src/compat/Debug.props b/src/compat/Debug.props
index e5e9f681..31bb9d91 100644
--- a/src/compat/Debug.props
+++ b/src/compat/Debug.props
@@ -15,7 +15,6 @@
   
_DEBUG;%(PreprocessorDefinitions)
   MultiThreadedDebugDLL
   EditAndContinue
-  true
 
   
   
diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c
index 889d6bb9..f4718fc2 100644
--- a/src/openvpn/block_dns.c
+++ b/src/openvpn/block_dns.c
@@ -109,9 +109,6 @@ DEFINE_GUID(
 
 static WCHAR *FIREWALL_NAME = L"OpenVPN";
 
-VOID NETIOAPI_API_
-InitializeIpInterfaceEntry(PMIB_IPINTERFACE_ROW Row);
-
 /*
  * Default msg handler does nothing
  */
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 2027d8dc..521cfca1 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -315,7 +315,8 @@ show_available_ciphers(void)
 }
 }
 
-qsort(cipher_list, num_ciphers, sizeof(*cipher_list), cipher_name_cmp);
+/* cast to non-const to prevent warning */
+qsort((EVP_CIPHER *)cipher_list, num_ciphers, sizeof(*cipher_list), 
cipher_name_cmp);
 
 for (i = 0; i < num_ciphers; i++)
 {
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 2c8db68d..2af25c1a 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1812,7 +1812,7 @@ do_open_tun(struct context *c)
 #ifdef _WIN32
 /* store (hide) interactive service handle in tuntap_options */
 c->c1.tuntap->options.msg_channel = c->options.msg_channel;
-msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned int) 
c->options.msg_channel);
+msg(D_ROUTE, "interactive service msg_channel=%" PRIu64, (unsigned long 
long) c->options.msg_channel);
 #endif
 
 /* allocate route list structure */
diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c
index 04868cd6..8178ff06 100644
--- a/src/openvpn/mtu.c
+++ b/src/openvpn/mtu.c
@@ -175,7 +175,7 @@ set_mtu_discover_type(int sd, int mtu_type, sa_family_t 
proto_af)
 #if defined(HAVE_SETSOCKOPT) && defined(IP_MTU_DISCOVER)
 case AF_INET:
 if (setsockopt
-(sd, IPPROTO_IP, IP_MTU_DISCOVER, _type, 
sizeof(mtu_type)))
+(sd, IPPROTO_IP, IP_MTU_DISCOVER, (const char 
*)_type, sizeof(mtu_type)))
 {
 msg(M_ERR, "Error setting IP_MTU_DISCOVER type=%d on 
TCP/UDP socket",
 mtu_type);
@@ -186,7 +186,7 @@ set_mtu_discover_type(int sd, int mtu_type, sa_family_t 
proto_af)
 #if defined(HAVE_SETSOCKOPT) && defined(IPV6_MTU_DISCOVER)
 case AF_INET6:
 if (setsockopt
-(sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, _type, 
sizeof(mtu_type)))
+(sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, (const char 
*)_type, sizeof(mtu_type)))
 {
 msg(M_ERR, "Error setting IPV6_MTU_DISCOVER type=%d on 
TCP6/UDP6 socket",
 mtu_type);
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index 53ac5482..c34733ea 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -28,23 +28,23 @@
   
 Application
 true
-Unicode
+NotSet
 v142
   
   
 Application
 true
-Unicode
+NotSet
 v142
   
   
 Application
-Unicode
+NotSet
 v142
   
   
 Application
-Unicode
+NotSet
 v142
   
   
@@ -74,7 +74,9 @@
 
   
..\compat;$(TAP_WINDOWS_HOME)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
   
_CONSOLE;%(PreprocessorDefinitions)
-  
UNICODE;%(UndefinePreprocessorDefinitions)
+  
%(UndefinePreprocessorDefinitions)
+  Level2
+  true
 
 
 
@@ -87,7 +89,9 @@
 
   
..\compat;$(TAP_WINDOWS_HOME)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
   
_CONSOLE;%(PreprocessorDefinitions)
-  
UNICODE;%(UndefinePreprocessorDefinitions)
+  
%(UndefinePreprocessorDefinitions)
+  Level2
+  true
 
 
 
@@ -100,7 +104,9 @@
 
   

Re: [Openvpn-devel] [PATCH v2] msvc: fix various level2 warnings

2020-06-25 Thread Lev Stipakov
Hi,

> > -qsort(cipher_list, num_ciphers, sizeof(*cipher_list), cipher_name_cmp);
> > +qsort((void *)cipher_list, num_ciphers, sizeof(*cipher_list), 
> > cipher_name_cmp);
>
> I find this one questionable.  Qsort wants a void *, but every pointer type
> should be convertible to void *, without a warning?

Yes, but this is about "const". VS shows "'function': different
'const' qualifiers".
I'll go with this one:

/* cast to non-const to prevent warning */
qsort((EVP_CIPHER *)cipher_list, num_ciphers,
sizeof(*cipher_list), cipher_name_cmp);

> > -msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned int) 
> > c->options.msg_channel);
> > +msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned long 
> > long) c->options.msg_channel);
>
> This gets a NAK.  "%u" is "unsigned int", not (never!) (unsigned long long) -

Good point, thanks. The warning was about casting from HANDLE to
unsigned int. I'll change
format specifier to "%llu".

> > -(sd, IPPROTO_IP, IP_MTU_DISCOVER, _type, 
> > sizeof(mtu_type)))
> > +(sd, IPPROTO_IP, IP_MTU_DISCOVER, (const char 
> > *)_type, sizeof(mtu_type)))
>
> I find this questionable as well.  Setsockopt() on Unix wants a
> "void *", not a "const char *" - and this cast suggest to the reader
> that we're dealing with "something character based", which it is not.

That's the case in Windows:

https://docs.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-setsockopt

> int setsockopt( SOCKET s, int level, int optname, const char *optval, int 
> optlen );

> >  show_available_tls_ciphers_list(const char *cipher_list,
> >  const char *tls_cert_profile,
> > -bool tls13);
> > +const bool tls13);
>
> I'm not sure why this is needed or beneficial?

Otherwise there is warning "formal parameter 3 different from declaration".

I noticed that mbedtls implementation doesn't have "const", so I'll
just remove it from openssl implementation and won't change header.

> >  void
> > -delete_route_connected_v6_net(struct tuntap *tt,
> > +delete_route_connected_v6_net(const struct tuntap *tt,
> >const struct env_set *es)
>
> I might be convinced that this is a useful change, but if it's needed(!)
> for delete_route_connected_v6_net(), the "cost" should be added to
> add_route_connected_v6_net() as well.

I looked closer and in fact second argument of delete_route_connected_v6_net()
is always NULL, so I'll remove that parameter.


-- 
-Lev


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


Re: [Openvpn-devel] [PATCH v2] msvc: fix various level2 warnings

2020-06-25 Thread Arne Schwabe


>> @@ -1812,7 +1812,7 @@ do_open_tun(struct context *c)
>>  #ifdef _WIN32
>>  /* store (hide) interactive service handle in tuntap_options */
>>  c->c1.tuntap->options.msg_channel = c->options.msg_channel;
>> -msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned int) 
>> c->options.msg_channel);
>> +msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned long long) 
>> c->options.msg_channel);
> 
> This gets a NAK.  "%u" is "unsigned int", not (never!) (unsigned long long) -
> if you want that, because the data type is so big, use %llu or %I64u
> (Windows special, according to common.h).

Or use the standardised ones. E.g. PRIu64 for uint64_t. We already use
that auth-token.c.

See also https://en.cppreference.com/w/cpp/header/cinttypes (or another
reference).


> 
>> index 04868cd6..8178ff06 100644
>> --- a/src/openvpn/mtu.c
>> +++ b/src/openvpn/mtu.c
>> @@ -175,7 +175,7 @@ set_mtu_discover_type(int sd, int mtu_type, sa_family_t 
>> proto_af)
>>  #if defined(HAVE_SETSOCKOPT) && defined(IP_MTU_DISCOVER)
>>  case AF_INET:
>>  if (setsockopt
>> -(sd, IPPROTO_IP, IP_MTU_DISCOVER, _type, 
>> sizeof(mtu_type)))
>> +(sd, IPPROTO_IP, IP_MTU_DISCOVER, (const char 
>> *)_type, sizeof(mtu_type)))
> 
> I find this questionable as well.  Setsockopt() on Unix wants a 
> "void *", not a "const char *" - and this cast suggest to the reader
> that we're dealing with "something character based", which it is not.
> 
>>  #if defined(HAVE_SETSOCKOPT) && defined(IPV6_MTU_DISCOVER)
>>  case AF_INET6:
>>  if (setsockopt
>> -(sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, _type, 
>> sizeof(mtu_type)))
>> +(sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, (const char 
>> *)_type, sizeof(mtu_type)))
> 
> Same here.
> 
>>  show_available_tls_ciphers_list(const char *cipher_list,
>>  const char *tls_cert_profile,
>> -bool tls13);
>> +const bool tls13);
> 
> I'm not sure why this is needed or beneficial?  This const stuff is
> good for pointers ("do not modify the pointed-to area") but for
> call-by-value arguments, the compiler can figure this out itself.

Clang tidy actually warns against this for integral types saying that
they are passed by value always in C++. Not sure if this also applies to C.

Arne



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


Re: [Openvpn-devel] [PATCH v2] msvc: fix various level2 warnings

2020-06-25 Thread Gert Doering
Hi,

On Thu, Jun 25, 2020 at 11:37:45AM +0300, Lev Stipakov wrote:
> @@ -315,7 +315,7 @@ show_available_ciphers(void)
>  }
>  }
>  
> -qsort(cipher_list, num_ciphers, sizeof(*cipher_list), cipher_name_cmp);
> +qsort((void *)cipher_list, num_ciphers, sizeof(*cipher_list), 
> cipher_name_cmp);

I find this one questionable.  Qsort wants a void *, but every pointer type
should be convertible to void *, without a warning?

(Adding casts for things that are explicitly always allowed and well-defined 
is something I do not like very much - it just makes "more code to read")


> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 2c8db68d..107bb4c9 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -1812,7 +1812,7 @@ do_open_tun(struct context *c)
>  #ifdef _WIN32
>  /* store (hide) interactive service handle in tuntap_options */
>  c->c1.tuntap->options.msg_channel = c->options.msg_channel;
> -msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned int) 
> c->options.msg_channel);
> +msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned long long) 
> c->options.msg_channel);

This gets a NAK.  "%u" is "unsigned int", not (never!) (unsigned long long) -
if you want that, because the data type is so big, use %llu or %I64u
(Windows special, according to common.h).


> index 04868cd6..8178ff06 100644
> --- a/src/openvpn/mtu.c
> +++ b/src/openvpn/mtu.c
> @@ -175,7 +175,7 @@ set_mtu_discover_type(int sd, int mtu_type, sa_family_t 
> proto_af)
>  #if defined(HAVE_SETSOCKOPT) && defined(IP_MTU_DISCOVER)
>  case AF_INET:
>  if (setsockopt
> -(sd, IPPROTO_IP, IP_MTU_DISCOVER, _type, 
> sizeof(mtu_type)))
> +(sd, IPPROTO_IP, IP_MTU_DISCOVER, (const char 
> *)_type, sizeof(mtu_type)))

I find this questionable as well.  Setsockopt() on Unix wants a 
"void *", not a "const char *" - and this cast suggest to the reader
that we're dealing with "something character based", which it is not.

>  #if defined(HAVE_SETSOCKOPT) && defined(IPV6_MTU_DISCOVER)
>  case AF_INET6:
>  if (setsockopt
> -(sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, _type, 
> sizeof(mtu_type)))
> +(sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, (const char 
> *)_type, sizeof(mtu_type)))

Same here.

>  show_available_tls_ciphers_list(const char *cipher_list,
>  const char *tls_cert_profile,
> -bool tls13);
> +const bool tls13);

I'm not sure why this is needed or beneficial?  This const stuff is
good for pointers ("do not modify the pointed-to area") but for
call-by-value arguments, the compiler can figure this out itself.

> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -233,10 +233,11 @@ do_set_mtu_service(const struct tuntap *tt, const short 
> family, const int mtu)
>  sizeof(set_mtu_message_t),
>  0
>  },
> -.iface = {.index = tt->adapter_index,.name = tt->actual_name },
> +.iface = {.index = tt->adapter_index},

Sure of that?  As in "are you sure the service does not need the name,
never"?

>  .mtu = mtu,
>  .family = family
>  };
> +strncpynt(mtu_msg.iface.name, tt->actual_name, 
> sizeof(mtu_msg.iface.name));

Well spotted.  This is interesting, the assignment ".name = tt->actual_name"
shouldn't be legal (as it's basically a hidden strcpy(), not a integral
type or structure assignment), but mingw is not complaining and the 
resulting binary works.  But this definitely is a necessary bugfix.


>  if (!send_msg_iservice(pipe, _msg, sizeof(mtu_msg), , "Set_mtu"))
>  {
> @@ -889,7 +890,7 @@ add_route_connected_v6_net(struct tuntap *tt,
>  }
>  
>  void
> -delete_route_connected_v6_net(struct tuntap *tt,
> +delete_route_connected_v6_net(const struct tuntap *tt,
>const struct env_set *es)

I might be convinced that this is a useful change, but if it's needed(!)
for delete_route_connected_v6_net(), the "cost" should be added to 
add_route_connected_v6_net() as well.

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 v2] msvc: fix various level2 warnings

2020-06-25 Thread Lev Stipakov
From: Lev Stipakov 

Also set warnings level to level2 and
enable "treat warnings as errors" flag.

Signed-off-by: Lev Stipakov 
---
 src/compat/Debug.props   |  1 -
 src/openvpn/block_dns.c  |  3 ---
 src/openvpn/crypto_openssl.c |  2 +-
 src/openvpn/init.c   |  2 +-
 src/openvpn/mtu.c|  4 ++--
 src/openvpn/openvpn.vcxproj  | 26 +-
 src/openvpn/options.c|  2 +-
 src/openvpn/route.c  |  2 +-
 src/openvpn/ssl_backend.h|  2 +-
 src/openvpn/tun.c|  5 +++--
 10 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/src/compat/Debug.props b/src/compat/Debug.props
index e5e9f681..31bb9d91 100644
--- a/src/compat/Debug.props
+++ b/src/compat/Debug.props
@@ -15,7 +15,6 @@
   
_DEBUG;%(PreprocessorDefinitions)
   MultiThreadedDebugDLL
   EditAndContinue
-  true
 
   
   
diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c
index 889d6bb9..f4718fc2 100644
--- a/src/openvpn/block_dns.c
+++ b/src/openvpn/block_dns.c
@@ -109,9 +109,6 @@ DEFINE_GUID(
 
 static WCHAR *FIREWALL_NAME = L"OpenVPN";
 
-VOID NETIOAPI_API_
-InitializeIpInterfaceEntry(PMIB_IPINTERFACE_ROW Row);
-
 /*
  * Default msg handler does nothing
  */
diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 2027d8dc..8805e25e 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -315,7 +315,7 @@ show_available_ciphers(void)
 }
 }
 
-qsort(cipher_list, num_ciphers, sizeof(*cipher_list), cipher_name_cmp);
+qsort((void *)cipher_list, num_ciphers, sizeof(*cipher_list), 
cipher_name_cmp);
 
 for (i = 0; i < num_ciphers; i++)
 {
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 2c8db68d..107bb4c9 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -1812,7 +1812,7 @@ do_open_tun(struct context *c)
 #ifdef _WIN32
 /* store (hide) interactive service handle in tuntap_options */
 c->c1.tuntap->options.msg_channel = c->options.msg_channel;
-msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned int) 
c->options.msg_channel);
+msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned long long) 
c->options.msg_channel);
 #endif
 
 /* allocate route list structure */
diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c
index 04868cd6..8178ff06 100644
--- a/src/openvpn/mtu.c
+++ b/src/openvpn/mtu.c
@@ -175,7 +175,7 @@ set_mtu_discover_type(int sd, int mtu_type, sa_family_t 
proto_af)
 #if defined(HAVE_SETSOCKOPT) && defined(IP_MTU_DISCOVER)
 case AF_INET:
 if (setsockopt
-(sd, IPPROTO_IP, IP_MTU_DISCOVER, _type, 
sizeof(mtu_type)))
+(sd, IPPROTO_IP, IP_MTU_DISCOVER, (const char 
*)_type, sizeof(mtu_type)))
 {
 msg(M_ERR, "Error setting IP_MTU_DISCOVER type=%d on 
TCP/UDP socket",
 mtu_type);
@@ -186,7 +186,7 @@ set_mtu_discover_type(int sd, int mtu_type, sa_family_t 
proto_af)
 #if defined(HAVE_SETSOCKOPT) && defined(IPV6_MTU_DISCOVER)
 case AF_INET6:
 if (setsockopt
-(sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, _type, 
sizeof(mtu_type)))
+(sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, (const char 
*)_type, sizeof(mtu_type)))
 {
 msg(M_ERR, "Error setting IPV6_MTU_DISCOVER type=%d on 
TCP6/UDP6 socket",
 mtu_type);
diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj
index 53ac5482..c34733ea 100644
--- a/src/openvpn/openvpn.vcxproj
+++ b/src/openvpn/openvpn.vcxproj
@@ -28,23 +28,23 @@
   
 Application
 true
-Unicode
+NotSet
 v142
   
   
 Application
 true
-Unicode
+NotSet
 v142
   
   
 Application
-Unicode
+NotSet
 v142
   
   
 Application
-Unicode
+NotSet
 v142
   
   
@@ -74,7 +74,9 @@
 
   
..\compat;$(TAP_WINDOWS_HOME)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
   
_CONSOLE;%(PreprocessorDefinitions)
-  
UNICODE;%(UndefinePreprocessorDefinitions)
+  
%(UndefinePreprocessorDefinitions)
+  Level2
+  true
 
 
 
@@ -87,7 +89,9 @@
 
   
..\compat;$(TAP_WINDOWS_HOME)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
   
_CONSOLE;%(PreprocessorDefinitions)
-  
UNICODE;%(UndefinePreprocessorDefinitions)
+  
%(UndefinePreprocessorDefinitions)
+  Level2
+  true
 
 
 
@@ -100,7 +104,9 @@
 
   
..\compat;$(TAP_WINDOWS_HOME)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
   
_CONSOLE;%(PreprocessorDefinitions)
-  
UNICODE;%(UndefinePreprocessorDefinitions)
+  
%(UndefinePreprocessorDefinitions)
+  Level2
+  

Re: [Openvpn-devel] [PATCH v4 3/3] Implement tls-groups option to specify eliptic curves/groups

2020-06-25 Thread Antonio Quartulli
Hi,

on my GitLab CI build test, the compilation failed with the following
message, while compiling against openssl-1.1:

 /usr/bin/ld: ssl_openssl.o: in function `tls_ctx_set_tls_groups':
/builds/ordex986/openvpn/src/openvpn/ssl_openssl.c:611: undefined
reference to `SSL_CTX_set1_groups'
collect2: error: ld returned 1 exit status


Any clue?

On 23/06/2020 11:21, Antonio Quartulli wrote:
> Hi,
> 
> On 22/06/2020 16:02, Arne Schwabe wrote:
> 
> [CUT]
> 
>> @@ -343,6 +348,42 @@ tls_ctx_set_cert_profile(struct tls_root_ctx *ctx, 
>> const char *profile)
>>  }
>>  }
>>  
>> +void
>> +tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
>> +{
>> +ASSERT(ctx);
>> +struct gc_arena gc = gc_new();
>> +
>> +/* Get number of groups and allocate an array in ctx */
>> +int groups_count = get_num_elements(groups, ':');
>> +ALLOC_ARRAY_CLEAR(ctx->groups, mbedtls_ecp_group_id, groups_count + 1)
>> +
>> +/* Parse allowed ciphers, getting IDs */
>> +int i = 0;
>> +char *tmp_groups = string_alloc(groups, );
>> +
>> +const char *token;
>> +while ((token = strsep(_groups, ":")))
>> +{
>> +const mbedtls_ecp_curve_info *ci =
>> +mbedtls_ecp_curve_info_from_name(token);
>> +if (!ci)
>> +{
>> +msg(M_WARN, "Warning unknown curve/group specified: %s", token);
>> +}
>> +else
>> +{
>> +ctx->groups[i] = ci->grp_id;
>> +i++;
>> +}
>> +token = strsep(_groups, ":");
> 
> The line above should be removed, otherwise end up doing strsep() twice
> in a row.
> 
> 
>> +}
>> +ctx->groups[i] = MBEDTLS_ECP_DP_NONE;
>> +
>> +gc_free();
>> +}
>> +
>> +
>>  void
>>  tls_ctx_check_cert_time(const struct tls_root_ctx *ctx)
>>  {
>> @@ -1043,6 +1084,11 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl,
>>  mbedtls_ssl_conf_ciphersuites(ks_ssl->ssl_config, 
>> ssl_ctx->allowed_ciphers);
>>  }
>>  
>> +if (ssl_ctx->groups)
>> +{
>> +mbedtls_ssl_conf_curves(ks_ssl->ssl_config, ssl_ctx->groups);
>> +}
>> +
>>  /* Disable record splitting (for now).  OpenVPN assumes records are sent
>>   * unfragmented, and changing that will require thorough review and
>>   * testing.  Since OpenVPN is not susceptible to BEAST, we can just
>> diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h
>> index 92381f1a..0525134f 100644
>> --- a/src/openvpn/ssl_mbedtls.h
>> +++ b/src/openvpn/ssl_mbedtls.h
>> @@ -105,6 +105,7 @@ struct tls_root_ctx {
>>  #endif
>>  struct external_context external_key; /**< External key context */
>>  int *allowed_ciphers;   /**< List of allowed ciphers for this 
>> connection */
>> +mbedtls_ecp_group_id *groups; /**< List of allowed groups for this 
>> connection */
>>  mbedtls_x509_crt_profile cert_profile; /**< Allowed certificate types */
>>  };
>>  
>> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
>> index a489053b..da7d252a 100644
>> --- a/src/openvpn/ssl_openssl.c
>> +++ b/src/openvpn/ssl_openssl.c
>> @@ -565,6 +565,57 @@ tls_ctx_set_cert_profile(struct tls_root_ctx *ctx, 
>> const char *profile)
>>  #endif /* ifdef HAVE_SSL_CTX_SET_SECURITY_LEVEL */
>>  }
>>  
>> +void
>> +tls_ctx_set_tls_groups(struct tls_root_ctx *ctx, const char *groups)
>> +{
>> +ASSERT(ctx);
>> +struct gc_arena gc = gc_new();
>> +/* This method could be as easy as
>> + *  SSL_CTX_set1_groups_list(ctx->ctx, groups)
>> + * but OpenSSL does not like the name secp256r1 for prime256v1
>> + * This is one of the important curves.
>> + * To support the same name for OpenSSL and mbedTLS, we do
>> + * this dance.
>> + */
>> +
>> +int groups_count = get_num_elements(groups, ':');
>> +
>> +int *glist;
>> +/* Allocate an array for them */
>> +ALLOC_ARRAY_CLEAR_GC(glist, int, groups_count, );
>> +
>> +/* Parse allowed ciphers, getting IDs */
>> +int glistlen = 0;
>> +char *tmp_groups = string_alloc(groups, );
>> +
>> +const char *token;
>> +while ((token = strsep(_groups, ":")))
>> +{
>> +if (streq(token, "secp256r1"))
>> +{
>> +token = "prime256v1";
>> +}
>> +int nid = OBJ_sn2nid(token);
>> +
>> +if (nid == 0)
> 
> From a style perspective, I think it looks better to add an empty line
> *before* "int nid =..." and remove the one after.
> 
> This way we also follow the same pattern:
> 
> x = a();
> if (x ..)
> {
> }
> 
> as other parts of the code.
> 
> 
> 
> 
> The rest looks good to me.
> 
> Regards,
> 

-- 
Antonio Quartulli


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