[Openvpn-devel,v2] ssl: reject a pushed epoch data format tag with a non-AEAD cipher
Commit Message
From: Antonio Quartulli <antonio@mandelbit.com>
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 <antonio@mandelbit.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1836
---
This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1836
This mail reflects revision 2 of this Change.
Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>
@@ -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)
{
@@ -1413,10 +1413,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 KEY_GEN_FAILED;
}
init_epoch_keys(ks, multi, key_type, server, key2);
}