Re: [Openvpn-devel] [PATCH] Fix various spelling mistakes
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
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
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
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
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
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
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
> > +/* > + * 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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