Re: [Openvpn-devel] [PATCH] Fix compilation on pre-EKM mbedTLS libraries.

2020-10-09 Thread Arne Schwabe
Am 09.10.20 um 22:33 schrieb Gert Doering:
> commit f0734e49956217 simplified key_state_export_keying_material(),
> changing the function prototype.  For older mbedTLS versions, there
> is "always fail" dummy function which was overlooked in that change.
> 
> Fix prototype.
> 
> Signed-off-by: Gert Doering 
> ---
>  src/openvpn/ssl_mbedtls.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
> index bb5633b7..b100d03e 100644
> --- a/src/openvpn/ssl_mbedtls.c
> +++ b/src/openvpn/ssl_mbedtls.c
> @@ -252,11 +252,10 @@ key_state_export_keying_material(struct tls_session 
> *session,
>  }
>  }
>  #else
> -unsigned char*
> +bool
>  key_state_export_keying_material(struct tls_session *session,
>   const char* label, size_t label_size,
> - size_t ekm_size,
> - struct gc_arena *gc)
> + void *ekm, size_t ekm_size)
>  {
>  /* Dummy function to avoid ifdefs in the common code */
>  return NULL;
> 

Well, if we fix, it we should fix all the way and not return NULL which
gets coerced to false.

Arne



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


[Openvpn-devel] [PATCH] Fix compilation on pre-EKM mbedTLS libraries.

2020-10-09 Thread Gert Doering
commit f0734e49956217 simplified key_state_export_keying_material(),
changing the function prototype.  For older mbedTLS versions, there
is "always fail" dummy function which was overlooked in that change.

Fix prototype.

Signed-off-by: Gert Doering 
---
 src/openvpn/ssl_mbedtls.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index bb5633b7..b100d03e 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -252,11 +252,10 @@ key_state_export_keying_material(struct tls_session 
*session,
 }
 }
 #else
-unsigned char*
+bool
 key_state_export_keying_material(struct tls_session *session,
  const char* label, size_t label_size,
- size_t ekm_size,
- struct gc_arena *gc)
+ void *ekm, size_t ekm_size)
 {
 /* Dummy function to avoid ifdefs in the common code */
 return NULL;
-- 
2.22.0



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


[Openvpn-devel] [PATCH applied] Re: Simplify key material exporter backend API

2020-10-09 Thread Gert Doering
I have stared at the code a bit ("amazingly straightforward, no crypto
convolutions at all :-)") and tortured the result a bit on the server 
and client test bed.  No complaints.

(I have excercised the "key-derivation tls-ekm" path, but not the 
generic EKM path - no test case for that one yet... need to add one,
it seems)

Your patch has been applied to the master branch.

commit f0734e499562175a6ebbefbfa12b730c976e2507
Author: Steffan Karger
Date:   Fri Oct 9 16:47:55 2020 +0200

 Simplify key material exporter backend API

 Signed-off-by: Steffan Karger 
 Acked-by: Arne Schwabe 
 Message-Id: <20201009144755.39719-1-stef...@karger.me>
 URL: 
https://www.mail-archive.com/search?l=mid=20201009144755.39719-1-stef...@karger.me
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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


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

2020-10-09 Thread Gert Doering
In addition to the thorough review from Steffan, I have tortured
this patch on the client and server side (read: patched client 
talking to an old server, old clients talking to a patched server,
patched client talking to patched server, with/without NCP) and
everything succeeded/failed as expected.

I have actually seen the handshake happen between openssl client
and server, and mbedtls 2.24.0 (gentoo) client / openssl server
(I have not checked if the handshake had any actual *effect*, just
if I could see it in the PUSH_REPLY message, and that ping worked
afterwards).

"PUSH_REPLY ... cipher none,key-derivation tls-ekm" looks weird :-)

Your patch has been applied to the master branch.

commit 6dc09d0d4520483716530e12a444b156720cdfcc
Author: Arne Schwabe
Date:   Fri Oct 9 13:54:53 2020 +0200

 Implement generating data channel keys via EKM/RFC 5705

 Signed-off-by: Arne Schwabe 
 Acked-by: Steffan Karger 
 Message-Id: <20201009115453.4279-1-a...@rfc2549.org>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21187.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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


Re: [Openvpn-devel] [PATCH] Simplify key material exporter backend API

2020-10-09 Thread Arne Schwabe
Am 09.10.20 um 16:47 schrieb Steffan Karger:
> Just pass pointer and length, instead of a gc and return (possibly)
> allocated memory. Saves us some gc instantiations and memcpy()s. Exact
> same functionality, 19 lines less code.
> 
> (Didn't want to delay the TLS EKM reviews for this, so submitted as a
> patch afterwards.)
> 
Acked-By: Arne Schwabe 



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


[Openvpn-devel] [PATCH applied] Re: networking_iproute2: fix memory leak in net_iface_mtu_set()

2020-10-09 Thread Gert Doering
Yeah, very obvious when you look at it :-)  (wasn't *me* who
ACKed that commit... but I had to check to be sure ;-) )

Your patch has been applied to the master and release/2.5 branch.

commit 1e6e083e042d58f9541bf74d343d52fc5681 (master)
commit a9c8225439f0acaa679ee59244b5b8660b561592 (release/2.5)
Author: Steffan Karger
Date:   Fri Oct 9 15:46:03 2020 +0200

 networking_iproute2: fix memory leak in net_iface_mtu_set()

 Signed-off-by: Steffan Karger 
 Acked-by: Antonio Quartulli 
 Message-Id: <20201009134603.36263-1-stef...@karger.me>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21189.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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


[Openvpn-devel] [PATCH] Simplify key material exporter backend API

2020-10-09 Thread Steffan Karger
Just pass pointer and length, instead of a gc and return (possibly)
allocated memory. Saves us some gc instantiations and memcpy()s. Exact
same functionality, 19 lines less code.

(Didn't want to delay the TLS EKM reviews for this, so submitted as a
patch afterwards.)

Signed-off-by: Steffan Karger 
---
 src/openvpn/ssl.c | 26 --
 src/openvpn/ssl_backend.h | 14 +-
 src/openvpn/ssl_mbedtls.c | 12 +---
 src/openvpn/ssl_openssl.c | 11 ---
 4 files changed, 22 insertions(+), 41 deletions(-)

diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index b572b7b8..87b51d96 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1786,23 +1786,14 @@ 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),
-sizeof(key2->keys),
-);
-if (!key2data)
+if (!key_state_export_keying_material(session, EXPORT_KEY_DATA_LABEL,
+  strlen(EXPORT_KEY_DATA_LABEL),
+  key2->keys, sizeof(key2->keys)))
 {
 return false;
 }
-memcpy(key2->keys, key2data, sizeof(key2->keys));
-secure_memzero(key2data, sizeof(key2->keys));
 key2->n = 2;
 
-gc_free();
 return true;
 }
 
@@ -2499,12 +2490,11 @@ export_user_keying_material(struct key_state_ssl *ssl,
 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,
-)))
+unsigned char *ekm = gc_malloc(session->opt->ekm_size, true, );
+if (key_state_export_keying_material(session,
+ session->opt->ekm_label,
+ session->opt->ekm_label_size,
+ ekm, session->opt->ekm_size))
 {
 unsigned int len = (size * 2) + 2;
 
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index 4bcb3181..c3d12e5b 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -398,18 +398,14 @@ void backend_tls_ctx_reload_crl(struct tls_root_ctx 
*ssl_ctx,
  * @param session  The session associated with the given key_state
  * @param labelThe label to use when exporting the key
  * @param label_size   The size of the label to use when exporting the key
- * @param ekm_size THe size of the exported/returned key material
- * @param gc   gc_arena that might be used to allocate the string
- * returned
- * @returnsThe exported key material, the caller may zero the
- * string but should not free it
+ * @param ekm  Buffer to return the exported key material in
+ * @param ekm_size The size of ekm, in bytes
+ * @returnstrue if exporting succeeded, false otherwise
  */
-
-unsigned char*
+bool
 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));
+ void *ekm, size_t ekm_size);
 
 /**/
 /** @addtogroup control_tls
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index f375e957..bb5633b7 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -219,11 +219,10 @@ mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned 
char *ms,
 return true;
 }
 
-unsigned char *
+bool
 key_state_export_keying_material(struct tls_session *session,
  const char* label, size_t label_size,
- size_t ekm_size,
- struct gc_arena *gc)
+ void *ekm, size_t ekm_size)
 {
 ASSERT(strlen(label) == label_size);
 
@@ -233,10 +232,9 @@ key_state_export_keying_material(struct tls_session 
*session,
  * there is no PRF, in both cases we cannot generate key material */
 if (cache->tls_prf_type == MBEDTLS_SSL_TLS_PRF_NONE)
 {
-return NULL;
+

Re: [Openvpn-devel] [PATCH] networking_iproute2: fix memory leak in net_iface_mtu_set()

2020-10-09 Thread Antonio Quartulli
Hi,

On 09/10/2020 15:46, Steffan Karger wrote:
> ASAN yelled at me that someone forgot to call argv_free(). Fix that.
> 
> Signed-off-by: Steffan Karger 

I bet I know that someone!

Thanks a lot for fixing this.

Acked-by: Antonio Quartulli 

(this is for 2.5)

-- 
Antonio Quartulli


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


[Openvpn-devel] [PATCH] networking_iproute2: fix memory leak in net_iface_mtu_set()

2020-10-09 Thread Steffan Karger
ASAN yelled at me that someone forgot to call argv_free(). Fix that.

Signed-off-by: Steffan Karger 
---
 src/openvpn/networking_iproute2.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/openvpn/networking_iproute2.c 
b/src/openvpn/networking_iproute2.c
index f3b9c614..3b460527 100644
--- a/src/openvpn/networking_iproute2.c
+++ b/src/openvpn/networking_iproute2.c
@@ -88,6 +88,8 @@ net_iface_mtu_set(openvpn_net_ctx_t *ctx, const char *iface, 
uint32_t mtu)
 argv_msg(M_INFO, );
 openvpn_execve_check(, ctx->es, S_FATAL, "Linux ip link set failed");
 
+argv_free();
+
 return 0;
 }
 
-- 
2.25.1



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


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

2020-10-09 Thread Steffan Karger
Hi,

On 09-10-2020 13:54, Arne Schwabe wrote:
> OpenVPN currently uses its own (based on TLS 1.0) key derivation
> mechanism to generate the 256 bytes key data in key2 struct that
> are then used used to generate encryption/hmac/iv vectors. While
> this mechanism is still secure, it is not state of the art.
> 
> Instead of modernising our own approach, this commit implements
> key derivation using the Keying Material Exporters API introduced
> by RFC 5705.
> 
> We also use an opportunistic approach of negotiating the use of
> EKM (exported key material) through an IV_PROTO flag and prefer
> EKM to our own PRF if both client and server support it. The
> use of EKM is pushed to the client as part of NCP as
> key-derivation tls-ekm.
> 
> We still exchange the random data (112 bytes from client to server
> and 64 byte from server to client) for the OpenVPN PRF but
> do not use it. Removing that exchange would break the handshake
> and make a key-method 3 or similar necessary.
> 
> As a side effect, this makes a little bit easier to have a FIPS compatible
> version of OpenVPN since we do not rely on calling MD5 anymore.
> 
> Side note: this commit breaks the (not yet merged) WolfSSL support as it
> claims to support EKM in the OpenSSL compat API but always returns an error
> if you try to use it.
> 
> Signed-off-by: Arne Schwabe 
> 
> Patch v2: rebase/change to V2 of EKM refactoring
> 
> Patch v3: add Changes.rst
> 
> Patch v4: Rebase on master.
> 
> Patch v5: Refuse internal label to be used with --keying-material-exporter,
>   polishing/fixes suggested by Steffan integrated
> 
> Signed-off-by: Arne Schwabe 

Still the duplicate signed-off line. But that's easy enough to fix at
commit time I guess.

> ---
>  Changes.rst  | 11 +++
>  doc/doxygen/doc_key_generation.h | 14 +++--
>  src/openvpn/crypto.h |  4 +++
>  src/openvpn/init.c   |  1 +
>  src/openvpn/multi.c  |  4 +++
>  src/openvpn/options.c| 19 +++
>  src/openvpn/options.h|  3 ++
>  src/openvpn/push.c   |  5 ++-
>  src/openvpn/ssl.c| 54 
>  src/openvpn/ssl.h|  2 ++
>  src/openvpn/ssl_backend.h|  1 +
>  src/openvpn/ssl_mbedtls.c|  7 ++---
>  12 files changed, 111 insertions(+), 14 deletions(-)
> 
> diff --git a/Changes.rst b/Changes.rst
> index f67e1d76..2a2829e7 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -1,3 +1,14 @@
> +Overview of changes in 2.6
> +==
> +
> +
> +New features
> +
> +Keying Material Exporters (RFC 5705) based key generation
> +As part of the cipher negotiation OpenVPN will automatically prefer
> +the RFC5705 based key material generation to the current custom
> +OpenVPN PRF. This feature requires OpenSSL or mbed TLS 2.18+.
> +
>  Overview of changes in 2.5
>  ==
>  
> diff --git a/doc/doxygen/doc_key_generation.h 
> b/doc/doxygen/doc_key_generation.h
> index 4bb9c708..cef773a9 100644
> --- a/doc/doxygen/doc_key_generation.h
> +++ b/doc/doxygen/doc_key_generation.h
> @@ -58,6 +58,12 @@
>   *
>   * @subsection key_generation_method_2 Key method 2
>   *
> + * There are two methods for generating key data when using key method 2
> + * the first is OpenVPN's traditional approach that exchanges random
> + * data and uses a PRF and the other is using the RFC5705 keying material
> + * exporter to generate the key material. For both methods the random
> + * data is exchange but only used in the traditional method.
> + *
>   * -# The client generates random material in the following amounts:
>   *- Pre-master secret: 48 bytes
>   *- Client's PRF seed for master secret: 32 bytes
> @@ -73,8 +79,12 @@
>   *server's random material.
>   *
>   * %Key method 2 %key expansion is performed by the \c
> - * generate_key_expansion() function.  Please refer to its source code for
> - * details of the %key expansion process.
> + * generate_key_expansion_oepnvpn_prf() function.  Please refer to its source

What is this oepnvpn your are talking about? :-p

(Interesting how you managed to re-introduce this typo from v4 to v5...)

> + * code for details of the %key expansion process.
> + *
> + * When the client sends the IV_PROTO_TLS_KEY_EXPORT flag and the server 
> replies
> + * with `key-derivation tls-ekm` the RFC5705 key material exporter with the
> + * label EXPORTER-OpenVPN-datakeys is used for the key data.
>   *
>   * @subsection key_generation_random Source of random material
>   *
> diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
> index 999f643e..1ad669ce 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 

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

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

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

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

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

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

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

Signed-off-by: Arne Schwabe 

Patch v2: rebase/change to V2 of EKM refactoring

Patch v3: add Changes.rst

Patch v4: Rebase on master.

Patch v5: Refuse internal label to be used with --keying-material-exporter,
  polishing/fixes suggested by Steffan integrated

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

diff --git a/Changes.rst b/Changes.rst
index f67e1d76..2a2829e7 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -1,3 +1,14 @@
+Overview of changes in 2.6
+==
+
+
+New features
+
+Keying Material Exporters (RFC 5705) based key generation
+As part of the cipher negotiation OpenVPN will automatically prefer
+the RFC5705 based key material generation to the current custom
+OpenVPN PRF. This feature requires OpenSSL or mbed TLS 2.18+.
+
 Overview of changes in 2.5
 ==
 
diff --git a/doc/doxygen/doc_key_generation.h b/doc/doxygen/doc_key_generation.h
index 4bb9c708..cef773a9 100644
--- a/doc/doxygen/doc_key_generation.h
+++ b/doc/doxygen/doc_key_generation.h
@@ -58,6 +58,12 @@
  *
  * @subsection key_generation_method_2 Key method 2
  *
+ * There are two methods for generating key data when using key method 2
+ * the first is OpenVPN's traditional approach that exchanges random
+ * data and uses a PRF and the other is using the RFC5705 keying material
+ * exporter to generate the key material. For both methods the random
+ * data is exchange but only used in the traditional method.
+ *
  * -# The client generates random material in the following amounts:
  *- Pre-master secret: 48 bytes
  *- Client's PRF seed for master secret: 32 bytes
@@ -73,8 +79,12 @@
  *server's random material.
  *
  * %Key method 2 %key expansion is performed by the \c
- * generate_key_expansion() function.  Please refer to its source code for
- * details of the %key expansion process.
+ * generate_key_expansion_oepnvpn_prf() function.  Please refer to its source
+ * code for details of the %key expansion process.
+ *
+ * When the client sends the IV_PROTO_TLS_KEY_EXPORT flag and the server 
replies
+ * with `key-derivation tls-ekm` the RFC5705 key material exporter with the
+ * label EXPORTER-OpenVPN-datakeys is used for the key data.
  *
  * @subsection key_generation_random Source of random material
  *
diff --git a/src/openvpn/crypto.h b/src/openvpn/crypto.h
index 999f643e..1ad669ce 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 data channel 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 f87c11e7..580a8550 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -687,6 +687,7 @@ restore_ncp_options(struct context *c)
 c->options.ciphername = c->c1.ciphername;
 

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

2020-10-09 Thread Arne Schwabe
Am 09.10.20 um 11:23 schrieb Steffan Karger:
> Hi,
> 
> On 25-08-2020 09:36, Arne Schwabe wrote:
>> OpenVPN currently uses its own (based on TLS 1.0) key derivation
>> mechanism to generate the 256 bytes key data in key2 struct that
>> are then used used to generate encryption/hmac/iv vectors. While
>> this mechanism is still secure, it is not state of the art.
>>
>> Instead of modernising our own approach, this commit implements
>> key derivation using the Keying Material Exporters API introduced
>> by RFC 5705.
> 
> Thanks! I'm glad to see you're picking up the work I haven't been able
> to find time for.
> 
>> We also use an opportunistic approach of negotiating the use of
>> EKM (exported key material) through an IV_PROTO flag and prefer
>> EKM to our own PRF if both client and server support it. The
>> use of EKM is pushed to the client as part of NCP as
>> key-derivation tls-ekm.
>>
>> We still exchange the random data (112 bytes from client to server
>> and 64 byte from server to client) for the OpenVPN PRF but
>> do not use it. Removing that exchange would break the handshake
>> and make a key-method 3 or similar necessary.
>>
>> As a side effect, this makes a little bit easier to have a FIPS compatible
>> version of OpenVPN since we do not rely on calling MD5 anymore.
>>
>> Side note: this commit breaks the (not yet merged) WolfSSL support as it
>> claims to support EKM in the OpenSSL compat API but always returns an error
>> if you try to use it.
>>
>> Signed-off-by: Arne Schwabe 
> 
> Signed off two times.
> 
>> Patch v2: rebase/change to V2 of EKM refactoring
>>
>> Patch v3: add Changes.rst
>>
>> Signed-off-by: Arne Schwabe 
>> ---
>>  Changes.rst  | 11 +++
>>  doc/doxygen/doc_key_generation.h | 14 +++--
>>  src/openvpn/crypto.h |  4 +++
>>  src/openvpn/init.c   |  1 +
>>  src/openvpn/multi.c  |  4 +++
>>  src/openvpn/options.c| 14 +
>>  src/openvpn/options.h|  3 ++
>>  src/openvpn/push.c   |  5 ++-
>>  src/openvpn/ssl.c| 54 
>>  src/openvpn/ssl.h|  2 ++
>>  src/openvpn/ssl_backend.h|  2 ++
>>  src/openvpn/ssl_mbedtls.c|  7 ++---
>>  12 files changed, 107 insertions(+), 14 deletions(-)
>>
>> diff --git a/Changes.rst b/Changes.rst
>> index f67e1d76..2a2829e7 100644
>> --- a/Changes.rst
>> +++ b/Changes.rst
>> @@ -1,3 +1,14 @@
>> +Overview of changes in 2.6
>> +==
>> +
>> +
>> +New features
>> +
>> +Keying Material Exporters (RFC 5705) based key generation
>> +As part of the cipher negotiation OpenVPN will automatically prefer
>> +the RFC5705 based key material generation to the current custom
>> +OpenVPN PRF. This feature requires OpenSSL or mbed TLS 2.18+.
>> +
>>  Overview of changes in 2.5
>>  ==
>>  
>> diff --git a/doc/doxygen/doc_key_generation.h 
>> b/doc/doxygen/doc_key_generation.h
>> index 4bb9c708..cef773a9 100644
>> --- a/doc/doxygen/doc_key_generation.h
>> +++ b/doc/doxygen/doc_key_generation.h
>> @@ -58,6 +58,12 @@
>>   *
>>   * @subsection key_generation_method_2 Key method 2
>>   *
>> + * There are two methods for generating key data when using key method 2
>> + * the first is OpenVPN's traditional approach that exchanges random
>> + * data and uses a PRF and the other is using the RFC5705 keying material
>> + * exporter to generate the key material. For both methods the random
>> + * data is exchange but only used in the traditional method.
>> + *
>>   * -# The client generates random material in the following amounts:
>>   *- Pre-master secret: 48 bytes
>>   *- Client's PRF seed for master secret: 32 bytes
>> @@ -73,8 +79,12 @@
>>   *server's random material.
>>   *
>>   * %Key method 2 %key expansion is performed by the \c
>> - * generate_key_expansion() function.  Please refer to its source code for
>> - * details of the %key expansion process.
>> + * generate_key_expansion_openvpn_prf() function.  Please refer to its 
>> source
>> + * code for details of the %key expansion process.
>> + *
>> + * When the client sends the IV_PROTO_TLS_KEY_EXPORT flag and the server 
>> replies
>> + * with `key-derivation tls-ekm` the RFC5705 key material exporter with the
>> + * label EXPORTER-OpenVPN-datakeys is used for the key data.
> 
> Did you request this label with IANA?

It is not really described well how this actually works, from what I
interpreted you need to write to the mailing list, which I did:
https://mailarchive.ietf.org/arch/msg/tls/IWFb5hLsqNlCcIMnEDDuPAkzXvg/

but I am lost what the next step would be.

> 
> Also, a user could now use --keying-material-exporter to export the data
> channel keys to a plugin. I'm inclined to say we should prevent that, by
> rejecting our internal label as a user label.



> 
>>   * @subsection key_generation_random Source of random material
>>   *
>> diff --git 

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

2020-10-09 Thread Steffan Karger
Hi,

On 25-08-2020 09:36, Arne Schwabe wrote:
> OpenVPN currently uses its own (based on TLS 1.0) key derivation
> mechanism to generate the 256 bytes key data in key2 struct that
> are then used used to generate encryption/hmac/iv vectors. While
> this mechanism is still secure, it is not state of the art.
> 
> Instead of modernising our own approach, this commit implements
> key derivation using the Keying Material Exporters API introduced
> by RFC 5705.

Thanks! I'm glad to see you're picking up the work I haven't been able
to find time for.

> We also use an opportunistic approach of negotiating the use of
> EKM (exported key material) through an IV_PROTO flag and prefer
> EKM to our own PRF if both client and server support it. The
> use of EKM is pushed to the client as part of NCP as
> key-derivation tls-ekm.
> 
> We still exchange the random data (112 bytes from client to server
> and 64 byte from server to client) for the OpenVPN PRF but
> do not use it. Removing that exchange would break the handshake
> and make a key-method 3 or similar necessary.
> 
> As a side effect, this makes a little bit easier to have a FIPS compatible
> version of OpenVPN since we do not rely on calling MD5 anymore.
> 
> Side note: this commit breaks the (not yet merged) WolfSSL support as it
> claims to support EKM in the OpenSSL compat API but always returns an error
> if you try to use it.
> 
> Signed-off-by: Arne Schwabe 

Signed off two times.

> Patch v2: rebase/change to V2 of EKM refactoring
> 
> Patch v3: add Changes.rst
> 
> Signed-off-by: Arne Schwabe 
> ---
>  Changes.rst  | 11 +++
>  doc/doxygen/doc_key_generation.h | 14 +++--
>  src/openvpn/crypto.h |  4 +++
>  src/openvpn/init.c   |  1 +
>  src/openvpn/multi.c  |  4 +++
>  src/openvpn/options.c| 14 +
>  src/openvpn/options.h|  3 ++
>  src/openvpn/push.c   |  5 ++-
>  src/openvpn/ssl.c| 54 
>  src/openvpn/ssl.h|  2 ++
>  src/openvpn/ssl_backend.h|  2 ++
>  src/openvpn/ssl_mbedtls.c|  7 ++---
>  12 files changed, 107 insertions(+), 14 deletions(-)
> 
> diff --git a/Changes.rst b/Changes.rst
> index f67e1d76..2a2829e7 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -1,3 +1,14 @@
> +Overview of changes in 2.6
> +==
> +
> +
> +New features
> +
> +Keying Material Exporters (RFC 5705) based key generation
> +As part of the cipher negotiation OpenVPN will automatically prefer
> +the RFC5705 based key material generation to the current custom
> +OpenVPN PRF. This feature requires OpenSSL or mbed TLS 2.18+.
> +
>  Overview of changes in 2.5
>  ==
>  
> diff --git a/doc/doxygen/doc_key_generation.h 
> b/doc/doxygen/doc_key_generation.h
> index 4bb9c708..cef773a9 100644
> --- a/doc/doxygen/doc_key_generation.h
> +++ b/doc/doxygen/doc_key_generation.h
> @@ -58,6 +58,12 @@
>   *
>   * @subsection key_generation_method_2 Key method 2
>   *
> + * There are two methods for generating key data when using key method 2
> + * the first is OpenVPN's traditional approach that exchanges random
> + * data and uses a PRF and the other is using the RFC5705 keying material
> + * exporter to generate the key material. For both methods the random
> + * data is exchange but only used in the traditional method.
> + *
>   * -# The client generates random material in the following amounts:
>   *- Pre-master secret: 48 bytes
>   *- Client's PRF seed for master secret: 32 bytes
> @@ -73,8 +79,12 @@
>   *server's random material.
>   *
>   * %Key method 2 %key expansion is performed by the \c
> - * generate_key_expansion() function.  Please refer to its source code for
> - * details of the %key expansion process.
> + * generate_key_expansion_openvpn_prf() function.  Please refer to its source
> + * code for details of the %key expansion process.
> + *
> + * When the client sends the IV_PROTO_TLS_KEY_EXPORT flag and the server 
> replies
> + * with `key-derivation tls-ekm` the RFC5705 key material exporter with the
> + * label EXPORTER-OpenVPN-datakeys is used for the key data.

Did you request this label with IANA?

Also, a user could now use --keying-material-exporter to export the data
channel keys to a plugin. I'm inclined to say we should prevent that, by
rejecting our internal label as a user label.

>   * @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

Sugestion: "data channel key