[Openvpn-devel] Patch: Export NotBefore and NotAfter items to the environment in client-connect
We're considering to use shorter-lived client certificates for our VPN
users. In an effort to prevent negative impact for our staff due to
expired certificates, we 'd like to keep track of imminent expiration
of certificates in the client-connect script (which we're using anyway
to check is the certificate matches the user id). Many certificate
attributes are passed to the script, but not the "NotAfter" and
"NotBefore" attributes.
The attached patch adds these to the mix.
Rolf
diff -ruN openvpn-2.4.7.orig/src/openvpn/ssl_verify.c openvpn-2.4.7/src/openvpn/ssl_verify.c
--- openvpn-2.4.7.orig/src/openvpn/ssl_verify.c 2019-02-20 13:28:23.0 +0100
+++ openvpn-2.4.7/src/openvpn/ssl_verify.c 2019-08-15 20:57:29.80338 +0200
@@ -448,6 +448,25 @@
}
/*
+ * Export ASN1_TIME items to the environment
+ */
+static void
+setenv_ASN1_TIME(struct env_set *es, char *envname, int envnamesize,
+ char *envprefix, int depth, const ASN1_TIME *asn1_time)
+{
+char timestamp[32];
+BIO *mem;
+
+mem = BIO_new(BIO_s_mem());
+if (ASN1_TIME_print (mem, asn1_time)) {
+timestamp[BIO_read(mem, timestamp, sizeof(timestamp)-1)] = '\0';
+openvpn_snprintf(envname, envnamesize, "%s_%d", envprefix, depth);
+setenv_str(es, envname, timestamp);
+}
+BIO_free(mem);
+}
+
+/*
* Export the subject, common_name, and raw certificate fields to the
* environment for later verification by scripts and plugins.
*/
@@ -505,6 +524,12 @@
openvpn_snprintf(envname, sizeof(envname), "tls_serial_hex_%d", cert_depth);
setenv_str(es, envname, serial);
+setenv_ASN1_TIME(es, envname, sizeof(envname), "tls_notbefore", cert_depth,
+ X509_get_notBefore(peer_cert));
+
+setenv_ASN1_TIME(es, envname, sizeof(envname), "tls_notafter", cert_depth,
+ X509_get_notAfter(peer_cert));
+
gc_free(&gc);
}
smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] ***UNCHECKED*** Patch: Export NotBefore and NotAfter items to the environment in client-connect
On 16/08/2019 10:27, Rolf Fokkens via Openvpn-devel wrote: > We're considering to use shorter-lived client certificates for our VPN > users. In an effort to prevent negative impact for our staff due to > expired certificates, we 'd like to keep track of imminent expiration > of certificates in the client-connect script (which we're using anyway > to check is the certificate matches the user id). Many certificate > attributes are passed to the script, but not the "NotAfter" and > "NotBefore" attributes. > > The attached patch adds these to the mix. This gets a Feature-ACK from me. This is useful information, and something other users in the community have asked for earlier too. But there are a few things here before starting to dive into the details. First of all, we want to have patches first into git master, and then we need to discuss in the community if this feature is something we want to backport to the 2.4 release. After a new release has stabilized (which 2.4 has), we are quite reluctant to add new features to those releases. Another thing is that I think it would be valuable to also print this information into the logs as well. The X509_get_notBefore() value is probably not so important unless that has a value which is in the future. The X509_get_notAfter() is fine to always log, but would be nice if it would come a M_WARN log entry if it has expired. To achieve this logging feature, setenv_ASN1_TIME() would need to be refactored a bit - possibly by returning a string as well as "is now() after the time stamp?" bool flag. The "printing" could happen to a gc_arena allocated buffer (which is available in verify_cert_set_env()). The logging should probably already happen in verify_cert(), which also has its own gc_arena. There are various alternatives to avoid doing the ASN1_TIME_print() preparations and processing multiple times (for logging and setenv), but I don't have a clear idea right now what could be a reasonable approach. And lastly, this code will break compilation if using ./configure --with-crypto-library=mbedtls ... This should also be improved. Other than that, the code looks reasonable at first glance (I have not compile tested it yet) -- kind regards, David Sommerseth OpenVPN Inc 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] Increase listen() backlog queue to 32
On 15/08/2019 17:53, Gert Doering wrote: > For reasons historically unknown, OpenVPN sets the listen() backlog > queue to "1", which signals the kernel "while there is one TCP connect > waiting for OpenVPN to handle it, refuse all others" - which, on > restarting a busy TCP server, will create connection issues. > > The exact "best" value of the backlog queue is subject of discussion, > but for a server that is not extremely busy with many connections > coming in in parallel, there is no real difference between "10" or "500", > as long as it's "more than 1". > > Found and debugged by "mjo" in Trac. > > Trac: #1208 > > Signed-off-by: Gert Doering Acked-By: David Sommerseth I agree with Antonio, and we should make it somewhat easier to modify. I'm not sure if there's value in having it as a runtime option, like --socket-backlog (or something like that), or as a value you can pass to ./configure at compile time. -- kind regards, David Sommerseth OpenVPN Inc 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] Adding support for wolfSSL backend
This patch adds the option to use wolfSSL as the ssl backend. To build
this patch:
1. wolfSSL needs to be built with the `--enable-all` configure option.
2. OpenVPN must be built with the `--with-crypto-library=wolfssl`
configure option.
Documentation regarding the wolfSSL SSL library may be found here:
https://www.wolfssl.com/
Sincerely
Juliusz
diff --git a/.gitignore b/.gitignore
index 0d68ec4b..d007cf62 100644
--- a/.gitignore
+++ b/.gitignore
@@ -72,3 +72,8 @@ nbproject
test-driver
compile
stamp-h2
+
+\.settings/
+\.project
+\.cproject
+\.autotools
diff --git a/configure.ac b/configure.ac
index e9f8a2f9..1013e5a0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -276,10 +276,10 @@ AC_ARG_WITH(
AC_ARG_WITH(
[crypto-library],
- [AS_HELP_STRING([--with-crypto-library=library], [build with the given crypto library, TYPE=openssl|mbedtls @<:@default=openssl@:>@])],
+ [AS_HELP_STRING([--with-crypto-library=library], [build with the given crypto library, TYPE=openssl|mbedtls|wolfssl @<:@default=openssl@:>@])],
[
case "${withval}" in
- openssl|mbedtls) ;;
+ openssl|mbedtls|wolfssl) ;;
*) AC_MSG_ERROR([bad value ${withval} for --with-crypto-library]) ;;
esac
],
@@ -1011,6 +1011,31 @@ elif test "${with_crypto_library}" = "mbedtls"; then
AC_DEFINE([ENABLE_CRYPTO_MBEDTLS], [1], [Use mbed TLS library])
CRYPTO_CFLAGS="${MBEDTLS_CFLAGS}"
CRYPTO_LIBS="${MBEDTLS_LIBS}"
+elif test "${with_crypto_library}" = "wolfssl"; then
+ AC_ARG_VAR([WOLFSSL_CFLAGS], [C compiler flags for wolfssl])
+ AC_ARG_VAR([WOLFSSL_LIBS], [linker flags for wolfssl])
+
+ saved_CFLAGS="${CFLAGS}"
+ saved_LIBS="${LIBS}"
+
+ if test -z "${WOLFSSL_CFLAGS}" -a -z "${WOLFSSL_LIBS}"; then
+ # if the user did not explicitly specify flags, try to autodetect
+ LIBS="${LIBS} -lwolfssl -lm -pthread"
+ AC_CHECK_LIB(
+ [wolfssl],
+ [wolfSSL_get_ciphers],
+ [],
+ [AC_MSG_ERROR([Could not link wolfSSL library.])]
+ )
+ fi
+
+ have_crypto_aead_modes="yes"
+
+ CFLAGS="${WOLFSSL_CFLAGS} ${CFLAGS}"
+ LIBS="${WOLFSSL_LIBS} ${LIBS}"
+ AC_DEFINE([ENABLE_CRYPTO_WOLFSSL], [1], [Use wolfSSL crypto library])
+ CRYPTO_CFLAGS="${WOLFSSL_CFLAGS}"
+ CRYPTO_LIBS="${WOLFSSL_LIBS}"
else
AC_MSG_ERROR([Invalid crypto library: ${with_crypto_library}])
fi
diff --git a/include/openvpn-plugin.h.in b/include/openvpn-plugin.h.in
index 103844f7..75b33a62 100644
--- a/include/openvpn-plugin.h.in
+++ b/include/openvpn-plugin.h.in
@@ -32,6 +32,12 @@
#define __OPENVPN_X509_CERT_T_DECLARED
typedef mbedtls_x509_crt openvpn_x509_cert_t;
#endif
+#elif defined(ENABLE_CRYPTO_WOLFSSL) /* ifdef ENABLE_CRYPTO_WOLFSSL */
+#include
+#ifndef __OPENVPN_X509_CERT_T_DECLARED
+#define __OPENVPN_X509_CERT_T_DECLARED
+typedef WOLFSSL_X509 openvpn_x509_cert_t;
+#endif
#else /* ifdef ENABLE_CRYPTO_MBEDTLS */
#include
#ifndef __OPENVPN_X509_CERT_T_DECLARED
@@ -332,7 +338,8 @@ struct openvpn_plugin_callbacks
typedef enum {
SSLAPI_NONE,
SSLAPI_OPENSSL,
-SSLAPI_MBEDTLS
+SSLAPI_MBEDTLS,
+SSLAPI_WOLFSSL
} ovpnSSLAPI;
/**
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index 30caa01f..5c19384e 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -50,6 +50,7 @@ openvpn_SOURCES = \
crypto.c crypto.h crypto_backend.h \
crypto_openssl.c crypto_openssl.h \
crypto_mbedtls.c crypto_mbedtls.h \
+ crypto_wolfssl.c crypto_wolfssl.h \
dhcp.c dhcp.h \
env_set.c env_set.h \
errlevel.h \
@@ -115,10 +116,12 @@ openvpn_SOURCES = \
ssl.c ssl.h ssl_backend.h \
ssl_openssl.c ssl_openssl.h \
ssl_mbedtls.c ssl_mbedtls.h \
+ ssl_wolfssl.c ssl_wolfssl.h \
ssl_common.h \
ssl_verify.c ssl_verify.h ssl_verify_backend.h \
ssl_verify_openssl.c ssl_verify_openssl.h \
ssl_verify_mbedtls.c ssl_verify_mbedtls.h \
+ ssl_verify_wolfssl.c ssl_verify_wolfssl.h \
status.c status.h \
syshead.h \
tls_crypt.c tls_crypt.h \
diff --git a/src/openvpn/crypto_backend.h b/src/openvpn/crypto_backend.h
index 7e9a4bd2..9699b50c 100644
--- a/src/openvpn/crypto_backend.h
+++ b/src/openvpn/crypto_backend.h
@@ -29,18 +29,21 @@
#ifndef CRYPTO_BACKEND_H_
#define CRYPTO_BACKEND_H_
+/* TLS uses a tag of 128 bytes, let's do the same for OpenVPN */
+#define OPENVPN_AEAD_TAG_LENGTH 16
+
#ifdef ENABLE_CRYPTO_OPENSSL
#include "crypto_openssl.h"
#endif
#ifdef ENABLE_CRYPTO_MBEDTLS
#include "crypto_mbedtls.h"
#endif
+#ifdef ENABLE_CRYPTO_WOLFSSL
+#include "crypto_wolfssl.h"
+#endif
#include "basic.h"
#include "buffer.h"
-/* TLS uses a tag of 128 bytes, let's do the same for OpenVPN */
-#define OPENVPN_AEAD_TAG_LENGTH 16
-
/* Maximum cipher block size (bytes) */
#define OPENVPN_MAX_CIPHER_BLOCK_SIZE 32
@@ -355,7 +358,7 @@ void cipher_ctx_free(cipher_ctx_t *ctx);
* @param key_len Length of the key, in bytes
* @param ktStatic cipher parameters to use
* @param enc Whether to encrypt or decrypt (either
- * \c MBEDTLS_OP_ENCRYPT or \c MBEDTLS_OP_DECRYPT).
+
Re: [Openvpn-devel] [PATCH] Increase listen() backlog queue to 32
Hi, On 16/08/2019 13:49, David Sommerseth wrote: > On 15/08/2019 17:53, Gert Doering wrote: >> For reasons historically unknown, OpenVPN sets the listen() backlog >> queue to "1", which signals the kernel "while there is one TCP connect >> waiting for OpenVPN to handle it, refuse all others" - which, on >> restarting a busy TCP server, will create connection issues. >> >> The exact "best" value of the backlog queue is subject of discussion, >> but for a server that is not extremely busy with many connections >> coming in in parallel, there is no real difference between "10" or "500", >> as long as it's "more than 1". >> >> Found and debugged by "mjo" in Trac. >> >> Trac: #1208 >> >> Signed-off-by: Gert Doering > > Acked-By: David Sommerseth > > I agree with Antonio, and we should make it somewhat easier to modify. I disagree with you on this point :D This is not something we expect people to play with. This is only a value that a developer with networking knowledge is expected to find and tweak. Hence my suggestion to make it a define in some header main header file. > I'm > not sure if there's value in having it as a runtime option, like > --socket-backlog (or something like that), or as a value you can pass to > ./configure at compile time. > Like above: yet another config option that the average joe can mess up and come up with unknown problems nobody will understand? nonono ;) Cheers, > > > > ___ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > -- Antonio Quartulli 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] Adding support for wolfSSL backend
Am 16.08.19 um 16:14 schrieb Juliusz Sosinowicz: > This patch adds the option to use wolfSSL as the ssl backend. To build > this patch: > That is great and it is also a very big patch. I skimmed only through the patch. +#ifdef ENABLE_CRYPTO_WOLFSSL +o->ciphername = "AES-256-CBC"; +#else o->ciphername = "BF-CBC"; +#endif Such silent changes that OpenVPN behaves different, is something we would like to avoid. Better to error out in this case than to behave diffently. Overall the WolfSSL feels to be a bit similar to OpenSSL. Is there any compatibility you are aiming at? Also it would be nice to have a summary for people on the OpenVPN perspective - Why WolfSSL in OpenVPN instead of mbed or OpenSSL - What features does WolfSSL offer in OpenVPN that mbed/OpenSSL don't have - What is missing with WolfSSL? That should also good to have in the patch like README.mbedtls. And one of the important question is: What are your future plans in terms of involvement in OpenVPN development and maintaince? I think since you are first time contributer and this a big patch, that is something resonable to ask. Arne ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Adding support for wolfSSL backend
Hi Juliusz, On 16/08/2019 16:14, Juliusz Sosinowicz wrote: > This patch adds the option to use wolfSSL as the ssl backend. To build > this patch: > > 1. wolfSSL needs to be built with the `--enable-all` configure option. > 2. OpenVPN must be built with the `--with-crypto-library=wolfssl` > configure option. > first of all thanks a lot for your contribution! I have looked at wolfSSL more than a year ago and back then it was implementing an OpenSSL compatibility layer, so that it could be used as drop-in replacement in all those programs using OpenSSL as main TLS library. Is this layer still in place? If so, why not using it to link OpenVPN against wolfSSL rather than adding yet another backend? The reason why I ask is that adding a new crypto backend drastically increases the maintenance cost for us. Therefore, reducing the change required in OpenVPN would be extremely beneficial. Thanks a lot. Best Regards, -- Antonio Quartulli 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 v6 4/7] Rewrite auth-token-gen to be based on HMAC based tokens
On 08/08/2019 16:51, Arne Schwabe wrote: > 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. > > Using the new config directive auth-gen-token-secret instead of > extending auth-gen-token (--auth-gen-token [lifetime] [secret-key]) was > chosen to allow inlinening the secret key. > > Patch V2: cleaned up code, use refactored read_pem_key_file function > Patch V3: clarify some design decision in the commit message > Patch V4: Use ephermal_generate_key > Patch V5: Use C99 PRIu64 instead of %lld int printf like statement, > fix strict aliasing > --- > doc/openvpn.8| 25 > src/openvpn/Makefile.am | 1 + > src/openvpn/auth_token.c | 273 +++ > src/openvpn/auth_token.h | 116 + > src/openvpn/init.c | 30 - > src/openvpn/openvpn.h| 1 + > src/openvpn/options.c| 22 +++- > 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, 640 insertions(+), 135 deletions(-) > create mode 100644 src/openvpn/auth_token.c > create mode 100644 src/openvpn/auth_token.h > Hi, Thanks a lot. This now only leaves the following warning when using gcc-4.8.5 and gcc-6.3.1 (both on RHEL 7.7) auth_token.c: In function ‘generate_auth_token’: auth_token.c:115:9: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing] initial_timestamp = *((uint64_t *)(old_tstamp_decode)); ^ This warning is not present when compiling with gcc-7.3.1, gcc-8.3.1, clang-3.4.2 nor clang-5.0.1. So I'm blaming buggy/confused older GCC compilers for this one. Since I've tested and reviewed the rest in earlier rounds and the change from previous version i sjust changing %lld to PRIu64, I'm giving this my ... Acked-By: David Sommerseth -- kind regards, David Sommerseth OpenVPN Inc ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Adding support for wolfSSL backend
Hi, On Fri, Aug 16, 2019 at 05:22:27PM +0200, Antonio Quartulli wrote: > The reason why I ask is that adding a new crypto backend drastically > increases the maintenance cost for us. ... and since we're already struggling with providing proper maintenance and getting new stuff integrated in a timely fashion, every new burden due to extra testing, extra backends to consider when adding crypto- related patches, etc., is a fairly big no-go today. Now, if we had 30 contributors that are all sitting idle waiting for patches to review, test and ACK, I might view this slightly differently... > Therefore, reducing the change > required in OpenVPN would be extremely beneficial. This. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Increase listen() backlog queue to 32
Wow, more ACKs than actual lines of codes affected .-) - I hear you wrt "have a #define somewhere" (and yes, we could do that) or "make it easier to configure for users" (nah... for the reasons Antonio gave) :-) Patch has been applied to the master and release/2.4 branch. 2.4, because (see the discussion in the ticket) this effectively acknowledges the changing nature of the internet, with ubiquitous background scanning on well-known ports like TCP/443 - thus getting in the way of production openvpn setups with "listen(s,1)". commit 6d8380c78bf77766454b93b49ab2ebf713b0be48 (master) commit ec0ca68f4ed1e6aa6f08f470b18e0198b7e5a4da (release/2.4) Author: Gert Doering Date: Thu Aug 15 17:53:19 2019 +0200 Increase listen() backlog queue to 32 Signed-off-by: Gert Doering Acked-by: Antonio Quartulli Acked-by: David Sommerseth Message-Id: <20190815155319.28249-1-g...@greenie.muc.de> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18758.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/6] networking: extend API for better memory management
Hi,
On Mon, Aug 05, 2019 at 11:25:26AM +0200, Antonio Quartulli wrote:
> From: Antonio Quartulli
>
> Networking backend implementations may need to allocate dynamic
> resources that require an explicit free/release.
> Since these cleanup are perfomed not very often, and only at specific
> times, it makes sense to have the upper layer signal when it's the right
> time to do so, by means of a new API call.
While I generally like this patch ("0.9 ACKs"), there is one thing that
rubs me totally the wrong way and that must go:
> +char *addr_str = (char *)print_in_addr_t(*addr, 0, &ctx->gc);
> +char *brd_str = (char *)print_in_addr_t(*broadcast, 0, &ctx->gc);
These casts - print_in_addr_t() returns a "const char *", it gets
assigned to a temporary variable of type "char *", and handed over to
a printf() variant. Such code should never ever have a cast.
If the compiler warns about loss of const (in which case I would
ask the question why the return value of print_in_addr_t() is declared
const at all, if it's a gc_malloc()ed string which *could* be written
to just fine...), then let's make "addr_str" and "brd_str" a const
as well.
But no casts in for "basic types assigned to single-use variables", ever.
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v6 5/7] Implement a permanent session id in auth-token
On 08/08/2019 16:52, Arne Schwabe wrote:
> 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 be only done
> explicitly when the external-auth keyword to auth-gen-token is present.
>
> Patch V2: Add Empty variants to work around behaviour in openvpn 3
> Patch V3: document the behaviour of external-auth better in the man page,
> rename 'auth' parameter to 'external-auth'
> Patch V4: Rebase on current master
> Patch V6: Fix tls_lock_username rejecting clients with empty username
> when explicitly accepting them with external-auth
> ---
> doc/openvpn.8| 37 +-
> src/openvpn/auth_token.c | 156 ---
> 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| 4 +-
> src/openvpn/ssl_common.h | 12 ++-
> src/openvpn/ssl_verify.c | 67 +++--
> 9 files changed, 270 insertions(+), 40 deletions(-)
[...snip...]
> @@ -97,6 +180,8 @@ generate_auth_token(const struct user_pass *up, struct
> tls_multi *multi)
> hmac_ctx_t *ctx = multi->opt.auth_token_key.hmac;
> ASSERT(hmac_ctx_size(ctx) == 256/8);
>
> +uint8_t sessid[AUTH_TOKEN_SESSION_ID_LEN];
> +
> if (multi->auth_token)
> {
> /* Just enough space to fit 8 bytes+ 1 extra to decode a non padded
> @@ -109,27 +194,65 @@ generate_auth_token(const struct user_pass *up, struct
> tls_multi *multi)
> * reuse the same session id and timestamp and null terminate it at
> * for base64 decode it only decodes the session id part of it
> */
> -char *old_tsamp_initial = multi->auth_token +
> strlen(SESSION_ID_PREFIX);
> +char *old_sessid = multi->auth_token + strlen(SESSION_ID_PREFIX);
> +char *old_tsamp_initial = old_sessid + AUTH_TOKEN_SESSION_ID_LEN*8/6;
>
> old_tsamp_initial[12] = '\0';
> ASSERT(openvpn_base64_decode(old_tsamp_initial, old_tstamp_decode,
> 9) == 9);
> -initial_timestamp = *((uint64_t *)(old_tstamp_decode));
> +
> +/*
> + * Avoid old gcc (4.8.x) complaining about strict aliasing
> + * by using a temporary variable instead of doing it in one
> + * line
> + */
> +uint64_t *tstamp_ptr = (uint64_t *) old_tstamp_decode;
> +initial_timestamp = *tstamp_ptr;
> +
> +old_tsamp_initial[0] = '\0';
> +ASSERT(openvpn_base64_decode(old_sessid, sessid,
> AUTH_TOKEN_SESSION_ID_LEN)==AUTH_TOKEN_SESSION_ID_LEN);
> +
The gcc-4.8 strict aliasing fix works, so that is good.
Could you explain (or comment) the rationale for the ASSERT() calls here?
If the decoded token is only managed by the server side, then this shouldn't
really fail. But unless I'm misunderstanding what happens in
verify_user_pass() [ssl_verify.c:1376], we put the "password" (token) from the
client into multi->auth_token. With the ASSERT() calls here, this could be
abused to trigger a DoS attack, by sending an invalid token value. Or am I
missing something?
And there is a superfluous additional extra new line, which should not be
needed.
[...snip...]
> diff --git a/src/openvpn/auth_token.h b/src/openvpn/auth_token.h
> index 600ac29f..c10afde9 100644
> --- a/src/openvpn/auth_token.h
> +++ b/src/openvpn/auth_token.h
[...snip...]
This file includes the following comment for verify_auth_token(), which I
believe is wrong now:
* Also calls generate_auth_token to update the auth-token to extend
* its validity
I don't see verify_auth_token() being close to call generate_auth_token(). Or
did I overlook something?
[...snip...]
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index d885f4d9..8f722497 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
[...snip...]
> @@ -553,9 +553,17 @@ struct tls_multi
> /**< Auth-token sent from client has valid hmac */
> #define AUTH_TOKEN_EXPIRED (1<<1)
> /**< Auth-token sent from client has expired */
> -#endif
> +#define AUTH_TOKEN_VALID_EMPTYUSER (1<<2)
> +/**<
> + * Auth-token is only valid for an empty username
> + * and not the username actually supplied from the client
> + *
> + * OpenVPN 3 clients sometimes the empty username with a
> + * username hint from their config.
The last OpenVPN 3 paragraph is hard to understand. Did you mean this?
* OpenVPN 3 clients sometimes empty or replaces the username with a
* username hint from their config.
[...snip...]
> diff --git a/src/openvpn/ssl_verify.c
Re: [Openvpn-devel] [PATCH v6 4/7] Rewrite auth-token-gen to be based on HMAC based tokens
Hi, On Fri, Aug 16, 2019 at 07:13:14PM +0200, David Sommerseth wrote: > On 08/08/2019 16:51, Arne Schwabe wrote: > > 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). [..] > Since I've tested and reviewed the rest in earlier rounds and the change from > previous version i sjust changing %lld to PRIu64, I'm giving > this my ... > > Acked-By: David Sommerseth While I'm all happy to see this ACK (and I trust David to have done a full review on the code changes), there is a slight problem with this - it breaks "--disable-server" builds: cc -DHAVE_CONFIG_H -I. -I../../../openvpn/src/openvpn -I../.. -I../../include -I../../../openvpn/include -I../../../openvpn/src/compat -I/usr/local/include -I/usr/local/include -DPLUGIN_LIBDIR=\"/usr/local/lib/openvpn/plugins\" -Wall -Wno-unused-parameter -Wno-unused-function -g -O2 -std=c99 -MT auth_token.o -MD -MP -MF .deps/auth_token.Tpo -c -o auth_token.o ../../../openvpn/src/openvpn/auth_token.c ../../../openvpn/src/openvpn/auth_token.c:100:16: error: no member named 'auth_token' in 'struct tls_multi' if (multi->auth_token) ~ ^ ../../../openvpn/src/openvpn/auth_token.c:112:42: error: no member named 'auth_token' in 'struct tls_multi' char *old_tsamp_initial = multi->auth_token + strlen(SESSION_ID_PREFIX); ~ ^ ../../../openvpn/src/openvpn/auth_token.c:119:21: error: no member named 'auth_token' in 'struct tls_multi' free(multi->auth_token); ~ ^ ... so, can I have a v7 of this, plus an ACK...? Sorry for being a spoilsport here. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany g...@greenie.muc.de signature.asc Description: PGP signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: openssl: Fix compilation without deprecated OpenSSL 1.1 APIs
Your patch has been applied to the master branch. Is this also suitable for release/2.4? "You folks tell me, I do the cherry-picking" (if it applies) :-) I have removed the extra spaces in "# if" constructs, as this is not something we use elsewhere on nested CPP expressions (it came up in the discussion, but was still part of this patch). Tested lightly with openssl 1.0.2o and 1.1.1. commit 8a01147ff77e4ae2e377744b89fbe4b6841b2bb0 (master) Author: Rosen Penev Date: Wed Jul 24 17:29:34 2019 +0200 openssl: Fix compilation without deprecated OpenSSL 1.1 APIs Signed-off-by: Rosen Penev Signed-off-by: Arne Schwabe Acked-by: Rosen Penev Acked-by: Steffan Karger Message-Id: <20190724152934.9884-1-a...@rfc2549.org> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18700.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 applied] Re: openssl: Fix compilation without deprecated OpenSSL 1.1 APIs
On Fri, Aug 16, 2019 at 12:31 PM Gert Doering wrote: > > Your patch has been applied to the master branch. > > Is this also suitable for release/2.4? "You folks tell me, I do the > cherry-picking" (if it applies) :-) 2.4 is what I did my testing on, so yes. > > I have removed the extra spaces in "# if" constructs, as this is not > something we use elsewhere on nested CPP expressions (it came up in the > discussion, but was still part of this patch). > > Tested lightly with openssl 1.0.2o and 1.1.1. > > commit 8a01147ff77e4ae2e377744b89fbe4b6841b2bb0 (master) > Author: Rosen Penev > Date: Wed Jul 24 17:29:34 2019 +0200 > > openssl: Fix compilation without deprecated OpenSSL 1.1 APIs > > Signed-off-by: Rosen Penev > Signed-off-by: Arne Schwabe > Acked-by: Rosen Penev > Acked-by: Steffan Karger > Message-Id: <20190724152934.9884-1-a...@rfc2549.org> > URL: > https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg18700.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 applied] Re: openssl: Fix compilation without deprecated OpenSSL 1.1 APIs
Hi,
On Fri, Aug 16, 2019 at 09:31:52PM +0200, Gert Doering wrote:
> Your patch has been applied to the master branch.
>
> Is this also suitable for release/2.4? "You folks tell me, I do the
> cherry-picking" (if it applies) :-)
>
> I have removed the extra spaces in "# if" constructs, as this is not
> something we use elsewhere on nested CPP expressions (it came up in the
> discussion, but was still part of this patch).
>
> Tested lightly with openssl 1.0.2o and 1.1.1.
I should have tested with mbedtls :-/ - buildbot tells me that a good
number of platforms have started core dumping on the mbedtls client tests
with this commit.
*** Error in `../src/openvpn/openvpn': free(): invalid next size (fast):
+0x00c74850 ***
./t_client.sh: line 262: 8896 Aborted (core dumped) $RUN_SUDO
+"${top_builddir}/src/openvpn/openvpn" $openvpn_conf >> $LOGDIR/$SUF:openvpn.log
OpenVPN running with PID 8896
(I have seen this on fedora29 and one of the FreeBSDs, but there is
"more red" - more details on mbedTLS versions in use can be provided)
Steffan, if you could have a look, this would be most appreciated...
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] networking: extend API for better memory management
From: Antonio Quartulli
Networking backend implementations may need to allocate dynamic
resources that require an explicit free/release.
Since these cleanup are perfomed not very often, and only at specific
times, it makes sense to have the upper layer signal when it's the right
time to do so, by means of a new API call.
For this purpose two news APIs have been implemented:
- net_ctx_free() to release all backend specific resources. Expected to
be called at application cleanup time;
- net_ctx_reset() to let backends release temporary resources (i.e.
reset garbage collectors). To be invoked after routines that
are expected to allocate memory (i.e. tun setup or shutdown).
In this patch related implementations for iproute2 and sitnl are also
provided.
Signed-off-by: Antonio Quartulli
---
Changes from v1:
- got rid of (now) useless casts in front of print_in_addr_t()
src/openvpn/networking.h | 26 +++
src/openvpn/networking_iproute2.c | 74 ---
src/openvpn/networking_iproute2.h | 1 +
src/openvpn/networking_sitnl.c| 12 +
src/openvpn/openvpn.c | 1 +
src/openvpn/route.c | 8
src/openvpn/tun.c | 5 +++
7 files changed, 82 insertions(+), 45 deletions(-)
diff --git a/src/openvpn/networking.h b/src/openvpn/networking.h
index cf967116..4075ed73 100644
--- a/src/openvpn/networking.h
+++ b/src/openvpn/networking.h
@@ -39,6 +39,18 @@ net_ctx_init(struct context *c, openvpn_net_ctx_t *ctx)
{
return 0;
}
+
+static inline void
+net_ctx_reset(openvpn_net_ctx_t *ctx)
+{
+(void)ctx;
+}
+
+static inline void
+net_ctx_free(openvpn_net_ctx_t *ctx)
+{
+(void)ctx;
+}
#endif
#if defined(ENABLE_SITNL) || defined(ENABLE_IPROUTE)
@@ -53,6 +65,20 @@ net_ctx_init(struct context *c, openvpn_net_ctx_t *ctx)
*/
int net_ctx_init(struct context *c, openvpn_net_ctx_t *ctx);
+/**
+ * Release resources allocated by the internal garbage collector
+ *
+ * @param ctx the implementation specific context
+ */
+void net_ctx_reset(openvpn_net_ctx_t *ctx);
+
+/**
+ * Release all resources allocated within the platform specific context object
+ *
+ * @param ctx the implementation specific context to release
+ */
+void net_ctx_free(openvpn_net_ctx_t *ctx);
+
/**
* Bring interface up or down.
*
diff --git a/src/openvpn/networking_iproute2.c
b/src/openvpn/networking_iproute2.c
index 5db9a78b..1ddeb5cf 100644
--- a/src/openvpn/networking_iproute2.c
+++ b/src/openvpn/networking_iproute2.c
@@ -43,10 +43,23 @@ net_ctx_init(struct context *c, openvpn_net_ctx_t *ctx)
ctx->es = NULL;
if (c)
ctx->es = c->es;
+ctx->gc = gc_new();
return 0;
}
+void
+net_ctx_reset(openvpn_net_ctx_t *ctx)
+{
+gc_reset(&ctx->gc);
+}
+
+void
+net_ctx_free(openvpn_net_ctx_t *ctx)
+{
+gc_free(&ctx->gc);
+}
+
int
net_iface_up(openvpn_net_ctx_t *ctx, const char *iface, bool up)
{
@@ -82,17 +95,14 @@ net_addr_v4_add(openvpn_net_ctx_t *ctx, const char *iface,
{
struct argv argv = argv_new();
-char *addr_str = (char *)print_in_addr_t(*addr, 0, NULL);
-char *brd_str = (char *)print_in_addr_t(*broadcast, 0, NULL);
+const char *addr_str = print_in_addr_t(*addr, 0, &ctx->gc);
+const char *brd_str = print_in_addr_t(*broadcast, 0, &ctx->gc);
argv_printf(&argv, "%s addr add dev %s %s/%d broadcast %s", iproute_path,
iface, addr_str, prefixlen, brd_str);
argv_msg(M_INFO, &argv);
openvpn_execve_check(&argv, ctx->es, S_FATAL, "Linux ip addr add failed");
-free(addr_str);
-free(brd_str);
-
argv_reset(&argv);
return 0;
@@ -103,7 +113,7 @@ net_addr_v6_add(openvpn_net_ctx_t *ctx, const char *iface,
const struct in6_addr *addr, int prefixlen)
{
struct argv argv = argv_new();
-char *addr_str = (char *)print_in6_addr(*addr, 0, NULL);
+char *addr_str = (char *)print_in6_addr(*addr, 0, &ctx->gc);
argv_printf(&argv, "%s -6 addr add %s/%d dev %s", iproute_path, addr_str,
prefixlen, iface);
@@ -111,8 +121,6 @@ net_addr_v6_add(openvpn_net_ctx_t *ctx, const char *iface,
openvpn_execve_check(&argv, ctx->es, S_FATAL,
"Linux ip -6 addr add failed");
-free(addr_str);
-
argv_reset(&argv);
return 0;
@@ -123,7 +131,7 @@ net_addr_v4_del(openvpn_net_ctx_t *ctx, const char *iface,
const in_addr_t *addr, int prefixlen)
{
struct argv argv = argv_new();
-char *addr_str = (char *)print_in_addr_t(*addr, 0, NULL);
+const char *addr_str = print_in_addr_t(*addr, 0, &ctx->gc);
argv_printf(&argv, "%s addr del dev %s %s/%d", iproute_path, iface,
addr_str, prefixlen);
@@ -131,8 +139,6 @@ net_addr_v4_del(openvpn_net_ctx_t *ctx, const char *iface,
argv_msg(M_INFO, &argv);
openvpn_execve_check(&argv, ctx->es, 0, "Linux ip addr del failed");
-free(addr_str);
-
argv_reset(&argv);
[Openvpn-devel] [PATCH] mbedtls: fix segfault by calling mbedtls_cipher_free() in cipher_ctx_free()
Commit ("openssl: Fix compilation without deprecated OpenSSL 1.1 APIs")
has removed the cipher_ctx_cleanup() API, as it is not anymore required
to be a distinct call. However, while doing so it also touched the
mbedtls backend in a wrong way causing a systematic segfault upon connection.
Basically mbedtls_cipher_free(ctx) was moved from the defunct
cipher_ctx_cleanup()
to md_ctx_free(), while it was supposed to go into cipher_ctx_free().
This was clearly wrong as also the type of the ctx variable was not
correct anymore.
Fix this mistake by actually moving mbedtls_cipher_free(ctx) to
cipher_ctx_free().
Signed-off-by: Antonio Quartulli
---
src/openvpn/crypto_mbedtls.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/openvpn/crypto_mbedtls.c b/src/openvpn/crypto_mbedtls.c
index f924323d..648a988e 100644
--- a/src/openvpn/crypto_mbedtls.c
+++ b/src/openvpn/crypto_mbedtls.c
@@ -591,6 +591,7 @@ cipher_ctx_new(void)
void
cipher_ctx_free(mbedtls_cipher_context_t *ctx)
{
+mbedtls_cipher_free(ctx);
free(ctx);
}
@@ -855,7 +856,6 @@ md_ctx_new(void)
void
md_ctx_free(mbedtls_md_context_t *ctx)
{
-mbedtls_cipher_free(ctx);
free(ctx);
}
--
2.22.1
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
