openvpn-devel — OpenVPN developers list

Re: [Openvpn-devel] [PATCH 1/2] Send auth-fail messages to clients on renegotiation failures via auth-token or user-pass expiry

[Eric T. <er...@sp...>]

Hi Arne,

The issue is your state is not accessible from where that boolean needs 
to be used unless I am missing something? Please advise if I'm mistaken 
or of another route.

Eric

---
Eric Thorpe
SparkLabs Developer
https://www.sparklabs.com
https://twitter.com/sparklabs
su...@sp...

On 13/08/2020 6:30 pm, Arne Schwabe wrote:
>>   /*
>>    * Send restart message from server to client.
>>    */
>> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
>> index 96897e48..b5cc9dc9 100644
>> --- a/src/openvpn/ssl_common.h
>> +++ b/src/openvpn/ssl_common.h
>> @@ -576,6 +576,7 @@ struct tls_multi
>>   
>>       char *remote_ciphername;    /**< cipher specified in peer's config file */
>>   
>> +    bool connection_established; /** Notifies future auth calls this is a reneg */
>>       /*
>>        * Our session objects.
>>        */
>>
> NAK from my side. Adding another variable to state machine for just this
> feature that duplicates already existing states is something I really
> want to avoid. This might look and work fine for this patch but at the
> end you end up with a plenthora of mini states and unclear
> interdependency. I worked hard in the connect-client patches to remove
> duplication of these states and are not eager to reduce them, especially
> not "connection_established" which duplicates the name of
> link_socket->connection_established that has a completely different meaning.
>
> Arne
>

Re: [Openvpn-devel] [PATCH 1/2] Send auth-fail messages to clients on renegotiation failures via auth-token or user-pass expiry

[Arne S. <ar...@rf...>]

>  /*
>   * Send restart message from server to client.
>   */
> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
> index 96897e48..b5cc9dc9 100644
> --- a/src/openvpn/ssl_common.h
> +++ b/src/openvpn/ssl_common.h
> @@ -576,6 +576,7 @@ struct tls_multi
>  
>      char *remote_ciphername;    /**< cipher specified in peer's config file */
>  
> +    bool connection_established; /** Notifies future auth calls this is a reneg */
>      /*
>       * Our session objects.
>       */
>

NAK from my side. Adding another variable to state machine for just this
feature that duplicates already existing states is something I really
want to avoid. This might look and work fine for this patch but at the
end you end up with a plenthora of mini states and unclear
interdependency. I worked hard in the connect-client patches to remove
duplication of these states and are not eager to reduce them, especially
not "connection_established" which duplicates the name of
link_socket->connection_established that has a completely different meaning.

Arne

[Openvpn-devel] [PATCH 1/2] Send auth-fail messages to clients on renegotiation failures via auth-token or user-pass expiry

[Eric T. <er...@sp...>]

Signed-off-by: Eric Thorpe <er...@sp...>
---
 src/openvpn/multi.c      |  2 ++
 src/openvpn/push.c       | 30 ++++++++++++++++++++++++++++++
 src/openvpn/ssl_common.h |  1 +
 src/openvpn/ssl_verify.c |  7 +++++++
 4 files changed, 40 insertions(+)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 13738180..288680c9 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2689,6 +2689,8 @@ multi_connection_established(struct multi_context *m, struct multi_instance *mi)
 
         mi->context.c2.context_auth = CAS_FAILED;
     }
+    /* Set connection established for reneg handling */
+    mi->context.c2.tls_multi->connection_established = true;
 
     /* increment number of current authenticated clients */
     ++m->n_clients;
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index e0d2eeaf..3567b22d 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -311,6 +311,36 @@ send_auth_pending_messages(struct context *c, const char *extra)
     return true;
 }
 
+/*
+* Send auth failed message from server to client without scheduling.
+* Main use for queuing a message during renegotiation
+*/
+void
+send_push_reply_auth_failed(struct tls_multi *multi, const char *client_reason)
+{
+    struct gc_arena gc = gc_new();
+    static const char auth_failed[] = "AUTH_FAILED";
+    size_t len;
+
+    len = (client_reason ? strlen(client_reason)+1 : 0) + sizeof(auth_failed);
+    if (len > PUSH_BUNDLE_SIZE)
+    {
+        len = PUSH_BUNDLE_SIZE;
+    }
+
+    {
+        struct buffer buf = alloc_buf_gc(len, &gc);
+        buf_printf(&buf, auth_failed);
+        if (client_reason)
+        {
+            buf_printf(&buf, ",%s", client_reason);
+        }
+        send_control_channel_string_dowork(multi, BSTR(&buf), D_PUSH);
+    }
+
+    gc_free(&gc);
+}
+
 /*
  * Send restart message from server to client.
  */
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index 96897e48..b5cc9dc9 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -576,6 +576,7 @@ struct tls_multi
 
     char *remote_ciphername;    /**< cipher specified in peer's config file */
 
+    bool connection_established; /** Notifies future auth calls this is a reneg */
     /*
      * Our session objects.
      */
diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 97ccb93b..8d8531c7 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1318,6 +1318,7 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
         }
         else
         {
+            send_push_reply_auth_failed(multi, "SESSION:Auth-token expired");
             wipe_auth_token(multi);
             ks->authenticated = KS_AUTH_FALSE;
             msg(M_WARN, "TLS: Username/auth-token authentication "
@@ -1432,6 +1433,12 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
     }
     else
     {
+        if (multi->connection_established)
+        {
+            /* Notify the client */
+            send_push_reply_auth_failed(multi, "SESSION:Auth failed");
+
+        }
         ks->authenticated = KS_AUTH_FALSE;
         msg(D_TLS_ERRORS, "TLS Auth Error: Auth Username/Password verification failed for peer");
     }
-- 
2.25.1

Re: [Openvpn-devel] [PATCH v2 1/2] Send auth fail to client on reneg failure

[Eric T. <er...@sp...>]

Have re-based to master and resent.

Cheers,
Eric

---
Eric Thorpe
SparkLabs Developer
https://www.sparklabs.com
https://twitter.com/sparklabs
su...@sp...

On 11/08/2020 7:32 pm, Gert Doering wrote:
> Hi,
>
> On Thu, Apr 11, 2019 at 12:07:27PM +1000, Eric Thorpe wrote:
>> This patch relies on Arne's "Add send_control_channel_string_dowork
>> variant" patch.
> So.  After we left you out in the cold for over a year (apologies
> for that), we should revisit your patch set.
>
> I checked with Arne, he says "no, that functionality is not in yet,
> but still interesting" - and all the *other* auth/plugin/async/SSO
> related code we had brewing is in.
>
> So - if you're not too frustrated with us yet: can you re-send?
>
> gert

[Openvpn-devel] [PATCH 2/2] Allows a plugin to provide a client_reason for authentication failure

[Eric T. <er...@sp...>]

Signed-off-by: Eric Thorpe <er...@sp...>
---
 src/openvpn/ssl_verify.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
index 8d8531c7..356b30fd 100644
--- a/src/openvpn/ssl_verify.c
+++ b/src/openvpn/ssl_verify.c
@@ -1157,6 +1157,9 @@ verify_user_pass_plugin(struct tls_session *session, struct tls_multi *multi,
     /* Is username defined? */
     if ((session->opt->ssl_flags & SSLF_AUTH_USER_PASS_OPTIONAL) || strlen(up->username))
     {
+        struct plugin_return pr, prfetch;
+        plugin_return_init(&pr);
+
         /* set username/password in private env space */
         setenv_str(session->opt->es, "username", up->username);
         setenv_str(session->opt->es, "password", up->password);
@@ -1180,7 +1183,23 @@ verify_user_pass_plugin(struct tls_session *session, struct tls_multi *multi,
 #endif
 
         /* call command */
-        retval = plugin_call(session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, NULL, session->opt->es);
+        retval = plugin_call(session->opt->plugins, OPENVPN_PLUGIN_AUTH_USER_PASS_VERIFY, NULL, &pr, session->opt->es);
+
+        /* Fetch client reason */
+        plugin_return_get_column(&pr, &prfetch, "client_reason");
+        if (plugin_return_defined(&prfetch))
+        {
+            int i;
+            for (i = 0; i < prfetch.n; ++i)
+            {
+                if (prfetch.list[i] && prfetch.list[i]->value)
+                {
+                    auth_set_client_reason(multi, prfetch.list[i]->value);
+                }
+            }
+        }
+
+        plugin_return_free(&pr);
 
 #ifdef PLUGIN_DEF_AUTH
         /* purge auth control filename (and file itself) for non-deferred returns */
@@ -1436,8 +1455,16 @@ verify_user_pass(struct user_pass *up, struct tls_multi *multi,
         if (multi->connection_established)
         {
             /* Notify the client */
-            send_push_reply_auth_failed(multi, "SESSION:Auth failed");
-
+            const char *client_reason;
+            if (multi->client_reason != NULL)
+            {
+                client_reason = multi->client_reason;
+            }
+            else
+            {
+                client_reason = "SESSION:Auth failed";
+            }
+            send_push_reply_auth_failed(multi, client_reason);
         }
         ks->authenticated = KS_AUTH_FALSE;
         msg(D_TLS_ERRORS, "TLS Auth Error: Auth Username/Password verification failed for peer");
-- 
2.25.1

Re: [Openvpn-devel] [PATCH v2 1/3] Refactor key_state_export_keying_material functions

[Steffan K. <st...@ka...>]

Hi,

On 12-08-2020 16:01, Arne Schwabe wrote:
> 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 <ar...@rf...>
> 
> Patch V2: Cache secrets for mbed TLS instead generating all ekms
>           in the call back function
> 
> Signed-off-by: Arne Schwabe <ar...@rf...>
> ---
>
>  [...]
> 
> --- a/src/openvpn/ssl_backend.h
> +++ b/src/openvpn/ssl_backend.h
> @@ -394,13 +394,23 @@ 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.
>   *
> + *

This extra newline seems unneeded.

>   * @param ks_ssl       The SSL channel's state info
>   * @param session      The session associated with the given key_state
> + * @param label        The label to use when exporting the key
> + * @param label_size   The size of the label to use when exporting the key
> + *
> + * @param gc           gc_arena that might be used to allocate the string
> + *                     returned
> + * @returns            The exported key material, the caller may zero the
> + *                     string but should not free it
>   */
>  
> -void
> -key_state_export_keying_material(struct key_state_ssl *ks_ssl,
> -                                 struct tls_session *session) __attribute__((nonnull));
> +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));

(Quoting a former colleague:) comment is a lie! It does not match the
actual function prototype.

All the code itself looks good to me. I *think* it could be made
slightly better even, but these changes make sense for the intended
goal, and look correct.

Tested both openssl and mbedtls builds (with -fsanitize=address), and
verified that this indeed produces the same exported values for both
builds and a build without this patch.

-Steffan

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

[tincanteksup <tin...@gm...>]

typos/grammar

On 12/08/2020 15:01, Arne Schwabe wrote:
> OpenVPN currently uses its own (based on TLS 1.0) key derivation
> mechansim to generate the 256 bytes key data in key2 struct that

mechansim -> mechanism


> 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 modernisating our own approach, this commit implements

modernisating - > modernising


> 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) that for the OpenVPN PRF but

client) that for - unnecessary 'that'

> do not use it. Removing that exchange would break the handshake
> and make a key-method 3 or similar necessary.
> 
> 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 <ar...@rf...>
> 
> Patch V2: rebase/change to V2 of EKM refactoring
> ---
>   Changes.rst                      | 11 +++++++
>   doc/doxygen/doc_key_generation.h | 15 ++++++++--
>   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                | 50 ++++++++++++++++++++++++++++++--
>   src/openvpn/ssl.h                |  2 ++
>   src/openvpn/ssl_backend.h        |  2 ++
>   src/openvpn/ssl_mbedtls.c        |  7 ++---
>   12 files changed, 107 insertions(+), 11 deletions(-)
> 
> diff --git a/Changes.rst b/Changes.rst
> index bacc98cd..2c28a399 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..bbd6c0c5 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,13 @@
>    *    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 and the server replies
> + * with `key-derivation tls-ekm` RFC5705 key material exporter with the label
> + * EXPORTER-OpenVPN-datakeys is used for the key data.

Grammar: split the sentence by a 'then'.

 > + * When the client sends the IV_PROTO_TLS_KEY_EXPORT and the server 
replies
 > + * with `key-derivation tls-ekm` then RFC5705 key material exporter 
with the label
 > + * EXPORTER-OpenVPN-datakeys is used for the key data.

Re: [Openvpn-devel] [PATCH v2 1/3] Refactor key_state_export_keying_material functions

[tincanteksup <tin...@gm...>]

typo

On 12/08/2020 15:01, Arne Schwabe wrote:
> 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.

sucess -> success

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

[Arne S. <ar...@rf...>]

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 <ar...@rf...>

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

Signed-off-by: Arne Schwabe <ar...@rf...>
---
 src/openvpn/ssl.c         | 36 ++++++++++++++++++-
 src/openvpn/ssl_backend.h | 16 +++++++--
 src/openvpn/ssl_mbedtls.c | 73 ++++++++++++++++++++-------------------
 src/openvpn/ssl_mbedtls.h | 12 +++++--
 src/openvpn/ssl_openssl.c | 43 +++++++++--------------
 5 files changed, 113 insertions(+), 67 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..eb7d541d 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -394,13 +394,23 @@ 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
+ * @param label        The label to use when exporting the key
+ * @param label_size   The size of the label to use when exporting the key
+ *
+ * @param gc           gc_arena that might be used to allocate the string
+ *                     returned
+ * @returns            The exported key material, the caller may zero the
+ *                     string but should not free it
  */
 
-void
-key_state_export_keying_material(struct key_state_ssl *ks_ssl,
-                                 struct tls_session *session) __attribute__((nonnull));
+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(ks_ssl->ctx->session->master)
+                    == sizeof(cache->master_secret), "master size mismatch");
 
-    memcpy(client_server_random, client_random, 32);
-    memcpy(client_server_random + 32, server_random, 32);
+    memcpy(cache->client_server_random, client_random, 32);
+    memcpy(cache->client_server_random + 32, server_random, 32);
+    memcpy(cache->master_secret, ms, sizeof(cache->master_secret));
+    cache->tls_prf_type = tls_prf_type;
 
-    const size_t ms_len = sizeof(ks_ssl->ctx->session->master);
-    int ret = mbedtls_ssl_tls_prf(tls_prf_type, ms, ms_len,
-                                  session->opt->ekm_label, client_server_random,
-                                  sizeof(client_server_random), ks_ssl->exported_key_material,
-                                  session->opt->ekm_size);
-
-    if (!mbed_ok(ret))
-    {
-        secure_memzero(ks_ssl->exported_key_material, session->opt->ekm_size);
-    }
-
-    secure_memzero(client_server_random, sizeof(client_server_random));
-
-    return ret;
+    return true;
 }
-#endif /* HAVE_EXPORT_KEYING_MATERIAL */
 
-void
-key_state_export_keying_material(struct key_state_ssl *ssl,
-                                 struct tls_session *session)
+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)
 {
-    if (ssl->exported_key_material)
+    ASSERT(strlen(label) == label_size);
+
+    struct tls_key_cache *cache = &session->key[KS_PRIMARY].ks_ssl.tls_key_cache;
+
+    /* If the type is NONE, we either have no cached secrets or
+     * there is no PRF, in both cases we cannot generate key material */
+    if (cache->tls_prf_type == MBEDTLS_SSL_TLS_PRF_NONE)
     {
-        unsigned int size = session->opt->ekm_size;
-        struct gc_arena gc = gc_new();
-        unsigned int len = (size * 2) + 2;
+        return NULL;
+    }
 
-        const char *key = format_hex_ex(ssl->exported_key_material,
-                                        size, len, 0, NULL, &gc);
-        setenv_str(session->opt->es, "exported_keying_material", key);
+    unsigned char *ekm = (unsigned char *) gc_malloc(ekm_size, true, gc);
+    int ret = mbedtls_ssl_tls_prf(cache->tls_prf_type, cache->master_secret,
+                                  sizeof(cache->master_secret),
+                                  label, cache->client_server_random,
+                                  sizeof(cache->client_server_random),
+                                  ekm, ekm_size);
 
-        dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s",
-             __func__, key);
-        gc_free(&gc);
+    if (mbed_ok(ret))
+    {
+        return ekm;
+    }
+    else
+    {
+        secure_memzero(ekm, session->opt->ekm_size);
+        return  NULL;
     }
 }
-
+#endif /* HAVE_EXPORT_KEYING_MATERIAL */
 
 bool
 tls_ctx_set_options(struct tls_root_ctx *ctx, unsigned int ssl_flags)
@@ -1178,7 +1181,7 @@ key_state_ssl_free(struct key_state_ssl *ks_ssl)
 {
     if (ks_ssl)
     {
-        free(ks_ssl->exported_key_material);
+        CLEAR(ks_ssl->tls_key_cache);
 
         if (ks_ssl->ctx)
         {
diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h
index 0525134f..17aae551 100644
--- a/src/openvpn/ssl_mbedtls.h
+++ b/src/openvpn/ssl_mbedtls.h
@@ -82,6 +82,15 @@ struct external_context {
     void *sign_ctx;
 };
 
+/** struct to cache TLS secrets for keying material exporter (RFC 5705).
+ * The constants (64 and 48) are inherent to TLS version and
+ * the whole keying material export will likely change when they change */
+struct tls_key_cache {
+    unsigned char client_server_random[64];
+    mbedtls_tls_prf_types tls_prf_type;
+    unsigned char master_secret[48];
+};
+
 /**
  * Structure that wraps the TLS context. Contents differ depending on the
  * SSL library used.
@@ -114,8 +123,7 @@ struct key_state_ssl {
     mbedtls_ssl_context *ctx;           /**< mbedTLS connection context */
     bio_ctx *bio_ctx;
 
-    /** Keying material exporter cache (RFC 5705). */
-    uint8_t *exported_key_material;
+    struct tls_key_cache tls_key_cache;
 
 };
 
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 5ba74402..f52c7c39 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -158,35 +158,26 @@ tls_ctx_initialised(struct tls_root_ctx *ctx)
     return NULL != ctx->ctx;
 }
 
-void
-key_state_export_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 = (unsigned char *) gc_malloc(size, true, &gc);
+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)
 
-        if (SSL_export_keying_material(ssl->ssl, ekm, size,
-                                       session->opt->ekm_label,
-                                       session->opt->ekm_label_size,
-                                       NULL, 0, 0))
-        {
-            unsigned int len = (size * 2) + 2;
+{
+    unsigned char *ekm = (unsigned char *) gc_malloc(ekm_size, true, gc);
 
-            const char *key = format_hex_ex(ekm, size, len, 0, NULL, &gc);
-            setenv_str(session->opt->es, "exported_keying_material", key);
+    SSL* ssl = session->key[KS_PRIMARY].ks_ssl.ssl;
 
-            dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s",
-                 __func__, key);
-        }
-        else
-        {
-            msg(M_WARN, "WARNING: Export keying material failed!");
-            setenv_del(session->opt->es, "exported_keying_material");
-        }
-        gc_free(&gc);
+    if (SSL_export_keying_material(ssl, ekm, ekm_size, label,
+                                   label_size, NULL, 0, 0) == 1)
+    {
+        return ekm;
+    }
+    else
+    {
+        secure_memzero(ekm, ekm_size);
+        return NULL;
     }
 }
 
-- 
2.26.2

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

[Arne S. <ar...@rf...>]

OpenVPN currently uses its own (based on TLS 1.0) key derivation
mechansim 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 modernisating 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) that 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.

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 <ar...@rf...>

Patch V2: rebase/change to V2 of EKM refactoring
---
 Changes.rst                      | 11 +++++++
 doc/doxygen/doc_key_generation.h | 15 ++++++++--
 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                | 50 ++++++++++++++++++++++++++++++--
 src/openvpn/ssl.h                |  2 ++
 src/openvpn/ssl_backend.h        |  2 ++
 src/openvpn/ssl_mbedtls.c        |  7 ++---
 12 files changed, 107 insertions(+), 11 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index bacc98cd..2c28a399 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..bbd6c0c5 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,13 @@
  *    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 and the server replies
+ * with `key-derivation tls-ekm` 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/multi.c
index 13738180..a5862020 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1817,6 +1817,10 @@ multi_client_set_protocol_options(struct context *c)
         c->c2.push_request_received = true;
     }
 
+#ifdef HAVE_EXPORT_KEYING_MATERIAL
+    o->data_channel_use_ekm = (proto & IV_PROTO_TLS_KEY_EXPORT);
+#endif
+
     /* Select cipher if client supports Negotiable Crypto Parameters */
     if (!o->ncp_enabled)
     {
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 8bf82c57..90e78a7b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7947,6 +7947,20 @@ add_option(struct options *options,
         }
         options->ncp_ciphers = p[1];
     }
+    else if (streq(p[0], "key-derivation") && p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_NCP)
+#ifdef HAVE_EXPORT_KEYING_MATERIAL
+        if (streq(p[1], "tls-ekm"))
+        {
+            options->data_channel_use_ekm = true;
+        }
+        else
+#endif
+        {
+            msg(msglevel, "Unknown key-derivation method %s", p[1]);
+        }
+    }
     else if (streq(p[0], "ncp-disable") && !p[1])
     {
         VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 877e9396..c730c6a7 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -648,6 +648,9 @@ struct options
     /* Useful when packets sent by openvpn itself are not subject
      * to the routing tables that would move packets into the tunnel. */
     bool allow_recursive_routing;
+
+    /* Use RFC 5705 key export */
+    bool data_channel_use_ekm;
 };
 
 #define streq(x, y) (!strcmp((x), (y)))
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index e0d2eeaf..17bba948 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -479,7 +479,10 @@ prepare_push_reply(struct context *c, struct gc_arena *gc,
     {
         push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername);
     }
-
+    if (o->data_channel_use_ekm)
+    {
+        push_option_fmt(gc, push_list, M_USAGE, "key-derivation tls-ekm");
+    }
     return true;
 }
 
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 06cc4c0b..946d841a 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1784,6 +1784,29 @@ init_key_contexts(struct key_ctx_bi *key,
 }
 
 
+static bool
+generate_key_expansion_tls_export(struct tls_session *session, struct key2 *key2)
+{
+    struct gc_arena gc = gc_new();
+    unsigned char *key2data;
+
+    key2data = key_state_export_keying_material(session,
+                                                EXPORT_KEY_DATA_LABEL,
+                                                strlen(EXPORT_KEY_DATA_LABEL),
+                                                EXPORT_KEY_DATA_EKM_SIZE,
+                                                &gc);
+    if (!key2data)
+    {
+        return false;
+    }
+    memcpy(key2->keys, key2data, sizeof(key2->keys));
+    secure_memzero(key2data, sizeof(key2->keys));
+    key2->n = 2;
+
+    gc_free(&gc);
+    return true;
+}
+
 static struct key2
 generate_key_expansion_oepnvpn_prf(const struct tls_session *session)
 {
@@ -1846,7 +1869,7 @@ generate_key_expansion_oepnvpn_prf(const struct tls_session *session)
  */
 static bool
 generate_key_expansion(struct key_ctx_bi *key,
-                       const struct tls_session *session)
+                       struct tls_session *session)
 {
     bool ret = false;
 
@@ -1859,7 +1882,20 @@ generate_key_expansion(struct key_ctx_bi *key,
 
     bool server = session->opt->server;
 
-    struct key2 key2 = generate_key_expansion_oepnvpn_prf(session);
+    struct key2 key2;
+
+    if (session->opt->crypto_flags & CO_USE_TLS_KEY_MATERIAL_EXPORT)
+    {
+        if(!generate_key_expansion_tls_export(session, &key2))
+        {
+            msg(D_TLS_ERRORS, "TLS Error: Keying material export failed");
+            goto exit;
+        }
+    }
+    else
+    {
+        key2 = generate_key_expansion_oepnvpn_prf(session);
+    }
 
     key2_print(&key2, &session->opt->key_type,
                "Master Encrypt", "Master Decrypt");
@@ -1988,6 +2024,11 @@ tls_session_update_crypto_params(struct tls_session *session,
         session->opt->crypto_flags |= CO_PACKET_ID_LONG_FORM;
     }
 
+    if (options->data_channel_use_ekm)
+    {
+        session->opt->crypto_flags |= CO_USE_TLS_KEY_MATERIAL_EXPORT;
+    }
+
     /* Update frame parameters: undo worst-case overhead, add actual overhead */
     frame_remove_from_extra_frame(frame, crypto_max_overhead());
     crypto_adjust_frame_parameters(frame, &session->opt->key_type,
@@ -2244,10 +2285,13 @@ push_peer_info(struct buffer *buf, struct tls_session *session)
          * push request, also signal that the client wants
          * to get push-reply messages without without requiring a round
          * trip for a push request message*/
-        if(session->opt->pull)
+        if (session->opt->pull)
         {
             iv_proto |= IV_PROTO_REQUEST_PUSH;
         }
+#ifdef HAVE_EXPORT_KEYING_MATERIAL
+        iv_proto |= IV_PROTO_TLS_KEY_EXPORT;
+#endif
 
         buf_printf(&out, "IV_PROTO=%d\n", iv_proto);
 
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 005628f6..f00f8abd 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -116,6 +116,8 @@
  * to wait for a push-request to send a push-reply */
 #define IV_PROTO_REQUEST_PUSH   (1<<2)
 
+/** Supports key derivation via TLS key material exporter [RFC5705] */
+#define IV_PROTO_TLS_KEY_EXPORT (1<<3)
 
 /* Default field in X509 to be username */
 #define X509_USERNAME_FIELD_DEFAULT "CN"
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index eb7d541d..2be32244 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -389,6 +389,8 @@ void key_state_ssl_free(struct key_state_ssl *ks_ssl);
 void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx,
                                 const char *crl_file, bool crl_inline);
 
+#define EXPORT_KEY_DATA_LABEL       "EXPORTER-OpenVPN-datakeys"
+#define EXPORT_KEY_DATA_EKM_SIZE    (2 * (MAX_CIPHER_KEY_LENGTH + MAX_HMAC_KEY_LENGTH))
 /**
  * Keying Material Exporters [RFC 5705] allows additional keying material to be
  * derived from existing TLS channel. This exported keying material can then be
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 4287b59e..da19fe38 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -1158,11 +1158,8 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl,
 
 #ifdef HAVE_EXPORT_KEYING_MATERIAL
     /* Initialize keying material exporter */
-    if (session->opt->ekm_size)
-    {
-        mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config,
-                                            mbedtls_ssl_export_keys_cb, session);
-    }
+    mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config,
+                                        mbedtls_ssl_export_keys_cb, session);
 #endif
 
     /* Initialise SSL context */
-- 
2.26.2

[Openvpn-devel] [PATCH v2 2/3] Move openvpn specific key expansion into its own function

[Arne S. <ar...@rf...>]

This moves the OpenVPN specific PRF into its own function also
simplifies the code a bit by passing tls_session directly instead of
5 of its fields.

Signed-off-by: Arne Schwabe <ar...@rf...>

Patch V2: Rebase
---
 src/openvpn/ssl.c | 109 +++++++++++++++++++++++++++++-----------------
 1 file changed, 69 insertions(+), 40 deletions(-)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 3fcaa25f..06cc4c0b 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1765,27 +1765,38 @@ openvpn_PRF(const uint8_t *secret,
     VALGRIND_MAKE_READABLE((void *)output, output_len);
 }
 
-/*
- * Using source entropy from local and remote hosts, mix into
- * master key.
- */
-static bool
-generate_key_expansion(struct key_ctx_bi *key,
-                       const struct key_type *key_type,
-                       const struct key_source2 *key_src,
-                       const struct session_id *client_sid,
-                       const struct session_id *server_sid,
-                       bool server)
+static void
+init_key_contexts(struct key_ctx_bi *key,
+                  const struct key_type *key_type,
+                  bool server,
+                  struct key2 *key2)
+{
+    /* Initialize OpenSSL key contexts */
+    int key_direction = server ? KEY_DIRECTION_INVERSE : KEY_DIRECTION_NORMAL;
+    init_key_ctx_bi(key, key2, key_direction, key_type, "Data Channel");
+
+    /* Initialize implicit IVs */
+    key_ctx_update_implicit_iv(&key->encrypt, (*key2).keys[(int)server].hmac,
+                               MAX_HMAC_KEY_LENGTH);
+    key_ctx_update_implicit_iv(&key->decrypt, (*key2).keys[1-(int)server].hmac,
+                               MAX_HMAC_KEY_LENGTH);
+
+}
+
+
+static struct key2
+generate_key_expansion_oepnvpn_prf(const struct tls_session *session)
 {
+
     uint8_t master[48] = { 0 };
-    struct key2 key2 = { 0 };
-    bool ret = false;
 
-    if (key->initialized)
-    {
-        msg(D_TLS_ERRORS, "TLS Error: key already initialized");
-        goto exit;
-    }
+    const struct key_state *ks = &session->key[KS_PRIMARY];
+    const struct key_source2 *key_src = ks->key_src;
+
+    const struct session_id *client_sid = session->opt->server ?
+                                          &ks->session_id_remote : &session->session_id;
+    const struct session_id *server_sid = !session->opt->server ?
+                                          &ks->session_id_remote : &session->session_id;
 
     /* debugging print of source key material */
     key_source2_print(key_src);
@@ -1803,6 +1814,7 @@ generate_key_expansion(struct key_ctx_bi *key,
                 master,
                 sizeof(master));
 
+    struct key2 key2;
     /* compute key expansion */
     openvpn_PRF(master,
                 sizeof(master),
@@ -1815,41 +1827,62 @@ generate_key_expansion(struct key_ctx_bi *key,
                 server_sid,
                 (uint8_t *)key2.keys,
                 sizeof(key2.keys));
+    secure_memzero(&master, sizeof(master));
 
+    /* We use the DES fixup here so we can drop it once we
+     * drop DES support and non RFC5705 key derivation */
+    for (int i = 0; i < 2; ++i)
+    {
+        fixup_key(&key2.keys[i], &session->opt->key_type);
+    }
     key2.n = 2;
 
-    key2_print(&key2, key_type, "Master Encrypt", "Master Decrypt");
+    return key2;
+}
+
+/*
+ * Using source entropy from local and remote hosts, mix into
+ * master key.
+ */
+static bool
+generate_key_expansion(struct key_ctx_bi *key,
+                       const struct tls_session *session)
+{
+    bool ret = false;
+
+    if (key->initialized)
+    {
+        msg(D_TLS_ERRORS, "TLS Error: key already initialized");
+        goto exit;
+    }
+
+
+    bool server = session->opt->server;
+
+    struct key2 key2 = generate_key_expansion_oepnvpn_prf(session);
+
+    key2_print(&key2, &session->opt->key_type,
+               "Master Encrypt", "Master Decrypt");
 
     /* check for weak keys */
     for (int i = 0; i < 2; ++i)
     {
-        fixup_key(&key2.keys[i], key_type);
-        if (!check_key(&key2.keys[i], key_type))
+        if (!check_key(&key2.keys[i], &session->opt->key_type))
         {
             msg(D_TLS_ERRORS, "TLS Error: Bad dynamic key generated");
             goto exit;
         }
     }
-
-    /* Initialize OpenSSL key contexts */
-    int key_direction = server ? KEY_DIRECTION_INVERSE : KEY_DIRECTION_NORMAL;
-    init_key_ctx_bi(key, &key2, key_direction, key_type, "Data Channel");
-
-    /* Initialize implicit IVs */
-    key_ctx_update_implicit_iv(&key->encrypt, key2.keys[(int)server].hmac,
-                               MAX_HMAC_KEY_LENGTH);
-    key_ctx_update_implicit_iv(&key->decrypt, key2.keys[1-(int)server].hmac,
-                               MAX_HMAC_KEY_LENGTH);
-
+    init_key_contexts(key, &session->opt->key_type, server, &key2);
     ret = true;
 
 exit:
-    secure_memzero(&master, sizeof(master));
     secure_memzero(&key2, sizeof(key2));
 
     return ret;
 }
 
+
 static void
 key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len)
 {
@@ -1879,10 +1912,7 @@ tls_session_generate_data_channel_keys(struct tls_session *session)
 {
     bool ret = false;
     struct key_state *ks = &session->key[KS_PRIMARY];   /* primary key */
-    const struct session_id *client_sid = session->opt->server ?
-                                          &ks->session_id_remote : &session->session_id;
-    const struct session_id *server_sid = !session->opt->server ?
-                                          &ks->session_id_remote : &session->session_id;
+
 
     if (ks->authenticated == KS_AUTH_FALSE)
     {
@@ -1891,9 +1921,8 @@ tls_session_generate_data_channel_keys(struct tls_session *session)
     }
 
     ks->crypto_options.flags = session->opt->crypto_flags;
-    if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi,
-                                &session->opt->key_type, ks->key_src, client_sid, server_sid,
-                                session->opt->server))
+
+    if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi, session))
     {
         msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed");
         goto cleanup;
-- 
2.26.2

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

[Steffan K. <ste...@fo...>]

Hi,

Couldn't resist giving this a quick look.

Feature-ACK on the patch set, but some comments on the approach:

On 12-08-2020 10:55, Arne Schwabe wrote:
> 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 <ar...@rf...>
> ---
>  src/openvpn/ssl.c         | 33 ++++++++++++++++++++++++-
>  src/openvpn/ssl_backend.h | 18 ++++++++++++--
>  src/openvpn/ssl_mbedtls.c | 22 ++++++-----------
>  src/openvpn/ssl_openssl.c | 51 +++++++++++++++++++++------------------
>  4 files changed, 83 insertions(+), 41 deletions(-)
> 
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index f16114c2..390114e1 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -2412,6 +2412,37 @@ 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(ssl, session,
> +                                                    EXPORT_KEY_USER, &gc)))
> +        {

Hmm, I don't think these abstractions are right. I would argue that the
crypto backends should know as few about OpenVPN specifics as possible.
So things like "EXPORT_KEY_USER" should probably not be passed to the
backend, and instead be handled in ssl.c.

Why not give the backend a prototype like

  key_state_export_keying_material(label, label_len, ekm, ekm_len)

I understand that this would mean an extra memcpy() for the mbedtls
code, but this is not performance-critical at all, right?

(In the follow-up patch, just cache the master secret and client/server
random, instead of the derived key material in the mbedtls backend, so
you can generate the export at will. For OpenSSL, this API should be
trivial.)

> +            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 +2572,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..8faaefd5 100644
> --- a/src/openvpn/ssl_backend.h
> +++ b/src/openvpn/ssl_backend.h
> @@ -389,18 +389,32 @@ void key_state_ssl_free(struct key_state_ssl *ks_ssl);
>  void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx,
>                                  const char *crl_file, bool crl_inline);
>  
> +
> +/* defines the different RFC5705 that are used in OpenVPN */

Make this /** to have it turn up nicely in the doxygen.

> +enum export_key_identifier {
> +    EXPORT_KEY_USER
> +};
> +
>  /**
>   * Keying Material Exporters [RFC 5705] allows additional keying material to be
>   * derived from existing TLS channel. This exported keying material can then be
>   * used for a variety of purposes.
>   *
> + * Note
> + *

Note ... what?

>   * @param ks_ssl       The SSL channel's state info
>   * @param session      The session associated with the given key_state
> + * @param key          The key to export.
> + * @param gc           gc_arena that might be used to allocate a string

Where "a string" is "the keying material", right? Maybe just say that :)

> + * @returns            The exported key material, the caller may zero the
> + *                     string but should not free it
>   */
>  
> -void
> +unsigned char*
>  key_state_export_keying_material(struct key_state_ssl *ks_ssl,
> -                                 struct tls_session *session) __attribute__((nonnull));
> +                                 struct tls_session *session,
> +                                 enum export_key_identifier export_key,
> +                                 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..8ae6ec7b 100644
> --- a/src/openvpn/ssl_mbedtls.c
> +++ b/src/openvpn/ssl_mbedtls.c
> @@ -231,24 +231,18 @@ mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned char *ms,
>  }
>  #endif /* HAVE_EXPORT_KEYING_MATERIAL */
>  
> -void
> +unsigned char *
>  key_state_export_keying_material(struct key_state_ssl *ssl,
> -                                 struct tls_session *session)
> +                                 struct tls_session *session,
> +                                 enum export_key_identifier key_id,
> +                                 struct gc_arena *gc)
>  {
> -    if (ssl->exported_key_material)
> +    if (key_id == EXPORT_KEY_USER)
>      {
> -        unsigned int size = session->opt->ekm_size;
> -        struct gc_arena gc = gc_new();
> -        unsigned int len = (size * 2) + 2;
> -
> -        const char *key = format_hex_ex(ssl->exported_key_material,
> -                                        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);
> -        gc_free(&gc);
> +        return ssl->exported_key_material;
>      }
> +
> +    return  NULL;
>  }
>  
>  
> diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
> index 5ba74402..1aad4f89 100644
> --- a/src/openvpn/ssl_openssl.c
> +++ b/src/openvpn/ssl_openssl.c
> @@ -158,35 +158,38 @@ tls_ctx_initialised(struct tls_root_ctx *ctx)
>      return NULL != ctx->ctx;
>  }
>  
> -void
> +unsigned char *
>  key_state_export_keying_material(struct key_state_ssl *ssl,
> -                                 struct tls_session *session)
> +                                 struct tls_session *session,
> +                                 enum export_key_identifier key_id,
> +                                 struct gc_arena *gc)
> +
>  {
> -    if (session->opt->ekm_size > 0)
> -    {
> -        unsigned int size = session->opt->ekm_size;
> -        struct gc_arena gc = gc_new();
> -        unsigned char *ekm = (unsigned char *) gc_malloc(size, true, &gc);
> +    const char *label;
> +    size_t label_size, ekm_size;
>  
> -        if (SSL_export_keying_material(ssl->ssl, ekm, size,
> -                                       session->opt->ekm_label,
> -                                       session->opt->ekm_label_size,
> -                                       NULL, 0, 0))
> -        {
> -            unsigned int len = (size * 2) + 2;
> +    if (key_id == EXPORT_KEY_USER)
> +    {
> +        label = session->opt->ekm_label;
> +        label_size = session->opt->ekm_label_size;
> +        ekm_size = session->opt->ekm_size;
> +    }
> +    else
> +    {
> +        ASSERT(0);
> +    }
>  
> -            const char *key = format_hex_ex(ekm, size, len, 0, NULL, &gc);
> -            setenv_str(session->opt->es, "exported_keying_material", key);
> +    unsigned char *ekm = (unsigned char *) gc_malloc(ekm_size, true, gc);
>  
> -            dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s",
> -                 __func__, key);
> -        }
> -        else
> -        {
> -            msg(M_WARN, "WARNING: Export keying material failed!");
> -            setenv_del(session->opt->es, "exported_keying_material");
> -        }
> -        gc_free(&gc);
> +    if (SSL_export_keying_material(ssl->ssl, ekm, ekm_size, label,
> +                                   label_size, NULL, 0, 0) == 1)
> +    {
> +        return ekm;
> +    }
> +    else
> +    {
> +        secure_memzero(ekm, ekm_size);
> +        return NULL;
>      }
>  }
>  
> 

-Steffan.

[Openvpn-devel] [PATCH applied] Re: Improve sections about older OpenVPN clients in cipher-negotiation.rst

[Gert D. <ge...@gr...>]

Acked-by: Gert Doering <ge...@gr...>

Good work, Richard and Arne, thanks!

Your patch has been applied to the master branch.

commit 9262f1454d78157226f20b15a374f3c750e19cdd
Author: Arne Schwabe
Date:   Wed Aug 12 10:54:12 2020 +0200

     Improve sections about older OpenVPN clients in cipher-negotiation.rst

     Signed-off-by: Arne Schwabe <ar...@rf...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@rf...>
     URL: https://www.mail-archive.com/ope...@li.../msg20714.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

Re: [Openvpn-devel] [PATCH] Workaround FreeBSD 12+ race condition on tun/tap open with IPv6.

[Arne S. <ar...@rf...>]

Am 23.07.20 um 14:19 schrieb Gert Doering:
> On FreeBSD 12 (tested and verified on 12.1-RELEASE-p2), after "ifconfig
> inet6" for a tun/tap interface, there sometimes is a race condition
> where the "IFDISABLED" flag shows up after a short time frame, under
> a second, and never clears itself.  This disables use of the configured
> IPv6 address on the interface, breaking IPv6 over tun/tap operation.
> 
> This only happens if ipv6_activate_all_interfaces="YES" is not
> set in /etc/rc.conf - but there might be reasons why this is not so.
> 
> As a workaround until this can be fixed on the FreeBSD side (or a
> better workaround is found), sleep(1) after ifconfig, then call
> "ifconfig $dev inet6 -ifdisable".
> 
> Yes, this is massively ugly but makes the problem completely go
> away for my test systems.
> 
> (The same effect can be achieved with an --up script that does this,
> but it's even less pretty - see trac ticket)
> 
> FreeBSD: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=248172
> 
> v2: reword text, refer to FreeBSD bug with much more details

Acked-By: Arne Schwabe <ar...@rf...>

This looks like a ugly workaround but if it is the best we can, so be it.

Arne

[Openvpn-devel] [PATCH] Improve sections about older OpenVPN clients in cipher-negotiation.rst

[Arne S. <ar...@rf...>]

 - Explain the IV_NCP=2 client situation in 2.4 a bit better.
 - Make more clear what exact versions are meant in the old client section
 - add a missing - in a heading

Thanks to Richard Bohnhomme for initial proof reading.

Signed-off-by: Arne Schwabe <ar...@rf...>
---
 doc/man-sections/cipher-negotiation.rst | 34 ++++++++++++++-----------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/doc/man-sections/cipher-negotiation.rst b/doc/man-sections/cipher-negotiation.rst
index 46c9d7cf..b2c20241 100644
--- a/doc/man-sections/cipher-negotiation.rst
+++ b/doc/man-sections/cipher-negotiation.rst
@@ -22,14 +22,18 @@ it is automatically added to this list. If both options are unset the default is
 
 OpenVPN 2.4 clients
 -------------------
-The negotiation support in OpenVPN 2.4 was a first implementation and still had some
-quirks. Its main goal was "upgrade to AES-256-GCM when possible".
+The negotiation support in OpenVPN 2.4 was the first iteration of the implementation
+and still had some quirks. Its main goal was "upgrade to AES-256-GCM when possible".
 An OpenVPN 2.4 client that is built against a crypto library that supports AES in GCM
 mode and does not have ``--ncp-disable`` will always announce support for
-`AES-256-GCM` and `AES-128-GCM` even if the ``--ncp-ciphers`` option does not include
-those two ciphers. It is therefore recommended to add `AES-256-GCM` and `AES-128-GCM`
-to the ``--ncp-ciphers`` options to workaround this bug.
+`AES-256-GCM` and `AES-128-GCM` to a server by sending :code:`IV_NCP=2`.
 
+This only causes a problem if ``--ncp-ciphers`` option has been changed from the
+default of :code:`AES-256-GCM:AES-128-GCM` to a value that does not include
+these two ciphers. When a OpenVPN servers try to use `AES-256-GCM` or
+`AES-128-GCM` the connection will then fail. It is therefore recommended to
+always have the `AES-256-GCM` and `AES-128-GCM` ciphers to the ``--ncp-ciphers``
+options to avoid this behaviour.
 
 OpenVPN 3 clients
 -----------------
@@ -42,7 +46,7 @@ To support OpenVPN 3.x based clients at least one of these ciphers needs to be
 included in the server's ``--data-ciphers`` option.
 
 
-OpenVPN 2.3 clients and older (and clients with ``--ncp-disable``)
+OpenVPN 2.3 and older clients (and clients with ``--ncp-disable``)
 ------------------------------------------------------------------
 When a client without cipher negotiation support connects to a server the
 cipher specified with the ``--cipher`` option in the client configuration
@@ -50,10 +54,10 @@ must be included in the ``--data-ciphers`` option of the server to allow
 the client to connect. Otherwise the client will be sent the ``AUTH_FAILED``
 message that indicates no shared cipher.
 
-If the client has been configured with the ``--enable-small``
-:code:``./configure`` argument, using ``data-ciphers-fallback cipher``
-in the server config file with the explicit cipher used by the client
-is necessary.
+If the client is 2.3 or older and has been configured with the
+``--enable-small``  :code:`./configure` argument, using
+``data-ciphers-fallback cipher`` in the server config file with the explicit
+cipher used by the client is necessary.
 
 OpenVPN 2.4 server
 ------------------
@@ -66,7 +70,7 @@ adding  `AES-128-GCM` and `AES-256-GCM` to the client's ``--data-ciphers``
 option is required. OpenVPN 2.5+ will only announce the ``IV_NCP=2`` flag if
 those ciphers are present.
 
-OpenVPN 2.3 and older servers (and servers with ``-ncp-disable``)
+OpenVPN 2.3 and older servers (and servers with ``--ncp-disable``)
 -----------------------------------------------------------------
 The cipher used by the server must be included in ``--data-ciphers`` to
 allow the client connecting to a server without cipher negotiation
@@ -74,10 +78,10 @@ support.
 (For compatibility OpenVPN 2.5 will also accept the cipher set with
 ``--cipher``)
 
-If the server has been configured with the ``--enable-small``
-:code:``./configure` argument, adding ``data-ciphers-fallback cipher``
-to the client config with the explicit cipher used by the server
-is necessary.
+If the server is 2.3 or older and  has been configured with the
+``--enable-small`` :code:`./configure` argument, adding
+``data-ciphers-fallback cipher`` to the client config with the explicit
+cipher used by the server is necessary.
 
 Blowfish in CBC mode (BF-CBC) deprecation
 ------------------------------------------
-- 
2.26.2

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

[Arne S. <ar...@rf...>]

OpenVPN currently uses its own (based on TLS 1.0) key derivation
mechansim 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 modernisating 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) that 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.

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 <ar...@rf...>
---
 doc/doxygen/doc_key_generation.h | 15 +++++++--
 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                | 49 ++++++++++++++++++++++++++++--
 src/openvpn/ssl.h                |  2 ++
 src/openvpn/ssl_backend.h        |  8 +++--
 src/openvpn/ssl_mbedtls.c        | 52 ++++++++++++++++++++++----------
 src/openvpn/ssl_mbedtls.h        |  3 +-
 src/openvpn/ssl_openssl.c        |  6 ++++
 13 files changed, 141 insertions(+), 25 deletions(-)

diff --git a/doc/doxygen/doc_key_generation.h b/doc/doxygen/doc_key_generation.h
index 4bb9c708..bbd6c0c5 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,13 @@
  *    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 and the server replies
+ * with `key-derivation tls-ekm` 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/multi.c
index 13738180..a5862020 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -1817,6 +1817,10 @@ multi_client_set_protocol_options(struct context *c)
         c->c2.push_request_received = true;
     }
 
+#ifdef HAVE_EXPORT_KEYING_MATERIAL
+    o->data_channel_use_ekm = (proto & IV_PROTO_TLS_KEY_EXPORT);
+#endif
+
     /* Select cipher if client supports Negotiable Crypto Parameters */
     if (!o->ncp_enabled)
     {
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 8bf82c57..90e78a7b 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7947,6 +7947,20 @@ add_option(struct options *options,
         }
         options->ncp_ciphers = p[1];
     }
+    else if (streq(p[0], "key-derivation") && p[1])
+    {
+        VERIFY_PERMISSION(OPT_P_NCP)
+#ifdef HAVE_EXPORT_KEYING_MATERIAL
+        if (streq(p[1], "tls-ekm"))
+        {
+            options->data_channel_use_ekm = true;
+        }
+        else
+#endif
+        {
+            msg(msglevel, "Unknown key-derivation method %s", p[1]);
+        }
+    }
     else if (streq(p[0], "ncp-disable") && !p[1])
     {
         VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_INSTANCE);
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index 877e9396..c730c6a7 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -648,6 +648,9 @@ struct options
     /* Useful when packets sent by openvpn itself are not subject
      * to the routing tables that would move packets into the tunnel. */
     bool allow_recursive_routing;
+
+    /* Use RFC 5705 key export */
+    bool data_channel_use_ekm;
 };
 
 #define streq(x, y) (!strcmp((x), (y)))
diff --git a/src/openvpn/push.c b/src/openvpn/push.c
index e0d2eeaf..17bba948 100644
--- a/src/openvpn/push.c
+++ b/src/openvpn/push.c
@@ -479,7 +479,10 @@ prepare_push_reply(struct context *c, struct gc_arena *gc,
     {
         push_option_fmt(gc, push_list, M_USAGE, "cipher %s", o->ciphername);
     }
-
+    if (o->data_channel_use_ekm)
+    {
+        push_option_fmt(gc, push_list, M_USAGE, "key-derivation tls-ekm");
+    }
     return true;
 }
 
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index e44017c6..b78362f1 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1784,6 +1784,28 @@ init_key_contexts(struct key_ctx_bi *key,
 }
 
 
+static bool
+generate_key_expansion_tls_export(struct tls_session *session, struct key2 *key2)
+{
+    struct key_state *ks = &session->key[KS_PRIMARY];
+    struct gc_arena gc = gc_new();
+    unsigned char *key2data;
+
+    key2data = key_state_export_keying_material(&ks->ks_ssl, session,
+                                                EXPORT_KEY_DATA,
+                                                &gc);
+    if (!key2data)
+    {
+        return false;
+    }
+    memcpy(key2->keys, key2data, sizeof(key2->keys));
+    secure_memzero(key2data, sizeof(key2->keys));
+    key2->n = 2;
+
+    gc_free(&gc);
+    return true;
+}
+
 static struct key2
 generate_key_expansion_oepnvpn_prf(const struct tls_session *session)
 {
@@ -1846,7 +1868,7 @@ generate_key_expansion_oepnvpn_prf(const struct tls_session *session)
  */
 static bool
 generate_key_expansion(struct key_ctx_bi *key,
-                       const struct tls_session *session)
+                       struct tls_session *session)
 {
     bool ret = false;
 
@@ -1859,7 +1881,20 @@ generate_key_expansion(struct key_ctx_bi *key,
 
     bool server = session->opt->server;
 
-    struct key2 key2 = generate_key_expansion_oepnvpn_prf(session);
+    struct key2 key2;
+
+    if (session->opt->crypto_flags & CO_USE_TLS_KEY_MATERIAL_EXPORT)
+    {
+        if(!generate_key_expansion_tls_export(session, &key2))
+        {
+            msg(D_TLS_ERRORS, "TLS Error: Keying material export failed");
+            goto exit;
+        }
+    }
+    else
+    {
+        key2 = generate_key_expansion_oepnvpn_prf(session);
+    }
 
     key2_print(&key2, &session->opt->key_type,
                "Master Encrypt", "Master Decrypt");
@@ -1988,6 +2023,11 @@ tls_session_update_crypto_params(struct tls_session *session,
         session->opt->crypto_flags |= CO_PACKET_ID_LONG_FORM;
     }
 
+    if (options->data_channel_use_ekm)
+    {
+        session->opt->crypto_flags |= CO_USE_TLS_KEY_MATERIAL_EXPORT;
+    }
+
     /* Update frame parameters: undo worst-case overhead, add actual overhead */
     frame_remove_from_extra_frame(frame, crypto_max_overhead());
     crypto_adjust_frame_parameters(frame, &session->opt->key_type,
@@ -2244,10 +2284,13 @@ push_peer_info(struct buffer *buf, struct tls_session *session)
          * push request, also signal that the client wants
          * to get push-reply messages without without requiring a round
          * trip for a push request message*/
-        if(session->opt->pull)
+        if (session->opt->pull)
         {
             iv_proto |= IV_PROTO_REQUEST_PUSH;
         }
+#ifdef HAVE_EXPORT_KEYING_MATERIAL
+        iv_proto |= IV_PROTO_TLS_KEY_EXPORT;
+#endif
 
         buf_printf(&out, "IV_PROTO=%d\n", iv_proto);
 
diff --git a/src/openvpn/ssl.h b/src/openvpn/ssl.h
index 005628f6..f00f8abd 100644
--- a/src/openvpn/ssl.h
+++ b/src/openvpn/ssl.h
@@ -116,6 +116,8 @@
  * to wait for a push-request to send a push-reply */
 #define IV_PROTO_REQUEST_PUSH   (1<<2)
 
+/** Supports key derivation via TLS key material exporter [RFC5705] */
+#define IV_PROTO_TLS_KEY_EXPORT (1<<3)
 
 /* Default field in X509 to be username */
 #define X509_USERNAME_FIELD_DEFAULT "CN"
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index 8faaefd5..e08f6e1e 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -390,11 +390,15 @@ void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx,
                                 const char *crl_file, bool crl_inline);
 
 
-/* defines the different RFC5705 that are used in OpenVPN */
+/* defines the different RFC5705 keys that are used in OpenVPN */
 enum export_key_identifier {
-    EXPORT_KEY_USER
+    EXPORT_KEY_USER,
+    EXPORT_KEY_DATA
 };
 
+#define EXPORT_KEY_DATA_LABEL       "EXPORTER-OpenVPN-datakeys"
+#define EXPORT_KEY_DATA_EKM_SIZE    (2 * (MAX_CIPHER_KEY_LENGTH + MAX_HMAC_KEY_LENGTH))
+
 /**
  * Keying Material Exporters [RFC 5705] allows additional keying material to be
  * derived from existing TLS channel. This exported keying material can then be
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 8ae6ec7b..3721163f 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -208,22 +208,41 @@ mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned char *ms,
     struct key_state_ssl *ks_ssl = &session->key[KS_PRIMARY].ks_ssl;
     unsigned char client_server_random[64];
 
-    ks_ssl->exported_key_material = gc_malloc(session->opt->ekm_size,
-                                              true, NULL);
-
     memcpy(client_server_random, client_random, 32);
     memcpy(client_server_random + 32, server_random, 32);
 
     const size_t ms_len = sizeof(ks_ssl->ctx->session->master);
-    int ret = mbedtls_ssl_tls_prf(tls_prf_type, ms, ms_len,
+
+    int ret;
+    /* Generate user exported key if requested */
+    if (session->opt->ekm_size)
+    {
+
+        ks_ssl->exported_key_material_user = gc_malloc(session->opt->ekm_size,
+                                                       true, NULL);
+        ret = mbedtls_ssl_tls_prf(tls_prf_type, ms, ms_len,
                                   session->opt->ekm_label, client_server_random,
-                                  sizeof(client_server_random), ks_ssl->exported_key_material,
+                                  sizeof(client_server_random),
+                                  ks_ssl->exported_key_material_user,
                                   session->opt->ekm_size);
 
-    if (!mbed_ok(ret))
-    {
-        secure_memzero(ks_ssl->exported_key_material, session->opt->ekm_size);
+        if (!mbed_ok(ret))
+        {
+            secure_memzero(ks_ssl->exported_key_material_user,
+                           session->opt->ekm_size);
+            ks_ssl->exported_key_material_user = NULL;
+        }
     }
+    /* We always generate the data channel key here even if we are not using
+     * it */
+
+    ks_ssl->exported_key_material_data = gc_malloc(EXPORT_KEY_DATA_EKM_SIZE,
+                                                   true, NULL);
+    ret = mbedtls_ssl_tls_prf(tls_prf_type, ms, ms_len,
+                              EXPORT_KEY_DATA_LABEL, client_server_random,
+                              sizeof(client_server_random),
+                              ks_ssl->exported_key_material_data,
+                              EXPORT_KEY_DATA_EKM_SIZE);
 
     secure_memzero(client_server_random, sizeof(client_server_random));
 
@@ -239,9 +258,12 @@ key_state_export_keying_material(struct key_state_ssl *ssl,
 {
     if (key_id == EXPORT_KEY_USER)
     {
-        return ssl->exported_key_material;
+        return ssl->exported_key_material_user;
+    }
+    else if (key_id == EXPORT_KEY_DATA)
+    {
+        return ssl->exported_key_material_data;
     }
-
     return  NULL;
 }
 
@@ -1149,11 +1171,8 @@ key_state_ssl_init(struct key_state_ssl *ks_ssl,
 
 #ifdef HAVE_EXPORT_KEYING_MATERIAL
     /* Initialize keying material exporter */
-    if (session->opt->ekm_size)
-    {
-        mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config,
-                                            mbedtls_ssl_export_keys_cb, session);
-    }
+    mbedtls_ssl_conf_export_keys_ext_cb(ks_ssl->ssl_config,
+                                        mbedtls_ssl_export_keys_cb, session);
 #endif
 
     /* Initialise SSL context */
@@ -1172,7 +1191,8 @@ key_state_ssl_free(struct key_state_ssl *ks_ssl)
 {
     if (ks_ssl)
     {
-        free(ks_ssl->exported_key_material);
+        free(ks_ssl->exported_key_material_user);
+        free(ks_ssl->exported_key_material_data);
 
         if (ks_ssl->ctx)
         {
diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h
index 0525134f..d5652350 100644
--- a/src/openvpn/ssl_mbedtls.h
+++ b/src/openvpn/ssl_mbedtls.h
@@ -115,7 +115,8 @@ struct key_state_ssl {
     bio_ctx *bio_ctx;
 
     /** Keying material exporter cache (RFC 5705). */
-    uint8_t *exported_key_material;
+    uint8_t *exported_key_material_user;
+    uint8_t *exported_key_material_data;
 
 };
 
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 1aad4f89..e807f581 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -174,6 +174,12 @@ key_state_export_keying_material(struct key_state_ssl *ssl,
         label_size = session->opt->ekm_label_size;
         ekm_size = session->opt->ekm_size;
     }
+    else if (key_id == EXPORT_KEY_DATA)
+    {
+        label = EXPORT_KEY_DATA_LABEL;
+        label_size = strlen(EXPORT_KEY_DATA_LABEL);
+        ekm_size = EXPORT_KEY_DATA_EKM_SIZE;
+    }
     else
     {
         ASSERT(0);
-- 
2.26.2

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

[Arne S. <ar...@rf...>]

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 <ar...@rf...>
---
 src/openvpn/ssl.c         | 33 ++++++++++++++++++++++++-
 src/openvpn/ssl_backend.h | 18 ++++++++++++--
 src/openvpn/ssl_mbedtls.c | 22 ++++++-----------
 src/openvpn/ssl_openssl.c | 51 +++++++++++++++++++++------------------
 4 files changed, 83 insertions(+), 41 deletions(-)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index f16114c2..390114e1 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2412,6 +2412,37 @@ 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(ssl, session,
+                                                    EXPORT_KEY_USER, &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 +2572,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..8faaefd5 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -389,18 +389,32 @@ void key_state_ssl_free(struct key_state_ssl *ks_ssl);
 void backend_tls_ctx_reload_crl(struct tls_root_ctx *ssl_ctx,
                                 const char *crl_file, bool crl_inline);
 
+
+/* defines the different RFC5705 that are used in OpenVPN */
+enum export_key_identifier {
+    EXPORT_KEY_USER
+};
+
 /**
  * Keying Material Exporters [RFC 5705] allows additional keying material to be
  * derived from existing TLS channel. This exported keying material can then be
  * used for a variety of purposes.
  *
+ * Note
+ *
  * @param ks_ssl       The SSL channel's state info
  * @param session      The session associated with the given key_state
+ * @param key          The key to export.
+ * @param gc           gc_arena that might be used to allocate a string
+ * @returns            The exported key material, the caller may zero the
+ *                     string but should not free it
  */
 
-void
+unsigned char*
 key_state_export_keying_material(struct key_state_ssl *ks_ssl,
-                                 struct tls_session *session) __attribute__((nonnull));
+                                 struct tls_session *session,
+                                 enum export_key_identifier export_key,
+                                 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..8ae6ec7b 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -231,24 +231,18 @@ mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned char *ms,
 }
 #endif /* HAVE_EXPORT_KEYING_MATERIAL */
 
-void
+unsigned char *
 key_state_export_keying_material(struct key_state_ssl *ssl,
-                                 struct tls_session *session)
+                                 struct tls_session *session,
+                                 enum export_key_identifier key_id,
+                                 struct gc_arena *gc)
 {
-    if (ssl->exported_key_material)
+    if (key_id == EXPORT_KEY_USER)
     {
-        unsigned int size = session->opt->ekm_size;
-        struct gc_arena gc = gc_new();
-        unsigned int len = (size * 2) + 2;
-
-        const char *key = format_hex_ex(ssl->exported_key_material,
-                                        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);
-        gc_free(&gc);
+        return ssl->exported_key_material;
     }
+
+    return  NULL;
 }
 
 
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index 5ba74402..1aad4f89 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -158,35 +158,38 @@ tls_ctx_initialised(struct tls_root_ctx *ctx)
     return NULL != ctx->ctx;
 }
 
-void
+unsigned char *
 key_state_export_keying_material(struct key_state_ssl *ssl,
-                                 struct tls_session *session)
+                                 struct tls_session *session,
+                                 enum export_key_identifier key_id,
+                                 struct gc_arena *gc)
+
 {
-    if (session->opt->ekm_size > 0)
-    {
-        unsigned int size = session->opt->ekm_size;
-        struct gc_arena gc = gc_new();
-        unsigned char *ekm = (unsigned char *) gc_malloc(size, true, &gc);
+    const char *label;
+    size_t label_size, ekm_size;
 
-        if (SSL_export_keying_material(ssl->ssl, ekm, size,
-                                       session->opt->ekm_label,
-                                       session->opt->ekm_label_size,
-                                       NULL, 0, 0))
-        {
-            unsigned int len = (size * 2) + 2;
+    if (key_id == EXPORT_KEY_USER)
+    {
+        label = session->opt->ekm_label;
+        label_size = session->opt->ekm_label_size;
+        ekm_size = session->opt->ekm_size;
+    }
+    else
+    {
+        ASSERT(0);
+    }
 
-            const char *key = format_hex_ex(ekm, size, len, 0, NULL, &gc);
-            setenv_str(session->opt->es, "exported_keying_material", key);
+    unsigned char *ekm = (unsigned char *) gc_malloc(ekm_size, true, gc);
 
-            dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s",
-                 __func__, key);
-        }
-        else
-        {
-            msg(M_WARN, "WARNING: Export keying material failed!");
-            setenv_del(session->opt->es, "exported_keying_material");
-        }
-        gc_free(&gc);
+    if (SSL_export_keying_material(ssl->ssl, ekm, ekm_size, label,
+                                   label_size, NULL, 0, 0) == 1)
+    {
+        return ekm;
+    }
+    else
+    {
+        secure_memzero(ekm, ekm_size);
+        return NULL;
     }
 }
 
-- 
2.26.2

[Openvpn-devel] [PATCH 2/3] Move openvpn specific key expansion into its own function

[Arne S. <ar...@rf...>]

This moves the OpenVPN specific PRF into its own function also
simplifies the code a bit by passing tls_session directly instead of
5 of its fields.

Signed-off-by: Arne Schwabe <ar...@rf...>
---
 src/openvpn/ssl.c | 109 +++++++++++++++++++++++++++++-----------------
 1 file changed, 69 insertions(+), 40 deletions(-)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 390114e1..e44017c6 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1765,27 +1765,38 @@ openvpn_PRF(const uint8_t *secret,
     VALGRIND_MAKE_READABLE((void *)output, output_len);
 }
 
-/*
- * Using source entropy from local and remote hosts, mix into
- * master key.
- */
-static bool
-generate_key_expansion(struct key_ctx_bi *key,
-                       const struct key_type *key_type,
-                       const struct key_source2 *key_src,
-                       const struct session_id *client_sid,
-                       const struct session_id *server_sid,
-                       bool server)
+static void
+init_key_contexts(struct key_ctx_bi *key,
+                  const struct key_type *key_type,
+                  bool server,
+                  struct key2 *key2)
+{
+    /* Initialize OpenSSL key contexts */
+    int key_direction = server ? KEY_DIRECTION_INVERSE : KEY_DIRECTION_NORMAL;
+    init_key_ctx_bi(key, key2, key_direction, key_type, "Data Channel");
+
+    /* Initialize implicit IVs */
+    key_ctx_update_implicit_iv(&key->encrypt, (*key2).keys[(int)server].hmac,
+                               MAX_HMAC_KEY_LENGTH);
+    key_ctx_update_implicit_iv(&key->decrypt, (*key2).keys[1-(int)server].hmac,
+                               MAX_HMAC_KEY_LENGTH);
+
+}
+
+
+static struct key2
+generate_key_expansion_oepnvpn_prf(const struct tls_session *session)
 {
+
     uint8_t master[48] = { 0 };
-    struct key2 key2 = { 0 };
-    bool ret = false;
 
-    if (key->initialized)
-    {
-        msg(D_TLS_ERRORS, "TLS Error: key already initialized");
-        goto exit;
-    }
+    const struct key_state *ks = &session->key[KS_PRIMARY];
+    const struct key_source2 *key_src = ks->key_src;
+
+    const struct session_id *client_sid = session->opt->server ?
+                                          &ks->session_id_remote : &session->session_id;
+    const struct session_id *server_sid = !session->opt->server ?
+                                          &ks->session_id_remote : &session->session_id;
 
     /* debugging print of source key material */
     key_source2_print(key_src);
@@ -1803,6 +1814,7 @@ generate_key_expansion(struct key_ctx_bi *key,
                 master,
                 sizeof(master));
 
+    struct key2 key2;
     /* compute key expansion */
     openvpn_PRF(master,
                 sizeof(master),
@@ -1815,41 +1827,62 @@ generate_key_expansion(struct key_ctx_bi *key,
                 server_sid,
                 (uint8_t *)key2.keys,
                 sizeof(key2.keys));
+    secure_memzero(&master, sizeof(master));
 
+    /* We use the DES fixup here so we can drop it once we
+     * drop DES support and non RFC5705 key derivation */
+    for (int i = 0; i < 2; ++i)
+    {
+        fixup_key(&key2.keys[i], &session->opt->key_type);
+    }
     key2.n = 2;
 
-    key2_print(&key2, key_type, "Master Encrypt", "Master Decrypt");
+    return key2;
+}
+
+/*
+ * Using source entropy from local and remote hosts, mix into
+ * master key.
+ */
+static bool
+generate_key_expansion(struct key_ctx_bi *key,
+                       const struct tls_session *session)
+{
+    bool ret = false;
+
+    if (key->initialized)
+    {
+        msg(D_TLS_ERRORS, "TLS Error: key already initialized");
+        goto exit;
+    }
+
+
+    bool server = session->opt->server;
+
+    struct key2 key2 = generate_key_expansion_oepnvpn_prf(session);
+
+    key2_print(&key2, &session->opt->key_type,
+               "Master Encrypt", "Master Decrypt");
 
     /* check for weak keys */
     for (int i = 0; i < 2; ++i)
     {
-        fixup_key(&key2.keys[i], key_type);
-        if (!check_key(&key2.keys[i], key_type))
+        if (!check_key(&key2.keys[i], &session->opt->key_type))
         {
             msg(D_TLS_ERRORS, "TLS Error: Bad dynamic key generated");
             goto exit;
         }
     }
-
-    /* Initialize OpenSSL key contexts */
-    int key_direction = server ? KEY_DIRECTION_INVERSE : KEY_DIRECTION_NORMAL;
-    init_key_ctx_bi(key, &key2, key_direction, key_type, "Data Channel");
-
-    /* Initialize implicit IVs */
-    key_ctx_update_implicit_iv(&key->encrypt, key2.keys[(int)server].hmac,
-                               MAX_HMAC_KEY_LENGTH);
-    key_ctx_update_implicit_iv(&key->decrypt, key2.keys[1-(int)server].hmac,
-                               MAX_HMAC_KEY_LENGTH);
-
+    init_key_contexts(key, &session->opt->key_type, server, &key2);
     ret = true;
 
 exit:
-    secure_memzero(&master, sizeof(master));
     secure_memzero(&key2, sizeof(key2));
 
     return ret;
 }
 
+
 static void
 key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t *key, size_t key_len)
 {
@@ -1879,10 +1912,7 @@ tls_session_generate_data_channel_keys(struct tls_session *session)
 {
     bool ret = false;
     struct key_state *ks = &session->key[KS_PRIMARY];   /* primary key */
-    const struct session_id *client_sid = session->opt->server ?
-                                          &ks->session_id_remote : &session->session_id;
-    const struct session_id *server_sid = !session->opt->server ?
-                                          &ks->session_id_remote : &session->session_id;
+
 
     if (ks->authenticated == KS_AUTH_FALSE)
     {
@@ -1891,9 +1921,8 @@ tls_session_generate_data_channel_keys(struct tls_session *session)
     }
 
     ks->crypto_options.flags = session->opt->crypto_flags;
-    if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi,
-                                &session->opt->key_type, ks->key_src, client_sid, server_sid,
-                                session->opt->server))
+
+    if (!generate_key_expansion(&ks->crypto_options.key_ctx_bi, session))
     {
         msg(D_TLS_ERRORS, "TLS Error: generate_key_expansion failed");
         goto cleanup;
-- 
2.26.2

[Openvpn-devel] [PATCH applied] Re: Cleanup tls_pre_decrypt_lite and tls_pre_encrypt

[Gert D. <ge...@gr...>]

Acked-by: Gert Doering <ge...@gr...>

"diff -w" shows that this is really "C99, early return, whitespace".

Nevertheless, full client and server tested :-) (and passes).

Your patch has been applied to the master branch.

commit 162499591d03155e853ed44c90c12771307ee0eb
Author: Arne Schwabe
Date:   Mon Aug 10 16:36:52 2020 +0200

     Cleanup tls_pre_decrypt_lite and tls_pre_encrypt

     Signed-off-by: Arne Schwabe <ar...@rf...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@rf...>
     URL: https://www.mail-archive.com/ope...@li.../msg20676.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH applied] Re: Refactor/Reformat tls_pre_decrypt

[Gert D. <ge...@gr...>]

Acked-by: Gert Doering <ge...@gr...>

Reviewed again (with and without "-w"), is really "just" moving
data packet handling into its own function and getting rid of
layers of indentation...

Subjected to client and server tests (which do excercise the
"success" case quite thoroughly, but not all possible error cases -
but the code in question hasn't really changed, just moved), passed 
both.

Your patch has been applied to the master branch.

commit a6a15f7030b25f374a527de57dba199dc64745a3
Author: Arne Schwabe
Date:   Tue Aug 11 12:55:41 2020 +0200

     Refactor/Reformat tls_pre_decrypt

     Signed-off-by: Arne Schwabe <ar...@rf...>
     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@rf...>
     URL: https://www.mail-archive.com/ope...@li.../msg20707.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [PATCH applied] Re: Document comp-lzo no and compress being incompatible

[Gert D. <ge...@gr...>]

Your patch has been applied to the master branch.

commit e02616d82105aeb77f8f811c94fd595ca1a96eae
Author: Arne Schwabe
Date:   Tue Aug 11 13:02:48 2020 +0200

     Document comp-lzo no and compress being incompatible

     Acked-by: Gert Doering <ge...@gr...>
     Message-Id: <202...@rf...>
     URL: https://www.mail-archive.com/ope...@li.../msg20708.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

Re: [Openvpn-devel] [PATCH v3 3/3] Implement the nopadding option to management-external-key for mbed TLS

[Arne S. <ar...@rf...>]

Am 11.08.20 um 11:58 schrieb Gert Doering:
> Hi,
> 
> On Wed, Oct 10, 2018 at 05:29:18PM +0200, Arne Schwabe wrote:
>> Although mbed TLS does not have a TLS 1.3 API yet and we do not really
>> know how mbed TLS will handle querying for TLS 1.3 signatures, being
>> able to use the same API with OpenSSL and mbed TLS is a nice feature.
>>
>> Since mbed TLS does not expose a way to do pkcs1 padding, copy the
>> trimmed down version of the pkcs1 copy to the OpenVPN source code.
> 
> What's the state of this patch?
> 
> The other parts (management-external-key, TLS 1.3, adjusted padding)
> have gone in, but this one got left dangling.
> 

Let's just close it for now. When mbed TLS gets TLS 1.3 support, we can
revisit this.

Arne

[Openvpn-devel] [PATCH v2] Document comp-lzo no and compress being incompatible

[Arne S. <ar...@rf...>]

Most of the new compress but not v2 version do use swap operation. For
'compress lzo' the swap option is not used for backwards compatibility.
For lz4 the swap option is also not a problem since there is no version
without swap. Unfortunately, compress introduced a second stub format
with swap, contrary to the one in 'comp-lzo no' that does not use swap.

Document this weirdness to let not others fall into this trap.

Patch V2: redo patch for rst man pages
---
 doc/man-sections/protocol-options.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/man-sections/protocol-options.rst b/doc/man-sections/protocol-options.rst
index 240d0edf..82e4b2bd 100644
--- a/doc/man-sections/protocol-options.rst
+++ b/doc/man-sections/protocol-options.rst
@@ -103,6 +103,9 @@ configured in a compatible way between both the local and remote side.
   ``lzo`` and ``lz4`` compression support via *IV_* variables to the
   server.
 
+  Note: the :code:`stub` (or empty) option is NOT compatible with the older
+  option ``--comp-lzo no``.
+
   ***Security Considerations***
 
   Compression and encryption is a tricky combination. If an attacker knows
-- 
2.26.2

[Openvpn-devel] [PATCH v2 01/17] Refactor/Reformat tls_pre_decrypt

[Arne S. <ar...@rf...>]

- Extract data packet handling to its own function
- Replace two instances of
          if (x) { code }
  with
          if (!x) return; code

- Remove extra curly braces that were used for pre C99 code style
  to be able to declare variables in the middle of a block

This patch is easier to review with "ignore white space" as the
diff is then a lot smaller in that case and the changes more obvious.

Patch V2: Fix function name spelling, cleanup goto code in the new
          handle_data_channel_packet function

Signed-off-by: Arne Schwabe <ar...@rf...>
---
 src/openvpn/ssl.c | 784 ++++++++++++++++++++++++----------------------
 1 file changed, 403 insertions(+), 381 deletions(-)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index eefa2424..7e2d37f9 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -3142,6 +3142,95 @@ nohard:
  * to implement a multiplexed TLS channel over the TCP/UDP port.
  */
 
+static inline void
+handle_data_channel_packet(struct tls_multi *multi,
+                           const struct link_socket_actual *from,
+                           struct buffer *buf,
+                           struct crypto_options **opt,
+                           bool floated,
+                           const uint8_t **ad_start)
+{
+    struct gc_arena gc = gc_new();
+
+    uint8_t c = *BPTR(buf);
+    int op = c >> P_OPCODE_SHIFT;
+    int key_id = c & P_KEY_ID_MASK;
+
+    /* data channel packet */
+    for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+    {
+        struct key_state *ks = multi->key_scan[i];
+
+        /*
+         * This is the basic test of TLS state compatibility between a local OpenVPN
+         * instance and its remote peer.
+         *
+         * If the test fails, it tells us that we are getting a packet from a source
+         * which claims reference to a prior negotiated TLS session, but the local
+         * OpenVPN instance has no memory of such a negotiation.
+         *
+         * It almost always occurs on UDP sessions when the passive side of the
+         * connection is restarted without the active side restarting as well (the
+         * passive side is the server which only listens for the connections, the
+         * active side is the client which initiates connections).
+         */
+        if (DECRYPT_KEY_ENABLED(multi, ks)
+            && key_id == ks->key_id
+            && (ks->authenticated == KS_AUTH_TRUE)
+            && (floated || link_socket_actual_match(from, &ks->remote_addr)))
+        {
+            if (!ks->crypto_options.key_ctx_bi.initialized)
+            {
+                msg(D_MULTI_DROPPED,
+                    "Key %s [%d] not initialized (yet), dropping packet.",
+                    print_link_socket_actual(from, &gc), key_id);
+                goto done;
+            }
+
+            /* return appropriate data channel decrypt key in opt */
+            *opt = &ks->crypto_options;
+            if (op == P_DATA_V2)
+            {
+                *ad_start = BPTR(buf);
+            }
+            ASSERT(buf_advance(buf, 1));
+            if (op == P_DATA_V1)
+            {
+                *ad_start = BPTR(buf);
+            }
+            else if (op == P_DATA_V2)
+            {
+                if (buf->len < 4)
+                {
+                    msg(D_TLS_ERRORS, "Protocol error: received P_DATA_V2 from %s but length is < 4",
+                        print_link_socket_actual(from, &gc));
+                    ++multi->n_soft_errors;
+                    goto done;
+                }
+                ASSERT(buf_advance(buf, 3));
+            }
+
+            ++ks->n_packets;
+            ks->n_bytes += buf->len;
+            dmsg(D_TLS_KEYSELECT,
+                 "TLS: tls_pre_decrypt, key_id=%d, IP=%s",
+                 key_id, print_link_socket_actual(from, &gc));
+            gc_free(&gc);
+            return;
+        }
+    }
+
+    msg(D_TLS_ERRORS,
+        "TLS Error: local/remote TLS keys are out of sync: %s [%d]",
+        print_link_socket_actual(from, &gc), key_id);
+
+done:
+    tls_clear_error();
+    buf->len = 0;
+    *opt = NULL;
+    gc_free(&gc);
+}
+
 /*
  *
  * When we are in TLS mode, this is the first routine which sees
@@ -3175,440 +3264,374 @@ tls_pre_decrypt(struct tls_multi *multi,
                 bool floated,
                 const uint8_t **ad_start)
 {
+
+    if (buf->len <= 0)
+    {
+        buf->len = 0;
+        *opt = NULL;
+        return false;
+    }
+
     struct gc_arena gc = gc_new();
     bool ret = false;
 
-    if (buf->len > 0)
+    /* get opcode  */
+    uint8_t pkt_firstbyte = *BPTR(buf);
+    int op = pkt_firstbyte >> P_OPCODE_SHIFT;
+
+    if ((op == P_DATA_V1) || (op == P_DATA_V2))
     {
-        int i;
-        int op;
-        int key_id;
+        handle_data_channel_packet(multi, from, buf, opt, floated, ad_start);
+        return false;
+    }
 
-        /* get opcode and key ID */
+    /* get key_id */
+    int key_id = pkt_firstbyte & P_KEY_ID_MASK;
+
+    /* control channel packet */
+    bool do_burst = false;
+    bool new_link = false;
+    struct session_id sid;         /* remote session ID */
+
+    /* verify legal opcode */
+    if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE)
+    {
+        if (op == P_CONTROL_HARD_RESET_CLIENT_V1
+            || op == P_CONTROL_HARD_RESET_SERVER_V1)
         {
-            uint8_t c = *BPTR(buf);
-            op = c >> P_OPCODE_SHIFT;
-            key_id = c & P_KEY_ID_MASK;
+            msg(D_TLS_ERRORS, "Peer tried unsupported key-method 1");
         }
+        msg(D_TLS_ERRORS,
+            "TLS Error: unknown opcode received from %s op=%d",
+            print_link_socket_actual(from, &gc), op);
+        goto error;
+    }
 
-        if ((op == P_DATA_V1) || (op == P_DATA_V2))
+    /* hard reset ? */
+    if (is_hard_reset_method2(op))
+    {
+        /* verify client -> server or server -> client connection */
+        if (((op == P_CONTROL_HARD_RESET_CLIENT_V2
+              || op == P_CONTROL_HARD_RESET_CLIENT_V3) && !multi->opt.server)
+            || ((op == P_CONTROL_HARD_RESET_SERVER_V2) && multi->opt.server))
         {
-            /* data channel packet */
-            for (i = 0; i < KEY_SCAN_SIZE; ++i)
-            {
-                struct key_state *ks = multi->key_scan[i];
-
-                /*
-                 * This is the basic test of TLS state compatibility between a local OpenVPN
-                 * instance and its remote peer.
-                 *
-                 * If the test fails, it tells us that we are getting a packet from a source
-                 * which claims reference to a prior negotiated TLS session, but the local
-                 * OpenVPN instance has no memory of such a negotiation.
-                 *
-                 * It almost always occurs on UDP sessions when the passive side of the
-                 * connection is restarted without the active side restarting as well (the
-                 * passive side is the server which only listens for the connections, the
-                 * active side is the client which initiates connections).
-                 */
-                if (DECRYPT_KEY_ENABLED(multi, ks)
-                    && key_id == ks->key_id
-                    && (ks->authenticated == KS_AUTH_TRUE)
-                    && (floated || link_socket_actual_match(from, &ks->remote_addr)))
-                {
-                    if (!ks->crypto_options.key_ctx_bi.initialized)
-                    {
-                        msg(D_MULTI_DROPPED,
-                            "Key %s [%d] not initialized (yet), dropping packet.",
-                            print_link_socket_actual(from, &gc), key_id);
-                        goto error_lite;
-                    }
-
-                    /* return appropriate data channel decrypt key in opt */
-                    *opt = &ks->crypto_options;
-                    if (op == P_DATA_V2)
-                    {
-                        *ad_start = BPTR(buf);
-                    }
-                    ASSERT(buf_advance(buf, 1));
-                    if (op == P_DATA_V1)
-                    {
-                        *ad_start = BPTR(buf);
-                    }
-                    else if (op == P_DATA_V2)
-                    {
-                        if (buf->len < 4)
-                        {
-                            msg(D_TLS_ERRORS, "Protocol error: received P_DATA_V2 from %s but length is < 4",
-                                print_link_socket_actual(from, &gc));
-                            goto error;
-                        }
-                        ASSERT(buf_advance(buf, 3));
-                    }
+            msg(D_TLS_ERRORS,
+                "TLS Error: client->client or server->server connection attempted from %s",
+                print_link_socket_actual(from, &gc));
+            goto error;
+        }
+    }
 
-                    ++ks->n_packets;
-                    ks->n_bytes += buf->len;
-                    dmsg(D_TLS_KEYSELECT,
-                         "TLS: tls_pre_decrypt, key_id=%d, IP=%s",
-                         key_id, print_link_socket_actual(from, &gc));
-                    gc_free(&gc);
-                    return ret;
-                }
-            }
+    /*
+     * Authenticate Packet
+     */
+    dmsg(D_TLS_DEBUG, "TLS: control channel, op=%s, IP=%s",
+         packet_opcode_name(op), print_link_socket_actual(from, &gc));
 
+    /* get remote session-id */
+    {
+        struct buffer tmp = *buf;
+        buf_advance(&tmp, 1);
+        if (!session_id_read(&sid, &tmp) || !session_id_defined(&sid))
+        {
             msg(D_TLS_ERRORS,
-                "TLS Error: local/remote TLS keys are out of sync: %s [%d]",
-                print_link_socket_actual(from, &gc), key_id);
-            goto error_lite;
+                "TLS Error: session-id not found in packet from %s",
+                print_link_socket_actual(from, &gc));
+            goto error;
         }
-        else                      /* control channel packet */
-        {
-            bool do_burst = false;
-            bool new_link = false;
-            struct session_id sid; /* remote session ID */
+    }
+
+    int i;
+    /* use session ID to match up packet with appropriate tls_session object */
+    for (i = 0; i < TM_SIZE; ++i)
+    {
+        struct tls_session *session = &multi->session[i];
+        struct key_state *ks = &session->key[KS_PRIMARY];
 
-            /* verify legal opcode */
-            if (op < P_FIRST_OPCODE || op > P_LAST_OPCODE)
+        dmsg(D_TLS_DEBUG,
+             "TLS: initial packet test, i=%d state=%s, mysid=%s, rec-sid=%s, rec-ip=%s, stored-sid=%s, stored-ip=%s",
+             i,
+             state_name(ks->state),
+             session_id_print(&session->session_id, &gc),
+             session_id_print(&sid, &gc),
+             print_link_socket_actual(from, &gc),
+             session_id_print(&ks->session_id_remote, &gc),
+             print_link_socket_actual(&ks->remote_addr, &gc));
+
+        if (session_id_equal(&ks->session_id_remote, &sid))
+        /* found a match */
+        {
+            if (i == TM_LAME_DUCK)
             {
-                if (op == P_CONTROL_HARD_RESET_CLIENT_V1
-                    || op == P_CONTROL_HARD_RESET_SERVER_V1)
-                {
-                    msg(D_TLS_ERRORS, "Peer tried unsupported key-method 1");
-                }
                 msg(D_TLS_ERRORS,
-                    "TLS Error: unknown opcode received from %s op=%d",
-                    print_link_socket_actual(from, &gc), op);
+                    "TLS ERROR: received control packet with stale session-id=%s",
+                    session_id_print(&sid, &gc));
                 goto error;
             }
+            dmsg(D_TLS_DEBUG,
+                 "TLS: found match, session[%d], sid=%s",
+                 i, session_id_print(&sid, &gc));
+            break;
+        }
+    }
 
-            /* hard reset ? */
-            if (is_hard_reset_method2(op))
-            {
-                /* verify client -> server or server -> client connection */
-                if (((op == P_CONTROL_HARD_RESET_CLIENT_V2
-                      || op == P_CONTROL_HARD_RESET_CLIENT_V3) && !multi->opt.server)
-                    || ((op == P_CONTROL_HARD_RESET_SERVER_V2) && multi->opt.server))
-                {
-                    msg(D_TLS_ERRORS,
-                        "TLS Error: client->client or server->server connection attempted from %s",
-                        print_link_socket_actual(from, &gc));
-                    goto error;
-                }
-            }
-
-            /*
-             * Authenticate Packet
-             */
-            dmsg(D_TLS_DEBUG, "TLS: control channel, op=%s, IP=%s",
-                 packet_opcode_name(op), print_link_socket_actual(from, &gc));
+    /*
+     * Hard reset and session id does not match any session in
+     * multi->session: Possible initial packet
+     */
+    if (i == TM_SIZE && is_hard_reset_method2(op))
+    {
+        struct tls_session *session = &multi->session[TM_ACTIVE];
+        struct key_state *ks = &session->key[KS_PRIMARY];
 
-            /* get remote session-id */
+        /*
+         * If we have no session currently in progress, the initial packet will
+         * open a new session in TM_ACTIVE rather than TM_UNTRUSTED.
+         */
+        if (!session_id_defined(&ks->session_id_remote))
+        {
+            if (multi->opt.single_session && multi->n_sessions)
             {
-                struct buffer tmp = *buf;
-                buf_advance(&tmp, 1);
-                if (!session_id_read(&sid, &tmp) || !session_id_defined(&sid))
-                {
-                    msg(D_TLS_ERRORS,
-                        "TLS Error: session-id not found in packet from %s",
-                        print_link_socket_actual(from, &gc));
-                    goto error;
-                }
+                msg(D_TLS_ERRORS,
+                    "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [1]",
+                    print_link_socket_actual(from, &gc));
+                goto error;
             }
 
-            /* use session ID to match up packet with appropriate tls_session object */
-            for (i = 0; i < TM_SIZE; ++i)
+#ifdef ENABLE_MANAGEMENT
+            if (management)
             {
-                struct tls_session *session = &multi->session[i];
-                struct key_state *ks = &session->key[KS_PRIMARY];
-
-                dmsg(D_TLS_DEBUG,
-                     "TLS: initial packet test, i=%d state=%s, mysid=%s, rec-sid=%s, rec-ip=%s, stored-sid=%s, stored-ip=%s",
-                     i,
-                     state_name(ks->state),
-                     session_id_print(&session->session_id, &gc),
-                     session_id_print(&sid, &gc),
-                     print_link_socket_actual(from, &gc),
-                     session_id_print(&ks->session_id_remote, &gc),
-                     print_link_socket_actual(&ks->remote_addr, &gc));
-
-                if (session_id_equal(&ks->session_id_remote, &sid))
-                /* found a match */
-                {
-                    if (i == TM_LAME_DUCK)
-                    {
-                        msg(D_TLS_ERRORS,
-                            "TLS ERROR: received control packet with stale session-id=%s",
-                            session_id_print(&sid, &gc));
-                        goto error;
-                    }
-                    dmsg(D_TLS_DEBUG,
-                         "TLS: found match, session[%d], sid=%s",
-                         i, session_id_print(&sid, &gc));
-                    break;
-                }
+                management_set_state(management,
+                                     OPENVPN_STATE_AUTH,
+                                     NULL,
+                                     NULL,
+                                     NULL,
+                                     NULL,
+                                     NULL);
             }
+#endif
 
-            /*
-             * Hard reset and session id does not match any session in
-             * multi->session: Possible initial packet
-             */
-            if (i == TM_SIZE && is_hard_reset_method2(op))
-            {
-                struct tls_session *session = &multi->session[TM_ACTIVE];
-                struct key_state *ks = &session->key[KS_PRIMARY];
-
-                /*
-                 * If we have no session currently in progress, the initial packet will
-                 * open a new session in TM_ACTIVE rather than TM_UNTRUSTED.
-                 */
-                if (!session_id_defined(&ks->session_id_remote))
-                {
-                    if (multi->opt.single_session && multi->n_sessions)
-                    {
-                        msg(D_TLS_ERRORS,
-                            "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [1]",
-                            print_link_socket_actual(from, &gc));
-                        goto error;
-                    }
+            msg(D_TLS_DEBUG_LOW,
+                "TLS: Initial packet from %s, sid=%s",
+                print_link_socket_actual(from, &gc),
+                session_id_print(&sid, &gc));
 
-#ifdef ENABLE_MANAGEMENT
-                    if (management)
-                    {
-                        management_set_state(management,
-                                             OPENVPN_STATE_AUTH,
-                                             NULL,
-                                             NULL,
-                                             NULL,
-                                             NULL,
-                                             NULL);
-                    }
-#endif
+            do_burst = true;
+            new_link = true;
+            i = TM_ACTIVE;
+            session->untrusted_addr = *from;
+        }
+    }
 
-                    msg(D_TLS_DEBUG_LOW,
-                        "TLS: Initial packet from %s, sid=%s",
-                        print_link_socket_actual(from, &gc),
-                        session_id_print(&sid, &gc));
+    /*
+     * If we detected new session in the last if block, i has
+     * changed to TM_ACTIVE, so check the condition again.
+     */
+    if (i == TM_SIZE && is_hard_reset_method2(op))
+    {
+        /*
+         * No match with existing sessions,
+         * probably a new session.
+         */
+        struct tls_session *session = &multi->session[TM_UNTRUSTED];
 
-                    do_burst = true;
-                    new_link = true;
-                    i = TM_ACTIVE;
-                    session->untrusted_addr = *from;
-                }
-            }
+        /*
+         * If --single-session, don't allow any hard-reset connection request
+         * unless it the the first packet of the session.
+         */
+        if (multi->opt.single_session)
+        {
+            msg(D_TLS_ERRORS,
+                "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [2]",
+                print_link_socket_actual(from, &gc));
+            goto error;
+        }
 
-            /*
-             * If we detected new session in the last if block, i has
-             * changed to TM_ACTIVE, so check the condition again.
-             */
-            if (i == TM_SIZE && is_hard_reset_method2(op))
-            {
-                /*
-                 * No match with existing sessions,
-                 * probably a new session.
-                 */
-                struct tls_session *session = &multi->session[TM_UNTRUSTED];
-
-                /*
-                 * If --single-session, don't allow any hard-reset connection request
-                 * unless it the the first packet of the session.
-                 */
-                if (multi->opt.single_session)
-                {
-                    msg(D_TLS_ERRORS,
-                        "TLS Error: Cannot accept new session request from %s due to session context expire or --single-session [2]",
-                        print_link_socket_actual(from, &gc));
-                    goto error;
-                }
+        if (!read_control_auth(buf, &session->tls_wrap, from,
+                               session->opt))
+        {
+            goto error;
+        }
 
-                if (!read_control_auth(buf, &session->tls_wrap, from,
-                                       session->opt))
-                {
-                    goto error;
-                }
+        /*
+         * New session-initiating control packet is authenticated at this point,
+         * assuming that the --tls-auth command line option was used.
+         *
+         * Without --tls-auth, we leave authentication entirely up to TLS.
+         */
+        msg(D_TLS_DEBUG_LOW,
+            "TLS: new session incoming connection from %s",
+            print_link_socket_actual(from, &gc));
 
-                /*
-                 * New session-initiating control packet is authenticated at this point,
-                 * assuming that the --tls-auth command line option was used.
-                 *
-                 * Without --tls-auth, we leave authentication entirely up to TLS.
-                 */
-                msg(D_TLS_DEBUG_LOW,
-                    "TLS: new session incoming connection from %s",
-                    print_link_socket_actual(from, &gc));
+        new_link = true;
+        i = TM_UNTRUSTED;
+        session->untrusted_addr = *from;
+    }
+    else
+    {
+        struct tls_session *session = &multi->session[i];
+        struct key_state *ks = &session->key[KS_PRIMARY];
 
-                new_link = true;
-                i = TM_UNTRUSTED;
-                session->untrusted_addr = *from;
-            }
-            else
+        /*
+         * Packet must belong to an existing session.
+         */
+        if (i != TM_ACTIVE && i != TM_UNTRUSTED)
+        {
+            msg(D_TLS_ERRORS,
+                "TLS Error: Unroutable control packet received from %s (si=%d op=%s)",
+                print_link_socket_actual(from, &gc),
+                i,
+                packet_opcode_name(op));
+            goto error;
+        }
+
+        /*
+         * Verify remote IP address
+         */
+        if (!new_link && !link_socket_actual_match(&ks->remote_addr, from))
+        {
+            msg(D_TLS_ERRORS, "TLS Error: Received control packet from unexpected IP addr: %s",
+                print_link_socket_actual(from, &gc));
+            goto error;
+        }
+
+        /*
+         * Remote is requesting a key renegotiation
+         */
+        if (op == P_CONTROL_SOFT_RESET_V1
+            && DECRYPT_KEY_ENABLED(multi, ks))
+        {
+            if (!read_control_auth(buf, &session->tls_wrap, from,
+                                   session->opt))
             {
-                struct tls_session *session = &multi->session[i];
-                struct key_state *ks = &session->key[KS_PRIMARY];
+                goto error;
+            }
 
-                /*
-                 * Packet must belong to an existing session.
-                 */
-                if (i != TM_ACTIVE && i != TM_UNTRUSTED)
-                {
-                    msg(D_TLS_ERRORS,
-                        "TLS Error: Unroutable control packet received from %s (si=%d op=%s)",
-                        print_link_socket_actual(from, &gc),
-                        i,
-                        packet_opcode_name(op));
-                    goto error;
-                }
+            key_state_soft_reset(session);
 
-                /*
-                 * Verify remote IP address
-                 */
-                if (!new_link && !link_socket_actual_match(&ks->remote_addr, from))
-                {
-                    msg(D_TLS_ERRORS, "TLS Error: Received control packet from unexpected IP addr: %s",
-                        print_link_socket_actual(from, &gc));
-                    goto error;
-                }
+            dmsg(D_TLS_DEBUG,
+                 "TLS: received P_CONTROL_SOFT_RESET_V1 s=%d sid=%s",
+                 i, session_id_print(&sid, &gc));
+        }
+        else
+        {
+            /*
+             * Remote responding to our key renegotiation request?
+             */
+            if (op == P_CONTROL_SOFT_RESET_V1)
+            {
+                do_burst = true;
+            }
 
-                /*
-                 * Remote is requesting a key renegotiation
-                 */
-                if (op == P_CONTROL_SOFT_RESET_V1
-                    && DECRYPT_KEY_ENABLED(multi, ks))
-                {
-                    if (!read_control_auth(buf, &session->tls_wrap, from,
-                                           session->opt))
-                    {
-                        goto error;
-                    }
+            if (!read_control_auth(buf, &session->tls_wrap, from,
+                                   session->opt))
+            {
+                goto error;
+            }
 
-                    key_state_soft_reset(session);
+            dmsg(D_TLS_DEBUG,
+                 "TLS: received control channel packet s#=%d sid=%s",
+                 i, session_id_print(&sid, &gc));
+        }
+    }
 
-                    dmsg(D_TLS_DEBUG,
-                         "TLS: received P_CONTROL_SOFT_RESET_V1 s=%d sid=%s",
-                         i, session_id_print(&sid, &gc));
-                }
-                else
-                {
-                    /*
-                     * Remote responding to our key renegotiation request?
-                     */
-                    if (op == P_CONTROL_SOFT_RESET_V1)
-                    {
-                        do_burst = true;
-                    }
+    /*
+     * We have an authenticated control channel packet (if --tls-auth was set).
+     * Now pass to our reliability layer which deals with
+     * packet acknowledgements, retransmits, sequencing, etc.
+     */
+    struct tls_session *session = &multi->session[i];
+    struct key_state *ks = &session->key[KS_PRIMARY];
 
-                    if (!read_control_auth(buf, &session->tls_wrap, from,
-                                           session->opt))
-                    {
-                        goto error;
-                    }
+    /* Make sure we were initialized and that we're not in an error state */
+    ASSERT(ks->state != S_UNDEF);
+    ASSERT(ks->state != S_ERROR);
+    ASSERT(session_id_defined(&session->session_id));
 
-                    dmsg(D_TLS_DEBUG,
-                         "TLS: received control channel packet s#=%d sid=%s",
-                         i, session_id_print(&sid, &gc));
-                }
-            }
+    /* Let our caller know we processed a control channel packet */
+    ret = true;
 
-            /*
-             * We have an authenticated control channel packet (if --tls-auth was set).
-             * Now pass to our reliability layer which deals with
-             * packet acknowledgements, retransmits, sequencing, etc.
-             */
-            {
-                struct tls_session *session = &multi->session[i];
-                struct key_state *ks = &session->key[KS_PRIMARY];
+    /*
+     * Set our remote address and remote session_id
+     */
+    if (new_link)
+    {
+        ks->session_id_remote = sid;
+        ks->remote_addr = *from;
+        ++multi->n_sessions;
+    }
+    else if (!link_socket_actual_match(&ks->remote_addr, from))
+    {
+        msg(D_TLS_ERRORS,
+            "TLS Error: Existing session control channel packet from unknown IP address: %s",
+            print_link_socket_actual(from, &gc));
+        goto error;
+    }
 
-                /* Make sure we were initialized and that we're not in an error state */
-                ASSERT(ks->state != S_UNDEF);
-                ASSERT(ks->state != S_ERROR);
-                ASSERT(session_id_defined(&session->session_id));
+    /*
+     * Should we do a retransmit of all unacknowledged packets in
+     * the send buffer?  This improves the start-up efficiency of the
+     * initial key negotiation after the 2nd peer comes online.
+     */
+    if (do_burst && !session->burst)
+    {
+        reliable_schedule_now(ks->send_reliable);
+        session->burst = true;
+    }
 
-                /* Let our caller know we processed a control channel packet */
-                ret = true;
+    /* Check key_id */
+    if (ks->key_id != key_id)
+    {
+        msg(D_TLS_ERRORS,
+            "TLS ERROR: local/remote key IDs out of sync (%d/%d) ID: %s",
+            ks->key_id, key_id, print_key_id(multi, &gc));
+        goto error;
+    }
 
-                /*
-                 * Set our remote address and remote session_id
-                 */
-                if (new_link)
-                {
-                    ks->session_id_remote = sid;
-                    ks->remote_addr = *from;
-                    ++multi->n_sessions;
-                }
-                else if (!link_socket_actual_match(&ks->remote_addr, from))
-                {
-                    msg(D_TLS_ERRORS,
-                        "TLS Error: Existing session control channel packet from unknown IP address: %s",
-                        print_link_socket_actual(from, &gc));
-                    goto error;
-                }
+    /*
+     * Process incoming ACKs for packets we can now
+     * delete from reliable send buffer
+     */
+    {
+        /* buffers all packet IDs to delete from send_reliable */
+        struct reliable_ack send_ack;
 
-                /*
-                 * Should we do a retransmit of all unacknowledged packets in
-                 * the send buffer?  This improves the start-up efficiency of the
-                 * initial key negotiation after the 2nd peer comes online.
-                 */
-                if (do_burst && !session->burst)
-                {
-                    reliable_schedule_now(ks->send_reliable);
-                    session->burst = true;
-                }
+        send_ack.len = 0;
+        if (!reliable_ack_read(&send_ack, buf, &session->session_id))
+        {
+            msg(D_TLS_ERRORS,
+                "TLS Error: reading acknowledgement record from packet");
+            goto error;
+        }
+        reliable_send_purge(ks->send_reliable, &send_ack);
+    }
 
-                /* Check key_id */
-                if (ks->key_id != key_id)
-                {
-                    msg(D_TLS_ERRORS,
-                        "TLS ERROR: local/remote key IDs out of sync (%d/%d) ID: %s",
-                        ks->key_id, key_id, print_key_id(multi, &gc));
-                    goto error;
-                }
+    if (op != P_ACK_V1 && reliable_can_get(ks->rec_reliable))
+    {
+        packet_id_type id;
 
-                /*
-                 * Process incoming ACKs for packets we can now
-                 * delete from reliable send buffer
-                 */
+        /* Extract the packet ID from the packet */
+        if (reliable_ack_read_packet_id(buf, &id))
+        {
+            /* Avoid deadlock by rejecting packet that would de-sequentialize receive buffer */
+            if (reliable_wont_break_sequentiality(ks->rec_reliable, id))
+            {
+                if (reliable_not_replay(ks->rec_reliable, id))
                 {
-                    /* buffers all packet IDs to delete from send_reliable */
-                    struct reliable_ack send_ack;
-
-                    send_ack.len = 0;
-                    if (!reliable_ack_read(&send_ack, buf, &session->session_id))
+                    /* Save incoming ciphertext packet to reliable buffer */
+                    struct buffer *in = reliable_get_buf(ks->rec_reliable);
+                    ASSERT(in);
+                    if (!buf_copy(in, buf))
                     {
-                        msg(D_TLS_ERRORS,
-                            "TLS Error: reading acknowledgement record from packet");
+                        msg(D_MULTI_DROPPED,
+                            "Incoming control channel packet too big, dropping.");
                         goto error;
                     }
-                    reliable_send_purge(ks->send_reliable, &send_ack);
+                    reliable_mark_active_incoming(ks->rec_reliable, in, id, op);
                 }
 
-                if (op != P_ACK_V1 && reliable_can_get(ks->rec_reliable))
-                {
-                    packet_id_type id;
-
-                    /* Extract the packet ID from the packet */
-                    if (reliable_ack_read_packet_id(buf, &id))
-                    {
-                        /* Avoid deadlock by rejecting packet that would de-sequentialize receive buffer */
-                        if (reliable_wont_break_sequentiality(ks->rec_reliable, id))
-                        {
-                            if (reliable_not_replay(ks->rec_reliable, id))
-                            {
-                                /* Save incoming ciphertext packet to reliable buffer */
-                                struct buffer *in = reliable_get_buf(ks->rec_reliable);
-                                ASSERT(in);
-                                if (!buf_copy(in, buf))
-                                {
-                                    msg(D_MULTI_DROPPED,
-                                        "Incoming control channel packet too big, dropping.");
-                                    goto error;
-                                }
-                                reliable_mark_active_incoming(ks->rec_reliable, in, id, op);
-                            }
-
-                            /* Process outgoing acknowledgment for packet just received, even if it's a replay */
-                            reliable_ack_acknowledge_packet_id(ks->rec_ack, id);
-                        }
-                    }
-                }
+                /* Process outgoing acknowledgment for packet just received, even if it's a replay */
+                reliable_ack_acknowledge_packet_id(ks->rec_ack, id);
             }
         }
     }
@@ -3621,7 +3644,6 @@ done:
 
 error:
     ++multi->n_soft_errors;
-error_lite:
     tls_clear_error();
     goto done;
 }
-- 
2.26.2

[Openvpn-devel] [PATCH v2] Add demo plugin that excercises "CLIENT_CONNECT" and "CLIENT_CONNECT_V2" paths

[Gert D. <ge...@gr...>]

This is a new "samples" plugin which does not do many useful things,
besides
 - show how a plugin is programmed
 - how the various messages get dispatched
 - how to pass back information from a client-connect/v2 plugin
 - how to do async-cc plugins  [not yet implemented]

the operation of the plugin is controlled by UV_WANT_* environment variables
controlled by the client ("--setenv UV_WANT_CC_FAIL 1 --push-peer-info"),
to "fail CLIENT_CONNECT" or "use async-cc for CLIENT_CONNECT_V2" or
"send 'disable' back from ...") - which is useful for automated testing
of server success/defer/fail code paths for the CLIENT_CONNECT_* functions.

See samples/sample-plugins/client-connect/README for details how to do this.

v2:
  - implement async / deferred operation both for CLIENT_CONNECT and
    CLIENT_CONNECT_V2 plugin calls
  - implement returning openvpn-controlled (setenv) config snippets
    (so the client side can verify in automated testing that the plugin
    operated correctly, without hard-coding something in the plugin code)
---
 sample/sample-plugins/client-connect/Makefile |  16 +
 sample/sample-plugins/client-connect/README   |  34 +
 .../client-connect/sample-client-connect.c    | 609 ++++++++++++++++++
 3 files changed, 659 insertions(+)
 create mode 100644 sample/sample-plugins/client-connect/Makefile
 create mode 100644 sample/sample-plugins/client-connect/README
 create mode 100644 sample/sample-plugins/client-connect/sample-client-connect.c

diff --git a/sample/sample-plugins/client-connect/Makefile b/sample/sample-plugins/client-connect/Makefile
new file mode 100644
index 00000000..ff187c43
--- /dev/null
+++ b/sample/sample-plugins/client-connect/Makefile
@@ -0,0 +1,16 @@
+all: sample-client-connect.so
+
+
+sample-client-connect.o: sample-client-connect.c
+
+# This directory is where we will look for openvpn-plugin.h
+CPPFLAGS=-I../../../include
+
+CC=gcc
+CFLAGS=-O2 -Wall -Wno-unused-variable -g
+
+.c.o:
+	$(CC) $(CPPFLAGS) $(CFLAGS) -fPIC -c $<
+
+sample-client-connect.so: sample-client-connect.o
+	$(CC) $(CFLAGS) -fPIC -shared $(LDFLAGS) -Wl,-soname,$@ -o $@ $< -lc
diff --git a/sample/sample-plugins/client-connect/README b/sample/sample-plugins/client-connect/README
new file mode 100644
index 00000000..be6ef947
--- /dev/null
+++ b/sample/sample-plugins/client-connect/README
@@ -0,0 +1,34 @@
+OpenVPN plugin examples.
+
+Examples provided:
+
+sample-client-connect.c
+
+  - hook to all plugin hooks that openvpn offers
+  - log which hook got called
+  - on CLIENT_CONNECT or CLIENT_CONNECT_V2 set some config variables
+    (controlled by "setenv plugin_cc_config ..." and "plugin_cc2_config"
+    in openvpn's config)
+
+  - if the environment variable UV_WANT_CC_FAIL is set, fail
+  - if the environment variable UV_WANT_CC_DISABLE is set, reject ("disable")
+  - if the environment variable UV_WANT_CC_ASYNC is set, go to
+    asynchronous/deferred mode on CLIENT_CONNECT, and sleep for
+    ${UV_WANT_CC_ASYNC} seconds
+
+  - if the environment variable UV_WANT_CC2_FAIL is set, fail CC2
+  - if the environment variable UV_WANT_CC2_DISABLE is set, reject ("disable")
+  - if the environment variable UV_WANT_CC2_ASYNC is set, go to
+    asynchronous/deferred mode on CLIENT_CONNECT_V2, and sleep for
+    ${UV_WANT_CC2_ASYNC} seconds
+
+    (this can be client-controlled with --setenv UV_WANT_CC_ASYNC nnn
+     etc. --> for easy testing server code paths)
+
+To build (not very sophisticated right now, needs gcc):
+
+  .../sample-plugins/client-connect$ make (Linux/BSD/etc.)
+
+To use in OpenVPN, add to config file:
+
+  plugin sample-client-connect.so (Linux/BSD/etc.)
diff --git a/sample/sample-plugins/client-connect/sample-client-connect.c b/sample/sample-plugins/client-connect/sample-client-connect.c
new file mode 100644
index 00000000..6e874524
--- /dev/null
+++ b/sample/sample-plugins/client-connect/sample-client-connect.c
@@ -0,0 +1,609 @@
+/*
+ *  OpenVPN -- An application to securely tunnel IP networks
+ *             over a single TCP/UDP port, with support for SSL/TLS-based
+ *             session authentication and key exchange,
+ *             packet encryption, packet authentication, and
+ *             packet compression.
+ *
+ *  Copyright (C) 2002-2018 OpenVPN Inc <sa...@op...>
+ *
+ *  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.
+ */
+
+/*
+ * This file implements a simple OpenVPN plugin module which
+ * will log the calls made, and send back some config statements
+ * when called on the CLIENT_CONNECT and CLIENT_CONNECT_V2 hooks.
+ *
+ * it can be asked to fail or go to async/deferred mode by setting
+ * environment variables (UV_WANT_CC_FAIL, UV_WANT_CC_ASYNC,
+ * UV_WANT_CC2_ASYNC) - mostly used as a testing vehicle for the
+ * server side code to handle these cases
+ *
+ * See the README file for build instructions and env control variables.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/wait.h>
+
+#include "openvpn-plugin.h"
+
+/* Pointers to functions exported from openvpn */
+static plugin_log_t plugin_log = NULL;
+static plugin_secure_memzero_t plugin_secure_memzero = NULL;
+static plugin_base64_decode_t plugin_base64_decode = NULL;
+
+/* module name for plugin_log() */
+static char *MODULE = "sample-cc";
+
+/*
+ * Our context, where we keep our state.
+ */
+
+struct plugin_context {
+    int verb;                           /* logging verbosity */
+};
+
+/* this is used for the CLIENT_CONNECT_V2 async/deferred handler
+ *
+ * the "CLIENT_CONNECT_V2" handler puts per-client information into
+ * this, and the "CLIENT_CONNECT_DEFER_V2" handler looks at it to see
+ * if it's time yet to succeed/fail
+ */
+struct plugin_per_client_context {
+    time_t sleep_until;                 /* wakeup time (time() + sleep) */
+    bool want_fail;
+    bool want_disable;
+    const char *client_config;
+};
+
+/*
+ * Given an environmental variable name, search
+ * the envp array for its value, returning it
+ * if found or NULL otherwise.
+ */
+static const char *
+get_env(const char *name, const char *envp[])
+{
+    if (envp)
+    {
+        int i;
+        const int namelen = strlen(name);
+        for (i = 0; envp[i]; ++i)
+        {
+            if (!strncmp(envp[i], name, namelen))
+            {
+                const char *cp = envp[i] + namelen;
+                if (*cp == '=')
+                {
+                    return cp + 1;
+                }
+            }
+        }
+    }
+    return NULL;
+}
+
+
+static int
+atoi_null0(const char *str)
+{
+    if (str)
+    {
+        return atoi(str);
+    }
+    else
+    {
+        return 0;
+    }
+}
+
+/* use v3 functions so we can use openvpn's logging and base64 etc. */
+OPENVPN_EXPORT int
+openvpn_plugin_open_v3(const int v3structver,
+                       struct openvpn_plugin_args_open_in const *args,
+                       struct openvpn_plugin_args_open_return *ret)
+{
+    const char **argv = args->argv;
+    const char **envp = args->envp;
+
+    /* Check API compatibility -- struct version 5 or higher needed */
+    if (v3structver < 5)
+    {
+        fprintf(stderr, "sample-client-connect: this plugin is incompatible with the running version of OpenVPN\n");
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    /*
+     * Allocate our context
+     */
+    struct plugin_context *context = calloc(1, sizeof(struct plugin_context));
+    if (!context)
+    {
+        goto error;
+    }
+
+    /*
+     * Intercept just about everything...
+     */
+    ret->type_mask =
+        OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_UP)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_DOWN)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_ROUTE_UP)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_IPCHANGE)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_TLS_VERIFY)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_CLIENT_CONNECT)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_CLIENT_CONNECT_V2)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_CLIENT_DISCONNECT)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_LEARN_ADDRESS)
+        |OPENVPN_PLUGIN_MASK(OPENVPN_PLUGIN_TLS_FINAL);
+
+    /* Save global pointers to functions exported from openvpn */
+    plugin_log = args->callbacks->plugin_log;
+    plugin_secure_memzero = args->callbacks->plugin_secure_memzero;
+    plugin_base64_decode = args->callbacks->plugin_base64_decode;
+
+    /*
+     * Get verbosity level from environment
+     */
+    context->verb = atoi_null0(get_env("verb", envp));
+
+    ret->handle = (openvpn_plugin_handle_t *) context;
+    plugin_log(PLOG_NOTE, MODULE, "initialization succeeded");
+    return OPENVPN_PLUGIN_FUNC_SUCCESS;
+
+error:
+    if (context)
+    {
+        free(context);
+    }
+    return OPENVPN_PLUGIN_FUNC_ERROR;
+}
+
+
+/* there are two possible interfaces for an openvpn plugin how
+ * to be called on "client connect", which primarily differ in the
+ * way config options are handed back to the client instance
+ * (see openvpn/multi.c, multi_client_connect_call_plugin_{v1,v2}())
+ *
+ * OPENVPN_PLUGIN_CLIENT_CONNECT
+ *   openvpn creates a temp file and passes the name to the plugin
+ *    (via argv[1] variable, argv[0] is the name of the plugin)
+ *   the plugin can write config statements to that file, and openvpn
+ *    reads it in like a "ccd/$cn" per-client config file
+ *
+ * OPENVPN_PLUGIN_CLIENT_CONNECT_V2
+ *   the caller passes in a pointer to an "openvpn_plugin_string_list"
+ *   (openvpn-plugin.h), which is a linked list of (name,value) pairs
+ *
+ *   we fill in one node with name="config" and value="our config"
+ *
+ *   both "l" and "l->name" and "l->value" are malloc()ed by the plugin
+ *   and free()ed by the caller (openvpn_plugin_string_list_free())
+ */
+
+/* helper function to write actual "here are your options" file,
+ * called from sync and sync handler
+ */
+int
+write_cc_options_file(const char *name, const char **envp)
+{
+    if (!name)
+    {
+        return OPENVPN_PLUGIN_FUNC_SUCCESS;
+    }
+
+    FILE *fp = fopen(name,"w");
+    if (!fp)
+    {
+        plugin_log(PLOG_ERR, MODULE, "fopen('%s') failed", name);
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    /* config to-be-sent can come from "setenv plugin_cc_config" in openvpn */
+    const char *p = get_env("plugin_cc_config", envp);
+    if (p)
+    {
+        fprintf(fp, "%s\n", p);
+    }
+
+    /* some generic config snippets so we know it worked */
+    fprintf(fp, "push \"echo sample-cc plugin 1 called\"\n");
+
+    /* if the caller wants, reject client by means of "disable" option */
+    if (get_env("UV_WANT_CC_DISABLE", envp))
+    {
+        plugin_log(PLOG_NOTE, MODULE, "env has UV_WANT_CC_DISABLE, reject");
+        fprintf(fp, "disable\n");
+    }
+    fclose(fp);
+
+    return OPENVPN_PLUGIN_FUNC_SUCCESS;
+}
+
+int
+cc_handle_deferred_v1(int seconds, const char *name, const char **envp)
+{
+    const char *ccd_file = get_env("client_connect_deferred_file", envp);
+    if (!ccd_file)
+    {
+        plugin_log(PLOG_NOTE, MODULE, "env has UV_WANT_CC_ASYNC=%d, but "
+                   "'client_connect_deferred_file' not set -> fail", seconds);
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    /* the CLIENT_CONNECT (v1) API is a bit tricky to work with, because
+     * completition can be signalled both by the "deferred_file" and by
+     * the new ...CLIENT_CONNECT_DEFER API - which is optional.
+     *
+     * For OpenVPN to be able to differenciate, we must create the file
+     * right away if we want to use that for signalling.
+     */
+    int fd = open(ccd_file, O_WRONLY);
+    if (fd < 0)
+    {
+        plugin_log(PLOG_ERR|PLOG_ERRNO, MODULE, "open('%s') failed", ccd_file);
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    if (write(fd, "2", 1) != 1)
+    {
+        plugin_log(PLOG_ERR|PLOG_ERRNO, MODULE, "write to '%s' failed", ccd_file );
+        close(fd);
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+    close(fd);
+
+    /* we do not want to complicate our lives with having to wait()
+     * for child processes (so they are not zombiefied) *and* we MUST NOT
+     * fiddle with signal handlers (= shared with openvpn main), so
+     * we use double-fork() trick.
+     */
+
+    /* fork, sleep, succeed/fail according to env vars */
+    pid_t p1 = fork();
+    if (p1 < 0)                 /* Fork failed */
+    {
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+    if (p1 > 0)                 /* parent process */
+    {
+        waitpid(p1, NULL, 0);
+        return OPENVPN_PLUGIN_FUNC_DEFERRED;
+    }
+
+    /* first gen child process, fork() again and exit() right away */
+    pid_t p2 = fork();
+    if (p2 < 0)
+    {
+        plugin_log(PLOG_ERR|PLOG_ERRNO, MODULE, "BACKGROUND: fork(2) failed");
+        exit(1);
+    }
+    if (p2 > 0)                 /* new parent: exit right away */
+    {
+        exit(0);
+    }
+
+    /* (grand-)child process
+     *  - never call "return" now (would mess up openvpn)
+     *  - return status is communicated by file
+     *  - then exit()
+     */
+
+    /* do mighty complicated work that will really take time here... */
+    plugin_log(PLOG_NOTE, MODULE, "in async/deferred handler, sleep(%d)", seconds);
+    sleep(seconds);
+
+    /* write config options to openvpn */
+    int ret = write_cc_options_file(name, envp);
+
+    /* by setting "UV_WANT_CC_FAIL" we can be triggered to fail */
+    const char *p = get_env("UV_WANT_CC_FAIL", envp);
+    if (p)
+    {
+        plugin_log(PLOG_NOTE, MODULE, "env has UV_WANT_CC_FAIL=%s -> fail", p);
+        ret = OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    /* now signal success/failure state to openvpn */
+    fd = open(ccd_file, O_WRONLY);
+    if (fd < 0)
+    {
+        plugin_log(PLOG_ERR|PLOG_ERRNO, MODULE, "open('%s') failed", ccd_file);
+        exit(1);
+    }
+
+    plugin_log(PLOG_NOTE, MODULE, "cc_handle_deferred_v1: done, signalling %s",
+               (ret == OPENVPN_PLUGIN_FUNC_SUCCESS) ? "success" : "fail" );
+
+    if (write(fd, (ret == OPENVPN_PLUGIN_FUNC_SUCCESS) ? "1" : "0", 1) != 1)
+    {
+        plugin_log(PLOG_ERR|PLOG_ERRNO, MODULE, "write to '%s' failed", ccd_file );
+    }
+    close(fd);
+
+    exit(0);
+}
+
+int
+openvpn_plugin_client_connect(struct plugin_context *context,
+                              const char **argv,
+                              const char **envp)
+{
+    /* log environment variables handed to us by OpenVPN, but
+     * only if "setenv verb" is 3 or higher (arbitrary number)
+     */
+    if (context->verb>=3)
+    {
+        for (int i = 0; argv[i]; i++)
+        {
+            plugin_log(PLOG_NOTE, MODULE, "per-client argv: %s", argv[i]);
+        }
+        for (int i = 0; envp[i]; i++)
+        {
+            plugin_log(PLOG_NOTE, MODULE, "per-client env: %s", envp[i]);
+        }
+    }
+
+    /* by setting "UV_WANT_CC_ASYNC" we go to async/deferred mode */
+    const char *p = get_env("UV_WANT_CC_ASYNC", envp);
+    if (p)
+    {
+        /* the return value will usually be OPENVPN_PLUGIN_FUNC_DEFERRED
+         * ("I will do my job in the background, check the status file!")
+         * but depending on env setup it might be "..._ERRROR"
+         */
+        return cc_handle_deferred_v1(atoi(p), argv[1], envp);
+    }
+
+    /* -- this is synchronous mode (openvpn waits for us) -- */
+
+    /* by setting "UV_WANT_CC_FAIL" we can be triggered to fail */
+    p = get_env("UV_WANT_CC_FAIL", envp);
+    if (p)
+    {
+        plugin_log(PLOG_NOTE, MODULE, "env has UV_WANT_CC_FAIL=%s -> fail", p);
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    /* does the caller want options?  give them some */
+    int ret = write_cc_options_file(argv[1], envp);
+
+    return ret;
+}
+
+int
+openvpn_plugin_client_connect_v2(struct plugin_context *context,
+                                 struct plugin_per_client_context *pcc,
+                                 const char **envp,
+                                 struct openvpn_plugin_string_list **return_list)
+{
+    /* by setting "UV_WANT_CC2_ASYNC" we go to async/deferred mode */
+    const char *want_async = get_env("UV_WANT_CC2_ASYNC", envp);
+    const char *want_fail = get_env("UV_WANT_CC2_FAIL", envp);
+    const char *want_disable = get_env("UV_WANT_CC2_DISABLE", envp);
+
+    /* config to push towards client - can be controlled by OpenVPN
+     * config ("setenv plugin_cc2_config ...") - mostly useful in a
+     * regression test environment to push stuff like routes which are
+     * then verified by t_client ping tests
+     */
+    const char *client_config = get_env("plugin_cc2_config", envp);
+    if (!client_config)
+    {
+        /* pick something meaningless which can be verified in client log */
+        client_config = "push \"setenv CC2 MOOH\"\n";
+    }
+
+    if (want_async)
+    {
+        /* we do no really useful work here, so we just tell the
+         * "CLIENT_CONNECT_DEFER_V2" handler that it should sleep
+         * and then "do things" via the per-client-context
+         */
+        pcc->sleep_until = time(NULL) + atoi(want_async);
+        pcc->want_fail = (want_fail != NULL);
+        pcc->want_disable = (want_disable != NULL);
+        pcc->client_config = client_config;
+        plugin_log(PLOG_NOTE, MODULE, "env has UV_WANT_CC2_ASYNC=%s -> set up deferred handler", want_async);
+        return OPENVPN_PLUGIN_FUNC_DEFERRED;
+    }
+
+    /* by setting "UV_WANT_CC2_FAIL" we can be triggered to fail here */
+    if (want_fail)
+    {
+        plugin_log(PLOG_NOTE, MODULE, "env has UV_WANT_CC2_FAIL=%s -> fail", want_fail);
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    struct openvpn_plugin_string_list *rl =
+        calloc(1, sizeof(struct openvpn_plugin_string_list));
+    if (!rl)
+    {
+        plugin_log(PLOG_ERR, MODULE, "malloc(return_list) failed");
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+    rl->name = strdup("config");
+    if (want_disable)
+    {
+        plugin_log(PLOG_NOTE, MODULE, "env has UV_WANT_CC2_DISABLE, reject");
+        rl->value = strdup("disable\n");
+    }
+    else
+    {
+        rl->value = strdup(client_config);
+    }
+
+    if (!rl->name || !rl->value)
+    {
+        plugin_log(PLOG_ERR, MODULE, "malloc(return_list->xx) failed");
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    *return_list = rl;
+
+    return OPENVPN_PLUGIN_FUNC_SUCCESS;
+}
+
+int
+openvpn_plugin_client_connect_defer_v2(struct plugin_context *context,
+                                       struct plugin_per_client_context *pcc,
+                                       struct openvpn_plugin_string_list
+                                       **return_list)
+{
+    time_t time_left = pcc->sleep_until - time(NULL);
+    plugin_log(PLOG_NOTE, MODULE, "defer_v2: seconds left=%d",
+               (int) time_left);
+
+    /* not yet due? */
+    if (time_left > 0)
+    {
+        return OPENVPN_PLUGIN_FUNC_DEFERRED;
+    }
+
+    /* client wants fail? */
+    if (pcc->want_fail)
+    {
+        plugin_log(PLOG_NOTE, MODULE, "env has UV_WANT_CC2_FAIL -> fail" );
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    /* fill in RL according to with-disable / without-disable */
+
+    /* TODO: unify this with non-deferred case */
+    struct openvpn_plugin_string_list *rl =
+        calloc(1, sizeof(struct openvpn_plugin_string_list));
+    if (!rl)
+    {
+        plugin_log(PLOG_ERR, MODULE, "malloc(return_list) failed");
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+    rl->name = strdup("config");
+    if (pcc->want_disable)
+    {
+        plugin_log(PLOG_NOTE, MODULE, "env has UV_WANT_CC2_DISABLE, reject");
+        rl->value = strdup("disable\n");
+    }
+    else
+    {
+        rl->value = strdup(pcc->client_config);
+    }
+
+    if (!rl->name || !rl->value)
+    {
+        plugin_log(PLOG_ERR, MODULE, "malloc(return_list->xx) failed");
+        return OPENVPN_PLUGIN_FUNC_ERROR;
+    }
+
+    *return_list = rl;
+
+    return OPENVPN_PLUGIN_FUNC_SUCCESS;
+}
+
+OPENVPN_EXPORT int
+openvpn_plugin_func_v2(openvpn_plugin_handle_t handle,
+                       const int type,
+                       const char *argv[],
+                       const char *envp[],
+                       void *per_client_context,
+                       struct openvpn_plugin_string_list **return_list)
+{
+    struct plugin_context *context = (struct plugin_context *) handle;
+    struct plugin_per_client_context *pcc = (struct plugin_per_client_context *) per_client_context;
+
+    /* for most functions, we just "don't do anything" but log the
+     * event received (so one can follow it in the log and understand
+     * the sequence of events).  CONNECT and CONNECT_V2 are handled
+     */
+    switch (type)
+    {
+        case OPENVPN_PLUGIN_UP:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_UP");
+            break;
+
+        case OPENVPN_PLUGIN_DOWN:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_DOWN");
+            break;
+
+        case OPENVPN_PLUGIN_ROUTE_UP:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_ROUTE_UP");
+            break;
+
+        case OPENVPN_PLUGIN_IPCHANGE:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_IPCHANGE");
+            break;
+
+        case OPENVPN_PLUGIN_TLS_VERIFY:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_TLS_VERIFY");
+            break;
+
+        case OPENVPN_PLUGIN_CLIENT_CONNECT:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_CLIENT_CONNECT");
+            return openvpn_plugin_client_connect(context, argv, envp);
+
+        case OPENVPN_PLUGIN_CLIENT_CONNECT_V2:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_CLIENT_CONNECT_V2");
+            return openvpn_plugin_client_connect_v2(context, pcc, envp,
+                                                    return_list);
+
+        case OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_CLIENT_CONNECT_DEFER_V2");
+            return openvpn_plugin_client_connect_defer_v2(context, pcc,
+                                                          return_list);
+
+        case OPENVPN_PLUGIN_CLIENT_DISCONNECT:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_CLIENT_DISCONNECT");
+            break;
+
+        case OPENVPN_PLUGIN_LEARN_ADDRESS:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_LEARN_ADDRESS");
+            break;
+
+        case OPENVPN_PLUGIN_TLS_FINAL:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_TLS_FINAL");
+            break;
+
+        default:
+            plugin_log(PLOG_NOTE, MODULE, "OPENVPN_PLUGIN_? type=%d\n", type);
+    }
+    return OPENVPN_PLUGIN_FUNC_SUCCESS;
+}
+
+OPENVPN_EXPORT void *
+openvpn_plugin_client_constructor_v1(openvpn_plugin_handle_t handle)
+{
+    printf("FUNC: openvpn_plugin_client_constructor_v1\n");
+    return calloc(1, sizeof(struct plugin_per_client_context));
+}
+
+OPENVPN_EXPORT void
+openvpn_plugin_client_destructor_v1(openvpn_plugin_handle_t handle, void *per_client_context)
+{
+    printf("FUNC: openvpn_plugin_client_destructor_v1\n");
+    free(per_client_context);
+}
+
+OPENVPN_EXPORT void
+openvpn_plugin_close_v1(openvpn_plugin_handle_t handle)
+{
+    struct plugin_context *context = (struct plugin_context *) handle;
+    printf("FUNC: openvpn_plugin_close_v1\n");
+    free(context);
+}
-- 
2.26.2