Acked-by: Gert Doering <[email protected]>
I have not tested the actuall ASSERT() crash, but went through the
change asking myself "so, what will happen instead, now?"
- handle_data_channel_packet() will now ignore all keys that are
no longer KS_AUTH_TRUE, so if there is no other key, it will end
up in "TLS error: local/remote TLS keys are out of sync" and drop
the incoming packet (buf->len = 0).
- tls_select_encryption_key() will also ignore those keys, and possibly
return NULL. This is only called from tls_pre_encrypt(), which will
log "TLS Warning: no data channel send key available" in this case,
and drop the outgoing packlet (buf->len = 0).
Looking at tls_deauthenticate(), the problem described sounds real - this
function sets all keys to KS_AUTH_FALSE, without changing ks->state,
thus the next incoming or outgoing packet for that peer would trigger
the ASSERT() (and this looks like could happen on regular renegotiation,
like on a username or CN change, so this sounds like an actual DoS vector
againt a "master" server...).
The patch should fix this, without introducing undesired new side effects
(like, bypassing KS_AUTH_FALSE).
Subjected to a full set of server tests (passed).
Your patch has been applied to the master branch.
commit 75f9fb6f5b9bf57578086c3e42870bba4a914d7c
Author: Arne Schwabe
Date: Tue Dec 7 18:01:54 2021 +0100
Fix triggering assertion of ks->authenticated after tls_deauthenticate
Signed-off-by: Arne Schwabe <[email protected]>
Acked-by: Gert Doering <[email protected]>
Message-Id: <[email protected]>
URL:
https://www.mail-archive.com/[email protected]/msg23340.html
Signed-off-by: Gert Doering <[email protected]>
--
kind regards,
Gert Doering
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
