Skip to content
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.

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

CONNECT fails if it can't send the entire request in one go #6950

Closed
ustcqidi opened this issue Apr 24, 2021 · 6 comments
Closed

CONNECT fails if it can't send the entire request in one go #6950

ustcqidi opened this issue Apr 24, 2021 · 6 comments

Comments

@ustcqidi
Copy link

@ustcqidi ustcqidi commented Apr 24, 2021

I did this

There are Proxy which need Kerberos Authentication, but the authentication flow sometimes failed . This not easy to reproduce.

After checking the wireshark captured file and source code of libcurl. We find the possible root cause of the issue.

CURLcode Curl_buffer_send(struct dynbuf *in,
                          struct connectdata *conn,
                          /* add the number of sent bytes to this
                             counter */
                          curl_off_t *bytes_written,
                          /* how much of the buffer contains body data */
                          size_t included_body_bytes,
                          int socketindex)
{
...
sendsize = CURLMIN(size, CURL_MAX_WRITE_SIZE);
...

else {
      if((size_t)amount != size)
        /* We have no continue-send mechanism now, fail. This can only happen
           when this function is used from the CONNECT sending function. We
           currently (stupidly) assume that the whole request is always sent
           away in the first single chunk.

           This needs FIXing.
        */
        return CURLE_SEND_ERROR;
    }
}

When the Kerberos Auth challenge is bigger than CURL_MAX_WRITE_SIZE (16384) , the request with auth will be failed. (Will be fallbacked to using NTLM in our user's proxy environment.)

image

curl/libcurl version

7.74.0

operating system

Windows

@bagder
Copy link
Member

@bagder bagder commented May 3, 2021

Any proposal on how to fix this?

@ustcqidi
Copy link
Author

@ustcqidi ustcqidi commented May 6, 2021

@bagder
We are fixing this by a workaround, just send the "real" size when sending "proxy-authorization header"

here is the pull request #7019

could you please help to review whether is it make sense ? Thanks.

@bagder
Copy link
Member

@bagder bagder commented May 6, 2021

btw, is this perchance in a CONNECT request?

@ustcqidi
Copy link
Author

@ustcqidi ustcqidi commented May 6, 2021

yes, in the function of CONNECT of http_proxy.c

static CURLcode CONNECT(struct connectdata *conn,
int sockindex,
const char *hostname,
int remote_port)

@bagder
Copy link
Member

@bagder bagder commented May 6, 2021

I figured out how to write a test case for this issue that reproduces your problem and shows that http_proxy.c:CONNECT needs to be fixed:

test363

This test limits the initial CONNECT request send to 100 bytes, which breaks curl.

@bagder bagder self-assigned this May 6, 2021
@bagder bagder changed the title Output Kerberos Auth failed if the challenge is bigger than CURL_MAX_WRITE_SIZE CONNECT fails if it can't send the entire request in one go May 6, 2021
bagder added a commit that referenced this issue May 6, 2021
Also added 'CURL_SMALLSENDS' to make Curl_write() send short packets,
which helped verifying this even more.

Add test 363 to verify.

Reported-by: ustcqidi on github
Fixes #6950
bagder added a commit that referenced this issue May 7, 2021
Also added 'CURL_SMALLSENDS' to make Curl_write() send short packets,
which helped verifying this even more.

Add test 363 to verify.

Reported-by: ustcqidi on github
Fixes #6950
Closes #7024
@ustcqidi
Copy link
Author

@ustcqidi ustcqidi commented May 8, 2021

@bagder
many thanks

@bagder bagder closed this in 51c0ebc May 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants