plaisthos has uploaded this change for review. (
http://gerrit.openvpn.net/c/openvpn/+/1728?usp=email )
Change subject: Do not differentiate TLS server and client context
initialisation
......................................................................
Do not differentiate TLS server and client context initialisation
OpenSSL has the quite curious way of allowing to create contexts
that allow only server or only client. This creates extra
complications when we want to use both server and client SSL
objects and does not seem to have any advantages.
We later explicitly tell initialise the SSL objects to be a server or
client object in key_state_ssl_init via SSL_set_accept_state or
SSL_set_connect_state. If this is mismatched we end up getting an
error from OpenSSL ("called a function you should not call") that
that ends up calling a function that is not defined in that
TLS_method.
Looking into the OpenSSL source (IMPLEMENT_tls_meth_func) the main
difference between the methods is whether they have a proper
accept/connect or have the ssl_undefined_function that triggers the
"called a function you should not call".
Our mBed TLS code basically does give the SSL contet any personlity
of client or server until we the same area where the OpenSSL code
calls the set accept/connect state call.
Change-Id: Iaf1f3475e4f27a920c028cd73b1a2497953583d0
Signed-off-by: Arne Schwabe <[email protected]>
---
M src/openvpn/ssl.c
M src/openvpn/ssl_backend.h
M src/openvpn/ssl_mbedtls.c
M src/openvpn/ssl_mbedtls.h
M src/openvpn/ssl_openssl.c
M tests/unit_tests/openvpn/test_ssl.c
6 files changed, 17 insertions(+), 65 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/28/1728/1
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 6d58a3d..76f1788 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -525,18 +525,11 @@
struct tls_root_ctx *new_ctx;
ALLOC_OBJ_CLEAR(new_ctx, struct tls_root_ctx);
- if (options->tls_server)
- {
- tls_ctx_server_new(new_ctx);
+ tls_ctx_new(new_ctx);
- if (options->dh_file)
- {
- tls_ctx_load_dh_params(new_ctx, options->dh_file,
options->dh_file_inline);
- }
- }
- else /* if client */
+ if (options->tls_server && options->dh_file)
{
- tls_ctx_client_new(new_ctx);
+ tls_ctx_load_dh_params(new_ctx, options->dh_file,
options->dh_file_inline);
}
/* Restrict allowed certificate crypto algorithms */
diff --git a/src/openvpn/ssl_backend.h b/src/openvpn/ssl_backend.h
index 816fb9c..a6d57f2 100644
--- a/src/openvpn/ssl_backend.h
+++ b/src/openvpn/ssl_backend.h
@@ -117,18 +117,11 @@
int tls_version_max(void);
/**
- * Initialise a library-specific TLS context for a server.
+ * Initialise a library-specific TLS context.
*
* @param ctx TLS context to initialise
*/
-void tls_ctx_server_new(struct tls_root_ctx *ctx);
-
-/**
- * Initialises a library-specific TLS context for a client.
- *
- * @param ctx TLS context to initialise
- */
-void tls_ctx_client_new(struct tls_root_ctx *ctx);
+void tls_ctx_new(struct tls_root_ctx *ctx);
/**
* Frees the library-specific TLSv1 context
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index 8a0f7d2..2dba50a 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -88,7 +88,7 @@
}
void
-tls_ctx_server_new(struct tls_root_ctx *ctx)
+tls_ctx_new(struct tls_root_ctx *ctx)
{
ASSERT(NULL != ctx);
CLEAR(*ctx);
@@ -99,24 +99,9 @@
ALLOC_OBJ_CLEAR(ctx->ca_chain, mbedtls_x509_crt);
- ctx->endpoint = MBEDTLS_SSL_IS_SERVER;
ctx->initialised = true;
}
-void
-tls_ctx_client_new(struct tls_root_ctx *ctx)
-{
- ASSERT(NULL != ctx);
- CLEAR(*ctx);
-
-#if MBEDTLS_VERSION_NUMBER < 0x04000000
- ALLOC_OBJ_CLEAR(ctx->dhm_ctx, mbedtls_dhm_context);
-#endif
- ALLOC_OBJ_CLEAR(ctx->ca_chain, mbedtls_x509_crt);
-
- ctx->endpoint = MBEDTLS_SSL_IS_CLIENT;
- ctx->initialised = true;
-}
void
tls_ctx_free(struct tls_root_ctx *ctx)
@@ -1140,7 +1125,8 @@
/* Initialise SSL config */
ALLOC_OBJ_CLEAR(ks_ssl->ssl_config, mbedtls_ssl_config);
mbedtls_ssl_config_init(ks_ssl->ssl_config);
- mbedtls_ssl_config_defaults(ks_ssl->ssl_config, ssl_ctx->endpoint,
MBEDTLS_SSL_TRANSPORT_STREAM,
+ int endpoint = is_server ? MBEDTLS_SSL_IS_SERVER : MBEDTLS_SSL_IS_CLIENT;
+ mbedtls_ssl_config_defaults(ks_ssl->ssl_config, endpoint,
MBEDTLS_SSL_TRANSPORT_STREAM,
MBEDTLS_SSL_PRESET_DEFAULT);
#ifdef MBEDTLS_DEBUG_C
/* We only want to have mbed TLS generate debug level logging when we would
@@ -1529,7 +1515,7 @@
struct tls_root_ctx tls_ctx;
const int *ciphers = mbedtls_ssl_list_ciphersuites();
- tls_ctx_server_new(&tls_ctx);
+ tls_ctx_new(&tls_ctx);
tls_ctx_set_cert_profile(&tls_ctx, tls_cert_profile);
tls_ctx_restrict_ciphers(&tls_ctx, cipher_list);
diff --git a/src/openvpn/ssl_mbedtls.h b/src/openvpn/ssl_mbedtls.h
index 6b678b2..32d4f60 100644
--- a/src/openvpn/ssl_mbedtls.h
+++ b/src/openvpn/ssl_mbedtls.h
@@ -114,8 +114,6 @@
{
bool initialised; /**< True if the context has been initialised */
- int endpoint; /**< Whether or not this is a server or a client */
-
#if MBEDTLS_VERSION_NUMBER < 0x04000000
mbedtls_dhm_context *dhm_ctx; /**< Diffie-Helmann-Merkle context
*/
#endif
diff --git a/src/openvpn/ssl_openssl.c b/src/openvpn/ssl_openssl.c
index ef99b22..c5de165 100644
--- a/src/openvpn/ssl_openssl.c
+++ b/src/openvpn/ssl_openssl.c
@@ -100,37 +100,19 @@
}
void
-tls_ctx_server_new(struct tls_root_ctx *ctx)
+tls_ctx_new(struct tls_root_ctx *ctx)
{
ASSERT(NULL != ctx);
- ctx->ctx = SSL_CTX_new_ex(tls_libctx, NULL, SSLv23_server_method());
+ ctx->ctx = SSL_CTX_new_ex(tls_libctx, NULL, TLS_method());
if (ctx->ctx == NULL)
{
- crypto_msg(M_FATAL, "SSL_CTX_new SSLv23_server_method");
+ crypto_msg(M_FATAL, "SSL_CTX_new TLS_method");
}
if (ERR_peek_error() != 0)
{
- crypto_msg(M_WARN, "Warning: TLS server context initialisation "
- "has warnings.");
- }
-}
-
-void
-tls_ctx_client_new(struct tls_root_ctx *ctx)
-{
- ASSERT(NULL != ctx);
-
- ctx->ctx = SSL_CTX_new_ex(tls_libctx, NULL, SSLv23_client_method());
-
- if (ctx->ctx == NULL)
- {
- crypto_msg(M_FATAL, "SSL_CTX_new SSLv23_client_method");
- }
- if (ERR_peek_error() != 0)
- {
- crypto_msg(M_WARN, "Warning: TLS client context initialisation "
+ crypto_msg(M_WARN, "Warning: TLS context initialisation "
"has warnings.");
}
}
diff --git a/tests/unit_tests/openvpn/test_ssl.c
b/tests/unit_tests/openvpn/test_ssl.c
index 0e9cecf..d7d645f 100644
--- a/tests/unit_tests/openvpn/test_ssl.c
+++ b/tests/unit_tests/openvpn/test_ssl.c
@@ -175,7 +175,7 @@
struct gc_arena gc = gc_new();
struct tls_root_ctx ctx = { 0 };
- tls_ctx_client_new(&ctx);
+ tls_ctx_new(&ctx);
tls_ctx_load_cert_file(&ctx, unittest_cert, true);
openvpn_x509_cert_t *cert = NULL;
@@ -208,13 +208,13 @@
/* test loading of inlined cert and key.
* loading the key also checks that it matches the loaded certificate
*/
- tls_ctx_client_new(&ctx);
+ tls_ctx_new(&ctx);
tls_ctx_load_cert_file(&ctx, unittest_cert, true);
assert_int_equal(tls_ctx_load_priv_file(&ctx, unittest_key, true), 0);
tls_ctx_free(&ctx);
/* test loading of cert and key from file */
- tls_ctx_client_new(&ctx);
+ tls_ctx_new(&ctx);
tls_ctx_load_cert_file(&ctx, global_state.certfile, false);
assert_int_equal(tls_ctx_load_priv_file(&ctx, global_state.keyfile,
false), 0);
tls_ctx_free(&ctx);
@@ -252,7 +252,7 @@
string_mod(BSTR(&keyuri), CC_ANY, CC_BACKSLASH, '/');
#endif /* _WIN32 */
- tls_ctx_client_new(&ctx);
+ tls_ctx_new(&ctx);
tls_ctx_load_cert_file(&ctx, BSTR(&certuri), false);
assert_int_equal(tls_ctx_load_priv_file(&ctx, BSTR(&keyuri), false), 0);
tls_ctx_free(&ctx);
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1728?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Iaf1f3475e4f27a920c028cd73b1a2497953583d0
Gerrit-Change-Number: 1728
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
