Hi,
Didn't find time to fully review, but I think this is moving into the
right direction. I did notice something I'd like you to consider:
On 06-07-2020 18:35, Arne Schwabe wrote:
> @@ -2466,7 +2466,7 @@ key_method_2_write(struct buffer *buf, struct
> tls_session *session)
> if (session->opt->server && !(session->opt->ncp_enabled
> && session->opt->mode == MODE_SERVER &&
> ks->key_id <= 0))
> {
> - if (ks->authenticated)
> + if (ks->authenticated != KS_AUTH_FALSE)
> {
Statements like these can be risky. I'm not sure how likely enum
ks_auth_state is to grow another state, but I could imagine someone
adding an explicit KS_AUTH_FAILED or so.
I think this patch should at least document the enum better, and clearly
state assumptions like "KS_AUTH_FALSE is the only state that represents
auth failure".
Or perhaps we can make is less fragile by modeling the enum like a state
machine and documenting allowed state transitions. That would allow you
to write things like "if (ks->authenticated < KS_AUTH_TRUE)" to express
"any state before authentication succeeded".
-Steffan
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
