From: Connor McAdams Subject: Re: [PATCH 5/8] secur32: Add support for setting DTLS timeouts. Message-Id: <20220126002455.GA3434199@connor-desktop> Date: Tue, 25 Jan 2022 19:24:55 -0500 In-Reply-To: <20220125153214.1402570-5-cmcadams@codeweavers.com> References: <20220125153214.1402570-1-cmcadams@codeweavers.com> <20220125153214.1402570-5-cmcadams@codeweavers.com> On Tue, Jan 25, 2022 at 10:32:11AM -0500, Connor McAdams wrote: > Add support for setting the DTLS timeout values, and set the > retransmission timeout value to 0 to allow for retransmission on each > call to schan_InitializeSecurityContext. > > Signed-off-by: Connor McAdams > --- > dlls/secur32/schannel.c | 6 ++++++ > dlls/secur32/schannel_gnutls.c | 22 ++++++++++++++++++++++ > dlls/secur32/secur32_priv.h | 8 ++++++++ > 3 files changed, 36 insertions(+) > > diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c > index fa5577d78e3..19e21da8d72 100644 > --- a/dlls/secur32/schannel.c > +++ b/dlls/secur32/schannel.c > @@ -809,6 +809,12 @@ static SECURITY_STATUS SEC_ENTRY schan_InitializeSecurityContextW( > else WARN("invalid buffer size %u\n", buffer->cbBuffer); > } > > + if (is_dtls_context(ctx)) > + { > + struct set_dtls_timeouts_params params = { ctx->transport.session, 0, 60000 }; > + GNUTLS_CALL( set_dtls_timeouts, ¶ms ); > + } > + > phNewContext->dwLower = handle; > phNewContext->dwUpper = 0; > } > diff --git a/dlls/secur32/schannel_gnutls.c b/dlls/secur32/schannel_gnutls.c > index 31fdb769677..ac51cbb5d9f 100644 > --- a/dlls/secur32/schannel_gnutls.c > +++ b/dlls/secur32/schannel_gnutls.c > @@ -60,6 +60,7 @@ static int (*pgnutls_cipher_get_block_size)(gnutls_cipher_algorithm_t); > static void (*pgnutls_transport_set_pull_timeout_function)(gnutls_session_t, > int (*)(gnutls_transport_ptr_t, unsigned int)); > static void (*pgnutls_dtls_set_mtu)(gnutls_session_t, unsigned int); > +static void (*pgnutls_dtls_set_timeouts)(gnutls_session_t, unsigned int, unsigned int); > > /* Not present in gnutls version < 3.2.0. */ > static int (*pgnutls_alpn_get_selected_protocol)(gnutls_session_t, gnutls_datum_t *); > @@ -198,6 +199,12 @@ static void compat_gnutls_dtls_set_mtu(gnutls_session_t session, unsigned int mt > FIXME("\n"); > } > > +static void compat_gnutls_dtls_set_timeouts(gnutls_session_t session, unsigned int retrans_timeout, > + unsigned int total_timeout) > +{ > + FIXME("\n"); > +} > + > static void init_schan_buffers(struct schan_buffers *s, const PSecBufferDesc desc, > int (*get_next_buffer)(const struct schan_transport *, struct schan_buffers *)) > { > @@ -989,6 +996,15 @@ static NTSTATUS schan_set_dtls_mtu( void *args ) > return SEC_E_OK; > } > > +static NTSTATUS schan_set_dtls_timeouts( void *args ) > +{ > + const struct set_dtls_timeouts_params *params = args; > + gnutls_session_t s = (gnutls_session_t)params->session; > + > + pgnutls_dtls_set_timeouts(s, params->retrans_timeout, params->total_timeout); > + return SEC_E_OK; > +} > + > static inline void reverse_bytes(BYTE *buf, ULONG len) > { > BYTE tmp; > @@ -1245,6 +1261,11 @@ static NTSTATUS process_attach( void *args ) > WARN("gnutls_dtls_set_mtu not found\n"); > pgnutls_dtls_set_mtu = compat_gnutls_dtls_set_mtu; > } > + if (!(pgnutls_dtls_set_timeouts = dlsym(libgnutls_handle, "gnutls_dtls_set_timeouts"))) > + { > + WARN("gnutls_dtls_set_timeouts not found\n"); > + pgnutls_dtls_set_timeouts = compat_gnutls_dtls_set_timeouts; > + } > if (!(pgnutls_privkey_export_x509 = dlsym(libgnutls_handle, "gnutls_privkey_export_x509"))) > { > WARN("gnutls_privkey_export_x509 not found\n"); > @@ -1308,6 +1329,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = > schan_set_application_protocols, > schan_set_dtls_mtu, > schan_set_session_target, > + schan_set_dtls_timeouts, > }; > > #endif /* SONAME_LIBGNUTLS */ > diff --git a/dlls/secur32/secur32_priv.h b/dlls/secur32/secur32_priv.h > index c17adc96dc6..64edc0581d6 100644 > --- a/dlls/secur32/secur32_priv.h > +++ b/dlls/secur32/secur32_priv.h > @@ -203,6 +203,13 @@ struct set_session_target_params > const char *target; > }; > > +struct set_dtls_timeouts_params > +{ > + schan_session session; > + unsigned int retrans_timeout; > + unsigned int total_timeout; > +}; > + > enum schan_funcs > { > unix_process_attach, > @@ -225,6 +232,7 @@ enum schan_funcs > unix_set_application_protocols, > unix_set_dtls_mtu, > unix_set_session_target, > + unix_set_dtls_timeouts, > }; > > #endif /* __SECUR32_PRIV_H__ */ > -- > 2.25.1 > All patches prior to this one are good, but this one and beyond will break things. Maybe this is the universe's way of telling me to send smaller patch sets. ;) Essentially, the issue seems to be that we need to go back to non-blocking mode with gnutls on DTLS contexts. We behave in a non-blocking manner to begin with, seeing as we return immediately from the pull_timeout callback. If we don't do this, it takes all sorts of effort to work around it, and make it not get into a weird state when no data is received. If it isn't in non-blocking mode, it doesn't seem to complete a handshake if retransmission is involved. Is there any good reason we switched away from GNUTLS_NONBLOCK in the first place? If so, I can try to work around it. But from my testing, having it enabled doesn't seem to cause issues. But maybe I'm missing something.