Re: [Openvpn-devel] [PATCH v2] Allow management to kill client instances by CN wildcard

2020-08-14 Thread Selva Nair 
Hi

On Fri, Aug 14, 2020 at 3:06 PM Vladislav Grishenko
 wrote:
>
> Hi,
>
> Yes, killing a client with cn ending in * will also lead to killing all the
> clients whose cn starts with that prefix.
> Use other char would no-intuitive (ex. +).
> What about optional "prefix" mode word for explicit mode (can be also
> enhanced one day with suffix/regexp/etc).
>
> kill cn [mode]: Kill the client instance(s) having common name cn.

That sounds good to me -- avoids the use of any special character.

Also, updating the "help" command of management interface was missed
in the previous version of the patch.

Selva

>
> --
> Best Regards, Vladislav Grishenko
>
> -Original Message-
> From: Selva Nair 
> Sent: Friday, August 14, 2020 11:22 PM
> To: openvpn-devel 
> Subject: Re: [Openvpn-devel] [PATCH v2] Allow management to kill client
> instances by CN wildcard
>
> Hi
>
> On Fri, Aug 14, 2020 at 1:36 PM Arne Schwabe  wrote:
> >
> > Am 14.08.20 um 19:12 schrieb Vladislav Grishenko:
> > > In case of some permanent part of common name (ex. domain) and/or
> > > long complex common name consisting of multiple x509 fields, it's
> > > handly to kill client instances via management interface with just
> > > prefix of common name, not by exact match only.
> > >
> > > Patch allows to use asterisk as wildcard placeholder in the last
> > > trailing symbol of kill command parameter.
> > > Single asterisk - empty prefix would be too greedy and can be too
> > > harmful, therefore not allowed. Wildcards in the middle of parameter
> > > string are not supported to keep the the things simple at the moment.
> > >
> > > v2: fine tune comments
> > >
> >
> > Thanks for v2,
> >
> > Acked-By; Arne Schwabe 
>
> '*' is an allowed character in x509 common name unless we explicitly forbid
> it. So killing a client with name ending in * would get tricky if not
> impossible without side effects.
>
> Selva
>
>
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>


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

[Openvpn-devel] OpenVPN 2.5-beta1 released

2020-08-14 Thread Samuli Seppänen 
The OpenVPN community project team is proud to release OpenVPN
2.5-beta1. Source code and Windows installers can be downloaded from



Debian and Ubuntu packages are available in the official apt repositories:



On RedHat derivatives we recommend using the Fedora Copr repository:



This is a new major release with many new features:

- Client-specific tls-crypt keys (--tls-crypt-v2)
- Added support for using the ChaCha20-Poly1305 cipher in the
OpenVPN data channel
- Improved Data channel cipher negotiation
- Removal of BF-CBC support in default configuration
- Asynchronous (deferred) authentication support for auth-pam plugin
- Deferred client-connect
- Faster connection setup
- Netlink support
- Wintun support
- IPv6-only operation
- Improved Windows 10 detection
- Linux VRF support
- TLS 1.3 support
- Support setting DHCP search domain
- Handle setting of tun/tap interface MTU on Windows
- HMAC based auth-token support
- VLAN support
- Support building of .msi installers for Windows
- Allow unicode search string in --cryptoapicert option (Windows)
- Support IPv4 configs with /31 netmasks now
- New option --block-ipv6 to reject all IPv6 packets (ICMPv6)
- IPv4-only VPN

More details on these new features as well as a list of deprecated
features and user-visible changes are available in Changes.rst:



For generic help use these support channels:

Official documentation:

Wiki: 
Forums: 
User mailing list: 
User IRC channel: #openvpn at irc.freenode.net

Please report bugs and ask development questions here:

Bug tracker and wiki: 
Developer mailing list: 
Developer IRC channel: #openvpn-devel at irc.freenode.net (requires
Freenode registration)




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] Allow management to kill client instances by CN wildcard

2020-08-14 Thread Vladislav Grishenko 
Hi,

Yes, killing a client with cn ending in * will also lead to killing all the
clients whose cn starts with that prefix.
Use other char would no-intuitive (ex. +).
What about optional "prefix" mode word for explicit mode (can be also
enhanced one day with suffix/regexp/etc).

kill cn [mode]: Kill the client instance(s) having common name cn.

--
Best Regards, Vladislav Grishenko

-Original Message-
From: Selva Nair  
Sent: Friday, August 14, 2020 11:22 PM
To: openvpn-devel 
Subject: Re: [Openvpn-devel] [PATCH v2] Allow management to kill client
instances by CN wildcard

Hi

On Fri, Aug 14, 2020 at 1:36 PM Arne Schwabe  wrote:
>
> Am 14.08.20 um 19:12 schrieb Vladislav Grishenko:
> > In case of some permanent part of common name (ex. domain) and/or 
> > long complex common name consisting of multiple x509 fields, it's 
> > handly to kill client instances via management interface with just 
> > prefix of common name, not by exact match only.
> >
> > Patch allows to use asterisk as wildcard placeholder in the last 
> > trailing symbol of kill command parameter.
> > Single asterisk - empty prefix would be too greedy and can be too 
> > harmful, therefore not allowed. Wildcards in the middle of parameter 
> > string are not supported to keep the the things simple at the moment.
> >
> > v2: fine tune comments
> >
>
> Thanks for v2,
>
> Acked-By; Arne Schwabe 

'*' is an allowed character in x509 common name unless we explicitly forbid
it. So killing a client with name ending in * would get tricky if not
impossible without side effects.

Selva


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



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

Re: [Openvpn-devel] [PATCH] Fix client's poor man NCP fallback

2020-08-14 Thread tincanteksup 

Hi,

I tested this patch and it does make --data-ciphers and 
--data-ciphers-fallback behave in their intended "fashion".


Unfortunately, the commit message is grammatically incorrect and also 
logically misleading.


The intended fashion is for --data-ciphers to recognise that the correct 
cipher *has* been chosen and use it accordingly.


And for --data-ciphers-fallback to *not*

be used in situations other than no OCC cipher.



Reported-by: Richard Bonhomme 
Tested-by: Richard Bonhomme 


On 14/08/2020 09:06, Arne Schwabe wrote:

OpenVPN 2.5 clients do not correctly do a fallback to the server server.
This commit fixes that logic and also fixes --data-ciphers-fallback to
be used in situations other than no OCC cipher.

To reproduce the error use a client with only --data-ciphers set against
a server without NCP.

 OPTIONS ERROR: failed to negotiate cipher with server.
 Add the server's cipher  ('AES-256-CBC') to --data-ciphers
 (currently 'AES-256-CBC') if you want to connect to this server.

Reported by: Richard Bonhomme 

Signed-off-by: Arne Schwabe 
---
  src/openvpn/ssl_ncp.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index f522b8f0..c9ab85ce 100644
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -296,13 +296,14 @@ check_pull_client_ncp(struct context *c, const int found)
  }
  /* If the server did not push a --cipher, we will switch to the
   * remote cipher if it is in our ncp-ciphers list */
-bool useremotecipher = tls_poor_mans_ncp(&c->options,
- 
c->c2.tls_multi->remote_ciphername);
-
+if(tls_poor_mans_ncp(&c->options, c->c2.tls_multi->remote_ciphername))
+{
+return true;
+}
  
  /* We could not figure out the peer's cipher but we have fallback

   * enabled */
-if (!useremotecipher && c->options.enable_ncp_fallback)
+if (!c->c2.tls_multi->remote_ciphername && c->options.enable_ncp_fallback)
  {
  return true;
  }




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

Re: [Openvpn-devel] [PATCH v2] Allow management to kill client instances by CN wildcard

2020-08-14 Thread Selva Nair 
Hi

On Fri, Aug 14, 2020 at 1:36 PM Arne Schwabe  wrote:
>
> Am 14.08.20 um 19:12 schrieb Vladislav Grishenko:
> > In case of some permanent part of common name (ex. domain) and/or
> > long complex common name consisting of multiple x509 fields, it's
> > handly to kill client instances via management interface with just
> > prefix of common name, not by exact match only.
> >
> > Patch allows to use asterisk as wildcard placeholder in the last
> > trailing symbol of kill command parameter.
> > Single asterisk - empty prefix would be too greedy and can be too
> > harmful, therefore not allowed. Wildcards in the middle of
> > parameter string are not supported to keep the the things simple at the 
> > moment.
> >
> > v2: fine tune comments
> >
>
> Thanks for v2,
>
> Acked-By; Arne Schwabe 

'*' is an allowed character in x509 common name unless we explicitly
forbid it. So killing a client with name ending in * would get tricky
if not impossible without side effects.

Selva


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

Re: [Openvpn-devel] [PATCH v2] Allow management to kill client instances by CN wildcard

2020-08-14 Thread Arne Schwabe 
Am 14.08.20 um 19:12 schrieb Vladislav Grishenko:
> In case of some permanent part of common name (ex. domain) and/or
> long complex common name consisting of multiple x509 fields, it's
> handly to kill client instances via management interface with just
> prefix of common name, not by exact match only.
> 
> Patch allows to use asterisk as wildcard placeholder in the last
> trailing symbol of kill command parameter.
> Single asterisk - empty prefix would be too greedy and can be too
> harmful, therefore not allowed. Wildcards in the middle of
> parameter string are not supported to keep the the things simple at the 
> moment.
> 
> v2: fine tune comments
>

Thanks for v2,

Acked-By; Arne Schwabe 



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] Allow management to kill client instances by CN wildcard

2020-08-14 Thread Vladislav Grishenko 
In case of some permanent part of common name (ex. domain) and/or
long complex common name consisting of multiple x509 fields, it's
handly to kill client instances via management interface with just
prefix of common name, not by exact match only.

Patch allows to use asterisk as wildcard placeholder in the last
trailing symbol of kill command parameter.
Single asterisk - empty prefix would be too greedy and can be too
harmful, therefore not allowed. Wildcards in the middle of
parameter string are not supported to keep the the things simple at the moment.

v2: fine tune comments

Signed-off-by: Vladislav Grishenko 
---
 doc/management-notes.txt |  2 ++
 src/openvpn/multi.c  | 15 ++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index 61daaf07..91073693 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -195,6 +195,8 @@ Command examples:
 
   kill Test-Client -- kill the client instance having a
   common name of "Test-Client".
+  kill Test-Cli*   -- kill the client instances having a
+  common name starting with "Test-Cli".
   kill 1.2.3.4:4000 -- kill the client instance having a
source address and port of 1.2.3.4:4000
 
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 13738180..36be5de2 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -3820,6 +3820,19 @@ management_callback_kill_by_cn(void *arg, const char 
*del_cn)
 struct hash_element *he;
 int count = 0;
 
+/* Check passed string for non-empty prefix with trailing asterisk */
+size_t len = strlen(del_cn);
+if (len > 1 && del_cn[len - 1] == '*')
+{
+/* Exclude trailing asterisk from string comparison */
+len--;
+}
+else
+{
+/* Include terminating NUL char to perform exact string comparison */
+len++;
+}
+
 hash_iterator_init(m->iter, &hi);
 while ((he = hash_iterator_next(&hi)))
 {
@@ -3827,7 +3840,7 @@ management_callback_kill_by_cn(void *arg, const char 
*del_cn)
 if (!mi->halt)
 {
 const char *cn = tls_common_name(mi->context.c2.tls_multi, false);
-if (cn && !strcmp(cn, del_cn))
+if (cn && !strncmp(cn, del_cn, len))
 {
 multi_signal_instance(m, mi, SIGTERM);
 ++count;
-- 
2.17.1



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

[Openvpn-devel] [PATCH v3 3/3] Implement generating data channel keys via EKM/RFC 5705

2020-08-14 Thread Arne Schwabe 
OpenVPN currently uses its own (based on TLS 1.0) key derivation
mechanism to generate the 256 bytes key data in key2 struct that
are then used used to generate encryption/hmac/iv vectors. While
this mechanism is still secure, it is not state of the art.

Instead of modernising our own approach, this commit implements
key derivation using the Keying Material Exporters API introduced
by RFC 5705.

We also use an opportunistic approach of negotiating the use of
EKM (exported key material) through an IV_PROTO flag and prefer
EKM to our own PRF if both client and server support it. The
use of EKM is pushed to the client as part of NCP as
key-derivation tls-ekm.

We still exchange the random data (112 bytes from client to server
and 64 byte from server to client) for the OpenVPN PRF but
do not use it. Removing that exchange would break the handshake
and make a key-method 3 or similar necessary.

As a side effect, this makes a little bit easier to have a FIPS compatible
version of OpenVPN since we do not rely on calling MD5 anymore.

Side note: this commit breaks the (not yet merged) WolfSSL support as it
claims to support EKM in the OpenSSL compat API but always returns an error
if you try to use it.

Signed-off-by: Arne Schwabe 

Patch V2: rebase/change to V2 of EKM refactoring

Patch V3: add Changes.rst

Signed-off-by: Arne Schwabe 
---
 Changes.rst  | 11 +++
 doc/doxygen/doc_key_generation.h | 14 ++--
 src/openvpn/crypto.h |  4 +++
 src/openvpn/init.c   |  1 +
 src/openvpn/multi.c  |  4 +++
 src/openvpn/options.c| 14 
 src/openvpn/options.h|  3 ++
 src/openvpn/push.c   |  5 ++-
 src/openvpn/ssl.c| 55 +---
 src/openvpn/ssl.h|  2 ++
 src/openvpn/ssl_backend.h|  2 ++
 src/openvpn/ssl_mbedtls.c|  7 ++--
 12 files changed, 109 insertions(+), 13 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index 0aee3603..4c652315 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -1,3 +1,14 @@
+Overview of changes in 2.6
+==
+
+
+New features
+
+Keying Material Exporters (RFC 5705) based key generation
+As part of the cipher negotiation OpenVPN will automatically prefer
+the RFC5705 based key material generation to the current custom
+OpenVPN PRF. This feature requires OpenSSL or mbed TLS 2.18+.
+
 Overview of changes in 2.5
 ==
 
diff --git a/doc/doxygen/doc_key_generation.h b/doc/doxygen/doc_key_generation.h
index 4bb9c708..cef773a9 100644
--- a/doc/doxygen/doc_key_generation.h
+++ b/doc/doxygen/doc_key_generation.h
@@ -58,6 +58,12 @@
  *
  * @subsection key_generation_method_2 Key method 2
  *
+ * There are two methods for generating key data when using key method 2
+ * the first is OpenVPN's traditional approach that exchanges random
+ * data and uses a PRF and the other is using the RFC5705 keying material
+ * exporter to generate the key material. For both methods the random
+ * data is exchange but only used in the traditional method.
+ *
  * -# The client generates random material in the following amounts:
  *- Pre-master secret: 48 bytes
  *- Client's PRF seed for master secret: 32 bytes
@@ -73,8 +79,12 @@
  *server's random material.
  *
  * %Key method 2 %key expansion is performed by the \c
- * generate_key_expansion() function.  Please refer to its source code for
- * details of the %key expansion process.
+ * generate_key_expansion_oepnvpn_prf() function.  Please refer to its source
+ * code for details of the %key expansion process.
+ *
+ * When the client sends the IV_PROTO_TLS_KEY_EXPORT flag and the server 
replies
+ * with `key-derivation tls-ekm` the RFC5705 key material exporter with the
+ * label EXPORTER-OpenVPN-datakeys is used for the key data.
  *
  * @subsection key_generation_random Source of random material
  *
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 999f643e..ec935ca5 100644
--- a/src/openvpn/crypto.h
+++ b/src/openvpn/crypto.h
@@ -254,6 +254,10 @@ struct crypto_options
 #define CO_MUTE_REPLAY_WARNINGS (1<<2)
 /**< Bit-flag indicating not to display
  *   replay warnings. */
+#define CO_USE_TLS_KEY_MATERIAL_EXPORT  (1<<3)
+/**< Bit-flag indicating that key derivation
+ * is done using TLS keying material export [RFC5705]
+ */
 unsigned int flags; /**< Bit-flags determining behavior of
  *   security operation functions. */
 };
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index dfa045b0..34a7313e 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -676,6 +676,7 @@ restore_ncp_options(struct context *c)
 c->options.ciphername = c->c1.ciphername;
 c->options.authname = c->c1.authname;
 c->options.keysize = c->c1.keysize;
+c->options.data_channel_use_ekm = false;
 }
 
 void
diff --git a/src/openvpn/multi.c b/src/openvpn/m

[Openvpn-devel] [PATCH v3 1/3] Refactor key_state_export_keying_material functions

2020-08-14 Thread Arne Schwabe 
This refactors the common code between mbed SSL and OpenSSL into
export_user_keying_material and also prepares the backend functions
to export more than one key.

Also fix checking the return value of SSL_export_keying_material
only 1 is a sucess, -1 is also an error.

Signed-off-by: Arne Schwabe 

Patch V2: Cache secrets for mbed TLS instead generating all ekms
  in the call back function

Patch V3: comment is no longer a lie. (fixed doxygen)

Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl.c | 36 ++-
 src/openvpn/ssl_backend.h | 20 +++
 src/openvpn/ssl_mbedtls.c | 73 ---
 src/openvpn/ssl_mbedtls.h | 12 +--
 src/openvpn/ssl_openssl.c | 43 +--
 5 files changed, 114 insertions(+), 70 deletions(-)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index f16114c2..3fcaa25f 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2412,6 +2412,40 @@ error:
 return false;
 }
 
+static void
+export_user_keying_material(struct key_state_ssl *ssl,
+struct tls_session *session)
+{
+if (session->opt->ekm_size > 0)
+{
+unsigned int size = session->opt->ekm_size;
+struct gc_arena gc = gc_new();
+
+unsigned char *ekm;
+if ((ekm = key_state_export_keying_material(session,
+session->opt->ekm_label,
+
session->opt->ekm_label_size,
+session->opt->ekm_size,
+&gc)))
+{
+unsigned int len = (size * 2) + 2;
+
+const char *key = format_hex_ex(ekm, size, len, 0, NULL, &gc);
+setenv_str(session->opt->es, "exported_keying_material", key);
+
+dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s",
+ __func__, key);
+secure_memzero(ekm, size);
+}
+else
+{
+msg(M_WARN, "WARNING: Export keying material failed!");
+setenv_del(session->opt->es, "exported_keying_material");
+}
+gc_free(&gc);
+}
+}
+
 /**
  * Handle reading key data, peer-info, username/password, OCC
  * from the TLS control channel (cleartext).
@@ -2541,7 +2575,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
 if ((ks->authenticated > KS_AUTH_FALSE)
 && plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL))
 {
-key_state_export_keying_material(&ks->ks_ssl, session);
+export_user_keying_material(&ks->ks_ssl, session);
 
 if (plugin_call(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL, NULL, 
NULL, session->opt->es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
 {
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index 7f52ab1e..cf9fba25 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -394,13 +394,21 @@ void backend_tls_ctx_reload_crl(struct tls_root_ctx 
*ssl_ctx,
  * derived from existing TLS channel. This exported keying material can then be
  * used for a variety of purposes.
  *
- * @param ks_ssl   The SSL channel's state info
  * @param session  The session associated with the given key_state
- */
-
-void
-key_state_export_keying_material(struct key_state_ssl *ks_ssl,
- struct tls_session *session) 
__attribute__((nonnull));
+ * @param labelThe label to use when exporting the key
+ * @param label_size   The size of the label to use when exporting the key
+ * @param ekm_size THe size of the exported/returned key material
+ * @param gc   gc_arena that might be used to allocate the string
+ * returned
+ * @returnsThe exported key material, the caller may zero the
+ * string but should not free it
+ */
+
+unsigned char*
+key_state_export_keying_material(struct tls_session *session,
+ const char* label, size_t label_size,
+ size_t ekm_size,
+ struct gc_arena *gc) __attribute__((nonnull));
 
 /**/
 /** @addtogroup control_tls
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 9c874788..4287b59e 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -206,51 +206,54 @@ mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned 
char *ms,
 {
 struct tls_session *session = p_expkey;
 struct key_state_ssl *ks_ssl = &session->key[KS_PRIMARY].ks_ssl;
-unsigned char client_server_random[64];
+struct tls_key_cache *cache = &ks_ssl->tls_key_cache;
 
-ks_ssl->exported_key_material = gc_malloc(session->opt->ekm_size,
-  true, NULL);
+static_assert(sizeof(

Re: [Openvpn-devel] [PATCH 1/2] Support multiple x509 field list to be username

2020-08-14 Thread Arne Schwabe 
Am 28.07.20 um 00:13 schrieb Vladislav Grishenko:
> OpenVPN has the ability to choose different x509 field in case "CN"
> can't be use used to be unique connected username since commit
> 935c62be9c0c8a256112df818bfb8470586a23b6.
> Unfortunately it's not enough in case client has multiple and valid
> certificates from PKI for different devices (ex. laptop, mobile, etc)
> with the same CN/UID.
> 
> Having --duplicate-cn as a workaround helps only partially - clients
> can be connected, but it breaks coexistance with --ifconfig-pool-persist,
> --client-config-dir and opens doors to DoS possibility since same client
> device (with the same cert) being reconnected doesn't replace previously
> connected session no more, so can exhaust server ressources (ex. address
> pool) and can prevent other clients to be connected.
> 
> With this patch, multiple x509 fields incl. "serialNumber" can be chosen
> to be username as --x509-username-files space-separated parameters.
> Multiple fields will be joined into one username using '/' delimeter for
> consistency with CN/addr logging and preserving ability for hierarchical
> ccd. As long as resulting username is unique, --duplicate-cn will not
> be required. Default value is preserved as "CN" only.
> 
> Openssl backend is the only supported at the moment, since so far MbedTLS
> has no alt user name support at all.
> ---
>  doc/man-sections/tls-options.rst |  9 ---
>  src/openvpn/init.c   |  4 +--
>  src/openvpn/options.c| 46 ++--
>  src/openvpn/options.h|  4 +--
>  src/openvpn/ssl.h|  1 +
>  src/openvpn/ssl_common.h |  8 +-
>  src/openvpn/ssl_verify.c | 35 
>  src/openvpn/ssl_verify_openssl.c | 15 +++
>  8 files changed, 83 insertions(+), 39 deletions(-)
> 
> diff --git a/doc/man-sections/tls-options.rst 
> b/doc/man-sections/tls-options.rst
> index 8c2db7cd..301f8be4 100644
> --- a/doc/man-sections/tls-options.rst
> +++ b/doc/man-sections/tls-options.rst
> @@ -632,13 +632,13 @@ certificates and keys: 
> https://github.com/OpenVPN/easy-rsa
>options can be defined to track multiple attributes.
>  
>  --x509-username-field args
> -  Field in the X.509 certificate subject to be used as the username
> +  Field list in the X.509 certificate subject to be used as the username
>(default :code:`CN`).
>  
>Valid syntax:
>::
>  
> - x509-username-field [ext:]fieldname
> + x509-username-field [ext:]fieldname [[ext:]fieldname...]
>  
>Typically, this option is specified with **fieldname** as
>either of the following:
> @@ -646,6 +646,7 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
>  
>   x509-username-field emailAddress
>   x509-username-field ext:subjectAltName
> + x509-username-field CN serialNumber
>  
>The first example uses the value of the :code:`emailAddress` attribute
>in the certificate's Subject field as the username. The second example
> @@ -653,7 +654,9 @@ certificates and keys: https://github.com/OpenVPN/easy-rsa
>``fieldname`` :code:`subjectAltName` be searched for an rfc822Name
>(email) field to be used as the username. In cases where there are
>multiple email addresses in :code:`ext:fieldname`, the last occurrence
> -  is chosen.
> +  is chosen. The last example uses the value of :code:`CN` attribute in
> +  the Subject field and the hex representation of certificate's serial
> +  number delimited by slash symbol as the resulting username.
>  
>When this option is used, the ``--verify-x509-name`` option will match
>against the chosen ``fieldname`` instead of the Common Name.
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 1ea4735d..11b417a8 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -2912,9 +2912,9 @@ do_init_crypto_tls(struct context *c, const unsigned 
> int flags)
>  to.verify_hash = options->verify_hash;
>  to.verify_hash_algo = options->verify_hash_algo;
>  #ifdef ENABLE_X509ALTUSERNAME
> -to.x509_username_field = (char *) options->x509_username_field;
> +memmove(to.x509_username_field, options->x509_username_field, 
> sizeof(to.x509_username_field));
>  #else
> -to.x509_username_field = X509_USERNAME_FIELD_DEFAULT;
> +to.x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT;
>  #endif

Can't we use the same code in both cases? And why memmove instead memcpy?


>  to.es = c->c2.es;
>  to.net_ctx = &c->net_ctx;
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index bc256b18..a51038dd 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -877,7 +877,7 @@ init_options(struct options *o, const bool init_gc)
>  o->tls_cert_profile = NULL;
>  o->ecdh_curve = NULL;
>  #ifdef ENABLE_X509ALTUSERNAME
> -o->x509_username_field = X509_USERNAME_FIELD_DEFAULT;
> +o->x509_username_field[0] = X509_USERNAME_FIELD_DEFAULT;
>  #endif
>  #ifdef ENAB

[Openvpn-devel] [PATCH applied] Re: Fix stack overflow in OpenSolaris NEXTADDR()

2020-08-14 Thread Gert Doering 
Patch has been applied to the master, 2.5 and 2.4 branch (bugfix).

commit 7e65483d1227adfb855844467e4d30894ffc355d (master)
commit 7b9dd9b091a3cad126642314ea945bafa4e91481 (release/2.5)
commit 5f88c077de8da4a4c5369ae67f5815e4abc50edc (release/2.4)
Author: Gert Doering
Date:   Thu Aug 13 12:13:01 2020 +0200

 Fix stack overflow in OpenSolaris NEXTADDR()

 Signed-off-by: Gert Doering 
 Acked-by: Arne Schwabe 
 Message-Id: <20200813101301.12720-1-g...@greenie.muc.de>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20731.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 2/2] Allow killing of client instances by cn with wildcards

2020-08-14 Thread Arne Schwabe 

>  int count = 0;
>  
> +/* Allow trailing wildcard */
> +int len = strlen(del_cn);
> +len += (len > 1 && del_cn[len-1] == '*') ? -1 : 1;

This is very compact and not very readable

A comment that says why you are adding +1 would be good.

I first thought it was incorrect and went through godbolt to figure out
the '*' string is longer so its prefix is short and normal string needs
the \0 instead: https://godbolt.org/z/vqsPeW


> +
>  hash_iterator_init(m->iter, &hi);
>  while ((he = hash_iterator_next(&hi)))
>  {
> @@ -3779,7 +3783,7 @@ management_callback_kill_by_cn(void *arg, const char 
> *del_cn)
>  if (!mi->halt)
>  {
>  const char *cn = tls_common_name(mi->context.c2.tls_multi, 
> false);
> -if (cn && !strcmp(cn, del_cn))
> +if (cn && !strncmp(cn, del_cn, len))
>  {
>  multi_signal_instance(m, mi, SIGTERM);
>  ++count;
> 

Feature-Ack. But I would like the string len magic be documented/written
in a better understandable 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] Fix stack overflow in OpenSolaris NEXTADDR()

2020-08-14 Thread Arne Schwabe 
Am 13.08.20 um 12:13 schrieb Gert Doering:
> Commit 5fde831c5807 fixed NEXTADDR() for all *BSDs and MacOS.
> 
> OpenSolaris has to use a slightly different macro due to lack of
> sockaddr->sa_len - but it has the same problem, first rounding up,
> then memmove()'ing.  Switch order.
> 
> Signed-off-by: Gert Doering 
> ---
>  src/openvpn/route.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/openvpn/route.c b/src/openvpn/route.c
> index 24563ed6..f127a90a 100644
> --- a/src/openvpn/route.c
> +++ b/src/openvpn/route.c
> @@ -3429,7 +3429,7 @@ struct rtmsg {
>  #if defined(TARGET_SOLARIS)
>  #define NEXTADDR(w, u) \
>  if (rtm_addrs & (w)) { \
> -l = ROUNDUP(sizeof(u)); memmove(cp, &(u), l); cp += l; \
> +l = sizeof(u); memmove(cp, &(u), l); cp += ROUNDUP(l); \
>  }
>  
>  #define ADVANCE(x, n) (x += ROUNDUP(sizeof(struct sockaddr_in)))
> 

Acked-By: Arne Schwabe 



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] Fix client's poor man NCP fallback

2020-08-14 Thread Arne Schwabe 
OpenVPN 2.5 clients do not correctly do a fallback to the server server.
This commit fixes that logic and also fixes --data-ciphers-fallback to
be used in situations other than no OCC cipher.

To reproduce the error use a client with only --data-ciphers set against
a server without NCP.

OPTIONS ERROR: failed to negotiate cipher with server.
Add the server's cipher  ('AES-256-CBC') to --data-ciphers
(currently 'AES-256-CBC') if you want to connect to this server.

Reported by: Richard Bonhomme 

Signed-off-by: Arne Schwabe 
---
 src/openvpn/ssl_ncp.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/openvpn/ssl_ncp.c b/src/openvpn/ssl_ncp.c
index f522b8f0..c9ab85ce 100644
--- a/src/openvpn/ssl_ncp.c
+++ b/src/openvpn/ssl_ncp.c
@@ -296,13 +296,14 @@ check_pull_client_ncp(struct context *c, const int found)
 }
 /* If the server did not push a --cipher, we will switch to the
  * remote cipher if it is in our ncp-ciphers list */
-bool useremotecipher = tls_poor_mans_ncp(&c->options,
- 
c->c2.tls_multi->remote_ciphername);
-
+if(tls_poor_mans_ncp(&c->options, c->c2.tls_multi->remote_ciphername))
+{
+return true;
+}
 
 /* We could not figure out the peer's cipher but we have fallback
  * enabled */
-if (!useremotecipher && c->options.enable_ncp_fallback)
+if (!c->c2.tls_multi->remote_ciphername && c->options.enable_ncp_fallback)
 {
 return true;
 }
-- 
2.26.2



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