From: "Rémi Bernon" <rbernon@codeweavers.com>
Subject: [PATCH 1/5] winhttp: Move connect end checks out of the loop.
Message-Id: <20210215114723.3898056-1-rbernon@codeweavers.com>
Date: Mon, 15 Feb 2021 12:47:19 +0100

This is only done when InitializeSecurityContextW returns SEC_E_OK,
which will break out of the loop, and the checks forcefully break out
of the loop if any failed.

Signed-off-by: Rémi Bernon <rbernon@codeweavers.com>
---

This implements TLS re-handshake properly, instead of incorrectly
returning a client certificate error. It happens with Gears Tactics
online server, although the game still succeeds eventually (as TLS
doesn't mandate for clients to reply to re-handshake requests).

These probably lack some tests, but I couldn't find a way to write
something that's working. I'm sending them anyway, at least for the
discussion, in case I missed something.

It would need an SSL server that asks for a re-handshake, but I think
Wine current implementation lacks some features to create the server
side of such test. I may be wrong though, and maybe I just failed to
figure how to write it?

 dlls/winhttp/net.c | 66 ++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/dlls/winhttp/net.c b/dlls/winhttp/net.c
index 0cc2bb2bef7..46dc6bd21f0 100644
--- a/dlls/winhttp/net.c
+++ b/dlls/winhttp/net.c
@@ -341,53 +341,51 @@ DWORD netconn_secure_connect( struct netconn *conn, WCHAR *hostname, DWORD secur
         status = InitializeSecurityContextW(cred_handle, &ctx, hostname,  isc_req_flags, 0, 0, &in_desc,
                 0, NULL, &out_desc, &attrs, NULL);
         TRACE("InitializeSecurityContext ret %08x\n", status);
+        if(status == SEC_E_OK && in_bufs[1].BufferType == SECBUFFER_EXTRA)
+            FIXME("SECBUFFER_EXTRA not supported\n");
+    }
 
-        if(status == SEC_E_OK) {
-            if(in_bufs[1].BufferType == SECBUFFER_EXTRA)
-                FIXME("SECBUFFER_EXTRA not supported\n");
-
-            status = QueryContextAttributesW(&ctx, SECPKG_ATTR_STREAM_SIZES, &conn->ssl_sizes);
-            if(status != SEC_E_OK) {
-                WARN("Could not get sizes\n");
-                break;
-            }
+    heap_free(read_buf);
 
-            status = QueryContextAttributesW(&ctx, SECPKG_ATTR_REMOTE_CERT_CONTEXT, (void*)&cert);
-            if(status == SEC_E_OK) {
-                res = netconn_verify_cert(cert, hostname, security_flags, check_revocation);
-                CertFreeCertificateContext(cert);
-                if(res != ERROR_SUCCESS) {
-                    WARN("cert verify failed: %u\n", res);
-                    break;
-                }
-            }else {
-                WARN("Could not get cert\n");
-                break;
-            }
+    if(status != SEC_E_OK)
+        goto failed;
 
-            conn->ssl_buf = heap_alloc(conn->ssl_sizes.cbHeader + conn->ssl_sizes.cbMaximumMessage + conn->ssl_sizes.cbTrailer);
-            if(!conn->ssl_buf) {
-                res = ERROR_OUTOFMEMORY;
-                break;
-            }
-        }
+    status = QueryContextAttributesW(&ctx, SECPKG_ATTR_STREAM_SIZES, &conn->ssl_sizes);
+    if(status != SEC_E_OK) {
+        WARN("Could not get sizes\n");
+        goto failed;
     }
 
-    heap_free(read_buf);
+    status = QueryContextAttributesW(&ctx, SECPKG_ATTR_REMOTE_CERT_CONTEXT, (void*)&cert);
+    if(status != SEC_E_OK) {
+        WARN("Could not get cert\n");
+        goto failed;
+    }
 
-    if(status != SEC_E_OK || res != ERROR_SUCCESS) {
-        WARN("Failed to initialize security context: %08x\n", status);
-        heap_free(conn->ssl_buf);
-        conn->ssl_buf = NULL;
-        DeleteSecurityContext(&ctx);
-        return ERROR_WINHTTP_SECURE_CHANNEL_ERROR;
+    res = netconn_verify_cert(cert, hostname, security_flags, check_revocation);
+    CertFreeCertificateContext(cert);
+    if(res != ERROR_SUCCESS) {
+        WARN("cert verify failed: %u\n", res);
+        goto failed;
     }
 
+    conn->ssl_buf = heap_alloc(conn->ssl_sizes.cbHeader + conn->ssl_sizes.cbMaximumMessage + conn->ssl_sizes.cbTrailer);
+    if(!conn->ssl_buf) {
+        res = ERROR_OUTOFMEMORY;
+        goto failed;
+    }
 
     TRACE("established SSL connection\n");
     conn->secure = TRUE;
     conn->ssl_ctx = ctx;
     return ERROR_SUCCESS;
+
+failed:
+    WARN("Failed to initialize security context: %08x\n", status);
+    heap_free(conn->ssl_buf);
+    conn->ssl_buf = NULL;
+    DeleteSecurityContext(&ctx);
+    return ERROR_WINHTTP_SECURE_CHANNEL_ERROR;
 }
 
 static DWORD send_ssl_chunk( struct netconn *conn, const void *msg, size_t size )

-- 
2.30.0