Skip to content

Resend POST requests for Kerberos in WinHTTP #2529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
chescock wants to merge 3 commits into from
Closed

Resend POST requests for Kerberos in WinHTTP #2529

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3df6395
If we are using Negotiate, POST requests with a body may fail with ER…
chescock Aug 6, 2014
5709f84
Send 0000 instead of an empty HTTP request.
chescock Aug 20, 2014
39d42d8
Provide the request data for single writes in WinHttpSendRequest inst…
chescock Aug 20, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 44 additions & 13 deletions src/transports/winhttp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ typedef struct {
unsigned chunk_buffer_len;
HANDLE post_body;
DWORD post_body_len;
char *small_post_body;
unsigned sent_request : 1,
received_response : 1,
chunked : 1;
Expand Down Expand Up @@ -798,37 +799,32 @@ static int winhttp_stream_write_single(
{
winhttp_stream *s = (winhttp_stream *)stream;
winhttp_subtransport *t = OWNING_SUBTRANSPORT(s);
DWORD bytes_written;

if (!s->request && winhttp_stream_connect(s) < 0)
return -1;

/* This implementation of write permits only a single call. */
if (s->sent_request) {
if (s->sent_request || s->small_post_body) {
giterr_set(GITERR_NET, "Subtransport configured for only one write");
return -1;
}

/* The buffer passed to WinHttpSendRequest must be valid until
* WinHttpReceiveResponse is called, so make a copy */
s->small_post_body = git__malloc(len);
GITERR_CHECK_ALLOC(s->small_post_body);
memcpy(s->small_post_body, buffer, len);

if (!WinHttpSendRequest(s->request,
WINHTTP_NO_ADDITIONAL_HEADERS, 0,
WINHTTP_NO_REQUEST_DATA, 0,
s->small_post_body, (DWORD)len,
(DWORD)len, 0)) {
giterr_set(GITERR_OS, "Failed to send request");
return -1;
}

s->sent_request = 1;

if (!WinHttpWriteData(s->request,
(LPCVOID)buffer,
(DWORD)len,
&bytes_written)) {
giterr_set(GITERR_OS, "Failed to write data");
return -1;
}

assert((DWORD)len == bytes_written);

return 0;
}

Expand Down Expand Up @@ -951,6 +947,36 @@ static int winhttp_stream_write_chunked(
return -1;
}

/* If we are using Negotiate, POST requests with a body may fail with ERROR_WINHTTP_RESEND_REQUEST
* We first send an empty request to get a 401 without writing the body twice */
if (t->cred &&
t->cred->credtype == GIT_CREDTYPE_DEFAULT &&
t->auth_mechanism == GIT_WINHTTP_AUTH_NEGOTIATE &&
s->verb == post_verb) {
DWORD bytes_written;

if (!WinHttpSendRequest(s->request,
WINHTTP_NO_ADDITIONAL_HEADERS, 0,
WINHTTP_NO_REQUEST_DATA, 0,
WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH, 0)) {
giterr_set(GITERR_OS, "Failed to send empty request");
return -1;
}

if (!WinHttpWriteData(s->request,
"4\r\n0000\r\n0\r\n\r\n", 14,

@carlosmn carlosmn Aug 29, 2014

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line looks off. Are you trying to do HTTP chunking by hand? Isn't WinHTTP supposed to do that? Do we even want chunking for this? We should be able to simply tell WinHTTP to send the flush packet since we know what length we want.

@chescock chescock Sep 10, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that code is doing HTTP chunking by hand. There is similar code already in write_chunk and winhttp_stream_read. It seems that the way you do chunking in WinHTTP is by adding the header manually and then writing the chunk boundaries manually.

The documentation for WinHttpSendRequest says that “dwTotalLength must not change between calls to WinHttpSendRequest for the same request”, and we need to use the same request handle in order to preserve the authentication information. When using chunked encoding, dwTotalLength is set to WINHTTP_IGNORE_REQUEST_TOTAL_LENGTH and so we’re able to send requests of different lengths, but I don’t see a way to use normal encoding if we’re trying to send an empty request followed by a real request on the same request handle.

&bytes_written)) {
giterr_set(GITERR_OS, "Failed to write empty request");
return -1;
}

if (!WinHttpReceiveResponse(s->request, NULL) &&
GetLastError() != ERROR_WINHTTP_RESEND_REQUEST) {
giterr_set(GITERR_OS, "Failed to read empty request");
return -1;
}
}

if (!WinHttpSendRequest(s->request,
WINHTTP_NO_ADDITIONAL_HEADERS, 0,
WINHTTP_NO_REQUEST_DATA, 0,
Expand Down Expand Up @@ -1019,6 +1045,11 @@ static void winhttp_stream_free(git_smart_subtransport_stream *stream)
s->post_body = NULL;
}

if (s->small_post_body) {
git__free(s->small_post_body);
s->small_post_body = NULL;
}

if (s->request_uri) {
git__free(s->request_uri);
s->request_uri = NULL;
Expand Down