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

http_negotiate: do not close connection until negotiation is completed #3275

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@elia-tufarolo
Copy link
Contributor

elia-tufarolo commented Nov 15, 2018

Aim of this PR is to fix the HTTP POST using CURLAUTH_NEGOTIATE.

When connecting to an HTTP URL through a proxy that accepts Negotiation based authorization, libcURL fails to login to the proxy. The Negotiate protocol requires two round trips with the proxy. During the first one the client provides the Negotiate flags and the proxy answers with a Negotiate challenge, returning also HTTP 407. In the second round trip, the client provides the answer to the challenge and, if the proxy accepts it, it returns HTTP 200.
By setting authstatus->done always to true after emitting the Negotiate authorization header, so even after just emitting the header for the first request, libcURL closes the connection after the first round trip. Then it reopens it and executes the second roundtrip, thus answering to the challenge: the proxy, receiving a new connection, loses the context and is unable to associate the challenge response to the original challenge. For this reason it returns again 407.

The change is aimed at setting authstatus->done to true only if the client thinks it is at the last stage of its negotiation, to prevent libcURL from closing the socket while the negotiation is still ongoing. The negotiation can be considered as complete when the final status of the SPNEGO decoding is a completion status (GSS_S_COMPLETE for GSSAPI and SEC_E_OK for SSPI).

Please review.

http_negotiate: do not close connection until negotiation is completed
Aim of this PR is to fix the HTTP POST using CURLAUTH_NEGOTIATE.
@bagder

bagder approved these changes Nov 15, 2018

Copy link
Member

bagder left a comment

Makes sense to me!

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Nov 16, 2018

Thanks!

@bagder bagder closed this in 07ebaf8 Nov 16, 2018

@kdudka

This comment has been minimized.

Copy link
Collaborator

kdudka commented Dec 14, 2018

This change broke fedkg new-sources, which is a utility that uploads files to Fedora look-aside cache used by Fedora packagers:

https://bugzilla.redhat.com/1659329

I have not had enough time to properly analyze what happened. Just sharing the information we have at this point...

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Dec 18, 2018

#3384 also identified this PR as the reason. I think we need to revert this and reconsider this approach. Any thoughts @elia-tufarolo?

@bagder bagder reopened this Dec 18, 2018

@bagder

This comment has been minimized.

Copy link
Member

bagder commented Dec 19, 2018

An obvious little problem here is the lack of negotiate test cases in our suite. And I can't reproduce.

@kdudka I presume that tool is using negotiate then?

@kdudka

This comment has been minimized.

Copy link
Collaborator

kdudka commented Dec 19, 2018

Yes, Fedora uses Kerberos to authenticate users at certain services. Some basic info about their use of Kerberos is available here:

https://fedoraproject.org/wiki/Infrastructure/Kerberos

Sorry, I have not been able to look at this closer yet.

bagder added a commit that referenced this pull request Jan 4, 2019

Revert "http_negotiate: do not close connection until negotiation is …
…completed"

This reverts commit 07ebaf8.

This also reopens PR #3275 which brought the change now reverted.

Fixes #3384

bagder added a commit that referenced this pull request Jan 5, 2019

Revert "http_negotiate: do not close connection until negotiation is …
…completed"

This reverts commit 07ebaf8.

This also reopens PR #3275 which brought the change now reverted.

Fixes #3384

bagder added a commit that referenced this pull request Jan 7, 2019

Revert "http_negotiate: do not close connection until negotiation is …
…completed"

This reverts commit 07ebaf8.

This also reopens PR #3275 which brought the change now reverted.

Fixes #3384
Closes #3439
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.