Since generating data channel keys does not happen when we have reach the
S_ACTIVE/S_GOT_KEY state anymore like it used to be before NCP, the
state that data channel keys deserves its own state in the TLS session
state machine.
The changes done by this commit are rather intrusive since they
move the key generation to a completely different place and also
rely on the state machine to decide if keys should be
generated rather than on the complicated conditions that were
implemented in the key_method_2_write/read methods.
A (intended) side effect of this change is that sessions that
are still in deferred state (ks->authenticated == KS_DEFERRED)
will not have data channel keys generated. This avoids corner
cases where a not fully authenticated sessions might leak data.
Signed-off-by: Arne Schwabe
Patch v2: rebased
Signed-off-by: Arne Schwabe
---
src/openvpn/forward.h| 2 +-
src/openvpn/init.c | 1 +
src/openvpn/ssl.c| 89 +---
src/openvpn/ssl.h| 10 +
src/openvpn/ssl_common.h | 9 +++-
5 files changed, 57 insertions(+), 54 deletions(-)
diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h
index 3461e6422..b8760099e 100644
--- a/src/openvpn/forward.h
+++ b/src/openvpn/forward.h
@@ -450,7 +450,7 @@ connection_established(struct context *c)
{
if (c->c2.tls_multi)
{
-return c->c2.tls_multi->multi_state >= CAS_CONNECT_DONE;
+return c->c2.tls_multi->multi_state >= CAS_WAITING_OPTIONS_IMPORT;
}
else
{
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 49c742928..4335d4870 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2202,6 +2202,7 @@ do_up(struct context *c, bool pulled_options, unsigned
int option_types_found)
}
c->c2.do_up_ran = true;
+c->c2.tls_multi->multi_state = CAS_CONNECT_DONE;
}
return true;
}
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index fd64b8d4e..b28d8edf8 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -788,6 +788,9 @@ state_name(int state)
case S_ERROR:
return "S_ERROR";
+case S_GENERATED_KEYS:
+return "S_GENERATED_KEYS";
+
default:
return "S_???";
}
@@ -1840,13 +1843,13 @@ key_ctx_update_implicit_iv(struct key_ctx *ctx, uint8_t
*key, size_t key_len)
* This erases the source material used to generate the data channel keys, and
* can thus be called only once per session.
*/
-static bool
+bool
tls_session_generate_data_channel_keys(struct tls_session *session)
{
bool ret = false;
struct key_state *ks = >key[KS_PRIMARY]; /* primary key */
-if (ks->authenticated == KS_AUTH_FALSE)
+if (ks->authenticated <= KS_AUTH_FALSE)
{
msg(D_TLS_ERRORS, "TLS Error: key_state not authenticated");
goto cleanup;
@@ -1862,6 +1865,9 @@ tls_session_generate_data_channel_keys(struct tls_session
*session)
tls_limit_reneg_bytes(session->opt->key_type.cipher,
>opt->renegotiate_bytes);
+/* set the state of the keys for the session to generated */
+ks->state = S_GENERATED_KEYS;
+
ret = true;
cleanup:
secure_memzero(ks->key_src, sizeof(*ks->key_src));
@@ -2375,31 +2381,6 @@ key_method_2_write(struct buffer *buf, struct tls_multi
*multi, struct tls_sessi
goto error;
}
-/*
- * Generate tunnel keys if we're a TLS server.
- *
- * If we're a p2mp server to allow NCP, the first key
- * generation is postponed until after the connect script finished and the
- * NCP options can be processed. Since that always happens at after connect
- * script options are available the CAS_CONNECT_DONE status is identical to
- * NCP options are processed and do not wait for NCP being finished.
- */
-if (ks->authenticated > KS_AUTH_FALSE && session->opt->server
-&& ((session->opt->mode == MODE_SERVER && multi->multi_state >=
CAS_CONNECT_DONE)
-|| (session->opt->mode == MODE_POINT_TO_POINT &&
!session->opt->pull)))
-{
-/* if key_id >= 1, is a renegotiation, so we use the already
established
- * parameters and do not need to delay anything. */
-
-/* key-id == 0 and multi_state >= CAS_CONNECT_DONE is a special case of
- * the server reusing the session of a reconnecting client. */
-if (!tls_session_generate_data_channel_keys(session))
-{
-msg(D_TLS_ERRORS, "TLS Error: server generate_key_expansion
failed");
-goto error;
-}
-}
-
return true;
error:
@@ -2599,21 +2580,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi
*multi, struct tls_sessio
setenv_del(session->opt->es, "exported_keying_material");
}
-
-/*
- * Generate tunnel keys if we're a client.
- * If --pull is enabled, the first key generation is postponed until after
the
- * pull/push, so we can process
