[Openvpn-devel] [PATCH] New man page - Simple corrections
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)
> 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
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
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
>> @@ -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
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
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
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