Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1836?usp=email
to review the following change.
Change subject: ssl: reject a pushed epoch data format tag with a non-AEAD
cipher
......................................................................
ssl: reject a pushed epoch data format tag with a non-AEAD cipher
The epoch data key format is defined for AEAD ciphers only. Both places
that enable it locally verify this - multi.c when picking the cipher to
push and ssl_ncp.c for p2p NCP - but the pulling side imports the
"aead-epoch" protocol flag without validating it against the cipher that
was actually negotiated.
A peer pushing "protocol-flags aead-epoch" together with a non-AEAD
cipher therefore makes us reach the M_FATAL in init_key_contexts() and
terminate the process. This can be triggered whenever a non-AEAD cipher
is part of our own --data-ciphers, which is not unusual in
configurations kept compatible with old peers, e.g.
data-ciphers AES-256-GCM:AES-256-CBC
Validate the combination in do_deferred_options(), next to the existing
data v2 check, so that the mismatch is reported as an OPTIONS ERROR and
the connection is restarted. Turn the now unreachable M_FATAL in
init_key_contexts() into a session error as well, so that no future code
path can promote this to a process exit.
Change-Id: Icc0721fd4a910110507d06b4e941e9e6dbb11e9f
Signed-off-by: Antonio Quartulli <[email protected]>
---
M src/openvpn/init.c
M src/openvpn/ssl.c
2 files changed, 18 insertions(+), 2 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/36/1836/1
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 0236886..84de986 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2711,6 +2711,18 @@
return false;
}
+ /* The epoch data format is defined for AEAD ciphers only. A peer may push
+ * the tag along with a non-AEAD cipher, so this has to be checked here
+ * rather than trusted */
+ if (epoch_data && !cipher_kt_mode_aead(c->options.ciphername))
+ {
+ msg(D_PUSH_ERRORS,
+ "OPTIONS ERROR: Epoch key data format tag requires an AEAD "
+ "cipher, but '%s' was negotiated.",
+ c->options.ciphername);
+ return false;
+ }
+
if (found & OPT_P_PUSH_MTU)
{
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 08ca306..1380b9f 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -1415,10 +1415,14 @@
{
if (!cipher_kt_mode_aead(key_type->cipher))
{
- msg(M_FATAL,
- "AEAD cipher (currently %s) "
+ /* The pulled options are validated in do_deferred_options(), so
+ * reaching this point means a code path escaped that check. Fail
+ * the session instead of the whole process */
+ msg(D_TLS_ERRORS,
+ "TLS Error: AEAD cipher (currently %s) "
"required for epoch data format.",
cipher_kt_name(key_type->cipher));
+ return false;
}
init_epoch_keys(ks, multi, key_type, server, key2);
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1836?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: Icc0721fd4a910110507d06b4e941e9e6dbb11e9f
Gerrit-Change-Number: 1836
Gerrit-PatchSet: 1
Gerrit-Owner: ordex <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
