winhttp: skip certificate check if unable to send request

In some circumstances (e.g. when proxies are involved), winhttp will fail to reach the WINHTTP_CALLBACK_STATUS_SENDING_REQUEST phase. If this occurs, we'll error with ERROR_WINHTTP_INCORRECT_HANDLE_STATE when attempting to query the server certificate context (see https://docs.microsoft.com/en-us/windows/win32/api/winhttp/nf-winhttp-winhttpsendrequest#remarks).

To avoid this, verify that WinHttpSendRequest has reached the WINHTTP_CALLBACK_STATUS_SENDING_REQUEST phase before checking the certificate. Since we're using WinHTTP in synchronous mode, we know for sure that once WinHttpSendRequest returns we've either sent it successfully or not.

NOTE: WINHTTP_CALLBACK_STATUS_SENDING_REQUEST appears to be deprecated with no direct replacement. WINHTTP_CALLBACK_STATUS_SENDREQUEST_COMPLETE is only available in async mode, and there doesn't appear to be a method of querying this flag outside of the status callback.
parent 592a3252
...@@ -111,7 +111,8 @@ typedef struct { ...@@ -111,7 +111,8 @@ typedef struct {
DWORD post_body_len; DWORD post_body_len;
unsigned sent_request : 1, unsigned sent_request : 1,
received_response : 1, received_response : 1,
chunked : 1; chunked : 1,
status_sending_request_reached: 1;
} winhttp_stream; } winhttp_stream;
typedef struct { typedef struct {
...@@ -713,12 +714,10 @@ static void CALLBACK winhttp_status( ...@@ -713,12 +714,10 @@ static void CALLBACK winhttp_status(
DWORD status; DWORD status;
GIT_UNUSED(connection); GIT_UNUSED(connection);
GIT_UNUSED(ctx);
GIT_UNUSED(info_len); GIT_UNUSED(info_len);
if (code != WINHTTP_CALLBACK_STATUS_SECURE_FAILURE) switch (code) {
return; case WINHTTP_CALLBACK_STATUS_SECURE_FAILURE:
status = *((DWORD *)info); status = *((DWORD *)info);
if ((status & WINHTTP_CALLBACK_STATUS_FLAG_CERT_CN_INVALID)) if ((status & WINHTTP_CALLBACK_STATUS_FLAG_CERT_CN_INVALID))
...@@ -737,6 +736,14 @@ static void CALLBACK winhttp_status( ...@@ -737,6 +736,14 @@ static void CALLBACK winhttp_status(
git_error_set(GIT_ERROR_HTTP, "security libraries could not be loaded"); git_error_set(GIT_ERROR_HTTP, "security libraries could not be loaded");
else else
git_error_set(GIT_ERROR_HTTP, "unknown security error %lu", status); git_error_set(GIT_ERROR_HTTP, "unknown security error %lu", status);
break;
case WINHTTP_CALLBACK_STATUS_SENDING_REQUEST:
((winhttp_stream *) ctx)->status_sending_request_reached = 1;
break;
}
} }
static int winhttp_connect( static int winhttp_connect(
...@@ -836,7 +843,12 @@ static int winhttp_connect( ...@@ -836,7 +843,12 @@ static int winhttp_connect(
goto on_error; goto on_error;
} }
if (WinHttpSetStatusCallback(t->connection, winhttp_status, WINHTTP_CALLBACK_FLAG_SECURE_FAILURE, 0) == WINHTTP_INVALID_STATUS_CALLBACK) { if (WinHttpSetStatusCallback(
t->connection,
winhttp_status,
WINHTTP_CALLBACK_FLAG_SECURE_FAILURE | WINHTTP_CALLBACK_FLAG_SEND_REQUEST,
0
) == WINHTTP_INVALID_STATUS_CALLBACK) {
git_error_set(GIT_ERROR_OS, "failed to set status callback"); git_error_set(GIT_ERROR_OS, "failed to set status callback");
goto on_error; goto on_error;
} }
...@@ -869,12 +881,12 @@ static int do_send_request(winhttp_stream *s, size_t len, bool chunked) ...@@ -869,12 +881,12 @@ static int do_send_request(winhttp_stream *s, size_t len, bool chunked)
success = WinHttpSendRequest(s->request, success = WinHttpSendRequest(s->request,
WINHTTP_NO_ADDITIONAL_HEADERS, 0, WINHTTP_NO_ADDITIONAL_HEADERS, 0,
WINHTTP_NO_REQUEST_DATA, 0, WINHTTP_NO_REQUEST_DATA, 0,
WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH, 0); WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH, (DWORD_PTR)s);
} else { } else {
success = WinHttpSendRequest(s->request, success = WinHttpSendRequest(s->request,
WINHTTP_NO_ADDITIONAL_HEADERS, 0, WINHTTP_NO_ADDITIONAL_HEADERS, 0,
WINHTTP_NO_REQUEST_DATA, 0, WINHTTP_NO_REQUEST_DATA, 0,
(DWORD)len, 0); (DWORD)len, (DWORD_PTR)s);
} }
if (success || GetLastError() != (DWORD)SEC_E_BUFFER_TOO_SMALL) if (success || GetLastError() != (DWORD)SEC_E_BUFFER_TOO_SMALL)
...@@ -911,7 +923,13 @@ static int send_request(winhttp_stream *s, size_t len, bool chunked) ...@@ -911,7 +923,13 @@ static int send_request(winhttp_stream *s, size_t len, bool chunked)
} }
} }
if (!request_failed || !cert_valid) { /*
* Only check the certificate if we were able to reach the sending request phase, or
* received a secure failure error. Otherwise, the server certificate won't be available
* since the request wasn't able to complete (e.g. proxy auth required)
*/
if (!cert_valid ||
(!request_failed && s->status_sending_request_reached)) {
git_error_clear(); git_error_clear();
if ((error = certificate_check(s, cert_valid)) < 0) { if ((error = certificate_check(s, cert_valid)) < 0) {
if (!git_error_last()) if (!git_error_last())
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment