Re: [Openvpn-devel] [PATCH] Fix various spelling mistakes

2019-01-22 Thread Jonathan Tooker
Looks like I missed that and a few others! I fixed some more spelling
errors across other things. Follow up patch/commit below. If I just need
to re-make the original patch let me know.

From: Jonathan Tooker 
Date: Tue, 22 Jan 2019 23:10:33 -0600
Subject: [PATCH] Another set of spelling fixes

---
 COPYING  |   6 +-
 ChangeLog| 140 +++
 Changes.rst  |   6 +-
 INSTALL  |   2 +-
 TODO.IPv6|   6 +-
 configure.ac |   2 +-
 m4/pkg.m4|   2 +-
 src/openvpn/console.h|   2 +-
 src/openvpn/ssl_verify_backend.h |   2 +-
 9 files changed, 84 insertions(+), 84 deletions(-)

diff --git a/COPYING b/COPYING
index 9c21c177..54cc9f24 100644
--- a/COPYING
+++ b/COPYING
@@ -157,7 +157,7 @@ OpenSSL License:
  * The implementation was written so as to conform with Netscapes SSL.
  * 
  * This library is free for commercial and non-commercial use as long as
- * the following conditions are aheared to.  The following conditions
+ * the following conditions are adhered to.  The following conditions
  * apply to all code found in this distribution, be it the RC4, RSA,
  * lhash, DES, etc., code; not just the SSL code.  The SSL documentation
  * included with this distribution is covered by the same copyright terms
@@ -182,7 +182,7 @@ OpenSSL License:
  *must display the following acknowledgement:
  *"This product includes cryptographic software written by
  * Eric Young (e...@cryptsoft.com)"
- *The word 'cryptographic' can be left out if the rouines from the library
+ *The word 'cryptographic' can be left out if the routines from the library
  *being used are not cryptographic related :-).
  * 4. If you include any Windows specific code (or a derivative thereof) from 
  *the apps directory (application code) you must include an 
acknowledgement:
@@ -200,7 +200,7 @@ OpenSSL License:
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  * 
- * The licence and distribution terms for any publically available version or
+ * The licence and distribution terms for any publicly available version or
  * derivative of this code cannot be changed.  i.e. this code cannot simply be
  * copied and put under another distribution licence
  * [including the GNU Public Licence.]
diff --git a/ChangeLog b/ChangeLog
index 1c06c7f1..077d7063 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -128,7 +128,7 @@ Gert Doering (10):
 
 Heiko Hund (4):
   put argv_* functions into own file, add unit tests
-  Remove unused and unecessary argv interfaces
+  Remove unused and unnecessary argv interfaces
   remove unused system_str from struct argv
   Factor out %sc handling from argv_printf()
 
@@ -230,7 +230,7 @@ Arne Schwabe (100):
   Split the PROTO_UDP_xx options into AF_INET/AF_INET6 and 
PROTO_TCP/PROTO_UDP part.
   Fix two instances of asserting AF_INET
   Fix assertion when SIGUSR1 is received while getaddrinfo is successful
-  Split link_socket_init_phase1 and link_socket_init_phase2 into smaller 
more managable/readable functions. No functional changes
+  Split link_socket_init_phase1 and link_socket_init_phase2 into smaller 
more manageable/readable functions. No functional changes
   Change proto_remote() function to return a constant string
   Remove the ip-remote-hint option.
   change the type of 'remote' to addrinfo*, and rename to 'remote_list'.
@@ -252,7 +252,7 @@ Arne Schwabe (100):
   Add gateway and device to android control messages
   Clean up of socket code.
   Fix assert when using port-share
-  Work around Solaris getaddrinfo() returing ai_protocol=0
+  Work around Solaris getaddrinfo() returning ai_protocol=0
   Fix man page and OSCP script: tls_serial_{n} is decimal
   Remove ENABLE_BUFFER_LIST
   Fix server routes not working in topology subnet with --server [v3]
@@ -335,7 +335,7 @@ David Sommerseth (44):
   autoconf: Fix typo
   t_client.sh: Check for fping/fping6 availability
   t_client.sh: Write errors to stderr and document requirements
-  t_client.sh: Add prepare/cleanup possibilties for each test case
+  t_client.sh: Add prepare/cleanup possibilities for each test case
   Fix file checks when --chroot is being used
   Adjusted autotools files to build more cleanly on newer 
autoconf/automake versions
   Improve error reporting on file access to --client-config-dir and 
--ccd-exclusive
@@ -382,7 +382,7 @@ Dorian Harmans (1):
   Add CHACHA20-POLY1305 ciphersuite IANA name translations.
 
 Felix Janda (1):
-  Use OPENVPN_ETH_P_* so that  is unecessary
+  Use OPENVPN_ETH_P_* so that  is unnecessary
 
 Fish (1):
   Add lz4 support to MSVC.
@@ -531,7 +531,7 @@ Holger Kummert (1):
   Del ipv6 addr on close of linux tun interface
 
 Hubert Kario 

Re: [Openvpn-devel] [PATCH] Fix various spelling mistakes

2019-01-22 Thread Simon Matter via Openvpn-devel
Hi,

> diff --git a/src/openvpn/console.h b/src/openvpn/console.h
> index 0ffd6683..62beacae 100644
> --- a/src/openvpn/console.h
> +++ b/src/openvpn/console.h
> @@ -33,9 +33,9 @@
>   */
> struct _query_user {
>  char *prompt; /**< Prompt to present to the user */
> -size_t prompt_len;/**< Lenght of the prompt string */
> +size_t prompt_len;/**< Length of the prompt string */
>  char *response;   /**< The user's response */
> -size_t response_len;  /**< Lenght the of the user reposone */
> +size_t response_len;  /**< Length the of the user reposone */
 
response

Regards,
Simon



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


[Openvpn-devel] [PATCH] Fix various spelling mistakes

2019-01-22 Thread Jonathan Tooker
Fork @ github: https://github.com/JDTX/openvpn 
(76ab12606155f51aaaf376a46f4a52a459af105c)

From: Jonathan Tooker 
Date: Tue, 22 Jan 2019 18:27:39 -0600
Subject: [PATCH] Fix various spelling mistakes

Fix spelling mistakes in code/headers/manpages/etc.
---
distro/rpm/openvpn.init.d.rhel|  2 +-
distro/rpm/openvpn.init.d.suse|  4 ++--
doc/keying-material-exporter.txt  |  2 +-
doc/openvpn.8 | 14 +++---
sample/sample-config-files/client.conf|  2 +-
sample/sample-keys/openssl.cnf|  4 ++--
src/openvpn/buffer.c  |  2 +-
src/openvpn/console.h |  6 +++---
src/openvpn/crypto.h  |  2 +-
src/openvpn/crypto_backend.h  |  2 +-
src/openvpn/fragment.c|  2 +-
src/openvpn/init.c| 18 +-
src/openvpn/mss.c |  2 +-
src/openvpn/options.c | 14 +++---
src/openvpn/packet_id.h   |  2 +-
src/openvpn/route.c   |  2 +-
src/openvpn/run_command.c |  4 ++--
src/openvpn/socket.c  | 12 ++--
src/openvpn/socket.h  |  2 +-
src/openvpn/ssl.c |  2 +-
src/openvpn/tun.c |  8 
src/openvpn/win32.c   |  2 +-
src/openvpn/win32.h   |  2 +-
src/openvpnmsica/msica_op.h   |  2 +-
src/plugins/auth-pam/README.auth-pam  |  4 ++--
src/plugins/auth-pam/utils.h  |  6 +++---
src/tapctl/tap.c  |  2 +-
tests/t_client.sh.in  |  2 +-
tests/unit_tests/openvpn/test_tls_crypt.c |  2 +-
29 files changed, 65 insertions(+), 65 deletions(-)

diff --git a/distro/rpm/openvpn.init.d.rhel b/distro/rpm/openvpn.init.d.rhel
index bfde2216..04125ca6 100755
--- a/distro/rpm/openvpn.init.d.rhel
+++ b/distro/rpm/openvpn.init.d.rhel
@@ -113,7 +113,7 @@ case "$1" in

# From a security perspective, I think it makes
   # sense to remove this, and have users who need
-  # it explictly enable in their --up scripts or
+ # it explicitly enable in their --up scripts or
   # firewall setups.

#echo 1 > /proc/sys/net/ipv4/ip_forward
diff --git a/distro/rpm/openvpn.init.d.suse b/distro/rpm/openvpn.init.d.suse
index 270024e8..1b4bcf06 100644
--- a/distro/rpm/openvpn.init.d.suse
+++ b/distro/rpm/openvpn.init.d.suse
@@ -72,7 +72,7 @@
#- removed sourcing "network"
#- removed network checking. it seemed not to work 
with SuSE.
#- added sourcing "rc.status", comments and 
"rc_reset" command
-#- removed "succes; echo" and "failure; echo" lines
+#   - removed "success; echo" and "failure; echo" lines
#- added "rc_status" lines at the end of each 
section
#- changed "service" to "/etc/init.d/" in "In 
addition to start/stop"
#  section above.
@@ -126,7 +126,7 @@ case "$1" in

# From a security perspective, I think it makes
   # sense to remove this, and have users who need
-  # it explictly enable in their --up scripts or
+ # it explicitly enable in their --up scripts or
   # firewall setups.

#echo 1 > /proc/sys/net/ipv4/ip_forward
diff --git a/doc/keying-material-exporter.txt b/doc/keying-material-exporter.txt
index 4187d828..4c1addc8 100644
--- a/doc/keying-material-exporter.txt
+++ b/doc/keying-material-exporter.txt
@@ -48,7 +48,7 @@ to application layer using well-defined mechanism.
[DerivedAAABindingKey][DerivedAAABindingKey]
   [AuthenticateBindingKeys]
Client   ---> Server
- [Confidental channel]
+ [Confidential channel]

 
 TLS Message flow for a full handshake
diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 7abcaf1e..e5c0626a 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -696,7 +696,7 @@ are used.

 If the
.B ipv6only
-keyword is present OpenVPN will bind only to IPv6 (as oposed
+keyword is present OpenVPN will bind only to IPv6 (as opposed
to IPv6 and IPv4) when a IPv6 socket is opened.

 .\"*
@@ -2221,7 +2221,7 @@ that
is parsed on the command line even though
the daemonization point occurs later.  If one of the
.B \-\-log
-options is present, it will supercede syslog
+options is present, it will supersede syslog
redirection.

 The optional
@@ -2332,7 +2332,7 @@ If
already exists it will be truncated.
This option takes effect
immediately when it 

[Openvpn-devel] [PATCH applied] Re: Rename tls_crypt_v2_read_keyfile into generic pem_read_key_file

2019-01-22 Thread Gert Doering
Acked-by: Gert Doering 

Explanation makes sense, code looks good (just moving, except for the
error messages which change to print "*pem_name" instead of static
"tls-crypt-v2" always).

Your patch has been applied to the master branch.

commit 784ad902438a6c70f1b9e4f545ac2bbb4230a048
Author: Arne Schwabe
Date:   Tue Jan 22 16:03:28 2019 +0100

 Rename tls_crypt_v2_read_keyfile into generic pem_read_key_file

 Acked-by: Gert Doering 
 Message-Id: <20190122150333.1061-1-a...@rfc2549.org>
 URL: 
https://www.mail-archive.com/search?l=mid=20190122150333.1061-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: Detect missing TAP driver and bail out gracefully

2019-01-22 Thread Gert Doering
Acked-by: Gert Doering 

Looks reasonable.  Plus, correct error message :-)

Your patch has been applied to the master branch.

commit 91bc1212b4b79ac9e2cbf6d345db5df716c42a5b
Author: Simon Rozman
Date:   Wed Dec 19 21:26:11 2018 +0100

 Detect missing TAP driver and bail out gracefully

 Acked-by: Gert Doering 
 Message-Id: <20181219202611.2144-4-si...@rozman.si>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18038.html
 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 3/4] options: add support for --transport-plugin

2019-01-22 Thread Arne Schwabe
Am 30.12.18 um 12:29 schrieb Antonio Quartulli:
> From: Robin Tarsiger 
> 
> Add a new config option to allow the user to specify a transport plugin
> implementing the new API. This plugin can be used to manipulate traffic
> in any way, as designed by the plugin developer.
> 
> The fondamental advantage of this plugin is that the core codebase does
typo fundamental.


> +.B \-\-transport-plugin module-pathname [connection-args]
  \- missing here.

> +Use the loaded plugin module identified by
> +.B module-pathname
> +to provide a transport layer for the connection. The
> +.B module-pathname
> +must be exactly equivalent to a pathname supplied to a
> +.B \-\-plugin
> +option. The same transport plugin may be used for
> +multiple connections, in which case the
> +.B \-\-plugin
> +option which loads it should only occur once. However,
> +only one transport plugin may be specified per
> +connection.

Specify if this includes the .dll/.so suffix or not.


> +If
> +.B connection-args
I think there is a \- missing here.

> +are present, these arguments are passed to the transport
> +plugin when establishing this connection specifically; this
> +is distinct from any per-plugin arguments which may have
> +been specified using the
> +.B \-\-plugin
> +option. Documentation for possible
> +.B connection-args
> +may be provided along with the plugin in use.
> +

> +#ifdef ENABLE_PLUGIN
> +/*
> + * "proto indirect" may not be specified directly without a
> + * transport-plugin, and vice versa.
> + */
> +if (ce->proto == PROTO_INDIRECT && !ce->transport_plugin_argv)
> +{
> +msg(M_USAGE, "--proto indirect may not be used without a 
> transport-plugin line");
> +}
> +
> +if (ce->transport_plugin_argv && ce->proto != PROTO_INDIRECT)
> +{
> +msg(M_USAGE, "--transport-plugin must be used with --proto 
> indirect");
> +}
> +#endif

Why are not doing that implicitly when transport-plugin is specified.
Any particular reason or just to get a more consistent way of specifying it?



Arne



signature.asc
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 2/4] socket: introduce INDIRECT transport protocol abstraction

2019-01-22 Thread Arne Schwabe
Am 30.12.18 um 12:28 schrieb Antonio Quartulli:
> From: Robin Tarsiger 
> 
> This new transport protocol is used to tell the core code that traffic
> should not be directly processed, but should rather be rerouted to a
> transport plugin. It is basically an abstraction as it does not say tell
> the code how to process the data, but simply forces its redirection to
> the external code.
> 
> Signed-off-by: Robin Tarsiger 
> [anto...@openvpn.net: refactored commits, restyled code]
> ---
>  src/openvpn/forward.c   |   5 ++
>  src/openvpn/socket.c| 146 ++--
>  src/openvpn/socket.h|  70 +++
>  src/openvpn/transport.h |   5 ++
>  4 files changed, 222 insertions(+), 4 deletions(-)
> 
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index 0a90fff0..a7092c7e 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -2150,6 +2150,11 @@ io_wait_dowork(struct context *c, const unsigned int 
> flags)
>  {
>  int i;
>  c->c2.event_set_status = 0;
> +#ifdef ENABLE_PLUGIN
> +c->c2.event_set_status |=
> +(socket_indirect_pump(c->c2.link_socket, esr, ) & 
> 3)

The & 3 looks like some defined should be used instead.



>  if (addr->ai_protocol == IPPROTO_UDP || addr->ai_socktype == SOCK_DGRAM)
>  {
>  sock->sd = create_socket_udp(addr, sock->sockflags);
> @@ -2279,7 +2320,11 @@ link_socket_init_phase2(struct link_socket *sock,
>  }
>  
>  /* If socket has not already been created create it now */
> -if (sock->sd == SOCKET_UNDEFINED)
> +if (sock->sd == SOCKET_UNDEFINED
> +#ifdef ENABLE_PLUGIN
> +&& !sock->indirect
> +#endif
> +)
>  {
>  /* If we have no --remote and have still not figured out the
>   * protocol family to use we will use the first of the bind */
> @@ -2300,7 +2345,11 @@ link_socket_init_phase2(struct link_socket *sock,
>  }
>  
>  /* Socket still undefined, give a warning and abort connection */
> -if (sock->sd == SOCKET_UNDEFINED)
> +if (sock->sd == SOCKET_UNDEFINED
> +#ifdef ENABLE_PLUGIN
> +&& !sock->indirect
> +#endif
> +)

Better use the inline funtcion proto_is_indirect (or similar
sock_is_indirect) that always returns false here and ifdef in header
than to add the ifdefs inline in the code.


>  bool
> @@ -3167,6 +3236,10 @@ proto_is_net(int proto)
>  bool
>  proto_is_dgram(int proto)
>  {
> +if (proto_is_indirect(proto))
> +{
> +return true;
> +}
>  return proto_is_udp(proto);
>  }

I think here need to be a good explaination why indirect is dgram but
not udp and also proto_is_dgram and proto_is_udp need to get some
comment to explain their difference in usage as this is now different.

>  
> @@ -3301,6 +3374,18 @@ proto_remote(int proto, bool remote)
>  return "TCPv4_CLIENT";
>  }
>  
> +#ifdef ENABLE_PLUGIN
> +if (proto == PROTO_INDIRECT)
> +{
> +/* FIXME: the string reported here should match the actual transport
> + * protocol being used, however in this function we have no 
> knowledge of
> + * what protocol is exactly being used by the transport-plugin.
> + * Therefore we simply return INDIRECT for now.
> + */
> +return "INDIRECT";
> +}
> +#endif

From reading the code this function is only used in creating the string
in options_string which needs to match between peers. As the plugin
emulates UDP/DGRAM behaviour I think we should instead return here the
same as a real UDP protocol.

>  
> +#ifdef ENABLE_PLUGIN
> +
> +int link_socket_read_indirect(struct link_socket *sock,
> +  struct buffer *buf,
> +  struct link_socket_actual *from);
> +
> +int link_socket_write_indirect(struct link_socket *sock,
> +   struct buffer *buf,
> +   struct link_socket_actual *from);
> +
> +bool proto_is_indirect(int proto);
> +

This prototype definition looks a bit weird here. I see not reason why
the real proto_is_indirect cannot be here

> +#else  /* ifdef ENABLE_PLUGIN */
> +
> +static int
> +link_socket_read_indirect(struct link_socket *sock,
> +
> +static int
> +link_socket_write_indirect(struct link_socket *sock,

> +static bool
> +proto_is_indirect(int proto)

I think functions implemented in the header should have the inline
keyword. At least the rest of the header files do it that way.


Arne



signature.asc
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 1/4] transport: introduce tranport API plugin codebase

2019-01-22 Thread Arne Schwabe
>  
> +/*
> + * FUNCTION: openvpn_plugin_get_vtab_v1
> + *

It would be nice if we also use the docutils style to document the new
functions in this file rather than is this different documentation style.

> + * This is only used for TRANSPORT plugins presently.  It is called to
> + * retrieve a vtable structure to be used for binding virtual sockets
> + * which use the transport provided by the plugin. The selector is an
> + * OPENVPN_VTAB constant. *size_out must be set to the size of the
> + * structure returned.

A reference to openvpn_transport_bind_vtab1 might be a good idea here.

> +/* On Windows, platform-native events to wait on are provided to OpenVPN 
> core as

I think for multi line comments we use the following style

/*
 * first line of text
 * last line of text
 */

but not sure. Might want to double check that style.

Rest of the file look fine but I also would like to see docstyle comments.
> +#endif
> diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
> index 197e62ba..41cd90aa 100644
> --- a/src/openvpn/Makefile.am
> +++ b/src/openvpn/Makefile.am
> @@ -119,6 +119,7 @@ openvpn_SOURCES = \
>   status.c status.h \
>   syshead.h \
>   tls_crypt.c tls_crypt.h \
> + transport.c transport.h \
>   tun.c tun.h \
>   win32.h win32.c \
>   cryptoapi.h cryptoapi.c

> +openvpn_transport_socket_t
> +transport_bind(const struct plugin_list *plugins,
> +   const char **transport_plugin_argv, sa_family_t ai_family,
> +   struct addrinfo *bind_addresses)
> +{
> +openvpn_plugin_handle_t handle;
> +openvpn_transport_args_t args;
> +openvpn_transport_socket_t indirect;
> +struct openvpn_transport_bind_vtab1 *vtab;
> +struct addrinfo *cur = NULL;
> +struct openvpn_sockaddr zero;
> +
> +if (!transport_prepare(plugins, transport_plugin_argv, , ,
> +   ))
> +{
> +msg(M_FATAL, "INDIRECT: Socket bind failed: provider plugin not 
> found");
> +}
> +
> +/* Partially replicates the functionality of socket_bind. No 
> bind_ipv6_only
> + * or other such options, presently.
> + */
> +if (bind_addresses)
> +{
> +for (cur = bind_addresses; cur; cur = cur->ai_next)
> +{
> +if (cur->ai_family == ai_family)
> +{
> +break;
> +}
> +}
> +
> +if (!cur)
> +{
> +msg(M_FATAL, "INDIRECT: Socket bind failed: Addr to bind has no 
> %s record",
> +addr_family_name(ai_family));
> +}
> +}
> +
> +if (cur)
> +{
> +indirect = vtab->bind(handle, args, cur->ai_addr, cur->ai_addrlen);
> +}
> +else if (ai_family == AF_UNSPEC)
> +{
> +msg(M_ERR, "INDIRECT: cannot bind with unspecified address family");
> +}
> +else
> +{
> +memset(, 0, sizeof(zero));
> +zero.addr.sa.sa_family = ai_family;
> +addr_zero_host();
> +indirect = vtab->bind(handle, args, , 
> af_addr_size(ai_family));
> +}
> +
> +if (!indirect)
> +{
> +msg(M_ERR, "INDIRECT: Socket bind failed");
> +}
> +
> +if (vtab->freeargs)
> +{
> +vtab->freeargs(args);
> +}
> +
> +return indirect;
> +}
> +
> +struct encapsulated_event_set
> +{
> +struct openvpn_transport_event_set_handle handle;
> +struct event_set *real;
> +};
> +
> +#if EVENT_READ == OPENVPN_TRANSPORT_EVENT_READ \
> +&& EVENT_WRITE == OPENVPN_TRANSPORT_EVENT_WRITE
> +#define TRANSPORT_EVENT_BITS_IDENTICAL 1
> +#else
> +#define TRANSPORT_EVENT_BITS_IDENTICAL 0
> +#endif

A small comment when this is identical like Win32 vs posix-y would be nice.


> +
> +unsigned
> +transport_pump(openvpn_transport_socket_t indirect,
> +   struct event_set_return *esr, int *esrlen)
> +{


The name 'pump' is confusing, especially without comment. Why not
transport_update_event? Something that tells what the function actually
does.

Arne




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


[Openvpn-devel] [PATCH applied] Re: Fix tls-auth/crypt in connection blocks with --persist-key

2019-01-22 Thread Gert Doering
Your patch has been applied to the master branch.

(I had a look at the patch as well, and second the ACK :) )

commit dcfc51457789d8a62ff8bd266dd3a3bf0a0c9763
Author: Steffan Karger
Date:   Sat Jan 19 11:34:00 2019 +0100

 Fix tls-auth/crypt in connection blocks with --persist-key

 Signed-off-by: Steffan Karger 
 Acked-by: Arne Schwabe 
 Message-Id: <20190119103400.12887-1-stef...@karger.me>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18123.html
 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: crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Gert Doering
Your patch has been applied to the master branch.

commit eb1fed3f3bb817332183672dd1ca665ece83d6a8
Author: Lev Stipakov
Date:   Tue Jan 22 15:41:03 2019 +0200

 crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer

 Signed-off-by: Lev Stipakov 
 Acked-by: Arne Schwabe 
 Message-Id: <1548164463-13366-1-git-send-email-lstipa...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18141.html
 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: White-list pull-filter and script-security in interactive service

2019-01-22 Thread Gert Doering
Acked-by: Gert Doering 

"Because it makes sense and moves toward making OpenVPN on Windows more
robust (pull-filter route-method) and secure (script-security)".  Code
change is simple enough :-)

Your patch has been applied to the master and release/2.4 branch
(security enhancement).

commit 0d94d433438f239ff7cf0749f765a503c698f5e8 (master)
commit b8190ecb33f8949f1b881c1cd240e8c1ea4fe144 (release/2.4)
Author: Selva Nair
Date:   Tue Jan 22 10:50:32 2019 -0500

 White-list pull-filter and script-security in interactive service

 Signed-off-by: Selva Nair 
 Acked-by: Gert Doering 
 Message-Id: <1548172232-11268-1-git-send-email-selva.n...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18154.html
 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] White-list pull-filter and script-security in interactive service

2019-01-22 Thread selva . nair
From: Selva Nair 

This allows the Windows GUI to use these options on the command
line without triggering user authorization errors.

Useful for
(i) ignoring certain pushed options such as "route-method" which
could otherwise bypass the interactive service
(ii) enforcing a safer script-security setting from the GUI

See also:
https://github.com/OpenVPN/openvpn-gui/issues/235#issuecomment-456142928

Signed-off-by: Selva Nair 
---
 src/openvpnserv/validate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/openvpnserv/validate.c b/src/openvpnserv/validate.c
index 9e1d7d2..9b01770 100644
--- a/src/openvpnserv/validate.c
+++ b/src/openvpnserv/validate.c
@@ -44,6 +44,8 @@ static const WCHAR *white_list[] =
 L"setenv",
 L"service",
 L"verb",
+L"pull-filter",
+L"script-security",
 
 NULL/* last value */
 };
-- 
2.6.2



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


[Openvpn-devel] [PATCH applied] Re: test_tls_crypt.c: fix global-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Gert Doering
Your patch has been applied to the master branch.

commit a3fd78d48616ab21908b116d5ce785986893e02d
Author: Lev Stipakov
Date:   Tue Jan 22 15:34:20 2019 +0200

 test_tls_crypt.c: fix global-buffer-overflow found by AddressSanitizer

 Signed-off-by: Lev Stipakov 
 Acked-by: Arne Schwabe 
 Message-Id: <1548164060-13144-1-git-send-email-lstipa...@gmail.com>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18140.html
 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 v3 0/7] introduce networking API and add netlink support for Linux

2019-01-22 Thread Arne Schwabe
Am 19.12.18 um 06:01 schrieb Antonio Quartulli:
> From a high level description of this patchset, please refer to
> "[PATCH 0/4] add netlink support for Linux" sent to the mailing list on
> Apr, 20th 2018.
>

This patch set seem to be missing the commit

implement platform generic networking API

from the v1 of the patch set, so I will review the v1 instead again as
it looks it is still identical in your git tree.

Arne



signature.asc
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] Fix tls-auth/crypt in connection blocks with --persist-key

2019-01-22 Thread Arne Schwabe
Am 19.01.19 um 11:34 schrieb Steffan Karger:
> If --persist-key was used, we would always try to pre-load the 'global'
> tls-auth/crypt file. That would result in using the wrong key (leading
> to a failed connection) or en error is there was to 'global' key:
> 
>   Sat Jan 19 11:09:01 2019 Cannot pre-load tls-auth keyfile ((null))
>   Sat Jan 19 11:09:01 2019 Exiting due to fatal error
> 
> Fix that by loading loading the key from the current connection entry.
> 

Acked-By: Arne Schabe 

This also changes the logic to be similar with the other logic used in
the function. The bug is pretty obvious by just looking at the code.

Arne



signature.asc
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] Fix tls-auth/crypt in connection blocks with --persist-key

2019-01-22 Thread Arne Schwabe
Am 19.01.19 um 11:30 schrieb Steffan Karger:
> If --persist-key was used, we would always try to pre-load the 'global'
> tls-auth/crypt file. That would result in using the wrong key (leading
> to a failed connection) or en error is there was to 'global' key:
> 
>   Sat Jan 19 11:09:01 2019 Cannot pre-load tls-auth keyfile ((null))
>   Sat Jan 19 11:09:01 2019 Exiting due to fatal error
> 
> Fix that by loading loading the key from the current connection entry.

Acked-By: Arne Schabe 
This also changes the logic to be similar with the other logic used in
the function.

Arne



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


[Openvpn-devel] [PATCH v2 4/6] Implement a permanent session id in auth-token

2019-01-22 Thread Arne Schwabe
From: Arne Schwabe 

This allows an external authentication method
(e.g. management interface) to track the connection and distinguish a
reconnection from multiple connections.

Addtionally this now also checks to workaround a problem with
OpenVPN 3 core that sometimes uses a username hint from the config
instead of using an empty username if the token would be valid
with an empty username. Accepting such token can only be explicitly
done if the auth keyword to auth-gen-token is present.

Patch V2: Add Empty variants to work around behaviour in openvpn 3
---
 doc/openvpn.8|  26 ++-
 src/openvpn/auth_token.c | 145 ---
 src/openvpn/auth_token.h |  15 +++-
 src/openvpn/init.c   |   1 +
 src/openvpn/manage.c |   4 +-
 src/openvpn/options.c|  14 +++-
 src/openvpn/options.h|   3 +-
 src/openvpn/ssl_common.h |  14 +++-
 src/openvpn/ssl_verify.c |  71 ---
 9 files changed, 251 insertions(+), 42 deletions(-)

diff --git a/doc/openvpn.8 b/doc/openvpn.8
index b1924898..4e540d3a 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -3710,7 +3710,7 @@ For a sample script that performs PAM authentication, see
 in the OpenVPN source distribution.
 .\"*
 .TP
-.B \-\-auth\-gen\-token [lifetime]
+.B \-\-auth\-gen\-token [lifetime] [auth]
 After successful user/password authentication, the OpenVPN
 server will with this option generate a temporary
 authentication token and push that to client.  On the following
@@ -3733,6 +3733,30 @@ This feature is useful for environments which is 
configured
 to use One Time Passwords (OTP) as part of the user/password
 authentications and that authentication mechanism does not
 implement any auth\-token support.
+
+When the auth keyword is present the normal authentication
+method will be called even if auth-token succeeds. This allows
+the normal to still check the validity of the account and do other
+checks. In this mode the environment will have a session_id variable
+that hold the session id from auth-gen-token. Also a environment
+variable session_state is present. This variable tells whether the
+auth-token has succeeded or not. It can have the following values:
+
+ - Initial:No token from client.
+ - Authenticated:  Token is valid and not expired
+ - Expired:Token is valid but has expired
+ - Invalid Token is invalid (failed HMAC or wrong length)
+ - AuthenticatedEmptyUser/ExpiredEmptyUser
+   The token is not valid with the username send from the client
+   but would be valid (or expired) if we assume an empty username was used
+   instead. These two cases are a workaround for behaviour in OpenVPN3. If
+   this workaround is not needed these two cases should be handled in the
+   same way as Invalid.
+
+.B Warning:
+Failure to properly handle Invalid/Expired auth-token with the auth
+option in use will lead to authentication bypass.
+
 .\"*
 .TP
 .B \-\-auth\-gen\-token\-secret [file]
diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index dc80456c..12bb724a 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -18,9 +18,13 @@
 
 const char *auth_token_pem_name = "OpenVPN auth-token server key";
 
+#define AUTH_TOKEN_SESSION_ID_LEN 12
+#if AUTH_TOKEN_SESSION_ID_LEN % 3
+#error AUTH_TOKEN_SESSION_ID_LEN needs to be multiple a 3
+#endif
 
 /* Size of the data of the token (not b64 encoded and without prefix) */
-#define TOKEN_DATA_LEN (2 * sizeof(int64_t) + 32)
+#define TOKEN_DATA_LEN (2 * sizeof(int64_t) + AUTH_TOKEN_SESSION_ID_LEN + 32)
 
 static struct key_type
 auth_token_kt(void)
@@ -42,6 +46,85 @@ auth_token_kt(void)
 }
 
 
+void
+add_session_token_env(struct tls_session *session, struct tls_multi *multi,
+  const struct user_pass *up)
+{
+if (!multi->opt.auth_token_generate)
+{
+return;
+}
+
+
+const char *state;
+
+if (!is_auth_token(up->password))
+{
+state = "Initial";
+}
+else if (multi->auth_token_state_flags & AUTH_TOKEN_HMAC_OK)
+{
+switch (multi->auth_token_state_flags & 
(AUTH_TOKEN_VALID_EMPTYUSER|AUTH_TOKEN_EXPIRED))
+{
+case 0:
+state = "Authenticated";
+break;
+
+case AUTH_TOKEN_EXPIRED:
+state = "Expired";
+break;
+
+case AUTH_TOKEN_VALID_EMPTYUSER:
+state = "AuthenticatedEmptyUser";
+break;
+
+case AUTH_TOKEN_VALID_EMPTYUSER | AUTH_TOKEN_EXPIRED:
+state = "ExpiredEmptyUser";
+break;
+
+default:
+/* Silence compiler warning, all four possible combinations 
are covered */
+ASSERT(0);
+}
+}
+else
+{
+state = "Invalid";
+}
+
+

[Openvpn-devel] [PATCH v2 3/6] Rewrite auth-token-gen to be based on HMAC based tokens

2019-01-22 Thread Arne Schwabe
The previous auth-token implementation had a serious problem, especially when
paired with an unpatched OpenVPN client that keeps trying the auth-token
(commit e61b401a).

The auth-token-gen implementation forgot the auth-token on reconnect, this
lead to reconnect with auth-token never working.

This new implementation implements the auth-token in a stateles variant. By
using HMAC to sign the auth-token the server can verify if a token has been
authenticated and by checking the embedded timestamp in the token it can
also verify that the auth-token is still valid.

Patch V2: cleaned up code, use refactored read_pem_key_file function
---
 doc/openvpn.8|  27 
 src/openvpn/Makefile.am  |   1 +
 src/openvpn/auth_token.c | 260 +++
 src/openvpn/auth_token.h | 116 +
 src/openvpn/init.c   |  34 -
 src/openvpn/openvpn.h|   1 +
 src/openvpn/options.c|  24 +++-
 src/openvpn/options.h|   4 +
 src/openvpn/push.c   |  70 +--
 src/openvpn/push.h   |   8 ++
 src/openvpn/ssl.c|   7 +-
 src/openvpn/ssl_common.h |  36 +++---
 src/openvpn/ssl_verify.c | 182 ---
 13 files changed, 635 insertions(+), 135 deletions(-)
 create mode 100644 src/openvpn/auth_token.c
 create mode 100644 src/openvpn/auth_token.h

diff --git a/doc/openvpn.8 b/doc/openvpn.8
index 7abcaf1e..b1924898 100644
--- a/doc/openvpn.8
+++ b/doc/openvpn.8
@@ -3720,6 +3720,9 @@ the token authentication internally and it will NOT do any
 additional authentications against configured external
 user/password authentication mechanisms.
 
+The tokens implemented by this mechanism include a initial timestamp
+and a renew timestamp and are secured by HMAC.
+
 The
 .B lifetime
 argument defines how long the generated token is valid.  The
@@ -3732,6 +3735,29 @@ authentications and that authentication mechanism does 
not
 implement any auth\-token support.
 .\"*
 .TP
+.B \-\-auth\-gen\-token\-secret [file]
+Specifies a file that hold a secret for the HMAC used in
+.B \-\-auth\-gen\-token
+If not present OpenVPN will generate a random secret on startup. This file
+should be used if auth-token should valid after restarting a server or if
+client should be able to roam between multiple OpenVPN server with their
+auth\-token.
+
+.\"*
+.TP
+.B \-\-auth\-gen\-token\-secret\-genkey
+When used together with the
+.B \-\-auth\-gen\-token\-secret
+option, this option will generate a new secret that can be used
+with
+.B \-\-auth\-gen\-token\-secret
+
+.B Note:
+this file should be kept secret to the server as anyone
+that access to this file will be to generate auth tokens
+that the OpenVPN server will accept as valid.
+.\"*
+.TP
 .B \-\-opt\-verify
 Clients that connect with options that are incompatible
 with those of the server will be disconnected.
@@ -6973,6 +6999,7 @@ X509_1_C=KG
 OpenVPN allows including files in the main configuration for the
 .B \-\-ca, \-\-cert, \-\-dh, \-\-extra\-certs, \-\-key, \-\-pkcs12, \-\-secret,
 .B \-\-crl\-verify, \-\-http\-proxy\-user\-pass, \-\-tls\-auth,
+.B \-\-auth\-gen\-token\-secret
 .B \-\-tls\-crypt,
 and
 .B \-\-tls\-crypt-v2
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index 197e62ba..78f94762 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -39,6 +39,7 @@ sbin_PROGRAMS = openvpn
 
 openvpn_SOURCES = \
argv.c argv.h \
+   auth_token.c auth_token.h \
base64.c base64.h \
basic.h \
buffer.c buffer.h \
diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
new file mode 100644
index ..dc80456c
--- /dev/null
+++ b/src/openvpn/auth_token.c
@@ -0,0 +1,260 @@
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#elif defined(_MSC_VER)
+#include "config-msvc.h"
+#endif
+
+#include "syshead.h"
+
+#include "base64.h"
+#include "buffer.h"
+#include "crypto.h"
+#include "openvpn.h"
+#include "ssl_common.h"
+#include "auth_token.h"
+#include "push.h"
+#include "integer.h"
+#include "ssl.h"
+
+const char *auth_token_pem_name = "OpenVPN auth-token server key";
+
+
+/* Size of the data of the token (not b64 encoded and without prefix) */
+#define TOKEN_DATA_LEN (2 * sizeof(int64_t) + 32)
+
+static struct key_type
+auth_token_kt(void)
+{
+struct key_type kt;
+/* We do not encrypt our session tokens */
+kt.cipher = NULL;
+kt.digest = md_kt_get("SHA256");
+
+if (!kt.digest)
+{
+msg(M_WARN, "ERROR: --tls-crypt requires HMAC-SHA-256 support.");
+return (struct key_type) { 0 };
+}
+
+kt.hmac_length = md_kt_size(kt.digest);
+
+return kt;
+}
+
+
+void
+auth_token_write_server_key_file(const char *filename)
+{
+write_pem_key_file(filename, auth_token_pem_name);
+}
+
+void
+auth_token_init_secret(struct key_ctx *key_ctx, const 

[Openvpn-devel] [PATCH v2 2/6] Allow pem_read_key_file to generate a random key

2019-01-22 Thread Arne Schwabe
From: Arne Schwabe 

This is useful for features that can use either a persistent
or an ephemeral key.
---
 src/openvpn/crypto.c| 23 ---
 src/openvpn/crypto.h|  4 +++-
 src/openvpn/tls_crypt.c |  5 +++--
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index ff9dbfdc..68a28dee 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1885,13 +1885,30 @@ cleanup:
 
 bool
 read_pem_key_file(struct buffer *key, const char *pem_name,
-  const char *key_file, const char *key_inline)
+  const char *key_file, const char *key_inline,
+  bool allow_random)
 {
 bool ret = false;
 struct buffer key_pem = { 0 };
 struct gc_arena gc = gc_new();
 
-if (strcmp(key_file, INLINE_FILE_TAG))
+
+if (allow_random && key_file == NULL)
+{
+msg(M_INFO, "Using random %s.", pem_name);
+uint8_t rand[BCAP(key)];
+if (!rand_bytes(rand, BCAP(key)))
+{
+msg(M_WARN, "ERROR: could not generate random key");
+}
+else
+{
+buf_write(key, rand, BCAP(key));
+ret = true;
+}
+goto cleanup;
+}
+else if (strcmp(key_file, INLINE_FILE_TAG))
 {
 key_pem = buffer_read_from_file(key_file, );
 if (!buf_valid(_pem))
@@ -1914,7 +1931,7 @@ read_pem_key_file(struct buffer *key, const char 
*pem_name,
 
 ret = true;
 cleanup:
-if (strcmp(key_file, INLINE_FILE_TAG))
+if (key_file && strcmp(key_file, INLINE_FILE_TAG))
 {
 buf_clear(_pem);
 }
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 09f7bb25..dfbfb38b 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -436,11 +436,13 @@ write_pem_key_file(const char *filename, const char 
*pem_name);
  * @param pem_name  the name used in the pem encoding start/end lines
  * @param key_file  name of the file to read
  * @param key_inlinea string holding the data in case of an inline key
+ * @param allow_random  allow generating a random key if no file is provided
  * @return  true if reading into key was successful
  */
 bool
 read_pem_key_file(struct buffer *key, const char *pem_name,
-  const char *key_file, const char *key_inline);
+  const char *key_file, const char *key_inline,
+  bool allow_random);
 
 /* Minimum length of the nonce used by the PRNG */
 #define NONCE_SECRET_LEN_MIN 16
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index d6a82252..baa193c3 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -301,7 +301,7 @@ tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct 
buffer *wkc_buf,
  + TLS_CRYPT_V2_MAX_WKC_LEN);
 
 if (!read_pem_key_file(_key, tls_crypt_v2_cli_pem_name,
-   key_file, key_inline))
+   key_file, key_inline, false))
 {
 msg(M_FATAL, "ERROR: invalid tls-crypt-v2 client key format");
 }
@@ -326,8 +326,9 @@ tls_crypt_v2_init_server_key(struct key_ctx *key_ctx, bool 
encrypt,
 struct buffer srv_key_buf;
 
 buf_set_write(_key_buf, (void *)_key, sizeof(srv_key));
+
 if (!read_pem_key_file(_key_buf, tls_crypt_v2_srv_pem_name,
-   key_file, key_inline))
+   key_file, key_inline, false))
 {
 msg(M_FATAL, "ERROR: invalid tls-crypt-v2 server key format");
 }
-- 
2.20.1



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


[Openvpn-devel] [PATCH v2 5/6] Sent indication that a session is expired to clients

2019-01-22 Thread Arne Schwabe
From: Arne Schwabe 

This allows OpenVPN 3 core to fall back to the original authentication
method.

This commit changes man_def_auth_set_client_reason to
auth_set_client_reason since it now used in more contexts.

Also remove a FIXME about client_reason not being freed, as it is freed
in tls_multi_free with auth_set_client_reason(multi, NULL);
---
 src/openvpn/auth_token.c |  3 +++
 src/openvpn/ssl.c|  6 ++
 src/openvpn/ssl_common.h | 10 +-
 src/openvpn/ssl_verify.c |  8 
 src/openvpn/ssl_verify.h | 15 ++-
 5 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/src/openvpn/auth_token.c b/src/openvpn/auth_token.c
index 12bb724a..74a76b72 100644
--- a/src/openvpn/auth_token.c
+++ b/src/openvpn/auth_token.c
@@ -15,6 +15,7 @@
 #include "push.h"
 #include "integer.h"
 #include "ssl.h"
+#include "ssl_verify.h"
 
 const char *auth_token_pem_name = "OpenVPN auth-token server key";
 
@@ -356,6 +357,8 @@ verify_auth_token(struct user_pass *up, struct tls_multi 
*multi,
 
 if (ret & AUTH_TOKEN_EXPIRED)
 {
+/* Tell client that the session token is expired */
+auth_set_client_reason(multi, "SESSION: token expired");
 msg(M_INFO, "--auth-token-gen: auth-token from client expired");
 }
 return ret;
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index fb557e37..52af514f 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1349,11 +1349,9 @@ tls_multi_free(struct tls_multi *multi, bool clear)
 
 ASSERT(multi);
 
-#ifdef MANAGEMENT_DEF_AUTH
-man_def_auth_set_client_reason(multi, NULL);
-
-#endif
 #if P2MP_SERVER
+auth_set_client_reason(multi, NULL);
+
 free(multi->peer_info);
 #endif
 
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index ccc01f15..be5a208f 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -525,16 +525,16 @@ struct tls_multi
 struct cert_hash_set *locked_cert_hash_set;
 
 #ifdef ENABLE_DEF_AUTH
-/*
- * An error message to send to client on AUTH_FAILED
- */
-char *client_reason;
-
 /* Time of last call to tls_authentication_status */
 time_t tas_last;
 #endif
 
 #if P2MP_SERVER
+/*
+ * An error message to send to client on AUTH_FAILED
+ */
+char *client_reason;
+
 /*
  * A multi-line string of general-purpose info received from peer
  * over control channel.
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 2ff99853..550dd653 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -803,9 +803,8 @@ cleanup:
 #define ACF_FAILED3
 #endif
 
-#ifdef MANAGEMENT_DEF_AUTH
 void
-man_def_auth_set_client_reason(struct tls_multi *multi, const char 
*client_reason)
+auth_set_client_reason(struct tls_multi* multi, const char* client_reason)
 {
 if (multi->client_reason)
 {
@@ -814,11 +813,12 @@ man_def_auth_set_client_reason(struct tls_multi *multi, 
const char *client_reaso
 }
 if (client_reason && strlen(client_reason))
 {
-/* FIXME: Last alloc will never be freed */
 multi->client_reason = string_alloc(client_reason, NULL);
 }
 }
 
+#ifdef MANAGEMENT_DEF_AUTH
+
 static inline unsigned int
 man_def_auth_test(const struct key_state *ks)
 {
@@ -1022,7 +1022,7 @@ tls_authenticate_key(struct tls_multi *multi, const 
unsigned int mda_key_id, con
 if (multi)
 {
 int i;
-man_def_auth_set_client_reason(multi, client_reason);
+auth_set_client_reason(multi, client_reason);
 for (i = 0; i < KEY_SCAN_SIZE; ++i)
 {
 struct key_state *ks = multi->key_scan[i];
diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h
index 64f27efb..c54b89a6 100644
--- a/src/openvpn/ssl_verify.h
+++ b/src/openvpn/ssl_verify.h
@@ -224,18 +224,23 @@ struct x509_track
 #ifdef MANAGEMENT_DEF_AUTH
 bool tls_authenticate_key(struct tls_multi *multi, const unsigned int 
mda_key_id, const bool auth, const char *client_reason);
 
-void man_def_auth_set_client_reason(struct tls_multi *multi, const char 
*client_reason);
+#endif
 
+#ifdef P2MP_SERVER
+/**
+ * Sets the reason why authentication of a client failed. This be will send to 
the client
+ * when the AUTH_FAILED message is sent
+ * An example would be "SESSION: Token expired"
+ * @param multi The multi tls struct
+ * @param client_reason The string to send to the client as part of 
AUTH_FAILED
+ */
+void auth_set_client_reason(struct tls_multi* multi, const char* 
client_reason);
 #endif
 
 static inline const char *
 tls_client_reason(struct tls_multi *multi)
 {
-#ifdef ENABLE_DEF_AUTH
 return multi->client_reason;
-#else
-return NULL;
-#endif
 }
 
 /** Remove any X509_ env variables from env_set es */
-- 
2.20.1



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


[Openvpn-devel] [PATCH v2 1/6] Rename tls_crypt_v2_read_keyfile into generic pem_read_key_file

2019-01-22 Thread Arne Schwabe
From: Arne Schwabe 

The function is fairly generic and to avoid duplicating the same
functionality move the function to crypto.c and change fixed string to
be the same as the pem_name parameter.
---
 src/openvpn/crypto.c| 39 ++
 src/openvpn/crypto.h| 12 +++
 src/openvpn/ssl.h   |  1 -
 src/openvpn/tls_crypt.c | 47 -
 4 files changed, 55 insertions(+), 44 deletions(-)

diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c
index 19136799..ff9dbfdc 100644
--- a/src/openvpn/crypto.c
+++ b/src/openvpn/crypto.c
@@ -1882,3 +1882,42 @@ cleanup:
 gc_free();
 return;
 }
+
+bool
+read_pem_key_file(struct buffer *key, const char *pem_name,
+  const char *key_file, const char *key_inline)
+{
+bool ret = false;
+struct buffer key_pem = { 0 };
+struct gc_arena gc = gc_new();
+
+if (strcmp(key_file, INLINE_FILE_TAG))
+{
+key_pem = buffer_read_from_file(key_file, );
+if (!buf_valid(_pem))
+{
+msg(M_WARN, "ERROR: failed to read %s file (%s)",
+pem_name, key_file);
+goto cleanup;
+}
+}
+else
+{
+buf_set_read(_pem, (const void *)key_inline, strlen(key_inline) + 
1);
+}
+
+if (!crypto_pem_decode(pem_name, key, _pem))
+{
+msg(M_WARN, "ERROR: %s pem decode failed", pem_name);
+goto cleanup;
+}
+
+ret = true;
+cleanup:
+if (strcmp(key_file, INLINE_FILE_TAG))
+{
+buf_clear(_pem);
+}
+gc_free();
+return ret;
+}
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index c0574ff6..09f7bb25 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -430,6 +430,18 @@ unsigned int crypto_max_overhead(void);
 void
 write_pem_key_file(const char *filename, const char *pem_name);
 
+/**
+ * Read key material from a PEM encoded files into the key structure
+ * @param key   the key structure that will hold the key material
+ * @param pem_name  the name used in the pem encoding start/end lines
+ * @param key_file  name of the file to read
+ * @param key_inlinea string holding the data in case of an inline key
+ * @return  true if reading into key was successful
+ */
+bool
+read_pem_key_file(struct buffer *key, const char *pem_name,
+  const char *key_file, const char *key_inline);
+
 /* Minimum length of the nonce used by the PRNG */
 #define NONCE_SECRET_LEN_MIN 16
 
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index eafb235e..660e9eb4 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -634,5 +634,4 @@ void
 show_available_tls_ciphers(const char *cipher_list,
const char *cipher_list_tls13,
const char *tls_cert_profile);
-
 #endif /* ifndef OPENVPN_SSL_H */
diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c
index eeac794b..d6a82252 100644
--- a/src/openvpn/tls_crypt.c
+++ b/src/openvpn/tls_crypt.c
@@ -278,45 +278,6 @@ error_exit:
 return false;
 }
 
-static inline bool
-tls_crypt_v2_read_keyfile(struct buffer *key, const char *pem_name,
-  const char *key_file, const char *key_inline)
-{
-bool ret = false;
-struct buffer key_pem = { 0 };
-struct gc_arena gc = gc_new();
-
-if (strcmp(key_file, INLINE_FILE_TAG))
-{
-key_pem = buffer_read_from_file(key_file, );
-if (!buf_valid(_pem))
-{
-msg(M_WARN, "ERROR: failed to read tls-crypt-v2 key file (%s)",
-key_file);
-goto cleanup;
-}
-}
-else
-{
-buf_set_read(_pem, (const void *)key_inline, strlen(key_inline) + 
1);
-}
-
-if (!crypto_pem_decode(pem_name, key, _pem))
-{
-msg(M_WARN, "ERROR: tls-crypt-v2 pem decode failed");
-goto cleanup;
-}
-
-ret = true;
-cleanup:
-if (strcmp(key_file, INLINE_FILE_TAG))
-{
-buf_clear(_pem);
-}
-gc_free();
-return ret;
-}
-
 static inline void
 tls_crypt_v2_load_client_key(struct key_ctx_bi *key, const struct key2 *key2,
  bool tls_server)
@@ -339,8 +300,8 @@ tls_crypt_v2_init_client_key(struct key_ctx_bi *key, struct 
buffer *wkc_buf,
 struct buffer client_key = alloc_buf(TLS_CRYPT_V2_CLIENT_KEY_LEN
  + TLS_CRYPT_V2_MAX_WKC_LEN);
 
-if (!tls_crypt_v2_read_keyfile(_key, tls_crypt_v2_cli_pem_name,
-   key_file, key_inline))
+if (!read_pem_key_file(_key, tls_crypt_v2_cli_pem_name,
+   key_file, key_inline))
 {
 msg(M_FATAL, "ERROR: invalid tls-crypt-v2 client key format");
 }
@@ -365,8 +326,8 @@ tls_crypt_v2_init_server_key(struct key_ctx *key_ctx, bool 
encrypt,
 struct buffer srv_key_buf;
 
 buf_set_write(_key_buf, (void *)_key, sizeof(srv_key));
-if 

[Openvpn-devel] [PATCH v2 6/6] Implement unit tests for auth-gen-token

2019-01-22 Thread Arne Schwabe
From: Arne Schwabe 

Patch V2: adapt unit tests to other V2 patches
---
 tests/unit_tests/openvpn/Makefile.am   |  18 +-
 tests/unit_tests/openvpn/test_auth_token.c | 375 +
 2 files changed, 392 insertions(+), 1 deletion(-)
 create mode 100644 tests/unit_tests/openvpn/test_auth_token.c

diff --git a/tests/unit_tests/openvpn/Makefile.am 
b/tests/unit_tests/openvpn/Makefile.am
index 4f137b2b..d3e6e87b 100644
--- a/tests/unit_tests/openvpn/Makefile.am
+++ b/tests/unit_tests/openvpn/Makefile.am
@@ -6,7 +6,7 @@ if HAVE_LD_WRAP_SUPPORT
 check_PROGRAMS += argv_testdriver buffer_testdriver
 endif
 
-check_PROGRAMS += crypto_testdriver packet_id_testdriver
+check_PROGRAMS += crypto_testdriver packet_id_testdriver auth_token_testdriver
 if HAVE_LD_WRAP_SUPPORT
 check_PROGRAMS += tls_crypt_testdriver
 endif
@@ -72,3 +72,19 @@ tls_crypt_testdriver_SOURCES = test_tls_crypt.c mock_msg.c \
$(openvpn_srcdir)/packet_id.c \
$(openvpn_srcdir)/platform.c \
$(openvpn_srcdir)/run_command.c
+
+auth_token_testdriver_CFLAGS  = @TEST_CFLAGS@ \
+   -I$(openvpn_includedir) -I$(compat_srcdir) -I$(openvpn_srcdir) \
+   $(OPTIONAL_CRYPTO_CFLAGS)
+auth_token_testdriver_LDFLAGS = @TEST_LDFLAGS@ \
+   $(OPTIONAL_CRYPTO_LIBS)
+
+auth_token_testdriver_SOURCES = test_auth_token.c mock_msg.c \
+   $(openvpn_srcdir)/buffer.c \
+   $(openvpn_srcdir)/crypto.c \
+   $(openvpn_srcdir)/crypto_mbedtls.c \
+   $(openvpn_srcdir)/crypto_openssl.c \
+   $(openvpn_srcdir)/otime.c \
+   $(openvpn_srcdir)/packet_id.c \
+   $(openvpn_srcdir)/platform.c \
+   $(openvpn_srcdir)/base64.c
diff --git a/tests/unit_tests/openvpn/test_auth_token.c 
b/tests/unit_tests/openvpn/test_auth_token.c
new file mode 100644
index ..a3591b4a
--- /dev/null
+++ b/tests/unit_tests/openvpn/test_auth_token.c
@@ -0,0 +1,375 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ * over a single UDP port, with support for SSL/TLS-based
+ * session authentication and key exchange,
+ * packet encryption, packet authentication, and
+ * packet compression.
+ *
+ *  Copyright (C) 2016-2018 Fox Crypto B.V. 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2
+ *  as published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#elif defined(_MSC_VER)
+#include "config-msvc.h"
+#endif
+
+#include "syshead.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "auth_token.c"
+
+#include "mock_msg.h"
+
+struct test_context {
+struct tls_multi multi;
+struct key_type kt;
+struct user_pass up;
+struct tls_session session;
+};
+
+static const char *now0key0 = 
"SESS_ID_AT_0123456789abcdefAE5JsQJOVfo8jnI3RL3tBaR5NkE4yPfcylFUHmHSc5Bu";
+
+static const char *zeroinline = "-BEGIN OpenVPN auth-token server 
key-\n"
+
"\n"
+
"\n"
+
"AAA=\n"
+"-END OpenVPN auth-token server key-";
+
+static const char *allx01inline = "-BEGIN OpenVPN auth-token server 
key-\n"
+  
"AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEB\n"
+  
"AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEB\n"
+  
"AQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQEBAQE=\n"
+  "-END OpenVPN auth-token server 
key-";
+
+static const char *random_key = "-BEGIN OpenVPN auth-token server 
key-\n"
+
"+mmmf7IQ5cymtMVjKYTWk8IOcYanRlpQmV9Tb3EjkHYxueBVDg3yqRgzeBlVGzNLD//rAPiOVhau\n"
+
"3NDBjNOQB8951bfs7Cc2mYfay92Bh2gRJ5XEM/DMfzCWN+7uU6NWoTTHr4FuojnIQtjtqVAj/JS9\n"
+
"w+dTSp/vYHl+c7uHd19uVRu/qLqV85+rm4tUGIjO7FfYuwyPqwmhuIsi3hs9QkSimh888FmBpoKY\n"
+
"/tbKVTJZmSERKti9KEwtV2eVAR0znN5KW7lCB3mHVAhN7bUpcoDjfCzYIFARxwswTFu9gFkwqUMY\n"
+"I1KUOgIsVNs4llACioeXplYekWETR+YkJwDc/A==\n"
+  

Re: [Openvpn-devel] [PATCH v2] test_tls_crypt.c: fix global-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Arne Schwabe
Am 22.01.19 um 14:34 schrieb Lev Stipakov:
> From: Lev Stipakov 
> 
> When writing data to buffer we incorrectly specify source length
>  - sizeof for pointer returns 8, but actual buffer length is 1.
> 
> Fix by replacing empty global string to local string literal and
> specifying the correct length.



Acked-By: Arne Schwabe

Arne




signature.asc
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] crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Arne Schwabe
Am 22.01.19 um 14:41 schrieb Lev Stipakov:
> From: Lev Stipakov 
> 
> OpenSSL's version of crypto_pem_encode() uses PEM_write_bio()
> function to write PEM-encoded data to BIO object. That method doesn't
> add NUL termanator, unlike its mbedTLS counterpart mbedtls_pem_write_buffer().
> 
> The code which uses PEM data treats it as a string, so missing NUL
> terminator makes sanitizer to compain.
> 
> Fix by adding a NUL terminator.
> 
> Signed-off-by: Lev Stipakov 
> ---
>  v2: use a dedivcated function to add a nul terminator
 ^^^ :)

Acked-By: Arne Schwabe

Arne



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


[Openvpn-devel] [PATCH v2] crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Lev Stipakov
From: Lev Stipakov 

OpenSSL's version of crypto_pem_encode() uses PEM_write_bio()
function to write PEM-encoded data to BIO object. That method doesn't
add NUL termanator, unlike its mbedTLS counterpart mbedtls_pem_write_buffer().

The code which uses PEM data treats it as a string, so missing NUL
terminator makes sanitizer to compain.

Fix by adding a NUL terminator.

Signed-off-by: Lev Stipakov 
---
 v2: use a dedivcated function to add a nul terminator

 src/openvpn/crypto_openssl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 9691ce0..c049e52 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -400,8 +400,9 @@ crypto_pem_encode(const char *name, struct buffer *dst,
 BUF_MEM *bptr;
 BIO_get_mem_ptr(bio, );
 
-*dst = alloc_buf_gc(bptr->length, gc);
+*dst = alloc_buf_gc(bptr->length + 1, gc);
 ASSERT(buf_write(dst, bptr->data, bptr->length));
+buf_null_terminate(dst);
 
 ret = true;
 cleanup:
-- 
2.7.4



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


[Openvpn-devel] [PATCH v2] test_tls_crypt.c: fix global-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Lev Stipakov
From: Lev Stipakov 

When writing data to buffer we incorrectly specify source length
 - sizeof for pointer returns 8, but actual buffer length is 1.

Fix by replacing empty global string to local string literal and
specifying the correct length.

Signed-off-by: Lev Stipakov 
---
 v2: use strlen(), fix misleading comments

 tests/unit_tests/openvpn/test_tls_crypt.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c 
b/tests/unit_tests/openvpn/test_tls_crypt.c
index b793a7a..17f7d89 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -49,8 +49,6 @@
 #define PARAM1  "param1"
 #define PARAM2  "param two"
 
-static const char *plaintext_short = "";
-
 static const char *test_server_key = \
 "-BEGIN OpenVPN tls-crypt-v2 server key-\n"
 "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4v\n"
@@ -148,10 +146,12 @@ test_tls_crypt_setup(void **state) {
 ctx->unwrapped = alloc_buf(TESTBUF_SIZE);
 
 /* Write test plaintext */
-buf_write(>source, plaintext_short, sizeof(plaintext_short));
+const char *plaintext = "1234567890";
+buf_write(>source, plaintext, strlen(plaintext));
 
-/* Write dummy opcode and session id */
-buf_write(>ciphertext, "012345678", 1 + 8);
+/* Write test ciphertext */
+const char *ciphertext = "012345678";
+buf_write(>ciphertext, ciphertext, strlen(ciphertext));
 
 return 0;
 }
-- 
2.7.4



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


Re: [Openvpn-devel] [PATCH 2/2] test_tls_crypt.c: fix global-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Arne Schwabe
Am 22.01.19 um 12:02 schrieb Lev Stipakov:
> From: Lev Stipakov 
> 
> When writing data to buffer we incorrectly specify source length
>  - sizeof for pointer returns 8, but actual buffer length is 1.
> 
> Fix by replacing empty global string to local string literal and
> specifying the correct length.
> 
> Signed-off-by: Lev Stipakov 
> ---
>  tests/unit_tests/openvpn/test_tls_crypt.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c 
> b/tests/unit_tests/openvpn/test_tls_crypt.c
> index b793a7a..b4ce03d 100644
> --- a/tests/unit_tests/openvpn/test_tls_crypt.c
> +++ b/tests/unit_tests/openvpn/test_tls_crypt.c
> @@ -49,8 +49,6 @@
>  #define PARAM1  "param1"
>  #define PARAM2  "param two"
>  
> -static const char *plaintext_short = "";
> -
>  static const char *test_server_key = \
>  "-BEGIN OpenVPN tls-crypt-v2 server key-\n"
>  "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4v\n"
> @@ -148,7 +146,7 @@ test_tls_crypt_setup(void **state) {
>  ctx->unwrapped = alloc_buf(TESTBUF_SIZE);
>  
>  /* Write test plaintext */
> -buf_write(>source, plaintext_short, sizeof(plaintext_short));
> +buf_write(>source, "1234567890", 10);
>  
>  /* Write dummy opcode and session id */
>  buf_write(>ciphertext, "012345678", 1 + 8);
> 

I would suggest plaintext_short="12345678"; and then use
strlen(plaintext_short)+1 in the buf_write.

But I can confirm that this indeed fixes the issue it should fix.

Arne



signature.asc
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 1/2] crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Arne Schwabe
Am 22.01.19 um 12:02 schrieb Lev Stipakov:
> From: Lev Stipakov 
> 
> OpenSSL's version of crypto_pem_encode() uses PEM_write_bio()
> function to write PEM-encoded data to BIO object. That method doesn't
> add NUL termanator, unlike its mbedTLS counterpart mbedtls_pem_write_buffer().
> 
> The code which uses PEM data treats it as a string, so missing NUL
> terminator makes sanitizer to compain.
> 
> Fix by adding a NUL terminator.
> 
> Signed-off-by: Lev Stipakov 
> ---
>  src/openvpn/crypto_openssl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
> index 9691ce0..6a49067 100644
> --- a/src/openvpn/crypto_openssl.c
> +++ b/src/openvpn/crypto_openssl.c
> @@ -400,8 +400,9 @@ crypto_pem_encode(const char *name, struct buffer *dst,
>  BUF_MEM *bptr;
>  BIO_get_mem_ptr(bio, );
>  
> -*dst = alloc_buf_gc(bptr->length, gc);
> +*dst = alloc_buf_gc(bptr->length + 1, gc);
>  ASSERT(buf_write(dst, bptr->data, bptr->length));
> +*BEND(dst) = '\0';

buf_null_terminate(dst) is a better function here :)

Otherwise ACK, fixes the problem.

Arne



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


[Openvpn-devel] [PATCH 2/2] test_tls_crypt.c: fix global-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Lev Stipakov
From: Lev Stipakov 

When writing data to buffer we incorrectly specify source length
 - sizeof for pointer returns 8, but actual buffer length is 1.

Fix by replacing empty global string to local string literal and
specifying the correct length.

Signed-off-by: Lev Stipakov 
---
 tests/unit_tests/openvpn/test_tls_crypt.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c 
b/tests/unit_tests/openvpn/test_tls_crypt.c
index b793a7a..b4ce03d 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -49,8 +49,6 @@
 #define PARAM1  "param1"
 #define PARAM2  "param two"
 
-static const char *plaintext_short = "";
-
 static const char *test_server_key = \
 "-BEGIN OpenVPN tls-crypt-v2 server key-\n"
 "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8gISIjJCUmJygpKissLS4v\n"
@@ -148,7 +146,7 @@ test_tls_crypt_setup(void **state) {
 ctx->unwrapped = alloc_buf(TESTBUF_SIZE);
 
 /* Write test plaintext */
-buf_write(>source, plaintext_short, sizeof(plaintext_short));
+buf_write(>source, "1234567890", 10);
 
 /* Write dummy opcode and session id */
 buf_write(>ciphertext, "012345678", 1 + 8);
-- 
2.7.4



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


[Openvpn-devel] [PATCH 1/2] crypto_openssl.c: fix heap-buffer-overflow found by AddressSanitizer

2019-01-22 Thread Lev Stipakov
From: Lev Stipakov 

OpenSSL's version of crypto_pem_encode() uses PEM_write_bio()
function to write PEM-encoded data to BIO object. That method doesn't
add NUL termanator, unlike its mbedTLS counterpart mbedtls_pem_write_buffer().

The code which uses PEM data treats it as a string, so missing NUL
terminator makes sanitizer to compain.

Fix by adding a NUL terminator.

Signed-off-by: Lev Stipakov 
---
 src/openvpn/crypto_openssl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c
index 9691ce0..6a49067 100644
--- a/src/openvpn/crypto_openssl.c
+++ b/src/openvpn/crypto_openssl.c
@@ -400,8 +400,9 @@ crypto_pem_encode(const char *name, struct buffer *dst,
 BUF_MEM *bptr;
 BIO_get_mem_ptr(bio, );
 
-*dst = alloc_buf_gc(bptr->length, gc);
+*dst = alloc_buf_gc(bptr->length + 1, gc);
 ASSERT(buf_write(dst, bptr->data, bptr->length));
+*BEND(dst) = '\0';
 
 ret = true;
 cleanup:
-- 
2.7.4



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