Re: [Openvpn-devel] [PATCH v3 1/3] Refactor key_state_export_keying_material functions
Hi,
On 14-08-2020 16:51, 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
>
> Patch V2: Cache secrets for mbed TLS instead generating all ekms
> in the call back function
>
> Patch V3: comment is no longer a lie. (fixed doxygen)
>
> Signed-off-by: Arne Schwabe
> ---
> src/openvpn/ssl.c | 36 ++-
> src/openvpn/ssl_backend.h | 20 +++
> src/openvpn/ssl_mbedtls.c | 73 ---
> src/openvpn/ssl_mbedtls.h | 12 +--
> src/openvpn/ssl_openssl.c | 43 +--
> 5 files changed, 114 insertions(+), 70 deletions(-)
>
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index f16114c2..3fcaa25f 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -2412,6 +2412,40 @@ error:
> return false;
> }
>
> +static void
> +export_user_keying_material(struct key_state_ssl *ssl,
> +struct tls_session *session)
> +{
> +if (session->opt->ekm_size > 0)
> +{
> +unsigned int size = session->opt->ekm_size;
> +struct gc_arena gc = gc_new();
> +
> +unsigned char *ekm;
> +if ((ekm = key_state_export_keying_material(session,
> +session->opt->ekm_label,
> +
> session->opt->ekm_label_size,
> +session->opt->ekm_size,
> +&gc)))
> +{
> +unsigned int len = (size * 2) + 2;
> +
> +const char *key = format_hex_ex(ekm, size, len, 0, NULL, &gc);
> +setenv_str(session->opt->es, "exported_keying_material", key);
> +
> +dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s",
> + __func__, key);
> +secure_memzero(ekm, size);
> +}
> +else
> +{
> +msg(M_WARN, "WARNING: Export keying material failed!");
> +setenv_del(session->opt->es, "exported_keying_material");
> +}
> +gc_free(&gc);
> +}
> +}
> +
> /**
> * Handle reading key data, peer-info, username/password, OCC
> * from the TLS control channel (cleartext).
> @@ -2541,7 +2575,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi
> *multi, struct tls_sessio
> if ((ks->authenticated > KS_AUTH_FALSE)
> && plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL))
> {
> -key_state_export_keying_material(&ks->ks_ssl, session);
> +export_user_keying_material(&ks->ks_ssl, session);
>
> if (plugin_call(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL,
> NULL, NULL, session->opt->es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
> {
> diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
> index 7f52ab1e..cf9fba25 100644
> --- a/src/openvpn/ssl_backend.h
> +++ b/src/openvpn/ssl_backend.h
> @@ -394,13 +394,21 @@ void backend_tls_ctx_reload_crl(struct tls_root_ctx
> *ssl_ctx,
> * derived from existing TLS channel. This exported keying material can then
> be
> * used for a variety of purposes.
> *
> - * @param ks_ssl The SSL channel's state info
> * @param session The session associated with the given key_state
> - */
> -
> -void
> -key_state_export_keying_material(struct key_state_ssl *ks_ssl,
> - struct tls_session *session)
> __attribute__((nonnull));
> + * @param labelThe label to use when exporting the key
> + * @param label_size The size of the label to use when exporting the key
> + * @param ekm_size THe size of the exported/returned key material
> + * @param gc gc_arena that might be used to allocate the string
> + * returned
> + * @returnsThe exported key material, the caller may zero the
> + * string but should not free it
> + */
> +
> +unsigned char*
> +key_state_export_keying_material(struct tls_session *session,
> + const char* label, size_t label_size,
> + size_t ekm_size,
> + struct gc_arena *gc)
> __attribute__((nonnull));
>
> /**/
> /** @addtogroup control_tls
> diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
> index 9c874788..4287b59e 100644
> --- a/src/openvpn/ssl_mbedtls.c
> +++ b/src/openvpn/ssl_mbedtls.c
> @@ -206,51 +206,54 @@ mbedtls_ssl_export_keys_cb(void *p_expkey, const
> unsigned char *ms,
> {
> struct tls_session *session = p_expkey;
> struct key_state_ssl *ks_ssl =
[Openvpn-devel] [PATCH v3 1/3] Refactor key_state_export_keying_material functions
This refactors the common code between mbed SSL and OpenSSL into
export_user_keying_material and also prepares the backend functions
to export more than one key.
Also fix checking the return value of SSL_export_keying_material
only 1 is a sucess, -1 is also an error.
Signed-off-by: Arne Schwabe
Patch V2: Cache secrets for mbed TLS instead generating all ekms
in the call back function
Patch V3: comment is no longer a lie. (fixed doxygen)
Signed-off-by: Arne Schwabe
---
src/openvpn/ssl.c | 36 ++-
src/openvpn/ssl_backend.h | 20 +++
src/openvpn/ssl_mbedtls.c | 73 ---
src/openvpn/ssl_mbedtls.h | 12 +--
src/openvpn/ssl_openssl.c | 43 +--
5 files changed, 114 insertions(+), 70 deletions(-)
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index f16114c2..3fcaa25f 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2412,6 +2412,40 @@ error:
return false;
}
+static void
+export_user_keying_material(struct key_state_ssl *ssl,
+struct tls_session *session)
+{
+if (session->opt->ekm_size > 0)
+{
+unsigned int size = session->opt->ekm_size;
+struct gc_arena gc = gc_new();
+
+unsigned char *ekm;
+if ((ekm = key_state_export_keying_material(session,
+session->opt->ekm_label,
+
session->opt->ekm_label_size,
+session->opt->ekm_size,
+&gc)))
+{
+unsigned int len = (size * 2) + 2;
+
+const char *key = format_hex_ex(ekm, size, len, 0, NULL, &gc);
+setenv_str(session->opt->es, "exported_keying_material", key);
+
+dmsg(D_TLS_DEBUG_MED, "%s: exported keying material: %s",
+ __func__, key);
+secure_memzero(ekm, size);
+}
+else
+{
+msg(M_WARN, "WARNING: Export keying material failed!");
+setenv_del(session->opt->es, "exported_keying_material");
+}
+gc_free(&gc);
+}
+}
+
/**
* Handle reading key data, peer-info, username/password, OCC
* from the TLS control channel (cleartext).
@@ -2541,7 +2575,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi
*multi, struct tls_sessio
if ((ks->authenticated > KS_AUTH_FALSE)
&& plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL))
{
-key_state_export_keying_material(&ks->ks_ssl, session);
+export_user_keying_material(&ks->ks_ssl, session);
if (plugin_call(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL, NULL,
NULL, session->opt->es) != OPENVPN_PLUGIN_FUNC_SUCCESS)
{
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index 7f52ab1e..cf9fba25 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -394,13 +394,21 @@ void backend_tls_ctx_reload_crl(struct tls_root_ctx
*ssl_ctx,
* derived from existing TLS channel. This exported keying material can then be
* used for a variety of purposes.
*
- * @param ks_ssl The SSL channel's state info
* @param session The session associated with the given key_state
- */
-
-void
-key_state_export_keying_material(struct key_state_ssl *ks_ssl,
- struct tls_session *session)
__attribute__((nonnull));
+ * @param labelThe label to use when exporting the key
+ * @param label_size The size of the label to use when exporting the key
+ * @param ekm_size THe size of the exported/returned key material
+ * @param gc gc_arena that might be used to allocate the string
+ * returned
+ * @returnsThe exported key material, the caller may zero the
+ * string but should not free it
+ */
+
+unsigned char*
+key_state_export_keying_material(struct tls_session *session,
+ const char* label, size_t label_size,
+ size_t ekm_size,
+ struct gc_arena *gc) __attribute__((nonnull));
/**/
/** @addtogroup control_tls
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 9c874788..4287b59e 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -206,51 +206,54 @@ mbedtls_ssl_export_keys_cb(void *p_expkey, const unsigned
char *ms,
{
struct tls_session *session = p_expkey;
struct key_state_ssl *ks_ssl = &session->key[KS_PRIMARY].ks_ssl;
-unsigned char client_server_random[64];
+struct tls_key_cache *cache = &ks_ssl->tls_key_cache;
-ks_ssl->exported_key_material = gc_malloc(session->opt->ekm_size,
- true, NULL);
+static_assert(sizeof(
