[PATCH] secur32: Split push/pull schannel transports

Anton Romanov theli.ua at gmail.com
Wed Sep 6 00:23:07 CDT 2017


On Sat, Aug 5, 2017 at 11:27 AM, Anton Romanov <theli.ua at gmail.com> wrote:
> This makes schan_EncryptMessage/schan_DecryptMessage thread safe between
> each other.
> MSDN states that it's supposed to be thread safe.
> Fixes Magic The Gathering: Online crash:
> https://bugs.winehq.org/show_bug.cgi?id=43453
>
> Signed-off-by: Anton Romanov <theli.ua at gmail.com>
> ---
>  dlls/secur32/schannel.c        | 33 +++++++++++++++++++--------------
>  dlls/secur32/schannel_gnutls.c | 12 +++++++-----
>  dlls/secur32/schannel_macosx.c | 21 ++++++++++++---------
>  dlls/secur32/secur32_priv.h    |  5 +++--
>  4 files changed, 41 insertions(+), 30 deletions(-)
>
> diff --git a/dlls/secur32/schannel.c b/dlls/secur32/schannel.c
> index 82374efd55..1f938e1a37 100644
> --- a/dlls/secur32/schannel.c
> +++ b/dlls/secur32/schannel.c
> @@ -59,6 +59,8 @@ struct schan_context
>      schan_imp_session session;
>      ULONG req_ctx_attr;
>      const CERT_CONTEXT *cert;
> +    struct schan_transport *push_transport;
> +    struct schan_transport *pull_transport;
>  };
>
>  static struct schan_handle *schan_handle_table;
> @@ -895,7 +897,7 @@ static SECURITY_STATUS SEC_ENTRY schan_InitializeSecurityContextW(
>      init_schan_buffers(&transport.in, pInput, schan_init_sec_ctx_get_next_input_buffer);
>      transport.in.limit = expected_size;
>      init_schan_buffers(&transport.out, pOutput, schan_init_sec_ctx_get_next_output_buffer);
> -    schan_imp_set_session_transport(ctx->session, &transport);
> +    schan_imp_set_session_transports(ctx->session, &transport, &transport);
>
>      /* Perform the TLS handshake */
>      ret = schan_imp_handshake(ctx->session);
> @@ -927,6 +929,13 @@ static SECURITY_STATUS SEC_ENTRY schan_InitializeSecurityContextW(
>      if (ctx->req_ctx_attr & ISC_REQ_STREAM)
>          *pfContextAttr |= ISC_RET_STREAM;
>
> +    ctx->push_transport = HeapAlloc(GetProcessHeap(), 0, sizeof(struct schan_transport));
> +    ctx->push_transport->ctx = ctx;
> +    ctx->pull_transport = HeapAlloc(GetProcessHeap(), 0, sizeof(struct schan_transport));
> +    ctx->pull_transport->ctx = ctx;
> +    schan_imp_set_session_transports(ctx->session, ctx->push_transport,
> +            ctx->pull_transport);
> +
>      return ret;
>  }
>
> @@ -1205,7 +1214,6 @@ static int schan_encrypt_message_get_next_buffer_token(const struct schan_transp
>  static SECURITY_STATUS SEC_ENTRY schan_EncryptMessage(PCtxtHandle context_handle,
>          ULONG quality, PSecBufferDesc message, ULONG message_seq_no)
>  {
> -    struct schan_transport transport;
>      struct schan_context *ctx;
>      struct schan_buffers *b;
>      SECURITY_STATUS status;
> @@ -1235,13 +1243,11 @@ static SECURITY_STATUS SEC_ENTRY schan_EncryptMessage(PCtxtHandle context_handle
>      data = HeapAlloc(GetProcessHeap(), 0, data_size);
>      memcpy(data, buffer->pvBuffer, data_size);
>
> -    transport.ctx = ctx;
> -    init_schan_buffers(&transport.in, NULL, NULL);
> +    init_schan_buffers(&ctx->push_transport->in, NULL, NULL);
>      if (schan_find_sec_buffer_idx(message, 0, SECBUFFER_STREAM_HEADER) != -1)
> -        init_schan_buffers(&transport.out, message, schan_encrypt_message_get_next_buffer);
> +        init_schan_buffers(&ctx->push_transport->out, message, schan_encrypt_message_get_next_buffer);
>      else
> -        init_schan_buffers(&transport.out, message, schan_encrypt_message_get_next_buffer_token);
> -    schan_imp_set_session_transport(ctx->session, &transport);
> +        init_schan_buffers(&ctx->push_transport->out, message, schan_encrypt_message_get_next_buffer_token);
>
>      length = data_size;
>      status = schan_imp_send(ctx->session, data, &length);
> @@ -1251,7 +1257,7 @@ static SECURITY_STATUS SEC_ENTRY schan_EncryptMessage(PCtxtHandle context_handle
>      if (length != data_size)
>          status = SEC_E_INTERNAL_ERROR;
>
> -    b = &transport.out;
> +    b = &ctx->push_transport->out;
>      b->desc->pBuffers[b->current_buffer_idx].cbBuffer = b->offset;
>      HeapFree(GetProcessHeap(), 0, data);
>
> @@ -1327,7 +1333,6 @@ static void schan_decrypt_fill_buffer(PSecBufferDesc message, ULONG buffer_type,
>  static SECURITY_STATUS SEC_ENTRY schan_DecryptMessage(PCtxtHandle context_handle,
>          PSecBufferDesc message, ULONG message_seq_no, PULONG quality)
>  {
> -    struct schan_transport transport;
>      struct schan_context *ctx;
>      SecBuffer *buffer;
>      SIZE_T data_size;
> @@ -1371,11 +1376,9 @@ static SECURITY_STATUS SEC_ENTRY schan_DecryptMessage(PCtxtHandle context_handle
>      data_size = expected_size - 5;
>      data = HeapAlloc(GetProcessHeap(), 0, data_size);
>
> -    transport.ctx = ctx;
> -    init_schan_buffers(&transport.in, message, schan_decrypt_message_get_next_buffer);
> -    transport.in.limit = expected_size;
> -    init_schan_buffers(&transport.out, NULL, NULL);
> -    schan_imp_set_session_transport(ctx->session, &transport);
> +    init_schan_buffers(&ctx->pull_transport->in, message, schan_decrypt_message_get_next_buffer);
> +    ctx->pull_transport->in.limit = expected_size;
> +    init_schan_buffers(&ctx->pull_transport->out, NULL, NULL);
>
>      while (received < data_size)
>      {
> @@ -1573,6 +1576,8 @@ void SECUR32_deinitSchannelSP(void)
>          {
>              struct schan_context *ctx = schan_free_handle(i, SCHAN_HANDLE_CTX);
>              schan_imp_dispose_session(ctx->session);
> +            HeapFree(GetProcessHeap(), 0, ctx->push_transport);
> +            HeapFree(GetProcessHeap(), 0, ctx->pull_transport);
>              HeapFree(GetProcessHeap(), 0, ctx);
>          }
>      }
> diff --git a/dlls/secur32/schannel_gnutls.c b/dlls/secur32/schannel_gnutls.c
> index bc3bbaf67d..97b2c2d767 100644
> --- a/dlls/secur32/schannel_gnutls.c
> +++ b/dlls/secur32/schannel_gnutls.c
> @@ -73,7 +73,7 @@ MAKE_FUNCPTR(gnutls_record_send);
>  MAKE_FUNCPTR(gnutls_server_name_set);
>  MAKE_FUNCPTR(gnutls_transport_get_ptr);
>  MAKE_FUNCPTR(gnutls_transport_set_errno);
> -MAKE_FUNCPTR(gnutls_transport_set_ptr);
> +MAKE_FUNCPTR(gnutls_transport_set_ptr2);
>  MAKE_FUNCPTR(gnutls_transport_set_pull_function);
>  MAKE_FUNCPTR(gnutls_transport_set_push_function);
>  #undef MAKE_FUNCPTR
> @@ -214,11 +214,13 @@ void schan_imp_dispose_session(schan_imp_session session)
>      pgnutls_deinit(s);
>  }
>
> -void schan_imp_set_session_transport(schan_imp_session session,
> -                                     struct schan_transport *t)
> +void schan_imp_set_session_transports(schan_imp_session session,
> +                                     struct schan_transport *t_push,
> +                                     struct schan_transport *t_pull)
>  {
>      gnutls_session_t s = (gnutls_session_t)session;
> -    pgnutls_transport_set_ptr(s, (gnutls_transport_ptr_t)t);
> +    pgnutls_transport_set_ptr2(s, (gnutls_transport_ptr_t)t_pull,
> +            (gnutls_transport_ptr_t)t_push);
>  }
>
>  void schan_imp_set_session_target(schan_imp_session session, const char *target)
> @@ -568,7 +570,7 @@ BOOL schan_imp_init(void)
>      LOAD_FUNCPTR(gnutls_server_name_set)
>      LOAD_FUNCPTR(gnutls_transport_get_ptr)
>      LOAD_FUNCPTR(gnutls_transport_set_errno)
> -    LOAD_FUNCPTR(gnutls_transport_set_ptr)
> +    LOAD_FUNCPTR(gnutls_transport_set_ptr2)
>      LOAD_FUNCPTR(gnutls_transport_set_pull_function)
>      LOAD_FUNCPTR(gnutls_transport_set_push_function)
>  #undef LOAD_FUNCPTR
> diff --git a/dlls/secur32/schannel_macosx.c b/dlls/secur32/schannel_macosx.c
> index 7f38133b4b..4cd562411b 100644
> --- a/dlls/secur32/schannel_macosx.c
> +++ b/dlls/secur32/schannel_macosx.c
> @@ -184,7 +184,8 @@ enum {
>
>  struct mac_session {
>      SSLContextRef context;
> -    struct schan_transport *transport;
> +    struct schan_transport *push_transport;
> +    struct schan_transport *pull_transport;
>  };
>
>
> @@ -631,9 +632,9 @@ static OSStatus schan_pull_adapter(SSLConnectionRef transport, void *buff,
>      int status;
>      OSStatus ret;
>
> -    TRACE("(%p/%p, %p, %p/%lu)\n", s, s->transport, buff, buff_len, *buff_len);
> +    TRACE("(%p/%p, %p, %p/%lu)\n", s, s->pull_transport, buff, buff_len, *buff_len);
>
> -    status = schan_pull(s->transport, buff, buff_len);
> +    status = schan_pull(s->pull_transport, buff, buff_len);
>      if (status == 0)
>      {
>          if (*buff_len == 0)
> @@ -690,9 +691,9 @@ static OSStatus schan_push_adapter(SSLConnectionRef transport, const void *buff,
>      int status;
>      OSStatus ret;
>
> -    TRACE("(%p/%p, %p, %p/%lu)\n", s, s->transport, buff, buff_len, *buff_len);
> +    TRACE("(%p/%p, %p, %p/%lu)\n", s, s->push_transport, buff, buff_len, *buff_len);
>
> -    status = schan_push(s->transport, buff, buff_len);
> +    status = schan_push(s->push_transport, buff, buff_len);
>      if (status == 0)
>      {
>          TRACE("Pushed %lu bytes\n", *buff_len);
> @@ -806,14 +807,16 @@ void schan_imp_dispose_session(schan_imp_session session)
>      HeapFree(GetProcessHeap(), 0, s);
>  }
>
> -void schan_imp_set_session_transport(schan_imp_session session,
> -                                     struct schan_transport *t)
> +void schan_imp_set_session_transports(schan_imp_session session,
> +                                     struct schan_transport *t_push,
> +                                     struct schan_transport *t_pull)
>  {
>      struct mac_session *s = (struct mac_session*)session;
>
> -    TRACE("(%p/%p, %p)\n", s, s->context, t);
> +    TRACE("(%p/%p, %p, %p)\n", s, s->context, t_push, t_pull);
>
> -    s->transport = t;
> +    s->push_transport = t_push;
> +    s->pull_transport = t_pull;
>  }
>
>  void schan_imp_set_session_target(schan_imp_session session, const char *target)
> diff --git a/dlls/secur32/secur32_priv.h b/dlls/secur32/secur32_priv.h
> index 9a60d65788..0bc8a57c1a 100644
> --- a/dlls/secur32/secur32_priv.h
> +++ b/dlls/secur32/secur32_priv.h
> @@ -246,8 +246,9 @@ extern schan_imp_session schan_session_for_transport(struct schan_transport* t)
>  /* schannel implementation interface */
>  extern BOOL schan_imp_create_session(schan_imp_session *session, schan_credentials *cred) DECLSPEC_HIDDEN;
>  extern void schan_imp_dispose_session(schan_imp_session session) DECLSPEC_HIDDEN;
> -extern void schan_imp_set_session_transport(schan_imp_session session,
> -                                            struct schan_transport *t) DECLSPEC_HIDDEN;
> +extern void schan_imp_set_session_transports(schan_imp_session session,
> +                                            struct schan_transport *t_push,
> +                                            struct schan_transport *t_pull) DECLSPEC_HIDDEN;
>  extern void schan_imp_set_session_target(schan_imp_session session, const char *target) DECLSPEC_HIDDEN;
>  extern SECURITY_STATUS schan_imp_handshake(schan_imp_session session) DECLSPEC_HIDDEN;
>  extern unsigned int schan_imp_get_session_cipher_block_size(schan_imp_session session) DECLSPEC_HIDDEN;
> --
> 2.11.0
>

So, it's been a month now. Is there something I could do to have this accepted?

Thanks.



More information about the wine-devel mailing list